-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[lldb][test] Free buffers in demangling tests to avoid leaks #142676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) ChangesTest case added by f669b9c / #137793. Note that the Full diff: https://github.com/llvm/llvm-project/pull/142676.diff 1 Files Affected:
diff --git a/lldb/unittests/Core/MangledTest.cpp b/lldb/unittests/Core/MangledTest.cpp
index dfdc026521379..e9dfd05459642 100644
--- a/lldb/unittests/Core/MangledTest.cpp
+++ b/lldb/unittests/Core/MangledTest.cpp
@@ -642,8 +642,10 @@ TEST_P(DemanglingInfoCorrectnessTestFixutre, Correctness) {
// function names.
if (Root->getKind() !=
llvm::itanium_demangle::Node::Kind::KFunctionEncoding &&
- Root->getKind() != llvm::itanium_demangle::Node::Kind::KDotSuffix)
+ Root->getKind() != llvm::itanium_demangle::Node::Kind::KDotSuffix) {
+ std::free(OB.getBuffer());
return;
+ }
ASSERT_TRUE(OB.NameInfo.hasBasename());
@@ -670,6 +672,7 @@ TEST_P(DemanglingInfoCorrectnessTestFixutre, Correctness) {
return_right, qualifiers, suffix);
EXPECT_EQ(reconstructed_name, demangled);
+ std::free(OB.getBuffer());
}
INSTANTIATE_TEST_SUITE_P(
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Maybe an opportunity to use a unique_ptr with a custom deleter? Something like:
struct TrackingOutputBufferDeleter {
void operator()(TrackingOutputBuffer* TOB) {
if (!TOB)
return;
std::free(TOB->getBuffer());
std::free(TOB);
}
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! i keep forgetting
Yes, that would be a good improvement, especially since the test method here has an early return. I'd like to land this as-is to plug the leak, but I'll start a new branch now w/ that suggestion. |
Sent #142815. The suggestion is almost perfect. The buffer can be free'd, but TOB must be deleted, otherwise there's a mismatch: |
Test case added by f669b9c / #137793. Note that the
DemanglingParts
case above also frees the buffer; this new test case is inconsistent.