Skip to content

Commit ef08eb6

Browse files
committed
[ARM][clang] Fix warning for VFP function calls from interrupts.
This warning has two issues: - The interrupt attribute doesn't only change how volatile registers are treated; it also causes the function to return using an exception return instruction. This warning allows calls from one function with the interrupt attribute to another, and the diagnostic text suggests that not having the attribute on the callee is the problem. Actually making such a call will lead to a double exception return, which is unpredictable according to the ARM architecture manual section B9.1.1, "Restrictions on exception return instructions". Even on machines where an exception return from user/system mode is tolerated, if the callee's interrupt type is anything other than a supervisor call or secure monitor call, it will also return to a different address than a normal function would. For example, returning from an "IRQ" handler will return to lr - 4, which will generally result in calling the same function again. - It is part of the -Wextra diagnostic group and can't be individually disabled when using -Wextra, which also means the diagnostic text of this specific warning appears in the documentation of -Wextra. This change addresses both issues. Rather than check that the callee has the interrupt attribute, check that it uses the soft-float feature, which will prevent use of VFP state. The warning is also given its own diagnostic group.
1 parent 1351aab commit ef08eb6

File tree

5 files changed

+35
-15
lines changed

5 files changed

+35
-15
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,10 @@ Modified Compiler Flags
352352
evaluating to ``true`` and an empty body such as ``while(1);``)
353353
are considered infinite, even when the ``-ffinite-loop`` flag is set.
354354

355+
- Removed "arm interrupt calling convention" warning that was included in
356+
``-Wextra`` but did not have its own flag. Added
357+
``-Warm-interrupt-vfp-clobber`` that enables the modified warning.
358+
355359
Removed Compiler Flags
356360
-------------------------
357361

@@ -484,6 +488,13 @@ Improvements to Clang's diagnostics
484488
}
485489
};
486490

491+
- On ARM, Clang no longer suggests adding ``__attribute__((interrupt))`` to normal
492+
functions that are called from interrupt handlers to prevent clobbering VFP
493+
registers as part of ``-Wextra``. Following this suggestion leads to
494+
unpredictable behavior. Instead, ``-Warm-interrupt-vfp-clobber`` can now be
495+
used to detect calling functions that don't have VFP disabled with
496+
``__attribute__((target("soft-float")))`` from an interrupt handler.
497+
487498
Improvements to Clang's time-trace
488499
----------------------------------
489500

clang/include/clang/Basic/Attr.td

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3090,6 +3090,13 @@ def Target : InheritableAttr {
30903090
}
30913091
}
30923092

3093+
bool hasFeature(StringRef Feature) const {
3094+
StringRef Features = getFeaturesStr();
3095+
SmallVector<StringRef, 1> AttrFeatures;
3096+
Features.split(AttrFeatures, ",");
3097+
return Features.contains(Feature);
3098+
}
3099+
30933100
bool isDefaultVersion() const { return getFeaturesStr() == "default"; }
30943101
}];
30953102
}

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,9 +336,10 @@ def warn_anyx86_excessive_regsave : Warning<
336336
" with attribute 'no_caller_saved_registers'"
337337
" or be compiled with '-mgeneral-regs-only'">,
338338
InGroup<DiagGroup<"excessive-regsave">>;
339-
def warn_arm_interrupt_calling_convention : Warning<
340-
"call to function without interrupt attribute could clobber interruptee's VFP registers">,
341-
InGroup<Extra>;
339+
def warn_arm_interrupt_vfp_clobber : Warning<
340+
"calling a VFP-enabled function from an interrupt could clobber the "
341+
"interruptee's VFP registers">,
342+
InGroup<DiagGroup<"arm-interrupt-vfp-clobber">>;
342343
def warn_interrupt_attribute_invalid : Warning<
343344
"%select{MIPS|MSP430|RISC-V}0 'interrupt' attribute only applies to "
344345
"functions that have %select{no parameters|a 'void' return type}1">,

clang/lib/Sema/SemaExpr.cpp

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6942,22 +6942,23 @@ ExprResult Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl,
69426942
return ExprError();
69436943
}
69446944

6945-
// Interrupt handlers don't save off the VFP regs automatically on ARM,
6946-
// so there's some risk when calling out to non-interrupt handler functions
6947-
// that the callee might not preserve them. This is easy to diagnose here,
6948-
// but can be very challenging to debug.
6949-
// Likewise, X86 interrupt handlers may only call routines with attribute
6950-
// no_caller_saved_registers since there is no efficient way to
6951-
// save and restore the non-GPR state.
69526945
if (auto *Caller = getCurFunctionDecl()) {
6946+
// Interrupt handlers don't save volatile VFP registers automatically on
6947+
// ARM, so calling other functions that use VFP will likely cause the
6948+
// interruptee's VFP state to be clobbered. This is easy to diagnose here,
6949+
// but can be very challenging to debug.
69536950
if (Caller->hasAttr<ARMInterruptAttr>()) {
69546951
bool VFP = Context.getTargetInfo().hasFeature("vfp");
6955-
if (VFP && (!FDecl || !FDecl->hasAttr<ARMInterruptAttr>())) {
6956-
Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_calling_convention);
6952+
if (VFP && (!FDecl || !FDecl->hasAttr<TargetAttr>() ||
6953+
!FDecl->getAttr<TargetAttr>()->hasFeature("soft-float"))) {
6954+
Diag(Fn->getExprLoc(), diag::warn_arm_interrupt_vfp_clobber);
69576955
if (FDecl)
69586956
Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
69596957
}
69606958
}
6959+
// X86 interrupt handlers may only call routines with attribute
6960+
// no_caller_saved_registers since there is no efficient way to
6961+
// save and restore the non-GPR state.
69616962
if (Caller->hasAttr<AnyX86InterruptAttr>() ||
69626963
Caller->hasAttr<AnyX86NoCallerSavedRegistersAttr>()) {
69636964
const TargetInfo &TI = Context.getTargetInfo();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,21 @@ __attribute__((interrupt(""))) void foo10(void) {}
2323
// expected-note@+2 {{'callee1' declared here}}
2424
#endif
2525
void callee1(void);
26-
__attribute__((interrupt("IRQ"))) void callee2(void);
26+
__attribute__((target("soft-float"))) void callee2(void);
2727
void caller1(void) {
2828
callee1();
2929
callee2();
3030
}
3131

3232
#ifndef SOFT
3333
__attribute__((interrupt("IRQ"))) void caller2(void) {
34-
callee1(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
34+
callee1(); // expected-warning {{calling a VFP-enabled function from an interrupt could clobber the interruptee's VFP registers}}
3535
callee2();
3636
}
3737

3838
void (*callee3)(void);
3939
__attribute__((interrupt("IRQ"))) void caller3(void) {
40-
callee3(); // expected-warning {{call to function without interrupt attribute could clobber interruptee's VFP registers}}
40+
callee3(); // expected-warning {{calling a VFP-enabled function from an interrupt could clobber the interruptee's VFP registers}}
4141
}
4242
#else
4343
__attribute__((interrupt("IRQ"))) void caller2(void) {

0 commit comments

Comments
 (0)