Re: [PATCH] Add an optional offset option to the "symbol-file" command

[Petr Tesarik <ptesarik at suse dot cz>]

Hi all,

any comment on my patch? If it's not good, can you elaborate on what
needs improvement, please?

Petr T

On Fri, 27 Apr 2018 11:24:49 +0200
Petr Tesarik <ptesarik@suse.cz> wrote:

> If the main file is relocated at runtime, all symbols are offset by
> a fixed amount.  Let the user specify this offset when loading a
> symbol file.  The Linux kernel with kASLR is one such example.
> 
> gdb/ChangeLog:
> 2018-04-27  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* symfile.c (symbol_file_command, symbol_file_add_main_1)
> 	(_initialize_symfile): Add option "-o" to symbol-file to add an
> 	offset to each section of the symbol file.
> 
> gdb/doc/ChangeLog:
> 2018-04-27  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* gdb.texinfo (Files): Document "symbol-file -o offset".
> 
> gdb/testsuite/ChangeLog:
> 2018-04-27  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* gdb.base/relocate.exp: Add test for "symbol-file -o ".
> ---
>  gdb/ChangeLog                       |  6 ++++++
>  gdb/doc/ChangeLog                   |  4 ++++
>  gdb/doc/gdb.texinfo                 |  7 ++++++-
>  gdb/symfile.c                       | 28 +++++++++++++++++++++-------
>  gdb/testsuite/ChangeLog             |  4 ++++
>  gdb/testsuite/gdb.base/relocate.exp | 33 +++++++++++++++++++++++++++++++++
>  6 files changed, 74 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index cd86be7fb3..56423e7044 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-04-27  Petr Tesarik  <ptesarik@suse.com>
> +
> +	* symfile.c (symbol_file_command, symbol_file_add_main_1)
> +	(_initialize_symfile): Add option "-o" to symbol-file to add an
> +	offset to each section of the symbol file.
> +
>  2018-04-26  Andrzej Kaczmarek  <andrzej.kaczmarek@codecoup.pl>
>  
>  	PR remote/9665
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 83d48781f9..7a67b80cda 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,7 @@
> +2018-04-27  Petr Tesarik  <ptesarik@suse.com>
> +
> +	* gdb.texinfo (Files): Document "symbol-file -o offset".
> +
>  2018-04-13  Andreas Arnez  <arnez@linux.vnet.ibm.com>
>  
>  	* gdb.texinfo (Symbols): Mention the fact that "info
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 28f083f96e..56a36d8225 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -18819,11 +18819,16 @@ if necessary to locate your program.  Omitting @var{filename} means to
>  discard information on the executable file.
>  
>  @kindex symbol-file
> -@item symbol-file @r{[} @var{filename} @r{]}
> +@item symbol-file @r{[} -o @var{offset} @r{]} @r{[} @var{filename} @r{]}
>  Read symbol table information from file @var{filename}.  @code{PATH} is
>  searched when necessary.  Use the @code{file} command to get both symbol
>  table and program to run from the same file.
>  
> +If an optional @var{offset} is specified, it is added to the start
> +address of each section in the symbol file.  This is useful if the
> +program is relocated at runtime, such as the Linux kernel with kASLR
> +enabled.
> +
>  @code{symbol-file} with no argument clears out @value{GDBN} information on your
>  program's symbol table.
>  
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 1e5297ee29..41788f0a67 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -87,7 +87,7 @@ int readnever_symbol_files;	/* Never read full symbols.  */
>  /* Functions this file defines.  */
>  
>  static void symbol_file_add_main_1 (const char *args, symfile_add_flags add_flags,
> -				    objfile_flags flags);
> +				    objfile_flags flags, CORE_ADDR offset);
>  
>  static const struct sym_fns *find_sym_fns (bfd *);
>  
> @@ -1222,16 +1222,20 @@ symbol_file_add (const char *name, symfile_add_flags add_flags,
>  void
>  symbol_file_add_main (const char *args, symfile_add_flags add_flags)
>  {
> -  symbol_file_add_main_1 (args, add_flags, 0);
> +  symbol_file_add_main_1 (args, add_flags, 0, 0);
>  }
>  
>  static void
>  symbol_file_add_main_1 (const char *args, symfile_add_flags add_flags,
> -			objfile_flags flags)
> +			objfile_flags flags, CORE_ADDR offset)
>  {
> +  struct objfile *objfile;
> +
>    add_flags |= current_inferior ()->symfile_flags | SYMFILE_MAINLINE;
>  
> -  symbol_file_add (args, add_flags, NULL, flags);
> +  objfile = symbol_file_add (args, add_flags, NULL, flags);
> +  if (offset != 0)
> +    objfile_rebase (objfile, offset);
>  
>    /* Getting new symbols may change our opinion about
>       what is frameless.  */
> @@ -1548,6 +1552,8 @@ symbol_file_command (const char *args, int from_tty)
>        symfile_add_flags add_flags = 0;
>        char *name = NULL;
>        bool stop_processing_options = false;
> +      bool expecting_offset = false;
> +      CORE_ADDR offset = 0;
>        int idx;
>        char *arg;
>  
> @@ -1559,7 +1565,12 @@ symbol_file_command (const char *args, int from_tty)
>  	{
>  	  if (stop_processing_options || *arg != '-')
>  	    {
> -	      if (name == NULL)
> +	      if (expecting_offset)
> +		{
> +		  offset = parse_and_eval_address (arg);
> +		  expecting_offset = false;
> +		}
> +	      else if (name == NULL)
>  		name = arg;
>  	      else
>  		error (_("Unrecognized argument \"%s\""), arg);
> @@ -1568,6 +1579,8 @@ symbol_file_command (const char *args, int from_tty)
>  	    flags |= OBJF_READNOW;
>  	  else if (strcmp (arg, "-readnever") == 0)
>  	    flags |= OBJF_READNEVER;
> +	  else if (strcmp (arg, "-o") == 0)
> +	    expecting_offset = true;
>  	  else if (strcmp (arg, "--") == 0)
>  	    stop_processing_options = true;
>  	  else
> @@ -1579,7 +1592,7 @@ symbol_file_command (const char *args, int from_tty)
>  
>        validate_readnow_readnever (flags);
>  
> -      symbol_file_add_main_1 (name, add_flags, flags);
> +      symbol_file_add_main_1 (name, add_flags, flags, offset);
>      }
>  }
>  
> @@ -3772,7 +3785,8 @@ symbolic debug information."
>  
>    c = add_cmd ("symbol-file", class_files, symbol_file_command, _("\
>  Load symbol table from executable file FILE.\n\
> -Usage: symbol-file [-readnow | -readnever] FILE\n\
> +Usage: symbol-file [-readnow | -readnever] [-o <OFF>] FILE\n\
> +OFF is an optional offset which is added to each section address.\n\
>  The `file' command can also load symbol tables, as well as setting the file\n\
>  to execute.\n" READNOW_READNEVER_HELP), &cmdlist);
>    set_cmd_completer (c, filename_completer);
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 34da102c62..68431cb035 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2018-04-27  Petr Tesarik  <ptesarik@suse.com>
> +
> +	* gdb.base/relocate.exp: Add test for "symbol-file -o ".
> +
>  2018-04-26  Pedro Alves  <palves@redhat.com>
>  
>  	* gdb.base/gnu-ifunc.exp (set-break): Test that GDB resolves
> diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
> index 89f2fffcd9..4383e79cb2 100644
> --- a/gdb/testsuite/gdb.base/relocate.exp
> +++ b/gdb/testsuite/gdb.base/relocate.exp
> @@ -196,6 +196,39 @@ if { "${function_foo_addr}" == "${new_function_foo_addr}" } {
>    pass "function foo has a different address"
>  }
>  
> +# Load the object using symbol-file with an offset and check that
> +# all addresses are moved by that offset.
> +
> +set offset 0x10000
> +clean_restart
> +gdb_test "symbol-file -o $offset $binfile" \
> +    "Reading symbols from ${binfile}\.\.\.done\." \
> +    "symbol-file with offset"
> +
> +# Make sure the address of a static variable is moved by offset.
> +set new_static_foo_addr [get_var_address static_foo]
> +if { "${new_static_foo_addr}" == "${static_foo_addr}" + $offset } {
> +  pass "static variable foo is moved by offset"
> +} else {
> +  fail "static variable foo is moved by offset"
> +}
> +
> +# Make sure the address of a global variable is moved by offset.
> +set new_global_foo_addr [get_var_address global_foo]
> +if { "${new_global_foo_addr}" == "${global_foo_addr}" + $offset } {
> +  pass "global variable foo is moved by offset"
> +} else {
> +  fail "global variable foo is moved by offset"
> +}
> +
> +# Make sure the address of a functaion is moved by offset.
> +set new_function_foo_addr [get_var_address function_foo]
> +if { "${new_function_foo_addr}" == "${function_foo_addr}" + $offset } {
> +  pass "function foo is moved by offset"
> +} else {
> +  fail "function foo is moved by offset"
> +}
> +
>  # Now try loading the object as an exec-file; we should be able to print
>  # the values of variables after we do this.
>