[PATCH] D44559: [Sema] Wrong width of result of mul operation

2018-03-30 Thread Andrew V. Tischenko via Phabricator via cfe-commits
avt77 added a comment.

In fact I think about mul only because of X86 nature: it always put the mul 
result in wider place (than its args). (Don't know about other CPUs - maybe the 
same?) As result we could change the current design to reflect this feature:

8bit x 8bit -> 16bit
16bit x 16bit -> 32bit
...


https://reviews.llvm.org/D44559



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


[PATCH] D43967: [ASTImporter] Add test helper Fixture

2018-03-30 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

Ping.

Could someone with commit rights please commit this? @szepet or @xazax.hun ?


Repository:
  rC Clang

https://reviews.llvm.org/D43967



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


[PATCH] D44786: Lowering x86 adds/addus/subs/subus intrinsics (clang)

2018-03-30 Thread Tomasz Krupa via Phabricator via cfe-commits
tkrupa updated this revision to Diff 140401.

Repository:
  rC Clang

https://reviews.llvm.org/D44786

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/avx2-builtins.c
  test/CodeGen/avx512bw-builtins.c
  test/CodeGen/avx512vlbw-builtins.c
  test/CodeGen/sse2-builtins.c

Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -47,25 +47,53 @@
 
 __m128i test_mm_adds_epi8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epi8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.padds.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.padds.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: add <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: icmp sle <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: icmp slt <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> %{{.*}}
+  // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
   return _mm_adds_epi8(A, B);
 }
 
 __m128i test_mm_adds_epi16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epi16
-  // CHECK: call <8 x i16> @llvm.x86.sse2.padds.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.padds.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: add <8 x i32> %{{.*}}, %{{.*}}
+  // CHECK: icmp sle <8 x i32> %{{.*}}, 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: icmp slt <8 x i32> %{{.*}}, 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> , <8 x i32> %{{.*}}
+  // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
   return _mm_adds_epi16(A, B);
 }
 
 __m128i test_mm_adds_epu8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epu8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.paddus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: zext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: zext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: add <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: icmp ule <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
   return _mm_adds_epu8(A, B);
 }
 
 __m128i test_mm_adds_epu16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_adds_epu16
-  // CHECK: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.paddus.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: zext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: zext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: add <8 x i32> %{{.*}}, %{{.*}}
+  // CHECK: icmp ule <8 x i32> %{{.*}}, 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
   return _mm_adds_epu16(A, B);
 }
 
@@ -1414,25 +1442,47 @@
 
 __m128i test_mm_subs_epi8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_subs_epi8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.psubs.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubs.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: sext <16 x i8> %{{.*}} to <16 x i16>
+  // CHECK: sub <16 x i16> %{{.*}}, %{{.*}}
+  // CHECK: icmp sle <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> 
+  // CHECK: icmp slt <16 x i16> %{{.*}}, 
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> , <16 x i16> %{{.*}}
+  // CHECK: trunc <16 x i16> %{{.*}} to <16 x i8>
   return _mm_subs_epi8(A, B);
 }
 
 __m128i test_mm_subs_epi16(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_subs_epi16
-  // CHECK: call <8 x i16> @llvm.x86.sse2.psubs.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK-NOT: call <8 x i16> @llvm.x86.sse2.psubs.w(<8 x i16> %{{.*}}, <8 x i16> %{{.*}})
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: sext <8 x i16> %{{.*}} to <8 x i32>
+  // CHECK: sub <8 x i32> %{{.*}}, %{{.*}}
+  // CHECK: icmp sle <8 x i32> %{{.*}}, 
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32> %{{.*}}, <8 x i32> 
+  // CHECK: icmp slt <8 x i32> %{{.*}},  
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i32>  , <8 x i32> %{{.*}}
+  // CHECK: trunc <8 x i32> %{{.*}} to <8 x i16>
   return _mm_subs_epi16(A, B);
 }
 
 __m128i test_mm_subs_epu8(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_subs_epu8
-  // CHECK: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK-NOT: call <16 x i8> @llvm.x86.sse2.psubus.b(<16 x i8> %{{.*}}, <16 x i8> %{{.*}})
+  // CHECK: icmp ugt <16 x i8> {{.*}}, {{.*}}
+  // CHECK: select <16 x i1> {{.*}}, <16 x i8> {{.*}}

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

2018-03-30 Thread Henry Wong via Phabricator via cfe-commits
MTC added a comment.

Thanks for your review, NoQ!

> Why do you need separate code for null and non-null character? The function's 
> semantics doesn't seem to care.

Thank you for your advice, I will remove the duplicate code!

> I'd rather consider the case of non-concrete character separately. Because 
> wiping a region with a symbol is not something we currently support; a 
> symbolic default binding over a region means a different thing and it'd be 
> equivalent to invalidation, so for non-concrete character we don't have a 
> better choice than to invalidate. For concrete non-zero character, on the 
> contrary, a default binding would work just fine.

Thank you for your reminding, I overlooked this point. However for non-concrete 
character, the symbol value, if we just invalidate the region, the constraint 
information of the non-concrete character will be lost. Do we need to consider 
this?

> Could you explain why didn't a straightforward `bindLoc` over a base region 
> wasn't doing the thing you wanted? I.e., why is new Store API function 
> necessary?

I am also very resistant to adding new APIs. One is that I am not very familiar 
with RegionStore. One is that adding a new API is always the last option.

- The semantics of `memset()` need default binding, and only `bindDefault()` 
can guarantee that. However `bindDefault()` is just used to initialize the 
region and can only be called once on the same region.
- Only when the region is `TypedValueRegion` and the value type is `ArrayType` 
or `StructType`, `bindLoc()` can add a default binding. So if we want to 
continue using `default binding`, `bindLoc()` is not the correct choice.

And if I use `bindLoc()` instead of `overwriteRegion()`, there will be some 
crashes occured because `bindLoc()` broke some assumptions, e.g. `bindArray()` 
believes that the value for binding should be `CompoundVal`, `LazyCompoundVal` 
or `UnknownVal`.


Repository:
  rC Clang

https://reviews.llvm.org/D44934



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


[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

2018-03-30 Thread Henry Wong via Phabricator via cfe-commits
MTC updated this revision to Diff 140405.
MTC added a comment.

According to @NoQ's suggestion, remove the duplicated code.


Repository:
  rC Clang

https://reviews.llvm.org/D44934

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/bstring.cpp
  test/Analysis/null-deref-ps-region.c
  test/Analysis/string.c

Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -1,7 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
-// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=alpha.security.taint,core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DVARIANT -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DUSE_BUILTINS -DVARIANT -analyzer-checker=alpha.security.taint,core,unix.cstring,unix.Malloc,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
+// RUN: %clang_analyze_cc1 -DSUPPRESS_OUT_OF_BOUND -analyzer-checker=core,unix.cstring,unix.Malloc,alpha.unix.cstring.BufferOverlap,alpha.unix.cstring.NotNullTerminated,debug.ExprInspection -analyzer-store=region -Wno-null-dereference -verify %s
 
 //===--===
 // Declarations
@@ -1159,6 +1160,248 @@
   clang_analyzer_eval(str[1] == 'b'); // expected-warning{{UNKNOWN}}
 }
 
+//===--===
+// memset()
+//===--===
+
+void *memset( void *dest, int ch, size_t count );
+
+void* malloc(size_t size);
+void free(void*);
+
+void memset1_char_array_null() {
+  char str[] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  memset(str, '\0', 2);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+}
+
+void memset2_char_array_null() {
+  char str[] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  memset(str, '\0', strlen(str) + 1);
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(str[2] == 0);  // expected-warning{{TRUE}}
+}
+
+void memset3_char_malloc_null() {
+  char *str = (char *)malloc(10 * sizeof(char));
+  memset(str + 1, '\0', 8);
+  clang_analyzer_eval(str[1] == 0); // expected-warning{{UNKNOWN}}
+  free(str);
+}
+
+void memset4_char_malloc_null() {
+  char *str = (char *)malloc(10 * sizeof(char));
+  //void *str = malloc(10 * sizeof(char));
+  memset(str, '\0', 10);
+  clang_analyzer_eval(str[1] == 0);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(str) == 0); // expected-warning{{TRUE}}
+  free(str);
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void memset5_char_malloc_overflow_null() {
+  char *str = (char *)malloc(10 * sizeof(char));
+  memset(str, '\0', 12);
+  clang_analyzer_eval(str[1] == 0); // expected-warning{{UNKNOWN}}
+  free(str);
+}
+#endif
+
+void memset6_char_array_nonnull() {
+  char str[] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  memset(str, '0', 2);
+  clang_analyzer_eval(str[0] == 'a');// expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{UNKNOWN}}
+}
+
+void memset7_char_array_nonnull() {
+  char str[5] = "abcd";
+  clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRUE}}
+  memset(str, '0', 5);
+  clang_analyzer_eval(str[0] == '0');// expected-warning{{TRUE}}
+  clang_analyzer_eval(strlen(str) >= 5); // expected-warning{{TRUE}}
+}
+
+#ifdef SUPPRESS_OUT_OF_BOUND
+void memset8_char_array_nonnull() {
+	char str[5] = "abcd";
+	clang_analyzer_eval(strlen(str) == 4); // expected-warning{{TRU

[PATCH] D45081: [analyzer] Remove the unused method declaration in `ValistChecker.cpp`.

2018-03-30 Thread Henry Wong via Phabricator via cfe-commits
MTC created this revision.
MTC added a reviewer: xazax.hun.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.
Herald added a reviewer: george.karpenkov.

`getVariableNameFromRegion()` seems useless.


Repository:
  rC Clang

https://reviews.llvm.org/D45081

Files:
  lib/StaticAnalyzer/Checkers/ValistChecker.cpp


Index: lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -56,7 +56,6 @@
 private:
   const MemRegion *getVAListAsRegion(SVal SV, const Expr *VAExpr,
  bool &IsSymbolic, CheckerContext &C) 
const;
-  StringRef getVariableNameFromRegion(const MemRegion *Reg) const;
   const ExplodedNode *getStartCallSite(const ExplodedNode *N,
const MemRegion *Reg) const;
 


Index: lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -56,7 +56,6 @@
 private:
   const MemRegion *getVAListAsRegion(SVal SV, const Expr *VAExpr,
  bool &IsSymbolic, CheckerContext &C) const;
-  StringRef getVariableNameFromRegion(const MemRegion *Reg) const;
   const ExplodedNode *getStartCallSite(const ExplodedNode *N,
const MemRegion *Reg) const;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41168: [X86] Lowering X86 avx512 sqrt intrinsics to IR

2018-03-30 Thread Tomasz Krupa via Phabricator via cfe-commits
tkrupa commandeered this revision.
tkrupa added a reviewer: uriel.k.
tkrupa added a comment.

I was assigned to finish this task.


https://reviews.llvm.org/D41168



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


[PATCH] D41168: [X86] Lowering X86 avx512 sqrt intrinsics to IR

2018-03-30 Thread Tomasz Krupa via Phabricator via cfe-commits
tkrupa updated this revision to Diff 140409.
tkrupa added a comment.

Removed renaming of the builtins (also in corresponding llvm patch).


Repository:
  rC Clang

https://reviews.llvm.org/D41168

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/avx-builtins.c
  test/CodeGen/avx512f-builtins.c
  test/CodeGen/avx512vl-builtins.c
  test/CodeGen/sse-builtins.c
  test/CodeGen/sse2-builtins.c

Index: test/CodeGen/sse2-builtins.c
===
--- test/CodeGen/sse2-builtins.c
+++ test/CodeGen/sse2-builtins.c
@@ -1188,7 +1188,7 @@
 
 __m128d test_mm_sqrt_pd(__m128d A) {
   // CHECK-LABEL: test_mm_sqrt_pd
-  // CHECK: call <2 x double> @llvm.x86.sse2.sqrt.pd(<2 x double> %{{.*}})
+  // CHECK: call <2 x double> @llvm.sqrt.v2f64(<2 x double> %{{.*}})
   return _mm_sqrt_pd(A);
 }
 
Index: test/CodeGen/sse-builtins.c
===
--- test/CodeGen/sse-builtins.c
+++ test/CodeGen/sse-builtins.c
@@ -655,7 +655,7 @@
 
 __m128 test_mm_sqrt_ps(__m128 x) {
   // CHECK-LABEL: test_mm_sqrt_ps
-  // CHECK: call <4 x float> @llvm.x86.sse.sqrt.ps(<4 x float> {{.*}})
+  // CHECK: call <4 x float> @llvm.sqrt.v4f32(<4 x float> {{.*}})
   return _mm_sqrt_ps(x);
 }
 
Index: test/CodeGen/avx512vl-builtins.c
===
--- test/CodeGen/avx512vl-builtins.c
+++ test/CodeGen/avx512vl-builtins.c
@@ -3160,49 +3160,49 @@
 }
 __m128d test_mm_mask_sqrt_pd(__m128d __W, __mmask8 __U, __m128d __A) {
   // CHECK-LABEL: @test_mm_mask_sqrt_pd
-  // CHECK: @llvm.x86.sse2.sqrt.pd
+  // CHECK: @llvm.sqrt.v2f64
   // CHECK: select <2 x i1> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}}
   return _mm_mask_sqrt_pd(__W,__U,__A); 
 }
 __m128d test_mm_maskz_sqrt_pd(__mmask8 __U, __m128d __A) {
   // CHECK-LABEL: @test_mm_maskz_sqrt_pd
-  // CHECK: @llvm.x86.sse2.sqrt.pd
+  // CHECK: @llvm.sqrt.v2f64
   // CHECK: select <2 x i1> %{{.*}}, <2 x double> %{{.*}}, <2 x double> %{{.*}}
   return _mm_maskz_sqrt_pd(__U,__A); 
 }
 __m256d test_mm256_mask_sqrt_pd(__m256d __W, __mmask8 __U, __m256d __A) {
   // CHECK-LABEL: @test_mm256_mask_sqrt_pd
-  // CHECK: @llvm.x86.avx.sqrt.pd.256
+  // CHECK: @llvm.sqrt.v4f64
   // CHECK: select <4 x i1> %{{.*}}, <4 x double> %{{.*}}, <4 x double> %{{.*}}
   return _mm256_mask_sqrt_pd(__W,__U,__A); 
 }
 __m256d test_mm256_maskz_sqrt_pd(__mmask8 __U, __m256d __A) {
   // CHECK-LABEL: @test_mm256_maskz_sqrt_pd
-  // CHECK: @llvm.x86.avx.sqrt.pd.256
+  // CHECK: @llvm.sqrt.v4f64
   // CHECK: select <4 x i1> %{{.*}}, <4 x double> %{{.*}}, <4 x double> %{{.*}}
   return _mm256_maskz_sqrt_pd(__U,__A); 
 }
 __m128 test_mm_mask_sqrt_ps(__m128 __W, __mmask8 __U, __m128 __A) {
   // CHECK-LABEL: @test_mm_mask_sqrt_ps
-  // CHECK: @llvm.x86.sse.sqrt.ps
+  // CHECK: @llvm.sqrt.v4f32
   // CHECK: select <4 x i1> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}
   return _mm_mask_sqrt_ps(__W,__U,__A); 
 }
 __m128 test_mm_maskz_sqrt_ps(__mmask8 __U, __m128 __A) {
   // CHECK-LABEL: @test_mm_maskz_sqrt_ps
-  // CHECK: @llvm.x86.sse.sqrt.ps
+  // CHECK: @llvm.sqrt.v4f32
   // CHECK: select <4 x i1> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}
   return _mm_maskz_sqrt_ps(__U,__A); 
 }
 __m256 test_mm256_mask_sqrt_ps(__m256 __W, __mmask8 __U, __m256 __A) {
   // CHECK-LABEL: @test_mm256_mask_sqrt_ps
-  // CHECK: @llvm.x86.avx.sqrt.ps.256
+  // CHECK: @llvm.sqrt.v8f32
   // CHECK: select <8 x i1> %{{.*}}, <8 x float> %{{.*}}, <8 x float> %{{.*}}
   return _mm256_mask_sqrt_ps(__W,__U,__A); 
 }
 __m256 test_mm256_maskz_sqrt_ps(__mmask8 __U, __m256 __A) {
   // CHECK-LABEL: @test_mm256_maskz_sqrt_ps
-  // CHECK: @llvm.x86.avx.sqrt.ps.256
+  // CHECK: @llvm.sqrt.v8f32
   // CHECK: select <8 x i1> %{{.*}}, <8 x float> %{{.*}}, <8 x float> %{{.*}}
   return _mm256_maskz_sqrt_ps(__U,__A); 
 }
