[clang][Sema] Emit warnings on incorrect AVR interrupt/signal handlers

[benshi001]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-risc-v

Author: Ben Shi (benshi001)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/125328.diff

6 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2-2)
• (modified) clang/lib/Sema/SemaAVR.cpp (+24)
• (modified) clang/lib/Sema/SemaMIPS.cpp (+2-2)
• (modified) clang/lib/Sema/SemaMSP430.cpp (+2-2)
• (modified) clang/lib/Sema/SemaRISCV.cpp (+2-2)
• (modified) clang/test/Sema/avr-interrupt-attr.c (+11-1)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 00a94eb7a30367..4a8dc4b3879540 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -351,8 +351,8 @@ def warn_arm_interrupt_vfp_clobber : Warning<
 def err_arm_interrupt_called : Error<
   "interrupt service routine cannot be called directly">;
 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">,
+   "%select{MIPS|MSP430|RISC-V|AVR}0 '%1' attribute only applies to "
+   "functions that have %select{no parameters|a 'void' return type}2">,
    InGroup<IgnoredAttributes>;
 def warn_riscv_repeated_interrupt_attribute : Warning<
   "repeated RISC-V 'interrupt' attribute">, InGroup<IgnoredAttributes>;
diff --git a/clang/lib/Sema/SemaAVR.cpp b/clang/lib/Sema/SemaAVR.cpp
index 47368780b62037..ff79fc89f827d1 100644
--- a/clang/lib/Sema/SemaAVR.cpp
+++ b/clang/lib/Sema/SemaAVR.cpp
@@ -30,6 +30,18 @@ void SemaAVR::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
   if (!AL.checkExactlyNumArgs(SemaRef, 0))
     return;
 
+  // AVR interrupt handlers must have no parameter and be void type.
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+    Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
+        << /*AVR*/ 3 << "interrupt" << 0;
+    return;
+  }
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+    Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
+        << /*AVR*/ 3 << "interrupt" << 1;
+    return;
+  }
+
   handleSimpleAttribute<AVRInterruptAttr>(*this, D, AL);
 }
 
@@ -43,6 +55,18 @@ void SemaAVR::handleSignalAttr(Decl *D, const ParsedAttr &AL) {
   if (!AL.checkExactlyNumArgs(SemaRef, 0))
     return;
 
+  // AVR signal handlers must have no parameter and be void type.
+  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
+    Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
+        << /*AVR*/ 3 << "signal" << 0;
+    return;
+  }
+  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
+    Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
+        << /*AVR*/ 3 << "signal" << 1;
+    return;
+  }
+
   handleSimpleAttribute<AVRSignalAttr>(*this, D, AL);
 }
 
diff --git a/clang/lib/Sema/SemaMIPS.cpp b/clang/lib/Sema/SemaMIPS.cpp
index 50d210e5b38140..2717c93e8c265c 100644
--- a/clang/lib/Sema/SemaMIPS.cpp
+++ b/clang/lib/Sema/SemaMIPS.cpp
@@ -272,13 +272,13 @@ void SemaMIPS::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
 
   if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
     Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
-        << /*MIPS*/ 0 << 0;
+        << /*MIPS*/ 0 << "interrupt" << 0;
     return;
   }
 
   if (!getFunctionOrMethodResultType(D)->isVoidType()) {
     Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
-        << /*MIPS*/ 0 << 1;
+        << /*MIPS*/ 0 << "interrupt" << 1;
     return;
   }
 
diff --git a/clang/lib/Sema/SemaMSP430.cpp b/clang/lib/Sema/SemaMSP430.cpp
index 4038a1ff61d63c..2b9e4769f06f0b 100644
--- a/clang/lib/Sema/SemaMSP430.cpp
+++ b/clang/lib/Sema/SemaMSP430.cpp
@@ -33,13 +33,13 @@ void SemaMSP430::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
 
   if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
     Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
-        << /*MSP430*/ 1 << 0;
+        << /*MSP430*/ 1 << "interrupt" << 0;
     return;
   }
 
   if (!getFunctionOrMethodResultType(D)->isVoidType()) {
     Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
-        << /*MSP430*/ 1 << 1;
+        << /*MSP430*/ 1 << "interrupt" << 1;
     return;
   }
 
diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp
index 163f7129a7b42b..51bca305f5690b 100644
--- a/clang/lib/Sema/SemaRISCV.cpp
+++ b/clang/lib/Sema/SemaRISCV.cpp
@@ -1458,13 +1458,13 @@ void SemaRISCV::handleInterruptAttr(Decl *D, const ParsedAttr &AL) {
 
   if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
     Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
-        << /*RISC-V*/ 2 << 0;
+        << /*RISC-V*/ 2 << "interrupt" << 0;
     return;
   }
 
   if (!getFunctionOrMethodResultType(D)->isVoidType()) {
     Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid)
-        << /*RISC-V*/ 2 << 1;
+        << /*RISC-V*/ 2 << "interrupt" << 1;
     return;
   }
 
diff --git a/clang/test/Sema/avr-interrupt-attr.c b/clang/test/Sema/avr-interrupt-attr.c
index 7aee7babcbe16a..065e93ffe9aaa8 100644
--- a/clang/test/Sema/avr-interrupt-attr.c
+++ b/clang/test/Sema/avr-interrupt-attr.c
@@ -5,4 +5,14 @@ struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attr
 
 __attribute__((interrupt(12))) void foo(void) { } // expected-error {{'interrupt' attribute takes no arguments}}
 
-__attribute__((interrupt)) void food(void) {}
+__attribute__((interrupt)) int fooa(void) { return 0; } // expected-warning {{'interrupt' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((interrupt)) void foob(int a) {} // expected-warning {{'interrupt' attribute only applies to functions that have no parameters}}
+
+__attribute__((signal)) int fooc(void) { return 0; } // expected-warning {{'signal' attribute only applies to functions that have a 'void' return type}}
+
+__attribute__((signal)) void food(int a) {} // expected-warning {{'signal' attribute only applies to functions that have no parameters}}
+
+__attribute__((interrupt)) void fooe(void) {}
+
+__attribute__((signal)) void foof(void) {}

[benshi001]

AVR interrupt/signal handlers must be void and has no argument, which in accordance with avr-gcc.

[jacquesguan]

LGTM

[benshi001]

it is strange: I have added the case avkevl mentioned to my repo, but this PR is not updated.

[benshi001]

Since the code is not updated automatically, I created a new PR for my code change.

#125997