Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-03 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 33959.
nmusgrave added a comment.

- Refined testing for bit fields.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-bit-field.cpp
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vector v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vector v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: test/CodeGenCXX/sanitize-dtor-bit-field.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-bit-field.cpp
@@ -0,0 +1,65 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -O0 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+// 24 bytes total
+struct Packed {
+  // Packed into 4 bytes
+  unsigned int a : 1;
+  unsigned int b : 1;
+  //unsigned int c : 1;
+  // Force alignment to next 4 bytes
+  unsigned int   : 0;
+  unsigned int d : 1;
+  // Force alignment, 8 more bytes
+  double e = 5.0;
+  // 4 bytes
+  unsigned int f : 1;
+  ~Packed() {}
+};
+Packed p;
+
+
+// 1 byte total
+struct Empty {
+  unsigned int : 0;
+  ~Empty() {}
+};
+Empty e;
+
+
+// 4 byte total
+struct Simple {
+  unsigned int a : 1;
+  ~Simple() {}
+};
+Simple s;
+
+
+// 8 bytes total
+struct Anon {
+  // 1 byte
+  unsigned int a : 1;
+  unsigned int b : 2;
+  // Force alignment to next byte
+  unsigned int   : 0;
+  unsigned int c : 1;
+  ~Anon() {}
+};
+Anon a;
+
+// CHECK-LABEL: define {{.*}}PackedD2Ev
+// CHECK: call void 

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-03 Thread Richard Smith via cfe-commits
rsmith added inline comments.


Comment at: lib/CodeGen/CGCXX.cpp:140-147
@@ -131,1 +139,10 @@
 
+  // If sanitizing memory to check for use-after-dtor, do not emit as
+  //  an alias, unless this class owns no members with trivial destructors.
+  const CXXMethodDecl *MD =
+  cast(AliasDecl.getDecl())->getCanonicalDecl();
+  if (isa(MD) &&
+  getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+  HasFieldWithTrivialDestructor(*this, MD->getParent()))
+return true;
+

Having checked a bit more, I think this test should go in 
`TryEmitBaseDestructorAsAlias`: it's OK to emit the complete object destructor 
for a class as an alias for the base subobject constructor for the same class 
even if this sanitizer is enabled, but it's not OK to emit the destructor for 
the class as an alias for a base class's destructor (which is what 
`TryEmitBaseDestructorAsAlias` is checking).

The check then becomes a lot simpler: with the sanitizer enabled, for each 
field we will do exactly one of (a) invoke a destructor (which will poison the 
memory) or (b) poison the memory ourselves. So if the sanitizer is enabled, we 
can emit an alias only if the derived class has no fields.


Comment at: lib/CodeGen/CGClass.cpp:1559-1560
@@ +1558,4 @@
+  // RecordDecl::field_iterator Field;
+  for (const auto F : Dtor->getParent()->fields()) {
+FieldDecl const *Field = F;
+// Poison field if it is trivial

Separately declaring `F` and `Field` seems unnecessary; how about:

  for (const FieldDecl *Field : Dtor->getParent()->fields()) {


Comment at: lib/CodeGen/CGClass.cpp:1592-1593
@@ +1591,4 @@
+  Context.getASTRecordLayout(Dtor->getParent());
+  llvm::ConstantInt *OffsetSizePtr;
+  llvm::Value *OffsetPtr;
+  CharUnits::QuantityType PoisonSize;

Initialize these as you declare them, rather than separately:

  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
// ...
  llvm::Value *OffsetPtr = // ...


Comment at: test/CodeGenCXX/sanitize-dtor-bit-field.cpp:1
@@ +1,2 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -O0 -fsanitize=memory -fsanitize-memory-use-after-dtor 
-disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | 
FileCheck %s

It'd also be good to test for the case where a bit-field is adjacent to 
non-sanitized fields:

  struct A { char c; ~A(); };
  struct B {
A x;
int n : 1;
A y;
  };


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-03 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 33978.
nmusgrave marked 4 inline comments as done.
nmusgrave added a comment.

- Simplified fields and checks for aliasing.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-bit-field.cpp
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vector v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vector v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: test/CodeGenCXX/sanitize-dtor-bit-field.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-bit-field.cpp
@@ -0,0 +1,84 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -O0 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+// 24 bytes total
+struct Packed {
+  // Packed into 4 bytes
+  unsigned int a : 1;
+  unsigned int b : 1;
+  //unsigned int c : 1;
+  // Force alignment to next 4 bytes
+  unsigned int   : 0;
+  unsigned int d : 1;
+  // Force alignment, 8 more bytes
+  double e = 5.0;
+  // 4 bytes
+  unsigned int f : 1;
+  ~Packed() {}
+};
+Packed p;
+
+
+// 1 byte total
+struct Empty {
+  unsigned int : 0;
+  ~Empty() {}
+};
+Empty e;
+
+
+// 4 byte total
+struct Simple {
+  unsigned int a : 1;
+  ~Simple() {}
+};
+Simple s;
+
+
+// 8 bytes total
+struct Anon {
+  // 1 byte
+  unsigned int a : 1;
+  unsigned int b : 2;
+  // Force alignment to next byte
+  unsigned int   : 0;
+  unsigned int c : 1;
+  ~Anon() {}
+};
+Anon an;
+
+

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-03 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 33982.
nmusgrave marked 2 inline comments as done.
nmusgrave added a comment.

- Clean method headers, style.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-bit-field.cpp
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vector v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vector v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: test/CodeGenCXX/sanitize-dtor-bit-field.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-bit-field.cpp
@@ -0,0 +1,84 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -O0 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-optzns -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+// 24 bytes total
+struct Packed {
+  // Packed into 4 bytes
+  unsigned int a : 1;
+  unsigned int b : 1;
+  //unsigned int c : 1;
+  // Force alignment to next 4 bytes
+  unsigned int   : 0;
+  unsigned int d : 1;
+  // Force alignment, 8 more bytes
+  double e = 5.0;
+  // 4 bytes
+  unsigned int f : 1;
+  ~Packed() {}
+};
+Packed p;
+
+
+// 1 byte total
+struct Empty {
+  unsigned int : 0;
+  ~Empty() {}
+};
+Empty e;
+
+
+// 4 byte total
+struct Simple {
+  unsigned int a : 1;
+  ~Simple() {}
+};
+Simple s;
+
+
+// 8 bytes total
+struct Anon {
+  // 1 byte
+  unsigned int a : 1;
+  unsigned int b : 2;
+  // Force alignment to next byte
+  unsigned int   : 0;
+  unsigned int c : 1;
+  ~Anon() {}
+};
+Anon an;
+
+
+struct CharStruct 

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-03 Thread Richard Smith via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM with a couple of tweaks.



Comment at: lib/CodeGen/CGCXX.cpp:45-46
@@ +44,4 @@
+  //  an alias, unless this class owns no members.
+  unsigned totalFields =
+  std::distance(D->getParent()->field_begin(), 
D->getParent()->field_end());
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor && totalFields > 0)

You can just use `!D->getParent()->field_empty()`


Comment at: lib/CodeGen/CGClass.cpp:1289
@@ -1288,7 +1288,3 @@
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext , const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext ,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext ,
  const CXXRecordDecl *BaseClassDecl,

I think you no longer need to make these functions members.


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-02 Thread Richard Smith via cfe-commits
rsmith added a comment.

I'd like to see some tests for poisoning bitfields, and particularly anonymous 
bitfields (I think the code works fine in those cases, but they're a bit 
special so explicitly testing them is useful).



Comment at: lib/CodeGen/CGCXX.cpp:151
@@ +150,3 @@
+  if (isa(MD) && getCodeGenOpts().SanitizeMemoryUseAfterDtor
+  && HasFieldWithTrivialDestructor(*this, MD->getParent()))
+return true;

&& goes on previous line.


Comment at: lib/CodeGen/CGClass.cpp:1547
@@ +1546,3 @@
+  // Nothing to poison.
+  if (Layout.getFieldCount() == 0)
+return;

eugenis wrote:
> Probably better check Dtor->getParent()->field_empty() for consistency.
This check is equivalent and more efficient.


Comment at: lib/CodeGen/CGClass.cpp:1559
@@ +1558,3 @@
+  RecordDecl::field_iterator Field;
+  for (Field = Dtor->getParent()->field_begin();
+   Field != Dtor->getParent()->field_end(); Field++) {

Can you use a range for?


Comment at: lib/CodeGen/CGClass.cpp:1587
@@ +1586,3 @@
+// layoutEndOffset: index of the ASTRecordLayout field to end poisoning
+//   (exclusive)
+void PoisonBlock(CodeGenFunction , unsigned layoutStartOffset,

Doxygenize this comment  (use ///, \param).


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-02 Thread Evgeniy Stepanov via cfe-commits
eugenis added inline comments.


Comment at: lib/CodeGen/CGCXX.cpp:45
@@ +44,3 @@
+  // destructors.
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor)
+return true;

This simply suppresses all dtor alias under UseAfterDtor, effectively disabling 
the second check below.
Remove it.


Comment at: lib/CodeGen/CGCXX.cpp:130
@@ -115,3 +129,3 @@
  bool InEveryTU) {
-  if (!getCodeGenOpts().CXXCtorDtorAliases)
+  if(!getCodeGenOpts().CXXCtorDtorAliases)
 return true;

formatting


Comment at: lib/CodeGen/CGCXX.cpp:147
@@ +146,3 @@
+  // If sanitizing memory to check for use-after-dtor, do not emit as
+  //  an alias, unless this class owns no members.
+  const CXXMethodDecl *MD =

... with trivial destructors


Comment at: lib/CodeGen/CGClass.cpp:1547
@@ +1546,3 @@
+  // Nothing to poison.
+  if (Layout.getFieldCount() == 0)
+return;

Probably better check Dtor->getParent()->field_empty() for consistency.


Comment at: lib/CodeGen/CGClass.cpp:1551
@@ +1550,3 @@
+  // Prevent the current stack frame from disappearing from the stack 
trace.
+  CGF.CurFn->addFnAttr("disable-tail-calls", "true");
+

disable tail calls only when !field_empty()


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-02 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 33887.
nmusgrave marked 4 inline comments as done.
nmusgrave added a comment.

- Update comments, consistent style for attribute checking.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vector v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vector v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1099,6 +1099,13 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  HasTrivialDestructorBody(ASTContext ,
+   const CXXRecordDecl *BaseClassDecl,
+   const CXXRecordDecl *MostDerivedClassDecl);
+  bool
+  FieldHasTrivialDestructorBody(ASTContext , const FieldDecl *Field);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1286,11 +1286,7 @@
   CM.finish();
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext , const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext ,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext ,
  const CXXRecordDecl *BaseClassDecl,
  const CXXRecordDecl *MostDerivedClassDecl)
 {
@@ -1332,9 +1328,8 @@
   return true;
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext ,
-   

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-02 Thread Evgeniy Stepanov via cfe-commits
eugenis added a comment.

LGTM but please wait for Richard's answer



Comment at: lib/CodeGen/CGClass.cpp:1551
@@ +1550,3 @@
+  // Prevent the current stack frame from disappearing from the stack 
trace.
+  CGF.CurFn->addFnAttr("disable-tail-calls", "true");
+

oh, right, that's already the case.


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-01 Thread Naomi Musgrave via cfe-commits
nmusgrave added inline comments.


Comment at: lib/CodeGen/CGCXX.cpp:42-44
@@ -33,1 +41,5 @@
 bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
+  // If sanitizing memory to check for use-after-dtor, do not emit as
+  // an alias, unless it has no fields or has only fields with non-trivial
+  // destructors.
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor)

vptr poisoning will be implemented in another CL after this is approved


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-09-01 Thread Richard Smith via cfe-commits
rsmith added inline comments.


Comment at: lib/CodeGen/CGCXX.cpp:42-44
@@ -33,1 +41,5 @@
 bool CodeGenModule::TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D) {
+  // If sanitizing memory to check for use-after-dtor, do not emit as
+  // an alias, unless it has no fields or has only fields with non-trivial
+  // destructors.
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor &&

I assume the rationale here is that the field and base class destructors will 
mark their storage as destroyed, so there's nothing else to mark? This may mean 
that vptrs and padding bytes don't get marked as destroyed, is that OK?


Comment at: lib/CodeGen/CGCXX.cpp:45-47
@@ +44,5 @@
+  // destructors.
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+  HasFieldWithTrivialDestructor(*this, D->getParent()))
+return true;
+

This check seems to be redundant, since `TryEmitDefinitionAsAlias` checks the 
same thing. Remove this copy?


Comment at: lib/CodeGen/CGCXX.cpp:135
@@ +134,3 @@
+  cast(AliasDecl.getDecl())->getCanonicalDecl();
+  assert(isa(MD));
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor &&

This function isn't (or wasn't) specific to destructors; I think you should be 
checking for this as part of your `if` below rather than asserting it.


Comment at: lib/CodeGen/CGCXX.cpp:136-138
@@ +135,5 @@
+  assert(isa(MD));
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+  HasFieldWithTrivialDestructor(*this, MD->getParent()))
+return true;
+

Please reorder this after the quicker, higher-level checks below. (Maybe move 
it to just before we create the mangled name?)


Comment at: lib/CodeGen/CGClass.cpp:1334-1335
@@ -1338,1 +1333,4 @@
 {
+  if (Field->getType()->isPointerType())
+return true;
+

This appears redundant; we'd return `true` for pointers on line 1341 anyway. 
(`getBaseElementType` only strips off array types, not pointer types).


Comment at: lib/CodeGen/CGClass.cpp:1549
@@ +1548,3 @@
+
+  // Nothing to poison
+  if (Layout.getFieldCount() == 0)

Add full stop.


Comment at: lib/CodeGen/CGClass.cpp:1554
@@ +1553,3 @@
+  // Iterate over fields declared in this class, and count all fields,
+  // including inherited from base classes.
+  unsigned totalFields = 0;

You're only counting the direct fields here, not the inherited ones.


Comment at: lib/CodeGen/CGClass.cpp:1555-1558
@@ +1554,6 @@
+  // including inherited from base classes.
+  unsigned totalFields = 0;
+  for (auto *Field : Dtor->getParent()->fields()) {
+(void)Field;
+totalFields += 1;
+  }

unsigned totalFields = std::distance(Dtor->getParent()->field_begin(),
 Dtor->getParent()->field_end());


Comment at: lib/CodeGen/CGClass.cpp:1567
@@ +1566,3 @@
+  unsigned inheritedFields = totalFields - Layout.getFieldCount();
+  // todo: don't use iterator for accessing fields
+  RecordDecl::field_iterator Field;

todo -> TODO (or FIXME)


Comment at: lib/CodeGen/CGClass.cpp:1621
@@ +1620,3 @@
+
+  if (layoutEndOffset >= Layout.getFieldCount() - 1) {
+PoisonSize = Layout.getNonVirtualSize().getQuantity() -

I think this is off by one. `layoutEndOffset` is the index of the first field 
that is not poisoned, so this will trigger if we're poisoning up to, but not 
including, the last field, as well as triggering if we're poisoning the last 
field.


Comment at: lib/CodeGen/CGClass.cpp:1644-1645
@@ +1643,4 @@
+  CGF.CGM.CreateRuntimeFunction(FnType, "__sanitizer_dtor_callback");
+  // Prevent the current stack frame from disappearing from the stack 
trace.
+  CGF.CurFn->addFnAttr("disable-tail-calls", "true");
+

Maybe do this once in `Emit` (if you poison anything) rather than doing it 
every time you emit a poison call?


Comment at: lib/CodeGen/CGClass.cpp:1722
@@ -1660,1 +1721,3 @@
 
+  if (CGM.getCodeGenOpts().SanitizeMemoryUseAfterDtor &&
+  SanOpts.has(SanitizerKind::Memory))

Please add a comment here. "Mark the lifetime of fields as ending after field 
destructors run and before we destroy base classes." or similar.


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-28 Thread Naomi Musgrave via cfe-commits
nmusgrave marked 3 inline comments as done.
nmusgrave added a comment.

http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-28 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 33436.
nmusgrave added a comment.

- Checking for existence of fields to poison in alias emission.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vectorint v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}(void (%struct.Derived*)* @_ZN7DerivedD1Ev to i8*){{.*}}(void (%struct.Derived*)* @_ZN7DerivedD0Ev to i8*)
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vectorint v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1098,6 +1098,13 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  HasTrivialDestructorBody(ASTContext Context,
+   const CXXRecordDecl *BaseClassDecl,
+   const CXXRecordDecl *MostDerivedClassDecl);
+  bool
+  FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1286,11 +1286,7 @@
   CM.finish();
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext Context,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext Context,
  const CXXRecordDecl *BaseClassDecl,
  const CXXRecordDecl *MostDerivedClassDecl)
 {
@@ -1332,10 +1328,12 

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-28 Thread Naomi Musgrave via cfe-commits
nmusgrave added inline comments.


Comment at: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp:30
@@ +29,2 @@
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}(void (%struct.Derived*)* @_ZN7DerivedD1Ev to 
i8*){{.*}}(void (%struct.Derived*)* @_ZN7DerivedD0Ev to i8*)

