rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.22k stars 12.7k forks source link

sparc-unknown-none-elf target broken #130172

Closed thejpster closed 7 hours ago

thejpster commented 1 month ago

If I try and build https://github.com/ferrous-systems/sparc-experiments with today's nightly, I get:

/home/jonathan/Downloads/gaisler/sparc-bcc-2.3.0-llvm/bin/sparc-gaisler-elf-ld: unknown architecture of input file `/tmp/rustc4jBo9z/symbols.o' is incompatible with sparc output

Using -Csave-temps, I can see:

$ file /tmp/rustc4jBo9z/symbols.o
/tmp/rustc4jBo9z/symbols.o: ELF 32-bit MSB relocatable, SPARC32PLUS, total store ordering, version 1 (SYSV), not stripped
$ file ./target/sparc-unknown-none-elf/debug/deps/sparc_demo_rust-8c8aedb85be3cab2.0unncljkfm3eoay971moazawq.rcgu.o 
./target/sparc-unknown-none-elf/debug/deps/sparc_demo_rust-8c8aedb85be3cab2.0unncljkfm3eoay971moazawq.rcgu.o: ELF 32-bit MSB relocatable, SPARC, version 1 (SYSV), with debug_info, not stripped

They are not the same.

Doing a bisect, it falls over on 2024-08-08. I suspect it might be https://github.com/rust-lang/rust/commit/d1d21ede82614f2b575cb16bcbabe75183721740, which probably should have picked Architecture::Sparc instead.

diff --git a/compiler/rustc_codegen_ssa/src/back/metadata.rs b/compiler/rustc_codegen_ssa/src/back/metadata.rs
index 0fd9d7fffe8..ea9327dc0f1 100644
--- a/compiler/rustc_codegen_ssa/src/back/metadata.rs
+++ b/compiler/rustc_codegen_ssa/src/back/metadata.rs
@@ -208,7 +208,7 @@ pub(crate) fn create_object_file(sess: &Session) -> Option<write::Object<'static
         "powerpc64" => (Architecture::PowerPc64, None),
         "riscv32" => (Architecture::Riscv32, None),
         "riscv64" => (Architecture::Riscv64, None),
-        "sparc" => (Architecture::Sparc32Plus, None),
+        "sparc" => (Architecture::Sparc, None),
         "sparc64" => (Architecture::Sparc64, None),
         "avr" => (Architecture::Avr, None),
         "msp430" => (Architecture::Msp430, None),

I did a build and my repo now builds and runs.

workingjubilee commented 1 month ago

@glaubitz Can you explain why Sparc32Plus is the "proper" architecture?

glaubitz commented 1 month ago

@glaubitz Can you explain why Sparc32Plus is the "proper" architecture?

It's been the default baseline for 32-bit SPARC on Linux for a long time already as it was the only 32-bit baseline for a long time to fully support atomic operations until Leon came around.

If you need to support a 32-bit SPARC baseline with EM_SPARC, we should find a way to differentiate between 32-bit SPARC on Linux and the sparc-unknown-none-elf target.

workingjubilee commented 1 month ago

I think we should probably conditionalize this on Linux, then (or on "none" as an OS, whatevr)

bjorn3 commented 1 month ago

Do the linux and none targets set a different target cpu? If so maybe switching based on the target cpu would work?

workingjubilee commented 1 month ago

Switching based on the target cpu seems incorrect?

bjorn3 commented 1 month ago

If you have a target cpu which doesn't support the sparc32plus isa, then using regular sparc32 as elf machine is correct and if the target cpu does support it, LLVM will likely emit instructions that require it and thus sparc32plus is the correct elf machine. Or am I misunderstanding the difference between the two elf machine ids?

workingjubilee commented 1 month ago

...wtf, they made their ISA extension level part of the ABI

apiraino commented 1 month ago

WG-prioritization assigning priority (Zulip discussion).

Also referencing the Zulip discussion.

@rustbot label -I-prioritize +P-low

glaubitz commented 1 month ago

It's unfortunate that the Leon designers introduced a new 32-bit SPARC baseline without changing the triplet name.

Classic 32-bit SPARC is definitely not suitable for modern software due to the lack of atomic operations which is why SPARC V8+ was created at some point. No idea though why a new ELF target machine was introduced for that.

thejpster commented 1 month ago

Cross-posting from Zulip:

Why does that line only affect symbols.o and not any other object file created by Rust?

And do you have any ideas on how we fix this so it works for Rust on Leon 3 (which you can buy today, definitely has users, and is growing) and people using Rust on Solaris/Linux on 32-bit SPARC (which I think is fair to call a legacy platform because they stopped making those machines in the mid 90s)?

glaubitz commented 1 month ago

As someone who has invested a lot of time and effort in the past ten years to keep open source on SPARC afloat, I disagree with your snarky comment. 32-bit SPARC on Linux and Solaris is still a valid multilib target on 64-bit SPARC since not all applications have been ported to 64-bit SPARC and especially since using 32-bit pointers can dramatically improve performance where necessary.

And the proper last 64-bit SPARC CPU was released in 2017 (SPARC M8), not in the mid-90s.

thejpster commented 1 month ago

Your point about 32 bit applications on 64 bit systems is fair. As the former user of an Ultra 5, the current owner on an Armv3 machine, and the founder of the #RustRetro channel on Matrix, I wasn't intending to be snarky. I apologise.

bjorn3 commented 1 month ago

Why does that line only affect symbols.o and not any other object file created by Rust?

Because symbols.o and lib.rmeta are the only object files created by rustc itself rather than the codegen backend. The LLVM backend creates the object files for all codegen units and already knows to pick the right elf machine.

thejpster commented 1 month ago

So always picking sparc32plus here is wrong then because that's not what LLVM chooses for this target.

I guess I can do a quick test to try all the SPARC targets and see what llvm does but I'm assuming that this works for 32 bit Linux and so LLVM is picking sparc32plus there.

thejpster commented 1 month ago

A little more digging tells me that sparc32plus is basically 64-bit SPARC V9 (for the UltraSPARC) but in 32-bit mode.

https://docs.oracle.com/cd/E19205-01/819-5267/bkazb/index.html

That's definitely wrong for sparc-unknown-elf and wrong for anyone who wants to use Rust on a SuperSPARC (a small group I suspect). Perhaps the 32-bit V8+ targets should be called sparc32plus-unknown-linux-gnu to avoid confusion? Or sparcv8plus-, perhaps.

Can this function use the full target name to pick the right EM type here?

I also spoke to Gaisler who confirmed their toolchain expects EM_SPARC.

thejpster commented 1 month ago

This seems like it might do the trick, but I can't test on Apple Silicon macOS unless I rebuild rustc inside an Rosetta 2 emulated Docker container (*), and I'm not doing that today.

diff --git a/compiler/rustc_codegen_ssa/src/back/metadata.rs b/compiler/rustc_codegen_ssa/src/back/metadata.rs
index 6215616e510..1707094b0ea 100644
--- a/compiler/rustc_codegen_ssa/src/back/metadata.rs
+++ b/compiler/rustc_codegen_ssa/src/back/metadata.rs
@@ -208,7 +208,13 @@ pub(crate) fn create_object_file(sess: &Session) -> Option<write::Object<'static
         "powerpc64" => (Architecture::PowerPc64, None),
         "riscv32" => (Architecture::Riscv32, None),
         "riscv64" => (Architecture::Riscv64, None),
-        "sparc" => (Architecture::Sparc32Plus, None),
+        "sparc" => if sess.target.options.cpu == "v9" {
+            // Target uses V8+, aka EM_SPARC32PLUS, aka 64-bit V9 but in 32-bit mode
+            (Architecture::Sparc32Plus, None)
+        } else {
+            // Target uses V7 or V8, aka EM_SPARC
+            (Architecture::Sparc, None)
+        }
         "sparc64" => (Architecture::Sparc64, None),
         "avr" => (Architecture::Avr, None),
         "msp430" => (Architecture::Msp430, None),

* because most SPARC toolchains are only available on x86 Linux

workingjubilee commented 1 month ago

Can this function use the full target name to pick the right EM type here?

sounds good to me. while some solutions are more principled here, I don't think there is any intrinsic reason not to use any of the information available to us to correct our target output, whether it's target-cpu or any part of the target string.

bjorn3 commented 1 month ago

The full target name can be effectively anything in the case of custom target spec json files. Only matching on individual fields in the target spec is guaranteed to work. If necessary we can add a new field to the target spec to determine if EM_SPARC32 or EM_SPARC32PLUS should be used, but if matching on the target cpu works fine, that would probably be better. That leaves the question though if -Ctarget-cpu=v9 on sparc-unknown-elf should use EM_SPARC32 or EM_SPARC32PLUS.

thejpster commented 1 month ago

Well I guess can basically pick at random and the first person to try it can tell us we got it wrong. But I suspect no-one will try it because the existing sparc-unknown-none-elf is aimed at the SPARC V8 LEON3, and the use-case outlined above of the v9 targets is 32-bit code running under a 64-bit OS, not bare-metal.

workingjubilee commented 1 month ago

The full target name can be effectively anything in the case of custom target spec json files.

deliberately combining "sparc-unknown-none-elf" with EM_SPARC32PLUS code while using a target-spec.json sounds like such an exquisitely specific choice that I kind of want to meet that person.

workingjubilee commented 1 month ago

For note: EM_SPARC and EM_SPARC32PLUS files should be able to be linked together, and should produce an EM_SPARC32PLUS file.

@glaubitz Was there a practical concern about emitting Sparc objects with an incorrect ELF machine?

bjorn3 commented 1 month ago

EM_SPARC and EM_SPARC32PLUS files should be able to be linked together, and should produce an EM_SPARC32PLUS file.

If that is true, using EM_SPARC unconditionally for symbols.o should be fine, right? symbols.o doesn't contain any machine code, so it could have used EM_ANY if such a thing existed.

glaubitz commented 1 month ago

On Oct 4, 2024, at 4:47 AM, Jubilee @.***> wrote: For note: EM_SPARC and EM_SPARC32PLUS files should be able to be linked together, and should produce an EM_SPARC32PLUS file. @glaubitz Was there a practical concern about emitting Sparc objects with an incorrect ELF machine?That would have to be tested first, I don’t know how the linker would behave in the end.The reason why I did not consider anything else but V8+ is because Rust itself did not work on 32-bit SPARC before all of the recent fixes that I submitted.If you look at the commit history, you’ll see a lot of fixes by me to enable Rust on 32-bit SPARC in the first place. So, I just assumed that I was the only person to have tested it.Outside of Leon, both Solaris and Linux assume V8+ as the baseline for 32-bit SPARC which is why I used that in the first place.

workingjubilee commented 1 month ago

@glaubitz Well it seems you broke taiki's SPARC testing, or a nearby commit did: https://github.com/taiki-e/setup-cross-toolchain-action/commit/8953e6c927f0185f159fa8bcb85a231893aee2e7

The V8+ Technical Specification states that EM_SPARC and EM_SPARC32PLUS should merge. If a linker can't support the published technical spec for a 30 year old architecture, and that's the only linker we have for that target, I don't see why we should bother with the target.

thejpster commented 1 month ago

The reason why I did not consider anything else but V8+ is because Rust itself did not work on 32-bit SPARC before all of the recent fixes that I submitted.If you look at the commit history, you’ll see a lot of fixes by me to enable Rust on 32-bit SPARC in the first place. So, I just assumed that I was the only person to have tested it.

When I added sparc-unknown-none-elf back in September 2023 to coincide with an ESA software summit, everything was working great. If that was after your changes went in then I thank you for making it so easy for me.

Rust on RTEMS on SPARC is in the RTEMS user manual (https://docs.rtems.org/branches/master/user/rust/bare-metal.html#build-and-run-on-sparc), although they don't observe this breakage because they build a C binary with a Rust static library linked in.