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

2023-09-24 Thread Takuya Shimizu via cfe-commits

hazohelet wrote:

> ping @hazohelet . This should be good to land and should help clear some red 
> in our CI for Linux kernel builds.

Thanks for the ping. I was away for a while. I'm merging this after updating 
the PR descriptions.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-22 Thread Nick Desaulniers via cfe-commits

nickdesaulniers wrote:

ping @hazohelet . This should be good to land and should help clear some red in 
our CI for Linux kernel builds.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-20 Thread Richard Smith via cfe-commits

https://github.com/zygoloid approved this pull request.

Thank you!

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-20 Thread Richard Smith via cfe-commits

https://github.com/zygoloid resolved 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-19 Thread Nick Desaulniers via cfe-commits

https://github.com/nickdesaulniers approved this pull request.

Thanks for the fix!

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-18 Thread Takuya Shimizu via cfe-commits

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

>From 5ee1a4f83c69b5e2910ea883dca7f0fa2c1a4bd3 Mon Sep 17 00:00:00 2001
From: Takuya Shimizu 
Date: Wed, 13 Sep 2023 17:43:11 +0900
Subject: [PATCH 1/4] [clang][Diagnostics] Separate Wformat-overflow and
 Wformat-truncation from Wfortify-source

Newly introduces `Wformat-overflow` and `Wformat-truncation` and imports the 
existing warning from `Wfortify-source` that correspond to GCC's counterpart so 
that they can be disabled separately from `Wfortify-source`.
---
 clang/docs/ReleaseNotes.rst   |   5 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   4 +-
 clang/lib/Sema/SemaChecking.cpp   |  12 +-
 .../Sema/warn-format-overflow-truncation.c| 153 ++
 clang/test/Sema/warn-fortify-source.c |  14 +-
 5 files changed, 171 insertions(+), 17 deletions(-)
 create mode 100644 clang/test/Sema/warn-format-overflow-truncation.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3cdad2f7b9f0e5a..628253c730cd1e0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -160,9 +160,12 @@ Improvements to Clang's diagnostics
 - Clang constexpr evaluator now diagnoses compound assignment operators against
   uninitialized variables as a read of uninitialized object.
   (`#51536 `_)
-- Clang's ``-Wfortify-source`` now diagnoses ``snprintf`` call that is known to
+- Clang's ``-Wformat-truncation`` now diagnoses ``snprintf`` call that is 
known to
   result in string truncation.
   (`#64871: `_).
