public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command
  2018-06-11 12:08 [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
  2018-06-11 12:08 ` [PATCH v2 2/4] Make add-symbol-file's address argument optional Petr Tesarik
@ 2018-06-11 12:08 ` Petr Tesarik
  2018-06-11 15:23   ` Eli Zaretskii
  2018-06-26  2:02   ` Simon Marchi
  2018-06-11 12:08 ` [PATCH v2 3/4] Make sure that sorting does not change section order Petr Tesarik
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-11 12:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Petr Tesarik, Jeff Mahoney

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.

gdb/ChangeLog:
2018-06-11  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-06-08  Petr Tesarik  <ptesarik@suse.com>

	* gdb.texinfo (Files): Document "symbol-file -o offset".

gdb/testsuite/ChangeLog:
2018-06-11  Petr Tesarik  <ptesarik@suse.com>

	* gdb.base/relocate.exp: Add test for "symbol-file -o ".
---
 gdb/ChangeLog                       |  6 ++++++
 gdb/NEWS                            |  3 +++
 gdb/doc/ChangeLog                   |  4 ++++
 gdb/doc/gdb.texinfo                 |  7 ++++++-
 gdb/symfile.c                       | 24 ++++++++++++++++++------
 gdb/testsuite/ChangeLog             |  4 ++++
 gdb/testsuite/gdb.base/relocate.exp | 24 ++++++++++++++++++++++++
 7 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 635f02523b..1b789d00f4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-06-11  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-06-11  Alan Hayward  <alan.hayward@arm.com>
 
 	* aarch64-tdep.c (aarch64_dwarf_reg_to_regnum): Add mappings.
diff --git a/gdb/NEWS b/gdb/NEWS
index 13da2f1d4e..101746567a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 8.1
 
+* The 'symbol-file' command now accepts an '-o' option to add a relative
+  offset to all sections.
+
 * The endianness used with the 'set endian auto' mode in the absence of
   an executable selected for debugging is now the last endianness chosen
   either by one of the 'set endian big' and 'set endian little' commands
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index d36affe4b6..b31c1ac324 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2018-06-11  Petr Tesarik  <ptesarik@suse.com>
+
+	* gdb.texinfo (Files): Document "symbol-file -o offset".
+
 2018-06-08  Gary Benson <gbenson@redhat.com>
 
 	* gdb.texinfo (Maintenance Commands): Document "maint check
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2c0ac33f8b..973365574f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18822,11 +18822,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 f8177ea8b1..461f60d074 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 reloff);
 
 static const struct sym_fns *find_sym_fns (bfd *);
 
@@ -1225,16 +1225,18 @@ 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 reloff)
 {
   add_flags |= current_inferior ()->symfile_flags | SYMFILE_MAINLINE;
 
-  symbol_file_add (args, add_flags, NULL, flags);
+  struct objfile *objfile = symbol_file_add (args, add_flags, NULL, flags);
+  if (reloff != 0)
+    objfile_rebase (objfile, reloff);
 
   /* Getting new symbols may change our opinion about
      what is frameless.  */
@@ -1551,6 +1553,7 @@ symbol_file_command (const char *args, int from_tty)
       symfile_add_flags add_flags = 0;
       char *name = NULL;
       bool stop_processing_options = false;
+      CORE_ADDR offset = 0;
       int idx;
       char *arg;
 
@@ -1571,6 +1574,14 @@ 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)
+	    {
+	      arg = built_argv[++idx];
+	      if (arg == NULL)
+		error (_("Missing argument to -o"));
+
+	      offset = parse_and_eval_address (arg);
+	    }
 	  else if (strcmp (arg, "--") == 0)
 	    stop_processing_options = true;
 	  else
@@ -1582,7 +1593,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);
     }
 }
 
@@ -3774,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 2324673e56..b29a2bfab3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2018-06-11  Petr Tesarik  <ptesarik@suse.com>
+
+	* gdb.base/relocate.exp: Add test for "symbol-file -o ".
+
 2018-06-08  Gary Benson <gbenson@redhat.com>
 
 	* gdb.threads/check-libthread-db.exp: New file.
diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
index 89f2fffcd9..77f6a88159 100644
--- a/gdb/testsuite/gdb.base/relocate.exp
+++ b/gdb/testsuite/gdb.base/relocate.exp
@@ -196,6 +196,30 @@ 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]
+gdb_assert {${new_static_foo_addr} == ${static_foo_addr} + $offset} \
+    "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]
+gdb_assert {${new_global_foo_addr} == ${global_foo_addr} + $offset} \
+    "global variable foo is moved by offset"
+
+# Make sure the address of a function is moved by offset.
+set new_function_foo_addr [get_var_address function_foo]
+gdb_assert {${new_function_foo_addr} == ${function_foo_addr} + $offset} \
+    "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.
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 3/4] Make sure that sorting does not change section order
  2018-06-11 12:08 [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
  2018-06-11 12:08 ` [PATCH v2 2/4] Make add-symbol-file's address argument optional Petr Tesarik
  2018-06-11 12:08 ` [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
@ 2018-06-11 12:08 ` Petr Tesarik
  2018-06-26  2:37   ` Simon Marchi
  2018-06-11 12:23 ` [PATCH v2 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Petr Tesarik @ 2018-06-11 12:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Petr Tesarik, Jeff Mahoney

Symbol files may contain multiple sections with the same name.
Section addresses specified add-symbol-file are assigned to the
corresponding BFD sections in addr_info_make_relative using sorted
indexes of both vectors.  Since the sort algorithm is not inherently
stable, the comparison function uses sectindex to maintain the
original order.  However, add_symbol_file_command uses zero for all
sections, so if the user specifies multiple sections with the same
name, they will be assigned randomly to symbol file sections with
the same name.

gdb/ChangeLog:
2018-06-11  Petr Tesarik  <ptesarik@suse.com>

	* symfile.c (add_symbol_file_command): Make sure that sections
	with the same name are sorted in the same order.
---
 gdb/ChangeLog | 5 +++++
 gdb/symfile.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1c5a1f6bfb..0f75992d4c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
 
+	* symfile.c (add_symbol_file_command): Make sure that sections
+	with the same name are sorted in the same order.
+
+2018-06-11  Petr Tesarik  <ptesarik@suse.com>
+
 	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
 	require the second argument.  If omitted, load sections at the
 	addresses specified in the file.
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 3e3ab20412..8b8b194334 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2185,7 +2185,7 @@ add_symbol_file_command (const char *args, int from_tty)
 
       /* Here we store the section offsets in the order they were
          entered on the command line.  */
-      section_addrs.emplace_back (addr, sec, 0);
+      section_addrs.emplace_back (addr, sec, section_addrs.size ());
       printf_unfiltered ("\t%s_addr = %s\n", sec,
 			 paddress (gdbarch, addr));
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 0/4] Allow loading symbol files with an offset
@ 2018-06-11 12:08 Petr Tesarik
  2018-06-11 12:08 ` [PATCH v2 2/4] Make add-symbol-file's address argument optional Petr Tesarik
                   ` (5 more replies)
  0 siblings, 6 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-11 12:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Petr Tesarik, Jeff Mahoney

Implement an "-o" option for symbol-file and add-symbol-file.

Changes since v1:
* Adjust help texts
* Update gdb/NEWS

Petr Tesarik (4):
  Add an optional offset option to the "symbol-file" command
  Make add-symbol-file's address argument optional
  Make sure that sorting does not change section order
  Add an optional offset option to the "add-symbol-file" command

 gdb/ChangeLog                       |  23 ++++++++
 gdb/NEWS                            |  10 ++++
 gdb/doc/ChangeLog                   |  13 +++++
 gdb/doc/gdb.texinfo                 |  24 ++++++--
 gdb/symfile.c                       | 108 +++++++++++++++++++++++++++++-------
 gdb/testsuite/ChangeLog             |  13 +++++
 gdb/testsuite/gdb.base/relocate.exp |  95 +++++++++++++++++++++++++++++++
 7 files changed, 259 insertions(+), 27 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 2/4] Make add-symbol-file's address argument optional
  2018-06-11 12:08 [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
@ 2018-06-11 12:08 ` Petr Tesarik
  2018-06-11 15:25   ` Eli Zaretskii
  2018-06-26  2:14   ` Simon Marchi
  2018-06-11 12:08 ` [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-11 12:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Petr Tesarik, Jeff Mahoney

The (first) .text section must be always specified as the second
non-option argument.  The documentation states that GDB cannot
figure out this address by itself.  This is true if the object file
was indeed relocated, but it is also confusing, because all other
sections can be omitted and will use the address provided by BFD.

gdb/ChangeLog:
2018-06-11  Petr Tesarik  <ptesarik@suse.com>

	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
	require the second argument.  If omitted, load sections at the
	addresses specified in the file.

gdb/doc/ChangeLog:
2018-06-11  Petr Tesarik  <ptesarik@suse.com>

	* gdb.texinfo (Files): The address argument for "add-symbol-file"
	is no longer mandatory.

gdb/testsuite/ChangeLog:
2018-06-11  Petr Tesarik  <ptesarik@suse.com>

	* gdb.base/relocate.exp: Test add-symbol-file behavior when the
	address argument is omitted.
---
 gdb/ChangeLog                       |  6 ++++++
 gdb/NEWS                            |  3 +++
 gdb/doc/ChangeLog                   |  5 +++++
 gdb/doc/gdb.texinfo                 | 13 ++++++++-----
 gdb/symfile.c                       | 25 ++++++++++++-------------
 gdb/testsuite/ChangeLog             |  5 +++++
 gdb/testsuite/gdb.base/relocate.exp | 15 +++++++++++++++
 7 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1b789d00f4..1c5a1f6bfb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
 
+	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
+	require the second argument.  If omitted, load sections at the
+	addresses specified in the file.
+
+2018-06-11  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.
diff --git a/gdb/NEWS b/gdb/NEWS
index 101746567a..54c0f1d19b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -6,6 +6,9 @@
 * The 'symbol-file' command now accepts an '-o' option to add a relative
   offset to all sections.
 
+* The 'add-symbol-file' command no longer requires the second argument
+  (address of the text section).
+
 * The endianness used with the 'set endian auto' mode in the absence of
   an executable selected for debugging is now the last endianness chosen
   either by one of the 'set endian big' and 'set endian little' commands
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index b31c1ac324..b8f48733f9 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,10 @@
 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
 
+	* gdb.texinfo (Files): The address argument for "add-symbol-file"
+	is no longer mandatory.
+
+2018-06-11  Petr Tesarik  <ptesarik@suse.com>
+
 	* gdb.texinfo (Files): Document "symbol-file -o offset".
 
 2018-06-08  Gary Benson <gbenson@redhat.com>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 973365574f..84600bfe5f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18917,18 +18917,21 @@ the program is running.  To do this, use the @code{kill} command
 
 @kindex add-symbol-file
 @cindex dynamic linking
-@item add-symbol-file @var{filename} @var{address}
-@itemx add-symbol-file @var{filename} @var{address} @r{[} -readnow @r{|} -readnever @r{]}
-@itemx add-symbol-file @var{filename} @var{address} -s @var{section} @var{address} @dots{}
+@item add-symbol-file @var{filename} @r{[} @var{address} @r{]}
+@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} @r{[} -readnow @r{|} -readnever @r{]}
+@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} -s @var{section} @var{address} @dots{}
 The @code{add-symbol-file} command reads additional symbol table
 information from the file @var{filename}.  You would use this command
 when @var{filename} has been dynamically loaded (by some other means)
 into the program that is running.  The @var{address} should give the memory
-address at which the file has been loaded; @value{GDBN} cannot figure
-this out for itself.  You can additionally specify an arbitrary number
+address at which the file has been loaded.
+You can additionally specify an arbitrary number
 of @samp{-s @var{section} @var{address}} pairs, to give an explicit
 section name and base address for that section.  You can specify any
 @var{address} as an expression.
+If @var{address} is omitted, @value{GDBN} will use the section
+addresses found in @var{filename}.  You can use @samp{-s} to
+override this default and load a section at a different address.
 
 The symbol table of the file @var{filename} is added to the symbol table
 originally read with the @code{symbol-file} command.  You can use the
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 461f60d074..3e3ab20412 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2161,29 +2161,26 @@ add_symbol_file_command (const char *args, int from_tty)
 
   validate_readnow_readnever (flags);
 
-  /* This command takes at least two arguments.  The first one is a
-     filename, and the second is the address where this file has been
-     loaded.  Abort now if this address hasn't been provided by the
-     user.  */
-  if (!seen_addr)
-    error (_("The address where %s has been loaded is missing"),
-	   filename.get ());
-
   /* Print the prompt for the query below.  And save the arguments into
      a sect_addr_info structure to be passed around to other
      functions.  We have to split this up into separate print
      statements because hex_string returns a local static
      string.  */
 
-  printf_unfiltered (_("add symbol table from file \"%s\" at\n"),
+  printf_unfiltered (_("add symbol table from file \"%s\""),
 		     filename.get ());
   section_addr_info section_addrs;
-  for (sect_opt &sect : sect_opts)
+  std::vector<sect_opt>::const_iterator it = sect_opts.begin ();
+  if (!seen_addr)
+    ++it;
+  for (; it != sect_opts.end (); ++it)
     {
       CORE_ADDR addr;
-      const char *val = sect.value;
-      const char *sec = sect.name;
+      const char *val = it->value;
+      const char *sec = it->name;
 
+      if (section_addrs.size () == 0)
+	printf_unfiltered (_(" at\n"));
       addr = parse_and_eval_address (val);
 
       /* Here we store the section offsets in the order they were
@@ -2198,6 +2195,8 @@ add_symbol_file_command (const char *args, int from_tty)
 	 At this point, we don't know what file type this is,
 	 so we can't determine what section names are valid.  */
     }
+  if (section_addrs.size () == 0)
+    printf_unfiltered ("\n");
 
   if (from_tty && (!query ("%s", "")))
     error (_("Not confirmed."));
@@ -3793,7 +3792,7 @@ to execute.\n" READNOW_READNEVER_HELP), &cmdlist);
 
   c = add_cmd ("add-symbol-file", class_files, add_symbol_file_command, _("\
 Load symbols from FILE, assuming FILE has been dynamically loaded.\n\
-Usage: add-symbol-file FILE ADDR [-readnow | -readnever | \
+Usage: add-symbol-file FILE [ADDR] [-readnow | -readnever | \
 -s SECT-NAME SECT-ADDR]...\n\
 ADDR is the starting address of the file's text.\n\
 Each '-s' argument provides a section name and address, and\n\
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b29a2bfab3..f2e7aa98a8 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
 
+	* gdb.base/relocate.exp: Test add-symbol-file behavior when the
+	address argument is omitted.
+
+2018-06-11  Petr Tesarik  <ptesarik@suse.com>
+
 	* gdb.base/relocate.exp: Add test for "symbol-file -o ".
 
 2018-06-08  Gary Benson <gbenson@redhat.com>
diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
index 77f6a88159..a3af8cea61 100644
--- a/gdb/testsuite/gdb.base/relocate.exp
+++ b/gdb/testsuite/gdb.base/relocate.exp
@@ -73,6 +73,21 @@ gdb_test_multiple "add-symbol-file -s .text 0x200 $binfile 0x100" $test {
 	gdb_test "n" "Not confirmed\." $test
     }
 }
+# Check that passing a single "-s .text" is equivallent to passing
+# the text address in a positional argument.
+set test "add-symbol-file -s .text, no address"
+gdb_test_multiple "add-symbol-file $binfile -s .text 0x100" $test {
+    -re "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x100\r\n\\(y or n\\) " {
+	gdb_test "n" "Not confirmed\." $test
+    }
+}
+# Check section addresses can be omitted.
+set test "add-symbol-file no address"
+gdb_test_multiple "add-symbol-file $binfile" $test {
+    -re "add symbol table from file \"${binfile}\"\r\n\\(y or n\\) " {
+	gdb_test "n" "Not confirmed\." $test
+    }
+}
 # Test that passing "--" disables option processing.
 gdb_test "add-symbol-file -- $binfile 0x100 -s .bss 0x3" \
     "Unrecognized argument \"-s\"" \
-- 
2.13.6

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 4/4] Add an optional offset option to the "add-symbol-file" command
  2018-06-11 12:08 [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
                   ` (2 preceding siblings ...)
  2018-06-11 12:08 ` [PATCH v2 3/4] Make sure that sorting does not change section order Petr Tesarik
@ 2018-06-11 12:23 ` Petr Tesarik
  2018-06-26  2:52   ` Simon Marchi
  2018-06-22  7:52 ` [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
  2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
  5 siblings, 1 reply; 26+ messages in thread
From: Petr Tesarik @ 2018-06-11 12:23 UTC (permalink / raw)
  To: gdb-patches; +Cc: Petr Tesarik, Jeff Mahoney

If all sections of a symbol file are loaded with a fixed offset, it
is easier to specify that offset than listing all sections
explicitly.  There is also a similar option for "symbol-file".

gdb/ChangeLog:
2018-06-11  Petr Tesarik  <ptesarik@suse.com>

	* symfile.c (add_symbol_file_command, _initialize_symfile): Add
	option "-o" to add-symbol-file-load to add an offset to each
	section's load address.

gdb/doc/ChangeLog:
2018-06-11  Petr Tesarik  <ptesarik@suse.com>

	* gdb.texinfo (Files): Document "add-symbol-file -o offset".

gdb/testsuite/ChangeLog:
2018-06-11  Petr Tesarik  <ptesarik@suse.com>

	* gdb.base/relocate.exp: Add test for "add-symbol-file -o ".
---
 gdb/ChangeLog                       |  6 ++++
 gdb/NEWS                            |  4 +++
 gdb/doc/ChangeLog                   |  4 +++
 gdb/doc/gdb.texinfo                 | 10 ++++--
 gdb/symfile.c                       | 61 +++++++++++++++++++++++++++++++++++--
 gdb/testsuite/ChangeLog             |  4 +++
 gdb/testsuite/gdb.base/relocate.exp | 56 ++++++++++++++++++++++++++++++++++
 7 files changed, 139 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0f75992d4c..337165e39e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
 
+	* symfile.c (add_symbol_file_command, _initialize_symfile): Add
+	option "-o" to add-symbol-file-load to add an offset to each
+	section's load address.
+
+2018-06-11  Petr Tesarik  <ptesarik@suse.com>
+
 	* symfile.c (add_symbol_file_command): Make sure that sections
 	with the same name are sorted in the same order.
 
diff --git a/gdb/NEWS b/gdb/NEWS
index 54c0f1d19b..016796a802 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -6,6 +6,10 @@
 * The 'symbol-file' command now accepts an '-o' option to add a relative
   offset to all sections.
 
+* Similarly, the 'add-symbol-file' command also accepts an '-o' option to add
+  a relative offset to all sections, but it allows to override the load
+  address of individual sections using '-s'.
+
 * The 'add-symbol-file' command no longer requires the second argument
   (address of the text section).
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index b8f48733f9..09ae56db1c 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,9 @@
 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
 
+	* gdb.texinfo (Files): Document "add-symbol-file -o offset".
+
+2018-06-11  Petr Tesarik  <ptesarik@suse.com>
+
 	* gdb.texinfo (Files): The address argument for "add-symbol-file"
 	is no longer mandatory.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 84600bfe5f..fb1b9ece62 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18917,9 +18917,9 @@ the program is running.  To do this, use the @code{kill} command
 
 @kindex add-symbol-file
 @cindex dynamic linking
-@item add-symbol-file @var{filename} @r{[} @var{address} @r{]}
-@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} @r{[} -readnow @r{|} -readnever @r{]}
-@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} -s @var{section} @var{address} @dots{}
+@item add-symbol-file @var{filename} @r{[} @var{address} @r{]} @r{[} -o @var{offset} @r{]}
+@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} @r{[} -o @var{offset} @r{]} @r{]} @r{[} -readnow @r{|} -readnever @r{]}
+@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} @r{[} -o @var{offset} @r{]} @r{]} -s @var{section} @var{address} @dots{}
 The @code{add-symbol-file} command reads additional symbol table
 information from the file @var{filename}.  You would use this command
 when @var{filename} has been dynamically loaded (by some other means)
@@ -18933,6 +18933,10 @@ If @var{address} is omitted, @value{GDBN} will use the section
 addresses found in @var{filename}.  You can use @samp{-s} to
 override this default and load a section at a different address.
 
+If an optional @var{offset} is specified, it is added to the start
+address of each section, except those for which the address was
+specified explicitly.
+
 The symbol table of the file @var{filename} is added to the symbol table
 originally read with the @code{symbol-file} command.  You can use the
 @code{add-symbol-file} command any number of times; the new symbol data
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 8b8b194334..a8904f1f4a 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2106,6 +2106,7 @@ add_symbol_file_command (const char *args, int from_tty)
 
   std::vector<sect_opt> sect_opts = { { ".text", NULL } };
   bool stop_processing_options = false;
+  CORE_ADDR offset = 0;
 
   dont_repeat ();
 
@@ -2113,6 +2114,7 @@ add_symbol_file_command (const char *args, int from_tty)
     error (_("add-symbol-file takes a file name and an address"));
 
   bool seen_addr = false;
+  bool seen_offset = false;
   gdb_argv argv (args);
 
   for (arg = argv[0], argcnt = 0; arg != NULL; arg = argv[++argcnt])
@@ -2150,6 +2152,15 @@ add_symbol_file_command (const char *args, int from_tty)
 	  sect_opts.push_back (sect);
 	  argcnt += 2;
 	}
+      else if (strcmp (arg, "-o") == 0)
+	{
+	  arg = argv[++argcnt];
+	  if (arg == NULL)
+	    error (_("Missing argument to -o"));
+
+	  offset = parse_and_eval_address (arg);
+	  seen_offset = true;
+	}
       else if (strcmp (arg, "--") == 0)
 	stop_processing_options = true;
       else
@@ -2195,7 +2206,13 @@ add_symbol_file_command (const char *args, int from_tty)
 	 At this point, we don't know what file type this is,
 	 so we can't determine what section names are valid.  */
     }
-  if (section_addrs.size () == 0)
+  if (seen_offset)
+      printf_unfiltered (_("%s offset by %s\n"),
+			 (section_addrs.size () == 0
+			  ? _(" with all sections")
+			  : _("with other sections")),
+			 paddress (gdbarch, offset));
+  else if (section_addrs.size () == 0)
     printf_unfiltered ("\n");
 
   if (from_tty && (!query ("%s", "")))
@@ -2204,6 +2221,42 @@ add_symbol_file_command (const char *args, int from_tty)
   objf = symbol_file_add (filename.get (), add_flags, &section_addrs,
 			  flags);
 
+  if (seen_offset)
+    {
+      std::vector<struct section_offsets> new_offsets (objf->num_sections,
+						       { offset });
+
+      std::vector<const struct other_sections *> sect_addrs_sorted
+	= addrs_section_sort (section_addrs);
+
+      section_addr_info objf_addrs
+	= build_section_addr_info_from_objfile (objf);
+      std::vector<const struct other_sections *> objf_addrs_sorted
+	= addrs_section_sort (objf_addrs);
+
+      std::vector<const struct other_sections *>::iterator sect_sorted_iter
+	= sect_addrs_sorted.begin ();
+      for (const struct other_sections *objf_sect : objf_addrs_sorted)
+	{
+	  const char *objf_name = addr_section_name (objf_sect->name.c_str ());
+	  int cmp = -1;
+
+	  while (cmp < 0 && sect_sorted_iter != sect_addrs_sorted.end ())
+	    {
+	      const struct other_sections *sect = *sect_sorted_iter;
+	      const char *sect_name = addr_section_name (sect->name.c_str ());
+	      cmp = strcmp (sect_name, objf_name);
+	      if (cmp <= 0)
+		++sect_sorted_iter;
+	    }
+
+	  if (cmp == 0)
+	    new_offsets[objf_sect->sectindex].offsets[0] = 0;
+	}
+
+      objfile_relocate (objf, new_offsets.data ());
+    }
+
   add_target_sections_of_objfile (objf);
 
   /* Getting new symbols may change our opinion about what is
@@ -3792,12 +3845,14 @@ to execute.\n" READNOW_READNEVER_HELP), &cmdlist);
 
   c = add_cmd ("add-symbol-file", class_files, add_symbol_file_command, _("\
 Load symbols from FILE, assuming FILE has been dynamically loaded.\n\
-Usage: add-symbol-file FILE [ADDR] [-readnow | -readnever | \
+Usage: add-symbol-file FILE [ADDR] [-o OFF] [-readnow | -readnever | \
 -s SECT-NAME SECT-ADDR]...\n\
 ADDR is the starting address of the file's text.\n\
 Each '-s' argument provides a section name and address, and\n\
 should be specified if the data and bss segments are not contiguous\n\
-with the text.  SECT-NAME is a section name to be loaded at SECT-ADDR.\n"
+with the text.  SECT-NAME is a section name to be loaded at SECT-ADDR.\n\
+OFF is an optional offset which is added to the default load addresses\n\
+of all sections for which no other address was specified.\n"
 READNOW_READNEVER_HELP),
 	       &cmdlist);
   set_cmd_completer (c, filename_completer);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f2e7aa98a8..ce34c4b208 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
 
+	* gdb.base/relocate.exp: Add test for "add-symbol-file -o ".
+
+2018-06-11  Petr Tesarik  <ptesarik@suse.com>
+
 	* gdb.base/relocate.exp: Test add-symbol-file behavior when the
 	address argument is omitted.
 
diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
index a3af8cea61..119495dd94 100644
--- a/gdb/testsuite/gdb.base/relocate.exp
+++ b/gdb/testsuite/gdb.base/relocate.exp
@@ -235,6 +235,62 @@ set new_function_foo_addr [get_var_address function_foo]
 gdb_assert {${new_function_foo_addr} == ${function_foo_addr} + $offset} \
     "function foo is moved by offset"
 
+# Load the object using add-symbol-file with an offset and check that
+# all addresses are moved by that offset.
+
+set offset 0x10000
+clean_restart
+gdb_test "add-symbol-file -o $offset $binfile" \
+    "Reading symbols from ${binfile}\.\.\.done\." \
+    "add-symbol-file with offset" \
+    "add symbol table from file \".*${testfile}\\.o\" with all sections offset by $offset\[\r\n\]+\\(y or n\\) " \
+    "y"
+
+# Make sure the address of a static variable is moved by offset.
+set new_static_foo_addr [get_var_address static_foo]
+gdb_assert { ${new_static_foo_addr} == ${static_foo_addr} + $offset } \
+    "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]
+gdb_assert { ${new_global_foo_addr} == ${global_foo_addr} + $offset } \
+    "global variable foo is moved by offset"
+
+# Make sure the address of a function is moved by offset.
+set new_function_foo_addr [get_var_address function_foo]
+gdb_assert { ${new_function_foo_addr} == ${function_foo_addr} + $offset } \
+    "function foo is moved by offset"
+
+# Re-load the object giving an explicit address for .text
+
+set text [ format "0x%x" [expr ${function_foo_addr} + 0x20000] ]
+clean_restart
+gdb_test "add-symbol-file $binfile -o $offset $text" \
+    "Reading symbols from ${binfile}\.\.\.done\." \
+    "add-symbol-file with offset, text address given" \
+    "add symbol table from file \".*${testfile}\\.o\" at\[ \t\r\n\]+\.text_addr = ${text}\[\r\n\]+with other sections offset by ${offset}\[\r\n\]+\\(y or n\\) " \
+    "y"
+
+# Make sure function has a different addresses now.
+set function_foo_addr [get_var_address function_foo]
+gdb_assert { ${function_foo_addr} != ${new_function_foo_addr} } \
+    "function foo has a different address"
+
+# Re-load the object giving an explicit address for .data
+
+set data [ format "0x%x" [expr ${global_foo_addr} + 0x20000] ]
+clean_restart
+gdb_test "add-symbol-file $binfile -o $offset -s .data $data" \
+    "Reading symbols from ${binfile}\.\.\.done\." \
+    "add-symbol-file with offset, data address given" \
+    "add symbol table from file \".*${testfile}\\.o\" at\[ \t\r\n\]+\.data_addr = ${data}\[\r\n\]+with other sections offset by ${offset}\[\r\n\]+\\(y or n\\) " \
+    "y"
+
+# Make sure variable has a different addresses now.
+set global_foo_addr [get_var_address global_foo]
+gdb_assert { ${global_foo_addr} != ${new_global_foo_addr} } \
+    "global variable foo has a different address"
+
 # Now try loading the object as an exec-file; we should be able to print
 # the values of variables after we do this.
 
-- 
2.13.6

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command
  2018-06-11 12:08 ` [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
@ 2018-06-11 15:23   ` Eli Zaretskii
  2018-06-26  2:02   ` Simon Marchi
  1 sibling, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2018-06-11 15:23 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches, jeffm

> From: Petr Tesarik <ptesarik@suse.cz>
> Cc: Petr Tesarik <ptesarik@suse.cz>,	Jeff Mahoney <jeffm@suse.com>
> Date: Mon, 11 Jun 2018 14:08:32 +0200
> 
> 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.
> 
> gdb/ChangeLog:
> 2018-06-11  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-06-08  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* gdb.texinfo (Files): Document "symbol-file -o offset".
> 
> gdb/testsuite/ChangeLog:
> 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* gdb.base/relocate.exp: Add test for "symbol-file -o ".

I already approved the documentation parts, didn't I?

Thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 2/4] Make add-symbol-file's address argument optional
  2018-06-11 12:08 ` [PATCH v2 2/4] Make add-symbol-file's address argument optional Petr Tesarik
@ 2018-06-11 15:25   ` Eli Zaretskii
  2018-06-11 16:50     ` Petr Tesarik
  2018-06-26  2:14   ` Simon Marchi
  1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2018-06-11 15:25 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches, jeffm

> From: Petr Tesarik <ptesarik@suse.cz>
> Cc: Petr Tesarik <ptesarik@suse.cz>,	Jeff Mahoney <jeffm@suse.com>
> Date: Mon, 11 Jun 2018 14:08:33 +0200
> 
> The (first) .text section must be always specified as the second
> non-option argument.  The documentation states that GDB cannot
> figure out this address by itself.  This is true if the object file
> was indeed relocated, but it is also confusing, because all other
> sections can be omitted and will use the address provided by BFD.
> 
> gdb/ChangeLog:
> 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
> 	require the second argument.  If omitted, load sections at the
> 	addresses specified in the file.
> 
> gdb/doc/ChangeLog:
> 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* gdb.texinfo (Files): The address argument for "add-symbol-file"
> 	is no longer mandatory.
> 
> gdb/testsuite/ChangeLog:
> 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* gdb.base/relocate.exp: Test add-symbol-file behavior when the
> 	address argument is omitted.

I have a feeling I already reviewed and approved this, but in case I
didn't, I approve it now.

Thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 2/4] Make add-symbol-file's address argument optional
  2018-06-11 15:25   ` Eli Zaretskii
@ 2018-06-11 16:50     ` Petr Tesarik
  2018-06-11 17:25       ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Petr Tesarik @ 2018-06-11 16:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, jeffm

On Mon, 11 Jun 2018 18:25:02 +0300
Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Petr Tesarik <ptesarik@suse.cz>
> > Cc: Petr Tesarik <ptesarik@suse.cz>,	Jeff Mahoney <jeffm@suse.com>
> > Date: Mon, 11 Jun 2018 14:08:33 +0200
> > 
> > The (first) .text section must be always specified as the second
> > non-option argument.  The documentation states that GDB cannot
> > figure out this address by itself.  This is true if the object file
> > was indeed relocated, but it is also confusing, because all other
> > sections can be omitted and will use the address provided by BFD.
> > 
> > gdb/ChangeLog:
> > 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> > 
> > 	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
> > 	require the second argument.  If omitted, load sections at the
> > 	addresses specified in the file.
> > 
> > gdb/doc/ChangeLog:
> > 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> > 
> > 	* gdb.texinfo (Files): The address argument for "add-symbol-file"
> > 	is no longer mandatory.
> > 
> > gdb/testsuite/ChangeLog:
> > 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> > 
> > 	* gdb.base/relocate.exp: Test add-symbol-file behavior when the
> > 	address argument is omitted.  
> 
> I have a feeling I already reviewed and approved this, but in case I
> didn't, I approve it now.

You did, but I resent the whole patch series again, improving the
changelog style and adding the help texts where necessary. I found it
better than fixing it up with a separate patch.

I'm sorry if that was confusing.

Petr T

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 2/4] Make add-symbol-file's address argument optional
  2018-06-11 16:50     ` Petr Tesarik
@ 2018-06-11 17:25       ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2018-06-11 17:25 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches, jeffm

> Date: Mon, 11 Jun 2018 18:50:48 +0200
> From: Petr Tesarik <ptesarik@suse.cz>
> Cc: gdb-patches@sourceware.org, jeffm@suse.com
> 
> > I have a feeling I already reviewed and approved this, but in case I
> > didn't, I approve it now.
> 
> You did, but I resent the whole patch series again, improving the
> changelog style and adding the help texts where necessary. I found it
> better than fixing it up with a separate patch.

Well, in the future just say in the text that precedes the patch that
the documentation parts were already approved, that should be enough.

Thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/4] Allow loading symbol files with an offset
  2018-06-11 12:08 [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
                   ` (3 preceding siblings ...)
  2018-06-11 12:23 ` [PATCH v2 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
@ 2018-06-22  7:52 ` Petr Tesarik
  2018-06-26  2:58   ` Simon Marchi
  2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
  5 siblings, 1 reply; 26+ messages in thread
From: Petr Tesarik @ 2018-06-22  7:52 UTC (permalink / raw)
  To: gdb-patches

Hi all,

I'm sorry if I'm being impatient, but what should be the next step to
get this patch set reviewed and accepted? Anything I can do to help?

Also note that I haven't signed the copyright assignment form yet. I
have no issues with doing that, but I have no idea how to get things
rolling.

TIA,
Petr T

On Mon, 11 Jun 2018 14:08:31 +0200
Petr Tesarik <ptesarik@suse.cz> wrote:

> Implement an "-o" option for symbol-file and add-symbol-file.
> 
> Changes since v1:
> * Adjust help texts
> * Update gdb/NEWS
> 
> Petr Tesarik (4):
>   Add an optional offset option to the "symbol-file" command
>   Make add-symbol-file's address argument optional
>   Make sure that sorting does not change section order
>   Add an optional offset option to the "add-symbol-file" command
> 
>  gdb/ChangeLog                       |  23 ++++++++
>  gdb/NEWS                            |  10 ++++
>  gdb/doc/ChangeLog                   |  13 +++++
>  gdb/doc/gdb.texinfo                 |  24 ++++++--
>  gdb/symfile.c                       | 108 +++++++++++++++++++++++++++++-------
>  gdb/testsuite/ChangeLog             |  13 +++++
>  gdb/testsuite/gdb.base/relocate.exp |  95 +++++++++++++++++++++++++++++++
>  7 files changed, 259 insertions(+), 27 deletions(-)
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command
  2018-06-11 12:08 ` [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
  2018-06-11 15:23   ` Eli Zaretskii
@ 2018-06-26  2:02   ` Simon Marchi
  1 sibling, 0 replies; 26+ messages in thread
From: Simon Marchi @ 2018-06-26  2:02 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches, Jeff Mahoney

On 2018-06-11 08:08, Petr Tesarik 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.
> 
> gdb/ChangeLog:
> 2018-06-11  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-06-08  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* gdb.texinfo (Files): Document "symbol-file -o offset".
> 
> gdb/testsuite/ChangeLog:
> 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* gdb.base/relocate.exp: Add test for "symbol-file -o ".

LGTM.

Simon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 2/4] Make add-symbol-file's address argument optional
  2018-06-11 12:08 ` [PATCH v2 2/4] Make add-symbol-file's address argument optional Petr Tesarik
  2018-06-11 15:25   ` Eli Zaretskii
