[llvm-branch-commits] [clang] 3e1cb0d - [CodeGen][MSan] Don't use offsets of zero-sized fields

[Vitaly Buka via llvm-branch-commits]

Author: Vitaly Buka
Date: 2020-12-07T13:37:40-08:00
New Revision: 3e1cb0db8a79b19996fcea210b639fe513a5eaf3

URL: https://github.com/llvm/llvm-project/commit/3e1cb0db8a79b19996fcea210b639fe513a5eaf3
DIFF: https://github.com/llvm/llvm-project/commit/3e1cb0db8a79b19996fcea210b639fe513a5eaf3.diff

LOG: [CodeGen][MSan] Don't use offsets of zero-sized fields

Such fields will likely have offset zero making
__sanitizer_dtor_callback poisoning wrong regions.
E.g. it can poison base class member from derived class constructor.

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

Added: 
    

Modified: 
    clang/lib/CodeGen/CGClass.cpp
    clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 0d17037db864..b900ed70152e 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -1700,12 +1700,23 @@ namespace {
         return FieldHasTrivialDestructorBody(Context, F);
       };
 
+      auto IsZeroSize = [&](const FieldDecl *F) {
+        return F->isZeroSize(Context);
+      };
+
+      // Poison blocks of fields with trivial destructors making sure that block
+      // begin and end do not point to zero-sized fields. They don't have
+      // correct offsets so can't be used to calculate poisoning range.
       for (auto It = Fields.begin(); It != Fields.end();) {
-        It = std::find_if(It, Fields.end(), IsTrivial);
+        It = std::find_if(It, Fields.end(), [&](const FieldDecl *F) {
+          return IsTrivial(F) && !IsZeroSize(F);
+        });
         if (It == Fields.end())
           break;
         auto Start = It++;
-        It = std::find_if_not(It, Fields.end(), IsTrivial);
+        It = std::find_if(It, Fields.end(), [&](const FieldDecl *F) {
+          return !IsTrivial(F) && !IsZeroSize(F);
+        });
 
         PoisonMembers(CGF, (*Start)->getFieldIndex(),
                       It == Fields.end() ? -1 : (*It)->getFieldIndex());

diff  --git a/clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp b/clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
index e876f93e49eb..74ba612f9922 100644
--- a/clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
+++ b/clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -O0 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-passes -std=c++20 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s --implicit-check-not "call void @__sanitizer_dtor_callback"
 // RUN: %clang_cc1 -O1 -fsanitize=memory -fsanitize-memory-use-after-dtor -disable-llvm-passes -std=c++20 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s --implicit-check-not "call void @__sanitizer_dtor_callback"
 
-// This test convers invalid behaviour which will be fixed in followup patches.
-
 struct Empty {};
 
 struct EmptyNonTrivial {
@@ -115,7 +113,7 @@ struct Struct {
 static_assert(sizeof(Struct) == 24);
 } // namespace T6
 // CHECK-LABEL: define {{.*}} @_ZN5empty2T66StructD2Ev(
-// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 21)
+// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 13)
 // CHECK-NEXT:    ret void
 
 namespace T7 {
@@ -130,7 +128,7 @@ static_assert(sizeof(Struct) == 24);
 } // namespace T7
 // CHECK-LABEL: define {{.*}} @_ZN5empty2T76StructD2Ev(
 // CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
-// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 21)
+// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 5)
 // CHECK-NEXT:    ret void
 
 namespace T8 {
@@ -160,7 +158,7 @@ static_assert(sizeof(Struct) == 24);
 } // namespace T9
 // CHECK-LABEL: define {{.*}} @_ZN5empty2T96StructD2Ev(
 // CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
-// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 21)
+// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 1)
 // CHECK-NEXT:    ret void
 
 namespace T10 {
@@ -190,7 +188,6 @@ static_assert(sizeof(Struct) == 24);
 } // namespace T11
 // CHECK-LABEL: define {{.*}} @_ZN5empty3T116StructD2Ev(
 // CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 16)
-// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 24)
 // CHECK-NEXT:    ret void
 
 namespace T12 {
@@ -317,6 +314,7 @@ struct Struct {
 static_assert(sizeof(Struct) == 24);
 } // namespace T8
 // CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial2T86StructD2Ev(
+// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 8)
 // CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 5)
 // CHECK-NEXT:    ret void
 
@@ -346,6 +344,7 @@ struct Struct {
 static_assert(sizeof(Struct) == 24);
 } // namespace T10
 // CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial3T106StructD2Ev(
+// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 12)
 // CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 1)
 // CHECK-NEXT:    ret void
 
@@ -375,4 +374,5 @@ static_assert(sizeof(Struct) == 24);
 } // namespace T12
 } // namespace empty_non_trivial
 // CHECK-LABEL: define {{.*}} @_ZN17empty_non_trivial3T126StructD2Ev(
+// CHECK:         call void @__sanitizer_dtor_callback(i8* {{.*}}, i64 16)
 // CHECK:         ret void