[clang] 72f6abb - [clang][Sema] Fix format size estimator's handling of %o, %x, %X with alternative form
Takuya Shimizu via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 11 20:23:00 PDT 2023
Author: Takuya Shimizu
Date: 2023-09-12T12:22:26+09:00
New Revision: 72f6abb9bca68bf1398b321a73ebe3158bca67e5
URL: https://github.com/llvm/llvm-project/commit/72f6abb9bca68bf1398b321a73ebe3158bca67e5
DIFF: https://github.com/llvm/llvm-project/commit/72f6abb9bca68bf1398b321a73ebe3158bca67e5.diff
LOG: [clang][Sema] Fix format size estimator's handling of %o, %x, %X with alternative form
The wrong handling of %x specifier with alternative form causes a false positive in linux kernel (https://github.com/ClangBuiltLinux/linux/issues/1923#issuecomment-1696075886)
The kernel code: https://github.com/torvalds/linux/blob/651a00bc56403161351090a9d7ddbd7095975324/drivers/media/pci/cx18/cx18-mailbox.c#L99
This patch fixes this handling, and also adds some standard wordings as comments to clarify the reason.
Reviewed By: nickdesaulniers
Differential Revision: https://reviews.llvm.org/D159138
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/warn-fortify-source.c
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fc0972c20476d09..c7b4432380120cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -164,7 +164,7 @@ Improvements to Clang's diagnostics
result in string truncation.
(`#64871: <https://github.com/llvm/llvm-project/issues/64871>`_).
Also clang no longer emits false positive warnings about the output length of
- ``%g`` format specifier.
+ ``%g`` format specifier and about ``%o, %x, %X`` with ``#`` flag.
- Clang now emits ``-Wcast-qual`` for functional-style cast expressions.
Bug Fixes in This Version
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3b4ac613da76aa8..fad70223362eddd 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -890,6 +890,8 @@ class EstimateSizeFormatHandler
// %g style conversion switches between %f or %e style dynamically.
// %g removes trailing zeros, and does not print decimal point if there are
// no digits that follow it. Thus %g can print a single digit.
+ // FIXME: If it is alternative form:
+ // For g and G conversions, trailing zeros are not removed from the result.
case analyze_format_string::ConversionSpecifier::gArg:
case analyze_format_string::ConversionSpecifier::GArg:
Size += 1;
@@ -947,18 +949,26 @@ class EstimateSizeFormatHandler
if (FS.hasAlternativeForm()) {
switch (FS.getConversionSpecifier().getKind()) {
- default:
- break;
- // Force a leading '0'.
+ // For o conversion, it increases the precision, if and only if necessary,
+ // to force the first digit of the result to be a zero
+ // (if the value and precision are both 0, a single 0 is printed)
case analyze_format_string::ConversionSpecifier::oArg:
- Size += 1;
- break;
- // Force a leading '0x'.
+ // For b conversion, a nonzero result has 0b prefixed to it.
+ case analyze_format_string::ConversionSpecifier::bArg:
+ // For x (or X) conversion, a nonzero result has 0x (or 0X) prefixed to
+ // it.
case analyze_format_string::ConversionSpecifier::xArg:
case analyze_format_string::ConversionSpecifier::XArg:
- Size += 2;
+ // Note: even when the prefix is added, if
+ // (prefix_width <= FieldWidth - formatted_length) holds,
+ // the prefix does not increase the format
+ // size. e.g.(("%#3x", 0xf) is "0xf")
+
+ // If the result is zero, o, b, x, X adds nothing.
break;
- // Force a period '.' before decimal, even if precision is 0.
+ // For a, A, e, E, f, F, g, and G conversions,
+ // the result of converting a floating-point number always contains a
+ // decimal-point
case analyze_format_string::ConversionSpecifier::aArg:
case analyze_format_string::ConversionSpecifier::AArg:
case analyze_format_string::ConversionSpecifier::eArg:
@@ -969,6 +979,9 @@ class EstimateSizeFormatHandler
case analyze_format_string::ConversionSpecifier::GArg:
Size += (Precision ? 0 : 1);
break;
+ // For other conversions, the behavior is undefined.
+ default:
+ break;
}
}
assert(SpecifierLen <= Size && "no underflow");
diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c
index 5ed9782b26fb788..de6171af8c14524 100644
--- a/clang/test/Sema/warn-fortify-source.c
+++ b/clang/test/Sema/warn-fortify-source.c
@@ -85,7 +85,7 @@ void call_memset(void) {
__builtin_memset(buf, 0xff, 11); // expected-warning {{'memset' will always overflow; destination buffer has size 10, but size argument is 11}}
}
-void call_snprintf(double d) {
+void call_snprintf(double d, int n) {
char buf[10];
__builtin_snprintf(buf, 10, "merp");
__builtin_snprintf(buf, 11, "merp"); // expected-warning {{'snprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
@@ -96,6 +96,13 @@ void call_snprintf(double d) {
__builtin_snprintf(buf, 1, "%.1000g", d); // expected-warning {{'snprintf' will always be truncated; specified size is 1, but format string expands to at least 2}}
__builtin_snprintf(buf, 5, "%.1000g", d);
__builtin_snprintf(buf, 5, "%.1000G", d);
+ __builtin_snprintf(buf, 10, " %#08x", n);
+ __builtin_snprintf(buf, 2, "%#x", n);
+ __builtin_snprintf(buf, 2, "%#X", n);
+ __builtin_snprintf(buf, 2, "%#o", n);
+ __builtin_snprintf(buf, 1, "%#x", n); // expected-warning {{'snprintf' will always be truncated; specified size is 1, but format string expands to at least 2}}
+ __builtin_snprintf(buf, 1, "%#X", n); // expected-warning {{'snprintf' will always be truncated; specified size is 1, but format string expands to at least 2}}
+ __builtin_snprintf(buf, 1, "%#o", n); // expected-warning {{'snprintf' will always be truncated; specified size is 1, but format string expands to at least 2}}
}
void call_vsnprintf(void) {
@@ -110,6 +117,13 @@ void call_vsnprintf(void) {
__builtin_vsnprintf(buf, 1, "%.1000g", list); // expected-warning {{'vsnprintf' will always be truncated; specified size is 1, but format string expands to at least 2}}
__builtin_vsnprintf(buf, 5, "%.1000g", list);
__builtin_vsnprintf(buf, 5, "%.1000G", list);
+ __builtin_vsnprintf(buf, 10, " %#08x", list);
+ __builtin_vsnprintf(buf, 2, "%#x", list);
+ __builtin_vsnprintf(buf, 2, "%#X", list);
+ __builtin_vsnprintf(buf, 2, "%#o", list);
+ __builtin_vsnprintf(buf, 1, "%#x", list); // expected-warning {{'vsnprintf' will always be truncated; specified size is 1, but format string expands to at least 2}}
+ __builtin_vsnprintf(buf, 1, "%#X", list); // expected-warning {{'vsnprintf' will always be truncated; specified size is 1, but format string expands to at least 2}}
+ __builtin_vsnprintf(buf, 1, "%#o", list); // expected-warning {{'vsnprintf' will always be truncated; specified size is 1, but format string expands to at least 2}}
}
void call_sprintf_chk(char *buf) {
@@ -147,7 +161,7 @@ void call_sprintf_chk(char *buf) {
__builtin___sprintf_chk(buf, 1, 2, "%%");
__builtin___sprintf_chk(buf, 1, 1, "%%"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
__builtin___sprintf_chk(buf, 1, 4, "%#x", 9);
- __builtin___sprintf_chk(buf, 1, 3, "%#x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands to at least 4}}
+ __builtin___sprintf_chk(buf, 1, 3, "%#x", 9);
__builtin___sprintf_chk(buf, 1, 4, "%p", (void *)9);
__builtin___sprintf_chk(buf, 1, 3, "%p", (void *)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands to at least 4}}
__builtin___sprintf_chk(buf, 1, 3, "%+d", 9);
@@ -184,7 +198,7 @@ void call_sprintf(void) {
sprintf(buf, "1234%lld", 9ll);
sprintf(buf, "12345%lld", 9ll); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
sprintf(buf, "12%#x", 9);
- sprintf(buf, "123%#x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+ sprintf(buf, "123%#x", 9);
sprintf(buf, "12%p", (void *)9);
sprintf(buf, "123%p", (void *)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
sprintf(buf, "123%+d", 9);
More information about the cfe-commits
mailing list