[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-10 Thread Pavel Labath via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rG3aa7e76677f1: MinidumpYAML: Add support for the memory info 
list stream (authored by labath).
Herald added a subscriber: hiraditya.

Changed prior to commit:
  https://reviews.llvm.org/D68645?vs=224065=224327#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68645

Files:
  llvm/include/llvm/BinaryFormat/Minidump.h
  llvm/include/llvm/ObjectYAML/MinidumpYAML.h
  llvm/lib/ObjectYAML/MinidumpEmitter.cpp
  llvm/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/test/tools/obj2yaml/basic-minidump.yaml

Index: llvm/test/tools/obj2yaml/basic-minidump.yaml
===
--- llvm/test/tools/obj2yaml/basic-minidump.yaml
+++ llvm/test/tools/obj2yaml/basic-minidump.yaml
@@ -55,6 +55,27 @@
 Memory Ranges:   
   - Start of Memory Range: 0x7C7D7E7F80818283
 Content:   '8485868788'
+  - Type:MemoryInfoList
+Memory Ranges:
+  - Base Address:0x
+Allocation Protect: [  ]
+Region Size: 0x0001
+State:   [ MEM_FREE ]
+Protect: [ PAGE_NO_ACCESS ]
+Type:[  ]
+  - Base Address:0x0001
+Allocation Protect: [ PAGE_READ_WRITE ]
+Region Size: 0x0001
+State:   [ MEM_COMMIT ]
+Type:[ MEM_MAPPED ]
+  - Base Address:0x0002
+Allocation Base: 0x
+Allocation Protect: [ PAGE_READ_WRITE, PAGE_WRITECOMBINE ]
+Reserved0:   0xDEADBEEF
+Region Size: 0x0001
+State:   [ MEM_COMMIT, MEM_FREE ]
+Type:[ MEM_PRIVATE, MEM_MAPPED ]
+Reserved1:   0xBAADF00D
 ...
 
 # CHECK:  --- !minidump
@@ -112,4 +133,25 @@
 # CHECK-NEXT: Memory Ranges:   
 # CHECK-NEXT:   - Start of Memory Range: 0x7C7D7E7F80818283
 # CHECK-NEXT: Content:   '8485868788'
+# CHECK-NEXT:   - Type:MemoryInfoList
+# CHECK-NEXT: Memory Ranges:
+# CHECK-NEXT:   - Base Address:   0x
+# CHECK-NEXT: Allocation Protect: [  ]
+# CHECK-NEXT: Region Size:0x0001
+# CHECK-NEXT: State:  [ MEM_FREE ]
+# CHECK-NEXT: Protect:[ PAGE_NO_ACCESS ]
+# CHECK-NEXT: Type:   [  ]
+# CHECK-NEXT:   - Base Address:   0x0001
+# CHECK-NEXT: Allocation Protect: [ PAGE_READ_WRITE ]
+# CHECK-NEXT: Region Size:0x0001
+# CHECK-NEXT: State:  [ MEM_COMMIT ]
+# CHECK-NEXT: Type:   [ MEM_MAPPED ]
+# CHECK-NEXT:   - Base Address:   0x0002
+# CHECK-NEXT: Allocation Base:0x
+# CHECK-NEXT: Allocation Protect: [ PAGE_READ_WRITE, PAGE_WRITECOMBINE ]
+# CHECK-NEXT: Reserved0:  0xDEADBEEF
+# CHECK-NEXT: Region Size:0x0001
+# CHECK-NEXT: State:  [ MEM_COMMIT, MEM_FREE ]
+# CHECK-NEXT: Type:   [ MEM_PRIVATE, MEM_MAPPED ]
+# CHECK-NEXT: Reserved1:  0xBAADF00D
 # CHECK-NEXT: ...
Index: llvm/lib/ObjectYAML/MinidumpYAML.cpp
===
--- llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -69,6 +69,8 @@
 
 Stream::StreamKind Stream::getKind(StreamType Type) {
   switch (Type) {
+  case StreamType::MemoryInfoList:
+return StreamKind::MemoryInfoList;
   case StreamType::MemoryList:
 return StreamKind::MemoryList;
   case StreamType::ModuleList:
@@ -93,6 +95,8 @@
 std::unique_ptr Stream::create(StreamType Type) {
   StreamKind Kind = getKind(Type);
   switch (Kind) {
+  case StreamKind::MemoryInfoList:
+return std::make_unique();
   case StreamKind::MemoryList:
 return std::make_unique();
   case StreamKind::ModuleList:
@@ -109,6 +113,25 @@
   llvm_unreachable("Unhandled stream kind!");
 }
 
+void yaml::ScalarBitSetTraits::bitset(
+IO , MemoryProtection ) {
+#define HANDLE_MDMP_PROTECT(CODE, NAME, NATIVENAME)\
+  IO.bitSetCase(Protect, #NATIVENAME, MemoryProtection::NAME);
+#include "llvm/BinaryFormat/MinidumpConstants.def"
+}
+
+void yaml::ScalarBitSetTraits::bitset(IO , MemoryState ) {
+#define HANDLE_MDMP_MEMSTATE(CODE, NAME, NATIVENAME)   \
+  IO.bitSetCase(State, #NATIVENAME, MemoryState::NAME);
+#include "llvm/BinaryFormat/MinidumpConstants.def"
+}
+
+void yaml::ScalarBitSetTraits::bitset(IO , MemoryType ) {
+#define HANDLE_MDMP_MEMTYPE(CODE, NAME, NATIVENAME)\
+  IO.bitSetCase(Type, #NATIVENAME, MemoryType::NAME);

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-10 Thread Pavel Labath via Phabricator via lldb-commits 
labath marked 3 inline comments as done.
labath added inline comments.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:116
+
+  MemoryInfoListStream()
+  : Stream(StreamKind::MemoryInfoList,

MaskRay wrote:
> Move default constructor above.
Done, I've also moved the constructor of SystemInfoStream for consistency.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645



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

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-10 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:111
+
+  explicit MemoryInfoListStream(std::vector Infos)
+  : Stream(StreamKind::MemoryInfoList,

grimar wrote:
> Maybe be more explicit here, i.e.
> 
> ```
> std::vector &
> ```
> ?
Or let the constructor take `iterator_range` 
as the argument, and change the call site below.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:116
+
+  MemoryInfoListStream()
+  : Stream(StreamKind::MemoryInfoList,

Move default constructor above.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645



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

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-09 Thread George Rimar via Phabricator via lldb-commits 
grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645



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

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-09 Thread Pavel Labath via Phabricator via lldb-commits 
labath updated this revision to Diff 224065.
labath marked 2 inline comments as done.
labath added a comment.

Address review comments


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645

Files:
  include/llvm/BinaryFormat/Minidump.h
  include/llvm/ObjectYAML/MinidumpYAML.h
  lib/ObjectYAML/MinidumpEmitter.cpp
  lib/ObjectYAML/MinidumpYAML.cpp
  test/tools/obj2yaml/basic-minidump.yaml

Index: test/tools/obj2yaml/basic-minidump.yaml
===
--- test/tools/obj2yaml/basic-minidump.yaml
+++ test/tools/obj2yaml/basic-minidump.yaml
@@ -55,6 +55,27 @@
 Memory Ranges:   
   - Start of Memory Range: 0x7C7D7E7F80818283
 Content:   '8485868788'
+  - Type:MemoryInfoList
+Memory Ranges:
+  - Base Address:0x
+Allocation Protect: [  ]
+Region Size: 0x0001
+State:   [ MEM_FREE ]
+Protect: [ PAGE_NO_ACCESS ]
+Type:[  ]
+  - Base Address:0x0001
+Allocation Protect: [ PAGE_READ_WRITE ]
+Region Size: 0x0001
+State:   [ MEM_COMMIT ]
+Type:[ MEM_MAPPED ]
+  - Base Address:0x0002
+Allocation Base: 0x
+Allocation Protect: [ PAGE_READ_WRITE, PAGE_WRITECOMBINE ]
+Reserved0:   0xDEADBEEF
+Region Size: 0x0001
+State:   [ MEM_COMMIT, MEM_FREE ]
+Type:[ MEM_PRIVATE, MEM_MAPPED ]
+Reserved1:   0xBAADF00D
 ...
 
 # CHECK:  --- !minidump
@@ -112,4 +133,25 @@
 # CHECK-NEXT: Memory Ranges:   
 # CHECK-NEXT:   - Start of Memory Range: 0x7C7D7E7F80818283
 # CHECK-NEXT: Content:   '8485868788'
+# CHECK-NEXT:   - Type:MemoryInfoList
+# CHECK-NEXT: Memory Ranges:
+# CHECK-NEXT:   - Base Address:   0x
+# CHECK-NEXT: Allocation Protect: [  ]
+# CHECK-NEXT: Region Size:0x0001
+# CHECK-NEXT: State:  [ MEM_FREE ]
+# CHECK-NEXT: Protect:[ PAGE_NO_ACCESS ]
+# CHECK-NEXT: Type:   [  ]
+# CHECK-NEXT:   - Base Address:   0x0001
+# CHECK-NEXT: Allocation Protect: [ PAGE_READ_WRITE ]
+# CHECK-NEXT: Region Size:0x0001
+# CHECK-NEXT: State:  [ MEM_COMMIT ]
+# CHECK-NEXT: Type:   [ MEM_MAPPED ]
+# CHECK-NEXT:   - Base Address:   0x0002
+# CHECK-NEXT: Allocation Base:0x
+# CHECK-NEXT: Allocation Protect: [ PAGE_READ_WRITE, PAGE_WRITECOMBINE ]
+# CHECK-NEXT: Reserved0:  0xDEADBEEF
+# CHECK-NEXT: Region Size:0x0001
+# CHECK-NEXT: State:  [ MEM_COMMIT, MEM_FREE ]
+# CHECK-NEXT: Type:   [ MEM_PRIVATE, MEM_MAPPED ]
+# CHECK-NEXT: Reserved1:  0xBAADF00D
 # CHECK-NEXT: ...
Index: lib/ObjectYAML/MinidumpYAML.cpp
===
--- lib/ObjectYAML/MinidumpYAML.cpp
+++ lib/ObjectYAML/MinidumpYAML.cpp
@@ -69,6 +69,8 @@
 
 Stream::StreamKind Stream::getKind(StreamType Type) {
   switch (Type) {
+  case StreamType::MemoryInfoList:
+return StreamKind::MemoryInfoList;
   case StreamType::MemoryList:
 return StreamKind::MemoryList;
   case StreamType::ModuleList:
@@ -93,6 +95,8 @@
 std::unique_ptr Stream::create(StreamType Type) {
   StreamKind Kind = getKind(Type);
   switch (Kind) {
+  case StreamKind::MemoryInfoList:
+return std::make_unique();
   case StreamKind::MemoryList:
 return std::make_unique();
   case StreamKind::ModuleList:
@@ -109,6 +113,25 @@
   llvm_unreachable("Unhandled stream kind!");
 }
 
+void yaml::ScalarBitSetTraits::bitset(
+IO , MemoryProtection ) {
+#define HANDLE_MDMP_PROTECT(CODE, NAME, NATIVENAME)\
+  IO.bitSetCase(Protect, #NATIVENAME, MemoryProtection::NAME);
+#include "llvm/BinaryFormat/MinidumpConstants.def"
+}
+
+void yaml::ScalarBitSetTraits::bitset(IO , MemoryState ) {
+#define HANDLE_MDMP_MEMSTATE(CODE, NAME, NATIVENAME)   \
+  IO.bitSetCase(State, #NATIVENAME, MemoryState::NAME);
+#include "llvm/BinaryFormat/MinidumpConstants.def"
+}
+
+void yaml::ScalarBitSetTraits::bitset(IO , MemoryType ) {
+#define HANDLE_MDMP_MEMTYPE(CODE, NAME, NATIVENAME)\
+  IO.bitSetCase(Type, #NATIVENAME, MemoryType::NAME);
+#include "llvm/BinaryFormat/MinidumpConstants.def"
+}
+
 void yaml::ScalarEnumerationTraits::enumeration(
 IO , ProcessorArchitecture ) {
 #define HANDLE_MDMP_ARCH(CODE, NAME)   \
@@ -215,6 +238,20 @@
   mapOptionalHex(IO, "AMD Extended

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-09 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lib/ObjectYAML/MinidumpEmitter.cpp:166
+Header.SizeOfEntry = sizeof(minidump::MemoryInfo);
+Header.NumberOfEntries = InfoList.Infos.size();
+File.allocateNewObject(Header);

grimar wrote:
> Probably just
> 
> ```
> minidump::MemoryInfoListHeader Header = {
> (support::ulittle32_t)sizeof(minidump::MemoryInfoListHeader),
> (support::ulittle32_t)sizeof(minidump::MemoryInfo),
> (support::ulittle64_t)InfoList.Infos.size()};
> ```
> 
> ?
> 
> Or perhaps it could have a constructor.
The thought of a constructor has crossed my mind too. I've now added it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645



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

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-09 Thread George Rimar via Phabricator via lldb-commits 
grimar added inline comments.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:111
+
+  explicit MemoryInfoListStream(std::vector Infos)
+  : Stream(StreamKind::MemoryInfoList,

Maybe be more explicit here, i.e.

```
std::vector &
```
?



Comment at: lib/ObjectYAML/MinidumpEmitter.cpp:166
+Header.SizeOfEntry = sizeof(minidump::MemoryInfo);
+Header.NumberOfEntries = InfoList.Infos.size();
+File.allocateNewObject(Header);

Probably just

```
minidump::MemoryInfoListHeader Header = {
(support::ulittle32_t)sizeof(minidump::MemoryInfoListHeader),
(support::ulittle32_t)sizeof(minidump::MemoryInfo),
(support::ulittle64_t)InfoList.Infos.size()};
```

?

Or perhaps it could have a constructor.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645



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

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-09 Thread James Henderson via Phabricator via lldb-commits 
jhenderson added subscribers: MaskRay, grimar.
jhenderson added a comment.

FYI, I'm going to be away for 2 and a half weeks from the end of work today, so 
won't have time to look at these if I don't get to them later today. I have no 
issues with other people reviewing them. You might want to add @grimar and 
@MaskRay to the reviews as they've been doing a lot of work in 
obj2yaml/yaml2obj recently.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645



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

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-08 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

LGTM


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645



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

[Lldb-commits] [PATCH] D68645: MinidumpYAML: Add support for the memory info list stream

2019-10-08 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D68645#1699705 , @JosephTremoulet 
wrote:

> Nit: Title says "thread" rather than "memory info"


Woops, sorry about that. Should be fixed now.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68645



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