Clean up IA32 Assembler's code_

[ghost]

assembler_ia32.h has a field, code_, which is not present in any of the other assemblers:

sdk/runtime/vm/compiler/assembler/assembler_ia32.h

Line 1093 in 618db8a

Object& code_;

It is only read in one place:

sdk/runtime/vm/compiler/assembler/assembler_ia32.cc

Lines 2699 to 2704 in 618db8a

	void Assembler::PushCodeObject() {
	ASSERT(IsNotTemporaryScopedHandle(code_));
	AssemblerBuffer::EnsureCapacity ensured(&buffer_);
	EmitUint8(0x68);
	buffer_.EmitObject(code_);
	}

And set in one place:

sdk/runtime/vm/object.cc

Lines 17254 to 17256 in 618db8a

	#ifdef TARGET_ARCH_IA32
	assembler->GetSelfHandle() = code.ptr();
	#endif

Note how we're setting the field through a getter:

sdk/runtime/vm/compiler/assembler/assembler_ia32.h

Line 1046 in 618db8a

Object& GetSelfHandle() const { return code_; }

This is already fishy, but this quirk is also causing a different bug, whereby we incorrectly set the Code object in Dart frames to the current Code, instead of the Code being called.

For example, in FfiCallInstr we commonly do the following across all architectures:

sdk/runtime/vm/compiler/backend/il_ia32.cc

Lines 1039 to 1041 in 618db8a

	// We need to create a dummy "exit frame". It will have a null code object.
	__ LoadObject(CODE_REG, Object::null_object());
	__ EnterDartFrame(0);

Except, on IA32 EnterDartFrame calls the above PushCodeObject which we saw always pushes whatever is in code_, and not what is in CODE_REG, as the comment otherwise claims (which is correct on other archs.):

sdk/runtime/vm/compiler/assembler/assembler_ia32.cc

Lines 2706 to 2709 in 618db8a

	void Assembler::EnterDartFrame(intptr_t frame_size) {
	EnterFrame(0);
	PushCodeObject();

This ultimately has the effect that FFI ExitFrames have a duplicate FfiTrampoline Code object in them where they should have Null, which in turn affects stack traces, etc.

Aside: This might also affect other users of EnterDartFrame, but I haven't checked.

[ghost]

This also affects all users of EnterStubFrame:

sdk/runtime/vm/compiler/assembler/assembler_ia32.cc

Lines 2732 to 2734 in 618db8a

	void Assembler::EnterStubFrame() {
	EnterDartFrame(0);
	}

Such as:

sdk/runtime/vm/compiler/stub_code_compiler_ia32.cc

Lines 70 to 72 in 618db8a

	__ movl(CODE_REG,
	Address(THR, target::Thread::call_to_runtime_stub_offset()));
	__ EnterStubFrame();

Which clearly doesn't do the same as it does in the other architectures.

[mkustermann]

... we incorrectly set the Code object in Dart frames to the current Code ...

There may be a misunderstanding here. The code object in a frame is representing the code object of that frame's function (not the caller's code object).

On most of our architectures (x64, arm, arm64) the caller is populating this code object as part of the call sequence: In order to call a function, the caller has to load the Functions Code and from there the Codes entrypoint address. The caller therefore has to load the code object anyway, so it loads it into the specified CODE_REG register - which makes it available to the callee (i.e. the callee can simply push CODE_REG - which is its own code object).

Now on ia32 we do not use code objects as such. Callers (e.g. a static call) will invoke its target directly - no need to get the entrypoint from a code object. The callee therefore has to get it's own code object in a different way, namely in the way you described above.

This is by-design.