From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id bRhPJgZEP2cdczkAWB0awg (envelope-from ) for ; Thu, 21 Nov 2024 09:30:30 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=FU85Fd39; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 7AD771E259; Thu, 21 Nov 2024 09:30:30 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 5C21B1E1CC for ; Thu, 21 Nov 2024 09:30:28 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9F8EF3856DD4 for ; Thu, 21 Nov 2024 14:30:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9F8EF3856DD4 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=FU85Fd39 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id A0627385AC3D for ; Thu, 21 Nov 2024 14:29:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A0627385AC3D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A0627385AC3D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1732199357; cv=none; b=boI9+m1mNULz5gmvf7vKQiQqQdqjW3rIU89/fs5iqsNfNA6iz0ZwQ2ycp1HJeJeiDTzEtAu173ISL549HlQoGLjUnHMwNSvrpCeLPNT4mlflBx6EhvNNExKVGwsgF5tGrcjNuNXrAUktJCkseOT8De3D4tD4GbBsBE0lwgkKTW0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1732199357; c=relaxed/simple; bh=Qu6QlYF7QnwnHr/aLn25TWXEj1vRhf7D6MwRjp7D8wI=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=eUQC8drZTVQRVK8Q+t799ARQWclrhiJHWQrtsrHGKZXCzGme0w25oB3bmqXCTNBsC1v7L8ErMiGr4SNkzw+wsy8Kqoo7sjz3ciPtOMDEakwGB4Cyy6GWCRTXpNysCHK1LPZZ7Jtvtx4lvd559w76y+gollYpsjrh8rD7o3KKguo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A0627385AC3D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1732199357; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Po0urB3pYbLV23fc7u7E3dMraPWPEU84jIJu4F6DsVw=; b=FU85Fd39C+z9JwJRkfhytk9whvxEDPUXAL5UpoFI5l7FJ++mzuoSGuwZDSSxWER6Vrdgzj aY+7aOV3qKoplMZT44yUonrr5ZHu+NcKhMs3YHBQS1SJvvh0u+iYohPgbO0XA4OwHqVFoL m2jddc0+cKzhrhWIG3Edo3jis8P4+Ro= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-387-eDON0xPXNwKD744J5KelEg-1; Thu, 21 Nov 2024 09:29:16 -0500 X-MC-Unique: eDON0xPXNwKD744J5KelEg-1 X-Mimecast-MFC-AGG-ID: eDON0xPXNwKD744J5KelEg Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-432d8843783so6531505e9.3 for ; Thu, 21 Nov 2024 06:29:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732199354; x=1732804154; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Po0urB3pYbLV23fc7u7E3dMraPWPEU84jIJu4F6DsVw=; b=DERGJY8kwjS9SKYGep/4CqmEuJ6SP7+9fzjFsXcSD6vcKAnBbb7hkh4zMUg17Smexh 4YnnhNutyUXXdc/iFQMQIUMu8pYsk8q8O0z0+9ODF0SpQNP2ei/U7pe4gQEXZtpp6rpe DHARnAnmW+BDPBE8exth7sYq5vtZN2aVTPghc/43qe2mFV++fWgf7mlra28oZuQV2aKS mO1NJtNKdNXSI/Kj0xpCHZeLMx/sUzMidaDe+4j+MZN7cIXluy0oszuR4G/b5Sc4+iIE 7UlEJKuB5PijZvkTM9xrpF555srGaz/gJvHk7pIRLfggjDs9D6eXZUWwMuLoSEiFhb1c n6bQ== X-Gm-Message-State: AOJu0Yws4e+/pDbug18Zf5aO7irKEJgg4lqZvCND2bVqEWNruKqLU/lK r1ASihaWgZ6L70VwE1jIL4OlGpVzyWc3i4+2c5Y2ILQrbivsDJUhFe5LBLeJJy/yzoX1vXyFId4 k5J//eAskobMNg6vWtWhiuSTXGf1DjPc631KYfRvxF6gAtGho1cylQ8loQSzZAonOtcKOf8f+Lv e+ZbQ7nFih5ioQ+JIvOq9cVwDnZ4t97J4QseRh5j2xPAU= X-Gm-Gg: ASbGncs0vHaDbvKW7olmon86Bj3SrScz4QNRmsoAwwPqNHfXxSKDaxB7jS0UqVF5KnZ ija++g6P/kHO14NWBV42RwOcc5FCTJiuUx+SK2cLGb+K0mnz9CuiNi6WFxz4APkjcKXYdJZUfTn 1PSFEAnY6ek11hVo/LEZ4Wt6pYhv5E4ZPU8hjOdOWpEDdZib/BvarZJ7BuutS1LRl1BBmzV4hRD ic8oEsOtawqfWh46EHukzRwkjdbVfF6BabcMIWVZdbY04IzHAYege5D4adyBWH3TB04zJg/ImkG 1w== X-Received: by 2002:a05:600c:4586:b0:431:12a8:7f1a with SMTP id 5b1f17b1804b1-433489d498bmr65487005e9.16.1732199353844; Thu, 21 Nov 2024 06:29:13 -0800 (PST) X-Google-Smtp-Source: AGHT+IGcnRPVRrcGQHx6smauUnNv2rb0ObxdaGpEM3hLIcC3Q4543LPQ5cVAL/ZVf5XQ85PW6wxEnQ== X-Received: by 2002:a05:600c:4586:b0:431:12a8:7f1a with SMTP id 5b1f17b1804b1-433489d498bmr65486675e9.16.1732199353269; Thu, 21 Nov 2024 06:29:13 -0800 (PST) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-433b46430f1sm57529465e9.43.2024.11.21.06.29.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Nov 2024 06:29:12 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Kevin Buettner Subject: [PATCHv2] gdb/build-id: protect against weirdly short build-ids Date: Thu, 21 Nov 2024 14:29:05 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: SVgK4MghYxbr3dyUJ327yNOEnKFonCIhriG8zSTQD3g_1732199355 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org In v2: - Print a debug message when a short build-id is ignored. I did consider a warning, but I wasn't sure if that was suitable or not. In the end some debug output means we can at least diagnose why the debug lookup failed. - Rebased onto something closer to current master. --- While looking at build_id_to_bfd_suffix (in gdb/build-id.c) I realised that GDB would likely not do what we wanted if a build-id was ever a single byte. Right now, build-ids generated by the GNU linker are 32 bytes, but there's nothing that forces this to be the case, it's pretty easy to create a fake, single byte, build-id. Given that the build-id is an external input (read from the objfile), GDB should protect itself against these edge cases. The problem with build_id_to_bfd_suffix is that this function creates the path used to lookup either the debug information, or an executable, based on its build-id. So a 3-byte build-id 0xaabbcc will look in the path: `$DEBUG_FILE_DIRECTORY/.build-id/aa/bbcc.debug`. However, a single byte build-id 0xaa, will look in the file: `$DEBUG_FILE_DIRECTORY/.build-id/aa/.debug` which doesn't seem right. Worse, when looking for an objfile given a build-id GDB will look for a file called `$DEBUG_FILE_DIRECTORY/.build-id/aa/` with a trailing '/' character. I propose that, in build_id_to_bfd_suffix we just return early if the build-id is 1 byte (or less) with a return value that indicates no separate file was found. For testing I made use of the DWARF assembler. I needed to update the build-id creation proc, the existing code assumes that the build-id is a multiple of 4 bytes, so I added some additional padding to ensure that the generated note was a multiple of 4 bytes, even if the build-id was not. I added a test with a 1 byte build-id, and also for the case where the build-id has zero length. The zero length case already does what you'd expect (no debug is loaded) as the bfd library rejects the build-id when loading it from the objfile, but adding this additional test is pretty cheap. --- gdb/build-id.c | 26 ++++- gdb/testsuite/gdb.dwarf2/short-build-id.exp | 119 ++++++++++++++++++++ gdb/testsuite/lib/dwarf.exp | 13 ++- 3 files changed, 149 insertions(+), 9 deletions(-) create mode 100644 gdb/testsuite/gdb.dwarf2/short-build-id.exp diff --git a/gdb/build-id.c b/gdb/build-id.c index 9d4b005489d..43a80dd3978 100644 --- a/gdb/build-id.c +++ b/gdb/build-id.c @@ -223,7 +223,13 @@ build_id_to_debug_bfd_1 (const std::string &original_link, /* Common code for finding BFDs of a given build-id. This function works with both debuginfo files (SUFFIX == ".debug") and executable - files (SUFFIX == ""). */ + files (SUFFIX == ""). + + The build-id will be split into a single byte sub-directory, followed by + the remaining build-id bytes as the filename, i.e. we use the lookup + format: `.build-id/xx/yy....zz`. As a consequence, if BUILD_ID_LEN is + less than 2 (bytes), no results will be found as there are not enough + bytes to form the `yy....zz` part of the lookup filename. */ static gdb_bfd_ref_ptr build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id, @@ -231,6 +237,16 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id, { SEPARATE_DEBUG_FILE_SCOPED_DEBUG_ENTER_EXIT; + if (build_id_len < 2) + { + /* Zero length build-ids are ignored by bfd. */ + gdb_assert (build_id_len > 0); + separate_debug_file_debug_printf + ("Ignoring short build-id `%s' for build-id based lookup", + bin2hex (build_id, build_id_len).c_str ()); + return {}; + } + /* Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will cause "/.build-id/..." lookups. */ @@ -249,11 +265,9 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id, std::string link = debugdir.get (); link += "/.build-id/"; - if (size > 0) - { - size--; - string_appendf (link, "%02x/", (unsigned) *data++); - } + gdb_assert (size > 1); + size--; + string_appendf (link, "%02x/", (unsigned) *data++); while (size-- > 0) string_appendf (link, "%02x", (unsigned) *data++); diff --git a/gdb/testsuite/gdb.dwarf2/short-build-id.exp b/gdb/testsuite/gdb.dwarf2/short-build-id.exp new file mode 100644 index 00000000000..f40d5e4b005 --- /dev/null +++ b/gdb/testsuite/gdb.dwarf2/short-build-id.exp @@ -0,0 +1,119 @@ +# Copyright 2024 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 . + +# Create a file with an artificially short (1-byte) build-id, and +# check that GDB doesn't try to load debug information. If we do try +# then we end up loading from: `debug-directory/.build-id/xx/.debug` +# which isn't right. + +load_lib dwarf.exp + +# This test can only be run on targets which support DWARF-2 and use gas. +require dwarf2_support + +# No remote host testing either. +require {!is_remote host} + +standard_testfile main.c + +# Create an assembler file which encodes BUILDID as the build-id. Compile +# this along with the global SRCFILE to create a test executable. +# +# Split the debug information out from the newly created executable and place +# it into the debug file directory. +# +# Load the executable into GDB and check to see if the debug information was +# loaded or not. For this test we are expecting that the debug information +# was not loaded. The reason is that, with short values for BUILDID, GDB ends +# up looking for the debug information in weird locations. +proc run_test { buildid } { + set len [string length $buildid] + + set asm_file [standard_output_file "$::testfile.$len.S"] + Dwarf::assemble $asm_file { + declare_labels int_label int_label2 + + upvar buildid buildid + + build_id $buildid + + cu { label cu_start } { + compile_unit {{language @DW_LANG_C}} { + int_label2: base_type { + {name int} + {byte_size 4 sdata} + {encoding @DW_ATE_signed} + } + + constant { + {name the_int} + {type :$int_label2} + {const_value 99 data1} + } + } + } + + aranges {} cu_start { + arange {} 0 0 + } + } + + set execfile [standard_output_file $::testfile.$len] + + if { [build_executable_from_specs "failed to build" \ + $execfile {debug no-build-id} \ + $::srcfile debug \ + $asm_file {}] } { + return + } + + # Create the debug directory. + set debugdir [standard_output_file "debugdir.$len"] + set build_id_dir $debugdir/.build-id/$buildid + remote_exec host "mkdir -p $build_id_dir" + + # Split out the debug information. + if {[gdb_gnu_strip_debug $execfile no-debuglink]} { + unresolved "failed to split out debug information" + return + } + + # Move the debug information into the debug directory. We place the debug + # information into a file called just '.debug'. GDB should not check this + # file, but at one point GDB would check this file, even though this + # doesn't make much sense. + set execfile_debug ${execfile}.debug + remote_exec host "mv $execfile_debug $build_id_dir/.debug" + + # Start GDB, set the debug-file-directory, and try loading the file. + clean_restart + + gdb_test_no_output "set debug-file-directory $debugdir" \ + "set debug-file-directory" + + gdb_file_cmd $execfile + + gdb_assert { $::gdb_file_cmd_debug_info eq "nodebug" } \ + "no debug should be loaded" + + # For sanity, read something that was encoded in the debug + # information, this should fail. + gdb_test "print the_int" \ + "(?:No symbol table is loaded|No symbol \"the_int\" in current context).*" +} + +foreach_with_prefix buildid { a4 "" } { + run_test $buildid +} diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp index 1002c75dd09..f6348972968 100644 --- a/gdb/testsuite/lib/dwarf.exp +++ b/gdb/testsuite/lib/dwarf.exp @@ -3008,25 +3008,32 @@ namespace eval Dwarf { proc _note {type name hexdata} { set namelen [expr [string length $name] + 1] + set datalen [expr [string length $hexdata] / 2] # Name size. _op .4byte $namelen # Data size. - _op .4byte [expr [string length $hexdata] / 2] + _op .4byte $datalen # Type. _op .4byte $type # The name. _op .ascii [_quote $name] - # Alignment. + # Alignment (to 4-byte boundary). set align 2 set total [expr {($namelen + (1 << $align) - 1) & -(1 << $align)}] for {set i $namelen} {$i < $total} {incr i} { - _op .byte 0 + _op .byte 0 padding } # The data. foreach {a b} [split $hexdata {}] { _op .byte 0x$a$b } + # Alignment (to 4-byte boundary). + set align 2 + set total [expr {($datalen + (1 << $align) - 1) & -(1 << $align)}] + for {set i $datalen} {$i < $total} {incr i} { + _op .byte 0 padding + } } # Emit a note section holding the given build-id. base-commit: 661611b9d71d9358305332d28a8b9d7bb47a4363 -- 2.25.4