Its checking that my aliasing is repressed when appropriate.Should I keep or 
get rid of this test case?


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-28 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 33463.
nmusgrave marked an inline comment as done.
nmusgrave added a comment.

- Alias-repressing test case ignores casting of pointers.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vectorint v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vectorint v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1098,6 +1098,13 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  HasTrivialDestructorBody(ASTContext Context,
+   const CXXRecordDecl *BaseClassDecl,
+   const CXXRecordDecl *MostDerivedClassDecl);
+  bool
+  FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1286,11 +1286,7 @@
   CM.finish();
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext Context,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext Context,
  const CXXRecordDecl *BaseClassDecl,
  const CXXRecordDecl *MostDerivedClassDecl)
 {
@@ -1332,10 +1328,12 @@
   return true;
 }
 

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-26 Thread Evgeniy Stepanov via cfe-commits
eugenis added a comment.

Richard, would you mind taking a look a this change?


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-25 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 33100.
nmusgrave marked an inline comment as done.
nmusgrave added a comment.

- Check flags before dtor sanitizing


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vectorint v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}(void (%struct.Derived*)* @_ZN7DerivedD1Ev to i8*){{.*}}(void (%struct.Derived*)* @_ZN7DerivedD0Ev to i8*)
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vectorint v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1098,6 +1098,13 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  HasTrivialDestructorBody(ASTContext Context,
+   const CXXRecordDecl *BaseClassDecl,
+   const CXXRecordDecl *MostDerivedClassDecl);
+  bool
+  FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1286,11 +1286,7 @@
   CM.finish();
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext Context,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext Context,
  const CXXRecordDecl *BaseClassDecl,
  const CXXRecordDecl *MostDerivedClassDecl)
 {
@@ 

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-25 Thread Evgeniy Stepanov via cfe-commits
eugenis added inline comments.


Comment at: lib/CodeGen/CGCXX.cpp:31
@@ -30,1 +30,3 @@
 
+static bool HasTrivialField(CodeGenModule CGM, const CXXDestructorDecl *D) {
+  for (const auto *Field : D-getParent()-fields())

I think this should be called HasFieldWithTrivialDestructor.
Also, pass D-getParent() to this function instead of D.


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-24 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 33037.
nmusgrave marked 4 inline comments as done.
nmusgrave added a comment.

- Simplify function invocations


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vectorint v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}(void (%struct.Derived*)* @_ZN7DerivedD1Ev to i8*){{.*}}(void (%struct.Derived*)* @_ZN7DerivedD0Ev to i8*)
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vectorint v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1098,6 +1098,13 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  HasTrivialDestructorBody(ASTContext Context,
+   const CXXRecordDecl *BaseClassDecl,
+   const CXXRecordDecl *MostDerivedClassDecl);
+  bool
+  FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1286,11 +1286,7 @@
   CM.finish();
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext Context,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext Context,
  const CXXRecordDecl *BaseClassDecl,
  const CXXRecordDecl *MostDerivedClassDecl)
 {
@@ 

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-23 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 32930.
nmusgrave added a comment.

- Refactoring dtor sanitizing emission order
- support for virtual functions  virtual bases WIP
- Repress dtor aliasing when sanitizing in dtor
- CFE test for dtor aliasing, and repression of aliasing in dtor code 
generation.
- More complex testing for destruction order.
- Poison trivial members one-by-one.
- Poisoning on field-by-field basis, with collective poisoning of trivial 
members when possible.
- Cleaned up implementation of calculating region to poison in dtor.
- Checking for existence of a single trivial field.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vectorint v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}(void (%struct.Derived*)* @_ZN7DerivedD1Ev to i8*){{.*}}(void (%struct.Derived*)* @_ZN7DerivedD0Ev to i8*)
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vectorint v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1098,6 +1098,13 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  HasTrivialDestructorBody(ASTContext Context,
+   const CXXRecordDecl *BaseClassDecl,
+   const CXXRecordDecl *MostDerivedClassDecl);
+  bool
+  FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===
--- 

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-21 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 32873.
nmusgrave marked 2 inline comments as done.
nmusgrave added a comment.

- Poisoning on field-by-field basis, with collective poisoning of trivial 
members when possible.
- Cleaned up implementation of calculating region to poison in dtor.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vectorint v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}(void (%struct.Derived*)* @_ZN7DerivedD1Ev to i8*){{.*}}(void (%struct.Derived*)* @_ZN7DerivedD0Ev to i8*)
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vectorint v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1098,6 +1098,14 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
+
+  bool
+  HasTrivialDestructorBody(ASTContext Context,
+   const CXXRecordDecl *BaseClassDecl,
+   const CXXRecordDecl *MostDerivedClassDecl);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1286,11 +1286,7 @@
   CM.finish();
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext Context, const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext Context,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext Context,
  

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-21 Thread Evgeniy Stepanov via cfe-commits
eugenis added inline comments.


Comment at: lib/CodeGen/CGCXX.cpp:41
@@ +40,3 @@
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor  Layout.getFieldCount()  0
+   HasTrivialDestructorBody(Context, D-getParent(), D-getParent())) {
+return true;

I'm not sure this is correct. It says not to use an alias if D has trivial 
body. Should not it be the other way around?


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-21 Thread Evgeniy Stepanov via cfe-commits
eugenis added inline comments.


Comment at: lib/CodeGen/CGClass.cpp:1578
@@ +1577,3 @@
+if (CGF.CGM.FieldHasTrivialDestructorBody(Context, Field) ||
+Field-getType()-isPointerType()) {
+  // Start sanitizing at this field

Why do you need to special-case pointers?


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-21 Thread Naomi Musgrave via cfe-commits
nmusgrave added inline comments.


Comment at: lib/CodeGen/CGCXX.cpp:41
@@ +40,3 @@
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor  Layout.getFieldCount()  0
+   HasTrivialDestructorBody(Context, D-getParent(), D-getParent())) {
+return true;

nmusgrave wrote:
 eugenis wrote:
  I'm not sure this is correct. It says not to use an alias if D has trivial 
  body. Should not it be the other way around?
 It's counter-intuitive:
 TryEmitBaseDestructorAsAlias returns false when TryEmitDefinitionAsAlias 
 returns false.
 TryEmitDefinitionAsAlias returns false when an alias is successfully created 
 (line 216 in this CL)
For example: line 3711 of clang/lib/CodeGen/MicrosoftCXXABI.cpp
 bool ProducedAlias = !CGM.TryEmitDefinitionAsAlias(
GlobalDecl(dtor, Dtor_Complete), GlobalDecl(dtor, Dtor_Base), true); 

^uses the negation of the function's return value


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-21 Thread Naomi Musgrave via cfe-commits
nmusgrave added inline comments.


Comment at: lib/CodeGen/CGClass.cpp:1578
@@ +1577,3 @@
+if (CGF.CGM.FieldHasTrivialDestructorBody(Context, Field) ||
+Field-getType()-isPointerType()) {
+  // Start sanitizing at this field

eugenis wrote:
 Why do you need to special-case pointers?
FieldHasTrivialDestructorBody doesn't catch pointers- it identifies their base 
type as some class, and returns false


http://reviews.llvm.org/D12022



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


Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

2015-08-19 Thread Naomi Musgrave via cfe-commits
nmusgrave updated this revision to Diff 32608.
nmusgrave added a comment.

- More complex testing for destruction order. Tests class with base, virtual 
base, trivial, and nontrivial member to ensure destruction order is correct.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,84 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template class T
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int y;
+  VirtualBase() { y = 10; }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int z;
+  Vectorint v;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// Declaration of virtual function table
+// CHECK: $_ZTV7Derived = comdat any
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}(void (%struct.Derived*)* @_ZN7DerivedD1Ev to i8*){{.*}}(void (%struct.Derived*)* @_ZN7DerivedD0Ev to i8*)
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: call void {{.*}}ZN7DerivedD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback(i8* %{{[0-9]*}}, i64 4)
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN4BaseD1Ev
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN4BaseD0Ev
+// CHECK: call void {{.*}}ZN4BaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback(i8* %{{[0-9]*}}, i64 4)
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback(i8* %{{[0-9]*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1367,52 +1367,6 @@
   return true;
 }
 
-// Generates function call for handling object poisoning, passing in
-// references to 'this' and its size as arguments.
-// Disables tail call elimination, to prevent the current stack frame from
-// disappearing from the stack trace.
-static void EmitDtorSanitizerCallback(CodeGenFunction CGF,
-  const CXXDestructorDecl *Dtor) {
-  const ASTRecordLayout Layout =
-  CGF.getContext().getASTRecordLayout(Dtor-getParent());
-
-  // Nothing to poison
-  if(Layout.getFieldCount() == 0)
-return;
-
-  // Construct pointer to region to begin poisoning, and calculate poison
-  // size, so that only members declared in this class are poisoned.
-  llvm::Value *OffsetPtr;
-  CharUnits::QuantityType PoisonSize;
-  ASTContext Context = CGF.getContext();
-
-  llvm::ConstantInt *OffsetSizePtr = llvm::ConstantInt::get(
-  CGF.SizeTy, Context.toCharUnitsFromBits(Layout.getFieldOffset(0)).
-  getQuantity());
-
-  OffsetPtr = CGF.Builder.CreateGEP(CGF.Builder.CreateBitCast(
-  CGF.LoadCXXThis(), CGF.Int8PtrTy), OffsetSizePtr);
-
-  PoisonSize = Layout.getSize().getQuantity() -
-  Context.toCharUnitsFromBits(Layout.getFieldOffset(0)).getQuantity();
-
-  llvm::Value *Args[] = {
-CGF.Builder.CreateBitCast(OffsetPtr, CGF.VoidPtrTy),
-llvm::ConstantInt::get(CGF.SizeTy, PoisonSize)};
-
-  llvm::Type *ArgTypes[] = {CGF.VoidPtrTy, CGF.SizeTy};
-
-  llvm::FunctionType *FnType =
-  llvm::FunctionType::get(CGF.VoidTy, ArgTypes, false);
-  llvm::Value *Fn =
-  CGF.CGM.CreateRuntimeFunction(FnType, __sanitizer_dtor_callback);
-
-  // Disables tail call elimination, to prevent the current stack frame from
-  //