[PATCH] D97764: [clang][patch] To solve PR26413, x86 interrupt routines may only call routines with no_saved_reg

[Melanie Blower via Phabricator via cfe-commits]

mibintc created this revision.
mibintc added reviewers: ABataev, aaron.ballman.
Herald added a subscriber: kristof.beyls.
mibintc requested review of this revision.
Herald added a project: clang.

In the bug report, @DavidKreitzer wrote:
clang should be giving an error for this test, because we have no good way to efficiently save & restore the non-GPR state.

The interrupt handler is required to save & restore all the register state that it uses. And according to the ABI, the call to subroutine1() may clobber arbitrary XMM, YMM, or ZMM state. The only way to reliably save & restore that state is to use xsave/xrstor, which would be very inefficient and is probably not what we want.

This patch implements the check described. There's a similar check for arm architecture and they give a warning. Folks tend to ignore warnings so I'll propose this as an error in the first go-round.  @ABataev wrote a similar error check so I'll add him as reviewer


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97764

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/attr-x86-interrupt.c


Index: clang/test/Sema/attr-x86-interrupt.c
===================================================================
--- clang/test/Sema/attr-x86-interrupt.c
+++ clang/test/Sema/attr-x86-interrupt.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -triple x86_64-pc-win32  -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple i386-pc-win32  -fsyntax-only -verify %s
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnux32  -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu  -fsyntax-only -verify %s -DNOCALLERSAVE=1
 
 struct a {
   int b;
@@ -39,6 +40,22 @@
 __attribute__((interrupt)) void foo7(int *a, unsigned b) {}
 __attribute__((interrupt)) void foo8(int *a) {}
 
+#ifdef _LP64
+typedef unsigned long Arg2Type;
+#elif defined(__x86_64__)
+typedef unsigned long long Arg2Type;
+#else
+typedef unsigned int Arg2Type;
+#endif
+#ifndef NOCALLERSAVE
+__attribute__((no_caller_saved_registers))
+#else
+// expected-error at +4 {{interrupt service routine may only call routine with attribute no_caller_saved_registers}}
+#endif
+void foo9(int *a, Arg2Type b) {}
+__attribute__((interrupt)) void fooA(int *a, Arg2Type b) {
+  foo9(a, b);
+}
 void g(void (*fp)(int *));
 int main(int argc, char **argv) {
   void *ptr = (void *)&foo7;
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -6552,12 +6552,20 @@
   // so there's some risk when calling out to non-interrupt handler functions
   // that the callee might not preserve them. This is easy to diagnose here,
   // but can be very challenging to debug.
-  if (auto *Caller = getCurFunctionDecl())
+  // Likewise, X86 interrupt handlers may only call routines with attribute
+  // no_caller_saved_registers since there is no efficient way to
+  // save and restore the non-GPR state.
+  if (auto *Caller = getCurFunctionDecl()) {
     if (Caller->hasAttr<ARMInterruptAttr>()) {
       bool VFP = Context.getTargetInfo().hasFeature("vfp");
       if (VFP && (!FDecl || !FDecl->hasAttr<ARMInterruptAttr>()))
         Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
     }
+    if (Caller->hasAttr<AnyX86InterruptAttr>() &&
+        ((!FDecl || !FDecl->hasAttr<AnyX86NoCallerSavedRegistersAttr>()))) {
+      Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave);
+    }
+  }
 
   // Promote the function operand.
   // We special-case function promotion here because we only allow promoting
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -293,6 +293,9 @@
   "a pointer as the first parameter|a %2 type as the second parameter}1">;
 def err_anyx86_interrupt_called : Error<
   "interrupt service routine cannot be called directly">;
+def err_anyx86_interrupt_regsave : Error<
+  "interrupt service routine may only call routine"
+  " with attribute no_caller_saved_registers">;
 def warn_arm_interrupt_calling_convention : Warning<
    "call to function without interrupt attribute could clobber interruptee's VFP registers">,
    InGroup<Extra>;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97764.327413.patch
Type: text/x-patch
Size: 3235 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210302/a3f867b1/attachment-0001.bin>