Patch: Fix elf_find_function () on a corner case

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Patch: Fix elf_find_function () on a corner case

jiez
I found an issue of ld of latest CVS on the following small case:

$ cat a.c

extern void bar ();

aaa ()
{
   bar ();
}

$ cat b.c

extern void bar ();

bbb ()
{
   bar ();
}

$ cat m.c

int main ()
{
   aaa ();
   bbb ();
   return 0;
}

$ gcc -c a.c b.c m.c
$ ld -r -o t.o a.o b.o
$ ld -o m m.o t.o
ld: warning: cannot find entry symbol _start; defaulting to 08048094
t.o: In function `aaa':b.c:(.text+0x7): undefined reference to `bar'
t.o: In function `bbb':b.c:(.text+0x17): undefined reference to `bar'


While we expect it to output:

ld: warning: cannot find entry symbol _start; defaulting to 08048094
t.o: In function `aaa': undefined reference to `bar'
t.o: In function `bbb': undefined reference to `bar'

I found the automaton in elf_find_function () misses such case. The
attached patch should fix it on such cases. It's tested using ld
testsuite and no regressions found.

Is it OK?

Thanks,
Jie


        * elf.c (elf_find_function): Don't report a filename at all for
        global symbols when there are two or more FILE symbols.

Index: elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.318
diff -u -p -r1.318 elf.c
--- elf.c 3 Nov 2005 02:53:38 -0000 1.318
+++ elf.c 22 Nov 2005 15:17:14 -0000
@@ -6652,9 +6652,9 @@ elf_find_function (bfd *abfd ATTRIBUTE_U
  default:
   break;
  case STT_FILE:
-  file = &q->symbol;
-  if (state == symbol_seen)
+  if (state == symbol_seen || file != NULL)
     state = file_after_symbol_seen;
+  file = &q->symbol;
   continue;
  case STT_SECTION:
   continue;
Reply | Threaded
Open this post in threaded view
|

Re: Patch: Fix elf_find_function () on a corner case

Jan Beulich
>ld: warning: cannot find entry symbol _start; defaulting to 08048094
>t.o: In function `aaa':b.c:(.text+0x7): undefined reference to `bar'
>t.o: In function `bbb':b.c:(.text+0x17): undefined reference to `bar'

I wouldn't want to loose the information on the source file, if that
information is available. This makes finding the function easier, and in
case of local functions or multiply defined (weak) ones disambiguates
their origin.

>While we expect it to output:
>
>ld: warning: cannot find entry symbol _start; defaulting to 08048094
>t.o: In function `aaa': undefined reference to `bar'
>t.o: In function `bbb': undefined reference to `bar'

Jan
Reply | Threaded
Open this post in threaded view
|

Re: Patch: Fix elf_find_function () on a corner case

jiez
On 11/23/05, Jan Beulich <[hidden email]> wrote:
> >ld: warning: cannot find entry symbol _start; defaulting to 08048094
> >t.o: In function `aaa':b.c:(.text+0x7): undefined reference to `bar'
> >t.o: In function `bbb':b.c:(.text+0x17): undefined reference to `bar'
>
> I wouldn't want to loose the information on the source file, if that
> information is available. This makes finding the function easier, and in
> case of local functions or multiply defined (weak) ones disambiguates
> their origin.
>
In this case, the best information we want is:

t.o: In function `aaa':a.c:(.text+0x7): undefined reference to `bar'
t.o: In function `bbb':b.c:(.text+0x17): undefined reference to `bar'

However, ld cannot give out such information since t.o was got by 'ld
-r' from a.o and b.o. There are two FILE symbols in t.o. For global
symbols like aaa and bbb, elf_find_function () cannot find their
files. The best thing it can do is not output misleading (wrong)
source file information.

> >While we expect it to output:
> >
> >ld: warning: cannot find entry symbol _start; defaulting to 08048094
> >t.o: In function `aaa': undefined reference to `bar'
> >t.o: In function `bbb': undefined reference to `bar'
>

Jie
Reply | Threaded
Open this post in threaded view
|

Re: Patch: Fix elf_find_function () on a corner case

Alan Modra
On Wed, Nov 23, 2005 at 01:15:16AM +0800, Jie Zhang wrote:
> files. The best thing it can do is not output misleading (wrong)
> source file information.

I agree with this.  However, I believe the reason for convoluted logic
here is that it was written to handle the case where we have multiple
file symbols in one source.  This used to happen with
    # <number> <filename>
lines inserted into assembly source until I changed ELF flavours of gas
to only emit a file symbol for the first such line.  Typically, you
would see these lines if using the C preprocessor on some generated
assembler.  Here's an example of such a file from glibc:

File: access.o

Symbol table '.symtab' contains 18 entries:
   Num:    Value  Size Type    Bind   Vis      Ndx Name
     0: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 00000000     0 FILE    LOCAL  DEFAULT  ABS <stdin>
     2: 00000000     0 FILE    LOCAL  DEFAULT  ABS /usr/src/packages/BUILD/kernel-headers/asm/unistd.h
     3: 00000000     0 FILE    LOCAL  DEFAULT  ABS /usr/src/packages/BUILD/kernel-headers/asm-ppc/unistd.h
     4: 00000000     0 FILE    LOCAL  DEFAULT  ABS /usr/src/packages/BUILD/kernel-headers/asm/unistd.h
     5: 00000000     0 FILE    LOCAL  DEFAULT  ABS <stdin>
     6: 00000000     0 FILE    LOCAL  DEFAULT  ABS <command line>
     7: 00000000     0 FILE    LOCAL  DEFAULT  ABS /usr/src/packages/BUILD/glibc-2.3/cc/config.h
     8: 00000000     0 FILE    LOCAL  DEFAULT  ABS <command line>
     9: 00000000     0 FILE    LOCAL  DEFAULT  ABS <built-in>
    10: 00000000     0 FILE    LOCAL  DEFAULT  ABS <stdin>
    11: 00000000     0 SECTION LOCAL  DEFAULT    1
    12: 00000000     0 SECTION LOCAL  DEFAULT    3
    13: 00000000     0 SECTION LOCAL  DEFAULT    4
    14: 00000000     0 SECTION LOCAL  DEFAULT    5
    15: 00000000    16 FUNC    GLOBAL DEFAULT    1 __access
    16: 00000000     0 NOTYPE  GLOBAL DEFAULT  UND __syscall_error
    17: 00000000    16 FUNC    WEAK   DEFAULT    1 access

This was from source piped to gcc, so there wasn't a real file to
report.  Instead gcc emitted a <stdin> file symbol.  Note that the file
symbols are in reverse order, which is why it generally works to
continue consuming file symbols in elf_find_function.  The last one
would be the main source file name.

You might think that because gas no longer emits more than one symbol
for # <number> <file> lines that we could dispense with this logic, but
that would be wrong on two counts.  Firstly, you can still get multiple
file symbols if multiple .file pseudo ops are given, and secondly, you
might be using gdb/objdump/whatever on older object files like the one
I show above.

So I'm rejecting the patch as it is.  You might be able to do something
reasonable with ld -r output by noticing that section symbols precede
file symbols for ld -r.

--
Alan Modra
IBM OzLabs - Linux Technology Centre
Reply | Threaded
Open this post in threaded view
|

Re: Patch: Fix elf_find_function () on a corner case

jiez
Alan Modra wrote:
> So I'm rejecting the patch as it is.  You might be able to do something
> reasonable with ld -r output by noticing that section symbols precede
> file symbols for ld -r.
>
Thanks!

This is my second try. I'm not sure if the section symbols always come
first for ld -r output. If so, this patch should work. If not, the state
machine in elf_find_function () may need rewrite.

Regards,
Jie


        * elf.c (elf_find_function): Don't report a filename at all for
        global symbols when there are two or more FILE symbols for
        ld -r output.

Index: elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.318
diff -u -p -r1.318 elf.c
--- elf.c 3 Nov 2005 02:53:38 -0000 1.318
+++ elf.c 28 Nov 2005 14:31:51 -0000
@@ -6633,6 +6633,7 @@ elf_find_function (bfd *abfd ATTRIBUTE_U
      ld -r doesn't do this.  So, for ld -r output, it is possible to
      make a better choice of file name for local symbols by ignoring
      file symbols appearing after a given local symbol.  */
+  int is_ld_r_output;
   enum { nothing_seen, symbol_seen, file_after_symbol_seen } state;
 
   filename = NULL;
@@ -6641,6 +6642,13 @@ elf_find_function (bfd *abfd ATTRIBUTE_U
   low_func = 0;
   state = nothing_seen;
 
+  /* The section symbols precede file symbols for ld -r.  */
+  if (ELF_ST_TYPE (((elf_symbol_type *) *symbols)->internal_elf_sym.st_info)
+      == STT_SECTION)
+    is_ld_r_output = 1;
+  else
+    is_ld_r_output = 0;
+
   for (p = symbols; *p != NULL; p++)
     {
       elf_symbol_type *q;
@@ -6652,9 +6660,9 @@ elf_find_function (bfd *abfd ATTRIBUTE_U
  default:
   break;
  case STT_FILE:
-  file = &q->symbol;
-  if (state == symbol_seen)
+  if (state == symbol_seen || (is_ld_r_output && file != NULL))
     state = file_after_symbol_seen;
+  file = &q->symbol;
   continue;
  case STT_SECTION:
   continue;
Reply | Threaded
Open this post in threaded view
|

Re: Patch: Fix elf_find_function () on a corner case

Alan Modra
On Mon, Nov 28, 2005 at 10:38:20PM +0800, Jie Zhang wrote:
> This is my second try. I'm not sure if the section symbols always come
> first for ld -r output. If so, this patch should work. If not, the state
> machine in elf_find_function () may need rewrite.

I'm applying this simpler change instead.

        * elf.c (elf_find_function): Don't ignore section syms.
        Simplify filename logic.

Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.322
diff -u -p -r1.322 elf.c
--- bfd/elf.c 27 Dec 2005 03:45:30 -0000 1.322
+++ bfd/elf.c 27 Dec 2005 09:01:01 -0000
@@ -6686,8 +6686,6 @@ elf_find_function (bfd *abfd ATTRIBUTE_U
   if (state == symbol_seen)
     state = file_after_symbol_seen;
   continue;
- case STT_SECTION:
-  continue;
  case STT_NOTYPE:
  case STT_FUNC:
   if (bfd_get_section (&q->symbol) == section
@@ -6696,12 +6694,10 @@ elf_find_function (bfd *abfd ATTRIBUTE_U
     {
       func = (asymbol *) q;
       low_func = q->symbol.value;
-      if (file == NULL)
- filename = NULL;
-      else if (ELF_ST_BIND (q->internal_elf_sym.st_info) != STB_LOCAL
-       && state == file_after_symbol_seen)
- filename = NULL;
-      else
+      filename = NULL;
+      if (file != NULL
+  && (ELF_ST_BIND (q->internal_elf_sym.st_info) == STB_LOCAL
+      || state != file_after_symbol_seen))
  filename = bfd_asymbol_name (file);
     }
   break;


--
Alan Modra
IBM OzLabs - Linux Technology Centre