Skip to content

Commit

Permalink
Linter: Warn when storage can never be freed up (#1932)
Browse files Browse the repository at this point in the history
* feat(lint+test): add skeleton of storage_never_freed (#1431)

* feat(lint): add utils and logic to collect `Vec`/`Mapping` types

It reuses functions implemented in #1914, so we need to rebase and
modify it when it is merged.

* feat(lint): Make it pass simple tests

* feat(lint): Support index operations (`vec[]`)

* feat(lint): Ignore fields that has mutable unsafe pointers

* chore(lint): Refactor

* feat(test): Inserting in method arg

* chore(test): Refactor

* feat(test): Insert inside insert

* chore(lint): Refactor

* feat(lint): Support local type aliases

* chore(lint): More accurate warning text

* chore(lint): Refactor

* feat(lint): Additional check for `self`

* chore(lint+tests): Refactor; update tests output

* chore: Update changelog

* chore: fmt
  • Loading branch information
jubnzv authored Oct 5, 2023
1 parent 9a5c212 commit 1b84f8a
Show file tree
Hide file tree
Showing 8 changed files with 680 additions and 2 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## Changed
### Changed
- Make `set_code_hash` generic - [#1906](https://github.com/paritytech/ink/pull/1906)
- Clean E2E configuration parsing - [#1922](https://github.com/paritytech/ink/pull/1922)

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

## Version 5.0.0-alpha

The preview release of the ink! 5.0.0 release.
Expand Down
6 changes: 6 additions & 0 deletions linting/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ path = "ui/pass/primitive_topic.rs"
[[example]]
name = "primitive_topic_fail"
path = "ui/fail/primitive_topic.rs"
[[example]]
name = "storage_never_freed_pass"
path = "ui/pass/storage_never_freed.rs"
[[example]]
name = "storage_never_freed_fail"
path = "ui/fail/storage_never_freed.rs"

[package.metadata.rust-analyzer]
rustc_private = true
Expand Down
148 changes: 148 additions & 0 deletions linting/src/ink_utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
// Copyright (C) Parity Technologies (UK) Ltd.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// 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::{
ExprKind,
HirId,
ItemId,
ItemKind,
QPath,
StmtKind,
Ty,
TyKind,
};
use rustc_lint::LateContext;

/// Returns `true` iff the ink storage attribute is defined for the given HIR
fn has_storage_attr(cx: &LateContext, hir: HirId) -> bool {
const INK_STORAGE: &str = "__ink_dylint_Storage";
let attrs = format!("{:?}", cx.tcx.hir().attrs(hir));
attrs.contains(INK_STORAGE)
}

/// Returns `ItemId` of the structure annotated with `#[ink(storage)]`
pub(crate) fn find_storage_struct(
cx: &LateContext,
item_ids: &[ItemId],
) -> Option<ItemId> {
item_ids
.iter()
.find(|&item_id| {
let item = cx.tcx.hir().item(*item_id);
if_chain! {
if has_storage_attr(cx, item.hir_id());
if let ItemKind::Struct(..) = item.kind;
then { true } else { false }

}
})
.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
/// implementations of a contract.
fn items_in_unnamed_const(cx: &LateContext<'_>, id: &ItemId) -> Vec<ItemId> {
if_chain! {
if let ItemKind::Const(ty, body_id) = cx.tcx.hir().item(*id).kind;
if let TyKind::Tup([]) = ty.kind;
let body = cx.tcx.hir().body(body_id);
if let ExprKind::Block(block, _) = body.value.kind;
then {
block.stmts.iter().fold(Vec::new(), |mut acc, stmt| {
if let StmtKind::Item(id) = stmt.kind {
// We don't call `items_in_unnamed_const` here recursively, because the source
// code generated by ink! doesn't have nested `const _: () = {}` expressions.
acc.push(id)
}
acc
})
} else {
vec![]
}
}
}

// 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<'_>,
item_ids: &[ItemId],
) -> Vec<ItemId> {
item_ids.iter().fold(Vec::new(), |mut acc, item_id| {
acc.push(*item_id);
acc.append(&mut items_in_unnamed_const(cx, item_id));
acc
})
}

// 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>,
item_ids: &[ItemId],
) -> Option<&'tcx Ty<'tcx>> {
item_ids
.iter()
.find_map(|item_id| {
if_chain! {
let item = cx.tcx.hir().item(*item_id);
if let ItemKind::Impl(item_impl) = &item.kind;
if let Some(trait_ref) = cx.tcx.impl_trait_ref(item.owner_id);
if match_def_path(
cx,
trait_ref.skip_binder().def_id,
&["ink_env", "contract", "ContractEnv"],
);
then { Some(&item_impl.self_ty) } else { None }
}
})
.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) {
(
TyKind::Path(QPath::Resolved(_, lhs_path)),
TyKind::Path(QPath::Resolved(_, rhs_path)),
) => lhs_path.res.eq(&rhs_path.res),
_ => false,
}
}

// 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<'_>,
item_ids: Vec<ItemId>,
) -> Option<ItemId> {
let contract_struct_ty = find_contract_ty_hir(cx, &item_ids)?;
item_ids
.iter()
.find(|item_id| {
if_chain! {
let item = cx.tcx.hir().item(**item_id);
if let ItemKind::Impl(item_impl) = &item.kind;
if item_impl.of_trait.is_none();
if eq_hir_struct_tys(contract_struct_ty, item_impl.self_ty);
then { true } else { false }
}
})
.copied()
}
8 changes: 7 additions & 1 deletion linting/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,22 @@ extern crate rustc_middle;
extern crate rustc_session;
extern crate rustc_span;

mod ink_utils;
mod primitive_topic;
mod storage_never_freed;

#[doc(hidden)]
#[no_mangle]
pub fn register_lints(
_sess: &rustc_session::Session,
lint_store: &mut rustc_lint::LintStore,
) {
lint_store.register_lints(&[primitive_topic::PRIMITIVE_TOPIC]);
lint_store.register_lints(&[
primitive_topic::PRIMITIVE_TOPIC,
storage_never_freed::STORAGE_NEVER_FREED,
]);
lint_store.register_late_pass(|_| Box::new(primitive_topic::PrimitiveTopic));
lint_store.register_late_pass(|_| Box::new(storage_never_freed::StorageNeverFreed));
}

#[test]
Expand Down
Loading

0 comments on commit 1b84f8a

Please sign in to comment.