[lldb-dev] Passing std::atomics by value
Jim Ingham via lldb-dev
lldb-dev at lists.llvm.org
Fri Aug 26 11:40:57 PDT 2016
> 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.
> 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
More information about the lldb-dev