Re: [PATCH v3 12/12] objtool/klp: Fold LoongArch paired ADD/SUB relocations into PCREL

[George Guo]

Hi Xi Ruoyao,

> Hmm, is this a flaw in LLVM?  We have
>
> config AS_HAS_THIN_ADD_SUB
>     def_bool $(cc-option,-Wa$(comma)-mthin-add-sub) || AS_IS_LLVM
>
> so we are already expecting LLVM to emit R_LARCH_64_PCREL always instead
> of ADD64/SUB64.

Yes, it looks like a gap in clang.  I measured the full matrix for
".quad sym - ." on the LoongArch box (GAS 2.43.50, GCC 14.2.1, clang
21.1.8):

                          default       -Wa,-mthin-add-sub
  as / gcc, local sym     PAIR          THIN
  as / gcc, extern sym    PAIR          THIN
  clang,    local sym     THIN          rejected (no such flag)
  clang,    extern sym    PAIR          rejected (no such flag)

  THIN = single R_LARCH_64_PCREL
  PAIR = R_LARCH_ADD64 + R_LARCH_SUB64 at one offset

Two facts fall out.

GAS defaults to the pair and needs -mthin-add-sub to thin.  The kernel
passes that flag (arch/loongarch/Makefile), so on GAS both cases are
thin.

clang has no -mthin-add-sub.  It rejects both -mthin-add-sub and
-Wa,-mthin-add-sub as unknown arguments, so cc-option drops the flag and
clang uses its default.  Its default already thins a locally-defined
"sym - .".  The logic is there.  But it does not cover an extern
"sym - .", which stays a pair.  There is no lever to force it.

So the AS_IS_LLVM branch in AS_HAS_THIN_ADD_SUB (commit a7e083772456)
reflects clang's default, and that default thins local but not extern.
This is the case your commit 120dd4118e58 flagged ("Clang has some other
issues"), still present in clang 21.

In the kernel it shows up in __jump_table.  A static key defined in the
same file (e.g. netstamp_needed_key) gets the thin form.  A tracepoint's
key (__tracepoint_netif_rx, defined in another TU) gets the pair.

It only breaks the klp-diff path, not a normal build.  The pair is
rejected by elf_create_reloc(), which allows one relocation per offset.
klp-diff hits that when it clones the __jump_table entry and re-creates
its relocations.  A normal objtool run only reads relocations, so it
never creates two at one offset.  That is why "make LLVM=1" builds fine
but klp-build fails with "duplicate reloc".

This patch folds the pair back into a single R_LARCH_64_PCREL, but only
in the klp-diff clone path (convert_reloc_sym), and only when the SUB
half targets the relocation's own position, i.e. a real "sym - .".

I am not sure this belongs in objtool, though.  Would you prefer:

  a) keep this, so klp-diff tolerates the pair on current clang; or
  b) treat it as an LLVM issue to fix (clang should thin the extern
     case too, matching what it already does for a local symbol), and
     leave objtool as is?

I have not found an existing LLVM report for the extern case.  I can file
one.

Thanks,
George