dblaikie added a comment.

In D81865#2176148 <https://reviews.llvm.org/D81865#2176148>, @froydnj wrote:

> In D81865#2176014 <https://reviews.llvm.org/D81865#2176014>, @bkramer wrote:
>
> > Nice, those relocations have annoyed me for years. I'm worried about 
> > whether the way you're accessing StaticDiagInfoDescriptionStringTable might 
> > be undefined behavior. I won't block this change on that though.
>
>
> Is there somebody who should review that particular bit?  My understanding is 
> that it is OK, but my understanding of the C++ standard is not necessarily 
> complete, and I'd like to get a second opinion.


I believe this falls under the (using cppreference ( 
https://en.cppreference.com/w/cpp/language/union ) , since it's a bit easier to 
read) UB clause: " It's undefined behavior to read from the member of the union 
that wasn't most recently written. Many compilers implement, as a non-standard 
language extension, the ability to read inactive members of a union."

Last member written to was the "StringTable" member, but then it's read from 
the "String" member, so that'd be UB. Commonly supported, but UB - not sure if 
we have a general statement that we depend on this behavior in LLVM, even 
though it's non-standard (but it's possible that we do make such an assumption 
about the compiler that's building LLVM). It'd be nice to avoid that, though - 
and not too difficult, I think - I /believe/ it's valid to take a pointer to an 
object, cast it to char*, compute a pointer to some specific member and then 
cast it back to the right type and access. But I could be wrong there. @rsmith 
would be the person to give an authoritative answer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81865/new/

https://reviews.llvm.org/D81865



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to