> After this change, stacks larger than 1Seems silly to pessimize the runtime, even slightly, to account for the partial register construction. DWARF bytecode ought to be powerful enough to express the calculations needed for restoring the true stack pointer if we're between immediate adjustments.
We found a bug in Go's ARM64 compiler
71–80 of 146 posts
Re: We found a bug in Go's ARM64 compiler
#72This problem strikes me more as a debuginfo generation bug than a "compiler" bug. > After this change, stacks larger than 1 Seems silly to pessimize the runtime, even slightly, to account for the partial register construction. DWARF bytecode ought to be powerful enough to express the calculations needed for restoring the true stack pointer if we're between immediate adjustments.
But isn't that the same thing here? The bug occurred in their production workflows, not in some specific debug builds, so with that seems pretty reasonable to call it a compiler bug?
Re: We found a bug in Go's ARM64 compiler
#73> This was a very fun problem to debug. I'm sure it was a relief to find a thorough solution that addressed the root cause. But it doesn't seem plausible that it was fun while it was unexplained. When I have this kind of bug it eats my whole attention. Something this deep is especially frustrating. Nobody suspects the standard library or the compiler. Devs have been taught from a young age that it's always you, not t…
It becomes fun when you narrow down to the solution. Before that it's hell. I don't think I'd be allowed spend weeks to debug something like this. Credit to Cloudflare's PMs.
https://blog.cloudflare.com/however-improbable-the-story-of-...
> But [the Cloudbleed sensitive information disclosure security incident] wasn’t the only consequence of the bug. Sometimes it could lead to an invalid memory read, causing the NGINX process to crash, and we had metrics showing these crashes in the weeks leading up to the discovery of Cloudbleed. So one of the measures we took to prevent such a problem happening again was to require that every crash be investigated in detail.
Since then, they have a "no crashes go uninvestigated" policy, which for the scale Cloudflare operates at, seems pretty impressive.
Re: We found a bug in Go's ARM64 compiler
#74That'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…
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.
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 handwritten go assembly.
Re: We found a bug in Go's ARM64 compiler
#75Earlier quoted context omitted.
What's stopping you applying today?
Fair question. Location primarily ( nothing in France ), and I’m not sure how ‘we’re looking for people who enjoy doing that kind of thing’( I very much do ) relates to the actual job offers, ie what job offer should I actually apply to. My background is not networking ( it’s math then hpc then broader stuff ) but I keep stumbling on similar problems ( including a beautiful one related to intel NICs a few years ago w…
Can't speak to the locations but the stuff you're interested/experienced in seems extremely likely to overlap with what they do. They do a lot of very deep technical things in all kinds of areas.
my recommendation if you want to talk to someone about it: search github/twitter/linkedin for ppl who work there on stuff you like, and just send them a message and ask for a 20 minute call!
have done it plenty of times, has always been extremely positive
Re: We found a bug in Go's ARM64 compiler
#76Earlier quoted context omitted.
It feels like this comment was almost a purely additive anecdote of your own experience with a similar kind of issue, but you've spoiled it by deciding to tell the author that they're incorrect about how they felt during the process? Maybe different people find different things fun.
Not saying he's wrong, sometimes the word "fun" connotes something slightly different what what it literally means. "Satisfying" is something I'd use for the end state. Maybe "challenging" for the intermediate state. But while you're in a high-pressure situation that you don't understand, that is rarely "fun" in the literal sense. You wouldn't pay to be given compiler race condition bugs, right?
This, and now there’s pernosco which makes everything much easier.
Now, under pressure, this is going to be a nightmare unless you have a high tolerance to stress.
Re: We found a bug in Go's ARM64 compiler
#77This problem strikes me more as a debuginfo generation bug than a "compiler" bug. > After this change, stacks larger than 1 Seems silly to pessimize the runtime, even slightly, to account for the partial register construction. DWARF bytecode ought to be powerful enough to express the calculations needed for restoring the true stack pointer if we're between immediate adjustments.
> This problem strikes me more as a debuginfo generation bug than a "compiler" bug. But isn't that the same thing here? The bug occurred in their production workflows, not in some specific debug builds, so with that seems pretty reasonable to call it a compiler bug?
As for the actual bug:
Unless you're unwinding the stack by walking the linked list of frames threaded through the frame pointer, then each time you unwind a level of the stack, you need to consult a table keyed on instruction pointer to look up how to compute the register contents of the previous frame based on register content of the current frame. One of the registers you can compute this way is the previous frame's stack pointer.
I haven't looked in depth at what the Go runtime is doing exactly, but at a glance, I don't see mention of frame pointers in the linked article, so I'm guessing Go uses the SP-and-unwind-table approach? If so, the real bug here is that the table didn't have separate entries for the two ADDs and so gave incorrect reconstruction instructions for one of them.
If, however, frame pointers are a load-bearing part of the Go runtime, and that runtime failed to update frame pointer (not just the stack pointer) in the contractually mandatory manner, well, that's a codegen bug and needs a codegen fix.
I guess I just don't like, as a matter of philosophy if not practical engineering, having frame pointers at all. Without the frame pointer, the program already contains all the information you need to unwind, at no runtime cost --- you pay for table lookups only when you unwind, not all the time, on straight-line code.
The purist in me doesn't like burning a register for debugging, but you have to use the right tool for the job I guess.
Re: We found a bug in Go's ARM64 compiler
#78Re: We found a bug in Go's ARM64 compiler
#79What 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.