|
From: <sv...@va...> - 2011-05-27 13:28:36
|
Author: sewardj
Date: 2011-05-27 14:23:44 +0100 (Fri, 27 May 2011)
New Revision: 11779
Log:
Further fixes for GDB server on Thumb code:
* Disabled several tests on ARM when gdb version < 7.1
gdb 7.0 has problems with next/step/... in ARM thumb code.
* Documented in manual-core.xml that ARM thumb code implies
a gdb version >= 7.1
* m_gdbserver.h/.c : take into account the thumb bit at several places
* use new IRStmt_IMark::delta field to distinguish ARM vs Thumb
instructions as committed in vex r2153
Patch from bug 214909 comment 99 (valgrind part).
(Philippe Waroquiers, phi...@sk...)
Modified:
trunk/coregrind/m_gdbserver/m_gdbserver.c
trunk/docs/xml/manual-core.xml
trunk/gdbserver_tests/filter_gdb
trunk/gdbserver_tests/make_local_links
trunk/gdbserver_tests/mcbreak.vgtest
trunk/gdbserver_tests/mcinfcallWSRU.stderrB.exp
trunk/gdbserver_tests/mcinfcallWSRU.vgtest
trunk/gdbserver_tests/mcwatchpoints.vgtest
Modified: trunk/coregrind/m_gdbserver/m_gdbserver.c
===================================================================
--- trunk/coregrind/m_gdbserver/m_gdbserver.c 2011-05-18 16:09:54 UTC (rev 11778)
+++ trunk/coregrind/m_gdbserver/m_gdbserver.c 2011-05-27 13:23:44 UTC (rev 11779)
@@ -84,10 +84,11 @@
/* An instruction instrumented for gdbserver looks like this:
1. Ist_Mark (0x1234)
- 2. helperc_CallDebugger (0x1234)
+ 2. Put (IP, 0x1234)
+ 3. helperc_CallDebugger (0x1234)
This will give control to gdb if there is a break at 0x1234
or if we are single stepping
- 3. ... here the real IR for the instruction at 0x1234
+ 4. ... here the real IR for the instruction at 0x1234
When there is a break at 0x1234:
if user does "continue" or "step" or similar,
@@ -180,24 +181,41 @@
single stepping (kind == GS_jump).
When gdbserver is not single stepping anymore, all GS_jump entries
are removed, their translations are invalidated.
+
+ Note for ARM: addr in GS_Address is the value without the thumb bit set.
*/
static VgHashTable gs_addresses = NULL;
+// Transform addr in the form stored in the list of addresses.
+// For the ARM architecture, we store it with the thumb bit set to 0.
+static Addr HT_addr ( Addr addr )
+{
+#if defined(VGA_arm)
+ return addr & ~(Addr)1;
+#else
+ return addr;
+#endif
+}
+
static void add_gs_address (Addr addr, GS_Kind kind, char* from)
{
GS_Address *p;
p = VG_(arena_malloc)(VG_AR_CORE, from, sizeof(GS_Address));
- p->addr = addr;
+ p->addr = HT_addr (addr);
p->kind = kind;
VG_(HT_add_node)(gs_addresses, p);
- VG_(discard_translations) (addr, 1, from);
+ /* It should be sufficient to discard a range of 1.
+ We use 2 to ensure the below is not sensitive to the presence
+ of thumb bit in the range of addresses to discard. */
+ VG_(discard_translations) (addr, 2, from);
}
static void remove_gs_address (GS_Address* g, char* from)
{
VG_(HT_remove) (gs_addresses, g->addr);
- VG_(discard_translations) (g->addr, 1, from);
+ // See add_gs_address for the explanation for the range 2 below.
+ VG_(discard_translations) (g->addr, 2, from);
VG_(arena_free) (VG_AR_CORE, g);
}
@@ -231,7 +249,7 @@
{
GS_Address *g;
- g = VG_(HT_lookup) (gs_addresses, (UWord)addr);
+ g = VG_(HT_lookup) (gs_addresses, (UWord)HT_addr(addr));
if (insert) {
/* insert a breakpoint at addr or upgrade its kind */
if (g == NULL) {
@@ -419,7 +437,8 @@
VG_(HT_ResetIter) (gs_addresses);
while ((g = VG_(HT_Next) (gs_addresses))) {
for (e = 0; e < vge->n_used; e++) {
- if (g->addr >= vge->base[e] && g->addr < vge->base[e] + vge->len[e]) {
+ if (g->addr >= HT_addr(vge->base[e])
+ && g->addr < HT_addr(vge->base[e]) + vge->len[e]) {
dlog(2,
"gdbserver_instrumentation_needed %p %s reason %s\n",
C2v(g->addr), sym(g->addr, /* is_code */ True),
@@ -484,7 +503,7 @@
static void invalidate_if_jump_not_yet_gdbserved (Addr addr, char* from)
{
- if (VG_(HT_lookup) (gs_addresses, (UWord)addr))
+ if (VG_(HT_lookup) (gs_addresses, (UWord)HT_addr(addr)))
return;
add_gs_address (addr, GS_jump, from);
}
@@ -859,9 +878,9 @@
return;
if (valgrind_single_stepping() ||
- ((g = VG_(HT_lookup) (gs_addresses, (UWord)iaddr)) &&
+ ((g = VG_(HT_lookup) (gs_addresses, (UWord)HT_addr(iaddr))) &&
(g->kind == GS_break))) {
- if (iaddr == ignore_this_break_once) {
+ if (iaddr == HT_addr(ignore_this_break_once)) {
dlog(1, "ignoring ignore_this_break_once %s\n",
sym(ignore_this_break_once, /* is_code */ True));
ignore_this_break_once = 0;
@@ -952,7 +971,8 @@
VexGuestLayout* layout,
VexGuestExtents* vge,
IRType gWordTy, IRType hWordTy,
- Addr iaddr, /* Addr of instruction being instrumented */
+ Addr iaddr, /* Addr of instruction being instrumented */
+ UChar delta, /* delta to add to iaddr to obtain IP */
IRSB* irsb) /* irsb block to which call is added */
{
void* fn;
@@ -969,9 +989,18 @@
remove the redundant store. And in any case, when debugging a
piece of code, the efficiency requirement is not critical: very
few blocks will be instrumented for debugging. */
-
- addStmtToIRSB(irsb, IRStmt_Put(layout->offset_IP , mkIRExpr_HWord(iaddr)));
+ /* For platforms on which the IP can differ from the addr of the instruction
+ being executed, we need to add the delta to obtain the IP.
+ This IP will be given to gdb (e.g. if a breakpoint is put at iaddr).
+
+ For ARM, this delta will ensure that the thumb bit is set in the
+ IP when executing thumb code. gdb uses this thumb bit a.o.
+ to properly guess the next IP for the 'step' and 'stepi' commands. */
+ vg_assert(delta <= 1);
+ addStmtToIRSB(irsb, IRStmt_Put(layout->offset_IP ,
+ mkIRExpr_HWord(iaddr + (Addr)delta)));
+
fn = &VG_(helperc_CallDebugger);
nm = "VG_(helperc_CallDebugger)";
args = mkIRExprVec_1(mkIRExpr_HWord (iaddr));
@@ -1065,6 +1094,7 @@
VG_(add_stmt_call_gdbserver) ( sb_in, layout, vge,
gWordTy, hWordTy,
st->Ist.IMark.addr,
+ st->Ist.IMark.delta,
sb_out);
/* There is an optimisation possible here for Vg_VgdbFull:
Put a guard ensuring we only call gdbserver if 'FullCallNeeded'.
Modified: trunk/docs/xml/manual-core.xml
===================================================================
--- trunk/docs/xml/manual-core.xml 2011-05-18 16:09:54 UTC (rev 11778)
+++ trunk/docs/xml/manual-core.xml 2011-05-27 13:23:44 UTC (rev 11779)
@@ -2248,6 +2248,9 @@
will report an error message when using the target
command. Debugging will not work because gdb will then not be
able to fetch the registers from the Valgrind gdbserver.
+ For ARM programs using the thumb instruction set, you must use
+ a gdb version >= 7.1 as previous versions have problems
+ with next/step/breakpoints/... in thumb code.
</para>
</listitem>
Modified: trunk/gdbserver_tests/filter_gdb
===================================================================
--- trunk/gdbserver_tests/filter_gdb 2011-05-18 16:09:54 UTC (rev 11778)
+++ trunk/gdbserver_tests/filter_gdb 2011-05-27 13:23:44 UTC (rev 11779)
@@ -46,6 +46,7 @@
# a location in the nptl lib rather than our sources (same as
# standard gdb gdbserver) gdb 7.0
# same special transform but for gdb 7.2
+# transform info thread of 7.3 into the layout of 7.2 and before.
# delete lines telling that some memory can't be accessed: this is
# a.o. produced by gdb 7.2 on arm (same with standard gdbserver)
# delete empty lines (the last line (only made of prompts) sometimes
@@ -58,6 +59,7 @@
-e 's/pid [0-9][0-9]*/pid ..../g' \
-e 's/Thread [0-9][0-9]*/Thread ..../g' \
-e '/\[Switching to Thread ....\]/d' \
+ -e 's/\(\[Switching to thread [1234] (Thread ....)\]\)#0/\1\n#0/' \
-e 's/^\([ \* ] [0-9] Thread .... (tid [0-9] VgTs_WaitSys) 0x........ in\).*$/\1 syscall .../' \
-e 's/#[0-9]\( 0x........ in sleeper_or_burner\)/#.\1/' \
-e '/^Reading symbols from .*\.\.\.done\./d' \
@@ -88,9 +90,13 @@
-e 's/0x........ in \(main (argc=1, argv=0x........) at watchpoints.c:[24][3689]\)/\1/' \
-e 's/0x........ in \(main () at clean_after_fork.c:32\)/\1/' \
-e 's/^0x........ in \*__GI_raise (sig=8).*/0x........ in test4 () at faultstatus.c:120\n120 volatile int v = 44\/zero();/' \
+ -e 's/^0x........ in raise (.*/0x........ in test4 () at faultstatus.c:120\n120 volatile int v = 44\/zero();/' \
-e '/ at ..\/nptl\/sysdeps\/unix\/sysv\/linux\/raise.c:[0-9]*/d' \
- -e '/ in ..\/nptl\/sysdeps\/unix\/sysv\/linux\/raise.c/d' \
+ -e '/ in ..\/nptl\/sysdeps\/unix\/sysv\/linux\/.*raise.c/d' \
-e 's/^0x........ in \.\{0,1\}raise () from \/lib[0-9]\{0,2\}\/libc\.so\../0x........ in test4 () at faultstatus.c:120\n120 volatile int v = 44\/zero();'/ \
+ -e '/Id Target Id Frame/d' \
+ -e 's/^\([ \*] [1234] \) *Thread /\1Thread /' \
+ -e 's/VgTs_WaitSys) 0x/VgTs_WaitSys) 0x/' \
-e '/Cannot access memory at address 0x......../d' \
-e '/^$/d' |
Modified: trunk/gdbserver_tests/make_local_links
===================================================================
--- trunk/gdbserver_tests/make_local_links 2011-05-18 16:09:54 UTC (rev 11778)
+++ trunk/gdbserver_tests/make_local_links 2011-05-27 13:23:44 UTC (rev 11779)
@@ -23,16 +23,33 @@
awk '{ if ( ($1 >= 7) || (($1 == 6) && ($2 >= 5)) ) print "version ok"}'`
if [ "$VERSIONOK" = "" ]
then
- echo "gdbserver tests suppressed as $1 version is not >= 6.5: " $VERSIONLINE
- rm gdbserver_tests/gdb
+ echo "gdbserver tests suppressed as $1 version is < 6.5: " $VERSIONLINE
+ rm -f gdbserver_tests/gdb
fi
+ # We need at least a 7.1 version on ARM to run tests doing step/next/...
+ # (gdb 7.0 has bugs in the 'guess next pc' heuristic in thumb mode).
+ if tests/arch_test arm
+ then
+ VERSIONOK=`echo $VERSION |
+ awk '{ if ( ($1 >= 8) || (($1 == 7) && ($2 >= 1)) ) print "version ok"}'`
+ if [ "$VERSIONOK" = "" ]
+ then
+ echo "gdbserver 'step/next' tests suppressed as arm $1 version is < 7.1: " $VERSIONLINE
+ rm -f gdbserver_tests/gdb.step
+ else
+ touch gdbserver_tests/gdb.step
+ fi
+ else
+ touch gdbserver_tests/gdb.step
+ fi
+
# We need at least a 7.2 version for gdb tests using eval command
VERSIONOK=`echo $VERSION |
awk '{ if ( ($1 >= 8) || (($1 == 7) && ($2 >= 2)) ) print "version ok"}'`
if [ "$VERSIONOK" = "" ]
then
- echo "gdbserver eval tests suppressed as $1 version is not >= 7.2: " $VERSIONLINE
+ echo "gdbserver eval tests suppressed as $1 version is < 7.2: " $VERSIONLINE
rm -f gdbserver_tests/gdb.eval
else
touch gdbserver_tests/gdb.eval
Modified: trunk/gdbserver_tests/mcbreak.vgtest
===================================================================
--- trunk/gdbserver_tests/mcbreak.vgtest 2011-05-18 16:09:54 UTC (rev 11778)
+++ trunk/gdbserver_tests/mcbreak.vgtest 2011-05-27 13:23:44 UTC (rev 11779)
@@ -1,6 +1,6 @@
# test execution control (break, next, step) and inferior calls
# when stopped on these events
-prereq: test -e gdb
+prereq: test -e gdb -a -f gdb.step
prog: t
vgopts: --tool=memcheck --vgdb=yes --vgdb-error=0 --vgdb-prefix=./vgdb-prefix-mcbreak
stdout_filter: filter_gdb
Modified: trunk/gdbserver_tests/mcinfcallWSRU.stderrB.exp
===================================================================
--- trunk/gdbserver_tests/mcinfcallWSRU.stderrB.exp 2011-05-18 16:09:54 UTC (rev 11778)
+++ trunk/gdbserver_tests/mcinfcallWSRU.stderrB.exp 2011-05-27 13:23:44 UTC (rev 11779)
@@ -20,10 +20,12 @@
Program received signal SIGTRAP, Trace/breakpoint trap.
0x........ in do_burn () at sleepers.c:39
39 for (i = 0; i < burn; i++) loopnr++;
-[Switching to thread 1 (Thread ....)]#0 0x........ in do_burn () at sleepers.c:39
+[Switching to thread 1 (Thread ....)]
+#0 0x........ in do_burn () at sleepers.c:39
39 for (i = 0; i < burn; i++) loopnr++;
$1 = void
-[Switching to thread 2 (Thread ....)]#0 0x........ in syscall ...
+[Switching to thread 2 (Thread ....)]
+#0 0x........ in syscall ...
Could not write register "xxx"; remote failure reply 'E.
ERROR changing register xxx regno y
gdb commands changing registers (pc, sp, ...) (e.g. 'jump',
@@ -31,7 +33,8 @@
can only be accepted if the thread is VgTs_Runnable or VgTs_Yielding state
Thread status is VgTs_WaitSys
'
-[Switching to thread 3 (Thread ....)]#0 0x........ in syscall ...
+[Switching to thread 3 (Thread ....)]
+#0 0x........ in syscall ...
Could not write register "xxx"; remote failure reply 'E.
ERROR changing register xxx regno y
gdb commands changing registers (pc, sp, ...) (e.g. 'jump',
@@ -39,7 +42,8 @@
can only be accepted if the thread is VgTs_Runnable or VgTs_Yielding state
Thread status is VgTs_WaitSys
'
-[Switching to thread 4 (Thread ....)]#0 0x........ in syscall ...
+[Switching to thread 4 (Thread ....)]
+#0 0x........ in syscall ...
Could not write register "xxx"; remote failure reply 'E.
ERROR changing register xxx regno y
gdb commands changing registers (pc, sp, ...) (e.g. 'jump',
Modified: trunk/gdbserver_tests/mcinfcallWSRU.vgtest
===================================================================
--- trunk/gdbserver_tests/mcinfcallWSRU.vgtest 2011-05-18 16:09:54 UTC (rev 11778)
+++ trunk/gdbserver_tests/mcinfcallWSRU.vgtest 2011-05-27 13:23:44 UTC (rev 11779)
@@ -5,8 +5,9 @@
# but this introduces too much dependencies to scheduler fairness.
args: 100 100000000 1000000000 -S-S-SB-
vgopts: --tool=memcheck --vgdb=yes --vgdb-error=0 --vgdb-prefix=./vgdb-prefix-mcinfcallWSRU
+# We need a non buggy gdb.step on arm thumb.
# Disable on Darwin: inferior call rejected as it cannot find malloc.
-prereq: test -e gdb && ../tests/os_test linux
+prereq: test -e gdb -a -f gdb.step && ../tests/os_test linux
# filter_gdb to replace pid and Thread numbers in the output of the program:
stderr_filter: filter_gdb
progB: gdb
Modified: trunk/gdbserver_tests/mcwatchpoints.vgtest
===================================================================
--- trunk/gdbserver_tests/mcwatchpoints.vgtest 2011-05-18 16:09:54 UTC (rev 11778)
+++ trunk/gdbserver_tests/mcwatchpoints.vgtest 2011-05-27 13:23:44 UTC (rev 11779)
@@ -1,6 +1,7 @@
# test the memcheck watchpoint functionality
# Note: we need --vgdb=full to stop at the instruction following the watchpoint.
-prereq: test -e gdb
+# We need a non buggy gdb.step as watchpoints are placed after a "step".
+prereq: test -e gdb -a -f gdb.step
prog: watchpoints
vgopts: --tool=memcheck --vgdb=full --vgdb-error=0 --vgdb-prefix=./vgdb-prefix-mcwatchpoints
stdout_filter: filter_make_empty
|