> On Aug 26, 2016, at 11:44 AM, Adrian McCarthy via lldb-dev > <lldb-dev@lists.llvm.org> wrote: > > Methods like Address::SetOffset and Address::Slide seem to have data races > despite m_offset being atomic. Callers of those methods would have to > guarantee that nothing else is trying to write to m_offset. And if they're > doing that, then there doesn't appear to be a need for std::atomic on that > field. > > BTW, I propose we move the member variables from protected to private. As > far as I can tell, there aren't any derived classes (yet), at least none that > access those members. If we need to add a mutex to the class itself, it'll > be better if any future derived classes access the data through accessors.
That seems fine to me. We haven't needed to subclass it in years, and probably won't. Jim > > On Fri, Aug 26, 2016 at 11:36 AM, Zachary Turner via lldb-dev > <lldb-dev@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. > > On Fri, Aug 26, 2016 at 11:29 AM Greg Clayton <gclay...@apple.com> wrote: > > > On Aug 26, 2016, at 11:24 AM, Greg Clayton via lldb-dev > > <lldb-dev@lists.llvm.org> wrote: > > > >> > >> On Aug 26, 2016, at 10:51 AM, Zachary Turner via lldb-dev > >> <lldb-dev@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev > > > _______________________________________________ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
