Skip to content

Commit 56c3b8e

Browse files
authored
[clang][Sema] Make format size estimator aware of %p's existence in format string (#65969)
This change introduces `-Wformat-overflow` and `-Wformat-truncation` warning flags that were formerly diagnosed from `-Wfortify-source` warning group. This also introduces `-Wformat-overflow-non-kprintf` and `-Wformat-truncation-non-kprintf`, both of which will be used when the format string contains `%p` format string. The rationale for this is that Linux kernel has its own extension for `%p` format specifier, and we need some way to suppress false positives in kernel codebase. The approach of this patch aims NOT to affect non-kernel codebases. Note that GCC stops format size estimation upon `%p` format specifier. As requested in #64871
1 parent ff7e854 commit 56c3b8e

File tree

7 files changed

+225
-24
lines changed

7 files changed

+225
-24
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,19 @@ Improvements to Clang's diagnostics
185185
- Clang constexpr evaluator now diagnoses compound assignment operators against
186186
uninitialized variables as a read of uninitialized object.
187187
(`#51536 <https://github.com/llvm/llvm-project/issues/51536>`_)
188-
- Clang's ``-Wfortify-source`` now diagnoses ``snprintf`` call that is known to
188+
- Clang's ``-Wformat-truncation`` now diagnoses ``snprintf`` call that is known to
189189
result in string truncation.
190190
(`#64871: <https://github.com/llvm/llvm-project/issues/64871>`_).
191+
Existing warnings that similarly warn about the overflow in ``sprintf``
192+
now falls under its own warning group ```-Wformat-overflow`` so that it can
193+
be disabled separately from ``Wfortify-source``.
194+
These two new warning groups have subgroups ``-Wformat-truncation-non-kprintf``
195+
and ``-Wformat-overflow-non-kprintf``, respectively. These subgroups are used when
196+
the format string contains ``%p`` format specifier.
197+
Because Linux kernel's codebase has format extensions for ``%p``, kernel developers
198+
are encouraged to disable these two subgroups by setting ``-Wno-format-truncation-non-kprintf``
199+
and ``-Wno-format-overflow-non-kprintf`` in order to avoid false positives on
200+
the kernel codebase.
191201
Also clang no longer emits false positive warnings about the output length of
192202
``%g`` format specifier and about ``%o, %x, %X`` with ``#`` flag.
193203
- Clang now emits ``-Wcast-qual`` for functional-style cast expressions.

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -961,10 +961,16 @@ def FormatNonStandard : DiagGroup<"format-non-iso">;
961961
def FormatY2K : DiagGroup<"format-y2k">;
962962
def FormatPedantic : DiagGroup<"format-pedantic">;
963963
def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
964+
965+
def FormatOverflowNonKprintf: DiagGroup<"format-overflow-non-kprintf">;
966+
def FormatOverflow: DiagGroup<"format-overflow", [FormatOverflowNonKprintf]>;
967+
def FormatTruncationNonKprintf: DiagGroup<"format-truncation-non-kprintf">;
968+
def FormatTruncation: DiagGroup<"format-truncation", [FormatTruncationNonKprintf]>;
969+
964970
def Format : DiagGroup<"format",
965971
[FormatExtraArgs, FormatZeroLength, NonNull,
966972
FormatSecurity, FormatY2K, FormatInvalidSpecifier,
967-
FormatInsufficientArgs]>,
973+
FormatInsufficientArgs, FormatOverflow, FormatTruncation]>,
968974
DiagCategory<"Format String Issue">;
969975
def FormatNonLiteral : DiagGroup<"format-nonliteral">;
970976
def Format2 : DiagGroup<"format=2",
@@ -1400,7 +1406,7 @@ def CrossTU : DiagGroup<"ctu">;
14001406

14011407
def CTADMaybeUnsupported : DiagGroup<"ctad-maybe-unsupported">;
14021408

1403-
def FortifySource : DiagGroup<"fortify-source">;
1409+
def FortifySource : DiagGroup<"fortify-source", [FormatOverflow, FormatTruncation]>;
14041410

14051411
def MaxTokens : DiagGroup<"max-tokens"> {
14061412
code Documentation = [{

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -854,15 +854,29 @@ def warn_fortify_strlen_overflow: Warning<
854854
" but the source string has length %2 (including NUL byte)">,
855855
InGroup<FortifySource>;
856856

857-
def warn_fortify_source_format_overflow : Warning<
857+
def subst_format_overflow : TextSubstitution<
858858
"'%0' will always overflow; destination buffer has size %1,"
859-
" but format string expands to at least %2">,
860-
InGroup<FortifySource>;
859+
" but format string expands to at least %2">;
860+
861+
def warn_format_overflow : Warning<
862+
"%sub{subst_format_overflow}0,1,2">,
863+
InGroup<FormatOverflow>;
864+
865+
def warn_format_overflow_non_kprintf : Warning<
866+
"%sub{subst_format_overflow}0,1,2">,
867+
InGroup<FormatOverflowNonKprintf>;
861868

862-
def warn_fortify_source_format_truncation: Warning<
869+
def subst_format_truncation: TextSubstitution<
863870
"'%0' will always be truncated; specified size is %1,"
864-
" but format string expands to at least %2">,
865-
InGroup<FortifySource>;
871+
" but format string expands to at least %2">;
872+
873+
def warn_format_truncation: Warning<
874+
"%sub{subst_format_truncation}0,1,2">,
875+
InGroup<FormatTruncation>;
876+
877+
def warn_format_truncation_non_kprintf: Warning<
878+
"%sub{subst_format_truncation}0,1,2">,
879+
InGroup<FormatTruncationNonKprintf>;
866880

867881
def warn_fortify_scanf_overflow : Warning<
868882
"'%0' may overflow; destination buffer in argument %1 has size "

clang/lib/Sema/SemaChecking.cpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,9 @@ class ScanfDiagnosticFormatHandler
854854
class EstimateSizeFormatHandler
855855
: public analyze_format_string::FormatStringHandler {
856856
size_t Size;
857+
/// Whether the format string contains Linux kernel's format specifier
858+
/// extension.
859+
bool IsKernelCompatible = true;
857860

858861
public:
859862
EstimateSizeFormatHandler(StringRef Format)
@@ -933,6 +936,10 @@ class EstimateSizeFormatHandler
933936

934937
// Just a pointer in the form '0xddd'.
935938
case analyze_format_string::ConversionSpecifier::pArg:
939+
// Linux kernel has its own extesion for `%p` specifier.
940+
// Kernel Document:
941+
// https://docs.kernel.org/core-api/printk-formats.html#pointer-types
942+
IsKernelCompatible = false;
936943
Size += std::max(FieldWidth, 2 /* leading 0x */ + Precision);
937944
break;
938945

@@ -990,6 +997,7 @@ class EstimateSizeFormatHandler
990997
}
991998

992999
size_t getSizeLowerBound() const { return Size; }
1000+
bool isKernelCompatible() const { return IsKernelCompatible; }
9931001

9941002
private:
9951003
static size_t computeFieldWidth(const analyze_printf::PrintfSpecifier &FS) {
@@ -1259,7 +1267,9 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
12591267
if (!analyze_format_string::ParsePrintfString(
12601268
H, FormatBytes, FormatBytes + StrLen, getLangOpts(),
12611269
Context.getTargetInfo(), false)) {
1262-
DiagID = diag::warn_fortify_source_format_overflow;
1270+
DiagID = H.isKernelCompatible()
1271+
? diag::warn_format_overflow
1272+
: diag::warn_format_overflow_non_kprintf;
12631273
SourceSize = llvm::APSInt::getUnsigned(H.getSizeLowerBound())
12641274
.extOrTrunc(SizeTypeWidth);
12651275
if (BuiltinID == Builtin::BI__builtin___sprintf_chk) {
@@ -1350,10 +1360,17 @@ void Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
13501360
llvm::APSInt::getUnsigned(H.getSizeLowerBound())
13511361
.extOrTrunc(SizeTypeWidth);
13521362
if (FormatSize > *SourceSize && *SourceSize != 0) {
1353-
DiagID = diag::warn_fortify_source_format_truncation;
1354-
DestinationSize = SourceSize;
1355-
SourceSize = FormatSize;
1356-
break;
1363+
unsigned TruncationDiagID =
1364+
H.isKernelCompatible() ? diag::warn_format_truncation
1365+
: diag::warn_format_truncation_non_kprintf;
1366+
SmallString<16> SpecifiedSizeStr;
1367+
SmallString<16> FormatSizeStr;
1368+
SourceSize->toString(SpecifiedSizeStr, /*Radix=*/10);
1369+
FormatSize.toString(FormatSizeStr, /*Radix=*/10);
1370+
DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall,
1371+
PDiag(TruncationDiagID)
1372+
<< GetFunctionName() << SpecifiedSizeStr
1373+
<< FormatSizeStr);
13571374
}
13581375
}
13591376
}

clang/test/Misc/warning-wall.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ CHECK-NEXT: -Wformat-security
1919
CHECK-NEXT: -Wformat-y2k
2020
CHECK-NEXT: -Wformat-invalid-specifier
2121
CHECK-NEXT: -Wformat-insufficient-args
22+
CHECK-NEXT: -Wformat-overflow
23+
CHECK-NEXT: -Wformat-overflow-non-kprintf
24+
CHECK-NEXT: -Wformat-truncation
25+
CHECK-NEXT: -Wformat-truncation-non-kprintf
2226
CHECK-NEXT: -Wfor-loop-analysis
2327
CHECK-NEXT: -Wframe-address
2428
CHECK-NEXT: -Wimplicit

0 commit comments

Comments
 (0)