Skip to content

Commit

Permalink
Linter: Warn about strict balance equality (#1914)
Browse files Browse the repository at this point in the history
* feat(linter): strict balance equality lint

The current implementation works only with intraprocedural MIR and does
not support taint propagation across function calls.

Closes #1811

* feat(lint): Handle temporary values resulted after Rvalue::Use

* fix(lint): spans to emit diagnostics

Previously, diagnostics did not work, since `terminator.span` is
resulted after macro expansion

* feat(tests): more tests

* feat(lint): Manually traverse functions in user-defined code

This is required to implement interprocedural analysis

* feat(lint): interprocedural analysis that finds tainted returns

* fix(lint): recursive calls in interprocedural analysis

* fix(lint): false negative on `CheckedBinaryOp`

* feat(lint): propagation through references

* feat(lint): Propagate tainted values through `&mut` arguments

* chore(lint): docstring, comments

* feat(lint): handle comparison of references in functions

* chore(tests): comments

* feat(lint+tests): updated `pass` test, fixed binop conditions

* feat(tests): test for lint suppressions

* chore(tests): fmt

* chore(tests): fmt

* chore: Add changelog entry

* chore(lint): Reuse utility functions introduced in #1932

* chore: Fix changelog

* chore: Fix comments
  • Loading branch information
jubnzv authored Oct 25, 2023
1 parent dacaa28 commit 9cf5b3d
Show file tree
Hide file tree
Showing 10 changed files with 876 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added
- Linter: `storage_never_freed` lint - [#1932](https://github.com/paritytech/ink/pull/1932)
- Linter: `strict_balance_equality` lint - [#1914](https://github.com/paritytech/ink/pull/1914)

## Version 5.0.0-alpha

Expand Down
6 changes: 6 additions & 0 deletions linting/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ path = "ui/pass/storage_never_freed.rs"
[[example]]
name = "storage_never_freed_fail"
path = "ui/fail/storage_never_freed.rs"
[[example]]
name = "strict_balance_equality_pass"
path = "ui/pass/strict_balance_equality.rs"
[[example]]
name = "strict_balance_equality_fail"
path = "ui/fail/strict_balance_equality.rs"

[package.metadata.rust-analyzer]
rustc_private = true
Expand Down
6 changes: 1 addition & 5 deletions linting/src/ink_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use clippy_utils::match_def_path;
use if_chain::if_chain;
use rustc_hir::{
Expand Down Expand Up @@ -51,7 +52,6 @@ pub(crate) fn find_storage_struct(
.copied()
}

// TODO: Extracted from #1914; reuse this in #1914 when it is merged
/// Returns `ItemId`s defined inside the code block of `const _: () = {}`.
///
/// The Rust code expanded after ink! code generation used these to define different
Expand All @@ -77,7 +77,6 @@ fn items_in_unnamed_const(cx: &LateContext<'_>, id: &ItemId) -> Vec<ItemId> {
}
}

// TODO: Extracted from #1914; reuse this in #1914 when it is merged
/// Collect all the `ItemId`s in nested `const _: () = {}`
pub(crate) fn expand_unnamed_consts(
cx: &LateContext<'_>,
Expand All @@ -90,7 +89,6 @@ pub(crate) fn expand_unnamed_consts(
})
}

// TODO: Extracted from #1914; reuse this in #1914 when it is merged
/// Finds type of the struct that implements a contract with user-defined code
fn find_contract_ty_hir<'tcx>(
cx: &LateContext<'tcx>,
Expand All @@ -114,7 +112,6 @@ fn find_contract_ty_hir<'tcx>(
.copied()
}

// TODO: Extracted from #1914; reuse this in #1914 when it is merged
/// Compares types of two user-defined structs
fn eq_hir_struct_tys(lhs: &Ty<'_>, rhs: &Ty<'_>) -> bool {
match (lhs.kind, rhs.kind) {
Expand All @@ -126,7 +123,6 @@ fn eq_hir_struct_tys(lhs: &Ty<'_>, rhs: &Ty<'_>) -> bool {
}
}

// TODO: Extracted from #1914; reuse this in #1914 when it is merged
/// Finds an ID of the implementation of the contract struct containing user-defined code
pub(crate) fn find_contract_impl_id(
cx: &LateContext<'_>,
Expand Down
7 changes: 7 additions & 0 deletions linting/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,24 @@
html_favicon_url = "https://use.ink/crate-docs/favicon.png"
)]
#![feature(rustc_private)]
#![feature(box_patterns)]

dylint_linting::dylint_library!();

extern crate rustc_ast;
extern crate rustc_errors;
extern crate rustc_hir;
extern crate rustc_index;
extern crate rustc_lint;
extern crate rustc_middle;
extern crate rustc_mir_dataflow;
extern crate rustc_session;
extern crate rustc_span;

mod ink_utils;
mod primitive_topic;
mod storage_never_freed;
mod strict_balance_equality;

#[doc(hidden)]
#[no_mangle]
Expand All @@ -41,9 +45,12 @@ pub fn register_lints(
lint_store.register_lints(&[
primitive_topic::PRIMITIVE_TOPIC,
storage_never_freed::STORAGE_NEVER_FREED,
strict_balance_equality::STRICT_BALANCE_EQUALITY,
]);
lint_store.register_late_pass(|_| Box::new(primitive_topic::PrimitiveTopic));
lint_store.register_late_pass(|_| Box::new(storage_never_freed::StorageNeverFreed));
lint_store
.register_late_pass(|_| Box::new(strict_balance_equality::StrictBalanceEquality));
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions linting/src/primitive_topic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use clippy_utils::{
diagnostics::span_lint_and_then,
is_lint_allowed,
Expand Down
1 change: 1 addition & 0 deletions linting/src/storage_never_freed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::ink_utils::{
expand_unnamed_consts,
find_contract_impl_id,
Expand Down
Loading

0 comments on commit 9cf5b3d

Please sign in to comment.