r299671 - Fix unused lambda capture. Follow up to r299653.

[Ivan Krasin via cfe-commits]

Author: krasin
Date: Thu Apr  6 12:42:05 2017
New Revision: 299671

URL: http://llvm.org/viewvc/llvm-project?rev=299671=rev
Log:
Fix unused lambda capture. Follow up to r299653.


Modified:
cfe/trunk/lib/Analysis/CloneDetection.cpp

Modified: cfe/trunk/lib/Analysis/CloneDetection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CloneDetection.cpp?rev=299671=299670=299671=diff
==
--- cfe/trunk/lib/Analysis/CloneDetection.cpp (original)
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp Thu Apr  6 12:42:05 2017
@@ -492,7 +492,7 @@ void RecursiveCloneTypeIIConstraint::con
 
 // Sort hash_codes in StmtsByHash.
 std::stable_sort(StmtsByHash.begin(), StmtsByHash.end(),
- [this](std::pair LHS,
+ [](std::pair LHS,
 std::pair RHS) {
return LHS.first < RHS.first;
  });


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

[libunwind] r299666 - Fix unused typedef. Follow up to r299575.

[Ivan Krasin via cfe-commits]

Author: krasin
Date: Thu Apr  6 12:35:35 2017
New Revision: 299666

URL: http://llvm.org/viewvc/llvm-project?rev=299666=rev
Log:
Fix unused typedef. Follow up to r299575.


Modified:
libunwind/trunk/src/AddressSpace.hpp

Modified: libunwind/trunk/src/AddressSpace.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/AddressSpace.hpp?rev=299666=299665=299666=diff
==
--- libunwind/trunk/src/AddressSpace.hpp (original)
+++ libunwind/trunk/src/AddressSpace.hpp Thu Apr  6 12:35:35 2017
@@ -383,7 +383,7 @@ inline bool LocalAddressSpace::findUnwin
 #if !defined(Elf_Phdr)
 typedef ElfW(Phdr) Elf_Phdr;
 #endif
-#if !defined(Elf_Addr)
+#if !defined(Elf_Addr) && defined(__ANDROID__)
 typedef ElfW(Addr) Elf_Addr;
 #endif
 


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

Re: r296374 - [ubsan] Factor out logic to emit a range check. NFC.

[Ivan Krasin via cfe-commits]

Hi Vedant,

not on top of my head. Dmitriy, can you please take a look?

krasin

On Mon, Feb 27, 2017 at 12:43 PM, Vedant Kumar  wrote:

> Hi Ivan,
>
> I saw a bot failure on your job after this commit:
>
>   http://lab.llvm.org:8011/builders/sanitizer-x86_64-
> linux-autoconf/builds/5467/steps/tsan%20analyze/logs/stdio
>   http://lab.llvm.org:8011/builders/sanitizer-x86_64-
> linux-autoconf/builds/5467/steps/build%20release%20tsan%
> 20with%20clang/logs/stdio
>
> However, I cannot reproduce it locally with a stage2 TSAN build.
>
> After staring at my diff I couldn't find anything that would explain the
> failure. No other bots seem upset.
>
> Do you have any idea about what's going on? Let me know if you want me to
> revert...
>
> vedant
>
> > On Feb 27, 2017, at 11:46 AM, Vedant Kumar via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Author: vedantk
> > Date: Mon Feb 27 13:46:19 2017
> > New Revision: 296374
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=296374=rev
> > Log:
> > [ubsan] Factor out logic to emit a range check. NFC.
> >
> > This is a readability improvement, but it will also help prep an
> > upcoming patch to detect UB loads from bitfields.
> >
> > Modified:
> >cfe/trunk/lib/CodeGen/CGExpr.cpp
> >cfe/trunk/lib/CodeGen/CodeGenFunction.h
> >
> > Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CGExpr.cpp?rev=296374=296373=296374=diff
> > 
> ==
> > --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Feb 27 13:46:19 2017
> > @@ -1301,6 +1301,46 @@ llvm::MDNode *CodeGenFunction::getRangeF
> >   return MDHelper.createRange(Min, End);
> > }
> >
> > +bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value,
> QualType Ty,
> > +   SourceLocation Loc) {
> > +  bool HasBoolCheck = SanOpts.has(SanitizerKind::Bool);
> > +  bool HasEnumCheck = SanOpts.has(SanitizerKind::Enum);
> > +  if (!HasBoolCheck && !HasEnumCheck)
> > +return false;
> > +
> > +  bool IsBool = hasBooleanRepresentation(Ty) ||
> > +NSAPI(CGM.getContext()).isObjCBOOLType(Ty);
> > +  bool NeedsBoolCheck = HasBoolCheck && IsBool;
> > +  bool NeedsEnumCheck = HasEnumCheck && Ty->getAs();
> > +  if (!NeedsBoolCheck && !NeedsEnumCheck)
> > +return false;
> > +
> > +  llvm::APInt Min, End;
> > +  if (!getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true,
> IsBool))
> > +return true;
> > +
> > +  SanitizerScope SanScope(this);
> > +  llvm::Value *Check;
> > +  --End;
> > +  if (!Min) {
> > +Check = Builder.CreateICmpULE(
> > +Value, llvm::ConstantInt::get(getLLVMContext(), End));
> > +  } else {
> > +llvm::Value *Upper = Builder.CreateICmpSLE(
> > +Value, llvm::ConstantInt::get(getLLVMContext(), End));
> > +llvm::Value *Lower = Builder.CreateICmpSGE(
> > +Value, llvm::ConstantInt::get(getLLVMContext(), Min));
> > +Check = Builder.CreateAnd(Upper, Lower);
> > +  }
> > +  llvm::Constant *StaticArgs[] = {EmitCheckSourceLocation(Loc),
> > +  EmitCheckTypeDescriptor(Ty)};
> > +  SanitizerMask Kind =
> > +  NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool;
> > +  EmitCheck(std::make_pair(Check, Kind), SanitizerHandler::
> LoadInvalidValue,
> > +StaticArgs, EmitCheckValue(Value));
> > +  return true;
> > +}
> > +
> > llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool
> Volatile,
> >QualType Ty,
> >SourceLocation Loc,
> > @@ -1353,35 +1393,9 @@ llvm::Value *CodeGenFunction::EmitLoadOf
> >   false /*ConvertTypeToTag*/);
> >   }
> >
> > -  bool IsBool = hasBooleanRepresentation(Ty) ||
> > -NSAPI(CGM.getContext()).isObjCBOOLType(Ty);
> > -  bool NeedsBoolCheck = SanOpts.has(SanitizerKind::Bool) && IsBool;
> > -  bool NeedsEnumCheck =
> > -  SanOpts.has(SanitizerKind::Enum) && Ty->getAs();
> > -  if (NeedsBoolCheck || NeedsEnumCheck) {
> > -SanitizerScope SanScope(this);
> > -llvm::APInt Min, End;
> > -if (getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true,
> IsBool)) {
> > -  --End;
> > -  llvm::Value *Check;
> > -  if (!Min)
> > -Check = Builder.CreateICmpULE(
> > -  Load, llvm::ConstantInt::get(getLLVMContext(), End));
> > -  else {
> > -llvm::Value *Upper = Builder.CreateICmpSLE(
> > -  Load, llvm::ConstantInt::get(getLLVMContext(), End));
> > -llvm::Value *Lower = Builder.CreateICmpSGE(
> > -  Load, llvm::ConstantInt::get(getLLVMContext(), Min));
> > -Check = Builder.CreateAnd(Upper, Lower);
> > -  }
> > -  llvm::Constant *StaticArgs[] = {
> > -EmitCheckSourceLocation(Loc),
> > -

[PATCH] D26560: Add a test for vcall on a null ptr.

[Ivan Krasin via cfe-commits]

krasin added a comment.

Ping.


https://reviews.llvm.org/D26560



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

[PATCH] D26560: Add a test for vcall on a null ptr.

[Ivan Krasin via cfe-commits]

krasin added inline comments.



Comment at: test/ubsan/TestCases/TypeCheck/null.cpp:1
-// RUN: %clangxx -fsanitize=null %s -O3 -o %t
-// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
-// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
-// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
-// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
-// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null -g %s -O3 -o %t
+// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD

pcc wrote:
> Why add the -g?
It's a debug left over. Thank you for the catch.



Comment at: test/ubsan/TestCases/TypeCheck/null.cpp:10
+
+#include 
 

pcc wrote:
> Is this #include needed?
Debug leftover. Removed. Thank you for spotting this.



Comment at: test/ubsan/TestCases/TypeCheck/null.cpp:35
+
+  if (argv[1][0] == 'T') {
+t = new T;

pcc wrote:
> Did you intend to add tests for these cases?
Actually, the real reason for adding these is that break_optimization didn't 
really fool the compiler, and I had to add some more logic to avoid letting it 
know that the pointer is always null => it's undefined behavior. In my case, I 
saw my return being ignored and two switch statements executed together.

I can't currently reproduce it now, most likely, because the fix has eliminated 
the virtual call on a pointer that is guaranteed to be null. So, I have removed 
these as well as break_optimization calls.



https://reviews.llvm.org/D26560



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

[PATCH] D26560: Add a test for vcall on a null ptr.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 78557.
krasin added a comment.

sync & address the comments.


https://reviews.llvm.org/D26560

Files:
  test/ubsan/TestCases/TypeCheck/null.cpp


Index: test/ubsan/TestCases/TypeCheck/null.cpp
===
--- test/ubsan/TestCases/TypeCheck/null.cpp
+++ test/ubsan/TestCases/TypeCheck/null.cpp
@@ -1,20 +1,34 @@
-// RUN: %clangxx -fsanitize=null %s -O3 -o %t
-// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
-// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
-// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
-// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
-// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null %s -O3 -o %t
+// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
+// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
+// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
+// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
+// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL
+// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2
 
 struct S {
   int f() { return 0; }
   int k;
 };
 
+struct T {
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  virtual int v() { return 2; }
+};
+
 int main(int, char **argv) {
   int *p = 0;
   S *s = 0;
+  T *t = 0;
+  U *u = 0;
 
   (void)*p; // ok!
+  (void)*t; // ok!
+  (void)*u; // ok!
 
   switch (argv[1][0]) {
   case 'l':
@@ -34,5 +48,11 @@
   case 'f':
 // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'S'
 return s->f();
+  case 't':
+// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'T'
+return t->v();
+  case 'u':
+// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'U'
+return u->v();
   }
 }


Index: test/ubsan/TestCases/TypeCheck/null.cpp
===
--- test/ubsan/TestCases/TypeCheck/null.cpp
+++ test/ubsan/TestCases/TypeCheck/null.cpp
@@ -1,20 +1,34 @@
-// RUN: %clangxx -fsanitize=null %s -O3 -o %t
-// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
-// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
-// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
-// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
-// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null %s -O3 -o %t
+// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
+// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
+// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
+// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
+// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL
+// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2
 
 struct S {
   int f() { return 0; }
   int k;
 };
 
+struct T {
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  virtual int v() { return 2; }
+};
+
 int main(int, char **argv) {
   int *p = 0;
   S *s = 0;
+  T *t = 0;
+  U *u = 0;
 
   (void)*p; // ok!
+  (void)*t; // ok!
+  (void)*u; // ok!
 
   switch (argv[1][0]) {
   case 'l':
@@ -34,5 +48,11 @@
   case 'f':
 // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'S'
 return s->f();
+  case 't':
+// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'T'
+return t->v();
+  case 'u':
+// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'U'
+return u->v();
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26560: Add a test for vcall on a null ptr.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 78405.
krasin added a comment.

sync


https://reviews.llvm.org/D26560

Files:
  test/ubsan/TestCases/TypeCheck/null.cpp


Index: test/ubsan/TestCases/TypeCheck/null.cpp
===
--- test/ubsan/TestCases/TypeCheck/null.cpp
+++ test/ubsan/TestCases/TypeCheck/null.cpp
@@ -1,20 +1,50 @@
-// RUN: %clangxx -fsanitize=null %s -O3 -o %t
-// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
-// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
-// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
-// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
-// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null -g %s -O3 -o %t
+// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
+// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
+// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
+// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
+// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL
+// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2
+
+#include 
 
 struct S {
   int f() { return 0; }
   int k;
 };
 
+struct T {
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  virtual int v() { return 2; }
+};
+
+static inline void break_optimization(void *arg) {
+  __asm__ __volatile__("" : : "r" (arg) : "memory");
+}
+
 int main(int, char **argv) {
   int *p = 0;
   S *s = 0;
+  T *t = 0;
+  U *u = 0;
+
+  if (argv[1][0] == 'T') {
+t = new T;
+  }
+  if (argv[1][0] == 'U') {
+u = new U;
+  }
+
+  break_optimization(s);
+  break_optimization(t);
+  break_optimization(u);
 
   (void)*p; // ok!
+  (void)*t; // ok!
 
   switch (argv[1][0]) {
   case 'l':
@@ -34,5 +64,13 @@
   case 'f':
 // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'S'
 return s->f();
+  case 't':
+  case 'T':
+// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'T'
+return t->v();
+  case 'u':
+  case 'U':
+// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on 
null pointer of type 'U'
+return u->v();
   }
 }


Index: test/ubsan/TestCases/TypeCheck/null.cpp
===
--- test/ubsan/TestCases/TypeCheck/null.cpp
+++ test/ubsan/TestCases/TypeCheck/null.cpp
@@ -1,20 +1,50 @@
-// RUN: %clangxx -fsanitize=null %s -O3 -o %t
-// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
-// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
-// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
-// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
-// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null -g %s -O3 -o %t
+// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD
+// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE
+// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE
+// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER
+// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN
+// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL
+// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2
+
+#include 
 
 struct S {
   int f() { return 0; }
   int k;
 };
 
+struct T {
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  virtual int v() { return 2; }
+};
+
+static inline void break_optimization(void *arg) {
+  __asm__ __volatile__("" : : "r" (arg) : "memory");
+}
+
 int main(int, char **argv) {
   int *p = 0;
   S *s = 0;
+  T *t = 0;
+  U *u = 0;
+
+  if (argv[1][0] == 'T') {
+t = new T;
+  }
+  if (argv[1][0] == 'U') {
+u = new U;
+  }
+
+  break_optimization(s);
+  break_optimization(t);
+  break_optimization(u);
 
   (void)*p; // ok!
+  (void)*t; // ok!
 
   switch (argv[1][0]) {
   case 'l':
@@ -34,5 +64,13 @@
   case 'f':
 // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'S'
 return s->f();
+  case 't':
+  case 'T':
+// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'T'
+return t->v();
+  case 'u':
+  case 'U':
+// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'U'
+return u->v();
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r287185 - Explicitly specify that ubsan-vtable-checks is x86-64.

[Ivan Krasin via cfe-commits]

Author: krasin
Date: Wed Nov 16 19:09:04 2016
New Revision: 287185

URL: http://llvm.org/viewvc/llvm-project?rev=287185=rev
Log:
Explicitly specify that ubsan-vtable-checks is x86-64.

This should fix a failure on PowerPC introduced by r287181.

Modified:
cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp

Modified: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp?rev=287185=287184=287185=diff
==
--- cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp Wed Nov 16 19:09:04 2016
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=ITANIUM
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=MSABI
-// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=ITANIUM
 // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=MSABI
 struct T {
   virtual ~T() {}


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

r287181 - Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

Author: krasin
Date: Wed Nov 16 18:39:48 2016
New Revision: 287181

URL: http://llvm.org/viewvc/llvm-project?rev=287181=rev
Log:
Insert a type check before reading vtable.

Summary:
this is to prevent a situation when a pointer is invalid or null,
but we get to reading from vtable before we can check that
(possibly causing a segfault without a good diagnostics).

Reviewers: pcc

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
Modified:
cfe/trunk/lib/CodeGen/CGExprCXX.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=287181=287180=287181=diff
==
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Wed Nov 16 18:39:48 2016
@@ -35,17 +35,6 @@ commonEmitCXXMemberOrOperatorCall(CodeGe
  "Trying to emit a member or operator call expr on a static method!");
   ASTContext  = CGF.getContext();
 
-  // C++11 [class.mfct.non-static]p2:
-  //   If a non-static member function of a class X is called for an object 
that
-  //   is not of type X, or of a type derived from X, the behavior is 
undefined.
-  SourceLocation CallLoc;
-  if (CE)
-CallLoc = CE->getExprLoc();
-  CGF.EmitTypeCheck(isa(MD)
-? CodeGenFunction::TCK_ConstructorCall
-: CodeGenFunction::TCK_MemberCall,
-CallLoc, This, C.getRecordType(MD->getParent()));
-
   // Push the this ptr.
   const CXXRecordDecl *RD =
   CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@ RValue CodeGenFunction::EmitCXXMemberOrO
 
   llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object 
that
+  //   is not of type X, or of a type derived from X, the behavior is 
undefined.
+  SourceLocation CallLoc;
+  ASTContext  = getContext();
+  if (CE)
+CallLoc = CE->getExprLoc();
+
+  EmitTypeCheck(isa(CalleeDecl)
+? CodeGenFunction::TCK_ConstructorCall
+: CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(), 
C.getRecordType(CalleeDecl->getParent()));
+
   // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
   // 'CalleeDecl' instead.
 
@@ -1763,6 +1765,15 @@ static void EmitObjectDelete(CodeGenFunc
  const CXXDeleteExpr *DE,
  Address Ptr,
  QualType ElementType) {
+  // C++11 [expr.delete]p3:
+  //   If the static type of the object to be deleted is different from its
+  //   dynamic type, the static type shall be a base class of the dynamic type
+  //   of the object to be deleted and the static type shall have a virtual
+  //   destructor or the behavior is undefined.
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+DE->getExprLoc(), Ptr.getPointer(),
+ElementType);
+
   // Find the destructor for the type, if applicable.  If the
   // destructor is virtual, we'll just emit the vcall and return.
   const CXXDestructorDecl *Dtor = nullptr;

Added: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp?rev=287181=auto
==
--- cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp Wed Nov 16 18:39:48 2016
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+  virtual ~T() {}
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  ~U();
+  virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} 
(%struct.T*{{.*}})***
+  // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* 
%{{[_a-z0-9]+}}, null
+  // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], 

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL287181: Insert a type check before reading vtable. (authored 
by krasin).

Changed prior to commit:
  https://reviews.llvm.org/D26559?vs=78279=78288#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26559

Files:
  cfe/trunk/lib/CodeGen/CGExprCXX.cpp
  cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp

Index: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
+++ cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+  virtual ~T() {}
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  ~U();
+  virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null
+  // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}}
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  return t->v();
+}
+
+// ITANIUM: define void @_Z9delete_itP1T
+// MSABI: define void @"\01?delete_it
+void delete_it(T *t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-VPTR: br i1 {{.*}} label %{{.*}}
+  // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  delete t;
+}
Index: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@
  "Trying to emit a member or operator call expr on a static method!");
   ASTContext  = CGF.getContext();
 
-  // C++11 [class.mfct.non-static]p2:
-  //   If a non-static member function of a class X is called for an object that
-  //   is not of type X, or of a type derived from X, the behavior is undefined.
-  SourceLocation CallLoc;
-  if (CE)
-CallLoc = CE->getExprLoc();
-  CGF.EmitTypeCheck(isa(MD)
-? CodeGenFunction::TCK_ConstructorCall
-: CodeGenFunction::TCK_MemberCall,
-CallLoc, This, C.getRecordType(MD->getParent()));
-
   // Push the this ptr.
   const CXXRecordDecl *RD =
   CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@
 
   llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object that
+  //   is not of type X, or of a type derived from X, the behavior is undefined.
+  SourceLocation CallLoc;
+  ASTContext  = getContext();
+  if (CE)
+CallLoc = CE->getExprLoc();
+
+  EmitTypeCheck(isa(CalleeDecl)
+? CodeGenFunction::TCK_ConstructorCall
+: CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()));
+
   // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
   // 'CalleeDecl' instead.
 
@@ -1763,6 +1765,15 @@
  const CXXDeleteExpr *DE,
  Address Ptr,
  QualType ElementType) {
+  // C++11 [expr.delete]p3:
+  //   If the static type of the object to be deleted is different from its
+  //   dynamic type, the static type shall be a base class of the dynamic type
+  //   of the object to be deleted and the static type shall have a virtual
+  //   destructor or the behavior is undefined.
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+DE->getExprLoc(), 

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 78279.
krasin added a comment.

inline


https://reviews.llvm.org/D26559

Files:
  lib/CodeGen/CGExprCXX.cpp
  test/CodeGenCXX/ubsan-vtable-checks.cpp


Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+  virtual ~T() {}
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  ~U();
+  virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} 
(%struct.T*{{.*}})***
+  // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* 
%{{[_a-z0-9]+}}, null
+  // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}}
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+  // Second, we check that vtable is actually loaded once the type check is 
done.
+  // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  return t->v();
+}
+
+// ITANIUM: define void @_Z9delete_itP1T
+// MSABI: define void @"\01?delete_it
+void delete_it(T *t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} 
(%struct.T*{{.*}})***
+  // CHECK-VPTR: br i1 {{.*}} label %{{.*}}
+  // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
+  // Second, we check that vtable is actually loaded once the type check is 
done.
+  // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  delete t;
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@
  "Trying to emit a member or operator call expr on a static method!");
   ASTContext  = CGF.getContext();
 
-  // C++11 [class.mfct.non-static]p2:
-  //   If a non-static member function of a class X is called for an object 
that
-  //   is not of type X, or of a type derived from X, the behavior is 
undefined.
-  SourceLocation CallLoc;
-  if (CE)
-CallLoc = CE->getExprLoc();
-  CGF.EmitTypeCheck(isa(MD)
-? CodeGenFunction::TCK_ConstructorCall
-: CodeGenFunction::TCK_MemberCall,
-CallLoc, This, C.getRecordType(MD->getParent()));
-
   // Push the this ptr.
   const CXXRecordDecl *RD =
   CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@
 
   llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object 
that
+  //   is not of type X, or of a type derived from X, the behavior is 
undefined.
+  SourceLocation CallLoc;
+  ASTContext  = getContext();
+  if (CE)
+CallLoc = CE->getExprLoc();
+
+  EmitTypeCheck(isa(CalleeDecl)
+? CodeGenFunction::TCK_ConstructorCall
+: CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(), 
C.getRecordType(CalleeDecl->getParent()));
+
   // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
   // 'CalleeDecl' instead.
 
@@ -1763,6 +1765,10 @@
  const CXXDeleteExpr *DE,
  Address Ptr,
  QualType ElementType) {
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+DE->getExprLoc(), Ptr.getPointer(),
+ElementType);
+
   // Find the destructor for the type, if applicable.  If the
   // destructor is virtual, we'll just emit the vcall and return.
   const CXXDestructorDecl *Dtor = nullptr;


Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: 

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 78277.
krasin added a comment.

Address minor comment.


https://reviews.llvm.org/D26559

Files:
  lib/CodeGen/CGExprCXX.cpp
  test/CodeGenCXX/ubsan-vtable-checks.cpp


Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm 
-fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm 
-fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+  virtual ~T() {}
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  ~U();
+  virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} 
(%struct.T*{{.*}})***
+  // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* 
%{{[_a-z0-9]+}}, null
+  // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}}
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+  // Second, we check that vtable is actually loaded once the type check is 
done.
+  // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  return t->v();
+}
+
+// ITANIUM: define void @_Z9delete_itP1T
+// MSABI: define void @"\01?delete_it
+void delete_it(T *t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} 
(%struct.T*{{.*}})***
+  // CHECK-VPTR: br i1 {{.*}} label %{{.*}}
+  // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
+  // Second, we check that vtable is actually loaded once the type check is 
done.
+  // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  delete t;
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@
  "Trying to emit a member or operator call expr on a static method!");
   ASTContext  = CGF.getContext();
 
-  // C++11 [class.mfct.non-static]p2:
-  //   If a non-static member function of a class X is called for an object 
that
-  //   is not of type X, or of a type derived from X, the behavior is 
undefined.
-  SourceLocation CallLoc;
-  if (CE)
-CallLoc = CE->getExprLoc();
-  CGF.EmitTypeCheck(isa(MD)
-? CodeGenFunction::TCK_ConstructorCall
-: CodeGenFunction::TCK_MemberCall,
-CallLoc, This, C.getRecordType(MD->getParent()));
-
   // Push the this ptr.
   const CXXRecordDecl *RD =
   CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@
 
   llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object 
