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

[David Blaikie]

On Fri, Jun 19, 2015 at 11:52 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> 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.
>

What I mean is we have a way of making them related, through a template
called simplify_type. Here's the version for QualType to allow QualTypes to
be casted to specific clang::Types:
http://clang.llvm.org/doxygen/structllvm_1_1simplify__type_3_01_1_1clang_1_1QualType_01_4.html


I think it /might/ work here, but I don't know/recall the specifics -
perhaps it doesn't fit here for some reason.


>
> >
> >            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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150620/89e3958f/attachment.html>