Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions clang/test/Analysis/Inputs/system-header-simulator-for-protobuf.h
Original file line number Diff line number Diff line change
@@ -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 *);
33 changes: 33 additions & 0 deletions clang/test/Analysis/NewDeleteLeaks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Loading