Bug 452194 - elfutils incorrectly reports assembler symbols
Summary: elfutils incorrectly reports assembler symbols
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: elfutils
Version: 8
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Denys Vlasenko
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 173278
TreeView+ depends on / blocked
 
Reported: 2008-06-20 02:16 UTC by Andrew Cagney
Modified: 2008-11-13 03:36 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-11-13 03:36:21 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
move base check to where it works, fix comparison (3.41 KB, patch)
2008-06-20 02:16 UTC, Andrew Cagney
no flags Details | Diff
Proposed patch for bugs 268961 and 452194 (8.90 KB, patch)
2008-08-13 11:18 UTC, Denys Vlasenko
no flags Details | Diff
Binary file tests/testfile50.o.bz2 (251 bytes, application/x-bzip2)
2008-08-13 11:20 UTC, Denys Vlasenko
no flags Details
Binary file tests/testfile51.o.bz2 (251 bytes, application/x-bzip2)
2008-08-13 11:21 UTC, Denys Vlasenko
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Sourceware 6659 0 None None None Never

Description Andrew Cagney 2008-06-20 02:16:38 UTC
See the patch, for instance:

foo:

bar:
   ...
   ...
   .size(bar)

<- you are here

Comment 1 Andrew Cagney 2008-06-20 02:16:38 UTC
Created attachment 309892 [details]
move base check to where it works, fix comparison

Comment 2 Denys Vlasenko 2008-07-16 11:12:39 UTC
Looking at it...

Comment 3 Denys Vlasenko 2008-07-16 12:42:46 UTC
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


Comment 4 Denys Vlasenko 2008-07-16 14:04:27 UTC
Scratch that, I used wrong a.out...

Comment 5 Denys Vlasenko 2008-07-16 14:53:13 UTC
(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?

Comment 6 Andrew Cagney 2008-07-16 17:00:27 UTC
(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

Comment 7 Denys Vlasenko 2008-07-17 12:24:18 UTC
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.


Comment 8 Jan Kratochvil 2008-07-17 18:22:59 UTC
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.


Comment 9 Denys Vlasenko 2008-08-13 11:18:51 UTC
Created attachment 314191 [details]
Proposed patch for bugs 268961 and 452194

Comment 10 Denys Vlasenko 2008-08-13 11:20:23 UTC
Created attachment 314192 [details]
Binary file tests/testfile50.o.bz2

Comment 11 Denys Vlasenko 2008-08-13 11:21:18 UTC
Created attachment 314194 [details]
Binary file tests/testfile51.o.bz2

Comment 12 Fedora Update System 2008-08-28 19:53:54 UTC
elfutils-0.137-2.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/elfutils-0.137-2.fc8

Comment 13 Fedora Update System 2008-08-28 19:56:08 UTC
elfutils-0.137-2.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/elfutils-0.137-2.fc9

Comment 14 Fedora Update System 2008-09-10 06:37:14 UTC
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

Comment 15 Fedora Update System 2008-09-10 07:02:44 UTC
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

Comment 16 Fedora Update System 2008-10-03 22:29:30 UTC
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

Comment 17 Fedora Update System 2008-10-03 22:34:59 UTC
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

Comment 18 Fedora Update System 2008-11-13 03:35:56 UTC
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.

Comment 19 Fedora Update System 2008-11-13 03:36:14 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.