From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 263B0382F98B for ; Fri, 20 May 2022 15:55:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 263B0382F98B Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-618-kN2FVU4kPDaI7JeHWbPg6A-1; Fri, 20 May 2022 11:55:05 -0400 X-MC-Unique: kN2FVU4kPDaI7JeHWbPg6A-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D730385A5AA for ; Fri, 20 May 2022 15:55:04 +0000 (UTC) Received: from comet.redhat.com (unknown [10.39.192.252]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2CBDC492C3B for ; Fri, 20 May 2022 15:55:04 +0000 (UTC) From: Nick Clifton To: binutils@sourceware.org Subject: Commit: objdump/readelf: Try harder to avoid following links to separate debug info files Date: Fri, 20 May 2022 16:55:00 +0100 Message-ID: <87y1ywyzaj.fsf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 May 2022 15:55:09 -0000 --=-=-= Content-Type: text/plain Hi Guys, I am applying the attached patch to change objdump's and readelf's behaviour slightly. Currently if the follow-debug-links configure option is enabled they will automatically load and follow debug links, even if the requested dumping operation(s) have nothing to do with debugging. This can cause performance slow downs if the use of debuginfod is also enabled and a debuginfod server cannot be contacted. So this patch makes two changes. Firstly the heuristic for deciding when to try loading separate debug info files is moved into separate functions. In addition these functions will only check the do_follow_links variable if the default value for that variable is zero. Ie, if it defaults to 1 then the new function will need some other reason for saying that the separate files should be loaded. The second change is to make the options that disable link following (-wN/-WN) and disable debuginfod use (-wU/-WU) *not* enable debug dumping. Ie these options can now be used to turn off these behaviours without enabling extra dumps. Cheers Nick binutils/ChangeLog 2022-05-20 Nick Clifton * dwarf.c (dwarf_select_sections_by_names): Return zero if no sections were selected. (dwarf_select_sections_by_letters): Likewise. * dwarf.h: (dwarf_select_sections_by_names): Update prototype. (dwarf_select_sections_by_letters): Update prototype. * objdump.c (might_need_separate_debug_info): New function. (dump_bfd): Call new function before attempting to load separate debug info files. (main): Do not enable dwarf section dumping for -WK or -WN. * readelf.c (parse_args): Do not enable dwarf section dumping for -wK or -wN. (might_need_separate_debug_info): New function. (process_object): Call new function before attempting to load separate debug info files. * testsuite/binutils-all/debuginfo.exp: Expect -WE and -wE debuginfod tests to pass. * testsuite/binutils-all/objdump.Wk: Add extra regexps. * testsuite/binutils-all/readelf.k: Add extra regexps. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=follow-links-less.patch diff --git a/binutils/dwarf.c b/binutils/dwarf.c index e61c63a0601..7de6f28161f 100644 --- a/binutils/dwarf.c +++ b/binutils/dwarf.c @@ -12062,7 +12062,11 @@ free_debug_memory (void) free_dwo_info (); } -void +/* Enable display of specific DWARF sections as determined by the comma + separated strings in NAMES. Returns non-zero if any displaying was + enabled. */ + +int dwarf_select_sections_by_names (const char *names) { typedef struct @@ -12115,6 +12119,7 @@ dwarf_select_sections_by_names (const char *names) }; const char *p; + int result = 0; p = names; while (*p) @@ -12129,6 +12134,7 @@ dwarf_select_sections_by_names (const char *names) && (p[len] == ',' || p[len] == '\0')) { * entry->variable = entry->val; + result |= entry->val; /* The --debug-dump=frames-interp option also enables the --debug-dump=frames option. */ @@ -12151,48 +12157,82 @@ dwarf_select_sections_by_names (const char *names) if (*p == ',') p++; } + + return result; } -void +/* Enable display of specific DWARF sections as determined by the characters + in LETTERS. Returns non-zero if any displaying was enabled. */ + +int dwarf_select_sections_by_letters (const char *letters) { - unsigned int lindex = 0; + typedef struct + { + const char letter; + int * variable; + int val; + bool cont; + } + debug_dump_letter_opts; - while (letters[lindex]) - switch (letters[lindex++]) - { - case 'A': do_debug_addr = 1; break; - case 'a': do_debug_abbrevs = 1; break; - case 'c': do_debug_cu_index = 1; break; + static const debug_dump_letter_opts letter_table [] = + { + { 'A', & do_debug_addr, 1, false}, + { 'a', & do_debug_abbrevs, 1, false }, + { 'c', & do_debug_cu_index, 1, false }, #ifdef HAVE_LIBDEBUGINFOD - case 'D': use_debuginfod = 1; break; - case 'E': use_debuginfod = 0; break; + { 'D', & use_debuginfod, 1, false }, + { 'E', & use_debuginfod, 0, false }, #endif - case 'F': do_debug_frames_interp = 1; /* Fall through. */ - case 'f': do_debug_frames = 1; break; - case 'g': do_gdb_index = 1; break; - case 'i': do_debug_info = 1; break; - case 'K': do_follow_links = 1; break; - case 'N': do_follow_links = 0; break; - case 'k': do_debug_links = 1; break; - case 'l': do_debug_lines |= FLAG_DEBUG_LINES_RAW; break; - case 'L': do_debug_lines |= FLAG_DEBUG_LINES_DECODED; break; - case 'm': do_debug_macinfo = 1; break; - case 'O': do_debug_str_offsets = 1; break; - case 'o': do_debug_loc = 1; break; - case 'p': do_debug_pubnames = 1; break; - case 'R': do_debug_ranges = 1; break; - case 'r': do_debug_aranges = 1; break; - case 's': do_debug_str = 1; break; - case 'T': do_trace_aranges = 1; break; - case 't': do_debug_pubtypes = 1; break; - case 'U': do_trace_info = 1; break; - case 'u': do_trace_abbrevs = 1; break; + { 'F', & do_debug_frames_interp, 1, true }, /* Note the fall through. */ + { 'f', & do_debug_frames, 1, false }, + { 'g', & do_gdb_index, 1, false }, + { 'i', & do_debug_info, 1, false }, + { 'K', & do_follow_links, 1, false }, + { 'k', & do_debug_links, 1, false }, + { 'L', & do_debug_lines, FLAG_DEBUG_LINES_DECODED, false }, + { 'l', & do_debug_lines, FLAG_DEBUG_LINES_RAW, false }, + { 'm', & do_debug_macinfo, 1, false }, + { 'N', & do_follow_links, 0, false }, + { 'O', & do_debug_str_offsets, 1, false }, + { 'o', & do_debug_loc, 1, false }, + { 'p', & do_debug_pubnames, 1, false }, + { 'R', & do_debug_ranges, 1, false }, + { 'r', & do_debug_aranges, 1, false }, + { 's', & do_debug_str, 1, false }, + { 'T', & do_trace_aranges, 1, false }, + { 't', & do_debug_pubtypes, 1, false }, + { 'U', & do_trace_info, 1, false }, + { 'u', & do_trace_abbrevs, 1, false }, + { 0, NULL, 0, false } + }; - default: - warn (_("Unrecognized debug option '%s'\n"), letters); - break; - } + int result = 0; + + while (* letters) + { + const debug_dump_letter_opts * entry; + + for (entry = letter_table; entry->letter; entry++) + { + if (entry->letter == * letters) + { + * entry->variable |= entry->val; + result |= entry->val; + + if (! entry->cont) + break; + } + } + + if (entry->letter == 0) + warn (_("Unrecognized debug letter option '%c'\n"), * letters); + + letters ++; + } + + return result; } void diff --git a/binutils/dwarf.h b/binutils/dwarf.h index ccce2461c81..040e674c6ce 100644 --- a/binutils/dwarf.h +++ b/binutils/dwarf.h @@ -250,8 +250,8 @@ extern void *open_debug_file (const char *); extern void free_debug_memory (void); -extern void dwarf_select_sections_by_names (const char *); -extern void dwarf_select_sections_by_letters (const char *); +extern int dwarf_select_sections_by_names (const char *); +extern int dwarf_select_sections_by_letters (const char *); extern void dwarf_select_sections_all (void); extern unsigned int * find_cu_tu_set (void *, unsigned int); diff --git a/binutils/objdump.c b/binutils/objdump.c index 060a136efa4..2edaa36b622 100644 --- a/binutils/objdump.c +++ b/binutils/objdump.c @@ -5188,6 +5188,26 @@ sign_extend_address (bfd *abfd ATTRIBUTE_UNUSED, return (((vma & ((mask << 1) - 1)) ^ mask) - mask); } +static bool +might_need_separate_debug_info (bool is_mainfile) +{ + /* We do not follow links from debug info files. */ + if (! is_mainfile) + return false; + + /* Since do_follow_links might be enabled by default, only treat it as an + indication that separate files should be loaded if setting it was a + deliberate user action. */ + if (DEFAULT_FOR_FOLLOW_LINKS == 0 && do_follow_links) + return true; + + if (process_links || dump_symtab || dump_debugging + || dump_dwarf_section_info) + return true; + + return false; +} + /* Dump selected contents of ABFD. */ static void @@ -5202,16 +5222,8 @@ dump_bfd (bfd *abfd, bool is_mainfile) else byte_get = NULL; - /* Load any separate debug information files. - We do this now and without checking do_follow_links because separate - debug info files may contain symbol tables that we will need when - displaying information about the main file. Any memory allocated by - load_separate_debug_files will be released when we call - free_debug_memory below. - - The test on is_mainfile is there because the chain of separate debug - info files is a global variable shared by all invocations of dump_bfd. */ - if (byte_get != NULL && is_mainfile) + /* Load any separate debug information files. */ + if (byte_get != NULL && might_need_separate_debug_info (is_mainfile)) { load_separate_debug_files (abfd, bfd_get_filename (abfd)); @@ -5783,20 +5795,30 @@ main (int argc, char **argv) do_follow_links = true; break; case 'W': - dump_dwarf_section_info = true; seenflag = true; if (optarg) - dwarf_select_sections_by_letters (optarg); + { + if (dwarf_select_sections_by_letters (optarg)) + dump_dwarf_section_info = true; + } else - dwarf_select_sections_all (); + { + dump_dwarf_section_info = true; + dwarf_select_sections_all (); + } break; case OPTION_DWARF: - dump_dwarf_section_info = true; seenflag = true; if (optarg) - dwarf_select_sections_by_names (optarg); + { + if (dwarf_select_sections_by_names (optarg)) + dump_dwarf_section_info = true; + } else - dwarf_select_sections_all (); + { + dwarf_select_sections_all (); + dump_dwarf_section_info = true; + } break; case OPTION_DWARF_DEPTH: { diff --git a/binutils/readelf.c b/binutils/readelf.c index d45e0920788..c35bfc12366 100644 --- a/binutils/readelf.c +++ b/binutils/readelf.c @@ -5526,31 +5526,39 @@ parse_args (struct dump_data *dumpdata, int argc, char ** argv) decompress_dumps = true; break; case 'w': - do_dump = true; - dump_any_debugging = true; if (optarg == NULL) { do_debugging = true; + do_dump = true; + dump_any_debugging = true; dwarf_select_sections_all (); } else { do_debugging = false; - dwarf_select_sections_by_letters (optarg); + if (dwarf_select_sections_by_letters (optarg)) + { + do_dump = true; + dump_any_debugging = true; + } } break; case OPTION_DEBUG_DUMP: - do_dump = true; - dump_any_debugging = true; if (optarg == NULL) { + do_dump = true; do_debugging = true; + dump_any_debugging = true; dwarf_select_sections_all (); } else { do_debugging = false; - dwarf_select_sections_by_names (optarg); + if (dwarf_select_sections_by_names (optarg)) + { + do_dump = true; + dump_any_debugging = true; + } } break; case OPTION_DWARF_DEPTH: @@ -22252,6 +22260,26 @@ initialise_dump_sects (Filedata * filedata) } } +static bool +might_need_separate_debug_info (Filedata * filedata) +{ + /* Debuginfo files do not need further separate file loading. */ + if (filedata->file_header.e_shstrndx == SHN_UNDEF) + return false; + + /* Since do_follow_links might be enabled by default, only treat it as an + indication that separate files should be loaded if setting it was a + deliberate user action. */ + if (DEFAULT_FOR_FOLLOW_LINKS == 0 && do_follow_links) + return true; + + if (process_links || do_syms || do_unwind + || dump_any_debugging || do_dump || do_debugging) + return true; + + return false; +} + /* Process one ELF object file according to the command line options. This file may actually be stored in an archive. The file is positioned at the start of the ELF object. Returns TRUE if no @@ -22335,7 +22363,7 @@ process_object (Filedata * filedata) if (! process_version_sections (filedata)) res = false; - if (filedata->file_header.e_shstrndx != SHN_UNDEF) + if (might_need_separate_debug_info (filedata)) have_separate_files = load_separate_debug_files (filedata, filedata->file_name); else have_separate_files = false; diff --git a/binutils/testsuite/binutils-all/debuginfod.exp b/binutils/testsuite/binutils-all/debuginfod.exp index f5935ad0348..d0e6db23f34 100644 --- a/binutils/testsuite/binutils-all/debuginfod.exp +++ b/binutils/testsuite/binutils-all/debuginfod.exp @@ -189,7 +189,7 @@ if { [regexp ".*DEBUGINFOD.*" $conf_objdump] } { test_fetch_debugaltlink $OBJDUMP "-Wk" set test "disabling debuginfod access" - setup_xfail *-*-* + # setup_xfail *-*-* test_fetch_debuglink $OBJDUMP "-W -WE" set test "debuginfod" @@ -202,7 +202,7 @@ if { [regexp ".*DEBUGINFOD.*" $conf_readelf] } { test_fetch_debugaltlink $READELF "-wk" set test "disabling debuginfod access" - setup_xfail *-*-* + # setup_xfail *-*-* test_fetch_debuglink $READELF "-w -wE" set test "debuginfod" diff --git a/binutils/testsuite/binutils-all/objdump.Wk b/binutils/testsuite/binutils-all/objdump.Wk index 35472328fc2..b0d7b1a91f9 100644 --- a/binutils/testsuite/binutils-all/objdump.Wk +++ b/binutils/testsuite/binutils-all/objdump.Wk @@ -1,8 +1,9 @@ +#... tmpdir/debuglink\.o: file format .* -Contents of the \.gnu_debuglink section: +Contents of the \.gnu_debuglink section.* Separate debug info file: this_is_a_debuglink\.debug CRC value: 0x12345678 -Contents of the \.gnu_debugaltlink section: +Contents of the \.gnu_debugaltlink section.* Separate debug info file: linkdebug\.debug Build-ID \(0x18 bytes\): 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 01 23 45 67 89 ab cd ef diff --git a/binutils/testsuite/binutils-all/readelf.k b/binutils/testsuite/binutils-all/readelf.k index fe033421187..c2d03a0e477 100644 --- a/binutils/testsuite/binutils-all/readelf.k +++ b/binutils/testsuite/binutils-all/readelf.k @@ -1,7 +1,8 @@ -Contents of the \.gnu_debuglink section: +#... +Contents of the \.gnu_debuglink section.* Separate debug info file: this_is_a_debuglink\.debug CRC value: 0x12345678 -Contents of the \.gnu_debugaltlink section: +Contents of the \.gnu_debugaltlink section.* Separate debug info file: linkdebug\.debug Build-ID \(0x18 bytes\): 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee ff 01 23 45 67 89 ab cd ef --=-=-=--