[PATCH] D76782: [CodeGenObjC] Fix a crash when attempting to copy a zero-sized bit-field in a non-trivial C struct

2020-04-06 Thread Erik Pilkington via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd33c7de8e11f: [CodeGenObjC] Fix a crash when attempting to 
copy a zero-sized bit-field in a… (authored by erik.pilkington).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76782/new/

https://reviews.llvm.org/D76782

Files:
  clang/include/clang/AST/NonTrivialTypeVisitor.h
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/test/CodeGenObjC/strong-in-c-struct.m


Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -887,4 +887,17 @@
   func(0);
 }
 
+struct ZeroBitfield {
+  int : 0;
+  id strong;
+};
+
+
+// CHECK: define linkonce_odr hidden void @__default_constructor_8_sv0
+// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_sv0
+void test_zero_bitfield() {
+  struct ZeroBitfield volatile a, b;
+  a = b;
+}
+
 #endif /* USESTRUCT */
Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -254,6 +254,10 @@
 
   void visitVolatileTrivial(QualType FT, const FieldDecl *FD,
 CharUnits CurStructOffset) {
+// Zero-length bit-fields don't need to be copied/assigned.
+if (FD && FD->isZeroLengthBitField(this->Ctx))
+  return;
+
 // Because volatile fields can be bit-fields and are individually copied,
 // their offset and width are in bits.
 uint64_t OffsetInBits =
@@ -543,6 +547,10 @@
 std::array Addrs) {
 LValue DstLV, SrcLV;
 if (FD) {
+  // No need to copy zero-length bit-fields.
+  if (FD->isZeroLengthBitField(this->CGF->getContext()))
+return;
+
   QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
   llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
   Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
Index: clang/include/clang/AST/NonTrivialTypeVisitor.h
===
--- clang/include/clang/AST/NonTrivialTypeVisitor.h
+++ clang/include/clang/AST/NonTrivialTypeVisitor.h
@@ -1,4 +1,4 @@
-//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types *- C++ 
--*-===//
+//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types -*- C++ 
-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.


Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -887,4 +887,17 @@
   func(0);
 }
 
+struct ZeroBitfield {
+  int : 0;
+  id strong;
+};
+
+
+// CHECK: define linkonce_odr hidden void @__default_constructor_8_sv0
+// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_sv0
+void test_zero_bitfield() {
+  struct ZeroBitfield volatile a, b;
+  a = b;
+}
+
 #endif /* USESTRUCT */
Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -254,6 +254,10 @@
 
   void visitVolatileTrivial(QualType FT, const FieldDecl *FD,
 CharUnits CurStructOffset) {
+// Zero-length bit-fields don't need to be copied/assigned.
+if (FD && FD->isZeroLengthBitField(this->Ctx))
+  return;
+
 // Because volatile fields can be bit-fields and are individually copied,
 // their offset and width are in bits.
 uint64_t OffsetInBits =
@@ -543,6 +547,10 @@
 std::array Addrs) {
 LValue DstLV, SrcLV;
 if (FD) {
+  // No need to copy zero-length bit-fields.
+  if (FD->isZeroLengthBitField(this->CGF->getContext()))
+return;
+
   QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
   llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
   Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
Index: clang/include/clang/AST/NonTrivialTypeVisitor.h
===
--- clang/include/clang/AST/NonTrivialTypeVisitor.h
+++ clang/include/clang/AST/NonTrivialTypeVisitor.h
@@ -1,4 +1,4 @@
-//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types *- C++ --*-===//
+//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

[PATCH] D76782: [CodeGenObjC] Fix a crash when attempting to copy a zero-sized bit-field in a non-trivial C struct

2020-04-06 Thread Akira Hatanaka via Phabricator via cfe-commits 
ahatanak accepted this revision.
ahatanak added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76782/new/

https://reviews.llvm.org/D76782



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

[PATCH] D76782: [CodeGenObjC] Fix a crash when attempting to copy a zero-sized bit-field in a non-trivial C struct

2020-04-06 Thread Erik Pilkington via Phabricator via cfe-commits 
erik.pilkington updated this revision to Diff 255385.
erik.pilkington marked 3 inline comments as done.
erik.pilkington added a comment.

Don't bother including zero length bit-fields in the mangling of the 
copy/destroy helpers.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76782/new/

https://reviews.llvm.org/D76782

Files:
  clang/include/clang/AST/NonTrivialTypeVisitor.h
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/test/CodeGenObjC/strong-in-c-struct.m


Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -887,4 +887,17 @@
   func(0);
 }
 
+struct ZeroBitfield {
+  int : 0;
+  id strong;
+};
+
+
+// CHECK: define linkonce_odr hidden void @__default_constructor_8_sv0
+// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_sv0
+void test_zero_bitfield() {
+  struct ZeroBitfield volatile a, b;
+  a = b;
+}
+
 #endif /* USESTRUCT */
Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -254,6 +254,10 @@
 
   void visitVolatileTrivial(QualType FT, const FieldDecl *FD,
 CharUnits CurStructOffset) {
+// Zero-length bit-fields don't need to be copied/assigned.
+if (FD && FD->isZeroLengthBitField(this->Ctx))
+  return;
+
 // Because volatile fields can be bit-fields and are individually copied,
 // their offset and width are in bits.
 uint64_t OffsetInBits =
@@ -543,6 +547,10 @@
 std::array Addrs) {
 LValue DstLV, SrcLV;
 if (FD) {
+  // No need to copy zero-length bit-fields.
+  if (FD->isZeroLengthBitField(this->CGF->getContext()))
+return;
+
   QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
   llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
   Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
Index: clang/include/clang/AST/NonTrivialTypeVisitor.h
===
--- clang/include/clang/AST/NonTrivialTypeVisitor.h
+++ clang/include/clang/AST/NonTrivialTypeVisitor.h
@@ -1,4 +1,4 @@
-//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types *- C++ 
--*-===//
+//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types -*- C++ 
-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.


Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -887,4 +887,17 @@
   func(0);
 }
 
+struct ZeroBitfield {
+  int : 0;
+  id strong;
+};
+
+
+// CHECK: define linkonce_odr hidden void @__default_constructor_8_sv0
+// CHECK: define linkonce_odr hidden void @__copy_assignment_8_8_sv0
+void test_zero_bitfield() {
+  struct ZeroBitfield volatile a, b;
+  a = b;
+}
+
 #endif /* USESTRUCT */
Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -254,6 +254,10 @@
 
   void visitVolatileTrivial(QualType FT, const FieldDecl *FD,
 CharUnits CurStructOffset) {
+// Zero-length bit-fields don't need to be copied/assigned.
+if (FD && FD->isZeroLengthBitField(this->Ctx))
+  return;
+
 // Because volatile fields can be bit-fields and are individually copied,
 // their offset and width are in bits.
 uint64_t OffsetInBits =
@@ -543,6 +547,10 @@
 std::array Addrs) {
 LValue DstLV, SrcLV;
 if (FD) {
+  // No need to copy zero-length bit-fields.
+  if (FD->isZeroLengthBitField(this->CGF->getContext()))
+return;
+
   QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
   llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
   Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
Index: clang/include/clang/AST/NonTrivialTypeVisitor.h
===
--- clang/include/clang/AST/NonTrivialTypeVisitor.h
+++ clang/include/clang/AST/NonTrivialTypeVisitor.h
@@ -1,4 +1,4 @@
-//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types *- C++ --*-===//
+//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
___
cfe-commits mailing list

[PATCH] D76782: [CodeGenObjC] Fix a crash when attempting to copy a zero-sized bit-field in a non-trivial C struct

2020-04-03 Thread Akira Hatanaka via Phabricator via cfe-commits 
ahatanak added a comment.

Thanks for fixing this!




Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:545
 LValue DstLV, SrcLV;
 if (FD) {
+  // No need to copy zero-length bit-fields.

Can you add the same check to `GenBinaryFuncName::visitVolatileTrivial` to 
avoid encoding the zero-length bit field into the name of the copy 
constructor/assignment functions? It isn't necessary to generate different 
functions for `ZeroBitfield`in the test case and the following struct:

```
struct S {
  id strong;
};
```



Comment at: clang/test/CodeGenObjC/strong-in-c-struct.m:894
+};
+
+void test_zero_bitfield() {

Can you check that the zero-sized field information isn't encoded into the name 
of the copy assignment function?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76782/new/

https://reviews.llvm.org/D76782



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

[PATCH] D76782: [CodeGenObjC] Fix a crash when attempting to copy a zero-sized bit-field in a non-trivial C struct

2020-03-25 Thread Erik Pilkington via Phabricator via cfe-commits 
erik.pilkington created this revision.
erik.pilkington added a reviewer: ahatanak.
Herald added subscribers: ributzka, dexonsmith, jkorous.

Zero sized bit-fields aren't included in the CGRecordLayout, so we shouldn't be 
calling EmitLValueForField for them.

rdar://60695105


https://reviews.llvm.org/D76782

Files:
  clang/include/clang/AST/NonTrivialTypeVisitor.h
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/test/CodeGenObjC/strong-in-c-struct.m


Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -887,4 +887,14 @@
   func(0);
 }
 
+struct ZeroBitfield {
+  int : 0;
+  id strong;
+};
+
+void test_zero_bitfield() {
+  struct ZeroBitfield volatile a, b;
+  a = b;
+}
+
 #endif /* USESTRUCT */
Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -543,6 +543,10 @@
 std::array Addrs) {
 LValue DstLV, SrcLV;
 if (FD) {
+  // No need to copy zero-length bit-fields.
+  if (FD->isZeroLengthBitField(this->CGF->getContext()))
+return;
+
   QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
   llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
   Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
Index: clang/include/clang/AST/NonTrivialTypeVisitor.h
===
--- clang/include/clang/AST/NonTrivialTypeVisitor.h
+++ clang/include/clang/AST/NonTrivialTypeVisitor.h
@@ -1,4 +1,4 @@
-//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types *- C++ 
--*-===//
+//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types -*- C++ 
-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.


Index: clang/test/CodeGenObjC/strong-in-c-struct.m
===
--- clang/test/CodeGenObjC/strong-in-c-struct.m
+++ clang/test/CodeGenObjC/strong-in-c-struct.m
@@ -887,4 +887,14 @@
   func(0);
 }
 
+struct ZeroBitfield {
+  int : 0;
+  id strong;
+};
+
+void test_zero_bitfield() {
+  struct ZeroBitfield volatile a, b;
+  a = b;
+}
+
 #endif /* USESTRUCT */
Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -543,6 +543,10 @@
 std::array Addrs) {
 LValue DstLV, SrcLV;
 if (FD) {
+  // No need to copy zero-length bit-fields.
+  if (FD->isZeroLengthBitField(this->CGF->getContext()))
+return;
+
   QualType RT = QualType(FD->getParent()->getTypeForDecl(), 0);
   llvm::PointerType *PtrTy = this->CGF->ConvertType(RT)->getPointerTo();
   Address DstAddr = this->getAddrWithOffset(Addrs[DstIdx], Offset);
Index: clang/include/clang/AST/NonTrivialTypeVisitor.h
===
--- clang/include/clang/AST/NonTrivialTypeVisitor.h
+++ clang/include/clang/AST/NonTrivialTypeVisitor.h
@@ -1,4 +1,4 @@
-//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types *- C++ --*-===//
+//===-- NonTrivialTypeVisitor.h - Visitor for non-trivial Types -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits