* [PATCH] gdb/rust: fix crash for expression debug with strings
@ 2023-05-09 11:23 Andrew Burgess
2023-05-09 14:22 ` Tom Tromey
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Burgess @ 2023-05-09 11:23 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
While working on another patch I did this:
(gdb) set debug expression 1
(gdb) set language rust
(gdb) p "foo"
Operation: OP_AGGREGATE
Type: &str
Fatal signal: Segmentation fault
... etc ...
The problem is that the second field of the rust_aggregate_operation
is created as a nullptr, this can be seen in rust-parse.c. in the
function rust_parser::parse_string().
However, in expop.h, in the function dump_for_expression, we make the
assumption that the expressions will never be nullptr.
I did consider moving the nullptr handling into a new function
rust_aggregate_operation::dump, however, as the expression debug
dumping code is not exercised as much as it might be, I would rather
that this code be hardened and able to handle a nullptr without
crashing, so I propose that we add nullptr handling into the general
dump_for_expression function. The behaviour is now:
(gdb) set debug expression 1
(gdb) set language rust
(gdb) p "foo"
Operation: OP_AGGREGATE
Type: &str
nullptr
Vector:
String: data_ptr
Operation: UNOP_ADDR
Operation: OP_STRING
String: foo
String: length
Operation: OP_LONG
Type: usize
Constant: 3
evaluation of this expression requires the target program to be active
(gdb)
There's a new test to check for this case.
---
gdb/expop.h | 5 ++++-
gdb/testsuite/gdb.rust/expr.exp | 21 +++++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/gdb/expop.h b/gdb/expop.h
index 854945c8688..a9da11cc70d 100644
--- a/gdb/expop.h
+++ b/gdb/expop.h
@@ -314,7 +314,10 @@ static inline void
dump_for_expression (struct ui_file *stream, int depth,
const operation_up &op)
{
- op->dump (stream, depth);
+ if (op == nullptr)
+ gdb_printf (stream, _("%*snullptr\n"), depth, "");
+ else
+ op->dump (stream, depth);
}
extern void dump_for_expression (struct ui_file *stream, int depth,
diff --git a/gdb/testsuite/gdb.rust/expr.exp b/gdb/testsuite/gdb.rust/expr.exp
index 908d1a3680b..ce2cce2677c 100644
--- a/gdb/testsuite/gdb.rust/expr.exp
+++ b/gdb/testsuite/gdb.rust/expr.exp
@@ -148,3 +148,24 @@ gdb_test "print r#" "No symbol 'r' in current context"
gdb_test "printf \"%d %d\\n\", 23+1, 23-1" "24 22"
gdb_test "print 5," "Syntax error near ','"
+
+# Check expression debug works for strings.
+gdb_test "with debug expression 1 -- print \"foo\"" \
+ [multi_line \
+ "Operation: OP_AGGREGATE" \
+ " Type: &str" \
+ " nullptr" \
+ " Vector:" \
+ " String: data_ptr" \
+ " Operation: UNOP_ADDR" \
+ " Operation: OP_STRING" \
+ " String: foo" \
+ " String: length" \
+ " Operation: OP_LONG" \
+ " Type: usize" \
+ " Constant: 3" \
+ "Operation: OP_LONG" \
+ " Type: i32" \
+ " Constant: 0" \
+ "evaluation of this expression requires the target program to be active"] \
+ "print a string with expression debug turned on"
base-commit: 55a75aae9d971d3d0f49884e3954ac4794559542
--
2.25.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb/rust: fix crash for expression debug with strings
2023-05-09 11:23 [PATCH] gdb/rust: fix crash for expression debug with strings Andrew Burgess
@ 2023-05-09 14:22 ` Tom Tromey
2023-05-10 14:01 ` Andrew Burgess
0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2023-05-09 14:22 UTC (permalink / raw)
To: Andrew Burgess via Gdb-patches; +Cc: Andrew Burgess
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
Andrew> I did consider moving the nullptr handling into a new function
Andrew> rust_aggregate_operation::dump, however, as the expression debug
Andrew> dumping code is not exercised as much as it might be, I would rather
Andrew> that this code be hardened and able to handle a nullptr without
Andrew> crashing, so I propose that we add nullptr handling into the general
Andrew> dump_for_expression function. The behaviour is now:
Makes sense to me.
Reviewed-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdb/rust: fix crash for expression debug with strings
2023-05-09 14:22 ` Tom Tromey
@ 2023-05-10 14:01 ` Andrew Burgess
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Burgess @ 2023-05-10 14:01 UTC (permalink / raw)
To: Tom Tromey, Andrew Burgess via Gdb-patches
Tom Tromey <tom@tromey.com> writes:
>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> I did consider moving the nullptr handling into a new function
> Andrew> rust_aggregate_operation::dump, however, as the expression debug
> Andrew> dumping code is not exercised as much as it might be, I would rather
> Andrew> that this code be hardened and able to handle a nullptr without
> Andrew> crashing, so I propose that we add nullptr handling into the general
> Andrew> dump_for_expression function. The behaviour is now:
>
> Makes sense to me.
>
> Reviewed-By: Tom Tromey <tom@tromey.com>
Pushed.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-10 14:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-09 11:23 [PATCH] gdb/rust: fix crash for expression debug with strings Andrew Burgess
2023-05-09 14:22 ` Tom Tromey
2023-05-10 14:01 ` Andrew Burgess
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).