[clang] avoid strict aliasing violation in assert

[rlibby]

Summary

StoredDeclsList::setOnlyValue wants to assert that the binary value
actually stored in the member PointerUnion Data is the same as the raw
NamedDecl pointer argument. However the way in which it was checking
this involved a cast which created a type-punned pointer, and
dereferencing that pointer broke strict aliasing rules.

I have a build of clang by gcc where gcc emitted code to read from the
pointer for the assert before writing to it and so caused the assertion
to fail.

I'm not sure this is the best alternative formulation for the assert.
Please feel free to suggest another spelling as appropriate.

[dim]

clang/include/clang/AST/DeclContextInternals.h

102

Hm, I don't have much experience with the PointerUnion class, but from a cursory look at llvm/include/llvm/ADT/PointerUnion.h, I would rather say that using dyn_cast here would be more appropriate? E.g. that definition literally says:

/// Returns the current pointer if it is of the specified pointer type,
/// otherwises returns null.

So something like:

assert(Data.dyn_cast<NameDecl> == ND &&
  "PointerUnion mangles the NamedDecl pointer!");

or even using the existing getAsDecl member function, which does exactly the same:

assert(Data.getAsDecl() == ND &&
  "PointerUnion mangles the NamedDecl pointer!");

Or is this particular check about the type being *only* the base class NamedDecl, and not any derived class?

[rlibby]

I am no expert here so I will defer, but I believe those suggestions are weaker assertions.

I believe it's really trying to assert that the NamedDecl type is the first template type in the point union, and is represented by a 0 bit in the addr, and that the pointer was appropriately aligned. So I tried to
choose something that would fail if those were not true. I think that the dyn_cast<> approach will not fail appropriately if the NamedDecl type is not first.

clang/include/clang/AST/DeclContextInternals.h

void setHasExternalDecls() {
     }
   }
 
   void setOnlyValue(NamedDecl *ND) {
     assert(!getAsVector() && "Not inline");
     Data = ND;
     // Make sure that Data is a plain NamedDecl* so we can use its address
     // at getLookupResult.
-    assert(*(NamedDecl **)&Data == ND &&
+    assert(Data.getAddrOfPtr1() && *Data.getAddrOfPtr1() == ND &&
            "PointerUnion mangles the NamedDecl pointer!");
   }
 
   void remove(NamedDecl *D) {
     assert(!isNull() && "removing from empty list");
     if (NamedDecl *Singleton = getAsDecl()) {
       assert(Singleton == D && "list is different singleton");
       (void)Singleton;