[PATCH] Fix GDB internal error by using text (instead of data) section offset

[PATCH] Fix GDB internal error by using text (instead of data) section offset

[Kevin Buettner]

Fedora Rawhide is now using gcc-12.0.  As part of updating to the
gcc-12.0 package set, Rawhide is also now using a version of libgcc_s
which lacks a .data section.  This causes gdb to fail in the following
fashion while debugging a program (such as gdb) which uses libgcc_s:

    (top-gdb) run
    Starting program: rawhide-master/bld/gdb/gdb
    ...
    objfiles.h:467: internal-error: sect_index_data not initialized
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    ...

I snipped the backtrace from the above output.  Instead, here's a
portion of a backtrace obtained using GDB's backtrace command.
(Obviously, in order to obtain it, I used a GDB which has been patched
with this commit.)

    #0  internal_error (
	file=0xc6a508 "gdb/objfiles.h", line=467,
	fmt=0xc6a4e8 "sect_index_data not initialized")
	at gdbsupport/errors.cc:51
    #1  0x00000000005f9651 in objfile::data_section_offset (this=0x4fa48f0)
	at gdb/objfiles.h:467
    #2  0x000000000097c5f8 in relocate_address (address=0x17244, objfile=0x4fa48f0)
	at gdb/stap-probe.c:1333
    #3  0x000000000097c630 in stap_probe::get_relocated_address (this=0xa1a17a0,
	objfile=0x4fa48f0)
	at gdb/stap-probe.c:1341
    #4  0x00000000004d7025 in create_exception_master_breakpoint_probe (
	objfile=0x4fa48f0)
	at gdb/breakpoint.c:3505
    #5  0x00000000004d7426 in create_exception_master_breakpoint ()
	at gdb/breakpoint.c:3575
    #6  0x00000000004efcc1 in breakpoint_re_set ()
	at gdb/breakpoint.c:13407
    #7  0x0000000000956998 in solib_add (pattern=0x0, from_tty=0, readsyms=1)
	at gdb/solib.c:1001
    #8  0x00000000009576a8 in handle_solib_event ()
	at gdb/solib.c:1269
    ...

The function 'relocate_address' in gdb/stap-probe.c attempts to do
its "relocation" by using objfile->data_section_offset().  That
method, data_section_offset() is defined as follows in objfiles.h:

  CORE_ADDR data_section_offset () const
  {
    return section_offsets[SECT_OFF_DATA (this)];
  }

The internal error occurs when the SECT_OFF_DATA macro finds that the
'sect_index_data' field is -1:

    #define SECT_OFF_DATA(objfile) \
	 ((objfile->sect_index_data == -1) \
	  ? (internal_error (__FILE__, __LINE__, \
			     _("sect_index_data not initialized")), -1)	\
	  : objfile->sect_index_data)

The obvious solution is to use some other section offset instead - as
I recall, on Linux, the section offsets (for those sections which
exist) will all be the same.  SECT_OFF_TEXT / text_section_offset
seems like a logical choice, so that's what I've used.  Actually, in
this context, I think that text_section_offset is a better choice even
setting aside the current difficulty.  (The breakpoint related code
which calls it is dealing with code addresses, not data addresses.
Therefore it's more likely to be correct even on OSes for which
section offsets can differ.)

Searching the sources turned up one other use of data_section_offset,
in gdb/dtrace-probe.c, so I've updated that code as well.  (I'd
guess that one was copied from the other.)

So, what happens if there's no .text section?  If that were to
occur, GDB would be in real trouble elsewhere since a search
for "text_section_offset" reveals 55 uses of this method, 27
of which are in DWARF related code.

Summary:

	* gdb/dtrace-probe.c (dtrace_probe::get_relocated_address):
	Use method text_section_offset instead of data_section_offset.
	* gdb/stap-probe.c (relocate_address): Likewise.
---
 gdb/dtrace-probe.c | 2 +-
 gdb/stap-probe.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index ea4999ccebe..9aeefb6060c 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -684,7 +684,7 @@ dtrace_probe::is_enabled () const
 CORE_ADDR
 dtrace_probe::get_relocated_address (struct objfile *objfile)
 {
-  return this->get_address () + objfile->data_section_offset ();
+  return this->get_address () + objfile->text_section_offset ();
 }
 
 /* Implementation of the get_argument_count method.  */
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index 2310fdec86d..c4b24df7564 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -1330,7 +1330,7 @@ stap_probe::parse_arguments (struct gdbarch *gdbarch)
 static CORE_ADDR
 relocate_address (CORE_ADDR address, struct objfile *objfile)
 {
-  return address + objfile->data_section_offset ();
+  return address + objfile->text_section_offset ();
 }
 
 /* Implementation of the get_relocated_address method.  */