@ 2018-06-26  2:14   ` Simon Marchi
  1 sibling, 0 replies; 26+ messages in thread
From: Simon Marchi @ 2018-06-26  2:14 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches, Jeff Mahoney

Hi Petr,

The patch LGTM, with some nits to address before pushing.

On 2018-06-11 08:08, Petr Tesarik wrote:
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 973365574f..84600bfe5f 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -18917,18 +18917,21 @@ the program is running.  To do this, use the
> @code{kill} command
> 
>  @kindex add-symbol-file
>  @cindex dynamic linking
> -@item add-symbol-file @var{filename} @var{address}
> -@itemx add-symbol-file @var{filename} @var{address} @r{[} -readnow
> @r{|} -readnever @r{]}
> -@itemx add-symbol-file @var{filename} @var{address} -s @var{section}
> @var{address} @dots{}
> +@item add-symbol-file @var{filename} @r{[} @var{address} @r{]}
> +@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} @r{[}
> -readnow @r{|} -readnever @r{]}
> +@itemx add-symbol-file @var{filename} @r{[} @var{address} @r{]} -s
> @var{section} @var{address} @dots{}
>  The @code{add-symbol-file} command reads additional symbol table
>  information from the file @var{filename}.  You would use this command
>  when @var{filename} has been dynamically loaded (by some other means)
>  into the program that is running.  The @var{address} should give the 
> memory
> -address at which the file has been loaded; @value{GDBN} cannot figure
> -this out for itself.  You can additionally specify an arbitrary number
> +address at which the file has been loaded.
> +You can additionally specify an arbitrary number
>  of @samp{-s @var{section} @var{address}} pairs, to give an explicit
>  section name and base address for that section.  You can specify any
>  @var{address} as an expression.
> +If @var{address} is omitted, @value{GDBN} will use the section
> +addresses found in @var{filename}.  You can use @samp{-s} to
> +override this default and load a section at a different address.

I really think that this section could use some improvements:

- There are two arguments named "address", so it's not clear what the 
text refers to.
- I don't think it's useful to have the synopsis on three different 
lines, since the options are not mutually exclusive.
- It should be made clear that the positional "address" argument 
specifies the start of the .text section.  Since it is now optional, I 
also think that this positional argument should be deprecated in favor 
of using "-s .text ..."...

But none of this is a direct consequence of your patch, so your patch 
looks ok to me.

> 
>  The symbol table of the file @var{filename} is added to the symbol 
> table
>  originally read with the @code{symbol-file} command.  You can use the
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 461f60d074..3e3ab20412 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -2161,29 +2161,26 @@ add_symbol_file_command (const char *args, int 
> from_tty)

There might be the error message:

       error (_("add-symbol-file takes a file name and an address"));

that would need to be updated, now that only the file name is mandatory.

> diff --git a/gdb/testsuite/gdb.base/relocate.exp
> b/gdb/testsuite/gdb.base/relocate.exp
> index 77f6a88159..a3af8cea61 100644
> --- a/gdb/testsuite/gdb.base/relocate.exp
> +++ b/gdb/testsuite/gdb.base/relocate.exp
> @@ -73,6 +73,21 @@ gdb_test_multiple "add-symbol-file -s .text 0x200
> $binfile 0x100" $test {
>  	gdb_test "n" "Not confirmed\." $test
>      }
>  }
> +# Check that passing a single "-s .text" is equivallent to passing

equivallent -> equivalent.

Thanks,

Simon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/4] Make sure that sorting does not change section order
  2018-06-11 12:08 ` [PATCH v2 3/4] Make sure that sorting does not change section order Petr Tesarik
@ 2018-06-26  2:37   ` Simon Marchi
  2018-06-26  5:10     ` Petr Tesarik
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Marchi @ 2018-06-26  2:37 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches, Jeff Mahoney

On 2018-06-11 08:08, Petr Tesarik wrote:
> Symbol files may contain multiple sections with the same name.
> Section addresses specified add-symbol-file are assigned to the
> corresponding BFD sections in addr_info_make_relative using sorted
> indexes of both vectors.  Since the sort algorithm is not inherently
> stable, the comparison function uses sectindex to maintain the
> original order.  However, add_symbol_file_command uses zero for all
> sections, so if the user specifies multiple sections with the same
> name, they will be assigned randomly to symbol file sections with
> the same name.
> 
> gdb/ChangeLog:
> 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> 
> 	* symfile.c (add_symbol_file_command): Make sure that sections
> 	with the same name are sorted in the same order.
> ---
>  gdb/ChangeLog | 5 +++++
>  gdb/symfile.c | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 1c5a1f6bfb..0f75992d4c 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,10 @@
>  2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> 
> +	* symfile.c (add_symbol_file_command): Make sure that sections
> +	with the same name are sorted in the same order.
> +
> +2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> +
>  	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
>  	require the second argument.  If omitted, load sections at the
>  	addresses specified in the file.
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 3e3ab20412..8b8b194334 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -2185,7 +2185,7 @@ add_symbol_file_command (const char *args, int 
> from_tty)
> 
>        /* Here we store the section offsets in the order they were
>           entered on the command line.  */
> -      section_addrs.emplace_back (addr, sec, 0);
> +      section_addrs.emplace_back (addr, sec, section_addrs.size ());
>        printf_unfiltered ("\t%s_addr = %s\n", sec,
>  			 paddress (gdbarch, addr));

It took me a while to acknowledge that this was correct, because 
other_sections::sectindex usually refers to the section index in the 
BFD.  After digging I understood that this field was actually unused 
until filled by addr_info_make_relative, and that you kind of 
re-purposed it.  It sounds like there should be some comment at 
other_sections::sectindex and probably in add_symbol_file_command to 
explain how it's used.

Another option would be to use std::stable_sort instead of std::sort.  
But it's more resource-hungry and not needed for all paths that lead to 
addrs_section_sort, so it would be a bit wasteful.

Simon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 4/4] Add an optional offset option to the "add-symbol-file" command
  2018-06-11 12:23 ` [PATCH v2 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
@ 2018-06-26  2:52   ` Simon Marchi
  0 siblings, 0 replies; 26+ messages in thread
From: Simon Marchi @ 2018-06-26  2:52 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches, Jeff Mahoney

On 2018-06-11 08:08, Petr Tesarik wrote:
> @@ -2195,7 +2206,13 @@ add_symbol_file_command (const char *args, int 
> from_tty)
>  	 At this point, we don't know what file type this is,
>  	 so we can't determine what section names are valid.  */
>      }
> -  if (section_addrs.size () == 0)
> +  if (seen_offset)
> +      printf_unfiltered (_("%s offset by %s\n"),
> +			 (section_addrs.size () == 0
> +			  ? _(" with all sections")
> +			  : _("with other sections")),
> +			 paddress (gdbarch, offset));
> +  else if (section_addrs.size () == 0)

Nice little detail, the message that adapts to how the command was used.

You can use ".empty ()" instead of .size () == 0" (in the previous 
patches too).

>      printf_unfiltered ("\n");
> 
>    if (from_tty && (!query ("%s", "")))
> @@ -2204,6 +2221,42 @@ add_symbol_file_command (const char *args, int 
> from_tty)
>    objf = symbol_file_add (filename.get (), add_flags, &section_addrs,
>  			  flags);
> 
> +  if (seen_offset)
> +    {
> +      std::vector<struct section_offsets> new_offsets 
> (objf->num_sections,
> +						       { offset });
> +
> +      std::vector<const struct other_sections *> sect_addrs_sorted
> +	= addrs_section_sort (section_addrs);
> +
> +      section_addr_info objf_addrs
> +	= build_section_addr_info_from_objfile (objf);
> +      std::vector<const struct other_sections *> objf_addrs_sorted
> +	= addrs_section_sort (objf_addrs);
> +
> +      std::vector<const struct other_sections *>::iterator 
> sect_sorted_iter
> +	= sect_addrs_sorted.begin ();
> +      for (const struct other_sections *objf_sect : objf_addrs_sorted)
> +	{
> +	  const char *objf_name = addr_section_name (objf_sect->name.c_str 
> ());
> +	  int cmp = -1;
> +
> +	  while (cmp < 0 && sect_sorted_iter != sect_addrs_sorted.end ())
> +	    {
> +	      const struct other_sections *sect = *sect_sorted_iter;
> +	      const char *sect_name = addr_section_name (sect->name.c_str 
> ());
> +	      cmp = strcmp (sect_name, objf_name);
> +	      if (cmp <= 0)
> +		++sect_sorted_iter;
> +	    }
> +
> +	  if (cmp == 0)
> +	    new_offsets[objf_sect->sectindex].offsets[0] = 0;
> +	}
> +
> +      objfile_relocate (objf, new_offsets.data ());
> +    }
> +

Could this new code be in a separate function?  This one is starting to 
get long.

Also, could you add a bit of comments to explain the intent of the code? 
  There is a lot of magic going on, it's a bit hard to read.

> diff --git a/gdb/testsuite/gdb.base/relocate.exp
> b/gdb/testsuite/gdb.base/relocate.exp
> index a3af8cea61..119495dd94 100644
> --- a/gdb/testsuite/gdb.base/relocate.exp
> +++ b/gdb/testsuite/gdb.base/relocate.exp
> @@ -235,6 +235,62 @@ set new_function_foo_addr [get_var_address 
> function_foo]
>  gdb_assert {${new_function_foo_addr} == ${function_foo_addr} + 
> $offset} \
>      "function foo is moved by offset"
> 
> +# Load the object using add-symbol-file with an offset and check that
> +# all addresses are moved by that offset.
> +
> +set offset 0x10000
> +clean_restart
> +gdb_test "add-symbol-file -o $offset $binfile" \
> +    "Reading symbols from ${binfile}\.\.\.done\." \
> +    "add-symbol-file with offset" \
> +    "add symbol table from file \".*${testfile}\\.o\" with all
> sections offset by $offset\[\r\n\]+\\(y or n\\) " \
> +    "y"
> +
> +# Make sure the address of a static variable is moved by offset.
> +set new_static_foo_addr [get_var_address static_foo]
> +gdb_assert { ${new_static_foo_addr} == ${static_foo_addr} + $offset } 
> \
> +    "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]
> +gdb_assert { ${new_global_foo_addr} == ${global_foo_addr} + $offset } 
> \
> +    "global variable foo is moved by offset"
> +
> +# Make sure the address of a function is moved by offset.
> +set new_function_foo_addr [get_var_address function_foo]
> +gdb_assert { ${new_function_foo_addr} == ${function_foo_addr} + 
> $offset } \
> +    "function foo is moved by offset"
> +
> +# Re-load the object giving an explicit address for .text
> +
> +set text [ format "0x%x" [expr ${function_foo_addr} + 0x20000] ]
> +clean_restart
> +gdb_test "add-symbol-file $binfile -o $offset $text" \
> +    "Reading symbols from ${binfile}\.\.\.done\." \
> +    "add-symbol-file with offset, text address given" \
> +    "add symbol table from file \".*${testfile}\\.o\" at\[
> \t\r\n\]+\.text_addr = ${text}\[\r\n\]+with other sections offset by
> ${offset}\[\r\n\]+\\(y or n\\) " \
> +    "y"
> +
> +# Make sure function has a different addresses now.

"has a different addresses" -> "has a different address", repeated a few 
times.

Simon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/4] Allow loading symbol files with an offset
  2018-06-22  7:52 ` [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
@ 2018-06-26  2:58   ` Simon Marchi
  2018-06-26  5:01     ` Petr Tesarik
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Marchi @ 2018-06-26  2:58 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches

On 2018-06-22 03:49, Petr Tesarik wrote:
> Hi all,
> 
> I'm sorry if I'm being impatient, but what should be the next step to
> get this patch set reviewed and accepted? Anything I can do to help?
> 
> Also note that I haven't signed the copyright assignment form yet. I
> have no issues with doing that, but I have no idea how to get things
> rolling.
> 
> TIA,
> Petr T

Hi Petr,

For individual contributors, the form can be found here, in the gnulib 
repo:

http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=doc/Copyright/request-assign.future;hb=HEAD

You can follow the instructions included there.  However, are you 
contributing on behalf of your employer?  If so, I am not sure of the 
procedure.  I would mention it in that form, and the folks at 
assign@gnu.org will probably know what to do.

Simon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/4] Allow loading symbol files with an offset
  2018-06-26  2:58   ` Simon Marchi
@ 2018-06-26  5:01     ` Petr Tesarik
  2018-06-26 15:15       ` Simon Marchi
  0 siblings, 1 reply; 26+ messages in thread
From: Petr Tesarik @ 2018-06-26  5:01 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Hi Simon,

On Mon, 25 Jun 2018 22:58:09 -0400
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> On 2018-06-22 03:49, Petr Tesarik wrote:
> > Hi all,
> > 
> > I'm sorry if I'm being impatient, but what should be the next step to
> > get this patch set reviewed and accepted? Anything I can do to help?
> > 
> > Also note that I haven't signed the copyright assignment form yet. I
> > have no issues with doing that, but I have no idea how to get things
> > rolling.
> > 
> > TIA,
> > Petr T  
> 
> Hi Petr,
> 
> For individual contributors, the form can be found here, in the gnulib 
> repo:
> 
> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=doc/Copyright/request-assign.future;hb=HEAD
> 
> You can follow the instructions included there.  However, are you 
> contributing on behalf of your employer?

Thank you very much for the pointers. I have now also found out who is
responsible for the blanket copyright assignment of Novell/SUSE, so
this part should be OK.

Petr T

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/4] Make sure that sorting does not change section order
  2018-06-26  2:37   ` Simon Marchi
@ 2018-06-26  5:10     ` Petr Tesarik
  2018-06-26 13:26       ` Simon Marchi
  0 siblings, 1 reply; 26+ messages in thread
From: Petr Tesarik @ 2018-06-26  5:10 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Mon, 25 Jun 2018 22:36:58 -0400
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> On 2018-06-11 08:08, Petr Tesarik wrote:
> > Symbol files may contain multiple sections with the same name.
> > Section addresses specified add-symbol-file are assigned to the
> > corresponding BFD sections in addr_info_make_relative using sorted
> > indexes of both vectors.  Since the sort algorithm is not inherently
> > stable, the comparison function uses sectindex to maintain the
> > original order.  However, add_symbol_file_command uses zero for all
> > sections, so if the user specifies multiple sections with the same
> > name, they will be assigned randomly to symbol file sections with
> > the same name.
> > 
> > gdb/ChangeLog:
> > 2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> > 
> > 	* symfile.c (add_symbol_file_command): Make sure that sections
> > 	with the same name are sorted in the same order.
> > ---
> >  gdb/ChangeLog | 5 +++++
> >  gdb/symfile.c | 2 +-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > index 1c5a1f6bfb..0f75992d4c 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,5 +1,10 @@
> >  2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> > 
> > +	* symfile.c (add_symbol_file_command): Make sure that sections
> > +	with the same name are sorted in the same order.
> > +
> > +2018-06-11  Petr Tesarik  <ptesarik@suse.com>
> > +
> >  	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
> >  	require the second argument.  If omitted, load sections at the
> >  	addresses specified in the file.
> > diff --git a/gdb/symfile.c b/gdb/symfile.c
> > index 3e3ab20412..8b8b194334 100644
> > --- a/gdb/symfile.c
> > +++ b/gdb/symfile.c
> > @@ -2185,7 +2185,7 @@ add_symbol_file_command (const char *args, int 
> > from_tty)
> > 
> >        /* Here we store the section offsets in the order they were
> >           entered on the command line.  */
> > -      section_addrs.emplace_back (addr, sec, 0);
> > +      section_addrs.emplace_back (addr, sec, section_addrs.size ());
> >        printf_unfiltered ("\t%s_addr = %s\n", sec,
> >  			 paddress (gdbarch, addr));  
> 
> It took me a while to acknowledge that this was correct, because 
> other_sections::sectindex usually refers to the section index in the 
> BFD.  After digging I understood that this field was actually unused 
> until filled by addr_info_make_relative, and that you kind of 
> re-purposed it.  It sounds like there should be some comment at 
> other_sections::sectindex and probably in add_symbol_file_command to 
> explain how it's used.

Agreed. As a matter of fact, it also took me some while to understand
why add_symbol_file_command could get away with setting the index to
zero for all sections...

> Another option would be to use std::stable_sort instead of std::sort.  
> But it's more resource-hungry and not needed for all paths that lead to 
> addrs_section_sort, so it would be a bit wasteful.

Yes, I tried to avoid that solution. OTOH it's unlikely that there are
any object files with more than a few dozen sections, and to my best
knowledge this code is never in the GDB hot path, so if you prefer
std::stable_sort for clarity, I'm not against. Please, advise.

Petr T

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/4] Make sure that sorting does not change section order
  2018-06-26  5:10     ` Petr Tesarik
@ 2018-06-26 13:26       ` Simon Marchi
  0 siblings, 0 replies; 26+ messages in thread
From: Simon Marchi @ 2018-06-26 13:26 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches

On 2018-06-26 01:09, Petr Tesarik wrote:
>> It took me a while to acknowledge that this was correct, because
>> other_sections::sectindex usually refers to the section index in the
>> BFD.  After digging I understood that this field was actually unused
>> until filled by addr_info_make_relative, and that you kind of
>> re-purposed it.  It sounds like there should be some comment at
>> other_sections::sectindex and probably in add_symbol_file_command to
>> explain how it's used.
> 
> Agreed. As a matter of fact, it also took me some while to understand
> why add_symbol_file_command could get away with setting the index to
> zero for all sections...
> 
>> Another option would be to use std::stable_sort instead of std::sort.
>> But it's more resource-hungry and not needed for all paths that lead 
>> to
>> addrs_section_sort, so it would be a bit wasteful.
> 
> Yes, I tried to avoid that solution. OTOH it's unlikely that there are
> any object files with more than a few dozen sections, and to my best
> knowledge this code is never in the GDB hot path, so if you prefer
> std::stable_sort for clarity, I'm not against. Please, advise.
> 
> Petr T

I think you solution is fine, it just needs to be documented that it's 
intentional that we use some other value than the section index in 
sectindex.

Simon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/4] Allow loading symbol files with an offset
  2018-06-26  5:01     ` Petr Tesarik
@ 2018-06-26 15:15       ` Simon Marchi
  2018-06-26 15:23         ` Petr Tesarik
  0 siblings, 1 reply; 26+ messages in thread
From: Simon Marchi @ 2018-06-26 15:15 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: gdb-patches

On 2018-06-26 01:00, Petr Tesarik wrote:
> Thank you very much for the pointers. I have now also found out who is
> responsible for the blanket copyright assignment of Novell/SUSE, so
> this part should be OK.
> 
> Petr T

Ah ok, thanks!  So no further action is required about the copyright 
assignment then?

Simon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/4] Allow loading symbol files with an offset
  2018-06-26 15:15       ` Simon Marchi
@ 2018-06-26 15:23         ` Petr Tesarik
  0 siblings, 0 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-26 15:23 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Tue, 26 Jun 2018 11:15:11 -0400
Simon Marchi <simon.marchi@polymtl.ca> wrote:

> On 2018-06-26 01:00, Petr Tesarik wrote:
> > Thank you very much for the pointers. I have now also found out who is
> > responsible for the blanket copyright assignment of Novell/SUSE, so
> > this part should be OK.
> > 
> > Petr T  
> 
> Ah ok, thanks!  So no further action is required about the copyright 
> assignment then?

Exactly. At least not at the moment.

