[Lldb-commits] [PATCH] Don't use CopyContext in RegisterContextWin

Adrian McCarthy amccarth at google.com
Thu Feb 12 16:20:48 PST 2015

Comment at: include/lldb/Host/windows/windows.h:22-23
@@ -21,4 +21,2 @@
 #undef CreateProcess
-#undef LoadImage
-#undef GetUserName
 #undef far
zturner wrote:
> What's this part for?
Removed duplicate lines (see lines 19-20).  I was in here to roll NTDDI_VERSION back to Vista, but it was already Vista.  The notes explain how this "worked".

Comment at: source/Plugins/Process/Windows/x86/RegisterContextWindows_x86.cpp:335
@@ +334,3 @@
+    // to avoid relying on AVX APIs that aren't available prior to Windows 7 SP1.
+    const std::size_t kAlignment = 16;
+    buffer.reset(new DataBufferHeap(sizeof(CONTEXT) + kAlignment, 0));
zturner wrote:
> Is alignment to 16 necessary?  I think that's only if you use CopyContext() and InitializeContext().  If you're not using any of the AVX stuff, I don't think it has special alignment requirements.  We might be able to get rid of this whole function, and all the cruft associated with the heap buffer, and just store the CONTEXT directly in the class.  Note that CONTEXT in WinNT.h is declared with DECLSPEC_ALIGN(8), so I think everything is already taken care of.
One definition of CONTEXT in winnt.h is tagged with __declspec(align(16)), so I assumed it was.  Another is tagged 8, and one is untagged.  The heap allocation seemed to be returning blocks aligned only to 4-byte boundaries (not even 8!?).

Do you know how to tell definitively which CONTEXT applies?

Comment at: source/Plugins/Process/Windows/x86/RegisterContextWindows_x86.cpp:337-338
@@ +336,4 @@
+    buffer.reset(new DataBufferHeap(sizeof(CONTEXT) + kAlignment, 0));
+    std::intptr_t address = reinterpret_cast<std::intptr_t>(buffer->GetBytes());
+    address += kAlignment - (address % kAlignment);
+    *context_ptr = reinterpret_cast<CONTEXT *>(address);
zturner wrote:
> checkout llvm/Support/MathExtras.h.  There are functions to do this kind of math for you.  (This only applies though if it's not already aligned correctly.  If it is all this code goes away)
Cool.  I've switched to llvm::alignAddr.

But I'll remove this altogether if the alignment isn't needed.



More information about the lldb-commits mailing list