PATCH: Fix gnu-efi-3.0b-041222 for ia32

I don't know where I should report gnu-efi bug. The ia32
support in
gnu-efi-3.0b-041222 doesn't work with newer gcc and
binutils. The
problem is in the linker script. The new gcc may put
functions in
.text* sections. The newer linker handles orphaned sections
differently. I tested this patch on both ia32 and ia64 with
different
versions of gcc and binutils by building elilo 3.6. Elilo
3.6 loads
and run kernel correctly on ia64. It seems to work find on
ia32.

Thanks.


H.J.
--- gnu-efi/gnuefi/elf_ia32_efi.lds.ia32	2004-12-22
07:52:28.000000000 -0800
+++ gnu-efi/gnuefi/elf_ia32_efi.lds	2006-03-17
11:47:21.000000000 -0800
 -9,7
+9,11  SECTIONS
   . = ALIGN(4096);
   .text :
   {
-   *(.text)
+   *(.text*)
+  }
+  .reloc :
+  {
+   *(.reloc)
   }
   . = ALIGN(4096);
   .data :
 -48,12
+52,9  SECTIONS
   . = ALIGN(4096);
   .dynstr   : { *(.dynstr) }
   . = ALIGN(4096);
-  .ignored.reloc :
+  /DISCARD/ :
   {
     *(.rel.reloc)
-  }
-  .ignored :
-  {
     *(.eh_frame)
   }
 }
--- gnu-efi/gnuefi/elf_ia64_efi.lds.ia32	2004-12-22
10:18:10.000000000 -0800
+++ gnu-efi/gnuefi/elf_ia64_efi.lds	2006-03-17
11:51:51.000000000 -0800
 -59,13
+59,10  SECTIONS
   .dynsym   : { *(.dynsym) }
   . = ALIGN(4096);
   .dynstr   : { *(.dynstr) }
-  .ignored_reloc :
+  /DISCARD/ :
   {
     *(.rela.plabel)
     *(.rela.reloc)
-  }
-  /DISCARD/ :
-  {
     *(.IA_64.unwind*)
     *(.IA64.unwind*)
   }
-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

On Fri, 2006-03-17 at 12:23, H. J. Lu wrote:
> -   *(.text)
> +   *(.text*)

That should be .text.*, not .text*.  The distinction here is
important.
Admittedly, it will probably never matter for gnu-efi, but
get this
wrong in binutils and you will break some end user code.



-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

On Fri, 2006-03-17 at 12:45, James E Wilson wrote:
> That should be .text.*, not .text*.

And because .text.* doesn't match .text, this actually
needs to be
    *(.text .text.*)
Try looking at ld --verbose output, and grepping for .text.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com

-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

On Fri, Mar 17, 2006 at 12:53:24PM -0800, James E Wilson
wrote:
> On Fri, 2006-03-17 at 12:45, James E Wilson wrote:
> > That should be .text.*, not .text*.
> 
> And because .text.* doesn't match .text, this actually
needs to be
>     *(.text .text.*)

I don't think so. linker calls fnmatch to match section
names by
patterns.  According to my man page:

NAME
       fnmatch - match filename or pathname

SYNOPSIS
       #include <fnmatch.h>

       int fnmatch(const char *pattern, const char *string,
int flags);

DESCRIPTION
       The  fnmatch()  function checks whether the string
argument
matches the
       pattern argument, which is a shell wildcard pattern.

".text*" should work as a shell wildcard
pattern.

> Try looking at ld --verbose output, and grepping for
.text.

This is the updated patch to include *(.gnu.linkonce.t.*)
sections.



H.J.
---
--- gnu-efi/gnuefi/elf_ia32_efi.lds.ia32	2004-12-22
07:52:28.000000000 -0800
+++ gnu-efi/gnuefi/elf_ia32_efi.lds	2006-03-17
13:30:58.000000000 -0800
 -9,7
+9,12  SECTIONS
   . = ALIGN(4096);
   .text :
   {
-   *(.text)
+   *(.text*)
+   *(.gnu.linkonce.t.*)
+  }
+  .reloc :
+  {
+   *(.reloc)
   }
   . = ALIGN(4096);
   .data :
 -48,12
+53,9  SECTIONS
   . = ALIGN(4096);
   .dynstr   : { *(.dynstr) }
   . = ALIGN(4096);
-  .ignored.reloc :
+  /DISCARD/ :
   {
     *(.rel.reloc)
-  }
-  .ignored :
-  {
     *(.eh_frame)
   }
 }
--- gnu-efi/gnuefi/elf_ia64_efi.lds.ia32	2004-12-22
10:18:10.000000000 -0800
+++ gnu-efi/gnuefi/elf_ia64_efi.lds	2006-03-17
13:31:11.000000000 -0800
 -9,7
+9,7  SECTIONS
   . = ALIGN(4096);
   .text :
   {
-   *(.text)
+   *(.text*)
    *(.gnu.linkonce.t*)
   }
   . = ALIGN(4096);
 -59,13
+59,10  SECTIONS
   .dynsym   : { *(.dynsym) }
   . = ALIGN(4096);
   .dynstr   : { *(.dynstr) }
-  .ignored_reloc :
+  /DISCARD/ :
   {
     *(.rela.plabel)
     *(.rela.reloc)
-  }
-  /DISCARD/ :
-  {
     *(.IA_64.unwind*)
     *(.IA64.unwind*)
   }
-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

On Fri, Mar 17, 2006 at 01:46:57PM -0800, H. J. Lu wrote:
> On Fri, Mar 17, 2006 at 12:53:24PM -0800, James E
Wilson wrote:
> > On Fri, 2006-03-17 at 12:45, James E Wilson wrote:
> > > That should be .text.*, not .text*.
> > 
> > And because .text.* doesn't match .text, this
actually needs to be
> >     *(.text .text.*)
> 
> I don't think so. linker calls fnmatch to match
section names by
> patterns.  According to my man page:
> 
> NAME
>        fnmatch - match filename or pathname
> 
> SYNOPSIS
>        #include <fnmatch.h>
> 
>        int fnmatch(const char *pattern, const char
*string, int flags);
> 
> DESCRIPTION
>        The  fnmatch()  function checks whether the
string argument
> matches the
>        pattern argument, which is a shell wildcard
pattern.
> 
> ".text*" should work as a shell wildcard
pattern.
> 
> > Try looking at ld --verbose output, and grepping
for .text.
> 
> This is the updated patch to include
*(.gnu.linkonce.t.*) sections.
> 

This new makes linker scripts similar to normal linker. It
should
generate the same output.


H.J.
---
--- gnu-efi/gnuefi/elf_ia32_efi.lds.ia32	2004-12-22
07:52:28.000000000 -0800
+++ gnu-efi/gnuefi/elf_ia32_efi.lds	2006-03-17
13:48:13.000000000 -0800
 -10,6
+10,12  SECTIONS
   .text :
   {
    *(.text)
+   *(.text.*)
+   *(.gnu.linkonce.t.*)
+  }
+  .reloc :
+  {
+   *(.reloc)
   }
   . = ALIGN(4096);
   .data :
 -48,12
+54,9  SECTIONS
   . = ALIGN(4096);
   .dynstr   : { *(.dynstr) }
   . = ALIGN(4096);
-  .ignored.reloc :
+  /DISCARD/ :
   {
     *(.rel.reloc)
-  }
-  .ignored :
-  {
     *(.eh_frame)
   }
 }
--- gnu-efi/gnuefi/elf_ia64_efi.lds.ia32	2004-12-22
10:18:10.000000000 -0800
+++ gnu-efi/gnuefi/elf_ia64_efi.lds	2006-03-17
13:48:20.000000000 -0800
 -10,6
+10,7  SECTIONS
   .text :
   {
    *(.text)
+   *(.text.*)
    *(.gnu.linkonce.t*)
   }
   . = ALIGN(4096);
 -59,13
+60,10  SECTIONS
   .dynsym   : { *(.dynsym) }
   . = ALIGN(4096);
   .dynstr   : { *(.dynstr) }
-  .ignored_reloc :
+  /DISCARD/ :
   {
     *(.rela.plabel)
     *(.rela.reloc)
-  }
-  /DISCARD/ :
-  {
     *(.IA_64.unwind*)
     *(.IA64.unwind*)
   }
-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

On Fri, Mar 17, 2006 at 01:46:57PM -0800, H. J. Lu wrote:
> On Fri, Mar 17, 2006 at 12:53:24PM -0800, James E
Wilson wrote:
> > On Fri, 2006-03-17 at 12:45, James E Wilson wrote:
> > > That should be .text.*, not .text*.
> > 
> > And because .text.* doesn't match .text, this
actually needs to be
> >     *(.text .text.*)
> 
> I don't think so. linker calls fnmatch to match
section names by
> patterns.  According to my man page:
> 
> DESCRIPTION
>        The  fnmatch()  function checks whether the
string argument
> matches the
>        pattern argument, which is a shell wildcard
pattern.
> 
> ".text*" should work as a shell wildcard
pattern.

I believe his point is that .text* matches .textual and
.texterific and
many other sections which shouldn't be matched.  So you
need to match
.text and .text.*, not just .text*
-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

On Fri, 2006-03-17 at 13:52, Matthew Wilcox wrote:
> I believe his point is that .text* matches .textual and
.texterific and
> many other sections which shouldn't be matched.  So
you need to match
> .text and .text.*, not just .text*

Exactly.

The best example I can give is that I've fixed broken
linker scripts in
libgloss that tried to match linkonce small data sections
using
.gnu.linkonce.s*, without realizing that linkonce small bss
sections are
.gnu.linkonce.sb.*.  This caused some C++ programs to fail
in strange
and difficult to debug ways.  Those dots are important.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com

-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

On Fri, 2006-03-17 at 13:52, H. J. Lu wrote:
>     *(.gnu.linkonce.t*)

This isn't something you added, but should be fixed just
the same.  This
will accidentally cause some thread-local data sections to
be merged
into the text section.  The thread-local sections may not
have existed
when this code was written, but they do now. 
.gnu.linkonce.td.* is used
for linkonce thread local data, and .gnu.linkonce.tb.* is
used for
linkonce thread local bss.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com

-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

H.J.,

On Fri, Mar 17, 2006 at 12:23:01PM -0800, H. J. Lu wrote:
> I don't know where I should report gnu-efi bug. The
ia32 support in
> gnu-efi-3.0b-041222 doesn't work with newer gcc and
binutils. The
> problem is in the linker script. The new gcc may put
functions in
> .text* sections. The newer linker handles orphaned
sections
> differently. I tested this patch on both ia32 and ia64
with different
> versions of gcc and binutils by building elilo 3.6.
Elilo 3.6 loads
> and run kernel correctly on ia64. It seems to work find
on ia32.
> 
As far as I know, I am still the maintainer of this package.
But 
I need to check with some of our Linux people at HP.

It is unfortunate that this package as such strong
dependency
of the GNU compiler and linker. This has caused a lot of
troubles
when all the recent updates to gcc. Yet I do not see a way
around
this.

Once you and James ettle on the right update I'll put it in
and push a new package.

Thanks for tracking this down.

> 
> H.J.
> --- gnu-efi/gnuefi/elf_ia32_efi.lds.ia32	2004-12-22
07:52:28.000000000 -0800
> +++ gnu-efi/gnuefi/elf_ia32_efi.lds	2006-03-17
11:47:21.000000000 -0800
>  -9,7 +9,11  SECTIONS
>    . = ALIGN(4096);
>    .text :
>    {
> -   *(.text)
> +   *(.text*)
> +  }
> +  .reloc :
> +  {
> +   *(.reloc)
>    }
>    . = ALIGN(4096);
>    .data :
>  -48,12 +52,9  SECTIONS
>    . = ALIGN(4096);
>    .dynstr   : { *(.dynstr) }
>    . = ALIGN(4096);
> -  .ignored.reloc :
> +  /DISCARD/ :
>    {
>      *(.rel.reloc)
> -  }
> -  .ignored :
> -  {
>      *(.eh_frame)
>    }
>  }
> --- gnu-efi/gnuefi/elf_ia64_efi.lds.ia32	2004-12-22
10:18:10.000000000 -0800
> +++ gnu-efi/gnuefi/elf_ia64_efi.lds	2006-03-17
11:51:51.000000000 -0800
>  -59,13 +59,10  SECTIONS
>    .dynsym   : { *(.dynsym) }
>    . = ALIGN(4096);
>    .dynstr   : { *(.dynstr) }
> -  .ignored_reloc :
> +  /DISCARD/ :
>    {
>      *(.rela.plabel)
>      *(.rela.reloc)
> -  }
> -  /DISCARD/ :
> -  {
>      *(.IA_64.unwind*)
>      *(.IA64.unwind*)
>    }

-- 

-Stephane
-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html

On Fri, Mar 17, 2006 at 03:15:48PM -0800, James E Wilson
wrote:
> On Fri, 2006-03-17 at 13:52, H. J. Lu wrote:
> >     *(.gnu.linkonce.t*)
> 
> This isn't something you added, but should be fixed
just the same.  This
> will accidentally cause some thread-local data sections
to be merged
> into the text section.  The thread-local sections may
not have existed
> when this code was written, but they do now. 
.gnu.linkonce.td.* is used
> for linkonce thread local data, and .gnu.linkonce.tb.*
is used for
> linkonce thread local bss.

I guess we never used TLS in EFI applications before. It
doesn't
really matter since TLS needs more than the linker script
change.
Here is the new patch.


H.J.
---
--- gnu-efi/gnuefi/elf_ia32_efi.lds.ia32	2004-12-22
07:52:28.000000000 -0800
+++ gnu-efi/gnuefi/elf_ia32_efi.lds	2006-03-17
13:48:13.000000000 -0800
 -10,6
+10,12  SECTIONS
   .text :
   {
    *(.text)
+   *(.text.*)
+   *(.gnu.linkonce.t.*)
+  }
+  .reloc :
+  {
+   *(.reloc)
   }
   . = ALIGN(4096);
   .data :
 -48,12
+54,9  SECTIONS
   . = ALIGN(4096);
   .dynstr   : { *(.dynstr) }
   . = ALIGN(4096);
-  .ignored.reloc :
+  /DISCARD/ :
   {
     *(.rel.reloc)
-  }
-  .ignored :
-  {
     *(.eh_frame)
   }
 }
--- gnu-efi/gnuefi/elf_ia64_efi.lds.ia32	2004-12-22
10:18:10.000000000 -0800
+++ gnu-efi/gnuefi/elf_ia64_efi.lds	2006-03-17
15:27:48.000000000 -0800
 -10,7
+10,8  SECTIONS
   .text :
   {
    *(.text)
-   *(.gnu.linkonce.t*)
+   *(.text.*)
+   *(.gnu.linkonce.t.*)
   }
   . = ALIGN(4096);
   __gp = ALIGN (8) + 0x200000;
 -59,13
+60,10  SECTIONS
   .dynsym   : { *(.dynsym) }
   . = ALIGN(4096);
   .dynstr   : { *(.dynstr) }
-  .ignored_reloc :
+  /DISCARD/ :
   {
     *(.rela.plabel)
     *(.rela.reloc)
-  }
-  /DISCARD/ :
-  {
     *(.IA_64.unwind*)
     *(.IA64.unwind*)
   }
-
To unsubscribe from this list: send the line
"unsubscribe linux-ia64" in
the body of a message to majordomovger.kernel.org
More majordomo info at  http://vge
r.kernel.org/majordomo-info.html