Petr T

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 0/4] Allow loading symbol files with an offset
  2018-06-11 12:08 [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
                   ` (4 preceding siblings ...)
  2018-06-22  7:52 ` [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
@ 2018-06-27 12:14 ` Petr Tesarik
  2018-06-27 12:14   ` [PATCH v3 3/4] Make sure that sorting does not change section order Petr Tesarik
                     ` (4 more replies)
  5 siblings, 5 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-27 12:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Eli Zaretskii, Petr Tesarik

Implement an "-o" option for symbol-file and add-symbol-file.

Changes since v2:
* Fix the error message if add-symbol-file is called without arguments
* Adjust texinfo description of add-symbol-file
* Clarify the use of other_sections::sectindex
* Use '.empty()' instead of '.size() == 0'
* Split off set_objfile_default_section_offset

Changes since v1:
* Adjust help texts
* Update gdb/NEWS

Petr Tesarik (4):
  Add an optional offset option to the "symbol-file" command
  Make add-symbol-file's address argument optional
  Make sure that sorting does not change section order
  Add an optional offset option to the "add-symbol-file" command

 gdb/ChangeLog                       |  24 ++++++
 gdb/NEWS                            |  10 +++
 gdb/doc/ChangeLog                   |  13 ++++
 gdb/doc/gdb.texinfo                 |  28 ++++---
 gdb/symfile.c                       | 141 ++++++++++++++++++++++++++++++------
 gdb/symfile.h                       |   4 +-
 gdb/testsuite/ChangeLog             |  13 ++++
 gdb/testsuite/gdb.base/relocate.exp |  95 ++++++++++++++++++++++++
 8 files changed, 294 insertions(+), 34 deletions(-)

-- 
2.16.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 2/4] Make add-symbol-file's address argument optional
  2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
  2018-06-27 12:14   ` [PATCH v3 3/4] Make sure that sorting does not change section order Petr Tesarik
@ 2018-06-27 12:14   ` Petr Tesarik
  2018-06-27 12:14   ` [PATCH v3 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-27 12:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Eli Zaretskii, Petr Tesarik

The (first) .text section must be always specified as the second
non-option argument.  The documentation states that GDB cannot
figure out this address by itself.  This is true if the object file
was indeed relocated, but it is also confusing, because all other
sections can be omitted and will use the address provided by BFD.

gdb/ChangeLog:
2018-06-27  Petr Tesarik  <ptesarik@suse.com>

	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
	require the second argument.  If omitted, load sections at the
	addresses specified in the file.

gdb/doc/ChangeLog:
2018-06-27  Petr Tesarik  <ptesarik@suse.com>

	* gdb.texinfo (Files): The address argument for "add-symbol-file"
	is no longer mandatory.

gdb/testsuite/ChangeLog:
2018-06-27  Petr Tesarik  <ptesarik@suse.com>

	* gdb.base/relocate.exp: Test add-symbol-file behavior when the
	address argument is omitted.

Note: Documentation originally approved by Eli Zaretskii <eliz@gnu.org>,
  but now improved based on Simon's suggestions, so it may need review.

---
 gdb/ChangeLog                       |  6 ++++++
 gdb/NEWS                            |  3 +++
 gdb/doc/ChangeLog                   |  5 +++++
 gdb/doc/gdb.texinfo                 | 17 ++++++++---------
 gdb/symfile.c                       | 27 +++++++++++++--------------
 gdb/testsuite/ChangeLog             |  5 +++++
 gdb/testsuite/gdb.base/relocate.exp | 15 +++++++++++++++
 7 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index dd52e7d7b8..e5303a9e9e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
+	require the second argument.  If omitted, load sections at the
+	addresses specified in the file.
+
 2018-06-27  Petr Tesarik  <ptesarik@suse.com>
 
 	* symfile.c (symbol_file_command, symbol_file_add_main_1)
diff --git a/gdb/NEWS b/gdb/NEWS
index 101746567a..54c0f1d19b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -6,6 +6,9 @@
 * The 'symbol-file' command now accepts an '-o' option to add a relative
   offset to all sections.
 
+* The 'add-symbol-file' command no longer requires the second argument
+  (address of the text section).
+
 * The endianness used with the 'set endian auto' mode in the absence of
   an executable selected for debugging is now the last endianness chosen
   either by one of the 'set endian big' and 'set endian little' commands
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 0e57166fe3..0f8664cfc1 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* gdb.texinfo (Files): The address argument for "add-symbol-file"
+	is no longer mandatory.
+
 2018-06-27  Petr Tesarik  <ptesarik@suse.com>
 
 	* gdb.texinfo (Files): Document "symbol-file -o offset".
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 328256236e..64c511d1e6 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18918,18 +18918,17 @@ the program is running.  To do this, use the @code{kill} command
 
 @kindex add-symbol-file
 @cindex dynamic linking
-@item add-symbol-file @var{filename} @var{address}
-@itemx add-symbol-file @var{filename} @var{address} @r{[} -readnow @r{|} -readnever @r{]}
-@itemx add-symbol-file @var{filename} @var{address} -s @var{section} @var{address} @dots{}
+@item add-symbol-file @var{filename} @r{[} -readnow @r{|} -readnever @r{]} @r{[} @var{textaddress} @r{]} @r{[} -s @var{section} @var{address} @dots{} @r{]}
 The @code{add-symbol-file} command reads additional symbol table
 information from the file @var{filename}.  You would use this command
 when @var{filename} has been dynamically loaded (by some other means)
-into the program that is running.  The @var{address} should give the memory
-address at which the file has been loaded; @value{GDBN} cannot figure
-this out for itself.  You can additionally specify an arbitrary number
-of @samp{-s @var{section} @var{address}} pairs, to give an explicit
-section name and base address for that section.  You can specify any
-@var{address} as an expression.
+into the program that is running.  The @var{textaddress} parameter gives
+the memory address at which the file's text section has been loaded.
+You can additionally specify the base address of other sections using
+an arbitrary number of @samp{-s @var{section} @var{address}} pairs.
+If a section is omitted, @value{GDBN} will use its default addresses
+as found in @var{filename}.  Any @var{address} or @var{textaddress}
+can be given as an expression.
 
 The symbol table of the file @var{filename} is added to the symbol table
 originally read with the @code{symbol-file} command.  You can use the
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 461f60d074..2a41fce64f 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2161,29 +2161,26 @@ add_symbol_file_command (const char *args, int from_tty)
 
   validate_readnow_readnever (flags);
 
-  /* This command takes at least two arguments.  The first one is a
-     filename, and the second is the address where this file has been
-     loaded.  Abort now if this address hasn't been provided by the
-     user.  */
-  if (!seen_addr)
-    error (_("The address where %s has been loaded is missing"),
-	   filename.get ());
-
   /* Print the prompt for the query below.  And save the arguments into
      a sect_addr_info structure to be passed around to other
      functions.  We have to split this up into separate print
      statements because hex_string returns a local static
      string.  */
 
-  printf_unfiltered (_("add symbol table from file \"%s\" at\n"),
+  printf_unfiltered (_("add symbol table from file \"%s\""),
 		     filename.get ());
   section_addr_info section_addrs;
-  for (sect_opt &sect : sect_opts)
+  std::vector<sect_opt>::const_iterator it = sect_opts.begin ();
+  if (!seen_addr)
+    ++it;
+  for (; it != sect_opts.end (); ++it)
     {
       CORE_ADDR addr;
-      const char *val = sect.value;
-      const char *sec = sect.name;
+      const char *val = it->value;
+      const char *sec = it->name;
 
+      if (section_addrs.empty ())
+	printf_unfiltered (_(" at\n"));
       addr = parse_and_eval_address (val);
 
       /* Here we store the section offsets in the order they were
@@ -2198,6 +2195,8 @@ add_symbol_file_command (const char *args, int from_tty)
 	 At this point, we don't know what file type this is,
 	 so we can't determine what section names are valid.  */
     }
+  if (section_addrs.empty ())
+    printf_unfiltered ("\n");
 
   if (from_tty && (!query ("%s", "")))
     error (_("Not confirmed."));
@@ -3793,8 +3792,8 @@ to execute.\n" READNOW_READNEVER_HELP), &cmdlist);
 
   c = add_cmd ("add-symbol-file", class_files, add_symbol_file_command, _("\
 Load symbols from FILE, assuming FILE has been dynamically loaded.\n\
-Usage: add-symbol-file FILE ADDR [-readnow | -readnever | \
--s SECT-NAME SECT-ADDR]...\n\
+Usage: add-symbol-file FILE [-readnow | -readnever] [ADDR] \
+[-s SECT-NAME SECT-ADDR]...\n\
 ADDR is the starting address of the file's text.\n\
 Each '-s' argument provides a section name and address, and\n\
 should be specified if the data and bss segments are not contiguous\n\
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index d9900d3e86..47f055cbfa 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* gdb.base/relocate.exp: Test add-symbol-file behavior when the
+	address argument is omitted.
+
 2018-06-27  Petr Tesarik  <ptesarik@suse.com>
 
 	* gdb.base/relocate.exp: Add test for "symbol-file -o ".
diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
index 77f6a88159..2b4c4a7044 100644
--- a/gdb/testsuite/gdb.base/relocate.exp
+++ b/gdb/testsuite/gdb.base/relocate.exp
@@ -73,6 +73,21 @@ gdb_test_multiple "add-symbol-file -s .text 0x200 $binfile 0x100" $test {
 	gdb_test "n" "Not confirmed\." $test
     }
 }
+# Check that passing a single "-s .text" is equivalent to passing
+# the text address in a positional argument.
+set test "add-symbol-file -s .text, no address"
+gdb_test_multiple "add-symbol-file $binfile -s .text 0x100" $test {
+    -re "add symbol table from file \"${binfile}\" at\r\n\t\.text_addr = 0x100\r\n\\(y or n\\) " {
+	gdb_test "n" "Not confirmed\." $test
+    }
+}
+# Check section addresses can be omitted.
+set test "add-symbol-file no address"
+gdb_test_multiple "add-symbol-file $binfile" $test {
+    -re "add symbol table from file \"${binfile}\"\r\n\\(y or n\\) " {
+	gdb_test "n" "Not confirmed\." $test
+    }
+}
 # Test that passing "--" disables option processing.
 gdb_test "add-symbol-file -- $binfile 0x100 -s .bss 0x3" \
     "Unrecognized argument \"-s\"" \
-- 
2.16.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 3/4] Make sure that sorting does not change section order
  2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
@ 2018-06-27 12:14   ` Petr Tesarik
  2018-06-27 12:14   ` [PATCH v3 2/4] Make add-symbol-file's address argument optional Petr Tesarik
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-27 12:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Eli Zaretskii, Petr Tesarik

Symbol files may contain multiple sections with the same name.
Section addresses specified by add-symbol-file are assigned to the
corresponding BFD sections in addr_info_make_relative using sorted
indexes of both vectors.  Since the sort algorithm is not inherently
stable, the comparison function uses sectindex to maintain the
original order.  However, add_symbol_file_command uses zero for all
sections, so if the user specifies multiple sections with the same
name, they will be assigned randomly to symbol file sections with
the same name.

gdb/ChangeLog:
2018-06-27  Petr Tesarik  <ptesarik@suse.com>

	* symfile.c (add_symbol_file_command): Make sure that sections
---
 gdb/ChangeLog |  5 +++++
 gdb/symfile.c | 11 +++++++++--
 gdb/symfile.h |  4 +++-
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e5303a9e9e..96ae6cc1a8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* symfile.c (add_symbol_file_command): Make sure that sections
+	with the same name are sorted in the same order.
+
 2018-06-27  Petr Tesarik  <ptesarik@suse.com>
 
 	* symfile.c (add_symbol_file_command, _initialize_symfile): Do not
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 2a41fce64f..b592be74cf 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -908,6 +908,9 @@ init_entry_point_info (struct objfile *objfile)
    into an offset from the section VMA's as it appears in the object
    file, and then call the file's sym_offsets function to convert this
    into a format-specific offset table --- a `struct section_offsets'.
+   The sectindex field is used to control the ordering of sections
+   with the same name.  Upon return, it is updated to contain the
+   correspondig BFD section index, or -1 if the section was not found.
 
    ADD_FLAGS encodes verbosity level, whether this is main symbol or
    an extra symbol file such as dynamically loaded code, and wether
@@ -2184,8 +2187,12 @@ add_symbol_file_command (const char *args, int from_tty)
       addr = parse_and_eval_address (val);
 
       /* Here we store the section offsets in the order they were
-         entered on the command line.  */
-      section_addrs.emplace_back (addr, sec, 0);
+         entered on the command line.  Every array element is
+         assigned an ascending section index to preserve the above
+         order over an unstable sorting algorithm.  This dummy
+         index is not used for any other purpose.
+      */
+      section_addrs.emplace_back (addr, sec, section_addrs.size ());
       printf_unfiltered ("\t%s_addr = %s\n", sec,
 			 paddress (gdbarch, addr));
 
diff --git a/gdb/symfile.h b/gdb/symfile.h
index d9185092ee..1b47669048 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -61,7 +61,9 @@ struct other_sections
   CORE_ADDR addr;
   std::string name;
 
-  /* SECTINDEX must be valid for associated BFD or set to -1.  */
+  /* SECTINDEX must be valid for associated BFD or set to -1.
+     See syms_from_objfile_1 for an exception to this rule.
+   */
   int sectindex;
 };
 
-- 
2.16.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 1/4] Add an optional offset option to the "symbol-file" command
  2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
  2018-06-27 12:14   ` [PATCH v3 3/4] Make sure that sorting does not change section order Petr Tesarik
  2018-06-27 12:14   ` [PATCH v3 2/4] Make add-symbol-file's address argument optional Petr Tesarik
@ 2018-06-27 12:14   ` Petr Tesarik
  2018-06-27 12:33   ` [PATCH v3 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
  2018-06-27 12:57   ` [PATCH v3 0/4] Allow loading symbol files with an offset Simon Marchi
  4 siblings, 0 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-27 12:14 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Eli Zaretskii, Petr Tesarik

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.

gdb/ChangeLog:
2018-06-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-06-27  Petr Tesarik  <ptesarik@suse.com>

	* gdb.texinfo (Files): Document "symbol-file -o offset".

gdb/testsuite/ChangeLog:
2018-06-27  Petr Tesarik  <ptesarik@suse.com>

	* gdb.base/relocate.exp: Add test for "symbol-file -o ".


Note: Documentation already approved by Eli Zaretskii <eliz@gnu.org>

---
 gdb/ChangeLog                       |  6 ++++++
 gdb/NEWS                            |  3 +++
 gdb/doc/ChangeLog                   |  4 ++++
 gdb/doc/gdb.texinfo                 |  7 ++++++-
 gdb/symfile.c                       | 24 ++++++++++++++++++------
 gdb/testsuite/ChangeLog             |  4 ++++
 gdb/testsuite/gdb.base/relocate.exp | 24 ++++++++++++++++++++++++
 7 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d4ae16c584..dd52e7d7b8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-06-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-06-26  Joel Brobecker  <brobecker@adacore.com>
 
 	* windows-nat.c (do_windows_fetch_inferior_registers): Rename
diff --git a/gdb/NEWS b/gdb/NEWS
index 13da2f1d4e..101746567a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 8.1
 
+* The 'symbol-file' command now accepts an '-o' option to add a relative
+  offset to all sections.
+
 * The endianness used with the 'set endian auto' mode in the absence of
   an executable selected for debugging is now the last endianness chosen
   either by one of the 'set endian big' and 'set endian little' commands
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index ade1799b06..0e57166fe3 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* gdb.texinfo (Files): Document "symbol-file -o offset".
+
 2018-06-14  Tom de Vries  <tdevries@suse.de>
 
 	* gdb.texinfo (Background Execution): Add @cindex for '&'.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a6bad13d9d..328256236e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18823,11 +18823,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{[} @var{filename} @r{[} -o @var{offset} @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 f8177ea8b1..461f60d074 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 reloff);
 
 static const struct sym_fns *find_sym_fns (bfd *);
 
@@ -1225,16 +1225,18 @@ 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 reloff)
 {
   add_flags |= current_inferior ()->symfile_flags | SYMFILE_MAINLINE;
 
-  symbol_file_add (args, add_flags, NULL, flags);
+  struct objfile *objfile = symbol_file_add (args, add_flags, NULL, flags);
+  if (reloff != 0)
+    objfile_rebase (objfile, reloff);
 
   /* Getting new symbols may change our opinion about
      what is frameless.  */
@@ -1551,6 +1553,7 @@ symbol_file_command (const char *args, int from_tty)
       symfile_add_flags add_flags = 0;
       char *name = NULL;
       bool stop_processing_options = false;
+      CORE_ADDR offset = 0;
       int idx;
       char *arg;
 
@@ -1571,6 +1574,14 @@ 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)
+	    {
+	      arg = built_argv[++idx];
+	      if (arg == NULL)
+		error (_("Missing argument to -o"));
+
+	      offset = parse_and_eval_address (arg);
+	    }
 	  else if (strcmp (arg, "--") == 0)
 	    stop_processing_options = true;
 	  else
@@ -1582,7 +1593,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);
     }
 }
 
@@ -3774,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 b2901dbcc5..d9900d3e86 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* gdb.base/relocate.exp: Add test for "symbol-file -o ".
+
 2018-06-26  Tom Tromey  <tom@tromey.com>
 
 	PR rust/22574:
diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
index 89f2fffcd9..77f6a88159 100644
--- a/gdb/testsuite/gdb.base/relocate.exp
+++ b/gdb/testsuite/gdb.base/relocate.exp
@@ -196,6 +196,30 @@ 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]
+gdb_assert {${new_static_foo_addr} == ${static_foo_addr} + $offset} \
+    "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]
+gdb_assert {${new_global_foo_addr} == ${global_foo_addr} + $offset} \
+    "global variable foo is moved by offset"
+
+# Make sure the address of a function is moved by offset.
+set new_function_foo_addr [get_var_address function_foo]
+gdb_assert {${new_function_foo_addr} == ${function_foo_addr} + $offset} \
+    "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.
 
