mbox series

[0/2] Fix objtool error reporting and handling of very long symbol names

Message ID cover.1696355111.git.aplattner@nvidia.com
Headers
Series Fix objtool error reporting and handling of very long symbol names |

Message

Aaron Plattner Oct. 5, 2023, 12:08 a.m. UTC 
  Two patches in this series:

First, when objtool encounters an error, it carefully propagates the return
code all the way up to main(), which proceeds to ignore it and return 0 no
matter what. This can cause problems with objtool to be missed because the
overall build succeeds. This appears to be a regression in commit
b51277eb9775c, which dropped a call to exit(ret) when a subcommand fails.

Fix that by returning the status code from main().


Second, very long symbol names with .cold variants cause objtool to fail.
This is due to using a small max length, which in turn is due to allocating
on the stack. However, there is not actually a requirement to allocate on
the stack in this (user space) code path, and in fact, the code is cleaner
with this fix: MAX_NAME_LEN is gone and the ugly manual NULL termination
is also removed.

The net result is a more capable objtool and slightly cleaner code.


Although this fix technically only applies to drivers that generate
unusually long symbol names, typically due to using C++ (and these cases
only appear to exist outside of the kernel tree so far), I think it's still
worth applying. That's because the net result is a more capable objtool:
one that lacks an arbitrary length limit for symbol names.

For example, Rust support is being added, and drivers will be the first
users of that support. And like C++, Rust also needs to mangle names [1].
So getting rid of the name length constraint is just good hygiene.

[1] https://rust-lang.github.io/rfcs/2603-rust-symbol-name-mangling-v0.html 


Aaron Plattner (2):
  objtool: return the result of objtool_run() so the build fails if
    objtool doesn't work
  objtool: use strndup() to avoid the need for a maximum symbol name
    length

 tools/objtool/elf.c     | 14 ++++++--------
 tools/objtool/objtool.c |  4 +---
 2 files changed, 7 insertions(+), 11 deletions(-)

Comments

Josh Poimboeuf Oct. 5, 2023, 11:58 p.m. UTC | #1 
On Wed, Oct 04, 2023 at 05:08:17PM -0700, Aaron Plattner wrote:
> Two patches in this series:
> 
> First, when objtool encounters an error, it carefully propagates the return
> code all the way up to main(), which proceeds to ignore it and return 0 no
> matter what. This can cause problems with objtool to be missed because the
> overall build succeeds. This appears to be a regression in commit
> b51277eb9775c, which dropped a call to exit(ret) when a subcommand fails.
> 
> Fix that by returning the status code from main().

Note most objtool errors are currently ignored anyway, due to check()
unconditionally returning 0.  But as the patch mentions, it will indeed
fix early errors.

> Second, very long symbol names with .cold variants cause objtool to fail.
> This is due to using a small max length, which in turn is due to allocating
> on the stack. However, there is not actually a requirement to allocate on
> the stack in this (user space) code path, and in fact, the code is cleaner
> with this fix: MAX_NAME_LEN is gone and the ugly manual NULL termination
> is also removed.

One benefit of the stack array is that reusing it is more performant
than doing a bunch of allocations.  But this is only for cold symbols,
so the slowdown should (hopefully) be negligible.

I'll run it through some testing.  Thanks!