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

[Jonas Devlieghere via lldb-commits]

Thanks!

On Fri, Feb 15, 2019 at 04:22 Pavel Labath <pavel at labath.sk> wrote:

> 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
> >
>
> --
Sent from my iPhone
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190215/78e1a444/attachment.html>