From: Nicholas Nethercote Date: Fri, 24 Jul 2009 07:38:29 +0000 (+0000) Subject: A fix for bug 186796: suppression symbol names were being truncated if they X-Git-Tag: svn/VALGRIND_3_5_0~267 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=68efb295036342df96de0b82927009623a87b32f;p=thirdparty%2Fvalgrind.git A fix for bug 186796: suppression symbol names were being truncated if they were longer than 200 chars. Now dynamic memory is used and so they can be arbitrarily long in theory, although in practice it bombs out at 100,000 for sanity purposes. This required adjusting the core/tool interface for read_extra_suppression_info(). git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10581 --- diff --git a/coregrind/m_errormgr.c b/coregrind/m_errormgr.c index 64ae47aa93..cf0b8dff14 100644 --- a/coregrind/m_errormgr.c +++ b/coregrind/m_errormgr.c @@ -964,14 +964,12 @@ static Int get_char ( Int fd, Char* out_buf ) return 1; } - -/* Get a non-blank, non-comment line of at most nBuf chars from fd. - Skips leading spaces on the line. Return True if EOF was hit instead. -*/ -Bool VG_(get_line) ( Int fd, Char* buf, Int nBuf ) +Bool VG_(get_line) ( Int fd, Char** bufpp, SizeT* nBufp ) { - Char ch; - Int n, i; + Char* buf = *bufpp; + SizeT nBuf = *nBufp; + Char ch; + Int n, i; while (True) { /* First, read until a non-blank char appears. */ while (True) { @@ -987,7 +985,14 @@ Bool VG_(get_line) ( Int fd, Char* buf, Int nBuf ) n = get_char(fd, &ch); if (n <= 0) return False; /* the next call will return True */ if (ch == '\n') break; - if (i > 0 && i == nBuf-1) i--; + if (i > 0 && i == nBuf-1) { + *nBufp = nBuf = nBuf * 2; + #define RIDICULOUS 100000 + vg_assert2(nBuf < RIDICULOUS, // Just a sanity check, really. + "VG_(get_line): line longer than %d chars, aborting\n", + RIDICULOUS); + *bufpp = buf = VG_(realloc)("errormgr.get_line.1", buf, nBuf); + } buf[i++] = ch; buf[i] = 0; } while (i > 1 && VG_(isspace)(buf[i-1])) { @@ -1053,11 +1058,11 @@ static Bool tool_name_present(Char *name, Char *names) */ static void load_one_suppressions_file ( Char* filename ) { -# define N_BUF 200 SysRes sres; Int fd, i, j, lineno = 0; Bool eof; - Char buf[N_BUF+1]; + SizeT nBuf = 200; + Char* buf = VG_(malloc)("errormgr.losf.1", nBuf); Char* tool_names; Char* supp_name; Char* err_str = NULL; @@ -1098,20 +1103,20 @@ static void load_one_suppressions_file ( Char* filename ) supp->string = supp->extra = NULL; - eof = VG_(get_line) ( fd, buf, N_BUF ); + eof = VG_(get_line) ( fd, &buf, &nBuf ); lineno++; if (eof) break; if (!VG_STREQ(buf, "{")) BOMB("expected '{' or end-of-file"); - eof = VG_(get_line) ( fd, buf, N_BUF ); + eof = VG_(get_line) ( fd, &buf, &nBuf ); lineno++; if (eof || VG_STREQ(buf, "}")) BOMB("unexpected '}'"); supp->sname = VG_(arena_strdup)(VG_AR_CORE, "errormgr.losf.2", buf); - eof = VG_(get_line) ( fd, buf, N_BUF ); + eof = VG_(get_line) ( fd, &buf, &nBuf ); lineno++; if (eof) BOMB("unexpected end-of-file"); @@ -1150,7 +1155,7 @@ static void load_one_suppressions_file ( Char* filename ) else { // Ignore rest of suppression while (True) { - eof = VG_(get_line) ( fd, buf, N_BUF ); + eof = VG_(get_line) ( fd, &buf, &nBuf ); lineno++; if (eof) BOMB("unexpected end-of-file"); if (VG_STREQ(buf, "}")) @@ -1161,7 +1166,7 @@ static void load_one_suppressions_file ( Char* filename ) if (VG_(needs).tool_errors && !VG_TDICT_CALL(tool_read_extra_suppression_info, - fd, buf, N_BUF, supp)) + fd, &buf, &nBuf, supp)) { BOMB("bad or missing extra suppression info"); } @@ -1169,7 +1174,7 @@ static void load_one_suppressions_file ( Char* filename ) /* the main frame-descriptor reading loop */ i = 0; while (True) { - eof = VG_(get_line) ( fd, buf, N_BUF ); + eof = VG_(get_line) ( fd, &buf, &nBuf ); lineno++; if (eof) BOMB("unexpected end-of-file"); @@ -1196,7 +1201,7 @@ static void load_one_suppressions_file ( Char* filename ) // lines and grab the '}'. if (!VG_STREQ(buf, "}")) { do { - eof = VG_(get_line) ( fd, buf, N_BUF ); + eof = VG_(get_line) ( fd, &buf, &nBuf ); lineno++; } while (!eof && !VG_STREQ(buf, "}")); } @@ -1227,6 +1232,7 @@ static void load_one_suppressions_file ( Char* filename ) supp->next = suppressions; suppressions = supp; } + VG_(free)(buf); VG_(close)(fd); return; @@ -1242,7 +1248,6 @@ static void load_one_suppressions_file ( Char* filename ) VG_(exit)(1); # undef BOMB -# undef N_BUF } diff --git a/coregrind/m_tooliface.c b/coregrind/m_tooliface.c index e6d1652e71..6f76d59788 100644 --- a/coregrind/m_tooliface.c +++ b/coregrind/m_tooliface.c @@ -230,7 +230,7 @@ void VG_(needs_tool_errors)( Bool show_TIDs, UInt (*update) (Error*), Bool (*recog) (Char*, Supp*), - Bool (*read_extra) (Int, Char*, Int, Supp*), + Bool (*read_extra) (Int, Char**, SizeT*, Supp*), Bool (*matches) (Error*, Supp*), Char* (*name) (Error*), void (*print_extra)(Error*) diff --git a/coregrind/pub_core_tooliface.h b/coregrind/pub_core_tooliface.h index f9a8005405..9e928cb491 100644 --- a/coregrind/pub_core_tooliface.h +++ b/coregrind/pub_core_tooliface.h @@ -122,7 +122,7 @@ typedef struct { Bool tool_show_ThreadIDs_for_errors; UInt (*tool_update_extra) (Error*); Bool (*tool_recognised_suppression) (Char*, Supp*); - Bool (*tool_read_extra_suppression_info) (Int, Char*, Int, Supp*); + Bool (*tool_read_extra_suppression_info) (Int, Char**, SizeT*, Supp*); Bool (*tool_error_matches_suppression) (Error*, Supp*); Char* (*tool_get_error_name) (Error*); void (*tool_print_extra_suppression_info)(Error*); diff --git a/exp-ptrcheck/pc_common.c b/exp-ptrcheck/pc_common.c index 3d75748d3c..365ffe95d1 100644 --- a/exp-ptrcheck/pc_common.c +++ b/exp-ptrcheck/pc_common.c @@ -736,14 +736,14 @@ Bool pc_is_recognised_suppression ( Char* name, Supp *su ) return True; } -Bool pc_read_extra_suppression_info ( Int fd, Char* buf, - Int nBuf, Supp* su ) +Bool pc_read_extra_suppression_info ( Int fd, Char** bufpp, + SizeT* nBufp, Supp* su ) { Bool eof; if (VG_(get_supp_kind)(su) == XS_SysParam) { - eof = VG_(get_line) ( fd, buf, nBuf ); + eof = VG_(get_line) ( fd, bufpp, nBufp ); if (eof) return False; - VG_(set_supp_string)(su, VG_(strdup)("pc.common.presi.1", buf)); + VG_(set_supp_string)(su, VG_(strdup)("pc.common.presi.1", *bufpp)); } return True; } diff --git a/exp-ptrcheck/pc_common.h b/exp-ptrcheck/pc_common.h index 9c440d1672..1e2549af4c 100644 --- a/exp-ptrcheck/pc_common.h +++ b/exp-ptrcheck/pc_common.h @@ -52,8 +52,8 @@ void pc_before_pp_Error ( Error* err ); void pc_pp_Error ( Error* err ); UInt pc_update_Error_extra ( Error* err ); Bool pc_is_recognised_suppression ( Char* name, Supp *su ); -Bool pc_read_extra_suppression_info ( Int fd, Char* buf, - Int nBuf, Supp* su ); +Bool pc_read_extra_suppression_info ( Int fd, Char** bufpp, + SizeT* nBufp, Supp* su ); Bool pc_error_matches_suppression (Error* err, Supp* su); Char* pc_get_error_name ( Error* err ); void pc_print_extra_suppression_info ( Error* err ); diff --git a/helgrind/hg_errors.c b/helgrind/hg_errors.c index 1d1acc2244..8a7ebe1117 100644 --- a/helgrind/hg_errors.c +++ b/helgrind/hg_errors.c @@ -1006,7 +1006,7 @@ Bool HG_(recognised_suppression) ( Char* name, Supp *su ) # undef TRY } -Bool HG_(read_extra_suppression_info) ( Int fd, Char* buf, Int nBuf, +Bool HG_(read_extra_suppression_info) ( Int fd, Char** bufpp, SizeT* nBufp, Supp* su ) { /* do nothing -- no extra suppression info present. Return True to diff --git a/helgrind/hg_errors.h b/helgrind/hg_errors.h index 9bb6502c09..9760b92790 100644 --- a/helgrind/hg_errors.h +++ b/helgrind/hg_errors.h @@ -40,7 +40,7 @@ void HG_(before_pp_Error) ( Error* err ); void HG_(pp_Error) ( Error* err ); UInt HG_(update_extra) ( Error* err ); Bool HG_(recognised_suppression) ( Char* name, Supp *su ); -Bool HG_(read_extra_suppression_info) ( Int fd, Char* buf, Int nBuf, +Bool HG_(read_extra_suppression_info) ( Int fd, Char** bufpp, SizeT* nBufp, Supp* su ); Bool HG_(error_matches_suppression) ( Error* err, Supp* su ); Char* HG_(get_error_name) ( Error* err ); diff --git a/include/pub_tool_errormgr.h b/include/pub_tool_errormgr.h index 0d22c1bbad..8369cf1299 100644 --- a/include/pub_tool_errormgr.h +++ b/include/pub_tool_errormgr.h @@ -87,10 +87,13 @@ extern Bool VG_(unique_error) ( ThreadId tid, ErrorKind ekind, ExeContext* where, Bool print_error, Bool allow_GDB_attach, Bool count_error ); -/* Gets a non-blank, non-comment line of at most nBuf chars from fd. - Skips leading spaces on the line. Returns True if EOF was hit instead. - Useful for reading in extra tool-specific suppression lines. */ -extern Bool VG_(get_line) ( Int fd, Char* buf, Int nBuf ); +/* Gets a non-blank, non-comment line from fd. bufpp is a pointer to a + pointer to a buffer that must be allocated with VG_(malloc); nBufp is a + pointer to size_t holding its size; if the buffer is too small for the + line, it will be realloc'd until big enough (updating *bufpp and *nBufp in + the process). (It will bomb out if the size gets ridiculous). Skips + leading spaces on the line. Returns True if EOF was hit instead. */ +extern Bool VG_(get_line) ( Int fd, Char** bufpp, SizeT* nBufp ); /* ------------------------------------------------------------------ */ diff --git a/include/pub_tool_tooliface.h b/include/pub_tool_tooliface.h index bce2b227f8..1a36f158aa 100644 --- a/include/pub_tool_tooliface.h +++ b/include/pub_tool_tooliface.h @@ -316,8 +316,10 @@ extern void VG_(needs_tool_errors) ( // Read any extra info for this suppression kind. Most likely for filling // in the `extra' and `string' parts (with VG_(set_supp_{extra, string})()) // of a suppression if necessary. Should return False if a syntax error - // occurred, True otherwise. - Bool (*read_extra_suppression_info)(Int fd, Char* buf, Int nBuf, Supp* su), + // occurred, True otherwise. bufpp and nBufp are the same as for + // VG_(get_line). + Bool (*read_extra_suppression_info)(Int fd, Char** bufpp, SizeT* nBufp, + Supp* su), // This should just check the kinds match and maybe some stuff in the // `string' and `extra' field if appropriate (using VG_(get_supp_*)() to diff --git a/memcheck/mc_errors.c b/memcheck/mc_errors.c index 4ccaa608ee..4e88f79889 100644 --- a/memcheck/mc_errors.c +++ b/memcheck/mc_errors.c @@ -1384,15 +1384,15 @@ Bool MC_(is_recognised_suppression) ( Char* name, Supp* su ) return True; } -Bool MC_(read_extra_suppression_info) ( Int fd, Char* buf, - Int nBuf, Supp *su ) +Bool MC_(read_extra_suppression_info) ( Int fd, Char** bufpp, + SizeT* nBufp, Supp *su ) { Bool eof; if (VG_(get_supp_kind)(su) == ParamSupp) { - eof = VG_(get_line) ( fd, buf, nBuf ); + eof = VG_(get_line) ( fd, bufpp, nBufp ); if (eof) return False; - VG_(set_supp_string)(su, VG_(strdup)("mc.resi.1", buf)); + VG_(set_supp_string)(su, VG_(strdup)("mc.resi.1", *bufpp)); } return True; } diff --git a/memcheck/mc_include.h b/memcheck/mc_include.h index fc7abf2085..f81b379073 100644 --- a/memcheck/mc_include.h +++ b/memcheck/mc_include.h @@ -312,8 +312,8 @@ UInt MC_(update_Error_extra) ( Error* err ); Bool MC_(is_recognised_suppression) ( Char* name, Supp* su ); -Bool MC_(read_extra_suppression_info) ( Int fd, Char* buf, - Int nBuf, Supp *su ); +Bool MC_(read_extra_suppression_info) ( Int fd, Char** buf, + SizeT* nBuf, Supp *su ); Bool MC_(error_matches_suppression) ( Error* err, Supp* su ); diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index 09fdb2a1d8..3d16a9573d 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -83,6 +83,7 @@ EXTRA_DIST = \ linux-syscalls-2007 linux-syscalls-2007.stderr.exp \ long_namespace_xml.vgtest long_namespace_xml.stdout.exp \ long_namespace_xml.stderr.exp \ + long-supps.vgtest long-supps.stderr.exp long-supps.supp \ lsframe1.vgtest lsframe1.stdout.exp lsframe1.stderr.exp \ lsframe2.vgtest lsframe2.stdout.exp lsframe2.stderr.exp \ mallinfo.stderr.exp mallinfo.vgtest \ @@ -202,11 +203,15 @@ check_PROGRAMS = \ fprw fwrite inits inline \ leak-0 \ leak-cases \ - leak-cycle leak-pool leak-tree \ + leak-cycle \ + leak-pool \ + leak-tree \ linux-syslog-syscall \ linux-syscalls-2007 \ long_namespace_xml \ - lsframe1 lsframe2 \ + long-supps \ + lsframe1 \ + lsframe2 \ mallinfo \ malloc_free_fill \ malloc_usable malloc1 malloc2 malloc3 manuel1 manuel2 manuel3 \ diff --git a/memcheck/tests/long-supps.c b/memcheck/tests/long-supps.c new file mode 100644 index 0000000000..78377e38db --- /dev/null +++ b/memcheck/tests/long-supps.c @@ -0,0 +1,20 @@ +// Bug 186796: function names of over 200 chars in suppressions were being +// truncated and so not matching. This 200 char limit is easily overcome with +// C++ templates. It now is assigned dynamically. + +#include + +#define F1000 \ +f1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890 + +void F1000(void) +{ + int* x = malloc(sizeof(int)); + x[1] = 1; +} + +int main(void) +{ + F1000(); + return 0; +} diff --git a/memcheck/tests/long-supps.stderr.exp b/memcheck/tests/long-supps.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/long-supps.supp b/memcheck/tests/long-supps.supp new file mode 100644 index 0000000000..3eabaf6351 --- /dev/null +++ b/memcheck/tests/long-supps.supp @@ -0,0 +1,7 @@ +{ + + Memcheck:Addr4 + fun:f1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890 + fun:main +} + diff --git a/memcheck/tests/long-supps.vgtest b/memcheck/tests/long-supps.vgtest new file mode 100644 index 0000000000..8750ac7c93 --- /dev/null +++ b/memcheck/tests/long-supps.vgtest @@ -0,0 +1,2 @@ +prog: long-supps +vgopts: --suppressions=long-supps.supp -q