Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
16 changes: 16 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,16 @@
// 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;

// 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 *);
34 changes: 34 additions & 0 deletions clang/test/Analysis/NewDeleteLeaks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,37 @@ 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 automaticall generated
// protobuf code that passes dynamically allocated memory to a certain function
// named GetOwnedMessageInternal.
namespace protobuf_leak {
#include "Inputs/system-header-simulator-for-protobuf.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just include this at the top, or create a separate test file.
Including into a namespace is pretty evil.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I moved the include to the top. (I didn't want to create a separate test file because I feel that it would be too small.)


class MessageLite { int SomeField; }; // Sufficient for our purposes.
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