trafficserver
trafficserver copied to clipboard
apache
Clean up spurious errors in output of ASAN build.
That is, errors for allocations that are still referenced, but not deallocated, when TS exists.
Includes a registry for items to be destroyed/deleted in static destruction, in the ASAN build only.
Ideally, all Au tests should run with a ASAN build with no errors. This PR doesn't fully achieve that goal, but it does make ASAN much more usable. It eliminates a lot of benign leak errors from the ASAN output.
There is an documented way in LeakSanitizer to ignore leaks: https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer#suppressions. We should use that method if we want to ignore leaks.
If we want to change the code to remove the leak, we should do it in a way that removes it in both sanitizer and non-sanitizer builds. Having different code for sanitizer vs non-sanitizer should be a last resort since it defeats the verification function of the sanitizers.
The intent of this PR is to make ASAN more usable. Much of ATS could politely be called "pre-Modern", but there doesn't seem to be consensus to make addressing that a priority. There has to be consensus on strategic goals. If we all start blocking PRs to try to enforce what we individually think strategic goals should be, that's a recipe for gridlock.
The registry only identifies and filters "leaks" that have been determined to be benign. It doesn't defeat the purpose of the sanitizer. To me, it seems more maintainable than a single file with a vague(pattern-based) list of all the benign leaks.
I prefer the file as its easier to reason about and it doesn't come with incidental code.
I prefer the file as its easier to reason about and it doesn't come with incidental code.
I don't understand "reason about". To me, when something is added to the registry, it clarifies the intent to allow process exit to clean up the allocation. Without having to go look in a separate file, remember the precise format of the ASAN output. Doesn't look like the meaning of "pattern" is well documented, whether it's a full regex, etc. What would the leak suppressor do if a pattern matched no leak? The registry can handle it if whether or not something is deallocated by process exit depends on runtime circumstances.
I just mean that when you are doing "ASAN" things and looking for leaks, all of the excluded ones are all in one place. This way managed with code is incidental to anything else. Also, we don't need to invent a new method to mark "benign" leaks when one is already provided.
When something is provided, one also has to consider how well documented and therefore usable it is.