* [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls @ 2025-11-03 18:51 xur 2025-11-03 18:51 ` [PATCH 2/2] objtool: dead_end function change for split functions xur 2025-11-03 21:01 ` [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls Peter Zijlstra 0 siblings, 2 replies; 6+ messages in thread From: xur @ 2025-11-03 18:51 UTC (permalink / raw) To: Josh Poimboeuf, Peter Zijlstra, Rong Xu Cc: linux-kernel, Sriraman Tallam, Han Shen, Krzysztof Pszeniczny From: Rong Xu <xur@google.com> If a function has multiple sibling calls, the dead_end check should only return true if all sibling call targets are also dead_end functions. Signed-off-by: Rong Xu <xur@google.com> Reviewed-by: Sriraman Tallam <tmsriram@google.com> Reviewed-by: Han Shen <shenhan@google.com> Reviewed-by: Krzysztof Pszeniczny <kpszeniczny@google.com> --- tools/objtool/check.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 9004fbc067693..c2ee3c3a84a62 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -314,7 +314,13 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func, return false; } - return __dead_end_function(file, insn_func(dest), recursion+1); + /* + * A function can have multiple sibling calls. All of + * them need to be dead ends for the function to be a + * dead end too. + */ + if (!__dead_end_function(file, insn_func(dest), recursion+1)) + return false; } } base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0 -- 2.51.2.997.g839fc31de9-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] objtool: dead_end function change for split functions 2025-11-03 18:51 [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls xur @ 2025-11-03 18:51 ` xur 2025-11-03 21:19 ` Peter Zijlstra 2025-11-03 21:01 ` [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls Peter Zijlstra 1 sibling, 1 reply; 6+ messages in thread From: xur @ 2025-11-03 18:51 UTC (permalink / raw) To: Josh Poimboeuf, Peter Zijlstra, Rong Xu Cc: linux-kernel, Sriraman Tallam, Han Shen, Krzysztof Pszeniczny From: Rong Xu <xur@google.com> Function Splitting can potentially move all return instructions into the cold (infrequently executed) section of the function. If this happens, the original function might be incorrectly flagged as a dead-end function. The consequence is an incomplete ORC table, which leads to an unwind error, and subsequently, a livepatch failure. This patch adds the support of the dead_end_function check for split function. Signed-off-by: Rong Xu <xur@google.com> Reviewed-by: Sriraman Tallam <tmsriram@google.com> Reviewed-by: Han Shen <shenhan@google.com> Reviewed-by: Krzysztof Pszeniczny <kpszeniczny@google.com> --- tools/objtool/check.c | 88 +++++++++++++++++++++++++++++++++---------- 1 file changed, 69 insertions(+), 19 deletions(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index c2ee3c3a84a62..b752cf508d09a 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -237,6 +237,73 @@ static bool is_rust_noreturn(const struct symbol *func) str_ends_with(func->name, "_fail")); } +static bool __dead_end_function(struct objtool_file *, struct symbol *, int); + +/* + * Check if the target of a sibling_call instruction is a dead_end function. + * Note insn must be a sibling call. + */ +static inline bool __dead_end_sibling_call(struct objtool_file *file, + struct instruction *insn, int recursion) { + struct instruction *dest = insn->jump_dest; + + if (!dest) + /* sibling call to another file */ + return false; + + /* local sibling call */ + if (recursion == 5) { + /* + * Infinite recursion: two functions have + * sibling calls to each other. This is a very + * rare case. It means they aren't dead ends. + */ + return false; + } + + return __dead_end_function(file, insn_func(dest), recursion+1); +} + +/* + * Handling split functions. Mimic the workflow in __dead_end_function. + */ +static bool __dead_end_split_func(struct objtool_file *file, + struct symbol *func, int recursion) +{ + char section_name[256]; + struct section *sec; + struct instruction *insn; + + /* + * Use a fixed-size buffer (max 256) to avoid malloc. If the section + * length exceeds this limit, we return a conservative value. This is + * a safe fallback and does not compromise functional correctness. + */ + if (snprintf(section_name, sizeof(section_name), ".text.split.%s", + func->name) >= sizeof(section_name)) { + fprintf(stderr, "Error: Function name '%s' too long.\n", func->name); + return false; + } + + sec = find_section_by_name(file->elf, section_name); + if (!sec) + return false; + + sec_for_each_insn(file, sec, insn) { + if (insn->type == INSN_RETURN) + return false; + } + + sec_for_each_insn(file, sec, insn) { + if (is_sibling_call(insn)) { + if (!__dead_end_sibling_call(file, insn, recursion)) + return false; + } + } + + return true; +} + /* * This checks to see if the given function is a "noreturn" function. * @@ -298,33 +365,16 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func, */ func_for_each_insn(file, func, insn) { if (is_sibling_call(insn)) { - struct instruction *dest = insn->jump_dest; - - if (!dest) - /* sibling call to another file */ - return false; - - /* local sibling call */ - if (recursion == 5) { - /* - * Infinite recursion: two functions have - * sibling calls to each other. This is a very - * rare case. It means they aren't dead ends. - */ - return false; - } - /* * A function can have multiple sibling calls. All of * them need to be dead ends for the function to be a * dead end too. */ - if (!__dead_end_function(file, insn_func(dest), recursion+1)) + if (!__dead_end_sibling_call(file, insn, recursion)) return false; } } - - return true; + return __dead_end_split_func(file, func, recursion); } static bool dead_end_function(struct objtool_file *file, struct symbol *func) -- 2.51.2.997.g839fc31de9-goog ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] objtool: dead_end function change for split functions 2025-11-03 18:51 ` [PATCH 2/2] objtool: dead_end function change for split functions xur @ 2025-11-03 21:19 ` Peter Zijlstra 2025-11-03 21:40 ` Rong Xu 0 siblings, 1 reply; 6+ messages in thread From: Peter Zijlstra @ 2025-11-03 21:19 UTC (permalink / raw) To: xur Cc: Josh Poimboeuf, linux-kernel, Sriraman Tallam, Han Shen, Krzysztof Pszeniczny On Mon, Nov 03, 2025 at 06:51:54PM +0000, xur@google.com wrote: > From: Rong Xu <xur@google.com> > > Function Splitting can potentially move all return instructions > into the cold (infrequently executed) section of the function. > If this happens, the original function might be incorrectly > flagged as a dead-end function. > > The consequence is an incomplete ORC table, which leads to an unwind > error, and subsequently, a livepatch failure. > > This patch adds the support of the dead_end_function check for > split function. > > Signed-off-by: Rong Xu <xur@google.com> > Reviewed-by: Sriraman Tallam <tmsriram@google.com> > Reviewed-by: Han Shen <shenhan@google.com> > Reviewed-by: Krzysztof Pszeniczny <kpszeniczny@google.com> > --- > tools/objtool/check.c | 88 +++++++++++++++++++++++++++++++++---------- > 1 file changed, 69 insertions(+), 19 deletions(-) > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > index c2ee3c3a84a62..b752cf508d09a 100644 > --- a/tools/objtool/check.c > +++ b/tools/objtool/check.c > @@ -237,6 +237,73 @@ static bool is_rust_noreturn(const struct symbol *func) > str_ends_with(func->name, "_fail")); > } > > +static bool __dead_end_function(struct objtool_file *, struct symbol *, int); > + > +/* > + * Check if the target of a sibling_call instruction is a dead_end function. > + * Note insn must be a sibling call. > + */ > +static inline bool __dead_end_sibling_call(struct objtool_file *file, > + struct instruction *insn, int recursion) { Please: cino=(0:0 also for functions { on a new line. > + struct instruction *dest = insn->jump_dest; > + > + if (!dest) > + /* sibling call to another file */ > + return false; I know this is just code movement, but this wants {} per coding style. > + > + /* local sibling call */ > + if (recursion == 5) { > + /* > + * Infinite recursion: two functions have > + * sibling calls to each other. This is a very > + * rare case. It means they aren't dead ends. > + */ > + return false; > + } > + > + return __dead_end_function(file, insn_func(dest), recursion+1); > +} > + > +/* > + * Handling split functions. Mimic the workflow in __dead_end_function. > + */ > +static bool __dead_end_split_func(struct objtool_file *file, > + struct symbol *func, int recursion) cino=(0:0 > +{ > + char section_name[256]; > + struct section *sec; > + struct instruction *insn; > + > + /* > + * Use a fixed-size buffer (max 256) to avoid malloc. If the section > + * length exceeds this limit, we return a conservative value. This is > + * a safe fallback and does not compromise functional correctness. > + */ > + if (snprintf(section_name, sizeof(section_name), ".text.split.%s", > + func->name) >= sizeof(section_name)) { That is a terribly confusing line-break to read. Might've been better to split after the greate-or-equal sign. > + fprintf(stderr, "Error: Function name '%s' too long.\n", func->name); > + return false; > + } > + > + sec = find_section_by_name(file->elf, section_name); > + if (!sec) > + return false; > + > + sec_for_each_insn(file, sec, insn) { > + if (insn->type == INSN_RETURN) > + return false; > + } > + > + sec_for_each_insn(file, sec, insn) { > + if (is_sibling_call(insn)) { > + if (!__dead_end_sibling_call(file, insn, recursion)) > + return false; > + } > + } > + > + return true; > +} > + > /* > * This checks to see if the given function is a "noreturn" function. > * > @@ -298,33 +365,16 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func, > */ > func_for_each_insn(file, func, insn) { > if (is_sibling_call(insn)) { > - struct instruction *dest = insn->jump_dest; > - > - if (!dest) > - /* sibling call to another file */ > - return false; > - > - /* local sibling call */ > - if (recursion == 5) { > - /* > - * Infinite recursion: two functions have > - * sibling calls to each other. This is a very > - * rare case. It means they aren't dead ends. > - */ > - return false; > - } > - > /* > * A function can have multiple sibling calls. All of > * them need to be dead ends for the function to be a > * dead end too. > */ > - if (!__dead_end_function(file, insn_func(dest), recursion+1)) > + if (!__dead_end_sibling_call(file, insn, recursion)) > return false; > } > } > - > - return true; > + return __dead_end_split_func(file, func, recursion); > } Aside from some coding style nits, this seems like it will do. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] objtool: dead_end function change for split functions 2025-11-03 21:19 ` Peter Zijlstra @ 2025-11-03 21:40 ` Rong Xu 0 siblings, 0 replies; 6+ messages in thread From: Rong Xu @ 2025-11-03 21:40 UTC (permalink / raw) To: Peter Zijlstra Cc: Josh Poimboeuf, linux-kernel, Sriraman Tallam, Han Shen, Krzysztof Pszeniczny Peter, thanks for the review! I will fix the style and send patch v2. -Rong On Mon, Nov 3, 2025 at 1:19 PM Peter Zijlstra <peterz@infradead.org> wrote: > > On Mon, Nov 03, 2025 at 06:51:54PM +0000, xur@google.com wrote: > > From: Rong Xu <xur@google.com> > > > > Function Splitting can potentially move all return instructions > > into the cold (infrequently executed) section of the function. > > If this happens, the original function might be incorrectly > > flagged as a dead-end function. > > > > The consequence is an incomplete ORC table, which leads to an unwind > > error, and subsequently, a livepatch failure. > > > > This patch adds the support of the dead_end_function check for > > split function. > > > > Signed-off-by: Rong Xu <xur@google.com> > > Reviewed-by: Sriraman Tallam <tmsriram@google.com> > > Reviewed-by: Han Shen <shenhan@google.com> > > Reviewed-by: Krzysztof Pszeniczny <kpszeniczny@google.com> > > --- > > tools/objtool/check.c | 88 +++++++++++++++++++++++++++++++++---------- > > 1 file changed, 69 insertions(+), 19 deletions(-) > > > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > > index c2ee3c3a84a62..b752cf508d09a 100644 > > --- a/tools/objtool/check.c > > +++ b/tools/objtool/check.c > > @@ -237,6 +237,73 @@ static bool is_rust_noreturn(const struct symbol *func) > > str_ends_with(func->name, "_fail")); > > } > > > > +static bool __dead_end_function(struct objtool_file *, struct symbol *, int); > > + > > +/* > > + * Check if the target of a sibling_call instruction is a dead_end function. > > + * Note insn must be a sibling call. > > + */ > > +static inline bool __dead_end_sibling_call(struct objtool_file *file, > > + struct instruction *insn, int recursion) { > > Please: cino=(0:0 > also for functions { on a new line. > > > + struct instruction *dest = insn->jump_dest; > > + > > + if (!dest) > > + /* sibling call to another file */ > > + return false; > > I know this is just code movement, but this wants {} per coding style. > > > + > > + /* local sibling call */ > > + if (recursion == 5) { > > + /* > > + * Infinite recursion: two functions have > > + * sibling calls to each other. This is a very > > + * rare case. It means they aren't dead ends. > > + */ > > + return false; > > + } > > + > > + return __dead_end_function(file, insn_func(dest), recursion+1); > > +} > > + > > +/* > > + * Handling split functions. Mimic the workflow in __dead_end_function. > > + */ > > +static bool __dead_end_split_func(struct objtool_file *file, > > + struct symbol *func, int recursion) > > cino=(0:0 > > > +{ > > + char section_name[256]; > > + struct section *sec; > > + struct instruction *insn; > > + > > + /* > > + * Use a fixed-size buffer (max 256) to avoid malloc. If the section > > + * length exceeds this limit, we return a conservative value. This is > > + * a safe fallback and does not compromise functional correctness. > > + */ > > + if (snprintf(section_name, sizeof(section_name), ".text.split.%s", > > + func->name) >= sizeof(section_name)) { > > That is a terribly confusing line-break to read. Might've been better to > split after the greate-or-equal sign. > > > + fprintf(stderr, "Error: Function name '%s' too long.\n", func->name); > > + return false; > > + } > > + > > + sec = find_section_by_name(file->elf, section_name); > > + if (!sec) > > + return false; > > + > > + sec_for_each_insn(file, sec, insn) { > > + if (insn->type == INSN_RETURN) > > + return false; > > + } > > + > > + sec_for_each_insn(file, sec, insn) { > > + if (is_sibling_call(insn)) { > > + if (!__dead_end_sibling_call(file, insn, recursion)) > > + return false; > > + } > > + } > > + > > + return true; > > +} > > + > > /* > > * This checks to see if the given function is a "noreturn" function. > > * > > @@ -298,33 +365,16 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func, > > */ > > func_for_each_insn(file, func, insn) { > > if (is_sibling_call(insn)) { > > - struct instruction *dest = insn->jump_dest; > > - > > - if (!dest) > > - /* sibling call to another file */ > > - return false; > > - > > - /* local sibling call */ > > - if (recursion == 5) { > > - /* > > - * Infinite recursion: two functions have > > - * sibling calls to each other. This is a very > > - * rare case. It means they aren't dead ends. > > - */ > > - return false; > > - } > > - > > /* > > * A function can have multiple sibling calls. All of > > * them need to be dead ends for the function to be a > > * dead end too. > > */ > > - if (!__dead_end_function(file, insn_func(dest), recursion+1)) > > + if (!__dead_end_sibling_call(file, insn, recursion)) > > return false; > > } > > } > > - > > - return true; > > + return __dead_end_split_func(file, func, recursion); > > } > > Aside from some coding style nits, this seems like it will do. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls 2025-11-03 18:51 [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls xur 2025-11-03 18:51 ` [PATCH 2/2] objtool: dead_end function change for split functions xur @ 2025-11-03 21:01 ` Peter Zijlstra 2025-11-03 21:11 ` Rong Xu 1 sibling, 1 reply; 6+ messages in thread From: Peter Zijlstra @ 2025-11-03 21:01 UTC (permalink / raw) To: xur Cc: Josh Poimboeuf, linux-kernel, Sriraman Tallam, Han Shen, Krzysztof Pszeniczny On Mon, Nov 03, 2025 at 06:51:53PM +0000, xur@google.com wrote: > From: Rong Xu <xur@google.com> > > If a function has multiple sibling calls, the dead_end check should > only return true if all sibling call targets are also dead_end > functions. Cute, however did you find this? > Signed-off-by: Rong Xu <xur@google.com> > Reviewed-by: Sriraman Tallam <tmsriram@google.com> > Reviewed-by: Han Shen <shenhan@google.com> > Reviewed-by: Krzysztof Pszeniczny <kpszeniczny@google.com> > --- > tools/objtool/check.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > index 9004fbc067693..c2ee3c3a84a62 100644 > --- a/tools/objtool/check.c > +++ b/tools/objtool/check.c > @@ -314,7 +314,13 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func, > return false; > } > > - return __dead_end_function(file, insn_func(dest), recursion+1); > + /* > + * A function can have multiple sibling calls. All of > + * them need to be dead ends for the function to be a > + * dead end too. > + */ > + if (!__dead_end_function(file, insn_func(dest), recursion+1)) > + return false; > } > } > > > base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0 > -- > 2.51.2.997.g839fc31de9-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls 2025-11-03 21:01 ` [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls Peter Zijlstra @ 2025-11-03 21:11 ` Rong Xu 0 siblings, 0 replies; 6+ messages in thread From: Rong Xu @ 2025-11-03 21:11 UTC (permalink / raw) To: Peter Zijlstra Cc: Josh Poimboeuf, linux-kernel, Sriraman Tallam, Han Shen, Krzysztof Pszeniczny While investigating a fix for the split function, I noticed this issue in the code. I'm not sure if we have multiple sibling calls in the current kernel source. But I think we should fix it. The split function issue did affect us in the production. Thanks, -Rong On Mon, Nov 3, 2025 at 1:01 PM Peter Zijlstra <peterz@infradead.org> wrote: > > On Mon, Nov 03, 2025 at 06:51:53PM +0000, xur@google.com wrote: > > From: Rong Xu <xur@google.com> > > > > If a function has multiple sibling calls, the dead_end check should > > only return true if all sibling call targets are also dead_end > > functions. > > Cute, however did you find this? > > > Signed-off-by: Rong Xu <xur@google.com> > > Reviewed-by: Sriraman Tallam <tmsriram@google.com> > > Reviewed-by: Han Shen <shenhan@google.com> > > Reviewed-by: Krzysztof Pszeniczny <kpszeniczny@google.com> > > --- > > tools/objtool/check.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > > index 9004fbc067693..c2ee3c3a84a62 100644 > > --- a/tools/objtool/check.c > > +++ b/tools/objtool/check.c > > @@ -314,7 +314,13 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func, > > return false; > > } > > > > - return __dead_end_function(file, insn_func(dest), recursion+1); > > + /* > > + * A function can have multiple sibling calls. All of > > + * them need to be dead ends for the function to be a > > + * dead end too. > > + */ > > + if (!__dead_end_function(file, insn_func(dest), recursion+1)) > > + return false; > > } > > } > > > > > > base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0 > > -- > > 2.51.2.997.g839fc31de9-goog > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-03 21:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-11-03 18:51 [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls xur 2025-11-03 18:51 ` [PATCH 2/2] objtool: dead_end function change for split functions xur 2025-11-03 21:19 ` Peter Zijlstra 2025-11-03 21:40 ` Rong Xu 2025-11-03 21:01 ` [PATCH 1/2] objtool: fix the check for dead_end function with multiple sibliing calls Peter Zijlstra 2025-11-03 21:11 ` Rong Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®