[llvm] r240214 - IndVarSimplify: Avoid UB from binding a reference to a null pointer

[Justin Bogner]

David Blaikie <dblaikie at gmail.com> writes:
> On Fri, Jun 19, 2015 at 11:24 PM, Justin Bogner <mail at justinbogner.com>
> wrote:
>
>     Author: bogner
>     Date: Sat Jun 20 01:24:05 2015
>     New Revision: 240214
>    
>     URL: http://llvm.org/viewvc/llvm-project?rev=240214&view=rev
>     Log:
>     IndVarSimplify: Avoid UB from binding a reference to a null pointer
>    
>     Calling operator* on a WeakVH whose Value is null hits undefined
>     behaviour, since we bind the value to a reference. Instead, go through
>     `operator Value*` so that we work with the pointer itself.
>    
>     Found by ubsan.
>    
>     Modified:
>         llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
>    
>     Modified: llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
>     URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/
>     Scalar/IndVarSimplify.cpp?rev=240214&r1=240213&r2=240214&view=diff
>     ========================================================================
>     ======
>     --- llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp (original)
>     +++ llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp Sat Jun 20
>     01:24:05 2015
>     @@ -2013,10 +2013,11 @@ bool IndVarSimplify::runOnLoop(Loop *L,
>    
>        // Now that we're done iterating through lists, clean up any
>     instructions
>        // which are now dead.
>     -  while (!DeadInsts.empty())
>     -    if (Instruction *Inst =
>     -          dyn_cast_or_null<Instruction>(&*DeadInsts.pop_back_val()))
>     +  while (!DeadInsts.empty()) {
>     +    Value *V = static_cast<Value *>(DeadInsts.pop_back_val());
>     +    if (Instruction *Inst = dyn_cast_or_null<Instruction>(V))
>
> Hmm - I think there's some fancy machinery in the llvm cast stuff to allow
> us to map through from different types (so we could say that casting a
> WeakVH retrieves the Value* first), maybe... (so you could just
> dyn_cast_or_null<Value*>(DeadInsts.pop_back_val()) directly)

I don't really understand what you're going for here - we need to cast
twice. We have a WeakVH, which we can get a Value* out of, then we
dyn_cast_or_null that to an Instruction*. There's no need to dyn_cast to
a Value* (we want the conversion operator), and obviously WeakVH can't
be cast to Instruction* directly, since they're unrelated types.

>
>            RecursivelyDeleteTriviallyDeadInstructions(Inst, TLI);
>     +  }
>    
>        // The Rewriter may not be used from this point on.
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at cs.uiuc.edu
>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits