7 ms·

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 wo

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 access you could push then pop the stack size maybe? Since presumably you're doing that pair of moves on function entry and exit. I'm not really sure what the garbage collector is looking for so maybe that doesn't work, but I'd be interested to hear some takes on it

titzer 1y ago
I think the right fix is that the compiler should, e.g. load the constant into a register using two moves and then emit a single add. It's one more instruction, but then the adjustment is atomic (i.e. a single instruction). Another option is to do the arithmetic in a temp register and then move it back.
bloak 1y ago
> So another win for being able to read arm assembly. Yes, though that weird stuff with dollars in it is not normal AArch64 assembly! The article could have mentioned the "stack moves once" rule.
- pjmlp 1y ago
  It is due to the Plan 9 Assembly dialect most likely, because it wasn't enough that we already have differences between AT&T and Intel. https://go.dev/doc/asm https://go.dev/doc/asm Still, I find great that Go got back the 1990's tradition that compiled languages have an assembler as part of their tooling, regardless of the syntax.
- Neywiny 1y ago
  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 generate a complicated C function, put it in godbolt, and there's plenty of push push push push ..... Pop Pop Pop Pop going on
  - rcxdude 1y ago
    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 of space if variable lifetimes don't overlap)
    - Neywiny 1y ago
      Yes
    - ori_b 1y ago
      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.
      - saagarjha 1y ago
        Wow, that’s horrible.
      - i80and 1y ago
        The engineers were so preoccupied with whether or not they could that they didn't stop to think if they should
  - JdeBP 1y ago
    You need to look at non-x86 architectures. It was common years ago on MIPS. * https://jdebp.uk/FGA/function-perilogues.html#StandardMIPS https://jdebp.uk/FGA/function-perilogues.html#StandardMIPS I wrote up the x86 equivalent of doing just two read-modify-write operations on the stack pointer over 16 years ago. * https://jdebp.uk/FGA/function-perilogues.html#Standardx86 https://jdebp.uk/FGA/function-perilogues.html#Standardx86
- freep1zza 1y ago
  > Yes, though that weird stuff with dollars in it is not normal AArch64 assembly! See the AT&T vs Intel syntax since you aren't familiar with assembly: https://en.wikipedia.org/wiki/X86_assembly_language#Syntax https://en.wikipedia.org/wiki/X86_assembly_language#Syntax
  - dpassens 1y ago
    That's an x86 thing, though.
    - indrora 1y ago
      There are more assembler dialects than I care to remember. The 2A06 assembler that people who write NES code (and later on SNES/GB/etc) use has some real quirks: $ prefixes a literal hex value but % is binary, but # in front of that is an address, registers are baked into the opcode (ldx -> load into X), and more. Playstation folks all just used MIPS dialects which are mostly AT&Tish but the PS2 used an Intel style assembler.
pjmlp 1y ago
Usually in runtimes like Java and .NET there are safepoints exactly to avoid changing context in the middle of a set of instructions.
- andygocke 1y ago
  Yeah but we have codegen bugs in .NET as well. The biggest difference that stood out to me in this write up, is we would have gone straight for “coredump” instead of other investigation tools. Our default mode of investigating memory corruption issues is dumps.
  - pjmlp 1y ago
    Sure, I have experienced them, e.g. once in 2006 using IBM's JVM implementation with Websphere. However it is probably not as problematic due to the way Go allows for Assembly being used directly. While the JVM and CLR don't allow for direct access to Assembly code, Go does, thus I assume expecting safepoints everywhere is not an option, as any subroutine call can land on code that was manually written.
    - yvdriess 1y ago
      Go users can only insert assembly wrapped in a function call. That might be safety related, I am not entirely sure. (Well technically there is a way to inject assembly without the function call overhead. That's what https://pkg.go.dev/runtime/internal/atomic https://pkg.go.dev/runtime/internal/atomic is doing. But you will need to modify the runtime and compiler toolchain for it.)
      - pjmlp 1y ago
        If you look the docs, they expect the developer to add specific information and use the registers in a specific way, otherwise Go will face runtime issues. Whereas when you go over CGO, you get a marshaling layer similar to how JNI, P/Invoke work, that take care of those issues.
Veserv 1y ago
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 for the 17-bit immediate for a total of 12 bytes of binary which is 3 executable instructions worth. [1] https://developer.arm.com/documentation/dui0801/l/A64-Data-Transfer-Instructions/LDR-pseudo-instruction https://developer.arm.com/documentation/dui0801/l/A64-Data-T...
- comex 1y ago
  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.
pklausler 1y ago
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.
- moefh 1y ago
  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 handwritten go assembly.
  - pklausler 1y ago
    Some architectures, and I believe aarch64 is one, have scratch registers reserved for being clobbered in special situations required by the assembler.
    - anyfoo 1y ago
      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.
huflungdung 1y ago
[dead]

titzer 1y ago

I think the right fix is that the compiler should, e.g. load the constant into a register using two moves and then emit a single add. It's one more instruction, but then the adjustment is atomic (i.e. a single instruction). Another option is to do the arithmetic in a temp register and then move it back.

bloak 1y ago

> So another win for being able to read arm assembly. Yes, though that weird stuff with dollars in it is not normal AArch64 assembly! The article could have mentioned the "stack moves once" rule.

pjmlp 1y ago
It is due to the Plan 9 Assembly dialect most likely, because it wasn't enough that we already have differences between AT&T and Intel. https://go.dev/doc/asm https://go.dev/doc/asm Still, I find great that Go got back the 1990's tradition that compiled languages have an assembler as part of their tooling, regardless of the syntax.
Neywiny 1y ago
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 generate a complicated C function, put it in godbolt, and there's plenty of push push push push ..... Pop Pop Pop Pop going on
- rcxdude 1y ago
  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 of space if variable lifetimes don't overlap)
  - Neywiny 1y ago
    Yes
  - ori_b 1y ago
    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.
    - saagarjha 1y ago
      Wow, that’s horrible.
    - i80and 1y ago
      The engineers were so preoccupied with whether or not they could that they didn't stop to think if they should
- JdeBP 1y ago
  You need to look at non-x86 architectures. It was common years ago on MIPS. * https://jdebp.uk/FGA/function-perilogues.html#StandardMIPS https://jdebp.uk/FGA/function-perilogues.html#StandardMIPS I wrote up the x86 equivalent of doing just two read-modify-write operations on the stack pointer over 16 years ago. * https://jdebp.uk/FGA/function-perilogues.html#Standardx86 https://jdebp.uk/FGA/function-perilogues.html#Standardx86
freep1zza 1y ago
> Yes, though that weird stuff with dollars in it is not normal AArch64 assembly! See the AT&T vs Intel syntax since you aren't familiar with assembly: https://en.wikipedia.org/wiki/X86_assembly_language#Syntax https://en.wikipedia.org/wiki/X86_assembly_language#Syntax
- dpassens 1y ago
  That's an x86 thing, though.
  - indrora 1y ago
    There are more assembler dialects than I care to remember. The 2A06 assembler that people who write NES code (and later on SNES/GB/etc) use has some real quirks: $ prefixes a literal hex value but % is binary, but # in front of that is an address, registers are baked into the opcode (ldx -> load into X), and more. Playstation folks all just used MIPS dialects which are mostly AT&Tish but the PS2 used an Intel style assembler.

pjmlp 1y ago

Usually in runtimes like Java and .NET there are safepoints exactly to avoid changing context in the middle of a set of instructions.

andygocke 1y ago
Yeah but we have codegen bugs in .NET as well. The biggest difference that stood out to me in this write up, is we would have gone straight for “coredump” instead of other investigation tools. Our default mode of investigating memory corruption issues is dumps.
- pjmlp 1y ago
  Sure, I have experienced them, e.g. once in 2006 using IBM's JVM implementation with Websphere. However it is probably not as problematic due to the way Go allows for Assembly being used directly. While the JVM and CLR don't allow for direct access to Assembly code, Go does, thus I assume expecting safepoints everywhere is not an option, as any subroutine call can land on code that was manually written.
  - yvdriess 1y ago
    Go users can only insert assembly wrapped in a function call. That might be safety related, I am not entirely sure. (Well technically there is a way to inject assembly without the function call overhead. That's what https://pkg.go.dev/runtime/internal/atomic https://pkg.go.dev/runtime/internal/atomic is doing. But you will need to modify the runtime and compiler toolchain for it.)
    - pjmlp 1y ago
      If you look the docs, they expect the developer to add specific information and use the registers in a specific way, otherwise Go will face runtime issues. Whereas when you go over CGO, you get a marshaling layer similar to how JNI, P/Invoke work, that take care of those issues.

Veserv 1y ago

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 for the 17-bit immediate for a total of 12 bytes of binary which is 3 executable instructions worth. [1] https://developer.arm.com/documentation/dui0801/l/A64-Data-Transfer-Instructions/LDR-pseudo-instruction https://developer.arm.com/documentation/dui0801/l/A64-Data-T...

comex 1y ago
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.

pklausler 1y ago

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.

moefh 1y ago
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 handwritten go assembly.
- pklausler 1y ago
  Some architectures, and I believe aarch64 is one, have scratch registers reserved for being clobbered in special situations required by the assembler.
  - anyfoo 1y ago
    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.

huflungdung 1y ago

[dead]