Fix an issue with the gdb step-over aka. "n" command

Message ID AM0PR08MB37142BC0D34B9B3605D37034E4250@AM0PR08MB3714.eurprd08.prod.outlook.com
State New, archived
Headers

Commit Message

Bernd Edlinger Dec. 28, 2019, 8:40 a.m. UTC
  On 12/20/19 7:13 AM, Simon Marchi wrote:
> Hi Bernd,
> 
> On 2019-12-19 5:53 p.m., Bernd Edlinger wrote:
>> Does this explanation make sense?
> 
> Yes.  Well I think so, I have to admit this is a bit over my head,
> there are a lot of pieces to put together to have a good understanding
> of the problem.  I just did a first read, I'll sleep on it and come
> back to it later.
> 
> Thanks for the small reproducer, this is extremely valuable.  I think it
> will be a good idea to integrate it as a test case in the test suite.
> 
> In your patch to dwarf2read.c, I was a bit surprised to see:
> 
>   m_last_subfile != m_cu->get_builder ()->get_current_subfile ()
> 
> So your fix only works if the inlined subroutine comes from another file?  If
> I move the tree_check function in next-inline.cc, the fix no longer applies,
> and we get the broken behavior.  From your previous email, I understand that
> this is expected.  I guess that if both are in the same file, we can't detect
> this situation using the same technique.
> 
> I also read about location views, since that's what Alexandre referred to.  It
> sounds like it's a magic solution that will allow GDB to do the right thing in
> this kind of situation.  If that's indeed the case, then it might be good to start
> exploring this path.  I'd like to have a better understanding of how this will help
> GDB make a smarter "next", and what kind of effort is needed to make GDB use it.  My
> understanding is that location views allow having an address mapped to multiple
> source locations.  For example, here's the problematic address in the next-inline
> test case I've compiled:
> 
>   ./next-inline.h:[++]
>   next-inline.h                                 28              0x1175               x
>   next-inline.h                                 30              0x1175       1       x
> 
>   ./next-inline.cc:[++]
>   next-inline.cc                                22              0x1175       2
> 
> Today, when I ask GDB "which source line does this address correspond to", it gives me
> one answer.  Does this mean that GDB will now say that 0x1175 corresponds to
> 
> - next-inline.h:28
> - next-inline.h:30
> - next-inline.cc:22
> 
> all at the same time?  Is one of these source locations more important than the others?
> If execution happens to stop exactly at this address, which location do we present to
> the user?
> 
> And to come back the problem at hand, how does this help GDB make a smarter "next"?
> 
> Btw, I stumbled on a bug with the TUI source display.  It might be caused by this patch,
> or it might be that this patch uncovers it.
> 
> When I do these actions:
> 
> - Start GDB with the next-inline test file (from this patch)
> - Enable the TUI
> - Type "start"
> - Type "s"
> - Type "n" twice
> 
> The TUI source display wrongfully jumps to the header file, line 24.
> When I type "frame", it says I'm stopped at next-line.cc:24.  So it
> is showing the right line number of the wrong file.
> 

I think meanwhile the display bug in the TUI window is fixed (by one of Tom Tromey's
recent patches), could you please check again?

I post both parts of the patch again for your reference here, and add a ChangeLog
which I forgot previously:

gdb:
2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* dwarf2read.c (lnp_state_machine): New member m_last_address.
	(lnp_state_machine::record_line): After a file changes or
	end sequence always assume a statement boundary.

gdb/testsuite:
2019-12-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.cp/next-inline.exp: New file.
	* gdb.cp/next-inline.cc: New file.
	* gdb.cp/next-inline.h: New file.



Thanks
Bernd.
  

Patch

From 95ba4780488b53104ea51cc0702f99a9a800984b Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Thu, 19 Dec 2019 23:41:37 +0100
Subject: [PATCH 2/2] Add a test case for step over inline functions

---
 gdb/testsuite/gdb.cp/next-inline.cc  | 34 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/next-inline.exp | 39 ++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.cp/next-inline.h   | 34 +++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+)
 create mode 100644 gdb/testsuite/gdb.cp/next-inline.cc
 create mode 100644 gdb/testsuite/gdb.cp/next-inline.exp
 create mode 100644 gdb/testsuite/gdb.cp/next-inline.h

diff --git a/gdb/testsuite/gdb.cp/next-inline.cc b/gdb/testsuite/gdb.cp/next-inline.cc
new file mode 100644
index 0000000..dcf5ab9
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/next-inline.cc
@@ -0,0 +1,34 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "next-inline.h"
+
+int __attribute__((noinline, noclone))
+get_alias_set (tree *t)
+{ if (t != NULL
+      && TREE_TYPE (t).z != 1
+      && TREE_TYPE (t).z != 2
+      && TREE_TYPE (t).z != 3)
+    return 0;
+  return 1;
+}
+
+tree xx;
+int main()
+{ get_alias_set(&xx);
+  abort();
+}
diff --git a/gdb/testsuite/gdb.cp/next-inline.exp b/gdb/testsuite/gdb.cp/next-inline.exp
new file mode 100644
index 0000000..6badc8c
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/next-inline.exp
@@ -0,0 +1,39 @@ 
+#   Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" "next-inline" \
+			  {next-inline.cc} \
+			  {debug nowarnings optimize=-O2}] } {
+    return -1
+}
+
+if ![runto_main] {
+    fail "can't run to main"
+    return
+}
+
+gdb_test "bt" "\\s*\\#0\\s+main.*" "in main"
+gdb_test "step" ".*" "step into get_alias_set"
+gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline"
+gdb_test "next" ".*" "next step 1"
+gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline 1"
+gdb_test "next" ".*" "next step 2"
+gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline 2"
+gdb_test "next" ".*" "next step 3"
+gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline 3"
+gdb_test "next" ".*" "next step 4"
+gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*next-inline\\.cc:.*" "not in inline 4"
diff --git a/gdb/testsuite/gdb.cp/next-inline.h b/gdb/testsuite/gdb.cp/next-inline.h
new file mode 100644
index 0000000..99fb1b2
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/next-inline.h
@@ -0,0 +1,34 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+struct tree{
+  volatile int x;
+  volatile int z;
+};
+
+#define TREE_TYPE(NODE) (*tree_check (NODE, 0))
+
+inline tree *
+tree_check (tree *t, int i)
+{
+  if (t->x != i)
+    abort();
+  tree *x = t;
+  return x;
+}
-- 
1.9.1