Error on invalid macho section specifier#155065
Conversation
|
Some changes occurred in compiler/rustc_hir/src/attrs cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_passes/src/check_attr.rs cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_attr_parsing |
|
just in case we're doing something weird that never got built or linked @bors try jobs=aarch64-apple |
This comment has been minimized.
This comment has been minimized.
error on invalid macho section specifier try-job: aarch64-apple
This comment has been minimized.
This comment has been minimized.
35e0a3f to
c90d31f
Compare
We're trying to move as many checks as possible to attribute parsing, with the "parse, don't validate" mentality, so if an attribute gets through parsing you can assume it is correct. Therefore I'd like this to be in the attribute parser |
|
Reminder, once the PR becomes ready for a review, use |
This comment has been minimized.
This comment has been minimized.
|
💔 Test for 3e6930f failed: CI. Failed job:
|
This comment has been minimized.
This comment has been minimized.
c90d31f to
30b9c6e
Compare
This comment has been minimized.
This comment has been minimized.
30b9c6e to
db42ae2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
…, r=JonathanBrouwer error on invalid macho section specifier The macho section specifier used by `#[link_section = "..."]` is more strict than e.g. the one for elf. LLVM will error when you get it wrong, which is easy to do if you're used to elf. So, provide some guidance for the simplest mistakes, based on the LLVM validation. Currently compilation fails with an LLVM error, see https://godbolt.org/z/WoE8EdK1K. The LLVM validation logic is at https://github.com/llvm/llvm-project/blob/a0f0d6342e0cd75b7f41e0e6aae0944393b68a62/llvm/lib/MC/MCSectionMachO.cpp#L199-L203 LLVM validates the other components of the section specifier too, but it feels a bit fragile to duplicate those checks. If you get that far, hopefully the LLVM errors will be sufficient to get unstuck. --- sidequest from rust-lang#147811 r? JonathanBrouwer specifically, is this the right place for this sort of validation? `rustc_attr_parsing` also does some validation.
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. |
7aeba4e to
a4f5c6e
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors r=JonathanBrouwer |
…, r=JonathanBrouwer Error on invalid macho section specifier The macho section specifier used by `#[link_section = "..."]` is more strict than e.g. the one for elf. LLVM will error when you get it wrong, which is easy to do if you're used to elf. So, provide some guidance for the simplest mistakes, based on the LLVM validation. Currently compilation fails with an LLVM error, see https://godbolt.org/z/WoE8EdK1K. The LLVM validation logic is at https://github.com/llvm/llvm-project/blob/a0f0d6342e0cd75b7f41e0e6aae0944393b68a62/llvm/lib/MC/MCSectionMachO.cpp#L199-L203 LLVM validates the other components of the section specifier too, but it feels a bit fragile to duplicate those checks. If you get that far, hopefully the LLVM errors will be sufficient to get unstuck. --- sidequest from rust-lang#147811 r? JonathanBrouwer specifically, is this the right place for this sort of validation? `rustc_attr_parsing` also does some validation.
…uwer Rollup of 4 pull requests Successful merges: - #146181 (Add intrinsic for launch-sized workgroup memory on GPUs) - #155065 (Error on invalid macho section specifier) - #155676 ( Reject implementing const Drop for types that are not const `Destruct` already) - #155783 (Do not suggest internal cfg trace attributes)
…uwer Rollup of 9 pull requests Successful merges: - #146181 (Add intrinsic for launch-sized workgroup memory on GPUs) - #154803 (Fix ICE from cfg_attr_trace ) - #155065 (Error on invalid macho section specifier) - #155485 (Add an edge-case test for `--remap-path-prefix` for `rustc` & `rustdoc`) - #155659 (cleanup, restructure and merge `tests/ui/deriving` into `tests/ui/derives`) - #155676 ( Reject implementing const Drop for types that are not const `Destruct` already) - #155696 (Add a higher-level API for parsing attributes) - #155769 (triagebot.toml: Ping Enselic when tests/debuginfo/basic-stepping.rs changes) - #155783 (Do not suggest internal cfg trace attributes)
Rollup merge of #155065 - folkertdev:macho-section-specifier, r=JonathanBrouwer Error on invalid macho section specifier The macho section specifier used by `#[link_section = "..."]` is more strict than e.g. the one for elf. LLVM will error when you get it wrong, which is easy to do if you're used to elf. So, provide some guidance for the simplest mistakes, based on the LLVM validation. Currently compilation fails with an LLVM error, see https://godbolt.org/z/WoE8EdK1K. The LLVM validation logic is at https://github.com/llvm/llvm-project/blob/a0f0d6342e0cd75b7f41e0e6aae0944393b68a62/llvm/lib/MC/MCSectionMachO.cpp#L199-L203 LLVM validates the other components of the section specifier too, but it feels a bit fragile to duplicate those checks. If you get that far, hopefully the LLVM errors will be sufficient to get unstuck. --- sidequest from #147811 r? JonathanBrouwer specifically, is this the right place for this sort of validation? `rustc_attr_parsing` also does some validation.
|
The Reference didn't need to change for this PR because it's silent on the format of these strings — they're target-defined. cc @rust-lang/lang-docs @rust-lang/fls |
|
Though if you do you want this documented, I've put up rust-lang/reference#2236 for it |
Ah, great. Thanks for that. Will have a look. |
rust-lang/rust#155065 recently added validation for link_section on macOS. I'm working around it here by just limiting this test to linux. Alternatives: - Show different syntaxes for different targets (but I would prefer to keep the example as plain as possible). - Mark it as `ignore` (want to avoid this if at all possible). Fixes rust-lang#2245
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [rust](https://github.com/rust-lang/rust) | minor | `1.96.1` → `1.97.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>rust-lang/rust (rust)</summary> ### [`v1.97.0`](https://github.com/rust-lang/rust/blob/HEAD/RELEASES.md#Version-1970-2026-07-09) [Compare Source](rust-lang/rust@1.96.1...1.97.0) \========================== <a id="1.97.0-Language"></a> ## Language - [Consider `Result<T, Uninhabited>` and `ControlFlow<Uninhabited, T>` to be equivalent to `T` for must use lint](rust-lang/rust#148214) - [Add allow-by-default `dead_code_pub_in_binary` lint for unused pub items in binary crates](rust-lang/rust#149509) - [Stabilize the `div32`, `lam-bh`, `lamcas`, `ld-seq-sa` and `scq` target features](rust-lang/rust#154510) - [Stabilize `cfg(target_has_atomic_primitive_alignment)`](rust-lang/rust#155006) - [Allow trailing `self` in imports in more cases](rust-lang/rust#155137) <a id="1.97.0-Platform-Support"></a> ## Platform Support - [nvptx64-nvidia-cuda: drop support for old architectures and old ISAs](rust-lang/rust#152443) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. [platform-support-doc]: https://doc.rust-lang.org/rustc/platform-support.html <a id="1.97.0-Stabilized-APIs"></a> ## Stabilized APIs - [`Default for RepeatN`](https://doc.rust-lang.org/stable/std/iter/struct.RepeatN.html#impl-Default-for-RepeatN%3CA%3E) - [`Copy for ffi::FromBytesUntilNulError`](https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html#impl-Copy-for-FromBytesUntilNulError) - [`Send for std::fs::File` on UEFI](rust-lang/rust#154003) - [`<{integer}>::isolate_highest_one`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.isolate_highest_one) - [`<{integer}>::isolate_lowest_one`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.isolate_lowest_one) - [`<{integer}>::highest_one`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.highest_one) - [`<{integer}>::lowest_one`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.lowest_one) - [`<{integer}>::bit_width`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.bit_width) - [`NonZero<{integer}>::isolate_highest_one`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.isolate_highest_one) - [`NonZero<{integer}>::isolate_lowest_one`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.isolate_lowest_one) - [`NonZero<{integer}>::highest_one`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.highest_one) - [`NonZero<{integer}>::lowest_one`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.lowest_one) - [`NonZero<{integer}>::bit_width`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.bit_width) These previously stable APIs are now stable in const contexts: - [`char::is_control`](https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_control) <a id="1.97.0-Cargo"></a> ## Cargo - [Stabilize `build.warnings` config.](rust-lang/cargo#16796) This controls how lint warnings from local packages are treated. Useful for enforcing a warning-free build in CI, replacing `-Dwarnings`. [docs](https://doc.rust-lang.org/nightly/cargo/reference/config.html#buildwarnings) - [Stabilize `resolver.lockfile-path` config.](rust-lang/cargo#16694) This allows specifying the path to the lockfile to use when resolving dependencies. Useful when working with read-only source directories. [docs](https://doc.rust-lang.org/nightly/cargo/reference/config.html#resolverlockfile-path) - [cargo-clean: Error when `--target-dir` doesn't look like a Cargo target directory.](rust-lang/cargo#16712) This prevents accidental deletion of non-target directories. - [Add `-m` shorthand for `--manifest-path`](rust-lang/cargo#16858) - [Remove `curl` dependency from `crates-io` crate](rust-lang/cargo#16936) <a id="1.97.0-Rustdoc"></a> ## Rustdoc - [Stabilize `--emit` flag](rust-lang/rust#146220) - [Stabilize `--remap-path-prefix`](rust-lang/rust#155307) <a id="1.97.0-Compatibility-Notes"></a> ## Compatibility Notes - [Emit a future-compatibility warning when relying on `f32: From<{float}>` to constrain `{float}`](rust-lang/rust#139087) - [Rust will use the v0 symbol mangling scheme by default.](rust-lang/rust#151994) This may cause some tools (such as debuggers or profilers, especially with old versions) to fail to demangle symbols emitted by Rust. It may also cause the formatting of text in backtraces to change. - [Prevent deref coercions in `pin!`, in order to prevent unsoundness.](rust-lang/rust#153457) The most likely case where this might impact users is: writing `pin!(x)` where `x` has type `&mut T` will now always correctly produce a value of type `Pin<&mut &mut T>`, instead of sometimes allowing a coercion that produces a value of type `Pin<&mut T>`. This coercion was previously incorrectly allowed since Rust 1.88.0. - [Deprecate `std::char` constants and functions](rust-lang/rust#153873) - [Warn on linker output by default](rust-lang/rust#153968) - [Remove hidden `f64` methods which have been deprecated since 1.0](rust-lang/rust#153975) - [report the `varargs_without_pattern` lint in deps](rust-lang/rust#154599) - [Forbid passing generic arguments to module path segments even if the module reexports a generic enum variant](rust-lang/rust#154971) - [Error on invalid macho `link_section` specifier](rust-lang/rust#155065) - The encoding of certain `enum`s [have changed](rust-lang/rust#155473). This is not a breaking change, as it only applies to `enum`s without layout guarantees, but is noted here as we've seen people impacted from having made assumptions about the layout algorithm. - [Error on `#[export_name = "..."]` where the name is empty](rust-lang/rust#155515) - [Syntactically reject tuple index shorthands in struct patterns](rust-lang/rust#155698) - [validate `#[link_name = "..."]` & `#[link(name = "...")]` parameters](rust-lang/rust#155817) - On Windows, after calling `shutdown` on a socket to shut down the write side, attempting to write to the socket will now produce a `BrokenPipe` error rather than `Other`. [Map `WSAESHUTDOWN` to `io::ErrorKind::BrokenPipe`](rust-lang/rust#156063) </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNTYuMCIsInVwZGF0ZWRJblZlciI6IjQzLjI1Ni4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6bWlub3IiXX0=-->
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [rust](https://github.com/rust-lang/rust) | | minor | `1.96.1` → `1.97.0` | | rust | stage | minor | `1.96-bookworm` → `1.97-bookworm` | --- ### Release Notes <details> <summary>rust-lang/rust (rust)</summary> ### [`v1.97.0`](https://github.com/rust-lang/rust/blob/HEAD/RELEASES.md#Version-1970-2026-07-09) [Compare Source](rust-lang/rust@1.96.1...1.97.0) \========================== <a id="1.97.0-Language"></a> ## Language - [Consider `Result<T, Uninhabited>` and `ControlFlow<Uninhabited, T>` to be equivalent to `T` for must use lint](rust-lang/rust#148214) - [Add allow-by-default `dead_code_pub_in_binary` lint for unused pub items in binary crates](rust-lang/rust#149509) - [Stabilize the `div32`, `lam-bh`, `lamcas`, `ld-seq-sa` and `scq` target features](rust-lang/rust#154510) - [Stabilize `cfg(target_has_atomic_primitive_alignment)`](rust-lang/rust#155006) - [Allow trailing `self` in imports in more cases](rust-lang/rust#155137) <a id="1.97.0-Platform-Support"></a> ## Platform Support - [nvptx64-nvidia-cuda: drop support for old architectures and old ISAs](rust-lang/rust#152443) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. [platform-support-doc]: https://doc.rust-lang.org/rustc/platform-support.html <a id="1.97.0-Stabilized-APIs"></a> ## Stabilized APIs - [`Default for RepeatN`](https://doc.rust-lang.org/stable/std/iter/struct.RepeatN.html#impl-Default-for-RepeatN%3CA%3E) - [`Copy for ffi::FromBytesUntilNulError`](https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html#impl-Copy-for-FromBytesUntilNulError) - [`Send for std::fs::File` on UEFI](rust-lang/rust#154003) - [`<{integer}>::isolate_highest_one`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.isolate_highest_one) - [`<{integer}>::isolate_lowest_one`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.isolate_lowest_one) - [`<{integer}>::highest_one`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.highest_one) - [`<{integer}>::lowest_one`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.lowest_one) - [`<{integer}>::bit_width`](https://doc.rust-lang.org/stable/std/primitive.u32.html#method.bit_width) - [`NonZero<{integer}>::isolate_highest_one`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.isolate_highest_one) - [`NonZero<{integer}>::isolate_lowest_one`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.isolate_lowest_one) - [`NonZero<{integer}>::highest_one`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.highest_one) - [`NonZero<{integer}>::lowest_one`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.lowest_one) - [`NonZero<{integer}>::bit_width`](https://doc.rust-lang.org/stable/std/num/struct.NonZero.html#method.bit_width) These previously stable APIs are now stable in const contexts: - [`char::is_control`](https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_control) <a id="1.97.0-Cargo"></a> ## Cargo - [Stabilize `build.warnings` config.](rust-lang/cargo#16796) This controls how lint warnings from local packages are treated. Useful for enforcing a warning-free build in CI, replacing `-Dwarnings`. [docs](https://doc.rust-lang.org/nightly/cargo/reference/config.html#buildwarnings) - [Stabilize `resolver.lockfile-path` config.](rust-lang/cargo#16694) This allows specifying the path to the lockfile to use when resolving dependencies. Useful when working with read-only source directories. [docs](https://doc.rust-lang.org/nightly/cargo/reference/config.html#resolverlockfile-path) - [cargo-clean: Error when `--target-dir` doesn't look like a Cargo target directory.](rust-lang/cargo#16712) This prevents accidental deletion of non-target directories. - [Add `-m` shorthand for `--manifest-path`](rust-lang/cargo#16858) - [Remove `curl` dependency from `crates-io` crate](rust-lang/cargo#16936) <a id="1.97.0-Rustdoc"></a> ## Rustdoc - [Stabilize `--emit` flag](rust-lang/rust#146220) - [Stabilize `--remap-path-prefix`](rust-lang/rust#155307) <a id="1.97.0-Compatibility-Notes"></a> ## Compatibility Notes - [Emit a future-compatibility warning when relying on `f32: From<{float}>` to constrain `{float}`](rust-lang/rust#139087) - [Rust will use the v0 symbol mangling scheme by default.](rust-lang/rust#151994) This may cause some tools (such as debuggers or profilers, especially with old versions) to fail to demangle symbols emitted by Rust. It may also cause the formatting of text in backtraces to change. - [Prevent deref coercions in `pin!`, in order to prevent unsoundness.](rust-lang/rust#153457) The most likely case where this might impact users is: writing `pin!(x)` where `x` has type `&mut T` will now always correctly produce a value of type `Pin<&mut &mut T>`, instead of sometimes allowing a coercion that produces a value of type `Pin<&mut T>`. This coercion was previously incorrectly allowed since Rust 1.88.0. - [Deprecate `std::char` constants and functions](rust-lang/rust#153873) - [Warn on linker output by default](rust-lang/rust#153968) - [Remove hidden `f64` methods which have been deprecated since 1.0](rust-lang/rust#153975) - [report the `varargs_without_pattern` lint in deps](rust-lang/rust#154599) - [Forbid passing generic arguments to module path segments even if the module reexports a generic enum variant](rust-lang/rust#154971) - [Error on invalid macho `link_section` specifier](rust-lang/rust#155065) - The encoding of certain `enum`s [have changed](rust-lang/rust#155473). This is not a breaking change, as it only applies to `enum`s without layout guarantees, but is noted here as we've seen people impacted from having made assumptions about the layout algorithm. - [Error on `#[export_name = "..."]` where the name is empty](rust-lang/rust#155515) - [Syntactically reject tuple index shorthands in struct patterns](rust-lang/rust#155698) - [validate `#[link_name = "..."]` & `#[link(name = "...")]` parameters](rust-lang/rust#155817) - On Windows, after calling `shutdown` on a socket to shut down the write side, attempting to write to the socket will now produce a `BrokenPipe` error rather than `Other`. [Map `WSAESHUTDOWN` to `io::ErrorKind::BrokenPipe`](rust-lang/rust#156063) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9jb250YWluZXIiLCJ0eXBlL21pbm9yIl19--> Reviewed-on: https://codeberg.org/towonel/towonel/pulls/58
View all comments
The macho section specifier used by
#[link_section = "..."]is more strict than e.g. the one for elf. LLVM will error when you get it wrong, which is easy to do if you're used to elf. So, provide some guidance for the simplest mistakes, based on the LLVM validation.Currently compilation fails with an LLVM error, see https://godbolt.org/z/WoE8EdK1K.
The LLVM validation logic is at
https://github.com/llvm/llvm-project/blob/a0f0d6342e0cd75b7f41e0e6aae0944393b68a62/llvm/lib/MC/MCSectionMachO.cpp#L199-L203
LLVM validates the other components of the section specifier too, but it feels a bit fragile to duplicate those checks. If you get that far, hopefully the LLVM errors will be sufficient to get unstuck.
sidequest from #147811
r? JonathanBrouwer
specifically, is this the right place for this sort of validation?
rustc_attr_parsingalso does some validation.