|
From: Konstantin S. <kon...@gm...> - 2009-02-09 13:52:03
|
Hi Julian, all, I have few questions on valgrind suppression mechanism. First: I've hit a performance issue in VG_(maybe_record_error). I was running a test that uses openssl library. This library has a hand-written assembly file which is not friendly to valgrind (valgrind produces broken stack traces, see the recent discussion at http://sourceforge.net/mailarchive/forum.php?thread_name=59e975c10902052331r2bf43900qec86f336cba6e531%40mail.gmail.com&forum_name=valgrind-developers). This assembly file results in memcheck warnings, like this: ==27019== Use of uninitialised value of size 8 ==27019== at 0xBD3214F: bn_mul_mont ==27019== by 0x7A2C730C750BD1E8: ??? ==27019== by 0xD075D9E9E5D0101: ??? ==27019== by 0x4EAF8BD8815341D7: ??? Since the stack traces are broken, each warning found by memcheck is distinct. Memcheck boils out after 1000-th report. So, I needed to suppress these reports. { fun:bn_mul_mont Memcheck:Value8 fun:bn_mul_mont } As the result, memcheck is slowed down by >100x (from 1 minute to few hours). Since all the errors found by memcheck are different, VG_(maybe_record_error) tries to add each of them to the list static Error* errors = NULL; Since all these errors are suppressed, memcheck does not boil out after 1000-th and goes quadratic. The simple fix which seems to work for me is to check is_suppressible_error before checking the 'errors' list. *** 556,561 **** --- 556,569 ---- em_errlist_searches++; p = errors; p_prev = NULL; + + Supp *supp = is_suppressible_error(&err); + if (supp) { + n_errs_suppressed++; + supp->count++; + return ERROR_IS_SUPPRESSED; + } + while (p != NULL) { em_errlist_cmps++; if (eq_Error(exe_res, p, &err)) { Is there any hidden reason not to do that? Second: Right now I often have to write two almost identical suppression rules, like this: { fun:bn_mul_mont Memcheck:Value8 <<<<<<<<<<<<<<<< fun:bn_mul_mont } { fun:bn_mul_mont Memcheck:Cond <<<<<<<<<<<<<< fun:bn_mul_mont } Is there any way to tell something like Memcheck:* or Memcheck:All? { fun:bn_mul_mont Memcheck:All <<<<<<<<<<<<<< fun:bn_mul_mont } If no, would you consider a patch that does it? Third: In case when a user needs to suppress warnings from a library he doesn't care about, the better solution for memcheck would be simply not to generate such warnings. ThreadSanitizer does this by not instrumenting functions listed in a text file (http://code.google.com/p/data-race-test/wiki/ThreadSanitizerIgnores). Would you consider adding something like this to the valgrind core? Thanks, --kcc |
|
From: Nicholas N. <n.n...@gm...> - 2009-02-24 23:29:24
|
On Tue, Feb 10, 2009 at 12:51 AM, Konstantin Serebryany <kon...@gm...> wrote: > > First: I've hit a performance issue in VG_(maybe_record_error). > I was running a test that uses openssl library. > This library has a hand-written assembly file which is not friendly to > valgrind (valgrind produces broken stack traces, see the recent > discussion at http://sourceforge.net/mailarchive/forum.php?thread_name=59e975c10902052331r2bf43900qec86f336cba6e531%40mail.gmail.com&forum_name=valgrind-developers). > This assembly file results in memcheck warnings, like this: > ==27019== Use of uninitialised value of size 8 > ==27019== at 0xBD3214F: bn_mul_mont > ==27019== by 0x7A2C730C750BD1E8: ??? > ==27019== by 0xD075D9E9E5D0101: ??? > ==27019== by 0x4EAF8BD8815341D7: ??? > Since the stack traces are broken, each warning found by memcheck is distinct. > Memcheck boils out after 1000-th report. > > So, I needed to suppress these reports. > { > fun:bn_mul_mont > Memcheck:Value8 > fun:bn_mul_mont > } > > As the result, memcheck is slowed down by >100x (from 1 minute to few hours). > Since all the errors found by memcheck are different, > VG_(maybe_record_error) tries to add each of them to the list > static Error* errors = NULL; > Since all these errors are suppressed, memcheck does not boil out > after 1000-th and goes quadratic. > > The simple fix which seems to work for me is to check > is_suppressible_error before checking the 'errors' list. > [...] > Is there any hidden reason not to do that? Not that I'm aware of. I'll take a look at it some time. > Second: > Right now I often have to write two almost identical suppression > rules, like this: > > { > fun:bn_mul_mont > Memcheck:Value8 <<<<<<<<<<<<<<<< > fun:bn_mul_mont > } > { > fun:bn_mul_mont > Memcheck:Cond <<<<<<<<<<<<<< > fun:bn_mul_mont > } > > Is there any way to tell something like Memcheck:* or Memcheck:All? > { > fun:bn_mul_mont > Memcheck:All <<<<<<<<<<<<<< > fun:bn_mul_mont > } > If no, would you consider a patch that does it? There is no way to do that. AFAIK you're the first person who's asked for it, so it doesn't seem like a high priority... > Third: > In case when a user needs to suppress warnings from a library he > doesn't care about, the better solution for memcheck would be simply > not to generate such warnings. > ThreadSanitizer does this by not instrumenting functions listed in a > text file (http://code.google.com/p/data-race-test/wiki/ThreadSanitizerIgnores). > Would you consider adding something like this to the valgrind core? That's only useful for suppressions where you only care about the first stack entry... that's only a small fraction of suppressions. Nick |
|
From: Nicholas N. <n.n...@gm...> - 2009-02-25 06:40:57
|
On Tue, Feb 10, 2009 at 12:51 AM, Konstantin Serebryany <kon...@gm...> wrote: > > First: I've hit a performance issue in VG_(maybe_record_error). > I was running a test that uses openssl library. > This library has a hand-written assembly file which is not friendly to > valgrind (valgrind produces broken stack traces, see the recent > discussion at http://sourceforge.net/mailarchive/forum.php?thread_name=59e975c10902052331r2bf43900qec86f336cba6e531%40mail.gmail.com&forum_name=valgrind-developers). > This assembly file results in memcheck warnings, like this: > ==27019== Use of uninitialised value of size 8 > ==27019== at 0xBD3214F: bn_mul_mont > ==27019== by 0x7A2C730C750BD1E8: ??? > ==27019== by 0xD075D9E9E5D0101: ??? > ==27019== by 0x4EAF8BD8815341D7: ??? > Since the stack traces are broken, each warning found by memcheck is distinct. > Memcheck boils out after 1000-th report. > > So, I needed to suppress these reports. > { > fun:bn_mul_mont > Memcheck:Value8 > fun:bn_mul_mont > } > > As the result, memcheck is slowed down by >100x (from 1 minute to few hours). > Since all the errors found by memcheck are different, > VG_(maybe_record_error) tries to add each of them to the list > static Error* errors = NULL; > Since all these errors are suppressed, memcheck does not boil out > after 1000-th and goes quadratic. > > The simple fix which seems to work for me is to check > is_suppressible_error before checking the 'errors' list. > > *** 556,561 **** > --- 556,569 ---- > em_errlist_searches++; > p = errors; > p_prev = NULL; > + > + Supp *supp = is_suppressible_error(&err); > + if (supp) { > + n_errs_suppressed++; > + supp->count++; > + return ERROR_IS_SUPPRESSED; > + } > + > while (p != NULL) { > em_errlist_cmps++; > if (eq_Error(exe_res, p, &err)) { > > Is there any hidden reason not to do that? This means that is_suppressible_error occurs before the tool has had a chance to update the 'extra' field. In some cases Memcheck uses the 'extra' field to decide whether an error matches a suppression. With this change the 'extra' field won't have been initialised, which I think may cause crashes or similar behaviour. Also, for those suppressions that don't cause a crash, no record will be made of the suppression, so the list of suppression records printed at the end with -v will be incomplete. I can't see a good way to preserve existing behaviour while also fixing your problem. I wonder if including suppressed errors in the don't-report-errors-after-too-many-have-been-found count is the best approach. Nick |