Assertion `cast<DIExpression>(Expr)->isValid() && "not an expression"' for entry value for indirect parameter

[Quuxplusone]

Bugzilla Link	PR44275
Status	NEW
Importance	P enhancement
Reported by	David Stenberg (david.stenberg@ericsson.com)
Reported on	2019-12-11 06:10:53 -0800
Last modified on	2019-12-13 02:29:11 -0800
Version	trunk
Hardware	PC Windows NT
CC	david.stenberg@ericsson.com, djordje.todorovic@rt-rk.com, jdevlieghere@apple.com, jeremy.morse.llvm@gmail.com, keith.walker@arm.com, llvm-bugs@lists.llvm.org, paul_robinson@playstation.sony.com
Fixed by commit(s)
Attachments
Blocks	PR44116
Blocked by
See also

Reproduced on trunk (987e7323fb53f968d5878483610fcf2319cdde86).

When compiling the following file:

    struct a {
      a(a &) {}
    };
    class b {
      a c;
    };
    class C {
    protected:
      C(int, int, b);
    };
    class d : C {
      d(int &, b);
    };
    int e;
    d::d(int &f, b g) : C(f, e, g) {}

using:

    $ clang++ -Xclang -femit-debug-entry-values foo.cpp -O2 -g -c

an assertion fails:

    clang-10: /path/to/monorepo/llvm/lib/CodeGen/MachineInstr.cpp:2019: llvm::MachineInstrBuilder llvm::BuildMI(llvm::MachineFunction&, const llvm::DebugLoc&, const llvm::MCInstrDesc&, bool, llvm::Register, const llvm::MDNode*, const llvm::MDNode*): Assertion `cast<DIExpression>(Expr)->isValid() && "not an expression"' failed.
    Stack dump:
    0.  Program arguments: /path/to/monorepo/build/bin/clang-10 -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -disable-free -main-file-name foo.cpp -mrelocation-model static -mthread-model posix -mframe-pointer=none -fmath-errno -fno-rounding-math -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -dwarf-column-info -fno-split-dwarf-inlining -debug-info-kind=limited -dwarf-version=4 -debugger-tuning=gdb -resource-dir /path/to/monorepo/build/lib/clang/10.0.0 -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++ -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/x86_64-linux-gnu -internal-isystem /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/backward -internal-isystem /usr/local/include -internal-isystem /path/to/monorepo/build/lib/clang/10.0.0/include -internal-externc-isystem /usr/include/x86_64-linux-gnu -internal-externc-isystem /include -internal-externc-isystem /usr/include -O2 -fdeprecated-macro -fdebug-compilation-dir /path/to/monorepo -ferror-limit 19 -fmessage-length 0 -fgnuc-version=4.2.1 -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -vectorize-loops -vectorize-slp -femit-debug-entry-values -faddrsig -o foo.o -x c++ foo.cpp
    1.  <eof> parser at end of file
    2.  Code generation
    3.  Running pass 'Function Pass Manager' on module 'foo.cpp'.
    4.  Running pass 'Live DEBUG_VALUE analysis' on function '@_ZN1dC2ERi1b'

This happens we are creating an entry value for:

    DBG_VALUE $rdx, $noreg, !"g", !DIExpression(DW_OP_deref), debug-location !42; foo.cpp:15:16 line no:15

Currently DIExpression::isValid() assumes the following for entry values:

      return I->get() == expr_op_begin()->get() && I->getArg(0) == 1 &&
             getNumElements() == 2;

It is the (getNumElements() == 2) check which causes the verifier to fail, due
to presence of the DW_OP_deref. That check is there since we're missing things
in DwarfExpression, and perhaps other places, to correctly emit entry values in
those cases.

This is what GCC emits for this case:

    00000028 0000000000000000 000000000000000a (DW_OP_breg1 (rdx): 0)
    0000002e 000000000000000a 0000000000000040 (DW_OP_entry_value: (DW_OP_reg1 (rdx)))

This happens often for C++ programs, so if we are not able to fix this quickly
I think we should change LiveDebugValues so that we don't run into this issue.

[Quuxplusone]

Is the https://reviews.llvm.org/D66746 potential quick fix for this?

[Quuxplusone]

Definitely, the code in LiveDebugValues::isEntryValueCandidate() could be changed to avoid the pre-existing expressions as entry values, with proper TODO marker. That will resolve this issue.

When we extend the DwarfExpression (and other places) with the support for this, we will change the part in LiveDebugValues::isEntryValueCandidate() as well.

WDYT?

[Quuxplusone]

(In reply to Djordje Todorovic from comment #2)
> Definitely, the code in LiveDebugValues::isEntryValueCandidate() could be
> changed to avoid the pre-existing expressions as entry values, with proper
> TODO marker. That will resolve this issue.
>
> When we extend the DwarfExpression (and other places) with the support for
> this, we will change the part in LiveDebugValues::isEntryValueCandidate() as
> well.
>
> WDYT?

That sounds good!

I'll bring D66746 back to life again, or perhaps create a new revision,
tomorrow.

[Quuxplusone]

Proposed quick fix to stop the failed assertion: https://reviews.llvm.org/D66746.

[Quuxplusone]

(In reply to David Stenberg from comment #4)
> Proposed quick fix to stop the failed assertion:
> https://reviews.llvm.org/D66746.

Landed on master as 5c7cc6f83d1f3ea2016d94e1c9cc25f814d2671b.

I do not intend on working with adding support for such entry values at the
moment at least. I did however write down some suggested changes, which I think
may be necessary to accomplish that, in the commit message.