[RFC PATCH v2 00/17] objtool: Function validation tracing

linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [RFC PATCH v2 00/17] objtool: Function validation tracing
@ 2025-06-19 14:56 Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 01/17] objtool: Move disassembly functions to a separated file Alexandre Chartre
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

Hi,

Version v2 of this RFC addresses all comments from Josh and Peter,
in particular:

- add --disas option to disassemble functions
- do not fail the build if libopcodes is not available. Instead objtool
  is then built without disassembly support. In that case, objtool prints
  a warning message if trying to use disassembly.

  Example:
  $ ./tools/objtool/objtool --disas --link vmlinux.o
  vmlinux.o: warning: objtool: Rebuild with libopcodes for disassembly support
  
- remove dbuffer
- rename VTRACE* to TRACE*
- add trace.[ch] for trace-related functions and macros


This RFC provides the following changes to objtool.

- Disassemble code with libopcodes instead of running objdump

  objtool executes the objdump command to disassemble code. In particular,
  if objtool fails to validate a function then it will use objdump to
  disassemble the entire file which is not very helpful when processing
  a large file (like vmlinux.o).

  Using libopcodes provides more control about the disassembly scope and
  output, and it is possible to disassemble a single instruction or
  a single function. Now when objtool fails to validate a function it
  will disassemble that single function instead of disassembling the
  entire file.

- Add the --trace <function> option to trace function validation

  Figuring out why a function validation has failed can be difficult because
  objtool checks all code flows (including alternatives) and maintains
  instructions states (in particular call frame information).

  The trace option allows to follow the function validation done by objtool
  instruction per instruction, see what objtool is doing and get function
  validation information. An output example is shown below.

- Add the --disas <function> option to disassemble functions

  Disassembly is done using libopcodes and it will show the different code
  alternatives.

Note: some changes are architecture specific (x86, powerpc, loongarch). Any
feedback about the behavior on powerpc and loongarch is welcome.

Thanks,

alex.


-----

Examples:
=========

Example 1 (--trace option): Trace the validation of the os_save() function
--------------------------------------------------------------------------

$ ./tools/objtool/objtool --hacks=jump_label --hacks=noinstr --hacks=skylake --ibt --orc --retpoline --rethunk --sls --static-call --uaccess --prefix=16 --link --trace os_xsave -v vmlinux.o
os_xsave: validation begin
 65c20: os_xsave+0x0  push %r12				- state: cfa=rsp+16 r12=(cfa-16) stack_size=16 
 65c22: os_xsave+0x2  mov 0x0(%rip),%eax # alternatives_patched 
 65c28: os_xsave+0x8  push %rbp 			- state: cfa=rsp+24 rbp=(cfa-24) stack_size=24 
 65c29: os_xsave+0x9  mov %rdi,%rbp 
 65c2c: os_xsave+0xc  push %rbx 			- state: cfa=rsp+32 rbx=(cfa-32) stack_size=32 
 65c2d: os_xsave+0xd  mov 0x8(%rdi),%rbx 
 65c31: os_xsave+0x11 mov %rbx,%r12 
 65c34: os_xsave+0x14 shr $0x20,%r12 
 65c38: os_xsave+0x18 test %eax,%eax 
 65c3a: os_xsave+0x1a je 65c6a <os_xsave+0x4a> 		- jump taken
 65c6a: os_xsave+0x4a | ud2 
 65c6c: os_xsave+0x4c | jmp 65c3c <os_xsave+0x1c> 	- unconditional jump
 65c3c: os_xsave+0x1c | xor %edx,%edx 
 65c3e: os_xsave+0x1e | mov %rbx,%rsi 
 65c41: os_xsave+0x21 | mov %rbp,%rdi 
 65c44: os_xsave+0x24 | callq xfd_validate_state 	- call
 65c49: os_xsave+0x29 | mov %ebx,%eax 
 65c4b: os_xsave+0x2b | mov %r12d,%edx 
 65c4e: os_xsave+0x2e | <alternative.65c4e> alt 1/4 begin
 65c55: os_xsave+0x35 | | test %ebx,%ebx 
 65c57: os_xsave+0x37 | | jne 65c6e <os_xsave+0x4e> 	- jump taken
 65c6e: os_xsave+0x4e | | | ud2 
 65c70: os_xsave+0x50 | | | pop %rbx 			- state: cfa=rsp+24 rbx=<undef> stack_size=24 
 65c71: os_xsave+0x51 | | | pop %rbp 			- state: cfa=rsp+16 rbp=<undef> stack_size=16 
 65c72: os_xsave+0x52 | | | pop %r12 			- state: cfa=rsp+8 r12=<undef> stack_size=8 
 65c74: os_xsave+0x54 | | | xor %eax,%eax 
 65c76: os_xsave+0x56 | | | xor %edx,%edx 
 65c78: os_xsave+0x58 | | | xor %esi,%esi 
 65c7a: os_xsave+0x5a | | | xor %edi,%edi 
 65c7c: os_xsave+0x5c | | | jmpq __x86_return_thunk 	- return
 65c57: os_xsave+0x37 | | jne 65c6e <os_xsave+0x4e> 	- jump not taken
 65c59: os_xsave+0x39 | | pop %rbx 			- state: cfa=rsp+24 rbx=<undef> stack_size=24 
 65c5a: os_xsave+0x3a | | pop %rbp 			- state: cfa=rsp+16 rbp=<undef> stack_size=16 
 65c5b: os_xsave+0x3b | | pop %r12 			- state: cfa=rsp+8 r12=<undef> stack_size=8 
 65c5d: os_xsave+0x3d | | xor %eax,%eax 
 65c5f: os_xsave+0x3f | | xor %edx,%edx 
 65c61: os_xsave+0x41 | | xor %esi,%esi 
 65c63: os_xsave+0x43 | | xor %edi,%edi 
 65c65: os_xsave+0x45 | | jmpq __x86_return_thunk - return
                      | <alternative.65c4e> alt 1/4 end
 65c4e: os_xsave+0x2e | <alternative.65c4e> alt 2/4 begin
 1c3d: .altinstr_replacement+0x1c3d | | xsaves64 0x40(%rbp) 
 65c53: os_xsave+0x33 | | xor %ebx,%ebx 
 65c55: os_xsave+0x35 | | test %ebx,%ebx - already visited
                      | <alternative.65c4e> alt 2/4 end
 65c4e: os_xsave+0x2e | <alternative.65c4e> alt 3/4 begin
 1c38: .altinstr_replacement+0x1c38 | | xsavec64 0x40(%rbp) 
 65c53: os_xsave+0x33 | | xor %ebx,%ebx - already visited
                      | <alternative.65c4e> alt 3/4 end
 65c4e: os_xsave+0x2e | <alternative.65c4e> alt 4/4 begin
 1c33: .altinstr_replacement+0x1c33 | | xsaveopt64 0x40(%rbp) 
 65c53: os_xsave+0x33 | | xor %ebx,%ebx - already visited
                      | <alternative.65c4e> alt 4/4 end
 65c4e: os_xsave+0x2e | <alternative.65c4e> alt default
 65c4e: os_xsave+0x2e | xsave64 0x40(%rbp) 
 65c53: os_xsave+0x33 | xor %ebx,%ebx - already visited
 65c3a: os_xsave+0x1a je 65c6a <os_xsave+0x4a> - jump not taken
 65c3c: os_xsave+0x1c xor %edx,%edx - already visited
os_xsave: validation end


Example 2 (--disas option): Disassemble perf_get_x86_pmu_capability()
---------------------------------------------------------------------

$ ./tools/objtool/objtool --disas=perf_get_x86_pmu_capability --link vmlinux.o
perf_get_x86_pmu_capability:
  d000:  perf_get_x86_pmu_capability         endbr64                                                   
  d004:  perf_get_x86_pmu_capability+0x4     callq  __fentry__                                         
  d009:  perf_get_x86_pmu_capability+0x9     mov    %rdi,%rdx                                          
                                             <alternative.d00c> default - begin
  d00c:  perf_get_x86_pmu_capability+0xc     | jmpq   .altinstr_aux+0x90                               
                                             <alternative.d00c> default - end
                                             <alternative.d00c> 1/2 - begin
                                             | <fake nop> (5 bytes)                                    
                                             <alternative.d00c> 1/2 end
                                             <alternative.d00c> 2/2 - begin
   5e5:  .altinstr_replacement+0x5e5         | jmpq   perf_get_x86_pmu_capability+0x3f                 
                                             <alternative.d00c> 2/2 end
  d011:  perf_get_x86_pmu_capability+0x11    ud2                                                       
  d013:  perf_get_x86_pmu_capability+0x13    movq   $0x0,(%rdx)                                        
  d01a:  perf_get_x86_pmu_capability+0x1a    movq   $0x0,0x8(%rdx)                                     
  d022:  perf_get_x86_pmu_capability+0x22    movq   $0x0,0x10(%rdx)                                    
  d02a:  perf_get_x86_pmu_capability+0x2a    movq   $0x0,0x18(%rdx)                                    
  d032:  perf_get_x86_pmu_capability+0x32    xor    %eax,%eax                                          
  d034:  perf_get_x86_pmu_capability+0x34    xor    %edx,%edx                                          
  d036:  perf_get_x86_pmu_capability+0x36    xor    %ecx,%ecx                                          
  d038:  perf_get_x86_pmu_capability+0x38    xor    %edi,%edi                                          
  d03a:  perf_get_x86_pmu_capability+0x3a    jmpq   __x86_return_thunk                                 
  d03f:  perf_get_x86_pmu_capability+0x3f    cmpq   $0x0,0x0(%rip)        # x86_pmu+0x10               
  d047:  perf_get_x86_pmu_capability+0x47    je     d013 <perf_get_x86_pmu_capability+0x13>            
  d049:  perf_get_x86_pmu_capability+0x49    mov    0x0(%rip),%eax        # x86_pmu+0x8                
  d04f:  perf_get_x86_pmu_capability+0x4f    mov    %eax,(%rdi)                                        
                                             <jump alternative.d051> default
  d051:  perf_get_x86_pmu_capability+0x51    | xchg   %ax,%ax                                          
                                             <jump alternative.d051> else
  d051:  perf_get_x86_pmu_capability+0x51    | jmp    d053 <perf_get_x86_pmu_capability+0x53>
                                             <jump alternative.d051> end
  d053:  perf_get_x86_pmu_capability+0x53    mov    0x0(%rip),%rdi        # x86_pmu+0xd8               
                                             <alternative.d05a> default - begin
  d05a:  perf_get_x86_pmu_capability+0x5a    | callq  __sw_hweight64                                   
                                             <alternative.d05a> default - end
                                             <alternative.d05a> 1/1 - begin
   5ea:  .altinstr_replacement+0x5ea         | popcnt %rdi,%rax                                        
                                             <alternative.d05a> 1/1 end
  d05f:  perf_get_x86_pmu_capability+0x5f    mov    %eax,0x4(%rdx)                                     
                                             <jump alternative.d062> default
  d062:  perf_get_x86_pmu_capability+0x62    | xchg   %ax,%ax                                          
                                             <jump alternative.d062> else
  d062:  perf_get_x86_pmu_capability+0x62    | jmp    d064 <perf_get_x86_pmu_capability+0x64>
                                             <jump alternative.d062> end
  d064:  perf_get_x86_pmu_capability+0x64    mov    0x0(%rip),%rdi        # x86_pmu+0xe0               
                                             <alternative.d06b> default - begin
  d06b:  perf_get_x86_pmu_capability+0x6b    | callq  __sw_hweight64                                   
                                             <alternative.d06b> default - end
                                             <alternative.d06b> 1/1 - begin
   5ef:  .altinstr_replacement+0x5ef         | popcnt %rdi,%rax                                        
                                             <alternative.d06b> 1/1 end
  d070:  perf_get_x86_pmu_capability+0x70    mov    %eax,0x8(%rdx)                                     
  d073:  perf_get_x86_pmu_capability+0x73    mov    0x0(%rip),%eax        # x86_pmu+0xf8               
  d079:  perf_get_x86_pmu_capability+0x79    mov    %eax,0xc(%rdx)                                     
  d07c:  perf_get_x86_pmu_capability+0x7c    mov    %eax,0x10(%rdx)                                    
  d07f:  perf_get_x86_pmu_capability+0x7f    mov    0x0(%rip),%rax        # x86_pmu+0x108              
  d086:  perf_get_x86_pmu_capability+0x86    mov    %eax,0x14(%rdx)                                    
  d089:  perf_get_x86_pmu_capability+0x89    mov    0x0(%rip),%eax        # x86_pmu+0x110              
  d08f:  perf_get_x86_pmu_capability+0x8f    mov    %eax,0x18(%rdx)                                    
  d092:  perf_get_x86_pmu_capability+0x92    movzbl 0x0(%rip),%ecx        # x86_pmu+0x1d1              
  d099:  perf_get_x86_pmu_capability+0x99    movzbl 0x1c(%rdx),%eax                                    
  d09d:  perf_get_x86_pmu_capability+0x9d    shr    %cl                                                
  d09f:  perf_get_x86_pmu_capability+0x9f    and    $0x1,%ecx                                          
  d0a2:  perf_get_x86_pmu_capability+0xa2    and    $0xfffffffe,%eax                                   
  d0a5:  perf_get_x86_pmu_capability+0xa5    or     %ecx,%eax                                          
  d0a7:  perf_get_x86_pmu_capability+0xa7    mov    %al,0x1c(%rdx)                                     
  d0aa:  perf_get_x86_pmu_capability+0xaa    xor    %eax,%eax                                          
  d0ac:  perf_get_x86_pmu_capability+0xac    xor    %edx,%edx                                          
  d0ae:  perf_get_x86_pmu_capability+0xae    xor    %ecx,%ecx                                          
  d0b0:  perf_get_x86_pmu_capability+0xb0    xor    %edi,%edi                                          
  d0b2:  perf_get_x86_pmu_capability+0xb2    jmpq   __x86_return_thunk                                 


-----

Alexandre Chartre (17):
  objtool: Move disassembly functions to a separated file
  objtool: Create disassembly context
  objtool: Disassemble code with libopcodes instead of running objdump
  tool build: Remove annoying newline in build output
  objtool: Print symbol during disassembly
  objtool: Improve offstr() output
  objtool: Store instruction disassembly result
  objtool: Disassemble instruction on warning or backtrace
  objtool: Extract code to validate instruction from the validate branch
    loop
  objtool: Record symbol name max length
  objtool: Add option to trace function validation
  objtool: Trace instruction state changes during function validation
  objtool: Improve register reporting during function validation
  objtool: Improve tracing of alternative instructions
  objtool: Do not validate IBT for .return_sites and .call_sites
  objtool: Add the --disas=<function-pattern> action
  objtool: Disassemble all alternatives when using --disas

 tools/build/Makefile.feature            |   4 +-
 tools/objtool/Build                     |   3 +
 tools/objtool/Makefile                  |  22 +
 tools/objtool/arch/loongarch/decode.c   |  30 ++
 tools/objtool/arch/powerpc/decode.c     |  31 ++
 tools/objtool/arch/x86/decode.c         |  34 ++
 tools/objtool/builtin-check.c           |   5 +-
 tools/objtool/check.c                   | 687 ++++++++++++++----------
 tools/objtool/disas.c                   | 569 ++++++++++++++++++++
 tools/objtool/include/objtool/arch.h    |  13 +
 tools/objtool/include/objtool/builtin.h |   2 +
 tools/objtool/include/objtool/check.h   |  32 +-
 tools/objtool/include/objtool/disas.h   |  69 +++
 tools/objtool/include/objtool/trace.h   | 111 ++++
 tools/objtool/include/objtool/warn.h    |  33 +-
 tools/objtool/trace.c                   | 147 +++++
 16 files changed, 1489 insertions(+), 303 deletions(-)
 create mode 100644 tools/objtool/disas.c
 create mode 100644 tools/objtool/include/objtool/disas.h
 create mode 100644 tools/objtool/include/objtool/trace.h
 create mode 100644 tools/objtool/trace.c

-- 
2.43.5


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 01/17] objtool: Move disassembly functions to a separated file
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 02/17] objtool: Create disassembly context Alexandre Chartre
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

objtool disassembles functions which have warnings. Move the code
to do that to a dedicated file. The code is just moved, it is not
changed.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/Build                     |  1 +
 tools/objtool/check.c                   | 81 ----------------------
 tools/objtool/disas.c                   | 90 +++++++++++++++++++++++++
 tools/objtool/include/objtool/objtool.h |  2 +
 4 files changed, 93 insertions(+), 81 deletions(-)
 create mode 100644 tools/objtool/disas.c

diff --git a/tools/objtool/Build b/tools/objtool/Build
index a3cdf8af6635..677bf9148cba 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -7,6 +7,7 @@ objtool-y += special.o
 objtool-y += builtin-check.o
 objtool-y += elf.o
 objtool-y += objtool.o
+objtool-y += disas.o
 
 objtool-$(BUILD_ORC) += orc_gen.o
 objtool-$(BUILD_ORC) += orc_dump.o
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f23bdda737aa..bd1974717fa3 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4561,87 +4561,6 @@ static int validate_reachable_instructions(struct objtool_file *file)
 	return warnings;
 }
 
-/* 'funcs' is a space-separated list of function names */
-static void disas_funcs(const char *funcs)
-{
-	const char *objdump_str, *cross_compile;
-	int size, ret;
-	char *cmd;
-
-	cross_compile = getenv("CROSS_COMPILE");
-	if (!cross_compile)
-		cross_compile = "";
-
-	objdump_str = "%sobjdump -wdr %s | gawk -M -v _funcs='%s' '"
-			"BEGIN { split(_funcs, funcs); }"
-			"/^$/ { func_match = 0; }"
-			"/<.*>:/ { "
-				"f = gensub(/.*<(.*)>:/, \"\\\\1\", 1);"
-				"for (i in funcs) {"
-					"if (funcs[i] == f) {"
-						"func_match = 1;"
-						"base = strtonum(\"0x\" $1);"
-						"break;"
-					"}"
-				"}"
-			"}"
-			"{"
-				"if (func_match) {"
-					"addr = strtonum(\"0x\" $1);"
-					"printf(\"%%04x \", addr - base);"
-					"print;"
-				"}"
-			"}' 1>&2";
-
-	/* fake snprintf() to calculate the size */
-	size = snprintf(NULL, 0, objdump_str, cross_compile, objname, funcs) + 1;
-	if (size <= 0) {
-		WARN("objdump string size calculation failed");
-		return;
-	}
-
-	cmd = malloc(size);
-
-	/* real snprintf() */
-	snprintf(cmd, size, objdump_str, cross_compile, objname, funcs);
-	ret = system(cmd);
-	if (ret) {
-		WARN("disassembly failed: %d", ret);
-		return;
-	}
-}
-
-static void disas_warned_funcs(struct objtool_file *file)
-{
-	struct symbol *sym;
-	char *funcs = NULL, *tmp;
-
-	for_each_sym(file, sym) {
-		if (sym->warned) {
-			if (!funcs) {
-				funcs = malloc(strlen(sym->name) + 1);
-				if (!funcs) {
-					ERROR_GLIBC("malloc");
-					return;
-				}
-				strcpy(funcs, sym->name);
-			} else {
-				tmp = malloc(strlen(funcs) + strlen(sym->name) + 2);
-				if (!tmp) {
-					ERROR_GLIBC("malloc");
-					return;
-				}
-				sprintf(tmp, "%s %s", funcs, sym->name);
-				free(funcs);
-				funcs = tmp;
-			}
-		}
-	}
-
-	if (funcs)
-		disas_funcs(funcs);
-}
-
 struct insn_chunk {
 	void *addr;
 	struct insn_chunk *next;
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
new file mode 100644
index 000000000000..77de46beb496
--- /dev/null
+++ b/tools/objtool/disas.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2015-2017 Josh Poimboeuf <jpoimboe@redhat.com>
+ */
+
+#include <objtool/arch.h>
+#include <objtool/warn.h>
+
+#include <linux/string.h>
+
+/* 'funcs' is a space-separated list of function names */
+static void disas_funcs(const char *funcs)
+{
+	const char *objdump_str, *cross_compile;
+	int size, ret;
+	char *cmd;
+
+	cross_compile = getenv("CROSS_COMPILE");
+	if (!cross_compile)
+		cross_compile = "";
+
+	objdump_str = "%sobjdump -wdr %s | gawk -M -v _funcs='%s' '"
+			"BEGIN { split(_funcs, funcs); }"
+			"/^$/ { func_match = 0; }"
+			"/<.*>:/ { "
+				"f = gensub(/.*<(.*)>:/, \"\\\\1\", 1);"
+				"for (i in funcs) {"
+					"if (funcs[i] == f) {"
+						"func_match = 1;"
+						"base = strtonum(\"0x\" $1);"
+						"break;"
+					"}"
+				"}"
+			"}"
+			"{"
+				"if (func_match) {"
+					"addr = strtonum(\"0x\" $1);"
+					"printf(\"%%04x \", addr - base);"
+					"print;"
+				"}"
+			"}' 1>&2";
+
+	/* fake snprintf() to calculate the size */
+	size = snprintf(NULL, 0, objdump_str, cross_compile, objname, funcs) + 1;
+	if (size <= 0) {
+		WARN("objdump string size calculation failed");
+		return;
+	}
+
+	cmd = malloc(size);
+
+	/* real snprintf() */
+	snprintf(cmd, size, objdump_str, cross_compile, objname, funcs);
+	ret = system(cmd);
+	if (ret) {
+		WARN("disassembly failed: %d", ret);
+		return;
+	}
+}
+
+void disas_warned_funcs(struct objtool_file *file)
+{
+	struct symbol *sym;
+	char *funcs = NULL, *tmp;
+
+	for_each_sym(file, sym) {
+		if (sym->warned) {
+			if (!funcs) {
+				funcs = malloc(strlen(sym->name) + 1);
+				if (!funcs) {
+					ERROR_GLIBC("malloc");
+					return;
+				}
+				strcpy(funcs, sym->name);
+			} else {
+				tmp = malloc(strlen(funcs) + strlen(sym->name) + 2);
+				if (!tmp) {
+					ERROR_GLIBC("malloc");
+					return;
+				}
+				sprintf(tmp, "%s %s", funcs, sym->name);
+				free(funcs);
+				funcs = tmp;
+			}
+		}
+	}
+
+	if (funcs)
+		disas_funcs(funcs);
+}
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index c0dc86a78ff6..4d3e94b70fd8 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -47,4 +47,6 @@ int check(struct objtool_file *file);
 int orc_dump(const char *objname);
 int orc_create(struct objtool_file *file);
 
+void disas_warned_funcs(struct objtool_file *file);
+
 #endif /* _OBJTOOL_H */
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 02/17] objtool: Create disassembly context
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 01/17] objtool: Move disassembly functions to a separated file Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 03/17] objtool: Disassemble code with libopcodes instead of running objdump Alexandre Chartre
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

Create a structure to store information for disassembling functions.
For now, it is just a wrapper around an objtool file.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c                   |  6 ++++-
 tools/objtool/disas.c                   | 32 +++++++++++++++++++++++--
 tools/objtool/include/objtool/disas.h   | 14 +++++++++++
 tools/objtool/include/objtool/objtool.h |  2 --
 4 files changed, 49 insertions(+), 5 deletions(-)
 create mode 100644 tools/objtool/include/objtool/disas.h

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index bd1974717fa3..cef3a32a6747 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -11,6 +11,7 @@
 #include <objtool/builtin.h>
 #include <objtool/cfi.h>
 #include <objtool/arch.h>
+#include <objtool/disas.h>
 #include <objtool/check.h>
 #include <objtool/special.h>
 #include <objtool/warn.h>
@@ -4591,6 +4592,7 @@ static void free_insns(struct objtool_file *file)
 
 int check(struct objtool_file *file)
 {
+	struct disas_context *disas_ctx;
 	int ret = 0, warnings = 0;
 
 	arch_initial_func_cfi_state(&initial_func_cfi);
@@ -4720,7 +4722,9 @@ int check(struct objtool_file *file)
 		if (opts.werror && warnings)
 			WARN("%d warning(s) upgraded to errors", warnings);
 		print_args();
-		disas_warned_funcs(file);
+		disas_ctx = disas_context_create(file);
+		disas_warned_funcs(disas_ctx);
+		disas_context_destroy(disas_ctx);
 	}
 
 	return ret;
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 77de46beb496..67eff2a14832 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -4,10 +4,35 @@
  */
 
 #include <objtool/arch.h>
+#include <objtool/disas.h>
 #include <objtool/warn.h>
 
 #include <linux/string.h>
 
+struct disas_context {
+	struct objtool_file *file;
+};
+
+struct disas_context *disas_context_create(struct objtool_file *file)
+{
+	struct disas_context *dctx;
+
+	dctx = malloc(sizeof(*dctx));
+	if (!dctx) {
+		ERROR_GLIBC("failed to allocate disassembly context");
+		return NULL;
+	}
+
+	dctx->file = file;
+
+	return dctx;
+}
+
+void disas_context_destroy(struct disas_context *dctx)
+{
+	free(dctx);
+}
+
 /* 'funcs' is a space-separated list of function names */
 static void disas_funcs(const char *funcs)
 {
@@ -58,12 +83,15 @@ static void disas_funcs(const char *funcs)
 	}
 }
 
-void disas_warned_funcs(struct objtool_file *file)
+void disas_warned_funcs(struct disas_context *dctx)
 {
 	struct symbol *sym;
 	char *funcs = NULL, *tmp;
 
-	for_each_sym(file, sym) {
+	if (!dctx)
+		return;
+
+	for_each_sym(dctx->file, sym) {
 		if (sym->warned) {
 			if (!funcs) {
 				funcs = malloc(strlen(sym->name) + 1);
diff --git a/tools/objtool/include/objtool/disas.h b/tools/objtool/include/objtool/disas.h
new file mode 100644
index 000000000000..5c543b69fc61
--- /dev/null
+++ b/tools/objtool/include/objtool/disas.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates.
+ */
+
+#ifndef _DISAS_H
+#define _DISAS_H
+
+struct disas_context;
+struct disas_context *disas_context_create(struct objtool_file *file);
+void disas_context_destroy(struct disas_context *dctx);
+void disas_warned_funcs(struct disas_context *dctx);
+
+#endif /* _DISAS_H */
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index 4d3e94b70fd8..c0dc86a78ff6 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -47,6 +47,4 @@ int check(struct objtool_file *file);
 int orc_dump(const char *objname);
 int orc_create(struct objtool_file *file);
 
-void disas_warned_funcs(struct objtool_file *file);
-
 #endif /* _OBJTOOL_H */
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 03/17] objtool: Disassemble code with libopcodes instead of running objdump
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 01/17] objtool: Move disassembly functions to a separated file Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 02/17] objtool: Create disassembly context Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 04/17] tool build: Remove annoying newline in build output Alexandre Chartre
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

objtool executes the objdump command to disassemble code. Use libopcodes
instead to have more control about the disassembly scope and output.
If libopcodes is not present then objtool is built without disassembly
support.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/Build                   |   3 +-
 tools/objtool/Makefile                |  22 ++++
 tools/objtool/arch/loongarch/decode.c |  12 ++
 tools/objtool/arch/powerpc/decode.c   |  12 ++
 tools/objtool/arch/x86/decode.c       |  12 ++
 tools/objtool/check.c                 |  10 +-
 tools/objtool/disas.c                 | 183 ++++++++++++++++----------
 tools/objtool/include/objtool/arch.h  |   9 ++
 tools/objtool/include/objtool/check.h |   5 +
 tools/objtool/include/objtool/disas.h |  29 ++++
 10 files changed, 224 insertions(+), 73 deletions(-)

diff --git a/tools/objtool/Build b/tools/objtool/Build
index 677bf9148cba..ee04fba8c9d1 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -7,7 +7,8 @@ objtool-y += special.o
 objtool-y += builtin-check.o
 objtool-y += elf.o
 objtool-y += objtool.o
-objtool-y += disas.o
+
+objtool-$(BUILD_DISAS) += disas.o
 
 objtool-$(BUILD_ORC) += orc_gen.o
 objtool-$(BUILD_ORC) += orc_dump.o
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 8c20361dd100..1100501f4bce 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -7,6 +7,15 @@ srctree := $(patsubst %/,%,$(dir $(CURDIR)))
 srctree := $(patsubst %/,%,$(dir $(srctree)))
 endif
 
+#
+# To support disassembly, objtool needs libopcodes which is provided
+# with libbdf (binutils-dev or binutils-devel package).
+#
+FEATURE_USER = .objtool
+FEATURE_TESTS = libbfd disassembler-init-styled
+FEATURE_DISPLAY = libbfd disassembler-init-styled
+include $(srctree)/tools/build/Makefile.feature
+
 LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/
 ifneq ($(OUTPUT),)
   LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
@@ -40,6 +49,18 @@ OBJTOOL_LDFLAGS := $(LIBELF_LIBS) $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS)
 elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(HOSTCC) $(OBJTOOL_CFLAGS) -x c -E - 2>/dev/null | grep elf_getshdr)
 OBJTOOL_CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
 
+ifeq ($(feature-disassembler-init-styled), 1)
+	OBJTOOL_CFLAGS += -DDISASM_INIT_STYLED
+endif
+
+BUILD_DISAS := n
+
+ifeq ($(feature-libbfd),1)
+	BUILD_DISAS := y
+	OBJTOOL_CFLAGS += -DDISAS
+	OBJTOOL_LDFLAGS += -lopcodes
+endif
+
 # Always want host compilation.
 HOST_OVERRIDES := CC="$(HOSTCC)" LD="$(HOSTLD)" AR="$(HOSTAR)"
 
@@ -56,6 +77,7 @@ ifeq ($(SRCARCH),loongarch)
 	BUILD_ORC := y
 endif
 
+export BUILD_DISAS
 export BUILD_ORC
 export srctree OUTPUT CFLAGS SRCARCH AWK
 include $(srctree)/tools/build/Makefile.include
diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index b6fdc68053cc..5bf383e7e2e5 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 #include <string.h>
 #include <objtool/check.h>
+#include <objtool/disas.h>
 #include <objtool/warn.h>
 #include <asm/inst.h>
 #include <asm/orc_types.h>
@@ -387,3 +388,14 @@ unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *tabl
 		return reloc->sym->offset + reloc_addend(reloc);
 	}
 }
+
+#ifdef DISAS
+
+int arch_disas_info_init(struct disassemble_info *dinfo)
+{
+	return disas_info_init(dinfo, bfd_arch_loongarch,
+			       bfd_mach_loongarch32, bfd_mach_loongarch64,
+			       NULL);
+}
+
+#endif /* DISAS */
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index c851c51d4bd3..3c6fced37bcc 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -3,6 +3,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <objtool/check.h>
+#include <objtool/disas.h>
 #include <objtool/elf.h>
 #include <objtool/arch.h>
 #include <objtool/warn.h>
@@ -128,3 +129,14 @@ unsigned int arch_reloc_size(struct reloc *reloc)
 		return 8;
 	}
 }
+
+#ifdef DISAS
+
+int arch_disas_info_init(struct disassemble_info *dinfo)
+{
+	return disas_info_init(dinfo, bfd_arch_powerpc,
+			       bfd_mach_ppc, bfd_mach_ppc64,
+			       NULL);
+}
+
+#endif /* DISAS */
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 98c4713c1b09..3c85506f9414 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -16,6 +16,7 @@
 
 #include <asm/orc_types.h>
 #include <objtool/check.h>
+#include <objtool/disas.h>
 #include <objtool/elf.h>
 #include <objtool/arch.h>
 #include <objtool/warn.h>
@@ -880,3 +881,14 @@ unsigned int arch_reloc_size(struct reloc *reloc)
 		return 8;
 	}
 }
+
+#ifdef DISAS
+
+int arch_disas_info_init(struct disassemble_info *dinfo)
+{
+	return disas_info_init(dinfo, bfd_arch_i386,
+			       bfd_mach_i386_i386, bfd_mach_x86_64,
+			       "att");
+}
+
+#endif /* DISAS */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index cef3a32a6747..8ba69e0c273b 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4702,8 +4702,6 @@ int check(struct objtool_file *file)
 			goto out;
 	}
 
-	free_insns(file);
-
 	if (opts.stats) {
 		printf("nr_insns_visited: %ld\n", nr_insns_visited);
 		printf("nr_cfi: %ld\n", nr_cfi);
@@ -4723,9 +4721,13 @@ int check(struct objtool_file *file)
 			WARN("%d warning(s) upgraded to errors", warnings);
 		print_args();
 		disas_ctx = disas_context_create(file);
-		disas_warned_funcs(disas_ctx);
-		disas_context_destroy(disas_ctx);
+		if (disas_ctx) {
+			disas_warned_funcs(disas_ctx);
+			disas_context_destroy(disas_ctx);
+		}
 	}
 
+	free_insns(file);
+
 	return ret;
 }
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 67eff2a14832..9fead6281102 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -4,18 +4,53 @@
  */
 
 #include <objtool/arch.h>
+#include <objtool/check.h>
 #include <objtool/disas.h>
 #include <objtool/warn.h>
 
+#include <bfd.h>
 #include <linux/string.h>
+#include <tools/dis-asm-compat.h>
 
 struct disas_context {
 	struct objtool_file *file;
+	disassembler_ftype disassembler;
+	struct disassemble_info info;
 };
 
+/*
+ * Initialize disassemble info arch, mach (32 or 64-bit) and options.
+ */
+int disas_info_init(struct disassemble_info *dinfo,
+		    int arch, int mach32, int mach64,
+		    const char *options)
+{
+	struct disas_context *dctx = dinfo->application_data;
+	struct objtool_file *file = dctx->file;
+
+	dinfo->arch = arch;
+
+	switch (file->elf->ehdr.e_ident[EI_CLASS]) {
+	case ELFCLASS32:
+		dinfo->mach = mach32;
+		break;
+	case ELFCLASS64:
+		dinfo->mach = mach64;
+		break;
+	default:
+		return -1;
+	}
+
+	dinfo->disassembler_options = options;
+
+	return 0;
+}
+
 struct disas_context *disas_context_create(struct objtool_file *file)
 {
 	struct disas_context *dctx;
+	struct disassemble_info *dinfo;
+	int err;
 
 	dctx = malloc(sizeof(*dctx));
 	if (!dctx) {
@@ -24,8 +59,49 @@ struct disas_context *disas_context_create(struct objtool_file *file)
 	}
 
 	dctx->file = file;
+	dinfo = &dctx->info;
+
+	init_disassemble_info_compat(dinfo, stdout,
+				     (fprintf_ftype)fprintf,
+				     fprintf_styled);
+
+	dinfo->read_memory_func = buffer_read_memory;
+	dinfo->application_data = dctx;
+
+	/*
+	 * bfd_openr() is not used to avoid doing ELF data processing
+	 * and caching that has already being done. Here, we just need
+	 * to identify the target file so we call an arch specific
+	 * function to fill some disassemble info (arch, mach).
+	 */
+
+	dinfo->arch = bfd_arch_unknown;
+	dinfo->mach = 0;
+
+	err = arch_disas_info_init(dinfo);
+	if (err || dinfo->arch == bfd_arch_unknown || dinfo->mach == 0) {
+		WARN("failed to init disassembly arch");
+		goto error;
+	}
+
+	dinfo->endian = (file->elf->ehdr.e_ident[EI_DATA] == ELFDATA2MSB) ?
+		BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE;
+
+	disassemble_init_for_target(dinfo);
+
+	dctx->disassembler = disassembler(dinfo->arch,
+					  dinfo->endian == BFD_ENDIAN_BIG,
+					  dinfo->mach, NULL);
+	if (!dctx->disassembler) {
+		WARN("failed to create disassembler function");
+		goto error;
+	}
 
 	return dctx;
+
+error:
+	free(dctx);
+	return NULL;
 }
 
 void disas_context_destroy(struct disas_context *dctx)
@@ -33,86 +109,57 @@ void disas_context_destroy(struct disas_context *dctx)
 	free(dctx);
 }
 
-/* 'funcs' is a space-separated list of function names */
-static void disas_funcs(const char *funcs)
+/*
+ * Disassemble a single instruction. Return the size of the instruction.
+ */
+static size_t disas_insn(struct disas_context *dctx,
+			 struct instruction *insn)
 {
-	const char *objdump_str, *cross_compile;
-	int size, ret;
-	char *cmd;
-
-	cross_compile = getenv("CROSS_COMPILE");
-	if (!cross_compile)
-		cross_compile = "";
-
-	objdump_str = "%sobjdump -wdr %s | gawk -M -v _funcs='%s' '"
-			"BEGIN { split(_funcs, funcs); }"
-			"/^$/ { func_match = 0; }"
-			"/<.*>:/ { "
-				"f = gensub(/.*<(.*)>:/, \"\\\\1\", 1);"
-				"for (i in funcs) {"
-					"if (funcs[i] == f) {"
-						"func_match = 1;"
-						"base = strtonum(\"0x\" $1);"
-						"break;"
-					"}"
-				"}"
-			"}"
-			"{"
-				"if (func_match) {"
-					"addr = strtonum(\"0x\" $1);"
-					"printf(\"%%04x \", addr - base);"
-					"print;"
-				"}"
-			"}' 1>&2";
-
-	/* fake snprintf() to calculate the size */
-	size = snprintf(NULL, 0, objdump_str, cross_compile, objname, funcs) + 1;
-	if (size <= 0) {
-		WARN("objdump string size calculation failed");
-		return;
-	}
-
-	cmd = malloc(size);
+	disassembler_ftype disasm = dctx->disassembler;
+	struct disassemble_info *dinfo = &dctx->info;
+
+	/*
+	 * Set the disassembler buffer to read data from the section
+	 * containing the instruction to disassemble.
+	 */
+	dinfo->buffer = insn->sec->data->d_buf;
+	dinfo->buffer_vma = 0;
+	dinfo->buffer_length = insn->sec->sh.sh_size;
+
+	return disasm(insn->offset, &dctx->info);
+}
 
-	/* real snprintf() */
-	snprintf(cmd, size, objdump_str, cross_compile, objname, funcs);
-	ret = system(cmd);
-	if (ret) {
-		WARN("disassembly failed: %d", ret);
-		return;
+/*
+ * Disassemble a function.
+ */
+static void disas_func(struct disas_context *dctx, struct symbol *func)
+{
+	struct instruction *insn;
+	size_t addr;
+
+	printf("%s:\n", func->name);
+	sym_for_each_insn(dctx->file, func, insn) {
+		addr = insn->offset;
+		printf(" %6lx:  %s+0x%-6lx      ",
+		       addr, func->name, addr - func->offset);
+		disas_insn(dctx, insn);
+		printf("\n");
 	}
+	printf("\n");
 }
 
+/*
+ * Disassemble all warned functions.
+ */
 void disas_warned_funcs(struct disas_context *dctx)
 {
 	struct symbol *sym;
-	char *funcs = NULL, *tmp;
 
 	if (!dctx)
 		return;
 
 	for_each_sym(dctx->file, sym) {
-		if (sym->warned) {
-			if (!funcs) {
-				funcs = malloc(strlen(sym->name) + 1);
-				if (!funcs) {
-					ERROR_GLIBC("malloc");
-					return;
-				}
-				strcpy(funcs, sym->name);
-			} else {
-				tmp = malloc(strlen(funcs) + strlen(sym->name) + 2);
-				if (!tmp) {
-					ERROR_GLIBC("malloc");
-					return;
-				}
-				sprintf(tmp, "%s %s", funcs, sym->name);
-				free(funcs);
-				funcs = tmp;
-			}
-		}
+		if (sym->warned)
+			disas_func(dctx, sym);
 	}
-
-	if (funcs)
-		disas_funcs(funcs);
 }
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 01ef6f415adf..19b1dec2db15 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -101,4 +101,13 @@ bool arch_pc_relative_reloc(struct reloc *reloc);
 unsigned int arch_reloc_size(struct reloc *reloc);
 unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table);
 
+#ifdef DISAS
+
+#include <bfd.h>
+#include <dis-asm.h>
+
+int arch_disas_info_init(struct disassemble_info *dinfo);
+
+#endif /* DISAS */
+
 #endif /* _ARCH_H */
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index 00fb745e7233..5290ac1ebbc1 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -125,4 +125,9 @@ struct instruction *next_insn_same_sec(struct objtool_file *file, struct instruc
 	     insn && insn->sec == _sec;					\
 	     insn = next_insn_same_sec(file, insn))
 
+#define sym_for_each_insn(file, sym, insn)				\
+	for (insn = find_insn(file, sym->sec, sym->offset);		\
+	     insn && insn->offset < sym->offset + sym->len;		\
+	     insn = next_insn_same_sec(file, insn))
+
 #endif /* _CHECK_H */
diff --git a/tools/objtool/include/objtool/disas.h b/tools/objtool/include/objtool/disas.h
index 5c543b69fc61..3ec3ce2e4e6f 100644
--- a/tools/objtool/include/objtool/disas.h
+++ b/tools/objtool/include/objtool/disas.h
@@ -7,8 +7,37 @@
 #define _DISAS_H
 
 struct disas_context;
+struct disassemble_info;
+
+#ifdef DISAS
+
 struct disas_context *disas_context_create(struct objtool_file *file);
 void disas_context_destroy(struct disas_context *dctx);
 void disas_warned_funcs(struct disas_context *dctx);
+int disas_info_init(struct disassemble_info *dinfo,
+		    int arch, int mach32, int mach64,
+		    const char *options);
+
+#else /* DISAS */
+
+#include <objtool/warn.h>
+
+static inline struct disas_context *disas_context_create(struct objtool_file *file)
+{
+	WARN("Rebuild with libopcodes for disassembly support");
+	return NULL;
+}
+
+static inline void disas_context_destroy(struct disas_context *dctx) {}
+static inline void disas_warned_funcs(struct disas_context *dctx) {}
+
+static inline int disas_info_init(struct disassemble_info *dinfo,
+				  int arch, int mach32, int mach64,
+				  const char *options)
+{
+	return -1;
+}
+
+#endif /* DISAS */
 
 #endif /* _DISAS_H */
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 04/17] tool build: Remove annoying newline in build output
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (2 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 03/17] objtool: Disassemble code with libopcodes instead of running objdump Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 05/17] objtool: Print symbol during disassembly Alexandre Chartre
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

Remove the newline which is printed during feature discovery
when nothing else is printed.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/build/Makefile.feature | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 57bd995ce6af..3e45164df894 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -326,5 +326,7 @@ endef
 
 ifeq ($(FEATURE_DISPLAY_DEFERRED),)
   $(call feature_display_entries)
-  $(info )
+  ifeq ($(feature_display),1)
+    $(info )
+  endif
 endif
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 05/17] objtool: Print symbol during disassembly
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (3 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 04/17] tool build: Remove annoying newline in build output Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 06/17] objtool: Improve offstr() output Alexandre Chartre
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

Print symbols referenced during disassembly instead of just printing
raw addresses. Also handle address relocation.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/arch/loongarch/decode.c |  7 +++
 tools/objtool/arch/powerpc/decode.c   |  7 +++
 tools/objtool/arch/x86/decode.c       | 14 +++++
 tools/objtool/check.c                 |  9 ---
 tools/objtool/disas.c                 | 89 +++++++++++++++++++++++++++
 tools/objtool/include/objtool/arch.h  |  2 +
 tools/objtool/include/objtool/check.h |  9 +++
 tools/objtool/include/objtool/warn.h  | 12 ++++
 8 files changed, 140 insertions(+), 9 deletions(-)

diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index 5bf383e7e2e5..09c43f008c9a 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -28,6 +28,13 @@ bool arch_pc_relative_reloc(struct reloc *reloc)
 	return false;
 }
 
+unsigned long arch_pc_relative_offset(struct instruction *insn,
+				      struct reloc *reloc)
+{
+	/* no PC relative relocation */
+	return 0;
+}
+
 bool arch_callee_saved_reg(unsigned char reg)
 {
 	switch (reg) {
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 3c6fced37bcc..9c3f49c45587 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -20,6 +20,13 @@ unsigned long arch_dest_reloc_offset(int addend)
 	return addend;
 }
 
+unsigned long arch_pc_relative_offset(struct instruction *insn,
+				      struct reloc *reloc)
+{
+	/* no PC relative relocation */
+	return 0;
+}
+
 bool arch_callee_saved_reg(unsigned char reg)
 {
 	return false;
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 3c85506f9414..6a39cc619d63 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -103,6 +103,20 @@ bool arch_pc_relative_reloc(struct reloc *reloc)
 	return false;
 }
 
+unsigned long arch_pc_relative_offset(struct instruction *insn,
+				      struct reloc *reloc)
+{
+	/*
+	 * Relocation information for a RIP-relative instruction is
+	 * based on the RIP value at the end of the instruction. So
+	 * to get the effective relocated address, the reference has
+	 * to be adjusted with the number of bytes between the
+	 * relocation offset and the end of the instruction.
+	 */
+	return reloc_addend(reloc) +
+		insn->offset + insn->len - reloc_offset(reloc);
+}
+
 #define ADD_OP(op) \
 	if (!(op = calloc(1, sizeof(*op)))) \
 		return -1; \
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8ba69e0c273b..a4d0a6c62bc0 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -132,15 +132,6 @@ static struct instruction *prev_insn_same_sym(struct objtool_file *file,
 	for (insn = next_insn_same_sec(file, insn); insn;		\
 	     insn = next_insn_same_sec(file, insn))
 
-static inline struct symbol *insn_call_dest(struct instruction *insn)
-{
-	if (insn->type == INSN_JUMP_DYNAMIC ||
-	    insn->type == INSN_CALL_DYNAMIC)
-		return NULL;
-
-	return insn->_call_dest;
-}
-
 static inline struct reloc *insn_jump_table(struct instruction *insn)
 {
 	if (insn->type == INSN_JUMP_DYNAMIC ||
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 9fead6281102..91d23f7518e3 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -14,10 +14,96 @@
 
 struct disas_context {
 	struct objtool_file *file;
+	struct instruction *insn;
 	disassembler_ftype disassembler;
 	struct disassemble_info info;
 };
 
+#define DINFO_FPRINTF(dinfo, ...)	\
+	((*(dinfo)->fprintf_func)((dinfo)->stream, __VA_ARGS__))
+
+static void disas_print_address(bfd_vma addr, struct disassemble_info *dinfo)
+{
+	struct disas_context *dctx = dinfo->application_data;
+	struct instruction *insn = dctx->insn;
+	struct objtool_file *file = dctx->file;
+	struct instruction *jump_dest;
+	struct symbol *call_dest;
+	unsigned long offset;
+	struct reloc *reloc;
+	bool is_reloc;
+	char symstr[1024];
+	char *str;
+
+	/*
+	 * If the instruction is a call/jump and it references a
+	 * destination then this is likely the address we are looking
+	 * up. So check it first.
+	 */
+	jump_dest = insn->jump_dest;
+	if (jump_dest && jump_dest->sym && jump_dest->offset == addr) {
+		sprint_name(symstr, jump_dest->sym->name, jump_dest->offset - jump_dest->sym->offset);
+		DINFO_FPRINTF(dinfo, "%lx <%s>", addr, symstr);
+		return;
+	}
+
+	/*
+	 * Assume the address is a relocation if it points to the next
+	 * instruction.
+	 */
+	is_reloc = (addr == insn->offset + insn->len);
+
+	/*
+	 * The call destination offset can be the address we are looking
+	 * up, or 0 if there is a relocation.
+	 */
+	call_dest = insn_call_dest(insn);
+	if (call_dest) {
+		if (call_dest->offset == addr) {
+			DINFO_FPRINTF(dinfo, "%lx <%s>", addr, call_dest->name);
+			return;
+		}
+		if (call_dest->offset == 0 && is_reloc) {
+			DINFO_FPRINTF(dinfo, "%s", call_dest->name);
+			return;
+		}
+	}
+
+	if (!is_reloc) {
+		DINFO_FPRINTF(dinfo, "0x%lx", addr);
+		return;
+	}
+
+	/*
+	 * If this is a relocation, check if we have relocation information
+	 * for this instruction.
+	 */
+	reloc = find_reloc_by_dest_range(file->elf, insn->sec,
+					 insn->offset, insn->len);
+	if (!reloc) {
+		DINFO_FPRINTF(dinfo, "0x%lx", addr);
+		return;
+	}
+
+	if (arch_pc_relative_reloc(reloc))
+		offset = arch_pc_relative_offset(insn, reloc);
+	else
+		offset = reloc_addend(reloc);
+
+	/*
+	 * If the relocation symbol is a section name (for example ".bss")
+	 * then we try to further resolve the name.
+	 */
+	if (reloc->sym->type == STT_SECTION) {
+		str = offstr(reloc->sym->sec, reloc->sym->offset + offset);
+		DINFO_FPRINTF(dinfo, "%s", str);
+		free(str);
+	} else {
+		sprint_name(symstr, reloc->sym->name, offset);
+		DINFO_FPRINTF(dinfo, "%s", symstr);
+	}
+}
+
 /*
  * Initialize disassemble info arch, mach (32 or 64-bit) and options.
  */
@@ -66,6 +152,7 @@ struct disas_context *disas_context_create(struct objtool_file *file)
 				     fprintf_styled);
 
 	dinfo->read_memory_func = buffer_read_memory;
+	dinfo->print_address_func = disas_print_address;
 	dinfo->application_data = dctx;
 
 	/*
@@ -118,6 +205,8 @@ static size_t disas_insn(struct disas_context *dctx,
 	disassembler_ftype disasm = dctx->disassembler;
 	struct disassemble_info *dinfo = &dctx->info;
 
+	dctx->insn = insn;
+
 	/*
 	 * Set the disassembler buffer to read data from the section
 	 * containing the instruction to disassemble.
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 19b1dec2db15..baa6eee1977f 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -97,6 +97,8 @@ bool arch_is_embedded_insn(struct symbol *sym);
 int arch_rewrite_retpolines(struct objtool_file *file);
 
 bool arch_pc_relative_reloc(struct reloc *reloc);
+unsigned long arch_pc_relative_offset(struct instruction *insn,
+				      struct reloc *reloc);
 
 unsigned int arch_reloc_size(struct reloc *reloc);
 unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table);
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index 5290ac1ebbc1..4adbcd760c6f 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -115,6 +115,15 @@ static inline bool is_jump(struct instruction *insn)
 	return is_static_jump(insn) || is_dynamic_jump(insn);
 }
 
+static inline struct symbol *insn_call_dest(struct instruction *insn)
+{
+	if (insn->type == INSN_JUMP_DYNAMIC ||
+	    insn->type == INSN_CALL_DYNAMIC)
+		return NULL;
+
+	return insn->_call_dest;
+}
+
 struct instruction *find_insn(struct objtool_file *file,
 			      struct section *sec, unsigned long offset);
 
diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index cb8fe846d9dd..125093d568be 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -17,6 +17,18 @@
 
 extern const char *objname;
 
+static inline int sprint_name(char *str, const char *name, unsigned long offset)
+{
+	int len;
+
+	if (offset)
+		len = sprintf(str, "%s+0x%lx", name, offset);
+	else
+		len = sprintf(str, "%s", name);
+
+	return len;
+}
+
 static inline char *offstr(struct section *sec, unsigned long offset)
 {
 	bool is_text = (sec->sh.sh_flags & SHF_EXECINSTR);
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 06/17] objtool: Improve offstr() output
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (4 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 05/17] objtool: Print symbol during disassembly Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 07/17] objtool: Store instruction disassembly result Alexandre Chartre
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

offset() formats a section offset into a "<symbol>+<offset>" string.
Improve the output to just "<symbol>" when the offset is zero.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/include/objtool/warn.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index 125093d568be..d89e6ae69143 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -43,12 +43,12 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 
 	if (sym) {
 		str = malloc(strlen(sym->name) + strlen(sec->name) + 40);
-		len = sprintf(str, "%s+0x%lx", sym->name, offset - sym->offset);
+		len = sprint_name(str, sym->name, offset - sym->offset);
 		if (opts.sec_address)
 			sprintf(str+len, " (%s+0x%lx)", sec->name, offset);
 	} else {
 		str = malloc(strlen(sec->name) + 20);
-		sprintf(str, "%s+0x%lx", sec->name, offset);
+		sprint_name(str, sec->name, offset);
 	}
 
 	return str;
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 07/17] objtool: Store instruction disassembly result
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (5 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 06/17] objtool: Improve offstr() output Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 08/17] objtool: Disassemble instruction on warning or backtrace Alexandre Chartre
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

When disassembling an instruction store the result instead of directly
printing it.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/disas.c | 78 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 6 deletions(-)

diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 91d23f7518e3..fbec062f40eb 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -12,9 +12,16 @@
 #include <linux/string.h>
 #include <tools/dis-asm-compat.h>
 
+/*
+ * Size of the buffer for storing the result of disassembling
+ * a single instruction.
+ */
+#define DISAS_RESULT_SIZE	1024
+
 struct disas_context {
 	struct objtool_file *file;
 	struct instruction *insn;
+	char result[DISAS_RESULT_SIZE];
 	disassembler_ftype disassembler;
 	struct disassemble_info info;
 };
@@ -22,6 +29,60 @@ struct disas_context {
 #define DINFO_FPRINTF(dinfo, ...)	\
 	((*(dinfo)->fprintf_func)((dinfo)->stream, __VA_ARGS__))
 
+
+static int disas_result_fprintf(struct disas_context *dctx,
+				const char *fmt, va_list ap)
+{
+	char *buf = dctx->result;
+	size_t avail, len;
+
+	len = strlen(buf);
+	if (len >= DISAS_RESULT_SIZE - 1) {
+		WARN_FUNC(dctx->insn->sec, dctx->insn->offset,
+			  "disassembly buffer is full");
+		return -1;
+	}
+	avail = DISAS_RESULT_SIZE - len;
+
+	len = vsnprintf(buf + len, avail, fmt, ap);
+	if (len < 0 || len >= avail) {
+		WARN_FUNC(dctx->insn->sec, dctx->insn->offset,
+			  "disassembly buffer is truncated");
+		return -1;
+	}
+
+	return 0;
+}
+
+static int disas_fprintf(void *stream, const char *fmt, ...)
+{
+	va_list arg;
+	int rv;
+
+	va_start(arg, fmt);
+	rv = disas_result_fprintf(stream, fmt, arg);
+	va_end(arg);
+
+	return rv;
+}
+
+/*
+ * For init_disassemble_info_compat().
+ */
+static int disas_fprintf_styled(void *stream,
+				enum disassembler_style style,
+				const char *fmt, ...)
+{
+	va_list arg;
+	int rv;
+
+	va_start(arg, fmt);
+	rv = disas_result_fprintf(stream, fmt, arg);
+	va_end(arg);
+
+	return rv;
+}
+
 static void disas_print_address(bfd_vma addr, struct disassemble_info *dinfo)
 {
 	struct disas_context *dctx = dinfo->application_data;
@@ -147,9 +208,8 @@ struct disas_context *disas_context_create(struct objtool_file *file)
 	dctx->file = file;
 	dinfo = &dctx->info;
 
-	init_disassemble_info_compat(dinfo, stdout,
-				     (fprintf_ftype)fprintf,
-				     fprintf_styled);
+	init_disassemble_info_compat(dinfo, dctx,
+				     disas_fprintf, disas_fprintf_styled);
 
 	dinfo->read_memory_func = buffer_read_memory;
 	dinfo->print_address_func = disas_print_address;
@@ -196,6 +256,11 @@ void disas_context_destroy(struct disas_context *dctx)
 	free(dctx);
 }
 
+static char *disas_result(struct disas_context *dctx)
+{
+	return dctx->result;
+}
+
 /*
  * Disassemble a single instruction. Return the size of the instruction.
  */
@@ -206,6 +271,7 @@ static size_t disas_insn(struct disas_context *dctx,
 	struct disassemble_info *dinfo = &dctx->info;
 
 	dctx->insn = insn;
+	dctx->result[0] = '\0';
 
 	/*
 	 * Set the disassembler buffer to read data from the section
@@ -229,10 +295,10 @@ static void disas_func(struct disas_context *dctx, struct symbol *func)
 	printf("%s:\n", func->name);
 	sym_for_each_insn(dctx->file, func, insn) {
 		addr = insn->offset;
-		printf(" %6lx:  %s+0x%-6lx      ",
-		       addr, func->name, addr - func->offset);
 		disas_insn(dctx, insn);
-		printf("\n");
+		printf(" %6lx:  %s+0x%-6lx      %s\n",
+		       addr, func->name, addr - func->offset,
+		       disas_result(dctx));
 	}
 	printf("\n");
 }
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 08/17] objtool: Disassemble instruction on warning or backtrace
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (6 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 07/17] objtool: Store instruction disassembly result Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 09/17] objtool: Extract code to validate instruction from the validate branch loop Alexandre Chartre
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

When an instruction warning (WARN_INSN) or backtrace (BT_INSN) is issued,
disassemble the instruction to provide more context.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c                 | 36 ++++++++++++++++++++++-----
 tools/objtool/disas.c                 |  5 ++--
 tools/objtool/include/objtool/check.h |  2 ++
 tools/objtool/include/objtool/disas.h | 13 ++++++++++
 tools/objtool/include/objtool/warn.h  | 16 ++++++++----
 5 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index a4d0a6c62bc0..beaafa1f0323 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4581,11 +4581,34 @@ static void free_insns(struct objtool_file *file)
 		free(chunk->addr);
 }
 
+static struct disas_context *objtool_disas_ctx;
+
+const char *objtool_disas_insn(struct instruction *insn)
+{
+	struct disas_context *dctx = objtool_disas_ctx;
+
+	if (!dctx)
+		return "";
+
+	disas_insn(dctx, insn);
+	return disas_result(dctx);
+}
+
 int check(struct objtool_file *file)
 {
-	struct disas_context *disas_ctx;
+	struct disas_context *disas_ctx = NULL;
 	int ret = 0, warnings = 0;
 
+	/*
+	 * If the verbose or backtrace option is used then we need a
+	 * disassembly context to disassemble instruction or function
+	 * on warning or backtrace.
+	 */
+	if (opts.verbose || opts.backtrace) {
+		disas_ctx = disas_context_create(file);
+		objtool_disas_ctx = disas_ctx;
+	}
+
 	arch_initial_func_cfi_state(&initial_func_cfi);
 	init_cfi_state(&init_cfi);
 	init_cfi_state(&func_cfi);
@@ -4711,11 +4734,12 @@ int check(struct objtool_file *file)
 		if (opts.werror && warnings)
 			WARN("%d warning(s) upgraded to errors", warnings);
 		print_args();
-		disas_ctx = disas_context_create(file);
-		if (disas_ctx) {
-			disas_warned_funcs(disas_ctx);
-			disas_context_destroy(disas_ctx);
-		}
+		disas_warned_funcs(disas_ctx);
+	}
+
+	if (disas_ctx) {
+		disas_context_destroy(disas_ctx);
+		objtool_disas_ctx = NULL;
 	}
 
 	free_insns(file);
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index fbec062f40eb..3831e46e0f35 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -256,7 +256,7 @@ void disas_context_destroy(struct disas_context *dctx)
 	free(dctx);
 }
 
-static char *disas_result(struct disas_context *dctx)
+char *disas_result(struct disas_context *dctx)
 {
 	return dctx->result;
 }
@@ -264,8 +264,7 @@ static char *disas_result(struct disas_context *dctx)
 /*
  * Disassemble a single instruction. Return the size of the instruction.
  */
-static size_t disas_insn(struct disas_context *dctx,
-			 struct instruction *insn)
+size_t disas_insn(struct disas_context *dctx, struct instruction *insn)
 {
 	disassembler_ftype disasm = dctx->disassembler;
 	struct disassemble_info *dinfo = &dctx->info;
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index 4adbcd760c6f..f3ea144d4746 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -139,4 +139,6 @@ struct instruction *next_insn_same_sec(struct objtool_file *file, struct instruc
 	     insn && insn->offset < sym->offset + sym->len;		\
 	     insn = next_insn_same_sec(file, insn))
 
+const char *objtool_disas_insn(struct instruction *insn);
+
 #endif /* _CHECK_H */
diff --git a/tools/objtool/include/objtool/disas.h b/tools/objtool/include/objtool/disas.h
index 3ec3ce2e4e6f..1aee1fbe0bb9 100644
--- a/tools/objtool/include/objtool/disas.h
+++ b/tools/objtool/include/objtool/disas.h
@@ -17,6 +17,8 @@ void disas_warned_funcs(struct disas_context *dctx);
 int disas_info_init(struct disassemble_info *dinfo,
 		    int arch, int mach32, int mach64,
 		    const char *options);
+size_t disas_insn(struct disas_context *dctx, struct instruction *insn);
+char *disas_result(struct disas_context *dctx);
 
 #else /* DISAS */
 
@@ -38,6 +40,17 @@ static inline int disas_info_init(struct disassemble_info *dinfo,
 	return -1;
 }
 
+static inline size_t disas_insn(struct disas_context *dctx,
+				struct instruction *insn)
+{
+	return -1;
+}
+
+static inline char *disas_result(struct disas_context *dctx)
+{
+	return NULL;
+}
+
 #endif /* DISAS */
 
 #endif /* _DISAS_H */
diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index d89e6ae69143..f001233b27df 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -89,9 +89,11 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 #define WARN_INSN(insn, format, ...)					\
 ({									\
 	struct instruction *_insn = (insn);				\
-	if (!_insn->sym || !_insn->sym->warned)				\
+	if (!_insn->sym || !_insn->sym->warned)	{			\
 		WARN_FUNC(_insn->sec, _insn->offset, format,		\
 			  ##__VA_ARGS__);				\
+		BT_INSN(_insn, "");					\
+	}								\
 	if (_insn->sym)							\
 		_insn->sym->warned = 1;					\
 })
@@ -99,10 +101,14 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 #define BT_INSN(insn, format, ...)				\
 ({								\
 	if (opts.verbose || opts.backtrace) {			\
-		struct instruction *_insn = (insn);		\
-		char *_str = offstr(_insn->sec, _insn->offset); \
-		WARN("  %s: " format, _str, ##__VA_ARGS__);	\
-		free(_str);					\
+		struct instruction *__insn = (insn);		\
+		char *_str = offstr(__insn->sec, __insn->offset); \
+		const char *_istr = objtool_disas_insn(__insn);	\
+		int _len;					\
+		_len = snprintf(NULL, 0, "  %s: " format,  _str, ##__VA_ARGS__);	\
+		_len = (_len < 50) ? 50 - _len : 0;		\
+		WARN("  %s: " format "  %*s%s", _str, ##__VA_ARGS__, _len, "", _istr); \
+		free(_str);						\
 	}							\
 })
 
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 09/17] objtool: Extract code to validate instruction from the validate branch loop
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (7 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 08/17] objtool: Disassemble instruction on warning or backtrace Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 10/17] objtool: Record symbol name max length Alexandre Chartre
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

The code to validate a branch loops through all instructions of the
branch and validate each instruction. Move the code to validate an
instruction to a separated function.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c | 388 ++++++++++++++++++++++--------------------
 1 file changed, 206 insertions(+), 182 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index beaafa1f0323..947fe57e9a6d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3528,254 +3528,278 @@ static bool skip_alt_group(struct instruction *insn)
 	return alt_insn->type == INSN_CLAC || alt_insn->type == INSN_STAC;
 }
 
-/*
- * Follow the branch starting at the given instruction, and recursively follow
- * any other branches (jumps).  Meanwhile, track the frame pointer state at
- * each instruction and validate all the rules described in
- * tools/objtool/Documentation/objtool.txt.
- */
 static int validate_branch(struct objtool_file *file, struct symbol *func,
-			   struct instruction *insn, struct insn_state state)
+			   struct instruction *insn, struct insn_state state);
+
+static int validate_insn(struct objtool_file *file, struct symbol *func,
+			 struct instruction *insn, struct insn_state *statep,
+			 struct instruction *prev_insn, struct instruction *next_insn,
+			 bool *dead_end)
 {
 	struct alternative *alt;
-	struct instruction *next_insn, *prev_insn = NULL;
-	struct section *sec;
 	u8 visited;
 	int ret;
 
-	if (func && func->ignore)
-		return 0;
+	/*
+	 * Any returns before the end of this function are effectively dead
+	 * ends, i.e. validate_branch() has reached the end of the branch.
+	 */
+	*dead_end = true;
 
-	sec = insn->sec;
+	visited = VISITED_BRANCH << statep->uaccess;
+	if (insn->visited & VISITED_BRANCH_MASK) {
+		if (!insn->hint && !insn_cfi_match(insn, &statep->cfi))
+			return 1;
 
-	while (1) {
-		next_insn = next_insn_to_validate(file, insn);
+		if (insn->visited & visited)
+			return 0;
+	} else {
+		nr_insns_visited++;
+	}
 
-		if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
-			/* Ignore KCFI type preambles, which always fall through */
-			if (!strncmp(func->name, "__cfi_", 6) ||
-			    !strncmp(func->name, "__pfx_", 6))
-				return 0;
+	if (statep->noinstr)
+		statep->instr += insn->instr;
 
-			if (file->ignore_unreachables)
-				return 0;
+	if (insn->hint) {
+		if (insn->restore) {
+			struct instruction *save_insn, *i;
 
-			WARN("%s() falls through to next function %s()",
-			     func->name, insn_func(insn)->name);
-			func->warned = 1;
+			i = insn;
+			save_insn = NULL;
 
-			return 1;
-		}
+			sym_for_each_insn_continue_reverse(file, func, i) {
+				if (i->save) {
+					save_insn = i;
+					break;
+				}
+			}
 
-		visited = VISITED_BRANCH << state.uaccess;
-		if (insn->visited & VISITED_BRANCH_MASK) {
-			if (!insn->hint && !insn_cfi_match(insn, &state.cfi))
+			if (!save_insn) {
+				WARN_INSN(insn, "no corresponding CFI save for CFI restore");
 				return 1;
+			}
 
-			if (insn->visited & visited)
-				return 0;
-		} else {
-			nr_insns_visited++;
+			if (!save_insn->visited) {
+				/*
+				 * If the restore hint insn is at the
+				 * beginning of a basic block and was
+				 * branched to from elsewhere, and the
+				 * save insn hasn't been visited yet,
+				 * defer following this branch for now.
+				 * It will be seen later via the
+				 * straight-line path.
+				 */
+				if (!prev_insn)
+					return 0;
+
+				WARN_INSN(insn, "objtool isn't smart enough to handle this CFI save/restore combo");
+				return 1;
+			}
+
+			insn->cfi = save_insn->cfi;
+			nr_cfi_reused++;
 		}
 
-		if (state.noinstr)
-			state.instr += insn->instr;
+		statep->cfi = *insn->cfi;
+	} else {
+		/* XXX track if we actually changed statep->cfi */
 
-		if (insn->hint) {
-			if (insn->restore) {
-				struct instruction *save_insn, *i;
+		if (prev_insn && !cficmp(prev_insn->cfi, &statep->cfi)) {
+			insn->cfi = prev_insn->cfi;
+			nr_cfi_reused++;
+		} else {
+			insn->cfi = cfi_hash_find_or_add(&statep->cfi);
+		}
+	}
 
-				i = insn;
-				save_insn = NULL;
+	insn->visited |= visited;
 
-				sym_for_each_insn_continue_reverse(file, func, i) {
-					if (i->save) {
-						save_insn = i;
-						break;
-					}
-				}
+	if (propagate_alt_cfi(file, insn))
+		return 1;
 
-				if (!save_insn) {
-					WARN_INSN(insn, "no corresponding CFI save for CFI restore");
-					return 1;
-				}
+	if (insn->alts) {
+		for (alt = insn->alts; alt; alt = alt->next) {
+			ret = validate_branch(file, func, alt->insn, *statep);
+			if (ret) {
+				BT_INSN(insn, "(alt)");
+				return ret;
+			}
+		}
+	}
 
-				if (!save_insn->visited) {
-					/*
-					 * If the restore hint insn is at the
-					 * beginning of a basic block and was
-					 * branched to from elsewhere, and the
-					 * save insn hasn't been visited yet,
-					 * defer following this branch for now.
-					 * It will be seen later via the
-					 * straight-line path.
-					 */
-					if (!prev_insn)
-						return 0;
+	if (skip_alt_group(insn))
+		return 0;
 
-					WARN_INSN(insn, "objtool isn't smart enough to handle this CFI save/restore combo");
-					return 1;
-				}
+	if (handle_insn_ops(insn, next_insn, statep))
+		return 1;
 
-				insn->cfi = save_insn->cfi;
-				nr_cfi_reused++;
-			}
+	switch (insn->type) {
 
-			state.cfi = *insn->cfi;
-		} else {
-			/* XXX track if we actually changed state.cfi */
+	case INSN_RETURN:
+		return validate_return(func, insn, statep);
 
-			if (prev_insn && !cficmp(prev_insn->cfi, &state.cfi)) {
-				insn->cfi = prev_insn->cfi;
-				nr_cfi_reused++;
-			} else {
-				insn->cfi = cfi_hash_find_or_add(&state.cfi);
-			}
+	case INSN_CALL:
+	case INSN_CALL_DYNAMIC:
+		ret = validate_call(file, insn, statep);
+		if (ret)
+			return ret;
+
+		if (opts.stackval && func && !is_special_call(insn) &&
+		    !has_valid_stack_frame(statep)) {
+			WARN_INSN(insn, "call without frame pointer save/setup");
+			return 1;
 		}
 
-		insn->visited |= visited;
+		break;
 
-		if (propagate_alt_cfi(file, insn))
-			return 1;
+	case INSN_JUMP_CONDITIONAL:
+	case INSN_JUMP_UNCONDITIONAL:
+		if (is_sibling_call(insn)) {
+			ret = validate_sibling_call(file, insn, statep);
+			if (ret)
+				return ret;
 
-		if (insn->alts) {
-			for (alt = insn->alts; alt; alt = alt->next) {
-				ret = validate_branch(file, func, alt->insn, state);
-				if (ret) {
-					BT_INSN(insn, "(alt)");
-					return ret;
-				}
+		} else if (insn->jump_dest) {
+			ret = validate_branch(file, func,
+					      insn->jump_dest, *statep);
+			if (ret) {
+				BT_INSN(insn, "(branch)");
+				return ret;
 			}
 		}
 
-		if (skip_alt_group(insn))
+		if (insn->type == INSN_JUMP_UNCONDITIONAL)
 			return 0;
 
-		if (handle_insn_ops(insn, next_insn, &state))
-			return 1;
-
-		switch (insn->type) {
-
-		case INSN_RETURN:
-			return validate_return(func, insn, &state);
+		break;
 
-		case INSN_CALL:
-		case INSN_CALL_DYNAMIC:
-			ret = validate_call(file, insn, &state);
+	case INSN_JUMP_DYNAMIC:
+	case INSN_JUMP_DYNAMIC_CONDITIONAL:
+		if (is_sibling_call(insn)) {
+			ret = validate_sibling_call(file, insn, statep);
 			if (ret)
 				return ret;
+		}
 
-			if (opts.stackval && func && !is_special_call(insn) &&
-			    !has_valid_stack_frame(&state)) {
-				WARN_INSN(insn, "call without frame pointer save/setup");
-				return 1;
-			}
+		if (insn->type == INSN_JUMP_DYNAMIC)
+			return 0;
 
-			break;
+		break;
 
-		case INSN_JUMP_CONDITIONAL:
-		case INSN_JUMP_UNCONDITIONAL:
-			if (is_sibling_call(insn)) {
-				ret = validate_sibling_call(file, insn, &state);
-				if (ret)
-					return ret;
+	case INSN_SYSCALL:
+		if (func && (!next_insn || !next_insn->hint)) {
+			WARN_INSN(insn, "unsupported instruction in callable function");
+			return 1;
+		}
 
-			} else if (insn->jump_dest) {
-				ret = validate_branch(file, func,
-						      insn->jump_dest, state);
-				if (ret) {
-					BT_INSN(insn, "(branch)");
-					return ret;
-				}
-			}
+		break;
 
-			if (insn->type == INSN_JUMP_UNCONDITIONAL)
-				return 0;
+	case INSN_SYSRET:
+		if (func && (!next_insn || !next_insn->hint)) {
+			WARN_INSN(insn, "unsupported instruction in callable function");
+			return 1;
+		}
 
+		return 0;
+
+	case INSN_STAC:
+		if (!opts.uaccess)
 			break;
 
-		case INSN_JUMP_DYNAMIC:
-		case INSN_JUMP_DYNAMIC_CONDITIONAL:
-			if (is_sibling_call(insn)) {
-				ret = validate_sibling_call(file, insn, &state);
-				if (ret)
-					return ret;
-			}
+		if (statep->uaccess) {
+			WARN_INSN(insn, "recursive UACCESS enable");
+			return 1;
+		}
 
-			if (insn->type == INSN_JUMP_DYNAMIC)
-				return 0;
+		statep->uaccess = true;
+		break;
 
+	case INSN_CLAC:
+		if (!opts.uaccess)
 			break;
 
-		case INSN_SYSCALL:
-			if (func && (!next_insn || !next_insn->hint)) {
-				WARN_INSN(insn, "unsupported instruction in callable function");
-				return 1;
-			}
+		if (!statep->uaccess && func) {
+			WARN_INSN(insn, "redundant UACCESS disable");
+			return 1;
+		}
 
-			break;
+		if (func_uaccess_safe(func) && !statep->uaccess_stack) {
+			WARN_INSN(insn, "UACCESS-safe disables UACCESS");
+			return 1;
+		}
 
-		case INSN_SYSRET:
-			if (func && (!next_insn || !next_insn->hint)) {
-				WARN_INSN(insn, "unsupported instruction in callable function");
-				return 1;
-			}
+		statep->uaccess = false;
+		break;
 
-			return 0;
+	case INSN_STD:
+		if (statep->df) {
+			WARN_INSN(insn, "recursive STD");
+			return 1;
+		}
 
-		case INSN_STAC:
-			if (!opts.uaccess)
-				break;
+		statep->df = true;
+		break;
 
-			if (state.uaccess) {
-				WARN_INSN(insn, "recursive UACCESS enable");
-				return 1;
-			}
+	case INSN_CLD:
+		if (!statep->df && func) {
+			WARN_INSN(insn, "redundant CLD");
+			return 1;
+		}
 
-			state.uaccess = true;
-			break;
+		statep->df = false;
+		break;
 
-		case INSN_CLAC:
-			if (!opts.uaccess)
-				break;
+	default:
+		break;
+	}
 
-			if (!state.uaccess && func) {
-				WARN_INSN(insn, "redundant UACCESS disable");
-				return 1;
-			}
+	*dead_end = insn->dead_end;
 
-			if (func_uaccess_safe(func) && !state.uaccess_stack) {
-				WARN_INSN(insn, "UACCESS-safe disables UACCESS");
-				return 1;
-			}
+	return 0;
+}
 
-			state.uaccess = false;
-			break;
+/*
+ * Follow the branch starting at the given instruction, and recursively follow
+ * any other branches (jumps).  Meanwhile, track the frame pointer state at
+ * each instruction and validate all the rules described in
+ * tools/objtool/Documentation/objtool.txt.
+ */
+static int validate_branch(struct objtool_file *file, struct symbol *func,
+			   struct instruction *insn, struct insn_state state)
+{
+	struct instruction *next_insn, *prev_insn = NULL;
+	struct section *sec;
+	bool dead_end;
+	int ret;
 
-		case INSN_STD:
-			if (state.df) {
-				WARN_INSN(insn, "recursive STD");
-				return 1;
-			}
+	if (func && func->ignore)
+		return 0;
 
-			state.df = true;
-			break;
+	sec = insn->sec;
 
-		case INSN_CLD:
-			if (!state.df && func) {
-				WARN_INSN(insn, "redundant CLD");
-				return 1;
-			}
+	while (1) {
+		next_insn = next_insn_to_validate(file, insn);
 
-			state.df = false;
-			break;
+		if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
+			/* Ignore KCFI type preambles, which always fall through */
+			if (!strncmp(func->name, "__cfi_", 6) ||
+			    !strncmp(func->name, "__pfx_", 6))
+				return 0;
 
-		default:
-			break;
+			if (file->ignore_unreachables)
+				return 0;
+
+			WARN("%s() falls through to next function %s()",
+			     func->name, insn_func(insn)->name);
+			func->warned = 1;
+
+			return 1;
 		}
 
-		if (insn->dead_end)
-			return 0;
+		ret = validate_insn(file, func, insn, &state, prev_insn, next_insn,
+				    &dead_end);
+		if (dead_end)
+			break;
 
 		if (!next_insn) {
 			if (state.cfi.cfa.base == CFI_UNDEFINED)
@@ -3793,7 +3817,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 		insn = next_insn;
 	}
 
-	return 0;
+	return ret;
 }
 
 static int validate_unwind_hint(struct objtool_file *file,
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 10/17] objtool: Record symbol name max length
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (8 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 09/17] objtool: Extract code to validate instruction from the validate branch loop Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 11/17] objtool: Add option to trace function validation Alexandre Chartre
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

Keep track of the maximum length of symbol names. This will help
formatting the code flow between different functions.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 947fe57e9a6d..413860f2d7fa 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -35,6 +35,8 @@ static struct cfi_state init_cfi;
 static struct cfi_state func_cfi;
 static struct cfi_state force_undefined_cfi;
 
+static size_t sym_name_max_len;
+
 struct instruction *find_insn(struct objtool_file *file,
 			      struct section *sec, unsigned long offset)
 {
@@ -2459,6 +2461,7 @@ static bool is_profiling_func(const char *name)
 static int classify_symbols(struct objtool_file *file)
 {
 	struct symbol *func;
+	size_t len;
 
 	for_each_sym(file, func) {
 		if (func->type == STT_NOTYPE && strstarts(func->name, ".L"))
@@ -2485,6 +2488,10 @@ static int classify_symbols(struct objtool_file *file)
 
 		if (is_profiling_func(func->name))
 			func->profiling_func = true;
+
+		len = strlen(func->name);
+		if (len > sym_name_max_len)
+			sym_name_max_len = len;
 	}
 
 	return 0;
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 11/17] objtool: Add option to trace function validation
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (9 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 10/17] objtool: Record symbol name max length Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 12/17] objtool: Trace instruction state changes during " Alexandre Chartre
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

Add an option to trace and have information during the validation
of specified functions. Functions are specified with the --trace
option which can be a single function name (e.g. --trace foo to
trace the function with the name "foo"), or a shell wildcard
pattern (e.g. --trace foo* to trace all functions with a name
starting with "foo").

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/Build                     |   1 +
 tools/objtool/builtin-check.c           |   1 +
 tools/objtool/check.c                   | 111 ++++++++++++++++++++----
 tools/objtool/disas.c                   |  83 ++++++++++++++++++
 tools/objtool/include/objtool/builtin.h |   1 +
 tools/objtool/include/objtool/check.h   |   8 +-
 tools/objtool/include/objtool/disas.h   |  11 +++
 tools/objtool/include/objtool/trace.h   |  68 +++++++++++++++
 tools/objtool/include/objtool/warn.h    |   1 +
 tools/objtool/trace.c                   |   9 ++
 10 files changed, 275 insertions(+), 19 deletions(-)
 create mode 100644 tools/objtool/include/objtool/trace.h
 create mode 100644 tools/objtool/trace.c

diff --git a/tools/objtool/Build b/tools/objtool/Build
index ee04fba8c9d1..6e62ffd40792 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -9,6 +9,7 @@ objtool-y += elf.o
 objtool-y += objtool.o
 
 objtool-$(BUILD_DISAS) += disas.o
+objtool-$(BUILD_DISAS) += trace.o
 
 objtool-$(BUILD_ORC) += orc_gen.o
 objtool-$(BUILD_ORC) += orc_dump.o
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 80239843e9f0..c53d738e4fb0 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -99,6 +99,7 @@ static const struct option check_options[] = {
 	OPT_STRING('o',  "output", &opts.output, "file", "output file name"),
 	OPT_BOOLEAN(0,   "sec-address", &opts.sec_address, "print section addresses in warnings"),
 	OPT_BOOLEAN(0,   "stats", &opts.stats, "print statistics"),
+	OPT_STRING(0,    "trace", &opts.trace, "func", "trace function validation"),
 	OPT_BOOLEAN('v', "verbose", &opts.verbose, "verbose warnings"),
 	OPT_BOOLEAN(0,   "Werror", &opts.werror, "return error on warnings"),
 
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 413860f2d7fa..1505dc8812fb 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2015-2017 Josh Poimboeuf <jpoimboe@redhat.com>
  */
 
+#include <fnmatch.h>
 #include <string.h>
 #include <stdlib.h>
 #include <inttypes.h>
@@ -14,6 +15,7 @@
 #include <objtool/disas.h>
 #include <objtool/check.h>
 #include <objtool/special.h>
+#include <objtool/trace.h>
 #include <objtool/warn.h>
 #include <objtool/endianness.h>
 
@@ -35,7 +37,9 @@ static struct cfi_state init_cfi;
 static struct cfi_state func_cfi;
 static struct cfi_state force_undefined_cfi;
 
-static size_t sym_name_max_len;
+struct disas_context *objtool_disas_ctx;
+
+size_t sym_name_max_len;
 
 struct instruction *find_insn(struct objtool_file *file,
 			      struct section *sec, unsigned long offset)
@@ -3512,8 +3516,10 @@ static bool skip_alt_group(struct instruction *insn)
 	struct instruction *alt_insn = insn->alts ? insn->alts->insn : NULL;
 
 	/* ANNOTATE_IGNORE_ALTERNATIVE */
-	if (insn->alt_group && insn->alt_group->ignore)
+	if (insn->alt_group && insn->alt_group->ignore) {
+		TRACE_INSN(insn, "alt group ignored");
 		return true;
+	}
 
 	/*
 	 * For NOP patched with CLAC/STAC, only follow the latter to avoid
@@ -3537,6 +3543,8 @@ static bool skip_alt_group(struct instruction *insn)
 
 static int validate_branch(struct objtool_file *file, struct symbol *func,
 			   struct instruction *insn, struct insn_state state);
+static int do_validate_branch(struct objtool_file *file, struct symbol *func,
+			      struct instruction *insn, struct insn_state state);
 
 static int validate_insn(struct objtool_file *file, struct symbol *func,
 			 struct instruction *insn, struct insn_state *statep,
@@ -3558,8 +3566,10 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		if (!insn->hint && !insn_cfi_match(insn, &statep->cfi))
 			return 1;
 
-		if (insn->visited & visited)
+		if (insn->visited & visited) {
+			TRACE_INSN(insn, "already visited");
 			return 0;
+		}
 	} else {
 		nr_insns_visited++;
 	}
@@ -3596,8 +3606,10 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 				 * It will be seen later via the
 				 * straight-line path.
 				 */
-				if (!prev_insn)
+				if (!prev_insn) {
+					TRACE_INSN(insn, "defer restore");
 					return 0;
+				}
 
 				WARN_INSN(insn, "objtool isn't smart enough to handle this CFI save/restore combo");
 				return 1;
@@ -3625,13 +3637,24 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		return 1;
 
 	if (insn->alts) {
+		int i, num_alts;
+
+		num_alts = 0;
+		for (alt = insn->alts; alt; alt = alt->next)
+			num_alts++;
+
+		i = 1;
 		for (alt = insn->alts; alt; alt = alt->next) {
+			TRACE_INSN(insn, "alternative %d/%d", i, num_alts);
 			ret = validate_branch(file, func, alt->insn, *statep);
 			if (ret) {
 				BT_INSN(insn, "(alt)");
 				return ret;
 			}
+			i++;
 		}
+
+		TRACE_INSN(insn, "alternative orig");
 	}
 
 	if (skip_alt_group(insn))
@@ -3643,10 +3666,16 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 	switch (insn->type) {
 
 	case INSN_RETURN:
+		TRACE_INSN(insn, "return");
 		return validate_return(func, insn, statep);
 
 	case INSN_CALL:
 	case INSN_CALL_DYNAMIC:
+		if (insn->type == INSN_CALL)
+			TRACE_INSN(insn, "call");
+		else
+			TRACE_INSN(insn, "indirect call");
+
 		ret = validate_call(file, insn, statep);
 		if (ret)
 			return ret;
@@ -3662,13 +3691,18 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 	case INSN_JUMP_CONDITIONAL:
 	case INSN_JUMP_UNCONDITIONAL:
 		if (is_sibling_call(insn)) {
+			TRACE_INSN(insn, "sibling call");
 			ret = validate_sibling_call(file, insn, statep);
 			if (ret)
 				return ret;
 
 		} else if (insn->jump_dest) {
-			ret = validate_branch(file, func,
-					      insn->jump_dest, *statep);
+			if (insn->type == INSN_JUMP_UNCONDITIONAL)
+				TRACE_INSN(insn, "unconditional jump");
+			else
+				TRACE_INSN(insn, "jump taken");
+
+			ret = validate_branch(file, func, insn->jump_dest, *statep);
 			if (ret) {
 				BT_INSN(insn, "(branch)");
 				return ret;
@@ -3678,10 +3712,12 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		if (insn->type == INSN_JUMP_UNCONDITIONAL)
 			return 0;
 
+		TRACE_INSN(insn, "jump not taken");
 		break;
 
 	case INSN_JUMP_DYNAMIC:
 	case INSN_JUMP_DYNAMIC_CONDITIONAL:
+		TRACE_INSN(insn, "indirect jump");
 		if (is_sibling_call(insn)) {
 			ret = validate_sibling_call(file, insn, statep);
 			if (ret)
@@ -3694,6 +3730,7 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		break;
 
 	case INSN_SYSCALL:
+		TRACE_INSN(insn, "syscall");
 		if (func && (!next_insn || !next_insn->hint)) {
 			WARN_INSN(insn, "unsupported instruction in callable function");
 			return 1;
@@ -3702,6 +3739,7 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		break;
 
 	case INSN_SYSRET:
+		TRACE_INSN(insn, "sysret");
 		if (func && (!next_insn || !next_insn->hint)) {
 			WARN_INSN(insn, "unsupported instruction in callable function");
 			return 1;
@@ -3710,6 +3748,7 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		return 0;
 
 	case INSN_STAC:
+		TRACE_INSN(insn, "stac");
 		if (!opts.uaccess)
 			break;
 
@@ -3722,6 +3761,7 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		break;
 
 	case INSN_CLAC:
+		TRACE_INSN(insn, "clac");
 		if (!opts.uaccess)
 			break;
 
@@ -3739,6 +3779,7 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		break;
 
 	case INSN_STD:
+		TRACE_INSN(insn, "std");
 		if (statep->df) {
 			WARN_INSN(insn, "recursive STD");
 			return 1;
@@ -3748,6 +3789,7 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		break;
 
 	case INSN_CLD:
+		TRACE_INSN(insn, "cld");
 		if (!statep->df && func) {
 			WARN_INSN(insn, "redundant CLD");
 			return 1;
@@ -3760,8 +3802,10 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 		break;
 	}
 
-	*dead_end = insn->dead_end;
+	if (insn->dead_end)
+		TRACE_INSN(insn, "dead end");
 
+	*dead_end = insn->dead_end;
 	return 0;
 }
 
@@ -3771,8 +3815,8 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
  * each instruction and validate all the rules described in
  * tools/objtool/Documentation/objtool.txt.
  */
-static int validate_branch(struct objtool_file *file, struct symbol *func,
-			   struct instruction *insn, struct insn_state state)
+static int do_validate_branch(struct objtool_file *file, struct symbol *func,
+			      struct instruction *insn, struct insn_state state)
 {
 	struct instruction *next_insn, *prev_insn = NULL;
 	struct section *sec;
@@ -3784,7 +3828,10 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 
 	sec = insn->sec;
 
-	while (1) {
+	do {
+
+		insn->trace = 0;
+
 		next_insn = next_insn_to_validate(file, insn);
 
 		if (func && insn_func(insn) && func != insn_func(insn)->pfunc) {
@@ -3796,6 +3843,8 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 			if (file->ignore_unreachables)
 				return 0;
 
+			TRACE_INSN(insn, "falls through to next function");
+
 			WARN("%s() falls through to next function %s()",
 			     func->name, insn_func(insn)->name);
 			func->warned = 1;
@@ -3805,10 +3854,15 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 
 		ret = validate_insn(file, func, insn, &state, prev_insn, next_insn,
 				    &dead_end);
-		if (dead_end)
-			break;
 
-		if (!next_insn) {
+		if (!insn->trace) {
+			if (ret)
+				TRACE_INSN(insn, "warning (%d)", ret);
+			else
+				TRACE_INSN(insn, NULL);
+		}
+
+		if (!dead_end && !next_insn) {
 			if (state.cfi.cfa.base == CFI_UNDEFINED)
 				return 0;
 			if (file->ignore_unreachables)
@@ -3822,7 +3876,20 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
 
 		prev_insn = insn;
 		insn = next_insn;
-	}
+
+	} while (!dead_end);
+
+	return ret;
+}
+
+static int validate_branch(struct objtool_file *file, struct symbol *func,
+			   struct instruction *insn, struct insn_state state)
+{
+	int ret;
+
+	trace_depth_inc();
+	ret = do_validate_branch(file, func, insn, state);
+	trace_depth_dec();
 
 	return ret;
 }
@@ -4237,9 +4304,19 @@ static int validate_symbol(struct objtool_file *file, struct section *sec,
 	if (opts.uaccess)
 		state->uaccess = sym->uaccess_safe;
 
+	if (opts.trace && !fnmatch(opts.trace, sym->name, 0)) {
+		trace_enable();
+		TRACE("%s: validation begin\n", sym->name);
+	}
+
 	ret = validate_branch(file, insn_func(insn), insn, *state);
 	if (ret)
 		BT_INSN(insn, "<=== (sym)");
+
+	TRACE("%s: validation %s\n\n", sym->name, ret ? "failed" : "end");
+	trace_disable();
+
+
 	return ret;
 }
 
@@ -4612,8 +4689,6 @@ static void free_insns(struct objtool_file *file)
 		free(chunk->addr);
 }
 
-static struct disas_context *objtool_disas_ctx;
-
 const char *objtool_disas_insn(struct instruction *insn)
 {
 	struct disas_context *dctx = objtool_disas_ctx;
@@ -4635,8 +4710,10 @@ int check(struct objtool_file *file)
 	 * disassembly context to disassemble instruction or function
 	 * on warning or backtrace.
 	 */
-	if (opts.verbose || opts.backtrace) {
+	if (opts.verbose || opts.backtrace || opts.trace) {
 		disas_ctx = disas_context_create(file);
+		if (!disas_ctx)
+			opts.trace = false;
 		objtool_disas_ctx = disas_ctx;
 	}
 
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 3831e46e0f35..004683c2b1ff 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -261,6 +261,89 @@ char *disas_result(struct disas_context *dctx)
 	return dctx->result;
 }
 
+
+#define DISAS_INSN_OFFSET_SPACE		10
+#define DISAS_INSN_SPACE		60
+
+/*
+ * Print a message in the instruction flow. If insn is not NULL then
+ * the instruction address is printed in addition of the message,
+ * otherwise only the message is printed. In all cases, the instruction
+ * itself is not printed.
+ */
+void disas_print_info(FILE *stream, struct instruction *insn, int depth,
+		      const char *format, ...)
+{
+	const char *addr_str;
+	va_list args;
+	int len;
+	int i;
+
+	len = sym_name_max_len + DISAS_INSN_OFFSET_SPACE;
+	if (insn && insn->sec) {
+		addr_str = offstr(insn->sec, insn->offset);
+		fprintf(stream, "%6lx:  %-*s  ", insn->offset, len, addr_str);
+		free((char *)addr_str);
+	} else {
+		len += 11;
+		fprintf(stream, "%-*s", len, "");
+	}
+
+	/* print vertical bars to show the code flow */
+	for (i = 0; i < depth; i++)
+		fprintf(stream, "| ");
+
+	va_start(args, format);
+	vfprintf(stream, format, args);
+	va_end(args);
+}
+
+/*
+ * Print an instruction address (offset and function), the instruction itself
+ * and an optional message.
+ */
+void disas_print_insn(FILE *stream, struct disas_context *dctx,
+		      struct instruction *insn, int depth,
+		      const char *format, ...)
+{
+	char fake_nop_insn[32];
+	const char *insn_str;
+	bool fake_nop;
+	va_list args;
+	int len;
+
+	/*
+	 * Alternative can insert a fake nop, sometimes with no
+	 * associated section so nothing to disassemble.
+	 */
+	fake_nop = (!insn->sec && insn->type == INSN_NOP);
+	if (fake_nop) {
+		snprintf(fake_nop_insn, 32, "<fake nop> (%d bytes)", insn->len);
+		insn_str = fake_nop_insn;
+	} else {
+		disas_insn(dctx, insn);
+		insn_str = disas_result(dctx);
+	}
+
+	/* print the instruction */
+	len = (depth+1) * 2 < DISAS_INSN_SPACE ? DISAS_INSN_SPACE - (depth+1) * 2 : 1;
+	disas_print_info(stream, insn, depth, "%-*s", len, insn_str);
+
+	/* print message if any */
+	if (!format)
+		return;
+
+	if (strcmp(format, "\n") == 0) {
+		fprintf(stream, "\n");
+		return;
+	}
+
+	fprintf(stream, " - ");
+	va_start(args, format);
+	vfprintf(stream, format, args);
+	va_end(args);
+}
+
 /*
  * Disassemble a single instruction. Return the size of the instruction.
  */
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 6b08666fa69d..b3c84b6fdc5f 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -37,6 +37,7 @@ struct opts {
 	const char *output;
 	bool sec_address;
 	bool stats;
+	const char *trace;
 	bool verbose;
 	bool werror;
 };
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index f3ea144d4746..e5f97acb6252 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -64,8 +64,9 @@ struct instruction {
 	    noendbr		: 1,
 	    unret		: 1,
 	    visited		: 4,
-	    no_reloc		: 1;
-		/* 10 bit hole */
+	    no_reloc		: 1,
+	    trace		: 1;
+		/* 9 bit hole */
 
 	struct alt_group *alt_group;
 	struct instruction *jump_dest;
@@ -141,4 +142,7 @@ struct instruction *next_insn_same_sec(struct objtool_file *file, struct instruc
 
 const char *objtool_disas_insn(struct instruction *insn);
 
+extern size_t sym_name_max_len;
+extern struct disas_context *objtool_disas_ctx;
+
 #endif /* _CHECK_H */
diff --git a/tools/objtool/include/objtool/disas.h b/tools/objtool/include/objtool/disas.h
index 1aee1fbe0bb9..5db75d06f219 100644
--- a/tools/objtool/include/objtool/disas.h
+++ b/tools/objtool/include/objtool/disas.h
@@ -19,6 +19,11 @@ int disas_info_init(struct disassemble_info *dinfo,
 		    const char *options);
 size_t disas_insn(struct disas_context *dctx, struct instruction *insn);
 char *disas_result(struct disas_context *dctx);
+void disas_print_info(FILE *stream, struct instruction *insn, int depth,
+		      const char *format, ...);
+void disas_print_insn(FILE *stream, struct disas_context *dctx,
+		      struct instruction *insn, int depth,
+		      const char *format, ...);
 
 #else /* DISAS */
 
@@ -51,6 +56,12 @@ static inline char *disas_result(struct disas_context *dctx)
 	return NULL;
 }
 
+static inline void disas_print_info(FILE *stream, struct instruction *insn,
+				    int depth, const char *format, ...) {}
+static inline void disas_print_insn(FILE *stream, struct disas_context *dctx,
+				    struct instruction *insn, int depth,
+				    const char *format, ...) {}
+
 #endif /* DISAS */
 
 #endif /* _DISAS_H */
diff --git a/tools/objtool/include/objtool/trace.h b/tools/objtool/include/objtool/trace.h
new file mode 100644
index 000000000000..ea0904a0ce00
--- /dev/null
+++ b/tools/objtool/include/objtool/trace.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates.
+ */
+
+#ifndef _TRACE_H
+#define _TRACE_H
+
+#include <objtool/check.h>
+#include <objtool/disas.h>
+
+#ifdef DISAS
+
+extern bool trace;
+extern int trace_depth;
+
+#define TRACE(fmt, ...)						\
+({	if (trace)						\
+		fprintf(stderr, fmt, ##__VA_ARGS__);		\
+})
+
+#define TRACE_INSN(insn, fmt, ...)				\
+({								\
+	if (trace) {						\
+		disas_print_insn(stderr, objtool_disas_ctx,	\
+				 insn, trace_depth - 1,	\
+				 fmt, ##__VA_ARGS__);		\
+		fprintf(stderr, "\n");				\
+		insn->trace = 1;				\
+	}							\
+})
+
+static inline void trace_enable(void)
+{
+	trace = true;
+	trace_depth = 0;
+}
+
+static inline void trace_disable(void)
+{
+	trace = false;
+}
+
+static inline void trace_depth_inc(void)
+{
+	if (trace)
+		trace_depth++;
+}
+
+static inline void trace_depth_dec(void)
+{
+	if (trace)
+		trace_depth--;
+}
+
+#else /* DISAS */
+
+#define TRACE(fmt, ...)
+#define TRACE_INSN(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) {}
+
+#endif
+
+#endif /* _TRACE_H */
diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index f001233b27df..e0b43441c577 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -109,6 +109,7 @@ static inline char *offstr(struct section *sec, unsigned long offset)
 		_len = (_len < 50) ? 50 - _len : 0;		\
 		WARN("  %s: " format "  %*s%s", _str, ##__VA_ARGS__, _len, "", _istr); \
 		free(_str);						\
+		__insn->trace = 1;				\
 	}							\
 })
 
diff --git a/tools/objtool/trace.c b/tools/objtool/trace.c
new file mode 100644
index 000000000000..bc3113ba72fd
--- /dev/null
+++ b/tools/objtool/trace.c
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates.
+ */
+
+#include <objtool/trace.h>
+
+bool trace;
+int trace_depth;
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 12/17] objtool: Trace instruction state changes during function validation
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (10 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 11/17] objtool: Add option to trace function validation Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 13/17] objtool: Improve register reporting " Alexandre Chartre
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

During function validation, objtool maintains a per-instruction state,
in particular to track call frame information. When tracing validation,
print any instruction state changes.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c                 |   8 +-
 tools/objtool/disas.c                 |   1 -
 tools/objtool/include/objtool/trace.h |  10 ++
 tools/objtool/trace.c                 | 131 ++++++++++++++++++++++++++
 4 files changed, 148 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 1505dc8812fb..8a51c871e1dc 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3551,6 +3551,8 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 			 struct instruction *prev_insn, struct instruction *next_insn,
 			 bool *dead_end)
 {
+	/* prev_state is not used if there is no disassembly support */
+	struct insn_state prev_state __maybe_unused;
 	struct alternative *alt;
 	u8 visited;
 	int ret;
@@ -3660,7 +3662,11 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 	if (skip_alt_group(insn))
 		return 0;
 
-	if (handle_insn_ops(insn, next_insn, statep))
+	prev_state = *statep;
+	ret = handle_insn_ops(insn, next_insn, statep);
+	TRACE_INSN_STATE(insn, &prev_state, statep);
+
+	if (ret)
 		return 1;
 
 	switch (insn->type) {
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 004683c2b1ff..376c7bedef47 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -29,7 +29,6 @@ struct disas_context {
 #define DINFO_FPRINTF(dinfo, ...)	\
 	((*(dinfo)->fprintf_func)((dinfo)->stream, __VA_ARGS__))
 
-
 static int disas_result_fprintf(struct disas_context *dctx,
 				const char *fmt, va_list ap)
 {
diff --git a/tools/objtool/include/objtool/trace.h b/tools/objtool/include/objtool/trace.h
index ea0904a0ce00..5b8abdb9b09f 100644
--- a/tools/objtool/include/objtool/trace.h
+++ b/tools/objtool/include/objtool/trace.h
@@ -30,6 +30,12 @@ extern int trace_depth;
 	}							\
 })
 
+#define TRACE_INSN_STATE(insn, sprev, snext)			\
+({								\
+	if (trace)						\
+		trace_insn_state(insn, sprev, snext);		\
+})
+
 static inline void trace_enable(void)
 {
 	trace = true;
@@ -53,10 +59,14 @@ static inline void trace_depth_dec(void)
 		trace_depth--;
 }
 
+void trace_insn_state(struct instruction *insn, struct insn_state *sprev,
+		      struct insn_state *snext);
+
 #else /* DISAS */
 
 #define TRACE(fmt, ...)
 #define TRACE_INSN(insn, fmt, ...)
+#define TRACE_INSN_STATE(insn, sprev, snext)
 
 static inline void trace_enable(void) {}
 static inline void trace_disable(void) {}
diff --git a/tools/objtool/trace.c b/tools/objtool/trace.c
index bc3113ba72fd..28c0257b02b5 100644
--- a/tools/objtool/trace.c
+++ b/tools/objtool/trace.c
@@ -7,3 +7,134 @@
 
 bool trace;
 int trace_depth;
+
+/*
+ * Macros to trace CFI state attributes changes.
+ */
+
+#define TRACE_CFI_ATTR(attr, prev, next, fmt, ...)		\
+({								\
+	if ((prev)->attr != (next)->attr)			\
+		TRACE("%s=" fmt " ", #attr, __VA_ARGS__); 	\
+})
+
+#define TRACE_CFI_ATTR_BOOL(attr, prev, next)			\
+	TRACE_CFI_ATTR(attr, prev, next,			\
+		       "%s", (next)->attr ? "true" : "false")
+
+#define TRACE_CFI_ATTR_NUM(attr, prev, next, fmt)		\
+	TRACE_CFI_ATTR(attr, prev, next, fmt, (next)->attr)
+
+#define CFI_REG_NAME_MAXLEN   16
+
+/*
+ * Return the name of a register. Note that the same static buffer
+ * is returned if the name is dynamically generated.
+ */
+static const char *cfi_reg_name(unsigned int reg)
+{
+	static char rname_buffer[CFI_REG_NAME_MAXLEN];
+
+	switch (reg) {
+	case CFI_UNDEFINED:
+		return "<undefined>";
+	case CFI_CFA:
+		return "cfa";
+	case CFI_SP_INDIRECT:
+		return "(sp)";
+	case CFI_BP_INDIRECT:
+		return "(bp)";
+	}
+
+	if (snprintf(rname_buffer, CFI_REG_NAME_MAXLEN, "r%d", reg) == 1)
+		return NULL;
+
+	return (const char *)rname_buffer;
+}
+
+/*
+ * Functions and macros to trace CFI registers changes.
+ */
+
+static void trace_cfi_reg(const char *prefix, int reg, const char *fmt,
+			  int base_prev, int offset_prev,
+			  int base_next, int offset_next)
+{
+	const char *rname;
+
+	if (base_prev == base_next && offset_prev == offset_next)
+		return;
+
+	if (prefix)
+		TRACE("%s:", prefix);
+
+	rname = cfi_reg_name(reg);
+
+	if (base_next == CFI_UNDEFINED) {
+		TRACE("%1$s=<undef> ", rname);
+	} else {
+		TRACE(fmt, rname,
+		      cfi_reg_name(base_next), offset_next);
+	}
+}
+
+static void trace_cfi_reg_val(const char *prefix, int reg,
+			      int base_prev, int offset_prev,
+			      int base_next, int offset_next)
+{
+	trace_cfi_reg(prefix, reg, "%1$s=%2$s%3$+d ",
+		      base_prev, offset_prev, base_next, offset_next);
+}
+
+static void trace_cfi_reg_ref(const char *prefix, int reg,
+			      int base_prev, int offset_prev,
+			      int base_next, int offset_next)
+{
+	trace_cfi_reg(prefix, reg, "%1$s=(%2$s%3$+d) ",
+		      base_prev, offset_prev, base_next, offset_next);
+}
+
+#define TRACE_CFI_REG_VAL(reg, prev, next)				\
+	trace_cfi_reg_val(NULL, reg, prev.base, prev.offset,		\
+			  next.base, next.offset)
+
+#define TRACE_CFI_REG_REF(reg, prev, next)				\
+	trace_cfi_reg_ref(NULL, reg, prev.base, prev.offset,		\
+			  next.base, next.offset)
+
+void trace_insn_state(struct instruction *insn, struct insn_state *sprev,
+		      struct insn_state *snext)
+{
+	struct cfi_state *cprev, *cnext;
+	int i;
+
+	if (!memcmp(sprev, snext, sizeof(struct insn_state)))
+		return;
+
+	cprev = &sprev->cfi;
+	cnext = &snext->cfi;
+
+	disas_print_insn(stderr, objtool_disas_ctx, insn,
+			 trace_depth - 1, " - state");
+
+	/* print registers changes */
+	TRACE_CFI_REG_VAL(CFI_CFA, cprev->cfa, cnext->cfa);
+	for (i = 0; i < CFI_NUM_REGS; i++) {
+		TRACE_CFI_REG_VAL(i, cprev->vals[i], cnext->vals[i]);
+		TRACE_CFI_REG_REF(i, cprev->regs[i], cnext->regs[i]);
+	}
+
+	/* print attributes changes */
+	TRACE_CFI_ATTR_NUM(stack_size, cprev, cnext, "%d");
+	TRACE_CFI_ATTR_BOOL(drap, cprev, cnext);
+	if (cnext->drap) {
+		trace_cfi_reg_val("drap", cnext->drap_reg,
+				  cprev->drap_reg, cprev->drap_offset,
+				  cnext->drap_reg, cnext->drap_offset);
+	}
+	TRACE_CFI_ATTR_BOOL(bp_scratch, cprev, cnext);
+	TRACE_CFI_ATTR_NUM(instr, sprev, snext, "%d");
+	TRACE_CFI_ATTR_NUM(uaccess_stack, sprev, snext, "%u");
+
+	TRACE("\n");
+}
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 13/17] objtool: Improve register reporting during function validation
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (11 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 12/17] objtool: Trace instruction state changes during " Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 14/17] objtool: Improve tracing of alternative instructions Alexandre Chartre
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

When tracing function validation, instruction state changes can
report changes involving registers. These registers are reported
with the name "r<num>" (e.g. "r3"). Print the CPU specific register
name instead of a generic name (e.g. print "rbx" instead of "r3"
on x86).

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/arch/loongarch/decode.c | 11 +++++++++++
 tools/objtool/arch/powerpc/decode.c   | 12 ++++++++++++
 tools/objtool/arch/x86/decode.c       |  8 ++++++++
 tools/objtool/include/objtool/arch.h  |  2 ++
 tools/objtool/trace.c                 |  7 +++++++
 5 files changed, 40 insertions(+)

diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index 09c43f008c9a..17eebf358e5d 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -8,6 +8,17 @@
 #include <linux/objtool_types.h>
 #include <arch/elf.h>
 
+const char *arch_reg_name[CFI_NUM_REGS] = {
+	"zero", "ra", "tp", "sp",
+	"a0", "a1", "a2", "a3",
+	"a4", "a5", "a6", "a7",
+	"t0", "t1", "t2", "t3",
+	"t4", "t5", "t6", "t7",
+	"t8", "u0", "fp", "s0",
+	"s1", "s2", "s3", "s4",
+	"s5", "s6", "s7", "s8"
+};
+
 int arch_ftrace_match(char *name)
 {
 	return !strcmp(name, "_mcount");
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 9c3f49c45587..74d1a8603535 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -10,6 +10,18 @@
 #include <objtool/builtin.h>
 #include <objtool/endianness.h>
 
+const char *arch_reg_name[CFI_NUM_REGS] = {
+	"r0",  "sp",  "r2",  "r3",
+	"r4",  "r5",  "r6",  "r7",
+	"r8",  "r9",  "r10", "r11",
+	"r12", "r13", "r14", "r15",
+	"r16", "r17", "r18", "r19",
+	"r20", "r21", "r22", "r23",
+	"r24", "r25", "r26", "r27",
+	"r28", "r29", "r30", "r31",
+	"ra"
+};
+
 int arch_ftrace_match(char *name)
 {
 	return !strcmp(name, "_mcount");
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 6a39cc619d63..6a8aad383695 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -24,6 +24,14 @@
 #include <objtool/builtin.h>
 #include <arch/elf.h>
 
+const char *arch_reg_name[CFI_NUM_REGS] = {
+	"rax", "rcx", "rdx", "rbx",
+	"rsp", "rbp", "rsi", "rdi",
+	"r8",  "r9",  "r10", "r11",
+	"r12", "r13", "r14", "r15",
+	"ra"
+};
+
 int arch_ftrace_match(char *name)
 {
 	return !strcmp(name, "__fentry__");
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index baa6eee1977f..f4aa8e134276 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -103,6 +103,8 @@ unsigned long arch_pc_relative_offset(struct instruction *insn,
 unsigned int arch_reloc_size(struct reloc *reloc);
 unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table);
 
+extern const char *arch_reg_name[CFI_NUM_REGS];
+
 #ifdef DISAS
 
 #include <bfd.h>
diff --git a/tools/objtool/trace.c b/tools/objtool/trace.c
index 28c0257b02b5..ea67c41e9128 100644
--- a/tools/objtool/trace.c
+++ b/tools/objtool/trace.c
@@ -34,6 +34,7 @@ int trace_depth;
 static const char *cfi_reg_name(unsigned int reg)
 {
 	static char rname_buffer[CFI_REG_NAME_MAXLEN];
+	const char *rname;
 
 	switch (reg) {
 	case CFI_UNDEFINED:
@@ -46,6 +47,12 @@ static const char *cfi_reg_name(unsigned int reg)
 		return "(bp)";
 	}
 
+	if (reg < CFI_NUM_REGS) {
+		rname = arch_reg_name[reg];
+		if (rname)
+			return rname;
+	}
+
 	if (snprintf(rname_buffer, CFI_REG_NAME_MAXLEN, "r%d", reg) == 1)
 		return NULL;
 
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 14/17] objtool: Improve tracing of alternative instructions
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (12 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 13/17] objtool: Improve register reporting " Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 15/17] objtool: Do not validate IBT for .return_sites and .call_sites Alexandre Chartre
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

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                 | 64 ++++++++++++++++++++++++---
 tools/objtool/include/objtool/trace.h | 33 ++++++++++++++
 2 files changed, 91 insertions(+), 6 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 8a51c871e1dc..43c88a5fd58a 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3517,7 +3517,7 @@ static bool skip_alt_group(struct instruction *insn)
 
 	/* ANNOTATE_IGNORE_ALTERNATIVE */
 	if (insn->alt_group && insn->alt_group->ignore) {
-		TRACE_INSN(insn, "alt group ignored");
+		TRACE_ALT(insn, "alt group ignored");
 		return true;
 	}
 
@@ -3641,22 +3641,74 @@ static int validate_insn(struct objtool_file *file, struct symbol *func,
 	if (insn->alts) {
 		int i, num_alts;
 
+		/*
+		 * Count the number of alternatives with an alt group.
+		 *
+		 * For a jump alternative, count is 0 and there is a single
+		 * alternative (with no alt group).
+		 *
+		 * For a group alternative, count is at least 1. In addition
+		 * there is an alternative that points to the code following
+		 * the alternative.
+		 */
 		num_alts = 0;
-		for (alt = insn->alts; alt; alt = alt->next)
-			num_alts++;
+		for (alt = insn->alts; alt; alt = alt->next) {
+			if (alt->insn->alt_group)
+				num_alts++;
+		}
 
 		i = 1;
 		for (alt = insn->alts; alt; alt = alt->next) {
-			TRACE_INSN(insn, "alternative %d/%d", i, num_alts);
+			if (!num_alts) {
+				/*
+				 * For a jump alternative, the non-default
+				 * branch is validated first. So if the
+				 * default instruction is a NOP then the
+				 * branch is validated first (jump taken),
+				 * otherwise the branch is not taken. Then
+				 * the default alternative is validated.
+				 */
+				if (insn->type == INSN_NOP)
+					TRACE_ALT_INFO(insn, "jump taken - begin");
+				else
+					TRACE_ALT_INFO(insn, "jump not taken - begin");
+			} else {
+				/*
+				 * For a group alternative, the code after the
+				 * alternative (alternative with no alt group)
+				 * is validated first. Then each alternative
+				 * is validated. Finally the default alternative
+				 * is validated.
+				 */
+				if (alt->insn->alt_group)
+					TRACE_ALT_INFO(insn, "alt %d/%d - begin", i, num_alts);
+				else
+					TRACE_ALT_INFO(insn, "after alternative - begin");
+			}
+
 			ret = validate_branch(file, func, alt->insn, *statep);
+
+			if (!num_alts) {
+				if (insn->type == INSN_NOP)
+					TRACE_ALT_INFO(insn, "jump taken - end");
+				else
+					TRACE_ALT_INFO(insn, "jump not taken - end");
+			} else {
+				if (alt->insn->alt_group)
+					TRACE_ALT_INFO_NOADDR(insn, "alt %d/%d - end", i, num_alts);
+				else
+					TRACE_ALT_INFO_NOADDR(insn, "after alternative - end");
+			}
+
 			if (ret) {
 				BT_INSN(insn, "(alt)");
 				return ret;
 			}
-			i++;
+			if (alt->insn->alt_group)
+				i++;
 		}
 
-		TRACE_INSN(insn, "alternative orig");
+		TRACE_ALT_INFO(insn, "default");
 	}
 
 	if (skip_alt_group(insn))
diff --git a/tools/objtool/include/objtool/trace.h b/tools/objtool/include/objtool/trace.h
index 5b8abdb9b09f..da3d41d6dedd 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(fmt) "<alternative.%lx> " fmt
+
+#define TRACE_ALT(insn, fmt, ...)				\
+	TRACE_INSN(insn, TRACE_ALT_FMT(fmt),			\
+		   (insn)->offset, ##__VA_ARGS__)
+
+#define TRACE_ALT_INFO(insn, fmt, ...)				\
+	TRACE_ADDR(insn, TRACE_ALT_FMT(fmt),			\
+		   (insn)->offset, ##__VA_ARGS__)
+
+#define TRACE_ALT_INFO_NOADDR(insn, fmt, ...)			\
+	TRACE_ADDR(NULL, TRACE_ALT_FMT(fmt),			\
+		   (insn)->offset, ##__VA_ARGS__)
+
 static inline void trace_enable(void)
 {
 	trace = true;
@@ -65,8 +94,12 @@ void trace_insn_state(struct instruction *insn, struct insn_state *sprev,
 #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) {}
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 15/17] objtool: Do not validate IBT for .return_sites and .call_sites
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (13 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 14/17] objtool: Improve tracing of alternative instructions Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 16/17] objtool: Add the --disas=<function-pattern> action Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 17/17] objtool: Disassemble all alternatives when using --disas Alexandre Chartre
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

The .return_sites and .call_sites sections reference text addresses,
but not with the intent to indirect branch to them, so they don't
need to be validated for IBT.

This is useful when running objtool on object files which already
have .return_sites or .call_sites sections, for example to re-run
objtool after it has reported an error or a warning.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 43c88a5fd58a..fecdf6f42358 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4645,6 +4645,8 @@ static int validate_ibt(struct objtool_file *file)
 		    !strcmp(sec->name, ".llvm.call-graph-profile")	||
 		    !strcmp(sec->name, ".llvm_bb_addr_map")		||
 		    !strcmp(sec->name, "__tracepoints")			||
+		    !strcmp(sec->name, ".return_sites")			||
+		    !strcmp(sec->name, ".call_sites")			||
 		    strstr(sec->name, "__patchable_function_entries"))
 			continue;
 
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 16/17] objtool: Add the --disas=<function-pattern> action
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (14 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 15/17] objtool: Do not validate IBT for .return_sites and .call_sites Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  2025-06-19 14:56 ` [RFC PATCH v2 17/17] objtool: Disassemble all alternatives when using --disas Alexandre Chartre
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

Add the --disas=<function-pattern> actions to disassemble the specified
functions. The function pattern can be a single function name (e.g.
--disas foo to disassemble the function with the name "foo"), or a shell
wildcard pattern (e.g. --disas foo* to disassemble all functions with a
name starting with "foo").

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/builtin-check.c           |  4 ++-
 tools/objtool/check.c                   | 35 ++++++++++++++-----------
 tools/objtool/disas.c                   | 28 ++++++++++++++++++++
 tools/objtool/include/objtool/builtin.h |  1 +
 tools/objtool/include/objtool/disas.h   |  2 ++
 5 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index c53d738e4fb0..49b37da28b90 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -73,6 +73,7 @@ static int parse_hacks(const struct option *opt, const char *str, int unset)
 
 static const struct option check_options[] = {
 	OPT_GROUP("Actions:"),
+	OPT_STRING_OPTARG('d',   "disas", &opts.disas, "function-pattern", "disassemble functions", "*"),
 	OPT_CALLBACK_OPTARG('h', "hacks", NULL, NULL, "jump_label,noinstr,skylake", "patch toolchain bugs/limitations", parse_hacks),
 	OPT_BOOLEAN('i', "ibt", &opts.ibt, "validate and annotate IBT"),
 	OPT_BOOLEAN('m', "mcount", &opts.mcount, "annotate mcount/fentry calls for ftrace"),
@@ -159,7 +160,8 @@ static bool opts_valid(void)
 		return false;
 	}
 
-	if (opts.hack_jump_label	||
+	if (opts.disas			||
+	    opts.hack_jump_label	||
 	    opts.hack_noinstr		||
 	    opts.ibt			||
 	    opts.mcount			||
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index fecdf6f42358..c7a70d47f104 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2563,7 +2563,7 @@ static int decode_sections(struct objtool_file *file)
 	 * Must be before add_jump_destinations(), which depends on 'func'
 	 * being set for alternatives, to enable proper sibling call detection.
 	 */
-	if (opts.stackval || opts.orc || opts.uaccess || opts.noinstr) {
+	if (opts.stackval || opts.orc || opts.uaccess || opts.noinstr || opts.disas) {
 		ret = add_special_section_alts(file);
 		if (ret)
 			return ret;
@@ -4766,14 +4766,15 @@ int check(struct objtool_file *file)
 	int ret = 0, warnings = 0;
 
 	/*
-	 * If the verbose or backtrace option is used then we need a
-	 * disassembly context to disassemble instruction or function
-	 * on warning or backtrace.
+	 * Create a disassembly context if we might disassemble any
+	 * instruction or function.
 	 */
-	if (opts.verbose || opts.backtrace || opts.trace) {
+	if (opts.verbose || opts.backtrace || opts.trace || opts.disas) {
 		disas_ctx = disas_context_create(file);
-		if (!disas_ctx)
+		if (!disas_ctx) {
+			opts.disas = false;
 			opts.trace = false;
+		}
 		objtool_disas_ctx = disas_ctx;
 	}
 
@@ -4892,19 +4893,21 @@ int check(struct objtool_file *file)
 	}
 
 out:
-	if (!ret && !warnings)
-		return 0;
-
-	if (opts.werror && warnings)
-		ret = 1;
-
-	if (opts.verbose) {
+	if (ret || warnings) {
 		if (opts.werror && warnings)
-			WARN("%d warning(s) upgraded to errors", warnings);
-		print_args();
-		disas_warned_funcs(disas_ctx);
+			ret = 1;
+
+		if (opts.verbose) {
+			if (opts.werror && warnings)
+				WARN("%d warning(s) upgraded to errors", warnings);
+			print_args();
+			disas_warned_funcs(disas_ctx);
+		}
 	}
 
+	if (opts.disas)
+		disas_funcs(disas_ctx);
+
 	if (disas_ctx) {
 		disas_context_destroy(disas_ctx);
 		objtool_disas_ctx = NULL;
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 376c7bedef47..97d506afdf45 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -3,6 +3,8 @@
  * Copyright (C) 2015-2017 Josh Poimboeuf <jpoimboe@redhat.com>
  */
 
+#include <fnmatch.h>
+
 #include <objtool/arch.h>
 #include <objtool/check.h>
 #include <objtool/disas.h>
@@ -399,3 +401,29 @@ void disas_warned_funcs(struct disas_context *dctx)
 			disas_func(dctx, sym);
 	}
 }
+
+void disas_funcs(struct disas_context *dctx)
+{
+	bool disas_all = !strcmp(opts.disas, "*");
+	struct section *sec;
+	struct symbol *sym;
+
+	for_each_sec(dctx->file, sec) {
+
+		if (!(sec->sh.sh_flags & SHF_EXECINSTR))
+			continue;
+
+		sec_for_each_sym(sec, sym) {
+			/*
+			 * If the function had a warning and the verbose
+			 * option is used then the function was already
+			 * disassemble.
+			 */
+			if (opts.verbose && sym->warned)
+				continue;
+
+			if (disas_all || fnmatch(opts.disas, sym->name, 0) == 0)
+				disas_func(dctx, sym);
+		}
+	}
+}
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index b3c84b6fdc5f..e41f3802d397 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -26,6 +26,7 @@ struct opts {
 	bool uaccess;
 	int prefix;
 	bool cfi;
+	const char *disas;
 
 	/* options: */
 	bool backtrace;
diff --git a/tools/objtool/include/objtool/disas.h b/tools/objtool/include/objtool/disas.h
index 5db75d06f219..5d2149ffac33 100644
--- a/tools/objtool/include/objtool/disas.h
+++ b/tools/objtool/include/objtool/disas.h
@@ -14,6 +14,7 @@ struct disassemble_info;
 struct disas_context *disas_context_create(struct objtool_file *file);
 void disas_context_destroy(struct disas_context *dctx);
 void disas_warned_funcs(struct disas_context *dctx);
+void disas_funcs(struct disas_context *dctx);
 int disas_info_init(struct disassemble_info *dinfo,
 		    int arch, int mach32, int mach64,
 		    const char *options);
@@ -37,6 +38,7 @@ static inline struct disas_context *disas_context_create(struct objtool_file *fi
 
 static inline void disas_context_destroy(struct disas_context *dctx) {}
 static inline void disas_warned_funcs(struct disas_context *dctx) {}
+static inline void disas_funcs(struct disas_context *dctx) {}
 
 static inline int disas_info_init(struct disassemble_info *dinfo,
 				  int arch, int mach32, int mach64,
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH v2 17/17] objtool: Disassemble all alternatives when using --disas
  2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
                   ` (15 preceding siblings ...)
  2025-06-19 14:56 ` [RFC PATCH v2 16/17] objtool: Add the --disas=<function-pattern> action Alexandre Chartre
@ 2025-06-19 14:56 ` Alexandre Chartre
  16 siblings, 0 replies; 18+ messages in thread
From: Alexandre Chartre @ 2025-06-19 14:56 UTC (permalink / raw)
  To: linux-kernel, mingo, jpoimboe, peterz; +Cc: alexandre.chartre

When using the --disas action, disassemble all code alternatives
instead of only the default alternative.

Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 tools/objtool/check.c                 |   8 --
 tools/objtool/disas.c                 | 152 +++++++++++++++++++++++++-
 tools/objtool/include/objtool/check.h |   8 ++
 3 files changed, 154 insertions(+), 14 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c7a70d47f104..fa66c8c6bf21 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -25,11 +25,6 @@
 #include <linux/static_call_types.h>
 #include <linux/string.h>
 
-struct alternative {
-	struct alternative *next;
-	struct instruction *insn;
-};
-
 static unsigned long nr_cfi, nr_cfi_reused, nr_cfi_cache;
 
 static struct cfi_init_state initial_func_cfi;
@@ -131,9 +126,6 @@ static struct instruction *prev_insn_same_sym(struct objtool_file *file,
 	     insn && insn->offset >= sym->offset;			\
 	     insn = prev_insn_same_sec(file, insn))
 
-#define sec_for_each_insn_from(file, insn)				\
-	for (; insn; insn = next_insn_same_sec(file, insn))
-
 #define sec_for_each_insn_continue(file, insn)				\
 	for (insn = next_insn_same_sec(file, insn); insn;		\
 	     insn = next_insn_same_sec(file, insn))
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index 97d506afdf45..9c202feee493 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -266,6 +266,12 @@ char *disas_result(struct disas_context *dctx)
 #define DISAS_INSN_OFFSET_SPACE		10
 #define DISAS_INSN_SPACE		60
 
+#define DISAS_PRINFO(insn, depth, format, ...)				\
+	disas_print_info(stdout, insn, depth, format "\n", ##__VA_ARGS__)
+
+#define DISAS_PRINSN(dctx, insn, depth)			\
+	disas_print_insn(stdout, dctx, insn, depth, "\n")
+
 /*
  * Print a message in the instruction flow. If insn is not NULL then
  * the instruction address is printed in addition of the message,
@@ -345,6 +351,82 @@ void disas_print_insn(FILE *stream, struct disas_context *dctx,
 	va_end(args);
 }
 
+static void disas_group_alt(struct disas_context *dctx,
+			    struct instruction *orig_insn)
+{
+	struct instruction *insn;
+	struct alternative *alt;
+	struct symbol *func;
+	int i, count;
+	int alt_id;
+
+	func = orig_insn->sym;
+	alt_id = orig_insn->offset;
+
+	count = 0;
+	for (alt = orig_insn->alts; alt; alt = alt->next) {
+		if (!alt->insn->alt_group)
+			/* jump alternative */
+			continue;
+
+		if (func && alt->insn->sec == func->sec)
+			/* exception handler */
+			continue;
+
+		/* count group alternatives, including fake nop */
+		count++;
+	}
+
+	i = 1;
+	for (alt = orig_insn->alts; alt; alt = alt->next) {
+
+		if (!alt->insn->alt_group)
+			continue;
+
+		/*
+		 * If the alternative instruction is in the same section
+		 * as the original instruction then that's an exception
+		 * handling i.e. branch to the alternative instruction if
+		 * there is an exception.
+		 */
+		if (func && alt->insn->sec == func->sec) {
+			DISAS_PRINFO(NULL, 0,
+				    "<alternative.%x> exception handler - begin", alt_id);
+			DISAS_PRINFO(orig_insn, 1, "jmp    %lx <%s+0x%lx>",
+				    alt->insn->offset, func->name,
+				    alt->insn->offset - func->offset);
+			DISAS_PRINFO(NULL, 0,
+				    "<alternative.%x> exception handler - end", alt_id);
+			continue;
+		}
+
+		DISAS_PRINFO(NULL, 0,
+			    "<alternative.%x> %d/%d - begin", alt_id, i, count);
+
+		insn = alt->insn->alt_group->first_insn;
+
+		sec_for_each_insn_from(dctx->file, insn) {
+
+			if (insn->alts) {
+				DISAS_PRINFO(insn, 0,
+					     "alt group: ignore nested alternative (not supported)");
+			}
+
+			DISAS_PRINSN(dctx, insn, 1);
+
+			if (insn == insn->alt_group->last_insn ||
+			    insn == insn->alt_group->nop)
+				break;
+
+			insn = next_insn_same_sec(dctx->file, insn);
+		}
+
+		DISAS_PRINFO(NULL, 0,
+			    "<alternative.%x> %d/%d end", alt_id, i, count);
+		i++;
+	}
+}
+
 /*
  * Disassemble a single instruction. Return the size of the instruction.
  */
@@ -367,21 +449,79 @@ size_t disas_insn(struct disas_context *dctx, struct instruction *insn)
 	return disasm(insn->offset, &dctx->info);
 }
 
+static void disas_jump_alt(struct disas_context *dctx, struct instruction *insn)
+{
+	struct instruction *alt_insn;
+	struct symbol *func;
+	int alt_id;
+
+	func = insn->sym;
+	alt_id = insn->offset;
+
+	DISAS_PRINFO(NULL, 0, "<jump alternative.%x> default", alt_id);
+	DISAS_PRINSN(dctx, insn, 1);
+
+	alt_insn = insn->alts->insn;
+	if (!alt_insn) {
+		WARN_INSN(insn, "no jump alternative");
+		return;
+	}
+
+	DISAS_PRINFO(NULL, 0, "<jump alternative.%x> else", alt_id);
+
+	if (insn->type == INSN_NOP) {
+		DISAS_PRINFO(insn, 1, "jmp    %lx <%s+0x%lx>",
+			    alt_insn->offset, func->name,
+			    alt_insn->offset - func->offset);
+	} else {
+		DISAS_PRINFO(insn, 1, "nop%d", insn->len);
+	}
+
+	DISAS_PRINFO(NULL, 0, "<jump alternative.%x> end", alt_id);
+
+	if (insn->alts->next)
+		WARN_INSN(insn, "more than one jump alternatives\n");
+}
+
 /*
  * Disassemble a function.
  */
 static void disas_func(struct disas_context *dctx, struct symbol *func)
 {
+	struct instruction *alt = NULL;
 	struct instruction *insn;
-	size_t addr;
+	int alt_id = 0;
+	int depth = 0;
 
 	printf("%s:\n", func->name);
 	sym_for_each_insn(dctx->file, func, insn) {
-		addr = insn->offset;
-		disas_insn(dctx, insn);
-		printf(" %6lx:  %s+0x%-6lx      %s\n",
-		       addr, func->name, addr - func->offset,
-		       disas_result(dctx));
+
+		if (insn->alts) {
+			if (alt) {
+				DISAS_PRINFO(insn, 0,
+					     "alt default: ignore nested alternative (not supported)");
+			} else if (insn->alt_group) {
+				alt = insn;
+				alt_id = alt->offset;
+				DISAS_PRINFO(NULL, 0,
+					    "<alternative.%x> default - begin", alt_id);
+				depth = 1;
+			} else {
+				disas_jump_alt(dctx, insn);
+				continue;
+			}
+		}
+
+		DISAS_PRINSN(dctx, insn, depth);
+
+		if (alt && (alt->alt_group->last_insn == insn ||
+			    alt->alt_group->nop == insn)) {
+			DISAS_PRINFO(NULL, 0,
+				    "<alternative.%x> default - end", alt_id);
+			disas_group_alt(dctx, alt);
+			alt = NULL;
+			depth = 0;
+		}
 	}
 	printf("\n");
 }
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index e5f97acb6252..748ee7631021 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -38,6 +38,11 @@ struct alt_group {
 	bool ignore;
 };
 
+struct alternative {
+	struct alternative *next;
+	struct instruction *insn;
+};
+
 #define INSN_CHUNK_BITS		8
 #define INSN_CHUNK_SIZE		(1 << INSN_CHUNK_BITS)
 #define INSN_CHUNK_MAX		(INSN_CHUNK_SIZE - 1)
@@ -135,6 +140,9 @@ struct instruction *next_insn_same_sec(struct objtool_file *file, struct instruc
 	     insn && insn->sec == _sec;					\
 	     insn = next_insn_same_sec(file, insn))
 
+#define sec_for_each_insn_from(file, insn)				\
+	for (; insn; insn = next_insn_same_sec(file, insn))
+
 #define sym_for_each_insn(file, sym, insn)				\
 	for (insn = find_insn(file, sym->sec, sym->offset);		\
 	     insn && insn->offset < sym->offset + sym->len;		\
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-06-19 14:57 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-19 14:56 [RFC PATCH v2 00/17] objtool: Function validation tracing Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 01/17] objtool: Move disassembly functions to a separated file Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 02/17] objtool: Create disassembly context Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 03/17] objtool: Disassemble code with libopcodes instead of running objdump Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 04/17] tool build: Remove annoying newline in build output Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 05/17] objtool: Print symbol during disassembly Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 06/17] objtool: Improve offstr() output Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 07/17] objtool: Store instruction disassembly result Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 08/17] objtool: Disassemble instruction on warning or backtrace Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 09/17] objtool: Extract code to validate instruction from the validate branch loop Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 10/17] objtool: Record symbol name max length Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 11/17] objtool: Add option to trace function validation Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 12/17] objtool: Trace instruction state changes during " Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 13/17] objtool: Improve register reporting " Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 14/17] objtool: Improve tracing of alternative instructions Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 15/17] objtool: Do not validate IBT for .return_sites and .call_sites Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 16/17] objtool: Add the --disas=<function-pattern> action Alexandre Chartre
2025-06-19 14:56 ` [RFC PATCH v2 17/17] objtool: Disassemble all alternatives when using --disas Alexandre Chartre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).