From 79c024436b26a0de2f1b0ddc25f8987b4c9933b8 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Fri, 26 Mar 2021 17:14:26 +0000 Subject: [PATCH] gdb/py: fix gdb.parameter('data-directory') It was reported on IRC that using gdb.parameter('data-directory') doesn't work correctly. The problem is that the data directory is stored in 'gdb_datadir', however the set/show command is associated with a temporary 'staged_gdb_datadir'. When the user does 'set data-directory VALUE', the VALUE is stored in 'staged_gdb_datadir' by GDB, then set_gdb_datadir is called. This in turn calls set_gdb_data_directory to copy the value from staged_gdb_datadir into gdb_datadir. However, set_gdb_data_directory will resolve relative paths, so the value stored in gdb_datadir might not match the value in staged_gdb_datadir. The Python gdb.parameter API fetches the parameter values by accessing the variable associated with the show command, so in this case staged_gdb_datadir. This causes two problems: 1. Initially staged_gdb_datadir is NULL, and remains as such until the user does 'set data-directory VALUE' (which might never happen), but gdb_datadir starts with GDB's default data-directory value. So initially from Python gdb.parameter('data-directory') will return the empty string, even though at GDB's CLI 'show data-directory' prints a real path. 2. If the user does 'set data-directory ./some/relative/path', GDB will resolve the relative path, thus, 'show data-directory' at the CLI will print an absolute path. However, the value is staged_gdb_datadir will still be the relative path, and gdb.parameter('data-directory') from Python will return the relative path. In this commit I fix both of these issues by: 1. Initialising the value in staged_gdb_datadir based on the initial value in gdb_datadir, and 2. In set_gdb_datadir, after calling set_gdb_data_directory, I copy the value in gdb_datadir back into staged_gdb_datadir. With these two changes in place the value in staged_gdb_datadir should always match the value in gdb_datadir, and accessing data-directory from Python should now work correctly. gdb/ChangeLog: * top.c (staged_gdb_datadir): Update comment. (set_gdb_datadir): Copy the value of gdb_datadir back into staged_datadir. (init_main): Initialise staged_gdb_datadir. gdb/testsuite/ChangeLog: * gdb.python/py-parameter.exp: Add test for reading data-directory using gdb.parameter API. --- gdb/ChangeLog | 7 ++++ gdb/testsuite/ChangeLog | 5 +++ gdb/testsuite/gdb.python/py-parameter.exp | 51 +++++++++++++++++++++++ gdb/top.c | 17 +++++++- 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index bcd3786598f..39a2d327183 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2021-04-07 Andrew Burgess + + * top.c (staged_gdb_datadir): Update comment. + (set_gdb_datadir): Copy the value of gdb_datadir back into + staged_datadir. + (init_main): Initialise staged_gdb_datadir. + 2021-04-06 Tom de Vries PR breakpoints/25884 diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index c6063be3832..04ed4ad96e3 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2021-04-07 Andrew Burgess + + * gdb.python/py-parameter.exp: Add test for reading data-directory + using gdb.parameter API. + 2021-04-06 Tom de Vries PR breakpoints/25884 diff --git a/gdb/testsuite/gdb.python/py-parameter.exp b/gdb/testsuite/gdb.python/py-parameter.exp index ef24dbe5d16..5543b21f3ac 100644 --- a/gdb/testsuite/gdb.python/py-parameter.exp +++ b/gdb/testsuite/gdb.python/py-parameter.exp @@ -37,6 +37,57 @@ if { [is_remote host] } { } gdb_test "python print (gdb.parameter ('directories'))" $directories +# Check we can correctly read the data-directory parameter. First, +# grab the value as read directly from the GDB CLI. +set dd "" +gdb_test_multiple "show data-directory" \ + "find the initial data-directory value" { + -re -wrap "GDB's data directory is \"(\[^\r\n\]+)\"\\." { + set dd $expect_out(1,string) + pass $gdb_test_name + } + } + +# Now print the data-directory from Python. +gdb_test "python print (gdb.parameter ('data-directory'))" $dd + +# Next change the data-directory to a relative path. Internally GDB +# will resolve this to an absolute path, which Python should then see. +# +# GDB is currently running in '...../build/gdb/testsuite/' and the +# test output is being written to: +# ...../build/gdb/testsuite/outputs/gdb.python/py-parameter/ +# +# So create the relative path './outputs/gdb.python/py-parameter/' and +# set the data-directory to that, we should then see the absolute path. + +set abs_path_to_output_dir [standard_output_file ""] +set abs_path_to_cwd $objdir +set rel_path_to_output_dir \ + [file join "." [string replace ${abs_path_to_output_dir} 0 \ + [string length ${abs_path_to_cwd}] ""]] +gdb_test_no_output "set data-directory ${rel_path_to_output_dir}" + +gdb_test "python print (gdb.parameter ('data-directory'))" \ + ${abs_path_to_output_dir} \ + "python sees absolute version of data-directory path" + +# While we're here, check we see the correct path at GDB's CLI. +gdb_test "show data-directory" \ + "GDB's data directory is \"${abs_path_to_output_dir}\"\\." \ + "check modified data-directory at the CLI" + +# Now lets set the data-directory back to what it was initially. +gdb_test_no_output "set data-directory ${dd}" + +# And check we see the restored value at CLI and from Python. +gdb_test "show data-directory" \ + "GDB's data directory is \"${dd}\"\\." \ + "check original data-directory was restored at the CLI" + +gdb_test "python print (gdb.parameter ('data-directory'))" ${dd} \ + "python sees restored data-directory value" + # Test a simple boolean parameter. with_test_prefix "boolean parameter" { gdb_test_multiline "Simple gdb booleanparameter" \ diff --git a/gdb/top.c b/gdb/top.c index 6dd2fe8a3b0..c374a8d02d3 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -2111,7 +2111,10 @@ show_exec_done_display_p (struct ui_file *file, int from_tty, value); } -/* New values of the "data-directory" parameter are staged here. */ +/* New values of the "data-directory" parameter are staged here. + Extension languages, for example Python's gdb.parameter API, will read + the value directory from this variable, so we must ensure that this + always contains the correct value. */ static char *staged_gdb_datadir; /* "set" command for the gdb_datadir configuration variable. */ @@ -2120,6 +2123,14 @@ static void set_gdb_datadir (const char *args, int from_tty, struct cmd_list_element *c) { set_gdb_data_directory (staged_gdb_datadir); + + /* SET_GDB_DATA_DIRECTORY will resolve relative paths in + STAGED_GDB_DATADIR, so we now copy the value from GDB_DATADIR + back into STAGED_GDB_DATADIR so the extension languages can read the + correct value. */ + free (staged_gdb_datadir); + staged_gdb_datadir = strdup (gdb_datadir.c_str ()); + gdb::observers::gdb_datadir_changed.notify (); } @@ -2290,7 +2301,9 @@ Use \"on\" to enable the notification, and \"off\" to disable it."), When set, GDB uses the specified path to search for data files."), set_gdb_datadir, show_gdb_datadir, &setlist, - &showlist); + &showlist); + /* Prime the initial value for data-directory. */ + staged_gdb_datadir = strdup (gdb_datadir.c_str ()); add_setshow_auto_boolean_cmd ("interactive-mode", class_support, &interactive_mode, _("\ -- 2.30.2