Skip to content
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.

Sign up for GitHub

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

Jump to bottom

-mfunction-return=thunk-extern does not affect sanitizer constructors #56514

Closed
nickdesaulniers opened this issue Jul 13, 2022 · 5 comments
Closed

-mfunction-return=thunk-extern does not affect sanitizer constructors #56514

nickdesaulniers opened this issue Jul 13, 2022 · 5 comments
Assignees
@nickdesaulniers
Labels
bug Indicates an unexpected problem or unintended behavior clang:codegen
Milestone
LLVM 15.0.0 Release

Comments

@nickdesaulniers
Copy link
Member

As reported by @nathanchance here, mixing -mfunction-return=thunk-extern with various sanitizers is not producing mitigated sanitizer constructor functions.

I know exactly what this is; the front-end attributes functions during clang's CodeGen of LLVM IR that they should be fn_ret_thunk_extern. Then LLVM synthesizes new functions without knowledge that new functions should have these attributes. We've seen this before, I think with stack protectors (nossp, ssp, etc).

I will fix this, but marking it a blocker for the clang-15 release, and to have something to point to so I can pause ongoing backporting issues related to retbleed for this.

cc @topperc @AaronBallman @kees
@nickdesaulniers nickdesaulniers added bug Indicates an unexpected problem or unintended behavior clang Clang issues not falling into any other category labels Jul 13, 2022
@nickdesaulniers nickdesaulniers added this to the LLVM 15.0.0 Release milestone Jul 13, 2022
@nickdesaulniers nickdesaulniers self-assigned this Jul 13, 2022
@llvmbot
Copy link
Member

llvmbot commented Jul 13, 2022

@llvm/issue-subscribers-bug

@nickdesaulniers
Copy link
Member Author

@nathanchance mentioned that this is a similar problem we've had with PAC/BTI: a88c722.

It's not just the sanitizers, CONFIG_GCOV_KERNEL=y + CONFIG_GCOV_PROFILE_ALL=y also has this issue:

...
drivers/base/platform.o: warning: objtool: __llvm_gcov_reset+0xa94: 'naked' return found in RETHUNK build
net/core/netpoll.o: warning: objtool: __llvm_gcov_reset+0xde3: 'naked' return found in RETHUNK build
net/netfilter/nf_conntrack_netlink.o: warning: objtool: __llvm_gcov_reset+0x1b94: 'naked' return found in RETHUNK build
fs/fs_parser.o: warning: objtool: __llvm_gcov_reset+0x344: 'naked' return found in RETHUNK build
drivers/acpi/debugfs.o: warning: objtool: __llvm_gcov_reset+0xb: 'naked' return found in RETHUNK build
mm/pagewalk.o: warning: objtool: __llvm_gcov_reset+0x77f: 'naked' return found in RETHUNK build
drivers/tty/serial/8250/8250_pnp.o: warning: objtool: __llvm_gcov_reset+0x231: 'naked' return found in RETHUNK build
fs/init.o: warning: objtool: __llvm_gcov_reset+0x2f7: 'naked' return found in RETHUNK build
drivers/gpu/drm/drm_fourcc.o: warning: objtool: __llvm_gcov_reset+0x1ce: 'naked' return found in RETHUNK build
...

so I should test gcov, too.

@nickdesaulniers
Copy link
Member Author

Something more like
commit 2786e67 ("[IR][sanitizer] Add module flag "frame-pointer" and set it for cc1 -mframe-pointer={non-leaf,all}")
and less like
commit b7926ce ("[IR] add fn attr for no_stack_protector; prevent inlining on mismatch")
is probably a good solution here.

@nickdesaulniers
Copy link
Member Author

https://reviews.llvm.org/D129709

@EugeneZelenko EugeneZelenko added clang:codegen and removed clang Clang issues not falling into any other category labels Jul 14, 2022
@llvmbot
Copy link
Member

llvmbot commented Jul 14, 2022

@llvm/issue-subscribers-clang-codegen

@tstellar tstellar moved this to Needs Fix in LLVM Release Status Aug 2, 2022
@nikic nikic moved this from Needs Fix to Done in LLVM Release Status Aug 2, 2022
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
@nickdesaulniers
[clang][CodeGen] add fn_ret_thunk_extern to synthetic fns
e87eef0 
Follow up fix to
commit 2240d72f15f3 ("[X86] initial -mfunction-return=thunk-extern
support")
https://reviews.llvm.org/D129572

@nathanchance reported that -mfunction-return=thunk-extern was failing
to annotate the asan and tsan contructors.
https://lore.kernel.org/llvm/Ys7pLq+tQk5xEa%2FB@dev-arch.thelio-3990X/

I then noticed the same occurring for gcov synthetic functions.

Similar to
commit 2786e67 ("[IR][sanitizer] Add module flag "frame-pointer" and set
it for cc1 -mframe-pointer={non-leaf,all}")
define a new module level MetaData, "fn_ret_thunk_extern", then when set
adds the fn_ret_thunk_extern IR Fn Attr to synthetically created
Functions.

Fixes llvm/llvm-project#56514

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D129709
unicornx pushed a commit to aosp-riscv/toolchain-llvm-android that referenced this issue Nov 28, 2022
@nickdesaulniers
Revert "Add patch for -mfunction-return=thunk-extern for retbleed"
5c925c0 
This reverts commit 58201cc.

Reason for revert: breaks testing branch and llvm/llvm-project#56514

Change-Id: I43f9249de3a89613311860bd53d83670db74ef7f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nickdesaulniers nickdesaulniers

Labels
bug Indicates an unexpected problem or unintended behavior clang:codegen
Projects
LLVM Release Status
Archived in project
Milestone
LLVM 15.0.0 Release
Development

No branches or pull requests
3 participants
@nickdesaulniers @EugeneZelenko @llvmbot