[PATCH] D138275: [clang][Interp] Avoid leaking init maps of local primitive arrays

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This shows up when the interpretation is interrupted, in this case because we're reading an uninitialized value, and so we don't get to the `destroy` instruction for the scope.

This adds a `destroyAll()` that calls deallocates all the local variables of all the scopes in the function. This works, but I was wondering if it should instead be a `cleanupLeftovers()` that's only used by the caller _if_ the interpretation wasn't successful.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138275

Files:
  clang/lib/AST/Interp/InterpFrame.cpp
  clang/lib/AST/Interp/InterpFrame.h
  clang/test/AST/Interp/cxx20.cpp


Index: clang/test/AST/Interp/cxx20.cpp
===================================================================
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -304,3 +304,15 @@
   }
   static_assert(testInc2() == 1, "");
 };
+
+constexpr int NoLeakHere() {
+  int abc[2];
+
+  abc[0] = 1;
+  return abc[1]; // expected-note {{read of object outside its lifetime}} \
+                 // ref-note {{read of uninitialized object}}
+}
+static_assert(NoLeakHere() == 3); // expected-error {{not an integral constant expression}} \
+                                  // expected-note {{in call to}} \
+                                  // ref-error {{not an integral constant expression}} \
+                                  // ref-note {{in call to}}
Index: clang/lib/AST/Interp/InterpFrame.h
===================================================================
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -47,6 +47,11 @@
   /// Invokes the destructors for a scope.
   void destroy(unsigned Idx);
 
+  /// Invokes the destructors for all scopes. This is used in the
+  /// InterpFrame destructor to avoid memory leaks in case the
+  /// interpretation was not successful.
+  void destroyAll();
+
   /// Pops the arguments off the stack.
   void popArgs();
 
Index: clang/lib/AST/Interp/InterpFrame.cpp
===================================================================
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -64,6 +64,7 @@
 }
 
 InterpFrame::~InterpFrame() {
+  destroyAll();
   for (auto &Param : Params)
     S.deallocate(reinterpret_cast<Block *>(Param.second.get()));
 }
@@ -74,6 +75,16 @@
   }
 }
 
+void InterpFrame::destroyAll() {
+  if (!Func)
+    return;
+  for (const Scope &Sc : Func->scopes()) {
+    for (auto &Local : Sc.locals()) {
+      S.deallocate(reinterpret_cast<Block *>(localBlock(Local.Offset)));
+    }
+  }
+}
+
 void InterpFrame::popArgs() {
   for (PrimType Ty : Func->args_reverse())
     TYPE_SWITCH(Ty, S.Stk.discard<T>());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138275.476395.patch
Type: text/x-patch
Size: 2059 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221118/c15f3c57/attachment.bin>