[lldb-dev] Passing std::atomics by value

[Ted Woodward via lldb-dev]

Would the misalignment really be a problem on x86? I thought the core handled misaligned loads and stores. Unlike, say, PPC.

Either way, I'd expect the compiler to automatically align a uint64.

--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project


-----Original Message-----
From: lldb-dev [mailto:lldb-dev-bounces at lists.llvm.org] On Behalf Of Jim Ingham via lldb-dev
Sent: Friday, August 26, 2016 2:32 PM
To: Zachary Turner <zturner at google.com>
Cc: LLDB <lldb-dev at lists.llvm.org>
Subject: Re: [lldb-dev] Passing std::atomics by value

That seems to me a compiler bug, not: "You can't pass structures with std::atomic" elements in them by value.  It would crazy to add a type to the C++ language that you couldn't use everywhere.  There doesn't seem to be any official restriction.  And anecdotally there's a reference to the problem in gcc for PPC (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62259) but that was considered a bug and fixed.

Not that it matters much, you are stuck with the compiler you're stuck with, bugs and all.

Jim

> On Aug 26, 2016, at 12:26 PM, Zachary Turner <zturner at google.com> wrote:
> 
> It could, in theory, it just doesn't.  I compiled a quick program using i686-darwin as the target and the generated assembly makes no attempt to pad the arguments.
> 
> I'll post some code later
> 
> On Fri, Aug 26, 2016 at 11:42 AM Jim Ingham <jingham at apple.com> wrote:
> 
> > On Aug 26, 2016, at 11:36 AM, Zachary Turner via lldb-dev <lldb-dev at lists.llvm.org> wrote:
> >
> > The thing is, the code is already full of data races.  I think the std::atomic is actually used incorrectly and is not even doing anything.
> >
> > That said, consider darwin on 32-bit, where I believe each stack frame is 4-byte aligned.  I dont' think there's any way the compiler can guarantee that a function parameter is 8-byte aligned without allocating from the heap, which is obviously impossible for a stack variable.
> 
> Why can't the compiler pad the argument slot on the stack so that the actual struct lives at a properly aligned location?  It's copying the value into the callee's stack frame, so it can put it wherever it wants.  And both caller and callee sites know the alignment requirements from the function signature, so they can both figure out where the actual struct lives in the argument slot based on the alignment of the stack slot.
> 
> Jim
> 
> 
> >
> > On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton <gclayton at apple.com> wrote:
> >
> > > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev <lldb-dev at lists.llvm.org> wrote:
> > >
> > >>
> > >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev <lldb-dev at lists.llvm.org> wrote:
> > >>
> > >> I recently updated to Visual Studio 2015 Update 3, which has improved its diagnostics.  As a result of this, LLDB is uncompilable due to a slew of errors of the following nature:
> > >>
> > >> D:\src\llvm\tools\lldb\include\lldb/Target/Process.h(3256): error C2719: 'default_stop_addr': formal parameter with requested alignment of 8 won't be aligned
> > >>
> > >> The issue comes down to the fact that lldb::Address contains a std::atomic<uint64_t>, and is being passed by value pervasively throughout the codebase.  There is no way to guarantee that this value is 8 byte aligned.  This has always been a bug, but until now the compiler just hasn't been reporting it.
> > >>
> > >> Someone correct me if I'm wrong, but I believe this is a problem on any 32-bit platform, and MSVC is just the only one erroring.
> > >>
> > >> I'm not really sure what to do about this.  Passing std::atomic<uint64>'s by value seems wrong to me.
> > >>
> > >> Looking at the code, I don't even know why it needs to be atomic.  It's not even being used safely.  We'll have a single function write the value and later read the value, even though it could have been used in the meantime. Maybe what is really intended is a mutex.  Or maybe it doesn't need to be atomic in the first place.
> > >>
> > >> Does anyone have a suggestion on what to do about this?  I'm currently blocked on this as I can't compile LLDB.
> > >
> > > Feel free to #ifdef around the m_offset member of Address and you can have the data race and compile. This seems like a compiler bug to me. If a struct/classe by value argument has alignment requirements, then the compiler should handle this correctly IMHO. Am I wrong????
> >
> > Or if this isn't a compiler bug, feel free to modify anything that was passing Address by value and make it a "const Address &".
> > _______________________________________________
> > lldb-dev mailing list
> > lldb-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> 

_______________________________________________
lldb-dev mailing list
lldb-dev at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev