Author: Richard Smith Date: 2020-08-06T13:28:50-07:00 New Revision: d6492d874478b1d3b1ce3adb4c3044618bec29e9
URL: https://github.com/llvm/llvm-project/commit/d6492d874478b1d3b1ce3adb4c3044618bec29e9 DIFF: https://github.com/llvm/llvm-project/commit/d6492d874478b1d3b1ce3adb4c3044618bec29e9.diff LOG: Add -Wtautological-value-range-compare warning. This warning diagnoses cases where an expression is compared to a constant, and the comparison is tautological due to the form of the expression (but not merely due to its type). This applies in cases such as comparisons of bit-fields and the result of bit-masks. The new warning is added to the Clang diagnostic group -Wtautological-constant-in-range-compare but not to the formerly-equivalent GCC-compatibility diagnostic group -Wtype-limits, which retains its old meaning of diagnosing only tautological comparisons to extremal values of a type (eg, int > INT_MAX). Reviewed By: rtrieu Differential Revision: https://reviews.llvm.org/D85256 Added: Modified: clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaChecking.cpp clang/test/Sema/tautological-constant-compare.c Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index be62461faef48..5ddd37e9972af 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -555,12 +555,16 @@ def IntInBoolContext : DiagGroup<"int-in-bool-context">; def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">; def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">; def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">; +// For compatibility with GCC. Tautological comparison warnings for constants +// that are an extremal value of the type. +def TypeLimits : DiagGroup<"type-limits", [TautologicalTypeLimitCompare, + TautologicalUnsignedZeroCompare, + TautologicalUnsignedEnumZeroCompare]>; +// Additional tautological comparison warnings based on the expression, not +// only on its type. +def TautologicalValueRangeCompare : DiagGroup<"tautological-value-range-compare">; def TautologicalInRangeCompare : DiagGroup<"tautological-constant-in-range-compare", - [TautologicalTypeLimitCompare, - TautologicalUnsignedZeroCompare, - TautologicalUnsignedEnumZeroCompare]>; -// For compatibility with GCC; -Wtype-limits = -Wtautological-constant-in-range-compare -def TypeLimits : DiagGroup<"type-limits", [TautologicalInRangeCompare]>; + [TypeLimits, TautologicalValueRangeCompare]>; def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">; def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare", [TautologicalOutOfRangeCompare]>; @@ -624,7 +628,6 @@ def ImplicitFallthrough : DiagGroup<"implicit-fallthrough", def InvalidPPToken : DiagGroup<"invalid-pp-token">; def Trigraphs : DiagGroup<"trigraphs">; -def : DiagGroup<"type-limits">; def UndefinedReinterpretCast : DiagGroup<"undefined-reinterpret-cast">; def ReinterpretBaseClass : DiagGroup<"reinterpret-base-class">; def Unicode : DiagGroup<"unicode">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 7bcff3eb4d8c5..1aeaf590cbb4b 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6588,6 +6588,12 @@ def warn_tautological_compare_objc_bool : Warning< "result of comparison of constant %0 with expression of type 'BOOL'" " is always %1, as the only well defined values for 'BOOL' are YES and NO">, InGroup<TautologicalObjCBoolCompare>; +def subst_int_range : TextSubstitution<"%0-bit %select{signed|unsigned}1 value">; +def warn_tautological_compare_value_range : Warning< + "result of comparison of " + "%select{%4|%sub{subst_int_range}1,2}0 %3 " + "%select{%sub{subst_int_range}1,2|%4}0 is always %5">, + InGroup<TautologicalValueRangeCompare>, DefaultIgnore; def warn_mixed_sign_comparison : Warning< "comparison of integers of diff erent signs: %0 and %1">, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 5c2092f1447ad..bbd856e9262e0 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -10792,15 +10792,16 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E, S.Context.hasSameUnqualifiedType(Constant->getType(), Other->getType())) return false; - // TODO: Investigate using GetExprRange() to get tighter bounds - // on the bit ranges. + IntRange OtherValueRange = + GetExprRange(S.Context, Other, S.isConstantEvaluated()); + QualType OtherT = Other->getType(); if (const auto *AT = OtherT->getAs<AtomicType>()) OtherT = AT->getValueType(); - IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT); + IntRange OtherTypeRange = IntRange::forValueOfType(S.Context, OtherT); // Special case for ObjC BOOL on targets where its a typedef for a signed char - // (Namely, macOS). + // (Namely, macOS). FIXME: IntRange::forValueOfType should do this. bool IsObjCSignedCharBool = S.getLangOpts().ObjC && S.NSAPIObj->isObjCBOOLType(OtherT) && OtherT->isSpecificBuiltinType(BuiltinType::SChar); @@ -10810,17 +10811,32 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E, bool OtherIsBooleanDespiteType = !OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue(); if (OtherIsBooleanDespiteType || IsObjCSignedCharBool) - OtherRange = IntRange::forBoolType(); + OtherTypeRange = OtherValueRange = IntRange::forBoolType(); - // Determine the promoted range of the other type and see if a comparison of - // the constant against that range is tautological. - PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(), - Value.isUnsigned()); - auto Cmp = OtherPromotedRange.compare(Value); + // Check if all values in the range of possible values of this expression + // lead to the same comparison outcome. + PromotedRange OtherPromotedValueRange(OtherValueRange, Value.getBitWidth(), + Value.isUnsigned()); + auto Cmp = OtherPromotedValueRange.compare(Value); auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp, RhsConstant); if (!Result) return false; + // Also consider the range determined by the type alone. This allows us to + // classify the warning under the proper diagnostic group. + bool TautologicalTypeCompare = false; + { + PromotedRange OtherPromotedTypeRange(OtherTypeRange, Value.getBitWidth(), + Value.isUnsigned()); + auto TypeCmp = OtherPromotedTypeRange.compare(Value); + if (auto TypeResult = PromotedRange::constantValue(E->getOpcode(), TypeCmp, + RhsConstant)) { + TautologicalTypeCompare = true; + Cmp = TypeCmp; + Result = TypeResult; + } + } + // Suppress the diagnostic for an in-range comparison if the constant comes // from a macro or enumerator. We don't want to diagnose // @@ -10849,6 +10865,14 @@ static bool CheckTautologicalComparison(Sema &S, BinaryOperator *E, OS << Value; } + if (!TautologicalTypeCompare) { + S.Diag(E->getOperatorLoc(), diag::warn_tautological_compare_value_range) + << RhsConstant << OtherValueRange.Width << OtherValueRange.NonNegative + << E->getOpcodeStr() << OS.str() << *Result + << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange(); + return true; + } + if (IsObjCSignedCharBool) { S.DiagRuntimeBehavior(E->getOperatorLoc(), E, S.PDiag(diag::warn_tautological_compare_objc_bool) diff --git a/clang/test/Sema/tautological-constant-compare.c b/clang/test/Sema/tautological-constant-compare.c index 4f9b43b9f8982..7bf70eda7df4d 100644 --- a/clang/test/Sema/tautological-constant-compare.c +++ b/clang/test/Sema/tautological-constant-compare.c @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST -verify -x c++ %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-constant-in-range-compare -DTEST=2 -verify -x c++ %s // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify %s // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtautological-type-limit-compare -DTEST -verify -x c++ %s // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -Wtype-limits -DTEST -verify %s @@ -545,9 +545,9 @@ int main() if (maybe >= e) return 0; - // For the time being, use the declared type of bit-fields rather than their - // length when determining whether a value is in-range. - // FIXME: Reconsider this. + // We only warn on out-of-range bitfields and expressions with limited range + // under -Wtantological-in-range-compare, not under -Wtype-limits, because + // the warning is not based on the type alone. struct A { int a : 3; unsigned b : 3; @@ -555,13 +555,36 @@ int main() unsigned long d : 3; } a; if (a.a < 3) {} - if (a.a < 4) {} + if (a.a < 4) {} // #bitfield1 if (a.b < 7) {} - if (a.b < 8) {} + if (a.b < 8) {} // #bitfield2 if (a.c < 3) {} - if (a.c < 4) {} + if (a.c < 4) {} // #bitfield3 if (a.d < 7) {} - if (a.d < 8) {} + if (a.d < 8) {} // #bitfield4 +#if TEST == 2 + // expected-warning@#bitfield1 {{comparison of 3-bit signed value < 4 is always true}} + // expected-warning@#bitfield2 {{comparison of 3-bit unsigned value < 8 is always true}} + // expected-warning@#bitfield3 {{comparison of 3-bit signed value < 4 is always true}} + // expected-warning@#bitfield4 {{comparison of 3-bit unsigned value < 8 is always true}} +#endif + + if ((s & 0xff) < 0) {} // #valuerange1 + if ((s & 0xff) < 1) {} + if ((s & -3) < -4) {} // #valuerange2 + if ((s & -3) < -3) {} + if ((s & -3) < 4u) {} // (true if s non-negative) + if ((s & -3) > 4u) {} // (true if s negative) + if ((s & -3) == 4u) {} // #valuerange3 (never true) + if ((s & -3) == 3u) {} + if ((s & -3) == -5u) {} // #valuerange4 + if ((s & -3) == -4u) {} +#if TEST == 2 + // expected-warning@#valuerange1 {{comparison of 8-bit unsigned value < 0 is always false}} + // expected-warning@#valuerange2 {{comparison of 3-bit signed value < -4 is always false}} + // expected-warning@#valuerange3 {{comparison of 3-bit signed value == 4 is always false}} + // expected-warning@#valuerange4 {{comparison of 3-bit signed value == 4294967291 is always false}} +#endif return 1; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
