From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 2/2] gdb/python: fix invalid use disassemble_info::stream
Date: Wed, 20 Jul 2022 14:14:37 +0100 [thread overview]
Message-ID: <681fea7207c965b614f1970c638bc7bb0898bd93.1658322626.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1658322626.git.aburgess@redhat.com>
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
next prev parent reply other threads:[~2022-07-20 13:14 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-17 10:36 [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess
2022-06-17 10:36 ` [PATCH 1/2] gdb: have gdb_disassemble_info carry 'this' in its stream pointer Andrew Burgess
2022-06-17 10:36 ` [PATCH 2/2] gdb: add support for disassembler styling using libopcodes Andrew Burgess
2022-06-17 11:16 ` Eli Zaretskii
2022-06-30 12:22 ` Andrew Burgess
2022-06-30 13:46 ` Eli Zaretskii
2022-07-04 10:15 ` Andrew Burgess
2022-07-20 2:09 ` Simon Marchi
2022-07-20 13:14 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess
2022-07-20 13:14 ` [PATCH 1/2] gdb: fix use of uninitialised gdb_printing_disassembler::m_in_comment Andrew Burgess
2022-07-20 14:21 ` Simon Marchi
2022-07-20 13:14 ` Andrew Burgess [this message]
2022-07-25 18:27 ` [PATCH 0/2] Fix some undefined haviour bugs in Python disassembler API Andrew Burgess
2022-07-04 10:17 ` [PATCH 0/2] Use libopcodes disassembler styling with GDB Andrew Burgess
2022-07-11 14:18 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=681fea7207c965b614f1970c638bc7bb0898bd93.1658322626.git.aburgess@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).