Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Kevin Buettner <kevinb@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH v2 4/5] Provide access to non SEC_HAS_CONTENTS core file sections
Date: Thu, 21 May 2020 13:40:43 +0100	[thread overview]
Message-ID: <8d80af1d-ef03-2db7-6dd0-4e7fa8dba283@redhat.com> (raw)
In-Reply-To: <20200521005020.697d1bf1@f31-4.lan>

On 5/21/20 8:50 AM, Kevin Buettner wrote:
> On Wed, 20 May 2020 17:45:28 +0100
> Pedro Alves <palves@redhat.com> wrote:
> 
>> So, as discussed at <https://sourceware.org/pipermail/gdb-patches/2020-May/168826.html>,
>> I still think that this is an heuristic, and that we're trading one
>> bug for another.  It does seem like the bug that we're fixing
>> is more important than the one we're introducing, so if there's really
>> nothing else in the core file that would distinguish the coremaker_ro
>> case from an anonymous mapping that should be read from the core (i.e.,
>> read as zeroes), then I suppose this is the best we can do, and the
>> patch looks good to me.  I would like to see the commit log and the
>> comments slightly adjusted to mention this, though.
> 
> On Linux, we're able to do "info proc mappings" when looking at a
> corefile.  Linux core files include a .note.linuxcore.file/pid section
> which contain the map data that you see when running "info proc
> mappings".
> 
> It may be possible to use these mappings to provide a more accurate
> view of memory contents when working with a core file.  In addition
> to file backed data that we already know about from the executable and
> shared libraries, there may be additional file backed data that can
> be found via files listed in the .note section mentioned earlier.  Or,
> as I found while investigating that one of those weird F27 mappings,
> there may be data from a shared library which can't be found in
> what GDB knows about when loading the shared library.  (In that
> particular case, the memory region in question contained some
> vtable data prior to being remapped - though I'm guessing about
> the remapping bit.)
> 
> If you think it worth pursuing, I can look at using the core-provided
> proc mapping information to provide a more accurate view of memory
> when looking at a core dump.
> 
> If I do this, is it still worth pushing this v2 series first?  Or should
> it wait until I attempt to incorporate info from the .note section?
> 
>> Could you remind me again why is it that the core dump includes a
>> load segment for coremaker_ro in the first place?  If this is
>> read-only data, why did the kernel decide to output a bss-like
>> load segment for it?
> 
> On F27 and F28, coremaker_ro was placed on a page that also had
> read/write data; it got included in with that data when the core file
> was written.
> 

Ah, right.  In this case, reading from the core should always work.

> On F29 and beyond (due to use of particular bintutils verions/options
> which participated in making the executable), coremaker_ro was placed
> on a page with only read-only data.  In this case, the core file
> didn't include data for that page; it lets the debugger find that data
> from the executable.

OK.  

I think I asked the wrong question.  Let's go back to what you described
in the commit log:

>    FAIL: gdb.base/corefile.exp: print coremaker_ro
>   
>    The reason for this is that all sections which have the BFD flag
>    SEC_ALLOC set, but for which SEC_HAS_CONTENTS is not set no longer
>    have zero size.  Some of these sections have data that can (and should)
>    be read from the executable.  (Sections for which SEC_HAS_CONTENTS
>    is set should be read from the core file; sections which do not have
>    this flag set need to either be read from the executable or, failing
>    that, from the core file using whatever BFD decides is the best value
>    to present to the user - it uses zeros.)
>    
>    At present, due to the way that the target strata are traversed when
>    attempting to access memory, the non-SEC_HAS_CONTENTS sections will be
>    read as zeroes from the process_stratum (which in this case is the
>    core file stratum) without first checking the file stratum, which is
>    where the data might actually be found.
>    
>    What we should be doing is this:
>    
>    - Attempt to access core file data for SEC_HAS_CONTENTS sections.
>    - Attempt to access executable file data if the above fails.
>    - Attempt to access core file data for non SEC_HAS_CONTENTS sections, if
>      both of the above fail.

The question should have been -- why is there a !SEC_HAS_CONTENTS load segment
for the page that contains coremaker_ro in the core file, at all?  
Or rather, why did the kernel decide to output a .bss load segment with
no contents for that page?  If there wasn't one, then we wouldn't need this
heuristic.  This is looking like a kernel bug to me.  Like, if the mapping
was file backed and wasn't touched, then it should have been skipped.
If it was touched (or for some other reason that could justify dumping the
mapping), then the kernel should have included its current contents
in the dump, instead of indicating "no contents".  No?

Thanks,
Pedro Alves



  reply	other threads:[~2020-05-21 12:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 17:11 [PATCH v2 0/5] Fix BZ 25631 - core file memory access problem Kevin Buettner
2020-05-13 17:11 ` [PATCH v2 1/5] Remove hack for GDB which sets the section size to 0 Kevin Buettner
2020-05-13 17:11 ` [PATCH v2 2/5] Adjust corefile.exp test to show regression after bfd hack removal Kevin Buettner
2020-05-20 16:24   ` Pedro Alves
2020-05-13 17:11 ` [PATCH v2 3/5] section_table_xfer_memory: Replace section name with callback predicate Kevin Buettner
2020-05-20 16:33   ` Pedro Alves
2020-05-13 17:11 ` [PATCH v2 4/5] Provide access to non SEC_HAS_CONTENTS core file sections Kevin Buettner
2020-05-20 16:45   ` Pedro Alves
2020-05-21  7:50     ` Kevin Buettner
2020-05-21 12:40       ` Pedro Alves [this message]
2020-05-21 14:23         ` Pedro Alves
2020-05-21 15:09           ` Kevin Buettner
2020-05-21 16:28             ` Pedro Alves
2020-05-21 17:06       ` Pedro Alves
2020-05-13 17:11 ` [PATCH v2 5/5] Test ability to access unwritten-to mmap data in core file Kevin Buettner
2020-05-20 16:46   ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8d80af1d-ef03-2db7-6dd0-4e7fa8dba283@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=kevinb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox