from:"Pierre Gousseau via cfe\-commits"

[clang] 937e63b - [X86] Fix x86-header-warnings.c test not detecting regressions as intended.

2020-04-08 Thread Pierre Gousseau via cfe-commits


Author: Pierre Gousseau
Date: 2020-04-08T15:22:44+01:00
New Revision: 937e63b8d5e961c2a7da25558bbcdd5388182b67

URL: 
https://github.com/llvm/llvm-project/commit/937e63b8d5e961c2a7da25558bbcdd5388182b67
DIFF: 
https://github.com/llvm/llvm-project/commit/937e63b8d5e961c2a7da25558bbcdd5388182b67.diff

LOG: [X86] Fix x86-header-warnings.c test not detecting regressions as intended.

Use -verify -fsyntax-only and expected-no-diagnostics as
recommended by Paul.

Reviewed By: probinson

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

Added: 


Modified: 
clang/test/Headers/x86-header-warnings.c

Removed: 




diff  --git a/clang/test/Headers/x86-header-warnings.c 
b/clang/test/Headers/x86-header-warnings.c
index ec7cfc6adaad..3306b95c1b48 100644
--- a/clang/test/Headers/x86-header-warnings.c
+++ b/clang/test/Headers/x86-header-warnings.c
@@ -1,16 +1,14 @@
 // Fix sign conversion warnings found by fsanitize=implicit-integer-sign-change
 // in intrinsic headers.
 // Preprocess file to workaround no warnings in system headers.
-// RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -ffreestanding -E 2>&1 \
-// RUN: | %clang_cc1 -x c - -triple x86_64-pc-linux-gnu -ffreestanding 
-Wsign-conversion -E -o - 2>&1 \
-// RUN: | FileCheck --allow-empty %s
+// RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -ffreestanding -E -CC 2>&1 \
+// RUN: | %clang_cc1 -x c - -triple x86_64-pc-linux-gnu -Wsign-conversion 
-fsyntax-only -verify
 // REQUIRES: x86-registered-target
 
 #include 
 
 void test0() {
-  // CHECK-LABEL: test0
-  // CHECK-NOT: warning:
+  // expected-no-diagnostics
   _MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_ON);
   _MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_OFF);
   _MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_MASK);



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

[clang] 08fab9e - [X86] Fix implicit sign conversion warnings in X86 headers.

2020-04-07 Thread Pierre Gousseau via cfe-commits


Author: Pierre Gousseau
Date: 2020-04-07T11:25:08+01:00
New Revision: 08fab9ebecf72a682279d75489dc2460121cbeed

URL: 
https://github.com/llvm/llvm-project/commit/08fab9ebecf72a682279d75489dc2460121cbeed
DIFF: 
https://github.com/llvm/llvm-project/commit/08fab9ebecf72a682279d75489dc2460121cbeed.diff

LOG: [X86] Fix implicit sign conversion warnings in X86 headers.

Warnings in emmintrin.h and xmmintrin.h are reported by
-fsanitize=implicit-integer-sign-change.

Reviewed By: RKSimon, craig.topper

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

Added: 
clang/test/Headers/x86-header-warnings.c

Modified: 
clang/lib/Headers/emmintrin.h
clang/lib/Headers/xmmintrin.h

Removed: 




diff  --git a/clang/lib/Headers/emmintrin.h b/clang/lib/Headers/emmintrin.h
index 993c688ce818..73a777b107c6 100644
--- a/clang/lib/Headers/emmintrin.h
+++ b/clang/lib/Headers/emmintrin.h
@@ -4970,10 +4970,10 @@ void _mm_pause(void);
 
 #define _MM_SHUFFLE2(x, y) (((x) << 1) | (y))
 
-#define _MM_DENORMALS_ZERO_ON   (0x0040)
-#define _MM_DENORMALS_ZERO_OFF  (0x)
+#define _MM_DENORMALS_ZERO_ON   (0x0040U)
+#define _MM_DENORMALS_ZERO_OFF  (0xU)
 
-#define _MM_DENORMALS_ZERO_MASK (0x0040)
+#define _MM_DENORMALS_ZERO_MASK (0x0040U)
 
 #define _MM_GET_DENORMALS_ZERO_MODE() (_mm_getcsr() & _MM_DENORMALS_ZERO_MASK)
 #define _MM_SET_DENORMALS_ZERO_MODE(x) (_mm_setcsr((_mm_getcsr() & 
~_MM_DENORMALS_ZERO_MASK) | (x)))

diff  --git a/clang/lib/Headers/xmmintrin.h b/clang/lib/Headers/xmmintrin.h
index 9b8de63f04d5..f4686691c7ed 100644
--- a/clang/lib/Headers/xmmintrin.h
+++ b/clang/lib/Headers/xmmintrin.h
@@ -2931,31 +2931,31 @@ _mm_movemask_ps(__m128 __a)
 
 #define _MM_SHUFFLE(z, y, x, w) (((z) << 6) | ((y) << 4) | ((x) << 2) | (w))
 
-#define _MM_EXCEPT_INVALID(0x0001)
-#define _MM_EXCEPT_DENORM (0x0002)
-#define _MM_EXCEPT_DIV_ZERO   (0x0004)
-#define _MM_EXCEPT_OVERFLOW   (0x0008)
-#define _MM_EXCEPT_UNDERFLOW  (0x0010)
-#define _MM_EXCEPT_INEXACT(0x0020)
-#define _MM_EXCEPT_MASK   (0x003f)
-
-#define _MM_MASK_INVALID  (0x0080)
-#define _MM_MASK_DENORM   (0x0100)
-#define _MM_MASK_DIV_ZERO (0x0200)
-#define _MM_MASK_OVERFLOW (0x0400)
-#define _MM_MASK_UNDERFLOW(0x0800)
-#define _MM_MASK_INEXACT  (0x1000)
-#define _MM_MASK_MASK (0x1f80)
-
-#define _MM_ROUND_NEAREST (0x)
-#define _MM_ROUND_DOWN(0x2000)
-#define _MM_ROUND_UP  (0x4000)
-#define _MM_ROUND_TOWARD_ZERO (0x6000)
-#define _MM_ROUND_MASK(0x6000)
-
-#define _MM_FLUSH_ZERO_MASK   (0x8000)
-#define _MM_FLUSH_ZERO_ON (0x8000)
-#define _MM_FLUSH_ZERO_OFF(0x)
+#define _MM_EXCEPT_INVALID(0x0001U)
+#define _MM_EXCEPT_DENORM (0x0002U)
+#define _MM_EXCEPT_DIV_ZERO   (0x0004U)
+#define _MM_EXCEPT_OVERFLOW   (0x0008U)
+#define _MM_EXCEPT_UNDERFLOW  (0x0010U)
+#define _MM_EXCEPT_INEXACT(0x0020U)
+#define _MM_EXCEPT_MASK   (0x003fU)
+
+#define _MM_MASK_INVALID  (0x0080U)
+#define _MM_MASK_DENORM   (0x0100U)
+#define _MM_MASK_DIV_ZERO (0x0200U)
+#define _MM_MASK_OVERFLOW (0x0400U)
+#define _MM_MASK_UNDERFLOW(0x0800U)
+#define _MM_MASK_INEXACT  (0x1000U)
+#define _MM_MASK_MASK (0x1f80U)
+
+#define _MM_ROUND_NEAREST (0xU)
+#define _MM_ROUND_DOWN(0x2000U)
+#define _MM_ROUND_UP  (0x4000U)
+#define _MM_ROUND_TOWARD_ZERO (0x6000U)
+#define _MM_ROUND_MASK(0x6000U)
+
+#define _MM_FLUSH_ZERO_MASK   (0x8000U)
+#define _MM_FLUSH_ZERO_ON (0x8000U)
+#define _MM_FLUSH_ZERO_OFF(0xU)
 
 #define _MM_GET_EXCEPTION_MASK() (_mm_getcsr() & _MM_MASK_MASK)
 #define _MM_GET_EXCEPTION_STATE() (_mm_getcsr() & _MM_EXCEPT_MASK)

diff  --git a/clang/test/Headers/x86-header-warnings.c 
b/clang/test/Headers/x86-header-warnings.c
new file mode 100644
index ..ec7cfc6adaad
--- /dev/null
+++ b/clang/test/Headers/x86-header-warnings.c
@@ -0,0 +1,43 @@
+// Fix sign conversion warnings found by fsanitize=implicit-integer-sign-change
+// in intrinsic headers.
+// Preprocess file to workaround no warnings in system headers.
+// RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -ffreestanding -E 2>&1 \
+// RUN: | %clang_cc1 -x c - -triple x86_64-pc-linux-gnu -ffreestanding 
-Wsign-conversion -E -o - 2>&1 \
+// RUN: | FileCheck --allow-empty %s
+// REQUIRES: x86-registered-target
+
+#include 
+
+void test0() {
+  // CHECK-LABEL: test0
+  // CHECK-NOT: warning:
+  _MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_ON);
+  _MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_OFF);
+  _MM_SET_DENORMALS_ZERO_MODE(_MM_DENORMALS_ZERO_MASK);
+
+  _MM_SET_EXCEPTION_STATE(_MM_EXCEPT_INVALID);
+  _MM_SET_EXCEPTION_STATE(_MM_EXCEPT_DENORM);
+  _MM_SET_EXCEPTION_STATE(_MM_EXCEPT_DIV_ZERO);
+  _MM_SET_EXCEPTION_STATE(_MM_EXCEPT_OVERFLOW);
+  _MM_SET_EXCEPTION_STATE(_MM_EXCEPT_UNDERFLOW);
+  _MM_SET_EXCEPTION_STATE(_MM_EXCEPT_INEXACT);
+  _MM_SET_EXCEPTIO

r358285 - [asan] Add gcc 8's driver option -fsanitize=pointer-compare and -fsanitize=pointer-substract.

2019-04-12 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Fri Apr 12 07:14:58 2019
New Revision: 358285

URL: http://llvm.org/viewvc/llvm-project?rev=358285&view=rev
Log:
[asan] Add gcc 8's driver option -fsanitize=pointer-compare and 
-fsanitize=pointer-substract.

Disabled by default as this is still an experimental feature.

Reviewed By: thakis

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

Modified:
cfe/trunk/include/clang/Basic/Sanitizers.def
cfe/trunk/include/clang/Driver/SanitizerArgs.h
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChains/CrossWindows.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp
cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
cfe/trunk/lib/Driver/ToolChains/MinGW.cpp
cfe/trunk/lib/Driver/ToolChains/NetBSD.cpp
cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp
cfe/trunk/lib/Driver/ToolChains/Solaris.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/include/clang/Basic/Sanitizers.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.def?rev=358285&r1=358284&r2=358285&view=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.def (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.def Fri Apr 12 07:14:58 2019
@@ -40,6 +40,12 @@
 // AddressSanitizer
 SANITIZER("address", Address)
 
+// Requires AddressSanitizer
+SANITIZER("pointer-compare", PointerCompare)
+
+// Requires AddressSanitizer
+SANITIZER("pointer-subtract", PointerSubtract)
+
 // Kernel AddressSanitizer (KASan)
 SANITIZER("kernel-address", KernelAddress)
 

Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/SanitizerArgs.h?rev=358285&r1=358284&r2=358285&view=diff
==
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h (original)
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h Fri Apr 12 07:14:58 2019
@@ -38,6 +38,8 @@ class SanitizerArgs {
   bool AsanPoisonCustomArrayCookie = false;
   bool AsanGlobalsDeadStripping = false;
   bool AsanUseOdrIndicator = false;
+  bool AsanInvalidPointerCmp = false;
+  bool AsanInvalidPointerSub = false;
   std::string HwasanAbi;
   bool LinkCXXRuntimes = false;
   bool NeedPIE = false;

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=358285&r1=358284&r2=358285&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Fri Apr 12 07:14:58 2019
@@ -775,8 +775,27 @@ SanitizerArgs::SanitizerArgs(const ToolC
 Args.hasFlag(options::OPT_fsanitize_address_use_odr_indicator,
  options::OPT_fno_sanitize_address_use_odr_indicator,
  AsanUseOdrIndicator);
+
+if (AllAddedKinds & SanitizerKind::PointerCompare & ~AllRemove) {
+  AsanInvalidPointerCmp = true;
+}
+
+if (AllAddedKinds & SanitizerKind::PointerSubtract & ~AllRemove) {
+  AsanInvalidPointerSub = true;
+}
+
   } else {
 AsanUseAfterScope = false;
+// -fsanitize=pointer-compare/pointer-subtract requires -fsanitize=address.
+SanitizerMask DetectInvalidPointerPairs =
+SanitizerKind::PointerCompare | SanitizerKind::PointerSubtract;
+if (AllAddedKinds & DetectInvalidPointerPairs & ~AllRemove) {
+  TC.getDriver().Diag(clang::diag::err_drv_argument_only_allowed_with)
+  << lastArgumentForMask(D, Args,
+ SanitizerKind::PointerCompare |
+ SanitizerKind::PointerSubtract)
+  << "-fsanitize=address";
+}
   }
 
   if (AllAddedKinds & SanitizerKind::HWAddress) {
@@ -963,6 +982,16 @@ void SanitizerArgs::addArgs(const ToolCh
   if (AsanUseOdrIndicator)
 CmdArgs.push_back("-fsanitize-address-use-odr-indicator");
 
+  if (AsanInvalidPointerCmp) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-asan-detect-invalid-pointer-cmp");
+  }
+
+  if (AsanInvalidPointerSub) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-asan-detect-invalid-pointer-sub");
+  }
+
   if (!HwasanAbi.empty()) {
 CmdArgs.push_back("-default-function-attr");
 CmdArgs.push_back(Args.MakeArgString("hwasan-abi=" + HwasanAbi));

Modified: cfe/trunk/lib/Driver/ToolChains/CrossWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CrossWindows.cpp?rev=358285&r1=358284&r2=358285&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CrossWindows.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CrossWindows.cpp Fri Apr 12 07:14:58 2019
@@

r357480 - [Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

2019-04-02 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Tue Apr  2 08:20:26 2019
New Revision: 357480

URL: http://llvm.org/viewvc/llvm-project?rev=357480&view=rev
Log:
[Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

Can be safely enabled on PS4.

Reviewed By: probinson

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

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=357480&r1=357479&r2=357480&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Tue Apr  2 08:20:26 2019
@@ -768,6 +768,7 @@ SanitizerArgs::SanitizerArgs(const ToolC
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
 AsanGlobalsDeadStripping =
 !TC.getTriple().isOSBinFormatELF() || TC.getTriple().isOSFuchsia() ||
+TC.getTriple().isPS4() ||
 Args.hasArg(options::OPT_fsanitize_address_globals_dead_stripping);
 
 AsanUseOdrIndicator =

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=357480&r1=357479&r2=357480&view=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Tue Apr  2 08:20:26 2019
@@ -221,6 +221,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address 
-fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -### -- %s 
2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
+// RUN: %clang -target x86_64-scei-ps4  -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // CHECK-ASAN-GLOBALS: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 // CHECK-NO-ASAN-GLOBALS-NOT: 
-cc1{{.*}}-fsanitize-address-globals-dead-stripping
 


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

r355190 - [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-03-01 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Fri Mar  1 02:05:15 2019
New Revision: 355190

URL: http://llvm.org/viewvc/llvm-project?rev=355190&view=rev
Log:
[Driver] Allow enum SanitizerOrdinal to represent more than 64 different 
sanitizer checks, NFC.

enum SanitizerOrdinal has reached maximum capacity, this change extends the 
capacity to 128 sanitizer checks.
This can eventually allow us to add gcc 8's options 
"-fsanitize=pointer-substract" and "-fsanitize=pointer-compare".

This is a recommit of r354873 but with a fix for unqualified lookup error in 
lldb cmake build bot.

Fixes: https://llvm.org/PR39425

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/Sanitizers.def
cfe/trunk/include/clang/Basic/Sanitizers.h
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Basic/SanitizerSpecialCaseList.cpp
cfe/trunk/lib/Basic/Sanitizers.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=355190&r1=355189&r2=355190&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Fri Mar  1 02:05:15 2019
@@ -2366,7 +2366,7 @@ def NoSanitize : InheritableAttr {
   let Documentation = [NoSanitizeDocs];
   let AdditionalMembers = [{
 SanitizerMask getMask() const {
-  SanitizerMask Mask = 0;
+  SanitizerMask Mask;
   for (auto SanitizerName : sanitizers()) {
 SanitizerMask ParsedMask =
 parseSanitizerValue(SanitizerName, /*AllowGroups=*/true);

Modified: cfe/trunk/include/clang/Basic/Sanitizers.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.def?rev=355190&r1=355189&r2=355190&view=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.def (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.def Fri Mar  1 02:05:15 2019
@@ -177,7 +177,7 @@ SANITIZER("scudo", Scudo)
 
 // Magic group, containing all sanitizers. For example, "-fno-sanitize=all"
 // can be used to disable all the sanitizers.
-SANITIZER_GROUP("all", All, ~0ULL)
+SANITIZER_GROUP("all", All, ~SanitizerMask())
 
 #undef SANITIZER
 #undef SANITIZER_GROUP

Modified: cfe/trunk/include/clang/Basic/Sanitizers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.h?rev=355190&r1=355189&r2=355190&view=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.h (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.h Fri Mar  1 02:05:15 2019
@@ -20,45 +20,166 @@
 #include 
 #include 
 
+namespace llvm {
+class hash_code;
+}
+
 namespace clang {
 
-using SanitizerMask = uint64_t;
+class SanitizerMask {
+  /// Number of array elements.
+  static constexpr unsigned kNumElem = 2;
+  /// Mask value initialized to 0.
+  uint64_t maskLoToHigh[kNumElem]{};
+  /// Number of bits in a mask.
+  static constexpr unsigned kNumBits = sizeof(decltype(maskLoToHigh)) * 8;
+  /// Number of bits in a mask element.
+  static constexpr unsigned kNumBitElem = sizeof(decltype(maskLoToHigh[0])) * 
8;
+
+public:
+  static constexpr bool checkBitPos(const unsigned Pos) {
+return Pos < kNumBits;
+  }
 
-namespace SanitizerKind {
+  /// Create a mask with a bit enabled at position Pos.
+  static SanitizerMask bitPosToMask(const unsigned Pos) {
+assert(Pos < kNumBits && "Bit position too big.");
+SanitizerMask mask;
+mask.maskLoToHigh[Pos / kNumBitElem] = 1ULL << Pos % kNumBitElem;
+return mask;
+  }
 
-// Assign ordinals to possible values of -fsanitize= flag, which we will use as
-// bit positions.
-enum SanitizerOrdinal : uint64_t {
-#define SANITIZER(NAME, ID) SO_##ID,
-#define SANITIZER_GROUP(NAME, ID, ALIAS) SO_##ID##Group,
-#include "clang/Basic/Sanitizers.def"
-  SO_Count
+  unsigned countPopulation() const {
+unsigned total = 0;
+for (const auto &Val : maskLoToHigh)
+  total += llvm::countPopulation(Val);
+return total;
+  }
+
+  void flipAllBits() {
+for (auto &Val : maskLoToHigh)
+  Val = ~Val;
+  }
+
+  bool isPowerOf2() const {
+return countPopulation() == 1;
+  }
+
+  llvm::hash_code hash_value() const;
+
+  explicit operator bool() const {
+for (const auto &Val : maskLoToHigh)
+  if (Val)
+return true;
+return false;
+  };
+
+  bool operator==(const SanitizerMask &V) const {
+for (unsigned k = 0; k < kNumElem; k++) {
+  if (maskLoToHigh[k] != V.maskLoToHigh[k])
+return false;
+}
+return true;
+  }
+
+  SanitizerMask &operator&=(const Sanitiz

r354875 - revert r354873 as this breaks lldb builds.

2019-02-26 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Tue Feb 26 05:50:29 2019
New Revision: 354875

URL: http://llvm.org/viewvc/llvm-project?rev=354875&view=rev
Log:
revert r354873 as this breaks lldb builds.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/Sanitizers.def
cfe/trunk/include/clang/Basic/Sanitizers.h
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Basic/SanitizerSpecialCaseList.cpp
cfe/trunk/lib/Basic/Sanitizers.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=354875&r1=354874&r2=354875&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 26 05:50:29 2019
@@ -2366,7 +2366,7 @@ def NoSanitize : InheritableAttr {
   let Documentation = [NoSanitizeDocs];
   let AdditionalMembers = [{
 SanitizerMask getMask() const {
-  SanitizerMask Mask;
+  SanitizerMask Mask = 0;
   for (auto SanitizerName : sanitizers()) {
 SanitizerMask ParsedMask =
 parseSanitizerValue(SanitizerName, /*AllowGroups=*/true);

Modified: cfe/trunk/include/clang/Basic/Sanitizers.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.def?rev=354875&r1=354874&r2=354875&view=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.def (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.def Tue Feb 26 05:50:29 2019
@@ -177,7 +177,7 @@ SANITIZER("scudo", Scudo)
 
 // Magic group, containing all sanitizers. For example, "-fno-sanitize=all"
 // can be used to disable all the sanitizers.
-SANITIZER_GROUP("all", All, ~SanitizerMask())
+SANITIZER_GROUP("all", All, ~0ULL)
 
 #undef SANITIZER
 #undef SANITIZER_GROUP

Modified: cfe/trunk/include/clang/Basic/Sanitizers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.h?rev=354875&r1=354874&r2=354875&view=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.h (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.h Tue Feb 26 05:50:29 2019
@@ -21,167 +21,44 @@
 #include 
 
 namespace clang {
-class SanitizerMask;
-}
 
-namespace llvm {
-class hashcode;
-hash_code hash_value(const clang::SanitizerMask &Arg);
-} // namespace llvm
+using SanitizerMask = uint64_t;
 
-namespace clang {
-
-class SanitizerMask {
-  /// Number of array elements.
-  static constexpr unsigned kNumElem = 2;
-  /// Mask value initialized to 0.
-  uint64_t maskLoToHigh[kNumElem]{};
-  /// Number of bits in a mask.
-  static constexpr unsigned kNumBits = sizeof(decltype(maskLoToHigh)) * 8;
-  /// Number of bits in a mask element.
-  static constexpr unsigned kNumBitElem = sizeof(decltype(maskLoToHigh[0])) * 
8;
-
-public:
-  static constexpr bool checkBitPos(const unsigned Pos) {
-return Pos < kNumBits;
-  }
-
-  /// Create a mask with a bit enabled at position Pos.
-  static SanitizerMask bitPosToMask(const unsigned Pos) {
-assert(Pos < kNumBits && "Bit position too big.");
-SanitizerMask mask;
-mask.maskLoToHigh[Pos / kNumBitElem] = 1ULL << Pos % kNumBitElem;
-return mask;
-  }
-
-  unsigned countPopulation() const {
-unsigned total = 0;
-for (const auto &Val : maskLoToHigh)
-  total += llvm::countPopulation(Val);
-return total;
-  }
-
-  void flipAllBits() {
-for (auto &Val : maskLoToHigh)
-  Val = ~Val;
-  }
-
-  bool isPowerOf2() const {
-return countPopulation() == 1;
-  }
-
-  llvm::hash_code hash_value() const;
-
-  explicit operator bool() const {
-for (const auto &Val : maskLoToHigh)
-  if (Val)
-return true;
-return false;
-  };
-
-  bool operator==(const SanitizerMask &V) const {
-for (unsigned k = 0; k < kNumElem; k++) {
-  if (maskLoToHigh[k] != V.maskLoToHigh[k])
-return false;
-}
-return true;
-  }
-
-  SanitizerMask &operator&=(const SanitizerMask &RHS) {
-for (unsigned k = 0; k < kNumElem; k++)
-  maskLoToHigh[k] &= RHS.maskLoToHigh[k];
-return *this;
-  }
+namespace SanitizerKind {
 
-  SanitizerMask &operator|=(const SanitizerMask &RHS) {
-for (unsigned k = 0; k < kNumElem; k++)
-  maskLoToHigh[k] |= RHS.maskLoToHigh[k];
-return *this;
-  }
-
-  bool operator!() const {
-for (const auto &Val : maskLoToHigh)
-  if (Val)
-return false;
-return true;
-  }
-
-  bool operator!=(const SanitizerMask &RHS) const { return !((*this) == RHS); }
-};
-
-inline SanitizerMask operator~(SanitizerMask v) {
-  v.flipAllBits();
-  return v;
-}
-
-inline SanitizerMask operator&

r354873 - [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-02-26 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Tue Feb 26 05:30:14 2019
New Revision: 354873

URL: http://llvm.org/viewvc/llvm-project?rev=354873&view=rev
Log:
[Driver] Allow enum SanitizerOrdinal to represent more than 64 different 
sanitizer checks, NFC.

enum SanitizerOrdinal has reached maximum capacity, this change extends the 
capacity to 128 sanitizer checks.
This can eventually allow us to add gcc 8's options 
"-fsanitize=pointer-substract" and "-fsanitize=pointer-compare".

Fixes: https://llvm.org/PR39425

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/Sanitizers.def
cfe/trunk/include/clang/Basic/Sanitizers.h
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Basic/SanitizerSpecialCaseList.cpp
cfe/trunk/lib/Basic/Sanitizers.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Driver/ToolChain.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=354873&r1=354872&r2=354873&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Feb 26 05:30:14 2019
@@ -2366,7 +2366,7 @@ def NoSanitize : InheritableAttr {
   let Documentation = [NoSanitizeDocs];
   let AdditionalMembers = [{
 SanitizerMask getMask() const {
-  SanitizerMask Mask = 0;
+  SanitizerMask Mask;
   for (auto SanitizerName : sanitizers()) {
 SanitizerMask ParsedMask =
 parseSanitizerValue(SanitizerName, /*AllowGroups=*/true);

Modified: cfe/trunk/include/clang/Basic/Sanitizers.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.def?rev=354873&r1=354872&r2=354873&view=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.def (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.def Tue Feb 26 05:30:14 2019
@@ -177,7 +177,7 @@ SANITIZER("scudo", Scudo)
 
 // Magic group, containing all sanitizers. For example, "-fno-sanitize=all"
 // can be used to disable all the sanitizers.
-SANITIZER_GROUP("all", All, ~0ULL)
+SANITIZER_GROUP("all", All, ~SanitizerMask())
 
 #undef SANITIZER
 #undef SANITIZER_GROUP

Modified: cfe/trunk/include/clang/Basic/Sanitizers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.h?rev=354873&r1=354872&r2=354873&view=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.h (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.h Tue Feb 26 05:30:14 2019
@@ -21,44 +21,167 @@
 #include 
 
 namespace clang {
+class SanitizerMask;
+}
 
-using SanitizerMask = uint64_t;
+namespace llvm {
+class hashcode;
+hash_code hash_value(const clang::SanitizerMask &Arg);
+} // namespace llvm
 
-namespace SanitizerKind {
+namespace clang {
 
-// Assign ordinals to possible values of -fsanitize= flag, which we will use as
-// bit positions.
-enum SanitizerOrdinal : uint64_t {
-#define SANITIZER(NAME, ID) SO_##ID,
-#define SANITIZER_GROUP(NAME, ID, ALIAS) SO_##ID##Group,
-#include "clang/Basic/Sanitizers.def"
-  SO_Count
+class SanitizerMask {
+  /// Number of array elements.
+  static constexpr unsigned kNumElem = 2;
+  /// Mask value initialized to 0.
+  uint64_t maskLoToHigh[kNumElem]{};
+  /// Number of bits in a mask.
+  static constexpr unsigned kNumBits = sizeof(decltype(maskLoToHigh)) * 8;
+  /// Number of bits in a mask element.
+  static constexpr unsigned kNumBitElem = sizeof(decltype(maskLoToHigh[0])) * 
8;
+
+public:
+  static constexpr bool checkBitPos(const unsigned Pos) {
+return Pos < kNumBits;
+  }
+
+  /// Create a mask with a bit enabled at position Pos.
+  static SanitizerMask bitPosToMask(const unsigned Pos) {
+assert(Pos < kNumBits && "Bit position too big.");
+SanitizerMask mask;
+mask.maskLoToHigh[Pos / kNumBitElem] = 1ULL << Pos % kNumBitElem;
+return mask;
+  }
+
+  unsigned countPopulation() const {
+unsigned total = 0;
+for (const auto &Val : maskLoToHigh)
+  total += llvm::countPopulation(Val);
+return total;
+  }
+
+  void flipAllBits() {
+for (auto &Val : maskLoToHigh)
+  Val = ~Val;
+  }
+
+  bool isPowerOf2() const {
+return countPopulation() == 1;
+  }
+
+  llvm::hash_code hash_value() const;
+
+  explicit operator bool() const {
+for (const auto &Val : maskLoToHigh)
+  if (Val)
+return true;
+return false;
+  };
+
+  bool operator==(const SanitizerMask &V) const {
+for (unsigned k = 0; k < kNumElem; k++) {
+  if (maskLoToHigh[k] != V.maskLoToHigh[k])
+return false;
+}
+return true;
+  }
+
+  SanitizerMask &operator&=(const

r352042 - Test commit: fix typo.

2019-01-24 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Thu Jan 24 03:44:24 2019
New Revision: 352042

URL: http://llvm.org/viewvc/llvm-project?rev=352042&view=rev
Log:
Test commit: fix typo.

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=352042&r1=352041&r2=352042&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Thu Jan 24 03:44:24 2019
@@ -251,7 +251,7 @@ SanitizerArgs::SanitizerArgs(const ToolC
   if (RemoveObjectSizeAtO0) {
 AllRemove |= SanitizerKind::ObjectSize;
 
-// The user explicitly enabled the object size sanitizer. Warn that
+// The user explicitly enabled the object size sanitizer. Warn
 // that this does nothing at -O0.
 if (Add & SanitizerKind::ObjectSize)
   D.Diag(diag::warn_drv_object_size_disabled_O0)


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

r349508 - [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4.

2018-12-18 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Tue Dec 18 09:03:35 2018
New Revision: 349508

URL: http://llvm.org/viewvc/llvm-project?rev=349508&view=rev
Log:
[Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or 
-nodefaultlibs on PS4.

NFC for targets other than PS4.

Respect -nostdlib and -nodefaultlibs when enabling asan or ubsan.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp
cfe/trunk/test/Driver/fsanitize.c
cfe/trunk/test/Driver/sanitizer-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=349508&r1=349507&r2=349508&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Dec 18 09:03:35 2018
@@ -4097,7 +4097,8 @@ void Clang::ConstructJob(Compilation &C,
 ABICompatArg->render(Args, CmdArgs);
 
   // Add runtime flag for PS4 when PGO, coverage, or sanitizers are enabled.
-  if (RawTriple.isPS4CPU()) {
+  if (RawTriple.isPS4CPU() &&
+  !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 PS4cpu::addProfileRTArgs(TC, Args, CmdArgs);
 PS4cpu::addSanitizerArgs(TC, CmdArgs);
   }

Modified: cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp?rev=349508&r1=349507&r2=349508&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp Tue Dec 18 09:03:35 2018
@@ -121,7 +121,8 @@ static void ConstructPS4LinkJob(const To
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  AddPS4SanitizerArgs(ToolChain, CmdArgs);
+  if(!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
+AddPS4SanitizerArgs(ToolChain, CmdArgs);
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
@@ -190,7 +191,8 @@ static void ConstructGoldLinkJob(const T
 assert(Output.isNothing() && "Invalid output.");
   }
 
-  AddPS4SanitizerArgs(ToolChain, CmdArgs);
+  if(!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
+AddPS4SanitizerArgs(ToolChain, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
 const char *crt1 = nullptr;

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=349508&r1=349507&r2=349508&view=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Tue Dec 18 09:03:35 2018
@@ -753,6 +753,10 @@
 // CHECK-ASAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
 // CHECK-ASAN-PS4-NOT: {{(\.(o|bc)"? |-l).*-lSceDbgAddressSanitizer_stub_weak}}
 // CHECK-ASAN-PS4: -lSceDbgAddressSanitizer_stub_weak
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nostdlib %s -### 
2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nodefaultlibs %s 
-### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address -nodefaultlibs 
-nostdlib  %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-NOLIB-PS4
+// CHECK-ASAN-NOLIB-PS4-NOT: SceDbgAddressSanitizer_stub_weak
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-MINIMAL
 // CHECK-ASAN-MINIMAL: error: invalid argument '-fsanitize-minimal-runtime' 
not allowed with '-fsanitize=address'

Modified: cfe/trunk/test/Driver/sanitizer-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/sanitizer-ld.c?rev=349508&r1=349507&r2=349508&view=diff
==
--- cfe/trunk/test/Driver/sanitizer-ld.c (original)
+++ cfe/trunk/test/Driver/sanitizer-ld.c Tue Dec 18 09:03:35 2018
@@ -673,6 +673,13 @@
 // CHECK-AUBSAN-PS4: "{{.*}}ld{{(.gold)?(.exe)?}}"
 // CHECK-AUBSAN-PS4: -lSceDbgAddressSanitizer_stub_weak
 
+// RUN: %clang -fsanitize=address,undefined %s -### -o %t.o 2>&1 \
+// RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
+// RUN: -shared \
+// RUN: -nostdlib \
+// RUN:   | FileCheck --check-prefix=CHECK-NOLIB-PS4 %s
+// CHECK-NOLIB-PS4-NOT: SceDbgAddressSanitizer_stub_weak
+
 // RUN: %clang -fsanitize=efficiency-cache-frag %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-unknown-linux -fuse-ld=ld \
 // RUN:   | FileCheck --check-prefix=CHECK-ESAN-LINUX %s


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

r334096 - [Driver] Add flag "--dependent-lib=..." when enabling asan or ubsan on PS4.

2018-06-06 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Wed Jun  6 07:04:15 2018
New Revision: 334096

URL: http://llvm.org/viewvc/llvm-project?rev=334096&view=rev
Log:
[Driver] Add flag "--dependent-lib=..." when enabling asan or ubsan on PS4.

NFC for targets other than PS4.

Simplify users' workflow when enabling asan or ubsan and calling the linker 
separately.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp
cfe/trunk/lib/Driver/ToolChains/PS4CPU.h
cfe/trunk/test/Driver/fsanitize.c
cfe/trunk/test/Driver/sanitizer-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=334096&r1=334095&r2=334096&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Jun  6 07:04:15 2018
@@ -3687,9 +3687,11 @@ void Clang::ConstructJob(Compilation &C,
   if (auto *ABICompatArg = Args.getLastArg(options::OPT_fclang_abi_compat_EQ))
 ABICompatArg->render(Args, CmdArgs);
 
-  // Add runtime flag for PS4 when PGO or Coverage are enabled.
-  if (RawTriple.isPS4CPU())
+  // Add runtime flag for PS4 when PGO, coverage, or sanitizers are enabled.
+  if (RawTriple.isPS4CPU()) {
 PS4cpu::addProfileRTArgs(getToolChain(), Args, CmdArgs);
+PS4cpu::addSanitizerArgs(getToolChain(), CmdArgs);
+  }
 
   // Pass options for controlling the default header search paths.
   if (Args.hasArg(options::OPT_nostdinc)) {

Modified: cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp?rev=334096&r1=334095&r2=334096&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/PS4CPU.cpp Wed Jun  6 07:04:15 2018
@@ -76,6 +76,15 @@ static void AddPS4SanitizerArgs(const To
   }
 }
 
+void tools::PS4cpu::addSanitizerArgs(const ToolChain &TC,
+ ArgStringList &CmdArgs) {
+  const SanitizerArgs &SanArgs = TC.getSanitizerArgs();
+  if (SanArgs.needsUbsanRt())
+CmdArgs.push_back("--dependent-lib=libSceDbgUBSanitizer_stub_weak.a");
+  if (SanArgs.needsAsanRt())
+CmdArgs.push_back("--dependent-lib=libSceDbgAddressSanitizer_stub_weak.a");
+}
+
 static void ConstructPS4LinkJob(const Tool &T, Compilation &C,
 const JobAction &JA, const InputInfo &Output,
 const InputInfoList &Inputs,

Modified: cfe/trunk/lib/Driver/ToolChains/PS4CPU.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/PS4CPU.h?rev=334096&r1=334095&r2=334096&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/PS4CPU.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/PS4CPU.h Wed Jun  6 07:04:15 2018
@@ -23,6 +23,8 @@ namespace PS4cpu {
 void addProfileRTArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
   llvm::opt::ArgStringList &CmdArgs);
 
+void addSanitizerArgs(const ToolChain &TC, llvm::opt::ArgStringList &CmdArgs);
+
 class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {
 public:
   Assemble(const ToolChain &TC)

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=334096&r1=334095&r2=334096&view=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Wed Jun  6 07:04:15 2018
@@ -621,6 +621,8 @@
 // RUN: %clang -target x86_64-scei-ps4 -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-ASAN-PS4
 // Make sure there are no *.{o,bc} or -l passed before the ASan library.
 // CHECK-ASAN-PS4-NOT: {{(\.(o|bc)"? |-l).*-lSceDbgAddressSanitizer_stub_weak}}
+// CHECK-ASAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
+// CHECK-ASAN-PS4-NOT: {{(\.(o|bc)"? |-l).*-lSceDbgAddressSanitizer_stub_weak}}
 // CHECK-ASAN-PS4: -lSceDbgAddressSanitizer_stub_weak
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-MINIMAL

Modified: cfe/trunk/test/Driver/sanitizer-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/sanitizer-ld.c?rev=334096&r1=334095&r2=334096&view=diff
==
--- cfe/trunk/test/Driver/sanitizer-ld.c (original)
+++ cfe/trunk/test/Driver/sanitizer-ld.c Wed Jun  6 07:04:15 2018
@@ -646,6 +646,7 @@
 // RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
 // RUN: -shared \
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-PS4 %s
+// CHECK-UBSAN-PS4: --dependent-lib=l

r280702 - [clang-cl] Check that we are in clang cl mode before enabling support for the CL environment variable.

2016-09-06 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Tue Sep  6 05:48:27 2016
New Revision: 280702

URL: http://llvm.org/viewvc/llvm-project?rev=280702&view=rev
Log:
[clang-cl] Check that we are in clang cl mode before enabling support for the 
CL environment variable.

Checking for the type of the command line tokenizer should not be the criteria 
to enable support for the CL environment variable, this change checks that we 
are in clang-cl mode instead.

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

Modified:
cfe/trunk/test/Driver/cl-options.c
cfe/trunk/tools/driver/driver.cpp

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=280702&r1=280701&r2=280702&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Tue Sep  6 05:48:27 2016
@@ -493,6 +493,8 @@
 // RUN: env CL="/Gy" _CL_="/Gy- -- %s" %clang_cl -### 2>&1 | FileCheck 
-check-prefix=ENV-_CL_ %s
 // ENV-_CL_-NOT: "-ffunction-sections"
 
+// RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \

Modified: cfe/trunk/tools/driver/driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/driver.cpp?rev=280702&r1=280701&r2=280702&view=diff
==
--- cfe/trunk/tools/driver/driver.cpp (original)
+++ cfe/trunk/tools/driver/driver.cpp Tue Sep  6 05:48:27 2016
@@ -393,7 +393,7 @@ int main(int argc_, const char **argv_)
 
   // Handle CL and _CL_ which permits additional command line options to be
   // prepended or appended.
-  if (Tokenizer == &llvm::cl::TokenizeWindowsCommandLine) {
+  if (ClangCLMode) {
 // Arguments in "CL" are prepended.
 llvm::Optional OptCL = llvm::sys::Process::GetEnv("CL");
 if (OptCL.hasValue()) {


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

Re: [PATCH] D23503: [clang-cl] Check that we are in clang cl mode before enabling support for the CL environment variable.

2016-09-06 Thread pierre gousseau via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL280702: [clang-cl] Check that we are in clang cl mode before 
enabling support for the… (authored by pgousseau).

Changed prior to commit:
  https://reviews.llvm.org/D23503?vs=68020&id=70376#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23503

Files:
  cfe/trunk/test/Driver/cl-options.c
  cfe/trunk/tools/driver/driver.cpp

Index: cfe/trunk/tools/driver/driver.cpp
===
--- cfe/trunk/tools/driver/driver.cpp
+++ cfe/trunk/tools/driver/driver.cpp
@@ -393,7 +393,7 @@
 
   // Handle CL and _CL_ which permits additional command line options to be
   // prepended or appended.
-  if (Tokenizer == &llvm::cl::TokenizeWindowsCommandLine) {
+  if (ClangCLMode) {
 // Arguments in "CL" are prepended.
 llvm::Optional OptCL = llvm::sys::Process::GetEnv("CL");
 if (OptCL.hasValue()) {
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -493,6 +493,8 @@
 // RUN: env CL="/Gy" _CL_="/Gy- -- %s" %clang_cl -### 2>&1 | FileCheck 
-check-prefix=ENV-_CL_ %s
 // ENV-_CL_-NOT: "-ffunction-sections"
 
+// RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \


Index: cfe/trunk/tools/driver/driver.cpp
===
--- cfe/trunk/tools/driver/driver.cpp
+++ cfe/trunk/tools/driver/driver.cpp
@@ -393,7 +393,7 @@
 
   // Handle CL and _CL_ which permits additional command line options to be
   // prepended or appended.
-  if (Tokenizer == &llvm::cl::TokenizeWindowsCommandLine) {
+  if (ClangCLMode) {
 // Arguments in "CL" are prepended.
 llvm::Optional OptCL = llvm::sys::Process::GetEnv("CL");
 if (OptCL.hasValue()) {
Index: cfe/trunk/test/Driver/cl-options.c
===
--- cfe/trunk/test/Driver/cl-options.c
+++ cfe/trunk/test/Driver/cl-options.c
@@ -493,6 +493,8 @@
 // RUN: env CL="/Gy" _CL_="/Gy- -- %s" %clang_cl -### 2>&1 | FileCheck -check-prefix=ENV-_CL_ %s
 // ENV-_CL_-NOT: "-ffunction-sections"
 
+// RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23503: [clang-cl] Check that we are in clang cl mode before enabling support for the CL environment variable.

2016-09-05 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping!


https://reviews.llvm.org/D23503



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

[PATCH] D23503: [clang-cl] Check that we are in clang cl mode before enabling support for the CL environment variable.

2016-08-15 Thread pierre gousseau via cfe-commits

pgousseau created this revision.
pgousseau added reviewers: rsmith, majnemer.
pgousseau added subscribers: cfe-commits, wristow, gbedwell, probinson.

Checking for the type of the command line tokenizer should not be the criteria 
to enable support for the CL environment variable, this change checks that we 
are in clang-cl mode instead.

https://reviews.llvm.org/D23503

Files:
  test/Driver/cl-options.c
  tools/driver/driver.cpp

Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -393,7 +393,7 @@
 
   // Handle CL and _CL_ which permits additional command line options to be
   // prepended or appended.
-  if (Tokenizer == &llvm::cl::TokenizeWindowsCommandLine) {
+  if (ClangCLMode) {
 // Arguments in "CL" are prepended.
 llvm::Optional OptCL = llvm::sys::Process::GetEnv("CL");
 if (OptCL.hasValue()) {
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -468,6 +468,8 @@
 // RUN: env CL="/Gy" _CL_="/Gy- -- %s" %clang_cl -### 2>&1 | FileCheck 
-check-prefix=ENV-_CL_ %s
 // ENV-_CL_-NOT: "-ffunction-sections"
 
+// RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -393,7 +393,7 @@
 
   // Handle CL and _CL_ which permits additional command line options to be
   // prepended or appended.
-  if (Tokenizer == &llvm::cl::TokenizeWindowsCommandLine) {
+  if (ClangCLMode) {
 // Arguments in "CL" are prepended.
 llvm::Optional OptCL = llvm::sys::Process::GetEnv("CL");
 if (OptCL.hasValue()) {
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -468,6 +468,8 @@
 // RUN: env CL="/Gy" _CL_="/Gy- -- %s" %clang_cl -### 2>&1 | FileCheck -check-prefix=ENV-_CL_ %s
 // ENV-_CL_-NOT: "-ffunction-sections"
 
+// RUN: env CL="%s" _CL_="%s" not %clang --rsp-quoting=windows -c
+
 // Accept "core" clang options.
 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22310: Make the test for fno-pch-timestamp compatible with read-only checkouts.

2016-07-14 Thread pierre gousseau via cfe-commits

pgousseau closed this revision.
pgousseau added a comment.

Committed in https://reviews.llvm.org/rL275415


https://reviews.llvm.org/D22310



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

Re: [PATCH] D22310: Make the test for fno-pch-timestamp compatible with read-only checkouts.

2016-07-14 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In https://reviews.llvm.org/D22310#484017, @bkramer wrote:

> lg


Thanks for the review!


https://reviews.llvm.org/D22310



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

r275415 - The test added in r275267 does not work on read-only checkouts because of the use of touch -m -t.

2016-07-14 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Thu Jul 14 08:58:27 2016
New Revision: 275415

URL: http://llvm.org/viewvc/llvm-project?rev=275415&view=rev
Log:
The test added in r275267 does not work on read-only checkouts because of the 
use of touch -m -t.
Following Tom Rybka suggestion, the test files are now copied to a temporary 
directory first.

Modified:
cfe/trunk/test/PCH/include-timestamp.cpp

Modified: cfe/trunk/test/PCH/include-timestamp.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/include-timestamp.cpp?rev=275415&r1=275414&r2=275415&view=diff
==
--- cfe/trunk/test/PCH/include-timestamp.cpp (original)
+++ cfe/trunk/test/PCH/include-timestamp.cpp Thu Jul 14 08:58:27 2016
@@ -1,23 +1,28 @@
 // Test that the timestamp is not included in the produced pch file with
 // -fno-pch-timestamp.
 
+// Copying files allow for read-only checkouts to run this test.
+// RUN: cp %S/Inputs/pragma-once2-pch.h %T
+// RUN: cp %S/Inputs/pragma-once2.h %T
+// RUN: cp %s %t1.cpp
+
 // Check timestamp is included by default.
-// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h
-// RUN: touch -m -a -t 201008011501 %S/Inputs/pragma-once2.h
-// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck 
-check-prefix=CHECK-TIMESTAMP %s
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %T/pragma-once2-pch.h
+// RUN: touch -m -a -t 201008011501 %T/pragma-once2.h
+// RUN: not %clang_cc1 -include-pch %t %t1.cpp 2>&1 | FileCheck 
-check-prefix=CHECK-TIMESTAMP %s
 
 // Check bitcode output as well.
 // RUN: llvm-bcanalyzer -dump %t | FileCheck 
-check-prefix=CHECK-BITCODE-TIMESTAMP-ON %s
 
 // Check timestamp inclusion is disabled by -fno-pch-timestamp.
-// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h 
-fno-pch-timestamp
-// RUN: touch -m -a -t 201008011502 %S/Inputs/pragma-once2.h
-// RUN: %clang_cc1 -include-pch %t %s 2>&1
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %T/pragma-once2-pch.h 
-fno-pch-timestamp
+// RUN: touch -m -a -t 201008011502 %T/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t %t1.cpp 2>&1
 
 // Check bitcode output as well.
 // RUN: llvm-bcanalyzer -dump %t | FileCheck 
-check-prefix=CHECK-BITCODE-TIMESTAMP-OFF %s
 
-#include "Inputs/pragma-once2.h"
+#include "pragma-once2.h"
 
 void g() { f(); }
 


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

[PATCH] D22310: Make the test for fno-pch-timestamp compatible with read-only checkouts.

2016-07-13 Thread pierre gousseau via cfe-commits

pgousseau created this revision.
pgousseau added a reviewer: rsmith.
pgousseau added subscribers: cfe-commits, trybka, bruno, gbedwell, wristow, 
probinson.

The test added in r275267 does not work on read-only checkouts because of the 
use of touch -t.
Following Tom Rybka suggestion, the test files are now copied to a temporary 
directory first.

http://reviews.llvm.org/D22310

Files:
  test/PCH/include-timestamp.cpp

Index: test/PCH/include-timestamp.cpp
===
--- test/PCH/include-timestamp.cpp
+++ test/PCH/include-timestamp.cpp
@@ -1,23 +1,28 @@
 // Test that the timestamp is not included in the produced pch file with
 // -fno-pch-timestamp.
 
+// Copying files allow for read-only checkouts to run this test.
+// RUN: cp %S/Inputs/pragma-once2-pch.h %T
+// RUN: cp %S/Inputs/pragma-once2.h %T
+// RUN: cp %s %t1.cpp
+
 // Check timestamp is included by default.
-// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h
-// RUN: touch -m -a -t 201008011501 %S/Inputs/pragma-once2.h
-// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck 
-check-prefix=CHECK-TIMESTAMP %s
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %T/pragma-once2-pch.h
+// RUN: touch -m -a -t 201008011501 %T/pragma-once2.h
+// RUN: not %clang_cc1 -include-pch %t %t1.cpp 2>&1 | FileCheck 
-check-prefix=CHECK-TIMESTAMP %s
 
 // Check bitcode output as well.
 // RUN: llvm-bcanalyzer -dump %t | FileCheck 
-check-prefix=CHECK-BITCODE-TIMESTAMP-ON %s
 
 // Check timestamp inclusion is disabled by -fno-pch-timestamp.
-// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h 
-fno-pch-timestamp
-// RUN: touch -m -a -t 201008011502 %S/Inputs/pragma-once2.h
-// RUN: %clang_cc1 -include-pch %t %s 2>&1
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %T/pragma-once2-pch.h 
-fno-pch-timestamp
+// RUN: touch -m -a -t 201008011502 %T/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t %t1.cpp 2>&1
 
 // Check bitcode output as well.
 // RUN: llvm-bcanalyzer -dump %t | FileCheck 
-check-prefix=CHECK-BITCODE-TIMESTAMP-OFF %s
 
-#include "Inputs/pragma-once2.h"
+#include "pragma-once2.h"
 
 void g() { f(); }
 


Index: test/PCH/include-timestamp.cpp
===
--- test/PCH/include-timestamp.cpp
+++ test/PCH/include-timestamp.cpp
@@ -1,23 +1,28 @@
 // Test that the timestamp is not included in the produced pch file with
 // -fno-pch-timestamp.
 
+// Copying files allow for read-only checkouts to run this test.
+// RUN: cp %S/Inputs/pragma-once2-pch.h %T
+// RUN: cp %S/Inputs/pragma-once2.h %T
+// RUN: cp %s %t1.cpp
+
 // Check timestamp is included by default.
-// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h
-// RUN: touch -m -a -t 201008011501 %S/Inputs/pragma-once2.h
-// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck -check-prefix=CHECK-TIMESTAMP %s
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %T/pragma-once2-pch.h
+// RUN: touch -m -a -t 201008011501 %T/pragma-once2.h
+// RUN: not %clang_cc1 -include-pch %t %t1.cpp 2>&1 | FileCheck -check-prefix=CHECK-TIMESTAMP %s
 
 // Check bitcode output as well.
 // RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-ON %s
 
 // Check timestamp inclusion is disabled by -fno-pch-timestamp.
-// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h -fno-pch-timestamp
-// RUN: touch -m -a -t 201008011502 %S/Inputs/pragma-once2.h
-// RUN: %clang_cc1 -include-pch %t %s 2>&1
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %T/pragma-once2-pch.h -fno-pch-timestamp
+// RUN: touch -m -a -t 201008011502 %T/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t %t1.cpp 2>&1
 
 // Check bitcode output as well.
 // RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-OFF %s
 
-#include "Inputs/pragma-once2.h"
+#include "pragma-once2.h"
 
 void g() { f(); }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-07-13 Thread pierre gousseau via cfe-commits

pgousseau abandoned this revision.
pgousseau added a comment.

Superseded by http://reviews.llvm.org/D20867


http://reviews.llvm.org/D20243



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

r275267 - [PCH] Add a fno-pch-timestamp option to cc1 to disable inclusion of timestamps in PCH files.

2016-07-13 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Wed Jul 13 09:21:11 2016
New Revision: 275267

URL: http://llvm.org/viewvc/llvm-project?rev=275267&view=rev
Log:
[PCH] Add a fno-pch-timestamp option to cc1 to disable inclusion of timestamps 
in PCH files.

This is to allow distributed build systems, that do not preserve time stamps, 
to use PCH files.

Second and last part of the patch proposed at:

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

Added:
cfe/trunk/test/PCH/Inputs/pragma-once2-pch.h
cfe/trunk/test/PCH/Inputs/pragma-once2.h
cfe/trunk/test/PCH/include-timestamp.cpp
Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Frontend/FrontendOptions.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/FrontendActions.cpp

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=275267&r1=275266&r2=275267&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Wed Jul 13 09:21:11 2016
@@ -509,6 +509,8 @@ def find_pch_source_EQ : Joined<["-"], "
   HelpText<"When building a pch, try to find the input file in include "
"directories, as if it had been included by the argument passed "
"to this flag.">;
+def fno_pch_timestamp : Flag<["-"], "fno-pch-timestamp">,
+  HelpText<"Disable inclusion of timestamp in precompiled headers">;
   
 
//===--===//
 // Language Options

Modified: cfe/trunk/include/clang/Frontend/FrontendOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendOptions.h?rev=275267&r1=275266&r2=275267&view=diff
==
--- cfe/trunk/include/clang/Frontend/FrontendOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/FrontendOptions.h Wed Jul 13 09:21:11 2016
@@ -154,6 +154,8 @@ public:
///< implicit module build.
   unsigned ModulesEmbedAllFiles : 1;   ///< Whether we should embed all 
used
///< files into the PCM file.
+  unsigned IncludeTimestamps : 1;  ///< Whether timestamps should be
+   ///< written to the produced PCH 
file.
 
   CodeCompleteOptions CodeCompleteOpts;
 
@@ -278,8 +280,8 @@ public:
 SkipFunctionBodies(false), UseGlobalModuleIndex(true),
 GenerateGlobalModuleIndex(true), ASTDumpDecls(false), 
ASTDumpLookups(false),
 BuildingImplicitModule(false), ModulesEmbedAllFiles(false),
-ARCMTAction(ARCMT_None), ObjCMTAction(ObjCMT_None),
-ProgramAction(frontend::ParseSyntaxOnly)
+IncludeTimestamps(true), ARCMTAction(ARCMT_None),
+ObjCMTAction(ObjCMT_None), ProgramAction(frontend::ParseSyntaxOnly)
   {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=275267&r1=275266&r2=275267&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed Jul 13 09:21:11 2016
@@ -1197,6 +1197,7 @@ static InputKind ParseFrontendArgs(Front
   Opts.ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
+  Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Args.hasArg(OPT_code_completion_macros);

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=275267&r1=275266&r2=275267&view=diff
==
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Wed Jul 13 09:21:11 2016
@@ -92,7 +92,10 @@ GeneratePCHAction::CreateASTConsumer(Com
   std::vector> Consumers;
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/
+  +CI.getFrontendOpts().IncludeTimestamps));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 

Added: cfe/tru

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-07-13 Thread pierre gousseau via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL275261: [PCH] Fix timestamp check on windows hosts. 
(authored by pgousseau).

Changed prior to commit:
  http://reviews.llvm.org/D20867?vs=59887&id=63794#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20867

Files:
  cfe/trunk/lib/Serialization/ASTReader.cpp

Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -2010,17 +2010,8 @@
   // For an overridden file, there is nothing to validate.
   if (!Overridden && //
   (StoredSize != File->getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).


Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -2010,17 +2010,8 @@
   // For an overridden file, there is nothing to validate.
   if (!Overridden && //
   (StoredSize != File->getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r275261 - [PCH] Fix timestamp check on windows hosts.

2016-07-13 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Wed Jul 13 06:58:28 2016
New Revision: 275261

URL: http://llvm.org/viewvc/llvm-project?rev=275261&view=rev
Log:
[PCH] Fix timestamp check on windows hosts.

On Linux, if the timestamp of a header file, included in the pch, is modified, 
then including the pch without regenerating it causes a fatal error, which is 
reasonable.
On Windows the check is ifdefed out, allowing the compilation to continue in a 
broken state.
The root of the broken state is that, if timestamps dont match, the 
preprocessor will reparse a header without discarding the pch data.
This leads to "#pragma once" header to be included twice.
The reason behind the ifdefing of the check lacks documentation, and was done 6 
years ago.
This change tentatively removes the ifdefing.

First part of patch proposed at:

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

Modified:
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=275261&r1=275260&r2=275261&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Jul 13 06:58:28 2016
@@ -2010,17 +2010,8 @@ InputFile ASTReader::getInputFile(Module
   // For an overridden file, there is nothing to validate.
   if (!Overridden && //
   (StoredSize != File->getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).


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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-07-13 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D20867#482465, @rsmith wrote:

> You have two independent functional changes in this patch: one adds a flag to 
> control the emission of timestamps into PCH files, and the other re-enables 
> timestamp checking on Win32. Please separate them out into distinct patches 
> to be committed separately.
>
> Both parts of this LGTM.


Splitting the patch sounds good, thanks Richard and Bruno for reviewing, will  
push soon.


http://reviews.llvm.org/D20867



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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-07-08 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D20867



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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-29 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D20867



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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-22 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D20867



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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-15 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D20867



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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-08 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

> > Are you asking for the 2 seconds tolerance to be part of this patch?

> 

> >  Or, as it seems the main problem here is the lack of a reproducible, are 
> > you ok with the idea of committing this patch first, to see if the 
> > inconsistent time stamps is still an issue?

> 

> 

> Yes, let's commit a patch to remove the #if first, and see if anything 
> actually breaks (and if so, what). There's no need to add an option for this 
> we have no uses for it.


Sounds good, does it mean it is ok to commit?
Sorry, regarding the option, not sure if you are saying: "if we have no uses 
for it" or "as we have no uses for it"?
The idea behind the option is to allow distributed builds system, that do not 
preserve time stamps, to use PCH files.


http://reviews.llvm.org/D20867



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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-07 Thread Pierre Gousseau via cfe-commits

It seems Phabricator did not picked up Richard's message so I have manually
copied and replied to it via Phabricator's web interface.

Cheers,

Pierre

On 6 June 2016 at 21:53, Richard Smith  wrote:

> On Wed, Jun 1, 2016 at 8:33 AM, pierre gousseau via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> pgousseau created this revision.
>> pgousseau added reviewers: rsmith, thakis.
>> pgousseau added subscribers: cfe-commits, wristow, probinson, gbedwell,
>> bruno, cameron314.
>>
>> On Linux, if the timestamp of a header file, included in the pch, is
>> modified, then including the pch without regenerating it causes a fatal
>> error, which is reasonable.
>> On Windows the check is ifdefed out, allowing the compilation to continue
>> in a broken state.
>> The root of the broken state is that, if timestamps dont match, the
>> preprocessor will reparse a header without discarding the pch data.
>> This leads to "#pragma once" header to be included twice.
>> The reason behind the ifdefing of the check lacks documentation, and was
>> done 6 years ago.
>>
>
> It's documented in the comment you deleted:
>
>// In our regression testing, the Windows file system seems to
>// have inconsistent modification times that sometimes
>// erroneously trigger this error-handling path.
>
> We should confirm whether this is in fact still the case. Maybe this is
> caused by building on a networked file system, where a locally-changed file
> might have a different mtime locally and remotely (the local mtime may be
> precise where the remote one has been rounded to a multiple of 2 seconds by
> an underlying FAT filesystem)? If it's something like that, we could
> perhaps work around this by rounding the mtime to a multiple of 2 seconds
> ourselves.
>
>
>> This change tentatively removes the ifdefing and adds a cc1 option to
>> disable the inclusion of timestamps in pch files, giving some flexibility
>> to build systems such as distributed builds.
>>
>> This change is a follow up to the discussion started in
>> http://reviews.llvm.org/D20243
>>
>> http://reviews.llvm.org/D20867
>>
>> Files:
>>   include/clang/Driver/CC1Options.td
>>   include/clang/Frontend/FrontendOptions.h
>>   lib/Frontend/CompilerInvocation.cpp
>>   lib/Frontend/FrontendActions.cpp
>>   lib/Serialization/ASTReader.cpp
>>   test/PCH/Inputs/include-timestamp-pch.h
>>   test/PCH/Inputs/include-timestamp.h
>>   test/PCH/include-timestamp.cpp
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-07 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

> > On Linux, if the timestamp of a header file, included in the pch, is 
> > modified, then including the pch without regenerating it causes a fatal 
> > error, which is reasonable.

> 

> >  On Windows the check is ifdefed out, allowing the compilation to continue 
> > in a broken state.

> 

> >  The root of the broken state is that, if timestamps dont match, the 
> > preprocessor will reparse a header without discarding the pch data.

> 

> >  This leads to "#pragma once" header to be included twice.

> 

> >  The reason behind the ifdefing of the check lacks documentation, and was 
> > done 6 years ago.

> 




> It's documented in the comment you deleted:

> 

>   // In our regression testing, the Windows file system seems to

>   // have inconsistent modification times that sometimes

>   // erroneously trigger this error-handling path.

>

> 

> We should confirm whether this is in fact still the case. Maybe this is 
> caused by building on a networked file system, where a locally-changed file 
> might have a different mtime locally and remotely (the local mtime may be 
> precise where the remote one has been rounded to a multiple of 2 seconds by 
> an underlying FAT filesystem)? If it's something like that, we could perhaps 
> work around this by rounding the mtime to a multiple of 2 seconds ourselves.


I am not sure how to reproduce this build scenario, would you be able to 
provide some more stepped details?
I have tried emitting and including a PCH on a networked FAT32 drive, but no 
false warnings observed so far.

Are you asking for the 2 seconds tolerance to be part of this patch?
Or, as it seems the main problem here is the lack of a reproducible, are you ok 
with the idea of committing this patch first, to see if the inconsistent time 
stamps is still an issue?


http://reviews.llvm.org/D20867



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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-07 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 59887.
pgousseau added a comment.

Following Bruno's comment:

- Remove call to sleep using touch -m -a -t


http://reviews.llvm.org/D20867

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/FrontendOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Serialization/ASTReader.cpp
  test/PCH/Inputs/pragma-once2-pch.h
  test/PCH/Inputs/pragma-once2.h
  test/PCH/include-timestamp.cpp

Index: test/PCH/include-timestamp.cpp
===
--- /dev/null
+++ test/PCH/include-timestamp.cpp
@@ -0,0 +1,27 @@
+// Test that the timestamp is not included in the produced pch file with
+// -fno-pch-timestamp.
+
+// Check timestamp is included by default.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h
+// RUN: touch -m -a -t 201008011501 %S/Inputs/pragma-once2.h
+// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck -check-prefix=CHECK-TIMESTAMP %s
+
+// Check bitcode output as well.
+// RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-ON %s
+
+// Check timestamp inclusion is disabled by -fno-pch-timestamp.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h -fno-pch-timestamp
+// RUN: touch -m -a -t 201008011502 %S/Inputs/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t %s 2>&1
+
+// Check bitcode output as well.
+// RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-OFF %s
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
+
+// CHECK-BITCODE-TIMESTAMP-ON: getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -92,7 +92,10 @@
   std::vector> Consumers;
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/
+  +CI.getFrontendOpts().IncludeTimestamps));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1144,6 +1144,7 @@
   Opts.ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
+  Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Args.hasArg(OPT_code_completion_macros);
Index: include/clang/Frontend/FrontendOptions.h
===
--- include/clang/Frontend/FrontendOptions.h
+++ include/clang/Frontend/FrontendOptions.h
@@ -153,6 +153,8 @@
///< implicit module build.
   unsigned ModulesEmbedAllFiles : 1;   ///< Whether we should embed all used
///< files into the PCM file.
+  unsigned IncludeTimestamps : 1;  ///< Whether timestamps should be
+   ///< written to the produced PCH file.
 
   CodeCompleteOptions CodeCompleteOpts;
 
@@ -277,8 +279,8 @@
 SkipFunctionBodies(false), UseGlobalModuleIndex(true),
 GenerateGlobalModuleIndex(true), ASTDumpDecls(false), ASTDumpLookups(false),
 BuildingImplicitModule(false), ModulesEmbedAllFiles(false),
-ARCMTAction(ARCMT_None), ObjCMTAction(ObjCMT_None),
-ProgramAction(frontend::ParseSyntaxOnly)
+IncludeTimestamps(true), ARCMTAction(ARCMT_None),
+ObjCMTAction(ObjCMT_None), ProgramAction(frontend::ParseSyntaxOnly)
   {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -507,6 +507,8 @@
   HelpT

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-02 Thread pierre gousseau via cfe-commits

pgousseau added inline comments.


Comment at: test/PCH/include-timestamp.cpp:8
@@ +7,3 @@
+// RUN: sleep 1
+// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck 
-check-prefix=CHECK-TIMESTAMP %s
+

Without the sleep the test fails for me, as it seems the call to touch does not 
have time to take effect. It might be something specific to gnuwin32 tools on 
Windows I am not sure.


http://reviews.llvm.org/D20867



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

Re: [PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-02 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 59355.
pgousseau added a comment.

Following Bruno's comments:

- Update input files' name to match http://reviews.llvm.org/D20243 review.
- Add llvm-bcanalyzer checks


http://reviews.llvm.org/D20867

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/FrontendOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Serialization/ASTReader.cpp
  test/PCH/Inputs/pragma-once2-pch.h
  test/PCH/Inputs/pragma-once2.h
  test/PCH/include-timestamp.cpp

Index: test/PCH/include-timestamp.cpp
===
--- /dev/null
+++ test/PCH/include-timestamp.cpp
@@ -0,0 +1,29 @@
+// Test that the timestamp is not included in the produced pch file with
+// -fno-pch-timestamp.
+
+// Check timestamp is included by default.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: sleep 1
+// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck -check-prefix=CHECK-TIMESTAMP %s
+
+// Check bitcode output as well.
+// RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-ON %s
+
+// Check timestamp inclusion is disabled by -fno-pch-timestamp.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/pragma-once2-pch.h -fno-pch-timestamp
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: sleep 1
+// RUN: %clang_cc1 -include-pch %t %s 2>&1
+
+// Check bitcode output as well.
+// RUN: llvm-bcanalyzer -dump %t | FileCheck -check-prefix=CHECK-BITCODE-TIMESTAMP-OFF %s
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
+
+// CHECK-BITCODE-TIMESTAMP-ON: getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -92,7 +92,10 @@
   std::vector> Consumers;
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/
+  +CI.getFrontendOpts().IncludeTimestamps));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1144,6 +1144,7 @@
   Opts.ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
+  Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Args.hasArg(OPT_code_completion_macros);
Index: include/clang/Frontend/FrontendOptions.h
===
--- include/clang/Frontend/FrontendOptions.h
+++ include/clang/Frontend/FrontendOptions.h
@@ -153,6 +153,8 @@
///< implicit module build.
   unsigned ModulesEmbedAllFiles : 1;   ///< Whether we should embed all used
///< files into the PCM file.
+  unsigned IncludeTimestamps : 1;  ///< Whether timestamps should be
+   ///< written to the produced PCH file.
 
   CodeCompleteOptions CodeCompleteOpts;
 
@@ -277,8 +279,8 @@
 SkipFunctionBodies(false), UseGlobalModuleIndex(true),
 GenerateGlobalModuleIndex(true), ASTDumpDecls(false), ASTDumpLookups(false),
 BuildingImplicitModule(false), ModulesEmbedAllFiles(false),
-ARCMTAction(ARCMT_None), ObjCMTAction(ObjCMT_None),
-ProgramAction(frontend::ParseSyntaxOnly)
+IncludeTimestamps(true), ARCMTAction(ARCMT_None),
+ObjCMTAction(ObjCMT_None), ProgramAction(frontend::ParseSyntaxOnly)
   {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
Index: include/clang/Driver/CC1Options.td
===
--- include/clang/Driver/CC1Options.td
+++ include/clan

[PATCH] D20867: [PCH] Fix timestamp check on windows hosts.

2016-06-01 Thread pierre gousseau via cfe-commits

pgousseau created this revision.
pgousseau added reviewers: rsmith, thakis.
pgousseau added subscribers: cfe-commits, wristow, probinson, gbedwell, bruno, 
cameron314.

On Linux, if the timestamp of a header file, included in the pch, is modified, 
then including the pch without regenerating it causes a fatal error, which is 
reasonable.
On Windows the check is ifdefed out, allowing the compilation to continue in a 
broken state.
The root of the broken state is that, if timestamps dont match, the 
preprocessor will reparse a header without discarding the pch data.
This leads to "#pragma once" header to be included twice.
The reason behind the ifdefing of the check lacks documentation, and was done 6 
years ago.
This change tentatively removes the ifdefing and adds a cc1 option to disable 
the inclusion of timestamps in pch files, giving some flexibility to build 
systems such as distributed builds.

This change is a follow up to the discussion started in 
http://reviews.llvm.org/D20243

http://reviews.llvm.org/D20867

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/FrontendOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/Serialization/ASTReader.cpp
  test/PCH/Inputs/include-timestamp-pch.h
  test/PCH/Inputs/include-timestamp.h
  test/PCH/include-timestamp.cpp

Index: test/PCH/include-timestamp.cpp
===
--- /dev/null
+++ test/PCH/include-timestamp.cpp
@@ -0,0 +1,18 @@
+// Test that the timestamp is not included in the produced pch file with
+// -fno-pch-timestamp.
+
+// Check timestamp is included by default.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/include-timestamp-pch.h
+// RUN: touch %S/Inputs/include-timestamp.h
+// RUN: sleep 1
+// RUN: not %clang_cc1 -include-pch %t %s 2>&1 | FileCheck -check-prefix=CHECK-TIMESTAMP %s
+
+// Check timestamp inclusion is disabled by -fno-pch-timestamp.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/Inputs/include-timestamp-pch.h -fno-pch-timestamp 
+// RUN: touch %S/Inputs/include-timestamp.h
+// RUN: sleep 1
+// RUN: %clang_cc1 -include-pch %t %s 2>&1
+
+#include "Inputs/include-timestamp.h"
+
+// CHECK-TIMESTAMP: fatal error: file {{.*}} has been modified since the precompiled header {{.*}} was built
Index: test/PCH/Inputs/include-timestamp.h
===
--- /dev/null
+++ test/PCH/Inputs/include-timestamp.h
@@ -0,0 +1,3 @@
+#pragma once
+
+inline void f() {}
Index: test/PCH/Inputs/include-timestamp-pch.h
===
--- /dev/null
+++ test/PCH/Inputs/include-timestamp-pch.h
@@ -0,0 +1,4 @@
+#include "include-timestamp.h"
+
+void g() { f(); }
+
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2010,17 +2010,8 @@
   // For an overridden file, there is nothing to validate.
   if (!Overridden && //
   (StoredSize != File->getSize() ||
-#if defined(LLVM_ON_WIN32)
-   false
-#else
-   // In our regression testing, the Windows file system seems to
-   // have inconsistent modification times that sometimes
-   // erroneously trigger this error-handling path.
-   //
-   // FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
 !DisableValidation)
-#endif
)) {
 if (Complain) {
   // Build a list of the PCH imports that got us here (in reverse).
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -92,7 +92,10 @@
   std::vector> Consumers;
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/
+  +CI.getFrontendOpts().IncludeTimestamps));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1144,6 +1144,7 @@
   Opts.ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
+  Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Arg

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-24 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D20243#437192, @thakis wrote:

> Hm, the ASTReader code this works around is over 6 years old (r100866). Maybe 
> we could try enabling the access time check instead?

Yes the issue described in ASTReader lacks a reproducible so it makes sense to 
tentatively re-enabling the timestamp check on windows. If the issue still 
occurs on some bots then at least we can document the issue better before 
re-disabling it.

I also would like to provide a switch to disable inclusion of timestamps. I 
will hopefully be proposing a patch for it soon.

http://reviews.llvm.org/D20243

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

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-24 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 58207.
pgousseau added a comment.

Moving REQUIRE line higher following Bruno's comments.


http://reviews.llvm.org/D20243

Files:
  lib/Frontend/FrontendActions.cpp
  lib/Serialization/ASTReader.cpp
  test/PCH/Inputs/pragma-once2-pch.h
  test/PCH/Inputs/pragma-once2.h
  test/PCH/pragma-once-timestamp.cpp

Index: test/PCH/pragma-once-timestamp.cpp
===
--- /dev/null
+++ test/PCH/pragma-once-timestamp.cpp
@@ -0,0 +1,24 @@
+// On Windows, timestamps for pch are not handled correctly.
+// This would cause pragma once to be ignored on distributed builds.
+// pragma-once2-pch.h includes pragma-once2.h which has a pragma once 
directive.
+// pragma-once2.h is then touched before using the generated pch.
+// On Linux this will cause an expected error, but on Windows we want to
+// ignore the timestamp as the timestamp handling on Windows is
+// inconsistent at the moment.
+
+// REQUIRES: system-windows
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once2-pch.h -fsyntax-only -verify 
%s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t -x c++-header %S/Inputs/pragma-once2-pch.h
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
+
Index: test/PCH/Inputs/pragma-once2.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2.h
@@ -0,0 +1,4 @@
+#pragma once
+
+inline void f() {}
+
Index: test/PCH/Inputs/pragma-once2-pch.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2-pch.h
@@ -0,0 +1,2 @@
+#include "pragma-once2.h"
+
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2013,6 +2013,8 @@
// In our regression testing, the Windows file system seems to
// have inconsistent modification times that sometimes
// erroneously trigger this error-handling path.
+   // For now timestamps are disabled for pch files on Windows (c.f
+   // GeneratePCHAction::CreateASTConsumer).
//
// FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -90,9 +90,15 @@
 
   auto Buffer = std::make_shared();
   std::vector> Consumers;
+  // FIXME: There is a known issue with timestamps appearing to be inconsistent
+  // on Windows (c.f. ASTReader::getInputFile) so we disable timestamps checks
+  // on Windows for now.
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/!HostTriple.isOSWindows()));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 


Index: test/PCH/pragma-once-timestamp.cpp
===
--- /dev/null
+++ test/PCH/pragma-once-timestamp.cpp
@@ -0,0 +1,24 @@
+// On Windows, timestamps for pch are not handled correctly.
+// This would cause pragma once to be ignored on distributed builds.
+// pragma-once2-pch.h includes pragma-once2.h which has a pragma once directive.
+// pragma-once2.h is then touched before using the generated pch.
+// On Linux this will cause an expected error, but on Windows we want to
+// ignore the timestamp as the timestamp handling on Windows is
+// inconsistent at the moment.
+
+// REQUIRES: system-windows
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once2-pch.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t -x c++-header %S/Inputs/pragma-once2-pch.h
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
+
Index: test/PCH/Inputs/pragma-once2.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2.h
@@ -0,0 +1,4 @@
+#pragma once
+
+inline void f() {}
+
Index: test/PCH/Inputs/pragma-once2-pch.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2-pch.h
@@ -0,0 +1,2 @@
+#include "

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-19 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D20243#433615, @thakis wrote:

> Did you see http://reviews.llvm.org/D19815 ? Does that help? Warren might 
> have opinions on this.

Yes thanks, I agree with Warren, this is a separate issue.
In the test I am adding I avoid the issue that Warren's review is for by only 
using '#pragma once' in a sub header of the PCH.

http://reviews.llvm.org/D20243

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

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-16 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 57324.
pgousseau added a comment.

Add newlines and change test's name following Paul's comments.


http://reviews.llvm.org/D20243

Files:
  lib/Frontend/FrontendActions.cpp
  lib/Serialization/ASTReader.cpp
  test/PCH/Inputs/pragma-once2-pch.h
  test/PCH/Inputs/pragma-once2.h
  test/PCH/pragma-once-timestamp.cpp

Index: test/PCH/pragma-once-timestamp.cpp
===
--- /dev/null
+++ test/PCH/pragma-once-timestamp.cpp
@@ -0,0 +1,24 @@
+// On Windows, timestamps for pch are not handled correctly.
+// This would cause pragma once to be ignored on distributed builds.
+// pragma-once2-pch.h includes pragma-once2.h which has a pragma once 
directive.
+// pragma-once2.h is then touched before using the generated pch.
+// On Linux this will cause an expected error, but on Windows we want to
+// ignore the timestamp as the timestamp handling on Windows is
+// inconsistent at the moment.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once2-pch.h -fsyntax-only -verify 
%s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t -x c++-header %S/Inputs/pragma-once2-pch.h
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// REQUIRES: system-windows
+
+// expected-no-diagnostics
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
+
Index: test/PCH/Inputs/pragma-once2.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2.h
@@ -0,0 +1,4 @@
+#pragma once
+
+inline void f() {}
+
Index: test/PCH/Inputs/pragma-once2-pch.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2-pch.h
@@ -0,0 +1,2 @@
+#include "pragma-once2.h"
+
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2013,6 +2013,8 @@
// In our regression testing, the Windows file system seems to
// have inconsistent modification times that sometimes
// erroneously trigger this error-handling path.
+   // For now timestamps are disabled for pch files on Windows (c.f
+   // GeneratePCHAction::CreateASTConsumer).
//
// FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -90,9 +90,15 @@
 
   auto Buffer = std::make_shared();
   std::vector> Consumers;
+  // FIXME: There is a known issue with timestamps appearing to be inconsistent
+  // on Windows (c.f. ASTReader::getInputFile) so we disable timestamps checks
+  // on Windows for now.
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/!HostTriple.isOSWindows()));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 


Index: test/PCH/pragma-once-timestamp.cpp
===
--- /dev/null
+++ test/PCH/pragma-once-timestamp.cpp
@@ -0,0 +1,24 @@
+// On Windows, timestamps for pch are not handled correctly.
+// This would cause pragma once to be ignored on distributed builds.
+// pragma-once2-pch.h includes pragma-once2.h which has a pragma once directive.
+// pragma-once2.h is then touched before using the generated pch.
+// On Linux this will cause an expected error, but on Windows we want to
+// ignore the timestamp as the timestamp handling on Windows is
+// inconsistent at the moment.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once2-pch.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t -x c++-header %S/Inputs/pragma-once2-pch.h
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// REQUIRES: system-windows
+
+// expected-no-diagnostics
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
+
Index: test/PCH/Inputs/pragma-once2.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2.h
@@ -0,0 +1,4 @@
+#pragma once
+
+inline void f() {}
+
Index: test/PCH/Inputs/pragma-once2-pch.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2-pch.h
@@ -0,0 +1,2 @@
+#i

Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-16 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D20243#429811, @probinson wrote:

> Please make sure the test files have newlines at the end.
>  "touch-pragma-once.cpp" should probably be named something like 
> "pragma-once-timestamp.cpp".


Will fix thanks!


http://reviews.llvm.org/D20243



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

[PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.

2016-05-13 Thread pierre gousseau via cfe-commits

pgousseau created this revision.
pgousseau added a reviewer: rsmith.
pgousseau added a subscriber: cfe-commits.

On Linux, if a header file included in the pch is modified then a fatal error 
is emitted, which is reasonable.
On Windows the check is ifdefed out, allowing the compilation to continue in a 
broken state.
This leads to "#pragma once" directives to be ignored.
This change disables the inclusion of timestamps in pch files on Windows, for 
now.

http://reviews.llvm.org/D20243

Files:
  lib/Frontend/FrontendActions.cpp
  lib/Serialization/ASTReader.cpp
  test/PCH/Inputs/pragma-once2-pch.h
  test/PCH/Inputs/pragma-once2.h
  test/PCH/touch-pragma-once.cpp

Index: test/PCH/touch-pragma-once.cpp
===
--- /dev/null
+++ test/PCH/touch-pragma-once.cpp
@@ -0,0 +1,23 @@
+// On Windows, timestamps for pch are not handled correctly.
+// This would cause pragma once to be ignored on distributed builds.
+// pragma-once2-pch.h includes pragma-once2.h which has a pragma once 
directive.
+// pragma-once2.h is then touched before using the generated pch.
+// On Linux this will cause an expected error, but on Windows we want to
+// ignore the timestamp as the timestamp handling on Windows is
+// inconsistent at the moment.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once2-pch.h -fsyntax-only -verify 
%s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t -x c++-header %S/Inputs/pragma-once2-pch.h
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// REQUIRES: system-windows
+
+// expected-no-diagnostics
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
\ No newline at end of file
Index: test/PCH/Inputs/pragma-once2.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2.h
@@ -0,0 +1,3 @@
+#pragma once
+
+inline void f() {}
\ No newline at end of file
Index: test/PCH/Inputs/pragma-once2-pch.h
===
--- /dev/null
+++ test/PCH/Inputs/pragma-once2-pch.h
@@ -0,0 +1 @@
+#include "pragma-once2.h"
\ No newline at end of file
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2013,6 +2013,8 @@
// In our regression testing, the Windows file system seems to
// have inconsistent modification times that sometimes
// erroneously trigger this error-handling path.
+   // For now timestamps are disabled for pch files on Windows (c.f
+   // GeneratePCHAction::CreateASTConsumer).
//
// FIXME: This probably also breaks HeaderFileInfo lookups on Windows.
(StoredTime && StoredTime != File->getModificationTime() &&
Index: lib/Frontend/FrontendActions.cpp
===
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -90,9 +90,15 @@
 
   auto Buffer = std::make_shared();
   std::vector> Consumers;
+  // FIXME: There is a known issue with timestamps appearing to be inconsistent
+  // on Windows (c.f. ASTReader::getInputFile) so we disable timestamps checks
+  // on Windows for now.
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
   Consumers.push_back(llvm::make_unique(
 CI.getPreprocessor(), OutputFile, nullptr, Sysroot,
-Buffer, CI.getFrontendOpts().ModuleFileExtensions));
+Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+/*AllowASTWithErrors*/false,
+/*IncludeTimestamps*/!HostTriple.isOSWindows()));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
   CI, InFile, OutputFile, OS, Buffer));
 


Index: test/PCH/touch-pragma-once.cpp
===
--- /dev/null
+++ test/PCH/touch-pragma-once.cpp
@@ -0,0 +1,23 @@
+// On Windows, timestamps for pch are not handled correctly.
+// This would cause pragma once to be ignored on distributed builds.
+// pragma-once2-pch.h includes pragma-once2.h which has a pragma once directive.
+// pragma-once2.h is then touched before using the generated pch.
+// On Linux this will cause an expected error, but on Windows we want to
+// ignore the timestamp as the timestamp handling on Windows is
+// inconsistent at the moment.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %S/Inputs/pragma-once2-pch.h -fsyntax-only -verify %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t -x c++-header %S/Inputs/pragma-once2-pch.h
+// RUN: touch %S/Inputs/pragma-once2.h
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s
+
+// REQUIRES: system-windows
+
+// expected-no-diagnostics
+
+#include "Inputs/pragma-once2.h"
+
+void g() { f(); }
\ No newline at en

Re: [PATCH] D16178: [analyzer] A quick fix on top of D12901/r257464

2016-01-14 Thread pierre gousseau via cfe-commits

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

LGTM!
Thanks Artom for finding the bug and new test case, sorry for missing those in 
my patch!
Removing "!IsNotTruncated" is definitely worth trying.
Please commit if code owners are happy with it too.

Pierre.


http://reviews.llvm.org/D16178



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

r257467 - [analyzer] Fix RangeConstraintManager's pinning of single value ranges.

2016-01-12 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Tue Jan 12 04:40:45 2016
New Revision: 257467

URL: http://llvm.org/viewvc/llvm-project?rev=257467&view=rev
Log:
[analyzer] Fix RangeConstraintManager's pinning of single value ranges.

This fix a bug in RangeSet::pin causing single value ranges to be considered 
non conventionally ordered.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
cfe/trunk/test/Analysis/range_casts.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp?rev=257467&r1=257466&r2=257467&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp Tue Jan 12 
04:40:45 2016
@@ -171,7 +171,7 @@ private:
   case APSIntType::RTR_Below:
 // The entire range is outside the symbol's set of possible values.
 // If this is a conventionally-ordered range, the state is infeasible.
-if (Lower < Upper)
+if (Lower <= Upper)
   return false;
 
 // However, if the range wraps around, it spans all possible values.
@@ -222,7 +222,7 @@ private:
   case APSIntType::RTR_Above:
 // The entire range is outside the symbol's set of possible values.
 // If this is a conventionally-ordered range, the state is infeasible.
-if (Lower < Upper)
+if (Lower <= Upper)
   return false;
 
 // However, if the range wraps around, it spans all possible values.

Modified: cfe/trunk/test/Analysis/range_casts.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/range_casts.c?rev=257467&r1=257466&r2=257467&view=diff
==
--- cfe/trunk/test/Analysis/range_casts.c (original)
+++ cfe/trunk/test/Analysis/range_casts.c Tue Jan 12 04:40:45 2016
@@ -73,6 +73,16 @@ void f7(long foo)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
 
+void f8(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
 void f9(long foo)
 {
   unsigned index = -1;
@@ -93,6 +103,16 @@ void f10(long foo)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
 
+void f11(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
 void f12(long foo)
 {
   unsigned index = -1;
@@ -102,6 +122,16 @@ void f12(long foo)
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
+
+void f13(int foo)
+{
+  unsigned short index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
 
 void f14(long foo)
 {


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

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

2016-01-12 Thread pierre gousseau via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL257464: [analyzer] Evaluate integral casts as cast symbols 
if truncations are detected. (authored by pgousseau).

Changed prior to commit:
  http://reviews.llvm.org/D12901?vs=43120&id=44613#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D12901

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
  cfe/trunk/test/Analysis/range_casts.c

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -83,7 +83,11 @@
   }
 
   SVal evalCast(SVal val, QualType castTy, QualType originalType);
-  
+
+  // Handles casts of type CK_IntegralCast.
+  SVal evalIntegralCast(ProgramStateRef state, SVal val, QualType castTy,
+QualType originalType);
+
   virtual SVal evalMinus(NonLoc val) = 0;
 
   virtual SVal evalComplement(NonLoc val) = 0;
Index: cfe/trunk/test/Analysis/range_casts.c
===
--- cfe/trunk/test/Analysis/range_casts.c
+++ cfe/trunk/test/Analysis/range_casts.c
@@ -0,0 +1,126 @@
+// This test checks that intersecting ranges does not cause 'system is over constrained' assertions in the case of eg: 32 bits unsigned integers getting their range from 64 bits signed integers.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store=region -verify %s
+
+void clang_analyzer_warnIfReached();
+
+void f1(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0) // because of foo range, index is in range [0; UINT_MAX]
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f2(unsigned long foo)
+{
+  int index = -1;
+  if (index < foo) index = foo; // index equals ULONG_MAX
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // no-warning
+}
+
+void f3(unsigned long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f4(long foo)
+{
+  int index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f5(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index == -1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f6(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index == -1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f7(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1 == 0) // Was not reached prior fix.
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f9(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1L == 0L) // Was not reached prior fix.
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f10(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0L)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f12(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1UL == 0L) // Was not reached prior fix.
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f14(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  long bar = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f15(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  unsigned int tmp = index + 1;
+  if (tmp == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{R

r257464 - [analyzer] Evaluate integral casts as cast symbols if truncations are detected.

2016-01-12 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Tue Jan 12 04:07:56 2016
New Revision: 257464

URL: http://llvm.org/viewvc/llvm-project?rev=257464&view=rev
Log:
[analyzer] Evaluate integral casts as cast symbols if truncations are detected.

The current workaround for truncations not being modelled is that the 
evaluation of integer to integer casts are simply bypassed and so the original 
symbol is used as the new casted symbol (cf 
SimpleSValBuilder::evalCastFromNonLoc).
This lead to the issue described in PR25078, as the RangeConstraintManager 
associates ranges with symbols.

The new evalIntegralCast method added by this patch wont bypass the cast if it 
finds the range of the symbol to be greater than the maximum value of the 
target type.

The fix to RangeSet::pin mentioned in the initial review will be committed 
separately.

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

Added:
cfe/trunk/test/Analysis/range_casts.c
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h?rev=257464&r1=257463&r2=257464&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h Tue 
Jan 12 04:07:56 2016
@@ -83,7 +83,11 @@ public:
   }
 
   SVal evalCast(SVal val, QualType castTy, QualType originalType);
-  
+
+  // Handles casts of type CK_IntegralCast.
+  SVal evalIntegralCast(ProgramStateRef state, SVal val, QualType castTy,
+QualType originalType);
+
   virtual SVal evalMinus(NonLoc val) = 0;
 
   virtual SVal evalComplement(NonLoc val) = 0;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=257464&r1=257463&r2=257464&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Tue Jan 12 04:07:56 2016
@@ -316,7 +316,6 @@ void ExprEngine::VisitCast(const CastExp
   case CK_ArrayToPointerDecay:
   case CK_BitCast:
   case CK_AddressSpaceConversion:
-  case CK_IntegralCast:
   case CK_NullToPointer:
   case CK_IntegralToPointer:
   case CK_PointerToIntegral:
@@ -348,6 +347,14 @@ void ExprEngine::VisitCast(const CastExp
 state = state->BindExpr(CastE, LCtx, V);
 Bldr.generateNode(CastE, Pred, state);
 continue;
+  }
+  case CK_IntegralCast: {
+// Delegate to SValBuilder to process.
+SVal V = state->getSVal(Ex, LCtx);
+V = svalBuilder.evalIntegralCast(state, V, T, ExTy);
+state = state->BindExpr(CastE, LCtx, V);
+Bldr.generateNode(CastE, Pred, state);
+continue;
   }
   case CK_DerivedToBase:
   case CK_UncheckedDerivedToBase: {

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=257464&r1=257463&r2=257464&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Tue Jan 12 04:07:56 2016
@@ -423,6 +423,45 @@ static bool shouldBeModeledWithNoOp(ASTC
   return true;
 }
 
+// Handles casts of type CK_IntegralCast.
+// At the moment, this function will redirect to evalCast, except when the 
range
+// of the original value is known to be greater than the max of the target 
type.
+SVal SValBuilder::evalIntegralCast(ProgramStateRef state, SVal val,
+   QualType castTy, QualType originalTy) {
+
+  // No truncations if target type is big enough.
+  if (getContext().getTypeSize(castTy) >= getContext().getTypeSize(originalTy))
+return evalCast(val, castTy, originalTy);
+
+  const SymExpr *se = val.getAsSymbolicExpression();
+  if (!se) // Let evalCast handle non symbolic expressions.
+return evalCast(val, castTy, originalTy);
+
+  // Find the maximum value of the target type.
+  APSIntType ToType(getContext().getTypeSize(castTy),
+castTy->isUnsignedIntegerType());
+  llvm::APSInt ToTypeMax = ToType.getMaxValue();
+  NonLoc ToTypeMaxVal =
+  makeIntVal(ToTypeMax.isUnsigned() ? ToTypeMax.getZExtValue()
+: ToTypeMax.getSExtValue(),
+ castTy)
+  .castAs();
+  // Check the range of the symbol being casted against the maximum value of 
the
+  // target type.
+  N

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

2016-01-11 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D12901#320680, @zaks.anna wrote:

> > This patch also fixes a bug in 'RangeSet::pin' causing single value ranges 
> > to not be considered conventionally ordered.
>
>
> Can that fix be submitted as a separate patch? Is there a test for it?


Yes I will look at creating a separate review for it.
Tests at lines 81, 111, 131 fail without the fix to RangeSet::pin.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:351
@@ -351,1 +350,3 @@
   }
+  case CK_IntegralCast: {
+// Delegate to SValBuilder to process.

zaks.anna wrote:
> SValBuilder::evalCast and SimpleSValBuilder::evalCastFromNonLoc perform a lot 
> of special casing. I am not sure we are not loosing anything if we bypass 
> them. For example, there is special handling of Booleans. We might want to 
> add this smarter handling of the integral conversions inside 
> SimpleSValBuilder::evalCastFromNonLoc, where you see the comment starting 
> with "If the types are the same or both are integers, ignore the cast."
Yes I initially looked at making the change in 'evalCastFromNonLoc' but the 
problem is that the change requires access to the ProgramState, so to avoid 
changing the interface of evalCast (used in around 50 places) I made the change 
here.
Looking at evalCast it does not seem to add any special handling to casts of 
type 'CK_IntegralCast' before calling 'evalCastFromNonLoc'?
Booleans casts should be associated with another type of cast than 
'CK_IntegralCast' and the new 'evalIntegralCast' will call 'evalCast' if it 
does not detect a truncation so it should be ok, what do you think?


http://reviews.llvm.org/D12901



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

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

2015-12-17 Thread pierre gousseau via cfe-commits

pgousseau updated the summary for this revision.
pgousseau updated this revision to Diff 43120.
pgousseau added a comment.

Following Gabor and Anna's advice:

- Instead of modifying assumeSymNE and assumeSymEQ, this patch adds a new 
method 'SValBuilder::evalIntegralCast'.

The current workaround for truncations not being modelled is that the 
evaluation of integer to integer casts are simply bypassed and so the original 
symbol is used as the new casted symbol (cf 
SimpleSValBuilder::evalCastFromNonLoc).
This lead to the issue described above, as the RangeConstraintManager 
associates ranges with symbols.

The new evalIntegralCast method added by this patch wont bypass the cast if it 
finds the range of the symbol to be greater than the maximum value of the 
target type.

This patch also fixes a bug in 'RangeSet::pin' causing single value ranges to 
not be considered conventionally ordered.

The patch has been tested with openssl-1.0.0d-src and bullet-2.82-r2704 with no 
regressions observed.

Please let me know if this an acceptable change?

Regards,

Pierre Gousseau
SN Systems - Sony Computer Entertainment


http://reviews.llvm.org/D12901

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  test/Analysis/range_casts.c

Index: test/Analysis/range_casts.c
===
--- /dev/null
+++ test/Analysis/range_casts.c
@@ -0,0 +1,156 @@
+// This test checks that intersecting ranges does not cause 'system is over constrained' assertions in the case of eg: 32 bits unsigned integers getting their range from 64 bits signed integers.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store=region -verify %s
+
+void clang_analyzer_warnIfReached();
+
+void f1(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0) // because of foo range, index is in range [0; UINT_MAX]
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f2(unsigned long foo)
+{
+  int index = -1;
+  if (index < foo) index = foo; // index equals ULONG_MAX
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // no-warning
+}
+
+void f3(unsigned long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f4(long foo)
+{
+  int index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f5(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index == -1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f6(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index == -1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f7(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1 == 0) // Was not reached prior fix.
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f8(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f9(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1L == 0L) // Was not reached prior fix.
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f10(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0L)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f11(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f12(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1UL == 0L) // Was not reached prior fix.
+clang_analyzer

Re: [PATCH] D13731: [Analyzer] Supporting function attributes in .model files.

2015-12-10 Thread pierre gousseau via cfe-commits

pgousseau abandoned this revision.
pgousseau added a comment.

Abandoning for now while evaluating Swift's APINotes.


http://reviews.llvm.org/D13731



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

Re: [PATCH] D13731: [Analyzer] Supporting function attributes in .model files.

2015-12-07 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D13731#302989, @zaks.anna wrote:

> Pierre,
>
> Have you seen the post about API Notes?
>  http://llvm.cc/t/cfe-dev-clang-and-swift/331
>
> I believe using API notes would be a better approach for adding annotations. 
> By the time the static analyzer sees the AST, the annotations would already 
> be there. The API Notes should already seamlessly work for nullability 
> annotations. Does that work for you? (Do we still need some parts of this 
> patch? I am not sure.)


Thanks for pointing it out! It is not clear yet if APINotes is suited for our 
problematic, will need to ask some questions first on the "Clang and Swift" 
cfe-dev thread.

> A related topic is deciding what type of attributes should be used by the 
> static analyzer checkers. Currently, there are 2 options, neither of which is 
> ideal:

> 

> 1. Extend clang with new attributes for static analyzes. This is not highly 
> desirable because the clang attributes namespace will get polluted and 
> compiler users might start using them. This approach is also not convenient 
> for proprietary checkers.

> 2. Annotate attribute (AnnotateAttr) is used in some places; however, this 
> attribute creeps into LLVMIR, which caused problems in the past.

> 

>   The best approach would be to put all analyzer attributes behind "a fence" 
> so that we could add/remove them without worrying that compiler(not analyzer) 
> users depend on them. The attribute would not be CodeGened.

> 

>   Here what it might look like: analyzer_annotate(family, arg…) family: id 
> arg: [id=] value value:  number | string | id

> 

>   Is anyone interested in implementing this?


The "analyzer_annotate" idea sounds good, I would need to do more evaluation 
before deciding this is suitable for us though.


http://reviews.llvm.org/D13731



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

r254181 - Test commit

2015-11-26 Thread Pierre Gousseau via cfe-commits

Author: pgousseau
Date: Thu Nov 26 16:08:58 2015
New Revision: 254181

URL: http://llvm.org/viewvc/llvm-project?rev=254181&view=rev
Log:
Test commit
Remove tabs.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp?rev=254181&r1=254180&r2=254181&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp Thu Nov 
26 16:08:58 2015
@@ -86,8 +86,7 @@ public:
   // Helpers.
   bool checkCall_strCommon(const CallExpr *CE, const FunctionDecl *FD);
 
-  typedef void (WalkAST::*FnCheck)(const CallExpr *,
-  const FunctionDecl *);
+  typedef void (WalkAST::*FnCheck)(const CallExpr *, const FunctionDecl *);
 
   // Checker-specific methods.
   void checkLoopConditionForFloat(const ForStmt *FS);


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

Re: [PATCH] D13731: [Analyzer] Supporting function attributes in .model files.

2015-11-23 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D13731



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

Re: [PATCH] D13731: [Analyzer] Supporting function attributes in .model files.

2015-11-17 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping!


http://reviews.llvm.org/D13731



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

Re: [PATCH] D13731: [RFC][Analyzer] Supporting function attributes in .model files.

2015-10-26 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 38438.
pgousseau added a comment.

Following Anna's review:

Remove unnecessary AST walk over declarations by reusing already captured 
declarations.
Add handling of merge conflicts using Sema merge methods.
Add condition at the end of ModelInjector::onBodySynthesis to prevent 
unnecessary model file parsing.
Fix 80 col in regression tests.
Add regression tests to exercise merge conflicts.

The const qualifier on model's declarations has to be removed as Sema merge 
methods take non const parameters.


http://reviews.llvm.org/D13731

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CodeInjector.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/BodyFarm.cpp
  lib/Analysis/BodyFarm.h
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/FrontendActions.cpp
  lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.h
  test/Analysis/Inputs/Models/modelFileHasAttributes.model
  test/Analysis/Inputs/Models/modelFileHasConflicts.model
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/model-attributes.cpp

Index: test/Analysis/model-attributes.cpp
===
--- /dev/null
+++ test/Analysis/model-attributes.cpp
@@ -0,0 +1,49 @@
+// This is testing the 'faux-attributes' analyzer option.
+// The declaration of 'modelFileHasAttributes' found in
+// modelFileHasAttributes.model has 'nonnull' attributes on the 2nd and 3rd
+// parameter, and a '_Nonnull' attribute on the 4th parameter.
+// The declaration of 'modelFileHasConflicts' has a visibility 'protected'
+// attribute and a '_Nullable' parameter attribute.
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown -analyze -analyzer-checker=core,nullability -analyzer-config faux-attributes=true,model-path=%S/Inputs/Models %s -verify
+
+int modelFileHasAttributes(int *p0, int *p1, int *p2, int *p3);
+void modelFileHasConflicts(int *_Nonnull p) __attribute__((visibility("hidden")));
+// expected-warning@-1 {{nullability specifier '_Nonnull' conflicts with existing specifier '_Nullable'}}
+// expected-note@-2 {{previous attribute is here}}
+// expected-note@-3 {{previous declaration is here}}
+
+int f0(int *x, int *y, int *z) {
+  int *p = 0;
+  modelFileHasAttributes(p, x, y, z); // no-warning
+  return 0;
+}
+
+int f1(int *x, int *y, int *z) {
+  int *p = 0;
+  modelFileHasAttributes(x, p, y, z);
+  // expected-warning@-1{{Null pointer passed as an argument to a 'nonnull' parameter}}
+  return 0;
+}
+
+int f2(int *x, int *y, int *z) {
+  int *p = 0;
+  modelFileHasAttributes(x, y, p, z);
+  // expected-warning@-1{{Null pointer passed as an argument to a 'nonnull' parameter}}
+  return 0;
+}
+
+int f2a(int *x, int *y, int *z) {
+  int *p = 0;
+  modelFileHasAttributes(x, y, z, p);
+  // expected-warning@-1{{Null passed to a callee that requires a non-null argument}}
+  return 0;
+}
+
+int f3() {
+  modelFileHasConflicts(0);
+  // expected-warning@-1 {{null passed to a callee that requires a non-null argument}}
+  // expected-error@./Inputs/Models/modelFileHasConflicts.model:2 {{visibility does not match previous declaration}}
+  // expected-warning@./Inputs/Models/modelFileHasConflicts.model:1 {{nullability specifier '_Nullable' conflicts with existing specifier '_Nonnull'}}
+  // expected-note@./Inputs/Models/modelFileHasConflicts.model:1 {{previous declaration is here}}
+  return 0;
+}
Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -24,6 +24,7 @@
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: faux-attributes = false
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: inline-lambdas = true
@@ -37,4 +38,4 @@
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 19
+// CHECK-NEXT: num-entries = 20
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -13,6 +13,7 @@
 // CHECK: [config]
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: faux-attributes = false
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: inline-lambdas = true
@

Re: [PATCH] D13731: [RFC][Analyzer] Supporting function attributes in .model files.

2015-10-21 Thread pierre gousseau via cfe-commits

pgousseau added inline comments.


Comment at: lib/Analysis/BodyFarm.h:43
@@ +42,3 @@
+  /// \brief Merge the attributes found in model files.
+  /// Note, this adds all attributes found in the model file without any sanity
+  /// checks.

zaks.anna wrote:
> Why do we not have sanity checks? What happens when there is a conflict?
I am worried that any custom sanity check I put would end up confusing the 
"faux-attributes" user ... I have just noticed that Sema has some merging 
capabilities, I will have a look, see if it can be reused. Thanks!


Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:505
@@ +504,3 @@
+// AST visitor used to merge model attributes.
+class WalkAST : public DataRecursiveASTVisitor {
+  AnalysisDeclContextManager &Mgr;

zaks.anna wrote:
> This name is too generic.
Will rename it thanks!


Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:541
@@ +540,3 @@
+// If "faux-attributes=true" is given, merge model's attributes.
+AnalysisDeclContextManager &ADCMgr = Mgr->getAnalysisDeclContextManager();
+if (ADCMgr.mergeModelAttributes()) {

zaks.anna wrote:
> Should we walk the entire AST here only to get the info from the few 
> functions in the farm? 
> 
> The AnalysisConsumer visitor tries to make sure the whole AST is not 
> serialized, which is very expensive when dealing with PCH files. (You an find 
> comments about it if you search for "PCH".)
I see, yes this can be optimized thanks!


Comment at: lib/StaticAnalyzer/Frontend/ModelConsumer.cpp:37
@@ +36,3 @@
+// We are interested in definitions and declarations.
+FunctionDecl *func = llvm::dyn_cast(*I);
+if (func) {

zaks.anna wrote:
> Why func lost constness here?
No reason, will fix thanks!


Comment at: lib/StaticAnalyzer/Frontend/ModelInjector.cpp:49
@@ -43,3 +48,3 @@
   // about file name index? Mangled names may not be suitable for that either.
-  if (Bodies.count(D->getName()) != 0)
+  if (Bodies.count(D->getName()) != 0 || Decls.count(D->getName()) != 0)
 return;

zaks.anna wrote:
> Does ModelInjector::onBodySynthesis return immediately if the model has been 
> parsed but the Decl is not in the map?
> 
> If that is not the case, wouldn't it be very expensive to call 
> onBodySynthesis on every decl, most of which are not modeled? If I understand 
> correctly, we would try to parse the model file for every such decl. 
> 
Yes, there might be cases where the model file's Decl might not match the model 
file filename, causing re-parsing. Will fix thanks!


Comment at: test/Analysis/model-attributes.cpp:2
@@ +1,3 @@
+// This is testing the 'faux-attributes' analyzer option.
+// The declaration of 'modelFileHasAttributes' found in 
modelFileHasAttributes.model has 'nonnull' attributes on the 2nd and 3rd 
parameter.
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -analyze 
-analyzer-checker=core -analyzer-config 
faux-attributes=true,model-path=%S/Inputs/Models %s -verify

zaks.anna wrote:
> 80 col?
Will fix!


http://reviews.llvm.org/D13731



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

Re: [PATCH] D13731: [RFC][Analyzer] Supporting function attributes in .model files.

2015-10-19 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 37765.
pgousseau added a comment.

Following Gabor's review:

Remove changes to UncheckedReturn checker.
Add a test using the NonNullParamChecker checker.
Abstract the origin of the attributes.

An analyzer option "faux-attributes" is added. This is 'false' by default and 
meant to prevent unnecessary parsing of model files.


http://reviews.llvm.org/D13731

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CodeInjector.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/BodyFarm.cpp
  lib/Analysis/BodyFarm.h
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/FrontendActions.cpp
  lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.h
  test/Analysis/Inputs/Models/modelFileHasAttributes.model
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/model-attributes.cpp

Index: test/Analysis/model-attributes.cpp
===
--- /dev/null
+++ test/Analysis/model-attributes.cpp
@@ -0,0 +1,23 @@
+// This is testing the 'faux-attributes' analyzer option.
+// The declaration of 'modelFileHasAttributes' found in modelFileHasAttributes.model has 'nonnull' attributes on the 2nd and 3rd parameter.
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -analyze -analyzer-checker=core -analyzer-config faux-attributes=true,model-path=%S/Inputs/Models %s -verify
+
+int modelFileHasAttributes(int * p0, int * p1, int * p2);
+
+int f0(int * x, int * y) {
+  int * p = 0;
+  modelFileHasAttributes(p, x, y); // no-warning
+  return 0;
+}
+
+int f1(int * x, int * y) {
+  int * p = 0;
+  modelFileHasAttributes(x, p, y); // expected-warning{{Null pointer passed as an argument to a 'nonnull' parameter}}
+  return 0;
+}
+
+int f2(int * x, int * y) {
+  int * p = 0;
+  modelFileHasAttributes(x, y, p); // expected-warning{{Null pointer passed as an argument to a 'nonnull' parameter}}
+  return 0;
+}
Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -24,6 +24,7 @@
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: faux-attributes = false
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: inline-lambdas = true
@@ -37,4 +38,4 @@
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 19
+// CHECK-NEXT: num-entries = 20
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -13,6 +13,7 @@
 // CHECK: [config]
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: faux-attributes = false
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: inline-lambdas = true
@@ -26,5 +27,5 @@
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 14
+// CHECK-NEXT: num-entries = 15
 
Index: test/Analysis/Inputs/Models/modelFileHasAttributes.model
===
--- /dev/null
+++ test/Analysis/Inputs/Models/modelFileHasAttributes.model
@@ -0,0 +1 @@
+int modelFileHasAttributes(int * p0, int * p1, int * p2 __attribute__((nonnull))) __attribute__((nonnull(2)));
\ No newline at end of file
Index: lib/StaticAnalyzer/Frontend/ModelInjector.h
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.h
+++ lib/StaticAnalyzer/Frontend/ModelInjector.h
@@ -45,6 +45,7 @@
   ModelInjector(CompilerInstance &CI);
   Stmt *getBody(const FunctionDecl *D) override;
   Stmt *getBody(const ObjCMethodDecl *D) override;
+  FunctionDecl *getModelDecl(const FunctionDecl *D) override;
 
 private:
   /// \brief Synthesize a body for a declaration
@@ -67,6 +68,9 @@
   // FIXME: double memoization is redundant, with memoization both here and in
   // BodyFarm.
   llvm::StringMap Bodies;
+
+  // Store the model's function declaration if provided.
+  llvm::StringMap Decls;
 };
 }
 }
Index: lib/StaticAnalyzer/Frontend/ModelInjector.cpp
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.cpp
+++ lib/StaticAnalyzer/Frontend/ModelInj

Re: [PATCH] D13731: [RFC][Analyzer] Supporting function attributes in .model files.

2015-10-15 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D13731#267905, @xazax.hun wrote:

> Hi!


Hi! Thank you for reviewing.

> I have some high level questions and notes about this patch.

> 

> I implemented the function modelling as a Google Summer of Code project and 
> Ted Kremenek was my mentor. I am happy that you found an useful application 
> of the function modeling system, and I think, in general, it is a good idea 
> to be able to store attributes for the modelled functions. However I am a bit 
> surprised, when I saw this patch. The main reason is that, models lack a lot 
> of functionality right now.

> 

> Main missing features:

> 

> - Ability to specify multiple model paths similar to how header include paths 
> are specified.

> - Support for methods, namespaces, overloaded functions.

> 

>   Did you find the feature useful despite those limitations? I am interested 
> to improve the situation in the future, unfortunately I find very little time 
> to work on this area recently, but I do welcome every changes.


Yes I think this is useful despite the missing features. With the current model 
file design I am confident those features could be easily added at a later 
stage?

> I have two comments before I start to review the patch itself. Right now this 
> patch contains two modifications. One for the .model files and one for a 
> checker. I think it would be better to separate these two changes into two 
> separate patches. If the motivation behind the merge of those patches is 
> that, you want to test a feature you implemented in .model files, than I 
> think there are other checker that are using attributes, so I think you 
> should be able to provide a test case with the two changes separated. (For 
> example using the nonnull parameter checker.)


I see, yes using the nonnull param checker for testing seems a better fit, I 
will update the patch.

> The other comment is that, the .model files are intended to work in a way, 
> that checkers should not be able utilize the information whether some data is 
> coming from a model file or the analyzed translation unit. In fact, this is 
> an abstraction, that makes it possible to develop the checkers and the 
> modelling mechanism independently. With you patch, the checker should observe 
> the model declaration explicitly. I think, a better design would somehow 
> merge the attributes that are coming from the original translation unit with 
> the attributes coming from the model files, and expore that set of 
> attributes. This way the checker could not tell what the source of the 
> information is. Unfortunately I do not know what would be the best way to 
> enfore this, since the checkers can just observe the AST of the original 
> translation unit and bypassing the models.


I agree, abstracting the attributes' origin is nicer, hopefully we can find a 
solution that is elegant enough. I will have a look.
Thanks!

Pierre


http://reviews.llvm.org/D13731



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

Re: [PATCH] D13731: [RFC][Analyzer] Supporting function attributes in .model files.

2015-10-15 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 37466.
pgousseau added a comment.

Fix incorrect caching of model files declarations.


http://reviews.llvm.org/D13731

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CodeInjector.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/BodyFarm.cpp
  lib/Analysis/BodyFarm.h
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  lib/StaticAnalyzer/Frontend/FrontendActions.cpp
  lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.h
  test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
  test/Analysis/unchecked-return.cpp

Index: test/Analysis/unchecked-return.cpp
===
--- /dev/null
+++ test/Analysis/unchecked-return.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -analyze -analyzer-checker=core,security.insecureAPI.UncheckedReturn -analyzer-config faux-bodies=true,model-path=%S/Inputs/Models %s -verify
+
+int returnNeedsCheckHasModelFile();
+
+int f0() {
+  returnNeedsCheckHasModelFile(); // expected-warning{{The return value from the call to 'returnNeedsCheckHasModelFile' is not checked.}}
+  return 0;
+}
Index: test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
===
--- /dev/null
+++ test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
@@ -0,0 +1 @@
+int returnNeedsCheckHasModelFile() __attribute__((warn_unused_result));
Index: lib/StaticAnalyzer/Frontend/ModelInjector.h
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.h
+++ lib/StaticAnalyzer/Frontend/ModelInjector.h
@@ -45,6 +45,7 @@
   ModelInjector(CompilerInstance &CI);
   Stmt *getBody(const FunctionDecl *D) override;
   Stmt *getBody(const ObjCMethodDecl *D) override;
+  FunctionDecl *getModelDecl(const FunctionDecl *D) override;
 
 private:
   /// \brief Synthesize a body for a declaration
@@ -67,6 +68,9 @@
   // FIXME: double memoization is redundant, with memoization both here and in
   // BodyFarm.
   llvm::StringMap Bodies;
+
+  // Store the model's function declaration if provided.
+  llvm::StringMap Decls;
 };
 }
 }
Index: lib/StaticAnalyzer/Frontend/ModelInjector.cpp
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.cpp
+++ lib/StaticAnalyzer/Frontend/ModelInjector.cpp
@@ -37,11 +37,16 @@
   return Bodies[D->getName()];
 }
 
+FunctionDecl *ModelInjector::getModelDecl(const FunctionDecl *D) {
+  onBodySynthesis(D);
+  return Decls[D->getName()];
+}
+
 void ModelInjector::onBodySynthesis(const NamedDecl *D) {
 
   // FIXME: what about overloads? Declarations can be used as keys but what
   // about file name index? Mangled names may not be suitable for that either.
-  if (Bodies.count(D->getName()) != 0)
+  if (Bodies.count(D->getName()) != 0 || Decls.count(D->getName()) != 0)
 return;
 
   SourceManager &SM = CI.getSourceManager();
@@ -60,6 +65,7 @@
 
   if (!llvm::sys::fs::exists(fileName.str())) {
 Bodies[D->getName()] = nullptr;
+Decls[D->getName()] = nullptr;
 return;
   }
 
@@ -95,7 +101,7 @@
 
   Instance.getPreprocessor().InitializeForModelFile();
 
-  ParseModelFileAction parseModelFile(Bodies);
+  ParseModelFileAction parseModelFile(Bodies, Decls);
 
   const unsigned ThreadStackSize = 8 << 20;
   llvm::CrashRecoveryContext CRC;
Index: lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
@@ -26,16 +26,20 @@
 using namespace clang;
 using namespace ento;
 
-ModelConsumer::ModelConsumer(llvm::StringMap &Bodies)
-: Bodies(Bodies) {}
+ModelConsumer::ModelConsumer(llvm::StringMap &Bodies,
+ llvm::StringMap &Decls)
+: Bodies(Bodies), Decls(Decls) {}
 
 bool ModelConsumer::HandleTopLevelDecl(DeclGroupRef D) {
   for (DeclGroupRef::iterator I = D.begin(), E = D.end(); I != E; ++I) {
 
-// Only interested in definitions.
-const FunctionDecl *func = llvm::dyn_cast(*I);
-if (func && func->hasBody()) {
-  Bodies.insert(std::make_pair(func->getName(), func->getBody()));
+// We are interested in definitions and declarations.
+FunctionDecl *func = llvm::dyn_cast(*I);
+if (func) {
+  Decls.insert(std::make_pair(func->getName(), func));
+  if (func->hasBody()) {
+Bodies.insert(std::make_pair(func->getName(), func->getBody()));
+  }
 }
   }
   return true;
Index: lib/StaticAnalyzer/Frontend/FrontendActions.cpp
===
--- lib/StaticAnalyzer/Frontend/FrontendAc

[PATCH] D13731: [RFC][Analyzer] Supporting function attributes in .model files.

2015-10-14 Thread pierre gousseau via cfe-commits

pgousseau created this revision.
pgousseau added a subscriber: cfe-commits.

Dear All,

I would like to propose a patch for specifying function attributes in .model 
files to be used by the analyzer checkers.

There are a number of checkers (such as UncheckedReturn) that contain hardcoded 
list of functions, I would like to make these checkers available to a wider 
number of functions by tagging the function definition with appropriate 
attribute information.  I have a situation where we cannot append the function 
attributes to library header files; instead I propose a patch that allows us to 
publish attribute information via .model files. The patch modifies the .model 
file parser to allow for the extraction of function declarations found in the 
.model file.  The UncheckedReturn checker is modified to handle the 
'warn_unused_result' attribute.  Attributes obtained from non-model files are 
still available to the analyzer, and the interface does not try to hide the 
origin of the attribute.

Please can you review this for submission

Regards,

Pierre Gousseau
SN-Systems - Sony Computer Entertainment

http://reviews.llvm.org/D13731

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CodeInjector.h
  include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  include/clang/StaticAnalyzer/Frontend/ModelConsumer.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/BodyFarm.cpp
  lib/Analysis/BodyFarm.h
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  lib/StaticAnalyzer/Frontend/FrontendActions.cpp
  lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.cpp
  lib/StaticAnalyzer/Frontend/ModelInjector.h
  test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
  test/Analysis/unchecked-return.cpp

Index: test/Analysis/unchecked-return.cpp
===
--- /dev/null
+++ test/Analysis/unchecked-return.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -analyze -analyzer-checker=core,security.insecureAPI.UncheckedReturn -analyzer-config faux-bodies=true,model-path=%S/Inputs/Models %s -verify
+
+int returnNeedsCheckHasModelFile();
+
+int f0() {
+  returnNeedsCheckHasModelFile(); // expected-warning{{The return value from the call to 'returnNeedsCheckHasModelFile' is not checked.}}
+  return 0;
+}
Index: test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
===
--- /dev/null
+++ test/Analysis/Inputs/Models/returnNeedsCheckHasModelFile.model
@@ -0,0 +1 @@
+int returnNeedsCheckHasModelFile() __attribute__((warn_unused_result));
Index: lib/StaticAnalyzer/Frontend/ModelInjector.h
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.h
+++ lib/StaticAnalyzer/Frontend/ModelInjector.h
@@ -45,6 +45,7 @@
   ModelInjector(CompilerInstance &CI);
   Stmt *getBody(const FunctionDecl *D) override;
   Stmt *getBody(const ObjCMethodDecl *D) override;
+  FunctionDecl *getModelDecl(const FunctionDecl *D) override;
 
 private:
   /// \brief Synthesize a body for a declaration
@@ -67,6 +68,9 @@
   // FIXME: double memoization is redundant, with memoization both here and in
   // BodyFarm.
   llvm::StringMap Bodies;
+
+  // Store the model's function declaration if provided.
+  llvm::StringMap Decls;
 };
 }
 }
Index: lib/StaticAnalyzer/Frontend/ModelInjector.cpp
===
--- lib/StaticAnalyzer/Frontend/ModelInjector.cpp
+++ lib/StaticAnalyzer/Frontend/ModelInjector.cpp
@@ -37,6 +37,11 @@
   return Bodies[D->getName()];
 }
 
+FunctionDecl *ModelInjector::getModelDecl(const FunctionDecl *D) {
+  onBodySynthesis(D);
+  return Decls[D->getName()];
+}
+
 void ModelInjector::onBodySynthesis(const NamedDecl *D) {
 
   // FIXME: what about overloads? Declarations can be used as keys but what
@@ -60,6 +65,7 @@
 
   if (!llvm::sys::fs::exists(fileName.str())) {
 Bodies[D->getName()] = nullptr;
+Decls[D->getName()] = nullptr;
 return;
   }
 
@@ -95,7 +101,7 @@
 
   Instance.getPreprocessor().InitializeForModelFile();
 
-  ParseModelFileAction parseModelFile(Bodies);
+  ParseModelFileAction parseModelFile(Bodies, Decls);
 
   const unsigned ThreadStackSize = 8 << 20;
   llvm::CrashRecoveryContext CRC;
Index: lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
===
--- lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/ModelConsumer.cpp
@@ -26,16 +26,20 @@
 using namespace clang;
 using namespace ento;
 
-ModelConsumer::ModelConsumer(llvm::StringMap &Bodies)
-: Bodies(Bodies) {}
+ModelConsumer::ModelConsumer(llvm::StringMap &Bodies,
+ llvm::StringMap &Decls)
+: Bodies(Bodies), Decls(Decls) {}
 
 bool ModelConsumer::HandleTopLevelDecl(DeclGroupRef D) {
   for (DeclGroupRef::iter

Re: [PATCH] D12901: [Static Analyzer] Assertion "System is over constrained" after truncating 64 bits integers to 32 bits. (PR25078)

2015-10-08 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D12901#262270, @zaks.anna wrote:

> I agree with Gabor. We should investigate how we can model the overflow on a 
> cast correctly.

Yes I agree with Gabor too. I meant this change as a temporary workaround only, 
I will investigate the modelling route and let you know.

Thanks!

http://reviews.llvm.org/D12901

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

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

2015-10-07 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

For some reasons the patch file I get from the "download raw diff link" does 
not contain the diff for the added .keep files? But the .keep files show up in 
phabricator, so I guess it is a phabricator issue?


Repository:
  rL LLVM

http://reviews.llvm.org/D13482



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

Re: [PATCH] D12901: [Static Analyzer] Intersecting ranges and 64 bit to 32 bit truncations causing "System is over constrained." assertions.

2015-10-02 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping !


http://reviews.llvm.org/D12901



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

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-24 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D12571#252366, @dcoughlin wrote:

> Looks good to me! Thanks Pierre! I will commit.


Much appreciated thanks !


http://reviews.llvm.org/D12571



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

Re: [PATCH] D12901: [Static Analyzer] Intersecting ranges and 64 bit to 32 bit truncations causing "System is over constrained." assertions.

2015-09-24 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 35604.
pgousseau added a comment.

Following Gabor's review:

Add a test factoring 'index + 1' in an extra variable.

Let me know if this looks reasonable ?

Regards,

Pierre


http://reviews.llvm.org/D12901

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.h
  test/Analysis/range_casts.c

Index: test/Analysis/range_casts.c
===
--- /dev/null
+++ test/Analysis/range_casts.c
@@ -0,0 +1,158 @@
+// This test checks that intersecting ranges does not cause 'system is over constrained' assertions in the case of eg: 32 bits unsigned integers getting their range from 64 bits signed integers.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store=region -verify %s
+
+void clang_analyzer_warnIfReached();
+
+void f1(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0) // because of foo range, index is in range [0; UINT_MAX]
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f2(unsigned long foo)
+{
+  int index = -1;
+  if (index < foo) index = foo; // index equals ULONG_MAX
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // no-warning
+}
+
+void f3(unsigned long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f4(long foo)
+{
+  int index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f5(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index == -1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f6(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index == -1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f7(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f8(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f9(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1L == 0L)
+// FIXME: should be reached
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f10(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0L)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f11(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f12(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1UL == 0L)
+// FIXME: should be reached
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f13(int foo)
+{
+  unsigned short index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f14(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  long bar = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f15(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  unsigned int tmp = index + 1;
+  if (tmp == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.h

Re: [PATCH] D12901: [Static Analyzer] Intersecting ranges and 64 bit to 32 bit truncations causing "System is over constrained." assertions.

2015-09-23 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D12901#248842, @xazax.hun wrote:

> Hi!
>
> Thank you for the patch!

Thanks for reviewing !

> What happens if you factor the "index + 1" expression out into a separate 
> variable?

>  E.g.: unsigned temp = index + 1; and use temp in the condition?

In this case the symbol 'temp' is still considered by the analyzer as a 'Sym + 
Int' symbol so the same code path is followed.
I will add the test case thanks !

> My impression is that, the ranges does not model the overflow behavior 
> correctly (which is well defined for unsigned values). I wondering why do you 
> think that, the right way to solve this is to modify assumeSymNE and 
> assumeSymEQ? Wouldn't it be better to actually handle the ranges properly on 
> assignments and other operations (such as +), so assumeSymNE and assumeSymEQ 
> can remain unmodified?

Yes handling ranges properly would be better! I am trying to fix the assertion 
without having to do too much re-engineering, I agree that changing 
assumeSymNE/assumeSymEQ's interfaces is not ideal but it has I hope the 
advantage of making the purpose of the change clearer and easier to revert 
should modelling of truncations/promotions be added ? Any ideas on how I could 
avoid changing the interface?

Regards,

Pierre

http://reviews.llvm.org/D12901

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

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-23 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 35505.
pgousseau added a comment.

Following Devin's review:

Fix a comment in 'IsFirstBufInBound'.
Remove incorrect assertion expecting 'RegionOffset::getOffset' to only return 
positives values.
Add test cases for regions having negatives offsets.

Updated a 'RegionOffset' class comment which seemed to suggest that 
'RegionOffset::getOffset' could not return negative values.
Added test cases and handling for arrays' regions whose upper boundary can 
overflow.

Let me know if this is an acceptable change ?

Regards,

Pierre Gousseau


http://reviews.llvm.org/D12571

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,916 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated.
+// Specific triple set to test structures of size 0.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n); // expected-note{{passing argument to parameter 'dst' here}}
+void *malloc(size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s2 = strdup("hola");
+  char input[] = {'a', 'b', 'c', 'd'};
+  char * dest = (char*)(b0.a.s1);
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(b0.a.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_a

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-18 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D12571#248464, @dcoughlin wrote:

> Here is a reduced test case:


Very useful thanks !



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:198
@@ +197,3 @@
+
+  // Return true if the destination buffer of the copy function must/may be in
+  // bound.

dcoughlin wrote:
> Since this returns true on unknown, it should be "may".
Makes sense thanks !


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1109
@@ +1108,3 @@
+  }
+  if (RO.getOffset() < 0) {
+// FIXME: should be an assert but this does occur on private build 
bots.

dcoughlin wrote:
> I think it is important to figure out enough of why the assert was triggering 
> so that we can add a test case for this 'if'. The preprocessed file that is 
> failing our bots is from the bugzilla for PR13765, in the 
> [[https://llvm.org/bugs/attachment.cgi?id=9155 |  clang2 attachment ]].
> 
> You can reproduce with:
> 
> ```
> clang -cc1 -analyze -analyzer-checker=core,cplusplus -fcxx-exceptions 
> -std=c++11 clang2/clang_crash_4pFspw.ii
> ```
Will have a look !


http://reviews.llvm.org/D12571



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

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-17 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

Ping !


http://reviews.llvm.org/D12571



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

Re: [PATCH] D12901: [Static Analyzer] Intersecting ranges and 64 bit to 32 bit truncations causing "System is over constrained." assertions.

2015-09-16 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 34900.
pgousseau added a comment.

Added some comments.


http://reviews.llvm.org/D12901

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.h
  test/Analysis/range_casts.c

Index: test/Analysis/range_casts.c
===
--- /dev/null
+++ test/Analysis/range_casts.c
@@ -0,0 +1,147 @@
+// This test checks that intersecting ranges does not cause 'system is over constrained' assertions in the case of eg: 32 bits unsigned integers getting their range from 64 bits signed integers.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store=region -verify %s
+
+void clang_analyzer_warnIfReached();
+
+void f1(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0) // because of foo range, index is in range [0; UINT_MAX]
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f2(unsigned long foo)
+{
+  int index = -1;
+  if (index < foo) index = foo; // index equals ULONG_MAX
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // no-warning
+}
+
+void f3(unsigned long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f4(long foo)
+{
+  int index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f5(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index == -1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f6(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index == -1)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f7(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f8(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f9(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1L == 0L)
+// FIXME: should be reached
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f10(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0L)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f11(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index + 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f12(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  if (index - 1UL == 0L)
+// FIXME: should be reached
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f13(int foo)
+{
+  unsigned short index = -1;
+  if (index < foo) index = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void f14(long foo)
+{
+  unsigned index = -1;
+  if (index < foo) index = foo;
+  long bar = foo;
+  if (index + 1 == 0)
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  else
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.h
===
--- lib/StaticAnalyzer/Core/SimpleConstraintManager.h
+++ lib/StaticAnalyzer/Core/SimpleConstraintManager.h
@@ -53,11 +53,13 @@
   // operation for the method being invoked.
   virtual ProgramStateRef assumeSymNE(ProgramStateRef state, SymbolRef sym,
  const llvm::APSInt& V,
- const llvm::

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-09 Thread pierre gousseau via cfe-commits

pgousseau updated the summary for this revision.
pgousseau updated this revision to Diff 34359.
pgousseau added a comment.

Following Devin's review:
Correct test for unknown length and unknown destination buffer.
Add comment to 'IsFirstBufInBound' behavior regarding unknown states.
Remove comments regarding tainted values.
Replace assertion regarding negative Region offset by an if statement.

Thanks for reviewing !

Pierre


http://reviews.llvm.org/D12571

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,756 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated.
+// Specific triple set to test structures of size 0.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n); // expected-note{{passing argument to parameter 'dst' here}}
+void *malloc(size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s2 = strdup("hola");
+  char input[] = {'a', 'b', 'c', 'd'};
+  char * dest = (char*)(b0.a.s1);
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(b0.a.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyze

Re: [PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-09 Thread pierre gousseau via cfe-commits

pgousseau added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:843
@@ +842,3 @@
+  if (!Length)
+return true;
+

dcoughlin wrote:
> There doesn't seem to be a test that cares about this returning true (as 
> compared to false).
Will add thanks, f263 was the test meant for it.


Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:855
@@ +854,3 @@
+  if (!BufLoc)
+return true;
+

dcoughlin wrote:
> There doesn't appear to be a test that cares about this returning true (as 
> compared to false).
Will add thanks, f34 was the test meant for it.


Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:863
@@ +862,3 @@
+  if (!R)
+return true;
+

dcoughlin wrote:
> What's the rationale for treating the array access as in-bounds if the BufEnd 
> is unknown? Or if LengthValue is unknown? Should these branches return false? 
> Either way, can you add a comment explaining why this is the right thing to 
> do and also update the doc comment of IsFirstBufInBound to reflect this 
> behavior (e.g., "Returns true if destination buffer of copy function must/may 
> be in bound")
Yes I mean to return true in the case of an unknown array access to not warn on 
unknown state, will add comment thanks.


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1109
@@ +1108,3 @@
+  }
+  assert(RO.getOffset() >= 0 && "Offset should not be negative");
+  uint64_t LowerOffset = RO.getOffset();

dcoughlin wrote:
> This assertion is triggering on our internal build bots. I'm working to get a 
> reduced test case.
I can replace the assert by an if statement meanwhile ?


Comment at: test/Analysis/pr22954.c:739
@@ +738,3 @@
+// Test tainted values.
+struct yy {
+  char s1[4];

dcoughlin wrote:
> A question: how does this test tainted values?
Yes this comment is wrong, this is not testing tainted values. Will change 
comment and description thanks.


http://reviews.llvm.org/D12571



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

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-09-03 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D11832#237110, @pgousseau wrote:

> In http://reviews.llvm.org/D11832#236672, @xazax.hun wrote:
>
> > I reverted the commit until this assertion is fixed.
> >
> > Steps to reproduce:
> >  Download the following preprocessed file: F804743: clang_crash_7QnDaH.i 
> > 
> >  Execute the analyzer on that one: clang -cc1 -analyze 
> > -analyzer-checker=core -analyzer-checker=unix -fblocks clang_crash_7QnDaH.i
>
>
> Thanks for the reproducible and revert, I will have a look !


I have now uploaded a patch for this at http://reviews.llvm.org/D12571


Repository:
  rL LLVM

http://reviews.llvm.org/D11832



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

[PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

2015-09-02 Thread pierre gousseau via cfe-commits

pgousseau created this revision.
pgousseau added reviewers: ayartsev, zaks.anna, dcoughlin, xazax.hun, ClockMan.
pgousseau added a subscriber: cfe-commits.

Fix assertions kindly reported by Gabor Horvath and Piotr Zegar caused by 
commit r246345 and reverted in r246479.
Original review in http://reviews.llvm.org/D11832
Issue is caused by 'IsFirstBufInBound(...)' I wrongly assumed that no 'Unknown' 
values could reach it.
Added tests for unknown and tainted arguments being passed to 'memcpy' call.

Please let me know if this is an acceptable change ?

Regards,

Pierre Gousseau
SN-Systems - Sony Computer Entertainment

http://reviews.llvm.org/D12571

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,756 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated.
+// Specific triple set to test structures of size 0.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n); // expected-note{{passing argument to parameter 'dst' here}}
+void *malloc(size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s2 = strdup("hola");
+  char input[] = {'a', 'b', 'c', 'd'};
+  char * dest = (char*)(b0.a.s1);
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(b0.a.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[2] == 'c'

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-09-01 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D11832#236672, @xazax.hun wrote:

> I reverted the commit until this assertion is fixed.
>
> Steps to reproduce:
>  Download the following preprocessed file: F804743: clang_crash_7QnDaH.i 
> 
>  Execute the analyzer on that one: clang -cc1 -analyze -analyzer-checker=core 
> -analyzer-checker=unix -fblocks clang_crash_7QnDaH.i


Thanks for the reproducible and revert, I will have a look !


Repository:
  rL LLVM

http://reviews.llvm.org/D11832



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

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-31 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D11832#235301, @dcoughlin wrote:

> I will commit. Thanks for your work on this, Pierre!


Much appreciated ! Thanks for your help.


Repository:
  rL LLVM

http://reviews.llvm.org/D11832



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

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-28 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 33402.
pgousseau added a comment.

Replace 'dyn_cast' by 'cast' following Devin's review.

If all looks good could someone commit this patch for me ?

Regards,

Pierre


http://reviews.llvm.org/D11832

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,697 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated.
+// Specific triple set to test structures of size 0.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n); // expected-note{{passing argument to parameter 'dst' here}}
+void *malloc(size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s2 = strdup("hola");
+  char input[] = {'a', 'b', 'c', 'd'};
+  char * dest = (char*)(b0.a.s1);
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(b0.a.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(b0.a.s2); // no warning
+  free(b0.s2); // no warning
+  return 0;
+}
+
+// Test that memor

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-28 Thread pierre gousseau via cfe-commits

pgousseau added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:863
@@ +862,3 @@
+
+  const ElementRegion *ER = dyn_cast(R);
+  // Cast is safe as BufVal's region is a FieldRegion.

dcoughlin wrote:
> You can use cast(R) here, which will assert that R is an 
> ElementRegion, and remove the assert below.
Will do ! Thanks.


http://reviews.llvm.org/D11832



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

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-27 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 33326.
pgousseau added a comment.

Following Devin's review:

Modified 'IsFirstBufInBound()' to return a bool instead of a state.
Removed redundant checks in 'IsFirstBufInBound'.
Added tests to exercise 'IsFirstBufInBound' more.
Removed if/else to avoid indentation.
Removed copy paste error.
Inverted guards in 'VisitCluster' to decrease indentation.
Added comments regarding 0 elements/0-sized elements arrays.
Fixed unused var warning with isa instead of dyn_cast.
Fixed test typo and added sizeof test.

Please let me know if this is an acceptable change ?

Regards,

Pierre


http://reviews.llvm.org/D11832

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,697 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated.
+// Specific triple set to test structures of size 0.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n); // expected-note{{passing argument to parameter 'dst' here}}
+void *malloc(size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s2 = strdup("hola");
+  char input[] = {'a', 'b', 'c', 'd'};
+  char * dest = (char*)(b0.a.s1);
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(b0.a.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[2] == 'c'); //

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-26 Thread pierre gousseau via cfe-commits

pgousseau added inline comments.


Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:825
@@ -816,1 +824,3 @@
 
+ProgramStateRef CStringChecker::IsFirstBufInBound(CheckerContext &C,
+  ProgramStateRef state,

dcoughlin wrote:
> It seems odd that you return a ProgramStateRef here but don't use it in the 
> only caller of this function. Could you just return a boolean instead? 
> Alternatively, if you really want to add the assumption that the buffer is 
> valid to the post-state, then you should thread the state returned from the 
> call to this function through the rest of CStringChecker::InvalidateBuffer.
Yes returning a bool would be better thanks, I did not meant for this function 
to add the assumption that the buffer is valid.


Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:840
@@ +839,3 @@
+  Optional Length = LengthVal.getAs();
+  if (!Length)
+return state;

dcoughlin wrote:
> You have a lot of early returns of the form:
> 
>   if (!Foo)
> return state;
> 
> that don't seem to be exercised in your tests. Can these actually trigger? If 
> so, you should add tests for these cases. If not, maybe 
> asserts/cast()/castAs() would be more appropriate.
> 
> Also: should these early returns return state? Or should they return null?
> 
> 
Yes most of those checks seems redundant and/or might need to return 
nullptr/false, will double check thanks.


Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:851
@@ +850,3 @@
+  SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());
+  if (Optional BufLoc = BufStart.getAs()) {
+

dcoughlin wrote:
> Can you rewrite this if/else to avoid the indentation? (See 
> http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return).
Yes thanks for the link !


Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:866
@@ +865,3 @@
+assert(ER->getValueType() == C.getASTContext().CharTy &&
+   "CheckLocation should only be called with char* ElementRegions");
+

dcoughlin wrote:
> "CheckLocation" is copy pasta here.
Will fix !


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1115
@@ +1114,3 @@
+  // or have a symbolic offset.
+  if (SuperR) {
+if (const ClusterBindings *C = B.lookup(SuperR)) {

dcoughlin wrote:
> This nesting is getting pretty deep. Can you invert some guards and change to 
> goto/continue where appropriate.
> 
> For example:
>   if (!SuperR)
> goto conjure_default;
> 
> and
> 
>   const ClusterBindings *C = B.lookup(SuperR)
>   if (!C)
> continue;
Makes sense thanks.


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1125
@@ +1124,3 @@
+ ((*ROffset >= LowerOffset && *ROffset < UpperOffset) ||
+  (LowerOffset == UpperOffset && *ROffset == LowerOffset {
+  B = B.removeBinding(I.getKey());

dcoughlin wrote:
> Please add a comment here to say that this is to handle arrays of 0 elements 
> and arrays of 0-sized elements.
Will do !


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1132
@@ +1131,3 @@
+  if (R)
+if (const SymbolicRegion *SR = dyn_cast(R))
+  VisitBinding(V);

dcoughlin wrote:
> If you are not going to use the result, you can use isa(R) 
> here instead of dyn_cast.
Will do !


Comment at: test/Analysis/pr22954.c:476
@@ +475,3 @@
+  clang_analyzer_eval(m->s3[3] == 1); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(m->s3[i] == 1); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(l->s1[0] == 1); // expected-warning{{UNKNOWN}}

dcoughlin wrote:
> Should this test m->s3[j] as well?
Yes that is what I meant thanks !


Comment at: test/Analysis/pr22954.c:498
@@ +497,3 @@
+}
+
+

dcoughlin wrote:
> Can you also add a test where the size argument to memcpy is the result of 
> sizeof()?
Will do !


http://reviews.llvm.org/D11832



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

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-24 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 32955.
pgousseau added a comment.

Encapsulate 'RegionAndSymbolInvalidationTraits' by overriding 'AddToWorkList' 
following Anton's review.
Also as pointed out by Devin's review:
Add tests cases and handling for regions, bindings in super regions, and 
aliased/casted regions, with symbolic offsets.
Add test case and handling for writing past the array.
Replace 'ROffset <= UpperOffset' by 'ROffset < UpperOffset'.

Please let me know if this an acceptable change ?
Regards,
Pierre


http://reviews.llvm.org/D11832

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,623 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated.
+// Specific triple set to test structures of size 0.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n);
+void *malloc(size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s2 = strdup("hola");
+  char input[] = {'a', 'b', 'c', 'd'};
+  char * dest = (char*)(b0.a.s1);
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(b0.a.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[3] == 'd'); // expected-wa

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-17 Thread pierre gousseau via cfe-commits

pgousseau added a comment.

In http://reviews.llvm.org/D11832#224929, @dcoughlin wrote:

> You should consider what should happen when the memcpy may write past the end 
> of the fixed-size array and add tests that specify correct behavior for these 
> cases. An important example is:
>
>   struct Foo {
> char data[4];
> int i;
>   };
>   
>   Foo f;
>   f.i = 10;
>  
>   memcpy(f.data, someBuf, 100);
>   
>   clang_analyzer_eval(f.i == 10); // What should this yield?
>   
>
> I think it is also important to add tests for regions at symbolic offsets, 
> for bindings in the super region having keys with symbolic offsets, and for 
> cases where there is potential aliasing and casting between regions with 
> symbolic offsets.


Yes it seems I overlooked symbolic offsets in the test cases, will handle those.

> Also, Jordan wrote up a description of the region store in 
> docs/analyzer/RegionStore.txt that you might find helpful if you haven't 
> already seen it.


Very usefull thanks !



Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:746
@@ -739,1 +745,3 @@
+return static_cast(this)->hasTrait(MR, IK);
+  }
 };

ayartsev wrote:
> Hmm.. Either we completely encapsulate 'RegionAndSymbolInvalidationTraits' in 
> the 'invalidateRegionsWorker' class or we move 
> 'RegionAndSymbolInvalidationTraits' (maybe renamed to the more general name) 
> to the base 'ClusterAnalysis' class. 
> In the suggested solution you on the one hand make processing of 
> 'RegionAndSymbolInvalidationTraits' specific to 'invalidateRegionsWorker' 
> class but on the other hand explicitly refer to 
> 'RegionAndSymbolInvalidationTraits::InvalidationKinds' and  
> 'RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion' in the 
> 'ClusterAnalysis' class.
> It seems to me the proper way to encapsulate 
> 'RegionAndSymbolInvalidationTraits' in the 'invalidateRegionsWorker' is to 
> just override 'AddToWorkList()' in subclasses (as you done with 'hasTrait()').
I agree yes overriding AddToWorkList would fit better, will upload new patch. 
Thanks !


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1110
@@ +1109,3 @@
+  assert(RO.getOffset() >= 0 && "Offset should not be negative");
+  uint64_t LowerOffset = RO.getOffset();
+  uint64_t UpperOffset = LowerOffset + *NumElements * ElemSize;

dcoughlin wrote:
> R0.getOffset() will assert if R0 is a symbolic region offset. This can happen 
> if the invalidated array is itself in an array (e.g., 
> someOtherArray[i].array) or is in a union.
Yes that definitely needs fixing. Thanks !


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1118
@@ +1117,3 @@
+   ++I) {
+uint64_t ROffset = I.getKey().getOffset();
+if (ROffset >= LowerOffset && ROffset <= UpperOffset)

dcoughlin wrote:
> getOffset() here will assert also if there is any key with a symbolic offset 
> in SuperR.
Will fix. Thanks !


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1119
@@ +1118,3 @@
+uint64_t ROffset = I.getKey().getOffset();
+if (ROffset >= LowerOffset && ROffset <= UpperOffset)
+  B = B.removeBinding(I.getKey());

dcoughlin wrote:
> Should this be ROffset < UpperOffset?
Yes, will change to (ROffset < UpperOffset || (LowerOffset == UpperOffset && 
ROffset == LowerOffset)).
To handle arrays of 0 elements and arrays of 0 sized elements.
Thanks !


http://reviews.llvm.org/D11832



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

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-12 Thread pierre gousseau via cfe-commits

pgousseau updated this revision to Diff 31940.
pgousseau added a comment.

Removed 'return' statements and added tests for empty arrays following Anna 
review.
Refactored parameter passing of 'TK_DoNotInvalidateSuperRegion' following Anton 
review, by adding a new member function 'hasTrait' to 'ClusterAnalysis'.

Please let me know if this is acceptable and if yes eventually commit it for me 
?
Regards,
Pierre


http://reviews.llvm.org/D11832

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,305 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s2 = strdup("hola");
+  char input[] = {'a', 'b', 'c', 'd'};
+  char * dest = (char*)(b0.a.s1);
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(b0.a.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(b0.a.s2); // no warning
+  free(b0.s2); // no warning
+  return 0;
+}
+
+int f5() {
+  struct aa a0 = {{1,

Re: [PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-11 Thread pierre gousseau via cfe-commits

pgousseau added inline comments.


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1098
@@ +1097,3 @@
+  if (!NumElements)
+return;
+  QualType ElementTy = AT->getElementType();

zaks.anna wrote:
> What happens on early returns? Here and the one below. Are there tests for 
> these cases?
Oops yes returning here is wrong, if we return here we skip the code conjuring 
the default value at line 1122.
I will change it to a goto conjure_default for !NumElements == true.
Also a value of 0 for ElemSize is ok so no need to return actually.
I will add a test for 0 sized elements' array and 0 elements array and update 
the patch.
What do you think ?
Thanks !


Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:2359
@@ -2314,1 +2358,3 @@
+RegionAndSymbolInvalidationTraits ITraits;
+W.AddToWorkList(*I, ITraits);
   }

ayartsev wrote:
> Too much unnecessary passing around of RegionAndSymbolInvalidationTraits 
> parameter. What about moving "RegionAndSymbolInvalidationTraits" member from 
> "invalidateRegionsWorker" class to the base class "ClusterAnalysis"?
Makes sense, I will update the patch.
Thanks !


http://reviews.llvm.org/D11832



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

[PATCH] D11832: [Patch] [Analyzer] false positive: Potential leak connected with memcpy (PR 22954)

2015-08-07 Thread pierre gousseau via cfe-commits

pgousseau created this revision.
pgousseau added reviewers: cfe-commits, ayartsev, xazax.hun.

Dear All,

I would like to propose a patch to avoid the false positive memory leak warning 
kindly reported by krzysztof in https://llvm.org/bugs/show_bug.cgi?id=22954

The issue seems originates from the CString checker's handling of 'memcpy' (and 
string copy functions in general).
Given the below code snippet:
--
struct aa { char *s; char data[32];};
...
a.s = malloc(nbytes);
memcpy(a.data, source, len);
...
--
As the CString checker handles the memcpy call, it requests the invalidation of 
the 'a.data' region. But the invalidation worker marks the whole memory region 
of 'a' as to be invalidated. The Malloc checker is not made aware of this 
causing the false positive.

Following advices from Anton Yartsev and Gabor Horvath on cfe-dev 
(http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-July/043786.html), this patch 
introduces a new trait 'TK_DoNotInvalidateSuperRegion', for the invalidation 
worker to take into account, when invalidating a destination buffer of type 
'FieldRegion'.

Please let me know if this is an acceptable change and if yes eventually commit 
it for me (as I do not have svn access) ?

Regards,

Pierre Gousseau
SN Systems - Sony Computer Entertainment

http://reviews.llvm.org/D11832

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,266 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s

82 matches

Mail list logo