rust icon indicating copy to clipboard operation
rust copied to clipboard
verified publisher icon rust-lang

Binary size regression on msp430 due to #98582

Open cr1901 opened this issue 3 years ago • 3 comments

I recently had CI start failing for some firmware I use to make sure Rust still compiles msp430 code correctly. It appears that as of #98582, Rust has stopped optimizing out dead panic code; there is no I/O device to send the panic strings to, and so panic strings- and setting up arguments to access panic strings- should be considered dead code.

Normally, the firmware size should take 1994 (1992+2) bytes:

cargo build --release -Zbuild-std=core --target=msp430-none-elf
msp430-elf-size target/msp430-none-elf/release/at2xt
   text    data     bss     dec     hex filename
   1992       2      48    2042     7fa target/msp430-none-elf/release/at2xt

However, the firmware size had gone up 100 bytes (2048+46 = 2094- 5% regression) before #98582 was reverted by #99495:

cargo build --release -Zbuild-std=core --target=msp430-none-elf
error: linking with `msp430-elf-gcc` failed: exit status: 1
  |
  = note: "msp430-elf-gcc" "/home/william/Projects/embedded/msp430/AT2XT/target/msp430-none-elf/release/deps/at2xt-4706a59318f01763.at2xt.aa970193-cgu.0.rcgu.o" "-L" "/home/william/Projects/embedded/msp430/AT2XT/target/msp430-none-elf/release/deps" "-L" "/home/william/Projects/embedded/msp430/AT2XT/target/release/deps" "-L" "/home/william/Projects/embedded/msp430/AT2XT/target/msp430-none-elf/release/build/msp430-rt-587a8251c4c89199/out" "-L" "/home/william/Projects/embedded/msp430/AT2XT/target/msp430-none-elf/release/build/msp430g2211-1199852debc08ead/out" "-L" "/home/william/Projects/toolchains/build-llvm-toolchain/build-rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/msp430-none-elf/lib" "-Wl,-Bstatic" "/tmp/rustcy1Mond/libmsp430_rt-19edab0518267d0f.rlib" "-Wl,--start-group" "-Wl,--end-group" "/home/william/Projects/embedded/msp430/AT2XT/target/msp430-none-elf/release/deps/libcompiler_builtins-1ef8666a5bfcc3f4.rlib" "-Wl,-Bdynamic" "-L" "/home/william/Projects/toolchains/build-llvm-toolchain/build-rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/msp430-none-elf/lib" "-o" "/home/william/Projects/embedded/msp430/AT2XT/target/msp430-none-elf/release/deps/at2xt-4706a59318f01763" "-nodefaultlibs" "-Tlink.x" "-mcpu=msp430" "-nostartfiles" "-lmul_none" "-lgcc"
  = note: /home/william/.local/bin/../lib/gcc/msp430-elf/9.2.0/../../../../msp430-elf/bin/ld: /home/william/Projects/embedded/msp430/AT2XT/target/msp430-none-elf/release/deps/at2xt-4706a59318f01763 section `.rodata' will not fit in region `ROM'
          /home/william/.local/bin/../lib/gcc/msp430-elf/9.2.0/../../../../msp430-elf/bin/ld: section .rodata VMA wraps around address space
          /home/william/.local/bin/../lib/gcc/msp430-elf/9.2.0/../../../../msp430-elf/bin/ld: section .vector_table LMA [000000000000ffe0,000000000000ffff] overlaps section .rodata LMA [000000000000ff48,000000000001000b]
          /home/william/.local/bin/../lib/gcc/msp430-elf/9.2.0/../../../../msp430-elf/bin/ld: region `ROM' overflowed by 46 bytes
          collect2: error: ld returned 1 exit status


error: could not compile `at2xt` due to previous error
error: Recipe `timer` failed on line 10 with exit code 101

I've created an MVCE from the failing CI to illustrate:

Code

Instructions

  1. Make sure the msp430-elf-gcc toolchain is installed. Optionally install just for convenience.

  2. git clone https://github.com/cr1901/msp430-size. Use commit e44cf66 specifically.

  3. Compile rustc and add your new rustc to rustup using rustup toolchain link override-name /path/to/override. Use this override for both rustc commits in the next step.

  4. Run the below command twice (using different target dirs):

    cargo +override-name rustc --manifest-path=./take-api/Cargo.toml --release -Zbuild-std=core --example=once -- --emit=obj=target/msp430-none-elf/release/examples/once.o,llvm-ir=target/msp430-none-elf/release/examples/once.ll,asm=target/msp430-none-elf/release/examples/once.s

    The above command needs to be run with two different compilers:

    • Once with a compiler before #98582 or after #99495.
    • Once with a compiler in between those PRs.
    • I used 4a742a691 and 03d488b48a, but the actual commits aren't essential.

  5. Extra compiler artifacts can be generated like so:

    msp430-elf-objdump -Cd target/msp430-none-elf/release/examples/once
msp430-elf-readelf -a --wide target/msp430-none-elf/release/examples/once
msp430-elf-objdump -Cd target/msp430-none-elf/release/examples/once.o
msp430-elf-readelf -a --wide target/msp430-none-elf/release/examples/once.o
msp430-elf-size target/msp430-none-elf/release/examples/once

Expected Results

For both Rust compiler commits, I expected to see the following output from msp430-elf-size, with no panic strings emitted:

william@xubuntu-dtrain:~/Projects/embedded/msp430/msp430-size$ msp430-elf-size -A target/msp430-none-elf/release/examples/once
target/msp430-none-elf/release/examples/once  :
section              size    addr
.vector_table          32   65504
.text                 168   49152
.rodata                 4   49320
.bss                    2     512
.data                   0     514
.MSP430.attributes     23       0
Total                 229
strings target/msp430-none-elf/release/examples/once | grep called

Instead, the compiler which had PR #98582 had a different size, and a panic string for Option::unwrap() failure was emitted into the binary:

william@xubuntu-dtrain:~/Projects/embedded/msp430/msp430-size$ msp430-elf-size -A target/msp430-none-elf/release/examples/once
target/msp430-none-elf/release/examples/once  :
section              size    addr
.vector_table          32   65504
.text                 200   49152
.rodata                48   49352
.bss                    2     512
.data                   0     514
.MSP430.attributes     23       0
Total                 305
strings target/msp430-none-elf/release/examples/once | grep called
?called `Option::unwrap()` on a `None` value

Regression Commits

Based on git bisect, this regression was introduced by PR #98582. Specifically 728c7e8bda215dc0ef055a817964f199427512e0 introduced the regression. This regression was fixed by #99495. However, @oli-obk asked me to open an issue anyway.

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

Other Context

Although I don't have numbers handy, this regression also affects libcores compiled with the panic_immediate_abort feature for less marked size regression.

cr1901 avatar Jul 24 '22 16:07 cr1901

can you check whether https://github.com/rust-lang/rust/pull/99806 does not have the same issue?

There's a try build that you can download at 2a2d919ccd8dc0e7f8c61e9e8d07f865fb05e842

oli-obk avatar Sep 16 '22 14:09 oli-obk

@oli-obk I wasn't sure how to download the try build, so I checked out the PR, compiled it locally, and then compared it to a recent commit (48de123) I also compiled:

william@xubuntu-dtrain:~/Projects/toolchains/rust$ git fetch origin pull/99806/head:oli-fix
william@xubuntu-dtrain:~/Projects/toolchains/rust$ git checkout oli-fix

Short version, assuming that commit I checked is close enough, looks like #99806 does not have the same issue and is good to go :).

AT2XT

oli-fix

msp430-elf-size target/msp430-none-elf/release/at2xt
   text    data     bss     dec     hex filename
   2010       2      48    2060     80c target/msp430-none-elf/release/at2xt

48de123

msp430-elf-size target/msp430-none-elf/release/at2xt
   text    data     bss     dec     hex filename
   2010       2      48    2060     80c target/msp430-none-elf/release/at2xt

msp430-size

oli-fix

+ msp430-elf-size target/msp430-none-elf/release/examples/once
   text    data     bss     dec     hex filename
    186       0       2     188      bc target/msp430-none-elf/release/examples/once

48de123

+ msp430-elf-size target/msp430-none-elf/release/examples/once
   text    data     bss     dec     hex filename
    186       0       2     188      bc target/msp430-none-elf/release/example

cr1901 avatar Sep 18 '22 00:09 cr1901

I wasn't sure how to download the try build

You can use rustup-toolchain-install-master with rustup-toolchain-install-master 2a2d919ccd8dc0e7f8c61e9e8d07f865fb05e842 to download the try build artifacts.

lqd avatar Sep 18 '22 10:09 lqd

Thanks for checking! I'm going to go ahead and close this issue then.

oli-obk avatar Sep 19 '22 12:09 oli-obk

@lqd I will try rustup-toolchain-install-master with 2a2d919ccd8dc0e7f8c61e9e8d07f865fb05e842 just for completeness' sake, but... how do I get past this error?

william@xubuntu-dtrain:~/Projects/embedded/msp430/msp430-size$ rustup component add rust-src --toolchain 2a2d919ccd8dc0e7f8c61e9e8d07f865fb05e842
error: 2a2d919ccd8dc0e7f8c61e9e8d07f865fb05e842 is a custom toolchain

I'm pretty sure I've seen this before, but have since forgotten how to solve it (the MCVE requires build-std=core)...

cr1901 avatar Sep 20 '22 03:09 cr1901

IIRC you can use -c when you initially install the try build artifacts.

lqd avatar Sep 20 '22 06:09 lqd

IIRC you can use -c when you initially install the try build artifacts.

Okay, just to make crystal clear, the try build at 2a2d919c is identical to the results I have above (i.e. no problems). Didn't think it would be different, but no harm in trying since I have all the toolchain stuff already installed.

Thanks for the help :D!

cr1901 avatar Sep 21 '22 17:09 cr1901