gdb: quote inferior arguments, if needed, when opening a core file
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
This commit fixes an issue with the commit:
commit d3d13bf876aae425ae0eff2ab0f1af9f7da0264a
Date: Thu Apr 25 09:36:43 2024 +0100
gdb: add gdbarch method to get execution context from core file
The above commit improves GDB's ability to display inferior arguments
when opening a core file, however, if an argument includes white
space, then this is not displayed as well as it should be. For
example:
(gdb) core-file /tmp/corefile-exec-context.2.core
[New LWP 4069711]
Reading symbols from /tmp/corefile-exec-context...
Core was generated by `/tmp/corefile-exec-context aaaaa bbbbb ccccc ddddd e e e e e'.
Program terminated with signal SIGABRT, Aborted.
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 return ret;
(gdb) show args
Argument list to give program being debugged when it is started is "aaaaa bbbbb ccccc ddddd e\ e\ e\ e\ e".
(gdb)
Notice the 'Core was generated by ...' line. In this case it is not
clear if the "e e e e e" is a single argument containing white space,
or 5 single arguments.
But when we 'show args' it is immediately clear that this is a single
argument, as the white space is now escaped.
This problem was caused by the above commit building the argument
string itself, and failing to consider white space escaping.
This commit changes things around, first we place the arguments into
the inferior, then, to print the 'Core was generated by ...' line, we
ask the inferior for the argument string. In this way the quoting is
handled just as it is for 'show args'. The initial output is now:
(gdb) core-file /tmp/corefile-exec-context.2.core
[New LWP 4069711]
Reading symbols from /tmp/corefile-exec-context...
Core was generated by `/tmp/corefile-exec-context aaaaa bbbbb ccccc ddddd e\ e\ e\ e\ e'.
Program terminated with signal SIGABRT, Aborted.
#0 0x00007f4f007af625 in raise () from /lib64/libc.so.6
(gdb)
Much better. The existing test is extended to cover this case.
---
gdb/corelow.c | 21 +++++++----------
.../gdb.base/corefile-exec-context.exp | 23 ++++++++++++++++++-
2 files changed, 30 insertions(+), 14 deletions(-)
base-commit: ac8f3fc9330da0302ebb491bf2bac8da5e035e35
Comments
On 1/15/25 12:54 PM, Andrew Burgess wrote:
> This commit fixes an issue with the commit:
>
> commit d3d13bf876aae425ae0eff2ab0f1af9f7da0264a
> Date: Thu Apr 25 09:36:43 2024 +0100
>
> gdb: add gdbarch method to get execution context from core file
>
> The above commit improves GDB's ability to display inferior arguments
> when opening a core file, however, if an argument includes white
> space, then this is not displayed as well as it should be. For
> example:
>
> (gdb) core-file /tmp/corefile-exec-context.2.core
> [New LWP 4069711]
> Reading symbols from /tmp/corefile-exec-context...
> Core was generated by `/tmp/corefile-exec-context aaaaa bbbbb ccccc ddddd e e e e e'.
> Program terminated with signal SIGABRT, Aborted.
> #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> 50 return ret;
> (gdb) show args
> Argument list to give program being debugged when it is started is "aaaaa bbbbb ccccc ddddd e\ e\ e\ e\ e".
> (gdb)
>
> Notice the 'Core was generated by ...' line. In this case it is not
> clear if the "e e e e e" is a single argument containing white space,
> or 5 single arguments.
>
> But when we 'show args' it is immediately clear that this is a single
> argument, as the white space is now escaped.
>
> This problem was caused by the above commit building the argument
> string itself, and failing to consider white space escaping.
>
> This commit changes things around, first we place the arguments into
> the inferior, then, to print the 'Core was generated by ...' line, we
> ask the inferior for the argument string. In this way the quoting is
> handled just as it is for 'show args'. The initial output is now:
>
> (gdb) core-file /tmp/corefile-exec-context.2.core
> [New LWP 4069711]
> Reading symbols from /tmp/corefile-exec-context...
> Core was generated by `/tmp/corefile-exec-context aaaaa bbbbb ccccc ddddd e\ e\ e\ e\ e'.
> Program terminated with signal SIGABRT, Aborted.
> #0 0x00007f4f007af625 in raise () from /lib64/libc.so.6
> (gdb)
>
> Much better. The existing test is extended to cover this case.
This is a nice change, thank you for doing this.
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> This commit changes things around, first we place the arguments into
Andrew> the inferior, then, to print the 'Core was generated by ...' line, we
Andrew> ask the inferior for the argument string. In this way the quoting is
Andrew> handled just as it is for 'show args'. The initial output is now:
Looks good to me. Thank you.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
Tom Tromey <tom@tromey.com> writes:
>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> This commit changes things around, first we place the arguments into
> Andrew> the inferior, then, to print the 'Core was generated by ...' line, we
> Andrew> ask the inferior for the argument string. In this way the quoting is
> Andrew> handled just as it is for 'show args'. The initial output is now:
>
> Looks good to me. Thank you.
> Approved-By: Tom Tromey <tom@tromey.com>
Tom / Gwen,
Thanks for the quick review. I pushed to master and gdb-16-branch.
Thanks,
Andrew
@@ -1169,27 +1169,22 @@ core_target_open (const char *arg, int from_tty)
if (ctx.valid ())
{
- std::string args;
- for (const auto &a : ctx.args ())
- {
- args += ' ';
- args += a.get ();
- }
-
- gdb_printf (_("Core was generated by `%ps%s'.\n"),
- styled_string (file_name_style.style (),
- ctx.execfn ()),
- args.c_str ());
-
/* Copy the arguments into the inferior. */
std::vector<char *> argv;
- for (const auto &a : ctx.args ())
+ for (const gdb::unique_xmalloc_ptr<char> &a : ctx.args ())
argv.push_back (a.get ());
gdb::array_view<char * const> view (argv.data (), argv.size ());
current_inferior ()->set_args (view);
/* And now copy the environment. */
current_inferior ()->environment = ctx.environment ();
+
+ /* Inform the user of executable and arguments. */
+ const std::string &args = current_inferior ()->args ();
+ gdb_printf (_("Core was generated by `%ps%s%s'.\n"),
+ styled_string (file_name_style.style (),
+ ctx.execfn ()),
+ (args.length () > 0 ? " " : ""), args.c_str ());
}
else
{
@@ -69,7 +69,7 @@ gdb_test_multiple "core-file $corefile_1" "load core file no args" {
}
# Generate a core file, this time pass some arguments to the inferior.
-set args "aaaaa bbbbb ccccc ddddd eeeee"
+set args "aaaaa bbbbb ccccc ddddd e\\\\ e\\\\ e\\\\ e\\\\ e"
set corefile [core_find $binfile {} $args]
if {$corefile == ""} {
untested "unable to create corefile"
@@ -101,6 +101,27 @@ gdb_test_multiple "core-file $corefile_2" "load core file with args" {
gdb_test "show args" \
"Argument list to give program being debugged when it is started is \"$args\"\\."
+# Move up to 'main'. Do it this way because we cannot know how many
+# frames up 'main' actually is.
+gdb_test_multiple "up" "move up to main" {
+ -re -wrap "#$decimal\\s+\[^\r\n\]+ in main .*" {
+ pass $gdb_test_name
+ }
+
+ -re -wrap "#$decimal\\s+\[^\r\n\]+ in .*" {
+ send_gdb "up\n"
+ exp_continue
+ }
+}
+
+# Check that the inferior was started with the expected arguments.
+gdb_test "print argc" " = 6"
+gdb_test "print argv\[1\]" " = $hex \"aaaaa\""
+gdb_test "print argv\[2\]" " = $hex \"bbbbb\""
+gdb_test "print argv\[3\]" " = $hex \"ccccc\""
+gdb_test "print argv\[4\]" " = $hex \"ddddd\""
+gdb_test "print argv\[5\]" " = $hex \"e e e e e\""
+
# Find the name of an environment variable that is not set.
set env_var_base "GDB_TEST_ENV_VAR_"
set env_var_name ""