From: Julian Seward Date: Fri, 27 May 2011 13:23:44 +0000 (+0000) Subject: Further fixes for GDB server on Thumb code: X-Git-Tag: svn/VALGRIND_3_7_0~458 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dd513678831a73feb7e7a3d1c6cc9f4c09a36b7a;p=thirdparty%2Fvalgrind.git 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, philippe.waroquiers@skynet.be) git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11779 --- diff --git a/coregrind/m_gdbserver/m_gdbserver.c b/coregrind/m_gdbserver/m_gdbserver.c index e3b0ffb71a..44669064fd 100644 --- a/coregrind/m_gdbserver/m_gdbserver.c +++ b/coregrind/m_gdbserver/m_gdbserver.c @@ -84,10 +84,11 @@ static char* ppCallReason(CallReason reason) /* 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 @@ typedef 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 @@ static void breakpoint (Bool insert, CORE_ADDR addr) { 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 @@ static VgVgdb VG_(gdbserver_instrumentation_needed) (VexGuestExtents* vge) 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 clear_watched_addresses(void) 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 @@ void VG_(helperc_CallDebugger) ( HWord iaddr ) 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 @@ static void VG_(add_stmt_call_gdbserver) 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,8 +989,17 @@ static void VG_(add_stmt_call_gdbserver) 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)"; @@ -1065,6 +1094,7 @@ IRSB* VG_(instrument_for_gdbserver_if_needed) 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'. diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml index 368b20beb9..3666aa51ba 100644 --- a/docs/xml/manual-core.xml +++ b/docs/xml/manual-core.xml @@ -2248,6 +2248,9 @@ Reading symbols from /lib/libc.so.6...(no debugging symbols found)...done. 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. diff --git a/gdbserver_tests/filter_gdb b/gdbserver_tests/filter_gdb index 42f494d13d..bb7b0eebea 100755 --- a/gdbserver_tests/filter_gdb +++ b/gdbserver_tests/filter_gdb @@ -46,6 +46,7 @@ $dir/filter_memcheck_monitor | # 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 @@ sed -e '/Remote debugging using/,/vgdb launched process attached/d' -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 @@ sed -e '/Remote debugging using/,/vgdb launched process attached/d' -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' | diff --git a/gdbserver_tests/make_local_links b/gdbserver_tests/make_local_links index c1355cad3d..7fbdd3352d 100755 --- a/gdbserver_tests/make_local_links +++ b/gdbserver_tests/make_local_links @@ -23,8 +23,25 @@ then 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 @@ -32,7 +49,7 @@ then 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 diff --git a/gdbserver_tests/mcbreak.vgtest b/gdbserver_tests/mcbreak.vgtest index afa6ad5838..d42fbd0d1a 100644 --- a/gdbserver_tests/mcbreak.vgtest +++ b/gdbserver_tests/mcbreak.vgtest @@ -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 diff --git a/gdbserver_tests/mcinfcallWSRU.stderrB.exp b/gdbserver_tests/mcinfcallWSRU.stderrB.exp index 860e07ef69..137e19a2ce 100644 --- a/gdbserver_tests/mcinfcallWSRU.stderrB.exp +++ b/gdbserver_tests/mcinfcallWSRU.stderrB.exp @@ -20,10 +20,12 @@ Continuing. 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 @@ set pc, calling from gdb a function in the debugged process, ...) 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 @@ set pc, calling from gdb a function in the debugged process, ...) 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', diff --git a/gdbserver_tests/mcinfcallWSRU.vgtest b/gdbserver_tests/mcinfcallWSRU.vgtest index 0bb4b6ad21..f9c1983af8 100644 --- a/gdbserver_tests/mcinfcallWSRU.vgtest +++ b/gdbserver_tests/mcinfcallWSRU.vgtest @@ -5,8 +5,9 @@ prog: sleepers # 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 diff --git a/gdbserver_tests/mcwatchpoints.vgtest b/gdbserver_tests/mcwatchpoints.vgtest index f50405327a..c1152296c5 100644 --- a/gdbserver_tests/mcwatchpoints.vgtest +++ b/gdbserver_tests/mcwatchpoints.vgtest @@ -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