google / sanitizers

AddressSanitizer, ThreadSanitizer, MemorySanitizer
Other
11.01k stars 998 forks source link

Feature Request: Allow a white list, as the black list based supression don't work well. #1731

Open aerosayan opened 4 months ago

aerosayan commented 4 months ago

Problem:

Many third party libraries like MPI leak memory intentionally (and they're safe), so suppressing errors from them should not be the responsibility of the users, in most cases.

Right now, ASAN 5 or 6 memory leaks for MPI, and since MPI runs many programs in parallel, the number of memory leak errors printed to screen become over 500.

This makes using ASAN impossible due to the large output on the screen.

I tried using the supression system by using leak:mpi and it works well, however, many memory leaks from asan are also printed, which come from MPI, but are tagged as from <unknown module>

An example of such a few errors:

Direct leak of 320 byte(s) in 10 object(s) allocated from:
    #0 0x7fa8a9052bc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #1 0x7fa8a4c7b3eb  (<unknown module>)

Direct leak of 231 byte(s) in 12 object(s) allocated from:
    #0 0x7fa8a8fdb3dd in strdup (/lib/x86_64-linux-gnu/libasan.so.5+0x963dd)
    #1 0x7fa8a4c314b3  (<unknown module>)

I do not know how to suppress the errors from these unknown modules.

The only way to suppress them, is by suppressing leaks from libasan and that's a bad idea, as if we suppress it, we completely suppress all error reports from ASAN.

Solution:

Good developers wrap malloc with a macro or a function, (let's say my_malloc()) and they use it throughout their code to allocate memory.

It would be easier to use ASAN, if ASAN allowed suppression using white lists ... i.e only show memory leaks and errors from code that uses my_malloc and nothing else.

Let's say, the configuration could be checkonly:leak:my_malloc.

Alternative Solution (hack):

In case this issue is not fixed due to some unforseen circumstances, I don't want others to be unable to use ASAN, as it's an amazing tool, so here's a hacky solution that works for me.

Thanks.

mmomtchev commented 2 months ago

Me too I need this feature in a somewhat different context: I use ASAN for a number of Node.js C++ addons. Node.js (and especially V8) have a large number of non-repeat-offending memory leaks - ie allocations that are performed a fixed number of times per process execution and are never deallocated. Currently I maintain a blacklist, but I have to constantly update it at every Node.js release. I think the feature shouldn't be very difficult to implement, I will try to push the PR, but this is a Google project and given my current standing with this company, I doubt they will take it.

vitalybuka commented 2 months ago

given my current standing with this company, I doubt they will take it.

I doubt most potential reviewers, include me, aware of that and will take that into account.

Also source code in question is part of LLVM https://github.com/llvm/llvm-project/tree/main/compiler-rt/lib/lsan

If we'd like to proceed there, I'd recommend a different solution:

There is https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/sanitizer/lsan_interface.h

namespace __lsan {
class ScopedDisabler {
public:
  ScopedDisabler() { __lsan_disable(); }
  ~ScopedDisabler() { __lsan_enable(); }
};

It flips thead local switch to ON/OFF for all allocations. We can make default values of the switch OFF, and then you can change your malloc

void* my_malloc() {
  ScopedEnabler enabled;
  // do stuff
}

I think this is even possible with existing lsan, without changes:

main thread (global constructor): calls __lsan_disable() once every new thread thread: calls __lsan_disable() once my_malloc: enabled/disable as needed

mmomtchev commented 2 months ago

given my current standing with this company, I doubt they will take it.

I doubt most potential reviewers, include me, aware of that and will take that into account.

Ok, I will definitely give it a try then. Hopefully they won't hate you for sending me in their project - because I am sort of GRNG's punishment for project owners at the moment.

What you propose is indeed a solution - especially for OP who has his own malloc - but in my case it would require to extensively litter my code with an optional macro - one in every V8 callback.