Re: [PATCH 1/2] Allow function prologues to have multiple repeating lines

[Alan Hayward <Alan dot Hayward at arm dot com>]

> On 5 Nov 2018, at 03:50, Simon Marchi <simark@simark.ca> wrote:
> 
> Hi Alan,
> 
> A side-note, I wasn't able to apply your patch directly, because the diff contains
> CRLF line terminators.  I took the body of your message, which is base64-encoded
> (this is not an issue, git-am can deal with this) and decoded it to a file (named
> patch2 below).  As-is, it wouldn't apply, but after passing it in dos2unix it worked
> fine.
> 
> $ git apply patch2
> error: patch failed: gdb/symtab.c:3925
> error: gdb/symtab.c: patch does not apply
> $ file patch2
> patch2: unified diff output, UTF-8 Unicode text, with CRLF line terminators
> $ dos2unix patch2
> dos2unix: converting file patch2 to Unix format...
> $ file patch2
> patch2: unified diff output, UTF-8 Unicode text
> $ git apply patch2
> * works *
> 
> Since you are working on a mac (according to the tail of the patch), which doesn't
> usually uses CRLF, I wonder how they got there.  Are you able to git-am your
> patch directly?

Before sending, git send-email gave me a conversion warning. Normally this doesn’t
happen. It gave me two options, and I let it do the default option. Looks like that
wasn’t the right one. Using git send-email has fixed all the issues I used to get
with odd characters. I’ll look into it if it pops up again before sending.


> 
> Anyway, comments on the actual patch:
> 
> I am able to generate a somewhat similar debug info using this ugly code:
> 
> int hello(int a) {int b = a + 1;
>    return a;
> }
> 
> int main()
> {
>  return hello(2);
> }
> 
> $ readelf --debug-dump=decodedline a.out
> Contents of the .debug_line section:
> 
> CU: ./test.c:
> File name                            Line number    Starting address   View
> test.c                                         1              0x1119
> test.c                                         1              0x1120
> test.c                                         2              0x1129
> test.c                                         3              0x112c
> ...
> 
> The user code starts at 0x1120.  How would the debugger know that in this case
> the prologue shouldn't extend up to 0x1129 (exclusive)?  With your patch applied,
> skip_prologue_using_sal returns 0x1129.  However, GDB ends up getting it right
> (I don't really know how and don't really have time right now to dig more):
> 
> (gdb) b hello
> Breakpoint 1 at 0x1120: file test.c, line 1.
> 
> So I'm just wondering if you see some potential problems with this.

Yes, this does cause issues. Your test case needs updating so that it uses global
vars and some stack usage (causing the 2 prologues). But when that happens, with
my patch, gdb will (incorrectly) stop at the “return a” line.

Looking a little more into it. This issue goes away if I use my hand built GCC on
Ubuntu. It might be something to do with PIC/PIE being enabled by default on
Ubuntu (although I’ve yet to re-create the issue on my hand build compiler, or
found a way of removing it on the ubuntu compiler. Needs something more than
-fno-pic ).

I’m now in the position where I think that, with the information we are using, it
is not possible to tell apart the difference between a two part prologue and the
case where function name and code are on the same line. (It’s a bit clearer in the
examples below).

The existing GDB code is making a fairly dodgy assumption to detect the first line.
But... it might be the best we can do.

I’ll continue looking see if I can find a way around this or a way to cover up the
issue in the testsuite.

> 
> On 2018-10-24 5:04 a.m., Alan Hayward wrote:
>> Compiling gdb.cp/ovldbreak.cc on Ubuntu places two identical line numbers
>> in the function prologue.
>> 
>> x86_64 Ubtunu 16.04 with GCC 5.4.0-6ubuntu1~16.04.4
>> 000000000040052f <main>:
>> Line 48
>>  40052f:	55                   	push   %rbp
>>  400530:	48 89 e5             	mov    %rsp,%rbp
>>  400533:	53                   	push   %rbx
>>  400534:	48 81 ec 88 00 00 00 	sub    $0x88,%rsp
>> Line 48
>>  40053b:	64 48 8b 04 25 28 00 	mov    %fs:0x28,%rax
>>  400542:	00 00
>>  400544:	48 89 45 e8          	mov    %rax,-0x18(%rbp)
>>  400548:	31 c0                	xor    %eax,%eax
>> Line 49
>>  40054a:	c6 45 85 02          	movb   $0x2,-0x7b(%rbp)
>> Line 50
>>  40054e:	c6 45 86 03          	movb   $0x3,-0x7a(%rbp)
>> etc
>> 
>> Aarch64 Ubuntu 16.04 with GCC 7.2.0-1ubuntu1~16.04
>> 0000000000400708 <main>:
>> Line 48
>>  400708:	d102c3ff 	sub	sp, sp, #0xb0
>>  40070c:	a9027bfd 	stp	x29, x30, [sp,#32]
>>  400710:	910083fd 	add	x29, sp, #0x20
>>  400714:	f9001bf3 	str	x19, [sp,#48]
>> Line 48
>>  400718:	90000100 	adrp	x0, 420000 <_GLOBAL_OFFSET_TABLE_+0x28>
>>  40071c:	9100e000 	add	x0, x0, #0x38
>>  400720:	f9400001 	ldr	x1, [x0]
>>  400724:	f90047a1 	str	x1, [x29,#136]
>>  400728:	d2800001 	mov	x1, #0x0                   	// #0
>> Line 49
>>  40072c:	52800040 	mov	w0, #0x2                   	// #2
>>  400730:	3900b7a0 	strb	w0, [x29,#45]
>> Line 50
>>  400734:	52800060 	mov	w0, #0x3                   	// #3
>>  400738:	3900bba0 	strb	w0, [x29,#46]
>> etc
>> 
>> Compare to openSUSE 13.3 AArch64 with GCC 7.2.1 20171020
>> 
>> 00000000004005e4 <main>:
>> Line 48
>>  4005e4:	d102c3ff 	sub	sp, sp, #0xb0
>>  4005e8:	a9027bfd 	stp	x29, x30, [sp, #32]
>>  4005ec:	910083fd 	add	x29, sp, #0x20
>>  4005f0:	f9001bf3 	str	x19, [sp, #48]
>> Line 49
>>  4005f4:	52800040 	mov	w0, #0x2                   	// #2
>>  4005f8:	39023fa0 	strb	w0, [x29, #143]
>> Line 50
>>  4005fc:	52800060 	mov	w0, #0x3                   	// #3
>>  400600:	39023ba0 	strb	w0, [x29, #142]
> 
> Just curious, did you get this output directly from using a tool, and if so which one?

In the testsuite dir:
objdump -dl outputs/gdb.cp/ovldbreak/ovldbreak

Then some minor editing to replace the full paths with “Line”.
And added in the missing lines - objdump skips printing the duplicated lines.
(I need to double check there isn’t anything else going on here).

Alternatively, I should have shown the output from gdb. I’ve added my own annotations.

Ubuntu:

(gdb) maint info line-table
objfile: /work/alahay01/gdb-HEAD/build-aarch64/gdb/testsuite/ovldbreak ((struct objfile *) 0x3b43c1c0)
compunit_symtab: ((struct compunit_symtab *) 0x3b430bc0)
symtab: /work/alahay01/gdb-HEAD/src/binutils-gdb/gdb/testsuite/gdb.cp/ovldbreak.cc ((struct symtab *) 0x3b430c40)
linetable: ((struct linetable *) 0x3b4b2780):
INDEX    LINE ADDRESS
0          45 0x0000000000400680
1          45 0x0000000000400680
2          48 0x0000000000400688 - main prologue (stack)
3          48 0x0000000000400698 - main prologue (GOT)
4          49 0x00000000004006ac - main first line
5          50 0x00000000004006b4
...

Redhat:

(gdb) maint info line-table
objfile: /work/alahay01/gdb-HEAD/build-aarch64/gdb/testsuite/outputs/gdb.cp/ovldbreak/ovldbreak ((struct objfile *) 0x18209660)
compunit_symtab: ((struct compunit_symtab *) 0x1816cd00)
symtab: /work/alahay01/gdb-HEAD/src/binutils-gdb/gdb/testsuite/gdb.cp/ovldbreak.cc ((struct symtab *) 0x1816cd80)
linetable: ((struct linetable *) 0x1826e020):
INDEX    LINE ADDRESS
0          45 0x0000000000400670
1          45 0x0000000000400670
2          48 0x0000000000400674    - main prologue (stack)
3          49 0x0000000000400684    - main first line
4          50 0x000000000040068c
...

Line 45 is another (empty) function.
Line 48 is the start of main.


I’ve now edited ovldbreak so that the code is:
int main () { char arg2 = 2;
    signed char arg3 =3;
So, the first line of code is now line 47.


Ubuntu:

(gdb) maint info line-table
objfile: /work/alahay01/gdb-HEAD/build-aarch64/gdb/testsuite/outputs/gdb.cp/ovldbreak/ovldbreak ((struct objfile *) 0x2772f1c0)
compunit_symtab: ((struct compunit_symtab *) 0x277328a0)
symtab: /work/alahay01/gdb-HEAD/src/binutils-gdb/gdb/testsuite/gdb.cp/ovldbreak.cc ((struct symtab *) 0x27732920)
linetable: ((struct linetable *) 0x277a5780):
INDEX    LINE ADDRESS
0          45 0x0000000000400680
1          45 0x0000000000400680
2          47 0x0000000000400688  - main prologue (stack)
3          47 0x0000000000400698  - main prologue (GOT)
				  - No entry for first line!
4          48 0x00000000004006b4
5          49 0x00000000004006bc
...


Redhat:

(gdb) maint info line-table
objfile: /work/alahay01/gdb-HEAD/build-aarch64/gdb/testsuite/outputs/gdb.cp/ovldbreak/ovldbreak ((struct objfile *) 0x27779660)
compunit_symtab: ((struct compunit_symtab *) 0x276dcd00)
symtab: /work/alahay01/gdb-HEAD/src/binutils-gdb/gdb/testsuite/gdb.cp/ovldbreak.cc ((struct symtab *) 0x276dcd80)
linetable: ((struct linetable *) 0x277de020):
INDEX    LINE ADDRESS
0          45 0x0000000000400670
1          45 0x0000000000400670
2          47 0x0000000000400674  - main prologue (stack)
3          47 0x0000000000400684  - main first line
4          48 0x000000000040068c
5          49 0x0000000000400694
...


So, I don’t think it’s possible to tell apart the first example with the final
example.



> 
>> 
>> skip_prologue_using_sal () does did not allow for the case where there might
>> be two SALs with the same line number in a function prologue. Allow this.
>> 
>> Fixes over 50 tests on Aarch64 Ubuntu.
>> 
>> 2018-10-24  Alan Hayward  <alan.hayward@arm.com>
>> 
>> 	* symtab.c (skip_prologue_using_sal): Don't break for equal line
>> 	numbers.
>> ---
>> gdb/symtab.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>> index 2e48d6527e..67ab5d40fa 100644
>> --- a/gdb/symtab.c
>> +++ b/gdb/symtab.c
>> @@ -3925,9 +3925,9 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr)
>> 	  sal = find_pc_line (prologue_sal.end, 0);
>> 	  if (sal.line == 0)
>> 	    break;
>> -	  /* Assume that a consecutive SAL for the same (or larger)
>> -	     line mark the prologue -> body transition.  */
>> -	  if (sal.line >= prologue_sal.line)
>> +	  /* Assume that a SAL to a larger line marks the prologue -> body
>> +	     transition.  */
>> +	  if (sal.line > prologue_sal.line)
>> 	    break;
>> 	  /* Likewise if we are in a different symtab altogether
>> 	     (e.g. within a file included via #include).  */
>> 
> 
> There is a comments just lower:
> 
> 	  /* The line number is smaller.  Check that it's from the
> 	     same function, not something inlined.  If it's inlined,
> 	     then there is no point comparing the line numbers.  */
> 
> It would probably need to be updated, smaller -> smaller or equal?
> 
> Simon