-- 
2.34.1

Re: [PATCH] Fix GDB internal error by using text (instead of data) section offset

[Florian Weimer]

* Kevin Buettner via Gdb-patches:

> So, what happens if there's no .text section?  If that were to
> occur, GDB would be in real trouble elsewhere since a search
> for "text_section_offset" reveals 55 uses of this method, 27
> of which are in DWARF related code.

The probes will always emit code, right?  So there should be a text
section.

Thanks,
Florian

Re: [PATCH] Fix GDB internal error by using text (instead of data) section offset

[Tom Tromey]

Kevin>     #3  0x000000000097c630 in stap_probe::get_relocated_address (this=0xa1a17a0,
Kevin> 	objfile=0x4fa48f0)
Kevin> 	at gdb/stap-probe.c:1341

One oddity here is that get_relocated_address calls this->get_address(),
which returns m_address, which is documented as:

  /* The address where the probe is inserted, relative to
     SECT_OFF_TEXT.  */
  CORE_ADDR m_address;

So using SECT_OFF_DATA here seems wrong in the first place.

Kevin> The obvious solution is to use some other section offset instead - as
Kevin> I recall, on Linux, the section offsets (for those sections which
Kevin> exist) will all be the same.

Is this a Linux thing or an ELF thing?  I don't know.

Kevin> So, what happens if there's no .text section?

Not probe-related but this is already a way to crash gdb:

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

... which is why I was curious about the ELF thing.  Like perhaps this
could all be made more robust somehow.

Your patch seems fine to me, though I don't have any idea if the dtrace
part is correct.

Tom

Re: [PATCH] Fix GDB internal error by using text (instead of data) section offset

[Simon Marchi]

On 2022-01-25 18:04, Kevin Buettner via Gdb-patches wrote:
> Fedora Rawhide is now using gcc-12.0.  As part of updating to the
> gcc-12.0 package set, Rawhide is also now using a version of libgcc_s
> which lacks a .data section.  This causes gdb to fail in the following
> fashion while debugging a program (such as gdb) which uses libgcc_s:
> 
>     (top-gdb) run
>     Starting program: rawhide-master/bld/gdb/gdb
>     ...
>     objfiles.h:467: internal-error: sect_index_data not initialized
>     A problem internal to GDB has been detected,
>     further debugging may prove unreliable.
>     ...
> 
> I snipped the backtrace from the above output.  Instead, here's a
> portion of a backtrace obtained using GDB's backtrace command.
> (Obviously, in order to obtain it, I used a GDB which has been patched
> with this commit.)
> 
>     #0  internal_error (
> 	file=0xc6a508 "gdb/objfiles.h", line=467,
> 	fmt=0xc6a4e8 "sect_index_data not initialized")
> 	at gdbsupport/errors.cc:51
>     #1  0x00000000005f9651 in objfile::data_section_offset (this=0x4fa48f0)
> 	at gdb/objfiles.h:467
>     #2  0x000000000097c5f8 in relocate_address (address=0x17244, objfile=0x4fa48f0)
> 	at gdb/stap-probe.c:1333
>     #3  0x000000000097c630 in stap_probe::get_relocated_address (this=0xa1a17a0,
> 	objfile=0x4fa48f0)
> 	at gdb/stap-probe.c:1341
>     #4  0x00000000004d7025 in create_exception_master_breakpoint_probe (
> 	objfile=0x4fa48f0)
> 	at gdb/breakpoint.c:3505
>     #5  0x00000000004d7426 in create_exception_master_breakpoint ()
> 	at gdb/breakpoint.c:3575
>     #6  0x00000000004efcc1 in breakpoint_re_set ()
> 	at gdb/breakpoint.c:13407
>     #7  0x0000000000956998 in solib_add (pattern=0x0, from_tty=0, readsyms=1)
> 	at gdb/solib.c:1001
>     #8  0x00000000009576a8 in handle_solib_event ()
> 	at gdb/solib.c:1269
>     ...
> 
> The function 'relocate_address' in gdb/stap-probe.c attempts to do
> its "relocation" by using objfile->data_section_offset().  That
> method, data_section_offset() is defined as follows in objfiles.h:
> 
>   CORE_ADDR data_section_offset () const
>   {
>     return section_offsets[SECT_OFF_DATA (this)];
>   }
> 
> The internal error occurs when the SECT_OFF_DATA macro finds that the
> 'sect_index_data' field is -1:
> 
>     #define SECT_OFF_DATA(objfile) \
> 	 ((objfile->sect_index_data == -1) \
> 	  ? (internal_error (__FILE__, __LINE__, \
> 			     _("sect_index_data not initialized")), -1)	\
> 	  : objfile->sect_index_data)
> 
> The obvious solution is to use some other section offset instead - as
> I recall, on Linux, the section offsets (for those sections which
> exist) will all be the same.  SECT_OFF_TEXT / text_section_offset
> seems like a logical choice, so that's what I've used.  Actually, in
> this context, I think that text_section_offset is a better choice even
> setting aside the current difficulty.  (The breakpoint related code
> which calls it is dealing with code addresses, not data addresses.
> Therefore it's more likely to be correct even on OSes for which
> section offsets can differ.)
> 
> Searching the sources turned up one other use of data_section_offset,
> in gdb/dtrace-probe.c, so I've updated that code as well.  (I'd
> guess that one was copied from the other.)
> 
> So, what happens if there's no .text section?  If that were to
> occur, GDB would be in real trouble elsewhere since a search
> for "text_section_offset" reveals 55 uses of this method, 27
> of which are in DWARF related code.

