[PATCH v3 14/28] objtool: Improve tracing of alternative instructions

Alexandre Chartre posted 28 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH v3 14/28] objtool: Improve tracing of alternative instructions
Posted by Alexandre Chartre 2 months, 4 weeks ago
When tracing function validation, improve the reporting of
alternative instruction by more clearly showing the different
alternatives beginning and end.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c                 | 32 +++++++++++++---
 tools/objtool/include/objtool/trace.h | 43 +++++++++++++++++++++
 tools/objtool/trace.c                 | 55 +++++++++++++++++++++++++++
 3 files changed, 124 insertions(+), 6 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 93268b7c015e3..de3ddb0fd6198 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3527,7 +3527,7 @@ static bool skip_alt_group(struct instruction *insn)
 
 	/* ANNOTATE_IGNORE_ALTERNATIVE */
 	if (insn->alt_group->ignore) {
-		TRACE_INSN(insn, "alt group ignored");
+		TRACE_ALT(insn, "alt group ignored");
 		return true;
 	}
 
@@ -3649,24 +3649,44 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		return 1;
 
 	if (insn->alts) {
+		char alt_name[35];
 		int i, num_alts;
 
 		num_alts = 0;
-		for (alt = insn->alts; alt; alt = alt->next)
-			num_alts++;
+		for (alt = insn->alts; alt; alt = alt->next) {
+			if (alt->type == ALT_TYPE_INSTRUCTIONS)
+				num_alts++;
+		}
 
 		i = 1;
 		for (alt = insn->alts; alt; alt = alt->next) {
-			TRACE_INSN(insn, "alternative %d/%d", i, num_alts);
+			if (trace) {
+				switch (alt->type) {
+				case ALT_TYPE_EX_TABLE:
+					strcpy(alt_name, "EXCEPTION");
+					break;
+				case ALT_TYPE_JUMP_TABLE:
+					strcpy(alt_name, "JUMP");
+					break;
+				case ALT_TYPE_INSTRUCTIONS:
+					snprintf(alt_name, sizeof(alt_name),
+						"ALTERNATIVE %d/%d", i, num_alts);
+					break;
+				}
+				trace_alt_begin(insn, alt, alt_name);
+			}
 			ret = validate_branch(file, func, alt->insn, *statep);
+			if (trace)
+				trace_alt_end(insn, alt, alt_name);
 			if (ret) {
 				BT_INSN(insn, "(alt)");
 				return ret;
 			}
-			i++;
+			if (alt->insn->alt_group)
+				i++;
 		}
 
-		TRACE_INSN(insn, "alternative orig");
+		TRACE_ALT_INFO_NOADDR(insn, "/ ", "DEFAULT");
 	}
 
 	if (skip_alt_group(insn))
diff --git a/tools/objtool/include/objtool/trace.h b/tools/objtool/include/objtool/trace.h
index 5b8abdb9b09fb..3282f3bc3a275 100644
--- a/tools/objtool/include/objtool/trace.h
+++ b/tools/objtool/include/objtool/trace.h
@@ -19,6 +19,21 @@ extern int trace_depth;
 		fprintf(stderr, fmt, ##__VA_ARGS__);		\
 })
 
+/*
+ * Print the instruction address and a message. The instruction
+ * itself is not printed.
+ */
+#define TRACE_ADDR(insn, fmt, ...)				\
+({								\
+	if (trace) {						\
+		disas_print_info(stderr, insn, trace_depth - 1, \
+				 fmt "\n", ##__VA_ARGS__);	\
+	}							\
+})
+
+/*
+ * Print the instruction address, the instruction and a message.
+ */
 #define TRACE_INSN(insn, fmt, ...)				\
 ({								\
 	if (trace) {						\
@@ -36,6 +51,20 @@ extern int trace_depth;
 		trace_insn_state(insn, sprev, snext);		\
 })
 
+#define TRACE_ALT_FMT(pfx, fmt) pfx "<alternative.%lx> " fmt
+
+#define TRACE_ALT(insn, fmt, ...)				\
+	TRACE_INSN(insn, TRACE_ALT_FMT("", fmt),		\
+		   (insn)->offset, ##__VA_ARGS__)
+
+#define TRACE_ALT_INFO(insn, pfx, fmt, ...)			\
+	TRACE_ADDR(insn, TRACE_ALT_FMT(pfx, fmt),		\
+		   (insn)->offset, ##__VA_ARGS__)
+
+#define TRACE_ALT_INFO_NOADDR(insn, pfx, fmt, ...)		\
+	TRACE_ADDR(NULL, TRACE_ALT_FMT(pfx, fmt),		\
+		   (insn)->offset, ##__VA_ARGS__)
+
 static inline void trace_enable(void)
 {
 	trace = true;
@@ -61,17 +90,31 @@ static inline void trace_depth_dec(void)
 
 void trace_insn_state(struct instruction *insn, struct insn_state *sprev,
 		      struct insn_state *snext);
+void trace_alt_begin(struct instruction *orig_insn, struct alternative *alt,
+		     char *alt_name);
+void trace_alt_end(struct instruction *orig_insn, struct alternative *alt,
+		   char *alt_name);
 
 #else /* DISAS */
 
 #define TRACE(fmt, ...)
+#define TRACE_ADDR(insn, fmt, ...)
 #define TRACE_INSN(insn, fmt, ...)
 #define TRACE_INSN_STATE(insn, sprev, snext)
+#define TRACE_ALT(insn, fmt, ...)
+#define TRACE_ALT_INFO(insn, fmt, ...)
+#define TRACE_ALT_INFO_NOADDR(insn, fmt, ...)
 
 static inline void trace_enable(void) {}
 static inline void trace_disable(void) {}
 static inline void trace_depth_inc(void) {}
 static inline void trace_depth_dec(void) {}
+static inline void trace_alt_begin(struct instruction *orig_insn,
+				   struct alternative *alt,
+				   char *alt_name) {};
+static inline void trace_alt_end(struct instruction *orig_insn,
+				 struct alternative *alt,
+				 char *alt_name) {};
 
 #endif
 
diff --git a/tools/objtool/trace.c b/tools/objtool/trace.c
index ef9250d4646bb..1c3d961b5123a 100644
--- a/tools/objtool/trace.c
+++ b/tools/objtool/trace.c
@@ -147,3 +147,58 @@ void trace_insn_state(struct instruction *insn, struct insn_state *sprev,
 
 	insn->trace = 1;
 }
+
+void trace_alt_begin(struct instruction *orig_insn, struct alternative *alt,
+		     char *alt_name)
+{
+	struct instruction *alt_insn;
+	char suffix[2];
+
+	alt_insn = alt->insn;
+
+	if (alt->type == ALT_TYPE_EX_TABLE) {
+		/*
+		 * When there is an exception table then the instruction
+		 * at the original location is executed but it can cause
+		 * an exception. In that case, the execution will be
+		 * redirected to the alternative instruction.
+		 *
+		 * The instruction at the original location can have
+		 * instruction alternatives, so we just print the location
+		 * of the instruction that can cause the exception and
+		 * not the instruction itself.
+		 */
+		TRACE_ALT_INFO_NOADDR(orig_insn, "/ ", "%s for instruction at 0x%lx <%s+0x%lx>",
+				      alt_name,
+				      orig_insn->offset, orig_insn->sym->name,
+				      orig_insn->offset - orig_insn->sym->offset);
+	} else {
+		TRACE_ALT_INFO_NOADDR(orig_insn, "/ ", "%s", alt_name);
+	}
+
+	if (alt->type == ALT_TYPE_JUMP_TABLE) {
+		/*
+		 * For a jump alternative, if the default instruction is
+		 * a NOP then it is replaced with the jmp instruction,
+		 * otherwise it is replaced with a NOP instruction.
+		 */
+		trace_depth++;
+		if (orig_insn->type == INSN_NOP) {
+			suffix[0] = (orig_insn->len == 5) ? 'q' : '\0';
+			TRACE_ADDR(orig_insn, "jmp%-3s %lx <%s+0x%lx>", suffix,
+				   alt_insn->offset, alt_insn->sym->name,
+				   alt_insn->offset - alt_insn->sym->offset);
+		} else {
+			TRACE_ADDR(orig_insn, "NOP%d", orig_insn->len);
+			trace_depth--;
+		}
+	}
+}
+
+void trace_alt_end(struct instruction *orig_insn, struct alternative *alt,
+		   char *alt_name)
+{
+	if (alt->type == ALT_TYPE_JUMP_TABLE && orig_insn->type == INSN_NOP)
+		trace_depth--;
+	TRACE_ALT_INFO_NOADDR(orig_insn, "\\ ", "%s end", alt_name);
+}
-- 
2.43.5
Re: [PATCH v3 14/28] objtool: Improve tracing of alternative instructions
Posted by kernel test robot 2 months, 3 weeks ago
Hi Alexandre,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.18-rc5]
[cannot apply to next-20251112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexandre-Chartre/objtool-Move-disassembly-functions-to-a-separated-file/20251113-013604
base:   linus/master
patch link:    https://lore.kernel.org/r/20251112160315.2207947-15-alexandre.chartre%40oracle.com
patch subject: [PATCH v3 14/28] objtool: Improve tracing of alternative instructions
:::::: branch date: 3 hours ago
:::::: commit date: 3 hours ago
config: powerpc-allnoconfig (https://download.01.org/0day-ci/archive/20251113/202511130458.4jK2ZjK0-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251113/202511130458.4jK2ZjK0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202511130458.4jK2ZjK0-lkp@intel.com/

All errors (new ones prefixed by >>):

   check.c: In function 'validate_insn':
>> check.c:3663:29: error: 'trace' undeclared (first use in this function)
    3663 |                         if (trace) {
         |                             ^~~~~
   check.c:3663:29: note: each undeclared identifier is reported only once for each function it appears in
   make[5]: *** [tools/build/Makefile.build:85: tools/objtool/check.o] Error 1
   make[5]: Target '__build' not remade because of errors.
   make[4]: *** [Makefile:87: tools/objtool/objtool-in.o] Error 2
   make[4]: Target 'all' not remade because of errors.
   make[3]: *** [Makefile:73: objtool] Error 2
   make[2]: *** [Makefile:1449: tools/objtool] Error 2
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki