On Mar 22, 2011, at 10:24 PM, Richard Trieu wrote:

> Add a warning for when reinterpret_cast leads to undefined behavior.  Also 
> updated reinterpret_cast tests and turned off this warning for tests that 
> don't need it. 


I'm not totally convinced that this warning is useful. The issue is that we're 
not really invoking undefined behavior by performing the cast, or even 
dereferencing the result of that cast, so long as the memory we're pointing to 
actually has an object of the right type. However, that's not what this warning 
is checking: this warning is checking whether the declared type before the 
cast, and the type we're casting to, are compatible. 

Overall, I expect that this warning will have a fairly high false-positive 
rate, since programmers tend to use reinterpret_cast in very specific ways 
where they know far more about the type of the object behind the 
pointer/reference than the type system says. To really catch problems with 
reinterpret_cast, we're likely to need some stronger analysis (e.g., something 
in the static analyzer) that checks that we aren't referring to the same lvalue 
in different places with different, incompatible types.

A few minor nits below.

Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td  (revision 128089)
+++ include/clang/Basic/DiagnosticSemaKinds.td  (working copy)
@@ -2562,6 +2562,10 @@
   "indirection of non-volatile null pointer will be deleted, not trap">, 
InGroup<NullDereference>;
 def note_indirection_through_null : Note<
   "consider using __builtin_trap() or qualifying pointer with 'volatile'">;
+def warn_pointer_indirection_from_incompatible_type : Warning<
+  "dereference of type %0 that was reinterpret_cast from type %1 has undefined 
"
+  "behavior.">,
+  InGroup<DiagGroup<"undefined_reinterpret_cast">>;

We tend to use hyphens instead of underscores in the diagnostic group name, 
since it's jarring to have a mix in "-Wno-undefined_reinterpret_cast".

Index: lib/Sema/SemaCXXCast.cpp
===================================================================
--- lib/Sema/SemaCXXCast.cpp    (revision 128089)
+++ lib/Sema/SemaCXXCast.cpp    (working copy)
@@ -1249,9 +1249,22 @@
   return TC_Success;
 }
 
+// Returns false if the reinterpret_cast has undefined behavior.
+// SrcType = A, DestType = B
+// *reinterpret_cast<&B>(&A)
+// reinterpret_cast<A&>(B)
+bool Sema::CompatibleReinterpretCast(QualType SrcType,
+                                     QualType DestType) {
+  if (Context.hasSameUnqualifiedType(DestType, SrcType)) {
+    return true;
+  }
+  if (DestType->isAnyCharacterType() || DestType->isVoidType() ||
+      SrcType->isAnyCharacterType() || SrcType->isVoidType()) {
+    return true;
+  }
+  return false;
+}

        - Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to