[PATCH] D132003: [clang][ARM][NFC] Clean up signed conversion and undefined macros in builtin header

[Dominic Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGac77b3fde120: [clang][ARM][NFC] Clean up signed conversion 
and undefined macros in builtin… (authored by ddcc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132003

Files:
  clang/lib/Headers/arm_acle.h

Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -64,7 +64,7 @@
 }
 #endif
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __dbg(t) __builtin_arm_dbg(t)
 #endif
 
@@ -82,7 +82,7 @@
 /* 8.6.1 Data prefetch */
 #define __pld(addr) __pldx(0, 0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __pldx(access_kind, cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, access_kind, 1)
 #else
@@ -93,7 +93,7 @@
 /* 8.6.2 Instruction prefetch */
 #define __pli(addr) __plix(0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __plix(cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, 0, 0)
 #else
@@ -140,17 +140,17 @@
 /* CLZ */
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __clz(uint32_t __t) {
-  return __builtin_clz(__t);
+  return (uint32_t)__builtin_clz(__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
 __clzl(unsigned long __t) {
-  return __builtin_clzl(__t);
+  return (unsigned long)__builtin_clzl(__t);
 }
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __clzll(uint64_t __t) {
-  return __builtin_clzll(__t);
+  return (uint64_t)__builtin_clzll(__t);
 }
 
 /* CLS */
@@ -201,7 +201,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rev16ll(uint64_t __t) {
-  return (((uint64_t)__rev16(__t >> 32)) << 32) | __rev16(__t);
+  return (((uint64_t)__rev16(__t >> 32)) << 32) | (uint64_t)__rev16((uint32_t)__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
@@ -216,7 +216,7 @@
 /* REVSH */
 static __inline__ int16_t __attribute__((__always_inline__, __nodebug__))
 __revsh(int16_t __t) {
-  return __builtin_bswap16(__t);
+  return (int16_t)__builtin_bswap16((uint16_t)__t);
 }
 
 /* RBIT */
@@ -227,7 +227,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rbitll(uint64_t __t) {
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
   return (((uint64_t)__builtin_arm_rbit(__t)) << 32) |
  __builtin_arm_rbit(__t >> 32);
 #else
@@ -247,7 +247,7 @@
 /*
  * 9.3 16-bit multiplications
  */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__,__nodebug__))
 __smulbb(int32_t __a, int32_t __b) {
   return __builtin_arm_smulbb(__a, __b);
@@ -281,13 +281,13 @@
  * intrinsics are implemented and the flag is enabled.
  */
 /* 9.4.1 Width-specified saturation intrinsics */
-#if __ARM_FEATURE_SAT
+#if defined(__ARM_FEATURE_SAT) && __ARM_FEATURE_SAT
 #define __ssat(x, y) __builtin_arm_ssat(x, y)
 #define __usat(x, y) __builtin_arm_usat(x, y)
 #endif
 
 /* 9.4.2 Saturating addition and subtraction intrinsics */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __qadd(int32_t __t, int32_t __v) {
   return __builtin_arm_qadd(__t, __v);
@@ -305,7 +305,7 @@
 #endif
 
 /* 9.4.3 Accumultating multiplications */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smlabb(int32_t __a, int32_t __b, int32_t __c) {
   return __builtin_arm_smlabb(__a, __b, __c);
@@ -334,13 +334,13 @@
 
 
 /* 9.5.4 Parallel 16-bit saturation */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 #define __ssat16(x, y) __builtin_arm_ssat16(x, y)
 #define __usat16(x, y) __builtin_arm_usat16(x, y)
 #endif
 
 /* 9.5.5 Packing and unpacking */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 typedef int32_t int8x4_t;
 typedef int32_t int16x2_t;
 typedef uint32_t uint8x4_t;
@@ -365,7 +365,7 @@
 #endif
 
 /* 9.5.6 Parallel selection */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ uint8x4_t __attribute__((__always_inline__, __nodebug__))
 __sel(uint8x4_t __a, uint8x4_t __b) {
   return __builtin_arm_sel(__a, __b);
@@ -373,7 +373,7 @@
 #endif
 
 /* 9.5.7 Parallel 8-bit addition and subtraction */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int8x4_t 

[PATCH] D132003: [clang][ARM][NFC] Clean up signed conversion and undefined macros in builtin header

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Yup, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132003

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

[PATCH] D132003: [clang][ARM][NFC] Clean up signed conversion and undefined macros in builtin header

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 458581.
ddcc marked an inline comment as done.
ddcc added a comment.

Fix typo in __revsh


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132003

Files:
  clang/lib/Headers/arm_acle.h

Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -64,7 +64,7 @@
 }
 #endif
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __dbg(t) __builtin_arm_dbg(t)
 #endif
 
@@ -82,7 +82,7 @@
 /* 8.6.1 Data prefetch */
 #define __pld(addr) __pldx(0, 0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __pldx(access_kind, cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, access_kind, 1)
 #else
@@ -93,7 +93,7 @@
 /* 8.6.2 Instruction prefetch */
 #define __pli(addr) __plix(0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __plix(cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, 0, 0)
 #else
@@ -140,17 +140,17 @@
 /* CLZ */
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __clz(uint32_t __t) {
-  return __builtin_clz(__t);
+  return (uint32_t)__builtin_clz(__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
 __clzl(unsigned long __t) {
-  return __builtin_clzl(__t);
+  return (unsigned long)__builtin_clzl(__t);
 }
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __clzll(uint64_t __t) {
-  return __builtin_clzll(__t);
+  return (uint64_t)__builtin_clzll(__t);
 }
 
 /* CLS */
@@ -201,7 +201,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rev16ll(uint64_t __t) {
-  return (((uint64_t)__rev16(__t >> 32)) << 32) | __rev16(__t);
+  return (((uint64_t)__rev16(__t >> 32)) << 32) | (uint64_t)__rev16((uint32_t)__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
@@ -216,7 +216,7 @@
 /* REVSH */
 static __inline__ int16_t __attribute__((__always_inline__, __nodebug__))
 __revsh(int16_t __t) {
-  return __builtin_bswap16(__t);
+  return (int16_t)__builtin_bswap16((uint16_t)__t);
 }
 
 /* RBIT */
@@ -227,7 +227,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rbitll(uint64_t __t) {
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
   return (((uint64_t)__builtin_arm_rbit(__t)) << 32) |
  __builtin_arm_rbit(__t >> 32);
 #else
@@ -247,7 +247,7 @@
 /*
  * 9.3 16-bit multiplications
  */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__,__nodebug__))
 __smulbb(int32_t __a, int32_t __b) {
   return __builtin_arm_smulbb(__a, __b);
@@ -281,13 +281,13 @@
  * intrinsics are implemented and the flag is enabled.
  */
 /* 9.4.1 Width-specified saturation intrinsics */
-#if __ARM_FEATURE_SAT
+#if defined(__ARM_FEATURE_SAT) && __ARM_FEATURE_SAT
 #define __ssat(x, y) __builtin_arm_ssat(x, y)
 #define __usat(x, y) __builtin_arm_usat(x, y)
 #endif
 
 /* 9.4.2 Saturating addition and subtraction intrinsics */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __qadd(int32_t __t, int32_t __v) {
   return __builtin_arm_qadd(__t, __v);
@@ -305,7 +305,7 @@
 #endif
 
 /* 9.4.3 Accumultating multiplications */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smlabb(int32_t __a, int32_t __b, int32_t __c) {
   return __builtin_arm_smlabb(__a, __b, __c);
@@ -334,13 +334,13 @@
 
 
 /* 9.5.4 Parallel 16-bit saturation */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 #define __ssat16(x, y) __builtin_arm_ssat16(x, y)
 #define __usat16(x, y) __builtin_arm_usat16(x, y)
 #endif
 
 /* 9.5.5 Packing and unpacking */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 typedef int32_t int8x4_t;
 typedef int32_t int16x2_t;
 typedef uint32_t uint8x4_t;
@@ -365,7 +365,7 @@
 #endif
 
 /* 9.5.6 Parallel selection */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ uint8x4_t __attribute__((__always_inline__, __nodebug__))
 __sel(uint8x4_t __a, uint8x4_t __b) {
   return __builtin_arm_sel(__a, __b);
@@ -373,7 +373,7 @@
 #endif
 
 /* 9.5.7 Parallel 8-bit addition and subtraction */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int8x4_t __attribute__((__always_inline__, __nodebug__))
 __qadd8(int8x4_t __a, int8x4_t __b) {
   return __builtin_arm_qadd8(__a, __b);
@@ -425,7 

[PATCH] D132003: [clang][ARM][NFC] Clean up signed conversion and undefined macros in builtin header

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added reviewers: tmatheson, javed.absar, SjoerdMeijer.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
ddcc requested review of this revision.
Herald added a project: clang.

These warnings were identified while debugging modules with Wsystem-headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132003

Files:
  clang/lib/Headers/arm_acle.h

Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -64,7 +64,7 @@
 }
 #endif
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __dbg(t) __builtin_arm_dbg(t)
 #endif
 
@@ -82,7 +82,7 @@
 /* 8.6.1 Data prefetch */
 #define __pld(addr) __pldx(0, 0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __pldx(access_kind, cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, access_kind, 1)
 #else
@@ -93,7 +93,7 @@
 /* 8.6.2 Instruction prefetch */
 #define __pli(addr) __plix(0, 0, addr)
 
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
 #define __plix(cache_level, retention_policy, addr) \
   __builtin_arm_prefetch(addr, 0, 0)
 #else
@@ -140,17 +140,17 @@
 /* CLZ */
 static __inline__ uint32_t __attribute__((__always_inline__, __nodebug__))
 __clz(uint32_t __t) {
-  return __builtin_clz(__t);
+  return (uint32_t)__builtin_clz(__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
 __clzl(unsigned long __t) {
-  return __builtin_clzl(__t);
+  return (unsigned long)__builtin_clzl(__t);
 }
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __clzll(uint64_t __t) {
-  return __builtin_clzll(__t);
+  return (uint64_t)__builtin_clzll(__t);
 }
 
 /* CLS */
@@ -201,7 +201,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rev16ll(uint64_t __t) {
-  return (((uint64_t)__rev16(__t >> 32)) << 32) | __rev16(__t);
+  return (((uint64_t)__rev16(__t >> 32)) << 32) | (uint64_t)__rev16((uint32_t)__t);
 }
 
 static __inline__ unsigned long __attribute__((__always_inline__, __nodebug__))
@@ -216,7 +216,7 @@
 /* REVSH */
 static __inline__ int16_t __attribute__((__always_inline__, __nodebug__))
 __revsh(int16_t __t) {
-  return __builtin_bswap16(__t);
+  return (int16_t)__builtin_bswap16((int16_t)__t);
 }
 
 /* RBIT */
@@ -227,7 +227,7 @@
 
 static __inline__ uint64_t __attribute__((__always_inline__, __nodebug__))
 __rbitll(uint64_t __t) {
-#if __ARM_32BIT_STATE
+#if defined(__ARM_32BIT_STATE) && __ARM_32BIT_STATE
   return (((uint64_t)__builtin_arm_rbit(__t)) << 32) |
  __builtin_arm_rbit(__t >> 32);
 #else
@@ -247,7 +247,7 @@
 /*
  * 9.3 16-bit multiplications
  */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__,__nodebug__))
 __smulbb(int32_t __a, int32_t __b) {
   return __builtin_arm_smulbb(__a, __b);
@@ -281,13 +281,13 @@
  * intrinsics are implemented and the flag is enabled.
  */
 /* 9.4.1 Width-specified saturation intrinsics */
-#if __ARM_FEATURE_SAT
+#if defined(__ARM_FEATURE_SAT) && __ARM_FEATURE_SAT
 #define __ssat(x, y) __builtin_arm_ssat(x, y)
 #define __usat(x, y) __builtin_arm_usat(x, y)
 #endif
 
 /* 9.4.2 Saturating addition and subtraction intrinsics */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __qadd(int32_t __t, int32_t __v) {
   return __builtin_arm_qadd(__t, __v);
@@ -305,7 +305,7 @@
 #endif
 
 /* 9.4.3 Accumultating multiplications */
-#if __ARM_FEATURE_DSP
+#if defined(__ARM_FEATURE_DSP) && __ARM_FEATURE_DSP
 static __inline__ int32_t __attribute__((__always_inline__, __nodebug__))
 __smlabb(int32_t __a, int32_t __b, int32_t __c) {
   return __builtin_arm_smlabb(__a, __b, __c);
@@ -334,13 +334,13 @@
 
 
 /* 9.5.4 Parallel 16-bit saturation */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 #define __ssat16(x, y) __builtin_arm_ssat16(x, y)
 #define __usat16(x, y) __builtin_arm_usat16(x, y)
 #endif
 
 /* 9.5.5 Packing and unpacking */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 typedef int32_t int8x4_t;
 typedef int32_t int16x2_t;
 typedef uint32_t uint8x4_t;
@@ -365,7 +365,7 @@
 #endif
 
 /* 9.5.6 Parallel selection */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ uint8x4_t __attribute__((__always_inline__, __nodebug__))
 __sel(uint8x4_t __a, uint8x4_t __b) {
   return __builtin_arm_sel(__a, __b);
@@ -373,7 +373,7 @@
 #endif
 
 /* 9.5.7 Parallel 8-bit addition and subtraction */
-#if __ARM_FEATURE_SIMD32
+#if defined(__ARM_FEATURE_SIMD32) && __ARM_FEATURE_SIMD32
 static __inline__ int8x4_t 

[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I missed line 19, yeah that makes sense. @iana is that ok with you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131213

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

[PATCH] D131213: [clang][Headers] Fix unintentional error in D130800

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added reviewers: aaron.ballman, iana.
Herald added a project: All.
ddcc requested review of this revision.
Herald added a project: clang.

Undefined macros evaluate to zero, so when checking for a smaller value,
we need to include the case when the macro is undefined.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131213

Files:
  clang/lib/Headers/stdbool.h


Index: clang/lib/Headers/stdbool.h
===
--- clang/lib/Headers/stdbool.h
+++ clang/lib/Headers/stdbool.h
@@ -23,7 +23,7 @@
 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__)
 /* Define _Bool as a GNU extension. */
 #define _Bool bool
-#if defined(__cplusplus) && __cplusplus < 201103L
+#if !defined(__cplusplus) || __cplusplus < 201103L
 /* For C++98, define bool, false, true as a GNU extension. */
 #define bool bool
 #define false false


Index: clang/lib/Headers/stdbool.h
===
--- clang/lib/Headers/stdbool.h
+++ clang/lib/Headers/stdbool.h
@@ -23,7 +23,7 @@
 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__)
 /* Define _Bool as a GNU extension. */
 #define _Bool bool
-#if defined(__cplusplus) && __cplusplus < 201103L
+#if !defined(__cplusplus) || __cplusplus < 201103L
 /* For C++98, define bool, false, true as a GNU extension. */
 #define bool bool
 #define false false
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbf19005714b: [clang][Headers] Avoid compiler warnings in 
builtin headers (authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D130800?vs=449458=449844#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/float.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdatomic.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/velintrin.h

Index: clang/lib/Headers/velintrin.h
===
--- clang/lib/Headers/velintrin.h
+++ clang/lib/Headers/velintrin.h
@@ -13,7 +13,7 @@
 typedef double __vr __attribute__((__vector_size__(2048)));
 
 // Vector mask registers
-#if __STDC_VERSION__ >= 199901L
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // For C99
 typedef _Bool __vm__attribute__((ext_vector_type(256)));
 typedef _Bool __vm256 __attribute__((ext_vector_type(256)));
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -13,7 +13,7 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
-#if __STDC_VERSION__ > 201710L &&  \
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
including the header file in C2x is also deprecated and we do not want to
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -96,13 +96,21 @@
 typedef __INT64_TYPE__ int64_t;
 # endif /* __int8_t_defined */
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t
 # define __uint_least64_t uint64_t
+# undef __int_least32_t
 # define __int_least32_t int64_t
+# undef __uint_least32_t
 # define __uint_least32_t uint64_t
+# undef __int_least16_t
 # define __int_least16_t int64_t
+# undef __uint_least16_t
 # define __uint_least16_t uint64_t
+# undef __int_least8_t
 # define __int_least8_t int64_t
+# undef __uint_least8_t
 # define __uint_least8_t uint64_t
 #endif /* __INT64_TYPE__ */
 
@@ -120,11 +128,17 @@
 typedef uint56_t uint_least56_t;
 typedef int56_t int_fast56_t;
 typedef uint56_t uint_fast56_t;
+# undef __int_least32_t
 # define __int_least32_t int56_t
+# undef __uint_least32_t
 # define __uint_least32_t uint56_t
+# undef __int_least16_t
 # define __int_least16_t int56_t
+# undef __uint_least16_t
 # define __uint_least16_t uint56_t
+# undef __int_least8_t
 # define __int_least8_t int56_t
+# undef __uint_least8_t
 # define __uint_least8_t uint56_t
 #endif /* __INT56_TYPE__ */
 
@@ -136,11 +150,17 @@
 typedef uint48_t uint_least48_t;
 typedef int48_t int_fast48_t;
 typedef uint48_t uint_fast48_t;
+# undef __int_least32_t
 # define __int_least32_t int48_t
+# undef __uint_least32_t
 # define __uint_least32_t uint48_t
+# undef __int_least16_t
 # define __int_least16_t int48_t
+# undef __uint_least16_t
 # define __uint_least16_t uint48_t
+# undef __int_least8_t
 # define __int_least8_t int48_t
+# undef __uint_least8_t
 # define __uint_least8_t uint48_t
 #endif /* __INT48_TYPE__ */
 
@@ -152,11 +172,17 @@
 typedef uint40_t uint_least40_t;
 typedef int40_t int_fast40_t;
 typedef uint40_t uint_fast40_t;
+# undef __int_least32_t
 # define __int_least32_t int40_t
+# undef __uint_least32_t
 # define __uint_least32_t uint40_t
+# undef __int_least16_t
 # define __int_least16_t int40_t
+# undef __uint_least16_t
 # define __uint_least16_t uint40_t
+# undef __int_least8_t
 # define __int_least8_t int40_t
+# undef __uint_least8_t
 # define __uint_least8_t uint40_t
 #endif /* __INT40_TYPE__ */
 
@@ -172,11 +198,17 @@
 typedef __UINT32_TYPE__ uint32_t;
 # endif /* __uint32_t_defined */
 
+# undef __int_least32_t
 # define __int_least32_t int32_t
+# undef __uint_least32_t
 # define __uint_least32_t uint32_t
+# undef __int_least16_t
 # define __int_least16_t int32_t
+# undef __uint_least16_t
 # define __uint_least16_t uint32_t
+# undef __int_least8_t
 # define __int_least8_t int32_t
+# undef __uint_least8_t
 # define __uint_least8_t uint32_t
 #endif /* __INT32_TYPE__ */
 
@@ -194,9 +226,13 @@
 typedef uint24_t uint_least24_t;
 typedef int24_t int_fast24_t;
 typedef uint24_t uint_fast24_t;
+# undef __int_least16_t
 # define __int_least16_t int24_t
+# undef __uint_least16_t
 # define __uint_least16_t uint24_t
+# undef __int_least8_t
 # define __int_least8_t int24_t

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: clang/lib/Headers/stdint.h:99-100
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t

iana wrote:
> iana wrote:
> > What are you seeing that's defining `__int_least64_t` and all these other 
> > ones by the time you get here? It's surprising to me that so much 
> > preprocessor state exists when you hit this point considering that this 
> > file doesn't include anything else. Is this some kind of artifact with how 
> > the OS module map is making a module for stdint.h?
> Oh I see, it's not these ones that are the problem, it's the redefinitions 
> below. I guess it's a bigger change, but I wonder if flipping the order would 
> be cleaner? e.g.
> ```
> #ifdef __INT32_TYPE__
> # define __int_least32_t int32_t
> #endif
> 
> #ifdef __INT64_TYPE__
> # ifndef __int_least32_t
> #  define __int_least32_t int64_t
> # endif
> #endif
> ```
> 
> I guess it's an extra line per type doing it that way, but maybe it's more 
> obvious? (Maybe I just don't do a lot of preprocessor programming, but 
> `#undef` feels like a code smell to me)
I'm a little reluctant to change this file too much since it's tedious and 
error-prone, but yeah if we can agree on a solution, I can go ahead and make 
the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

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

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 449458.
ddcc added a comment.

Drop changes to CUDA/HIP/OpenMP headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/float.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdatomic.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/velintrin.h

Index: clang/lib/Headers/velintrin.h
===
--- clang/lib/Headers/velintrin.h
+++ clang/lib/Headers/velintrin.h
@@ -13,7 +13,7 @@
 typedef double __vr __attribute__((__vector_size__(2048)));
 
 // Vector mask registers
-#if __STDC_VERSION__ >= 199901L
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // For C99
 typedef _Bool __vm__attribute__((ext_vector_type(256)));
 typedef _Bool __vm256 __attribute__((ext_vector_type(256)));
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -13,7 +13,7 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
-#if __STDC_VERSION__ > 201710L &&  \
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
including the header file in C2x is also deprecated and we do not want to
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -96,13 +96,21 @@
 typedef __INT64_TYPE__ int64_t;
 # endif /* __int8_t_defined */
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t
 # define __uint_least64_t uint64_t
+# undef __int_least32_t
 # define __int_least32_t int64_t
+# undef __uint_least32_t
 # define __uint_least32_t uint64_t
+# undef __int_least16_t
 # define __int_least16_t int64_t
+# undef __uint_least16_t
 # define __uint_least16_t uint64_t
+# undef __int_least8_t
 # define __int_least8_t int64_t
+# undef __uint_least8_t
 # define __uint_least8_t uint64_t
 #endif /* __INT64_TYPE__ */
 
@@ -120,11 +128,17 @@
 typedef uint56_t uint_least56_t;
 typedef int56_t int_fast56_t;
 typedef uint56_t uint_fast56_t;
+# undef __int_least32_t
 # define __int_least32_t int56_t
+# undef __uint_least32_t
 # define __uint_least32_t uint56_t
+# undef __int_least16_t
 # define __int_least16_t int56_t
+# undef __uint_least16_t
 # define __uint_least16_t uint56_t
+# undef __int_least8_t
 # define __int_least8_t int56_t
+# undef __uint_least8_t
 # define __uint_least8_t uint56_t
 #endif /* __INT56_TYPE__ */
 
@@ -136,11 +150,17 @@
 typedef uint48_t uint_least48_t;
 typedef int48_t int_fast48_t;
 typedef uint48_t uint_fast48_t;
+# undef __int_least32_t
 # define __int_least32_t int48_t
+# undef __uint_least32_t
 # define __uint_least32_t uint48_t
+# undef __int_least16_t
 # define __int_least16_t int48_t
+# undef __uint_least16_t
 # define __uint_least16_t uint48_t
+# undef __int_least8_t
 # define __int_least8_t int48_t
+# undef __uint_least8_t
 # define __uint_least8_t uint48_t
 #endif /* __INT48_TYPE__ */
 
@@ -152,11 +172,17 @@
 typedef uint40_t uint_least40_t;
 typedef int40_t int_fast40_t;
 typedef uint40_t uint_fast40_t;
+# undef __int_least32_t
 # define __int_least32_t int40_t
+# undef __uint_least32_t
 # define __uint_least32_t uint40_t
+# undef __int_least16_t
 # define __int_least16_t int40_t
+# undef __uint_least16_t
 # define __uint_least16_t uint40_t
+# undef __int_least8_t
 # define __int_least8_t int40_t
+# undef __uint_least8_t
 # define __uint_least8_t uint40_t
 #endif /* __INT40_TYPE__ */
 
@@ -172,11 +198,17 @@
 typedef __UINT32_TYPE__ uint32_t;
 # endif /* __uint32_t_defined */
 
+# undef __int_least32_t
 # define __int_least32_t int32_t
+# undef __uint_least32_t
 # define __uint_least32_t uint32_t
+# undef __int_least16_t
 # define __int_least16_t int32_t
+# undef __uint_least16_t
 # define __uint_least16_t uint32_t
+# undef __int_least8_t
 # define __int_least8_t int32_t
+# undef __uint_least8_t
 # define __uint_least8_t uint32_t
 #endif /* __INT32_TYPE__ */
 
@@ -194,9 +226,13 @@
 typedef uint24_t uint_least24_t;
 typedef int24_t int_fast24_t;
 typedef uint24_t uint_fast24_t;
+# undef __int_least16_t
 # define __int_least16_t int24_t
+# undef __uint_least16_t
 # define __uint_least16_t uint24_t
+# undef __int_least8_t
 # define __int_least8_t int24_t
+# undef __uint_least8_t
 # define __uint_least8_t uint24_t
 #endif /* __INT24_TYPE__ */
 
@@ -205,9 +241,13 @@
 typedef __INT16_TYPE__ int16_t;
 #endif /* __int8_t_defined */
 typedef __UINT16_TYPE__ uint16_t;
+# 

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: clang/lib/Headers/__clang_hip_cmath.h:381
 // decltype is only available in C++11 and above.
-#if __cplusplus >= 201103L
+#if defined(__cplusplus) && __cplusplus >= 201103L
 // __hip_promote

tra wrote:
> HIP headers are also not expected to compile w/o C++ and we may want a 
> `#error`here, too. 
> 
> Actually, I'm not even sure if we shopuld/need to add `#if 
> defined(__cplusplus)`in files that are clearly C++ -- undefined macro warning 
> will be the least of one's worries if they attempt to compile it w/o C++.
> 
> Perhaps it would make sense to limit these changes to the headers intended to 
> be used from both C and C++? Both CUDA and HIP are expected to never be 
> included from a non-C++ compilation and will never trigger the undefined 
> macro warning on `__cplusplus`.
> 
> @yaxunl - FYI.
Sure, I'm fine with leaving those headers alone since I'm not familiar with 
them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

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

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 449403.
ddcc added a comment.

Error out on undef __cplusplus in CUDA headers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/__clang_cuda_builtin_vars.h
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Headers/__clang_hip_cmath.h
  clang/lib/Headers/__clang_hip_math.h
  clang/lib/Headers/cuda_wrappers/algorithm
  clang/lib/Headers/cuda_wrappers/complex
  clang/lib/Headers/cuda_wrappers/new
  clang/lib/Headers/float.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/openmp_wrappers/new
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdatomic.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/velintrin.h

Index: clang/lib/Headers/velintrin.h
===
--- clang/lib/Headers/velintrin.h
+++ clang/lib/Headers/velintrin.h
@@ -13,7 +13,7 @@
 typedef double __vr __attribute__((__vector_size__(2048)));
 
 // Vector mask registers
-#if __STDC_VERSION__ >= 199901L
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // For C99
 typedef _Bool __vm__attribute__((ext_vector_type(256)));
 typedef _Bool __vm256 __attribute__((ext_vector_type(256)));
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -13,7 +13,7 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
-#if __STDC_VERSION__ > 201710L &&  \
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
including the header file in C2x is also deprecated and we do not want to
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -96,13 +96,21 @@
 typedef __INT64_TYPE__ int64_t;
 # endif /* __int8_t_defined */
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t
 # define __uint_least64_t uint64_t
+# undef __int_least32_t
 # define __int_least32_t int64_t
+# undef __uint_least32_t
 # define __uint_least32_t uint64_t
+# undef __int_least16_t
 # define __int_least16_t int64_t
+# undef __uint_least16_t
 # define __uint_least16_t uint64_t
+# undef __int_least8_t
 # define __int_least8_t int64_t
+# undef __uint_least8_t
 # define __uint_least8_t uint64_t
 #endif /* __INT64_TYPE__ */
 
@@ -120,11 +128,17 @@
 typedef uint56_t uint_least56_t;
 typedef int56_t int_fast56_t;
 typedef uint56_t uint_fast56_t;
+# undef __int_least32_t
 # define __int_least32_t int56_t
+# undef __uint_least32_t
 # define __uint_least32_t uint56_t
+# undef __int_least16_t
 # define __int_least16_t int56_t
+# undef __uint_least16_t
 # define __uint_least16_t uint56_t
+# undef __int_least8_t
 # define __int_least8_t int56_t
+# undef __uint_least8_t
 # define __uint_least8_t uint56_t
 #endif /* __INT56_TYPE__ */
 
@@ -136,11 +150,17 @@
 typedef uint48_t uint_least48_t;
 typedef int48_t int_fast48_t;
 typedef uint48_t uint_fast48_t;
+# undef __int_least32_t
 # define __int_least32_t int48_t
+# undef __uint_least32_t
 # define __uint_least32_t uint48_t
+# undef __int_least16_t
 # define __int_least16_t int48_t
+# undef __uint_least16_t
 # define __uint_least16_t uint48_t
+# undef __int_least8_t
 # define __int_least8_t int48_t
+# undef __uint_least8_t
 # define __uint_least8_t uint48_t
 #endif /* __INT48_TYPE__ */
 
@@ -152,11 +172,17 @@
 typedef uint40_t uint_least40_t;
 typedef int40_t int_fast40_t;
 typedef uint40_t uint_fast40_t;
+# undef __int_least32_t
 # define __int_least32_t int40_t
+# undef __uint_least32_t
 # define __uint_least32_t uint40_t
+# undef __int_least16_t
 # define __int_least16_t int40_t
+# undef __uint_least16_t
 # define __uint_least16_t uint40_t
+# undef __int_least8_t
 # define __int_least8_t int40_t
+# undef __uint_least8_t
 # define __uint_least8_t uint40_t
 #endif /* __INT40_TYPE__ */
 
@@ -172,11 +198,17 @@
 typedef __UINT32_TYPE__ uint32_t;
 # endif /* __uint32_t_defined */
 
+# undef __int_least32_t
 # define __int_least32_t int32_t
+# undef __uint_least32_t
 # define __uint_least32_t uint32_t
+# undef __int_least16_t
 # define __int_least16_t int32_t
+# undef __uint_least16_t
 # define __uint_least16_t uint32_t
+# undef __int_least8_t
 # define __int_least8_t int32_t
+# undef __uint_least8_t
 # define __uint_least8_t uint32_t
 #endif /* __INT32_TYPE__ */
 
@@ -194,9 +226,13 @@
 typedef uint24_t uint_least24_t;
 typedef int24_t int_fast24_t;
 typedef uint24_t uint_fast24_t;
+# 

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_builtin_vars.h:37
 
-#if __cplusplus >= 201103L
+#if defined(__cplusplus) && __cplusplus >= 201103L
 #define __DELETE =delete

tra wrote:
> Are there actual use cases where CUDA headers are used from a non-C++ 
> compilation? I do not think they will compile with non-C++ compilation at all.
> 
> I'm curious how we managed to run into a situation where CUDA headers may be 
> used w/o C++. 
> 
> I think a better fix here and other CUDA headers may be to add 
> ```
> #if !defined(__cplusplus)
> #error CUDA headers must be used from a CUDA/C++ compilation.
> #endif
> ```
Sure, I can do that. I'm not familiar with CUDA or its headers, so I previously 
just updated them all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

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

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 449118.
ddcc added a comment.

Undef macros instead, handle other header files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/__clang_cuda_builtin_vars.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Headers/__clang_hip_cmath.h
  clang/lib/Headers/__clang_hip_math.h
  clang/lib/Headers/cuda_wrappers/algorithm
  clang/lib/Headers/cuda_wrappers/complex
  clang/lib/Headers/cuda_wrappers/new
  clang/lib/Headers/float.h
  clang/lib/Headers/limits.h
  clang/lib/Headers/openmp_wrappers/new
  clang/lib/Headers/stdarg.h
  clang/lib/Headers/stdatomic.h
  clang/lib/Headers/stdbool.h
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h
  clang/lib/Headers/stdnoreturn.h
  clang/lib/Headers/velintrin.h

Index: clang/lib/Headers/velintrin.h
===
--- clang/lib/Headers/velintrin.h
+++ clang/lib/Headers/velintrin.h
@@ -13,7 +13,7 @@
 typedef double __vr __attribute__((__vector_size__(2048)));
 
 // Vector mask registers
-#if __STDC_VERSION__ >= 199901L
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
 // For C99
 typedef _Bool __vm__attribute__((ext_vector_type(256)));
 typedef _Bool __vm256 __attribute__((ext_vector_type(256)));
Index: clang/lib/Headers/stdnoreturn.h
===
--- clang/lib/Headers/stdnoreturn.h
+++ clang/lib/Headers/stdnoreturn.h
@@ -13,7 +13,7 @@
 #define noreturn _Noreturn
 #define __noreturn_is_defined 1
 
-#if __STDC_VERSION__ > 201710L &&  \
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ > 201710L) &&   \
 !defined(_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS)
 /* The noreturn macro is deprecated in C2x. We do not mark it as such because
including the header file in C2x is also deprecated and we do not want to
Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -96,13 +96,21 @@
 typedef __INT64_TYPE__ int64_t;
 # endif /* __int8_t_defined */
 typedef __UINT64_TYPE__ uint64_t;
+# undef __int_least64_t
 # define __int_least64_t int64_t
+# undef __uint_least64_t
 # define __uint_least64_t uint64_t
+# undef __int_least32_t
 # define __int_least32_t int64_t
+# undef __uint_least32_t
 # define __uint_least32_t uint64_t
+# undef __int_least16_t
 # define __int_least16_t int64_t
+# undef __uint_least16_t
 # define __uint_least16_t uint64_t
+# undef __int_least8_t
 # define __int_least8_t int64_t
+# undef __uint_least8_t
 # define __uint_least8_t uint64_t
 #endif /* __INT64_TYPE__ */
 
@@ -120,11 +128,17 @@
 typedef uint56_t uint_least56_t;
 typedef int56_t int_fast56_t;
 typedef uint56_t uint_fast56_t;
+# undef __int_least32_t
 # define __int_least32_t int56_t
+# undef __uint_least32_t
 # define __uint_least32_t uint56_t
+# undef __int_least16_t
 # define __int_least16_t int56_t
+# undef __uint_least16_t
 # define __uint_least16_t uint56_t
+# undef __int_least8_t
 # define __int_least8_t int56_t
+# undef __uint_least8_t
 # define __uint_least8_t uint56_t
 #endif /* __INT56_TYPE__ */
 
@@ -136,11 +150,17 @@
 typedef uint48_t uint_least48_t;
 typedef int48_t int_fast48_t;
 typedef uint48_t uint_fast48_t;
+# undef __int_least32_t
 # define __int_least32_t int48_t
+# undef __uint_least32_t
 # define __uint_least32_t uint48_t
+# undef __int_least16_t
 # define __int_least16_t int48_t
+# undef __uint_least16_t
 # define __uint_least16_t uint48_t
+# undef __int_least8_t
 # define __int_least8_t int48_t
+# undef __uint_least8_t
 # define __uint_least8_t uint48_t
 #endif /* __INT48_TYPE__ */
 
@@ -152,11 +172,17 @@
 typedef uint40_t uint_least40_t;
 typedef int40_t int_fast40_t;
 typedef uint40_t uint_fast40_t;
+# undef __int_least32_t
 # define __int_least32_t int40_t
+# undef __uint_least32_t
 # define __uint_least32_t uint40_t
+# undef __int_least16_t
 # define __int_least16_t int40_t
+# undef __uint_least16_t
 # define __uint_least16_t uint40_t
+# undef __int_least8_t
 # define __int_least8_t int40_t
+# undef __uint_least8_t
 # define __uint_least8_t uint40_t
 #endif /* __INT40_TYPE__ */
 
@@ -172,11 +198,17 @@
 typedef __UINT32_TYPE__ uint32_t;
 # endif /* __uint32_t_defined */
 
+# undef __int_least32_t
 # define __int_least32_t int32_t
+# undef __uint_least32_t
 # define __uint_least32_t uint32_t
+# undef __int_least16_t
 # define __int_least16_t int32_t
+# undef __uint_least16_t
 # define __uint_least16_t uint32_t
+# undef __int_least8_t
 # define __int_least8_t int32_t
+# undef __uint_least8_t
 # define __uint_least8_t uint32_t
 #endif /* __INT32_TYPE__ */
 
@@ -194,9 +226,13 @@
 typedef uint24_t uint_least24_t;
 typedef int24_t int_fast24_t;
 typedef uint24_t uint_fast24_t;
+# undef __int_least16_t
 # define 

[PATCH] D130800: [clang][Headers] Avoid compiler warnings in builtin headers

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added reviewers: efriedma, rnk, aaron.ballman.
Herald added a project: All.
ddcc requested review of this revision.
Herald added a project: clang.

While debugging module support using -Wsystem-headers, we discovered that if
-Werror, and -Wundef or -Wmacro-redefined are specified, they can cause errors
to be generated in these builtin headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130800

Files:
  clang/lib/Headers/stddef.h
  clang/lib/Headers/stdint.h


Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -91,6 +91,9 @@
  * defined if there exists an exact-width type of equal or greater width.
  */
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmacro-redefined"
+
 #ifdef __INT64_TYPE__
 # ifndef __int8_t_defined /* glibc sys/types.h also defines int64_t*/
 typedef __INT64_TYPE__ int64_t;
@@ -857,5 +860,7 @@
 #define WINT_WIDTH   __WINT_WIDTH__
 #endif
 
+#pragma GCC diagnostic pop
+
 #endif /* __STDC_HOSTED__ */
 #endif /* __CLANG_STDINT_H */
Index: clang/lib/Headers/stddef.h
===
--- clang/lib/Headers/stddef.h
+++ clang/lib/Headers/stddef.h
@@ -98,7 +98,8 @@
 #endif /* defined(__need_NULL) */
 
 #if defined(__need_STDDEF_H_misc)
-#if __STDC_VERSION__ >= 201112L || __cplusplus >= 201103L
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) ||  
\
+(defined(__cplusplus) && __cplusplus >= 201103L)
 #include "__stddef_max_align_t.h"
 #endif
 #define offsetof(t, d) __builtin_offsetof(t, d)


Index: clang/lib/Headers/stdint.h
===
--- clang/lib/Headers/stdint.h
+++ clang/lib/Headers/stdint.h
@@ -91,6 +91,9 @@
  * defined if there exists an exact-width type of equal or greater width.
  */
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmacro-redefined"
+
 #ifdef __INT64_TYPE__
 # ifndef __int8_t_defined /* glibc sys/types.h also defines int64_t*/
 typedef __INT64_TYPE__ int64_t;
@@ -857,5 +860,7 @@
 #define WINT_WIDTH   __WINT_WIDTH__
 #endif
 
+#pragma GCC diagnostic pop
+
 #endif /* __STDC_HOSTED__ */
 #endif /* __CLANG_STDINT_H */
Index: clang/lib/Headers/stddef.h
===
--- clang/lib/Headers/stddef.h
+++ clang/lib/Headers/stddef.h
@@ -98,7 +98,8 @@
 #endif /* defined(__need_NULL) */
 
 #if defined(__need_STDDEF_H_misc)
-#if __STDC_VERSION__ >= 201112L || __cplusplus >= 201103L
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L) ||  \
+(defined(__cplusplus) && __cplusplus >= 201103L)
 #include "__stddef_max_align_t.h"
 #endif
 #define offsetof(t, d) __builtin_offsetof(t, d)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc10248829357: Add test for disabling Dead Virtual Function 
Elimination (authored by ddcc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

Files:
  clang/test/CodeGenCXX/virtual-function-elimination.cpp


Index: clang/test/CodeGenCXX/virtual-function-elimination.cpp
===
--- clang/test/CodeGenCXX/virtual-function-elimination.cpp
+++ clang/test/CodeGenCXX/virtual-function-elimination.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -flto -flto-unit 
-fvirtual-function-elimination -fwhole-program-vtables -emit-llvm -o - %s | 
FileCheck %s
-
+// RUN: %clang -target x86_64-unknown-linux -flto 
-fvirtual-function-elimination -fno-virtual-function-elimination 
-fwhole-program-vtables -S -emit-llvm -o - %s | FileCheck %s -check-prefix=NOVFE
 
 struct __attribute__((visibility("default"))) A {
   virtual void foo();
@@ -8,9 +8,13 @@
 void test_1(A *p) {
   // A has default visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_1P1A
+// NOVFE-LABEL: define dso_local void @_Z6test_1P1A
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -22,9 +26,13 @@
 void test_2(B *p) {
   // B has public LTO visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_2P1B
+// NOVFE-LABEL: define dso_local void @_Z6test_2P1B
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -37,10 +45,14 @@
 void test_3(C *p) {
   // C has hidden visibility, so we generate type.checked.load to allow VFE.
 // CHECK-LABEL: define void @_Z6test_3P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_3P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 0, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -48,10 +60,14 @@
   // When using type.checked.load, we pass the vtable offset to the intrinsic,
   // rather than adding it to the pointer with a GEP.
 // CHECK-LABEL: define void @_Z6test_4P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_4P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 8, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->bar();
 }
 
@@ -64,12 +80,17 @@
   // function pointer to the intrinsic, this information would be lost. No
   // codegen changes on the non-virtual side.
 // CHECK-LABEL: define void @_Z6test_5P1CMS_FvvE(
+// NOVFE-LABEL: define dso_local void @_Z6test_5P1CMS_FvvE(
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr i8, i8* %vtable, i64 {{%.+}}
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* 
[[FN_PTR_ADDR]], i32 0, metadata !"_ZTSM1CFvvE.virtual")
+// NOVFE-NOT: call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, i32 0, 
metadata !"_ZTSM1CFvvE.virtual")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 
 // CHECK: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
+// NOVFE: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
 // CHECK: call void [[PHI]](
+// NOVFE: call void 

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Hmm, looks like this was already fixed by 
e5158b52730d323bb8cd2cba6dc6c89b90cba452 
. I guess 
I'll just commit the test then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

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

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 296826.
ddcc added a comment.

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

Files:
  clang/test/CodeGenCXX/virtual-function-elimination.cpp


Index: clang/test/CodeGenCXX/virtual-function-elimination.cpp
===
--- clang/test/CodeGenCXX/virtual-function-elimination.cpp
+++ clang/test/CodeGenCXX/virtual-function-elimination.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -flto -flto-unit 
-fvirtual-function-elimination -fwhole-program-vtables -emit-llvm -o - %s | 
FileCheck %s
-
+// RUN: %clang -target x86_64-unknown-linux -flto 
-fvirtual-function-elimination -fno-virtual-function-elimination 
-fwhole-program-vtables -S -emit-llvm -o - %s | FileCheck %s -check-prefix=NOVFE
 
 struct __attribute__((visibility("default"))) A {
   virtual void foo();
@@ -8,9 +8,13 @@
 void test_1(A *p) {
   // A has default visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_1P1A
+// NOVFE-LABEL: define dso_local void @_Z6test_1P1A
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -22,9 +26,13 @@
 void test_2(B *p) {
   // B has public LTO visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_2P1B
+// NOVFE-LABEL: define dso_local void @_Z6test_2P1B
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -37,10 +45,14 @@
 void test_3(C *p) {
   // C has hidden visibility, so we generate type.checked.load to allow VFE.
 // CHECK-LABEL: define void @_Z6test_3P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_3P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 0, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -48,10 +60,14 @@
   // When using type.checked.load, we pass the vtable offset to the intrinsic,
   // rather than adding it to the pointer with a GEP.
 // CHECK-LABEL: define void @_Z6test_4P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_4P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 8, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->bar();
 }
 
@@ -64,12 +80,17 @@
   // function pointer to the intrinsic, this information would be lost. No
   // codegen changes on the non-virtual side.
 // CHECK-LABEL: define void @_Z6test_5P1CMS_FvvE(
+// NOVFE-LABEL: define dso_local void @_Z6test_5P1CMS_FvvE(
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr i8, i8* %vtable, i64 {{%.+}}
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* 
[[FN_PTR_ADDR]], i32 0, metadata !"_ZTSM1CFvvE.virtual")
+// NOVFE-NOT: call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, i32 0, 
metadata !"_ZTSM1CFvvE.virtual")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 
 // CHECK: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
+// NOVFE: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
 // CHECK: call void [[PHI]](
+// NOVFE: call void [[PHI]](
   (p->*q)();
 }


Index: clang/test/CodeGenCXX/virtual-function-elimination.cpp
===
--- 

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 296782.
ddcc added a comment.

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

Files:
  clang/include/clang/Driver/Options.td
  clang/test/CodeGenCXX/virtual-function-elimination.cpp


Index: clang/test/CodeGenCXX/virtual-function-elimination.cpp
===
--- clang/test/CodeGenCXX/virtual-function-elimination.cpp
+++ clang/test/CodeGenCXX/virtual-function-elimination.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux -flto -flto-unit 
-fvirtual-function-elimination -fwhole-program-vtables -emit-llvm -o - %s | 
FileCheck %s
-
+// RUN: %clang -target x86_64-unknown-linux -flto 
-fno-virtual-function-elimination -fwhole-program-vtables -S -emit-llvm -o - %s 
| FileCheck %s -check-prefix=NOVFE
 
 struct __attribute__((visibility("default"))) A {
   virtual void foo();
@@ -8,9 +8,13 @@
 void test_1(A *p) {
   // A has default visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_1P1A
+// NOVFE-LABEL: define dso_local void @_Z6test_1P1A
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.A*)*, 
void (%struct.A*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.A*)*, void (%struct.A*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -22,9 +26,13 @@
 void test_2(B *p) {
   // B has public LTO visibility, so no need for type.checked.load.
 // CHECK-LABEL: define void @_Z6test_2P1B
+// NOVFE-LABEL: define dso_local void @_Z6test_2P1B
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
+// NOVFE: [[FN_PTR_ADDR:%.+]] = getelementptr inbounds void (%struct.B*)*, 
void (%struct.B*)** {{%.+}}, i64 0
 // CHECK: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.B*)*, void (%struct.B*)** 
[[FN_PTR_ADDR]]
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[FN_PTR]](
   p->foo();
 }
 
@@ -37,10 +45,14 @@
 void test_3(C *p) {
   // C has hidden visibility, so we generate type.checked.load to allow VFE.
 // CHECK-LABEL: define void @_Z6test_3P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_3P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 0, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[LOAD:%.+]] = load void (%struct.C*)*, void (%struct.C*)** {{%.+}}, 
align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[LOAD]](
   p->foo();
 }
 
@@ -48,10 +60,14 @@
   // When using type.checked.load, we pass the vtable offset to the intrinsic,
   // rather than adding it to the pointer with a GEP.
 // CHECK-LABEL: define void @_Z6test_4P1C
+// NOVFE-LABEL: define dso_local void @_Z6test_4P1C
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, 
i32 8, metadata !"_ZTS1C")
+// NOVFE: call i1 @llvm.type.test(i8* {{%.+}}, metadata !"_ZTS1C")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
+// NOVFE: [[LOAD:%.+]] = load void (%struct.C*)*, void (%struct.C*)** {{%.+}}, 
align 8
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
 // CHECK: call void [[FN_PTR]](
+// NOVFE: call void [[LOAD]](
   p->bar();
 }
 
@@ -64,12 +80,17 @@
   // function pointer to the intrinsic, this information would be lost. No
   // codegen changes on the non-virtual side.
 // CHECK-LABEL: define void @_Z6test_5P1CMS_FvvE(
+// NOVFE-LABEL: define dso_local void @_Z6test_5P1CMS_FvvE(
 // CHECK: [[FN_PTR_ADDR:%.+]] = getelementptr i8, i8* %vtable, i64 {{%.+}}
 // CHECK: [[LOAD:%.+]] = call { i8*, i1 } @llvm.type.checked.load(i8* 
[[FN_PTR_ADDR]], i32 0, metadata !"_ZTSM1CFvvE.virtual")
+// NOVFE-NOT: call { i8*, i1 } @llvm.type.checked.load(i8* {{%.+}}, i32 0, 
metadata !"_ZTSM1CFvvE.virtual")
 // CHECK: [[FN_PTR_I8:%.+]] = extractvalue { i8*, i1 } [[LOAD]], 0
 // CHECK: [[FN_PTR:%.+]] = bitcast i8* [[FN_PTR_I8]] to void (%struct.C*)*
+// NOVFE: [[FN_PTR:%.+]] = load void (%struct.C*)*, void (%struct.C*)** 
{{%.+}}, align 8
 
 // CHECK: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
+// NOVFE: [[PHI:%.+]] = phi void (%struct.C*)* {{.*}}[ [[FN_PTR]], {{.*}} ]
 // CHECK: call void [[PHI]](
+// NOVFE: call void [[PHI]](
   (p->*q)();
 }
Index: clang/include/clang/Driver/Options.td
===
--- 

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88349

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

[PATCH] D88349: Fix inconsistent flag for disabling Dead Virtual Function Elimination

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added a reviewer: ostannard.
Herald added a subscriber: dang.
Herald added a project: clang.
ddcc requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88349

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1975,7 +1975,7 @@
 def fvirtual_function_elimination : Flag<["-"], 
"fvirtual-function-elimination">, Group,
   Flags<[CoreOption, CC1Option]>,
   HelpText<"Enables dead virtual function elimination optimization. Requires 
-flto=full">;
-def fno_virtual_function_elimination : Flag<["-"], 
"fno-virtual-function_elimination">, Group,
+def fno_virtual_function_elimination : Flag<["-"], 
"fno-virtual-function-elimination">, Group,
   Flags<[CoreOption]>;
 
 def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>,


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1975,7 +1975,7 @@
 def fvirtual_function_elimination : Flag<["-"], "fvirtual-function-elimination">, Group,
   Flags<[CoreOption, CC1Option]>,
   HelpText<"Enables dead virtual function elimination optimization. Requires -flto=full">;
-def fno_virtual_function_elimination : Flag<["-"], "fno-virtual-function_elimination">, Group,
+def fno_virtual_function_elimination : Flag<["-"], "fno-virtual-function-elimination">, Group,
   Flags<[CoreOption]>;
 
 def fwrapv : Flag<["-"], "fwrapv">, Group, Flags<[CC1Option]>,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77704: [gold] Add support for loading pass plugins

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

ping, any feedback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77704



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

[PATCH] D77704: [gold] Add support for loading pass plugins

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In order to reload the gold plugin, I'm modifying the Clang driver to pass in 
our own path as a separate argument, which is the most generic approach. 
Another method would be to use e.g. `dladdr()` to grab our own path from one of 
our exported functions, but that method appears to be a glibc extension which 
isn't cross-platform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77704



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

[PATCH] D77704: [gold] Add support for loading pass plugins

[Dominic Chen via Phabricator via cfe-commits]

ddcc marked an inline comment as done.
ddcc added a comment.

Thanks, seems to be working now for statically-built passes on a default LLVM 
build without `LLVM_LINK_LLVM_DYLIB`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77704



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

[PATCH] D77704: [gold] Add support for loading pass plugins

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 259981.
ddcc added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Support statically-built passes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77704

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  llvm/tools/gold/CMakeLists.txt
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/gold/gold.exports


Index: llvm/tools/gold/gold.exports
===
--- llvm/tools/gold/gold.exports
+++ /dev/null
@@ -1 +0,0 @@
-onload
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Object/Error.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -207,6 +208,12 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // List of new PM pass plugins
+  static std::vector pass_plugins;
+  // Path to ourselves, and whether we have been reloaded for pass plugin 
symbol
+  // resolution
+  static std::string self;
+  static bool self_reloaded = false;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -300,6 +307,30 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt.consume_front("self=")) {
+  // Store our own path, in case we need to be reloaded
+  self = std::string(opt);
+} else if (opt.consume_front("load=")) {
+  std::string Error, Path(opt);
+
+  // This plugin is loaded by gold under RTLD_LOCAL, which means that our
+  // LLVM symbols are not available to subsequent pass plugins. Since this
+  // will break statically-built pass plugins, we reload with RTLD_GLOBAL.
+  if (!self_reloaded) {
+if (self.size()) {
+  if (sys::DynamicLibrary::LoadLibraryPermanently(self.c_str(), 
))
+message(LDPL_ERROR, "Unable to reload LLVM gold plugin: %s",
+Error.c_str());
+  else
+self_reloaded = true;
+} else
+  message(LDPL_ERROR, "Unable to retrieve path to LLVM gold plugin!");
+  }
+
+  if (sys::DynamicLibrary::LoadLibraryPermanently(Path.c_str(), ))
+message(LDPL_ERROR, "Unable to load plugin: %s", Error.c_str());
+} else if (opt.consume_front("load-pass-plugin=")) {
+  pass_plugins.push_back(opt.data());
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -935,6 +966,8 @@
   Conf.UseNewPM = options::new_pass_manager;
   // Debug new pass manager if requested
   Conf.DebugPassManager = options::debug_pass_manager;
+  // Pass plugins to load
+  Conf.PassPlugins = std::move(options::pass_plugins);
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
Index: llvm/tools/gold/CMakeLists.txt
===
--- llvm/tools/gold/CMakeLists.txt
+++ llvm/tools/gold/CMakeLists.txt
@@ -1,5 +1,3 @@
-set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/gold.exports)
-
 if( LLVM_ENABLE_PIC AND LLVM_BINUTILS_INCDIR )
   include_directories( ${LLVM_BINUTILS_INCDIR} )
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -379,6 +379,9 @@
 Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Plugin));
+
+// Pass in our own path in case we need to reload
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=self=") + Plugin));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to


Index: llvm/tools/gold/gold.exports
===
--- llvm/tools/gold/gold.exports
+++ /dev/null
@@ -1 +0,0 @@
-onload
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -23,6 +23,7 @@
 #include "llvm/Object/Error.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -207,6 +208,12 @@
   static std::string 

[PATCH] D77705: [Driver] Forward pass plugin arguments to gold

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Originally, I tried forwarding the `-Xclang -load` arguments, but couldn't 
access the `options::OPT_plugin` arguments from the argument list. I'm not 
familiar with tablegen and argument processing, there was some issue with the 
group not being available causing the arguments to be skipped when filtering 
the argument list, so I ended up reusing the `OPT_fplugin` and 
`OPT_fpass_plugin` arguments instead. As far as I can tell, `OPT_fplugin` seems 
to work for both Clang and LLVM plugins, but I haven't used the former.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77705



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

[PATCH] D77705: [Driver] Forward pass plugin arguments to gold

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added a reviewer: tejohnson.
Herald added a project: clang.

Support forwarding `-fplugin` and `-fpass-plugin` arguments for loading pass 
plugins

Depends on: D77704 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77705

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -474,10 +474,18 @@
 ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
 if (Path.empty() || llvm::sys::fs::is_directory(Path))
   llvm::sys::path::append(Path, "default.profdata");
-CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") 
+
- Path));
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") + Path));
   }
 
+  // Forward arguments for loading plugins (old/new PM)
+  for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ))
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=load=") + A->getValue()));
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-plugin-opt=load-pass-plugin=") + A->getValue()));
+
   // Need this flag to turn on new pass manager via Gold plugin.
   if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
options::OPT_fno_experimental_new_pass_manager,


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -474,10 +474,18 @@
 ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
 if (Path.empty() || llvm::sys::fs::is_directory(Path))
   llvm::sys::path::append(Path, "default.profdata");
-CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") +
- Path));
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") + Path));
   }
 
+  // Forward arguments for loading plugins (old/new PM)
+  for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ))
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=load=") + A->getValue()));
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ))
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-plugin-opt=load-pass-plugin=") + A->getValue()));
+
   // Need this flag to turn on new pass manager via Gold plugin.
   if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
options::OPT_fno_experimental_new_pass_manager,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I don't have the time to rebase and retest this currently, so it might be 
better for someone else to take over this patch. Unfortunately, it's been long 
enough that I don't remember the details of these changes.


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

https://reviews.llvm.org/D28955



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1433646 , @mikhail.ramalho 
wrote:

> To fix that, I changed the script slightly: we first try to dinamically get 
> the Z3's version, if we fail and we are cross compiling, then we try to parse 
> the headers. Right now, it should just work but I'd like to add a message to 
> warn the user that we found the lib but we don't know if it'll link.


Thanks, sounds good to me.

In D54978#1394613 , @thakis wrote:

> mikhail.ramalho: My guess is that we need to pass on LLVM_OPTIMIZED_TABLEGEN 
> to the child cmake invocation in 
> http://llvm-cs.pcc.me.uk/cmake/modules/CrossCompile.cmake#50 (like we pass on 
> a few other variables) to fix this.


Do you know if this configuration builds fine now?


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: llvm/cmake/modules/FindZ3.cmake:92
+
+  set(Z3_VERSION_STRING ${Z3_MAJOR}.${Z3_MINOR}.${Z3_MAJOR})
+  unset(z3_version_str)

Should be ${Z3_MAJOR}.${Z3_MINOR}.${Z3_BUILD_NUMBER}


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1431935 , @delcypher wrote:

> Would one of you be able to file a bug against Z3 to fix this? I am no longer 
> in a position to contribute to Z3 so I can't do this.


I've opened https://github.com/Z3Prover/z3/issues/2184 .

In D54978#1431936 , @delcypher wrote:

> This output means you built Z3 from source that was not in a git repository. 
> In this case the header file should look the same for both Z3's CMake and 
> Python build systems.


That's strange, I have been building from a git repository.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> 2. Instead of parsing `Z3_FULL_VERSION`, we can parse `Z3_MAJOR_VERSION`, 
> `Z3_MINOR_VERSION` and `Z3_BUILD_NUMBER` which are also available in the same 
> header.


Since the differences in version string depending on the build system are 
intended behavior, it seems (2) would be preferable?


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> 2. Instead of parsing `Z3_FULL_VERSION`, we can parse `Z3_MAJOR_VERSION`, 
> `Z3_MINOR_VERSION` and `Z3_BUILD_NUMBER` which are also available in the same 
> header.


Sounds like this might be the way to go, since the format seems to be more 
stable.


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

The only relevant commit that I can find is 
https://github.com/Z3Prover/z3/commit/2cb4223979cc94e2ebc4e49a9e83adbdcd2b6979 
, but it first landed in z3 4.6.0. It looks like it's specific to CMake though, 
so is it different if you use the python build? I haven't tried the CMake build.


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1431430 , @mikhail.ramalho 
wrote:

> Sorry for the massive delay, but I just updated the `FindZ3` script to 
> retrieve the version from the lib. I changed it to use `try_run` instead of 
> `try_compile` so we can get the version number.
>
> I tried to use @brzycki code to get the version from the header, however, 
> it's not working for Z3 4.8.4. In Z3 4.8.3 the FULL_VERSION is a nice `"Z3 
> 4.8.3.0"` but in version 4.8.4 it's `"Z3 4.8.4.10272 d6df51951f4c master 
> z3-4.8.4"` and cmake fails with the following message:


Thanks for making the changes! Is this being parsed from e.g. 
`/usr/include/z3_version.h`? I checked their code, and I have a local build of 
z3 4.8.5.0, but I'm not seeing those changes to the version string:

  #define Z3_FULL_VERSION"Z3 4.8.5.0"


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1397316 , @mikhail.ramalho 
wrote:

> I just sent the first prototype of the solution. All the magic happens inside 
> the `CHECK_CXX_SOURCE_RUNS` call.


I think hardcoding the version into the binary is too brittle. Instead, your 
program can just print out the current z3 version (much like the current 
execution of the z3 binary), and the remaining logic can remain in CMake, with 
the fallbacks as suggested above by @brzycki.


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1396403 , @brzycki wrote:

> That is almost exactly what I was thinking about this morning. I'd prefer the 
> following since it's more robust for older versions:


The old `version.h` header isn't externally exposed, only the newer 
`z3_version.h` header starting with version 4.8.1. I built a copy of 4.7.1 from 
source, and I don't see it, so unfortunately I think the second check for 
`version.h` is superfluous. Instead, I think it'd be ok to still query the 
executable as the primary, then fallback to this header as the secondary?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1395425 , @brzycki wrote:

> This works for everything I could throw at it. If you think it's reasonable I 
> can open another ticket and have the code reviewed as a separate fix.


Sounds good to me, although I think it'd be good to emit a warning or at least 
a message about assuming the version due to a missing executable.

In D54978#1395561 , @brzycki wrote:

> I looked at the Z3 source and they do have a `z3_version.h` file now and was 
> added in version 4.4.2.0. You may be able to use the header directly, but I'm 
> not sure how `find_package()` parses for library versions and if this is 
> useful or not.  The generated header is named `src/util/version.h` in this 
> initial commit:
>  
> https://github.com/Z3Prover/z3/commit/251527603df01904f70ed884f8695fedab5caed9


You're right, it looks like it was originally internal-only, but them became 
exposed as part of the interface with the second commit, starting around the 
release of z3 4.8.1. It's been a while since I've used CMake, but perhaps 
something like this:

  if(Z3_INCLUDE_DIR AND EXISTS "${Z3_INCLUDE_DIR }/z3_version.h")
file(STRINGS "${Z3_INCLUDE_DIR }/z3_version.h" z3_version_str REGEX 
"^#define[\t ]+Z3_FULL_VERSION[\t ]+\".*\"")
  
string(REGEX REPLACE "^.*Z3_FULL_VERSION[\t ]+\"Z3 ([0-9\.]+)\".*$" "\\1" 
Z3_VERSION_STRING "${z3_version_str}")
unset(z3_version_str)
  endif()



In D54978#1395476 , @mikhail.ramalho 
wrote:

> I'm wondering if we can remove the binary requirement all together: is it 
> possible to run a small program that would return EXIT_SUCCESS if the library 
> is the correct version?


I see other projects do something similar; e.g. 
https://github.com/SRI-CSL/sally/blob/master/cmake/FindZ3.cmake#L20 . I'm less 
fond of that approach because it involves even more moving parts, but then 
again, parsing C header files with regular expressions isn't particularly 
robust either.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In D54978#1395268 , @brzycki wrote:

> I think I found why others are struggling to recreate this issue. I did not 
> have the package `z3/bionic` installed. This provides the `/usr/bin/z3` 
> executable which `llvm/cmake/modules/FindZ3.cmake` relies upon to determine 
> the correct `Z3_VERSION_STRING`.


Yeah, that's what I meant by best-effort only. Due to upstream Z3's design, 
without the binary, there is no easy way to retrieve the current installed 
version, so when I originally wrote the Z3 integration, it seemed fine to let 
the version check pass. Given the issues that have come up, it might make more 
sense to at least emit a warning, or explicitly fail if the version can't be 
determined, and prompt the user to install the binary.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added subscribers: delcypher, ddcc.
ddcc added a comment.

The likely reason for this versioning problem is that the current versioning 
implementation in FindZ3.cmake is best-effort only: among other conditions, if 
the z3 binary is available, it will execute it and parse out the version number 
from standard output, otherwise, it fails silently. This is because upstream Z3 
doesn't define the API version in a header file, and uses a homebrew 
python-based build system that also doesn't export the version. I believe 
@delcypher 's CMake-based build system for upstream Z3 might solve this 
problem, but I haven't looked at it in a long time, and it it appears to be 
stalled ( https://github.com/Z3Prover/z3/issues/986 ).

I also agree that more notice about this patch would have been appreciated; I 
didn't hear it until I read LLVM weekly today.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D47726: [Analyzer][Z3] Test fixes for Z3 constraint manager

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> @ddcc  To be completely honest, I see a few design issues with the current 
> implementation of Z3 backend,
>  the main one being that it checks satisfiability after every single exploded 
> node.
>  To the best of my knowledge, reasonable scalability would not be achieved 
> with such an approach,
>  and I'm not sure how feasible would it be to change it without rewriting 
> most of the checkers.

I agree, though a number of these are limitations in CSA, and not specifically 
the backend.

> Thus we currently do not plan to set up a Z3 bot, but if you wish to maintain 
> we certainly can provide pointers on how this can be done.

I don't plan to set one up either. Just compiling clang/llvm is already very 
resource intensive.

> What do you think if we introduce `ninja check-clang-analyzer` to run the 
> analyzer tests, and `ninja check-clang-analyzer-z3` to run all the analyzer 
> tests with Z3?
>  [there might be another ninja target for running all analyzer tests, I just 
> don't remember which one. But at the end of the day there should be away to 
> run analyzer tests without Z3 even when Z3 is linked in]

Sounds good.


Repository:
  rC Clang

https://reviews.llvm.org/D47726



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

[PATCH] D47726: [Analyzer][Z3] Test fixes for Z3 constraint manager

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a subscriber: dcoughlin.
ddcc added a comment.

In https://reviews.llvm.org/D47726#1121395, @george.karpenkov wrote:

> - Do all tests for Z3 run when LLVM is configured to use Z3? I'm not sure if 
> that's the right thing: do all tests start to take 10x time to run once Z3 is 
> enabled?


The test approach is what @dcoughlin suggested in 
https://reviews.llvm.org/D28952, where the tests are run using each constraint 
manager that is available. At that time, he measured an increase in test time 
from 25s to 90s (see https://reviews.llvm.org/D28952#686284).

> - Is anyone interested in running a buildbot running those?

@dcoughlin mentioned that he (Apple?) would set up a buildbot for this, but I 
don't know if that happened.


Repository:
  rC Clang

https://reviews.llvm.org/D47726



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

[PATCH] D47617: [Analyzer] Fix Z3ConstraintManager crash (PR37646)

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D47617#1119257, @george.karpenkov wrote:

> LGTM with a nit on a test name.


Same.

In https://reviews.llvm.org/D47617#1119268, @NoQ wrote:

> Also does this test need to be z3-specific? We would also not like to crash 
> here without z3.


I had the same though originally about the `REQUIRES` line. Since this code 
path is very specific to Z3ConstraintManager, it didn't seem useful to run the 
test on normal buildbots. But I have no preference either way.


Repository:
  rC Clang

https://reviews.llvm.org/D47617



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

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333704: [analyzer] fix bug with 1-bit APSInt types in 
Z3ConstraintManager (authored by ddcc, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47603

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  cfe/trunk/test/Analysis/apsint.c

Index: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -987,6 +987,9 @@
   // TODO: Refactor to put elsewhere
   QualType getAPSIntType(const llvm::APSInt ) const;
 
+  // Get the QualTy for the input APSInt, and fix it if it has a bitwidth of 1.
+  std::pair fixAPSInt(const llvm::APSInt ) const;
+
   // Perform implicit type conversion on binary symbolic expressions.
   // May modify all input parameters.
   // TODO: Refactor to use built-in conversion functions
@@ -1038,27 +1041,31 @@
   // The expression may be casted, so we cannot call getZ3DataExpr() directly
   Z3Expr Exp = getZ3Expr(Sym, );
 
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType FromTy;
+  llvm::APSInt NewFromInt;
+  std::tie(NewFromInt, FromTy) = fixAPSInt(From);
+  Z3Expr FromExp = Z3Expr::fromAPSInt(NewFromInt);
 
   // Construct single (in)equality
   if (From == To)
 return assumeZ3Expr(State, Sym,
 getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE,
- FromExp, RTy, nullptr));
+ FromExp, FromTy, nullptr));
 
+  QualType ToTy;
+  llvm::APSInt NewToInt;
+  std::tie(NewToInt, ToTy) = fixAPSInt(To);
+  Z3Expr ToExp = Z3Expr::fromAPSInt(NewToInt);
+  assert(FromTy == ToTy && "Range values have different types!");
   // Construct two (in)equalities, and a logical and/or
-  Z3Expr LHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr);
+  Z3Expr LHS = getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp,
+FromTy, nullptr);
   Z3Expr RHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr);
+  getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, ToTy, nullptr);
   return assumeZ3Expr(
   State, Sym,
-  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy));
+  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS,
+RetTy->isSignedIntegerOrEnumerationType()));
 }
 
 ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State,
@@ -1145,8 +1152,8 @@
 
 const llvm::APSInt *Z3ConstraintManager::getSymVal(ProgramStateRef State,
SymbolRef Sym) const {
-  BasicValueFactory  = getBasicVals();
-  ASTContext  = BV.getContext();
+  BasicValueFactory  = getBasicVals();
+  ASTContext  = BVF.getContext();
 
   if (const SymbolData *SD = dyn_cast(Sym)) {
 QualType Ty = Sym->getType();
@@ -1180,7 +1187,7 @@
   return nullptr;
 
 // This is the only solution, store it
-return (Value);
+return (Value);
   } else if (const SymbolCast *SC = dyn_cast(Sym)) {
 SymbolRef CastSym = SC->getOperand();
 QualType CastTy = SC->getType();
@@ -1191,7 +1198,7 @@
 const llvm::APSInt *Value;
 if (!(Value = getSymVal(State, CastSym)))
   return nullptr;
-return (SC->getType(), *Value);
+return (SC->getType(), *Value);
   } else if (const BinarySymExpr *BSE = dyn_cast(Sym)) {
 const llvm::APSInt *LHS, *RHS;
 if (const SymIntExpr *SIE = dyn_cast(BSE)) {
@@ -1215,7 +1222,7 @@
 QualType LTy = getAPSIntType(*LHS), RTy = getAPSIntType(*RHS);
 doIntTypeConversion(
 ConvertedLHS, LTy, ConvertedRHS, RTy);
-return BV.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
+return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
   }
 
   llvm_unreachable("Unsupported expression to get symbol value!");
@@ -1342,13 +1349,15 @@
   BinaryOperator::Opcode Op = BSE->getOpcode();
 
   if (const SymIntExpr *SIE = dyn_cast(BSE)) {
-RTy = getAPSIntType(SIE->getRHS());
 Z3Expr LHS = getZ3SymExpr(SIE->getLHS(), , hasComparison);
-Z3Expr RHS = Z3Expr::fromAPSInt(SIE->getRHS());
+llvm::APSInt NewRInt;
+std::tie(NewRInt, RTy) = fixAPSInt(SIE->getRHS());
+Z3Expr RHS = Z3Expr::fromAPSInt(NewRInt);
 return getZ3BinExpr(LHS, LTy, Op, RHS, RTy, RetTy);
   } else if (const IntSymExpr *ISE = dyn_cast(BSE)) {
-LTy = getAPSIntType(ISE->getLHS());
-Z3Expr LHS = Z3Expr::fromAPSInt(ISE->getLHS());
+llvm::APSInt 

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 149356.
ddcc added a comment.

Add test, address comments


Repository:
  rC Clang

https://reviews.llvm.org/D47603

Files:
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/apsint.c

Index: test/Analysis/apsint.c
===
--- /dev/null
+++ test/Analysis/apsint.c
@@ -0,0 +1,7 @@
+// REQUIRES: z3
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux-gnu -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+_Bool a() {
+  return !({ a(); });
+}
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -987,6 +987,9 @@
   // TODO: Refactor to put elsewhere
   QualType getAPSIntType(const llvm::APSInt ) const;
 
+  // Get the QualTy for the input APSInt, and fix it if it has a bitwidth of 1.
+  std::pair fixAPSInt(const llvm::APSInt ) const;
+
   // Perform implicit type conversion on binary symbolic expressions.
   // May modify all input parameters.
   // TODO: Refactor to use built-in conversion functions
@@ -1038,27 +1041,31 @@
   // The expression may be casted, so we cannot call getZ3DataExpr() directly
   Z3Expr Exp = getZ3Expr(Sym, );
 
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType FromTy;
+  llvm::APSInt NewFromInt;
+  std::tie(NewFromInt, FromTy) = fixAPSInt(From);
+  Z3Expr FromExp = Z3Expr::fromAPSInt(NewFromInt);
 
   // Construct single (in)equality
   if (From == To)
 return assumeZ3Expr(State, Sym,
 getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE,
- FromExp, RTy, nullptr));
+ FromExp, FromTy, nullptr));
 
+  QualType ToTy;
+  llvm::APSInt NewToInt;
+  std::tie(NewToInt, ToTy) = fixAPSInt(To);
+  Z3Expr ToExp = Z3Expr::fromAPSInt(NewToInt);
+  assert(FromTy == ToTy && "Range values have different types!");
   // Construct two (in)equalities, and a logical and/or
-  Z3Expr LHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr);
+  Z3Expr LHS = getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp,
+FromTy, nullptr);
   Z3Expr RHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr);
+  getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, ToTy, nullptr);
   return assumeZ3Expr(
   State, Sym,
-  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy));
+  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS,
+RetTy->isSignedIntegerOrEnumerationType()));
 }
 
 ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State,
@@ -1145,8 +1152,8 @@
 
 const llvm::APSInt *Z3ConstraintManager::getSymVal(ProgramStateRef State,
SymbolRef Sym) const {
-  BasicValueFactory  = getBasicVals();
-  ASTContext  = BV.getContext();
+  BasicValueFactory  = getBasicVals();
+  ASTContext  = BVF.getContext();
 
   if (const SymbolData *SD = dyn_cast(Sym)) {
 QualType Ty = Sym->getType();
@@ -1180,7 +1187,7 @@
   return nullptr;
 
 // This is the only solution, store it
-return (Value);
+return (Value);
   } else if (const SymbolCast *SC = dyn_cast(Sym)) {
 SymbolRef CastSym = SC->getOperand();
 QualType CastTy = SC->getType();
@@ -1191,7 +1198,7 @@
 const llvm::APSInt *Value;
 if (!(Value = getSymVal(State, CastSym)))
   return nullptr;
-return (SC->getType(), *Value);
+return (SC->getType(), *Value);
   } else if (const BinarySymExpr *BSE = dyn_cast(Sym)) {
 const llvm::APSInt *LHS, *RHS;
 if (const SymIntExpr *SIE = dyn_cast(BSE)) {
@@ -1215,7 +1222,7 @@
 QualType LTy = getAPSIntType(*LHS), RTy = getAPSIntType(*RHS);
 doIntTypeConversion(
 ConvertedLHS, LTy, ConvertedRHS, RTy);
-return BV.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
+return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
   }
 
   llvm_unreachable("Unsupported expression to get symbol value!");
@@ -1342,13 +1349,15 @@
   BinaryOperator::Opcode Op = BSE->getOpcode();
 
   if (const SymIntExpr *SIE = dyn_cast(BSE)) {
-RTy = getAPSIntType(SIE->getRHS());
 Z3Expr LHS = getZ3SymExpr(SIE->getLHS(), , hasComparison);
-Z3Expr RHS = Z3Expr::fromAPSInt(SIE->getRHS());
+llvm::APSInt NewRInt;
+std::tie(NewRInt, RTy) = fixAPSInt(SIE->getRHS());
+Z3Expr RHS = Z3Expr::fromAPSInt(NewRInt);
 return getZ3BinExpr(LHS, LTy, Op, RHS, RTy, RetTy);
   } else if (const IntSymExpr 

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D47603#1118138, @vlad.tsyrklevich wrote:

> In https://reviews.llvm.org/D47603#1118106, @george.karpenkov wrote:
>
> > Would it be possible to add tests? I know we have very few unit tests, but 
> > I assume you could actually use an integration test to exercise this path?
>
>
> I tested this change and it fixes PR37622. There's a simple crash reproducer 
> included there.


Cool, thanks for the repro! It's been long enough since I've touched this code 
that I don't recall the original failing testcase. I'll add the test to this 
revision.




Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1044
   Z3Expr Exp = getZ3Expr(Sym, );
-
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType LTy;
+  llvm::APSInt NewFromInt;

george.karpenkov wrote:
> What does `L` stand for here? It's confusing because `L/R` usually stand for 
> left/right-hand-side in this context.
They correspond to the inferred left/right hand-side inferred types, but inside 
the subsequent Z3Expr `LHS` and `RHS` variables. This is confusing, so I'll 
rename them to `FromTy` and `ToTy`.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1154
+  BasicValueFactory  = getBasicVals();
+  ASTContext  = BVF.getContext();
 

george.karpenkov wrote:
> that's a separate change, but OK
Yeah, this is one of several small miscellaneous changes that didn't make the 
original commit. It seemed a bit excessive to open separate revisions for each, 
so I've just been merging them into the next patch. I'm not sure which is 
preferable?



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1426
+*Ty = getAPSIntType(Int);
+  return Int;
+}

george.karpenkov wrote:
> It's redundant to mutate the argument passed by reference and also return it.
> Could we take a single `APSInt` parameter by value and return 
> `std::pair` ?
Sure.


Repository:
  rC Clang

https://reviews.llvm.org/D47603



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

[PATCH] D47603: [analyzer] fix bug with 1-bit APSInt types in Z3ConstraintManager

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.
ddcc added reviewers: george.karpenkov, NoQ.
Herald added subscribers: a.sidorin, szepet, xazax.hun.

Clang does not have a corresponding QualType for a 1-bit APSInt, so use the 
BoolTy and extend the APSInt. Split from https://reviews.llvm.org/D35450.


Repository:
  rC Clang

https://reviews.llvm.org/D47603

Files:
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp

Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -987,6 +987,10 @@
   // TODO: Refactor to put elsewhere
   QualType getAPSIntType(const llvm::APSInt ) const;
 
+  // Fix the input APSInt if it is has a bitwidth of 1, and set the type.
+  const llvm::APSInt (const llvm::APSInt , llvm::APSInt ,
+QualType *Ty = nullptr) const;
+
   // Perform implicit type conversion on binary symbolic expressions.
   // May modify all input parameters.
   // TODO: Refactor to use built-in conversion functions
@@ -1037,28 +1041,29 @@
   QualType RetTy;
   // The expression may be casted, so we cannot call getZ3DataExpr() directly
   Z3Expr Exp = getZ3Expr(Sym, );
-
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType LTy;
+  llvm::APSInt NewFromInt;
+  Z3Expr FromExp = Z3Expr::fromAPSInt(fixAPSInt(From, NewFromInt, ));
 
   // Construct single (in)equality
   if (From == To)
 return assumeZ3Expr(State, Sym,
 getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE,
- FromExp, RTy, nullptr));
+ FromExp, LTy, nullptr));
 
+  QualType RTy;
+  llvm::APSInt NewToInt;
+  Z3Expr ToExp = Z3Expr::fromAPSInt(fixAPSInt(To, NewToInt, ));
+  assert(LTy == RTy && "Range values have different types!");
   // Construct two (in)equalities, and a logical and/or
   Z3Expr LHS =
-  getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr);
+  getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, LTy, nullptr);
   Z3Expr RHS =
   getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr);
   return assumeZ3Expr(
   State, Sym,
-  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy));
+  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS,
+RetTy->isSignedIntegerOrEnumerationType()));
 }
 
 ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State,
