[PATCH] D46738: [DebugInfo] Fix PR37395.

[Hsiangkai Wang via Phabricator via llvm-commits]

HsiangKai added a comment.

In https://reviews.llvm.org/D46738#1117785, @aprantl wrote:

> In https://reviews.llvm.org/D46738#1117258, @HsiangKai wrote:
>
> > In https://reviews.llvm.org/D46738#1114890, @aprantl wrote:
> >
> > > My general problem with this is that a debug info intrinsic with a null operand is usually indicative of a bug in a transformation pass that forgot to update the debug intrinsic after modifying the value it was pointing to. If the value goes away then the debug intrinsic should use an undef value (to end the range started by an earlier dbg intrinsic describing the same variable) or be removed entirely (if all instructions dealing with that variable have been removed as dead code). I don't want to make it easier to for transformation passes to not update the debug info correctly. If all you need is that DILabels are handled correctly, then this patch should just specifically enable that case.
> >
> >
> > Debug info intrinsic with a null operand could not indicate a bug in transformation passes, in getVariableLocation() at least. It will return nullptr in getVariableLocation() and the caller just skip the intrinsic.
> >
> > Following is the code fragment from llvm::insertDebugValuesForPHIs()
> >
> >   ValueToValueMapTy DbgValueMap;
> >   for (auto &I : *BB) {
> >     if (auto DbgII = dyn_cast<DbgInfoIntrinsic>(&I)) {
> >       if (auto *Loc = dyn_cast_or_null<PHINode>(DbgII->getVariableLocation()))
> >         DbgValueMap.insert({Loc, DbgII});
> >     }
> >   }
> >
> >
> > You could see that when getVariableLocation() return nullptr, it just skips the intrinsic.
> >
> > The assertion is triggered when the intrinsic has operands and the first operand is not value, not has a null operand.
>
>
> Do you think you could post an example were this assertion incorrectly fires for a dbg,value intrinsic? I would really like to understand this case better.


I think this assertion will not be incorrectly fired for dbg.value. dbg.value always has its location address as its first operand.
In addition, the argument of getVariableLocation(), AllowNullOp, has default value as true. So, it seems to allow null operand for dbg.value.

  Value *Op = getArgOperand(0);
  if (AllowNullOp && !Op)
    return nullptr;

If the first operand of dbg.value is not location address and not nullptr, the assertion will be triggered.


Repository:
  rL LLVM

https://reviews.llvm.org/D46738