From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (vps-42846194.vps.ovh.net [IPv6:2001:41d0:801:2000::2400]) by sourceware.org (Postfix) with ESMTPS id A508638515D3 for ; Tue, 6 Sep 2022 10:29:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A508638515D3 Received: from ubuntu.lan (unknown [IPv6:2a02:390:9086::635]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id D2F8F891A2; Tue, 6 Sep 2022 10:29:14 +0000 (UTC) Date: Tue, 6 Sep 2022 10:29:08 +0000 From: Lancelot SIX To: Kevin Buettner Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] BFD error message suppression test case Message-ID: <20220906102908.qrvisybbnyg4idhp@ubuntu.lan> References: <20220903004759.2082950-1-kevinb@redhat.com> <20220903004759.2082950-3-kevinb@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220903004759.2082950-3-kevinb@redhat.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (lndn.lancelotsix.com [0.0.0.0]); Tue, 06 Sep 2022 10:29:14 +0000 (UTC) X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_SBL_CSS, SPF_HELO_NONE, 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: Tue, 06 Sep 2022 10:29:18 -0000 Hi, When applying this patch, I have: Applying: BFD error message suppression test case [...]/rebase-apply/patch:269: trailing whitespace. # in the dynamic linker/loader due to using a mangled shared object. warning: 1 line adds whitespace errors. Also, on my systems (ubuntu 22.04 and ubuntu 20.04), the test fails: (gdb) catch load bfd-errors-lib Catchpoint 1 (load) (gdb) PASS: gdb.base/bfd-errors.exp: catch load bfd-errors-lib run Starting program: .../gdb/testsuite/outputs/gdb.base/bfd-errors/bfd-errors-main .../gdb/testsuite/outputs/gdb.base/bfd-errors/bfd-errors-main: symbol lookup error: .../gdb/testsuite/outputs/gdb.base/bfd-errors/bfd-errors-main: undefined symbol: foo4 [Inferior 1 (process 19165) exited with code 0177] (gdb) FAIL: gdb.base/bfd-errors.exp: run to catchpoint (the program exited) PASS: gdb.base/bfd-errors.exp: consolidated bfd errors FAIL: gdb.base/bfd-errors.exp: print all unique bfd error I have not really looked at the errors in detail. I also have minor comments inlined in the patch. On Fri, Sep 02, 2022 at 05:47:59PM -0700, 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. > --- > gdb/testsuite/gdb.base/bfd-errors-lib.c | 44 +++++ > gdb/testsuite/gdb.base/bfd-errors-main.c | 31 ++++ > gdb/testsuite/gdb.base/bfd-errors.exp | 211 +++++++++++++++++++++++ > 3 files changed, 286 insertions(+) > create mode 100644 gdb/testsuite/gdb.base/bfd-errors-lib.c > create mode 100644 gdb/testsuite/gdb.base/bfd-errors-main.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-main.c b/gdb/testsuite/gdb.base/bfd-errors-main.c > new file mode 100644 > index 00000000000..153f29704eb > --- /dev/null > +++ b/gdb/testsuite/gdb.base/bfd-errors-main.c > @@ -0,0 +1,31 @@ > +/* 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 > + > +extern void foo1(); > +extern void foo2(); > +extern void foo3(); > +extern void foo4(); > + > +int main () > +{ > + printf ("in main\n"); > + foo1 (); > + foo2 (); > + foo3 (); > + foo4 (); > + return 0; > +} > diff --git a/gdb/testsuite/gdb.base/bfd-errors.exp b/gdb/testsuite/gdb.base/bfd-errors.exp > new file mode 100644 > index 00000000000..6d5a9428dc0 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/bfd-errors.exp > @@ -0,0 +1,211 @@ > +# 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 sets a catchpoint on the shared library load > +# operation and GDB is run to that catchpoint. On the way to running > +# to the catchpoint, the test observes and records the BFD errors that > +# were output. Finally, data collected while running to the > +# catchpoint 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 [list debug ldflags=-Wl,-Bsymbolic] > + > +# Main executable names and flags: > +set main_basename ${::gdb_test_file_name}-main > +set srcfile ${srcdir}/${subdir}/${main_basename}.c > +set binfile [standard_output_file ${main_basename}] > +set bin_flags [list debug shlib=${binfile_lib}] > + > +# Compile shared library and main executable: > +if { [gdb_compile_shlib ${srcfile_lib} ${binfile_lib} $lib_flags] != "" > + || [gdb_compile ${srcfile} ${binfile} executable $bin_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 > +} > + > +# 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 > +} > + > +# 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 > + > +# 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 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 > +gdb_load ${binfile} > +gdb_load_shlib $binfile_lib > + > +# We don't run to main because doing so will sometimes cause a SIGSEGV > +# in the dynamic linker/loader due to using a mangled shared object. > +# Instead, we'll stop at the shared library load. > +gdb_test "catch load ${lib_basename}" "Catchpoint.*load.*" > + > +gdb_run_cmd > + > +# Count number of distinct BFD error messages via 'bfd_error_count' > +# array while running to the catchpoint. > +set testname "run to catchpoint" > +gdb_test_multiple "" $testname { > + -re "(BFD:\[^\r\n\]*)\[\r\n\]+" { > + incr bfd_error_count($expect_out(1,string)) > + exp_continue > + } > + -re "Catchpoint .*${lib_basename}.*$gdb_prompt $" { > + pass $testname You could use $gdb_test_name here. This would avoid the need of the testname variable. > + } > +} > + > +# Examine counts recorded in the 'bfd_error_count' array to see if any > +# message was printed multiple times. > +set testname "consolidated bfd errors" > +set more_than_one 0 > +foreach index [array names bfd_error_count] { > + if { $bfd_error_count($index) > 1 } { > + incr more_than_one > + } > +} > +if { $more_than_one > 0 } { > + fail $testname > +} else { > + pass $testname > +} > + I guess you could use gdb_assert { $more_than_one > 0 } "consolidated bfd errors" > +# There should have been at least two distinct BFD errors printed. > +set testname "print all unique bfd errors" > +if { [array size bfd_error_count] >= 2 } { > + pass $testname > +} else { > + fail $testname > +} Similarly, I think gdb_assert would be more concise. Best, Lancelot. > + > +gdb_exit > + > +return 0 > -- > 2.37.2 >