[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach created this revision.
dmaclach added a project: clang.
Herald added a subscriber: cfe-commits.

Issue a warning when the Objective C runtime encoding generated for an iVar, 
method, or block exceeds a user configurable value.

Off by default.

Note that handling iVars and methods will also get us properties.

Objective C runtime information can get very large (several K) and most of it 
is unused. Having the warning allows us to diagnose where the problem exists in 
our code.


Repository:
  rC Clang

https://reviews.llvm.org/D55544

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Sema/Sema.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/objc-large-encoding-warn.m

Index: test/SemaObjC/objc-large-encoding-warn.m
===
--- test/SemaObjC/objc-large-encoding-warn.m
+++ test/SemaObjC/objc-large-encoding-warn.m
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -Wobjc-encoding-larger-than=10 -Wno-objc-root-class %s
+
+struct MyStruct {
+  int a;
+  int b;
+  struct MyStruct *c;
+};
+
+@interface MyClass {
+  struct MyStruct iVarThatWarns; // expected-warning {{instance variable encoding of size 24 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  struct MyStruct iVarThatDoesntWarn;
+#pragma clang diagnostic pop
+  id idThatDoesntWarn;
+}
+@end
+
+@implementation MyClass
+- (void)methodThatWarns:(struct MyStruct)aStruct {} // expected-warning {{instance method 'methodThatWarns:' encoding of size 33 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+- (void)methodThatDoesntWarn:(struct MyStruct)aStruct {}
+#pragma clang diagnostic pop
+- (void)methodThatAlsoDoesntWarn {}
+@end
+
+void BlockFunc() {
+  void(^aBlockThatWarns)(struct MyStruct) = ^(struct MyStruct a) {}; // expected-warning {{block argument encoding of size 31 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  void(^aBlockThatDoesntWarn)(struct MyStruct) = ^(struct MyStruct a) {};
+#pragma clang diagnostic pop
+  void(^aBlockThatAlsoDoesntWarn)() = ^() {};
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13777,6 +13777,16 @@
   if (getCurFunction())
 getCurFunction()->addBlock(BD);
 
+  /// Check for block objective C encoding size
+  if (getLangOpts().ObjC) {
+std::string encoding = Context.getObjCEncodingForBlock(Result);
+unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+if (encodingSize && encoding.length() > encodingSize) {
+  Diag(BD->getLocation(), diag::warn_objc_block_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+
   return Result;
 }
 
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -3875,6 +3875,23 @@
   }
 }
 
+// Run through the ivars and see if any of their encodings are > ObjCLargeEncodingSize.
+void Sema::DiagnoseLargeIvarEncodings(ObjCImplementationDecl *ID) {
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize == 0) return;
+  for (ObjCIvarDecl *ivar = ID->getClassInterface()->all_declared_ivar_begin();
+   ivar; ivar = ivar->getNextIvar()) {
+QualType QT = Context.getBaseElementType(ivar->getType());
+std::string encoding;
+QualType NotEncodedT;
+Context.getObjCEncodingForType(QT, encoding, nullptr, &NotEncodedT);
+if (encoding.length() > encodingSize) {
+  Diag(ivar->getLocation(), diag::warn_objc_ivar_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+}
+
 // Note: For class/category implementations, allMethods is always null.
 Decl *Sema::ActOnAtEnd(Scope *S, SourceRange AtEnd, ArrayRef allMethods,
ArrayRef allTUVars) {
@@ -4006,6 +4023,7 @@
   if (IDecl->hasDesignatedInitializers())
 DiagnoseMissingDesignatedInitOverrides(IC, IDecl);
   DiagnoseWeakIvars(*this, IC);
+  DiagnoseLargeIvarEncodings(IC);
   DiagnoseRetainableFlexibleArrayMember(*this, IDecl);
 
   bool HasRootClassAttr = IDecl->hasAttr();
@@ -4768,6 +4786,15 @@
 }
   }
 
+  /// Check for method encoding size.
+  std::string encoding = Context.getObjCEncodingForMethodDecl(ObjCMethod);
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize && encoding.length() > encodingSize) {
+Diag(ObjCMethod->getLocation(), diag::wa

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 177732.
dmaclach added a comment.

Added some spacing around early exit as requested by theraven.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55544/new/

https://reviews.llvm.org/D55544

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Sema/Sema.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/objc-large-encoding-warn.m

Index: test/SemaObjC/objc-large-encoding-warn.m
===
--- test/SemaObjC/objc-large-encoding-warn.m
+++ test/SemaObjC/objc-large-encoding-warn.m
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -Wobjc-encoding-larger-than=10 -Wno-objc-root-class %s
+
+struct MyStruct {
+  int a;
+  int b;
+  struct MyStruct *c;
+};
+
+@interface MyClass {
+  struct MyStruct iVarThatWarns; // expected-warning {{instance variable encoding of size 24 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  struct MyStruct iVarThatDoesntWarn;
+#pragma clang diagnostic pop
+  id idThatDoesntWarn;
+}
+@end
+
+@implementation MyClass
+- (void)methodThatWarns:(struct MyStruct)aStruct {} // expected-warning {{instance method 'methodThatWarns:' encoding of size 33 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+- (void)methodThatDoesntWarn:(struct MyStruct)aStruct {}
+#pragma clang diagnostic pop
+- (void)methodThatAlsoDoesntWarn {}
+@end
+
+void BlockFunc() {
+  void(^aBlockThatWarns)(struct MyStruct) = ^(struct MyStruct a) {}; // expected-warning {{block argument encoding of size 31 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  void(^aBlockThatDoesntWarn)(struct MyStruct) = ^(struct MyStruct a) {};
+#pragma clang diagnostic pop
+  void(^aBlockThatAlsoDoesntWarn)() = ^() {};
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13777,6 +13777,16 @@
   if (getCurFunction())
 getCurFunction()->addBlock(BD);
 
+  /// Check for block objective C encoding size
+  if (getLangOpts().ObjC) {
+std::string encoding = Context.getObjCEncodingForBlock(Result);
+unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+if (encodingSize && encoding.length() > encodingSize) {
+  Diag(BD->getLocation(), diag::warn_objc_block_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+
   return Result;
 }
 
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -3875,6 +3875,25 @@
   }
 }
 
+// Run through the ivars and see if any of their encodings are larger than
+// ObjCLargeEncodingSize.
+void Sema::DiagnoseLargeIvarEncodings(ObjCImplementationDecl *ID) {
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize == 0) return;
+
+  for (ObjCIvarDecl *ivar = ID->getClassInterface()->all_declared_ivar_begin();
+   ivar; ivar = ivar->getNextIvar()) {
+QualType QT = Context.getBaseElementType(ivar->getType());
+std::string encoding;
+QualType NotEncodedT;
+Context.getObjCEncodingForType(QT, encoding, nullptr, &NotEncodedT);
+if (encoding.length() > encodingSize) {
+  Diag(ivar->getLocation(), diag::warn_objc_ivar_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+}
+
 // Note: For class/category implementations, allMethods is always null.
 Decl *Sema::ActOnAtEnd(Scope *S, SourceRange AtEnd, ArrayRef allMethods,
ArrayRef allTUVars) {
@@ -4006,6 +4025,7 @@
   if (IDecl->hasDesignatedInitializers())
 DiagnoseMissingDesignatedInitOverrides(IC, IDecl);
   DiagnoseWeakIvars(*this, IC);
+  DiagnoseLargeIvarEncodings(IC);
   DiagnoseRetainableFlexibleArrayMember(*this, IDecl);
 
   bool HasRootClassAttr = IDecl->hasAttr();
@@ -4768,6 +4788,15 @@
 }
   }
 
+  /// Check for method encoding size.
+  std::string encoding = Context.getObjCEncodingForMethodDecl(ObjCMethod);
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize && encoding.length() > encodingSize) {
+Diag(ObjCMethod->getLocation(), diag::warn_objc_method_encoding_too_large)
+  << (ObjCMethod->isInstanceMethod() ? "instance" : "class")
+  << ObjCMethod->getDeclName() << (unsigned)encoding.length()
+  << (unsigned)encodingSize;
+  }
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Ind

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

In D55544#1326956 , @theraven wrote:

> It would probably be a good idea to have a similar check on properties, as 
> property encoding strings contain the type encoding (plus extra stuff).


Properties are already picked up based on the ivars and methods that they 
generate.

> I wonder if we also want an option in code generation to replace very long 
> type encodings (or encodings of specifically annotated ivars?) with `"?"` 
> ('unknown type')?

Yeah, this is the next step. I'm trying to decide how best to do this. For a 
large cross platform code base I don't necessarily want to have folks having to 
annotate every C++ class they use with a somewhat non-portable annotation. At 
the same time you can't make all structs/classes encode as ? because I think it 
will mess up some existing Objective C patterns such as NSCoding.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55544/new/

https://reviews.llvm.org/D55544



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 177744.
dmaclach marked an inline comment as done.
dmaclach added a comment.

Full Diffs as requested.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55544/new/

https://reviews.llvm.org/D55544

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Sema/Sema.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/objc-large-encoding-warn.m

Index: test/SemaObjC/objc-large-encoding-warn.m
===
--- test/SemaObjC/objc-large-encoding-warn.m
+++ test/SemaObjC/objc-large-encoding-warn.m
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -Wobjc-encoding-larger-than=10 -Wno-objc-root-class %s
+
+struct MyStruct {
+  int a;
+  int b;
+  struct MyStruct *c;
+};
+
+@interface MyClass {
+  struct MyStruct iVarThatWarns; // expected-warning {{instance variable encoding of size 24 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  struct MyStruct iVarThatDoesntWarn;
+#pragma clang diagnostic pop
+  id idThatDoesntWarn;
+}
+@end
+
+@implementation MyClass
+- (void)methodThatWarns:(struct MyStruct)aStruct {} // expected-warning {{instance method 'methodThatWarns:' encoding of size 33 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+- (void)methodThatDoesntWarn:(struct MyStruct)aStruct {}
+#pragma clang diagnostic pop
+- (void)methodThatAlsoDoesntWarn {}
+@end
+
+void BlockFunc() {
+  void(^aBlockThatWarns)(struct MyStruct) = ^(struct MyStruct a) {}; // expected-warning {{block argument encoding of size 31 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  void(^aBlockThatDoesntWarn)(struct MyStruct) = ^(struct MyStruct a) {};
+#pragma clang diagnostic pop
+  void(^aBlockThatAlsoDoesntWarn)() = ^() {};
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13777,6 +13777,16 @@
   if (getCurFunction())
 getCurFunction()->addBlock(BD);
 
+  /// Check for block objective C encoding size
+  if (getLangOpts().ObjC) {
+std::string encoding = Context.getObjCEncodingForBlock(Result);
+unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+if (encodingSize && encoding.length() > encodingSize) {
+  Diag(BD->getLocation(), diag::warn_objc_block_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+
   return Result;
 }
 
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -3875,6 +3875,25 @@
   }
 }
 
+// Run through the ivars and see if any of their encodings are larger than
+// ObjCLargeEncodingSize.
+void Sema::DiagnoseLargeIvarEncodings(ObjCImplementationDecl *ID) {
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize == 0) return;
+
+  for (ObjCIvarDecl *ivar = ID->getClassInterface()->all_declared_ivar_begin();
+   ivar; ivar = ivar->getNextIvar()) {
+QualType QT = Context.getBaseElementType(ivar->getType());
+std::string encoding;
+QualType NotEncodedT;
+Context.getObjCEncodingForType(QT, encoding, nullptr, &NotEncodedT);
+if (encoding.length() > encodingSize) {
+  Diag(ivar->getLocation(), diag::warn_objc_ivar_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+}
+
 // Note: For class/category implementations, allMethods is always null.
 Decl *Sema::ActOnAtEnd(Scope *S, SourceRange AtEnd, ArrayRef allMethods,
ArrayRef allTUVars) {
@@ -4006,6 +4025,7 @@
   if (IDecl->hasDesignatedInitializers())
 DiagnoseMissingDesignatedInitOverrides(IC, IDecl);
   DiagnoseWeakIvars(*this, IC);
+  DiagnoseLargeIvarEncodings(IC);
   DiagnoseRetainableFlexibleArrayMember(*this, IDecl);
 
   bool HasRootClassAttr = IDecl->hasAttr();
@@ -4768,6 +4788,15 @@
 }
   }
 
+  /// Check for method encoding size.
+  std::string encoding = Context.getObjCEncodingForMethodDecl(ObjCMethod);
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize && encoding.length() > encodingSize) {
+Diag(ObjCMethod->getLocation(), diag::warn_objc_method_encoding_too_large)
+  << (ObjCMethod->isInstanceMethod() ? "instance" : "class")
+  << ObjCMethod->getDeclName() << (unsigned)encoding.length()
+  << (unsigned)encodingSize;
+  }
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 177746.
dmaclach added a comment.

Updated to fix Stephane's good catch of Objective C vs Objective-C


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55544/new/

https://reviews.llvm.org/D55544

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Sema/Sema.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclObjC.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/objc-large-encoding-warn.m

Index: test/SemaObjC/objc-large-encoding-warn.m
===
--- test/SemaObjC/objc-large-encoding-warn.m
+++ test/SemaObjC/objc-large-encoding-warn.m
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -Wobjc-encoding-larger-than=10 -Wno-objc-root-class %s
+
+struct MyStruct {
+  int a;
+  int b;
+  struct MyStruct *c;
+};
+
+@interface MyClass {
+  struct MyStruct iVarThatWarns; // expected-warning {{instance variable encoding of size 24 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  struct MyStruct iVarThatDoesntWarn;
+#pragma clang diagnostic pop
+  id idThatDoesntWarn;
+}
+@end
+
+@implementation MyClass
+- (void)methodThatWarns:(struct MyStruct)aStruct {} // expected-warning {{instance method 'methodThatWarns:' encoding of size 33 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+- (void)methodThatDoesntWarn:(struct MyStruct)aStruct {}
+#pragma clang diagnostic pop
+- (void)methodThatAlsoDoesntWarn {}
+@end
+
+void BlockFunc() {
+  void(^aBlockThatWarns)(struct MyStruct) = ^(struct MyStruct a) {}; // expected-warning {{block argument encoding of size 31 is larger than 10 bytes}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wobjc-encoding-larger-than="
+  void(^aBlockThatDoesntWarn)(struct MyStruct) = ^(struct MyStruct a) {};
+#pragma clang diagnostic pop
+  void(^aBlockThatAlsoDoesntWarn)() = ^() {};
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -13777,6 +13777,16 @@
   if (getCurFunction())
 getCurFunction()->addBlock(BD);
 
+  /// Check for block objective C encoding size
+  if (getLangOpts().ObjC) {
+std::string encoding = Context.getObjCEncodingForBlock(Result);
+unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+if (encodingSize && encoding.length() > encodingSize) {
+  Diag(BD->getLocation(), diag::warn_objc_block_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+
   return Result;
 }
 
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -3875,6 +3875,25 @@
   }
 }
 
+// Run through the ivars and see if any of their encodings are larger than
+// ObjCLargeEncodingSize.
+void Sema::DiagnoseLargeIvarEncodings(ObjCImplementationDecl *ID) {
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize == 0) return;
+
+  for (ObjCIvarDecl *ivar = ID->getClassInterface()->all_declared_ivar_begin();
+   ivar; ivar = ivar->getNextIvar()) {
+QualType QT = Context.getBaseElementType(ivar->getType());
+std::string encoding;
+QualType NotEncodedT;
+Context.getObjCEncodingForType(QT, encoding, nullptr, &NotEncodedT);
+if (encoding.length() > encodingSize) {
+  Diag(ivar->getLocation(), diag::warn_objc_ivar_encoding_too_large)
+<< (unsigned)encoding.length() << (unsigned)encodingSize;
+}
+  }
+}
+
 // Note: For class/category implementations, allMethods is always null.
 Decl *Sema::ActOnAtEnd(Scope *S, SourceRange AtEnd, ArrayRef allMethods,
ArrayRef allTUVars) {
@@ -4006,6 +4025,7 @@
   if (IDecl->hasDesignatedInitializers())
 DiagnoseMissingDesignatedInitOverrides(IC, IDecl);
   DiagnoseWeakIvars(*this, IC);
+  DiagnoseLargeIvarEncodings(IC);
   DiagnoseRetainableFlexibleArrayMember(*this, IDecl);
 
   bool HasRootClassAttr = IDecl->hasAttr();
@@ -4768,6 +4788,15 @@
 }
   }
 
+  /// Check for method encoding size.
+  std::string encoding = Context.getObjCEncodingForMethodDecl(ObjCMethod);
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize && encoding.length() > encodingSize) {
+Diag(ObjCMethod->getLocation(), diag::warn_objc_method_encoding_too_large)
+  << (ObjCMethod->isInstanceMethod() ? "instance" : "class")
+  << ObjCMethod->getDeclName() << (unsigned)encoding.length()
+  << (unsigned)encodingSize;
+  }
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;

[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added inline comments.



Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:53
+diag(Block->getCaretLocation(),
+ "Objective-C type encoding for block expression exceeds %0 "
+ "characters")

I found it very useful in my diagnostics to know how big it actually was. Can 
you add that?



Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:101
+  diag(EncodedDecl->getLocation(),
+   "Objective-C type encoding for %0 exceeds %1 characters")
+  << EncodedDecl << Threshold;

Can you add what the actual size was in the output?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55640/new/

https://reviews.llvm.org/D55640



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55544: Warning: objc-encodings-larger-than=

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

Akira/John/Erik - any thoughts?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55544/new/

https://reviews.llvm.org/D55544



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67865: [clang-tidy] Finds uses of OSRead* calls on macOS that may mask unexpected behavior due to unaligned reads

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach created this revision.
dmaclach added a reviewer: alexfh.
Herald added subscribers: cfe-commits, kristof.beyls, xazax.hun, mgorny.
Herald added a project: clang.

We ran into a problem in protocol buffers recently when compiling with Xcode 11 
that caused crashes on 32 bit ARM architectures due to undefined behavior 
caused by OSRead* calls in OSByteOrder.h when reading unaligned addresses.

These potential errors can easily be fixed by replacing the OSRead* call with a 
memcpy and then a OSSwap{Big|Little}ToHostInt{16|32|64}.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D67865

Files:
  clang-tidy/objc/AvoidOSReadCheck.cpp
  clang-tidy/objc/AvoidOSReadCheck.h
  clang-tidy/objc/CMakeLists.txt
  clang-tidy/objc/ObjCTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/objc-avoid-osread.rst
  test/clang-tidy/objc-avoid-osread.m

Index: test/clang-tidy/objc-avoid-osread.m
===
--- /dev/null
+++ test/clang-tidy/objc-avoid-osread.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s objc-avoid-osread %t
+
+void f() {
+  const char *buff = "";
+  OSReadBigInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+}
Index: docs/clang-tidy/checks/objc-avoid-osread.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/objc-avoid-osread.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - objc-avoid-osread
+
+objc-avoid-osread
+=
+
+Finds usages of ``OSRead{Big|Little}Int{16|32|64}`` and associated functions which
+should be avoided due to potential unaligned read problems.
+
+This check will detect following function invocations:
+
+- ``OSReadBigInt``
+- ``OSReadLittleInt``
+- ``OSReadBigInt16``
+- ``OSReadBigInt32``
+- ``OSReadBigInt64``

[PATCH] D67865: [clang-tidy] Finds uses of OSRead* calls on macOS that may mask unexpected behavior due to unaligned reads

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 221420.
dmaclach added a comment.

Fixed up review comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67865/new/

https://reviews.llvm.org/D67865

Files:
  clang-tidy/objc/AvoidOSReadCheck.cpp
  clang-tidy/objc/AvoidOSReadCheck.h
  clang-tidy/objc/CMakeLists.txt
  clang-tidy/objc/ObjCTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/objc-avoid-osread.rst
  test/clang-tidy/objc-avoid-osread.m

Index: test/clang-tidy/objc-avoid-osread.m
===
--- /dev/null
+++ test/clang-tidy/objc-avoid-osread.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s objc-avoid-osread %t
+
+void f() {
+  const char *buff = "";
+  OSReadBigInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+}
Index: docs/clang-tidy/checks/objc-avoid-osread.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/objc-avoid-osread.rst
@@ -0,0 +1,37 @@
+.. title:: clang-tidy - objc-avoid-osread
+
+objc-avoid-osread
+=
+
+Finds usages of ``OSRead{Big|Little}Int{16|32|64}`` and associated functions which
+should be avoided due to potential unaligned read problems.
+
+This check will detect following function invocations:
+
+- ``OSReadBigInt``
+- ``OSReadLittleInt``
+- ``OSReadBigInt16``
+- ``OSReadBigInt32``
+- ``OSReadBigInt64``
+- ``OSReadLittleInt16``
+- ``OSReadLittleInt32``
+- ``OSReadLittleInt64``
+- ``OSReadSwapInt16``
+- ``OSReadSwapInt3``
+- ``OSReadSwapInt64``
+- ``_OSReadInt16``
+- ``_OSReadInt32``
+- ``_OSReadInt64``
+
+The OSRead* functions from libkern/OSByteOrder.h have undocumented dependencies on aligned reads due to type punning by casting through a pointer:
+
+.. code:: c
+
+   OS_INLINE uint64_t _OSReadInt64(const v

[PATCH] D67865: [clang-tidy] Finds uses of OSRead* calls on macOS that may mask unexpected behavior due to unaligned reads

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach marked 5 inline comments as done.
dmaclach added inline comments.



Comment at: clang-tidy/objc/AvoidOSReadCheck.cpp:21
+void AvoidOSReadCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  callExpr(callee((functionDecl(hasAnyName(

Eugene.Zelenko wrote:
> Please add check if language is Objective-C.
So this isn't specific to Objective C, and applies to all C based languages. 
This is more Darwin specific. I based this CL on the AvoidSpinlockCheck which 
is very similar in this respect. If we want to do clean up on this later we 
should probably do them together.



Comment at: docs/clang-tidy/checks/objc-avoid-osread.rst:6
+
+Finds usages of ``OSRead{Big|Little}Int{16|32|64}`` and associated functions 
which
+should be avoided due to potential unaligned read problems.

Eugene.Zelenko wrote:
> Please synchronize with sentence in Release Notes.
Done. I coped this version into the release notes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67865/new/

https://reviews.llvm.org/D67865



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67865: [clang-tidy] Finds uses of OSRead* calls on macOS that may mask unexpected behavior due to unaligned reads

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 221433.
dmaclach added a comment.

Updated based on review


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67865/new/

https://reviews.llvm.org/D67865

Files:
  clang-tidy/objc/AvoidOSReadCheck.cpp
  clang-tidy/objc/AvoidOSReadCheck.h
  clang-tidy/objc/CMakeLists.txt
  clang-tidy/objc/ObjCTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/objc-avoid-osread.rst
  test/clang-tidy/objc-avoid-osread.m

Index: test/clang-tidy/objc-avoid-osread.m
===
--- /dev/null
+++ test/clang-tidy/objc-avoid-osread.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s objc-avoid-osread %t
+
+void f() {
+  const char *buff = "";
+  OSReadBigInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadBigInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadLittleInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  OSReadSwapInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt16(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt32(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+  _OSReadInt64(buff, 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use memcpy and OSSwap{Big|Little}ToHostInt{16|32|64} instead of OSRead* calls to avoid potential unaligned read issues [objc-avoid-osread]
+}
Index: docs/clang-tidy/checks/objc-avoid-osread.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/objc-avoid-osread.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - objc-avoid-osread
+
+objc-avoid-osread
+=
+
+Finds usages of ``OSRead{Big|Little}Int{16|32|64}`` and associated functions which
+should be avoided due to potential unaligned read problems.
+
+This check will detect following function invocations:
+
+- ``OSReadBigInt``
+- ``OSReadLittleInt``
+- ``OSReadBigInt16``
+- ``OSReadBigInt32``
+- ``OSReadBigInt64``
+- ``OSReadLittleInt16``
+- ``OSReadLittleInt32``
+- ``OSReadLittleInt64``
+- ``OSReadSwapInt16``
+- ``OSReadSwapInt3``
+- ``OSReadSwapInt64``
+- ``_OSReadInt16``
+- ``_OSReadInt32``
+- ``_OSReadInt64``
+
+The ``OSRead{Big|Little}Int{16|32|64}`` functions from ``libkern/OSByteOrder.h`` have
+undocumented dependencies on aligned reads due to type punning by casting through a 
+pointer:
+
+.. code:: c
+
+   OS_INL

[PATCH] D67865: [clang-tidy] Finds uses of OSRead* calls on macOS that may mask unexpected behavior due to unaligned reads

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

Any other comments on this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67865/new/

https://reviews.llvm.org/D67865



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

Thanks Puyan! Awesome to see this moving forward.




Comment at: clang/test/CodeGenObjC/objc-direct-wrapper.m:30
+#if ENABLE_VISIBLE_OBJC_DIRECT
+#define OBJC_DIRECT __attribute((objc_direct)) 
__attribute__((objc_direct_visible))
+#else

This is the case that mwyman described above where we would prefer to only have 
the single attribute correct?



Comment at: clang/test/CodeGenObjC/objc-direct-wrapper.m:35
+
+@interface C
+- (void)testMethod:(int)arg1 bar:(float)arg2 OBJC_DIRECT;

I'd like to see a test for the protocol case for coverage.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86049/new/

https://reviews.llvm.org/D86049

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

Has anything happened with this at all or did it get dropped?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86049/new/

https://reviews.llvm.org/D86049

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128031: Don't emit `-Wnullability-completeness` warnings on `weak` Objective-C properties.

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach accepted this revision.
dmaclach added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128031/new/

https://reviews.llvm.org/D128031

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72876: Create a clang-tidy check to warn when -dealloc is implemented inside an ObjC class category.

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach requested changes to this revision.
dmaclach added inline comments.
This revision now requires changes to proceed.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/objc-dealloc-in-categories.rst:10
+
+Classes implement ``-dealloc` to perform important actions just before an
+object is deallocated, but if a category on the class implements ``-dealloc``

Need an end quote on dealloc


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72876/new/

https://reviews.llvm.org/D72876



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144926: darwin platforms do not support static-lsan with TSan or ASan but were "silently failing" in that they would just ignore the flag and link in the dynamic runtimes instead.

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach created this revision.
Herald added a project: All.
dmaclach requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This matches the pre-existing UBSan failure path.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144926

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -457,6 +457,18 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
 // CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: static UndefinedBehaviorSanitizer runtime is not supported on darwin
 
+// RUN: %clang -fsanitize=address -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-STATIC-DARWIN %s
+// CHECK-ASAN-STATIC-DARWIN: {{.*}}error: static AddressSanitizer runtime is not supported on darwin
+
+// RUN: %clang -fsanitize=thread -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-TSAN-STATIC-DARWIN %s
+// CHECK-TSAN-STATIC-DARWIN: {{.*}}error: static ThreadSanitizer runtime is not supported on darwin
+
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1426,24 +1426,42 @@
 
   const SanitizerArgs &Sanitize = getSanitizerArgs(Args);
 
-  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
-getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
-return;
+  if (!Sanitize.needsSharedRt()) {
+const char *sanitizer = nullptr;
+if (Sanitize.needsUbsanRt()) {
+  sanitizer = "UndefinedBehaviorSanitizer";
+} else if (Sanitize.needsAsanRt()) {
+  sanitizer = "AddressSanitizer";
+} else if (Sanitize.needsTsanRt()) {
+  sanitizer = "ThreadSanitizer";
+}
+if (sanitizer) {
+  getDriver().Diag(diag::err_drv_unsupported_static_sanitizer_darwin)
+  << sanitizer;
+  return;
+}
   }
 
   if (Sanitize.linkRuntimes()) {
-if (Sanitize.needsAsanRt())
+if (Sanitize.needsAsanRt()) {
+  assert(Sanitize.needsSharedRt() &&
+ "Static sanitizer runtimes not supported");
   AddLinkSanitizerLibArgs(Args, CmdArgs, "asan");
+}
 if (Sanitize.needsLsanRt())
   AddLinkSanitizerLibArgs(Args, CmdArgs, "lsan");
 if (Sanitize.needsUbsanRt()) {
-  assert(Sanitize.needsSharedRt() && "Static sanitizer runtimes not supported");
-  AddLinkSanitizerLibArgs(Args, CmdArgs,
-  Sanitize.requiresMinimalRuntime() ? "ubsan_minimal"
-: "ubsan");
+  assert(Sanitize.needsSharedRt() &&
+ "Static sanitizer runtimes not supported");
+  AddLinkSanitizerLibArgs(
+  Args, CmdArgs,
+  Sanitize.requiresMinimalRuntime() ? "ubsan_minimal" : "ubsan");
 }
-if (Sanitize.needsTsanRt())
+if (Sanitize.needsTsanRt()) {
+  assert(Sanitize.needsSharedRt() &&
+ "Static sanitizer runtimes not supported");
   AddLinkSanitizerLibArgs(Args, CmdArgs, "tsan");
+}
 if (Sanitize.needsFuzzer() && !Args.hasArg(options::OPT_dynamiclib)) {
   AddLinkSanitizerLibArgs(Args, CmdArgs, "fuzzer", /*shared=*/false);
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1215,7 +1215,7 @@
 def shared_libsan : Flag<["-"], "shared-libsan">,
   HelpText<"Dynamically link the sanitizer runtime">;
 def static_libsan : Flag<["-"], "static-libsan">,
-  HelpText<"Statically link the sanitizer runtime">;
+  HelpText<"Statically link the sanitizer runtime (Not supported for ASan, TSan or UBSan on darwin)">;
 def : Flag<["-"], "shared-libasan">, Alias;
 def fasm : Flag<["-"], "fasm">, Group;
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -220,8 +220,8 @@
   "malformed sanitizer coverage ignorelist: '%0'">;
 def err_drv_malformed_sanitizer_metadata_ignorelist : Error<
   "malformed sanitizer metadata ignorelist: '%0'">;
-def err_drv_unsu

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 500942.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -457,6 +457,18 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
 // CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: static UndefinedBehaviorSanitizer runtime is not supported on darwin
 
+// RUN: %clang -fsanitize=address -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-STATIC-DARWIN %s
+// CHECK-ASAN-STATIC-DARWIN: {{.*}}error: static AddressSanitizer runtime is not supported on darwin
+
+// RUN: %clang -fsanitize=thread -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-TSAN-STATIC-DARWIN %s
+// CHECK-TSAN-STATIC-DARWIN: {{.*}}error: static ThreadSanitizer runtime is not supported on darwin
+
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1426,24 +1426,42 @@
 
   const SanitizerArgs &Sanitize = getSanitizerArgs(Args);
 
-  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
-getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
-return;
+  if (!Sanitize.needsSharedRt()) {
+const char *sanitizer = nullptr;
+if (Sanitize.needsUbsanRt()) {
+  sanitizer = "UndefinedBehaviorSanitizer";
+} else if (Sanitize.needsAsanRt()) {
+  sanitizer = "AddressSanitizer";
+} else if (Sanitize.needsTsanRt()) {
+  sanitizer = "ThreadSanitizer";
+}
+if (sanitizer) {
+  getDriver().Diag(diag::err_drv_unsupported_static_sanitizer_darwin)
+  << sanitizer;
+  return;
+}
   }
 
   if (Sanitize.linkRuntimes()) {
-if (Sanitize.needsAsanRt())
+if (Sanitize.needsAsanRt()) {
+  assert(Sanitize.needsSharedRt() &&
+ "Static sanitizer runtimes not supported");
   AddLinkSanitizerLibArgs(Args, CmdArgs, "asan");
+}
 if (Sanitize.needsLsanRt())
   AddLinkSanitizerLibArgs(Args, CmdArgs, "lsan");
 if (Sanitize.needsUbsanRt()) {
-  assert(Sanitize.needsSharedRt() && "Static sanitizer runtimes not supported");
-  AddLinkSanitizerLibArgs(Args, CmdArgs,
-  Sanitize.requiresMinimalRuntime() ? "ubsan_minimal"
-: "ubsan");
+  assert(Sanitize.needsSharedRt() &&
+ "Static sanitizer runtimes not supported");
+  AddLinkSanitizerLibArgs(
+  Args, CmdArgs,
+  Sanitize.requiresMinimalRuntime() ? "ubsan_minimal" : "ubsan");
 }
-if (Sanitize.needsTsanRt())
+if (Sanitize.needsTsanRt()) {
+  assert(Sanitize.needsSharedRt() &&
+ "Static sanitizer runtimes not supported");
   AddLinkSanitizerLibArgs(Args, CmdArgs, "tsan");
+}
 if (Sanitize.needsFuzzer() && !Args.hasArg(options::OPT_dynamiclib)) {
   AddLinkSanitizerLibArgs(Args, CmdArgs, "fuzzer", /*shared=*/false);
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1215,7 +1215,7 @@
 def shared_libsan : Flag<["-"], "shared-libsan">,
   HelpText<"Dynamically link the sanitizer runtime">;
 def static_libsan : Flag<["-"], "static-libsan">,
-  HelpText<"Statically link the sanitizer runtime">;
+  HelpText<"Statically link the sanitizer runtime (Not supported for ASan, TSan or UBSan on darwin)">;
 def : Flag<["-"], "shared-libasan">, Alias;
 def fasm : Flag<["-"], "fasm">, Group;
 
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -220,8 +220,8 @@
   "malformed sanitizer coverage ignorelist: '%0'">;
 def err_drv_malformed_sanitizer_metadata_ignorelist : Error<
   "malformed sanitizer metadata ignorelist: '%0'">;
-def err_drv_unsupported_static_ubsan_darwin : Error<
-  "static Und

[PATCH] D144926: darwin platforms do not support static-lsan with TSan or ASan but were "silently failing" in that they would just ignore the flag and link in the dynamic runtimes instead.

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach abandoned this revision.
dmaclach added a comment.

Accidentally uploaded


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144926/new/

https://reviews.llvm.org/D144926

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

Updated with buildable patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

@usama54321 or @yln are you able to commit for me?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

Yep. Apologies. Been a long time since I committed anything to LLVM. I'll try 
and take a look tonight/first thing tomorrow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 501899.
dmaclach added a comment.
Herald added subscribers: Sanitizers, Enna1.

Updated with fixed tests for `replaceable_new_delete.cpp`.

Split `replaceable_new_delete.cpp` into `replaceable_new_delete_shared.cpp` and 
`replaceable_new_delete_static.cpp`.
Static is marked as failing on darwin.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/test/asan/TestCases/replaceable_new_delete.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp

Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
===
--- /dev/null
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
@@ -0,0 +1,35 @@
+// Ensure that operator new/delete are still replaceable using static-libsan.
+
+// FIXME: Weak symbols aren't supported on Windows, although some code in
+// compiler-rt already exists to solve this problem. We should probably define
+// the new/delete interceptors as "weak" using those workarounds as well.
+// UNSUPPORTED: target={{.*windows.*}}
+
+// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
+
+// darwin only supports shared-libsan, so this should fail.
+// XFAIL: darwin
+ 
+#include 
+#include 
+#include 
+
+void *operator new[](size_t size) {
+  fprintf(stderr, "replaced new\n");
+  return malloc(size);
+}
+
+void operator delete[](void *ptr) noexcept {
+  fprintf(stderr, "replaced delete\n");
+  return free(ptr);
+}
+
+int main(int argc, char **argv) {
+  // CHECK: replaced new
+  char *x = new char[5];
+  // CHECK: replaced delete
+  delete[] x;
+  // CHECK: ERROR: AddressSanitizer
+  *x = 13;
+  return 0;
+}
Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
===
--- compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
@@ -1,4 +1,4 @@
-// Ensure that operator new/delete are still replaceable.
+// Ensure that operator new/delete are still replaceable using shared-libsan.
 
 // FIXME: Weak symbols aren't supported on Windows, although some code in
 // compiler-rt already exists to solve this problem. We should probably define
@@ -6,7 +6,6 @@
 // UNSUPPORTED: target={{.*windows.*}}
 
 // RUN: %clangxx %s -o %t -fsanitize=address -shared-libsan && not %run %t 2>&1 | FileCheck %s
-// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
 
 #include 
 #include 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -457,6 +457,18 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
 // CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: static UndefinedBehaviorSanitizer runtime is not supported on darwin
 
+// RUN: %clang -fsanitize=address -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-STATIC-DARWIN %s
+// CHECK-ASAN-STATIC-DARWIN: {{.*}}error: static AddressSanitizer runtime is not supported on darwin
+
+// RUN: %clang -fsanitize=thread -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-TSAN-STATIC-DARWIN %s
+// CHECK-TSAN-STATIC-DARWIN: {{.*}}error: static ThreadSanitizer runtime is not supported on darwin
+
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1426,24 +1426,42 @@
 
   const SanitizerArgs &Sanitize = getSanitizerArgs(Args);
 
-  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
-getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
-return;
+  if (!Sanitize.needsSharedRt()) {
+const char *sanitizer = nullptr;
+if (Sanitize.needsUbsanRt()) {
+  sanitizer = "UndefinedBehaviorSanitizer";
+} else if (Sanitize.needsAsanRt()) {
+  sanitizer = "AddressSanitizer";
+} else if (Sanitize.needsTsanRt()) {
+  sanitizer = "ThreadSanitizer";
+}
+if (sanitizer) {
+  getDriver().Diag(diag::err_drv_un

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added inline comments.



Comment at: 
compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp:10-11
+
+// darwin only supports shared-libsan, so this should fail.
+// XFAIL: darwin
+ 

yln wrote:
> This should work, right?
No.. darwin should fail with the `-static-libsan` flag. This is the test that 
was failing and caused the rollback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 502138.
dmaclach marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/test/asan/TestCases/replaceable_new_delete.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp

Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
===
--- /dev/null
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
@@ -0,0 +1,35 @@
+// Ensure that operator new/delete are still replaceable using static-libsan.
+
+// FIXME: Weak symbols aren't supported on Windows, although some code in
+// compiler-rt already exists to solve this problem. We should probably define
+// the new/delete interceptors as "weak" using those workarounds as well.
+// UNSUPPORTED: target={{.*windows.*}}
+
+// darwin only supports `shared-libsan`.
+// UNSUPPORTED: darwin
+
+// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
+
+#include 
+#include 
+#include 
+
+void *operator new[](size_t size) {
+  fprintf(stderr, "replaced new\n");
+  return malloc(size);
+}
+
+void operator delete[](void *ptr) noexcept {
+  fprintf(stderr, "replaced delete\n");
+  return free(ptr);
+}
+
+int main(int argc, char **argv) {
+  // CHECK: replaced new
+  char *x = new char[5];
+  // CHECK: replaced delete
+  delete[] x;
+  // CHECK: ERROR: AddressSanitizer
+  *x = 13;
+  return 0;
+}
Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
===
--- compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
@@ -1,4 +1,4 @@
-// Ensure that operator new/delete are still replaceable.
+// Ensure that operator new/delete are still replaceable using shared-libsan.
 
 // FIXME: Weak symbols aren't supported on Windows, although some code in
 // compiler-rt already exists to solve this problem. We should probably define
@@ -6,7 +6,6 @@
 // UNSUPPORTED: target={{.*windows.*}}
 
 // RUN: %clangxx %s -o %t -fsanitize=address -shared-libsan && not %run %t 2>&1 | FileCheck %s
-// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
 
 #include 
 #include 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -457,6 +457,18 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
 // CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: static UndefinedBehaviorSanitizer runtime is not supported on darwin
 
+// RUN: %clang -fsanitize=address -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-STATIC-DARWIN %s
+// CHECK-ASAN-STATIC-DARWIN: {{.*}}error: static AddressSanitizer runtime is not supported on darwin
+
+// RUN: %clang -fsanitize=thread -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-TSAN-STATIC-DARWIN %s
+// CHECK-TSAN-STATIC-DARWIN: {{.*}}error: static ThreadSanitizer runtime is not supported on darwin
+
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1426,24 +1426,42 @@
 
   const SanitizerArgs &Sanitize = getSanitizerArgs(Args);
 
-  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
-getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
-return;
+  if (!Sanitize.needsSharedRt()) {
+const char *sanitizer = nullptr;
+if (Sanitize.needsUbsanRt()) {
+  sanitizer = "UndefinedBehaviorSanitizer";
+} else if (Sanitize.needsAsanRt()) {
+  sanitizer = "AddressSanitizer";
+} else if (Sanitize.needsTsanRt()) {
+  sanitizer = "ThreadSanitizer";
+}
+if (sanitizer) {
+  getDriver().Diag(diag::err_drv_unsupported_static_sanitizer_darwin)
+  << sanitizer;
+  return;
+}
   }
 
   if (Sanitize.linkRuntimes()) {
-if (Sanitize.needsAsanRt())
+if (Sanitize.needsAsanRt()) {
+  assert(Sanitize.needsSharedRt() &&
+ "Static saniti

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach added a comment.

Went with unsupported.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144672: [Sanitizers] Error when attempting to use `static-lsan` with `TSan` or `Asan` on darwin

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach updated this revision to Diff 502474.
dmaclach added a comment.

Moved to `REQUIRES: asan-static-runtime`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144672/new/

https://reviews.llvm.org/D144672

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/test/asan/TestCases/replaceable_new_delete.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
  compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp

Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
===
--- /dev/null
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_static.cpp
@@ -0,0 +1,35 @@
+// Ensure that operator new/delete are still replaceable using static-libsan.
+
+// FIXME: Weak symbols aren't supported on Windows, although some code in
+// compiler-rt already exists to solve this problem. We should probably define
+// the new/delete interceptors as "weak" using those workarounds as well.
+// UNSUPPORTED: target={{.*windows.*}}
+
+// darwin only supports `shared-libsan`.
+// REQUIRES: asan-static-runtime
+
+// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
+
+#include 
+#include 
+#include 
+
+void *operator new[](size_t size) {
+  fprintf(stderr, "replaced new\n");
+  return malloc(size);
+}
+
+void operator delete[](void *ptr) noexcept {
+  fprintf(stderr, "replaced delete\n");
+  return free(ptr);
+}
+
+int main(int argc, char **argv) {
+  // CHECK: replaced new
+  char *x = new char[5];
+  // CHECK: replaced delete
+  delete[] x;
+  // CHECK: ERROR: AddressSanitizer
+  *x = 13;
+  return 0;
+}
Index: compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
===
--- compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
+++ compiler-rt/test/asan/TestCases/replaceable_new_delete_shared.cpp
@@ -1,4 +1,4 @@
-// Ensure that operator new/delete are still replaceable.
+// Ensure that operator new/delete are still replaceable using shared-libsan.
 
 // FIXME: Weak symbols aren't supported on Windows, although some code in
 // compiler-rt already exists to solve this problem. We should probably define
@@ -6,7 +6,6 @@
 // UNSUPPORTED: target={{.*windows.*}}
 
 // RUN: %clangxx %s -o %t -fsanitize=address -shared-libsan && not %run %t 2>&1 | FileCheck %s
-// RUN: %clangxx %s -o %t -fsanitize=address -static-libsan && not %run %t 2>&1 | FileCheck %s
 
 #include 
 #include 
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -457,6 +457,18 @@
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-STATIC-DARWIN %s
 // CHECK-UBSAN-STATIC-DARWIN: {{.*}}error: static UndefinedBehaviorSanitizer runtime is not supported on darwin
 
+// RUN: %clang -fsanitize=address -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ASAN-STATIC-DARWIN %s
+// CHECK-ASAN-STATIC-DARWIN: {{.*}}error: static AddressSanitizer runtime is not supported on darwin
+
+// RUN: %clang -fsanitize=thread -### %s 2>&1 \
+// RUN: --target=x86_64-apple-darwin -fuse-ld=ld -static-libsan \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-TSAN-STATIC-DARWIN %s
+// CHECK-TSAN-STATIC-DARWIN: {{.*}}error: static ThreadSanitizer runtime is not supported on darwin
+
 // RUN: %clang -fsanitize=address,undefined -### %s 2>&1 \
 // RUN: --target=i386-unknown-linux -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1426,24 +1426,42 @@
 
   const SanitizerArgs &Sanitize = getSanitizerArgs(Args);
 
-  if (!Sanitize.needsSharedRt() && Sanitize.needsUbsanRt()) {
-getDriver().Diag(diag::err_drv_unsupported_static_ubsan_darwin);
-return;
+  if (!Sanitize.needsSharedRt()) {
+const char *sanitizer = nullptr;
+if (Sanitize.needsUbsanRt()) {
+  sanitizer = "UndefinedBehaviorSanitizer";
+} else if (Sanitize.needsAsanRt()) {
+  sanitizer = "AddressSanitizer";
+} else if (Sanitize.needsTsanRt()) {
+  sanitizer = "ThreadSanitizer";
+}
+if (sanitizer) {
+  getDriver().Diag(diag::err_drv_unsupported_static_sanitizer_darwin)
+  << sanitizer;
+  return;
+}
   }
 
   if (Sanitize.linkRuntimes()) {
-if (Sanitize.needsAsanRt())
+if (Sanitize.needsAsanRt()) {
+  assert(Sanitize.needsShared

[PATCH] D120372: [clang] 'unused-but-set-variable' warning should not apply to __attribute__((objc_precise_lifetime) Objective-C pointers

[Dave MacLachlan via Phabricator via cfe-commits]

dmaclach accepted this revision.
dmaclach added inline comments.



Comment at: clang/test/SemaObjC/objc-precise-lifetime-unused-variable.m:7
+  // no diagnostics for objects with precise lifetime semantics.
+  __attribute__((objc_precise_lifetime)) id x;
+  x = getFoo();

what happens in the case that x is never assigned to anything? Do we still get 
a warning?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120372/new/

https://reviews.llvm.org/D120372

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits