[ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses

[ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses

[Ulrich Weigand]

Hello,

the new hbreak2.exp tests are failing on ARM.  It turns out that when
attempting to set a HW breakpoint on an address that is not aligned
to 4 bytes (on Thumb), the kernel rejects GDB's ptrace calls as invalid.

After some discussion with Will Deacon (who implemented the kernel side
of this interface), we've decided to fix this on the GDB side:  GDB
tries to align the address to 4 bytes and uses the "byte address select"
feature to specify only the two upper bytes in that range.  However,
the kernel interface does not (yet) support this use of the BAS field.
On the other hand, the kernel *does* support just specifying the
unaligned address in the address field, and setting up the BAS field
for a normal 2-byte access.  (This use is not supported by the hardware,
but will get fixed up by the kernel.)

Since this alternative way is supported by all existing kernels (that
support the HW breakpoint ptrace interface), and will remain supported
in the future (even once kernels may start supporting more general
use of the BAS field), we can fix this simply by changing GDB to use
that alternative.

The patch below implements this in both GDB and gdbserver.  Tested on
arm-linux-gnueabi (Cortex-A9), fixes the hbreak2 regressions.

Committed to mainline.

Joel, would this be OK for the 7.5 branch at this point?

In general, what's the timeline for 7.5?  I've noticed a couple of
other test case regressions when testing the branch on ARM, s390,
and Cell ...

Bye,
Ulrich


ChangeLog:

	* arm-linux-nat.c (arm_linux_hw_breakpoint_initialize): Do not
	attempt to 4-byte-align HW breakpoint addresses for Thumb.

gdbserver/ChangeLog:

	* linux-arm-low.c (arm_linux_hw_point_initialize): Do not attempt
	to 4-byte-align HW breakpoint addresses for Thumb.

Index: gdb/arm-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-linux-nat.c,v
retrieving revision 1.55
diff -u -p -r1.55 arm-linux-nat.c
--- gdb/arm-linux-nat.c	6 Jul 2012 16:49:43 -0000	1.55
+++ gdb/arm-linux-nat.c	30 Jul 2012 15:00:33 -0000
@@ -896,11 +896,17 @@ arm_linux_hw_breakpoint_initialize (stru
   /* We have to create a mask for the control register which says which bits
      of the word pointed to by address to break on.  */
   if (arm_pc_is_thumb (gdbarch, address))
-    mask = 0x3 << (address & 2);
+    {
+      mask = 0x3;
+      address &= ~1;
+    }
   else
-    mask = 0xf;
+    {
+      mask = 0xf;
+      address &= ~3;
+    }
 
-  p->address = (unsigned int) (address & ~3);
+  p->address = (unsigned int) address;
   p->control = arm_hwbp_control_initialize (mask, arm_hwbp_break, 1);
 }
 
Index: gdb/gdbserver/linux-arm-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-arm-low.c,v
retrieving revision 1.34
diff -u -p -r1.34 linux-arm-low.c
--- gdb/gdbserver/linux-arm-low.c	24 Apr 2012 15:03:43 -0000	1.34
+++ gdb/gdbserver/linux-arm-low.c	30 Jul 2012 15:00:33 -0000
@@ -474,17 +474,17 @@ arm_linux_hw_point_initialize (char type
 	{
 	case 2:	 /* 16-bit Thumb mode breakpoint */
 	case 3:  /* 32-bit Thumb mode breakpoint */
-	  mask = 0x3 << (addr & 2);
+	  mask = 0x3;
+	  addr &= ~1;
 	  break;
 	case 4:  /* 32-bit ARM mode breakpoint */
 	  mask = 0xf;
+	  addr &= ~3;
 	  break;
 	default:
 	  /* Unsupported. */
 	  return -1;
 	}
-
-      addr &= ~3;
     }
   else
     {
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses

[Joel Brobecker]

> Joel, would this be OK for the 7.5 branch at this point?

Sure, as long as a GM is confident about a given change, that is
good enough for me.

> In general, what's the timeline for 7.5?  I've noticed a couple of
> other test case regressions when testing the branch on ARM, s390,
> and Cell ...

The branch was created on July 17th, and the target date for release
creation is 2 weeks after that, which would have been today. I thought
there was still one open issue, but the release page says we're clean
(except for your issue).

The easiest for me would probably to create it on Friday, assuming
that we don't discover something new by then.

-- 
Joel

Remaining 7.5 regressions (Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses)

[Ulrich Weigand]

Joel Brobecker wrote:
> > Joel, would this be OK for the 7.5 branch at this point?
> 
> Sure, as long as a GM is confident about a given change, that is
> good enough for me.

OK, thanks.  I've committed this patch now, as well as two others:
http://sourceware.org/ml/gdb-patches/2012-08/msg00017.html
http://sourceware.org/ml/gdb-patches/2012-08/msg00018.html

> > In general, what's the timeline for 7.5?  I've noticed a couple of
> > other test case regressions when testing the branch on ARM, s390,
> > and Cell ...
> 
> The branch was created on July 17th, and the target date for release
> creation is 2 weeks after that, which would have been today. I thought
> there was still one open issue, but the release page says we're clean
> (except for your issue).

In addition to the failures fixed by the above patches, I'm still seeing:

- Failures in gdb.base/pc-fp.exp on various platforms, as described here:
  http://sourceware.org/ml/gdb-patches/2012-07/msg00823.html
  (just an output formatting issue)

- Failures in gdb.mi/mi-var-rtti.exp on various platforms, see:
  http://sourceware.org/ml/gdb-patches/2012-07/msg00458.html
  (seems to be a bug in the test case)

- Failures in gdb.threads/watchpoint-fork.exp on ARM and PowerPC.
  This looks like a pre-existing bug that hardware watchpoints are not
  handled correctly across forks, which is now exposed since a test
  case for this scenario was added.

- Sporadic timeouts (races?) in gdb.threads/siginfo-threads.exp and
  gdb.threads/ia64-sigill.exp on ARM   (unclear)

- Some new C++ regressions on ARM / s390x (could be compiler issues?)

- Failures in various core file tests on PowerPC (needs investigation)

- Failures in watchpoint.exp on SPU (needs investigation)

- Failures in gdb.server tests on SPU (needs investigation)

- Failures in gdb.threads/siginfo-threads.exp on s390 (needs investigation)

- Failures in gdb.dwarf2/dw2-icc-opaque.exp on SPU and s390 (likewise)

- Testcase harness failures when running a multi-lib configuration:
   ERROR: tcl error sourcing ../../../gdb-7_5/gdb/testsuite/gdb.base/inferior-died.exp.
   ERROR: can't set "seen": variable is array
   ERROR: tcl error sourcing /home/uweigand/fsf/gdb-head/gdb/testsuite/gdb.threads/linux-dp.exp.
   ERROR: can't array set "seen": variable isn't array
  (Invalid re-use of a variable name?)

All these are regressions from 7.4 to 7.5 as far as I can see ...

> The easiest for me would probably to create it on Friday, assuming
> that we don't discover something new by then.

I'll see what I can track down and fix until Friday.  Sorry for starting
my test series a bit late this time ...

Bye,
Ulrich

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

Re: Remaining 7.5 regressions (Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses)

[Sergio Durigan Junior]

On Wednesday, August 01 2012, Ulrich Weigand wrote:

> Joel Brobecker wrote:

>> > In general, what's the timeline for 7.5?  I've noticed a couple of
>> > other test case regressions when testing the branch on ARM, s390,
>> > and Cell ...
>> 
>> The branch was created on July 17th, and the target date for release
>> creation is 2 weeks after that, which would have been today. I thought
>> there was still one open issue, but the release page says we're clean
>> (except for your issue).
>
> In addition to the failures fixed by the above patches, I'm still seeing:
>
> - Failures in gdb.base/pc-fp.exp on various platforms, as described here:
>   http://sourceware.org/ml/gdb-patches/2012-07/msg00823.html
>   (just an output formatting issue)

This patch will probably go in tomorrow when I wake up, if Pedro
approves it.  It is also simple enough to be committed to 7.5.

> - Failures in gdb.mi/mi-var-rtti.exp on various platforms, see:
>   http://sourceware.org/ml/gdb-patches/2012-07/msg00458.html
>   (seems to be a bug in the test case)

I see them also on s390x as you pointed out, but not on ppc64.

> - Failures in gdb.threads/watchpoint-fork.exp on ARM and PowerPC.
>   This looks like a pre-existing bug that hardware watchpoints are not
>   handled correctly across forks, which is now exposed since a test
>   case for this scenario was added.

I seem some failures on s390x as well:

    +FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint after the second fork (timeout)
    +FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: watchpoint after the second fork
    +FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: finish

> - Some new C++ regressions on ARM / s390x (could be compiler issues?)

Could you tell which C++ regressions are those?  I see this on
PPC64/s390x:

-PASS: gdb.cp/inherit.exp: print g_vB (FIXME v3 vtbl ptr)
-PASS: gdb.cp/inherit.exp: print g_vC (FIXME v3 vtbl ptr)
+FAIL: gdb.cp/inherit.exp: print g_vB
+FAIL: gdb.cp/inherit.exp: print g_vC
...
-PASS: gdb.cp/inherit.exp: print g_vE (FIXME v3 vtbl ptr)
+FAIL: gdb.cp/inherit.exp: print g_vE

-PASS: gdb.cp/virtbase.exp: print *this
+FAIL: gdb.cp/virtbase.exp: print *this
...
-PASS: gdb.cp/virtbase.exp: print *(D *) e
+FAIL: gdb.cp/virtbase.exp: print *(D *) e


I am investigating the last two (virtbase.exp), since I had a brief
discussion with Jan about the inherit.exp one and he mentioned it is
probably a compiler issue (though I could not confirm yet).

> - Failures in various core file tests on PowerPC (needs investigation)

I am not seeing this on ppc64 RHEL 6.3.

> - Failures in gdb.threads/siginfo-threads.exp on s390 (needs investigation)

Fully passing for me on s390x RHEL 6.3.

> - Failures in gdb.dwarf2/dw2-icc-opaque.exp on SPU and s390 (likewise)

I can confirm on s390x, and I am also seeing on ppc64.

>> The easiest for me would probably to create it on Friday, assuming
>> that we don't discover something new by then.
>
> I'll see what I can track down and fix until Friday.  Sorry for starting
> my test series a bit late this time ...

Please keep me informed if it is possible, I am also tackling some
regressions as you are.

Thanks,

-- 
Sergio

Re: Remaining 7.5 regressions (Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses)

[Ulrich Weigand]

Sergio Durigan Junior wrote:
> On Wednesday, August 01 2012, Ulrich Weigand wrote:
> > In addition to the failures fixed by the above patches, I'm still seeing:
> >
> > - Failures in gdb.base/pc-fp.exp on various platforms, as described here:
> >   http://sourceware.org/ml/gdb-patches/2012-07/msg00823.html
> >   (just an output formatting issue)
> 
> This patch will probably go in tomorrow when I wake up, if Pedro
> approves it.  It is also simple enough to be committed to 7.5.

OK, thanks!
 
> > - Failures in gdb.mi/mi-var-rtti.exp on various platforms, see:
> >   http://sourceware.org/ml/gdb-patches/2012-07/msg00458.html
> >   (seems to be a bug in the test case)
> 
> I see them also on s390x as you pointed out, but not on ppc64.

Well, the test case reads uninitialized stack contents, so the
particulars of the effect can well depend on the platform ...

> > - Failures in gdb.threads/watchpoint-fork.exp on ARM and PowerPC.
> >   This looks like a pre-existing bug that hardware watchpoints are not
> >   handled correctly across forks, which is now exposed since a test
> >   case for this scenario was added.
> 
> I seem some failures on s390x as well:
> 
>     +FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: breakpoint after the second fork (timeout)
>     +FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: watchpoint after the second fork
>     +FAIL: gdb.threads/watchpoint-fork.exp: child: singlethreaded: finish

Yes, it's the same issue.

> > - Some new C++ regressions on ARM / s390x (could be compiler issues?)
> 
> Could you tell which C++ regressions are those?  I see this on
> PPC64/s390x:
> 
> -PASS: gdb.cp/inherit.exp: print g_vB (FIXME v3 vtbl ptr)
> -PASS: gdb.cp/inherit.exp: print g_vC (FIXME v3 vtbl ptr)
> +FAIL: gdb.cp/inherit.exp: print g_vB
> +FAIL: gdb.cp/inherit.exp: print g_vC
> ...
> -PASS: gdb.cp/inherit.exp: print g_vE (FIXME v3 vtbl ptr)
> +FAIL: gdb.cp/inherit.exp: print g_vE
> 
> -PASS: gdb.cp/virtbase.exp: print *this
> +FAIL: gdb.cp/virtbase.exp: print *this
> ...
> -PASS: gdb.cp/virtbase.exp: print *(D *) e
> +FAIL: gdb.cp/virtbase.exp: print *(D *) e

Exactly, those are the ones.  I haven't looked further ...

> > - Failures in various core file tests on PowerPC (needs investigation)
> 
> I am not seeing this on ppc64 RHEL 6.3.

This is actually not a PowerPC but a general problem; in fact it is a problem
with older kernels (I happen to run my PowerPC tests on RHEL 5 since RHEL 6
no longer supports Cell).  My patch series to make "info proc" work remotely
caused GDB to use "pread" instead of "read" to access /proc/.../maps -- and
on older kernels this just errors out with -ESPIPE.

I'm testing a patch to fall back to lseek/read if that happens.

> > - Failures in gdb.threads/siginfo-threads.exp on s390 (needs investigation)
> 
> Fully passing for me on s390x RHEL 6.3.

The failure happens only when testing with -m31; apparently the signal info
struct isn't properly converted between 31-bit and 64-bit format.

B.t.w. I'm now seeing the same issue on PowerPC with -m32 on a 64-bit
kernel ...

> > - Failures in gdb.dwarf2/dw2-icc-opaque.exp on SPU and s390 (likewise)
> 
> I can confirm on s390x, and I am also seeing on ppc64.

It's a test case bug; I'll commit a patch shortly.

In addition, I also have patches for these issues (likewise testsuite):

> - Failures in watchpoint.exp on SPU (needs investigation)
> 
> - Failures in gdb.dwarf2/dw2-icc-opaque.exp on SPU and s390 (likewise)
> 
> - Testcase harness failures when running a multi-lib configuration:
>    ERROR: tcl error sourcing ../../../gdb-7_5/gdb/testsuite/gdb.base/inferior-died.exp.
>    ERROR: can't set "seen": variable is array
>    ERROR: tcl error sourcing /home/uweigand/fsf/gdb-head/gdb/testsuite/gdb.threads/linux-dp.exp.
>    ERROR: can't array set "seen": variable isn't array
>   (Invalid re-use of a variable name?)

and I understood the reason for this issue:

> - Failures in gdb.server tests on SPU (needs investigation)

which is that the SPU gdbserver does not support multi-process mode.


Bye,
Ulrich

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

Re: Remaining 7.5 regressions (Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses)

[Joel Brobecker]

> In addition to the failures fixed by the above patches, I'm still seeing:

Would you guys mind keeping the list up to date on the 7.5 release
wiki page? This gives everyone an easy location for finding out what
still needs to be fixed before we can release...

    http://sourceware.org/gdb/wiki/GDB_7.5_Release

Thanks!
-- 
Joel

Re: Remaining 7.5 regressions (Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses)

[Ulrich Weigand]

Some updates:

> - Failures in various core file tests on PowerPC (needs investigation)

This is now fixed here:
http://sourceware.org/ml/gdb-patches/2012-08/msg00070.html

> - Failures in watchpoint.exp on SPU (needs investigation)

http://sourceware.org/ml/gdb-patches/2012-08/msg00071.html

> - Failures in gdb.dwarf2/dw2-icc-opaque.exp on SPU and s390 (likewise)

http://sourceware.org/ml/gdb-patches/2012-08/msg00072.html

> - Testcase harness failures when running a multi-lib configuration:
>    ERROR: tcl error sourcing ../../../gdb-7_5/gdb/testsuite/gdb.base/inferior-died.exp.
>    ERROR: can't set "seen": variable is array
>    ERROR: tcl error sourcing /home/uweigand/fsf/gdb-head/gdb/testsuite/gdb.threads/linux-dp.exp.
>    ERROR: can't array set "seen": variable isn't array
>   (Invalid re-use of a variable name?)

http://sourceware.org/ml/gdb-patches/2012-08/msg00073.html

Joel Brobecker wrote:

> Would you guys mind keeping the list up to date on the 7.5 release
> wiki page? This gives everyone an easy location for finding out what
> still needs to be fixed before we can release...
> 
>     http://sourceware.org/gdb/wiki/GDB_7.5_Release

I'll update the list with those issues that are still failing.

However, most of them seem to be things that actually never worked,
it's just that we didn't have tests to show it (e.g. hardware
watchpoints across forks).  Those don't look like release
blockers to me ...

Bye,
Ulrich

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

Re: Remaining 7.5 regressions (Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses)

[Ulrich Weigand]

Joel Brobecker wrote:

> Would you guys mind keeping the list up to date on the 7.5 release
> wiki page? This gives everyone an easy location for finding out what
> still needs to be fixed before we can release...
> 
>     http://sourceware.org/gdb/wiki/GDB_7.5_Release

I've completed another full test sequence on ARM, Cell (PowerPC & SPU)
and s390, both native and remote, with the current 7.5 branch.  Of the
issues I originally described, the following "real" problems remain:

- Hardware watchpoints across fork are not supported (ARM, ppc, s390)
- Missing 32-/64-bit siginfo conversion code (ppc, s390)
- SPU gdbserver does not support multi-process mode

Since all these are are pre-existing problems, they shouldn't be
treated as release blockers, so I have *not* put them on the Wiki.

In addition, there's the following testsuite-only issues:

- A few C++ test regressions (potentially compiler issues)
- Failures in gdb.base/pc-fp.exp
- Failures in gdb.mi/mi-var-rtti.exp

Sergio: I haven't been working on these ...  If you feel they
should go on the Wiki as release blockers, would you mind doing
so yourself?

Bye,
Ulrich

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

Re: Remaining 7.5 regressions (Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses)

[Sergio Durigan Junior]

On Thursday, August 02 2012, Ulrich Weigand wrote:

> Joel Brobecker wrote:
>
>> Would you guys mind keeping the list up to date on the 7.5 release
>> wiki page? This gives everyone an easy location for finding out what
>> still needs to be fixed before we can release...
>> 
>>     http://sourceware.org/gdb/wiki/GDB_7.5_Release
>
> I've completed another full test sequence on ARM, Cell (PowerPC & SPU)
> and s390, both native and remote, with the current 7.5 branch.  Of the
> issues I originally described, the following "real" problems remain:
>
> - Hardware watchpoints across fork are not supported (ARM, ppc, s390)
> - Missing 32-/64-bit siginfo conversion code (ppc, s390)
> - SPU gdbserver does not support multi-process mode
>
> Since all these are are pre-existing problems, they shouldn't be
> treated as release blockers, so I have *not* put them on the Wiki.

Thanks for this.

> In addition, there's the following testsuite-only issues:
>
> - A few C++ test regressions (potentially compiler issues)

Still couldn't find the problem in GDB.

> - Failures in gdb.base/pc-fp.exp

I have committed the patch for this.

> - Failures in gdb.mi/mi-var-rtti.exp

Probably won't have time to tackle this until tomorrow, but it does not
seem to be a blocker.

> Sergio: I haven't been working on these ...  If you feel they
> should go on the Wiki as release blockers, would you mind doing
> so yourself?

No problem.  As far as I have investigated, I do not feel they are
release blockers, so I guess we are pretty much covered for the release.

Thanks,

-- 
Sergio

Re: Remaining 7.5 regressions (Re: [ARM, commit, RFA 7.5] Fix HW breakpoints on unaligned addresses)

[Joel Brobecker]

> Since all these are are pre-existing problems, they shouldn't be
> treated as release blockers, so I have *not* put them on the Wiki.

Ah! Thanks for clarifying. I agree with you that they should not be
blocking the release.

Thanks :)
-- 
Joel