Index: include/llvm/CodeGen/SelectionDAGNodes.h =================================================================== --- include/llvm/CodeGen/SelectionDAGNodes.h +++ include/llvm/CodeGen/SelectionDAGNodes.h @@ -923,7 +923,10 @@ inline SDValue::SDValue(SDNode *node, unsigned resno) : Node(node), ResNo(resno) { - assert((!Node || ResNo < Node->getNumValues()) && + // Explicitly check for !ResNo to avoid use-after-free, because there are + // callers that use SDValue(N, 0) with a deleted N to indicate successful + // combines. + assert((!Node || !ResNo || ResNo < Node->getNumValues()) && "Invalid result number for the given node!"); assert(ResNo < -2U && "Cannot use result numbers reserved for DenseMaps."); } Index: include/llvm/Support/ArrayRecycler.h =================================================================== --- include/llvm/Support/ArrayRecycler.h +++ include/llvm/Support/ArrayRecycler.h @@ -48,6 +48,8 @@ FreeList *Entry = Bucket[Idx]; if (!Entry) return nullptr; + __asan_unpoison_memory_region(Entry, + sizeof(T) * Capacity::get(Idx).getSize()); Bucket[Idx] = Entry->Next; return reinterpret_cast(Entry); } @@ -60,6 +62,8 @@ Bucket.resize(size_t(Idx) + 1); Entry->Next = Bucket[Idx]; Bucket[Idx] = Entry; + __asan_poison_memory_region(Entry, + sizeof(T) * Capacity::get(Idx).getSize()); } public: Index: include/llvm/Support/Recycler.h =================================================================== --- include/llvm/Support/Recycler.h +++ include/llvm/Support/Recycler.h @@ -43,6 +43,9 @@ FreeNode *pop_val() { auto *Val = FreeList; + // Always unpoison the full Size, because the combiner relies on being able + // to morph SDNodes into MachineSDNodes. + __asan_unpoison_memory_region(FreeList, Size); FreeList = FreeList->Next; return Val; } @@ -50,6 +53,7 @@ void push(FreeNode *N) { N->Next = FreeList; FreeList = N; + __asan_poison_memory_region(FreeList, Size); } public: Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp =================================================================== --- lib/CodeGen/SelectionDAG/DAGCombiner.cpp +++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp @@ -446,10 +446,11 @@ /// This is a helper function for MergeConsecutiveStores. When the source /// elements of the consecutive stores are all constants or all extracted /// vector elements, try to merge them into one larger store. - /// \return True if a merged store was created. - bool MergeStoresOfConstantsOrVecElts(SmallVectorImpl &StoreNodes, - EVT MemVT, unsigned NumStores, - bool IsConstantSrc, bool UseVector); + /// \return number of stores that were merged into a merged store (always + /// a prefix of \p StoreNode). + unsigned MergeStoresOfConstantsOrVecElts( + SmallVectorImpl &StoreNodes, EVT MemVT, unsigned NumStores, + bool IsConstantSrc, bool UseVector); /// This is a helper function for MergeConsecutiveStores. /// Stores that may be merged are placed in StoreNodes. @@ -466,8 +467,10 @@ /// Merge consecutive store operations into a wide store. /// This optimization uses wide integers or vectors when possible. - /// \return True if some memory operations were changed. - bool MergeConsecutiveStores(StoreSDNode *N); + /// \return number of stores that were merged into a merged store (the + /// affected nodes are stored as a prefix in \p StoreNodes). + unsigned MergeConsecutiveStores(StoreSDNode *N, + SmallVectorImpl &StoreNodes); /// \brief Try to transform a truncation where C is a constant: /// (trunc (and X, C)) -> (and (trunc X), (trunc C)) @@ -2198,8 +2201,8 @@ SDValue Op1 = Node->getOperand(1); SDValue combined; for (SDNode::use_iterator UI = Op0.getNode()->use_begin(), - UE = Op0.getNode()->use_end(); UI != UE; ++UI) { - SDNode *User = *UI; + UE = Op0.getNode()->use_end(); UI != UE;) { + SDNode *User = *UI++; if (User == Node || User->use_empty()) continue; // Convert the other matching node(s), too; @@ -11288,12 +11291,12 @@ return DAG.getBuildVector(Ty, SL, BuildVector); } -bool DAGCombiner::MergeStoresOfConstantsOrVecElts( +unsigned DAGCombiner::MergeStoresOfConstantsOrVecElts( SmallVectorImpl &StoreNodes, EVT MemVT, unsigned NumStores, bool IsConstantSrc, bool UseVector) { // Make sure we have something to merge. if (NumStores < 2) - return false; + return 0; int64_t ElementSizeBytes = MemVT.getSizeInBits() / 8; LSBaseSDNode *FirstInChain = StoreNodes[0].MemNode; @@ -11413,7 +11416,7 @@ } } - return true; + return NumStores; } void DAGCombiner::getStoreMergeAndAliasCandidates( @@ -11555,9 +11558,10 @@ return true; } -bool DAGCombiner::MergeConsecutiveStores(StoreSDNode* St) { +unsigned DAGCombiner::MergeConsecutiveStores( + StoreSDNode* St, SmallVectorImpl &StoreNodes) { if (OptLevel == CodeGenOpt::None) - return false; + return 0; EVT MemVT = St->getMemoryVT(); int64_t ElementSizeBytes = MemVT.getSizeInBits() / 8; @@ -11566,10 +11570,10 @@ // This function cannot currently deal with non-byte-sized memory sizes. if (ElementSizeBytes * 8 != MemVT.getSizeInBits()) - return false; + return 0; if (!MemVT.isSimple()) - return false; + return 0; // Perform an early exit check. Do not bother looking at stored values that // are not constants, loads, or extracted vector elements. @@ -11581,18 +11585,18 @@ StoredVal.getOpcode() == ISD::EXTRACT_SUBVECTOR); if (!IsConstantSrc && !IsLoadSrc && !IsExtractVecSrc) - return false; + return 0; // Don't merge vectors into wider vectors if the source data comes from loads. // TODO: This restriction can be lifted by using logic similar to the // ExtractVecSrc case. if (MemVT.isVector() && IsLoadSrc) - return false; + return 0; // Only look at ends of store sequences. SDValue Chain = SDValue(St, 0); if (Chain->hasOneUse() && Chain->use_begin()->getOpcode() == ISD::STORE) - return false; + return 0; // Save the LoadSDNodes that we find in the chain. // We need to make sure that these nodes do not interfere with @@ -11600,19 +11604,17 @@ SmallVector AliasLoadNodes; // Save the StoreSDNodes that we find in the chain. - SmallVector StoreNodes; - getStoreMergeAndAliasCandidates(St, StoreNodes, AliasLoadNodes); // Check if there is anything to merge. if (StoreNodes.size() < 2) - return false; + return 0; // only do dependence check in AA case bool UseAA = CombinerAA.getNumOccurrences() > 0 ? CombinerAA : DAG.getSubtarget().useAA(); if (UseAA && !checkMergeStoreCandidatesForDependencies(StoreNodes)) - return false; + return 0; // Sort the memory operands according to their distance from the // base pointer. As a secondary criteria: make sure stores coming @@ -11740,7 +11742,7 @@ // consecutive loads). if (StoreValOpcode != ISD::EXTRACT_VECTOR_ELT && StoreValOpcode != ISD::EXTRACT_SUBVECTOR) - return false; + return 0; // Find a legal type for the vector store. unsigned Elts = i + 1; @@ -11807,14 +11809,14 @@ } if (LoadNodes.size() < 2) - return false; + return 0; // If we have load/store pair instructions and we only have two values, // don't bother. unsigned RequiredAlignment; if (LoadNodes.size() == 2 && TLI.hasPairedLoad(MemVT, RequiredAlignment) && St->getAlignment() >= RequiredAlignment) - return false; + return 0; LoadSDNode *FirstLoad = cast(LoadNodes[0].MemNode); unsigned FirstLoadAS = FirstLoad->getAddressSpace(); @@ -11888,7 +11890,7 @@ NumElem = std::min(LastLegalType, NumElem); if (NumElem < 2) - return false; + return 0; // Collect the chains from all merged stores. SmallVector MergeStoreChains; @@ -11960,7 +11962,7 @@ } } - return true; + return NumElem; } SDValue DAGCombiner::replaceStoreChain(StoreSDNode *ST, SDValue BetterChain) { @@ -12205,14 +12207,27 @@ if (!LegalTypes) { bool EverChanged = false; - do { + for (;;) { // There can be multiple store sequences on the same chain. // Keep trying to merge store sequences until we are unable to do so // or until we merge the last store on the chain. - bool Changed = MergeConsecutiveStores(ST); - EverChanged |= Changed; - if (!Changed) break; - } while (ST->getOpcode() != ISD::DELETED_NODE); + SmallVector StoreNodes; + unsigned NumChanged = MergeConsecutiveStores(ST, StoreNodes); + if (!NumChanged) + break; + + EverChanged = true; + + bool ChangedST = false; + for (unsigned i = 0; i < NumChanged; ++i) { + if (StoreNodes[i].MemNode == ST) { + ChangedST = true; + break; + } + } + if (ChangedST) + break; + } if (EverChanged) return SDValue(N, 0);