[PATCH] D47979: [X86] Lowering Mask Scalar add/sub/mul/div intrinsics to native IR (Clang part)

2018-06-09 Thread Craig Topper via Phabricator via cfe-commits 
craig.topper added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:9926
+Value *Div = Builder.CreateFDiv(A, B);
+llvm::VectorType *MaskTy = llvm::VectorType::get(Builder.getInt1Ty(),
+ 
cast(Mask->getType())->getBitWidth());

Can we just emit the and+icmp that the other operations end up with?


Repository:
  rC Clang

https://reviews.llvm.org/D47979



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

[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

2018-06-09 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a reviewer: t.p.northover.
rjmccall added a comment.

Okay, as a code change this seems more reasonable to me.  I haven't really 
thought through the ABI-compatibility issues, though.  CC'ing Tim.


https://reviews.llvm.org/D46013



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

[PATCH] D47627: [ASTContext] Make getAddrSpaceQualType replace address spaces.

2018-06-09 Thread John McCall via Phabricator via cfe-commits 
rjmccall added a comment.

In https://reviews.llvm.org/D47627#1121970, @ebevhan wrote:

> There's actually a bit more to it than in these two patches. The complete 
> diff to this function in our downstream Clang looks like this:
>
>QualType
>ASTContext::getAddrSpaceQualType(QualType T, unsigned AddressSpace) const {
>   -  QualType CanT = getCanonicalType(T);
>   -  if (CanT.getAddressSpace() == AddressSpace)
>   +  if (T.getLocalQualifiers().getAddressSpace() == AddressSpace)
>return T;
>   
>  // If we are composing extended qualifiers together, merge together
>  // into one ExtQuals node.
>  QualifierCollector Quals;
>  const Type *TypeNode = Quals.strip(T);
>   
>  // If this type already has an address space specified, it cannot get
>  // another one.
>   -  assert(!Quals.hasAddressSpace() &&
>   - "Type cannot be in multiple addr spaces!");
>   -  Quals.addAddressSpace(AddressSpace);
>   +  if (Quals.hasAddressSpace())
>   +Quals.removeAddressSpace();
>   +  if (AddressSpace)
>   +Quals.addAddressSpace(AddressSpace);
>   
>  return getExtQualType(TypeNode, Quals);
>}
>  
>
>
>   In our downstream Clang, functions have address spaces. The desired address 
> space is applied in getFunctionType, using getAddrSpaceQualType. Due to how 
> FunctionTypes are built, it's possible to end up with noncanonical 
> FunctionType that doesn't have an address space whose canonical type does 
> have one. For example, when building the noncanonical type `void (*)(void * 
> const)`, we will first build its canonical type `void (_AS *)(void *)`. Since 
> getAddrSpaceQualType looks at the canonical type to determine whether the 
> address space is already applied, it's skipped and we end up with this 
> discrepancy.
>   
>
> Now that I test it again, I can't seem to induce the assertion. I suspect I 
> might just have changed it because the documentation said that was how it was 
> supposed to work. Perhaps I should be submitting the upper diff instead? 
> Though, considering that this cannot be reproduced in trunk maybe I simply 
> shouldn't submit it at all.


Well, the documentation mismatch is worth fixing even if the code isn't.  But I 
think at best your use-case calls for weakening the assertion to be that any 
existing address space isn't *different*, yeah.

Separately, I'm not sure that's really the right representation for a Harvard 
architecture (which is what I assume you're trying to extend Clang to support); 
I think you should probably just teach the compiler that function pointers are 
different.


Repository:
  rC Clang

https://reviews.llvm.org/D47627



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

[PATCH] D46651: [OpenCL] Support new/delete in Sema

2018-06-09 Thread John McCall via Phabricator via cfe-commits 
rjmccall added inline comments.



Comment at: lib/Sema/SemaDeclCXX.cpp:13010
+  if (SemaRef.getLangOpts().OpenCLCPlusPlus) {
+if (auto *PtrTy = ResultType.getTypePtr()->getAs()) {
+  ResultType = RemoveAddressSpaceFromPtr(SemaRef, PtrTy);

`getTypePtr()` is done automatically by `operator->` on `QualType`.  This can 
be fixed in multiple places in this patch.



Comment at: lib/Sema/SemaExprCXX.cpp:2030
+  }
+}
 

Anastasia wrote:
> svenvh wrote:
> > rjmccall wrote:
> > > I think a better interpretation of this rule would be to just error on 
> > > attempts to use the standard non-placement operator new/delete instead of 
> > > trying to outlaw the operator declarations.  For example, I don't know 
> > > why a user-defined non-global operator new would be problematic.
> > Good point, I have raised this with Khronos, so I will hold this off until 
> > we have clarification.
> The decision by Khronos is to allow all user defined new/delete operators 
> (even global). I have submitted the change to the spec. The next publishing 
> date is however in July.
Okay.  I agree with your decision in this patch to treat this as a defect in 
the spec and thus to go ahead and do the right thing now.


https://reviews.llvm.org/D46651



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

[PATCH] D44539: [Sema][Objective-C] Add check to warn when property of objc type has assign attribute

2018-06-09 Thread John McCall via Phabricator via cfe-commits 
rjmccall added inline comments.



Comment at: lib/Sema/SemaObjCProperty.cpp:2554
+  PropertyTy->isObjCRetainableType() &&
+  !PropertyTy->isObjCClassType()) {
+Diag(Loc, diag::warn_objc_property_assign_on_object);

Please use `isObjCARCImplicitlyUnretainedType` here.



Comment at: test/SemaObjC/property-in-class-extension-1.m:18
 
-@property (assign, readonly) NSString* changeMemoryModel; // expected-note 
{{property declared here}}
+@property (unsafe_unretained, readonly) NSString* changeMemoryModel; // 
expected-note {{property declared here}}
 

Whoa, why is this test case using `-Weverything`?  That seems unnecessarily 
fragile.


Repository:
  rC Clang

https://reviews.llvm.org/D44539



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

2018-06-09 Thread Csaba Dabis via Phabricator via cfe-commits 
Charusso updated this revision to Diff 150645.
Charusso added a comment.

Fix some comment.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

2018-06-09 Thread Csaba Dabis via Phabricator via cfe-commits 
Charusso updated this revision to Diff 150644.
Charusso added a comment.

Clang format.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void good_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = wcschr(src, '\0');
+}
+
+void bad_wmemmove(wchar_t *dest, const wchar_t *src) {
+  wmemmove(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

2018-06-09 Thread Csaba Dabis via Phabricator via cfe-commits 
Charusso added a comment.

@lebedev.ri there is all the false positive results from the last publicated 
result-dump:

F6334659: curl-lib-curl_path-c.html 
second result: F6334660: ffmpeg-libavformat-sdp.c.html 

all result: F6334663: openssl-apps-ca-c.html 
F6334665: postgresql-src-interfaces-ecpg-preproc-pgc-c.html 

F6334667: redis-src-redis-benchmark-c.html 

(Note: the two `memchr()` result were false positive in that post and there is 
no new result with the new matcher.)

Does the new test cases cover your advice?




Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:105
+
+   An integer non-zero value specifying if the target version implements ``_s``
+   suffixed memory and character handler functions which is safer than older

whisperity wrote:
> Why is this an integer, rather than a bool?
This is how the other Tidy checkers are use their options, I do not know either.


https://reviews.llvm.org/D45050



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

2018-06-09 Thread Csaba Dabis via Phabricator via cfe-commits 
Charusso updated this revision to Diff 150643.
Charusso marked 4 inline comments as done.
Charusso added a comment.

`memchr()` revision: it is problematic if the second argument is `'\0'`, there 
is no other case. Added a new type of matcher, to match function calls. More 
static functions and test cases and huge refactor.


https://reviews.llvm.org/D45050

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-not-null-terminated-result-in-initialization.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-strlen-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-before-safe.c
  test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c

Index: test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-not-null-terminated-result-wcslen-safe.c
@@ -0,0 +1,150 @@
+// RUN: %check_clang_tidy %s bugprone-not-null-terminated-result %t -- \
+// RUN: -- -std=c11
+
+typedef unsigned int size_t;
+typedef unsigned int wchar_t;
+typedef int errno_t;
+size_t wcslen(const wchar_t *);
+wchar_t *wcschr(const wchar_t *, int);
+errno_t *wcsncpy_s(wchar_t *, const wchar_t *, size_t);
+
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *malloc(size_t);
+void *realloc(void *, size_t);
+
+void *wmemcpy(void *, const void *, size_t);
+errno_t wmemcpy_s(void *, size_t, const void *, size_t);
+void *wmemchr(const void *, int, size_t);
+void *wmemmove(void *, const void *, size_t);
+errno_t wmemmove_s(void *, size_t, const void *, size_t);
+void *wmemset(void *, int, size_t);
+
+int wcsncmp(const wchar_t *, const wchar_t *, size_t);
+size_t wcsxfrm(wchar_t *, const wchar_t *, size_t);
+
+
+void bad_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'alloca' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: alloca(wcslen(src) + 1);
+}
+
+void good_alloca(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)alloca(wcslen(src) + 1);
+}
+
+void bad_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc(wcslen(src), sizeof(wchar_t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'calloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void good_calloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)calloc((wcslen(src) + 1), sizeof(wchar_t));
+}
+
+void bad_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc(wcslen(src) * 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'malloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: malloc((wcslen(src) * 2) + 1);
+}
+
+void good_malloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)malloc((wcslen(src) * 2) + 1);
+}
+
+void bad_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + wcslen(src)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memory allocated by 'realloc' is insufficient to hold the null terminator [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void good_realloc(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)realloc(dest, (wcslen(dest) + (wcslen(src) + 1)));
+}
+
+void bad_wmemcpy(wchar_t *dest, const wchar_t *src) {
+  wmemcpy(dest, src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_safe(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wmemcpy_s(dest, wcslen(dest), src, wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: the result from calling 'wmemcpy_s' is not null-terminated [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcsncpy_s(dest, src, wcslen(src));
+}
+
+void good_wmemcpy_s(wchar_t *dest, const wchar_t *src) {
+  wcsncpy_s(dest, src, wcslen(src));
+}
+
+void bad_wmemchr(wchar_t *dest, const wchar_t *src) {
+  dest = (wchar_t *)wmemchr(src, '\0', wcslen(src));
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: the length is too short for the last '\0' [bugprone-not-null-terminated-result]
+  // CHECK-FIXES: wcschr(src, '\0');
+}
+
+void

[PATCH] D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions

2018-06-09 Thread Shoaib Meenai via Phabricator via cfe-commits 
smeenai added a comment.

@rnk remember how I was asking you about inlining into catchpads on IRC a few 
days back? It was in relation to this change.

If I have a file `finally1.m`:

  void f(void);
  void g() {
@try {
  f();
} @finally {
  f();
}
  }

and compile it with:

  clang -cc1 -triple i686-windows-msvc -fobjc-runtime=ios-6.0 -fexceptions 
-fobjc-exceptions -Os -emit-llvm -o - finally1.m

the @finally calls out to the captured statement instead of inlining it:

  finally.catchall: ; preds = %catch.dispatch
%1 = catchpad within %0 [i8* null, i32 64, i8* null]
call fastcc void @__captured_stmt() [ "funclet"(token %1) ]
call void @_CxxThrowException(i8* null, %eh.ThrowInfo* null) #3 [ 
"funclet"(token %1) ]
unreachable

If I add an outer try-catch scope, as in `finally2.m`:

  void f(void);
  void g() {
@try {
  @try {
f();
  } @finally {
f();
  }
} @catch (...) {
}
  }

and run the same compilation command:

  clang -cc1 -triple i686-windows-msvc -fobjc-runtime=ios-6.0 -fexceptions 
-fobjc-exceptions -Os -emit-llvm -o - finally2.m

the @finally inlines the captured statement:

  finally.catchall: ; preds = %catch.dispatch
%2 = catchpad within %0 [i8* null, i32 64, i8* null]
invoke void @f() #2 [ "funclet"(token %2) ]
to label %invoke.cont1 unwind label %catch.dispatch4
  
  invoke.cont1: ; preds = %finally.catchall
invoke void @_CxxThrowException(i8* null, %eh.ThrowInfo* null) #3 [ 
"funclet"(token %2) ]
to label %unreachable unwind label %catch.dispatch4

Any idea why we would see inlining in one case and not the other? i686 vs. 
x86-64 doesn't make any difference, and neither does -Os vs. -O1 vs. -O2.


Repository:
  rC Clang

https://reviews.llvm.org/D47988



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

[PATCH] D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions

2018-06-09 Thread Shoaib Meenai via Phabricator via cfe-commits 
smeenai created this revision.
smeenai added reviewers: DHowett-MSFT, compnerd, majnemer, rjmccall, rnk.

We're implementing funclet-compatible code generation for Obj-C
exceptions when using the MSVC ABI. The idea is that the Obj-C runtime
will wrap Obj-C exceptions inside C++ exceptions, which allows for
interoperability with C++ exceptions (for Obj-C++) and zero-cost
exceptions. This is the approach taken by e.g. WinObjC, and I believe it
to be the best approach for Obj-C exceptions in the MSVC ABI.

This change implements emitting the actual funclet-compatible IR. The
generic exceptions codegen already takes care of emitting the proper
catch dispatch structures, but we need to ensure proper handling of
catch parameters and scoping (emitting the catchret). Finally blocks are
handled quite differently from Itanium; they're expected to be outlined
by the frontend, which limits some control flow possibilities but also
greatly simplifies their codegen. See r334251 for further discussion of
why frontend outlining is used.

Worked on with Saleem Abdulrasool .


Repository:
  rC Clang

https://reviews.llvm.org/D47988

Files:
  lib/CodeGen/CGCXXABI.h
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCMac.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/MicrosoftCXXABI.cpp
  test/CodeGenObjC/catch-lexical-block.m
  test/CodeGenObjC/exceptions-msvc.m

Index: test/CodeGenObjC/exceptions-msvc.m
===
--- test/CodeGenObjC/exceptions-msvc.m
+++ test/CodeGenObjC/exceptions-msvc.m
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple i686--windows-msvc -fobjc-runtime=ios-6.0 -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X86 %s
-// RUN: %clang_cc1 -triple i686--windows-msvc -fobjc-runtime=ios-6.0 -fobjc-arc -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X86 %s
-// RUN: %clang_cc1 -triple x86_64--windows-msvc -fobjc-runtime=ios-6.0 -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X64 %s
-// RUN: %clang_cc1 -triple x86_64--windows-msvc -fobjc-runtime=ios-6.0 -fobjc-arc -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X64 %s
+// RUN: %clang_cc1 -triple i686--windows-msvc -fobjc-runtime=ios-6.0 -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X86 --enable-var-scope %s
+// RUN: %clang_cc1 -triple i686--windows-msvc -fobjc-runtime=ios-6.0 -fobjc-arc -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X86,ARC --enable-var-scope %s
+// RUN: %clang_cc1 -triple x86_64--windows-msvc -fobjc-runtime=ios-6.0 -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X64 --enable-var-scope %s
+// RUN: %clang_cc1 -triple x86_64--windows-msvc -fobjc-runtime=ios-6.0 -fobjc-arc -fdeclspec -fexceptions -fobjc-exceptions -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,X64,ARC --enable-var-scope %s
 
 #if __has_feature(objc_arc)
 #define WEAK __weak
@@ -67,8 +67,9 @@
 @protocol P;
 
 void f(void);
+void __declspec(nothrow) g(void);
 
-void g() {
+void generate_ehtypes() {
   @try {
 f();
   } @catch (I *) {
@@ -79,3 +80,393 @@
   } @catch (id) {
   }
 }
+
+void basic_try_catch_1() {
+  @try {
+f();
+  } @catch (...) {
+g();
+  }
+}
+
+// CHECK-LABEL: define {{.*}}void @basic_try_catch_1(){{.*}} {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:invoke void @f()
+// CHECK-NEXT:to label %[[INVOKECONT:[^ ]+]] unwind label %[[DISPATCH:[^ ]+]]
+// CHECK:   [[DISPATCH]]:
+// CHECK-NEXT:%[[CATCHSWITCH:[^ ]+]] = catchswitch within none [label %[[CATCH:[^ ]+]]] unwind to caller
+// CHECK:   [[INVOKECONT]]:
+// CHECK-NEXT:br label %[[EHCONT:[^ ]+]]
+// CHECK:   [[EHCONT]]:
+// CHECK-NEXT:ret void
+// CHECK:   [[CATCH]]:
+// CHECK-NEXT:%[[CATCHPAD:[^ ]+]] = catchpad within %[[CATCHSWITCH]] [i8* null, i32 64, i8* null]
+// CHECK-NEXT:call void @g(){{.*}} [ "funclet"(token %[[CATCHPAD]]) ]
+// CHECK-NEXT:catchret from %[[CATCHPAD]] to label %[[CATCHRETDEST:[^ ]+]]
+// CHECK:   [[CATCHRETDEST]]:
+// CHECK-NEXT:br label %[[EHCONT]]
+// CHECK-NEXT:  }
+
+void basic_try_catch_2() {
+  @try {
+f();
+  } @catch (id) {
+g();
+  }
+}
+
+// CHECK-LABEL: define {{.*}}void @basic_try_catch_2(){{.*}} {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:invoke void @f()
+// CHECK-NEXT:to label %[[INVOKECONT:[^ ]+]] unwind label %[[DISPATCH:[^ ]+]]
+// CHECK:   [[DISPATCH]]:
+// CHECK-NEXT:%[[CATCHSWITCH:[^ ]+]] = catchswitch within none [label %[[CATCH:[^ ]+]]] unwind to caller
+// CHECK:   [[INVOKECONT]]:
+// CHECK-NEXT:br label %[[EHCONT:[^ ]+]]
+// CHECK:   [[EHCONT]]:
+// CHECK-NEXT:ret void
+// CHECK:

[PATCH] D47987: Provide only one declaration of __throw_runtime_error

2018-06-09 Thread Dimitry Andric via Phabricator via cfe-commits 
dim created this revision.
dim added reviewers: hiraditya, mclow.lists, EricWF.
Herald added subscribers: christof, krytarowski, emaste.

In https://reviews.llvm.org/rL279903, @hiraditya added a second declaration of
`__throw_runtime_error` to `include/__locale`.  The original declaration
was in `__locale` too, but @mclow.lists moved it from there to
`stdexcept`.

In FreeBSD we compile most things with gcc's -Wredundant-decls, which
warns about such redundant redeclations, so I would like to remove the
one in `stdexcept`, as all the other callers of the function already
include `__locale` anyway.

While here, move the declaration in `__locale` to just above its first
invocation.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47987

Files:
  include/__locale
  include/stdexcept


Index: include/stdexcept
===
--- include/stdexcept
+++ include/stdexcept
@@ -182,9 +182,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-// in the dylib
-_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
-
 _LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
 void __throw_logic_error(const char*__msg)
 {
Index: include/__locale
===
--- include/__locale
+++ include/__locale
@@ -201,6 +201,8 @@
 friend class locale::__imp;
 };
 
+_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
+
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 locale::locale(const locale& __other, _Facet* __f)
@@ -1229,8 +1231,6 @@
 _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
codecvt_byname)
 _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
codecvt_byname)
 _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS 
codecvt_byname)
-
-_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
 
 template 
 struct __narrow_to_utf8


Index: include/stdexcept
===
--- include/stdexcept
+++ include/stdexcept
@@ -182,9 +182,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-// in the dylib
-_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
-
 _LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
 void __throw_logic_error(const char*__msg)
 {
Index: include/__locale
===
--- include/__locale
+++ include/__locale
@@ -201,6 +201,8 @@
 friend class locale::__imp;
 };
 
+_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
+
 template 
 inline _LIBCPP_INLINE_VISIBILITY
 locale::locale(const locale& __other, _Facet* __f)
@@ -1229,8 +1231,6 @@
 _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS codecvt_byname)
 _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS codecvt_byname)
 _LIBCPP_EXTERN_TEMPLATE2(class _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS codecvt_byname)
-
-_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_runtime_error(const char*);
 
 template 
 struct __narrow_to_utf8
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker

2018-06-09 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334352: [analyzer] Clean up the program state map of 
DanglingInternalBufferChecker. (authored by rkovacs, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47416?vs=150637=150638#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47416

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -26,7 +26,8 @@
 
 namespace {
 
-class DanglingInternalBufferChecker : public Checker {
+class DanglingInternalBufferChecker : public Checker {
   CallDescription CStrFn;
 
 public:
@@ -36,6 +37,9 @@
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent , CheckerContext ) const;
+
+  /// Clean up the ProgramState map.
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 };
 
 } // end anonymous namespace
@@ -76,12 +80,29 @@
   // FIXME: What if Origin is null?
   const Expr *Origin = Call.getOriginExpr();
   State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
+  State = State->remove(TypedR);
   C.addTransition(State);
   return;
 }
   }
 }
 
+void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper ,
+ CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+  RawPtrMapTy RPM = State->get();
+  for (const auto Entry : RPM) {
+if (!SymReaper.isLive(Entry.second))
+  State = State->remove(Entry.first);
+if (!SymReaper.isLiveRegion(Entry.first)) {
+  // Due to incomplete destructor support, some dead regions might still
+  // remain in the program state map. Clean them up.
+  State = State->remove(Entry.first);
+}
+  }
+  C.addTransition(State);
+}
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager ) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker();


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -26,7 +26,8 @@
 
 namespace {
 
-class DanglingInternalBufferChecker : public Checker {
+class DanglingInternalBufferChecker : public Checker {
   CallDescription CStrFn;
 
 public:
@@ -36,6 +37,9 @@
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent , CheckerContext ) const;
+
+  /// Clean up the ProgramState map.
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 };
 
 } // end anonymous namespace
@@ -76,12 +80,29 @@
   // FIXME: What if Origin is null?
   const Expr *Origin = Call.getOriginExpr();
   State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
+  State = State->remove(TypedR);
   C.addTransition(State);
   return;
 }
   }
 }
 
+void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper ,
+ CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+  RawPtrMapTy RPM = State->get();
+  for (const auto Entry : RPM) {
+if (!SymReaper.isLive(Entry.second))
+  State = State->remove(Entry.first);
+if (!SymReaper.isLiveRegion(Entry.first)) {
+  // Due to incomplete destructor support, some dead regions might still
+  // remain in the program state map. Clean them up.
+  State = State->remove(Entry.first);
+}
+  }
+  C.addTransition(State);
+}
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager ) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker

2018-06-09 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ accepted this revision.
NoQ added a comment.

Great, thanks!


https://reviews.llvm.org/D47416



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

[PATCH] D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker

2018-06-09 Thread Reka Kovacs via Phabricator via cfe-commits 
rnkovacs updated this revision to Diff 150637.
rnkovacs marked 3 inline comments as done.
rnkovacs added a comment.

Addressed comments.


https://reviews.llvm.org/D47416

Files:
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp


Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -26,7 +26,8 @@
 
 namespace {
 
-class DanglingInternalBufferChecker : public Checker {
+class DanglingInternalBufferChecker : public Checker {
   CallDescription CStrFn;
 
 public:
@@ -36,6 +37,9 @@
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent , CheckerContext ) const;
+
+  /// Clean up the ProgramState map.
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 };
 
 } // end anonymous namespace
@@ -76,12 +80,29 @@
   // FIXME: What if Origin is null?
   const Expr *Origin = Call.getOriginExpr();
   State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
+  State = State->remove(TypedR);
   C.addTransition(State);
   return;
 }
   }
 }
 
+void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper ,
+ CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+  RawPtrMapTy RPM = State->get();
+  for (const auto Entry : RPM) {
+if (!SymReaper.isLive(Entry.second))
+  State = State->remove(Entry.first);
+if (!SymReaper.isLiveRegion(Entry.first)) {
+  // Due to incomplete destructor support, some dead regions might still
+  // remain in the program state map. Clean them up.
+  State = State->remove(Entry.first);
+}
+  }
+  C.addTransition(State);
+}
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager ) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker();


Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -26,7 +26,8 @@
 
 namespace {
 
-class DanglingInternalBufferChecker : public Checker {
+class DanglingInternalBufferChecker : public Checker {
   CallDescription CStrFn;
 
 public:
@@ -36,6 +37,9 @@
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent , CheckerContext ) const;
+
+  /// Clean up the ProgramState map.
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 };
 
 } // end anonymous namespace
@@ -76,12 +80,29 @@
   // FIXME: What if Origin is null?
   const Expr *Origin = Call.getOriginExpr();
   State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
+  State = State->remove(TypedR);
   C.addTransition(State);
   return;
 }
   }
 }
 
+void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper ,
+ CheckerContext ) const {
+  ProgramStateRef State = C.getState();
+  RawPtrMapTy RPM = State->get();
+  for (const auto Entry : RPM) {
+if (!SymReaper.isLive(Entry.second))
+  State = State->remove(Entry.first);
+if (!SymReaper.isLiveRegion(Entry.first)) {
+  // Due to incomplete destructor support, some dead regions might still
+  // remain in the program state map. Clean them up.
+  State = State->remove(Entry.first);
+}
+  }
+  C.addTransition(State);
+}
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager ) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

2018-06-09 Thread Eric Fiselier via Phabricator via cfe-commits 
EricWF updated this revision to Diff 150635.
EricWF added a comment.

Remove `-nostdinc++` check as requested.


https://reviews.llvm.org/D45015

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Driver/ToolChains/Darwin.cpp
  lib/Frontend/InitPreprocessor.cpp
  lib/Sema/SemaExprCXX.cpp
  test/Driver/unavailable_aligned_allocation.cpp
  test/Lexer/aligned-allocation.cpp

Index: test/Lexer/aligned-allocation.cpp
===
--- /dev/null
+++ test/Lexer/aligned-allocation.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -DEXPECT_DEFINED
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-alloc-unavailable
+//
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.12.0 -fexceptions -std=c++17 -verify %s \
+// RUN:   -faligned-allocation -faligned-alloc-unavailable
+
+// Test that __cpp_aligned_new is not defined when CC1 is passed
+// -faligned-alloc-unavailable by the Darwin driver, even when aligned
+// allocation is actually enabled.
+
+// expected-no-diagnostics
+#ifdef EXPECT_DEFINED
+# ifndef __cpp_aligned_new
+#   error "__cpp_aligned_new" should be defined
+# endif
+#else
+# ifdef __cpp_aligned_new
+#   error "__cpp_aligned_new" should not be defined
+# endif
+#endif
Index: test/Driver/unavailable_aligned_allocation.cpp
===
--- test/Driver/unavailable_aligned_allocation.cpp
+++ test/Driver/unavailable_aligned_allocation.cpp
@@ -51,4 +51,13 @@
 // RUN: -c -### %s 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=AVAILABLE
 //
+// Check that passing -faligned-allocation or -fno-aligned-allocation stops the
+// driver from passing -faligned-alloc-unavailable to cc1.
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -faligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+//
+// RUN: %clang -target x86_64-apple-macosx10.12 -fno-aligned-allocation -c -### %s 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=AVAILABLE
+
 // AVAILABLE-NOT: "-faligned-alloc-unavailable"
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1697,9 +1697,9 @@
 StringRef OSName = AvailabilityAttr::getPlatformNameSourceSpelling(
 S.getASTContext().getTargetInfo().getPlatformName());
 
-S.Diag(Loc, diag::warn_aligned_allocation_unavailable)
- << IsDelete << FD.getType().getAsString() << OSName
- << alignedAllocMinVersion(T.getOS()).getAsString();
+S.Diag(Loc, diag::err_aligned_allocation_unavailable)
+<< IsDelete << FD.getType().getAsString() << OSName
+<< alignedAllocMinVersion(T.getOS()).getAsString();
 S.Diag(Loc, diag::note_silence_unligned_allocation_unavailable);
   }
 }
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -551,7 +551,7 @@
 Builder.defineMacro("__cpp_nontype_template_args", "201411");
 Builder.defineMacro("__cpp_fold_expressions", "201603");
   }
-  if (LangOpts.AlignedAllocation)
+  if (LangOpts.AlignedAllocation && !LangOpts.AlignedAllocationUnavailable)
 Builder.defineMacro("__cpp_aligned_new", "201606");
 
   // TS features.
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -2018,7 +2018,12 @@
 void Darwin::addClangTargetOptions(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ,
Action::OffloadKind DeviceOffloadKind) const {
-  if (isAlignedAllocationUnavailable())
+  // Pass "-faligned-alloc-unavailable" only when the user hasn't manually
+  // enabled or disabled aligned allocations and hasn't specified a custom
+  // standard library installation.
+  if (!DriverArgs.hasArgNoClaim(options::OPT_faligned_allocation,
+options::OPT_fno_aligned_allocation) &&
+  isAlignedAllocationUnavailable())
 CC1Args.push_back("-faligned-alloc-unavailable");
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6419,12 +6419,12 @@
   "type %0 requires %1 bytes of alignment and the default allocator only "
   "guarantees %2 bytes">,
   InGroup, DefaultIgnore;
-def warn_aligned_allocation_unavailable :Warning<
+def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is only "
-  "available on %2 %3 or newer">, InGroup,

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

2018-06-09 Thread Eric Fiselier via Phabricator via cfe-commits 
EricWF added a comment.

In https://reviews.llvm.org/D45015#1127046, @vsapsai wrote:

> In https://reviews.llvm.org/D45015#1121762, @EricWF wrote:
>
> > In https://reviews.llvm.org/D45015#1121581, @ahatanak wrote:
> >
> > > Could you elaborate on what kind of changes you are planning to make in 
> > > libc++ after committing this patch?
> >
> >
> > Libc++ shouldn't actually need any changes if this current patch lands. 
> > Currently libc++ is in a "incorrect" state where
> >  it generates calls to `__builtin_operator_new(size_t, align_val_t)` when 
> > `__cpp_aligned_new` is defined but when aligned new/delete
> >  are actually unavailable.
> >
> > If we change `__cpp_aligned_new` to no longer be defined when aligned new 
> > is unavailable, then libc++ will start doing the right thing.
> >  See r328180 
> > 
> >  for the relevent commits which made these libc++ changes.
>
>
> Looks like in libc++ we need to remove `_LIBCPP_STD_VER` check for aligned 
> allocations, something like
>
>#if !defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) && \
>   -(!(defined(_LIBCPP_BUILDING_NEW) || _LIBCPP_STD_VER > 14 || \
>   +(!(defined(_LIBCPP_BUILDING_NEW) || \
>(defined(__cpp_aligned_new) && __cpp_aligned_new >= 201606)))
># define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
>#endif
>
>
> Is that correct? I didn't check the rest of the code, probably 
> TEST_HAS_NO_ALIGNED_ALLOCATION needs some clean up too.


Huh, that original formulation is correct-ish, and so is your proposed changes. 
There are two problems here:

1. We want to expose the aligned allocation declarations to users in C++17, 
even if the compiler will never call them because

the feature has been disabled.

2. We don't want to generate calls to them via `__builtin_operator_new` when 
we've only enabled the declarations because of C++17.

I'll make some changes to libc++ to address this. Thanks for pointing it out.


https://reviews.llvm.org/D45015



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

[PATCH] D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker

2018-06-09 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:92-94
+  if (!SymReaper.hasDeadSymbols())
+return;
+

This optimization is in fact incorrect due to an old bug that i didn't yet get 
to fixing: D18860. The proposed patch would most likely remove the check 
anyway, because the set of dead symbols is not well-defined. So i think we 
shouldn't add it.



Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:98
+  for (const auto Entry : RPM) {
+if (SymReaper.isDead(Entry.second))
+  State = State->remove(Entry.first);

For the same reason (D18860), it should be more correct to use `!isLive()`. 
Otherwise you may miss some symbols.



Comment at: 
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:100-103
+if (!SymReaper.isLiveRegion(Entry.first))
+  // Due to incomplete destructor support, some dead regions might still
+  // remain in the program state map. Clean them up.
+  State = State->remove(Entry.first);

I mildly advocate for braces here because that's as many as three lines to wrap.


https://reviews.llvm.org/D47416



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

r334348 - [analyzer] Add dangling internal buffer check.

2018-06-09 Thread Reka Kovacs via cfe-commits 
Author: rkovacs
Date: Sat Jun  9 06:03:49 2018
New Revision: 334348

URL: http://llvm.org/viewvc/llvm-project?rev=334348=rev
Log:
[analyzer] Add dangling internal buffer check.

This check will mark raw pointers to C++ standard library container internal
buffers 'released' when the objects themselves are destroyed. Such information
can be used by MallocChecker to warn about use-after-free problems.

In this first version, 'std::basic_string's are supported.

Differential Revision: https://reviews.llvm.org/D47135

Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=334348=334347=334348=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Sat Jun  9 
06:03:49 2018
@@ -300,6 +300,11 @@ def VirtualCallChecker : Checker<"Virtua
 
 let ParentPackage = CplusplusAlpha in {
 
+def DanglingInternalBufferChecker : Checker<"DanglingInternalBuffer">,
+  HelpText<"Check for internal raw pointers of C++ standard library containers 
"
+   "used after deallocation">,
+  DescFile<"DanglingInternalBufferChecker.cpp">;
+
 def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">,
   HelpText<"Reports destructions of polymorphic objects with a non-virtual "
"destructor in their base class">,

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h?rev=334348=auto
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h (added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h Sat Jun  9 06:03:49 
2018
@@ -0,0 +1,28 @@
+//===--- AllocationState.h - *- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ALLOCATIONSTATE_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ALLOCATIONSTATE_H
+
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+
+namespace clang {
+namespace ento {
+
+namespace allocation_state {
+
+ProgramStateRef markReleased(ProgramStateRef State, SymbolRef Sym,
+ const Expr *Origin);
+
+} // end namespace allocation_state
+
+} // end namespace ento
+} // end namespace clang
+
+#endif

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=334348=334347=334348=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Sat Jun  9 06:03:49 
2018
@@ -27,6 +27,7 @@ add_clang_library(clangStaticAnalyzerChe
   CloneChecker.cpp
   ConversionChecker.cpp
   CXXSelfAssignmentChecker.cpp
+  DanglingInternalBufferChecker.cpp
   DeadStoresChecker.cpp
   DebugCheckers.cpp
   DeleteWithNonVirtualDtorChecker.cpp

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp?rev=334348=auto
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp 
(added)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp Sat 
Jun  9 06:03:49 2018
@@ -0,0 +1,88 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ 
-*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines a check that marks a raw pointer to a C++ standard library
+// container's inner buffer released when the object is destroyed. This
+// information can be used by MallocChecker to detect use-after-free problems.
+//

[PATCH] D47416: [analyzer] Clean up the program state map of DanglingInternalBufferChecker

2018-06-09 Thread Reka Kovacs via Phabricator via cfe-commits 
rnkovacs updated this revision to Diff 150625.
rnkovacs marked an inline comment as done.
rnkovacs edited the summary of this revision.
rnkovacs added a comment.
Herald added a subscriber: mikhail.ramalho.

Fixed naming and added an extra pass for regions left behind by incomplete 
destructors.


https://reviews.llvm.org/D47416

Files:
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp


Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -26,7 +26,8 @@
 
 namespace {
 
-class DanglingInternalBufferChecker : public Checker {
+class DanglingInternalBufferChecker : public Checker {
   CallDescription CStrFn;
 
 public:
@@ -36,6 +37,9 @@
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent , CheckerContext ) const;
+
+  /// Clean up the ProgramState map.
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 };
 
 } // end anonymous namespace
@@ -76,12 +80,32 @@
   // FIXME: What if Origin is null?
   const Expr *Origin = Call.getOriginExpr();
   State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
+  State = State->remove(TypedR);
   C.addTransition(State);
   return;
 }
   }
 }
 
+void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper ,
+ CheckerContext ) const {
+  if (!SymReaper.hasDeadSymbols())
+return;
+
+  ProgramStateRef State = C.getState();
+  RawPtrMapTy RPM = State->get();
+  for (const auto Entry : RPM) {
+if (SymReaper.isDead(Entry.second))
+  State = State->remove(Entry.first);
+if (!SymReaper.isLiveRegion(Entry.first))
+  // Due to incomplete destructor support, some dead regions might still
+  // remain in the program state map. Clean them up.
+  State = State->remove(Entry.first);
+  }
+
+  C.addTransition(State);
+}
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager ) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker();


Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -26,7 +26,8 @@
 
 namespace {
 
-class DanglingInternalBufferChecker : public Checker {
+class DanglingInternalBufferChecker : public Checker {
   CallDescription CStrFn;
 
 public:
@@ -36,6 +37,9 @@
   /// corresponding string object region in the ProgramState. Mark the symbol
   /// released if the string object is destroyed.
   void checkPostCall(const CallEvent , CheckerContext ) const;
+
+  /// Clean up the ProgramState map.
+  void checkDeadSymbols(SymbolReaper , CheckerContext ) const;
 };
 
 } // end anonymous namespace
@@ -76,12 +80,32 @@
   // FIXME: What if Origin is null?
   const Expr *Origin = Call.getOriginExpr();
   State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
+  State = State->remove(TypedR);
   C.addTransition(State);
   return;
 }
   }
 }
 
+void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper ,
+ CheckerContext ) const {
+  if (!SymReaper.hasDeadSymbols())
+return;
+
+  ProgramStateRef State = C.getState();
+  RawPtrMapTy RPM = State->get();
+  for (const auto Entry : RPM) {
+if (SymReaper.isDead(Entry.second))
+  State = State->remove(Entry.first);
+if (!SymReaper.isLiveRegion(Entry.first))
+  // Due to incomplete destructor support, some dead regions might still
+  // remain in the program state map. Clean them up.
+  State = State->remove(Entry.first);
+  }
+
+  C.addTransition(State);
+}
+
 void ento::registerDanglingInternalBufferChecker(CheckerManager ) {
   registerNewDeleteChecker(Mgr);
   Mgr.registerChecker();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47693: [X86] Use target independent masked expandload and compressstore intrinsics to implement expandload/compressstore builtins.

2018-06-09 Thread Simon Pilgrim via Phabricator via cfe-commits 
RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

We're going to need more codegen tests on the llvm side - test coverage 
(fast-isel and intrinsics) isn't great. But LGTM on the clang side.


Repository:
  rC Clang

https://reviews.llvm.org/D47693



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

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

2018-06-09 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334348: [analyzer] Add dangling internal buffer check. 
(authored by rkovacs, committed by ).
Herald added subscribers: llvm-commits, mikhail.ramalho.

Changed prior to commit:
  https://reviews.llvm.org/D47135?vs=148827=150623#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47135

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
  cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/test/Analysis/dangling-internal-buffer.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -300,6 +300,11 @@
 
 let ParentPackage = CplusplusAlpha in {
 
+def DanglingInternalBufferChecker : Checker<"DanglingInternalBuffer">,
+  HelpText<"Check for internal raw pointers of C++ standard library containers "
+   "used after deallocation">,
+  DescFile<"DanglingInternalBufferChecker.cpp">;
+
 def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">,
   HelpText<"Reports destructions of polymorphic objects with a non-virtual "
"destructor in their base class">,
Index: cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
===
--- cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
+++ cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,71 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -analyzer-output=text -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+  ~basic_string();
+  const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+  const char *c;
+  {
+std::string s;
+c = s.c_str();
+  }
+  consume(c); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_wchar_t() {
+  const wchar_t *w;
+  {
+std::wstring ws;
+w = ws.c_str();
+  }
+  consume(w); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char16_t() {
+  const char16_t *c16;
+  {
+std::u16string s16;
+c16 = s16.c_str();
+  }
+  consume(c16); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+  const char32_t *c32;
+  {
+std::u32string s32;
+c32 = s32.c_str();
+  }
+  consume(c32); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_ok() {
+  const char *c;
+  std::string s;
+  {
+c = s.c_str();
+  }
+  consume(c); // no-warning
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,88 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines a check that marks a raw pointer to a C++ standard library
+// container's inner buffer released when the object is destroyed. This
+// information can be used by MallocChecker to detect use-after-free problems.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "AllocationState.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class DanglingInternalBufferChecker : public Checker {
+  CallDescription CStrFn;
+
+public:
+

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

2018-06-09 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1127190, @jfb wrote:

> In https://reviews.llvm.org/D47290#1126443, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D47290#1125028, @aaron.ballman wrote:
> >
> > > Okay, that's fair, but the vendor-specific type for my Windows example is 
> > > spelled `DWORD`. I'm really worried that this special case will become a 
> > > precedent and we'll wind up with -Wformat being relaxed for everything 
> > > based on the same rationale. If that's how the community wants -Wformat 
> > > to work, cool, but I'd like to know if we're intending to change (what I 
> > > see as) the design of this warning.
> >
> >
> > I spoke with @jfb offline and am less concerned about this patch now. He's 
> > welcome to correct me if I misrepresent anything, but the precedent this 
> > sets is that a platform "owner" (someone with authority, not Joe Q 
> > Random-User) can relax -Wformat as in this patch, but this is not a 
> > precedent for a blanket change to -Wformat just because the UB happens to 
> > work and we "know" it.
>
>
> Thanks for asking these questions Aaron, it's helped answer everyone's 
> concerns and explain our respective positions. You've certainly summarized 
> what I was thinking.


Excellent!

> It sounds like you're both OK moving forward with this patch?

I am okay moving forward.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47979: [X86] Lowering Mask Scalar add/sub/mul/div intrinsics to native IR (Clang part)

2018-06-09 Thread Tomasz Krupa via Phabricator via cfe-commits 
tkrupa created this revision.
tkrupa added reviewers: craig.topper, RKSimon, spatel, sroland.
Herald added a subscriber: cfe-commits.

I did the div differently because it got split to three basic blocks with
a branch condition (due to div being an expensive operation)
and couldn't be combined back.

Corresponding LLVM revision: https://reviews.llvm.org/D47978


Repository:
  rC Clang

https://reviews.llvm.org/D47979

Files:
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/avx512fintrin.h
  test/CodeGen/avx512f-builtins.c

Index: test/CodeGen/avx512f-builtins.c
===
--- test/CodeGen/avx512f-builtins.c
+++ test/CodeGen/avx512f-builtins.c
@@ -2298,12 +2298,29 @@
 }
 __m128 test_mm_mask_add_ss(__m128 __W, __mmask8 __U, __m128 __A, __m128 __B) {
   // CHECK-LABEL: @test_mm_mask_add_ss
-  // CHECK: @llvm.x86.avx512.mask.add.ss.round
+  // CHECK-NOT: @llvm.x86.avx512.mask.add.ss.round
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: fadd float %{{.*}}, %{{.*}}
+  // CHECK: insertelement <4 x float> %{{.*}}, i32 0
+  // CHECK: and i32 {{.*}}, 1
+  // CHECK: icmp ne i32 %{{.*}}, 0
+  // CHECK: br {{.*}}, {{.*}}, {{.*}}
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: insertelement <4 x float> %{{.*}}, float %{{.*}}, i32 0
   return _mm_mask_add_ss(__W,__U,__A,__B); 
 }
 __m128 test_mm_maskz_add_ss(__mmask8 __U, __m128 __A, __m128 __B) {
   // CHECK-LABEL: @test_mm_maskz_add_ss
-  // CHECK: @llvm.x86.avx512.mask.add.ss.round
+  // CHECK-NOT: @llvm.x86.avx512.mask.add.ss.round
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: fadd float %{{.*}}, %{{.*}}
+  // CHECK: insertelement <4 x float> %{{.*}}, i32 0
+  // CHECK: and i32 {{.*}}, 1
+  // CHECK: icmp ne i32 %{{.*}}, 0
+  // CHECK: br {{.*}}, {{.*}}, {{.*}}
+  // CHECK: insertelement <4 x float> %{{.*}}, float %{{.*}}, i32 0
   return _mm_maskz_add_ss(__U,__A,__B); 
 }
 __m128d test_mm_add_round_sd(__m128d __A, __m128d __B) {
@@ -2323,12 +2340,29 @@
 }
 __m128d test_mm_mask_add_sd(__m128d __W, __mmask8 __U, __m128d __A, __m128d __B) {
   // CHECK-LABEL: @test_mm_mask_add_sd
-  // CHECK: @llvm.x86.avx512.mask.add.sd.round
+  // CHECK-NOT: @llvm.x86.avx512.mask.add.sd.round
+  // CHECK: extractelement <2 x double> %{{.*}}, i32 0
+  // CHECK: extractelement <2 x double> %{{.*}}, i32 0
+  // CHECK: fadd double %{{.*}}, %{{.*}}
+  // CHECK: insertelement <2 x double> {{.*}}, i32 0
+  // CHECK: and i32 {{.*}}, 1
+  // CHECK: icmp ne i32 %{{.*}}, 0
+  // CHECK: br {{.*}}, {{.*}}, {{.*}}
+  // CHECK: extractelement <2 x double> %{{.*}}, i32 0
+  // CHECK: insertelement <2 x double> %{{.*}}, double %{{.*}}, i32 0
   return _mm_mask_add_sd(__W,__U,__A,__B); 
 }
 __m128d test_mm_maskz_add_sd(__mmask8 __U, __m128d __A, __m128d __B) {
   // CHECK-LABEL: @test_mm_maskz_add_sd
-  // CHECK: @llvm.x86.avx512.mask.add.sd.round
+  // CHECK-NOT: @llvm.x86.avx512.mask.add.sd.round
+  // CHECK: extractelement <2 x double> %{{.*}}, i32 0
+  // CHECK: extractelement <2 x double> %{{.*}}, i32 0
+  // CHECK: fadd double %{{.*}}, %{{.*}}
+  // CHECK: insertelement <2 x double> {{.*}}, i32 0
+  // CHECK: and i32 {{.*}}, 1
+  // CHECK: icmp ne i32 %{{.*}}, 0
+  // CHECK: br {{.*}}, {{.*}}, {{.*}}
+  // CHECK: insertelement <2 x double> %{{.*}}, double %{{.*}}, i32 0
   return _mm_maskz_add_sd(__U,__A,__B); 
 }
 __m512d test_mm512_sub_round_pd(__m512d __A, __m512d __B) {
@@ -2402,12 +2436,29 @@
 }
 __m128 test_mm_mask_sub_ss(__m128 __W, __mmask8 __U, __m128 __A, __m128 __B) {
   // CHECK-LABEL: @test_mm_mask_sub_ss
-  // CHECK: @llvm.x86.avx512.mask.sub.ss.round
+  // CHECK-NOT: @llvm.x86.avx512.mask.sub.ss.round
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: fsub float %{{.*}}, %{{.*}}
+  // CHECK: insertelement <4 x float> {{.*}}, i32 0
+  // CHECK: and i32 {{.*}}, 1
+  // CHECK: icmp ne i32 %{{.*}}, 0
+  // CHECK: br {{.*}}, {{.*}}, {{.*}}
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: insertelement <4 x float> %{{.*}}, float %{{.*}}, i32 0
   return _mm_mask_sub_ss(__W,__U,__A,__B); 
 }
 __m128 test_mm_maskz_sub_ss(__mmask8 __U, __m128 __A, __m128 __B) {
   // CHECK-LABEL: @test_mm_maskz_sub_ss
-  // CHECK: @llvm.x86.avx512.mask.sub.ss.round
+  // CHECK-NOT: @llvm.x86.avx512.mask.sub.ss.round
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: extractelement <4 x float> %{{.*}}, i32 0
+  // CHECK: fsub float %{{.*}}, %{{.*}}
+  // CHECK: insertelement <4 x float> {{.*}}, i32 0
+  // CHECK: and i32 {{.*}}, 1
+  // CHECK: icmp ne i32 %{{.*}}, 0
+  // CHECK: br {{.*}}, {{.*}}, {{.*}}
+  // CHECK: insertelement <4 x float> %{{.*}}, float %{{.*}}, i32 0
   return _mm_maskz_sub_ss(__U,__A,__B); 
 }
 __m128d test_mm_sub_round_sd(__m128d __A, __m128d __B) {
@@

[PATCH] D47044: [analyzer] Ensure that we only visit a destructor for a reference if type information is available.

2018-06-09 Thread Henry Wong via Phabricator via cfe-commits 
MTC added a comment.

LGTM, @NoQ May have further feedback. Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D47044



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

2018-06-09 Thread JF Bastien via Phabricator via cfe-commits 
jfb added a comment.

In https://reviews.llvm.org/D47290#1126443, @aaron.ballman wrote:

> In https://reviews.llvm.org/D47290#1125028, @aaron.ballman wrote:
>
> > Okay, that's fair, but the vendor-specific type for my Windows example is 
> > spelled `DWORD`. I'm really worried that this special case will become a 
> > precedent and we'll wind up with -Wformat being relaxed for everything 
> > based on the same rationale. If that's how the community wants -Wformat to 
> > work, cool, but I'd like to know if we're intending to change (what I see 
> > as) the design of this warning.
>
>
> I spoke with @jfb offline and am less concerned about this patch now. He's 
> welcome to correct me if I misrepresent anything, but the precedent this sets 
> is that a platform "owner" (someone with authority, not Joe Q Random-User) 
> can relax -Wformat as in this patch, but this is not a precedent for a 
> blanket change to -Wformat just because the UB happens to work and we "know" 
> it.


Thanks for asking these questions Aaron, it's helped answer everyone's concerns 
and explain our respective positions. You've certainly summarized what I was 
thinking.

It sounds like you're both OK moving forward with this patch?


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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