[PowerPC] Finished kill_canary implementation and debugging

[pscoro]

Summary

Made a new phabricator review because of git issues

[nemanjai]

"Made a new phabricator review because of git issues" is not an appropriate description of a review/revision.

Hopefully the description you add will describe what this intrinsic is supposed to do. It seems to me that this is a poorly designed feature if it is meant to work the way it was implemented. Namely, it seems like this intrinsic clobbers the stack protect global value rather than clobbering the corresponding value on the stack for the specific function it is enclosed in. I would have thought that it will clobber the stack in the function, thereby allowing stack protection to work as expected for other functions in the module.

llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp

5014 ↗

(On Diff #440234)

I think it would be preferable to handle this intrinsic in one place. The nop is not actually necessary here. We should simply remove the intrinsic from the stream in PPCISelLowering.cpp and not pass it on.

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

10699

The formatting of this entire block is quite messed up. Please run clang-format on this.

10705

Do we use this?

10711

Is it ok to just ignore a failure to get the GV here? Should this not be an assert?

10723–10735

What is happening here? We load the value, XOR it with itself, store it again? Isn't that just zeroing it out? Why do we even need to load it then?

llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

3

At the very least, this has to also include a RUN line for Linux.

[pscoro]

fixed XOR usage, added linux tests and other small fixes

[lkail]

Summary should be updated as @nemanjai has said.

[shchenz]

Please make sure the patch is able to compile first. I fixed two build failures, it still fails : (

llvm/include/llvm/IR/IntrinsicsPowerPC.td

198

Need rebase your code. GCCBuiltin is not a valid class anymore. See https://reviews.llvm.org/D127460

llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp

5013 ↗

(On Diff #441374)

Nit: avoid this unnecessary change

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

58

Do we need this header? A little strange...

10715

Please make sure when you post the patch, it is ok to compile. DL seems undefined, we use dl here.

Also please read the coding style of llvm first. https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

canaryLoc does not follow the style, should be CanaryLoc.

10725

All these new added codes need clang-format

llvm/test/CodeGen/PowerPC/kill-canary-intrinsic-aix.ll

5 ↗

(On Diff #441374)

We can merge these two files kill-canary-intrinsic-aix.ll and kill-canary-intrinsic-linux.ll into one file with different runlines but with different check prefix.

[pscoro]

patch fix

clang/include/clang/Basic/BuiltinsPPC.def

 BUILTIN(__builtin_ppc_fetch_and_add, "iiD*i", "")
 BUILTIN(__builtin_ppc_fetch_and_addlp, "LiLiD*Li", "")
 BUILTIN(__builtin_ppc_fetch_and_and, "UiUiD*Ui", "")
 BUILTIN(__builtin_ppc_fetch_and_andlp, "ULiULiD*ULi", "")
 BUILTIN(__builtin_ppc_fetch_and_or, "UiUiD*Ui", "")
 BUILTIN(__builtin_ppc_fetch_and_orlp, "ULiULiD*ULi", "")
 BUILTIN(__builtin_ppc_fetch_and_swap, "UiUiD*Ui", "")
 BUILTIN(__builtin_ppc_fetch_and_swaplp, "ULiULiD*ULi", "")
+BUILTIN(__builtin_ppc_kill_canary, "v", "")
 BUILTIN(__builtin_ppc_ldarx, "LiLiD*", "")
 BUILTIN(__builtin_ppc_lwarx, "iiD*", "")
 BUILTIN(__builtin_ppc_lharx, "ssD*", "")
 BUILTIN(__builtin_ppc_lbarx, "ccD*", "")
 BUILTIN(__builtin_ppc_stdcx, "iLiD*Li", "")
 BUILTIN(__builtin_ppc_stwcx, "iiD*i", "")
 BUILTIN(__builtin_ppc_sthcx, "isD*s", "")
 BUILTIN(__builtin_ppc_stbcx, "icD*i", "")

clang/lib/Basic/Targets/PPC.cpp

 //===--- PPC.cpp - Implement PPC target feature support -------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
static void defineXLCompatMacros(MacroBuilder &Builder) {
   Builder.defineMacro("__fetch_and_add", "__builtin_ppc_fetch_and_add");
   Builder.defineMacro("__fetch_and_addlp", "__builtin_ppc_fetch_and_addlp");
   Builder.defineMacro("__fetch_and_and", "__builtin_ppc_fetch_and_and");
   Builder.defineMacro("__fetch_and_andlp", "__builtin_ppc_fetch_and_andlp");
   Builder.defineMacro("__fetch_and_or", "__builtin_ppc_fetch_and_or");
   Builder.defineMacro("__fetch_and_orlp", "__builtin_ppc_fetch_and_orlp");
   Builder.defineMacro("__fetch_and_swap", "__builtin_ppc_fetch_and_swap");
   Builder.defineMacro("__fetch_and_swaplp", "__builtin_ppc_fetch_and_swaplp");
+  Builder.defineMacro("__kill_canary", "__builtin_ppc_kill_canary");
   Builder.defineMacro("__ldarx", "__builtin_ppc_ldarx");
   Builder.defineMacro("__lwarx", "__builtin_ppc_lwarx");
   Builder.defineMacro("__lharx", "__builtin_ppc_lharx");
   Builder.defineMacro("__lbarx", "__builtin_ppc_lbarx");
   Builder.defineMacro("__stfiw", "__builtin_ppc_stfiw");
   Builder.defineMacro("__stdcx", "__builtin_ppc_stdcx");
   Builder.defineMacro("__stwcx", "__builtin_ppc_stwcx");
   Builder.defineMacro("__sthcx", "__builtin_ppc_sthcx");

clang/test/CodeGen/PowerPC/builtins-ppc-stackprotect.c

+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-linux-gnu \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | \
+// RUN:  FileCheck %s
+// RUN: %clang_cc1 -O2 -triple powerpc64le-unknown-linux-gnu \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr8 | \
+// RUN:  FileCheck %s
+// RUN: %clang_cc1 -O2 -triple powerpc-unknown-aix \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | \
+// RUN:  FileCheck %s
+// RUN: %clang_cc1 -O2 -triple powerpc64-unknown-aix \
+// RUN:   -emit-llvm %s -o - -target-cpu pwr7 | \
+// RUN:  FileCheck %s
+
+void test_builtin_ppc_kill_canary() {
+  // CHECK: call void @llvm.ppc.kill.canary()
+  __builtin_ppc_kill_canary();
+}
+
+void test_kill_canary() {
+  // CHECK: call void @llvm.ppc.kill.canary()
+  __kill_canary();
+}

llvm/include/llvm/Debuginfod/Debuginfod.h

 //===-- llvm/Debuginfod/Debuginfod.h - Debuginfod client --------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///

 #include "llvm/Debuginfod/HTTPServer.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/RWMutex.h"
 #include "llvm/Support/Timer.h"
 
 #include <chrono>
+#include <condition_variable>
 #include <queue>
 
 namespace llvm {
 
 typedef ArrayRef<uint8_t> BuildIDRef;
 
 typedef SmallVector<uint8_t, 10> BuildID;
 

llvm/include/llvm/IR/IntrinsicsPowerPC.td

def int_ppc_minfl
       : Intrinsic<
             [llvm_double_ty],
             [llvm_double_ty, llvm_double_ty, llvm_double_ty, llvm_vararg_ty],
             [IntrNoMem]>;
   def int_ppc_minfs
       : Intrinsic<[llvm_float_ty],
                   [llvm_float_ty, llvm_float_ty, llvm_float_ty, llvm_vararg_ty],
                   [IntrNoMem]>;
+  def int_ppc_kill_canary
+      : ClangBuiltin<"__builtin_ppc_kill_canary">, 
+        Intrinsic<[],
+                  [],
+                  [IntrWriteMem, IntrHasSideEffects]>;
 }
 
 let TargetPrefix = "ppc" in {  // All PPC intrinsics start with "llvm.ppc.".
   /// PowerPC_Vec_Intrinsic - Base class for all altivec intrinsics.
   class PowerPC_Vec_Intrinsic<string GCCIntSuffix, list<LLVMType> ret_types,
                               list<LLVMType> param_types,
                               list<IntrinsicProperty> properties>
     : ClangBuiltin<!strconcat("__builtin_altivec_", GCCIntSuffix)>,

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

 //===-- PPCISelLowering.cpp - PPC DAG Lowering Implementation -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

 #include "llvm/CodeGen/RuntimeLibcalls.h"
 #include "llvm/CodeGen/SelectionDAG.h"
 #include "llvm/CodeGen/SelectionDAGNodes.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/CodeGen/TargetLoweringObjectFileImpl.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
 #include "llvm/CodeGen/ValueTypes.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/IRBuilder.h"
if (Ty == MVT::i128) {
       Val = DAG.getNode(ISD::TRUNCATE, DL, MVT::i64, Val);
     }
     return SDValue(
         DAG.getMachineNode(PPC::CFENCE8, DL, MVT::Other,
                            DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, Val),
                            Op.getOperand(0)),
         0);
   }
+  case Intrinsic::ppc_kill_canary: {
+    MachineFunction &MF = DAG.getMachineFunction();
+    const Module *M = MF.getMMI().getModule();
+
+    /* If SafeStack or !StackProtector, kill_canary not supported */
+    if (MF.getFunction().hasFnAttribute(Attribute::SafeStack) ||
+        !MF.getFunction().hasStackProtectorFnAttr()) {
+      DAG.ReplaceAllUsesOfValueWith(
+          SDValue(Op.getNode(), 0),
+          Op->getOperand(0)); // prepare node for deletion
+      break;
+    }
+
+    EVT VT = DAG.getTargetLoweringInfo().getPointerTy(DAG.getDataLayout());
+
+    SDValue Load;
+    SDValue Store;
+
+    const uint64_t XORWord =
+        0xFFFFFFFFFFFFFFFF; // XORing with 0b111...111 will never
+                            // result in the original word
+
+    if (useLoadStackGuardNode()) { // linux uses LOAD_STACK_GUARD node instead
+                                   // of having a canary word global value
+      MachineSDNode *LSG =
+          DAG.getMachineNode(PPC::LOAD_STACK_GUARD, DL, VT, Op->getOperand(0));
+      Load = SDValue(LSG, 0);
+
+      /* frame index used to determine stack guard location if
+       * LOAD_STACK_GUARD is used */
+      MachineFrameInfo &MFI = MF.getFrameInfo();
+      int SPI = MFI.getStackProtectorIndex(); // should return -1
+      PPCFunctionInfo *FuncInfo = MF.getInfo<PPCFunctionInfo>();
+      SDValue FIN = DAG.getFrameIndex(FuncInfo->getVarArgsFrameIndex(), VT);
+
+      // XOR canary word and store back
+      Store = DAG.getStore(
+          Op->getOperand(0), DL,
+          DAG.getNode(ISD::XOR, DL, VT, Load, DAG.getConstant(XORWord, DL, VT)),
+          DAG.getNode( // add frame index, stack protector index, return node
+                       // result
+              ISD::ADD, DL, VT, FIN, DAG.getConstant(SPI, DL, VT)),
+          MachinePointerInfo());
+
+    } else if (Value *GV =
+                   getSDagStackGuard(*M)) { // on aix, load from global value
+      VT = DAG.getTargetLoweringInfo().getValueType(DAG.getDataLayout(),
+                                                    GV->getType(), true);
+      SDValue CanaryLoc =
+          DAG.getGlobalAddress(dyn_cast<GlobalValue>(GV), DL, VT);
+
+      // Load from global value
+      Load = DAG.getLoad(VT, DL, Op->getOperand(0), CanaryLoc,
+                         MachinePointerInfo());
+
+      // XOR canary word and store back
+      Store = DAG.getStore(
+          Op->getOperand(0), DL,
+          DAG.getNode(ISD::XOR, DL, VT, Load, DAG.getConstant(XORWord, DL, VT)),
+          CanaryLoc, MachinePointerInfo());
+    } else {
+      llvm_unreachable("Unhandled stack guard case");
+    }
+
+    return Store;
+  }
   default:
     break;
   }
   return SDValue();
 }
 
 // Lower scalar BSWAP64 to xxbrd.
 SDValue PPCTargetLowering::LowerBSWAP(SDValue Op, SelectionDAG &DAG) const {

llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=LINUX
+
+declare void @llvm.ppc.kill.canary()
+define dso_local void @test_kill_canary() {
+; AIX-LABEL: test_kill_canary:
+; AIX:       # %bb.0: # %entry
+; AIX-NEXT:    blr
+;
+; LINUX-LABEL: test_kill_canary:
+; LINUX:       # %bb.0: # %entry
+; LINUX-NEXT:    blr
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
+
+attributes #0 = { sspreq }
+; Function Attrs: sspreq
+define dso_local void @test_kill_canary_ssp() #0 {
+; AIX-LABEL: test_kill_canary_ssp:
+; AIX:       # %bb.0: # %entry
+; AIX-NEXT:    mflr r0
+; AIX-NEXT:    std r0, 16(r1)
+; AIX-NEXT:    stdu r1, -128(r1)
+; AIX-NEXT:    ld r3, L..C0(r2) # @__ssp_canary_word
+; AIX-NEXT:    ld r4, 0(r3)
+; AIX-NEXT:    std r4, 120(r1)
+; AIX-NEXT:    ld r4, 0(r3)
+; AIX-NEXT:    xori r4, r4, 65535
+; AIX-NEXT:    xoris r4, r4, 65535
+; AIX-NEXT:    std r4, 0(r3)
+; AIX-NEXT:    ld r3, 0(r3)
+; AIX-NEXT:    ld r4, 120(r1)
+; AIX-NEXT:    cmpld r3, r4
+; AIX-NEXT:    bne cr0, L..BB1_2
+; AIX-NEXT:  # %bb.1: # %entry
+; AIX-NEXT:    addi r1, r1, 128
+; AIX-NEXT:    ld r0, 16(r1)
+; AIX-NEXT:    mtlr r0
+; AIX-NEXT:    blr
+; AIX-NEXT:  L..BB1_2: # %entry
+; AIX-NEXT:    bl .__stack_chk_fail[PR]
+; AIX-NEXT:    nop
+;
+; LINUX-LABEL: test_kill_canary_ssp:
+; LINUX:       # %bb.0: # %entry
+; LINUX-NEXT:    mflr r0
+; LINUX-NEXT:    std r0, 16(r1)
+; LINUX-NEXT:    stdu r1, -128(r1)
+; LINUX-NEXT:    .cfi_def_cfa_offset 128
+; LINUX-NEXT:    .cfi_offset lr, 16
+; LINUX-NEXT:    ld r3, -28688(r13)
+; LINUX-NEXT:    std r3, 120(r1)
+; LINUX-NEXT:    ld r3, -28688(r13)
+; LINUX-NEXT:    xori r3, r3, 65535
+; LINUX-NEXT:    xoris r3, r3, 65535
+; LINUX-NEXT:    std r3, 120(r1)
+; LINUX-NEXT:    ld r3, 120(r1)
+; LINUX-NEXT:    ld r4, -28688(r13)
+; LINUX-NEXT:    cmpld r4, r3
+; LINUX-NEXT:    bne cr0, .LBB1_2
+; LINUX-NEXT:  # %bb.1: # %entry
+; LINUX-NEXT:    addi r1, r1, 128
+; LINUX-NEXT:    ld r0, 16(r1)
+; LINUX-NEXT:    mtlr r0
+; LINUX-NEXT:    blr
+; LINUX-NEXT:  .LBB1_2: # %entry
+; LINUX-NEXT:    bl __stack_chk_fail
+; LINUX-NEXT:    nop
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}