Christophe Leroy wrote:

Le 30/06/2022 à 10:05, Naveen N. Rao a écrit :

quoted

Christophe Leroy wrote:

quoted

quoted

The builtin variant of unreachable (__builtin_unreachable()) works.

How about using that instead of unreachable() ?

In fact the problem comes from the macro annotate_unreachable() which 
is called by unreachable() before calling __build_unreachable().

Seems like this macro adds (after the unconditional trap twui) a call 
to an empty function whose address is listed in section 
.discard.unreachable

     1c78:       00 00 e0 0f     twui    r0,0
     1c7c:       55 e7 ff 4b     bl      3d0 
<qdisc_root_sleeping_lock.part.0>


RELOCATION RECORDS FOR [.discard.unreachable]:
OFFSET           TYPE              VALUE
0000000000000000 R_PPC64_REL32     .text+0x00000000000003d0

The problem is that that function has size 0:

00000000000003d0 l     F .text    0000000000000000 
qdisc_root_sleeping_lock.part.0


And objtool is not prepared for a function with size 0.

annotate_unreachable() seems to have been introduced in commit 
649ea4d5a624f0 ("objtool: Assume unannotated UD2 instructions are dead 
ends").

Objtool considers 'ud2' instruction to be fatal, so BUG() has 
__builtin_unreachable(), rather than unreachable(). See commit 
bfb1a7c91fb775 ("x86/bug: Merge annotate_reachable() into _BUG_FLAGS() 
asm"). For the same reason, __WARN_FLAGS() is annotated with 
_ASM_REACHABLE so that objtool can differentiate warnings from a BUG().

On powerpc, we use trap variants for both and don't have a special 
instruction for a BUG(). As such, for _WARN_FLAGS(), using 
__builtin_unreachable() suffices to achieve optimal code generation from 
the compiler. Objtool would consider subsequent instructions to be 
reachable. For BUG(), we can continue to use unreachable() so that 
objtool can differentiate these from traps used in warnings.

Not sure I understand what you mean.

__WARN_FLAGS() and BUG() both use 'twui' which is unconditionnal trap, 
as such both are the same.

On the other side, WARN_ON() and BUG_ON() use tlbnei which is a 
conditionnel trap.

Objtool classifies 'ud2' as INSN_BUG, and 'int3' as INSN_TRAP. In x86 
BUG(), there is no need for an annotation since objtool assumes that 
'ud2' terminates control flow. But, for __WARN_FLAGS(), since 'ud2' is 
used, an explicit annotate_reachable() is needed. That's _reachable_, to 
indicate that the control flow can continue with the next instruction.

On powerpc, we should (eventually) classify all trap variants as 
INSN_TRAP. Even in the absence of that classification today, objtool 
assumes that control flow continues with the next instruction. With your 
work to utilize asm goto for __WARN_FLAGS(), with no extra instructions 
being generated, I think it is appropriate to just use 
__builtin_unreachable() and to not use the annotation.

In any case, we are only hitting this since gcc is generating a 'bl' due 
to that annotation. We are not yet enabling full objtool validation on 
powerpc, so I think we can revisit this at that point.

quoted

quoted

The following changes to objtool seem to fix the problem, most warning 
are gone with that change.

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 63218f5799c2..37c0a268b7ea 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c

@@ -77,6 +77,8 @@ static int symbol_by_offset(const void *key, const 

struct rb_node *node)

      if (*o < s->offset)
          return -1;
+    if (*o == s->offset && !s->len)
+        return 0;
      if (*o >= s->offset + s->len)
          return 1;

@@ -400,7 +402,7 @@ static void elf_add_symbol(struct elf *elf, struct 

symbol *sym)
       * Don't store empty STT_NOTYPE symbols in the rbtree.  They
       * can exist within a function, confusing the sorting.
       */
-    if (!sym->len)
+    if (sym->type == STT_NOTYPE && !sym->len)
          rb_erase(&sym->node, &sym->sec->symbol_tree);
  }

Is there a reason to do this, rather than change __WARN_FLAGS() to use 
__builtin_unreachable()? Or, are you seeing an issue with unreachable() 
elsewhere in the kernel?

At the moment I'm trying to understand what the issue is, and explore 
possible fixes. I guess if we tell objtool that after 'twui' subsequent 
instructions are unreachable, then __builtin_unreachable() is enough.

Yes, see my explanation above. Since no 'bl' is emitted with the 
builtin, objtool won't complain, especially for mcount.

I think we should also understand why annotate_unreachable() gives us a 
so bad result and see if it can be changed to something cleaner than a 
'bl' to an empty function that has no instructions.

Indeed. Not really sure. annotate_unreachable() wants to take the 
address of the instruction after the trap. But, in reality, due to use 
of asm goto for __WARN_FLAGS, no instructions would be generated. I 
wonder if that combination causes such code to be emitted.


- Naveen