[llvm-commits] [llvm] r153465 - /llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp

[Duncan Sands]

Hi Nadav,

> PR12357: The pointer was used before it was checked.

> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Mon Mar 26 15:39:18 2012
> @@ -915,11 +915,13 @@
>
>     // Handle gep(bitcast x) and gep(gep x, 0, 0, 0).
>     Value *StrippedPtr = PtrOp->stripPointerCasts();
> -  PointerType *StrippedPtrTy = dyn_cast<PointerType>(StrippedPtr->getType());
> +
>     // We do not handle pointer-vector geps here
>     if (!StrippedPtr)
>       return 0;
>
> +  PointerType *StrippedPtrTy = dyn_cast<PointerType>(StrippedPtr->getType());
> +
>     if (StrippedPtr != PtrOp&&
>       StrippedPtrTy->getAddressSpace() == GEP.getPointerAddressSpace()) {

are you sure the mistake wasn't that

     if (!StrippedPtr)
       return 0;

should have been

     if (!StrippedPtrTy)
       return 0;

?

After all, StrippedPtr cannot ever be null, so the test is pointless for it.
However the dynamic cast generating StrippedPtrTy can result in a null value,
which is not checked for.  Also, if StrippedPtrTy is null then I think that
corresponds to exactly the situation of a GEP on a vector of pointers mentioned
in the comment on the test.

Ciao, Duncan.