[PATCH] D66045: Improve detection of same value in comparisons

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: test/SemaCXX/self-comparison.cpp:87
+};
+} // namespace member_tests

rtrieu wrote:
> jfb wrote:
> > rtrieu wrote:
> > > rtrieu wrote:
> > > > jfb wrote:
> > > > > The test only has `==`. Do other operators trigger?
> > > > All the standard comparison operators will work here.  I'll add tests.
> > > All operator tests added.
> > `operator<=>`?
> > 
> > Is there a separate test for user-defined operators as well? What makes 
> > sense in these cases? Technically a user-defined comparison could do 
> > anything... but I think this warning makes sense for them as well.
> operator<=> test was added to test/SemaCXX/compare-cxx2a.cpp because the 
> operator requires header that this test doesn't include.
> 
> The warning currently doesn't check user-defined operators and I've include a 
> test below to show that.  If a warning for user-defined operators was 
> created, it would not belong in -Wtautological-compare because we would not 
> know the actual result.  The best we could do is to say there was a self 
> compare with a user-defined operator.  It would also need to hook into the 
> AST at a different point.  Builtin compare operators are checked in 
> BinaryOperator nodes.  User-defined operators would need to be checked at 
> CallExpr or CXXOperatorCallExpr instead.  This is different enough that it 
> should be in a different patch.
Thanks, this looks great!


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

https://reviews.llvm.org/D66045



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

[PATCH] D66068: cmake: Make building clang-shlib optional

[Jan Vesely via Phabricator via cfe-commits]

jvesely added a comment.

In D66068#1627706 , @beanz wrote:

> I want to dissect this a bit.
>
> In D66068#1627451 , @E5ten wrote:
>
> > I am in favour of adding a user-facing option to disable generating this 
> > duplicate library for users that don't need it
>
>
> Why do you call this duplicate? It is unique. There is no other library in 
> the clang build that serves the role of this library.


It duplicates functionality provided by separate/component libraries.
Why can't this be an option the same way I can pick when building llvm?

>> there should be an option to disable linking a library that takes a long 
>> time to link and isn't necessary for a lot of users.
> 
> I think this is nuanced. When you say "takes a long time to link", I'm 
> curious why you say that. For me it takes 45s to link on my laptop in a Linux 
> VM using LLD in a build configuration that also includes all our backends 
> (which is kinda a worst-case scenario), and 10s to link if I only include 
> X86. That doesn't seem like a super long time, and it doesn't rely on any 
> billion-dollar compute farms.

Is this a debug build?

> While it does slow down full-build times (slightly), I think the benefit is 
> less broken bots which benefits the community as a whole.

do you have any numbers to support that claim?

> Going back to @jvesely's original email, I'm not sure why this adds minutes 
> to your build time. I'd be curious if there are other low-hanging fruit that 
> would improve your productivity without the community cost of adding new 
> build configurations that disable building and testing things that we 
> actually ship.

My first guess would be the difference between debug and release build 
(presence of debug info). the size of the library is 1.6GB on my system:

  $ du -h /usr/local/llvm-git/lib/libclang-cpp.so.10svn 
  1.6G  /usr/local/llvm-git/lib/libclang-cpp.so.10svn

just reading the inputs and writing the output library will take 30s on a 
100MB/s hdd (my laptop is slower than that) and it hasn't done any linking yet.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66068



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

r368796 - Remove unreachable blocks before splitting a coroutine.

[John McCall via cfe-commits]

Author: rjmccall
Date: Tue Aug 13 20:54:13 2019
New Revision: 368796

URL: http://llvm.org/viewvc/llvm-project?rev=368796=rev
Log:
Remove unreachable blocks before splitting a coroutine.

The suspend-crossing algorithm is not correct in the presence of uses
that cannot be reached on some successor path from their defs.

Added:
cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll

Added: cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll?rev=368796=auto
==
--- cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll (added)
+++ cfe/trunk/test/CodeGenCoroutines/coro-retcon-unreachable.ll Tue Aug 13 
20:54:13 2019
@@ -0,0 +1,46 @@
+; RUN: opt < %s -coro-early -coro-split -S | FileCheck %s
+target datalayout = "E-p:64:64"
+
+%swift.type = type { i64 }
+%swift.opaque = type opaque
+%T4red215EmptyCollectionV = type opaque
+%TSi = type <{ i64 }>
+
+define hidden swiftcc { i8*, %swift.opaque* } @no_suspends(i8* %buffer, i64 
%arg) #1 {
+  %id = call token @llvm.coro.id.retcon.once(i32 32, i32 8, i8* %buffer, i8* 
bitcast (void (i8*, i1)* @prototype to i8*), i8* bitcast (i8* (i64)* @malloc to 
i8*), i8* bitcast (void (i8*)* @free to i8*))
+  %begin = call i8* @llvm.coro.begin(token %id, i8* null)
+  call void @print(i64 %arg)
+  call void @llvm.trap()
+  unreachable
+
+bb1:
+  call void @print(i64 %arg)
+  call i1 @llvm.coro.end(i8* %begin, i1 false)
+  unreachable
+}
+; CHECK-LABEL: define hidden swiftcc { i8*, %swift.opaque* } @no_suspends(
+; CHECK: call token @llvm.coro.id.retcon.once
+; CHECK-NEXT:call void @print(i64 %arg)
+; CHECK-NEXT:call void @llvm.trap()
+; CHECK-NEXT:unreachable
+
+declare swiftcc void @prototype(i8* noalias dereferenceable(32), i1)
+declare void @print(i64)
+
+declare noalias i8* @malloc(i64) #5
+declare void @free(i8* nocapture) #5
+
+declare token @llvm.coro.id.retcon.once(i32, i32, i8*, i8*, i8*, i8*) #5
+declare i8* @llvm.coro.begin(token, i8* writeonly) #5
+declare token @llvm.coro.alloca.alloc.i64(i64, i32) #5
+declare i8* @llvm.coro.alloca.get(token) #5
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) #6
+declare i1 @llvm.coro.suspend.retcon.i1(...) #5
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) #6
+declare void @llvm.coro.alloca.free(token) #5
+declare i1 @llvm.coro.end(i8*, i1) #5
+
+declare void @llvm.trap()
+
+attributes #1 = { noreturn nounwind }
+attributes #5 = { nounwind }


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

[PATCH] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

[Richard Trieu via Phabricator via cfe-commits]

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

In D66044#1626008 , @NoQ wrote:

> Looks great, thanks! I'd appreciate direct CFG tests for the part that 
> changes the CFG (cf. `test/Analysis/cfg.cpp`), but i don't insist. Let's make 
> sure @jfb is happy and commit :)


I added a test case to show the new unreachable node in the CFG.


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

https://reviews.llvm.org/D66044



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

[PATCH] D66044: Extend -Wtautological-overlap-compare to accept negative values and integer conversions

[Richard Trieu via Phabricator via cfe-commits]

rtrieu updated this revision to Diff 215021.

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

https://reviews.llvm.org/D66044

Files:
  lib/Analysis/CFG.cpp
  lib/Analysis/ReachableCode.cpp
  test/Analysis/cfg.cpp
  test/Sema/warn-overlap.c
  test/Sema/warn-unreachable.c
  test/SemaCXX/warn-unreachable.cpp

Index: test/SemaCXX/warn-unreachable.cpp
===
--- test/SemaCXX/warn-unreachable.cpp
+++ test/SemaCXX/warn-unreachable.cpp
@@ -393,6 +393,9 @@
   else
 calledFun();// expected-warning {{will never be executed}}
 
+  if (y == -1 && y != -1)  // expected-note {{silence}}
+calledFun();// expected-warning {{will never be executed}}
+
   // TODO: Extend warning to the following code:
   if (x < -1)
 calledFun();
@@ -408,6 +411,4 @@
   else
 calledFun();
 
-  if (y == -1 && y != -1)
-calledFun();
 }
Index: test/Sema/warn-unreachable.c
===
--- test/Sema/warn-unreachable.c
+++ test/Sema/warn-unreachable.c
@@ -433,7 +433,7 @@
 }
 
 void unaryOpNoFixit() {
-  if (- 1)
+  if (~ 1)
 return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
   unaryOpNoFixit(); // expected-warning {{code will never be executed}}
 }
Index: test/Sema/warn-overlap.c
===
--- test/Sema/warn-overlap.c
+++ test/Sema/warn-overlap.c
@@ -141,3 +141,20 @@
   return x < 1 || x != 0;
   // expected-warning@-1{{overlapping comparisons always evaluate to true}}
 }
+
+int integer_conversion(unsigned x, int y) {
+  return x > 4 || x < 10;
+  // expected-warning@-1{{overlapping comparisons always evaluate to true}}
+  return y > 4u || y < 10u;
+  // expected-warning@-1{{overlapping comparisons always evaluate to true}}
+}
+
+int negative_compare(int x) {
+  return x > -1 || x < 1;
+  // expected-warning@-1{{overlapping comparisons always evaluate to true}}
+}
+
+int no_warning(unsigned x) {
+  return x >= 0 || x == 1;
+  // no warning since "x >= 0" is caught by a different tautological warning.
+}
Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -498,6 +498,26 @@
 }
 } // end namespace pr37688_deleted_union_destructor
 
+// CHECK-LABEL: int overlap_compare(int x)
+// CHECK: [B2]
+// CHECK-NEXT:   1: 1
+// CHECK-NEXT:  2: return [B2.1];
+// CHECK-NEXT:  Preds (1): B3(Unreachable)
+// CHECK-NEXT:  Succs (1): B0
+// CHECK: [B3]
+// CHECK-NEXT:   1: x
+// CHECK-NEXT:   2: [B3.1] (ImplicitCastExpr, LValueToRValue, int)
+// CHECK-NEXT:   3: 5
+// CHECK-NEXT:   4: [B3.2] > [B3.3]
+// CHECK-NEXT:   T: if [B4.5] && [B3.4]
+// CHECK-NEXT:   Preds (1): B4
+// CHECK-NEXT:   Succs (2): B2(Unreachable) B1
+int overlap_compare(int x) {
+  if (x == -1 && x > 5)
+return 1;
+
+  return 2;
+}
 
 // CHECK-LABEL: template<> int *PR18472()
 // CHECK: [B2 (ENTRY)]
@@ -522,4 +542,3 @@
 void PR18472_helper() {
   PR18472();
 }
-
Index: lib/Analysis/ReachableCode.cpp
===
--- lib/Analysis/ReachableCode.cpp
+++ lib/Analysis/ReachableCode.cpp
@@ -247,7 +247,7 @@
 }
 case Stmt::UnaryOperatorClass: {
   const UnaryOperator *UO = cast(S);
-  if (UO->getOpcode() != UO_LNot)
+  if (UO->getOpcode() != UO_LNot && UO->getOpcode() != UO_Minus)
 return false;
   bool SilenceableCondValNotSet =
   SilenceableCondVal && SilenceableCondVal->getBegin().isInvalid();
Index: lib/Analysis/CFG.cpp
===
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -70,11 +70,35 @@
   return D->getLocation();
 }
 
+/// Returns true on constant values based around a single IntegerLiteral.
+/// Allow for use of parentheses, integer casts, and negative signs.
+static bool IsIntegerLiteralConstantExpr(const Expr *E) {
+  // Allow parentheses
+  E = E->IgnoreParens();
+
+  // Allow conversions to different integer kind.
+  if (const auto *CE = dyn_cast(E)) {
+if (CE->getCastKind() != CK_IntegralCast)
+  return false;
+E = CE->getSubExpr();
+  }
+
+  // Allow negative numbers.
+  if (const auto *UO = dyn_cast(E)) {
+if (UO->getOpcode() != UO_Minus)
+  return false;
+E = UO->getSubExpr();
+  }
+
+  return isa(E);
+}
+
 /// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral
-/// or EnumConstantDecl from the given Expr. If it fails, returns nullptr.
+/// constant expression or EnumConstantDecl from the given Expr. If it fails,
+/// returns nullptr.
 static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) {
   E = E->IgnoreParens();
-  if (isa(E))
+  if (IsIntegerLiteralConstantExpr(E))
 return E;
   if (auto *DR = dyn_cast(E->IgnoreParenImpCasts()))
 return isa(DR->getDecl()) ? DR : nullptr;
@@ -121,11 +145,11 @@
 static bool 

[PATCH] D66100: Add __has_builtin support for builtin function-like type traits.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368785: Add __has_builtin support for builtin function-like 
type traits. (authored by rsmith, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66100?vs=214673=215017#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66100

Files:
  cfe/trunk/docs/LanguageExtensions.rst
  cfe/trunk/include/clang/Basic/Features.def
  cfe/trunk/include/clang/Basic/TokenKinds.def
  cfe/trunk/lib/Lex/PPMacroExpansion.cpp
  cfe/trunk/test/Preprocessor/feature_tests.c
  cfe/trunk/test/Preprocessor/feature_tests.cpp

Index: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
===
--- cfe/trunk/lib/Lex/PPMacroExpansion.cpp
+++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp
@@ -1617,21 +1617,38 @@
 return true;
   }
   return true;
+} else if (II->getTokenID() != tok::identifier ||
+   II->hasRevertedTokenIDToIdentifier()) {
+  // Treat all keywords that introduce a custom syntax of the form
+  //
+  //   '__some_keyword' '(' [...] ')'
+  //
+  // as being "builtin functions", even if the syntax isn't a valid
+  // function call (for example, because the builtin takes a type
+  // argument).
+  if (II->getName().startswith("__builtin_") ||
+  II->getName().startswith("__is_") ||
+  II->getName().startswith("__has_"))
+return true;
+  return llvm::StringSwitch(II->getName())
+  .Case("__array_rank", true)
+  .Case("__array_extent", true)
+  .Case("__reference_binds_to_temporary", true)
+  .Case("__underlying_type", true)
+  .Default(false);
 } else {
   return llvm::StringSwitch(II->getName())
-  .Case("__make_integer_seq", LangOpts.CPlusPlus)
-  .Case("__type_pack_element", LangOpts.CPlusPlus)
-  .Case("__builtin_available", true)
-  .Case("__is_target_arch", true)
-  .Case("__is_target_vendor", true)
-  .Case("__is_target_os", true)
-  .Case("__is_target_environment", true)
-  .Case("__builtin_LINE", true)
-  .Case("__builtin_FILE", true)
-  .Case("__builtin_FUNCTION", true)
-  .Case("__builtin_COLUMN", true)
-  .Case("__builtin_bit_cast", true)
-  .Default(false);
+  // Report builtin templates as being builtins.
+  .Case("__make_integer_seq", LangOpts.CPlusPlus)
+  .Case("__type_pack_element", LangOpts.CPlusPlus)
+  // Likewise for some builtin preprocessor macros.
+  // FIXME: This is inconsistent; we usually suggest detecting
+  // builtin macros via #ifdef. Don't add more cases here.
+  .Case("__is_target_arch", true)
+  .Case("__is_target_vendor", true)
+  .Case("__is_target_os", true)
+  .Case("__is_target_environment", true)
+  .Default(false);
 }
   });
   } else if (II == Ident__is_identifier) {
Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -38,7 +38,9 @@
 -
 
 This function-like macro takes a single identifier argument that is the name of
-a builtin function.  It evaluates to 1 if the builtin is supported or 0 if not.
+a builtin function, a builtin pseudo-function (taking one or more type
+arguments), or a builtin template.
+It evaluates to 1 if the builtin is supported or 0 if not.
 It can be used like this:
 
 .. code-block:: c++
@@ -55,6 +57,14 @@
   #endif
   ...
 
+.. note::
+
+  Prior to Clang 10, ``__has_builtin`` could not be used to detect most builtin
+  pseudo-functions.
+
+  ``__has_builtin`` should not be used to detect support for a builtin macro;
+  use ``#ifdef`` instead.
+
 .. _langext-__has_feature-__has_extension:
 
 ``__has_feature`` and ``__has_extension``
@@ -1041,8 +1051,8 @@
 
 More information could be found `here `_.
 
-Checks for Type Trait Primitives
-
+Type Trait Primitives
+=
 
 Type trait primitives are special builtin constant expressions that can be used
 by the standard C++ library to facilitate or simplify the implementation of
@@ -1058,20 +1068,173 @@
 
 Clang supports the `GNU C++ type traits
 `_ and a subset of the
-`Microsoft Visual C++ Type traits

r368785 - Add __has_builtin support for builtin function-like type traits.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Aug 13 19:30:11 2019
New Revision: 368785

URL: http://llvm.org/viewvc/llvm-project?rev=368785=rev
Log:
Add __has_builtin support for builtin function-like type traits.

Summary:
Previously __has_builtin(__builtin_*) would return false for
__builtin_*s that we modeled as keywords rather than as functions
(because they take type arguments). With this patch, all builtins
that are called with function-call-like syntax return true from
__has_builtin (covering __builtin_* and also the __is_* and __has_* type
traits and the handful of similar builtins without such a prefix).

Update the documentation on __has_builtin and on type traits to match.
While doing this I noticed the type trait documentation was out of date
and incomplete; that's fixed here too.

Reviewers: aaron.ballman

Subscribers: jfb, kristina, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Preprocessor/feature_tests.cpp
Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/include/clang/Basic/Features.def
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/lib/Lex/PPMacroExpansion.cpp
cfe/trunk/test/Preprocessor/feature_tests.c

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=368785=368784=368785=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Tue Aug 13 19:30:11 2019
@@ -38,7 +38,9 @@ version checks".
 -
 
 This function-like macro takes a single identifier argument that is the name of
-a builtin function.  It evaluates to 1 if the builtin is supported or 0 if not.
+a builtin function, a builtin pseudo-function (taking one or more type
+arguments), or a builtin template.
+It evaluates to 1 if the builtin is supported or 0 if not.
 It can be used like this:
 
 .. code-block:: c++
@@ -55,6 +57,14 @@ It can be used like this:
   #endif
   ...
 
+.. note::
+
+  Prior to Clang 10, ``__has_builtin`` could not be used to detect most builtin
+  pseudo-functions.
+
+  ``__has_builtin`` should not be used to detect support for a builtin macro;
+  use ``#ifdef`` instead.
+
 .. _langext-__has_feature-__has_extension:
 
 ``__has_feature`` and ``__has_extension``
@@ -1041,8 +1051,8 @@ For example, compiling code with ``-fmod
 
 More information could be found `here 
`_.
 
-Checks for Type Trait Primitives
-
+Type Trait Primitives
+=
 
 Type trait primitives are special builtin constant expressions that can be used
 by the standard C++ library to facilitate or simplify the implementation of
@@ -1058,20 +1068,173 @@ the supported set of system headers, cur
 
 Clang supports the `GNU C++ type traits
 `_ and a subset of the
-`Microsoft Visual C++ Type traits
-`_.
+`Microsoft Visual C++ type traits
+`_,
+as well as nearly all of the
+`Embarcadero C++ type traits
+`_.
+
+The following type trait primitives are supported by Clang. Those traits marked
+(C++) provide implementations for type traits specified by the C++ standard;
+``__X(...)`` has the same semantics and constraints as the corresponding
+``std::X_t<...>`` or ``std::X_v<...>`` type trait.
+
+* ``__array_rank(type)`` (Embarcadero):
+  Returns the number of levels of array in the type ``type``:
+  ``0`` if ``type`` is not an array type, and
+  ``__array_rank(element) + 1`` if ``type`` is an array of ``element``.
+* ``__array_extent(type, dim)`` (Embarcadero):
+  The ``dim``'th array bound in the type ``type``, or ``0`` if
+  ``dim >= __array_rank(type)``.
+* ``__has_nothrow_assign`` (GNU, Microsoft, Embarcadero):
+  Deprecated, use ``__is_nothrow_assignable`` instead.
+* ``__has_nothrow_move_assign`` (GNU, Microsoft):
+  Deprecated, use ``__is_nothrow_assignable`` instead.
+* ``__has_nothrow_copy`` (GNU, Microsoft):
+  Deprecated, use ``__is_nothrow_constructible`` instead.
+* ``__has_nothrow_constructor`` (GNU, Microsoft):
+  Deprecated, use ``__is_nothrow_constructible`` instead.
+* ``__has_trivial_assign`` (GNU, Microsoft, Embarcadero):
+  Deprecated, use ``__is_trivially_assignable`` instead.
+* ``__has_trivial_move_assign`` (GNU, Microsoft):
+  Deprecated, use ``__is_trivially_assignable`` instead.
+* ``__has_trivial_copy`` (GNU, Microsoft):
+  Deprecated, use ``__is_trivially_constructible`` instead.
+* ``__has_trivial_constructor`` (GNU, Microsoft):
+  Deprecated, use ``__is_trivially_constructible`` instead.
+* ``__has_trivial_move_constructor`` (GNU, Microsoft):
+  Deprecated, use 

[PATCH] D65182: [analyzer] Add fix-it hint support.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 215011.
NoQ added a comment.

Rebase!


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

https://reviews.llvm.org/D65182

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/dead-stores.c
  clang/test/Analysis/edges-new.mm
  clang/test/Analysis/objc-arc.m
  clang/test/Analysis/plist-output.m
  clang/test/Analysis/virtualcall-fixits.cpp

Index: clang/test/Analysis/virtualcall-fixits.cpp
===
--- /dev/null
+++ clang/test/Analysis/virtualcall-fixits.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN: -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
+// RUN: %s 2>&1 | FileCheck -check-prefix=TEXT %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN: -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
+// RUN: -analyzer-config fixits-as-remarks=true \
+// RUN: -analyzer-output=plist -o %t.plist -verify %s
+// RUN: cat %t.plist | FileCheck -check-prefix=PLIST %s
+
+struct S {
+  virtual void foo();
+  S() {
+foo();
+// expected-warning@-1{{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
+// expected-remark-re@-2.*}}:5 - {{.*}}:5 - 'S::'}}
+  }
+  ~S();
+};
+
+// TEXT: warning: Call to virtual method 'S::foo' during construction
+// TEXT-SAME: bypasses virtual dispatch
+// TEXT-NEXT: foo();
+// TEXT-NEXT: ^
+// TEXT-NEXT: S::
+// TEXT-NEXT: 1 warning generated.
+
+// PLIST:  fixits
+// PLIST-NEXT:  
+// PLIST-NEXT:   
+// PLIST-NEXT:remove_range
+// PLIST-NEXT:
+// PLIST-NEXT: 
+// PLIST-NEXT:  line13
+// PLIST-NEXT:  col5
+// PLIST-NEXT:  file0
+// PLIST-NEXT: 
+// PLIST-NEXT: 
+// PLIST-NEXT:  line13
+// PLIST-NEXT:  col4
+// PLIST-NEXT:  file0
+// PLIST-NEXT: 
+// PLIST-NEXT:
+// PLIST-NEXT:insert_string
+// PLIST-NEXT:S::
+// PLIST-NEXT:   
+// PLIST-NEXT:  
Index: clang/test/Analysis/plist-output.m
===
--- clang/test/Analysis/plist-output.m
+++ clang/test/Analysis/plist-output.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -o %t.plist
+// RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t.plist
 // RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/plist-output.m.plist -
 
 void test_null_init(void) {
Index: clang/test/Analysis/objc-arc.m
===
--- clang/test/Analysis/objc-arc.m
+++ clang/test/Analysis/objc-arc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.cocoa.RetainCount,deadcode -verify -fblocks -analyzer-opt-analyze-nested-blocks -fobjc-arc -analyzer-output=plist-multi-file -o %t.plist %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.cocoa.RetainCount,deadcode -verify -fblocks -analyzer-opt-analyze-nested-blocks -fobjc-arc -analyzer-output=plist-multi-file -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t.plist %s
 // RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/objc-arc.m.plist -
 
 typedef signed char BOOL;
Index: clang/test/Analysis/edges-new.mm
===
--- clang/test/Analysis/edges-new.mm
+++ clang/test/Analysis/edges-new.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,deadcode.DeadStores,osx.cocoa.RetainCount,unix.Malloc,unix.MismatchedDeallocator -analyzer-output=plist -o %t -w %s
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,deadcode.DeadStores,osx.cocoa.RetainCount,unix.Malloc,unix.MismatchedDeallocator -analyzer-output=plist -analyzer-config deadcode.DeadStores:ShowFixIts=true -o %t -w %s
 // RUN: %normalize_plist <%t | diff -ub %S/Inputs/expected-plists/edges-new.mm.plist -
 
 

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 215007.
NoQ added a comment.

Make the checkers independent and extract modeling into a dependency.

This means that you can potentially enable the non-pure-virtual call checker 
without enabling the pure virtual call checker. This doesn't contradict 
backwards compatibility as long as the driver's default checker list is 
trusted, because the pure virtual call checker is enabled by default anyway and 
nobody was ever disabling it manually.


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

https://reviews.llvm.org/D64274

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/virtualcall-plist.cpp
  clang/test/Analysis/virtualcall.cpp
  clang/test/Analysis/virtualcall.h

Index: clang/test/Analysis/virtualcall.h
===
--- clang/test/Analysis/virtualcall.h
+++ clang/test/Analysis/virtualcall.h
@@ -2,12 +2,7 @@
   class Z {
   public:
 Z() {
-  foo();
-#if !PUREONLY
-	// expected-warning-re@-2 ^}}Call to virtual function during construction}}
-	// expected-note-re@-3 ^}}This constructor of an object of type 'Z' has not returned when the virtual method was called}}
-	// expected-note-re@-4 ^}}Call to virtual function during construction}}	
-#endif
+  foo(); // impure-warning {{Call to virtual method 'Z::foo' during construction bypasses virtual dispatch}}
 }
 virtual int foo();
   };
Index: clang/test/Analysis/virtualcall.cpp
===
--- clang/test/Analysis/virtualcall.cpp
+++ clang/test/Analysis/virtualcall.cpp
@@ -1,9 +1,38 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-output=text -verify -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=impure %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=pure -std=c++11 %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:-analyzer-config \
+// RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=none %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=pure,impure -std=c++11 %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
+// RUN:-analyzer-config \
+// RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=pure %s
+
+
+// We expect no diagnostics when all checks are disabled.
+// none-no-diagnostics
 
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:PureOnly=true -DPUREONLY=1 -analyzer-output=text -verify -std=c++11 %s
 
 #include "virtualcall.h"
 
+void clang_analyzer_warnIfReached();
+
 class A {
 public:
   A();
@@ -13,54 +42,32 @@
   virtual int foo() = 0;
   virtual void bar() = 0;
   void f() {
-foo();
-	// expected-warning-re@-1 ^}}Call to pure virtual function during construction}}
-	// expected-note-re@-2 ^}}Call to pure virtual function during construction}}
+foo(); // pure-warning{{Call to pure virtual method 'A::foo' during construction has undefined behavior}}
+clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
-class B : public A {
+A::A() {
+  f();
+}
+
+class B {
 public:
-  B() { // expected-note {{Calling default constructor for 'A'}}
-foo(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 ^}}Call to virtual function during construction}}
-	// expected-note-re@-3 ^}}This constructor of an object of type 'B' has not returned when the virtual method was called}}
-  	// expected-note-re@-4 ^}}Call to virtual function during construction}}
-#endif
+  B() {
+foo(); // impure-warning {{Call to virtual method 'B::foo' during construction bypasses virtual dispatch}}
   }
   ~B();
 
   virtual int foo();
   

Re: r368739 - Don't use std::errc

[Alexey Bataev via cfe-commits]

Sorry, forgot to change the description. Been busy switching to the newer 
version of the compiler.

Best regards,
Alexey Bataev

13 авг. 2019 г., в 21:32, Nico Weber 
mailto:tha...@chromium.org>> написал(а):

From the review: This is for gcc 4.8 compat, not for the other reasons 
mentioned in the CL description.

On Tue, Aug 13, 2019 at 3:31 PM Alexey Bataev via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: abataev
Date: Tue Aug 13 12:32:36 2019
New Revision: 368739

URL: http://llvm.org/viewvc/llvm-project?rev=368739=rev
Log:
Don't use std::errc

Summary:
As noted on Errc.h:

// * std::errc is just marked with is_error_condition_enum. This means that
//   common patters like AnErrorCode == errc::no_such_file_or_directory take
//   4 virtual calls instead of two comparisons.

And on some libstdc++ those virtual functions conclude that


int main() {
  std::error_code foo = 
std::make_error_code(std::errc::no_such_file_or_directory);
  return foo == std::errc::no_such_file_or_directory;
}
-

should exit with 0.

Reviewers: thakis, rnk, jfb

Reviewed By: thakis

Subscribers: lebedev.ri, dexonsmith, xbolva00, cfe-commits, caomhin

Tags: #clang

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

Modified:
cfe/trunk/lib/Lex/HeaderSearch.cpp

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=368739=368738=368739=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Tue Aug 13 12:32:36 2019
@@ -30,6 +30,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Capacity.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -315,9 +316,9 @@ const FileEntry *HeaderSearch::getFileAn
 // For rare, surprising errors (e.g. "out of file handles"), diag the EC
 // message.
 std::error_code EC = File.getError();
-if (EC != std::errc::no_such_file_or_directory &&
-EC != std::errc::invalid_argument && EC != std::errc::is_a_directory &&
-EC != std::errc::not_a_directory) {
+if (EC != llvm::errc::no_such_file_or_directory &&
+EC != llvm::errc::invalid_argument &&
+EC != llvm::errc::is_a_directory && EC != llvm::errc::not_a_directory) 
{
   Diags.Report(IncludeLoc, diag::err_cannot_open_file)
   << FileName << EC.message();
 }


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

[PATCH] D66143: Don't use std::errc

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

FWIW it looks like 4.8 support is going away in days, not weeks, see D66188 
.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66143



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

[PATCH] D66045: Improve detection of same value in comparisons

[Richard Trieu via Phabricator via cfe-commits]

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



Comment at: test/SemaCXX/self-comparison.cpp:87
+};
+} // namespace member_tests

jfb wrote:
> rtrieu wrote:
> > rtrieu wrote:
> > > jfb wrote:
> > > > The test only has `==`. Do other operators trigger?
> > > All the standard comparison operators will work here.  I'll add tests.
> > All operator tests added.
> `operator<=>`?
> 
> Is there a separate test for user-defined operators as well? What makes sense 
> in these cases? Technically a user-defined comparison could do anything... 
> but I think this warning makes sense for them as well.
operator<=> test was added to test/SemaCXX/compare-cxx2a.cpp because the 
operator requires header that this test doesn't include.

The warning currently doesn't check user-defined operators and I've include a 
test below to show that.  If a warning for user-defined operators was created, 
it would not belong in -Wtautological-compare because we would not know the 
actual result.  The best we could do is to say there was a self compare with a 
user-defined operator.  It would also need to hook into the AST at a different 
point.  Builtin compare operators are checked in BinaryOperator nodes.  
User-defined operators would need to be checked at CallExpr or 
CXXOperatorCallExpr instead.  This is different enough that it should be in a 
different patch.


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

https://reviews.llvm.org/D66045



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

Re: r368739 - Don't use std::errc

[Nico Weber via cfe-commits]

>From the review: This is for gcc 4.8 compat, not for the other reasons
mentioned in the CL description.

On Tue, Aug 13, 2019 at 3:31 PM Alexey Bataev via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: abataev
> Date: Tue Aug 13 12:32:36 2019
> New Revision: 368739
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368739=rev
> Log:
> Don't use std::errc
>
> Summary:
> As noted on Errc.h:
>
> // * std::errc is just marked with is_error_condition_enum. This means that
> //   common patters like AnErrorCode == errc::no_such_file_or_directory
> take
> //   4 virtual calls instead of two comparisons.
>
> And on some libstdc++ those virtual functions conclude that
>
> 
> int main() {
>   std::error_code foo =
> std::make_error_code(std::errc::no_such_file_or_directory);
>   return foo == std::errc::no_such_file_or_directory;
> }
> -
>
> should exit with 0.
>
> Reviewers: thakis, rnk, jfb
>
> Reviewed By: thakis
>
> Subscribers: lebedev.ri, dexonsmith, xbolva00, cfe-commits, caomhin
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D66143
>
> Modified:
> cfe/trunk/lib/Lex/HeaderSearch.cpp
>
> Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=368739=368738=368739=diff
>
> ==
> --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
> +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Tue Aug 13 12:32:36 2019
> @@ -30,6 +30,7 @@
>  #include "llvm/ADT/StringRef.h"
>  #include "llvm/Support/Allocator.h"
>  #include "llvm/Support/Capacity.h"
> +#include "llvm/Support/Errc.h"
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/FileSystem.h"
>  #include "llvm/Support/Path.h"
> @@ -315,9 +316,9 @@ const FileEntry *HeaderSearch::getFileAn
>  // For rare, surprising errors (e.g. "out of file handles"), diag the
> EC
>  // message.
>  std::error_code EC = File.getError();
> -if (EC != std::errc::no_such_file_or_directory &&
> -EC != std::errc::invalid_argument && EC !=
> std::errc::is_a_directory &&
> -EC != std::errc::not_a_directory) {
> +if (EC != llvm::errc::no_such_file_or_directory &&
> +EC != llvm::errc::invalid_argument &&
> +EC != llvm::errc::is_a_directory && EC !=
> llvm::errc::not_a_directory) {
>Diags.Report(IncludeLoc, diag::err_cannot_open_file)
><< FileName << EC.message();
>  }
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66045: Improve detection of same value in comparisons

[Richard Trieu via Phabricator via cfe-commits]

rtrieu updated this revision to Diff 215002.

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

https://reviews.llvm.org/D66045

Files:
  include/clang/AST/Expr.h
  lib/AST/Expr.cpp
  lib/Analysis/CFG.cpp
  lib/Sema/SemaExpr.cpp
  test/Analysis/array-struct-region.cpp
  test/Sema/warn-overlap.c
  test/SemaCXX/compare-cxx2a.cpp
  test/SemaCXX/self-comparison.cpp

Index: test/SemaCXX/self-comparison.cpp
===
--- test/SemaCXX/self-comparison.cpp
+++ test/SemaCXX/self-comparison.cpp
@@ -40,3 +40,84 @@
 Y::n == Y::n;
 }
 template bool g(); // should not produce any warnings
+
+namespace member_tests {
+struct B {
+  int field;
+  static int static_field;
+  int test(B b) {
+return field == field;  // expected-warning {{self-comparison always evaluates to true}}
+return static_field == static_field;  // expected-warning {{self-comparison always evaluates to true}}
+return static_field == b.static_field;  // expected-warning {{self-comparison always evaluates to true}}
+return B::static_field == this->static_field;  // expected-warning {{self-comparison always evaluates to true}}
+return this == this;  // expected-warning {{self-comparison always evaluates to true}}
+
+return field == b.field;
+return this->field == b.field;
+  }
+};
+
+enum {
+  I0,
+  I1,
+  I2,
+};
+
+struct S {
+  int field;
+  static int static_field;
+  int array[4];
+};
+
+struct T {
+  int field;
+  static int static_field;
+  int array[4];
+  S s;
+};
+
+int struct_test(S s1, S s2, S *s3, T t) {
+  return s1.field == s1.field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s2.field == s2.field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.static_field == s2.static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return S::static_field == s1.static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array == s1.array;  // expected-warning {{self-comparison always evaluates to true}}
+  return t.s.static_field == S::static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s3->field == s3->field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s3->static_field == S::static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array[0] == s1.array[0];  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array[I1] == s1.array[I1];  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array[s2.array[0]] == s1.array[s2.array[0]];  // expected-warning {{self-comparison always evaluates to true}}
+  return s3->array[t.field] == s3->array[t.field];  // expected-warning {{self-comparison always evaluates to true}}
+
+  // Try all operators
+  return t.field == t.field;  // expected-warning {{self-comparison always evaluates to true}}
+  return t.field <= t.field;  // expected-warning {{self-comparison always evaluates to true}}
+  return t.field >= t.field;  // expected-warning {{self-comparison always evaluates to true}}
+
+  return t.field != t.field;  // expected-warning {{self-comparison always evaluates to false}}
+  return t.field < t.field;  // expected-warning {{self-comparison always evaluates to false}}
+  return t.field > t.field;  // expected-warning {{self-comparison always evaluates to false}}
+
+  // no warning
+  return s1.field == s2.field;
+  return s2.array == s1.array;
+  return s2.array[0] == s1.array[0];
+  return s1.array[I1] == s1.array[I2];
+
+  return s1.static_field == t.static_field;
+};
+
+struct U {
+  bool operator!=(const U&);
+};
+
+bool operator==(const U&, const U&);
+
+// May want to warn on this in the future.
+int user_defined(U u) {
+  return u == u;
+  return u != u;
+}
+
+} // namespace member_tests
Index: test/SemaCXX/compare-cxx2a.cpp
===
--- test/SemaCXX/compare-cxx2a.cpp
+++ test/SemaCXX/compare-cxx2a.cpp
@@ -8,12 +8,18 @@
 #define ASSERT_TYPE(...) static_assert(__is_same(__VA_ARGS__))
 #define ASSERT_EXPR_TYPE(Expr, Expect) static_assert(__is_same(decltype(Expr), Expect));
 
+struct S {
+  static int x[5];
+};
+
 void self_compare() {
   int a;
   int *b = nullptr;
+  S s;
 
   (void)(a <=> a); // expected-warning {{self-comparison always evaluates to 'std::strong_ordering::equal'}}
   (void)(b <=> b); // expected-warning {{self-comparison always evaluates to 'std::strong_ordering::equal'}}
+  (void)(s.x[a] <=> S::x[a]); // expected-warning {{self-comparison always evaluates to 'std::strong_ordering::equal'}}
 }
 
 void test0(long a, unsigned long b) {
Index: test/Sema/warn-overlap.c
===
--- test/Sema/warn-overlap.c
+++ test/Sema/warn-overlap.c
@@ -141,3 +141,17 @@
   return x < 1 || x != 0;
   // 

[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 215000.
NoQ added a comment.

I'll merge D65180  into this patch because 
it's otherwise too painful to update. Like, D65180 
 untangles bug types and controls enabledness 
by initializing bug types, but in order to untangle the checkers into a pair 
with a common dependency, i already need such control, and i don't want to 
implement a temporary solution only to discard it immediately.

This update only merges the two patches; for ease of review it doesn't address 
any comments.


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

https://reviews.llvm.org/D64274

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/virtualcall-plist.cpp
  clang/test/Analysis/virtualcall.cpp
  clang/test/Analysis/virtualcall.h

Index: clang/test/Analysis/virtualcall.h
===
--- clang/test/Analysis/virtualcall.h
+++ clang/test/Analysis/virtualcall.h
@@ -2,12 +2,7 @@
   class Z {
   public:
 Z() {
-  foo();
-#if !PUREONLY
-	// expected-warning-re@-2 ^}}Call to virtual function during construction}}
-	// expected-note-re@-3 ^}}This constructor of an object of type 'Z' has not returned when the virtual method was called}}
-	// expected-note-re@-4 ^}}Call to virtual function during construction}}	
-#endif
+  foo(); // impure-warning {{Call to virtual method 'Z::foo' during construction bypasses virtual dispatch}}
 }
 virtual int foo();
   };
Index: clang/test/Analysis/virtualcall.cpp
===
--- clang/test/Analysis/virtualcall.cpp
+++ clang/test/Analysis/virtualcall.cpp
@@ -1,9 +1,33 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-output=text -verify -std=c++11 %s
-
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:PureOnly=true -DPUREONLY=1 -analyzer-output=text -verify -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=expected,impure %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=expected -std=c++11 %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+// RUN:-analyzer-config \
+// RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=expected %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=expected,impure -std=c++11 %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.PureVirtualCall \
+// RUN:-analyzer-checker=optin.cplusplus.VirtualCall \
+// RUN:-analyzer-config \
+// RUN:optin.cplusplus.VirtualCall:PureOnly=true \
+// RUN:-analyzer-checker=debug.ExprInspection \
+// RUN:-std=c++11 -verify=expected %s
 
 #include "virtualcall.h"
 
+void clang_analyzer_warnIfReached();
+
 class A {
 public:
   A();
@@ -13,54 +37,32 @@
   virtual int foo() = 0;
   virtual void bar() = 0;
   void f() {
-foo();
-	// expected-warning-re@-1 ^}}Call to pure virtual function during construction}}
-	// expected-note-re@-2 ^}}Call to pure virtual function during construction}}
+foo(); // expected-warning{{Call to pure virtual method 'A::foo' during construction has undefined behavior}}
+clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
-class B : public A {
+A::A() {
+  f();
+}
+
+class B {
 public:
-  B() { // expected-note {{Calling default constructor for 'A'}}
-foo(); 
-#if !PUREONLY
-  	// expected-warning-re@-2 ^}}Call to virtual function during construction}}
-	// expected-note-re@-3 ^}}This constructor of an object of type 'B' has not returned when the virtual method was called}}
-  	// expected-note-re@-4 ^}}Call to virtual function during construction}}
-#endif
+  B() {
+foo(); // impure-warning {{Call to virtual method 'B::foo' during construction bypasses virtual 

[PATCH] D65180: [analyzer] VirtualCallChecker: Improve warning messages.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ abandoned this revision.
NoQ added a comment.

Merged into D64274 .


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

https://reviews.llvm.org/D65180



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

[PATCH] D66192: [analyzer] Don't delete TaintConfig copy constructor

[Alex Langford via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368779: [analyzer] Dont delete TaintConfig copy 
constructor (authored by xiaobai, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D66192?vs=214998=214999#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66192

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -71,9 +71,9 @@
 std::vector Sinks;
 
 TaintConfiguration() = default;
-TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(const TaintConfiguration &) = default;
 TaintConfiguration(TaintConfiguration &&) = default;
-TaintConfiguration =(const TaintConfiguration &) = delete;
+TaintConfiguration =(const TaintConfiguration &) = default;
 TaintConfiguration =(TaintConfiguration &&) = default;
   };
 


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -71,9 +71,9 @@
 std::vector Sinks;
 
 TaintConfiguration() = default;
-TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(const TaintConfiguration &) = default;
 TaintConfiguration(TaintConfiguration &&) = default;
-TaintConfiguration =(const TaintConfiguration &) = delete;
+TaintConfiguration =(const TaintConfiguration &) = default;
 TaintConfiguration =(TaintConfiguration &&) = default;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66192: [analyzer] Don't delete TaintConfig copy constructor

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

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

This really shouldn't matter much. Go for it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66192



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

r368778 - [ORC] Fix clang-interpreter example code broken by r368707.

[Douglas Yung via cfe-commits]

Author: dyung
Date: Tue Aug 13 18:03:35 2019
New Revision: 368778

URL: http://llvm.org/viewvc/llvm-project?rev=368778=rev
Log:
[ORC] Fix clang-interpreter example code broken by r368707.

Modified:
cfe/trunk/examples/clang-interpreter/main.cpp

Modified: cfe/trunk/examples/clang-interpreter/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/clang-interpreter/main.cpp?rev=368778=368777=368778=diff
==
--- cfe/trunk/examples/clang-interpreter/main.cpp (original)
+++ cfe/trunk/examples/clang-interpreter/main.cpp Tue Aug 13 18:03:35 2019
@@ -61,11 +61,12 @@ private:
 return llvm::make_unique();
   }
 
-  SimpleJIT(std::unique_ptr TM, DataLayout DL,
-DynamicLibrarySearchGenerator ProcessSymbolsGenerator)
+  SimpleJIT(
+  std::unique_ptr TM, DataLayout DL,
+  std::unique_ptr ProcessSymbolsGenerator)
   : TM(std::move(TM)), DL(std::move(DL)) {
 llvm::sys::DynamicLibrary::LoadLibraryPermanently(nullptr);
-ES.getMainJITDylib().setGenerator(std::move(ProcessSymbolsGenerator));
+ES.getMainJITDylib().addGenerator(std::move(ProcessSymbolsGenerator));
   }
 
 public:


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

[PATCH] D66192: [analyzer] Don't delete TaintConfig copy constructor

[Alex Langford via Phabricator via cfe-commits]

xiaobai created this revision.
xiaobai added reviewers: compnerd, Szelethus, boga95, NoQ, alexshap.
Herald added subscribers: Charusso, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

Explicitly deleting the copy constructor makes compiling the function 
`ento::registerGenericTaintChecker` difficult with some compilers. When we
construct an `llvm::Optional`, the optional is constructed with a
const TaintConfig reference which it then uses to invoke the deleted TaintConfig
copy constructor.

I've observered this failing with clang 3.8 on Ubuntu 16.04.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66192

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -71,9 +71,9 @@
 std::vector Sinks;
 
 TaintConfiguration() = default;
-TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(const TaintConfiguration &) = default;
 TaintConfiguration(TaintConfiguration &&) = default;
-TaintConfiguration =(const TaintConfiguration &) = delete;
+TaintConfiguration =(const TaintConfiguration &) = default;
 TaintConfiguration =(TaintConfiguration &&) = default;
   };
 


Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -71,9 +71,9 @@
 std::vector Sinks;
 
 TaintConfiguration() = default;
-TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(const TaintConfiguration &) = default;
 TaintConfiguration(TaintConfiguration &&) = default;
-TaintConfiguration =(const TaintConfiguration &) = delete;
+TaintConfiguration =(const TaintConfiguration &) = default;
 TaintConfiguration =(TaintConfiguration &&) = default;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

[Artem Dergachev via Phabricator via cfe-commits]

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

Wish granted!


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

https://reviews.llvm.org/D65287



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

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368777: [analyzer][NFC] Prepare visitors for different 
tracking kinds (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64270?vs=211752=214995#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64270

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -85,6 +85,40 @@
   const BugReport );
 };
 
+namespace bugreporter {
+
+/// Specifies the type of tracking for an expression.
+enum class TrackingKind {
+  /// Default tracking kind -- specifies that as much information should be
+  /// gathered about the tracked expression value as possible.
+  Thorough,
+  /// Specifies that a more moderate tracking should be used for the expression
+  /// value. This will essentially make sure that functions relevant to the it
+  /// aren't pruned, but otherwise relies on the user reading the code or
+  /// following the arrows.
+  Condition
+};
+
+/// Attempts to add visitors to track expression value back to its point of
+/// origin.
+///
+/// \param N A node "downstream" from the evaluation of the statement.
+/// \param E The expression value which we are tracking
+/// \param R The bug report to which visitors should be attached.
+/// \param EnableNullFPSuppression Whether we should employ false positive
+/// suppression (inlined defensive checks, returned null).
+///
+/// \return Whether or not the function was able to add visitors for this
+/// statement. Note that returning \c true does not actually imply
+/// that any visitors were added.
+bool trackExpressionValue(const ExplodedNode *N, const Expr *E, BugReport ,
+  TrackingKind TKind = TrackingKind::Thorough,
+  bool EnableNullFPSuppression = true);
+
+const Expr *getDerefExpr(const Stmt *S);
+
+} // namespace bugreporter
+
 /// Finds last store into the given region,
 /// which is different from a given symbolic value.
 class FindLastStoreBRVisitor final : public BugReporterVisitor {
@@ -96,15 +130,28 @@
   /// bug, we are going to employ false positive suppression.
   bool EnableNullFPSuppression;
 
+  using TrackingKind = bugreporter::TrackingKind;
+  TrackingKind TKind;
+
 public:
   /// Creates a visitor for every VarDecl inside a Stmt and registers it with
   /// the BugReport.
   static void registerStatementVarDecls(BugReport , const Stmt *S,
-bool EnableNullFPSuppression);
+bool EnableNullFPSuppression,
+TrackingKind TKind);
 
+  /// \param V We're searching for the store where \c R received this value.
+  /// \param R The region we're tracking.
+  /// \param EnableNullFPSuppression Whether we should employ false positive
+  /// suppression (inlined defensive checks, returned null).
+  /// \param TKind May limit the amount of notes added to the bug report.
   FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R,
- bool InEnableNullFPSuppression)
-  : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression) {}
+ bool InEnableNullFPSuppression,
+ TrackingKind TKind)
+  : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression),
+TKind(TKind) {
+assert(R);
+  }
 
   void Profile(llvm::FoldingSetNodeID ) const override;
 
@@ -347,27 +394,6 @@
BugReport ) override;
 };
 
-namespace bugreporter {
-
-/// Attempts to add visitors to track expression value back to its point of
-/// origin.
-///
-/// \param N A node "downstream" from the evaluation of the statement.
-/// \param E The expression value which we are tracking
-/// \param R The bug report to which visitors should be attached.
-/// \param EnableNullFPSuppression Whether we should employ false positive
-/// suppression (inlined defensive checks, returned null).
-///
-/// \return Whether or not the function was able to add visitors for this
-/// statement. Note that returning \c true does not actually imply
-/// that any 

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

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

Szelethus added a comment.

If you have no major objections, could you please accept formally? :)

And I'll promise to draw CFG graphs all day tomorrow as an exercise after 
reading the order of operator evaluation rules >.<


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

https://reviews.llvm.org/D65287



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

r368777 - [analyzer][NFC] Prepare visitors for different tracking kinds

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Aug 13 17:48:57 2019
New Revision: 368777

URL: http://llvm.org/viewvc/llvm-project?rev=368777=rev
Log:
[analyzer][NFC] Prepare visitors for different tracking kinds

When we're tracking a variable that is responsible for a null pointer
dereference or some other sinister programming error, we of course would like to
gather as much information why we think that the variable has that specific
value as possible. However, the newly introduced condition tracking shows that
tracking all values this thoroughly could easily cause an intolerable growth in
the bug report's length.

There are a variety of heuristics we discussed on the mailing list[1] to combat
this, all of them requiring to differentiate in between tracking a "regular
value" and a "condition".

This patch introduces the new `bugreporter::TrackingKind` enum, adds it to
several visitors as a non-optional argument, and moves some functions around to
make the code a little more coherent.

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-June/062613.html

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

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=368777=368776=368777=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
Tue Aug 13 17:48:57 2019
@@ -85,6 +85,40 @@ public:
   const BugReport );
 };
 
+namespace bugreporter {
+
+/// Specifies the type of tracking for an expression.
+enum class TrackingKind {
+  /// Default tracking kind -- specifies that as much information should be
+  /// gathered about the tracked expression value as possible.
+  Thorough,
+  /// Specifies that a more moderate tracking should be used for the expression
+  /// value. This will essentially make sure that functions relevant to the it
+  /// aren't pruned, but otherwise relies on the user reading the code or
+  /// following the arrows.
+  Condition
+};
+
+/// Attempts to add visitors to track expression value back to its point of
+/// origin.
+///
+/// \param N A node "downstream" from the evaluation of the statement.
+/// \param E The expression value which we are tracking
+/// \param R The bug report to which visitors should be attached.
+/// \param EnableNullFPSuppression Whether we should employ false positive
+/// suppression (inlined defensive checks, returned null).
+///
+/// \return Whether or not the function was able to add visitors for this
+/// statement. Note that returning \c true does not actually imply
+/// that any visitors were added.
+bool trackExpressionValue(const ExplodedNode *N, const Expr *E, BugReport ,
+  TrackingKind TKind = TrackingKind::Thorough,
+  bool EnableNullFPSuppression = true);
+
+const Expr *getDerefExpr(const Stmt *S);
+
+} // namespace bugreporter
+
 /// Finds last store into the given region,
 /// which is different from a given symbolic value.
 class FindLastStoreBRVisitor final : public BugReporterVisitor {
@@ -96,15 +130,28 @@ class FindLastStoreBRVisitor final : pub
   /// bug, we are going to employ false positive suppression.
   bool EnableNullFPSuppression;
 
+  using TrackingKind = bugreporter::TrackingKind;
+  TrackingKind TKind;
+
 public:
   /// Creates a visitor for every VarDecl inside a Stmt and registers it with
   /// the BugReport.
   static void registerStatementVarDecls(BugReport , const Stmt *S,
-bool EnableNullFPSuppression);
+bool EnableNullFPSuppression,
+TrackingKind TKind);
 
+  /// \param V We're searching for the store where \c R received this value.
+  /// \param R The region we're tracking.
+  /// \param EnableNullFPSuppression Whether we should employ false positive
+  /// suppression (inlined defensive checks, returned null).
+  /// \param TKind May limit the amount of notes added to the bug report.
   FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R,
- bool InEnableNullFPSuppression)
-  : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression) {}
+ bool InEnableNullFPSuppression,
+ TrackingKind TKind)
+  : R(R), 

[PATCH] D66131: [analyzer] Don't track the condition of foreach loops

[Artem Dergachev via Phabricator via cfe-commits]

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

Do it! 8)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66131



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

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1750
+  //   [B1] -> [B2] -> [B3] -> [sink]
+  // assert(A && B || C);\   \   \
+  //  ---> [go on with the execution]

Szelethus wrote:
> NoQ wrote:
> > xazax.hun wrote:
> > > baloghadamsoftware wrote:
> > > > xazax.hun wrote:
> > > > > I wonder if the CFG is correct for your example. If A evaluates to 
> > > > > false, we still check C before the sink. If A evaluates to true we 
> > > > > still check B before going on. But maybe it is just late for me :)
> > > > I think an arrow from `[B1]` to `[B3]` is missing for the `A == false` 
> > > > case.
> > > I am still not sure I like this picture. Looking at [B1] I had the 
> > > impression it has 3 successors which is clearly not the case. 
> > *confirms that 3 successors is 1 successor too many*
> Alright. I think I got it now (eventually).
There shouldn't be a direct edge from `[B1]` to the sink. If `A` is true, we 
proceed to evaluate `B`. If `A` is false, we proceed to evaluate `C`.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1753-1755
+  // It so happens that CFGBlock::getTerminatorCondition returns 'A' for block
+  // B1, 'A && B' for B2, and 'A && B || C' for B3. Let's check whether we
+  // reached the end of the condition!

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > Szelethus wrote:
> > > > > NoQ wrote:
> > > > > > Clever trick, but why not match for logical operators directly? 
> > > > > > Something like this:
> > > > > > ```lang=c++
> > > > > > if (auto B = dyn_cast(OuterCond))
> > > > > >   if (B->isLogicalOp())
> > > > > > return isAssertlikeBlock(Else, Context);
> > > > > > ```
> > > > > What about `assert(a + b && "Shouldn't be zero!");`?
> > > > Mmm, could you elaborate? >.<
> > > ```lang=c++
> > > if (auto B = dyn_cast(OuterCond))
> > >   if (B->isLogicalOp())
> > > return isAssertlikeBlock(Else, Context);
> > > ```
> > > We don't need `isLogicalOp()` here.
> > > 
> > > Also, I should probably have a testcase for the elvis operator (`?:`).
> > Mmm, i'm still confused. The `a + b` in your example doesn't appear as an 
> > else-block terminator anywhere. And i don't see why would `a + b` behave 
> > differently compared to a simple `a`, while i do see why would, say, `a && 
> > b` behave differently.
> Right. I take it all back. But I guess we might as well just `assert` that 
> it's a logical operator, if it has 2 successors.
Aha, great!


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

https://reviews.llvm.org/D65287



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

[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

[John McCall via Phabricator via cfe-commits]

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

One minor request (which will be dead code in your patch), but otherwise LGTM.




Comment at: clang/include/clang/Basic/IdentifierTable.h:754
+  /// If this selector is the specific keyword selector described by Names.
+  bool isKeywordSelector(ArrayRef Names);
+

Thanks.  Could you go ahead and add `isUnarySelector(StringRef)` for 
consistency?


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

https://reviews.llvm.org/D27165



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

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:26
   void addCheckFactories(ClangTidyCheckFactories ) override {
+using readability::NamespaceCommentCheck;
+

aaron.ballman wrote:
> I would rather use the fully-qualified names below -- the namespaces are 
> actually of interest when needing to see what checks rely on what other 
> modules quickly.
In that case I think I need some more detail on the alphabetical order I need 
to preserve. Do the namespaces factor into the order? If so, then 
PreferIsaOrDynCastInConditionalsCheck was in the wrong place prior to this 
patch and add_new_check.py is behaving correctly. If not, then the existing 
order was correct but add_new_check.py is inserting new lines incorrectly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65919



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

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

This revision was automatically updated to reflect the committed changes.
Szelethus marked 2 inline comments as done.
Closed by commit rG0df9c8c57802: [analyzer] Track the right hand side of the 
last store regardless of its value (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D64287?vs=210333=214982#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64287

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/track-control-dependency-conditions.cpp
  clang/test/Analysis/uninit-const.c
  clang/test/Analysis/uninit-const.cpp

Index: clang/test/Analysis/uninit-const.cpp
===
--- clang/test/Analysis/uninit-const.cpp
+++ clang/test/Analysis/uninit-const.cpp
@@ -66,8 +66,8 @@
 
 void f6_2(void) {
   int t;   //expected-note {{'t' declared without an initial value}}
-  int  = t;
-  int  = p;
+  int  = t;  //expected-note {{'p' initialized here}}
+  int  = p;  //expected-note {{'s' initialized here}}
   int  = s;  //expected-note {{'q' initialized here}}
   doStuff6(q); //expected-warning {{1st function call argument is an uninitialized value}}
//expected-note@-1 {{1st function call argument is an uninitialized value}}
@@ -96,7 +96,7 @@
 
 
 void f5(void) {
-  int t;
+  int t;   // expected-note {{'t' declared without an initial value}}
   int* tp = // expected-note {{'tp' initialized here}}
   doStuff_uninit(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
Index: clang/test/Analysis/uninit-const.c
===
--- clang/test/Analysis/uninit-const.c
+++ clang/test/Analysis/uninit-const.c
@@ -24,15 +24,15 @@
 void doStuff_variadic(const int *u, ...){};
 
 void f_1(void) {
-  int t;
+  int t;   // expected-note {{'t' declared without an initial value}}
   int* tp = // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }
 
 void f_1_1(void) {
-  int t;
-  int* tp1 = 
+  int t; // expected-note {{'t' declared without an initial value}}
+  int *tp1 =  // expected-note {{'tp1' initialized here}}
   int* tp2 = tp1;// expected-note {{'tp2' initialized here}}
   doStuff_pointerToConstInt(tp2);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -40,12 +40,15 @@
 
 
 int *f_2_sub(int *p) {
-  return p;
+  return p; // expected-note {{Returning pointer (loaded from 'p')}}
 }
 
 void f_2(void) {
-  int t;
-  int* p = f_2_sub();
+  int t;// expected-note {{'t' declared without an initial value}}
+  int *p = f_2_sub(); // expected-note {{Passing value via 1st parameter 'p'}}
+// expected-note@-1{{Calling 'f_2_sub'}}
+// expected-note@-2{{Returning from 'f_2_sub'}}
+// expected-note@-3{{'p' initialized here}}
   int* tp = p; // expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp); // expected-warning {{1st function call argument is a pointer to uninitialized value}}
   // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -62,7 +65,7 @@
 }
 
 void f_5(void) {
-  int ta[5];
+  int ta[5];   // expected-note {{'ta' initialized here}}
   int* tp = ta;// expected-note {{'tp' initialized here}}
   doStuff_pointerToConstInt(tp);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -99,7 +102,7 @@
 }
 
 void f_9(void) {
-  int  a[6];
+  int a[6];// expected-note {{'a' initialized here}}
   int const *ptau = a; // expected-note {{'ptau' initialized here}}
   doStuff_arrayOfConstInt(ptau);// expected-warning {{1st function call argument is a pointer to uninitialized value}}
// expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
@@ -173,7 +176,7 @@
 
 // uninit pointer, uninit val
 void f_variadic_unp_unv(void) {
-  int t;
+  int t; // expected-note {{'t' declared without an initial value}}
   int v;
   int* tp =// expected-note {{'tp' initialized here}}
   doStuff_variadic(tp,v);  // expected-warning {{1st function call argument 

r368773 - [analyzer] Track the right hand side of the last store regardless of its value

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Aug 13 16:48:10 2019
New Revision: 368773

URL: http://llvm.org/viewvc/llvm-project?rev=368773=rev
Log:
[analyzer] Track the right hand side of the last store regardless of its value

Summary:
The following code snippet taken from D64271#1572188 has an issue: namely,
because `flag`'s value isn't undef or a concrete int, it isn't being tracked.

int flag;
bool coin();

void foo() {
  flag = coin();
}

void test() {
  int *x = 0;
  int local_flag;
  flag = 1;

  foo();
  local_flag = flag;
  if (local_flag)
x = new int;

  foo();
  local_flag = flag;
  if (local_flag)
*x = 5;
}

This, in my opinion, makes no sense, other values may be interesting too.
Originally added by rC185608.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
cfe/trunk/test/Analysis/uninit-const.c
cfe/trunk/test/Analysis/uninit-const.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=368773=368772=368773=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Tue Aug 13 
16:48:10 2019
@@ -1394,15 +1394,11 @@ FindLastStoreBRVisitor::VisitNode(const
   // If we have an expression that provided the value, try to track where it
   // came from.
   if (InitE) {
-if (V.isUndef() ||
-V.getAs() || V.getAs()) {
-  if (!IsParam)
-InitE = InitE->IgnoreParenCasts();
-  bugreporter::trackExpressionValue(StoreSite, InitE, BR,
-   EnableNullFPSuppression);
-}
-ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
- BR, EnableNullFPSuppression);
+if (!IsParam)
+  InitE = InitE->IgnoreParenCasts();
+
+bugreporter::trackExpressionValue(StoreSite, InitE, BR,
+  EnableNullFPSuppression);
   }
 
   // Okay, we've found the binding. Emit an appropriate message.
@@ -1945,19 +1941,20 @@ bool bugreporter::trackExpressionValue(c
 // We should not try to dereference pointers at all when we don't care
 // what is written inside the pointer.
 bool CanDereference = true;
-if (const auto *SR = dyn_cast(L->getRegion()))
+if (const auto *SR = L->getRegionAs()) {
   if (SR->getSymbol()->getType()->getPointeeType()->isVoidType())
 CanDereference = false;
+} else if (const auto *AR = L->getRegionAs())
+  CanDereference = false;
 
 // At this point we are dealing with the region's LValue.
 // However, if the rvalue is a symbolic region, we should track it as well.
 // Try to use the correct type when looking up the value.
 SVal RVal;
-if (ExplodedGraph::isInterestingLValueExpr(Inner)) {
+if (ExplodedGraph::isInterestingLValueExpr(Inner))
   RVal = LVState->getRawSVal(L.getValue(), Inner->getType());
-} else if (CanDereference) {
+else if (CanDereference)
   RVal = LVState->getSVal(L->getRegion());
-}
 
 if (CanDereference)
   if (auto KV = RVal.getAs())

Modified: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp?rev=368773=368772=368773=diff
==
--- cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp (original)
+++ cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp Tue Aug 13 
16:48:10 2019
@@ -366,6 +366,30 @@ void f(int y) {
 }
 } // end of namespace tracked_condition_written_in_nested_stackframe
 
+namespace condition_written_in_nested_stackframe_before_assignment {
+int flag = 0;
+int getInt();
+
+void foo() {
+  flag = getInt(); // tracking-note{{Value assigned to 'flag'}}
+}
+
+void f() {
+  int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+  int y = 0;
+
+  foo();// tracking-note{{Calling 'foo'}}
+// tracking-note@-1{{Returning from 'foo'}}
+  y = flag; // tracking-note{{Value assigned to 'y'}}
+
+  if (y)// expected-note{{Assuming 'y' is not equal to 0}}
+// expected-note@-1{{Taking true branch}}
+// debug-note@-2{{Tracking condition 'y'}}
+*x = 5; // expected-warning{{Dereference of null pointer}}
+// expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace condition_written_in_nested_stackframe_before_assignment
+
 namespace collapse_point_not_in_condition {
 
 [[noreturn]] void halt();

Modified: cfe/trunk/test/Analysis/uninit-const.c
URL: 

[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 214977.
erik.pilkington added a comment.

Address review comments.


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

https://reviews.llvm.org/D27165

Files:
  clang/include/clang/AST/FormatString.h
  clang/include/clang/Basic/IdentifierTable.h
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaObjC/format-strings-objc.m

Index: clang/test/SemaObjC/format-strings-objc.m
===
--- clang/test/SemaObjC/format-strings-objc.m
+++ clang/test/SemaObjC/format-strings-objc.m
@@ -25,7 +25,11 @@
 @protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end
 @interface NSObject  {} @end
 typedef float CGFloat;
-@interface NSString : NSObject - (NSUInteger)length; @end
+@interface NSString : NSObject 
+- (NSUInteger)length;
++(instancetype)stringWithFormat:(NSString *)fmt, ...
+__attribute__((format(__NSString__, 1, 2)));
+@end
 @interface NSSimpleCString : NSString {} @end
 @interface NSConstantString : NSSimpleCString @end
 extern void *_NSConstantStringClassReference;
@@ -302,3 +306,39 @@
 }
 
 @end
+
+@interface NSBundle : NSObject
+- (NSString *)localizedStringForKey:(NSString *)key
+  value:(nullable NSString *)value
+  table:(nullable NSString *)tableName
+ __attribute__((format_arg(1)));
+
+- (NSString *)someRandomMethod:(NSString *)key
+ value:(nullable NSString *)value
+ table:(nullable NSString *)tableName
+__attribute__((format_arg(1)));
+@end
+
+void useLocalizedStringForKey(NSBundle *bndl) {
+  [NSString stringWithFormat:
+  [bndl localizedStringForKey:@"%d" // expected-warning{{more '%' conversions than data arguments}}
+  value:0
+  table:0]];
+  // No warning, @"flerp" doesn't have a format specifier.
+  [NSString stringWithFormat: [bndl localizedStringForKey:@"flerp" value:0 table:0], 43, @"flarp"];
+
+  [NSString stringWithFormat:
+  [bndl localizedStringForKey:@"%f"
+value:0
+table:0], 42]; // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+
+  [NSString stringWithFormat:
+  [bndl someRandomMethod:@"%f"
+   value:0
+   table:0], 42]; // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+
+  [NSString stringWithFormat:
+  [bndl someRandomMethod:@"flerp"
+   value:0
+   table:0], 42]; // expected-warning{{data argument not used by format string}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6575,7 +6575,8 @@
   bool inFunctionCall,
   Sema::VariadicCallType CallType,
   llvm::SmallBitVector ,
-  UncoveredArgHandler );
+  UncoveredArgHandler ,
+  bool IgnoreStringsWithoutSpecifiers);
 
 // Determine if an expression is a string literal or constant string.
 // If this function returns false on the arguments to a function expecting a
@@ -6588,7 +6589,8 @@
   Sema::VariadicCallType CallType, bool InFunctionCall,
   llvm::SmallBitVector ,
   UncoveredArgHandler ,
-  llvm::APSInt Offset) {
+  llvm::APSInt Offset,
+  bool IgnoreStringsWithoutSpecifiers = false) {
   if (S.isConstantEvaluated())
 return SLCT_NotALiteral;
  tryAgain:
@@ -6639,17 +6641,17 @@
   Left = checkFormatStringExpr(S, C->getTrueExpr(), Args,
HasVAListArg, format_idx, firstDataArg,
Type, CallType, InFunctionCall,
-   CheckedVarArgs, UncoveredArg, Offset);
+   CheckedVarArgs, UncoveredArg, Offset,
+   IgnoreStringsWithoutSpecifiers);
   if (Left == SLCT_NotALiteral || !CheckRight) {
 return Left;
   }
 }
 
-StringLiteralCheckType Right =
-checkFormatStringExpr(S, C->getFalseExpr(), Args,
-  HasVAListArg, format_idx, firstDataArg,
-  Type, CallType, InFunctionCall, CheckedVarArgs,
-  UncoveredArg, Offset);
+StringLiteralCheckType Right = checkFormatStringExpr(
+S, C->getFalseExpr(), Args, HasVAListArg, format_idx, firstDataArg,
+   

[PATCH] D66176: Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

Taking a step back and thinking about this, the compile command uses "-S", but 
explicitly specifies the output file to be "%t/module.pcm.o" (even though it is 
generating an assembly file instead of an object file), could we change the 
test to just check for "-o {{.*}}module.pcm.o "? That would work in all cases I 
think except for the extreme corner case where that string was part of the 
build directory name.




Comment at: clang/test/Driver/modules.cpp:19
+// CHECK-COMPILE-SAME: {{ -o }}
+// CHECK-COMPILE-SAME: module{{2*}}.{{pcm.o|s}}
 // CHECK-COMPILE-SAME: -x pcm

I'm not sure what the regex module{{2*}} is supposed to match, or prevent from 
matching? Was this intentional?

It seems it would match "module", "module2", "module22", "module222", etc. When 
would the compiler ever generate anything other than the first? Unless you are 
trying to protect yourself against a build directory that contains module in 
the name, but I'm not sure how this helps that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66176



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

[PATCH] D65349: [analyzer] Be more careful with destructors of non-regions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 214972.
NoQ marked 2 inline comments as done.
NoQ added a comment.

Fxd!


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

https://reviews.llvm.org/D65349

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/dtor.cpp

Index: clang/test/Analysis/dtor.cpp
===
--- clang/test/Analysis/dtor.cpp
+++ clang/test/Analysis/dtor.cpp
@@ -540,3 +540,33 @@
   clang_analyzer_eval(__alignof(NonTrivial) > 0); // expected-warning{{TRUE}}
 }
 }
+
+namespace dtor_over_loc_concrete_int {
+struct A {
+  ~A() {}
+};
+
+struct B {
+  A a;
+  ~B() {}
+};
+
+struct C : A {
+  ~C() {}
+};
+
+void testB() {
+  B *b = (B *)-1;
+  b->~B(); // no-crash
+}
+
+void testC() {
+  C *c = (C *)-1;
+  c->~C(); // no-crash
+}
+
+void testAutoDtor() {
+  const A  = *(A *)-1;
+  // no-crash
+}
+} // namespace dtor_over_loc_concrete_int
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -604,7 +604,7 @@
 bool IsBaseDtor,
 ExplodedNode *Pred,
 ExplodedNodeSet ,
-const EvalCallOptions ) {
+EvalCallOptions ) {
   assert(S && "A destructor without a trigger!");
   const LocationContext *LCtx = Pred->getLocationContext();
   ProgramStateRef State = Pred->getState();
@@ -612,7 +612,6 @@
   const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl();
   assert(RecordDecl && "Only CXXRecordDecls should have destructors");
   const CXXDestructorDecl *DtorDecl = RecordDecl->getDestructor();
-
   // FIXME: There should always be a Decl, otherwise the destructor call
   // shouldn't have been added to the CFG in the first place.
   if (!DtorDecl) {
@@ -626,9 +625,27 @@
 return;
   }
 
+  if (!Dest) {
+// We're trying to destroy something that is not a region. This may happen
+// for a variety of reasons (unknown target region, concrete integer instead
+// of target region, etc.). The current code makes an attempt to recover.
+// FIXME: We probably don't really need to recover when we're dealing
+// with concrete integers specifically.
+CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+if (const Expr *E = dyn_cast_or_null(S)) {
+  Dest = MRMgr.getCXXTempObjectRegion(E, Pred->getLocationContext());
+} else {
+  static SimpleProgramPointTag T("ExprEngine", "SkipInvalidDestructor");
+  NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+  Bldr.generateSink(Pred->getLocation().withTag(),
+Pred->getState(), Pred);
+  return;
+}
+  }
+
   CallEventManager  = getStateManager().getCallEventManager();
   CallEventRef Call =
-CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx);
+  CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx);
 
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
 Call->getSourceRange().getBegin(),
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -977,8 +977,8 @@
   Region = makeZeroElementRegion(state, loc::MemRegionVal(Region), varType,
  CallOpts.IsArrayCtorOrDtor).getAsRegion();
 
-  VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), /*IsBase=*/ false,
- Pred, Dst, CallOpts);
+  VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(),
+ /*IsBase=*/false, Pred, Dst, CallOpts);
 }
 
 void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
@@ -1036,8 +1036,9 @@
   SVal BaseVal = getStoreManager().evalDerivedToBase(ThisVal, BaseTy,
  Base->isVirtual());
 
-  VisitCXXDestructor(BaseTy, BaseVal.castAs().getRegion(),
- CurDtor->getBody(), /*IsBase=*/ true, Pred, Dst, {});
+  EvalCallOptions CallOpts;
+  VisitCXXDestructor(BaseTy, BaseVal.getAsRegion(), CurDtor->getBody(),
+ /*IsBase=*/true, Pred, Dst, CallOpts);
 }
 
 void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
@@ -1048,10 +1049,10 @@
   const LocationContext *LCtx = Pred->getLocationContext();
 
   const auto *CurDtor = cast(LCtx->getDecl());
-  Loc ThisVal = getSValBuilder().getCXXThis(CurDtor,
-LCtx->getStackFrame());
-  SVal FieldVal =
-  State->getLValue(Member, State->getSVal(ThisVal).castAs());
+ 

[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

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

This revision was automatically updated to reflect the committed changes.
Szelethus marked 5 inline comments as done.
Closed by commit rL368771: [analyzer] Prune calls to functions with linear CFGs 
that return a non-zero… (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64232?vs=212801=214969#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64232

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  cfe/trunk/test/Analysis/diagnostics/find_last_store.c
  cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
  cfe/trunk/test/Analysis/uninit-vals.c

Index: cfe/trunk/test/Analysis/diagnostics/find_last_store.c
===
--- cfe/trunk/test/Analysis/diagnostics/find_last_store.c
+++ cfe/trunk/test/Analysis/diagnostics/find_last_store.c
@@ -2,13 +2,11 @@
 typedef struct { float b; } c;
 void *a();
 void *d() {
-  return a(); // expected-note{{Returning pointer}}
+  return a();
 }
 
 void no_find_last_store() {
-  c *e = d(); // expected-note{{Calling 'd'}}
-  // expected-note@-1{{Returning from 'd'}}
-  // expected-note@-2{{'e' initialized here}}
+  c *e = d(); // expected-note{{'e' initialized here}}
 
   (void)(e || e->b); // expected-note{{Assuming 'e' is null}}
   // expected-note@-1{{Left side of '||' is false}}
Index: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
===
--- cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
+++ cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
@@ -119,7 +119,7 @@
 bool coin();
 
 bool foo() {
-  return coin(); // tracking-note{{Returning value}}
+  return coin();
 }
 
 int bar;
@@ -127,12 +127,10 @@
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
-  if (int flag = foo()) // tracking-note{{Calling 'foo'}}
-// tracking-note@-1{{Returning from 'foo'}}
-// tracking-note@-2{{'flag' initialized here}}
-// debug-note@-3{{Tracking condition 'flag'}}
-// expected-note@-4{{Assuming 'flag' is not equal to 0}}
-// expected-note@-5{{Taking true branch}}
+  if (int flag = foo()) // tracking-note{{'flag' initialized here}}
+// debug-note@-1{{Tracking condition 'flag'}}
+// expected-note@-2{{Assuming 'flag' is not equal to 0}}
+// expected-note@-3{{Taking true branch}}
 
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
@@ -143,39 +141,114 @@
 bool coin();
 
 struct ConvertsToBool {
-  operator bool() const { return coin(); } // tracking-note{{Returning value}}
+  operator bool() const { return coin(); }
 };
 
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
   if (ConvertsToBool())
-// tracking-note@-1 {{Calling 'ConvertsToBool::operator bool'}}
-// tracking-note@-2{{Returning from 'ConvertsToBool::operator bool'}}
-// debug-note@-3{{Tracking condition 'ConvertsToBool()'}}
-// expected-note@-4{{Assuming the condition is true}}
-// expected-note@-5{{Taking true branch}}
+// debug-note@-1{{Tracking condition 'ConvertsToBool()'}}
+// expected-note@-2{{Assuming the condition is true}}
+// expected-note@-3{{Taking true branch}}
 *x = 5; // expected-warning{{Dereference of null pointer}}
 // expected-note@-1{{Dereference of null pointer}}
 }
 
 } // end of namespace variable_declaration_in_condition
 
+namespace important_returning_pointer_loaded_from {
+bool coin();
+
+int *getIntPtr();
+
+void storeValue(int **i) {
+  *i = getIntPtr(); // tracking-note{{Value assigned to 'i'}}
+}
+
+int *conjurePointer() {
+  int *i;
+  storeValue(); // tracking-note{{Calling 'storeValue'}}
+  // tracking-note@-1{{Returning from 'storeValue'}}
+  return i;   // tracking-note{{Returning pointer (loaded from 'i')}}
+}
+
+void f(int *ptr) {
+  if (ptr) // expected-note{{Assuming 'ptr' is null}}
+   // expected-note@-1{{Taking false branch}}
+;
+  if (!conjurePointer())
+// tracking-note@-1{{Calling 'conjurePointer'}}
+// tracking-note@-2{{Returning from 'conjurePointer'}}
+// debug-note@-3{{Tracking condition '!conjurePointer()'}}
+// expected-note@-4{{Assuming the condition is true}}
+// expected-note@-5{{Taking true branch}}
+*ptr = 5; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1{{Dereference of null pointer}}
+}
+} // end of namespace important_returning_pointer_loaded_from
+
+namespace 

r368771 - [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Aug 13 16:22:33 2019
New Revision: 368771

URL: http://llvm.org/viewvc/llvm-project?rev=368771=rev
Log:
[analyzer] Prune calls to functions with linear CFGs that return a non-zero 
constrained value

During the evaluation of D62883, I noticed a bunch of totally
meaningless notes with the pattern of "Calling 'A'" -> "Returning value"
-> "Returning from 'A'", which added no value to the report at all.

This patch (not only affecting tracked conditions mind you) prunes
diagnostic messages to functions that return a value not constrained to
be 0, and are also linear.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/find_last_store.c
cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
cfe/trunk/test/Analysis/uninit-vals.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=368771=368770=368771=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Tue Aug 13 
16:22:33 2019
@@ -841,7 +841,7 @@ namespace {
 /// This visitor is intended to be used when another visitor discovers that an
 /// interesting value comes from an inlined function call.
 class ReturnVisitor : public BugReporterVisitor {
-  const StackFrameContext *StackFrame;
+  const StackFrameContext *CalleeSFC;
   enum {
 Initial,
 MaybeUnsuppress,
@@ -853,10 +853,9 @@ class ReturnVisitor : public BugReporter
   AnalyzerOptions& Options;
 
 public:
-  ReturnVisitor(const StackFrameContext *Frame,
-bool Suppressed,
+  ReturnVisitor(const StackFrameContext *Frame, bool Suppressed,
 AnalyzerOptions )
-  : StackFrame(Frame), EnableNullFPSuppression(Suppressed),
+  : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
 Options(Options) {}
 
   static void *getTag() {
@@ -866,7 +865,7 @@ public:
 
   void Profile(llvm::FoldingSetNodeID ) const override {
 ID.AddPointer(ReturnVisitor::getTag());
-ID.AddPointer(StackFrame);
+ID.AddPointer(CalleeSFC);
 ID.AddBoolean(EnableNullFPSuppression);
   }
 
@@ -950,7 +949,6 @@ public:
   if (Optional RetLoc = RetVal.getAs())
 EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
-BR.markInteresting(CalleeContext);
 BR.addVisitor(llvm::make_unique(CalleeContext,
EnableNullFPSuppression,
Options));
@@ -960,7 +958,7 @@ public:
   BugReporterContext ,
   BugReport ) {
 // Only print a message at the interesting return statement.
-if (N->getLocationContext() != StackFrame)
+if (N->getLocationContext() != CalleeSFC)
   return nullptr;
 
 Optional SP = N->getLocationAs();
@@ -974,7 +972,7 @@ public:
 // Okay, we're at the right return statement, but do we have the return
 // value available?
 ProgramStateRef State = N->getState();
-SVal V = State->getSVal(Ret, StackFrame);
+SVal V = State->getSVal(Ret, CalleeSFC);
 if (V.isUnknownOrUndef())
   return nullptr;
 
@@ -1008,6 +1006,8 @@ public:
 SmallString<64> Msg;
 llvm::raw_svector_ostream Out(Msg);
 
+bool WouldEventBeMeaningless = false;
+
 if (State->isNull(V).isConstrainedTrue()) {
   if (V.getAs()) {
 
@@ -1030,10 +1030,19 @@ public:
 } else {
   if (auto CI = V.getAs()) {
 Out << "Returning the value " << CI->getValue();
-  } else if (V.getAs()) {
-Out << "Returning pointer";
   } else {
-Out << "Returning value";
+// There is nothing interesting about returning a value, when it is
+// plain value without any constraints, and the function is guaranteed
+// to return that every time. We could use CFG::isLinear() here, but
+// constexpr branches are obvious to the compiler, not necesserily to
+// the programmer.
+if (N->getCFG().size() == 3)
+  WouldEventBeMeaningless = true;
+
+if (V.getAs())
+  Out << "Returning pointer";
+else
+  Out << "Returning value";
   }
 }
 
@@ -1052,11 +1061,20 @@ public:
   Out << " (loaded from '" << *DD << "')";
 }
 
-PathDiagnosticLocation L(Ret, BRC.getSourceManager(), StackFrame);
+PathDiagnosticLocation L(Ret, BRC.getSourceManager(), CalleeSFC);
 if (!L.isValid() || !L.asLocation().isValid())
   return nullptr;
 
-return std::make_shared(L, Out.str());
+auto EventPiece = std::make_shared(L, Out.str());
+
+// If we determined that the note is meaningless, make it 

[PATCH] D66186: Fix warning on printf('%hd', [char])

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 214966.
Nathan-Huckleberry added a comment.

- Add comment in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66186

Files:
  clang/lib/AST/FormatString.cpp
  clang/test/FixIt/format.m
  clang/test/Sema/format-strings-enum-fixed-type.cpp
  clang/test/Sema/format-strings.c


Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -277,8 +277,8 @@
 
 void should_understand_small_integers() {
   printf("%hhu", (short) 10); // expected-warning{{format specifies type 
'unsigned char' but the argument has type 'short'}}
-  printf("%hu\n", (unsigned char) 1); // expected-warning{{format specifies 
type 'unsigned short' but the argument has type 'unsigned char'}}
-  printf("%hu\n", (uint8_t)1); // expected-warning{{format specifies type 
'unsigned short' but the argument has type 'uint8_t'}}
+  printf("%hu\n", (unsigned char)1); // no-warning
+  printf("%hu\n", (uint8_t)1);   // no-warning
 }
 
 void test11(void *p, char *s) {
Index: clang/test/Sema/format-strings-enum-fixed-type.cpp
===
--- clang/test/Sema/format-strings-enum-fixed-type.cpp
+++ clang/test/Sema/format-strings-enum-fixed-type.cpp
@@ -80,9 +80,9 @@
   printf("%hhd", CharConstant); // no-warning
 
   // This is not correct but it is safe. We warn because '%hd' shows intent.
-  printf("%hd", input); // expected-warning{{format specifies type 'short' but 
the argument has underlying type 'char'}}
-  printf("%hd", CharConstant); // expected-warning{{format specifies type 
'short'}}
-  
+  printf("%hd", input);// no-warning
+  printf("%hd", CharConstant); // no-warning
+
   // This is not correct but it matches the promotion rules (and is safe).
   printf("%d", input); // no-warning
   printf("%d", CharConstant); // no-warning
Index: clang/test/FixIt/format.m
===
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -205,9 +205,7 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%f"
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
 
-  NSLog(@"%C", (char)0x260300); // expected-warning{{format specifies type 
'unichar' (aka 'unsigned short') but the argument has type 'char'}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:22}:"(unichar)"
+  NSLog(@"%C", (char)0x260300);
 
   NSLog(@"%C", 'a'); // expected-warning{{format specifies type 'unichar' (aka 
'unsigned short') but the argument has type 'char'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
Index: clang/lib/AST/FormatString.cpp
===
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -386,8 +386,10 @@
   case BuiltinType::SChar:
   case BuiltinType::Char_U:
   case BuiltinType::UChar:
-return T == C.UnsignedCharTy || T == C.SignedCharTy ? Match
-: NoMatch;
+return T == C.UnsignedCharTy || T == C.SignedCharTy ||
+   T == C.UnsignedShortTy || T == C.ShortTy
+   ? Match
+   : NoMatch;
   case BuiltinType::Short:
 return T == C.UnsignedShortTy ? Match : NoMatch;
   case BuiltinType::UShort:


Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -277,8 +277,8 @@
 
 void should_understand_small_integers() {
   printf("%hhu", (short) 10); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
-  printf("%hu\n", (unsigned char) 1); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'unsigned char'}}
-  printf("%hu\n", (uint8_t)1); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'uint8_t'}}
+  printf("%hu\n", (unsigned char)1); // no-warning
+  printf("%hu\n", (uint8_t)1);   // no-warning
 }
 
 void test11(void *p, char *s) {
Index: clang/test/Sema/format-strings-enum-fixed-type.cpp
===
--- clang/test/Sema/format-strings-enum-fixed-type.cpp
+++ clang/test/Sema/format-strings-enum-fixed-type.cpp
@@ -80,9 +80,9 @@
   printf("%hhd", CharConstant); // no-warning
 
   // This is not correct but it is safe. We warn because '%hd' shows intent.
-  printf("%hd", input); // expected-warning{{format specifies type 'short' but the argument has underlying type 'char'}}
-  

[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.h:372
+  REQUIRES_DESTRUCTION = 0x4,
 };
 

Does `llvm::Value*` actually guarantee three bits?  Presumably on 64-bit 
platforms, but we do support 32-bit hosts.

Fortunately, I don't actually think there's any reason to be space-conscious in 
this class; all the values are short-lived.  Might as well pull all the fields 
out into a bit-field or something.



Comment at: clang/lib/CodeGen/CGCall.h:390
+  return Value.getInt() & REQUIRES_DESTRUCTION;
+}
   };

I think the "is externally destructed" semantics make more sense — that is, the 
*default* should be that a destructor needs to get pushed, and maybe specific 
contexts should suppress that.  That will also let you avoid all the ugly code 
to compute whether destruction is required in a bunch of generic places.

Actually, "externally destructed" is a really problematic idea; we need to come 
up with a better solution for contexts that need to be careful about destructor 
ordering like this, and it should probably be based on passing around and 
deactivating destructors.  But that's for later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66094



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

Re: r367940 - [Sema] Add -Wpointer-compare

[David Blaikie via cfe-commits]

This seems a bit narrow - could it be generalized to all cases of '\0' as a
null pointer?

On Mon, Aug 5, 2019 at 3:14 PM George Burgess IV via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: gbiv
> Date: Mon Aug  5 15:15:40 2019
> New Revision: 367940
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367940=rev
> Log:
> [Sema] Add -Wpointer-compare
>
> This patch adds a warning that diagnoses comparisons of pointers to
> '\0'. This is often indicative of a bug (e.g. the user might've
> forgotten to dereference the pointer).
>
> Patch by Elaina Guan!
>
> Differential Revision: https://reviews.llvm.org/D65595
>
> Added:
> cfe/trunk/test/Sema/warn-nullchar-nullptr.c
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Sema/SemaExpr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=367940=367939=367940=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Aug  5
> 15:15:40 2019
> @@ -3296,6 +3296,10 @@ def warn_impcast_bool_to_null_pointer :
>  def warn_non_literal_null_pointer : Warning<
>  "expression which evaluates to zero treated as a null pointer
> constant of "
>  "type %0">, InGroup;
> +def warn_pointer_compare : Warning<
> +"comparing a pointer to a null character constant; did you mean "
> +"to compare to %select{NULL|(void *)0}0?">,
> +InGroup>;
>  def warn_impcast_null_pointer_to_integer : Warning<
>  "implicit conversion of %select{NULL|nullptr}0 constant to %1">,
>  InGroup;
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=367940=367939=367940=diff
>
> ==
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Mon Aug  5 15:15:40 2019
> @@ -10022,6 +10022,7 @@ public:
>QualType CheckShiftOperands( // C99 6.5.7
>  ExprResult , ExprResult , SourceLocation Loc,
>  BinaryOperatorKind Opc, bool IsCompAssign = false);
> +  void CheckPtrComparisonWithNullChar(ExprResult , ExprResult );
>QualType CheckCompareOperands( // C99 6.5.8/9
>ExprResult , ExprResult , SourceLocation Loc,
>BinaryOperatorKind Opc);
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=367940=367939=367940=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Aug  5 15:15:40 2019
> @@ -10443,6 +10443,32 @@ static QualType checkArithmeticOrEnumera
>return S.Context.getLogicalOperationType();
>  }
>
> +void Sema::CheckPtrComparisonWithNullChar(ExprResult , ExprResult
> ) {
> +  if (!NullE.get()->getType()->isAnyPointerType())
> +return;
> +  int NullValue = PP.isMacroDefined("NULL") ? 0 : 1;
> +  if (!E.get()->getType()->isAnyPointerType() &&
> +  E.get()->isNullPointerConstant(Context,
> + Expr::NPC_ValueDependentIsNotNull) ==
> +Expr::NPCK_ZeroExpression) {
> +if (const auto *CL = dyn_cast(E.get())) {
> +  if (CL->getValue() == 0)
> +Diag(E.get()->getExprLoc(), diag::warn_pointer_compare)
> +<< NullValue
> +<< FixItHint::CreateReplacement(E.get()->getExprLoc(),
> +NullValue ? "NULL" : "(void
> *)0");
> +} else if (const auto *CE = dyn_cast(E.get())) {
> +TypeSourceInfo *TI = CE->getTypeInfoAsWritten();
> +QualType T =
> Context.getCanonicalType(TI->getType()).getUnqualifiedType();
> +if (T == Context.CharTy)
> +  Diag(E.get()->getExprLoc(), diag::warn_pointer_compare)
> +  << NullValue
> +  << FixItHint::CreateReplacement(E.get()->getExprLoc(),
> +  NullValue ? "NULL" : "(void
> *)0");
> +  }
> +  }
> +}
> +
>  // C99 6.5.8, C++ [expr.rel]
>  QualType Sema::CheckCompareOperands(ExprResult , ExprResult ,
>  SourceLocation Loc,
> @@ -10476,6 +10502,10 @@ QualType Sema::CheckCompareOperands(Expr
>}
>
>checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/true);
> +  if (!getLangOpts().CPlusPlus && BinaryOperator::isEqualityOp(Opc)) {
> +CheckPtrComparisonWithNullChar(LHS, RHS);
> +CheckPtrComparisonWithNullChar(RHS, LHS);
> +  }
>
>// Handle vector comparisons separately.
>if (LHS.get()->getType()->isVectorType() ||
>
> Added: cfe/trunk/test/Sema/warn-nullchar-nullptr.c
> URL:
> 

[PATCH] D66186: Fix warning on printf('%hd', [char])

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Nathan-Huckleberry added a reviewer: rsmith.
Nathan-Huckleberry added a subscriber: nickdesaulniers.

Link: https://bugs.llvm.org/show_bug.cgi?id=41467


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66186

Files:
  clang/lib/AST/FormatString.cpp
  clang/test/FixIt/format.m
  clang/test/Sema/format-strings-enum-fixed-type.cpp
  clang/test/Sema/format-strings.c


Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -277,8 +277,8 @@
 
 void should_understand_small_integers() {
   printf("%hhu", (short) 10); // expected-warning{{format specifies type 
'unsigned char' but the argument has type 'short'}}
-  printf("%hu\n", (unsigned char) 1); // expected-warning{{format specifies 
type 'unsigned short' but the argument has type 'unsigned char'}}
-  printf("%hu\n", (uint8_t)1); // expected-warning{{format specifies type 
'unsigned short' but the argument has type 'uint8_t'}}
+  printf("%hu\n", (unsigned char)1);
+  printf("%hu\n", (uint8_t)1);
 }
 
 void test11(void *p, char *s) {
Index: clang/test/Sema/format-strings-enum-fixed-type.cpp
===
--- clang/test/Sema/format-strings-enum-fixed-type.cpp
+++ clang/test/Sema/format-strings-enum-fixed-type.cpp
@@ -80,9 +80,9 @@
   printf("%hhd", CharConstant); // no-warning
 
   // This is not correct but it is safe. We warn because '%hd' shows intent.
-  printf("%hd", input); // expected-warning{{format specifies type 'short' but 
the argument has underlying type 'char'}}
-  printf("%hd", CharConstant); // expected-warning{{format specifies type 
'short'}}
-  
+  printf("%hd", input);// no-warning
+  printf("%hd", CharConstant); // no-warning
+
   // This is not correct but it matches the promotion rules (and is safe).
   printf("%d", input); // no-warning
   printf("%d", CharConstant); // no-warning
Index: clang/test/FixIt/format.m
===
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -205,9 +205,7 @@
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%f"
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
 
-  NSLog(@"%C", (char)0x260300); // expected-warning{{format specifies type 
'unichar' (aka 'unsigned short') but the argument has type 'char'}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:22}:"(unichar)"
+  NSLog(@"%C", (char)0x260300);
 
   NSLog(@"%C", 'a'); // expected-warning{{format specifies type 'unichar' (aka 
'unsigned short') but the argument has type 'char'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
Index: clang/lib/AST/FormatString.cpp
===
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -386,8 +386,10 @@
   case BuiltinType::SChar:
   case BuiltinType::Char_U:
   case BuiltinType::UChar:
-return T == C.UnsignedCharTy || T == C.SignedCharTy ? Match
-: NoMatch;
+return T == C.UnsignedCharTy || T == C.SignedCharTy ||
+   T == C.UnsignedShortTy || T == C.ShortTy
+   ? Match
+   : NoMatch;
   case BuiltinType::Short:
 return T == C.UnsignedShortTy ? Match : NoMatch;
   case BuiltinType::UShort:


Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -277,8 +277,8 @@
 
 void should_understand_small_integers() {
   printf("%hhu", (short) 10); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
-  printf("%hu\n", (unsigned char) 1); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'unsigned char'}}
-  printf("%hu\n", (uint8_t)1); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'uint8_t'}}
+  printf("%hu\n", (unsigned char)1);
+  printf("%hu\n", (uint8_t)1);
 }
 
 void test11(void *p, char *s) {
Index: clang/test/Sema/format-strings-enum-fixed-type.cpp
===
--- clang/test/Sema/format-strings-enum-fixed-type.cpp
+++ clang/test/Sema/format-strings-enum-fixed-type.cpp
@@ -80,9 +80,9 @@
   printf("%hhd", CharConstant); // no-warning
 
   // This is not correct but it is safe. We warn because '%hd' shows intent.
-  printf("%hd", input); // expected-warning{{format specifies type 'short' but the argument has underlying type 

[PATCH] D66043: Add to -Wparentheses case of bitwise-and ("&") and bitwise-or ("|") verses conditional operator ("?:")

[Richard Trieu via Phabricator via cfe-commits]

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



Comment at: test/Sema/parentheses.c:148
+
+  (void)(x | b ? 1 : 2); // expected-warning {{operator '?:' has lower 
precedence than '|'}} expected-note 2{{place parentheses}}
+  (void)(x & b ? 1 : 2); // expected-warning {{operator '?:' has lower 
precedence than '&'}} expected-note 2{{place parentheses}}

MaskRay wrote:
> rtrieu wrote:
> > MaskRay wrote:
> > > I hope these `| ? :` `& ? :` warnings are disabled-by-default.
> > These new warnings reuse the existing parentheses warnings, which is 
> > diag::warn_precedence_conditional.  That is on by default, so this one as 
> > written is also on by default..
> I agree that 
> 
> `cond1 ? 0xf0 : 0x10 | cond2 ? 0x5 : 0x2;` is confusing and justifies a 
> warning. But **what is tested here is different**.
> 
> That is why I created D65192, because such warnings are very annoying as 
> enabled-by-default diagnostics.
> 
> I think this change will make it even harder to remove some annoying 
> -Wparentheses warnings..
I can add tests for the other case.  This one was used because it was shorter 
and easier to copy.

Would creating a parentheses subgroup (like -Wbitwise-conditional-parentheses), 
duping the warning into it, and marking it DefaultIgnore be a better 
alternative for you?


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

https://reviews.llvm.org/D66043



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

[PATCH] D65344: [analyzer] exploded-graph-rewriter: NFC: Replace explorers with trimmers.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368767: [analyzer] exploded-graph-rewriter: NFC: Refactor 
explorers into trimmers. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65344?vs=211992=214962#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65344

Files:
  cfe/trunk/test/Analysis/exploded-graph-rewriter/explorers.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
  cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
@@ -0,0 +1,37 @@
+// RUN: %exploded_graph_rewriter %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK,BASIC
+// RUN: %exploded_graph_rewriter -s %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK,SINGLE
+
+// FIXME: Substitution doesn't seem to work on Windows.
+// UNSUPPORTED: system-windows
+
+Node0x1 [shape=record,label=
+ "{{ "node_id": 1, "pointer": "0x1", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": []}\l}"];
+
+Node0x2 [shape=record,label=
+ "{{ "node_id": 2, "pointer": "0x2", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": []}\l}"];
+
+Node0x3 [shape=record,label=
+ "{{ "node_id": 3, "pointer": "0x3", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": []}\l}"];
+
+Node0x4 [shape=record,label=
+ "{{ "node_id": 4, "pointer": "0x4", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": []}\l}"];
+
+// CHECK: Node0x1 -> Node0x2;
+Node0x1 -> Node0x2;
+
+// BASIC: Node0x1 -> Node0x3;
+// SINGLE-NOT: Node0x1 -> Node0x3;
+Node0x1 -> Node0x3;
+
+// CHECK: Node0x2 -> Node0x4;
+Node0x2 -> Node0x4;
+
+// BASIC: Node0x3 -> Node0x4;
+// SINGLE-NOT: Node0x3 -> Node0x4;
+Node0x3 -> Node0x4;
Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -882,37 +882,36 @@
 visitor.visit_end_of_graph()
 
 
-# SinglePathExplorer traverses only a single path - the leftmost path
-# from the root. Useful when the trimmed graph is still too large
-# due to a large amount of equivalent reports.
-class SinglePathExplorer(object):
-def __init__(self):
-super(SinglePathExplorer, self).__init__()
+#===---===#
+# Trimmers cut out parts of the ExplodedGraph so that to focus on other parts.
+# Trimmers can be combined together by applying them sequentially.
+#===---===#
 
-def explore(self, graph, visitor):
-visitor.visit_begin_graph(graph)
 
-# Keep track of visited nodes in order to avoid loops.
-visited = set()
+# SinglePathTrimmer keeps only a single path - the leftmost path from the root.
+# Useful when the trimmed graph is still too large.
+class SinglePathTrimmer(object):
+def __init__(self):
+super(SinglePathTrimmer, self).__init__()
+
+def trim(self, graph):
+visited_nodes = set()
 node_id = graph.root_id
 while True:
-visited.add(node_id)
+visited_nodes.add(node_id)
 node = graph.nodes[node_id]
-logging.debug('Visiting ' + node_id)
-visitor.visit_node(node)
-if len(node.successors) == 0:
-break
-
-succ_id = node.successors[0]
-succ = graph.nodes[succ_id]
-logging.debug('Visiting edge: %s -> %s ' % (node_id, succ_id))
-visitor.visit_edge(node, succ)
-if succ_id in visited:
+if len(node.successors) > 0:
+succ_id = node.successors[0]
+succ = graph.nodes[succ_id]
+node.successors = [succ_id]
+succ.predecessors = [node_id]
+if succ_id in visited_nodes:
+break
+node_id = succ_id
+else:
 break
-
-node_id = succ_id
-
-visitor.visit_end_of_graph()
+graph.nodes = {node_id: graph.nodes[node_id]
+   for node_id in visited_nodes}
 
 
 #===---===#
@@ -960,10 +959,18 @@
 raw_line = raw_line.strip()
 graph.add_raw_line(raw_line)
 
-explorer = SinglePathExplorer() if args.single_path else BasicExplorer()
+

[PATCH] D65427: [analyzer] exploded-graph-rewriter: Implement Store pointers.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368769: [analyzer] exploded-graph-rewriter: Implement 
displaying Store pointers. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65427?vs=212257=214964#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65427

Files:
  cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
  cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
  cfe/trunk/utils/analyzer/exploded-graph-rewriter.py


Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -644,6 +644,10 @@
 if st is None:
 self._dump(' Nothing!')
 else:
+if self._dark_mode:
+self._dump(' (%s)' % st.ptr)
+else:
+self._dump(' (%s)' % st.ptr)
 if prev_st is not None:
 if s.store.is_different(prev_st):
 self._dump('')
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
@@ -4,6 +4,7 @@
 // UNSUPPORTED: system-windows
 
 // CHECK: Store: 
+// CHECK-SAME: (0x2)
 // CHECK-SAME: 
 // CHECK-SAME:   
 // CHECK-SAME: 
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
@@ -9,7 +9,7 @@
 // UNSUPPORTED: system-windows
 
 void escapes() {
-  // CHECK: Store: 
+  // CHECK: Store:  (0x{{[0-9a-f]*}})
   // CHECK-SAME: foo0
   // CHECK-SAME: Element\{"foo",0 S64b,char\}
   // CHECK: Environment: 


Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -644,6 +644,10 @@
 if st is None:
 self._dump(' Nothing!')
 else:
+if self._dark_mode:
+self._dump(' (%s)' % st.ptr)
+else:
+self._dump(' (%s)' % st.ptr)
 if prev_st is not None:
 if s.store.is_different(prev_st):
 self._dump('')
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
@@ -4,6 +4,7 @@
 // UNSUPPORTED: system-windows
 
 // CHECK: Store: 
+// CHECK-SAME: (0x2)
 // CHECK-SAME: 
 // CHECK-SAME:   
 // CHECK-SAME: 
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
@@ -9,7 +9,7 @@
 // UNSUPPORTED: system-windows
 
 void escapes() {
-  // CHECK: Store: 
+  // CHECK: Store:  (0x{{[0-9a-f]*}})
   // CHECK-SAME: foo0
   // CHECK-SAME: Element\{"foo",0 S64b,char\}
   // CHECK: Environment: 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65345: [analyzer] exploded-graph-rewriter: Implement manual graph trimming.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368768: [analyzer] exploded-graph-rewriter: Implement manual 
graph trimming. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65345?vs=211985=214963#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65345

Files:
  cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
  cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
@@ -1,7 +1,17 @@
 // RUN: %exploded_graph_rewriter %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK,BASIC
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,FOUR
 // RUN: %exploded_graph_rewriter -s %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK,SINGLE
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,FOUR
+// RUN: %exploded_graph_rewriter --to=0x2 %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,NOFOUR
+// RUN: %exploded_graph_rewriter --to 2 %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,NOFOUR
+// RUN: %exploded_graph_rewriter --to 2,3 %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,NOFOUR
+// RUN: %exploded_graph_rewriter --to 4 %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,FOUR
+// RUN: %exploded_graph_rewriter --to 4 -s %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,FOUR
 
 // FIXME: Substitution doesn't seem to work on Windows.
 // UNSUPPORTED: system-windows
@@ -22,16 +32,16 @@
  "{{ "node_id": 4, "pointer": "0x4", "has_report": false, "is_sink": false,
   "program_state": null, "program_points": []}\l}"];
 
-// CHECK: Node0x1 -> Node0x2;
 Node0x1 -> Node0x2;
-
-// BASIC: Node0x1 -> Node0x3;
-// SINGLE-NOT: Node0x1 -> Node0x3;
 Node0x1 -> Node0x3;
-
-// CHECK: Node0x2 -> Node0x4;
 Node0x2 -> Node0x4;
-
-// BASIC: Node0x3 -> Node0x4;
-// SINGLE-NOT: Node0x3 -> Node0x4;
 Node0x3 -> Node0x4;
+
+// ONE: Node0x1
+// NOTONE-NOT: Node0x1
+// TWO: Node0x2
+// NOTTWO-NOT: Node0x2
+// THREE: Node0x3
+// NOTTHREE-NOT: Node0x3
+// FOUR: Node0x4
+// NOTFOUR-NOT: Node0x4
Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -914,6 +914,52 @@
for node_id in visited_nodes}
 
 
+# TargetedTrimmer keeps paths that lead to specific nodes and discards all
+# other paths. Useful when you cannot use -trim-egraph (e.g. when debugging
+# a crash).
+class TargetedTrimmer(object):
+def __init__(self, target_nodes):
+super(TargetedTrimmer, self).__init__()
+self._target_nodes = target_nodes
+
+@staticmethod
+def parse_target_node(node, graph):
+if node.startswith('0x'):
+ret = 'Node' + node
+assert ret in graph.nodes
+return ret
+else:
+for other_id in graph.nodes:
+other = graph.nodes[other_id]
+if other.node_id == int(node):
+return other_id
+
+@staticmethod
+def parse_target_nodes(target_nodes, graph):
+return [TargetedTrimmer.parse_target_node(node, graph)
+for node in target_nodes.split(',')]
+
+def trim(self, graph):
+queue = self._target_nodes
+visited_nodes = set()
+
+while len(queue) > 0:
+node_id = queue.pop()
+visited_nodes.add(node_id)
+node = graph.nodes[node_id]
+for pred_id in node.predecessors:
+if pred_id not in visited_nodes:
+queue.append(pred_id)
+graph.nodes = {node_id: graph.nodes[node_id]
+   for node_id in visited_nodes}
+for node_id in graph.nodes:
+node = graph.nodes[node_id]
+node.successors = [succ_id for succ_id in node.successors
+   if succ_id in visited_nodes]
+node.predecessors = [succ_id for succ_id in node.predecessors
+ if succ_id in visited_nodes]
+
+
 #===---===#
 # The entry point to the script.
 #===---===#
@@ -939,6 +985,11 @@
 help='only display the leftmost path in the graph '
  '(useful for trimmed graphs that still '
  'branch too much)')
+parser.add_argument('--to', 

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368766: [analyzer] exploded-graph-rewriter: Open the 
converted graph immediately. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65250?vs=211640=214961#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65250

Files:
  cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg
  cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg
@@ -8,7 +8,7 @@
 config.test_format = lit.formats.ShTest(use_lit_shell == "0")
 
 config.substitutions.append(('%exploded_graph_rewriter',
- '\'%s\' %s' % (
+ '\'%s\' %s --dump-dot-only' % (
  config.python_executable,
  lit.util.which('exploded-graph-rewriter.py',
 os.path.join(
Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -394,16 +394,25 @@
 # A visitor that dumps the ExplodedGraph into a DOT file with fancy HTML-based
 # syntax highlighing.
 class DotDumpVisitor(object):
-def __init__(self, do_diffs, dark_mode, gray_mode, topo_mode):
+def __init__(self, do_diffs, dark_mode, gray_mode,
+ topo_mode, dump_dot_only):
 super(DotDumpVisitor, self).__init__()
 self._do_diffs = do_diffs
 self._dark_mode = dark_mode
 self._gray_mode = gray_mode
 self._topo_mode = topo_mode
+self._dump_dot_only = dump_dot_only
+self._output = []
 
-@staticmethod
-def _dump_raw(s):
-print(s, end='')
+def _dump_raw(self, s):
+if self._dump_dot_only:
+print(s, end='')
+else:
+self._output.append(s)
+
+def output(self):
+assert not self._dump_dot_only
+return ''.join(self._output)
 
 def _dump(self, s):
 s = s.replace('&', '') \
@@ -812,6 +821,44 @@
 def visit_end_of_graph(self):
 self._dump_raw('}\n')
 
+if not self._dump_dot_only:
+import sys
+import tempfile
+
+def write_temp_file(suffix, data):
+fd, filename = tempfile.mkstemp(suffix=suffix)
+print('Writing "%s"...' % filename)
+with os.fdopen(fd, 'w') as fp:
+fp.write(data)
+print('Done! Please remember to remove the file.')
+return filename
+
+try:
+import graphviz
+except ImportError:
+# The fallback behavior if graphviz is not installed!
+print('Python graphviz not found. Please invoke')
+print('  $ pip install graphviz')
+print('in order to enable automatic conversion to HTML.')
+print()
+print('You may also convert DOT to SVG manually via')
+print('  $ dot -Tsvg input.dot -o output.svg')
+print()
+write_temp_file('.dot', self.output())
+return
+
+svg = graphviz.pipe('dot', 'svg', self.output())
+
+filename = write_temp_file(
+'.html', '%s' % (
+ '#1a1a1a' if self._dark_mode else 'white', svg))
+if sys.platform == 'win32':
+os.startfile(filename)
+elif sys.platform == 'darwin':
+os.system('open "%s"' % filename)
+else:
+os.system('xdg-open "%s"' % filename)
+
 
 #===---===#
 # Explorers know how to traverse the ExplodedGraph in a certain order.
@@ -874,8 +921,10 @@
 
 
 def main():
-parser = argparse.ArgumentParser()
-parser.add_argument('filename', type=str)
+parser = argparse.ArgumentParser(
+description='Display and manipulate Exploded Graph dumps.')
+parser.add_argument('filename', type=str,
+help='the .dot file produced by the Static Analyzer')
 parser.add_argument('-v', '--verbose', action='store_const',
 dest='loglevel', const=logging.DEBUG,
 default=logging.WARNING,
@@ -897,6 +946,11 @@
 parser.add_argument('--gray', action='store_const', dest='gray',
 const=True, default=False,
  

[PATCH] D66176: Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Tom Stellard via Phabricator via cfe-commits]

tstellar updated this revision to Diff 214960.
tstellar added a comment.

Another attempt to fix this that depends less on the build directory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66176

Files:
  clang/test/Driver/modules.cpp


Index: clang/test/Driver/modules.cpp
===
--- clang/test/Driver/modules.cpp
+++ clang/test/Driver/modules.cpp
@@ -15,7 +15,8 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// CHECK-COMPILE-SAME: {{ -o }}
+// CHECK-COMPILE-SAME: module{{2*}}.{{pcm.o|s}}
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 


Index: clang/test/Driver/modules.cpp
===
--- clang/test/Driver/modules.cpp
+++ clang/test/Driver/modules.cpp
@@ -15,7 +15,8 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// CHECK-COMPILE-SAME: {{ -o }}
+// CHECK-COMPILE-SAME: module{{2*}}.{{pcm.o|s}}
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r368767 - [analyzer] exploded-graph-rewriter: NFC: Refactor explorers into trimmers.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Aug 13 16:04:50 2019
New Revision: 368767

URL: http://llvm.org/viewvc/llvm-project?rev=368767=rev
Log:
[analyzer] exploded-graph-rewriter: NFC: Refactor explorers into trimmers.

Explorers aren't the right abstraction. For the purposes of displaying svg files
we don't care in which order do we explore the nodes. We may care about this for
other analyses, but we're not there yet.

The function of cutting out chunks of the graph is performed poorly by
the explorers, because querying predecessors/successors on the explored nodes
yields original successors/predecessors even if they aren't being explored.

Introduce a new entity, "trimmers", that do one thing but to it right: cut out
chunks of the graph. Trimmers mutate the graph, so stale edges aren't even
visible to their consumers in the pipeline. Additionally, trimmers are
intrinsically composable: multiple trimmers can be applied to the graph
sequentially.

Refactor the single-path explorer into the single-path trimmer.
Rename the test file for consistency.

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

Added:
cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
Removed:
cfe/trunk/test/Analysis/exploded-graph-rewriter/explorers.dot
Modified:
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Removed: cfe/trunk/test/Analysis/exploded-graph-rewriter/explorers.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/explorers.dot?rev=368766=auto
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/explorers.dot (original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/explorers.dot (removed)
@@ -1,37 +0,0 @@
-// RUN: %exploded_graph_rewriter %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK,BASIC
-// RUN: %exploded_graph_rewriter -s %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK,SINGLE
-
-// FIXME: Substitution doesn't seem to work on Windows.
-// UNSUPPORTED: system-windows
-
-Node0x1 [shape=record,label=
- "{{ "node_id": 1, "pointer": "0x1", "has_report": false, "is_sink": false,
-  "program_state": null, "program_points": []}\l}"];
-
-Node0x2 [shape=record,label=
- "{{ "node_id": 2, "pointer": "0x2", "has_report": false, "is_sink": false,
-  "program_state": null, "program_points": []}\l}"];
-
-Node0x3 [shape=record,label=
- "{{ "node_id": 3, "pointer": "0x3", "has_report": false, "is_sink": false,
-  "program_state": null, "program_points": []}\l}"];
-
-Node0x4 [shape=record,label=
- "{{ "node_id": 4, "pointer": "0x4", "has_report": false, "is_sink": false,
-  "program_state": null, "program_points": []}\l}"];
-
-// CHECK: Node0x1 -> Node0x2;
-Node0x1 -> Node0x2;
-
-// BASIC: Node0x1 -> Node0x3;
-// SINGLE-NOT: Node0x1 -> Node0x3;
-Node0x1 -> Node0x3;
-
-// CHECK: Node0x2 -> Node0x4;
-Node0x2 -> Node0x4;
-
-// BASIC: Node0x3 -> Node0x4;
-// SINGLE-NOT: Node0x3 -> Node0x4;
-Node0x3 -> Node0x4;

Added: cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot?rev=368767=auto
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot (added)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot Tue Aug 13 
16:04:50 2019
@@ -0,0 +1,37 @@
+// RUN: %exploded_graph_rewriter %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK,BASIC
+// RUN: %exploded_graph_rewriter -s %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK,SINGLE
+
+// FIXME: Substitution doesn't seem to work on Windows.
+// UNSUPPORTED: system-windows
+
+Node0x1 [shape=record,label=
+ "{{ "node_id": 1, "pointer": "0x1", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": []}\l}"];
+
+Node0x2 [shape=record,label=
+ "{{ "node_id": 2, "pointer": "0x2", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": []}\l}"];
+
+Node0x3 [shape=record,label=
+ "{{ "node_id": 3, "pointer": "0x3", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": []}\l}"];
+
+Node0x4 [shape=record,label=
+ "{{ "node_id": 4, "pointer": "0x4", "has_report": false, "is_sink": false,
+  "program_state": null, "program_points": []}\l}"];
+
+// CHECK: Node0x1 -> Node0x2;
+Node0x1 -> Node0x2;
+
+// BASIC: Node0x1 -> Node0x3;
+// SINGLE-NOT: Node0x1 -> Node0x3;
+Node0x1 -> Node0x3;
+
+// CHECK: Node0x2 -> Node0x4;
+Node0x2 -> Node0x4;
+
+// BASIC: Node0x3 -> Node0x4;
+// SINGLE-NOT: Node0x3 -> Node0x4;
+Node0x3 -> Node0x4;

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=368767=368766=368767=diff
==
--- 

r368769 - [analyzer] exploded-graph-rewriter: Implement displaying Store pointers.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Aug 13 16:04:56 2019
New Revision: 368769

URL: http://llvm.org/viewvc/llvm-project?rev=368769=rev
Log:
[analyzer] exploded-graph-rewriter: Implement displaying Store pointers.

They're useful when trying to understand what's going on
inside your LazyCompoundValues.

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

Modified:
cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c?rev=368769=368768=368769=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c (original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/escapes.c Tue Aug 13 
16:04:56 2019
@@ -9,7 +9,7 @@
 // UNSUPPORTED: system-windows
 
 void escapes() {
-  // CHECK: Store: 
+  // CHECK: Store:  (0x{{[0-9a-f]*}})
   // CHECK-SAME: foo0
   // CHECK-SAME: Element\{"foo",0 S64b,char\}
   // CHECK: Environment: 

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot?rev=368769=368768=368769=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot (original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot Tue Aug 13 
16:04:56 2019
@@ -4,6 +4,7 @@
 // UNSUPPORTED: system-windows
 
 // CHECK: Store: 
+// CHECK-SAME: (0x2)
 // CHECK-SAME: 
 // CHECK-SAME:   
 // CHECK-SAME: 

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=368769=368768=368769=diff
==
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py (original)
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py Tue Aug 13 16:04:56 2019
@@ -644,6 +644,10 @@ class DotDumpVisitor(object):
 if st is None:
 self._dump(' Nothing!')
 else:
+if self._dark_mode:
+self._dump(' (%s)' % st.ptr)
+else:
+self._dump(' (%s)' % st.ptr)
 if prev_st is not None:
 if s.store.is_different(prev_st):
 self._dump('')


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

r368766 - [analyzer] exploded-graph-rewriter: Open the converted graph immediately.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Aug 13 16:04:47 2019
New Revision: 368766

URL: http://llvm.org/viewvc/llvm-project?rev=368766=rev
Log:
[analyzer] exploded-graph-rewriter: Open the converted graph immediately.

Change the default behavior: the tool no longer dumps the rewritten .dot file
to stdout, but instead it automatically converts it into an .html file
(which essentially wraps an .svg file) and immediately opens it with
the default web browser.

This means that the tool should now be fairly easy to use:

  $ exploded-graph-rewriter.py /tmp/ExprEngine.dot

The benefits of wrapping the .svg file into an .html file are:

- It'll open in a web browser, which is the intended behavior.
  An .svg file would be open with an image viewer/editor instead.
- It avoids the white background around the otherwise dark svg area
  in dark mode.

The feature can be turned off by passing a flag '--rewrite-only'.
The LIT substitution is updated to enforce the old mode because
we don't want web browsers opening on our buildbots.

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

Modified:
cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg?rev=368766=368765=368766=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg (original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/lit.local.cfg Tue Aug 13 
16:04:47 2019
@@ -8,7 +8,7 @@ use_lit_shell = os.environ.get("LIT_USE_
 config.test_format = lit.formats.ShTest(use_lit_shell == "0")
 
 config.substitutions.append(('%exploded_graph_rewriter',
- '\'%s\' %s' % (
+ '\'%s\' %s --dump-dot-only' % (
  config.python_executable,
  lit.util.which('exploded-graph-rewriter.py',
 os.path.join(

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=368766=368765=368766=diff
==
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py (original)
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py Tue Aug 13 16:04:47 2019
@@ -394,16 +394,25 @@ class ExplodedGraph(object):
 # A visitor that dumps the ExplodedGraph into a DOT file with fancy HTML-based
 # syntax highlighing.
 class DotDumpVisitor(object):
-def __init__(self, do_diffs, dark_mode, gray_mode, topo_mode):
+def __init__(self, do_diffs, dark_mode, gray_mode,
+ topo_mode, dump_dot_only):
 super(DotDumpVisitor, self).__init__()
 self._do_diffs = do_diffs
 self._dark_mode = dark_mode
 self._gray_mode = gray_mode
 self._topo_mode = topo_mode
+self._dump_dot_only = dump_dot_only
+self._output = []
 
-@staticmethod
-def _dump_raw(s):
-print(s, end='')
+def _dump_raw(self, s):
+if self._dump_dot_only:
+print(s, end='')
+else:
+self._output.append(s)
+
+def output(self):
+assert not self._dump_dot_only
+return ''.join(self._output)
 
 def _dump(self, s):
 s = s.replace('&', '') \
@@ -812,6 +821,44 @@ class DotDumpVisitor(object):
 def visit_end_of_graph(self):
 self._dump_raw('}\n')
 
+if not self._dump_dot_only:
+import sys
+import tempfile
+
+def write_temp_file(suffix, data):
+fd, filename = tempfile.mkstemp(suffix=suffix)
+print('Writing "%s"...' % filename)
+with os.fdopen(fd, 'w') as fp:
+fp.write(data)
+print('Done! Please remember to remove the file.')
+return filename
+
+try:
+import graphviz
+except ImportError:
+# The fallback behavior if graphviz is not installed!
+print('Python graphviz not found. Please invoke')
+print('  $ pip install graphviz')
+print('in order to enable automatic conversion to HTML.')
+print()
+print('You may also convert DOT to SVG manually via')
+print('  $ dot -Tsvg input.dot -o output.svg')
+print()
+write_temp_file('.dot', self.output())
+return
+
+svg = graphviz.pipe('dot', 'svg', self.output())
+
+filename = write_temp_file(
+'.html', '%s' % (
+ '#1a1a1a' if self._dark_mode else 'white', svg))
+if 

r368765 - [analyzer] Disable the checker-plugins test on Darwin.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Aug 13 16:04:44 2019
New Revision: 368765

URL: http://llvm.org/viewvc/llvm-project?rev=368765=rev
Log:
[analyzer] Disable the checker-plugins test on Darwin.

Fixes a buildbot.

Modified:
cfe/trunk/test/Analysis/checker-plugins.c

Modified: cfe/trunk/test/Analysis/checker-plugins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/checker-plugins.c?rev=368765=368764=368765=diff
==
--- cfe/trunk/test/Analysis/checker-plugins.c (original)
+++ cfe/trunk/test/Analysis/checker-plugins.c Tue Aug 13 16:04:44 2019
@@ -1,5 +1,9 @@
 // REQUIRES: plugins
 
+// FIXME: This test fails on clang-stage2-cmake-RgSan,
+// see also https://reviews.llvm.org/D62445#1613268
+// UNSUPPORTED: darwin
+
 // RUN: %clang_analyze_cc1 -verify %s \
 // RUN:   -load %llvmshlibdir/SampleAnalyzerPlugin%pluginext \
 // RUN:   -analyzer-checker='example.MainCallChecker'


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

r368768 - [analyzer] exploded-graph-rewriter: Implement manual graph trimming.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Aug 13 16:04:53 2019
New Revision: 368768

URL: http://llvm.org/viewvc/llvm-project?rev=368768=rev
Log:
[analyzer] exploded-graph-rewriter: Implement manual graph trimming.

When -trim-egraph is unavailable (say, when you're debugging a crash on
a real-world code that takes too long to reduce), it makes sense to view
the untrimmed graph up to the crashing node's predecessor, then dump the ID
(or a pointer) of the node in the attached debugger, and then trim
the dumped graph in order to keep only paths from the root to the node.

The newly added --to flag does exactly that:

$ exploded-graph-rewriter.py ExprEngine.dot --to 0x12229acd0

Multiple nodes can be specified. Stable IDs of nodes can be used
instead of pointers.

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

Modified:
cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot?rev=368768=368767=368768=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot (original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/trimmers.dot Tue Aug 13 
16:04:53 2019
@@ -1,7 +1,17 @@
 // RUN: %exploded_graph_rewriter %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK,BASIC
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,FOUR
 // RUN: %exploded_graph_rewriter -s %s \
-// RUN: | FileCheck %s -check-prefixes=CHECK,SINGLE
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,FOUR
+// RUN: %exploded_graph_rewriter --to=0x2 %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,NOFOUR
+// RUN: %exploded_graph_rewriter --to 2 %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,NOFOUR
+// RUN: %exploded_graph_rewriter --to 2,3 %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,NOFOUR
+// RUN: %exploded_graph_rewriter --to 4 %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,THREE,FOUR
+// RUN: %exploded_graph_rewriter --to 4 -s %s \
+// RUN: | FileCheck %s -check-prefixes=ONE,TWO,NOTHREE,FOUR
 
 // FIXME: Substitution doesn't seem to work on Windows.
 // UNSUPPORTED: system-windows
@@ -22,16 +32,16 @@ Node0x4 [shape=record,label=
  "{{ "node_id": 4, "pointer": "0x4", "has_report": false, "is_sink": false,
   "program_state": null, "program_points": []}\l}"];
 
-// CHECK: Node0x1 -> Node0x2;
 Node0x1 -> Node0x2;
-
-// BASIC: Node0x1 -> Node0x3;
-// SINGLE-NOT: Node0x1 -> Node0x3;
 Node0x1 -> Node0x3;
-
-// CHECK: Node0x2 -> Node0x4;
 Node0x2 -> Node0x4;
-
-// BASIC: Node0x3 -> Node0x4;
-// SINGLE-NOT: Node0x3 -> Node0x4;
 Node0x3 -> Node0x4;
+
+// ONE: Node0x1
+// NOTONE-NOT: Node0x1
+// TWO: Node0x2
+// NOTTWO-NOT: Node0x2
+// THREE: Node0x3
+// NOTTHREE-NOT: Node0x3
+// FOUR: Node0x4
+// NOTFOUR-NOT: Node0x4

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=368768=368767=368768=diff
==
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py (original)
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py Tue Aug 13 16:04:53 2019
@@ -914,6 +914,52 @@ class SinglePathTrimmer(object):
for node_id in visited_nodes}
 
 
+# TargetedTrimmer keeps paths that lead to specific nodes and discards all
+# other paths. Useful when you cannot use -trim-egraph (e.g. when debugging
+# a crash).
+class TargetedTrimmer(object):
+def __init__(self, target_nodes):
+super(TargetedTrimmer, self).__init__()
+self._target_nodes = target_nodes
+
+@staticmethod
+def parse_target_node(node, graph):
+if node.startswith('0x'):
+ret = 'Node' + node
+assert ret in graph.nodes
+return ret
+else:
+for other_id in graph.nodes:
+other = graph.nodes[other_id]
+if other.node_id == int(node):
+return other_id
+
+@staticmethod
+def parse_target_nodes(target_nodes, graph):
+return [TargetedTrimmer.parse_target_node(node, graph)
+for node in target_nodes.split(',')]
+
+def trim(self, graph):
+queue = self._target_nodes
+visited_nodes = set()
+
+while len(queue) > 0:
+node_id = queue.pop()
+visited_nodes.add(node_id)
+node = graph.nodes[node_id]
+for pred_id in node.predecessors:
+if pred_id not in visited_nodes:
+queue.append(pred_id)
+graph.nodes = {node_id: graph.nodes[node_id]
+   for node_id in visited_nodes}
+for node_id in graph.nodes:
+   

[PATCH] D65250: [analyzer] exploded-graph-rewriter: Improve user-friendliness.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 2 inline comments as done.
NoQ added inline comments.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:846
+print('  $ dot -Tsvg input.dot -o output.svg')
+print()
+write_temp_file('.dot', self.output())

Charusso wrote:
> Why we need multiple prints to print one message? According to the Stack 
> Overflow:
> ```
> print("""
> Line1
> Line2
> """)
> ```
> 
> Link: 
> https://stackoverflow.com/questions/34980251/how-to-print-multiple-lines-of-text-with-python
I dislike what it does to my formatting. We're in 4 nested scopes and it 
suddenly requires me to make an un-indented block.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:953
+ 'displaying it, dump the rewritten dot file '
+ 'to stdout')
 args = parser.parse_args()

Charusso wrote:
> This flag is a little-bit too long and does not really emphasize what it 
> suppose to do. What about just `--dump` or `--stdout`?
Renamed to `--dump-dot-only`.


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

https://reviews.llvm.org/D65250



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

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 214954.
mgehre marked an inline comment as done.
mgehre added a comment.

Add tests for rvalue-ref overloads


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66164

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp


Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -168,7 +168,15 @@
 struct optional {
   optional();
   optional(const T&);
-  T *();
+  T *() &;
+  T &*() &&;
+  T () &;
+  T &() &&;
+};
+
+template
+struct stack {
+  T ();
 };
 }
 
@@ -186,6 +194,16 @@
   return s.c_str(); // expected-warning {{address of stack memory associated 
with local variable 's' returned}}
 }
 
+int () {
+  std::optional o;
+  return o.value(); // expected-warning {{reference to stack memory associated 
with local variable 'o' returned}}
+}
+
+int () {
+  std::optional o;
+  return *o; // expected-warning {{reference to stack memory associated with 
local variable 'o' returned}}
+}
+
 const char *danglingRawPtrFromTemp() {
   return std::basic_string().c_str(); // expected-warning {{returning 
address of local temporary object}}
 }
@@ -201,9 +219,10 @@
 }
 
 void danglingReferenceFromTempOwner() {
-  int  = *std::optional(); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
-  int  = *std::optional(5); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
-  int  = std::vector().at(3); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
+  int & = *std::optional();  // expected-warning {{object 
backing the pointer will be destroyed at the end of the full-expression}}
+  int & = *std::optional(5);// expected-warning {{object 
backing the pointer will be destroyed at the end of the full-expression}}
+  int & = std::optional(5).value(); // expected-warning {{object 
backing the pointer will be destroyed at the end of the full-expression}}
+  int  = std::vector().at(3);   // expected-warning {{object 
backing the pointer will be destroyed at the end of the full-expression}}
 }
 
 std::vector getTempVec();
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6591,7 +6591,7 @@
  OO == OverloadedOperatorKind::OO_Star;
 }
 return llvm::StringSwitch(Callee->getName())
-.Cases("front", "back", "at", true)
+.Cases("front", "back", "at", "top", "value", true)
 .Default(false);
   }
   return false;


Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -168,7 +168,15 @@
 struct optional {
   optional();
   optional(const T&);
-  T *();
+  T *() &;
+  T &*() &&;
+  T () &;
+  T &() &&;
+};
+
+template
+struct stack {
+  T ();
 };
 }
 
@@ -186,6 +194,16 @@
   return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
 }
 
+int () {
+  std::optional o;
+  return o.value(); // expected-warning {{reference to stack memory associated with local variable 'o' returned}}
+}
+
+int () {
+  std::optional o;
+  return *o; // expected-warning {{reference to stack memory associated with local variable 'o' returned}}
+}
+
 const char *danglingRawPtrFromTemp() {
   return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
 }
@@ -201,9 +219,10 @@
 }
 
 void danglingReferenceFromTempOwner() {
-  int  = *std::optional(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
-  int  = *std::optional(5); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
-  int  = std::vector().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int & = *std::optional();  // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int & = *std::optional(5);// expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int & = std::optional(5).value(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int  = std::vector().at(3);   // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
 }
 
 std::vector getTempVec();
Index: clang/lib/Sema/SemaInit.cpp

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

[Matthias Gehre via Phabricator via cfe-commits]

mgehre marked 3 inline comments as done.
mgehre added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6583
 .Cases("end", "rend", "cend", "crend", true)
-.Cases("c_str", "data", "get", true)
+.Cases("c_str", "data", "get", "value", true)
 // Map and set types.

xazax.hun wrote:
> Oh, this one needs to be updated, as `value` returns a reference not a 
> pointer. 
Yes, I noticed the same.



Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:172
   T *();
+  T ();
+};

xazax.hun wrote:
> mgehre wrote:
> > xazax.hun wrote:
> > > I actually was a bit lazy when I added these tests. Both `value` and 
> > > `operator*` is overloaded on `&&`. But if you do not feel like adjusting 
> > > the tests this is fine, I can do it myself later :)
> > I'll change it to use the `&` variant in the test - the `&&` cannot dangle 
> > as far as I understand.
> It can!
> 
> Consider the following code:
> 
> ```
> int & = *std::optional(5);
> // r dangles here.
> ```
Oh, sure!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66164



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

[PATCH] D65453: Improve the accuracy of the Clang call graph analysis

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm, the patch doesn't pass its own test for me, could you double-check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65453



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

[PATCH] D66176: Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

It's probably a pathological case, but wouldn't this still fail if the build 
directory contained ".s" followed by a space in the name? Something like 
"build-foo.s bar"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66176



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

r368762 - [clang][DirectoryWatcher] Fix Windows stub after LLVM change

[Jan Korous via cfe-commits]

Author: jkorous
Date: Tue Aug 13 15:39:50 2019
New Revision: 368762

URL: http://llvm.org/viewvc/llvm-project?rev=368762=rev
Log:
[clang][DirectoryWatcher] Fix Windows stub after LLVM change

r367979 changed DirectoryWatcher::Create to return an llvm::Expected.
Adjust the Windows stub accordingly.

(upstreamed from github.com/apple/swift-clang)

Modified:
cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp

Modified: cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp?rev=368762=368761=368762=diff
==
--- cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp 
(original)
+++ cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp Tue Aug 
13 15:39:50 2019
@@ -40,9 +40,11 @@ public:
 };
 } // namespace
 
-std::unique_ptr clang::DirectoryWatcher::create(
+llvm::Expected>
+clang::DirectoryWatcher::create(
 StringRef Path,
 std::function, bool)> 
Receiver,
 bool WaitForInitialSync) {
-return nullptr;
+  return llvm::Expected>(
+  llvm::errorCodeToError(std::make_error_code(std::errc::not_supported)));
 }


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

r368761 - [clang] DirectoryWatcher for Windows stubs (to fix build break).

[Jan Korous via cfe-commits]

Author: jkorous
Date: Tue Aug 13 15:39:26 2019
New Revision: 368761

URL: http://llvm.org/viewvc/llvm-project?rev=368761=rev
Log:
[clang] DirectoryWatcher for Windows stubs (to fix build break).

This is just a code skeleton for DirectoryWatcher-windows.cpp so the
build on Windows stops breaking.

(upstreamed from github.com/apple/swift-clang)

Added:
cfe/trunk/lib/DirectoryWatcher/windows/
cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
Modified:
cfe/trunk/lib/DirectoryWatcher/CMakeLists.txt

Modified: cfe/trunk/lib/DirectoryWatcher/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/DirectoryWatcher/CMakeLists.txt?rev=368761=368760=368761=diff
==
--- cfe/trunk/lib/DirectoryWatcher/CMakeLists.txt (original)
+++ cfe/trunk/lib/DirectoryWatcher/CMakeLists.txt Tue Aug 13 15:39:26 2019
@@ -17,6 +17,8 @@ elseif(CMAKE_SYSTEM_NAME MATCHES "Linux"
 list(APPEND DIRECTORY_WATCHER_SOURCES linux/DirectoryWatcher-linux.cpp)
 find_package(Threads REQUIRED)
   endif()
+elseif(CMAKE_SYSTEM_NAME MATCHES "Windows")
+  list(APPEND DIRECTORY_WATCHER_SOURCES windows/DirectoryWatcher-windows.cpp)
 else()
   list(APPEND DIRECTORY_WATCHER_SOURCES 
default/DirectoryWatcher-not-implemented.cpp)
 endif()

Added: cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp?rev=368761=auto
==
--- cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp (added)
+++ cfe/trunk/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp Tue Aug 
13 15:39:26 2019
@@ -0,0 +1,48 @@
+//===- DirectoryWatcher-windows.cpp - Windows-platform directory watching 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// TODO: This is not yet an implementation, but it will make it so Windows
+//   builds don't fail.
+
+#include "DirectoryScanner.h"
+#include "clang/DirectoryWatcher/DirectoryWatcher.h"
+
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/AlignOf.h"
+#include "llvm/Support/Errno.h"
+#include "llvm/Support/Mutex.h"
+#include "llvm/Support/Path.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace {
+
+using namespace llvm;
+using namespace clang;
+
+class DirectoryWatcherWindows : public clang::DirectoryWatcher {
+public:
+  ~DirectoryWatcherWindows() override { }
+  void InitialScan() { }
+  void EventReceivingLoop() { }
+  void StopWork() { }
+};
+} // namespace
+
+std::unique_ptr clang::DirectoryWatcher::create(
+StringRef Path,
+std::function, bool)> 
Receiver,
+bool WaitForInitialSync) {
+return nullptr;
+}


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

[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added a reviewer: gribozavr.
Herald added a subscriber: Szelethus.
Herald added a project: clang.

This fixes inference of gsl::Pointer on std::set::iterator with libstdc++ (the 
typedef for iterator
on the template is a DependentNameType - we can only put the gsl::Pointer 
attribute
on the underlaying record after instantiation)

inference of gsl::Pointer on std::vector::iterator with libc++ (the class was 
forward-declared,
we added the gsl::Pointer on the canonical decl (the forward decl), and later 
when the
template was instantiated, there was no attribute on the definition so it was 
not instantiated).

and a duplicate gsl::Pointer on some class with libstdc++ (we first added an 
attribute to
a incomplete instantiation, and then another was copied from the template 
definition
when the instantiation was completed).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66179

Files:
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp

Index: clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
===
--- clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
+++ clang/test/SemaCXX/attr-gsl-owner-pointer-std.cpp
@@ -92,6 +92,59 @@
 static_assert(sizeof(unordered_map::iterator), ""); // Force instantiation.
 } // namespace inlinens
 
+// The iterator typedef is a DependentNameType.
+template 
+class __unordered_multimap_iterator {};
+// CHECK: ClassTemplateDecl {{.*}} __unordered_multimap_iterator
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __unordered_multimap_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template 
+class __unordered_multimap_base {
+public:
+  using iterator = __unordered_multimap_iterator;
+};
+
+template 
+class unordered_multimap {
+  // CHECK: ClassTemplateDecl {{.*}} unordered_multimap
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} unordered_multimap
+  // CHECK: OwnerAttr {{.*}}
+public:
+  using _Mybase = __unordered_multimap_base;
+  using iterator = typename _Mybase::iterator;
+};
+static_assert(sizeof(unordered_multimap::iterator), ""); // Force instantiation.
+
+// The canonical declaration of the iterator template is not its definition.
+template 
+class __unordered_multiset_iterator;
+// CHECK: ClassTemplateDecl {{.*}} __unordered_multiset_iterator
+// CHECK: PointerAttr
+// CHECK: ClassTemplateSpecializationDecl {{.*}} __unordered_multiset_iterator
+// CHECK: TemplateArgument type 'int'
+// CHECK: PointerAttr
+
+template 
+class __unordered_multiset_iterator {
+  // CHECK: ClassTemplateDecl {{.*}} prev {{.*}} __unordered_multiset_iterator
+  // CHECK: PointerAttr
+};
+
+template 
+class unordered_multiset {
+  // CHECK: ClassTemplateDecl {{.*}} unordered_multiset
+  // CHECK: OwnerAttr {{.*}}
+  // CHECK: ClassTemplateSpecializationDecl {{.*}} unordered_multiset
+  // CHECK: OwnerAttr {{.*}}
+public:
+  using iterator = __unordered_multiset_iterator;
+};
+
+static_assert(sizeof(unordered_multiset::iterator), ""); // Force instantiation.
+
 // std::list has an implicit gsl::Owner attribute,
 // but explicit attributes take precedence.
 template 
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -552,6 +552,18 @@
   continue;
 }
 
+if (auto *A = dyn_cast(TmplAttr)) {
+  if (!New->hasAttr())
+New->addAttr(A->clone(Context));
+  continue;
+}
+
+if (auto *A = dyn_cast(TmplAttr)) {
+  if (!New->hasAttr())
+New->addAttr(A->clone(Context));
+  continue;
+}
+
 assert(!TmplAttr->isPackExpansion());
 if (TmplAttr->isLateParsed() && LateAttrs) {
   // Late parsed attributes must be instantiated and attached after the
@@ -711,6 +723,9 @@
 
   SemaRef.InstantiateAttrs(TemplateArgs, D, Typedef);
 
+  if (D->getUnderlyingType()->getAs())
+SemaRef.inferGslPointerAttribute(Typedef);
+
   Typedef->setAccess(D->getAccess());
 
   return Typedef;
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -88,13 +88,20 @@
 template 
 static void addGslOwnerPointerAttributeIfNotExisting(ASTContext ,
  CXXRecordDecl *Record) {
-  CXXRecordDecl *Canonical = Record->getCanonicalDecl();
-  if (Canonical->hasAttr() || Canonical->hasAttr())
+  if (Record->hasAttr() || Record->hasAttr())
 return;
 
-  Canonical->addAttr(::new (Context) Attribute(SourceRange{}, Context,
-   /*DerefType*/ nullptr,
-   /*Spelling=*/0));
+  Record->addAttr(::new (Context) 

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 214950.
mgehre marked 2 inline comments as done.
mgehre added a comment.

Fix commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66164

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp


Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -168,7 +168,14 @@
 struct optional {
   optional();
   optional(const T&);
-  T *();
+  T *() &;
+  T &*() &&;
+  T () &;
+};
+
+template
+struct stack {
+  T ();
 };
 }
 
@@ -186,6 +193,16 @@
   return s.c_str(); // expected-warning {{address of stack memory associated 
with local variable 's' returned}}
 }
 
+int () {
+  std::optional o;
+  return o.value(); // expected-warning {{reference to stack memory associated 
with local variable 'o' returned}}
+}
+
+int () {
+  std::optional o;
+  return *o; // expected-warning {{reference to stack memory associated with 
local variable 'o' returned}}
+}
+
 const char *danglingRawPtrFromTemp() {
   return std::basic_string().c_str(); // expected-warning {{returning 
address of local temporary object}}
 }
@@ -201,9 +218,8 @@
 }
 
 void danglingReferenceFromTempOwner() {
-  int  = *std::optional(); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
-  int  = *std::optional(5); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
-  int  = std::vector().at(3); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
+  int  = std::vector().at(3); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
+  int  = std::stack().top();  // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
 }
 
 std::vector getTempVec();
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6591,7 +6591,7 @@
  OO == OverloadedOperatorKind::OO_Star;
 }
 return llvm::StringSwitch(Callee->getName())
-.Cases("front", "back", "at", true)
+.Cases("front", "back", "at", "top", "value", true)
 .Default(false);
   }
   return false;


Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -168,7 +168,14 @@
 struct optional {
   optional();
   optional(const T&);
-  T *();
+  T *() &;
+  T &*() &&;
+  T () &;
+};
+
+template
+struct stack {
+  T ();
 };
 }
 
@@ -186,6 +193,16 @@
   return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
 }
 
+int () {
+  std::optional o;
+  return o.value(); // expected-warning {{reference to stack memory associated with local variable 'o' returned}}
+}
+
+int () {
+  std::optional o;
+  return *o; // expected-warning {{reference to stack memory associated with local variable 'o' returned}}
+}
+
 const char *danglingRawPtrFromTemp() {
   return std::basic_string().c_str(); // expected-warning {{returning address of local temporary object}}
 }
@@ -201,9 +218,8 @@
 }
 
 void danglingReferenceFromTempOwner() {
-  int  = *std::optional(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
-  int  = *std::optional(5); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
-  int  = std::vector().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int  = std::vector().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int  = std::stack().top();  // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
 }
 
 std::vector getTempVec();
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6591,7 +6591,7 @@
  OO == OverloadedOperatorKind::OO_Star;
 }
 return llvm::StringSwitch(Callee->getName())
-.Cases("front", "back", "at", true)
+.Cases("front", "back", "at", "top", "value", true)
 .Default(false);
   }
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:172
   T *();
+  T ();
+};

mgehre wrote:
> xazax.hun wrote:
> > I actually was a bit lazy when I added these tests. Both `value` and 
> > `operator*` is overloaded on `&&`. But if you do not feel like adjusting 
> > the tests this is fine, I can do it myself later :)
> I'll change it to use the `&` variant in the test - the `&&` cannot dangle as 
> far as I understand.
It can!

Consider the following code:

```
int & = *std::optional(5);
// r dangles here.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66164



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

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

[Matthias Gehre via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:172
   T *();
+  T ();
+};

xazax.hun wrote:
> I actually was a bit lazy when I added these tests. Both `value` and 
> `operator*` is overloaded on `&&`. But if you do not feel like adjusting the 
> tests this is fine, I can do it myself later :)
I'll change it to use the `&` variant in the test - the `&&` cannot dangle as 
far as I understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66164



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

[PATCH] D66173: [Codegen] Updated test for D66158

[Johannes Doerfert via Phabricator via cfe-commits]

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

In D66173#1628079 , @xbolva00 wrote:

> hm.. memset?


When I say memset, here and in the other review, I mean memmove.

In D66173#1628088 , @xbolva00 wrote:

> Explicit memcpy


thx. LGTM.


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

https://reviews.llvm.org/D66173



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

[PATCH] D66151: [clang-doc] Fix bitcode writer

[Julie Hockett via Phabricator via cfe-commits]

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

One comment then LGTM




Comment at: clang-tools-extra/clang-doc/Serialize.cpp:507
   Reference{ParentUSR, Parent->getNameAsString(), InfoType::IT_record};
-  Func.Access = D->getAccess();
+  Func.Access = D->getAccessUnsafe();
 

In a C++ method, it should have an access associated with it, and so AS_none 
isn't a valid value.


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

https://reviews.llvm.org/D66151



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Jian Cai via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1922
+  const bool Thumb = Opcode == ARM::tBL_PUSHLR;
+  MachineOperand ReturnAddr = MI.getOperand(0);
+  assert(ReturnAddr.getReg() == ARM::LR && "expect LR register!");

nickdesaulniers wrote:
> This could be `Register ReturnReg = MI.getOperand(0).getReg();` then the 
> below cleaned up. DRY
> 
> (and a few more opportunities in the return values of 
> `ARMTargetLowering::LowerINTRINSIC_VOID`)
> 
> With that change, LGTM, and thank you for the patch!
Thank you for all the comments! I have made changes accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D66176: Fix Driver/modules.cpp test to work when build directory name contains '.s'

[Tom Stellard via Phabricator via cfe-commits]

tstellar created this revision.
tstellar added reviewers: dyung, rsmith, hansw.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66176

Files:
  clang/test/Driver/modules.cpp


Index: clang/test/Driver/modules.cpp
===
--- clang/test/Driver/modules.cpp
+++ clang/test/Driver/modules.cpp
@@ -15,7 +15,8 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | 
FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// Check for extra space to avoid failures when the build directory contains 
'.s' in its path.
+// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}{{ }}
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 


Index: clang/test/Driver/modules.cpp
===
--- clang/test/Driver/modules.cpp
+++ clang/test/Driver/modules.cpp
@@ -15,7 +15,8 @@
 // RUN: %clang -std=c++2a %t/module.pcm -S -o %t/module.pcm.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-COMPILE
 //
 // CHECK-COMPILE: -cc1 {{.*}} {{-emit-obj|-S}}
-// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}
+// Check for extra space to avoid failures when the build directory contains '.s' in its path.
+// CHECK-COMPILE-SAME: -o {{.*}}.{{pcm.o|s}}{{ }}
 // CHECK-COMPILE-SAME: -x pcm
 // CHECK-COMPILE-SAME: {{.*}}.pcm
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 214945.
jcai19 marked an inline comment as done.
jcai19 added a comment.

Updates based on comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019

Files:
  clang/lib/Basic/Targets/ARM.cpp
  llvm/include/llvm/IR/IntrinsicsARM.td
  llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMISelLowering.h
  llvm/lib/Target/ARM/ARMInstrInfo.td
  llvm/lib/Target/ARM/ARMInstrThumb.td
  llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
  llvm/test/CodeGen/ARM/gnu_mcount_nc.ll

Index: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/gnu_mcount_nc.ll
@@ -0,0 +1,41 @@
+; RUN: llc -mtriple=armv7a-linux-gnueabihf -verify-machineinstrs %s -o - | FileCheck %s --check-prefix=CHECK-ARM
+; RUN: llc -mtriple=armv7a-linux-gnueabihf -verify-machineinstrs -fast-isel %s -o - | FileCheck %s --check-prefix=CHECK-ARM-FAST-ISEL
+; RUN: llc -mtriple=armv7a-linux-gnueabihf -verify-machineinstrs -global-isel -global-isel-abort=2 %s -o - | FileCheck %s --check-prefix=CHECK-ARM-GLOBAL-ISEL
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf -verify-machineinstrs %s -o - | FileCheck %s --check-prefix=CHECK-THUMB
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf -verify-machineinstrs -fast-isel %s -o - | FileCheck %s --check-prefix=CHECK-THUMB-FAST-ISEL
+; RUN: llc -mtriple=thumbv7a-linux-gnueabihf -verify-machineinstrs -global-isel -global-isel-abort=2 %s -o - | FileCheck %s --check-prefix=CHECK-THUMB-GLOBAL-ISEL
+
+define dso_local void @callee() #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-ARM-FAST-ISEL:  stmdb   sp!, {lr}
+; CHECK-ARM-FAST-ISEL-NEXT: bl  __gnu_mcount_nc
+; CHECK-ARM-GLOBAL-ISEL:stmdb   sp!, {lr}
+; CHECK-ARM-GLOBAL-ISEL-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+; CHECK-THUMB-FAST-ISEL:push{lr}
+; CHECK-THUMB-FAST-ISEL-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB-GLOBAL-ISEL:  push{lr}
+; CHECK-THUMB-GLOBAL-ISEL-NEXT: bl  __gnu_mcount_nc
+  ret void
+}
+
+define dso_local void @caller() #0 {
+; CHECK-ARM:stmdb   sp!, {lr}
+; CHECK-ARM-NEXT:   bl  __gnu_mcount_nc
+; CHECK-ARM-FAST-ISEL:  stmdb   sp!, {lr}
+; CHECK-ARM-FAST-ISEL-NEXT: bl  __gnu_mcount_nc
+; CHECK-ARM-GLOBAL-ISEL:stmdb   sp!, {lr}
+; CHECK-ARM-GLOBAL-ISEL-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB:  push{lr}
+; CHECK-THUMB-NEXT: bl  __gnu_mcount_nc
+; CHECK-THUMB-FAST-ISEL:push{lr}
+; CHECK-THUMB-FAST-ISEL-NEXT:   bl  __gnu_mcount_nc
+; CHECK-THUMB-GLOBAL-ISEL:  push{lr}
+; CHECK-THUMB-GLOBAL-ISEL-NEXT: bl  __gnu_mcount_nc
+  call void @callee()
+  ret void
+}
+
+attributes #0 = { nofree nounwind "instrument-function-entry-inlined"="llvm.arm.gnu.eabi.mcount" }
Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -24,7 +24,7 @@
 
   if (Func == "mcount" ||
   Func == ".mcount" ||
-  Func == "\01__gnu_mcount_nc" ||
+  Func == "llvm.arm.gnu.eabi.mcount" ||
   Func == "\01_mcount" ||
   Func == "\01mcount" ||
   Func == "__mcount" ||
Index: llvm/lib/Target/ARM/ARMInstrThumb.td
===
--- llvm/lib/Target/ARM/ARMInstrThumb.td
+++ llvm/lib/Target/ARM/ARMInstrThumb.td
@@ -565,6 +565,13 @@
   4, IIC_Br,
   [(ARMcall_nolink tGPR:$func)]>,
 Requires<[IsThumb, IsThumb1Only]>, Sched<[WriteBr]>;
+
+  // Also used for Thumb2
+  // push lr before the call
+  def tBL_PUSHLR : tPseudoInst<(outs), (ins GPRlr:$ra, pred:$p, thumb_bl_target:$func),
+  4, IIC_Br,
+  []>,
+ Requires<[IsThumb]>, Sched<[WriteBr]>;
 }
 
 let isBranch = 1, isTerminator = 1, isBarrier = 1 in {
Index: llvm/lib/Target/ARM/ARMInstrInfo.td
===
--- llvm/lib/Target/ARM/ARMInstrInfo.td
+++ llvm/lib/Target/ARM/ARMInstrInfo.td
@@ -2350,6 +2350,12 @@
   def BMOVPCB_CALL : ARMPseudoInst<(outs), (ins arm_bl_target:$func),
8, IIC_Br, [(ARMcall_nolink tglobaladdr:$func)]>,
   Requires<[IsARM]>, Sched<[WriteBr]>;
+
+  // push lr before the call
+  def BL_PUSHLR : ARMPseudoInst<(outs), (ins GPRlr:$ra, arm_bl_target:$func),
+  4, IIC_Br,
+  []>,
+ Requires<[IsARM]>, Sched<[WriteBr]>;
 }
 
 let 

[PATCH] D66173: [Codegen] Updated test for D66158

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

xbolva00 updated this revision to Diff 214940.
xbolva00 added a comment.

Explicit memcpy


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

https://reviews.llvm.org/D66173

Files:
  test/CodeGen/struct-copy.c


Index: test/CodeGen/struct-copy.c
===
--- test/CodeGen/struct-copy.c
+++ test/CodeGen/struct-copy.c
@@ -1,7 +1,39 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep 'call.*llvm.memcpy'
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
 struct x { int a[100]; };
 
-
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[P_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:[[Q_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:store %struct.x* [[P:%.*]], %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:store %struct.x* [[Q:%.*]], %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load %struct.x*, %struct.x** [[P_ADDR]], 
align 8
+// CHECK-NEXT:[[TMP1:%.*]] = load %struct.x*, %struct.x** [[Q_ADDR]], 
align 8
+// CHECK-NEXT:[[TMP2:%.*]] = bitcast %struct.x* [[TMP0]] to i8*
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast %struct.x* [[TMP1]] to i8*
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 [[TMP2]], 
i8* align 4 [[TMP3]], i64 400, i1 false)
+// CHECK-NEXT:ret void
+//
 void foo(struct x *P, struct x *Q) {
   *P = *Q;
 }
+
+// CHECK-LABEL: @bar(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[P_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:[[Q_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:store %struct.x* [[P:%.*]], %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:store %struct.x* [[Q:%.*]], %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load %struct.x*, %struct.x** [[P_ADDR]], 
align 8
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast %struct.x* [[TMP0]] to i8*
+// CHECK-NEXT:[[TMP2:%.*]] = load %struct.x*, %struct.x** [[Q_ADDR]], 
align 8
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast %struct.x* [[TMP2]] to i8*
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 [[TMP1]], 
i8* align 4 [[TMP3]], i64 400, i1 false)
+// CHECK-NEXT:ret void
+//
+void bar(struct x *P, struct x *Q) {
+  __builtin_memcpy(P, Q, sizeof(struct x));
+}
+
+// CHECK: declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture 
writeonly, i8* noalias nocapture readonly, i64, i1 immarg)


Index: test/CodeGen/struct-copy.c
===
--- test/CodeGen/struct-copy.c
+++ test/CodeGen/struct-copy.c
@@ -1,7 +1,39 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep 'call.*llvm.memcpy'
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
 struct x { int a[100]; };
 
-
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[P_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:[[Q_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:store %struct.x* [[P:%.*]], %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:store %struct.x* [[Q:%.*]], %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load %struct.x*, %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = load %struct.x*, %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP2:%.*]] = bitcast %struct.x* [[TMP0]] to i8*
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast %struct.x* [[TMP1]] to i8*
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 [[TMP2]], i8* align 4 [[TMP3]], i64 400, i1 false)
+// CHECK-NEXT:ret void
+//
 void foo(struct x *P, struct x *Q) {
   *P = *Q;
 }
+
+// CHECK-LABEL: @bar(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[P_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:[[Q_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:store %struct.x* [[P:%.*]], %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:store %struct.x* [[Q:%.*]], %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load %struct.x*, %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast %struct.x* [[TMP0]] to i8*
+// CHECK-NEXT:[[TMP2:%.*]] = load %struct.x*, %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast %struct.x* [[TMP2]] to i8*
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 [[TMP1]], i8* align 4 [[TMP3]], i64 400, i1 false)
+// CHECK-NEXT:ret void
+//
+void bar(struct x *P, struct x *Q) {
+  __builtin_memcpy(P, Q, sizeof(struct x));
+}
+
+// CHECK: declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66173: [Codegen] Updated test for D66158

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

xbolva00 added a comment.

hm.. memset?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66173



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

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:6583
 .Cases("end", "rend", "cend", "crend", true)
-.Cases("c_str", "data", "get", true)
+.Cases("c_str", "data", "get", "value", true)
 // Map and set types.

Oh, this one needs to be updated, as `value` returns a reference not a pointer. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66164



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

[PATCH] D65578: [analyzer][NFC] Make sure that the BugReport is not modified during the construction of non-visitor pieces

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368755: [analyzer][NFC] Make sure that the BugReport is not 
modified during the… (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65578?vs=212811=214937#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65578

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -82,7 +82,7 @@
   /// Generates the default final diagnostic piece.
   static PathDiagnosticPieceRef getDefaultEndPath(const BugReporterContext ,
   const ExplodedNode *N,
-  BugReport );
+  const BugReport );
 };
 
 /// Finds last store into the given region,
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -105,6 +105,7 @@
 
   const ExplodedNode *ErrorNode = nullptr;
   SmallVector Ranges;
+  const SourceRange ErrorNodeRange;
   ExtraTextList ExtraText;
   NoteList Notes;
 
@@ -155,16 +156,22 @@
   llvm::SmallSet TrackedConditions;
 
 public:
-  BugReport(const BugType& bt, StringRef desc, const ExplodedNode *errornode)
-  : BT(bt), Description(desc), ErrorNode(errornode) {}
+  BugReport(const BugType , StringRef desc, const ExplodedNode *errornode)
+  : BT(bt), Description(desc), ErrorNode(errornode),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
-  BugReport(const BugType& bt, StringRef shortDesc, StringRef desc,
+  BugReport(const BugType , StringRef shortDesc, StringRef desc,
 const ExplodedNode *errornode)
   : BT(bt), ShortDescription(shortDesc), Description(desc),
-ErrorNode(errornode) {}
+ErrorNode(errornode),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
   BugReport(const BugType , StringRef desc, PathDiagnosticLocation l)
-  : BT(bt), Description(desc), Location(l) {}
+  : BT(bt), Description(desc), Location(l),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
   /// Create a BugReport with a custom uniqueing location.
   ///
@@ -323,7 +330,7 @@
   }
 
   /// Get the SourceRanges associated with the report.
-  virtual llvm::iterator_range getRanges();
+  virtual llvm::iterator_range getRanges() const;
 
   /// Add custom or predefined bug report visitors to this report.
   ///
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -267,7 +267,7 @@
 PathDiagnosticPieceRef
 BugReporterVisitor::getDefaultEndPath(const BugReporterContext ,
   const ExplodedNode *EndPathNode,
-  BugReport ) {
+  const BugReport ) {
   PathDiagnosticLocation L = PathDiagnosticLocation::createEndOfPath(
   EndPathNode, BRC.getSourceManager());
 
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -192,13 +192,17 @@
 };
 
 /// Contains every contextual information needed for constructing a
-/// PathDiagnostic object for a given bug report. This class (and aside from
-/// some caching BugReport does in the background) and its fields are immutable,
-/// and passes a BugReportConstruct object around during the construction.
+/// PathDiagnostic object for a given bug report. This class and its fields are
+/// immutable, and passes a BugReportConstruct object around during the
+/// construction.
 class PathDiagnosticBuilder : public 

r368755 - [analyzer][NFC] Make sure that the BugReport is not modified during the construction of non-visitor pieces

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Aug 13 15:03:08 2019
New Revision: 368755

URL: http://llvm.org/viewvc/llvm-project?rev=368755=rev
Log:
[analyzer][NFC] Make sure that the BugReport is not modified during the 
construction of non-visitor pieces

I feel this is kinda important, because in a followup patch I'm adding different
kinds of interestingness, and propagating the correct kind in BugReporter.cpp is
just one less thing to worry about.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=368755=368754=368755=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Tue 
Aug 13 15:03:08 2019
@@ -105,6 +105,7 @@ protected:
 
   const ExplodedNode *ErrorNode = nullptr;
   SmallVector Ranges;
+  const SourceRange ErrorNodeRange;
   ExtraTextList ExtraText;
   NoteList Notes;
 
@@ -155,16 +156,22 @@ protected:
   llvm::SmallSet TrackedConditions;
 
 public:
-  BugReport(const BugType& bt, StringRef desc, const ExplodedNode *errornode)
-  : BT(bt), Description(desc), ErrorNode(errornode) {}
+  BugReport(const BugType , StringRef desc, const ExplodedNode *errornode)
+  : BT(bt), Description(desc), ErrorNode(errornode),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
-  BugReport(const BugType& bt, StringRef shortDesc, StringRef desc,
+  BugReport(const BugType , StringRef shortDesc, StringRef desc,
 const ExplodedNode *errornode)
   : BT(bt), ShortDescription(shortDesc), Description(desc),
-ErrorNode(errornode) {}
+ErrorNode(errornode),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
   BugReport(const BugType , StringRef desc, PathDiagnosticLocation l)
-  : BT(bt), Description(desc), Location(l) {}
+  : BT(bt), Description(desc), Location(l),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
   /// Create a BugReport with a custom uniqueing location.
   ///
@@ -323,7 +330,7 @@ public:
   }
 
   /// Get the SourceRanges associated with the report.
-  virtual llvm::iterator_range getRanges();
+  virtual llvm::iterator_range getRanges() const;
 
   /// Add custom or predefined bug report visitors to this report.
   ///

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=368755=368754=368755=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
Tue Aug 13 15:03:08 2019
@@ -82,7 +82,7 @@ public:
   /// Generates the default final diagnostic piece.
   static PathDiagnosticPieceRef getDefaultEndPath(const BugReporterContext 
,
   const ExplodedNode *N,
-  BugReport );
+  const BugReport );
 };
 
 /// Finds last store into the given region,

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h?rev=368755=368754=368755=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
 Tue Aug 13 15:03:08 2019
@@ -67,7 +67,7 @@ public:
   ExplodedNode *n, SymbolRef sym,
   StringRef endText);
 
-  llvm::iterator_range getRanges() override {
+  llvm::iterator_range getRanges() const override {
 if (!isLeak)
   return BugReport::getRanges();
 return llvm::make_range(ranges_iterator(), ranges_iterator());

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 

r368754 - Add a missing header comment, NFC

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Aug 13 15:01:39 2019
New Revision: 368754

URL: http://llvm.org/viewvc/llvm-project?rev=368754=rev
Log:
Add a missing header comment, NFC

Modified:
cfe/trunk/lib/AST/FormatStringParsing.h

Modified: cfe/trunk/lib/AST/FormatStringParsing.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/FormatStringParsing.h?rev=368754=368753=368754=diff
==
--- cfe/trunk/lib/AST/FormatStringParsing.h (original)
+++ cfe/trunk/lib/AST/FormatStringParsing.h Tue Aug 13 15:01:39 2019
@@ -1,3 +1,16 @@
+//===- FormatStringParsing.h - Format String Parsing *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This provides some shared functions between printf and scanf format string
+// parsing code.
+//
+//===--===//
+
 #ifndef LLVM_CLANG_LIB_ANALYSIS_FORMATSTRINGPARSING_H
 #define LLVM_CLANG_LIB_ANALYSIS_FORMATSTRINGPARSING_H
 


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

[PATCH] D66058: [NFC][clang] Move much of the argument handling code from Driver::BuildActions to Driver::handleArguments.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Ugh, this is still not the most structured handling of the arguments.  But, 
yeah, this seems like it should be equivalent.  Fine by me if @aaron.ballman 
has no more comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66058



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

[PATCH] D66156: Removed dead code from clang/tools/libclang/CXIndexDataConsumer.{cpp,h}

[Jan Korous via Phabricator via cfe-commits]

jkorous added a reviewer: akyrtzi.
jkorous added a subscriber: akyrtzi.
jkorous added a comment.

LGTM but let's wait for @akyrtzi to confirm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66156



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

[PATCH] D66173: [Codegen] Updated test for D66158

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Might not be the best test given that this should arguably be a memset not a 
memcpy.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66173



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

[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I agree with just special-casing this for now.




Comment at: clang/lib/Sema/SemaChecking.cpp:6800
+MD->getSelector() ==
+S.NSAPIObj->getLocalizedStringForKeySelector()) {
+  IgnoreStringsWithoutSpecifiers = true;

It's more efficient to do these checks with `isStr` instead of looking up an 
`IdentifierInfo`.  You can add a 
`Selector::isKeywordSelector(ArrayRef)` method to make that equally 
convenient for selectors.


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

https://reviews.llvm.org/D27165



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

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I'm convinced, let's keep everything as is but turn this into an 
`-analyzer-config` flag. We generally wanted to reduce the number of frontend 
flags because they are more taxing on backwards compatibility, so it makes 
perfect sense.


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

https://reviews.llvm.org/D66042



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

[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Tests?




Comment at: clang/lib/Driver/Driver.cpp:3420
 
+  // Add a link action if necessary.
+  if (!MergerInputs.empty()) {

This isn't really a link action ...



Comment at: clang/lib/Driver/Driver.cpp:3423
+Action *LA = C.MakeAction(MergerInputs, 
types::TY_Image);
+LA = OffloadBuilder.processHostLinkAction(LA);
+Actions.push_back(LA);

I don't understand why the offload bundler is part of the interface merge phase.



Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:11
+#include "CommonArgs.h"
+#include "clang/Config/config.h" // for GCC_INSTALL_PREFIX
+#include "clang/Driver/Compilation.h"

Why do you need `GCC_INSTALL_PREFIX`?  GCC doesn't have interface stubs yet 
AFAIK.



Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:34
+  const char *LinkingOutput) const {
+  llvm::errs() << "Construct merger job\n";
+  SmallVector CmdArgs;

debugging left overs?



Comment at: clang/lib/Driver/ToolChains/InterfaceStubs.cpp:36
+  SmallVector CmdArgs;
+  const char *Exec = "/bin/echo";
+  // CmdArgs.push_back("-o");

Hmm, this seems wrong :-).  You should search for it relative to the toolchain 
and then fallback.



Comment at: clang/lib/Driver/Types.cpp:328
+IfsModePhaseList, std::back_inserter(P), [](phases::ID Phase) {
+  return Phase <= ((DAL.getLastArg(options::OPT_c)) ? phases::Compile
+: 
phases::IfsMerge);

How does `-c` `-emit-interface-stubs` and multiple input play together?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63978



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

r368752 - [analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of interestingness propagation

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Aug 13 14:48:17 2019
New Revision: 368752

URL: http://llvm.org/viewvc/llvm-project?rev=368752=rev
Log:
[analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of 
interestingness propagation

Apparently this does literally nothing.

When you think about this, it makes sense. If something is really important,
we're tracking it anyways, and that system is sophisticated enough to mark
actually interesting statements as such. I wouldn't say that it's even likely
that subexpressions are also interesting (array[10 - x + x]), so I guess even
if this produced any effects, its probably undesirable.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=368752=368751=368752=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Tue Aug 13 14:48:17 2019
@@ -99,8 +99,6 @@ using CallWithEntryStack = SmallVector>;
 
-using InterestingExprs = llvm::DenseSet;
-
 /// A map from PathDiagnosticPiece to the LocationContext of the inlined
 /// function call it represents.
 using LocationContextMap =
@@ -126,7 +124,6 @@ public:
   /// TODO: PathDiagnostic has a stack doing the same thing, shouldn't we use
   /// that instead?
   CallWithEntryStack CallStack;
-  InterestingExprs IE;
   /// The bug report we're constructing. For ease of use, this field is kept
   /// public, though some "shortcut" getters are provided for commonly used
   /// methods of PathDiagnostic.
@@ -943,74 +940,6 @@ void PathDiagnosticBuilder::generateMini
   }
 }
 
-// Cone-of-influence: support the reverse propagation of "interesting" symbols
-// and values by tracing interesting calculations backwards through evaluated
-// expressions along a path.  This is probably overly complicated, but the idea
-// is that if an expression computed an "interesting" value, the child
-// expressions are also likely to be "interesting" as well (which then
-// propagates to the values they in turn compute).  This reverse propagation
-// is needed to track interesting correlations across function call boundaries,
-// where formal arguments bind to actual arguments, etc.  This is also needed
-// because the constraint solver sometimes simplifies certain symbolic values
-// into constants when appropriate, and this complicates reasoning about
-// interesting values.
-
-static void reversePropagateIntererstingSymbols(BugReport ,
-InterestingExprs ,
-const ProgramState *State,
-const Expr *Ex,
-const LocationContext *LCtx) {
-  SVal V = State->getSVal(Ex, LCtx);
-  if (!(R.isInteresting(V) || IE.count(Ex)))
-return;
-
-  switch (Ex->getStmtClass()) {
-default:
-  if (!isa(Ex))
-break;
-  LLVM_FALLTHROUGH;
-case Stmt::BinaryOperatorClass:
-case Stmt::UnaryOperatorClass: {
-  for (const Stmt *SubStmt : Ex->children()) {
-if (const auto *child = dyn_cast_or_null(SubStmt)) {
-  IE.insert(child);
-  SVal ChildV = State->getSVal(child, LCtx);
-  R.markInteresting(ChildV);
-}
-  }
-  break;
-}
-  }
-
-  R.markInteresting(V);
-}
-
-static void reversePropagateInterestingSymbols(BugReport ,
-   InterestingExprs ,
-   const ProgramState *State,
-   const LocationContext 
*CalleeCtx)
-{
-  // FIXME: Handle non-CallExpr-based CallEvents.
-  const StackFrameContext *Callee = CalleeCtx->getStackFrame();
-  const Stmt *CallSite = Callee->getCallSite();
-  if (const auto *CE = dyn_cast_or_null(CallSite)) {
-if (const auto *FD = dyn_cast(CalleeCtx->getDecl())) {
-  FunctionDecl::param_const_iterator PI = FD->param_begin(),
- PE = FD->param_end();
-  CallExpr::const_arg_iterator AI = CE->arg_begin(), AE = CE->arg_end();
-  for (; AI != AE && PI != PE; ++AI, ++PI) {
-if (const Expr *ArgE = *AI) {
-  if (const ParmVarDecl *PD = *PI) {
-Loc LV = State->getLValue(PD, CalleeCtx);
-if (R.isInteresting(LV) || R.isInteresting(State->getRawSVal(LV)))
-  IE.insert(ArgE);
-  }
-}
-  }
-}
-  }
-}
-
 
//===--===//
 // Functions for determining if a loop was executed 0 times.
 
//===--===//
@@ -1219,13 +1148,6 @@ void 

[PATCH] D65487: [analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of interestingness propagation

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368752: [analyzer][NFC] Refactoring BugReporter.cpp P6.: 
Completely get rid of… (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65487?vs=212459=214931#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65487

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -99,8 +99,6 @@
 using VisitorsDiagnosticsTy =
 llvm::DenseMap>;
 
-using InterestingExprs = llvm::DenseSet;
-
 /// A map from PathDiagnosticPiece to the LocationContext of the inlined
 /// function call it represents.
 using LocationContextMap =
@@ -126,7 +124,6 @@
   /// TODO: PathDiagnostic has a stack doing the same thing, shouldn't we use
   /// that instead?
   CallWithEntryStack CallStack;
-  InterestingExprs IE;
   /// The bug report we're constructing. For ease of use, this field is kept
   /// public, though some "shortcut" getters are provided for commonly used
   /// methods of PathDiagnostic.
@@ -943,74 +940,6 @@
   }
 }
 
-// Cone-of-influence: support the reverse propagation of "interesting" symbols
-// and values by tracing interesting calculations backwards through evaluated
-// expressions along a path.  This is probably overly complicated, but the idea
-// is that if an expression computed an "interesting" value, the child
-// expressions are also likely to be "interesting" as well (which then
-// propagates to the values they in turn compute).  This reverse propagation
-// is needed to track interesting correlations across function call boundaries,
-// where formal arguments bind to actual arguments, etc.  This is also needed
-// because the constraint solver sometimes simplifies certain symbolic values
-// into constants when appropriate, and this complicates reasoning about
-// interesting values.
-
-static void reversePropagateIntererstingSymbols(BugReport ,
-InterestingExprs ,
-const ProgramState *State,
-const Expr *Ex,
-const LocationContext *LCtx) {
-  SVal V = State->getSVal(Ex, LCtx);
-  if (!(R.isInteresting(V) || IE.count(Ex)))
-return;
-
-  switch (Ex->getStmtClass()) {
-default:
-  if (!isa(Ex))
-break;
-  LLVM_FALLTHROUGH;
-case Stmt::BinaryOperatorClass:
-case Stmt::UnaryOperatorClass: {
-  for (const Stmt *SubStmt : Ex->children()) {
-if (const auto *child = dyn_cast_or_null(SubStmt)) {
-  IE.insert(child);
-  SVal ChildV = State->getSVal(child, LCtx);
-  R.markInteresting(ChildV);
-}
-  }
-  break;
-}
-  }
-
-  R.markInteresting(V);
-}
-
-static void reversePropagateInterestingSymbols(BugReport ,
-   InterestingExprs ,
-   const ProgramState *State,
-   const LocationContext *CalleeCtx)
-{
-  // FIXME: Handle non-CallExpr-based CallEvents.
-  const StackFrameContext *Callee = CalleeCtx->getStackFrame();
-  const Stmt *CallSite = Callee->getCallSite();
-  if (const auto *CE = dyn_cast_or_null(CallSite)) {
-if (const auto *FD = dyn_cast(CalleeCtx->getDecl())) {
-  FunctionDecl::param_const_iterator PI = FD->param_begin(),
- PE = FD->param_end();
-  CallExpr::const_arg_iterator AI = CE->arg_begin(), AE = CE->arg_end();
-  for (; AI != AE && PI != PE; ++AI, ++PI) {
-if (const Expr *ArgE = *AI) {
-  if (const ParmVarDecl *PD = *PI) {
-Loc LV = State->getLValue(PD, CalleeCtx);
-if (R.isInteresting(LV) || R.isInteresting(State->getRawSVal(LV)))
-  IE.insert(ArgE);
-  }
-}
-  }
-}
-  }
-}
-
 //===--===//
 // Functions for determining if a loop was executed 0 times.
 //===--===//
@@ -1219,13 +1148,6 @@
 C.updateLocCtxMap(>path, CE->getCalleeContext());
 
 if (C.shouldAddPathEdges()) {
-  const Stmt *S = CE->getCalleeContext()->getCallSite();
-  // Propagate the interesting symbols accordingly.
-  if (const auto *Ex = dyn_cast_or_null(S)) {
-reversePropagateIntererstingSymbols(
-*getBugReport(), C.IE, C.getCurrentNode()->getState().get(), Ex,
-C.getCurrLocationContext());
-  }
   

[PATCH] D66173: [Codegen] Updated test for D66158

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

xbolva00 created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D66173

Files:
  test/CodeGen/struct-copy.c


Index: test/CodeGen/struct-copy.c
===
--- test/CodeGen/struct-copy.c
+++ test/CodeGen/struct-copy.c
@@ -1,7 +1,21 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep 'call.*llvm.memcpy'
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
 struct x { int a[100]; };
 
-
 void foo(struct x *P, struct x *Q) {
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[P_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:[[Q_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:store %struct.x* [[P:%.*]], %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:store %struct.x* [[Q:%.*]], %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load %struct.x*, %struct.x** [[P_ADDR]], 
align 8
+// CHECK-NEXT:[[TMP1:%.*]] = load %struct.x*, %struct.x** [[Q_ADDR]], 
align 8
+// CHECK-NEXT:[[TMP2:%.*]] = bitcast %struct.x* [[TMP0]] to i8*
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast %struct.x* [[TMP1]] to i8*
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 [[TMP2]], 
i8* align 4 [[TMP3]], i64 400, i1 false)
+// CHECK-NEXT:ret void
+//
   *P = *Q;
 }
+
+// CHECK: declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture 
writeonly, i8* noalias nocapture readonly, i64, i1 immarg)


Index: test/CodeGen/struct-copy.c
===
--- test/CodeGen/struct-copy.c
+++ test/CodeGen/struct-copy.c
@@ -1,7 +1,21 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep 'call.*llvm.memcpy'
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
 struct x { int a[100]; };
 
-
 void foo(struct x *P, struct x *Q) {
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[P_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:[[Q_ADDR:%.*]] = alloca %struct.x*, align 8
+// CHECK-NEXT:store %struct.x* [[P:%.*]], %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:store %struct.x* [[Q:%.*]], %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load %struct.x*, %struct.x** [[P_ADDR]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = load %struct.x*, %struct.x** [[Q_ADDR]], align 8
+// CHECK-NEXT:[[TMP2:%.*]] = bitcast %struct.x* [[TMP0]] to i8*
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast %struct.x* [[TMP1]] to i8*
+// CHECK-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 [[TMP2]], i8* align 4 [[TMP3]], i64 400, i1 false)
+// CHECK-NEXT:ret void
+//
   *P = *Q;
 }
+
+// CHECK: declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66168: [WebAssembly] Make clang emit correct va_arg code for structs

[Guanzhong Chen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368750: [WebAssembly] Make clang emit correct va_arg code 
for structs (authored by quantum, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66168?vs=214916=214927#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66168

Files:
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/test/CodeGen/wasm-varargs.c


Index: cfe/trunk/test/CodeGen/wasm-varargs.c
===
--- cfe/trunk/test/CodeGen/wasm-varargs.c
+++ cfe/trunk/test/CodeGen/wasm-varargs.c
@@ -87,12 +87,13 @@
 // CHECK:   [[VA1:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_start(i8* [[VA1]])
 // CHECK:   [[ARGP_CUR:%[^,=]+]] = load i8*, i8** [[VA]], align 4
-// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* 
[[ARGP_CUR]], i32 12
+// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* 
[[ARGP_CUR]], i32 4
 // CHECK:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
-// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]*
-// CHECK:   [[R4:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
-// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R3]] to i8*
-// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R4]], i8* align 
4 [[R5]], i32 12, i1 false)
+// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]**
+// CHECK:   [[R4:%[^,=]+]] = load [[STRUCT_S]]*, [[STRUCT_S]]** %0, align 4
+// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
+// CHECK:   [[R6:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R4]] to i8*
+// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R5]], i8* align 
4 [[R6]], i32 12, i1 false)
 // CHECK:   [[VA2:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_end(i8* [[VA2]])
 // CHECK:   ret void
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -833,10 +833,12 @@
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction , Address VAListAddr,
   QualType Ty) const {
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*IsIndirect=*/ false,
+  bool IsIndirect =
+  isAggregateTypeForABI(Ty) && !isSingleElementStruct(Ty, getContext());
+  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,
   getContext().getTypeInfoInChars(Ty),
   CharUnits::fromQuantity(4),
-  /*AllowHigherAlign=*/ true);
+  /*AllowHigherAlign=*/true);
 }
 
 
//===--===//


Index: cfe/trunk/test/CodeGen/wasm-varargs.c
===
--- cfe/trunk/test/CodeGen/wasm-varargs.c
+++ cfe/trunk/test/CodeGen/wasm-varargs.c
@@ -87,12 +87,13 @@
 // CHECK:   [[VA1:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_start(i8* [[VA1]])
 // CHECK:   [[ARGP_CUR:%[^,=]+]] = load i8*, i8** [[VA]], align 4
-// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* [[ARGP_CUR]], i32 12
+// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* [[ARGP_CUR]], i32 4
 // CHECK:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
-// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]*
-// CHECK:   [[R4:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
-// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R3]] to i8*
-// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R4]], i8* align 4 [[R5]], i32 12, i1 false)
+// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]**
+// CHECK:   [[R4:%[^,=]+]] = load [[STRUCT_S]]*, [[STRUCT_S]]** %0, align 4
+// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
+// CHECK:   [[R6:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R4]] to i8*
+// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R5]], i8* align 4 [[R6]], i32 12, i1 false)
 // CHECK:   [[VA2:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_end(i8* [[VA2]])
 // CHECK:   ret void
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -833,10 +833,12 @@
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction , Address VAListAddr,
   QualType Ty) const {
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*IsIndirect=*/ false,
+  bool IsIndirect =
+  isAggregateTypeForABI(Ty) && !isSingleElementStruct(Ty, getContext());
+  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, 

[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 214925.
erik.pilkington added a comment.

Special case localizedStringForKey instead of using an attribute.


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

https://reviews.llvm.org/D27165

Files:
  clang/include/clang/AST/FormatString.h
  clang/include/clang/AST/NSAPI.h
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaObjC/format-strings-objc.m

Index: clang/test/SemaObjC/format-strings-objc.m
===
--- clang/test/SemaObjC/format-strings-objc.m
+++ clang/test/SemaObjC/format-strings-objc.m
@@ -25,7 +25,11 @@
 @protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end
 @interface NSObject  {} @end
 typedef float CGFloat;
-@interface NSString : NSObject - (NSUInteger)length; @end
+@interface NSString : NSObject 
+- (NSUInteger)length;
++(instancetype)stringWithFormat:(NSString *)fmt, ...
+__attribute__((format(__NSString__, 1, 2)));
+@end
 @interface NSSimpleCString : NSString {} @end
 @interface NSConstantString : NSSimpleCString @end
 extern void *_NSConstantStringClassReference;
@@ -302,3 +306,39 @@
 }
 
 @end
+
+@interface NSBundle : NSObject
+- (NSString *)localizedStringForKey:(NSString *)key
+  value:(nullable NSString *)value
+  table:(nullable NSString *)tableName
+ __attribute__((format_arg(1)));
+
+- (NSString *)someRandomMethod:(NSString *)key
+ value:(nullable NSString *)value
+ table:(nullable NSString *)tableName
+__attribute__((format_arg(1)));
+@end
+
+void useLocalizedStringForKey(NSBundle *bndl) {
+  [NSString stringWithFormat:
+  [bndl localizedStringForKey:@"%d" // expected-warning{{more '%' conversions than data arguments}}
+  value:0
+  table:0]];
+  // No warning, @"flerp" doesn't have a format specifier.
+  [NSString stringWithFormat: [bndl localizedStringForKey:@"flerp" value:0 table:0], 43, @"flarp"];
+
+  [NSString stringWithFormat:
+  [bndl localizedStringForKey:@"%f"
+value:0
+table:0], 42]; // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+
+  [NSString stringWithFormat:
+  [bndl someRandomMethod:@"%f"
+   value:0
+   table:0], 42]; // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+
+  [NSString stringWithFormat:
+  [bndl someRandomMethod:@"flerp"
+   value:0
+   table:0], 42]; // expected-warning{{data argument not used by format string}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6575,7 +6575,8 @@
   bool inFunctionCall,
   Sema::VariadicCallType CallType,
   llvm::SmallBitVector ,
-  UncoveredArgHandler );
+  UncoveredArgHandler ,
+  bool IgnoreStringsWithoutSpecifiers);
 
 // Determine if an expression is a string literal or constant string.
 // If this function returns false on the arguments to a function expecting a
@@ -6588,7 +6589,8 @@
   Sema::VariadicCallType CallType, bool InFunctionCall,
   llvm::SmallBitVector ,
   UncoveredArgHandler ,
-  llvm::APSInt Offset) {
+  llvm::APSInt Offset,
+  bool IgnoreStringsWithoutSpecifiers = false) {
   if (S.isConstantEvaluated())
 return SLCT_NotALiteral;
  tryAgain:
@@ -6639,17 +6641,17 @@
   Left = checkFormatStringExpr(S, C->getTrueExpr(), Args,
HasVAListArg, format_idx, firstDataArg,
Type, CallType, InFunctionCall,
-   CheckedVarArgs, UncoveredArg, Offset);
+   CheckedVarArgs, UncoveredArg, Offset,
+   IgnoreStringsWithoutSpecifiers);
   if (Left == SLCT_NotALiteral || !CheckRight) {
 return Left;
   }
 }
 
-StringLiteralCheckType Right =
-checkFormatStringExpr(S, C->getFalseExpr(), Args,
-  HasVAListArg, format_idx, firstDataArg,
-  Type, CallType, InFunctionCall, CheckedVarArgs,
-  UncoveredArg, Offset);
+StringLiteralCheckType Right = checkFormatStringExpr(
+S, C->getFalseExpr(), Args, HasVAListArg, format_idx, firstDataArg,
+

[PATCH] D63978: Clang Interface Stubs merger plumbing for Driver

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 214924.
plotfi edited the summary of this revision.
plotfi added a comment.
Herald added a subscriber: mgorny.

Updated. Much better cleaner implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63978

Files:
  clang/include/clang/Driver/Action.h
  clang/include/clang/Driver/Phases.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.def
  clang/lib/Driver/Action.cpp
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.h
  clang/lib/Driver/Types.cpp

Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -319,6 +319,17 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
+  // Treat Interface Stubs like its own compilation mode.
+  else if (DAL.getLastArg(options::OPT_emit_iterface_stubs)) {
+llvm::SmallVector IfsModePhaseList;
+types::getCompilationPhases(types::TY_IFS, IfsModePhaseList);
+llvm::copy_if(
+IfsModePhaseList, std::back_inserter(P), [](phases::ID Phase) {
+  return Phase <= ((DAL.getLastArg(options::OPT_c)) ? phases::Compile
+: phases::IfsMerge);
+});
+  }
+
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
@@ -327,7 +338,6 @@
DAL.getLastArg(options::OPT_rewrite_objc) ||
DAL.getLastArg(options::OPT_rewrite_legacy_objc) ||
DAL.getLastArg(options::OPT__migrate) ||
-   DAL.getLastArg(options::OPT_emit_iterface_stubs) ||
DAL.getLastArg(options::OPT__analyze, options::OPT__analyze_auto) ||
DAL.getLastArg(options::OPT_emit_ast))
 llvm::copy_if(PhaseList, std::back_inserter(P),
Index: clang/lib/Driver/ToolChains/InterfaceStubs.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/InterfaceStubs.h
@@ -0,0 +1,40 @@
+//===---  InterfaceStubs.cpp - Base InterfaceStubs Implementations C++  ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_IFS_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_IFS_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+#include 
+
+using namespace clang;
+using namespace llvm::opt;
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace ifstool {
+class LLVM_LIBRARY_VISIBILITY Merger : public Tool {
+public:
+  Merger(const ToolChain ) : Tool("IFS::Merger", "merger", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return false; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput) const override;
+};
+} // end namespace ifstool
+} // end namespace tools
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_IFS_H
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -0,0 +1,42 @@
+//===---  InterfaceStubs.cpp - Base InterfaceStubs Implementations C++  ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "InterfaceStubs.h"
+#include "CommonArgs.h"
+#include "clang/Config/config.h" // for GCC_INSTALL_PREFIX
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/DriverDiagnostic.h"
+#include "clang/Driver/Options.h"
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/TargetParser.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+using namespace clang::driver;
+using namespace clang;
+using namespace llvm::opt;
+
+void tools::ifstool::Merger::ConstructJob(Compilation , 

[PATCH] D27165: Add format_dynamic_key_arg attribute to improve "-Wformat" warnings for functions that load the formatting string dynamically based on a key value

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington commandeered this revision.
erik.pilkington added a reviewer: arphaman.
erik.pilkington added a comment.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: LLVM.

Stealing this (with Alex's permission). I think it makes more sense to add a 
special case for the method -[NSBundle localizedStringForKey:value:table:] 
rather then add a new general purpose attribute that'll only be used in one 
place.


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

https://reviews.llvm.org/D27165



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

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

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

Szelethus added a comment.

In D66042#1627842 , @Charusso wrote:

> Any analyzer config flag is equally accessible to anyone as the driver flags 
> as they are both flags. The only difference is the config flags are more code 
> to implement, and a lot more difficult to use. @NoQ, why the hell would we 
> pick another type of flag which makes zero improvement? The goal is to 
> introduce the best possible solution, which is already here.


Well, `-analyzer-checker` and `-analyzer-silence-checker` would be on the same 
level, while there isn't a consensus on that silencing checkers is desired. 
Config flags are exactly one layer lower, that much more harder is it to 
discover. They are *just* a tad bit more uncomfortable to use, but that 
shouldn't matter inside `scan-build`s implementation, should it?

By the way, are they longer to implement? We gain 5 LOC by deleting the 
existing flags. Depending on how long you're like to make your description, the 
config flag, with a newline, would add 3-4 more.

  ANALYZER_OPTION(
  StringRef, CheckersAndPackagesToSilence, "analyzer-silence-checkers",
  "A comma-separated list of checkers and packages to silence.", "")

The changes you made in `clang/lib/Frontend/CompilerInvocation.cpp` should be 
moved after the call to `parseAnalyzerConfigs`, and instead of this:

  Opts.CheckerSilenceVector.clear();
  for (const Arg *A : Args.filtered(OPT_analyzer_silence_checker)) {
A->claim();
// We can have a list of comma separated checker names, e.g:
// '-analyzer-checker=cocoa,unix'
StringRef checkerList = A->getValue();
SmallVector checkers;
checkerList.split(checkers, ",");
for (auto checker : checkers)
  Opts.CheckerSilenceVector.emplace_back(checker);
  }

We should do this:

  assert(Opts.CheckerSilenceVector.empty());
  llvm::SmallVector CheckerList;
  Opts.CheckersAndPackagesToSilence.split(CheckerList, ',');
  for (StringRef Checker : CheckerList)
  Opts.CheckerSilenceVector.emplace_back(checker);

Thats another 5 LOC minus, we actually lost 7! I suspect not even the 
additional scan-build would make it worse.


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

https://reviews.llvm.org/D66042



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

r368750 - [WebAssembly] Make clang emit correct va_arg code for structs

[Guanzhong Chen via cfe-commits]

Author: quantum
Date: Tue Aug 13 14:41:11 2019
New Revision: 368750

URL: http://llvm.org/viewvc/llvm-project?rev=368750=rev
Log:
[WebAssembly] Make clang emit correct va_arg code for structs

Summary:
In the WebAssembly backend, when lowering variadic function calls, non-single
member aggregate type arguments are always passed by pointer.

However, when emitting va_arg code in clang, the arguments are instead read as
if they are passed directly. This results in the pointer being read as the
actual structure.

Fixes https://github.com/emscripten-core/emscripten/issues/9042.

Reviewers: tlively, sbc100, kripken, aheejin, dschuff

Reviewed By: dschuff

Subscribers: dschuff, jgravelle-google, sunfish, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/test/CodeGen/wasm-varargs.c

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=368750=368749=368750=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Tue Aug 13 14:41:11 2019
@@ -833,10 +833,12 @@ ABIArgInfo WebAssemblyABIInfo::classifyR
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction , Address VAListAddr,
   QualType Ty) const {
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*IsIndirect=*/ false,
+  bool IsIndirect =
+  isAggregateTypeForABI(Ty) && !isSingleElementStruct(Ty, getContext());
+  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,
   getContext().getTypeInfoInChars(Ty),
   CharUnits::fromQuantity(4),
-  /*AllowHigherAlign=*/ true);
+  /*AllowHigherAlign=*/true);
 }
 
 
//===--===//

Modified: cfe/trunk/test/CodeGen/wasm-varargs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/wasm-varargs.c?rev=368750=368749=368750=diff
==
--- cfe/trunk/test/CodeGen/wasm-varargs.c (original)
+++ cfe/trunk/test/CodeGen/wasm-varargs.c Tue Aug 13 14:41:11 2019
@@ -87,12 +87,13 @@ struct S test_struct(char *fmt, ...) {
 // CHECK:   [[VA1:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_start(i8* [[VA1]])
 // CHECK:   [[ARGP_CUR:%[^,=]+]] = load i8*, i8** [[VA]], align 4
-// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* 
[[ARGP_CUR]], i32 12
+// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* 
[[ARGP_CUR]], i32 4
 // CHECK:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
-// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]*
-// CHECK:   [[R4:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
-// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R3]] to i8*
-// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R4]], i8* align 
4 [[R5]], i32 12, i1 false)
+// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]**
+// CHECK:   [[R4:%[^,=]+]] = load [[STRUCT_S]]*, [[STRUCT_S]]** %0, align 4
+// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
+// CHECK:   [[R6:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R4]] to i8*
+// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R5]], i8* align 
4 [[R6]], i32 12, i1 false)
 // CHECK:   [[VA2:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_end(i8* [[VA2]])
 // CHECK:   ret void


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

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Sorry, should have done my std::string, StringRef, and Twine homework a lot 
better!

Thanks for your help and suggestions, will fix this.


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

https://reviews.llvm.org/D64564



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

[PATCH] D66151: [clang-doc] Fix bitcode writer

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 214923.
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a comment.

Default value of AccessSpecifier attributes is now AS_public.
Multiple tests modified.


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

https://reviews.llvm.org/D66151

Files:
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -34,6 +34,7 @@
   "path/to/A/Namespace");
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
+  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
   I.ChildEnums.emplace_back();
   I.ChildEnums.back().Name = "OneEnum";
 
@@ -138,6 +139,7 @@
   - USR: ''
 Name:'OneFunction'
 ReturnType:  {}
+Access:  Public
 ChildEnums:
   - USR: ''
 Name:'OneEnum'
@@ -154,6 +156,8 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
+  I.Access = AccessSpecifier::AS_none;
+
   I.ReturnType =
   TypeInfo(EmptySID, "void", InfoType::IT_default, "path/to/void");
   I.Params.emplace_back("int", "path/to/int", "P");
@@ -242,6 +246,7 @@
   I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   I.Params.emplace_back("int", "I");
   I.Params.emplace_back("int", "J");
+  I.Access = AccessSpecifier::AS_none;
 
   CommentInfo Top;
   Top.Kind = "FullComment";
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -103,6 +103,7 @@
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   F.Namespace.emplace_back(EmptySID, "B", InfoType::IT_namespace);
   F.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -299,6 +300,7 @@
   F.Name = "F";
   F.ReturnType = TypeInfo(EmptySID, "int", InfoType::IT_default);
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -314,6 +316,7 @@
   F.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   F.Params.emplace_back("int", "I");
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -379,6 +382,7 @@
   F.ReturnType = TypeInfo(EmptySID, "int", InfoType::IT_default);
   F.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
   F.Params.emplace_back("int", "x");
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 
@@ -389,6 +393,7 @@
   ExportedF.ReturnType = TypeInfo(EmptySID, "double", InfoType::IT_default);
   ExportedF.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
   ExportedF.Params.emplace_back("double", "y");
+  ExportedF.Access = AccessSpecifier::AS_none;
   ExpectedBWithExportedFunction.ChildFunctions.emplace_back(
   std::move(ExportedF));
   CheckNamespaceInfo(, BWithExportedFunction);
Index: clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
@@ -31,6 +31,7 @@
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
+  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
   I.ChildEnums.emplace_back();
   I.ChildEnums.back().Name = "OneEnum";
 
@@ -127,7 +128,7 @@
 
 ### OneFunction
 
-* OneFunction()*
+*public  OneFunction()*
 
 
 
@@ -153,6 +154,8 @@
   I.DefLoc = Location(10, 

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Thanks for your help! And I will wait a few days.

After this, I will look at that PR, will have a look at diagnostics, and then 
at the LLVM side of things.


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

https://reviews.llvm.org/D65776



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

[PATCH] D66121: Debug Info: Nest Objective-C property function decls inside their container.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.h:406
+ CGBuilderTy ,
+ const ObjCContainerDecl *CD = nullptr);
 

aprantl wrote:
> rjmccall wrote:
> > aprantl wrote:
> > > rjmccall wrote:
> > > > Why does this have to be an extra parameter?  The method's DC should be 
> > > > the container.
> > > ```
> > > @protocol BarProto
> > > @property struct Bar *bar;
> > > @end
> > > 
> > > @interface Foo 
> > > @end
> > > 
> > > @implementation Foo {}
> > > @synthesize bar = _bar;
> > > @end
> > > ```
> > > 
> > > The ObjCMethodDecl for the synthesized `bar` accessors is the method decl 
> > > in the protocol `BarProto`, as there is no method decl put into the AST 
> > > inside of `Foo`. Its DeclContext (lexical an regular) therefor is the 
> > > Protocol, which is not what we want as the decl context for a synthesized 
> > > method called `-[Foo setBar:]` in the debug info.
> > Wait, really?  That seems pretty unfortunate.  Should we synthesize a real 
> > implementation method?
> That certainly sounds like the more elegant solution. I didn't know enough 
> about Clang's ObjC implementation to understand whether this was feasible. Do 
> you happen to know where a good entry point for doing this would be?
It might be a consequential change, but I think the right place would just be 
in `Sema::ActOnPropertyImplDecl`, near the bottom where it pulls out the getter 
and setter methods.  You should be able to just create new implicit 
`ObjCMethodDecl`s as redeclarations of the getter/setter and add them to the 
`ObjCImplDecl`.  It might also be nice to link them off of the 
`ObjCPropertyImplDecl`, but that's not as necessary.

If you do that, you'll probably need to update IRGen so that we don't attempt 
to emit the synthesized methods twice.


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

https://reviews.llvm.org/D66121



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

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D66042#1627842 , @Charusso wrote:

> @Szelethus, here is a really cool example: 
> https://clang.llvm.org/docs/ClangCommandLineReference.html.


These are driver flags. They are indeed well-documented and user-facing. 
Frontend flags aren't.

In D66042#1627842 , @Charusso wrote:

> The goal is to introduce the best possible solution


This sounds like a fairly impractical goal to set. Also perfection is highly 
subjective. There are much bigger problems in the Static Analyzer than this 
whole debate of "what kind of flag do we want it to be?". We've already 
received some useful input, but i feel it's just not worth it to spend much 
more time debating here.


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

https://reviews.llvm.org/D66042



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

r368748 - Remove the extra `;`.

[Michael Liao via cfe-commits]

Author: hliao
Date: Tue Aug 13 14:26:42 2019
New Revision: 368748

URL: http://llvm.org/viewvc/llvm-project?rev=368748=rev
Log:
Remove the extra `;`.

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h?rev=368748=368747=368748=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h Tue 
Aug 13 14:26:42 2019
@@ -515,7 +515,7 @@ public:
   GRBugReporter(BugReporterData& d, ExprEngine& eng)
   : BugReporter(d, GRBugReporterKind), Eng(eng) {}
 
-  ~GRBugReporter() override = default;;
+  ~GRBugReporter() override = default;
 
   /// getGraph - Get the exploded graph created by the analysis engine
   ///  for the analyzed method or function.


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

[PATCH] D66168: [WebAssembly] Make clang emit correct va_arg code for structs

[Derek Schuff via Phabricator via cfe-commits]

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

LGTM. Nice that we had a test which tested the wrong thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66168



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

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

LG! But let's wait for Dmitri :)




Comment at: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp:172
   T *();
+  T ();
+};

I actually was a bit lazy when I added these tests. Both `value` and 
`operator*` is overloaded on `&&`. But if you do not feel like adjusting the 
tests this is fine, I can do it myself later :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66164



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

[PATCH] D66168: [WebAssembly] Make clang emit correct va_arg code for structs

[Guanzhong Chen via Phabricator via cfe-commits]

quantum created this revision.
quantum added reviewers: tlively, sbc100, kripken, aheejin.
Herald added subscribers: cfe-commits, sunfish, jgravelle-google, dschuff.
Herald added a project: clang.

In the WebAssembly backend, when lowering variadic function calls, non-single
member aggregate type arguments are always passed by pointer.

However, when emitting va_arg code in clang, the arguments are instead read as
if they are passed directly. This results in the pointer being read as the
actual structure.

Fixes https://github.com/emscripten-core/emscripten/issues/9042.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66168

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/wasm-varargs.c


Index: clang/test/CodeGen/wasm-varargs.c
===
--- clang/test/CodeGen/wasm-varargs.c
+++ clang/test/CodeGen/wasm-varargs.c
@@ -87,12 +87,13 @@
 // CHECK:   [[VA1:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_start(i8* [[VA1]])
 // CHECK:   [[ARGP_CUR:%[^,=]+]] = load i8*, i8** [[VA]], align 4
-// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* 
[[ARGP_CUR]], i32 12
+// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* 
[[ARGP_CUR]], i32 4
 // CHECK:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
-// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]*
-// CHECK:   [[R4:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
-// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R3]] to i8*
-// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R4]], i8* align 
4 [[R5]], i32 12, i1 false)
+// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]**
+// CHECK:   [[R4:%[^,=]+]] = load [[STRUCT_S]]*, [[STRUCT_S]]** %0, align 4
+// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
+// CHECK:   [[R6:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R4]] to i8*
+// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R5]], i8* align 
4 [[R6]], i32 12, i1 false)
 // CHECK:   [[VA2:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_end(i8* [[VA2]])
 // CHECK:   ret void
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -833,10 +833,12 @@
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction , Address VAListAddr,
   QualType Ty) const {
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*IsIndirect=*/ false,
+  bool IsIndirect =
+  isAggregateTypeForABI(Ty) && !isSingleElementStruct(Ty, getContext());
+  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,
   getContext().getTypeInfoInChars(Ty),
   CharUnits::fromQuantity(4),
-  /*AllowHigherAlign=*/ true);
+  /*AllowHigherAlign=*/true);
 }
 
 
//===--===//


Index: clang/test/CodeGen/wasm-varargs.c
===
--- clang/test/CodeGen/wasm-varargs.c
+++ clang/test/CodeGen/wasm-varargs.c
@@ -87,12 +87,13 @@
 // CHECK:   [[VA1:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_start(i8* [[VA1]])
 // CHECK:   [[ARGP_CUR:%[^,=]+]] = load i8*, i8** [[VA]], align 4
-// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* [[ARGP_CUR]], i32 12
+// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* [[ARGP_CUR]], i32 4
 // CHECK:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
-// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]*
-// CHECK:   [[R4:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
-// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R3]] to i8*
-// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R4]], i8* align 4 [[R5]], i32 12, i1 false)
+// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]**
+// CHECK:   [[R4:%[^,=]+]] = load [[STRUCT_S]]*, [[STRUCT_S]]** %0, align 4
+// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
+// CHECK:   [[R6:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R4]] to i8*
+// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R5]], i8* align 4 [[R6]], i32 12, i1 false)
 // CHECK:   [[VA2:%[^,=]+]] = bitcast i8** [[VA]] to i8*
 // CHECK:   call void @llvm.va_end(i8* [[VA2]])
 // CHECK:   ret void
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -833,10 +833,12 @@
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction , Address VAListAddr,
   QualType Ty) const {
-  return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*IsIndirect=*/ false,
+  bool IsIndirect =
+  

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

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

Szelethus accepted this revision.
Szelethus added a comment.

LGTM, thanks! Do you need someone to commit this on your behalf? Also, could 
you please make the comments capitalized, terminated, and fitting in 80 columns?




Comment at: clang/test/Analysis/enum-cast-out-of-range.c:14-24
+  enum unscoped_unspecified_t InvalidBeforeRangeBegin = (enum 
unscoped_unspecified_t)(-5); // expected-warning {{The value provided to the 
cast expression is not in the valid range of values for the enum}}
+  enum unscoped_unspecified_t ValidNegativeValue1 = (enum 
unscoped_unspecified_t)(-4); // OK.
+  enum unscoped_unspecified_t ValidNegativeValue2 = (enum 
unscoped_unspecified_t)(-3); // OK.
+  enum unscoped_unspecified_t InvalidInsideRange1 = (enum 
unscoped_unspecified_t)(-2); // expected-warning {{The value provided to the 
cast expression is not in the valid range of values for the enum}}
+  enum unscoped_unspecified_t InvalidInsideRange2 = (enum 
unscoped_unspecified_t)(-1); // expected-warning {{The value provided to the 
cast expression is not in the valid range of values for the enum}}
+  enum unscoped_unspecified_t InvalidInsideRange3 = (enum 
unscoped_unspecified_t)(0); // expected-warning {{The value provided to the 
cast expression is not in the valid range of values for the enum}}
+  enum unscoped_unspecified_t ValidPositiveValue1 = (enum 
unscoped_unspecified_t)(1); // OK.

Note that you don't need to add the entire warning message here, and since its 
all the same, we could shorten it. Could you please format these lines like 
this?:

```lang=c++
  enum unscoped_unspecified_t InvalidAfterRangeEnd = (enum 
unscoped_unspecified_t)(5);
  // expected-warning@-1{{not in the valid range of values for the enum}}
```

If by some miracle the entire warning message fits in 80 columns, go for it! :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D66014



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

[PATCH] D66164: [LifetimeAnalysis] Support std::stack::top() and std::optional::value()

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added a reviewer: gribozavr.
Herald added a project: clang.

Diagnose dangling pointers that come from std::stack::top() and 
std::optional::value().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66164

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/Sema/warn-lifetime-analysis-nocfg.cpp


Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -169,6 +169,12 @@
   optional();
   optional(const T&);
   T *();
+  T ();
+};
+
+template 
+struct stack {
+  T ();
 };
 }
 
@@ -204,6 +210,8 @@
   int  = *std::optional(); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
   int  = *std::optional(5); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
   int  = std::vector().at(3); // expected-warning {{object backing the 
pointer will be destroyed at the end of the full-expression}}
+  int  = std::optional(5).value(); // expected-warning {{object 
backing the pointer will be destroyed at the end of the full-expression}}
+  int  = std::stack().top();   // expected-warning {{object 
backing the pointer will be destroyed at the end of the full-expression}}
 }
 
 std::vector getTempVec();
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6580,7 +6580,7 @@
 return llvm::StringSwitch(Callee->getName())
 .Cases("begin", "rbegin", "cbegin", "crbegin", true)
 .Cases("end", "rend", "cend", "crend", true)
-.Cases("c_str", "data", "get", true)
+.Cases("c_str", "data", "get", "value", true)
 // Map and set types.
 .Cases("find", "equal_range", "lower_bound", "upper_bound", true)
 .Default(false);
@@ -6591,7 +6591,7 @@
  OO == OverloadedOperatorKind::OO_Star;
 }
 return llvm::StringSwitch(Callee->getName())
-.Cases("front", "back", "at", true)
+.Cases("front", "back", "at", "top", true)
 .Default(false);
   }
   return false;


Index: clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
===
--- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -169,6 +169,12 @@
   optional();
   optional(const T&);
   T *();
+  T ();
+};
+
+template 
+struct stack {
+  T ();
 };
 }
 
@@ -204,6 +210,8 @@
   int  = *std::optional(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   int  = *std::optional(5); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
   int  = std::vector().at(3); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int  = std::optional(5).value(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
+  int  = std::stack().top();   // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
 }
 
 std::vector getTempVec();
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -6580,7 +6580,7 @@
 return llvm::StringSwitch(Callee->getName())
 .Cases("begin", "rbegin", "cbegin", "crbegin", true)
 .Cases("end", "rend", "cend", "crend", true)
-.Cases("c_str", "data", "get", true)
+.Cases("c_str", "data", "get", "value", true)
 // Map and set types.
 .Cases("find", "equal_range", "lower_bound", "upper_bound", true)
 .Default(false);
@@ -6591,7 +6591,7 @@
  OO == OverloadedOperatorKind::OO_Star;
 }
 return llvm::StringSwitch(Callee->getName())
-.Cases("front", "back", "at", true)
+.Cases("front", "back", "at", "top", true)
 .Default(false);
   }
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Any analyzer config flag is equally accessible to anyone as the driver flags as 
they are both flags. The only difference is the config flags are more code to 
implement, and a lot more difficult to use. @NoQ, why the hell would we pick 
another type of flag which makes zero improvement? The goal is to introduce the 
best possible solution, which is already here.

@Szelethus, here is a really cool example: 
https://clang.llvm.org/docs/ClangCommandLineReference.html. Man, I would love 
to use all of the best possible flags by hand and know all the flags in my 
head. At least from the tooling side my flags are easy to use, "out of the 
box", with zero overhead. Also the idea of this type of flag is invented by 
@NoQ and I could not say no to the best possible way.


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

https://reviews.llvm.org/D66042



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

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D66042#162 , @Szelethus wrote:

> Yup. We will be able to present the drastic improvement made on LLVM 
> analyses, while giving us some breathing room to polish a long-term solution. 
> I suspect `-analyzer-config silence-checkers=core -analyzer-config 
> silence-checkers=something.Else` wont work (I believe the last value will be 
> used in the invocation), but if only scan-build is using it, we can do 
> `-analyzer-config silence-checkers=core,something.Else`.


Note that scan-build exposes `-analyzer-config` as, well, `-analyzer-config`. 
So if we turn this into a config, changes in scan-build become relatively 
unnecessary.


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

https://reviews.llvm.org/D66042



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