[PATCH] D150499: [AST] Initialized data after TypeSourceInfo

[Vitaly Buka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4498663f3de0: [AST] Initialized data after TypeSourceInfo 
(authored by vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150499

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -3018,7 +3018,7 @@
 
   auto *TInfo =
 (TypeSourceInfo*)BumpAlloc.Allocate(sizeof(TypeSourceInfo) + DataSize, 8);
-  new (TInfo) TypeSourceInfo(T);
+  new (TInfo) TypeSourceInfo(T, DataSize);
   return TInfo;
 }
 
Index: clang/include/clang/AST/TypeLoc.h
===
--- clang/include/clang/AST/TypeLoc.h
+++ clang/include/clang/AST/TypeLoc.h
@@ -240,6 +240,11 @@
   static SourceRange getLocalSourceRangeImpl(TypeLoc TL);
 };
 
+inline TypeSourceInfo::TypeSourceInfo(QualType ty, size_t DataSize) : Ty(ty) {
+  // Init data attached to the object. See getTypeLoc.
+  memset(this + 1, 0, DataSize);
+}
+
 /// Return the TypeLoc for a type source info.
 inline TypeLoc TypeSourceInfo::getTypeLoc() const {
   // TODO: is this alignment already sufficient?
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6640,7 +6640,7 @@
 
   QualType Ty;
 
-  TypeSourceInfo(QualType ty) : Ty(ty) {}
+  TypeSourceInfo(QualType ty, size_t DataSize); // implemented in TypeLoc.h
 
 public:
   /// Return the type wrapped by this type source info.


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -3018,7 +3018,7 @@
 
   auto *TInfo =
 (TypeSourceInfo*)BumpAlloc.Allocate(sizeof(TypeSourceInfo) + DataSize, 8);
-  new (TInfo) TypeSourceInfo(T);
+  new (TInfo) TypeSourceInfo(T, DataSize);
   return TInfo;
 }
 
Index: clang/include/clang/AST/TypeLoc.h
===
--- clang/include/clang/AST/TypeLoc.h
+++ clang/include/clang/AST/TypeLoc.h
@@ -240,6 +240,11 @@
   static SourceRange getLocalSourceRangeImpl(TypeLoc TL);
 };
 
+inline TypeSourceInfo::TypeSourceInfo(QualType ty, size_t DataSize) : Ty(ty) {
+  // Init data attached to the object. See getTypeLoc.
+  memset(this + 1, 0, DataSize);
+}
+
 /// Return the TypeLoc for a type source info.
 inline TypeLoc TypeSourceInfo::getTypeLoc() const {
   // TODO: is this alignment already sufficient?
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6640,7 +6640,7 @@
 
   QualType Ty;
 
-  TypeSourceInfo(QualType ty) : Ty(ty) {}
+  TypeSourceInfo(QualType ty, size_t DataSize); // implemented in TypeLoc.h
 
 public:
   /// Return the type wrapped by this type source info.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150499: [AST] Initialized data after TypeSourceInfo

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added reviewers: kstoimenov, thurston.
vitalybuka added a comment.

Oh, I rerun msan bot locally and it looked like issue dissipated, but I 
probably checked wrong logs.
So I enabled -Oz on the bot, to catch more bugs, but the issue is still there 
https://lab.llvm.org/buildbot/#/builders/5/builds/33926

Let's land this trivial fix, and if owners of the AST believe the root-cause is 
different, we can investigate later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150499

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

[PATCH] D150499: [AST] Initialized data after TypeSourceInfo

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150499

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

[PATCH] D150499: [AST] Initialized data after TypeSourceInfo

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

ping, any concerns regarding this patch?
i want to make MSAN stricter on our bot by reducing -O level


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150499

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

[PATCH] D150499: [AST] Initialized data after TypeSourceInfo

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

In D150499#4341115 , @rjmccall wrote:

> This memory is supposed to be initialized when we copy the `TypeLoc`.  Are 
> you observing that not happening?

Yes. There is the msan report in the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150499

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

[PATCH] D150499: [AST] Initialized data after TypeSourceInfo

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

This memory is supposed to be initialized when we copy the `TypeLoc`.  Are you 
observing that not happening?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150499

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

[PATCH] D150499: [AST] Initialized data after TypeSourceInfo

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka updated this revision to Diff 521847.
vitalybuka added a comment.

use 0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150499

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -3019,7 +3019,7 @@
 
   auto *TInfo =
 (TypeSourceInfo*)BumpAlloc.Allocate(sizeof(TypeSourceInfo) + DataSize, 8);
-  new (TInfo) TypeSourceInfo(T);
+  new (TInfo) TypeSourceInfo(T, DataSize);
   return TInfo;
 }
 
Index: clang/include/clang/AST/TypeLoc.h
===
--- clang/include/clang/AST/TypeLoc.h
+++ clang/include/clang/AST/TypeLoc.h
@@ -240,6 +240,11 @@
   static SourceRange getLocalSourceRangeImpl(TypeLoc TL);
 };
 
