Re: [PATCH 2/2] gdb: Preserve is-stmt lines when switch between files

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 4/11/20 5:52 AM, Bernd Edlinger wrote:
> Andrew,
> 
> Just, a minor nit:
> 
> /home/ed/gnu/binutils-gdb/.git/rebase-apply/patch:665: new blank line at EOF.

Hmm, I hope you did not think that is me beginning to be insane, that was just the
incomprehensible output of "git am" on your patch part 2/2, funny isn't it?

I think that means:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inline-header-3.exp b/gdb/testsuite/gdb.dwarf2/dw2-inline-header-3.exp
index c683dc4..161a1fc 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-inline-header-3.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-header-3.exp
@@ -188,5 +188,3 @@ for { set i 0 } { $i < 100 && $keep_going } { incr i } {
 
 gdb_assert { $found_line_19 && $found_line_20 } \
     "found line 19 and 20"
-
-
-- 

Another bit I had to fix in order to make it compile

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a146ed1..36e8b24 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -19786,7 +19786,7 @@ class lnp_state_machine
   m_discriminator = 0;
 
   m_last_address = m_address;
-  m_stmt_at_addr = false;
+  m_stmt_at_address = false;
 }
 
 void

Which makes me wonder if you compiled that at all, what you sent to me.
Or if that was actually a different version than intended, so please
double-check that.

If that is the case, please re-submit your test case patches as extra
patch files, and the code change also extra, since I intend to use
your test cases in my patch, and not having to dissect the patches
would be great.

And in order to compare the test results better, I restored the
step-and-next-inline.exp test:

diff --git a/gdb/testsuite/gdb.cp/step-and-next-inline.exp b/gdb/testsuite/gdb.cp/step-and-next-inline.exp
index a95e211..e3a5793 100644
--- a/gdb/testsuite/gdb.cp/step-and-next-inline.exp
+++ b/gdb/testsuite/gdb.cp/step-and-next-inline.exp
@@ -24,13 +24,6 @@ if { ![supports_statement_frontiers] } {
 proc do_test { use_header } {
     global srcfile testfile
 
-    if { $use_header } {
-	# This test will not pass due to poor debug information
-	# generated by GCC (at least upto 10.x).  See
-	# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94474
-	return
-    }
-
     set options {c++ debug nowarnings optimize=-O2\ -gstatement-frontiers}
     if { $use_header } {
 	lappend options additional_flags=-DUSE_NEXT_INLINE_H
@@ -59,37 +52,20 @@ proc do_test { use_header } {
     gdb_test "step" ".*" "step into get_alias_set"
     gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
 	"not in inline 1"
-    # It's possible that this first failure (when not using a header
-    # file) is GCC's fault, though the remaining failures would best
-    # be fixed by adding location views support (though it could be
-    # that some easier heuristic could be figured out).  Still, it is
-    # not certain that the first failure wouldn't also be fixed by
-    # having location view support, so for now it is tagged as such.
-    if {!$use_header} { setup_kfail "*-*-*" symtab/25507 }
     gdb_test "next" ".*TREE_TYPE.*" "next step 1"
     gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
 	"not in inline 2"
     gdb_test "next" ".*TREE_TYPE.*" "next step 2"
     gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
 	"not in inline 3"
-    if {!$use_header} { setup_kfail "*-*-*" symtab/25507 }
     gdb_test "next" ".*TREE_TYPE.*" "next step 3"
     gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \
 	"not in inline 4"
-    if {!$use_header} { setup_kfail "*-*-*" symtab/25507 }
     gdb_test "next" "return 0.*" "next step 4"
     gdb_test "bt" \
 	"\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \
 	"not in inline 5"
 
-    if {!$use_header} {
-	# With the debug from GCC 10.x (and earlier) GDB is currently
-	# unable to successfully complete the following tests when we
-	# are not using a header file.
-	kfail symtab/25507 "stepping tests"
-	return
-    }
-
     clean_restart ${executable}
 
     if ![runto_main] {


> 
> I am trying to find out which patch is better.
> So I applied your patch, an try to see where the differences are.
> 
> 
> Please be patient..
> 

Hmm, okay, I compared the two patches now.

First please don't take that personally :-)
but I think mine is a tiny bit better, because it
fixes the step-and-next-inline test case entirely.

And fixes your test case also almost completely:
 /gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/step-and-next-inline.exp ...
 PASS: gdb.cp/step-and-next-inline.exp: no_header: in get_alias_set pass 2
-PASS: gdb.cp/step-and-next-inline.exp: no_header: in inline 1 pass 2
-PASS: gdb.cp/step-and-next-inline.exp: no_header: in inline 2 pass 2
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: in inline 1 pass 2
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: in inline 2 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: no_header: in inline 3 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: no_header: in main
 PASS: gdb.cp/step-and-next-inline.exp: no_header: in main pass 2
-PASS: gdb.cp/step-and-next-inline.exp: no_header: next step 1
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: next step 1
 PASS: gdb.cp/step-and-next-inline.exp: no_header: next step 2
-PASS: gdb.cp/step-and-next-inline.exp: no_header: next step 3
-PASS: gdb.cp/step-and-next-inline.exp: no_header: next step 4
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: next step 3
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: next step 4
 PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 1
 PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 1 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 2
-PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 2 pass 2
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: not in inline 2 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 3
 PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 3 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 4
 PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 4 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: no_header: not in inline 5
-PASS: gdb.cp/step-and-next-inline.exp: no_header: step 1
-PASS: gdb.cp/step-and-next-inline.exp: no_header: step 2
-PASS: gdb.cp/step-and-next-inline.exp: no_header: step 3
-PASS: gdb.cp/step-and-next-inline.exp: no_header: step 4
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: step 1
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: step 2
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: step 3
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: step 4
 PASS: gdb.cp/step-and-next-inline.exp: no_header: step 5
 PASS: gdb.cp/step-and-next-inline.exp: no_header: step 6
-PASS: gdb.cp/step-and-next-inline.exp: no_header: step 7
+FAIL: gdb.cp/step-and-next-inline.exp: no_header: step 7
 PASS: gdb.cp/step-and-next-inline.exp: no_header: step into get_alias_set
 PASS: gdb.cp/step-and-next-inline.exp: no_header: step into get_alias_set pass 2
 PASS: gdb.cp/step-and-next-inline.exp: use_header: in get_alias_set pass 2
-PASS: gdb.cp/step-and-next-inline.exp: use_header: in inline 1 pass 2
-PASS: gdb.cp/step-and-next-inline.exp: use_header: in inline 2 pass 2
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: in inline 1 pass 2
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: in inline 2 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: use_header: in inline 3 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: use_header: in main
 PASS: gdb.cp/step-and-next-inline.exp: use_header: in main pass 2
-PASS: gdb.cp/step-and-next-inline.exp: use_header: next step 1
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: next step 1
 PASS: gdb.cp/step-and-next-inline.exp: use_header: next step 2
-PASS: gdb.cp/step-and-next-inline.exp: use_header: next step 3
-PASS: gdb.cp/step-and-next-inline.exp: use_header: next step 4
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: next step 3
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: next step 4
 PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 1
-PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 1 pass 2
-PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 2
-PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 2 pass 2
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: not in inline 1 pass 2
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: not in inline 2
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: not in inline 2 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 3
 PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 3 pass 2
-PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 4
-PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 4 pass 2
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: not in inline 4
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: not in inline 4 pass 2
 PASS: gdb.cp/step-and-next-inline.exp: use_header: not in inline 5
-PASS: gdb.cp/step-and-next-inline.exp: use_header: step 1
-PASS: gdb.cp/step-and-next-inline.exp: use_header: step 2
-PASS: gdb.cp/step-and-next-inline.exp: use_header: step 3
-PASS: gdb.cp/step-and-next-inline.exp: use_header: step 4
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: step 1
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: step 2
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: step 3
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: step 4
 PASS: gdb.cp/step-and-next-inline.exp: use_header: step 5
 PASS: gdb.cp/step-and-next-inline.exp: use_header: step 6
-PASS: gdb.cp/step-and-next-inline.exp: use_header: step 7
+FAIL: gdb.cp/step-and-next-inline.exp: use_header: step 7
 PASS: gdb.cp/step-and-next-inline.exp: use_header: step into get_alias_set
 PASS: gdb.cp/step-and-next-inline.exp: use_header: step into get_alias_set pass 2
 /gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/subtypes.exp ...
@@ -52391,7 +52381,7 @@
 PASS: gdb.dwarf2/dw2-inline-header-2.exp: check for breakpoint at dw2-inline-header.c
 PASS: gdb.dwarf2/dw2-inline-header-2.exp: check for breakpoint at dw2-inline-header.h
 /gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.dwarf2/dw2-inline-header-3.exp ...
-FAIL: gdb.dwarf2/dw2-inline-header-3.exp: continue to breakpoint: dw2-inline-header.h:22
+PASS: gdb.dwarf2/dw2-inline-header-3.exp: continue to breakpoint: dw2-inline-header.h:22
 PASS: gdb.dwarf2/dw2-inline-header-3.exp: found line 19 and 20
 PASS: gdb.dwarf2/dw2-inline-header-3.exp: info breakpoints

So if it would be a socker game that would be 26:1 :-/

Regarding the one test where my patch is still able to be improved,
gdb.dwarf2/dw2-inline-header-3.exp: continue to breakpoint: dw2-inline-header.h:22

I think the issue there is that dw2-inline-header.h:22
and dw2-inline-header.c:18 is at the same adress.
To me it appears a bit arbitrary, which of the two lines the stack trace shows,
but on the other hand, if there is a breakpoint on dw2-inline-header.h:22
that is a clear indication that the user want to see  dw2-inline-header.h:22
while if there is a breakpoint on dw2-inline-header.c:18 the user would
certainly want to see dw2-inline-header.c:18 even if both are only different
views of the same address.  Yes.  I wonder if we can choose the SAL entry
which is presented, in a was that prefers SAL where there is a breakpoint.
That can't be impossible given all we alread achieved in this cursade ;-)
for better debug experience, don't you agree?

Looking at the debug info in the test case, I see a wrong subroutine
range here:

 <2><52>: Abbrev Number: 5 (DW_TAG_inlined_subroutine)
    <53>   DW_AT_abstract_origin: <0x31>
    <57>   DW_AT_low_pc      : 0x401122
    <5f>   DW_AT_high_pc     : 0x40115e
    <67>   DW_AT_call_file   : 1
    <68>   DW_AT_call_line   : 18

See the line table the subroutine is from 0x401136 to 0x401140:

  [0x000000ba]  Set File Name to entry 2 in the File Name Table
  [0x000000bc]  Extended opcode 2: set Address to 0x401136
  [0x000000c7]  Advance Line by 4 to 21
  [0x000000c9]  Copy
  [0x000000ca]  Extended opcode 2: set Address to 0x401140
  [0x000000d5]  Advance Line by 1 to 22
  [0x000000d7]  Copy
  [0x000000d8]  Advance Line by -4 to 18
  [0x000000da]  Set File Name to entry 1 in the File Name Table


with that wrong subroutine table I think my patch cas not a chance
to give correct results.  Could you please fix the subroutine
table, and then we see what I have to fix on my patch in order
to make it pass?

Also please check the subroutine entries on the other
dw2-inline-header tests, if the subroutine range info have similar
problems, I need correct ranges, otherwise my patch is unable to
produce correct output.


Thanks
Bernd.