public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
@ 2020-05-14 17:22 mlimber
  2020-05-14 17:32 ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: mlimber @ 2020-05-14 17:22 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 847 bytes --]

I have replicated this bug in gdb versions 10.0.50.20200514-git and also
8.2-0ubuntu1~14.04.1. I am using an executable that dynamically links an
shared object with no text segment, viz., libicudata.so.52.2. (FWIW, lldb-8
does not crash with this same executable + library.)

I located a patch by Jeremie Courreges-Anglas here:
https://marc.info/?l=openbsd-ports&m=146569238229407&w=2. I applied said
patch, rebuilt gdb, and now I can debug the same executable with gdb.

I am not an expert on the inner workings of gdb, but I assume those who
review this code can better judge the overall soundness of this change.

I did not create a new test, but I can try it if necessary.

I have not previously completed an FSF copyright assignment. I have a
"request for disclaimer" email prepared to send if so directed by the gdb
maintainers.

Cheers!

M

[-- Attachment #2: 0001-PR-symtab-25678-Set-entry-point-when-text-segment-is.patch --]
[-- Type: application/octet-stream, Size: 1293 bytes --]

From a1bf21d22eaf8f7725c38703d29c748fc2161b33 Mon Sep 17 00:00:00 2001
From: mlimber <mlimber@gmail.com>
Date: Thu, 14 May 2020 13:09:05 -0400
Subject: [PATCH] PR symtab/25678: Set entry point when text segment is
 missing. From patch by Jeremie Courreges-Anglas.

---
 gdb/ChangeLog | 7 +++++++
 gdb/symfile.c | 7 ++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c4da2a9..b25414f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2020-05-14  M. Limber  <mlimber@gmail.com>
+
+	PR symtab/25678
+	* symfile.c: Set entry point when text segment is missing.
+	From patch by Jeremie Courreges-Anglas: 
+	https://marc.info/?l=openbsd-ports&m=146569238229407&w=2
+
 2020-05-14  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
 	    Tom de Vries  <tdevries@suse.de>
 	    Pedro Alves  <palves@redhat.com>
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 7c862d5..e03c591 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -878,7 +878,12 @@ struct symfile_segment_data *
 	}
 
       if (!found)
-	ei->the_bfd_section_index = SECT_OFF_TEXT (objfile);
+	{
+	  if (objfile->sect_index_text == -1)
+	    ei->entry_point_p = 0;
+	  else
+	    ei->the_bfd_section_index = objfile->sect_index_text;	
+	}
     }
 }
 
-- 
1.9.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-14 17:22 [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text mlimber
@ 2020-05-14 17:32 ` Simon Marchi
  2020-05-14 17:48   ` mlimber
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-14 17:32 UTC (permalink / raw)
  To: mlimber, gdb-patches

On 2020-05-14 1:22 p.m., mlimber via Gdb-patches wrote:
> I have replicated this bug in gdb versions 10.0.50.20200514-git and also
> 8.2-0ubuntu1~14.04.1. I am using an executable that dynamically links an
> shared object with no text segment, viz., libicudata.so.52.2. (FWIW, lldb-8
> does not crash with this same executable + library.)
> 
> I located a patch by Jeremie Courreges-Anglas here:
> https://marc.info/?l=openbsd-ports&m=146569238229407&w=2. I applied said
> patch, rebuilt gdb, and now I can debug the same executable with gdb.
> 
> I am not an expert on the inner workings of gdb, but I assume those who
> review this code can better judge the overall soundness of this change.
> 
> I did not create a new test, but I can try it if necessary.
> 
> I have not previously completed an FSF copyright assignment. I have a
> "request for disclaimer" email prepared to send if so directed by the gdb
> maintainers.
> 
> Cheers!
> 
> M
> 

Hi,

Thanks for the patch.  For reference, could you please provide some instructions
on how to replicate this bug?  How to compile a minimal test case and the GDB
commands to trigger the bug.  It should ideally be put in the commit message to
act as a reference, and it will be useful to write a test.

We'll want a test for sure, so if you want to start writing one it would be
appreciated.  I know that writing GDB test is not the most obvious thing to
do when you are not familiar with it, so don't hesitate to ask for help.

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-14 17:32 ` Simon Marchi
@ 2020-05-14 17:48   ` mlimber
  2020-05-14 17:57     ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: mlimber @ 2020-05-14 17:48 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Sure. Clearer repro steps:

1. Build an application linking to an SO that has no text segment. I
believe there is an SO attached to the bug ticket that will work.

(My actual use case is a little complicated. I'm building a Qt 5.3 app and
Qt's libs have libicu*.so.52.2 as dependencies. Thus, I am indirectly using
libicudata.so.52.2, similar to what a recent commenter on the ticket
reported.)

2. Execute `gdb my_exe`.

3. Type 'r' at the gdb prompt to run.

4. Output from gdb 10:

    Starting program: [snip]
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library
"/lib/x86_64-linux-gnu/libthread_db.so.1".
    symfile.c:881: internal-error: sect_index_text not initialized
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    Quit this debugging session? (y or n)

Output from gdb 8.2:

    Starting program: [snip]
    [Thread debugging using libthread_db enabled]
    Using host libthread_db library
"/lib/x86_64-linux-gnu/libthread_db.so.1".
    /build/gdb-q2KLFj/gdb-8.2/gdb/symfile.c:891: internal-error:
sect_index_text not initialized
    A problem internal to GDB has been detected,
    further debugging may prove unreliable.
    Quit this debugging session? (y or n)

Is it legitimate to add binary files like an SO as part of the test, or
must I build all artifacts from source? If desired, I can attach the
offending SO to the ticket once my account is setup (waiting on setup
email).

M

On Thu, May 14, 2020 at 1:32 PM Simon Marchi <simark@simark.ca> wrote:

> On 2020-05-14 1:22 p.m., mlimber via Gdb-patches wrote:
> > I have replicated this bug in gdb versions 10.0.50.20200514-git and also
> > 8.2-0ubuntu1~14.04.1. I am using an executable that dynamically links an
> > shared object with no text segment, viz., libicudata.so.52.2. (FWIW,
> lldb-8
> > does not crash with this same executable + library.)
> >
> > I located a patch by Jeremie Courreges-Anglas here:
> > https://marc.info/?l=openbsd-ports&m=146569238229407&w=2. I applied said
> > patch, rebuilt gdb, and now I can debug the same executable with gdb.
> >
> > I am not an expert on the inner workings of gdb, but I assume those who
> > review this code can better judge the overall soundness of this change.
> >
> > I did not create a new test, but I can try it if necessary.
> >
> > I have not previously completed an FSF copyright assignment. I have a
> > "request for disclaimer" email prepared to send if so directed by the gdb
> > maintainers.
> >
> > Cheers!
> >
> > M
> >
>
> Hi,
>
> Thanks for the patch.  For reference, could you please provide some
> instructions
> on how to replicate this bug?  How to compile a minimal test case and the
> GDB
> commands to trigger the bug.  It should ideally be put in the commit
> message to
> act as a reference, and it will be useful to write a test.
>
> We'll want a test for sure, so if you want to start writing one it would be
> appreciated.  I know that writing GDB test is not the most obvious thing to
> do when you are not familiar with it, so don't hesitate to ask for help.
>
> Simon
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-14 17:48   ` mlimber
@ 2020-05-14 17:57     ` Simon Marchi
  2020-05-14 19:12       ` mlimber
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-14 17:57 UTC (permalink / raw)
  To: mlimber; +Cc: gdb-patches

On 2020-05-14 1:48 p.m., mlimber wrote:
> Sure. Clearer repro steps:
> 
> 1. Build an application linking to an SO that has no text segment. I believe there is an SO attached to the bug ticket that will work.
> 
> (My actual use case is a little complicated. I'm building a Qt 5.3 app and Qt's libs have libicu*.so.52.2 as dependencies. Thus, I am indirectly using libicudata.so.52.2, similar to what a recent commenter on the ticket reported.)
> 
> 2. Execute `gdb my_exe`.
> 
> 3. Type 'r' at the gdb prompt to run.
> 
> 4. Output from gdb 10:
> 
>     Starting program: [snip]
>     [Thread debugging using libthread_db enabled]
>     Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>     symfile.c:881: internal-error: sect_index_text not initialized
>     A problem internal to GDB has been detected,
>     further debugging may prove unreliable.
>     Quit this debugging session? (y or n)
> 
> Output from gdb 8.2:
> 
>     Starting program: [snip]
>     [Thread debugging using libthread_db enabled]
>     Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>     /build/gdb-q2KLFj/gdb-8.2/gdb/symfile.c:891: internal-error: sect_index_text not initialized
>     A problem internal to GDB has been detected,
>     further debugging may prove unreliable.
>     Quit this debugging session? (y or n)
> 
> Is it legitimate to add binary files like an SO as part of the test, or must I build all artifacts from source? If desired, I can attach the offending SO to the ticket once my account is setup (waiting on setup email).
> 
> M

Thanks.  The best is to give a source snippet and compiler commands used to build
it, so someone reading the commit message has everything they need to reproduce the
issue if needed.  It can also be good to mention which compiler version (including if
it comes from a distro package) you use, because sometimes it matters.

In this case, I'm guessing that compiling a simple shared library that has only one
global variable and no code will be enough to reproduce the issue?

For the test, we currently don't check in binary artifacts, they are all generated
as part of the test.  Grep for `gdb_compile_shlib` to see how to generate a shared
library.

I'm thinking that it would be useful to check in some binary artifacts, but only
for really hard to reproduce cases.  Like, gcc version x.y.z on this distro with
this option generated something weird, and we want to make sure we don't crash on
it.  But we don't do that at the moment.

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-14 17:57     ` Simon Marchi
@ 2020-05-14 19:12       ` mlimber
  2020-05-14 19:28         ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: mlimber @ 2020-05-14 19:12 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Unfortunately, the simpler repro cases I have tried don't trigger the
failure, e.g.,

// main.c
extern int g_global_var;
int main()
  {
    return g_global_var;
  }

// libglobal.c
extern int g_global_var;
int g_global_var = 42;

I build it like:

gcc -shared -nostdlib -fPIC -o libglobal.so libglobal.c
gcc -o main main.c -lglobal -L. -Wl,--rpath,\$ORIGIN

Running it in GDB works fine. Seems like something more is required.

Even following the repro steps listed in the first comment or linking
against libicudata.so in a simple program like above work fine for me.

My more complicated, real-world use case does consistently repro the bug
before the patch but does not after.

More digging required. Suggestions welcome!

M


On Thu, May 14, 2020 at 1:57 PM Simon Marchi <simark@simark.ca> wrote:

> On 2020-05-14 1:48 p.m., mlimber wrote:
> > Sure. Clearer repro steps:
> >
> > 1. Build an application linking to an SO that has no text segment. I
> believe there is an SO attached to the bug ticket that will work.
> >
> > (My actual use case is a little complicated. I'm building a Qt 5.3 app
> and Qt's libs have libicu*.so.52.2 as dependencies. Thus, I am indirectly
> using libicudata.so.52.2, similar to what a recent commenter on the ticket
> reported.)
> >
> > 2. Execute `gdb my_exe`.
> >
> > 3. Type 'r' at the gdb prompt to run.
> >
> > 4. Output from gdb 10:
> >
> >     Starting program: [snip]
> >     [Thread debugging using libthread_db enabled]
> >     Using host libthread_db library
> "/lib/x86_64-linux-gnu/libthread_db.so.1".
> >     symfile.c:881: internal-error: sect_index_text not initialized
> >     A problem internal to GDB has been detected,
> >     further debugging may prove unreliable.
> >     Quit this debugging session? (y or n)
> >
> > Output from gdb 8.2:
> >
> >     Starting program: [snip]
> >     [Thread debugging using libthread_db enabled]
> >     Using host libthread_db library
> "/lib/x86_64-linux-gnu/libthread_db.so.1".
> >     /build/gdb-q2KLFj/gdb-8.2/gdb/symfile.c:891: internal-error:
> sect_index_text not initialized
> >     A problem internal to GDB has been detected,
> >     further debugging may prove unreliable.
> >     Quit this debugging session? (y or n)
> >
> > Is it legitimate to add binary files like an SO as part of the test, or
> must I build all artifacts from source? If desired, I can attach the
> offending SO to the ticket once my account is setup (waiting on setup
> email).
> >
> > M
>
> Thanks.  The best is to give a source snippet and compiler commands used
> to build
> it, so someone reading the commit message has everything they need to
> reproduce the
> issue if needed.  It can also be good to mention which compiler version
> (including if
> it comes from a distro package) you use, because sometimes it matters.
>
> In this case, I'm guessing that compiling a simple shared library that has
> only one
> global variable and no code will be enough to reproduce the issue?
>
> For the test, we currently don't check in binary artifacts, they are all
> generated
> as part of the test.  Grep for `gdb_compile_shlib` to see how to generate
> a shared
> library.
>
> I'm thinking that it would be useful to check in some binary artifacts,
> but only
> for really hard to reproduce cases.  Like, gcc version x.y.z on this
> distro with
> this option generated something weird, and we want to make sure we don't
> crash on
> it.  But we don't do that at the moment.
>
> Simon
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-14 19:12       ` mlimber
@ 2020-05-14 19:28         ` Simon Marchi
  2020-05-15 18:33           ` mlimber
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-14 19:28 UTC (permalink / raw)
  To: mlimber; +Cc: gdb-patches

On 2020-05-14 3:12 p.m., mlimber via Gdb-patches wrote:
> Unfortunately, the simpler repro cases I have tried don't trigger the
> failure, e.g.,
> 
> // main.c
> extern int g_global_var;
> int main()
>   {
>     return g_global_var;
>   }
> 
> // libglobal.c
> extern int g_global_var;
> int g_global_var = 42;
> 
> I build it like:
> 
> gcc -shared -nostdlib -fPIC -o libglobal.so libglobal.c
> gcc -o main main.c -lglobal -L. -Wl,--rpath,\$ORIGIN
> 
> Running it in GDB works fine. Seems like something more is required.
> 
> Even following the repro steps listed in the first comment or linking
> against libicudata.so in a simple program like above work fine for me.
> 
> My more complicated, real-world use case does consistently repro the bug
> before the patch but does not after.
> 
> More digging required. Suggestions welcome!
> 
> M

If you can make a reproducer that uses Qt, that's a good start.  Then we can
track down what's special about this use case, and try to make a reproducer
that doesn't use it.

But for the record, I was able to reproduce the crash using the instructions
in the bug.  Keith (comment 3) did too.

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-14 19:28         ` Simon Marchi
@ 2020-05-15 18:33           ` mlimber
  2020-05-16 20:39             ` mlimber
                               ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: mlimber @ 2020-05-15 18:33 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Hrm. I've tried a number of smaller programs to repro this, including the
steps in the comments using the attachment or building my own, X11, OpenGL,
libicu tests, and also sample Qt apps that link `libicudata.so`. Alas, I
cannot recreate it in small-ish form.

I've tried various combinations of these on Ubuntu 14.04 x64 building with
gcc 4.9 and also Ubuntu 20.04 x64 with gcc 9.3.

I can reliably repro it with my full-scale Qt app, but I can't submit that.
When I debug gdb itself, I see where it is going awry in my app -- with
libicudata.so.52, which is a weird data-only library.

I have updated the patch to be a little cleaner by handling this case
earlier rather than at the end since that the problem is apparently that we
have a non-zero starting address but no text section. Thus, I think it fits
better like:

--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -829,7 +829,8 @@ struct symfile_segment_data *
       ei->entry_point_p = 1;
     }
   else if (bfd_get_file_flags (objfile->obfd) & DYNAMIC
-   && bfd_get_start_address (objfile->obfd) != 0)
+   && bfd_get_start_address (objfile->obfd) != 0
+   && objfile->sect_index_text != -1)
     {
       /* Some shared libraries may have entry points set and be
runnable.  There's no clear way to indicate this, so just check
for values other than zero.  */
      ei->entry_point = bfd_get_start_address (objfile->obfd);
      ei->entry_point_p = 1;
     }
   else
     {
-      /* Examination of non-executable.o files.  Short-circuit this stuff.
 */
+      /* Examination of non-executable.o files or shared library with
+         no text segment.  Short-circuit this stuff.  */
       ei->entry_point_p = 0;
     }

The issue description also says that one needs >2 segments, but I don't see
where that comes in yet. Perhaps that's what I'm missing in trying to repro
it?

I'm willing to keep trying. Can you recommend any way for me to repro the
issue in a small test case?

Thanks!

M

On Thu, May 14, 2020 at 3:28 PM Simon Marchi <simark@simark.ca> wrote:

> On 2020-05-14 3:12 p.m., mlimber via Gdb-patches wrote:
> > Unfortunately, the simpler repro cases I have tried don't trigger the
> > failure, e.g.,
> >
> > // main.c
> > extern int g_global_var;
> > int main()
> >   {
> >     return g_global_var;
> >   }
> >
> > // libglobal.c
> > extern int g_global_var;
> > int g_global_var = 42;
> >
> > I build it like:
> >
> > gcc -shared -nostdlib -fPIC -o libglobal.so libglobal.c
> > gcc -o main main.c -lglobal -L. -Wl,--rpath,\$ORIGIN
> >
> > Running it in GDB works fine. Seems like something more is required.
> >
> > Even following the repro steps listed in the first comment or linking
> > against libicudata.so in a simple program like above work fine for me.
> >
> > My more complicated, real-world use case does consistently repro the bug
> > before the patch but does not after.
> >
> > More digging required. Suggestions welcome!
> >
> > M
>
> If you can make a reproducer that uses Qt, that's a good start.  Then we
> can
> track down what's special about this use case, and try to make a reproducer
> that doesn't use it.
>
> But for the record, I was able to reproduce the crash using the
> instructions
> in the bug.  Keith (comment 3) did too.
>
> Simon
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-15 18:33           ` mlimber
@ 2020-05-16 20:39             ` mlimber
  2020-05-16 21:05               ` Simon Marchi
  2020-05-17  3:31             ` Simon Marchi
  2020-05-18 18:01             ` Simon Marchi
  2 siblings, 1 reply; 21+ messages in thread
From: mlimber @ 2020-05-16 20:39 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

Thanks, with the comment you left on the issue and the attached
libtestcase.so, I can repro it. However, neither of my two patches (which
are basically the same anyway) fully fix it because it just bombs out
later. More work required, which I'm willing to attempt.

In the name of building all tests from source, can you advise on how to
create an so like libtestcase.so? I've tried to fiddle with options and
version scripts to no avail. My objdump looks close but not exact, and my
attempts don't repro the error.

M

On Fri, May 15, 2020 at 2:33 PM mlimber <mlimber@gmail.com> wrote:

> Hrm. I've tried a number of smaller programs to repro this, including the
> steps in the comments using the attachment or building my own, X11, OpenGL,
> libicu tests, and also sample Qt apps that link `libicudata.so`. Alas, I
> cannot recreate it in small-ish form.
>
> I've tried various combinations of these on Ubuntu 14.04 x64 building with
> gcc 4.9 and also Ubuntu 20.04 x64 with gcc 9.3.
>
> I can reliably repro it with my full-scale Qt app, but I can't submit
> that. When I debug gdb itself, I see where it is going awry in my app --
> with libicudata.so.52, which is a weird data-only library.
>
> I have updated the patch to be a little cleaner by handling this case
> earlier rather than at the end since that the problem is apparently that we
> have a non-zero starting address but no text section. Thus, I think it fits
> better like:
>
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -829,7 +829,8 @@ struct symfile_segment_data *
>        ei->entry_point_p = 1;
>      }
>    else if (bfd_get_file_flags (objfile->obfd) & DYNAMIC
> -   && bfd_get_start_address (objfile->obfd) != 0)
> +   && bfd_get_start_address (objfile->obfd) != 0
> +   && objfile->sect_index_text != -1)
>      {
>        /* Some shared libraries may have entry points set and be
> runnable.  There's no clear way to indicate this, so just check
> for values other than zero.  */
>       ei->entry_point = bfd_get_start_address (objfile->obfd);
>       ei->entry_point_p = 1;
>      }
>    else
>      {
> -      /* Examination of non-executable.o files.  Short-circuit this
> stuff.  */
> +      /* Examination of non-executable.o files or shared library with
> +         no text segment.  Short-circuit this stuff.  */
>        ei->entry_point_p = 0;
>      }
>
> The issue description also says that one needs >2 segments, but I don't
> see where that comes in yet. Perhaps that's what I'm missing in trying to
> repro it?
>
> I'm willing to keep trying. Can you recommend any way for me to repro the
> issue in a small test case?
>
> Thanks!
>
> M
>
> On Thu, May 14, 2020 at 3:28 PM Simon Marchi <simark@simark.ca> wrote:
>
>> On 2020-05-14 3:12 p.m., mlimber via Gdb-patches wrote:
>> > Unfortunately, the simpler repro cases I have tried don't trigger the
>> > failure, e.g.,
>> >
>> > // main.c
>> > extern int g_global_var;
>> > int main()
>> >   {
>> >     return g_global_var;
>> >   }
>> >
>> > // libglobal.c
>> > extern int g_global_var;
>> > int g_global_var = 42;
>> >
>> > I build it like:
>> >
>> > gcc -shared -nostdlib -fPIC -o libglobal.so libglobal.c
>> > gcc -o main main.c -lglobal -L. -Wl,--rpath,\$ORIGIN
>> >
>> > Running it in GDB works fine. Seems like something more is required.
>> >
>> > Even following the repro steps listed in the first comment or linking
>> > against libicudata.so in a simple program like above work fine for me.
>> >
>> > My more complicated, real-world use case does consistently repro the bug
>> > before the patch but does not after.
>> >
>> > More digging required. Suggestions welcome!
>> >
>> > M
>>
>> If you can make a reproducer that uses Qt, that's a good start.  Then we
>> can
>> track down what's special about this use case, and try to make a
>> reproducer
>> that doesn't use it.
>>
>> But for the record, I was able to reproduce the crash using the
>> instructions
>> in the bug.  Keith (comment 3) did too.
>>
>> Simon
>>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-16 20:39             ` mlimber
@ 2020-05-16 21:05               ` Simon Marchi
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2020-05-16 21:05 UTC (permalink / raw)
  To: mlimber; +Cc: gdb-patches

On 2020-05-16 4:39 p.m., mlimber wrote:
> Thanks, with the comment you left on the issue and the attached libtestcase.so, I can repro it. However, neither of my two patches (which are basically the same anyway) fully fix it because it just bombs out later. More work required, which I'm willing to attempt.
> 
> In the name of building all tests from source, can you advise on how to create an so like libtestcase.so? I've tried to fiddle with options and version scripts to no avail. My objdump looks close but not exact, and my attempts don't repro the error.

I was also looking at that right now.

The particularity of libtestcase.so is that it has a .debug_info section, so
the DWARF debug info reader (dwarf/read.c) kicks in to read the partial symbols
(a quick pass to read the debug symbols, to gather just enough things to build
some index of what's in it) but there is no .text section.  The DWARF-reading
code assumes that there is a .text section, hence the failed assertion.

For reference, the DWARF in the file is the following:

.debug_info contents:
0x00000000: Compile Unit: length = 0x00000032 version = 0x0005 unit_type = DW_UT_compile abbr_offset = 0x0000 addr_size = 0x08 (next unit at 0x00000036)

0x0000000c: DW_TAG_compile_unit
              DW_AT_name        ("scratch.kyuu")
              DW_AT_use_UTF8    (true)

0x0000001a:   DW_TAG_variable
                DW_AT_name      ("foo")
                DW_AT_external  (0x00)
                DW_AT_type      (0x0000002e "s32")
                DW_AT_location  (DW_OP_addr 0x2250)

0x0000002e:   DW_TAG_base_type
                DW_AT_name      ("s32")
                DW_AT_encoding  (DW_ATE_signed)
                DW_AT_byte_size (0x04)

0x00000035:   NULL


I am not sure how to end up there in a "legitimate" way.  Even when I compile
a shared library with gcc with just a single global variable, a .text section is
created. So perhaps that was created with some particular options, or some other tool

In any case, GDB should be fixed, because even with some invalid / crafted input,
we shouldn't hit an assertion.

It shouldn't be difficult to craft an executable like this I suppose, just compile
a standard shared library with debug info and remove .text using objcopy?  I don't
have time to try that right now.

For the test case that will go into gdb's testsuite, it can either be that (remove
the section using objcopy) or some manually-crafted DWARF (see other examples in
testsuite/gdb.dwarf2).

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-15 18:33           ` mlimber
  2020-05-16 20:39             ` mlimber
