Off-by-one in generated DWARF columns

[RReverser]

While parsing DWARF in custom tooling, I've noticed that Rust generates 0-based columns, whereas other compilers, including LLVM-based (e.g. Clang and Swift) generate 1-based column.

This creates issues for tools that want to extract source spans, generate error messages or otherwise link to the original source location.

Let's take an example for Clang:

extern void abort();

void assert_less(int num1, int num2) {
    if (num1 >= num2) {
        abort();
    }
}

int main() {
    assert_less(10, 20);
    assert_less(30, 20);
    return 0;
}

This generates the following debug info (Godbolt):

!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 9.0.0 (tags/RELEASE_900/final 372344)", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2, nameTableKind: None)
!1 = !DIFile(filename: "example.c", directory: "/home/ubuntu")
...
!7 = distinct !DISubprogram(name: "assert_less", scope: !8, file: !8, line: 3, type: !9, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
!8 = !DIFile(filename: "./example.c", directory: "/home/ubuntu")
!9 = !DISubroutineType(types: !2)
!10 = !DILocation(line: 4, column: 9, scope: !7)
!11 = !DILocation(line: 4, column: 17, scope: !7)
!12 = !DILocation(line: 4, column: 14, scope: !7)
!13 = !DILocation(line: 5, column: 9, scope: !7)
!14 = !DILocation(line: 7, column: 1, scope: !7)
!15 = distinct !DISubprogram(name: "main", scope: !8, file: !8, line: 9, type: !9, scopeLine: 9, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
!16 = !DILocation(line: 10, column: 5, scope: !15)
!17 = !DILocation(line: 11, column: 5, scope: !15)
!18 = !DILocation(line: 12, column: 5, scope: !15)

You can see how locations for expressions start at the first char, e.g. 5:9 for abort(...), 10:5 for assert_less(...) etc.

Now let's take a Rust example:

extern {
    fn abort();
}

fn assert_less(num1: i32, num2: i32) {
    if num1 >= num2 {
        unsafe {
            abort();
        }
    }
}

pub fn main() {
    assert_less(10, 20);
    assert_less(30, 20);
}

This generates debug info (Godbolt):

...
!2 = distinct !DICompileUnit(language: DW_LANG_Rust, file: !3, producer: "clang LLVM (rustc version 1.38.0 (625451e37 2019-09-23))", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4)
!3 = !DIFile(filename: "./example.rs", directory: "/home/ubuntu")
!4 = !{}
!5 = distinct !DISubprogram(name: "assert_less", linkageName: "_ZN7example11assert_less17h3f90e1d508137775E", scope: !6, file: !3, line: 5, type: !7, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, unit: !2, templateParams: !4, retainedNodes: !4)
!6 = !DINamespace(name: "example", scope: null)
!7 = !DISubroutineType(types: !4)
!8 = !DILocation(line: 6, column: 7, scope: !5)
!9 = !DILocation(line: 6, column: 4, scope: !5)
!10 = !DILocation(line: 8, column: 12, scope: !11)
!11 = distinct !DILexicalBlock(scope: !5, file: !3, line: 7, column: 8)
!12 = !DILocation(line: 11, column: 1, scope: !5)
!13 = distinct !DISubprogram(name: "main", linkageName: "_ZN7example4main17h3efe614eed321469E", scope: !6, file: !3, line: 13, type: !7, scopeLine: 13, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2, templateParams: !4, retainedNodes: !4)
!14 = !DILocation(line: 14, column: 4, scope: !13)
!15 = !DILocation(line: 15, column: 4, scope: !13)
!16 = !DILocation(line: 16, column: 1, scope: !13)

You can see how locations for beginning of the line are still starting at column 1 like in Clang, but locations for various expressions are like 8:12 for abort, 14:4 for assert_less, etc. - that is, starting right before the expression rather than at its first character.

I wasn't sure which one is correct, so I also checked Swift compiler (it does the same as Clang) and GCC (it turns out not to implement column information yet).

I also looked at LLVM docs, and their examples for debug information also use 1-based columns, pointing at the first char of an expression: https://llvm.org/docs/SourceLevelDebugging.html

Finally, I checked the DWARF spec, and, while I couldn't find exactly how columns are supposed to be represented for expressions, I found this for declarations and expect it to be true / consistent for other items as well:

The value of the DW_AT_decl_line attribute represents the source line number at which the first character of the identifier of the declared object appears. The value 0 indicates that no source line has been specified.

The value of the DW_AT_decl_column attribute represents the source column number at which the first character of the identifier of the declared object appears. The value 0 indicates that no column has been specified.

All in all, it looks like Rust is the one generating columns that violate LLVM and, subsequently, DWARF descriptions and expectations.

[RReverser]

@est31 Looks like you implemented relevant code in #51980, so maybe want to take a look? I'm happy to make a PR, but not sure if my assumptions here are wrong or something changed since the initial implementation that introduced the bug.

[est31]

@RReverser it was definitely not intentional to make them 0 based or inconsistent to what clang's doing. I agree that this is a bug. This problem had been present in other places too:

• column!() should start at 1 not 0 #46868
• Stabilize the panic_col feature #46762 (comment)
• Tracking issue for proc_macro::Span inspection APIs #54725 (comment)

So this bug isn't unique. I guess the root cause of this issue is Rust storing column numbers 0 based internally.

[RReverser]

@est31 Thanks for the confirmation. Do you want me to send a PR or could you make a change?

[est31]

Thanks, but the PR you linked was in fact one of my last PRs to this repo. I have since directed the focus of my free work to other projects. While I'm still open to paid work for contributions in the rust-lang org, this change is too small for that :). TLDR: I'd prefer if you did it :).

[RReverser]

Fair enough. If nobody does this earlier, I'll try to make a PR myself once I have some more spare time. Need to understand the best place to make this sort of adjustment first, because, as you said, it potentially affects several places.

[est31]

I'm still around to give gratis consulting :).

Quick & easy fix would be in source_loc.rs where my PR introduced the change. For such a fix figuring out the right way to test would probably be the hardest part.

A longer term solution of the root of this problem would be to make spans start at column 1 everywhere in the compiler. Maybe I'm missing something and this is actually a bad idea, but the best way to determine that is to attempt a change. You'd probably have to remove +1's in a few places :).

I suggest doing the easy fix first, then this bug is fixed. And in a second step, the internal format can be changed.

Btw kudos to @bjorn3 : I just looked up cranelift's source code and they seem to add 1: https://github.com/bjorn3/rustc_codegen_cranelift/blob/0934dc84fd73d32b4964d6f80390bc9582e9152b/src/debuginfo.rs#L306

[RReverser]

Quick & easy fix would be in source_loc.rs where my PR introduced the change.

Yeah, but I think I'll also need to account for the beginning of line, which is currently correctly reported at column 1 and shouldn't be moved forward. It's not hard to add a condition here, but that feels weird and error-prone.

[est31]

Interesting, good point. However, I believe this isn't pointing to columns of 1 because of special casing for line starts. If I take the line with abort() in your example and remove all indentation, it removes column info in LLVM IR, indicating that 0 was passed to LLVM (which then removes it as it was 0).

The spans where the column is 1 are all pointing to closing braces } of functions and are attached to ret statements. If you add a space prior to the }, the column becomes 2 instead of remaining at 1. So I don't think that the 1 is meant to indicate the } character, but what's behind it. I've also done some experiments with closures where you can omit the }:

