aptos-labs / aptos-core

Aptos is a layer 1 blockchain built to support the widespread use of blockchain through better technology and user experience.
https://aptosfoundation.org
Other
5.89k stars 3.57k forks source link

[Bug][move-compiler-v2] Spurious Unused assignment warnings with inlining in certain tests #13880

Open brmataptos opened 5 days ago

brmataptos commented 5 days ago

๐Ÿ› Bug

I've only seen these messages in some big runs, where V2 is used to pre-compile libraries for aptos-transactional-test-harness (for V2 tests).

% UB=1 cargo test --profile ci -p move-compiler -p move-compiler-v2 -p move-compiler-v2-transactional-tests -p move-compiler-transactional-tests -p aptos-transactional-test-harness |& tee test.out2
...

test runner::aptos_test_harness/randomness_safety.move                 ... ok
warning: Unused assignment to `to`. Consider removing or prefixing with an underscore: `_to`
    โ”Œโ”€ /Users/brm/code/aptos-core/aptos-move/framework/aptos-framework/sources/object.move:544:16
    โ”‚
544 โ”‚     inline fun transfer_raw_inner(object: address, to: address) acquires ObjectCore {
    โ”‚                ^^^^^^^^^^^^^^^^^^
    ยท
621 โ”‚         transfer_raw_inner(object_addr, BURN_ADDRESS);
    โ”‚         --------------------------------------------- from a call inlined at this callsite

warning: Unused assignment to `c`. Consider removing or prefixing with an underscore: `_c`
     โ”Œโ”€ /Users/brm/code/aptos-core/aptos-move/framework/aptos-stdlib/sources/math64.move:41:23
     โ”‚
  41 โ”‚     public inline fun mul_div(a: u64, b: u64, c: u64): u64 {
     โ”‚                       ^^^^^^^
     โ”‚
     โ”Œโ”€ /Users/brm/code/aptos-core/aptos-move/framework/aptos-framework/sources/delegation_pool.move:2000:13
     โ”‚
2000 โ”‚             math64::mul_div(active - pool_active, pool.operator_commission_percentage, MAX_FEE)
     โ”‚             ----------------------------------------------------------------------------------- from a call inlined at this callsite

warning: Unused assignment to `c`. Consider removing or prefixing with an underscore: `_c`
     โ”Œโ”€ /Users/brm/code/aptos-core/aptos-move/framework/aptos-stdlib/sources/math64.move:41:23
     โ”‚
  41 โ”‚     public inline fun mul_div(a: u64, b: u64, c: u64): u64 {
     โ”‚                       ^^^^^^^
     โ”‚
     โ”Œโ”€ /Users/brm/code/aptos-core/aptos-move/framework/aptos-framework/sources/delegation_pool.move:2008:13
     โ”‚  
2008 โ”‚ โ•ญ             math64::mul_div(
2009 โ”‚ โ”‚                 pending_inactive - pool_pending_inactive,
2010 โ”‚ โ”‚                 pool.operator_commission_percentage,
2011 โ”‚ โ”‚                 MAX_FEE
2012 โ”‚ โ”‚             )
     โ”‚ โ•ฐโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€' from a call inlined at this callsite

The code currently looks like:

module aptos_framework::object {
   ...
    inline fun transfer_raw_inner(object: address, to: address) acquires ObjectCore {
        let object_core = borrow_global_mut<ObjectCore>(object);
        if (object_core.owner != to) {
            if (std::features::module_event_migration_enabled()) {
                event::emit(
                    Transfer {
                        object,
                        from: object_core.owner,
                        to,
                    },
                );
            };
            event::emit_event(
                &mut object_core.transfer_events,
                TransferEvent {
                    object,
                    from: object_core.owner,
                    to,
                },
            );
            object_core.owner = to;
        };
    }
    ...
    public entry fun burn<T: key>(owner: &signer, object: Object<T>) acquires ObjectCore {
        let original_owner = signer::address_of(owner);
        assert!(is_owner(object, original_owner), error::permission_denied(ENOT_OBJECT_OWNER));
        let object_addr = object.inner;
        move_to(&create_signer(object_addr), TombStone { original_owner });
        transfer_raw_inner(object_addr, BURN_ADDRESS);
    }
    ...
brmataptos commented 3 days ago

You can reproduce the test output along with useful information for debugging with

MVC_LOG=trace MVC_BACKTRACE=1 RUST_BACKTRACE=1 UB=1 cargo test --profile ci -p move-compiler -p move-compiler-v2 -p move-compiler-v2-transactional-tests -p move-compiler-transactional-tests -p aptos-transactional-test-harness >& test.out25

That produces a 583MB file, so don't load it in an editor. Load it in less, and search (/) first for Unused assignment, showing something like:

ESC[0mESC[1mESC[38;5;11mwarningESC[0mESC[1m: Unused assignment to `to`. Consider removing or prefixing with an underscore: `_to`
Backtrace: Backtrace [
    { fn: "std::backtrace_rs::backtrace::libunwind::trace", file: "/rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/std/src/../../backtrace/src/backtrace/libunwind.rs", line: 105 },
    { fn: "std::backtrace_rs::backtrace::trace_unsynchronized", file: "/rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/std/src/../../backtrace/src/backtrace/mod.rs", line: 66 },

Now, search backwards (?) for object\.move:621:

 17: move_to<object::TombStone>($t9, $t11)
     # at /Users/brm/code/aptos-core/aptos-move/framework/aptos-framework/sources/object.move:544:16+18
     # live vars: $t6
 18: $t12 := move($t6)
     # at /Users/brm/code/aptos-core/aptos-move/framework/aptos-framework/sources/object.move:621:41+12
     # live vars: $t12
 19: $t14 := 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
     # at /Users/brm/code/aptos-core/aptos-move/framework/aptos-framework/sources/object.move:544:16+18
     # live vars: $t12, $t14
 20: $t13 := move($t14)
     # at /Users/brm/code/aptos-core/aptos-move/framework/aptos-framework/sources/object.move:545:27+37
     # live vars: $t12

Note that $t12 is assigned and is live at instruction 18. Why is it being flagged as unused?

brmataptos commented 3 days ago

I've managed to reproduce the test failure with a slightly smaller test output:

MVC_LOG=trace MVC_BACKTRACE=1 RUST_BACKTRACE=1 UB=1 cargo test --profile ci -p aptos-transactional-test-harness v2-tests/smoke_test >& test.out26

This time it's only 210MB. Still too big for an editor.

brmataptos commented 3 days ago

I added a simpler test case in third_party/move/move-compiler-v2/tests/unused-assignment/object_test.move on branch https://github.com/aptos-labs/aptos-core/tree/brm-issue-13880. Enjoy.