130 lines
6.0 KiB
Diff
130 lines
6.0 KiB
Diff
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001
|
|
From: Keith Seitz <keiths@redhat.com>
|
|
Date: Mon, 27 Jul 2020 16:47:19 -0400
|
|
Subject: gdb-rhbz1842691-corefile-mem-access-2of15.patch
|
|
|
|
;; Adjust corefile.exp test to show regression after bfd hack removal
|
|
;; Kevin Buettner, RH BZ 1842691
|
|
|
|
Author: Kevin Buettner <kevinb@redhat.com>
|
|
Date: Tue May 12 17:44:19 2020 -0700
|
|
|
|
Adjust corefile.exp test to show regression after bfd hack removal
|
|
|
|
In his review of my BZ 25631 patch series, Pedro was unable to
|
|
reproduce the regression which should occur after patch #1, "Remove
|
|
hack for GDB which sets the section size to 0", is applied.
|
|
|
|
Pedro was using an ld version older than 2.30. Version 2.30
|
|
introduced the linker option -z separate-code. Here's what the man
|
|
page has to say about it:
|
|
|
|
Create separate code "PT_LOAD" segment header in the object. This
|
|
specifies a memory segment that should contain only instructions
|
|
and must be in wholly disjoint pages from any other data.
|
|
|
|
In ld version 2.31, use of separate-code became the default for
|
|
Linux/x86. So, really, 2.31 or later is required in order to see the
|
|
regression that occurs in recent Linux distributions when only the
|
|
bfd hack removal patch is applied.
|
|
|
|
For the test case in question, use of the separate-code linker option
|
|
means that the global variable "coremaker_ro" ends up in a separate
|
|
load segment (though potentially with other read-only data). The
|
|
upshot of this is that when only patch #1 is applied, GDB won't be
|
|
able to correctly access coremaker_ro. The reason for this is due
|
|
to the fact that this section will now have a non-zero size, but
|
|
will not have contents from the core file to find this data.
|
|
So GDB will ask BFD for the contents and BFD will respond with
|
|
zeroes for anything from those sections. GDB should instead be
|
|
looking in the executable for this data. Failing that, it can
|
|
then ask BFD for a reasonable value. This is what a later patch
|
|
in this series does.
|
|
|
|
When using ld versions earlier than 2.31 (or 2.30 w/ the
|
|
-z separate-code option explicitly provided to the linker), there is
|
|
the possibility that coremaker_ro ends up being placed near other data
|
|
which is recorded in the core file. That means that the correct value
|
|
will end up in the core file, simply because it resides on a page that
|
|
the kernel chooses to put in the core file. This is why Pedro wasn't
|
|
able to reproduce the regression that should occur after fixing the
|
|
BFD hack.
|
|
|
|
This patch places a big chunk of memory, two pages worth on x86, in
|
|
front of "coremaker_ro" to attempt to force it onto another page
|
|
without requiring use of that new-fangled linker switch.
|
|
|
|
Speaking of which, I considered changing the test to use
|
|
-z separate-code, but this won't work because it didn't
|
|
exist prior to version 2.30. The linker would probably complain
|
|
of an unrecognized switch. Also, it likely won't be available in
|
|
other linkers not based on current binutils. I.e. it probably won't
|
|
work in FreeBSD, NetBSD, etc.
|
|
|
|
To make this more concrete, this is what *should* happen when
|
|
attempting to access coremaker_ro when only patch #1 is applied:
|
|
|
|
Core was generated by `/mesquite2/sourceware-git/f28-coresegs/bld/gdb/testsuite/outputs/gdb.base/coref'.
|
|
Program terminated with signal SIGABRT, Aborted.
|
|
#0 0x00007f68205deefb in raise () from /lib64/libc.so.6
|
|
(gdb) p coremaker_ro
|
|
$1 = 0
|
|
|
|
Note that this result is wrong; 201 should have been printed instead.
|
|
But that's the point of the rest of the patch series.
|
|
|
|
However, without this commit, or when using an old Linux distro with
|
|
a pre-2.31 ld, this is what you might see instead:
|
|
|
|
Core was generated by `/mesquite2/sourceware-git/f28-coresegs/bld/gdb/testsuite/outputs/gdb.base/coref'.
|
|
Program terminated with signal SIGABRT, Aborted.
|
|
#0 0x00007f63dd658efb in raise () from /lib64/libc.so.6
|
|
(gdb) p coremaker_ro
|
|
$1 = 201
|
|
|
|
I.e. it prints the right answer, which sort of makes it seem like the
|
|
rest of the series isn't required.
|
|
|
|
Now, back to the patch itself... what should be the size of the memory
|
|
chunk placed before coremaker_ro?
|
|
|
|
It needs to be at least as big as the page size (PAGE_SIZE) from
|
|
the kernel. For x86 and several other architectures this value is
|
|
4096. I used MAPSIZE which is defined to be 8192 in coremaker.c.
|
|
So it's twice as big as what's currently needed for most Linux
|
|
architectures. The constant PAGE_SIZE is available from <sys/user.h>,
|
|
but this isn't portable either. In the end, it seemed simpler to
|
|
just pick a value and hope that it's big enough. (Running a separate
|
|
program which finds the page size via sysconf(_SC_PAGESIZE) and then
|
|
passes it to the compilation via a -D switch seemed like overkill
|
|
for a case which is rendered moot by recent linker versions.)
|
|
|
|
Further information can be found here:
|
|
|
|
https://sourceware.org/pipermail/gdb-patches/2020-May/168168.html
|
|
https://sourceware.org/pipermail/gdb-patches/2020-May/168170.html
|
|
|
|
Thanks to H.J. Lu for telling me about the '-z separate-code' linker
|
|
switch.
|
|
|
|
gdb/testsuite/ChangeLog:
|
|
|
|
* gdb.base/coremaker.c (filler_ro): New global constant.
|
|
|
|
diff --git a/gdb/testsuite/gdb.base/coremaker.c b/gdb/testsuite/gdb.base/coremaker.c
|
|
--- a/gdb/testsuite/gdb.base/coremaker.c
|
|
+++ b/gdb/testsuite/gdb.base/coremaker.c
|
|
@@ -42,6 +42,12 @@ char *buf2;
|
|
int coremaker_data = 1; /* In Data section */
|
|
int coremaker_bss; /* In BSS section */
|
|
|
|
+/* Place a chunk of memory before coremaker_ro to improve the chances
|
|
+ that coremaker_ro will end up on it's own page. See:
|
|
+
|
|
+ https://sourceware.org/pipermail/gdb-patches/2020-May/168168.html
|
|
+ https://sourceware.org/pipermail/gdb-patches/2020-May/168170.html */
|
|
+const unsigned char filler_ro[MAPSIZE] = {1, 2, 3, 4, 5, 6, 7, 8};
|
|
const int coremaker_ro = 201; /* In Read-Only Data section */
|
|
|
|
/* Note that if the mapping fails for any reason, we set buf2
|