Add a check that the DILocalVariable fragment size in dbg.declare does not exceed the size of the alloca.
This would have caught the invalid debuginfo regenerated by rustc in https://github.com/llvm/llvm-project/issues/64149.
Paths
| Differential D158743
[Verifier] Sanity check alloca size against DILocalVariable fragment size ClosedPublic Authored by nikic on Aug 24 2023, 7:58 AM.
Details
Summary Add a check that the DILocalVariable fragment size in dbg.declare does not exceed the size of the alloca. This would have caught the invalid debuginfo regenerated by rustc in https://github.com/llvm/llvm-project/issues/64149.
Diff Detail
Event Timeline
This revision now requires changes to proceed.Aug 24 2023, 11:07 AM This revision is now accepted and ready to land.Aug 24 2023, 11:09 AM Comment Actions Use isComplex(), update failing tests. I've split X86/stack-frame-layout-remarks.ll into two files for 64-bit and 32-bit.
Closed by commit rG183f49c3e0f4: [Verifier] Sanity check alloca size against DILocalVariable fragment size (authored by nikic). · Explain WhyAug 28 2023, 12:17 AM This revision was automatically updated to reflect the committed changes. Comment Actions Looks like this found a legitimate issue in Clang debuginfo for trivial_abi with non-trivial dtor: https://clang.godbolt.org/z/sWsEMvWas The dbg.declare points to an i1 %nrvo variable, rather than %retval. This revision is now accepted and ready to land.Aug 28 2023, 2:57 AM Comment Actions I've submitted D158972 to fix the clang bug. An additional problem that turned up is that a number of tests that don't specify a target have 64-bit specific debuginfo and thus fail when tested against 32-bit targets. I've fixed these either by tweaking things to be pointer size independent, or by testing these using the x86_64 target. (The Generic/dwarf-public-names.ll test got dropped entirely, because X86/dwarf-public-names.ll already exists and is more extensive to boot.) Closed by commit rG47324cfd7d8c: Reapply [Verifier] Sanity check alloca size against DILocalVariable fragment… (authored by nikic). · Explain WhySep 15 2023, 5:52 AM This revision was automatically updated to reflect the committed changes. Comment Actions It looks like we're hitting this new assertion in rust built with llvm @ HEAD in a few places (after @durin42's https://github.com/llvm/llvm-project/commit/1f33911f50294c07f672a49e311776693823d0bc, which fixed some segfaults): @nikic could you please take a look? Also would it be OK if you reverted this temporarily until we fix the rust side (internally we're building rust and clang with llvm close to head and this prevents us from building rust)? Comment Actions
Okay, I've reverted this for now. I've posted a small reproducer for the rustc bug in https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/LLVM.20HEAD.20llvm.2Edbg.2Edeclare.2Falloca.20size.20mismatch.
Revision Contents
Diff 556850 llvm/lib/IR/Verifier.cpp
llvm/test/CodeGen/ARM/stack-frame-layout-remarks.ll
llvm/test/CodeGen/BPF/warn-stack.ll
llvm/test/CodeGen/X86/stack-frame-layout-remarks-32.ll
llvm/test/CodeGen/X86/stack-frame-layout-remarks-64.ll
llvm/test/CodeGen/X86/stack-frame-layout-remarks.ll
llvm/test/DebugInfo/Generic/discriminated-union.ll
llvm/test/DebugInfo/Generic/dwarf-public-names.ll
llvm/test/DebugInfo/Generic/member-order.ll
llvm/test/DebugInfo/Generic/tu-composite.ll
llvm/test/DebugInfo/Generic/univariant-discriminated-union.ll
llvm/test/DebugInfo/X86/PR20038.ll
llvm/test/DebugInfo/X86/fi-piece.ll
llvm/test/DebugInfo/X86/member-order.ll
llvm/test/DebugInfo/X86/tu-composite.ll
llvm/test/DebugInfo/invalid-sizes.ll
llvm/test/Linker/type-unique-odr-a.ll
llvm/test/Linker/type-unique-odr-b.ll
llvm/test/Linker/type-unique-simple2-a.ll
llvm/test/Linker/type-unique-simple2-b.ll
llvm/test/Transforms/InstCombine/dbg-scalable-store-fixed-frag.ll
llvm/test/Transforms/InstCombine/dbg-simplify-alloca-size.ll
llvm/test/Transforms/Util/dbg-user-of-aext.ll
|
This is my attempt to only handle empty DIExpression or DW_OP_LLVM_fragment, but not things like DW_OP_deref. Is there some cleaner way to check this?