Page MenuHomePhabricator

[DebugInfo] Fix a fatal error originating from split-macro support
Changes PlannedPublic

Authored by SouraVX on Jun 1 2020, 12:07 PM.

Details

Summary

When generting DWARFv5 split dwarf macro info, debug_line_offset in
macro section header was pointing to debug_line_start symbol in .debug_line
section in primary binary resulting in relocation and subsequent
fatal error:

fatal error: error in backend: A dwo section may not contain relocations

In this case this should point to debug_line_start symbol in debug_line.dwo
section. LLVM does not emit debug_line.dwo section except for type
units.
As of now skip this field emission, since it is optional.

Link to previous discussion:
http://lists.llvm.org/pipermail/llvm-dev/2020-February/138861.html

Diff Detail

Unit TestsFailed

TimeTest
1,270 mslibarcher.races::task-two.c
Script: -- : 'RUN: at line 14'; /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/openmp/tools/archer/tests -I /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/lib -Wl,-rpath,/mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/lib /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/openmp/tools/archer/tests/races/task-two.c -o /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-two.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=1' /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/openmp/tools/archer/tests/deflake.bash /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-two.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/task-two.c.tmp.log | /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/workspace/BETA_amd64_debian_testing_clang8/llvm-project/openmp/tools/archer/tests/races/task-two.c

Event Timeline

SouraVX created this revision.Jun 1 2020, 12:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2020, 12:07 PM
SouraVX updated this revision to Diff 267697.Jun 1 2020, 12:17 PM

Added corrected test case.

I don't think this field (debug_line_offset) is optional if the debug_macro.dwo section contains DW_MACRO_start_file: "If a DW_MACRO_start_file entry is present, the header contains a reference to the .debug_line section of the compilation."

I'm not entirely clear on how that's meant to work for Split DWARF - what does GCC do? Seems like this might have to, unfortunately duplicate more file names (same design limitation as DWARF type units, though) into .debug_line.dwo.

I don't think this field (debug_line_offset) is optional if the debug_macro.dwo section contains DW_MACRO_start_file: "If a DW_MACRO_start_file entry is present, the header contains a reference to the .debug_line section of the compilation."

Yes, but spec seems a bit ambiguous in stating this. "If zero, that field is omitted. Pg. 166, line 14-16", Not explicitly stating the fact/case where it should be always present.
And spec also doesn't say anything about explicit presence of debug_line.dwo when debug_macro.dwo is present, although it seems it should be present.

I'm not entirely clear on how that's meant to work for Split DWARF - what does GCC do? Seems like this might have to, unfortunately duplicate more file names (same design limitation as DWARF type units, though) into .debug_line.dwo.

GCC(trunk) in non-split mode uses *_strp and for split case uses some forms(Not available in v5).
objdump(binutils 2.32) reports:

objdump: Error:  Unknown macro opcode 22 seen
objdump: Error:  Unknown macro opcode da seen

Anyways in ASM, it's clear that debug_line_offset referring to debug_line_start in debug_line.dwo. Dispensing relocation with a difference of labels(Start & End labels of debug_line.dwo section)

        .section        .debug_macro.dwo,"e",@progbits
.Ldebug_macro0:
        .value  0x5
        .byte   0x2
        .long   .Lskeleton_debug_line0
        .byte   0x7
...
       .section        .debug_line.dwo,"e",@progbits
.Lskeleton_debug_line0:
        .long   .LELT0-.LSLT0

I think if we want this then debug_line.dwo should be enabled first(Not always, but for type units and for macro info).

The correct fix is to emit the .debug_line.dwo section (which repeats the directory/file tables from the .debug_line section in the .o file) and have .debug_macro.dwo point to it. "If a DW_MACRO_start_file entry is present, the header contains a reference to the .debug_line section of the compilation." (p.169) Yes, it would be clearer to mention both normal and .dwo cases, and I will file a clarification issue about that, but I think the intent is clear.

The description of the flag on p.166 is merely describing the relationship of the flag and the existence of the field. It doesn't mean you can arbitrarily choose to omit the field.

The correct fix is to emit the .debug_line.dwo section (which repeats the directory/file tables from the .debug_line section in the .o file) and have .debug_macro.dwo point to it. "If a DW_MACRO_start_file entry is present, the header contains a reference to the .debug_line section of the compilation." (p.169) Yes, it would be clearer to mention both normal and .dwo cases, and I will file a clarification issue about that, but I think the intent is clear.

The description of the flag on p.166 is merely describing the relationship of the flag and the existence of the field. It doesn't mean you can arbitrarily choose to omit the field.

+1 to all that - thanks Paul!

SouraVX planned changes to this revision.Tue, Jun 9, 9:10 AM

I've raised another review D81476 for addressing this. Will revise this afterwards.
Thank You.