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.

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

New lint: Performing arithmetic on numeric value of function pointer #9517

Open
Aaron1011 opened this issue Sep 22, 2022 · 10 comments
Open

New lint: Performing arithmetic on numeric value of function pointer #9517

Aaron1011 opened this issue Sep 22, 2022 · 10 comments
Labels
A-lint Area: New lints

Comments

@Aaron1011
Copy link
Member

What it does

This lints on any expression of the form some_arithmetic_expr(function_pointer as NumericType, other_type).

For example (note the lack of parenthesis - all of the function used here are function pointers)

let alloc_size: u64 = std::mem::size_of::<f32> as u64 * 5u64;
let extra_space: u32 = 1 + std::mem::size_of::<char> as u32;

Lint Name

function_pointer_arithmetic

Category

correctness

Advantage

Performing arithmetic on a function pointer value (cast as a numeric type) is virtually never correct. Rust provides no guarantees about the relative location of functions in memory, or the actual in-memory assembly code at a function's address. In particular, multiplying or dividing by a pointer value is completely meaningless.

Drawbacks

None

Example

When writing low-level code, it's often useful to calculate sizes by multiplying by std::mem::size_of::<SomeType>. For example:

let alloc_size: u64 = num_elements * std::mem::size_of::<MyType>() as u64

However, Rust allows casting a function pointer to any numeric type. If you accidentally commit the parenthesis in the function call, you'll obtain the following valid code:

let alloc_size: u64 = num_elements * std::mem::size_of::<MyType> as u64

This multiplies a function address by num_elements, which is completely useless. The overall result will be completely unrelated to the size of MyType (and depending on the address of std::mem::size_of::<MyType>, it may also be very large). The code looks almost identical to the correct version, which could make it very difficult to notice the cause of the incorrect value.

@Aaron1011 Aaron1011 added the A-lint Area: New lints label Sep 22, 2022
@kraktus
Copy link
Contributor

kraktus commented Sep 22, 2022

There’s an experiment about forbidding them: rust-lang/rust#95228 first comments talk about a lint, which I haven’t found traces about

@Aaron1011
Copy link
Member Author

That's a very long-term change, and actually disallowing pointer as usize would be behind a new edition.

@c410-f3r
Copy link
Contributor

arithmetic-side-effects is intended to warn any kind of arithmetic operation that can cause any kind of possible side effect. As such, I will incorporate this use case in the upcoming days but anyone is free to extract the internal logic to create a standalone lint.

@Aaron1011
Copy link
Member Author

I don't think this related to arithmetic-side-effects. Multiplying or adding to a function-pointer-derive number doesn't produce any side effects (beyond the overflow behavior that arithmetic-side-effects) already captures. The issue is that it's a completely meaningless operation, and can be written accidentally by leaving off the parenthesis in a function call.

@c410-f3r
Copy link
Contributor

c410-f3r commented Sep 24, 2022

The "side effect" here would be a wrong or surprising arithmetic result that can produce wrong final outputs but regardless of the merit, I honestly don't want to ride the ol' debate train. As such, I withdraw my participation in this use case.

@kraktus
Copy link
Contributor

kraktus commented Sep 24, 2022

Seem it’s already implemented with clippy::fn_to_numeric_cast_any

@niluxv
Copy link

niluxv commented Sep 27, 2022

There’s an experiment about forbidding them: rust-lang/rust#95228 first comments talk about a lint, which I haven’t found traces about

Yes, this is about forbidding all ptr to int as casts. It is available as an unstable lint (lossy_provenance_casts). However, it leaves function pointers alone for now because function pointers are strange.

So that is indeed mostly unrelated to the clippy::fn_to_numeric_cast_any lint.

@kraktus
Copy link
Contributor

kraktus commented Sep 28, 2022

There’s an experiment about forbidding them: rust-lang/rust#95228 first comments talk about a lint, which I haven’t found traces about

Yes, this is about forbidding all ptr to int as casts. It is available as an unstable lint (lossy_provenance_casts). However, it leaves function pointers alone for now because function pointers are strange.

So that is indeed mostly unrelated to the clippy::fn_to_numeric_cast_any lint.

Did you wanted to say related? I tried on the playground and fn_to_numeric_cast_any is rightly triggered.

@niluxv
Copy link

niluxv commented Sep 29, 2022

Sorry, with "that" I meant the lossy_provenance_casts lint stuff, not the question of the OP. I completely agree with you that the clippy::fn_to_numeric_cast_any lint is what the OP asks for (so maybe the issue can be closed, unless the request is to make the lint warn by default?).

@Aaron1011
Copy link
Member Author

Aaron1011 commented Sep 30, 2022

This is not the same thing as clippy::fn_to_numeric_cast_any. There are legitimate usecases for casting a function pointer to a numeric type (even if those eventually turn into explicit provenance-api calls in new editions). However, multiplying or dividing by that value (or any pointer-derived numeric value) is pretty much guaranteed to be a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

4 participants