[Lldb-commits] [lldb] r353778 - Define _ENABLE_EXTENDED_ALIGNED_STORAGE on Windows.

[Pavel Labath via lldb-commits]

Ah, it's our struct XSAVE friend again. Yeah, I think I have a pretty 
good idea of what this is doing, and I believe we don't need any special 
alignment there, so I just went ahead and removed it. Assuming this 
doesn't cause any issues, you should be able to revert this patch.

On 13/02/2019 18:46, Jonas Devlieghere wrote:
> Hi Pavel,
> 
> I think the offending struct is XSAVE in RegisterContext_x86.h. I don't 
> really know this code, would you mind having a look? It looks like you 
> touched the alignment in the past and seems to know what this this is 
> doing :-)
> 
> tree f8fc1af1c62b728260b6c950649c55f7aba513f6
> parent 6857692687979d934e188d117eadc93f48d52a0c
> author Pavel Labath <pavel at labath.sk <mailto:pavel at labath.sk>> Wed Sep 
> 12 08:50:08 2018 +0000
> committer Pavel Labath <pavel at labath.sk <mailto:pavel at labath.sk>> Wed 
> Sep 12 08:50:08 2018 +0000
> 
> Reduce alignment on struct XSAVE, fixing a gcc warning
> 
> The warning is about heap-allocating a struct with bigger alignment
> requirements than the standard heap allocator provides.
> 
> AFAICT, all uses of the XSAVE struct are already heap-allocated, so this
> high alignment does not actually have any effect and removing it should
> be NFC.
> 
> I have also done some digging in the commit history. This alignment
> requirement was since the XSAVE struct was introduced in r180572 when
> adding AVX register support for linux. It does not mention the alignment
> specifically, so I am guessing this was just put there because the
> corresponging XSAVE cpu instruction requires its buffer to be 64-byte
> aligned. However, LLDB will not be normally reading this struct via the
> XSAVE instruction directly. Instead we will ask the kernel to copy the
> buffer saved when suspeding the inferior. This should not require such
> strict alignment (in fact, linux kernel will happily do this for any
> alignment).
> 
> llvm-svn: 342029
> 
> 
> 
> 
> 
> On Mon, Feb 11, 2019 at 10:55 PM Pavel Labath <pavel at labath.sk 
> <mailto:pavel at labath.sk>> wrote:
> 
>     On 12/02/2019 01:30, Jonas Devlieghere via lldb-commits wrote:
>      > Author: jdevlieghere
>      > Date: Mon Feb 11 16:30:21 2019
>      > New Revision: 353778
>      >
>      > URL: http://llvm.org/viewvc/llvm-project?rev=353778&view=rev
>      > Log:
>      > Define _ENABLE_EXTENDED_ALIGNED_STORAGE on Windows.
>      >
>      > Apparently there are multiple places where MSVC complains about
>      > instantiations with extended aligment. I think it's better to define
>      > `_ENABLE_EXTENDED_ALIGNED_STORAGE` as suggested by the error message.
>      >
>      > I don't have access to a Windows machine so this is all speculative.
>      >
> 
>     I think it would be worth investigating where is this large alignment
>     coming from (my guess: something inside the RegisterContextDarwin
>     structs). We've had a case in the past where we were heap-allocating a
>     struct with a 64-byte alignment. MSVC would give a warning there, but
>     gcc&clang would silently under-align the result.
> 
>     In that case, the large alignment was actually unneeded, so I fixed
>     both
>     issues by just lowering it. I suspect something similar may be possible
>     here.
> 
>     pl
>