Instruction execution gets incorrect results

[smeng9]

Hi, I hope you had a good weekend.

I have run into some issues in emulation and I have come up with a minimal example that can reproduce the bug. The x86 emulation of following code is not correct, when using either the browser inspect console on https://alexaltea.github.io/unicorn.js/demo.html?arch=x86 or latest library compiled by myself using Emscripten 2.0.9

I have tried official unicorn c and python bindings on version 1.0.1 dating back to 2017 and the results are correct. So I suspect there is something wrong in the unicorn.js, maybe tiny code interpreter or some memory corruption? Would you get a chance to take a look? Thanks!

var addr = 0x10000; var code = [ 0x31, 0xC0, // xor eax, eax 0x89, 0xCB, // mov ebx, ecx ];

// Initialize engine var e = new uc.Unicorn(uc.ARCH_X86, uc.MODE_32);

// Write registers and memory e.reg_write_i32(uc.X86_REG_EAX, 0x456); e.mem_map(addr, 4*1024, uc.PROT_ALL); e.mem_write(addr, code)

// Start emulator var begin = addr; var until = addr + code.length; e.emu_start(begin, until, 0, 1); // Single stepping

// Read registers var r0 = e.reg_read_i32(uc.X86_REG_EAX); // Should be 0x0, but is 0x44, why?

[AlexAltea]

That is incredibly painful to debug, I've run into such issues before (typically after upgrading the Emscripten SDK version). Sometimes updates in Unicorn/Emscripten can cause certain structures to change in size/alignment. When previously aligned members suddenly get misaligned, things start silently failing during memory accesses.

In the past I've addressed this by building with -s SAFE_HEAP=1. Could you try that and see if there's any suspicious warning?

[smeng9]

I highly suspect there is memory corruption caused by Emscripten, because xor ebx, ebx works correctly and it is unlikely to be an issue of tiny code interpreter. However, I did not receive any warning even when -s SAFE_HEAP is turned on. The resulting memory corruption value 0x44 is invariant across different versions of Emscripten from 1.37 till 2.0, so it should be an persistent issue, not something caused by an update.

I'm not very familiar with Emscripten internals, it is really painful for me. Besides debugging flags offered by emcc that did not work this time, is there any other way to catch memory issue?

[AlexAltea]

Another check: Does Unicorn.js built as a native binary have the same issue? You can test this by running patching the sources with build.py but compiling as usual following Unicorn's instructions. Then you can run your PoC natively and see if the same bug appears (if it doesn't then this should be reported to Emscripten).

[smeng9]

Run natively causes segmentation fault. Seems part of the stack is corrupted, missing 7fff

Now I have come up with two patching solutions that may work as a temporary bypass

Force the alignment of struct defined here https://github.com/unicorn-engine/unicorn/blob/4d7193eeb39314b79e654ca1c1f62d911af4aea4/qemu/target-i386/cpu.h#L840 The drawback of this patch is although I got execution correctly this time, we cannot catch all the silent memory issues.

Forcing the alignment of Emscripten by adding -s FORCE_ALIGNED_MEMORY=1 The drawback of this patch is also apparent, because we have to roll Emscripten back before 2015 for some removed feature.

What is your thought on this?

[AlexAltea]

The drawback of this patch is although I got execution correctly this time, we cannot catch all the silent memory issues.

I don't understand the last part of your sentence? What issues wouldn't we catch?

Also, I wonder why an alignment issue causes problems when building it natively. I assume you were run it under a x86 host so unaligned memory accesses should be fine. Can you elaborate more on your first workaround?

Forcing the alignment of Emscripten by adding -s FORCE_ALIGNED_MEMORY=1

It seems it was removed not so long ago: https://github.com/emscripten-core/emscripten/pull/8260 Unfortunately, the issues appear both in native and through Emscripten, so we don't have much leverage to demand this setting to be reenabled.

[smeng9]

Also, I wonder why an alignment issue causes problems when building it natively. I assume you were run it under a x86 host so unaligned memory accesses should be fine. Can you elaborate more on your first workaround?

The first work around is adding an unused large dummy struct, for example target_ulong regs0[CPU_NB_REGS]; before target_ulong regs[CPU_NB_REGS]; so even if the dummy struct gets corrupted the real register values would still be correct.

This patch at least works for the test case I raised.

I don't understand the last part of your sentence? What issues wouldn't we catch?

The issues we can not catch is some other corrupted data structure, because we don't know where and how they get corrupted.

[AlexAltea]

Ok, that sounds like an Out-of-bounds access rather than misaligned access. There must be some bug deep within the TCG frontend (or TCI backend).

I'd bet it's the latter: one of the main motivation for QEMU/Unicorn developers to deprecate/remove the TCI backend is that it's not frequently used (why would anyone choose something that is x10~x100 slower than JIT counterparts). As a result, it has received lest attention, and it's in quite an unmaintainable state right now...

The only realistic solutions I see are:

• Short term: Let's accept that the very nature of Unicorn.js is that of a hacky project. We are porting something that was never meant to be ported to JavaScript. Let's patch it as you suggested: target_ulong regs[CPU_NB_REGS]; -> target_ulong regs[CPU_NB_REGS + 10 /* HACK: OOB access. See issue #25 */];.

• Long term: Add a TCG->WASM backend. There has been several public/private attempts of this AFAIK. But without the upstream support of QEMU developers we will eventually end up with a broken backend like TCI too. We need QEMU developers to accept the backend if we want to get anywhere.

[smeng9]

Closing this issue as the bug is resolved in later versions when unicorn>=1.0.2.