Blame SOURCES/gdb-rhbz1842691-corefile-mem-access-2of15.patch

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