r371326 - [NewPM][Sancov] Create the Sancov Pass after building the pipelines

2019-09-08 Thread Leonard Chan via cfe-commits 
Author: leonardchan
Date: Sun Sep  8 00:30:17 2019
New Revision: 371326

URL: http://llvm.org/viewvc/llvm-project?rev=371326&view=rev
Log:
[NewPM][Sancov] Create the Sancov Pass after building the pipelines

We're running into linker errors from missing sancov sections:

```
ld.lld: error: relocation refers to a discarded section: __sancov_guards
>>> defined in 
>>> user-arm64-ubsan-sancov-full.shlib/obj/third_party/ulib/scudo/scudo.wrappers_c.cc.o
>>> referenced by common.h:26 (../../zircon/third_party/ulib/scudo/common.h:26)
... many other references
```

I believe this is due to a pass in the default pipeline that somehow discards
these sections. The ModuleSanitizerCoveragePass was initially added at the
start of the pipeline. This now adds it to the end of the pipeline for
optimized and unoptimized builds.

Differential Revision: https://reviews.llvm.org/D67323

Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=371326&r1=371325&r2=371326&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Sun Sep  8 00:30:17 2019
@@ -1149,16 +1149,6 @@ void EmitAssemblyHelper::EmitAssemblyWit
 EntryExitInstrumenterPass(/*PostInlining=*/false)));
   });
 
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
-auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
-PB.registerPipelineStartEPCallback(
-[SancovOpts](ModulePassManager &MPM) {
-  MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
-});
-  }
-
   // Register callbacks to schedule sanitizer passes at the appropriate 
part of
   // the pipeline.
   // FIXME: either handle asan/the remaining sanitizers or error out
@@ -1226,6 +1216,13 @@ void EmitAssemblyHelper::EmitAssemblyWit
   }
 }
 
+if (CodeGenOpts.SanitizeCoverageType ||
+CodeGenOpts.SanitizeCoverageIndirectCalls ||
+CodeGenOpts.SanitizeCoverageTraceCmp) {
+  auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
+  MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
+}
+
 if (LangOpts.Sanitize.has(SanitizerKind::HWAddress)) {
   bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
   MPM.addPass(HWAddressSanitizerPass(
@@ -1237,13 +1234,6 @@ void EmitAssemblyHelper::EmitAssemblyWit
 }
 
 if (CodeGenOpts.OptimizationLevel == 0) {
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
-auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
-MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
-  }
-
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
 }
   }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67323: [NewPM][Sancov] Create the Sancov Pass after building the pipelines

2019-09-08 Thread Leonard Chan via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL371326: [NewPM][Sancov] Create the Sancov Pass after 
building the pipelines (authored by leonardchan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67323?vs=219254&id=219257#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67323/new/

https://reviews.llvm.org/D67323

Files:
  cfe/trunk/lib/CodeGen/BackendUtil.cpp


Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -1149,16 +1149,6 @@
 EntryExitInstrumenterPass(/*PostInlining=*/false)));
   });
 
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
-auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
-PB.registerPipelineStartEPCallback(
-[SancovOpts](ModulePassManager &MPM) {
-  MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
-});
-  }
-
   // Register callbacks to schedule sanitizer passes at the appropriate 
part of
   // the pipeline.
   // FIXME: either handle asan/the remaining sanitizers or error out
@@ -1226,6 +1216,13 @@
   }
 }
 
+if (CodeGenOpts.SanitizeCoverageType ||
+CodeGenOpts.SanitizeCoverageIndirectCalls ||
+CodeGenOpts.SanitizeCoverageTraceCmp) {
+  auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
+  MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
+}
+
 if (LangOpts.Sanitize.has(SanitizerKind::HWAddress)) {
   bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
   MPM.addPass(HWAddressSanitizerPass(
@@ -1237,13 +1234,6 @@
 }
 
 if (CodeGenOpts.OptimizationLevel == 0) {
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
-auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
-MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
-  }
-
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
 }
   }


Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -1149,16 +1149,6 @@
 EntryExitInstrumenterPass(/*PostInlining=*/false)));
   });
 
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
-auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
-PB.registerPipelineStartEPCallback(
-[SancovOpts](ModulePassManager &MPM) {
-  MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
-});
-  }
-
   // Register callbacks to schedule sanitizer passes at the appropriate part of
   // the pipeline.
   // FIXME: either handle asan/the remaining sanitizers or error out
@@ -1226,6 +1216,13 @@
   }
 }
 
