This revision was automatically updated to reflect the committed changes.
Closed by commit rL282529: Adding a RegisterContextMinidump_x86_64 converter
(authored by dvlahovski).
Changed prior to commit:
https://reviews.llvm.org/D24919?vs=72688&id=72689#toc
Repository:
rL LLVM
https://reviews
dvlahovski updated this revision to Diff 72688.
dvlahovski added a comment.
I like the combined approach more. So this is the implementation
https://reviews.llvm.org/D24919
Files:
include/lldb/lldb-private-types.h
source/Plugins/Process/minidump/CMakeLists.txt
source/Plugins/Process/minid
zturner accepted this revision.
This revision is now accepted and ready to land.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:50
@@ +49,3 @@
+size = 8;
+break;
+ }
If they are just smaller (but never bigger), you can cal
dvlahovski added inline comments.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+#define REG_VAL(x) *(reinterpret_cast(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
amccarth wrote:
> `EXPECT_xxx` will check the condit
amccarth added inline comments.
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:177
@@ +176,3 @@
+#define REG_VAL(x) *(reinterpret_cast(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
`EXPECT_xxx` will check the condition and report if it
dvlahovski added a comment.
Thanks for the explanation and examples! :) Will keep this in mind in the
future.
https://reviews.llvm.org/D24919
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/l
zturner added a comment.
`ASSERT_EQ` causes the test method to return immediately if the condition
fails. `EXPECT_EQ` will continue running the same test body. I like to use
`ASSERT_EQ`, for example, if you are checking the value of a pointer returned
by a function. First you want to check i
dvlahovski updated this revision to Diff 72659.
dvlahovski added a comment.
Addressing Zachary's and Adrian's comments
https://reviews.llvm.org/D24919
Files:
source/Plugins/Process/minidump/CMakeLists.txt
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/M
dvlahovski marked 5 inline comments as done.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:49
@@ +48,3 @@
+writeRegister(source_data, result_base, ®_info[lldb_cs_x86_64], 2);
+ }
+
zturner wrote:
> dvlahovski wrote:
> > sizeo
dvlahovski added inline comments.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:47-48
@@ +46,4 @@
+
+ if (*context_flags & uint32_t(MinidumpContext_x86_64_Flags::Control)) {
+writeRegister(source_data, result_base, ®_info[lldb_cs_x86_64], 2);
amccarth added inline comments.
Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:1
@@ +1,2 @@
+//===-- Registers_x86_64.cpp *- C++
-*-===//
+//
Should match file name.
Comment at:
zturner added inline comments.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:49
@@ +48,3 @@
+writeRegister(source_data, result_base, ®_info[lldb_cs_x86_64], 2);
+ }
+
dvlahovski wrote:
> sizeof(uint16_t), sizeof(uint32_t), et
dvlahovski added inline comments.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:49
@@ +48,3 @@
+writeRegister(source_data, result_base, ®_info[lldb_cs_x86_64], 2);
+ }
+
sizeof(uint16_t), sizeof(uint32_t), etc ?
https://rev
dvlahovski added inline comments.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:47-48
@@ +46,4 @@
+
+ if (*context_flags & uint32_t(MinidumpContext_x86_64_Flags::Control)) {
+writeRegister(source_data, result_base, ®_info[lldb_cs_x86_64], 2);
labath added a comment.
lgtm, after Zachary is happy.
https://reviews.llvm.org/D24919
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
dvlahovski added a comment.
I will fix the comments that Zachary made in the next revision
https://reviews.llvm.org/D24919
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
dvlahovski updated this revision to Diff 72509.
dvlahovski marked 4 inline comments as done.
dvlahovski added a comment.
Updating the CL regarding Pavel's comments
https://reviews.llvm.org/D24919
Files:
source/Plugins/Process/minidump/CMakeLists.txt
source/Plugins/Process/minidump/MinidumpP
zturner added inline comments.
Comment at:
source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp:46-47
@@ +45,4 @@
+ source_data = source_data.drop_front(6 * 8); // p[1-6] home registers
+ const uint32_t *context_flags;
+ consumeObject(source_data, context_flags);
labath added inline comments.
Comment at: source/Plugins/Process/minidump/MinidumpParser.h:42
@@ -41,1 +41,3 @@
+ const uint8_t *GetBaseAddr();
+
Replace these two functions with `llvm::ArrayRef GetData() const`
Comment at: source/Plugins/Proc
dvlahovski added a comment.
I tested this on the yet-to-be-submitted plugin code and it works fine on Linux
(should work fine everywhere for that matter). I can do a backtrace, go up/down
the stack frames and, of course, see all of the registers with `register read`
https://reviews.llvm.org/D2
dvlahovski created this revision.
dvlahovski added reviewers: labath, zturner.
dvlahovski added subscribers: amccarth, lldb-commits.
Herald added subscribers: mgorny, beanz.
This is a register context converter from Minidump to Linux reg context.
This knows the layout of the register context in th
21 matches
Mail list logo
