jroelofs created this revision.
Herald added a subscriber: aemerson.

The idea for this originated from a really tricky bug: ISRs on ARM don't 
automatically save off the VFP regs, so if say, memcpy gets interrupted and the 
ISR itself calls memcpy, the regs are left clobbered when the ISR is done.

If the equivalent problem is likely to happen on other arches, I'd be happy to 
extend this to them. Suggestions on a more arch-neutral spelling of the warning 
itself would be helpful in that case.


https://reviews.llvm.org/D28820

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


Index: test/Sema/arm-interrupt-attr.c
===================================================================
--- test/Sema/arm-interrupt-attr.c
+++ test/Sema/arm-interrupt-attr.c
@@ -17,3 +17,14 @@
 __attribute__((interrupt)) void foo8() {}
 __attribute__((interrupt())) void foo9() {}
 __attribute__((interrupt(""))) void foo10() {}
+
+void callee1();
+__attribute__((interrupt("IRQ"))) void callee2();
+void caller1() {
+  callee1();
+  callee2();
+}
+__attribute__((interrupt("IRQ"))) void caller2() {
+  callee1(); // expected-warning {{call to function without interrupt 
attribute could clobber interruptee's VFP registers}}
+  callee2();
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5395,6 +5395,15 @@
     return ExprError();
   }
 
+  // Interrupt handlers don't save off the VFP regs automatically on ARM,
+  // 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())
+    if (Caller->hasAttr<ARMInterruptAttr>())
+      if (!FDecl->hasAttr<ARMInterruptAttr>())
+        Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
+
   // Promote the function operand.
   // We special-case function promotion here because we only allow promoting
   // builtin functions to function pointers in the callee of a call.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -259,6 +259,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 warn_arm_interrupt_calling_convention : Warning<
+   "call to function without interrupt attribute could clobber interruptee's 
VFP registers">,
+   InGroup<Extra>;
 def warn_mips_interrupt_attribute : Warning<
    "MIPS 'interrupt' attribute only applies to functions that have "
    "%select{no parameters|a 'void' return type}0">,


Index: test/Sema/arm-interrupt-attr.c
===================================================================
--- test/Sema/arm-interrupt-attr.c
+++ test/Sema/arm-interrupt-attr.c
@@ -17,3 +17,14 @@
 __attribute__((interrupt)) void foo8() {}
 __attribute__((interrupt())) void foo9() {}
 __attribute__((interrupt(""))) void foo10() {}
+
+void callee1();
+__attribute__((interrupt("IRQ"))) void callee2();
+void caller1() {
+  callee1();
+  callee2();
+}
+__attribute__((interrupt("IRQ"))) void caller2() {
+  callee1(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
+  callee2();
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5395,6 +5395,15 @@
     return ExprError();
   }
 
+  // Interrupt handlers don't save off the VFP regs automatically on ARM,
+  // 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())
+    if (Caller->hasAttr<ARMInterruptAttr>())
+      if (!FDecl->hasAttr<ARMInterruptAttr>())
+        Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
+
   // Promote the function operand.
   // We special-case function promotion here because we only allow promoting
   // builtin functions to function pointers in the callee of a call.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -259,6 +259,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 warn_arm_interrupt_calling_convention : Warning<
+   "call to function without interrupt attribute could clobber interruptee's VFP registers">,
+   InGroup<Extra>;
 def warn_mips_interrupt_attribute : Warning<
    "MIPS 'interrupt' attribute only applies to functions that have "
    "%select{no parameters|a 'void' return type}0">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to