that
+  //   is not of type X, or of a type derived from X, the behavior is 
undefined.
+  SourceLocation CallLoc;
+  ASTContext  = getContext();
+  if (CE)
+CallLoc = CE->getExprLoc();
+
+  EmitTypeCheck(isa(CalleeDecl)
+? CodeGenFunction::TCK_ConstructorCall
+: CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(), 
C.getRecordType(CalleeDecl->getParent()));
+
   // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
   // 'CalleeDecl' instead.
 
@@ -1763,6 +1765,12 @@
  const CXXDeleteExpr *DE,
  Address Ptr,
  QualType ElementType) {
+  SourceLocation CallLoc;
+  CallLoc = DE->getExprLoc();
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+CallLoc, Ptr.getPointer(),
+ElementType);
+
   // Find the destructor for the type, if applicable.  If the
   // destructor is virtual, we'll just emit the vcall and return.
   const CXXDestructorDecl *Dtor = nullptr;


Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s 

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 78276.
krasin added a comment.

Fix the test under -Asserts build.


https://reviews.llvm.org/D26559

Files:
  lib/CodeGen/CGExprCXX.cpp
  test/CodeGenCXX/ubsan-vtable-checks.cpp

Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+  virtual ~T() {}
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  ~U();
+  virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null
+  // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}}
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  return t->v();
+}
+
+// ITANIUM: define void @_Z9delete_itP1T
+// MSABI: define void @"\01?delete_it
+void delete_it(T *t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-VPTR: br i1 {{.*}} label %{{.*}}
+  // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  delete t;
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@
  "Trying to emit a member or operator call expr on a static method!");
   ASTContext  = CGF.getContext();
 
-  // C++11 [class.mfct.non-static]p2:
-  //   If a non-static member function of a class X is called for an object that
-  //   is not of type X, or of a type derived from X, the behavior is undefined.
-  SourceLocation CallLoc;
-  if (CE)
-CallLoc = CE->getExprLoc();
-  CGF.EmitTypeCheck(isa(MD)
-? CodeGenFunction::TCK_ConstructorCall
-: CodeGenFunction::TCK_MemberCall,
-CallLoc, This, C.getRecordType(MD->getParent()));
-
   // Push the this ptr.
   const CXXRecordDecl *RD =
   CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@
 
   llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object that
+  //   is not of type X, or of a type derived from X, the behavior is undefined.
+  SourceLocation CallLoc;
+  ASTContext  = getContext();
+  if (CE)
+CallLoc = CE->getExprLoc();
+
+  EmitTypeCheck(isa(CalleeDecl)
+? CodeGenFunction::TCK_ConstructorCall
+: CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()));
+
   // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
   // 'CalleeDecl' instead.
 
@@ -1763,6 +1765,13 @@
  const CXXDeleteExpr *DE,
  Address Ptr,
  QualType ElementType) {
+  SourceLocation CallLoc;
+  if (DE)
+CallLoc = DE->getExprLoc();
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+CallLoc, Ptr.getPointer(),
+ElementType);
+
   // Find the destructor for the type, if applicable.  If the
   // destructor is virtual, we'll just emit the vcall and return.
   const CXXDestructorDecl *Dtor = nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 78240.
krasin marked an inline comment as done.
krasin added a comment.

Sync to Clang ToT.


https://reviews.llvm.org/D26559

Files:
  lib/CodeGen/CGExprCXX.cpp
  test/CodeGenCXX/ubsan-vtable-checks.cpp

Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+  virtual ~T() {}
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  ~U();
+  virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null
+  // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  return t->v();
+}
+
+// ITANIUM: define void @_Z9delete_itP1T
+// MSABI: define void @"\01?delete_it
+void delete_it(T *t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-VPTR: br i1 {{.*}} label %handler.dynamic_type_cache_miss
+  // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  delete t;
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@
  "Trying to emit a member or operator call expr on a static method!");
   ASTContext  = CGF.getContext();
 
-  // C++11 [class.mfct.non-static]p2:
-  //   If a non-static member function of a class X is called for an object that
-  //   is not of type X, or of a type derived from X, the behavior is undefined.
-  SourceLocation CallLoc;
-  if (CE)
-CallLoc = CE->getExprLoc();
-  CGF.EmitTypeCheck(isa(MD)
-? CodeGenFunction::TCK_ConstructorCall
-: CodeGenFunction::TCK_MemberCall,
-CallLoc, This, C.getRecordType(MD->getParent()));
-
   // Push the this ptr.
   const CXXRecordDecl *RD =
   CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@
 
   llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object that
+  //   is not of type X, or of a type derived from X, the behavior is undefined.
+  SourceLocation CallLoc;
+  ASTContext  = getContext();
+  if (CE)
+CallLoc = CE->getExprLoc();
+
+  EmitTypeCheck(isa(CalleeDecl)
+? CodeGenFunction::TCK_ConstructorCall
+: CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()));
+
   // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
   // 'CalleeDecl' instead.
 
@@ -1763,6 +1765,13 @@
  const CXXDeleteExpr *DE,
  Address Ptr,
  QualType ElementType) {
+  SourceLocation CallLoc;
+  if (DE)
+CallLoc = DE->getExprLoc();
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+CallLoc, Ptr.getPointer(),
+ElementType);
+
   // Find the destructor for the type, if applicable.  If the
   // destructor is virtual, we'll just emit the vcall and return.
   const CXXDestructorDecl *Dtor = nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

.. but the follow up will be in a separate CL, as I have not even started
to work on it yet.

On Tue, Nov 15, 2016 at 6:31 PM, Ivan Krasin <kra...@google.com> wrote:

> Okay, I will keep it as a separate file then. It's very likely that I will
> soon send a follow up, as there's another very similar case known to not
> work properly (based on the experience with UBSan Vptr bot in Chromium)
>
> On Tue, Nov 15, 2016 at 6:27 PM, Richard Smith <rich...@metafoo.co.uk>
> wrote:
>
>> On Tue, Nov 15, 2016 at 6:20 PM, Ivan Krasin <kra...@google.com> wrote:
>>
>>> Thank you, Richard.
>>>
>>> Shall I merge the newly introduced test/CodeGenCXX/ubsan-vtable-checks.cpp
>>> into catch-undef-behavior.cpp or it's more clear when it's standalone?
>>>
>>
>> Up to you. catch-undef-behavior.cpp is getting unwieldy, so a separate
>> test file doesn't seem like a bad thing.
>>
>>
>>> On Tue, Nov 15, 2016 at 5:51 PM, Richard Smith <rich...@metafoo.co.uk>
>>> wrote:
>>>
>>>> On Fri, Nov 11, 2016 at 3:02 PM, Ivan Krasin via cfe-commits <
>>>> cfe-commits@lists.llvm.org> wrote:
>>>>
>>>>> krasin added a comment.
>>>>>
>>>>> Small correction: all UBSan type checks tests live in compiler-rt.
>>>>
>>>>
>>>> Actually, most of the UBSan tests live in 
>>>> test/CodeGenCXX/catch-undef-behavior.cpp
>>>> in Clang; the compiler-rt tests are merely aiming to test that the runtime
>>>> produces the correct diagnostics. There are a few more test files testing
>>>> UBSan besides that one; you can find the tests by grepping for "fsanitize="
>>>> in Clang's test/ directory.
>>>>
>>>>
>>>>> There is a test for UBsan + devirtualization inside tools/clang. My
>>>>> point still stands.
>>>>>
>>>>>
>>>>> https://reviews.llvm.org/D26559
>>>>>
>>>>>
>>>>>
>>>>> ___
>>>>> cfe-commits mailing list
>>>>> cfe-commits@lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>
>>>>
>>>>
>>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

Okay, I will keep it as a separate file then. It's very likely that I will
soon send a follow up, as there's another very similar case known to not
work properly (based on the experience with UBSan Vptr bot in Chromium)

On Tue, Nov 15, 2016 at 6:27 PM, Richard Smith <rich...@metafoo.co.uk>
wrote:

> On Tue, Nov 15, 2016 at 6:20 PM, Ivan Krasin <kra...@google.com> wrote:
>
>> Thank you, Richard.
>>
>> Shall I merge the newly introduced test/CodeGenCXX/ubsan-vtable-checks.cpp
>> into catch-undef-behavior.cpp or it's more clear when it's standalone?
>>
>
> Up to you. catch-undef-behavior.cpp is getting unwieldy, so a separate
> test file doesn't seem like a bad thing.
>
>
>> On Tue, Nov 15, 2016 at 5:51 PM, Richard Smith <rich...@metafoo.co.uk>
>> wrote:
>>
>>> On Fri, Nov 11, 2016 at 3:02 PM, Ivan Krasin via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
>>>> krasin added a comment.
>>>>
>>>> Small correction: all UBSan type checks tests live in compiler-rt.
>>>
>>>
>>> Actually, most of the UBSan tests live in 
>>> test/CodeGenCXX/catch-undef-behavior.cpp
>>> in Clang; the compiler-rt tests are merely aiming to test that the runtime
>>> produces the correct diagnostics. There are a few more test files testing
>>> UBSan besides that one; you can find the tests by grepping for "fsanitize="
>>> in Clang's test/ directory.
>>>
>>>
>>>> There is a test for UBsan + devirtualization inside tools/clang. My
>>>> point still stands.
>>>>
>>>>
>>>> https://reviews.llvm.org/D26559
>>>>
>>>>
>>>>
>>>> ___
>>>> cfe-commits mailing list
>>>> cfe-commits@lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>
>>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

Thank you, Richard.

Shall I merge the newly introduced test/CodeGenCXX/ubsan-vtable-checks.cpp
into catch-undef-behavior.cpp or it's more clear when it's standalone?

On Tue, Nov 15, 2016 at 5:51 PM, Richard Smith <rich...@metafoo.co.uk>
wrote:

> On Fri, Nov 11, 2016 at 3:02 PM, Ivan Krasin via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> krasin added a comment.
>>
>> Small correction: all UBSan type checks tests live in compiler-rt.
>
>
> Actually, most of the UBSan tests live in 
> test/CodeGenCXX/catch-undef-behavior.cpp
> in Clang; the compiler-rt tests are merely aiming to test that the runtime
> produces the correct diagnostics. There are a few more test files testing
> UBSan besides that one; you can find the tests by grepping for "fsanitize="
> in Clang's test/ directory.
>
>
>> There is a test for UBsan + devirtualization inside tools/clang. My point
>> still stands.
>>
>>
>> https://reviews.llvm.org/D26559
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin marked 2 inline comments as done.
krasin added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1820-1833
+  ASTContext  = getContext();
+  SourceLocation CallLoc = CE ? CE->getLocStart() : SourceLocation();
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(),
+Context.getRecordType(Dtor->getParent()));
+
   // We have only one destructor in the vftable but can get both behaviors

pcc wrote:
> If you undo this part do the tests still pass?
Thank you for the catch. Yes, the other check covers this case too. Reverted 
this file.


https://reviews.llvm.org/D26559



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

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 78104.
krasin added a comment.

Address comments.


https://reviews.llvm.org/D26559

Files:
  lib/CodeGen/CGExprCXX.cpp
  test/CodeGenCXX/ubsan-vtable-checks.cpp

Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+  virtual ~T() {}
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  ~U();
+  virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null
+  // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  return t->v();
+}
+
+// ITANIUM: define void @_Z9delete_itP1T
+// MSABI: define void @"\01?delete_it
+void delete_it(T *t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-VPTR: br i1 {{.*}} label %handler.dynamic_type_cache_miss
+  // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  delete t;
+}
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@
  "Trying to emit a member or operator call expr on a static method!");
   ASTContext  = CGF.getContext();
 
-  // C++11 [class.mfct.non-static]p2:
-  //   If a non-static member function of a class X is called for an object that
-  //   is not of type X, or of a type derived from X, the behavior is undefined.
-  SourceLocation CallLoc;
-  if (CE)
-CallLoc = CE->getExprLoc();
-  CGF.EmitTypeCheck(isa(MD)
-? CodeGenFunction::TCK_ConstructorCall
-: CodeGenFunction::TCK_MemberCall,
-CallLoc, This, C.getRecordType(MD->getParent()));
-
   // Push the this ptr.
   const CXXRecordDecl *RD =
   CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@
 
   llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object that
+  //   is not of type X, or of a type derived from X, the behavior is undefined.
+  SourceLocation CallLoc;
+  ASTContext  = getContext();
+  if (CE)
+CallLoc = CE->getExprLoc();
+
+  EmitTypeCheck(isa(CalleeDecl)
+? CodeGenFunction::TCK_ConstructorCall
+: CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()));
+
   // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
   // 'CalleeDecl' instead.
 
@@ -1763,6 +1765,13 @@
  const CXXDeleteExpr *DE,
  Address Ptr,
  QualType ElementType) {
+  SourceLocation CallLoc;
+  if (DE)
+CallLoc = DE->getExprLoc();
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+CallLoc, Ptr.getPointer(),
+ElementType);
+
   // Find the destructor for the type, if applicable.  If the
   // destructor is virtual, we'll just emit the vcall and return.
   const CXXDestructorDecl *Dtor = nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin added a comment.

Friendly ping.


https://reviews.llvm.org/D26559



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

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin added inline comments.



Comment at: lib/CodeGen/CGExprCXX.cpp:93
+
+  EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+CallLoc, This, C.getRecordType(DD->getParent()));

pcc wrote:
> krasin wrote:
> > pcc wrote:
> > > pcc wrote:
> > > > Is it correct to emit a type check at this point? Looking at [0] it 
> > > > looks like this function is only called from the Microsoft C++ ABI 
> > > > after we have already resolved the virtual function pointer.
> > > > 
> > > > [0] 
> > > > http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/CGExprCXX.cpp/rEmitCXXDestructorCall
> > > What about this comment?
> > Sorry, I have missed the comment. I have added this check here to preserve 
> > the existing behavior.
> > 
> > From what you describe, there could be a very similar issue related to the 
> > Microsoft C++ ABI to the one that I fix here. I am okay to file a bug or 
> > add a note about this, your choice.
> Have you looked at how hard it would be to fix this?
Done.

Note: since I don't have an ability to test on Windows, this CL is now 
high-risk. If it breaks any Windows-specific tests, when submitted, I will 
revert the CL and undo the MSABI change. After all, the previous state was to 
keep things working as they were before.


https://reviews.llvm.org/D26559



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

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 77941.
krasin added a comment.

Do better job with destructors.


https://reviews.llvm.org/D26559

Files:
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  test/CodeGenCXX/ubsan-vtable-checks.cpp

Index: test/CodeGenCXX/ubsan-vtable-checks.cpp
===
--- /dev/null
+++ test/CodeGenCXX/ubsan-vtable-checks.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI
+// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI
+struct T {
+  virtual ~T() {}
+  virtual int v() { return 1; }
+};
+
+struct U : T {
+  ~U();
+  virtual int v() { return 2; }
+};
+
+U::~U() {}
+
+// ITANIUM: define i32 @_Z5get_vP1T
+// MSABI: define i32 @"\01?get_v
+int get_v(T* t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null
+  // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch
+  // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  return t->v();
+}
+
+// ITANIUM: define void @_Z9delete_itP1T
+// MSABI: define void @"\01?delete_it
+void delete_it(T *t) {
+  // First, we check that vtable is not loaded before a type check.
+  // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  // CHECK-VPTR: br i1 {{.*}} label %handler.dynamic_type_cache_miss
+  // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort
+  // Second, we check that vtable is actually loaded once the type check is done.
+  // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})***
+  delete t;
+}
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1817,16 +1817,21 @@
   assert(CE == nullptr || CE->arg_begin() == CE->arg_end());
   assert(DtorType == Dtor_Deleting || DtorType == Dtor_Complete);
 
+  ASTContext  = getContext();
+  SourceLocation CallLoc = CE ? CE->getLocStart() : SourceLocation();
+  CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+CallLoc, This.getPointer(),
+Context.getRecordType(Dtor->getParent()));
+
   // We have only one destructor in the vftable but can get both behaviors
   // by passing an implicit int parameter.
   GlobalDecl GD(Dtor, Dtor_Deleting);
   const CGFunctionInfo *FInfo = ().arrangeCXXStructorDeclaration(
   Dtor, StructorType::Deleting);
   llvm::Type *Ty = CGF.CGM.getTypes().GetFunctionType(*FInfo);
   CGCallee Callee = getVirtualFunctionPointer(
-  CGF, GD, This, Ty, CE ? CE->getLocStart() : SourceLocation());
+  CGF, GD, This, Ty, CallLoc);
 
-  ASTContext  = getContext();
   llvm::Value *ImplicitParam = llvm::ConstantInt::get(
   llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()),
   DtorType == Dtor_Deleting);
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -35,17 +35,6 @@
  "Trying to emit a member or operator call expr on a static method!");
   ASTContext  = CGF.getContext();
 
-  // C++11 [class.mfct.non-static]p2:
-  //   If a non-static member function of a class X is called for an object that
-  //   is not of type X, or of a type derived from X, the behavior is undefined.
-  SourceLocation CallLoc;
-  if (CE)
-CallLoc = CE->getExprLoc();
-  CGF.EmitTypeCheck(isa(MD)
-? CodeGenFunction::TCK_ConstructorCall
-: CodeGenFunction::TCK_MemberCall,
-CallLoc, This, C.getRecordType(MD->getParent()));
-
   // Push the this ptr.
   const CXXRecordDecl *RD =
   CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
@@ -293,6 +282,19 @@
 
   llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo);
 
+  // C++11 [class.mfct.non-static]p2:
+  //   If a non-static member function of a class X is called for an object that
+  //   is not of type X, or of a type derived from X, the 

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin added inline comments.



Comment at: lib/CodeGen/CGExprCXX.cpp:93
+
+  EmitTypeCheck(CodeGenFunction::TCK_MemberCall,
+CallLoc, This, C.getRecordType(DD->getParent()));

pcc wrote:
> pcc wrote:
> > Is it correct to emit a type check at this point? Looking at [0] it looks 
> > like this function is only called from the Microsoft C++ ABI after we have 
> > already resolved the virtual function pointer.
> > 
> > [0] 
> > http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/CGExprCXX.cpp/rEmitCXXDestructorCall
> What about this comment?
Sorry, I have missed the comment. I have added this check here to preserve the 
existing behavior.

From what you describe, there could be a very similar issue related to the 
Microsoft C++ ABI to the one that I fix here. I am okay to file a bug or add a 
note about this, your choice.


https://reviews.llvm.org/D26559



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

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin added a comment.

Small correction: all UBSan type checks tests live in compiler-rt. There is a 
test for UBsan + devirtualization inside tools/clang. My point still stands.


https://reviews.llvm.org/D26559



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

[PATCH] D26559: Insert a type check before reading vtable.

[Ivan Krasin via cfe-commits]

krasin added a comment.

All UBSan test cases happen to live in compiler-rt. I have added a couple of 
test cases in https://reviews.llvm.org/D26560.

Also, I am not against adding Clang counterparts for them which would test IL 
instead of the output from running. But it probably deserves a separate CL, as 
it has nothing to do with fixing this particular bug.


https://reviews.llvm.org/D26559



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

[libcxx] r280335 - Fix libc++ configuration with -fsanitize-coverage

[Ivan Krasin via cfe-commits]

Author: krasin
Date: Wed Aug 31 20:38:32 2016
New Revision: 280335

URL: http://llvm.org/viewvc/llvm-project?rev=280335=rev
Log:
Fix libc++ configuration with -fsanitize-coverage

Summary:
a recent change (r280015) in libc++ configuration broke LibFuzzer bot:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fuzzer/builds/12245

It's not restricted just to that bot; any code that uses the sanitize coverage 
and configures libc++ hits it.

This CL fixes the issue.

Reviewers: compnerd

Subscribers: aizatsky

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

Modified:
libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake
libcxx/trunk/cmake/config-ix.cmake

Modified: libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake?rev=280335=280334=280335=diff
==
--- libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake (original)
+++ libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake Wed Aug 31 20:38:32 2016
@@ -16,6 +16,9 @@ function(check_cxx_atomics varname)
   if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize)
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all")
   endif()
+  if (CMAKE_C_FLAGS MATCHES -fsanitize-coverage OR CMAKE_CXX_FLAGS MATCHES 
-fsanitize-coverage)
+set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} 
-fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters")
+  endif()
   check_cxx_source_compiles("
 #include 
 #include 

Modified: libcxx/trunk/cmake/config-ix.cmake
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/config-ix.cmake?rev=280335=280334=280335=diff
==
--- libcxx/trunk/cmake/config-ix.cmake (original)
+++ libcxx/trunk/cmake/config-ix.cmake Wed Aug 31 20:38:32 2016
@@ -27,6 +27,9 @@ if (LIBCXX_SUPPORTS_NODEFAULTLIBS_FLAG)
   if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize)
 set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all")
   endif ()
+  if (CMAKE_C_FLAGS MATCHES -fsanitize-coverage OR CMAKE_CXX_FLAGS MATCHES 
-fsanitize-coverage)
+set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} 
-fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters")
+  endif ()
 endif ()
 
 include(CheckLibcxxAtomic)


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

Re: [PATCH] D15921: [analyzer] Utility to match function calls.

[Ivan Krasin via cfe-commits]

krasin added a subscriber: krasin.
krasin added a comment.

FYI: this revision has likely broken the build: 
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/9561

FAIL: Clang :: Analysis/simple-stream-checks.c (367 of 8927)

- TEST 'Clang :: Analysis/simple-stream-checks.c' FAILED 

Script:
---

/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/./bin/clang
 -cc1 -internal-isystem 
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/../lib/clang/3.9.0/include
 -nostdsysteminc -analyze -analyzer-checker=core,alpha.unix.SimpleStream 
-verify 
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/test/Analysis/simple-stream-checks.c
--


Repository:
  rL LLVM

http://reviews.llvm.org/D15921



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

Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 33849.
krasin added a comment.

Addressed a nit.


http://reviews.llvm.org/D12544

Files:
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/Inputs/resource_dir/asan_blacklist.txt
  test/Driver/fsanitize-blacklist.c
  test/Frontend/print-header-includes.c

Index: test/Frontend/print-header-includes.c
===
--- test/Frontend/print-header-includes.c
+++ test/Frontend/print-header-includes.c
@@ -14,7 +14,7 @@
 // MS-NOT: Note
 
 // RUN: echo "fun:foo" > %t.blacklist
-// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout
+// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout
 // RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s
 // MS-BLACKLIST: Note: including file: {{.*\.blacklist}}
 // MS-BLACKLIST: Note: including file: {{.*test.h}}
Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -13,13 +13,25 @@
 // RUN: echo "badline" > %t.bad
 
 // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST
-// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good
-// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.second
+// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good" "-fsanitize-blacklist={{.*}}.second
+
+// Now, check for -fdepfile-entry flags.
+// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST2
+// CHECK-BLACKLIST2: -fdepfile-entry={{.*}}.good" "-fdepfile-entry={{.*}}.second
+
+// Check that the default blacklist is not added as an extra dependency.
+// RUN: %clang -fsanitize=address -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST --implicit-check-not=fdepfile-entry
+// CHECK-DEFAULT-BLACKLIST: -fsanitize-blacklist={{.*}}asan_blacklist.txt
 
 // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
 // RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE --check-prefix=DELIMITERS
 // CHECK-NO-SANITIZE-NOT: -fsanitize-blacklist
 
+// Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
+// Now, check for the absense of -fdepfile-entry flags.
+// RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE2 --check-prefix=DELIMITERS
+// CHECK-NO-SANITIZE2-NOT: -fdepfile-entry
+
 // Flag -fno-sanitize-blacklist wins if it is specified later.
 // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fno-sanitize-blacklist %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-BLACKLIST --check-prefix=DELIMITERS
 // CHECK-NO-BLACKLIST-NOT: -fsanitize-blacklist
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -709,7 +709,7 @@
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
-  Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist);
+  Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry);
   auto ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ExtraDeps.insert(Opts.ExtraDeps.end(), ModuleFiles.begin(),
 ModuleFiles.end());
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -176,6 +176,7 @@
   RecoverableSanitizers.clear();
   TrapSanitizers.clear();
   BlacklistFiles.clear();
+  ExtraDeps.clear();
   CoverageFeatures = 0;
   MsanTrackOrigins = 0;
   MsanUseAfterDtor = false;
@@ -383,13 +384,16 @@
 if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) {
   Arg->claim();
   std::string BLPath = Arg->getValue();
-  if (llvm::sys::fs::exists(BLPath))
+  if (llvm::sys::fs::exists(BLPath)) {
 BlacklistFiles.push_back(BLPath);
-  else
+ExtraDeps.push_back(BLPath);
+  } else
 D.Diag(clang::diag::err_drv_no_such_file) << BLPath;
+
 } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) {
   Arg->claim();
   BlacklistFiles.clear();
+  ExtraDeps.clear();
 }
   }
   // Validate blacklists format.
@@ -563,6 +567,11 @@
 BlacklistOpt += BLPath;
 CmdArgs.push_back(Args.MakeArgString(BlacklistOpt));
   }
+  for (const auto  : ExtraDeps) {
+SmallString<64> 

Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 33764.
krasin added a comment.

Add a test for default blacklist.


http://reviews.llvm.org/D12544

Files:
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/Inputs/resource_dir/asan_blacklist.txt
  test/Driver/fsanitize-blacklist.c
  test/Frontend/print-header-includes.c

Index: test/Frontend/print-header-includes.c
===
--- test/Frontend/print-header-includes.c
+++ test/Frontend/print-header-includes.c
@@ -14,7 +14,7 @@
 // MS-NOT: Note
 
 // RUN: echo "fun:foo" > %t.blacklist
-// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout
+// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout
 // RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s
 // MS-BLACKLIST: Note: including file: {{.*\.blacklist}}
 // MS-BLACKLIST: Note: including file: {{.*test.h}}
Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -13,13 +13,26 @@
 // RUN: echo "badline" > %t.bad
 
 // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST
-// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good
-// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.second
+// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good" "-fsanitize-blacklist={{.*}}.second
+
+// Now, check for -fdepfile-entry flags.
+// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST2
+// CHECK-BLACKLIST2: -fdepfile-entry={{.*}}.good" "-fdepfile-entry={{.*}}.second
+
+// Check that the default blacklist is not added as an extra dependency.
+// RUN: %clang -fsanitize=address -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST
+// CHECK-DEFAULT-BLACKLIST: -fsanitize-blacklist={{.*}}asan_blacklist.txt
+// CHECK-DEFAULT-BLACKLIST-NOT: -fdepfile-entry
 
 // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
 // RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE --check-prefix=DELIMITERS
 // CHECK-NO-SANITIZE-NOT: -fsanitize-blacklist
 
+// Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
+// Now, check for the absense of -fdepfile-entry flags.
+// RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE2 --check-prefix=DELIMITERS
+// CHECK-NO-SANITIZE2-NOT: -fdepfile-entry
+
 // Flag -fno-sanitize-blacklist wins if it is specified later.
 // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fno-sanitize-blacklist %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-BLACKLIST --check-prefix=DELIMITERS
 // CHECK-NO-BLACKLIST-NOT: -fsanitize-blacklist
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -709,7 +709,7 @@
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
-  Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist);
+  Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry);
   auto ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ExtraDeps.insert(Opts.ExtraDeps.end(), ModuleFiles.begin(),
 ModuleFiles.end());
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -176,6 +176,7 @@
   RecoverableSanitizers.clear();
   TrapSanitizers.clear();
   BlacklistFiles.clear();
+  ExtraDeps.clear();
   CoverageFeatures = 0;
   MsanTrackOrigins = 0;
   MsanUseAfterDtor = false;
@@ -383,13 +384,16 @@
 if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) {
   Arg->claim();
   std::string BLPath = Arg->getValue();
-  if (llvm::sys::fs::exists(BLPath))
+  if (llvm::sys::fs::exists(BLPath)) {
 BlacklistFiles.push_back(BLPath);
-  else
+ExtraDeps.push_back(BLPath);
+  } else
 D.Diag(clang::diag::err_drv_no_such_file) << BLPath;
+
 } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) {
   Arg->claim();
   BlacklistFiles.clear();
+  ExtraDeps.clear();
 }
   }
   // Validate blacklists format.
@@ -563,6 +567,11 @@
 BlacklistOpt += BLPath;
 CmdArgs.push_back(Args.MakeArgString(BlacklistOpt));
   }
+  for (const auto  : ExtraDeps) {

Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.

[Ivan Krasin via cfe-commits]

krasin added a comment.

In http://reviews.llvm.org/D12544#237819, @pcc wrote:

> It's probably time to add something like the driver test I was talking about 
> in http://reviews.llvm.org/D12021. Without that, you will have no test 
> coverage for the functional change here.


Done, please, take a look.


http://reviews.llvm.org/D12544



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

Re: r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

Thank you, Yaron.

Sorry for breaking the build, it obviously passed locally and I didn't
think about all the variety of the supported configs. :(

Does LLVM have try bots, so that I can run the tests with my patch before
committing it?

krasin

On Thu, Aug 13, 2015 at 3:40 AM, Renato Golin renato.go...@linaro.org
wrote:

 On 13 August 2015 at 07:15, Yaron Keren via cfe-commits
 cfe-commits@lists.llvm.org wrote:
  CHECK-EIGHT is failing bots, see
 
 
 http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24306/steps/check-all/logs/FAIL%3A%20Clang%3A%3Adependency-gen.c


 That check only works if compiler-rt is built in, and supports the
 target. I wouldn't put that in a Clang test.

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

Re: r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

I have created http://reviews.llvm.org/D12021, please take a look.

On Thu, Aug 13, 2015 at 4:13 PM, Ivan Krasin kra...@google.com wrote:

 Hi there,

 I will remove both test cases, which rely on the existence of the default
 blacklist. It's handled by the driver anyway, and the code I changed is in
 the frontend; at this point, there's no such a thing, as a default
 blacklist, all of them are explicit.
 Just a sec.

 krasin

 On Thu, Aug 13, 2015 at 4:09 PM, NAKAMURA Takumi geek4ci...@gmail.com
 wrote:

 Tweaked a test in r244970.
 Could you split it out if you would like to run it for the default target?
 On Fri, Aug 14, 2015 at 5:07 AM Ivan Krasin via cfe-commits 
 cfe-commits@lists.llvm.org wrote:

 Thank you, Yaron. Understood.

 On Thu, Aug 13, 2015 at 11:04 AM, Yaron Keren yaron.ke...@gmail.com
 wrote:

 There is great variety of OSs, compilers, configs in the bots. You
 can't get the smae with try bots unless everything is duplicated which is a
 waste. You test locally, commit, watch the bots here:

  http://lab.llvm.org:8011/grid

 or wait for buildbot failure e-mails which arrive a bit slower.
 Breaks may happen even if you test locally, so it's critical be around
 after the commit to to resolve such issues.

  http://llvm.org/docs/DeveloperPolicy.html#quality



 2015-08-13 20:51 GMT+03:00 Ivan Krasin kra...@google.com:

 Thank you, Yaron.

 Sorry for breaking the build, it obviously passed locally and I didn't
 think about all the variety of the supported configs. :(

 Does LLVM have try bots, so that I can run the tests with my patch
 before committing it?

 krasin


 On Thu, Aug 13, 2015 at 3:40 AM, Renato Golin renato.go...@linaro.org
 wrote:

 On 13 August 2015 at 07:15, Yaron Keren via cfe-commits
 cfe-commits@lists.llvm.org wrote:
  CHECK-EIGHT is failing bots, see
 
 
 http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24306/steps/check-all/logs/FAIL%3A%20Clang%3A%3Adependency-gen.c


 That check only works if compiler-rt is built in, and supports the
 target. I wouldn't put that in a Clang test.



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



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

[PATCH] D12021: Remove test cases, which rely on the default sanitizer blacklists.The default blacklists may vary across different architectures andconfigurations. It was not wise to include into http

[Ivan Krasin via cfe-commits]

krasin created this revision.
krasin added a reviewer: chapuni.
krasin added subscribers: pcc, cfe-commits.

http://reviews.llvm.org/D12021

Files:
  test/Frontend/dependency-gen.c

Index: test/Frontend/dependency-gen.c
===
--- test/Frontend/dependency-gen.c
+++ test/Frontend/dependency-gen.c
@@ -24,11 +24,6 @@
 // RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto 
-fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s
 // CHECK-SEVEN: .blacklist
 // CHECK-SEVEN: {{ }}x.h
-// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only 
-fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s
-// CHECK-EIGHT: {{ }}x.h
-// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only 
-fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck 
-check-prefix=CHECK-NINE %s
-// CHECK-NINE-NOT: asan_blacklist.txt
-// CHECK-NINE: {{ }}x.h
 #ifndef INCLUDE_FLAG_TEST
 #include x.h
 #endif


Index: test/Frontend/dependency-gen.c
===
--- test/Frontend/dependency-gen.c
+++ test/Frontend/dependency-gen.c
@@ -24,11 +24,6 @@
 // RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s
 // CHECK-SEVEN: .blacklist
 // CHECK-SEVEN: {{ }}x.h
-// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s
-// CHECK-EIGHT: {{ }}x.h
-// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s
-// CHECK-NINE-NOT: asan_blacklist.txt
-// CHECK-NINE: {{ }}x.h
 #ifndef INCLUDE_FLAG_TEST
 #include x.h
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

Hi there,

I will remove both test cases, which rely on the existence of the default
blacklist. It's handled by the driver anyway, and the code I changed is in
the frontend; at this point, there's no such a thing, as a default
blacklist, all of them are explicit.
Just a sec.

krasin

On Thu, Aug 13, 2015 at 4:09 PM, NAKAMURA Takumi geek4ci...@gmail.com
wrote:

 Tweaked a test in r244970.
 Could you split it out if you would like to run it for the default target?
 On Fri, Aug 14, 2015 at 5:07 AM Ivan Krasin via cfe-commits 
 cfe-commits@lists.llvm.org wrote:

 Thank you, Yaron. Understood.

 On Thu, Aug 13, 2015 at 11:04 AM, Yaron Keren yaron.ke...@gmail.com
 wrote:

 There is great variety of OSs, compilers, configs in the bots. You can't
 get the smae with try bots unless everything is duplicated which is a
 waste. You test locally, commit, watch the bots here:

  http://lab.llvm.org:8011/grid

 or wait for buildbot failure e-mails which arrive a bit slower.
 Breaks may happen even if you test locally, so it's critical be around
 after the commit to to resolve such issues.

  http://llvm.org/docs/DeveloperPolicy.html#quality



 2015-08-13 20:51 GMT+03:00 Ivan Krasin kra...@google.com:

 Thank you, Yaron.

 Sorry for breaking the build, it obviously passed locally and I didn't
 think about all the variety of the supported configs. :(

 Does LLVM have try bots, so that I can run the tests with my patch
 before committing it?

 krasin


 On Thu, Aug 13, 2015 at 3:40 AM, Renato Golin renato.go...@linaro.org
 wrote:

 On 13 August 2015 at 07:15, Yaron Keren via cfe-commits
 cfe-commits@lists.llvm.org wrote:
  CHECK-EIGHT is failing bots, see
 
 
 http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24306/steps/check-all/logs/FAIL%3A%20Clang%3A%3Adependency-gen.c


 That check only works if compiler-rt is built in, and supports the
 target. I wouldn't put that in a Clang test.



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


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

Re: r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

Thank you, Yaron. Understood.

On Thu, Aug 13, 2015 at 11:04 AM, Yaron Keren yaron.ke...@gmail.com wrote:

 There is great variety of OSs, compilers, configs in the bots. You can't
 get the smae with try bots unless everything is duplicated which is a
 waste. You test locally, commit, watch the bots here:

  http://lab.llvm.org:8011/grid

 or wait for buildbot failure e-mails which arrive a bit slower.
 Breaks may happen even if you test locally, so it's critical be around
 after the commit to to resolve such issues.

  http://llvm.org/docs/DeveloperPolicy.html#quality



 2015-08-13 20:51 GMT+03:00 Ivan Krasin kra...@google.com:

 Thank you, Yaron.

 Sorry for breaking the build, it obviously passed locally and I didn't
 think about all the variety of the supported configs. :(

 Does LLVM have try bots, so that I can run the tests with my patch before
 committing it?

 krasin


 On Thu, Aug 13, 2015 at 3:40 AM, Renato Golin renato.go...@linaro.org
 wrote:

 On 13 August 2015 at 07:15, Yaron Keren via cfe-commits
 cfe-commits@lists.llvm.org wrote:
  CHECK-EIGHT is failing bots, see
 
 
 http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24306/steps/check-all/logs/FAIL%3A%20Clang%3A%3Adependency-gen.c


 That check only works if compiler-rt is built in, and supports the
 target. I wouldn't put that in a Clang test.



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

Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 31979.
krasin added a comment.

Windows compat


http://reviews.llvm.org/D11968

Files:
  include/clang/Frontend/DependencyOutputOptions.h
  include/clang/Frontend/Utils.h
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/DependencyFile.cpp
  lib/Frontend/HeaderIncludeGen.cpp
  test/Frontend/dependency-gen.c

Index: test/Frontend/dependency-gen.c
===
--- test/Frontend/dependency-gen.c
+++ test/Frontend/dependency-gen.c
@@ -20,7 +20,16 @@
 // RUN: cd a/b
 // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s
 // CHECK-SIX: {{ }}x.h
-
+// RUN: echo fun:foo  %t.blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s
+// CHECK-SEVEN: {{ }}x.h
+// CHECK-SEVEN: .blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s
+// CHECK-EIGHT: {{ }}x.h
+// CHECK-EIGHT: asan_blacklist.txt
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s
+// CHECK-NINE: {{ }}x.h
+// CHECK-NINE-NOT: asan_blacklist.txt
 #ifndef INCLUDE_FLAG_TEST
 #include x.h
 #endif
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -46,7 +46,36 @@
 };
 }
 
-void clang::AttachHeaderIncludeGen(Preprocessor PP, bool ShowAllHeaders,
+static void PrintHeaderInfo(raw_ostream *OutputFile, const char* Filename,
+bool ShowDepth, unsigned CurrentIncludeDepth,
+bool MSStyle) {
+// Write to a temporary string to avoid unnecessary flushing on errs().
+SmallString512 Pathname(Filename);
+if (!MSStyle)
+  Lexer::Stringify(Pathname);
+
+SmallString256 Msg;
+if (MSStyle)
+  Msg += Note: including file:;
+
+if (ShowDepth) {
+  // The main source file is at depth 1, so skip one dot.
+  for (unsigned i = 1; i != CurrentIncludeDepth; ++i)
+Msg += MSStyle ? ' ' : '.';
+
+  if (!MSStyle)
+Msg += ' ';
+}
+Msg += Pathname;
+Msg += '\n';
+
+OutputFile-write(Msg.data(), Msg.size());
+OutputFile-flush();
+}
+
+void clang::AttachHeaderIncludeGen(Preprocessor PP,
+   const std::vectorstd::string ExtraHeaders,
+   bool ShowAllHeaders,
StringRef OutputPath, bool ShowDepth,
bool MSStyle) {
   raw_ostream *OutputFile = MSStyle ? llvm::outs() : llvm::errs();
@@ -69,6 +98,14 @@
 }
   }
 
+  // Print header info for extra headers, pretending they were discovered
+  // by the regular preprocessor. The primary use case is to support
+  // proper generation of Make / Ninja file dependencies for implicit includes,
+  // such as sanitizer blacklists. It's only important for cl.exe
+  // compatibility, the GNU way to generate rules is -M / -MM / -MD / -MMD.
+  for (auto Header : ExtraHeaders) {
+PrintHeaderInfo(OutputFile, Header.c_str(), ShowDepth, 2, MSStyle);
+  }
   PP.addPPCallbacks(llvm::make_uniqueHeaderIncludesCallback(PP,
   ShowAllHeaders,
   OutputFile,
@@ -112,27 +149,7 @@
   // Dump the header include information we are past the predefines buffer or
   // are showing all headers.
   if (ShowHeader  Reason == PPCallbacks::EnterFile) {
-// Write to a temporary string to avoid unnecessary flushing on errs().
-SmallString512 Filename(UserLoc.getFilename());
-if (!MSStyle)
-  Lexer::Stringify(Filename);
-
-SmallString256 Msg;
-if (MSStyle)
-  Msg += Note: including file:;
-
-if (ShowDepth) {
-  // The main source file is at depth 1, so skip one dot.
-  for (unsigned i = 1; i != CurrentIncludeDepth; ++i)
-Msg += MSStyle ? ' ' : '.';
-
-  if (!MSStyle)
-Msg += ' ';
-}
-Msg += Filename;
-Msg += '\n';
-
-OutputFile-write(Msg.data(), Msg.size());
-OutputFile-flush();
+PrintHeaderInfo(OutputFile, UserLoc.getFilename(),
+ShowDepth, CurrentIncludeDepth, MSStyle);
   }
 }
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -182,7 +182,11 @@
   AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
   SeenMissingHeader(false),
   IncludeModuleFiles(Opts.IncludeModuleFiles),
-  OutputFormat(Opts.OutputFormat) {}
+  OutputFormat(Opts.OutputFormat) {
+for (auto ExtraDep : Opts.ExtraDeps) {
+  

Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

krasin added a comment.

Thank you, Peter.

I will commit once I have restored my password (the email to Chris is sent)


http://reviews.llvm.org/D11968



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

r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

Author: krasin
Date: Wed Aug 12 23:04:37 2015
New Revision: 244867

URL: http://llvm.org/viewvc/llvm-project?rev=244867view=rev
Log:
Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

Summary:
Clang sanitizers, such as AddressSanitizer, ThreadSanitizer, MemorySanitizer,
Control Flow Integrity and others, use blacklists to specify which types / 
functions
should not be instrumented to avoid false positives or suppress known failures.

This change adds the blacklist filenames to the list of dependencies of the 
rules,
generated with -M/-MM/-MD/-MMD. This lets CMake/Ninja recognize that certain
C/C++/ObjC files need to be recompiled (if a blacklist is updated).

Reviewers: pcc

Subscribers: rsmith, honggyu.kim, pcc, cfe-commits

Differential Revision: http://reviews.llvm.org/D11968

Modified:
cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
cfe/trunk/include/clang/Frontend/Utils.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/DependencyFile.cpp
cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp
cfe/trunk/test/Frontend/dependency-gen.c
cfe/trunk/test/Frontend/print-header-includes.c

Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h?rev=244867r1=244866r2=244867view=diff
==
--- cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h Wed Aug 12 
23:04:37 2015
@@ -47,6 +47,9 @@ public:
   /// must contain at least one entry.
   std::vectorstd::string Targets;
 
+  /// A list of filenames to be used as extra dependencies for every target.
+  std::vectorstd::string ExtraDeps;
+
   /// \brief The file to write GraphViz-formatted header dependencies to.
   std::string DOTOutputFile;
 

Modified: cfe/trunk/include/clang/Frontend/Utils.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=244867r1=244866r2=244867view=diff
==
--- cfe/trunk/include/clang/Frontend/Utils.h (original)
+++ cfe/trunk/include/clang/Frontend/Utils.h Wed Aug 12 23:04:37 2015
@@ -148,6 +148,9 @@ public:
 /// AttachHeaderIncludeGen - Create a header include list generator, and attach
 /// it to the given preprocessor.
 ///
+/// \param ExtraHeaders - If not empty, will write the header filenames, just
+/// like they were included during a regular preprocessing. Useful for
+/// implicit include dependencies, like sanitizer blacklists.
 /// \param ShowAllHeaders - If true, show all header information instead of 
just
 /// headers following the predefines buffer. This is useful for making sure
 /// includes mentioned on the command line are also reported, but differs from
@@ -156,7 +159,9 @@ public:
 /// information to, instead of writing to stderr.
 /// \param ShowDepth - Whether to indent to show the nesting of the includes.
 /// \param MSStyle - Whether to print in cl.exe /showIncludes style.
-void AttachHeaderIncludeGen(Preprocessor PP, bool ShowAllHeaders = false,
+void AttachHeaderIncludeGen(Preprocessor PP,
+const std::vectorstd::string ExtraHeaders,
+bool ShowAllHeaders = false,
 StringRef OutputPath = ,
 bool ShowDepth = true, bool MSStyle = false);
 

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=244867r1=244866r2=244867view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Aug 12 23:04:37 2015
@@ -354,17 +354,19 @@ void CompilerInstance::createPreprocesso
 
   // Handle generating header include information, if requested.
   if (DepOpts.ShowHeaderIncludes)
-AttachHeaderIncludeGen(*PP);
+AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps);
   if (!DepOpts.HeaderIncludeOutputFile.empty()) {
 StringRef OutputPath = DepOpts.HeaderIncludeOutputFile;
 if (OutputPath == -)
   OutputPath = ;
-AttachHeaderIncludeGen(*PP, /*ShowAllHeaders=*/true, OutputPath,
+AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps,
+   /*ShowAllHeaders=*/true, OutputPath,
/*ShowDepth=*/false);
   }
 
   if (DepOpts.PrintShowIncludes) {
-AttachHeaderIncludeGen(*PP, /*ShowAllHeaders=*/false, /*OutputPath=*/,
+AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps,
+   /*ShowAllHeaders=*/false, /*OutputPath=*/,
/*ShowDepth=*/true, /*MSStyle=*/true);
   }
 }

Modified: 

Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 31895.
krasin marked an inline comment as done.
krasin added a comment.

Add more test cases.


http://reviews.llvm.org/D11968

Files:
  include/clang/Frontend/DependencyOutputOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/DependencyFile.cpp
  test/Frontend/dependency-gen.c

Index: test/Frontend/dependency-gen.c
===
--- test/Frontend/dependency-gen.c
+++ test/Frontend/dependency-gen.c
@@ -20,7 +20,16 @@
 // RUN: cd a/b
 // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck 
-check-prefix=CHECK-SIX %s
 // CHECK-SIX: {{ }}x.h
-
+// RUN: echo fun:foo  %t.blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto 
-fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s
+// CHECK-SEVEN: {{ }}x.h
+// CHECK-SEVEN: .blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | 
FileCheck -check-prefix=CHECK-EIGHT %s
+// CHECK-EIGHT: {{ }}x.h
+// CHECK-EIGHT: asan_blacklist.txt
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . 
-fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s
+// CHECK-NINE: {{ }}x.h
+// CHECK-NINE-NOT: asan_blacklist.txt
 #ifndef INCLUDE_FLAG_TEST
 #include x.h
 #endif
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -162,6 +162,7 @@
   const Preprocessor *PP;
   std::string OutputFile;
   std::vectorstd::string Targets;
+  std::vectorstd::string ExtraDeps;
   bool IncludeSystemHeaders;
   bool PhonyTarget;
   bool AddMissingHeaderDeps;
@@ -177,6 +178,7 @@
 public:
   DFGImpl(const Preprocessor *_PP, const DependencyOutputOptions Opts)
 : PP(_PP), OutputFile(Opts.OutputFile), Targets(Opts.Targets),
+  ExtraDeps(Opts.ExtraDeps),
   IncludeSystemHeaders(Opts.IncludeSystemHeaders),
   PhonyTarget(Opts.UsePhonyTargets),
   AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
@@ -411,6 +413,11 @@
 return;
   }
 
+  // Add extra dependencies to the end of the list.
+  for (auto ExtraDep : ExtraDeps) {
+AddFilename(ExtraDep);
+  }
+
   // Write out the dependency targets, trying to avoid overly long
   // lines when possible. We try our best to emit exactly the same
   // dependency file as GCC (4.2), assuming the included files are the
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -705,6 +705,10 @@
   Args.getLastArgValue(OPT_module_dependency_dir);
   if (Args.hasArg(OPT_MV))
 Opts.OutputFormat = DependencyOutputFormat::NMake;
+  // Add sanitizer blacklists as extra dependencies.
+  // They won't be discovered by the regular preprocessor, so
+  // we let make / ninja to know about this implicit dependency.
+  Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist);
 }
 
 bool clang::ParseDiagnosticArgs(DiagnosticOptions Opts, ArgList Args,
Index: include/clang/Frontend/DependencyOutputOptions.h
===
--- include/clang/Frontend/DependencyOutputOptions.h
+++ include/clang/Frontend/DependencyOutputOptions.h
@@ -47,6 +47,9 @@
   /// must contain at least one entry.
   std::vectorstd::string Targets;
 
+  /// A list of filenames to be used as extra dependencies for every target.
+  std::vectorstd::string ExtraDeps;
+
   /// \brief The file to write GraphViz-formatted header dependencies to.
   std::string DOTOutputFile;
 


Index: test/Frontend/dependency-gen.c
===
--- test/Frontend/dependency-gen.c
+++ test/Frontend/dependency-gen.c
@@ -20,7 +20,16 @@
 // RUN: cd a/b
 // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s
 // CHECK-SIX: {{ }}x.h
-
+// RUN: echo fun:foo  %t.blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s
+// CHECK-SEVEN: {{ }}x.h
+// CHECK-SEVEN: .blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s
+// CHECK-EIGHT: {{ }}x.h
+// CHECK-EIGHT: asan_blacklist.txt
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s
+// CHECK-NINE: {{ }}x.h
+// CHECK-NINE-NOT: asan_blacklist.txt
 #ifndef INCLUDE_FLAG_TEST
 #include x.h
 #endif
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -162,6 +162,7 @@
   const Preprocessor *PP;
   std::string OutputFile;
   std::vectorstd::string Targets;
+  std::vectorstd::string 

Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[Ivan Krasin via cfe-commits]

krasin added a comment.

In http://reviews.llvm.org/D11968#222338, @pcc wrote:

 We should also make blacklists appear in the `--show-includes` output.


Is it about Windows compatibility? Do you mean that the output of

  bin/clang-cl /Zs /showIncludes ~/lala.cc -fsanitize=address

should include the default blacklist (and any explicit blacklists, if they were 
specified)?



Comment at: lib/Frontend/DependencyFile.cpp:416-420
@@ -413,2 +415,7 @@
 
+  // Add extra dependencies to the end of the list.
+  for (auto ExtraDep : ExtraDeps) {
+AddFilename(ExtraDep);
+  }
+
   // Write out the dependency targets, trying to avoid overly long

pcc wrote:
  If you move this code to the constructor you won't need to add an extra data 
 member to this class.
I considered that. In this case, extra deps will appear even before the source 
file itself.

Right now, for my local test code, it generates:

```
lala.o: /usr/local/google/home/krasin/lala.cc \
  /usr/local/google/home/krasin/foo.h \
  /usr/local/google/home/krasin/bar.h \
  /usr/local/google/home/krasin/blacklist.txt
```

If moved to the constructor, it will become:

```
lala.o: /usr/local/google/home/krasin/blacklist.txt \
  /usr/local/google/home/krasin/lala.cc \
  /usr/local/google/home/krasin/foo.h \
  /usr/local/google/home/krasin/bar.h
```

This decreases the readability of the generated rules, as the main source 
becomes harder to discover.

Please, let me know, if you think it's not a problem; I will move the code to 
the constructor and eliminate the extra member.



http://reviews.llvm.org/D11968



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