From: Julian Seward Date: Thu, 28 Jan 2010 15:23:54 +0000 (+0000) Subject: Followup fix to r11006. Don't pass va_list by value through client X-Git-Tag: svn/VALGRIND_3_6_0~392 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=eb5d6433bdd7547a21dce6edc73817297796e52e;p=thirdparty%2Fvalgrind.git 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 67fb7bf0e3..fbc93ca3aa 100644 --- 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 d99802188f..1585d71a0f 100644 --- 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 e311f66acd..ca10b1a0dd 100644 --- 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; }