Dangling debug value or bug in argument elimination pass?

I will start from afar… When a dead function argument is removed, should we keep around the debug info for it?

I have the following case:

define internal fastcc void @foo(i8* %aa, i8* %reg, i8* %field, i32 %bb, …) unnamed_addr #3 !dbg !28 {

entry:

call void @llvm.dbg.value(metabb i8* %aa, i64 0, metabb !34, metabb !47), !dbg !57

call void @llvm.dbg.value(metabb i8* %reg, i64 0, metabb !35, metabb !47), !dbg !58

call void @llvm.dbg.value(metabb i8* %field, i64 0, metabb !36, metabb !47), !dbg !59

call void @llvm.dbg.value(metabb i32 %bb, i64 0, metabb !37, metabb !47), !dbg !60

During dead argument elimination pass, some of the arguments are found “dead” (which they are) and removed, without removing their correspondent llvm.dbg.value:

DAE - Removing argument 1 (reg) from foo

DAE - Removing argument 2 (field) from foo

define internal fastcc void @foo(i8* %aa, i32 %bb, …) unnamed_addr #3 !dbg !28 {

entry:

call void @llvm.dbg.value(metabb !2, i64 0, metabb !34, metabb !47), !dbg !57

call void @llvm.dbg.value(metabb i8* null, i64 0, metabb !35, metabb !47), !dbg !58

call void @llvm.dbg.value(metabb i8* null, i64 0, metabb !36, metabb !47), !dbg !59

call void @llvm.dbg.value(metabb !2, i64 0, metabb !37, metabb !47), !dbg !60

From that point on we carry dead debug info and correspondent metadata (DILocalVariable):

!33 = !{!34, !35, !36, !37, …}

!34 = !DILocalVariable(name: “aa”, arg: 1, scope: !28, file: !11, line: 90, type: !14)

!35 = !DILocalVariable(name: “reg”, arg: 2, scope: !28, file: !11, line: 90, type: !31)

!36 = !DILocalVariable(name: “field”, arg: 3, scope: !28, file: !11, line: 90, type: !31)

!37 = !DILocalVariable(name: “bb”, arg: 4, scope: !28, file: !11, line: 90, type: !6)

In some instances it is wasteful but harmless, in others verifier is extremely unhappy about it.

The question is:

  1. Should llvm.dbg.value for dead argument has been deleted?

  2. Should DILocalVariable has been deleted?

Thanks…

Sergei

Unclear. It’s going to be awkward for debug info regardless (describing the variable is important to name lookup, describing the parameter’s more important just so the user can at least see it’s optimized out - but could break the ability to invoke the function (because the debugger may use all the function parameters to figure out the ABI, even if the parameter doesn’t have a location description))

Certainly we shouldn’t create stuff that the verifier thinks is bad (do you have a specific example of where that comes up?) but I don’t know whether the fix is in the creation, the verifier, or both.

(do you have a specific example of where that comes up?)

Yes. If I use CloneFunctionInto on such scope, the hell breaks loose. At that point I have debug entries that are effectively dead and not being updated during cloning.

Unfortunately I do not yet have a clear test case that would show it on the public tree…

Sergei

David, Peter,

Let me try it one more time, now with an example… Short of upstreaming a fake function clone pass, let me only illustrate the issue in raw debug output. Hopefully it should be sufficient.

Try this:

clang -Os -g -fno-strict-aliasing test_arg_del.ll -mllvm -debug -mllvm -print-after-all

After

DAE - Removing argument 1 (reg) from foo

DAE - Removing argument 2 (field) from foo

DAE - Removing argument 4 (offset) from foo

We are going to have something like this:

*** IR Dump After Dead Argument Elimination ***; ModuleID = ‘test_arg_del.ll’

define internal fastcc void @foo(i8* %base, i64 %data, i64 %shift) unnamed_addr #3 !dbg !32 {

entry:

call void @llvm.dbg.value(metadata i8* null, i64 0, metadata !39, metadata !20), !dbg !47

call void @llvm.dbg.value(metadata i8* null, i64 0, metadata !40, metadata !20), !dbg !48

These are debug calls for the removed arguments, with actual metadata attached to it. If this code is now cloned using llvm::CloneFunctionInto()… After Peter’s patch was introduced:

commit af289e04413504c3bdc252e08c3fe17bf7ea6dc8

Author: Peter Collingbourne peter@pcc.me.uk

Cloning: Reduce complexity of debug info cloning and fix correctness issue.

Commit r260791 contained an error in that it would introduce a cross-module

reference in the old module. It also introduced O(N^2) complexity in the

module cloner by requiring the entire module to be visited for each function.

Fix both of these problems by avoiding use of the CloneDebugInfoMetadata

function (which is only designed to do intra-module cloning) and cloning

function-attached metadata in the same way that we clone all other metadata.

Differential Revision: ⚙ D18583 Cloning: Reduce complexity of debug info cloning and fix correctness issue.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@264935 91177308-0d34-0410-b5e6-96231b3b80d8

we will have stale DISubprogram because RemapInstruction called from CloneFunctionInto() (here)

// Loop over all instructions, fixing each one as we find it…

for (Instruction &II : *BB) {

RemapInstruction(&II, VMap,

ModuleLevelChanges ? RF_None : RF_NoModuleLevelChanges,

TypeMapper, Materializer);

}

…will actually skip over “metadata i8* null” operands of “call void @llvm.dbg.value” and will not update correspondent debug records… Now the first time verifier takes a look at this, we got an assert.

My question is, conceptually, where this should be fixed? Should cloning code somehow iterate over “unreachable” metadata in a function being cloned, or such metadata should not have been allowed to remain after the dead argument removal?

If we want some debug info remain in place for removed argument (which is fine), what should be passed to debug call? (how call void @llvm.dbg.value should look like?)

I think the problem was there for a while, but until Peter fixed metadata generation for cloned function in the mentioned patch, it did not surface.

Thanks for any leads and/or suggestions.

Sergei

test_arg_del.ll (13.8 KB)

test_arg_del.i (746 Bytes)

I don’t really know what the right choice is wrt dead code - I’d check what we do with (non parameter) variables that are optimized away, perhaps?

I think it is very similar issue with inlining…

In the same example function foo() is fully inlined and deleted, but DISubprogram for it still exist… but will not be picked up by ValueMapper since it is orphan…. While references to it will be, and after cloning they will be plugged in a different context without updating.

!23 = distinct !DISubprogram(name: “foo”, scope: !1, file: !1, line: 3, type: !24, isLocal: true, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: true, variables: !28)

There got to be some mechanism to handle debug info without explicit user.

Sergei