LEAK suppressions vs. POSSIBLE LEAK suppressions

[derekbruening]

From timurrrr@google.com on January 19, 2011 12:45:51

Derek,
can you please explain the background of the decision to use two types of suppressions for leaks?
I think this is a bit too strict.
I think in some cases it requires the user to have two different suppressions for related leaks with similar stacktraces
(though I'm not sure I can imagine an example now)

Memcheck has only "Memcheck:Leak" suppression type for leaks which matches both definitely lost memory and possibly lost.

Original issue: http://code.google.com/p/drmemory/issues/detail?id=273

[derekbruening]

From bruen...@google.com on January 19, 2011 10:27:24

If an app has a set of heap objects allocated at one particular call site for which it only stores a pointer to the middle (for whatever purpose, and that doesn't match Dr. Memory's standard possible leak heuristics), it would want to suppress any possible leaks reported for that call site. However, the app wants to know when it has a real leak for that call site, so it does NOT want to suppress definitely lost leaks.

If you find yourself often suppressing both maybe a new type of suppression should be added that matches both: but it seems that specifying just possible leaks is valuable, and I would imagine is the most common type of leak suppression. What kind of scenario are you thinking of that produces both possible and definite leaks from the same call site where you want to suppress both types? In general when you suppress definite leaks, are you suppressing b/c you're unable or unwilling to fix but you acknowledge they're leaks, or b/c they're false positives?

[derekbruening]

From timurrrr@google.com on January 21, 2011 05:04:47

In general when you suppress definite leaks, are you suppressing b/c you're unable or unwilling to fix but you acknowledge they're leaks, or b/c they're false positives?
In general a person who writes suppression and code author are different persons.

In Chromium, the following scenario is usual:
a) a developer does a commit which introduces a new report (e.g. leak report)
b) a bot on the waterfall goes red
c) memory waterfall sheriff notices that, files a bug, notifies the dev and commits a suppression so the bot goes green on the next run
d) the developer fixes the bug and removes the suppression in a single commit

It's very often the case when the memory sheriff is not familiar at all with the code with errors so he can't fix it as soon as the dev can.

What kind of scenario are you thinking of that produces both possible and definite leaks from the same call site where you want to suppress both types?
For example, http://src.chromium.org/viewvc/chrome/trunk/src/tools/valgrind/drmemory/suppressions.txt?r1=72032&r2=72124 for http://build.chromium.org/p/chromium.fyi/builders/Windows%20Tests%20(DrMemory)/builds/1570/steps/memory%20test:%20printing/logs/stdio POSSIBLE LEAK 32 direct bytes 0x00163318-0x00163338 + 0 indirect bytes

1 0x7c809a7f LocalAlloc KERNEL32.dll

...
#11 0x77f1be50 CreateDCW GDI32.dll
#12 0x005d0d93 printing::PrintingContextWin::AllocateContext printing\printing_context_win.cc:465
#13 0x005d0bce printing::PrintingContextWin::GetPrinterSettings printing\printing_context_win.cc:433
#14 0x005cf9d9 printing::PrintingContextWin::InitWithSettings printing\printing_context_win.cc:226
#15 0x0041b190 PrintingContextTest_Base_Test::TestBody printing\printing_context_win_unittest.cc:115
#16 0x005b1b17 testing::HandleExceptionsInMethodIfSupportedtesting::Test,void testing\gtest\src\gtest.cc:2130

23:57:36 drmemory_analyze.py [ERROR]
LEAK 128 direct bytes 0x01d23758-0x01d237d8 + 0 indirect bytes

1 0x3f525711 DrvEnableDriver mxdwdrv.dll

...
#16 0x77f1be50 CreateDCW GDI32.dll
#17 0x005d0d93 printing::PrintingContextWin::AllocateContext printing\printing_context_win.cc:465
#18 0x005d0bce printing::PrintingContextWin::GetPrinterSettings printing\printing_context_win.cc:433
#19 0x005cf9d9 printing::PrintingContextWin::InitWithSettings printing\printing_context_win.cc:226
#20 0x0041b190 PrintingContextTest_Base_Test::TestBody printing\printing_context_win_unittest.cc:115
#21 0x005b1b17 testing::HandleExceptionsInMethodIfSupportedtesting::Test,void testing\gtest\src\gtest.cc:2130

I'm not sure if leak+possible_leak are often though.

To address your argument, maybe we can do something like this:
"POSSIBLE LEAK" suppressions apply only to "POSSIBLE LEAK" reports (this is the way we have it now)
while "LEAK" suppressions should apply to both "LEAK" reports and "POSSIBLE LEAK" reports.

What do you think?

[derekbruening]

From bruen...@google.com on January 21, 2011 07:43:24

the DC example involves a real leak and a possible leak that may be a false positive so keeping them separate seems the right thing: we don't want to mask the true positive just b/c we're trying to suppress the false positive

having LEAK match POSSIBLE LEAK does seem reasonable

[derekbruening]

From timurrrr@google.com on April 13, 2011 10:30:57

Should be fixed by r250

Status: Fixed