[clang-tools-extra] r298421 - Prevent cppcoreguidelines-pro-bounds-array-to-pointer-decay from diagnosing array to pointer decay stemming from system macros.
Breno Guimarães via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 21 18:57:10 PDT 2017
Hey guys,
I'm sorry the test did not make the decay explicit. It's indeed sort of
tricky: https://bugs.llvm.org/show_bug.cgi?id=32239
Basically, assert is expanded into:
(__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e)
Well, __func__ and __FILE__ are const char[], while __assert_rtn
expects const char*.
I've also seen this in my own environment (Linux) with slightly
different function names, but basically the same problem.
Any ideas of how to rewrite the test (or the code?) to have the change pushed?
Thanks!
Breno G.
On Tue, Mar 21, 2017 at 10:36 PM, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Thank you!
>
> On 21 March 2017 at 18:22, Aaron Ballman <aaron at aaronballman.com> wrote:
>
>> On Tue, Mar 21, 2017 at 9:15 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> > On 21 March 2017 at 12:01, Aaron Ballman via cfe-commits
>> > <cfe-commits at lists.llvm.org> wrote:
>> >>
>> >> Author: aaronballman
>> >> Date: Tue Mar 21 14:01:17 2017
>> >> New Revision: 298421
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=298421&view=rev
>> >> Log:
>> >> Prevent cppcoreguidelines-pro-bounds-array-to-pointer-decay from
>> >> diagnosing array to pointer decay stemming from system macros.
>> >>
>> >> Patch by Breno Rodrigues Guimaraes.
>> >>
>> >> Modified:
>> >>
>> >> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoun
>> dsArrayToPointerDecayCheck.cpp
>> >>
>> >> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pr
>> o-bounds-array-to-pointer-decay.cpp
>> >>
>> >> Modified:
>> >> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoun
>> dsArrayToPointerDecayCheck.cpp
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayChe
>> ck.cpp?rev=298421&r1=298420&r2=298421&view=diff
>> >>
>> >> ============================================================
>> ==================
>> >> ---
>> >> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoun
>> dsArrayToPointerDecayCheck.cpp
>> >> (original)
>> >> +++
>> >> clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoun
>> dsArrayToPointerDecayCheck.cpp
>> >> Tue Mar 21 14:01:17 2017
>> >> @@ -47,6 +47,25 @@ AST_MATCHER_P(Expr, hasParentIgnoringImp
>> >> return InnerMatcher.matches(*E, Finder, Builder);
>> >> }
>> >>
>> >> +AST_MATCHER(ImplicitCastExpr, isArrayToPointerDecay) {
>> >> + return Node.getCastKind() == CK_ArrayToPointerDecay;
>> >> +}
>> >> +
>> >> +AST_MATCHER(ImplicitCastExpr, sysSymbolDecayInSysHeader) {
>> >> + auto &SM = Finder->getASTContext().getSourceManager();
>> >> + if (SM.isInSystemMacro(Node.getLocStart())) {
>> >> + if (isa<PredefinedExpr>(Node.getSubExpr()))
>> >> + return true;
>> >> +
>> >> + if (const auto *SymbolDeclRef =
>> >> dyn_cast<DeclRefExpr>(Node.getSubExpr())) {
>> >> + const ValueDecl *SymbolDecl = SymbolDeclRef->getDecl();
>> >> + if (SymbolDecl && SM.isInSystemHeader(SymbolDecl
>> ->getLocation()))
>> >> + return true;
>> >> + }
>> >> + }
>> >> + return false;
>> >> +}
>> >> +
>> >> void ProBoundsArrayToPointerDecayCheck::registerMatchers(MatchFinder
>> >> *Finder) {
>> >> if (!getLangOpts().CPlusPlus)
>> >> return;
>> >> @@ -56,10 +75,12 @@ void ProBoundsArrayToPointerDecayCheck::
>> >> // 2) inside a range-for over an array
>> >> // 3) if it converts a string literal to a pointer
>> >> Finder->addMatcher(
>> >> - implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
>> >> + implicitCastExpr(isArrayToPointerDecay(),
>> >> + unless(hasParent(arraySubscriptExpr())),
>> >>
>> >> unless(hasParentIgnoringImpCasts(explicitCastExpr())),
>> >> unless(isInsideOfRangeBeginEndStmt()),
>> >> - unless(hasSourceExpression(stringLiteral())))
>> >> + unless(hasSourceExpression(stringLiteral())),
>> >> + unless(sysSymbolDecayInSysHeader()))
>> >> .bind("cast"),
>> >> this);
>> >> }
>> >> @@ -67,8 +88,6 @@ void ProBoundsArrayToPointerDecayCheck::
>> >> void ProBoundsArrayToPointerDecayCheck::check(
>> >> const MatchFinder::MatchResult &Result) {
>> >> const auto *MatchedCast =
>> >> Result.Nodes.getNodeAs<ImplicitCastExpr>("cast");
>> >> - if (MatchedCast->getCastKind() != CK_ArrayToPointerDecay)
>> >> - return;
>> >>
>> >> diag(MatchedCast->getExprLoc(), "do not implicitly decay an array
>> into
>> >> a "
>> >> "pointer; consider using
>> >> gsl::array_view or "
>> >>
>> >> Modified:
>> >> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pr
>> o-bounds-array-to-pointer-decay.cpp
>> >> URL:
>> >> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-
>> pointer-decay.cpp?rev=298421&r1=298420&r2=298421&view=diff
>> >>
>> >> ============================================================
>> ==================
>> >> ---
>> >> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pr
>> o-bounds-array-to-pointer-decay.cpp
>> >> (original)
>> >> +++
>> >> clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pr
>> o-bounds-array-to-pointer-decay.cpp
>> >> Tue Mar 21 14:01:17 2017
>> >> @@ -1,4 +1,5 @@
>> >> // RUN: %check_clang_tidy %s
>> >> cppcoreguidelines-pro-bounds-array-to-pointer-decay %t
>> >> +#include <assert.h>
>> >
>> >
>> > This test is using a header that we do not supply (unlike stddef.h which
>> > Clang provides itself). That does not seem especially reasonable to me;
>> this
>> > test is failing for us as a result. Can you supply a fake <assert.h>
>> system
>> > header as an input to this test?
>> >
>> >>
>> >> #include <stddef.h>
>> >>
>> >> namespace gsl {
>> >> @@ -34,6 +35,11 @@ void f() {
>> >>
>> >> for (auto &e : a) // OK, iteration internally decays array to
>> pointer
>> >> e = 1;
>> >> +
>> >> + assert(false); // OK, array decay inside system header macro
>> >
>> >
>> > Huh? What decay is this referring to?
>>
>> I am now wondering the same question...
>>
>> I've rolled this back r298470 and will follow up with the author.
>>
>> Thanks!
>>
>> ~Aaron
>>
>> >
>> >> +
>> >> + assert(a);
>> >> + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not implicitly decay
>> an
>> >> array into a pointer; consider using gsl::array_view or an explicit
>> cast
>> >> instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
>> >> }
>> >>
>> >> const char *g() {
>> >>
>> >>
>> >> _______________________________________________
>> >> cfe-commits mailing list
>> >> cfe-commits at lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >
>> >
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
--
Breno Rodrigues Guimarães
Universidade Federal de Minas Gerais - UFMG, Brasil
(Federal University of Minas Gerais, Brazil)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170321/b96bf831/attachment.html>
More information about the cfe-commits
mailing list