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

Message ID 20181024090427.22721-2-alan.hayward@arm.com
State New, archived
Headers

Commit Message

Alan Hayward Oct. 24, 2018, 9:04 a.m. UTC
  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(-)
  

Comments

Simon Marchi Nov. 5, 2018, 4:16 a.m. UTC | #1
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
  

Patch

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 \