Invalid wasm module produced when combining -fPIC with -O1

[bjorn3]

Reproduction

cat > foo.c <<EOF
struct table {
  int (*f1)(char, int);
  int (*f2)(void);
} functable;

int g(char, int);

void h() {
  struct table t;
  t.f1 = g;
  __atomic_store(&functable.f1, &t.f1, 5);
  __atomic_store(&functable.f2, &t.f2, 5);
}
void i(char buf, int j) {
  h();
  functable.f1(buf, j);
}
EOF
wasi-sdk-24.0-x86_64-linux/bin/clang -fPIC -O1 foo.c -c -o foo.wasm
wabt-1.0.31/bin/wasm-validate foo.wasm

Result

foo.wasm:00000db: error: type mismatch at end of function, expected [] but got [i32]

Version

wasi-sdk-24.0-x86_64-linux/bin/clang -v
clang version 18.1.2-wasi-sdk (https://github.com/llvm/llvm-project 26a1d6601d727a96f4301d0d8647b5a42760ae0c)
Target: wasm32-unknown-wasi
Thread model: posix

26a1d66 (LLVM 18.1.2)

[EugeneZelenko]

@bjorn3: Could you please try 19 or main branch?

[llvmbot]

@llvm/issue-subscribers-backend-webassembly

Author: None (bjorn3)

# Reproduction

cat > foo.c <<EOF
struct table {
  int (*f1)(char, int);
  int (*f2)(void);
} functable;

int g(char, int);

void h() {
  struct table t;
  t.f1 = g;
  __atomic_store(&functable.f1, &t.f1, 5);
  __atomic_store(&functable.f2, &t.f2, 5);
}
void i(char buf, int j) {
  h();
  functable.f1(buf, j);
}
EOF
wasi-sdk-24.0-x86_64-linux/bin/clang -fPIC -O1 foo.c -c -o foo.wasm
wabt-1.0.31/bin/wasm-validate foo.wasm

Result

foo.wasm:00000db: error: type mismatch at end of function, expected [] but got [i32]

Version

wasi-sdk-24.0-x86_64-linux/bin/clang -v
clang version 18.1.2-wasi-sdk (https://github.com/llvm/llvm-project 26a1d6601d727a96f4301d0d8647b5a42760ae0c)
Target: wasm32-unknown-wasi
Thread model: posix

26a1d66 (LLVM 18.1.2)

[aheejin]

The issue seems to be still present in the ToT LLVM.

$ wasm-objdump -d foo.o

foo.o:  file format wasm 0x1

Code Disassembly:

0000a0 func[1] <h>:
 0000a1: 23 80 80 80 80 00          | global.get 0 <env.__memory_base>
 0000a7: 41 80 80 80 80 00          | i32.const 0
 0000ad: 6a                         | i32.add
 0000ae: 23 81 80 80 80 00          | global.get 1 <GOT.func.g>
 0000b4: 36 02 00                   | i32.store 2 0
 0000b7: 0b                         | end

0000b9 func[2] <i>:
 0000ba: 23 80 80 80 80 00          | global.get 0 <env.__memory_base>
 0000c0: 41 80 80 80 80 00          | i32.const 0
 0000c6: 6a                         | i32.add
 0000c7: 23 81 80 80 80 00          | global.get 1 <GOT.func.g>
 0000cd: 36 02 00                   | i32.store 2 0
 0000d0: 20 00                      | local.get 0
 0000d2: 20 01                      | local.get 1
 0000d4: 10 81 80 80 80 00          | call 1 <h>
 0000da: 1a                         | drop
 0000db: 0b                         | end

So h is inlined into i, and call 1 should point to g, but it somehow is pointing to h incorrectly.
Any ideas? @sbc100

[sbc100]

@aheejin is foo.o above the object file of the linked wasm file?

[sbc100]

The problem looks like that wrong relocation type is being used. If we look at the object files we see:

 0000d2: 10 80 80 80 80 00          | call 0 <env.g>
           0000d3: R_WASM_GLOBAL_INDEX_LEB 1 <env.g>

So the relocation type is asking for a global index, not a functions index. The relocation type for a call instruction should be R_WASM_FUNCTION_INDEX_LEB.

For PIC code I don't think we should ever be generating direct calls like this since function indexes are not known until runtime, all calls should be indirect, so actually I think the relocation type is correct but it should be global.get 1 <GOT.func.g> + call_indirect not a direct call.

[TerrorJack]

For PIC code I don't think we should ever be generating direct calls like this since function indexes are not known until runtime

I don't get it; function pointers are indeed unknown at link-time due to table_base, but the function indices should already be known at link-time, since they are used by direct calls in the same shared library?

[sbc100]

For PIC code I don't think we should ever be generating direct calls like this since function indexes are not known until runtime

I don't get it; function pointers are indeed unknown at link-time due to table_base, but the function indices should already be known at link-time, since they are used by direct calls in the same shared library?

Sorry you are correct. Using R_WASM_FUNCTION_INDEX_LEB is ok for functions that are known to be dll local.

The problem remains though that R_WASM_GLOBAL_INDEX_LEB should never be used with a direct call. R_WASM_GLOBAL_INDEX_LEB is only for indirect calls, so its still a miscompile of some kind.