[Polly] Fix use-after-free.

VirtualUse of type UseKind::Inter expects the definition of a
llvm::Value to be represented in another statement. In the bug report
that statement has been removed due to its domain being empty.
Scop::InstStmtMap for the llvm::Value's defintion still pointed to the
removed statement, which resulted in the use-after-free.

The defintion statement was removed by Simplify because it was
considered to not be reachable by other uses; trivially because it is
never executed due to its empty domain. However, no such thing happend
to the using statement using the value altough its domain is also empty.

Fix by always removing statements with empty domains in Simplify since
these are not properly analyzable. A UseKind::Inter should always have a
statement with its defintion due to LLVM's SSA form.
Scop::removeStmtNotInDomainMap() also removes statements with empty
domains but does so without considering the context as used by
Simplify's analyzes.

In another angle, InstStmtMap pointing to removed statements should not
happen either and ForwardOpTree would have bailed out if the llvm::Value
definition was not represented by a statement. This will be corrected in
a followup-commit.

This fixes llvm.org/PR47098

Author: Meinersbur

polly/include/polly/ScopInfo.h

@@ -1949,18 +1949,6 @@ class Scop {
   void addScopStmt(Region *R, StringRef Name, Loop *SurroundingLoop,
                    std::vector<Instruction *> EntryBlockInstructions);
 
-  /// Remove statements from the list of scop statements.
-  ///
-  /// @param ShouldDelete  A function that returns true if the statement passed
-  ///                      to it should be deleted.
-  /// @param AfterHoisting If true, also remove from data access lists.
-  ///                      These lists are filled during
-  ///                      ScopBuilder::buildAccessRelations. Therefore, if this
-  ///                      method is called before buildAccessRelations, false
-  ///                      must be passed.
-  void removeStmts(std::function<bool(ScopStmt &)> ShouldDelete,
-                   bool AfterHoisting = true);
-
   /// Removes @p Stmt from the StmtMap.
   void removeFromStmtMap(ScopStmt &Stmt);
 
@@ -2321,6 +2309,19 @@ class Scop {
     MinMaxAliasGroups.back().first = MinMaxAccessesReadWrite;
     MinMaxAliasGroups.back().second = MinMaxAccessesReadOnly;
   }
+
+  /// Remove statements from the list of scop statements.
+  ///
+  /// @param ShouldDelete  A function that returns true if the statement passed
+  ///                      to it should be deleted.
+  /// @param AfterHoisting If true, also remove from data access lists.
+  ///                      These lists are filled during
+  ///                      ScopBuilder::buildAccessRelations. Therefore, if this
+  ///                      method is called before buildAccessRelations, false
+  ///                      must be passed.
+  void removeStmts(std::function<bool(ScopStmt &)> ShouldDelete,
+                   bool AfterHoisting = true);
+
   /// Get an isl string representing the context.
   std::string getContextStr() const;
 

polly/lib/Transform/Simplify.cpp

@@ -41,6 +41,8 @@ static int const SimplifyMaxDisjuncts = 4;
 TWO_STATISTICS(ScopsProcessed, "Number of SCoPs processed");
 TWO_STATISTICS(ScopsModified, "Number of SCoPs simplified");
 
+TWO_STATISTICS(TotalEmptyDomainsRemoved,
+               "Number of statement with empty domains removed in any SCoP");
 TWO_STATISTICS(TotalOverwritesRemoved, "Number of removed overwritten writes");
 TWO_STATISTICS(TotalWritesCoalesced, "Number of writes coalesced with another");
 TWO_STATISTICS(TotalRedundantWritesRemoved,
@@ -124,6 +126,9 @@ class Simplify : public ScopPass {
   /// The last/current SCoP that is/has been processed.
   Scop *S;
 
+  /// Number of statements with empty domains removed from the SCoP.
+  int EmptyDomainsRemoved = 0;
+
   /// Number of writes that are overwritten anyway.
   int OverwritesRemoved = 0;
 
@@ -147,10 +152,36 @@ class Simplify : public ScopPass {
 
   /// Return whether at least one simplification has been applied.
   bool isModified() const {
-    return OverwritesRemoved > 0 || WritesCoalesced > 0 ||
-           RedundantWritesRemoved > 0 || EmptyPartialAccessesRemoved > 0 ||
-           DeadAccessesRemoved > 0 || DeadInstructionsRemoved > 0 ||
-           StmtsRemoved > 0;
+    return EmptyDomainsRemoved > 0 || OverwritesRemoved > 0 ||
+           WritesCoalesced > 0 || RedundantWritesRemoved > 0 ||
+           EmptyPartialAccessesRemoved > 0 || DeadAccessesRemoved > 0 ||
+           DeadInstructionsRemoved > 0 || StmtsRemoved > 0;
+  }
+
+  /// Remove statements that are never executed due to their domains being
+  /// empty.
+  ///
+  /// In contrast to Scop::simplifySCoP, this removes based on the SCoP's
+  /// effective domain, i.e. including the SCoP's context as used by some other
+  /// simplification methods in this pass. This is necessary because the
+  /// analysis on empty domains is unreliable, e.g. remove a scalar value
+  /// definition MemoryAccesses, but not its use.
+  void removeEmptyDomainStmts() {
+    size_t NumStmtsBefore = S->getSize();
+
+    auto ShouldDelete = [](ScopStmt &Stmt) -> bool {
+      auto EffectiveDomain =
+          Stmt.getDomain().intersect_params(Stmt.getParent()->getContext());
+      return EffectiveDomain.is_empty();
+    };
+    S->removeStmts(ShouldDelete);
+
+    assert(NumStmtsBefore >= S->getSize());
+    EmptyDomainsRemoved = NumStmtsBefore - S->getSize();
+    LLVM_DEBUG(dbgs() << "Removed " << EmptyDomainsRemoved << " (of "
+                      << NumStmtsBefore
+                      << ") statements with empty domains \n");
+    TotalEmptyDomainsRemoved[CallNo] += EmptyDomainsRemoved;
   }
 
   /// Remove writes that are overwritten unconditionally later in the same
@@ -587,6 +618,8 @@ class Simplify : public ScopPass {
   /// Print simplification statistics to @p OS.
   void printStatistics(llvm::raw_ostream &OS, int Indent = 0) const {
     OS.indent(Indent) << "Statistics {\n";
+    OS.indent(Indent + 4) << "Empty domains removed: " << EmptyDomainsRemoved
+                          << '\n';
     OS.indent(Indent + 4) << "Overwrites removed: " << OverwritesRemoved
                           << '\n';
     OS.indent(Indent + 4) << "Partial writes coalesced: " << WritesCoalesced
@@ -633,6 +666,9 @@ class Simplify : public ScopPass {
     this->S = &S;
     ScopsProcessed[CallNo]++;
 
+    LLVM_DEBUG(dbgs() << "Removing statements that are never executed...\n");
+    removeEmptyDomainStmts();
+
     LLVM_DEBUG(dbgs() << "Removing partial writes that never happen...\n");
     removeEmptyPartialAccesses();
 
@@ -683,6 +719,7 @@ class Simplify : public ScopPass {
   virtual void releaseMemory() override {
     S = nullptr;
 
+    EmptyDomainsRemoved = 0;
     OverwritesRemoved = 0;
     WritesCoalesced = 0;
     RedundantWritesRemoved = 0;

polly/test/Simplify/func-b320a7.ll

@@ -0,0 +1,82 @@
+; RUN: opt %loadPolly -polly-simplify -polly-optree -analyze < %s | FileCheck %s -match-full-lines
+
+; llvm.org/PR47098
+; Use-after-free by reference to Stmt remaining in InstStmtMap after removing it has been removed by Scop::simplifyScop.
+; Removal happened in -polly-simplify, Reference was using in -polly-optree.
+; Check that Simplify removes the definition of %0 as well of its use.
+
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc19.26.28806"
+
+@"?var_27@@3JA" = external dso_local local_unnamed_addr global i32, align 4
+@"?var_28@@3HA" = external dso_local local_unnamed_addr global i32, align 4
+
+; Function Attrs: nofree norecurse nounwind uwtable
+define dso_local void @"?test@@YAXHEQEAY3M@1BI@BJ@H@Z"(i32 %a, i8 %b, [12 x [2 x [24 x [25 x i32]]]]* nocapture readonly %c) local_unnamed_addr {
+entry:
+  br label %entry.split
+
+entry.split:                                      ; preds = %entry
+  %sext = shl i32 %a, 16
+  %conv4 = ashr exact i32 %sext, 16
+  %sub = add nsw i32 %conv4, -25941
+  %cmp535 = icmp sgt i32 %sext, 1700069376
+  br i1 %cmp535, label %for.cond8.preheader.lr.ph, label %for.cond.cleanup.critedge
+
+for.cond8.preheader.lr.ph:                        ; preds = %entry.split
+  %conv10 = zext i8 %b to i64
+  %sub11 = add nsw i64 %conv10, -129
+  %cmp1232.not = icmp eq i64 %sub11, 0
+  br i1 %cmp1232.not, label %for.cond8.preheader, label %for.cond8.preheader.us
+
+for.cond8.preheader.us:                           ; preds = %for.cond8.preheader.lr.ph, %for.cond8.for.cond.cleanup13_crit_edge.us
+  %e.036.us = phi i16 [ %add.us, %for.cond8.for.cond.cleanup13_crit_edge.us ], [ 0, %for.cond8.preheader.lr.ph ]
+  %idxprom.us = sext i16 %e.036.us to i64
+  br label %for.body14.us
+
+for.body14.us:                                    ; preds = %for.cond8.preheader.us, %for.body14.us
+  %indvars.iv = phi i64 [ 0, %for.cond8.preheader.us ], [ %indvars.iv.next, %for.body14.us ]
+  %arrayidx19.us = getelementptr inbounds [12 x [2 x [24 x [25 x i32]]]], [12 x [2 x [24 x [25 x i32]]]]* %c, i64 6, i64 2, i64 1, i64 %idxprom.us, i64 %indvars.iv
+  %0 = load i32, i32* %arrayidx19.us, align 4, !tbaa !3
+  store i32 0, i32* @"?var_28@@3HA", align 4, !tbaa !3
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond.not = icmp eq i64 %indvars.iv.next, %sub11
+  br i1 %exitcond.not, label %for.cond8.for.cond.cleanup13_crit_edge.us, label %for.body14.us
+
+for.cond8.for.cond.cleanup13_crit_edge.us:        ; preds = %for.body14.us
+  %add.us = add i16 %e.036.us, 4
+  %conv2.us = sext i16 %add.us to i32
+  %cmp5.us = icmp sgt i32 %sub, %conv2.us
+  br i1 %cmp5.us, label %for.cond8.preheader.us, label %for.cond.cleanup.critedge.loopexit38
+
+for.cond.cleanup.critedge.loopexit38:             ; preds = %for.cond8.for.cond.cleanup13_crit_edge.us
+  store i32 %0, i32* @"?var_27@@3JA", align 4, !tbaa !7
+  br label %for.cond.cleanup.critedge
+
+for.cond.cleanup.critedge:                        ; preds = %for.cond8.preheader, %for.cond.cleanup.critedge.loopexit38, %entry.split
+  ret void
+
+for.cond8.preheader:                              ; preds = %for.cond8.preheader.lr.ph, %for.cond8.preheader
+  %e.036 = phi i16 [ %add, %for.cond8.preheader ], [ 0, %for.cond8.preheader.lr.ph ]
+  %add = add i16 %e.036, 4
+  %conv2 = sext i16 %add to i32
+  %cmp5 = icmp sgt i32 %sub, %conv2
+  br i1 %cmp5, label %for.cond8.preheader, label %for.cond.cleanup.critedge
+}
+
+!3 = !{!4, !4, i64 0}
+!4 = !{!"int", !5, i64 0}
+!5 = !{!"omnipotent char", !6, i64 0}
+!6 = !{!"Simple C++ TBAA"}
+!7 = !{!8, !8, i64 0}
+!8 = !{!"long", !5, i64 0}
+
+
+; CHECK: Statistics {
+; CHECK:     Empty domains removed: 2
+; CHECK: }
+
+; CHECK: After accesses {
+; CHECK-NOT: Stmt_for_body14_us_a
+; CHECK-NOT: Stmt_for_body14_us
+; CHECK: }