From 729b89edabc1c67c0b6272c22a7ab9e4f4c4f957 Mon Sep 17 00:00:00 2001 From: Stephen Brennan Date: Thu, 3 Apr 2025 15:17:42 -0700 Subject: [PATCH] Fix bad relocations when module sh_addr is nonzero In the previous commit, we fixed crash's logic which determines the module load addresses in the presence of sections with nonzero sh_addr. This allowed GDB to correctly locate public variables (i.e. GLOBAL symbols) via the ELF symbol table (msymbols). However, LOCAL symbols still had incorrect addresses. The root cause for those issues is not in crash. Instead, GDB does not expect sections with nonzero sh_addr, and so it slipped up in multiple places: 1. In default_symfile_offsets(), GDB detects the nonzero sh_addr field and fails to apply the user-supplied section offsets. The result is that later, in symfile_relocate_debug_section(), relocations are applied to the DWARF info using the wrong section addresses, which results in invalid addresses for variables. Clearly, this has happened before, because crash has special-cased the "__ksymtab*" section names to avoid this condition. To resolve this, we simply drop the check for nonzero sh_addr altogether: the only ET_REL files we encounter should be kernel modules, so there's no real reason to be picky. 2. Even with that fixed, the user-supplied section addresses are clobbered by the addr_info_make_relative(), which subtracts out section offsets during its operation. To resolve this, undo the operation for ET_REL files where a section address was provided by the user (i.e. crash). With these two fixes, both local and global variables from a module section with nonzero sh_addr are correctly reported. Behavior is unchanged for modules with a zero sh_addr. Signed-off-by: Stephen Brennan --- gdb-16.2.patch | 87 +++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 37 deletions(-) diff --git a/gdb-16.2.patch b/gdb-16.2.patch index ee9bd2b..a6edba2 100644 --- a/gdb-16.2.patch +++ b/gdb-16.2.patch @@ -623,37 +623,50 @@ gdb_printf (_("Backtrace stopped: %s\n"), frame_stop_reason_string (trailing)); } ---- gdb-16.2/gdb/symfile.c.orig -+++ gdb-16.2/gdb/symfile.c -@@ -633,7 +633,26 @@ default_symfile_offsets (struct objfile *objfile, - for (cur_sec = abfd->sections; cur_sec != NULL; cur_sec = cur_sec->next) - /* We do not expect this to happen; just skip this step if the - relocatable file has a section with an assigned VMA. */ +--- gdb-16.2/gdb/symfile.c.orig 2025-04-03 14:56:36.640330084 -0700 ++++ gdb-16.2/gdb/symfile.c 2025-04-03 15:07:43.239691104 -0700 +@@ -341,8 +341,13 @@ place_section (bfd *abfd, asection *sect + return; + + /* If the user specified an offset, honor it. */ +- if (offsets[gdb_bfd_section_index (abfd, sect)] != 0) ++ if (offsets[gdb_bfd_section_index (abfd, sect)] != 0) { ++ /* addr_info_make_relative() subtracts out the section VMA. But if the user ++ * specified an offset, they have already taken this into account. Add it ++ * back in */ ++ offsets[gdb_bfd_section_index (abfd, sect)] += bfd_section_vma(sect); + return; ++ } + + /* Otherwise, let's try to find a place for the section. */ + start_addr = (lowest + align - 1) & -align; +@@ -628,16 +633,8 @@ default_symfile_offsets (struct objfile + if ((bfd_get_file_flags (objfile->obfd.get ()) & (EXEC_P | DYNAMIC)) == 0) + { + bfd *abfd = objfile->obfd.get (); +- asection *cur_sec; +- +- for (cur_sec = abfd->sections; cur_sec != NULL; cur_sec = cur_sec->next) +- /* We do not expect this to happen; just skip this step if the +- relocatable file has a section with an assigned VMA. */ - if (bfd_section_vma (cur_sec) != 0) -+ if (bfd_section_vma (cur_sec) != 0 -+ /* -+ * Kernel modules may have some non-zero VMAs, i.e., like the -+ * __ksymtab and __ksymtab_gpl sections in this example: -+ * -+ * Section Headers: -+ * [Nr] Name Type Address Offset -+ * Size EntSize Flags Link Info Align -+ * ... -+ * [ 8] __ksymtab PROGBITS 0000000000000060 0000ad90 -+ * 0000000000000010 0000000000000000 A 0 0 16 -+ * [ 9] .rela__ksymtab RELA 0000000000000000 0000ada0 -+ * 0000000000000030 0000000000000018 43 8 8 -+ * [10] __ksymtab_gpl PROGBITS 0000000000000070 0000add0 -+ * 00000000000001a0 0000000000000000 A 0 0 16 -+ * ... -+ * -+ * but they should be treated as if they are NULL. -+ */ -+ && strncmp (bfd_section_name (cur_sec), "__k", 3) != 0) - break; - - if (cur_sec == NULL) -@@ -1069,6 +1088,12 @@ symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name, +- break; ++ asection *cur_sec = NULL; + +- if (cur_sec == NULL) +- { + section_offsets &offsets = objfile->section_offsets; + + /* Pick non-overlapping offsets for sections the user did not +@@ -685,7 +682,6 @@ default_symfile_offsets (struct objfile + offsets[cur_sec->index]); + offsets[cur_sec->index] = 0; + } +- } + } + + /* Remember the bfd indexes for the .text, .data, .bss and +@@ -1069,6 +1065,12 @@ symbol_file_add_with_addrs (const gdb_bf objfile *objfile = objfile::make (abfd, current_program_space, name, flags, parent); @@ -666,7 +679,7 @@ /* We either created a new mapped symbol table, mapped an existing symbol table file which has not had initial symbol reading -@@ -1095,6 +1120,7 @@ symbol_file_add_with_addrs (const gdb_bfd_ref_ptr &abfd, const char *name, +@@ -1095,6 +1097,7 @@ symbol_file_add_with_addrs (const gdb_bf styled_string (file_name_style.style (), name)); objfile->expand_all_symtabs (); @@ -674,7 +687,7 @@ } /* Note that we only print a message if we have no symbols and have -@@ -1352,6 +1378,10 @@ show_debug_file_directory (struct ui_file *file, int from_tty, +@@ -1352,6 +1355,10 @@ show_debug_file_directory (struct ui_fil #if ! defined (DEBUG_SUBDIRECTORY) #define DEBUG_SUBDIRECTORY ".debug" #endif @@ -685,7 +698,7 @@ /* Find a separate debuginfo file for OBJFILE, using DIR as the directory where the original file resides (may not be the same as -@@ -1390,6 +1420,15 @@ find_separate_debug_file (const char *dir, +@@ -1390,6 +1397,15 @@ find_separate_debug_file (const char *di if (separate_debug_file_exists (debugfile, crc32, objfile, warnings)) return debugfile; @@ -701,7 +714,7 @@ /* Then try in the global debugfile directories. Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will -@@ -1545,6 +1584,14 @@ find_separate_debug_file_by_debuglink +@@ -1545,6 +1561,14 @@ find_separate_debug_file_by_debuglink } } @@ -716,7 +729,7 @@ return debugfile; } -@@ -2318,8 +2365,10 @@ add_symbol_file_command (const char *args, int from_tty) +@@ -2318,8 +2342,10 @@ add_symbol_file_command (const char *arg else if (section_addrs.empty ()) gdb_printf ("\n"); @@ -727,7 +740,7 @@ objf = symbol_file_add (filename.get (), add_flags, §ion_addrs, flags); -@@ -2660,6 +2709,7 @@ reread_symbols (int from_tty) +@@ -2660,6 +2686,7 @@ reread_symbols (int from_tty) objfile_name (objfile))); objfile->expand_all_symtabs (); @@ -735,7 +748,7 @@ } if (!objfile_has_symbols (objfile)) -@@ -3638,6 +3688,15 @@ bfd_byte * +@@ -3638,6 +3665,15 @@ bfd_byte * symfile_relocate_debug_section (struct objfile *objfile, asection *sectp, bfd_byte *buf) { -- 2.43.5