Hi everyone,

Hope I'm not too late for this discussion.

I'm not familiar with ppc so it spent me some time to reproduce this. 
But at last I didn't make it.

What I did:

1 checkout to tip/objtool/core

2 apply this patch

3 recover the unreachable() after WARN_ENTRY(..

4 compile on defconfig/allyesconfig

If anyone can point out which file will generate this problem it will be 
perfect.

On 2022/6/30 16:05, Naveen N. Rao wrote:

Christophe Leroy wrote:

quoted

Hi Sathvika,

Adding ARM people as they seem to face the same kind of problem (see 
https://patchwork.kernel.org/project/linux-kbuild/patch/20220623014917.199563-33-chenzhongjin@huawei.com/)

For my situation, the problem is, if there is an unreachable() used in 
the switch default case with nothing else, compiler will generate a NOP 
and is still a jump to this NOP branch while it is marked in 
.discard.unreachable.

When objtool deal with .discard.unreachable, it will *do nothing* to 
this NOP itself, but mark the previous instruction as "dead_end" (see 
check.c:ignore_unreachable_insn()). And checking will stop when reach 
this dead_end insn.

0x4: jne 0x14        <= jump for switch case

..

0x10: ret                <= dead_end

0x14: nop              <= real unreachable instructiond

However, actually we have a jump to NOP, which makes this reachable to 
this branch, and when this NOP is at end of function, it get a "fall 
through" warning.


I changed the unreachable to -EINVAL but it was criticized by the 
committer because he thought it is objtool's job to deal with these 
special cases.

quoted

Le 27/06/2022 à 17:35, Sathvika Vasireddy a écrit :

quoted

On 25/06/22 12:16, Christophe Leroy wrote:

quoted

Le 24/06/2022 à 20:32, Sathvika Vasireddy a écrit :

quoted

objtool is throwing *unannotated intra-function call*
warnings with a few instructions that are marked
unreachable. Remove unreachable() from WARN_ON()
to fix these warnings, as the codegen remains same
with and without unreachable() in WARN_ON().

Did you try the two exemples described in commit 1e688dd2a3d6
("powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() 
with
asm goto") ?

Without your patch:

00000640 <test>:
   640:    81 23 00 84     lwz     r9,132(r3)
   644:    71 29 40 00     andi.   r9,r9,16384
   648:    40 82 00 0c     bne     654 <test+0x14>
   64c:    80 63 00 0c     lwz     r3,12(r3)
   650:    4e 80 00 20     blr
   654:    0f e0 00 00     twui    r0,0

00000658 <test9w>:
   658:    2c 04 00 00     cmpwi   r4,0
   65c:    41 82 00 0c     beq     668 <test9w+0x10>
   660:    7c 63 23 96     divwu   r3,r3,r4
   664:    4e 80 00 20     blr
   668:    0f e0 00 00     twui    r0,0
   66c:    38 60 00 00     li      r3,0
   670:    4e 80 00 20     blr


With your patch:

00000640 <test>:
   640:    81 23 00 84     lwz     r9,132(r3)
   644:    71 29 40 00     andi.   r9,r9,16384
   648:    40 82 00 0c     bne     654 <test+0x14>
   64c:    80 63 00 0c     lwz     r3,12(r3)
   650:    4e 80 00 20     blr
   654:    0f e0 00 00     twui    r0,0
   658:    4b ff ff f4     b       64c <test+0xc>        <==

0000065c <test9w>:
   65c:    2c 04 00 00     cmpwi   r4,0
   660:    41 82 00 0c     beq     66c <test9w+0x10>
   664:    7c 63 23 96     divwu   r3,r3,r4
   668:    4e 80 00 20     blr
   66c:    0f e0 00 00     twui    r0,0
   670:    38 60 00 00     li      r3,0            <==
   674:    4e 80 00 20     blr                <==
   678:    38 60 00 00     li      r3,0
   67c:    4e 80 00 20     blr

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.

It is right and on arm64 only BUG() has unreachable and there is no 
annotation for __WARN_FLAGS(). Objtool works correctly on this. For that 
I support that unreachable() annotation shouldn't be with __WARN_FLAGS() 
because there should be an accessible branch after WARN() micro. However 
in the previous case, it's wired that compiler generates a bl to 
unreachable symbol, IIUC it is not a illegal call? (if it is allowed on 
ppc then objtool should be tell to recognize this)


It seems that your decoding only care about INSN_CALL for mcount, so 
maybe temporarily these control flow checking makes non-sense for you so 
the solution could actually be looser.

Anyway, maybe I can help more if I can reproduce that on my own machine.


Best,

Chen

.