[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa50272f8261f: Update MinidumpYAML to use minidump::Exception
for exception stream (authored by JosephTremoulet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,200 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+// Test that we can parse a normal-looking ExceptionStream.
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation.
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 225459.
JosephTremoulet marked an inline comment as done.
JosephTremoulet added a comment.
- Rebase
- Use Twine instead of formatv
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,200 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+// Test that we can parse a normal-looking ExceptionStream.
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation.
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+ Parameter 13:
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet marked an inline comment as done.
JosephTremoulet added inline comments.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:389
+ for (size_t Index = 0; Index < Exception.MaxParameters; ++Index) {
+SmallString<16> Name = formatv("Parameter {0}", Index);
+support::ulittle64_t = Exception.ExceptionInformation[Index];
JosephTremoulet wrote:
> MaskRay wrote:
> > You may use `("Parameter " + Twine(Index)).str()` to get rid of the
> > "llvm/Support/FormatVariadic.h" dependency.
> Is #including FormatVariadic.h a bad thing? I did it this way to avoid heap
> allocation.
...finally realized that Twine avoids the heap too :). Thanks for the
suggestion, updated.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet marked an inline comment as done.
JosephTremoulet added inline comments.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:389
+ for (size_t Index = 0; Index < Exception.MaxParameters; ++Index) {
+SmallString<16> Name = formatv("Parameter {0}", Index);
+support::ulittle64_t = Exception.ExceptionInformation[Index];
MaskRay wrote:
> You may use `("Parameter " + Twine(Index)).str()` to get rid of the
> "llvm/Support/FormatVariadic.h" dependency.
Is #including FormatVariadic.h a bad thing? I did it this way to avoid heap
allocation.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
MaskRay added inline comments.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:389
+ for (size_t Index = 0; Index < Exception.MaxParameters; ++Index) {
+SmallString<16> Name = formatv("Parameter {0}", Index);
+support::ulittle64_t = Exception.ExceptionInformation[Index];
You may use `("Parameter " + Twine(Index)).str()` to get rid of the
"llvm/Support/FormatVariadic.h" dependency.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
grimar added inline comments. Comment at: llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml:3 + +# Test that we report an error for an ExceptionStream where the specified +# number of parameters is greater than the number of ExceptionInformation JosephTremoulet wrote: > grimar wrote: > > nit: It is became common to use ## for comments in yaml2obj and actually > > many other LLVM tools test cases. > > (here and below) > Sorry, when you say "here and below", do you just mean lines 3 and 4-5, or do > you also mean the CHECK line (line 20)? Grepping in test/tools it appears to > be very uncommon to use ## for CHECK lines, but that's what I'd have > otherwise assumed "and below" referred to... Sorry, I think I misread the line 20, i.e. I read it as a comment. Indeed we do not use ## for CHECK lines. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 225044.
JosephTremoulet added a comment.
- punctuation fixes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,200 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+// Test that we can parse a normal-looking ExceptionStream.
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation.
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+ Parameter 13: 0x33
+ Parameter 14: 0x22
+Thread Context:
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet marked an inline comment as done. JosephTremoulet added inline comments. Comment at: llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml:3 + +# Test that we report an error for an ExceptionStream where the specified +# number of parameters is greater than the number of ExceptionInformation grimar wrote: > nit: It is became common to use ## for comments in yaml2obj and actually many > other LLVM tools test cases. > (here and below) Sorry, when you say "here and below", do you just mean lines 3 and 4-5, or do you also mean the CHECK line (line 20)? Grepping in test/tools it appears to be very uncommon to use ## for CHECK lines, but that's what I'd have otherwise assumed "and below" referred to... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.
LGTM with 2 nits.
Comment at: llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml:3
+
+# Test that we report an error for an ExceptionStream where the specified
+# number of parameters is greater than the number of ExceptionInformation
nit: It is became common to use ## for comments in yaml2obj and actually many
other LLVM tools test cases.
(here and below)
Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:143
+
+// Test that we can parse a normal-looking ExceptionStream
+TEST(MinidumpYAML, ExceptionStream) {
nit: Missing a full stop (here and below).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
labath added a comment. Thanks. I think this is great. @grimar, do you have any more comments? Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:377-383 + // TODO: We could provide a reasonable default for ThreadContext by searching + // the Thread stream for a thread with the given ID and using its Context. + // That would require a couple changes: + // 1. We'd need to pass the whole dump around as a context argument. + // 2. We'd need to ensure that the Thread stream got processed before + // the Exception stream (or make Exception's ThreadContext required + // when the Exception stream is processed before the Thread stream). JosephTremoulet wrote: > labath wrote: > > I've been thinking about this for a while now, and while that idea has some > > appeal, I am not sure if this would ever really be a "good" idea. > > Currently, this format allows you to easily create > > syntactically-valid-but-probably-nonsensical minidumps (multiple thread > > list streams, multiple threads with the same ID, etc..). All of that would > > be more difficult if we started depending on strict "correctness" of other > > parts of the minidump in order to compute something here. > > > > Even if I was doing this, I'd probably implement this differently -- make > > the context always optional, but then check for consistency higher up (the > > validate call of the entire minidump object or something). Anyway, maybe > > just delete this todo? :) > I've removed the TODO, but I reserve the right to think this would be a good > idea :). Speficially because I think you could still model in YAML anything > you could put in a minidump file, by explicitly providing the ThreadContext > even when it has a default. > > (This is in contrast to the other validation stuff I had in earlier > revisions, where I was just misunderstanding the point of yaml validation -- > so thanks for explaining it!) Fair enough. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 224857.
JosephTremoulet marked 2 inline comments as done.
JosephTremoulet added a comment.
- Move test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,200 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+// Test that we can parse a normal-looking ExceptionStream
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+ Parameter 13: 0x33
+ Parameter 14: 0x22
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
labath added inline comments.
Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:310-316
+ ASSERT_FALSE(ExpectedFile);
+ auto Unhandled =
+ handleErrors(ExpectedFile.takeError(), [](const StringError ) {
+EXPECT_EQ(static_cast(std::errc::invalid_argument),
+ error.convertToErrorCode().value());
+ });
+ EXPECT_THAT_ERROR(std::move(Unhandled), Succeeded());
JosephTremoulet wrote:
> labath wrote:
> > Maybe:
> > ```
> > EXPECT_THAT_EXPECTED(ExpectedFile, Failed(
> > testing::Property(::convertToErrorCode,
> > make_error_code(errc::invalid_argument;
> > ```
> > ?
> >
> > Though, this might actually be best off as a lit test where you just
> > FileCheck the exact error message.
> Moved to lit, thanks for the suggestion. The obvious place seemed to be
> llvm/test/ObjectYAML, but there weren't any minidump tests there so I added a
> minidump subdirectory for the new test. Please let me know if there was a
> better place that I just overlooked.
there are some in `test/tools/yaml2obj`. I'd put it next to those.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet marked 4 inline comments as done.
JosephTremoulet added inline comments.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:377-383
+ // TODO: We could provide a reasonable default for ThreadContext by searching
+ // the Thread stream for a thread with the given ID and using its Context.
+ // That would require a couple changes:
+ // 1. We'd need to pass the whole dump around as a context argument.
+ // 2. We'd need to ensure that the Thread stream got processed before
+ // the Exception stream (or make Exception's ThreadContext required
+ // when the Exception stream is processed before the Thread stream).
labath wrote:
> I've been thinking about this for a while now, and while that idea has some
> appeal, I am not sure if this would ever really be a "good" idea. Currently,
> this format allows you to easily create
> syntactically-valid-but-probably-nonsensical minidumps (multiple thread list
> streams, multiple threads with the same ID, etc..). All of that would be more
> difficult if we started depending on strict "correctness" of other parts of
> the minidump in order to compute something here.
>
> Even if I was doing this, I'd probably implement this differently -- make the
> context always optional, but then check for consistency higher up (the
> validate call of the entire minidump object or something). Anyway, maybe just
> delete this todo? :)
I've removed the TODO, but I reserve the right to think this would be a good
idea :). Speficially because I think you could still model in YAML anything you
could put in a minidump file, by explicitly providing the ThreadContext even
when it has a default.
(This is in contrast to the other validation stuff I had in earlier revisions,
where I was just misunderstanding the point of yaml validation -- so thanks for
explaining it!)
Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:310-316
+ ASSERT_FALSE(ExpectedFile);
+ auto Unhandled =
+ handleErrors(ExpectedFile.takeError(), [](const StringError ) {
+EXPECT_EQ(static_cast(std::errc::invalid_argument),
+ error.convertToErrorCode().value());
+ });
+ EXPECT_THAT_ERROR(std::move(Unhandled), Succeeded());
labath wrote:
> Maybe:
> ```
> EXPECT_THAT_EXPECTED(ExpectedFile, Failed(
> testing::Property(::convertToErrorCode,
> make_error_code(errc::invalid_argument;
> ```
> ?
>
> Though, this might actually be best off as a lit test where you just
> FileCheck the exact error message.
Moved to lit, thanks for the suggestion. The obvious place seemed to be
llvm/test/ObjectYAML, but there weren't any minidump tests there so I added a
minidump subdirectory for the new test. Please let me know if there was a
better place that I just overlooked.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 224853.
JosephTremoulet added a comment.
- Remove TODO, lit-ify negative test and tighten check
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/ObjectYAML/minidump/ExceptionMissingParameter.yaml
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,200 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+// Test that we can parse a normal-looking ExceptionStream
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+ Parameter 13: 0x33
+ Parameter 14: 0x22
+Thread
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
labath added inline comments.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:394
+ mapOptionalHex(IO, "Exception Address", Exception.ExceptionAddress, 0);
+ IO.mapOptional("Number Parameters", Exception.NumberParameters,
+ support::ulittle32_t(0u));
JosephTremoulet wrote:
> labath wrote:
> > This file has a helper function for this (`mapOptional(IO, "name", value,
> > 0)`. I'd consider changing the field name to "Number of Parameters" even
> > though it does not match the field name, as it reads weird without that.
> > I'm not sure why the microsoft naming is inconsistent here -- most of the
> > other minidump structs have "of" in their name already (BaseOfImage,
> > SizeOfImage, etc.), but at least we can be consistent.
> Updated to use the helper, and changed the name in the YAML to "Number of
> Parameters". Let me know if it's important to you to also change the name of
> the field in the llvm::minidump::Exception type to `NumberOfParameters` -- I
> wasn't sure if you were suggesting that, and regardless my preference would
> be to leave that as-is to match breakpad aside from casing, as otherwise it's
> hard to know where to stop (e.g. change "ExceptionInformation" to
> "Parameters" to match "NumberOfParameters" and the YAML? Reconcile the
> several different ways that alignment padding fields are named? etc.)
I wasn't sure about that either -- that's why it was phrased so vaguely. While
I generally tried to stick to the original naming, I have already done some
renames to the field names in existing structures -- IIRC, it was limited to
unifying the naming for the various "reserved"/"unused" fields. Anyway, I think
that keeping the original name in this particular case is fine.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:377-383
+ // TODO: We could provide a reasonable default for ThreadContext by searching
+ // the Thread stream for a thread with the given ID and using its Context.
+ // That would require a couple changes:
+ // 1. We'd need to pass the whole dump around as a context argument.
+ // 2. We'd need to ensure that the Thread stream got processed before
+ // the Exception stream (or make Exception's ThreadContext required
+ // when the Exception stream is processed before the Thread stream).
I've been thinking about this for a while now, and while that idea has some
appeal, I am not sure if this would ever really be a "good" idea. Currently,
this format allows you to easily create
syntactically-valid-but-probably-nonsensical minidumps (multiple thread list
streams, multiple threads with the same ID, etc..). All of that would be more
difficult if we started depending on strict "correctness" of other parts of the
minidump in order to compute something here.
Even if I was doing this, I'd probably implement this differently -- make the
context always optional, but then check for consistency higher up (the validate
call of the entire minidump object or something). Anyway, maybe just delete
this todo? :)
Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:310-316
+ ASSERT_FALSE(ExpectedFile);
+ auto Unhandled =
+ handleErrors(ExpectedFile.takeError(), [](const StringError ) {
+EXPECT_EQ(static_cast(std::errc::invalid_argument),
+ error.convertToErrorCode().value());
+ });
+ EXPECT_THAT_ERROR(std::move(Unhandled), Succeeded());
Maybe:
```
EXPECT_THAT_EXPECTED(ExpectedFile, Failed(
testing::Property(::convertToErrorCode,
make_error_code(errc::invalid_argument;
```
?
Though, this might actually be best off as a lit test where you just FileCheck
the exact error message.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 224805.
JosephTremoulet added a comment.
- ...and fix namespace...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,229 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+// Test that we can parse a normal-looking ExceptionStream
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+ Parameter 13: 0x33
+ Parameter 14: 0x22
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 224804.
JosephTremoulet added a comment.
- Fix Expected<> types
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,229 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+// Test that we can parse a normal-looking ExceptionStream
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+ Parameter 13: 0x33
+ Parameter 14: 0x22
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 224800.
JosephTremoulet marked an inline comment as done.
JosephTremoulet added a comment.
- Apply review feedback (-auto, -memset, +comments)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,229 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+// Test that we can parse a normal-looking ExceptionStream
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+ Parameter 13: 0x33
+ Parameter 14: 0x22
+Thread Context:
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
grimar added inline comments.
Comment at: llvm/include/llvm/ObjectYAML/MinidumpYAML.h:113-114
+ ExceptionStream()
+ : Stream(StreamKind::Exception, minidump::StreamType::Exception) {
+memset(, 0, sizeof(minidump::ExceptionStream));
+ }
I'd avoid memset:
```
ExceptionStream()
: Stream(StreamKind::Exception, minidump::StreamType::Exception),
MDExceptionStream({}) {
```
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:525
+ case StreamKind::Exception: {
+auto ExpectedExceptionStream = File.getExceptionStream();
+if (!ExpectedExceptionStream)
We often avoid using `auto` when return type is not obvious.
Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:143
+
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
I'd add a comment for each test to describe what they do.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 224607.
JosephTremoulet added a comment.
Address review feedback
- Add Exception stream to minidump-basic.yaml to test obj2yaml
direction
- Reorder ExceptionStream definition and constructors
- Use the mapOptional helper
- Replace "Number Parameters" with "Number of Parameters" in YAML
- Stop using mapping traits validation for number of parameters,
update test to ensure correctly de-yamlizing an exception record
with an out-of-bounds number of parameters
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/test/tools/obj2yaml/basic-minidump.yaml
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,218 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number of Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number of Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet marked 4 inline comments as done.
JosephTremoulet added a comment.
Added Exception stream to minidump-basic.yaml as suggested.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:394
+ mapOptionalHex(IO, "Exception Address", Exception.ExceptionAddress, 0);
+ IO.mapOptional("Number Parameters", Exception.NumberParameters,
+ support::ulittle32_t(0u));
labath wrote:
> This file has a helper function for this (`mapOptional(IO, "name", value,
> 0)`. I'd consider changing the field name to "Number of Parameters" even
> though it does not match the field name, as it reads weird without that. I'm
> not sure why the microsoft naming is inconsistent here -- most of the other
> minidump structs have "of" in their name already (BaseOfImage, SizeOfImage,
> etc.), but at least we can be consistent.
Updated to use the helper, and changed the name in the YAML to "Number of
Parameters". Let me know if it's important to you to also change the name of
the field in the llvm::minidump::Exception type to `NumberOfParameters` -- I
wasn't sure if you were suggesting that, and regardless my preference would be
to leave that as-is to match breakpad aside from casing, as otherwise it's hard
to know where to stop (e.g. change "ExceptionInformation" to "Parameters" to
match "NumberOfParameters" and the YAML? Reconcile the several different ways
that alignment padding fields are named? etc.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
labath added a comment.
Thanks. IIUC, all the existing tests just cover the yaml2obj direction. Could
you add something for the other direction too? Maybe add an exception stream to
`test/tools/obj2yaml/basic-minidump.yaml`?
Comment at: llvm/include/llvm/ObjectYAML/MinidumpYAML.h:162-181
+/// ExceptionStream minidump stream.
+struct ExceptionStream : public Stream {
+ minidump::ExceptionStream MDExceptionStream;
+ yaml::BinaryRef ThreadContext;
+
+ explicit ExceptionStream(const minidump::ExceptionStream ,
+ ArrayRef ThreadContext)
I've been trying to keep this somewhat sorted. Could you move this before the
`MemoryInfoListStream` class? Also, in the previous patch we've moved the
default constructors to front. It would be good to make this consistent with
that.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:394
+ mapOptionalHex(IO, "Exception Address", Exception.ExceptionAddress, 0);
+ IO.mapOptional("Number Parameters", Exception.NumberParameters,
+ support::ulittle32_t(0u));
This file has a helper function for this (`mapOptional(IO, "name", value, 0)`.
I'd consider changing the field name to "Number of Parameters" even though it
does not match the field name, as it reads weird without that. I'm not sure why
the microsoft naming is inconsistent here -- most of the other minidump structs
have "of" in their name already (BaseOfImage, SizeOfImage, etc.), but at least
we can be consistent.
Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:408-414
+StringRef yaml::MappingTraits::validate(
+yaml::IO , minidump::Exception ) {
+ if (Exception.NumberParameters > Exception::MaxParameters)
+return "Exception reports too many parameters";
+ return "";
+}
+
Could you remove this bit too? While it is technically invalid, this is not
something that yaml2obj needs to care about (as it does not prevent successful
serialization), and it would be nice to be able to use it to generate a test
case with an invalid number (because that is something lldb should care about
and expect/handle)..
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream
JosephTremoulet updated this revision to Diff 224469.
JosephTremoulet added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
- Update test input yaml Exception stream
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68657/new/
https://reviews.llvm.org/D68657
Files:
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
llvm/include/llvm/ObjectYAML/MinidumpYAML.h
llvm/lib/ObjectYAML/MinidumpEmitter.cpp
llvm/lib/ObjectYAML/MinidumpYAML.cpp
llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,197 @@
(ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
}
+
+TEST(MinidumpYAML, ExceptionStream) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+ Number Parameters: 2
+ Parameter 0: 0x22
+ Parameter 1: 0x24
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(2u, Exception.NumberParameters);
+ EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+ EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x7
+Exception Record:
+ Exception Code: 0x23
+ Exception Flags: 0x5
+ Exception Record: 0x0102030405060708
+ Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected ExpectedStream =
+ File.getExceptionStream();
+
+ ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+ const minidump::ExceptionStream = *ExpectedStream;
+ EXPECT_EQ(0x7u, Stream.ThreadId);
+ const minidump::Exception = Stream.ExceptionRecord;
+ EXPECT_EQ(0x23u, Exception.ExceptionCode);
+ EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+ EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+ EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+ EXPECT_EQ(0u, Exception.NumberParameters);
+
+ Expected> ExpectedContext =
+ File.getRawData(Stream.ThreadContext);
+ ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+ EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+ 0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Exception
+Thread ID: 0x8
+Exception Record:
+ Exception Code: 0
+ Number Parameters: 16
+ Parameter 0: 0x0
+ Parameter 1: 0xff
+ Parameter 2: 0xee
+ Parameter 3: 0xdd
+ Parameter 4: 0xcc
+ Parameter 5: 0xbb
+ Parameter 6: 0xaa
+ Parameter 7: 0x99
+ Parameter 8: 0x88
+ Parameter 9: 0x77
+ Parameter 10: 0x66
+ Parameter 11: 0x55
+ Parameter 12: 0x44
+ Parameter 13: 0x33
+ Parameter 14: 0x22
+ Parameter 15: 0x11
+Thread Context: 3DeadBeefDefacedABadCafe)");
+ ASSERT_FALSE(ExpectedFile);
+ auto Unhandled =
+ handleErrors(ExpectedFile.takeError(), [](const StringError ) {
+EXPECT_EQ(static_cast(std::errc::invalid_argument),
+ error.convertToErrorCode().value());
+ });
+
