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

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