[PATCH] gdb: use make_scoped_restore to restore gdbpy_current_objfile

[PATCH] gdb: use make_scoped_restore to restore gdbpy_current_objfile

[Andrew Burgess]

The current mechanism by which the Python gdb.current_objfile is
maintained does not allow for nested auto-load events.  It is assumed
that one an auto-load script has finished loading then the current
objfile should be set back to NULL.  In a nested situation, we should
be restoring the previous value.

We already have an RAII class to handle save/restore type behaviour,
so lets just switch to use that.

The test is a little contrived, but is simple enough, and triggers the
bug.  The real use case might involve the auto-load script calling
functions (either in the just-loaded object file, or in the main
executable), which in turn trigger further auto-loads to occur.

gdb/ChangeLog:

	* python/python.c (gdbpy_source_objfile_script): Use
	make_scoped_restore to restore gdbpy_current_objfile.
	(gdbpy_execute_objfile_script): Likewise.

gdb/testsuite/ChangeLog:

	* gdb.python/py-auto-load-chaining-f1.c: New file.
	* gdb.python/py-auto-load-chaining-f1.o-gdb.py: New file.
	* gdb.python/py-auto-load-chaining-f2.c: New file.
	* gdb.python/py-auto-load-chaining-f2.o-gdb.py: New file.
	* gdb.python/py-auto-load-chaining.c: New file.
	* gdb.python/py-auto-load-chaining.exp: New file.
---
 gdb/ChangeLog                                 |  6 ++
 gdb/python/python.c                           | 10 +--
 gdb/testsuite/ChangeLog                       |  9 +++
 .../gdb.python/py-auto-load-chaining-f1.c     | 24 ++++++
 .../py-auto-load-chaining-f1.o-gdb.py         | 37 +++++++++
 .../gdb.python/py-auto-load-chaining-f2.c     | 24 ++++++
 .../py-auto-load-chaining-f2.o-gdb.py         | 24 ++++++
 .../gdb.python/py-auto-load-chaining.c        | 58 ++++++++++++++
 .../gdb.python/py-auto-load-chaining.exp      | 78 +++++++++++++++++++
 9 files changed, 264 insertions(+), 6 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-auto-load-chaining-f1.c
 create mode 100644 gdb/testsuite/gdb.python/py-auto-load-chaining-f1.o-gdb.py
 create mode 100644 gdb/testsuite/gdb.python/py-auto-load-chaining-f2.c
 create mode 100644 gdb/testsuite/gdb.python/py-auto-load-chaining-f2.o-gdb.py
 create mode 100644 gdb/testsuite/gdb.python/py-auto-load-chaining.c
 create mode 100644 gdb/testsuite/gdb.python/py-auto-load-chaining.exp

diff --git a/gdb/python/python.c b/gdb/python/python.c
index 009c0c4c6d7..9eed258c181 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1388,11 +1388,10 @@ gdbpy_source_objfile_script (const struct extension_language_defn *extlang,
     return;
 
   gdbpy_enter enter_py (objfile->arch (), current_language);
-  gdbpy_current_objfile = objfile;
+  scoped_restore restire_current_objfile
+    = make_scoped_restore (&gdbpy_current_objfile, objfile);
 
   python_run_simple_file (file, filename);
-
-  gdbpy_current_objfile = NULL;
 }
 
 /* Set the current objfile to OBJFILE and then execute SCRIPT
@@ -1410,11 +1409,10 @@ gdbpy_execute_objfile_script (const struct extension_language_defn *extlang,
     return;
 
   gdbpy_enter enter_py (objfile->arch (), current_language);
-  gdbpy_current_objfile = objfile;
+  scoped_restore restire_current_objfile
+    = make_scoped_restore (&gdbpy_current_objfile, objfile);
 
   PyRun_SimpleString (script);
-
-  gdbpy_current_objfile = NULL;
 }
 
 /* Return the current Objfile, or None if there isn't one.  */
