Menu
▾
▴
[Valgrind-developers] Valgrind: r14157 - in /trunk: NEWS memcheck/docs/mc-manual.xml memcheck/mc_errors.c memcheck/mc_include.h memcheck/mc_malloc_wrappers.c memcheck/tests/accounting.stderr.exp memcheck/tests/malloc3.stderr.exp
Author: florian
Date: Sun Jul 13 14:41:55 2014
New Revision: 14157
Log:
Provide a back trace when a function argument of a known allocation
function is presumably negative. Fixes BZ 79311.
Modified:
trunk/NEWS
trunk/memcheck/docs/mc-manual.xml
trunk/memcheck/mc_errors.c
trunk/memcheck/mc_include.h
trunk/memcheck/mc_malloc_wrappers.c
trunk/memcheck/tests/accounting.stderr.exp
trunk/memcheck/tests/malloc3.stderr.exp
Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Sun Jul 13 14:41:55 2014
@@ -71,6 +71,9 @@
* The C++ demangler has been updated for better C++11 support.
+* Error messages about fishy arguments (formerly known as silly arguments)
+ now include a back-trace to aid debugging.
+
* ==================== FIXED BUGS ====================
The following bugs have been fixed or resolved. Note that "n-i-bz"
@@ -1053,6 +1056,7 @@
https://bugs.kde.org/show_bug.cgi?id=XXXXXX
where XXXXXX is the bug number as listed below.
+ 79311 malloc silly arg warning does not give stack trace
210935 port valgrind.h (not valgrind) to win32 to support client requests
214223 valgrind SIGSEGV on startup gcc 4.4.1 ppc32 (G4) Ubuntu 9.10
243404 Port to zSeries
Modified: trunk/memcheck/docs/mc-manual.xml
==============================================================================
--- trunk/memcheck/docs/mc-manual.xml (original)
+++ trunk/memcheck/docs/mc-manual.xml Sun Jul 13 14:41:55 2014
@@ -45,6 +45,12 @@
</listitem>
<listitem>
+ <para>Passing a fishy (presumably negative) value to the
+ <computeroutput>size</computeroutput> parameter of a memory
+ allocation function.</para>
+ </listitem>
+
+ <listitem>
<para>Memory leaks.</para>
</listitem>
</itemizedlist>
@@ -378,6 +384,48 @@
</sect2>
+<sect2 id="mc-manual.fishyvalue"
+ xreflabel="Fishy argument values">
+<title>Fishy argument values</title>
+
+<para>All memory allocation functions take an argument specifying the
+size of the memory block that should be allocated. Clearly, the requested
+size should be a non-negative value and is typically not excessively large.
+For instance, it is extremely unlikly that the size of an allocation
+request exceeds 2**63 bytes on a 64-bit machine. It is much more likely that
+such a value is the result of an erroneous size calculation and is in effect
+a negative value (that just happens to appear excessively large because
+the bit pattern is interpreted as an unsigned integer).
+Such a value is called a "fishy value".
+
+The <varname>size</varname> argument of the following allocation functions
+is checked for being fishy:
+<function>malloc</function>,
+<function>calloc</function>,
+<function>realloc</function>,
+<function>memalign</function>,
+<function>new</function>,
+<function>new []</function>.
+<function>__builtin_new</function>,
+<function>__builtin_vec_new</function>,
+For <function>calloc</function> both arguments are being checked.
+</para>
+
+<para>For example:</para>
+<programlisting><![CDATA[
+==32233== Argument 'size' of function malloc has a fishy (possibly negative) value: -3
+==32233== at 0x4C2CFA7: malloc (vg_replace_malloc.c:298)
+==32233== by 0x400555: foo (fishy.c:15)
+==32233== by 0x400583: main (fishy.c:23)
+]]></programlisting>
+
+<para>In earlier Valgrind versions those values were being referred to
+as "silly arguments" and no back-trace was included.
+</para>
+
+</sect2>
+
+
<sect2 id="mc-manual.leaks" xreflabel="Memory leak detection">
<title>Memory leak detection</title>
Modified: trunk/memcheck/mc_errors.c
==============================================================================
--- trunk/memcheck/mc_errors.c (original)
+++ trunk/memcheck/mc_errors.c Sun Jul 13 14:41:55 2014
@@ -75,6 +75,7 @@
Err_Overlap,
Err_Leak,
Err_IllegalMempool,
+ Err_FishyValue,
}
MC_ErrorTag;
@@ -178,6 +179,15 @@
AddrInfo ai;
} IllegalMempool;
+ // A fishy function argument value
+ // An argument value is considered fishy if the corresponding
+ // parameter has SizeT type and the value when interpreted as a
+ // signed number is negative.
+ struct {
+ const HChar *function_name;
+ const HChar *argument_name;
+ SizeT value;
+ } FishyValue;
} Err;
};
@@ -655,6 +665,27 @@
break;
}
+ case Err_FishyValue:
+ if (xml) {
+ emit( " <kind>FishyValue</kind>\n" );
+ emit( " <what>");
+ emit( "Argument '%s' of function %s has a fishy "
+ "(possibly negative) value: %ld\n",
+ extra->Err.FishyValue.argument_name,
+ extra->Err.FishyValue.function_name,
+ (SSizeT)extra->Err.FishyValue.value);
+ emit( "</what>");
+ VG_(pp_ExeContext)( VG_(get_error_where)(err) );
+ } else {
+ emit( "Argument '%s' of function %s has a fishy "
+ "(possibly negative) value: %ld\n",
+ extra->Err.FishyValue.argument_name,
+ extra->Err.FishyValue.function_name,
+ (SSizeT)extra->Err.FishyValue.value);
+ VG_(pp_ExeContext)( VG_(get_error_where)(err) );
+ }
+ break;
+
default:
VG_(printf)("Error:\n unknown Memcheck error code %d\n",
VG_(get_error_kind)(err));
@@ -835,6 +866,25 @@
/*allow_GDB_attach*/False, count_error );
}
+Bool MC_(record_fishy_value_error) ( ThreadId tid, const HChar *function_name,
+ const HChar *argument_name, SizeT value)
+{
+ MC_Error extra;
+
+ tl_assert(VG_INVALID_THREADID != tid);
+
+ if ((SSizeT)value >= 0) return False; // not a fishy value
+
+ extra.Err.FishyValue.function_name = function_name;
+ extra.Err.FishyValue.argument_name = argument_name;
+ extra.Err.FishyValue.value = value;
+
+ VG_(maybe_record_error)(
+ tid, Err_FishyValue, /*addr*/0, /*s*/NULL, &extra );
+
+ return True;
+}
+
void MC_(record_user_error) ( ThreadId tid, Addr a,
Bool isAddrErr, UInt otag )
{
@@ -904,6 +954,12 @@
case Err_Cond:
return True;
+ case Err_FishyValue:
+ return VG_STREQ(extra1->Err.FishyValue.function_name,
+ extra2->Err.FishyValue.function_name) &&
+ VG_STREQ(extra1->Err.FishyValue.argument_name,
+ extra2->Err.FishyValue.argument_name);
+
case Err_Addr:
return ( extra1->Err.Addr.szB == extra2->Err.Addr.szB
? True : False );
@@ -1033,6 +1089,7 @@
//case Err_Value:
//case Err_Cond:
case Err_Overlap:
+ case Err_FishyValue:
// For Err_Leaks the returned size does not matter -- they are always
// shown with VG_(unique_error)() so they 'extra' not copied. But
// we make it consistent with the others.
@@ -1186,6 +1243,7 @@
OverlapSupp, // Overlapping blocks in memcpy(), strcpy(), etc
LeakSupp, // Something to be suppressed in a leak check.
MempoolSupp, // Memory pool suppression.
+ FishyValueSupp,// Fishy value suppression.
}
MC_SuppKind;
@@ -1213,6 +1271,7 @@
else if (VG_STREQ(name, "Value4")) skind = Value4Supp;
else if (VG_STREQ(name, "Value8")) skind = Value8Supp;
else if (VG_STREQ(name, "Value16")) skind = Value16Supp;
+ else if (VG_STREQ(name, "FishyValue")) skind = FishyValueSupp;
else
return False;
@@ -1234,6 +1293,11 @@
UInt leak_search_gen;
};
+typedef struct {
+ const HChar *function_name;
+ const HChar *argument_name;
+} MC_FishyValueExtra;
+
Bool MC_(read_extra_suppression_info) ( Int fd, HChar** bufpp,
SizeT* nBufp, Int* lineno, Supp *su )
{
@@ -1265,6 +1329,26 @@
} else {
return False; // unknown extra line.
}
+ } else if (VG_(get_supp_kind)(su) == FishyValueSupp) {
+ MC_FishyValueExtra *extra;
+ HChar *p;
+
+ eof = VG_(get_line) ( fd, bufpp, nBufp, lineno );
+ if (eof) return True;
+
+ extra = VG_(malloc)("mc.resi.3", sizeof *extra);
+ extra->function_name = VG_(strdup)("mv.resi.4", *bufpp);
+
+ // The suppression string is: function_name(argument_name)
+ p = VG_(strchr)(extra->function_name, '(');
+ if (p == NULL) return False; // malformed suppression string
+ *p++ = '\0';
+ extra->argument_name = p;
+ p = VG_(strchr)(p, ')');
+ if (p == NULL) return False; // malformed suppression string
+ *p = '\0';
+
+ VG_(set_supp_extra)(su, extra);
}
return True;
}
@@ -1334,6 +1418,16 @@
case MempoolSupp:
return (ekind == Err_IllegalMempool);
+ case FishyValueSupp: {
+ MC_FishyValueExtra *supp_extra = VG_(get_supp_extra)(su);
+
+ return (ekind == Err_FishyValue) &&
+ VG_STREQ(extra->Err.FishyValue.function_name,
+ supp_extra->function_name) &&
+ VG_STREQ(extra->Err.FishyValue.argument_name,
+ supp_extra->argument_name);
+ }
+
default:
VG_(printf)("Error:\n"
" unknown suppression type %d\n",
@@ -1357,6 +1451,7 @@
case Err_Overlap: return "Overlap";
case Err_Leak: return "Leak";
case Err_Cond: return "Cond";
+ case Err_FishyValue: return "FishyValue";
case Err_Addr: {
MC_Error* extra = VG_(get_error_extra)(err);
switch ( extra->Err.Addr.szB ) {
@@ -1400,6 +1495,12 @@
(buf, nBuf-1, "match-leak-kinds: %s",
pp_Reachedness_for_leak_kinds(extra->Err.Leak.lr->key.state));
return True;
+ } else if (Err_FishyValue == ekind) {
+ MC_Error* extra = VG_(get_error_extra)(err);
+ VG_(snprintf)
+ (buf, nBuf-1, "%s(%s)", extra->Err.FishyValue.function_name,
+ extra->Err.FishyValue.argument_name);
+ return True;
} else {
return False;
}
Modified: trunk/memcheck/mc_include.h
==============================================================================
--- trunk/memcheck/mc_include.h (original)
+++ trunk/memcheck/mc_include.h Sun Jul 13 14:41:55 2014
@@ -441,6 +441,9 @@
Bool print_record,
Bool count_error );
+Bool MC_(record_fishy_value_error) ( ThreadId tid, const HChar* function,
+ const HChar *argument_name, SizeT value );
+
/* Parses a set of leak kinds (separated by ,).
and give the resulting set in *lks.
If parsing is succesful, returns True and *lks contains the resulting set.
Modified: trunk/memcheck/mc_malloc_wrappers.c
==============================================================================
--- trunk/memcheck/mc_malloc_wrappers.c (original)
+++ trunk/memcheck/mc_malloc_wrappers.c Sun Jul 13 14:41:55 2014
@@ -335,33 +335,6 @@
/*--- client_malloc(), etc ---*/
/*------------------------------------------------------------*/
-// XXX: should make this a proper error (bug #79311).
-static Bool complain_about_silly_args(SizeT sizeB, const HChar* fn)
-{
- // Cast to a signed type to catch any unexpectedly negative args. We're
- // assuming here that the size asked for is not greater than 2^31 bytes
- // (for 32-bit platforms) or 2^63 bytes (for 64-bit platforms).
- if ((SSizeT)sizeB < 0) {
- if (!VG_(clo_xml))
- VG_(message)(Vg_UserMsg, "Warning: silly arg (%ld) to %s()\n",
- (SSizeT)sizeB, fn );
- return True;
- }
- return False;
-}
-
-static Bool complain_about_silly_args2(SizeT n, SizeT sizeB)
-{
- if ((SSizeT)n < 0 || (SSizeT)sizeB < 0) {
- if (!VG_(clo_xml))
- VG_(message)(Vg_UserMsg,
- "Warning: silly args (%ld,%ld) to calloc()\n",
- (SSizeT)n, (SSizeT)sizeB);
- return True;
- }
- return False;
-}
-
/* Allocate memory and note change in memory available */
void* MC_(new_block) ( ThreadId tid,
Addr p, SizeT szB, SizeT alignB,
@@ -406,7 +379,7 @@
void* MC_(malloc) ( ThreadId tid, SizeT n )
{
- if (complain_about_silly_args(n, "malloc")) {
+ if (MC_(record_fishy_value_error)(tid, "malloc", "size", n)) {
return NULL;
} else {
return MC_(new_block) ( tid, 0, n, VG_(clo_alignment),
@@ -416,7 +389,7 @@
void* MC_(__builtin_new) ( ThreadId tid, SizeT n )
{
- if (complain_about_silly_args(n, "__builtin_new")) {
+ if (MC_(record_fishy_value_error)(tid, "__builtin_new", "size", n)) {
return NULL;
} else {
return MC_(new_block) ( tid, 0, n, VG_(clo_alignment),
@@ -426,7 +399,7 @@
void* MC_(__builtin_vec_new) ( ThreadId tid, SizeT n )
{
- if (complain_about_silly_args(n, "__builtin_vec_new")) {
+ if (MC_(record_fishy_value_error)(tid, "__builtin_vec_new", "size", n)) {
return NULL;
} else {
return MC_(new_block) ( tid, 0, n, VG_(clo_alignment),
@@ -436,7 +409,7 @@
void* MC_(memalign) ( ThreadId tid, SizeT alignB, SizeT n )
{
- if (complain_about_silly_args(n, "memalign")) {
+ if (MC_(record_fishy_value_error)(tid, "memalign", "size", n)) {
return NULL;
} else {
return MC_(new_block) ( tid, 0, n, alignB,
@@ -446,7 +419,8 @@
void* MC_(calloc) ( ThreadId tid, SizeT nmemb, SizeT size1 )
{
- if (complain_about_silly_args2(nmemb, size1)) {
+ if (MC_(record_fishy_value_error)(tid, "calloc", "nmemb", nmemb) ||
+ MC_(record_fishy_value_error)(tid, "calloc", "size", size1)) {
return NULL;
} else {
return MC_(new_block) ( tid, 0, nmemb*size1, VG_(clo_alignment),
@@ -538,7 +512,7 @@
Addr a_new;
SizeT old_szB;
- if (complain_about_silly_args(new_szB, "realloc"))
+ if (MC_(record_fishy_value_error)(tid, "realloc", "size", new_szB))
return NULL;
cmalloc_n_frees ++;
Modified: trunk/memcheck/tests/accounting.stderr.exp
==============================================================================
--- trunk/memcheck/tests/accounting.stderr.exp (original)
+++ trunk/memcheck/tests/accounting.stderr.exp Sun Jul 13 14:41:55 2014
@@ -1,5 +1,8 @@
-Warning: silly arg (-1) to realloc()
+Argument 'size' of function realloc has a fishy (possibly negative) value: -1
+ at 0x........: realloc (vg_replace_malloc.c:...)
+ by 0x........: main (accounting.c:17)
+
HEAP SUMMARY:
in use at exit: 0 bytes in 0 blocks
@@ -8,4 +11,4 @@
For a detailed leak analysis, rerun with: --leak-check=full
For counts of detected and suppressed errors, rerun with: -v
-ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
+ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Modified: trunk/memcheck/tests/malloc3.stderr.exp
==============================================================================
--- trunk/memcheck/tests/malloc3.stderr.exp (original)
+++ trunk/memcheck/tests/malloc3.stderr.exp Sun Jul 13 14:41:55 2014
@@ -1,2 +1,8 @@
-Warning: silly arg (-1) to malloc()
-Warning: silly args (0,-1) to calloc()
+Argument 'size' of function malloc has a fishy (possibly negative) value: -1
+ at 0x........: malloc (vg_replace_malloc.c:...)
+ by 0x........: main (malloc3.c:15)
+
+Argument 'size' of function calloc has a fishy (possibly negative) value: -1
+ at 0x........: calloc (vg_replace_malloc.c:...)
+ by 0x........: main (malloc3.c:23)
+
|
