Thread (41 messages) 41 messages, 7 authors, 2022-07-08

Re: [RFC PATCH v3 11/12] powerpc: Remove unreachable() from WARN_ON()

From: Christophe Leroy <hidden>
Date: 2022-06-29 18:30:31
Also in: linux-arm-kernel, lkml
Subsystem: objtool, the rest · Maintainers: Josh Poimboeuf, Peter Zijlstra, Linus Torvalds

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/)

Le 27/06/2022 à 17:35, Sathvika Vasireddy a écrit :
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.


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);
  }

---

I also had objtool running for ever on 
arch/powerpc/sysdev/xics/icp-hv.o, which I fixed with the below hack:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 51b6dcec8d6a..ef2303ad6381 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -529,7 +529,7 @@ static struct instruction *find_last_insn(struct 
objtool_file *file,
  	unsigned int offset;
  	unsigned int end = (sec->sh.sh_size > 10) ? sec->sh.sh_size - 10 : 0;

-	for (offset = sec->sh.sh_size - 1; offset >= end && !insn; offset--)
+	for (offset = sec->sh.sh_size - 1; offset && offset >= end && !insn; 
offset--)
  		insn = find_insn(file, sec, offset);

  	return insn;
---

Now I only have the following two warnings:

arch/powerpc/sysdev/xics/icp-hv.o: warning: objtool: can't find 
unreachable insn at .text.unlikely+0x0
drivers/crypto/vmx/aesp8-ppc.o: warning: objtool: 
aes_p8_set_encrypt_key+0x44: unannotated intra-function call

The first one is linked to the infinite loop I hacked. So I now have to 
understand what the problem really is.

The second one is an assembly file aesp8-ppc.S which lacks the changes 
you did in other assembly files in patch 12.

Christophe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help