[PATCH] RISC-V: Fix "withand" in LEB128 error messages

[Fangrui Song]

On Wed, Dec 6, 2023 at 10:15 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
>
> On Wed, 06 Dec 2023 10:03:43 PST (-0800), i@maskray.me wrote:
> > On Wed, Dec 6, 2023 at 9:52 AM Palmer Dabbelt <palmer@rivosinc.com> wrote:
> >>
> >> This was split over multiple lines and ended up missing a space.
> >>
> >> Reported-by: David Abdurachmanov <davidlt@rivosinc.com>
> >> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> >> ---
> >>  bfd/elfnn-riscv.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
> >> index 5c4bf4bc3cb..042266e791b 100644
> >> --- a/bfd/elfnn-riscv.c
> >> +++ b/bfd/elfnn-riscv.c
> >> @@ -2521,7 +2521,7 @@ riscv_elf_relocate_section (bfd *output_bfd,
> >>           else
> >>             {
> >>               msg = ("Mismatched R_RISCV_SET_ULEB128, it must be paired with"
> >> -                    "and applied before R_RISCV_SUB_ULEB128");
> >> +                    " and applied before R_RISCV_SUB_ULEB128");
> >>               r = bfd_reloc_dangerous;
> >>             }
> >>           break;
> >> @@ -2537,7 +2537,7 @@ riscv_elf_relocate_section (bfd *output_bfd,
> >>           else
> >>             {
> >>               msg = ("Mismatched R_RISCV_SUB_ULEB128, it must be paired with"
> >> -                    "and applied after R_RISCV_SET_ULEB128");
> >> +                    " and applied after R_RISCV_SET_ULEB128");
> >>               r = bfd_reloc_dangerous;
> >>             }
> >>           break;
> >> --
> >> 2.42.1
> >>
> >
> > LGTM. Ideally this error message should be tested using `#error:`
> > .reloc directive can be used to create a relocation.
> >
> > .reloc ., R_RISCV_SET_ULEB128, w2
>
> Ya, seems reasonable.  IIUC there's still some discussion in psABI land
> as to exactly what the required semantics of these are, though, so maybe
> we hold off on writing tests until things settle down?

Yes I am aware of
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/413 whose
justification isn't clear.

---

In llvm-project, a somewhat common practice is to pre-commit test
improvement, and when a change arises, make a commit including both
the functional change and the test updates.
This makes it clearer how an individual commit changes the behavior
and sometimes makes a large patch smaller.

Of course in some cases this practice adds some complexity and may not
be worth doing.

I think the behavior of R_RISCV_SET_ULEB128 is worth checking. The
test for this error message seems overdue.