[PATCH] D90599: Fix a leak in `ASTUnit::LoadFromCommandLine()` wehn compiler invocation fails

[Boris Staletic via Phabricator via cfe-commits]

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>