[Verifier] Sanity check alloca size against DILocalVariable fragment size

[nikic]

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.

[nikic]

llvm/lib/IR/Verifier.cpp

6309

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?

[nikic]

Update one more test.

[aprantl]

llvm/lib/IR/Verifier.cpp

6309

Yeah, you need to check that the expression directly points into the alloca and isn't the result computation that uses the contents of the alloca as an input.

[aprantl]

llvm/lib/IR/Verifier.cpp

6309

!isComplex() might do what you want.

[nikic]

Use isComplex(), update failing tests. I've split X86/stack-frame-layout-remarks.ll into two files for 64-bit and 32-bit.

[nikic]

Actually do the isComplex() change...

[nikic]

llvm/lib/IR/Verifier.cpp

6309

Thanks, isComplex() was indeed what I was looking for.

[aprantl]

llvm/lib/IR/Verifier.cpp

6309

We should come up with a better name for isComplex().
Anyway, this LGTM now!

[nikic]

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.

[nikic]

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.)

[krasimir]

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):
https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/22373#018aa862-a088-4d6e-a9a7-967348e12f4b/824-825

@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)?

[nikic]

In D158743#4647390, @krasimir wrote:

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):
https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/22373#018aa862-a088-4d6e-a9a7-967348e12f4b/824-825

@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)?

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.