@@ -1145,8 +1150,8 @@
 
 const llvm::APSInt *Z3ConstraintManager::getSymVal(ProgramStateRef State,
SymbolRef Sym) const {
-  BasicValueFactory  = getBasicVals();
-  ASTContext  = BV.getContext();
+  BasicValueFactory  = getBasicVals();
+  ASTContext  = BVF.getContext();
 
   if (const SymbolData *SD = dyn_cast(Sym)) {
 QualType Ty = Sym->getType();
@@ -1180,7 +1185,7 @@
   return nullptr;
 
 // This is the only solution, store it
-return (Value);
+return (Value);
   } else if (const SymbolCast *SC = dyn_cast(Sym)) {
 SymbolRef CastSym = SC->getOperand();
 QualType CastTy = SC->getType();
@@ -1191,7 +1196,7 @@
 const llvm::APSInt *Value;
 if (!(Value = getSymVal(State, CastSym)))
   return nullptr;
-return (SC->getType(), *Value);
+return (SC->getType(), *Value);
   } else if (const BinarySymExpr *BSE = dyn_cast(Sym)) {
 const llvm::APSInt *LHS, *RHS;
 if (const SymIntExpr *SIE = dyn_cast(BSE)) {
@@ -1215,7 +1220,7 @@
 QualType LTy = getAPSIntType(*LHS), RTy = getAPSIntType(*RHS);
 doIntTypeConversion(
 ConvertedLHS, LTy, ConvertedRHS, RTy);
-return BV.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
+return BVF.evalAPSInt(BSE->getOpcode(), ConvertedLHS, ConvertedRHS);
   }
 
   llvm_unreachable("Unsupported expression to get symbol value!");
@@ -1342,13 +1347,13 @@
   BinaryOperator::Opcode Op = BSE->getOpcode();
 
   if (const SymIntExpr *SIE = dyn_cast(BSE)) {
-RTy = getAPSIntType(SIE->getRHS());
+llvm::APSInt NewRInt;
 Z3Expr LHS = getZ3SymExpr(SIE->getLHS(), , hasComparison);
-Z3Expr RHS = Z3Expr::fromAPSInt(SIE->getRHS());
+Z3Expr RHS = Z3Expr::fromAPSInt(fixAPSInt(SIE->getRHS(), NewRInt, ));
 return getZ3BinExpr(LHS, LTy, Op, RHS, RTy, RetTy);
   } else if (const IntSymExpr *ISE = dyn_cast(BSE)) {
-LTy = getAPSIntType(ISE->getLHS());
-Z3Expr LHS = Z3Expr::fromAPSInt(ISE->getLHS());
+llvm::APSInt NewLInt;
+Z3Expr LHS = Z3Expr::fromAPSInt(fixAPSInt(ISE->getLHS(), NewLInt, ));
 Z3Expr RHS = getZ3SymExpr(ISE->getRHS(), , hasComparison);
 return 

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D35450#1116535, @george.karpenkov wrote:

> @ddcc Hi, are you still interested in landing the fixes associated with this 
> patch? I can take a look as I'm currently reviewing 
> https://reviews.llvm.org/D45517, but it is likely that the patch would need 
> to be changed substantially before it could land.


@george.karpenkov Yeah, I've got this and a couple of other patches still 
awaiting review. If it's easier, I can also split out the APSInt fix into a 
separate patch.


https://reviews.llvm.org/D35450



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

[PATCH] D45517: [analyzer] WIP: False positive refutation with Z3

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

FYI the fix for the 1-bit APSInt issue is in 
https://reviews.llvm.org/D35450#change-ifYnQ3IlVso


https://reviews.llvm.org/D45517



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@NoQ Does the proposal in https://reviews.llvm.org/D28955#652465 satisfy your 
concern?


https://reviews.llvm.org/D28955



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@dcoughlin No, all three patches are separate. I have been testing them with 
each applied incrementally onto the previous, with the order trunk, 
https://reviews.llvm.org/D35450, https://reviews.llvm.org/D28954, then 
https://reviews.llvm.org/D28955 (this). But since these are a lot of 
dependencies, I've refactored this patch to apply directly on top of 
https://reviews.llvm.org/D35450, though the floating-point specific changes 
will need to be merged into https://reviews.llvm.org/D28954. Also, I think 
https://reviews.llvm.org/D28954 is waiting for review.


https://reviews.llvm.org/D28955



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 113619.
ddcc added a comment.

Rebase, factor out floating-point changes, fix Z3 type bug, support general 
APSInt comparison


https://reviews.llvm.org/D28955

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/PR3991.m
  test/Analysis/dead-stores.m
  test/Analysis/explain-svals.cpp
  test/Analysis/malloc.c
  test/Analysis/misc-ps-eager-assume.m
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -146,7 +146,7 @@
 void test_isgraph_isprint(int x) {
   char y = x;
   if (isgraph(y))
-clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
+clang_analyzer_eval(isprint(y)); // expected-warning{{TRUE}}
 }
 
 int isdigit(int);
Index: test/Analysis/misc-ps-eager-assume.m
===
--- test/Analysis/misc-ps-eager-assume.m
+++ test/Analysis/misc-ps-eager-assume.m
@@ -1,5 +1,4 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks %s -analyzer-eagerly-assume
-// expected-no-diagnostics
 
 // Delta-reduced header stuff (needed for test cases).
 typedef signed char BOOL;
@@ -56,7 +55,7 @@
 void handle_symbolic_cast_in_condition(void) {
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
 
-  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"];
+  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"]; // expected-warning {{Assignment of a non-Boolean value}}
   NSMutableArray* array = needsAnArray ? [[NSMutableArray alloc] init] : 0;
   if(needsAnArray)
 [array release];
Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1656,13 +1656,13 @@
 void testOffsetPassedToStrlen() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
+  size_t length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
 }
 
 void testOffsetPassedToStrlenThenFree() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string);
+  size_t length = strlen(string);
   free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}}
 }
 
@@ -1705,7 +1705,7 @@
 }
 
 char *dupstrNoWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocNoWarn(len + 1);
   strcpy(p, s); // no-warning
   return p;
@@ -1721,7 +1721,7 @@
 }
 
 char *dupstrWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocWarn(len + 1);
   strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
   return p;
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -41,11 +41,19 @@
 
 void test_2(char *ptr, int ext) {
   clang_analyzer_explain((void *) "asdf"); // expected-warning-re^pointer to element of type 'char' with index 0 of string literal "asdf"$
+#ifdef ANALYZER_CM_Z3
+  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^cast of type 'int' of metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+#else
   clang_analyzer_explain(strlen(ptr)); // expected-warning-re^metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+#endif
   clang_analyzer_explain(conjure()); // expected-warning-re^symbol of type 'int' conjured at statement 'conjure\(\)'$
   clang_analyzer_explain(glob); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob'$
   clang_analyzer_explain(glob_ptr); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob_ptr'$
+#ifdef ANALYZER_CM_Z3
+  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^cast of type 'int' of extent of pointee of argument 'ptr'$
+#else
   

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

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

All testcases pass, except the issue with `range_casts.c`. The cause is still 
the range intersection discussed in https://reviews.llvm.org/D35450#810469.


https://reviews.llvm.org/D35450



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 113505.
ddcc added a comment.

Rebase, make complexity limits configurable


https://reviews.llvm.org/D35450

Files:
  include/clang/AST/Expr.h
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/loop-unrolling.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,7 +67,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
+  if (index - 1 == 0)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
@@ -87,7 +87,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
+  if (index - 1L == 0L)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
@@ -117,7 +117,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
+  if (index - 1UL == 0L)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364
   if (symLHS && symRHS &&
-  (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
+  (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
 return makeNonLoc(symLHS, Op, symRHS, ResultTy);

zaks.anna wrote:
> ddcc wrote:
> > zaks.anna wrote:
> > > As a follow up to the previous version of this patch, I do not think we 
> > > should set the default complexity limit to 1.
> > > 
> > > What is the relation between this limit and the limit in 
> > > VisitNonLocSymbolVal? If they are related, would it be worthwhile turning 
> > > these into an analyzer option?
> > To clarify, the current version of this patch does not modify the `MaxComp` 
> > parameter.
> > 
> > My understanding is that `MaxComp` is the upper bound for building a new 
> > `NonLoc` symbol from two children, based on the sum of the number of child 
> > symbols (complexity) across both children.
> > 
> > In contrast, the limit in `VisitNonLocSymbolVal` (@NoQ, correct me if I'm 
> > wrong), is the upper bound for recursively evaluating and inlining a 
> > `NonLoc` symbol, triggered from `simplifySVal` by `evalBinOpNN`. Note that 
> > these two latter functions indirectly call each other recursively (through 
> > `evalBinOp`), causing the previous runtime blowup. Furthermore, each call 
> > to `computeComplexity`will re-iterate through all child symbols of the 
> > current symbol, but only the first complexity check at the root symbol is 
> > actually necessary, because by definition the complexity of a child symbol 
> > at each recursive call is monotonically decreasing.
> > 
> > I think it'd be useful to allow both to be configurable, but I don't see a 
> > direct relationship between the two.
> > To clarify, the current version of this patch does not modify the MaxComp 
> > parameter.
> 
> I know. Also, currently, it is only used in the unsupported taint tracking 
> mode and only for tainted symbols. I would like a different smaller default 
> for all expressions.
Ok. I can make both configurable as part of this patch, with a new default of 
10 for `VisitNonLocSymbolVal`. But I've never used the taint tracking mode, so 
I don't know what would be a reasonable default for `MaxComp`.


https://reviews.llvm.org/D35450



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364
   if (symLHS && symRHS &&
-  (symLHS->computeComplexity() + symRHS->computeComplexity()) <  MaxComp)
+  (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp)
 return makeNonLoc(symLHS, Op, symRHS, ResultTy);

zaks.anna wrote:
> As a follow up to the previous version of this patch, I do not think we 
> should set the default complexity limit to 1.
> 
> What is the relation between this limit and the limit in 
> VisitNonLocSymbolVal? If they are related, would it be worthwhile turning 
> these into an analyzer option?
To clarify, the current version of this patch does not modify the `MaxComp` 
parameter.

My understanding is that `MaxComp` is the upper bound for building a new 
`NonLoc` symbol from two children, based on the sum of the number of child 
symbols (complexity) across both children.

In contrast, the limit in `VisitNonLocSymbolVal` (@NoQ, correct me if I'm 
wrong), is the upper bound for recursively evaluating and inlining a `NonLoc` 
symbol, triggered from `simplifySVal` by `evalBinOpNN`. Note that these two 
latter functions indirectly call each other recursively (through `evalBinOp`), 
causing the previous runtime blowup. Furthermore, each call to 
`computeComplexity`will re-iterate through all child symbols of the current 
symbol, but only the first complexity check at the root symbol is actually 
necessary, because by definition the complexity of a child symbol at each 
recursive call is monotonically decreasing.

I think it'd be useful to allow both to be configurable, but I don't see a 
direct relationship between the two.


https://reviews.llvm.org/D35450



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@NoQ ping


https://reviews.llvm.org/D35450



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

[PATCH] D28954: [analyzer] Add support for symbolic float expressions

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

As an aside, I think it'd be good to land https://reviews.llvm.org/D28954 and 
https://reviews.llvm.org/D28955 first, since they affect accuracy and precision 
of various analyzer parts that this depends on.

> Here are some examples that should all be UNKNOWN (right?) but are not in the 
> current patch, assuming a and b hold unconstrained symbols of floating-point 
> type:

Yeah, those should definitely be fixed. In general, I tried to avoid performing 
simplifications on anything not of floating-point type, particularly in 
SimpleSValBuilder, but there are probably cases that I've missed.

In your example `clang_analyzer_eval(a < 4.0 || a >= 4.0)` (and likewise for 
the rest), the following is happening:

1. At ExprEngineC.cpp:VisitLogicalExpr(), we hit this logical expression for 
the first time, the introspection fails, and we generate the SVal `(((double) 
(reg_$0)) >= 4.0E+0) != 0` that is bound to `a < 4.0 || a >= 4.0`.
2. The next time around, the introspection succeeds, and we generate the SVal 
`1 S32b` that is bound to `a < 4.0 || a >= 4.0`.
3. Now, when we hit ExprInspectionChecker.cpp:getArgumentValueString(), we 
retrieve the SVal `1 S32b`, and attempt to assert it.
4. Then, we hit SimpleConstraintManager.cpp:assumeAux(), and fall into the 
`nonloc::ConcreteIntKind` case. When `Assumption` is `true`, we are fine and 
return the original `State`, but then when `Assumption` is `false`, we return 
`nullptr`.
5. Back in ExprInspectionChecker.cpp:getArgumentValueString(), we see `StTrue 
!= nullptr` and `StFalse == nullptr`, and we print `TRUE` instead of `UNKNOWN`.

I'm not familiar with `VisitLogicalExpr()` and why integer constants are being 
bound to the logical expressions. Wouldn't we simply want to assume that the 
logical expression, when expressed as a symbolic constraint, is either 
true/false in each respective child state?




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:150
 
+def FloatingPointMathChecker : Checker<"FPMath">,
+  HelpText<"Check for domain errors in floating-point math functions">,

dcoughlin wrote:
> It is fine to have this in alpha now. What package to do envision this in 
> after it is ready? Is this something that should always be on, or should be 
> opted into on a per-project basis?
This checker is a bit of a toy, in that (last I checked) Clang doesn't support 
the floating-point environment, which is typically used to install global 
floating-point exception handlers (for e.g. NaN, etc). As a result, there are 
probably going to lots of false-positives on real codebases.

Additionally, it requires the z3 solver, which probably isn't being built by 
default in most Linux distributions (and I doubt we're at the point of asking 
package maintainers to add a dependency for clang on libz3, even for those that 
do build that package).

So I think it should definitely be optional.



Comment at: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp:80
+DefinedOrUnknownSVal isPInf = SVB.evalEQ(state, V, SPInf);
+if (isPInf.isConstant(1)) {
+  C.addTransition(state->BindExpr(CE, LCtx, isPInf));

dcoughlin wrote:
> Is there a reason you're not using assumeDual() here?
I'm not sure how `assumeDual` helps here?  The explicit checks `isPInf` and 
`isNInf` implement the short-circuit effect of logical or, whereas omitting 
them and binding directly to `isInf` miss the short-circuit effect.



Comment at: lib/StaticAnalyzer/Core/BasicValueFactory.cpp:118
+const llvm::APFloat ::getValue(const llvm::APFloat& X) {
+  llvm::FoldingSetNodeID ID;
+  void *InsertPos;

dcoughlin wrote:
> This logic is nearly identical to that in getValue(const llvm::APSInt& X). 
> Can the logic be factored out in some sort of zero-costish abstraction? 
> (Perhaps templatized over the value type?)
The tricky part is that there are associate class member variables for each 
function (`APSIntSet` and `APFloatSet`). I can factor out `getValue` to a 
template, but then I'd need to introduce a templated helper function with two 
specializations to retrieve the class member variable for the input template 
type. I'm not sure if that'd be zero-cost?



Comment at: lib/StaticAnalyzer/Core/BasicValueFactory.cpp:331
+case BO_Div:
+  // Divide by zero
+  if (V1.isFinite() && V2.isInfinity())

dcoughlin wrote:
> Is this comment correct? Is this really a divide by zero?
> 
> I'm also a bit surprised APFloat::divide() doesn't handle this case.
I don't recall why I wrote this...



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1092
 
+  assert(!V.isFloat());
+

dcoughlin wrote:
> I'm wondering whether we should rename this method to "getKnownIntValue()" 
> and then just return nullptr here. What are the merits of trapping vs. 
> returning nullptr here? 
The trapping should be for 

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 107190.
ddcc added a comment.

Minor style fix


https://reviews.llvm.org/D35450

Files:
  include/clang/AST/Expr.h
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,7 +67,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
+  if (index - 1 == 0)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
@@ -87,7 +87,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
+  if (index - 1L == 0L)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
@@ -117,7 +117,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
+  if (index - 1UL == 0L)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// 

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

As an update, after fixing the typo and updating the tests, the assertion in 
`range_casts.c` is no longer triggered and everything seems fine now.


https://reviews.llvm.org/D35450



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 106896.
ddcc added a comment.

Fix tests after typo fix


https://reviews.llvm.org/D35450

Files:
  include/clang/AST/Expr.h
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,7 +67,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
+  if (index - 1 == 0)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
@@ -87,7 +87,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
+  if (index - 1L == 0L)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
@@ -117,7 +117,7 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
+  if (index - 1UL == 0L)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// 

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> Is diff 1 the original diff from https://reviews.llvm.org/D28953? It was ok 
> to reopen it, but the new revision is also fine.

No, diff 1 is already different; it contains most of the bugfixes. I couldn't 
find any way to reopen the previous review, and `arc diff` wouldn't let me 
update a closed review.

> Regarding 1-bit bools: did you notice https://reviews.llvm.org/D32328 and 
> https://reviews.llvm.org/D35041, do they accidentally help?

I did see those changes, but I think they're a little different. The test 
failures I ran into were cases where the input is a 1-bit APSInt, and 
attempting to retrieve the type for that gives a null QualType.


https://reviews.llvm.org/D35450



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 106883.
ddcc added a comment.

Fix typo


https://reviews.llvm.org/D35450

Files:
  include/clang/AST/Expr.h
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Compared with https://reviews.llvm.org/D28953, this revision fixes the test 
failure with `PR3991.m` with RangeConstraintManager, and a few other failures 
with Z3ConstraintManager. However, there's one remaining test failure in 
`range_casts.c` that I'm not sure how to resolve. For reference, this is the 
simplified code snippet from that testcase:

  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}}
  }

In debug mode, an assertion about the system being over-constrained is thrown 
from `ConstraintManager.h`. This is because the new 
`Simplifier::VisitSymbolCast` function will attempt to evaluate the cast 
`(unsigned int) (reg_$0)`, which is suppressed by the call to 
`haveSameType()` in `SimpleSValBuilder::evalCastFromNonLoc` 
(https://reviews.llvm.org/D28955 fixes this, but only for Z3ConstraintManager), 
generating just the symbol `reg_$0`. Subsequently, the analyzer will 
attempt to evaluate the expression `((reg_$0) + 1U) == 0U` with the 
range `reg_$0 : { [4294967296, 9223372036854775807] }`, or `[UINT_MAX 
+ 1, LONG_MAX]`. However, in the case where the assumption is true, 
RangeConstraintManager takes the intersection of the previous range with 
`[UINT_MAX, UINT_MAX]` and produces the empty set, and likewise where the 
assumption is false, the intersection with `[UINT_MAX - 1, 0]` again produces 
the empty set. As a result, both program states are NULL, triggering the 
assertion.

I'm now somewhat inclined to drop the addition of 
`Simplified::VisitSymbolCast()` and those associated testsuite changes, because 
ignoring type casts is clearly incorrect and will introduce both false 
negatives and false positives.


https://reviews.llvm.org/D35450



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

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 106758.
ddcc added a comment.

Modify Z3RangeConstraintManager::fixAPSInt() and add Expr::isCommutativeOp()


https://reviews.llvm.org/D35450

Files:
  include/clang/AST/Expr.h
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// 

[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.

Generate more IntSymExpr constraints, perform SVal simplification for 
IntSymExpr and SymbolCast constraints, and create fully symbolic SymExprs


https://reviews.llvm.org/D35450

Files:
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Reverted in https://reviews.llvm.org/rL307853


Repository:
  rL LLVM

https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Reverted in https://reviews.llvm.org/rL307853


Repository:
  rL LLVM

https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL307833: [analyzer] Support generating and reasoning over 
more symbolic constraint types (authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D28953?vs=106084=106284#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28953

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  cfe/trunk/test/Analysis/analyzer_test.py
  cfe/trunk/test/Analysis/bitwise-ops.c
  cfe/trunk/test/Analysis/bool-assignment.c
  cfe/trunk/test/Analysis/conditional-path-notes.c
  cfe/trunk/test/Analysis/explain-svals.cpp
  cfe/trunk/test/Analysis/plist-macros-z3.cpp
  cfe/trunk/test/Analysis/plist-macros.cpp
  cfe/trunk/test/Analysis/range_casts.c
  cfe/trunk/test/Analysis/std-c-library-functions.c

Index: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -1034,31 +1034,39 @@
 ProgramStateRef Z3ConstraintManager::assumeSymInclusiveRange(
 ProgramStateRef State, SymbolRef Sym, const llvm::APSInt ,
 const llvm::APSInt , bool InRange) {
+  ASTContext  = getBasicVals().getContext();
+  // FIXME: This should be a cast from a 1-bit integer type to a boolean type,
+  // but the former is not available in Clang. Instead, extend the APSInt
+  // directly.
+  bool isBoolTy = From.getBitWidth() == 1 && getAPSIntType(From).isNull();
+
   QualType RetTy;
   // The expression may be casted, so we cannot call getZ3DataExpr() directly
   Z3Expr Exp = getZ3Expr(Sym, );
-
-  assert((getAPSIntType(From) == getAPSIntType(To)) &&
- "Range values have different types!");
-  QualType RTy = getAPSIntType(From);
-  bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType();
-  Z3Expr FromExp = Z3Expr::fromAPSInt(From);
-  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
+  QualType RTy = isBoolTy ? Ctx.BoolTy : getAPSIntType(From);
+  Z3Expr FromExp =
+  isBoolTy ? Z3Expr::fromAPSInt(From.extend(Ctx.getTypeSize(Ctx.BoolTy)))
+   : Z3Expr::fromAPSInt(From);
 
   // Construct single (in)equality
   if (From == To)
 return assumeZ3Expr(State, Sym,
 getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE,
  FromExp, RTy, nullptr));
 
+  assert((getAPSIntType(From) == getAPSIntType(To)) &&
+ "Range values have different types!");
+
+  Z3Expr ToExp = Z3Expr::fromAPSInt(To);
   // Construct two (in)equalities, and a logical and/or
   Z3Expr LHS =
   getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr);
   Z3Expr RHS =
   getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr);
   return assumeZ3Expr(
   State, Sym,
-  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy));
+  Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS,
+RetTy->isSignedIntegerOrEnumerationType()));
 }
 
 ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State,
@@ -1406,6 +1414,7 @@
QualType , QualType ) const {
   ASTContext  = getBasicVals().getContext();
 
+  assert(!LTy.isNull() && !RTy.isNull() && "Input type is null!");
   // Perform type conversion
   if (LTy->isIntegralOrEnumerationType() &&
   RTy->isIntegralOrEnumerationType()) {
@@ -1468,10 +1477,10 @@
 void Z3ConstraintManager::doIntTypeConversion(T , QualType , T ,
   QualType ) const {
   ASTContext  = getBasicVals().getContext();
-
   uint64_t LBitWidth = Ctx.getTypeSize(LTy);
   uint64_t RBitWidth = Ctx.getTypeSize(RTy);
 
+  assert(!LTy.isNull() && !RTy.isNull() && "Input type is null!");
   // Always perform integer promotion before checking type equality.
   // Otherwise, e.g. (bool) a + (bool) b could trigger a backend assertion
   if (LTy->isPromotableIntegerType()) {
Index: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -100,7 +100,7 @@
 
   if (T->isNullPtrType())
 return makeZeroVal(T);
-  
+
   if (!SymbolManager::canSymbolicate(T))
 return UnknownVal();
 
@@ -354,17 +354,14 @@
BinaryOperator::Opcode Op,
NonLoc LHS, NonLoc RHS,
QualType ResultTy) {
-  if (!State->isTainted(RHS) && !State->isTainted(LHS))
-return UnknownVal();
-
   const SymExpr *symLHS = LHS.getAsSymExpr();
   const SymExpr *symRHS = RHS.getAsSymExpr();
   // 

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 106084.
ddcc added a comment.

Split plist-macros.cpp, and update analyzer_test.py to support tests that 
require not z3


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer_test.py
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros-z3.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
-
+// REQUIRES: !z3
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
@@ -11,13 +11,13 @@
   y++;
   y--;
   mallocmemory
-  y++; 
+  y++;
   y++;
   delete x; // expected-warning {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
 }
 
 void macroIsFirstInFunction(int y) {
-  mallocmemory 
+  mallocmemory
   y++; // expected-warning {{Potential leak of memory pointed to by 'x'}}
 }
 
@@ -39,7 +39,7 @@
   return *p; // expected-warning {{Dereference of null pointer}}
 }
 
-#define macroWithArg(mp) mp==0 
+#define macroWithArg(mp) mp==0
 int macroWithArgInExpression(int *p, int y) {;
   y++;
   if (macroWithArg(p))
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I've also uploaded the results to https://dcddcc.com/csa


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I tested the following software, both before and after applying this patch, 
using RangeConstraintManager.

Software, Version, Compile Time (before), Bugs Reported (before), Compile Time 
(after), Bugs Reported (after)
openssl, 1.1.0f, 11 min, 126, 12 min, 126
sqlite, 3.18.2, 31 min, 86, 31 min, 82
wireshark, 2.2.7, 105 min, 109, 119 min, 108

For sqlite, the differences of four reported bugs all appear to be false 
positives, but for wireshark, the one difference appears to be a false negative.




Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:363
   // instead of generating an Unknown value and propagate the taint info to it.
-  const unsigned MaxComp = 1; // 10 28X
 

NoQ wrote:
> ddcc wrote:
> > zaks.anna wrote:
> > > Reducing the MaxComp is going to regress taint analysis..
> > > 
> > > > I've updated this revision to account for the recent SVal 
> > > > simplification commit by @NoQ, 
> > > 
> > > Which commit introduced the regression?
> > > 
> > > > but now there is an exponential blowup problem that prevents testcase 
> > > > PR24184.cpp from terminating, 
> > > 
> > > What triggers the regression? Removing the if statement above? Does the 
> > > regression only effect the Z3 "mode" (I am guessing not since you said 
> > > "due to an interaction between Simplifier::VisitNonLocSymbolVal() and 
> > > SValBuilder::makeSymExprValNN()")? 
> > > 
> > > Reducing the MaxComp is going to regress taint analysis..
> > 
> > I think the original intention was to increase `MaxComp`, not decrease it, 
> > but I will restore the original value here.
> > 
> > > What triggers the regression? Removing the if statement above? Does the 
> > > regression only effect the Z3 "mode"
> > 
> > No, the regression isn't specifically due to this code, but with @NoQ 's 
> > patch for `SVal` simplification (rL300178), and this commit extending it to 
> > handle `SymbolCast` and `IntSymExpr`, the cast of `ST *` used in the loop 
> > of case 3 of PR24184.cpp becomes "simplified" (inlined) repeatedly on each 
> > recursive iteration through `Simplifier::VisitNonLocSymbolVal()` -> 
> > `SValBuilder::makeSymExprValNN()`, causing a big slowdown in runtime 
> > performance.
> > 
> > The naive way to prevent it is to drop `MaxComp` (but this isn't 
> > reasonable, since it needs to be absurdly low, e.g. `10`). Alternatively, 
> > simplification for `SymbolCast` can be dropped from this commit (but it 
> > will eliminate some of the other analysis improvements), or, most likely, 
> > introduce another parameter to reduce recursion between these two functions.
> We talked a bit and we're pretty much fine with reduce `MaxComp` to 10 if 
> it's worth it in terms of performance.
Just to clarify, the current change from 100 to 10 in 
`simplifySVal::SimplyVisitNonLocSymbolVal` resolves the problem; the value of 
`MaxComp` here is unchanged.



Comment at: test/Analysis/plist-macros.cpp:2
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix 
-analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix 
-analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config 
path-diagnostics-alternate=ture %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s

NoQ wrote:
> > `path-diagnostics-alternate=ture`
> 
> Ouch. Thanks for fixing this.
> 
> Maybe it'd be great to implement an option that enables validating analyzer 
> config option values, and turn this option on in `%clang_analyze_cc1`.
I agree that'd be useful, but I think that it should be a separate future patch.


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 105913.
ddcc marked an inline comment as done.
ddcc added a comment.

Drop duplicate code


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
 
 
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col25
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

ping


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I forgot to mention that the only remaining test failure is on 
`plist-macros.cpp`; there is a `Assuming condition is true` path note that only 
appears with the RangeConstraintManager but not with Z3ConstraintManager, and I 
can't `#ifdef` it because the annotations are checked by `FileCheck`.


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 103239.
ddcc added a comment.

Rebase, decrease simplification complexity


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
 
 
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col25
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// 

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

> Can we drop computing these for some expressions that we know the 
> RangeConstraintManager will not utilize?

It's possible, though I'm not sure what the actual limitations of the 
RangeConstraintManager are, since there are a lot of intermediate steps that 
attempt to transform unsupported expressions into supported ones.

> We could implement the TODO described below and possibly also lower the 
> MaxComp value. This means that instead of keeping a complex expression and 
> constraints on every symbol used in that expression, we would conjure a new 
> symbol and associate a new constrain derived from the expression with it. 
> (Would this strategy also work for the Z3 case?)

I think it's feasible, but would probably require some more to code to handle 
`SymbolConjured` (right now all `SymbolData` are treated as variables).

> Would it help to decrease 100 to 10 here?

Yes, changing it to 10 eliminates the excessive recursion; combined runtime 
drops to 282 sec on the testcases (~8 sec for Range only, ~271 sec for Z3 only; 
doesn't sum due to separate executions). This appears to be the most 
straightforward solution.

> (2) With RangeConstraintManager, simplification is not entirely idempotent: 
> we may simplify a symbol further when one of its sub-symbols gets constrained 
> to a constant in a new state. However, apart from that, we might be able to 
> avoid re-simplifying the same symbol by caching results based on the (symbol, 
> state's constraint data) pair. Probably it may work with Z3 as well.

Yeah, that would also fix this issue. In general, I think there's plenty of 
room for improvements from caching, especially for queries to e.g. 
`getSymVal()`.


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:356
QualType ResultTy) {
-  if (!State->isTainted(RHS) && !State->isTainted(LHS))
-return UnknownVal();

zaks.anna wrote:
> I am concerned that removing the guard will regress performance in the 
> vanilla case. (Note that Z3 support as well as taint are not on by default.)
> 
> I am curious how much of the regression you've measured could be gained back 
> if we make this conditional.
To clarify, the changes made in this patch aren't specific to Z3 support, 
especially simplifying `SymbolCast` and `IntSymExpr`. With the exception of 
`PR24184.cpp` and `plist-macros.cpp`, all testcases pass with both the default 
and Z3 constraint managers. However, creating additional constraints does have 
performance overhead, and it may be useful to consider the parameters for 
gating this functionality.

On a combined execution (Range + Z3) through the testcases, except the two 
mentioned above, the runtime is 327 sec with this patch applied, and 195 sec 
without this patch applied. On a separate execution through the testcases with 
only the Z3 constraint manager, I get runtimes 320 and 191, respectively.

For testing purposes, I also tried the following code, which has combined 
runtime 311 sec, but loses the accuracy improvements with the Range constraint 
manager on `bitwise-ops.c`, `conditional-path-notes.c`, `explain-svals.cpp`, 
and `std-c-library-functions.c`.

```
ConstraintManager  = getStateManager().getConstraintManager();
if (!State->isTainted(RHS) && !State->isTainted(LHS) && !CM.isZ3())
```



Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:363
   // instead of generating an Unknown value and propagate the taint info to it.
-  const unsigned MaxComp = 1; // 10 28X
 

zaks.anna wrote:
> Reducing the MaxComp is going to regress taint analysis..
> 
> > I've updated this revision to account for the recent SVal simplification 
> > commit by @NoQ, 
> 
> Which commit introduced the regression?
> 
> > but now there is an exponential blowup problem that prevents testcase 
> > PR24184.cpp from terminating, 
> 
> What triggers the regression? Removing the if statement above? Does the 
> regression only effect the Z3 "mode" (I am guessing not since you said "due 
> to an interaction between Simplifier::VisitNonLocSymbolVal() and 
> SValBuilder::makeSymExprValNN()")? 
> 
> Reducing the MaxComp is going to regress taint analysis..

I think the original intention was to increase `MaxComp`, not decrease it, but 
I will restore the original value here.

> What triggers the regression? Removing the if statement above? Does the 
> regression only effect the Z3 "mode"

No, the regression isn't specifically due to this code, but with @NoQ 's patch 
for `SVal` simplification (rL300178), and this commit extending it to handle 
`SymbolCast` and `IntSymExpr`, the cast of `ST *` used in the loop of case 3 of 
PR24184.cpp becomes "simplified" (inlined) repeatedly on each recursive 
iteration through `Simplifier::VisitNonLocSymbolVal()` -> 
`SValBuilder::makeSymExprValNN()`, causing a big slowdown in runtime 
performance.

The naive way to prevent it is to drop `MaxComp` (but this isn't reasonable, 
since it needs to be absurdly low, e.g. `10`). Alternatively, simplification 
for `SymbolCast` can be dropped from this commit (but it will eliminate some of 
the other analysis improvements), or, most likely, introduce another parameter 
to reduce recursion between these two functions.


https://reviews.llvm.org/D28953



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

[PATCH] D33308: [analyzer]: Improve test handling with multiple constraint managers

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305480: [analyzer]: Improve test handling with multiple 
constraint managers (authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D33308?vs=99394=102682#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33308

Files:
  cfe/trunk/test/Analysis/analyzer_test.py


Index: cfe/trunk/test/Analysis/analyzer_test.py
===
--- cfe/trunk/test/Analysis/analyzer_test.py
+++ cfe/trunk/test/Analysis/analyzer_test.py
@@ -5,24 +5,39 @@
 class AnalyzerTest(lit.formats.ShTest):
 
 def execute(self, test, litConfig):
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=range')
+results = []
 
-if result.code == lit.Test.FAIL:
-return result
+# Parse any test requirements ('REQUIRES: ')
+saved_test = test
+lit.TestRunner.parseIntegratedTestScript(test)
+
+if 'z3' not in test.requires:
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=range'))
+
+if results[-1].code == lit.Test.FAIL:
+return results[-1]
 
 # If z3 backend available, add an additional run line for it
 if test.config.clang_staticanalyzer_z3 == '1':
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=z3 
-DANALYZER_CM_Z3'))
 
-return result
+# Combine all result outputs into the last element
+for x in results:
+if x != results[-1]:
+results[-1].output = x.output + results[-1].output
+
+if results:
+return results[-1]
+return lit.Test.Result(lit.Test.UNSUPPORTED,
+"Test requires the following unavailable features: z3")
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
 saved_substitutions = list(test.config.substitutions)
 test.config.substitutions.append(('%analyze', substitution))
 result = lit.TestRunner.executeShTest(test, litConfig,
-  self.execute_external)
+self.execute_external)
 test.config.substitutions = saved_substitutions
 
 return result


Index: cfe/trunk/test/Analysis/analyzer_test.py
===
--- cfe/trunk/test/Analysis/analyzer_test.py
+++ cfe/trunk/test/Analysis/analyzer_test.py
@@ -5,24 +5,39 @@
 class AnalyzerTest(lit.formats.ShTest):
 
 def execute(self, test, litConfig):
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=range')
+results = []
 
-if result.code == lit.Test.FAIL:
-return result
+# Parse any test requirements ('REQUIRES: ')
+saved_test = test
+lit.TestRunner.parseIntegratedTestScript(test)
+
+if 'z3' not in test.requires:
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=range'))
+
+if results[-1].code == lit.Test.FAIL:
+return results[-1]
 
 # If z3 backend available, add an additional run line for it
 if test.config.clang_staticanalyzer_z3 == '1':
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3'))
 
-return result
+# Combine all result outputs into the last element
+for x in results:
+if x != results[-1]:
+results[-1].output = x.output + results[-1].output
+
+if results:
+return results[-1]
+return lit.Test.Result(lit.Test.UNSUPPORTED,
+"Test requires the following unavailable features: z3")
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
 saved_substitutions = list(test.config.substitutions)
 test.config.substitutions.append(('%analyze', substitution))
 result = lit.TestRunner.executeShTest(test, litConfig,
-  self.execute_external)
+self.execute_external)
 test.config.substitutions = saved_substitutions
 
 return result
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33308: [analyzer]: Improve test handling with multiple constraint managers

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@dcoughlin @zaks.anna @NoQ @xazax.hun Ping, I'd appreciate it if I could get a 
review for this (https://reviews.llvm.org/D33308), 
https://reviews.llvm.org/D28955, https://reviews.llvm.org/D28953, and 
https://reviews.llvm.org/D28954. Rebasing and fixing up these commits is fairly 
time consuming.


https://reviews.llvm.org/D33308



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

[PATCH] D28954: [analyzer] Add support for symbolic float expressions

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 99522.
ddcc added a comment.
Herald added a subscriber: eraman.

Rebase, avoid generating floating-point constraints if unsupported by 
constraint manager


https://reviews.llvm.org/D28954

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/FloatingPointMath.cpp
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/diagnostics/macros.cpp
  test/Analysis/float-rules.c
  test/Analysis/float.c
  test/Analysis/inline.cpp
  test/Analysis/operator-calls.cpp

Index: test/Analysis/operator-calls.cpp
===
--- test/Analysis/operator-calls.cpp
+++ test/Analysis/operator-calls.cpp
@@ -81,8 +81,8 @@
   void test(int coin) {
 // Force a cache-out when we try to conjure a temporary region for the operator call.
 // ...then, don't crash.
-clang_analyzer_eval(+(coin ? getSmallOpaque() : getSmallOpaque())); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(+(coin ? getLargeOpaque() : getLargeOpaque())); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(+(coin ? getSmallOpaque() : getSmallOpaque())); // expected-warning{{TRUE}}
+clang_analyzer_eval(+(coin ? getLargeOpaque() : getLargeOpaque())); // expected-warning{{TRUE}}
   }
 }
 
Index: test/Analysis/inline.cpp
===
--- test/Analysis/inline.cpp
+++ test/Analysis/inline.cpp
@@ -285,11 +285,11 @@
   }
 
   void testFloatReference() {
-clang_analyzer_eval(defaultFloatReference(1) == -1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(defaultFloatReference() == -42); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(defaultFloatReference(1) == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(defaultFloatReference() == -42); // expected-warning{{TRUE}}
 
-clang_analyzer_eval(defaultFloatReferenceZero(1) == -1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(defaultFloatReferenceZero() == 0); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(defaultFloatReferenceZero(1) == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(defaultFloatReferenceZero() == 0); // expected-warning{{TRUE}}
   }
 
   char defaultString(const char *s = "abc") {
Index: test/Analysis/float.c
===
--- /dev/null
+++ test/Analysis/float.c
@@ -0,0 +1,83 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+// REQUIRES: z3
+
+void clang_analyzer_eval(int);
+
+void float1() {
+  float z1 = 0.0, z2 = -0.0;
+  clang_analyzer_eval(z1 == z2); // expected-warning{{TRUE}}
+}
+
+void float2(float a, float b) {
+  clang_analyzer_eval(a == b); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a != b); // expected-warning{{UNKNOWN}}
+}
+
+void float3(float a, float b) {
+  if (__builtin_isnan(a) || __builtin_isnan(b) || a < b) {
+clang_analyzer_eval(a > b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+return;
+  }
+  clang_analyzer_eval(a >= b); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == b); // expected-warning{{UNKNOWN}}
+}
+
+void float4(float a) {
+  if (__builtin_isnan(a) || a < 0.0f)
+return;
+  clang_analyzer_eval(a >= 0.0Q); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a >= 0.0F); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a >= 0.0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a >= 0); // expected-warning{{TRUE}}
+}
+
+void float5() {
+  double value = 1.0;
+  double pinf = __builtin_inf();
+  double ninf = -__builtin_inf();
+  double nan = __builtin_nan("");
+
+  /* NaN */
+  clang_analyzer_eval(__builtin_isnan(value)); // expected-warning{{FALSE}}
+  clang_analyzer_eval(__builtin_isnan(nan)); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(__builtin_isnan(0.0 / 0.0)); // 

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 99521.
ddcc added a comment.

Fix typo in SymbolCast simplification


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
 
 
@@ -85,6 +85,7 @@
 void test2(int *p) {
   CALL_FN(p);
 }
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -636,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col25
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:  

[PATCH] D33308: [analyzer]: Improve test handling with multiple constraint managers

[Dominic Chen via Phabricator via cfe-commits]

ddcc created this revision.

Modify the test infrastructure to properly handle tests that require z3, and 
merge together the output of all tests on success. This is required for 
https://reviews.llvm.org/D28954.


https://reviews.llvm.org/D33308

Files:
  test/Analysis/analyzer_test.py


Index: test/Analysis/analyzer_test.py
===
--- test/Analysis/analyzer_test.py
+++ test/Analysis/analyzer_test.py
@@ -5,24 +5,39 @@
 class AnalyzerTest(lit.formats.ShTest):
 
 def execute(self, test, litConfig):
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=range')
+results = []
 
-if result.code == lit.Test.FAIL:
-return result
+# Parse any test requirements ('REQUIRES: ')
+saved_test = test
+lit.TestRunner.parseIntegratedTestScript(test)
+
+if 'z3' not in test.requires:
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=range'))
+
+if results[-1].code == lit.Test.FAIL:
+return results[-1]
 
 # If z3 backend available, add an additional run line for it
 if test.config.clang_staticanalyzer_z3 == '1':
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=z3 
-DANALYZER_CM_Z3'))
 
-return result
+# Combine all result outputs into the last element
+for x in results:
+if x != results[-1]:
+results[-1].output = x.output + results[-1].output
+
+if results:
+return results[-1]
+return lit.Test.Result(lit.Test.UNSUPPORTED,
+"Test requires the following unavailable features: z3")
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
 saved_substitutions = list(test.config.substitutions)
 test.config.substitutions.append(('%analyze', substitution))
 result = lit.TestRunner.executeShTest(test, litConfig,
-  self.execute_external)
+self.execute_external)
 test.config.substitutions = saved_substitutions
 
 return result


Index: test/Analysis/analyzer_test.py
===
--- test/Analysis/analyzer_test.py
+++ test/Analysis/analyzer_test.py
@@ -5,24 +5,39 @@
 class AnalyzerTest(lit.formats.ShTest):
 
 def execute(self, test, litConfig):
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=range')
+results = []
 
-if result.code == lit.Test.FAIL:
-return result
+# Parse any test requirements ('REQUIRES: ')
+saved_test = test
+lit.TestRunner.parseIntegratedTestScript(test)
+
+if 'z3' not in test.requires:
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=range'))
+
+if results[-1].code == lit.Test.FAIL:
+return results[-1]
 
 # If z3 backend available, add an additional run line for it
 if test.config.clang_staticanalyzer_z3 == '1':
-result = self.executeWithAnalyzeSubstitution(
-test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+results.append(self.executeWithAnalyzeSubstitution(
+saved_test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3'))
 
-return result
+# Combine all result outputs into the last element
+for x in results:
+if x != results[-1]:
+results[-1].output = x.output + results[-1].output
+
+if results:
+return results[-1]
+return lit.Test.Result(lit.Test.UNSUPPORTED,
+"Test requires the following unavailable features: z3")
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
 saved_substitutions = list(test.config.substitutions)
 test.config.substitutions.append(('%analyze', substitution))
 result = lit.TestRunner.executeShTest(test, litConfig,
-  self.execute_external)
+self.execute_external)
 test.config.substitutions = saved_substitutions
 
 return result
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

I've updated this revision to account for the recent SVal simplification commit 
by @NoQ, but now there is an exponential recursion problem that prevents 
testcase `PR24184.cpp` from terminating, due to an interaction between 
`Simplifier::VisitNonLocSymbolVal()` and `SValBuilder::makeSymExprValNN()`. I'm 
not quite sure what the best way to resolve this is; from some blind testing, I 
ended up needing to set `MaxComp` to `10` to force termination in a reasonable 
amount of time, but this restricts its usefulness for other constraints.


https://reviews.llvm.org/D28953



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 99392.
ddcc added a comment.

Address SVal simplification from https://reviews.llvm.org/D31886


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/bool-assignment.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/plist-macros.cpp
  test/Analysis/range_casts.c
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/range_casts.c
===
--- test/Analysis/range_casts.c
+++ test/Analysis/range_casts.c
@@ -67,8 +67,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1 == 0) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1 == 0)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -87,8 +87,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1L == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1L == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
@@ -117,8 +117,8 @@
 {
   unsigned index = -1;
   if (index < foo) index = foo;
-  if (index - 1UL == 0L) // Was not reached prior fix.
-clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  if (index - 1UL == 0L)
+clang_analyzer_warnIfReached(); // no-warning
   else
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
 }
Index: test/Analysis/plist-macros.cpp
===
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist
 // RUN: FileCheck --input-file=%t.plist %s
 
 
@@ -636,6 +636,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line36
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line36
+// CHECK-NEXT:  col25
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D28952#757375, @iris wrote:

> Thank you for helping me build clang with z3.I have already applied the above 
> updating.But now I have another question, when running clang with '-Xanalyzer 
> -analyzer-constraints=z3' there is always be a fatal error and I don't know 
> whether it is right to use a command like 'clang -Xanalyzer 
> -analyzer-constraints=z3 -analyzer-checker=debug.DumpExplodedGraph test.cpp-' 
> to get the ExplodedGraph which is analyzed by z3constraint?Sorry to bother.


You probably want something along the lines of `clang --analyze -Xanalyzer 
-analyzer-constraints=z3 -Xanalyzer -analyzer-checker=debug.ViewExplodedGraph 
test.c`. In particular, there is a difference between using clang as a compiler 
driver or frontend (see FAQ entry on cc1 at 
https://clang.llvm.org/docs/FAQ.html). This is also mentioned in the checker 
development manual: https://clang-analyzer.llvm.org/checker_dev_manual.html .


Repository:
  rL LLVM

https://reviews.llvm.org/D28952



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

[PATCH] D28954: [analyzer] Add support for symbolic float expressions

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D28954#754478, @lirhea wrote:

> In https://reviews.llvm.org/D28954#714936, @ddcc wrote:
>
> > Rebase, update tests, fix bugs
>
>
> Excuse me,I want to download full codes of this version,but I have no idea 
> how to do it,can you tell me?
>  And my system is windows, I haven't install anything about Phabricator.
>  Thank you!


You're probably better off waiting for these patches 
(https://reviews.llvm.org/D28952, https://reviews.llvm.org/D28953, and 
https://reviews.llvm.org/D28954) to land rather than grabbing the commits, 
since some of these diffs are old and I haven't had a chance to rebase and 
retest them yet. But if you want to try them now, you'll need to compile 
Clang/LLVM from source, since they haven't landed in any release. I'm also not 
sure how well the CMake bindings for Z3 work on Windows, it's not a platform 
I've tested. Either grab the raw diffs of these three commits using "download 
raw diff" from the web interface, or using Arcanist's `arc patch` commit. 
Alternatively, I have an older tree with all three commits at 
https://github.com/ddcc/clang/tree/debug , just revert the debugging commit.


https://reviews.llvm.org/D28954



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

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 98545.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -69,7 +69,7 @@
   static int stat;
   clang_analyzer_explain(x + 1); // expected-warning-re^\(argument 'x'\) \+ 1$
   clang_analyzer_explain(1 + y); // expected-warning-re^\(argument 'y'\) \+ 1$
-  clang_analyzer_explain(x + y); // expected-warning-re^unknown value$
+  clang_analyzer_explain(x + y); // expected-warning-re^\(argument 'x'\) \+ \(argument 'y'\)$
   clang_analyzer_explain(z); // expected-warning-re^undefined value$
   clang_analyzer_explain(); // expected-warning-re^pointer to local variable 'z'$
   clang_analyzer_explain(stat); // expected-warning-re^signed 32-bit integer '0'$
Index: test/Analysis/conditional-path-notes.c
===
--- test/Analysis/conditional-path-notes.c
+++ test/Analysis/conditional-path-notes.c
@@ -77,7 +77,8 @@
 
 void testNonDiagnosableBranchArithmetic(int a, int b) {
   if (a - b) {
-// expected-note@-1 {{Taking true branch}}
+// expected-note@-1 {{Assuming the condition is true}}
+// expected-note@-2 {{Taking true branch}}
 *(volatile int *)0 = 1; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1 {{Dereference of null pointer}}
   }
@@ -1573,12 +1574,75 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line79
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col11
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
@@ -1594,25 +1658,25 @@
 // CHECK-NEXT: start
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // 

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

It's been a while since I looked at the code, but I don't believe that all of 
the new constraints are necessarily unsupported by the current range constraint 
manager. Rather, they were just not being generated by the SimpleSValBuilder. 
The changes pass the testsuite for both the Range and Z3 constraint managers, 
without any special testcase handling that is Z3 specific.

Here are some runtime performance numbers on the testsuite:

Pre-Commit
RangeConstraintManager: 8.24s
Z3ConstraintManager: 247.29s

Post-Commit
RangeConstraintManager: 10.23s
Z3ConstraintManager: 250.35s

And for memory usage (max RSS):

Pre-Commit
edges-new.mm (RangeConstraintManager): 42688k
edges-new.mm (Z3ConstraintManager): 52916k
malloc-plist.c (RangeConstraintManager): 39812k
malloc-plist.c (Z3ConstraintManager): 50544k

Post-Commit
edges-new.mm (RangeConstraintManager): 43032k
edges-new.mm (Z3ConstraintManager): 52956k
malloc-plist.c (RangeConstraintManager): 40204k
malloc-plist.c (Z3ConstraintManager): 50364k


https://reviews.llvm.org/D28953



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

In https://reviews.llvm.org/D28952#750558, @iris wrote:

> How can I make z3constraintmanager.cpp work in the command line?Or how to 
> make z3 work?


You'll need a bleeding-edge build of Clang/LLVM, since this isn't available in 
any stable release yet. First, build or install a recent version of z3; 
`libz3-dev` in Debian/Ubuntu may work. Pass `-DCLANG_ANALYZER_BUILD_Z3=ON` to 
`cmake`, when building LLVM.  Then, when running `clang`, pass `-Xanalyzer 
-analyzer-constraints=z3`.


Repository:
  rL LLVM

https://reviews.llvm.org/D28952



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

@dcoughlin : ping


https://reviews.llvm.org/D28955



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

[PATCH] D31756: [cmake] Support Gentoo install for z3

[Dominic Chen via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D31756



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

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299463: [analyzer] Add new Z3 constraint manager backend 
(authored by ddcc).

Changed prior to commit:
  https://reviews.llvm.org/D28952?vs=93974=94107#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28952

Files:
  cfe/trunk/CMakeLists.txt
  cfe/trunk/cmake/modules/FindZ3.cmake
  cfe/trunk/include/clang/Config/config.h.cmake
  cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  cfe/trunk/test/Analysis/expr-inspection.c
  cfe/trunk/test/Analysis/lit.local.cfg
  cfe/trunk/test/Analysis/unsupported-types.c
  cfe/trunk/test/lit.cfg
  cfe/trunk/test/lit.site.cfg.in

Index: cfe/trunk/include/clang/Config/config.h.cmake
===
--- cfe/trunk/include/clang/Config/config.h.cmake
+++ cfe/trunk/include/clang/Config/config.h.cmake
@@ -38,6 +38,9 @@
 /* Define if we have libxml2 */
 #cmakedefine CLANG_HAVE_LIBXML ${CLANG_HAVE_LIBXML}
 
+/* Define if we have z3 and want to build it */
+#cmakedefine CLANG_ANALYZER_WITH_Z3 ${CLANG_ANALYZER_WITH_Z3}
+
 /* Define if we have sys/resource.h (rlimits) */
 #cmakedefine CLANG_HAVE_RLIMITS ${CLANG_HAVE_RLIMITS}
 
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
@@ -22,6 +22,7 @@
 #endif
 
 ANALYSIS_CONSTRAINTS(RangeConstraints, "range", "Use constraint tracking of concrete value ranges", CreateRangeConstraintManager)
+ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver", CreateZ3ConstraintManager)
 
 #ifndef ANALYSIS_DIAGNOSTICS
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -184,6 +184,9 @@
 CreateRangeConstraintManager(ProgramStateManager ,
  SubEngine *subengine);
 
+std::unique_ptr
+CreateZ3ConstraintManager(ProgramStateManager , SubEngine *subengine);
+
 } // end GR namespace
 
 } // end clang namespace
Index: cfe/trunk/cmake/modules/FindZ3.cmake
===
--- cfe/trunk/cmake/modules/FindZ3.cmake
+++ cfe/trunk/cmake/modules/FindZ3.cmake
@@ -0,0 +1,28 @@
+find_path(Z3_INCLUDE_DIR NAMES z3.h
+   PATH_SUFFIXES libz3
+   )
+
+find_library(Z3_LIBRARIES NAMES z3 libz3
+   )
+
+find_program(Z3_EXECUTABLE z3)
+
+if(Z3_INCLUDE_DIR AND Z3_EXECUTABLE)
+execute_process (COMMAND ${Z3_EXECUTABLE} -version
+  OUTPUT_VARIABLE libz3_version_str
+  ERROR_QUIET
+  OUTPUT_STRIP_TRAILING_WHITESPACE)
+
+string(REGEX REPLACE "^Z3 version ([0-9.]+)" "\\1"
+   Z3_VERSION_STRING "${libz3_version_str}")
+unset(libz3_version_str)
+endif()
+
+# handle the QUIETLY and REQUIRED arguments and set Z3_FOUND to TRUE if
+# all listed variables are TRUE
+include(FindPackageHandleStandardArgs)
+FIND_PACKAGE_HANDLE_STANDARD_ARGS(Z3
+  REQUIRED_VARS Z3_LIBRARIES Z3_INCLUDE_DIR
+  VERSION_VAR Z3_VERSION_STRING)
+
+mark_as_advanced(Z3_INCLUDE_DIR Z3_LIBRARIES)
Index: cfe/trunk/test/lit.site.cfg.in
===
--- cfe/trunk/test/lit.site.cfg.in
+++ cfe/trunk/test/lit.site.cfg.in
@@ -18,6 +18,7 @@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clang_staticanalyzer_z3 = "@CLANG_ANALYZER_WITH_Z3@"
 config.clang_examples = @CLANG_BUILD_EXAMPLES@
 config.enable_shared = @ENABLE_SHARED@
 config.enable_backtrace = @ENABLE_BACKTRACES@
Index: cfe/trunk/test/lit.cfg
===
--- cfe/trunk/test/lit.cfg
+++ cfe/trunk/test/lit.cfg
@@ -361,6 +361,9 @@
 if config.clang_staticanalyzer:
 config.available_features.add("staticanalyzer")
 
+if config.clang_staticanalyzer_z3 == '1':
+config.available_features.add("z3")
+
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
 config.available_features.add('crash-recovery')
Index: cfe/trunk/test/Analysis/unsupported-types.c
===
--- cfe/trunk/test/Analysis/unsupported-types.c
+++ cfe/trunk/test/Analysis/unsupported-types.c
@@ -0,0 +1,31 

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93974.
ddcc added a comment.

Fix support for 128-bit APInt creation, drop pkg-config from CMake module


https://reviews.llvm.org/D28952

Files:
  CMakeLists.txt
  cmake/modules/FindZ3.cmake
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Core/Analyses.def
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/expr-inspection.c
  test/Analysis/lit.local.cfg
  test/Analysis/unsupported-types.c
  test/lit.cfg
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -18,6 +18,7 @@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clang_staticanalyzer_z3 = "@CLANG_ANALYZER_WITH_Z3@"
 config.clang_examples = @CLANG_BUILD_EXAMPLES@
 config.enable_shared = @ENABLE_SHARED@
 config.enable_backtrace = @ENABLE_BACKTRACES@
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -361,6 +361,9 @@
 if config.clang_staticanalyzer:
 config.available_features.add("staticanalyzer")
 
+if config.clang_staticanalyzer_z3 == '1':
+config.available_features.add("z3")
+
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
 config.available_features.add('crash-recovery')
Index: test/Analysis/unsupported-types.c
===
--- /dev/null
+++ test/Analysis/unsupported-types.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-unknown-linux -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple powerpc64-linux-gnu -verify %s
+
+#define _Complex_I  (__extension__ 1.0iF)
+
+void clang_analyzer_eval(int);
+
+void complex_float(double _Complex x, double _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void complex_int(int _Complex x, int _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void longdouble_float(long double x, long double y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 0.0L && y != 1.0L)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 1.0L); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -10,6 +10,10 @@
 if result.code == lit.Test.FAIL:
 return result
 
+# If z3 backend available, add an additional run line for it
+if test.config.clang_staticanalyzer_z3 == '1':
+result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+
 return result
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -19,4 +19,4 @@
 
 // CHECK: Expressions:
 // CHECK-NEXT: clang_analyzer_printState : {clang_analyzer_printState}
-// CHECK-NEXT: Ranges are empty.
+// CHECK-NEXT: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -0,0 +1,1618 @@
+//== Z3ConstraintManager.cpp *- C++ -*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include 

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: cmake/modules/FindZ3.cmake:3
+# in the find_path() and find_library() calls
+find_package(PkgConfig QUIET)
+PKG_CHECK_MODULES(PC_Z3 QUIET libz3)

delcypher wrote:
> @ddcc Seeing as you don't want to use the new upstream Z3 CMake package 
> config files I'll try to review this.
> 
> Upstream Z3 does not come with pkg-config files for the native library so I'm 
> wondering where you expect this to work. Does a Linux distro add their own 
> `.pc` files?
> 
> It looks like you only use these paths as hints so this so this looks like 
> it'll work even without the pkg-config files.
See below.



Comment at: cmake/modules/FindZ3.cmake:5
+PKG_CHECK_MODULES(PC_Z3 QUIET libz3)
+set(Z3_DEFINITIONS ${PC_LIBZ3_CFLAGS_OTHER})
+

delcypher wrote:
> @ddcc This CMake variable is set but never used. Also based on the name it 
> suggests that they are compiler definitions rather than other compiler flags. 
> Does `_CFLAGS_OTHER` have those semantics? It's unclear from 
> https://cmake.org/cmake/help/v3.7/module/FindPkgConfig.html#command:pkg_check_modules
>  what they are supposed to be.
> 
> To consume these flags you could add `target_compile_definitions()` and 
> `target_compile_options()` to all users of Z3. A more elegant way of doing 
> this is to create an imported library target (e.g. `z3::libz3`) and set 
> compile definitions, compile options and include directories on this imported 
> target with `INTERFACE` visibility so that these usage requirements 
> implicitly propagate to all users of `z3::libz3`.
I'm not very familiar with CMake, so I based it off of the FindLibXml2.cmake 
from the upstream CMake repository. The `pkg-config` part isn't used currently, 
but I left it in case z3 does get a proper package.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:619
+
+llvm::APSInt Int = llvm::APSInt(Float.bitcastToAPInt(), true);
+Z3Expr Z3Int = Z3Expr::fromAPSInt(Int);

delcypher wrote:
> @ddcc Why use APSInt here? Why not APInt, we are looking at raw bits and 
> don't want to interpret the most significant bit in a special way.
Since the rest of the code already handles `APSInt`, I just reused that instead 
of implementing another method for `APInt`. The overhead is one additional bool 
used to store the sign, which doesn't make much difference, since it needs to 
be specified anyway with `APInt::toString*()`.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:668
+default:
+  llvm_unreachable("Unsupported sort to integer!");
+case Z3_BV_SORT: {

delcypher wrote:
> Is `Z3_FLOATING_POINT_SORT` possible in your implementation?
I don't think so. Things change around a bit with D28954, but by the time this 
method is called, the casting should have already been handled elsewhere.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:674
+  // are the same size.
+  Z3_get_numeral_uint64(Z3Context::ZC, AST,
+reinterpret_cast<__uint64 *>());

The only problem I see now is that if a IEEEquad is fed in, it will generate a 
128-bit bitvector, which will be truncated at this point. But the z3 API 
doesn't support retrieving a rational into anything larger than 64-bit, so 
perhaps converting it to string is the way to go here.


https://reviews.llvm.org/D28952



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

[PATCH] D28955: [analyzer] Enable support for symbolic extension/truncation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93591.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D28955

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/dead-stores.m
  test/Analysis/explain-svals.cpp
  test/Analysis/malloc.c
  test/Analysis/misc-ps-eager-assume.m
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -146,7 +146,7 @@
 void test_isgraph_isprint(int x) {
   char y = x;
   if (isgraph(y))
-clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
+clang_analyzer_eval(isprint(y)); // expected-warning{{TRUE}}
 }
 
 int isdigit(int);
Index: test/Analysis/misc-ps-eager-assume.m
===
--- test/Analysis/misc-ps-eager-assume.m
+++ test/Analysis/misc-ps-eager-assume.m
@@ -1,5 +1,4 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks %s -analyzer-eagerly-assume
-// expected-no-diagnostics
 
 // Delta-reduced header stuff (needed for test cases).
 typedef signed char BOOL;
@@ -56,7 +55,7 @@
 void handle_symbolic_cast_in_condition(void) {
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
 
-  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"];
+  BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"]; // expected-warning {{Assignment of a non-Boolean value}}
   NSMutableArray* array = needsAnArray ? [[NSMutableArray alloc] init] : 0;
   if(needsAnArray)
 [array release];
Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1656,13 +1656,13 @@
 void testOffsetPassedToStrlen() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
+  size_t length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
 }
 
 void testOffsetPassedToStrlenThenFree() {
   char * string = malloc(sizeof(char)*10);
   string += 1;
-  int length = strlen(string);
+  size_t length = strlen(string);
   free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}}
 }
 
@@ -1705,7 +1705,7 @@
 }
 
 char *dupstrNoWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocNoWarn(len + 1);
   strcpy(p, s); // no-warning
   return p;
@@ -1721,7 +1721,7 @@
 }
 
 char *dupstrWarn(const char *s) {
-  const int len = strlen(s);
+  const size_t len = strlen(s);
   char *p = (char*) smallocWarn(len + 1);
   strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
   return p;
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -41,11 +41,11 @@
 
 void test_2(char *ptr, int ext) {
   clang_analyzer_explain((void *) "asdf"); // expected-warning-re^pointer to element of type 'char' with index 0 of string literal "asdf"$
-  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
+  clang_analyzer_explain(strlen(ptr)); // expected-warning-re^cast of type 'int' of metadata of type 'unsigned long' tied to pointee of argument 'ptr'$
   clang_analyzer_explain(conjure()); // expected-warning-re^symbol of type 'int' conjured at statement 'conjure\(\)'$
   clang_analyzer_explain(glob); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob'$
   clang_analyzer_explain(glob_ptr); // expected-warning-re^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob_ptr'$
-  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^extent of pointee of argument 'ptr'$
+  clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re^cast of type 'int' of extent of pointee of argument 'ptr'$
   int *x = new int[ext];
   clang_analyzer_explain(x); // expected-warning-re^pointer to element of type 'int' with index 0 of heap segment that starts at symbol of type 'int \*' conjured at statement 'new int \[ext\]'$
   // Sic! What gets computed is the extent of the element-region.
Index: test/Analysis/dead-stores.m

[PATCH] D28954: [analyzer] Add support for symbolic float expressions

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93590.
ddcc added a comment.

Rebase, update tests, fix bugs


https://reviews.llvm.org/D28954

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
  include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Symbols.def
  lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/FloatingPointMath.cpp
  lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SVals.cpp
  lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/diagnostics/macros.cpp
  test/Analysis/float-rules.c
  test/Analysis/float.c
  test/Analysis/inline.cpp
  test/Analysis/lit.local.cfg
  test/Analysis/operator-calls.cpp

Index: test/Analysis/operator-calls.cpp
===
--- test/Analysis/operator-calls.cpp
+++ test/Analysis/operator-calls.cpp
@@ -81,8 +81,8 @@
   void test(int coin) {
 // Force a cache-out when we try to conjure a temporary region for the operator call.
 // ...then, don't crash.
-clang_analyzer_eval(+(coin ? getSmallOpaque() : getSmallOpaque())); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(+(coin ? getLargeOpaque() : getLargeOpaque())); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(+(coin ? getSmallOpaque() : getSmallOpaque())); // expected-warning{{TRUE}}
+clang_analyzer_eval(+(coin ? getLargeOpaque() : getLargeOpaque())); // expected-warning{{TRUE}}
   }
 }
 
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -1,20 +1,35 @@
+import lit.Test
 import lit.TestRunner
 import sys
 
 # Custom format class for static analyzer tests
 class AnalyzerTest(lit.formats.ShTest, object):
 
 def execute(self, test, litConfig):
-result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=range')
+results = []
 
-if result.code == lit.Test.FAIL:
-return result
+# Parse any test requirements ('REQUIRES: ')
+saved_test = test
+lit.TestRunner.parseIntegratedTestScript(test)
+
+if 'z3' not in test.requires:
+results.append(self.executeWithAnalyzeSubstitution(saved_test, litConfig, '-analyzer-constraints=range'))
+
+if results[-1].code == lit.Test.FAIL:
+return results[-1]
 
 # If z3 backend available, add an additional run line for it
 if test.config.clang_staticanalyzer_z3 == '1':
-result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+results.append(self.executeWithAnalyzeSubstitution(saved_test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3'))
 
-return result
+# Combine all result outputs into the last element
+for x in results:
+if x != results[-1]:
+results[-1].output = x.output + results[-1].output
+
+if results:
+return results[-1]
+return lit.Test.Result(lit.Test.UNSUPPORTED, "Test requires the following unavailable features: z3")
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
 saved_substitutions = list(test.config.substitutions)
Index: test/Analysis/inline.cpp
===
--- test/Analysis/inline.cpp
+++ test/Analysis/inline.cpp
@@ -285,11 +285,11 @@
   }
 
   void testFloatReference() {
-clang_analyzer_eval(defaultFloatReference(1) == -1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(defaultFloatReference() == -42); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(defaultFloatReference(1) == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(defaultFloatReference() == -42); // expected-warning{{TRUE}}
 
-clang_analyzer_eval(defaultFloatReferenceZero(1) == -1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(defaultFloatReferenceZero() == 0); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(defaultFloatReferenceZero(1) == -1); // expected-warning{{TRUE}}
+clang_analyzer_eval(defaultFloatReferenceZero() == 0); // expected-warning{{TRUE}}
   }
 
 

[PATCH] D28953: [analyzer] Eliminate analyzer limitations on symbolic constraint generation

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93589.
ddcc added a comment.

Rebase


https://reviews.llvm.org/D28953

Files:
  include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/bitwise-ops.c
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/std-c-library-functions.c

Index: test/Analysis/std-c-library-functions.c
===
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -57,8 +57,7 @@
   size_t y = fread(buf, sizeof(int), 10, fp);
   clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
   size_t z = fwrite(buf, sizeof(int), y, fp);
-  // FIXME: should be TRUE once symbol-symbol constraint support is improved.
-  clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
 }
 
 ssize_t getline(char **, size_t *, FILE *);
Index: test/Analysis/explain-svals.cpp
===
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -69,7 +69,7 @@
   static int stat;
   clang_analyzer_explain(x + 1); // expected-warning-re^\(argument 'x'\) \+ 1$
   clang_analyzer_explain(1 + y); // expected-warning-re^\(argument 'y'\) \+ 1$
-  clang_analyzer_explain(x + y); // expected-warning-re^unknown value$
+  clang_analyzer_explain(x + y); // expected-warning-re^\(argument 'x'\) \+ \(argument 'y'\)$
   clang_analyzer_explain(z); // expected-warning-re^undefined value$
   clang_analyzer_explain(); // expected-warning-re^pointer to local variable 'z'$
   clang_analyzer_explain(stat); // expected-warning-re^signed 32-bit integer '0'$
Index: test/Analysis/conditional-path-notes.c
===
--- test/Analysis/conditional-path-notes.c
+++ test/Analysis/conditional-path-notes.c
@@ -77,7 +77,8 @@
 
 void testNonDiagnosableBranchArithmetic(int a, int b) {
   if (a - b) {
-// expected-note@-1 {{Taking true branch}}
+// expected-note@-1 {{Assuming the condition is true}}
+// expected-note@-2 {{Taking true branch}}
 *(volatile int *)0 = 1; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1 {{Dereference of null pointer}}
   }
@@ -1573,12 +1574,75 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line79
+// CHECK-NEXT:   col7
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col7
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line79
+// CHECK-NEXT:  col11
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Assuming the condition is true
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line79
+// CHECK-NEXT:col7
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
@@ -1594,25 +1658,25 @@
 // CHECK-NEXT: start
 // CHECK-NEXT:  
 // CHECK-NEXT:   
-// CHECK-NEXT:line81
+// CHECK-NEXT:line82
 // CHECK-NEXT:col5
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // 

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93588.
ddcc added a comment.

Fix erroneous comment


https://reviews.llvm.org/D28952

Files:
  CMakeLists.txt
  cmake/modules/FindZ3.cmake
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Core/Analyses.def
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/expr-inspection.c
  test/Analysis/lit.local.cfg
  test/Analysis/unsupported-types.c
  test/lit.cfg
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -18,6 +18,7 @@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clang_staticanalyzer_z3 = "@CLANG_ANALYZER_WITH_Z3@"
 config.clang_examples = @CLANG_BUILD_EXAMPLES@
 config.enable_shared = @ENABLE_SHARED@
 config.enable_backtrace = @ENABLE_BACKTRACES@
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -361,6 +361,9 @@
 if config.clang_staticanalyzer:
 config.available_features.add("staticanalyzer")
 
+if config.clang_staticanalyzer_z3 == '1':
+config.available_features.add("z3")
+
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
 config.available_features.add('crash-recovery')
Index: test/Analysis/unsupported-types.c
===
--- /dev/null
+++ test/Analysis/unsupported-types.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-unknown-linux -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple powerpc64-linux-gnu -verify %s
+
+#define _Complex_I  (__extension__ 1.0iF)
+
+void clang_analyzer_eval(int);
+
+void complex_float(double _Complex x, double _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void complex_int(int _Complex x, int _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void longdouble_float(long double x, long double y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 0.0L && y != 1.0L)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 1.0L); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -10,6 +10,10 @@
 if result.code == lit.Test.FAIL:
 return result
 
+# If z3 backend available, add an additional run line for it
+if test.config.clang_staticanalyzer_z3 == '1':
+result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+
 return result
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -19,4 +19,4 @@
 
 // CHECK: Expressions:
 // CHECK-NEXT: clang_analyzer_printState : {clang_analyzer_printState}
-// CHECK-NEXT: Ranges are empty.
+// CHECK-NEXT: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -0,0 +1,1606 @@
+//== Z3ConstraintManager.cpp *- C++ -*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h"
+
+#include "clang/Config/config.h"
+

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc updated this revision to Diff 93587.
ddcc marked 4 inline comments as done.
ddcc added a comment.

Use direct bitcasting instead of string conversion for APFloat casting, add 
reference counting for Z3_sort, eliminate some duplicate code


https://reviews.llvm.org/D28952

Files:
  CMakeLists.txt
  cmake/modules/FindZ3.cmake
  include/clang/Config/config.h.cmake
  include/clang/StaticAnalyzer/Core/Analyses.def
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/expr-inspection.c
  test/Analysis/lit.local.cfg
  test/Analysis/unsupported-types.c
  test/lit.cfg
  test/lit.site.cfg.in

Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -18,6 +18,7 @@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
+config.clang_staticanalyzer_z3 = "@CLANG_ANALYZER_WITH_Z3@"
 config.clang_examples = @CLANG_BUILD_EXAMPLES@
 config.enable_shared = @ENABLE_SHARED@
 config.enable_backtrace = @ENABLE_BACKTRACES@
Index: test/lit.cfg
===
--- test/lit.cfg
+++ test/lit.cfg
@@ -361,6 +361,9 @@
 if config.clang_staticanalyzer:
 config.available_features.add("staticanalyzer")
 
+if config.clang_staticanalyzer_z3 == '1':
+config.available_features.add("z3")
+
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
 config.available_features.add('crash-recovery')
Index: test/Analysis/unsupported-types.c
===
--- /dev/null
+++ test/Analysis/unsupported-types.c
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-unknown-linux -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple powerpc64-linux-gnu -verify %s
+
+#define _Complex_I  (__extension__ 1.0iF)
+
+void clang_analyzer_eval(int);
+
+void complex_float(double _Complex x, double _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void complex_int(int _Complex x, int _Complex y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 1.0 + 3.0 * _Complex_I && y != 1.0 - 4.0 * _Complex_I)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 2.0 - 1.0 * _Complex_I); // expected-warning{{UNKNOWN}}
+}
+
+void longdouble_float(long double x, long double y) {
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  if (x != 0.0L && y != 1.0L)
+return
+  clang_analyzer_eval(x == y); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(x + y == 1.0L); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/lit.local.cfg
===
--- test/Analysis/lit.local.cfg
+++ test/Analysis/lit.local.cfg
@@ -10,6 +10,10 @@
 if result.code == lit.Test.FAIL:
 return result
 
+# If z3 backend available, add an additional run line for it
+if test.config.clang_staticanalyzer_z3 == '1':
+result = self.executeWithAnalyzeSubstitution(test, litConfig, '-analyzer-constraints=z3 -DANALYZER_CM_Z3')
+
 return result
 
 def executeWithAnalyzeSubstitution(self, test, litConfig, substitution):
Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -19,4 +19,4 @@
 
 // CHECK: Expressions:
 // CHECK-NEXT: clang_analyzer_printState : {clang_analyzer_printState}
-// CHECK-NEXT: Ranges are empty.
+// CHECK-NEXT: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -0,0 +1,1606 @@
+//== Z3ConstraintManager.cpp *- C++ -*--==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include 

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added a comment.

Thanks for the feedback! My main constraint is that the results from the 
floating-point analysis weren't very interesting (see #652894 <#652894>), so 
I'm not actively working on further development.

> FYI I've been working on floating point support in KLEE and have extended it 
> to support floating point (note however only the Z3 backend actually supports 
> consuming floating point constraints). I've not yet open sourced what I've 
> done as I'm not entirely happy with the design but if there is interest we 
> could see if we could figure out a way of pulling klee::Expr (and the solver 
> bits) out of KLEE to make a standalone library. Note there is a project 
> called metaSMT that uses template meta programming to give the same interface 
> to multiple solvers. KLEE does support it but I'm not familiar with it.

I agree that it'd be useful to move to a more generic SMT interface, 
potentially SMT-LIB instead of something specific to a solver, but this is more 
of a long-term goal at the moment.

> KLEE has a few optimization ideas that you could consider implementing that 
> certainly help in the context of symbolic execution...

Likewise, I think adding a constraint cache and implementing performance 
optimizations would be the next step to getting this enabled by default, and 
perhaps deprecating the range constraint manager, but the implementation is a 
little tricky because there's state also being stored in the `ProgramState` 
object. Since those are reference counted, there's a little more work involved 
than just adding an intermediate layer with e.g. an `std::map`. But again, it's 
not something I have the time for at the moment.




Comment at: CMakeLists.txt:188
 
+find_package(Z3 4.5)
+

delcypher wrote:
> delcypher wrote:
> > @ddcc It is of course up to you but I recently [[ added support for using 
> > `libz3` directly | added support for using `libz3` directly ]] from CMake. 
> > via it's own CMake config package. You only get this if Z3 was built with 
> > CMake so you might not want this restriction.  This feature has only just 
> > landed though and might not be sufficient for your needs.  If you take a 
> > look at Z3's example projects they are now built with this mechanism when 
> > building with CMake.
> > 
> > If you are interested I'd be more than happy to work with you to get this 
> > feature ready for your needs in upstream Z3 so you can use it here.
> Sorry that URL should be https://github.com/Z3Prover/z3/pull/926
I think this is useful, and upstream z3 has been in need of better packaging. 
But until it's used by default over the current python script and the z3 folks 
actively maintain it, I'm a little hesitant to depend on it.



Comment at: include/clang/Config/config.h.cmake:42
+/* Define if we have z3 and want to build it */
+#cmakedefine CLANG_ANALYZER_WITH_Z3 ${CLANG_ANALYZER_WITH_Z3}
+

delcypher wrote:
> Do you really want such a specific name? How about 
> `CLANG_ANALYSER_HAS_SMT_SOLVER`?
There's been a bit of back and forth over this name in previous revisions, so 
I'm a little hesitant to change it. In essence, there are two CMake variables, 
one for whether z3 is present, and another for whether the user requested to 
build with z3, which are exported to a single C-level definition that is true 
iff both CMake variables are true.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:70
+public:
+  static Z3_context ZC;
+

delcypher wrote:
> @ddc
> This decision of having a global context might come back to bite you. 
> Especially if you switch to doing parallel solving in the future. This is why 
> KLEE's `Z3ASTHandle` and `Z3SortHandle` store the context so it's possible to 
> use different context.
I agree that it is an inherent limitation, but since the static analyzer itself 
is only single-threaded anyway, the entire analyzer will need significant 
changes before this becomes a problem.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:81
+
+class Z3Expr {
+  friend class Z3Model;

delcypher wrote:
> @ddcc 
> [[ 
> https://github.com/klee/klee/blob/1f13e9dbf9db2095b6612a47717c2b86e4aaba72/lib/Solver/Z3Builder.h#L20
>  | In KLEE I have something similar to represent Z3Expr ]] called Z3ASTHandle 
> and Z3SortHandle for doing manual reference counting. You might want to take 
> a look at. I don't see you doing reference counting on sorts here so I think 
> you might be leaking memory.
> 
> We also have a handy `dump()` method on `Z3ASTHandle` and `Z3SortHandle` for 
> debugging.
Originally, my understanding was that reference counting wasn't necessary for 
`Z3_sort` objects, based on the API documentation, and the lack of 
`Z3_sort_inc_ref()`/`Z3_sort_dec_ref()` functions. But, taking another look at 
the z3 source code, it seems that `Z3_sort` is casted directly to `Z3_ast`,  so 
I 

[PATCH] D28952: [analyzer] Add new Z3 constraint manager backend

[Dominic Chen via Phabricator via cfe-commits]

ddcc added inline comments.



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:60
+// Set timeout to 15000ms = 15s
+Z3_set_param_value(Config, "timeout", "15000");
+  }

xazax.hun wrote:
> Sorry for being a bit late in the party, I was wondering whether this timeout 
> can be a source of non-determinism. The constraint solver might get lucky or 
> unlucky based on the load or the underlying hardware to solve some of the 
> constraints. We might end up with different results over different runs which 
> can be confusing for the users. Is there any other way, to set something like 
> a timeout, like limiting the number of steps inside the solver?
I believe Z3 uses various heuristics internally, so the exact execution trace 
can differ between multiple executions. However, I don't believe that this 
would be a common issue, since queries on fixed-width bitvector are decidable. 
The 15 sec timeout is intended to prevent a single query from stalling the 
static analyzer; I've only experienced this with complex queries over 
floating-point types (e.g. D28953 + D28954 + D28955), which can degenerate to 
bitblasting. I don't have the exact numbers right now, but given that the 200+ 
tests in the testsuite complete in ~90 secs with this patch, the average time 
per test is < 0.5 sec, which includes tens if not hundreds of individual SMT 
queries. As far as I know, this timeout parameter is the only support 
configuration parameter. I'd also like to point out that this solver isn't used 
unless the user both builds with it and specifies it at runtime.


https://reviews.llvm.org/D28952



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