This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 3/4] Make sure that sorting does not change section order


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]