[PATCH] D155844: [clang][dataflow] fix failing assert in copyRecord

[Paul Semel via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG145f353fd679: [clang][dataflow] fix failing assert in 
copyRecord (authored by paulsemel).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155844

Files:
  clang/lib/Analysis/FlowSensitive/RecordOps.cpp
  clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -194,6 +194,40 @@
   });
 }
 
+TEST(TransferTest, CopyRecordFromDerivedToBase) {
+  std::string Code = R"(
+struct A {
+  int i;
+};
+
+struct B : public A {
+};
+
+void target(A a, B b) {
+  (void)a.i;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> ,
+ ASTContext ) {
+Environment Env = getEnvironmentAtAnnotation(Results, "p").fork();
+
+const ValueDecl *IDecl = findValueDecl(ASTCtx, "i");
+auto  = getLocForDecl(ASTCtx, Env, "a");
+auto  = getLocForDecl(ASTCtx, Env, "b");
+
+EXPECT_NE(Env.getValue(*A.getChild(*IDecl)),
+  Env.getValue(*B.getChild(*IDecl)));
+
+copyRecord(B, A, Env);
+
+EXPECT_EQ(Env.getValue(*A.getChild(*IDecl)),
+  Env.getValue(*B.getChild(*IDecl)));
+  });
+}
+
 } // namespace
 } // namespace test
 } // namespace dataflow
Index: clang/lib/Analysis/FlowSensitive/RecordOps.cpp
===
--- clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -17,18 +17,27 @@
 void clang::dataflow::copyRecord(AggregateStorageLocation ,
  AggregateStorageLocation ,
  Environment ) {
+  auto SrcType = Src.getType().getCanonicalType().getUnqualifiedType();
+  auto DstType = Dst.getType().getCanonicalType().getUnqualifiedType();
+
+  auto SrcDecl = SrcType->getAsCXXRecordDecl();
+  auto DstDecl = DstType->getAsCXXRecordDecl();
+
+  bool compatibleTypes =
+  SrcType == DstType ||
+  (SrcDecl && DstDecl && SrcDecl->isDerivedFrom(DstDecl));
+  (void)compatibleTypes;
+
   LLVM_DEBUG({
-if (Dst.getType().getCanonicalType().getUnqualifiedType() !=
-Src.getType().getCanonicalType().getUnqualifiedType()) {
+if (!compatibleTypes) {
   llvm::dbgs() << "Source type " << Src.getType() << "\n";
   llvm::dbgs() << "Destination type " << Dst.getType() << "\n";
 }
   });
-  assert(Dst.getType().getCanonicalType().getUnqualifiedType() ==
- Src.getType().getCanonicalType().getUnqualifiedType());
+  assert(compatibleTypes);
 
-  for (auto [Field, SrcFieldLoc] : Src.children()) {
-StorageLocation *DstFieldLoc = Dst.getChild(*Field);
+  for (auto [Field, DstFieldLoc] : Dst.children()) {
+StorageLocation *SrcFieldLoc = Src.getChild(*Field);
 
 assert(Field->getType()->isReferenceType() ||
(SrcFieldLoc != nullptr && DstFieldLoc != nullptr));


Index: clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -194,6 +194,40 @@
   });
 }
 
+TEST(TransferTest, CopyRecordFromDerivedToBase) {
+  std::string Code = R"(
+struct A {
+  int i;
+};
+
+struct B : public A {
+};
+
+void target(A a, B b) {
+  (void)a.i;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> ,
+ ASTContext ) {
+Environment Env = getEnvironmentAtAnnotation(Results, "p").fork();
+
+const ValueDecl *IDecl = findValueDecl(ASTCtx, "i");
+auto  = getLocForDecl(ASTCtx, Env, "a");
+auto  = getLocForDecl(ASTCtx, Env, "b");
+
+EXPECT_NE(Env.getValue(*A.getChild(*IDecl)),
+  Env.getValue(*B.getChild(*IDecl)));
+
+copyRecord(B, A, Env);
+
+EXPECT_EQ(Env.getValue(*A.getChild(*IDecl)),
+  Env.getValue(*B.getChild(*IDecl)));
+  });
+}
+
 } // namespace
 } // namespace test
 } // namespace dataflow
Index: clang/lib/Analysis/FlowSensitive/RecordOps.cpp
===
--- clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -17,18 +17,27 @@
 void clang::dataflow::copyRecord(AggregateStorageLocation ,
  AggregateStorageLocation ,
  Environment ) {
+  auto SrcType = 

[PATCH] D144892: [clang][dataflow] unnamed bitfields should be discarded in InitListExpr

[Paul Semel via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG96d035c1dcd7: [clang][dataflow] unnamed bitfields should be 
discarded in InitListExpr (authored by paulsemel).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144892

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5080,4 +5080,28 @@
   {BuiltinOptions{ContextSensitiveOptions{}}});
 }
 