+if (CodeGenOpts.SanitizeCoverageType ||
+CodeGenOpts.SanitizeCoverageIndirectCalls ||
+CodeGenOpts.SanitizeCoverageTraceCmp) {
+  auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
+  MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
+}
+
 if (LangOpts.Sanitize.has(SanitizerKind::HWAddress)) {
   bool Recover = CodeGenOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
   MPM.addPass(HWAddressSanitizerPass(
@@ -1237,13 +1234,6 @@
 }
 
 if (CodeGenOpts.OptimizationLevel == 0) {
-  if (CodeGenOpts.SanitizeCoverageType ||
-  CodeGenOpts.SanitizeCoverageIndirectCalls ||
-  CodeGenOpts.SanitizeCoverageTraceCmp) {
-auto SancovOpts = getSancovOptsFromCGOpts(CodeGenOpts);
-MPM.addPass(ModuleSanitizerCoveragePass(SancovOpts));
-  }
-
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66336: [ASTImporter] Add development internals docs

2019-09-08 Thread Aleksei Sidorin via Phabricator via cfe-commits 
a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66336/new/

https://reviews.llvm.org/D66336



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

2019-09-08 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added inline comments.



Comment at: clang/test/Profile/misexpect-branch-cold.c:4
+// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -verify -Wmisexpect
+

paulkirth wrote:
> lebedev.ri wrote:
> > Is there a test where `-Wmisexpect` isn't present, to verify that it is 
> > off-by-default?
> We can add one, but is that necessary? Don't the tests for diagnostics cover 
> those already?
To clarify: i'm interested in the case where the PGO data is provided but 
`-Wmisexpect` is *not* specified.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66324/new/

https://reviews.llvm.org/D66324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58531: [clang] Specify type of pthread_create builtin

2019-09-08 Thread James Clarke via Phabricator via cfe-commits 
jrtc27 added a comment.

In D58531#1611356 , @jdoerfert wrote:

> In D58531#1599209 , @probinson wrote:
>
> > We've started running into this too in building the PS4 system. +jdoerfert 
> > who added pthread_create to the builtin list.
> >
> > Looking at the patch, it seems straightforward enough although clearly 
> > needs clang-format-diff run over it.
> >  I don't touch Clang that much so I'm reluctant to okay it myself.
>
>
> @probinson Thanks for making me aware of this patch.
>
> > A separate point is whether it makes sense to be emitting this warning in 
> > the first place for GE_Missing_type. I'd argue that, if we don't know the 
> > type of the builtin, we should never emit the warning
>
> @jrtc27 I hope the immediate need for this is gone after D58091 
>  was finally committed? Given that I caused 
> this mess I can take a look at the patch if you are still interested in it, 
> are you?


Yes, now that it's not a warning if we don't provide a type this is no longer 
completely necessary. I would still be slightly in favour of this patch though 
as then we can detect dodgy `pthread_create` declarations, but it's not a big 
deal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58531/new/

https://reviews.llvm.org/D58531



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58531: [clang] Specify type of pthread_create builtin

2019-09-08 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

I like the patch and I think it is fine.

Small nits: 
Could we have a test for " we can detect dodgy pthread_create declarations" and 
maybe `pthread[_attr]_t`?
There is an unresolved comment by @shrines.
@probinson: "it seems straightforward enough although clearly needs 
clang-format-diff run over it."

I'll accept this assuming the above points are easy to fix and given that no 
one expressed concerns but only positive comments were made.




Comment at: clang/lib/AST/ASTContext.cpp:9239
+  ASTContext::GetBuiltinTypeError 
&Error,
+  unsigned *IntegerConstantArgs);
+

I like the static helper function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58531/new/

https://reviews.llvm.org/D58531



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r371342 - Move prop-sink branch to monorepo.

2019-09-08 Thread Gabor Borsik via cfe-commits 
Author: boga95
Date: Sun Sep  8 12:23:43 2019
New Revision: 371342

URL: http://llvm.org/viewvc/llvm-project?rev=371342&view=rev
Log:
Move prop-sink branch to monorepo.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
cfe/trunk/test/Analysis/taint-generic.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=371342&r1=371341&r2=371342&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Sun Sep  8 
12:23:43 2019
@@ -115,27 +115,44 @@ private:
   static Optional getPointedToSVal(CheckerContext &C, const Expr *Arg);
 
   /// Check for CWE-134: Uncontrolled Format String.
-  static const char MsgUncontrolledFormatString[];
+  static constexpr llvm::StringLiteral MsgUncontrolledFormatString =
+  "Untrusted data is used as a format string "
+  "(CWE-134: Uncontrolled Format String)";
   bool checkUncontrolledFormatString(const CallExpr *CE,
  CheckerContext &C) const;
 
   /// Check for:
   /// CERT/STR02-C. "Sanitize data passed to complex subsystems"
   /// CWE-78, "Failure to Sanitize Data into an OS Command"
-  static const char MsgSanitizeSystemArgs[];
+  static constexpr llvm::StringLiteral MsgSanitizeSystemArgs =
+  "Untrusted data is passed to a system call "
+  "(CERT/STR02-C. Sanitize data passed to complex subsystems)";
   bool checkSystemCall(const CallExpr *CE, StringRef Name,
CheckerContext &C) const;
 
   /// Check if tainted data is used as a buffer size ins strn.. functions,
   /// and allocators.
-  static const char MsgTaintedBufferSize[];
+  static constexpr llvm::StringLiteral MsgTaintedBufferSize =
+  "Untrusted data is used to specify the buffer size "
+  "(CERT/STR31-C. Guarantee that storage for strings has sufficient space "
+  "for character data and the null terminator)";
   bool checkTaintedBufferSize(const CallExpr *CE, const FunctionDecl *FDecl,
   CheckerContext &C) const;
 
+  /// Check if tainted data is used as a custom sink's parameter.
+  static constexpr llvm::StringLiteral MsgCustomSink =
+  "Untrusted data is passed to a user-defined sink";
+  bool checkCustomSinks(const CallExpr *CE, StringRef Name,
+CheckerContext &C) const;
+
   /// Generate a report if the expression is tainted or points to tainted data.
-  bool generateReportIfTainted(const Expr *E, const char Msg[],
+  bool generateReportIfTainted(const Expr *E, StringRef Msg,
CheckerContext &C) const;
 
+  struct TaintPropagationRule;
+  using NameRuleMap = llvm::StringMap;
+  using NameArgMap = llvm::StringMap;
+
   /// A struct used to specify taint propagation rules for a function.
   ///
   /// If any of the possible taint source arguments is tainted, all of the
@@ -175,7 +192,8 @@ private:
 
 /// Get the propagation rule for a given function.
 static TaintPropagationRule
-getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name,
+getTaintPropagationRule(const NameRuleMap &CustomPropagations,
+const FunctionDecl *FDecl, StringRef Name,
 CheckerContext &C);
 
 void addSrcArg(unsigned A) { SrcArgs.push_back(A); }
@@ -211,9 +229,6 @@ private:
CheckerContext &C);
   };
 
-  using NameRuleMap = llvm::StringMap;
-  using NameArgMap = llvm::StringMap;
-
   /// Defines a map between the propagation function's name and
   /// TaintPropagationRule.
   NameRuleMap CustomPropagations;
@@ -228,18 +243,11 @@ private:
 const unsigned GenericTaintChecker::ReturnValueIndex;
 const unsigned GenericTaintChecker::InvalidArgIndex;
 
-const char GenericTaintChecker::MsgUncontrolledFormatString[] =
-"Untrusted data is used as a format string "
-"(CWE-134: Uncontrolled Format String)";
-
-const char GenericTaintChecker::MsgSanitizeSystemArgs[] =
-"Untrusted data is passed to a system call "
-"(CERT/STR02-C. Sanitize data passed to complex subsystems)";
-
-const char GenericTaintChecker::MsgTaintedBufferSize[] =
-"Untrusted data is used to specify the buffer size "
-"(CERT/STR31-C. Guarantee that storage for strings has sufficient space "
-"for character data and the null terminator)";
+// FIXME: these lines can be removed in C++17
+constexpr llvm::StringLiteral GenericTaintChecker::MsgUncontrolledFormatString;
+constexpr llvm::StringLiteral GenericTaintChecker::MsgSanitizeSystemArgs;
+constexpr llvm::StringLiteral GenericTaintChecker::MsgTaintedBufferSize;
+constexpr llvm::StringLiteral GenericTaintChecker::MsgCustomSink;
 } // end of anonymous namespace
 
 using TaintConfig = GenericTaintC

