Skip to content

Conversation

@vbvictor
Copy link
Contributor

If field width is specified, the sign/space is already accounted for within the field width, so no additional size is needed.

Fixes #143951.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 15, 2025

@llvm/pr-subscribers-clang

Author: Baranov Victor (vbvictor)

Changes

If field width is specified, the sign/space is already accounted for within the field width, so no additional size is needed.

Fixes #143951.


Full diff: https://github.com/llvm/llvm-project/pull/144274.diff

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+4-1)
  • (modified) clang/test/Sema/warn-format-overflow-truncation.c (+10)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 33ee8a53b5f37..63a530f6ed622 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -629,6 +629,9 @@ Improvements to Clang's diagnostics
   #GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490,
   #GH36703, #GH32903, #GH23312, #GH69874.
 
+- Fixed false positives in ``-Wformat-truncation`` and ``-Wformat-overflow``
+  diagnostics when floating-point numbers had both width field and plus or space
+  prefix specified.
 
 Improvements to Clang's time-trace
 ----------------------------------
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 69276ce418fa6..8501ce681e903 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1012,7 +1012,10 @@ class EstimateSizeFormatHandler
       break;
     }
 
-    Size += FS.hasPlusPrefix() || FS.hasSpacePrefix();
+    // If field width is specified, the sign/space is already accounted for
+    // within the field width, so no additional size is needed.
+    if ((FS.hasPlusPrefix() || FS.hasSpacePrefix()) && FieldWidth == 0)
+      Size += 1;
 
     if (FS.hasAlternativeForm()) {
       switch (FS.getConversionSpecifier().getKind()) {
diff --git a/clang/test/Sema/warn-format-overflow-truncation.c b/clang/test/Sema/warn-format-overflow-truncation.c
index c64a1ed8aaa05..5fa770d3d3c51 100644
--- a/clang/test/Sema/warn-format-overflow-truncation.c
+++ b/clang/test/Sema/warn-format-overflow-truncation.c
@@ -43,6 +43,11 @@ void call_snprintf(double d, int n, int *ptr) {
   __builtin_snprintf(node_name, sizeof(node_name), "%pOFn", ptr); // nonkprintf-warning {{'snprintf' will always be truncated; specified size is 6, but format string expands to at least 7}}
   __builtin_snprintf(node_name, sizeof(node_name), "12345%pOFn", ptr); // nonkprintf-warning {{'snprintf' will always be truncated; specified size is 6, but format string expands to at least 12}}
   __builtin_snprintf(node_name, sizeof(node_name), "123456%pOFn", ptr); // nonkprintf-warning {{'snprintf' will always be truncated; specified size is 6, but format string expands to at least 13}}
+  __builtin_snprintf(buf, 6, "%5.1f", 9.f);
+  __builtin_snprintf(buf, 6, "%+5.1f", 9.f);
+  __builtin_snprintf(buf, 6, "% 5.1f", 9.f);
+  __builtin_snprintf(buf, 6, "%+6.1f", 9.f); // kprintf-warning {{'snprintf' will always be truncated; specified size is 6, but format string expands to at least 7}}
+  __builtin_snprintf(buf, 6, "% 6.1f", 9.f); // kprintf-warning {{'snprintf' will always be truncated; specified size is 6, but format string expands to at least 7}}
 }
 
 void call_vsnprintf(void) {
@@ -153,4 +158,9 @@ void call_sprintf(void) {
   sprintf(buf, "%+.3f", 9.f); // kprintf-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
   sprintf(buf, "%.0e", 9.f);
   sprintf(buf, "5%.1e", 9.f); // kprintf-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 8}}
+  sprintf(buf, "%5.1f", 9.f);
+  sprintf(buf, "%+5.1f", 9.f);
+  sprintf(buf, "% 5.1f", 9.f);
+  sprintf(buf, "%+6.1f", 9.f); // kprintf-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "% 6.1f", 9.f); // kprintf-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
 }

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM aside from a nit with the release note. Thank you for the fix!

@vbvictor vbvictor force-pushed the incorrect-snprintf-warning branch from 4fea62a to dc4b105 Compare June 29, 2025 19:43
@vbvictor vbvictor merged commit 96b9b2e into llvm:main Jun 30, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect warning message

4 participants