Ensure registers holding the stack canary are zeroed when no longer needed

[willdeacon]

GCC bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96191 prompted me to look at clang's code generation for that testcase:

.Lmain$local:
// %bb.0:
    sub sp, sp, #32             // =32
    adrp    x8, __stack_chk_guard
    ldr x8, [x8, :lo12:__stack_chk_guard]
    mov w0, #10
    stp x29, x30, [sp, #16]     // 16-byte Folded Spill
    add x29, sp, #16            // =16
    str x8, [sp, #8]
    bl  sub
    adrp    x9, __stack_chk_guard
    ldr x8, [sp, #8]
    ldr x9, [x9, :lo12:__stack_chk_guard]
    cmp x9, x8
    b.ne    .LBB0_2
// %bb.1:
    ldp x29, x30, [sp, #16]     // 16-byte Folded Reload
    mov w0, wzr
    add sp, sp, #32             // =32
    ret
.LBB0_2:
    bl  __stack_chk_fail
.Lfunc_end0:
    .size   main, .Lfunc_end0-main

It appears that clang isn't zeroing the registers holding the stack canary at all. For example, in the prologue after loading into x8 and in the epilogue after restoring into x8 and reloading the global canary into x9.

[nickdesaulniers]

cc @kees

[nickdesaulniers]

cc @kbeyls

[kbeyls]

https://llvm.org/pr46994 is an LLVM bug tracker issue covering implementing this. It currently seems unclear in how far this would actually improve hardening and, if implemented, if this would need to go under a different command line option to enable users to select which tradeoff they want on the hardening vs performance overhead impact.

[nickdesaulniers]

I still think this is worth doing; though I don't understand why the GCC bug seems to be specific to ARM; wouldn't all architectures appreciate such protection, or was it simply that ARM backends had such a bug?

[tsautereau-anssi]

@nickdesaulniers To answer your question, yes, only the AArch64 backend had this issue, which was noticed during review of the RISC-V implementation.

[nickdesaulniers]

@bwendling attempted this in https://github.com/llvm/llvm-project/pull/65461.

[tsautereau-anssi]

If I understand his message correctly, what @bwendling attempted is actually to clear the stack protector guard information located on the stack, not the register that held the stack guard value, which he is going to work on now.

[bwendling]

@tsautereau-anssi That's correct. The other change may still be useful, but the register clearing is more important.