[PATCH 2/2] gdb/python: fix invalid use disassemble_info::stream

Andrew Burgess aburgess@redhat.com
Wed Jul 20 13:14:37 GMT 2022


After this commit:

  commit 81384924cdcc9eb2676dd9084b76845d7d0e0759
  Date:   Tue Apr 5 11:06:16 2022 +0100

      gdb: have gdb_disassemble_info carry 'this' in its stream pointer

The disassemble_info::stream field will no longer be a ui_file*.  That
commit failed to update one location in py-disasm.c though.

While running some tests using the Python disassembler API, I
triggered a call to gdbpy_disassembler::print_address_func, and, as I
had compiled GDB with the undefined behaviour sanitizer, GDB crashed
as the code currently (incorrectly) casts the stream field to be a
ui_file*.

In this commit I fix this error.

In order to test this case I had to tweak the existing test case a
little.  I also spotted some debug printf statements in py-disasm.py,
which I have removed.
---
 gdb/python/py-disasm.c                 |  2 +-
 gdb/testsuite/gdb.python/py-disasm.c   |  8 +++++++-
 gdb/testsuite/gdb.python/py-disasm.exp | 22 ++++++++++++++--------
 gdb/testsuite/gdb.python/py-disasm.py  |  3 ---
 4 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 4c78ca350c2..c37452fcf72 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -626,7 +626,7 @@ gdbpy_disassembler::print_address_func (bfd_vma addr,
 {
   gdbpy_disassembler *dis
     = static_cast<gdbpy_disassembler *> (info->application_data);
-  print_address (dis->arch (), addr, (struct ui_file *) info->stream);
+  print_address (dis->arch (), addr, dis->stream ());
 }
 
 /* constructor.  */
diff --git a/gdb/testsuite/gdb.python/py-disasm.c b/gdb/testsuite/gdb.python/py-disasm.c
index ee0bb157f4d..e5c4d2f1d0e 100644
--- a/gdb/testsuite/gdb.python/py-disasm.c
+++ b/gdb/testsuite/gdb.python/py-disasm.c
@@ -16,10 +16,16 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 int
-main ()
+test ()
 {
   asm ("nop");
   asm ("nop");	/* Break here.  */
   asm ("nop");
   return 0;
 }
+
+int
+main ()
+{
+  return test ();
+}
diff --git a/gdb/testsuite/gdb.python/py-disasm.exp b/gdb/testsuite/gdb.python/py-disasm.exp
index 1b9cd4465ac..1f94d3e60f3 100644
--- a/gdb/testsuite/gdb.python/py-disasm.exp
+++ b/gdb/testsuite/gdb.python/py-disasm.exp
@@ -110,8 +110,8 @@ foreach plan $test_plans {
 	    gdb_test_no_output "python add_global_disassembler($global_disassembler_name)"
 	}
 
-	# Disassemble main, and check the disassembler output.
-	gdb_test "disassemble main" $expected_pattern
+	# Disassemble test, and check the disassembler output.
+	gdb_test "disassemble test" $expected_pattern
     }
 }
 
@@ -138,21 +138,21 @@ with_test_prefix "DisassemblerResult errors" {
 with_test_prefix "GLOBAL tagging disassembler" {
     py_remove_all_disassemblers
     gdb_test_no_output "python gdb.disassembler.register_disassembler(TaggingDisassembler(\"GLOBAL\"), None)"
-    gdb_test "disassemble main" "${base_pattern}\\s+## tag = GLOBAL\r\n.*"
+    gdb_test "disassemble test" "${base_pattern}\\s+## tag = GLOBAL\r\n.*"
 }
 
 # Now register an architecture specific disassembler, and check it
 # overrides the global disassembler.
 with_test_prefix "LOCAL tagging disassembler" {
     gdb_test_no_output "python gdb.disassembler.register_disassembler(TaggingDisassembler(\"LOCAL\"), \"${curr_arch}\")"
-    gdb_test "disassemble main" "${base_pattern}\\s+## tag = LOCAL\r\n.*"
+    gdb_test "disassemble test" "${base_pattern}\\s+## tag = LOCAL\r\n.*"
 }
 
 # Now remove the architecture specific disassembler, and check that
 # the global disassembler kicks back in.
 with_test_prefix "GLOBAL tagging disassembler again" {
     gdb_test_no_output "python gdb.disassembler.register_disassembler(None, \"${curr_arch}\")"
-    gdb_test "disassemble main" "${base_pattern}\\s+## tag = GLOBAL\r\n.*"
+    gdb_test "disassemble test" "${base_pattern}\\s+## tag = GLOBAL\r\n.*"
 }
 
 # Check that a DisassembleInfo becomes invalid after the call into the
@@ -160,7 +160,7 @@ with_test_prefix "GLOBAL tagging disassembler again" {
 with_test_prefix "DisassembleInfo becomes invalid" {
     py_remove_all_disassemblers
     gdb_test_no_output "python add_global_disassembler(GlobalCachingDisassembler)"
-    gdb_test "disassemble main" "${base_pattern}\\s+## CACHED\r\n.*"
+    gdb_test "disassemble test" "${base_pattern}\\s+## CACHED\r\n.*"
     gdb_test "python GlobalCachingDisassembler.check()" "PASS"
 }
 
@@ -168,10 +168,10 @@ with_test_prefix "DisassembleInfo becomes invalid" {
 with_test_prefix "memory source api" {
     py_remove_all_disassemblers
     gdb_test_no_output "python analyzing_disassembler = add_global_disassembler(AnalyzingDisassembler)"
-    gdb_test "disassemble main" "${base_pattern}\r\n.*"
+    gdb_test "disassemble test" "${base_pattern}\r\n.*"
     gdb_test "python analyzing_disassembler.find_replacement_candidate()" \
 	"Replace from $hex to $hex with NOP"
-    gdb_test "disassemble main" "${base_pattern}\r\n.*" \
+    gdb_test "disassemble test" "${base_pattern}\r\n.*" \
 	"second disassembler pass"
     gdb_test "python analyzing_disassembler.check()" \
 	"PASS"
@@ -196,6 +196,12 @@ with_test_prefix "maint info python-disassemblers" {
 	     "\[^\r\n\]+BuiltinDisassembler\\s+\\(Matches current architecture\\)" \
 	     "GLOBAL\\s+BuiltinDisassembler"] \
 	"list disassemblers, multiple disassemblers registered"
+
+    # Check that disassembling main (with the BuiltinDisassembler in
+    # place) doesn't cause GDB to crash.  The hope is that
+    # disassembling main will result in a call to print_address, which
+    # is where the problem was.
+    gdb_test "disassemble main" ".*"
 }
 
 # Check the attempt to create a "new" DisassembleInfo object fails.
diff --git a/gdb/testsuite/gdb.python/py-disasm.py b/gdb/testsuite/gdb.python/py-disasm.py
index ff7ffdb97d9..3b9008b1c54 100644
--- a/gdb/testsuite/gdb.python/py-disasm.py
+++ b/gdb/testsuite/gdb.python/py-disasm.py
@@ -584,7 +584,6 @@ class AnalyzingDisassembler(Disassembler):
         if self._nop_index is None and result.string == "nop":
             self._nop_index = len(self._pass_1_length)
             # The offset in the following read_memory call defaults to 0.
-            print("APB: Reading nop bytes")
             self._nop_bytes = info.read_memory(result.length)
 
         # Record information about each instruction that is disassembled.
@@ -661,8 +660,6 @@ class AnalyzingDisassembler(Disassembler):
     def check(self):
         """Call this after the second disassembler pass to validate the output."""
         if self._check != self._pass_2_insn:
-            print("APB, Check : %s" % self._check)
-            print("APB, Result: %s" % self._pass_2_insn)
             raise gdb.GdbError("mismatch")
         print("PASS")
 
-- 
2.25.4



More information about the Gdb-patches mailing list