+  Existing warnings that similarly warn about the overflow in ``sprintf``
+  now falls under its own warning group ```-Wformat-overflow`` so that it can
+  be disabled separately from ``Wfortify-source``.
   Also clang no longer emits false positive warnings about the output length of
   ``%g`` format specifier and about ``%o, %x, %X`` with ``#`` flag.
 - Clang now emits ``-Wcast-qual`` for functional-style cast expressions.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0ac4df8edb242f6..7f708a5267e0052 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -857,12 +857,12 @@ def warn_fortify_strlen_overflow: Warning<
 def warn_fortify_source_format_overflow : Warning<
   "'%0' will always overflow; destination buffer has size %1,"
   " but format string expands to at least %2">,
-  InGroup;
+  InGroup>;
 
 def warn_fortify_source_format_truncation: Warning<
   "'%0' will always be truncated; specified size is %1,"
   " but format string expands to at least %2">,
-  InGroup;
+  InGroup>;
 
 def warn_fortify_scanf_overflow : Warning<
   "'%0' may overflow; destination buffer in argument %1 has size "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index fad70223362eddd..8260e72bd99e839 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1350,10 +1350,14 @@ void 
Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
 llvm::APSInt::getUnsigned(H.getSizeLowerBound())
 .extOrTrunc(SizeTypeWidth);
 if (FormatSize > *SourceSize && *SourceSize != 0) {
-  DiagID = diag::warn_fortify_source_format_truncation;
-  DestinationSize = SourceSize;
-  SourceSize = FormatSize;
-  break;
+  SmallString<16> SpecifiedSizeStr;
+  SmallString<16> FormatSizeStr;
+  SourceSize->toString(SpecifiedSizeStr, /*Radix=*/10);
+  FormatSize.toString(FormatSizeStr, /*Radix=*/10);
+  DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall,
+  
PDiag(diag::warn_fortify_source_format_truncation)
+  << GetFunctionName() << SpecifiedSizeStr
+  << FormatSizeStr);
 }
   }
 }
diff --git a/clang/test/Sema/warn-format-overflow-truncation.c 
b/clang/test/Sema/warn-format-overflow-truncation.c
new file mode 100644
index 000..dada1f9355afe07
--- /dev/null
+++ b/clang/test/Sema/warn-format-overflow-truncation.c
@@ -0,0 +1,153 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify 
-DUSE_BUILTINS
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -Wno-format-truncation 
-Wno-format-overflow %s -verify=off
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 
-Wno-format-truncation -Wno-format-overflow %s -verify=off
+
+typedef unsigned long size_t;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern int 

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

2023-09-14 Thread Takuya Shimizu via cfe-commits

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

>From 5ee1a4f83c69b5e2910ea883dca7f0fa2c1a4bd3 Mon Sep 17 00:00:00 2001
From: Takuya Shimizu 
Date: Wed, 13 Sep 2023 17:43:11 +0900
Subject: [PATCH 1/3] [clang][Diagnostics] Separate Wformat-overflow and
 Wformat-truncation from Wfortify-source

Newly introduces `Wformat-overflow` and `Wformat-truncation` and imports the 
existing warning from `Wfortify-source` that correspond to GCC's counterpart so 
that they can be disabled separately from `Wfortify-source`.
---
 clang/docs/ReleaseNotes.rst   |   5 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   4 +-
 clang/lib/Sema/SemaChecking.cpp   |  12 +-
 .../Sema/warn-format-overflow-truncation.c| 153 ++
 clang/test/Sema/warn-fortify-source.c |  14 +-
 5 files changed, 171 insertions(+), 17 deletions(-)
 create mode 100644 clang/test/Sema/warn-format-overflow-truncation.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3cdad2f7b9f0e5a..628253c730cd1e0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -160,9 +160,12 @@ Improvements to Clang's diagnostics
 - Clang constexpr evaluator now diagnoses compound assignment operators against
   uninitialized variables as a read of uninitialized object.
   (`#51536 `_)
-- Clang's ``-Wfortify-source`` now diagnoses ``snprintf`` call that is known to
+- Clang's ``-Wformat-truncation`` now diagnoses ``snprintf`` call that is 
known to
   result in string truncation.
   (`#64871: `_).
+  Existing warnings that similarly warn about the overflow in ``sprintf``
+  now falls under its own warning group ```-Wformat-overflow`` so that it can
+  be disabled separately from ``Wfortify-source``.
   Also clang no longer emits false positive warnings about the output length of
   ``%g`` format specifier and about ``%o, %x, %X`` with ``#`` flag.
 - Clang now emits ``-Wcast-qual`` for functional-style cast expressions.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0ac4df8edb242f6..7f708a5267e0052 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -857,12 +857,12 @@ def warn_fortify_strlen_overflow: Warning<
 def warn_fortify_source_format_overflow : Warning<
   "'%0' will always overflow; destination buffer has size %1,"
   " but format string expands to at least %2">,
-  InGroup;
+  InGroup>;
 
 def warn_fortify_source_format_truncation: Warning<
   "'%0' will always be truncated; specified size is %1,"
   " but format string expands to at least %2">,
-  InGroup;
+  InGroup>;
 
 def warn_fortify_scanf_overflow : Warning<
   "'%0' may overflow; destination buffer in argument %1 has size "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index fad70223362eddd..8260e72bd99e839 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1350,10 +1350,14 @@ void 
Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
 llvm::APSInt::getUnsigned(H.getSizeLowerBound())
 .extOrTrunc(SizeTypeWidth);
 if (FormatSize > *SourceSize && *SourceSize != 0) {
-  DiagID = diag::warn_fortify_source_format_truncation;
-  DestinationSize = SourceSize;
-  SourceSize = FormatSize;
-  break;
+  SmallString<16> SpecifiedSizeStr;
+  SmallString<16> FormatSizeStr;
+  SourceSize->toString(SpecifiedSizeStr, /*Radix=*/10);
+  FormatSize.toString(FormatSizeStr, /*Radix=*/10);
+  DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall,
+  
PDiag(diag::warn_fortify_source_format_truncation)
+  << GetFunctionName() << SpecifiedSizeStr
+  << FormatSizeStr);
 }
   }
 }
diff --git a/clang/test/Sema/warn-format-overflow-truncation.c 
b/clang/test/Sema/warn-format-overflow-truncation.c
new file mode 100644
index 000..dada1f9355afe07
--- /dev/null
+++ b/clang/test/Sema/warn-format-overflow-truncation.c
@@ -0,0 +1,153 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify 
-DUSE_BUILTINS
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -Wno-format-truncation 
-Wno-format-overflow %s -verify=off
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 
-Wno-format-truncation -Wno-format-overflow %s -verify=off
+
+typedef unsigned long size_t;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern int 

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

2023-09-14 Thread Takuya Shimizu via cfe-commits


@@ -1350,10 +1360,17 @@ void 
Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
 llvm::APSInt::getUnsigned(H.getSizeLowerBound())
 .extOrTrunc(SizeTypeWidth);
 if (FormatSize > *SourceSize && *SourceSize != 0) {
-  DiagID = diag::warn_fortify_source_format_truncation;
-  DestinationSize = SourceSize;
-  SourceSize = FormatSize;
-  break;

hazohelet wrote:

This is intentional.
When the truncation warning was grouped under `Wfortify-source`, I thought it 
made sense to override the warning because there was no way to disable only one 
of `warn_fortify_source_size_mismatch` and the truncation warning.
Now that `Wformat-truncation` is made, I no longer think this warning should 
hide the `Wfortify-source` warning.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-14 Thread Takuya Shimizu via cfe-commits


@@ -961,10 +961,16 @@ def FormatNonStandard : DiagGroup<"format-non-iso">;
 def FormatY2K : DiagGroup<"format-y2k">;
 def FormatPedantic : DiagGroup<"format-pedantic">;
 def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
+
+def FormatOverflowNonKprintf: DiagGroup<"format-overflow-non-kprintf">;
+def FormatOverflow: DiagGroup<"format-overflow", [FormatOverflowNonKprintf]>;
+def FormatTruncationNonKprintf: DiagGroup<"format-truncation-non-kprintf">;
+def FormatTruncation: DiagGroup<"format-truncation", 
[FormatTruncationNonKprintf]>;

hazohelet wrote:

Thanks. It makes sense to add this to `FortifySource` for backward 
compatibility.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Nick Desaulniers via cfe-commits

nickdesaulniers wrote:

@nathanchance and I have done a bunch of testing of this revision (see comments 
https://github.com/ClangBuiltLinux/linux/issues/1923#issuecomment-1718144462 
and down); this looks like something that will work nicely for the Linux 
kernel's use case! Thanks for all of the work here @hazohelet !

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Richard Smith via cfe-commits

https://github.com/zygoloid dismissed 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Richard Smith via cfe-commits

https://github.com/zygoloid review_request_removed 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Richard Smith via cfe-commits

https://github.com/zygoloid review_requested 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Richard Smith via cfe-commits

https://github.com/zygoloid commented:

I'm happy with the overall approach here; please don't block on further review 
from me.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Richard Smith via cfe-commits


@@ -1350,10 +1360,17 @@ void 
Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
 llvm::APSInt::getUnsigned(H.getSizeLowerBound())
 .extOrTrunc(SizeTypeWidth);
 if (FormatSize > *SourceSize && *SourceSize != 0) {
-  DiagID = diag::warn_fortify_source_format_truncation;
-  DestinationSize = SourceSize;
-  SourceSize = FormatSize;
-  break;

zygoloid wrote:

This diagnostic used to override the production of the 
`warn_fortify_source_size_mismatch` warning, and now is produced in addition to 
it. Is that intentional?

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Richard Smith via cfe-commits


@@ -961,10 +961,16 @@ def FormatNonStandard : DiagGroup<"format-non-iso">;
 def FormatY2K : DiagGroup<"format-y2k">;
 def FormatPedantic : DiagGroup<"format-pedantic">;
 def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
+
+def FormatOverflowNonKprintf: DiagGroup<"format-overflow-non-kprintf">;
+def FormatOverflow: DiagGroup<"format-overflow", [FormatOverflowNonKprintf]>;
+def FormatTruncationNonKprintf: DiagGroup<"format-truncation-non-kprintf">;
+def FormatTruncation: DiagGroup<"format-truncation", 
[FormatTruncationNonKprintf]>;

zygoloid wrote:

Should these be added to the `FortifySource` group in addition to adding them 
to `Format`?

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Richard Smith via cfe-commits

https://github.com/zygoloid edited 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Richard Smith via cfe-commits

https://github.com/zygoloid resolved 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet resolved 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet resolved 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet resolved 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet resolved 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-13 Thread Takuya Shimizu via cfe-commits

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

>From 5ee1a4f83c69b5e2910ea883dca7f0fa2c1a4bd3 Mon Sep 17 00:00:00 2001
From: Takuya Shimizu 
Date: Wed, 13 Sep 2023 17:43:11 +0900
Subject: [PATCH 1/2] [clang][Diagnostics] Separate Wformat-overflow and
 Wformat-truncation from Wfortify-source

Newly introduces `Wformat-overflow` and `Wformat-truncation` and imports the 
existing warning from `Wfortify-source` that correspond to GCC's counterpart so 
that they can be disabled separately from `Wfortify-source`.
---
 clang/docs/ReleaseNotes.rst   |   5 +-
 .../clang/Basic/DiagnosticSemaKinds.td|   4 +-
 clang/lib/Sema/SemaChecking.cpp   |  12 +-
 .../Sema/warn-format-overflow-truncation.c| 153 ++
 clang/test/Sema/warn-fortify-source.c |  14 +-
 5 files changed, 171 insertions(+), 17 deletions(-)
 create mode 100644 clang/test/Sema/warn-format-overflow-truncation.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3cdad2f7b9f0e5a..628253c730cd1e0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -160,9 +160,12 @@ Improvements to Clang's diagnostics
 - Clang constexpr evaluator now diagnoses compound assignment operators against
   uninitialized variables as a read of uninitialized object.
   (`#51536 `_)
-- Clang's ``-Wfortify-source`` now diagnoses ``snprintf`` call that is known to
+- Clang's ``-Wformat-truncation`` now diagnoses ``snprintf`` call that is 
known to
   result in string truncation.
   (`#64871: `_).
+  Existing warnings that similarly warn about the overflow in ``sprintf``
+  now falls under its own warning group ```-Wformat-overflow`` so that it can
+  be disabled separately from ``Wfortify-source``.
   Also clang no longer emits false positive warnings about the output length of
   ``%g`` format specifier and about ``%o, %x, %X`` with ``#`` flag.
 - Clang now emits ``-Wcast-qual`` for functional-style cast expressions.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0ac4df8edb242f6..7f708a5267e0052 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -857,12 +857,12 @@ def warn_fortify_strlen_overflow: Warning<
 def warn_fortify_source_format_overflow : Warning<
   "'%0' will always overflow; destination buffer has size %1,"
   " but format string expands to at least %2">,
-  InGroup;
+  InGroup>;
 
 def warn_fortify_source_format_truncation: Warning<
   "'%0' will always be truncated; specified size is %1,"
   " but format string expands to at least %2">,
-  InGroup;
+  InGroup>;
 
 def warn_fortify_scanf_overflow : Warning<
   "'%0' may overflow; destination buffer in argument %1 has size "
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index fad70223362eddd..8260e72bd99e839 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1350,10 +1350,14 @@ void 
Sema::checkFortifiedBuiltinMemoryFunction(FunctionDecl *FD,
 llvm::APSInt::getUnsigned(H.getSizeLowerBound())
 .extOrTrunc(SizeTypeWidth);
 if (FormatSize > *SourceSize && *SourceSize != 0) {
-  DiagID = diag::warn_fortify_source_format_truncation;
-  DestinationSize = SourceSize;
-  SourceSize = FormatSize;
-  break;
+  SmallString<16> SpecifiedSizeStr;
+  SmallString<16> FormatSizeStr;
+  SourceSize->toString(SpecifiedSizeStr, /*Radix=*/10);
+  FormatSize.toString(FormatSizeStr, /*Radix=*/10);
+  DiagRuntimeBehavior(TheCall->getBeginLoc(), TheCall,
+  
PDiag(diag::warn_fortify_source_format_truncation)
+  << GetFunctionName() << SpecifiedSizeStr
+  << FormatSizeStr);
 }
   }
 }
diff --git a/clang/test/Sema/warn-format-overflow-truncation.c 
b/clang/test/Sema/warn-format-overflow-truncation.c
new file mode 100644
index 000..dada1f9355afe07
--- /dev/null
+++ b/clang/test/Sema/warn-format-overflow-truncation.c
@@ -0,0 +1,153 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify 
-DUSE_BUILTINS
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -Wno-format-truncation 
-Wno-format-overflow %s -verify=off
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 
-Wno-format-truncation -Wno-format-overflow %s -verify=off
+
+typedef unsigned long size_t;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern int 

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

2023-09-13 Thread Takuya Shimizu via cfe-commits

hazohelet wrote:

Thanks for the detailed suggestion! I agree it's the appropriate approach.

I was planning to separate `Wformat-truncation` from `Wfortify-source` sometime 
after this PR (as was requested in the same issue), but now that we are going 
to introduce a new warning group, we should finish the separation before 
proceeding further on this.
Sadly we don't have any way to make stacked patches as of now, so I'll merge 
the separation change to this PR. If reviewers think it's unreasonable, let's 
make another PR for the separation then.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Richard Smith via cfe-commits


@@ -851,6 +851,50 @@ class ScanfDiagnosticFormatHandler
   }
 };
 
+/// `I` points to the next character of `%p` format.
+/// This functon checks if the subsequent character can be linux kernel's
+/// extnded format specifier
+static inline constexpr bool canBeLinuxFormatExtension(const char *I,
+   const char *E) {
+  assert(I < E && "format string not yet exhausted");
+  // Kernel Document: https://docs.kernel.org/core-api/printk-formats.html
+  switch (*I) {
+  default:
+return false;
+  case 'S':
+  case 's':
+  case 'B':
+  case 'R':
+  case 'r':
+  case 'h':
+  case 'b':
+  case 'M':
+  case 'm':
+  case 'I':
+  case 'i':
+  case 'E':
+  case 'U':
+  case 'V':
+  case 'K':
+  case 'N':
+  case '4':
+  case 'a':
+  case 'd':
+  case 't':
+  case 'C':
+  case 'D':
+  case 'g':
+  case 'G':
+  case 'O':
+  case 'f':
+  case 'x':
+  case 'e':
+  case 'u':
+  case 'k':
+return true;
+  }
+}
+
 class EstimateSizeFormatHandler

zygoloid wrote:

Here's what I'd suggest:

- Add a flag to this class to track whether we've seen any specifiers that the 
Linux kernel gives unusual behavior to.
- Set the flag after line 933 (`   case 
analyze_format_string::ConversionSpecifier::pArg:`).
- On line 1251 (`DiagID = diag::warn_fortify_source_format_overflow;`), set 
`DiagID` to a different value that's in a separate diagnostic group with a 
different `-W` flag that's nested within the `FortifySource` group.

That'll mean that no-one loses any diagnostic quality, and we have a new flag, 
say `-Wno-fortify-source-non-kprintf` that the Linux kernel can use to turn off 
these warnings.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Richard Smith via cfe-commits

https://github.com/zygoloid edited 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Richard Smith via cfe-commits

https://github.com/zygoloid requested changes to this pull request.

This looks like the wrong approach to me. I really think you should be 
splitting the warning into two parts, so that only the kernel developers see a 
change in diagnostic output. That would seem to make this patch simpler as well 
as avoiding a (probably unimportant) regression for all other Clang users.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Nick Desaulniers via cfe-commits


@@ -851,6 +851,50 @@ class ScanfDiagnosticFormatHandler
   }
 };
 
+/// `I` points to the next character of `%p` format.
+/// This functon checks if the subsequent character can be linux kernel's
+/// extnded format specifier
+static inline constexpr bool canBeLinuxFormatExtension(const char *I,
+   const char *E) {
+  assert(I < E && "format string not yet exhausted");
+  // Kernel Document: https://docs.kernel.org/core-api/printk-formats.html
+  switch (*I) {

nickdesaulniers wrote:

I think it would also be helpful to say something along the lines of:

see the `pointer()` function in 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/vsprintf.c.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Nick Desaulniers via cfe-commits


@@ -851,6 +851,50 @@ class ScanfDiagnosticFormatHandler
   }
 };
 
+/// `I` points to the next character of `%p` format.
+/// This functon checks if the subsequent character can be linux kernel's
+/// extnded format specifier
+static inline constexpr bool canBeLinuxFormatExtension(const char *I,
+   const char *E) {
+  assert(I < E && "format string not yet exhausted");
+  // Kernel Document: https://docs.kernel.org/core-api/printk-formats.html
+  switch (*I) {

nickdesaulniers wrote:

The URL with anchor 
https://docs.kernel.org/core-api/printk-formats.html#pointer-types may be more 
precise to use in the comment.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Nick Desaulniers via cfe-commits


@@ -163,6 +163,10 @@ Improvements to Clang's diagnostics
 - Clang's ``-Wfortify-source`` now diagnoses ``snprintf`` call that is known to
   result in string truncation.
   (`#64871: `_).
+  Clang stops calculating the minimum length of the output format string upon 
%p
+  if the succeeding character can be a part of the format specifier in linux's
+  format extension in order to avoid false positive warnings against the linux
+  code base.

nickdesaulniers wrote:

perhaps worth referring to "the Linux kernel's ..." rather than just "linux".

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Nick Desaulniers via cfe-commits

nickdesaulniers wrote:

> Is there some way we can narrow the scope of this patch so we don't lose 
> warnings for normal `snprintf`s, only for the kernel one? If we really can't 
> tell the difference from the source code, we could move the affected warnings 
> to a different warning group instead, so the kernel can turn them off.

@zygoloid is this approach acceptable?

One thing I do worry about not having an explicit flag; we support clang-11+ 
for building the Linux kernel.

Let's say the kernel adds a new extension format specifiers `%pn`.  We need to 
fix up clang to recognize that now, too.  Then we fix clang in say clang-19.  
clang-18 still warns and we can't turn it off.

Perhaps it would be better to have the flag to disable `%p` without disabling 
the rest of `-Wfortify-source`, than try to keep these two in sync.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Takuya Shimizu via cfe-commits


@@ -851,6 +851,50 @@ class ScanfDiagnosticFormatHandler
   }
 };
 
+/// `I` points to the next character of `%p` format.
+/// This functon checks if the subsequent character can be linux kernel's
+/// extnded format specifier
+static inline constexpr bool canBeLinuxFormatExtension(const char *I,
+   const char *E) {
+  assert(I < E && "format string not yet exhausted");
+  // Kernel Document: https://docs.kernel.org/core-api/printk-formats.html
+  switch (*I) {

hazohelet wrote:

These switch cases correspond to 
https://github.com/torvalds/linux/blob/0bb80ecc33a8fb5a682236443c1e740d5c917d1d/lib/vsprintf.c#L2406-L2412
Note that kernel extension `%pA` is for Rust and thus excluded here.
ref: https://docs.kernel.org/core-api/printk-formats.html

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Takuya Shimizu via cfe-commits

hazohelet wrote:

> Is there some way we can narrow the scope of this patch so we don't lose 
> warnings for normal `snprintf`s, only for the kernel one? If we really can't 
> tell the difference from the source code, we could move the affected warnings 
> to a different warning group instead, so the kernel can turn them off.

Thanks for the feedback. I narrowed the scope by checking whether the next 
character of `%p` can be part of the kernel extended format specifier.
If people write `%pM` or a similar outside kernel, it still aborts the format 
size estimator and could lead to false negative. But I think it would hurt 
people much less because people usually put non-alphanumeric character after 
`%p`.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-12 Thread Takuya Shimizu via cfe-commits

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

>From 80d8743441ce1e25f19dbc48bce480b5becfa208 Mon Sep 17 00:00:00 2001
From: Takuya Shimizu 
Date: Tue, 12 Sep 2023 00:39:44 +0900
Subject: [PATCH 1/3] [clang][Sema] Stop format size estimator upon %p to adapt
 to linux kernel's extension

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
---
 clang/include/clang/AST/FormatString.h |  6 ++
 clang/lib/AST/PrintfFormatString.cpp   |  8 +++-
 clang/lib/Sema/SemaChecking.cpp| 10 ++
 clang/test/Sema/warn-fortify-source.c  | 16 +---
 4 files changed, 36 insertions(+), 4 deletions(-)

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 ,
+  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 ,
 // 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(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 ,
+   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 ,
  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 

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

2023-09-11 Thread Richard Smith via cfe-commits

zygoloid wrote:

Is there some way we can narrow the scope of this patch so we don't lose 
warnings for normal `snprintf`s, only for the kernel one? If we really can't 
tell the difference from the source code, we could move the affected warnings 
to a different warning group instead, so the kernel can turn them off.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Nick Desaulniers via cfe-commits


@@ -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 ,
+   unsigned RemainLen) override {
+if (FS.getConversionSpecifier().getKind() ==
+analyze_printf::PrintfConversionSpecifier::pArg) {
+  assert(Size >= RemainLen && "no underflow");
+  Size -= RemainLen;
+  return true;
+}
+return false;

nickdesaulniers wrote:

Might be shorter to invert the condition and return early.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Nick Desaulniers via cfe-commits

https://github.com/nickdesaulniers commented:

Thanks for the patch!

Can we document this behavior somewhere?

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Nick Desaulniers via cfe-commits


@@ -429,7 +429,13 @@ bool 
clang::analyze_format_string::ParsePrintfString(FormatStringHandler ,
 // 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

nickdesaulniers wrote:

End comment with correct punctuation.

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Nick Desaulniers via cfe-commits

https://github.com/nickdesaulniers edited 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Takuya Shimizu via cfe-commits


@@ -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}}

hazohelet wrote:

GCC doesn't warn on this ([live demo](https://godbolt.org/z/bEbYx5Gar)). It's 
probably because GCC even ignores the terminating null char, but I think it 
makes sense to consider the null char and add 1 (Probably it doesn't matter 
much, though).

https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread via cfe-commits

llvmbot wrote:

@llvm/pr-subscribers-clang


Changes

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) 



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 ,
+  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 ,
 // 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(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 ,
+   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 ,
  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, 

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

2023-09-11 Thread via cfe-commits

https://github.com/llvmbot labeled 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet review_requested 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet review_requested 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet review_requested 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet labeled 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet labeled 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Takuya Shimizu via cfe-commits

https://github.com/hazohelet review_requested 
https://github.com/llvm/llvm-project/pull/65969
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-09-11 Thread Takuya Shimizu via cfe-commits

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

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


>From 80d8743441ce1e25f19dbc48bce480b5becfa208 Mon Sep 17 00:00:00 2001
From: Takuya Shimizu 
Date: Tue, 12 Sep 2023 00:39:44 +0900
Subject: [PATCH] [clang][Sema] Stop format size estimator upon %p to adapt to
 linux kernel's extension

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
---
 clang/include/clang/AST/FormatString.h |  6 ++
 clang/lib/AST/PrintfFormatString.cpp   |  8 +++-
 clang/lib/Sema/SemaChecking.cpp| 10 ++
 clang/test/Sema/warn-fortify-source.c  | 16 +---
 4 files changed, 36 insertions(+), 4 deletions(-)

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 ,
+  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 ,
 // 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(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 ,
+   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 ,
  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