Unused enum value seems to be breaking generated binary
brychanrobot opened this issue · 11 comments
I was trying to update firmware and got a warning that https://github.com/probe-rs/hs-probe-firmware/blob/master/firmware/src/usb/winusb.rs#L31 is unused. The build seemed to still work and it was only a warning so I forged ahead, but I found that when I flashed the firmware the probe became unresponsive including its ability to load new firmware. I shorted the appropriate leads to get it into the fallback stm32 bootloader and tried removing the unused line. This time everything worked perfectly. I'm not sure why a warning would break everything, but thought it was an issue worth raising.
I compared the binaries generated with and without that change. The only differences were causes by the git-version macro. When I patch that out and replace it by a constant string, I get byte-by-byte identical firmwares whether or not I comment out the unused enum variant HeaderConfiguration.
So it seems extremely unlikely that your observation is directly related to the warning.
Please check again if the issue is reproducible. Perhaps something else went wrong while flashing the firmware.
If it is really reproducible, my guess would be that the changed version string (which also changes in length because of the -modified suffix to the version number, in case you don't commit the change) causes some alignment changes, which in turn triggers some bug somewhere else. Unlikely, but not impossible.
(EDIT: For completeness, tested on commit 224f872, using rust version 1.67.0-beta.4)
I tried again and it did repro. When I leave the warning in place the probe becomes unresponsive after flashing. Commenting out the unused enum value again caused it to work correctly.
I'm also compiling commit 224f872, but using rustc version 1.66.0
I get the same issue building with rust 1.66 on windows.
cargo objdump --release -- --all-headers working.txt not_working.txt
The only change I see in those objdump outputs is that the text section grows by 24 bytes and the rodata section grows by 8 bytes.
I also tried it with 1.66.0 (on linux), with similar results: Commenting out the line causes the same change in section size. After replacing git_version!() with some fixed dummy string, commenting out the unused enum variant HeaderConfiguration does no longer affect the generated binary at all.
I did not have time to actually flash the generated binaries to an hs-probe, yet.
If my theory is correct that the only difference is the length of the git version string, any other change should have the same effect - even if it's not even in the source code, for example a change to README.md. Could you try that? Keep winusb.rs unchanged, so that the warning about the unused enum variant is still there, and instead just change something in README.md, and trigger a recompile?
Just changing some bytes in README.md did not change the generated binary and it still was not working.
I tried different things like adding a unused const to winusb.rs which also did not help.
But adding a unused enum variant to MsDescriptorTypes again resulted in a working build.
Anyway after running cargo update I‘m now not able to reproduce the issue anymore.
@brychanrobot Can you also try to run cargo update to see if it fixes your build?
Before running cargo update, please save your Cargo.lock, in case we want to reproduce the issue later.
@brychanrobot did you build with nightly? Nightly seems to be broken, cargo +stable build ... worked for me. (I have the winusb warning and it does NOT affect anything as far as I can tell).
I just tested tried this on my hsprobe.
Build the current git version with stable 1.70 and it will not enumerate.
Updating to latest cortex-m-rt and stm32ral fixes it.
Smells like an alignment issue...
Last toolchain that is capable of building a working firmware is 1.66. 1.67 one is dead. Doing what @9names suggested gets rid of the problem. cortex-m had some alignement issues but I don't remember in which version exactly. In any case this probably should be fixed. Among others, cortex-m and stm32ral should be reexported from hs-probe-bsp to avoid double dependency declaration both in firmware and hs-probe-bsp. If I find time I'll try to publish a PR.
The versions of cortex-m-rt with the alignment issue got yanked from crates.io, the version hs-probe is using is older than that.
I had the same symptoms as here, cargo update seems to have sorted everything. Recovered by shorting the pins marked with the little white line - on hs-probe v1.4 it's between a capacitor and resistor near pin 1 of the main micro.