[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ubqjeplvslhnspqw6pnqwo7c6sq2ygdtmkuqr4q3hjlxfkuwii@xn63k6qz22mz>
Date: Tue, 28 Oct 2025 17:21:04 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Peter Zijlstra <peterz@...radead.org>, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, Alexandre Chartre <alexandre.chartre@...cle.com>
Subject: Re: odd objtool 'unreachable instruction' warning
On Tue, Oct 28, 2025 at 12:29:11PM -0700, Linus Torvalds wrote:
> Josh, Peter,
>  due to another entirely unrelated discussion, I ended up resurrecting
> my "make asm readable" patch that I have had in my local tree when I
> want to look at the actual generated code for user accesses.
> 
> That is a local hack that just removes the alternative noise for the
> common ops, so that I actually see the fences and clac/stac
> instructions as such, instead of seeing them as nops in the object
> file or as horrible noise in the assembler output.
> 
> So that patch is not something I'd ever commit in general, but it's
> really useful for checking code generation - but it results in odd
> objdump warnings these days (I say "these days", because I've used
> that patch locally over the years, and the objdump warning hasn't
> always been there).
> 
> It's a pretty odd warning, because the code looks fine to me, but I
> might be missing something obvious.
> 
> Anyway, this is clearly not a big and urgent problem, but I'd love for
> you to take a look. I'm attaching the patch I use so  you can see what
> I mean.. Any ideas what triggers that warning? Because I'd love to
> keep this patch in my local tree without having objtool be upset with
> me....
Ironically I think this "bug" was introduced by a patch whose goal was
to fix a regression in STAC/CLAC code readability:
  2d12c6fb7875 ("objtool: Remove ANNOTATE_IGNORE_ALTERNATIVE from CLAC/STAC")
See the diff below, hopefully that fixes things for you?  I can post a
proper patch in a bit.
On a related note, it would be nice if we could make that codegen more
readable...  Here were a few formats I had played with before, any
thoughts?
Option 1:
# <ALTERNATIVE>
# ORIGINAL:
771:	nop
772:	.skip -(((775f-774f)-(772b-771b)) > 0) * ((775f-774f)-(772b-771b)), 0x90; 773: .pushsection .altinstructions, "a"; .long 771b - .; .long 774f - .; .4byte ( 9*32+20); .byte 773b-771b; .byte 775f-774f; .popsection; .pushsection .altinstr_replacement, "ax"
# REPLACEMENT: [X86_FEATURE_SMAP]
774:	stac
775:	.popsection
# </ALTERNATIVE>
Option 2:
# <ALTERNATIVE>
# ORIGINAL:    nop
# REPLACEMENT: stac [X86_FEATURE_SMAP]
771:	nop
772:	.skip -(((775f-774f)-(772b-771b)) > 0) * ((775f-774f)-(772b-771b)), 0x90; 773: .pushsection .altinstructions, "a"; .long 771b - .; .long 774f - .; .4byte ( 9*32+20); .byte 773b-771b; .byte 775f-774f; .popsection; .pushsection .altinstr_replacement, "ax"
774:	stac
775:	.popsection
# </ALTERNATIVE>
Note there's also an objtool "disas" feature Alexandre is working on
which will show the disassembly annotated with runtime patching info
(alternatives, static branches, etc):
  https://lore.kernel.org/20250619145659.1377970-1-alexandre.chartre@oracle.com
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 620854fdaaf63..9004fbc067693 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3516,8 +3516,11 @@ static bool skip_alt_group(struct instruction *insn)
 {
 	struct instruction *alt_insn = insn->alts ? insn->alts->insn : NULL;
 
+	if (!insn->alt_group)
+		return false;
+
 	/* ANNOTATE_IGNORE_ALTERNATIVE */
-	if (insn->alt_group && insn->alt_group->ignore)
+	if (insn->alt_group->ignore)
 		return true;
 
 	/*
Powered by blists - more mailing lists
 
