[Valgrind-developers] valgrind: r13381: fix gdbsrv inferior calls when PT_GNU_STACK declares stack not executable

[<sv...@va...>]

philippe	2013-05-02 23:06:31 +0100 (Thu, 02 May 2013)

  New Revision: 13381

  Log:
    fix gdbsrv inferior calls when PT_GNU_STACK declares stack not executable
    With rev 13368, Valgrind obeys PT_GNU_STACK making the stack not
    executable. This makes inferior function call with GDB >= 7.5 failing,
    as GDB places a breakpoint on the stack, which must be decoded
    and translated by Valgrind to have the inferior function call properly done.
    => introduce a special case in the conditions to allow translation
    when a segment is not executable but is readable and there is a
    breakpoint at the address.

  Modified files:
    trunk/coregrind/m_gdbserver/m_gdbserver.c
    trunk/coregrind/m_translate.c
    trunk/coregrind/pub_core_gdbserver.h


  Modified: trunk/coregrind/pub_core_gdbserver.h (+3 -0)
===================================================================
--- trunk/coregrind/pub_core_gdbserver.h	2013-04-27 02:34:05 +01:00 (rev 13380)
+++ trunk/coregrind/pub_core_gdbserver.h	2013-05-02 23:06:31 +01:00 (rev 13381)
@@ -76,6 +76,9 @@
 Bool VG_(gdbserver_point) (PointKind kind, Bool insert, 
                            Addr addr, int len);
 
+/* True if there is a breakpoint at addr. */
+Bool VG_(has_gdbserver_breakpoint) (Addr addr);
+
 /* Entry point invoked by vgdb when it uses ptrace to cause a gdbserver
    invocation. A magic value is passed by vgdb in check as a verification
    that the call has been properly pushed by vgdb. */

  Modified: trunk/coregrind/m_gdbserver/m_gdbserver.c (+9 -0)
===================================================================
--- trunk/coregrind/m_gdbserver/m_gdbserver.c	2013-04-27 02:34:05 +01:00 (rev 13380)
+++ trunk/coregrind/m_gdbserver/m_gdbserver.c	2013-05-02 23:06:31 +01:00 (rev 13381)
@@ -394,6 +394,15 @@
    return True;
 }
 
+Bool VG_(has_gdbserver_breakpoint) (Addr addr)
+{
+   GS_Address *g;
+   if (!gdbserver_called)
+      return False;
+   g = VG_(HT_lookup) (gs_addresses, (UWord)HT_addr(addr));
+   return (g != NULL && g->kind == GS_break);
+}
+
 Bool VG_(is_watched)(PointKind kind, Addr addr, Int szB)
 {
    Word n_elems;

  Modified: trunk/coregrind/m_translate.c (+14 -5)
===================================================================
--- trunk/coregrind/m_translate.c	2013-04-27 02:34:05 +01:00 (rev 13380)
+++ trunk/coregrind/m_translate.c	2013-05-02 23:06:31 +01:00 (rev 13381)
@@ -745,9 +745,9 @@
 /* --------- Various helper functions for translation --------- */
 
 /* Look for reasons to disallow making translations from the given
-   segment. */
+   segment/addr. */
 
-static Bool translations_allowable_from_seg ( NSegment const* seg )
+static Bool translations_allowable_from_seg ( NSegment const* seg, Addr addr )
 {
 #  if defined(VGA_x86) || defined(VGA_s390x) || defined(VGA_mips32) \
       || defined(VGA_mips64)
@@ -757,7 +757,16 @@
 #  endif
    return seg != NULL
           && (seg->kind == SkAnonC || seg->kind == SkFileC || seg->kind == SkShmC)
-          && (seg->hasX || (seg->hasR && allowR));
+          && (seg->hasX 
+              || (seg->hasR && (allowR
+                                || VG_(has_gdbserver_breakpoint) (addr))));
+   /* If GDB/gdbsrv has inserted a breakpoint at addr, assume this is a valid
+      location to translate if seg is not executable but is readable.
+      This is needed for inferior function calls from GDB: GDB inserts a
+      breakpoint on the stack, and expects to regain control before the
+      breakpoint instruction at the breakpoint address is really
+      executed. For this, the breakpoint instruction must be translated
+      so as to have the call to gdbserver executed. */
 }
 
 
@@ -852,7 +861,7 @@
       allow a chase. */
 
    /* Destination not in a plausible segment? */
-   if (!translations_allowable_from_seg(seg))
+   if (!translations_allowable_from_seg(seg, addr))
       goto dontchase;
 
    /* Destination is redirected? */
@@ -1418,7 +1427,7 @@
    { /* BEGIN new scope specially for 'seg' */
    NSegment const* seg = VG_(am_find_nsegment)(addr);
 
-   if ( (!translations_allowable_from_seg(seg))
+   if ( (!translations_allowable_from_seg(seg, addr))
         || addr == TRANSTAB_BOGUS_GUEST_ADDR ) {
       if (VG_(clo_trace_signals))
          VG_(message)(Vg_DebugMsg, "translations not allowed here (0x%llx)"