[clang] [clang][Sema] Stop format size estimator upon %p to adapt to linux kernel's extension (PR #65969)

[via cfe-commits]

llvmbot wrote:

@llvm/pr-subscribers-clang

<details>
<summary>Changes</summary>

GCC stops counting format string's size when it sees %p format in order to avoid `Wformat-truncation` false positive against Linux kernel's format extension (%pOF, for example).
This change makes clang's behavior align with GCC's.
As requested in https://github.com/llvm/llvm-project/issues/64871

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

4 Files Affected:

- (modified) clang/include/clang/AST/FormatString.h (+6) 
- (modified) clang/lib/AST/PrintfFormatString.cpp (+7-1) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+10) 
- (modified) clang/test/Sema/warn-fortify-source.c (+13-3) 


<pre>
diff --git a/clang/include/clang/AST/FormatString.h b/clang/include/clang/AST/FormatString.h
index 5c4ad9baaef608c..97ce3cfe0b25c68 100644
--- a/clang/include/clang/AST/FormatString.h
+++ b/clang/include/clang/AST/FormatString.h
@@ -742,6 +742,12 @@ class FormatStringHandler {
     return true;
   }
 
+  virtual bool
+  ShouldStopOnFormatSpecifier(const analyze_printf::PrintfSpecifier &FS,
+                              unsigned RemainLen) {
+    return false;
+  }
+
   /// Handle mask types whose sizes are not between one and eight bytes.
   virtual void handleInvalidMaskType(StringRef MaskType) {}
 
diff --git a/clang/lib/AST/PrintfFormatString.cpp b/clang/lib/AST/PrintfFormatString.cpp
index f0b9d0ecaf23461..750f584d8d37a76 100644
--- a/clang/lib/AST/PrintfFormatString.cpp
+++ b/clang/lib/AST/PrintfFormatString.cpp
@@ -429,7 +429,13 @@ bool clang::analyze_format_string::ParsePrintfString(FormatStringHandler &H,
     // we can recover from?
     if (!FSR.hasValue())
       continue;
-    // We have a format specifier.  Pass it to the callback.
+    // We have a format specifier.
+    // In case the handler needs to abort parsing upon specific specifiers
+    if (H.ShouldStopOnFormatSpecifier(
+            FSR.getValue(), static_cast<unsigned>(E - FSR.getStart()))) {
+      return false;
+    }
+    // Pass the format specifier to the callback.
     if (!H.HandlePrintfSpecifier(FSR.getValue(), FSR.getStart(),
                                  I - FSR.getStart(), Target))
       return true;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 3b4ac613da76aa8..1a6ff6cfa19b661 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -860,6 +860,16 @@ class EstimateSizeFormatHandler
       : Size(std::min(Format.find(0), Format.size()) +
              1 /* null byte always written by sprintf */) {}
 
+  bool ShouldStopOnFormatSpecifier(const analyze_printf::PrintfSpecifier &FS,
+                                   unsigned RemainLen) override {
+    if (FS.getConversionSpecifier().getKind() ==
+        analyze_printf::PrintfConversionSpecifier::pArg) {
+      assert(Size >= RemainLen && "no underflow");
+      Size -= RemainLen;
+      return true;
+    }
+    return false;
+  }
   bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
                              const char *, unsigned SpecifierLen,
                              const TargetInfo &) override {
diff --git a/clang/test/Sema/warn-fortify-source.c b/clang/test/Sema/warn-fortify-source.c
index 5ed9782b26fb788..fba6c718be4a33a 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 *ptr) {
   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,11 @@ 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);
+  char node_name[6];
+  __builtin_snprintf(node_name, sizeof(node_name), "%pOFn", ptr);
+  __builtin_snprintf(node_name, sizeof(node_name), "12345%pOFn", ptr);
+  __builtin_snprintf(node_name, sizeof(node_name), "123456%pOFn", ptr); // expected-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), "1234567%pOFn", ptr); // expected-warning {{'snprintf' will always be truncated; specified size is 6, but format string expands to at least 8}}
 }
 
 void call_vsnprintf(void) {
@@ -110,6 +115,11 @@ 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);
+  char node_name[6];
+  __builtin_snprintf(node_name, sizeof(node_name), "%pOFn", list);
+  __builtin_snprintf(node_name, sizeof(node_name), "12345%pOFn", list);
+  __builtin_snprintf(node_name, sizeof(node_name), "123456%pOFn", list); // expected-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), "1234567%pOFn", list); // expected-warning {{'snprintf' will always be truncated; specified size is 6, but format string expands to at least 8}}
 }
 
 void call_sprintf_chk(char *buf) {
@@ -149,7 +159,7 @@ void call_sprintf_chk(char *buf) {
   __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, 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, "%p", (void *)9);
   __builtin___sprintf_chk(buf, 1, 3, "%+d", 9);
   __builtin___sprintf_chk(buf, 1, 2, "%+d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 3}}
   __builtin___sprintf_chk(buf, 1, 3, "% i", 9);
@@ -186,7 +196,7 @@ void call_sprintf(void) {
   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, "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%p", (void *)9);
   sprintf(buf, "123%+d", 9);
   sprintf(buf, "1234%+d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
   sprintf(buf, "123% i", 9);
</pre>

</details>

https://github.com/llvm/llvm-project/pull/65969