[PATCH 2/2] Check for line notes on GCC as well as Clang

[PATCH 2/2] Check for line notes on GCC as well as Clang

[Alan Hayward]

i386 and amd64 only skip the prologue using line notes if the compiler is
Clang. Also allow this for GCC.

With this change we can revert ovldbreak.exp back to only allowing line 49
as the breakpoint for main.

Fixes over 50 tests on Ubuntu x86_64.

Note: Many other targets do NOT have any Clang/GNU check - they allow any
valid result from skip_prologue_using_sal. Maybe that is the better fix
here. However, I was erring on the side of caution.

gdb/ChangeLog:

2018-10-24  Alan Hayward  <alan.hayward@arm.com>

	* amd64-tdep.c (amd64_skip_prologue): Add GNU check.
	* i386-tdep.c (i386_skip_prologue): Likewise.

gdb/testsuite/ChangeLog:

2018-10-24  Alan Hayward  <alan.hayward@arm.com>

	* gdb.cp/ovldbreak.exp: Only allow line 49 for main breakpoint.
---
 gdb/amd64-tdep.c                   | 7 ++++---
 gdb/i386-tdep.c                    | 7 ++++---
 gdb/testsuite/gdb.cp/ovldbreak.exp | 2 +-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 088542d72b..cda462b5d8 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -2474,12 +2474,13 @@ amd64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
 	= skip_prologue_using_sal (gdbarch, func_addr);
       struct compunit_symtab *cust = find_pc_compunit_symtab (func_addr);
 
-      /* Clang always emits a line note before the prologue and another
-	 one after.  We trust clang to emit usable line notes.  */
+      /* Clang/GCC always emits a line note before the prologue and another
+	 one after.  We trust Clang/GCC to emit usable line notes.  */
       if (post_prologue_pc
 	  && (cust != NULL
 	      && COMPUNIT_PRODUCER (cust) != NULL
-	      && startswith (COMPUNIT_PRODUCER (cust), "clang ")))
+	      && (startswith (COMPUNIT_PRODUCER (cust), "clang ")
+		  || startswith (COMPUNIT_PRODUCER (cust), "GNU "))))
         return std::max (start_pc, post_prologue_pc);
     }
 
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a6994aaf12..73e1c6c420 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -1844,12 +1844,13 @@ i386_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR start_pc)
 	= skip_prologue_using_sal (gdbarch, func_addr);
       struct compunit_symtab *cust = find_pc_compunit_symtab (func_addr);
 
-      /* Clang always emits a line note before the prologue and another
-	 one after.  We trust clang to emit usable line notes.  */
+      /* Clang/GCC always emits a line note before the prologue and another
+	 one after.  We trust Clang/GCC to emit usable line notes.  */
       if (post_prologue_pc
 	  && (cust != NULL
 	      && COMPUNIT_PRODUCER (cust) != NULL
-	      && startswith (COMPUNIT_PRODUCER (cust), "clang ")))
+	      && (startswith (COMPUNIT_PRODUCER (cust), "clang ")
+		  || startswith (COMPUNIT_PRODUCER (cust), "GNU "))))
         return std::max (start_pc, post_prologue_pc);
     }
  
diff --git a/gdb/testsuite/gdb.cp/ovldbreak.exp b/gdb/testsuite/gdb.cp/ovldbreak.exp
index f3f329d293..a2f5620ca8 100644
--- a/gdb/testsuite/gdb.cp/ovldbreak.exp
+++ b/gdb/testsuite/gdb.cp/ovldbreak.exp
@@ -209,7 +209,7 @@ for {set idx 0} {$idx < [llength $overloads]} {incr idx} {
 
 # Verify the breakpoints.
 set bptable "Num\[\t \]+Type\[\t \]+Disp Enb Address\[\t \]+What.*\[\r\n]+"
-append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+$hex\[\t \]+in main(\\((|void)\\))? at.*$srcfile:4\[89\]\[\r\n\]+"
+append bptable "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep\[\t \]y\[\t \]+$hex\[\t \]+in main(\\((|void)\\))? at.*$srcfile:49\[\r\n\]+"
 append bptable "\[\t \]+breakpoint already hit 1 time\[\r\n\]+."
 foreach ovld $overloads {
     append bptable [format "\[0-9\]+\[\t \]+breakpoint\[\t \]+keep y\[\t \]+$hex\[\t \]+in foo::overload1arg\\(%s\\) at.*$srcfile:%d\[\r\n\]+" $ovld \
-- 
2.17.1 (Apple Git-112)

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

[Alan Hayward]

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]

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).  */
-- 
2.17.1 (Apple Git-112)

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

[Alan Hayward]

Ping for this and part two.

There was no 0/2. Thinking about it now, I probably should have said:


[0/2] Fix start line for function breakpoints on Ubuntu

These two patches came out of the change I made to ovldbreak.exp due to the
breakpoint at main starting on the { instead of the first line. This happens
only on Ubuntu.

Part one is a generic fix. Part two is an addition to the fix specifically
for i386/amd64. Part two also reverts the line number part of ovldbreak.exp.

Fixes roughly 50 test cases for Ubuntu.

Alan.



> On 24 Oct 2018, at 10:04, Alan Hayward <Alan.Hayward@arm.com> 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]
> 
> 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).  */
> -- 
> 2.17.1 (Apple Git-112)
> 

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

[Simon Marchi]

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?

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.

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?

> 
> 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

Re: [PATCH 2/2] Check for line notes on GCC as well as Clang

[Simon Marchi]

Hi Alan,

Is a "line note" a concept I am not familiar with, or is it just an entry in the
DWARF line number program?

On 2018-10-24 5:04 a.m., Alan Hayward wrote:
> i386 and amd64 only skip the prologue using line notes if the compiler is
> Clang. Also allow this for GCC.

Without understanding the problem at hand: since we were only using this info
with clang, I assume that there was a point where it wasn't trustworthy with gcc?
If so, since when is it trustworthy with gcc, long enough?

> With this change we can revert ovldbreak.exp back to only allowing line 49
> as the breakpoint for main.
> 
> Fixes over 50 tests on Ubuntu x86_64.
> 
> Note: Many other targets do NOT have any Clang/GNU check - they allow any
> valid result from skip_prologue_using_sal. Maybe that is the better fix
> here. However, I was erring on the side of caution.

When running the test locally before this series, everything passes.  With this
series applied, I get:

$ make check TESTS="gdb.cp/ovldbreak.exp"
...
Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.cp/ovldbreak.exp ...
ERROR: can't read "might_kfail": no such variable
...

I assume that this particular error is due to a typo, so let's say I fix the
faulty line (s/might_kfail/might_fail/), then I get:

FAIL: gdb.cp/ovldbreak.exp: continue to bp overloaded : int
FAIL: gdb.cp/ovldbreak.exp: continue to bp overloaded : unsigned_int
FAIL: gdb.cp/ovldbreak.exp: continue to bp overloaded : long_int
FAIL: gdb.cp/ovldbreak.exp: continue to bp overloaded : unsigned_long_int
FAIL: gdb.cp/ovldbreak.exp: continue to bp overloaded : float

This is with the current gcc on Arch Linux:

gcc (GCC) 8.2.1 20180831
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Again, I don't really have time to dig into it.

Simon

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

[Alan Hayward]

> 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

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

[Tom Tromey]

>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:

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

What does the DWARF look like?

Ideally I think gdb would just rely on the DWARF to mark the end of the
prologue.  However, of course that can't always be done -- but I would
much rather have a blacklist of bad compilers than the current approach
of a whitelist of good ones; and I'm wondering if that is achievable at
all.  Maybe doing this well would mean exposing a bit more info from the
DWARF to gdb's line table -- but that seems totally fine.

Maybe I'm really not understanding the problem, feel free to say so :)
But I'd like to understand better; my own forays into this area haven't
turned out so well.

thanks,
Tom