AArch64: Disable the Load Stack Guard on OpenBSD/aarch64

[brad]

Summary

From our tree..

Disable the Load Stack Guard for OpenBSD on AArch64. We don't use it
on any other platform and it causes a segfault in combination with our
IR Stack Guard.

[t.p.northover]

There should be a test for this.

[brad]

In D30407#687439, @t.p.northover wrote:

There should be a test for this.

Any suggestions?

[t.p.northover]

OK, I've digged a bit more deeply and I don't think this is the right fix. Currently we seem to have 2 relevant toggles:

• getIRStackGuard can return nullptr or something valid.
• useLoadStackGuardNode can return true or false.

In most places (that return nullptr in the first case), useLoadStackGuardNode switches between a pseudo-instruction that will load from the specified global, and an explicit load from that global in the DAG. However, in the one place where you have an @llvm.stackprotector call the load is (correctly) skipped, unless useLoadStackGuardNode returns true.

So we've got two issues here:

• The handling of @llvm.stackprotector is pretty clearly buggy: there should be no option to useLoadStackGuard because the load has already happened according to the IR semantics.
• OpenBSD almost certainly doesn't need this divergent code-path anyway, and should just be overriding the guard's global variable. As far as I can tell the divergence was introduced without any intent by r188766 (purely for the convenience of not having to make sure OpenBSD works with the new scheme, I expect).

The newer (SDAG) scheme appears to have enhanced security (no spills of the guard, better epilogue behaviour), so it's probably in OpenBSD's best interests to switch.

[t.p.northover]

Oh, some references:

The handling of @llvm.stackprotector is pretty clearly buggy: there should be no option to useLoadStackGuard because the load has already happened according to the IR semantics.

This is SelectionDAGBuilder.cpp:5461 in trunk.

OpenBSD almost certainly doesn't need this divergent code-path anyway, and should just be overriding the guard's global variable. As far as I can tell the divergence was introduced without any intent by r188766 (purely for the convenience of not having to make sure OpenBSD works with the new scheme, I expect).

This might be as simple as moving OpenBSD's special case in TargetLoweringBase.cpp (getIRStackGuard) to the following 2 functions, possibly with some light refactoring to avoid duplication.

lib/Target/AArch64/AArch64ISelLowering.cpp

 }
 
 bool AArch64TargetLowering::useLoadStackGuardNode() const {
-  if (Subtarget->isTargetAndroid() || Subtarget->isTargetFuchsia())
+  if (Subtarget->isTargetAndroid() || Subtarget->isTargetFuchsia() ||
+      Subtarget->isTargetOpenBSD())
     return TargetLowering::useLoadStackGuardNode();
   return true;
 }

lib/Target/AArch64/AArch64Subtarget.h

   bool isTargetWindows() const { return TargetTriple.isOSWindows(); }
   bool isTargetAndroid() const { return TargetTriple.isAndroid(); }
   bool isTargetFuchsia() const { return TargetTriple.isOSFuchsia(); }
+  bool isTargetOpenBSD() const { return TargetTriple.isOSOpenBSD(); }
 
   bool isTargetCOFF() const { return TargetTriple.isOSBinFormatCOFF(); }
   bool isTargetELF() const { return TargetTriple.isOSBinFormatELF(); }