bytecodealliance / wasm-micro-runtime

WebAssembly Micro Runtime (WAMR)
Apache License 2.0
4.78k stars 605 forks source link

spec test on nuttx is broken #2312

Closed Zzzabiyaka closed 1 year ago

Zzzabiyaka commented 1 year ago

https://github.com/bytecodealliance/wasm-micro-runtime/actions/workflows/spec_test_on_nuttx.yml

  expected: 'SystemExit(1)'
  got: 'iwasm --heap-size=0 -v=0 --repl /tmp/tmplr32ph0k.aot
AOT module load failed: resolve symbol stack_sizes failed
nsh> '
 got: 'iwasm --heap-size=0 -v=0 --repl /tmp/tmp7vdu4b44.aot
AOT module load failed: cannot apply relocation to text section for aot file generated with "--enable-indirect-mode" flag
nsh> '

@wenyongh @no1wudi for visibility

no1wudi commented 1 year ago

Seems it's broken after https://github.com/bytecodealliance/wasm-micro-runtime/pull/2244 ? Could you take a look ? @yamt

wenyongh commented 1 year ago

The log is like below:

Running: ../../../wamr-compiler/build/wamrc --target=riscv32 --target-abi=ilp32 --cpu=generic-rv32 --cpu-features=+m,+a,+c --disable-simd --format=llvmir-opt --disable-llvm-lto -o /tmp/tmp37vce71_.aot /tmp/tmpdh1n6wgu.wasm
Create AoT compiler with:
  target:        riscv32
  target cpu:    generic-rv32
  target triple: riscv32-pc-linux-ilp32
  cpu features:  +m,+a,+c
  opt level:     3
  size level:    3
  output format: optimized LLVM IR
Compile success, file /tmp/tmp37vce71_.aot was generated.
Leaving tempfiles: ['/tmp/tmpw34_jqtz.wast', '/tmp/tmpdh1n6wgu.wasm']
==================== LAST LOG END ====================
============> run bulk failed with a non-zero return code 101
==================== LAST LOG of bulk ====================
  expected: 'SystemExit(1)'
  got: 'iwasm --heap-size=0 -v=0 --repl /tmp/tmpxfng3wkc.aot
AOT module load failed: resolve symbol stack_sizes failed

Seems there is something wrong in handling the stack_sizes global array when the target is riscv.

yamt commented 1 year ago

i will take a look tomorrow.

yamt commented 1 year ago

the code in question tries to avoid requiring the symbol a bit. https://github.com/bytecodealliance/wasm-micro-runtime/blob/ac9e789951de1a62a396e080cbf529fb81a96967/core/iwasm/compilation/aot_llvm.c#L1425-L1428

but riscv still seems to need the symbol.

000000e8 <aot_func#0>:
      e8: 13 01 01 ff   addi    sp, sp, -16
      ec: 23 26 11 00   sw      ra, 12(sp)

000000f0 <.LBB1_3>:
      f0: 97 05 00 00   auipc   a1, 0
                        000000f0:  R_RISCV_PCREL_HI20   stack_sizes
      f4: 93 85 05 00   mv      a1, a1
                        000000f4:  R_RISCV_PCREL_LO12_I .LBB1_3
      f8: 83 a5 05 00   lw      a1, 0(a1)
      fc: 03 26 05 01   lw      a2, 16(a0)

as our aot format doesn't seem to have a symbol table, probably it's simplest to put the table into a dedicated data section and assume the offset is 0. how do you think?

also, i guess we need to implement these riscv relocation types. (R_RISCV_PCREL_HI20 and R_RISCV_PCREL_LO12_I)

does it make sense? as i'm not familiar with riscv, i want to ask an opinion from someone familiar with riscv before trying to implement a solution.

yamt commented 1 year ago

the same symptom for armhf too.

wenyongh commented 1 year ago

as our aot format doesn't seem to have a symbol table, probably it's simplest to put the table into a dedicated data section and assume the offset is 0. how do you think?

@yamt Yes, I think it should be good to put the global array into a dedicated section, like what wamrc --enable-llvm-pgo does:

wamrc --enable-llvm-pgo --format=llvmir-opt -o test.ll test.wasm

The LLVM IRs generated are like:

@"__profc_aot_func#0" = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat, align 8
@"__profd_aot_func#0" = private global { i64, i64, i64, ptr, ptr, i32, [2 x i16] } { i64 4432924599349043253, i64 742261418966908927, i64 sub (i64 ptrtoint (ptr @"__profc_aot_func#0" to i64), i64 ptrtoint (ptr @"__profd_aot_func#0" to i64)), ptr @"aot_func#0", ptr null, i32 1, [2 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($"__profc_aot_func#0"), align 8

also, i guess we need to implement these riscv relocation types. (R_RISCV_PCREL_HI20 and R_RISCV_PCREL_LO12_I)

does it make sense? as i'm not familiar with riscv, i want to ask an opinion from someone familiar with riscv before trying to implement a solution.

Not sure, but we can have a try, the R_RISCV_HI20/R_RISCV_LO12_I/R_RISCV_LO12_S have already been implemented, R_RISCV_PCREL_HI20/R_RISCV_PCREL_LO12_I may be similar to them, and we can also refer to document here: https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#relocations

Seems that R_RISCV_HI20 is to replace the HI20 (20 bits) of the address (32 bits) to patch with symbol_addr + reloc_addend (S + A), while R_RISCV_PCREL_HI20 is to replace the HI20 with symbol_addr + reloc_addend - Position of the relocation (S + A - P).

yamt commented 1 year ago

ok. i will put it in a dedicated section first. (i suppose this alone is enough to fix armhf) and then try to implement those riscv relocation types.

yamt commented 1 year ago

AOT module load failed: cannot apply relocation to text section for aot file generated with "--enable-indirect-mode" flag

for this one, i filed a separate issue: https://github.com/bytecodealliance/wasm-micro-runtime/issues/2316