Test mangled named on Windows too

[pacak]

We need to strip underscore on 32bit Windows and MacOS and leave it as is on Linux and 64bit Windows.

Fixes #137

[pacak]

@evmar

I have some tests for no_mangle names (sample_cdylib folder), currently they are passing for MacOS and Linux. I tried enabling them for Windows too. cargo-show-asm is able to find function add, but not _mul. The only difference is that _mul starts with underscore. Even in your example it reads .section .text,"xr",one_only,_mainCRTStartup as mainCRTStartup. Branch for as far as I understand MacOS adds an extra _ and codepath for MacOS strips it. I guess we need to add something to handle the _ prefix differently on those two OSes.

[pacak]

I guess it's just strip_underscore parameter for get_item_in_section, I'll make a fix.

[evmar]

As far as I understand it, the preceding underscore is C-level name mangling and is used for cdecl symbols.

I tried this:

#[no_mangle]
pub fn test1() {}
#[no_mangle]
pub fn _test2() {}
#[no_mangle]
pub extern "C" fn test3() {}
#[no_mangle]
pub extern "C" fn _test4() {}

The resulting symbols:

$ egrep '^_?_test' target/i586-pc-windows-msvc/debug/deps/ops.s
_test1:
__test2:
_test3:
__test4:
$ egrep 'section.*_?_test' target/i586-pc-windows-msvc/debug/deps/ops.s
        .section        .text,"xr",one_only,_test1
        .section        .text,"xr",one_only,__test2
        .section        .text,"xr",one_only,_test3
        .section        .text,"xr",one_only,__test4

So I think it's correct to strip an underscore (?)

[evmar]

Oh for completeness:

$ egrep 'glob.*_?_test' target/i586-pc-windows-msvc/debug/deps/ops.s 
        .globl  _test1
        .globl  __test2
        .globl  _test3
        .globl  __test4

[pacak]

In that case I don't understand what's going on... In sample_cdylib I have a function _mul - defined with a single underscore and it remains as a single underscore in asm on a CI machine, stripping turns it into mul and when test ask for _mul - it fails to find it. With changes in this pull request test passes on all 3 platforms I support.

[evmar]

I found this discussion of the leading underscore: https://stackoverflow.com/questions/27487756/calling-convention-name-mangling-in-c

That response says:

if you read the documentation on MSDN for __cdecl very, very closely, you'll see that it mentions that the "Underscore character (_) is prefixed to names, except when __cdecl functions that use C linkage are exported."

However, I repeated the above test with extern "cdecl" and I still get an extra underscore, so I think it is correct to always strip.

[pacak]

Can you check the output on sample_cdylib with/without most recent commit in this branch?

[evmar]

cargo-show-asm/sample_cdylib$ RUSTFLAGS='-C linker=rust-lld -L /Users/evmar/redist/crt/lib/x86 -L /Users/evmar/redist/sdk/lib/ucrt/x86 -L /Users/evmar/redist/sdk/lib/um/x86 --emit asm' cargo build --target i586-pc-windows-msvc 

$ grep globl target/i586-pc-windows-msvc/debug/deps/xx.s
        .globl  @feat.00
        .globl  _add
        .globl  _sub
        .globl  __mul

Hmm. Maybe some difference between the toolchain I'm using and the one used by your builder?

[pacak]

From filenames - you are doing some sort of cross compilation from Mac to Windows. Runner is native Windows.

[evmar]

Yes, this is cross compilation. From the perspective of rustc I imagine it has to be the same codepath to generate windows code regardless of the platform you're starting from, shouldn't it?

[evmar]

Aha, it's 32-bit vs 64-bit... for reasons I am wholly unclear on.

~/projects/cargo-show-asm$ grep globl sample_cdylib/target/x86_64-pc-windows-msvc/release/deps/xx.s
      .globl  @feat.00
        .globl  add
        .globl  sub
        .globl  _mul
~/projects/cargo-show-asm$ grep globl sample_cdylib/target/i586-pc-windows-msvc/release/deps/xx.s
        .globl  @feat.00
        .globl  _add
        .globl  _sub
        .globl  __mul

[pacak]

So we want stripping on 64-bit, but not on 32-bit. At least on Windows. 🤔

[evmar]

A final doc reference that confirms this:

In a 64-bit environment, C or extern "C" functions are only decorated when using the __vectorcall calling convention.

[evmar]

I guess you have the cargo machinery in place to identify the target platform. For Windows aside from CPU arch there is also -pc-windows-msvc vs -pc-windows-gnu and I don't know if they also have subtlety here. :(

I think the number of people who are targeting 32-bit Windows is vanishingly small, so I might suggest just picking the behavior that works well for 64-bit Windows (that is, not stripping underscores) and leaving the rest for someone to worry about another time, because it's probably not worth your time!

[pacak]

So just to double check - is what I have in this branch working for you (possibly with incorrect underscore prefixed)?

[evmar]

Maybe add a comment here like "64-bit Windows does not add underscores; TODO: on 32-bit Windows we ought to remove underscores" just so the next person doesn't need to rediscover this

[evmar]

Yes!

Current branch behavior:

32-bit:

$ RUSTFLAGS="-C linker=rust-lld -L $XWIN/crt/lib/x86 -L $XWIN/sdk/lib/ucrt/x86 -L $XWIN/sdk/lib/um/x86 --emit asm" ./target/debug/cargo-asm --manifest-path sample_cdylib/Cargo.toml --target i586-pc-windows-msvc
warning: `/Users/evmar/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
warning: `/Users/evmar/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
   Compiling xx v0.1.0 (/Users/evmar/projects/cargo-show-asm/sample_cdylib)
    Finished `release` profile [optimized] target(s) in 0.10s

Try one of those by name or a sequence number
0 "__mul" [13]
1 "_add" [14]
2 "_sub" [13]

64-bit:

$ RUSTFLAGS="-C linker=rust-lld -L $XWIN/crt/lib/x86_64 -L $XWIN/sdk/lib/ucrt/x86_64 -L $XWIN/sdk/lib/um/x86_64 --emit asm" ./target/debug/cargo-asm --manifest-path sample_cdylib/Cargo.toml --target x86_64-pc-windows-msvc
warning: `/Users/evmar/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
warning: `/Users/evmar/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
   Compiling xx v0.1.0 (/Users/evmar/projects/cargo-show-asm/sample_cdylib)
    Finished `release` profile [optimized] target(s) in 0.08s

Try one of those by name or a sequence number
0 "_mul" [11]
1 "add" [9]
2 "sub" [11]

[evmar]

Also just to double check, this branch on the crate I started this all from:

$ RUSTFLAGS='-C panic=abort' ~/projects/cargo-show-asm/target/debug/cargo-asm --target i586-pc-windows-msvc -p no_std --bin ops
warning: `/Users/evmar/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
warning: `/Users/evmar/.cargo/config` is deprecated in favor of `config.toml`
note: if you need to support cargo 1.38 or earlier, you can symlink `config` to `config.toml`
    Finished `release` profile [optimized] target(s) in 0.01s

Try one of those by name or a sequence number
0 "__test2" [8]
1 "_mainCRTStartup" [13]
2 "_rust_begin_unwind" [15]
3 "ops::fmt_flags" [163]
4 "ops::test" [155]

The extra underscores are fine, no worries.

[pacak]

Okay. I'll try to release today/tomorrow and will make a ticket for 32bit windows for future me when I next time feel bored. And I think I can close #137.