Skip to content

Commit

Permalink
implement rules
Browse files Browse the repository at this point in the history
  • Loading branch information
jamedzung committed Mar 22, 2024
1 parent 0b20942 commit 008aaa8
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 4 deletions.
28 changes: 24 additions & 4 deletions external-crates/move/crates/move-compiler/src/linters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@

use move_symbol_pool::Symbol;

use crate::{command_line::compiler::Visitor, diagnostics::codes::WarningFilter};

use crate::{
command_line::compiler::Visitor, diagnostics::codes::WarningFilter,
linters::shift_overflow::ShiftOperationOverflow, typing::visitor::TypingVisitor,
};
pub mod shift_overflow;
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum LintLevel {
// No linters
Expand All @@ -17,16 +20,33 @@ pub enum LintLevel {

pub const ALLOW_ATTR_CATEGORY: &str = "lint";
pub const LINT_WARNING_PREFIX: &str = "Lint ";
pub const SHILF_OVERFLOW_FILTER_NAME: &str = "shift_overflow";

pub const LINTER_DEFAULT_DIAG_CODE: u8 = 1;

pub enum LinterDiagCategory {
ShiftOperationOverflow,
}

pub fn known_filters() -> (Option<Symbol>, Vec<WarningFilter>) {
(Some(ALLOW_ATTR_CATEGORY.into()), vec![])
(
Some(ALLOW_ATTR_CATEGORY.into()),
vec![WarningFilter::code(
Some(LINT_WARNING_PREFIX),
LinterDiagCategory::ShiftOperationOverflow as u8,
LINTER_DEFAULT_DIAG_CODE,
Some(SHILF_OVERFLOW_FILTER_NAME),
)],
)
}

pub fn linter_visitors(level: LintLevel) -> Vec<Visitor> {
match level {
LintLevel::None => vec![],
LintLevel::Default | LintLevel::All => {
vec![]
vec![shift_overflow::ShiftOperationOverflow::visitor(
ShiftOperationOverflow,
)]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
//! Detect potential overflow scenarios where the number of bits being shifted exceeds the bit width of
//! the variable being shifted, which could lead to unintended behavior or loss of data. If such a
//! potential overflow is detected, a warning is generated to alert the developer.
use crate::{
diag,
diagnostics::{
codes::{custom, DiagnosticInfo, Severity},
WarningFilters,
},
expansion::ast::Value_,
naming::ast::{BuiltinTypeName_, TypeName_, Type_},
parser::ast::BinOp_,
shared::{program_info::TypingProgramInfo, CompilationEnv},
typing::{
ast::{self as T, UnannotatedExp_},
visitor::{TypingVisitorConstructor, TypingVisitorContext},
},
};
use move_ir_types::location::Loc;
use std::str::FromStr;

use super::{LinterDiagCategory, LINTER_DEFAULT_DIAG_CODE, LINT_WARNING_PREFIX};

const SHIFT_OPERATION_OVERFLOW_DIAG: DiagnosticInfo = custom(
LINT_WARNING_PREFIX,
Severity::Warning,
LinterDiagCategory::ShiftOperationOverflow as u8,
LINTER_DEFAULT_DIAG_CODE,
"Potential overflow detected. The number of bits being shifted exceeds the bit width of the variable being shifted.",
);

pub struct ShiftOperationOverflow;

pub struct Context<'a> {
env: &'a mut CompilationEnv,
}

impl TypingVisitorConstructor for ShiftOperationOverflow {
type Context<'a> = Context<'a>;

fn context<'a>(
env: &'a mut CompilationEnv,
_program_info: &'a TypingProgramInfo,
_program: &T::Program_,
) -> Self::Context<'a> {
Context { env }
}
}

impl TypingVisitorContext for Context<'_> {
fn visit_exp_custom(&mut self, exp: &mut T::Exp) -> bool {
// Check if the expression is a binary operation and if it is a shift operation.
if let UnannotatedExp_::BinopExp(lhs, op, _, rhs) = &exp.exp.value {
// can't do let UnannotatedExp_::BinopExp(lhs, BinOp_::Shl | BinOp_::Shr, _, rhs) = &exp.exp.value else { return };
// because the op is Spanned<BinOp_> and not BinOp_
if matches!(op.value, BinOp_::Shl | BinOp_::Shr) {
match (
get_bit_width(&lhs.ty.value),
get_shift_amount(&rhs.exp.value),
) {
(Some(bit_width), Some(shift_amount)) if shift_amount >= bit_width => {
report_overflow(self.env, shift_amount, bit_width, op.loc);
}
_ => (),
}
}
}
false
}
fn add_warning_filter_scope(&mut self, filter: WarningFilters) {
self.env.add_warning_filter_scope(filter)
}

fn pop_warning_filter_scope(&mut self) {
self.env.pop_warning_filter_scope()
}
}

fn get_bit_width(ty: &Type_) -> Option<u128> {
ty.builtin_name().and_then(|typ| match typ.value {
BuiltinTypeName_::U8 => Some(8),
BuiltinTypeName_::U16 => Some(16),
BuiltinTypeName_::U32 => Some(32),
BuiltinTypeName_::U64 => Some(64),
BuiltinTypeName_::U128 => Some(128),
BuiltinTypeName_::U256 => Some(256),
_ => None,
})
}

fn get_shift_amount(value: &UnannotatedExp_) -> Option<u128> {
if let UnannotatedExp_::Value(v) = value {
match &v.value {
Value_::U8(v) => Some(*v as u128),
_ => None,
}
} else {
None
}
}

fn report_overflow(env: &mut CompilationEnv, shift_amount: u128, bit_width: u128, loc: Loc) {
let msg = format!(
"The {} of bits being shifted exceeds the {} bit width of the variable being shifted.",
shift_amount, bit_width
);
let diag = diag!(SHIFT_OPERATION_OVERFLOW_DIAG, (loc, msg));
env.add_diag(diag);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
warning[Lint W00001]: Potential overflow detected. The number of bits being shifted exceeds the bit width of the variable being shifted.
┌─ tests/custom_rules/shift_overflow.move:5:20
5 │ let _b = x << 64; // Should not raise an issue
│ ^^ The 64 of bits being shifted exceeds the 64 bit width of the variable being shifted.
= This warning can be suppressed with '#[allow(lint(shift_overflow))]' applied to the 'module' or module member ('const', 'fun', or 'struct')

warning[Lint W00001]: Potential overflow detected. The number of bits being shifted exceeds the bit width of the variable being shifted.
┌─ tests/custom_rules/shift_overflow.move:6:20
6 │ let _b = x << 65; // Should raise an issue
│ ^^ The 65 of bits being shifted exceeds the 64 bit width of the variable being shifted.
= This warning can be suppressed with '#[allow(lint(shift_overflow))]' applied to the 'module' or module member ('const', 'fun', or 'struct')

warning[Lint W00001]: Potential overflow detected. The number of bits being shifted exceeds the bit width of the variable being shifted.
┌─ tests/custom_rules/shift_overflow.move:7:20
7 │ let _b = x >> 66; // Should raise an issue
│ ^^ The 66 of bits being shifted exceeds the 64 bit width of the variable being shifted.
= This warning can be suppressed with '#[allow(lint(shift_overflow))]' applied to the 'module' or module member ('const', 'fun', or 'struct')

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module 0x42::M {

fun func1(x: u64) {
let _b = x << 24;
let _b = x << 64; // Should raise an issue
let _b = x << 65; // Should raise an issue
let _b = x >> 66; // Should raise an issue
}
}

0 comments on commit 008aaa8

Please sign in to comment.