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