[PATCH] D71827: [clang] avoid strict aliasing violation in assert

[Ryan Libby via Phabricator via cfe-commits]

rlibby added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71827



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

[PATCH] D71827: [clang] avoid strict aliasing violation in assert

[Ryan Libby via Phabricator via cfe-commits]

rlibby created this revision.
rlibby added reviewers: akyrtzi, rsmith.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71827

Files:
  clang/include/clang/AST/DeclContextInternals.h


Index: clang/include/clang/AST/DeclContextInternals.h
===
--- clang/include/clang/AST/DeclContextInternals.h
+++ clang/include/clang/AST/DeclContextInternals.h
@@ -99,7 +99,7 @@
 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!");
   }
 


Index: clang/include/clang/AST/DeclContextInternals.h
===
--- clang/include/clang/AST/DeclContextInternals.h
+++ clang/include/clang/AST/DeclContextInternals.h
@@ -99,7 +99,7 @@
 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!");
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits