Dereferencing NULL pointer in IndVarSimplify.cpp?

Hi,

Here is the code in IndVarSimplify.cpp.

    SmallVector<WeakVH, 16> DeadInsts;

  while (!DeadInsts.empty())
    if (Instruction *Inst =
          dyn_cast_or_null<Instruction>(&*DeadInsts.pop_back_val()))
      RecursivelyDeleteTriviallyDeadInstructions(Inst, TLI);

Since DeadInsts.pop_back_val() is WeakVH which could hold a NULL
pointer, the expression, &*DeadInsts.pop_back_val(), could be &*NULL.
Then NULL pointer is dereferenced here.

I wrote a small test case and it works just fine. But is this a
well-defined behavior in the standard?

Thanks,
Liang

Hi,

Here is the code in IndVarSimplify.cpp.

   SmallVector<WeakVH, 16> DeadInsts;

while (!DeadInsts.empty())
   if (Instruction *Inst =
         dyn_cast_or_null<Instruction>(&*DeadInsts.pop_back_val()))
     RecursivelyDeleteTriviallyDeadInstructions(Inst, TLI);

Since DeadInsts.pop_back_val() is WeakVH which could hold a NULL
pointer, the expression, &*DeadInsts.pop_back_val(), could be &*NULL.
Then NULL pointer is dereferenced here.

I wrote a small test case and it works just fine. But is this a
well-defined behavior in the standard?

Try clang-dev or a c++ list for questions about the standard.

I think it would have been nicer to write (Value*)DeadInsts.pop_back_val()
-Andy

Hi,

Here is the code in IndVarSimplify.cpp.

  SmallVector<WeakVH, 16> DeadInsts;

while (!DeadInsts.empty())
  if (Instruction *Inst =
        dyn_cast_or_null<Instruction>(&*DeadInsts.pop_back_val()))
    RecursivelyDeleteTriviallyDeadInstructions(Inst, TLI);

Since DeadInsts.pop_back_val() is WeakVH which could hold a NULL
pointer, the expression, &*DeadInsts.pop_back_val(), could be &*NULL.
Then NULL pointer is dereferenced here.

I wrote a small test case and it works just fine. But is this a
well-defined behavior in the standard?

This is UB, but `&*nullptr` often "works" so I'm not surprised you
couldn't expose it with a testcase.

Try clang-dev or a c++ list for questions about the standard.

I think it would have been nicer to write (Value*)DeadInsts.pop_back_val()
-Andy

+1 (or `static_cast<Value *>(DeadInsts.pop_back_val())`).

Hi,

Here is the code in IndVarSimplify.cpp.

  SmallVector<WeakVH, 16> DeadInsts;

while (!DeadInsts.empty())
  if (Instruction *Inst =
        dyn_cast_or_null<Instruction>(&*DeadInsts.pop_back_val()))
    RecursivelyDeleteTriviallyDeadInstructions(Inst, TLI);

Since DeadInsts.pop_back_val() is WeakVH which could hold a NULL
pointer, the expression, &*DeadInsts.pop_back_val(), could be &*NULL.
Then NULL pointer is dereferenced here.

I wrote a small test case and it works just fine. But is this a
well-defined behavior in the standard?

This is UB, but `&*nullptr` often "works" so I'm not surprised you
couldn't expose it with a testcase.

Thanks, Duncan and Andrew. This confused (and surprised) me quite a
bit actually.