[PATCH] D146466: [clang] diagnose function fallthrough

Nick Desaulniers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 23 11:46:07 PDT 2023


nickdesaulniers added a comment.

In D146466#4217363 <https://reviews.llvm.org/D146466#4217363>, @efriedma wrote:

> There are limits to how much we can do by just changing the code clang generates... but the two particular cases you mention probably could be "fixed" by messing with the IR generated by clang.  Sure, that probably makes sense to pursue.  (If you're going to pick an arbitrary value, zero is probably a better choice than "freeze poison".)

I don't think that could be a clang codegen of IR change.  What I had in mind is closer to:

  diff --git a/llvm/lib/Transforms/Utils/LoopUnroll.cpp b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
  index 916c02692823..aa1f288b51ee 100644
  --- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp
  +++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
  @@ -236,9 +236,17 @@ void llvm::simplifyLoopAfterUnroll(Loop *L, bool SimplifyIVs, LoopInfo *LI,
     SmallVector<WeakTrackingVH, 16> DeadInsts;
     for (BasicBlock *BB : L->getBlocks()) {
       for (Instruction &Inst : llvm::make_early_inc_range(*BB)) {
  -      if (Value *V = simplifyInstruction(&Inst, {DL, nullptr, DT, AC}))
  -        if (LI->replacementPreservesLCSSAForm(&Inst, V))
  +      if (Value *V = simplifyInstruction(&Inst, {DL, nullptr, DT, AC})) {
  +        // TODO: check if sanitizer is enabled
  +        if (isa<PoisonValue>(V) && isa<LoadInst>(Inst)) {
  +          errs() << "XXX: found poison (replacing a load)!\n";
  +          FreezeInst *FI = new FreezeInst(V);
  +          FI->insertInto(BB, Inst.getIterator());
  +          if (LI->replacementPreservesLCSSAForm(FI, V))
  +            Inst.replaceAllUsesWith(FI);
  +        } else if (LI->replacementPreservesLCSSAForm(&Inst, V))
             Inst.replaceAllUsesWith(V);
  +      }
         if (isInstructionTriviallyDead(&Inst))
           DeadInsts.emplace_back(&Inst);
       }

Which seems to do exactly what I want (minus only doing so when `-fsanitize=array-bounds` is enabled; not sure we leave such breadcrumbs behind in IR; still looking to see if we can tell in LLVM when specific sanitizers are enabled)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146466/new/

https://reviews.llvm.org/D146466



More information about the cfe-commits mailing list