+TEST(TransferTest, UnnamedBitfieldInitializer) {
+  std::string Code = R"(
+struct B {};
+struct A {
+  unsigned a;
+  unsigned : 4;
+  unsigned c;
+  B b;
+};
+void target() {
+  A a = {};
+  A test = a;
+  (void)test.c;
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> ,
+ ASTContext ) {
+// This doesn't need a body because this test was crashing the 
framework
+// before handling correctly Unnamed bitfields in `InitListExpr`.
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -732,7 +732,15 @@
 Env.setValue(Loc, *Val);
 
 if (Type->isStructureOrClassType()) {
-  for (auto It : llvm::zip(Type->getAsRecordDecl()->fields(), S->inits())) 
{
+  // Unnamed bitfields are only used for padding and are not appearing in
+  // `InitListExpr`'s inits. However, those fields do appear in 
RecordDecl's
+  // field list, and we thus need to remove them before mapping inits to
+  // fields to avoid mapping inits to the wrongs fields.
+  std::vector Fields;
+  llvm::copy_if(
+  Type->getAsRecordDecl()->fields(), std::back_inserter(Fields),
+  [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
+  for (auto It : llvm::zip(Fields, S->inits())) {
 const FieldDecl *Field = std::get<0>(It);
 assert(Field != nullptr);
 


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5080,4 +5080,28 @@
   {BuiltinOptions{ContextSensitiveOptions{}}});
 }
 
+TEST(TransferTest, UnnamedBitfieldInitializer) {
+  std::string Code = R"(
+struct B {};
+struct A {
+  unsigned a;
+  unsigned : 4;
+  unsigned c;
+  B b;
+};
+void target() {
+  A a = {};
+  A test = a;
+  (void)test.c;
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> ,
+ ASTContext ) {
+// This doesn't need a body because this test was crashing the framework
+// before handling correctly Unnamed bitfields in `InitListExpr`.
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -732,7 +732,15 @@
 Env.setValue(Loc, *Val);
 
 if (Type->isStructureOrClassType()) {
-  for (auto It : llvm::zip(Type->getAsRecordDecl()->fields(), S->inits())) {
+  // Unnamed bitfields are only used for padding and are not appearing in
+  // `InitListExpr`'s inits. However, those fields do appear in RecordDecl's
+  // field list, and we thus need to remove them before mapping inits to
+  // fields to avoid mapping inits to the wrongs fields.
+  std::vector Fields;
+  llvm::copy_if(
+  Type->getAsRecordDecl()->fields(), std::back_inserter(Fields),
+  [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
+  for (auto It : llvm::zip(Fields, S->inits())) {
 const FieldDecl *Field = std::get<0>(It);
 assert(Field != nullptr);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144546: [clang][dataflow] Fix assert for CXXConstructExpr argument number

[Paul Semel via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe6e753d173db: [clang][dataflow] Fix wrong assert for 
CXXConstructExpr (authored by paulsemel).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144546

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2102,6 +2102,54 @@
   });
 }
 
+TEST(TransferTest, CopyConstructorWithDefaultArgument) {
+  std::string Code = R"(
+struct A {
+  int Baz;
+  A() = default;
+  A(const A& a, bool def = true) { Baz = a.Baz; }
+};
+
+void target() {
+  A Foo;
+  (void)Foo.Baz;
+  A Bar = Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> ,
+ ASTContext ) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment  = getEnvironmentAtAnnotation(Results, "p");
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarLoc = cast(
+Env.getStorageLocation(*BarDecl, SkipPast::None));
+
+const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *BarVal = cast(Env.getValue(*BarLoc));
+EXPECT_EQ(FooVal, BarVal);
+
+const auto *FooBazVal =
+cast(Env.getValue(FooLoc->getChild(*BazDecl)));
+const auto *BarBazVal =
+cast(Env.getValue(BarLoc->getChild(*BazDecl)));
+EXPECT_EQ(FooBazVal, BarBazVal);
+  });
+}
+
 TEST(TransferTest, CopyConstructorWithParens) {
   std::string Code = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -561,7 +561,9 @@
 assert(ConstructorDecl != nullptr);
 
 if (ConstructorDecl->isCopyOrMoveConstructor()) {
-  assert(S->getNumArgs() == 1);
+  // It is permissible for a copy/move constructor to have additional
+  // parameters as long as they have default arguments defined for them.
+  assert(S->getNumArgs() != 0);
 
   const Expr *Arg = S->getArg(0);
   assert(Arg != nullptr);


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2102,6 +2102,54 @@
   });
 }
 
+TEST(TransferTest, CopyConstructorWithDefaultArgument) {
+  std::string Code = R"(
+struct A {
+  int Baz;
+  A() = default;
+  A(const A& a, bool def = true) { Baz = a.Baz; }
+};
+
+void target() {
+  A Foo;
+  (void)Foo.Baz;
+  A Bar = Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> ,
+ ASTContext ) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment  = getEnvironmentAtAnnotation(Results, "p");
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooLoc = cast(
+Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *BarLoc = cast(
+Env.getStorageLocation(*BarDecl, SkipPast::None));
+
+const auto *FooVal = cast(Env.getValue(*FooLoc));
+const auto *BarVal = cast(Env.getValue(*BarLoc));
+EXPECT_EQ(FooVal, BarVal);
+
+const auto *FooBazVal =
+cast(Env.getValue(FooLoc->getChild(*BazDecl)));
+const auto *BarBazVal =
+cast(Env.getValue(BarLoc->getChild(*BazDecl)));
+EXPECT_EQ(FooBazVal, BarBazVal);
+  });
+}
+
 TEST(TransferTest, CopyConstructorWithParens) {
   std::string Code = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 153511.
paulsemel added a comment.

Fixed version problem. Now building.


Repository:
  rC Clang

https://reviews.llvm.org/D47953

Files:
  lib/CodeGen/CGBuiltin.cpp


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1170,6 +1170,7 @@
   RecordDecl *RD = RT->getDecl()->getDefinition();
   ASTContext  = RD->getASTContext();
   const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+  const CGRecordLayout  = CGF.getTypes().getCGRecordLayout(RD);
   std::string Pad = std::string(Lvl * 4, ' ');
 
   Value *GString =
@@ -1208,7 +1209,7 @@
   FieldPtr, CGF.ConvertType(Context.getPointerType(FD->getType(;
 else
   FieldPtr = CGF.Builder.CreateStructGEP(CGF.ConvertType(RType), FieldPtr,
- FD->getFieldIndex());
+ CGRL.getLLVMFieldNo(FD));
 
 GString = CGF.Builder.CreateGlobalStringPtr(
 llvm::Twine(Pad)
@@ -1236,10 +1237,37 @@
  ? Types[Context.VoidPtrTy]
  : Types[CanonicalType];
 
+if (FD->isBitField())
+  FieldPtr = CGF.Builder.CreatePointerCast(
+  FieldPtr, CGF.ConvertType(Context.getPointerType(CanonicalType)));
+
 Address FieldAddress = Address(FieldPtr, Align);
 FieldPtr = CGF.Builder.CreateLoad(FieldAddress);
 
-// FIXME Need to handle bitfield here
+if (FD->isBitField()) {
+  const CGBitFieldInfo  = CGRL.getBitFieldInfo(FD);
+  if (Info.IsSigned) {
+unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
+if (HighBits)
+  FieldPtr = CGF.Builder.CreateShl(FieldPtr, HighBits);
+if (Info.Offset + HighBits)
+  FieldPtr = CGF.Builder.CreateAShr(FieldPtr, Info.Offset + HighBits);
+  } else {
+if (Info.Offset)
+  FieldPtr = CGF.Builder.CreateLShr(FieldPtr, Info.Offset);
+if (static_cast(Info.Offset) + Info.Size < Info.StorageSize)
+  FieldPtr = CGF.Builder.CreateAnd(
+  FieldPtr,
+  ConstantInt::get(CGF.ConvertType(CanonicalType),
+   llvm::APInt::getLowBitsSet(
+   CGF.CGM.getDataLayout().getTypeSizeInBits(
+   CGF.ConvertType(CanonicalType)),
+   Info.Size)));
+  }
+  FieldPtr = CGF.Builder.CreateIntCast(
+  FieldPtr, CGF.ConvertType(CanonicalType), Info.IsSigned);
+}
+
 GString = CGF.Builder.CreateGlobalStringPtr(
 Format.concat(llvm::Twine('\n')).str());
 TmpRes = CGF.Builder.CreateCall(Func, {GString, FieldPtr});


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1170,6 +1170,7 @@
   RecordDecl *RD = RT->getDecl()->getDefinition();
   ASTContext  = RD->getASTContext();
   const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+  const CGRecordLayout  = CGF.getTypes().getCGRecordLayout(RD);
   std::string Pad = std::string(Lvl * 4, ' ');
 
   Value *GString =
@@ -1208,7 +1209,7 @@
   FieldPtr, CGF.ConvertType(Context.getPointerType(FD->getType(;
 else
   FieldPtr = CGF.Builder.CreateStructGEP(CGF.ConvertType(RType), FieldPtr,
- FD->getFieldIndex());
+ CGRL.getLLVMFieldNo(FD));
 
 GString = CGF.Builder.CreateGlobalStringPtr(
 llvm::Twine(Pad)
@@ -1236,10 +1237,37 @@
  ? Types[Context.VoidPtrTy]
  : Types[CanonicalType];
 
+if (FD->isBitField())
+  FieldPtr = CGF.Builder.CreatePointerCast(
+  FieldPtr, CGF.ConvertType(Context.getPointerType(CanonicalType)));
+
 Address FieldAddress = Address(FieldPtr, Align);
 FieldPtr = CGF.Builder.CreateLoad(FieldAddress);
 
-// FIXME Need to handle bitfield here
+if (FD->isBitField()) {
+  const CGBitFieldInfo  = CGRL.getBitFieldInfo(FD);
+  if (Info.IsSigned) {
+unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
+if (HighBits)
+  FieldPtr = CGF.Builder.CreateShl(FieldPtr, HighBits);
+if (Info.Offset + HighBits)
+  FieldPtr = CGF.Builder.CreateAShr(FieldPtr, Info.Offset + HighBits);
+  } else {
+if (Info.Offset)
+  FieldPtr = CGF.Builder.CreateLShr(FieldPtr, Info.Offset);
+if (static_cast(Info.Offset) + Info.Size < Info.StorageSize)
+  FieldPtr = CGF.Builder.CreateAnd(
+  FieldPtr,
+  ConstantInt::get(CGF.ConvertType(CanonicalType),
+   llvm::APInt::getLowBitsSet(
+   CGF.CGM.getDataLayout().getTypeSizeInBits(
+   

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

In https://reviews.llvm.org/D47953#1143044, @dblaikie wrote:

> This doesn't seem to build for me - so hard to debug/probe it:
>
> llvm/src/tools/clang/lib/CodeGen/CGBuiltin.cpp:1264:65: error: no viable 
> conversion from 'clang::QualType' to 'llvm::Type *'
>
>   CGF.CGM.getDataLayout().getTypeSizeInBits(CanonicalType),
> ^
>
> llvm/src/include/llvm/IR/DataLayout.h:560:53: note: passing argument to 
> parameter 'Ty' here
>  inline uint64_t DataLayout::getTypeSizeInBits(Type *Ty) const {
>
>   ^
>
> 1 error generated.


Very sorry about it, I should have paid more attention..


Repository:
  rC Clang

https://reviews.llvm.org/D47953



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

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

First thanks all for reviewing !

Basically, what's happening is that it works good with non-packed structures.
Here is an example for packed structure (with unsigned signed short):

  c
  struct lol {
unsigned short a:3;
unsigned short b:2;
unsigned short c:6;
unsigned short d:2;
unsigned short e:6;
  } __attribute__((packed));
  
  int main(void)
  {
struct lol lolo = {
.a = 2,
.b = 1,
.c = 13,
.d = 2,
.e = 9
};
__builtin_dump_struct(, );
return 0;
  }

Here is the output I get :

  sh
  unsigned short a : 2
  unsigned short b : 1
  unsigned short c : 13
  unsigned short d : 2
  unsigned short e : 1

The last `unsigned short` is incorrect.

If I switch this to signed integers, I get :

  sh
  unsigned short a : 0
  unsigned short b : 0
  unsigned short c : 0
  unsigned short d : 0
  unsigned short e : 0

All the fields are zeroed. I absolutely do not understand what is happening...
Thanks for trying to help guys !


Repository:
  rC Clang

https://reviews.llvm.org/D47953



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

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

I will for sure add tests @lebedev.ri . Fact is that, for the moment, this is 
not working as expected.
This is why I am asking for a bit of help about this bitfield handling :)




Comment at: lib/CodeGen/CGBuiltin.cpp:1250
+  if (Info.IsSigned) {
+unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
+if (HighBits)

aaron.ballman wrote:
> What happens if this overflows due to being < 0?
How could this be < 0 ?
If it is, it means that there was a problem during bitfield construction isn't 
it ?
I mean StorageSize is always greater that Offset + Size


Repository:
  rC Clang

https://reviews.llvm.org/D47953



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

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

ping :) @aaron.ballman


Repository:
  rC Clang

https://reviews.llvm.org/D47953



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

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

This version is working for non packed structures. For packed structures, it 
might sometimes work, sometimes not.
The resulting assembly code seems to be the right one.
If someone went through bitfields manipulation, please do not hesitate to 
comment !
Requesting for help :)


Repository:
  rC Clang

https://reviews.llvm.org/D47953



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

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel created this revision.
paulsemel added a reviewer: aaron.ballman.

This is an attempt for adding bitfield support to __builtin_dump_struct.


Repository:
  rC Clang

https://reviews.llvm.org/D47953

Files:
  lib/CodeGen/CGBuiltin.cpp


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1170,6 +1170,7 @@
   RecordDecl *RD = RT->getDecl()->getDefinition();
   ASTContext  = RD->getASTContext();
   const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+  const CGRecordLayout  = CGF.getTypes().getCGRecordLayout(RD);
   std::string Pad = std::string(Lvl * 4, ' ');
 
   Value *GString =
@@ -1208,7 +1209,7 @@
   FieldPtr, CGF.ConvertType(Context.getPointerType(FD->getType(;
 else
   FieldPtr = CGF.Builder.CreateStructGEP(CGF.ConvertType(RType), FieldPtr,
- FD->getFieldIndex());
+ CGRL.getLLVMFieldNo(FD));
 
 GString = CGF.Builder.CreateGlobalStringPtr(
 llvm::Twine(Pad)
@@ -1236,10 +1237,37 @@
  ? Types[Context.VoidPtrTy]
  : Types[CanonicalType];
 
+if (FD->isBitField())
+  FieldPtr = CGF.Builder.CreatePointerCast(
+  FieldPtr, CGF.ConvertType(Context.getPointerType(CanonicalType)));
+
 Address FieldAddress = Address(FieldPtr, Align);
 FieldPtr = CGF.Builder.CreateLoad(FieldAddress);
 
-// FIXME Need to handle bitfield here
+if (FD->isBitField()) {
+  const CGBitFieldInfo  = CGRL.getBitFieldInfo(FD);
+  if (Info.IsSigned) {
+unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
+if (HighBits)
+  FieldPtr = CGF.Builder.CreateShl(FieldPtr, HighBits);
+if (Info.Offset + HighBits)
+  FieldPtr = CGF.Builder.CreateAShr(FieldPtr, Info.Offset + HighBits);
+  } else {
+if (Info.Offset)
+  FieldPtr = CGF.Builder.CreateLShr(FieldPtr, Info.Offset);
+if (static_cast(Info.Offset) + Info.Size < Info.StorageSize)
+  FieldPtr = CGF.Builder.CreateAnd(
+  FieldPtr,
+  ConstantInt::get(
+  CGF.ConvertType(CanonicalType),
+  llvm::APInt::getLowBitsSet(
+  CGF.CGM.getDataLayout().getTypeSizeInBits(CanonicalType),
+  Info.Size)));
+  }
+  FieldPtr = CGF.Builder.CreateIntCast(
+  FieldPtr, CGF.ConvertType(CanonicalType), Info.IsSigned);
+}
+
 GString = CGF.Builder.CreateGlobalStringPtr(
 Format.concat(llvm::Twine('\n')).str());
 TmpRes = CGF.Builder.CreateCall(Func, {GString, FieldPtr});


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1170,6 +1170,7 @@
   RecordDecl *RD = RT->getDecl()->getDefinition();
   ASTContext  = RD->getASTContext();
   const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+  const CGRecordLayout  = CGF.getTypes().getCGRecordLayout(RD);
   std::string Pad = std::string(Lvl * 4, ' ');
 
   Value *GString =
@@ -1208,7 +1209,7 @@
   FieldPtr, CGF.ConvertType(Context.getPointerType(FD->getType(;
 else
   FieldPtr = CGF.Builder.CreateStructGEP(CGF.ConvertType(RType), FieldPtr,
- FD->getFieldIndex());
+ CGRL.getLLVMFieldNo(FD));
 
 GString = CGF.Builder.CreateGlobalStringPtr(
 llvm::Twine(Pad)
@@ -1236,10 +1237,37 @@
  ? Types[Context.VoidPtrTy]
  : Types[CanonicalType];
 
+if (FD->isBitField())
+  FieldPtr = CGF.Builder.CreatePointerCast(
+  FieldPtr, CGF.ConvertType(Context.getPointerType(CanonicalType)));
+
 Address FieldAddress = Address(FieldPtr, Align);
 FieldPtr = CGF.Builder.CreateLoad(FieldAddress);
 
-// FIXME Need to handle bitfield here
+if (FD->isBitField()) {
+  const CGBitFieldInfo  = CGRL.getBitFieldInfo(FD);
+  if (Info.IsSigned) {
+unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
+if (HighBits)
+  FieldPtr = CGF.Builder.CreateShl(FieldPtr, HighBits);
+if (Info.Offset + HighBits)
+  FieldPtr = CGF.Builder.CreateAShr(FieldPtr, Info.Offset + HighBits);
+  } else {
+if (Info.Offset)
+  FieldPtr = CGF.Builder.CreateLShr(FieldPtr, Info.Offset);
+if (static_cast(Info.Offset) + Info.Size < Info.StorageSize)
+  FieldPtr = CGF.Builder.CreateAnd(
+  FieldPtr,
+  ConstantInt::get(
+  CGF.ConvertType(CanonicalType),
+  llvm::APInt::getLowBitsSet(
+  CGF.CGM.getDataLayout().getTypeSizeInBits(CanonicalType),
+  Info.Size)));
+  }
+  FieldPtr = 

[PATCH] D46064: [llvm-objcopy] Add --localize-symbol option

[Paul Semel via Phabricator via cfe-commits]

paulsemel created this revision.
paulsemel added reviewers: jakehehrlich, echristo.
Herald added a subscriber: llvm-commits.

This option permit to localize a symbol by given its name.


Repository:
  rL LLVM

https://reviews.llvm.org/D46064

Files:
  test/tools/llvm-objcopy/localize.test
  tools/llvm-objcopy/Opts.td
  tools/llvm-objcopy/llvm-objcopy.cpp

Index: tools/llvm-objcopy/llvm-objcopy.cpp
===
--- tools/llvm-objcopy/llvm-objcopy.cpp
+++ tools/llvm-objcopy/llvm-objcopy.cpp
@@ -118,6 +118,7 @@
   std::vector Keep;
   std::vector OnlyKeep;
   std::vector AddSection;
+  std::vector LocalizeSymbol;
   bool StripAll;
   bool StripAllGNU;
   bool StripDebug;
@@ -196,6 +197,14 @@
 });
   }
 
+  if (!Config.LocalizeSymbol.empty()) {
+Obj.SymbolTable->localize([](const Symbol ) {
+  return std::find(std::begin(Config.LocalizeSymbol),
+   std::end(Config.LocalizeSymbol),
+   Sym.Name) != std::end(Config.LocalizeSymbol);
+});
+  }
+
   SectionPred RemovePred = [](const SectionBase &) { return false; };
 
   // Removes:
@@ -398,6 +407,8 @@
   Config.StripNonAlloc = InputArgs.hasArg(OBJCOPY_strip_non_alloc);
   Config.ExtractDWO = InputArgs.hasArg(OBJCOPY_extract_dwo);
   Config.LocalizeHidden = InputArgs.hasArg(OBJCOPY_localize_hidden);
+  for (auto Arg : InputArgs.filtered(OBJCOPY_localize_symbol))
+Config.LocalizeSymbol.push_back(Arg->getValue());
 
   return Config;
 }
Index: tools/llvm-objcopy/Opts.td
===
--- tools/llvm-objcopy/Opts.td
+++ tools/llvm-objcopy/Opts.td
@@ -56,3 +56,8 @@
   HelpText<"Remove all sections that are not DWARF .dwo sections from file">;
 def localize_hidden : Flag<["-", "--"], "localize-hidden">,
   HelpText<"Mark all symbols that have hidden or internal visibility as local">;
+defm localize_symbol : Eq<"localize-symbol">,
+ MetaVarName<"symbol">,
+ HelpText<"Localize ">;
+def L : JoinedOrSeparate<["-"], "L">,
+Alias;
Index: test/tools/llvm-objcopy/localize.test
===
--- /dev/null
+++ test/tools/llvm-objcopy/localize.test
@@ -0,0 +1,104 @@
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy --localize-symbol defaultGlobal  %t %t2
+# RUN: llvm-readobj -symbols %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:ELFDATA2LSB
+  Type:ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x1000
+AddressAlign:0x0010
+Size:64
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC ]
+Address: 0x2000
+AddressAlign:0x0010
+Content: ""
+Symbols:
+  Local:
+- Name: hiddenLocal
+  Type: STT_FUNC
+  Section:  .text
+  Value:0x1008
+  Size: 8
+  Visibility: STV_HIDDEN
+  Weak:
+- Name: hiddenWeak
+  Type: STT_FUNC
+  Section:  .text
+  Value:0x1010
+  Size: 8
+  Visibility: STV_HIDDEN
+  Global:
+- Name: defaultGlobal
+  Type: STT_FUNC
+  Size: 8
+  Section:  .text
+  Value:0x1000
+- Name: hiddenGlobal
+  Type: STT_OBJECT
+  Section:  .data
+  Value:0x2006
+  Size: 2
+  Visibility: STV_HIDDEN
+
+#CHECK: Symbols [
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:Name:
+#CHECK-NEXT:Value: 0x0
+#CHECK-NEXT:Size: 0
+#CHECK-NEXT:Binding: Local
+#CHECK-NEXT:Type: None
+#CHECK-NEXT:Other: 0
+#CHECK-NEXT:Section: Undefined
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:Name: hiddenLocal
+#CHECK-NEXT:Value: 0x1008
+#CHECK-NEXT:Size: 8
+#CHECK-NEXT:Binding: Local
+#CHECK-NEXT:Type: Function
+#CHECK-NEXT:Other [
+#CHECK-NEXT:  STV_HIDDEN
+#CHECK-NEXT:]
+#CHECK-NEXT:Section: .text
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:Name: defaultGlobal
+#CHECK-NEXT:Value: 0x1000
+#CHECK-NEXT:Size: 8
+#CHECK-NEXT:Binding: Local
+#CHECK-NEXT:Type: Function
+#CHECK-NEXT:Other: 0
+#CHECK-NEXT:Section: .text
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:Name: hiddenGlobal
+#CHECK-NEXT:Value: 0x2006
+#CHECK-NEXT:Size: 2
+#CHECK-NEXT:Binding: Global
+#CHECK-NEXT:Type: Object
+#CHECK-NEXT:Other [
+#CHECK-NEXT:  STV_HIDDEN
+#CHECK-NEXT:]
+#CHECK-NEXT:Section: .data
+#CHECK-NEXT:  }
+#CHECK-NEXT:  Symbol {
+#CHECK-NEXT:Name: hiddenWeak
+#CHECK-NEXT:Value: 0x1010
+#CHECK-NEXT:Size: 8
+#CHECK-NEXT:Binding: Weak
+#CHECK-NEXT:Type: Function
+#CHECK-NEXT:Other [
+#CHECK-NEXT:  

[PATCH] D46033: add check for long double for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel closed this revision.
paulsemel added a comment.

commited on r330808


Repository:
  rC Clang

https://reviews.llvm.org/D46033



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

[PATCH] D46033: add check for long double for __builtin_dump_struct

[Paul Semel via Phabricator via cfe-commits]

paulsemel created this revision.
paulsemel added a reviewer: aaron.ballman.

This patch adds a test for long double format.


Repository:
  rC Clang

https://reviews.llvm.org/D46033

Files:
  test/CodeGen/dump-struct-builtin.c


Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -107,6 +107,12 @@
 // CHECK-NEXT: [[FORMAT_U17:@[0-9]+]] = private unnamed_addr constant [6 x i8] 
c"%hhd\0A\00"
 // CHECK-NEXT: [[END_STRUCT_U17:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
 
+// CHECK: @unit18.a = private unnamed_addr constant %struct.U18A { x86_fp80 
0xK3FFF8FCD67FD3F5B6000 }, align 16
+// CHECK-NEXT: [[STRUCT_STR_U18:@[0-9]+]] = private unnamed_addr constant [15 
x i8] c"struct U18A {\0A\00"
+// CHECK-NEXT: [[FIELD_U18:@[0-9]+]] = private unnamed_addr constant [17 x i8] 
c"long double a : \00"
+// CHECK-NEXT: [[FORMAT_U18:@[0-9]+]] = private unnamed_addr constant [5 x i8] 
c"%Lf\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U18:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
 int printf(const char *fmt, ...) {
 return 0;
 }
@@ -417,6 +423,24 @@
   __builtin_dump_struct(, );
 }
 
+void unit18() {
+  struct U18A {
+long double a;
+  };
+
+  struct U18A a = {
+  .a = 1.123456,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([15 x i8], 
[15 x i8]* [[STRUCT_STR_U18]], i32 0, i32 0))
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U18A, 
%struct.U18A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([17 x i8], 
[17 x i8]* [[FIELD_U18]], i32 0, i32 0))
+  // CHECK: [[LOAD1:%[0-9]+]] = load x86_fp80, x86_fp80* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([5 x i8], 
[5 x i8]* [[FORMAT_U18]], i32 0, i32 0), x86_fp80 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], 
[3 x i8]* [[END_STRUCT_U18]], i32 0, i32 0)
+  __builtin_dump_struct(, );
+}
+
 void test1() {
   struct T1A {
 int a;


Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -107,6 +107,12 @@
 // CHECK-NEXT: [[FORMAT_U17:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%hhd\0A\00"
 // CHECK-NEXT: [[END_STRUCT_U17:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
 
+// CHECK: @unit18.a = private unnamed_addr constant %struct.U18A { x86_fp80 0xK3FFF8FCD67FD3F5B6000 }, align 16
+// CHECK-NEXT: [[STRUCT_STR_U18:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"struct U18A {\0A\00"
+// CHECK-NEXT: [[FIELD_U18:@[0-9]+]] = private unnamed_addr constant [17 x i8] c"long double a : \00"
+// CHECK-NEXT: [[FORMAT_U18:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%Lf\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U18:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
 int printf(const char *fmt, ...) {
 return 0;
 }
@@ -417,6 +423,24 @@
   __builtin_dump_struct(, );
 }
 
+void unit18() {
+  struct U18A {
+long double a;
+  };
+
+  struct U18A a = {
+  .a = 1.123456,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([15 x i8], [15 x i8]* [[STRUCT_STR_U18]], i32 0, i32 0))
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U18A, %struct.U18A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([17 x i8], [17 x i8]* [[FIELD_U18]], i32 0, i32 0))
+  // CHECK: [[LOAD1:%[0-9]+]] = load x86_fp80, x86_fp80* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([5 x i8], [5 x i8]* [[FORMAT_U18]], i32 0, i32 0), x86_fp80 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], [3 x i8]* [[END_STRUCT_U18]], i32 0, i32 0)
+  __builtin_dump_struct(, );
+}
+
 void test1() {
   struct T1A {
 int a;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

In https://reviews.llvm.org/D45615#1069755, @aaron.ballman wrote:

> LGTM! I will go ahead and commit on your behalf. Btw, if you're thinking of 
> making additional patches, now might be a good time to ask for commit 
> privileges 
> (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access).


Thanks ! I actually really want to continue improving the builtin, so yes, I 
will make additional patches ! I just asked for accesses today :)


Repository:
  rC Clang

https://reviews.llvm.org/D45615



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

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 142750.
paulsemel added a comment.

Removed the two existing types.
Added the correct format sizing.
Added tests for those formats.
These version is now relying on the previous patch : 
https://reviews.llvm.org/D45665


Repository:
  rC Clang

https://reviews.llvm.org/D45615

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/dump-struct-builtin.c


Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s
 
 #include "Inputs/stdio.h"
+#include 
 
 // CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, 
align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x 
i8] c"struct U1A {\0A\00"
@@ -94,6 +95,18 @@
 // CHECK-NEXT: [[FORMAT_U15:@[0-9]+]] = private unnamed_addr constant [4 x i8] 
c"%p\0A\00"
 // CHECK-NEXT: [[END_STRUCT_U15:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
 
+// CHECK: @unit16.a = private unnamed_addr constant %struct.U16A { i8 12 }, 
align 1
+// CHECK-NEXT: [[STRUCT_STR_U16:@[0-9]+]] = private unnamed_addr constant [15 
x i8] c"struct U16A {\0A\00"
+// CHECK-NEXT: [[FIELD_U16:@[0-9]+]] = private unnamed_addr constant [13 x i8] 
c"uint8_t a : \00"
+// CHECK-NEXT: [[FORMAT_U16:@[0-9]+]] = private unnamed_addr constant [6 x i8] 
c"%hhu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U16:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
+// CHECK: @unit17.a = private unnamed_addr constant %struct.U17A { i8 12 }, 
align 1
+// CHECK-NEXT: [[STRUCT_STR_U17:@[0-9]+]] = private unnamed_addr constant [15 
x i8] c"struct U17A {\0A\00"
+// CHECK-NEXT: [[FIELD_U17:@[0-9]+]] = private unnamed_addr constant [12 x i8] 
c"int8_t a : \00"
+// CHECK-NEXT: [[FORMAT_U17:@[0-9]+]] = private unnamed_addr constant [6 x i8] 
c"%hhd\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U17:@[0-9]+]] = private unnamed_addr constant [3 x 
i8] c"}\0A\00"
+
 int printf(const char *fmt, ...) {
 return 0;
 }
@@ -368,6 +381,42 @@
   __builtin_dump_struct(, );
 }
 
+void unit16() {
+  struct U16A {
+uint8_t a;
+  };
+
+  struct U16A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([15 x i8], 
[15 x i8]* [[STRUCT_STR_U16]], i32 0, i32 0))
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U16A, 
%struct.U16A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([13 x i8], 
[13 x i8]* [[FIELD_U16]], i32 0, i32 0))
+  // CHECK: [[LOAD1:%[0-9]+]] = load i8, i8* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], 
[6 x i8]* [[FORMAT_U16]], i32 0, i32 0), i8 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], 
[3 x i8]* [[END_STRUCT_U16]], i32 0, i32 0)
+  __builtin_dump_struct(, );
+}
+
+void unit17() {
+  struct U17A {
+int8_t a;
+  };
+
+  struct U17A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([15 x i8], 
[15 x i8]* [[STRUCT_STR_U17]], i32 0, i32 0))
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U17A, 
%struct.U17A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([12 x i8], 
[12 x i8]* [[FIELD_U17]], i32 0, i32 0))
+  // CHECK: [[LOAD1:%[0-9]+]] = load i8, i8* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], 
[6 x i8]* [[FORMAT_U17]], i32 0, i32 0), i8 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], 
[3 x i8]* [[END_STRUCT_U17]], i32 0, i32 0)
+  __builtin_dump_struct(, );
+}
+
 void test1() {
   struct T1A {
 int a;
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -949,6 +949,8 @@
   if (Types.empty()) {
 Types[Context.CharTy] = "%c";
 Types[Context.BoolTy] = "%d";
+Types[Context.SignedCharTy] = "%hhd";
+Types[Context.UnsignedCharTy] = "%hhu";
 Types[Context.IntTy] = "%d";
 Types[Context.UnsignedIntTy] = "%u";
 Types[Context.LongTy] = "%ld";


Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 
 #include "Inputs/stdio.h"
+#include 
 
 // CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00"
@@ -94,6 +95,18 @@
 // CHECK-NEXT: [[FORMAT_U15:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%p\0A\00"
 // CHECK-NEXT: 

[PATCH] D45665: [builtin-dump-struct] printf formats generation testing

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

In https://reviews.llvm.org/D45665#1069075, @aaron.ballman wrote:

> LGTM!


Nice to hear ! As for the last time, as I don't have any rights to push this, I 
would really appreciate if you could do it for me ! :)


Repository:
  rC Clang

https://reviews.llvm.org/D45665



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

[PATCH] D45665: [builtin-dump-struct] printf format checking

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 142548.
paulsemel added a comment.

Fixed typo..


Repository:
  rC Clang

https://reviews.llvm.org/D45665

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/dump-struct-builtin.c

Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -2,6 +2,98 @@
 
 #include "Inputs/stdio.h"
 
+// CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
+// CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00"
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [11 x i8] c"short a : \00"
+// CHECK-NEXT: [[FORMAT_U1:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hd\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
+// CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00"
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [20 x i8] c"unsigned short a : \00"
+// CHECK-NEXT: [[FORMAT_U2:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00"
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [9 x i8] c"int a : \00"
+// CHECK-NEXT: [[FORMAT_U3:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%d\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00"
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [18 x i8] c"unsigned int a : \00"
+// CHECK-NEXT: [[FORMAT_U4:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%u\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U5A {\0A\00"
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [10 x i8] c"long a : \00"
+// CHECK-NEXT: [[FORMAT_U5:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%ld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U6A {\0A\00"
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"unsigned long a : \00"
+// CHECK-NEXT: [[FORMAT_U6:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%lu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U7A {\0A\00"
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"long long a : \00"
+// CHECK-NEXT: [[FORMAT_U7:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%lld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit8.a = private unnamed_addr constant %struct.U8A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U8:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U8A {\0A\00"
+// CHECK-NEXT: [[FIELD_U8:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned long long a : \00"
+// CHECK-NEXT: [[FORMAT_U8:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%llu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U8:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit9.a = private unnamed_addr constant %struct.U9A { i8 97 }, align 1
+// CHECK-NEXT: [[STRUCT_STR_U9:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U9A {\0A\00"
+// CHECK-NEXT: [[FIELD_U9:@[0-9]+]] = private unnamed_addr constant [10 x i8] c"char a : \00"
+// CHECK-NEXT: [[FORMAT_U9:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%c\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U9:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @.str = private unnamed_addr constant [4 x i8] c"LSE\00", align 1
+
+// CHECK: @unit10.a = private unnamed_addr constant %struct.U10A { i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i32 0, i32 0) }, align 8
+// 

[PATCH] D45665: [builtin-dump-struct] printf format checking

[Paul Semel via Phabricator via cfe-commits]

paulsemel created this revision.
paulsemel added a reviewer: aaron.ballman.

Added check for correct formats in CodeGen tests, to ensure we are printing the 
fields correctly.
Added a fix that outcomed from this previous add


Repository:
  rC Clang

https://reviews.llvm.org/D45665

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/dump-struct-builtin.c

Index: test/CodeGen/dump-struct-builtin.c
===
--- test/CodeGen/dump-struct-builtin.c
+++ test/CodeGen/dump-struct-builtin.c
@@ -2,6 +2,98 @@
 
 #include "Inputs/stdio.h"
 
+// CHECK: @unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
+// CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00"
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [11 x i8] c"short a : \00"
+// CHECK-NEXT: [[FORMAT_U1:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hd\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
+// CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00"
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [20 x i8] c"unsigned short a : \00"
+// CHECK-NEXT: [[FORMAT_U2:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00"
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [9 x i8] c"int a : \00"
+// CHECK-NEXT: [[FORMAT_U3:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%d\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
+// CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00"
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [18 x i8] c"unsigned int a : \00"
+// CHECK-NEXT: [[FORMAT_U4:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%u\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U5A {\0A\00"
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [10 x i8] c"long a : \00"
+// CHECK-NEXT: [[FORMAT_U5:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%ld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U6A {\0A\00"
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"unsigned long a : \00"
+// CHECK-NEXT: [[FORMAT_U6:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%lu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U7A {\0A\00"
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"long long a : \00"
+// CHECK-NEXT: [[FORMAT_U7:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%lld\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit8.a = private unnamed_addr constant %struct.U8A { i64 12 }, align 8
+// CHECK-NEXT: [[STRUCT_STR_U8:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U8A {\0A\00"
+// CHECK-NEXT: [[FIELD_U8:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned long long a : \00"
+// CHECK-NEXT: [[FORMAT_U8:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%llu\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U8:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @unit9.a = private unnamed_addr constant %struct.U9A { i8 97 }, align 1
+// CHECK-NEXT: [[STRUCT_STR_U9:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U9A {\0A\00"
+// CHECK-NEXT: [[FIELD_U9:@[0-9]+]] = private unnamed_addr constant [10 x i8] c"char a : \00"
+// CHECK-NEXT: [[FORMAT_U9:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%c\0A\00"
+// CHECK-NEXT: [[END_STRUCT_U9:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00"
+
+// CHECK: @.str = private unnamed_addr constant [4 x i8] c"LSE\00", align 1
+
+// CHECK: @unit10.a = 

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

In https://reviews.llvm.org/D45615#1067068, @aaron.ballman wrote:

> In https://reviews.llvm.org/D45615#1066975, @paulsemel wrote:
>
> > In https://reviews.llvm.org/D45615#1066973, @lebedev.ri wrote:
> >
> > > Tests?
> >
> >
> > I can do this. But currently, I am not testing the formats in the tests..
>
>
> Now might be a good time to test that, because this patch almost added a bug 
> by missing the length modifiers. Also, all patches should come with some 
> tests to demonstrate the behavioral differences from the current trunk.


Sure. I will do another review for the existing format testing, and then go 
back to this one to finish the type handling !


Repository:
  rC Clang

https://reviews.llvm.org/D45615



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

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

In https://reviews.llvm.org/D45615#1066973, @lebedev.ri wrote:

> Tests?


I can do this. But currently, I am not testing the formats in the tests..


Repository:
  rC Clang

https://reviews.llvm.org/D45615



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

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

[Paul Semel via Phabricator via cfe-commits]

paulsemel created this revision.
paulsemel added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.

There was missing some basic types format (like uint8_t..). This patch is meant 
to print them the correct way.


Repository:
  rC Clang

https://reviews.llvm.org/D45615

Files:
  lib/CodeGen/CGBuiltin.cpp


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -949,6 +949,10 @@
   if (Types.empty()) {
 Types[Context.CharTy] = "%c";
 Types[Context.BoolTy] = "%d";
+Types[Context.SignedCharTy] = "%d";
+Types[Context.ShortTy] = "%d";
+Types[Context.UnsignedCharTy] = "%u";
+Types[Context.UnsignedShortTy] = "%u";
 Types[Context.IntTy] = "%d";
 Types[Context.UnsignedIntTy] = "%u";
 Types[Context.LongTy] = "%ld";


Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -949,6 +949,10 @@
   if (Types.empty()) {
 Types[Context.CharTy] = "%c";
 Types[Context.BoolTy] = "%d";
+Types[Context.SignedCharTy] = "%d";
+Types[Context.ShortTy] = "%d";
+Types[Context.UnsignedCharTy] = "%u";
+Types[Context.UnsignedShortTy] = "%u";
 Types[Context.IntTy] = "%d";
 Types[Context.UnsignedIntTy] = "%u";
 Types[Context.LongTy] = "%ld";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 141920.
paulsemel added a comment.

Added triple option to CodeGen test so that it matches with the correct 
architecture


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c
  test/Sema/builtin-dump-struct.c

Index: test/Sema/builtin-dump-struct.c
===
--- /dev/null
+++ test/Sema/builtin-dump-struct.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -fno-spell-checking -verify %s
+
+void invalid_uses() {
+  struct A {
+  };
+  struct A a;
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
+  int (*badfunc2)(int, ...);
+  void (*badfunc3)(const char *, ...);
+  int (*badfunc4)(char *, ...);
+  int (*badfunc5)(void);
+
+  __builtin_dump_struct(); // expected-error {{too few arguments to function call, expected 2, have 0}}
+  __builtin_dump_struct(1);// expected-error {{too few arguments to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, goodfunc); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc2); // expected-error {{passing 'int (*)(int, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(int, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc3); // expected-error {{passing 'void (*)(const char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('void (*)(const char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc4); // expected-error {{passing 'int (*)(char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc5); // expected-error {{passing 'int (*)(void)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(void)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(a, goodfunc);  // expected-error {{passing 'struct A' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('struct A' vs structure pointer)}}
+}
+
+void valid_uses() {
+  struct A {
+  };
+  union B {
+  };
+
+  int (*goodfunc)(const char *, ...);
+  int (*goodfunc2)();
+  struct A a;
+  union B b;
+
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc2);
+}
Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,391 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+int printf(const char *fmt, ...) {
+return 0;
+}
+
+void unit1() {
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit2() {
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit3() {
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

Thanks a lot for your help, updating the patch !


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

Ok, I found the problem. In fact the size of `long` is 4 bytes on your machine, 
but 8 bytes on mine.
This makes this `// CHECK: [[LOAD1:%[0-9]+]] = load i64, i64* [[RES1]],` fail.
Do you know a smart way to do it without dealing with type sizes ?


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 141913.
paulsemel added a comment.

Fixed printf warning generated in tests/CodeGen.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c
  test/Sema/builtin-dump-struct.c

Index: test/Sema/builtin-dump-struct.c
===
--- /dev/null
+++ test/Sema/builtin-dump-struct.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -fno-spell-checking -verify %s
+
+void invalid_uses() {
+  struct A {
+  };
+  struct A a;
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
+  int (*badfunc2)(int, ...);
+  void (*badfunc3)(const char *, ...);
+  int (*badfunc4)(char *, ...);
+  int (*badfunc5)(void);
+
+  __builtin_dump_struct(); // expected-error {{too few arguments to function call, expected 2, have 0}}
+  __builtin_dump_struct(1);// expected-error {{too few arguments to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, goodfunc); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc2); // expected-error {{passing 'int (*)(int, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(int, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc3); // expected-error {{passing 'void (*)(const char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('void (*)(const char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc4); // expected-error {{passing 'int (*)(char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc5); // expected-error {{passing 'int (*)(void)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(void)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(a, goodfunc);  // expected-error {{passing 'struct A' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('struct A' vs structure pointer)}}
+}
+
+void valid_uses() {
+  struct A {
+  };
+  union B {
+  };
+
+  int (*goodfunc)(const char *, ...);
+  int (*goodfunc2)();
+  struct A a;
+  union B b;
+
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc2);
+}
Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,391 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+int printf(const char *fmt, ...) {
+return 0;
+}
+
+void unit1() {
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit2() {
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit3() {
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: 

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

Sorry about it, I also have the warning on my machine, but not the error you 
get...
Those test are actually working on my different linux machines, that's really 
weird.
This one is actually really weird, because I could find manually the missing 
pattern in your output.. I just don't get what is happening.


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 141890.
paulsemel added a comment.

Patch rebased.
Minor fix for single quotes escaping.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c
  test/Sema/builtin-dump-struct.c

Index: test/Sema/builtin-dump-struct.c
===
--- /dev/null
+++ test/Sema/builtin-dump-struct.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -fno-spell-checking -verify %s
+
+void invalid_uses() {
+  struct A {
+  };
+  struct A a;
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
+  int (*badfunc2)(int, ...);
+  void (*badfunc3)(const char *, ...);
+  int (*badfunc4)(char *, ...);
+  int (*badfunc5)(void);
+
+  __builtin_dump_struct(); // expected-error {{too few arguments to function call, expected 2, have 0}}
+  __builtin_dump_struct(1);// expected-error {{too few arguments to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, goodfunc); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc2); // expected-error {{passing 'int (*)(int, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(int, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc3); // expected-error {{passing 'void (*)(const char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('void (*)(const char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc4); // expected-error {{passing 'int (*)(char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc5); // expected-error {{passing 'int (*)(void)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(void)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(a, goodfunc);  // expected-error {{passing 'struct A' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('struct A' vs structure pointer)}}
+}
+
+void valid_uses() {
+  struct A {
+  };
+  union B {
+  };
+
+  int (*goodfunc)(const char *, ...);
+  int (*goodfunc2)();
+  struct A a;
+  union B b;
+
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc2);
+}
Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,387 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+void unit1() {
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit2() {
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit3() {
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i32, i32* [[RES1]],
+  // CHECK: 

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

In https://reviews.llvm.org/D44093#1063348, @aaron.ballman wrote:

> In https://reviews.llvm.org/D44093#1063340, @paulsemel wrote:
>
> > I don't really know what's the procedure right now.. What should I do once 
> > you both accepted the patch ? :)
>
>
> You're good to commit the patch and address the concerns raised by 
> @arichardson in a follow-up patch. Do you need someone to commit on your 
> behalf?


Thanks! Yes please, I don't think I have the right accesses to do it myself


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

I don't really know what's the procedure right now.. What should I do once you 
both accepted the patch ? :)


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

In https://reviews.llvm.org/D44093#1063122, @arichardson wrote:

> I'm also often restricted to using printf for debugging so this looks really 
> useful!
>
> However, before committing this I feel like the test should also verify that 
> the format strings that are generated are sensible.
>
> Also what should happens when you have enum members in your struct or maybe 
> even C++ pointers to members?


So, for the moment, we are only handling basic types. That said, for the enum 
in C, we will print according to the type of the enum.
In the future versions, I really want to be able to print the name of the enum 
so that the output is more relevent.
Anyway, the rule I followed for the moment is : if I don't recognize the type, 
I print it as an address.




Comment at: test/Sema/builtin-dump-struct.c:42
+  __builtin_dump_struct(, goodfunc2);
+}

arichardson wrote:
> I think there should also be a test here that we get an error when the struct 
> contains bitfields instead of crashing/generating nonsense in CodeGen.
Do you really think that I should throw an error just because there is a 
bitfield ?
I was thinking about just accepting the fact that the bitfield outputs are not 
correct but permit the user to pretty print the remaining part of the structure.
What do you think ?


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 141762.
paulsemel added a comment.

Added a good test for the `int (*)()` function prototype, as we decided to 
accept it as a valid function for the dumper


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c
  test/Sema/builtin-dump-struct.c

Index: test/Sema/builtin-dump-struct.c
===
--- /dev/null
+++ test/Sema/builtin-dump-struct.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -fno-spell-checking -verify %s
+
+void invalid_uses() {
+  struct A {
+  };
+  struct A a;
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
+  int (*badfunc2)(int, ...);
+  void (*badfunc3)(const char *, ...);
+  int (*badfunc4)(char *, ...);
+  int (*badfunc5)(void);
+
+  __builtin_dump_struct(); // expected-error {{too few arguments to function call, expected 2, have 0}}
+  __builtin_dump_struct(1);// expected-error {{too few arguments to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, goodfunc); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc2); // expected-error {{passing 'int (*)(int, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(int, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc3); // expected-error {{passing 'void (*)(const char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('void (*)(const char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc4); // expected-error {{passing 'int (*)(char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc5); // expected-error {{passing 'int (*)(void)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(void)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(a, goodfunc);  // expected-error {{passing 'struct A' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('struct A' vs structure pointer)}}
+}
+
+void valid_uses() {
+  struct A {
+  };
+  union B {
+  };
+
+  int (*goodfunc)(const char *, ...);
+  int (*goodfunc2)();
+  struct A a;
+  union B b;
+
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc2);
+}
Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,387 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+void unit1() {
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit2() {
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit3() {
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added inline comments.



Comment at: test/Sema/builtin-dump-struct.c:8
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);

aaron.ballman wrote:
> paulsemel wrote:
> > aaron.ballman wrote:
> > > paulsemel wrote:
> > > > aaron.ballman wrote:
> > > > > Can you also add a test for: `int (*badfunc4)(char *, ...);` and `int 
> > > > > (*badfunc5)();`
> > > > Isn't `int (*func)()` is a valid prototype for a printf like function 
> > > > in C ?
> > > > I instead added `int (*func)(void)` to the test cases.
> > > > Isn't int (*func)() is a valid prototype for a printf like function in 
> > > > C ?
> > > 
> > > No, because it's missing the `const char *` as the mandatory first 
> > > parameter. Do you want that to be allowed and hope the callee has it 
> > > correct on their side, or do you want it to diagnose as not being a valid 
> > > function?
> > Actually, from a kernel developer perspective, I would say it's better to 
> > let the user do its stuff on his side, because kernel is full of trick !
> > But if you think I'd rather check whether we have `int (*)(const char *, 
> > ...)` at any time, we can go for it !
> Okay, if you think it'd be beneficial to allow a function without a 
> prototype, I'm okay with it. Can you make it an explicit "good" test case?
Sure :)


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 141745.
paulsemel marked an inline comment as done.
paulsemel added a comment.

Updated the amperstamp in the Sema test to be consistent with the remaining 
part of it.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c
  test/Sema/builtin-dump-struct.c

Index: test/Sema/builtin-dump-struct.c
===
--- /dev/null
+++ test/Sema/builtin-dump-struct.c
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -fno-spell-checking -verify %s
+
+void invalid_uses() {
+  struct A {
+  };
+  struct A a;
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
+  int (*badfunc2)(int, ...);
+  void (*badfunc3)(const char *, ...);
+  int (*badfunc4)(char *, ...);
+  int (*badfunc5)(void);
+
+  __builtin_dump_struct(); // expected-error {{too few arguments to function call, expected 2, have 0}}
+  __builtin_dump_struct(1);// expected-error {{too few arguments to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, goodfunc); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc2); // expected-error {{passing 'int (*)(int, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(int, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc3); // expected-error {{passing 'void (*)(const char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('void (*)(const char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc4); // expected-error {{passing 'int (*)(char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc5); // expected-error {{passing 'int (*)(void)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(void)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(a, goodfunc);  // expected-error {{passing 'struct A' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('struct A' vs structure pointer)}}
+}
+
+void valid_uses() {
+  struct A {
+  };
+  union B {
+  };
+
+  int (*goodfunc)(const char *, ...);
+  struct A a;
+  union B b;
+
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc);
+}
Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,387 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+void unit1() {
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit2() {
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit3() {
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i32, i32* 

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel marked an inline comment as done.
paulsemel added a comment.

No problem, thanks for getting back on this ! I was busy because of my midterms 
anyway :)




Comment at: test/Sema/builtin-dump-struct.c:8
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);

aaron.ballman wrote:
> paulsemel wrote:
> > aaron.ballman wrote:
> > > Can you also add a test for: `int (*badfunc4)(char *, ...);` and `int 
> > > (*badfunc5)();`
> > Isn't `int (*func)()` is a valid prototype for a printf like function in C ?
> > I instead added `int (*func)(void)` to the test cases.
> > Isn't int (*func)() is a valid prototype for a printf like function in C ?
> 
> No, because it's missing the `const char *` as the mandatory first parameter. 
> Do you want that to be allowed and hope the callee has it correct on their 
> side, or do you want it to diagnose as not being a valid function?
Actually, from a kernel developer perspective, I would say it's better to let 
the user do its stuff on his side, because kernel is full of trick !
But if you think I'd rather check whether we have `int (*)(const char *, ...)` 
at any time, we can go for it !



Comment at: test/Sema/builtin-dump-struct.c:17
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to 
parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 
2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, ); // expected-error {{passing 'void *' to 
parameter of incompatible type 'structure pointer type': type mismatch at 1st 
parameter ('void *' vs 'structure pointer type')}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int 
(*)(const char *)' to parameter of incompatible type 'int (*)(const char *, 
...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int 
(*)(const char *, ...)')}}

aaron.ballman wrote:
> paulsemel wrote:
> > aaron.ballman wrote:
> > > Why ``?
> > Yes, we already have a test like this anyway :)
> It was more a question of why the ampersand on the second argument -- I think 
> that can be dropped and it'll be consistent with the rest of the tests.
Sure, sorry, I missed this. Totally agree with you, I am going to make the 
changes.


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 140304.
paulsemel marked 3 inline comments as done.
paulsemel added a comment.

Added Aaron suggestions.
Fixed a bug when having nested anonymous unions and structures.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c
  test/Sema/builtin-dump-struct.c

Index: test/Sema/builtin-dump-struct.c
===
--- /dev/null
+++ test/Sema/builtin-dump-struct.c
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -fno-spell-checking -verify %s
+
+void invalid_uses() {
+  struct A {
+  };
+  struct A a;
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
+  int (*badfunc2)(int, ...);
+  void (*badfunc3)(const char *, ...);
+  int (*badfunc4)(char *, ...);
+  int (*badfunc5)(void);
+
+  __builtin_dump_struct(); // expected-error {{too few arguments to function call, expected 2, have 0}}
+  __builtin_dump_struct(1);// expected-error {{too few arguments to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, ); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc2); // expected-error {{passing 'int (*)(int, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(int, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc3); // expected-error {{passing 'void (*)(const char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('void (*)(const char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc4); // expected-error {{passing 'int (*)(char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc5); // expected-error {{passing 'int (*)(void)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(void)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(a, goodfunc);  // expected-error {{passing 'struct A' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('struct A' vs structure pointer)}}
+}
+
+void valid_uses() {
+  struct A {
+  };
+  union B {
+  };
+
+  int (*goodfunc)(const char *, ...);
+  struct A a;
+  union B b;
+
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc);
+}
Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,387 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+void unit1() {
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit2() {
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit3() {
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i32, i32* [[RES1]],
+  

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel marked 6 inline comments as done.
paulsemel added inline comments.



Comment at: test/Sema/builtin-dump-struct.c:8
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);

aaron.ballman wrote:
> Can you also add a test for: `int (*badfunc4)(char *, ...);` and `int 
> (*badfunc5)();`
Isn't `int (*func)()` is a valid prototype for a printf like function in C ?
I instead added `int (*func)(void)` to the test cases.



Comment at: test/Sema/builtin-dump-struct.c:15
+  __builtin_dump_struct(1);// expected-error {{too few arguments 
to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to 
parameter of incompatible type 'structure pointer type': type mismatch at 1st 
parameter ('int' vs 'structure pointer type')}}
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to 
parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 
2nd parameter ('int' vs 'int (*)(const char *, ...)')}}

aaron.ballman wrote:
> Hrm, the `'structure pointer type'` in this diagnostic is unfortunate because 
> it's being quoted as though it were a real type -- you could drop the single 
> quotes. If you think the resulting diagnostic reads too strangely, perhaps we 
> will have to go back to a custom diagnostic after all.
I think it will be better to just put something like `structure pointer`, so 
that we understand the type we are talking about.
But this diagnostic seems great, still, what do you think about sticking with 
it ?



Comment at: test/Sema/builtin-dump-struct.c:17
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to 
parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 
2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, ); // expected-error {{passing 'void *' to 
parameter of incompatible type 'structure pointer type': type mismatch at 1st 
parameter ('void *' vs 'structure pointer type')}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int 
(*)(const char *)' to parameter of incompatible type 'int (*)(const char *, 
...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int 
(*)(const char *, ...)')}}

aaron.ballman wrote:
> Why ``?
Yes, we already have a test like this anyway :)


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 138832.
paulsemel added a comment.

Applied Aaron's suggestions
Added Sema tests for the typechecking


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c
  test/Sema/builtin-dump-struct.c

Index: test/Sema/builtin-dump-struct.c
===
--- /dev/null
+++ test/Sema/builtin-dump-struct.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple i386-unknown-unknown -fsyntax-only -fno-spell-checking -verify %s
+
+void invalid_uses() {
+  struct A {
+  };
+  struct A a;
+  void *b;
+  int (*goodfunc)(const char *, ...);
+  int (*badfunc1)(const char *);
+  int (*badfunc2)(int, ...);
+  void (*badfunc3)(const char *, ...);
+
+  __builtin_dump_struct(); // expected-error {{too few arguments to function call, expected 2, have 0}}
+  __builtin_dump_struct(1);// expected-error {{too few arguments to function call, expected 2, have 1}}
+  __builtin_dump_struct(1, 2); // expected-error {{passing 'int' to parameter of incompatible type 'structure pointer type': type mismatch at 1st parameter ('int' vs 'structure pointer type')}}
+  __builtin_dump_struct(, 2);// expected-error {{passing 'int' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(b, ); // expected-error {{passing 'void *' to parameter of incompatible type 'structure pointer type': type mismatch at 1st parameter ('void *' vs 'structure pointer type')}}
+  __builtin_dump_struct(, badfunc1); // expected-error {{passing 'int (*)(const char *)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(const char *)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc2); // expected-error {{passing 'int (*)(int, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('int (*)(int, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(, badfunc3); // expected-error {{passing 'void (*)(const char *, ...)' to parameter of incompatible type 'int (*)(const char *, ...)': type mismatch at 2nd parameter ('void (*)(const char *, ...)' vs 'int (*)(const char *, ...)')}}
+  __builtin_dump_struct(a, goodfunc);  // expected-error {{passing 'struct A' to parameter of incompatible type 'structure pointer type': type mismatch at 1st parameter ('struct A' vs 'structure pointer type')}}
+}
+
+void valid_uses() {
+  struct A {
+  };
+  union B {
+  };
+
+  int (*goodfunc)(const char *, ...);
+  struct A a;
+  union B b;
+
+  __builtin_dump_struct(, goodfunc);
+  __builtin_dump_struct(, goodfunc);
+}
Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,254 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+void unit1() {
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit2() {
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit3() {
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i32, i32* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i32 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit4() {
+  struct U4A {
+unsigned int a;
+  };
+
+  struct U4A a = {
+  .a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U4A, %struct.U4A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i32, i32* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i32 [[LOAD1]])
+  // 

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 138687.
paulsemel added a comment.

Added some tests (unit tests for almost every types) and some other tests with 
tricky cases.

More tests are coming soon.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  test/CodeGen/dump-struct-builtin.c

Index: test/CodeGen/dump-struct-builtin.c
===
--- /dev/null
+++ test/CodeGen/dump-struct-builtin.c
@@ -0,0 +1,270 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/stdio.h"
+
+void unit1()
+{
+  struct U1A {
+short a;
+  };
+
+  struct U1A a = {
+.a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U1A, %struct.U1A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit2()
+{
+  struct U2A {
+unsigned short a;
+  };
+
+  struct U2A a = {
+.a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U2A, %struct.U2A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i16, i16* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i16 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit3()
+{
+  struct U3A {
+int a;
+  };
+
+  struct U3A a = {
+.a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U3A, %struct.U3A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i32, i32* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i32 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit4()
+{
+  struct U4A {
+unsigned int a;
+  };
+
+  struct U4A a = {
+.a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U4A, %struct.U4A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i32, i32* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i32 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit5()
+{
+  struct U5A {
+long a;
+  };
+
+  struct U5A a = {
+.a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U5A, %struct.U5A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i64, i64* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i64 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit6()
+{
+  struct U6A {
+unsigned long a;
+  };
+
+  struct U6A a = {
+.a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U6A, %struct.U6A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i64, i64* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i64 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit7()
+{
+  struct U7A {
+long long a;
+  };
+
+  struct U7A a = {
+.a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U7A, %struct.U7A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i64, i64* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i64 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit8()
+{
+  struct U8A {
+long long a;
+  };
+
+  struct U8A a = {
+.a = 12,
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U8A, %struct.U8A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i64, i64* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i64 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void unit9()
+{
+  struct U9A {
+char a;
+  };
+
+  struct U9A a = {
+.a = 'a',
+  };
+
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[RES1:%[0-9]+]] = getelementptr inbounds %struct.U9A, %struct.U9A* %a, i32 0, i32 0
+  // CHECK: call i32 (i8*, ...) @printf(
+  // CHECK: [[LOAD1:%[0-9]+]] = load i8, i8* [[RES1]],
+  // CHECK: call i32 (i8*, ...) @printf({{.*}}, i8 [[LOAD1]])
+  // CHECK: call i32 (i8*, ...) @printf(
+  __builtin_dump_struct(, );
+}
+
+void 

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

Hi,

In https://reviews.llvm.org/D44093#1034610, @lebedev.ri wrote:

> BTW, as far as i can tell this still has zero test coverage (no new tests are 
> being added).
>  I'd expect to see the tests for the actual output
>
> - one struct per each type it is able to print
> - probably some tests showing error handling, and possibly the availability 
> of the builtin is somehow tested, too?


Sure, I am going to work on it, now that the patch seems to be kind of "Okay" 
for its first version !

Thanks !


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 137998.
paulsemel added a comment.

Applied Francis' suggestions


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp

Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1110,6 +1110,65 @@
 // so ensure that they are declared.
 DeclareGlobalNewDelete();
 break;
+  case Builtin::BI__builtin_dump_struct: {
+// We first want to ensure we are called with 2 arguments
+if (checkArgCount(*this, TheCall, 2))
+  return ExprError();
+// Ensure that the first argument is of type 'struct XX *'
+const Expr *PtrArg = TheCall->getArg(0)->IgnoreParenImpCasts();
+const QualType PtrArgType = PtrArg->getType();
+if (!PtrArgType->isPointerType()) {
+  this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< PtrArgType << "\'structure pointer type\'"
+<< 1 << 0 << 3 << 1
+<< PtrArgType << "\'structure pointer type\'";
+  return ExprError();
+}
+
+const RecordType *RT = PtrArgType->getPointeeType()->getAs();
+if (!RT) {
+  this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< PtrArgType << "\'structure pointer type\'"
+<< 1 << 0 << 3 << 1
+<< PtrArgType << "\'structure pointer type\'";
+  return ExprError();
+}
+// Ensure that the second argument is of type 'FunctionType'
+const Expr *FnPtrArg = TheCall->getArg(1)->IgnoreImpCasts();
+const QualType FnPtrArgType = FnPtrArg->getType();
+if (!FnPtrArgType->isPointerType()) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+  return ExprError();
+}
+
+const FunctionType *FuncType =
+  FnPtrArgType->getPointeeType()->getAs();
+
+if (!FuncType) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+  return ExprError();
+}
+
+if (const FunctionProtoType *FT = dyn_cast(FuncType)) {
+  if (!FT->isVariadic() ||
+  FT->getReturnType() != Context.IntTy) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType<< "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+return ExprError();
+  }
+}
+
+TheCall->setType(Context.IntTy);
+break;
+  }
 
   // check secure string manipulation functions where overflows
   // are detectable at compile time
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -14,6 +14,7 @@
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
 #include "CGOpenCLRuntime.h"
+#include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
@@ -930,6 +931,90 @@
   return RValue::get(Overflow);
 }
 
+static llvm::Value *dumpRecord(CodeGenFunction , QualType RType,
+ Value*& RecordPtr, CharUnits Align,
+ Value *Func, int Lvl)
+{
+  const RecordType *RT = RType->getAs();
+  ASTContext& Context = CGF.getContext();
+  RecordDecl *RD = RT->getDecl()->getDefinition();
+  ASTContext& Ctx = RD->getASTContext();
+  const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+  std::string Pad = std::string(Lvl * 4, ' ');
+
+  Value *GString = CGF.Builder.CreateGlobalStringPtr(RType.getAsString()
+ + " {\n");
+  Value *Res = CGF.Builder.CreateCall(Func, {GString});
+
+  static llvm::DenseMap Types;
+  if (Types.empty()) {
+Types[Context.CharTy] = "%c";
+Types[Context.BoolTy] = "%d";
+Types[Context.IntTy] = "%d";
+Types[Context.UnsignedIntTy] = "%u";
+Types[Context.LongTy] = "%ld";
+Types[Context.UnsignedLongTy] = "%lu";
+Types[Context.LongLongTy] = "%lld";
+Types[Context.UnsignedLongLongTy] = "%llu";
+Types[Context.ShortTy] = "%hd";
+Types[Context.UnsignedShortTy] = "%hu";
+Types[Context.VoidPtrTy] = "%p";
+Types[Context.FloatTy] = "%f";
+Types[Context.DoubleTy] = "%f";
+Types[Context.LongDoubleTy] = "%Lf";
+Types[Context.getPointerType(Context.CharTy)] = "%s";
+  }
+
+  for (const auto *FD : RD->fields()) {
+uint64_t Off = RL.getFieldOffset(FD->getFieldIndex());
+Off = Ctx.toCharUnitsFromBits(Off).getQuantity();
+
+Value *FieldPtr = RecordPtr;
+

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 137775.
paulsemel added a comment.

Added recursive type pretty-printing as suggested by Aaron.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp

Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1110,6 +1110,66 @@
 // so ensure that they are declared.
 DeclareGlobalNewDelete();
 break;
+  case Builtin::BI__builtin_dump_struct: {
+// We first want to ensure we are called with 2 arguments
+if (checkArgCount(*this, TheCall, 2))
+  return ExprError();
+// Ensure that the first argument is of type 'struct XX *'
+const Expr *PtrArg = TheCall->getArg(0)->IgnoreParenImpCasts();
+const QualType PtrArgType = PtrArg->getType();
+if (!PtrArgType->isPointerType()) {
+  this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< PtrArgType << "\'structure pointer type\'"
+<< 1 << 0 << 3 << 1
+<< PtrArgType << "\'structure pointer type\'";
+  return ExprError();
+}
+
+const RecordType *RT = PtrArgType->getPointeeType()->getAs();
+if (!RT) {
+  this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< PtrArgType << "\'structure pointer type\'"
+<< 1 << 0 << 3 << 1
+<< PtrArgType << "\'structure pointer type\'";
+  return ExprError();
+}
+// Ensure that the second argument is of type 'FunctionType'
+const Expr *FnPtrArg = TheCall->getArg(1)->IgnoreImpCasts();
+const QualType FnPtrArgType = FnPtrArg->getType();
+if (!FnPtrArgType->isPointerType()) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+  return ExprError();
+}
+
+const FunctionType *FuncType =
+  FnPtrArgType->getPointeeType()->getAs();
+
+if (!FuncType) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+  return ExprError();
+}
+
+const FunctionProtoType *FT = dyn_cast(FuncType);
+if (FT) {
+  if (!FT->isVariadic() ||
+  FT->getReturnType() != Context.IntTy) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType<< "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+return ExprError();
+  }
+}
+
+TheCall->setType(Context.IntTy);
+break;
+  }
 
   // check secure string manipulation functions where overflows
   // are detectable at compile time
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -14,6 +14,7 @@
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
 #include "CGOpenCLRuntime.h"
+#include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
@@ -930,6 +931,93 @@
   return RValue::get(Overflow);
 }
 
+static Value *dumpRecord(CodeGenFunction , QualType RType,
+ Value*& RecordPtr, CharUnits Align,
+ Value *Func, int Lvl)
+{
+  const RecordType *RT = RType->getAs();
+  ASTContext& Context = CGF.getContext();
+  RecordDecl *RD = RT->getDecl()->getDefinition();
+  ASTContext& Ctx = RD->getASTContext();
+  const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+  std::string Pad = std::string(Lvl * 4, ' ');
+
+  Value *GString = CGF.Builder.CreateGlobalStringPtr(RType.getAsString()
+ + " {\n");
+  Value *Res = CGF.Builder.CreateCall(Func, {GString});
+
+  static llvm::DenseMap Types;
+  if (Types.empty()) {
+Types[Context.CharTy] = "%c";
+Types[Context.BoolTy] = "%d";
+Types[Context.IntTy] = "%d";
+Types[Context.UnsignedIntTy] = "%u";
+Types[Context.LongTy] = "%ld";
+Types[Context.UnsignedLongTy] = "%lu";
+Types[Context.LongLongTy] = "%lld";
+Types[Context.UnsignedLongLongTy] = "%llu";
+Types[Context.ShortTy] = "%hd";
+Types[Context.UnsignedShortTy] = "%hu";
+Types[Context.VoidPtrTy] = "%p";
+Types[Context.FloatTy] = "%f";
+Types[Context.DoubleTy] = "%f";
+Types[Context.LongDoubleTy] = "%Lf";
+Types[Context.getPointerType(Context.CharTy)] = "%s";
+}
+
+  for (const auto *FD : RD->fields()) {
+uint64_t Off = RL.getFieldOffset(FD->getFieldIndex());
+Off = Ctx.toCharUnitsFromBits(Off).getQuantity();
+
+

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:1252
+  Types[getContext().getPointerType(getContext().CharTy)] = "%s";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().CharTy, "%s")
+}

aaron.ballman wrote:
> paulsemel wrote:
> > aaron.ballman wrote:
> > > What about other types that have format specifiers (ptrdiff_t, size_t, 
> > > intptr_t, char16_t, etc)?
> > So, for typedef, why not apply the `QualType::getCanonicalType` to our 
> > field type, so that we try to get rid of those intermediate typedefs ?
> It should be possible to do for type aliases, because you know the canonical 
> type information. However, that won't work for builtin types that aren't a 
> typedef like `char16_t`.
Sure, but in this case, the only soluntion is to determine how we want to print 
those builtins and add those and the static map


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 137578.
paulsemel marked 3 inline comments as done.
paulsemel added a comment.

Applied Aaron suggestions


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp

Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1110,6 +1110,66 @@
 // so ensure that they are declared.
 DeclareGlobalNewDelete();
 break;
+  case Builtin::BI__builtin_dump_struct: {
+// We first want to ensure we are called with 2 arguments
+if (checkArgCount(*this, TheCall, 2))
+  return ExprError();
+// Ensure that the first argument is of type 'struct XX *'
+const Expr *PtrArg = TheCall->getArg(0)->IgnoreParenImpCasts();
+const QualType PtrArgType = PtrArg->getType();
+if (!PtrArgType->isPointerType()) {
+  this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< PtrArgType << "\'structure pointer type\'"
+<< 1 << 0 << 3 << 1
+<< PtrArgType << "\'structure pointer type\'";
+  return ExprError();
+}
+
+const RecordType *RT = PtrArgType->getPointeeType()->getAs();
+if (!RT) {
+  this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< PtrArgType << "\'structure pointer type\'"
+<< 1 << 0 << 3 << 1
+<< PtrArgType << "\'structure pointer type\'";
+  return ExprError();
+}
+// Ensure that the second argument is of type 'FunctionType'
+const Expr *FnPtrArg = TheCall->getArg(1)->IgnoreImpCasts();
+const QualType FnPtrArgType = FnPtrArg->getType();
+if (!FnPtrArgType->isPointerType()) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+  return ExprError();
+}
+
+const FunctionType *FuncType =
+  FnPtrArgType->getPointeeType()->getAs();
+
+if (!FuncType) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+  return ExprError();
+}
+
+const FunctionProtoType *FT = dyn_cast(FuncType);
+if (FT) {
+  if (!FT->isVariadic() ||
+  FT->getReturnType() != Context.IntTy) {
+  this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType<< "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+return ExprError();
+  }
+}
+
+TheCall->setType(Context.IntTy);
+break;
+  }
 
   // check secure string manipulation functions where overflows
   // are detectable at compile time
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -14,6 +14,7 @@
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
 #include "CGOpenCLRuntime.h"
+#include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
@@ -1196,6 +1197,84 @@
 return RValue::get(ComplexVal.first);
   }
 
+  case Builtin::BI__builtin_dump_struct: {
+Value *Func = EmitScalarExpr(E->getArg(1)->IgnoreImpCasts());
+CharUnits Arg0Align = EmitPointerWithAlignment(E->getArg(0)).getAlignment();
+
+const Expr *Arg0 = E->getArg(0)->IgnoreImpCasts();
+QualType Arg0Type = Arg0->getType()->getPointeeType();
+const RecordType *RT = Arg0Type->getAs();
+
+RecordDecl *RD = RT->getDecl()->getDefinition();
+ASTContext  = RD->getASTContext();
+const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+
+Value *GString = Builder.CreateGlobalStringPtr(Arg0Type.getAsString()
+   + " {\n");
+Value *Res = Builder.CreateCall(Func, {GString});
+
+static llvm::DenseMap Types;
+if (Types.empty()) {
+  Types[getContext().CharTy] = "%c";
+  Types[getContext().BoolTy] = "%d";
+  Types[getContext().IntTy] = "%d";
+  Types[getContext().UnsignedIntTy] = "%u";
+  Types[getContext().LongTy] = "%ld";
+  Types[getContext().UnsignedLongTy] = "%lu";
+  Types[getContext().LongLongTy] = "%lld";
+  Types[getContext().UnsignedLongLongTy] = "%llu";
+  Types[getContext().ShortTy] = "%hd";
+  Types[getContext().UnsignedShortTy] = "%hu";
+  Types[getContext().VoidPtrTy] = "%p";
+  Types[getContext().FloatTy] = "%f";
+  Types[getContext().DoubleTy] = "%f";
+  Types[getContext().LongDoubleTy] = "%Lf";
+  

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel marked 12 inline comments as done.
paulsemel added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:1231
+  Types[getContext().VoidPtrTy] = "%p";
+  Types[getContext().FloatTy] = "%f";
+  Types[getContext().DoubleTy] = "%f";

aaron.ballman wrote:
> paulsemel wrote:
> > aaron.ballman wrote:
> > > It's unfortunate that you cannot distinguish between `float` and 
> > > `double`. Do you need to use the format specifiers exactly?
> > > 
> > > What about other type information, like qualifiers or array extents? How 
> > > should this handle types that do not exist in this mapping, like 
> > > structure or enum types?
> > So, I've think about it. What I am going to do is  that if I do not know 
> > the type of the field, I am just going to print it as a pointer. I know 
> > that it is not the best solution, but I think it's a okay-ish solution 
> > until I implement the other types.
> > For the qualifiers, at it is printed the same way with and without those, I 
> > can just add the entries in the DenseMap.
> > So, I've think about it. What I am going to do is that if I do not know the 
> > type of the field, I am just going to print it as a pointer. I know that it 
> > is not the best solution, but I think it's a okay-ish solution until I 
> > implement the other types.
> 
> Eek. That seems unfortunate. I'm thinking about very common use cases, like:
> ```
> struct S {
>   int i, j;
>   float x, y;
> };
> 
> struct T {
>   struct S s;
>   int k;
> };
> ```
> Printing out `s` as a pointer seems... not particularly useful.
Yes, I see that this is true for other types that I am not handling for the 
moment.. What do you think is the best behavior for those cases ?
Just not print anything and go to the next entry ?



Comment at: lib/CodeGen/CGBuiltin.cpp:1218
+Types[getContext().getConstType(type)] = format; \
+Types[getContext().getVolatileType(type)] = format; \
+Types[getContext().getConstType(getContext().getVolatileType(type))] = 
format;

aaron.ballman wrote:
> This seems insufficient, as there are other qualifiers (restrict, ObjC GC 
> qualifiers, etc). I think a better way to do this is to call 
> `QualType::getUnqualifiedType()` on the type accessing the map.
Yes, I think you're totally right !



Comment at: lib/CodeGen/CGBuiltin.cpp:1252
+  Types[getContext().getPointerType(getContext().CharTy)] = "%s";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().CharTy, "%s")
+}

aaron.ballman wrote:
> What about other types that have format specifiers (ptrdiff_t, size_t, 
> intptr_t, char16_t, etc)?
So, for typedef, why not apply the `QualType::getCanonicalType` to our field 
type, so that we try to get rid of those intermediate typedefs ?



Comment at: lib/Sema/SemaChecking.cpp:1121
+  this->Diag(Arg0->getLocStart(), 
diag::err_dump_struct_invalid_argument_type)
+<< Arg0->getType() << "structure pointer type";
+  return ExprError();

aaron.ballman wrote:
> The string literals should be part of a `%select` in the diagnostic itself 
> rather than printed this way.
So, I am now using an other diagnostic definition, so I am constrained to keep 
using string literals I think..



Comment at: lib/Sema/SemaChecking.cpp:1135
+  this->Diag(Arg1->getLocStart(), 
diag::err_dump_struct_invalid_argument_type)
+<< Arg1->getType() << "printf like function pointer type";
+  return ExprError();

aaron.ballman wrote:
> What is a "printf like function pointer type"?
I have changed this to the actual prototype of a "printf like" function so that 
it's way clearer of what I am expecting !


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 137371.
paulsemel added a comment.

Updated with more context.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp

Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1110,6 +1110,53 @@
 // so ensure that they are declared.
 DeclareGlobalNewDelete();
 break;
+  case Builtin::BI__builtin_dump_struct: {
+// We check for argument number
+if (checkArgCount(*this, TheCall, 2))
+  return ExprError();
+// Ensure that the first argument is of type 'struct XX *'
+const Expr *Arg0 = TheCall->getArg(0)->IgnoreImpCasts();
+if (!Arg0->getType()->isPointerType()) {
+  this->Diag(Arg0->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+<< Arg0->getType() << "structure pointer type";
+  return ExprError();
+}
+QualType Arg0Type = Arg0->getType()->getPointeeType();
+const RecordType *RT = Arg0Type->getAs();
+if (!RT) {
+  this->Diag(Arg0->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+<< Arg0->getType() << "structure pointer type";
+  return ExprError();
+}
+// Ensure that the second argument is of type 'FunctionType'
+const Expr *Arg1 = TheCall->getArg(1)->IgnoreImpCasts();
+if (!Arg1->getType()->isPointerType()) {
+  this->Diag(Arg1->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+<< Arg1->getType() << "printf like function pointer type";
+  return ExprError();
+}
+
+const FunctionType *FuncType = Arg1->getType()->getPointeeType()->getAs();
+
+if (!FuncType) {
+  this->Diag(Arg1->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+<< Arg1->getType() << "printf like function pointer type";
+  return ExprError();
+}
+
+const FunctionProtoType *FT = dyn_cast(FuncType);
+if (FT) {
+  if (!FT->isVariadic()) {
+this->Diag(Arg1->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+  << Arg1->getType() << "printf like function pointer type";
+return ExprError();
+  }
+}
+
+
+TheCall->setType(Context.IntTy);
+break;
+  }
 
   // check secure string manipulation functions where overflows
   // are detectable at compile time
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -14,6 +14,7 @@
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
 #include "CGOpenCLRuntime.h"
+#include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
@@ -1196,6 +1197,103 @@
 return RValue::get(ComplexVal.first);
   }
 
+  case Builtin::BI__builtin_dump_struct: {
+Value *Func = EmitScalarExpr(E->getArg(1)->IgnoreImpCasts());
+CharUnits Arg0Align = EmitPointerWithAlignment(E->getArg(0)).getAlignment();
+
+const Expr *Arg0 = E->getArg(0)->IgnoreImpCasts();
+QualType Arg0Type = Arg0->getType()->getPointeeType();
+const RecordType *RT = Arg0Type->getAs();
+
+RecordDecl *RD = RT->getDecl()->getDefinition();
+ASTContext  = RD->getASTContext();
+const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+
+Value *GString = Builder.CreateGlobalStringPtr(Arg0Type.getAsString()
+   + " {\n");
+Value *Res = Builder.CreateCall(Func, {GString});
+
+#define GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(type, format) \
+Types[getContext().getConstType(type)] = format; \
+Types[getContext().getVolatileType(type)] = format; \
+Types[getContext().getConstType(getContext().getVolatileType(type))] = format;
+
+static llvm::DenseMap Types;
+if (Types.empty()) {
+  Types[getContext().CharTy] = "%c";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().CharTy, "%c")
+  Types[getContext().BoolTy] = "%d";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().BoolTy, "%d")
+  Types[getContext().IntTy] = "%d";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().IntTy, "%d")
+  Types[getContext().UnsignedIntTy] = "%u";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().UnsignedIntTy, "%u")
+  Types[getContext().LongTy] = "%ld";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().LongTy, "%ld")
+  Types[getContext().UnsignedLongTy] = "%lu";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().UnsignedLongTy, "%lu")
+  Types[getContext().LongLongTy] = "%lld";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().LongLongTy, "%lld")
+  Types[getContext().UnsignedLongLongTy] = "%llu";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().UnsignedLongLongTy, "%llu")
+  

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 137369.
paulsemel added a comment.

Applied Aaron suggestion changes.
Added parameters checking in Sema.


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp

Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1110,6 +1110,53 @@
 // so ensure that they are declared.
 DeclareGlobalNewDelete();
 break;
+  case Builtin::BI__builtin_dump_struct: {
+// We check for argument number
+if (checkArgCount(*this, TheCall, 2))
+  return ExprError();
+// Ensure that the first argument is of type 'struct XX *'
+const Expr *Arg0 = TheCall->getArg(0)->IgnoreImpCasts();
+if (!Arg0->getType()->isPointerType()) {
+  this->Diag(Arg0->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+<< Arg0->getType() << "structure pointer type";
+  return ExprError();
+}
+QualType Arg0Type = Arg0->getType()->getPointeeType();
+const RecordType *RT = Arg0Type->getAs();
+if (!RT) {
+  this->Diag(Arg0->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+<< Arg0->getType() << "structure pointer type";
+  return ExprError();
+}
+// Ensure that the second argument is of type 'FunctionType'
+const Expr *Arg1 = TheCall->getArg(1)->IgnoreImpCasts();
+if (!Arg1->getType()->isPointerType()) {
+  this->Diag(Arg1->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+<< Arg1->getType() << "printf like function pointer type";
+  return ExprError();
+}
+
+const FunctionType *FuncType = Arg1->getType()->getPointeeType()->getAs();
+
+if (!FuncType) {
+  this->Diag(Arg1->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+<< Arg1->getType() << "printf like function pointer type";
+  return ExprError();
+}
+
+const FunctionProtoType *FT = dyn_cast(FuncType);
+if (FT) {
+  if (!FT->isVariadic()) {
+this->Diag(Arg1->getLocStart(), diag::err_dump_struct_invalid_argument_type)
+  << Arg1->getType() << "printf like function pointer type";
+return ExprError();
+  }
+}
+
+
+TheCall->setType(Context.IntTy);
+break;
+  }
 
   // check secure string manipulation functions where overflows
   // are detectable at compile time
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -14,6 +14,7 @@
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
 #include "CGOpenCLRuntime.h"
+#include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
@@ -1196,6 +1197,103 @@
 return RValue::get(ComplexVal.first);
   }
 
+  case Builtin::BI__builtin_dump_struct: {
+Value *Func = EmitScalarExpr(E->getArg(1)->IgnoreImpCasts());
+CharUnits Arg0Align = EmitPointerWithAlignment(E->getArg(0)).getAlignment();
+
+const Expr *Arg0 = E->getArg(0)->IgnoreImpCasts();
+QualType Arg0Type = Arg0->getType()->getPointeeType();
+const RecordType *RT = Arg0Type->getAs();
+
+RecordDecl *RD = RT->getDecl()->getDefinition();
+ASTContext  = RD->getASTContext();
+const ASTRecordLayout  = Ctx.getASTRecordLayout(RD);
+
+Value *GString = Builder.CreateGlobalStringPtr(Arg0Type.getAsString()
+   + " {\n");
+Value *Res = Builder.CreateCall(Func, {GString});
+
+#define GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(type, format) \
+Types[getContext().getConstType(type)] = format; \
+Types[getContext().getVolatileType(type)] = format; \
+Types[getContext().getConstType(getContext().getVolatileType(type))] = format;
+
+static llvm::DenseMap Types;
+if (Types.empty()) {
+  Types[getContext().CharTy] = "%c";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().CharTy, "%c")
+  Types[getContext().BoolTy] = "%d";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().BoolTy, "%d")
+  Types[getContext().IntTy] = "%d";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().IntTy, "%d")
+  Types[getContext().UnsignedIntTy] = "%u";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().UnsignedIntTy, "%u")
+  Types[getContext().LongTy] = "%ld";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().LongTy, "%ld")
+  Types[getContext().UnsignedLongTy] = "%lu";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().UnsignedLongTy, "%lu")
+  Types[getContext().LongLongTy] = "%lld";
+  GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().LongLongTy, "%lld")
+  Types[getContext().UnsignedLongLongTy] = "%llu";
+  

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel marked 2 inline comments as done.
paulsemel added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:1206
+QualType Arg0Type = Arg0->getType()->getPointeeType();
+const RecordType *RT = Arg0Type->getAs();
+

aaron.ballman wrote:
> You can use `const auto *` here because the type is spelled out in the 
> initializer.
After thinking about it, I find it clearer to spell the full type when possible 
(that's also probably because I'm used to code in C :) )



Comment at: lib/CodeGen/CGBuiltin.cpp:1231
+  Types[getContext().VoidPtrTy] = "%p";
+  Types[getContext().FloatTy] = "%f";
+  Types[getContext().DoubleTy] = "%f";

aaron.ballman wrote:
> It's unfortunate that you cannot distinguish between `float` and `double`. Do 
> you need to use the format specifiers exactly?
> 
> What about other type information, like qualifiers or array extents? How 
> should this handle types that do not exist in this mapping, like structure or 
> enum types?
So, I've think about it. What I am going to do is  that if I do not know the 
type of the field, I am just going to print it as a pointer. I know that it is 
not the best solution, but I think it's a okay-ish solution until I implement 
the other types.
For the qualifiers, at it is printed the same way with and without those, I can 
just add the entries in the DenseMap.


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44154: [checker] PoC : Unsequenced Modification Checker

[Paul Semel via Phabricator via cfe-commits]

paulsemel added a comment.

Hi Aleksei,

In https://reviews.llvm.org/D44154#1028612, @a.sidorin wrote:

> Hi Paul,
>
> You didn't add any reviewer. Do you need a review for this checker? Are you 
> going to contribute this code to the upstream?


I was actually uploading this because Artem Dergachev told me that he was 
curious about this checker, but I'm pretty sure that this implementation is not 
good enough to be upstreamed !


Repository:
  rC Clang

https://reviews.llvm.org/D44154



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

[PATCH] D44154: [checker] PoC : Unsequenced Modification Checker

[Paul Semel via Phabricator via cfe-commits]

paulsemel created this revision.
Herald added subscribers: cfe-commits, mgorny.

\!/ This is only a proof of concept !

The purpose of this checker is to improve the detection of unsequenced 
modifications. Indeed, there is a warning the tries to detect those, but this 
is not efficient at all.
The goal of this checker is to detect more complicated UM.


Repository:
  rC Clang

https://reviews.llvm.org/D44154

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/UnsequencedModificationChecker.cpp

Index: lib/StaticAnalyzer/Checkers/UnsequencedModificationChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UnsequencedModificationChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UnsequencedModificationChecker.cpp
@@ -0,0 +1,450 @@
+//=== UnsequencedModificationChecker.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This defines UnsequencedModificationChecker, a builtin check in ExprEngine
+// that performs checks for unsequenced modifications.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+  enum UBType
+  {
+UB_VAR,
+UB_ARRAY,
+UB_POINTER,
+UB_POINTERINDEX
+  };
+
+  class UBDecl
+  {
+public:
+  UBDecl(const VarDecl *vd)
+  {
+this->idx = SVal();
+this->type = UB_VAR;
+this->vd = vd;
+  };
+  UBDecl(const VarDecl *vd, enum UBType t)
+: vd(vd), type(t)
+  {
+this->idx = SVal();
+  }
+  UBDecl(const VarDecl *vd, SVal index)
+  {
+this->type = UB_ARRAY;
+this->vd = vd;
+this->idx = index;
+  };
+  void changeType(enum UBType t)
+  {
+this->type = t;
+  };
+  bool operator==(const UBDecl& ub)
+  {
+if (this->type != ub.type)
+  return false;
+switch(this->type)
+{
+  case UB_VAR:
+  case UB_POINTER:
+return this->vd == ub.vd;
+  case UB_ARRAY:
+  case UB_POINTERINDEX:
+return this->vd == ub.vd && this->idx == ub.idx;
+};
+return false;
+  };
+  const VarDecl *vd;
+  SVal idx;
+  enum UBType type;
+private:
+  };
+
+  enum UBAction
+  {
+UBUSED = 1,
+UBMODIFIED = 2,
+UBMULTIPLE = 4,
+  };
+
+  class UBListElement
+  {
+public:
+  UBListElement(std::shared_ptr& ub, enum UBAction action)
+:ub(ub), action(action)
+  { };
+  UBListElement(std::shared_ptr& ub, uint32_t action)
+:ub(ub), action(action)
+  { };
+  bool operator==(const UBDecl& elt)
+  {
+return *ub == elt;
+  };
+  std::shared_ptr ub;
+  uint32_t action;
+  };
+
+  class UBList
+  {
+public:
+  UBList() = default;
+  void push(std::shared_ptr ub, enum UBAction action)
+  {
+auto it = std::find(list.begin(), list.end(), *ub);
+if (it != list.end())
+{
+  if (it->action == action && action == UBMODIFIED)
+it->action = UBMULTIPLE;
+  else
+it->action |= action;
+}
+else
+  list.push_back(UBListElement(ub, action));
+  };
+  void pushCallExpr(std::shared_ptr ub, uint32_t action)
+  {
+auto it = std::find(list.begin(), list.end(), *ub);
+if (it != list.end())
+{
+  if (action != it->action)
+it->action = UBMODIFIED;
+}
+else
+  list.push_back(UBListElement(ub, action));
+  }
+  void push(std::shared_ptr ub, uint32_t action)
+  {
+auto it = std::find(list.begin(), list.end(), *ub);
+if (it != list.end())
+{
+  if (it->action == action && action == UBMODIFIED)
+it->action = UBMULTIPLE;
+  else
+it->action |= action;
+}
+else
+  list.push_back(UBListElement(ub, action));
+  };
+  size_t size()
+  {
+return list.size();
+  };
+  std::shared_ptr find_UB()
+  {
+for (auto  : list)
+{
+  if (elt.action == UBMULTIPLE || elt.action == 

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel updated this revision to Diff 137141.
paulsemel added a comment.

Applied Aaron change suggestions


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp

Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -14,6 +14,7 @@
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
 #include "CGOpenCLRuntime.h"
+#include "CGRecordLayout.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
@@ -1196,6 +1197,80 @@
 return RValue::get(ComplexVal.first);
   }
 
+  case Builtin::BI__builtin_dump_struct: {
+Value *Func = EmitScalarExpr(E->getArg(1)->IgnoreImpCasts());
+CharUnits Arg0Align = EmitPointerWithAlignment(E->getArg(0)).getAlignment();
+
+const Expr *Arg0 = E->getArg(0)->IgnoreImpCasts();
+QualType Arg0Type = Arg0->getType()->getPointeeType();
+const RecordType *RT = Arg0Type->getAs();
+
+assert(RT && "The first argument must be a record type");
+
+RecordDecl *RD = RT->getDecl()->getDefinition();
+ASTContext  = RD->getASTContext();
+const ASTRecordLayout  = ASTContext.getASTRecordLayout(RD);
+
+Value *GString = Builder.CreateGlobalStringPtr(Arg0Type.getAsString()
+   + " {\n");
+Value *Res = Builder.CreateCall(Func, {GString});
+
+static llvm::DenseMap Types;
+if (Types.empty()) {
+  Types[getContext().CharTy] = "%c";
+  Types[getContext().BoolTy] = "%d";
+  Types[getContext().IntTy] = "%d";
+  Types[getContext().UnsignedIntTy] = "%u";
+  Types[getContext().LongTy] = "%ld";
+  Types[getContext().UnsignedLongTy] = "%lu";
+  Types[getContext().LongLongTy] = "%lld";
+  Types[getContext().UnsignedLongLongTy] = "%llu";
+  Types[getContext().ShortTy] = "%hd";
+  Types[getContext().UnsignedShortTy] = "%hu";
+  Types[getContext().VoidPtrTy] = "%p";
+  Types[getContext().FloatTy] = "%f";
+  Types[getContext().DoubleTy] = "%f";
+  Types[getContext().LongDoubleTy] = "%Lf";
+  Types[getContext().getPointerType(getContext().CharTy)] = "%s";
+}
+
+/* field : RecordDecl::field_iterator */
+for (auto *FD : RD->fields()) {
+  uint64_t Off = RL.getFieldOffset(FD->getFieldIndex());
+  Off = ASTContext.toCharUnitsFromBits(Off).getQuantity();
+
+  Value *FieldPtr = EmitScalarExpr(E->getArg(0));
+  if (Off) {
+FieldPtr = Builder.CreatePtrToInt(FieldPtr, IntPtrTy);
+FieldPtr = Builder.CreateAdd(FieldPtr, ConstantInt::get(IntPtrTy, Off));
+FieldPtr = Builder.CreateIntToPtr(FieldPtr, VoidPtrTy);
+  }
+  std::string Format = FD->getType().getAsString() + std::string(" ") +
+FD->getNameAsString() + " : ";
+
+  Format += Types[FD->getType()];
+
+  QualType ResPtrType = getContext().getPointerType(FD->getType());
+  llvm::Type *ResType = ConvertType(ResPtrType);
+  FieldPtr = Builder.CreatePointerCast(FieldPtr, ResType);
+  Address FieldAddress = Address(FieldPtr, Arg0Align);
+  FieldPtr = Builder.CreateLoad(FieldAddress);
+
+  // Need to handle bitfield here
+
+  GString = Builder.CreateGlobalStringPtr(Format + "\n");
+  Value *TmpRes = Builder.CreateCall(Func, {GString, FieldPtr});
+  Res = Builder.CreateAdd(Res, TmpRes);
+}
+
+std::string Format = "}\n";
+GString = Builder.CreateGlobalStringPtr(Format);
+Value *TmpRes = Builder.CreateCall(Func, {GString});
+Res = Builder.CreateAdd(Res, TmpRes);
+
+return RValue::get(Res);
+  }
+
   case Builtin::BI__builtin_cimag:
   case Builtin::BI__builtin_cimagf:
   case Builtin::BI__builtin_cimagl:
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -1374,6 +1374,7 @@
 BUILTIN(__builtin_operator_new, "v*z", "c")
 BUILTIN(__builtin_operator_delete, "vv*", "n")
 BUILTIN(__builtin_char_memchr, "c*cC*iz", "n")
+BUILTIN(__builtin_dump_struct, "ivC*v*", "n")
 
 // Safestack builtins
 BUILTIN(__builtin___get_unsafe_stack_start, "v*", "Fn")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel marked 4 inline comments as done.
paulsemel added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:1208
+
+assert(RT && "The first argument must be a record type");
+

aaron.ballman wrote:
> I don't see anything enforcing this constraint, so users are likely to hit 
> this assertion rather than a compile error.
I actually didn't manage to enforce this in the builtin declaration as we can 
only declare simple types (I am probably missing something)



Comment at: lib/CodeGen/CGBuiltin.cpp:1258
+
+  //Need to handle bitfield here
+

aaron.ballman wrote:
> Are you intending to implement this as part of this functionality?
Yes, my goal is to be able to dump the bitfields correctly, particularly if the 
structure is packed (for dumping a GDT for example).
I just didn't manage to do it properly for the moment.


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D44093: [BUILTINS] structure pretty printer

[Paul Semel via Phabricator via cfe-commits]

paulsemel created this revision.
paulsemel added a reviewer: aaron.ballman.

The purpose of this new builtin is to be able to pretty print any structure at 
runtime.
This might be really useful when debugging is not an option or is fastidious 
(like for kernel development).


Repository:
  rC Clang

https://reviews.llvm.org/D44093

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp

Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -17,6 +17,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
+#include "CGRecordLayout.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
@@ -1196,6 +1197,79 @@
 return RValue::get(ComplexVal.first);
   }
 
+  case Builtin::BI__builtin_dump_struct: {
+Value *Func = EmitScalarExpr(E->getArg(1)->IgnoreImpCasts());
+CharUnits Arg0Align = EmitPointerWithAlignment(E->getArg(0)).getAlignment();
+
+const Expr *e = E->getArg(0)->IgnoreImpCasts();
+QualType type = e->getType()->getPointeeType();
+const RecordType *RT = type->getAs();
+
+assert(RT && "The first argument must be a record type");
+
+RecordDecl *RD = RT->getDecl()->getDefinition();
+auto  = RD->getASTContext();
+const ASTRecordLayout  = ASTContext.getASTRecordLayout(RD);
+
+Value *string = Builder.CreateGlobalStringPtr(type.getAsString() + " {\n");
+Value *res = Builder.CreateCall(Func, {string});
+
+static llvm::DenseMap Types;
+Types[getContext().CharTy] = "%c";
+Types[getContext().BoolTy] = "%d";
+Types[getContext().IntTy] = "%d";
+Types[getContext().UnsignedIntTy] = "%u";
+Types[getContext().LongTy] = "%ld";
+Types[getContext().UnsignedLongTy] = "%lu";
+Types[getContext().LongLongTy] = "%lld";
+Types[getContext().UnsignedLongLongTy] = "%llu";
+Types[getContext().ShortTy] = "%hd";
+Types[getContext().UnsignedShortTy] = "%hu";
+Types[getContext().VoidPtrTy] = "%p";
+Types[getContext().FloatTy] = "%f";
+Types[getContext().DoubleTy] = "%f";
+Types[getContext().LongDoubleTy] = "%Lf";
+Types[getContext().getPointerType(getContext().CharTy)] = "%s";
+
+/* field : RecordDecl::field_iterator */
+for (auto field = RD->field_begin(); field != RD->field_end(); field++)
+{
+  uint64_t off = RL.getFieldOffset(field->getFieldIndex());
+  off = ASTContext.toCharUnitsFromBits(off).getQuantity();
+
+  Value *FieldPtr = EmitScalarExpr(E->getArg(0));
+  if (off)
+  {
+FieldPtr = Builder.CreatePtrToInt(FieldPtr, IntPtrTy);
+FieldPtr = Builder.CreateAdd(FieldPtr, ConstantInt::get(IntPtrTy, off));
+FieldPtr = Builder.CreateIntToPtr(FieldPtr, VoidPtrTy);
+  }
+  std::string str = field->getType().getAsString() + std::string(" ") +
+field->getNameAsString() + " : ";
+
+  str += Types[field->getType()];
+
+  QualType ResPtrType = getContext().getPointerType(field->getType());
+  llvm::Type *ResType = ConvertType(ResPtrType);
+  FieldPtr = Builder.CreatePointerCast(FieldPtr, ResType);
+  Address address = Address(FieldPtr, Arg0Align);
+  FieldPtr = Builder.CreateLoad(address);
+
+  //Need to handle bitfield here
+
+  string = Builder.CreateGlobalStringPtr(str + "\n");
+  Value *tmp = Builder.CreateCall(Func, {string, FieldPtr});
+  res = Builder.CreateAdd(res, tmp);
+}
+
+std::string str = "}\n";
+string = Builder.CreateGlobalStringPtr(str);
+Value *tmp = Builder.CreateCall(Func, {string});
+res = Builder.CreateAdd(res, tmp);
+
+return RValue::get(res);
+  }
+
   case Builtin::BI__builtin_cimag:
   case Builtin::BI__builtin_cimagf:
   case Builtin::BI__builtin_cimagl:
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -1374,6 +1374,7 @@
 BUILTIN(__builtin_operator_new, "v*z", "c")
 BUILTIN(__builtin_operator_delete, "vv*", "n")
 BUILTIN(__builtin_char_memchr, "c*cC*iz", "n")
+BUILTIN(__builtin_dump_struct, "ivC*v*", "n")
 
 // Safestack builtins
 BUILTIN(__builtin___get_unsafe_stack_start, "v*", "Fn")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits