This, like the x86_64 case, reads the register values from the minidump
file, and emits a binary buffer that is ordered using the offsets from
the RegisterInfoInterface argument. That way we can reuse an existing
register context.
Added unit tests.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
looks good, minor comments below.
source/Plugins/Process/minidump/RegisterContextMinidump_x86_32.h | ||
---|---|---|
37 ↗ | (On Diff #75315) | You don't need the WithRegIface suffix. That's an ObjC style, which we don't use in LLVM |
52 ↗ | (On Diff #75315) | typo |
115 ↗ | (On Diff #75315) | This comment doesn't seem to apply to the macro below it. |
unittests/Process/minidump/MinidumpParserTest.cpp | ||
279 ↗ | (On Diff #75315) | If you want to split them off to a different file, do it now. If not, remove the todo. (I don't see a reason for the split btw) |
unittests/Process/minidump/MinidumpParserTest.cpp | ||
---|---|---|
279 ↗ | (On Diff #75315) | Yes, I removed the TODO in my next CL. I'll leave the tests here. |
unittests/Process/minidump/MinidumpParserTest.cpp | ||
---|---|---|
340 ↗ | (On Diff #75578) | It seems a shame to remove the // clang-format off comment for this section. The aligned version is easier to read. |
unittests/Process/minidump/MinidumpParserTest.cpp | ||
---|---|---|
340 ↗ | (On Diff #75578) | I asked Dimitar to remove that. My feeling is that the directive should be reserved for just the extreme cases, where we would end up with a completely unreadable mess without it (like the current register contexts... ugh...). It's not a particularly strong feeling but e.g., there are zero occurrences of this in llvm. |