Live data from Hacker News

We found a bug in Go's ARM64 compiler

blog.cloudflare.com

81–90 of 146 posts

Re: We found a bug in Go's ARM64 compiler

#81
post #13

What ARM64 machines are you using and what are they used for? Last year you were announcing Gen 12 servers on AMD EPYC ( https://blog.cloudflare.com/gen-12-servers/ ), but IIRC there weren’t any mentions of ARM64. But now it seems you’re running ARM64 in full production.

I seem to recall Cloudflare hosts their some of their non-edge compute on public clouds? Like control plane stuff. Could be that.

Re: We found a bug in Go's ARM64 compiler

#82
post #24
post #3

That's an incredible find and once I saw the assembly I was right along with them on the debug path. Interestingly it doesn't need to be assembly for this to work, it's just that that's where the split was. The IR could've done it, it just doesn't for very good reasons. So another win for being able to read arm assembly. Unsure if this would be another way to do it but to save an instruction at the cost of a memory a…

You would normally use the “LDR Rd, =expr” pseudo-instruction form [1]. For immediates not directly constructible, it puts a copy of the immediate value in a PC-relative memory location, then does a PC-relative load into register. So that would turn the whole sequence of “add constant to SP” into 2 executable instructions, 1 for constructing immediate and 1 for adding for a total of 8 bytes, and a 4 byte data area fo…

I've usually seen compilers handle large constants with MOV/MOVK sequences (encoding 16 bits of data per 32-bit instruction) instead of loading them from memory. Loading from memory was more common on 32-bit ARM.

Re: We found a bug in Go's ARM64 compiler

#83
post #74

Earlier quoted context omitted.

I'm a little surprised that this bug wasn't fixed in the assembler as a special case for immediate adds to RSP. If the patch was to the compiler only, other instances of the bug could be lurking out there in aarch64 assembly code.

Would that be wise? The implemented solution uses a temporary register to hold the full value being added to rsp. I don't know enough about how people use the go assembler, but I imagine it would be very surprising if `add $imm, rsp, rsp` clobbered an unrelated register when `$imm` is large enough. Especially since what's clobbered is the designated "temporary register", which I imagine is used all the time in handwr…

Some architectures, and I believe aarch64 is one, have scratch registers reserved for being clobbered in special situations required by the assembler.

Re: We found a bug in Go's ARM64 compiler

#85
post #23
post #15

Earlier quoted context omitted.

I've never heard of that rule (though tbh I'm not allocating > 64KB of stack when I'm in assembly) and it seems Google hasn't either. While I'm sure it makes sense, I don't think I've ever seen that be enforced. At least in C/C++. Maybe it makes more sense for these stack inspecting garbage collectors but I've also heard of ones that just scan the stack without unwinding anything. I did a test asking Google's AI to g…

Did you compile with optimisations? I think GCC will do a bunch of activity on the stack with -O0, but it'll generally coalesce everything into one push/pop per function with optimisations (not because of any rule, but just because it's faster). alloca and other dynamic stack allocation may break this, but normal variables should in pretty much all just get turned into one block on the stack (with appropriate re-use…

It will generate code to touch each page of the stack, because otherwise a very large stack allocation controlled by users (eg, in the case of a variable sized array) can be turned into a pointer to any location in memory by an attacker. Faulting in each page of the stack turns that into a crash.

There was a userspace thread library I came across a long time ago that used variable length arrays to switch between thread stacks; the scheduler would allocate an array of the right size to bump the stack pointer to the different thread's stack.

Re: We found a bug in Go's ARM64 compiler

#86
post #74

Earlier quoted context omitted.

Would that be wise? The implemented solution uses a temporary register to hold the full value being added to rsp. I don't know enough about how people use the go assembler, but I imagine it would be very surprising if `add $imm, rsp, rsp` clobbered an unrelated register when `$imm` is large enough. Especially since what's clobbered is the designated "temporary register", which I imagine is used all the time in handwr…

Some architectures, and I believe aarch64 is one, have scratch registers reserved for being clobbered in special situations required by the assembler.

Not really, or at least not that I know if in the case of arm64. What you have is calling conventions that specify what one function/procedure/whatever can expect both from the caller and the callee's side.n I.e. some registers are caller-saved, some are callee-saved, which basically means the called function can treat them as "scratch".

Additionally, they call out interactions with the OS/execution environment. For example, x18 is the "platform register", and it's unspecified what the OS does with it. It's entirely possible that it clobbers it on context switch or during an interrupt or whatever. So don't use that one unless you have a contract with the OS itself.

But locally, i.e. "from instruction to instruction", no such convention exists to my knowledge, and you probably don't want to have registers that pseudo-instructions might trash inadvertently in general, because it means you can't optimally use these registers.

It's possible for pseudo-instructions or generally macros to be documented as, e.g., "this macro uses x3 as a temporary register and trashes it", but in my experience most macros that need additional temporary registers actually ask you to specify them as part of the macro invocation.

E.g. suppose you have a macro "weirdhash" that takes two registers and saves some kind of hash of them in a third register, but that also needs an extra register to perform its work. You would call it with:

    weirdhash x9, x10, x11, x0
Where x0 would be the scratch register you don't care about.

Re: We found a bug in Go's ARM64 compiler

#87
post #52
post #44

Earlier quoted context omitted.

> Nobodys fault really, but bad results ensued. Uh, the fault is entirely in writing an assembler _that is not an assembler_, but rather something that is _almost_ like one but then 1% like an IR instead. It's an unforced error.

Assemblers used to do a ton of stuff back in the day

Oh yeah. S/360 assembly almost looks like a high level language sometimes. In MVS, functions of the OS and standard libraries (or its equivalent) were implemented as elaborate macros, with their own invocation syntax, whereas nowadays you'd expect a function that you'd call (dynamically linked or not), with parameters passed in registers.

At least in the 90s, there were actually macro assemblers that supported OOP programming in assembly. Borland Turbo Assembler 5.0 comes to mind, if was kind of fun.

Re: We found a bug in Go's ARM64 compiler

#88
post #17

The real lesson here should be that doing crazy shit like swizzling the program counter in a signal handler and writing your own assembler is not a good idea.

Those are both completely normal things to do when you're implementing a programming language. For example, the Hotspot JVM uses SIGSEGV to stop the world for garbage collection.

Re: We found a bug in Go's ARM64 compiler

#90
post #84

I thought Cloudflare was 100% Rust, and x86 (EPYC) these days. Interesting to hear Go & ARM in use.

Cloudflare has long kept Arm builds of everything even when they deployed to x86 only, to make it easy to switch when it made sense.

And yeah, a lot of Rust but also a lot of Go.

Post reply on HN