There's a bug about this, shared lib without a .text section:

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

Simon

Re: [PATCH] Fix GDB internal error by using text (instead of data) section offset

[Kevin Buettner]

On Wed, 26 Jan 2022 11:03:33 -0700
Tom Tromey <tom@tromey.com> wrote:

> Kevin>     #3  0x000000000097c630 in stap_probe::get_relocated_address (this=0xa1a17a0,
> Kevin> 	objfile=0x4fa48f0)
> Kevin> 	at gdb/stap-probe.c:1341  
> 
> One oddity here is that get_relocated_address calls this->get_address(),
> which returns m_address, which is documented as:
> 
>   /* The address where the probe is inserted, relative to
>      SECT_OFF_TEXT.  */
>   CORE_ADDR m_address;
> 
> So using SECT_OFF_DATA here seems wrong in the first place.

Agreed.  (And, thanks for finding that.)

> Kevin> The obvious solution is to use some other section offset instead - as
> Kevin> I recall, on Linux, the section offsets (for those sections which
> Kevin> exist) will all be the same.  
> 
> Is this a Linux thing or an ELF thing?  I don't know.

Some ABIs specify that the difference between two addresses in
memory must be the same difference between those same addresses
specified by the executable.  The System V ABI, which most (though
not all) Linux systems use, is one such ABI.

Take a look at the first paragraph in the section titled "Base
Address" in:

    http://www.sco.com/developers/gabi/2012-12-31/ch5.pheader.html

The key part from that paragraph (for this discussion) is this:

    Because position-independent code on those platforms uses relative
    addressing between segments, the difference between virtual
    addresses in memory must match the difference between virtual
    addresses in the file.  The differences between the virtual
    address of any segment in memory and the corresponding virtual
    address in the file is thus a single constant value for any one
    executable or shared object in a given process. 

One ABI which does not have this property is the FDPIC ABI:

    ftp://ftp.redhat.com/pub/redhat/gnupro/FRV/FDPIC-ABI.txt

The FDPIC ABI was first created for the Fujitsu FR-V processor, but a
search shows that variations have been used by some ARM and SH
Linux implementations.  It's useful for processors which lack an
MMU.  In such a system, all processes share the same address space;
read-only sections such as .text can be shared among processes, but
processes will each have their own copy/version of writeable sections
such as .data.  That being the case, the difference between addresses
in .text and .data will be different for each process and will also
almost certainly be different from the addresses specified by the
executable.

> Kevin> So, what happens if there's no .text section?  
> 
> Not probe-related but this is already a way to crash gdb:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=19342
> https://sourceware.org/bugzilla/show_bug.cgi?id=25678
> 
> ... which is why I was curious about the ELF thing.  Like perhaps this
> could all be made more robust somehow.

I've looked at those bugs and skimmed Simon's RFC patches for dealing
with the lack of a .text section in GDB.  In the second patch, he
introduces a function, find_bfd_section_for_address().  I think this
is the right thing to do.  I have some quibbles with some other things
which are done in that series, but I'll try to raise those if/when
Simon resurrects that series.

> Your patch seems fine to me, though I don't have any idea if the dtrace
> part is correct.

Thanks for looking it over!  I'll push it after making some revisions
to the commit log.  (E.g. I'll remove the part about it being a
Linux thing and instead say that it's ABI dependent.  I'll also add
some additional info mentioning existing bugs to the part about
the .text section.)