Re: Introduce support for location views

[Andrew Waterman <andrew at sifive dot com>]

On Tue, Jul 18, 2017 at 9:51 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Jul 17, 2017, Nick Clifton <nickc@redhat.com> wrote:
>
>> The riscv64-elf target also has:
>
>>   GAS REGRESSION: undefined symbols un sleb128 directive
>
> This is caused by riscv's rejection of non-constant sleb128 operands, in
> s_riscv_leb128.  The comments above that function suggest relaxation
> problems would arise, but configuring riscv to use the regular s_leb128
> instead fixed this and a number of other test failures.  Is the
> rationaled behind this rejection of non-constant operands to leb128
> still applicable?

In general, the RISC-V target does not support subtraction of code
labels at assembly time, as the delta may be adjusted by the linker.
In principle, this restriction could be lifted in the future by adding
additional relocations to inform the linker that it must adjust a
sleb128 or uleb128.  However, the change you've proposed will at this
time break the RISC-V GCC port.  GCC checks at configure time whether
uleb128 supports label subtraction, and avoids it if not.  If it
erroneously believes label subtraction works, it will generate DWARF
that will not work on RISC-V.

>
> FWIW, changes in the LVU patch (or in the patchset in branch
> users/aoliva/SFN) require leb128 operands to be defined and resolved,
> but parses the operands as deferred expressions, and only checks for
> definedness/resolvedness at write-out time.
>
> Parsing deferred expressions in s_leb128 operands is a very important
> requirement for location views, since in some cases involving alignment
> between .loc directives, view numbers can only be determined after
> relaxation, and they may very well appear in view-augmented location
> lists, given the SFN+LVU patches for GCC to emit them.
>
>>   GAS REGRESSION: DWARF2 6
>>   GAS REGRESSION: DWARF2 7
>>   GAS REGRESSION: DWARF2 10
>>   GAS REGRESSION: DWARF2 12
>>   GAS REGRESSION: DWARF2 13
>>   GAS REGRESSION: DWARF2 15
>>   GAS REGRESSION: DWARF2 18
>
> Most of these have to do with the assembler's failure to resolve local
> symbol differences to constants (by design, to make room for linker
> relaxations), resolving them to ADD16/SUB16 relocation pairs, and
> readelf's failure to support these relocation types.  There are
> preexisting tests that fail for the same reasons.
>
> Once I changed readelf to silently ignore these relocations, a few of
> these new tests pass, but most still fail just because without computing
> and applying the relocations, the offsets in debug sections don't match
> the expectations.  A few cases I checked by hand would actually yield
> the expected value.
>
>
> FTR, here's the patch I used for testing, that might serve as a starting
> point for readelf improvements, and for the removal of the special
> requirement on leb128 operands in the assembler.
>
>
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index b2f75c0..efb4c62 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -11938,6 +11938,21 @@ target_specific_reloc_handling (Elf_Internal_Rela * reloc,
>         break;
>        }
>
> +    case EM_RISCV:
> +      {
> +       if (reloc == NULL)
> +         return TRUE;
> +
> +       switch (reloc_type)
> +         {
> +         case 34: /* R_RISCV_ADD16 */
> +         case 38: /* R_RISCV_SUB16 */
> +           /* FIXME: just silence the error for now.  */
> +           return TRUE;
> +         }
> +       break;
> +      }
> +
>      case EM_RL78:
>        {
>         static bfd_vma saved_sym1 = 0;
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 55c41c5..d3c2b38e 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -2543,24 +2543,6 @@ riscv_elf_final_processing (void)
>    elf_elfheader (stdoutput)->e_flags |= elf_flags;
>  }
>
> -/* Parse the .sleb128 and .uleb128 pseudos.  Only allow constant expressions,
> -   since these directives break relaxation when used with symbol deltas.  */
> -
> -static void
> -s_riscv_leb128 (int sign)
> -{
> -  expressionS exp;
> -  char *save_in = input_line_pointer;
> -
> -  expression (&exp);
> -  if (exp.X_op != O_constant)
> -    as_bad (_("non-constant .%cleb128 is not supported"), sign ? 's' : 'u');
> -  demand_empty_rest_of_line ();
> -
> -  input_line_pointer = save_in;
> -  return s_leb128 (sign);
> -}
> -
>  /* Pseudo-op table.  */
>
>  static const pseudo_typeS riscv_pseudo_table[] =
> @@ -2573,8 +2555,6 @@ static const pseudo_typeS riscv_pseudo_table[] =
>    {"dtprelword", s_dtprel, 4},
>    {"dtpreldword", s_dtprel, 8},
>    {"bss", s_bss, 0},
> -  {"uleb128", s_riscv_leb128, 0},
> -  {"sleb128", s_riscv_leb128, 1},
>
>    { NULL, NULL, 0 },
>  };
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer