ebevhan created this revision. ebevhan added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits.
CGLoopInfo was keeping pointers to parent loop LoopInfos, but when the loop info vector grew, it reallocated the storage and invalidated all of the parent pointers, causing use-after-free. Manage the lifetimes of the LoopInfos separately so that the pointers aren't stale. Repository: rC Clang https://reviews.llvm.org/D66206 Files: lib/CodeGen/CGLoopInfo.cpp lib/CodeGen/CGLoopInfo.h test/CodeGen/loop-info-asan.c Index: test/CodeGen/loop-info-asan.c =================================================================== --- /dev/null +++ test/CodeGen/loop-info-asan.c @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -o /dev/null + +// This test should not exhibit use-after-free in LoopInfo. + +int a() { + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + ; +} Index: lib/CodeGen/CGLoopInfo.h =================================================================== --- lib/CodeGen/CGLoopInfo.h +++ lib/CodeGen/CGLoopInfo.h @@ -275,11 +275,11 @@ bool hasInfo() const { return !Active.empty(); } /// Return the LoopInfo for the current loop. HasInfo should be called /// first to ensure LoopInfo is present. - const LoopInfo &getInfo() const { return Active.back(); } + const LoopInfo &getInfo() const { return *Active.back(); } /// The set of attributes that will be applied to the next pushed loop. LoopAttributes StagedAttrs; /// Stack of active loops. - llvm::SmallVector<LoopInfo, 4> Active; + llvm::SmallVector<std::unique_ptr<LoopInfo>, 4> Active; }; } // end namespace CodeGen Index: lib/CodeGen/CGLoopInfo.cpp =================================================================== --- lib/CodeGen/CGLoopInfo.cpp +++ lib/CodeGen/CGLoopInfo.cpp @@ -554,8 +554,9 @@ void LoopInfoStack::push(BasicBlock *Header, const llvm::DebugLoc &StartLoc, const llvm::DebugLoc &EndLoc) { - Active.push_back(LoopInfo(Header, StagedAttrs, StartLoc, EndLoc, - Active.empty() ? nullptr : &Active.back())); + Active.emplace_back( + new LoopInfo(Header, StagedAttrs, StartLoc, EndLoc, + Active.empty() ? nullptr : Active.back().get())); // Clear the attributes so nested loops do not inherit them. StagedAttrs.clear(); } @@ -747,16 +748,16 @@ void LoopInfoStack::pop() { assert(!Active.empty() && "No active loops to pop"); - Active.back().finish(); + Active.back()->finish(); Active.pop_back(); } void LoopInfoStack::InsertHelper(Instruction *I) const { if (I->mayReadOrWriteMemory()) { SmallVector<Metadata *, 4> AccessGroups; - for (const LoopInfo &AL : Active) { + for (const auto &AL : Active) { // Here we assume that every loop that has an access group is parallel. - if (MDNode *Group = AL.getAccessGroup()) + if (MDNode *Group = AL->getAccessGroup()) AccessGroups.push_back(Group); } MDNode *UnionMD = nullptr;
Index: test/CodeGen/loop-info-asan.c =================================================================== --- /dev/null +++ test/CodeGen/loop-info-asan.c @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s -o /dev/null + +// This test should not exhibit use-after-free in LoopInfo. + +int a() { + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + for (;;) + ; +} Index: lib/CodeGen/CGLoopInfo.h =================================================================== --- lib/CodeGen/CGLoopInfo.h +++ lib/CodeGen/CGLoopInfo.h @@ -275,11 +275,11 @@ bool hasInfo() const { return !Active.empty(); } /// Return the LoopInfo for the current loop. HasInfo should be called /// first to ensure LoopInfo is present. - const LoopInfo &getInfo() const { return Active.back(); } + const LoopInfo &getInfo() const { return *Active.back(); } /// The set of attributes that will be applied to the next pushed loop. LoopAttributes StagedAttrs; /// Stack of active loops. - llvm::SmallVector<LoopInfo, 4> Active; + llvm::SmallVector<std::unique_ptr<LoopInfo>, 4> Active; }; } // end namespace CodeGen Index: lib/CodeGen/CGLoopInfo.cpp =================================================================== --- lib/CodeGen/CGLoopInfo.cpp +++ lib/CodeGen/CGLoopInfo.cpp @@ -554,8 +554,9 @@ void LoopInfoStack::push(BasicBlock *Header, const llvm::DebugLoc &StartLoc, const llvm::DebugLoc &EndLoc) { - Active.push_back(LoopInfo(Header, StagedAttrs, StartLoc, EndLoc, - Active.empty() ? nullptr : &Active.back())); + Active.emplace_back( + new LoopInfo(Header, StagedAttrs, StartLoc, EndLoc, + Active.empty() ? nullptr : Active.back().get())); // Clear the attributes so nested loops do not inherit them. StagedAttrs.clear(); } @@ -747,16 +748,16 @@ void LoopInfoStack::pop() { assert(!Active.empty() && "No active loops to pop"); - Active.back().finish(); + Active.back()->finish(); Active.pop_back(); } void LoopInfoStack::InsertHelper(Instruction *I) const { if (I->mayReadOrWriteMemory()) { SmallVector<Metadata *, 4> AccessGroups; - for (const LoopInfo &AL : Active) { + for (const auto &AL : Active) { // Here we assume that every loop that has an access group is parallel. - if (MDNode *Group = AL.getAccessGroup()) + if (MDNode *Group = AL->getAccessGroup()) AccessGroups.push_back(Group); } MDNode *UnionMD = nullptr;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits