refactor: change to the official riscv32emac toolchain

[jarkkojs]

Change from the Parity's rv32e fork to the official rustc toolchain.

Makes sure that also PolkaVM exports get relocation entry with the '--relocatable' parameters. Otherwise the linker will only add the relocs, whic are referenced by the code directly.

Flag DWARF processing under the feature flag 'dwarf' for the moment.

Link: https://github.com/paritytech/rustc-rv32e-toolchain/
Closes: #178

[jarkkojs]

Not worth commenting just yet. There's still a few issues but all are related just twists in DWARF data, which did not exist when compiled with Parity's toolchain.

Summary:

• Most of the fixes already in place might need some tweaking but mostly are correctly identified bugs.
• There's one issue at least left that I simply did not yet investigate properly but can be addressed probably just by looking the binary with rz-bin.
• @athei asked to remove .cargo/config.toml but I'm not sure about this but open for feedback. @koute on the other asked to remove build-bechmark.sh. So some overall conclusions and consensus "do this with build shenanigans" would be nice here. No focus on this before I've fixed the remaining DWARF issues.

So I do a temporary work branch to finalize this pull request, thus does not yet much sense to give any detailed feedback on the code itself. I've tried to add some rz-bin output to each commit so that it describes clearly the situation in the binary and kind of helps to understand from those grounds what the code change does (as otherwise these make no sense at all).

[jarkkojs]

So this is the iteration #2 of the pull request.

Opens:

$ git grep detect-or-install-riscv-toolchain.sh     
.github/workflows/rust.yml:      run: ./ci/jobs/detect-or-install-riscv-toolchain.sh
.github/workflows/rust.yml:      run: ./ci/jobs/detect-or-install-riscv-toolchain.sh
ci/jobs/build-guests.sh:source ./ci/jobs/detect-or-install-riscv-toolchain.sh
ci/jobs/clippy.sh:source ./ci/jobs/detect-or-install-riscv-toolchain.sh
ci/jobs/rustfmt.sh:source ./ci/jobs/detect-or-install-riscv-toolchain.sh
ci/run-all-tests.sh:source ./ci/jobs/detect-or-install-riscv-toolchain.sh
guest-programs/build-examples.sh:source ../ci/jobs/detect-or-install-riscv-toolchain.sh

I think it is better to have a checkpoint (feedback) first before fixing up these (in a separate commit).

Changes to dwarfs.rs are bottom because I wanted to test them also with the previous toolchain. With these changes now also build-benchmark.sh passes with the new toolchain.

[koute]

Looks mostly good to me!

We can merge this once we fix clippy.

[jarkkojs]

So I there was issue I was looking into on removing that CI script see first this temp diff:

diff --git a/crates/polkavm-linker/src/program_from_elf.rs b/crates/polkavm-linker/src/program_from_elf.rs
index 0c60355..d18dcff 100644
--- a/crates/polkavm-linker/src/program_from_elf.rs
+++ b/crates/polkavm-linker/src/program_from_elf.rs
@@ -1408,6 +1408,7 @@ where
     let _ = b.read(target.offset as usize)?;
 
     let version = b.read_byte()?;
+    log::debug!("section {} {}", section.name(), version);
     if version != 1 && version != 2 {
         return Err(format!("unsupported extern metadata version: '{version}' (expected '1' or '2')"));
     }
diff --git a/guest-programs/build-examples.sh b/guest-programs/build-examples.sh
index 39542a3..0e00312 100755
--- a/guest-programs/build-examples.sh
+++ b/guest-programs/build-examples.sh
@@ -12,13 +12,22 @@ fi
 
 function build_example () {
     output_path="output/$1.polkavm"
-    current_dir=$(pwd)
 
     echo "> Building: '$1' (-> $output_path)"
-    RUSTFLAGS="-C target-feature=+c -C relocation-model=pie -C link-arg=--emit-relocs -C link-arg=--unique --remap-path-prefix=$(pwd)= --remap-path-prefix=$HOME=~" cargo build -q --release --bin $1 -p $1
-    cd ..
-    cargo run -q -p polkatool link --run-only-if-newer -s guest-programs/target/riscv32ema-unknown-none-elf/release/$1 -o guest-programs/$output_path
-    cd $current_dir
+
+    RUSTFLAGS="--remap-path-prefix=$(pwd)= --remap-path-prefix=$HOME=~" \
+    cargo build  \
+        -Z build-std=core,alloc \
+        --target "$PWD/riscv32emac-unknown-none-polkavm.json" \
+        -q --release --bin $1 -p $1
+
+    pushd ..
+
+    cargo run -q -p polkatool link \
+        --run-only-if-newer guest-programs/target/riscv32emac-unknown-none-polkavm/release/$1 \
+        -o guest-programs/$output_path
+
+    popd
 }
 
 build_example "example-hello-world"

The result:

❯ RUST_BACKTRACE=1 RUST_LOG="polkavm_linker::program_from_elf=debug" guest-programs/build-examples.sh   
> Building: 'example-hello-world' (-> output/example-hello-world.polkavm)
~/work/github.com/jarkko/polkavm ~/work/github.com/jarkko/polkavm/guest-programs
[DEBUG polkavm_linker::program_from_elf] section .polkavm_metadata 1
[DEBUG polkavm_linker::program_from_elf] section .polkavm_metadata 0
ERROR: failed to link "guest-programs/target/riscv32emac-unknown-none-polkavm/release/example-hello-world": failed to parse extern metadata: unsupported extern metadata version: '0' (expected '1' or '2')

I sanity checked again with old toolchain that my DWARF changes could not possibly cause any of this and they do not. build-examples.sh works perfectly with all of them. I also checked what happens with benchmarks and with them the version is 1 for both exports.

I looked at also polkavm-derive crate and found the generators and it version 1 is defined for METADATA.

I keep also Friday off work (like I did Wed) to keep my hours in balance so looking into this next Monday.

PS. A more standard convention would to use ELF note section (e.g. .note.polkavm) for this. Just saying this in completeness no strong push ATM :-)

[jarkkojs]

I'll check on Monday what the binary dumps comparing exports section (or whatever it was called) with old/new toolchain (dunno why did not came to mind Thu). I'd consider also leveraging Goblin for ELF parsing in the long run but it is irrelevant for the moment.

Did also some further tests and yeah, DWARF parsing becomes much more sane when having two-passes: subprogram pass and subroutine pass. Also something not relevant for the moment. Since subprograms alone have a few special cases alone it'd be also easier to be robust on those. It neither can slow things down as you do same amount of work in O(n).

[jarkkojs]

zZz and wait for input... I wrote on Sunday (and sort of used half of my Monday, also rebased the patch set and fixed merge conflicts and hit my head wall with this issue under discussion) extra patch that simply allows to optionally pick benchmark with a command-line parameter. It's easier to fix build and linking issues because you don't have to comment out stuff by hand from build-benchmarks.sh.

[jarkkojs]

After fixing up the parameters in build-examples.sh according to the comments:

❯ guest-programs/build-examples.sh 
> Building: 'example-hello-world' (-> output/example-hello-world.polkavm)
~/work/github.com/jarkko/polkavm ~/work/github.com/jarkko/polkavm/guest-programs
ERROR: failed to link "guest-programs/target/riscv32emac-unknown-none-polkavm/release/example-hello-world": failed to parse extern metadata: unsupported extern metadata version: '0' (expected '1' or '2')

So the problem still persists.

[jarkkojs]

Contents of the section .polkavm_metadata

struct PolkaVM_Metadata {
    unsigned char version;
    unsigned int flags;
    unsigned int symbol_length;
    const char * symbol;
    unsigned char input_regs;
    unsigned char output_regs;
} __attribute__ ((packed));

FROM:

     12181 02000000 00100000 00f40001 00000100  ................
     12190 00000000 01000000 000b0000 00040101  ................
     121a0 000201                               ...

TO:

     0000 00020000 00001000 00000000 00000001  ................
     0010 00000000 01000000 000b0000 00000000  ................
     0020 000201                               ...

Contents of the section .polkavm_exports

::core::arch::global_asm!(
    ".pushsection .polkavm_exports,\"R\",@note\n",
    ".byte 1\n", // Version.
    ".4byte {metadata}",
    ".4byte {function}",
    ".popsection\n",
    metadata = sym METADATA,
    function = sym trampoline,
);

FROM:

     0000 01942101 00641101 00                 ..!..d...

TO:

     0000 01000000 00000000 00                 .........

[jarkkojs]

Follow-up: #199

[jarkkojs]

Looking this next time on Monday, if it is still open but I'd hope that since I created follow-up issues and this passes the CI, it'd be a great time to merge it, in order to get a milestone (and move forward with #199).

[jarkkojs]

Please use "rebase and merge" instead of "squash and merge" so that my splits don't go wasted, and will in future useful for git bisect and git blame -s when seeking for failing commits. The default was "squash and merge", thus the remark.

[koute]

Please use "rebase and merge" instead of "squash and merge" so that my splits don't go wasted

If the commits are nice I always use rebase and merge (and in general I personally prefer a rebase-based workflow myself), but FYI this is not true for Parity at large (if you're going to contribute to polkadot-sdk then there the policy is to never rebase, and to always squash).

[jarkkojs]

Please use "rebase and merge" instead of "squash and merge" so that my splits don't go wasted

If the commits are nice I always use rebase and merge (and in general I personally prefer a rebase-based workflow myself), but FYI this is not true for Parity at large (if you're going to contribute to polkadot-sdk then there the policy is to never rebase, and to always squash).

OK cool it was only what Github seems to recommend me as default (again apparently) :-)

[jarkkojs]

I'll revisit the remaining issues on Monday. Took 4 weeks in the end to get here but zero actual technical debt generated (maybe even a bit reduced in details). It somehow exploded from that once additional ld parameter I added to the original JSON (--relocatable).

[jarkkojs]

Also I think Monday would be good time because then the response time any possible issues is fastest (next week I'll be available full 5 days).

[jarkkojs]

I addressed the reported issues so I'll merge this (including #199).