[1/4] Remove hack for GDB which sets the section size to 0

Message ID 20200305004243.334607-2-kevinb@redhat.com
State New
Headers show
Series
  • Fix BZ 25631 - core file memory access problem
Related show

Commit Message

Kevin Buettner March 5, 2020, 12:42 a.m.
[Note: This patch will require approval from binutils maintainers.
I'll send this patch to that list separately, but include it in this
series for completeness.]

This commit removes a hack for GDB which was introduced in 2007.
See:

    https://sourceware.org/ml/binutils/2007-08/msg00044.html

That hack mostly allowed GDB's handling of core files to continue to
work without any changes to GDB.

The problem with setting the section size to zero is that GDB won't
know how big that section is/was.  Often, this doesn't matter because
the data in question are found in the exec file.  But it can happen
that the section describes memory that had been allocated, but never
written to.  In this instance, the contents of that memory region are
not written to the core file.  Also, since the region in question was
dynamically allocated, it won't appear in the exec file.  We don't
want these regions to appear as inaccessible to GDB (since they *were*
accessible when the process was live), so it's important that GDB know
the size of the region.

I've made changes to GDB which correctly handles this case.  When
attempting to access memory, GDB will first consider core file data
for which both SEC_ALLOC and SEC_HAS_CONTENTS is set.  Next, if that
fails, GDB will attempt to find the data in the exec file.  Finally,
if that also fails, GDB will attempt to access memory in the sections
which are flagged as SEC_ALLOC, but not SEC_HAS_CONTENTS.

bfd/ChangeLog:

	* elf.c (_bfd_elf_make_section_from_phdr): Remove hack for GDB.

Change-Id: I7cce707aa3c217addbc27589730a77620199843f
---
 bfd/elf.c | 8 --------
 1 file changed, 8 deletions(-)

-- 
2.24.1

Comments

Luis Machado via Gdb-patches March 18, 2020, 4:28 p.m. | #1
FYI, Nick Clifton has approved this patch.

On Wed,  4 Mar 2020 17:42:40 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> [Note: This patch will require approval from binutils maintainers.

> I'll send this patch to that list separately, but include it in this

> series for completeness.]

> 

> This commit removes a hack for GDB which was introduced in 2007.

> See:

> 

>     https://sourceware.org/ml/binutils/2007-08/msg00044.html

> 

> That hack mostly allowed GDB's handling of core files to continue to

> work without any changes to GDB.

> 

> The problem with setting the section size to zero is that GDB won't

> know how big that section is/was.  Often, this doesn't matter because

> the data in question are found in the exec file.  But it can happen

> that the section describes memory that had been allocated, but never

> written to.  In this instance, the contents of that memory region are

> not written to the core file.  Also, since the region in question was

> dynamically allocated, it won't appear in the exec file.  We don't

> want these regions to appear as inaccessible to GDB (since they *were*

> accessible when the process was live), so it's important that GDB know

> the size of the region.

> 

> I've made changes to GDB which correctly handles this case.  When

> attempting to access memory, GDB will first consider core file data

> for which both SEC_ALLOC and SEC_HAS_CONTENTS is set.  Next, if that

> fails, GDB will attempt to find the data in the exec file.  Finally,

> if that also fails, GDB will attempt to access memory in the sections

> which are flagged as SEC_ALLOC, but not SEC_HAS_CONTENTS.

> 

> bfd/ChangeLog:

> 

> 	* elf.c (_bfd_elf_make_section_from_phdr): Remove hack for GDB.

> 

> Change-Id: I7cce707aa3c217addbc27589730a77620199843f

> ---

>  bfd/elf.c | 8 --------

>  1 file changed, 8 deletions(-)

> 

> diff --git a/bfd/elf.c b/bfd/elf.c

> index c4d6718aaa..89c61acc40 100644

> --- a/bfd/elf.c

> +++ b/bfd/elf.c

> @@ -3007,14 +3007,6 @@ _bfd_elf_make_section_from_phdr (bfd *abfd,

>        newsect->alignment_power = bfd_log2 (align);

>        if (hdr->p_type == PT_LOAD)

>  	{

> -	  /* Hack for gdb.  Segments that have not been modified do

> -	     not have their contents written to a core file, on the

> -	     assumption that a debugger can find the contents in the

> -	     executable.  We flag this case by setting the fake

> -	     section size to zero.  Note that "real" bss sections will

> -	     always have their contents dumped to the core file.  */

> -	  if (bfd_get_format (abfd) == bfd_core)

> -	    newsect->size = 0;

>  	  newsect->flags |= SEC_ALLOC;

>  	  if (hdr->p_flags & PF_X)

>  	    newsect->flags |= SEC_CODE;

> -- 

> 2.24.1

>
Luis Machado via Gdb-patches May 4, 2020, 6:09 p.m. | #2
On Thu, 05 Mar 2020 01:42:40 +0100, Kevin Buettner wrote:
> This commit removes a hack for GDB which was introduced in 2007.

> See:

> 

>     https://sourceware.org/ml/binutils/2007-08/msg00044.html

> 

> That hack mostly allowed GDB's handling of core files to continue to

> work without any changes to GDB.


I believe this hack is no longer needed since GDB parses /proc/PID/smaps:
	Implement support for checking /proc/PID/coredump_filter
	https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=df8411da087dc05481926f4c4a82deabc5bc3859


Jan

Patch

diff --git a/bfd/elf.c b/bfd/elf.c
index c4d6718aaa..89c61acc40 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -3007,14 +3007,6 @@  _bfd_elf_make_section_from_phdr (bfd *abfd,
       newsect->alignment_power = bfd_log2 (align);
       if (hdr->p_type == PT_LOAD)
 	{
-	  /* Hack for gdb.  Segments that have not been modified do
-	     not have their contents written to a core file, on the
-	     assumption that a debugger can find the contents in the
-	     executable.  We flag this case by setting the fake
-	     section size to zero.  Note that "real" bss sections will
-	     always have their contents dumped to the core file.  */
-	  if (bfd_get_format (abfd) == bfd_core)
-	    newsect->size = 0;
 	  newsect->flags |= SEC_ALLOC;
 	  if (hdr->p_flags & PF_X)
 	    newsect->flags |= SEC_CODE;