r247696 - Mention that libcxx is required to build Compiler-RT tests on OS X.

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Tue Sep 15 10:05:47 2015
New Revision: 247696

URL: http://llvm.org/viewvc/llvm-project?rev=247696&view=rev
Log:
Mention that libcxx is required to build Compiler-RT tests on OS X.

Modified:
cfe/trunk/www/get_started.html

Modified: cfe/trunk/www/get_started.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/get_started.html?rev=247696&r1=247695&r2=247696&view=diff
==
--- cfe/trunk/www/get_started.html (original)
+++ cfe/trunk/www/get_started.html Tue Sep 15 10:05:47 2015
@@ -76,6 +76,14 @@ follows:
 cd ../..
   
   
+  Checkout libcxx: (only required to build and run Compiler-RT tests on OS 
X, optional otherwise)
+  
+cd llvm/projects
+svn co http://llvm.org/svn/llvm-project/libcxx/trunk
+libcxx
+cd ../..
+  
+  
   Build LLVM and Clang:
   
 mkdir build (in-tree build is not supported)


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

Re: [PATCH] D5102: [analyzer][Bugfix/improvement] Fix for PR16833

[Yury Gribov via cfe-commits]

ygribov added a subscriber: ygribov.
ygribov added a comment.

Folks, could someone commit this for us?


http://reviews.llvm.org/D5102



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

[PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov created this revision.
ygribov added reviewers: zaks.anna, dcoughlin, jordan_rose, krememek.
ygribov added a subscriber: cfe-commits.
ygribov set the repository for this revision to rL LLVM.

Hi all,

This checker verifies that vfork is used safely. Vforked process shared stack 
with parent process so it's range of actions is significantly limited (can't 
write variables, can't call functions not in whitelist, etc.).

The patch grew out of complicated 2-day debugging of production SW caused by 
well-known vfork bug in xtables (see 
http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).

Is something like this interesting for upstream?

Repository:
  rL LLVM

http://reviews.llvm.org/D14014

Files:
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  test/Analysis/Inputs/system-header-simulator.h
  test/Analysis/vfork-1.c
  test/Analysis/vfork-2.c

Index: test/Analysis/vfork-2.c
===
--- /dev/null
+++ test/Analysis/vfork-2.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.unix.Vfork %s -verify
+
+#include "Inputs/system-header-simulator.h"
+
+void foo() {
+  // See "libxtables: move some code to avoid cautions in vfork man page"
+  if (vfork() == 0) {
+execl("prog", "arg1", 0);
+exit(1);  // expected-warning{{}}
+  }
+}
+
Index: test/Analysis/vfork-1.c
===
--- /dev/null
+++ test/Analysis/vfork-1.c
@@ -0,0 +1,104 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin.NoReturnFunctions,alpha.unix.Vfork %s -verify
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Check that child process is checked
+int f1(int x) {
+  pid_t pid = vfork();
+  if (pid != 0)
+return 0;
+
+  switch (x) {
+  case 0:
+// writing pid is ok
+pid = 1;
+// calling whitelisted routines is ok
+execl("", "", 0);
+_exit(1);
+break;
+  case 1:
+// writing variables is prohibited
+x = 0; // expected-warning{{}}
+break;
+  case 2:
+// calling functions is prohibited
+foo(); // expected-warning{{}}
+break;
+  default:
+// returning from function is prohibited
+return 0;
+  }
+
+  while(1);
+} // expected-warning{{}}
+
+// Same but without explicit pid
+int f2(int x) {
+  pid_t pid = vfork();
+
+  switch (x) {
+  case 0:
+// writing pid is ok
+pid = 1;
+// calling whitelisted routines is ok
+execl("", "", 0);
+_exit(1);
+break;
+  case 1:
+// writing variables is prohibited
+x = 0; // expected-warning{{}}
+break;
+  case 2:
+// calling functions is prohibited
+foo(); // expected-warning{{}}
+break;
+  default:
+// returning from function is prohibited
+return 0;
+  }
+
+  while(1);
+} // expected-warning{{}}
+
+// check that parent process can do anything
+int f3(int x) {
+  if (vfork() == 0)
+_exit(1);
+  x = 0;
+  foo();
+  return 0;
+}
+
+// unbound pids are special so test them separately
+void f4(int x) {
+  switch (x) {
+  case 0:
+vfork();
+x = 0; // expected-warning{{}}
+break;
+
+  case 1:
+{
+  char args[2];
+  switch (vfork()) {
+  case 0:
+args[0] = 0; // expected-warning{{}}
+exit(1);
+  }
+  break;
+}
+
+  case 2:
+{
+  pid_t pid;
+  if ((pid = vfork()) == 0)
+while(1);
+  break;
+}
+  }
+  while(1);
+}
+
+
Index: test/Analysis/Inputs/system-header-simulator.h
===
--- test/Analysis/Inputs/system-header-simulator.h
+++ test/Analysis/Inputs/system-header-simulator.h
@@ -86,3 +86,13 @@
   char * p;
 } SomeStruct;
 void fakeSystemHeaderCall(SomeStruct *);
+
+typedef int pid_t;
+pid_t fork(void);
+pid_t vfork(void);
+int execl(const char *path, const char *arg, ...);
+
+void exit(int status) __attribute__ ((__noreturn__));
+void _exit(int status) __attribute__ ((__noreturn__));
+void _Exit(int status) __attribute__ ((__noreturn__));
+
Index: lib/StaticAnalyzer/Checkers/VforkChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/VforkChecker.cpp
@@ -0,0 +1,260 @@
+//===- VforkChecker.cpp  Vfork usage checks --*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines chroot checker, which checks improper use of chroot.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/ParentMap.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/Sta

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added a comment.

> I didn't know vfork() is in regular use.


Neither did I, until I had to debug the damned code. Actually Android has some 
5 or 6 uses of it.

> I'm also not convinced that flagging all stores in the vfork child process is 
> the right thing to do,

>  since the FP rate could be quite high.


I also have these concerns. Manpage is very strict about behavior of vforked 
processes (for a good reason though). Currently most uses of vfork are 
incorrect according to this checker.

I think there is some potential to relax the constraints:

- at the very least we can allow calls to functions marked as inline as those 
won't result in any memory writes
- we can even allow to call any functions which are guaranteed to not change 
global state

> For example, doing 'x = malloc(4); *x = 0;' in the child process seems fine 
> to me.


I also thought about similar examples. Here are some not so obvious 
side-effects:

- you'll get a memory leak (as malloc will be shared by parent and child 
processes)
- in the following case behavior of parent is undefined because original value 
of variable will be corrupted:

  x = some_very_important_lost;
  if (vfork()==0) {
x = ...; // some_very_important_data is lost
_exit(1);
  }
  // parent code
  if (some_very_important_data) {
// UB
  }

- even if child does not explicitly write any parent's variables, compiler may 
decide to merge stack slots so write into a local variable in child process may 
corrupt a seemingly unrelated variable in parent



Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:10
@@ +9,3 @@
+//
+//  This file defines chroot checker, which checks improper use of chroot.
+//

vsk wrote:
> This comment needs an update.
Yikes, thanks for pointing this out.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:38
@@ +37,3 @@
+//  |
+//  --return--> bug
+class VforkChecker : public Checker Please clarify this comment -- it might be better to state a list of specific 
> buggy conditions.
> 
> E.g, are writes into heap-allocated allowed? Where are return statements 
> allowed?
This primitive scheme simply mirrors the manpage which is very strict about 
this - write to _any_ data besides return value of vfork is disallowed, ditto 
for function calls (and so "return" only applies to current function).


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:44
@@ +43,3 @@
+
+  // This bug refers to dangerous code in vforked child.
+  mutable std::unique_ptr BT_BadChildCode;

vsk wrote:
> This comment isn't needed -- a description to this effect can go in the 
> top-of-file comment.
Right.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:65
@@ +64,3 @@
+
+  // these constructs are prohibited in vforked child
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;

vsk wrote:
> This comment doesn't add much, please remove it.
Ok.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:75
@@ +74,3 @@
+REGISTER_TRAIT_WITH_PROGRAMSTATE(VforkLhs, const void *)
+#define VFORK_LHS_NONE ((void *)(uintptr_t)1)
+

vsk wrote:
> Why not nullptr? If there's a good reason, please explain it with a comment.
Ok, this indeed deserves a comment. FTR: nullptr means parent process, 
VFORK_LHS_NONE means vforked process without explicit LHS and rest means LHS 
variable.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:147
@@ +146,3 @@
+return D;
+  } while (0);
+

vsk wrote:
> This is hard to read, please simplify this. E.g;
> 
>   if (PD = dyn-cast(P)) { /* handle cases where P is-a DeclStmt */ }
>   else if (Assign = dyn-cast(P)) { /* handle cases where P is-a binop */ }
> 
> Using loop constructs here shouldn't be necessary.
In this case each statement depends on the previous one so we'll have to resort 
to deeply nested conditionals. I thought that do-while-false is a common 
pattern for simplifying such constructs but I may be wrong.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:167
@@ +166,3 @@
+  // otherwise no lhs
+  return (const VarDecl *)VFORK_LHS_NONE;
+}

vsk wrote:
> Typically nullptr is used here.
See my comment above, these two encode two different things.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:186
@@ +185,3 @@
+ CheckerContext &C) const {
+  // we can't call vfork in child so don't bother
+  ProgramStateRef State = C.getState();

dcoughlin wrote:
> vsk wrote:
> > Actually, if this is the case, wouldn't it be worthwhile to flag calls to 
> > vfork() in child processes?
> This will get caught by `checkPreCall`, right? Because `vfork` is not in the 
> white list.
They will be flagged in checkPreCall. But I should prob

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added a comment.

> One thing to note (which I assume you are already aware of) is that we 
> already have the "security.insecureAPI.vfork" checker,

>  an AST check that warns on *every* use of vfork.


Yes, I was aware of this one. Unfortunately as I mentioned above vfork is 
probably to stay with us for quite a while.

> This check is on by default (which I think makes sense, given the variety of 
> ways that vfork is problematic) --

>  so users of your more permissive check would have to disable the default 
> check.


This is a valid concern because it greatly complicates enablement of 
VforkChecker for a casual user. Do we have some mechanism so that one checker 
(here VforkChecker) could disable/modify behavior of it's less precise 
companion (insecureAPI)? This sounds like a generally useful feature (although 
it damages the modularity of checkers).



Comment at: lib/StaticAnalyzer/Checkers/Checkers.td:360
@@ +359,3 @@
+def VforkChecker : Checker<"Vfork">,
+  HelpText<"Check for proper usage of vfork.">,
+  DescFile<"VforkChecker.cpp">;

dcoughlin wrote:
> The convention here is to not have a '.' after the help text.
Got it.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:61
@@ +60,3 @@
+
+  // these are used to track vfork state
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;

dcoughlin wrote:
> LLVM convention is to use capitalization and punctuation for comments. (See 
> http://llvm.org/docs/CodingStandards.html#commenting).
Thanks, I should probably re-read the std once again.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:74
@@ +73,3 @@
+// region child is allowed to write)
+REGISTER_TRAIT_WITH_PROGRAMSTATE(VforkLhs, const void *)
+#define VFORK_LHS_NONE ((void *)(uintptr_t)1)

dcoughlin wrote:
> Is it possible to use a name that implies a more semantic notion? For 
> example, "VforkResultRegion"?
That would be the fourth iteration of the name) But yes, "Result region" sounds 
more descriptive.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:102
@@ +101,3 @@
+
+  for (SmallVectorImpl::iterator
+ I = VforkWhitelist.begin(), E = VforkWhitelist.end();

dcoughlin wrote:
> I think it is better to use SmallSet for VforkWhitelist rather than duplicate 
> that functionality here.
Ok.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:115
@@ +114,3 @@
+BT_BadChildCode.reset(
+  new BuiltinBug(this, "Dangerous construct in vforked process",
+ "Prohibited construct after successful "

dcoughlin wrote:
> What do you think about making the warnings more descriptive here? For 
> example, you could one warning saying that "Child can only modify variable 
> used to store result of vfork()", one that says "Child can only call _exit() 
> or `exec()` family of functions", "Child must not return from function 
> calling vfork()", etc.
> 
> These descriptions would help the user not have to guess what they are doing 
> wrong.
Yes, given the public ignorance about vfork's idiosyncrasies that makes good 
sense.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:118
@@ +117,3 @@
+ "vfork"));
+  ExplodedNode *N = C.generateSink(C.getState(), C.getPredecessor());
+  auto Report = llvm::make_unique(*BT_BadChildCode, 

dcoughlin wrote:
> You should use `C.generateErrorNode()` here, which expresses the intent to 
> create a node for the purposes of error reporting.
Will do.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:135
@@ +134,3 @@
+  do {
+const DeclStmt *PD = dyn_cast_or_null(P);
+if (!PD)

dcoughlin wrote:
> For dyn_cast and friends and you use `auto` to avoid repeating the type 
> twice. For example:
> 
> ```
> const auto *PD = dyn_cast_or_null(P);
> ```
True. This was forward-ported from our ancient version so lacks C++11 beauties.


Comment at: test/Analysis/vfork-1.c:1
@@ +1,2 @@
+// RUN: %clang_cc1 -analyze 
-analyzer-checker=core.builtin.NoReturnFunctions,alpha.unix.Vfork %s -verify
+

dcoughlin wrote:
> Tests generally need to have all of core turned on (e.g., 
> `-analyzer-checker=core,alpha.unix.Vfork`) because the analyzer implements 
> key functionality in the core checkers. (It is not safe to run a 
> path-sensitive checker without core turned on).
Ok, good to know.


Comment at: test/Analysis/vfork-1.c:68
@@ +67,3 @@
+  if (vfork() == 0)
+_exit(1);
+  x = 0;

dcoughlin wrote:
> I think it would be good to add `// no-warning` on the lines that shouldn't 
> warn. This will make it easier for people tracking down test failures to know 
> that there really shouldn't be a warning on that line.
Sounds like a good maintenance practice, I'll definitely add it.

=

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added a comment.

> This is a valid concern because it greatly complicates enablement of 
> VforkChecker for a casual user.


I think at the very least I can check that InsecureAPI is enable and issue a 
warning to user.


Repository:
  rL LLVM

http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:47
@@ +46,3 @@
+
+  bool isChildProcess(const ProgramStateRef State) const;
+

a.sidorin wrote:
> I think it's a good idea to make some functions static and/or move them out 
> of class definition.
Right. Which one is preferred btw?


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:97
@@ +96,3 @@
+
+ASTContext &Ctx = C.getASTContext();
+for (const char **id = ids; *id; ++id)

a.sidorin wrote:
> What about combination of std::transform + std::sort + std::binary_search?
I think I should use SmallSet here as suggested by Devin.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:149
@@ +148,3 @@
+
+  // see if it's an ordinary assignment
+  do {

a.sidorin wrote:
> You can use early return to escape do{}.
In this particular case - yes. But what's wrong about original do-while-false? 
I thought it's a common idiom...


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:196
@@ +195,3 @@
+  SVal VforkRetVal = Call.getReturnValue();
+  SymbolRef Sym = VforkRetVal.getAsSymbol();
+  Optional DVal =

a.sidorin wrote:
> Is check for non-concrete value is really required?
This may be remains of old code, I'll re-check.


Repository:
  rL LLVM

http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:149
@@ +148,3 @@
+
+  // see if it's an ordinary assignment
+  do {

ygribov wrote:
> a.sidorin wrote:
> > You can use early return to escape do{}.
> In this particular case - yes. But what's wrong about original 
> do-while-false? I thought it's a common idiom...
FYI there are several uses of do-while-false in CSA codebase.


Repository:
  rL LLVM

http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov removed rL LLVM as the repository for this revision.
ygribov updated this revision to Diff 38423.
ygribov added a comment.

Updated based on review.


http://reviews.llvm.org/D14014

Files:
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  test/Analysis/Inputs/system-header-simulator.h
  test/Analysis/vfork.c

Index: test/Analysis/vfork.c
===
--- /dev/null
+++ test/Analysis/vfork.c
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify -x c++ %s
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Ensure that child process is properly checked.
+int f1(int x) {
+  pid_t pid = vfork();
+  if (pid != 0)
+return 0;
+
+  switch (x) {
+  case 0:
+// Ensure that modifying pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0;
+  }
+
+  while(1);
+} // expected-warning{{}}
+
+// Same as previous but without explicit pid variable.
+int f2(int x) {
+  pid_t pid = vfork();
+
+  switch (x) {
+  case 0:
+// Ensure that writing pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0;
+  }
+
+  while(1);
+} // expected-warning{{}}
+
+// Ensure that parent process isn't restricted.
+int f3(int x) {
+  if (vfork() == 0)
+_exit(1);
+  x = 0; // no-warning
+  foo(); // no-warning
+  return 0;
+} // no-warning
+
+// Unbound pids are special so test them separately.
+void f4(int x) {
+  switch (x) {
+  case 0:
+vfork();
+x = 0; // expected-warning{{}}
+break;
+
+  case 1:
+{
+  char args[2];
+  switch (vfork()) {
+  case 0:
+args[0] = 0; // expected-warning{{}}
+exit(1);
+  }
+  break;
+}
+
+  case 2:
+{
+  pid_t pid;
+  if ((pid = vfork()) == 0)
+while(1); // no-warning
+  break;
+}
+  }
+  while(1);
+} //no-warning
+
+
+void f5() {
+  // See "libxtables: move some code to avoid cautions in vfork man page"
+  // (http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).
+  if (vfork() == 0) {
+execl("prog", "arg1", 0); // no-warning
+exit(1);  // expected-warning{{}}
+  }
+}
+
Index: test/Analysis/Inputs/system-header-simulator.h
===
--- test/Analysis/Inputs/system-header-simulator.h
+++ test/Analysis/Inputs/system-header-simulator.h
@@ -86,3 +86,13 @@
   char * p;
 } SomeStruct;
 void fakeSystemHeaderCall(SomeStruct *);
+
+typedef int pid_t;
+pid_t fork(void);
+pid_t vfork(void);
+int execl(const char *path, const char *arg, ...);
+
+void exit(int status) __attribute__ ((__noreturn__));
+void _exit(int status) __attribute__ ((__noreturn__));
+void _Exit(int status) __attribute__ ((__noreturn__));
+
Index: lib/StaticAnalyzer/Checkers/VforkChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/VforkChecker.cpp
@@ -0,0 +1,238 @@
+//===- VforkChecker.cpp  Vfork usage checks --*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines vfork checker which checks for dangerous uses of vfork.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/AST/ParentMap.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "cla

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov marked 43 inline comments as done.
ygribov added a comment.

http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added a comment.

> > This is a valid concern because it greatly complicates enablement of 
> > VforkChecker for a casual user.

> 

> 

> I think at the very least I can check that InsecureAPI is enable and issue a 
> warning to user.


Actually I think that's not a huge problem. InsecureAPI checker would issue a 
warning but not emit a sink node. So it wouldn't influence VforkChecker at all.


http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added a comment.

Done!


http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added a comment.

> What happens when this checker and the security.insecureAPI.vfork are enabled 
> at the same time?


Both checkers will emit warnings independently (which I think is ok).

> Did you run this checker on a large body of code? Did it find any issues?


Yes, I've ran it on Android 5 and found several violations (function calls, 
assignments). Frankly I think that most existing uses of vfork do not obey the 
specification.

> What is needed to turn this into a non-alpha checker?


I guess that's question for maintainers) We have to consider that vfork is used 
rarely enough.



Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:44
@@ +43,3 @@
+// Pattern matches to extract lhs in `lhs = vfork()' statement.
+static const VarDecl *GetAssignedVariable(const CallEvent &Call,
+   CheckerContext &C) {

zaks.anna wrote:
> This should be added as a utility function. Does this exist elsewhere?
Frankly this function only handles two common patterns right now (assignment 
and initialization). This was enough for all uses of vfork which I've seen but 
I'm afraid that general case may require much more work.

I'll try to find any dups.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:63
@@ +62,3 @@
+  return D;
+  } while (0);
+

zaks.anna wrote:
> Can this be rewritten so that it is more clear why this terminates? 
> Also, I'd prefer to use "while(true)".
Actually this isn't really a loop - it's a common do-while-false idiom which 
allows to reduce amount of nesting. You can check for similar pattern in 
ArrayBoundCheckerV2.cpp and also other parts of codebase.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:155
@@ +154,3 @@
+if (!BT)
+  BT.reset(new BuiltinBug(this, "Dangerous construct in vforked process"));
+

zaks.anna wrote:
> "a vforked process"?
Right.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:163
@@ +162,3 @@
+auto Report = llvm::make_unique(*BT, os.str(), N);
+C.emitReport(std::move(Report));
+  }

zaks.anna wrote:
> Ideally, we would point out where the vfork has occurred along the path with 
> a BugReportVisitor. (But it's not a blocker.)
Yeah, that would be nice. But vfork code blocks are typically very small (5-10 
LOC) so it'll all be clear anyway.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:208
@@ +207,3 @@
+  && !isCallWhitelisted(Call.getCalleeIdentifier(), C))
+reportBug("Calling functions (except exec(3) or _exit(2))", C);
+}

zaks.anna wrote:
> We are not listing the full whitelist here. Should we drop the "(except ..)" 
> from the message?
What about
  except exec*() or _exit()
?


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:226
@@ +225,3 @@
+
+  reportBug("Assigning variables (except return value of vfork)", C);
+}

zaks.anna wrote:
> "except return value" -> "except the return value"? Or maybe we should drop 
> the "(except ...)" here as well to make the message shorter.
I'd rather keep it.


Comment at: test/Analysis/vfork.c:24
@@ +23,3 @@
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{}}
+break;

zaks.anna wrote:
> We should check for the exact expected warning in regression tests.
Ok.


http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov updated this revision to Diff 38740.
ygribov added a comment.

Updated after Anna's review.


http://reviews.llvm.org/D14014

Files:
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  test/Analysis/Inputs/system-header-simulator.h
  test/Analysis/vfork.c

Index: test/Analysis/vfork.c
===
--- /dev/null
+++ test/Analysis/vfork.c
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify -x c++ %s
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Ensure that child process is properly checked.
+int f1(int x) {
+  pid_t pid = vfork();
+  if (pid != 0)
+return 0;
+
+  switch (x) {
+  case 0:
+// Ensure that modifying pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{Assigning variables (except return value of vfork) is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{Calling functions (except exec*() or _exit()) is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0;
+  }
+
+  while(1);
+} // expected-warning{{Returning from a function is prohibited after a successful vfork}}
+
+// Same as previous but without explicit pid variable.
+int f2(int x) {
+  pid_t pid = vfork();
+
+  switch (x) {
+  case 0:
+// Ensure that writing pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{Assigning variables (except return value of vfork) is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{Calling functions (except exec*() or _exit()) is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0;
+  }
+
+  while(1);
+} // expected-warning{{Returning from a function is prohibited after a successful vfork}}
+
+// Ensure that parent process isn't restricted.
+int f3(int x) {
+  if (vfork() == 0)
+_exit(1);
+  x = 0; // no-warning
+  foo(); // no-warning
+  return 0;
+} // no-warning
+
+// Unbound pids are special so test them separately.
+void f4(int x) {
+  switch (x) {
+  case 0:
+vfork();
+x = 0; // expected-warning{{Assigning variables (except return value of vfork) is prohibited after a successful vfork}}
+break;
+
+  case 1:
+{
+  char args[2];
+  switch (vfork()) {
+  case 0:
+args[0] = 0; // expected-warning{{Assigning variables (except return value of vfork) is prohibited after a successful vfork}}
+exit(1);
+  }
+  break;
+}
+
+  case 2:
+{
+  pid_t pid;
+  if ((pid = vfork()) == 0)
+while(1); // no-warning
+  break;
+}
+  }
+  while(1);
+} //no-warning
+
+
+void f5() {
+  // See "libxtables: move some code to avoid cautions in vfork man page"
+  // (http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).
+  if (vfork() == 0) {
+execl("prog", "arg1", 0); // no-warning
+exit(1);  // expected-warning{{Calling functions (except exec*() or _exit()) is prohibited after a successful vfork}}
+  }
+}
+
Index: test/Analysis/Inputs/system-header-simulator.h
===
--- test/Analysis/Inputs/system-header-simulator.h
+++ test/Analysis/Inputs/system-header-simulator.h
@@ -86,3 +86,13 @@
   char * p;
 } SomeStruct;
 void fakeSystemHeaderCall(SomeStruct *);
+
+typedef int pid_t;
+pid_t fork(void);
+pid_t vfork(void);
+int execl(const char *path, const char *arg, ...);
+
+void exit(int status) __attribute__ ((__noreturn__));
+void _exit(int status) __attribute__ ((__noreturn__));
+void _Exit(int status) __attribute__ ((__noreturn__));
+
Index: lib/StaticAnalyzer/Checkers/VforkChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/VforkChecker.cpp
@@ -0,0 +1,239 @@
+//===- VforkChecker.cpp  Vfork usage checks --*- C++ -*-==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov marked 15 inline comments as done.


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:45
@@ +44,3 @@
+   CheckerContext &C) {
+  const Expr *CE = Call.getOriginExpr();
+

It seems that other checkers do more or less the same throw-away predicates 
(e.g. see isAssignmentOp in DereferenceChecker.cpp).


Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:164
@@ +163,3 @@
+// TODO: mark vfork call in BugReportVisitor
+C.emitReport(std::move(Report));
+  }

I've added a TODO.


http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov updated this revision to Diff 38926.
ygribov marked 2 inline comments as done.
ygribov added a comment.

Updated after review.


http://reviews.llvm.org/D14014

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  test/Analysis/Inputs/system-header-simulator.h
  test/Analysis/vfork.c

Index: test/Analysis/vfork.c
===
--- /dev/null
+++ test/Analysis/vfork.c
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify -x c++ %s
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Ensure that child process is properly checked.
+int f1(int x) {
+  pid_t pid = vfork();
+  if (pid != 0)
+return 0;
+
+  switch (x) {
+  case 0:
+// Ensure that modifying pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0;
+  }
+
+  while(1);
+} // expected-warning{{Return from this function is prohibited after a successful vfork}}
+
+// Same as previous but without explicit pid variable.
+int f2(int x) {
+  pid_t pid = vfork();
+
+  switch (x) {
+  case 0:
+// Ensure that writing pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0;
+  }
+
+  while(1);
+} // expected-warning{{Return from this function is prohibited after a successful vfork}}
+
+// Ensure that parent process isn't restricted.
+int f3(int x) {
+  if (vfork() == 0)
+_exit(1);
+  x = 0; // no-warning
+  foo(); // no-warning
+  return 0;
+} // no-warning
+
+// Unbound pids are special so test them separately.
+void f4(int x) {
+  switch (x) {
+  case 0:
+vfork();
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+
+  case 1:
+{
+  char args[2];
+  switch (vfork()) {
+  case 0:
+args[0] = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+exit(1);
+  }
+  break;
+}
+
+  case 2:
+{
+  pid_t pid;
+  if ((pid = vfork()) == 0)
+while(1); // no-warning
+  break;
+}
+  }
+  while(1);
+} //no-warning
+
+
+void f5() {
+  // See "libxtables: move some code to avoid cautions in vfork man page"
+  // (http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).
+  if (vfork() == 0) {
+execl("prog", "arg1", 0); // no-warning
+exit(1);  // expected-warning{{This function call is prohibited after a successful vfork}}
+  }
+}
+
Index: test/Analysis/Inputs/system-header-simulator.h
===
--- test/Analysis/Inputs/system-header-simulator.h
+++ test/Analysis/Inputs/system-header-simulator.h
@@ -86,3 +86,13 @@
   char * p;
 } SomeStruct;
 void fakeSystemHeaderCall(SomeStruct *);
+
+typedef int pid_t;
+pid_t fork(void);
+pid_t vfork(void);
+int execl(const char *path, const char *arg, ...);
+
+void exit(int status) __attribute__ ((__noreturn__));
+void _exit(int status) __attribute__ ((__noreturn__));
+void _Exit(int status) __attribute__ ((__noreturn__));
+
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 
 // Recursively find any substatements containing macros
@@ -70,3 +71,26 @@
 
   return false;
 }
+
+// Extract lhs and rhs from assi

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov marked 5 inline comments as done.
ygribov added a comment.

http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:118
@@ +117,3 @@
+std::tie(VD, Init) = parseAssignment(S);
+if (VD && Init)
+  S = Init;

Semantics is slightly changed for assignment case here: originally S would have 
been changed iff Init is not NULL but now I also require valid VarDecl on LHS. 
Frankly I'm not sure if it's a serious change (probably not).


http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov updated this revision to Diff 39040.
ygribov added a comment.

Updated warning messages.


http://reviews.llvm.org/D14014

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  test/Analysis/Inputs/system-header-simulator.h
  test/Analysis/vfork.c

