spirv-opt: Handle ID overflow in AggressiveDCEPass (#6366)

Refactored AggressiveDCEPass to return Pass::Status instead of bool
in several key functions. This allows the pass to gracefully fail
and propagate the error when the module's ID bound is reached,
preventing crashes due to null pointers or zero IDs.

Part of #6260

Author: s-perron

source/opt/aggressive_dead_code_elim_pass.cpp

@@ -273,29 +273,30 @@ void AggressiveDCEPass::AddBreaksAndContinuesToWorklist(
   });
 }
 
-bool AggressiveDCEPass::AggressiveDCE(Function* func) {
-  if (func->IsDeclaration()) return false;
+Pass::Status AggressiveDCEPass::AggressiveDCE(Function* func) {
+  if (func->IsDeclaration()) return Pass::Status::SuccessWithoutChange;
   std::list<BasicBlock*> structured_order;
   cfg()->ComputeStructuredOrder(func, &*func->begin(), &structured_order);
   live_local_vars_.clear();
   InitializeWorkList(func, structured_order);
   ProcessWorkList(func);
-  ProcessDebugInformation(structured_order);
+  if (ProcessDebugInformation(structured_order) == Pass::Status::Failure)
+    return Pass::Status::Failure;
   ProcessWorkList(func);
   return KillDeadInstructions(func, structured_order);
 }
 
-void AggressiveDCEPass::ProcessDebugInformation(
+Pass::Status AggressiveDCEPass::ProcessDebugInformation(
     std::list<BasicBlock*>& structured_order) {
   for (auto bi = structured_order.begin(); bi != structured_order.end(); bi++) {
-    (*bi)->ForEachInst([this](Instruction* inst) {
+    bool succeeded = (*bi)->WhileEachInst([this](Instruction* inst) {
       // DebugDeclare is not dead. It must be converted to DebugValue in a
       // later pass
       if (inst->IsNonSemanticInstruction() &&
           inst->GetShader100DebugOpcode() ==
               NonSemanticShaderDebugInfo100DebugDeclare) {
         AddToWorklist(inst);
-        return;
+        return true;
       }
 
       // If the Value of a DebugValue is killed, set Value operand to Undef
@@ -307,6 +308,9 @@ void AggressiveDCEPass::ProcessDebugInformation(
         if (!live_insts_.Set(def->unique_id())) {
           AddToWorklist(inst);
           uint32_t undef_id = Type2Undef(def->type_id());
+          if (undef_id == 0) {
+            return false;
+          }
           inst->SetInOperand(kDebugValueValue, {undef_id});
           context()->get_def_use_mgr()->UpdateDefUse(inst);
           id = inst->GetSingleWordInOperand(kDebugValueLocalVariable);
@@ -318,14 +322,18 @@ void AggressiveDCEPass::ProcessDebugInformation(
           auto expression = get_def_use_mgr()->GetDef(id);
           AddToWorklist(expression);
           context()->get_def_use_mgr()->UpdateDefUse(expression);
-          return;
+          return true;
         }
       }
+      return true;
     });
+
+    if (!succeeded) return Pass::Status::Failure;
   }
+  return Pass::Status::SuccessWithoutChange;
 }
 
-bool AggressiveDCEPass::KillDeadInstructions(
+Pass::Status AggressiveDCEPass::KillDeadInstructions(
     const Function* func, std::list<BasicBlock*>& structured_order) {
   bool modified = false;
   for (auto bi = structured_order.begin(); bi != structured_order.end();) {
@@ -360,6 +368,9 @@ bool AggressiveDCEPass::KillDeadInstructions(
           // Find an undef for the return value and make sure it gets kept by
           // the pass.
           auto undef_id = Type2Undef(func->type_id());
+          if (undef_id == 0) {
+            return Pass::Status::Failure;
+          }
           auto undef = get_def_use_mgr()->GetDef(undef_id);
           live_insts_.Set(undef->unique_id());
           merge_terminator->SetOpcode(spv::Op::OpReturnValue);
@@ -378,7 +389,8 @@ bool AggressiveDCEPass::KillDeadInstructions(
       ++bi;
     }
   }
-  return modified;
+  return modified ? Pass::Status::SuccessWithChange
+                  : Pass::Status::SuccessWithoutChange;
 }
 
 void AggressiveDCEPass::ProcessWorkList(Function* func) {
@@ -638,7 +650,7 @@ void AggressiveDCEPass::InitializeWorkList(
   }
 }
 
-void AggressiveDCEPass::InitializeModuleScopeLiveInstructions() {
+Pass::Status AggressiveDCEPass::InitializeModuleScopeLiveInstructions() {
   // Keep all execution modes.
   for (auto& exec : get_module()->execution_modes()) {
     AddToWorklist(&exec);
@@ -713,6 +725,9 @@ void AggressiveDCEPass::InitializeModuleScopeLiveInstructions() {
   }
   if (debug_global_seen) {
     auto dbg_none = context()->get_debug_info_mgr()->GetDebugInfoNone();
+    if (dbg_none == nullptr) {
+      return Pass::Status::Failure;
+    }
     AddToWorklist(dbg_none);
   }
 
@@ -726,6 +741,7 @@ void AggressiveDCEPass::InitializeModuleScopeLiveInstructions() {
       AddToWorklist(&dbg);
     }
   }
+  return Pass::Status::SuccessWithoutChange;
 }
 
 Pass::Status AggressiveDCEPass::ProcessImpl() {
@@ -753,15 +769,23 @@ Pass::Status AggressiveDCEPass::ProcessImpl() {
   // Eliminate Dead functions.
   bool modified = EliminateDeadFunctions();
 
-  InitializeModuleScopeLiveInstructions();
+  if (InitializeModuleScopeLiveInstructions() == Pass::Status::Failure) {
+    return Pass::Status::Failure;
+  }
 
   // Run |AggressiveDCE| on the remaining functions.  The order does not matter,
   // since |AggressiveDCE| is intra-procedural.  This can mean that function
   // will become dead if all function call to them are removed.  These dead
   // function will still be in the module after this pass.  We expect this to be
   // rare.
   for (Function& fp : *context()->module()) {
-    modified |= AggressiveDCE(&fp);
+    Pass::Status function_status = AggressiveDCE(&fp);
+    if (function_status == Pass::Status::Failure) {
+      return Pass::Status::Failure;
+    }
+    if (function_status == Pass::Status::SuccessWithChange) {
+      modified = true;
+    }
   }
 
   // If the decoration manager is kept live then the context will try to keep it

source/opt/aggressive_dead_code_elim_pass.h

@@ -85,8 +85,10 @@ class AggressiveDCEPass : public MemPass {
   }
 
   // Adds entry points, execution modes and workgroup size decorations to the
-  // worklist for processing with the first function.
-  void InitializeModuleScopeLiveInstructions();
+  // worklist for processing with the first function. Returns
+  // Pass::Status::Failure if it could not create a required debug instruction.
+  // Returns Pass::Status::SuccessWithoutChange otherwise.
+  Pass::Status InitializeModuleScopeLiveInstructions();
 
   // Add |inst| to worklist_ and live_insts_.
   void AddToWorklist(Instruction* inst) {
@@ -136,7 +138,7 @@ class AggressiveDCEPass : public MemPass {
   // existing control structures will remain. This can leave not-insignificant
   // sequences of ultimately useless code.
   // TODO(): Remove useless control constructs.
-  bool AggressiveDCE(Function* func);
+  Pass::Status AggressiveDCE(Function* func);
 
   Pass::Status ProcessImpl();
 
@@ -154,11 +156,17 @@ class AggressiveDCEPass : public MemPass {
   // marked as live in the work list. DebugDeclare's are marked live now, and
   // DebugValue Value operands are set to OpUndef.  The work list will be empty
   // at the end.
-  void ProcessDebugInformation(std::list<BasicBlock*>& structured_order);
+  // Returns Pass::Status::Failure if it could not create an OpUndef.
+  // Otherwise, returns Pass::Status::SuccessWithChange if it made changes,
+  Pass::Status ProcessDebugInformation(
+      std::list<BasicBlock*>& structured_order);
 
   // Kills any instructions in |func| that have not been marked as live.
-  bool KillDeadInstructions(const Function* func,
-                            std::list<BasicBlock*>& structured_order);
+  // Returns Pass::Status::Failure if it could not create an OpUndef.
+  // Otherwise, returns Pass::Status::SuccessWithChange if it made changes,
+  // and Pass::Status::SuccessWithoutChange otherwise.
+  Pass::Status KillDeadInstructions(const Function* func,
+                                    std::list<BasicBlock*>& structured_order);
 
   // Adds the instructions that define the operands of |inst| to the work list.
   void AddOperandsToWorkList(const Instruction* inst);

source/opt/debug_info_manager.cpp

@@ -390,6 +390,7 @@ Instruction* DebugInfoManager::GetDebugInfoNone() {
   if (debug_info_none_inst_ != nullptr) return debug_info_none_inst_;
 
   uint32_t result_id = context()->TakeNextId();
+  if (result_id == 0) return nullptr;
   std::unique_ptr<Instruction> dbg_info_none_inst(new Instruction(
       context(), spv::Op::OpExtInst, context()->get_type_mgr()->GetVoidTypeId(),
       result_id,