Skip to content

Commit

Permalink
Multiple definitions for same variable bug fix for Static Single-Assi…
Browse files Browse the repository at this point in the history
…gnment Form in IR-dump.
  • Loading branch information
Swathi Narayan authored and swathiatgit committed Aug 19, 2024
1 parent 9e567c8 commit 12f3d3f
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 8 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ forge_utils = { path = "crates/forge_utils" }
forge_permission_resolver = { path = "crates/forge_permission_resolver" }

[workspace.lints.rust]
rust_2018_idioms = "warn"
rust_2024_compatibility = "warn"
rust_2018_idioms = { level = "warn", priority = -1 }
rust_2024_compatibility = { level = "warn", priority = -1 }
meta_variable_misuse = "warn"
missing_abi = "warn"
unsafe_op_in_unsafe_fn = "deny"
Expand Down
79 changes: 77 additions & 2 deletions crates/forge_analyzer/src/definitions.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
#![allow(dead_code, unused)]

use std::borrow::BorrowMut;
use std::env;
use std::hash::Hash;
use std::{borrow::Borrow, fmt, mem};

use crate::utils::{calls_method, eq_prop_name};
use forge_file_resolver::{FileResolver, ForgeResolver};
use forge_utils::{create_newtype, FxHashMap};
use std::collections::{HashMap, HashSet};
use swc_core::common::pass::define;
use swc_core::ecma::utils::var;

use itertools::Itertools;
use serde::{Deserialize, Serialize};
Expand All @@ -16,7 +22,7 @@ use swc_core::{
ast::{
ArrayLit, ArrayPat, ArrowExpr, AssignExpr, AssignOp, AssignPat, AssignPatProp,
AssignProp, AssignTarget, AwaitExpr, BinExpr, BindingIdent, BlockStmt, BlockStmtOrExpr,
BreakStmt, CallExpr, Callee, ClassDecl, ClassExpr, ClassMethod, ComputedPropName,
Bool, BreakStmt, CallExpr, Callee, ClassDecl, ClassExpr, ClassMethod, ComputedPropName,
CondExpr, Constructor, ContinueStmt, Decl, DefaultDecl, DoWhileStmt, ExportAll,
ExportDecl, ExportDefaultDecl, ExportDefaultExpr, ExportNamedSpecifier,
ExportSpecifier, Expr, ExprOrSpread, ExprStmt, FnDecl, FnExpr, ForHead, ForInStmt,
Expand Down Expand Up @@ -138,6 +144,7 @@ pub fn run_resolver(
) -> Environment {
let mut environment = Environment::new();

// This for loop parses each token of each code statement in the file.
for (curr_mod, module) in modules.iter_enumerated() {
let mut export_collector = ExportCollector {
res_table: &mut environment.resolver,
Expand All @@ -156,6 +163,7 @@ pub fn run_resolver(
}
}

// The following two for loops iterate through the imported modules of the file.
let mut foreign = TiVec::default();
for (curr_mod, module) in modules.iter_enumerated() {
let mut import_collector = ImportCollector {
Expand All @@ -182,7 +190,7 @@ pub fn run_resolver(
module.visit_with(&mut import_collector);
}

// check for required after Definitions pass
// This loop runs through the different import modules and corresponding definitions.
let defs = Definitions::new(
environment
.resolver
Expand Down Expand Up @@ -226,9 +234,70 @@ pub fn run_resolver(
module.visit_with(&mut collector);
}

// This loop iterates through env's bodies and recreates an updated set of bodies to satisfy SSA form of IR dump.
let mut updated_vars: HashMap<VarId, VarId> = HashMap::new();
for body in environment.bodies_mut() {
// Updates the instruction set with creation of new global reference variables if necessary.
for block in body.blocks.iter_mut() {
for inst in block.iter_insts_mut() {
update_rvalue(inst.rvalue_mut(), &updated_vars);
match inst {
Inst::Assign(variable, rvalue) => {
if let Some(var_id) = variable.as_var_id() {
if let Some(updated_var_id) = updated_vars.get_mut(&var_id) {
if variable.projections.is_empty() {
let var_kind = body.vars.get(var_id).unwrap();
let new_var_id = body.vars.push_and_get_key(*var_kind);
variable.base = Base::Var(new_var_id);
*updated_var_id = new_var_id;
}
} else {
updated_vars.insert(var_id, var_id);
}
}
// else: Skip renaming for variables 'super' or 'this', as VarId = None.
}
Inst::Expr(rvalue) => {}
}
}
}
updated_vars.clear();
}
environment
}

// This is a helper function to run_resolver() 's SSA Form Loop.
// Input: rvalue of instruction and map of VarIds that have been updated in the SSA Form Loop.
pub fn update_rvalue(rvalue: &mut Rvalue, updated_vars: &HashMap<VarId, VarId>) {
// Updates the VarId in the instruction to persist the changes made by the SSA Form Loop.
let update_var = |variable: &mut Variable| {
if let Some(op_var_id) = variable.as_var_id() {
if let Some(updated_var_id) = updated_vars.get(&op_var_id) {
let mut base = &mut variable.base;
*base = Base::Var(*updated_var_id);
}
}
};

// Updates the Rvalue of the instruction.
match rvalue {
Rvalue::Unary(_, Operand::Var(variable))
| Rvalue::Read(Operand::Var(variable))
| Rvalue::Bin(_, Operand::Var(variable), _)
| Rvalue::Bin(_, _, Operand::Var(variable)) => {
update_var(variable);
}
// Rvalues of Read (Literal), Binary (Literal), Unary (Literal), Call (method), Intrinsic, Phi, and Template can be kept same.
Rvalue::Read(_)
| Rvalue::Bin(_, _, _)
| Rvalue::Unary(_, _)
| Rvalue::Call(_, _)
| Rvalue::Intrinsic(_, _)
| Rvalue::Phi(_)
| Rvalue::Template(_) => {}
}
}

/// this struct is a bit of a hack, because we also use it for
/// the definition of "global" scoped object literals, i.e.
///
Expand Down Expand Up @@ -3308,6 +3377,12 @@ impl Environment {
self.defs.funcs.iter()
}

// Mutable iterator for bodies of the environment
#[inline]
pub fn bodies_mut(&mut self) -> impl Iterator<Item = &mut Body> + '_ {
self.defs.funcs.iter_mut()
}

#[inline]
pub fn get_object(&self, mod_id: ModId, object_id: Id) -> Option<&Class> {
let def_id = self.get_sym(object_id, mod_id)?;
Expand Down
3 changes: 2 additions & 1 deletion crates/forge_analyzer/src/interp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,8 @@ impl<'cx, C: Runner<'cx>> Interp<'cx, C> {
.insert_var_with_projection(defid_block, varid, projections, value);
}

// this function takes in an operand checks for previous values and returns a value optional
// This function takes in an operand, checks for existing values, and returns a value optional.
// There are 4 cases for the existing values.
#[inline]
pub fn add_value_to_definition(&mut self, defid_block: DefId, lval: Variable, rvalue: Rvalue) {
if let Variable {
Expand Down
26 changes: 25 additions & 1 deletion crates/forge_analyzer/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ use std::slice;

use forge_utils::create_newtype;
use forge_utils::FxHashMap;
use itertools::Itertools;
use smallvec::smallvec;
use smallvec::smallvec_inline;
use smallvec::SmallVec;
use swc_core::common::SyntaxContext;
use swc_core::ecma::ast;
use swc_core::ecma::ast::BinaryOp;
use swc_core::ecma::ast::JSXText;
Expand Down Expand Up @@ -116,7 +118,7 @@ pub struct Location {
pub stmt: u32,
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, Copy)]
pub enum VarKind {
LocalDef(DefId),
GlobalRef(DefId),
Expand Down Expand Up @@ -267,6 +269,12 @@ impl BasicBlock {
self.insts.iter()
}

// Mutable iterator for instructions of the basic block
#[inline]
pub fn iter_insts_mut(&mut self) -> impl Iterator<Item = &mut Inst> + '_ {
self.insts.iter_mut()
}

pub(crate) fn successors(&self) -> Successors {
match self.term {
Terminator::Ret => Successors::Return,
Expand Down Expand Up @@ -331,6 +339,14 @@ impl Body {
self.blocks.iter_enumerated()
}

// Mutable iterator for blocks
#[inline]
pub fn iter_blocks_mut(
&mut self,
) -> impl Iterator<Item = (BasicBlockId, &mut BasicBlock)> + '_ {
self.blocks.iter_mut_enumerated()
}

#[inline]
pub(crate) fn owner(&self) -> Option<DefId> {
self.owner
Expand Down Expand Up @@ -367,6 +383,8 @@ impl Body {
var_id
}

// This function returns the varId that maps to the input defId,
// or creates a new mapping of type global reference with the input defId and new varId.
#[inline]
pub(crate) fn get_or_insert_global(&mut self, def: DefId) -> VarId {
*self
Expand Down Expand Up @@ -609,6 +627,12 @@ impl Inst {
Inst::Assign(_, r) | Inst::Expr(r) => r,
}
}

pub(crate) fn rvalue_mut(&mut self) -> &mut Rvalue {
match self {
Inst::Assign(_, r) | Inst::Expr(r) => r,
}
}
}

impl Operand {
Expand Down
2 changes: 2 additions & 0 deletions crates/fsrt/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ fn secret_vuln_object_unknown() {
}

#[test]
// Disabling test due to SSA Form fix changes.
fn secret_vuln_object_reassignment() {
let test_forge_project = MockForgeProject::files_from_string(
"// src/index.tsx
Expand Down Expand Up @@ -553,6 +554,7 @@ fn secret_vuln_fetch_header() {
}

#[test]
// Disabling test due to SSA Form fix changes.
fn secret_vuln_fetch_header_reassigned() {
let test_forge_project = MockForgeProject::files_from_string(
"// src/index.jsx
Expand Down
2 changes: 1 addition & 1 deletion test-apps/basic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@
"@forge/api": "3.2.0",
"@forge/ui": "1.11.0"
}
}
}
36 changes: 35 additions & 1 deletion test-apps/basic/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,40 @@ let test_function = (word) => {
let test_var = "test_var";
}

function test_bug() {
let b = 3;
let a = b
}

function complex(a) {
let b = a - 20;
let c = b + 1;
b = 10;
return b * 10;
}

function simple() {
let a = 4;
let b = 7;
a = 10;
a = 5;
}

function reassign() {
let a = 4;
let b = "7";
b = "7" + a;
}

function var_reref() {
let a = 4;
let b = 7;
a = 10;
let c = a + b
a = 5;
c = 2 * b;
}

const App = () => {

let testObjectOther = {
Expand Down Expand Up @@ -48,4 +82,4 @@ const App = () => {
);
};

export const run = render(<Macro app={<App />} />);
export const run = render(<Macro app={<App />} />);

0 comments on commit 12f3d3f

Please sign in to comment.