cranelift-module should document that finalize_definitions needs to be called before finish

[jyn514]

• What are the steps to reproduce the issue? Can you include a CLIF test case,
  ideally reduced with the bugpoint clif-util command?

1. git clone https://github.com/jyn514/rcc && cd rcc
2. git checkout cranelift-object
3. cargo run <<< 'int i = 1; int main(void) { return i; }'; ./a.out; echo $?

• What do you expect to happen? What does actually happen? Does it panic, and
  if so, with which assertion?

I expect this to return 1. Instead it returns (on my machine) 64.
I feel reasonably confident this is not a bug in my code because the exact same code works fine using cranelift-faerie (you can reproduce by checking out the master branch).
CLIF (can reproduced with cargo run -- --debug-asm):

function u0:0() -> i32 system_v {
    gv0 = symbol colocated u1:0

ebb0:
    v0 = global_value.i64 gv0
    v1 = load.i32 v0
    return v1
}

Generated assembly. Note how the lea points to the next instruction which is not correct.

00000000000005fa <main>:
 5fa:	40 55                	rex push %rbp
 5fc:	48 89 e5             	mov    %rsp,%rbp
 5ff:	48 8d 05 00 00 00 00 	lea    0x0(%rip),%rax        # 606 <main+0xc>
 606:	40 8b 00             	rex mov (%rax),%eax
 609:	40 5d                	rex pop %rbp
 60b:	c3                   	retq   
 60c:	0f 1f 40 00          	nopl   0x0(%rax)

• Which Cranelift version / commit hash / branch are you using?

cranelift-object 0.51.0 (git+https://github.com/bytecodealliance/cranelift#ec787eb281bb2e18e191508c17abe694e91f0677)

• If relevant, can you include some extra information about your environment?
  (Rust version, operating system, architecture...)

Rust version: rustc 1.39.0 (4560ea788 2019-11-04)
System info: Linux 5.0.0-37-generic on Ubuntu 18.04 x86_64 with glibc.

[jyn514]

I know the CLIF is not super helpful but I'm not sure how to show the data/global objects I've sent to cranelift. If anyone knows, I would love to add that!

[jyn514]

Note that I've reproduced this bug on Windows as well so it's not something specific to the object file format.

[bjorn3]

What are the relocations? (objdump -rd/readelf -R=.text)

[jyn514]

I'm not sure there are any ...

$ objdump -rd object-bug.o 

object-bug.o:     file format elf64-x86-64


Disassembly of section .text:

0000000000000000 <main>:
   0:	40 55                	rex push %rbp
   2:	48 89 e5             	mov    %rsp,%rbp
   5:	48 8d 05 00 00 00 00 	lea    0x0(%rip),%rax        # c <main+0xc>
   c:	40 8b 00             	rex mov (%rax),%eax
   f:	40 5d                	rex pop %rbp
  11:	c3                   	retq   

Here's the symbol table if that helps:

$ objdump -t
SYMBOL TABLE:
0000000000000000 g    df *ABS*	0000000000000000 <stdin>
0000000000000000 g     O .data	0000000000000004 i
0000000000000000 g     F .text	0000000000000012 main

[jyn514]

For comparison, this is an object file emitted by clang for the same code:

$ objdump -rd
0000000000000000 <main>:
   0:	55                   	push   %rbp
   1:	48 89 e5             	mov    %rsp,%rbp
   4:	c7 45 fc 00 00 00 00 	movl   $0x0,-0x4(%rbp)
   b:	8b 04 25 00 00 00 00 	mov    0x0,%eax
			e: R_X86_64_32S	i
  12:	5d                   	pop    %rbp
  13:	c3                   	retq 
$ objdump -t
SYMBOL TABLE:
0000000000000000 l    df *ABS*	0000000000000000 -
0000000000000000 l    d  .text	0000000000000000 .text
0000000000000000 g     O .data	0000000000000004 i
0000000000000000 g     F .text	0000000000000014 main

And a file emitted by cranelift-faerie:

$ objdump -rd
0000000000000000 <main>:
   0:	40 55                	rex push %rbp
   2:	48 89 e5             	mov    %rsp,%rbp
   5:	48 8d 05 00 00 00 00 	lea    0x0(%rip),%rax        # c <main+0xc>
			8: R_X86_64_PC32	.data.i-0x4
   c:	40 8b 00             	rex mov (%rax),%eax
   f:	40 5d                	rex pop %rbp
  11:	c3                   	retq   
$ objdump -t
SYMBOL TABLE:
0000000000000000 l    df *ABS*	0000000000000000 <stdin>
0000000000000000 l    d  .text.main	0000000000000000 .text.main
0000000000000000 l    d  .data.i	0000000000000000 .data.i
0000000000000000 g     F .text.main	0000000000000012 main
0000000000000000 g     O .data.i	0000000000000004 i

[bjorn3]

Strange. Never had problems with cranelift-object myself.

[jyn514]

I didn't call Module::finalize_definitions: https://github.com/bytecodealliance/cranelift/blob/master/cranelift-object/src/backend.rs#L369. This could use more documentation, I didn't realize I needed to.

[philipc]

It's possible that I've misunderstood cranelift-module, and cranelift-object is wrong to be doing the relocations in finalize_definitions. Maybe it should do them all in finish (or at least do them if they haven't been done yet). I was copying what cranelift-simplejit did, but there's a bit of a mismatch between the usage of cranelift-simplejit and cranelift-object in that cranelift-simplejit provides the usable results after finalize_definitions is called, and finish is only for cleanup, whereas cranelift-object doesn't return a usable result until finish is called.

[jyn514]

In my PR, I just called finalize_definitions automatically in cranelift-module if it hadn't already been called. That takes the responsibility off both the end-user and the implementors of Backend.