const FOO: fn(u32) -> bool = |_v| true; // 38
const FOO: fn(u32) -> bool = |_v| true // 38
;
const FOO: fn(u32) -> bool = |_v| (true); // 40
const FOO: fn(u32) -> bool = (|_v| true); // 40
const FOO: fn(u32) -> bool = |_v| { true }; // 42
const FOO: fn(u32) -> bool = |_v| { true
}; // 1

These confirm the hypothesis: all spans, if you add 1, point to right behind the last character of the function. Further experiments show that even if a function has multiple return statements spread out over the function, it has only one LLVM ret statement that has this behaviour. Not sure whether it should be considered a bug or not, just to keep it in mind.

[RReverser]

@est31 Ah, thanks, you're right, I haven't noticed that column is 1 only for closing braces. That makes way more sense now.

The case with multiple ret is interesting - do you have an example code? All the minimal samples with multiple returns I've tried so far compile down to a single ret with a phi instruction, which doesn't help to reproduce these differences.

[RReverser]

I haven't noticed that column is 1 only for closing braces. That makes way more sense now.

Although this probably still means that I need to handle this special case and not just add 1 since it would point yet another char further beyond the last one.

[est31]

The case with multiple ret is interesting - do you have an example code? All the minimal samples with multiple returns I've tried so far compile down to a single ret with a phi instruction, which doesn't help to reproduce these differences.

Sorry that sentence I wrote was easy to misunderstand. I got the same results as you. All the return statements get converted to jumps to a section on the end, which then has a single ret instruction per function (not multiple). That ret has that specific span that points to beyond the end of the function.

Further experiments show that even if a function has multiple return statements spread out over the function, it has only one LLVM ret statement that has this behaviour.

[RReverser]

Ah, yeah, I thought you meant that there are several ret statements but only one has this behaviour.