-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
add test for symbol visibility of #[naked]
functions
#128362
add test for symbol visibility of #[naked]
functions
#128362
Conversation
This PR modifies cc @jieyouxu |
Do we already have a test for global? If not, it might be worth adding one. |
global what? I don't understand what you're suggesting here. |
I was suggesting that you might as well add a check for |
8040197
to
940a109
Compare
now updated to match the style of #128607 (the aim there is to not use llvm CLI tools, which is a good idea) |
@bors r+ |
…isibility, r=bjorn3 add test for symbol visibility of `#[naked]` functions tracking issue: rust-lang#90957 This test is extracted from rust-lang#128004 That PR attempts to generated naked functions as an extern function declaration, combined with a global asm block that provides the implementation for that declaration. In order to link declaration and definition together, some flavor of external linking must be used: LLVM will error for other linkage types. Specifically the allowed options are `#[linkage = "external"]` and `#[linkage = "extern_weak"]`. That is kind of an implementation detail though: to the user, a naked function should just behave like a normal function. Hence it should be visible to the linker under the same circumstances as a normal, vanilla function and have the same attributes (Weak, External). Getting this behavior right will require some care, so I think it's a good idea to lock it in now, before making any changes, to make sure we don't regress. Are there any interesting cases that I missed here? E.g. is checking on different architectures worth it? I don't think the other binary types (rlib etc) are relevant here, but may be missing something. r? `@bjorn3`
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#128305 (improve error message when `global_asm!` uses `asm!` operands) - rust-lang#128362 (add test for symbol visibility of `#[naked]` functions) - rust-lang#128526 (time.rs: remove "Basic usage text") - rust-lang#128531 (Miri: add a flag to do recursive validity checking) - rust-lang#128589 (allow setting `link-shared` and `static-libstdcpp` with CI LLVM) - rust-lang#128615 (rustdoc: make the hover trail for doc anchors a bit bigger) - rust-lang#128620 (Update rinja version to 0.3.0) r? `@ghost` `@rustbot` modify labels: rollup
…isibility, r=bjorn3 add test for symbol visibility of `#[naked]` functions tracking issue: rust-lang#90957 This test is extracted from rust-lang#128004 That PR attempts to generated naked functions as an extern function declaration, combined with a global asm block that provides the implementation for that declaration. In order to link declaration and definition together, some flavor of external linking must be used: LLVM will error for other linkage types. Specifically the allowed options are `#[linkage = "external"]` and `#[linkage = "extern_weak"]`. That is kind of an implementation detail though: to the user, a naked function should just behave like a normal function. Hence it should be visible to the linker under the same circumstances as a normal, vanilla function and have the same attributes (Weak, External). Getting this behavior right will require some care, so I think it's a good idea to lock it in now, before making any changes, to make sure we don't regress. Are there any interesting cases that I missed here? E.g. is checking on different architectures worth it? I don't think the other binary types (rlib etc) are relevant here, but may be missing something. r? ``@bjorn3``
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#128362 (add test for symbol visibility of `#[naked]` functions) - rust-lang#128526 (time.rs: remove "Basic usage text") - rust-lang#128531 (Miri: add a flag to do recursive validity checking) - rust-lang#128578 (rustdoc: Cleanup `CacheBuilder` code for building search index) - rust-lang#128589 (allow setting `link-shared` and `static-libstdcpp` with CI LLVM) - rust-lang#128615 (rustdoc: make the hover trail for doc anchors a bit bigger) - rust-lang#128620 (Update rinja version to 0.3.0) r? `@ghost` `@rustbot` modify labels: rollup
@bors r- .. ^^ |
940a109
to
5ee6701
Compare
that typo has now been resolved (and the commits squashed) @rustbot ready (we could try a try action though?) |
I don't think a try build is necessary for now. If it fails again, a try build may be worthwhile. @bors r+ |
so, not even
@ChrisDenton I don't think this does much different to the |
According to Object::dynamic_symbols.
I'm not sure why it would work with windows-gnu. |
hmm okay, weird. even based on the source that really should not work on any windows target. I guess I can filter |
d7ddc3a
to
ee43eef
Compare
@bors try |
…ibility, r=<try> add test for symbol visibility of `#[naked]` functions tracking issue: rust-lang#90957 This test is extracted from rust-lang#128004 That PR attempts to generated naked functions as an extern function declaration, combined with a global asm block that provides the implementation for that declaration. In order to link declaration and definition together, some flavor of external linking must be used: LLVM will error for other linkage types. Specifically the allowed options are `#[linkage = "external"]` and `#[linkage = "extern_weak"]`. That is kind of an implementation detail though: to the user, a naked function should just behave like a normal function. Hence it should be visible to the linker under the same circumstances as a normal, vanilla function and have the same attributes (Weak, External). Getting this behavior right will require some care, so I think it's a good idea to lock it in now, before making any changes, to make sure we don't regress. Are there any interesting cases that I missed here? E.g. is checking on different architectures worth it? I don't think the other binary types (rlib etc) are relevant here, but may be missing something. r? `@bjorn3` try-job: x86_64-msvc try-job: i686-msvc try-job: i686-mingw try-job: x86_64-mingw
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
well:
so, I'd propose to skip msvc, maybe all of windows, for now? |
ee43eef
to
3462e45
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added //@ only-linux
, which combined with //@ only-x86_64
Can you use |
3462e45
to
1967951
Compare
@@ -0,0 +1,98 @@ | |||
//@ ignore-windows | |||
//@ only-x86_64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the only-x86_64
really necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the inline/global assembly will break on aarch64 right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right
@bors r+ |
…isibility, r=bjorn3 add test for symbol visibility of `#[naked]` functions tracking issue: rust-lang#90957 This test is extracted from rust-lang#128004 That PR attempts to generated naked functions as an extern function declaration, combined with a global asm block that provides the implementation for that declaration. In order to link declaration and definition together, some flavor of external linking must be used: LLVM will error for other linkage types. Specifically the allowed options are `#[linkage = "external"]` and `#[linkage = "extern_weak"]`. That is kind of an implementation detail though: to the user, a naked function should just behave like a normal function. Hence it should be visible to the linker under the same circumstances as a normal, vanilla function and have the same attributes (Weak, External). Getting this behavior right will require some care, so I think it's a good idea to lock it in now, before making any changes, to make sure we don't regress. Are there any interesting cases that I missed here? E.g. is checking on different architectures worth it? I don't think the other binary types (rlib etc) are relevant here, but may be missing something. r? `@bjorn3`
Rollup of 7 pull requests Successful merges: - rust-lang#128107 (Migrate `raw-dylib-alt-calling-convention`, `raw-dylib-c` and `redundant-libs` `run-make` tests to rmake) - rust-lang#128362 (add test for symbol visibility of `#[naked]` functions) - rust-lang#128417 (Add `f16` and `f128` math functions) - rust-lang#128638 (run-make: enable msvc for `link-dedup`) - rust-lang#128647 (Enable msvc for link-args-order) - rust-lang#128649 (run-make: Enable msvc for `no-duplicate-libs` and `zero-extend-abi-param-passing`) - rust-lang#128766 (Trivial grammar fix in const keyword docs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 7 pull requests Successful merges: - rust-lang#128107 (Migrate `raw-dylib-alt-calling-convention`, `raw-dylib-c` and `redundant-libs` `run-make` tests to rmake) - rust-lang#128362 (add test for symbol visibility of `#[naked]` functions) - rust-lang#128417 (Add `f16` and `f128` math functions) - rust-lang#128638 (run-make: enable msvc for `link-dedup`) - rust-lang#128647 (Enable msvc for link-args-order) - rust-lang#128649 (run-make: Enable msvc for `no-duplicate-libs` and `zero-extend-abi-param-passing`) - rust-lang#128766 (Trivial grammar fix in const keyword docs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 7 pull requests Successful merges: - rust-lang#128107 (Migrate `raw-dylib-alt-calling-convention`, `raw-dylib-c` and `redundant-libs` `run-make` tests to rmake) - rust-lang#128362 (add test for symbol visibility of `#[naked]` functions) - rust-lang#128417 (Add `f16` and `f128` math functions) - rust-lang#128638 (run-make: enable msvc for `link-dedup`) - rust-lang#128647 (Enable msvc for link-args-order) - rust-lang#128649 (run-make: Enable msvc for `no-duplicate-libs` and `zero-extend-abi-param-passing`) - rust-lang#128766 (Trivial grammar fix in const keyword docs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#128362 - folkertdev:naked-function-symbol-visibility, r=bjorn3 add test for symbol visibility of `#[naked]` functions tracking issue: rust-lang#90957 This test is extracted from rust-lang#128004 That PR attempts to generated naked functions as an extern function declaration, combined with a global asm block that provides the implementation for that declaration. In order to link declaration and definition together, some flavor of external linking must be used: LLVM will error for other linkage types. Specifically the allowed options are `#[linkage = "external"]` and `#[linkage = "extern_weak"]`. That is kind of an implementation detail though: to the user, a naked function should just behave like a normal function. Hence it should be visible to the linker under the same circumstances as a normal, vanilla function and have the same attributes (Weak, External). Getting this behavior right will require some care, so I think it's a good idea to lock it in now, before making any changes, to make sure we don't regress. Are there any interesting cases that I missed here? E.g. is checking on different architectures worth it? I don't think the other binary types (rlib etc) are relevant here, but may be missing something. r? ``@bjorn3``
tracking issue: #90957
This test is extracted from #128004
That PR attempts to generated naked functions as an extern function declaration, combined with a global asm block that provides the implementation for that declaration.
In order to link declaration and definition together, some flavor of external linking must be used: LLVM will error for other linkage types. Specifically the allowed options are
#[linkage = "external"]
and#[linkage = "extern_weak"]
. That is kind of an implementation detail though: to the user, a naked function should just behave like a normal function.Hence it should be visible to the linker under the same circumstances as a normal, vanilla function and have the same attributes (Weak, External). Getting this behavior right will require some care, so I think it's a good idea to lock it in now, before making any changes, to make sure we don't regress.
Are there any interesting cases that I missed here? E.g. is checking on different architectures worth it? I don't think the other binary types (rlib etc) are relevant here, but may be missing something.
r? @bjorn3