Skip to content

[CLANG] Add warning when INF or NAN are used in a binary operation or as function argument in fast math mode. #76873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jan 22, 2024
Merged
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,11 @@ Improvements to Clang's diagnostics
- Clang now diagnoses narrowing conversions involving const references.
(`#63151: <https://github.com/llvm/llvm-project/issues/63151>`_).
- Clang now diagnoses unexpanded packs within the template argument lists of function template specializations.
- The warning `-Wnan-and-infinity-disabled` is now emitted when ``INFINITY``
or ``NAN`` are used in arithmetic operations or function arguments in
floating-point mode where ``INFINITY`` or ``NAN`` don't have the expected
values.

- Clang now diagnoses attempts to bind a bitfield to an NTTP of a reference type as erroneous
converted constant expression and not as a reference to subobject.

Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticCommonKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ def warn_pragma_debug_missing_argument : Warning<
def warn_pragma_debug_unexpected_argument : Warning<
"unexpected argument to debug command">, InGroup<IgnoredPragmas>;

def warn_fp_nan_inf_when_disabled : Warning<
"use of %select{infinity|NaN}0%select{| via a macro}1 results in undefined behavior "
"due to the currently enabled floating-point options">,
InGroup<DiagGroup<"nan-and-infinity-disabled">>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def warn_fp_nan_inf_when_disabled : Warning<
"use of %select{infinity|NaN}0%select{| via a macro}1 results in undefined behavior "
"due to the currently enabled floating-point options">,
InGroup<DiagGroup<"nan-and-infinity-disabled">>;
def warn_fp_nan_inf_when_disabled : Warning<
"use of %select{infinity|NaN}0%select{| via a macro}1 is undefined behavior "
"due to the currently enabled floating-point options">,
InGroup<DiagGroup<"nan-infinity-disabled">>;

Sorry for still puttering around with what color to paint the bikeshed, but I think this might be slightly more concise wording and a somewhat better diagnostic group.

}

// Parse && Sema
Expand Down
5 changes: 5 additions & 0 deletions clang/include/clang/Lex/Preprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -2835,6 +2835,10 @@ class Preprocessor {
if (Identifier.getIdentifierInfo()->isRestrictExpansion() &&
!SourceMgr.isInMainFile(Identifier.getLocation()))
emitRestrictExpansionWarning(Identifier);

if (Identifier.getIdentifierInfo()->getName() == "INFINITY" ||
Identifier.getIdentifierInfo()->getName() == "NAN")
emitRestrictInfNaNWarning(Identifier);
}

static void processPathForFileMacro(SmallVectorImpl<char> &Path,
Expand All @@ -2850,6 +2854,7 @@ class Preprocessor {
void emitMacroDeprecationWarning(const Token &Identifier) const;
void emitRestrictExpansionWarning(const Token &Identifier) const;
void emitFinalMacroWarning(const Token &Identifier, bool IsUndef) const;
void emitRestrictInfNaNWarning(const Token &Identifier) const;

/// This boolean state keeps track if the current scanned token (by this PP)
/// is in an "-Wunsafe-buffer-usage" opt-out region. Assuming PP scans a
Expand Down
7 changes: 5 additions & 2 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -13901,8 +13901,9 @@ class Sema final {

bool SemaBuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall);
bool SemaBuiltinVAStartARMMicrosoft(CallExpr *Call);
bool SemaBuiltinUnorderedCompare(CallExpr *TheCall);
bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs);
bool SemaBuiltinUnorderedCompare(CallExpr *TheCall, unsigned BuiltinID);
bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs,
unsigned BuiltinID);
bool SemaBuiltinComplex(CallExpr *TheCall);
bool SemaBuiltinVSX(CallExpr *TheCall);
bool SemaBuiltinOSLogFormat(CallExpr *TheCall);
Expand Down Expand Up @@ -14006,6 +14007,8 @@ class Sema final {
SourceRange range,
llvm::SmallBitVector &CheckedVarArgs);

void CheckInfNaNFunction(const CallExpr *Call, const FunctionDecl *FDecl);

void CheckAbsoluteValueFunction(const CallExpr *Call,
const FunctionDecl *FDecl);

Expand Down
5 changes: 5 additions & 0 deletions clang/lib/Lex/Preprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1457,6 +1457,11 @@ void Preprocessor::emitRestrictExpansionWarning(const Token &Identifier) const {
Diag(Info.Location, diag::note_pp_macro_annotation) << 1;
}

void Preprocessor::emitRestrictInfNaNWarning(const Token &Identifier) const {
if (getLangOpts().NoHonorInfs)
Diag(Identifier, diag::warn_fp_nan_inf_when_disabled) << 0 << 1;
}

void Preprocessor::emitFinalMacroWarning(const Token &Identifier,
bool IsUndef) const {
const MacroAnnotations &A =
Expand Down
48 changes: 42 additions & 6 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2169,6 +2169,7 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
ICEArguments &= ~(1 << ArgNo);
}

FPOptions FPO;
switch (BuiltinID) {
case Builtin::BI__builtin___CFStringMakeConstantString:
// CFStringMakeConstantString is currently not implemented for GOFF (i.e.,
Expand Down Expand Up @@ -2245,15 +2246,15 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
case Builtin::BI__builtin_islessequal:
case Builtin::BI__builtin_islessgreater:
case Builtin::BI__builtin_isunordered:
if (SemaBuiltinUnorderedCompare(TheCall))
if (SemaBuiltinUnorderedCompare(TheCall, BuiltinID))
return ExprError();
break;
case Builtin::BI__builtin_fpclassify:
if (SemaBuiltinFPClassification(TheCall, 6))
if (SemaBuiltinFPClassification(TheCall, 6, BuiltinID))
return ExprError();
break;
case Builtin::BI__builtin_isfpclass:
if (SemaBuiltinFPClassification(TheCall, 2))
if (SemaBuiltinFPClassification(TheCall, 2, BuiltinID))
return ExprError();
break;
case Builtin::BI__builtin_isfinite:
Expand All @@ -2267,7 +2268,7 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
case Builtin::BI__builtin_signbit:
case Builtin::BI__builtin_signbitf:
case Builtin::BI__builtin_signbitl:
if (SemaBuiltinFPClassification(TheCall, 1))
if (SemaBuiltinFPClassification(TheCall, 1, BuiltinID))
return ExprError();
break;
case Builtin::BI__builtin_shufflevector:
Expand Down Expand Up @@ -7636,6 +7637,7 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,

CheckAbsoluteValueFunction(TheCall, FDecl);
CheckMaxUnsignedZero(TheCall, FDecl);
CheckInfNaNFunction(TheCall, FDecl);

if (getLangOpts().ObjC)
DiagnoseCStringFormatDirectiveInCFAPI(*this, FDecl, Args, NumArgs);
Expand Down Expand Up @@ -9105,10 +9107,15 @@ bool Sema::SemaBuiltinVAStartARMMicrosoft(CallExpr *Call) {

/// SemaBuiltinUnorderedCompare - Handle functions like __builtin_isgreater and
/// friends. This is declared to take (...), so we have to check everything.
bool Sema::SemaBuiltinUnorderedCompare(CallExpr *TheCall) {
bool Sema::SemaBuiltinUnorderedCompare(CallExpr *TheCall, unsigned BuiltinID) {
if (checkArgCount(*this, TheCall, 2))
return true;

if (BuiltinID == Builtin::BI__builtin_isunordered &&
TheCall->getFPFeaturesInEffect(getLangOpts()).getNoHonorNaNs())
Diag(TheCall->getBeginLoc(), diag::warn_fp_nan_inf_when_disabled)
<< 1 << 0 << TheCall->getSourceRange();

ExprResult OrigArg0 = TheCall->getArg(0);
ExprResult OrigArg1 = TheCall->getArg(1);

Expand Down Expand Up @@ -9143,10 +9150,23 @@ bool Sema::SemaBuiltinUnorderedCompare(CallExpr *TheCall) {
/// SemaBuiltinSemaBuiltinFPClassification - Handle functions like
/// __builtin_isnan and friends. This is declared to take (...), so we have
/// to check everything.
bool Sema::SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs) {
bool Sema::SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs,
unsigned BuiltinID) {
if (checkArgCount(*this, TheCall, NumArgs))
return true;

FPOptions FPO = TheCall->getFPFeaturesInEffect(getLangOpts());
if (FPO.getNoHonorInfs() && (BuiltinID == Builtin::BI__builtin_isfinite ||
BuiltinID == Builtin::BI__builtin_isinf ||
BuiltinID == Builtin::BI__builtin_isinf_sign))
Diag(TheCall->getBeginLoc(), diag::warn_fp_nan_inf_when_disabled)
<< 0 << 0 << TheCall->getSourceRange();

if (FPO.getNoHonorNaNs() && (BuiltinID == Builtin::BI__builtin_isnan ||
BuiltinID == Builtin::BI__builtin_isunordered))
Diag(TheCall->getBeginLoc(), diag::warn_fp_nan_inf_when_disabled)
<< 1 << 0 << TheCall->getSourceRange();

bool IsFPClass = NumArgs == 2;

// Find out position of floating-point argument.
Expand Down Expand Up @@ -12893,6 +12913,22 @@ static bool IsStdFunction(const FunctionDecl *FDecl,
return true;
}

void Sema::CheckInfNaNFunction(const CallExpr *Call,
const FunctionDecl *FDecl) {
FPOptions FPO = Call->getFPFeaturesInEffect(getLangOpts());
if ((IsStdFunction(FDecl, "isnan") || IsStdFunction(FDecl, "isunordered") ||
(Call->getBuiltinCallee() == Builtin::BI__builtin_nanf)) &&
FPO.getNoHonorNaNs())
Diag(Call->getBeginLoc(), diag::warn_fp_nan_inf_when_disabled)
<< 1 << 0 << Call->getSourceRange();
else if ((IsStdFunction(FDecl, "isinf") ||
(IsStdFunction(FDecl, "isfinite") ||
(FDecl->getIdentifier() && FDecl->getName() == "infinity"))) &&
FPO.getNoHonorInfs())
Diag(Call->getBeginLoc(), diag::warn_fp_nan_inf_when_disabled)
<< 0 << 0 << Call->getSourceRange();
}

// Warn when using the wrong abs() function.
void Sema::CheckAbsoluteValueFunction(const CallExpr *Call,
const FunctionDecl *FDecl) {
Expand Down
Loading