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

[vitalybuka]

Summary

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.

[morehouse]

LGTM

[eugenis]

LGTM

clang/lib/CodeGen/CGClass.cpp

void Emit(CodeGenFunction &CGF, Flags flags) override {
       ASTContext &Context = CGF.getContext();
 
       const RecordDecl *Decl = Dtor->getParent();
       auto Fields = Decl->fields();
       auto IsTrivial = [&](const FieldDecl *F) {
         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());
       }
     }
 
   private:
     /// \param layoutStartOffset index of the ASTRecordLayout field to

clang/test/CodeGenCXX/sanitize-dtor-zero-size-field.cpp

 // 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 {
   ~EmptyNonTrivial();
 };
 
 struct Trivial {
   int a;
struct Struct {
   [[no_unique_address]] Empty e;
   Trivial f1;
   int f2;
   char f3;
 } var;
 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 {
 struct Struct {
   Trivial f1;
   NonTrivial nt;
   [[no_unique_address]] Empty e;
   int f2;
   char f3;
 } var;
 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 {
 struct Struct {
   Trivial f1;
   [[no_unique_address]] Empty e;
   NonTrivial nt;
   int f2;
struct Struct {
   NonTrivial nt;
   [[no_unique_address]] Empty e;
   char f3;
 } var;
 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 {
 struct Struct {
   Trivial f1;
   int f2;
   [[no_unique_address]] Empty e;
   NonTrivial nt;
struct Struct {
   char f3;
   NonTrivial nt;
   [[no_unique_address]] Empty e;
 } var;
 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 {
 struct Struct {
   Trivial f1;
   int f2;
   char f3;
   [[no_unique_address]] Empty e;
struct Struct {
   [[no_unique_address]] EmptyNonTrivial e;
   NonTrivial nt;
   int f2;
   char f3;
 } var;
 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
 
 namespace T9 {
 struct Struct {
   Trivial f1;
   int f2;
   NonTrivial nt;
struct Struct {
   int f2;
   [[no_unique_address]] EmptyNonTrivial e;
   NonTrivial nt;
   char f3;
 } var;
 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
 
 namespace T11 {
 struct Struct {
   Trivial f1;
   int f2;
   char f3;
struct Struct {
   char f3;
   [[no_unique_address]] EmptyNonTrivial e;
   NonTrivial nt;
 } var;
 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