Add checking for no_mangle to unsafe_code lint #72209

Nemo157 · 2020-05-14T19:08:29Z

fixes #72188

r? @estebank

src/librustc_lint/builtin.rs

Dylan-DPC-zz · 2020-05-14T20:19:08Z

r? @estebank

ollie27 · 2020-05-14T20:37:49Z

#[export_name = "..."] is unsafe for the same reason as #[no_mangle] so it should also be detected by this lint.

Dylan-DPC-zz · 2020-06-04T19:37:54Z

@estebank any updates on this?

estebank

Sorry for the delay. We'll need to do a crater run with this to make sure we don't cause major regressions in the wild as this is being added to an existing lint. Beyond these nitpicks, it looks good to me.

estebank · 2020-06-19T16:08:41Z

src/librustc_lint/builtin.rs

@@ -272,6 +272,32 @@ impl EarlyLintPass for UnsafeCode {
                })
            }

+            ast::ItemKind::Fn(..) => {
+                if attr::contains_name(&it.attrs, sym::no_mangle) {
+                    self.report_unsafe(cx, it.span, |lint| {


Can we use it.name.span instead here and in the other 2 places?

estebank · 2020-06-19T16:10:36Z

src/librustc_lint/builtin.rs

+            ast::ItemKind::Fn(..) => {
+                if attr::contains_name(&it.attrs, sym::no_mangle) {
+                    self.report_unsafe(cx, it.span, |lint| {
+                        lint.build("declaration of a `no_mangle` function").emit();


Could we add a note to explain why the annotation is problematic and or a link to the docs?

Nemo157 · 2020-06-30T13:18:04Z

@estebank fixed the nitpicks. Are you able to trigger the crater run?

Dylan-DPC-zz · 2020-06-30T18:59:22Z

@bors try (before crater run)

bors · 2020-06-30T18:59:37Z

⌛ Trying commit 36c75f4d7dc23bad408f0eb92cd30365434cf2a2 with merge 00bf438af9d3399b4145c87e4f9f6133e40fd673...

bors · 2020-06-30T22:08:51Z

☀️ Try build successful - checks-actions, checks-azure
Build commit: 00bf438af9d3399b4145c87e4f9f6133e40fd673 (00bf438af9d3399b4145c87e4f9f6133e40fd673)

Dylan-DPC-zz · 2020-07-17T21:00:29Z

@craterbot check

craterbot · 2020-07-17T21:00:43Z

👌 Experiment pr-72209 created and queued.
🤖 Automatically detected try build 00bf438af9d3399b4145c87e4f9f6133e40fd673
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

danielhenrymantilla · 2020-07-19T16:23:35Z

src/test/ui/lint/lint-unsafe-code.stderr

-LL | unsafe fn baz() {}
-   | ^^^^^^^^^^^^^^^^^^
+LL | #[no_mangle] fn foo() {}
+   |                 ^^^


Could the lint span point to the attribute itself, rather than the identifier of the item being defined?

Suggested change

| ^^^

| ^^^^^^^^^^^^

Makes sense, I'll try to get this changed tomorrow.

craterbot · 2020-07-28T13:15:04Z

🚧 Experiment pr-72209 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-07-28T13:44:52Z

🚨 Experiment pr-72209 has encountered an error: command Command { std: "/workspace/cargo-home/bin/rustup-toolchain-install-master" "e070765f6e6c1035c7c728be486e7e69cb2b4808" "-c" "cargo", kill_on_drop: false } failed
🛠️ If the error is fixed use the retry command.

🆘 Can someone from the infra team check in on this? @rust-lang/infra
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Mark-Simulacrum · 2020-07-28T17:23:27Z

@craterbot retry seems to have been spurious

craterbot · 2020-07-28T17:23:32Z

🚨 Error: failed to parse the command

🆘 If you have any trouble with Crater please ping @rust-lang/infra!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Mark-Simulacrum · 2020-07-28T17:37:21Z

@craterbot retry

craterbot · 2020-07-28T17:37:22Z

🛠️ Experiment pr-72209 queued again.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-07-28T22:18:04Z

🚧 Experiment pr-72209 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

nikomatsakis · 2020-10-19T20:18:34Z

@rfcbot reviewed

I'm nominated for lang team discussion next week to draw attention to this.

pnkfelix · 2020-11-02T20:25:50Z

@rfcbot reviewed

rfcbot · 2020-11-02T20:25:53Z

🔔 This is now entering its final comment period, as per the review above. 🔔

rfcbot · 2020-11-12T20:27:53Z

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.

The RFC will be merged soon.

Dylan-DPC-zz · 2021-01-17T00:56:51Z

@estebank this is waiting for your final review. Looks ready imo 😃

nikomatsakis · 2021-02-08T14:32:49Z

@bors r+

I'm sniping this one. Sorry @estebank =)

bors · 2021-02-08T14:32:50Z

📌 Commit fc8a3ad has been approved by nikomatsakis

Rollup of 11 pull requests Successful merges: - rust-lang#72209 (Add checking for no_mangle to unsafe_code lint) - rust-lang#80732 (Allow Trait inheritance with cycles on associated types take 2) - rust-lang#81697 (Add "every" as a doc alias for "all".) - rust-lang#81826 (Prefer match over combinators to make some Box methods inlineable) - rust-lang#81834 (Resolve typedef in HashMap lldb pretty-printer only if possible) - rust-lang#81841 ([rustbuild] Output rustdoc-json-types docs ) - rust-lang#81849 (Expand the docs for ops::ControlFlow a bit) - rust-lang#81876 (parser: Fix panic in 'const impl' recovery) - rust-lang#81882 (:arrow_up: rust-analyzer) - rust-lang#81888 (Fix pretty printer macro_rules with semicolon.) - rust-lang#81896 (Remove outdated comment in windows' mutex.rs) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Nemo157 commented May 14, 2020

View reviewed changes

src/librustc_lint/builtin.rs Outdated Show resolved Hide resolved

rust-highfive assigned estebank May 14, 2020

jonas-schievink added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 15, 2020

joelpalmer added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2020

Elinvynia added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 3, 2020

Elinvynia added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2020

estebank approved these changes Jun 19, 2020

View reviewed changes

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 17, 2020

danielhenrymantilla reviewed Jul 19, 2020

View reviewed changes

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Nov 2, 2020

nikomatsakis removed the I-nominated label Nov 10, 2020

spastorino removed the to-announce Announce this issue on triage meeting label Nov 19, 2020

This comment has been minimized.

Sign in to view

crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 18, 2020

Nemo157 mentioned this pull request Jan 15, 2021

Mark no_mangle functions as unsafe geiger-rs/cargo-geiger#173

Merged

nikomatsakis approved these changes Feb 8, 2021

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 8, 2021

Dylan-DPC-zz mentioned this pull request Feb 9, 2021

Rollup of 11 pull requests #81905

Merged

bors merged commit f8b330d into rust-lang:master Feb 9, 2021

rustbot added this to the 1.52.0 milestone Feb 9, 2021

Nemo157 deleted the lint-no-mangle-in-unsafe-code branch February 9, 2021 09:06

Nemo157 mentioned this pull request Feb 24, 2021

Add allow-by-default lints for all attributes with unsafe behaviors #82499

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add checking for no_mangle to unsafe_code lint #72209

Add checking for no_mangle to unsafe_code lint #72209

Nemo157 commented May 14, 2020 •

edited

Loading

Dylan-DPC-zz commented May 14, 2020

ollie27 commented May 14, 2020

Dylan-DPC-zz commented Jun 4, 2020

estebank left a comment

estebank Jun 19, 2020

estebank Jun 19, 2020

Nemo157 commented Jun 30, 2020

Dylan-DPC-zz commented Jun 30, 2020

bors commented Jun 30, 2020

bors commented Jun 30, 2020

Dylan-DPC-zz commented Jul 17, 2020

craterbot commented Jul 17, 2020

danielhenrymantilla Jul 19, 2020 •

edited

Loading

Nemo157 Aug 1, 2020

craterbot commented Jul 28, 2020

craterbot commented Jul 28, 2020

Mark-Simulacrum commented Jul 28, 2020

craterbot commented Jul 28, 2020

Mark-Simulacrum commented Jul 28, 2020

craterbot commented Jul 28, 2020

craterbot commented Jul 28, 2020

nikomatsakis commented Oct 19, 2020

pnkfelix commented Nov 2, 2020

rfcbot commented Nov 2, 2020

rfcbot commented Nov 12, 2020

This comment has been minimized.

Dylan-DPC-zz commented Jan 17, 2021

nikomatsakis commented Feb 8, 2021

bors commented Feb 8, 2021

Add checking for no_mangle to unsafe_code lint #72209

Add checking for no_mangle to unsafe_code lint #72209

Conversation

Nemo157 commented May 14, 2020 • edited Loading

Dylan-DPC-zz commented May 14, 2020

ollie27 commented May 14, 2020

Dylan-DPC-zz commented Jun 4, 2020

estebank left a comment

Choose a reason for hiding this comment

estebank Jun 19, 2020

Choose a reason for hiding this comment

estebank Jun 19, 2020

Choose a reason for hiding this comment

Nemo157 commented Jun 30, 2020

Dylan-DPC-zz commented Jun 30, 2020

bors commented Jun 30, 2020

bors commented Jun 30, 2020

Dylan-DPC-zz commented Jul 17, 2020

craterbot commented Jul 17, 2020

danielhenrymantilla Jul 19, 2020 • edited Loading

Choose a reason for hiding this comment

Nemo157 Aug 1, 2020

Choose a reason for hiding this comment

craterbot commented Jul 28, 2020

craterbot commented Jul 28, 2020

Mark-Simulacrum commented Jul 28, 2020

craterbot commented Jul 28, 2020

Mark-Simulacrum commented Jul 28, 2020

craterbot commented Jul 28, 2020

craterbot commented Jul 28, 2020

nikomatsakis commented Oct 19, 2020

pnkfelix commented Nov 2, 2020

rfcbot commented Nov 2, 2020

rfcbot commented Nov 12, 2020

This comment has been minimized.

Dylan-DPC-zz commented Jan 17, 2021

nikomatsakis commented Feb 8, 2021

bors commented Feb 8, 2021

Nemo157 commented May 14, 2020 •

edited

Loading

danielhenrymantilla Jul 19, 2020 •

edited

Loading