<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Nov 21, 2014 at 1:03 PM, Roman Divacky <span dir="ltr"><<a href="mailto:rdivacky@freebsd.org" target="_blank">rdivacky@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rdivacky<br>
Date: Fri Nov 21 15:03:10 2014<br>
New Revision: 222568<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=222568&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=222568&view=rev</a><br>
Log:<br>
Implement -Wcast-qual, fixing #13772.<br>
<br>
Many thanks to dblaikie for his advices and suggestions!<br>
<br>
Added:<br>
cfe/trunk/test/Sema/warn-cast-qual.c<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
cfe/trunk/lib/Sema/SemaCast.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=222568&r1=222567&r2=222568&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=222568&r1=222567&r2=222568&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Nov 21 15:03:10 2014<br>
@@ -60,7 +60,7 @@ def ExternCCompat : DiagGroup<"extern-c-<br>
def KeywordCompat : DiagGroup<"keyword-compat">;<br>
def GNUCaseRange : DiagGroup<"gnu-case-range">;<br>
def CastAlign : DiagGroup<"cast-align">;<br>
-def : DiagGroup<"cast-qual">;<br>
+def CastQual : DiagGroup<"cast-qual">;<br>
def : DiagGroup<"char-align">;<br>
def Comment : DiagGroup<"comment">;<br>
def GNUComplexInteger : DiagGroup<"gnu-complex-integer">;<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=222568&r1=222567&r2=222568&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=222568&r1=222567&r2=222568&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov 21 15:03:10 2014<br>
@@ -6115,6 +6115,11 @@ def warn_zero_size_struct_union_compat :<br>
def warn_zero_size_struct_union_in_extern_c : Warning<"%select{|empty }0"<br>
"%select{struct|union}1 has size 0 in C, %select{size 1|non-zero size}2 in C++">,<br>
InGroup<ExternCCompat>;<br>
+def warn_cast_qual : Warning<"cast from %0 to %1 drops %select{const and "<br>
+ "volatile qualifiers|const qualifier|volatile qualifier}2">,<br>
+ InGroup<CastQual>, DefaultIgnore;<br>
+def warn_cast_qual2 : Warning<"cast from %0 to %1 must have all intermediate "<br>
+ "pointers const qualified to be safe">, InGroup<CastQual>, DefaultIgnore;<br></blockquote><div><br></div><div>I think this is overstating the point; there are safe casts that do not have 'const' added in all intermediate points. It's a bit awkward, but we could use something like this:</div><div><br></div><div>"cast from pointer to %0 to pointer to non-const type %1 is not const-correct; storing a value through the resulting pointer may make the original pointer point to an object that is not of type %0"</div><div><br></div><div>Also, can you give this diagnostic a better name, like warn_cast_qual_indirect or warn_cast_qual_non_const_intermediate?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
} // End of general sema category.<br>
<br>
// inline asm.<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaCast.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=222568&r1=222567&r2=222568&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=222568&r1=222567&r2=222568&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaCast.cpp Fri Nov 21 15:03:10 2014<br>
@@ -142,9 +142,6 @@ namespace {<br>
};<br>
}<br>
<br>
-static bool CastsAwayConstness(Sema &Self, QualType SrcType, QualType DestType,<br>
- bool CheckCVR, bool CheckObjCLifetime);<br>
-<br>
// The Try functions attempt a specific way of casting. If they succeed, they<br>
// return TC_Success. If their way of casting is not appropriate for the given<br>
// arguments, they return TC_NotApplicable and *may* set diag to a diagnostic<br>
@@ -462,7 +459,10 @@ static bool UnwrapDissimilarPointerTypes<br>
/// \param CheckObjCLifetime Whether to check Objective-C lifetime qualifiers.<br>
static bool<br>
CastsAwayConstness(Sema &Self, QualType SrcType, QualType DestType,<br>
- bool CheckCVR, bool CheckObjCLifetime) {<br>
+ bool CheckCVR, bool CheckObjCLifetime,<br>
+ QualType *TheOffendingSrcType = nullptr,<br>
+ QualType *TheOffendingDestType = nullptr,<br>
+ Qualifiers *CastAwayQualifiers = nullptr) {<br>
// If the only checking we care about is for Objective-C lifetime qualifiers,<br>
// and we're not in ARC mode, there's nothing to check.<br>
if (!CheckCVR && CheckObjCLifetime &&<br>
@@ -487,6 +487,8 @@ CastsAwayConstness(Sema &Self, QualType<br>
// Find the qualifiers. We only care about cvr-qualifiers for the<br>
// purpose of this check, because other qualifiers (address spaces,<br>
// Objective-C GC, etc.) are part of the type's identity.<br>
+ QualType PrevUnwrappedSrcType = UnwrappedSrcType;<br>
+ QualType PrevUnwrappedDestType = UnwrappedDestType;<br>
while (UnwrapDissimilarPointerTypes(UnwrappedSrcType, UnwrappedDestType)) {<br>
// Determine the relevant qualifiers at this level.<br>
Qualifiers SrcQuals, DestQuals;<br>
@@ -497,6 +499,13 @@ CastsAwayConstness(Sema &Self, QualType<br>
if (CheckCVR) {<br>
RetainedSrcQuals.setCVRQualifiers(SrcQuals.getCVRQualifiers());<br>
RetainedDestQuals.setCVRQualifiers(DestQuals.getCVRQualifiers());<br>
+<br>
+ if (RetainedSrcQuals != RetainedDestQuals && TheOffendingSrcType &&<br>
+ TheOffendingDestType && CastAwayQualifiers) {<br>
+ *TheOffendingSrcType = PrevUnwrappedSrcType;<br>
+ *TheOffendingDestType = PrevUnwrappedDestType;<br>
+ *CastAwayQualifiers = RetainedSrcQuals - RetainedDestQuals;<br>
+ }<br>
}<br>
<br>
if (CheckObjCLifetime &&<br>
@@ -505,6 +514,9 @@ CastsAwayConstness(Sema &Self, QualType<br>
<br>
cv1.push_back(RetainedSrcQuals);<br>
cv2.push_back(RetainedDestQuals);<br>
+<br>
+ PrevUnwrappedSrcType = UnwrappedSrcType;<br>
+ PrevUnwrappedDestType = UnwrappedDestType;<br>
}<br>
if (cv1.empty())<br>
return false;<br>
@@ -2371,6 +2383,30 @@ void CastOperation::CheckCStyleCast() {<br>
<br>
if (Kind == CK_BitCast)<br>
checkCastAlign();<br>
+<br>
+ // -Wcast-qual<br>
+ QualType TheOffendingSrcType, TheOffendingDestType;<br>
+ Qualifiers CastAwayQualifiers;<br>
+ if (SrcType->isAnyPointerType() && DestType->isAnyPointerType() &&<br>
+ CastsAwayConstness(Self, SrcType, DestType, true, false,<br>
+ &TheOffendingSrcType, &TheOffendingDestType,<br>
+ &CastAwayQualifiers)) {<br>
+ int qualifiers = -1;<br>
+ if (CastAwayQualifiers.hasConst() && CastAwayQualifiers.hasVolatile()) {<br>
+ qualifiers = 0;<br>
+ } else if (CastAwayQualifiers.hasConst()) {<br>
+ qualifiers = 1;<br>
+ } else if (CastAwayQualifiers.hasVolatile()) {<br>
+ qualifiers = 2;<br>
+ }<br>
+ // This is a variant of int **x; const int **y = (const int **)x;<br>
+ if (qualifiers == -1)<br>
+ Self.Diag(SrcExpr.get()->getLocStart(), diag::warn_cast_qual2) <<<br>
+ SrcType << DestType;<br>
+ else<br>
+ Self.Diag(SrcExpr.get()->getLocStart(), diag::warn_cast_qual) <<<br>
+ TheOffendingSrcType << TheOffendingDestType << qualifiers;<br>
+ }<br>
}<br>
<br>
ExprResult Sema::BuildCStyleCastExpr(SourceLocation LPLoc,<br>
<br>
Added: cfe/trunk/test/Sema/warn-cast-qual.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-cast-qual.c?rev=222568&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-cast-qual.c?rev=222568&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/warn-cast-qual.c (added)<br>
+++ cfe/trunk/test/Sema/warn-cast-qual.c Fri Nov 21 15:03:10 2014<br>
@@ -0,0 +1,29 @@<br>
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -Wcast-qual -verify %s<br>
+<br>
+#include <stdint.h><br>
+<br>
+void foo() {<br>
+ const char * const ptr = 0;<br>
+ const char * const *ptrptr = 0;<br>
+ char *y = (char *)ptr; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}<br>
+ char **y1 = (char **)ptrptr; // expected-warning {{cast from 'const char *const' to 'char *' drops const qualifier}}<br></blockquote><div><br></div><div>I don't think this wording is precise enough -- we are not casting from 'const char *const' to 'char*', we are casting from 'const char *const *' to 'char**'. If you want to strip off layers of pointers here, you should indicate you did so in the diagnostic wording: "cast from pointer to 'const char * const' to pointer to 'char *' drops const qualifier" would be better.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ const char **y2 = (const char **)ptrptr; // expected-warning {{cast from 'const char *const *' to 'const char **' drops const qualifier}}<br>
+<br>
+ char *z = (char *)(uintptr_t)(const void *)ptr; // no warning<br>
+ char *z1 = (char *)(const void *)ptr; // expected-warning {{cast from 'const void *' to 'char *' drops const qualifier}}<br>
+<br>
+ volatile char *vol = 0;<br>
+ char *vol2 = (char *)vol; // expected-warning {{cast from 'volatile char *' to 'char *' drops volatile qualifier}}<br>
+ const volatile char *volc = 0;<br>
+ char *volc2 = (char *)volc; // expected-warning {{cast from 'const volatile char *' to 'char *' drops const and volatile qualifiers}}<br>
+<br>
+ int **intptrptr;<br>
+ const int **intptrptrc = (const int **)intptrptr; // expected-warning {{cast from 'int **' to 'const int **' must have all intermediate pointers const qualified}}<br>
+ volatile int **intptrptrv = (volatile int **)intptrptr; // expected-warning {{cast from 'int **' to 'volatile int **' must have all intermediate pointers const qualified}}<br>
+<br>
+ int *intptr;<br>
+ const int *intptrc = (const int *)intptr; // no warning<br>
+<br>
+ const char **charptrptrc;<br>
+ char **charptrptr = (char **)charptrptrc; // expected-warning {{cast from 'const char *' to 'char *' drops const qualifier}}<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>