From ea92b1f9d0fc31f1fd97ad04eb0412003a37cb0d Mon Sep 17 00:00:00 2001 From: Karl-Johan Karlsson Date: Sun, 31 Mar 2024 11:09:52 +0200 Subject: [Sema] Implement support for -Wformat-signedness (#74440) In gcc there exist a modifier option -Wformat-signedness that turns on additional signedness warnings in the already existing -Wformat warning. This patch implements that support in clang. This is done by adding a dummy warning diag::warn_format_conversion_argument_type_mismatch_signedness that is never emitted and only used as an option to toggle the signedness warning in -Wformat. This will ensure gcc compatibility. --- clang/lib/AST/FormatString.cpp | 29 +++++++++++++++++++---------- clang/lib/Sema/SemaChecking.cpp | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 12 deletions(-) (limited to 'clang/lib') diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp index 0c80ad1..da8164ba 100644 --- a/clang/lib/AST/FormatString.cpp +++ b/clang/lib/AST/FormatString.cpp @@ -413,7 +413,7 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const { return Match; if (const auto *BT = argTy->getAs()) { // Check if the only difference between them is signed vs unsigned - // if true, we consider they are compatible. + // if true, return match signedness. switch (BT->getKind()) { default: break; @@ -423,44 +423,53 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const { [[fallthrough]]; case BuiltinType::Char_S: case BuiltinType::SChar: + if (T == C.UnsignedShortTy || T == C.ShortTy) + return NoMatchTypeConfusion; + if (T == C.UnsignedCharTy) + return NoMatchSignedness; + if (T == C.SignedCharTy) + return Match; + break; case BuiltinType::Char_U: case BuiltinType::UChar: if (T == C.UnsignedShortTy || T == C.ShortTy) return NoMatchTypeConfusion; - if (T == C.UnsignedCharTy || T == C.SignedCharTy) + if (T == C.UnsignedCharTy) return Match; + if (T == C.SignedCharTy) + return NoMatchSignedness; break; case BuiltinType::Short: if (T == C.UnsignedShortTy) - return Match; + return NoMatchSignedness; break; case BuiltinType::UShort: if (T == C.ShortTy) - return Match; + return NoMatchSignedness; break; case BuiltinType::Int: if (T == C.UnsignedIntTy) - return Match; + return NoMatchSignedness; break; case BuiltinType::UInt: if (T == C.IntTy) - return Match; + return NoMatchSignedness; break; case BuiltinType::Long: if (T == C.UnsignedLongTy) - return Match; + return NoMatchSignedness; break; case BuiltinType::ULong: if (T == C.LongTy) - return Match; + return NoMatchSignedness; break; case BuiltinType::LongLong: if (T == C.UnsignedLongLongTy) - return Match; + return NoMatchSignedness; break; case BuiltinType::ULongLong: if (T == C.LongLongTy) - return Match; + return NoMatchSignedness; break; } // "Partially matched" because of promotions? diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 2684535..11401b6 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -12446,6 +12446,19 @@ isArithmeticArgumentPromotion(Sema &S, const ImplicitCastExpr *ICE) { S.Context.getFloatingTypeOrder(From, To) < 0; } +static analyze_format_string::ArgType::MatchKind +handleFormatSignedness(analyze_format_string::ArgType::MatchKind Match, + DiagnosticsEngine &Diags, SourceLocation Loc) { + if (Match == analyze_format_string::ArgType::NoMatchSignedness) { + Match = + Diags.isIgnored( + diag::warn_format_conversion_argument_type_mismatch_signedness, Loc) + ? analyze_format_string::ArgType::Match + : analyze_format_string::ArgType::NoMatch; + } + return Match; +} + bool CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, const char *StartSpecifier, @@ -12489,6 +12502,9 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, ArgType::MatchKind ImplicitMatch = ArgType::NoMatch; ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy); + ArgType::MatchKind OrigMatch = Match; + + Match = handleFormatSignedness(Match, S.getDiagnostics(), E->getExprLoc()); if (Match == ArgType::Match) return true; @@ -12512,6 +12528,14 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, ICE->getType() == S.Context.UnsignedIntTy) { // All further checking is done on the subexpression ImplicitMatch = AT.matchesType(S.Context, ExprTy); + if (OrigMatch == ArgType::NoMatchSignedness && + ImplicitMatch != ArgType::NoMatchSignedness) + // If the original match was a signedness match this match on the + // implicit cast type also need to be signedness match otherwise we + // might introduce new unexpected warnings from -Wformat-signedness. + return true; + ImplicitMatch = handleFormatSignedness( + ImplicitMatch, S.getDiagnostics(), E->getExprLoc()); if (ImplicitMatch == ArgType::Match) return true; } @@ -12633,6 +12657,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, case ArgType::Match: case ArgType::MatchPromotion: case ArgType::NoMatchPromotionTypeConfusion: + case ArgType::NoMatchSignedness: llvm_unreachable("expected non-matching"); case ArgType::NoMatchPedantic: Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic; @@ -12668,8 +12693,10 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, CastFix << (S.LangOpts.CPlusPlus ? ">" : ")"); SmallVector Hints; - if (AT.matchesType(S.Context, IntendedTy) != ArgType::Match || - ShouldNotPrintDirectly) + ArgType::MatchKind IntendedMatch = AT.matchesType(S.Context, IntendedTy); + IntendedMatch = handleFormatSignedness(IntendedMatch, S.getDiagnostics(), + E->getExprLoc()); + if ((IntendedMatch != ArgType::Match) || ShouldNotPrintDirectly) Hints.push_back(FixItHint::CreateReplacement(SpecRange, os.str())); if (const CStyleCastExpr *CCast = dyn_cast(E)) { @@ -12738,6 +12765,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS, case ArgType::Match: case ArgType::MatchPromotion: case ArgType::NoMatchPromotionTypeConfusion: + case ArgType::NoMatchSignedness: llvm_unreachable("expected non-matching"); case ArgType::NoMatchPedantic: Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic; @@ -12949,6 +12977,7 @@ bool CheckScanfHandler::HandleScanfSpecifier( analyze_format_string::ArgType::MatchKind Match = AT.matchesType(S.Context, Ex->getType()); + Match = handleFormatSignedness(Match, S.getDiagnostics(), Ex->getExprLoc()); bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic; if (Match == analyze_format_string::ArgType::Match) return true; -- cgit v1.1