GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

[Ulrich Weigand]

Hello Joel,

I've now analyzed the GDB test suite for regressions on the platforms I'm
most interested in: s390(x)-linux, ppc(64)-linux, and spu-elf.  Unfortunately,
this did show up a number of real regressions as compared to the last release.
I'm sorry I didn't get to do this earlier; I hope it's not too late to get
(some of) these problems fixed in the GDB 7.0 release ...


1. Write access to bit fields broken
   This is a real regression caused by Dan's bitfield changes.  It shows up
   as multiple store.exp failures on 64-bit big-endian platforms.
   Proposed fix is here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00856.html

2. Displaced stepping broken on PowerPC
   This is a real regression caused by the ARM displaced stepping changes.
   The effect is that non-stop mode no longer works at all on PowerPC.
   Proposed fix is here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00855.html

3. Displaced stepping missing on S/390
   Displaced stepping was in fact never supported on S/390, but this fact
   now shows up as non-stop test case regressions.  I'd suggest to simply
   add support for displaced stepping on this platform.
   Fix (committed to mainline) is here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00853.html

4. Obj-C method calls broken on 64-bit PowerPC
   It looks like this never worked on ppc64 due to lack of support for
   platforms using function descriptors throughout the Obj-C code.
   However, it now shows up as about 20 test cases running into timeout,
   causing a significant increase in run time for the test suite, so it
   would be good to get it fixed simply for that reason.
   Proposed fix is here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00857.html

5. PIE detection not enabled on PowerPC and S/390
   The new code to detect PIE executables was not actually enabled on
   these platforms, causing the new test case to fail.
   Obvious fix (committed to mainline) is here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00852.html
  
6. SPU gdbserver regressions when killing inferior
   This is a real regression caused by gdbserver changes to enable multi-
   executable support in gdbserver, which were not fully implemented for SPU.
   Fix (committed to mainline) is here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00851.html

7. Many-thread test cases crash (stack overflow) on 64-bit S/390
   These are new tests, so not strictly a regression, but easy to fix.
   Test-case only fix (committed to mainline) is here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00850.html

8. dump.exp test case regression on 64-bit S/390
   A test case bug was exposed by fixes to type handling in GDB.
   Test-case only fix (committed to mainline) is here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00849.html

9. Silence noisy test case compiler failures on SPU
   A new C++ test fails during compilation due to the resulting executable
   exceeding local storage size.  The new PIE test fails noisily due to PIE
   not being supported by the compiler.  Not regressions in the strict sense,
   but cause confusing output during the regression run.
   Test-case only fixes (committed to mainline) are here:
   http://sourceware.org/ml/gdb-patches/2009-09/msg00848.html
   http://sourceware.org/ml/gdb-patches/2009-09/msg00847.html


In addition, I'm still seeing new failures in some of the gdb.opt tests
(which appear at first glance to be causes by problems in the debug info
created by the compiler) and some of the gdb.python tests (which I haven't
yet analyzed due to lack of Python skills) ...

How should we proceed with these issues?  Should I commit the fixes
to the branch?  What's the deadline?

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

[Joel Brobecker]

> I've now analyzed the GDB test suite for regressions on the platforms
> I'm most interested in: s390(x)-linux, ppc(64)-linux, and spu-elf.
> Unfortunately, this did show up a number of real regressions as
> compared to the last release.

:-(. Thanks for the heads up.

> How should we proceed with these issues?  Should I commit the fixes
> to the branch?  What's the deadline?

The target date for release is Wed, so we're about 48 hours away.
If you feel confident about your fix, then you should go ahead and
commit to the branch.  According to the guidelines I remember reading
somewhere (it might be in the MAINTAINERS file), if the change only
affects one target, we can also discuss putting that change in. For
the rest of your patches, we need to look at them individually and
evaluate the risk. Based on that, we can decide whether delaying
the release is necessary, and whether each patch should be applied
to the branch and if yes, whether it should be applied now (for 7.0),
or only after it has had a chance to be tested a little more on the
HEAD.

-- 
Joel

Re: GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

[Ulrich Weigand]

Joel Brobecker wrote:

> The target date for release is Wed, so we're about 48 hours away.
> If you feel confident about your fix, then you should go ahead and
> commit to the branch.  According to the guidelines I remember reading
> somewhere (it might be in the MAINTAINERS file), if the change only
> affects one target, we can also discuss putting that change in. For
> the rest of your patches, we need to look at them individually and
> evaluate the risk. Based on that, we can decide whether delaying
> the release is necessary, and whether each patch should be applied
> to the branch and if yes, whether it should be applied now (for 7.0),
> or only after it has had a chance to be tested a little more on the
> HEAD.


Well, my feeling would be:

- Put in the patches that are testsuite-only (7, 8, 9 in my list)
  They should be harmless and significantly clean up test suite
  results on some platforms

- Put in the solely platform-specific patches (3, 5, 6)
  I've tested these, and they definitely help on those platforms

- The bitfield regression (1) seems a serious error affecting
  multiple platforms that really should be fixed, and the patch
  seems straightforward ...

(All the above patches are already in mainline.)


The remaining two PowerPC-related patches (not yet in mainline)
are more difficult:

- The displaced stepping regression is unfortunate, as it completely
  breaks a new feature.  The patch *should* affect only PowerPC, but
  it does need to touch generic files (infrun.c), so there's always
  some risk.  In any case, I'd prefer to get at least some feedback
  before putting it in ...

- The Obj-C changes are not really a regression, so it may not really
  be critical to put those in.  On the other hand, they just touch
  Obj-C code (except for one PowerPC-specific bugfix), and they 
  drastically improve the situation on PowerPC-64, so it would be
  nice ...  Again, I'd definitely like some feedback first.


Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

[Joel Brobecker]

I was going to reply again to your initial email, going through each
patch one by one, but since I already commented on some of the patches
directly on gdb-patches, I'll reply to this message instead.

> - Put in the patches that are testsuite-only (7, 8, 9 in my list)
>   They should be harmless and significantly clean up test suite
>   results on some platforms

Yeah, I agree.

> - Put in the solely platform-specific patches (3, 5, 6)
>   I've tested these, and they definitely help on those platforms

  3. Displaced stepping missing on S/390
     This one seems safe. At worst, non-stop would still be broken...

  5. PIE detection not enabled on PowerPC and S/390
     I suggested we pass on this one because it's just a missing warning.
     But it only affects the targets that you know much better than
     I do, so I'll trust your judgement on this one.

  6. SPU gdbserver regressions when killing inferior
     Agreed that it would be nice to have it for 7.0.

> - The bitfield regression (1) seems a serious error affecting
>   multiple platforms that really should be fixed, and the patch
>   seems straightforward ...

I think it's OK to put this in 7.0 as well. Daniel reviewed
your patch, so that's two pairs of eyes.

> - The displaced stepping regression is unfortunate, as it completely
>   breaks a new feature.  The patch *should* affect only PowerPC, but
>   it does need to touch generic files (infrun.c), so there's always
>   some risk.  In any case, I'd prefer to get at least some feedback
>   before putting it in ...

This one seems a little more problematic indeed. Is that a regression
compared to 6.8? If it is, perhaps we could try to get it fixed for
7.0.1 instead?

> - The Obj-C changes are not really a regression, so it may not really
>   be critical to put those in.  On the other hand, they just touch
>   Obj-C code (except for one PowerPC-specific bugfix), and they 
>   drastically improve the situation on PowerPC-64, so it would be
>   nice ...  Again, I'd definitely like some feedback first.

Given the severity of the problem (SEGV), I think we should put your
patch in 7.0.

-- 
Joel

Re: GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

[Ulrich Weigand]

Joel Brobecker wrote:

> I was going to reply again to your initial email, going through each
> patch one by one, but since I already commented on some of the patches
> directly on gdb-patches, I'll reply to this message instead.

Thanks for your review!

> > - Put in the patches that are testsuite-only (7, 8, 9 in my list)
> >   They should be harmless and significantly clean up test suite
> >   results on some platforms
> 
> Yeah, I agree.

They're in the branch now.

> > - Put in the solely platform-specific patches (3, 5, 6)
> >   I've tested these, and they definitely help on those platforms
> 
>   3. Displaced stepping missing on S/390
>      This one seems safe. At worst, non-stop would still be broken...

Checked in.

>   5. PIE detection not enabled on PowerPC and S/390
>      I suggested we pass on this one because it's just a missing warning.
>      But it only affects the targets that you know much better than
>      I do, so I'll trust your judgement on this one.

Yeah, I guess we don't really need this one.

>   6. SPU gdbserver regressions when killing inferior
>      Agreed that it would be nice to have it for 7.0.

Checked in.

> > - The bitfield regression (1) seems a serious error affecting
> >   multiple platforms that really should be fixed, and the patch
> >   seems straightforward ...
> 
> I think it's OK to put this in 7.0 as well. Daniel reviewed
> your patch, so that's two pairs of eyes.

Checked in as well.

> > - The displaced stepping regression is unfortunate, as it completely
> >   breaks a new feature.  The patch *should* affect only PowerPC, but
> >   it does need to touch generic files (infrun.c), so there's always
> >   some risk.  In any case, I'd prefer to get at least some feedback
> >   before putting it in ...
> 
> This one seems a little more problematic indeed. Is that a regression
> compared to 6.8? If it is, perhaps we could try to get it fixed for
> 7.0.1 instead?

This is now in mainline, but not yet the branch.  It is not strictly a
regression, as 6.8 did not support the non-stop feature at all.  However,
the current state on the branch is that 7.0 does support the feature,
you can enable it without warning, and then if you run the program, you
may see (unpredictable) segmentation faults or other crashes due to
execution running beyond the displaced instruction copy into random
memory ...

Due to the severity of the problem I'd prefer to have the fix in 7.0.
But if you'd rather do a 7.0.1 I guess I can live with that as well.
Let me know whether I should check it in ...

> > - The Obj-C changes are not really a regression, so it may not really
> >   be critical to put those in.  On the other hand, they just touch
> >   Obj-C code (except for one PowerPC-specific bugfix), and they 
> >   drastically improve the situation on PowerPC-64, so it would be
> >   nice ...  Again, I'd definitely like some feedback first.
> 
> Given the severity of the problem (SEGV), I think we should put your
> patch in 7.0.

This is now checked in (mainline and branch) as well.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

[Joel Brobecker]

> > > - The displaced stepping regression is unfortunate, as it completely
> > >   breaks a new feature.  The patch *should* affect only PowerPC, but
> > >   it does need to touch generic files (infrun.c), so there's always
> > >   some risk.  In any case, I'd prefer to get at least some feedback
> > >   before putting it in ...
[...]
> Due to the severity of the problem I'd prefer to have the fix in 7.0.
> But if you'd rather do a 7.0.1 I guess I can live with that as well.
> Let me know whether I should check it in ...

The following is mostly for the record since I replied directly
to the associated gdb-patches thread.

I was going to suggest two approaches:
  - Either we disable the new feature entirely, which is clearly
    a bummer when we know how to fix the breakage;
  - Or commit the patch is one of the GMs thinks the patch is safe.

Since then I noticed that Pedro said he thinks it's safe, so we'll
have the patch in 7.0! :-)

-- 
Joel

Re: GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

[Ulrich Weigand]

Joel Brobecker wrote:

> The following is mostly for the record since I replied directly
> to the associated gdb-patches thread.
> 
> I was going to suggest two approaches:
>   - Either we disable the new feature entirely, which is clearly
>     a bummer when we know how to fix the breakage;
>   - Or commit the patch is one of the GMs thinks the patch is safe.
> 
> Since then I noticed that Pedro said he thinks it's safe, so we'll
> have the patch in 7.0! :-)

OK, thanks!  It's checked in now.

Thanks again for your support in getting these issues fixed!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: GDB 7.0 regressions: s390(x)-linux, ppc(64)-linux, spu-elf

[Tom Tromey]

>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:

Ulrich> In addition, I'm still seeing new failures in some of the
Ulrich> gdb.opt tests (which appear at first glance to be causes by
Ulrich> problems in the debug info created by the compiler) and some of
Ulrich> the gdb.python tests (which I haven't yet analyzed due to lack
Ulrich> of Python skills) ...

Could you send me the gdb.log with the Python test output in it?
I can take a quick look.

Tom