Index: test/CodeGen/avx512f-builtins.c
===
--- test/CodeGen/avx512f-builtins.c
+++ test/CodeGen/avx512f-builtins.c
@@ -5,84 +5,100 @@
 __m512d test_mm512_sqrt_pd(__m512d a)
 {
   // CHECK-LABEL: @test_mm512_sqrt_pd
-  // CHECK: @llvm.x86.avx512.mask.sqrt.pd.512
+  // CHECK: @llvm.sqrt.v8f64
   return _mm512_sqrt_pd(a);
 }
 
 __m512d test_mm512_mask_sqrt_pd (__m512d __W, __mmask8 __U, __m512d __A)
 {
   // CHECK-LABEL: @test_mm512_mask_sqrt_pd 
-  // CHECK: @llvm.x86.avx512.mask.sqrt.pd.512
+  // CHECK: @llvm.sqrt.v8f64
+  // CHECK: bitcast
+  // CHECK: select
   return _mm512_mask_sqrt_pd (__W,__U,__A);
 }
 
 __m512d test_mm512_maskz_sqrt_pd (__mmask8 __U, __m512d __A)
 {
   // CHECK-LABEL: @test_mm512_maskz_sqrt_pd 
-  // CHECK: @llvm.x86.avx512.mask.sqrt.pd.512
+  // CHECK: @llvm.sqrt.v8f64
+  // CHECK: bitcast
+  // CHECK: select
   return _mm512_maskz_sqrt_pd (__U,__A);
 }
 
 __m512d test_mm512_mask_sqrt_round_pd(__m512d __W,__mmask8 __U,__m512d __A)
 {
   // CHECK-LABEL: @test_mm512_mask_sqrt_round_pd
-  // CHECK: @llvm.x86.a

[PATCH] D45081: [analyzer] Remove the unused method declaration in `ValistChecker.cpp`.

2018-03-30 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

LG! Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D45081



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


[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section

2018-03-30 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo updated this revision to Diff 140410.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.
Herald added a subscriber: JDevlieghere.

Added and using a helper function `isCIE` to avoid using the error return path 
from `decodeFDE`.

Technically, the decision between a full section and a plain FDE probably 
should be outside of `_unw_add_dynamic_fde`, in `__register_frame` in 
`UnwindLevel1-gcc-ext.c`. That'd require adding a C wrapper for `isCIE` in 
`libunwind.cpp` though.


https://reviews.llvm.org/D44494

Files:
  src/DwarfParser.hpp
  src/UnwindCursor.hpp
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -326,20 +326,27 @@
 
 /// IPI: for __register_frame()
 void _unw_add_dynamic_fde(unw_word_t fde) {
-  CFI_Parser::FDE_Info fdeInfo;
-  CFI_Parser::CIE_Info cieInfo;
-  const char *message = CFI_Parser::decodeFDE(
-   LocalAddressSpace::sThisAddressSpace,
-  (LocalAddressSpace::pint_t) fde, &fdeInfo, &cieInfo);
-  if (message == NULL) {
-// dynamically registered FDEs don't have a mach_header group they are in.
-// Use fde as mh_group
-unw_word_t mh_group = fdeInfo.fdeStart;
-DwarfFDECache::add((LocalAddressSpace::pint_t)mh_group,
-  fdeInfo.pcStart, fdeInfo.pcEnd,
-  fdeInfo.fdeStart);
+  if (CFI_Parser::isCIE(LocalAddressSpace::sThisAddressSpace,
+  (LocalAddressSpace::pint_t) fde)) {
+// This wasn't a plain FDE, but it started with a CIE - register a full
+// .eh_frame section.
+DwarfFDECache::addSection((LocalAddressSpace::pint_t)fde);
   } else {
-_LIBUNWIND_DEBUG_LOG("_unw_add_dynamic_fde: bad fde: %s", message);
+CFI_Parser::FDE_Info fdeInfo;
+CFI_Parser::CIE_Info cieInfo;
+const char *message = CFI_Parser::decodeFDE(
+ LocalAddressSpace::sThisAddressSpace,
+(LocalAddressSpace::pint_t) fde, &fdeInfo, &cieInfo);
+if (message == NULL) {
+  // dynamically registered FDEs don't have a mach_header group they are in.
+  // Use fde as mh_group
+  unw_word_t mh_group = fdeInfo.fdeStart;
+  DwarfFDECache::add((LocalAddressSpace::pint_t)mh_group,
+fdeInfo.pcStart, fdeInfo.pcEnd,
+fdeInfo.fdeStart);
+} else {
+  _LIBUNWIND_DEBUG_LOG("_unw_add_dynamic_fde: bad fde: %s", message);
+}
   }
 }
 
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -44,10 +44,12 @@
 public:
   static pint_t findFDE(pint_t mh, pint_t pc);
   static void add(pint_t mh, pint_t ip_start, pint_t ip_end, pint_t fde);
+  static void addSection(pint_t section_start);
   static void removeAllIn(pint_t mh);
   static void iterateCacheEntries(void (*func)(unw_word_t ip_start,
unw_word_t ip_end,
unw_word_t fde, unw_word_t mh));
+  template static void iterateSections(Func func);
 
 private:
 
@@ -70,6 +72,11 @@
   static entry *_bufferUsed;
   static entry *_bufferEnd;
   static entry _initialBuffer[64];
+
+  static pint_t *_sections;
+  static pint_t *_sectionsUsed;
+  static pint_t *_sectionsEnd;
+  static pint_t _initialSectionsBuffer[64];
 };
 
 template 
@@ -88,6 +95,21 @@
 typename DwarfFDECache::entry DwarfFDECache::_initialBuffer[64];
 
 template 
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sections = _initialSectionsBuffer;
+
+template 
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sectionsUsed = _initialSectionsBuffer;
+
+template 
+typename DwarfFDECache::pint_t *
+DwarfFDECache::_sectionsEnd = &_initialSectionsBuffer[64];
+
+template 
+typename DwarfFDECache::pint_t DwarfFDECache::_initialSectionsBuffer[64];
+
+template 
 RWMutex DwarfFDECache::_lock;
 
 #ifdef __APPLE__
@@ -144,6 +166,28 @@
 }
 
 template 
+void DwarfFDECache::addSection(pint_t section_start) {
+#if !defined(_LIBUNWIND_NO_HEAP)
+  _LIBUNWIND_LOG_IF_FALSE(_lock.lock());
+  if (_sectionsUsed >= _sectionsEnd) {
+size_t oldSize = (size_t)(_sectionsEnd - _sections);
+size_t newSize = oldSize * 4;
+// Can't use operator new (we are below it).
+pint_t *newSections = (pint_t *)malloc(newSize * sizeof(pint_t));
+memcpy(newSections, _sections, oldSize * sizeof(pint_t));
+if (_sections != _initialSectionsBuffer)
+  free(_sections);
+_sections = newSections;
+_sectionsUsed = &newSections[oldSize];
+_sectionsEnd = &newSections[newSize];
+  }
+  *_sectionsUsed = section_start;
+  ++_sectionsUsed;
+  _LIBUNWIND_LOG_IF_FALSE(_lock.unlock());
+#endif
+}
+
+template 
 void DwarfFDECache::removeAllIn(pint_t mh) {
   _LIBUNWIND_LOG_IF_FALSE(

[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-03-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 140411.
lebedev.ri added a comment.

- Rebased
- Created https://reviews.llvm.org/D45082 with llvm diff to prevent stage2 
failure, and to disscuss the options. Similar diff will be needed at least for 
libc++ tests.


Repository:
  rC Clang

https://reviews.llvm.org/D44883

Files:
  docs/ReleaseNotes.rst
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/implicit-exception-spec.cpp
  test/SemaCXX/member-init.cpp
  test/SemaCXX/warn-self-assign-builtin.cpp
  test/SemaCXX/warn-self-assign-field-builtin.cpp
  test/SemaCXX/warn-self-assign-field-overloaded.cpp
  test/SemaCXX/warn-self-assign-overloaded.cpp
  test/SemaCXX/warn-self-assign.cpp

Index: test/SemaCXX/warn-self-assign-overloaded.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-overloaded.cpp
@@ -0,0 +1,96 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DDUMMY -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV1 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV2 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV3 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV4 -verify %s
+
+#ifdef DUMMY
+struct S {};
+#else
+struct S {
+#if defined(V0)
+  S() = default;
+#elif defined(V1)
+  S &operator=(const S &) = default;
+#elif defined(V2)
+  S &operator=(S &) = default;
+#elif defined(V3)
+  S &operator=(const S &);
+#elif defined(V4)
+  S &operator=(S &);
+#else
+#error Define something!
+#endif
+  S &operator*=(const S &);
+  S &operator/=(const S &);
+  S &operator%=(const S &);
+  S &operator+=(const S &);
+  S &operator-=(const S &);
+  S &operator<<=(const S &);
+  S &operator>>=(const S &);
+  S &operator&=(const S &);
+  S &operator|=(const S &);
+  S &operator^=(const S &);
+  S &operator=(const volatile S &) volatile;
+};
+#endif
+
+void f() {
+  S a, b;
+  a = a; // expected-warning{{explicitly assigning}}
+  b = b; // expected-warning{{explicitly assigning}}
+  a = b;
+  b = a = b;
+  a = a = a; // expected-warning{{explicitly assigning}}
+  a = b = b = a;
+
+#ifndef DUMMY
+  a *= a;
+  a /= a; // expected-warning {{explicitly assigning}}
+  a %= a; // expected-warning {{explicitly assigning}}
+  a += a;
+  a -= a; // expected-warning {{explicitly assigning}}
+  a <<= a;
+  a >>= a;
+  a &= a; // expected-warning {{explicitly assigning}}
+  a |= a; // expected-warning {{explicitly assigning}}
+  a ^= a; // expected-warning {{explicitly assigning}}
+#endif
+}
+
+void false_positives() {
+#define OP =
+#define LHS a
+#define RHS a
+  S a;
+  // These shouldn't warn due to the use of the preprocessor.
+  a OP a;
+  LHS = a;
+  a = RHS;
+  LHS OP RHS;
+#undef OP
+#undef LHS
+#undef RHS
+
+  // A way to silence the warning.
+  a = (S &)a;
+
+#ifndef DUMMY
+  // Volatile stores aren't side-effect free.
+  volatile S vol_a;
+  vol_a = vol_a;
+  volatile S &vol_a_ref = vol_a;
+  vol_a_ref = vol_a_ref;
+#endif
+}
+
+template 
+void g() {
+  T a;
+  a = a; // expected-warning{{explicitly assigning}}
+}
+void instantiate() {
+  g();
+  g();
+}
Index: test/SemaCXX/warn-self-assign-field-overloaded.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-overloaded.cpp
@@ -0,0 +1,145 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DDUMMY -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV1 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV2 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV3 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV4 -verify %s
+
+#ifdef DUMMY
+struct S {};
+#else
+struct S {
+#if defined(V0)
+  S() = default;
+#elif defined(V1)
+  S &operator=(const S &) = default;
+#elif defined(V2)
+  S &operator=(S &) = default;
+#elif defined(V3)
+  S &operator=(const S &);
+#elif defined(V4)
+  S &operator=(S &);
+#else
+#error Define something!
+#endif
+  S &operator*=(const S &);
+  S &operator/=(const S &);
+  S &operator%=(const S &);
+  S &operator+=(const S &);
+  S &operator-=(const S &);
+  S &operator<<=(const S &);
+  S &operator>>=(const S &);
+  S &operator&=(const S &);
+  S &operator|=(const S &);
+  S &operator^=(const S &);
+  S &operator=(const volatile S &) volatile;
+};
+#endif
+struct C {
+  S a;
+  S b;
+
+  void f() {
+a = a; // expected-warning {{assigning field to itself}}
+b = b; // expected-warning {{assigning field to itself}}
+a = b;
+
+this->a = a;   // expected-warning {{assigning field to itself}}
+this->b = b;   // expected-warning {{assigning field to itself}}
+a = this->a;   // expected-warning {{assigning field to itself}}
+b = this->b;   // expected-warning {{assigning field to itself}}
+this->a = this->a; // expected-warning {{assi

[PATCH] D45045: [DebugInfo] Generate debug information about labels

2018-03-30 Thread Wei-Ren Chen via Phabricator via cfe-commits
chenwj requested changes to this revision.
chenwj added a comment.
This revision now requires changes to proceed.

Nits.




Comment at: lib/CodeGen/CGDebugInfo.cpp:3644
+void CGDebugInfo::EmitLabel(const LabelDecl *D,
+   CGBuilderTy &Builder) {
+  assert(DebugKind >= codegenoptions::LimitedDebugInfo);

Indent.



Comment at: lib/CodeGen/CGStmt.cpp:535
+
+  // Emit debug info for label.
+  if (HaveInsertPoint())

I assume you emit debug info for the label only if it's reachable by checking 
`HaveInsertPoint()`. If so, make the comment as


```
// Emit debug info for the label only if it's reachable.
```

I prefer adding braces here. Please check indent as well.


Repository:
  rC Clang

https://reviews.llvm.org/D45045



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


[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

2018-03-30 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D44602#1047574, @lebedev.ri wrote:

> - No longer count variables declared in macro expansion. Please see FIXME, i 
> think this is too broad.


Ping, thoughts?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



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


[PATCH] D45086: [analyzer] Unroll the loop when it has a unsigned counter.

2018-03-30 Thread Henry Wong via Phabricator via cfe-commits
MTC created this revision.
MTC added reviewers: szepet, a.sidorin, NoQ.
Herald added subscribers: cfe-commits, rnkovacs, xazax.hun.
Herald added a reviewer: george.karpenkov.
MTC edited the summary of this revision.

The original implementation in the `LoopUnrolling.cpp` didn't consider the case 
where the counter is unsigned. This case is only handled in 
`simpleCondition()`, but this is not enough, we also need to deal with the 
unsinged counter with the counter initialization.

Since `IntegerLiteral` is `signed`, there is a `ImplicitCastExpr` 
in `unsigned counter = IntergerLiteral`. This patch add the 
`ignoringParenImpCasts()` in the `IntegerLiteral` matcher.


Repository:
  rC Clang

https://reviews.llvm.org/D45086

Files:
  lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  test/Analysis/loop-unrolling.cpp


Index: test/Analysis/loop-unrolling.cpp
===
--- test/Analysis/loop-unrolling.cpp
+++ test/Analysis/loop-unrolling.cpp
@@ -36,6 +36,29 @@
   return 0;
 }
 
+int simple_unroll3_unsinged() {
+  int a[9];
+  int k = 42;
+  for (unsigned i = 0; i < 9; i++) {
+clang_analyzer_numTimesReached(); // expected-warning {{9}}
+a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
+int simple_unroll4_unsinged() {
+  int a[9];
+  int k = 42;
+  unsigned i;
+  for (i = (0); i < 9; i++) {
+clang_analyzer_numTimesReached(); // expected-warning {{9}}
+a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
 int simple_no_unroll1() {
   int a[9];
   int k = 42;
Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -141,13 +141,15 @@
   return forStmt(
  hasCondition(simpleCondition("initVarName")),
  // Initialization should match the form: 'int i = 6' or 'i = 42'.
- hasLoopInit(anyOf(
- declStmt(hasSingleDecl(varDecl(
- allOf(hasInitializer(integerLiteral().bind("initNum")),
-   equalsBoundNode("initVarName"),
- binaryOperator(hasLHS(declRefExpr(to(
-varDecl(equalsBoundNode("initVarName"),
-hasRHS(integerLiteral().bind("initNum"),
+ hasLoopInit(
+ anyOf(declStmt(hasSingleDecl(
+   varDecl(allOf(hasInitializer(ignoringParenImpCasts(
+ 
integerLiteral().bind("initNum"))),
+ equalsBoundNode("initVarName"),
+   binaryOperator(hasLHS(declRefExpr(to(varDecl(
+  equalsBoundNode("initVarName"),
+  hasRHS(ignoringParenImpCasts(
+  
integerLiteral().bind("initNum")),
  // Incrementation should be a simple increment or decrement
  // operator call.
  hasIncrement(unaryOperator(


Index: test/Analysis/loop-unrolling.cpp
===
--- test/Analysis/loop-unrolling.cpp
+++ test/Analysis/loop-unrolling.cpp
@@ -36,6 +36,29 @@
   return 0;
 }
 
+int simple_unroll3_unsinged() {
+  int a[9];
+  int k = 42;
+  for (unsigned i = 0; i < 9; i++) {
+clang_analyzer_numTimesReached(); // expected-warning {{9}}
+a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
+int simple_unroll4_unsinged() {
+  int a[9];
+  int k = 42;
+  unsigned i;
+  for (i = (0); i < 9; i++) {
+clang_analyzer_numTimesReached(); // expected-warning {{9}}
+a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
 int simple_no_unroll1() {
   int a[9];
   int k = 42;
Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -141,13 +141,15 @@
   return forStmt(
  hasCondition(simpleCondition("initVarName")),
  // Initialization should match the form: 'int i = 6' or 'i = 42'.
- hasLoopInit(anyOf(
- declStmt(hasSingleDecl(varDecl(
- allOf(hasInitializer(integerLiteral().bind("initNum")),
-   equalsBoundNode("initVarName"),
- binaryOperator(hasLHS(declRefExpr(to(
-varDecl(equalsBoundNode("initVarName"),
-hasRHS(integerLiteral().bind("initNum"),
+ hasLoopInit(
+ anyOf(declStmt(hasSingleDecl(
+   varDe

r328860 - [analyzer] Remove the unused method declaration in `ValistChecker.cpp`.

2018-03-30 Thread Henry Wong via cfe-commits
Author: henrywong
Date: Fri Mar 30 06:37:50 2018
New Revision: 328860

URL: http://llvm.org/viewvc/llvm-project?rev=328860&view=rev
Log:
[analyzer] Remove the unused method declaration in `ValistChecker.cpp`.

Summary: `getVariableNameFromRegion()` seems useless.

Reviewers: xazax.hun, george.karpenkov

Reviewed By: xazax.hun

Subscribers: szepet, rnkovacs, a.sidorin, cfe-commits, MTC

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp?rev=328860&r1=328859&r2=328860&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp Fri Mar 30 06:37:50 
2018
@@ -56,7 +56,6 @@ public:
 private:
   const MemRegion *getVAListAsRegion(SVal SV, const Expr *VAExpr,
  bool &IsSymbolic, CheckerContext &C) 
const;
-  StringRef getVariableNameFromRegion(const MemRegion *Reg) const;
   const ExplodedNode *getStartCallSite(const ExplodedNode *N,
const MemRegion *Reg) const;
 


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


[PATCH] D45081: [analyzer] Remove the unused method declaration in `ValistChecker.cpp`.

2018-03-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328860: [analyzer] Remove the unused method declaration in 
`ValistChecker.cpp`. (authored by henrywong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45081?vs=140408&id=140419#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45081

Files:
  lib/StaticAnalyzer/Checkers/ValistChecker.cpp


Index: lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -56,7 +56,6 @@
 private:
   const MemRegion *getVAListAsRegion(SVal SV, const Expr *VAExpr,
  bool &IsSymbolic, CheckerContext &C) 
const;
-  StringRef getVariableNameFromRegion(const MemRegion *Reg) const;
   const ExplodedNode *getStartCallSite(const ExplodedNode *N,
const MemRegion *Reg) const;
 


Index: lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -56,7 +56,6 @@
 private:
   const MemRegion *getVAListAsRegion(SVal SV, const Expr *VAExpr,
  bool &IsSymbolic, CheckerContext &C) const;
-  StringRef getVariableNameFromRegion(const MemRegion *Reg) const;
   const ExplodedNode *getStartCallSite(const ExplodedNode *N,
const MemRegion *Reg) const;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45045: [DebugInfo] Generate DILabel metadata for labels.

2018-03-30 Thread Hsiangkai Wang via Phabricator via cfe-commits
HsiangKai updated this revision to Diff 140423.
HsiangKai retitled this revision from "[DebugInfo] Generate debug information 
about labels" to "[DebugInfo] Generate DILabel metadata for labels.".
HsiangKai edited the summary of this revision.
HsiangKai added a comment.

1. No generating llvm.dbg.label intrinsic.
2. Modify commits according to chenwj's comments.


Repository:
  rC Clang

https://reviews.llvm.org/D45045

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGStmt.cpp


Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -531,6 +531,19 @@
   }
 
   EmitBlock(Dest.getBlock());
+
+  // Emit debug info for the label only if it's reachable.
+  if (HaveInsertPoint()) {
+if (CGDebugInfo *DI = getDebugInfo()) {
+  if (CGM.getCodeGenOpts().getDebugInfo() >=
+  codegenoptions::LimitedDebugInfo) {
+DI->setLocation(D->getLocation());
+auto BB = Dest.getBlock();
+DI->EmitLabel(D, BB, Builder);
+  }
+}
+  }
+
   incrementProfileCounter(D->getStmt());
 }
 
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -395,6 +395,9 @@
llvm::Value *AI,
CGBuilderTy &Builder);
 
+  /// Emit call to \c llvm.dbg.label for an label.
+  void EmitLabel(const LabelDecl *D, llvm::BasicBlock *BB, CGBuilderTy 
&Builder);
+
   /// Emit call to \c llvm.dbg.declare for an imported variable
   /// declaration in a block.
   void EmitDeclareOfBlockDeclRefVariable(const VarDecl *variable,
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3640,6 +3640,24 @@
   return EmitDeclare(VD, Storage, llvm::None, Builder);
 }
 
+void CGDebugInfo::EmitLabel(const LabelDecl *D, llvm::BasicBlock *BB,
+CGBuilderTy &Builder) {
+  assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+  assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
+
+  if (D->hasAttr())
+return;
+
+  auto *Scope = cast(LexicalBlockStack.back());
+  StringRef Name = D->getName();
+  llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
+  unsigned Line = getLineNumber(D->getLocation());
+
+  // Create the descriptor for the label.
+  auto *L = DBuilder.createLabel(Scope, Name, Unit, Line);
+  BB->setLabel(L);
+}
+
 llvm::DIType *CGDebugInfo::CreateSelfType(const QualType &QualTy,
   llvm::DIType *Ty) {
   llvm::DIType *CachedTy = getTypeOrNull(QualTy);


Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -531,6 +531,19 @@
   }
 
   EmitBlock(Dest.getBlock());
+
+  // Emit debug info for the label only if it's reachable.
+  if (HaveInsertPoint()) {
+if (CGDebugInfo *DI = getDebugInfo()) {
+  if (CGM.getCodeGenOpts().getDebugInfo() >=
+  codegenoptions::LimitedDebugInfo) {
+DI->setLocation(D->getLocation());
+auto BB = Dest.getBlock();
+DI->EmitLabel(D, BB, Builder);
+  }
+}
+  }
+
   incrementProfileCounter(D->getStmt());
 }
 
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -395,6 +395,9 @@
llvm::Value *AI,
CGBuilderTy &Builder);
 
+  /// Emit call to \c llvm.dbg.label for an label.
+  void EmitLabel(const LabelDecl *D, llvm::BasicBlock *BB, CGBuilderTy &Builder);
+
   /// Emit call to \c llvm.dbg.declare for an imported variable
   /// declaration in a block.
   void EmitDeclareOfBlockDeclRefVariable(const VarDecl *variable,
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3640,6 +3640,24 @@
   return EmitDeclare(VD, Storage, llvm::None, Builder);
 }
 
+void CGDebugInfo::EmitLabel(const LabelDecl *D, llvm::BasicBlock *BB,
+CGBuilderTy &Builder) {
+  assert(DebugKind >= codegenoptions::LimitedDebugInfo);
+  assert(!LexicalBlockStack.empty() && "Region stack mismatch, stack empty!");
+
+  if (D->hasAttr())
+return;
+
+  auto *Scope = cast(LexicalBlockStack.back());
+  StringRef Name = D->getName();
+  llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
+  unsigned Line = getLineNumber(D->getLocation());
+
+  // Create the descriptor for the label.
+  auto *L = DBuilder.createLabel(Scope, Name, Unit, Line);
+  BB->setLabel(L);
+}
+
 llvm::DIType *CGDebugInfo:

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

2018-03-30 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny created this revision.
jdenny added reviewers: rsmith, hfinkel.

Diagnostics print _Bool as bool when the latter is defined as the
former.  However, diagnostics were altering the printing policy for
-ast-print as well.  The printed source was then invalid because the
preprocessor eats the bool definition.

Problematic diagnostics included suppressed warnings (e.g., add
-Wno-unused-value to the new test case), including those that are
suppressed by default.

This patch fixes this bug and cleans up some related confusing
function names and comments.


https://reviews.llvm.org/D45093

Files:
  include/clang/AST/ASTContext.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  test/Misc/ast-print-bool.c

Index: test/Misc/ast-print-bool.c
===
--- /dev/null
+++ test/Misc/ast-print-bool.c
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+//
+// RUN: %clang_cc1 -verify -ast-print %s -xc++ -DDIAG \
+// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL
+
+#if DEF_BOOL_CBOOL
+# define bool _Bool
+#elif DEF_BOOL_INT
+# define bool int
+#endif
+
+// BOOL-AS-CBOOL: _Bool i;
+// BOOL-AS-INT:   int i;
+// BOOL-AS-BOOL:  bool i;
+bool i;
+
+#ifndef __cplusplus
+// CBOOL: _Bool j;
+_Bool j;
+#endif
+
+// Induce a diagnostic (and verify we actually managed to do so), which used to
+// permanently alter the -ast-print printing policy for _Bool.  How bool is
+// defined by the preprocessor is examined only once per compilation, when the
+// diagnostic is emitted, and it used to affect the entirety of -ast-print, so
+// test only one definition of bool per compilation.
+#if DIAG
+void fn() { 1; } // expected-warning {{expression result unused}}
+#else
+// expected-no-diagnostics
+#endif
Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -505,7 +505,7 @@
   llvm::raw_svector_ostream OS(TemplateArgsStr);
   Template->printName(OS);
   printTemplateArgumentList(OS, Active->template_arguments(),
-getPrintingPolicy());
+getDiagPrintingPolicy());
   Diags.Report(Active->PointOfInstantiation,
diag::note_default_arg_instantiation_here)
 << OS.str()
@@ -571,7 +571,7 @@
   llvm::raw_svector_ostream OS(TemplateArgsStr);
   FD->printName(OS);
   printTemplateArgumentList(OS, Active->template_arguments(),
-getPrintingPolicy());
+getDiagPrintingPolicy());
   Diags.Report(Active->PointOfInstantiation,
diag::note_default_function_arg_instantiation_here)
 << OS.str()
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -3012,7 +3012,7 @@
   std::string Description;
   {
 llvm::raw_string_ostream Out(Description);
-FailedCond->printPretty(Out, nullptr, getPrintingPolicy());
+FailedCond->printPretty(Out, nullptr, getDiagPrintingPolicy());
   }
   return { FailedCond, Description };
 }
@@ -9821,7 +9821,7 @@
 }
 
 Out << " = ";
-Args[I].print(getPrintingPolicy(), Out);
+Args[I].print(getDiagPrintingPolicy(), Out);
   }
 
   Out << ']';
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5545,7 +5545,7 @@
 // conversion; use it.
 QualType ConvTy = Conversion->getConversionType().getNonReferenceType();
 std::string TypeStr;
-ConvTy.getAsStringInternal(TypeStr, SemaRef.getPrintingPolicy());
+ConvTy.getAsStringInternal(TypeStr, SemaRef.getDiagPrintingPolicy());
 
 Converter.diagnoseExplicitConv(SemaRef, Loc, T, ConvTy)
 << FixItHint::CreateInsertion(From->getLocStart(),
Index: lib/Sema/SemaLookup.cpp
==

[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

2018-03-30 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:1347
+} else if (Current.isOneOf(tok::identifier, tok::kw_new) &&
+   Current.Previous && Current.Previous->is(TT_CastRParen) &&
+   Current.Previous->MatchingParen &&

Isn't it wrong that we detect this as a cast r_paren in the first place?


Repository:
  rC Clang

https://reviews.llvm.org/D44996



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


[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

2018-03-30 Thread Daniel Jasper via Phabricator via cfe-commits
djasper accepted this revision.
djasper added a comment.

Looks good.


Repository:
  rC Clang

https://reviews.llvm.org/D44994



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


[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

2018-03-30 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:1347
+} else if (Current.isOneOf(tok::identifier, tok::kw_new) &&
+   Current.Previous && Current.Previous->is(TT_CastRParen) &&
+   Current.Previous->MatchingParen &&

djasper wrote:
> Isn't it wrong that we detect this as a cast r_paren in the first place?
Fantastic question, I asked myself the same thing.

I tried a few variations on this (leaving it as `TT_Unknown`, making a new 
type, etc.) and discovered there is at least one existing place which relies on 
the `TT_CastRParen` type as an indicator of ObjC code. Example:

https://github.com/llvm-mirror/clang/blob/e37a191e99773959118155304ec2ed0bc0d591c2/lib/Format/TokenAnnotator.cpp#L394

I can fix those, but if I do so, I think it should be a separate diff. What do 
you think?


Repository:
  rC Clang

https://reviews.llvm.org/D44996



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


[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

2018-03-30 Thread Whisperity via Phabricator via cfe-commits
whisperity created this revision.
whisperity added reviewers: alexfh, klimek.
whisperity added a project: clang.
Herald added subscribers: dkrupp, rnkovacs.

This patch extends upon https://reviews.llvm.org/D41947 because the interface 
that was landed from that patch isn't much user-friendly.

Injecting a custom virtual file system into the tool is a dangerous operation. 
If the real file system (where the installed Clang's headers reside) are not 
mirrored, it only turns out at `ClangTool::run()` that something was not 
mounted properly. Originally, the execution of a `ClangTool` always used the 
real filesystem.

Starting with https://reviews.llvm.org/D41947, the client code setting the tool 
up must manually create the OverlayFS  (which is, in turn, added as an 
OverlayFS into the tool' inner OverlayFS) containing the real system and the 
user's intention. I believe this logic should not be duplicated in client code, 
and the more traditional use-case of overlaying the filesystem view with some 
files, but by default keeping the real deal beneath it must be reflected better 
on the interface. Client code can now much more **explicitly** specify the 
complete cessation of real filesystem usage.


Repository:
  rC Clang

https://reviews.llvm.org/D45094

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp
  unittests/Tooling/ToolingTest.cpp

Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -402,24 +402,48 @@
   EXPECT_FALSE(Found);
 }
 
-TEST(ClangToolTest, BaseVirtualFileSystemUsage) {
+TEST(ClangToolVFSTest, MustGiveFileSystem) {
+  FixedCompilationDatabase Compilations("/", std::vector());
+
+  EXPECT_DEATH(ClangTool(Compilations, std::vector(),
+ std::make_shared(),
+ nullptr, false),
+   "without providing a FileSystem");
+}
+
+TEST(ClangToolVFSTest, VirtualFileSystemUsage) {
   FixedCompilationDatabase Compilations("/", std::vector());
-  llvm::IntrusiveRefCntPtr OverlayFileSystem(
-  new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
   llvm::IntrusiveRefCntPtr InMemoryFileSystem(
-  new vfs::InMemoryFileSystem);
-  OverlayFileSystem->pushOverlay(InMemoryFileSystem);
+new vfs::InMemoryFileSystem);
 
   InMemoryFileSystem->addFile(
-  "a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
+"/a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int main() {}"));
 
-  ClangTool Tool(Compilations, std::vector(1, "a.cpp"),
- std::make_shared(), OverlayFileSystem);
+  ClangTool Tool(Compilations, std::vector(1, "/a.cpp"),
+ std::make_shared(),
+ InMemoryFileSystem, false);
   std::unique_ptr Action(
-  newFrontendActionFactory());
+newFrontendActionFactory());
   EXPECT_EQ(0, Tool.run(Action.get()));
 }
 
+TEST(ClangToolVFSTest, VFSDoesntContainEveryFile) {
+  FixedCompilationDatabase Compilations("/", std::vector());
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+new vfs::InMemoryFileSystem);
+
+  InMemoryFileSystem->addFile(
+"/a.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"
+   "int main() {}"));
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cpp"),
+ std::make_shared(),
+ InMemoryFileSystem, false);
+  std::unique_ptr Action(
+newFrontendActionFactory());
+  EXPECT_NE(0, Tool.run(Action.get()));
+}
+
 // Check getClangStripDependencyFileAdjuster doesn't strip args after -MD/-MMD.
 TEST(ClangToolTest, StripDependencyFileAdjuster) {
   FixedCompilationDatabase Compilations("/", {"-MD", "-c", "-MMD", "-w"});
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -358,13 +358,22 @@
 ClangTool::ClangTool(const CompilationDatabase &Compilations,
  ArrayRef SourcePaths,
  std::shared_ptr PCHContainerOps,
- IntrusiveRefCntPtr BaseFS)
+ IntrusiveRefCntPtr FileSystem,
+ bool AlsoUseRealFileSystem)
 : Compilations(Compilations), SourcePaths(SourcePaths),
   PCHContainerOps(std::move(PCHContainerOps)),
-  OverlayFileSystem(new vfs::OverlayFileSystem(std::move(BaseFS))),
-  InMemoryFileSystem(new vfs::InMemoryFileSystem),
-  Files(new FileManager(FileSystemOptions(), OverlayFileSystem)) {
+  InMemoryFileSystem(new vfs::InMemoryFileSystem) {
+  assert(!(!AlsoUseRealFileSystem && !FileSystem) && "ClangTool initialized "
+ "without providing a FileSystem to get files from!");
+  if (AlsoUseRealFileSystem) {
+OverlayFileSystem = new vfs::OverlayFileSystem(vfs::getRealFileSystem());
+if (FileSystem)
+  OverlayFileSystem->pushOverlay(FileSystem);
+  }
+  else
+OverlayFileSyst

[PATCH] D45095: [clang-tidy] Align usage of ClangTool interface with new VFS injection

2018-03-30 Thread Whisperity via Phabricator via cfe-commits
whisperity created this revision.
whisperity added a reviewer: alexfh.
whisperity added a project: clang-tools-extra.
Herald added subscribers: dkrupp, rnkovacs, baloghadamsoftware, mgorny.

Aligns the interface landed in https://reviews.llvm.org/D41535 to the patch 
which makes VFS injection more user-friendly. The interface how `ClangTidy` and 
the error reporter gets the filesystem to use has also been made more explicit: 
now `ErrorReporter` gets the exact Clang Tool('s filesystem layout) that 
Clang-Tidy used to run checks.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45095

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidy.h
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp

Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -339,10 +339,8 @@
 
 llvm::IntrusiveRefCntPtr
 getVfsOverlayFromFile(const std::string &OverlayFile) {
-  llvm::IntrusiveRefCntPtr OverlayFS(
-  new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
   llvm::ErrorOr> Buffer =
-  OverlayFS->getBufferForFile(OverlayFile);
+  vfs::getRealFileSystem()->getBufferForFile(OverlayFile);
   if (!Buffer) {
 llvm::errs() << "Can't load virtual filesystem overlay file '"
  << OverlayFile << "': " << Buffer.getError().message()
@@ -357,8 +355,7 @@
  << OverlayFile << "'.\n";
 return nullptr;
   }
-  OverlayFS->pushOverlay(FS);
-  return OverlayFS;
+  return FS;
 }
 
 static int clangTidyMain(int argc, const char **argv) {
@@ -432,10 +429,10 @@
 llvm::cl::PrintHelpMessage(/*Hidden=*/false, /*Categorized=*/true);
 return 1;
   }
-  llvm::IntrusiveRefCntPtr BaseFS(
-  VfsOverlay.empty() ? vfs::getRealFileSystem()
+  llvm::IntrusiveRefCntPtr VirtualFileSystem(
+  VfsOverlay.empty() ? nullptr
  : getVfsOverlayFromFile(VfsOverlay));
-  if (!BaseFS)
+  if (!VfsOverlay.empty() && !VirtualFileSystem)
 return 1;
 
   ProfileData Profile;
@@ -445,8 +442,10 @@
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider));
-  runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
-   EnableCheckProfile ? &Profile : nullptr);
+  ClangTool Tool(OptionsParser.getCompilations(), PathList,
+ std::make_shared(),
+ VirtualFileSystem, true);
+  runClangTidy(Context, Tool, EnableCheckProfile ? &Profile : nullptr);
   ArrayRef Errors = Context.getErrors();
   bool FoundErrors =
   std::find_if(Errors.begin(), Errors.end(), [](const ClangTidyError &E) {
@@ -459,7 +458,7 @@
 
   // -fix-errors implies -fix.
   handleErrors(Context, (FixErrors || Fix) && !DisableFixes, WErrorCount,
-   BaseFS);
+   Tool);
 
   if (!ExportFixes.empty() && !Errors.empty()) {
 std::error_code EC;
Index: clang-tidy/tool/CMakeLists.txt
===
--- clang-tidy/tool/CMakeLists.txt
+++ clang-tidy/tool/CMakeLists.txt
@@ -16,6 +16,7 @@
   clangAST
   clangASTMatchers
   clangBasic
+  clangFrontend
   clangTidy
   clangTidyAndroidModule
   clangTidyAbseilModule
Index: clang-tidy/ClangTidy.h
===
--- clang-tidy/ClangTidy.h
+++ clang-tidy/ClangTidy.h
@@ -16,6 +16,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -225,9 +226,7 @@
 /// \param Profile if provided, it enables check profile collection in
 /// MatchFinder, and will contain the result of the profile.
 void runClangTidy(clang::tidy::ClangTidyContext &Context,
-  const tooling::CompilationDatabase &Compilations,
-  ArrayRef InputFiles,
-  llvm::IntrusiveRefCntPtr BaseFS,
+  clang::tooling::ClangTool &Tool,
   ProfileData *Profile = nullptr);
 
 // FIXME: This interface will need to be significantly extended to be useful.
@@ -238,7 +237,7 @@
 /// clang-format configuration file is found, the given \P FormatStyle is used.
 void handleErrors(ClangTidyContext &Context, bool Fix,
   unsigned &WarningsAsErrorsCount,
-  llvm::IntrusiveRefCntPtr BaseFS);
+  clang::tooling::ClangTool &Tool);
 
 /// \brief Serializes replacements into YAML and writes them to the specified
 /// output stream.
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -38,7 +38,6 @@
 #include "clang/Tooling/DiagnosticsYaml.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/ReplacementsYaml.h"
-#include "clan

r328871 - [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

2018-03-30 Thread Ben Hamilton via cfe-commits
Author: benhamilton
Date: Fri Mar 30 08:38:45 2018
New Revision: 328871

URL: http://llvm.org/viewvc/llvm-project?rev=328871&view=rev
Log:
[clang-format] Ensure wrapped ObjC selectors with 1 arg obey 
IndentWrappedFunctionNames

Summary:
In D43121, @Typz introduced logic to avoid indenting 2-or-more
argument ObjC selectors too far to the right if the first component
of the selector was longer than the others.

This had a small side effect of causing wrapped ObjC selectors with
exactly 1 argument to not obey IndentWrappedFunctionNames:

```
- (aa)
aa;
```

This diff fixes the issue by ensuring we align wrapped 1-argument
ObjC selectors correctly:

```
- (aa)
aa;
```

Test Plan: New tests added. Test failed before change, passed
  after change. Ran tests with:
  % make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: djasper, klimek, Typz, jolesiak

Reviewed By: djasper, jolesiak

Subscribers: cfe-commits, Typz

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

Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=328871&r1=328870&r2=328871&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Fri Mar 30 08:38:45 2018
@@ -896,12 +896,20 @@ unsigned ContinuationIndenter::getNewLin
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
-  if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().Indent,
- State.FirstIndent + Style.ContinuationIndentWidth)
-  : State.Stack.back().Indent) +
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(MinIndent,
+ State.FirstIndent + 
Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the first
+  // part of an ObjC selector (or a selector component which is
+  // not colon-aligned due to block formatting).
+  //
+  // Otherwise, we are indenting a subsequent part of an ObjC
+  // selector which should be colon-aligned to the longest
+  // component of the ObjC selector.
+  //
+  // In either case, we want to respect Style.IndentWrappedFunctionNames.
+  return MinIndent +
  std::max(NextNonComment->LongestObjCSelectorName,
   NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=328871&r1=328870&r2=328871&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Fri Mar 30 08:38:45 2018
@@ -537,6 +537,22 @@ TEST_F(FormatTestObjC, FormatObjCMethodD
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"


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


[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

2018-03-30 Thread Ben Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328871: [clang-format] Ensure wrapped ObjC selectors with 1 
arg obey… (authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44994?vs=140271&id=140436#toc

Repository:
  rC Clang

https://reviews.llvm.org/D44994

Files:
  lib/Format/ContinuationIndenter.cpp
  unittests/Format/FormatTestObjC.cpp


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,20 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
-  if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().Indent,
- State.FirstIndent + Style.ContinuationIndentWidth)
-  : State.Stack.back().Indent) +
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(MinIndent,
+ State.FirstIndent + 
Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the first
+  // part of an ObjC selector (or a selector component which is
+  // not colon-aligned due to block formatting).
+  //
+  // Otherwise, we are indenting a subsequent part of an ObjC
+  // selector which should be colon-aligned to the longest
+  // component of the ObjC selector.
+  //
+  // In either case, we want to respect Style.IndentWrappedFunctionNames.
+  return MinIndent +
  std::max(NextNonComment->LongestObjCSelectorName,
   NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -537,6 +537,22 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,20 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
-  if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().Indent,
- State.FirstIndent + Style.ContinuationIndentWidth)
-  : State.Stack.back().Indent) +
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(MinIndent,
+ State.FirstIndent + Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the first
+  // part of an ObjC selector (or a selector component which is
+  // not colon-aligned due to block formatting).
+  //
+  // Otherwise, we are indenting a subsequent part of an ObjC
+  // selector which should be colon-aligned to the longest
+  // component of the ObjC selector.
+  //
+  // In either case, we want to respect Style.IndentWrappedFunctionNames.
+  return MinIndent +
  std::max(NextNonComment->LongestObjCSelectorName,
   NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
Index: unittests/Format/FormatTestObjC.cpp
==

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

2018-03-30 Thread Ben Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
benhamilton marked an inline comment as done.
Closed by commit rL328871: [clang-format] Ensure wrapped ObjC selectors with 1 
arg obey… (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D44994

Files:
  cfe/trunk/lib/Format/ContinuationIndenter.cpp
  cfe/trunk/unittests/Format/FormatTestObjC.cpp


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,20 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
-  if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().Indent,
- State.FirstIndent + Style.ContinuationIndentWidth)
-  : State.Stack.back().Indent) +
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(MinIndent,
+ State.FirstIndent + 
Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the first
+  // part of an ObjC selector (or a selector component which is
+  // not colon-aligned due to block formatting).
+  //
+  // Otherwise, we are indenting a subsequent part of an ObjC
+  // selector which should be colon-aligned to the longest
+  // component of the ObjC selector.
+  //
+  // In either case, we want to respect Style.IndentWrappedFunctionNames.
+  return MinIndent +
  std::max(NextNonComment->LongestObjCSelectorName,
   NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -537,6 +537,22 @@
"   aShortf:(NSRect)theRect {\n"
"}");
 
+  // Make sure selectors with 0, 1, or more arguments are indented
+  // when IndentWrappedFunctionNames is true.
+  verifyFormat("- (a)\n"
+   ";\n");
+  verifyFormat("- (a)\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   " aaa:(int)a\n"
+   ":(int)a;\n");
+  verifyFormat("- (a)\n"
+   ":(int)a\n"
+   " aaa:(int)a;\n");
+
   // Format pairs correctly.
   Style.ColumnLimit = 80;
   verifyFormat("- (void)drawRectOn:(id)surface\n"


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -896,12 +896,20 @@
 return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
   if (NextNonComment->is(TT_SelectorName)) {
 if (!State.Stack.back().ObjCSelectorNameFound) {
-  if (NextNonComment->LongestObjCSelectorName == 0)
-return State.Stack.back().Indent;
-  return (Style.IndentWrappedFunctionNames
-  ? std::max(State.Stack.back().Indent,
- State.FirstIndent + Style.ContinuationIndentWidth)
-  : State.Stack.back().Indent) +
+  unsigned MinIndent = State.Stack.back().Indent;
+  if (Style.IndentWrappedFunctionNames)
+MinIndent = std::max(MinIndent,
+ State.FirstIndent + Style.ContinuationIndentWidth);
+  // If LongestObjCSelectorName is 0, we are indenting the first
+  // part of an ObjC selector (or a selector component which is
+  // not colon-aligned due to block formatting).
+  //
+  // Otherwise, we are indenting a subsequent part of an ObjC
+  // selector which should be colon-aligned to the longest
+  // component of the ObjC selector.
+  //
+  // In either case, we want to respect Style.IndentWrappedFunctionNames.
+  return MinIndent +
  std::max(NextNonComment->LongestObjCSelectorName,
   NextNonComment->ColumnWidth) -
  Ne

[PATCH] D44559: [Sema] Wrong width of result of mul operation

2018-03-30 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

While X86 does have multiplies that return double width results, it also has 
16/32/64-bit forms of imul that only return the lower portion of the result. 
Those multiplies are typically faster and have fewer uops than the double width 
multiplies so we prefer to use that instruction when possible. We are currently 
using the double width multiply for 8*8->8 multiplies because there is no 8-bit 
single width multiply.


https://reviews.llvm.org/D44559



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


[PATCH] D45096: Allow the creation of human-friendly ASTDumper to arbitrary output stream

2018-03-30 Thread Whisperity via Phabricator via cfe-commits
whisperity created this revision.
whisperity added reviewers: alexfh, klimek, rsmith.
whisperity added a project: clang.
Herald added subscribers: dkrupp, rnkovacs.

`ASTPrinter` allows setting the ouput to any O-Stream, but that printer creates 
source-code-like syntax (and is also marked with a `FIXME`). The nice, 
colourful, mostly human-readable `ASTDumper` only works on the standard error, 
which is not feasible in case a user wants to see the AST of a file through a 
code navigation/comprehension tool.

This small addition of an overload solves generating a nice colourful AST block 
for the users of a tool I'm working on, CodeCompass 
, as opposed to having to duplicate the 
behaviour of definitions that only exist in the anonymous namespace of 
implementation TUs related to this module.


Repository:
  rC Clang

https://reviews.llvm.org/D45096

Files:
  include/clang/Frontend/ASTConsumers.h
  lib/Frontend/ASTConsumers.cpp


Index: lib/Frontend/ASTConsumers.cpp
===
--- lib/Frontend/ASTConsumers.cpp
+++ lib/Frontend/ASTConsumers.cpp
@@ -142,8 +142,18 @@
 bool DumpDecls,
 bool Deserialize,
 bool DumpLookups) {
+  return CreateASTDumper(nullptr, FilterString,
+ DumpDecls, Deserialize, DumpLookups);
+}
+
+std::unique_ptr
+clang::CreateASTDumper(std::unique_ptr Out,
+   StringRef FilterString,
+   bool DumpDecls,
+   bool Deserialize,
+   bool DumpLookups) {
   assert((DumpDecls || Deserialize || DumpLookups) && "nothing to dump");
-  return llvm::make_unique(nullptr,
+  return llvm::make_unique(std::move(Out),
Deserialize ? ASTPrinter::DumpFull :
DumpDecls ? ASTPrinter::Dump :
ASTPrinter::None,
Index: include/clang/Frontend/ASTConsumers.h
===
--- include/clang/Frontend/ASTConsumers.h
+++ include/clang/Frontend/ASTConsumers.h
@@ -40,6 +40,13 @@
  bool DumpDecls, bool Deserialize,
  bool DumpLookups);
 
+// AST dumper: dumps the raw AST in human-readable form to the given output
+// stream.
+std::unique_ptr CreateASTDumper(std::unique_ptr OS,
+ StringRef FilterString,
+ bool DumpDecls, bool Deserialize,
+ bool DumpLookups);
+
 // AST Decl node lister: prints qualified names of all filterable AST Decl
 // nodes.
 std::unique_ptr CreateASTDeclNodeLister();


Index: lib/Frontend/ASTConsumers.cpp
===
--- lib/Frontend/ASTConsumers.cpp
+++ lib/Frontend/ASTConsumers.cpp
@@ -142,8 +142,18 @@
 bool DumpDecls,
 bool Deserialize,
 bool DumpLookups) {
+  return CreateASTDumper(nullptr, FilterString,
+ DumpDecls, Deserialize, DumpLookups);
+}
+
+std::unique_ptr
+clang::CreateASTDumper(std::unique_ptr Out,
+   StringRef FilterString,
+   bool DumpDecls,
+   bool Deserialize,
+   bool DumpLookups) {
   assert((DumpDecls || Deserialize || DumpLookups) && "nothing to dump");
-  return llvm::make_unique(nullptr,
+  return llvm::make_unique(std::move(Out),
Deserialize ? ASTPrinter::DumpFull :
DumpDecls ? ASTPrinter::Dump :
ASTPrinter::None,
Index: include/clang/Frontend/ASTConsumers.h
===
--- include/clang/Frontend/ASTConsumers.h
+++ include/clang/Frontend/ASTConsumers.h
@@ -40,6 +40,13 @@
  bool DumpDecls, bool Deserialize,
  bool DumpLookups);
 
+// AST dumper: dumps the raw AST in human-readable form to the given output
+// stream.
+std::unique_ptr CreateASTDumper(std::unique_ptr OS,
+ StringRef FilterString,
+ bool DumpDecls, bool Deserialize,
+ bool DumpLookups);
+
 // AST Decl node lister: prints qualified names of all filterable AST Decl
 // nodes.
 std::unique_ptr CreateASTDeclNodeLister();
___
cfe-commits 

[PATCH] D44552: [Coroutines] Find custom allocators in class scope

2018-03-30 Thread Gor Nishanov via Phabricator via cfe-commits
GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LGTM with some stylistic suggestions




Comment at: lib/Sema/SemaExprCXX.cpp:2351
+return true;
+  else
+LookupQualifiedName(R, Context.getTranslationUnitDecl());

drop else?

```
if (R.empty()) {
  if (NewScope == AFS_Class)
return true;

  LookupQualifiedName(R, Context.getTranslationUnitDecl());
}
```



Comment at: lib/Sema/SemaExprCXX.cpp:2402
+  return true;
+else {
+  DeclareGlobalNewDelete();

drop else? so that it will read:

```
  if (FoundDelete.empty()) {
if (DeleteScope == AFS_Class)
  return true;

DeclareGlobalNewDelete();
LookupQualifiedName(FoundDelete, Context.getTranslationUnitDecl());
  }
```


Repository:
  rC Clang

https://reviews.llvm.org/D44552



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


[PATCH] D45061: [NVPTX, CUDA] Use custom feature detection to handle NVPTX target builtins.

2018-03-30 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/include/clang/Basic/Cuda.h:55
 
+static inline const std::vector CudaKnownArchList() {
+  return {CudaArch::SM_20, CudaArch::SM_21, CudaArch::SM_30, CudaArch::SM_32,

jlebar wrote:
> Why 'static'?
Old habits.  Just 'inline' should do.



Comment at: clang/include/clang/Basic/Cuda.h:59
+  CudaArch::SM_53, CudaArch::SM_60, CudaArch::SM_61, CudaArch::SM_62,
+  CudaArch::SM_70, CudaArch::SM_72};
+}

jlebar wrote:
> Could we instead rely on the fact that the CudaArch enum is dense and goes 
> from UNKNOWN to LAST?  One less bug to make...
Actually this function is not needed. It's a leftover from an old iteration of 
the code. I'll remove it.
As for why array -- that's what I wanted to do.  Turns out that iterating over 
enums is a pain.  I can't increment/decrement it and had to cast to an int for 
that. And then back, in order to use it.


https://reviews.llvm.org/D45061



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


[PATCH] D45061: [NVPTX, CUDA] Use custom feature detection to handle NVPTX target builtins.

2018-03-30 Thread Artem Belevich via Phabricator via cfe-commits
tra updated this revision to Diff 140447.
tra added a comment.

Removed unneeded function.


https://reviews.llvm.org/D45061

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/test/CodeGen/builtins-nvptx-ptx50.cu
  clang/test/CodeGen/builtins-nvptx.c
  llvm/lib/Target/NVPTX/NVPTX.td
  llvm/lib/Target/NVPTX/NVPTXSubtarget.h

Index: llvm/lib/Target/NVPTX/NVPTXSubtarget.h
===
--- llvm/lib/Target/NVPTX/NVPTXSubtarget.h
+++ llvm/lib/Target/NVPTX/NVPTXSubtarget.h
@@ -48,10 +48,6 @@
   // FrameLowering class because TargetFrameLowering is abstract.
   NVPTXFrameLowering FrameLowering;
 
-protected:
-  // Processor supports scoped atomic operations.
-  bool HasAtomScope;
-
 public:
   /// This constructor initializes the data members to match that
   /// of the specified module.
@@ -74,7 +70,7 @@
   }
 
   bool hasAtomAddF64() const { return SmVersion >= 60; }
-  bool hasAtomScope() const { return HasAtomScope; }
+  bool hasAtomScope() const { return SmVersion >= 60; }
   bool hasAtomBitwise64() const { return SmVersion >= 32; }
   bool hasAtomMinMax64() const { return SmVersion >= 32; }
   bool hasLDG() const { return SmVersion >= 32; }
Index: llvm/lib/Target/NVPTX/NVPTX.td
===
--- llvm/lib/Target/NVPTX/NVPTX.td
+++ llvm/lib/Target/NVPTX/NVPTX.td
@@ -53,9 +53,6 @@
 def SM70 : SubtargetFeature<"sm_70", "SmVersion", "70",
  "Target SM 7.0">;
 
-def SATOM : SubtargetFeature<"satom", "HasAtomScope", "true",
- "Atomic operations with scope">;
-
 // PTX Versions
 def PTX32 : SubtargetFeature<"ptx32", "PTXVersion", "32",
  "Use PTX version 3.2">;
@@ -88,10 +85,10 @@
 def : Proc<"sm_50", [SM50, PTX40]>;
 def : Proc<"sm_52", [SM52, PTX41]>;
 def : Proc<"sm_53", [SM53, PTX42]>;
-def : Proc<"sm_60", [SM60, PTX50, SATOM]>;
-def : Proc<"sm_61", [SM61, PTX50, SATOM]>;
-def : Proc<"sm_62", [SM62, PTX50, SATOM]>;
-def : Proc<"sm_70", [SM70, PTX60, SATOM]>;
+def : Proc<"sm_60", [SM60, PTX50]>;
+def : Proc<"sm_61", [SM61, PTX50]>;
+def : Proc<"sm_62", [SM62, PTX50]>;
+def : Proc<"sm_70", [SM70, PTX60]>;
 
 def NVPTXInstrInfo : InstrInfo {
 }
Index: clang/test/CodeGen/builtins-nvptx.c
===
--- clang/test/CodeGen/builtins-nvptx.c
+++ clang/test/CodeGen/builtins-nvptx.c
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_60 \
 // RUN:-fcuda-is-device -S -emit-llvm -o - -x cuda %s \
 // RUN:   | FileCheck -check-prefix=CHECK -check-prefix=LP64 %s
+// RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_61 \
+// RUN:-fcuda-is-device -S -emit-llvm -o - -x cuda %s \
+// RUN:   | FileCheck -check-prefix=CHECK -check-prefix=LP64 %s
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown -target-cpu sm_53 \
 // RUN:   -DERROR_CHECK -fcuda-is-device -S -o /dev/null -x cuda -verify %s
 
@@ -292,245 +295,245 @@
 #if ERROR_CHECK || __CUDA_ARCH__ >= 600
 
   // CHECK: call i32 @llvm.nvvm.atomic.add.gen.i.cta.i32.p0i32
-  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_i' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_i' needs target feature sm_60}}
   __nvvm_atom_cta_add_gen_i(ip, i);
   // LP32: call i32 @llvm.nvvm.atomic.add.gen.i.cta.i32.p0i32
   // LP64: call i64 @llvm.nvvm.atomic.add.gen.i.cta.i64.p0i64
-  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_l' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_l' needs target feature sm_60}}
   __nvvm_atom_cta_add_gen_l(&dl, l);
   // CHECK: call i64 @llvm.nvvm.atomic.add.gen.i.cta.i64.p0i64
-  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_ll' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_ll' needs target feature sm_60}}
   __nvvm_atom_cta_add_gen_ll(&sll, ll);
   // CHECK: call i32 @llvm.nvvm.atomic.add.gen.i.sys.i32.p0i32
-  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_i' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_i' needs target feature sm_60}}
   __nvvm_atom_sys_add_gen_i(ip, i);
   // LP32: call i32 @llvm.nvvm.atomic.add.gen.i.sys.i32.p0i32
   // LP64: call i64 @llvm.nvvm.atomic.add.gen.i.sys.i64.p0i64
-  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_l' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_l' needs target feature sm_60}}
   __nvvm_atom_sys_add_gen_l(&dl, l);
   // CHECK: call i64 @llvm.nvvm.atomic.add.gen.i.sys.i64.p0i64
-  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_ll' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_ll' needs target feature sm_60}}
   __nvvm_atom_sys_add_gen_ll(&sll, ll);
 
   // CHECK: call float @llvm.nvvm.atomic.add.gen.f.cta.f32.p0f32
-  // expe

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

You noted, that the C++ warning would catch this case, but does not get 
triggered in C. Would it be wort to generalize the concern and have a warning 
that catch the comparison, regardless of the macro?

Do other major libraries define a similar macro but name it differently? If so, 
including there names would be worthwhile.




Comment at: clang-tidy/bugprone/ComparisonInTempFailureRetryCheck.cpp:22
+namespace {
+AST_MATCHER(Expr, isBinOpComparison) {
+  const auto *BinOp = dyn_cast(Node.IgnoreParenCasts());

I think this matcher already exists.

https://github.com/JonasToth/clang-tools-extra/blob/master/clang-tidy/utils/Matchers.h

`isEqualityOperator` would work? See the other matchers there too. If the 
matchers there would not do it, maybe add one there.


https://reviews.llvm.org/D45059



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


[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-03-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clangd/FindSymbols.cpp:31
+
+  if (supportedSymbolKinds &&
+  std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),

malaperle wrote:
> MaskRay wrote:
> > This std::find loop adds some overhead to each candidate... In my 
> > experience the client usually doesn't care about the returned symbol kinds, 
> > they are used to give a category name. You can always patch the upstream to 
> > add missing categories.
> > 
> > This is one instance where LSP is over specified. nvm I don't have strong 
> > opinion here
> I have a client that throws an exception when the symbolkind is not known and 
> the whole request fails, so I think it's worth checking. But if it's too slow 
> I can look at making it faster. Unfortunately, I cannot patch any upstream 
> project :)
https://github.com/gluon-lang/languageserver-types/blob/master/src/lib.rs#L2016

LanguageClient-neovim returns empty candidate list if one of the candidates has 
unknown SymbolKind. Apparently they should be more tolerant and there is an 
issue tracking it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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


[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-03-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clangd/FindSymbols.cpp:31
+
+  if (supportedSymbolKinds &&
+  std::find(supportedSymbolKinds->begin(), supportedSymbolKinds->end(),

MaskRay wrote:
> malaperle wrote:
> > MaskRay wrote:
> > > This std::find loop adds some overhead to each candidate... In my 
> > > experience the client usually doesn't care about the returned symbol 
> > > kinds, they are used to give a category name. You can always patch the 
> > > upstream to add missing categories.
> > > 
> > > This is one instance where LSP is over specified. nvm I don't have strong 
> > > opinion here
> > I have a client that throws an exception when the symbolkind is not known 
> > and the whole request fails, so I think it's worth checking. But if it's 
> > too slow I can look at making it faster. Unfortunately, I cannot patch any 
> > upstream project :)
> https://github.com/gluon-lang/languageserver-types/blob/master/src/lib.rs#L2016
> 
> LanguageClient-neovim returns empty candidate list if one of the candidates 
> has unknown SymbolKind. Apparently they should be more tolerant and there is 
> an issue tracking it.
If it was not an internal confidential client, I would like to know its name, 
unless the confidentiality includes the existence of the client.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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


[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section

2018-03-30 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment.

The "struct object" is an implementation detail of the unwind implementation. 
You are guaranteed historically to get at least 8 longs / 8 pointers for 
internal use statically allocated in each object. What is stored inside is up 
to the unwind implementation.


https://reviews.llvm.org/D44494



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


[PATCH] D45086: [analyzer] Unroll the loop when it has a unsigned counter.

2018-03-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.

Looks reasonable


Repository:
  rC Clang

https://reviews.llvm.org/D45086



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


[PATCH] D45071: [analyzer] Track null or undef values through pointer arithmetic.

2018-03-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.

LGTM with a nit.
Also I don't quite understand why being additive is important? Isn't pointer 
subtraction basically the same?




Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:78
 
+const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
+  if (B->isAdditiveOp() && B->getType()->isPointerType()) {

static.
+1 for using functions.


Repository:
  rC Clang

https://reviews.llvm.org/D45071



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


[PATCH] D44955: [CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr.

2018-03-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.

LGTM with a nit, provided a proper fix can not be landed in a timely enough 
matter.




Comment at: lib/Analysis/CFG.cpp:740
 cleanupConstructionContext(CE);
-return;
+if (const ConstructionContext *CC =
+ConstructionContext::createFromLayers(

auto


Repository:
  rC Clang

https://reviews.llvm.org/D44955



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


[PATCH] D44854: [analyzer] Be more careful about C++17 copy elision.

2018-03-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.

LGTM provided comments are answered. Field rename would be appreciated, if 
possible.




Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:215
+  // able to find construction context at all.
+  CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+} else if (!isa(

Is this field `false` by default? Also, double negation is harder to read (e.g. 
`not modelled properly = false`  vs. `modelled property = true`), but I guess 
that should have been said earlier.


https://reviews.llvm.org/D44854



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


[PATCH] D18860: [analyzer] Fix the "Zombie symbols" issue.

2018-03-30 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment.

@NoQ can we get the evaluation re-done?


https://reviews.llvm.org/D18860



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


[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-30 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv updated this revision to Diff 140459.
george.burgess.iv marked an inline comment as done.
george.burgess.iv added a comment.

Addressed feedback


https://reviews.llvm.org/D45059

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ComparisonInTempFailureRetryCheck.cpp
  clang-tidy/bugprone/ComparisonInTempFailureRetryCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-comparison-in-temp-failure-retry.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-comparison-in-temp-failure-retry.c

Index: test/clang-tidy/bugprone-comparison-in-temp-failure-retry.c
===
--- /dev/null
+++ test/clang-tidy/bugprone-comparison-in-temp-failure-retry.c
@@ -0,0 +1,109 @@
+// RUN: %check_clang_tidy %s bugprone-comparison-in-temp-failure-retry %t
+
+#define TEMP_FAILURE_RETRY(x)  \
+  ({   \
+typeof(x) __z; \
+do \
+  __z = (x);   \
+while (__z == -1); \
+__z;   \
+  })
+
+int foo();
+int bar(int a);
+
+void test() {
+  int i;
+  TEMP_FAILURE_RETRY((i = foo()));
+  TEMP_FAILURE_RETRY(foo());
+  TEMP_FAILURE_RETRY((foo()));
+
+  TEMP_FAILURE_RETRY(foo() == 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY [bugprone-comparison-in-temp-failure-retry]
+  TEMP_FAILURE_RETRY((foo() == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+  TEMP_FAILURE_RETRY((int)(foo() == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+  TEMP_FAILURE_RETRY(bar(foo() == 1));
+  TEMP_FAILURE_RETRY((bar(foo() == 1)));
+  TEMP_FAILURE_RETRY((bar(foo() == 1)) == 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+  TEMP_FAILURE_RETRY(((bar(foo() == 1)) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:41: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+  TEMP_FAILURE_RETRY((int)((bar(foo() == 1)) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+#define INDIRECT TEMP_FAILURE_RETRY
+  INDIRECT(foo());
+  INDIRECT((foo() == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+  INDIRECT(bar(foo() == 1));
+  INDIRECT((int)((bar(foo() == 1)) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+#define TFR(x) TEMP_FAILURE_RETRY(x)
+  TFR(foo());
+  TFR((foo() == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+  TFR(bar(foo() == 1));
+  TFR((int)((bar(foo() == 1)) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+#define ADD_TFR(x) (1 + TEMP_FAILURE_RETRY(x) + 1)
+  ADD_TFR(foo());
+  ADD_TFR(foo() == 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+  ADD_TFR(bar(foo() == 1));
+  ADD_TFR((int)((bar(foo() == 1)) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+#define ADDP_TFR(x) (1 + TEMP_FAILURE_RETRY((x)) + 1)
+  ADDP_TFR(foo());
+  ADDP_TFR((foo() == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+  ADDP_TFR(bar(foo() == 1));
+  ADDP_TFR((int)((bar(foo() == 1)) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+#define MACRO TEMP_FAILURE_RETRY(foo() == 1)
+  MACRO;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+  // Be sure that being a macro arg doesn't mess with this.
+#define ID(x) (x)
+  ID(ADDP_TFR(bar(foo() == 1)));
+  ID(ADDP_TFR(bar(foo() == 1) == 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+  ID(MACRO);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+
+#define CMP(x) x == 1
+  TEMP_FAILURE_RETRY(CMP(foo()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: Top-level comparisons should be moved out of TEMP_FAILURE_RETRY
+}
+
+// I 

[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-30 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment.

Thanks for the feedback!

> You noted, that the C++ warning would catch this case, but does not get 
> triggered in C. Would it be wort to generalize the concern and have a warning 
> that catch the comparison, regardless of the macro?

I'm not quite sure how we would go about that with confidence. The code we'd 
need to catch looks something like:

  typeof(foo() == y) x;
  /* snip */
  bar(x == -1); // warning: comparison is pointless

If we could tell that `x`'s type was inferred from a comparison op, we could 
warn here. However, if the user used `x` as anything but a C++ `bool` in `/* 
snip */`, the warning would be incorrect. We could maybe do some sort of range 
analysis by walking the AST, but that sounds like more like a static analyzer 
thing.

...And none of that would catch glibc's `TEMP_FAILURE_RETRY` macro, unless we 
wanted to apply said analysis to all integral variables.

> Do other major libraries define a similar macro but name it differently? If 
> so, including there names would be worthwhile.

Dunno. A quick search for alternatives on MacOS said "no; #define it yourself," 
and I know so little about Windows that I have no clue if a 
`TEMP_FAILURE_RETRY`-like macro would even be reasonable there. :)

If you have anything in mind, I'm happy to add it.


https://reviews.llvm.org/D45059



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


r328891 - Remove unused CHECK lines leftover from r306928.

2018-03-30 Thread Eli Friedman via cfe-commits
Author: efriedma
Date: Fri Mar 30 11:39:28 2018
New Revision: 328891

URL: http://llvm.org/viewvc/llvm-project?rev=328891&view=rev
Log:
Remove unused CHECK lines leftover from r306928.

The RUN lines were removed, but the corresponding CHECK lines never
went away.


Modified:
cfe/trunk/test/Driver/arm-execute-only.c

Modified: cfe/trunk/test/Driver/arm-execute-only.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-execute-only.c?rev=328891&r1=328890&r2=328891&view=diff
==
--- cfe/trunk/test/Driver/arm-execute-only.c (original)
+++ cfe/trunk/test/Driver/arm-execute-only.c Fri Mar 30 11:39:28 2018
@@ -21,10 +21,6 @@
 // RUN: %clang -target armv7m-eabi -x assembler -mexecute-only %s -c -### 2>&1 
\
 // RUN:| FileCheck %s -check-prefix CHECK-NO-EXECUTE-ONLY -check-prefix 
CHECK-NO-EXECUTE-ONLY-ASM
 
-//
-// CHECK-NO-EXECUTE-ONLY-NOT: "-backend-option" "-arm-execute-only"
-// CHECK-EXECUTE-ONLY: "-backend-option" "-arm-execute-only"
-
 // CHECK-EXECUTE-ONLY-NOT-SUPPORTED: error: execute only is not supported for 
the thumbv6m sub-architecture
 // CHECK-EXECUTE-ONLY-NO-MOVT: error: option '-mexecute-only' cannot be 
specified with '-mno-movt'
 // CHECK-EXECUTE-ONLY-LONG-CALLS: error: option '-mexecute-only' cannot be 
specified with '-mlong-calls'


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


[PATCH] D45101: [ObjC] Use the name specified by objc_runtime_name instead of the class identifier

2018-03-30 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision.
ahatanak added a reviewer: rjmccall.

This patch fixes a few places in CGObjCMac.cpp where the class identifier was 
used instead of the name specified by objc_runtime_name.

rdar://problem/37910822


Repository:
  rC Clang

https://reviews.llvm.org/D45101

Files:
  lib/CodeGen/CGObjCMac.cpp
  test/CodeGenObjC/objc-runtime-name.m


Index: test/CodeGenObjC/objc-runtime-name.m
===
--- /dev/null
+++ test/CodeGenObjC/objc-runtime-name.m
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple i386-apple-macosx10.13.0 
-fobjc-runtime=macosx-fragile-10.13.0 -fobjc-subscripting-legacy-runtime 
-emit-llvm -o - %s | FileCheck %s
+
+// Check that the runtime name is emitted and used instead of the class
+// identifier.
+
+// CHECK: module asm {{.*}}objc_class_name_XYZ=0
+// CHECK: module asm {{.*}}globl .objc_class_name_XYZ
+// CHECK: module asm {{.*}}lazy_reference .objc_class_name_XYZ
+
+// CHECK: @[[OBJC_CLASS_NAME:.*]] = private unnamed_addr constant [4 x i8] 
c"XYZ{{.*}}, section "__TEXT,__cstring,cstring_literals",
+// CHECK: = private global {{.*}} bitcast ([4 x i8]* @[[OBJC_CLASS_NAME]] to 
{{.*}}), section "__OBJC,__cls_refs,literal_pointers,no_dead_strip"
+
+__attribute__((objc_root_class,objc_runtime_name("XYZ")))
+@interface A
++(void)m1;
+@end
+
+@implementation A
++(void)m1 {}
+@end
+
+void test(void) {
+  [A m1];
+}
Index: lib/CodeGen/CGObjCMac.cpp
===
--- lib/CodeGen/CGObjCMac.cpp
+++ lib/CodeGen/CGObjCMac.cpp
@@ -3401,7 +3401,9 @@
   See EmitClassExtension();
 */
 void CGObjCMac::GenerateClass(const ObjCImplementationDecl *ID) {
-  DefinedSymbols.insert(ID->getIdentifier());
+  IdentifierInfo *RuntimeName =
+  &CGM.getContext().Idents.get(ID->getObjCRuntimeNameAsString());
+  DefinedSymbols.insert(RuntimeName);
 
   std::string ClassName = ID->getNameAsString();
   // FIXME: Gross
@@ -4980,7 +4982,9 @@
   if (ID->hasAttr())
 return EmitClassRefViaRuntime(CGF, ID, ObjCTypes);
 
-  return EmitClassRefFromId(CGF, ID->getIdentifier());
+  IdentifierInfo *RuntimeName =
+  &CGM.getContext().Idents.get(ID->getObjCRuntimeNameAsString());
+  return EmitClassRefFromId(CGF, RuntimeName);
 }
 
 llvm::Value *CGObjCMac::EmitNSAutoreleasePoolClassRef(CodeGenFunction &CGF) {


Index: test/CodeGenObjC/objc-runtime-name.m
===
--- /dev/null
+++ test/CodeGenObjC/objc-runtime-name.m
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple i386-apple-macosx10.13.0 -fobjc-runtime=macosx-fragile-10.13.0 -fobjc-subscripting-legacy-runtime -emit-llvm -o - %s | FileCheck %s
+
+// Check that the runtime name is emitted and used instead of the class
+// identifier.
+
+// CHECK: module asm {{.*}}objc_class_name_XYZ=0
+// CHECK: module asm {{.*}}globl .objc_class_name_XYZ
+// CHECK: module asm {{.*}}lazy_reference .objc_class_name_XYZ
+
+// CHECK: @[[OBJC_CLASS_NAME:.*]] = private unnamed_addr constant [4 x i8] c"XYZ{{.*}}, section "__TEXT,__cstring,cstring_literals",
+// CHECK: = private global {{.*}} bitcast ([4 x i8]* @[[OBJC_CLASS_NAME]] to {{.*}}), section "__OBJC,__cls_refs,literal_pointers,no_dead_strip"
+
+__attribute__((objc_root_class,objc_runtime_name("XYZ")))
+@interface A
++(void)m1;
+@end
+
+@implementation A
++(void)m1 {}
+@end
+
+void test(void) {
+  [A m1];
+}
Index: lib/CodeGen/CGObjCMac.cpp
===
--- lib/CodeGen/CGObjCMac.cpp
+++ lib/CodeGen/CGObjCMac.cpp
@@ -3401,7 +3401,9 @@
   See EmitClassExtension();
 */
 void CGObjCMac::GenerateClass(const ObjCImplementationDecl *ID) {
-  DefinedSymbols.insert(ID->getIdentifier());
+  IdentifierInfo *RuntimeName =
+  &CGM.getContext().Idents.get(ID->getObjCRuntimeNameAsString());
+  DefinedSymbols.insert(RuntimeName);
 
   std::string ClassName = ID->getNameAsString();
   // FIXME: Gross
@@ -4980,7 +4982,9 @@
   if (ID->hasAttr())
 return EmitClassRefViaRuntime(CGF, ID, ObjCTypes);
 
-  return EmitClassRefFromId(CGF, ID->getIdentifier());
+  IdentifierInfo *RuntimeName =
+  &CGM.getContext().Idents.get(ID->getObjCRuntimeNameAsString());
+  return EmitClassRefFromId(CGF, RuntimeName);
 }
 
 llvm::Value *CGObjCMac::EmitNSAutoreleasePoolClassRef(CodeGenFunction &CGF) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45102: Fix typo in comment -fmath-errno=0 -> -fno-math-errno

2018-03-30 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 created this revision.
Herald added subscribers: cfe-commits, aheejin.

The former is not a valid clang argument


Repository:
  rC Clang

https://reviews.llvm.org/D45102

Files:
  include/clang/Basic/Builtins.def


Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -89,7 +89,7 @@
 //  S:N: -> similar to the s:N: attribute, but the function is like vscanf
 //  in that it accepts its arguments as a va_list rather than
 //  through an ellipsis
-//  e -> const, but only when -fmath-errno=0
+//  e -> const, but only when -fno-math-errno
 //  j -> returns_twice (like setjmp)
 //  u -> arguments are not evaluated for their side-effects
 //  FIXME: gcc has nonnull


Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -89,7 +89,7 @@
 //  S:N: -> similar to the s:N: attribute, but the function is like vscanf
 //  in that it accepts its arguments as a va_list rather than
 //  through an ellipsis
-//  e -> const, but only when -fmath-errno=0
+//  e -> const, but only when -fno-math-errno
 //  j -> returns_twice (like setjmp)
 //  u -> arguments are not evaluated for their side-effects
 //  FIXME: gcc has nonnull
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45101: [ObjC] Use the name specified by objc_runtime_name instead of the class identifier

2018-03-30 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Note that CGObjCNonFragileABIMac::EmitClassRef also passes the class identifier 
to CGObjCNonFragileABIMac::EmitClassRefFromId, but it doesn't cause a problem. 
CGObjCNonFragileABIMac::EmitClassRefFromId uses the identifier only when the 
ObjCInterfaceDecl passed to it is null and that happens only when it is called 
from CGObjCNonFragileABIMac::EmitNSAutoreleasePoolClassRef.


Repository:
  rC Clang

https://reviews.llvm.org/D45101



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


[PATCH] D44854: [analyzer] Be more careful about C++17 copy elision.

2018-03-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:215
+  // able to find construction context at all.
+  CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+} else if (!isa(

george.karpenkov wrote:
> Is this field `false` by default? Also, double negation is harder to read 
> (e.g. `not modelled properly = false`  vs. `modelled property = true`), but I 
> guess that should have been said earlier.
Yeah, flags within `EvalCallOptions` are all "red" flags to warn `evalCall()` 
that something is fishy about the call. They're all off by default. I'm not 
sure what's the best way to change that. I don't want each branch to set like 4 
different flags, so they should be non-"red" by default.


https://reviews.llvm.org/D44854



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


[PATCH] D45086: [analyzer] Unroll the loop when it has a unsigned counter.

2018-03-30 Thread Peter Szecsi via Phabricator via cfe-commits
szepet accepted this revision.
szepet added a comment.

Yepp, thanks for the patch! One small typo below.




Comment at: test/Analysis/loop-unrolling.cpp:50
+
+int simple_unroll4_unsinged() {
+  int a[9];

typo: unsigned


Repository:
  rC Clang

https://reviews.llvm.org/D45086



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


[PATCH] D44955: [CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr.

2018-03-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 140464.
NoQ added a comment.

Use auto.


https://reviews.llvm.org/D44955

Files:
  include/clang/Analysis/ConstructionContext.h
  lib/Analysis/CFG.cpp
  lib/Analysis/ConstructionContext.cpp
  test/Analysis/missing-bind-temporary.cpp

Index: test/Analysis/missing-bind-temporary.cpp
===
--- /dev/null
+++ test/Analysis/missing-bind-temporary.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++14 -verify %s
+
+void clang_analyzer_eval(bool);
+
+int global;
+
+namespace variant_0 {
+// This variant of the code works correctly. Function foo() is not a template
+// function. Note that there are two destructors within foo().
+
+class A {
+public:
+  ~A() { ++global; }
+};
+
+class B {
+  A a;
+};
+
+// CHECK: void foo(int)
+// CHECK:   [B1]
+// CHECK-NEXT:1:  (CXXConstructExpr, [B1.2], class variant_0::B)
+// CHECK-NEXT:2: variant_0::B i;
+// CHECK-NEXT:3: operator=
+// CHECK-NEXT:4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_0::B &(*)(class variant_0::B &&) noexcept)
+// CHECK-NEXT:5: i
+// CHECK-NEXT:6: {} (CXXConstructExpr, [B1.7], [B1.8], class variant_0::B)
+// CHECK-NEXT:7: [B1.6] (BindTemporary)
+// CHECK-NEXT:8: [B1.7]
+// CHECK-NEXT:9: [B1.5] = [B1.8] (OperatorCall)
+// CHECK-NEXT:   10: ~variant_0::B() (Temporary object destructor)
+// CHECK-NEXT:   11: [B1.2].~B() (Implicit destructor)
+void foo(int) {
+  B i;
+  i = {};
+}
+
+void bar() {
+  global = 0;
+  foo(1);
+  clang_analyzer_eval(global == 2); // expected-warning{{TRUE}}
+}
+
+} // end namespace variant_0
+
+namespace variant_1 {
+// Suddenly, if we turn foo() into a template, we are missing a
+// CXXBindTemporaryExpr in the AST, and therefore we're missing a
+// temporary destructor in the CFG.
+
+class A {
+public:
+  ~A() { ++global; }
+};
+
+class B {
+  A a;
+};
+
+// FIXME: Find the construction context for {} and enforce the temporary
+// destructor.
+// CHECK: template<> void foo(int)
+// CHECK:   [B1]
+// CHECK-NEXT:1:  (CXXConstructExpr, [B1.2], class variant_1::B)
+// CHECK-NEXT:2: variant_1::B i;
+// CHECK-NEXT:3: operator=
+// CHECK-NEXT:4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_1::B &(*)(class variant_1::B &&) noexcept)
+// CHECK-NEXT:5: i
+// CHECK-NEXT:6: {} (CXXConstructExpr, class variant_1::B)
+// CHECK-NEXT:7: [B1.6]
+// CHECK-NEXT:8: [B1.5] = [B1.7] (OperatorCall)
+// CHECK-NEXT:9: [B1.2].~B() (Implicit destructor)
+template  void foo(T) {
+  B i;
+  i = {};
+}
+
+void bar() {
+  global = 0;
+  foo(1);
+  // FIXME: Should be TRUE, i.e. we should call (and inline) two destructors.
+  clang_analyzer_eval(global == 2); // expected-warning{{UNKNOWN}}
+}
+
+} // end namespace variant_1
+
+namespace variant_2 {
+// Making field 'a' in class 'B' public turns the class into an aggregate.
+// In this case there is no constructor at {} - only an aggregate
+// initialization. Aggregate initialization is unsupported for now.
+
+class A {
+public:
+  ~A() { ++global; }
+};
+
+class B {
+public:
+  A a;
+};
+
+// CHECK: template<> void foo(int)
+// CHECK:   [B1]
+// CHECK-NEXT:1:  (CXXConstructExpr, [B1.2], class variant_2::B)
+// CHECK-NEXT:2: variant_2::B i;
+// CHECK-NEXT:3: operator=
+// CHECK-NEXT:4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_2::B &(*)(class variant_2::B &&) noexcept)
+// CHECK-NEXT:5: i
+// CHECK-NEXT:6: {}
+// CHECK-NEXT:7: {}
+// CHECK-NEXT:8: [B1.7] (BindTemporary)
+// CHECK-NEXT:9: [B1.8]
+// CHECK-NEXT:   10: [B1.5] = [B1.9] (OperatorCall)
+// CHECK-NEXT:   11: ~variant_2::B() (Temporary object destructor)
+// CHECK-NEXT:   12: [B1.2].~B() (Implicit destructor)
+template  void foo(T) {
+  B i;
+  i = {};
+}
+
+void bar() {
+  global = 0;
+  foo(1);
+  // FIXME: Should be TRUE, i.e. we should call (and inline) two destructors.
+  clang_analyzer_eval(global == 2); // expected-warning{{UNKNOWN}}
+}
+
+} // end namespace variant_2
Index: lib/Analysis/ConstructionContext.cpp
===
--- lib/Analysis/ConstructionContext.cpp
+++ lib/Analysis/ConstructionContext.cpp
@@ -101,9 +101,12 @@
 if (const auto *MTE = dyn_cast(S)) {
   // If the object requires destruction and is not lifetime-extended,
   // then it must have a BTE within its MTE.
-  assert(MTE->getType().getCanonicalType()
+  // FIXME: This should be an assertion.
+  if (!(MTE->getType().getCanonicalType()
 ->getAsCXXRecordDecl()->hasTrivialDestructor() ||
- MTE->getStorageDuration() != SD_FullExpression);
+ MTE->getStorageDuration() != SD_FullExpression))
+return nullptr;
+
   assert(Top

[PATCH] D45071: [analyzer] Track null or undef values through pointer arithmetic.

2018-03-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 140466.
NoQ added a comment.

Substraction is an additive operation. Added tests for that. Added even more 
tests.


https://reviews.llvm.org/D45071

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/inlining/inline-defensive-checks.c
  test/Analysis/null-deref-path-notes.c

Index: test/Analysis/null-deref-path-notes.c
===
--- test/Analysis/null-deref-path-notes.c
+++ test/Analysis/null-deref-path-notes.c
@@ -1,9 +1,52 @@
-// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core,unix -analyzer-output=text -verify %s
 
 // Avoid the crash when finding the expression for tracking the origins
 // of the null pointer for path notes.
 void pr34373() {
   int *a = 0; // expected-note{{'a' initialized to a null pointer value}}
   (a + 0)[0]; // expected-warning{{Array access results in a null pointer dereference}}
   // expected-note@-1{{Array access results in a null pointer dereference}}
 }
+
+typedef __typeof(sizeof(int)) size_t;
+void *memcpy(void *dest, const void *src, unsigned long count);
+
+void f1(char *source) {
+  char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}}
+  memcpy(destination + 0, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f2(char *source) {
+  char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}}
+  memcpy(destination - 0, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f3(char *source) {
+  char *destination = 0; // FIXME: There should be a note here as well.
+  destination = destination + 0; // expected-note{{Null pointer value stored to 'destination'}}
+  memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f4(char *source) {
+  char *destination = 0; // FIXME: There should be a note here as well.
+  destination = destination - 0; // expected-note{{Null pointer value stored to 'destination'}}
+  memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f5(char *source) {
+  char *destination1 = 0; // expected-note{{'destination1' initialized to a null pointer value}}
+  char *destination2 = destination1 + 0; // expected-note{{'destination2' initialized to a null pointer value}}
+  memcpy(destination2, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+// expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f6(char *source) {
+  char *destination1 = 0; // expected-note{{'destination1' initialized to a null pointer value}}
+  char *destination2 = destination1 - 0; // expected-note{{'destination2' initialized to a null pointer value}}
+  memcpy(destination2, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+// expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
Index: test/Analysis/inlining/inline-defensive-checks.c
===
--- test/Analysis/inlining/inline-defensive-checks.c
+++ test/Analysis/inlining/inline-defensive-checks.c
@@ -159,8 +159,7 @@
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
   idc(s);
   int *x = &(s->f2) - 1;
-  // FIXME: Should not warn.
-  *x = 7; // expected-warning{{Dereference of null pointer}}
+  *x = 7; // no-warning
 }
 
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -75,6 +75,17 @@
   return false;
 }
 
+static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
+  if (B->isAdditiveOp() && B->getType()->isPointerType()) {
+if (B->getLHS()->getType()->isPointerType()) {
+  return B->getLHS();
+} else if (B->getRHS()->getType()->isPointerType()) {
+  return B->getRHS();
+}
+  }
+  return nullptr;
+}
+
 /// Given that expression S represents a pointer that would be dereferenced,
 /// try t

[PATCH] D45071: [analyzer] Track null or undef values through pointer arithmetic.

2018-03-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked an inline comment as done.
NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:78
 
+const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
+  if (B->isAdditiveOp() && B->getType()->isPointerType()) {

george.karpenkov wrote:
> static.
> +1 for using functions.
Whoops.


https://reviews.llvm.org/D45071



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


r328893 - [CFG] [analyzer] Avoid modeling C++17 constructors that aren't fully supported.

2018-03-30 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Fri Mar 30 12:21:18 2018
New Revision: 328893

URL: http://llvm.org/viewvc/llvm-project?rev=328893&view=rev
Log:
[CFG] [analyzer] Avoid modeling C++17 constructors that aren't fully supported.

Not enough work has been done so far to ensure correctness of construction
contexts in the CFG when C++17 copy elision is in effect, so for now we
should drop construction contexts in the CFG and in the analyzer when
they seem different from what we support anyway.

This includes initializations with conditional operators and return values
across multiple stack frames.

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

Added:
cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp
Modified:
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=328893&r1=328892&r2=328893&view=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Fri Mar 30 12:21:18 2018
@@ -1302,6 +1302,15 @@ void CFGBuilder::findConstructionContext
   }
   case Stmt::ConditionalOperatorClass: {
 auto *CO = cast(Child);
+if (!dyn_cast_or_null(Layer->getTriggerStmt())) {
+  // If the object returned by the conditional operator is not going to be 
a
+  // temporary object that needs to be immediately materialized, then
+  // it must be C++17 with its mandatory copy elision. Do not yet promise
+  // to support this case.
+  assert(!CO->getType()->getAsCXXRecordDecl() || CO->isGLValue() ||
+ Context->getLangOpts().CPlusPlus17);
+  break;
+}
 findConstructionContexts(Layer, CO->getLHS());
 findConstructionContexts(Layer, CO->getRHS());
 break;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=328893&r1=328892&r2=328893&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Fri Mar 30 12:21:18 2018
@@ -203,13 +203,24 @@ ExprEngine::getRegionForConstructedObjec
   // TODO: What exactly happens when we are? Does the temporary object live
   // long enough in the region store in this case? Would checkers think
   // that this object immediately goes out of scope?
-  // TODO: We assume that the call site has a temporary object construction
-  // context. This is no longer true in C++17 or when copy elision is
-  // performed. We may need to unwrap multiple stack frames here and we
-  // won't necessarily end up with a temporary at the end.
   const LocationContext *TempLCtx = LCtx;
-  if (const LocationContext *CallerLCtx =
-  LCtx->getCurrentStackFrame()->getParent()) {
+  const StackFrameContext *SFC = LCtx->getCurrentStackFrame();
+  if (const LocationContext *CallerLCtx = SFC->getParent()) {
+auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
+   .getAs();
+if (!RTC) {
+  // We were unable to find the correct construction context for the
+  // call in the parent stack frame. This is equivalent to not being
+  // able to find construction context at all.
+  CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+} else if (!isa(
+   RTC->getConstructionContext())) {
+  // FXIME: The return value is constructed directly into a
+  // non-temporary due to C++17 mandatory copy elision. This is not
+  // implemented yet.
+  assert(getContext().getLangOpts().CPlusPlus17);
+  CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+}
 TempLCtx = CallerLCtx;
   }
   CallOpts.IsTemporaryCtorOrDtor = true;

Modified: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg-rich-constructors.cpp?rev=328893&r1=328892&r2=328893&view=diff
==
--- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp (original)
+++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp Fri Mar 30 12:21:18 2018
@@ -108,6 +108,9 @@ void simpleVariableInitializedByValue()
   C c = C::get();
 }
 
+// FIXME: Find construction contexts for both branches in C++17.
+// Note that once it gets detected, the test for the get() branch would not
+// fail, because FileCheck allows partial matches.
 // CHECK: void simpleVariableWithTernaryOperator(bool coin)
 // CHECK:[B1]
 // CXX11-NEXT: 1: [B4.2] ? [B2.5] : [B

[PATCH] D44854: [analyzer] Be more careful about C++17 copy elision.

2018-03-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328893: [CFG] [analyzer] Avoid modeling C++17 constructors 
that aren't fully supported. (authored by dergachev, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D44854

Files:
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/cfg-rich-constructors.cpp
  test/Analysis/cxx17-mandatory-elision.cpp
  test/Analysis/temp-obj-dtors-cfg-output.cpp

Index: test/Analysis/cfg-rich-constructors.cpp
===
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -108,6 +108,9 @@
   C c = C::get();
 }
 
+// FIXME: Find construction contexts for both branches in C++17.
+// Note that once it gets detected, the test for the get() branch would not
+// fail, because FileCheck allows partial matches.
 // CHECK: void simpleVariableWithTernaryOperator(bool coin)
 // CHECK:[B1]
 // CXX11-NEXT: 1: [B4.2] ? [B2.5] : [B3.6]
@@ -122,15 +125,15 @@
 // CXX11-NEXT: 3: [B2.2]() (CXXRecordTypedCall, [B2.4])
 // CXX11-NEXT: 4: [B2.3]
 // CXX11-NEXT: 5: [B2.4] (CXXConstructExpr, [B1.2], class C)
-// CXX17-NEXT: 3: [B2.2]() (CXXRecordTypedCall, [B1.2])
+// CXX17-NEXT: 3: [B2.2]()
 // CHECK:[B3]
 // CHECK-NEXT: 1: 0
 // CHECK-NEXT: 2: [B3.1] (ImplicitCastExpr, NullToPointer, class C *)
 // CXX11-NEXT: 3: [B3.2] (CXXConstructExpr, [B3.5], class C)
 // CXX11-NEXT: 4: C([B3.3]) (CXXFunctionalCastExpr, ConstructorConversion, class C)
 // CXX11-NEXT: 5: [B3.4]
 // CXX11-NEXT: 6: [B3.5] (CXXConstructExpr, [B1.2], class C)
-// CXX17-NEXT: 3: [B3.2] (CXXConstructExpr, [B1.2], class C)
+// CXX17-NEXT: 3: [B3.2] (CXXConstructExpr, class C)
 // CXX17-NEXT: 4: C([B3.3]) (CXXFunctionalCastExpr, ConstructorConversion, class C)
 // CHECK:[B4]
 // CHECK-NEXT: 1: coin
Index: test/Analysis/cxx17-mandatory-elision.cpp
===
--- test/Analysis/cxx17-mandatory-elision.cpp
+++ test/Analysis/cxx17-mandatory-elision.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s
+
+void clang_analyzer_eval(bool);
+
+template  struct AddressVector {
+  T *buf[10];
+  int len;
+
+  AddressVector() : len(0) {}
+
+  void push(T *t) {
+buf[len] = t;
+++len;
+  }
+};
+
+class ClassWithoutDestructor {
+  AddressVector &v;
+
+public:
+  ClassWithoutDestructor(AddressVector &v) : v(v) {
+v.push(this);
+  }
+
+  ClassWithoutDestructor(ClassWithoutDestructor &&c) : v(c.v) { v.push(this); }
+  ClassWithoutDestructor(const ClassWithoutDestructor &c) : v(c.v) {
+v.push(this);
+  }
+};
+
+ClassWithoutDestructor make1(AddressVector &v) {
+  return ClassWithoutDestructor(v);
+}
+ClassWithoutDestructor make2(AddressVector &v) {
+  return make1(v);
+}
+ClassWithoutDestructor make3(AddressVector &v) {
+  return make2(v);
+}
+
+void testMultipleReturns() {
+  AddressVector v;
+  ClassWithoutDestructor c = make3(v);
+
+#if __cplusplus >= 201703L
+  // FIXME: Both should be TRUE.
+  clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{FALSE}}
+#else
+  clang_analyzer_eval(v.len == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] != v.buf[1]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[1] != v.buf[2]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[2] != v.buf[3]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[3] != v.buf[4]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[4] == &c); // expected-warning{{TRUE}}
+#endif
+}
Index: test/Analysis/temp-obj-dtors-cfg-output.cpp
===
--- test/Analysis/temp-obj-dtors-cfg-output.cpp
+++ test/Analysis/temp-obj-dtors-cfg-output.cpp
@@ -218,6 +218,9 @@
 // Don't try to figure out how to perform construction of the record here.
 const C &bar1() { return foo1(); } // no-crash
 C &&bar2() { return foo2(); } // no-crash
+const C &bar3(bool coin) {
+  return coin ? foo1() : foo1(); // no-crash
+}
 } // end namespace pass_references_through
 
 // CHECK:   [B1 (ENTRY)]
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -203,13 +203,24 @@
   // TODO: What exactly happens when we are? Does the temporary object live
   // long enough in the region store in this case? Would checkers think
   // that this object immediately goes out of scope?
-  // TODO: We assume that the call site has a temporary object construction
-  // context. This is no longer true in 

[PATCH] D44854: [analyzer] Be more careful about C++17 copy elision.

2018-03-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328893: [CFG] [analyzer] Avoid modeling C++17 constructors 
that aren't fully supported. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44854?vs=140168&id=140469#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44854

Files:
  cfe/trunk/lib/Analysis/CFG.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
  cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp
  cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -203,13 +203,24 @@
   // TODO: What exactly happens when we are? Does the temporary object live
   // long enough in the region store in this case? Would checkers think
   // that this object immediately goes out of scope?
-  // TODO: We assume that the call site has a temporary object construction
-  // context. This is no longer true in C++17 or when copy elision is
-  // performed. We may need to unwrap multiple stack frames here and we
-  // won't necessarily end up with a temporary at the end.
   const LocationContext *TempLCtx = LCtx;
-  if (const LocationContext *CallerLCtx =
-  LCtx->getCurrentStackFrame()->getParent()) {
+  const StackFrameContext *SFC = LCtx->getCurrentStackFrame();
+  if (const LocationContext *CallerLCtx = SFC->getParent()) {
+auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()]
+   .getAs();
+if (!RTC) {
+  // We were unable to find the correct construction context for the
+  // call in the parent stack frame. This is equivalent to not being
+  // able to find construction context at all.
+  CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+} else if (!isa(
+   RTC->getConstructionContext())) {
+  // FXIME: The return value is constructed directly into a
+  // non-temporary due to C++17 mandatory copy elision. This is not
+  // implemented yet.
+  assert(getContext().getLangOpts().CPlusPlus17);
+  CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+}
 TempLCtx = CallerLCtx;
   }
   CallOpts.IsTemporaryCtorOrDtor = true;
Index: cfe/trunk/lib/Analysis/CFG.cpp
===
--- cfe/trunk/lib/Analysis/CFG.cpp
+++ cfe/trunk/lib/Analysis/CFG.cpp
@@ -1302,6 +1302,15 @@
   }
   case Stmt::ConditionalOperatorClass: {
 auto *CO = cast(Child);
+if (!dyn_cast_or_null(Layer->getTriggerStmt())) {
+  // If the object returned by the conditional operator is not going to be a
+  // temporary object that needs to be immediately materialized, then
+  // it must be C++17 with its mandatory copy elision. Do not yet promise
+  // to support this case.
+  assert(!CO->getType()->getAsCXXRecordDecl() || CO->isGLValue() ||
+ Context->getLangOpts().CPlusPlus17);
+  break;
+}
 findConstructionContexts(Layer, CO->getLHS());
 findConstructionContexts(Layer, CO->getRHS());
 break;
Index: cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp
===
--- cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp
+++ cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s
+
+void clang_analyzer_eval(bool);
+
+template  struct AddressVector {
+  T *buf[10];
+  int len;
+
+  AddressVector() : len(0) {}
+
+  void push(T *t) {
+buf[len] = t;
+++len;
+  }
+};
+
+class ClassWithoutDestructor {
+  AddressVector &v;
+
+public:
+  ClassWithoutDestructor(AddressVector &v) : v(v) {
+v.push(this);
+  }
+
+  ClassWithoutDestructor(ClassWithoutDestructor &&c) : v(c.v) { v.push(this); }
+  ClassWithoutDestructor(const ClassWithoutDestructor &c) : v(c.v) {
+v.push(this);
+  }
+};
+
+ClassWithoutDestructor make1(AddressVector &v) {
+  return ClassWithoutDestructor(v);
+}
+ClassWithoutDestructor make2(AddressVector &v) {
+  return make1(v);
+}
+ClassWithoutDestructor make3(AddressVector &v) {
+  return make2(v);
+}
+
+void testMultipleReturns() {
+  AddressVector v;
+  ClassWithoutDestructor c = make3(v);
+
+#if __cplusplus >= 201703L
+  // FIXME: Both should be TRUE.
+  clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{FALSE}}
+#else
+  clang_analyzer_eval(

r328895 - [CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr.

2018-03-30 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Fri Mar 30 12:25:39 2018
New Revision: 328895

URL: http://llvm.org/viewvc/llvm-project?rev=328895&view=rev
Log:
[CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr.

Sometimes template instantiation causes CXXBindTemporaryExpr to be missing in
its usual spot. In CFG, temporary destructors work by relying on
CXXBindTemporaryExprs, so they won't work in this case.

Avoid the crash and notify the clients that we've encountered an unsupported AST
by failing to provide the ill-formed construction context for the temporary.

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

Added:
cfe/trunk/test/Analysis/missing-bind-temporary.cpp
Modified:
cfe/trunk/include/clang/Analysis/ConstructionContext.h
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/Analysis/ConstructionContext.cpp

Modified: cfe/trunk/include/clang/Analysis/ConstructionContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ConstructionContext.h?rev=328895&r1=328894&r2=328895&view=diff
==
--- cfe/trunk/include/clang/Analysis/ConstructionContext.h (original)
+++ cfe/trunk/include/clang/Analysis/ConstructionContext.h Fri Mar 30 12:25:39 
2018
@@ -131,7 +131,8 @@ private:
 
 public:
   /// Consume the construction context layer, together with its parent layers,
-  /// and wrap it up into a complete construction context.
+  /// and wrap it up into a complete construction context. May return null
+  /// if layers do not form any supported construction context.
   static const ConstructionContext *
   createFromLayers(BumpVectorContext &C,
const ConstructionContextLayer *TopLayer);

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=328895&r1=328894&r2=328895&view=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Fri Mar 30 12:25:39 2018
@@ -736,12 +736,12 @@ private:
 if (BuildOpts.AddRichCXXConstructors) {
   if (const ConstructionContextLayer *Layer =
   ConstructionContextMap.lookup(CE)) {
-const ConstructionContext *CC =
-ConstructionContext::createFromLayers(cfg->getBumpVectorContext(),
-  Layer);
-B->appendConstructor(CE, CC, cfg->getBumpVectorContext());
 cleanupConstructionContext(CE);
-return;
+if (const auto *CC = ConstructionContext::createFromLayers(
+cfg->getBumpVectorContext(), Layer)) {
+  B->appendConstructor(CE, CC, cfg->getBumpVectorContext());
+  return;
+}
   }
 }
 
@@ -757,12 +757,12 @@ private:
   if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context)) {
 if (const ConstructionContextLayer *Layer =
 ConstructionContextMap.lookup(CE)) {
-  const ConstructionContext *CC =
-  
ConstructionContext::createFromLayers(cfg->getBumpVectorContext(),
-Layer);
-  B->appendCXXRecordTypedCall(CE, CC, cfg->getBumpVectorContext());
   cleanupConstructionContext(CE);
-  return;
+  if (const auto *CC = ConstructionContext::createFromLayers(
+  cfg->getBumpVectorContext(), Layer)) {
+B->appendCXXRecordTypedCall(CE, CC, cfg->getBumpVectorContext());
+return;
+  }
 }
   }
 }

Modified: cfe/trunk/lib/Analysis/ConstructionContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ConstructionContext.cpp?rev=328895&r1=328894&r2=328895&view=diff
==
--- cfe/trunk/lib/Analysis/ConstructionContext.cpp (original)
+++ cfe/trunk/lib/Analysis/ConstructionContext.cpp Fri Mar 30 12:25:39 2018
@@ -101,9 +101,12 @@ const ConstructionContext *ConstructionC
 if (const auto *MTE = dyn_cast(S)) {
   // If the object requires destruction and is not lifetime-extended,
   // then it must have a BTE within its MTE.
-  assert(MTE->getType().getCanonicalType()
+  // FIXME: This should be an assertion.
+  if (!(MTE->getType().getCanonicalType()
 ->getAsCXXRecordDecl()->hasTrivialDestructor() ||
- MTE->getStorageDuration() != SD_FullExpression);
+ MTE->getStorageDuration() != SD_FullExpression))
+return nullptr;
+
   assert(TopLayer->isLast());
   return create(C, nullptr, MTE);
 }

Added: cfe/trunk/test/Analysis/missing-bind-temporary.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/missing-bind-temporary.cpp?rev=328895&view=auto
==
--- cfe/trunk/test/Analysis/missing-bind-temporary.cpp (add

[PATCH] D44955: [CFG] [analyzer] Work around a disappearing CXXBindTemporaryExpr.

2018-03-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL328895: [CFG] [analyzer] Work around a disappearing 
CXXBindTemporaryExpr. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44955?vs=140464&id=140471#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44955

Files:
  cfe/trunk/include/clang/Analysis/ConstructionContext.h
  cfe/trunk/lib/Analysis/CFG.cpp
  cfe/trunk/lib/Analysis/ConstructionContext.cpp
  cfe/trunk/test/Analysis/missing-bind-temporary.cpp

Index: cfe/trunk/test/Analysis/missing-bind-temporary.cpp
===
--- cfe/trunk/test/Analysis/missing-bind-temporary.cpp
+++ cfe/trunk/test/Analysis/missing-bind-temporary.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++14 -verify %s
+
+void clang_analyzer_eval(bool);
+
+int global;
+
+namespace variant_0 {
+// This variant of the code works correctly. Function foo() is not a template
+// function. Note that there are two destructors within foo().
+
+class A {
+public:
+  ~A() { ++global; }
+};
+
+class B {
+  A a;
+};
+
+// CHECK: void foo(int)
+// CHECK:   [B1]
+// CHECK-NEXT:1:  (CXXConstructExpr, [B1.2], class variant_0::B)
+// CHECK-NEXT:2: variant_0::B i;
+// CHECK-NEXT:3: operator=
+// CHECK-NEXT:4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_0::B &(*)(class variant_0::B &&) noexcept)
+// CHECK-NEXT:5: i
+// CHECK-NEXT:6: {} (CXXConstructExpr, [B1.7], [B1.8], class variant_0::B)
+// CHECK-NEXT:7: [B1.6] (BindTemporary)
+// CHECK-NEXT:8: [B1.7]
+// CHECK-NEXT:9: [B1.5] = [B1.8] (OperatorCall)
+// CHECK-NEXT:   10: ~variant_0::B() (Temporary object destructor)
+// CHECK-NEXT:   11: [B1.2].~B() (Implicit destructor)
+void foo(int) {
+  B i;
+  i = {};
+}
+
+void bar() {
+  global = 0;
+  foo(1);
+  clang_analyzer_eval(global == 2); // expected-warning{{TRUE}}
+}
+
+} // end namespace variant_0
+
+namespace variant_1 {
+// Suddenly, if we turn foo() into a template, we are missing a
+// CXXBindTemporaryExpr in the AST, and therefore we're missing a
+// temporary destructor in the CFG.
+
+class A {
+public:
+  ~A() { ++global; }
+};
+
+class B {
+  A a;
+};
+
+// FIXME: Find the construction context for {} and enforce the temporary
+// destructor.
+// CHECK: template<> void foo(int)
+// CHECK:   [B1]
+// CHECK-NEXT:1:  (CXXConstructExpr, [B1.2], class variant_1::B)
+// CHECK-NEXT:2: variant_1::B i;
+// CHECK-NEXT:3: operator=
+// CHECK-NEXT:4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_1::B &(*)(class variant_1::B &&) noexcept)
+// CHECK-NEXT:5: i
+// CHECK-NEXT:6: {} (CXXConstructExpr, class variant_1::B)
+// CHECK-NEXT:7: [B1.6]
+// CHECK-NEXT:8: [B1.5] = [B1.7] (OperatorCall)
+// CHECK-NEXT:9: [B1.2].~B() (Implicit destructor)
+template  void foo(T) {
+  B i;
+  i = {};
+}
+
+void bar() {
+  global = 0;
+  foo(1);
+  // FIXME: Should be TRUE, i.e. we should call (and inline) two destructors.
+  clang_analyzer_eval(global == 2); // expected-warning{{UNKNOWN}}
+}
+
+} // end namespace variant_1
+
+namespace variant_2 {
+// Making field 'a' in class 'B' public turns the class into an aggregate.
+// In this case there is no constructor at {} - only an aggregate
+// initialization. Aggregate initialization is unsupported for now.
+
+class A {
+public:
+  ~A() { ++global; }
+};
+
+class B {
+public:
+  A a;
+};
+
+// CHECK: template<> void foo(int)
+// CHECK:   [B1]
+// CHECK-NEXT:1:  (CXXConstructExpr, [B1.2], class variant_2::B)
+// CHECK-NEXT:2: variant_2::B i;
+// CHECK-NEXT:3: operator=
+// CHECK-NEXT:4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, class variant_2::B &(*)(class variant_2::B &&) noexcept)
+// CHECK-NEXT:5: i
+// CHECK-NEXT:6: {}
+// CHECK-NEXT:7: {}
+// CHECK-NEXT:8: [B1.7] (BindTemporary)
+// CHECK-NEXT:9: [B1.8]
+// CHECK-NEXT:   10: [B1.5] = [B1.9] (OperatorCall)
+// CHECK-NEXT:   11: ~variant_2::B() (Temporary object destructor)
+// CHECK-NEXT:   12: [B1.2].~B() (Implicit destructor)
+template  void foo(T) {
+  B i;
+  i = {};
+}
+
+void bar() {
+  global = 0;
+  foo(1);
+  // FIXME: Should be TRUE, i.e. we should call (and inline) two destructors.
+  clang_analyzer_eval(global == 2); // expected-warning{{UNKNOWN}}
+}
+
+} // end namespace variant_2
Index: cfe/trunk/lib/Analysis/ConstructionContext.cpp
===
--- cfe/trunk/lib/Analysis/ConstructionContext.cpp
+++ cfe/trunk/lib/Analysis/ConstructionContext.cpp
@@ -101,9 +101,12 @@
 if (const auto *MTE = dyn_cast(S)) {
   // If the object requires destruction and is not lifetime-extended,
   // then i

r328896 - [analyzer] Track null or undef values through pointer arithmetic.

2018-03-30 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Fri Mar 30 12:27:42 2018
New Revision: 328896

URL: http://llvm.org/viewvc/llvm-project?rev=328896&view=rev
Log:
[analyzer] Track null or undef values through pointer arithmetic.

Pointer arithmetic on null or undefined pointers results in null or undefined
pointers. This is obvious for undefined pointers; for null pointers it follows
from our incorrect-but-somehow-working approach that declares that 0 (Loc)
doesn't necessarily represent a pointer of numeric address value 0, but instead
it represents any pointer that will cause a valid "null pointer dereference"
issue when dereferenced.

For now we've been seeing through pointer arithmetic at the original dereference
expression, i.e. in bugreporter::getDerefExpr(), but not during further
investigation of the value's origins in bugreporter::trackNullOrUndefValue().
The patch fixes it.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c
cfe/trunk/test/Analysis/null-deref-path-notes.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=328896&r1=328895&r2=328896&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Mar 30 
12:27:42 2018
@@ -75,6 +75,17 @@ bool bugreporter::isDeclRefExprToReferen
   return false;
 }
 
+static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
+  if (B->isAdditiveOp() && B->getType()->isPointerType()) {
+if (B->getLHS()->getType()->isPointerType()) {
+  return B->getLHS();
+} else if (B->getRHS()->getType()->isPointerType()) {
+  return B->getRHS();
+}
+  }
+  return nullptr;
+}
+
 /// Given that expression S represents a pointer that would be dereferenced,
 /// try to find a sub-expression from which the pointer came from.
 /// This is used for tracking down origins of a null or undefined value:
@@ -101,14 +112,8 @@ const Expr *bugreporter::getDerefExpr(co
   E = CE->getSubExpr();
 } else if (const auto *B = dyn_cast(E)) {
   // Pointer arithmetic: '*(x + 2)' -> 'x') etc.
-  if (B->getType()->isPointerType()) {
-if (B->getLHS()->getType()->isPointerType()) {
-  E = B->getLHS();
-} else if (B->getRHS()->getType()->isPointerType()) {
-  E = B->getRHS();
-} else {
-  break;
-}
+  if (const Expr *Inner = peelOffPointerArithmetic(B)) {
+E = Inner;
   } else {
 // Probably more arithmetic can be pattern-matched here,
 // but for now give up.
@@ -1412,6 +1417,11 @@ static const Expr *peelOffOuterExpr(cons
   NI = NI->getFirstPred();
 } while (NI);
   }
+
+  if (auto *BO = dyn_cast(Ex))
+if (const Expr *SubEx = peelOffPointerArithmetic(BO))
+  return peelOffOuterExpr(SubEx, N);
+
   return Ex;
 }
 

Modified: cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c?rev=328896&r1=328895&r2=328896&view=diff
==
--- cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c (original)
+++ cfe/trunk/test/Analysis/inlining/inline-defensive-checks.c Fri Mar 30 
12:27:42 2018
@@ -159,8 +159,7 @@ void idcTrackZeroValueThroughUnaryPointe
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
   idc(s);
   int *x = &(s->f2) - 1;
-  // FIXME: Should not warn.
-  *x = 7; // expected-warning{{Dereference of null pointer}}
+  *x = 7; // no-warning
 }
 
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {

Modified: cfe/trunk/test/Analysis/null-deref-path-notes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-path-notes.c?rev=328896&r1=328895&r2=328896&view=diff
==
--- cfe/trunk/test/Analysis/null-deref-path-notes.c (original)
+++ cfe/trunk/test/Analysis/null-deref-path-notes.c Fri Mar 30 12:27:42 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core 
-analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core,unix 
-analyzer-output=text -verify %s
 
 // Avoid the crash when finding the expression for tracking the origins
 // of the null pointer for path notes.
@@ -7,3 +7,46 @@ void pr34373() {
   (a + 0)[0]; // expected-warning{{Array access results in a null pointer 
dereference}}
   // expected-note@-1{{Array access results in a null pointer 
dereference}}
 }
+
+typedef __typeof(sizeof(int)) size_t;
+void *memcpy(void *dest, const void *src, uns

[PATCH] D45071: [analyzer] Track null or undef values through pointer arithmetic.

2018-03-30 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328896: [analyzer] Track null or undef values through 
pointer arithmetic. (authored by dergachev, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45071

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/inlining/inline-defensive-checks.c
  test/Analysis/null-deref-path-notes.c

Index: test/Analysis/inlining/inline-defensive-checks.c
===
--- test/Analysis/inlining/inline-defensive-checks.c
+++ test/Analysis/inlining/inline-defensive-checks.c
@@ -159,8 +159,7 @@
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
   idc(s);
   int *x = &(s->f2) - 1;
-  // FIXME: Should not warn.
-  *x = 7; // expected-warning{{Dereference of null pointer}}
+  *x = 7; // no-warning
 }
 
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {
Index: test/Analysis/null-deref-path-notes.c
===
--- test/Analysis/null-deref-path-notes.c
+++ test/Analysis/null-deref-path-notes.c
@@ -1,9 +1,52 @@
-// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core,unix -analyzer-output=text -verify %s
 
 // Avoid the crash when finding the expression for tracking the origins
 // of the null pointer for path notes.
 void pr34373() {
   int *a = 0; // expected-note{{'a' initialized to a null pointer value}}
   (a + 0)[0]; // expected-warning{{Array access results in a null pointer dereference}}
   // expected-note@-1{{Array access results in a null pointer dereference}}
 }
+
+typedef __typeof(sizeof(int)) size_t;
+void *memcpy(void *dest, const void *src, unsigned long count);
+
+void f1(char *source) {
+  char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}}
+  memcpy(destination + 0, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f2(char *source) {
+  char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}}
+  memcpy(destination - 0, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f3(char *source) {
+  char *destination = 0; // FIXME: There should be a note here as well.
+  destination = destination + 0; // expected-note{{Null pointer value stored to 'destination'}}
+  memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f4(char *source) {
+  char *destination = 0; // FIXME: There should be a note here as well.
+  destination = destination - 0; // expected-note{{Null pointer value stored to 'destination'}}
+  memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f5(char *source) {
+  char *destination1 = 0; // expected-note{{'destination1' initialized to a null pointer value}}
+  char *destination2 = destination1 + 0; // expected-note{{'destination2' initialized to a null pointer value}}
+  memcpy(destination2, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+// expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f6(char *source) {
+  char *destination1 = 0; // expected-note{{'destination1' initialized to a null pointer value}}
+  char *destination2 = destination1 - 0; // expected-note{{'destination2' initialized to a null pointer value}}
+  memcpy(destination2, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+// expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -75,6 +75,17 @@
   return false;
 }
 
+static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
+  if (B->isAdditiveOp() && B->getType()->isPointerType()) {
+if (B->getLHS()->getType()->isPointerType()) {
+  return B->getLHS();
+} else if (B->getRHS()->getType()->isPointerType()) {
+  return B->getRHS();
+}
+  }
+  return nullptr;
+}
+
 /// G

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-30 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 140475.
zinovy.nis marked 3 inline comments as done.
zinovy.nis added a comment.

- Cosmetic fixes.


https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,177 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to direct parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' refers to a member overridden in subclass; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Check aliased type names
+using A1 = A;
+typedef A A2;
+
+class C2 : public B {
+public:
+  int virt_1() override { return A1::virt_1() + A2::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:49: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' {{.*}}; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name 'B::virt_1' {{.*}}; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace
+
+namespace N1 {
+class A {
+public:
+  A() = default;
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+};
+} // namespace N1
+
+namespace N2 {
+class BN : public N1::A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N2
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'A::virt_1' {{.*}}; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+class CNN : public N2::BN {
+public:
+  int virt_1() override { return N1::A::virt_1() + N2::BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name 'N1::A::virt_1' {{.*}}; did you mean 'N2::BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return N2::BN::virt_1() + 

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-30 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis added a comment.

BTW, I've recently found few dozens of issues in the Chromium code with my 
check.
For ex.:

  browser/src/cc/layers/nine_patch_layer_impl.cc:89:51: warning: qualified name 
'cc::LayerImpl::LayerAsJson' refers to a member overridden in subclass; did you 
mean 'cc::UIResourceLayerImpl'? [bugprone-parent-virtual-call]
std::unique_ptr result = LayerImpl::LayerAsJson();
^~
cc::UIResourceLayerImpl::


https://reviews.llvm.org/D44295



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


[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-30 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

> I'm not quite sure how we would go about that with confidence. The code we'd 
> need to catch looks something like:
> 
> typeof(foo() == y) x;
>  /* snip */
>  bar(x == -1); // warning: comparison is pointless
> 
> If we could tell that x's type was inferred from a comparison op, we could 
> warn here. However, if the user used x as anything but a C++ bool in /* snip 
> */, the warning would be incorrect. We could maybe do some sort of range 
> analysis by walking the AST, but that sounds like more like a static analyzer 
> thing.
> 
> ...And none of that would catch glibc's TEMP_FAILURE_RETRY macro, unless we 
> wanted to apply said analysis to all integral variables.

Yes that sounds reasonable. As i am no good C programer i have little insight 
in these issues. Please correct me if i am wrong!

C89 has no `bool` type and no `stdbool.h` but it has been added to later 
standards? That means the generalization //could// theoretically be done for 
later C standards, because we could check if the `bool` typedef had been used? 
If yes, would the check benefit?

I do not want to overthink the whole issue, because your check addresses a 
valid usecase and has general appliance. But if we can do more/better, we 
should do it :)

> If you have anything in mind, I'm happy to add it.

No clue :D I think @aaron.ballman knows C good and might be a better reviewer 
for this code in general :)


https://reviews.llvm.org/D45059



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


r328903 - [analyzer] Fix test triple in missing-bind-temporary.cpp.

2018-03-30 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Fri Mar 30 14:22:35 2018
New Revision: 328903

URL: http://llvm.org/viewvc/llvm-project?rev=328903&view=rev
Log:
[analyzer] Fix test triple in missing-bind-temporary.cpp.

Otherwise the default triple for x86-windows-msvc2015 auto-inserts
__attribute__((thiscall)) to some calls.

Fixes the respective buildbot.

Modified:
cfe/trunk/test/Analysis/missing-bind-temporary.cpp

Modified: cfe/trunk/test/Analysis/missing-bind-temporary.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/missing-bind-temporary.cpp?rev=328903&r1=328902&r2=328903&view=diff
==
--- cfe/trunk/test/Analysis/missing-bind-temporary.cpp (original)
+++ cfe/trunk/test/Analysis/missing-bind-temporary.cpp Fri Mar 30 14:22:35 2018
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux 
-analyzer-checker=debug.DumpCFG %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-std=c++14 -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux 
-analyzer-checker=core,debug.ExprInspection -std=c++14 -verify %s
 
 void clang_analyzer_eval(bool);
 


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


[PATCH] D45059: [clang-tidy] Add check to catch comparisons in TEMP_FAILURE_RETRY

2018-03-30 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added a comment.

> C89 has no bool type and no stdbool.h but it has been added to later 
> standards? That means the generalization could theoretically be done for 
> later C standards, because we could check if the bool typedef had been used? 
> If yes, would the check benefit?

Sure, and we do complain about `x == -1` if the type of `x` is `_Bool`, but the 
type of `x == y` appears to be an `int` in all of {c89, c99, c11}: 
https://godbolt.org/g/BciGZT :)


https://reviews.llvm.org/D45059



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


[PATCH] D45109: Remove -cc1 option "-backend-option"

2018-03-30 Thread Eli Friedman via Phabricator via cfe-commits
efriedma created this revision.
Herald added subscribers: eraman, javed.absar, mehdi_amini.

It means essentially the same thing as -mllvm; there isn't any reason to have 
separate options.


Repository:
  rC Clang

https://reviews.llvm.org/D45109

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto-backend-option.ll
  test/CodeGenCUDA/link-device-bitcode.cu
  test/Driver/aarch64-fix-cortex-a53-835769.c
  test/Driver/apple-kext-mkernel.c
  test/Driver/arm-restrict-it.c
  test/Driver/debug-options.c
  test/Driver/mglobal-merge.c
  test/Driver/woa-restrict-it.c
  test/Frontend/backend-option.c

Index: test/Frontend/backend-option.c
===
--- test/Frontend/backend-option.c
+++ /dev/null
@@ -1,4 +0,0 @@
-// RUN: %clang_cc1 %s -emit-llvm -backend-option -time-passes -o - 2>&1 | FileCheck %s
-// RUN: %clang_cc1 %s -emit-llvm -backend-option -time-passes -o - -triple spir-unknown-unknown 2>&1 | FileCheck %s
-// CHECK: Pass execution timing report
-
Index: test/Driver/woa-restrict-it.c
===
--- test/Driver/woa-restrict-it.c
+++ test/Driver/woa-restrict-it.c
@@ -1,4 +1,4 @@
 // RUN: %clang -target armv7-windows -### %s 2>&1 | FileCheck %s
 
-// CHECK: "-backend-option" "-arm-restrict-it"
+// CHECK: "-mllvm" "-arm-restrict-it"
 
Index: test/Driver/mglobal-merge.c
===
--- test/Driver/mglobal-merge.c
+++ test/Driver/mglobal-merge.c
@@ -10,8 +10,8 @@
 // RUN:   -mno-global-merge
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
 
-// CHECK-NGM-ARM: "-backend-option" "-arm-global-merge=false"
-// CHECK-NGM-AARCH64: "-backend-option" "-aarch64-enable-global-merge=false"
+// CHECK-NGM-ARM: "-mllvm" "-arm-global-merge=false"
+// CHECK-NGM-AARCH64: "-mllvm" "-aarch64-enable-global-merge=false"
 
 // RUN: %clang -target armv7-unknown-unknown -### -fsyntax-only %s 2> %t \
 // RUN:   -mglobal-merge
@@ -25,8 +25,8 @@
 // RUN:   -mglobal-merge
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
 
-// CHECK-GM-ARM: "-backend-option" "-arm-global-merge=true"
-// CHECK-GM-AARCH64: "-backend-option" "-aarch64-enable-global-merge=true"
+// CHECK-GM-ARM: "-mllvm" "-arm-global-merge=true"
+// CHECK-GM-AARCH64: "-mllvm" "-aarch64-enable-global-merge=true"
 
 // RUN: %clang -target armv7-unknown-unknown -### -fsyntax-only %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-NONE < %t %s
Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -225,9 +225,9 @@
 //
 // GARANGE: -generate-arange-section
 //
-// FDTS: "-backend-option" "-generate-type-units"
+// FDTS: "-mllvm" "-generate-type-units"
 //
-// NOFDTS-NOT: "-backend-option" "-generate-type-units"
+// NOFDTS-NOT: "-mllvm" "-generate-type-units"
 //
 // CI: "-dwarf-column-info"
 //
Index: test/Driver/arm-restrict-it.c
===
--- test/Driver/arm-restrict-it.c
+++ test/Driver/arm-restrict-it.c
@@ -4,12 +4,12 @@
 // RUN: %clang -target armv8a-none-gnueabi -mrestrict-it -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-RESTRICTED < %t %s
 
-// CHECK-RESTRICTED: "-backend-option" "-arm-restrict-it"
+// CHECK-RESTRICTED: "-mllvm" "-arm-restrict-it"
 
 // RUN: %clang -target arm-none-gnueabi -mno-restrict-it -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-NO-RESTRICTED < %t %s
 
 // RUN: %clang -target armv8a-none-gnueabi -mno-restrict-it -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-NO-RESTRICTED < %t %s
 
-// CHECK-NO-RESTRICTED: "-backend-option" "-arm-no-restrict-it"
+// CHECK-NO-RESTRICTED: "-mllvm" "-arm-no-restrict-it"
Index: test/Driver/apple-kext-mkernel.c
===
--- test/Driver/apple-kext-mkernel.c
+++ test/Driver/apple-kext-mkernel.c
@@ -26,7 +26,3 @@
 // RUN: %clang -target x86_64-apple-darwin10 \
 // RUN:   -Werror -fno-builtin -fno-exceptions -fno-common -fno-rtti \
 // RUN:   -mkernel -fsyntax-only %s
-
-// RUN: %clang -c %s -target armv7k-apple-watchos -fapple-kext -mwatchos-version-min=1.0.0 -### 2>&1 \
-// RUN:   | FileCheck %s --check-prefix=CHECK-WATCH
-// CHECK-WATCH-NOT: "-backend-option" "-arm-long-calls"
Index: test/Driver/aarch64-fix-cortex-a53-835769.c
===
--- test/Driver/aarch64-fix-cortex-a53-835769.c
+++ test/Driver/aarch64-fix-cortex-a53-835769.c
@@ -8,6 +8,6 @@
 // RUN: %clang -target aarch64-android-eabi %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-YES %s
 
-// CHECK-DEF-NOT: "-backend-option" "-aarch64-fix-cortex-a53-835769"
-// CHECK-YES: "-backend-option" "-aarch64-fix-cortex-a53-835769=

[PATCH] D45061: [NVPTX, CUDA] Use custom feature detection to handle NVPTX target builtins.

2018-03-30 Thread Artem Belevich via Phabricator via cfe-commits
tra updated this revision to Diff 140493.
tra added a comment.

Commented out unused argument.


https://reviews.llvm.org/D45061

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/test/CodeGen/builtins-nvptx-ptx50.cu
  clang/test/CodeGen/builtins-nvptx.c
  llvm/lib/Target/NVPTX/NVPTX.td
  llvm/lib/Target/NVPTX/NVPTXSubtarget.h

Index: llvm/lib/Target/NVPTX/NVPTXSubtarget.h
===
--- llvm/lib/Target/NVPTX/NVPTXSubtarget.h
+++ llvm/lib/Target/NVPTX/NVPTXSubtarget.h
@@ -48,10 +48,6 @@
   // FrameLowering class because TargetFrameLowering is abstract.
   NVPTXFrameLowering FrameLowering;
 
-protected:
-  // Processor supports scoped atomic operations.
-  bool HasAtomScope;
-
 public:
   /// This constructor initializes the data members to match that
   /// of the specified module.
@@ -74,7 +70,7 @@
   }
 
   bool hasAtomAddF64() const { return SmVersion >= 60; }
-  bool hasAtomScope() const { return HasAtomScope; }
+  bool hasAtomScope() const { return SmVersion >= 60; }
   bool hasAtomBitwise64() const { return SmVersion >= 32; }
   bool hasAtomMinMax64() const { return SmVersion >= 32; }
   bool hasLDG() const { return SmVersion >= 32; }
Index: llvm/lib/Target/NVPTX/NVPTX.td
===
--- llvm/lib/Target/NVPTX/NVPTX.td
+++ llvm/lib/Target/NVPTX/NVPTX.td
@@ -53,9 +53,6 @@
 def SM70 : SubtargetFeature<"sm_70", "SmVersion", "70",
  "Target SM 7.0">;
 
-def SATOM : SubtargetFeature<"satom", "HasAtomScope", "true",
- "Atomic operations with scope">;
-
 // PTX Versions
 def PTX32 : SubtargetFeature<"ptx32", "PTXVersion", "32",
  "Use PTX version 3.2">;
@@ -88,10 +85,10 @@
 def : Proc<"sm_50", [SM50, PTX40]>;
 def : Proc<"sm_52", [SM52, PTX41]>;
 def : Proc<"sm_53", [SM53, PTX42]>;
-def : Proc<"sm_60", [SM60, PTX50, SATOM]>;
-def : Proc<"sm_61", [SM61, PTX50, SATOM]>;
-def : Proc<"sm_62", [SM62, PTX50, SATOM]>;
-def : Proc<"sm_70", [SM70, PTX60, SATOM]>;
+def : Proc<"sm_60", [SM60, PTX50]>;
+def : Proc<"sm_61", [SM61, PTX50]>;
+def : Proc<"sm_62", [SM62, PTX50]>;
+def : Proc<"sm_70", [SM70, PTX60]>;
 
 def NVPTXInstrInfo : InstrInfo {
 }
Index: clang/test/CodeGen/builtins-nvptx.c
===
--- clang/test/CodeGen/builtins-nvptx.c
+++ clang/test/CodeGen/builtins-nvptx.c
@@ -5,6 +5,9 @@
 // RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_60 \
 // RUN:-fcuda-is-device -S -emit-llvm -o - -x cuda %s \
 // RUN:   | FileCheck -check-prefix=CHECK -check-prefix=LP64 %s
+// RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_61 \
+// RUN:-fcuda-is-device -S -emit-llvm -o - -x cuda %s \
+// RUN:   | FileCheck -check-prefix=CHECK -check-prefix=LP64 %s
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown -target-cpu sm_53 \
 // RUN:   -DERROR_CHECK -fcuda-is-device -S -o /dev/null -x cuda -verify %s
 
@@ -292,245 +295,245 @@
 #if ERROR_CHECK || __CUDA_ARCH__ >= 600
 
   // CHECK: call i32 @llvm.nvvm.atomic.add.gen.i.cta.i32.p0i32
-  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_i' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_i' needs target feature sm_60}}
   __nvvm_atom_cta_add_gen_i(ip, i);
   // LP32: call i32 @llvm.nvvm.atomic.add.gen.i.cta.i32.p0i32
   // LP64: call i64 @llvm.nvvm.atomic.add.gen.i.cta.i64.p0i64
-  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_l' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_l' needs target feature sm_60}}
   __nvvm_atom_cta_add_gen_l(&dl, l);
   // CHECK: call i64 @llvm.nvvm.atomic.add.gen.i.cta.i64.p0i64
-  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_ll' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_cta_add_gen_ll' needs target feature sm_60}}
   __nvvm_atom_cta_add_gen_ll(&sll, ll);
   // CHECK: call i32 @llvm.nvvm.atomic.add.gen.i.sys.i32.p0i32
-  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_i' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_i' needs target feature sm_60}}
   __nvvm_atom_sys_add_gen_i(ip, i);
   // LP32: call i32 @llvm.nvvm.atomic.add.gen.i.sys.i32.p0i32
   // LP64: call i64 @llvm.nvvm.atomic.add.gen.i.sys.i64.p0i64
-  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_l' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_l' needs target feature sm_60}}
   __nvvm_atom_sys_add_gen_l(&dl, l);
   // CHECK: call i64 @llvm.nvvm.atomic.add.gen.i.sys.i64.p0i64
-  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_ll' needs target feature satom}}
+  // expected-error@+1 {{'__nvvm_atom_sys_add_gen_ll' needs target feature sm_60}}
   __nvvm_atom_sys_add_gen_ll(&sll, ll);
 
   // CHECK: call float @llvm.nvvm.atomic.add.gen.f.cta.f32.p0f32
-  // 

r328906 - [ASTImporter] Add test helper Fixture

2018-03-30 Thread Peter Szecsi via cfe-commits
Author: szepet
Date: Fri Mar 30 15:03:29 2018
New Revision: 328906

URL: http://llvm.org/viewvc/llvm-project?rev=328906&view=rev
Log:
[ASTImporter] Add test helper Fixture

Add a helper test Fixture, so we can add tests which can check internal
attributes of AST nodes like getPreviousDecl(), isVirtual(), etc.
This enables us to check if a redeclaration chain is correctly built during
import, if the virtual flag is preserved during import, etc. We cannot check
such attributes with the existing testImport.
Also, this fixture makes it possible to import from several "From" contexts.

We also added several test cases here, some of them are disabled.
We plan to pass the disabled tests in other patches.

Patch by Gabor Marton!

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


Added:
cfe/trunk/unittests/AST/DeclMatcher.h
Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=328906&r1=328905&r2=328906&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Fri Mar 30 15:03:29 2018
@@ -17,6 +17,8 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+
+#include "DeclMatcher.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -29,7 +31,7 @@ static bool isCXX(Language Lang) {
   return Lang == Lang_CXX || Lang == Lang_CXX11;
 }
 
-static RunOptions getRunOptionsForLanguage(Language Lang) {
+static ArgVector getBasicRunOptionsForLanguage(Language Lang) {
   ArgVector BasicArgs;
   // Test with basic arguments.
   switch (Lang) {
@@ -49,6 +51,11 @@ static RunOptions getRunOptionsForLangua
   case Lang_OBJCXX:
 llvm_unreachable("Not implemented yet!");
   }
+  return BasicArgs;
+}
+
+static RunOptions getRunOptionsForLanguage(Language Lang) {
+  ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang);
 
   // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
   // default behaviour.
@@ -61,6 +68,19 @@ static RunOptions getRunOptionsForLangua
   return {BasicArgs};
 }
 
+// Creates a virtual file and assigns that to the context of given AST. If the
+// file already exists then the file will not be created again as a duplicate.
+static void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
+  const std::string &Code) {
+  assert(ToAST);
+  ASTContext &ToCtx = ToAST->getASTContext();
+  auto *OFS = static_cast(
+  ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
+  auto *MFS =
+  static_cast(OFS->overlays_begin()->get());
+  MFS->addFile(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Code.c_str()));
+}
+
 template
 testing::AssertionResult
 testImport(const std::string &FromCode, const ArgVector &FromArgs,
@@ -79,11 +99,7 @@ testImport(const std::string &FromCode,
 
   // Add input.cc to virtual file system so importer can 'find' it
   // while importing SourceLocations.
-  vfs::OverlayFileSystem *OFS = static_cast(
-
ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
-  vfs::InMemoryFileSystem *MFS = static_cast(
-OFS->overlays_begin()->get());
-  MFS->addFile(InputFileName, 0, llvm::MemoryBuffer::getMemBuffer(FromCode));
+  createVirtualFileIfNeeded(ToAST.get(), InputFileName, FromCode);
 
   ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
@@ -133,6 +149,165 @@ void testImport(const std::string &FromC
  Verifier, AMatcher));
 }
 
+const StringRef DeclToImportID = "declToImport";
+
+// This class provides generic methods to write tests which can check internal
+// attributes of AST nodes like getPreviousDecl(), isVirtual(), etc.  Also,
+// this fixture makes it possible to import from several "From" contexts.
+class ASTImporterTestBase : public ::testing::TestWithParam {
+
+  const char *const InputFileName = "input.cc";
+  const char *const OutputFileName = "output.cc";
+
+  // Buffer for the To context, must live in the test scope.
+  std::string ToCode;
+
+  struct TU {
+// Buffer for the context, must live in the test scope.
+std::string Code;
+std::string FileName;
+std::unique_ptr Unit;
+TranslationUnitDecl *TUDecl = nullptr;
+TU(StringRef Code, StringRef FileName, ArgVector Args)
+: Code(Code), FileName(FileName),
+  Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
+ this->FileName)),
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+  };
+
+  // We may have several From contexts and related translation units. In each
+  // AST, the buffers for the source are handled via references and are 

[PATCH] D43967: [ASTImporter] Add test helper Fixture

2018-03-30 Thread Peter Szecsi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328906: [ASTImporter] Add test helper Fixture (authored by 
szepet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43967?vs=139791&id=140497#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43967

Files:
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/DeclMatcher.h

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -17,6 +17,8 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+
+#include "DeclMatcher.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -29,7 +31,7 @@
   return Lang == Lang_CXX || Lang == Lang_CXX11;
 }
 
-static RunOptions getRunOptionsForLanguage(Language Lang) {
+static ArgVector getBasicRunOptionsForLanguage(Language Lang) {
   ArgVector BasicArgs;
   // Test with basic arguments.
   switch (Lang) {
@@ -49,6 +51,11 @@
   case Lang_OBJCXX:
 llvm_unreachable("Not implemented yet!");
   }
+  return BasicArgs;
+}
+
+static RunOptions getRunOptionsForLanguage(Language Lang) {
+  ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang);
 
   // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
   // default behaviour.
@@ -61,6 +68,19 @@
   return {BasicArgs};
 }
 
+// Creates a virtual file and assigns that to the context of given AST. If the
+// file already exists then the file will not be created again as a duplicate.
+static void createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
+  const std::string &Code) {
+  assert(ToAST);
+  ASTContext &ToCtx = ToAST->getASTContext();
+  auto *OFS = static_cast(
+  ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
+  auto *MFS =
+  static_cast(OFS->overlays_begin()->get());
+  MFS->addFile(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Code.c_str()));
+}
+
 template
 testing::AssertionResult
 testImport(const std::string &FromCode, const ArgVector &FromArgs,
@@ -79,11 +99,7 @@
 
   // Add input.cc to virtual file system so importer can 'find' it
   // while importing SourceLocations.
-  vfs::OverlayFileSystem *OFS = static_cast(
-ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
-  vfs::InMemoryFileSystem *MFS = static_cast(
-OFS->overlays_begin()->get());
-  MFS->addFile(InputFileName, 0, llvm::MemoryBuffer::getMemBuffer(FromCode));
+  createVirtualFileIfNeeded(ToAST.get(), InputFileName, FromCode);
 
   ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
@@ -133,6 +149,165 @@
  Verifier, AMatcher));
 }
 
+const StringRef DeclToImportID = "declToImport";
+
+// This class provides generic methods to write tests which can check internal
+// attributes of AST nodes like getPreviousDecl(), isVirtual(), etc.  Also,
+// this fixture makes it possible to import from several "From" contexts.
+class ASTImporterTestBase : public ::testing::TestWithParam {
+
+  const char *const InputFileName = "input.cc";
+  const char *const OutputFileName = "output.cc";
+
+  // Buffer for the To context, must live in the test scope.
+  std::string ToCode;
+
+  struct TU {
+// Buffer for the context, must live in the test scope.
+std::string Code;
+std::string FileName;
+std::unique_ptr Unit;
+TranslationUnitDecl *TUDecl = nullptr;
+TU(StringRef Code, StringRef FileName, ArgVector Args)
+: Code(Code), FileName(FileName),
+  Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
+ this->FileName)),
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+  };
+
+  // We may have several From contexts and related translation units. In each
+  // AST, the buffers for the source are handled via references and are set
+  // during the creation of the AST. These references must point to a valid
+  // buffer until the AST is alive. Thus, we must use a list in order to avoid
+  // moving of the stored objects because that would mean breaking the
+  // references in the AST. By using a vector a move could happen when the
+  // vector is expanding, with the list we won't have these issues.
+  std::list FromTUs;
+
+public:
+  // We may have several From context but only one To context.
+  std::unique_ptr ToAST;
+
+  // Returns the argument vector used for a specific language, this set
+  // can be tweaked by the test parameters.
+  ArgVector getArgVectorForLanguage(Language Lang) {
+ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+ArgVector ExtraArgs = GetParam();
+for (const auto& Arg : ExtraArgs) {
+  Args.push_back(Arg);
+}
+return Args;
+  }
+
+  // Creates an AST both for the Fr

[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

2018-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.
rnk added reviewers: rjmccall, rsmith, hans.
Herald added a subscriber: Prazek.

The following class hierarchy requires that we be able to emit a
this-adjusting thunk for B::foo in C's vftable:

  struct Incomplete;
  struct A {
virtual A* foo(Incomplete p) = 0;
  };
  struct B : virtual A {
void foo(Incomplete p) override;
  };
  struct C : B { int c; };

This TU is valid, but lacks a definition of 'Incomplete', which makes it
hard to build a thunk for the final overrider, B::foo.

Before this change, Clang gives up attempting to emit the thunk, because
it assumes that if the parameter types are incomplete, it must be
emitting the thunk for optimization purposes. This is untrue for the MS
ABI, where the implementation of B::foo has no idea what thunks C's
vftable may require. Clang needs to emit the thunk without necessarily
having access to the complete prototype of foo.

This change makes Clang emit a musttail variadic call when it needs such
a thunk. I call these "unprototyped" thunks, because they only prototype
the "this" parameter, which must always come first in the MS C++ ABI.

These thunks work, but they create ugly LLVM IR. If the call to the
thunk is devirtualized, it will be a call to a bitcast of a function
pointer. Today, LLVM cannot inline through such a call, but I want to
address that soon, because we also use this pattern for virtual member
pointer thunks.

This change also implements an old FIXME in the code about reusing the
thunk's computed CGFunctionInfo as much as possible. Now we don't end up
computing the thunk's mangled name and arranging it's prototype up to
around three times.

Fixes PR36952


https://reviews.llvm.org/D45112

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/CodeGenTypes.h
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/ms-thunks-unprototyped-return.cpp
  clang/test/CodeGenCXX/ms-thunks-unprototyped.cpp

Index: clang/test/CodeGenCXX/ms-thunks-unprototyped.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-thunks-unprototyped.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -fno-rtti-data -triple x86_64-windows-msvc -emit-llvm %s -o - | FileCheck %s
+
+// In this example, C does not override B::foo, but it needs to emit a thunk to
+// adjust for the relative difference of position between A-in-B and A-in-C.
+
+struct Incomplete;
+template 
+struct DoNotInstantiate {
+  typename T::does_not_exist field;
+};
+template 
+struct InstantiateLater;
+
+struct A {
+  virtual void foo(Incomplete p) = 0;
+  virtual void bar(DoNotInstantiate p) = 0;
+  virtual int baz(InstantiateLater p) = 0;
+};
+struct B : virtual A {
+  void foo(Incomplete p) override;
+  void bar(DoNotInstantiate p) override;
+  inline int baz(InstantiateLater p) override;
+};
+struct C : B { int c; };
+C c;
+
+// CHECK: @"??_7C@@6B@" = linkonce_odr unnamed_addr constant
+// CHECK-SAME: void (%struct.B*, ...)* @"?foo@B@@W7EAAXUIncomplete@@@Z"
+// CHECK-SAME: void (%struct.B*, ...)* @"?bar@B@@W7EAAXU?$DoNotInstantiate@H@@@Z"
+// CHECK-SAME: i32 (i8*, i32)* @"?baz@B@@W7EAAHU?$InstantiateLater@H@@@Z"
+
+// The thunks should have a -8 adjustment.
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"?foo@B@@W7EAAXUIncomplete@@@Z"(%struct.B* %this, ...)
+// CHECK: %[[THIS_ADJ_i8:[^ ]*]] = getelementptr i8, i8* {{.*}}, i32 -8
+// CHECK: %[[THIS_ADJ:[^ ]*]] = bitcast i8* %[[THIS_ADJ_i8]] to %struct.B*
+// CHECK: musttail call void (%struct.B*, ...) {{.*}}@"?foo@B@@UEAAXUIncomplete@@@Z"
+// CHECK-SAME: (%struct.B* %[[THIS_ADJ]], ...)
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"?bar@B@@W7EAAXU?$DoNotInstantiate@H@@@Z"(%struct.B* %this, ...)
+// CHECK: %[[THIS_ADJ_i8:[^ ]*]] = getelementptr i8, i8* {{.*}}, i32 -8
+// CHECK: %[[THIS_ADJ:[^ ]*]] = bitcast i8* %[[THIS_ADJ_i8]] to %struct.B*
+// CHECK: musttail call void (%struct.B*, ...) {{.*}}@"?bar@B@@UEAAXU?$DoNotInstantiate@H@@@Z"
+// CHECK-SAME: (%struct.B* %[[THIS_ADJ]], ...)
+// CHECK-NEXT: ret void
+
+// If we complete the definition later, things work out.
+template  struct InstantiateLater { T x; };
+inline int B::baz(InstantiateLater p) { return p.x; }
+
+// CHECK-LABEL: define linkonce_odr dso_local i32 @"?baz@B@@W7EAAHU?$InstantiateLater@H@@@Z"(i8* %this.coerce, i32 %p.coerce)
+// CHECK: = getelementptr i8, i8* {{.*}}, i32 -8
+// CHECK: tail call i32 @"?baz@B@@UEAAHU?$InstantiateLater@H@@@Z"(i8* {{[^,]*}}, i32 {{.*}})
+
+// CHECK-LABEL: define linkonce_odr dso_local i32 @"?baz@B@@UEAAHU?$InstantiateLater@H@@@Z"(i8* %this.coerce, i32 %p.coerce)
Index: clang/test/CodeGenCXX/ms-thunks-unprototyped-return.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ms-thunks-unprototyped-return.cpp
@@ -0,0 +1,15 @@
+/

LLVM buildmaster will be unavailable for shot time after 5:30 pm PST today

2018-03-30 Thread Galina Kistanova via cfe-commits
Hello everyone,

LLVM buildmaster will be unavailable for shot time after 5:30 pm PST today
for updates and maintenance.

Thanks



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


[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

2018-03-30 Thread David Majnemer via Phabricator via cfe-commits
majnemer added a comment.

Does this help PR25641?


https://reviews.llvm.org/D45112



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


Buildbot numbers for the week of 3/11/2018 - 3/17/2018

2018-03-30 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the week of 3/11/2018 - 3/17/2018.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername| was_red
--+-
 clang-ppc64le-linux-lnt  | 67:28:25
 clang-cmake-aarch64-full | 43:23:34
 llvm-clang-x86_64-expensive-checks-win   | 29:07:43
 clang-lld-x86_64-2stage  | 26:53:14
 clang-x64-ninja-win7 | 26:20:35
 lld-x86_64-darwin13  | 19:56:08
 clang-with-thin-lto-ubuntu   | 17:43:18
 clang-with-lto-ubuntu| 17:08:54
 lldb-x86_64-ubuntu-14.04-android | 10:26:28
 clang-s390x-linux-multistage | 09:02:35
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 08:40:21
 lld-perf-testsuite   | 08:10:12
 sanitizer-x86_64-linux-bootstrap-msan| 07:46:29
 lldb-windows7-android| 07:39:48
 clang-ppc64be-linux-multistage   | 07:34:41
 sanitizer-x86_64-linux-android   | 07:26:47
 sanitizer-x86_64-linux   | 07:23:10
 sanitizer-x86_64-linux-fast  | 06:33:37
 clang-atom-d525-fedora-rel   | 06:32:54
 clang-s390x-linux| 06:11:56
 clang-x86-windows-msvc2015   | 06:03:19
 clang-cmake-thumbv7-full-sh  | 05:55:41
 clang-cmake-armv7-selfhost   | 05:35:11
 clang-cmake-armv7-selfhost-neon  | 05:31:41
 sanitizer-x86_64-linux-bootstrap | 05:28:12
 clang-ppc64be-linux  | 05:03:01
 clang-tools-sphinx-docs  | 04:33:03
 lldb-x86_64-ubuntu-14.04-cmake   | 04:00:25
 sanitizer-x86_64-linux-bootstrap-ubsan   | 03:53:04
 clang-cmake-armv7-quick  | 03:39:45
 clang-cmake-aarch64-global-isel  | 03:33:59
 clang-cmake-armv8-selfhost-neon  | 03:07:01
 clang-cmake-aarch64-lld  | 03:06:40
 clang-cmake-aarch64-quick| 03:04:13
 lld-x86_64-freebsd   | 02:43:40
 clang-s390x-linux-lnt| 02:38:43
 clang-cmake-armv8-full   | 02:34:26
 sanitizer-ppc64le-linux  | 02:32:23
 clang-cmake-armv8-quick  | 02:26:34
 clang-ppc64le-linux-multistage   | 02:16:41
 clang-cmake-armv7-global-isel| 02:16:17
 openmp-clang-x86_64-linux-debian | 02:15:29
 lld-x86_64-win7  | 02:13:59
 clang-bpf-build  | 02:05:50
 lldb-amd64-ninja-netbsd8 | 02:00:40
 clang-cmake-armv7-full   | 01:53:31
 sanitizer-ppc64be-linux  | 01:52:18
 lldb-x86_64-ubuntu-14.04-buildserver | 01:48:27
 perf-x86_64-penryn-O3-polly-parallel-fast| 01:45:12
 clang-x86_64-debian-fast | 01:42:42
 lldb-x86_64-darwin-13.4  | 01:31:32
 lldb-x86-windows-msvc2015| 01:30:25
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast   | 01:29:54
 lldb-amd64-ninja-freebsd11   | 01:23:17
 clang-cuda-build | 01:15:43
 clang-cmake-x86_64-avx2-linux| 01:12:18
 reverse-iteration| 01:09:46
 clang-cmake-x86_64-sde-avx512-linux  | 01:09:38
 clang-cmake-x86_64-avx2-linux-perf   | 01:08:46
 clang-ppc64le-linux  | 01:03:11
 clang-cmake-armv8-lnt| 00:57:24
 clang-hexagon-elf| 00:52:45
 llvm-hexagon-elf | 00:42:16
 clang-x86_64-linux-abi-test  | 00:37:37
 polly-amd64-linux| 00:28:54
 sanitizer-x86_64-linux-autoconf  | 00:26:36
 sanitizer-windows| 00:19:42
(67 rows)


"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green):
 

Buildbot numbers for the week of 3/18/2018 - 3/24/2018

2018-03-30 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the last week of 3/18/2018 - 3/24/2018.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   | was_red
-+-
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 64:02:37
 reverse-iteration   | 39:04:42
 clang-cmake-armv7-selfhost-neon | 27:38:35
 clang-cmake-armv7-selfhost  | 27:13:46
 clang-x64-ninja-win7| 26:37:43
 clang-x86-windows-msvc2015  | 26:02:31
 polly-amd64-linux   | 25:57:10
 lld-perf-testsuite  | 25:22:43
 polly-arm-linux | 25:04:02
 llvm-clang-x86_64-expensive-checks-win  | 24:59:05
 clang-lld-x86_64-2stage | 23:18:31
 clang-ppc64be-linux-multistage  | 22:42:49
 sanitizer-ppc64le-linux | 21:41:01
 llvm-sphinx-docs| 21:01:55
 clang-cmake-armv8-selfhost-neon | 19:51:07
 clang-with-lto-ubuntu   | 19:50:58
 clang-cmake-armv7-global-isel   | 19:43:14
 clang-with-thin-lto-ubuntu  | 19:34:19
 clang-cmake-armv7-quick | 19:27:14
 clang-cmake-thumbv7-full-sh | 19:08:17
 clang-hexagon-elf   | 19:06:53
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc-tot-latest-std | 18:53:23
 clang-cmake-armv8-quick | 18:08:31
 clang-cmake-armv8-full  | 17:47:23
 sanitizer-ppc64be-linux | 16:58:18
 lldb-x86_64-darwin-13.4 | 16:54:28
 libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan  | 16:44:24
 lldb-x86_64-ubuntu-14.04-cmake  | 15:18:00
 clang-ppc64be-linux | 14:19:06
 clang-cmake-armv7-full  | 14:10:05
 lldb-windows7-android   | 13:55:27
 clang-cmake-aarch64-lld | 11:54:00
 clang-ppc64le-linux-multistage  | 11:42:12
 sanitizer-x86_64-linux-autoconf | 10:17:41
 sanitizer-x86_64-linux-android  | 10:15:54
 clang-ppc64le-linux-lnt | 10:08:51
 clang-cmake-aarch64-full| 09:01:25
 clang-s390x-linux-multistage| 08:31:51
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 08:26:24
 clang-s390x-linux-lnt   | 08:17:24
 clang-x86_64-debian-fast| 05:49:13
 clang-s390x-linux   | 05:44:52
 clang-bpf-build | 03:55:31
 clang-cmake-aarch64-quick   | 03:43:07
 clang-cmake-aarch64-global-isel | 03:41:38
 sanitizer-x86_64-linux-bootstrap| 03:32:24
 clang-cmake-x86_64-sde-avx512-linux | 03:27:19
 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu  | 03:23:43
 libcxx-libcxxabi-x86_64-linux-ubuntu-tsan   | 03:14:07
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11  | 03:11:24
 libcxx-libcxxabi-x86_64-linux-ubuntu-asan   | 03:09:12
 clang-atom-d525-fedora-rel  | 03:09:02
 libcxx-libcxxabi-x86_64-linux-ubuntu-32bit  | 03:07:35
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a  | 03:02:32
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17  | 03:01:57
 clang-ppc64le-linux | 02:59:09
 libcxx-libcxxabi-x86_64-linux-ubuntu-msan   | 02:52:04
 clang-cmake-armv7-lnt   | 02:52:04
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14  | 02:51:30
 sanitizer-x86_64-linux-bootstrap-ubsan  | 02:32:10
 clang-cmake-x86_64-avx2-linux   | 02:31:07
 sanitizer-x86_64-linux  | 02:10:03
 lld-x86_64-darwi

[PATCH] D45117: [analyzer] Fix diagnostics in callees of interesting callees.

2018-03-30 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.

`removeUnneededCalls()` is responsible for removing path diagnostic pieces 
within functions that don't contain "interesting" events. It is the 
functionality that's controlled by the `-analyzer-config prune-paths and it 
makes bug reports much tidier (though sometimes it removes too much, and in 
this case it's great to disable this option when debugging).

While relatively simple, it contains a bug: when a stack frame is known to be 
interesting, the function doesn't descend into it to prune anything within it, 
even other callees that are totally boring.

This is rarely apparent, but in C++ there are a lot of implicit calls that 
suddenly become visible because of this bug.


Repository:
  rC Clang

https://reviews.llvm.org/D45117

Files:
  lib/StaticAnalyzer/Core/BugReporter.cpp
  test/Analysis/inlining/path-notes.c

Index: test/Analysis/inlining/path-notes.c
===
--- test/Analysis/inlining/path-notes.c
+++ test/Analysis/inlining/path-notes.c
@@ -140,6 +140,22 @@
// expected-note@-1 {{Dereference of null pointer}}
 }
 
+void boringCallee() {
+}
+
+void interestingCallee(int *x) {
+  *x = 0; // expected-note{{The value 0 is assigned to 'x'}}
+  boringCallee(); // no-note
+}
+
+int testBoringCalleeOfInterestingCallee() {
+  int x;
+  interestingCallee(&x); // expected-note{{Calling 'interestingCallee'}}
+ // expected-note@-1{{Returning from 'interestingCallee'}}
+  return 1 / x; // expected-warning{{Division by zero}}
+// expected-note@-1{{Division by zero}}
+}
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -3377,4 +3393,290 @@
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:path
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line152
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line152
+// CHECK-NEXT:col5
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line153
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line153
+// CHECK-NEXT:col19
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line153
+// CHECK-NEXT:   col3
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line153
+// CHECK-NEXT:  col3
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line153
+// CHECK-NEXT:  col23
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Calling 'interestingCallee'
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Calling 'interestingCallee'
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line146
+// CHECK-NEXT:   col1
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth1
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Entered call from 'testBoringCalleeOfInterestingCallee'
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Entered call from 'testBoringCalleeOfInterestingCallee'
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line146
+// CHECK-NEXT:col1
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line146
+// CHECK-NEXT:col4
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line147
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-

r328912 - [analyzer] Fix assertion crash in CStringChecker

2018-03-30 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Mar 30 18:20:08 2018
New Revision: 328912

URL: http://llvm.org/viewvc/llvm-project?rev=328912&view=rev
Log:
[analyzer] Fix assertion crash in CStringChecker

An offset might be unknown.

rdar://39054939

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/string.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=328912&r1=328911&r2=328912&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Fri Mar 30 
18:20:08 2018
@@ -395,8 +395,10 @@ ProgramStateRef CStringChecker::CheckBuf
 
   // Compute the offset of the last element to be accessed: size-1.
   NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs();
-  NonLoc LastOffset = svalBuilder
-  .evalBinOpNN(state, BO_Sub, *Length, One, sizeTy).castAs();
+  SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy);
+  if (Offset.isUnknown())
+return nullptr;
+  NonLoc LastOffset = Offset.castAs();
 
   // Check that the first buffer is sufficiently long.
   SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());
@@ -862,9 +864,10 @@ bool CStringChecker::IsFirstBufInBound(C
 
   // Compute the offset of the last element to be accessed: size-1.
   NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs();
-  NonLoc LastOffset =
-  svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy)
-  .castAs();
+  SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy);
+  if (Offset.isUnknown())
+return true; // cf top comment
+  NonLoc LastOffset = Offset.castAs();
 
   // Check that the first buffer is sufficiently long.
   SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());

Modified: cfe/trunk/test/Analysis/string.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=328912&r1=328911&r2=328912&view=diff
==
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Fri Mar 30 18:20:08 2018
@@ -30,6 +30,7 @@ typedef typeof(sizeof(int)) size_t;
 void clang_analyzer_eval(int);
 
 int scanf(const char *restrict format, ...);
+void *memcpy(void *, const void *, unsigned long);
 
 //===--===
 // strlen()
@@ -1173,6 +1174,7 @@ void strcat_symbolic_src_length(char *sr
   clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{UNKNOWN}}
 }
 
+
 // The analyzer_eval call below should evaluate to true. Most likely the same
 // issue as the test above.
 void strncpy_exactly_matching_buffer2(char *y) {
@@ -1185,3 +1187,12 @@ void strncpy_exactly_matching_buffer2(ch
// This time, we know that y fits in x anyway.
   clang_analyzer_eval(strlen(x) <= 3); // expected-warning{{UNKNOWN}}
 }
+
+struct S {
+  char f;
+};
+
+void nocrash_on_locint_offset(void *addr, void* from, struct S s) {
+  int iAdd = (int) addr;
+  memcpy(((void *) &(s.f)), from, iAdd);
+}


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


r328910 - [analyzer] Fix liveness calculation for C++17 structured bindings

2018-03-30 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Mar 30 18:20:06 2018
New Revision: 328910

URL: http://llvm.org/viewvc/llvm-project?rev=328910&view=rev
Log:
[analyzer] Fix liveness calculation for C++17 structured bindings

C++ structured bindings for non-tuple-types are defined in a peculiar
way, where the resulting declaration is not a VarDecl, but a
BindingDecl.
That means a lot of existing machinery stops working.

rdar://36912381

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

Added:
cfe/trunk/test/Analysis/live-bindings-test.cpp
Modified:
cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
cfe/trunk/lib/Analysis/LiveVariables.cpp

Modified: cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h?rev=328910&r1=328909&r2=328910&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h Fri Mar 30 
18:20:06 2018
@@ -33,15 +33,18 @@ public:
 
 llvm::ImmutableSet liveStmts;
 llvm::ImmutableSet liveDecls;
+llvm::ImmutableSet liveBindings;
 
 bool equals(const LivenessValues &V) const;
 
 LivenessValues()
-  : liveStmts(nullptr), liveDecls(nullptr) {}
+  : liveStmts(nullptr), liveDecls(nullptr), liveBindings(nullptr) {}
 
 LivenessValues(llvm::ImmutableSet LiveStmts,
-   llvm::ImmutableSet LiveDecls)
-  : liveStmts(LiveStmts), liveDecls(LiveDecls) {}
+   llvm::ImmutableSet LiveDecls,
+   llvm::ImmutableSet LiveBindings)
+: liveStmts(LiveStmts), liveDecls(LiveDecls),
+  liveBindings(LiveBindings) {}
 
 bool isLive(const Stmt *S) const;
 bool isLive(const VarDecl *D) const;

Modified: cfe/trunk/lib/Analysis/LiveVariables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/LiveVariables.cpp?rev=328910&r1=328909&r2=328910&view=diff
==
--- cfe/trunk/lib/Analysis/LiveVariables.cpp (original)
+++ cfe/trunk/lib/Analysis/LiveVariables.cpp Fri Mar 30 18:20:06 2018
@@ -77,6 +77,7 @@ public:
   AnalysisDeclContext &analysisContext;
   llvm::ImmutableSet::Factory SSetFact;
   llvm::ImmutableSet::Factory DSetFact;
+  llvm::ImmutableSet::Factory BSetFact;
   llvm::DenseMap 
blocksEndToLiveness;
   llvm::DenseMap 
blocksBeginToLiveness;
   llvm::DenseMap stmtsToLiveness;
@@ -97,6 +98,7 @@ public:
 : analysisContext(ac),
   SSetFact(false), // Do not canonicalize ImmutableSets by default.
   DSetFact(false), // This is a *major* performance win.
+  BSetFact(false),
   killAtAssign(KillAtAssign) {}
 };
 }
@@ -114,6 +116,12 @@ bool LiveVariables::LivenessValues::isLi
 }
 
 bool LiveVariables::LivenessValues::isLive(const VarDecl *D) const {
+  if (const auto *DD = dyn_cast(D)) {
+bool alive = false;
+for (const BindingDecl *BD : DD->bindings())
+  alive |= liveBindings.contains(BD);
+return alive;
+  }
   return liveDecls.contains(D);
 }
 
@@ -145,14 +153,19 @@ LiveVariablesImpl::merge(LiveVariables::
 DSetRefA(valsA.liveDecls.getRootWithoutRetain(), 
DSetFact.getTreeFactory()),
 DSetRefB(valsB.liveDecls.getRootWithoutRetain(), 
DSetFact.getTreeFactory());
   
+  llvm::ImmutableSetRef
+BSetRefA(valsA.liveBindings.getRootWithoutRetain(), 
BSetFact.getTreeFactory()),
+BSetRefB(valsB.liveBindings.getRootWithoutRetain(), 
BSetFact.getTreeFactory());
 
   SSetRefA = mergeSets(SSetRefA, SSetRefB);
   DSetRefA = mergeSets(DSetRefA, DSetRefB);
+  BSetRefA = mergeSets(BSetRefA, BSetRefB);
   
   // asImmutableSet() canonicalizes the tree, allowing us to do an easy
   // comparison afterwards.
   return LiveVariables::LivenessValues(SSetRefA.asImmutableSet(),
-   DSetRefA.asImmutableSet());  
+   DSetRefA.asImmutableSet(),
+   BSetRefA.asImmutableSet());  
 }
 
 bool LiveVariables::LivenessValues::equals(const LivenessValues &V) const {
@@ -322,6 +335,11 @@ void TransferFunctions::Visit(Stmt *S) {
   }
 }
 
+static bool writeShouldKill(const VarDecl *VD) {
+  return VD && !VD->getType()->isReferenceType() &&
+!isAlwaysAlive(VD);
+}
+
 void TransferFunctions::VisitBinaryOperator(BinaryOperator *B) {
   if (B->isAssignmentOp()) {
 if (!LV.killAtAssign)
@@ -329,21 +347,25 @@ void TransferFunctions::VisitBinaryOpera
 
 // Assigning to a variable?
 Expr *LHS = B->getLHS()->IgnoreParens();
-
-if (DeclRefExpr *DR = dyn_cast(LHS))
-  if (const VarDecl *VD = dyn_cast(DR->getDecl())) {
-// Assignments to references don't kill the ref's address
-if (VD->getType()->isReferenceType())
-  return;
 
-if (!isAlwaysAlive(VD)) {
-  // The variable i

r328911 - [analyzer] Cache offset computation for MemRegion

2018-03-30 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Mar 30 18:20:07 2018
New Revision: 328911

URL: http://llvm.org/viewvc/llvm-project?rev=328911&view=rev
Log:
[analyzer] Cache offset computation for MemRegion

Achieves almost a 200% speedup on the example where the performance of
visitors was problematic.

Performance on sqlite3 is unaffected.

rdar://38818362

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=328911&r1=328910&r2=328911&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Fri 
Mar 30 18:20:07 2018
@@ -86,6 +86,7 @@ public:
 
 private:
   const Kind kind;
+  mutable Optional cachedOffset;
 
 protected:
   MemRegion(Kind k) : kind(k) {}

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=328911&r1=328910&r2=328911&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Fri Mar 30 18:20:07 2018
@@ -1259,47 +1259,46 @@ static bool isImmediateBase(const CXXRec
   return false;
 }
 
-RegionOffset MemRegion::getAsOffset() const {
-  const MemRegion *R = this;
+static RegionOffset calculateOffset(const MemRegion *R) {
   const MemRegion *SymbolicOffsetBase = nullptr;
   int64_t Offset = 0;
 
   while (1) {
 switch (R->getKind()) {
-case CodeSpaceRegionKind:
-case StackLocalsSpaceRegionKind:
-case StackArgumentsSpaceRegionKind:
-case HeapSpaceRegionKind:
-case UnknownSpaceRegionKind:
-case StaticGlobalSpaceRegionKind:
-case GlobalInternalSpaceRegionKind:
-case GlobalSystemSpaceRegionKind:
-case GlobalImmutableSpaceRegionKind:
+case MemRegion::CodeSpaceRegionKind:
+case MemRegion::StackLocalsSpaceRegionKind:
+case MemRegion::StackArgumentsSpaceRegionKind:
+case MemRegion::HeapSpaceRegionKind:
+case MemRegion::UnknownSpaceRegionKind:
+case MemRegion::StaticGlobalSpaceRegionKind:
+case MemRegion::GlobalInternalSpaceRegionKind:
+case MemRegion::GlobalSystemSpaceRegionKind:
+case MemRegion::GlobalImmutableSpaceRegionKind:
   // Stores can bind directly to a region space to set a default value.
   assert(Offset == 0 && !SymbolicOffsetBase);
   goto Finish;
 
-case FunctionCodeRegionKind:
-case BlockCodeRegionKind:
-case BlockDataRegionKind:
+case MemRegion::FunctionCodeRegionKind:
+case MemRegion::BlockCodeRegionKind:
+case MemRegion::BlockDataRegionKind:
   // These will never have bindings, but may end up having values requested
   // if the user does some strange casting.
   if (Offset != 0)
 SymbolicOffsetBase = R;
   goto Finish;
 
-case SymbolicRegionKind:
-case AllocaRegionKind:
-case CompoundLiteralRegionKind:
-case CXXThisRegionKind:
-case StringRegionKind:
-case ObjCStringRegionKind:
-case VarRegionKind:
-case CXXTempObjectRegionKind:
+case MemRegion::SymbolicRegionKind:
+case MemRegion::AllocaRegionKind:
+case MemRegion::CompoundLiteralRegionKind:
+case MemRegion::CXXThisRegionKind:
+case MemRegion::StringRegionKind:
+case MemRegion::ObjCStringRegionKind:
+case MemRegion::VarRegionKind:
+case MemRegion::CXXTempObjectRegionKind:
   // Usual base regions.
   goto Finish;
 
-case ObjCIvarRegionKind:
+case MemRegion::ObjCIvarRegionKind:
   // This is a little strange, but it's a compromise between
   // ObjCIvarRegions having unknown compile-time offsets (when using the
   // non-fragile runtime) and yet still being distinct, non-overlapping
@@ -1307,14 +1306,14 @@ RegionOffset MemRegion::getAsOffset() co
   // of computing offsets.
   goto Finish;
 
-case CXXBaseObjectRegionKind: {
+case MemRegion::CXXBaseObjectRegionKind: {
   const CXXBaseObjectRegion *BOR = cast(R);
   R = BOR->getSuperRegion();
 
   QualType Ty;
   bool RootIsSymbolic = false;
   if (const TypedValueRegion *TVR = dyn_cast(R)) {
-Ty = TVR->getDesugaredValueType(getContext());
+Ty = TVR->getDesugaredValueType(R->getContext());
   } else if (const SymbolicRegion *SR = dyn_cast(R)) {
 // If our base region is symbolic, we don't know what type it really 
is.
 // Pretend the type of the symbol is the true dynamic type.
@@ -1348,17 +1347,17 @@ RegionOffset MemRegion::getAsOffset() co

[PATCH] D45115: [analyzer] Fix assertion crash in CStringChecker

2018-03-30 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328912: [analyzer] Fix assertion crash in CStringChecker 
(authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D45115

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/string.c


Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -30,6 +30,7 @@
 void clang_analyzer_eval(int);
 
 int scanf(const char *restrict format, ...);
+void *memcpy(void *, const void *, unsigned long);
 
 //===--===
 // strlen()
@@ -1173,6 +1174,7 @@
   clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{UNKNOWN}}
 }
 
+
 // The analyzer_eval call below should evaluate to true. Most likely the same
 // issue as the test above.
 void strncpy_exactly_matching_buffer2(char *y) {
@@ -1185,3 +1187,12 @@
// This time, we know that y fits in x anyway.
   clang_analyzer_eval(strlen(x) <= 3); // expected-warning{{UNKNOWN}}
 }
+
+struct S {
+  char f;
+};
+
+void nocrash_on_locint_offset(void *addr, void* from, struct S s) {
+  int iAdd = (int) addr;
+  memcpy(((void *) &(s.f)), from, iAdd);
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -395,8 +395,10 @@
 
   // Compute the offset of the last element to be accessed: size-1.
   NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs();
-  NonLoc LastOffset = svalBuilder
-  .evalBinOpNN(state, BO_Sub, *Length, One, sizeTy).castAs();
+  SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy);
+  if (Offset.isUnknown())
+return nullptr;
+  NonLoc LastOffset = Offset.castAs();
 
   // Check that the first buffer is sufficiently long.
   SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());
@@ -862,9 +864,10 @@
 
   // Compute the offset of the last element to be accessed: size-1.
   NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs();
-  NonLoc LastOffset =
-  svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy)
-  .castAs();
+  SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy);
+  if (Offset.isUnknown())
+return true; // cf top comment
+  NonLoc LastOffset = Offset.castAs();
 
   // Check that the first buffer is sufficiently long.
   SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());


Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -30,6 +30,7 @@
 void clang_analyzer_eval(int);
 
 int scanf(const char *restrict format, ...);
+void *memcpy(void *, const void *, unsigned long);
 
 //===--===
 // strlen()
@@ -1173,6 +1174,7 @@
   clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{UNKNOWN}}
 }
 
+
 // The analyzer_eval call below should evaluate to true. Most likely the same
 // issue as the test above.
 void strncpy_exactly_matching_buffer2(char *y) {
@@ -1185,3 +1187,12 @@
 	// This time, we know that y fits in x anyway.
   clang_analyzer_eval(strlen(x) <= 3); // expected-warning{{UNKNOWN}}
 }
+
+struct S {
+  char f;
+};
+
+void nocrash_on_locint_offset(void *addr, void* from, struct S s) {
+  int iAdd = (int) addr;
+  memcpy(((void *) &(s.f)), from, iAdd);
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -395,8 +395,10 @@
 
   // Compute the offset of the last element to be accessed: size-1.
   NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs();
-  NonLoc LastOffset = svalBuilder
-  .evalBinOpNN(state, BO_Sub, *Length, One, sizeTy).castAs();
+  SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy);
+  if (Offset.isUnknown())
+return nullptr;
+  NonLoc LastOffset = Offset.castAs();
 
   // Check that the first buffer is sufficiently long.
   SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());
@@ -862,9 +864,10 @@
 
   // Compute the offset of the last element to be accessed: size-1.
   NonLoc One = svalBuilder.makeIntVal(1, sizeTy).castAs();
-  NonLoc LastOffset =
-  svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy)
-  .castAs();
+  SVal Offset = svalBuilder.evalBinOpNN(state, BO_Sub, *Length, One, sizeTy);
+  if (Offset.isUnknown())
+return true; // cf top comment
+  NonLoc LastOffset = Offset.castAs();
 
   // Check that the first buffer is sufficiently long.
   SVal BufStart 

[PATCH] D45113: [analyzer] Cache computation of regionAsOffset

2018-03-30 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC328911: [analyzer] Cache offset computation for MemRegion 
(authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D45113

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Core/MemRegion.cpp

Index: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -86,6 +86,7 @@
 
 private:
   const Kind kind;
+  mutable Optional cachedOffset;
 
 protected:
   MemRegion(Kind k) : kind(k) {}
Index: lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- lib/StaticAnalyzer/Core/MemRegion.cpp
+++ lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1259,62 +1259,61 @@
   return false;
 }
 
-RegionOffset MemRegion::getAsOffset() const {
-  const MemRegion *R = this;
+static RegionOffset calculateOffset(const MemRegion *R) {
   const MemRegion *SymbolicOffsetBase = nullptr;
   int64_t Offset = 0;
 
   while (1) {
 switch (R->getKind()) {
-case CodeSpaceRegionKind:
-case StackLocalsSpaceRegionKind:
-case StackArgumentsSpaceRegionKind:
-case HeapSpaceRegionKind:
-case UnknownSpaceRegionKind:
-case StaticGlobalSpaceRegionKind:
-case GlobalInternalSpaceRegionKind:
-case GlobalSystemSpaceRegionKind:
-case GlobalImmutableSpaceRegionKind:
+case MemRegion::CodeSpaceRegionKind:
+case MemRegion::StackLocalsSpaceRegionKind:
+case MemRegion::StackArgumentsSpaceRegionKind:
+case MemRegion::HeapSpaceRegionKind:
+case MemRegion::UnknownSpaceRegionKind:
+case MemRegion::StaticGlobalSpaceRegionKind:
+case MemRegion::GlobalInternalSpaceRegionKind:
+case MemRegion::GlobalSystemSpaceRegionKind:
+case MemRegion::GlobalImmutableSpaceRegionKind:
   // Stores can bind directly to a region space to set a default value.
   assert(Offset == 0 && !SymbolicOffsetBase);
   goto Finish;
 
-case FunctionCodeRegionKind:
-case BlockCodeRegionKind:
-case BlockDataRegionKind:
+case MemRegion::FunctionCodeRegionKind:
+case MemRegion::BlockCodeRegionKind:
+case MemRegion::BlockDataRegionKind:
   // These will never have bindings, but may end up having values requested
   // if the user does some strange casting.
   if (Offset != 0)
 SymbolicOffsetBase = R;
   goto Finish;
 
-case SymbolicRegionKind:
-case AllocaRegionKind:
-case CompoundLiteralRegionKind:
-case CXXThisRegionKind:
-case StringRegionKind:
-case ObjCStringRegionKind:
-case VarRegionKind:
-case CXXTempObjectRegionKind:
+case MemRegion::SymbolicRegionKind:
+case MemRegion::AllocaRegionKind:
+case MemRegion::CompoundLiteralRegionKind:
+case MemRegion::CXXThisRegionKind:
+case MemRegion::StringRegionKind:
+case MemRegion::ObjCStringRegionKind:
+case MemRegion::VarRegionKind:
+case MemRegion::CXXTempObjectRegionKind:
   // Usual base regions.
   goto Finish;
 
-case ObjCIvarRegionKind:
+case MemRegion::ObjCIvarRegionKind:
   // This is a little strange, but it's a compromise between
   // ObjCIvarRegions having unknown compile-time offsets (when using the
   // non-fragile runtime) and yet still being distinct, non-overlapping
   // regions. Thus we treat them as "like" base regions for the purposes
   // of computing offsets.
   goto Finish;
 
-case CXXBaseObjectRegionKind: {
+case MemRegion::CXXBaseObjectRegionKind: {
   const CXXBaseObjectRegion *BOR = cast(R);
   R = BOR->getSuperRegion();
 
   QualType Ty;
   bool RootIsSymbolic = false;
   if (const TypedValueRegion *TVR = dyn_cast(R)) {
-Ty = TVR->getDesugaredValueType(getContext());
+Ty = TVR->getDesugaredValueType(R->getContext());
   } else if (const SymbolicRegion *SR = dyn_cast(R)) {
 // If our base region is symbolic, we don't know what type it really is.
 // Pretend the type of the symbol is the true dynamic type.
@@ -1348,17 +1347,17 @@
 continue;
 
   CharUnits BaseOffset;
-  const ASTRecordLayout &Layout = getContext().getASTRecordLayout(Child);
+  const ASTRecordLayout &Layout = R->getContext().getASTRecordLayout(Child);
   if (BOR->isVirtual())
 BaseOffset = Layout.getVBaseClassOffset(BOR->getDecl());
   else
 BaseOffset = Layout.getBaseClassOffset(BOR->getDecl());
 
   // The base offset is in chars, not in bits.
-  Offset += BaseOffset.getQuantity() * getContext().getCharWidth();
+  Offset += BaseOffset.getQuantity() * R->getContext().getCharWidth();
   break;
 }
-case ElementRegionKind: {
+case MemRegion::E

r328913 - [analyzer] Hopefully fix the ARM buildbot.

2018-03-30 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Fri Mar 30 19:17:15 2018
New Revision: 328913

URL: http://llvm.org/viewvc/llvm-project?rev=328913&view=rev
Log:
[analyzer] Hopefully fix the ARM buildbot.

Modified:
cfe/trunk/test/Analysis/bstring.c
cfe/trunk/test/Analysis/string.c

Modified: cfe/trunk/test/Analysis/bstring.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bstring.c?rev=328913&r1=328912&r2=328913&view=diff
==
--- cfe/trunk/test/Analysis/bstring.c (original)
+++ cfe/trunk/test/Analysis/bstring.c Fri Mar 30 19:17:15 2018
@@ -474,3 +474,12 @@ char radar_11125445_memcopythenlogfirstb
   free(bytes);
   return x;
 }
+
+struct S {
+  char f;
+};
+
+void nocrash_on_locint_offset(void *addr, void* from, struct S s) {
+  int iAdd = (int) addr;
+  memcpy(((void *) &(s.f)), from, iAdd);
+}

Modified: cfe/trunk/test/Analysis/string.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=328913&r1=328912&r2=328913&view=diff
==
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Fri Mar 30 19:17:15 2018
@@ -30,7 +30,6 @@ typedef typeof(sizeof(int)) size_t;
 void clang_analyzer_eval(int);
 
 int scanf(const char *restrict format, ...);
-void *memcpy(void *, const void *, unsigned long);
 
 //===--===
 // strlen()
@@ -1187,12 +1186,3 @@ void strncpy_exactly_matching_buffer2(ch
// This time, we know that y fits in x anyway.
   clang_analyzer_eval(strlen(x) <= 3); // expected-warning{{UNKNOWN}}
 }
-
-struct S {
-  char f;
-};
-
-void nocrash_on_locint_offset(void *addr, void* from, struct S s) {
-  int iAdd = (int) addr;
-  memcpy(((void *) &(s.f)), from, iAdd);
-}


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


[PATCH] D45101: [ObjC] Use the name specified by objc_runtime_name instead of the class identifier

2018-03-30 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D45101#1053246, @ahatanak wrote:

> Note that CGObjCNonFragileABIMac::EmitClassRef also passes the class 
> identifier to CGObjCNonFragileABIMac::EmitClassRefFromId, but it doesn't 
> cause a problem. CGObjCNonFragileABIMac::EmitClassRefFromId uses the 
> identifier only when the ObjCInterfaceDecl passed to it is null and that 
> happens only when it is called from 
> CGObjCNonFragileABIMac::EmitNSAutoreleasePoolClassRef.


Hmm.  Alright.

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D45101



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


[PATCH] D44238: [CFG] Fix automatic destructors when a member is bound to a reference.

2018-03-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Seems fine to me, but you might want someone with analyzer experience to review.


Repository:
  rC Clang

https://reviews.llvm.org/D44238



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


[PATCH] D45086: [analyzer] Unroll the loop when it has a unsigned counter.

2018-03-30 Thread Henry Wong via Phabricator via cfe-commits
MTC updated this revision to Diff 140524.
MTC added a comment.

Fix typo, `unsinged` -> `unsigned`


Repository:
  rC Clang

https://reviews.llvm.org/D45086

Files:
  lib/StaticAnalyzer/Core/LoopUnrolling.cpp
  test/Analysis/loop-unrolling.cpp


Index: test/Analysis/loop-unrolling.cpp
===
--- test/Analysis/loop-unrolling.cpp
+++ test/Analysis/loop-unrolling.cpp
@@ -36,6 +36,29 @@
   return 0;
 }
 
+int simple_unroll3_unsigned() {
+  int a[9];
+  int k = 42;
+  for (unsigned i = 0; i < 9; i++) {
+clang_analyzer_numTimesReached(); // expected-warning {{9}}
+a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
+int simple_unroll4_unsigned() {
+  int a[9];
+  int k = 42;
+  unsigned i;
+  for (i = (0); i < 9; i++) {
+clang_analyzer_numTimesReached(); // expected-warning {{9}}
+a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
 int simple_no_unroll1() {
   int a[9];
   int k = 42;
Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -141,13 +141,15 @@
   return forStmt(
  hasCondition(simpleCondition("initVarName")),
  // Initialization should match the form: 'int i = 6' or 'i = 42'.
- hasLoopInit(anyOf(
- declStmt(hasSingleDecl(varDecl(
- allOf(hasInitializer(integerLiteral().bind("initNum")),
-   equalsBoundNode("initVarName"),
- binaryOperator(hasLHS(declRefExpr(to(
-varDecl(equalsBoundNode("initVarName"),
-hasRHS(integerLiteral().bind("initNum"),
+ hasLoopInit(
+ anyOf(declStmt(hasSingleDecl(
+   varDecl(allOf(hasInitializer(ignoringParenImpCasts(
+ 
integerLiteral().bind("initNum"))),
+ equalsBoundNode("initVarName"),
+   binaryOperator(hasLHS(declRefExpr(to(varDecl(
+  equalsBoundNode("initVarName"),
+  hasRHS(ignoringParenImpCasts(
+  
integerLiteral().bind("initNum")),
  // Incrementation should be a simple increment or decrement
  // operator call.
  hasIncrement(unaryOperator(


Index: test/Analysis/loop-unrolling.cpp
===
--- test/Analysis/loop-unrolling.cpp
+++ test/Analysis/loop-unrolling.cpp
@@ -36,6 +36,29 @@
   return 0;
 }
 
+int simple_unroll3_unsigned() {
+  int a[9];
+  int k = 42;
+  for (unsigned i = 0; i < 9; i++) {
+clang_analyzer_numTimesReached(); // expected-warning {{9}}
+a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
+int simple_unroll4_unsigned() {
+  int a[9];
+  int k = 42;
+  unsigned i;
+  for (i = (0); i < 9; i++) {
+clang_analyzer_numTimesReached(); // expected-warning {{9}}
+a[i] = 42;
+  }
+  int b = 22 / (k - 42); // expected-warning {{Division by zero}}
+  return 0;
+}
+
 int simple_no_unroll1() {
   int a[9];
   int k = 42;
Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -141,13 +141,15 @@
   return forStmt(
  hasCondition(simpleCondition("initVarName")),
  // Initialization should match the form: 'int i = 6' or 'i = 42'.
- hasLoopInit(anyOf(
- declStmt(hasSingleDecl(varDecl(
- allOf(hasInitializer(integerLiteral().bind("initNum")),
-   equalsBoundNode("initVarName"),
- binaryOperator(hasLHS(declRefExpr(to(
-varDecl(equalsBoundNode("initVarName"),
-hasRHS(integerLiteral().bind("initNum"),
+ hasLoopInit(
+ anyOf(declStmt(hasSingleDecl(
+   varDecl(allOf(hasInitializer(ignoringParenImpCasts(
+ integerLiteral().bind("initNum"))),
+ equalsBoundNode("initVarName"),
+   binaryOperator(hasLHS(declRefExpr(to(varDecl(
+  equalsBoundNode("initVarName"),
+  hasRHS(ignoringParenImpCasts(
+  integerLiteral().bind("initNum")),
  // Incrementation should be a simple increment or dec