Skip to content

release/19.x: [clang][Sema] Add support for OpenBSD's syslog format attribute (#97366) #100703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Jul 26, 2024

Backport e788788

Requested by: @brad0

@llvmbot llvmbot requested a review from Endilll as a code owner July 26, 2024 06:34
@llvmbot llvmbot added this to the LLVM 19.X Release milestone Jul 26, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Jul 26, 2024

@Sirraide What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from Sirraide July 26, 2024 06:35
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 26, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Jul 26, 2024

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport e788788

Requested by: @brad0


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

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/Sema/Sema.h (+1)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+3-2)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+2-2)
  • (modified) clang/test/Sema/attr-format.c (+7)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5b6ee9830b507..562f383e3b232 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -629,6 +629,8 @@ Attribute Changes in Clang
   The attributes declare constraints about a function's behavior pertaining to blocking and
   heap memory allocation.
 
+- Introduced a new format attribute ``__attribute__((format(syslog, 1, 2)))`` from OpenBSD.
+
 Improvements to Clang's diagnostics
 -----------------------------------
 - Clang now emits an error instead of a warning for ``-Wundefined-internal``
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 7bfdaaae45a93..2ec6367eccea0 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2214,6 +2214,7 @@ class Sema final : public SemaBase {
     FST_FreeBSDKPrintf,
     FST_OSTrace,
     FST_OSLog,
+    FST_Syslog,
     FST_Unknown
   };
   static FormatStringType GetFormatStringType(const FormatAttr *Format);
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index cf1196ad23c21..5dfeae27a3aca 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -6030,7 +6030,7 @@ static const Expr *maybeConstEvalStringLiteral(ASTContext &Context,
 Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) {
   return llvm::StringSwitch<FormatStringType>(Format->getType()->getName())
       .Case("scanf", FST_Scanf)
-      .Cases("printf", "printf0", FST_Printf)
+      .Cases("printf", "printf0", "syslog", FST_Printf)
       .Cases("NSString", "CFString", FST_NSString)
       .Case("strftime", FST_Strftime)
       .Case("strfmon", FST_Strfmon)
@@ -6124,6 +6124,7 @@ bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
     case FST_Kprintf:
     case FST_FreeBSDKPrintf:
     case FST_Printf:
+    case FST_Syslog:
       Diag(FormatLoc, diag::note_format_security_fixit)
         << FixItHint::CreateInsertion(FormatLoc, "\"%s\", ");
       break;
@@ -7860,7 +7861,7 @@ static void CheckFormatString(
 
   if (Type == Sema::FST_Printf || Type == Sema::FST_NSString ||
       Type == Sema::FST_FreeBSDKPrintf || Type == Sema::FST_OSLog ||
-      Type == Sema::FST_OSTrace) {
+      Type == Sema::FST_OSTrace || Type == Sema::FST_Syslog) {
     CheckPrintfHandler H(
         S, FExpr, OrigFormatExpr, Type, firstDataArg, numDataArgs,
         (Type == Sema::FST_NSString || Type == Sema::FST_OSTrace), Str, APK,
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 5fd8622c90dd8..3e53182b5130c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3397,8 +3397,8 @@ static FormatAttrKind getFormatAttrKind(StringRef Format) {
       // Otherwise, check for supported formats.
       .Cases("scanf", "printf", "printf0", "strfmon", SupportedFormat)
       .Cases("cmn_err", "vcmn_err", "zcmn_err", SupportedFormat)
-      .Case("kprintf", SupportedFormat)         // OpenBSD.
-      .Case("freebsd_kprintf", SupportedFormat) // FreeBSD.
+      .Cases("kprintf", "syslog", SupportedFormat) // OpenBSD.
+      .Case("freebsd_kprintf", SupportedFormat)    // FreeBSD.
       .Case("os_trace", SupportedFormat)
       .Case("os_log", SupportedFormat)
 
diff --git a/clang/test/Sema/attr-format.c b/clang/test/Sema/attr-format.c
index 1f4c864d4f78b..5a8b1ac9eca5c 100644
--- a/clang/test/Sema/attr-format.c
+++ b/clang/test/Sema/attr-format.c
@@ -99,3 +99,10 @@ void forward_fixed(const char *fmt, _Bool b, char i, short j, int k, float l, do
   a(fmt, b, i, j, k, l, m);
 }
 
+// OpenBSD
+// same as format(printf(...))...
+void a2(const char *a, ...) __attribute__((format(syslog, 1, 2)));    // no-error
+void b2(const char *a, ...) __attribute__((format(syslog, 1, 1)));    // expected-error {{'format' attribute parameter 3 is out of bounds}}
+void c2(const char *a, ...) __attribute__((format(syslog, 0, 2)));    // expected-error {{'format' attribute parameter 2 is out of bounds}}
+void d2(const char *a, int c) __attribute__((format(syslog, 1, 2)));  // expected-warning {{GCC requires a function with the 'format' attribute to be variadic}}
+void e2(char *str, int c, ...) __attribute__((format(syslog, 2, 3))); // expected-error {{format argument not a string type}}

@Endilll
Copy link
Contributor

Endilll commented Jul 26, 2024

I'm not sure we want to backport a feature this late.
I also don't see any discussion of this being critical for 19 in #97366

@Sirraide
Copy link
Member

Yeah, this isn’t really a bugfix, it’s adding a small feature that doesn’t seem critical or anything like that to me either, so I don’t think there is a need to backport this.

@tru
Copy link
Collaborator

tru commented Jul 29, 2024

We are open to finish of features in RC1-2, but if there is no big need for this it's better to leave it out.

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.

I don't see a need to backport this for 19.x, it seems reasonable to let it bake for a while in 20.x (though I would be surprised if there was significant negative fallout from the changes).

@tru
Copy link
Collaborator

tru commented Jul 29, 2024

I will close this since there seems to be consensus that we don't need to backport it. If there is differing opinions - feel free to re-open.

@tru tru closed this Jul 29, 2024
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
Development

Successfully merging this pull request may close these issues.

6 participants