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 {} {