[PATCH] Reduce gdb.base/many-headers.c iterations on Solaris [PR34555]

[PATCH] Reduce gdb.base/many-headers.c iterations on Solaris [PR34555]

[Rainer Orth]

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

As detailed in PR corefiles/34555, when reading the core file in the
gdb.base/many-headers.exp test, gdb consumes excessive amounts of memory
on Solaris.  If left unchecked, e.g. with ulimit -Sv 8388608, the test
will consume all available swap space.  Since Solaris doesn't do lazy
allocation, this will cause the system to become unusably slow.

To avoid this, this patch reduces the iteration count by a factor of 100.

Tested on sparcv9-sun-solaris2.11, x86_64-pc-solaris2.11, and
x86_64-pc-linux-gnu.

Ok for trunk and the gdb-18 branch?


AFAICT, the issue is purely in gdb's Solaris corefile reader, which
seems to need a complete rewrite rather than a couple of bugfixes.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-many-headers-iter.patch --]
[-- Type: text/x-patch, Size: 626 bytes --]

# HG changeset patch
# Parent  c3dbfb6080fbfc4d3b581adca10f1951ecb880e6
Reduce gdb.base/many-headers.c iterations on Solaris [PR34555]

diff --git a/gdb/testsuite/gdb.base/many-headers.c b/gdb/testsuite/gdb.base/many-headers.c
--- a/gdb/testsuite/gdb.base/many-headers.c
+++ b/gdb/testsuite/gdb.base/many-headers.c
@@ -25,7 +25,12 @@ main (void)
   char *ptr;
   int ind, cnt;
 
+#if defined __sun__ && defined __svr4__
+  /* PR corefiles/34555.  */
+  cnt = 1000;
+#else
   cnt = 100000;
