Add lint about redefining runtime symbols by Urgau · Pull Request #146505 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation26 Commits3 Checks10 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This PR adds lint to warn about redefinition of runtime symbols1 that are assumed and used by core2 and std.

We have had multiple reports of users tripping over this:

`redefining_runtime_symbols`

Old proposed name: clashingfunctionnameswithfundamentalfunctions

(warn-by-default)

The redefining_runtime_symbols lint checks for items whose symbol name redefines a runtime symbols expected by core and/or std.

Example

#[unsafe(no_mangle)] pub fn strlen() {} // redefines the libc strlen function

warning: redefinition of the runtime `strlen` symbol used by the standard library
 --> a.rs:2:1
  |
2 | pub fn strlen() {}
  | ^^^^^^^^^^^^^^^^^^
  |
  = note: extra care must be taken when redefining those symbols, they must match exactly (ABI, function arguments, function return type, behavior, ...)
  = note: see <https://doc.rust-lang.org/core/index.html#how-to-use-the-core-library> for the more details
  = help: either allow this lint or remove any `#[unsafe(no_mangle)]` or `#[unsafe(export_name = "strlen")]`
  = note: `#[warn(redefining_runtime_symbols)]` on by default

Explanation

Up-most care is required when redefining runtime symbols assumed and used by the standard library. They must follow the C specification, not use any standard-library facility or undefined behavior may occur.

The symbols currently checked are respectively:

from core2: memcpy, memmove, memset, memcmp, bcmp, strlen
from std: open/open64, read, write, close

@rustbot labels +I-lang-nominated +T-lang +needs-fcp +A-lints
cc @traviscross
r? compiler

Footnotes

previous lint name clashing_function_names_with_fundamental_functions, bike-shed at https://github.com/rust-lang/rust/pull/146505#issuecomment-3288716835 ↩
https://doc.rust-lang.org/core/index.html#how-to-use-the-core-library ↩ ↩2

rustbot added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Sep 13, 2025

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request

Sep 13, 2025

…

Add lint warn about clashing function names with fundamental functions

This comment has been minimized.

☀️ Try build successful (CI)
Build commit: fb73ebd (fb73ebde4cd6ae9fb27bac8017eab9dc519131f9, parent: 064cc81354a940e297a1be4dfa9e26759c8431be)

This comment has been minimized.

Finished benchmarking commit (fb73ebd): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary -0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	8.2%	[8.2%, 8.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.7%	[-2.5%, -1.1%]	6
All ❌✅ (primary)	-	-	0

Cycles

Results (secondary -2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.6%	[-3.6%, -1.6%]	2
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 468.794s -> 471.13s (0.50%)
Artifact size: 388.08 MiB -> 388.08 MiB (0.00%)

Urgau changed the title~~Add lint warn about clashing function names with fundamental functions~~ Add lint about clashing function names with fundamental functions

Sep 13, 2025

Urgau marked this pull request as ready for review

September 13, 2025 17:00

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

rustbot added A-lints

Area: Lints (warnings about flaws in source code) such as unused_mut.

I-lang-nominated

Nominated for discussion during a lang team meeting.

needs-fcp

This change is insta-stable, or significant enough to need a team FCP to proceed.

T-lang

Relevant to the language team

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Sep 13, 2025

This comment has been minimized.

To bikeshed the name:

I wouldn't call this "clashing" unless the lint is checking specifically that the signature doesn't match what is expected for each symbol. We have an existing clashing_extern_declarations lint, and that one does check for mismatched signatures. Maybe "redefined/redefining", "shadowed/shadowing", "colliding", etc. would work.

I wouldn't refer to "function names" here because a "function name" in Rust refers to the name of the function in Rust rather than to the symbol name, and we want to focus on the symbol name here.

Also, shouldn't we be checking for more than just functions? This breaks things too:

#[unsafe(no_mangle)] static read: () = ();

I probably wouldn't call these functions "fundamental". Maybe "runtime", "system", "platform", "builtin", "libc", or similar would work.

Perhaps redefining_runtime_symbols (or maybe redefined_runtime_symbols) would be a decent name?

traviscross added the P-lang-drag-2

Lang team prioritization drag level 2.https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang.

label

Sep 13, 2025

Urgau changed the title~~Add lint about clashing function names with fundamental functions~~ Add lint about redefining runtime symbols

Sep 13, 2025

Perhaps redefining_runtime_symbols (or maybe redefined_runtime_symbols) would be a decent name?

redefining_runtime_symbols works for me. Changed the lint name as such.

Also, shouldn't we be checking for more than just functions?

Indeed, forgot that statics also have a symbol. Fixed.

Regarding the lint level, I made it warn-by-default since there are legitimate reasons to implement those symbols (like when implementing a libc), but maybe it should be deny-by-default?

lcnr added S-waiting-on-team

DEPRECATED: Use the team-based variants `S-waiting-on-t-lang`, `S-waiting-on-t-compiler`, ...

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Sep 17, 2025

Three potential improvements to this:

Could we detect whether the signature of the exported function is compatible with what Rust's standard library expects (e.g. strlen), and make that case a deny-by-default lint ("this is very likely to cause crashes")? That's separate from a potential warn-by-default lint for a compatible redefinition.

Also, could we somehow suppress the warning for symbols only used by std, if std isn't being linked in at all?

Finally, could we allow-by-default this lint if you're in a "standalone"/"freestanding" mode (e.g. you're on a -none target and not linking libc at all)?

Note that there are legitimate reasons for people to define these symbols, and we want to avoid giving people the impression that Rust is the wrong language to write such code in. For instance, OS kernels, or intentional interposing of these symbols.

Wishlist: it'd be really nice if rustc looked at the things that you are extern fning or no_mangleing and checking to make sure that those are consistent with anything that your dependencies are doing.

One further note: once we have the deny case for having the wrong type, we should evaluate whether the warn case is catching more accidents or catching mostly people who are doing this intentionally. If the latter, we may wish to change it from warn to allow or drop it.

joshtriplett added I-lang-radar

Items that are on lang's radar and will need eventual work or consideration.

and removed I-lang-radar

Items that are on lang's radar and will need eventual work or consideration.

labels

Nov 5, 2025

Here's a thought I mentioned in the meeting. @scottmcm, in particular, suggested it was a strong point and encouraged capturing it here.

I see it as important to the statement of what Rust is -- to our story -- that you can use Rust to write a kernel or a libc -- that it's a C competitor in that sense.

It's for this reason that, unless we've targeted the lint such that we're sure that we're detecting actual UB, I wouldn't want to ever go deny-by-default with this. I don't want us to suggest, with our linting, that "Rust isn't the language for you if you want to do this kind of work."

We have other things to do here first, but on reflection, I think I'd actually propose the lint name runtime_symbol_definitions.

Generally our lint names represent the class of construct that is being allowed (or denied, etc.). Here, what we're linting are definitions of certain runtime symbols. We don't even know for certain, until much later, whether these are in fact redefinitions.

This is the notion that I had been trying to capture with redefining_runtime_symbols. Though it could be read as a gerund, I had actually meant the present participle as an adjective, in the sense that the linted item is causing a defined symbol to be added to the symbol table, and the presence of that defined runtime symbol will effect the redefinition. I'd preferred this to redefined_runtime_symbols because that put the focus in the wrong place, on the symbol being redefined rather than on the construct doing the redefining, which is what we're specifically allowing or denying. But saying runtime_symbol_definitions is more clear and to the point than either of these.