r361997 - [analyzer] print() JSONify: getNodeLabel implementation

Russell Gallop via cfe-commits cfe-commits at lists.llvm.org
Thu May 30 06:59:47 PDT 2019


Hi Csaba,

The test dump_egraph.cpp appears to be flaky on Windows. For example here:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26183
.

C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\Analysis\dump_egraph.cpp:23:11:
error: CHECK: expected string not found in input
// CHECK: \"store\": [\l      \{ \"cluster\":
\"t\", \"items\": [\l        \{
\"kind\": \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no
stmt, #1\}\"

Running locally, it fails after 2-5 runs for me, running:
python bin/llvm-lit.py -v ../clang/test/Analysis/dump_egraph.cpp

Please could you take a look?

Note that I'm not certain it was this commit that started the flakiness, it
is the latest which changed the failing line.

Thanks
Russ

On Wed, 29 May 2019 at 19:02, Csaba Dabis via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: charusso
> Date: Wed May 29 11:05:53 2019
> New Revision: 361997
>
> URL: http://llvm.org/viewvc/llvm-project?rev=361997&view=rev
> Log:
> [analyzer] print() JSONify: getNodeLabel implementation
>
> Summary: This patch also rewrites the ProgramPoint printing.
>
> Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus
>
> Reviewed By: NoQ
>
> Subscribers: cfe-commits, szepet, rnkovacs, a.sidorin, mikhail.ramalho,
>              donat.nagy, dkrupp
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D62346
>
> Modified:
>     cfe/trunk/include/clang/Analysis/ProgramPoint.h
>     cfe/trunk/lib/Analysis/ProgramPoint.cpp
>     cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
>     cfe/trunk/test/Analysis/dump_egraph.c
>     cfe/trunk/test/Analysis/dump_egraph.cpp
>
> Modified: cfe/trunk/include/clang/Analysis/ProgramPoint.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ProgramPoint.h?rev=361997&r1=361996&r2=361997&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Analysis/ProgramPoint.h (original)
> +++ cfe/trunk/include/clang/Analysis/ProgramPoint.h Wed May 29 11:05:53
> 2019
> @@ -213,7 +213,7 @@ public:
>      ID.AddPointer(getTag());
>    }
>
> -  void print(StringRef CR, llvm::raw_ostream &Out) const;
> +  void printJson(llvm::raw_ostream &Out, const char *NL = "\n") const;
>
>    LLVM_DUMP_METHOD void dump() const;
>
>
> Modified: cfe/trunk/lib/Analysis/ProgramPoint.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ProgramPoint.cpp?rev=361997&r1=361996&r2=361997&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Analysis/ProgramPoint.cpp (original)
> +++ cfe/trunk/lib/Analysis/ProgramPoint.cpp Wed May 29 11:05:53 2019
> @@ -43,151 +43,152 @@ ProgramPoint ProgramPoint::getProgramPoi
>  }
>
>  LLVM_DUMP_METHOD void ProgramPoint::dump() const {
> -  return print(/*CR=*/"\n", llvm::errs());
> +  return printJson(llvm::errs());
>  }
>
> -static void printLocation(raw_ostream &Out, SourceLocation SLoc,
> -                          const SourceManager &SM,
> -                          StringRef CR,
> -                          StringRef Postfix) {
> -  if (SLoc.isFileID()) {
> -    Out << CR << "line=" << SM.getExpansionLineNumber(SLoc)
> -        << " col=" << SM.getExpansionColumnNumber(SLoc) << Postfix;
> +static void printLocation(raw_ostream &Out, SourceLocation Loc,
> +                          const SourceManager &SM) {
> +  Out << "\"location\": ";
> +  if (!Loc.isFileID()) {
> +    Out << "null";
> +    return;
>    }
> +
> +  Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
> +      << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
>  }
>
> -void ProgramPoint::print(StringRef CR, llvm::raw_ostream &Out) const {
> +void ProgramPoint::printJson(llvm::raw_ostream &Out, const char *NL)
> const {
>    const ASTContext &Context =
>        getLocationContext()->getAnalysisDeclContext()->getASTContext();
>    const SourceManager &SM = Context.getSourceManager();
> +
> +  Out << "\"kind\": \"";
>    switch (getKind()) {
>    case ProgramPoint::BlockEntranceKind:
> -    Out << "Block Entrance: B"
> +    Out << "BlockEntrance\""
> +        << ", \"block_id\": "
>          << castAs<BlockEntrance>().getBlock()->getBlockID();
>      break;
>
>    case ProgramPoint::FunctionExitKind: {
>      auto FEP = getAs<FunctionExitPoint>();
> -    Out << "Function Exit: B" << FEP->getBlock()->getBlockID();
> +    Out << "FunctionExit\""
> +        << ", \"block_id\": " << FEP->getBlock()->getBlockID()
> +        << ", \"stmt_id\": ";
> +
>      if (const ReturnStmt *RS = FEP->getStmt()) {
> -      Out << CR << " Return: S" << RS->getID(Context) << CR;
> -      RS->printPretty(Out, /*helper=*/nullptr,
> Context.getPrintingPolicy(),
> -                      /*Indentation=*/2, /*NewlineSymbol=*/CR);
> +      Out << RS->getID(Context) << ", \"stmt\": \"";
> +      RS->printPretty(Out, /*Helper=*/nullptr,
> Context.getPrintingPolicy());
> +      Out << '\"';
> +    } else {
> +      Out << "null, \"stmt\": null";
>      }
>      break;
>    }
>    case ProgramPoint::BlockExitKind:
> -    assert(false);
> +    llvm_unreachable("BlockExitKind");
>      break;
> -
>    case ProgramPoint::CallEnterKind:
> -    Out << "CallEnter";
> +    Out << "CallEnter\"";
>      break;
> -
>    case ProgramPoint::CallExitBeginKind:
> -    Out << "CallExitBegin";
> +    Out << "CallExitBegin\"";
>      break;
> -
>    case ProgramPoint::CallExitEndKind:
> -    Out << "CallExitEnd";
> +    Out << "CallExitEnd\"";
>      break;
> -
>    case ProgramPoint::PostStmtPurgeDeadSymbolsKind:
> -    Out << "PostStmtPurgeDeadSymbols";
> +    Out << "PostStmtPurgeDeadSymbols\"";
>      break;
> -
>    case ProgramPoint::PreStmtPurgeDeadSymbolsKind:
> -    Out << "PreStmtPurgeDeadSymbols";
> +    Out << "PreStmtPurgeDeadSymbols\"";
>      break;
> -
>    case ProgramPoint::EpsilonKind:
> -    Out << "Epsilon Point";
> +    Out << "EpsilonPoint\"";
>      break;
>
> -  case ProgramPoint::LoopExitKind: {
> -    LoopExit LE = castAs<LoopExit>();
> -    Out << "LoopExit: " << LE.getLoopStmt()->getStmtClassName();
> +  case ProgramPoint::LoopExitKind:
> +    Out << "LoopExit\", \"stmt\": \""
> +        << castAs<LoopExit>().getLoopStmt()->getStmtClassName() << '\"';
>      break;
> -  }
>
>    case ProgramPoint::PreImplicitCallKind: {
>      ImplicitCallPoint PC = castAs<ImplicitCallPoint>();
> -    Out << "PreCall: ";
> +    Out << "PreCall\", \"stmt\": \"";
>      PC.getDecl()->print(Out, Context.getLangOpts());
> -    printLocation(Out, PC.getLocation(), SM, CR, /*Postfix=*/CR);
> +    Out << "\", ";
> +    printLocation(Out, PC.getLocation(), SM);
>      break;
>    }
>
>    case ProgramPoint::PostImplicitCallKind: {
>      ImplicitCallPoint PC = castAs<ImplicitCallPoint>();
> -    Out << "PostCall: ";
> +    Out << "PostCall\", \"stmt\": \"";
>      PC.getDecl()->print(Out, Context.getLangOpts());
> -    printLocation(Out, PC.getLocation(), SM, CR, /*Postfix=*/CR);
> +    Out << "\", ";
> +    printLocation(Out, PC.getLocation(), SM);
>      break;
>    }
>
>    case ProgramPoint::PostInitializerKind: {
> -    Out << "PostInitializer: ";
> +    Out << "PostInitializer\", ";
>      const CXXCtorInitializer *Init =
> castAs<PostInitializer>().getInitializer();
> -    if (const FieldDecl *FD = Init->getAnyMember())
> -      Out << *FD;
> -    else {
> +    if (const FieldDecl *FD = Init->getAnyMember()) {
> +      Out << "\"field_decl\": \"" << *FD << '\"';
> +    } else {
> +      Out << "\"type\": \"";
>        QualType Ty = Init->getTypeSourceInfo()->getType();
>        Ty = Ty.getLocalUnqualifiedType();
>        Ty.print(Out, Context.getLangOpts());
> +      Out << '\"';
>      }
>      break;
>    }
>
>    case ProgramPoint::BlockEdgeKind: {
>      const BlockEdge &E = castAs<BlockEdge>();
> -    Out << "Edge: (B" << E.getSrc()->getBlockID() << ", B"
> -        << E.getDst()->getBlockID() << ')';
> -
> -    if (const Stmt *T = E.getSrc()->getTerminatorStmt()) {
> -      SourceLocation SLoc = T->getBeginLoc();
> -
> -      Out << "\\|Terminator: ";
> -      E.getSrc()->printTerminator(Out, Context.getLangOpts());
> -      printLocation(Out, SLoc, SM, CR, /*Postfix=*/"");
> -
> -      if (isa<SwitchStmt>(T)) {
> -        const Stmt *Label = E.getDst()->getLabel();
> -
> -        if (Label) {
> -          if (const auto *C = dyn_cast<CaseStmt>(Label)) {
> -            Out << CR << "case ";
> -            if (C->getLHS())
> -              C->getLHS()->printPretty(
> -                  Out, nullptr, Context.getPrintingPolicy(),
> -                  /*Indentation=*/0, /*NewlineSymbol=*/CR);
> -
> -            if (const Stmt *RHS = C->getRHS()) {
> -              Out << " .. ";
> -              RHS->printPretty(Out, nullptr, Context.getPrintingPolicy(),
> -                               /*Indetation=*/0, /*NewlineSymbol=*/CR);
> -            }
> -
> -            Out << ":";
> -          } else {
> -            assert(isa<DefaultStmt>(Label));
> -            Out << CR << "default:";
> -          }
> -        } else
> -          Out << CR << "(implicit) default:";
> -      } else if (isa<IndirectGotoStmt>(T)) {
> -        // FIXME
> +    const Stmt *T = E.getSrc()->getTerminatorStmt();
> +    Out << "Edge\", \"src_id\": " << E.getSrc()->getBlockID()
> +        << ", \"dst_id\": " << E.getDst()->getBlockID()
> +        << ", \"terminator\": " << (!T ? "null, \"term_kind\": null" :
> "\"");
> +    if (!T)
> +      break;
> +
> +    E.getSrc()->printTerminator(Out, Context.getLangOpts());
> +    Out << "\", ";
> +    printLocation(Out, T->getBeginLoc(), SM);
> +    Out << ", \"term_kind\": \"";
> +
> +    if (isa<SwitchStmt>(T)) {
> +      Out << "SwitchStmt\", \"case\": ";
> +      if (const Stmt *Label = E.getDst()->getLabel()) {
> +        if (const auto *C = dyn_cast<CaseStmt>(Label)) {
> +          Out << "{ \"lhs\": ";
> +          if (const Stmt *LHS = C->getLHS())
> +            LHS->printPretty(Out, nullptr, Context.getPrintingPolicy());
> +          else
> +            Out << "null";
> +          Out << ", \"rhs\": ";
> +          if (const Stmt *RHS = C->getRHS())
> +            RHS->printPretty(Out, nullptr, Context.getPrintingPolicy());
> +          else
> +            Out << "null";
> +          Out << " }";
> +        } else {
> +          assert(isa<DefaultStmt>(Label));
> +          Out << "\"default\"";
> +        }
>        } else {
> -        Out << CR << "Condition: ";
> -        if (*E.getSrc()->succ_begin() == E.getDst())
> -          Out << "true";
> -        else
> -          Out << "false";
> +        Out << "\"implicit default\"";
>        }
> -
> -      Out << CR;
> +    } else if (isa<IndirectGotoStmt>(T)) {
> +      // FIXME: More info.
> +      Out << "IndirectGotoStmt\"";
> +    } else {
> +      Out << "Condition\", \"value\": "
> +          << (*E.getSrc()->succ_begin() == E.getDst() ? "true" : "false");
>      }
> -
>      break;
>    }
>
> @@ -195,22 +196,37 @@ void ProgramPoint::print(StringRef CR, l
>      const Stmt *S = castAs<StmtPoint>().getStmt();
>      assert(S != nullptr && "Expecting non-null Stmt");
>
> -    Out << S->getStmtClassName() << " S" << S->getID(Context) << " <"
> -        << (const void *)S << "> ";
> -    S->printPretty(Out, /*helper=*/nullptr, Context.getPrintingPolicy(),
> -                   /*Indentation=*/2, /*NewlineSymbol=*/CR);
> -    printLocation(Out, S->getBeginLoc(), SM, CR, /*Postfix=*/"");
> +    llvm::SmallString<256> TempBuf;
> +    llvm::raw_svector_ostream TempOut(TempBuf);
> +
> +    Out << "Statement\", \"stmt_kind\": \"" << S->getStmtClassName()
> +        << "\", \"stmt_id\": " << S->getID(Context)
> +        << ", \"pointer\": \"" << (const void *)S << "\", \"pretty\": ";
> +
> +    // See whether the current statement is pretty-printable.
> +    S->printPretty(TempOut, /*Helper=*/nullptr,
> Context.getPrintingPolicy());
> +    if (!TempBuf.empty()) {
> +      Out << '\"' << TempBuf.str().trim() << "\", ";
> +      TempBuf.clear();
> +    } else {
> +      Out << "null, ";
> +    }
> +
> +    printLocation(Out, S->getBeginLoc(), SM);
>
> +    Out << ", \"stmt_point_kind\": ";
>      if (getAs<PreStmt>())
> -      Out << CR << "PreStmt" << CR;
> +      Out << "\"PreStmt\"";
>      else if (getAs<PostLoad>())
> -      Out << CR << "PostLoad" << CR;
> +      Out << "\"PostLoad\"";
>      else if (getAs<PostStore>())
> -      Out << CR << "PostStore" << CR;
> +      Out << "\"PostStore\"";
>      else if (getAs<PostLValue>())
> -      Out << CR << "PostLValue" << CR;
> +      Out << "\"PostLValue\"";
>      else if (getAs<PostAllocatorCall>())
> -      Out << CR << "PostAllocatorCall" << CR;
> +      Out << "\"PostAllocatorCall\"";
> +    else
> +      Out << "null";
>
>      break;
>    }
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=361997&r1=361996&r2=361997&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed May 29 11:05:53
> 2019
> @@ -162,12 +162,12 @@ public:
>          << "\", \"argument_index\": ";
>
>      if (getItem().getKind() == ConstructionContextItem::ArgumentKind)
> -      Out << getItem().getIndex() << '\"';
> +      Out << getItem().getIndex();
>      else
>        Out << "null";
>
>      // Pretty-print
> -    Out << ", \"pretty\": \"";
> +    Out << ", \"pretty\": ";
>
>      if (S) {
>        llvm::SmallString<256> TempBuf;
> @@ -176,13 +176,13 @@ public:
>        // See whether the current statement is pretty-printable.
>        S->printPretty(TempOut, Helper, PP);
>        if (!TempBuf.empty()) {
> -        Out << TempBuf.str().trim() << '\"';
> +        Out << '\"' << TempBuf.str().trim() << '\"';
>          TempBuf.clear();
>        } else {
>          Out << "null";
>        }
>      } else {
> -      Out << I->getAnyMember()->getNameAsString() << '\"';
> +      Out << '\"' << I->getAnyMember()->getNameAsString() << '\"';
>      }
>    }
>
> @@ -3079,37 +3079,55 @@ struct DOTGraphTraits<ExplodedGraph*> :
>    }
>
>    static std::string getNodeLabel(const ExplodedNode *N, ExplodedGraph
> *G){
> -    std::string sbuf;
> -    llvm::raw_string_ostream Out(sbuf);
> +    std::string Buf;
> +    llvm::raw_string_ostream Out(Buf);
>
> +    const bool IsDot = true;
> +    const unsigned int Space = 1;
>      ProgramStateRef State = N->getState();
>
> +    Out << "{ \"node_id\": \"" << (const void *)N
> +        << "\", \"state_id\": " << State->getID()
> +        << ", \"has_report\": " << (nodeHasBugReport(N) ? "true" :
> "false")
> +        << ",\\l";
> +
> +    Indent(Out, Space, IsDot) << "\"program_points\": [\\l";
> +
>      // Dump program point for all the previously skipped nodes.
>      traverseHiddenNodes(
>          N,
>          [&](const ExplodedNode *OtherNode) {
> -          OtherNode->getLocation().print(/*CR=*/"\\l", Out);
> +          Indent(Out, Space + 1, IsDot) << "{ ";
> +          OtherNode->getLocation().printJson(Out, /*NL=*/"\\l");
> +          Out << ", \"tag\": ";
>            if (const ProgramPointTag *Tag =
> OtherNode->getLocation().getTag())
> -            Out << "\\lTag:" << Tag->getTagDescription();
> -          if (N->isSink())
> -            Out << "\\lNode is sink\\l";
> -          if (nodeHasBugReport(N))
> -            Out << "\\lBug report attached\\l";
> +            Out << '\"' << Tag->getTagDescription() << "\" }";
> +          else
> +            Out << "null }";
>          },
> -        [&](const ExplodedNode *) { Out << "\\l--------\\l"; },
> +       // Adds a comma and a new-line between each program point.
> +        [&](const ExplodedNode *) { Out << ",\\l"; },
>          [&](const ExplodedNode *) { return false; });
>
> -    Out << "\\l\\|";
> -
> -    Out << "StateID: ST" << State->getID() << ", NodeID: N" << N->getID(G)
> -        << " <" << (const void *)N << ">\\|";
> +    Out << "\\l"; // Adds a new-line to the last program point.
> +    Indent(Out, Space, IsDot) << "],\\l";
>
>      bool SameAsAllPredecessors =
>          std::all_of(N->pred_begin(), N->pred_end(), [&](const
> ExplodedNode *P) {
>            return P->getState() == State;
>          });
> -    if (!SameAsAllPredecessors)
> -      State->printDOT(Out, N->getLocationContext());
> +
> +    if (!SameAsAllPredecessors) {
> +      State->printDOT(Out, N->getLocationContext(), Space);
> +    } else {
> +      Indent(Out, Space, IsDot) << "\"program_state\": null";
> +    }
> +
> +    Out << "\\l}";
> +    if (!N->succ_empty())
> +      Out << ',';
> +    Out << "\\l";
> +
>      return Out.str();
>    }
>  };
>
> Modified: cfe/trunk/test/Analysis/dump_egraph.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dump_egraph.c?rev=361997&r1=361996&r2=361997&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/dump_egraph.c (original)
> +++ cfe/trunk/test/Analysis/dump_egraph.c Wed May 29 11:05:53 2019
> @@ -11,6 +11,10 @@ int foo() {
>  }
>
>  // CHECK: digraph "Exploded Graph" {
> -// CHECK: Edge: (B2, B1)
> -// CHECK: Block Entrance: B1
> -// CHECK: Bug report attached
> +
> +// CHECK: \"program_points\": [\l    \{ \"kind\":
> \"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null,
> \"term_kind\": null, \"tag\": null
> \}\l  ],\l  \"program_state\": null
> +
> +// CHECK: \"program_points\": [\l    \{ \"kind\":
> \"BlockEntrance\", \"block_id\": 1
> +
> +// CHECK: \"has_report\": true
> +
>
> Modified: cfe/trunk/test/Analysis/dump_egraph.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dump_egraph.cpp?rev=361997&r1=361996&r2=361997&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/dump_egraph.cpp (original)
> +++ cfe/trunk/test/Analysis/dump_egraph.cpp Wed May 29 11:05:53 2019
> @@ -16,9 +16,9 @@ void foo() {
>    T t;
>  }
>
> -// CHECK: \"constructing_objects\": [\l    \{
> \"location_context\": \"#0 Call\", \"calling\": \"foo\", \"call_line\":
> null, \"items\": [\l      \{ \"lctx_id\": 1,
> \"stmt_id\": 1155, \"kind\": \"construct into local variable\",
> \"argument_index\": null, \"pretty\": \"T t;\", \"value\": \"&t\"
> +// CHECK: \"constructing_objects\":
> [\l      \{ \"location_context\": \"#0
> Call\", \"calling\": \"foo\", \"call_line\": null, \"items\":
> [\l        \{ \"lctx_id\": 1,
> \"stmt_id\": 1155, \"kind\": \"construct into local variable\",
> \"argument_index\": null, \"pretty\": \"T t;\", \"value\": \"&t\"
>
> -// CHECK: \"constructing_objects\": [\l    \{
> \"location_context\": \"#0 Call\", \"calling\": \"T::T\", \"call_line\":
> \"16\", \"items\": [\l      \{ \"lctx_id\":
> 2, \"init_id\": 1092, \"kind\": \"construct into member variable\",
> \"argument_index\": null, \"pretty\": \"s\", \"value\": \"&t-\>s\"
> +// CHECK: \"constructing_objects\":
> [\l      \{ \"location_context\": \"#0
> Call\", \"calling\": \"T::T\", \"call_line\": \"16\", \"items\":
> [\l        \{ \"lctx_id\": 2,
> \"init_id\": 1092, \"kind\": \"construct into member variable\",
> \"argument_index\": null, \"pretty\": \"s\", \"value\": \"&t-\>s\"
>
> -// CHECK: \"store\": [\l    \{ \"cluster\": \"t\",
> \"items\": [\l      \{ \"kind\": \"Default\",
> \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"
> +// CHECK: \"store\": [\l      \{
> \"cluster\": \"t\", \"items\":
> [\l        \{ \"kind\":
> \"Default\", \"offset\": 0, \"value\": \"conj_$3\{int, LC3, no stmt, #1\}\"
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190530/e87d4d9e/attachment-0001.html>


More information about the cfe-commits mailing list