Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 - //