https://github.com/isuckatcs created 
https://github.com/llvm/llvm-project/pull/68246

`InterpStack` is using an `std::vector<>` to track the `ItemTypes`. As a 
result, the new types are inserted
to the back of the `std::vector<>`, however `dump()` was reading the types from 
the front (the bottom 
of the stack) and printing the value on the top of the stack.

This lead to a crash if the type on the bottom had a different type from the 
type on the top. E.g.:
```
Items: 2. Size: 40
0/8: 0
1/40: 0x5590cddc0460 {16, 16, 32}
```

The same method also miscalculated the offsets during printing the stack, which 
was a source of
incorrect stack dumps and future crashes.

This patch changes the order of iteration of the types and fixes the offset 
calculation. 

As for testing the change, the issue is that it needs to be done as a unittest, 
however from 
`clang/unittests` we don't have access to `clang/lib`, where `Interp` resides. 
Although the previous
implementation didn't have unittests either, so I'm not sure if we actually 
care that much or not.


>From c877e870ca4d93b206d3fbe1d0ee019ecbce206b Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isucka...@users.noreply.github.com>
Date: Wed, 4 Oct 2023 20:53:45 +0200
Subject: [PATCH] fix stack dump

---
 clang/lib/AST/Interp/InterpStack.cpp | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/clang/lib/AST/Interp/InterpStack.cpp 
b/clang/lib/AST/Interp/InterpStack.cpp
index da4b36f8c1bf351..b1e042c967d4241 100644
--- a/clang/lib/AST/Interp/InterpStack.cpp
+++ b/clang/lib/AST/Interp/InterpStack.cpp
@@ -85,20 +85,25 @@ void InterpStack::shrink(size_t Size) {
 
 void InterpStack::dump() const {
 #ifndef NDEBUG
-  llvm::errs() << "Items: " << ItemTypes.size() << ". Size: " << size() << 
"\n";
+  llvm::errs() << "Items: " << ItemTypes.size() << ". Size: " << size() << 
'\n';
   if (ItemTypes.empty())
     return;
 
   size_t Index = 0;
-  size_t Offset = align(primSize(ItemTypes[0]));
-  for (PrimType Ty : ItemTypes) {
-    llvm::errs() << Index << "/" << Offset << ": ";
-    TYPE_SWITCH(Ty, {
+  size_t Offset = 0;
+
+  // The type of the item on the top of the stack is inserted to the back
+  // of the vector, so the iteration has to happen backwards.
+  for (auto TyIt = ItemTypes.rbegin(); TyIt != ItemTypes.rend(); ++TyIt) {
+    Offset += align(primSize(*TyIt));
+
+    llvm::errs() << Index << '/' << Offset << ": ";
+    TYPE_SWITCH(*TyIt, {
       const T &V = peek<T>(Offset);
       llvm::errs() << V;
     });
-    llvm::errs() << "\n";
-    Offset += align(primSize(Ty));
+    llvm::errs() << '\n';
+
     ++Index;
   }
 #endif

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to