[Bug corefiles/25631] GDB cannot access unwritten-to mmap'd buffer from core file

["cvs-commit at gcc dot gnu.org" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=25631

--- Comment #3 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Kevin Buettner <kevinb@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=678c7a56ced1828d37a554ec97f672496f135054

commit 678c7a56ced1828d37a554ec97f672496f135054
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.