[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

2019-09-08 Thread Borsik Gábor via Phabricator via cfe-commits 
boga95 closed this revision.
boga95 marked 10 inline comments as done.
boga95 added a comment.

Closed by 080ecafdd8b3e990e5ad19202d089c91c9c9b164 
.




Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:251
+const llvm::StringLiteral GenericTaintChecker::MsgCustomSink =
+"Untrusted data is passed to a user defined sink";
+;

NoQ wrote:
> It should be "user-defined" because it's a single adjective.
> 
> I recommend against using the word "sink" in user-facing messages because 
> it's too jargony. Do we have a better word for this?
Currently, I don't have any better idea. I will think about it.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:118
   /// Check for CWE-134: Uncontrolled Format String.
-  static const char MsgUncontrolledFormatString[];
+  static constexpr llvm::StringLiteral MsgUncontrolledFormatString =
+  "Untrusted data is used as a format string "

steakhal wrote:
> Shouldn't we still need an out-of-class initializer part for each static 
> constexpr class member variable?
> These would provide the memory locations for the declarations.
> ```
> constexpr llvm::StringLiteral 
> GenericTaintChecker::MsgUncontrolledFormatString;
> constexpr llvm::StringLiteral GenericTaintChecker::MsgSanitizeSystemArgs;
> constexpr llvm::StringLiteral GenericTaintChecker::MsgTaintedBufferSize;
> constexpr llvm::StringLiteral GenericTaintChecker::MsgCustomSink;
> ```
Constexpr values cannot be initialized out of the class, that's why I moved 
them to here.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:191
 static TaintPropagationRule
-getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name,
+getTaintPropagationRule(const GenericTaintChecker *Checker,
+const FunctionDecl *FDecl, StringRef Name,

steakhal wrote:
> Szelethus wrote:
> > How about only passing `CustomPropagations`?
> I would even consider to move this function out of the whole class. (Not only 
> this function, but the others as well. Like isStdin, etc.)
> I think pure, free-functions (in an anonymous namespace) are easier to reason 
> about.
I could do that in a separate patch if necessary.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:605
 // Mark the given argument.
-assert(ArgNum < CE->getNumArgs());
 State = State->add(ArgNum);

Szelethus wrote:
> I get that there isn't much substance to this assert, but why remove it? We 
> might as well populate the lines in between that and the branch.
I think there is no need for this. Maybe I can make the ArgNum const.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59637/new/

https://reviews.llvm.org/D59637



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65699: [Driver] Prioritize SYSROOT/usr/include over RESOURCE_DIR/include on linux-musl

2019-09-08 Thread Khem Raj via Phabricator via cfe-commits 
raj.khem added a comment.

Can we bring this to 9.x release branch as well please ?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65699/new/

https://reviews.llvm.org/D65699



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-09-08 Thread Petr Hosek via Phabricator via cfe-commits 
phosek added a comment.

In D59754#1656217 , @leonardchan wrote:

> Hi! We've noticed that for our arm bots, we're getting some flaky builds that 
> sometimes fail with `error: array designators are a C99 extension 
> [-Werror,-Wc99-designator]` and sometimes don't fail. 2 questions:
>
> 1. I can't see it off the patch immediately, but do you know why for arm 
> specifically we can only get this warning sometimes?
> 2. I noticed that for the `test/SemaCXX/c99.cpp` test, this warning is also 
> diagnosed for the `-std=c++17` case. Are C-style designated initializers only 
> invalid in c++20, or are they also invalid in 17?
>
>   Thanks.


@rsmith ping? We're still hitting this issue in our build. Should this warning 
be diagnosed even when using `-std=c++17`?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59754/new/

https://reviews.llvm.org/D59754



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67335: [analyzer][NFC] Refactor the checker registration unit test file

2019-09-08 Thread Kristóf Umann via Phabricator via cfe-commits 
Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, Charusso, dcoughlin, 
baloghadamsoftware.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

Nothing exciting to see here! The new interface allows for more fine tuning 
(register but disable a checker, add custom checker registry functions, etc), 
that was basically the point.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67335

Files:
  clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  clang/unittests/StaticAnalyzer/CheckerRegistration.h
  clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -6,11 +6,13 @@
 //
 //===--===//
 
+#include "CheckerRegistration.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "clang/Tooling/Tooling.h"
@@ -20,52 +22,10 @@
 namespace ento {
 namespace {
 
-template 
-class TestAction : public ASTFrontendAction {
-  class DiagConsumer : public PathDiagnosticConsumer {
-llvm::raw_ostream &Output;
-
-  public:
-DiagConsumer(llvm::raw_ostream &Output) : Output(Output) {}
-void FlushDiagnosticsImpl(std::vector &Diags,
-  FilesMade *filesMade) override {
-  for (const auto *PD : Diags)
-Output << PD->getCheckerName() << ":" << PD->getShortDescription();
-}
-
-StringRef getName() const override { return "Test"; }
-  };
-
-  llvm::raw_ostream &DiagsOutput;
-
-public:
-  TestAction(llvm::raw_ostream &DiagsOutput) : DiagsOutput(DiagsOutput) {}
-
-  std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
- StringRef File) override {
-std::unique_ptr AnalysisConsumer =
-CreateAnalysisConsumer(Compiler);
-AnalysisConsumer->AddDiagnosticConsumer(new DiagConsumer(DiagsOutput));
-Compiler.getAnalyzerOpts()->CheckersAndPackages = {
-{"custom.CustomChecker", true}};
-AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
-  Registry.addChecker("custom.CustomChecker", "Description", "");
-});
-return std::move(AnalysisConsumer);
-  }
-};
-
-template 
-bool runCheckerOnCode(const std::string &Code, std::string &Diags) {
-  llvm::raw_string_ostream OS(Diags);
-  return tooling::runToolOnCode(new TestAction(OS), Code);
-}
-template 
-bool runCheckerOnCode(const std::string &Code) {
-  std::string Diags;
-  return runCheckerOnCode(Code, Diags);
-}
-
+//===--===//
+// Just a minimal test for how checker registration works with statically
+// linked, non TableGen generated checkers.
+//===--===//
 
 class CustomChecker : public Checker {
 public:
@@ -77,12 +37,25 @@
   }
 };
 
+void addCustomChecker(AnalysisASTConsumer &AnalysisConsumer,
+  AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"custom.CustomChecker", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+Registry.addChecker("custom.CustomChecker", "Description",
+   "");
+  });
+}
+
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
-  EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
-  EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description");
+  EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
+  EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description\n");
 }
 
+//===--===//
+// Pretty much the same.
+//===--===//
+
 class LocIncDecChecker : public Checker {
 public:
   void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
@@ -94,11 +67,20 @@
   }
 };
 
+void addLocIncDecChecker(AnalysisASTConsumer &AnalysisConsumer,
+ AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"test.LocIncDecChecker", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+Registry.addChecker("test.L

[PATCH] D67336: [analyzer][NFC] Introduce SuperChecker<>, a convenient alternative to Checker<> for storing subcheckers

2019-09-08 Thread Kristóf Umann via Phabricator via cfe-commits 
Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, dcoughlin, Charusso, 
baloghadamsoftware, dkrupp.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet, whisperity.

Please don't take a shot for each time I write checker, it'd end bad.

The term "super checker" or "modeling checker", and the term "subchecker" 
always existed conceptually, but they weren't ever formalized. In the last half 
a year or so, we referred to them as

- Super/modeling checker: A checker that models some C++ code, but doesn't (or, 
as of now, just //shouldn't//) emit any diagnostics, at least not under its own 
name.
- Subcheckers: Checkers that are a part of super/modeling checkers, 
enabling/disabling them (ideally) only toggles whether a diagnostic is emitted 
from the checker it is a part of. They don't possess a checker object on their 
own, and are basically glorified checker options.

While checker dependencies were in similar shoes (existed conceptually but not 
formalized), this change isn't //as// critical, it just removes boilerplate 
code. When you look at `IteratorChecker`, `SecuritySyntaxChecker` or 
`RetainCountBase` they all use a similar, some cases faulty implementation to 
keep track of which subcheckers are enabled and what their name is, so its 
about time we combined them.

I envision this interface to be used to enforce our currently non-existent 
specification on the checker system.

In detail:

- Introduce `SuperChecker`:
  - It is essentially the same as `Checker`, but requires an enum template 
argument to keep track of which subcheckers are enabled.
  - While previously we defined subcheckers as checkers that don't have a 
checker object on their own, `SuperChecker` does create a `CheckerBase` object 
for them, but they still can't have checker callbacks.
- Add `CheckerManager::registerSubChecker` adds a new checker to a 
`SuperChecker`. It is ensured runtime that the `SuperChecker` was previously 
registered, and that a subchecker isn't registered multiple times.
- Add thorough test cases for the new interface.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67336

Files:
  clang/include/clang/StaticAnalyzer/Core/Checker.h
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -81,6 +81,127 @@
   runCheckerOnCode("void f() { int *p; (*p)++; }"));
 }
 
+//===--===//
+// Subchecker system.
+//===--===//
+
+enum CXX23ModelingDiagKind { IntPointer, NonLoad };
+
+class CXX23Modeling
+: public SuperChecker {
+public:
+  void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
+BugReporter &BR) const {
+BR.EmitBasicReport(D, this, "Custom diagnostic", categories::LogicError,
+   "Sketchy C++23 code modeled",
+   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+
+if (const CheckerBase *IntPointerChecker = getSubChecker())
+  BR.EmitBasicReport(D, IntPointerChecker, "Custom diagnostic",
+ categories::LogicError, "Sketchy C++23 int pointer",
+ PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+
+if (const CheckerBase *NonLoadChecker = getSubChecker())
+  BR.EmitBasicReport(D, NonLoadChecker, "Custom diagnostic",
+ categories::LogicError,
+ "Sketchy C++23 pointer non-loaded",
+ PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  }
+};
+
+void registerCXX23IntPointer(CheckerManager &Mgr) {
+  Mgr.registerSubChecker();
+}
+
+void registerCXX23NonLoad(CheckerManager &Mgr) {
+  Mgr.registerSubChecker();
+}
+
+void addButDontSpecifyCXX23Modeling(AnalysisASTConsumer &AnalysisConsumer,
+  AnalyzerOptions &AnOpts) {
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+Registry.addChecker("test.CXX23Modeling", "Description", "");
+  });
+}
+
+void addAndEnableCXX23Modeling(AnalysisASTConsumer &AnalysisConsumer,
+  AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"test.CXX23Modeling", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+Registry.addChecker("test.CXX23Modeling", "Description", "");
+  });
+}
+
+void addButDisableCXX23Modeling(AnalysisASTConsumer &AnalysisConsumer,
+  AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"test.CXX23Mode

[PATCH] D67335: [analyzer][NFC] Refactor the checker registration unit test file

2019-09-08 Thread Kristóf Umann via Phabricator via cfe-commits 
Szelethus marked an inline comment as done.
Szelethus added inline comments.



Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:84
 
-}
-}
-}
+} // namespace
+} // namespace ento

Well, according to `clangd`, this is how it's supposed to be done in LLVM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67335/new/

https://reviews.llvm.org/D67335



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

2019-09-08 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added a comment.

In D59754#1656217 , @leonardchan wrote:

> Hi! We've noticed that for our arm bots, we're getting some flaky builds that 
> sometimes fail with `error: array designators are a C99 extension 
> [-Werror,-Wc99-designator]` and sometimes don't fail. 2 questions:
>
> 1. I can't see it off the patch immediately, but do you know why for arm 
> specifically we can only get this warning sometimes?


That is strange; can you provide me with buildbot links for a pass and a 
failure?

> 2. I noticed that for the `test/SemaCXX/c99.cpp` test, this warning is also 
> diagnosed for the `-std=c++17` case. Are C-style designated initializers only 
> invalid in c++20, or are they also invalid in 17?

C-style designated initializers are invalid in all C++ standards before C++20, 
and some of the features are still invalid in C++20. We allow those features as 
an extension, but as of this change we produce a warning by default when that 
extension is used. We really should have been doing that for years, but it 
matters a lot more now, because people are going to look at Clang's diagnostics 
to understand which parts of C designator syntax are valid in C++ and which 
parts are not.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59754/new/

https://reviews.llvm.org/D59754



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65699: [Driver] Prioritize SYSROOT/usr/include over RESOURCE_DIR/include on linux-musl

2019-09-08 Thread Khem Raj via Phabricator via cfe-commits 
raj.khem added a comment.

one issue I now see is that when we have some thing like

static const struct {

  const wchar_t *name;
  int (*func)(EditLine *, int, const wchar_t **);
  } cmds[] = {
  { L"bind",  map_bind},
  { L"echotc",terminal_echotc },
  { L"edit",  el_editmode },
  { L"history",   hist_command},
  { L"telltc",terminal_telltc },
  { L"settc", terminal_settc  },
  { L"setty", tty_stty},
  { NULL, NULL}
  };

clang complains

warning: incompatible pointer types initializing 'const wchar_t *' (aka 'const 
long *') with an expression of type 'int [5]' [-Wincompatible-pointer-types]

  { L"bind",  map_bind},
^~~

it seems L"..." prefix is treated as `int`  where as in musl wchar_t is of long 
int type


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65699/new/

https://reviews.llvm.org/D65699



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits