Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Thu, 20 Mar 2025 16:54:58 -0500
Sahil Gupta <s.gupta@arista.com> wrote:

> Hi Steven,
> 
> On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts because
> of the parsing of the start_mcount_loc and stop_mcount_loc values in
> get_mcount_loc():
> 
>   *_start = strtoul(start_buff, NULL, 16);
> 
> and
> 
>  *_stop = strtoul(stop_buff, NULL, 16);
> 
> This code makes the (often correct) assumption that the host and the target
> have the same architecture, however it runs into issues when compiling for
> a 64-bit target on a 32-bit host, as unsigned long is shorter than the
> pointer width. As a result, I've noticed that both start and stop max out
> at 2^32 - 1.

So this has been broken for some time?

> 
> It seems that commit 4acda8ed fixes this issue inadvertently by directly
> extracting them from the ELF using the correct width. I'm wondering if it
> is possible to backport this as well as the other sorttable refactors to
> 6.12.0 since they fix this issue.

You should ask Greg KH on this.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

> > Hi Steven,
> >
> > On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts because
> > of the parsing of the start_mcount_loc and stop_mcount_loc values in
> > get_mcount_loc():
> >
> >   *_start = strtoul(start_buff, NULL, 16);
> >
> > and
> >
> >  *_stop = strtoul(stop_buff, NULL, 16);
> >
> > This code makes the (often correct) assumption that the host and the target
> > have the same architecture, however it runs into issues when compiling for
> > a 64-bit target on a 32-bit host, as unsigned long is shorter than the
> > pointer width. As a result, I've noticed that both start and stop max out
> > at 2^32 - 1.
>
> So this has been broken for some time?

Since 2021, it seems like. However, this slipped from my radar since
we've been using 5.10 for the longest time, which didn't have
CONFIG_BUILDTIME_MCOUNT_SORT.

> >
> > It seems that commit 4acda8ed fixes this issue inadvertently by directly
> > extracting them from the ELF using the correct width. I'm wondering if it
> > is possible to backport this as well as the other sorttable refactors to
> > 6.12.0 since they fix this issue.
>
> You should ask Greg KH on this.

Sounds good, tagging Greg.

Sahil


On Mon, Mar 24, 2025 at 12:35 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 20 Mar 2025 16:54:58 -0500
> Sahil Gupta <s.gupta@arista.com> wrote:
>
> > Hi Steven,
> >
> > On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts because
> > of the parsing of the start_mcount_loc and stop_mcount_loc values in
> > get_mcount_loc():
> >
> >   *_start = strtoul(start_buff, NULL, 16);
> >
> > and
> >
> >  *_stop = strtoul(stop_buff, NULL, 16);
> >
> > This code makes the (often correct) assumption that the host and the target
> > have the same architecture, however it runs into issues when compiling for
> > a 64-bit target on a 32-bit host, as unsigned long is shorter than the
> > pointer width. As a result, I've noticed that both start and stop max out
> > at 2^32 - 1.
>
> So this has been broken for some time?
>
> >
> > It seems that commit 4acda8ed fixes this issue inadvertently by directly
> > extracting them from the ELF using the correct width. I'm wondering if it
> > is possible to backport this as well as the other sorttable refactors to
> > 6.12.0 since they fix this issue.
>
> You should ask Greg KH on this.
>
> -- Steve
>

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Greg KH]

On Mon, Mar 24, 2025 at 01:39:12PM -0500, Sahil Gupta wrote:
> > > Hi Steven,
> > >
> > > On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts because
> > > of the parsing of the start_mcount_loc and stop_mcount_loc values in
> > > get_mcount_loc():
> > >
> > >   *_start = strtoul(start_buff, NULL, 16);
> > >
> > > and
> > >
> > >  *_stop = strtoul(stop_buff, NULL, 16);
> > >
> > > This code makes the (often correct) assumption that the host and the target
> > > have the same architecture, however it runs into issues when compiling for
> > > a 64-bit target on a 32-bit host, as unsigned long is shorter than the
> > > pointer width. As a result, I've noticed that both start and stop max out
> > > at 2^32 - 1.
> >
> > So this has been broken for some time?
> 
> Since 2021, it seems like. However, this slipped from my radar since
> we've been using 5.10 for the longest time, which didn't have
> CONFIG_BUILDTIME_MCOUNT_SORT.
> 
> > >
> > > It seems that commit 4acda8ed fixes this issue inadvertently by directly
> > > extracting them from the ELF using the correct width. I'm wondering if it
> > > is possible to backport this as well as the other sorttable refactors to
> > > 6.12.0 since they fix this issue.
> >
> > You should ask Greg KH on this.
> 
> Sounds good, tagging Greg.

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Mon, 24 Mar 2025 11:52:33 -0700
Greg KH <gregkh@linuxfoundation.org> wrote:

> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

Hi Greg,

This isn't a patch submission. It's more of a question if something should
be backported because it happens to fix a long standing issue.

It appears that the ftrace mcount sorttable code was broken if you built a
64 bit kernel on a 32 bit machine. I'm guessing it was broken since 5.17?
A recent update to this code coincidentally fixes that issue. That update
landed in 6.14. The question is, is it fine to backport the changes that
fix this? It may not be totally trivial to do so, but I could likely just
backport the changes that address this issue.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Greg KH]

On Mon, Mar 24, 2025 at 03:07:03PM -0400, Steven Rostedt wrote:
> On Mon, 24 Mar 2025 11:52:33 -0700
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > <formletter>
> > 
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read:
> >     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to do this properly.
> > 
> > </formletter>
> 
> Hi Greg,
> 
> This isn't a patch submission. It's more of a question if something should
> be backported because it happens to fix a long standing issue.

Sorry, that wasn't obvious to me.

> It appears that the ftrace mcount sorttable code was broken if you built a
> 64 bit kernel on a 32 bit machine. I'm guessing it was broken since 5.17?
> A recent update to this code coincidentally fixes that issue. That update
> landed in 6.14. The question is, is it fine to backport the changes that
> fix this? It may not be totally trivial to do so, but I could likely just
> backport the changes that address this issue.

It's up to the maintainer of the subsytem as to what they wish to see
happen.  I will always defer to them as they are the ones that have to
deal with emails from users :)

thanks,

greg k-h

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Tue, 25 Mar 2025 07:58:36 -0400
Greg KH <gregkh@linuxfoundation.org> wrote:

> It's up to the maintainer of the subsytem as to what they wish to see
> happen.  I will always defer to them as they are the ones that have to
> deal with emails from users :)

Thanks Greg for the reply.

Now bringing the question to Sahil.

This looks to be broken in 5.17 (when the mcount sorting was added). And
you are now using 6.12. How important is it for you to build 64 bit kernels
on 32 bit machines?

Nobody noticed this for a long time. One solution may be to simply disable
mcount sorting at build time when it is detected that the host is 32 bit
and the target is 64 bit. I don't know how easy it is to detect that at
build time, as it needs to unset a kernel CONFIG option. Perhaps Kconfig
can detect that?

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

On Tue, 25 Mar 2025 05:15:10 -0700 (PDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> How important is it for you to build 64 bit kernels
> on 32 bit machines?

Yeah I don't disagree at all, it is a fairly idiosyncratic thing to
do. There are some historical and business justifications for still
building 32-bit in the first place, but until we deduplicate the
kernel build, we will continue to have this issue.

> One solution may be to simply disable
> mcount sorting at build time when it is detected that the host is 32 bit
> and the target is 64 bit.

We are currently doing this. I imagine the performance difference is
trivial but if we can sort at build time, we might as well. I have a
fairly simple alternate solution that isn't backporting all of those
patches. The core idea is to introduce another macro, parse_addr, that
is defined as

 # define parse_addr(buf) strtoull(buf, NULL, 16)

when SORTTABLE_64 is defined and

 # define parse_addr(buf) strtoul(buf, NULL, 16)

when it isn't. Seems like a fair thing to do considering unsigned long
long is guaranteed to be at least 64-bit and unsigned long is
guaranteed to be at least 32-bit. I can post the patch in a stable-
thread.

Sahil

On Tue, Mar 25, 2025 at 7:15 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 25 Mar 2025 07:58:36 -0400
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > It's up to the maintainer of the subsytem as to what they wish to see
> > happen.  I will always defer to them as they are the ones that have to
> > deal with emails from users :)
>
> Thanks Greg for the reply.
>
> Now bringing the question to Sahil.
>
> This looks to be broken in 5.17 (when the mcount sorting was added). And
> you are now using 6.12. How important is it for you to build 64 bit kernels
> on 32 bit machines?
>
> Nobody noticed this for a long time. One solution may be to simply disable
> mcount sorting at build time when it is detected that the host is 32 bit
> and the target is 64 bit. I don't know how easy it is to detect that at
> build time, as it needs to unset a kernel CONFIG option. Perhaps Kconfig
> can detect that?
>
> -- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Tue, 25 Mar 2025 12:52:43 -0500
Sahil Gupta <s.gupta@arista.com> wrote:

> On Tue, 25 Mar 2025 05:15:10 -0700 (PDT)
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > How important is it for you to build 64 bit kernels
> > on 32 bit machines?  
> 
> Yeah I don't disagree at all, it is a fairly idiosyncratic thing to
> do. There are some historical and business justifications for still
> building 32-bit in the first place, but until we deduplicate the
> kernel build, we will continue to have this issue.

Note, that the regression is only the failure in the build of your setup.
As your setup never supported sorting the mcount location at build time,
that part is not the regression.

> 
> > One solution may be to simply disable
> > mcount sorting at build time when it is detected that the host is 32 bit
> > and the target is 64 bit.  
> 
> We are currently doing this. I imagine the performance difference is
> trivial but if we can sort at build time, we might as well. I have a
> fairly simple alternate solution that isn't backporting all of those
> patches. The core idea is to introduce another macro, parse_addr, that
> is defined as
> 
>  # define parse_addr(buf) strtoull(buf, NULL, 16)
> 
> when SORTTABLE_64 is defined and
> 
>  # define parse_addr(buf) strtoul(buf, NULL, 16)
> 
> when it isn't. Seems like a fair thing to do considering unsigned long
> long is guaranteed to be at least 64-bit and unsigned long is
> guaranteed to be at least 32-bit. I can post the patch in a stable-
> thread.

I'm not against you doing that, but it is specific for your setup.

If you do this, make sure to test it by enabling:

  CONFIG_FTRACE_SORT_STARTUP_TEST

Which will verify that the table is indeed sorted at run time. There could
be another bug that could make it build, but not sort it properly, and
without that config, you may not know, but it will cause ftrace to not work
properly.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

On Tue, 25 Mar 2025 11:01:41 -0700 (PDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> I'm not against you doing that, but it is specific for your setup.
>
> If you do this, make sure to test it by enabling:
>
>   CONFIG_FTRACE_SORT_STARTUP_TEST
>
> Which will verify that the table is indeed sorted at run time. There could
> be another bug that could make it build, but not sort it properly, and
> without that config, you may not know, but it will cause ftrace to not work
> properly.

That sounds good to me. I didn't use CONFIG_FTRACE_SORT_STARTUP_TEST,
but I manually verified the section was sorted by checking the ELF
using a Python script. Maybe that is something we can consider adding
as a build step in the future?

Sahil


On Tue, Mar 25, 2025 at 1:01 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 25 Mar 2025 12:52:43 -0500
> Sahil Gupta <s.gupta@arista.com> wrote:
>
> > On Tue, 25 Mar 2025 05:15:10 -0700 (PDT)
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > > How important is it for you to build 64 bit kernels
> > > on 32 bit machines?
> >
> > Yeah I don't disagree at all, it is a fairly idiosyncratic thing to
> > do. There are some historical and business justifications for still
> > building 32-bit in the first place, but until we deduplicate the
> > kernel build, we will continue to have this issue.
>
> Note, that the regression is only the failure in the build of your setup.
> As your setup never supported sorting the mcount location at build time,
> that part is not the regression.
>
> >
> > > One solution may be to simply disable
> > > mcount sorting at build time when it is detected that the host is 32 bit
> > > and the target is 64 bit.
> >
> > We are currently doing this. I imagine the performance difference is
> > trivial but if we can sort at build time, we might as well. I have a
> > fairly simple alternate solution that isn't backporting all of those
> > patches. The core idea is to introduce another macro, parse_addr, that
> > is defined as
> >
> >  # define parse_addr(buf) strtoull(buf, NULL, 16)
> >
> > when SORTTABLE_64 is defined and
> >
> >  # define parse_addr(buf) strtoul(buf, NULL, 16)
> >
> > when it isn't. Seems like a fair thing to do considering unsigned long
> > long is guaranteed to be at least 64-bit and unsigned long is
> > guaranteed to be at least 32-bit. I can post the patch in a stable-
> > thread.
>
> I'm not against you doing that, but it is specific for your setup.
>
> If you do this, make sure to test it by enabling:
>
>   CONFIG_FTRACE_SORT_STARTUP_TEST
>
> Which will verify that the table is indeed sorted at run time. There could
> be another bug that could make it build, but not sort it properly, and
> without that config, you may not know, but it will cause ftrace to not work
> properly.
>
> -- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Steven Rostedt]

On Tue, 25 Mar 2025 13:10:17 -0500
Sahil Gupta <s.gupta@arista.com> wrote:

> That sounds good to me. I didn't use CONFIG_FTRACE_SORT_STARTUP_TEST,
> but I manually verified the section was sorted by checking the ELF
> using a Python script. Maybe that is something we can consider adding
> as a build step in the future?

Note, that config only enables the runtime verification. Which means you
need to run the kernel for the check if it worked. It is not a build time
feature. And since it checks at run time, it does add a small overhead (to
iterate all functions), where you may not want it enabled in production.

-- Steve

Re: ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

On Tue, 25 Mar 2025 11:18:52 -0700 (PDT)
Steven Rostedt <rostedt@goodmis.org> wrote:

> Note, that config only enables the runtime verification. Which means you
> need to run the kernel for the check if it worked. It is not a build time
> feature. And since it checks at run time, it does add a small overhead (to
> iterate all functions), where you may not want it enabled in production.

Right, sounds good. I'll test it with CONFIG_FTRACE_SORT_STARTUP_TEST
and send the patch via a stable- thread if it succeeds.

Sahil


On Tue, Mar 25, 2025 at 1:18 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 25 Mar 2025 13:10:17 -0500
> Sahil Gupta <s.gupta@arista.com> wrote:
>
> > That sounds good to me. I didn't use CONFIG_FTRACE_SORT_STARTUP_TEST,
> > but I manually verified the section was sorted by checking the ELF
> > using a Python script. Maybe that is something we can consider adding
> > as a build step in the future?
>
> Note, that config only enables the runtime verification. Which means you
> need to run the kernel for the check if it worked. It is not a build time
> feature. And since it checks at run time, it does add a small overhead (to
> iterate all functions), where you may not want it enabled in production.
>
> -- Steve

[PATCH 6.1 6.6 6.12 6.13] scripts/sorttable: fix ELF64 mcount_loc address parsing when compiling on 32-bit

[Sahil Gupta]

The ftrace __mcount_loc buildtime sort does not work properly when the host is
32-bit and the target is 64-bit. sorttable parses the start and stop addresses
by calling strtoul on the buffer holding the hexadecimal string. Since the
target is 64-bit but unsigned long on 32-bit machines is 32 bits, strtoul,
and by extension the start and stop addresses, can max out to 2^32 - 1.

This patch adds a new macro, parse_addr, that corresponds to a strtoul
or strtoull call based on whether you are operating on a 32-bit ELF or
a 64-bit ELF. This way, the correct width is guaranteed whether or not
the host is 32-bit. This should cleanly apply on all of the 6.x stable
kernels.

Manually verified that the __mcount_loc section is sorted by parsing the
ELF and verified tests corresponding to CONFIG_FTRACE_SORT_STARTUP_TEST
for kernels built on a 32-bit and a 64-bit host.

Signed-off-by: Sahil Gupta <s.gupta@arista.com>
---
 scripts/sorttable.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index 7bd0184380d3..9ed7acca9f30 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -40,6 +40,7 @@
 #undef uint_t
 #undef _r
 #undef _w
+#undef parse_addr
 
 #ifdef SORTTABLE_64
 # define extable_ent_size	16
@@ -65,6 +66,7 @@
 # define uint_t			uint64_t
 # define _r			r8
 # define _w			w8
+# define parse_addr(buf)	strtoull(buf, NULL, 16)
 #else
 # define extable_ent_size	8
 # define compare_extable	compare_extable_32
@@ -89,6 +91,7 @@
 # define uint_t			uint32_t
 # define _r			r
 # define _w			w
+# define parse_addr(buf)	strtoul(buf, NULL, 16)
 #endif
 
 #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
@@ -246,13 +249,13 @@ static void get_mcount_loc(uint_t *_start, uint_t *_stop)
 		len = strlen(start_buff);
 		start_buff[len - 1] = '\0';
 	}
-	*_start = strtoul(start_buff, NULL, 16);
+	*_start = parse_addr(start_buff);
 
 	while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
 		len = strlen(stop_buff);
 		stop_buff[len - 1] = '\0';
 	}
-	*_stop = strtoul(stop_buff, NULL, 16);
+	*_stop = parse_addr(stop_buff);
 
 	pclose(file_start);
 	pclose(file_stop);
-- 
2.47.0

Re: [PATCH 6.1 6.6 6.12 6.13] scripts/sorttable: fix ELF64 mcount_loc address parsing when compiling on 32-bit

[Greg KH]

On Tue, Mar 25, 2025 at 05:06:56PM -0700, Sahil Gupta wrote:
> The ftrace __mcount_loc buildtime sort does not work properly when the host is
> 32-bit and the target is 64-bit. sorttable parses the start and stop addresses
> by calling strtoul on the buffer holding the hexadecimal string. Since the
> target is 64-bit but unsigned long on 32-bit machines is 32 bits, strtoul,
> and by extension the start and stop addresses, can max out to 2^32 - 1.
> 
> This patch adds a new macro, parse_addr, that corresponds to a strtoul
> or strtoull call based on whether you are operating on a 32-bit ELF or
> a 64-bit ELF. This way, the correct width is guaranteed whether or not
> the host is 32-bit. This should cleanly apply on all of the 6.x stable
> kernels.
> 
> Manually verified that the __mcount_loc section is sorted by parsing the
> ELF and verified tests corresponding to CONFIG_FTRACE_SORT_STARTUP_TEST
> for kernels built on a 32-bit and a 64-bit host.
> 
> Signed-off-by: Sahil Gupta <s.gupta@arista.com>
> ---
>  scripts/sorttable.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

What is the upstream git commit of this?

If it's not upstream, then you need to document the heck out of why we
can't take whatever is upstream already, which I don't see here :(

thanks,

greg k-h

Re: [PATCH 6.1 6.6 6.12 6.13] scripts/sorttable: fix ELF64 mcount_loc address parsing when compiling on 32-bit

[Steven Rostedt]

On Tue, 25 Mar 2025 20:23:31 -0400
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Tue, Mar 25, 2025 at 05:06:56PM -0700, Sahil Gupta wrote:
> > The ftrace __mcount_loc buildtime sort does not work properly when the host is
> > 32-bit and the target is 64-bit. sorttable parses the start and stop addresses
> > by calling strtoul on the buffer holding the hexadecimal string. Since the
> > target is 64-bit but unsigned long on 32-bit machines is 32 bits, strtoul,
> > and by extension the start and stop addresses, can max out to 2^32 - 1.
> > 
> > This patch adds a new macro, parse_addr, that corresponds to a strtoul
> > or strtoull call based on whether you are operating on a 32-bit ELF or
> > a 64-bit ELF. This way, the correct width is guaranteed whether or not
> > the host is 32-bit. This should cleanly apply on all of the 6.x stable
> > kernels.
> > 
> > Manually verified that the __mcount_loc section is sorted by parsing the
> > ELF and verified tests corresponding to CONFIG_FTRACE_SORT_STARTUP_TEST
> > for kernels built on a 32-bit and a 64-bit host.
> > 
> > Signed-off-by: Sahil Gupta <s.gupta@arista.com>
> > ---
> >  scripts/sorttable.h | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)  
> 
> What is the upstream git commit of this?
> 
> If it's not upstream, then you need to document the heck out of why we
> can't take whatever is upstream already, which I don't see here :(

I guess it is loosely based on 4acda8edefa1 ("scripts/sorttable: Get
start/stop_mcount_loc from ELF file directly"), which may take a bit of
work to backport (or we just add everything that this commit depends on).

-- Steve

Re: [PATCH 6.1 6.6 6.12 6.13] scripts/sorttable: fix ELF64 mcount_loc address parsing when compiling on 32-bit

[Steven Rostedt]

On Tue, 25 Mar 2025 20:32:36 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> I guess it is loosely based on 4acda8edefa1 ("scripts/sorttable: Get
> start/stop_mcount_loc from ELF file directly"), which may take a bit of
> work to backport (or we just add everything that this commit depends on).

And looking at what was done, it was my rewrite of the sorttable.c code.

If it's OK to backport a rewrite, then we could just do that.

See commits:

  4f48a28b37d5 scripts/sorttable: Remove unused write functions
  7ffc0d0819f4 scripts/sorttable: Make compare_extable() into two functions
  157fb5b3cfd2 scripts/sorttable: Convert Elf_Ehdr to union
  545f6cf8f4c9 scripts/sorttable: Replace Elf_Shdr Macro with a union
  200d015e73b4 scripts/sorttable: Convert Elf_Sym MACRO over to a union
  1dfb59a228dd scripts/sorttable: Add helper functions for Elf_Ehdr
  67afb7f50440 scripts/sorttable: Add helper functions for Elf_Shdr
  17bed33ac12f scripts/sorttable: Add helper functions for Elf_Sym
  58d87678a0f4 scripts/sorttable: Move code from sorttable.h into sorttable.c

-- Steev

Re: [PATCH 6.1 6.6 6.12 6.13] scripts/sorttable: fix ELF64 mcount_loc address parsing when compiling on 32-bit

[Greg KH]

On Tue, Mar 25, 2025 at 08:37:23PM -0400, Steven Rostedt wrote:
> On Tue, 25 Mar 2025 20:32:36 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > I guess it is loosely based on 4acda8edefa1 ("scripts/sorttable: Get
> > start/stop_mcount_loc from ELF file directly"), which may take a bit of
> > work to backport (or we just add everything that this commit depends on).
> 
> And looking at what was done, it was my rewrite of the sorttable.c code.
> 
> If it's OK to backport a rewrite, then we could just do that.
> 
> See commits:
> 
>   4f48a28b37d5 scripts/sorttable: Remove unused write functions
>   7ffc0d0819f4 scripts/sorttable: Make compare_extable() into two functions
>   157fb5b3cfd2 scripts/sorttable: Convert Elf_Ehdr to union
>   545f6cf8f4c9 scripts/sorttable: Replace Elf_Shdr Macro with a union
>   200d015e73b4 scripts/sorttable: Convert Elf_Sym MACRO over to a union
>   1dfb59a228dd scripts/sorttable: Add helper functions for Elf_Ehdr
>   67afb7f50440 scripts/sorttable: Add helper functions for Elf_Shdr
>   17bed33ac12f scripts/sorttable: Add helper functions for Elf_Sym
>   58d87678a0f4 scripts/sorttable: Move code from sorttable.h into sorttable.c

Backport away!

Re: [PATCH 6.1 6.6 6.12 6.13] scripts/sorttable: fix ELF64 mcount_loc address parsing when compiling on 32-bit

[Steven Rostedt]

On Tue, 25 Mar 2025 20:45:00 -0400
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Tue, Mar 25, 2025 at 08:37:23PM -0400, Steven Rostedt wrote:
> > On Tue, 25 Mar 2025 20:32:36 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >   
> > > I guess it is loosely based on 4acda8edefa1 ("scripts/sorttable: Get
> > > start/stop_mcount_loc from ELF file directly"), which may take a bit of
> > > work to backport (or we just add everything that this commit depends on).  
> > 
> > And looking at what was done, it was my rewrite of the sorttable.c code.
> > 
> > If it's OK to backport a rewrite, then we could just do that.
> > 
> > See commits:
> > 
> >   4f48a28b37d5 scripts/sorttable: Remove unused write functions
> >   7ffc0d0819f4 scripts/sorttable: Make compare_extable() into two functions
> >   157fb5b3cfd2 scripts/sorttable: Convert Elf_Ehdr to union
> >   545f6cf8f4c9 scripts/sorttable: Replace Elf_Shdr Macro with a union
> >   200d015e73b4 scripts/sorttable: Convert Elf_Sym MACRO over to a union
> >   1dfb59a228dd scripts/sorttable: Add helper functions for Elf_Ehdr
> >   67afb7f50440 scripts/sorttable: Add helper functions for Elf_Shdr
> >   17bed33ac12f scripts/sorttable: Add helper functions for Elf_Sym
> >   58d87678a0f4 scripts/sorttable: Move code from sorttable.h into sorttable.c  
> 
> Backport away!

Actually, I only did a git log on scripts/sorttable.c. I left out
sorttable.h which gives me this list:

$ git log --pretty=oneline --abbrev-commit --reverse v6.12..4acda8edefa1ce66d3de845f1c12745721cd14c3 scripts/sorttable.[ch] 

0210d251162f scripts/sorttable: fix orc_sort_cmp() to maintain symmetry and transitivity
28b24394c6e9 scripts/sorttable: Remove unused macro defines
4f48a28b37d5 scripts/sorttable: Remove unused write functions
6f2c2f93a190 scripts/sorttable: Remove unneeded Elf_Rel
66990c003306 scripts/sorttable: Have the ORC code use the _r() functions to read
7ffc0d0819f4 scripts/sorttable: Make compare_extable() into two functions
157fb5b3cfd2 scripts/sorttable: Convert Elf_Ehdr to union
545f6cf8f4c9 scripts/sorttable: Replace Elf_Shdr Macro with a union
200d015e73b4 scripts/sorttable: Convert Elf_Sym MACRO over to a union
1dfb59a228dd scripts/sorttable: Add helper functions for Elf_Ehdr
67afb7f50440 scripts/sorttable: Add helper functions for Elf_Shdr
17bed33ac12f scripts/sorttable: Add helper functions for Elf_Sym
1b649e6ab8dc scripts/sorttable: Use uint64_t for mcount sorting
58d87678a0f4 scripts/sorttable: Move code from sorttable.h into sorttable.c
4acda8edefa1 scripts/sorttable: Get start/stop_mcount_loc from ELF file directly

-- Steve

Re: [PATCH 6.1 6.6 6.12 6.13] scripts/sorttable: fix ELF64 mcount_loc address parsing when compiling on 32-bit

[Sahil Gupta]

Works for me. At least from our previous discussion, it seemed like
backporting would be laborious, which is why I offered this
alternative patch. But if we can just backport the series, then
there's no reason to not do so.

Thanks,
Sahil

On Tue, Mar 25, 2025 at 7:47 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Tue, 25 Mar 2025 20:45:00 -0400
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > On Tue, Mar 25, 2025 at 08:37:23PM -0400, Steven Rostedt wrote:
> > > On Tue, 25 Mar 2025 20:32:36 -0400
> > > Steven Rostedt <rostedt@goodmis.org> wrote:
> > >
> > > > I guess it is loosely based on 4acda8edefa1 ("scripts/sorttable: Get
> > > > start/stop_mcount_loc from ELF file directly"), which may take a bit of
> > > > work to backport (or we just add everything that this commit depends on).
> > >
> > > And looking at what was done, it was my rewrite of the sorttable.c code.
> > >
> > > If it's OK to backport a rewrite, then we could just do that.
> > >
> > > See commits:
> > >
> > >   4f48a28b37d5 scripts/sorttable: Remove unused write functions
> > >   7ffc0d0819f4 scripts/sorttable: Make compare_extable() into two functions
> > >   157fb5b3cfd2 scripts/sorttable: Convert Elf_Ehdr to union
> > >   545f6cf8f4c9 scripts/sorttable: Replace Elf_Shdr Macro with a union
> > >   200d015e73b4 scripts/sorttable: Convert Elf_Sym MACRO over to a union
> > >   1dfb59a228dd scripts/sorttable: Add helper functions for Elf_Ehdr
> > >   67afb7f50440 scripts/sorttable: Add helper functions for Elf_Shdr
> > >   17bed33ac12f scripts/sorttable: Add helper functions for Elf_Sym
> > >   58d87678a0f4 scripts/sorttable: Move code from sorttable.h into sorttable.c
> >
> > Backport away!
>
> Actually, I only did a git log on scripts/sorttable.c. I left out
> sorttable.h which gives me this list:
>
> $ git log --pretty=oneline --abbrev-commit --reverse v6.12..4acda8edefa1ce66d3de845f1c12745721cd14c3 scripts/sorttable.[ch]
>
> 0210d251162f scripts/sorttable: fix orc_sort_cmp() to maintain symmetry and transitivity
> 28b24394c6e9 scripts/sorttable: Remove unused macro defines
> 4f48a28b37d5 scripts/sorttable: Remove unused write functions
> 6f2c2f93a190 scripts/sorttable: Remove unneeded Elf_Rel
> 66990c003306 scripts/sorttable: Have the ORC code use the _r() functions to read
> 7ffc0d0819f4 scripts/sorttable: Make compare_extable() into two functions
> 157fb5b3cfd2 scripts/sorttable: Convert Elf_Ehdr to union
> 545f6cf8f4c9 scripts/sorttable: Replace Elf_Shdr Macro with a union
> 200d015e73b4 scripts/sorttable: Convert Elf_Sym MACRO over to a union
> 1dfb59a228dd scripts/sorttable: Add helper functions for Elf_Ehdr
> 67afb7f50440 scripts/sorttable: Add helper functions for Elf_Shdr
> 17bed33ac12f scripts/sorttable: Add helper functions for Elf_Sym
> 1b649e6ab8dc scripts/sorttable: Use uint64_t for mcount sorting
> 58d87678a0f4 scripts/sorttable: Move code from sorttable.h into sorttable.c
> 4acda8edefa1 scripts/sorttable: Get start/stop_mcount_loc from ELF file directly
>
> -- Steve

Re: [PATCH 6.1 6.6 6.12 6.13] scripts/sorttable: fix ELF64 mcount_loc address parsing when compiling on 32-bit

[Sahil Gupta]

Just wondering if this backport is still going to happen. I know this
is fairly bottom of the barrel so I wouldn't be surprised if it
doesn't.

Sahil

ftrace: sorttable unable to sort ELF64 on 32-bit host

[Sahil Gupta]

Hi Steven,

On 6.12.0, sorttable is unable to sort 64-bit ELFs on 32-bit hosts
because of the parsing of the start_mcount_loc and stop_mcount_loc
values in get_mcount_loc():

  *_start = strtoul(start_buff, NULL, 16);

and

 *_stop = strtoul(stop_buff, NULL, 16);

This code makes the (often correct) assumption that the host and the
target have the same architecture, however it runs into issues when
compiling for a 64-bit target on a 32-bit host, as unsigned long is
shorter than the pointer width. As a result, I've noticed that both
start and stop max out at 2^32 - 1.

It seems that commit 4acda8ed fixes this issue inadvertently by
directly extracting them from the ELF using the correct width. I'm
wondering if it is possible to backport this as well as the other
sorttable refactors to 6.12.0 since they fix this issue.

Thanks,
Sahil