Bug 452194

Created attachment 309892 [details]
move base check to where it works, fix comparison

Looking at it...

Testcase:

bar_inside_foo.s
================
	.section .text,"ax",@progbits
	.globl bar
	.globl foo
foo:
	xorl	%eax, %eax
bar:
	xorl	%eax, %eax
	ret
	.size   bar, . - bar
	xorl	%eax, %eax
	ret
	.size   foo, . - foo

# as bar_inside_foo.s 
# objdump -d a.out 
a.out:     file format elf64-x86-64
Disassembly of section .text:
0000000000000000 <foo>:
   0:	31 c0                	xor    %eax,%eax
0000000000000002 <bar>:
   2:	31 c0                	xor    %eax,%eax
   4:	c3                   	retq   
   5:	31 c0                	xor    %eax,%eax
   7:	c3                   	retq   

Before patch:
# src/addr2line -f -e a.out 0 1 2 3 4 5 6 7 8
foo
??:0
??
??:0
bar
??:0
bar
??:0
bar
??:0
??
??:0
??
??:0
??
??:0
??
??:0

addresses in the part of foo which are past the end of bar are not resolved to foo.

After patch:

# src/addr2line -f -e a.out 0 1 2 3 4 5 6 7 8
foo
??:0
foo
??:0
bar
??:0
bar
??:0
bar
??:0
foo
??:0
foo
??:0
foo
??:0
??
??:0

Scratch that, I used wrong a.out...

(In reply to comment #0)
> See the patch, for instance:
> 
> foo:
> 
> bar:
>    ...
>    ...
>    .size(bar)
> 
> <- you are here

I cannot reproduce this bug. I instrumented original and patched elfutils and no
matter which assembly files I throw at it and it doesn't matter which addr2line
commands I use, original and patched version are producing the same result.

Andrew, can you provide a testcase?

(In reply to comment #3)
> Testcase:
> 
> bar_inside_foo.s
> ================
> 	.section .text,"ax",@progbits
> 	.globl bar
> 	.globl foo
> foo:
> 	xorl	%eax, %eax
> bar:
> 	xorl	%eax, %eax
> 	ret
> 	.size   bar, . - bar
> 	xorl	%eax, %eax
> 	ret
> 	.size   foo, . - foo

No second .size.  The sequence is:

   foo: # as in an "assembler symbol"
   bar: # a sized symbol
      ...
      .size bar ...
   <-- you are here

broadly what happens is:

   - the code finds foo, records it as a fall-back
   - the code finds bar, rejects it; but doesn't throw away foo since that is no
longer valid

which leads to "foo" instead of not-found being returned

Patch boils down to

--- elfutils.0/libdwfl/dwfl_module_addrsym.c	2008-07-15 14:39:07.000000000 +0200
+++ elfutils.1/libdwfl/dwfl_module_addrsym.c	2008-07-16 14:45:18.000000000 +0200
@@ -111,25 +111,27 @@ dwfl_module_addrsym (Dwfl_Module *mod, G
   GElf_Addr min_label = addr;
 
   /* Look through the symbol table for a matching symbol.  */
   for (int i = 1; i < syments; ++i)
     {
       GElf_Sym sym;
       GElf_Word shndx;
       const char *name = INTUSE(dwfl_module_getsym) (mod, i, &sym, &shndx);
+      /* Even if we don't choose this symbol, its existence excludes
+	 any sizeless symbol (assembly label) that is below its upper
+	 bound.  */
+      if (name != NULL && sym.st_value <= addr
+	  && sym.st_value + sym.st_size > min_label) {
+	min_label = sym.st_value + sym.st_size;
+      }
       if (name != NULL
 	  && sym.st_value <= addr
 	  && (sym.st_size == 0 || addr - sym.st_value < sym.st_size))
 	{
-	  /* Even if we don't choose this symbol, its existence
-	     excludes any sizeless symbol (assembly label) that
-	     is inside its bounds.  */
-	  if (sym.st_value + sym.st_size > addr)
-	    min_label = sym.st_value + sym.st_size;
 
 	  /* This symbol is a better candidate than the current one
 	     if it's a named symbol, not a section or file symbol,
 	     and is closer to ADDR or is global when it was local.  */
 	  if (name[0] != '\0'
 	      && GELF_ST_TYPE (sym.st_info) != STT_SECTION
 	      && GELF_ST_TYPE (sym.st_info) != STT_FILE)
 	    {


but the bug does not trigger on current elfutils, it was spotted by frysk
developers after they changed

GElf_Addr min_label = addr;

to

GElf_Addr min_label = 0;

because they want to match sizeless symbols below addr too.

I am going to discuss proper course of action on the mailing list.

Just my $0.02 that in the first place I miss here a real-world debuggee program
sample which requires the requested behavior change.  I do not see why this
corner case should behave specifically this or that way.

Created attachment 314191 [details]
Proposed patch for bugs 268961 and 452194

Created attachment 314192 [details]
Binary file tests/testfile50.o.bz2

Created attachment 314194 [details]
Binary file tests/testfile51.o.bz2

elfutils-0.137-2.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/elfutils-0.137-2.fc8

elfutils-0.137-2.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/elfutils-0.137-2.fc9

elfutils-0.137-2.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update elfutils'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-7460

elfutils-0.137-2.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update elfutils'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-7464

elfutils-0.137-3.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update elfutils'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-7464

elfutils-0.137-3.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update elfutils'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-7460

elfutils-0.137-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

elfutils-0.137-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.