From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 792F03858D28 for ; Wed, 7 Sep 2022 19:05:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 792F03858D28 Received: from [172.16.0.64] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 25B391E13B; Wed, 7 Sep 2022 15:05:16 -0400 (EDT) Message-ID: <29c14cf6-9e3f-9a47-8ab4-afdb7f7c27ab@simark.ca> Date: Wed, 7 Sep 2022 15:05:15 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Subject: Re: [PATCH v2 2/2] BFD error message suppression test case Content-Language: fr To: Kevin Buettner , gdb-patches@sourceware.org Cc: lsix@lancelotsix.com References: <20220907173050.8739-1-kevinb@redhat.com> <20220907173050.8739-3-kevinb@redhat.com> From: Simon Marchi In-Reply-To: <20220907173050.8739-3-kevinb@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, KAM_TK, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Sep 2022 19:05:19 -0000 On 9/7/22 13:30, Kevin Buettner via Gdb-patches wrote: > This commit adds a GDB test case which tests GDB's BFD error handler > hook for suppressing output of all but the first identical messages. > > See the comment at the beginning of bfd-errors.exp for details about > this new test. > > I've tested this test for both 32- and 64-bit ELF files and also > on both little endian and big endian machines. It also works for > both native and remote targets. The only major restriction is that > it only works for ELF targets. I admire your committment at writing such a test case! LGTM, just some nits below. > --- > gdb/testsuite/gdb.base/bfd-errors-lib.c | 44 ++++++ > gdb/testsuite/gdb.base/bfd-errors.exp | 187 ++++++++++++++++++++++++ > 2 files changed, 231 insertions(+) > create mode 100644 gdb/testsuite/gdb.base/bfd-errors-lib.c > create mode 100644 gdb/testsuite/gdb.base/bfd-errors.exp > > diff --git a/gdb/testsuite/gdb.base/bfd-errors-lib.c b/gdb/testsuite/gdb.base/bfd-errors-lib.c > new file mode 100644 > index 00000000000..2b582d14c58 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/bfd-errors-lib.c > @@ -0,0 +1,44 @@ > +/* Copyright 2022 Free Software Foundation, Inc. > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . > +*/ > + > +#include > + > +void > +foo1 () > +{ > + printf ("foo1 in lib\n"); > + return; > +} > + > +void > +foo2() > +{ > + printf ("foo2 in lib\n"); > + return; > +} > + > +void > +foo3() > +{ > + printf ("foo3 in lib\n"); > + return; > +} > + > +void > +foo4() > +{ > + printf ("foo4 in lib\n"); > + return; > +} > diff --git a/gdb/testsuite/gdb.base/bfd-errors.exp b/gdb/testsuite/gdb.base/bfd-errors.exp > new file mode 100644 > index 00000000000..141ff6a0765 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/bfd-errors.exp > @@ -0,0 +1,187 @@ > +# Copyright 2022 Free Software Foundation, Inc. > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# Tools which use the BFD library will output error messages of the > +# form "BFD: some messsage" when a problem with the file upon which it > +# operating is found. E.g. an actual message (modulo some shortening > +# of the pathname) from this test is: > +# > +# BFD: bfd-errors-lib.so: invalid string offset 1154 >= 154 for section `.dynstr' > +# > +# For some problems with executable files or libraries, BFD will > +# attempt to output many identical messages. Code has been added to > +# GDB to suppress messages which are identical to earlier messages > +# that have already been printed. > +# > +# This test makes sure that (all but the first) identical BFD messages > +# are suppresssed and also that differing messages are output at least > +# once. > +# > +# To accomplish this, a shared object with at least four symbols is > +# created. The .dynsym section is extracted and offsets which should > +# refer to strings in the .dynstr section are changed to be > +# larger than the size of the .dynstr section. Only two (different) > +# offsets are used; thus BFD will attempt to output at least two pairs > +# of identical messages. (And it would do this too if not intercepted > +# by the hook placed by GDB.) After modifying the extracted section, > +# the mangled section is placed back into the shared object. > +# > +# This test then loads the shared library's symbol table (and other > +# debug info) using the 'add-symbol-file' command. While doing this, > +# the test observes and records the BFD errors that were output. > +# Finally, data collected while adding the shared library symbols are > +# examined to make sure that identical messages were suppressed while > +# also making sure that at least two messages have been printed. > + > +# This test can't be run on targets lacking shared library support > +# or for non-ELF targets. > +if { [skip_shlib_tests] || ![is_elf_target] } { > + return 0 > +} > + > +# Library file names and flags: > +set lib_basename ${::gdb_test_file_name}-lib > +set srcfile_lib ${srcdir}/${subdir}/${lib_basename}.c > +set binfile_lib [standard_output_file ${lib_basename}.so] > +set lib_flags debug > + > +# Compile shared library: > +if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $lib_flags] != "" } { > + untested "failed to compile" > + return -1 > +} > + > +# Open the shared library and determine some basic facts. The key > +# things that we need to learn are 1) whether the solib is 32-bit or > +# 64-bit ELF file, and 2) the endianness. > +set solib_fp [open ${binfile_lib} r] > +fconfigure $solib_fp -translation binary > + > +# Read and check EI_MAG to verify that it's really an ELF file. > +set data [read $solib_fp 4] > +if { ![string equal $data "\x7fELF"] } { > + untested "shared library is not an ELF file" > + return -1 > +} Should you close $solib_fp in this error path? > + > +# Read EI_CLASS for ELF32 versus ELF64. > +set data [read $solib_fp 1] > +set is_elf64 [string equal $data "\x02"] > + > +# Read EI_DATA to determine data encoding (byte order). > +set data [read $solib_fp 1] > +set is_big_endian [string equal $data "\x02"] > + > +close $solib_fp > + > +set objcopy_program [gdb_find_objcopy] > + > +# Extract the .dynsym and .dynstr section from the shared object. > +if {[run_on_host \ > + "objcopy dump-section" \ > + ${objcopy_program} \ > + "--dump-section .dynsym=${binfile_lib}.dynsym --dump-section .dynstr=${binfile_lib}.dynstr ${binfile_lib}"]} { > + untested "failed objcopy dump-section" > + return -1 > +} Properly supporting remote-host testing is a pain (mostly because nobody seems to have such a setup today), but I thought I'd mention it anyway: here, you use run_on_host, but the "open" above runs on the build. So one or the other will not work if the build machine != the host machine. > + > +# Determine length of .dynstr. We'll use the length for creating invalid > +# offsets into .dynstr. > +set dynstr_fp [open ${binfile_lib}.dynstr r] > +fconfigure $dynstr_fp -translation binary > +set dynstr_len [string length [read $dynstr_fp]] > +close $dynstr_fp Would "file size" work? https://www.tcl.tk/man/tcl8.4/TclCmd/file.html#M34 > + > +# Open the file containing .dynsym and determine its length. In this > +# case, we want to know the length in order to compute the total number > +# of symbols that it contains. We also leave the file open for a > +# while so that we can write invalid offsets to it. > +set dynsym_fp [open ${binfile_lib}.dynsym r+] > +fconfigure $dynsym_fp -translation binary > +set dynsym_len [string length [read $dynsym_fp]] > + > +# SZ is the size of the Elf32_Sym / Elf64_Sym struct. OFF is the > +# offset into the file. CNT is one greater than the number of symbols > +# left to mangle. Note that, in the loop below, the first symbol is > +# skipped. This is intentional since the first symbol is defined by > +# the ELF specification to be the undefined symbol. > +set off 0 > +if { $is_elf64 } { > + set sz 24 > +} else { > + set sz 16 > +} > +set cnt [expr $dynsym_len / $sz] > + > +# Create 32-bit patterns (bad offsets) to write into the st_name area. > +if { $is_big_endian } { > + set pat(0) [binary format I [expr $dynstr_len + 1000]] > + set pat(1) [binary format I [expr $dynstr_len + 2000]] > +} else { > + set pat(0) [binary format i [expr $dynstr_len + 1000]] > + set pat(1) [binary format i [expr $dynstr_len + 2000]] > +} > + > +# Mangle st_name for the symbols following the first (STN_UNDEF) entry. > +while { [incr cnt -1] > 0 } { > + seek $dynsym_fp [incr off $sz] > + puts $dynsym_fp $pat([expr $cnt % 2]) > +} > + > +close $dynsym_fp > + > +# Replace .dynsym section in shared object with the mangled version. > +if {[run_on_host \ > + "objcopy update-section" \ > + ${objcopy_program} \ > + "--update-section .dynsym=${binfile_lib}.dynsym ${binfile_lib}"]} { > + untested "failed objcopy update-section" > + return -1 > +} > + > +gdb_start > +gdb_reinitialize_dir $srcdir/$subdir Use clean_restart. Simon