-- 
2.16.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 4/4] Add an optional offset option to the "add-symbol-file" command
  2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
                     ` (2 preceding siblings ...)
  2018-06-27 12:14   ` [PATCH v3 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
@ 2018-06-27 12:33   ` Petr Tesarik
  2018-06-27 12:57   ` [PATCH v3 0/4] Allow loading symbol files with an offset Simon Marchi
  4 siblings, 0 replies; 26+ messages in thread
From: Petr Tesarik @ 2018-06-27 12:33 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Eli Zaretskii, Petr Tesarik

If all sections of a symbol file are loaded with a fixed offset, it
is easier to specify that offset than listing all sections
explicitly.  There is also a similar option for "symbol-file".

gdb/ChangeLog:
2018-06-27  Petr Tesarik  <ptesarik@suse.com>

	* symfile.c (add_symbol_file_command, _initialize_symfile): Add
	option "-o" to add-symbol-file-load to add an offset to each
	section's load address.
	* symfile.c (set_objfile_default_section_offset): New function.

gdb/doc/ChangeLog:
2018-06-27  Petr Tesarik  <ptesarik@suse.com>

	* gdb.texinfo (Files): Document "add-symbol-file -o offset".

gdb/testsuite/ChangeLog:
2018-06-27  Petr Tesarik  <ptesarik@suse.com>

	* gdb.base/relocate.exp: Add test for "add-symbol-file -o ".

Note: Documentation already approved by Eli Zaretskii <eliz@gnu.org>

---
 gdb/ChangeLog                       |  7 ++++
 gdb/NEWS                            |  4 ++
 gdb/doc/ChangeLog                   |  4 ++
 gdb/doc/gdb.texinfo                 |  6 ++-
 gdb/symfile.c                       | 83 +++++++++++++++++++++++++++++++++++--
 gdb/testsuite/ChangeLog             |  4 ++
 gdb/testsuite/gdb.base/relocate.exp | 56 +++++++++++++++++++++++++
 7 files changed, 160 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 96ae6cc1a8..8c1bad9d1b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* symfile.c (add_symbol_file_command, _initialize_symfile): Add
+	option "-o" to add-symbol-file-load to add an offset to each
+	section's load address.
+	* symfile.c (set_objfile_default_section_offset): New function.
+
 2018-06-27  Petr Tesarik  <ptesarik@suse.com>
 
 	* symfile.c (add_symbol_file_command): Make sure that sections
diff --git a/gdb/NEWS b/gdb/NEWS
index 54c0f1d19b..016796a802 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -6,6 +6,10 @@
 * The 'symbol-file' command now accepts an '-o' option to add a relative
   offset to all sections.
 
+* Similarly, the 'add-symbol-file' command also accepts an '-o' option to add
+  a relative offset to all sections, but it allows to override the load
+  address of individual sections using '-s'.
+
 * The 'add-symbol-file' command no longer requires the second argument
   (address of the text section).
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 0f8664cfc1..292058c823 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,7 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* gdb.texinfo (Files): Document "add-symbol-file -o offset".
+
 2018-06-27  Petr Tesarik  <ptesarik@suse.com>
 
 	* gdb.texinfo (Files): The address argument for "add-symbol-file"
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 64c511d1e6..7fb6ac5636 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18918,7 +18918,7 @@ the program is running.  To do this, use the @code{kill} command
 
 @kindex add-symbol-file
 @cindex dynamic linking
-@item add-symbol-file @var{filename} @r{[} -readnow @r{|} -readnever @r{]} @r{[} @var{textaddress} @r{]} @r{[} -s @var{section} @var{address} @dots{} @r{]}
+@item add-symbol-file @var{filename} @r{[} -readnow @r{|} -readnever @r{]} @r{[} -o @var{offset} @r{]} @r{[} @var{textaddress} @r{]} @r{[} -s @var{section} @var{address} @dots{} @r{]}
 The @code{add-symbol-file} command reads additional symbol table
 information from the file @var{filename}.  You would use this command
 when @var{filename} has been dynamically loaded (by some other means)
@@ -18930,6 +18930,10 @@ If a section is omitted, @value{GDBN} will use its default addresses
 as found in @var{filename}.  Any @var{address} or @var{textaddress}
 can be given as an expression.
 
+If an optional @var{offset} is specified, it is added to the start
+address of each section, except those for which the address was
+specified explicitly.
+
 The symbol table of the file @var{filename} is added to the symbol table
 originally read with the @code{symbol-file} command.  You can use the
 @code{add-symbol-file} command any number of times; the new symbol data
diff --git a/gdb/symfile.c b/gdb/symfile.c
index b592be74cf..48eca5cc0e 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2078,6 +2078,61 @@ print_transfer_performance (struct ui_file *stream,
   uiout->text (".\n");
 }
 
+/* Add an OFFSET to the start address of each section in OBJF, except
+   sections that were specified in ADDRS.  */
+
+static void
+set_objfile_default_section_offset (struct objfile *objf,
+				    const section_addr_info &addrs,
+				    CORE_ADDR offset)
+{
+  /* Add OFFSET to all sections by default.  */
+  std::vector<struct section_offsets> offsets (objf->num_sections,
+					       { offset });
+
+  /* Create sorted lists of all sections in ADDRS as well as all
+     sections in OBJF.  */
+
+  std::vector<const struct other_sections *> addrs_sorted
+    = addrs_section_sort (addrs);
+
+  section_addr_info objf_addrs
+    = build_section_addr_info_from_objfile (objf);
+  std::vector<const struct other_sections *> objf_addrs_sorted
+    = addrs_section_sort (objf_addrs);
+
+  /* Walk the BFD section list, and if a matching section is found in
+     ADDRS_SORTED_LIST, set its offset to zero to keep its address
+     unchanged.
+
+     Note that both lists may contain multiple sections with the same
+     name, and then the sections from ADDRS are matched in BFD order
+     (thanks to sectindex).  */
+
+  std::vector<const struct other_sections *>::iterator addrs_sorted_iter
+    = addrs_sorted.begin ();
+  for (const struct other_sections *objf_sect : objf_addrs_sorted)
+    {
+      const char *objf_name = addr_section_name (objf_sect->name.c_str ());
+      int cmp = -1;
+
+      while (cmp < 0 && addrs_sorted_iter != addrs_sorted.end ())
+	{
+	  const struct other_sections *sect = *addrs_sorted_iter;
+	  const char *sect_name = addr_section_name (sect->name.c_str ());
+	  cmp = strcmp (sect_name, objf_name);
+	  if (cmp <= 0)
+	    ++addrs_sorted_iter;
+	}
+
+      if (cmp == 0)
+	offsets[objf_sect->sectindex].offsets[0] = 0;
+    }
+
+  /* Apply the new section offsets.  */
+  objfile_relocate (objf, offsets.data ());
+}
+
 /* This function allows the addition of incrementally linked object files.
    It does not modify any state in the target, only in the debugger.  */
 /* Note: ezannoni 2000-04-13 This function/command used to have a
@@ -2109,6 +2164,7 @@ add_symbol_file_command (const char *args, int from_tty)
 
   std::vector<sect_opt> sect_opts = { { ".text", NULL } };
   bool stop_processing_options = false;
+  CORE_ADDR offset = 0;
 
   dont_repeat ();
 
@@ -2116,6 +2172,7 @@ add_symbol_file_command (const char *args, int from_tty)
     error (_("add-symbol-file takes a file name and an address"));
 
   bool seen_addr = false;
+  bool seen_offset = false;
   gdb_argv argv (args);
 
   for (arg = argv[0], argcnt = 0; arg != NULL; arg = argv[++argcnt])
@@ -2153,6 +2210,15 @@ add_symbol_file_command (const char *args, int from_tty)
 	  sect_opts.push_back (sect);
 	  argcnt += 2;
 	}
+      else if (strcmp (arg, "-o") == 0)
+	{
+	  arg = argv[++argcnt];
+	  if (arg == NULL)
+	    error (_("Missing argument to -o"));
+
+	  offset = parse_and_eval_address (arg);
+	  seen_offset = true;
+	}
       else if (strcmp (arg, "--") == 0)
 	stop_processing_options = true;
       else
@@ -2202,7 +2268,13 @@ add_symbol_file_command (const char *args, int from_tty)
 	 At this point, we don't know what file type this is,
 	 so we can't determine what section names are valid.  */
     }
-  if (section_addrs.empty ())
+  if (seen_offset)
+      printf_unfiltered (_("%s offset by %s\n"),
+			 (section_addrs.empty ()
+			  ? _(" with all sections")
+			  : _("with other sections")),
+			 paddress (gdbarch, offset));
+  else if (section_addrs.empty ())
     printf_unfiltered ("\n");
 
   if (from_tty && (!query ("%s", "")))
@@ -2211,6 +2283,9 @@ add_symbol_file_command (const char *args, int from_tty)
   objf = symbol_file_add (filename.get (), add_flags, &section_addrs,
 			  flags);
 
+  if (seen_offset)
+    set_objfile_default_section_offset (objf, section_addrs, offset);
+
   add_target_sections_of_objfile (objf);
 
   /* Getting new symbols may change our opinion about what is
@@ -3799,12 +3874,14 @@ to execute.\n" READNOW_READNEVER_HELP), &cmdlist);
 
   c = add_cmd ("add-symbol-file", class_files, add_symbol_file_command, _("\
 Load symbols from FILE, assuming FILE has been dynamically loaded.\n\
-Usage: add-symbol-file FILE [-readnow | -readnever] [ADDR] \
+Usage: add-symbol-file FILE [-readnow | -readnever] [-o OFF] [ADDR] \
 [-s SECT-NAME SECT-ADDR]...\n\
 ADDR is the starting address of the file's text.\n\
 Each '-s' argument provides a section name and address, and\n\
 should be specified if the data and bss segments are not contiguous\n\
-with the text.  SECT-NAME is a section name to be loaded at SECT-ADDR.\n"
+with the text.  SECT-NAME is a section name to be loaded at SECT-ADDR.\n\
+OFF is an optional offset which is added to the default load addresses\n\
+of all sections for which no other address was specified.\n"
 READNOW_READNEVER_HELP),
 	       &cmdlist);
   set_cmd_completer (c, filename_completer);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 47f055cbfa..13f0693e22 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2018-06-27  Petr Tesarik  <ptesarik@suse.com>
+
+	* gdb.base/relocate.exp: Add test for "add-symbol-file -o ".
+
 2018-06-27  Petr Tesarik  <ptesarik@suse.com>
 
 	* gdb.base/relocate.exp: Test add-symbol-file behavior when the
diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
index 2b4c4a7044..b6f18d9c69 100644
--- a/gdb/testsuite/gdb.base/relocate.exp
+++ b/gdb/testsuite/gdb.base/relocate.exp
@@ -235,6 +235,62 @@ set new_function_foo_addr [get_var_address function_foo]
 gdb_assert {${new_function_foo_addr} == ${function_foo_addr} + $offset} \
     "function foo is moved by offset"
 
+# Load the object using add-symbol-file with an offset and check that
+# all addresses are moved by that offset.
+
+set offset 0x10000
+clean_restart
+gdb_test "add-symbol-file -o $offset $binfile" \
+    "Reading symbols from ${binfile}\.\.\.done\." \
+    "add-symbol-file with offset" \
+    "add symbol table from file \".*${testfile}\\.o\" with all sections offset by $offset\[\r\n\]+\\(y or n\\) " \
+    "y"
+
+# Make sure the address of a static variable is moved by offset.
+set new_static_foo_addr [get_var_address static_foo]
+gdb_assert { ${new_static_foo_addr} == ${static_foo_addr} + $offset } \
+    "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]
+gdb_assert { ${new_global_foo_addr} == ${global_foo_addr} + $offset } \
+    "global variable foo is moved by offset"
+
+# Make sure the address of a function is moved by offset.
+set new_function_foo_addr [get_var_address function_foo]
+gdb_assert { ${new_function_foo_addr} == ${function_foo_addr} + $offset } \
+    "function foo is moved by offset"
+
+# Re-load the object giving an explicit address for .text
+
+set text [ format "0x%x" [expr ${function_foo_addr} + 0x20000] ]
+clean_restart
+gdb_test "add-symbol-file $binfile -o $offset $text" \
+    "Reading symbols from ${binfile}\.\.\.done\." \
+    "add-symbol-file with offset, text address given" \
+    "add symbol table from file \".*${testfile}\\.o\" at\[ \t\r\n\]+\.text_addr = ${text}\[\r\n\]+with other sections offset by ${offset}\[\r\n\]+\\(y or n\\) " \
+    "y"
+
+# Make sure function has a different addresses now.
+set function_foo_addr [get_var_address function_foo]
+gdb_assert { ${function_foo_addr} != ${new_function_foo_addr} } \
+    "function foo has a different address"
+
+# Re-load the object giving an explicit address for .data
+
+set data [ format "0x%x" [expr ${global_foo_addr} + 0x20000] ]
+clean_restart
+gdb_test "add-symbol-file $binfile -o $offset -s .data $data" \
+    "Reading symbols from ${binfile}\.\.\.done\." \
+    "add-symbol-file with offset, data address given" \
+    "add symbol table from file \".*${testfile}\\.o\" at\[ \t\r\n\]+\.data_addr = ${data}\[\r\n\]+with other sections offset by ${offset}\[\r\n\]+\\(y or n\\) " \
+    "y"
+
+# Make sure variable has a different addresses now.
+set global_foo_addr [get_var_address global_foo]
+gdb_assert { ${global_foo_addr} != ${new_global_foo_addr} } \
+    "global variable foo has a different address"
+
 # Now try loading the object as an exec-file; we should be able to print
 # the values of variables after we do this.
 
-- 
2.16.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 0/4] Allow loading symbol files with an offset
  2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
                     ` (3 preceding siblings ...)
  2018-06-27 12:33   ` [PATCH v3 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
@ 2018-06-27 12:57   ` Simon Marchi
  4 siblings, 0 replies; 26+ messages in thread
From: Simon Marchi @ 2018-06-27 12:57 UTC (permalink / raw)
  To: Petr Tesarik, gdb-patches; +Cc: Simon Marchi, Eli Zaretskii

On 2018-06-27 08:14 AM, Petr Tesarik wrote:
> Implement an "-o" option for symbol-file and add-symbol-file.
> 
> Changes since v2:
> * Fix the error message if add-symbol-file is called without arguments
> * Adjust texinfo description of add-symbol-file
> * Clarify the use of other_sections::sectindex
> * Use '.empty()' instead of '.size() == 0'
> * Split off set_objfile_default_section_offset
> 
> Changes since v1:
> * Adjust help texts
> * Update gdb/NEWS
> 
> Petr Tesarik (4):
>   Add an optional offset option to the "symbol-file" command
>   Make add-symbol-file's address argument optional
>   Make sure that sorting does not change section order
>   Add an optional offset option to the "add-symbol-file" command
> 
>  gdb/ChangeLog                       |  24 ++++++
>  gdb/NEWS                            |  10 +++
>  gdb/doc/ChangeLog                   |  13 ++++
>  gdb/doc/gdb.texinfo                 |  28 ++++---
>  gdb/symfile.c                       | 141 ++++++++++++++++++++++++++++++------
>  gdb/symfile.h                       |   4 +-
>  gdb/testsuite/ChangeLog             |  13 ++++
>  gdb/testsuite/gdb.base/relocate.exp |  95 ++++++++++++++++++++++++
>  8 files changed, 294 insertions(+), 34 deletions(-)
> 

Thanks Petr, this version LGTM.

Simon

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2018-06-27 12:57 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-11 12:08 [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
2018-06-11 12:08 ` [PATCH v2 2/4] Make add-symbol-file's address argument optional Petr Tesarik
2018-06-11 15:25   ` Eli Zaretskii
2018-06-11 16:50     ` Petr Tesarik
2018-06-11 17:25       ` Eli Zaretskii
2018-06-26  2:14   ` Simon Marchi
2018-06-11 12:08 ` [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
2018-06-11 15:23   ` Eli Zaretskii
2018-06-26  2:02   ` Simon Marchi
2018-06-11 12:08 ` [PATCH v2 3/4] Make sure that sorting does not change section order Petr Tesarik
2018-06-26  2:37   ` Simon Marchi
2018-06-26  5:10     ` Petr Tesarik
2018-06-26 13:26       ` Simon Marchi
2018-06-11 12:23 ` [PATCH v2 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
2018-06-26  2:52   ` Simon Marchi
2018-06-22  7:52 ` [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
2018-06-26  2:58   ` Simon Marchi
2018-06-26  5:01     ` Petr Tesarik
2018-06-26 15:15       ` Simon Marchi
2018-06-26 15:23         ` Petr Tesarik
2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
2018-06-27 12:14   ` [PATCH v3 3/4] Make sure that sorting does not change section order Petr Tesarik
2018-06-27 12:14   ` [PATCH v3 2/4] Make add-symbol-file's address argument optional Petr Tesarik
2018-06-27 12:14   ` [PATCH v3 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
2018-06-27 12:33   ` [PATCH v3 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
2018-06-27 12:57   ` [PATCH v3 0/4] Allow loading symbol files with an offset Simon Marchi

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).