Index: test/Analysis/vfork.c
===
--- /dev/null
+++ test/Analysis/vfork.c
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.Vfork -verify -x c++ %s
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Ensure that child process is properly checked.
+int f1(int x) {
+  pid_t pid = vfork();
+  if (pid != 0)
+return 0;
+
+  switch (x) {
+  case 0:
+// Ensure that modifying pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0; // expected-warning{{Return is prohibited after a successful vfork; call _exit() instead}}
+  }
+
+  while(1);
+}
+
+// Same as previous but without explicit pid variable.
+int f2(int x) {
+  pid_t pid = vfork();
+
+  switch (x) {
+  case 0:
+// Ensure that writing pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0; // expected-warning{{Return is prohibited after a successful vfork; call _exit() instead}}
+  }
+
+  while(1);
+}
+
+// Ensure that parent process isn't restricted.
+int f3(int x) {
+  if (vfork() == 0)
+_exit(1);
+  x = 0; // no-warning
+  foo(); // no-warning
+  return 0;
+} // no-warning
+
+// Unbound pids are special so test them separately.
+void f4(int x) {
+  switch (x) {
+  case 0:
+vfork();
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+
+  case 1:
+{
+  char args[2];
+  switch (vfork()) {
+  case 0:
+args[0] = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+exit(1);
+  }
+  break;
+}
+
+  case 2:
+{
+  pid_t pid;
+  if ((pid = vfork()) == 0)
+while(1); // no-warning
+  break;
+}
+  }
+  while(1);
+} //no-warning
+
+
+void f5() {
+  // See "libxtables: move some code to avoid cautions in vfork man page"
+  // (http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).
+  if (vfork() == 0) {
+execl("prog", "arg1", 0); // no-warning
+exit(1);  // expected-warning{{This function call is prohibited after a successful vfork}}
+  }
+}
+
Index: test/Analysis/Inputs/system-header-simulator.h
===
--- test/Analysis/Inputs/system-header-simulator.h
+++ test/Analysis/Inputs/system-header-simulator.h
@@ -86,3 +86,13 @@
   char * p;
 } SomeStruct;
 void fakeSystemHeaderCall(SomeStruct *);
+
+typedef int pid_t;
+pid_t fork(void);
+pid_t vfork(void);
+int execl(const char *path, const char *arg, ...);
+
+void exit(int status) __attribute__ ((__noreturn__));
+void _exit(int status) __attribute__ ((__noreturn__));
+void _Exit(int status) __attribute__ ((__noreturn__));
+
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 
 // Recursively find any substatements containing macros
@@ -70,3 +71,26 @@
 
   return false;
 }
+
+// Extract lhs and rhs from assignment statement
+std::pair
+cla

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov marked 2 inline comments as done.
ygribov added a comment.

http://reviews.llvm.org/D14014



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

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov updated this revision to Diff 39345.
ygribov added a comment.

Moved to unix package (and thus enabled by default). I now also test for 
collaboration with security.InsecureAPI.vfork.


http://reviews.llvm.org/D14014

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  test/Analysis/Inputs/system-header-simulator.h
  test/Analysis/vfork.c

Index: test/Analysis/vfork.c
===
--- /dev/null
+++ test/Analysis/vfork.c
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,security.insecureAPI.vfork,unix.Vfork -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,security.insecureAPI.vfork,unix.Vfork -verify -x c++ %s
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Ensure that child process is properly checked.
+int f1(int x) {
+  pid_t pid = vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+  if (pid != 0)
+return 0;
+
+  switch (x) {
+  case 0:
+// Ensure that modifying pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0; // expected-warning{{Return is prohibited after a successful vfork; call _exit() instead}}
+  }
+
+  while(1);
+}
+
+// Same as previous but without explicit pid variable.
+int f2(int x) {
+  pid_t pid = vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+
+  switch (x) {
+  case 0:
+// Ensure that writing pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0; // expected-warning{{Return is prohibited after a successful vfork; call _exit() instead}}
+  }
+
+  while(1);
+}
+
+// Ensure that parent process isn't restricted.
+int f3(int x) {
+  if (vfork() == 0) // expected-warning{{Call to function 'vfork' is insecure}}
+_exit(1);
+  x = 0; // no-warning
+  foo(); // no-warning
+  return 0;
+} // no-warning
+
+// Unbound pids are special so test them separately.
+void f4(int x) {
+  switch (x) {
+  case 0:
+vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+
+  case 1:
+{
+  char args[2];
+  switch (vfork()) { // expected-warning{{Call to function 'vfork' is insecure}}
+  case 0:
+args[0] = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+exit(1);
+  }
+  break;
+}
+
+  case 2:
+{
+  pid_t pid;
+  if ((pid = vfork()) == 0) // expected-warning{{Call to function 'vfork' is insecure}}
+while(1); // no-warning
+  break;
+}
+  }
+  while(1);
+} //no-warning
+
+
+void f5() {
+  // See "libxtables: move some code to avoid cautions in vfork man page"
+  // (http://lists.netfilter.org/pipermail/netfilter-buglog/2014-October/003280.html).
+  if (vfork() == 0) { // expected-warning{{Call to function 'vfork' is insecure}}
+execl("prog", "arg1", 0); // no-warning
+exit(1);  // expected-warning{{This function call is prohibited after a successful vfork}}
+  }
+}
+
Index: test/Analysis/Inputs/system-header-simulator.h
===
--- test/Analysis/Inputs/system-header-simulator.h
+++ test/Analysis/Inputs/system-header-simulator.h
@@ -92,3 +92,13 @@
   char * p;
 } SomeStruct;
 void fakeSystemHeaderCall(SomeStruct *);
+
+typedef int pid_t;
+pid_t fork(void);
+pid_t vfork(void);
+int execl(const char *path, const char *arg, ...);
+
+void exit(int status) __attribute__ ((__noreturn__));
+void _exit(int status) __attribute__ ((__noreturn__));
+void _Exit(int status) __attribute__ ((__noreturn__));
+
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
=

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added a comment.

> I now also test for collaboration with security.InsecureAPI.vfork.


Should probably clarify: I've added checks to testcase to verify that new 
checker properly interacts with (existing) InsecureAPI.vfork checker,


http://reviews.llvm.org/D14014



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

r252285 - [analyzer] Add VforkChecker to find unsafe code in vforked process.

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Fri Nov  6 05:16:31 2015
New Revision: 252285

URL: http://llvm.org/viewvc/llvm-project?rev=252285&view=rev
Log:
[analyzer] Add VforkChecker to find unsafe code in vforked process.

This checker looks for unsafe constructs in vforked process:
function calls (excluding whitelist), memory write and returns.
This was originally motivated by a vfork-related bug in xtables package.

Patch by Yury Gribov.

Differential revision: http://reviews.llvm.org/D14014

Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
cfe/trunk/test/Analysis/vfork.c
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
cfe/trunk/test/Analysis/Inputs/system-header-simulator.h

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h?rev=252285&r1=252284&r2=252285&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h 
Fri Nov  6 05:16:31 2015
@@ -15,9 +15,13 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
 
 #include "clang/AST/Stmt.h"
+#include 
 
 namespace clang {
 
+class Expr;
+class VarDecl;
+
 namespace ento {
 
 bool containsMacro(const Stmt *S);
@@ -35,6 +39,9 @@ template  bool containsStmt(con
   return false;
 }
 
+std::pair
+parseAssignment(const Stmt *S);
+
 } // end GR namespace
 
 } // end clang namespace

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=252285&r1=252284&r2=252285&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Fri Nov  6 05:16:31 
2015
@@ -76,6 +76,7 @@ add_clang_library(clangStaticAnalyzerChe
   UndefinedAssignmentChecker.cpp
   UnixAPIChecker.cpp
   UnreachableCodeChecker.cpp
+  VforkChecker.cpp
   VLASizeChecker.cpp
   VirtualCallChecker.cpp
 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td?rev=252285&r1=252284&r2=252285&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td Fri Nov  6 05:16:31 2015
@@ -362,6 +362,10 @@ def MismatchedDeallocatorChecker : Check
   HelpText<"Check for mismatched deallocators.">,
   DescFile<"MallocChecker.cpp">;
 
+def VforkChecker : Checker<"Vfork">,
+  HelpText<"Check for proper usage of vfork">,
+  DescFile<"VforkChecker.cpp">;
+
 } // end "unix"
 
 let ParentPackage = UnixAlpha in {

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp?rev=252285&r1=252284&r2=252285&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp Fri Nov  6 
05:16:31 2015
@@ -19,6 +19,7 @@
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -111,15 +112,11 @@ void DereferenceChecker::reportBug(Progr
 S = expr->IgnoreParenLValueCasts();
 
   if (IsBind) {
-if (const BinaryOperator *BO = dyn_cast(S)) {
-  if (BO->isAssignmentOp())
-S = BO->getRHS();
-} else if (const DeclStmt *DS = dyn_cast(S)) {
-  assert(DS->isSingleDecl() && "We process decls one by one");
-  if (const VarDecl *VD = dyn_cast(DS->getSingleDecl()))
-if (const Expr *Init = VD->getAnyInitializer())
-  S = Init;
-}
+const VarDecl *VD;
+const Expr *Init;
+std::tie(VD, Init) = parseAssignment(S);
+if (VD && Init)
+  S = Init;
   }
 
   switch (S->getStmtClass()) {

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp?rev=252285&view=auto
==

Re: [PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL252285: [analyzer] Add VforkChecker to find unsafe code in 
vforked process. (authored by ygribov).

Changed prior to commit:
  http://reviews.llvm.org/D14014?vs=39345&id=39508#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D14014

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Checkers/Checkers.td
  cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  cfe/trunk/test/Analysis/Inputs/system-header-simulator.h
  cfe/trunk/test/Analysis/vfork.c

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -15,9 +15,13 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
 
 #include "clang/AST/Stmt.h"
+#include 
 
 namespace clang {
 
+class Expr;
+class VarDecl;
+
 namespace ento {
 
 bool containsMacro(const Stmt *S);
@@ -35,6 +39,9 @@
   return false;
 }
 
+std::pair
+parseAssignment(const Stmt *S);
+
 } // end GR namespace
 
 } // end clang namespace
Index: cfe/trunk/test/Analysis/vfork.c
===
--- cfe/trunk/test/Analysis/vfork.c
+++ cfe/trunk/test/Analysis/vfork.c
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,security.insecureAPI.vfork,unix.Vfork -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,security.insecureAPI.vfork,unix.Vfork -verify -x c++ %s
+
+#include "Inputs/system-header-simulator.h"
+
+void foo();
+
+// Ensure that child process is properly checked.
+int f1(int x) {
+  pid_t pid = vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+  if (pid != 0)
+return 0;
+
+  switch (x) {
+  case 0:
+// Ensure that modifying pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0; // expected-warning{{Return is prohibited after a successful vfork; call _exit() instead}}
+  }
+
+  while(1);
+}
+
+// Same as previous but without explicit pid variable.
+int f2(int x) {
+  pid_t pid = vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+
+  switch (x) {
+  case 0:
+// Ensure that writing pid is ok.
+pid = 1; // no-warning
+// Ensure that calling whitelisted routines is ok.
+execl("", "", 0); // no-warning
+_exit(1); // no-warning
+break;
+  case 1:
+// Ensure that writing variables is prohibited.
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+  case 2:
+// Ensure that calling functions is prohibited.
+foo(); // expected-warning{{This function call is prohibited after a successful vfork}}
+break;
+  default:
+// Ensure that returning from function is prohibited.
+return 0; // expected-warning{{Return is prohibited after a successful vfork; call _exit() instead}}
+  }
+
+  while(1);
+}
+
+// Ensure that parent process isn't restricted.
+int f3(int x) {
+  if (vfork() == 0) // expected-warning{{Call to function 'vfork' is insecure}}
+_exit(1);
+  x = 0; // no-warning
+  foo(); // no-warning
+  return 0;
+} // no-warning
+
+// Unbound pids are special so test them separately.
+void f4(int x) {
+  switch (x) {
+  case 0:
+vfork(); // expected-warning{{Call to function 'vfork' is insecure}}
+x = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+break;
+
+  case 1:
+{
+  char args[2];
+  switch (vfork()) { // expected-warning{{Call to function 'vfork' is insecure}}
+  case 0:
+args[0] = 0; // expected-warning{{This assignment is prohibited after a successful vfork}}
+exit(1);
+  }
+  break;
+}
+
+  case 2:
+{
+  pid_t pid;
+  if ((pid = vfork()) == 0) // expected-warning{{Call to function 'vfork' is insecure}}
+while(1); // no-warning
+  break;
+}
+  }
+  while(1);
+} //no-warning
+
+
+void f5() {
+  // See "libxtables: move some code to avoid cautions in vfork man page"
+  // (http://lists.netfilter.org/pipermail/netfilter-bug

r252721 - [ASan] Allow -fsanitize-recover=address.

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Wed Nov 11 04:45:48 2015
New Revision: 252721

URL: http://llvm.org/viewvc/llvm-project?rev=252721&view=rev
Log:
[ASan] Allow -fsanitize-recover=address.

Differential Revision: http://reviews.llvm.org/D14243

Modified:
cfe/trunk/docs/UsersManual.rst
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=252721&r1=252720&r2=252721&view=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Wed Nov 11 04:45:48 2015
@@ -1096,8 +1096,9 @@ are listed below.
 
By default, non-fatal checks are those enabled by 
UndefinedBehaviorSanitizer,
except for ``-fsanitize=return`` and ``-fsanitize=unreachable``. Some
-   sanitizers (e.g. :doc:`AddressSanitizer`) may not support recovery,
-   and always crash the program after the issue is detected.
+   sanitizers may not support recovery (or not support it by default
+   e.g. :doc:`AddressSanitizer`), and always crash the program after the issue
+   is detected.
 
Note that the ``-fsanitize-trap`` flag has precedence over this flag.
This means that if a check has been configured to trap elsewhere on the

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=252721&r1=252720&r2=252721&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Wed Nov 11 04:45:48 2015
@@ -194,14 +194,20 @@ static void addSanitizerCoveragePass(con
 
 static void addAddressSanitizerPasses(const PassManagerBuilder &Builder,
   legacy::PassManagerBase &PM) {
-  PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/false));
-  PM.add(createAddressSanitizerModulePass(/*CompileKernel*/false));
+  const PassManagerBuilderWrapper &BuilderWrapper =
+  static_cast(Builder);
+  const CodeGenOptions &CGOpts = BuilderWrapper.getCGOpts();
+  bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Address);
+  PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/false, Recover));
+  PM.add(createAddressSanitizerModulePass(/*CompileKernel*/false, Recover));
 }
 
 static void addKernelAddressSanitizerPasses(const PassManagerBuilder &Builder,
 legacy::PassManagerBase &PM) {
-  PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/true));
-  PM.add(createAddressSanitizerModulePass(/*CompileKernel*/true));
+  PM.add(createAddressSanitizerFunctionPass(/*CompileKernel*/true,
+/*Recover*/true));
+  PM.add(createAddressSanitizerModulePass(/*CompileKernel*/true,
+  /*Recover*/true));
 }
 
 static void addMemorySanitizerPass(const PassManagerBuilder &Builder,

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=252721&r1=252720&r2=252721&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Nov 11 04:45:48 2015
@@ -33,7 +33,7 @@ enum : SanitizerMask {
   NeedsUnwindTables = Address | Thread | Memory | DataFlow,
   SupportsCoverage = Address | Memory | Leak | Undefined | Integer | DataFlow,
   RecoverableByDefault = Undefined | Integer,
-  Unrecoverable = Address | Unreachable | Return,
+  Unrecoverable = Unreachable | Return,
   LegacyFsanitizeRecoverMask = Undefined | Integer,
   NeedsLTO = CFI,
   TrappingSupported =

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=252721&r1=252720&r2=252721&view=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Wed Nov 11 04:45:48 2015
@@ -160,22 +160,25 @@
 // RUN: %clang -target arm-linux-androideabi %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ANDROID-NO-ASAN
 // CHECK-ANDROID-NO-ASAN: "-mrelocation-model" "pic"
 
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-RECOVER
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined 
-fsanitize-recover -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined 
-fsanitize-recover=all -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined 
-fno-sanitize-recover=undefined -### 2>&1 | FileCheck %s 
--check-prefix=CHEC

r259646 - [analyzer] AnalysisConsumer: print fully-qualified function name while displaying progress

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Wed Feb  3 07:35:33 2016
New Revision: 259646

URL: http://llvm.org/viewvc/llvm-project?rev=259646&view=rev
Log:
[analyzer] AnalysisConsumer: print fully-qualified function name while 
displaying progress

-analyzer-display progress option prints only function names which may be 
ambiguous. This patch forces AnalysisConsumer to print fully-qualified function 
names.
Patch by Alex Sidorin!

Differential Revision: http://reviews.llvm.org/D16804

Added:
cfe/trunk/test/Analysis/analyze_display_progress.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=259646&r1=259645&r2=259646&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Wed Feb  3 
07:35:33 2016
@@ -274,7 +274,7 @@ public:
   llvm::errs() << ": " << Loc.getFilename();
   if (isa(D) || isa(D)) {
 const NamedDecl *ND = cast(D);
-llvm::errs() << ' ' << *ND << '\n';
+llvm::errs() << ' ' << ND->getQualifiedNameAsString() << '\n';
   }
   else if (isa(D)) {
 llvm::errs() << ' ' << "block(line:" << Loc.getLine() << ",col:"

Added: cfe/trunk/test/Analysis/analyze_display_progress.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyze_display_progress.cpp?rev=259646&view=auto
==
--- cfe/trunk/test/Analysis/analyze_display_progress.cpp (added)
+++ cfe/trunk/test/Analysis/analyze_display_progress.cpp Wed Feb  3 07:35:33 
2016
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+void f() {};
+void g() {};
+void h() {}
+
+struct SomeStruct {
+  void f() {}
+};
+
+struct SomeOtherStruct {
+  void f() {}
+};
+
+namespace ns {
+  struct SomeStruct {
+void f() {}
+  };
+}
+
+// CHECK: analyze_display_progress.cpp f
+// CHECK: analyze_display_progress.cpp g
+// CHECK: analyze_display_progress.cpp h
+// CHECK: analyze_display_progress.cpp SomeStruct::f
+// CHECK: analyze_display_progress.cpp SomeOtherStruct::f
+// CHECK: analyze_display_progress.cpp ns::SomeStruct::f


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

Re: [PATCH] D16804: [analyzer] AnalysisConsumer: print fully-qualified function name while displaying progress

[Yury Gribov via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL259646: [analyzer] AnalysisConsumer: print fully-qualified 
function name while… (authored by ygribov).

Changed prior to commit:
  http://reviews.llvm.org/D16804?vs=46640&id=46777#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D16804

Files:
  cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  cfe/trunk/test/Analysis/analyze_display_progress.cpp

Index: cfe/trunk/test/Analysis/analyze_display_progress.cpp
===
--- cfe/trunk/test/Analysis/analyze_display_progress.cpp
+++ cfe/trunk/test/Analysis/analyze_display_progress.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+void f() {};
+void g() {};
+void h() {}
+
+struct SomeStruct {
+  void f() {}
+};
+
+struct SomeOtherStruct {
+  void f() {}
+};
+
+namespace ns {
+  struct SomeStruct {
+void f() {}
+  };
+}
+
+// CHECK: analyze_display_progress.cpp f
+// CHECK: analyze_display_progress.cpp g
+// CHECK: analyze_display_progress.cpp h
+// CHECK: analyze_display_progress.cpp SomeStruct::f
+// CHECK: analyze_display_progress.cpp SomeOtherStruct::f
+// CHECK: analyze_display_progress.cpp ns::SomeStruct::f
Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -274,7 +274,7 @@
   llvm::errs() << ": " << Loc.getFilename();
   if (isa(D) || isa(D)) {
 const NamedDecl *ND = cast(D);
-llvm::errs() << ' ' << *ND << '\n';
+llvm::errs() << ' ' << ND->getQualifiedNameAsString() << '\n';
   }
   else if (isa(D)) {
 llvm::errs() << ' ' << "block(line:" << Loc.getLine() << ",col:"


Index: cfe/trunk/test/Analysis/analyze_display_progress.cpp
===
--- cfe/trunk/test/Analysis/analyze_display_progress.cpp
+++ cfe/trunk/test/Analysis/analyze_display_progress.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+void f() {};
+void g() {};
+void h() {}
+
+struct SomeStruct {
+  void f() {}
+};
+
+struct SomeOtherStruct {
+  void f() {}
+};
+
+namespace ns {
+  struct SomeStruct {
+void f() {}
+  };
+}
+
+// CHECK: analyze_display_progress.cpp f
+// CHECK: analyze_display_progress.cpp g
+// CHECK: analyze_display_progress.cpp h
+// CHECK: analyze_display_progress.cpp SomeStruct::f
+// CHECK: analyze_display_progress.cpp SomeOtherStruct::f
+// CHECK: analyze_display_progress.cpp ns::SomeStruct::f
Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -274,7 +274,7 @@
   llvm::errs() << ": " << Loc.getFilename();
   if (isa(D) || isa(D)) {
 const NamedDecl *ND = cast(D);
-llvm::errs() << ' ' << *ND << '\n';
+llvm::errs() << ' ' << ND->getQualifiedNameAsString() << '\n';
   }
   else if (isa(D)) {
 llvm::errs() << ' ' << "block(line:" << Loc.getLine() << ",col:"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r259647 - Forgot to remove file in previous commit.

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Wed Feb  3 07:36:31 2016
New Revision: 259647

URL: http://llvm.org/viewvc/llvm-project?rev=259647&view=rev
Log:
Forgot to remove file in previous commit.

Removed:
cfe/trunk/test/Analysis/analyze_display_progress.c

Removed: cfe/trunk/test/Analysis/analyze_display_progress.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyze_display_progress.c?rev=259646&view=auto
==
--- cfe/trunk/test/Analysis/analyze_display_progress.c (original)
+++ cfe/trunk/test/Analysis/analyze_display_progress.c (removed)
@@ -1,9 +0,0 @@
-// RUN: %clang_cc1 -analyze -analyzer-display-progress %s 2>&1 | FileCheck %s
-
-void f() {};
-void g() {};
-void h() {}
-
-// CHECK: analyze_display_progress.c f
-// CHECK: analyze_display_progress.c g
-// CHECK: analyze_display_progress.c h
\ No newline at end of file


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

r261204 - [analyzer] Add --force-analyze-debug-code option to scan-build

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Thu Feb 18 05:08:46 2016
New Revision: 261204

URL: http://llvm.org/viewvc/llvm-project?rev=261204&view=rev
Log:
[analyzer] Add --force-analyze-debug-code option to scan-build
to force debug build and hopefully enable more precise warnings.

Static Analyzer is much more efficient when built in debug mode
(-UNDEBUG) so we advice users to enable it manually. This may be
inconvenient in case of large complex projects (think about Linux
distros e.g. Android or Tizen). This patch adds a flag to scan-build
which inserts -UNDEBUG automatically.

Differential Revision: http://reviews.llvm.org/D16200

Modified:
cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
cfe/trunk/tools/scan-build/bin/scan-build
cfe/trunk/tools/scan-build/libexec/ccc-analyzer
cfe/trunk/www/analyzer/scan-build.html

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py?rev=261204&r1=261203&r2=261204&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py Thu Feb 18 05:08:46 
2016
@@ -106,7 +106,8 @@ def run_analyzer(args, output_dir):
 'output_dir': output_dir,
 'output_format': args.output_format,
 'output_failures': args.output_failures,
-'direct_args': analyzer_params(args)
+'direct_args': analyzer_params(args),
+'force_analyze_debug_code' : args.force_analyze_debug_code
 }
 
 logging.debug('run analyzer against compilation database')
@@ -138,7 +139,9 @@ def setup_environment(args, destination,
 'ANALYZE_BUILD_REPORT_DIR': destination,
 'ANALYZE_BUILD_REPORT_FORMAT': args.output_format,
 'ANALYZE_BUILD_REPORT_FAILURES': 'yes' if args.output_failures else '',
-'ANALYZE_BUILD_PARAMETERS': ' '.join(analyzer_params(args))
+'ANALYZE_BUILD_PARAMETERS': ' '.join(analyzer_params(args)),
+'ANALYZE_BUILD_FORCE_ANALYZE_DEBUG_CODE'
+: 'yes' if args.force_analyze_debug_code else ''
 })
 return environment
 
@@ -168,6 +171,8 @@ def analyze_build_wrapper(cplusplus):
 'output_failures': os.getenv('ANALYZE_BUILD_REPORT_FAILURES'),
 'direct_args': os.getenv('ANALYZE_BUILD_PARAMETERS',
  '').split(' '),
+'force_analyze_debug_code':
+os.getenv('ANALYZE_BUILD_FORCE_ANALYZE_DEBUG_CODE'),
 'directory': os.getcwd(),
 }
 # get relevant parameters from command line arguments
@@ -450,6 +455,13 @@ def create_parser(from_build_command):
 Could be usefull when project contains 3rd party libraries.
 The directory path shall be absolute path as file names in
 the compilation database.""")
+advanced.add_argument(
+'--force-analyze-debug-code',
+dest='force_analyze_debug_code',
+action='store_true',
+help="""Tells analyzer to enable assertions in code even if they were
+disabled during compilation, enabling more precise
+results.""")
 
 plugins = parser.add_argument_group('checker options')
 plugins.add_argument(

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/runner.py?rev=261204&r1=261203&r2=261204&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py Thu Feb 18 05:08:46 
2016
@@ -41,6 +41,7 @@ def require(required):
 
 @require(['command', 'directory', 'file',  # an entry from compilation database
   'clang', 'direct_args',  # compiler name, and arguments from command
+  'force_analyze_debug_code',  # preprocessing options
   'output_dir', 'output_format', 'output_failures'])
 def run(opts):
 """ Entry point to run (or not) static analyzer against a single entry
@@ -164,9 +165,13 @@ def set_analyzer_output(opts, continuati
 opts.update({'output': ['-o', opts['output_dir']]})
 return continuation(opts)
 
-
-@require(['file', 'directory', 'clang', 'direct_args', 'language',
-  'output_dir', 'output_format', 'output_failures'])
+def force_analyze_debug_code(cmd):
+""" Enable assert()'s by undefining NDEBUG. """
+cmd.append('-UNDEBUG')
+
+@require(['file', 'directory', 'clang', 'direct_args',
+  'force_analyze_debug_code', 'language', 'output_dir',
+  'output_format', 'output_failures'])
 def create_commands(opts, continuation=set_analyzer_output):
 """ Create comman

Re: [PATCH] D17376: dump_ast_matchers.py: fix replacement regexps

[Yury Gribov via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL261219: [analyzer] dump_ast_matchers.py: fix replacement 
regexps (authored by ygribov).

Changed prior to commit:
  http://reviews.llvm.org/D17376?vs=48306&id=48316#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D17376

Files:
  cfe/trunk/docs/tools/dump_ast_matchers.py

Index: cfe/trunk/docs/tools/dump_ast_matchers.py
===
--- cfe/trunk/docs/tools/dump_ast_matchers.py
+++ cfe/trunk/docs/tools/dump_ast_matchers.py
@@ -363,11 +363,11 @@
 
 reference = open('../LibASTMatchersReference.html').read()
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % node_matcher_table
+   node_matcher_table, reference, flags=re.S)
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % narrowing_matcher_table
+   narrowing_matcher_table, reference, flags=re.S)
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % traversal_matcher_table
+   traversal_matcher_table, reference, flags=re.S)
 
 with open('../LibASTMatchersReference.html', 'wb') as output:
   output.write(reference)


Index: cfe/trunk/docs/tools/dump_ast_matchers.py
===
--- cfe/trunk/docs/tools/dump_ast_matchers.py
+++ cfe/trunk/docs/tools/dump_ast_matchers.py
@@ -363,11 +363,11 @@
 
 reference = open('../LibASTMatchersReference.html').read()
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % node_matcher_table
+   node_matcher_table, reference, flags=re.S)
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % narrowing_matcher_table
+   narrowing_matcher_table, reference, flags=re.S)
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % traversal_matcher_table
+   traversal_matcher_table, reference, flags=re.S)
 
 with open('../LibASTMatchersReference.html', 'wb') as output:
   output.write(reference)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r261219 - [analyzer] dump_ast_matchers.py: fix replacement regexps

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Thu Feb 18 09:43:56 2016
New Revision: 261219

URL: http://llvm.org/viewvc/llvm-project?rev=261219&view=rev
Log:
[analyzer] dump_ast_matchers.py: fix replacement regexps

Patch by Alex Sidorin!

Differential Revision: http://reviews.llvm.org/D17376

Modified:
cfe/trunk/docs/tools/dump_ast_matchers.py

Modified: cfe/trunk/docs/tools/dump_ast_matchers.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/tools/dump_ast_matchers.py?rev=261219&r1=261218&r2=261219&view=diff
==
--- cfe/trunk/docs/tools/dump_ast_matchers.py (original)
+++ cfe/trunk/docs/tools/dump_ast_matchers.py Thu Feb 18 09:43:56 2016
@@ -363,11 +363,11 @@ traversal_matcher_table = sort_table('TR
 
 reference = open('../LibASTMatchersReference.html').read()
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % node_matcher_table
+   node_matcher_table, reference, flags=re.S)
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % narrowing_matcher_table
+   narrowing_matcher_table, reference, flags=re.S)
 reference = re.sub(r'',
-   '%s', reference, flags=re.S) % traversal_matcher_table
+   traversal_matcher_table, reference, flags=re.S)
 
 with open('../LibASTMatchersReference.html', 'wb') as output:
   output.write(reference)


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

Re: [PATCH] D19666: [ubsan] Add -fubsan-strip-path-components=N

[Yury Gribov via cfe-commits]

ygribov added a subscriber: ygribov.
ygribov added a comment.

Can we have generic option for other sanitizers?


http://reviews.llvm.org/D19666



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

Re: [PATCH] D15888: [Analyzer] Change the default SA checkers for PS4

[Yury Gribov via cfe-commits]

ygribov added a subscriber: ygribov.
ygribov added a comment.

What's the problem with Vfork though?


http://reviews.llvm.org/D15888



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

r257318 - AnalysisConsumer: use canonical decl for both lookup and store of

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Mon Jan 11 03:38:48 2016
New Revision: 257318

URL: http://llvm.org/viewvc/llvm-project?rev=257318&view=rev
Log:
AnalysisConsumer: use canonical decl for both lookup and store of
visited decls.

Due to redeclarations, the function may have different declarations used
in CallExpr and in the definition. However, we need to use a unique
declaration for both store and lookup in VisitedCallees. This patch
fixes issues with analysis in topological order. A simple test is
included.

Patch by Alex Sidorin!

Differential Revision: http://reviews.llvm.org/D15410

Added:
cfe/trunk/test/Analysis/inlining/analysis-order.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=257318&r1=257317&r2=257318&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Mon Jan 11 
03:38:48 2016
@@ -496,10 +496,11 @@ void AnalysisConsumer::HandleDeclsCallGr
(Mgr->options.InliningMode == All ? nullptr : &VisitedCallees));
 
 // Add the visited callees to the global visited set.
-for (SetOfConstDecls::iterator I = VisitedCallees.begin(),
-   E = VisitedCallees.end(); I != E; ++I) {
-Visited.insert(*I);
-}
+for (const Decl *Callee : VisitedCallees)
+  // Decls from CallGraph are already canonical. But Decls coming from
+  // CallExprs may be not. We should canonicalize them manually.
+  Visited.insert(isa(Callee) ? Callee
+ : Callee->getCanonicalDecl());
 VisitedAsTopLevel.insert(D);
   }
 }

Added: cfe/trunk/test/Analysis/inlining/analysis-order.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/analysis-order.c?rev=257318&view=auto
==
--- cfe/trunk/test/Analysis/inlining/analysis-order.c (added)
+++ cfe/trunk/test/Analysis/inlining/analysis-order.c Mon Jan 11 03:38:48 2016
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin.NoReturnFunctions 
-analyzer-display-progress %s 2>&1 | FileCheck %s
+
+// Do not analyze test1() again because it was inlined
+void test1();
+
+void test2() {
+  test1();
+}
+
+void test1() {
+}
+
+// CHECK: analysis-order.c test2
+// CHECK-NEXT: analysis-order.c test1
+// CHECK-NEXT: analysis-order.c test2


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

Re: [PATCH] D15410: AnalysisConsumer: use canonical decl for both lookup and store of visited decls

[Yury Gribov via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL257318: AnalysisConsumer: use canonical decl for both lookup 
and store of (authored by ygribov).

Changed prior to commit:
  http://reviews.llvm.org/D15410?vs=43726&id=6#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15410

Files:
  cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  cfe/trunk/test/Analysis/inlining/analysis-order.c

Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -496,10 +496,11 @@
(Mgr->options.InliningMode == All ? nullptr : &VisitedCallees));
 
 // Add the visited callees to the global visited set.
-for (SetOfConstDecls::iterator I = VisitedCallees.begin(),
-   E = VisitedCallees.end(); I != E; ++I) {
-Visited.insert(*I);
-}
+for (const Decl *Callee : VisitedCallees)
+  // Decls from CallGraph are already canonical. But Decls coming from
+  // CallExprs may be not. We should canonicalize them manually.
+  Visited.insert(isa(Callee) ? Callee
+ : Callee->getCanonicalDecl());
 VisitedAsTopLevel.insert(D);
   }
 }
Index: cfe/trunk/test/Analysis/inlining/analysis-order.c
===
--- cfe/trunk/test/Analysis/inlining/analysis-order.c
+++ cfe/trunk/test/Analysis/inlining/analysis-order.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin.NoReturnFunctions 
-analyzer-display-progress %s 2>&1 | FileCheck %s
+
+// Do not analyze test1() again because it was inlined
+void test1();
+
+void test2() {
+  test1();
+}
+
+void test1() {
+}
+
+// CHECK: analysis-order.c test2
+// CHECK-NEXT: analysis-order.c test1
+// CHECK-NEXT: analysis-order.c test2


Index: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -496,10 +496,11 @@
(Mgr->options.InliningMode == All ? nullptr : &VisitedCallees));
 
 // Add the visited callees to the global visited set.
-for (SetOfConstDecls::iterator I = VisitedCallees.begin(),
-   E = VisitedCallees.end(); I != E; ++I) {
-Visited.insert(*I);
-}
+for (const Decl *Callee : VisitedCallees)
+  // Decls from CallGraph are already canonical. But Decls coming from
+  // CallExprs may be not. We should canonicalize them manually.
+  Visited.insert(isa(Callee) ? Callee
+ : Callee->getCanonicalDecl());
 VisitedAsTopLevel.insert(D);
   }
 }
Index: cfe/trunk/test/Analysis/inlining/analysis-order.c
===
--- cfe/trunk/test/Analysis/inlining/analysis-order.c
+++ cfe/trunk/test/Analysis/inlining/analysis-order.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core.builtin.NoReturnFunctions -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+// Do not analyze test1() again because it was inlined
+void test1();
+
+void test2() {
+  test1();
+}
+
+void test1() {
+}
+
+// CHECK: analysis-order.c test2
+// CHECK-NEXT: analysis-order.c test1
+// CHECK-NEXT: analysis-order.c test2
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

[Yury Gribov via cfe-commits]

ygribov added a subscriber: ygribov.


Comment at: test/Driver/fsanitize.c:221
@@ +220,3 @@
+// RUN: %clang -target x86_64-apple-darwin10 
-resource-dir=%S/Inputs/resource_dir -fsanitize=memory -fsanitize=thread,memory 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-TSAN-MSAN-DARWIN1
+// CHECK-MSAN-TSAN-MSAN-DARWIN1: unsupported option '-fsanitize=thread,memory' 
for target 'x86_64-apple-darwin10'
+// CHECK-MSAN-TSAN-MSAN-DARWIN1-NOT: unsupported option

samsonov wrote:
> beanz wrote:
> > kubabrecka wrote:
> > > samsonov wrote:
> > > > Again, I feel like we're lying to users here: `-fsanitize=thread` *is* 
> > > > supported for this target, it just requires building a runtime.
> > > I'd like to see this from the point-of-view of a binary distribution.  If 
> > > the binary distribution (e.g. the one from llvm.org or Apple's Clang in 
> > > Xcode) doesn't contain a runtime library, then the sanitizer is *not* 
> > > supported in that distribution.  Also, see 
> > > http://reviews.llvm.org/D14846, we'd like to have CMake options to select 
> > > which runtimes will be built.  If you deliberately choose not to build 
> > > ThreadSanitizer, then that sanitizer is *not* supported in your version 
> > > of Clang.  If you're experimenting and porting a runtime to a new 
> > > platform, then this sanitizer *is* supported in your version of Clang.
> > Maybe the point is we should have a different error message for cases where 
> > the runtime is just missing. Something like "runtime components for 
> > '-fsanitize=thread' not available"
> I see, so essentially you want to use a different approach for determining 
> sanitizer availability (on OS X for now): if the library is present, then we 
> support sanitizer, otherwise we don't: i.e. the binary distribution is the 
> source of truth, not the list of sanitizers hardcoded into Clang driver 
> source code. I'm fine with that, and see why it would make sense.
> 
> It's just that error message looks misleading: the problem is not TSan is 
> unsupported for target, it's just unavailable in this distribution for one 
> reason or another.
> the binary distribution is the source of truth, not the list of sanitizers 
> hardcoded into Clang driver source code.

This will not work for cross-compilers. It _may_ be ok for OSX but not for 
other platforms.


http://reviews.llvm.org/D15225



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

Re: [PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

[Yury Gribov via cfe-commits]

ygribov added inline comments.


Comment at: test/Driver/fsanitize.c:221
@@ +220,3 @@
+// RUN: %clang -target x86_64-apple-darwin10 
-resource-dir=%S/Inputs/resource_dir -fsanitize=memory -fsanitize=thread,memory 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-TSAN-MSAN-DARWIN1
+// CHECK-MSAN-TSAN-MSAN-DARWIN1: unsupported option '-fsanitize=thread,memory' 
for target 'x86_64-apple-darwin10'
+// CHECK-MSAN-TSAN-MSAN-DARWIN1-NOT: unsupported option

kubabrecka wrote:
> ygribov wrote:
> > samsonov wrote:
> > > beanz wrote:
> > > > kubabrecka wrote:
> > > > > samsonov wrote:
> > > > > > Again, I feel like we're lying to users here: `-fsanitize=thread` 
> > > > > > *is* supported for this target, it just requires building a runtime.
> > > > > I'd like to see this from the point-of-view of a binary distribution. 
> > > > >  If the binary distribution (e.g. the one from llvm.org or Apple's 
> > > > > Clang in Xcode) doesn't contain a runtime library, then the sanitizer 
> > > > > is *not* supported in that distribution.  Also, see 
> > > > > http://reviews.llvm.org/D14846, we'd like to have CMake options to 
> > > > > select which runtimes will be built.  If you deliberately choose not 
> > > > > to build ThreadSanitizer, then that sanitizer is *not* supported in 
> > > > > your version of Clang.  If you're experimenting and porting a runtime 
> > > > > to a new platform, then this sanitizer *is* supported in your version 
> > > > > of Clang.
> > > > Maybe the point is we should have a different error message for cases 
> > > > where the runtime is just missing. Something like "runtime components 
> > > > for '-fsanitize=thread' not available"
> > > I see, so essentially you want to use a different approach for 
> > > determining sanitizer availability (on OS X for now): if the library is 
> > > present, then we support sanitizer, otherwise we don't: i.e. the binary 
> > > distribution is the source of truth, not the list of sanitizers hardcoded 
> > > into Clang driver source code. I'm fine with that, and see why it would 
> > > make sense.
> > > 
> > > It's just that error message looks misleading: the problem is not TSan is 
> > > unsupported for target, it's just unavailable in this distribution for 
> > > one reason or another.
> > > the binary distribution is the source of truth, not the list of 
> > > sanitizers hardcoded into Clang driver source code.
> > 
> > This will not work for cross-compilers. It _may_ be ok for OSX but not for 
> > other platforms.
> Why not?  On Linux, there are statically-linked sanitizers, if you want to 
> cross-compile, you need to have the runtime libraries for the target.  And 
> dynamic libraries are a similar story – they're version-tied to the compiler 
> and your compiler should really have the libraries of the sanitizers that it 
> supports.
Ah, for some strange reason I thought that you were searching runtime lib in 
sysroot rather than compiler root.


http://reviews.llvm.org/D15225



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

r259030 - Small refactor in isBeforeInTranslationUnit.

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Thu Jan 28 03:27:46 2016
New Revision: 259030

URL: http://llvm.org/viewvc/llvm-project?rev=259030&view=rev
Log:
Small refactor in isBeforeInTranslationUnit.

Differential Revision: http://reviews.llvm.org/D15804

Modified:
cfe/trunk/lib/Basic/SourceManager.cpp

Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=259030&r1=259029&r2=259030&view=diff
==
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Thu Jan 28 03:27:46 2016
@@ -2089,10 +2089,10 @@ bool SourceManager::isBeforeInTranslatio
 
   // Clear the lookup cache, it depends on a common location.
   IsBeforeInTUCache.clear();
-  llvm::MemoryBuffer *LBuf = getBuffer(LOffs.first);
-  llvm::MemoryBuffer *RBuf = getBuffer(ROffs.first);
-  bool LIsBuiltins = strcmp("", LBuf->getBufferIdentifier()) == 0;
-  bool RIsBuiltins = strcmp("", RBuf->getBufferIdentifier()) == 0;
+  const char *LB = getBuffer(LOffs.first)->getBufferIdentifier();
+  const char *RB = getBuffer(ROffs.first)->getBufferIdentifier();
+  bool LIsBuiltins = strcmp("", LB) == 0;
+  bool RIsBuiltins = strcmp("", RB) == 0;
   // Sort built-in before non-built-in.
   if (LIsBuiltins || RIsBuiltins) {
 if (LIsBuiltins != RIsBuiltins)
@@ -2101,8 +2101,8 @@ bool SourceManager::isBeforeInTranslatio
 // lower IDs come first.
 return LOffs.first < ROffs.first;
   }
-  bool LIsAsm = strcmp("", LBuf->getBufferIdentifier()) == 0;
-  bool RIsAsm = strcmp("", RBuf->getBufferIdentifier()) == 0;
+  bool LIsAsm = strcmp("", LB) == 0;
+  bool RIsAsm = strcmp("", RB) == 0;
   // Sort assembler after built-ins, but before the rest.
   if (LIsAsm || RIsAsm) {
 if (LIsAsm != RIsAsm)


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

r259031 - Fix isBeforeInTranslationUnit to not abort on macros defined in cmdline.

[Yury Gribov via cfe-commits]

Author: ygribov
Date: Thu Jan 28 03:28:18 2016
New Revision: 259031

URL: http://llvm.org/viewvc/llvm-project?rev=259031&view=rev
Log:
Fix isBeforeInTranslationUnit to not abort on macros defined in cmdline.

Differential Revision: http://reviews.llvm.org/D15804

Modified:
cfe/trunk/lib/Basic/SourceManager.cpp

Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=259031&r1=259030&r2=259031&view=diff
==
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Thu Jan 28 03:28:18 2016
@@ -2110,6 +2110,14 @@ bool SourceManager::isBeforeInTranslatio
 assert(LOffs.first == ROffs.first);
 return false;
   }
+  bool LIsScratch = strcmp("", LB) == 0;
+  bool RIsScratch = strcmp("", RB) == 0;
+  // Sort scratch after inline asm, but before the rest.
+  if (LIsScratch || RIsScratch) {
+if (LIsScratch != RIsScratch)
+  return LIsScratch;
+return LOffs.second < ROffs.second;
+  }
   llvm_unreachable("Unsortable locations found");
 }
 


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

[PATCH] D14652: [analyzer] Improve modeling of static initializers.

[Yury Gribov via cfe-commits]

ygribov created this revision.
ygribov added reviewers: zaks.anna, dcoughlin, jordan_rose.
ygribov added a subscriber: cfe-commits.

Conversions between unrelated pointer types (e.g. char * and void *) involve 
bitcasts which were not properly modeled in case of static initializers. The 
patch fixes this problem.

The problem was originally spotted by Artem Dergachev.

http://reviews.llvm.org/D14652

Files:
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  test/Analysis/inline.cpp

Index: test/Analysis/inline.cpp
===
--- test/Analysis/inline.cpp
+++ test/Analysis/inline.cpp
@@ -275,7 +275,7 @@
 
 clang_analyzer_eval(defaultReferenceZero(1) == -1); // 
expected-warning{{TRUE}}
 clang_analyzer_eval(defaultReferenceZero() == 0); // 
expected-warning{{TRUE}}
-}
+  }
 
   double defaultFloatReference(const double &i = 42) {
 return -i;
@@ -300,6 +300,13 @@
 clang_analyzer_eval(defaultString("xyz") == 'y'); // 
expected-warning{{TRUE}}
 clang_analyzer_eval(defaultString() == 'b'); // expected-warning{{TRUE}}
   }
+
+  const void * const void_string = "abc";
+
+  void testBitcastedString() {
+clang_analyzer_eval(0 != void_string); // expected-warning{{TRUE}}
+clang_analyzer_eval(0 != ((char *)void_string)[1]); // 
expected-warning{{TRUE}}
+  }
 }
 
 namespace OperatorNew {
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -275,11 +275,17 @@
 
   case Stmt::ImplicitCastExprClass: {
 const CastExpr *CE = cast(E);
-if (CE->getCastKind() == CK_ArrayToPointerDecay) {
-  Optional ArrayVal = getConstantVal(CE->getSubExpr());
-  if (!ArrayVal)
+switch (CE->getCastKind()) {
+default:
+  break;
+case CK_ArrayToPointerDecay:
+case CK_BitCast: {
+  const Expr *SE = CE->getSubExpr();
+  Optional Val = getConstantVal(SE);
+  if (!Val)
 return None;
-  return evalCast(*ArrayVal, CE->getType(), CE->getSubExpr()->getType());
+  return evalCast(*Val, CE->getType(), SE->getType());
+}
 }
 // FALLTHROUGH
   }


Index: test/Analysis/inline.cpp
===
--- test/Analysis/inline.cpp
+++ test/Analysis/inline.cpp
@@ -275,7 +275,7 @@
 
 clang_analyzer_eval(defaultReferenceZero(1) == -1); // expected-warning{{TRUE}}
 clang_analyzer_eval(defaultReferenceZero() == 0); // expected-warning{{TRUE}}
-}
+  }
 
   double defaultFloatReference(const double &i = 42) {
 return -i;
@@ -300,6 +300,13 @@
 clang_analyzer_eval(defaultString("xyz") == 'y'); // expected-warning{{TRUE}}
 clang_analyzer_eval(defaultString() == 'b'); // expected-warning{{TRUE}}
   }
+
+  const void * const void_string = "abc";
+
+  void testBitcastedString() {
+clang_analyzer_eval(0 != void_string); // expected-warning{{TRUE}}
+clang_analyzer_eval(0 != ((char *)void_string)[1]); // expected-warning{{TRUE}}
+  }
 }
 
 namespace OperatorNew {
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -275,11 +275,17 @@
 
   case Stmt::ImplicitCastExprClass: {
 const CastExpr *CE = cast(E);
-if (CE->getCastKind() == CK_ArrayToPointerDecay) {
-  Optional ArrayVal = getConstantVal(CE->getSubExpr());
-  if (!ArrayVal)
+switch (CE->getCastKind()) {
+default:
+  break;
+case CK_ArrayToPointerDecay:
+case CK_BitCast: {
+  const Expr *SE = CE->getSubExpr();
+  Optional Val = getConstantVal(SE);
+  if (!Val)
 return None;
-  return evalCast(*ArrayVal, CE->getType(), CE->getSubExpr()->getType());
+  return evalCast(*Val, CE->getType(), SE->getType());
+}
 }
 // FALLTHROUGH
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14652: [analyzer] Improve modeling of static initializers.

[Yury Gribov via cfe-commits]

ygribov added inline comments.


Comment at: test/Analysis/inline.cpp:308
@@ +307,3 @@
+clang_analyzer_eval(0 != void_string); // expected-warning{{TRUE}}
+clang_analyzer_eval(0 != ((char *)void_string)[1]); // 
expected-warning{{TRUE}}
+  }

zaks.anna wrote:
> Why are we checking that the first element is not '0'?
We could check s[0] as well, there is no difference actually.


http://reviews.llvm.org/D14652



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

Re: [PATCH] D14652: [analyzer] Improve modeling of static initializers.

[Yury Gribov via cfe-commits]

ygribov added inline comments.


Comment at: test/Analysis/inline.cpp:308
@@ +307,3 @@
+clang_analyzer_eval(0 != void_string); // expected-warning{{TRUE}}
+clang_analyzer_eval(0 != ((char *)void_string)[1]); // 
expected-warning{{TRUE}}
+  }

zaks.anna wrote:
> ygribov wrote:
> > zaks.anna wrote:
> > > Why are we checking that the first element is not '0'?
> > We could check s[0] as well, there is no difference actually.
> Why we are checking for non-equality to '0' instead of checking for equality 
> to 'b'?
Ah yeah, that would be more precise.


Repository:
  rL LLVM

http://reviews.llvm.org/D14652



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