Followup fix to r11006. Don't pass va_list by value through client

requests, since there's no guarantee it is the same size as a machine
word.

This renames the private client request VG_USERREQ__INTERNAL_PRINTF to
VG_USERREQ__INTERNAL_PRINTF_VALIST_BY_REF and changes the
argument-passing accordingly.

The public client requests VG_USERREQ__PRINTF and
VG_USERREQ__PRINTF_BACKTRACE are now deprecated, and handled only in
the case where sizeof(UWord) == sizeof(va_list).  In all other cases V
will now print a detailed error message and abort.  This breaks binary
compatibility of apps compiled using VALGRIND_PRINTF and
VALGRIND_PRINTF_BACKTRACE, but that's not easy to avoid.

VG_USERREQ__PRINTF and VG_USERREQ__PRINTF_BACKTRACE are now replaced
by VG_USERREQ__PRINTF_VALIST_BY_REF and
VG_USERREQ__PRINTF_BACKTRACE_VALIST_BY_REF.  The end-user macros
VALGRIND_PRINTF and VALGRIND_PRINTF_BACKTRACE have been adjusted to
use these new requests instead.

Overall result is that source level compatibility of code using
VALGRIND_PRINTF{,_BACKTRACE} is retained, but binary level
compatibility may be broken, necessitating a rebuild of code using
these macros.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11032

diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c
index 67fb7bf0e3e954d462633febef282eb3d1e632cb..fbc93ca3aac661f031eddd2644677e8d53877a4f 100644 (file)
--- a/coregrind/m_scheduler/scheduler.c
+++ b/coregrind/m_scheduler/scheduler.c
@@ -1408,50 +1408,73 @@ void do_client_request ( ThreadId tid )
          break;
 
       case VG_USERREQ__PRINTF: {
+         /* JRS 2010-Jan-28: this is DEPRECATED; use the
+            _VALIST_BY_REF version instead */
+         if (sizeof(va_list) != sizeof(UWord))
+            goto va_list_casting_error_NORETURN;
          union {
             va_list vargs;
-            unsigned long ul;
-         } args;
-         Int count;
-         args.ul = (unsigned long)arg[2];
-         count =
-            VG_(vmessage)( Vg_ClientMsg, (char *)arg[1], args.vargs );
-            VG_(message_flush)();
-            SET_CLREQ_RETVAL( tid, count );
-         break; }
+            unsigned long uw;
+         } u;
+         u.uw = (unsigned long)arg[2];
+         Int count = 
+            VG_(vmessage)( Vg_ClientMsg, (char *)arg[1], u.vargs );
+         VG_(message_flush)();
+         SET_CLREQ_RETVAL( tid, count );
+         break;
+      }
 
-      case VG_USERREQ__INTERNAL_PRINTF: {
+      case VG_USERREQ__PRINTF_BACKTRACE: {
+         /* JRS 2010-Jan-28: this is DEPRECATED; use the
+            _VALIST_BY_REF version instead */
+         if (sizeof(va_list) != sizeof(UWord))
+            goto va_list_casting_error_NORETURN;
          union {
             va_list vargs;
-            unsigned long ul;
-         } args;
-         Int count;
-         args.ul = (unsigned long)arg[2];
-         count =
-            VG_(vmessage)( Vg_DebugMsg, (char *)arg[1], args.vargs );
-            VG_(message_flush)();
-            SET_CLREQ_RETVAL( tid, count );
-         break; }
+            unsigned long uw;
+         } u;
+         u.uw = (unsigned long)arg[2];
+         Int count =
+            VG_(vmessage)( Vg_ClientMsg, (char *)arg[1], u.vargs );
+         VG_(message_flush)();
+         VG_(get_and_pp_StackTrace)( tid, VG_(clo_backtrace_size) );
+         SET_CLREQ_RETVAL( tid, count );
+         break;
+      }
+
+      case VG_USERREQ__PRINTF_VALIST_BY_REF: {
+         va_list* vargsp = (va_list*)arg[2];
+         Int count = 
+            VG_(vmessage)( Vg_ClientMsg, (char *)arg[1], *vargsp );
+         VG_(message_flush)();
+         SET_CLREQ_RETVAL( tid, count );
+         break;
+      }
+
+      case VG_USERREQ__PRINTF_BACKTRACE_VALIST_BY_REF: {
+         va_list* vargsp = (va_list*)arg[2];
+         Int count =
+            VG_(vmessage)( Vg_ClientMsg, (char *)arg[1], *vargsp );
+         VG_(message_flush)();
+         VG_(get_and_pp_StackTrace)( tid, VG_(clo_backtrace_size) );
+         SET_CLREQ_RETVAL( tid, count );
+         break;
+      }
+
+      case VG_USERREQ__INTERNAL_PRINTF_VALIST_BY_REF: {
+         va_list* vargsp = (va_list*)arg[2];
+         Int count = 
+            VG_(vmessage)( Vg_DebugMsg, (char *)arg[1], *vargsp );
+         VG_(message_flush)();
+         SET_CLREQ_RETVAL( tid, count );
+         break;
+      }
 
       case VG_USERREQ__ADD_IFUNC_TARGET: {
          VG_(redir_add_ifunc_target)( arg[1], arg[2] );
          SET_CLREQ_RETVAL( tid, 0);
          break; }
 
-      case VG_USERREQ__PRINTF_BACKTRACE: {
-         union {
-            va_list vargs;
-            unsigned long ul;
-         } args;
-         Int count;
-         args.ul = (unsigned long)arg[2];
-         count =
-            VG_(vmessage)( Vg_ClientMsg, (char *)arg[1], args.vargs );
-            VG_(message_flush)();
-            VG_(get_and_pp_StackTrace)( tid, VG_(clo_backtrace_size) );
-            SET_CLREQ_RETVAL( tid, count );
-         break; }
-
       case VG_USERREQ__STACK_REGISTER: {
          UWord sid = VG_(register_stack)((Addr)arg[1], (Addr)arg[2]);
          SET_CLREQ_RETVAL( tid, sid );
@@ -1555,6 +1578,32 @@ void do_client_request ( ThreadId tid )
          }
          break;
    }
+   return;
+
+   /*NOTREACHED*/
+  va_list_casting_error_NORETURN:
+   VG_(umsg)(
+      "Valgrind: fatal error - cannot continue: use of the deprecated\n"
+      "client requests VG_USERREQ__PRINTF or VG_USERREQ__PRINTF_BACKTRACE\n"
+      "on a platform where they cannot be supported.  Please use the\n"
+      "equivalent _VALIST_BY_REF versions instead.\n"
+      "\n"
+      "This is a binary-incompatible change in Valgrind's client request\n"
+      "mechanism.  It is unfortunate, but difficult to avoid.  End-users\n"
+      "are expected to almost never see this message.  The only case in\n"
+      "which you might see this message is if your code uses the macros\n"
+      "VALGRIND_PRINTF or VALGRIND_PRINTF_BACKTRACE.  If so, you will need\n"
+      "to recompile such code, using the header files from this version of\n"
+      "Valgrind, and not any previous version.\n"
+      "\n"
+      "If you see this mesage in any other circumstances, it is probably\n"
+      "a bug in Valgrind.  In this case, please file a bug report at\n"
+      "\n"
+      "   http://www.valgrind.org/support/bug_reports.html\n"
+      "\n"
+      "Will now abort.\n"
+   );
+   vg_assert(0);
 }
 
 

diff --git a/coregrind/pub_core_clreq.h b/coregrind/pub_core_clreq.h
index d99802188f4a043f74b5086f0efaed11df4e582c..1585d71a0f71bb1def8ba56172e80bb635b641ec 100644 (file)
--- a/coregrind/pub_core_clreq.h
+++ b/coregrind/pub_core_clreq.h
@@ -47,8 +47,8 @@ typedef
       /* Get the tool's malloc-wrapping functions */
       VG_USERREQ__GET_MALLOCFUNCS   = 0x3030,
 
-      /* Internal equivalent of VALGRIND_PRINTF . */
-      VG_USERREQ__INTERNAL_PRINTF   = 0x3103,
+      /* Internal equivalent of VALGRIND_PRINTF_VALIST_BY_REF . */
+      VG_USERREQ__INTERNAL_PRINTF_VALIST_BY_REF = 0x3103,
 
       /* Add a target for an indirect function redirection. */
       VG_USERREQ__ADD_IFUNC_TARGET  = 0x3104,
@@ -64,16 +64,13 @@ static int VALGRIND_INTERNAL_PRINTF(const char *format, ...)
 static int VALGRIND_INTERNAL_PRINTF(const char *format, ...)
 {
    unsigned long _qzz_res = 0;
-   union {
-      va_list vargs;
-      unsigned long ul;
-   } args;
-   va_start(args.vargs, format);
+   va_list vargs;
+   va_start(vargs, format);
    VALGRIND_DO_CLIENT_REQUEST(
-      _qzz_res, 0, VG_USERREQ__INTERNAL_PRINTF,
-      (unsigned long)format, (unsigned long)(args.ul), 0, 0, 0
+      _qzz_res, 0, VG_USERREQ__INTERNAL_PRINTF_VALIST_BY_REF,
+      (unsigned long)format, (unsigned long)&vargs, 0, 0, 0
    );
-   va_end(args.vargs);
+   va_end(vargs);
    return _qzz_res;
 }
 

diff --git a/include/valgrind.h b/include/valgrind.h
index e311f66acdb2c5ad2632c22d56c35e7f5a645246..ca10b1a0ddfd6d7a1a0f37e376e68b04cb1a1dea 100644 (file)
--- a/include/valgrind.h
+++ b/include/valgrind.h
@@ -4122,8 +4122,17 @@ typedef
           VG_USERREQ__MEMPOOL_EXISTS   = 0x130a,
 
           /* Allow printfs to valgrind log. */
+          /* The first two pass the va_list argument by value, which
+             assumes it is the same size as or smaller than a UWord,
+             which generally isn't the case.  Hence are deprecated.
+             The second two pass the vargs by reference and so are
+             immune to this problem. */
+          /* both :: char* fmt, va_list vargs (DEPRECATED) */
           VG_USERREQ__PRINTF           = 0x1401,
           VG_USERREQ__PRINTF_BACKTRACE = 0x1402,
+          /* both :: char* fmt, va_list* vargs */
+          VG_USERREQ__PRINTF_VALIST_BY_REF = 0x1403,
+          VG_USERREQ__PRINTF_BACKTRACE_VALIST_BY_REF = 0x1404,
 
           /* Stack support. */
           VG_USERREQ__STACK_REGISTER   = 0x1501,
@@ -4183,16 +4192,14 @@ static int
 VALGRIND_PRINTF(const char *format, ...)
 {
    unsigned long _qzz_res;
-   union {
-      va_list vargs;
-      unsigned long ul;
-   } args;
-   va_start(args.vargs, format);
-   VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, VG_USERREQ__PRINTF,
+   va_list vargs;
+   va_start(vargs, format);
+   VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0,
+                              VG_USERREQ__PRINTF_VALIST_BY_REF,
                               (unsigned long)format,
-                              (unsigned long)(args.ul),
+                              (unsigned long)&vargs, 
                               0, 0, 0);
-   va_end(args.vargs);
+   va_end(vargs);
    return (int)_qzz_res;
 }
 
@@ -4202,16 +4209,14 @@ static int
 VALGRIND_PRINTF_BACKTRACE(const char *format, ...)
 {
    unsigned long _qzz_res;
-   union {
-      va_list vargs;
-      unsigned long ul;
-   } args;
-   va_start(args.vargs, format);
-   VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0, VG_USERREQ__PRINTF_BACKTRACE,
+   va_list vargs;
+   va_start(vargs, format);
+   VALGRIND_DO_CLIENT_REQUEST(_qzz_res, 0,
+                              VG_USERREQ__PRINTF_BACKTRACE_VALIST_BY_REF,
                               (unsigned long)format,
-                              (unsigned long)(args.ul),
+                              (unsigned long)&vargs, 
                               0, 0, 0);
-   va_end(args.vargs);
+   va_end(vargs);
    return (int)_qzz_res;
 }