llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Radovan Božić (bozicrHT) <details> <summary>Changes</summary> This PR extends `bugprone-narrowing-conversion` with a new `WarnOnTimeTNarrowingConversion` option. When enabled, the check diagnoses conversions from `time_t` values to integer types that may not preserve the full range of `time_t`. This is intended to help with Y2038-related audits. This follows the motivation discussed in the LLVM Discourse [RFC](https://discourse.llvm.org/t/rfc-y2038-check-for-lossy-time-t-conversions/91060). --- Full diff: https://github.com/llvm/llvm-project/pull/209856.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp (+122) - (modified) clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h (+10) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-time-t.cpp (+78) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp index 934b365a07cad..3310c9f77123d 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.cpp @@ -55,6 +55,7 @@ NarrowingConversionsCheck::NarrowingConversionsCheck(StringRef Name, WarnWithinTemplateInstantiation( Options.get("WarnWithinTemplateInstantiation", false)), WarnOnEquivalentBitWidth(Options.get("WarnOnEquivalentBitWidth", true)), + WarnOnTimeTNarrowingConversion(Options.get("WarnOnTimeTNarrowingConversion", false)), IgnoreConversionFromTypes(Options.get("IgnoreConversionFromTypes", "")), PedanticMode(Options.get("PedanticMode", false)) {} @@ -69,6 +70,7 @@ void NarrowingConversionsCheck::storeOptions( Options.store(Opts, "WarnWithinTemplateInstantiation", WarnWithinTemplateInstantiation); Options.store(Opts, "WarnOnEquivalentBitWidth", WarnOnEquivalentBitWidth); + Options.store(Opts, "WarnOnTimeTNarrowingConversion", WarnOnTimeTNarrowingConversion); Options.store(Opts, "IgnoreConversionFromTypes", IgnoreConversionFromTypes); Options.store(Opts, "PedanticMode", PedanticMode); } @@ -184,6 +186,24 @@ void NarrowingConversionsCheck::registerMatchers(MatchFinder *Finder) { unless(hasOperatorName("="))) .bind("binary_op"), this); + + // Explicit casts are checked only in the time_t narrowing mode, + // so the default behavior of this check remains unchanged. + if (WarnOnTimeTNarrowingConversion) { + Finder->addMatcher( + traverse( + TK_AsIs, + explicitCastExpr( + hasDestinationType(hasUnqualifiedDesugaredType(builtinType())), + hasSourceExpression( + expr(hasType(hasUnqualifiedDesugaredType(builtinType())))), + unless(hasSourceExpression(IsCeilFloorCallExpr)), + WarnWithinTemplateInstantiation + ? stmt() + : stmt(unless(isInTemplateInstantiation()))) + .bind("explicit_cast")), + this); + } } static const BuiltinType *getBuiltinType(const Expr &E) { @@ -238,6 +258,48 @@ struct IntegerRange { llvm::APSInt Upper; }; +bool hasTimeTTypedef(QualType QT) { + while (true) { + const auto *T = QT.getTypePtrOrNull(); + if (!T) + return false; + + if (const auto *TT = dyn_cast<TypedefType>(T)) { + const auto *TD = TT->getDecl(); + if (TD->getName() == "time_t" || TD->getName() == "__time_t") + return true; + } + + QualType NextInChain = QT->getLocallyUnqualifiedSingleStepDesugaredType(); + NextInChain = NextInChain.getNonReferenceType().getUnqualifiedType(); + if (NextInChain == QT) + break; + + QT = NextInChain; + } + + return false; +} + +bool exprMentionsTimeT(const Expr *E) { + + if (!E) + return false; + + E = E->IgnoreParenImpCasts(); + + if (hasTimeTTypedef(E->getType())) + return true; + + for (const Stmt *Child : E->children()) { + if (const auto *ChildExpr = dyn_cast_or_null<Expr>(Child)) + if (exprMentionsTimeT(ChildExpr)) + return true; + } + + return false; +} + } // namespace static IntegerRange createFromType(const ASTContext &Context, @@ -387,6 +449,54 @@ void NarrowingConversionsCheck::diagNarrowTypeOrConstant( diagNarrowType(SourceLoc, Lhs, Rhs); } +void NarrowingConversionsCheck::diagTimeTConversion(SourceLocation SourceLoc, + const Expr &Lhs, + const Expr &Rhs) { + diag(SourceLoc, + "conversion from %0 to %1 may not preserve the full range of time_t") + << getUnqualifiedType(Rhs) << getUnqualifiedType(Lhs); +} + +void NarrowingConversionsCheck::handleTimeTOnlyCast( + const ASTContext &Context, const CastExpr &Cast) { + const Expr &Lhs = Cast; + const Expr &Rhs = *Cast.getSubExprAsWritten(); + + if (Lhs.isInstantiationDependent() || Rhs.isInstantiationDependent()) + return; + + const SourceLocation SourceLoc = Cast.getExprLoc(); + handleTimeTConversion(Context, SourceLoc, Lhs, Rhs); +} + +void NarrowingConversionsCheck::handleTimeTConversion( + const ASTContext &Context, SourceLocation SourceLoc, + const Expr &Lhs, const Expr &Rhs) { + if (!WarnOnTimeTNarrowingConversion) + return; + + if (!exprMentionsTimeT(&Rhs)) + return; + + const BuiltinType *FromType = getBuiltinType(Rhs); + const BuiltinType *ToType = getBuiltinType(Lhs); + + if (!FromType || !ToType) + return; + + if (!ToType->isInteger() || !FromType->isInteger()) + return; + + // Usually not useful for Y2038. Keep bool conversions quiet. + if (ToType->getKind() == BuiltinType::Bool) + return; + + if (isWideEnoughToHold(Context, *FromType, *ToType)) + return; + + diagTimeTConversion(SourceLoc, Lhs, Rhs); +} + void NarrowingConversionsCheck::handleIntegralCast(const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs, @@ -572,6 +682,10 @@ void NarrowingConversionsCheck::handleConditionalOperatorArgument( void NarrowingConversionsCheck::handleImplicitCast( const ASTContext &Context, const ImplicitCastExpr &Cast) { + if (WarnOnTimeTNarrowingConversion) { + handleTimeTOnlyCast(Context, Cast); + return; + } if (Cast.getExprLoc().isMacroID()) return; const Expr &Lhs = Cast; @@ -626,6 +740,12 @@ void NarrowingConversionsCheck::handleBinaryOperator(const ASTContext &Context, const Expr &Rhs = *Op.getRHS(); if (Lhs.isInstantiationDependent() || Rhs.isInstantiationDependent()) return; + + if (WarnOnTimeTNarrowingConversion) { + handleTimeTConversion(Context, Op.getExprLoc(), Lhs, Rhs); + return; + } + if (handleConditionalOperator(Context, Lhs, Rhs)) return; handleBinaryOperator(Context, Rhs.getBeginLoc(), Lhs, Rhs); @@ -636,6 +756,8 @@ void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) { handleBinaryOperator(*Result.Context, *Op); else if (const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast")) handleImplicitCast(*Result.Context, *Cast); + else if (const auto *ECast = Result.Nodes.getNodeAs<ExplicitCastExpr>("explicit_cast")) + handleTimeTOnlyCast(*Result.Context, *ECast); else llvm_unreachable("must be binary operator or cast expression"); } diff --git a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h index e506e5b0315db..035a82383a0a5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/NarrowingConversionsCheck.h @@ -53,6 +53,9 @@ class NarrowingConversionsCheck : public ClangTidyCheck { SourceLocation SourceLoc, const Expr &Lhs, const Expr &Rhs); + void diagTimeTConversion(SourceLocation SourceLoc, const Expr &Lhs, + const Expr &Rhs); + void handleIntegralCast(const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs, const Expr &Rhs); @@ -79,6 +82,10 @@ class NarrowingConversionsCheck : public ClangTidyCheck { void handleFloatingCast(const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs, const Expr &Rhs); + void handleTimeTConversion(const ASTContext &Context, + SourceLocation SourceLoc, const Expr &Lhs, + const Expr &Rhs); + void handleBinaryOperator(const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs, const Expr &Rhs); @@ -93,6 +100,8 @@ class NarrowingConversionsCheck : public ClangTidyCheck { void handleBinaryOperator(const ASTContext &Context, const BinaryOperator &Op); + void handleTimeTOnlyCast(const ASTContext &Context, const CastExpr &Cast); + bool isWarningInhibitedByEquivalentSize(const ASTContext &Context, const BuiltinType &FromType, const BuiltinType &ToType) const; @@ -102,6 +111,7 @@ class NarrowingConversionsCheck : public ClangTidyCheck { const bool WarnOnFloatingPointNarrowingConversion; const bool WarnWithinTemplateInstantiation; const bool WarnOnEquivalentBitWidth; + const bool WarnOnTimeTNarrowingConversion; const StringRef IgnoreConversionFromTypes; const bool PedanticMode; }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-time-t.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-time-t.cpp new file mode 100644 index 0000000000000..de2880e6d83b6 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/narrowing-conversions-time-t.cpp @@ -0,0 +1,78 @@ +// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \ +// RUN: bugprone-narrowing-conversions %t -- \ +// RUN: -config='{CheckOptions: {bugprone-narrowing-conversions.WarnOnTimeTNarrowingConversion: true}}' + +typedef long time_t; +typedef time_t mytime; +typedef mytime opaque_time; + +namespace std { + using ::time_t; +} + +using int32_t = int; +using uint32_t = unsigned int; +using int64_t = long; +using uint64_t = unsigned long; + +time_t time(time_t *); + +void takes_int(int); +int returns_int(float f) { + return f; +} + +void ignore(int i, long l, double d) { + short s1 = i; + short s2 = (short)i; + int i1 = l, i2 = (int)l; + float f = d; + takes_int(l); +} + +void implicit(time_t t) { + + int32_t i32 = t; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:19: warning: conversion from 'time_t' (aka 'long') to 'int32_t' (aka 'int') may not preserve the full range of time_t + + uint32_t u32 = t; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:20: warning: conversion from 'time_t' (aka 'long') to 'uint32_t' (aka 'unsigned int') may not preserve the full range of time_t + + takes_int(t); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:15: warning: conversion from 'time_t' (aka 'long') to 'int' may not preserve the full range of time_t + + bool b = t; +} + +void explicit_test(time_t t) { + int i = (int)t; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:13: warning: conversion from 'time_t' (aka 'long') to 'int' may not preserve the full range of time_t + + int32_t i32 = (int32_t)t; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:19: warning: conversion from 'time_t' (aka 'long') to 'int32_t' (aka 'int') may not preserve the full range of time_t + + short s = (short)t; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:15: warning: conversion from 'time_t' (aka 'long') to 'short' may not preserve the full range of time_t + + int c = static_cast<int>(t); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:13: warning: conversion from 'time_t' (aka 'long') to 'int' may not preserve the full range of time_t + + int j = int(t); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:13: warning: conversion from 'time_t' (aka 'long') to 'int' may not preserve the full range of time_t +} + +void misc(std::time_t t, int offset) { + int i = t; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:13: warning: conversion from 'std::time_t' (aka 'long') to 'int' may not preserve the full range of time_t + + opaque_time op = time(nullptr); + uint32_t u32 = (uint32_t)op; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:20: warning: conversion from 'opaque_time' (aka 'long') to 'uint32_t' (aka 'unsigned int') may not preserve the full range of time_t + + int oper = t + offset; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:16: warning: conversion from 'std::time_t' (aka 'long') to 'int' may not preserve the full range of time_t + +#define TO_UINT32(arg) ((uint32_t)(arg)) + uint32_t trunc = TO_UINT32(t); + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:22: warning: conversion from 'std::time_t' (aka 'long') to 'uint32_t' (aka 'unsigned int') may not preserve the full range of time_t +} `````````` </details> https://github.com/llvm/llvm-project/pull/209856 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