+#endif
   for (ind = 0; ind < cnt; ind++)
     {
       ptr = mmap (NULL, 100, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);

Re: [PATCH] Reduce gdb.base/many-headers.c iterations on Solaris [PR34555]

[Andrew Burgess]

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

GDB doesn't usually place bug IDs in the subject line as you're doing
here.  Instead bugs should be mentioned in the text, as you do, using
the 'PR component/number' syntax, then there should be a 'Bug: URL' tag
added at the end of the commit message.

> As detailed in PR corefiles/34555, when reading the core file in the
> gdb.base/many-headers.exp test, gdb consumes excessive amounts of memory
> on Solaris.  If left unchecked, e.g. with ulimit -Sv 8388608, the test
> will consume all available swap space.  Since Solaris doesn't do lazy
> allocation, this will cause the system to become unusably slow.
>
> To avoid this, this patch reduces the iteration count by a factor of 100.

My issue with this change is that the point of the test is to create
many headers.  If we reduce the header count then the test is no longer
doing what it says it is, and the whole test becomes, well, pointless.

I wonder if it would be better to just add this near the top of the test
script:

  if {[istarget "*-*-solaris*"]} {
      # The Solaris core file reader consumes excessive memory with many
      # program headers (PR corefiles/34555).
      kfail gdb/34555 $gdb_test_file_name
      return
  }

While looking at this I spotted that gdb.base/bigcore.exp already bails
out early for Solaris, I wonder if this is the same underlying issue?
The bug number mentioned in bigcore.exp (gdb/1551) is, I think, from
pre-bugzilla days, so I've not been able to track that to an actual bug
I could look at.

>
> Tested on sparcv9-sun-solaris2.11, x86_64-pc-solaris2.11, and
> x86_64-pc-linux-gnu.
>
> Ok for trunk and the gdb-18 branch?
>
>
> AFAICT, the issue is purely in gdb's Solaris corefile reader, which
> seems to need a complete rewrite rather than a couple of bugfixes.

Having touched lots of core file stuff recently, I'd be interested to
know more about where exactly the problem is.

Thanks,
Andrew

Re: [PATCH] Reduce gdb.base/many-headers.c iterations on Solaris [PR34555]

[Rainer Orth]

Hi Andrew,

> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
>
> GDB doesn't usually place bug IDs in the subject line as you're doing
> here.  Instead bugs should be mentioned in the text, as you do, using
> the 'PR component/number' syntax, then there should be a 'Bug: URL' tag
> added at the end of the commit message.

as I'd mentioned before, the once close cousins gcc, binutils, and gdb
have diverged more and more over time, not only in such conventions.
This is particularly confusing to occasional contributors...

>> As detailed in PR corefiles/34555, when reading the core file in the
>> gdb.base/many-headers.exp test, gdb consumes excessive amounts of memory
>> on Solaris.  If left unchecked, e.g. with ulimit -Sv 8388608, the test
>> will consume all available swap space.  Since Solaris doesn't do lazy
>> allocation, this will cause the system to become unusably slow.
>>
>> To avoid this, this patch reduces the iteration count by a factor of 100.
>
> My issue with this change is that the point of the test is to create
> many headers.  If we reduce the header count then the test is no longer
> doing what it says it is, and the whole test becomes, well, pointless.
>
> I wonder if it would be better to just add this near the top of the test
> script:
>
>   if {[istarget "*-*-solaris*"]} {
>       # The Solaris core file reader consumes excessive memory with many
>       # program headers (PR corefiles/34555).
>       kfail gdb/34555 $gdb_test_file_name
>       return
>   }

Honestly I don't care: my primary point is preventing gdb make check
from bringing down machines it is run on.

> While looking at this I spotted that gdb.base/bigcore.exp already bails
> out early for Solaris, I wonder if this is the same underlying issue?

No directly AFAICS: for the unmodified many-headers.c test, the core
dumps are 407 MB (amd64) or (sparcv9), but only consume 5.1 or 4.1 MB on
disk.  However, creating those core files alone is very slow: ca. 17
minutes.

> The bug number mentioned in bigcore.exp (gdb/1551) is, I think, from
> pre-bugzilla days, so I've not been able to track that to an actual bug
> I could look at.

That message goes back to the original test back in 2004, originally
applying only to NetBSD, but having been extended to various other
systems, including Solaris, shortly thereafter.  I haven't searched for
the patch submissions, though.  However, if the test relies on Linux
features, it might be better to restrict it to Linux rather than
skipping it on more and more other systems.

>> AFAICT, the issue is purely in gdb's Solaris corefile reader, which
>> seems to need a complete rewrite rather than a couple of bugfixes.
>
> Having touched lots of core file stuff recently, I'd be interested to
> know more about where exactly the problem is.

I tried to get some Solaris corefile patches in the Solaris userland
repo into shape for upstream submission back in 2018.  When trying so, I
found that the NT_PRSTATUS NOTE sections which are from the old
ioctl-based procfs interface and the only ones handled by bfd/elf.c have
been obsoleted and superceded by NT_PSTATUS ones introduced in Solaris
2.6 back in 1997 with the introduction of the new structured procfs.

ISTM that Solaris corefile support needs a complete rewrite to deal with
almost 30 years of neglect.  Considering the general state of gdb on
Solaris (3000+ testsuite failures, 400+ tests timing out during make
check), corefile support is the very least of my concerns.

I'm slowly reaching the same conclusion as 8 years ago when I abandoned
those efforts: getting gdb into shape on Solaris is way beyond may
abilities and time I'm able to invest.  When trying to get binutils
ready for setting up buildbots, it took me almost a year just analyzing
the testsuite failures, which were only fixed with tons of help from
both Alan Modra who did most of the actual fixes, and Ali Bahrami, the
Solaris linker/ELF utilities maintainer, me at the time very much
neglecting my GCC work.

GDB seems an order of magnitude worse.  Add to that the huge gdb
codebase, an implemention language I know close to nothing about (highly
idiomatic C++17), and several other factors, ISTM that the best I can
achieve is getting buildbots up to make sure gdb at least compiles and
passes the most basic of tests.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [PATCH] Reduce gdb.base/many-headers.c iterations on Solaris [PR34555]

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> While looking at this I spotted that gdb.base/bigcore.exp already bails
Andrew> out early for Solaris, I wonder if this is the same underlying issue?
Andrew> The bug number mentioned in bigcore.exp (gdb/1551) is, I think, from
Andrew> pre-bugzilla days, so I've not been able to track that to an actual bug
Andrew> I could look at.

You can find it by adding 7105, which happened to be the last bug number
at the time that the old GNATS database was imported to Bugzilla.

So this one is:

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

You can also search for "Converted from Gnats 1551".

Tom