Bug 266386 - --xml=yes silently cancels --leak-check=no
Summary: --xml=yes silently cancels --leak-check=no
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (other bugs)
Version First Reported In: 3.7 SVN
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-15 17:13 UTC by Timur Iskhodzhanov
Modified: 2020-03-24 21:48 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Timur Iskhodzhanov 2011-02-15 17:13:18 UTC
I've been playing with Valgrind for Chromium and found out that despite --leak-check=no it does its leak-checking if --xml=yes is set.

So, a simple repro is:
$ valgrind --leak-check=no --xml=yes --xml-file=xml.log ./leak && cat xml.log
where valgrind = r11560 built from sources
leak = "int main() { void * ptr = malloc(42); }"

In the Memcheck sources, there is the following code:

5596 static void mc_post_clo_init ( void )
5597 {
5598    /* If we've been asked to emit XML, mash around various other
5599       options so as to constrain the output somewhat. */
5600    if (VG_(clo_xml)) {
5601       /* Extract as much info as possible from the leak checker. */
5602       /* MC_(clo_show_reachable) = True; */
5603       MC_(clo_leak_check) = LC_Full;
5604    }
(svn annotate shows njn at these lines)

What's the background behind this alteration?
This is behaviour is clearly counter-intuitive.

I see two options:
1) Remove this if(){} statement  // best for me
2) If (1) is not possible, abort if --leak-check!=full when --xml=yes.
Comment 1 Julian Seward 2011-02-15 17:33:15 UTC
The rationale is as stated at line 5598/9.  Why do you care?
Comment 2 Timur Iskhodzhanov 2011-02-15 17:44:51 UTC
> "If we've been asked to emit XML, mash around various other
> options so as to constrain the output somewhat."
You mean this one?

That doesn't make any sense for me to justify "provide no way to disable leak checking".
At least, typing
$ valgrind --leak-check=no <any other options> ./myprog
and seeing leaks is not what user expects, right?

Sometimes we want to run Valgrind without leak-checking (e.g. doing security-related testing - leaks are not exploitable, right?).
Indeed, we can post-process the XMLs to strip the leaks
BUT
I'm also assuming --leak-check=no may be faster.
I expect it to be much faster on exit when running a huge app like Chromium
(see http://crbug.com/17453 esp. the comment #15 - we have to wait a couple of minutes for the leak-check...)

Do you expect any issues if we just remove this if() statement?
Comment 3 Julian Seward 2011-02-15 17:56:47 UTC
(In reply to comment #2)
> > "If we've been asked to emit XML, mash around various other
> > options so as to constrain the output somewhat."
> You mean this one?

Yes.

> That doesn't make any sense for me to justify "provide no way to disable leak
> checking".
> At least, typing
> $ valgrind --leak-check=no <any other options> ./myprog
> and seeing leaks is not what user expects, right?

The XML output is intended to be read by a GUI, not a human.

> I'm also assuming --leak-check=no may be faster.

This is a fair point, yes.  Basic problem is we already have enough
problems verifying the large number of different Valgrind
configuration/use scenarios at each release.  The real reason to
restrict the XML output is to reduce our verification overheads.

> Do you expect any issues if we just remove this if() statement?

Probably is OK, but there's only one way to find out for sure.

FTR, if leak checking is taking 2 cpu minutes, you should file a
bug report, including of course a simple test case.
Comment 4 Timur Iskhodzhanov 2011-02-15 18:18:12 UTC
> The XML output is intended to be read by a GUI, not a human.
Right, but after all it's a human running the command line. (*)

> Basic problem is we already have enough
> problems verifying the large number of different Valgrind
> configuration/use scenarios at each release.
The default option is --leak-check=full anyways, so removing this if() won't affect anyone who does not agree with (*)
Okay, I'll just remove it from valgrind-variant and give it a try on the Chromium bots.

> The real reason to restrict the XML output is to reduce our verification overheads.
At the cost of runtime overhead :)

> if leak checking is taking 2 cpu minutes, you should file a
> bug report, including of course a simple test case
I think the main problem is just that Chromium is too large :(
So I doubt a "simple" test case is possible.
To reproduce with Chromium see:
https://bugs.kde.org/show_bug.cgi?id=266390
Prepare for a couple of hours of checkout&build but it's worth it - Chromium is a great test and benchmark for Valgrind :)
Comment 5 Andrew Marlow 2020-02-18 08:15:40 UTC
I have just come across this error and was wondering when/if it is going to be fixed please. I am using the -xml option when running valgrind in a jenkins job. Originally I wasn't using it (didn't know about it) and I found that when valgrind reports an error it is a pain to go through the jenkins console log looking for when valgrind reports a problem. It's much easier to find issues when the output is XML.

At the moment, due to this bug, I have to run without the --xml option then if valgrind errors (using -error-exit=1) I re-run with the --xml option.
Comment 6 Timur Iskhodzhanov 2020-03-24 21:48:57 UTC
You might be able to find the code archive of "valgrind-variant" (used to be hosted on Google Code) that we ended up creating and using for Chromium.

> At the moment, due to this bug, I have to run without the --xml option
> then if valgrind errors (using -error-exit=1) I re-run with the --xml option.

I would recommend against rerunning Valgrind like that, since some reports are flaky, i.e. they can happen the first time (return exit code 1) and not happen the second time (when you rerun with --xml).