Re: [PATCH] lib/Kconfig.debug: Add check for non-constant .{s,u}leb128 support to DWARF5

[Nathan Chancellor]

On Mon, Oct 03, 2022 at 03:47:30AM +0900, Masahiro Yamada wrote:
> On Thu, Sep 29, 2022 at 3:25 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
> >
> > When building with a RISC-V kernel with DWARF5 debug info using clang
> > and the GNU assembler, several instances of the following error appear:
> >
> >   /tmp/vgettimeofday-48aa35.s:2963: Error: non-constant .uleb128 is not supported
> >
> > Dumping the .s file reveals these .uleb128 directives come from
> > .debug_loc and .debug_ranges:
> >
> >   .Ldebug_loc0:
> >           .byte   4                               # DW_LLE_offset_pair
> >           .uleb128 .Lfunc_begin0-.Lfunc_begin0    #   starting offset
> >           .uleb128 .Ltmp1-.Lfunc_begin0           #   ending offset
> >           .byte   1                               # Loc expr size
> >           .byte   90                              # DW_OP_reg10
> >           .byte   0                               # DW_LLE_end_of_list
> >
> >   .Ldebug_ranges0:
> >           .byte   4                               # DW_RLE_offset_pair
> >           .uleb128 .Ltmp6-.Lfunc_begin0           #   starting offset
> >           .uleb128 .Ltmp27-.Lfunc_begin0          #   ending offset
> >           .byte   4                               # DW_RLE_offset_pair
> >           .uleb128 .Ltmp28-.Lfunc_begin0          #   starting offset
> >           .uleb128 .Ltmp30-.Lfunc_begin0          #   ending offset
> >           .byte   0                               # DW_RLE_end_of_list
> >
> > There is an outstanding binutils issue to support a non-constant operand
> > to .sleb128 and .uleb128 in GAS for RISC-V but there does not appear to
> > be any movement on it, due to concerns over how it would work with
> > linker relaxation.
> >
> > To avoid these build errors, prevent DWARF5 from being selected when
> > using clang and an assembler that does not have support for these symbol
> > deltas, which can be easily checked in Kconfig with as-instr plus the
> > small test program from the dwz test suite from the binutils issue.
> >
> > Link: https://sourceware.org/bugzilla/show_bug.cgi?id=27215
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1719
> > Tested-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> > ---
> >  lib/Kconfig.debug | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index d3e5f36bb01e..19de03ead2ed 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -231,6 +231,9 @@ config DEBUG_INFO
> >           in the "Debug information" choice below, indicating that debug
> >           information will be generated for build targets.
> >
> > +config AS_HAS_NON_CONST_LEB128
> > +       def_bool $(as-instr,.uleb128 .Lexpr_end4 - .Lexpr_start3\n.Lexpr_start3:\n.Lexpr_end4:)
> > +
> >  choice
> >         prompt "Debug information"
> >         depends on DEBUG_KERNEL
> > @@ -277,6 +280,10 @@ config DEBUG_INFO_DWARF5
> >         bool "Generate DWARF Version 5 debuginfo"
> >         select DEBUG_INFO
> >         depends on !CC_IS_CLANG || (CC_IS_CLANG && (AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502)))
> > +       # Clang is known to generate .{s,u}leb128 with symbol deltas with
> > +       # DWARF5, which some targets may not support.
> > +       # https://sourceware.org/bugzilla/show_bug.cgi?id=27215
> 
> If you plan to patch both DWARF_TOOLCHAIN_DEFAULT and DWARF5,
> it will be cleaner to move this comment to AS_HAS_NON_CONST_LEB128.

Sure, that sounds reasonable! I can base this change on the series that
you recently sent:

https://lore.kernel.org/20221002181107.51286-1-masahiroy@xxxxxxxxxx/

> > +       depends on !CC_IS_CLANG || AS_HAS_NON_CONST_LEB128
> 
> The condition "!CC_IS_CLANG" is repeated here.
> 
> If you use the following patch as basic,
> https://lore.kernel.org/lkml/20221002181107.51286-2-masahiroy@xxxxxxxxxx/T/#u
> 
> you can write the code like this:
> 
> !CC_IS_CLANG || AS_IS_LLVM || (AS_IS_GNU && AS_VERSION >= 23502 &&
> AS_HAS_NON_CONST_LEB128)

Right, I had initially had something along this line but the length of
the line bothered some folks in pre-review so I opted for a second line.
With your clean ups, it seems reasonable to move it back to the original
line.

> Another big hammer solution is to give up Clang+GAS for CONFIG_DEBUG_INFO.
> If we go this way, this patch is unneeded, though.
> Thoughts?

I think this is a simple enough solution to avoid that big hammer at the
moment but if we continue to run into corner cases like this, that is
certainly worth considering.

Cheers,
Nathan