+inline TypeSourceInfo::TypeSourceInfo(QualType ty, size_t DataSize) : Ty(ty) {
+  // Init data attached to the object. See getTypeLoc.
+  memset(this + 1, 0, DataSize);
+}
+
 /// Return the TypeLoc for a type source info.
 inline TypeLoc TypeSourceInfo::getTypeLoc() const {
   // TODO: is this alignment already sufficient?
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6640,7 +6640,7 @@
 
   QualType Ty;
 
-  TypeSourceInfo(QualType ty) : Ty(ty) {}
+  TypeSourceInfo(QualType ty, size_t DataSize); // implemented in TypeLoc.h
 
 public:
   /// Return the type wrapped by this type source info.


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -3019,7 +3019,7 @@
 
   auto *TInfo =
 (TypeSourceInfo*)BumpAlloc.Allocate(sizeof(TypeSourceInfo) + DataSize, 8);
-  new (TInfo) TypeSourceInfo(T);
+  new (TInfo) TypeSourceInfo(T, DataSize);
   return TInfo;
 }
 
Index: clang/include/clang/AST/TypeLoc.h
===
--- clang/include/clang/AST/TypeLoc.h
+++ clang/include/clang/AST/TypeLoc.h
@@ -240,6 +240,11 @@
   static SourceRange getLocalSourceRangeImpl(TypeLoc TL);
 };
 
+inline TypeSourceInfo::TypeSourceInfo(QualType ty, size_t DataSize) : Ty(ty) {
+  // Init data attached to the object. See getTypeLoc.
+  memset(this + 1, 0, DataSize);
+}
+
 /// Return the TypeLoc for a type source info.
 inline TypeLoc TypeSourceInfo::getTypeLoc() const {
   // TODO: is this alignment already sufficient?
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6640,7 +6640,7 @@
 
   QualType Ty;
 
-  TypeSourceInfo(QualType ty) : Ty(ty) {}
+  TypeSourceInfo(QualType ty, size_t DataSize); // implemented in TypeLoc.h
 
 public:
   /// Return the type wrapped by this type source info.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150499: [AST] Initialized data after TypeSourceInfo

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka created this revision.
Herald added a project: All.
vitalybuka requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is no initialization of the data between allocation
and first getBeginLoc call.

llvm-project/clang/lib/AST/ASTContext.cpp:3022
llvm-project/clang/lib/AST/TypeLoc.cpp:222

Msan report https://reviews.llvm.org/P8306


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150499

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -3019,7 +3019,7 @@
 
   auto *TInfo =
 (TypeSourceInfo*)BumpAlloc.Allocate(sizeof(TypeSourceInfo) + DataSize, 8);
-  new (TInfo) TypeSourceInfo(T);
+  new (TInfo) TypeSourceInfo(T, DataSize);
   return TInfo;
 }
 
Index: clang/include/clang/AST/TypeLoc.h
===
--- clang/include/clang/AST/TypeLoc.h
+++ clang/include/clang/AST/TypeLoc.h
@@ -240,6 +240,11 @@
   static SourceRange getLocalSourceRangeImpl(TypeLoc TL);
 };
 
+inline TypeSourceInfo::TypeSourceInfo(QualType ty, size_t DataSize) : Ty(ty) {
+  // Init data attached to the object. See getTypeLoc.
+  memset(this + 1, 255, DataSize);
+}
+
 /// Return the TypeLoc for a type source info.
 inline TypeLoc TypeSourceInfo::getTypeLoc() const {
   // TODO: is this alignment already sufficient?
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6640,7 +6640,7 @@
 
   QualType Ty;
 
-  TypeSourceInfo(QualType ty) : Ty(ty) {}
+  TypeSourceInfo(QualType ty, size_t DataSize); // implemented in TypeLoc.h
 
 public:
   /// Return the type wrapped by this type source info.


Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -3019,7 +3019,7 @@
 
   auto *TInfo =
 (TypeSourceInfo*)BumpAlloc.Allocate(sizeof(TypeSourceInfo) + DataSize, 8);
-  new (TInfo) TypeSourceInfo(T);
+  new (TInfo) TypeSourceInfo(T, DataSize);
   return TInfo;
 }
 
Index: clang/include/clang/AST/TypeLoc.h
===
--- clang/include/clang/AST/TypeLoc.h
+++ clang/include/clang/AST/TypeLoc.h
@@ -240,6 +240,11 @@
   static SourceRange getLocalSourceRangeImpl(TypeLoc TL);
 };
 
+inline TypeSourceInfo::TypeSourceInfo(QualType ty, size_t DataSize) : Ty(ty) {
+  // Init data attached to the object. See getTypeLoc.
+  memset(this + 1, 255, DataSize);
+}
+
 /// Return the TypeLoc for a type source info.
 inline TypeLoc TypeSourceInfo::getTypeLoc() const {
   // TODO: is this alignment already sufficient?
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -6640,7 +6640,7 @@
 
   QualType Ty;
 
-  TypeSourceInfo(QualType ty) : Ty(ty) {}
+  TypeSourceInfo(QualType ty, size_t DataSize); // implemented in TypeLoc.h
 
 public:
   /// Return the type wrapped by this type source info.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits