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