-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[clang][Sema] Accept gnu format attributes #160255
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
Conversation
FormatStringType::Syslog is never instantiated, we can remove it safely.
This patch teaches clang accepts gnu_printf, gnu_scanf and gnu_strftime. These attributes are aliases for printf, scanf and strftime. Ref: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
|
@llvm/pr-subscribers-clang Author: Xing Guo (higuoxing) ChangesThis patch teaches clang accepts gnu_printf, gnu_scanf and gnu_strftime. Ref: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html Full diff: https://github.com/llvm/llvm-project/pull/160255.diff 6 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index d017d1f829015..5edfc29d93781 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -503,7 +503,6 @@ enum class FormatStringType {
FreeBSDKPrintf,
OSTrace,
OSLog,
- Syslog,
Unknown
};
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 740b472b0eb16..614a7ba36a623 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -6862,10 +6862,10 @@ StringRef Sema::GetFormatStringTypeName(FormatStringType FST) {
FormatStringType Sema::GetFormatStringType(StringRef Flavor) {
return llvm::StringSwitch<FormatStringType>(Flavor)
- .Case("scanf", FormatStringType::Scanf)
- .Cases("printf", "printf0", "syslog", FormatStringType::Printf)
+ .Cases("gnu_scanf", "scanf", FormatStringType::Scanf)
+ .Cases("gnu_printf", "printf", "printf0", "syslog", FormatStringType::Printf)
.Cases("NSString", "CFString", FormatStringType::NSString)
- .Case("strftime", FormatStringType::Strftime)
+ .Cases("gnu_strftime", "strftime", FormatStringType::Strftime)
.Case("strfmon", FormatStringType::Strfmon)
.Cases("kprintf", "cmn_err", "vcmn_err", "zcmn_err",
FormatStringType::Kprintf)
@@ -6986,7 +6986,6 @@ bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
case FormatStringType::Kprintf:
case FormatStringType::FreeBSDKPrintf:
case FormatStringType::Printf:
- case FormatStringType::Syslog:
Diag(FormatLoc, diag::note_format_security_fixit)
<< FixItHint::CreateInsertion(FormatLoc, "\"%s\", ");
break;
@@ -9103,8 +9102,7 @@ static void CheckFormatString(
if (Type == FormatStringType::Printf || Type == FormatStringType::NSString ||
Type == FormatStringType::Kprintf ||
Type == FormatStringType::FreeBSDKPrintf ||
- Type == FormatStringType::OSLog || Type == FormatStringType::OSTrace ||
- Type == FormatStringType::Syslog) {
+ Type == FormatStringType::OSLog || Type == FormatStringType::OSTrace) {
bool IsObjC =
Type == FormatStringType::NSString || Type == FormatStringType::OSTrace;
if (ReferenceFormatString == nullptr) {
@@ -9140,8 +9138,7 @@ bool Sema::CheckFormatStringsCompatible(
if (Type != FormatStringType::Printf && Type != FormatStringType::NSString &&
Type != FormatStringType::Kprintf &&
Type != FormatStringType::FreeBSDKPrintf &&
- Type != FormatStringType::OSLog && Type != FormatStringType::OSTrace &&
- Type != FormatStringType::Syslog)
+ Type != FormatStringType::OSLog && Type != FormatStringType::OSTrace)
return true;
bool IsObjC =
@@ -9175,8 +9172,7 @@ bool Sema::ValidateFormatString(FormatStringType Type,
if (Type != FormatStringType::Printf && Type != FormatStringType::NSString &&
Type != FormatStringType::Kprintf &&
Type != FormatStringType::FreeBSDKPrintf &&
- Type != FormatStringType::OSLog && Type != FormatStringType::OSTrace &&
- Type != FormatStringType::Syslog)
+ Type != FormatStringType::OSLog && Type != FormatStringType::OSTrace)
return true;
FormatStringLiteral RefLit = Str;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index b876911384f6f..1f19931b74be6 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3629,10 +3629,10 @@ static FormatAttrKind getFormatAttrKind(StringRef Format) {
// Check for formats that get handled specially.
.Case("NSString", NSStringFormat)
.Case("CFString", CFStringFormat)
- .Case("strftime", StrftimeFormat)
+ .Cases("gnu_strftime", "strftime", StrftimeFormat)
// Otherwise, check for supported formats.
- .Cases("scanf", "printf", "printf0", "strfmon", SupportedFormat)
+ .Cases("gnu_scanf", "scanf", "gnu_printf", "printf", "printf0", "strfmon", SupportedFormat)
.Cases("cmn_err", "vcmn_err", "zcmn_err", SupportedFormat)
.Cases("kprintf", "syslog", SupportedFormat) // OpenBSD.
.Case("freebsd_kprintf", SupportedFormat) // FreeBSD.
diff --git a/clang/test/Sema/attr-format.c b/clang/test/Sema/attr-format.c
index 5b9e4d02bbaf9..de6a99b15780e 100644
--- a/clang/test/Sema/attr-format.c
+++ b/clang/test/Sema/attr-format.c
@@ -106,3 +106,11 @@ void b2(const char *a, ...) __attribute__((format(syslog, 1, 1))); // expecte
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}}
+
+// gnu_printf
+// same as format(pritf(...))...
+void a2(const char *a, ...) __attribute__((format(gnu_printf, 1, 2))); // no-error
+void b2(const char *a, ...) __attribute__((format(gnu_printf, 1, 1))); // expected-error {{'format' attribute parameter 3 is out of bounds}}
+void c2(const char *a, ...) __attribute__((format(gnu_printf, 0, 2))); // expected-error {{'format' attribute parameter 2 is out of bounds}}
+void d2(const char *a, int c) __attribute__((format(gnu_printf, 1, 2))); // expected-warning {{GCC requires a function with the 'format' attribute to be variadic}}
+void e2(char *str, int c, ...) __attribute__((format(gnu_printf, 2, 3))); // expected-error {{format argument not a string type}}
diff --git a/clang/test/Sema/format-strings-scanf.c b/clang/test/Sema/format-strings-scanf.c
index d1f694f3595cf..22c1cce2f989b 100644
--- a/clang/test/Sema/format-strings-scanf.c
+++ b/clang/test/Sema/format-strings-scanf.c
@@ -30,6 +30,7 @@ int fscanf(FILE * restrict, const char * restrict, ...) ;
int scanf(const char * restrict, ...) ;
int sscanf(const char * restrict, const char * restrict, ...) ;
int my_scanf(const char * restrict, ...) __attribute__((__format__(__scanf__, 1, 2)));
+int my_gnu_scanf(const char * restrict, ...) __attribute__((__format__(gnu_scanf, 1, 2)));
int vscanf(const char * restrict, va_list);
int vfscanf(FILE * restrict, const char * restrict, va_list);
@@ -98,6 +99,7 @@ void test_variants(int *i, const char *s, ...) {
fscanf(f, "%ld", i); // expected-warning{{format specifies type 'long *' but the argument has type 'int *'}}
sscanf(buf, "%ld", i); // expected-warning{{format specifies type 'long *' but the argument has type 'int *'}}
my_scanf("%ld", i); // expected-warning{{format specifies type 'long *' but the argument has type 'int *'}}
+ my_gnu_scanf("%ld", i); // expected-warning{{format specifies type 'long *' but the argument has type 'int *'}}
va_list ap;
va_start(ap, s);
diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c
index 4bff30c313c8f..4a2ee8900bb5a 100644
--- a/clang/test/Sema/format-strings.c
+++ b/clang/test/Sema/format-strings.c
@@ -679,6 +679,7 @@ void pr18905(void) {
void __attribute__((format(strfmon,1,2))) monformat(const char *fmt, ...);
void __attribute__((format(strftime,1,0))) dateformat(const char *fmt);
+void __attribute__((format(gnu_strftime,1,0))) gnu_dateformat(const char *fmt);
// Other formats
void test_other_formats(void) {
@@ -687,6 +688,8 @@ void test_other_formats(void) {
monformat(str); // expected-warning{{format string is not a string literal (potentially insecure)}}
dateformat(""); // expected-warning{{format string is empty}}
dateformat(str); // no-warning (using strftime non-literal is not unsafe)
+ gnu_dateformat(""); // expected-warning{{format string is empty}}
+ gnu_dateformat(str); // no-warning (using strftime non-literal is not unsafe)
}
// Do not warn about unused arguments coming from system headers.
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Sirraide
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine, but is it documented anywhere (by GCC) that e.g. gnu_printf behaves the same as printf?
Also, this still needs a release note
Looking at the issue you linked, they at least were equivalent in 2008 when they were added to GCC |
Yes. They are still equivalent at the moment. https://gcc.gnu.org/cgit/gcc/tree/gcc/c-family/c-format.cc?h=trunk#n5268 Maybe we can add one more alias gnu_strfmon for strfmon? |
Co-authored-by: Sirraide <aeternalmail@gmail.com>
That seems fine then; and yeah, we can add that one as well |
Sirraide
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Sirraide <aeternalmail@gmail.com>
* main: (502 commits) GlobalISel: Adjust insert point when expanding G_[SU]DIVREM (llvm#160683) [LV] Add coverage for fixing-up scalar resume values (llvm#160492) AMDGPU: Convert wave_any test to use update_mc_test_checks [LV] Add partial reduction tests multiplying extend with constants. Revert "[MLIR] Implement remark emitting policies in MLIR" (llvm#160681) [NFC][InstSimplify] Refactor fminmax-folds.ll test (llvm#160504) [LoongArch] Pre-commit tests for [x]vldi instructions with special constant splats (llvm#159228) [BOLT] Fix dwarf5-dwoid-no-dwoname.s (llvm#160676) [lldb][test] Refactor and expand TestMemoryRegionDirtyPages.py (llvm#156035) [gn build] Port 833d5f0 AMDGPU: Ensure both wavesize features are not set (llvm#159234) [LoopInterchange] Bail out when finding a dependency with all `*` elements (llvm#149049) [libc++] Avoid constructing additional objects when using map::at (llvm#157866) [lldb][test] Make hex prefix optional in DWARF union types test [X86] Add missing prefixes to trunc-sat tests (llvm#160662) [AMDGPU] Fix vector legalization for bf16 valu ops (llvm#158439) [LoongArch][NFC] Pre-commit tests for `[x]vadda.{b/h/w/d}` [mlir][tosa] Relax constraint on matmul verifier requiring equal operand types (llvm#155799) [clang][Sema] Accept gnu format attributes (llvm#160255) [LoongArch][NFC] Add tests for element extraction from binary add operation (llvm#159725) ...
| FreeBSDKPrintf, | ||
| OSTrace, | ||
| OSLog, | ||
| Syslog, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not really explained. It feels like a drive by fix, which we usually discourage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I fixed it by a separate commit. When merging this PR, commits are squashed and I forget to write a commit message for this change. Shall I revert this part of change?
This patch teaches clang accepts gnu_printf, gnu_scanf, gnu_strftime and gnu_strfmon. These attributes are aliases for printf, scanf, strftime and strfmon. Ref: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html Fixes: llvm#16219 --------- Co-authored-by: Sirraide <aeternalmail@gmail.com>
This patch teaches clang accepts gnu_printf, gnu_scanf and gnu_strftime.
These attributes are aliases for printf, scanf and strftime.
Ref: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
Fixes: #16219