fix: rearrange so assert(false) not on return path

Author: priyasiddharth

lib/seadsa/Graph.cc

@@ -219,7 +219,8 @@ void Node::addAccessedType(unsigned off, llvm::Type *type) {
         tmp.push_back({o + i * sz, elementTy});
 
       workList.insert(workList.end(), tmp.rbegin(), tmp.rend());
-    } else if (const FixedVectorType *vty = dyn_cast<const FixedVectorType>(t)) {
+    } else if (const FixedVectorType *vty =
+                   dyn_cast<const FixedVectorType>(t)) {
       uint64_t sz = vty->getElementType()->getPrimitiveSizeInBits() / 8;
       WorkList tmp;
       const size_t numElements(vty->getNumElements());
@@ -398,9 +399,8 @@ const Cell &Node::getLink(Field _f) const {
   assert(!FieldType::IsNotTypeAware());
   assert(!f.hasOmniType());
   Field omni = f.mkOmniField();
-  if (m_links.count(omni)) return *m_links.at(omni);
-
-  assert(false); // unreachable
+  assert(m_links.count(omni));
+  return *m_links.at(omni);
 }
 
 void Node::setLink(const Field _f, const Cell &c) {
@@ -463,7 +463,8 @@ void Node::addLink(Field _f, const Cell &c) {
   m_links = std::move(new_links);
 
   // -- recreate links that have been removed by adding them through a cell
-  // -- pointing to the current node. The cell takes care of resolving forwarding
+  // -- pointing to the current node. The cell takes care of resolving
+  // forwarding
   Cell cc(*this, 0);
   for (auto &kv : saved_links) {
     cc.addLink(kv.first, *kv.second);

lib/seadsa/RemovePtrToInt.cc

@@ -168,13 +168,14 @@ static bool visitStoreInst(StoreInst *SI, Function &F, const DataLayout &DL,
 class PointerPromoter : public InstVisitor<PointerPromoter, Value *> {
   Type *m_ty;
   SmallPtrSetImpl<Instruction *> &m_toRemove;
-  DenseMap<Value*, Value*> &m_toReplace;
+  DenseMap<Value *, Value *> &m_toReplace;
   // -- to break cycles in PHINodes
   SmallPtrSet<Instruction *, 16> m_visited;
 
 public:
-  PointerPromoter(SmallPtrSetImpl<Instruction *> &toRemove, DenseMap<Value*, Value*> &toReplace)
-    : m_ty(nullptr), m_toRemove(toRemove), m_toReplace(toReplace) {}
+  PointerPromoter(SmallPtrSetImpl<Instruction *> &toRemove,
+                  DenseMap<Value *, Value *> &toReplace)
+      : m_ty(nullptr), m_toRemove(toRemove), m_toReplace(toReplace) {}
 
   Value *visitInstruction(Instruction &I) { return nullptr; }
 
@@ -227,8 +228,9 @@ class PointerPromoter : public InstVisitor<PointerPromoter, Value *> {
     IRBuilder<> IRB(&I);
 
     ptr = IRB.CreateBitCast(ptr, IRB.getInt8PtrTy());
-    auto *gep = IRB.CreateGEP(ptr->getType()->getScalarType()->getPointerElementType(),
-                              ptr, {I.getOperand(1)});
+    auto *gep =
+        IRB.CreateGEP(ptr->getType()->getScalarType()->getPointerElementType(),
+                      ptr, I.getOperand(1));
     return IRB.CreateBitCast(gep, m_ty);
   }
 
@@ -270,7 +272,7 @@ class PointerPromoter : public InstVisitor<PointerPromoter, Value *> {
         m_toRemove.insert(SI);
       } else if (isa<IntToPtrInst>(u)) {
         /* skip  */;
-      } else  {
+      } else {
         // -- if there is an interesting user, schedule it to be replaced
         if (!p2i) {
           IRB.SetInsertPoint(&I);
@@ -303,12 +305,11 @@ class PointerPromoter : public InstVisitor<PointerPromoter, Value *> {
   }
 };
 
-static bool
-visitIntToPtrInst(IntToPtrInst *I2P, Function &F, const DataLayout &DL,
-                  DominatorTree &DT,
-                  SmallDenseMap<PHINode *, PHINode *> &NewPhis,
-                  SmallPtrSetImpl<Instruction *> &MaybeUnusedInsts,
-                  DenseMap<Value*, Value*> &RenameMap) {
+static bool visitIntToPtrInst(IntToPtrInst *I2P, Function &F,
+                              const DataLayout &DL, DominatorTree &DT,
+                              SmallDenseMap<PHINode *, PHINode *> &NewPhis,
+                              SmallPtrSetImpl<Instruction *> &MaybeUnusedInsts,
+                              DenseMap<Value *, Value *> &RenameMap) {
   assert(I2P);
 
   auto *IntVal = I2P->getOperand(0);
@@ -413,7 +414,7 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
   auto &DT = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
   SmallPtrSet<StoreInst *, 8> StoresToErase;
   SmallPtrSet<Instruction *, 16> MaybeUnusedInsts;
-  DenseMap<Value*, Value*> RenameMap;
+  DenseMap<Value *, Value *> RenameMap;
   SmallDenseMap<PHINode *, PHINode *> NewPhis;
 
   for (auto &BB : F) {
@@ -429,7 +430,8 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
       }
 
       if (auto *I2P = dyn_cast<IntToPtrInst>(&I)) {
-        Changed |= visitIntToPtrInst(I2P, F, DL, DT, NewPhis, MaybeUnusedInsts, RenameMap);
+        Changed |= visitIntToPtrInst(I2P, F, DL, DT, NewPhis, MaybeUnusedInsts,
+                                     RenameMap);
         continue;
       }
     }
@@ -442,7 +444,7 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
       llvm::make_filter_range(MaybeUnusedInsts, [](Instruction *I) {
         return isInstructionTriviallyDead(I);
       }));
-  
+
   // TODO: RecursivelyDeleteTriviallyDeadInstructions takes a vector of
   // WeakTrackingVH since LLVM 11, which implies that `MaybeUnusedInsts` should
   // be a vector of WeakTrackingVH as well. See comment:
@@ -451,11 +453,10 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
   // removed before this call. So it should be safe to keep its original type
   // and map it to WeakTrackingVH as the following code does.
   SmallVector<WeakTrackingVH, 16> TriviallyDeadHandles;
-  for (auto i : TriviallyDeadInstructions)  
-      TriviallyDeadHandles.emplace_back(WeakTrackingVH(i));
-  
-  RecursivelyDeleteTriviallyDeadInstructions(TriviallyDeadHandles);
+  for (auto i : TriviallyDeadInstructions)
+    TriviallyDeadHandles.emplace_back(WeakTrackingVH(i));
 
+  RecursivelyDeleteTriviallyDeadInstructions(TriviallyDeadHandles);
 
   for (auto kv : RenameMap) {
     kv.first->replaceAllUsesWith(kv.second);
@@ -464,9 +465,7 @@ bool RemovePtrToInt::runOnFunction(Function &F) {
   DOG(llvm::errs() << "\n~~~~~~~ End of RP2I on " << F.getName() << " ~~~~~ \n";
       llvm::errs().flush());
 
-  if (Changed) {
-    assert(!llvm::verifyFunction(F, &llvm::errs()));
-  }
+  if (Changed) { assert(!llvm::verifyFunction(F, &llvm::errs())); }
 
   // llvm::errs() << F << "\n";
   return Changed;