Re: [PATCH v2 35/62] objtool: Refactor add_jump_destinations()
On Fri, May 23, 2025 at 07:46:19AM -0400, Joe Lawrence wrote:
> > - } else if (reloc->sym->sec->idx) {
> > - dest_sec = reloc->sym->sec;
> > - dest_off = reloc->sym->sym.st_value +
> > - arch_dest_reloc_offset(reloc_addend(reloc));
>
> Way back in ("[PATCH v2 18/62] objtool: Fix x86 addend calculation"),
> arch_dest_reloc_offset() was replaced with arch_insn_adjusted_addend(),
> so I think that patch missed this callsite and breaks bisectability.
Fixed, thanks.
--
Josh
Re: [PATCH v2 35/62] objtool: Refactor add_jump_destinations()
On 5/9/25 4:16 PM, Josh Poimboeuf wrote:
> The add_jump_destinations() logic is a bit weird and convoluted after
> being incrementally tweaked over the years. Refactor it to hopefully be
> more logical and straightforward.
>
> Signed-off-by: Josh Poimboeuf
> ---
> tools/objtool/check.c | 227 +---
> tools/objtool/include/objtool/elf.h | 4 +-
> 2 files changed, 104 insertions(+), 127 deletions(-)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 66cbeebd16ea..e4ca5edf73ad 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1439,9 +1439,14 @@ static void add_return_call(struct objtool_file *file,
> struct instruction *insn,
> }
>
> static bool is_first_func_insn(struct objtool_file *file,
> -struct instruction *insn, struct symbol *sym)
> +struct instruction *insn)
> {
> - if (insn->offset == sym->offset)
> + struct symbol *func = insn_func(insn);
> +
> + if (!func)
> + return false;
> +
> + if (insn->offset == func->offset)
> return true;
>
> /* Allow direct CALL/JMP past ENDBR */
> @@ -1449,52 +1454,32 @@ static bool is_first_func_insn(struct objtool_file
> *file,
> struct instruction *prev = prev_insn_same_sym(file, insn);
>
> if (prev && prev->type == INSN_ENDBR &&
> - insn->offset == sym->offset + prev->len)
> + insn->offset == func->offset + prev->len)
> return true;
> }
>
> return false;
> }
>
> -/*
> - * A sibling call is a tail-call to another symbol -- to differentiate from a
> - * recursive tail-call which is to the same symbol.
> - */
> -static bool jump_is_sibling_call(struct objtool_file *file,
> - struct instruction *from, struct instruction
> *to)
> -{
> - struct symbol *fs = from->sym;
> - struct symbol *ts = to->sym;
> -
> - /* Not a sibling call if from/to a symbol hole */
> - if (!fs || !ts)
> - return false;
> -
> - /* Not a sibling call if not targeting the start of a symbol. */
> - if (!is_first_func_insn(file, to, ts))
> - return false;
> -
> - /* Disallow sibling calls into STT_NOTYPE */
> - if (is_notype_sym(ts))
> - return false;
> -
> - /* Must not be self to be a sibling */
> - return fs->pfunc != ts->pfunc;
> -}
> -
> /*
> * Find the destination instructions for all jumps.
> */
> static int add_jump_destinations(struct objtool_file *file)
> {
> - struct instruction *insn, *jump_dest;
> + struct instruction *insn;
> struct reloc *reloc;
> - struct section *dest_sec;
> - unsigned long dest_off;
> int ret;
>
> for_each_insn(file, insn) {
> struct symbol *func = insn_func(insn);
> + struct instruction *dest_insn;
> + struct section *dest_sec;
> + struct symbol *dest_sym;
> + unsigned long dest_off;
> + bool dest_undef = false;
> +
> + if (!is_static_jump(insn))
> + continue;
>
> if (insn->jump_dest) {
> /*
> @@ -1503,129 +1488,121 @@ static int add_jump_destinations(struct
> objtool_file *file)
>*/
> continue;
> }
> - if (!is_static_jump(insn))
> - continue;
>
> reloc = insn_reloc(file, insn);
> if (!reloc) {
> dest_sec = insn->sec;
> dest_off = arch_jump_destination(insn);
> - } else if (is_sec_sym(reloc->sym)) {
> + } else if (is_undef_sym(reloc->sym)) {
> + dest_sym = reloc->sym;
> + dest_undef = true;
> + } else {
> dest_sec = reloc->sym->sec;
> - dest_off = arch_insn_adjusted_addend(insn, reloc);
> - } else if (reloc->sym->retpoline_thunk) {
> + dest_off = reloc->sym->sym.st_value +
> +arch_insn_adjusted_addend(insn, reloc);
> + }
> +
> + if (!dest_undef) {
> + dest_insn = find_insn(file, dest_sec, dest_off);
> + if (!dest_insn) {
> + struct symbol *sym =
> find_symbol_by_offset(dest_sec, dest_off);
> +
> + /*
> + * retbleed_untrain_ret() jumps to
> + * __x86_return_thunk(), but objtool can't find
> + * the thunk's starting RET instruction,
> + * because the RET is also in the middle of
> + * another instruction. Objtool only knows
> + * about the outer instruct
[PATCH v2 35/62] objtool: Refactor add_jump_destinations()
The add_jump_destinations() logic is a bit weird and convoluted after
being incrementally tweaked over the years. Refactor it to hopefully be
more logical and straightforward.
Signed-off-by: Josh Poimboeuf
---
tools/objtool/check.c | 227 +---
tools/objtool/include/objtool/elf.h | 4 +-
2 files changed, 104 insertions(+), 127 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 66cbeebd16ea..e4ca5edf73ad 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1439,9 +1439,14 @@ static void add_return_call(struct objtool_file *file,
struct instruction *insn,
}
static bool is_first_func_insn(struct objtool_file *file,
- struct instruction *insn, struct symbol *sym)
+ struct instruction *insn)
{
- if (insn->offset == sym->offset)
+ struct symbol *func = insn_func(insn);
+
+ if (!func)
+ return false;
+
+ if (insn->offset == func->offset)
return true;
/* Allow direct CALL/JMP past ENDBR */
@@ -1449,52 +1454,32 @@ static bool is_first_func_insn(struct objtool_file
*file,
struct instruction *prev = prev_insn_same_sym(file, insn);
if (prev && prev->type == INSN_ENDBR &&
- insn->offset == sym->offset + prev->len)
+ insn->offset == func->offset + prev->len)
return true;
}
return false;
}
-/*
- * A sibling call is a tail-call to another symbol -- to differentiate from a
- * recursive tail-call which is to the same symbol.
- */
-static bool jump_is_sibling_call(struct objtool_file *file,
-struct instruction *from, struct instruction
*to)
-{
- struct symbol *fs = from->sym;
- struct symbol *ts = to->sym;
-
- /* Not a sibling call if from/to a symbol hole */
- if (!fs || !ts)
- return false;
-
- /* Not a sibling call if not targeting the start of a symbol. */
- if (!is_first_func_insn(file, to, ts))
- return false;
-
- /* Disallow sibling calls into STT_NOTYPE */
- if (is_notype_sym(ts))
- return false;
-
- /* Must not be self to be a sibling */
- return fs->pfunc != ts->pfunc;
-}
-
/*
* Find the destination instructions for all jumps.
*/
static int add_jump_destinations(struct objtool_file *file)
{
- struct instruction *insn, *jump_dest;
+ struct instruction *insn;
struct reloc *reloc;
- struct section *dest_sec;
- unsigned long dest_off;
int ret;
for_each_insn(file, insn) {
struct symbol *func = insn_func(insn);
+ struct instruction *dest_insn;
+ struct section *dest_sec;
+ struct symbol *dest_sym;
+ unsigned long dest_off;
+ bool dest_undef = false;
+
+ if (!is_static_jump(insn))
+ continue;
if (insn->jump_dest) {
/*
@@ -1503,129 +1488,121 @@ static int add_jump_destinations(struct objtool_file
*file)
*/
continue;
}
- if (!is_static_jump(insn))
- continue;
reloc = insn_reloc(file, insn);
if (!reloc) {
dest_sec = insn->sec;
dest_off = arch_jump_destination(insn);
- } else if (is_sec_sym(reloc->sym)) {
+ } else if (is_undef_sym(reloc->sym)) {
+ dest_sym = reloc->sym;
+ dest_undef = true;
+ } else {
dest_sec = reloc->sym->sec;
- dest_off = arch_insn_adjusted_addend(insn, reloc);
- } else if (reloc->sym->retpoline_thunk) {
+ dest_off = reloc->sym->sym.st_value +
+ arch_insn_adjusted_addend(insn, reloc);
+ }
+
+ if (!dest_undef) {
+ dest_insn = find_insn(file, dest_sec, dest_off);
+ if (!dest_insn) {
+ struct symbol *sym =
find_symbol_by_offset(dest_sec, dest_off);
+
+ /*
+* retbleed_untrain_ret() jumps to
+* __x86_return_thunk(), but objtool can't find
+* the thunk's starting RET instruction,
+* because the RET is also in the middle of
+* another instruction. Objtool only knows
+* about the outer instruction.
+*/
+ if (sym && sym->embedded_insn) {
+ add_ret
