[PATCH v2 1/2] objtool: fix the check for dead_end function with multiple sibliing calls

[PATCH v2 1/2] objtool: fix the check for dead_end function with multiple sibliing calls

[xur]

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

[PATCH v2 2/2] objtool: dead_end function change for split functions

[xur]

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>

ChangeLOG:
 v2: Coding style changes suggested by Peter Zijlstra.
---
 tools/objtool/check.c | 91 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 72 insertions(+), 19 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c2ee3c3a84a62..4864d54cdd79e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -237,6 +237,76 @@ 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;
+	int n;
+
+	/*
+	 * 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.
+	 */
+	n = sizeof(section_name);
+	if (snprintf(section_name, n, ".text.split.%s", func->name) >= n) {
+		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 +368,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

Re: [PATCH v2 2/2] objtool: dead_end function change for split functions

[Rong Xu]

Peter and Josh,

Do you have any other suggestions for these two patches?

-Rong

On Mon, Nov 3, 2025 at 1:52 PM <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>
>
> ChangeLOG:
>  v2: Coding style changes suggested by Peter Zijlstra.
> ---
>  tools/objtool/check.c | 91 ++++++++++++++++++++++++++++++++++---------
>  1 file changed, 72 insertions(+), 19 deletions(-)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index c2ee3c3a84a62..4864d54cdd79e 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -237,6 +237,76 @@ 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;
> +       int n;
> +
> +       /*
> +        * 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.
> +        */
> +       n = sizeof(section_name);
> +       if (snprintf(section_name, n, ".text.split.%s", func->name) >= n) {
> +               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 +368,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
>

Re: [PATCH v2 2/2] objtool: dead_end function change for split functions

[Josh Poimboeuf]

On Mon, Nov 03, 2025 at 09:52:44PM +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.

I assume there were also some ignored objtool warnings?  Ignoring those
can have catastrophic consequences (beyond just failing livepatches) so
I highly recommend building with CONFIG_OBJTOOL_WERROR.

> 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>

I was scratching my head about this one for a while.  .text.split.<func>
is for <func>.cold code, which should automatically get iterated by the
outer func_for_each_insn() construct in __dead_end_function().  So this
patch shouldn't be necessary.

After some testing with Clang -fsplit-machine-functions, I'm realizing
the problem is that unlike GCC, Clang .cold code (in .text.split.*)
isn't STT_FUNC.  Which breaks several objtool assumptions.  So something
like the below is needed.

I've also discovered a few more .cold-related issues... working on
fixing those.

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 3f20b257ab25..ab2d4fe37fb3 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -502,8 +502,11 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
 	if (strstarts(sym->name, ".klp.sym"))
 		sym->klp = 1;
 
-	if (!sym->klp && is_func_sym(sym) && strstr(sym->name, ".cold"))
+	if (!sym->klp && is_func_sym(sym) && strstr(sym->name, ".cold")) {
 		sym->cold = 1;
+		sym->type = STT_FUNC;
+	}
+
 	sym->pfunc = sym->cfunc = sym;
 
 	sym->demangled_name = demangle_name(sym);

Re: [PATCH v2 2/2] objtool: dead_end function change for split functions

[Josh Poimboeuf]

On Mon, Nov 17, 2025 at 01:43:26PM -0800, Josh Poimboeuf wrote:
> On Mon, Nov 03, 2025 at 09:52:44PM +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.
> 
> I assume there were also some ignored objtool warnings?  Ignoring those
> can have catastrophic consequences (beyond just failing livepatches) so
> I highly recommend building with CONFIG_OBJTOOL_WERROR.
> 
> > 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>
> 
> I was scratching my head about this one for a while.  .text.split.<func>
> is for <func>.cold code, which should automatically get iterated by the
> outer func_for_each_insn() construct in __dead_end_function().  So this
> patch shouldn't be necessary.
> 
> After some testing with Clang -fsplit-machine-functions, I'm realizing
> the problem is that unlike GCC, Clang .cold code (in .text.split.*)
> isn't STT_FUNC.  Which breaks several objtool assumptions.  So something
> like the below is needed.

That last diff had a blatant bug, here is the proper fix:

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 3f20b257ab25..5a9efbda880b 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -502,8 +502,16 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
 	if (strstarts(sym->name, ".klp.sym"))
 		sym->klp = 1;
 
-	if (!sym->klp && is_func_sym(sym) && strstr(sym->name, ".cold"))
+	if (!sym->klp && strstr(sym->name, ".cold")) {
 		sym->cold = 1;
+
+		/*
+		 * Clang doesn't mark cold subfunctions as STT_FUNC, which
+		 * breaks several objtool assumptions.  Fake it.
+		 */
+		sym->type = STT_FUNC;
+	}
+
 	sym->pfunc = sym->cfunc = sym;
 
 	sym->demangled_name = demangle_name(sym);

Re: [PATCH v2 2/2] objtool: dead_end function change for split functions

[Rong Xu]

Josh,

You're right, your fix makes a lot of sense. I wasn't aware that the
split function was already handled for GCC.

Yes, clang sets the split part of the function as STT_NOTYPE, which is
different from GCC.

I assume you will post the new patches (together with other fixes for
clang's split function) soon. Any time line?

Also how about the other patch in this patch set. Does that make sense?

Thanks for the advice on getting rid of objtool warnings in our
builds! We tried but it's hard to get rid of all the warnings. But
I'll keep close eyes on these unreachable code warnings.

Thanks,

-Rong

On Mon, Nov 17, 2025 at 1:50 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Nov 17, 2025 at 01:43:26PM -0800, Josh Poimboeuf wrote:
> > On Mon, Nov 03, 2025 at 09:52:44PM +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.
> >
> > I assume there were also some ignored objtool warnings?  Ignoring those
> > can have catastrophic consequences (beyond just failing livepatches) so
> > I highly recommend building with CONFIG_OBJTOOL_WERROR.
> >
> > > 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>
> >
> > I was scratching my head about this one for a while.  .text.split.<func>
> > is for <func>.cold code, which should automatically get iterated by the
> > outer func_for_each_insn() construct in __dead_end_function().  So this
> > patch shouldn't be necessary.
> >
> > After some testing with Clang -fsplit-machine-functions, I'm realizing
> > the problem is that unlike GCC, Clang .cold code (in .text.split.*)
> > isn't STT_FUNC.  Which breaks several objtool assumptions.  So something
> > like the below is needed.
>
> That last diff had a blatant bug, here is the proper fix:
>
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index 3f20b257ab25..5a9efbda880b 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -502,8 +502,16 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym)
>         if (strstarts(sym->name, ".klp.sym"))
>                 sym->klp = 1;
>
> -       if (!sym->klp && is_func_sym(sym) && strstr(sym->name, ".cold"))
> +       if (!sym->klp && strstr(sym->name, ".cold")) {
>                 sym->cold = 1;
> +
> +               /*
> +                * Clang doesn't mark cold subfunctions as STT_FUNC, which
> +                * breaks several objtool assumptions.  Fake it.
> +                */
> +               sym->type = STT_FUNC;
> +       }
> +
>         sym->pfunc = sym->cfunc = sym;
>
>         sym->demangled_name = demangle_name(sym);

Re: [PATCH v2 2/2] objtool: dead_end function change for split functions

[Josh Poimboeuf]

On Mon, Nov 17, 2025 at 02:57:02PM -0800, Rong Xu wrote:
> Josh,
> 
> You're right, your fix makes a lot of sense. I wasn't aware that the
> split function was already handled for GCC.

FYI, GCC puts its cold functions in .text.unlikely[.*] rather than
.text.split.*.  And it does so automatically, without
-fsplit-machine-functions.

> Yes, clang sets the split part of the function as STT_NOTYPE, which is
> different from GCC.
> 
> I assume you will post the new patches (together with other fixes for
> clang's split function) soon. Any time line?
> 
> Also how about the other patch in this patch set. Does that make sense?

I think patch 1/2 looked ok, let me go stare at it again.

> Thanks for the advice on getting rid of objtool warnings in our
> builds! We tried but it's hard to get rid of all the warnings. But
> I'll keep close eyes on these unreachable code warnings.

Here's my work in progress:

  git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git objtool-split-fix

I hope to post the patches in a day or so if the bots don't complain.

If they don't fix all your warnings, let me know and I can help look.

-- 
Josh

Re: [PATCH v2 1/2] objtool: fix the check for dead_end function with multiple sibliing calls

[Josh Poimboeuf]

On Mon, Nov 03, 2025 at 09:52:43PM +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.
> 
> 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;
>  		}
>  	}
>  

There's a typo in the subject "sibliing", and "fix" should be
capitalized:

  objtool: Fix the check for dead_end function with multiple sibling calls

Otherwise it LGTM.

Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>

-- 
Josh