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.
I've had problems with 5b54cf1a2892767fe949826a32d7820732028a38
<https://reviews.llvm.org/rG5b54cf1a2892767fe949826a32d7820732028a38> because
we were emitting a `Destroy` op for a local variable before we were done using
that local variable. This moves the local's `Block` to a `DeadBlock` but leaves
the metadata (a `InlineDescriptor` in the case of local variables)
uninitialized, so we ran into failed builds on the msan builders.
This patch simply zeroes the metadata after calling destroying the local
variable. The only difference is that we will now run into an assertion because
the `Descriptor*` is `nullptr`, etc.
The added code could be in a `#ifndef _NDEBUG` block, and that would be better
for performance of course, but... that wouldn't have helped in the case of the
failing msan builders anyway I think.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D142277
Files:
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/EvalEmitter.h
clang/lib/AST/Interp/InterpFrame.cpp
Index: clang/lib/AST/Interp/InterpFrame.cpp
===================================================================
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -75,6 +75,10 @@
void InterpFrame::destroy(unsigned Idx) {
for (auto &Local : Func->getScope(Idx).locals()) {
S.deallocate(reinterpret_cast<Block *>(localBlock(Local.Offset)));
+ // Just make sure we're runnnig into some kind of error when a
+ // local variable is used after being destroyed.
+ InlineDescriptor *ID = localInlineDesc(Local.Offset);
+ std::memset(ID, 0, sizeof(InlineDescriptor));
}
}
Index: clang/lib/AST/Interp/EvalEmitter.h
===================================================================
--- clang/lib/AST/Interp/EvalEmitter.h
+++ clang/lib/AST/Interp/EvalEmitter.h
@@ -92,6 +92,8 @@
/// Temporaries which require storage.
llvm::DenseMap<unsigned, std::unique_ptr<char[]>> Locals;
+ InlineDescriptor *localInlineDesc(unsigned LocalIndex);
+
// The emitter always tracks the current instruction and sets OpPC to a token
// value which is mapped to the location of the opcode being evaluated.
CodePtr OpPC;
Index: clang/lib/AST/Interp/EvalEmitter.cpp
===================================================================
--- clang/lib/AST/Interp/EvalEmitter.cpp
+++ clang/lib/AST/Interp/EvalEmitter.cpp
@@ -49,25 +49,33 @@
EvalEmitter::LabelTy EvalEmitter::getLabel() { return NextLabel++; }
+InlineDescriptor *EvalEmitter::localInlineDesc(unsigned LocalIndex) {
+ auto It = Locals.find(LocalIndex);
+ assert(It != Locals.end() && "Missing local variable");
+ Block *B = reinterpret_cast<Block *>(It->second.get());
+ return reinterpret_cast<InlineDescriptor *>(B->rawData());
+}
+
Scope::Local EvalEmitter::createLocal(Descriptor *D) {
// Allocate memory for a local.
auto Memory = std::make_unique<char[]>(sizeof(Block) + D->getAllocSize());
auto *B = new (Memory.get()) Block(D, /*isStatic=*/false);
B->invokeCtor();
- // Initialize local variable inline descriptor.
- InlineDescriptor &Desc = *reinterpret_cast<InlineDescriptor *>(B->rawData());
- Desc.Desc = D;
- Desc.Offset = sizeof(InlineDescriptor);
- Desc.IsActive = true;
- Desc.IsBase = false;
- Desc.IsMutable = false;
- Desc.IsConst = false;
- Desc.IsInitialized = false;
-
// Register the local.
unsigned Off = Locals.size();
Locals.insert({Off, std::move(Memory)});
+
+ // Initialize local variable inline descriptor.
+ InlineDescriptor *Desc = localInlineDesc(Off);
+ Desc->Desc = D;
+ Desc->Offset = sizeof(InlineDescriptor);
+ Desc->IsActive = true;
+ Desc->IsBase = false;
+ Desc->IsMutable = false;
+ Desc->IsConst = false;
+ Desc->IsInitialized = false;
+
return {Off, D};
}
@@ -240,8 +248,8 @@
assert(It != Locals.end() && "Missing local variable");
auto *B = reinterpret_cast<Block *>(It->second.get());
*reinterpret_cast<T *>(B->data()) = S.Stk.pop<T>();
- InlineDescriptor &Desc = *reinterpret_cast<InlineDescriptor *>(B->rawData());
- Desc.IsInitialized = true;
+ InlineDescriptor *Desc = localInlineDesc(I);
+ Desc->IsInitialized = true;
return true;
}
@@ -254,6 +262,10 @@
auto It = Locals.find(Local.Offset);
assert(It != Locals.end() && "Missing local variable");
S.deallocate(reinterpret_cast<Block *>(It->second.get()));
+ // Just make sure we're runnnig into some kind of error when a
+ // local variable is used after being destroyed.
+ InlineDescriptor *ID = localInlineDesc(I);
+ std::memset(ID, 0, sizeof(InlineDescriptor));
}
return true;
Index: clang/lib/AST/Interp/InterpFrame.cpp
===================================================================
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -75,6 +75,10 @@
void InterpFrame::destroy(unsigned Idx) {
for (auto &Local : Func->getScope(Idx).locals()) {
S.deallocate(reinterpret_cast<Block *>(localBlock(Local.Offset)));
+ // Just make sure we're runnnig into some kind of error when a
+ // local variable is used after being destroyed.
+ InlineDescriptor *ID = localInlineDesc(Local.Offset);
+ std::memset(ID, 0, sizeof(InlineDescriptor));
}
}
Index: clang/lib/AST/Interp/EvalEmitter.h
===================================================================
--- clang/lib/AST/Interp/EvalEmitter.h
+++ clang/lib/AST/Interp/EvalEmitter.h
@@ -92,6 +92,8 @@
/// Temporaries which require storage.
llvm::DenseMap<unsigned, std::unique_ptr<char[]>> Locals;
+ InlineDescriptor *localInlineDesc(unsigned LocalIndex);
+
// The emitter always tracks the current instruction and sets OpPC to a token
// value which is mapped to the location of the opcode being evaluated.
CodePtr OpPC;
Index: clang/lib/AST/Interp/EvalEmitter.cpp
===================================================================
--- clang/lib/AST/Interp/EvalEmitter.cpp
+++ clang/lib/AST/Interp/EvalEmitter.cpp
@@ -49,25 +49,33 @@
EvalEmitter::LabelTy EvalEmitter::getLabel() { return NextLabel++; }
+InlineDescriptor *EvalEmitter::localInlineDesc(unsigned LocalIndex) {
+ auto It = Locals.find(LocalIndex);
+ assert(It != Locals.end() && "Missing local variable");
+ Block *B = reinterpret_cast<Block *>(It->second.get());
+ return reinterpret_cast<InlineDescriptor *>(B->rawData());
+}
+
Scope::Local EvalEmitter::createLocal(Descriptor *D) {
// Allocate memory for a local.
auto Memory = std::make_unique<char[]>(sizeof(Block) + D->getAllocSize());
auto *B = new (Memory.get()) Block(D, /*isStatic=*/false);
B->invokeCtor();
- // Initialize local variable inline descriptor.
- InlineDescriptor &Desc = *reinterpret_cast<InlineDescriptor *>(B->rawData());
- Desc.Desc = D;
- Desc.Offset = sizeof(InlineDescriptor);
- Desc.IsActive = true;
- Desc.IsBase = false;
- Desc.IsMutable = false;
- Desc.IsConst = false;
- Desc.IsInitialized = false;
-
// Register the local.
unsigned Off = Locals.size();
Locals.insert({Off, std::move(Memory)});
+
+ // Initialize local variable inline descriptor.
+ InlineDescriptor *Desc = localInlineDesc(Off);
+ Desc->Desc = D;
+ Desc->Offset = sizeof(InlineDescriptor);
+ Desc->IsActive = true;
+ Desc->IsBase = false;
+ Desc->IsMutable = false;
+ Desc->IsConst = false;
+ Desc->IsInitialized = false;
+
return {Off, D};
}
@@ -240,8 +248,8 @@
assert(It != Locals.end() && "Missing local variable");
auto *B = reinterpret_cast<Block *>(It->second.get());
*reinterpret_cast<T *>(B->data()) = S.Stk.pop<T>();
- InlineDescriptor &Desc = *reinterpret_cast<InlineDescriptor *>(B->rawData());
- Desc.IsInitialized = true;
+ InlineDescriptor *Desc = localInlineDesc(I);
+ Desc->IsInitialized = true;
return true;
}
@@ -254,6 +262,10 @@
auto It = Locals.find(Local.Offset);
assert(It != Locals.end() && "Missing local variable");
S.deallocate(reinterpret_cast<Block *>(It->second.get()));
+ // Just make sure we're runnnig into some kind of error when a
+ // local variable is used after being destroyed.
+ InlineDescriptor *ID = localInlineDesc(I);
+ std::memset(ID, 0, sizeof(InlineDescriptor));
}
return true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
