Skip to content

Linux build: silence objtool warnings #17410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AttilaFueloep
Copy link
Contributor

Motivation and Context

Follow up for #17401 (3084336).

Description

After #17401 the Linux build produces some stack related warnings. Silence them with the STACK_FRAME_NON_STANDARD macro.

How Has This Been Tested?

Built and loaded the module, verified that __warn_thunk isn't called.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@AttilaFueloep
Copy link
Contributor Author

Looks like RHEL 8 is missing the STACK_FRAME_NON_STANDARD assembler macro but has the C #define. Need to check the RHEL 8 Linux sources to find a solution.

@john-cabaj
Copy link

john-cabaj commented Jun 2, 2025

I've been running into this warning, though as a build error, while trying to get a zfs build for 6.15. After bisecting the kernel, it appears to be failing the build because of this commit:

36799069b48198e5ce92d99310060c4aecb4b3e3 is the first bad commit
commit 36799069b48198e5ce92d99310060c4aecb4b3e3
Author: Josh Poimboeuf <[email protected]>
Date:   Fri Mar 14 12:29:11 2025 -0700

    objtool: Add CONFIG_OBJTOOL_WERROR
    
    Objtool warnings can be indicative of crashes, broken live patching, or
    even boot failures.  Ignoring them is not recommended.
    
    Add CONFIG_OBJTOOL_WERROR to upgrade objtool warnings to errors by
    enabling the objtool --Werror option.  Also set --backtrace to print the
    branches leading up to the warning, which can help considerably when
    debugging certain warnings.
    
    To avoid breaking bots too badly for now, make it the default for real
    world builds only (!COMPILE_TEST).
    
    Co-developed-by: Brendan Jackman <[email protected]>
    Signed-off-by: Josh Poimboeuf <[email protected]>
    Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
    Link: https://lore.kernel.org/r/3e7c109313ff15da6c80788965cc7450115b0196.1741975349.git.jpoimboe@kernel.org

 lib/Kconfig.debug    | 11 +++++++++++
 scripts/Makefile.lib |  1 +
 2 files changed, 12 insertions(+)

@AttilaFueloep AttilaFueloep mentioned this pull request Jun 2, 2025
14 tasks
@AttilaFueloep
Copy link
Contributor Author

Thanks for noticing. I'd assume that this PR would fix the build for you. Did you try? (The failing check is due to an ancient kernel.)

@john-cabaj
Copy link

I can try shortly. I'm disabling the option in the kernel first, then will come back and try this patch (while re-enabling the config).

@behlendorf
Copy link
Contributor

@AttilaFueloep for RHEL 8 adding a check to config/kernel-objtool.m4 which attempts to use STACK_FRAME_NON_STANDARD seems like the best thing to do. If the check fails we can define STACK_FRAME_NON_STANDARD and the assembler macro ourselves.

As you'd expect, I did verify the quick fix of adding only the missing assembler macro does work but that's going to be fragile long term.

@AttilaFueloep
Copy link
Contributor Author

@behlendorf Well it's complicated. Newer kernels have an assembler macro for marking the stack non standard. for certain functions whereas the RHEL 8 kernel has not. To check for the macro inside config/kernel-objtool.m4 we'd need
tooling to assemble an .S file, which is currently missing. I'm currently trying to wrap my head around how to add this. The major problem is that you can't build a loadable kernel module out of assembly due to the MODULE_*() macros from module.h but all the tooling expects to build a module, so my added tooling fails in the modpost stage.

@john-cabaj
Copy link

@AttilaFueloep - this patch + CONFIG_OBJTOOL_WERROR=y works, as does CONFIG_OBJTOOL_WERROR=n

@AttilaFueloep
Copy link
Contributor Author

@behlendorf

As you'd expect, I did verify the quick fix of adding only the missing assembler macro does work but that's going to be fragile long term.

Exactly, that's why my current plan is to check for the macro inside config/kernel-objtool.m4 and add an .section .discard... to the assembly file if it's missing, this would also also avoid copy pasting the GPL'ed macro.

@AttilaFueloep
Copy link
Contributor Author

@john-cabaj Nice, thanks for confirming.

@behlendorf
Copy link
Contributor

we'd need tooling to assemble an .S file, which is currently missing

Verifying we can build the kmod would absolutely be best, so don't let me deter you from getting that working. But adding a primitive check for the oddball case where the #define is there by the .macro is missing is perhaps good enough for this corner case. Something like this, d71dfa0, for the autotools bit.

@AttilaFueloep
Copy link
Contributor Author

Yeah, nice. Looking at all the MODULE_* #defines I guess it would be quite challenging to make a module out of assembly. Given this blocks 2.2.8, I'll go with the grep approach, at least for now. (I'll file the changes I've made so far for a later retry.) I'll try to come up with something in a couple of hours. (Currently busy with other things.)

@AttilaFueloep
Copy link
Contributor Author

If this passes the CI I'll squash and rebase.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jun 2, 2025
After openzfs#17401 the Linux build produces some stack related warnings.

Silence them with the `STACK_FRAME_NON_STANDARD` macro.

Signed-off-by: Attila Fülöp <[email protected]>
Co-authored-by: Brian Behlendorf <[email protected]>
@AttilaFueloep AttilaFueloep force-pushed the zfs-silence-objtool branch from 82f54d0 to fa695c2 Compare June 3, 2025 12:18
Signed-off-by: Attila Fülöp <[email protected]>
@AttilaFueloep
Copy link
Contributor Author

Squashed, rebased and added a refinement to the grep regexp I forgot to commit.

@AttilaFueloep
Copy link
Contributor Author

Searching the build logs reveals that there still are some objtool warnings left. On newer kernels (Alma10 and Fedora builders, locally on Arch) objtool complains about {spl_panic(),luaD_throw()} is missing a __noreturn annotation. I'll try to address them in a follow up commit.

The Ubuntu22 builder has this strange warning which isn't seen on the other builders:

/home/zfs/zfs/module/zstd/lib/decompress/zstd_decompress_block.o: warning: objtool: ZSTD_decompressSequences_bmi2.constprop.0()+0x90c: call without frame pointer save/setup

Not sure what to make out of it.

@AttilaFueloep
Copy link
Contributor Author

@john-cabaj It seems you are not seeing the remaining objtool warnings on your builds. Is something special with your kernel builds or does it fix itself on 6.15? Which distro are you using?

@AttilaFueloep
Copy link
Contributor Author

The spl_panic () is tricky. ELF doesn't support the noreturn attribute so objtool needs to maintain a global list of extern noreturn functions. spl_panic(), if called from other compilation units and declared as noreturn, triggers falls through to next function warnings since the compiler doesn't add a return at the end and objtool doesn't know about it. So either way we end up with warnings.

The proper fix would be to add spl_panic() to the above mentioned list, which, for known reasons, won't happen. Not sure how to solve this, STACK_FRAME_NON_STANDARD doesn't help here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants