Repository navigation
toolchain: Bump to 1.99.1 - #2775
Conversation
It turns out that #2761 caused a formatting regression due to rust-lang/rust#158371. This was fixed in >= 1.98.1, so we are bumping to the current stable to obtain this fix. I checked the contents of a build with the `strings` binutil, and remapping still seems to be working as expected (I see `rustc`, `crates.io`, etc.).
in a new `linker_messages` lint.
This is not necessarily guaranteed to be suitably aligned for rodata.
| start = true | ||
| task-slots = ["jefe"] | ||
| stacksize = 1328 | ||
| stacksize = 1456 |
There was a problem hiding this comment.
This one only bumped by like 30 bytes, I just gave it 128 to make it less noisy in the future.
| priority = 5 | ||
| max-sizes = {flash = 65536, ram = 32768} | ||
| stacksize = 4096 | ||
| stacksize = 8192 |
There was a problem hiding this comment.
This one did go up by like 1KiB, it just seemed to be LLVM inlining noise, and where it did/didn't figure out how to share stack slots for large returned results.
Most other tasks give this stacksize = 16384 already anyway.
There was a problem hiding this comment.
Doesn't need to happen in this PR, but I would also support retiring gemini-bu entirely – Grapefruit is a full replacement and is much more common.
labbott
left a comment
There was a problem hiding this comment.
Adding the approval because I'm blocking. I will admit I find the as_chunks to be a little weird at first with the .0 everywhere but that's a me issue.
| // 50x 10-bit values can't overflow a u16 | ||
| let mut sum = 0u16; | ||
| for c in buf.chunks_exact(2) { | ||
| for c in buf.as_chunks::<2>().0 { |
There was a problem hiding this comment.
Yeah, it's somewhat of a less-pretty syntax, but it gives you (&[[u8; 2]], &[u8]) instead of Iterator<Item = &[u8]>. chunks_exact(_mut) also ignored any remainder, hence the just looking at the .0 part.
The main benefit is that you KNOW you are looking at &[u8; 2] instead of &[u8], which hopefully should remove some potential panicking branches, and ideally optimize better (no bounds checks for slicing in-bands of N).
| } else { | ||
| let cwd = std::env::current_dir().unwrap().join("scripts"); | ||
|
|
||
| println!("cargo::rustc-link-arg=--verbose"); |
There was a problem hiding this comment.
The --verbose arg made the linker (rust-lld) print all of it's inputs to stderr. Before, linker output was only shown when linking failed, so excess output on success wasn't visible.
This behavior has changed, and now the linker's output is ALWAYS shown, under the new linker_messages lint class. This meant that the --verbose arg here was always printing linker output on every build of endoscope, which was gross.
If we need to debug something with linking, we can bring back --verbose while debugging.
There was a problem hiding this comment.
This caused output like this every time that endoscope was built:
warning: linker stderr: rust-lld: set maximum concurrency to 16, specify --threads= to change
rust-lld: /Users/james/oxide/hubris/target/bindeps/swd/thumbv7em-none-eabihf/release/deps/endoscope-54536f7bacf9621e.endoscope.8ce9857115f0f07d-cgu.0.rcgu.o
rust-lld: /Users/james/oxide/hubris/target/bindeps/swd/thumbv7em-none-eabihf/release/deps/rustcIP0Q45/libcortex_m-36a0646d479047f1.rlib
rust-lld: /Users/james/oxide/hubris/target/bindeps/swd/thumbv7em-none-eabihf/release/deps/rustcIP0Q45/libcortex_m_rt-5766ac831cdd5562.rlib
rust-lld: /Users/james/.rustup/toolchains/1.99.1-aarch64-apple-darwin/lib/rustlib/thumbv7em-none-eabihf/lib/libcompiler_builtins-87e44e78be234358.rlib
rust-lld: /Users/james/oxide/hubris/lib/endoscope/scripts/stm32h753.x
rust-lld: /Users/james/oxide/hubris/lib/endoscope/scripts/endoscope.x
|
= note: `#[warn(linker_messages)]` on by default
warning: `endoscope` (bin "endoscope") generated 1 warning
Finished `release` profile [optimized + debuginfo] target(s) in 8.95s
| } > RAM | ||
|
|
||
| /* ### .rodata */ | ||
| .rodata __etext : ALIGN(4) |
There was a problem hiding this comment.
Did they remove this symbol?
There was a problem hiding this comment.
No, this symbol was not removed.
This was causing a warning, because there were items in .rodata that were 8-byte aligned, and pinning the start address of .rodata to __etext (which isn't necessarily 8-byte aligned) meant that the linker HAD to start the section there. Removing this pinned start location allows the linker to naturally align the start of the section to whatever is required, instead of forcing it to start where we tell it.
AFAICT, the linker would still insert necessary padding if .rodata was only 4-byte aligned, and it wanted to place an 8-byte aligned object, so theres no wrong behavior this is fixing, the linker just gets grumpy about it and emits a warning about it.
There was a problem hiding this comment.
This caused warnings like this (for both the kernel and endoscope IIRC):
warning: linker stderr: rust-lld: address (0x58e8c) of section .rodata is not a multiple of alignment (8)
|
= note: `#[warn(linker_messages)]` on by default
warning: `oxide-rot-1` (bin "oxide-rot-1") generated 1 warning
Finished `release` profile [optimized + debuginfo] target(s) in 1.03s
It turns out that #2761 caused a formatting regression due to rust-lang/rust#158371.
This was fixed in >= 1.98.1, so we are bumping to the current stable to obtain this fix.
Fixed some new lints for ringbuf.
I checked the contents of a build with the
stringsbinutil, and path remapping still seems to be working as expected (I seerustc,crates.io, etc.).Requested Smoke Testing
humility dumphumility#755 (pre-existing)humility monorail statuslooking good on Sidecar