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@+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>;
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@+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>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
