hyc you're right, this simple program works fine: paste.debian.net/hidden/fb9a8aea

and if you comment the second sys_icache_invalidate, it starts to output random numbers in the second printf

I suspect we have an undefined behavior somewhere in the code, and compiler takes "advantage" of it :D

well, ubsan did find something: paste.debian.net/hidden/70411197

fixing it didn't help

lol, it produces different hashes even if I just run JIT compiler once and then just call the generated program repeatedly :D

yep, the code is generated only once, write protection is on but hashes still change occasionally: paste.debian.net/hidden/cb3e2e49

magic

I think the hash gets wrong when the thread is rescheduled to another CPU core. If I increase RANDOMX_PROGRAM_ITERATIONS, I get more incorrect hashes. If I decrease it 4 times, all hashes are corrrect

I think it's an unaligned memory read or write somewhere in the JIT code. M1 doesn't segfault on this, it just silently reads garbage :D

hyc selsta "On AArch64 X18 is used as a platform register for some platforms, so to generate portable executables it should not be used by the compiler"

lol

RandomX aarch64 code uses it of course

I guess new macOS SDK uses it too and changes it

I'm not sure what's happening, but it looks like x18 can change right during the program execution. And it's not related to JIT at all.

This program produces different values: paste.debian.net/hidden/03d0943c (nop instructions are to get correct timings for the bug to happen)

Even without JIT, when I call it directly

If I change x18 to x10 in the program code, everything starts working magically

I was wrong, the previous paste is the working code

paste.debian.net/hidden/03d0943c works

paste.debian.net/hidden/51dbf4fa has bug

find 2 differences :D

Now I have to rewrite the whole aarch64 JIT to not use x18, that's unfortunate

seems Gimp had the same issue when porting to M1 Mac

also interesting that this didn't trigger on any other aarch64 system yet, seems modern Android also has x18 reserved

it was documented here, since we thought it's JIT related we kept looking at the wrong documentation developer.apple.com/documentation/x…ting-arm64-code-for-apple-platforms

yes, but now I need to rewrite the JIT

It still fails now, but at least it's consistent and in one place. So probably some other bug somewhere in the rewritten code.

*and fails in the same place

github.com/SChernykh/RandomX/tree/fix-x18 - the randomness went away, it always fails at "[85] Hash test 2b (compiler)" now

Probably some RandomX instruction is not fixed properly

But Hash test 2a (compiler) always passes, that's a good sign

I'll come back to it in the evening, exhausted now. Maybe hyc or tevador can spot the bug in the meantime.

also fails at 2b here at 300a0adb47603dedb42228ccb2b211104f4da45af709cd7547cd049e9489c969

selsta I fixed it: github.com/SChernykh/RandomX/blob/f…it_compiler_a64.cpp#L486C34-L486C34

It used x18 and x19 before, then I changed it to x20, x21 but it should've been x19, x20

Now it passes all tests

<p​lowsof:matrix.org> 8-D

You can make fresh git clone then git checkout fix-18 and try it

All tests PASSED

nice :D

hashes match in benchmark, yay paste.debian.net/hidden/ef3cf135

can you try to run the benchmark too?

./randomx-benchmark --mine --init 8 --threads 8 --jit

Finally: tevador/RandomX #281

nice

So it wasn't JIT after all :D

C/C++ CI / build-alpine (aarch64, latest-stable) fails, is this new?

Interesting

It crashed in dataset initialization

But it doesn't crash on M1, weird

wtf

It looks like I PR'd wrong code

no, all good on M1

well, now I have to debug it in qemu because M1 works

selsta I fixed it, all green now

once this is merged 4 issues can be closed

will retest once I'm home

nice work sech1

yeah

but it's weird

asm code saved and restored register x18

it's almost like some kernel interrupt uses it, so it can change in the middle of a function

whatever, if Apple say "no touch" that's what we do :D

;)

my faith in JIT restored

:)

this bug resulted in so much weird behavior lol

even with the old SDK it would crash when started as a detached process

glad all of this is fixed now and I don't have to constantly change the SDK

Apple docs also mention register x29, but it's only used to restore nice callstacks, so we can ignore it

p2pool and xmrig also suffered from it (I found reported issues on Github)

ok, yeah randomx-tests passed here on my M1

randomx-benchmark is more relevant, it computes 1000 hashes

tevador tevador/RandomX #281

I'm still on older SDK which never crashed before

1000 nonces ran fine here

I tested with newer SDK where it crashed or produced invalid hashes without the fix

but I guess now that we have this nailed I feel safe in upgrading my machine ;)

btw I think we have some unnecessary icache flushes in the code

it flushes cache twice

probably we added extra in prior attempts to kill the bug

The one in setPagesRX should be enough

I'll remove them later in a separate PR

Actually, __builtin___clear_cache in setPagesRX only works on macOS, so __builtin___clear_cache in JIT compiler is still needed :D

so it was a RandomX bug after all

yes

It's even mentioned here: en.wikipedia.org/wiki/Calling_convention#ARM_(A64)

But I didn't pay attention, I though that saving and restoring it before returning is enough

But it's actually a "volatile" register

I wouldn't call it a bug. The code correctly saved and restored x18, it was just not designed to be interrupted by kernel (or whatever else) and x18 being different after the interrupt

btw this explains why it works with SDK < 13: github.com/apple-oss-distributions/…sfmk/arm64/machine_task.c#L279-L286

Even in this code: paste.debian.net/hidden/51dbf4fa where x18 is read right after it's written, it can still fail

The kernel has an explicit exception for it

gawd

hmm patchwork.kernel.org/project/linux-…4.19528-8-ard.biesheuvel⊙lo

so it's used in task switching

that explains it

then it's indeed a volatile register from used code's point of view

*user code

yes, so you'll get wrong hashes if there is a context switch in the middle of a program run

if you wrote that register you prob wouldn't survive a context switch

No, that would be a potential kernel exploit :D

Kernel initializes it before using it

it will probably just store some garbage in the register

if all it did was corrupt a hash calc, why did users get SEGVs then

because old RandomX code used x18 in dataset loads

if x18 was used to access memory in RandomX code, it could cause a segfault

x18 was used everywhere, it was a "temporary register"

ah. very temporary.

most places didn't access memory, so they would just cause an invalid hash

but a couple places used x18 for memory loads

you can just check the patch

so if there was a context switch between setting x18 and reading memory, boom

reminds me of my RISC-V trouble with the x4 register, but it's used for a different purpose there

yes, here: tevador/RandomX #281/files#diff-7d6ee9862e6ac45d68d215a25c3fad24c846de3b0c30e923ff2819677e929cbaL325

boom

aha

next release we can add both risc-v support and fix macOS JIT

great

I didn't get risc-v gitian working yet but it's ready on RandomX side

since #281 is merged, shouldn't #262 be closed? it didn't auto-close because it wasn't mentioned in commit msg

yes

UB sanitizer also found a couple of unaligned memory reads in aarch64 JIT, but I'll fix it in a separate PR tomorrow

low priority because it's accessing 8 bytes which are only 4-byte aligned. But all current ARM CPUs don't mind it

cool