[PATCH] D90599: Fix a leak in `ASTUnit::LoadFromCommandLine()` wehn compiler invocation fails
Boris Staletic via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 2 05:36:58 PST 2020
bstaletic created this revision.
bstaletic added a reviewer: aprantl.
bstaletic added projects: clang, clang-c.
Herald added a subscriber: cfe-commits.
bstaletic requested review of this revision.
1. Here <https://github.com/llvm/llvm-project/blob/master/clang/tools/libclang/CIndex.cpp#L3567-L3571> we allocate buffers to hold the contents of unsaved files.
2. The `RemappedFiles` vector is then passed to `ASTUnit::LoadFromCommandLine()` as an `ArrayRef`, here <https://github.com/llvm/llvm-project/blob/master/clang/tools/libclang/CIndex.cpp#L3632>
3. In ASTUnit::LoadFromCommandLine() <https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/ASTUnit.cpp#L1735> we try to make a compiler invocation and exit early if that fails <https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/ASTUnit.cpp#L1758-L1761>.
4. If we did exit early from that function, both Unit and UnitErr are nullptr <https://github.com/llvm/llvm-project/blob/master/clang/tools/libclang/CIndex.cpp#L3641-L3642>, making the function exit without ever cleaning up the memory allocations from step 1.
Notes:
1. The function allocating all those `RemappedFile` objects is not actually the same as the one deallocating, in the happy case.
1.1. I did not figure out where exactly does it get deleted, because the above seems to be enough data for now.
2. My attempted solution is to iterate over remapped files and deallocate in this block <https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/ASTUnit.cpp#L1760-L1761>, but it seems odd to deallocate through `ArrayRef`.
Fixes https://bugs.llvm.org/show_bug.cgi?id=47832
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D90599
Files:
clang/lib/Frontend/ASTUnit.cpp
Index: clang/lib/Frontend/ASTUnit.cpp
===================================================================
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1758,7 +1758,11 @@
CI = createInvocationFromCommandLine(
llvm::makeArrayRef(ArgBegin, ArgEnd), Diags, VFS);
if (!CI)
+ {
+ for (const auto &RF : RemappedFiles)
+ delete RF.second;
return nullptr;
+ }
}
// Override any files that need remapping
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90599.302246.patch
Type: text/x-patch
Size: 476 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201102/9d0d851c/attachment.bin>
More information about the cfe-commits
mailing list