@ 2020-05-17  3:31             ` Simon Marchi
  2020-05-17  7:01               ` Andreas Schwab
  2020-05-18 18:01             ` Simon Marchi
  2 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-17  3:31 UTC (permalink / raw)
  To: mlimber; +Cc: gdb-patches

On 2020-05-15 2:33 p.m., mlimber wrote:
> I can reliably repro it with my full-scale Qt app, but I can't submit that. When I debug gdb itself, I see where it is going awry in my app -- with libicudata.so.52, which is a weird data-only library.

Out of curiosity, where does this libicudata.so.52 come from?

All the versions of libicudata.so I found in Ubuntu/Debian do have a .text
section.  Maybe knowing where it comes from, we can trace back how it was
built.

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-17  3:31             ` Simon Marchi
@ 2020-05-17  7:01               ` Andreas Schwab
  2020-05-17 14:01                 ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Andreas Schwab @ 2020-05-17  7:01 UTC (permalink / raw)
  To: Simon Marchi; +Cc: mlimber, gdb-patches

On Mai 16 2020, Simon Marchi wrote:

> All the versions of libicudata.so I found in Ubuntu/Debian do have a .text
> section.

This is a debian local patch.  By default, libicudata is linked with
-nodefaultlibs -nostdlib, and the only included object file has no .text
section.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-17  7:01               ` Andreas Schwab
@ 2020-05-17 14:01                 ` Simon Marchi
  2020-05-17 14:08                   ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-17 14:01 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: mlimber, gdb-patches

On 2020-05-17 3:01 a.m., Andreas Schwab wrote:
> On Mai 16 2020, Simon Marchi wrote:
> 
>> All the versions of libicudata.so I found in Ubuntu/Debian do have a .text
>> section.
> 
> This is a debian local patch.  By default, libicudata is linked with
> -nodefaultlibs -nostdlib, and the only included object file has no .text
> section.
> 
> Andreas.

Thanks Andreas for the tip, I didn't think of checking the distro patches.

I was able to reproduce with this:

$ cat allo.c
int salut;
$ gcc allo.c -fPIC -o allo.o -g3 -O0 -c
$ gcc allo.o -shared -nostdlib -nodefaultlibs -o allo.so
$ ./gdb -q -nx a.out
Reading symbols from a.out...
(gdb) start
Temporary breakpoint 1 at 0x1136: file test.c, line 1.
Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out
/home/smarchi/src/binutils-gdb/gdb/objfiles.h:524: internal-error: sect_index_text not initialized
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-17 14:01                 ` Simon Marchi
@ 2020-05-17 14:08                   ` Simon Marchi
  0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2020-05-17 14:08 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: mlimber, gdb-patches

On 2020-05-17 10:01 a.m., Simon Marchi wrote:
> Thanks Andreas for the tip, I didn't think of checking the distro patches.
> 
> I was able to reproduce with this:
> 
> $ cat allo.c
> int salut;
> $ gcc allo.c -fPIC -o allo.o -g3 -O0 -c
> $ gcc allo.o -shared -nostdlib -nodefaultlibs -o allo.so
> $ ./gdb -q -nx a.out
> Reading symbols from a.out...
> (gdb) start
> Temporary breakpoint 1 at 0x1136: file test.c, line 1.
> Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out
> /home/smarchi/src/binutils-gdb/gdb/objfiles.h:524: internal-error: sect_index_text not initialized
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
> 
> Simon
> 

Ah nevermind, I spoke to soon.  I forgot to rebuild the main executable,
to I was still running some old test case.  When doing that, it doesn't
hit the assert:

$ cat allo.c
int salut;
$ gcc allo.c -fPIC -o allo.o -g3 -O0 -c
$ gcc allo.o -shared -nostdlib -nodefaultlibs -o allo.so
$ cat test.c
int main() {}
$ gcc test.c -Wl,--no-as-needed ./allo.so -g3 -O0 -o
$ ./gdb --data-directory=data-directory -q -nx a.out
Reading symbols from a.out...
(gdb) start
Temporary breakpoint 1 at 0x1136: file test.c, line 1.
Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out

Temporary breakpoint 1, main () at test.c:1
1       int main() {}
(gdb)

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-15 18:33           ` mlimber
  2020-05-16 20:39             ` mlimber
  2020-05-17  3:31             ` Simon Marchi
@ 2020-05-18 18:01             ` Simon Marchi
  2020-05-18 21:11               ` mlimber
  2 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-18 18:01 UTC (permalink / raw)
  To: mlimber; +Cc: gdb-patches

On 2020-05-15 2:33 p.m., mlimber wrote:
> Hrm. I've tried a number of smaller programs to repro this, including the steps in the comments using the attachment or building my own, X11, OpenGL, libicu tests, and also sample Qt apps that link `libicudata.so`. Alas, I cannot recreate it in small-ish form.
> 
> I've tried various combinations of these on Ubuntu 14.04 x64 building with gcc 4.9 and also Ubuntu 20.04 x64 with gcc 9.3.
> 
> I can reliably repro it with my full-scale Qt app, but I can't submit that. When I debug gdb itself, I see where it is going awry in my app -- with libicudata.so.52, which is a weird data-only library.

Could you please tell us where this particular libicudata.so.52 comes from?  I'd like to compare
it with other libicudata.so files to understand what's so special about this one.

Simon


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-18 18:01             ` Simon Marchi
@ 2020-05-18 21:11               ` mlimber
  2020-05-18 21:44                 ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: mlimber @ 2020-05-18 21:11 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Mon, May 18, 2020 at 2:01 PM Simon Marchi <simark@simark.ca> wrote:

> On 2020-05-15 2:33 p.m., mlimber wrote:
> > Hrm. I've tried a number of smaller programs to repro this, including
> the steps in the comments using the attachment or building my own, X11,
> OpenGL, libicu tests, and also sample Qt apps that link `libicudata.so`.
> Alas, I cannot recreate it in small-ish form.
> >
> > I've tried various combinations of these on Ubuntu 14.04 x64 building
> with gcc 4.9 and also Ubuntu 20.04 x64 with gcc 9.3.
> >
> > I can reliably repro it with my full-scale Qt app, but I can't submit
> that. When I debug gdb itself, I see where it is going awry in my app --
> with libicudata.so.52, which is a weird data-only library.
>
> Could you please tell us where this particular libicudata.so.52 comes
> from?  I'd like to compare
> it with other libicudata.so files to understand what's so special about
> this one.
>
> Simon
>

I built libicudata.so.52.2 from source (no customizations; just needed the
particular version my version of Qt was looking for, and I modified the
RPATH of the three ICU .so's to look in $ORIGIN for their dependencies
since I distribute my app with its dependencies). Note that someone else
also reported the libicudata.so being the culprit and that I have not yet
triggered the error with the libicudata.so in a smaller program.

The build steps for the offending .so right out of the build log for the
library is:
```
gcc -D_REENTRANT  -DU_HAVE_ELF_H=1 -DU_HAVE_ATOMIC=1
 -DU_ATTRIBUTE_DEPRECATED= -O2 -std=c99 -Wall -pedantic -Wshadow
-Wpointer-arith -Wmissing-prototypes -Wwrite-strings   -c -I../common
-I../common -DPIC -fPIC -o ./out/tmp/icudt52l_dat.o ./out/tmp/icudt52l_dat.s
gcc -O2 -std=c99 -Wall -pedantic -Wshadow -Wpointer-arith
-Wmissing-prototypes -Wwrite-strings   -static-libstdc++  -shared
-Wl,-Bsymbolic -nodefaultlibs -nostdlib -o ../lib/libicudata.so.52.2
./out/tmp/icudt52l_dat.o -Wl,-soname -Wl,libicudata.so.52  -Wl,-Bsymbolic
```

The assembly file is generated at build-time and is of the form:
```
.globl icudt52_dat
.section .note.GNU-stack,"",%progbits
.section .rodata
.balign 16
.type icudt52_dat,%object
icudt52_dat:

.long
0x27DA0080,0x14,0x020000,0x446E6D43,1,3,0x706F4320,0x67697279,0x28207468,0x32202943,0x2C333130,0x746E4920,0x616E7265,0x6E6F6974,0x42206C61,0x6E697375,0x20737365,0x6863614D,0x73656E69,0x726F4320,0x61726F70,0x6E6F6974,0x646E6120,0x68746F20,0x2E737265,0x6C6C4120,0x67695220,0x20737468,0x65736552,0x64657672,0x202E,0
.long
0x0C64,0x6324,0x018370,0x6334,0x01B900,0x6347,0x01B9A0,0x635A,0x01BA00,0x636B,0x01C580,0x637F,0x01C5E0,0x638F,0x01CF90,0x63A2,0x01CFF0,0x63B2,0x020420,0x63C5,0x020480,0x63D5,0x026160,0x63E9,0x0261C0,0x63FC,0x026220,0x640F,0x026280,0x6422,0x0262E0,0x6435
[snip lots more data]
```
I can make the binary of the .so available to you if desired (it is 23 MB).

M

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-18 21:11               ` mlimber
@ 2020-05-18 21:44                 ` Simon Marchi
  2020-05-19 14:36                   ` mlimber
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-18 21:44 UTC (permalink / raw)
  To: mlimber; +Cc: gdb-patches

On 2020-05-18 5:11 p.m., mlimber wrote:
> I built libicudata.so.52.2 from source (no customizations; just needed the particular version my version of Qt was looking for, and I modified the RPATH of the three ICU .so's to look in $ORIGIN for their dependencies since I distribute my app with its dependencies). Note that someone else also reported the libicudata.so being the culprit and that I have not yet triggered the error with the libicudata.so in a smaller program.
> 
> The build steps for the offending .so right out of the build log for the library is:
> ```
> gcc -D_REENTRANT  -DU_HAVE_ELF_H=1 -DU_HAVE_ATOMIC=1  -DU_ATTRIBUTE_DEPRECATED= -O2 -std=c99 -Wall -pedantic -Wshadow -Wpointer-arith -Wmissing-prototypes -Wwrite-strings   -c -I../common -I../common -DPIC -fPIC -o ./out/tmp/icudt52l_dat.o ./out/tmp/icudt52l_dat.s
> gcc -O2 -std=c99 -Wall -pedantic -Wshadow -Wpointer-arith -Wmissing-prototypes -Wwrite-strings   -static-libstdc++  -shared -Wl,-Bsymbolic -nodefaultlibs -nostdlib -o ../lib/libicudata.so.52.2 ./out/tmp/icudt52l_dat.o -Wl,-soname -Wl,libicudata.so.52  -Wl,-Bsymbolic
> ```
> 
> The assembly file is generated at build-time and is of the form:
> ```
> .globl icudt52_dat
> .section .note.GNU-stack,"",%progbits
> .section .rodata
> .balign 16
> .type icudt52_dat,%object
> icudt52_dat:
> 
> .long 0x27DA0080,0x14,0x020000,0x446E6D43,1,3,0x706F4320,0x67697279,0x28207468,0x32202943,0x2C333130,0x746E4920,0x616E7265,0x6E6F6974,0x42206C61,0x6E697375,0x20737365,0x6863614D,0x73656E69,0x726F4320,0x61726F70,0x6E6F6974,0x646E6120,0x68746F20,0x2E737265,0x6C6C4120,0x67695220,0x20737468,0x65736552,0x64657672,0x202E,0
> .long 0x0C64,0x6324,0x018370,0x6334,0x01B900,0x6347,0x01B9A0,0x635A,0x01BA00,0x636B,0x01C580,0x637F,0x01C5E0,0x638F,0x01CF90,0x63A2,0x01CFF0,0x63B2,0x020420,0x63C5,0x020480,0x63D5,0x026160,0x63E9,0x0261C0,0x63FC,0x026220,0x640F,0x026280,0x6422,0x0262E0,0x6435
> [snip lots more data]
> ```
> I can make the binary of the .so available to you if desired (it is 23 MB).

If you can upload it somewhere, it wouldn't hurt.  I'm also trying to build
icu from the same release.

Right now I'm trying to make some sense of the `symfile_find_segment_sections`
function here:

https://github.com/bminor/binutils-gdb/blob/966dc1a27c55ccb298cb8c7c41c9cc2985cc321a/gdb/symfile.c#L3701

I suspect it's buggy, and "hides" the bug when using some shared libraries
(including the one I build myself) which would explain why we see it with some
libraries that are missing a .text section and not others.

When called for a library that has no .text section, if that library has two
segments (which is the case of the library I created), then that function will
initialize objfile::sect_index_text to 0 (or some other value that insinuates
that there is a .text section).  That sounds wrong to me: if the library has no
.text section, it would be better to leave sect_index_text to -1, and make sure
the rest of the code can cope with that.  If I remove `symfile_find_segment_sections`
completely, sect_index_text stays -1 for my library and I hit the assertion.  I then
believe that the correct way forward would be to update the dwarf2/read.c code to
deal with a .text section not present.  Normally, if there's no .text section, there
should be no debug info describing code stuff (only data stuff), so it shouldn't be
necessary to use sect_index_text.

The library attached to the bug (libtestcase.so) has the particularity of having
3 segments.  So `symfile_find_segment_sections` is skipped, sect_index_text stays
-1, and we see the assertion.

So I'm curious, in your libicudata.so library, how many segments there are.  That
can be checked with:

  $ readelf -l libicudata.so.52 | grep LOAD

If the libicudata.so.52 is really the problematic one, I'm a bit surprised that you
don't always see the problem when debugging a program that uses it.

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-18 21:44                 ` Simon Marchi
@ 2020-05-19 14:36                   ` mlimber
  2020-05-19 14:44                     ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: mlimber @ 2020-05-19 14:36 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Mon, May 18, 2020 at 5:44 PM Simon Marchi <simark@simark.ca> wrote:

> If you can upload it somewhere, it wouldn't hurt.  I'm also trying to build
> icu from the same release.
>

Here are my ICU libs (built with gcc 4.9 for x64):

https://www.dropbox.com/sh/1esshbzshll0hzq/AACBgQgwuSEM_pYQ0HYG_ybXa


> Right now I'm trying to make some sense of the
> `symfile_find_segment_sections`
> function here:
>
>
> https://github.com/bminor/binutils-gdb/blob/966dc1a27c55ccb298cb8c7c41c9cc2985cc321a/gdb/symfile.c#L3701
>
> I suspect it's buggy, and "hides" the bug when using some shared libraries
> (including the one I build myself) which would explain why we see it with
> some
> libraries that are missing a .text section and not others.
>
> When called for a library that has no .text section, if that library has
> two
> segments (which is the case of the library I created), then that function
> will
> initialize objfile::sect_index_text to 0 (or some other value that
> insinuates
> that there is a .text section).  That sounds wrong to me: if the library
> has no
> .text section, it would be better to leave sect_index_text to -1, and make
> sure
> the rest of the code can cope with that.  If I remove
> `symfile_find_segment_sections`
> completely, sect_index_text stays -1 for my library and I hit the
> assertion.  I then
> believe that the correct way forward would be to update the dwarf2/read.c
> code to
> deal with a .text section not present.  Normally, if there's no .text
> section, there
> should be no debug info describing code stuff (only data stuff), so it
> shouldn't be
> necessary to use sect_index_text.
>
> The library attached to the bug (libtestcase.so) has the particularity of
> having
> 3 segments.  So `symfile_find_segment_sections` is skipped,
> sect_index_text stays
> -1, and we see the assertion.
>

I have skimmed that code, but we're beyond my ken here. The things I
observe in that code are:

1. This only acts on files with 1 or 2 segments. (It gets skipped for
libtestcase.so as you say.)

2. If the segment info is 1 or 2, it sets two segment indices to refer to
this one segment. Perhaps that's legit (I'm a naif when it comes to ELF
details), but it struck me as odd.

3. Line 300, where this function is called, has this curious comment:

/* This is where things get really weird... We MUST have valid
indices for the various sect_index_* members or gdb will abort.
So if for example, there is no ".text" section, we have to
accomodate that. First, check for a file with the standard
one or two segments. */


> So I'm curious, in your libicudata.so library, how many segments there
> are.  That
> can be checked with:
>
>   $ readelf -l libicudata.so.52 | grep LOAD
>

I have two load segments:

readelf -l libicudata.so

Elf file type is DYN (Shared object file)
Entry point 0x2b6
There are 6 program headers, starting at offset 64

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x000000000166a940 0x000000000166a940  R      200000
  LOAD           0x000000000166af30 0x000000000186af30 0x000000000186af30
                 0x00000000000000d0 0x00000000000000d0  RW     200000
  DYNAMIC        0x000000000166af30 0x000000000186af30 0x000000000186af30
                 0x00000000000000d0 0x00000000000000d0  RW     8
  NOTE           0x0000000000000190 0x0000000000000190 0x0000000000000190
                 0x0000000000000024 0x0000000000000024  R      4
  GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RW     10
  GNU_RELRO      0x000000000166af30 0x000000000186af30 0x000000000186af30
                 0x00000000000000d0 0x00000000000000d0  R      1

I note that there is an entry point specified on the second line of output,
which is curious to me since there is no code in this library.

Perhaps if we forced an extra load segment in this .so, it would produce
different results due to skipping the function cited above.


> If the libicudata.so.52 is really the problematic one, I'm a bit surprised
> that you
> don't always see the problem when debugging a program that uses it.
>

I'm also not sure why it sometimes happens and sometimes doesn't. Could it
be something with how or when it is loaded -- say, in a certain sequence or
via a manual dlopen() instead of via dynamic linking info?

M

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-19 14:36                   ` mlimber
@ 2020-05-19 14:44                     ` Simon Marchi
  2020-05-20 13:24                       ` mlimber
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-19 14:44 UTC (permalink / raw)
  To: mlimber; +Cc: gdb-patches

On 2020-05-19 10:36 a.m., mlimber wrote:
> I have skimmed that code, but we're beyond my ken here. The things I observe in that code are:
> 
> 1. This only acts on files with 1 or 2 segments. (It gets skipped for libtestcase.so as you say.)
> 
> 2. If the segment info is 1 or 2, it sets two segment indices to refer to this one segment. Perhaps that's legit (I'm a naif when it comes to ELF details), but it struck me as odd.
> 
> 3. Line 300, where this function is called, has this curious comment:
> 
> /* This is where things get really weird... We MUST have valid
> 
> indices for the various sect_index_* members or gdb will abort.
> 
> So if for example, there is no ".text" section, we have to
> 
> accomodate that. First, check for a file with the standard
> 
> one or two segments. */

I reached the same conclusions.

> 
>  
> 
>     So I'm curious, in your libicudata.so library, how many segments there are.  That
>     can be checked with:
> 
>       $ readelf -l libicudata.so.52 | grep LOAD
> 
> 
> I have two load segments:
> 
> readelf -l libicudata.so
> 
> Elf file type is DYN (Shared object file)
> Entry point 0x2b6
> There are 6 program headers, starting at offset 64
> 
> Program Headers:
>   Type           Offset             VirtAddr           PhysAddr
>                  FileSiz            MemSiz              Flags  Align
>   LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
>                  0x000000000166a940 0x000000000166a940  R      200000
>   LOAD           0x000000000166af30 0x000000000186af30 0x000000000186af30
>                  0x00000000000000d0 0x00000000000000d0  RW     200000
>   DYNAMIC        0x000000000166af30 0x000000000186af30 0x000000000186af30
>                  0x00000000000000d0 0x00000000000000d0  RW     8
>   NOTE           0x0000000000000190 0x0000000000000190 0x0000000000000190
>                  0x0000000000000024 0x0000000000000024  R      4
>   GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
>                  0x0000000000000000 0x0000000000000000  RW     10
>   GNU_RELRO      0x000000000166af30 0x000000000186af30 0x000000000186af30
>                  0x00000000000000d0 0x00000000000000d0  R      1

Are we inspecting the same library?  In the libicudata.so.52 you've sent, there
are three load segments:

$ readelf -l libicudata.so.52.2 | grep LOAD
  LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x166a940 0x166a940 R   0x200000
  LOAD           0x166af30 0x000000000186af30 0x000000000186af30 0x0000d0 0x0000d0 RW  0x200000
  LOAD           0x166c000 0x000000000186b000 0x000000000186b000 0x000180 0x000180 RW  0x1000

I successfully reproduced the bug using your lib.  Since there's no DWARF
info, it fails in init_entry_point_info.  With my lib, it fails earlier,
when the DWARF info is read.  Anyway, it's all variations of the same bug,
some code assumes that sect_index_text is set to some valid value.<

> I note that there is an entry point specified on the second line of output, which is curious to me since there is no code in this library.

I noticed that too, shared libraries have entry points... that fields looks
mandatory in the ELF header, so it can probably just be ignored.

> 
> Perhaps if we forced an extra load segment in this .so, it would produce different results due to skipping the function cited above.
>  
> 
>     If the libicudata.so.52 is really the problematic one, I'm a bit surprised that you
>     don't always see the problem when debugging a program that uses it.
> 
> 
> I'm also not sure why it sometimes happens and sometimes doesn't. Could it be something with how or when it is loaded -- say, in a certain sequence or via a manual dlopen() instead of via dynamic linking info?

The only reason I would see is that you might not be loading the libicudata.so
you think you are loading.

Simon


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-19 14:44                     ` Simon Marchi
@ 2020-05-20 13:24                       ` mlimber
  2020-05-20 14:12                         ` Simon Marchi
  0 siblings, 1 reply; 21+ messages in thread
From: mlimber @ 2020-05-20 13:24 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Tue, May 19, 2020 at 10:50 AM Simon Marchi <simark@simark.ca> wrote:

> Are we inspecting the same library?  In the libicudata.so.52 you've sent,
> there
> are three load segments:
>
> $ readelf -l libicudata.so.52.2 | grep LOAD
>   LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x166a940
> 0x166a940 R   0x200000
>   LOAD           0x166af30 0x000000000186af30 0x000000000186af30 0x0000d0
> 0x0000d0 RW  0x200000
>   LOAD           0x166c000 0x000000000186b000 0x000000000186b000 0x000180
> 0x000180 RW  0x1000
>

Ah, my bad. The lib freshly rebuilt from source has two segments, but the
one I uploaded has three because I manually set the RPATH on it. (The data
lib doesn't really need to be on this since it doesn't have other
dependencies, but it just got caught up in a glob of all the ICU libs.)

So the additional step needed after building from source is to run
`patchelf --set-rpath \$ORIGIN libicudata.so.52.2`.

To reduce confusion (maybe), I have added two files to the same dropbox
folder -- libicudata-unpatched.so.52.2 and libicudata-with-rpath.so.52.2.
(I have left the original files I uploaded there for now.)

(For my own application, I have rebuilt libicu* again after running
`./configure --enable-rpath=yes`, and then I get the RPATHs I need so I
don't need the patchelf at all. Thus my app's problem is resolved.)


> I successfully reproduced the bug using your lib.  Since there's no DWARF
> info, it fails in init_entry_point_info.  With my lib, it fails earlier,
> when the DWARF info is read.  Anyway, it's all variations of the same bug,
> some code assumes that sect_index_text is set to some valid value.<
>

I can now also repro the original bug in a rinky-dink program suitable for
unit testing. Can you supply the steps to build a small program to repro
the DWARF-related bug?

Will both bugs be fixed by a change in one place? That is, is my second
patch irrelevant because we'll ultimately fix both bugs at some higher
level? If the patch is still valid, I could work to submit an updated patch
and test case for my non-DWARF bug now, and then you (or you and I) can
work up a test case and fix -- possibly under a new bug ticket -- for the
DWARF bug.

M

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-20 13:24                       ` mlimber
@ 2020-05-20 14:12                         ` Simon Marchi
  2020-05-20 15:04                           ` mlimber
  0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2020-05-20 14:12 UTC (permalink / raw)
  To: mlimber; +Cc: gdb-patches

On 2020-05-20 9:24 a.m., mlimber wrote:
> On Tue, May 19, 2020 at 10:50 AM Simon Marchi <simark@simark.ca <mailto:simark@simark.ca>> wrote:
> 
>     Are we inspecting the same library?  In the libicudata.so.52 you've sent, there
>     are three load segments:
> 
>     $ readelf -l libicudata.so.52.2 | grep LOAD
>       LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x166a940 0x166a940 R   0x200000
>       LOAD           0x166af30 0x000000000186af30 0x000000000186af30 0x0000d0 0x0000d0 RW  0x200000
>       LOAD           0x166c000 0x000000000186b000 0x000000000186b000 0x000180 0x000180 RW  0x1000
> 
> 
> Ah, my bad. The lib freshly rebuilt from source has two segments, but the one I uploaded has three because I manually set the RPATH on it. (The data lib doesn't really need to be on this since it doesn't have other dependencies, but it just got caught up in a glob of all the ICU libs.)
> 
> So the additional step needed after building from source is to run `patchelf --set-rpath \$ORIGIN libicudata.so.52.2`.

Ah that explains the three segments.  Good to know, thanks!

> To reduce confusion (maybe), I have added two files to the same dropbox folder -- libicudata-unpatched.so.52.2 and libicudata-with-rpath.so.52.2. (I have left the original files I uploaded there for now.)

Thanks, it should now be trivial to reproduce with a dummy library knowing
what you said above.

> (For my own application, I have rebuilt libicu* again after running `./configure --enable-rpath=yes`, and then I get the RPATHs I need so I don't need the patchelf at all. Thus my app's problem is resolved.)
>  
> 
>     I successfully reproduced the bug using your lib.  Since there's no DWARF
>     info, it fails in init_entry_point_info.  With my lib, it fails earlier,
>     when the DWARF info is read.  Anyway, it's all variations of the same bug,
>     some code assumes that sect_index_text is set to some valid value.<
> 
> 
> I can now also repro the original bug in a rinky-dink program suitable for unit testing. Can you supply the steps to build a small program to repro the DWARF-related bug?

You mean as part of the GDB testsuite?  You can check and use this test I've made here:

  https://sourceware.org/pipermail/gdb-patches/2020-May/168769.html

It's not complete, it's missing license headers for example.

Once applied in your repo, it can be ran with:

  $ make check TESTS="gdb.base/solib-no-text.exp"

The transcript then be read at gdb/testsuite/gdb.log.

> Will both bugs be fixed by a change in one place? That is, is my second patch irrelevant because we'll ultimately fix both bugs at some higher level? If the patch is still valid, I could work to submit an updated patch and test case for my non-DWARF bug now, and then you (or you and I) can work up a test case and fix -- possibly under a new bug ticket -- for the DWARF bug.

There are two paths forward I see:

(1) make sure sect_index_text is always initialized, even if there's no .text section
(2) make GDB aware that sect_index_text could be left to -1

If we chose (1), then the fixes in your patches wouldn't be needed, as sect_index_text will
never be -1.

If we chose (2), then we should get rid of the code that invents a sect_index_text value
when there's no .text section.  The fixes in your patches would be needed (or something
equivalent), but there would be many other similar fixes needed.

I posted this RFC patch that summarizes the problem and starts to implement (2):

  https://sourceware.org/pipermail/gdb-patches/2020-May/168767.html

Simon

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text
  2020-05-20 14:12                         ` Simon Marchi
@ 2020-05-20 15:04                           ` mlimber
  0 siblings, 0 replies; 21+ messages in thread
From: mlimber @ 2020-05-20 15:04 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On Wed, May 20, 2020 at 10:12 AM Simon Marchi <simark@simark.ca> wrote:

> On 2020-05-20 9:24 a.m., mlimber wrote:
> > Will both bugs be fixed by a change in one place? That is, is my second
> patch irrelevant because we'll ultimately fix both bugs at some higher
> level? If the patch is still valid, I could work to submit an updated patch
> and test case for my non-DWARF bug now, and then you (or you and I) can
> work up a test case and fix -- possibly under a new bug ticket -- for the
> DWARF bug.
>
> There are two paths forward I see:
>
> (1) make sure sect_index_text is always initialized, even if there's no
> .text section
> (2) make GDB aware that sect_index_text could be left to -1
>
> If we chose (1), then the fixes in your patches wouldn't be needed, as
> sect_index_text will
> never be -1.
>
> If we chose (2), then we should get rid of the code that invents a
> sect_index_text value
> when there's no .text section.  The fixes in your patches would be needed
> (or something
> equivalent), but there would be many other similar fixes needed.
>
> I posted this RFC patch that summarizes the problem and starts to
> implement (2):
>
>   https://sourceware.org/pipermail/gdb-patches/2020-May/168767.html
>

Thanks for digging into this! I took a look. I'm still willing to assist if
needed, but you've gone beyond my knowledge of GDB and ELF internals, so
I'm not sure if I can be very useful to you. Let me know if there is
something further I can do.

M

PS, I'm surprised mailing list patches are still a thing in the age of
Github!

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2020-05-20 15:04 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-14 17:22 [PATCH] [PR 25678] gdb crashes with "internal-error: sect_index_text not initialized" when .text mlimber
2020-05-14 17:32 ` Simon Marchi
2020-05-14 17:48   ` mlimber
2020-05-14 17:57     ` Simon Marchi
2020-05-14 19:12       ` mlimber
2020-05-14 19:28         ` Simon Marchi
2020-05-15 18:33           ` mlimber
2020-05-16 20:39             ` mlimber
2020-05-16 21:05               ` Simon Marchi
2020-05-17  3:31             ` Simon Marchi
2020-05-17  7:01               ` Andreas Schwab
2020-05-17 14:01                 ` Simon Marchi
2020-05-17 14:08                   ` Simon Marchi
2020-05-18 18:01             ` Simon Marchi
2020-05-18 21:11               ` mlimber
2020-05-18 21:44                 ` Simon Marchi
2020-05-19 14:36                   ` mlimber
2020-05-19 14:44                     ` Simon Marchi
2020-05-20 13:24                       ` mlimber
2020-05-20 14:12                         ` Simon Marchi
2020-05-20 15:04                           ` mlimber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).