https://github.com/benshi001 updated https://github.com/llvm/llvm-project/pull/125997
>From 2c02723208bd48e82621e3bee5b778f6c8bcd2e5 Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Mon, 27 Jan 2025 13:04:45 +0800 Subject: [PATCH 1/2] [clang][Sema] Emit warnings about incorrect AVR interrupt/signal handlers 1. interrupt/signal handlers can not have parameters 2. interrupt/signal handlers must be 'void' type --- .../clang/Basic/DiagnosticSemaKinds.td | 4 ++-- clang/lib/Sema/SemaAVR.cpp | 24 +++++++++++++++++++ clang/lib/Sema/SemaMIPS.cpp | 4 ++-- clang/lib/Sema/SemaMSP430.cpp | 4 ++-- clang/lib/Sema/SemaRISCV.cpp | 4 ++-- clang/test/Sema/avr-interript-signal-attr.c | 22 +++++++++++++++++ clang/test/Sema/avr-interrupt-attr.c | 8 ------- clang/test/Sema/avr-signal-attr.c | 8 ------- 8 files changed, 54 insertions(+), 24 deletions(-) create mode 100644 clang/test/Sema/avr-interript-signal-attr.c delete mode 100644 clang/test/Sema/avr-interrupt-attr.c delete mode 100644 clang/test/Sema/avr-signal-attr.c diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 7b3b932c482baa2..3f732938d6b6b3a 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 47368780b620379..ff79fc89f827d16 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 50d210e5b381405..2717c93e8c265c7 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 4038a1ff61d63ce..2b9e4769f06f0be 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 163f7129a7b42bb..51bca305f5690b3 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-interript-signal-attr.c b/clang/test/Sema/avr-interript-signal-attr.c new file mode 100644 index 000000000000000..7360dbdedb695ca --- /dev/null +++ b/clang/test/Sema/avr-interript-signal-attr.c @@ -0,0 +1,22 @@ +// RUN: %clang_cc1 %s -triple avr-unknown-unknown -verify -fsyntax-only +struct a { int b; }; + +struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}} + +__attribute__((interrupt(12))) void foo(void) { } // expected-error {{'interrupt' attribute takes no arguments}} + +__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__((interrupt)) void foof() {} + +__attribute__((signal)) void foog(void) {} + +__attribute__((signal)) void fooh() {} diff --git a/clang/test/Sema/avr-interrupt-attr.c b/clang/test/Sema/avr-interrupt-attr.c deleted file mode 100644 index 7aee7babcbe16a8..000000000000000 --- a/clang/test/Sema/avr-interrupt-attr.c +++ /dev/null @@ -1,8 +0,0 @@ -// RUN: %clang_cc1 %s -triple avr-unknown-unknown -verify -fsyntax-only -struct a { int b; }; - -struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}} - -__attribute__((interrupt(12))) void foo(void) { } // expected-error {{'interrupt' attribute takes no arguments}} - -__attribute__((interrupt)) void food(void) {} diff --git a/clang/test/Sema/avr-signal-attr.c b/clang/test/Sema/avr-signal-attr.c deleted file mode 100644 index 1ec36c74a25ebbc..000000000000000 --- a/clang/test/Sema/avr-signal-attr.c +++ /dev/null @@ -1,8 +0,0 @@ -// RUN: %clang_cc1 %s -triple avr-unknown-unknown -verify -fsyntax-only -struct a { int b; }; - -struct a test __attribute__((signal)); // expected-warning {{'signal' attribute only applies to functions}} - -__attribute__((signal(12))) void foo(void) { } // expected-error {{'signal' attribute takes no arguments}} - -__attribute__((signal)) void food(void) {} >From 4ee7c5c1b43269b2ddfbee47f50055d74e2283b9 Mon Sep 17 00:00:00 2001 From: Ben Shi <benn...@tencent.com> Date: Thu, 6 Feb 2025 18:48:26 +0800 Subject: [PATCH 2/2] fix according to reviewer's comments --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 9 +++++---- clang/lib/Sema/SemaAVR.cpp | 16 ++++++++-------- clang/lib/Sema/SemaMIPS.cpp | 8 ++++---- clang/lib/Sema/SemaMSP430.cpp | 8 ++++---- clang/lib/Sema/SemaRISCV.cpp | 8 ++++---- 5 files changed, 25 insertions(+), 24 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 3f732938d6b6b3a..2501e39a7b9b8b3 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -350,10 +350,11 @@ def warn_arm_interrupt_vfp_clobber : Warning< InGroup<DiagGroup<"arm-interrupt-vfp-clobber">>; def err_arm_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def warn_interrupt_attribute_invalid : Warning< - "%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_interrupt_signal_attribute_invalid : Warning< + "%select{MIPS|MSP430|RISC-V|AVR}0 '%select{interrupt|signal}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>; def note_riscv_repeated_interrupt_attribute : Note< diff --git a/clang/lib/Sema/SemaAVR.cpp b/clang/lib/Sema/SemaAVR.cpp index ff79fc89f827d16..389c55404fde3d8 100644 --- a/clang/lib/Sema/SemaAVR.cpp +++ b/clang/lib/Sema/SemaAVR.cpp @@ -32,13 +32,13 @@ void SemaAVR::handleInterruptAttr(Decl *D, const ParsedAttr &AL) { // 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; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*AVR*/ 3 << /*interrupt*/ 0 << 0; return; } if (!getFunctionOrMethodResultType(D)->isVoidType()) { - Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid) - << /*AVR*/ 3 << "interrupt" << 1; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*AVR*/ 3 << /*interrupt*/ 0 << 1; return; } @@ -57,13 +57,13 @@ void SemaAVR::handleSignalAttr(Decl *D, const ParsedAttr &AL) { // 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; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*AVR*/ 3 << /*signal*/ 1 << 0; return; } if (!getFunctionOrMethodResultType(D)->isVoidType()) { - Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid) - << /*AVR*/ 3 << "signal" << 1; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*AVR*/ 3 << /*signal*/ 1 << 1; return; } diff --git a/clang/lib/Sema/SemaMIPS.cpp b/clang/lib/Sema/SemaMIPS.cpp index 2717c93e8c265c7..ce6ad6d9cc3f305 100644 --- a/clang/lib/Sema/SemaMIPS.cpp +++ b/clang/lib/Sema/SemaMIPS.cpp @@ -271,14 +271,14 @@ void SemaMIPS::handleInterruptAttr(Decl *D, const ParsedAttr &AL) { } if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { - Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid) - << /*MIPS*/ 0 << "interrupt" << 0; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*MIPS*/ 0 << /*interrupt*/ 0 << 0; return; } if (!getFunctionOrMethodResultType(D)->isVoidType()) { - Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid) - << /*MIPS*/ 0 << "interrupt" << 1; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*MIPS*/ 0 << /*interrupt*/ 0 << 1; return; } diff --git a/clang/lib/Sema/SemaMSP430.cpp b/clang/lib/Sema/SemaMSP430.cpp index 2b9e4769f06f0be..5bf931e388f030b 100644 --- a/clang/lib/Sema/SemaMSP430.cpp +++ b/clang/lib/Sema/SemaMSP430.cpp @@ -32,14 +32,14 @@ void SemaMSP430::handleInterruptAttr(Decl *D, const ParsedAttr &AL) { } if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { - Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid) - << /*MSP430*/ 1 << "interrupt" << 0; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*MSP430*/ 1 << /*interrupt*/ 0 << 0; return; } if (!getFunctionOrMethodResultType(D)->isVoidType()) { - Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid) - << /*MSP430*/ 1 << "interrupt" << 1; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*MSP430*/ 1 << /*interrupt*/ 0 << 1; return; } diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp index 51bca305f5690b3..8a5037d04512573 100644 --- a/clang/lib/Sema/SemaRISCV.cpp +++ b/clang/lib/Sema/SemaRISCV.cpp @@ -1457,14 +1457,14 @@ 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 << "interrupt" << 0; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*RISC-V*/ 2 << /*interrupt*/ 0 << 0; return; } if (!getFunctionOrMethodResultType(D)->isVoidType()) { - Diag(D->getLocation(), diag::warn_interrupt_attribute_invalid) - << /*RISC-V*/ 2 << "interrupt" << 1; + Diag(D->getLocation(), diag::warn_interrupt_signal_attribute_invalid) + << /*RISC-V*/ 2 << /*interrupt*/ 0 << 1; return; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits