Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP1:GA
gdb.12817
gdb-rhbz1371380-gcore-elf-headers.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File gdb-rhbz1371380-gcore-elf-headers.patch of Package gdb.12817
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior <sergiodj@redhat.com> Date: Tue, 23 Apr 2019 18:17:57 -0400 Subject: gdb-rhbz1371380-gcore-elf-headers.patch ;; Implement dump of mappings with ELF headers by gcore ;; RHBZ 1371380, Sergio Durigan Junior. Implement dump of mappings with ELF headers by gcore This patch has a long story, but it all started back in 2015, with commit df8411da087dc05481926f4c4a82deabc5bc3859 ("Implement support for checking /proc/PID/coredump_filter"). The purpose of that commit was to bring GDB's corefile generation closer to what the Linux kernel does. However, back then, I did not implement the full support for the dumping of memory mappings containing ELF headers (like mappings of DSOs or executables). These mappings were being dumped most of time, though, because the default value of /proc/PID/coredump_filter is 0x33, which would cause anonymous private mappings (DSOs/executable code mappings have this type) to be dumped. Well, until something happened on binutils... A while ago, I noticed something strange was happening with one of our local testcases on Fedora GDB: it was failing due to some strange build-id problem. On Fedora GDB, we (unfortunately) carry a bunch of "local" patches, and some of these patches actually extend upstream's build-id support in order to generate more useful information for the user of a Fedora system (for example, when the user loads a corefile into GDB, we detect whether the executable that generated that corefile is present, and if it's not we issue a warning suggesting that it should be installed, while also providing the build-id of the executable). A while ago, Fedora GDB stopped printing those warnings. I wanted to investigate this right away, and spent some time trying to determine what was going on, but other things happened and I got sidetracked. Meanwhile, the bug started to be noticed by some of our users, and its priority started changing. Then, someone on IRC also mentioned the problem, and when I tried helping him, I noticed he wasn't running Fedora. Hm... So maybe the bug was *also* present upstream. After "some" time investigating, and with a lot of help from Keith and others, I was finally able to determine that yes, the bug is also present upstream, and that even though it started with a change in ld, it is indeed a GDB issue. So, as I said, the problem started with binutils, more specifically after the following commit was pushed: commit f6aec96dce1ddbd8961a3aa8a2925db2021719bb Author: H.J. Lu <hjl.tools@gmail.com> Date: Tue Feb 27 11:34:20 2018 -0800 ld: Add --enable-separate-code This commit makes ld use "-z separate-code" by default on x86-64 machines. What this means is that code pages and data pages are now separated in the binary, which is confusing GDB when it tries to decide what to dump. BTW, Fedora 28 binutils doesn't have this code, which means that Fedora 28 GDB doesn't have the problem. From Fedora 29 on, binutils was rebased and incorporated the commit above, which started causing Fedora GDB to fail. Anyway, the first thing I tried was to pass "-z max-page-size" and specify a bigger page size (I saw a patch that did this and was proposed to Linux, so I thought it might help). Obviously, this didn't work, because the real "problem" is that ld will always use separate pages for code and data. So I decided to look into how GDB dumped the pages, and that's where I found the real issue. What happens is that, because of "-z separate-code", the first two pages of the ELF binary are (from /proc/PID/smaps): 00400000-00401000 r--p 00000000 fc:01 799548 /file Size: 4 kB KernelPageSize: 4 kB MMUPageSize: 4 kB Rss: 4 kB Pss: 4 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 4 kB Private_Dirty: 0 kB Referenced: 4 kB Anonymous: 0 kB LazyFree: 0 kB AnonHugePages: 0 kB ShmemPmdMapped: 0 kB Shared_Hugetlb: 0 kB Private_Hugetlb: 0 kB Swap: 0 kB SwapPss: 0 kB Locked: 0 kB THPeligible: 0 VmFlags: rd mr mw me dw sd 00401000-00402000 r-xp 00001000 fc:01 799548 /file Size: 4 kB KernelPageSize: 4 kB MMUPageSize: 4 kB Rss: 4 kB Pss: 4 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 0 kB Private_Dirty: 4 kB Referenced: 4 kB Anonymous: 4 kB LazyFree: 0 kB AnonHugePages: 0 kB ShmemPmdMapped: 0 kB Shared_Hugetlb: 0 kB Private_Hugetlb: 0 kB Swap: 0 kB SwapPss: 0 kB Locked: 0 kB THPeligible: 0 VmFlags: rd ex mr mw me dw sd Whereas before, we had only one: 00400000-00401000 r-xp 00000000 fc:01 798593 /file Size: 4 kB KernelPageSize: 4 kB MMUPageSize: 4 kB Rss: 4 kB Pss: 4 kB Shared_Clean: 0 kB Shared_Dirty: 0 kB Private_Clean: 0 kB Private_Dirty: 4 kB Referenced: 4 kB Anonymous: 4 kB LazyFree: 0 kB AnonHugePages: 0 kB ShmemPmdMapped: 0 kB Shared_Hugetlb: 0 kB Private_Hugetlb: 0 kB Swap: 0 kB SwapPss: 0 kB Locked: 0 kB THPeligible: 0 VmFlags: rd ex mr mw me dw sd Notice how we have "Anonymous" data mapped into the page. This will be important. So, the way GDB decides which pages it should dump has been revamped by my patch in 2015, and now it takes the contents of /proc/PID/coredump_filter into account. The default value for Linux is 0x33, which means: Dump anonymous private, anonymous shared, ELF headers and HugeTLB private pages. Or: filter_flags filterflags = (COREFILTER_ANON_PRIVATE | COREFILTER_ANON_SHARED | COREFILTER_ELF_HEADERS | COREFILTER_HUGETLB_PRIVATE); Now, it is important to keep in mind that GDB doesn't always have *all* of the necessary information to exactly determine the type of a page, so the whole algorithm is based on heuristics (you can take a look at linux-tdep.c:dump_mapping_p and linux-tdep.c:linux_find_memory_regions_full for more info). Before the patch to make ld use "-z separate-code", the (single) page containing data and code was being flagged as an anonymous (due to the non-zero "Anonymous:" field) private (due to the "r-xp" permission), which means that it was being dumped into the corefile. That's why it was working fine. Now, as you can imagine, when "-z separate-code" is used, the *data* page (which is where the ELF notes are, including the build-id one) now doesn't have any "Anonymous:" mapping, so the heuristic is flagging it as file-backed private, which is *not* dumped by default. The next question I had to answer was: how come a corefile generated by the Linux kernel was correct? Well, the answer is that GDB, unlike Linux, doesn't actually implement the COREFILTER_ELF_HEADERS support. On Linux, even though the data page is also treated as a file-backed private mapping, it is also checked to see if there are any ELF headers in the page, and then, because we *do* have ELF headers there, it is dumped. So, after more time trying to think of ways to fix this, I was able to implement an algorithm that reads the first few bytes of the memory mapping being processed, and checks to see if the ELF magic code is present. This is basically what Linux does as well, except that, if it finds the ELF magic code, it just dumps one page to the corefile, whereas GDB will dump the whole mapping. But I don't think that's a big issue, to be honest. It's also important to explain that we *only* perform the ELF magic code check if: - The algorithm has decided *not* to dump the mapping so far, and; - The mapping is private, and; - The mapping's offset is zero, and; - The user has requested us to dump mappings with ELF headers. IOW, we're not going to blindly check every mapping. As for the testcase, I struggled even more trying to write it. Since our build-id support on upstream GDB is not very extensive, it's not really possible to determine whether a corefile contains build-id information or not just by using GDB. So, after thinking a lot about the problem, I decided to rely on an external tool, eu-unstrip, in order to verify whether the dump was successful. I verified the test here on my machine, and everything seems to work as expected (i.e., it fails without the patch, and works with the patch applied). We are working hard to upstream our "local" Fedora GDB patches, and we intend to submit our build-id extension patches "soon", so hopefully we'll be able to use GDB itself to perform this verification. I built and regtested this on the BuildBot, and no problems were found. gdb/ChangeLog: 2019-04-25 Sergio Durigan Junior <sergiodj@redhat.com> PR corefiles/11608 PR corefiles/18187 * linux-tdep.c (dump_mapping_p): Add new parameters ADDR and OFFSET. Verify if current mapping contains an ELF header. (linux_find_memory_regions_full): Adjust call to dump_mapping_p. gdb/testsuite/ChangeLog: 2019-04-25 Sergio Durigan Junior <sergiodj@redhat.com> PR corefiles/11608 PR corefiles/18187 * gdb.base/coredump-filter-build-id.exp: New file. diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -591,8 +591,8 @@ mapping_is_anonymous_p (const char *filename) } /* Return 0 if the memory mapping (which is related to FILTERFLAGS, V, - MAYBE_PRIVATE_P, and MAPPING_ANONYMOUS_P) should not be dumped, or - greater than 0 if it should. + MAYBE_PRIVATE_P, MAPPING_ANONYMOUS_P, ADDR and OFFSET) should not + be dumped, or greater than 0 if it should. In a nutshell, this is the logic that we follow in order to decide if a mapping should be dumped or not. @@ -630,12 +630,17 @@ mapping_is_anonymous_p (const char *filename) see 'p' in the permission flags, then we assume that the mapping is private, even though the presence of the 's' flag there would mean VM_MAYSHARE, which means the mapping could still be private. - This should work OK enough, however. */ + This should work OK enough, however. + + - Even if, at the end, we decided that we should not dump the + mapping, we still have to check if it is something like an ELF + header (of a DSO or an executable, for example). If it is, and + if the user is interested in dump it, then we should dump it. */ static int dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v, int maybe_private_p, int mapping_anon_p, int mapping_file_p, - const char *filename) + const char *filename, ULONGEST addr, ULONGEST offset) { /* Initially, we trust in what we received from our caller. This value may not be very precise (i.e., it was probably gathered @@ -645,6 +650,7 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v, (assuming that the version of the Linux kernel being used supports it, of course). */ int private_p = maybe_private_p; + int dump_p; /* We always dump vDSO and vsyscall mappings, because it's likely that there'll be no file to read the contents from at core load time. @@ -685,13 +691,13 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v, /* This is a special situation. It can happen when we see a mapping that is file-backed, but that contains anonymous pages. */ - return ((filterflags & COREFILTER_ANON_PRIVATE) != 0 - || (filterflags & COREFILTER_MAPPED_PRIVATE) != 0); + dump_p = ((filterflags & COREFILTER_ANON_PRIVATE) != 0 + || (filterflags & COREFILTER_MAPPED_PRIVATE) != 0); } else if (mapping_anon_p) - return (filterflags & COREFILTER_ANON_PRIVATE) != 0; + dump_p = (filterflags & COREFILTER_ANON_PRIVATE) != 0; else - return (filterflags & COREFILTER_MAPPED_PRIVATE) != 0; + dump_p = (filterflags & COREFILTER_MAPPED_PRIVATE) != 0; } else { @@ -700,14 +706,55 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v, /* This is a special situation. It can happen when we see a mapping that is file-backed, but that contains anonymous pages. */ - return ((filterflags & COREFILTER_ANON_SHARED) != 0 - || (filterflags & COREFILTER_MAPPED_SHARED) != 0); + dump_p = ((filterflags & COREFILTER_ANON_SHARED) != 0 + || (filterflags & COREFILTER_MAPPED_SHARED) != 0); } else if (mapping_anon_p) - return (filterflags & COREFILTER_ANON_SHARED) != 0; + dump_p = (filterflags & COREFILTER_ANON_SHARED) != 0; else - return (filterflags & COREFILTER_MAPPED_SHARED) != 0; + dump_p = (filterflags & COREFILTER_MAPPED_SHARED) != 0; } + + /* Even if we decided that we shouldn't dump this mapping, we still + have to check whether (a) the user wants us to dump mappings + containing an ELF header, and (b) the mapping in question + contains an ELF header. If (a) and (b) are true, then we should + dump this mapping. + + A mapping contains an ELF header if it is a private mapping, its + offset is zero, and its first word is ELFMAG. */ + if (!dump_p && private_p && offset == 0 + && (filterflags & COREFILTER_ELF_HEADERS) != 0) + { + /* Let's check if we have an ELF header. */ + gdb::unique_xmalloc_ptr<char> header; + int errcode; + + /* Useful define specifying the size of the ELF magical + header. */ +#ifndef SELFMAG +#define SELFMAG 4 +#endif + + /* Read the first SELFMAG bytes and check if it is ELFMAG. */ + if (target_read_string (addr, &header, SELFMAG, &errcode) == SELFMAG + && errcode == 0) + { + const char *h = header.get (); + + /* The EI_MAG* and ELFMAG* constants come from + <elf/common.h>. */ + if (h[EI_MAG0] == ELFMAG0 && h[EI_MAG1] == ELFMAG1 + && h[EI_MAG2] == ELFMAG2 && h[EI_MAG3] == ELFMAG3) + { + /* This mapping contains an ELF header, so we + should dump it. */ + dump_p = 1; + } + } + } + + return dump_p; } /* Implement the "info proc" command. */ @@ -1311,7 +1358,7 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch, if (has_anonymous) should_dump_p = dump_mapping_p (filterflags, &v, priv, mapping_anon_p, mapping_file_p, - filename); + filename, addr, offset); else { /* Older Linux kernels did not support the "Anonymous:" counter. diff --git a/gdb/testsuite/gdb.base/coredump-filter-build-id.exp b/gdb/testsuite/gdb.base/coredump-filter-build-id.exp new file mode 100644 --- /dev/null +++ b/gdb/testsuite/gdb.base/coredump-filter-build-id.exp @@ -0,0 +1,69 @@ +# Copyright 2019 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 <http://www.gnu.org/licenses/>. + +# Test whether GDB's gcore/generate-core-file command can dump memory +# mappings with ELF headers, containing a build-id note. +# +# Due to the fact that we don't have an easy way to process a corefile +# and look for specific notes using GDB/dejagnu, we rely on an +# external tool, eu-unstrip, to verify if the corefile contains +# build-ids. + +standard_testfile "normal.c" + +# This test is Linux x86_64 only. +if { ![istarget *-*-linux*] } { + untested "$testfile.exp" + return -1 +} +if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } { + untested "$testfile.exp" + return -1 +} + +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } { + return -1 +} + +if { ![runto_main] } { + untested "could not run to main" + return -1 +} + +# First we need to generate a corefile. +set corefilename "[standard_output_file gcore.test]" +if { ![gdb_gcore_cmd "$corefilename" "save corefile"] } { + verbose -log "Could not save corefile" + untested "$testfile.exp" + return -1 +} + +# Determine if GDB dumped the mapping containing the build-id. This +# is done by invoking an external program (eu-unstrip). +if { [catch "exec [gdb_find_eu-unstrip] -n --core $corefilename" output] == 0 } { + set line [lindex [split $output "\n"] 0] + set test "gcore dumped mapping with build-id" + + verbose -log "First line of eu-unstrip: $line" + + if { [regexp "^${hex}\\+${hex} \[a-f0-9\]+@${hex}.*[string_to_regexp $binfile]$" $line] } { + pass "$test" + } else { + fail "$test" + } +} else { + verbose -log "Could not execute eu-unstrip program" + untested "$testfile.exp" +} diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp --- a/gdb/testsuite/lib/future.exp +++ b/gdb/testsuite/lib/future.exp @@ -162,6 +162,16 @@ proc gdb_find_readelf {} { return $readelf } +proc gdb_find_eu-unstrip {} { + global EU_UNSTRIP_FOR_TARGET + if [info exists EU_UNSTRIP_FOR_TARGET] { + set eu_unstrip $EU_UNSTRIP_FOR_TARGET + } else { + set eu_unstrip [transform eu-unstrip] + } + return $eu_unstrip +} + proc gdb_default_target_compile {source destfile type options} { global target_triplet global tool_root_dir
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
