[llvm] e39c16b - Ignore modified attribute list if it yields invalid IR

[Brian Gesiak via llvm-commits]

Author: Manish Kausik H
Date: 2023-07-11T00:32:06-04:00
New Revision: e39c16b1d98f1b1abc0ea4c5a4820798e4a22b64

URL: https://github.com/llvm/llvm-project/commit/e39c16b1d98f1b1abc0ea4c5a4820798e4a22b64
DIFF: https://github.com/llvm/llvm-project/commit/e39c16b1d98f1b1abc0ea4c5a4820798e4a22b64.diff

LOG: Ignore modified attribute list if it yields invalid IR

If modified attribute list is invalid, reverting the change is a
low-cost maintainence solution as compared to examples like
[this](https://github.com/llvm/llvm-project/blob/main/llvm/tools/bugpoint/CrashDebugger.cpp#L368).
This will ensure that the ListReducer maintains the sanctity of any
new attribute dependencies added in the future/already present.

Reviewed By: modocache

Differential Revision: https://reviews.llvm.org/D154348

Added: 
    

Modified: 
    llvm/tools/bugpoint/CrashDebugger.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/tools/bugpoint/CrashDebugger.cpp b/llvm/tools/bugpoint/CrashDebugger.cpp
index c90e1afd8ca448..0ca8fa28c4af03 100644
--- a/llvm/tools/bugpoint/CrashDebugger.cpp
+++ b/llvm/tools/bugpoint/CrashDebugger.cpp
@@ -70,6 +70,18 @@ cl::opt<bool> VerboseErrors("verbose-errors",
                             cl::init(false));
 }
 
+static bool isValidModule(std::unique_ptr<Module> &M,
+                          bool ExitOnFailure = true) {
+  if (!llvm::verifyModule(*M.get(), &llvm::errs()))
+    return true;
+
+  if (ExitOnFailure) {
+    llvm::errs() << "verify failed!\n";
+    exit(1);
+  }
+  return false;
+}
+
 namespace llvm {
 class ReducePassList : public ListReducer<std::string> {
   BugDriver &BD;
@@ -368,6 +380,10 @@ bool ReduceCrashingFunctionAttributes::TestFuncAttrs(
   if (F->hasFnAttribute(Attribute::OptimizeNone))
     F->addFnAttr(Attribute::NoInline);
 
+  // If modifying the attribute list leads to invalid IR, revert the change
+  if (!isValidModule(M, /*ExitOnFailure=*/false))
+    return false;
+
   // Try running on the hacked up program...
   if (TestFn(BD, M.get())) {
     BD.setNewProgram(std::move(M)); // It crashed, keep the trimmed version...
@@ -510,14 +526,7 @@ bool ReduceCrashingBlocks::TestBlocks(std::vector<const BasicBlock *> &BBs) {
     ToProcess.clear();
   }
   // Verify we didn't break anything
-  std::vector<std::string> Passes;
-  Passes.push_back("verify");
-  std::unique_ptr<Module> New = BD.runPassesOn(M.get(), Passes);
-  if (!New) {
-    errs() << "verify failed!\n";
-    exit(1);
-  }
-  M = std::move(New);
+  isValidModule(M);
 
   // Try running on the hacked up program...
   if (TestFn(BD, M.get())) {
@@ -618,14 +627,7 @@ bool ReduceCrashingConditionals::TestBlocks(
     ToProcess.clear();
   }
   // Verify we didn't break anything
-  std::vector<std::string> Passes;
-  Passes.push_back("verify");
-  std::unique_ptr<Module> New = BD.runPassesOn(M.get(), Passes);
-  if (!New) {
-    errs() << "verify failed!\n";
-    exit(1);
-  }
-  M = std::move(New);
+  isValidModule(M);
 
   // Try running on the hacked up program...
   if (TestFn(BD, M.get())) {
@@ -711,14 +713,7 @@ bool ReduceSimplifyCFG::TestBlocks(std::vector<const BasicBlock *> &BBs) {
       simplifyCFG(&*BBIt++, TTI);
     }
   // Verify we didn't break anything
-  std::vector<std::string> Passes;
-  Passes.push_back("verify");
-  std::unique_ptr<Module> New = BD.runPassesOn(M.get(), Passes);
-  if (!New) {
-    errs() << "verify failed!\n";
-    exit(1);
-  }
-  M = std::move(New);
+  isValidModule(M);
 
   // Try running on the hacked up program...
   if (TestFn(BD, M.get())) {
@@ -797,9 +792,7 @@ bool ReduceCrashingInstructions::TestInsts(
       }
 
   // Verify that this is still valid.
-  legacy::PassManager Passes;
-  Passes.add(createVerifierPass(/*FatalErrors=*/false));
-  Passes.run(*M);
+  isValidModule(M, /*ExitOnFailure=*/false);
 
   // Try running on the hacked up program...
   if (TestFn(BD, M.get())) {
@@ -869,9 +862,7 @@ bool ReduceCrashingMetadata::TestInsts(std::vector<Instruction *> &Insts) {
     }
 
   // Verify that this is still valid.
-  legacy::PassManager Passes;
-  Passes.add(createVerifierPass(/*FatalErrors=*/false));
-  Passes.run(*M);
+  isValidModule(M, /*ExitOnFailure=*/false);
 
   // Try running on the hacked up program...
   if (TestFn(BD, M.get())) {
@@ -944,9 +935,7 @@ bool ReduceCrashingNamedMD::TestNamedMDs(std::vector<std::string> &NamedMDs) {
     NamedMD->eraseFromParent();
 
   // Verify that this is still valid.
-  legacy::PassManager Passes;
-  Passes.add(createVerifierPass(/*FatalErrors=*/false));
-  Passes.run(*M);
+  isValidModule(M, /*ExitOnFailure=*/false);
 
   // Try running on the hacked up program...
   if (TestFn(BD, M.get())) {
@@ -1009,9 +998,7 @@ bool ReduceCrashingNamedMDOps::TestNamedMDOps(
   }
 
   // Verify that this is still valid.
-  legacy::PassManager Passes;
-  Passes.add(createVerifierPass(/*FatalErrors=*/false));
-  Passes.run(*M);
+  isValidModule(M, /*ExitOnFailure=*/false);
 
   // Try running on the hacked up program...
   if (TestFn(BD, M.get())) {