Hi, this results in a false positive on webrtc, on this code:
https://cs.chromium.org/chromium/src/third_party/libwebp/src/enc/picture_csp_enc.c?q=picture_csp_enc.c&sq=package:chromium&dr&l=1002 The code does this: #ifdef WORDS_BIGENDIAN #define ALPHA_OFFSET 0 // uint32_t 0xff000000 is 0xff,00,00,00 in memory #else #define ALPHA_OFFSET 3 // uint32_t 0xff000000 is 0x00,00,00,ff in memory #endif // ... const uint8_t* const argb = (const uint8_t*)picture->argb; const uint8_t* const a = argb + (0 ^ ALPHA_OFFSET); const uint8_t* const r = argb + (1 ^ ALPHA_OFFSET); const uint8_t* const g = argb + (2 ^ ALPHA_OFFSET); const uint8_t* const b = argb + (3 ^ ALPHA_OFFSET); The idea is to get bytes 0, 1, 2, 3 as a, r, g, b if ALPHA_OFFSET is 0, or bytes 3, 2, 1, 0 if ALPHA_OFFSET is 3. Maybe this shouldn't fire if the rhs is a macro? (On all of Chromium, this fired 3 times; in the other 2 cases the rhs was a literal as well, and those two were true positives.) On Sun, Aug 18, 2019 at 3:13 PM David Bolvansky via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: xbolva00 > Date: Sun Aug 18 12:14:14 2019 > New Revision: 369217 > > URL: http://llvm.org/viewvc/llvm-project?rev=369217&view=rev > Log: > [Diagnostics] Diagnose misused xor as pow > > Summary: > Motivation: > https://twitter.com/jfbastien/status/1139298419988549632 > https://twitter.com/mikemx7f/status/1139335901790625793 > https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=10+%5E&search=Search > > Reviewers: jfb, rsmith, regehr, aaron.ballman > > Reviewed By: aaron.ballman > > Subscribers: lebedev.ri, Quuxplusone, erik.pilkington, riccibruno, > dexonsmith, cfe-commits > > Tags: #clang > > Differential Revision: https://reviews.llvm.org/D63423 > > Added: > cfe/trunk/test/SemaCXX/warn-xor-as-pow.cpp > Modified: > cfe/trunk/include/clang/Basic/DiagnosticGroups.td > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > cfe/trunk/lib/Sema/SemaExpr.cpp > > Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=369217&r1=369216&r2=369217&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Sun Aug 18 12:14:14 > 2019 > @@ -512,6 +512,7 @@ def CompareDistinctPointerType : DiagGro > def GNUUnionCast : DiagGroup<"gnu-union-cast">; > def GNUVariableSizedTypeNotAtEnd : > DiagGroup<"gnu-variable-sized-type-not-at-end">; > def Varargs : DiagGroup<"varargs">; > +def XorUsedAsPow : DiagGroup<"xor-used-as-pow">; > > def Unsequenced : DiagGroup<"unsequenced">; > // GCC name for -Wunsequenced > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=369217&r1=369216&r2=369217&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Aug 18 > 12:14:14 2019 > @@ -3326,6 +3326,15 @@ def warn_address_of_reference_bool_conve > "code; pointer may be assumed to always convert to true">, > InGroup<UndefinedBoolConversion>; > > +def warn_xor_used_as_pow_base_extra : Warning< > + "result of '%0' is %1; did you mean '%2' (%3)?">, > + InGroup<XorUsedAsPow>; > +def warn_xor_used_as_pow_base : Warning< > + "result of '%0' is %1; did you mean '%2'?">, > + InGroup<XorUsedAsPow>; > +def note_xor_used_as_pow_silence : Note< > + "replace expression with '%0' to silence this warning">; > + > def warn_null_pointer_compare : Warning< > "comparison of %select{address of|function|array}0 '%1' %select{not > |}2" > "equal to a null pointer is always %select{true|false}2">, > > Modified: cfe/trunk/lib/Sema/SemaExpr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=369217&r1=369216&r2=369217&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) > +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sun Aug 18 12:14:14 2019 > @@ -11011,6 +11011,107 @@ QualType Sema::CheckVectorCompareOperand > return GetSignedVectorType(vType); > } > > +static void diagnoseXorMisusedAsPow(Sema &S, ExprResult &LHS, ExprResult > &RHS, > + SourceLocation Loc) { > + // Do not diagnose macros. > + if (Loc.isMacroID()) > + return; > + > + bool Negative = false; > + const auto *LHSInt = dyn_cast<IntegerLiteral>(LHS.get()); > + const auto *RHSInt = dyn_cast<IntegerLiteral>(RHS.get()); > + > + if (!LHSInt) > + return; > + if (!RHSInt) { > + // Check negative literals. > + if (const auto *UO = dyn_cast<UnaryOperator>(RHS.get())) { > + if (UO->getOpcode() != UO_Minus) > + return; > + RHSInt = dyn_cast<IntegerLiteral>(UO->getSubExpr()); > + if (!RHSInt) > + return; > + Negative = true; > + } else { > + return; > + } > + } > + > + if (LHSInt->getValue().getBitWidth() != > RHSInt->getValue().getBitWidth()) > + return; > + > + CharSourceRange ExprRange = CharSourceRange::getCharRange( > + LHSInt->getBeginLoc(), > S.getLocForEndOfToken(RHSInt->getLocation())); > + llvm::StringRef ExprStr = > + Lexer::getSourceText(ExprRange, S.getSourceManager(), > S.getLangOpts()); > + > + CharSourceRange XorRange = > + CharSourceRange::getCharRange(Loc, S.getLocForEndOfToken(Loc)); > + llvm::StringRef XorStr = > + Lexer::getSourceText(XorRange, S.getSourceManager(), > S.getLangOpts()); > + // Do not diagnose if xor keyword/macro is used. > + if (XorStr == "xor") > + return; > + > + const llvm::APInt &LeftSideValue = LHSInt->getValue(); > + const llvm::APInt &RightSideValue = RHSInt->getValue(); > + const llvm::APInt XorValue = LeftSideValue ^ RightSideValue; > + > + std::string LHSStr = Lexer::getSourceText( > + CharSourceRange::getTokenRange(LHSInt->getSourceRange()), > + S.getSourceManager(), S.getLangOpts()); > + std::string RHSStr = Lexer::getSourceText( > + CharSourceRange::getTokenRange(RHSInt->getSourceRange()), > + S.getSourceManager(), S.getLangOpts()); > + > + int64_t RightSideIntValue = RightSideValue.getSExtValue(); > + if (Negative) { > + RightSideIntValue = -RightSideIntValue; > + RHSStr = "-" + RHSStr; > + } > + > + StringRef LHSStrRef = LHSStr; > + StringRef RHSStrRef = RHSStr; > + // Do not diagnose binary, hexadecimal, octal literals. > + if (LHSStrRef.startswith("0b") || LHSStrRef.startswith("0B") || > + RHSStrRef.startswith("0b") || RHSStrRef.startswith("0B") || > + LHSStrRef.startswith("0x") || LHSStrRef.startswith("0X") || > + RHSStrRef.startswith("0x") || RHSStrRef.startswith("0X") || > + (LHSStrRef.size() > 1 && LHSStrRef.startswith("0")) || > + (RHSStrRef.size() > 1 && RHSStrRef.startswith("0"))) > + return; > + > + if (LeftSideValue == 2 && RightSideIntValue >= 0) { > + std::string SuggestedExpr = "1 << " + RHSStr; > + bool Overflow = false; > + llvm::APInt One = (LeftSideValue - 1); > + llvm::APInt PowValue = One.sshl_ov(RightSideValue, Overflow); > + if (Overflow) { > + if (RightSideIntValue < 64) > + S.Diag(Loc, diag::warn_xor_used_as_pow_base) > + << ExprStr << XorValue.toString(10, true) << ("1LL << " + > RHSStr) > + << FixItHint::CreateReplacement(ExprRange, "1LL << " + > RHSStr); > + else > + // TODO: 2 ^ 64 - 1 > + return; > + } else { > + S.Diag(Loc, diag::warn_xor_used_as_pow_base_extra) > + << ExprStr << XorValue.toString(10, true) << SuggestedExpr > + << PowValue.toString(10, true) > + << FixItHint::CreateReplacement( > + ExprRange, (RightSideIntValue == 0) ? "1" : > SuggestedExpr); > + } > + > + S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0x2 ^ " + > RHSStr); > + } else if (LeftSideValue == 10) { > + std::string SuggestedValue = "1e" + std::to_string(RightSideIntValue); > + S.Diag(Loc, diag::warn_xor_used_as_pow_base) > + << ExprStr << XorValue.toString(10, true) << SuggestedValue > + << FixItHint::CreateReplacement(ExprRange, SuggestedValue); > + S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0xA ^ " + > RHSStr); > + } > +} > + > QualType Sema::CheckVectorLogicalOperands(ExprResult &LHS, ExprResult > &RHS, > SourceLocation Loc) { > // Ensure that either both operands are of the same vector type, or > @@ -11054,6 +11155,9 @@ inline QualType Sema::CheckBitwiseOperan > if (Opc == BO_And) > diagnoseLogicalNotOnLHSofCheck(*this, LHS, RHS, Loc, Opc); > > + if (Opc == BO_Xor) > + diagnoseXorMisusedAsPow(*this, LHS, RHS, Loc); > + > ExprResult LHSResult = LHS, RHSResult = RHS; > QualType compType = UsualArithmeticConversions(LHSResult, RHSResult, > IsCompAssign); > @@ -17640,4 +17744,4 @@ ExprResult Sema::ActOnObjCAvailabilityCh > > return new (Context) > ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy); > -} > +} > \ No newline at end of file > > Added: cfe/trunk/test/SemaCXX/warn-xor-as-pow.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-xor-as-pow.cpp?rev=369217&view=auto > > ============================================================================== > --- cfe/trunk/test/SemaCXX/warn-xor-as-pow.cpp (added) > +++ cfe/trunk/test/SemaCXX/warn-xor-as-pow.cpp Sun Aug 18 12:14:14 2019 > @@ -0,0 +1,105 @@ > +// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-used-as-pow %s > +// RUN: %clang_cc1 -x c -fsyntax-only -verify %s > +// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s > 2>&1 | FileCheck %s > + > +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wxor-used-as-pow %s > +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s > +// RUN: %clang_cc1 -x c++ -fsyntax-only -fdiagnostics-parseable-fixits %s > 2>&1 | FileCheck %s > + > +#define FOOBAR(x, y) (x * y) > +#define XOR(x, y) (x ^ y) > +#define TWO 2 > +#define TEN 10 > +#define TWO_ULL 2ULL > +#define EPSILON 10 ^ -300 > + > +#define flexor 7 > + > +#ifdef __cplusplus > +constexpr long long operator"" _xor(unsigned long long v) { return v; } > + > +constexpr long long operator"" _0b(unsigned long long v) { return v; } > +constexpr long long operator"" _0X(unsigned long long v) { return v; } > +#else > +#define xor ^ // iso646.h > +#endif > + > +void test(unsigned a, unsigned b) { > + unsigned res; > + res = a ^ 5; > + res = 2 ^ b; > + res = a ^ b; > + res = 2 ^ -1; > + res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2; did you mean > '1 << 0' (1)?}} > + // CHECK: > fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1" > + // expected-note@-2 {{replace expression with '0x2 ^ 0' > to silence this warning}} > + res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3; did you mean > '1 << 1' (2)?}} > + // CHECK: > fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 1" > + // expected-note@-2 {{replace expression with '0x2 ^ 1' > to silence this warning}} > + res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0; did you mean > '1 << 2' (4)?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 2" > + // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence > this warning}} > + res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10; did you > mean '1 << 8' (256)?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 8" > + // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence > this warning}} > + res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10; did you > mean '1 << 8' (256)?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << 8" > + // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence > this warning}} > + res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18; did you > mean '1 << 16' (65536)?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1 << 16" > + // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence > this warning}} > + res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8; did you > mean '1 << TEN' (1024)?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << TEN" > + // expected-note@-2 {{replace expression with '0x2 ^ TEN' to silence > this warning}} > + res = 0x2 ^ 16; > + res = 2 xor 16; > + > + res = 2 ^ 0x4; > + res = 2 ^ 04; > + res = 0x2 ^ 10; > + res = 0X2 ^ 10; > + res = 02 ^ 10; > + res = FOOBAR(2, 16); > + res = 0b10 ^ 16; > + res = 0B10 ^ 16; > + res = 2 ^ 0b100; > + res = XOR(2, 16); > + unsigned char two = 2; > + res = two ^ 16; > + res = TWO_ULL ^ 16; > + res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; did you > mean '1LL << 32'?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL << 32" > + // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence > this warning}} > + res = 2 ^ 64; > + > + res = EPSILON; > + res = 10 ^ 0; // expected-warning {{result of '10 ^ 0' is 10; did you > mean '1e0'?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e0" > + // expected-note@-2 {{replace expression with '0xA ^ 0' to silence > this warning}} > + res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11; did you > mean '1e1'?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e1" > + // expected-note@-2 {{replace expression with '0xA ^ 1' to silence > this warning}} > + res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8; did you > mean '1e2'?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e2" > + // expected-note@-2 {{replace expression with '0xA ^ 2' to silence > this warning}} > + res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14; did you > mean '1e4'?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e4" > + // expected-note@-2 {{replace expression with '0xA ^ 4' to silence > this warning}} > + res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0; did you > mean '1e10'?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1e10" > + // expected-note@-2 {{replace expression with '0xA ^ 10' to silence > this warning}} > + res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0; did > you mean '1e10'?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e10" > + // expected-note@-2 {{replace expression with '0xA ^ 10' to silence > this warning}} > + res = 10 ^ 100; // expected-warning {{result of '10 ^ 100' is 110; did > you mean '1e100'?}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e100" > + // expected-note@-2 {{replace expression with '0xA ^ 100' to silence > this warning}} > + res = 0xA ^ 10; > + res = 10 xor 10; > +#ifdef __cplusplus > + res = 10 ^ 5_xor; > + res = 10_xor ^ 5; > + res = 10 ^ 5_0b; > + res = 10_0X ^ 5; > +#endif > +} > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits