Failure to link due to unsupported instruction Add64

[cmichi]

I've got a Rust program that builds fine, but during polkavm_linker::program_from_elf I get this error:

R_RISCV_LO12_I for an unsupported instruction: 0x2118452243 (RegImm { kind: Add64, dst: S0, src: A0, imm: 2020 }) (at <section #11+16 ('.text': '_ZN3ink7codegen8dispatch9execution12deny_payment17h59580f872247d6ebE'+16)>)

The respective program is built using

cargo build 
  --target=path/to/riscv64emac-unknown-none-polkavm.json 
  --release 
  -Zbuild-std=core,alloc 
  -no-default-features 
  -Zbuild-std-features=panic_immediate_abort

I tried different recent Rust versions, that didn't have an impact. It's the same with polkavm 0.21 and the latest polkavm/master.

I checked the origin of the error and it's due to an unimplemented match arm for Add64.

I've put the binary here.

You'll get a panic when attempting to link it:

// $ INPUT=/path/to/reproduce-binary cargo run

fn main() {
    let input_path = std::env::var("INPUT").expect("no INPUT in env");
    let config = polkavm_linker::Config::default();
    let orig = std::fs::read(input_path).expect("Failed to read {input_path:?}");
    let _linked = polkavm_linker::program_from_elf(config, orig.as_ref())
        .unwrap();
}

The program is complex, and breaking it down to a more easily digestible reproducer will be complex. So before doing it, I wanted to create this issue to see if it already gets you somewhere. Alternatively, I can also provide you with the project to build.

[aman4150]

It is a missing relocation handling of R_RISCV_LO12_I with Add64 instruction. I'll post a fix.

Thanks for reporting this @cmichi!

[cmichi]

Thanks for the quick reaction, @aman4150! It's really helpful for us to have such a quick turnaround cycle.

[cmichi]

@aman4150 The reproducer that I linked above still fails, the error now is ProgramFromElfError(Other("R_RISCV_LO12_I with a zero destination register")).

I get the error locally when recompiling the Rust program + relinking it with the latest master. It also happens with the binary linked above. Could you take another look?

[cmichi]

@aman4150 Did you see my message above? Should we reopen this issue or should I create a new one?

[aman4150]

Sorry, I missed the previous message.

The reproducer that I linked above still fails, the error now is ProgramFromElfError(Other("R_RISCV_LO12_I with a zero destination register")).

I see, that's odd your destination register is zero. I'll reopen the issue and have a look at this tomorrow.

[cmichi]

@aman4150 Thanks! If you have suggestions for how to work around it that would also be cool.

I found that downgrading to polkavm-linker = 0.19 enables us to link successfully, but we can't interact anymore with polkadot-sdk then (as it's already on a later version).

[aman4150]

Hi @cmichi, I confirmed I can link the reproducer binary you've provided with the latest version of the linker.

Also, it is unlikely that compiler would emit a zero destination register, because then there is no point of relocation.

Could you please apply this patch and run it again? It will give us pointers where the linker fails:

diff --git a/crates/polkavm-linker/src/program_from_elf.rs b/crates/polkavm-linker/src/program_from_elf.rs
index 6efa086..b1cebbe 100644
--- a/crates/polkavm-linker/src/program_from_elf.rs
+++ b/crates/polkavm-linker/src/program_from_elf.rs
@@ -8494,7 +8494,11 @@ where
                                 imm: _,
                             } => {
                                 let Some(dst) = cast_reg_non_zero(dst)? else {
-                                    return Err(ProgramFromElfError::other("R_RISCV_LO12_I with a zero destination register"));
+                                    // return Err(ProgramFromElfError::other("R_RISCV_LO12_I with a zero destination register"));
+                                    return Err(ProgramFromElfError::other(format!(
+                                        "R_RISCV_LO12_I with a zero destination register: 0x{inst_raw:08} ({inst:?}) (at {loc})",
+                                        loc = current_location.fmt_human_readable(elf),
+                                    )));
                                 };
 
                                 InstExt::Basic(BasicInst::LoadAddress { dst, target })

[cmichi]

Hi @cmichi, I confirmed I can link the reproducer binary you've provided with the latest version of the linker.

You mean that you are not getting the error with polkavm-linker/master?

I applied the patch above, but the output didn't change. I then found that the error is triggered here.

When I apply your patch there I get:

ProgramFromElfError(Other("R_RISCV_LO12_I with a zero destination register: 0x2235940867 (Load { kind: U8, dst: Zero, base: A1, offset: -1964 }) (at <section #201+14 ('.text': '_ZN5alloc5alloc6Global10alloc_impl17h48ae9df3e65ff38dE'+14)>)"))

[cmichi]

@aman4150 It indeed works with the binary I put above for the original bug. Sorry about that, here's the one that fails for zero destination register: https://github.com/use-ink/polkavm/raw/9d705fd7b050b3ebeb2cf89ee9b1511ca1bbb08c/examples/trigger-bug/reproducer-zero-reg.

[aman4150]

This is the faulting instruction sequence:

   13492:	000165b7          	lui	a1,0x16
			13492: R_RISCV_HI20	__rust_no_alloc_shim_is_unstable
   13496:	8545c003          	lbu	zero,-1964(a1) # 15854 <__rust_no_alloc_shim_is_unstable>
			13496: R_RISCV_LO12_I	__rust_no_alloc_shim_is_unstable

It suggests the code is trying to load from invalid (0x0) address. Which sounds like a bug in the code. Could you please share the original code that generates this sequence?

[koute]

It suggests the code is trying to load from invalid (0x0) address. Which sounds like a bug in the code. Could you please share the original code that generates this sequence?

That's not a load from an invalid address; that's a load into the zero register, i.e. a dummy load. I've seen this before; some of LLVM's optimization passes sometimes rewrite the destination register to zero and leave it for some reason. See Inst::Load handling in convert_instruction.

[cmichi]

@aman4150 I guess the original code is not needed then?

[aman4150]

right, I misspoke, it's a load into the zero register, which didn't make sense to me.

Thanks for confirming you've seen this before @koute

@cmichi, this should fix the problem:

diff --git a/crates/polkavm-linker/src/program_from_elf.rs b/crates/polkavm-linker/src/program_from_elf.rs
index 6efa086..ae9eab7 100644
--- a/crates/polkavm-linker/src/program_from_elf.rs
+++ b/crates/polkavm-linker/src/program_from_elf.rs
@@ -8505,11 +8505,11 @@ where
                                 base: _,
                                 offset: _,
                             } => {
-                                let Some(dst) = cast_reg_non_zero(dst)? else {
-                                    return Err(ProgramFromElfError::other("R_RISCV_LO12_I with a zero destination register"));
-                                };
-
-                                InstExt::Basic(BasicInst::LoadAbsolute { kind, dst, target })
+                                if let Some(dst) = cast_reg_non_zero(dst)? {
+                                    InstExt::Basic(BasicInst::LoadAbsolute { kind, dst, target })
+                                } else {
+                                    InstExt::nop()
+                                }
                             }
                             _ => {
                                 return Err(ProgramFromElfError::other(format!(

I'll post a PR in some time.