[Bug] Disambiguate symbol names by module address

[brson]

In https://github.com/solana-labs/move/pull/289 I mentioned that we have problems with symbol name collisions when two functions in different modules have the same names.

The obvious solution to this is to include the unique module address in symbol names.

I have attempted to do this in https://github.com/brson/move/tree/symbol-names, but so far not completely succeeded. That branch has two rbpf-tests cases that fail to load because of "InvalidAccountData".

[brson]

The problem is that rbpf has a maximum symbol name length of 64 bytes (SYMBOL_NAME_LENGTH_MAXIMUM) and my patch happens to put some symbols over that number.

Rust generates some huge symbols though, so I wonder if there is some post-processing step solana does to rust binaries to change symbol names.

[dmakarov]

Solana strips symbols from loadable binaries.

[brson]

I mentioned in https://github.com/solana-labs/move/issues/345 that running llvm-objcopy --strip-all does not strip symbol names from the dynamic symbol table (.dynsym) that is needed for relocations in rbpf, where the error is occurring.

I'm a little stumped again.

The dynsym table in the mvector03 test looks in part like this:

Symbol table '.dynsym' contains 71 entries:
   Num:    Value          Size Type    Bind   Vis       Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND
     1: 0000000000000120   136 FUNC    GLOBAL DEFAULT     1 vector_tests__destroy_empty_300
     2: 0000000000016808    32 FUNC    GLOBAL DEFAULT     1 move_native_vector_empty
     3: 0000000000018510    96 FUNC    GLOBAL DEFAULT     1 move_native_vector_destroy_empty
     4: 00000000000001a8  1048 FUNC    GLOBAL DEFAULT     1 vector_tests__length_300
     5: 0000000000016828    16 FUNC    GLOBAL DEFAULT     1 move_native_vector_length
     6: 0000000000013590    16 FUNC    GLOBAL DEFAULT     1 move_rt_abort
     7: 0000000000016d08  1152 FUNC    GLOBAL DEFAULT     1 move_native_vector_push_back
     8: 00000000000005c0   384 FUNC    GLOBAL DEFAULT     1 vector_tests__append_empties_is_empty_300
     9: 0000000000017a98  2680 FUNC    GLOBAL DEFAULT     1 move_native_vector_pop_back
    10: 0000000000018570  3616 FUNC    GLOBAL DEFAULT     1 move_native_vector_swap
    11: 0000000000000d20  1928 FUNC    GLOBAL DEFAULT     1 vector_tests__append_respects_order_empty_lhs_300
    12: 0000000000016838  1232 FUNC    GLOBAL DEFAULT     1 move_native_vector_borrow
    13: 00000000000014a8  1912 FUNC    GLOBAL DEFAULT     1 vector_tests__append_respects_order_empty_rhs_300
    14: 0000000000001c20  1768 FUNC    GLOBAL DEFAULT     1 vector_tests__append_respects_order_nonempty_rhs_lhs_300

These names are getting too long for rbpf to accept.

I wonder if we need to LTO everything and make a bunch of these symbols internal. Maybe then they wouldn't need relocations. (Or rather, I wonder if that is how cargo-build-sbf is avoiding huge symbols in rust's relocations).

[dmakarov]

That was an issue before I changed default visibility of symbols generated by rust compiler to hidden. Now all of the symbols that come from rust std library should not need dynamic relocation. I think by now only the unresolved symbols of Solana syscalls should need dynamic relocations. Are you using the new platform-tools version 1.38?..

[brson]

That was an issue before I changed default visibility of symbols generated by rust compiler to hidden. Now all of the symbols that come from rust std library should not need dynamic relocation. I think by now only the unresolved symbols of Solana syscalls should need dynamic relocations. Are you using the new platform-tools version 1.38?..

I am not sure how to tell what version of platform tools I am using.

The issue is not in rustc-generated code though. The symbols coming out of our move llvm backend are too large.

[dmakarov]

The issue is not in rustc-generated code though. The symbols coming out of our move llvm backend are too large.

Yes, move-native symbols can exceed 64 characters length. We probably have to keep them short or somehow use fixed length hashes in-place of actual function names in the generated code (not sure how this could be implemented though).