[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)

2023-12-03 Thread Jared Grubb via cfe-commits

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

Looks good to me!

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


[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)

2023-12-03 Thread Jared Grubb via cfe-commits

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


[clang] [clang-format] Remove duplicates in @property using std::set (PR #74235)

2023-12-03 Thread Jared Grubb via cfe-commits


@@ -171,18 +171,18 @@ TEST_F(ObjCPropertyAttributeOrderFixerTest, 
HandlesDuplicatedAttributes) {
   Style.ObjCPropertyAttributeOrder = {"a", "b", "c"};
 
   // Just a dup and nothing else.
-  verifyFormat("@property(a, a) int p;", Style);
+  verifyFormat("@property(a) int p;", "@property(a, a) int p;", Style);
 
   // A dup and something else.
-  verifyFormat("@property(a, a, b) int p;", "@property(a, b, a) int p;", 
Style);
+  verifyFormat("@property(a, b) int p;", "@property(a, b, a) int p;", Style);
 
   // Duplicates using `=`: stable-sort irrespective of their value.

jaredgrubb wrote:

Nit: the comment about stable-sort isn't really applicable now.

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


[clang] [Clang] Add diagnostic about "%P" specifier with Objective-C pointers (#89968) (PR #89977)

2024-04-24 Thread Jared Grubb via cfe-commits

https://github.com/jaredgrubb created 
https://github.com/llvm/llvm-project/pull/89977

A Darwin extension '%P' combined with an Objective-C pointer seems to always be 
a bug.

'%P' will dump bytes at the pointed-to address (in contrast to '%p' which dumps 
the pointer itself). This extension is only allowed in "OS Log" contexts and is 
intended to be used like `%{uuid_t}.*16P` or `%{timeval}.*P`. If an ObjC 
pointer is used, then the internal runtime structure (aka, the is-a pointer and 
other runtime metadata) will be dumped, which (IMO) is never the expectation.

A simple diagnostic can help flag these scenarios.

>From 1d8e6a51d6df6ea760e5c857fffcfae238932091 Mon Sep 17 00:00:00 2001
From: Jared Grubb 
Date: Wed, 24 Apr 2024 12:05:20 -0700
Subject: [PATCH] [Clang] Add diagnostic to flag using "%P" specifier with
 Objective-C pointers (#89968)

A Darwin extension '%P' combined with an Objective-C pointer seems to always be 
a
bug.

'%P' will dump bytes at the pointed-to address (in contrast to '%p' which dumps
the pointer itself). This extension is only allowed in "OS Log" contexts and is 
intended
to be used like `%{uuid_t}.*16P` or `%{timeval}.*P`. If an ObjC pointer is used,
then the internal runtime structure (aka, the is-a pointer and other runtime 
metadata)
will be dumped, which (IMO) is never the expectation.

A simple diagnostic can help flag these scenarios.
---
 clang/include/clang/Basic/DiagnosticSemaKinds.td |  3 +++
 clang/lib/Sema/SemaChecking.cpp  | 11 +++
 clang/test/SemaObjC/format-strings-oslog.m   |  5 -
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6732a1a98452ad..d7de9cf2c86e0e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9901,6 +9901,9 @@ def warn_format_invalid_annotation : Warning<
 def warn_format_P_no_precision : Warning<
   "using '%%P' format specifier without precision">,
   InGroup;
+def warn_format_P_with_objc_pointer : Warning<
+  "using '%%P' format specifier with an Objective-C pointer results in dumping 
runtime object structure, not object value">,
+  InGroup;
 def warn_printf_ignored_flag: Warning<
   "flag '%0' is ignored when flag '%1' is present">,
   InGroup;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 67132701b41cfd..53dbd58b0252a2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12544,6 +12544,17 @@ CheckPrintfHandler::checkFormatExpr(const 
analyze_printf::PrintfSpecifier &FS,
 return true;
   }
 
+  // Diagnose attempts to use '%P' with ObjC object types, which will result in
+  // dumping raw class data (like is-a pointer), not actual data.
+  if (FS.getConversionSpecifier().getKind() == ConversionSpecifier::PArg &&
+  ExprTy->isObjCObjectPointerType()) {
+const CharSourceRange &CSR =
+getSpecifierRange(StartSpecifier, SpecifierLen);
+EmitFormatDiagnostic(S.PDiag(diag::warn_format_P_with_objc_pointer),
+ E->getExprLoc(), false, CSR);
+return true;
+  }
+
   ArgType::MatchKind ImplicitMatch = ArgType::NoMatch;
   ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
   ArgType::MatchKind OrigMatch = Match;
diff --git a/clang/test/SemaObjC/format-strings-oslog.m 
b/clang/test/SemaObjC/format-strings-oslog.m
index 20fec93b653bd5..af5aef3d617954 100644
--- a/clang/test/SemaObjC/format-strings-oslog.m
+++ b/clang/test/SemaObjC/format-strings-oslog.m
@@ -44,15 +44,18 @@ void test_os_log_format(const char *pc, int i, void *p, 
void *buf) {
 }
 
 // Test os_log_format primitive with ObjC string literal format argument.
-void test_objc(const char *pc, int i, void *p, void *buf, NSString *nss) {
+void test_objc(const char *pc, int i, void *p, void *buf, NSString *nss, id 
obj) {
   __builtin_os_log_format(buf, @"");
   __builtin_os_log_format(buf, @"%d"); // expected-warning {{more '%' 
conversions than data arguments}}
   __builtin_os_log_format(buf, @"%d", i);
+
   __builtin_os_log_format(buf, @"%P", p); // expected-warning {{using '%P' 
format specifier without precision}}
   __builtin_os_log_format(buf, @"%.10P", p);
   __builtin_os_log_format(buf, @"%.*P", p); // expected-warning {{field 
precision should have type 'int', but argument has type 'void *'}}
   __builtin_os_log_format(buf, @"%.*P", i, p);
   __builtin_os_log_format(buf, @"%.*P", i, i); // expected-warning {{format 
specifies type 'void *' but the argument has type 'int'}}
+  __builtin_os_log_format(buf, @"%.8P", nss);// expected-warning {{using 
'%P' format specifier with an Objective-C pointer results in dumping runtime 
object structure, not object value}}
+  __builtin_os_log_format(buf, @"%.*P", i, obj); // expected-warning {{using 
'%P' format specifier with an Objective-C pointer results in dumping runtime 
object structure,

[clang] [Clang] Add diagnostic about "%P" specifier with Objective-C pointers (#89968) (PR #89977)

2024-04-24 Thread Jared Grubb via cfe-commits

jaredgrubb wrote:

Cc: @ahatanaka @egorzhdan
Also cc @joker-eph as it looks like you were original author of some of this, 
in case you're still following it :)

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


[clang] [Clang] Add diagnostic about "%P" specifier with Objective-C pointers (#89968) (PR #89977)

2024-04-26 Thread Jared Grubb via cfe-commits


@@ -44,15 +44,18 @@ void test_os_log_format(const char *pc, int i, void *p, 
void *buf) {
 }
 
 // Test os_log_format primitive with ObjC string literal format argument.
-void test_objc(const char *pc, int i, void *p, void *buf, NSString *nss) {
+void test_objc(const char *pc, int i, void *p, void *buf, NSString *nss, id 
obj) {
   __builtin_os_log_format(buf, @"");
   __builtin_os_log_format(buf, @"%d"); // expected-warning {{more '%' 
conversions than data arguments}}
   __builtin_os_log_format(buf, @"%d", i);
+
   __builtin_os_log_format(buf, @"%P", p); // expected-warning {{using '%P' 
format specifier without precision}}
   __builtin_os_log_format(buf, @"%.10P", p);
   __builtin_os_log_format(buf, @"%.*P", p); // expected-warning {{field 
precision should have type 'int', but argument has type 'void *'}}
   __builtin_os_log_format(buf, @"%.*P", i, p);
   __builtin_os_log_format(buf, @"%.*P", i, i); // expected-warning {{format 
specifies type 'void *' but the argument has type 'int'}}
+  __builtin_os_log_format(buf, @"%.8P", nss);// expected-warning {{using 
'%P' format specifier with an Objective-C pointer results in dumping runtime 
object structure, not object value}}

jaredgrubb wrote:

I don't think so, but I can be convinced. IMO, using a `(void*)` cast could be 
the escape-hatch if someone actually really _does_ want to dump the ObjC 
runtime structure but also squash this warning.

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