diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 83d79b43afe9f..70baab54df563 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -3812,6 +3812,15 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly( return true; } + // Protobuf function declared in `generated_message_util.h` that takes + // ownership of the second argument. As the first and third arguments are + // allocation arenas and won't be tracked by this checker, there is no reason + // to set `EscapingSymbol`. (Also, this is an implementation detail of + // Protobuf, so it's better to be a bit more permissive.) + if (FName == "GetOwnedMessageInternal") { + return true; + } + // Handle cases where we know a buffer's /address/ can escape. // Note that the above checks handle some special cases where we know that // even though the address escapes, it's still our responsibility to free the diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-protobuf.h b/clang/test/Analysis/Inputs/system-header-simulator-for-protobuf.h new file mode 100644 index 0000000000000..cb12b55a00925 --- /dev/null +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-protobuf.h @@ -0,0 +1,18 @@ +// Like the compiler, the static analyzer treats some functions differently if +// they come from a system header -- for example, it is assumed that system +// functions do not arbitrarily free() their parameters, and that some bugs +// found in system headers cannot be fixed by the user and should be +// suppressed. +#pragma clang system_header + +class Arena; +class MessageLite { + int SomeArbitraryField; +}; + +// Originally declared in generated_message_util.h +MessageLite *GetOwnedMessageInternal(Arena *, MessageLite *, Arena *); + +// Not a real protobuf function -- just introduced to validate that this file +// is handled as a system header. +void SomeOtherFunction(MessageLite *); diff --git a/clang/test/Analysis/NewDeleteLeaks.cpp b/clang/test/Analysis/NewDeleteLeaks.cpp index b2bad7e76fad0..d9c4b77d1e6fc 100644 --- a/clang/test/Analysis/NewDeleteLeaks.cpp +++ b/clang/test/Analysis/NewDeleteLeaks.cpp @@ -13,6 +13,8 @@ // RUN: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes=true #include "Inputs/system-header-simulator-for-malloc.h" +// For the tests in namespace protobuf_leak: +#include "Inputs/system-header-simulator-for-protobuf.h" //===----------------------------------------------------------------------===// // Report for which we expect NoOwnershipChangeVisitor to add a new note. @@ -218,3 +220,34 @@ void caller() { (void)n; } // no-warning: No potential memory leak here, because that's been already reported. } // namespace symbol_reaper_lifetime + +// Check that we do not report false positives in automatically generated +// protobuf code that passes dynamically allocated memory to a certain function +// named GetOwnedMessageInternal. +namespace protobuf_leak { +Arena *some_arena, *some_submessage_arena; + +MessageLite *protobuf_leak() { + MessageLite *p = new MessageLite(); // Real protobuf code instantiates a + // subclass of MessageLite, but that's + // not relevant for the bug. + MessageLite *q = GetOwnedMessageInternal(some_arena, p, some_submessage_arena); + return q; + // No leak at end of function -- the pointer escapes in GetOwnedMessageInternal. +} + +void validate_system_header() { + // The case protobuf_leak would also pass if GetOwnedMessageInternal wasn't + // declared in a system header. This test verifies that another function + // declared in the same header behaves differently (doesn't escape memory) to + // demonstrate that GetOwnedMessageInternal is indeed explicitly recognized + // by the analyzer. + + // expected-note@+1 {{Memory is allocated}} + MessageLite *p = new MessageLite(); + SomeOtherFunction(p); + // expected-warning@+2 {{Potential leak of memory pointed to by 'p'}} + // expected-note@+1 {{Potential leak of memory pointed to by 'p'}} +} + +} // namespace protobuf_leak