[Sema] Teach -Warm-interrupt-safety about func ptrs

[jroelofs]

Summary

Fixes:
https://bugs.llvm.org/show_bug.cgi?id=35527
https://bugs.llvm.org/show_bug.cgi?id=35528

[jroelofs]

clang/lib/Sema/SemaExpr.cpp

5931

This feels very fragile, and I know is missing at least one case. Is there a better way to reliably get at the typedef's attributes from here?

[efriedma]

clang/lib/Sema/SemaExpr.cpp

5931

Fundamentally, the way canonical types work in C and C++ means that you shouldn't be trying to dig attributes out of typedefs. If it's a property of the type, it should be stored as a member of the FunctionType, like we do with calling conventions etc.

clang/include/clang/Basic/DiagnosticGroups.td

 // A warning group for warnings about features supported by HIP but
 // ignored by CUDA.
 def HIPOnly : DiagGroup<"hip-only">;
 
 // Warnings which cause linking of the runtime libraries like
 // libc and the CRT to be skipped.
 def AVRRtlibLinkingQuirks : DiagGroup<"avr-rtlib-linking-quirks">;
 
+// A warning group for specific gotchas surrounding use of interrupt handlers on ARM
+def ARMInterruptSafety : DiagGroup<"arm-interrupt-safety">;
+
 // A warning group for things that will change semantics in the future.
 def FutureCompat : DiagGroup<"future-compat">;
 
 def InvalidOrNonExistentDirectory : DiagGroup<"invalid-or-nonexistent-directory">;
 
 def OptionIgnored : DiagGroup<"option-ignored">;
 
 def UnknownArgument : DiagGroup<"unknown-argument">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_anyx86_interrupt_attribute : Error<
   "%select{x86|x86-64}0 'interrupt' attribute only applies to functions that "
   "have %select{a 'void' return type|"
   "only a pointer parameter optionally followed by an integer parameter|"
   "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>;
+   InGroup<ARMInterruptSafety>;
 def warn_interrupt_attribute_invalid : Warning<
    "%select{MIPS|MSP430|RISC-V}0 'interrupt' attribute only applies to "
    "functions that have %select{no parameters|a 'void' return type}1">,
    InGroup<IgnoredAttributes>;
 def warn_riscv_repeated_interrupt_attribute : Warning<
   "repeated RISC-V 'interrupt' attribute">, InGroup<IgnoredAttributes>;
 def note_riscv_repeated_interrupt_attribute : Note<
   "repeated RISC-V 'interrupt' attribute is here">;

clang/lib/Sema/SemaExpr.cpp

     Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_called);
     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>()) {
-      bool VFP = Context.getTargetInfo().hasFeature("vfp");
-      if (VFP && (!FDecl || !FDecl->hasAttr<ARMInterruptAttr>()))
-        Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
-    }
+  if (Context.getTargetInfo().hasFeature("vfp"))
+    if (auto *Caller = getCurFunctionDecl())
+      if (Caller->hasAttr<ARMInterruptAttr>()) {
+        const Decl *CalleeDecl = FDecl;
+        if (const auto *UO = dyn_cast<UnaryOperator>(Fn->IgnoreParens())) {
+          if (const auto *TT =
+                  dyn_cast<TypedefType>(UO->getSubExpr()->getType()))
+            CalleeDecl = TT->getDecl();
+        } else if (const auto *TT = Fn->getType()->getAs<TypedefType>()) {
+          CalleeDecl = TT->getDecl();
+        }
+        if (!CalleeDecl || !CalleeDecl->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.
   ExprResult Result;
   QualType ResultTy;
   if (BuiltinID &&
       Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) {

clang/test/Sema/arm-interrupt-attr.c

-// RUN: %clang_cc1 %s -triple arm-apple-darwin  -target-feature +vfp2 -verify -fsyntax-only
-// RUN: %clang_cc1 %s -triple thumb-apple-darwin  -target-feature +vfp3 -verify -fsyntax-only
-// RUN: %clang_cc1 %s -triple armeb-none-eabi  -target-feature +vfp4 -verify -fsyntax-only
-// RUN: %clang_cc1 %s -triple thumbeb-none-eabi  -target-feature +neon -verify -fsyntax-only
-// RUN: %clang_cc1 %s -triple thumbeb-none-eabi -target-feature +neon -target-feature +soft-float -DSOFT -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple arm-apple-darwin  -target-feature +vfp2 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple thumb-apple-darwin  -target-feature +vfp3 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple armeb-none-eabi  -target-feature +vfp4 -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple thumbeb-none-eabi  -target-feature +neon -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Warm-interrupt-safety -triple thumbeb-none-eabi -target-feature +neon -target-feature +soft-float -DSOFT -verify -fsyntax-only
 
 __attribute__((interrupt(IRQ))) void foo() {} // expected-error {{'interrupt' attribute requires a string}}
 __attribute__((interrupt("irq"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: irq}}
 
 __attribute__((interrupt("IRQ", 1))) void foo2() {} // expected-error {{'interrupt' attribute takes no more than 1 argument}}
 
 __attribute__((interrupt("IRQ"))) void foo3() {}
 __attribute__((interrupt("FIQ"))) void foo4() {}
 __attribute__((interrupt("SWI"))) void foo5() {}
 __attribute__((interrupt("ABORT"))) void foo6() {}
 __attribute__((interrupt("UNDEF"))) void foo7() {}
 
 __attribute__((interrupt)) void foo8() {}
 __attribute__((interrupt())) void foo9() {}
 __attribute__((interrupt(""))) void foo10() {}
 
 void callee1();
 __attribute__((interrupt("IRQ"))) void callee2();
 void caller1() {
   callee1();
   callee2();
 }
 
-#ifndef SOFT
 __attribute__((interrupt("IRQ"))) void caller2() {
+#ifndef SOFT
   callee1(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
-  callee2();
-}
-
-void (*callee3)();
-__attribute__((interrupt("IRQ"))) void caller3() {
-  callee3(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
-}
 #else
-__attribute__((interrupt("IRQ"))) void caller2() {
   callee1();
+#endif
   callee2();
 }
 
 void (*callee3)();
 __attribute__((interrupt("IRQ"))) void caller3() {
+#ifndef SOFT
+  callee3(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
+#else
   callee3();
+#endif
 }
+
+void __attribute__((interrupt("IRQ"))) bugzilla35527() {
+  typedef void __attribute__((interrupt("IRQ"))) (*interrupt_callback_t)(int i, void *ctx);
+  interrupt_callback_t interrupt_callee;
+  interrupt_callee(42, 0);
+  (interrupt_callee)(42, 0);
+  (*interrupt_callee)(42, 0);
+
+  typedef void (*callback_t)(int i, void *ctx);
+  callback_t callee;
+#ifndef SOFT
+  callee(37, 0); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
+#else
+  callee(37, 0);
 #endif
+}