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

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