[PATCH] D55654: [clang] [Driver] [NetBSD] Add -D_REENTRANT when using sanitizers

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: lib/Driver/ToolChains/NetBSD.cpp:465
+  const SanitizerArgs &SanArgs = getSanitizerArgs();
+  if (SanArgs.needsAsanRt() || SanArgs.needsHwasanRt() ||
+  SanArgs.needsDfsanRt() || SanArgs.needsLsanRt() ||

There should be some way to express any sanitizer (-fsanitize) and we want 
_REENTRANT for all of them.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55654



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

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Looks good. Is there anything in www/cxx_status.html that we should update to 
track that we've implemented this part of P1353R0?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55741



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

[PATCH] D55657: [libcxx] [regex] Use distinct __regex_word on NetBSD

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349293: [regex] Use distinct __regex_word on NetBSD 
(authored by mgorny, committed by ).
Herald added subscribers: llvm-commits, christof.

Changed prior to commit:
  https://reviews.llvm.org/D55657?vs=178113&id=178387#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55657

Files:
  libcxx/trunk/include/regex


Index: libcxx/trunk/include/regex
===
--- libcxx/trunk/include/regex
+++ libcxx/trunk/include/regex
@@ -990,6 +990,10 @@
 
 #if defined(__mips__) && defined(__GLIBC__)
 static const char_class_type __regex_word = 
static_cast(_ISbit(15));
+#elif defined(__NetBSD__)
+// NetBSD defines classes up to 0x2000
+// see sys/ctype_bits.h, _CTYPE_Q
+static const char_class_type __regex_word = 0x8000;
 #else
 static const char_class_type __regex_word = 0x80;
 #endif


Index: libcxx/trunk/include/regex
===
--- libcxx/trunk/include/regex
+++ libcxx/trunk/include/regex
@@ -990,6 +990,10 @@
 
 #if defined(__mips__) && defined(__GLIBC__)
 static const char_class_type __regex_word = static_cast(_ISbit(15));
+#elif defined(__NetBSD__)
+// NetBSD defines classes up to 0x2000
+// see sys/ctype_bits.h, _CTYPE_Q
+static const char_class_type __regex_word = 0x8000;
 #else
 static const char_class_type __regex_word = 0x80;
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55576: [libcxx] [test] [support] Use socket()+bind() to create unix sockets portably

[Michał Górny via Phabricator via cfe-commits]

mgorny added a reviewer: emaste.
mgorny added a comment.

@emaste, maybe you could take a look at this too. I think it would solve the 
problem for FreeBSD as well.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55576



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

r349300 - Thread safety analysis: Allow scoped releasing of capabilities

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Sun Dec 16 06:15:30 2018
New Revision: 349300

URL: http://llvm.org/viewvc/llvm-project?rev=349300&view=rev
Log:
Thread safety analysis: Allow scoped releasing of capabilities

Summary:
The pattern is problematic with C++ exceptions, and not as widespread as
scoped locks, but it's still used by some, for example Chromium.

We are a bit stricter here at join points, patterns that are allowed for
scoped locks aren't allowed here. That could still be changed in the
future, but I'd argue we should only relax this if people ask for it.

Fixes PR36162.

Reviewers: aaron.ballman, delesley, pwnall

Reviewed By: delesley, pwnall

Subscribers: pwnall, cfe-commits

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

Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=349300&r1=349299&r2=349300&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Sun Dec 16 06:15:30 2018
@@ -42,6 +42,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ImmutableMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -890,28 +891,46 @@ public:
 
 class ScopedLockableFactEntry : public FactEntry {
 private:
-  SmallVector UnderlyingMutexes;
+  enum UnderlyingCapabilityKind {
+UCK_Acquired,  ///< Any kind of acquired capability.
+UCK_ReleasedShared,///< Shared capability that was released.
+UCK_ReleasedExclusive, ///< Exclusive capability that was released.
+  };
+
+  using UnderlyingCapability =
+  llvm::PointerIntPair;
+
+  SmallVector UnderlyingMutexes;
 
 public:
   ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc,
-  const CapExprSet &Excl, const CapExprSet &Shrd)
+  const CapExprSet &Excl, const CapExprSet &Shrd,
+  const CapExprSet &ExclRel, const CapExprSet &ShrdRel)
   : FactEntry(CE, LK_Exclusive, Loc, false) {
 for (const auto &M : Excl)
-  UnderlyingMutexes.push_back(M.sexpr());
+  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
 for (const auto &M : Shrd)
-  UnderlyingMutexes.push_back(M.sexpr());
+  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+for (const auto &M : ExclRel)
+  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+for (const auto &M : ShrdRel)
+  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
   }
 
   void
   handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
 SourceLocation JoinLoc, LockErrorKind LEK,
 ThreadSafetyHandler &Handler) const override {
-for (const auto *UnderlyingMutex : UnderlyingMutexes) {
-  if (FSet.findLock(FactMan, CapabilityExpr(UnderlyingMutex, false))) {
+for (const auto &UnderlyingMutex : UnderlyingMutexes) {
+  const auto *Entry = FSet.findLock(
+  FactMan, CapabilityExpr(UnderlyingMutex.getPointer(), false));
+  if ((UnderlyingMutex.getInt() == UCK_Acquired && Entry) ||
+  (UnderlyingMutex.getInt() != UCK_Acquired && !Entry)) {
 // If this scoped lock manages another mutex, and if the underlying
-// mutex is still held, then warn about the underlying mutex.
+// mutex is still/not held, then warn about the underlying mutex.
 Handler.handleMutexHeldEndOfScope(
-"mutex", sx::toString(UnderlyingMutex), loc(), JoinLoc, LEK);
+"mutex", sx::toString(UnderlyingMutex.getPointer()), loc(), 
JoinLoc,
+LEK);
   }
 }
   }
@@ -919,17 +938,14 @@ public:
   void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
   ThreadSafetyHandler &Handler,
   StringRef DiagKind) const override {
-for (const auto *UnderlyingMutex : UnderlyingMutexes) {
-  CapabilityExpr UnderCp(UnderlyingMutex, false);
+for (const auto &UnderlyingMutex : UnderlyingMutexes) {
+  CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
 
-  // We're relocking the underlying mutexes. Warn on double locking.
-  if (FSet.findLock(FactMan, UnderCp)) {
-Handler.handleDoubleLock(DiagKind, UnderCp.toString(), entry.loc());
-  } else {
-FSet.removeLock(FactMan, !UnderCp);
-FSet.addLock(FactMan, llvm::make_unique(
-  UnderCp, entry.kind(), entry.loc()));
-  }
+  if (UnderlyingMutex.getInt() == UCK_Acquired)
+lock(FSet, FactMan, UnderCp, entry.kind(), entry.loc(), &Handler,
+ DiagKind)

[PATCH] D52578: Thread safety analysis: Allow scoped releasing of capabilities

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349300: Thread safety analysis: Allow scoped releasing of 
capabilities (authored by aaronpuchert, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D52578

Files:
  cfe/trunk/lib/Analysis/ThreadSafety.cpp
  cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Index: cfe/trunk/lib/Analysis/ThreadSafety.cpp
===
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp
@@ -42,6 +42,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ImmutableMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
@@ -890,28 +891,46 @@
 
 class ScopedLockableFactEntry : public FactEntry {
 private:
-  SmallVector UnderlyingMutexes;
+  enum UnderlyingCapabilityKind {
+UCK_Acquired,  ///< Any kind of acquired capability.
+UCK_ReleasedShared,///< Shared capability that was released.
+UCK_ReleasedExclusive, ///< Exclusive capability that was released.
+  };
+
+  using UnderlyingCapability =
+  llvm::PointerIntPair;
+
+  SmallVector UnderlyingMutexes;
 
 public:
   ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc,
-  const CapExprSet &Excl, const CapExprSet &Shrd)
+  const CapExprSet &Excl, const CapExprSet &Shrd,
+  const CapExprSet &ExclRel, const CapExprSet &ShrdRel)
   : FactEntry(CE, LK_Exclusive, Loc, false) {
 for (const auto &M : Excl)
-  UnderlyingMutexes.push_back(M.sexpr());
+  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
 for (const auto &M : Shrd)
-  UnderlyingMutexes.push_back(M.sexpr());
+  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+for (const auto &M : ExclRel)
+  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+for (const auto &M : ShrdRel)
+  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
   }
 
   void
   handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
 SourceLocation JoinLoc, LockErrorKind LEK,
 ThreadSafetyHandler &Handler) const override {
-for (const auto *UnderlyingMutex : UnderlyingMutexes) {
-  if (FSet.findLock(FactMan, CapabilityExpr(UnderlyingMutex, false))) {
+for (const auto &UnderlyingMutex : UnderlyingMutexes) {
+  const auto *Entry = FSet.findLock(
+  FactMan, CapabilityExpr(UnderlyingMutex.getPointer(), false));
+  if ((UnderlyingMutex.getInt() == UCK_Acquired && Entry) ||
+  (UnderlyingMutex.getInt() != UCK_Acquired && !Entry)) {
 // If this scoped lock manages another mutex, and if the underlying
-// mutex is still held, then warn about the underlying mutex.
+// mutex is still/not held, then warn about the underlying mutex.
 Handler.handleMutexHeldEndOfScope(
-"mutex", sx::toString(UnderlyingMutex), loc(), JoinLoc, LEK);
+"mutex", sx::toString(UnderlyingMutex.getPointer()), loc(), JoinLoc,
+LEK);
   }
 }
   }
@@ -919,17 +938,14 @@
   void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
   ThreadSafetyHandler &Handler,
   StringRef DiagKind) const override {
-for (const auto *UnderlyingMutex : UnderlyingMutexes) {
-  CapabilityExpr UnderCp(UnderlyingMutex, false);
+for (const auto &UnderlyingMutex : UnderlyingMutexes) {
+  CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
 
-  // We're relocking the underlying mutexes. Warn on double locking.
-  if (FSet.findLock(FactMan, UnderCp)) {
-Handler.handleDoubleLock(DiagKind, UnderCp.toString(), entry.loc());
-  } else {
-FSet.removeLock(FactMan, !UnderCp);
-FSet.addLock(FactMan, llvm::make_unique(
-  UnderCp, entry.kind(), entry.loc()));
-  }
+  if (UnderlyingMutex.getInt() == UCK_Acquired)
+lock(FSet, FactMan, UnderCp, entry.kind(), entry.loc(), &Handler,
+ DiagKind);
+  else
+unlock(FSet, FactMan, UnderCp, entry.loc(), &Handler, DiagKind);
 }
   }
 
@@ -938,32 +954,49 @@
 bool FullyRemove, ThreadSafetyHandler &Handler,
 StringRef DiagKind) const override {
 assert(!Cp.negative() && "Managing object cannot be negative.");
-for (const auto *UnderlyingMutex : UnderlyingMutexes) {
-  CapabilityExpr UnderCp(UnderlyingMutex, false);
-  auto UnderEntry = llvm::make_unique(
-  !UnderCp, LK_Exclusive, UnlockLoc);
-
-  if (FullyRemove) {
-// We'

[PATCH] D55576: [libcxx] [test] [support] Use socket()+bind() to create unix sockets portably

[Dimitry Andric via Phabricator via cfe-commits]

dim accepted this revision.
dim added a comment.

LGTM.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55576



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

[PATCH] D55576: [libcxx] [test] [support] Use socket()+bind() to create unix sockets portably

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX349305: [test] [support] Use socket()+bind() to create 
unix sockets portably (authored by mgorny, committed by ).

Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55576

Files:
  test/support/filesystem_dynamic_test_helper.py


Index: test/support/filesystem_dynamic_test_helper.py
===
--- test/support/filesystem_dynamic_test_helper.py
+++ test/support/filesystem_dynamic_test_helper.py
@@ -1,5 +1,6 @@
 import sys
 import os
+import socket
 import stat
 
 # Ensure that this is being run on a specific platform
@@ -76,8 +77,13 @@
 
 
 def create_socket(source):
-mode = 0o600 | stat.S_IFSOCK
-os.mknod(sanitize(source), mode)
+sock = socket.socket(socket.AF_UNIX)
+sanitized_source = sanitize(source)
+# AF_UNIX sockets may have very limited path length, so split it
+# into chdir call (with technically unlimited length) followed
+# by bind() relative to the directory
+os.chdir(os.path.dirname(sanitized_source))
+sock.bind(os.path.basename(sanitized_source))
 
 
 if __name__ == '__main__':


Index: test/support/filesystem_dynamic_test_helper.py
===
--- test/support/filesystem_dynamic_test_helper.py
+++ test/support/filesystem_dynamic_test_helper.py
@@ -1,5 +1,6 @@
 import sys
 import os
+import socket
 import stat
 
 # Ensure that this is being run on a specific platform
@@ -76,8 +77,13 @@
 
 
 def create_socket(source):
-mode = 0o600 | stat.S_IFSOCK
-os.mknod(sanitize(source), mode)
+sock = socket.socket(socket.AF_UNIX)
+sanitized_source = sanitize(source)
+# AF_UNIX sockets may have very limited path length, so split it
+# into chdir call (with technically unlimited length) followed
+# by bind() relative to the directory
+os.chdir(os.path.dirname(sanitized_source))
+sock.bind(os.path.basename(sanitized_source))
 
 
 if __name__ == '__main__':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55576: [libcxx] [test] [support] Use socket()+bind() to create unix sockets portably

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

@dim, thanks for the review. Should I also try removing the following 
restriction?

  #if !defined(__APPLE__) && !defined(__FreeBSD__) // No support for domain 
sockets
  {env.create_socket("socket"), file_type::socket},
  #endif


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55576



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

[PATCH] D55576: [libcxx] [test] [support] Use socket()+bind() to create unix sockets portably

[Dimitry Andric via Phabricator via cfe-commits]

dim added a comment.

In D55576#1332370 , @mgorny wrote:

> @dim, thanks for the review. Should I also try removing the following 
> restriction?
>
>   #if !defined(__APPLE__) && !defined(__FreeBSD__) // No support for domain 
> sockets
>   {env.create_socket("socket"), file_type::socket},
>   #endif


Well, `test/support/filesystem_test_helper.hpp` also has this comment:

// OS X and FreeBSD doesn't support socket files so we shouldn't even
// allow tests to call this unguarded.
  #if !defined(__FreeBSD__) && !defined(__APPLE__)
  std::string create_socket(std::string file) {
  file = sanitize_path(std::move(file));
  fs_helper_run(fs_make_cmd("create_socket", file));
  return file;
  }
  #endif

but it looks like you now fixed it with this change to 
`filesystem_dynamic_test_helper.py`.  So I guess after this, it should be safe 
to remove the FreeBSD specific exception.  I don't know about macOS, though.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55576



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

r349308 - Thread safety analysis: Avoid intermediate copies [NFC]

[Aaron Puchert via cfe-commits]

Author: aaronpuchert
Date: Sun Dec 16 08:19:11 2018
New Revision: 349308

URL: http://llvm.org/viewvc/llvm-project?rev=349308&view=rev
Log:
Thread safety analysis: Avoid intermediate copies [NFC]

The main reason is to reduce the number of constructor arguments though,
especially since many of them had the same type.

Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=349308&r1=349307&r2=349308&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Sun Dec 16 08:19:11 2018
@@ -903,18 +903,23 @@ private:
   SmallVector UnderlyingMutexes;
 
 public:
-  ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc,
-  const CapExprSet &Excl, const CapExprSet &Shrd,
-  const CapExprSet &ExclRel, const CapExprSet &ShrdRel)
-  : FactEntry(CE, LK_Exclusive, Loc, false) {
-for (const auto &M : Excl)
-  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
-for (const auto &M : Shrd)
-  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
-for (const auto &M : ExclRel)
-  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
-for (const auto &M : ShrdRel)
-  UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
+  ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
+  : FactEntry(CE, LK_Exclusive, Loc, false) {}
+
+  void addExclusiveLock(const CapabilityExpr &M) {
+UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+  }
+
+  void addSharedLock(const CapabilityExpr &M) {
+UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+  }
+
+  void addExclusiveUnlock(const CapabilityExpr &M) {
+UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+  }
+
+  void addSharedUnlock(const CapabilityExpr &M) {
+UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
   }
 
   void
@@ -1938,15 +1943,20 @@ void BuildLockset::handleCall(const Expr
 // FIXME: does this store a pointer to DRE?
 CapabilityExpr Scp = Analyzer->SxBuilder.translateAttrExpr(&DRE, nullptr);
 
-std::copy(ScopedExclusiveReqs.begin(), ScopedExclusiveReqs.end(),
-  std::back_inserter(ExclusiveLocksToAdd));
-std::copy(ScopedSharedReqs.begin(), ScopedSharedReqs.end(),
-  std::back_inserter(SharedLocksToAdd));
-Analyzer->addLock(FSet,
-  llvm::make_unique(
-  Scp, MLoc, ExclusiveLocksToAdd, SharedLocksToAdd,
-  ExclusiveLocksToRemove, SharedLocksToRemove),
-  CapDiagKind);
+auto ScopedEntry = llvm::make_unique(Scp, MLoc);
+for (const auto &M : ExclusiveLocksToAdd)
+  ScopedEntry->addExclusiveLock(M);
+for (const auto &M : ScopedExclusiveReqs)
+  ScopedEntry->addExclusiveLock(M);
+for (const auto &M : SharedLocksToAdd)
+  ScopedEntry->addSharedLock(M);
+for (const auto &M : ScopedSharedReqs)
+  ScopedEntry->addSharedLock(M);
+for (const auto &M : ExclusiveLocksToRemove)
+  ScopedEntry->addExclusiveUnlock(M);
+for (const auto &M : SharedLocksToRemove)
+  ScopedEntry->addSharedUnlock(M);
+Analyzer->addLock(FSet, std::move(ScopedEntry), CapDiagKind);
   }
 }
 


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

[PATCH] D55730: [analyzer] MoveChecker Pt.9: Add a "peaceful" mode in which it finds only 100% authentic bugs.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:188-195
+  void setAggressiveness(StringRef Str) {
+Aggressiveness =
+llvm::StringSwitch(Str)
+.Case("KnownsOnly", AK_KnownsOnly)
+.Case("KnownsAndLocals", AK_KnownsAndLocals)
+.Case("All", AK_All)
+.Default(AK_KnownsAndLocals); // A sane default.

NoQ wrote:
> Szelethus wrote:
> > You can acquire a `DiagnosticsEngine` through `ASTContext`, which can be 
> > acquired from `CheckerManager`. I think it'd prefer to see an error if I 
> > messed up the input.
> Hmm. I should probably also honor the compatibility mode then.
And think if i want to bail out entirely.

Maybe next time? Let's figure out how exactly do we want these errors to look 
and behave before introducing restrictions.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55730



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

[PATCH] D55730: [analyzer] MoveChecker Pt.9: Add a "peaceful" mode in which it finds only 100% authentic bugs.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MoveChecker.cpp:188-195
+  void setAggressiveness(StringRef Str) {
+Aggressiveness =
+llvm::StringSwitch(Str)
+.Case("KnownsOnly", AK_KnownsOnly)
+.Case("KnownsAndLocals", AK_KnownsAndLocals)
+.Case("All", AK_All)
+.Default(AK_KnownsAndLocals); // A sane default.

NoQ wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > You can acquire a `DiagnosticsEngine` through `ASTContext`, which can be 
> > > acquired from `CheckerManager`. I think it'd prefer to see an error if I 
> > > messed up the input.
> > Hmm. I should probably also honor the compatibility mode then.
> And think if i want to bail out entirely.
> 
> Maybe next time? Let's figure out how exactly do we want these errors to look 
> and behave before introducing restrictions.
Hmmm, maybe adding an error handling method to `CheckerManager` would be 
elegant enough. I'll leave it up to you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55730



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

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp:136
   Agg f8 = {EnumVal};  // OK
+  // expected-warning@+1 {{implicit conversion from 'int' to 'float' changes 
value from 123456789 to 1.2345679E+8}}
   Agg f9 = {123456789};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}

aaron.ballman wrote:
> I don't think we want the warning triggered in either of these cases -- they 
> already have an error diagnostic on the same line for the same issue.
Any recommended way how it should be handled?


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

https://reviews.llvm.org/D52835



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

[PATCH] D55749: [Driver] Automatically enable -munwind-tables if -fseh-exceptions is enabled

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, mgrang, ssijaric.
Herald added subscribers: JDevlieghere, kristof.beyls, javed.absar, aprantl.

For targets where SEH exceptions are used by default (on MinGW, only x86_64 so 
far), -munwind-tables are added automatically. If
-fseh-exeptions is enabled on a target where SEH exeptions are availble but not 
enabled by default yet (aarch64), we need to pass -munwind-tables if 
-fseh-exceptions was specified.

Once things have settled and this works fine in practice, the default can be 
switched from dwarf to SEH. Or is this unnecessary churn and we should just 
keep off this until the default can be changed?


Repository:
  rC Clang

https://reviews.llvm.org/D55749

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/windows-exceptions.cpp


Index: test/Driver/windows-exceptions.cpp
===
--- test/Driver/windows-exceptions.cpp
+++ test/Driver/windows-exceptions.cpp
@@ -2,8 +2,11 @@
 // RUN: %clang -target x86_64-windows-msvc -c %s -### 2>&1 | FileCheck 
-check-prefix=MSVC %s
 // RUN: %clang -target i686-windows-gnu -c %s -### 2>&1 | FileCheck 
-check-prefix=MINGW-DWARF %s
 // RUN: %clang -target x86_64-windows-gnu -c %s -### 2>&1 | FileCheck 
-check-prefix=MINGW-SEH %s
+// RUN: %clang -target aarch64-windows-gnu -c %s -### 2>&1 | FileCheck 
-check-prefix=MINGW-DWARF %s
+// RUN: %clang -target aarch64-windows-gnu -fseh-exceptions -c %s -### 2>&1 | 
FileCheck -check-prefix=MINGW-SEH %s
 
 MSVC-NOT: -fdwarf-exceptions
 MSVC-NOT: -fseh-exceptions
 MINGW-DWARF: -fdwarf-exceptions
+MINGW-SEH: -munwind-tables
 MINGW-SEH: -fseh-exceptions
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3917,15 +3917,20 @@
   if (Freestanding)
 CmdArgs.push_back("-ffreestanding");
 
+  Arg *ExceptionArg = Args.getLastArg(options::OPT_fsjlj_exceptions,
+  options::OPT_fseh_exceptions,
+  options::OPT_fdwarf_exceptions);
   // This is a coarse approximation of what llvm-gcc actually does, both
   // -fasynchronous-unwind-tables and -fnon-call-exceptions interact in more
   // complicated ways.
-  bool AsynchronousUnwindTables =
-  Args.hasFlag(options::OPT_fasynchronous_unwind_tables,
-   options::OPT_fno_asynchronous_unwind_tables,
-   (TC.IsUnwindTablesDefault(Args) ||
-TC.getSanitizerArgs().needsUnwindTables()) &&
-   !Freestanding);
+  bool AsynchronousUnwindTables = Args.hasFlag(
+  options::OPT_fasynchronous_unwind_tables,
+  options::OPT_fno_asynchronous_unwind_tables,
+  (TC.IsUnwindTablesDefault(Args) ||
+   (ExceptionArg &&
+ExceptionArg->getOption().matches(options::OPT_fseh_exceptions)) ||
+   TC.getSanitizerArgs().needsUnwindTables()) &&
+  !Freestanding);
   if (Args.hasFlag(options::OPT_funwind_tables, options::OPT_fno_unwind_tables,
AsynchronousUnwindTables))
 CmdArgs.push_back("-munwind-tables");
@@ -4704,11 +4709,8 @@
 addExceptionArgs(Args, InputType, TC, KernelOrKext, Runtime, CmdArgs);
 
   // Handle exception personalities
-  Arg *A = Args.getLastArg(options::OPT_fsjlj_exceptions,
-   options::OPT_fseh_exceptions,
-   options::OPT_fdwarf_exceptions);
-  if (A) {
-const Option &Opt = A->getOption();
+  if (ExceptionArg) {
+const Option &Opt = ExceptionArg->getOption();
 if (Opt.matches(options::OPT_fsjlj_exceptions))
   CmdArgs.push_back("-fsjlj-exceptions");
 if (Opt.matches(options::OPT_fseh_exceptions))


Index: test/Driver/windows-exceptions.cpp
===
--- test/Driver/windows-exceptions.cpp
+++ test/Driver/windows-exceptions.cpp
@@ -2,8 +2,11 @@
 // RUN: %clang -target x86_64-windows-msvc -c %s -### 2>&1 | FileCheck -check-prefix=MSVC %s
 // RUN: %clang -target i686-windows-gnu -c %s -### 2>&1 | FileCheck -check-prefix=MINGW-DWARF %s
 // RUN: %clang -target x86_64-windows-gnu -c %s -### 2>&1 | FileCheck -check-prefix=MINGW-SEH %s
+// RUN: %clang -target aarch64-windows-gnu -c %s -### 2>&1 | FileCheck -check-prefix=MINGW-DWARF %s
+// RUN: %clang -target aarch64-windows-gnu -fseh-exceptions -c %s -### 2>&1 | FileCheck -check-prefix=MINGW-SEH %s
 
 MSVC-NOT: -fdwarf-exceptions
 MSVC-NOT: -fseh-exceptions
 MINGW-DWARF: -fdwarf-exceptions
+MINGW-SEH: -munwind-tables
 MINGW-SEH: -fseh-exceptions
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3917,15 +3917,20 @@
   if (Freestanding)
 CmdArgs.push_back("-ffreestanding");
 
+  Arg *ExceptionArg = Args.getLastArg(options::OPT_fsjlj_except

[PATCH] D55566: [analyzer] LiveVariables: Fix a zombie expression problem, add testing infrastructure.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Tested it a little bit. It causes an extremely slight skew in warnings. I'll 
reduce a few of them to see if this is mostly just budgets reached earlier or 
later, or these are sensible improvements/regressions, but generally this looks 
pretty safe, so i'll try to commit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55566



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

[PATCH] D55566: [analyzer] LiveVariables: Fix a zombie expression problem, add testing infrastructure.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

@Szelethus: i'm very happy that someone's reading this :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55566



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

[PATCH] D55566: [analyzer] LiveVariables: Fix a zombie expression problem, add testing infrastructure.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL349320: [analyzer] Fix some expressions staying live too 
long. Add a debug checker. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55566?vs=177753&id=178413#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55566

Files:
  cfe/trunk/docs/analyzer/DebugChecks.rst
  cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
  cfe/trunk/lib/Analysis/LiveVariables.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
  cfe/trunk/test/Analysis/live-stmts.cpp
  cfe/trunk/test/Analysis/use-after-move.cpp

Index: cfe/trunk/docs/analyzer/DebugChecks.rst
===
--- cfe/trunk/docs/analyzer/DebugChecks.rst
+++ cfe/trunk/docs/analyzer/DebugChecks.rst
@@ -30,9 +30,12 @@
 - debug.DumpLiveVars: Show the results of live variable analysis for each
   top-level function being analyzed.
 
+- debug.DumpLiveStmts: Show the results of live statement analysis for each
+  top-level function being analyzed.
+
 - debug.ViewExplodedGraph: Show the Exploded Graphs generated for the
   analysis of different functions in the input translation unit. When there
-  are several functions analyzed, display one graph per function. Beware 
+  are several functions analyzed, display one graph per function. Beware
   that these graphs may grow very large, even for small functions.
 
 Path Tracking
Index: cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
===
--- cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
+++ cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
@@ -88,9 +88,13 @@
   ///  before the given block-level expression (see runOnAllBlocks).
   bool isLive(const Stmt *Loc, const Stmt *StmtVal);
 
-  /// Print to stderr the liveness information associated with
+  /// Print to stderr the variable liveness information associated with
   /// each basic block.
-  void dumpBlockLiveness(const SourceManager& M);
+  void dumpBlockLiveness(const SourceManager &M);
+
+  /// Print to stderr the statement liveness information associated with
+  /// each basic block.
+  void dumpStmtLiveness(const SourceManager &M);
 
   void runOnAllBlocks(Observer &obs);
 
Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -707,6 +707,9 @@
 def LiveVariablesDumper : Checker<"DumpLiveVars">,
   HelpText<"Print results of live variable analysis">;
 
+def LiveStatementsDumper : Checker<"DumpLiveStmts">,
+  HelpText<"Print results of live statement analysis">;
+
 def CFGViewer : Checker<"ViewCFG">,
   HelpText<"View Control-Flow Graphs using GraphViz">;
 
Index: cfe/trunk/test/Analysis/use-after-move.cpp
===
--- cfe/trunk/test/Analysis/use-after-move.cpp
+++ cfe/trunk/test/Analysis/use-after-move.cpp
@@ -778,6 +778,45 @@
   }
 }
 
+void checkMoreLoopZombies1(bool flag) {
+  while (flag) {
+Empty e{};
+if (true)
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
+bool coin();
+
+void checkMoreLoopZombies2(bool flag) {
+  while (flag) {
+Empty e{};
+while (coin())
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
+void checkMoreLoopZombies3(bool flag) {
+  while (flag) {
+Empty e{};
+do
+  e; // expected-warning {{expression result unused}}
+while (coin());
+Empty f = std::move(e); // no-warning
+  }
+}
+
+void checkMoreLoopZombies4(bool flag) {
+  while (flag) {
+Empty e{};
+for (; coin();)
+  e; // expected-warning {{expression result unused}}
+Empty f = std::move(e); // no-warning
+  }
+}
+
 struct MoveOnlyWithDestructor {
   MoveOnlyWithDestructor();
   ~MoveOnlyWithDestructor();
Index: cfe/trunk/test/Analysis/live-stmts.cpp
===
--- cfe/trunk/test/Analysis/live-stmts.cpp
+++ cfe/trunk/test/Analysis/live-stmts.cpp
@@ -0,0 +1,167 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveStmts %s 2>&1\
+// RUN:   | FileCheck %s
+
+int coin();
+
+
+int testThatDumperWorks(int x, int y, int z) {
+  return x ? y : z;
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live stateme

r349320 - [analyzer] Fix some expressions staying live too long. Add a debug checker.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Sun Dec 16 15:44:06 2018
New Revision: 349320

URL: http://llvm.org/viewvc/llvm-project?rev=349320&view=rev
Log:
[analyzer] Fix some expressions staying live too long. Add a debug checker.

StaticAnalyzer uses the CFG-based RelaxedLiveVariables analysis in order to,
in particular, figure out values of which expressions are still needed.
When the expression becomes "dead", it is garbage-collected during
the dead binding scan.

Expressions that constitute branches/bodies of control flow statements,
eg. `E1' in `if (C1) E1;' but not `E2' in `if (C2) { E2; }', were kept alive
for too long. This caused false positives in MoveChecker because it relies
on cleaning up loop-local variables when they go out of scope, but some of those
live-for-too-long expressions were keeping a reference to those variables.

Fix liveness analysis to correctly mark these expressions as dead.

Add a debug checker, debug.DumpLiveStmts, in order to test expressions liveness.

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

Added:
cfe/trunk/test/Analysis/live-stmts.cpp
Modified:
cfe/trunk/docs/analyzer/DebugChecks.rst
cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/Analysis/LiveVariables.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
cfe/trunk/test/Analysis/use-after-move.cpp

Modified: cfe/trunk/docs/analyzer/DebugChecks.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/DebugChecks.rst?rev=349320&r1=349319&r2=349320&view=diff
==
--- cfe/trunk/docs/analyzer/DebugChecks.rst (original)
+++ cfe/trunk/docs/analyzer/DebugChecks.rst Sun Dec 16 15:44:06 2018
@@ -30,9 +30,12 @@ using a 'dot' format viewer (such as Gra
 - debug.DumpLiveVars: Show the results of live variable analysis for each
   top-level function being analyzed.
 
+- debug.DumpLiveStmts: Show the results of live statement analysis for each
+  top-level function being analyzed.
+
 - debug.ViewExplodedGraph: Show the Exploded Graphs generated for the
   analysis of different functions in the input translation unit. When there
-  are several functions analyzed, display one graph per function. Beware 
+  are several functions analyzed, display one graph per function. Beware
   that these graphs may grow very large, even for small functions.
 
 Path Tracking

Modified: cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h?rev=349320&r1=349319&r2=349320&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h Sun Dec 16 
15:44:06 2018
@@ -88,9 +88,13 @@ public:
   ///  before the given block-level expression (see runOnAllBlocks).
   bool isLive(const Stmt *Loc, const Stmt *StmtVal);
 
-  /// Print to stderr the liveness information associated with
+  /// Print to stderr the variable liveness information associated with
   /// each basic block.
-  void dumpBlockLiveness(const SourceManager& M);
+  void dumpBlockLiveness(const SourceManager &M);
+
+  /// Print to stderr the statement liveness information associated with
+  /// each basic block.
+  void dumpStmtLiveness(const SourceManager &M);
 
   void runOnAllBlocks(Observer &obs);
 

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=349320&r1=349319&r2=349320&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Sun Dec 16 
15:44:06 2018
@@ -707,6 +707,9 @@ def DominatorsTreeDumper : Checker<"Dump
 def LiveVariablesDumper : Checker<"DumpLiveVars">,
   HelpText<"Print results of live variable analysis">;
 
+def LiveStatementsDumper : Checker<"DumpLiveStmts">,
+  HelpText<"Print results of live statement analysis">;
+
 def CFGViewer : Checker<"ViewCFG">,
   HelpText<"View Control-Flow Graphs using GraphViz">;
 

Modified: cfe/trunk/lib/Analysis/LiveVariables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/LiveVariables.cpp?rev=349320&r1=349319&r2=349320&view=diff
==
--- cfe/trunk/lib/Analysis/LiveVariables.cpp (original)
+++ cfe/trunk/lib/Analysis/LiveVariables.cpp Sun Dec 16 15:44:06 2018
@@ -93,6 +93,7 @@ public:
  LiveVariables::Observer *obs = nullptr);
 
   void dumpBlockLiveness(const SourceManager& M);
+  void dumpStmtLiveness(const SourceManager& M);
 
   LiveVariablesImpl(AnalysisDeclContext &ac, bool KillAtAss

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor, thank you a lot! 
I have to say that even having your patch, I still don't understand why the 
patch fixes the problem (and what the problem is). As I see, the patch handles 
the cases where some fields or friends have DeclContext different from ToRD. 
Could you please provide examples so I can add them to the test suite or, at 
least, just investigate?


Repository:
  rC Clang

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

https://reviews.llvm.org/D44100



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

[PATCH] D54918: [analyzer] Apply clang-format to GenericTaintChecker.cpp

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

I appreciate the cleanup in general and any work on taint analysis in 
particular, thanks!

In D54918#1332089 , @Szelethus wrote:

> I vaguely remember @george.karpenkov having many great points against it -- 
> please don't commit until he can take a look :)


Essentially, it messes with git blame, and also causes a lot of merge conflicts 
for downstream developers (we have a few of those). But if you plan to work on 
this checker actively anyway, please feel free to start with formatting.

Here's the relevant quote from the guidelines 
:

> Our long term goal is for the entire codebase to follow the convention, but 
> we explicitly *do not* want patches that do large-scale reformatting of 
> existing code. On the other hand, it is reasonable to rename the methods of a 
> class if you’re about to change it in some other way. Just do the 
> reformatting as a separate commit from the functionality change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54918



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

[PATCH] D55734: [analyzer] Revise GenericTaintChecker's internal representation

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hi, thanks again for looking into this! I have a single piece of doubt that 
this doesn't change the behavior (the comment about `pread`), but other than 
that, this diff is good to go, in my opinion.




Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:212
+  llvm::StringSwitch(Name)
+  .Case("atoi", TaintPropagationRule({0}, {ReturnValueIndex}))
+  .Case("atol", TaintPropagationRule({0}, {ReturnValueIndex}))

`llvm::ArrayRef` has a fancy feature: it can be constructed not only from an 
initializer list, but also from a single element. This could have allowed 
skipping `{}` in these constructors when there is just one value in the list. 
That's what i would have done, but i guess it's up to you to decide which 
approach is prettier.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:224
+  .Case("read", TaintPropagationRule({0, 2}, {1, ReturnValueIndex}))
+  .Case("pread", TaintPropagationRule({0, 2, 3}, {1, 
ReturnValueIndex}))
+  .Case("gets", TaintPropagationRule({}, {0, ReturnValueIndex}))

Now `pread` doesn't produce a tainted result when only the buffer is tainted 
but other arguments are not. Is it a functional change? I guess we should be 
able to add tests for it. You can also commit this patch with `{0, 1, 2, 3}` 
and remove `1` in the next patch that also adds a test.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:466
   bool IsTainted = false;
-  for (ArgVector::const_iterator I = SrcArgs.begin(),
- E = SrcArgs.end(); I != E; ++I) {
+  for (ArgVector::const_iterator I = SrcArgs.begin(), E = SrcArgs.end(); I != 
E;
+   ++I) {

`auto` here as well?



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:469
 unsigned ArgNum = *I;
-
-if (ArgNum == InvalidArgIndex) {
-  // Check if any of the arguments is tainted, but skip the
-  // destination arguments.
-  for (unsigned int i = 0; i < CE->getNumArgs(); ++i) {
-if (isDestinationArgument(i))
-  continue;
-if ((IsTainted = isTaintedOrPointsToTainted(CE->getArg(i), State, C)))
-  break;
-  }
-  break;
-}
-
 if (CE->getNumArgs() < (ArgNum + 1))
   return State;

The more natural way to write this is `ArgNum >= CE->getNumArgs()`, as opposed 
to `ArgNum < CE->getNumArgs()` which is the usual boundary when dealing with 
arrays that start at 0.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:488
   // Mark the arguments which should be tainted after the function returns.
-  for (ArgVector::const_iterator I = DstArgs.begin(),
- E = DstArgs.end(); I != E; ++I) {
+  for (ArgVector::const_iterator I = DstArgs.begin(), E = DstArgs.end(); I != 
E;
+   ++I) {

`auto` here as well?


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

https://reviews.llvm.org/D55734



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

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Mm, yeah, but it's still weird to see it in `checkPreCall` for 
`CXXDestructorCall`.

I suspect that your new warning covers these tests as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54834



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

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ugh. MSVC buildbot 

 is acting weird again. Also it is not acting deterministically 
.

I was also yelled at by it in D55388 .

Which makes me suspect that it has something to do with checker registration, 
as i change checker option related behavior in my patch, and the crashes in 
`MallocChecker` deal with loading checker name.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54834



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

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Wait, no, i didn't do anything with checker options yet. Something else then, i 
guess.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54834



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

RE: r349281 - [analyzer][MallocChecker][NFC] Document and reorganize some functions

[via cfe-commits]

Hi Kristof,

Your change seems to have caused 7 new test failures on the PS4 Windows bot:

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/22257/

Clang :: Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp
Clang :: Analysis/MismatchedDeallocator-path-notes.cpp
Clang :: Analysis/NewDelete-intersections.mm
Clang :: Analysis/free.c
Clang :: Analysis/inner-pointer.cpp
Clang :: Analysis/malloc.c
Clang :: Analysis/malloc.mm

Can you please take a look?

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Kristof 
Umann via cfe-commits
Sent: Saturday, December 15, 2018 10:34
To: cfe-commits@lists.llvm.org
Subject: r349281 - [analyzer][MallocChecker][NFC] Document and reorganize some 
functions

Author: szelethus
Date: Sat Dec 15 10:34:00 2018
New Revision: 349281

URL: http://llvm.org/viewvc/llvm-project?rev=349281&view=rev
Log:
[analyzer][MallocChecker][NFC] Document and reorganize some functions

This patch merely reorganizes some things, and features no functional change.

In detail:

* Provided documentation, or moved existing documentation in more obvious
places.
* Added dividers. (the //===--===// thing).
* Moved getAllocationFamily, printAllocDeallocName, printExpectedAllocName and
printExpectedDeallocName in the global namespace on top of the file where
AllocationFamily is declared, as they are very strongly related.
* Moved isReleased and MallocUpdateRefState near RefState's definition for the
same reason.
* Realloc modeling was very poor in terms of variable and structure naming, as
well as documentation, so I renamed some of them and added much needed docs.
* Moved function IdentifierInfos to a separate struct, and moved isMemFunction,
isCMemFunction adn isStandardNewDelete inside it. This makes the patch affect
quite a lot of lines, should I extract it to a separate one?
* Moved MallocBugVisitor out of MallocChecker.
* Preferred switches to long else-if branches in some places.
* Neatly organized some RUN: lines.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/malloc-annotations.c
cfe/trunk/test/Analysis/malloc.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=349281&r1=349280&r2=349281&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Sat Dec 15 10:34:00 
2018
@@ -7,8 +7,41 @@
 //
 
//===--===//
 //
-// This file defines malloc/free checker, which checks for potential memory
-// leaks, double free, and use-after-free problems.
+// This file defines a variety of memory management related checkers, such as
+// leak, double free, and use-after-free.
+//
+// The following checkers are defined here:
+//
+//   * MallocChecker
+//   Despite its name, it models all sorts of memory allocations and
+//   de- or reallocation, including but not limited to malloc, free,
+//   relloc, new, delete. It also reports on a variety of memory misuse
+//   errors.
+//   Many other checkers interact very closely with this checker, in fact,
+//   most are merely options to this one. Other checkers may register
+//   MallocChecker, but do not enable MallocChecker's reports (more details
+//   to follow around its field, ChecksEnabled).
+//   It also has a boolean "Optimistic" checker option, which if set to 
true
+//   will cause the checker to model user defined memory management related
+//   functions annotated via the attribute ownership_takes, ownership_holds
+//   and ownership_returns.
+//
+//   * NewDeleteChecker
+//   Enables the modeling of new, new[], delete, delete[] in MallocChecker,
+//   and checks for related double-free and use-after-free errors.
+//
+//   * NewDeleteLeaksChecker
+//   Checks for leaks related to new, new[], delete, delete[].
+//   Depends on NewDeleteChecker.
+//
+//   * MismatchedDeallocatorChecker
+//   Enables checking whether memory is deallocated with the correspending
+//   allocation function in MallocChecker, such as malloc() allocated
+//   regions are only freed by free(), new by delete, new[] by delete[].
+//
+//  InnerPointerChecker interacts very closely with MallocChecker, but unlike
+//  the above checkers, it has it's own file, hence the many 
InnerPointerChecker
+//  related headers and non-static functions.
 //
 
//===--===//
 
@@ -37,6 +70,10 @@
 using namespace clang;
 using namespace ento;
 
+//===--===//
+// The types of allocat

[PATCH] D55388: [analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349326: Speculatively re-apply "[analyzer] MoveChecker: 
Add checks for dereferencing..." (authored by dergachev, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55388?vs=178328&id=178422#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55388

Files:
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -1,20 +1,26 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=unexplored_first_queue
+// RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
-// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_warnIfReached();
+
 class B {
 public:
   B() = default;
@@ -849,7 +855,19 @@
 // expected-note@-4{{Object 'P' is moved}}
 // expected-note@-4{{Method called on moved-from object 'P'}}
 #endif
-*P += 1; // FIXME: Should warn that the pointer is null.
+
+// Because that well-defined state is null, dereference is still UB.
+// Note that in aggressive mode we already warned about 'P',
+// so no extra warning is generated.
+*P += 1;
+#ifndef AGGRESSIVE
+// expected-warning@-2{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+// expected-note@-14{{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+// expected-note@-4{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+#endif
+
+// The program should have crashed by now.
+clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
@@ -866,3 +884,9 @@
   P.get(); // expected-warning{{Method called on moved-from object 'P'}}
// expected-note@-1{{Method called on moved-from object 'P'}}
 }
+
+void localUniquePtrWithArrow(std::unique_ptr P) {
+  std::unique_ptr Q = std::move(P); // expected-note{{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+  P->foo(); // expected-warning{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+// expected-note@-1{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+}
Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -61,19 +61,35 @@
   const char *NL, const char *Sep) const override;
 
 private:
-  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move };
+  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move, MK_Dereference };
+  enum StdObjectKind : unsigned { SK_NonStd, SK_Unsafe, SK_Safe, SK_SmartPtr };
+
+  static bool misuseCausesCrash(MisuseKind MK) {
+return MK == MK_Dereference;
+  }
 
   struct ObjectKind {
-bool Local : 1; // Is this a local variable or a local rvalue reference?
-bool STL : 1; // Is this an object of a standard type?
+// Is this a local variable or a local rvalue reference?
+bool IsLocal : 1;
+// Is this an STL object? If so, of what kind?
+StdObjectKind StdKind : 2;
+  };
+
+  // STL smart pointers are automatically re-initialized to null when moved
+  // from. So we can't warn on many methods, but we can warn when it is
+  // dereferenced, which is UB even if the resulting lvalue never gets read.
+  const llvm::StringSet<>

r349326 - Speculatively re-apply "[analyzer] MoveChecker: Add checks for dereferencing..."

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Sun Dec 16 21:25:23 2018
New Revision: 349326

URL: http://llvm.org/viewvc/llvm-project?rev=349326&view=rev
Log:
Speculatively re-apply "[analyzer] MoveChecker: Add checks for dereferencing..."

This re-applies commit r349226 that was reverted in r349233 due to failures
on clang-x64-windows-msvc.

Specify enum type as unsigned for use in bit field. Otherwise overflows
may cause UB.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
cfe/trunk/test/Analysis/use-after-move.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp?rev=349326&r1=349325&r2=349326&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp Sun Dec 16 21:25:23 
2018
@@ -61,19 +61,35 @@ public:
   const char *NL, const char *Sep) const override;
 
 private:
-  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move };
+  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move, MK_Dereference };
+  enum StdObjectKind : unsigned { SK_NonStd, SK_Unsafe, SK_Safe, SK_SmartPtr };
+
+  static bool misuseCausesCrash(MisuseKind MK) {
+return MK == MK_Dereference;
+  }
 
   struct ObjectKind {
-bool Local : 1; // Is this a local variable or a local rvalue reference?
-bool STL : 1; // Is this an object of a standard type?
+// Is this a local variable or a local rvalue reference?
+bool IsLocal : 1;
+// Is this an STL object? If so, of what kind?
+StdObjectKind StdKind : 2;
+  };
+
+  // STL smart pointers are automatically re-initialized to null when moved
+  // from. So we can't warn on many methods, but we can warn when it is
+  // dereferenced, which is UB even if the resulting lvalue never gets read.
+  const llvm::StringSet<> StdSmartPtrClasses = {
+  "shared_ptr",
+  "unique_ptr",
+  "weak_ptr",
   };
 
   // Not all of these are entirely move-safe, but they do provide *some*
   // guarantees, and it means that somebody is using them after move
   // in a valid manner.
-  // TODO: We can still try to identify *unsafe* use after move, such as
-  // dereference of a moved-from smart pointer (which is guaranteed to be 
null).
-  const llvm::StringSet<> StandardMoveSafeClasses = {
+  // TODO: We can still try to identify *unsafe* use after move,
+  // like we did with smart pointers.
+  const llvm::StringSet<> StdSafeClasses = {
   "basic_filebuf",
   "basic_ios",
   "future",
@@ -82,11 +98,8 @@ private:
   "promise",
   "shared_future",
   "shared_lock",
-  "shared_ptr",
   "thread",
-  "unique_ptr",
   "unique_lock",
-  "weak_ptr",
   };
 
   // Should we bother tracking the state of the object?
@@ -97,10 +110,25 @@ private:
 // their user to re-use the storage. The latter is possible because STL
 // objects are known to end up in a valid but unspecified state after the
 // move and their state-reset methods are also known, which allows us to
-// predict precisely when use-after-move is invalid. In aggressive mode,
-// warn on any use-after-move because the user has intentionally asked us
-// to completely eliminate use-after-move in his code.
-return IsAggressive || OK.Local || OK.STL;
+// predict precisely when use-after-move is invalid.
+// Some STL objects are known to conform to additional contracts after 
move,
+// so they are not tracked. However, smart pointers specifically are 
tracked
+// because we can perform extra checking over them.
+// In aggressive mode, warn on any use-after-move because the user has
+// intentionally asked us to completely eliminate use-after-move
+// in his code.
+return IsAggressive || OK.IsLocal
+|| OK.StdKind == SK_Unsafe || OK.StdKind == 
SK_SmartPtr;
+  }
+
+  // Some objects only suffer from some kinds of misuses, but we need to track
+  // them anyway because we cannot know in advance what misuse will we find.
+  bool shouldWarnAbout(ObjectKind OK, MisuseKind MK) const {
+// Additionally, only warn on smart pointers when they are dereferenced (or
+// local or we are aggressive).
+return shouldBeTracked(OK) &&
+   (IsAggressive || OK.IsLocal
+ || OK.StdKind != SK_SmartPtr || MK == MK_Dereference);
   }
 
   // Obtains ObjectKind of an object. Because class declaration cannot always
@@ -108,17 +136,17 @@ private:
   ObjectKind classifyObject(const MemRegion *MR, const CXXRecordDecl *RD) 
const;
 
   // Classifies the object and dumps a user-friendly description string to
-  // the stream. Return value is equivalent to classifyObject.
-  ObjectKind explainObject(llvm::raw_ostream &OS,
-   const MemRegion *MR, con

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

[Chris Kennelly via Phabricator via cfe-commits]

ckennelly updated this revision to Diff 178423.
ckennelly added a comment.

Added update to cxx_status.html for partial implementation of feature test 
macros adopted in San Diego


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

https://reviews.llvm.org/D55741

Files:
  lib/Frontend/InitPreprocessor.cpp
  test/Lexer/cxx-features.cpp
  www/cxx_status.html


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -1054,9 +1054,9 @@
 Available in Clang?
  
 
-  SD-6: SG10 feature test recommendations
-  http://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations";>SD-6
-  N/A
+  SD-6: SG10 feature test recommendations
+  http://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations";>SD-6
+  N/A
   
 Clang 3.4 (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3745";>N3745)
   
@@ -1081,6 +1081,11 @@
 Clang 7 (http://wg21.link/p0096r5";>P0096R5)
   
 
+
+  
+WIP (http://wg21.link/p1353r0";>P1353R0)
+  
+
 

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.

Thanks! Do you need someone to commit for you?


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

https://reviews.llvm.org/D55741



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

r349327 - [analyzer] MoveChecker: Add an option to suppress warnings on locals.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Sun Dec 16 22:19:32 2018
New Revision: 349327

URL: http://llvm.org/viewvc/llvm-project?rev=349327&view=rev
Log:
[analyzer] MoveChecker: Add an option to suppress warnings on locals.

Re-using a moved-from local variable is most likely a bug because there's
rarely a good motivation for not introducing a separate variable instead.
We plan to keep emitting such warnings by default.

Introduce a flag that allows disabling warnings on local variables that are
not of a known move-unsafe type. If it doesn't work out as we expected,
we'll just flip the flag.

We still warn on move-unsafe objects and unsafe operations on known move-safe
objects.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
cfe/trunk/test/Analysis/use-after-move.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp?rev=349327&r1=349326&r2=349327&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp Sun Dec 16 22:19:32 
2018
@@ -26,7 +26,6 @@ using namespace clang;
 using namespace ento;
 
 namespace {
-
 struct RegionState {
 private:
   enum Kind { Moved, Reported } K;
@@ -42,7 +41,9 @@ public:
   bool operator==(const RegionState &X) const { return K == X.K; }
   void Profile(llvm::FoldingSetNodeID &ID) const { ID.AddInteger(K); }
 };
+} // end of anonymous namespace
 
+namespace {
 class MoveChecker
 : public Checker {
@@ -62,8 +63,18 @@ public:
 
 private:
   enum MisuseKind { MK_FunCall, MK_Copy, MK_Move, MK_Dereference };
+  // This needs to be unsigned in order to avoid undefined behavior
+  // when putting it into a tight bitfield.
   enum StdObjectKind : unsigned { SK_NonStd, SK_Unsafe, SK_Safe, SK_SmartPtr };
 
+  enum AggressivenessKind { // In any case, don't warn after a reset.
+AK_Invalid = -1,
+AK_KnownsOnly = 0,  // Warn only about known move-unsafe classes.
+AK_KnownsAndLocals = 1, // Also warn about all local objects.
+AK_All = 2, // Warn on any use-after-move.
+AK_NumKinds = AK_All
+  };
+
   static bool misuseCausesCrash(MisuseKind MK) {
 return MK == MK_Dereference;
   }
@@ -117,8 +128,9 @@ private:
 // In aggressive mode, warn on any use-after-move because the user has
 // intentionally asked us to completely eliminate use-after-move
 // in his code.
-return IsAggressive || OK.IsLocal
-|| OK.StdKind == SK_Unsafe || OK.StdKind == 
SK_SmartPtr;
+return (Aggressiveness == AK_All) ||
+   (Aggressiveness >= AK_KnownsAndLocals && OK.IsLocal) ||
+   OK.StdKind == SK_Unsafe || OK.StdKind == SK_SmartPtr;
   }
 
   // Some objects only suffer from some kinds of misuses, but we need to track
@@ -127,8 +139,9 @@ private:
 // Additionally, only warn on smart pointers when they are dereferenced (or
 // local or we are aggressive).
 return shouldBeTracked(OK) &&
-   (IsAggressive || OK.IsLocal
- || OK.StdKind != SK_SmartPtr || MK == MK_Dereference);
+   ((Aggressiveness == AK_All) ||
+(Aggressiveness >= AK_KnownsAndLocals && OK.IsLocal) ||
+OK.StdKind != SK_SmartPtr || MK == MK_Dereference);
   }
 
   // Obtains ObjectKind of an object. Because class declaration cannot always
@@ -173,10 +186,17 @@ private:
 bool Found;
   };
 
-  bool IsAggressive = false;
+  AggressivenessKind Aggressiveness;
 
 public:
-  void setAggressiveness(bool Aggressive) { IsAggressive = Aggressive; }
+  void setAggressiveness(StringRef Str) {
+Aggressiveness =
+llvm::StringSwitch(Str)
+.Case("KnownsOnly", AK_KnownsOnly)
+.Case("KnownsAndLocals", AK_KnownsAndLocals)
+.Case("All", AK_All)
+.Default(AK_KnownsAndLocals); // A sane default.
+  };
 
 private:
   mutable std::unique_ptr BT;
@@ -717,6 +737,6 @@ void MoveChecker::printState(raw_ostream
 }
 void ento::registerMoveChecker(CheckerManager &mgr) {
   MoveChecker *chk = mgr.registerChecker();
-  chk->setAggressiveness(mgr.getAnalyzerOptions().getCheckerBooleanOption(
-  "Aggressive", false, chk));
+  chk->setAggressiveness(
+  mgr.getAnalyzerOptions().getCheckerStringOption("WarnOn", "", chk));
 }

Modified: cfe/trunk/test/Analysis/use-after-move.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/use-after-move.cpp?rev=349327&r1=349326&r2=349327&view=diff
==
--- cfe/trunk/test/Analysis/use-after-move.cpp (original)
+++ cfe/trunk/test/Analysis/use-after-move.cpp Sun Dec 16 22:19:32 2018
@@ -9,12 +9,22 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++

[PATCH] D55730: [analyzer] MoveChecker Pt.9: Add a "peaceful" mode in which it finds only 100% authentic bugs.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL349327: [analyzer] MoveChecker: Add an option to suppress 
warnings on locals. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55730?vs=178322&id=178427#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55730

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  cfe/trunk/test/Analysis/use-after-move.cpp

Index: cfe/trunk/test/Analysis/use-after-move.cpp
===
--- cfe/trunk/test/Analysis/use-after-move.cpp
+++ cfe/trunk/test/Analysis/use-after-move.cpp
@@ -9,12 +9,22 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-config alpha.cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
 // RUN:  -analyzer-checker debug.ExprInspection
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-config alpha.cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
+// RUN:  -analyzer-checker debug.ExprInspection
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
+// RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
+// RUN:  -analyzer-config alpha.cplusplus.Move:WarnOn=All -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
+// RUN:  -analyzer-config alpha.cplusplus.Move:WarnOn=All -DAGGRESSIVE\
 // RUN:  -analyzer-checker debug.ExprInspection
 
 #include "Inputs/system-header-simulator-cxx.h"
@@ -119,18 +129,33 @@
 void simpleMoveCtorTest() {
   {
 A a;
-A b = std::move(a); // expected-note {{Object 'a' is moved}}
-a.foo();// expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
+A b = std::move(a);
+a.foo();
+#ifndef PEACEFUL
+// expected-note@-3 {{Object 'a' is moved}}
+// expected-warning@-3 {{Method called on moved-from object 'a'}}
+// expected-note@-4{{Method called on moved-from object 'a'}}
+#endif
   }
   {
 A a;
-A b = std::move(a); // expected-note {{Object 'a' is moved}}
-b = a;  // expected-warning {{Moved-from object 'a' is copied}} expected-note {{Moved-from object 'a' is copied}}
+A b = std::move(a);
+b = a;
+#ifndef PEACEFUL
+// expected-note@-3 {{Object 'a' is moved}}
+// expected-warning@-3 {{Moved-from object 'a' is copied}}
+// expected-note@-4{{Moved-from object 'a' is copied}}
+#endif
   }
   {
 A a;
-A b = std::move(a); // expected-note {{Object 'a' is moved}}
-b = std::move(a);   // expected-warning {{Moved-from object 'a' is moved}} expected-note {{Moved-from object 'a' is moved}}
+A b = std::move(a);
+b = std::move(a);
+#ifndef PEACEFUL
+// expected-note@-3 {{Object 'a' is moved}}
+// expected-warning@-3 {{Moved-from object 'a' is moved}}
+// expected-note@-4{{Moved-from object 'a' is moved}}
+#endif
   }
 }
 
@@ -138,20 +163,35 @@
   {
 A a;
 A b;
-b = std::move(a); // expected-note {{Object 'a' is moved}}
-a.foo();  // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}}
+b = std::move(a);
+a.foo();
+#ifndef PEACEFUL
+// expected-note@-3 {{Object 'a' is moved}}
+// expected-warning@-3 {{Method called on moved-from object 'a'}}
+// expected-note@-4{{Method called on moved-from object 'a'}}
+#endif
   }
   {
 A a;
 A b;
-b = std::move(a); // expected-note {{Object 'a' is moved}}
-A c(a);   // expected-warning {{Moved-from object 'a' is copied}} expected-note {{Moved-from object 'a' is copied}}
+b = std::move(a);
+A c(a);
+#ifndef PEACEFUL
+// expected-note@-3 {{Object 'a' is moved}}
+// expected-warning@-3 {{Moved-from object 'a' is copied}}
+// expected-note@-4{{Moved-from object 'a' is copied}}
+#endif
   }
   {
 A a;
 A b;
-b = std::move(a);  // expected-no

[PATCH] D38675: [analyzer] MoveChecker Pt.10: Move the checker out of alpha state.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Currently the checker is very quiet. I have seen exactly 6 warnings on internal 
codebases on which i tested it (on which we otherwise emit around 12000 
warnings), and all of them were good. The only false positive i've seen so far 
was fixed in D55566 .

We might find more problems with liveness analysis being too conservative, but 
i'm not seeing many of them right now.

We might also come to a conclusion that the local variable heuristic is not 
that good, but we have an option to quickly turn it off by flipping a flag 
introduced in D55730 .

So i think this is safe to enable.


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

https://reviews.llvm.org/D38675



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

r349328 - [analyzer] MoveChecker: Enable by default as cplusplus.Move.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Sun Dec 16 22:30:39 2018
New Revision: 349328

URL: http://llvm.org/viewvc/llvm-project?rev=349328&view=rev
Log:
[analyzer] MoveChecker: Enable by default as cplusplus.Move.

This checker warns you when you re-use an object after moving it.

Mostly developed by Peter Szecsi!

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/test/Analysis/mismatched-iterator.cpp
cfe/trunk/test/Analysis/use-after-move.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=349328&r1=349327&r2=349328&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Sun Dec 16 
22:30:39 2018
@@ -275,6 +275,9 @@ def NewDeleteLeaksChecker : Checker<"New
 def CXXSelfAssignmentChecker : Checker<"SelfAssignment">,
   HelpText<"Checks C++ copy and move assignment operators for self 
assignment">;
 
+def MoveChecker: Checker<"Move">,
+   HelpText<"Find use-after-move bugs in C++">;
+
 } // end: "cplusplus"
 
 let ParentPackage = CplusplusOptIn in {
@@ -303,9 +306,6 @@ def MismatchedIteratorChecker : Checker<
   HelpText<"Check for use of iterators of different containers where iterators 
"
"of the same container are expected">;
 
-def MoveChecker: Checker<"Move">,
- HelpText<"Find use-after-move bugs in C++">;
-
 def UninitializedObjectChecker: Checker<"UninitializedObject">,
  HelpText<"Reports uninitialized fields after object construction">;
 

Modified: cfe/trunk/test/Analysis/mismatched-iterator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/mismatched-iterator.cpp?rev=349328&r1=349327&r2=349328&view=diff
==
--- cfe/trunk/test/Analysis/mismatched-iterator.cpp (original)
+++ cfe/trunk/test/Analysis/mismatched-iterator.cpp Sun Dec 16 22:30:39 2018
@@ -100,7 +100,7 @@ void good_move_find2(std::vector &v
 void good_move_find3(std::vector &v1, std::vector &v2, int n) {
   auto i0 = v2.cend();
   v1 = std::move(v2);
-  v2.push_back(n);
+  v2.push_back(n); // expected-warning{{Method called on moved-from object of 
type 'std::vector'}}
   std::find(v2.cbegin(), i0, n); // no-warning
 }
 
@@ -125,6 +125,7 @@ void bad_move_find1(std::vector &v1
   auto i0 = v2.cbegin();
   v1 = std::move(v2);
   std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different 
containers used where the same container is expected}}
+   // expected-warning@-1{{Method called on 
moved-from object of type 'std::vector'}}
 }
 
 void bad_insert_find(std::vector &v1, std::vector &v2, int n, int m) 
{
@@ -167,12 +168,14 @@ void bad_move(std::vector &v1, std:
   const auto i0 = ++v2.cbegin();
   v1 = std::move(v2);
   v2.erase(i0); // expected-warning{{Container accessed using foreign iterator 
argument}}
+// expected-warning@-1{{Method called on moved-from object of 
type 'std::vector'}}
 }
 
 void bad_move_find2(std::vector &v1, std::vector &v2, int n) {
   auto i0 = --v2.cend();
   v1 = std::move(v2);
   std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different 
containers used where the same container is expected}}
+   // expected-warning@-1{{Method called on 
moved-from object of type 'std::vector'}}
 }
 
 void bad_move_find3(std::vector &v1, std::vector &v2, int n) {

Modified: cfe/trunk/test/Analysis/use-after-move.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/use-after-move.cpp?rev=349328&r1=349327&r2=349328&view=diff
==
--- cfe/trunk/test/Analysis/use-after-move.cpp (original)
+++ cfe/trunk/test/Analysis/use-after-move.cpp Sun Dec 16 22:30:39 2018
@@ -1,30 +1,30 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config 
eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
 // RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config 
eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
 // RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text 

[PATCH] D38675: [analyzer] MoveChecker Pt.10: Move the checker out of alpha state.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC349328: [analyzer] MoveChecker: Enable by default as 
cplusplus.Move. (authored by dergachev, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D38675

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  test/Analysis/mismatched-iterator.cpp
  test/Analysis/use-after-move.cpp

Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -100,7 +100,7 @@
 void good_move_find3(std::vector &v1, std::vector &v2, int n) {
   auto i0 = v2.cend();
   v1 = std::move(v2);
-  v2.push_back(n);
+  v2.push_back(n); // expected-warning{{Method called on moved-from object of type 'std::vector'}}
   std::find(v2.cbegin(), i0, n); // no-warning
 }
 
@@ -125,6 +125,7 @@
   auto i0 = v2.cbegin();
   v1 = std::move(v2);
   std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+   // expected-warning@-1{{Method called on moved-from object of type 'std::vector'}}
 }
 
 void bad_insert_find(std::vector &v1, std::vector &v2, int n, int m) {
@@ -167,12 +168,14 @@
   const auto i0 = ++v2.cbegin();
   v1 = std::move(v2);
   v2.erase(i0); // expected-warning{{Container accessed using foreign iterator argument}}
+// expected-warning@-1{{Method called on moved-from object of type 'std::vector'}}
 }
 
 void bad_move_find2(std::vector &v1, std::vector &v2, int n) {
   auto i0 = --v2.cend();
   v1 = std::move(v2);
   std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+   // expected-warning@-1{{Method called on moved-from object of type 'std::vector'}}
 }
 
 void bad_move_find3(std::vector &v1, std::vector &v2, int n) {
Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -1,30 +1,30 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
 // RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
 // RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config alpha.cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
 // RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config alpha.cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=KnownsOnly -DPEACEFUL\
 // RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
-// RUN:  -analyzer-config alpha.cplusplus.Move:WarnOn=All -DAGGRESSIVE\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
 // RUN:  -analyzer-checker debug.ExprInspection
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
-// RUN:  -analyzer-config alpha.cplusplus.Move:WarnOn=All -DAGGRESSIVE\
+// RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
 // RUN:  -analyzer-checker d

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ok, D55388  seems to have landed (3 builds 
without failures in move checker), so i guess these are separate problems after 
all.

The first failing build seems to be 
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/2662 - it 
includes both this commit and rC349281 .


Repository:
  rC Clang

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

https://reviews.llvm.org/D54834



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