From patchwork Mon May 20 13:08:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 90481 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 50CB4385840D for ; Mon, 20 May 2024 13:09:06 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id C8BDA3858D28 for ; Mon, 20 May 2024 13:08:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C8BDA3858D28 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 C8BDA3858D28 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=1716210518; cv=none; b=vUxfycfLxDj5RPddunwLwStGAXOjdFDDdMuM/vkySUq9wGR4jMhOcKIaV3ggqtrY/w/hN97bauNAotsy79QiVJHilt/YFD9mwg49k58FtN5mhADlW91JvLpUU1OlRKOSu0b+aEZ4jZ2M7/wmV5awherKhTE5iYOxCBHjlxU0WRo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716210518; c=relaxed/simple; bh=OLvsy0bfR7ai5V0ICobUeQWsSE1C4YKY4zOdXeNpPYk=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=TyT63/RqKcZsBH0UOdjSNmXr7PtWVOdfXoCgR380MBZzuvhwC6cHarOj/34zu6sWn+TIBLOMHHS9K5qybg7Qi92aEZUiTkreC3+i5lZ/AW2ojpqXwfaiM8rdhviYZ4WqXBTnz7QFDcDKVOIeR+gKsKjZNlxBXCNxSlsNpCAobTo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1716210515; 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=JuvvgCd248q4//0C36FSq7CNc3XC5yG//18/FHVZlrU=; b=N/jPlqdvLGPAGDWhmf4JsviE6x9WC3QcJdcVgTT3UcGnItrnumJi1xXIP8sXCkEoEjPDtO GYxQqmRyvPvNwEM5z+R7Wz6UuXgCwY9eYeX1J8pxyWYNBRZFM2eMM3ULMw6Xa4m4EBU++C rZTUtr7Prwgs1yQX3wMlELe5VayXvcg= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-655-5VqvuhNvO3OAXmNU_oLQ-g-1; Mon, 20 May 2024 09:08:34 -0400 X-MC-Unique: 5VqvuhNvO3OAXmNU_oLQ-g-1 Received: by mail-ej1-f71.google.com with SMTP id a640c23a62f3a-a599dbd2b6aso710951066b.2 for ; Mon, 20 May 2024 06:08:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716210512; x=1716815312; 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=JuvvgCd248q4//0C36FSq7CNc3XC5yG//18/FHVZlrU=; b=pT6zQ9MKiW4fEdKS7vEX0AZyP1tt41N/nyFgzwvBAZ20XSO+HJB1haieMfVqiAdgv7 eFMy+S10VIsmWwXlzZcnObydd8sg2MMHlBAnAkxcChPTt5z+K0Oh2Q0vQn68poqP6f2y SCZHn87SwxOyg8+fPkBTy7xI2ielcERV3L6CCdmfRZnu+HDofhfyRjfe442/gbs92TtB i9VtL9rhiRdPhsaSA37vbQKFKfOXqmGunkTP5Fu+8CJGsdmRPuoCpBbzCJ4r5AY6yRUV lIf45sNUtCnH11yMm3bdj0tfewwnCULvcUN4F1ZhQUXiemN4X5KtrfGhzQJHJAzP1NA1 f1PA== X-Gm-Message-State: AOJu0YzZ4LAXQ5TvSRc+zteDQWbvTOf58ylwKwfB4GhVWEEuMxsOWb6Y iHoYYjiZg7hhOW/1ttIkRHuij65Yh9ncQ2bZiPZyi9xmCDHUzGq2z6pv1FOcEZqt00g1SbfKnAb oc3JiOsP3a/w4pR3N1p8ZiGSW5sk8ymEGBr/8qGv424sfGL08W5d5mOzlpILn7oCpnmUWBKW6fA vWL7IKhGEaCqAr+5m8JQKXqE+Navw+J8JiWnRamuZyJeE= X-Received: by 2002:a17:906:a08:b0:a59:c3e2:712f with SMTP id a640c23a62f3a-a5a2d536941mr1927972866b.9.1716210511856; Mon, 20 May 2024 06:08:31 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGI9/OQbEzlQbTZ4eCf3H3jJnpa+pofn34kpZkjY2jLbo9I3iiWZBqw9xay8IqaXHal7ebxyg== X-Received: by 2002:a17:906:a08:b0:a59:c3e2:712f with SMTP id a640c23a62f3a-a5a2d536941mr1927969566b.9.1716210510971; Mon, 20 May 2024 06:08:30 -0700 (PDT) Received: from localhost ([31.111.84.240]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a5a17891fefsm1460665366b.62.2024.05.20.06.08.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 May 2024 06:08:30 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 1/4] gdb/corefile: remove unavailable target sections Date: Mon, 20 May 2024 14:08:24 +0100 Message-Id: <7de11f50f98cda2284195ff997f9c7462946571c.1716210406.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 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_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org When GDB opens a core file the bfd library processes the core file and creates sections within the bfd object to represent each of the segments within the core file. GDB then creates two target_section lists, m_core_section_table and m_core_file_mappings. These, as well as m_core_unavailable_mappings, are used by GDB to implement core_target::xfer_partial, that is, to allow reading from inferior memory. The m_core_section_table list represents sections within the core file itself. The sections in this list can be split into two groups based on whether the section has the SEC_HAS_CONTENTS flag set or not. Sections that have SEC_HAS_CONTENTS have their contents copied into the core file while sections that do not have the SEC_HAS_CONTENTS flag will not have their contents copied into the core file when it is created. The m_core_file_mappings list is created when GDB parses the mapped file list in the core file. Every mapped file will also be covered by entries in the m_core_section_table list, however, when parts of a file are mapped read-only then the corresponding entry in m_core_section_table will not have the SEC_HAS_CONTENTS flag set. If parts of a file are mapped read/write then the corresponding m_core_section_table entry will have the SEC_HAS_CONTENTS flag, and the corresponding (possibly modified contents) will be copied into the core file. However, GDB only creates entries in m_core_file_mappings if it is able to find the correct on-disk file to open. If the corresponding file can't be found then an entry is added to m_core_unavailable_mappings. An attempt to read from a region covered by this list will be redirected to the lower file target in the hope that a read-only section from the executable (supplied by the user) might satisfy the read. And so, in core_target::xfer_partial, the order in which the various lists are checked to satisfy a read are: 1. Check m_core_section_table for sections with SEC_HAS_CONTENTS flag, this reads content from the core file bfd directly, 2. Check m_core_file_mappings, this will read content from mapped files assuming that GDB was able to find the file on-disk, 3. Check m_core_unavailable_mappings and redirect to the file target, this allows the user supplied executable to satisfy some reads, 4. Check m_core_section_table for sections without SEC_HAS_CONTENTS flag, this returns zero for these regions. If a file is recorded as mapped in the core file, but the file was mapped read only, then the matching entry in m_core_section_table will not have the SEC_HAS_CONTENTS flag set. If GDB can find the file on disk then an entry is added to m_core_file_mappings, and reads from the region covered by the file will be satisfied by #2 in the list above. However, if GDB can't find the file then an entry is added to the m_core_unavailable_mappings list. An attempt will be made to satisfy the read at stage #3 in the list above, however, if this file is not the executable, then stage #3 will not satisfy the read. We then fall through to stage #4, the section does not have the SEC_HAS_CONTENTS flag, and so we end up returning zero. In this case, I think this is the wrong thing to do. If the core file tells us that a file is mapped in, but GDB can't find the file on disk, then we should instead return a failure when trying to read from the region covered by the file, anything else is just misleading. And so, I propose that in core_target::core_target() when the core file is initially opened and m_core_section_table is setup, we should remove entries that correspond to m_core_unavailable_mappings entries. This means that an attempt to read from a mapped file, when GDB is not able to find the on-disk file, and the contents of the mapped file were not copied into the core file, then the read will fail. This feels like the only sensible option. I've extended an existing test to exercise this case. The test maps a single file twice, once in read-only mode, and once in read-write mode. We then remove the on-disk file and load the core file in GDB. The mapping which was read-write is still accessible (the content of this region is found in the core file bfd object), while the read-only mapping is no longer accessible. --- gdb/corelow.c | 58 +++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/corefile.exp | 39 +++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/gdb/corelow.c b/gdb/corelow.c index 462a05591c1..4a489d95706 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -166,6 +166,11 @@ class core_target final : public process_stratum_target ULONGEST len, ULONGEST *xfered_len); + /* Remove from m_core_section_table any entries that are covered by an + unavailable mapping. This will prevent xfer_partial from returning + zero for unavailable mappings. */ + void filter_unavailable_mappings_from_section_table (); + /* FIXME: kettenis/20031023: Eventually this field should disappear. */ struct gdbarch *m_core_gdbarch = NULL; @@ -198,6 +203,59 @@ core_target::core_target () m_core_section_table = build_section_table (current_program_space->core_bfd ()); build_file_mappings (); + + if (!m_core_unavailable_mappings.empty ()) + filter_unavailable_mappings_from_section_table (); +} + +/* See comment in class declaration. */ + +void +core_target::filter_unavailable_mappings_from_section_table () +{ + auto filter_cb = [&] (const target_section &tsect) -> bool + { + /* If TSECT has contents then this means that there is content to be + read within the core file bfd itself. If TSECT does not have + content then we are relying on reading the content from some other + file that was mapped into the inferior at the time the core file was + created, but if GDB couldn't find that file then we are not able to + read the content and TSECT should be discarded. */ + if ((tsect.the_bfd_section->flags & SEC_HAS_CONTENTS) != 0) + return false; + + /* Find the first entry in m_core_unavailable_mappings whose start + address is not less than or equal to the start of this target + section. The previous unavailable mapping might cover TSECT. */ + auto it = std::lower_bound (m_core_unavailable_mappings.begin (), + m_core_unavailable_mappings.end (), + tsect.addr, + [] (const mem_range &mr, + const CORE_ADDR &addr) + { + return mr.start <= addr; + }); + + /* If there is a previous unavailable mapping, then that mapping's + start address is less than, or equal to the start of TSECT. The + unavailable mapping might cover TSECT. If it does then we will + delete TSECT. */ + if (it != m_core_unavailable_mappings.begin ()) + { + --it; + + if (it->start <= tsect.addr + && (it->start + it->length) >= tsect.endaddr) + return true; + } + + return false; + }; + + auto end = std::remove_if (m_core_section_table.begin (), + m_core_section_table.end (), + filter_cb); + m_core_section_table.erase (end, m_core_section_table.end ()); } /* Construct the table for file-backed mappings if they exist. diff --git a/gdb/testsuite/gdb.base/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp index 28b723e1f85..ee152e629ab 100644 --- a/gdb/testsuite/gdb.base/corefile.exp +++ b/gdb/testsuite/gdb.base/corefile.exp @@ -199,6 +199,45 @@ gdb_test "up" "#\[0-9\]* *(\[0-9xa-fH'\]* in)? .* \\(.*\\).*" "up (reinit)" gdb_test "core" "No core file now." +# Temporarily move coremmap.data out of the way and reload the core +# file. We should still be able to read buf2 as the contents of this +# are written into the core file. In contrast buf2ro should no longer +# be readable as the contents of this region are not within the core +# file, GDB relies on reading this from the coremmap.data file, which +# can no longer be found. +set coremmap_data_filename \ + [standard_output_file coredir.[getpid]/coremmap.data] +set coremmap_data_backup_filename \ + [standard_output_file coredir.[getpid]/coremmap.data.backup] +remote_exec host "mv ${coremmap_data_filename} \ + ${coremmap_data_backup_filename}" + +clean_restart $binfile + +# Load the core file and check we get a warning about the +# coremmap.data file being missing. +gdb_test_multiple "core-file $corefile" "warn about coremmap.data missing" { + -re -wrap "warning: Can't open file \[^\r\n\]+/coremmap.data during file-backed mapping note processing\r\n.*" { + pass $gdb_test_name + } +} + +# This xfail was just copied from earlier in the script where we also +# read from buf2. +setup_xfail "*-*-sunos*" "*-*-aix*" +gdb_test "x/8bd buf2" \ + ".*:.*0.*1.*2.*3.*4.*5.*6.*7.*" \ + "accessing mmapped data in core file with coremmap.data removed" + +gdb_test "x/8bd buf2ro" \ + "$hex\[^:\]*:\\s+Cannot access memory at address $hex" \ + "accessing read-only mmapped data in core file with coremmap.data removed" + +# Restore the coremmap.data file so later tests don't give warnings +# when the core file is reloaded. +remote_exec host "mv ${coremmap_data_backup_filename} \ + ${coremmap_data_filename}" + # Test that we can unload the core with the "detach" command. proc_with_prefix corefile_detach {} { From patchwork Mon May 20 13:08:25 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 90482 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 793493858C33 for ; Mon, 20 May 2024 13:09:38 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id CCA813858CDA for ; Mon, 20 May 2024 13:08:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CCA813858CDA 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 CCA813858CDA 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=1716210524; cv=none; b=BKOxidB7ZTXi52Yx0HArk2XGFkc15OqrhryTmPsrBJoSrWL81+5tUomDrUmN57nmgya5g458mcvml/96vOsj+qIaiYTuHwnUoULo3ffe+ERxr2iaytqMPS2v6YVWYW1AJj2jI2sVO3tmdCF1loj0tG9yunAvkTqp8QY5D1sB9hs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716210524; c=relaxed/simple; bh=dRng07sRL0R9wLl85G3jE3oSlYeMkJRQs2V67z1Mukw=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=pHw/ndo4ayPfeTbjwL80K5aKHoInmrLWgtKMAkk6q3vQbAResfavalc7tarGZ/0BFRmxgS+jp3lpVyW49YAqKTRRDllUiIpaDwTnmW5IeSqiLNgNGti1B2NlrcMs0h1aOh1PvqKKfs5+kCjmT7mwl5407zqOEDGWdUNx1EPMb4E= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1716210517; 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=EuHQVKGD8EDAEeEClmECZR1zKAN4P3DMtDkojRkspxg=; b=eY8PLip2ZxDR8OqAUbIHhih3JuyOz7T44iLwL8kvUAh/QGc5JDP+bSQ1xuB33MzUTXjEfP a5q7p4x1tBy8gQEBOEnGcurG1gFQ6b6U5SYKKH85VbDZlMxJ5BkJFPilQd58nHThMCmfKj mimC432GgJ9xwgyPSQecRolHh+FHdpU= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-635-X5hvjjJrMU2m4GYeAkra2A-1; Mon, 20 May 2024 09:08:35 -0400 X-MC-Unique: X5hvjjJrMU2m4GYeAkra2A-1 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-a59a0014904so674779766b.2 for ; Mon, 20 May 2024 06:08:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716210514; x=1716815314; 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=EuHQVKGD8EDAEeEClmECZR1zKAN4P3DMtDkojRkspxg=; b=cbsXab1KBUxxoSTI3XenucmvKB+RGXMewTSW/5MoOEi34GqjulN9MB56c8ap2uFyOn yxK+dkcGHjr6g0TcHqGX3NXvd6rsYSuwgKNPDh/pyZlS/cStIrz6JmSGGOCCOB7wvXLu X7UExqmK5ZLutYYof9evYA8Ak6XMbMA0WwvlvgpdEj0wAARXxI9dsDYBU0r1KjCXbHuo ORe4fVS9DIKU2a1QtZxtLOIMdypqEugkfr57876vhv6QZpv5Fwfx/VLcbkHjyOu/2ZuA Oj4MI2k8QHOwPlXoxCwKTU/AeKZI3fIc5HHyteqRG5doI7FbOcFSfDdaXcXDYBUoUlsg KHew== X-Gm-Message-State: AOJu0YylPo5toeBGUJ6MLgi93UCmeVSysg4IM5j3s6ntFRH1/TaDqP6R uWiJEbEpUgeUx7iGtzfh8p56mDGe2xlRfjxnnKLT82Dc2gqXMTvhnt7aPHCRWFZLDBstHn+aEbn NErR3+E8SPDsiaB4WMaxoze7SNbiYuK9+JdyEj8V4xBVEB54YaieCs7k31tf6+1PtZ4WOAoU6lm hxVSn2CmQq7DMFOj8WsvEzZQbn8+yV4nQ8gjA9qh6B20U= X-Received: by 2002:a17:907:3f9f:b0:a59:cd46:fe89 with SMTP id a640c23a62f3a-a5a2d665690mr2256562466b.59.1716210513511; Mon, 20 May 2024 06:08:33 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGexvZZTpTgxq53RthkS4kqEZnyH1qmMpmFMTML78WwTzAHtMgq8cdEzOj9AIEPlSw7kmz3Yg== X-Received: by 2002:a17:907:3f9f:b0:a59:cd46:fe89 with SMTP id a640c23a62f3a-a5a2d665690mr2256558066b.59.1716210512335; Mon, 20 May 2024 06:08:32 -0700 (PDT) Received: from localhost ([31.111.84.240]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a5a1781cf60sm1476394066b.14.2024.05.20.06.08.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 May 2024 06:08:31 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 2/4] gdb/corefile: improve file backed mapping handling Date: Mon, 20 May 2024 14:08:25 +0100 Message-Id: <8f5d366855fe1514a4b501b26ec6789b1d72b3de.1716210406.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org This commit improves how GDB handles file backed mappings within a core file, specifically, this is a restructuring of the function core_target::build_file_mapping. The primary motivation for this commit was to put in place the infrastructure to support the next commit in this series, but this commit does itself make some improvements. Currently in core_target::build_file_mapping we use gdbarch_read_core_file_mappings to iterate over the mapped regions within a core file. For each region a callback is invoked which is passed details of the mapping; the file the mapping is from, the offset into the file, and the address range at which the mapping exists. We are also passed the build-id for the mapped file in some cases. We are only told the build-id for the mapped region which actually contains the ELF header of the mapped file. Other regions of the same mapped ELF will not have the build-id passed to the callback. Within core_target::build_file_mapping, in the per-region callback, we try to find the mapped file based on its filename. If the file can't be found, and if we have a build-id then we'll ask debuginfod to download the file. However we find the file, we cache the opened bfd object, which is good. Subsequent mappings from the same file will not have a build-id set, but by that point we already have a cached open bfd object, so the lack of build-id is irrelevant. The problem with the above is that if we find a matching file based on the filename, then we accept that file, even if we have a build-id, and the build-id doesn't match. Currently, the mapped region processing is done in a single pass, we call gdbarch_read_core_file_mappings, and for each mapping, as we see it, we create the data structures needed to represent that mapping. In this commit, I will change this to a two phase process. In the first phase the mappings are grouped together based on the name of the mapped file. At the end of phase one we have a 'struct mapped_file', a new struct, for each mapped file. This struct associates an optional build-id with a list of mapped regions. In the second phase we try to find the file using its filename. If the file is found, and the 'struct mapped_file' has a build-id, then we'll compare the build-id with the file we found. This allows us to reject on-disk files which have changed since the core file was created. If no suitable file was found (either no file found, or a build-id mismatch) then we can use debuginfod to potentially download a suitable file. NOTE: In the future we could potentially add additional sanity checks here, for example, if a data-file is mapped, and has no build-id, we can estimate a minimum file size based on the expected mappings. If the file we find is not big enough then we can reject the on-disk file. But I don't know how useful this would actually be, so I've not done that for now. Having found (or not) a suitable file then we can create the data structures for each mapped region just as we did before. The new functionality here is the extra build-id check, and the possibility of rejecting an on-disk file if the build-id doesn't match. This change could have been done within the existing single phase approach I think, however, in the next approach I need to have all the mapped regions associated with the expected build-id, and the new two phase structure allows me to do that, this is the reason for such an extensive rewrite in this commit. There's a new test that exercises GDB's ability to find mapped files via the build-id, and this downloading from debuginfod. --- gdb/corelow.c | 274 ++++++++++---- .../gdb.debuginfod/corefile-mapped-file-1.c | 24 ++ .../gdb.debuginfod/corefile-mapped-file-2.c | 22 ++ .../gdb.debuginfod/corefile-mapped-file-3.c | 45 +++ .../gdb.debuginfod/corefile-mapped-file.exp | 356 ++++++++++++++++++ 5 files changed, 648 insertions(+), 73 deletions(-) create mode 100644 gdb/testsuite/gdb.debuginfod/corefile-mapped-file-1.c create mode 100644 gdb/testsuite/gdb.debuginfod/corefile-mapped-file-2.c create mode 100644 gdb/testsuite/gdb.debuginfod/corefile-mapped-file-3.c create mode 100644 gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp diff --git a/gdb/corelow.c b/gdb/corelow.c index 4a489d95706..28d619594d8 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -53,6 +53,7 @@ #include "cli/cli-cmds.h" #include "xml-tdesc.h" #include "memtag.h" +#include "cli/cli-style.h" #ifndef O_LARGEFILE #define O_LARGEFILE 0 @@ -277,6 +278,48 @@ core_target::build_file_mappings () std::unordered_map bfd_map; std::unordered_set unavailable_paths; + /* Type holding information about a single file mapped into the inferior + at the point when the core file was created. Associates a build-id + with the list of regions the file is mapped into. */ + struct mapped_file + { + /* Type for a region of a file that was mapped into the inferior when + the core file was generated. */ + struct region + { + /* Constructor. See member variables for argument descriptions. */ + region (CORE_ADDR start_, CORE_ADDR end_, CORE_ADDR file_ofs_) + : start (start_), + end (end_), + file_ofs (file_ofs_) + { /* Nothing. */ } + + /* The inferior address for the start of the mapped region. */ + CORE_ADDR start; + + /* The inferior address immediately after the mapped region. */ + CORE_ADDR end; + + /* The offset within the mapped file for this content. */ + CORE_ADDR file_ofs; + }; + + /* If not nullptr, then this is the build-id associated with this + file. */ + const bfd_build_id *build_id = nullptr; + + /* If true then we have seen multiple different build-ids associated + with the same filename. The build_id field will have been set back + to nullptr, and we should not set build_id in future. */ + bool ignore_build_id_p = false; + + /* All the mapped regions of this file. */ + std::vector regions; + }; + + /* All files mapped into the core file. The key is the filename. */ + std::unordered_map mapped_files; + /* See linux_read_core_file_mappings() in linux-tdep.c for an example read_core_file_mappings method. */ gdbarch_read_core_file_mappings (m_core_gdbarch, @@ -297,87 +340,172 @@ core_target::build_file_mappings () weed out non-file-backed mappings. */ gdb_assert (filename != nullptr); - if (unavailable_paths.find (filename) != unavailable_paths.end ()) + /* Add this mapped region to the data for FILENAME. */ + mapped_file &file_data = mapped_files[filename]; + file_data.regions.emplace_back (start, end, file_ofs); + if (build_id != nullptr && !file_data.ignore_build_id_p) { - /* We have already seen some mapping for FILENAME but failed to - find/open the file. There is no point in trying the same - thing again so just record that the range [start, end) is - unavailable. */ - m_core_unavailable_mappings.emplace_back (start, end - start); - return; - } - - struct bfd *bfd = bfd_map[filename]; - if (bfd == nullptr) - { - /* Use exec_file_find() to do sysroot expansion. It'll - also strip the potential sysroot "target:" prefix. If - there is no sysroot, an equivalent (possibly more - canonical) pathname will be provided. */ - gdb::unique_xmalloc_ptr expanded_fname - = exec_file_find (filename, NULL); - - if (expanded_fname == nullptr && build_id != nullptr) - debuginfod_exec_query (build_id->data, build_id->size, - filename, &expanded_fname); - - if (expanded_fname == nullptr) + if (file_data.build_id == nullptr) + file_data.build_id = build_id; + else if (!build_id_equal (build_id, file_data.build_id)) { - m_core_unavailable_mappings.emplace_back (start, end - start); - unavailable_paths.insert (filename); - warning (_("Can't open file %s during file-backed mapping " - "note processing"), - filename); - return; + warning (_("Multiple build-ids found for %ps"), + styled_string (file_name_style.style (), filename)); + file_data.build_id = nullptr; + file_data.ignore_build_id_p = true; } + } + }); - bfd = bfd_openr (expanded_fname.get (), "binary"); + for (const auto &iter : mapped_files) + { + const std::string &filename = iter.first; + const mapped_file &file_data = iter.second; + + /* Use exec_file_find() to do sysroot expansion. It'll + also strip the potential sysroot "target:" prefix. If + there is no sysroot, an equivalent (possibly more + canonical) pathname will be provided. */ + gdb::unique_xmalloc_ptr expanded_fname + = exec_file_find (filename.c_str (), nullptr); + + bool build_id_mismatch = false; + if (expanded_fname != nullptr && file_data.build_id != nullptr) + { + /* We temporarily open the bfd as a structured target, this + allows us to read the build-id from the bfd if there is one. + For this task it's OK if we reuse an already open bfd object, + so we make this call through GDB's bfd cache. Once we've + checked the build-id (if there is one) we'll drop this + reference and re-open the bfd using the "binary" target. */ + gdb_bfd_ref_ptr tmp_bfd + = gdb_bfd_open (expanded_fname.get (), gnutarget); + + if (tmp_bfd != nullptr + && bfd_check_format (tmp_bfd.get (), bfd_object) + && build_id_bfd_get (tmp_bfd.get ()) != nullptr) + { + /* The newly opened TMP_BFD has a build-id, and this mapped + file has a build-id extracted from the core-file. Check + the build-id's match, and if not, reject TMP_BFD. */ + const struct bfd_build_id *found + = build_id_bfd_get (tmp_bfd.get ()); + if (!build_id_equal (found, file_data.build_id)) + build_id_mismatch = true; + } + } - if (bfd == nullptr || !bfd_check_format (bfd, bfd_object)) - { - m_core_unavailable_mappings.emplace_back (start, end - start); - unavailable_paths.insert (filename); - warning (_("Can't open file %s which was expanded to %s " + gdb_bfd_ref_ptr abfd; + if (expanded_fname != nullptr && !build_id_mismatch) + { + struct bfd *b = bfd_openr (expanded_fname.get (), "binary"); + abfd = gdb_bfd_ref_ptr::new_reference (b); + } + + if ((expanded_fname == nullptr + || abfd == nullptr + || !bfd_check_format (abfd.get (), bfd_object)) + && file_data.build_id != nullptr) + { + expanded_fname = nullptr; + debuginfod_exec_query (file_data.build_id->data, + file_data.build_id->size, + filename.c_str (), &expanded_fname); + if (expanded_fname != nullptr) + { + struct bfd *b = bfd_openr (expanded_fname.get (), "binary"); + abfd = gdb_bfd_ref_ptr::new_reference (b); + } + } + + if (expanded_fname == nullptr + || abfd == nullptr + || !bfd_check_format (abfd.get (), bfd_object)) + { + /* If ABFD was opened, but the wrong format, close it now. */ + abfd = nullptr; + + /* Record all regions for this file as unavailable. */ + for (const mapped_file::region ®ion : file_data.regions) + m_core_unavailable_mappings.emplace_back (region.start, + region.end + - region.start); + + /* And give the user an appropriate warning. */ + if (build_id_mismatch) + { + if (expanded_fname == nullptr + || filename == expanded_fname.get ()) + warning (_("File %ps doesn't match build-id from core-file " + "during file-backed mapping processing"), + styled_string (file_name_style.style (), + filename.c_str ())); + else + warning (_("File %ps which was expanded to %ps, doesn't match " + "build-id from core-file during file-backed " + "mapping processing"), + styled_string (file_name_style.style (), + filename.c_str ()), + styled_string (file_name_style.style (), + expanded_fname.get ())); + } + else + { + if (expanded_fname == nullptr + || filename == expanded_fname.get ()) + warning (_("Can't open file %ps during file-backed mapping " + "note processing"), + styled_string (file_name_style.style (), + filename.c_str ())); + else + warning (_("Can't open file %ps which was expanded to %ps " "during file-backed mapping note processing"), - filename, expanded_fname.get ()); + styled_string (file_name_style.style (), + filename.c_str ()), + styled_string (file_name_style.style (), + expanded_fname.get ())); + } + } + else + { + /* Ensure that the bfd will be closed when core_bfd is closed. + This can be checked before/after a core file detach via "maint + info bfds". */ + gdb_bfd_record_inclusion (current_program_space->core_bfd (), + abfd.get ()); + + /* Create sections for each mapped region. */ + for (const mapped_file::region ®ion : file_data.regions) + { + /* Make new BFD section. All sections have the same name, + which is permitted by bfd_make_section_anyway(). */ + asection *sec = bfd_make_section_anyway (abfd.get (), "load"); + if (sec == nullptr) + error (_("Can't make section")); + sec->filepos = region.file_ofs; + bfd_set_section_flags (sec, SEC_READONLY | SEC_HAS_CONTENTS); + bfd_set_section_size (sec, region.end - region.start); + bfd_set_section_vma (sec, region.start); + bfd_set_section_lma (sec, region.start); + bfd_set_section_alignment (sec, 2); + + /* Set target_section fields. */ + m_core_file_mappings.emplace_back (region.start, region.end, sec); + } + } - if (bfd != nullptr) - bfd_close (bfd); - return; - } - /* Ensure that the bfd will be closed when core_bfd is closed. - This can be checked before/after a core file detach via - "maint info bfds". */ - gdb_bfd_record_inclusion (current_program_space->core_bfd (), bfd); - bfd_map[filename] = bfd; - } + /* If this is a bfd of a shared library, record its soname and + build-id. */ + if (file_data.build_id != nullptr && abfd != nullptr) + { + gdb::unique_xmalloc_ptr soname + = gdb_bfd_read_elf_soname (bfd_get_filename (abfd.get ())); - /* Make new BFD section. All sections have the same name, - which is permitted by bfd_make_section_anyway(). */ - asection *sec = bfd_make_section_anyway (bfd, "load"); - if (sec == nullptr) - error (_("Can't make section")); - sec->filepos = file_ofs; - bfd_set_section_flags (sec, SEC_READONLY | SEC_HAS_CONTENTS); - bfd_set_section_size (sec, end - start); - bfd_set_section_vma (sec, start); - bfd_set_section_lma (sec, start); - bfd_set_section_alignment (sec, 2); - - /* Set target_section fields. */ - m_core_file_mappings.emplace_back (start, end, sec); - - /* If this is a bfd of a shared library, record its soname - and build id. */ - if (build_id != nullptr) - { - gdb::unique_xmalloc_ptr soname - = gdb_bfd_read_elf_soname (bfd->filename); - if (soname != nullptr) - set_cbfd_soname_build_id (current_program_space->cbfd, - soname.get (), build_id); - } - }); + if (soname != nullptr) + set_cbfd_soname_build_id (current_program_space->cbfd, + soname.get (), file_data.build_id); + } + } normalize_mem_ranges (&m_core_unavailable_mappings); } diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-1.c b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-1.c new file mode 100644 index 00000000000..dd6b3f105f2 --- /dev/null +++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-1.c @@ -0,0 +1,24 @@ +/* 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 . */ + +/* This is in the shared library. */ +extern int foo (void); + +int +main (void) +{ + int res = foo (); + return res; +} diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-2.c b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-2.c new file mode 100644 index 00000000000..ccf35b75ef8 --- /dev/null +++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-2.c @@ -0,0 +1,22 @@ +/* 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 . */ + +volatile int *library_ptr = (int *) POINTER_VALUE; + +int +foo (void) +{ + return *library_ptr; +} diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-3.c b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-3.c new file mode 100644 index 00000000000..45d78336930 --- /dev/null +++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file-3.c @@ -0,0 +1,45 @@ +/* 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 . */ + +#include +#include +#include +#include +#include +#include +#include + +volatile void* library_base_address = 0; +volatile int *ptr = 0; + +int +main () +{ + struct stat buf; + int res; + + int fd = open (SHLIB_FILENAME, O_RDONLY); + assert (fd != -1); + + res = fstat (fd, &buf); + assert (res == 0); + + library_base_address + = mmap (NULL, buf.st_size, PROT_READ, MAP_PRIVATE, fd, 0); + + res = *ptr; /* Undefined behaviour here. */ + + return 0; +} diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp new file mode 100644 index 00000000000..6e3301e1c8d --- /dev/null +++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp @@ -0,0 +1,356 @@ +# 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 . */ + +# This test checks GDB's ability to use build-ids when setting up file backed +# mappings as part of reading a core-file. +# +# A core-file contains a list of the files that were mapped into the process +# at the time of the core-file creation. If the file was mapped read-only +# then the file contents will not be present in the core-file, but instead GDB +# is expected to open the mapped file and read the contents from there if +# needed. And this is what GDB does. +# +# GDB (via the BFD library) will also spot if a mapped looks like a valid ELF +# and contains a build-id, this build-id is passed back to GDB so that GDB can +# validate the on-disk file it finds matches the file that was mapped when the +# core-file was created. +# +# In addition, if the on-disk file is found to have a non-matching build-id +# then GDB can use debuginfod to (try) and download a suitable file. +# +# This test is about checking that this file backed mapping mechanism works +# correctly; that GDB will spot when the build-ids fail to match and will +# refuse to load an incorrect file. Additionally we check that the correct +# file can be downloaded from debuginfod. +# +# The test is rather contrived though. Instead of relying on having a shared +# library mapped at the time of crash we mmap a shared library into the +# process and then check this mapping within the test. +# +# The problem with using a normal shared library load for this test is that +# the shared library list is processed as part of a separate step when opening +# the core file. Right now this separate step doesn't check the build-ids +# correctly, so GDB will potentially open the wrong shared library file. The +# sections of this incorrect shared library are then added to GDB's list of +# target sections, and are used to satisfy memory reads, which can give the +# wrong results. +# +# This obviously needs fixing, but is a separate problem from the one being +# tested here, so this test deliberately checks the mapping using a file that +# is mmaped rather than loaded as a shared library, as such the file is in the +# core-files list of mapped files, but is not in the shared library list. +# +# Despite this test living in the gdb.debuginfod/ directory, only the last +# part of this test actually uses debuginfod, everything up to that point is +# pretty generic. + +require {!is_remote host} +require {!is_remote target} + +load_lib debuginfod-support.exp + +require allow_shlib_tests + +standard_testfile -1.c -2.c -3.c + +# Compile an executable that loads the shared library as an actual +# shared library, then use GDB to figure out the offset of the +# variable 'library_ptr' within the library. +set library_filename [standard_output_file "libfoo.so"] +set binfile2 [standard_output_file "library_loader"] + +if {[prepare_for_testing_full "build exec which loads the shared library" \ + [list $library_filename \ + { debug shlib build-id \ + additional_flags=-DPOINTER_VALUE=0x12345678 } \ + $srcfile2 {}] \ + [list $binfile2 [list debug shlib=$library_filename ] \ + $srcfile { debug }]] != 0} { + return +} + +if {![runto_main]} { + return +} + +if { [is_address_zero_readable] } { + return +} + +set ptr_address [get_hexadecimal_valueof "&library_ptr" "unknown"] + +set ptr_offset "unknown" +gdb_test_multiple "info proc mappings" "" { + -re "^\\s+($hex)\\s+($hex)\\s+$hex\\s+($hex)\[^\r\n\]+$library_filename\r\n" { + set low_addr $expect_out(1,string) + set high_addr $expect_out(2,string) + set file_offset $expect_out(3,string) + + if {[expr $ptr_address >= $low_addr] && [expr $ptr_address < $high_addr]} { + set mapping_offset [expr $ptr_address - $low_addr] + set ptr_offset [format 0x%x [expr $file_offset + $mapping_offset]] + } + + exp_continue + } + + -re "^$gdb_prompt $" { + } + + -re "(^\[^\r\n\]*)\r\n" { + set tmp $expect_out(1,string) + exp_continue + } +} + +gdb_assert { $ptr_offset ne "unknown" } \ + "found pointer offset" + +set ptr_size [get_integer_valueof "sizeof (library_ptr)" "unknown"] +set ptr_format_char "" +if { $ptr_size == 2 } { + set ptr_format_char "b" +} elseif { $ptr_size == 4 } { + set ptr_format_char "w" +} elseif { $ptr_size == 8 } { + set ptr_format_char "g" +} +if { $ptr_format_char eq "" } { + untested "could not figure out size of library_ptr variable" + return +} + +# Helper proc to read a value from inferior memory. Reads at address held in +# global PTR_ADDRESS, and use PTR_FORMAT_CHAR for the size of the read. +proc read_ptr_value { } { + set value "" + gdb_test_multiple "x/1${::ptr_format_char}x ${::ptr_address}" "" { + -re -wrap "^${::hex}(?:\\s+<\[^>\]+>)?:\\s+($::hex)" { + set value $expect_out(1,string) + } + -re -wrap "^${::hex}(?:\\s+<\[^>\]+>)?:\\s+Cannot access memory at address ${::hex}" { + set value "unavailable" + } + } + return $value +} + +set ptr_expected_value [read_ptr_value] +if { $ptr_expected_value eq "" } { + untested "could not find expected value for library_ptr" + return +} + +# Now compile a second executable. This one doesn't load the shared +# library as an actual shared library, but instead mmaps the library +# into the executable. +# +# Load this executable within GDB and confirm that we can use the +# offset we calculated previously to view the value of 'library_ptr'. +set opts [list debug additional_flags=-DSHLIB_FILENAME=\"$library_filename\"] +if {[prepare_for_testing "prepare second executable" $binfile \ + $srcfile3 $opts] != 0} { + return +} + +if {![runto_main]} { + return +} + +gdb_breakpoint [gdb_get_line_number "Undefined behaviour here" $srcfile3] +gdb_continue_to_breakpoint "run to breakpoint" + +set library_base_address \ + [get_hexadecimal_valueof "library_base_address" "unknown"] +set ptr_address [format 0x%x [expr $library_base_address + $ptr_offset]] + +set ptr_value [read_ptr_value] +gdb_assert { $ptr_value == $ptr_expected_value } \ + "check value of pointer variable" + +# Now rerun the second executable outside of GDB. The executable should crash +# and generate a corefile. +set corefile [core_find $binfile] +if {$corefile eq ""} { + untested "could not generate core file" + return +} + +# Load a core file from the global COREFILE. Use TESTNAME as the name +# of the test. +# +# If LINE_RE is not the empty string then this is a regexp for a line +# that we expect to see in the output when loading the core file, if +# the line is not present then this test will fail. +# +# Any lines beginning with 'warning: ' will cause this test to fail. +# +# A couple of other standard lines that are produced when loading a +# core file are also checked for, just to make sure the core file +# loading has progressed as expected. +proc load_core_file { testname { line_re "" } } { + set code {} + + if { $line_re ne "" } { + append code { + -re "^$line_re\r\n" { + set saw_expected_line true + exp_continue + } + } + set saw_expected_line false + } else { + set saw_expected_line true + } + + set saw_unknown_warning false + set saw_generated_by_line false + set saw_prog_terminated_line false + + append code { + -re "^warning: \[^\r\n\]+\r\n" { + set saw_unknown_warning true + exp_continue + } + + -re "^Core was generated by \[^\r\n\]+\r\n" { + set saw_generated_by_line true + exp_continue + } + + -re "^Program terminated with signal SIGSEGV, Segmentation fault\\.\r\n" { + set saw_prog_terminated_line true + exp_continue + } + + -re "^$::gdb_prompt $" { + gdb_assert {$saw_generated_by_line \ + && $saw_prog_terminated_line \ + && $saw_expected_line \ + && !$saw_unknown_warning} \ + $gdb_test_name + } + + -re "^\[^\r\n\]*\r\n" { + exp_continue + } + } + + set res [catch { return [gdb_test_multiple "core-file $::corefile" \ + "$testname" $code] } string] + + if {$res == 1} { + global errorInfo errorCode + return -code error -errorinfo $errorInfo -errorcode $errorCode $string + } elseif {$res == 2} { + return $string + } else { + # We expect RES to be 2 (TCL_RETURN) or 1 (TCL_ERROR). If we get + # here then somehow the 'catch' above finished without hitting + # either of those cases, which is .... weird. + perror "unexepcted return value, code = $res, value = $string" + return -1 + } +} + +# And now restart GDB, load the core-file and check that the library shows as +# being mapped in, and that we can still read the library_ptr value from +# memory. +clean_restart $binfile + +load_core_file "load core file" + +set library_base_address [get_hexadecimal_valueof "library_base_address" \ + "unknown" "get library_base_address in core-file"] +set ptr_address [format 0x%x [expr $library_base_address + $ptr_offset]] + +set ptr_value [read_ptr_value] +gdb_assert { $ptr_value == $ptr_expected_value } \ + "check value of pointer variable from core-file" + +# Now move the shared library file away and restart GDB. This time when we +# load the core-file we should see a warning that GDB has failed to map in the +# library file. An attempt to read the variable from the library file should +# fail / give a warning. +set library_backup_filename [standard_output_file "libfoo.so.backup"] +remote_exec build "mv \"$library_filename\" \"$library_backup_filename\"" + +clean_restart $binfile + +load_core_file "load corefile with library file missing" \ + "warning: Can't open file [string_to_regexp $library_filename] during file-backed mapping note processing" + +set ptr_value [read_ptr_value] +gdb_assert { $ptr_value eq "unavailable" } \ + "check value of pointer is unavailable with library file missing" + +# Build a new version of the shared library, keep the library the same size, +# but change the contents so the build-id changes. Then restart GDB and load +# the core-file again. GDB should spot that the build-id for the shared +# library is not as expected, and should refuse to map in the shared library. +if {[build_executable "build second version of shared library" \ + $library_filename $srcfile2 \ + { debug shlib build-id \ + additional_flags=-DPOINTER_VALUE=0x11223344 }] != 0} { + return +} + +clean_restart $binfile + +load_core_file "load corefile with wrong library in place" \ + "warning: File [string_to_regexp $library_filename] doesn't match build-id from core-file during file-backed mapping processing" + +set ptr_value [read_ptr_value] +gdb_assert { $ptr_value eq "unavailable" } \ + "check value of pointer is unavailable with wrong library in place" + +# Setup a debuginfod server which can serve the original shared library file. +# Then restart GDB and load the core-file. GDB should download the original +# shared library from debuginfod and use that to provide the file backed +# mapping. +if {![allow_debuginfod_tests]} { + untested "skippig debuginfod parts of this test" + return +} + +set server_dir [standard_output_file "debuginfod.server"] +file mkdir $server_dir +file rename -force $library_backup_filename $server_dir + +prepare_for_debuginfod cache db + +set url [start_debuginfod $db $server_dir] +if { $url eq "" } { + unresolved "failed to start debuginfod server" + return +} + +with_debuginfod_env $cache { + setenv DEBUGINFOD_URLS $url + + clean_restart + gdb_test_no_output "set debuginfod enabled on" \ + "enabled debuginfod for initial test" + gdb_load $binfile + + load_core_file "load corefile, download library from debuginfod" \ + "Downloading\[^\r\n\]* executable for [string_to_regexp $library_filename]\\.\\.\\." + + set ptr_value [read_ptr_value] + gdb_assert { $ptr_value == $ptr_expected_value } \ + "check value of pointer variable after downloading library file" +} + +stop_debuginfod From patchwork Mon May 20 13:08:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 90483 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 495E03858401 for ; Mon, 20 May 2024 13:10:04 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id C50293858D39 for ; Mon, 20 May 2024 13:08:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C50293858D39 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 C50293858D39 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=1716210525; cv=none; b=XxqoZpz/S1pGHgPtqQITNUdNRHANS4aHFeCangwPxnj3PAEsh6SKxOj/Kyi3MaWhovhuxT/wgAs7W9YyaMlZ/nN4J4+SRgL+nN8OaxCa0B2LZbhgfeR6SwdBxaKgH5SiVZ3c2mpjmEtEVwrp1MXuYRihez62j+4ehgZZ9+SWPZ8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716210525; c=relaxed/simple; bh=T7emvc+JJ6PbBAcEWx7veCuVPqiQ68aC5gYHYfZE+Lc=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=bzYKcN3G29DZg7zclhHwRG4fLc8MumEfBQ6H+raErF05paY0RpmcvAUrYrUOoduZODhRxYeJV5iLE67jQ94nQ/x1q8zi7T5VystX2YZ7/eY0Nt80dsyyotB5o4uve53ZKNLlpqtx6Wz0vIxbhK6r8g43SE/nx6YMoTGMotGgnko= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1716210519; 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=LN/ac/awS7HivOF0DaTcWni1Uvv5PXUj7tPZrWuiZ98=; b=BzHfgRA3cUVoZJNSgz+sSA/7cAz0zLlxmXWVHGamUE2ZTE0oOafhqcZCIMQzPB1rJI+ya4 Qh0EOBIpeB45NxC8rWi5iAedeih5vRX1ycVuu3LjQ24UN3S+5h3NztRqsL8D8Z6UcGyBES RmXfhi7X2rILdIsw+JV2Gh3OWoXCEYo= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-61-boL42ZdCOlKXSCycZ5Nrkg-1; Mon, 20 May 2024 09:08:37 -0400 X-MC-Unique: boL42ZdCOlKXSCycZ5Nrkg-1 Received: by mail-ej1-f71.google.com with SMTP id a640c23a62f3a-a592c35ac06so1145478766b.0 for ; Mon, 20 May 2024 06:08:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716210516; x=1716815316; 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=LN/ac/awS7HivOF0DaTcWni1Uvv5PXUj7tPZrWuiZ98=; b=oWKlkxgOH/u2oigW84ffY7QNBeAjPyDlrrYHF34FiXqkgi7/lqzH/ahVCgl1buMr5W vYpwuEgHk6B2LHMTIiAj7xBq5nJf4qg0PJhZ5/KUyXORxhFudWQu1JTyj5c/b3GIPuB6 oYSVdBos2H6ozkitjq4p7KqU81IjnjGeaO7Z/qTEmVjb3Qj2dJAo8wEcXLicqEmduCkx T1iyYgcdt0di0xZzu2SRSdQR0qy/l50Rd70Qmt6jn4Hwifg/qGMxCb8GIgTOEtjL7FGY JVbnG7BFHtHS2H3vqbpirKCr6qQO3hXvEJLrG/ewc3LYFwPq5TWA6cfzEqcg6qXVaoo3 bJHw== X-Gm-Message-State: AOJu0YzPUscAYbBUsW0J8wa2r07sDib/YPQ8NSwGaAn6OSqWeWZZISty YpcJWFzhG5GHipxfTbSTTpl10lsNFa0P+F48NUoJ2skULc/8LG4qYBEzmYPub7V2oYyKAZ0LjBc UVnwUs7NKYHLmMsYP/PVls0iZBFcPRP4r9yr7KnS+MzQ3zg6jl9YAf+ARKcxfmfCIIOC/9+BGiY gRwXyhZyUg69BAdXeMEe6pneG2MFYxnLx4wmrzfL4KmVM= X-Received: by 2002:a17:906:a2d3:b0:a59:bbea:14e8 with SMTP id a640c23a62f3a-a5d5b018e3dmr550498866b.17.1716210515260; Mon, 20 May 2024 06:08:35 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH/aSMjksqPps/N49lQfBZzo44mwAKUwbV/4zcBKem+d2CuSlBeSfevQPi24nf3F5PxjB4iew== X-Received: by 2002:a17:906:a2d3:b0:a59:bbea:14e8 with SMTP id a640c23a62f3a-a5d5b018e3dmr550493666b.17.1716210513840; Mon, 20 May 2024 06:08:33 -0700 (PDT) Received: from localhost ([31.111.84.240]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a5a179c7f78sm1447769066b.133.2024.05.20.06.08.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 May 2024 06:08:33 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 3/4] gdb: improve shared library build-id check for core-files Date: Mon, 20 May 2024 14:08:26 +0100 Message-Id: <63ba29e96b46b3345457d3e8257e9b80cd0d5fc8.1716210406.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org When GDB opens a core file, in 'core_target::build_file_mappings ()', we collection information about the files that are mapped into the core file, specifically, the build-id and the DT_SONAME attribute for the file, which will be set for some shared libraries. We then cache the DT_SONAME to build-id information on the core file bfd object in the function set_cbfd_soname_build_id. Later, when we are loading the shared libraries for the core file, we can use the libraries file name to look in the DT_SONAME to build-id map, and, if we find a matching entry, we can use the build-id to validate that we are loading the correct shared library. This works OK, but has some limitations: not every shared library will have a DT_SONAME attribute. Though it is good practice to add such an attribute, it's not required. A library without this attribute will not have its build-id checked, which can lead to GDB loading the wrong shared library. What I want to do in this commit is to improve GDB's ability to use the build-id extracted during the ::build_file_mappings phase to both validate the shared libraries being loaded, and then to use these build-ids to potentially find (via debuginfod) the shared library. To do this two changes need to be made to GDB. First, rather than just recording the soname to build-id mapping in set_cbfd_soname_build_id, I now also record, the full mapped filename to build-id mapping, and also the memory ranges to build-id mapping for every memory range covered by a mapped file. Second, I've added a new callback solib_ops::find_solib_addr. This callback takes a solib object and returns an (optional) address within the inferior that is part of this library. Now, when we load a shared library for a core file, we do the following lookups: 1. Is the exact filename of the shared library found in the filename to build-id map? If so then use this build-id for validation. 2. Find an address within the shared library using ::find_solib_addr and then look for an entry in the mapped address to build-id map. If an entry is found then use this build-id. 3. Finally, look in the soname to build-id map. If an entry is found then use this build-id. The addition of step #2 here means that GDB is now far more likely to find a suitable build-id for a shared library. Having acquired a build-id the existing code for using debuginfod to lookup a shared library object can trigger more often. On top of this, I also build a reverse build-id to filename map in set_cbfd_soname_build_id. This is useful as often a shared library is implemented as a symbolic link to the actual shared library file. The mapped file information is stored based on the actual final, real file name, while the shared library information holds the original symbolic link file name. If when loading the shared library, we find the symbolic link has disappeared, then we can use the build-id to file name map to check if the actual file is still around, if it is (and if the build-id matches) then we can fall back to use that file. I've also renamed set_cbfd_soname_build_id to set_cbfd_solib_build_id and get_cbfd_soname_build_id to get_cbfd_solib_build_id as the 'soname' in the function names was now only part of the larger job these functions do. --- gdb/corelow.c | 33 +- gdb/solib-aix.c | 6 + gdb/solib-darwin.c | 6 + gdb/solib-dsbt.c | 6 + gdb/solib-frv.c | 6 + gdb/solib-svr4.c | 10 + gdb/solib-target.c | 6 + gdb/solib.c | 373 +++++++++++++++--- gdb/solib.h | 39 +- gdb/solist.h | 22 ++ gdb/testsuite/gdb.base/solib-search.exp | 6 +- .../gdb.debuginfod/solib-with-soname-1.c | 39 ++ .../gdb.debuginfod/solib-with-soname-2.c | 41 ++ .../gdb.debuginfod/solib-with-soname.exp | 260 ++++++++++++ gdb/testsuite/lib/gdb.exp | 13 + 15 files changed, 802 insertions(+), 64 deletions(-) create mode 100644 gdb/testsuite/gdb.debuginfod/solib-with-soname-1.c create mode 100644 gdb/testsuite/gdb.debuginfod/solib-with-soname-2.c create mode 100644 gdb/testsuite/gdb.debuginfod/solib-with-soname.exp diff --git a/gdb/corelow.c b/gdb/corelow.c index 28d619594d8..85bc3c26bea 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -418,6 +418,10 @@ core_target::build_file_mappings () } } + std::vector ranges; + for (const mapped_file::region ®ion : file_data.regions) + ranges.emplace_back (region.start, region.end - region.start); + if (expanded_fname == nullptr || abfd == nullptr || !bfd_check_format (abfd.get (), bfd_object)) @@ -494,16 +498,29 @@ core_target::build_file_mappings () } } - /* If this is a bfd of a shared library, record its soname and - build-id. */ - if (file_data.build_id != nullptr && abfd != nullptr) + /* If this is a bfd with a build-id then record the filename, + optional soname (DT_SONAME .dynamic attribute), and the range of + addresses at which this bfd is mapped. This information can be + used to perform build-id checking when loading the shared + libraries. */ + if (file_data.build_id != nullptr) { - gdb::unique_xmalloc_ptr soname - = gdb_bfd_read_elf_soname (bfd_get_filename (abfd.get ())); + normalize_mem_ranges (&ranges); + + const char *actual_filename = nullptr; + gdb::unique_xmalloc_ptr soname; + if (abfd != nullptr) + { + actual_filename = bfd_get_filename (abfd.get ()); + soname = gdb_bfd_read_elf_soname (actual_filename); + } - if (soname != nullptr) - set_cbfd_soname_build_id (current_program_space->cbfd, - soname.get (), file_data.build_id); + set_cbfd_solib_build_id (current_program_space->cbfd, + soname.get (), + filename.c_str (), + actual_filename, + std::move (ranges), + file_data.build_id); } } diff --git a/gdb/solib-aix.c b/gdb/solib-aix.c index a50bb165c19..a9242408137 100644 --- a/gdb/solib-aix.c +++ b/gdb/solib-aix.c @@ -689,6 +689,12 @@ const solib_ops solib_aix_so_ops = solib_aix_open_symbol_file_object, solib_aix_in_dynsym_resolve_code, solib_aix_bfd_open, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + default_find_solib_addr, }; void _initialize_solib_aix (); diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c index f0828fdf102..e9a85db1e5e 100644 --- a/gdb/solib-darwin.c +++ b/gdb/solib-darwin.c @@ -665,4 +665,10 @@ const solib_ops darwin_so_ops = open_symbol_file_object, darwin_in_dynsym_resolve_code, darwin_bfd_open, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + default_find_solib_addr, }; diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c index 11225f72ed0..479f32182d2 100644 --- a/gdb/solib-dsbt.c +++ b/gdb/solib-dsbt.c @@ -913,6 +913,12 @@ const solib_ops dsbt_so_ops = open_symbol_file_object, dsbt_in_dynsym_resolve_code, solib_bfd_open, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + default_find_solib_addr, }; void _initialize_dsbt_solib (); diff --git a/gdb/solib-frv.c b/gdb/solib-frv.c index 39508fab4c8..7502fe1f560 100644 --- a/gdb/solib-frv.c +++ b/gdb/solib-frv.c @@ -1084,4 +1084,10 @@ const solib_ops frv_so_ops = open_symbol_file_object, frv_in_dynsym_resolve_code, solib_bfd_open, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + default_find_solib_addr, }; diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 1d4a50568d7..91359293e75 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -3353,6 +3353,15 @@ svr4_iterate_over_objfiles_in_search_order } } +/* See solib_ops::find_solib_addr in solist.h. */ + +static std::optional +svr4_find_solib_addr (solib &so) +{ + auto *li = gdb::checked_static_cast (so.lm_info.get ()); + return li->l_addr_inferior; +} + const struct solib_ops svr4_so_ops = { svr4_relocate_section_addresses, @@ -3368,6 +3377,7 @@ const struct solib_ops svr4_so_ops = svr4_keep_data_in_core, svr4_update_solib_event_breakpoints, svr4_handle_solib_event, + svr4_find_solib_addr, }; void _initialize_svr4_solib (); diff --git a/gdb/solib-target.c b/gdb/solib-target.c index 6563da05a47..f89761f109f 100644 --- a/gdb/solib-target.c +++ b/gdb/solib-target.c @@ -412,4 +412,10 @@ const solib_ops solib_target_so_ops = solib_target_open_symbol_file_object, solib_target_in_dynsym_resolve_code, solib_bfd_open, + nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + default_find_solib_addr, }; diff --git a/gdb/solib.c b/gdb/solib.c index db2ff5ce9a3..c39dfbcc78e 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -475,56 +475,304 @@ solib_bfd_open (const char *pathname) return abfd; } -/* Mapping of a core file's shared library sonames to their respective - build-ids. Added to the registries of core file bfds. */ +/* A mem_range and the build-id associated with the file mapped into the + given range. */ -typedef std::unordered_map soname_build_id_map; +struct mem_range_and_build_id +{ + mem_range_and_build_id (mem_range &&r, const bfd_build_id *id) + : range (r), + build_id (id) + { /* Nothing. */ } + + /* A range of memory addresses. */ + mem_range range; + + /* The build-id of the file mapped into RANGE. */ + const bfd_build_id *build_id; +}; + +/* When a core file is opened set_cbfd_solib_build_id is called and the + data it is passed is used to create an instance of this class. + + This class maps various information extracted from parsing the core + file onto the corresponding build-id. + + When we latter load the shared libraries as part of the core file + opening process we can use this data structure to find the build-id + that corresponds to a shared library. + + Knowing the build-id allows for two things, first, we can refuse to + load an incorrect shared library (a file is found, but the build-id + doesn't match), and second, we can use various strategies to try and + find the correct shared library based on the build-id (e.g. we might be + able to download the shared library from debuginfod). */ + +struct solib_core_file_build_id_data +{ + /* A type that maps a string to a build-id. */ + using string_to_build_id_map + = std::unordered_map; + + /* A type for a reverse build-id to string map. We only store a sting + pointer in this map as we point back to the strings held in a + string_to_build_id_map, this avoids duplicating some strings. */ + using build_id_to_string_map + = std::unordered_map; + + /* When loading a core file, the build-ids are extracted based on the + file backed mappings. This map associates the name of a file that was + mapped into the core file with the corresponding build-id. When + opening a shared library, if the shared library filename appears in + this map, then we can use the corresponding build-id. The build-id + pointers in this map will never be nullptr. */ + + string_to_build_id_map filename_to_build_id_map; + + /* Map a build-id pointer back to a filename. The std::string pointers + in this map point back to the strings in FILENAME_TO_BUILD_ID_MAP. + If we end up figuring out the build-id from SONAME_TO_BUILD_ID_MAP or + from ADDRESS_TO_BUILD_ID_LIST then it might be that we can't find an + on-disk file. In this case we can use this reverse map to find the + name of the mapped file, which might be usable. */ + + build_id_to_string_map build_id_to_filename_map; + + /* If the file that was mapped into the core file was a shared library + then it might have a DT_SONAME tag in its .dynamic section, this tag + contains the name of a shared object. When opening a shared library, + if it's basename appears in this map then we can use the corresponding + build-id. + + In the rare case that two different files have the same DT_SONAME + value then the build-id pointer in this map will be nullptr, this + indicates that it's not possible to find a build-id based on the + DT_SONAME value. */ + + string_to_build_id_map soname_to_build_id_map; + + /* This vector maps memory ranges onto an associated build-id. The + ranges are those of the files mapped into the core file. When opening + a shared library, if non of the string to build-id maps provide a + build-id then we can find a representative address for the library by + calling solib_ops::find_solib_addr() then then look in this vector for + a memory range that covers this address. If a suitable match is found + then we use the corresponding build-id. + + Entries in this vector should not overlap, and are sorted be + increasing memory address. Within each entry the build-id pointer + will not be nullptr. */ + + std::vector address_to_build_id_list; + + /* The address_to_build_id_list is built over multiple calls to + set_cbfd_solib_build_id, each call corresponds to a single mapped + file and might add multiple entries to address_to_build_id_list. It + would be wasteful to sort address_to_build_id_list each time some new + entries are added, better to record that the address_to_build_id_list + is unsorted, and then sort once the first time we are going to perform + a lookup in address_to_build_id_list, that is what this flag is for. + When this flag is false address_to_build_id_list is not sorted, and + needs sorting before we search the list. When this flag is true the + address_to_build_id_list is sorted. */ + + bool address_to_build_id_list_sorted = false; +}; /* Key used to associate a soname_build_id_map to a core file bfd. */ -static const struct registry::key +static const struct registry::key cbfd_soname_build_id_data_key; /* See solib.h. */ void -set_cbfd_soname_build_id (gdb_bfd_ref_ptr abfd, const char *soname, - const bfd_build_id *build_id) +set_cbfd_solib_build_id (gdb_bfd_ref_ptr cbfd, const char *soname, + const char *expected_filename, + const char *actual_filename, + std::vector &&ranges, + const bfd_build_id *build_id) { - gdb_assert (abfd.get () != nullptr); - gdb_assert (soname != nullptr); + gdb_assert (cbfd.get () != nullptr); gdb_assert (build_id != nullptr); + gdb_assert (expected_filename != nullptr); + + solib_core_file_build_id_data *lookup_data + = cbfd_soname_build_id_data_key.get (cbfd.get ()); + + if (lookup_data == nullptr) + { + lookup_data = cbfd_soname_build_id_data_key.emplace (cbfd.get ()); + gdb_assert (lookup_data != nullptr); + } + + if (soname != nullptr) + { + /* If we already have an entry with this SONAME then this indicates + that the inferior has two files mapped into memory with different + file names (and most likely different build-ids), but with the + same DT_SONAME attribute. In this case we can't use the + DT_SONAME to figure out the expected build-id of a shared + library, so poison the entry for this SONAME by setting the entry + to nullptr. */ + auto it + = lookup_data->soname_to_build_id_map.find (soname); + if (it != lookup_data->soname_to_build_id_map.end () + && it->second != nullptr + && !build_id_equal (it->second, build_id)) + lookup_data->soname_to_build_id_map[soname] = nullptr; + else + lookup_data->soname_to_build_id_map[soname] = build_id; + } + + /* When the core file is initially opened and the mapped files are + parsed, we group the build-id information based on the file name. As + a consequence, we should see each EXPECTED_FILENAME value exactly + once. This means that each insertion should always succeed. */ + const auto [it, inserted] + = lookup_data->filename_to_build_id_map.emplace (expected_filename, + build_id); + gdb_assert (inserted); + + /* Setup the reverse build-id to file name map. We store a pointer to + the string in the filename_to_build_id_map, this saves creating + another copy of this string. The two maps have the same lifetime, so + this should be fine. */ + if (actual_filename != nullptr) + lookup_data->build_id_to_filename_map.emplace (build_id, + actual_filename); + + /* Setup the list of memory range to build-id objects. */ + for (mem_range &r : ranges) + lookup_data->address_to_build_id_list.emplace_back (std::move (r), + build_id); + + /* At this point the address_to_build_id_list is unsorted (we just added + some entries to the end of the list). All entries should be added + before any look-ups are performed, and the list is only sorted when + the first look-up is performed. */ + gdb_assert (!lookup_data->address_to_build_id_list_sorted); +} - soname_build_id_map *mapptr - = cbfd_soname_build_id_data_key.get (abfd.get ()); +/* Helper for get_cbfd_solib_build_id. BUILD_ID is a build-id that was + found in LOOKUP_DATA. Look-up the corresponding file name in + LOOKUP_DATA.build_id_to_filename_map and return a pair containing + BUILD_ID and the file name pointer that we find. If no corresponding + filename is found in the build_id_to_filename_map then the returned + pair contains BUILD_ID and an empty string. If BUILD_ID is nullptr + then the returned pair contains nullptr and an empty string. */ + +static std::pair +get_cbfd_solib_make_result (solib_core_file_build_id_data *lookup_data, + const bfd_build_id *build_id) +{ + if (build_id != nullptr) + { + gdb_assert (lookup_data != nullptr); - if (mapptr == nullptr) - mapptr = cbfd_soname_build_id_data_key.emplace (abfd.get ()); + auto it = lookup_data->build_id_to_filename_map.find (build_id); + if (it != lookup_data->build_id_to_filename_map.end ()) + return { build_id, it->second }; + } - (*mapptr)[soname] = build_id_to_string (build_id); + return { build_id, {} }; } -/* If SONAME had a build-id associated with it in ABFD's registry by a - previous call to set_cbfd_soname_build_id then return the build-id - as a NULL-terminated hex string. */ +/* CBFD is the current core file bfd object, which might be nullptr, + FILENAME is the file name of the shared library GDB is trying to load, + and SOLIB_ADDR is (optionally) an address within the shared library. -static gdb::unique_xmalloc_ptr -get_cbfd_soname_build_id (gdb_bfd_ref_ptr abfd, const char *soname) + Look through the data previously registered via calls to + set_cbfd_solib_build_id and try to find a build-id corresponding to + FILENAME. + + If a build-id is found then return a pair, the first item is the + build-id and the second item is a string which contains the file name of + the file GDB opened to provide the file backed mapping. This file name + string might be empty if GDB didn't find a file to provide the file + backed mapping. + + If no build-id can be found for FILENAME then GDB will return a pair + containing nullptr (for the build-id) and an empty string for the file + name. */ + +static std::pair +get_cbfd_solib_build_id (gdb_bfd_ref_ptr cbfd, const char *filename, + const std::optional &solib_addr) { - if (abfd.get () == nullptr || soname == nullptr) - return {}; + if (cbfd.get () == nullptr) + return { nullptr, {} }; - soname_build_id_map *mapptr - = cbfd_soname_build_id_data_key.get (abfd.get ()); + solib_core_file_build_id_data *lookup_data + = cbfd_soname_build_id_data_key.get (cbfd.get ()); - if (mapptr == nullptr) - return {}; + if (lookup_data == nullptr) + return { nullptr, {} }; - auto it = mapptr->find (lbasename (soname)); - if (it == mapptr->end ()) - return {}; + if (filename != nullptr) + { + /* If there's a matching entry in filename_to_build_id_map then the + associated build-id will not be nullptr, and can be used to + validate that FILENAME is correct. */ + auto it = lookup_data->filename_to_build_id_map.find (filename); + if (it != lookup_data->filename_to_build_id_map.end ()) + return get_cbfd_solib_make_result (lookup_data, it->second); + } + + if (solib_addr.has_value ()) + { + /* On the first lookup, sort the address_to_build_id_list. */ + if (!lookup_data->address_to_build_id_list_sorted) + { + std::sort (lookup_data->address_to_build_id_list.begin (), + lookup_data->address_to_build_id_list.end (), + [] (const mem_range_and_build_id &a, + const mem_range_and_build_id &b) { + return a.range < b.range; + }); + lookup_data->address_to_build_id_list_sorted = true; + } + + /* Look for the first entry whose range's start address is not less + than, or equal too, the address SOLIB_ADDR. If we find such an + entry, then the previous entry's range might contain SOLIB_ADDR. + If it does then that previous entry's build-id can be used. */ + auto it = std::lower_bound + (lookup_data->address_to_build_id_list.begin (), + lookup_data->address_to_build_id_list.end (), + *solib_addr, + [] (const mem_range_and_build_id &a, + const CORE_ADDR &b) { + return a.range.start <= b; + }); + + if (it != lookup_data->address_to_build_id_list.begin ()) + { + --it; - return make_unique_xstrdup (it->second.c_str ()); + if (it->range.contains (*solib_addr)) + return get_cbfd_solib_make_result (lookup_data, it->build_id); + } + } + + if (filename != nullptr) + { + /* If the basename of FILENAME appears in the soname_to_build_id_map + then when the mapped files were processed, we saw a file with a + DT_SONAME attribute corresponding to FILENAME, use that build-id + to validate FILENAME. + + However, the build-id in this map might be nullptr if we saw + multiple mapped files with the same DT_SONAME attribute (though + this should be pretty rare). */ + auto it + = lookup_data->soname_to_build_id_map.find (lbasename (filename)); + if (it != lookup_data->soname_to_build_id_map.end () + && it->second != nullptr) + return get_cbfd_solib_make_result (lookup_data, it->second); + } + + return { nullptr, {} }; } /* Given a pointer to one of the shared objects in our list of mapped @@ -546,36 +794,53 @@ solib_map_sections (solib &so) gdb::unique_xmalloc_ptr filename (tilde_expand (so.so_name.c_str ())); gdb_bfd_ref_ptr abfd (ops->bfd_open (filename.get ())); - gdb::unique_xmalloc_ptr build_id_hexstr - = get_cbfd_soname_build_id (current_program_space->cbfd, - so.so_name.c_str ()); + std::optional solib_addr = ops->find_solib_addr (so); + + const auto [expected_build_id, mapped_filename] + = get_cbfd_solib_build_id (current_program_space->cbfd, + so.so_name.c_str (), + solib_addr); /* If we already know the build-id of this solib from a core file, verify it matches ABFD's build-id. If there is a mismatch or the solib wasn't found, attempt to query debuginfod for the correct solib. */ - if (build_id_hexstr.get () != nullptr) + if (expected_build_id != nullptr) { - bool mismatch = false; + bool mismatch = (abfd != nullptr + && build_id_bfd_get (abfd.get ()) != nullptr + && !build_id_equal (expected_build_id, + build_id_bfd_get (abfd.get ()))); - if (abfd != nullptr && abfd->build_id != nullptr) - { - std::string build_id = build_id_to_string (abfd->build_id); - - if (build_id != build_id_hexstr.get ()) - mismatch = true; - } if (abfd == nullptr || mismatch) { - scoped_fd fd = debuginfod_exec_query ( - (const unsigned char *) build_id_hexstr.get (), 0, - so.so_name.c_str (), &filename); - - if (fd.get () >= 0) - abfd = ops->bfd_open (filename.get ()); - else if (mismatch) - warning (_ ("Build-id of %ps does not match core file."), - styled_string (file_name_style.style (), - filename.get ())); + /* If GDB found a suitable file during the file mapping + processing stage then lets use that. We don't check the + build-id after opening this file, either this file was found + by build-id, in which case it's going to match, or this file + doesn't have a build-id, so checking tells us nothing. + However, if it was good enough during the mapped file + processing, we assume it's good enough now. */ + if (!mapped_filename.empty ()) + abfd = ops->bfd_open (mapped_filename.c_str ()); + else + abfd = nullptr; + + if (abfd == nullptr) + { + scoped_fd fd = debuginfod_exec_query + (expected_build_id->data, expected_build_id->size, + so.so_name.c_str (), &filename); + + if (fd.get () >= 0) + abfd = ops->bfd_open (filename.get ()); + else if (mismatch) + { + warning (_ ("Build-id of %ps does not match core file."), + styled_string (file_name_style.style (), + filename.get ())); + abfd = nullptr; + } + } } } @@ -1705,6 +1970,14 @@ remove_user_added_objfile (struct objfile *objfile) } } +/* See solist.h. */ + +std::optional +default_find_solib_addr (solib &so) +{ + return {}; +} + void _initialize_solib (); void diff --git a/gdb/solib.h b/gdb/solib.h index f7a93c0718f..9cd9f111cde 100644 --- a/gdb/solib.h +++ b/gdb/solib.h @@ -29,6 +29,7 @@ struct program_space; #include "gdb_bfd.h" #include "symfile-add-flags.h" #include "gdbsupport/function-view.h" +#include "memrange.h" /* Value of the 'set debug solib' configuration variable. */ @@ -136,11 +137,39 @@ extern void update_solib_breakpoints (void); extern void handle_solib_event (void); -/* Associate SONAME with BUILD_ID in ABFD's registry so that it can be - retrieved with get_cbfd_soname_build_id. */ +/* This function is used to pass information extracted when processing the + core file's mapped file section through to the shared library processing + parts of GDB. When the mapped files are processed GDB is (sometimes) + able to find the build-ids for the mapped files. These build-ids are + recorded by calling this function, and can be used during shared library + loading to ensure the correct shared libraries are loaded. -extern void set_cbfd_soname_build_id (gdb_bfd_ref_ptr abfd, - const char *soname, - const bfd_build_id *build_id); + CBFD is the core file bfd object to record this information against. + + SONAME is the DT_SONAME attribute extracted from the .dynamic section of + the shared library that was mapped into the core file. This can be + nullptr. + + EXPECTED_FILENAME is the name of the file that was mapped into the + inferior as extracted from the core file, this should never be nullptr. + + ACTUAL_FILENAME is the name of the actual file GDB found to provide the + mapped file information, this can be nullptr if GDB failed to find a + suitable file. + + RANGES is the list of memory ranges at which this file was mapped into + the inferior. + + BUILD_ID this is the build-id for this mapped file, this will never be + nullptr. Not every mapped file will have a build-id, but there's no + point calling this function if we failed to find a build-id; this is all + about allowing the shared library code to find the build-id. */ + +extern void set_cbfd_solib_build_id (gdb_bfd_ref_ptr cbfd, + const char *soname, + const char *expected_filename, + const char *actual_filename, + std::vector &&ranges, + const bfd_build_id *build_id); #endif /* SOLIB_H */ diff --git a/gdb/solist.h b/gdb/solist.h index f0d22080de1..bd60987f49c 100644 --- a/gdb/solist.h +++ b/gdb/solist.h @@ -167,6 +167,23 @@ struct solib_ops NULL, in which case no specific preprocessing is necessary for this target. */ void (*handle_event) (void); + + /* Return an address within the inferior's address space which is known + to be part of SO. If there is no such address, or GDB doesn't know + how to figure out such an address then an empty optional is + returned. + + The returned address can be used when loading the shared libraries + for a core file. GDB knows the build-ids for (some) files mapped + into the inferior's address space, and knows the address ranges which + those mapped files cover. If GDB can figure out a representative + address for the library then this can be used to match a library to a + mapped file, and thus to a build-id. GDB can then use this + information to help locate the shared library objfile, if the objfile + is not in the expected place (as defined by the shared libraries file + name). */ + + std::optional (*find_solib_addr) (solib &so); }; /* A unique pointer to a so_list. */ @@ -186,4 +203,9 @@ extern gdb_bfd_ref_ptr solib_bfd_fopen (const char *pathname, int fd); /* Find solib binary file and open it. */ extern gdb_bfd_ref_ptr solib_bfd_open (const char *in_pathname); +/* A default implementation of the solib_ops::find_solib_addr callback. + This just returns an empty std::optional indicating GDB is + unable to find an address within the library SO. */ +extern std::optional default_find_solib_addr (solib &so); + #endif diff --git a/gdb/testsuite/gdb.base/solib-search.exp b/gdb/testsuite/gdb.base/solib-search.exp index 581046a1ed8..bbfe3ae719a 100644 --- a/gdb/testsuite/gdb.base/solib-search.exp +++ b/gdb/testsuite/gdb.base/solib-search.exp @@ -43,7 +43,11 @@ set right_binfile2_lib \ set binfile1_lib [standard_output_file ${libname1}.so] set binfile2_lib [standard_output_file ${libname2}.so] -set lib_flags [list debug ldflags=-Wl,-Bsymbolic] +# When this test was written, GDB's ability to track down shared +# libraries for a core file based on the build-id much poorer. As GDB +# has improved we now need to disable build-ids in order for this test +# to function as expected. +set lib_flags [list debug no-build-id ldflags=-Wl,-Bsymbolic] set wrong_lib_flags "$lib_flags additional_flags=-DARRAY_SIZE=1" set right_lib_flags "$lib_flags additional_flags=-DARRAY_SIZE=8192 additional_flags=-DRIGHT" diff --git a/gdb/testsuite/gdb.debuginfod/solib-with-soname-1.c b/gdb/testsuite/gdb.debuginfod/solib-with-soname-1.c new file mode 100644 index 00000000000..30e9267dc01 --- /dev/null +++ b/gdb/testsuite/gdb.debuginfod/solib-with-soname-1.c @@ -0,0 +1,39 @@ +/* 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 . */ + +#include + +/* It is important that these two variables have names of the same length + so that the debug information in the two library versions is laid out + the same. If they differ then the .dynamic section might move, which + will trigger a different check within GDB than the one we actually want + to check. */ + +#if LIB_VERSION == 1 +volatile int *library_1_var = (volatile int *) 0x12345678; +#elif LIB_VERSION == 2 +volatile int *library_2_var = (volatile int *) 0x11223344; +#else +# error Unknown library version +#endif + +int +foo (void) +{ + /* This should trigger a core dump. */ + abort (); + + return 0; +} diff --git a/gdb/testsuite/gdb.debuginfod/solib-with-soname-2.c b/gdb/testsuite/gdb.debuginfod/solib-with-soname-2.c new file mode 100644 index 00000000000..a51d48e2f2e --- /dev/null +++ b/gdb/testsuite/gdb.debuginfod/solib-with-soname-2.c @@ -0,0 +1,41 @@ +/* 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 . */ + +#include +#include +#include + +/* This is in the shared library. */ +extern int foo (void); + +/* This is updated by the .exp file. */ +char *libname = "libfoo_2.so"; + +int +main (void) +{ + void *handle; + int res, tmp; + + handle = dlopen (libname, RTLD_LAZY); + assert (handle != NULL); + + res = foo (); + + tmp = dlclose (handle); + assert (tmp == 0); + + return res; +} diff --git a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp new file mode 100644 index 00000000000..44c4268edd3 --- /dev/null +++ b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp @@ -0,0 +1,260 @@ +# 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 . */ + +# This test exercises GDB's ability to validate build-ids when loading +# shared libraries for a core file. +# +# The test creates two "versions" of a shared library, sets up a +# symlink to point to one version of the library, and creates a core file. +# +# We then try re-loading the core file and executable and check that +# GDB is able to correctly load the shared library. To confuse things +# we retarget the library symlink at the other version of the library. +# +# After that we repeat the test, but this time deleting the symlink +# completely. +# +# Then we remove the version of the library completely, at this point +# we do expect GDB to give a warning about being unable to load the library. +# +# And finally, we setup debuginfod and have it serve the missing +# library file, GDB should correctly download the library file. +# +# Despite this test living in the gdb.debuginfod/ directory, only the last +# part of this test actually uses debuginfod, everything up to that point is +# pretty generic. + +load_lib debuginfod-support.exp + +require allow_shlib_tests +require {istarget "*-linux*"} +require {!is_remote host} +require {!using_fission} + +standard_testfile -1.c -2.c + +# Build two similar, but slightly different versions of the shared +# library. Both libraries have DT_SONAME set to the generic +# libfoo.so, we'll create a symlink with that name later. +set library_1_filename [standard_output_file "libfoo_1.so"] +set library_2_filename [standard_output_file "libfoo_2.so"] + +# The generic name for the library. +set library_filename [standard_output_file "libfoo.so"] + +# When compiling a shared library the -Wl,-soname,NAME option is +# automatically added based on the final name of the library. We want +# to compile libfoo_1.so, but set the soname to libfoo.so. To achieve +# this we first compile into libfoo.so, and then rename the library to +# libfoo_1.so. +if {[build_executable "build libfoo_1.so" $library_filename \ + $srcfile \ + { debug shlib build-id \ + additional_flags=-DLIB_VERSION=1 }] == -1} { + return +} +remote_exec build "mv ${library_filename} ${library_1_filename}" + +# See the comment above, but this time we rename to libfoo_2.so. +if {[build_executable "build libfoo_2.so" $library_filename \ + $srcfile \ + { debug shlib build-id \ + additional_flags=-DLIB_VERSION=2 }] == -1} { + return +} +remote_exec build "mv ${library_filename} ${library_2_filename}" + +# Create libfoo.so symlink to the libfoo_1.so library. If this +# symlink creation fails then we assume we can't create symlinks on +# this host. If this succeeds then later symlink creation is required +# to succeed, and will trigger an FAIL if it doesn't. +set status \ + [remote_exec build \ + "ln -sf ${library_1_filename} ${library_filename}"] +if {[lindex $status 0] != 0} { + unsupported "host does not support symbolic links" + return +} + +# Build the executable. This links against libfoo.so, which is +# poining at libfoo_1.so. Just to confuse things even more, this +# executable uses dlopen to load libfoo_2.so. Weird! +if { [build_executable "build executable" ${binfile} ${srcfile2} \ + [list debug shlib=${library_filename} shlib_load]] == -1 } { + return +} + +# If the board file is automatically splitting the debug information +# into a separate file (e.g. the cc-with-gnu-debuglink.exp board) then +# this test isn't going to work. +clean_restart +gdb_file_cmd $binfile +if {$gdb_file_cmd_debug_info ne "debug"} { + unsupported "failed to find debug information" + return +} +if {[regexp "${testfile}.debug" $gdb_file_cmd_msg]} { + unsupported "debug information has been split to a separate file" + return +} + +# Run BINFILE which will generate a corefile. +set corefile [core_find $binfile] +if {$corefile eq ""} { + untested "could not generate core file" + return +} + +# Helper proc to load global BINFILE and then load global COREFILE. +# +# If EXPECT_WARNING is true then we require a warning about being +# unable to load the shared library symbols, otherwise, EXPECT_WARNING +# is false and we require no warning. +# +# If EXPECT_DOWNLOAD is true then we require a line indicating that +# the shared library is being downloaded from debuginfod, otherwise +# the shared library should not be downloaded. +proc load_exec_and_core_file { expect_warning expect_download testname } { + with_test_prefix $testname { + clean_restart $::binfile + + set saw_warning false + set saw_download false + set saw_generated false + set saw_terminated false + + gdb_test_multiple "core-file $::corefile" "load core file" { + -re "^Core was generated by \[^\r\n\]+\r\n" { + set saw_generated true + exp_continue + } + -re "^Program terminated with signal \[^\r\n\]+\r\n" { + set saw_terminated true + exp_continue + } + -re "^warning: Can't open file \[^\r\n\]+ during file-backed mapping note processing\r\n" { + # Ignore warnings from the file backed mapping phase. + exp_continue + } + -re "^warning: Could not load shared library symbols for \[^\r\n\]+/libfoo\\.so\\.\r\nDo you need \"set solib-search-path\" or \"set sysroot\".\r\n" { + set saw_warning true + exp_continue + } + -re "^Downloading executable for \[^\r\n\]+/libfoo_1\\.so\\.\\.\\.\r\n" { + set saw_download true + exp_continue + } + -re "^$::gdb_prompt $" { + gdb_assert { $saw_generated && $saw_terminated \ + && $saw_warning == $expect_warning \ + && $saw_download == $expect_download } \ + $gdb_test_name + } + -re "^\[^\r\n\]*\r\n" { + exp_continue + } + } + + # If we don't expect a warning then debug symbols from the + # shared library should be available. Confirm we can read a + # variable from the shared library. If we do expect a warning + # then the shared library debug symbols have not loaded, and + # the library variable should not be available. + if { !$expect_warning } { + gdb_test "print/x library_1_var" " = 0x12345678" \ + "check library_1_var can be read" + } else { + gdb_test "print/x library_1_var" \ + "^No symbol \"library_1_var\" in current context\\." \ + "check library_1_var cannot be read" + } + } +} + +# Initial test, just load the executable and core file. At this point +# everything should load fine as everything is where we expect to find +# it. +load_exec_and_core_file false false \ + "load core file, all libraries as expected" + +# Update libfoo.so symlink to point at the second library then reload +# the core file. GDB should spot that the symlink points to the wrong +# file, but should be able to figure out the correct file to load as +# the right file will be in the mapped file list. +set status [remote_exec build \ + "ln -sf ${library_2_filename} ${library_filename}"] +gdb_assert { [lindex $status 0] == 0 } \ + "update library symlink to point to the wrong file" + +load_exec_and_core_file false false \ + "load core file, symlink points to wrong file" + +# Remove libfoo.so symlink and reload the core file. As in the +# previous test GDB should be able to figure out the correct file to +# load as the correct file will still appear in the mapped file list. +set status [remote_exec build "rm -f ${library_filename}"] +gdb_assert { [lindex $status 0] == 0 } "remove library symlink" + +load_exec_and_core_file false false \ + "load core file, symlink removed" + +# Remove LIBRARY_1_FILENAME. We'll now see a warning that the mapped +# file can't be loaded (we ignore that warning), and we'll see a +# warning that the shared library can't be loaded. +set library_1_backup_filename ${library_1_filename}.backup +set status \ + [remote_exec build \ + "mv ${library_1_filename} ${library_1_backup_filename}"] +gdb_assert { [lindex $status 0] == 0 } \ + "remove libfoo_1.so" + +load_exec_and_core_file true false \ + "load core file, libfoo_1.so removed" + +# Setup a debuginfod server which can serve the original shared +# library file. +if {![allow_debuginfod_tests]} { + untested "skippig debuginfod parts of this test" + return +} + +set server_dir [standard_output_file "debuginfod.server"] +file mkdir $server_dir +file rename -force $library_1_backup_filename $server_dir + +prepare_for_debuginfod cache db + +set url [start_debuginfod $db $server_dir] +if { $url eq "" } { + unresolved "failed to start debuginfod server" + return +} + +with_debuginfod_env $cache { + setenv DEBUGINFOD_URLS $url + + save_vars { GDBFLAGS } { + append GDBFLAGS " -ex \"set debuginfod enabled on\"" + + # Reload the executable and core file. GDB should download + # the file libfoo_1.so using debuginfod during the mapped file + # phase, but should then reuse that download during the shared + # library phase. + load_exec_and_core_file false true \ + "load core file, use debuginfod" + } +} + +stop_debuginfod diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 0d78691c381..c958ff18d2a 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -5211,6 +5211,7 @@ proc quote_for_host { args } { # debug information # - text_segment=addr: Tell the linker to place the text segment at ADDR. # - build-id: Ensure the final binary includes a build-id. +# - no-build-id: Ensure the final binary does not include a build-id. # - column-info/no-column-info: Enable/Disable generation of column table # information. # @@ -5316,6 +5317,18 @@ proc gdb_compile {source dest type options} { lappend new_options "additional_flags=-Wl,--build-id" } + # If the 'no-build-id' option is used then disable the build-id. + if {[lsearch -exact $options no-build-id] > 0} { + lappend new_options "additional_flags=-Wl,--build-id=none" + } + + # Sanity check. If both 'build-id' and 'no-build-id' are used + # then what is expected from us! + if {[lsearch -exact $options build-id] > 0 + && [lsearch -exact $options no-build-id] > 0} { + error "cannot use build-id and no-build-id options" + } + # Treating .c input files as C++ is deprecated in Clang, so # explicitly force C++ language. if { !$getting_compiler_info From patchwork Mon May 20 13:08:27 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 90484 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4DFE93858D34 for ; Mon, 20 May 2024 13:10:13 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id DC54D3858C53 for ; Mon, 20 May 2024 13:08:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DC54D3858C53 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 DC54D3858C53 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=1716210524; cv=none; b=hK77Vzlhs5QkK3Op/ruLicvrRLic0AExXKiO2ibivCw/CfizJoCMoKS4R4zVdSymFSZ3OWNkZrvIfdevj8N+7wKWpxE7v20LSKrwtu66HKCn6ezrg2yvtll2Y6SEMjzXIPGZ2JL8nmqmzc9UkzPNhs0KQokuF/6SFchhv/Vg+hE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716210524; c=relaxed/simple; bh=NgzeSHLteNf6HLoaMgmNgmXNcRCkNmvkHD6FW+By+dQ=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=DcnQGkDGj9AbehCso9CJUViNrAMlQiSE9MzADYE+myt7f6hIAyVxJFzjAFDjB1tuJVEwhOgBr/xN+QT1ZiQaRwPYr/nvpMHGSh8SDyZtLO2VuRzSnQ15CJs9ZxIpZ8p1ldWKuMOamE/4/23CgcnAi42/7TfSnG1QXD9032EHs2A= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1716210520; 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=uLiQRyXwITKF+xL2FN4HcKRegGB/r/oGZkKhd2WMYE4=; b=YqoI2IYclRLz0HJ4vhQkeGr6FbpiJOneq67edME5hZdTHAoh7ymb+E334mzbyMyypddsiw pPlNjL+kSYAi2yC9gUhhVsYYCNr8+P7QMHSpwlBO7vuvohqItdwdW1Xa3RtVxPPc7keWBo 7L8mihbG5quQSq5RUs8ybx730UzskrA= Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-35-1bRBW4AZPqm5LEDn8YpFeg-1; Mon, 20 May 2024 09:08:38 -0400 X-MC-Unique: 1bRBW4AZPqm5LEDn8YpFeg-1 Received: by mail-lf1-f70.google.com with SMTP id 2adb3069b0e04-523936877dbso4530646e87.2 for ; Mon, 20 May 2024 06:08:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716210516; x=1716815316; 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=uLiQRyXwITKF+xL2FN4HcKRegGB/r/oGZkKhd2WMYE4=; b=UK0Gytg5J9tQjDaL/gr1jbaCCHNj0n84GzNFyCmtI+sCI0Jg7yweVLIZUfaKwCRlhZ UYmBP7vff9sUfCLg3ZTrn/qMxmc40lnIIkMhxhpGs/TuykmVFjeInIUefwAzvab6/mt4 L5/n/f5u1OJNOx7w/PvDr1Vagwld/Pit7oPZ/r1NW64YxHSUja0zTB7WG4+U/Dmi1rgx xc9VAtR4VvaS3OBjskbpOAxW0pE+24dg8fRI6BtYKU1i9hw/IohlVeNXdpvl0QFXrWyL dNWaXQTFNPAkT4Vr/xAbkyxLlnXhH5igRgxvdjWdpjkAFeQQ9btQddFdB3KWxU5pOgbj 2iBw== X-Gm-Message-State: AOJu0YwAPw6IiWkyoUqNOuTYMacu8d4nwAOVznujCo/j0nEU67y52ucF xUopTlYPp9Fw8l58yU3oRKJoLrIibRtpw500P4FE7AgvoJ0uvv+Me43IS2prz7mKh1KaxRlfYli GX5Kgx0Kjhcb25H0Kor6AX6iDfUoN47j6faJ9g3sby/GZYMSAokooTyp23QIR+X0pu4uaT5Q6RA HQQ1DjCS9rgc9qDqK6nizzW33lxoo58J38avIfE+WJBuQ= X-Received: by 2002:a19:5f5a:0:b0:51e:1146:16fd with SMTP id 2adb3069b0e04-5220fb768c0mr19551594e87.9.1716210515943; Mon, 20 May 2024 06:08:35 -0700 (PDT) X-Google-Smtp-Source: AGHT+IH+BH7f5B1HBfbEYVItKNeZx1J77s17GtFay+F65bpz3pTVkmqwPX3QRPyS8f+c4xc1SQ765Q== X-Received: by 2002:a19:5f5a:0:b0:51e:1146:16fd with SMTP id 2adb3069b0e04-5220fb768c0mr19551564e87.9.1716210515228; Mon, 20 May 2024 06:08:35 -0700 (PDT) Received: from localhost ([31.111.84.240]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-574bcad0362sm11621270a12.20.2024.05.20.06.08.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 May 2024 06:08:34 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 4/4] gdb: unify build-id to objfile lookup code Date: Mon, 20 May 2024 14:08:27 +0100 Message-Id: <2bc90e98483e0849b627cd2ca6de5f775e24d702.1716210406.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 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_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP 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.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org There are 3 places where we currently call debuginfod_exec_query to lookup an objfile for a given build-id. In one of these places we first call build_id_to_exec_bfd which also looks up an objfile given a build-id, but this function looks on disk for a symlink in the .build-id/ sub-directory (within the debug-file-directory). I can't think of any reason why we shouldn't call build_id_to_exec_bfd before every call to debuginfod_exec_query. So, in this commit I have added a new function in build-id.c, find_exec_by_build_id, this function calls build_id_to_exec_bfd, and if that fails, then calls debuginfod_exec_query. Everywhere we call debuginfod_exec_query is updated to call the new function, and in locate_exec_from_corefile_build_id, the existing call to build_id_to_exec_bfd is removed as calling find_exec_by_build_id does this for us. One slight weird thing is in core_target::build_file_mappings, here we call find_exec_by_build_id which returns a gdb_bfd_ref_ptr for the opened file, however we immediately reopen the file as "binary". The reason for this is that all the bfds opened in ::build_file_mappings need to be opened as "binary" (see the function comments for why). I did consider passing a target type into find_exec_by_build_id, which could then be forwarded to build_id_to_exec_bfd and used to open the BFD as "binary", however, if you follow the call chain you'll end up in build_id_to_debug_bfd_1, where we actually open the bfd. Notice in here that we call build_id_verify to double check the build-id of the file we found, this requires that the bfd not be opened as "binary". What this means is that we always have to first open the bfd using the gnutarget target type (for the build-id check), and then we would have to reopen it as "binary". There seems little point pushing the reopen logic into find_exec_by_build_id, so we just do this in the ::build_file_mappings function. I've extended the tests to cover the two cases which actually changed in this commit. --- gdb/build-id.c | 42 ++++++++++++++++++- gdb/build-id.h | 21 ++++++---- gdb/corelow.c | 40 ++++++------------ gdb/solib.c | 22 ++++------ .../gdb.debuginfod/corefile-mapped-file.exp | 24 +++++++++++ .../gdb.debuginfod/solib-with-soname.exp | 32 +++++++++++++- gdb/testsuite/lib/gdb.exp | 7 +++- 7 files changed, 134 insertions(+), 54 deletions(-) diff --git a/gdb/build-id.c b/gdb/build-id.c index 41667d5e5cf..27642b58d56 100644 --- a/gdb/build-id.c +++ b/gdb/build-id.c @@ -26,6 +26,8 @@ #include "filenames.h" #include "gdbcore.h" #include "cli/cli-style.h" +#include "gdbsupport/scoped_fd.h" +#include "debuginfod-support.h" /* See build-id.h. */ @@ -198,9 +200,11 @@ build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id) return build_id_to_bfd_suffix (build_id_len, build_id, ".debug"); } -/* See build-id.h. */ +/* Find and open a BFD for an executable file given a build-id. If no BFD + can be found, return NULL. The returned reference to the BFD must be + released by the caller. */ -gdb_bfd_ref_ptr +static gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len, const bfd_byte *build_id) { return build_id_to_bfd_suffix (build_id_len, build_id, ""); @@ -243,3 +247,37 @@ find_separate_debug_file_by_buildid (struct objfile *objfile, return std::string (); } + +/* See build-id.h. */ + +gdb_bfd_ref_ptr +find_exec_by_build_id (const bfd_build_id *build_id, + const char *expected_filename) +{ + /* Try to find the executable (or shared object) by looking for a + (sym)link on disk from the build-id to the object file. */ + gdb_bfd_ref_ptr abfd = build_id_to_exec_bfd (build_id->size, + build_id->data); + + if (abfd != nullptr) + return abfd; + + /* Attempt to query debuginfod for the executable. */ + gdb::unique_xmalloc_ptr path; + scoped_fd fd = debuginfod_exec_query (build_id->data, build_id->size, + expected_filename, &path); + if (fd.get () >= 0) + { + abfd = gdb_bfd_open (path.get (), gnutarget); + + if (abfd == nullptr) + warning (_("\"%ps\" from debuginfod cannot be opened as bfd: %s"), + styled_string (file_name_style.style (), path.get ()), + gdb_bfd_errmsg (bfd_get_error (), nullptr).c_str ()); + else if (!build_id_verify (abfd.get (), build_id->size, + build_id->data)) + abfd = nullptr; + } + + return abfd; +} diff --git a/gdb/build-id.h b/gdb/build-id.h index c5f20f8782e..3df122a0cbf 100644 --- a/gdb/build-id.h +++ b/gdb/build-id.h @@ -40,13 +40,6 @@ extern int build_id_verify (bfd *abfd, extern gdb_bfd_ref_ptr build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id); -/* Find and open a BFD for an executable file given a build-id. If no BFD - can be found, return NULL. The returned reference to the BFD must be - released by the caller. */ - -extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len, - const bfd_byte *build_id); - /* Find the separate debug file for OBJFILE, by using the build-id associated with OBJFILE's BFD. If successful, returns the file name for the separate debug file, otherwise, return an empty string. @@ -60,6 +53,20 @@ extern gdb_bfd_ref_ptr build_id_to_exec_bfd (size_t build_id_len, extern std::string find_separate_debug_file_by_buildid (struct objfile *objfile, deferred_warnings *warnings); +/* Find an executable (or shared library) that matches BUILD_ID. This is + done by first checking in the debug-file-directory for a .build-id/ + sub-directory, and looking for a symlink in there that points to the + required file. + + If that doesn't find us a file then we call to debuginfod to see if it + can provide the required file. + + EXPECTED_FILENAME is used in output messages from debuginfod, this + should be the file we were looking for but couldn't find. */ + +extern gdb_bfd_ref_ptr find_exec_by_build_id (const bfd_build_id *build_id, + const char *expected_filename); + /* Return an hex-string representation of BUILD_ID. */ static inline std::string diff --git a/gdb/corelow.c b/gdb/corelow.c index 85bc3c26bea..6975719c1f2 100644 --- a/gdb/corelow.c +++ b/gdb/corelow.c @@ -47,7 +47,6 @@ #include "gdbsupport/pathstuff.h" #include "gdbsupport/scoped_fd.h" #include "gdbsupport/x86-xstate.h" -#include "debuginfod-support.h" #include #include #include "cli/cli-cmds.h" @@ -407,13 +406,19 @@ core_target::build_file_mappings () || !bfd_check_format (abfd.get (), bfd_object)) && file_data.build_id != nullptr) { - expanded_fname = nullptr; - debuginfod_exec_query (file_data.build_id->data, - file_data.build_id->size, - filename.c_str (), &expanded_fname); - if (expanded_fname != nullptr) + abfd = find_exec_by_build_id (file_data.build_id, + filename.c_str ()); + + if (abfd != nullptr) { + /* The find_exec_by_build_id will have opened ABFD using the + GNUTARGET global bfd type, however, we need the bfd opened + as the binary type (see the function's header comment), so + now we reopen ABFD with the desired binary type. */ + expanded_fname + = make_unique_xstrdup (bfd_get_filename (abfd.get ())); struct bfd *b = bfd_openr (expanded_fname.get (), "binary"); + gdb_assert (b != nullptr); abfd = gdb_bfd_ref_ptr::new_reference (b); } } @@ -774,28 +779,7 @@ locate_exec_from_corefile_build_id (bfd *abfd, int from_tty) return; gdb_bfd_ref_ptr execbfd - = build_id_to_exec_bfd (build_id->size, build_id->data); - - if (execbfd == nullptr) - { - /* Attempt to query debuginfod for the executable. */ - gdb::unique_xmalloc_ptr execpath; - scoped_fd fd = debuginfod_exec_query (build_id->data, build_id->size, - abfd->filename, &execpath); - - if (fd.get () >= 0) - { - execbfd = gdb_bfd_open (execpath.get (), gnutarget); - - if (execbfd == nullptr) - warning (_("\"%s\" from debuginfod cannot be opened as bfd: %s"), - execpath.get (), - gdb_bfd_errmsg (bfd_get_error (), nullptr).c_str ()); - else if (!build_id_verify (execbfd.get (), build_id->size, - build_id->data)) - execbfd.reset (nullptr); - } - } + = find_exec_by_build_id (build_id, abfd->filename); if (execbfd != nullptr) { diff --git a/gdb/solib.c b/gdb/solib.c index c39dfbcc78e..3292f361176 100644 --- a/gdb/solib.c +++ b/gdb/solib.c @@ -45,7 +45,6 @@ #include "gdb_bfd.h" #include "gdbsupport/filestuff.h" #include "gdbsupport/scoped_fd.h" -#include "debuginfod-support.h" #include "source.h" #include "cli/cli-style.h" @@ -826,20 +825,15 @@ solib_map_sections (solib &so) abfd = nullptr; if (abfd == nullptr) - { - scoped_fd fd = debuginfod_exec_query - (expected_build_id->data, expected_build_id->size, - so.so_name.c_str (), &filename); + abfd = find_exec_by_build_id (expected_build_id, + so.so_name.c_str ()); - if (fd.get () >= 0) - abfd = ops->bfd_open (filename.get ()); - else if (mismatch) - { - warning (_ ("Build-id of %ps does not match core file."), - styled_string (file_name_style.style (), - filename.get ())); - abfd = nullptr; - } + if (abfd == nullptr && mismatch) + { + warning (_ ("Build-id of %ps does not match core file."), + styled_string (file_name_style.style (), + filename.get ())); + abfd = nullptr; } } } diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp index 6e3301e1c8d..b5dee228ca0 100644 --- a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp +++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp @@ -296,6 +296,30 @@ set ptr_value [read_ptr_value] gdb_assert { $ptr_value eq "unavailable" } \ "check value of pointer is unavailable with library file missing" +# Now symlink the .build-id/xx/xxx...xxx filename within the debug +# directory to library we just moved aside. Restart GDB and setup the +# debug-file-directory before loading the core file. +# +# GDB should lookup the file to map via the build-id link in the +# .build-id/ directory. +set debugdir [standard_output_file "debugdir"] +set build_id_filename \ + $debugdir/[build_id_debug_filename_get $library_backup_filename ""] + +remote_exec build "mkdir -p [file dirname $build_id_filename]" +remote_exec build "ln -sf $library_backup_filename $build_id_filename" + +clean_restart $binfile + +gdb_test_no_output "set debug-file-directory $debugdir" \ + "set debug-file-directory" + +load_core_file "load corefile, lookup in debug-file-directory" + +set ptr_value [read_ptr_value] +gdb_assert { $ptr_value == $ptr_expected_value } \ + "check value of pointer variable from core-file, lookup in debug-file-directory" + # Build a new version of the shared library, keep the library the same size, # but change the contents so the build-id changes. Then restart GDB and load # the core-file again. GDB should spot that the build-id for the shared diff --git a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp index 44c4268edd3..cdd9730a22c 100644 --- a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp +++ b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp @@ -126,10 +126,19 @@ if {$corefile eq ""} { # If EXPECT_DOWNLOAD is true then we require a line indicating that # the shared library is being downloaded from debuginfod, otherwise # the shared library should not be downloaded. -proc load_exec_and_core_file { expect_warning expect_download testname } { +# +# If DEBUGDIR is not the empty string then 'debug-file-directory' is +# set to the value of DEBUGDIR. +proc load_exec_and_core_file { expect_warning expect_download testname \ + {debugdir ""} } { with_test_prefix $testname { clean_restart $::binfile + if { $debugdir ne "" } { + gdb_test_no_output "set debug-file-directory $debugdir" \ + "set debug directory" + } + set saw_warning false set saw_download false set saw_generated false @@ -223,6 +232,27 @@ gdb_assert { [lindex $status 0] == 0 } \ load_exec_and_core_file true false \ "load core file, libfoo_1.so removed" +# Symlink the .build-id/xx/xxx...xxx filename within the debug +# directory to LIBRARY_1_BACKUP_FILENAME, now when we restart GDB it +# should find the missing library within the debug directory. +set debugdir [standard_output_file "debugdir"] +set build_id_filename \ + $debugdir/[build_id_debug_filename_get $library_1_backup_filename ""] +set status \ + [remote_exec build \ + "mkdir -p [file dirname $build_id_filename]"] +gdb_assert { [lindex $status 0] == 0 } \ + "create sub-directory within the debug directory" +set status \ + [remote_exec build \ + "ln -sf $library_1_backup_filename $build_id_filename"] +gdb_assert { [lindex $status 0] == 0 } \ + "create symlink within the debug directory " + +load_exec_and_core_file false false \ + "load core file, find libfoo_1.so through debug-file-directory" \ + $debugdir + # Setup a debuginfod server which can serve the original shared # library file. if {![allow_debuginfod_tests]} { diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index c958ff18d2a..e369b0be96a 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -8019,14 +8019,17 @@ proc get_build_id { filename } { # Return the build-id hex string (usually 160 bits as 40 hex characters) # converted to the form: .build-id/ab/cdef1234...89.debug +# +# The '.debug' suffix can be changed by passing the SUFFIX argument. +# # Return "" if no build-id found. -proc build_id_debug_filename_get { filename } { +proc build_id_debug_filename_get { filename {suffix ".debug"} } { set data [get_build_id $filename] if { $data == "" } { return "" } regsub {^..} $data {\0/} data - return ".build-id/${data}.debug" + return ".build-id/${data}${suffix}" } # DEST should be a file compiled with debug information. This proc