This is an archive of the discontinued LLVM Phabricator instance.

[Driver] Default riscv*- triples to -fdebug-default-version=4
ClosedPublic

Authored by MaskRay on Aug 10 2023, 3:58 PM.

Details

Summary

This adds a RISC-V special case to ToolChain::GetDefaultDwarfVersion,
affecting Linux/Haiku/RISCVToolChain.

DWARF v5 .debug_loclists/.debug_rnglists's
DW_LLE_offset_pair/DW_RLE_offset_pair entry kinds utilitize .uleb128 A-B
directives where A and B reference local labels in code sections.
When A and B are separated by a RISC-V linker-relaxable instruction,
A-B is incorrectly folded without a relocation, causing incorrect debug
information.

void ext(void);
int foo(int x) {ext(); return 0;}
// DW_AT_location [DW_FORM_loclistx] of a DW_TAG_formal_parameter references a DW_LLE_offset_pair that can be incorrect after linker relaxation.

int ext(void);
void foo() { {
  int ret = ext();
  if (__builtin_expect(ret, 0))
    ext();
} }
// DW_AT_ranges [DW_FORM_rnglistx] of a DW_TAG_lexical_block references a DW_RLE_offset_pair that can be incorrect after linker relaxation.

D157657 will implement R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128
relocations, fixing the issue, but the relocation is only supported by
bleeding-edge binutils 2.41 and not by lld/ELF yet.

The goal is to make the emitted DWARF correct after linking.
Many users don't care about the default DWARF version, but a linker
error will be unacceptable. Let's just downgrade the default DWARF
version, before binutils>=2.41 is more widely available.

An alternative compatibility option is to add a toggle to DwarfDebug.cpp,
but that doesn't seem like a good idea.

This patch is planned for the main and release/17.x branches.

Diff Detail

Event Timeline

MaskRay created this revision.Aug 10 2023, 3:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2023, 3:58 PM
MaskRay requested review of this revision.Aug 10 2023, 3:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2023, 3:58 PM
MaskRay updated this revision to Diff 549192.Aug 10 2023, 3:59 PM

add a comment

RISCVToolchain also needs an override

Also Haiku I guess, as another OS that has a somewhat working RISC-V port but falls back on ToolChain's default implementation. So maybe this just belongs in the default implementation, given all the ones that need modifying are the ones that end up there.

MaskRay updated this revision to Diff 549233.Aug 10 2023, 8:49 PM
MaskRay retitled this revision from [Driver] Default riscv*-linux* to -fdebug-default-version=4 to [Driver] Default riscv*- triples to -fdebug-default-version=4.
MaskRay edited the summary of this revision. (Show Details)

Thanks for mentioning RISCVToolChain/Haiku.

Update ToolChain::

kito-cheng accepted this revision.Aug 10 2023, 11:32 PM

LGTM, I am OK with that, actually our downstream toolchain has downgrade the dwarf version to 4 too, because binutils/gdb not well support before...

This revision is now accepted and ready to land.Aug 10 2023, 11:32 PM
asb accepted this revision.Aug 14 2023, 7:03 AM

LGTM, this seems like a good workaround.