diff --git a/gdb/testsuite/gdb.python/py-auto-load-chaining-f1.c b/gdb/testsuite/gdb.python/py-auto-load-chaining-f1.c
new file mode 100644
index 00000000000..77250bd91d6
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-auto-load-chaining-f1.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+/* Just a dummy function.  */
+
+int
+f1 ()
+{
+  return 1;
+}
diff --git a/gdb/testsuite/gdb.python/py-auto-load-chaining-f1.o-gdb.py b/gdb/testsuite/gdb.python/py-auto-load-chaining-f1.o-gdb.py
new file mode 100644
index 00000000000..4f5a4e78594
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-auto-load-chaining-f1.o-gdb.py
@@ -0,0 +1,37 @@
+# Copyright (C) 2021 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/>.
+
+# This script is auto-loaded when the py-auto-load-chaining-f1.o
+# object is loaded.
+
+import re
+
+print ("Entering f1.o auto-load script")
+
+print ("Current objfile is: %s"
+       % gdb.current_objfile ().filename)
+
+print ("Chain loading f2.o...")
+
+filename = gdb.current_objfile ().filename
+filename = re.sub (r"-f1.o$", "-f2.o", filename)
+r2 = gdb.lookup_global_symbol ('region_2').value ()
+gdb.execute ("add-symbol-file %s 0x%x" % (filename, r2))
+
+print ("After loading f2.o...")
+print ("Current objfile is: %s"
+       % gdb.current_objfile ().filename)
+
+print ("Leaving f1.o auto-load script")
diff --git a/gdb/testsuite/gdb.python/py-auto-load-chaining-f2.c b/gdb/testsuite/gdb.python/py-auto-load-chaining-f2.c
new file mode 100644
index 00000000000..f2eeab8f389
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-auto-load-chaining-f2.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+/* Just a dummy function.  */
+
+int
+f2 ()
+{
+  return 2;
+}
diff --git a/gdb/testsuite/gdb.python/py-auto-load-chaining-f2.o-gdb.py b/gdb/testsuite/gdb.python/py-auto-load-chaining-f2.o-gdb.py
new file mode 100644
index 00000000000..f5c29cd792f
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-auto-load-chaining-f2.o-gdb.py
@@ -0,0 +1,24 @@
+# Copyright (C) 2021 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/>.
+
+# This script is auto-loaded when the py-auto-load-chaining-f2.o
+# object is loaded.
+
+print ("Entering f2.o auto-load script")
+
+print ("Current objfile is: %s"
+       % gdb.current_objfile ().filename)
+
+print ("Leaving f2.o auto-load script")
diff --git a/gdb/testsuite/gdb.python/py-auto-load-chaining.c b/gdb/testsuite/gdb.python/py-auto-load-chaining.c
new file mode 100644
index 00000000000..5b7feed90aa
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-auto-load-chaining.c
@@ -0,0 +1,58 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <stdio.h>
+
+/* These will hold the addresses for two memory regions.  */
+
+void *region_1;
+void *region_2;
+
+/* Allocate a page of memory using mmap, and return a pointer.  */
+
+void *
+allocate_page (void)
+{
+  void *addr;
+  int pgsize = sysconf(_SC_PAGE_SIZE);
+  addr = mmap (NULL, pgsize, PROT_EXEC | PROT_READ | PROT_WRITE,
+               MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (addr == MAP_FAILED)
+    perror ("mmap");
+}
+
+/* Only called so we can create a breakpoint.  */
+
+void
+breakpt (void)
+{
+  /* Nothing.  */
+}
+
+/* The test.  */
+
+int
+main (void)
+{
+  region_1 = allocate_page ();
+  region_2 = allocate_page ();
+
+  breakpt ();	/* Break Here.  */
+}
diff --git a/gdb/testsuite/gdb.python/py-auto-load-chaining.exp b/gdb/testsuite/gdb.python/py-auto-load-chaining.exp
new file mode 100644
index 00000000000..29414e8a775
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-auto-load-chaining.exp
@@ -0,0 +1,78 @@
+# Copyright (C) 2021 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/>.
+
+# Test that the value of gdb.current_objfile is correct even when one
+# auto-load script loads a second objfile, which triggers the
+# execution of another (nested) objfile script.
+
+load_lib gdb-python.exp
+
+standard_testfile .c -f1.c -f2.c
+
+# Two additional object files needed for this test.
+set f1_o [standard_output_file ${gdb_test_file_name}-f1.o]
+set f2_o [standard_output_file ${gdb_test_file_name}-f2.o]
+
+# Now build the object files.
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile2}" ${f1_o} object {}] != ""} {
+  untested "failed to compile object file f1.o"
+  return -1
+}
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile3}" ${f2_o} object {}] != ""} {
+  untested "failed to compile object file f2.o"
+  return -1
+}
+
+# Copy the two Python scripts to where the tests are being run.
+set remote_python_file [gdb_remote_download host \
+			    ${srcdir}/${subdir}/${testfile}-f1.o-gdb.py]
+set remote_python_file [gdb_remote_download host \
+			    ${srcdir}/${subdir}/${testfile}-f2.o-gdb.py]
+
+# Build the main test executable and start GDB.
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}]} {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main] {
+   return -1
+}
+
+set safe_path [standard_output_file ""]
+gdb_test_no_output "set auto-load safe-path ${safe_path}" \
+    "set auto-load safe-path"
+
+gdb_breakpoint [gdb_get_line_number "Break Here"]
+gdb_continue_to_breakpoint "run to test breakpoint"
+
+gdb_test_no_output "set confirm off"
+gdb_test "add-symbol-file ${f1_o} region_1" \
+    [multi_line \
+	 "Entering f1\\.o auto-load script" \
+	 "Current objfile is: \[^\r\n\]+/py-auto-load-chaining-f1\\.o" \
+	 "Chain loading f2\\.o\\.\\.\\." \
+	 "add symbol table from file \"\[^\r\n\]+/py-auto-load-chaining-f2\\.o\" at" \
+	 "\\s+\\.text_addr = $hex" \
+	 "Entering f2\\.o auto-load script" \
+	 "Current objfile is: \[^\r\n\]+/py-auto-load-chaining-f2\\.o" \
+	 "Leaving f2\\.o auto-load script" \
+	 "After loading f2\\.o\\.\\.\\." \
+	 "Current objfile is: \[^\r\n\]+/py-auto-load-chaining-f1\\.o" \
+	 "Leaving f1\\.o auto-load script"] \
+    "add-symbol-file f1.o"
-- 
2.25.4

Re: [PATCH] gdb: use make_scoped_restore to restore gdbpy_current_objfile

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> The current mechanism by which the Python gdb.current_objfile is
Andrew> maintained does not allow for nested auto-load events.  It is assumed
Andrew> that one an auto-load script has finished loading then the current
Andrew> objfile should be set back to NULL.  In a nested situation, we should
Andrew> be restoring the previous value.

Andrew> We already have an RAII class to handle save/restore type behaviour,
Andrew> so lets just switch to use that.

Andrew> The test is a little contrived, but is simple enough, and triggers the
Andrew> bug.  The real use case might involve the auto-load script calling
Andrew> functions (either in the just-loaded object file, or in the main
Andrew> executable), which in turn trigger further auto-loads to occur.

That test sure looks like a lot of work :-)

Andrew> 	* python/python.c (gdbpy_source_objfile_script): Use
Andrew> 	make_scoped_restore to restore gdbpy_current_objfile.
Andrew> 	(gdbpy_execute_objfile_script): Likewise.

This looks good to me.

Tom