Workaround: Do not inline functions that have pointer args (#4899)

This reverts the inliner change that was done in #4823. Without this, when sharing stack data across contract boundaries, we end up with an illegal access. --------- Co-authored-by: IGI-111 <igi-111@protonmail.com>
FuelLabs · Aug 2, 2023 · 1cb1a46 · 1cb1a46
1 parent 1799db8
commit 1cb1a46
Show file tree

Hide file tree

Showing 10 changed files with 248 additions and 10 deletions.
diff --git a/sway-ir/src/optimize/inline.rs b/sway-ir/src/optimize/inline.rs
@@ -151,6 +151,14 @@ pub fn inline_in_module(
             return true;
         }
 
+        if func.args_iter(ctx).any(|(_name, arg_val)| {
+            arg_val.get_type(ctx).map_or(false, |ty| {
+                ty.is_ptr(ctx) || !(ty.is_unit(ctx) | ty.is_bool(ctx) | ty.is_uint(ctx))
+            })
+        }) {
+            return true;
+        }
+
         false
     };
 

diff --git a/.../e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle.json b/.../e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle.json
@@ -7,7 +7,7 @@
         "typeArguments": null
       },
       "name": "C0",
-      "offset": 4180
+      "offset": 4212
     },
     {
       "configurableType": {
@@ -16,7 +16,7 @@
         "typeArguments": null
       },
       "name": "C1",
-      "offset": 4188
+      "offset": 4220
     },
     {
       "configurableType": {
@@ -25,7 +25,7 @@
         "typeArguments": null
       },
       "name": "C2",
-      "offset": 4204
+      "offset": 4236
     },
     {
       "configurableType": {
@@ -34,7 +34,7 @@
         "typeArguments": []
       },
       "name": "C3",
-      "offset": 4236
+      "offset": 4268
     },
     {
       "configurableType": {
@@ -43,7 +43,7 @@
         "typeArguments": []
       },
       "name": "C4",
-      "offset": 4252
+      "offset": 4284
     },
     {
       "configurableType": {
@@ -52,7 +52,7 @@
         "typeArguments": []
       },
       "name": "C5",
-      "offset": 4268
+      "offset": 4300
     },
     {
       "configurableType": {
@@ -61,7 +61,7 @@
         "typeArguments": null
       },
       "name": "C6",
-      "offset": 4284
+      "offset": 4316
     },
     {
       "configurableType": {
@@ -70,7 +70,7 @@
         "typeArguments": null
       },
       "name": "C7",
-      "offset": 4300
+      "offset": 4332
     },
     {
       "configurableType": {
@@ -79,7 +79,7 @@
         "typeArguments": null
       },
       "name": "C9",
-      "offset": 4348
+      "offset": 4380
     }
   ],
   "functions": [

diff --git a/...ests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw b/...ests/test_programs/should_pass/require_contract_deployment/call_basic_storage/src/main.sw
@@ -2,7 +2,7 @@ script;
 use basic_storage_abi::{BasicStorage, Quad};
 
 fn main() -> u64 {
-    let addr = abi(BasicStorage, 0xceb326912590088f7eb8659b2aaa9cf3519bfdb8e50a85a6e85650b143ab048a);
+    let addr = abi(BasicStorage, 0xf27fbea4cc1d96076e287edbc01670190c5fb5a4a3527286e4f3b01b9314a271);
     let key = 0x0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
     let value = 4242;
 

diff --git a/test/src/sdk-harness/Forc.lock b/test/src/sdk-harness/Forc.lock
@@ -340,6 +340,11 @@ name = 'test_script'
 source = 'member'
 dependencies = ['std']
 
+[[package]]
+name = 'test_script_bytes'
+source = 'member'
+dependencies = ['std']
+
 [[package]]
 name = 'token_ops'
 source = 'member'

diff --git a/test/src/sdk-harness/Forc.toml b/test/src/sdk-harness/Forc.toml
@@ -15,6 +15,7 @@ members = [
   "test_projects/hashing",
   "test_projects/logging",
   "test_projects/low_level_call",
+  "test_projects/low_level_call_bytes",
   "test_projects/messages",
   "test_projects/option_field_order",
   "test_projects/option_in_abi",

diff --git a/test/src/sdk-harness/test_projects/harness.rs b/test/src/sdk-harness/test_projects/harness.rs
@@ -17,6 +17,7 @@ mod generics_in_abi;
 mod hashing;
 mod logging;
 mod low_level_call;
+mod low_level_call_bytes;
 mod messages;
 mod methods;
 mod option_field_order;

diff --git a/test/src/sdk-harness/test_projects/low_level_call_bytes/Forc.lock b/test/src/sdk-harness/test_projects/low_level_call_bytes/Forc.lock
@@ -0,0 +1,13 @@
+[[package]]
+name = 'core'
+source = 'path+from-root-C9E1B0494AB5FDF1'
+
+[[package]]
+name = 'std'
+source = 'path+from-root-C9E1B0494AB5FDF1'
+dependencies = ['core']
+
+[[package]]
+name = 'test_script_bytes'
+source = 'member'
+dependencies = ['std']
diff --git a/test/src/sdk-harness/test_projects/low_level_call_bytes/Forc.toml b/test/src/sdk-harness/test_projects/low_level_call_bytes/Forc.toml
@@ -0,0 +1,8 @@
+[project]
+authors = ["Fuel Labs <contact@fuel.sh>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "test_script_bytes"
+
+[dependencies]
+std = { path = "../../../../../sway-lib-std" }
diff --git a/test/src/sdk-harness/test_projects/low_level_call_bytes/mod.rs b/test/src/sdk-harness/test_projects/low_level_call_bytes/mod.rs
@@ -0,0 +1,182 @@
+use fuel_vm::fuel_tx::{Bytes32, ContractId, Output, TxPointer, UtxoId};
+use fuels::{
+    accounts::wallet::WalletUnlocked,
+    prelude::*,
+    types::{input::Input, Bits256, SizedAsciiString},
+};
+
+macro_rules! fn_selector {
+    ( $fn_name: ident ( $($fn_arg: ty),* )  ) => {
+         ::fuels::core::codec::resolve_fn_selector(stringify!($fn_name), &[$( <$fn_arg as ::fuels::core::traits::Parameterize>::param_type() ),*]).to_vec()
+    }
+}
+macro_rules! calldata {
+    ( $($arg: expr),* ) => {
+        ::fuels::core::codec::ABIEncoder::encode(&[$(::fuels::core::traits::Tokenizable::into_token($arg)),*]).unwrap().resolve(0)
+    }
+}
+
+// Load abi from json
+abigen!(
+    Contract(
+        name = "TestContract",
+        abi = "test_artifacts/low_level_callee_contract/out/debug/test_contract-abi.json"
+    ),
+    Script(
+        name = "TestScript",
+        abi = "test_projects/low_level_call_bytes/out/debug/test_script_bytes-abi.json"
+    )
+);
+
+async fn low_level_call(
+    id: ContractId,
+    wallet: WalletUnlocked,
+    function_selector: Vec<u8>,
+    calldata: Vec<u8>,
+    single_value_type_arg: bool,
+) {
+    // Build the script instance
+    let script_instance = TestScript::new(
+        wallet,
+        "test_projects/low_level_call_bytes/out/debug/test_script_bytes.bin",
+    );
+
+    // Add the contract being called to the inputs and outputs
+    let contract_input = Input::Contract {
+        utxo_id: UtxoId::new(Bytes32::zeroed(), 0),
+        balance_root: Bytes32::zeroed(),
+        state_root: Bytes32::zeroed(),
+        tx_pointer: TxPointer::default(),
+        contract_id: id,
+    };
+
+    let contract_output = Output::Contract {
+        input_index: 0u8,
+        balance_root: Bytes32::zeroed(),
+        state_root: Bytes32::zeroed(),
+    };
+
+    // Run the script which will call the contract
+    let tx = script_instance
+        .main(
+            id,
+            fuels::types::Bytes(function_selector),
+            fuels::types::Bytes(calldata),
+            single_value_type_arg,
+        )
+        .with_inputs(vec![contract_input])
+        .with_outputs(vec![contract_output])
+        .tx_params(TxParameters::default().set_gas_limit(10_000_000));
+
+    tx.call().await.unwrap();
+}
+
+async fn get_contract_instance() -> (TestContract<WalletUnlocked>, ContractId, WalletUnlocked) {
+    // Launch a local network and deploy the contract
+    let mut wallets = launch_custom_provider_and_get_wallets(
+        WalletsConfig::new(
+            Some(1),             /* Single wallet */
+            Some(1),             /* Single coin (UTXO) */
+            Some(1_000_000_000), /* Amount per coin */
+        ),
+        None,
+        None,
+    )
+    .await;
+    let wallet = wallets.pop().unwrap();
+
+    let id = Contract::load_from(
+        "test_artifacts/low_level_callee_contract/out/debug/test_contract.bin",
+        LoadConfiguration::default(),
+    )
+    .unwrap()
+    .deploy(&wallet, TxParameters::default())
+    .await
+    .unwrap();
+
+    let instance = TestContract::new(id.clone(), wallet.clone());
+
+    (instance, id.into(), wallet)
+}
+
+#[tokio::test]
+async fn can_call_with_one_word_arg() {
+    let (instance, id, wallet) = get_contract_instance().await;
+
+    let function_selector = fn_selector!(set_value(u64));
+
+    let calldata = calldata!(42u64);
+
+    // Calling "set_value(u64)" with argument "42" should set the value to 42
+    low_level_call(id, wallet, function_selector, calldata, true).await;
+    let result = instance.methods().get_value().call().await.unwrap().value;
+    assert_eq!(result, 42);
+}
+
+#[tokio::test]
+async fn can_call_with_multi_word_arg() {
+    let (instance, id, wallet) = get_contract_instance().await;
+
+    let function_selector = fn_selector!(set_b256_value(Bits256));
+
+    let calldata = calldata!(Bits256([1u8; 32]));
+
+    low_level_call(id, wallet, function_selector, calldata, false).await;
+    let result = instance
+        .methods()
+        .get_b256_value()
+        .call()
+        .await
+        .unwrap()
+        .value;
+    assert_eq!(result, Bits256([1u8; 32]));
+}
+
+#[tokio::test]
+async fn can_call_with_multiple_args() {
+    let (instance, id, wallet) = get_contract_instance().await;
+
+    let function_selector = fn_selector!(set_value_multiple(u64, u64));
+    let calldata = calldata!(23u64, 42u64);
+
+    low_level_call(id, wallet, function_selector, calldata, false).await;
+    let result = instance.methods().get_value().call().await.unwrap().value;
+    assert_eq!(result, 23 + 42);
+}
+
+#[tokio::test]
+async fn can_call_with_multiple_args_complex() {
+    let (instance, id, wallet) = get_contract_instance().await;
+
+    let function_selector =
+        fn_selector!(set_value_multiple_complex(MyStruct, SizedAsciiString::<4>));
+    let calldata = calldata!(
+        MyStruct {
+            a: true,
+            b: [1, 2, 3],
+        },
+        SizedAsciiString::<4>::try_from("fuel").unwrap()
+    );
+
+    low_level_call(id, wallet, function_selector, calldata, false).await;
+
+    let result_uint = instance.methods().get_value().call().await.unwrap().value;
+    let result_bool = instance
+        .methods()
+        .get_bool_value()
+        .call()
+        .await
+        .unwrap()
+        .value;
+    let result_str = instance
+        .methods()
+        .get_str_value()
+        .call()
+        .await
+        .unwrap()
+        .value;
+
+    assert_eq!(result_uint, 2);
+    assert!(result_bool);
+    assert_eq!(result_str, "fuel");
+}
diff --git a/test/src/sdk-harness/test_projects/low_level_call_bytes/src/main.sw b/test/src/sdk-harness/test_projects/low_level_call_bytes/src/main.sw
@@ -0,0 +1,20 @@
+script;
+
+use std::bytes::Bytes;
+use std::constants::BASE_ASSET_ID;
+use std::low_level_call::{call_with_function_selector, call_with_function_selector_vec, CallParams};
+
+fn main(
+    target: ContractId,
+    function_selector: Bytes,
+    calldata: Bytes,
+    single_value_type_arg: bool,
+) {
+    let call_params = CallParams {
+        coins: 0,
+        asset_id: BASE_ASSET_ID,
+        gas: 10_000_000,
+    };
+
+    call_with_function_selector(target, function_selector, calldata, single_value_type_arg, call_params);
+}