coverage work in progress

[brmataptos]

Description

• Change aptos move coverage to work better for red-black-map test case.

• Currently includes voluminous debugging output to help resolve remaining iussues.

• Also adds --show-locs flag to aptos move disassemble command to allow showing
  locations for compiler debugging (in a somewhat hard-to use form, unfortunately).

• Also add more logging of Loc information if MVC_LOG=debug to facilitate debugging
  of remaining location tracking probems.

Details of changes to aptos move coverage to work better for red-black-map test case:

• Track both covered and uncovered locations in source coverage,
  and try to more carefully handle inlined code. Notably, when
  computing coverage for a module, we search for code in other modules
  that may correspond to this module (due to inlining). To better
  handle instructions with imprecise code attribution, we consider
  the source corresponding to an instruction to be the minimal source
  set which doesn't overlap others.
• Notably:

        // Some locations contain others, but should not subsume them, e.g.:
        //     31:  if (p) {
        //     32:     ...
        //     33:  } else {
        //     34:     ...
        //     35   }
        // May have 3 corresponding bytecodes, with locations:
        //     L1: 31-35
        //     L2: 32
        //     L3: 34
        // If L1 is covered but L3 is not, then we want to subtract
        // L3 from the covered region.
        //
        // OTOH, we may see L3 in multiple runs, once covered and once
        // not.  So we need to
        //   (1) subtract identical covered locations from uncovered locations
        //   (2) subtract uncovered locations from any covered locations they overlap
        //   (3) subtract covered locations from any uncovered lcoations they overlap

How Has This Been Tested?

Run in a code directory, with a source module such as such as red_black_map in a file with the same name:

MVC_EXP=peephole-optimization=off MVC_DEBUG=1 MVC_LOG=debug aptos move test --coverage --move-2 --dev >& test.out
RUST_BACKTRACE=1 aptos move coverage source --color always --module red_black_map --dev --move-2 >& coverage.out
aptos move disassemble --show-locs --bytecode-path build/RedBlackMap/bytecode_modules/red_black_map.mv

The three output files may be useful:

• test.out shows the compile debug log. Note that we are disabling peephole optimizations to avoid complication here.
• coverage.out shows the newly corrected test coverage output, which is correct w.r.t. the code attributions,
  but those are incorrect. There is currently a lot of extra debugigng output, so you will want to skip to the end
  of the file and scroll backwards (using less -R coverage.out to see colors)
• build/RedBlackMap/bytecode_modules/red_black_map.mv.asm shows the code attribution as file_hash8:offset1-offset2
  at the end of each instruction, using offsets from beginning of the source file.

Key Areas to Review

The coverage output looks reasonable, but the assembly listing showing
locs reveals that there is a problem with bytecode attribution to
source code.

Notably, if you look at the red_black_map.mv.asm output, the basic block
which corresponds to source code around line 90-92 is clearly misattributed.
The source code in red_black_map.move is the final 3 lines shown here (the first line is a call which is inlined):

88:               // Case_I6
89:               self.rotate(grandparent_index, 1 - child_direction_of_parent);
90:               self.nodes[parent_index].color = Color::Black;
91:               self.nodes[grandparent_index].color = Color::Red;
92:               return;

These lines are at lines 90-92, character offset 2769-2921, in red_black_map.move.

The corresponding code can be found in the assembly code by searching
for PackVariant calls for Color/Black and Color/Red in close
succession in the same block, followed by a Ret. I see it as block
B34 in my listing:

% less build/RedBlackMap/bytecode_modules/red_black_map.mv.asm
...
          405: MoveLoc[16]($t38: &mut u64)                                      2eeec0b7:5160-5170
          406: WriteRef                                                         2eeec0b7:5155-5189
B34:
          407: PackVariant[1](Color/Black)                                      2eeec0b7:5155-5189
          408: CopyLoc[0](self: &mut Map<V>)                                    2eeec0b7:5155-5189
          409: MutBorrowFieldGeneric[1](Map.nodes: vector<Node<V>>)             2eeec0b7:5260-5272
          410: MoveLoc[11](parent_index: u64)                                   2eeec0b7:5260-5272
          411: VecMutBorrow(3)                                                  2eeec0b7:5276-5304
          412: MutBorrowFieldGeneric[4](Node.color: Color)                      2eeec0b7:5276-5304
          413: WriteRef                                                         2eeec0b7:5305-5310
          414: PackVariant[0](Color/Red)                                        2eeec0b7:5276-5311
          415: MoveLoc[0](self: &mut Map<V>)                                    2eeec0b7:5276-5311
          416: MutBorrowFieldGeneric[1](Map.nodes: vector<Node<V>>)             2eeec0b7:5260-5311
          417: MoveLoc[13]($t28: u64)                                           2eeec0b7:5260-5311
          418: VecMutBorrow(3)                                                  2eeec0b7:5260-5311
          419: MutBorrowFieldGeneric[4](Node.color: Color)                      2eeec0b7:5260-5311
          420: WriteRef                                                         2eeec0b7:5256-5344
          421: Ret                                                              2eeec0b7:5313-5318
B35:
          422: LdU64(0)                                                         2eeec0b7:5313-5318

Including an extra line or two of context, as shown here, reveals that the locations are at least
off by one line, as the line at the end of each block seems to have the same location as the beginning
of the following block. In fact the problem is worse than this, as the locations here are nowhere
near the positions 2769-2921 of the corresponding source code.

We find these locations applied to some quite offset code, shown here:

          430: WriteRef                                                         2eeec0b7:5358-5434
          431: Branch(407)                                                      2eeec0b7:2817-2829
B37:
          432: LdU64(0)                                                         2eeec0b7:2784-2794
          433: StLoc[22]($t81: u64)                                             2eeec0b7:2784-2794
          434: Branch(276)                                                      2eeec0b7:2784-2808
B38:
          435: MoveLoc[23](self: &mut Map<V>)                                   2eeec0b7:2784-2808
          436: MutBorrowFieldGeneric[0](Map.root: u64)                          2eeec0b7:2784-2814
          437: StLoc[16]($t38: &mut u64)                                        2eeec0b7:2784-2829
          438: CopyLoc[26]($t94: u64)                                           2eeec0b7:2885-2895
          439: MoveLoc[16]($t38: &mut u64)                                      2eeec0b7:2847-2857
          440: WriteRef                                                         2eeec0b7:2847-2857
          441: Branch(284)                                                      2eeec0b7:2847-2876
B39:
          442: PackVariant[1](Color/Black)                                      2eeec0b7:2847-2876
          443: CopyLoc[0](self: &mut Map<V>)                                    2eeec0b7:2847-2882
          444: MutBorrowFieldGeneric[1](Map.nodes: vector<Node<V>>)             2eeec0b7:2847-2895
          445: MoveLoc[11](parent_index: u64)                                   2eeec0b7:2913-2919
          446: VecMutBorrow(3)                                                  2eeec0b7:5340-5344

Referring to the file test.out generated above, we can look at the corresponding lines in the
stackless bytecode. Search from the bottom of that file for the regexp "# at .*red_black_map.move:90",
e.g., using less, and scroll up a bit to see the related code. We find the following code:

% less test.out
...
     # attr_id AttrId(306)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:156:13+76
     # live vars: $t0, $t4, $t28, $t38, $t149
250: write_ref($t38, $t149)
     # attr_id AttrId(311)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:157:16+50
     # live vars: $t0, $t4, $t28
251: label L55
     # attr_id AttrId(313)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:90:50+12
     # live vars: $t0, $t4, $t28
252: $t34 := pack_variant 0xabc::red_black_map::Color::Black()
     # attr_id AttrId(314)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:90:17+10
     # live vars: $t0, $t4, $t28, $t34
253: $t32 := borrow_field<0xabc::red_black_map::Map<#0>>.nodes($t0)
     # attr_id AttrId(315)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:90:17+24
     # live vars: $t0, $t4, $t28, $t32, $t34
254: $t192 := vector::borrow_mut<0xabc::red_black_map::Node<#0>>($t32, $t4)
     # attr_id AttrId(316)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:90:17+30
     # live vars: $t0, $t28, $t34, $t192
255: $t52 := borrow_field<0xabc::red_black_map::Node<#0>>.color($t192)
     # attr_id AttrId(317)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:90:17+45
     # live vars: $t0, $t28, $t34, $t52
256: write_ref($t52, $t34)
     # attr_id AttrId(318)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:91:55+10
     # live vars: $t0, $t28
257: $t34 := pack_variant 0xabc::red_black_map::Color::Red()
     # attr_id AttrId(319)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:91:17+10
     # live vars: $t0, $t28, $t34
258: $t32 := borrow_field<0xabc::red_black_map::Map<#0>>.nodes($t0)
     # attr_id AttrId(320)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:91:17+29
     # live vars: $t28, $t32, $t34
259: $t192 := vector::borrow_mut<0xabc::red_black_map::Node<#0>>($t32, $t28)
     # attr_id AttrId(321)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:91:17+35
     # live vars: $t34, $t192
260: $t52 := borrow_field<0xabc::red_black_map::Node<#0>>.color($t192)
     # attr_id AttrId(322)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:91:17+48
     # live vars: $t34, $t52
261: write_ref($t52, $t34)
     # attr_id AttrId(323)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:92:17+6
     # live vars:
262: return ()
     # attr_id AttrId(301)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:155:22+4
     # live vars: $t0, $t4, $t28, $t149, $t157
263: label L57
     # attr_id AttrId(302)
     # at /Users/brm/code/econia-red-black-map-b/sources/red_black_map.move:155:22+4
     # flush: $t105
     # live vars: $t0, $t4, $t28, $t149, $t157
264: $t105 := 0

Note that the attributes precede each line. Here we see that the stackless bytecode
is better attributed to the original source code, except that:

• Labels seem to have the location of the preceding code, rather than the succeeding
  code

So we have 2 problems:

• Stackless bytecode labels are misattributed to the preceding code, rather than the
  code they are labeling
• Generation of final bytecode loses the correct locations.

Final code generation occurs in
third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs, and
I've added more debugging output to start to try to understand what's going on,
(search for "Generating code for bytecode" and "setting location for" in test.out)
but it's still not clear why the code is getting the wrong locations.

Not related to Peephole

With peephole opts off:

Type of Change

• New feature
• Bug fix
• Breaking change
• Performance improvement
• Refactoring
• Dependency update
• Documentation update
• Tests

Which Components or Systems Does This Change Impact?

• Validator Node
• Full Node (API, Indexer, etc.)
• Move/Aptos Virtual Machine
• Aptos Framework
• Aptos CLI/SDK
• Developer Infrastructure
• Move Compiler
• Other (specify)

Checklist

• I have read and followed the CONTRIBUTING doc
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I identified and added all stakeholders and component owners affected by this change as reviewers
• I tested both happy and unhappy path of the functionality
• I have made corresponding changes to the documentation

[trunk-io]

⏱️ 14m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-move-tests	9m	🟩
rust-cargo-deny	2m	🟩
check-dynamic-deps	2m	🟩
general-lints	26s	🟩
semgrep/ci	26s	🟩
file_change_determinator	10s	🟩
permission-check	8s	🟩 🟩
permission-check	4s	🟩 🟩

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

[brmataptos]

@vineethk I just took a look again at this example, and I think the labels are ok. Looking at the split_critical_edges_processor, inserted labels have the same AttrId as the preceding code, which may lead to some small slop but it isn't incorrect; it shouldn't affect coverage issues.

So I suggest focusing on function_generator.rs.