[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

2019-07-28 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 212085.
void added a comment.
Herald added subscribers: s.egerton, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, 
apazos, simoncook, johnrusso, rbar, asb.

Move a few more tests around. Some go to the LLVM side.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943

Files:
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/pr41027.c
  test/Sema/inline-asm-validate-riscv.c
  test/Sema/inline-asm-validate-x86.c
  test/Sema/pr41027.c

Index: test/Sema/pr41027.c
===
--- test/Sema/pr41027.c
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64 -fsyntax-only %s
-// XFAIL: *
-
-inline void pr41027(unsigned a, unsigned b) {
-  if (__builtin_constant_p(a)) {
-__asm__ volatile("outl %0,%w1" : : "a"(b), "n"(a));
-  } else {
-__asm__ volatile("outl %0,%w1" : : "a"(b), "d"(a));
-  }
-}
Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -4,9 +4,6 @@
 void I(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 32;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "I"(j)); // expected-error{{constraint 'I' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "I"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'I'}}
@@ -21,9 +18,6 @@
 void J(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 64;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "J"(j)); // expected-error{{constraint 'J' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "J"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'J'}}
@@ -38,9 +32,6 @@
 void K(int i, int j) {
   static const int BelowMin = -129;
   static const int AboveMax = 128;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "K"(j)); // expected-error{{constraint 'K' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "K"(BelowMin)); // expected-error{{value '-129' out of range for constraint 'K'}}
@@ -60,9 +51,6 @@
   static const int Valid1 = 0xff;
   static const int Valid2 = 0x;
   static const int Valid3 = 0x;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "L"(j)); // expected-error{{constraint 'L' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "L"(Invalid1)); // expected-error{{value '1' out of range for constraint 'L'}}
@@ -89,9 +77,6 @@
 void M(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 4;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "M"(j)); // expected-error{{constraint 'M' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "M"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'M'}}
@@ -106,9 +91,6 @@
 void N(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 256;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "N"(j)); // expected-error{{constraint 'N' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "N"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'N'}}
@@ -123,9 +105,6 @@
 void O(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 128;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "O"(j)); // expected-error{{constraint 'O' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "O"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'O'}}
@@ -146,10 +125,6 @@
   __asm__ __volatile__("\n#define S_A abcd%0\n" : : "n"(&((struct s*)0)->a));
   // This offset-from-null pointer can be used as an integer constant expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
-  // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // expected-error{{constraint 'n' expects an integer constant expression}}
-  // Floating-point is also not okay.
-  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // expected-error{{constraint 'n' expects an integer constant expression}}
 #ifdef AMD64
   // This arbitrary pointer is fine.
   __asm__ __volatile__("\n#define BEEF abcd%0\n" : : "n"((int*)0xdeadbeef));
Index: test/Sema/inline-asm-validate-riscv.c
==

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

2019-07-28 Thread Borsik Gábor via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367190: [analyzer] Add yaml parser to GenericTaintChecker 
(authored by boga95, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59555?vs=212065&id=212094#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59555

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
  cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/Yaml.h
  cfe/trunk/test/Analysis/Inputs/taint-generic-config-ill-formed.yaml
  cfe/trunk/test/Analysis/Inputs/taint-generic-config-invalid-arg.yaml
  cfe/trunk/test/Analysis/Inputs/taint-generic-config.yaml
  cfe/trunk/test/Analysis/analyzer-config.c
  cfe/trunk/test/Analysis/taint-generic.c

Index: cfe/trunk/lib/StaticAnalyzer/Checkers/Yaml.h
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/Yaml.h
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/Yaml.h
@@ -0,0 +1,59 @@
+//== Yaml.h  -*- 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 file defines convenience functions for handling YAML configuration files
+// for checkers/packages.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKER_YAML_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CHECKER_YAML_H
+
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "llvm/Support/YAMLTraits.h"
+
+namespace clang {
+namespace ento {
+
+/// Read the given file from the filesystem and parse it as a yaml file. The
+/// template parameter must have a yaml MappingTraits.
+/// Emit diagnostic error in case of any failure.
+template 
+llvm::Optional getConfiguration(CheckerManager &Mgr, Checker *Chk,
+   StringRef Option, StringRef ConfigFile) {
+  if (ConfigFile.trim().empty())
+return None;
+
+  llvm::vfs::FileSystem *FS = llvm::vfs::getRealFileSystem().get();
+  llvm::ErrorOr> Buffer =
+  FS->getBufferForFile(ConfigFile.str());
+
+  if (std::error_code ec = Buffer.getError()) {
+Mgr.reportInvalidCheckerOptionValue(Chk, Option,
+"a valid filename instead of '" +
+std::string(ConfigFile) + "'");
+return None;
+  }
+
+  llvm::yaml::Input Input(Buffer.get()->getBuffer());
+  T Config;
+  Input >> Config;
+
+  if (std::error_code ec = Input.error()) {
+Mgr.reportInvalidCheckerOptionValue(Chk, Option,
+"a valid yaml file: " + ec.message());
+return None;
+  }
+
+  return Config;
+}
+
+} // namespace ento
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MOVE_H
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -15,16 +15,18 @@
 //===--===//
 
 #include "Taint.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "Yaml.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
-#include 
-#include 
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 
 using namespace clang;
@@ -44,14 +46,51 @@
 
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
 
-  void printState(raw_ostream &Out, ProgramStateRef State,
-  const char *NL, const char *Sep) const override;
+  void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
+  const char *Sep) const override;
 
-private:
-  static const unsigned InvalidArgIndex = UINT_MAX;
+  using ArgVector = SmallVector;
+  using SignedArgVector = SmallVector;
+
+  enum class VariadicType { None, Src, Dst };
+
+  /// Used to parse the configuration file.
+  struct TaintConfiguration {
+using NameArgsPair = std::pair;
+
+struct Propagation {
+  std::string Name;
+  

r367190 - [analyzer] Add yaml parser to GenericTaintChecker

2019-07-28 Thread Gabor Borsik via cfe-commits
Author: boga95
Date: Sun Jul 28 06:38:04 2019
New Revision: 367190

URL: http://llvm.org/viewvc/llvm-project?rev=367190&view=rev
Log:
[analyzer] Add yaml parser to GenericTaintChecker

While we implemented taint propagation rules for several
builtin/standard functions, there's a natural desire for users to add
such rules to custom functions.

A series of patches will implement an option that allows users to
annotate their functions with taint propagation rules through a YAML
file. This one adds parsing of the configuration file, which may be
specified in the commands line with the analyzer config:
alpha.security.taint.TaintPropagation:Config. The configuration may
contain propagation rules, filter functions (remove taint) and sink
functions (give a warning if it gets a tainted value).

I also added a new header for future checkers to conveniently read YAML
files as checker options.

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

Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/Yaml.h   (with props)
cfe/trunk/test/Analysis/Inputs/taint-generic-config-ill-formed.yaml   (with 
props)
cfe/trunk/test/Analysis/Inputs/taint-generic-config-invalid-arg.yaml   
(with props)
cfe/trunk/test/Analysis/Inputs/taint-generic-config.yaml   (with props)
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/taint-generic.c

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=367190&r1=367189&r2=367190&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Sun Jul 28 
06:38:04 2019
@@ -799,6 +799,13 @@ let ParentPackage = Taint in {
 
 def GenericTaintChecker : Checker<"TaintPropagation">,
   HelpText<"Generate taint information used by other checkers">,
+  CheckerOptions<[
+CmdLineOption,
+  ]>,
   Documentation;
 
 } // end "alpha.security.taint"

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=367190&r1=367189&r2=367190&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Sun Jul 28 
06:38:04 2019
@@ -15,16 +15,18 @@
 
//===--===//
 
 #include "Taint.h"
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "Yaml.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
-#include 
-#include 
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/YAMLTraits.h"
+#include 
 #include 
 
 using namespace clang;
@@ -44,14 +46,51 @@ public:
 
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
 
-  void printState(raw_ostream &Out, ProgramStateRef State,
-  const char *NL, const char *Sep) const override;
+  void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
+  const char *Sep) const override;
 
-private:
-  static const unsigned InvalidArgIndex = UINT_MAX;
+  using ArgVector = SmallVector;
+  using SignedArgVector = SmallVector;
+
+  enum class VariadicType { None, Src, Dst };
+
+  /// Used to parse the configuration file.
+  struct TaintConfiguration {
+using NameArgsPair = std::pair;
+
+struct Propagation {
+  std::string Name;
+  ArgVector SrcArgs;
+  SignedArgVector DstArgs;
+  VariadicType VarType;
+  unsigned VarIndex;
+};
+
+std::vector Propagations;
+std::vector Filters;
+std::vector Sinks;
+
+TaintConfiguration() = default;
+TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(TaintConfiguration &&) = default;
+TaintConfiguration &operator=(const TaintConfiguration &) = delete;
+TaintConfiguration &operator=(TaintConfiguration &&) = default;
+  };
+
+  /// Convert SignedArgVector to ArgVector.
+  ArgVector convertToArgVector(CheckerManager &Mgr, const std::string &Option,
+   SignedArgVector Args);
+
+  /// Parse the config.
+  void parseConfiguration(CheckerManager &Mgr, const std::string &

r367193 - Buildbot fix for r367190

2019-07-28 Thread Gabor Borsik via cfe-commits
Author: boga95
Date: Sun Jul 28 07:57:41 2019
New Revision: 367193

URL: http://llvm.org/viewvc/llvm-project?rev=367193&view=rev
Log:
Buildbot fix for r367190

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

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=367193&r1=367192&r2=367193&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Sun Jul 28 
07:57:41 2019
@@ -811,7 +811,7 @@ void ento::registerGenericTaintChecker(C
   llvm::Optional Config =
   getConfiguration(Mgr, Checker, Option, ConfigFile);
   if (Config)
-Checker->parseConfiguration(Mgr, Option, std::move(Config).getValue());
+Checker->parseConfiguration(Mgr, Option, std::move(Config.getValue()));
 }
 
 bool ento::shouldRegisterGenericTaintChecker(const LangOptions &LO) {


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


r367184 - [FunctionAttrs] Annotate "willreturn" for intrinsics

2019-07-28 Thread Hideto Ueno via cfe-commits
Author: uenoku
Date: Sat Jul 27 23:09:56 2019
New Revision: 367184

URL: http://llvm.org/viewvc/llvm-project?rev=367184&view=rev
Log:
[FunctionAttrs] Annotate "willreturn" for intrinsics

Summary:
In D62801, new function attribute `willreturn` was introduced. In short, a 
function with `willreturn` is guaranteed to come back to the call site(more 
precise definition is in LangRef).

In this patch, willreturn is annotated for LLVM intrinsics.

Reviewers: jdoerfert

Reviewed By: jdoerfert

Subscribers: jvesely, nhaehnle, sstefan1, llvm-commits

Tags: #llvm

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

Modified:
cfe/trunk/test/CodeGen/libcalls.c

Modified: cfe/trunk/test/CodeGen/libcalls.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/libcalls.c?rev=367184&r1=367183&r2=367184&view=diff
==
--- cfe/trunk/test/CodeGen/libcalls.c (original)
+++ cfe/trunk/test/CodeGen/libcalls.c Sat Jul 27 23:09:56 2019
@@ -124,4 +124,4 @@ void test_builtins(double d, float f, lo
 }
 
 // CHECK-NO-DAG: attributes [[NUW_RN]] = { nounwind readnone{{.*}} }
-// CHECK-NO-DAG: attributes [[NUW_RNI]] = { nounwind readnone speculatable }
+// CHECK-NO-DAG: attributes [[NUW_RNI]] = { nounwind readnone speculatable 
willreturn }


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


[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

2019-07-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:99
+
+  if (ans == yes || no) // expected-warning {{enum constant in boolean 
context}}
+return a;

Why do we want to diagnose this case in C++ but not in C?


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

https://reviews.llvm.org/D63082



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


[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

2019-07-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D63139#1603667 , @xbolva00 wrote:

> Ping again
>
> Is ‘CaseStmt´ AST C++ issue is a blocker for this patch or not?


I do not think it is a blocker for this patch. It's the existing behavior of 
the AST and while annoying, it can be improved later.




Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8192
+def warn_unreachable_stmt_in_switch : Warning<
+  "statement will be never executed">, 
InGroup>;
 def warn_bool_switch_condition : Warning<

I thought we had a warning group for this already, and we do, it's 
`-Wunreachable-code`. I think the new diagnostic group be a child of the 
existing one, if we need the group at all.



Comment at: test/SemaCXX/warn-unreachable-stmt-switch.cpp:1-4
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify  %s

You should add a RUN line that also passes `-Wunreachable-code` to ensure this 
doesn't introduce duplicate diagnostics. It seems some number of these are 
already covered by that warning class: https://godbolt.org/z/f60YxB


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

https://reviews.llvm.org/D63139



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


[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

2019-07-28 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 marked an inline comment as done.
xbolva00 added inline comments.



Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:99
+
+  if (ans == yes || no) // expected-warning {{enum constant in boolean 
context}}
+return a;

aaron.ballman wrote:
> Why do we want to diagnose this case in C++ but not in C?
I think we can and should but for unknown reason (for me) this is GCC’s 
behaviour.

Maybe it is too noisy for C codebases? 

Maybe introduce -Wenum-in-bool-context as subgroup of -Wint-in-bool-context? 
And enable it for C and C++?


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

https://reviews.llvm.org/D63082



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


Phabricator stability

2019-07-28 Thread Aaron Ballman via cfe-commits
Just an FYI, but it seems like Phabricator may be having some issues
this weekend. When trying to perform reviews, a few folks have noticed
they get errors about insufficient space to save various internal
tables. Additionally, it seems that emails from submitted reviews are
not always being generated. It is possible that the machine is running
out of disk space, or otherwise needs some attention.

If a Phab admin is available to look into the server, that would be appreciated.

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


[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

2019-07-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:11656
+  IsListInit =
+  IsListInit || (isa(OrigE) && S.getLangOpts().CPlusPlus);
+

Do you want to perform the `isa<>` on `OrigE` or on `E` which has had paren and 
implicit casts stripped?


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

https://reviews.llvm.org/D64666



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


[PATCH] D63276: [AST] Add FunctionDecl::getParametersSourceRange()

2019-07-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Phab looked to be having disk issues when I added my comments, so I'm pinging 
this review just to be sure the latest comments make it to the mailing list.


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

https://reviews.llvm.org/D63276



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


[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

2019-07-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:103
   MaybeParseCXX11Attributes(Attrs, nullptr, /*MightBeObjCMessageSend*/ true);
+
   if (!MaybeParseOpenCLUnrollHintAttribute(Attrs))

Spurious newline?



Comment at: clang/lib/Parse/ParseStmt.cpp:218
  DeclEnd, Attrs);
+  if (SeenGNUAttributes) {
+DeclStart = GNUAttributeLoc;

Elide braces.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1279
 continue;
-  if (S.getLangOpts().CPlusPlus11) {
+  if (S.getLangOpts().CPlusPlus11 || S.getLangOpts().C99) {
 const Stmt *Term = B->getTerminatorStmt();

What is special about C99 here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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


[PATCH] D65233: driver: Don't warn about assembler flags being unused when not assembling; different approach

2019-07-28 Thread Bjorn Pettersson via Phabricator via cfe-commits
bjope added a comment.

In D65233#1603783 , @thakis wrote:

> In D65233#1603741 , @bjope wrote:
>
> > I made another fixup (similar to https://reviews.llvm.org/rL367176) here 
> > https://reviews.llvm.org/rL367182.
> >
> > Can someone please take a look (a post-commit review) of 
> > https://reviews.llvm.org/rL367182 to verify that I did not misunderstand 
> > the intention with the test somehow?
>
>
> Looks good. Thanks much for the fix!


Nice to know that I did not mess up the test in my attempt to silence the 
buildbots. So thanks for taking a look.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65233



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


[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

2019-07-28 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1228
+  if (MacroName.empty()) {
+if (!PreferClangAttr)
+  MacroName = "[[fallthrough]]";

This code is not tested?

I think you can use tests from my older patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64838



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


[PATCH] D65378: [analyzer][NFC] Refactoring BugReporter.cpp P1.: Store interesting symbols/regions in a simple set

2019-07-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, Charusso, 
baloghadamsoftware, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

The goal of this refactoring effort was to better understand how 
interestingness was propagated in BugReporter.cpp, which eventually turned out 
to be a dead end, but with such a twist, I wouldn't even want to spoil it ahead 
of time. However, I did get to learn //a lot// about how things are working in 
there.

In these series of patches, as well as cleaning up the code big time, I invite 
you to study how BugReporter.cpp operates, and discuss how we could design this 
file to reduce the horrible mess that it is.

---

This patch reverts a great part of rC162028 
, which holds the title //"Allow multiple 
PathDiagnosticConsumers to be used with a BugReporter at the same time."//. 
This, however doesn't imply that there's any need for multiple "layers" or 
stacks of interesting symbols and regions, quite the contrary, I would argue 
that we would like to generate the same amount of information for all output 
types, and only process them differently.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65378

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2058,12 +2058,6 @@
   Callbacks.clear();
 }
 
-BugReport::~BugReport() {
-  while (!interestingSymbols.empty()) {
-popInterestingSymbolsAndRegions();
-  }
-}
-
 const Decl *BugReport::getDeclWithIssue() const {
   if (DeclWithIssue)
 return DeclWithIssue;
@@ -2101,10 +2095,10 @@
   if (!sym)
 return;
 
-  getInterestingSymbols().insert(sym);
+  InterestingSymbols.insert(sym);
 
   if (const auto *meta = dyn_cast(sym))
-getInterestingRegions().insert(meta->getRegion());
+InterestingRegions.insert(meta->getRegion());
 }
 
 void BugReport::markInteresting(const MemRegion *R) {
@@ -2112,10 +2106,10 @@
 return;
 
   R = R->getBaseRegion();
-  getInterestingRegions().insert(R);
+  InterestingRegions.insert(R);
 
   if (const auto *SR = dyn_cast(R))
-getInterestingSymbols().insert(SR->getSymbol());
+InterestingSymbols.insert(SR->getSymbol());
 }
 
 void BugReport::markInteresting(SVal V) {
@@ -2138,18 +2132,18 @@
 return false;
   // We don't currently consider metadata symbols to be interesting
   // even if we know their region is interesting. Is that correct behavior?
-  return getInterestingSymbols().count(sym);
+  return InterestingSymbols.count(sym);
 }
 
 bool BugReport::isInteresting(const MemRegion *R) {
   if (!R)
 return false;
   R = R->getBaseRegion();
-  bool b = getInterestingRegions().count(R);
+  bool b = InterestingRegions.count(R);
   if (b)
 return true;
   if (const auto *SR = dyn_cast(R))
-return getInterestingSymbols().count(SR->getSymbol());
+return InterestingSymbols.count(SR->getSymbol());
   return false;
 }
 
@@ -2159,33 +2153,6 @@
   return InterestingLocationContexts.count(LC);
 }
 
-void BugReport::lazyInitializeInterestingSets() {
-  if (interestingSymbols.empty()) {
-interestingSymbols.push_back(new Symbols());
-interestingRegions.push_back(new Regions());
-  }
-}
-
-BugReport::Symbols &BugReport::getInterestingSymbols() {
-  lazyInitializeInterestingSets();
-  return *interestingSymbols.back();
-}
-
-BugReport::Regions &BugReport::getInterestingRegions() {
-  lazyInitializeInterestingSets();
-  return *interestingRegions.back();
-}
-
-void BugReport::pushInterestingSymbolsAndRegions() {
-  interestingSymbols.push_back(new Symbols(getInterestingSymbols()));
-  interestingRegions.push_back(new Regions(getInterestingRegions()));
-}
-
-void BugReport::popInterestingSymbolsAndRegions() {
-  delete interestingSymbols.pop_back_val();
-  delete interestingRegions.pop_back_val();
-}
-
 const Stmt *BugReport::getStmt() const {
   if (!ErrorNode)
 return nullptr;
@@ -2236,12 +2203,6 @@
 // Methods for BugReporter and subclasses.
 //===--===//
 
-BugReportEquivClass::~BugReportEquivClass() = default;
-
-GRBugReporter::~GRBugReporter() = default;
-
-BugReporterData::~BugReporterData() = default;
-
 ExplodedGraph &GRBugReporter::getGraph() { return Eng.getGraph(); }
 
 ProgramStateManager&
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -107,22 +107,19 @@
   E

[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

2019-07-28 Thread Borsik Gábor via Phabricator via cfe-commits
boga95 updated this revision to Diff 212119.
boga95 added a subscriber: steakhal.

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

https://reviews.llvm.org/D59637

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/test/Analysis/taint-generic.c

Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -338,3 +338,43 @@
   if (i < rhs)
 *(volatile int *) 0; // no-warning
 }
+
+
+// Test configuration
+int mySource1();
+void mySource2(int*);
+void myScanf(const char*, ...);
+int myPropagator(int, int*);
+int mySnprintf(char*, size_t, const char*, ...);
+void mySink(int, int, int);
+
+void testConfigurationSources1() {
+  int x = mySource1();
+  Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationSources2() {
+  int x;
+  mySource2(&x);
+  Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationSources3() {
+  int x, y;
+  myScanf("%d %d", &x, &y);
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationPropagation() {
+  int x = mySource1();
+  int y;
+  myPropagator(x, &y);
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testConfigurationSinks() {
+  int x = mySource1();
+  mySink(x, 1, 2); // expected-warning {{Untrusted data is passed to a user defined sink}}
+  mySink(1, x, 2); // no-warning
+  mySink(1, 2, x); // expected-warning {{Untrusted data is passed to a user defined sink}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -132,6 +132,11 @@
   bool checkTaintedBufferSize(const CallExpr *CE, const FunctionDecl *FDecl,
   CheckerContext &C) const;
 
+  /// Check if tainted data is used as a custom sink's parameter.
+  static const char MsgCustomSink[];
+  bool checkCustomSinks(const CallExpr *CE, StringRef Name,
+CheckerContext &C) const;
+
   /// Generate a report if the expression is tainted or points to tainted data.
   bool generateReportIfTainted(const Expr *E, const char Msg[],
CheckerContext &C) const;
@@ -175,7 +180,8 @@
 
 /// Get the propagation rule for a given function.
 static TaintPropagationRule
-getTaintPropagationRule(const FunctionDecl *FDecl, StringRef Name,
+getTaintPropagationRule(const GenericTaintChecker *Checker,
+const FunctionDecl *FDecl, StringRef Name,
 CheckerContext &C);
 
 void addSrcArg(unsigned A) { SrcArgs.push_back(A); }
@@ -240,6 +246,10 @@
 "Untrusted data is used to specify the buffer size "
 "(CERT/STR31-C. Guarantee that storage for strings has sufficient space "
 "for character data and the null terminator)";
+
+const char GenericTaintChecker::MsgCustomSink[] =
+"Untrusted data is passed to a user defined sink";
+;
 } // end of anonymous namespace
 
 using TaintConfig = GenericTaintChecker::TaintConfiguration;
@@ -330,7 +340,8 @@
 
 GenericTaintChecker::TaintPropagationRule
 GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
-const FunctionDecl *FDecl, StringRef Name, CheckerContext &C) {
+const GenericTaintChecker *Checker, const FunctionDecl *FDecl,
+StringRef Name, CheckerContext &C) {
   // TODO: Currently, we might lose precision here: we always mark a return
   // value as tainted even if it's just a pointer, pointing to tainted data.
 
@@ -424,6 +435,10 @@
   // or smart memory copy:
   // - memccpy - copying until hitting a special character.
 
+  auto It = Checker->CustomPropagations.find(Name);
+  if (It != Checker->CustomPropagations.end())
+return It->getValue();
+
   return TaintPropagationRule();
 }
 
@@ -464,7 +479,7 @@
 
   // First, try generating a propagation rule for this function.
   TaintPropagationRule Rule =
-  TaintPropagationRule::getTaintPropagationRule(FDecl, Name, C);
+  TaintPropagationRule::getTaintPropagationRule(this, FDecl, Name, C);
   if (!Rule.isNull()) {
 State = Rule.process(CE, C);
 if (!State)
@@ -536,6 +551,9 @@
   if (checkTaintedBufferSize(CE, FDecl, C))
 return true;
 
+  if (checkCustomSinks(CE, Name, C))
+return true;
+
   return false;
 }
 
@@ -572,8 +590,12 @@
   // Check for taint in arguments.
   bool IsTainted = true;
   for (unsigned ArgNum : SrcArgs) {
-if (ArgNum >= CE->getNumArgs())
-  return State;
+if (ArgNum >= CE->getNumArgs()) {
+  StringRef Name = C.getCalleeName(C.getCalleeDecl(CE));
+  llvm::errs() << "Skip out of bound SrcArg: " << Name << ":" << ArgNum
+   << '\n';
+  continue;
+

[PATCH] D65381: [analyzer][NFC] Refactoring BugReporter.cpp P3.: std::shared_pointer -> PathDiagnosticPieceRef

2019-07-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, baloghadamsoftware, 
dcoughlin, Charusso.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus edited the summary of this revision.

Exactly what it says on the tin!

  find clang/ -type f -exec sed -i 
's/std::shared_ptr/PathDiagnosticPieceRef/g' {} \;
  git diff -U3 --no-color HEAD^ | clang-format-diff-6.0 -p1 -i


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65381

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp
  clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h
  clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
  
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.h
  clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Index: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -473,16 +473,16 @@
  const Preprocessor &PP,
  const PathPieces &Path) {
   PlistPrinter Printer(FM, AnOpts, PP);
-  assert(std::is_partitioned(
-   Path.begin(), Path.end(),
-   [](const std::shared_ptr &E)
- { return E->getKind() == PathDiagnosticPiece::Note; }) &&
+  assert(std::is_partitioned(Path.begin(), Path.end(),
+ [](const PathDiagnosticPieceRef &E) {
+   return E->getKind() == PathDiagnosticPiece::Note;
+ }) &&
  "PathDiagnostic is not partitioned so that notes precede the rest");
 
   PathPieces::const_iterator FirstNonNote = std::partition_point(
-  Path.begin(), Path.end(),
-  [](const std::shared_ptr &E)
-{ return E->getKind() == PathDiagnosticPiece::Note; });
+  Path.begin(), Path.end(), [](const PathDiagnosticPieceRef &E) {
+return E->getKind() == PathDiagnosticPiece::Note;
+  });
 
   PathPieces::const_iterator I = Path.begin();
 
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -658,16 +658,14 @@
   // Process the path.
   // Maintain the counts of extra note pieces separately.
   unsigned TotalPieces = path.size();
-  unsigned TotalNotePieces =
-  std::count_if(path.begin(), path.end(),
-[](const std::shared_ptr &p) {
-  return isa(*p);
-});
-  unsigned PopUpPieceCount =
-  std::count_if(path.begin(), path.end(),
-[](const std::shared_ptr &p) {
-  return isa(*p);
-});
+  unsigned TotalNotePieces = std::count_if(
+  path.begin(), path.end(), [](const PathDiagnosticPieceRef &p) {
+return isa(*p);
+  });
+  unsigned PopUpPieceCount = std::count_if(
+  path.begin(), path.end(), [](const PathDiagnosticPieceRef &p) {
+return isa(*p);
+  });
 
   unsigned TotalRegularPieces = TotalPieces - TotalNotePieces - PopUpPieceCount;
   unsigned NumRegularPieces = TotalRegularPieces;
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -255,20 +255,19 @@
 // Implementation of BugReporterVisitor.
 //===--===//
 
-std::shared_ptr
-BugReporterVisitor::getEndPath(BugReporterCont

[PATCH] D65382: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it const

2019-07-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, Charusso, dcoughlin, 
baloghadamsoftware, rnkovacs.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

When I'm new to a file/codebase, I personally find C++'s strong static type 
system to be a great aid. `BugReporter.cpp` is still painful to read however: 
function calls are made with mile long parameter lists, seemingly all of them 
taken with a non-const reference/pointer. This patch fixes nothing but this: 
make a few things const, and hammer it until it compiles.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65382

Files:
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCUnusedIVarsChecker.cpp
  
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
  clang/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp

Index: clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -135,7 +135,7 @@
   // Do not collect nodes for non-consumed Stmt or Expr to ensure precise
   // diagnostic generation; specifically, so that we could anchor arrows
   // pointing to the beginning of statements (as written in code).
-  ParentMap &PM = progPoint.getLocationContext()->getParentMap();
+  const ParentMap &PM = progPoint.getLocationContext()->getParentMap();
   if (!PM.isConsumedExpr(Ex))
 return false;
 
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -1033,7 +1033,7 @@
 ObjCMessageKind ObjCMethodCall::getMessageKind() const {
   if (!Data) {
 // Find the parent, ignoring implicit casts.
-ParentMap &PM = getLocationContext()->getParentMap();
+const ParentMap &PM = getLocationContext()->getParentMap();
 const Stmt *S = PM.getParentIgnoreParenCasts(getOriginExpr());
 
 // Check if parent is a PseudoObjectExpr.
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2314,7 +2314,7 @@
 SourceLocation BeginLoc = OriginalExpr->getBeginLoc();
 SourceLocation EndLoc = OriginalExpr->getEndLoc();
 if (BeginLoc.isMacroID() && EndLoc.isMacroID()) {
-  SourceManager &SM = BRC.getSourceManager();
+  const SourceManager &SM = BRC.getSourceManager();
   const LangOptions &LO = BRC.getASTContext().getLangOpts();
   if (Lexer::isAtStartOfMacroExpansion(BeginLoc, SM, LO) &&
   Lexer::isAtEndOfMacroExpansion(EndLoc, SM, LO)) {
@@ -2628,7 +2628,7 @@
 BugReporterContext &BRC, const ExplodedNode *N, BugReport &BR) {
   // Here we suppress false positives coming from system headers. This list is
   // based on known issues.
-  AnalyzerOptions &Options = BRC.getAnalyzerOptions();
+  const AnalyzerOptions &Options = BRC.getAnalyzerOptions();
   const Decl *D = N->getLocationContext()->getDecl();
 
   if (AnalysisDeclContext::isInStdNamespace(D)) {
@@ -2695,7 +2695,7 @@
 
   // Skip reports within the sys/queue.h macros as we do not have the ability to
   // reason about data structure shapes.
-  SourceManager &SM = BRC.getSourceManager();
+  const SourceManager &SM = BRC.getSourceManager();
   FullSourceLoc Loc = BR.getLocation(SM).asLocation();
   while (Loc.isMacroID()) {
 Loc = Loc.getSpellingLoc();
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -190,8 +190,8 @@
 /// Recursively scan through a path and prune out calls and macros pieces
 /// that aren't needed.  Return true if afterwards the path contains
 /// "interesting stuff" which means it shouldn't be pruned from the parent path.
-static bool removeUnneededCalls(PathPieces &pieces, BugRepo

r367199 - Improve MSVC visualizers for DeclSpec and TemplateName

2019-07-28 Thread Mike Spertus via cfe-commits
Author: mps
Date: Sun Jul 28 20:34:40 2019
New Revision: 367199

URL: http://llvm.org/viewvc/llvm-project?rev=367199&view=rev
Log:
Improve MSVC visualizers for DeclSpec and TemplateName

DeclSpec now shows the TypeRep, ExprRep, or DeclRep as appropriate
TemplateName decodes and displays the StorageType
A few minor refinements to other types

Modified:
cfe/trunk/utils/ClangVisualizers/clang.natvis

Modified: cfe/trunk/utils/ClangVisualizers/clang.natvis
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ClangVisualizers/clang.natvis?rev=367199&r1=367198&r2=367199&view=diff
==
--- cfe/trunk/utils/ClangVisualizers/clang.natvis (original)
+++ cfe/trunk/utils/ClangVisualizers/clang.natvis Sun Jul 28 20:34:40 2019
@@ -49,6 +49,7 @@ For later versions of Visual Studio, no
 {(clang::FunctionProtoType 
*)this,view(right)na}
 {*(clang::TemplateSpecializationType *)this}
 {*(clang::DeducedTemplateSpecializationType 
*)this}
+{*(clang::DeducedTemplateSpecializationType 
*)this,view(cpp)}
 {*(clang::InjectedClassNameType *)this}
 {*(clang::DependentNameType *)this}
 {*(clang::PackExpansionType *)this}
@@ -203,6 +204,7 @@ For later versions of Visual Studio, no
 {*this,view(TorC)} 
{*this,view(MaybeEllipses)}{Name,view(cpp)} 
{this,view(Initializer)na} 
   
   
+{*TemplatedDecl,view(cpp)}
 template{TemplateParams,na} 
{*TemplatedDecl};
 
   TemplateParams,na
@@ -252,7 +254,57 @@ For later versions of Visual Studio, no
   (SubstTemplateTemplateParmPackStorage*)this
 
   
+  
+  
+
+
+  {(clang::TemplateDecl *)(Val.Value & ~3LL),view(cpp)na}
+
+
+  {(clang::TemplateDecl *)(Val.Value & ~3LL),na}
+
+
+  {(clang::UncommonTemplateNameStorage *)(Val.Value & 
~3LL),view(cpp)na}
+
+
+  {(clang::UncommonTemplateNameStorage *)(Val.Value & ~3LL),na}
+
+
+  {(clang::QualifiedTemplateName *)(Val.Value & ~3LL),view(cpp)na}
+
+
+  {(clang::QualifiedTemplateName *)(Val.Value & ~3LL),na}
+
+
+  {(clang::DependentTemplateName *)(Val.Value & ~3LL),view(cpp)na}
+
+
+  {(clang::DependentTemplateName *)(Val.Value & ~3LL),na}
+
+
+  "TemplateDecl",s8b
+  
+(clang::TemplateDecl *)(Val.Value & ~3LL)
+  
+  "UncommonTemplateNameStorage",s8b
+  
+(clang::UncommonTemplateNameStorage *)(Val.Value & ~3LL)
+  
+  "QualifiedTemplateName",s8b
+  
+(clang::QualifiedTemplateName *)(Val.Value & ~3LL)
+  
+  "DependentTemplateName",s8b
+  
+(clang::DependentTemplateName *)(Val.Value & ~3LL)
+  
+  Val
+
+
+  
   
+{Storage,view(cpp)na}
 {Storage,na}
 
   Storage
@@ -608,11 +660,12 @@ For later versions of Visual Studio, no
   
   
 {CanonicalType,view(cpp)}
+{Template,view(cpp)}
 {Template}
 
   Template
   CanonicalType,view(cpp)
-  *(clang::DeducedType *)this
+  (clang::DeducedType *)this
   Template
 
   
@@ -711,7 +764,31 @@ For later versions of Visual Studio, no
 {(clang::tok::TokenKind)Kind,en}
   
   
-[{(clang::DeclSpec::SCS)StorageClassSpec}], 
[{(clang::TypeSpecifierType)TypeSpecType}]
+
+  , [{TypeRep}]
+
+
+  , [{ExprRep}]
+
+
+  , [{DeclRep}]
+
+
+[{(clang::DeclSpec::SCS)StorageClassSpec,en}], 
[{(clang::TypeSpecifierType)TypeSpecType,en}]{this,view(extra)na}
+
+  (clang::DeclSpec::SCS)StorageClassSpec
+  (clang::TypeSpecifierType)TypeSpecType
+  
+TypeRep
+  
+  
+ExprRep
+  
+  
+DeclRep
+  
+
+
   
   
 {Name,s}


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


[PATCH] D64863: [clangd] Ignore diags from builtin files

2019-07-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 212131.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Always set LastDiagWasAdjusted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64863

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -907,7 +907,6 @@
 int x = 5/0;)cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", Header.code()}};
-  auto diags = TU.build().getDiagnostics();
   EXPECT_THAT(TU.build().getDiagnostics(),
   UnorderedElementsAre(AllOf(
   Diag(Main.range(), "in included file: C++ requires "
@@ -915,6 +914,19 @@
   WithNote(Diag(Header.range(), "error occurred here");
 }
 
+TEST(IgnoreDiags, FromNonWrittenSources) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  Annotations Header(R"cpp(
+int x = 5/0;
+int b = [[FOO]];)cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.AdditionalFiles = {{"a.h", Header.code()}};
+  TU.ExtraArgs = {"-DFOO=NOOO"};
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
+}
+
 } // namespace
 
 } // namespace clangd
Index: clang-tools-extra/clangd/Diagnostics.h
===
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -145,6 +145,9 @@
   std::vector Output;
   llvm::Optional LangOpts;
   llvm::Optional LastDiag;
+  // Set by adjustDiagFromHeader to indicate the fact that LastDiag was not
+  // inside main file initially.
+  bool LastDiagWasAdjusted = false;
   llvm::DenseSet IncludeLinesWithErrors;
   bool LastPrimaryDiagnosticWasSuppressed = false;
 };
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -20,6 +20,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
@@ -107,8 +108,12 @@
   return halfOpenToRange(M, R);
 }
 
-void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
+bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
   const LangOptions &LangOpts) {
+  // We only report diagnostics with at least error severity from headers.
+  if (D.Severity < DiagnosticsEngine::Level::Error)
+return false;
+
   const SourceLocation &DiagLoc = Info.getLocation();
   const SourceManager &SM = Info.getSourceManager();
   SourceLocation IncludeInMainFile;
@@ -119,13 +124,14 @@
IncludeLocation = GetIncludeLoc(IncludeLocation))
 IncludeInMainFile = IncludeLocation;
   if (IncludeInMainFile.isInvalid())
-return;
+return false;
 
   // Update diag to point at include inside main file.
   D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
   D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
   D.Range.end = sourceLocToPosition(
   SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
+  D.InsideMainFile = true;
 
   // Add a note that will point to real diagnostic.
   const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
@@ -138,6 +144,7 @@
 
   // Update message to mention original file.
   D.Message = llvm::Twine("in included file: ", D.Message).str();
+  return true;
 }
 
 bool isInsideMainFile(const clang::Diagnostic &D) {
@@ -465,6 +472,7 @@
   }
 
   bool InsideMainFile = isInsideMainFile(Info);
+  SourceManager &SM = Info.getSourceManager();
 
   auto FillDiagBase = [&](DiagBase &D) {
 D.Range = diagnosticRange(Info, *LangOpts);
@@ -472,8 +480,7 @@
 Info.FormatDiagnostic(Message);
 D.Message = Message.str();
 D.InsideMainFile = InsideMainFile;
-D.File = Info.getSourceManager().getFilename(Info.getLocation());
-auto &SM = Info.getSourceManager();
+D.File = SM.getFilename(Info.getLocation());
 D.AbsFile = getCanonicalPath(
 SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM);
 D.Severity = DiagLevel;
@@ -496,10 +503,9 @@
   if (FixIt.RemoveRange.getBegin().isMacroID() ||
   FixIt.RemoveRange.getEnd().isMacroID())
 return false;
-  if (!isInsideMainFile(FixIt.RemoveRange.getBegin(),
-Info.getSourceManager()))
+  if (!isInsideMainFile(FixIt.RemoveRange.getBegin(), SM))
 return false;
-  Edits.push_back(toTextE

[PATCH] D64863: [clangd] Ignore diags from builtin files

2019-07-28 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563
 FillDiagBase(*LastDiag);
-adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
+if (!InsideMainFile)
+  LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts);

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > We probably want to **always** set the value of this field to avoid 
> > > accidentally reading the flag for the previous `LastDiag`
> > I don't follow, this is always set in line `475` at the beginning of 
> > `HandleDiagnostic` method.
> I might be missing something, but the only assignment to 
> `LastDiagWasAdjusted` seems to be here (apart from initialization at the 
> declaration site)
oopsy, fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64863



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