[PATCH] D73696: [clang][Index] Introduce a TemplateParm SymbolKind

2020-02-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 243091.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- As discussed offline, do not expose new symbol kinds to libclang
- Set types for templateparms in HoverInfo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73696

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/Index/IndexSymbol.h
  clang/lib/Index/IndexSymbol.cpp
  clang/tools/libclang/CXIndexDataConsumer.cpp
  clang/unittests/Index/IndexTests.cpp

Index: clang/unittests/Index/IndexTests.cpp
===
--- clang/unittests/Index/IndexTests.cpp
+++ clang/unittests/Index/IndexTests.cpp
@@ -249,8 +249,13 @@
   Index->Symbols.clear();
   tooling::runToolOnCode(std::make_unique(Index, Opts), Code);
   EXPECT_THAT(Index->Symbols,
-  AllOf(Contains(QName("Foo::T")), Contains(QName("Foo::I")),
-Contains(QName("Foo::C")), Contains(QName("Foo::NoRef";
+  AllOf(Contains(AllOf(QName("Foo::T"),
+   Kind(SymbolKind::TemplateTypeParm))),
+Contains(AllOf(QName("Foo::I"),
+   Kind(SymbolKind::NonTypeTemplateParm))),
+Contains(AllOf(QName("Foo::C"),
+   Kind(SymbolKind::TemplateTemplateParm))),
+Contains(QName("Foo::NoRef";
 }
 
 TEST(IndexTest, UsingDecls) {
Index: clang/tools/libclang/CXIndexDataConsumer.cpp
===
--- clang/tools/libclang/CXIndexDataConsumer.cpp
+++ clang/tools/libclang/CXIndexDataConsumer.cpp
@@ -1245,6 +1245,9 @@
   case SymbolKind::Macro:
   case SymbolKind::ClassProperty:
   case SymbolKind::Using:
+  case SymbolKind::TemplateTypeParm:
+  case SymbolKind::TemplateTemplateParm:
+  case SymbolKind::NonTypeTemplateParm:
 return CXIdxEntity_Unexposed;
 
   case SymbolKind::Enum: return CXIdxEntity_Enum;
Index: clang/lib/Index/IndexSymbol.cpp
===
--- clang/lib/Index/IndexSymbol.cpp
+++ clang/lib/Index/IndexSymbol.cpp
@@ -357,6 +357,15 @@
 case Decl::VarTemplate:
   llvm_unreachable("variables handled before");
   break;
+case Decl::TemplateTypeParm:
+  Info.Kind = SymbolKind::TemplateTypeParm;
+  break;
+case Decl::TemplateTemplateParm:
+  Info.Kind = SymbolKind::TemplateTemplateParm;
+  break;
+case Decl::NonTypeTemplateParm:
+  Info.Kind = SymbolKind::NonTypeTemplateParm;
+  break;
 // Other decls get the 'unknown' kind.
 default:
   break;
@@ -517,6 +526,9 @@
   case SymbolKind::ConversionFunction: return "conversion-func";
   case SymbolKind::Parameter: return "param";
   case SymbolKind::Using: return "using";
+  case SymbolKind::TemplateTypeParm: return "template-type-param";
+  case SymbolKind::TemplateTemplateParm: return "template-template-param";
+  case SymbolKind::NonTypeTemplateParm: return "non-type-template-param";
   }
   llvm_unreachable("invalid symbol kind");
 }
Index: clang/include/clang/Index/IndexSymbol.h
===
--- clang/include/clang/Index/IndexSymbol.h
+++ clang/include/clang/Index/IndexSymbol.h
@@ -54,6 +54,9 @@
 
   Parameter,
   Using,
+  TemplateTypeParm,
+  TemplateTemplateParm,
+  NonTypeTemplateParm,
 };
 
 enum class SymbolLanguage : uint8_t {
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -573,6 +573,42 @@
  // pattern.
  HI.Documentation = "comment from primary";
}},
+  {// Template Type Parameter
+   R"cpp(
+  template  void foo();
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "T";
+ HI.Kind = index::SymbolKind::TemplateTypeParm;
+ HI.NamespaceScope = "";
+ HI.Definition = "typename T = int";
+ HI.LocalScope = "foo::";
+ HI.Type = "typename";
+   }},
+  {// TemplateTemplate Type Parameter
+   R"cpp(
+  template  class [[^T]]> void foo();
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "T";
+ HI.Kind = index::SymbolKind::TemplateTemplateParm;
+ HI.NamespaceScope = "";
+ HI.Definition = "template  class T";
+ HI.LocalScope = "foo::";
+ HI.Type = "template  class";
+   }},
+  {// NonType Template Parameter
+   R"cpp(
+  template  void foo();
+  )cpp",
+   [](HoverInfo &HI) {
+ H

[PATCH] D73696: [clang][Index] Introduce a TemplateParm SymbolKind

2020-02-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Protocol.cpp:265
   case index::SymbolKind::Parameter:
+  case index::SymbolKind::TemplateParm:
 return SymbolKind::Variable;

sammccall wrote:
> this seems kind of dubious, maybe worth a comment?
> (If we had the decl here we'd distinguish between type and non-type, right?)
we have all 3 of them now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73696



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


[PATCH] D73350: [analyzer] Small StreamChecker refactoring (NFC).

2020-02-07 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 243092.
balazske added a comment.

Reformat.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73350

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


Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -135,31 +135,32 @@
 }
 
 void StreamChecker::evalFopen(const CallEvent &Call, CheckerContext &C) const {
-  ProgramStateRef state = C.getState();
-  SValBuilder &svalBuilder = C.getSValBuilder();
+  ProgramStateRef State = C.getState();
+  SValBuilder &SVB = C.getSValBuilder();
   const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+
   auto *CE = dyn_cast_or_null(Call.getOriginExpr());
   if (!CE)
 return;
 
-  DefinedSVal RetVal =
-  svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
-  .castAs();
-  state = state->BindExpr(CE, C.getLocationContext(), RetVal);
+  DefinedSVal RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
+   .castAs();
+  SymbolRef RetSym = RetVal.getAsSymbol();
+  assert(RetSym && "RetVal must be a symbol here.");
+
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
 
-  ConstraintManager &CM = C.getConstraintManager();
   // Bifurcate the state into two: one with a valid FILE* pointer, the other
   // with a NULL.
-  ProgramStateRef stateNotNull, stateNull;
-  std::tie(stateNotNull, stateNull) = CM.assumeDual(state, RetVal);
+  ProgramStateRef StateNotNull, StateNull;
+  std::tie(StateNotNull, StateNull) =
+  C.getConstraintManager().assumeDual(State, RetVal);
 
-  SymbolRef Sym = RetVal.getAsSymbol();
-  assert(Sym && "RetVal must be a symbol here.");
-  stateNotNull = stateNotNull->set(Sym, StreamState::getOpened());
-  stateNull = stateNull->set(Sym, StreamState::getOpenFailed());
+  StateNotNull = StateNotNull->set(RetSym, 
StreamState::getOpened());
+  StateNull = StateNull->set(RetSym, StreamState::getOpenFailed());
 
-  C.addTransition(stateNotNull);
-  C.addTransition(stateNull);
+  C.addTransition(StateNotNull);
+  C.addTransition(StateNull);
 }
 
 void StreamChecker::evalFreopen(const CallEvent &Call,
@@ -228,8 +229,6 @@
 
   if (!C.isDifferent() && StateChanged)
 C.addTransition(State);
-
-  return;
 }
 
 void StreamChecker::checkArgNullStream(const CallEvent &Call, CheckerContext 
&C,


Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -135,31 +135,32 @@
 }
 
 void StreamChecker::evalFopen(const CallEvent &Call, CheckerContext &C) const {
-  ProgramStateRef state = C.getState();
-  SValBuilder &svalBuilder = C.getSValBuilder();
+  ProgramStateRef State = C.getState();
+  SValBuilder &SVB = C.getSValBuilder();
   const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+
   auto *CE = dyn_cast_or_null(Call.getOriginExpr());
   if (!CE)
 return;
 
-  DefinedSVal RetVal =
-  svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
-  .castAs();
-  state = state->BindExpr(CE, C.getLocationContext(), RetVal);
+  DefinedSVal RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
+   .castAs();
+  SymbolRef RetSym = RetVal.getAsSymbol();
+  assert(RetSym && "RetVal must be a symbol here.");
+
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
 
-  ConstraintManager &CM = C.getConstraintManager();
   // Bifurcate the state into two: one with a valid FILE* pointer, the other
   // with a NULL.
-  ProgramStateRef stateNotNull, stateNull;
-  std::tie(stateNotNull, stateNull) = CM.assumeDual(state, RetVal);
+  ProgramStateRef StateNotNull, StateNull;
+  std::tie(StateNotNull, StateNull) =
+  C.getConstraintManager().assumeDual(State, RetVal);
 
-  SymbolRef Sym = RetVal.getAsSymbol();
-  assert(Sym && "RetVal must be a symbol here.");
-  stateNotNull = stateNotNull->set(Sym, StreamState::getOpened());
-  stateNull = stateNull->set(Sym, StreamState::getOpenFailed());
+  StateNotNull = StateNotNull->set(RetSym, StreamState::getOpened());
+  StateNull = StateNull->set(RetSym, StreamState::getOpenFailed());
 
-  C.addTransition(stateNotNull);
-  C.addTransition(stateNull);
+  C.addTransition(StateNotNull);
+  C.addTransition(StateNull);
 }
 
 void StreamChecker::evalFreopen(const CallEvent &Call,
@@ -228,8 +229,6 @@
 
   if (!C.isDifferent() && StateChanged)
 C.addTransition(State);
-
-  return;
 }
 
 void StreamChecker::checkArgNullStream(const CallEvent &Call, CheckerContext &C,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ht

[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

2020-02-07 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

I genuinely think that in the following case we should warn, since the user 
already had a chance to express the range assumption using an `assert`.

I think that regardless which checker in what condition checks for a given 
constraint.
If the expression is tainted, we should warn each cases if the constraint 
cannot be proven.
If that is NOT tainted, we should conservatively assume that the precondition 
is satisfied.

---

**PS**: after checking the exploded graph for the following example, I 
recognized that the range based constraint solver is not smart enough to prove 
that `x` must be in range.
Even if we express the necessary information using asserts.
I'm not so sure about warning for this case, after seeing this :|

  int scanf(const char *restrict format, ...);
  void clang_analyzer_eval(int);
  
  extern void __assert_fail (__const char *__assertion, __const char *__file,
  unsigned int __line, __const char *__function)
   __attribute__ ((__noreturn__));
  #define assert(expr) \
((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
  
  
  void foo(int y, int z) {
assert(y <= 10);
assert(z >= 20);
int x;
scanf("%d", &x);
if (x < y || x > z)
  return;
  
// x should be in range [10, 20]
clang_analyzer_eval(0 <= x && x < 256);
  
// we want to warn if x is not proven to be in that range
// mySink(x); // requires x to be in [0, 255]
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73536



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


[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D72705#1861750 , @balazske wrote:

> Uploading new diff with added comments and renaming.


Great, very much appreciated! It took a loong time, but I think I got whats 
going on. So, step by step, this is what we're going if a function of interest 
is called:

1. Put the return value in a map because we need to keep an eye out for it.
2. If that value is ever read (which we'll know about in `checkLocation`), we 
will thoroughly check that statement whether it
  1. Qualifies as a check for an error, in which case we take the return value 
out of the map
  2. Is a read, in which case we emit a warning
  3. Is an assignment (like `for (P = aligned_alloc(4, 8); P;)` or `  void *P = 
aligned_alloc(4, 8);`), in which case we jump to step 2 with the current 
statement's parent in the AST.
  4. Does nothing, in which case we leave it in the map for now.

That is the essence of what's going on, is that correct?

> The code is at least saved here, can be used as starting point for smaller 
> changes.

Thanks! For now, lets discuss how the current solution works, let other 
reviewers take a look at the high level idea, and after reaching some 
consensus, try to see how we can split this patch up into logical chunks.

In D72705#1859711 , @balazske wrote:

> Generally, is it a problem if there are multiple checkers that may detect 
> same defects but with different approach?


If avoidable, yes. If not, I think its an acceptable sacrifice.

> Or should the code of this checker (for example) made more complicated only 
> to filter out cases that may be detected by other checkers too?

My point was different. Are we sure that it is an error not to check the error 
value here, before going out of scope?:

  void test_Null_UnusedVar() {
void *P = aligned_alloc(4, 8);
  } // expected-warning{{Return value was not checked}}

I'm not debating whether this core is incorrect, only the fact that not 
checking the return value //isn't// the issue here. In any case, let's drop 
this topic for now, because it adds very little to the real point of this 
patch, seeing how the problem should be solved as a whole. We should revisit 
this specific case in a small, followup patch.




Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:220-223
+/// Information about a specific function call that has an error return code to
+/// check. This data is stored in a map and indexed by the SymbolRef that 
stands
+/// for the result of the function call.
+struct CalledFunctionData {

The comments leave me to believe that the correct class name would be 
`FunctionCallData`, right? Because `FnInfo` does the job of describing the 
called function, and this one deals with the actual call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705



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


[PATCH] D74117: [AArch64][SVE] SVE2 intrinsics for character match & histogram generation

2020-02-07 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes accepted this revision.
c-rhodes added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74117



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


[clang] 39f50da - Support -fstack-clash-protection for x86

2020-02-07 Thread via cfe-commits

Author: serge_sans_paille
Date: 2020-02-07T10:56:15+01:00
New Revision: 39f50da2a357a8f685b3540246c5d762734e035f

URL: 
https://github.com/llvm/llvm-project/commit/39f50da2a357a8f685b3540246c5d762734e035f
DIFF: 
https://github.com/llvm/llvm-project/commit/39f50da2a357a8f685b3540246c5d762734e035f.diff

LOG: Support -fstack-clash-protection for x86

Implement protection against the stack clash attack [0] through inline stack
probing.

Probe stack allocation every PAGE_SIZE during frame lowering or dynamic
allocation to make sure the page guard, if any, is touched when touching the
stack, in a similar manner to GCC[1].

This extends the existing `probe-stack' mechanism with a special value 
`inline-asm'.
Technically the former uses function call before stack allocation while this
patch provides inlined stack probes and chunk allocation.

Only implemented for x86.

[0] https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt
[1] https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00556.html

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

Added: 
clang/test/CodeGen/stack-clash-protection.c
clang/test/Driver/stack-clash-protection.c
llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
llvm/test/CodeGen/X86/stack-clash-large.ll
llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
llvm/test/CodeGen/X86/stack-clash-medium.ll
llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
llvm/test/CodeGen/X86/stack-clash-small.ll
llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/include/clang/Basic/TargetInfo.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/X86.h
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/lib/Target/X86/X86CallFrameOptimization.cpp
llvm/lib/Target/X86/X86FrameLowering.cpp
llvm/lib/Target/X86/X86FrameLowering.h
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.h
llvm/lib/Target/X86/X86InstrCompiler.td
llvm/lib/Target/X86/X86InstrInfo.td

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 24c8ee2bc9ef..609e7fa66c00 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -1917,6 +1917,10 @@ Use a strong heuristic to apply stack protectors to 
functions
 
 Emit section containing metadata on function stack sizes
 
+.. option:: -fstack-clash-protection, -fno-stack-clash-protection
+
+Instrument stack allocation to prevent stack clash attacks (x86, non-Windows 
only).
+
 .. option:: -fstandalone-debug, -fno-limit-debug-info, -fno-standalone-debug
 
 Emit full debug info for all types used by the program

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cb8d73932bf6..856effd19718 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -61,6 +61,10 @@ New Compiler Flags
 --
 
 
+- -fstack-clash-protection will provide a protection against the stack clash
+  attack for x86 architecture through automatic probing of each page of
+  allocated stack.
+
 Deprecated Compiler Flags
 -
 

diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index be905a4ed852..d1062f3c7b04 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -150,6 +150,7 @@ CODEGENOPT(NoWarn, 1, 0) ///< Set when 
-Wa,--no-warn is enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.
 CODEGENOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain
  ///< inline line tables.
+CODEGENOPT(StackClashProtector, 1, 0) ///< Set when -fstack-clash-protection 
is enabled.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is 
enabled.
 CODEGENOPT(NoInfsFPMath  , 1, 0) ///< Assume FP arguments, results not 
+-Inf.
 CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP 
zero

diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td 
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 20b49605eb6f..95cb81f09922 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -239,6 +239,10 @@ def note_invalid_subexpr

[PATCH] D68720: Support -fstack-clash-protection for x86

2020-02-07 Thread serge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG39f50da2a357: Support -fstack-clash-protection for x86 
(authored by serge-sans-paille).

Changed prior to commit:
  https://reviews.llvm.org/D68720?vs=240844&id=243101#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll
  llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Index: llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg);
+
+define void @foo() local_unnamed_addr #0 {
+
+;CHECK-LABEL: foo:
+;CHECK: # %bb.0:
+;CHECK-NEXT:	subq	$4096, %rsp # imm = 0x1000
+; it's important that we don't use the call as a probe here
+;CHECK-NEXT:	movq	$0, (%rsp)
+;CHECK-NEXT:	subq	$3912, %rsp # imm = 0xF48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8016
+;CHECK-NEXT:	movq	%rsp, %rdi
+;CHECK-NEXT:	movl	$8000, %edx # imm = 0x1F40
+;CHECK-NEXT:	xorl	%esi, %esi
+;CHECK-NEXT:	callq	memset
+;CHECK-NEXT:	addq	$8008, %rsp # imm = 0x1F48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8
+;CHECK-NEXT:	retq
+
+  %a = alloca i8, i64 8000, align 16
+  call void @llvm.memset.p0i8.i64(i8* align 16 %a, i8 0, i64 8000, i1 false)
+  ret void
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll

[PATCH] D67399: [ARM] Follow AACPS for preserving number of loads/stores of volatile bit-fields

2020-02-07 Thread Diogo N. Sampaio via Phabricator via cfe-commits
dnsampaio updated this revision to Diff 243102.
dnsampaio added a comment.

Revordered some tests adding and definition of the "isAAPCS" function from 
patch D72932 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67399

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/aapcs-bitfield.c

Index: clang/test/CodeGen/aapcs-bitfield.c
===
--- clang/test/CodeGen/aapcs-bitfield.c
+++ clang/test/CodeGen/aapcs-bitfield.c
@@ -1,6 +1,8 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 | FileCheck %s -check-prefix=LE
 // RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 | FileCheck %s -check-prefix=BE
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad | FileCheck %s -check-prefixes=LE,LENUMLOADS
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad | FileCheck %s -check-prefixes=BE,BENUMLOADS
 
 struct st0 {
   short c : 7;
@@ -144,7 +146,7 @@
 void st2_check_store(struct st2 *m) {
   m->c = 1;
 }
-
+// Volatile access is allowed to use 16 bits
 struct st3 {
   volatile short c : 7;
 };
@@ -191,7 +193,7 @@
 void st3_check_store(struct st3 *m) {
   m->c = 1;
 }
-
+// Volatile access to st4.c should use a char ld/st
 struct st4 {
   int b : 9;
   volatile char c : 5;
@@ -324,16 +326,16 @@
 // LE-LABEL: @st6_check_load(
 // LE-NEXT:  entry:
 // LE-NEXT:[[TMP0:%.*]] = getelementptr [[STRUCT_ST6:%.*]], %struct.st6* [[M:%.*]], i32 0, i32 0
-// LE-NEXT:[[BF_LOAD:%.*]] = load i16, i16* [[TMP0]], align 4
+// LE-NEXT:[[BF_LOAD:%.*]] = load volatile i16, i16* [[TMP0]], align 4
 // LE-NEXT:[[BF_SHL:%.*]] = shl i16 [[BF_LOAD]], 4
 // LE-NEXT:[[BF_ASHR:%.*]] = ashr exact i16 [[BF_SHL]], 4
 // LE-NEXT:[[BF_CAST:%.*]] = sext i16 [[BF_ASHR]] to i32
 // LE-NEXT:[[B:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 1
-// LE-NEXT:[[TMP1:%.*]] = load i8, i8* [[B]], align 2, !tbaa !3
+// LE-NEXT:[[TMP1:%.*]] = load volatile i8, i8* [[B]], align 2
 // LE-NEXT:[[CONV:%.*]] = sext i8 [[TMP1]] to i32
 // LE-NEXT:[[ADD:%.*]] = add nsw i32 [[BF_CAST]], [[CONV]]
 // LE-NEXT:[[C:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 2
-// LE-NEXT:[[BF_LOAD1:%.*]] = load i8, i8* [[C]], align 1
+// LE-NEXT:[[BF_LOAD1:%.*]] = load volatile i8, i8* [[C]], align 1
 // LE-NEXT:[[BF_SHL2:%.*]] = shl i8 [[BF_LOAD1]], 3
 // LE-NEXT:[[BF_ASHR3:%.*]] = ashr exact i8 [[BF_SHL2]], 3
 // LE-NEXT:[[BF_CAST4:%.*]] = sext i8 [[BF_ASHR3]] to i32
@@ -343,21 +345,21 @@
 // BE-LABEL: @st6_check_load(
 // BE-NEXT:  entry:
 // BE-NEXT:[[TMP0:%.*]] = getelementptr [[STRUCT_ST6:%.*]], %struct.st6* [[M:%.*]], i32 0, i32 0
-// BE-NEXT:[[BF_LOAD:%.*]] = load i16, i16* [[TMP0]], align 4
+// BE-NEXT:[[BF_LOAD:%.*]] = load volatile i16, i16* [[TMP0]], align 4
 // BE-NEXT:[[BF_ASHR:%.*]] = ashr i16 [[BF_LOAD]], 4
 // BE-NEXT:[[BF_CAST:%.*]] = sext i16 [[BF_ASHR]] to i32
 // BE-NEXT:[[B:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 1
-// BE-NEXT:[[TMP1:%.*]] = load i8, i8* [[B]], align 2, !tbaa !3
+// BE-NEXT:[[TMP1:%.*]] = load volatile i8, i8* [[B]], align 2
 // BE-NEXT:[[CONV:%.*]] = sext i8 [[TMP1]] to i32
 // BE-NEXT:[[ADD:%.*]] = add nsw i32 [[BF_CAST]], [[CONV]]
 // BE-NEXT:[[C:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 2
-// BE-NEXT:[[BF_LOAD1:%.*]] = load i8, i8* [[C]], align 1
+// BE-NEXT:[[BF_LOAD1:%.*]] = load volatile i8, i8* [[C]], align 1
 // BE-NEXT:[[BF_ASHR2:%.*]] = ashr i8 [[BF_LOAD1]], 3
 // BE-NEXT:[[BF_CAST3:%.*]] = sext i8 [[BF_ASHR2]] to i32
 // BE-NEXT:[[ADD4:%.*]] = add nsw i32 [[ADD]], [[BF_CAST3]]
 // BE-NEXT:ret i32 [[ADD4]]
 //
-int st6_check_load(struct st6 *m) {
+int st6_check_load(volatile struct st6 *m) {
   int x = m->a;
   x += m->b;
   x += m->c;
@@ -372,7 +374,7 @@
 // LE-NEXT:[[BF_SET:%.*]] = or i16 [[BF_CLEAR]], 1
 // LE-NEXT:store i16 [[BF_SET]], i16* [[TMP0]], align 4
 // LE-NEXT:[[B:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 1
-// LE-NEXT:store i8 2, i8* [[B]], align 2, !tbaa !3
+// LE-NEXT:store i8 2, i8* [[B]], align 2
 // LE-NEXT:[[C:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 2
 // LE-NEXT:[[BF_LOAD1:%.*]] = load i8, i8* [[C]], align 1
 // LE-NEXT:[[BF_CLEAR2:%.*]] = and i8 [[BF_LOAD1]], -32
@@ -388,7 +390,7 @@
 // BE-NEXT:[[BF_SET:%.*]] = or i16 [[BF_CLEAR]], 

[PATCH] D67399: [ARM] Follow AACPS for preserving number of loads/stores of volatile bit-fields

2020-02-07 Thread Diogo N. Sampaio via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d869180c4ad: [ARM] Follow AACPS for preserving number of 
loads/stores of volatile bit-fields (authored by dnsampaio).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67399

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/aapcs-bitfield.c

Index: clang/test/CodeGen/aapcs-bitfield.c
===
--- clang/test/CodeGen/aapcs-bitfield.c
+++ clang/test/CodeGen/aapcs-bitfield.c
@@ -1,6 +1,8 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 | FileCheck %s -check-prefix=LE
 // RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 | FileCheck %s -check-prefix=BE
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad | FileCheck %s -check-prefixes=LE,LENUMLOADS
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad | FileCheck %s -check-prefixes=BE,BENUMLOADS
 
 struct st0 {
   short c : 7;
@@ -144,7 +146,7 @@
 void st2_check_store(struct st2 *m) {
   m->c = 1;
 }
-
+// Volatile access is allowed to use 16 bits
 struct st3 {
   volatile short c : 7;
 };
@@ -191,7 +193,7 @@
 void st3_check_store(struct st3 *m) {
   m->c = 1;
 }
-
+// Volatile access to st4.c should use a char ld/st
 struct st4 {
   int b : 9;
   volatile char c : 5;
@@ -324,16 +326,16 @@
 // LE-LABEL: @st6_check_load(
 // LE-NEXT:  entry:
 // LE-NEXT:[[TMP0:%.*]] = getelementptr [[STRUCT_ST6:%.*]], %struct.st6* [[M:%.*]], i32 0, i32 0
-// LE-NEXT:[[BF_LOAD:%.*]] = load i16, i16* [[TMP0]], align 4
+// LE-NEXT:[[BF_LOAD:%.*]] = load volatile i16, i16* [[TMP0]], align 4
 // LE-NEXT:[[BF_SHL:%.*]] = shl i16 [[BF_LOAD]], 4
 // LE-NEXT:[[BF_ASHR:%.*]] = ashr exact i16 [[BF_SHL]], 4
 // LE-NEXT:[[BF_CAST:%.*]] = sext i16 [[BF_ASHR]] to i32
 // LE-NEXT:[[B:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 1
-// LE-NEXT:[[TMP1:%.*]] = load i8, i8* [[B]], align 2, !tbaa !3
+// LE-NEXT:[[TMP1:%.*]] = load volatile i8, i8* [[B]], align 2
 // LE-NEXT:[[CONV:%.*]] = sext i8 [[TMP1]] to i32
 // LE-NEXT:[[ADD:%.*]] = add nsw i32 [[BF_CAST]], [[CONV]]
 // LE-NEXT:[[C:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 2
-// LE-NEXT:[[BF_LOAD1:%.*]] = load i8, i8* [[C]], align 1
+// LE-NEXT:[[BF_LOAD1:%.*]] = load volatile i8, i8* [[C]], align 1
 // LE-NEXT:[[BF_SHL2:%.*]] = shl i8 [[BF_LOAD1]], 3
 // LE-NEXT:[[BF_ASHR3:%.*]] = ashr exact i8 [[BF_SHL2]], 3
 // LE-NEXT:[[BF_CAST4:%.*]] = sext i8 [[BF_ASHR3]] to i32
@@ -343,21 +345,21 @@
 // BE-LABEL: @st6_check_load(
 // BE-NEXT:  entry:
 // BE-NEXT:[[TMP0:%.*]] = getelementptr [[STRUCT_ST6:%.*]], %struct.st6* [[M:%.*]], i32 0, i32 0
-// BE-NEXT:[[BF_LOAD:%.*]] = load i16, i16* [[TMP0]], align 4
+// BE-NEXT:[[BF_LOAD:%.*]] = load volatile i16, i16* [[TMP0]], align 4
 // BE-NEXT:[[BF_ASHR:%.*]] = ashr i16 [[BF_LOAD]], 4
 // BE-NEXT:[[BF_CAST:%.*]] = sext i16 [[BF_ASHR]] to i32
 // BE-NEXT:[[B:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 1
-// BE-NEXT:[[TMP1:%.*]] = load i8, i8* [[B]], align 2, !tbaa !3
+// BE-NEXT:[[TMP1:%.*]] = load volatile i8, i8* [[B]], align 2
 // BE-NEXT:[[CONV:%.*]] = sext i8 [[TMP1]] to i32
 // BE-NEXT:[[ADD:%.*]] = add nsw i32 [[BF_CAST]], [[CONV]]
 // BE-NEXT:[[C:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 2
-// BE-NEXT:[[BF_LOAD1:%.*]] = load i8, i8* [[C]], align 1
+// BE-NEXT:[[BF_LOAD1:%.*]] = load volatile i8, i8* [[C]], align 1
 // BE-NEXT:[[BF_ASHR2:%.*]] = ashr i8 [[BF_LOAD1]], 3
 // BE-NEXT:[[BF_CAST3:%.*]] = sext i8 [[BF_ASHR2]] to i32
 // BE-NEXT:[[ADD4:%.*]] = add nsw i32 [[ADD]], [[BF_CAST3]]
 // BE-NEXT:ret i32 [[ADD4]]
 //
-int st6_check_load(struct st6 *m) {
+int st6_check_load(volatile struct st6 *m) {
   int x = m->a;
   x += m->b;
   x += m->c;
@@ -372,7 +374,7 @@
 // LE-NEXT:[[BF_SET:%.*]] = or i16 [[BF_CLEAR]], 1
 // LE-NEXT:store i16 [[BF_SET]], i16* [[TMP0]], align 4
 // LE-NEXT:[[B:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 1
-// LE-NEXT:store i8 2, i8* [[B]], align 2, !tbaa !3
+// LE-NEXT:store i8 2, i8* [[B]], align 2
 // LE-NEXT:[[C:%.*]] = getelementptr inbounds [[STRUCT_ST6]], %struct.st6* [[M]], i32 0, i32 2
 // LE-NEXT:[[BF_LOAD1:%.*]] = load i8, i8* [[C]], align 1
 // LE-NEXT:[[BF_CLEAR2:%.*]] = and i8 [[BF_LOAD1]], -32
@@ -388,7 +390,7 @@
 // BE-NEXT:[[BF_SET:%.*]] = or i16 [

[PATCH] D68720: Support -fstack-clash-protection for x86

2020-02-07 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

This seems to be breaking the buildbots, e.g.
http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/33475/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Astack-clash-protection.c
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/62154/steps/test-check-all/logs/stdio
http://45.33.8.238/win/7706/step_7.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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


[clang] f6d9842 - Revert "Support -fstack-clash-protection for x86"

2020-02-07 Thread via cfe-commits

Author: serge-sans-paille
Date: 2020-02-07T11:36:53+01:00
New Revision: f6d98429fcdba97988fa1e3ec10dc2ca943dd0da

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

LOG: Revert "Support -fstack-clash-protection for x86"

This reverts commit 39f50da2a357a8f685b3540246c5d762734e035f.

The -fstack-clash-protection is being passed to the linker too, which
is not intended.

Reverting and fixing that in a later commit.

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/include/clang/Basic/TargetInfo.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/X86.h
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/lib/Target/X86/X86CallFrameOptimization.cpp
llvm/lib/Target/X86/X86FrameLowering.cpp
llvm/lib/Target/X86/X86FrameLowering.h
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.h
llvm/lib/Target/X86/X86InstrCompiler.td
llvm/lib/Target/X86/X86InstrInfo.td

Removed: 
clang/test/CodeGen/stack-clash-protection.c
clang/test/Driver/stack-clash-protection.c
llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
llvm/test/CodeGen/X86/stack-clash-large.ll
llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
llvm/test/CodeGen/X86/stack-clash-medium.ll
llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
llvm/test/CodeGen/X86/stack-clash-small.ll
llvm/test/CodeGen/X86/stack-clash-unknown-call.ll



diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 609e7fa66c00..24c8ee2bc9ef 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -1917,10 +1917,6 @@ Use a strong heuristic to apply stack protectors to 
functions
 
 Emit section containing metadata on function stack sizes
 
-.. option:: -fstack-clash-protection, -fno-stack-clash-protection
-
-Instrument stack allocation to prevent stack clash attacks (x86, non-Windows 
only).
-
 .. option:: -fstandalone-debug, -fno-limit-debug-info, -fno-standalone-debug
 
 Emit full debug info for all types used by the program

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 856effd19718..cb8d73932bf6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -61,10 +61,6 @@ New Compiler Flags
 --
 
 
-- -fstack-clash-protection will provide a protection against the stack clash
-  attack for x86 architecture through automatic probing of each page of
-  allocated stack.
-
 Deprecated Compiler Flags
 -
 

diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 48c0df49e32d..21e391912584 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -150,7 +150,6 @@ CODEGENOPT(NoWarn, 1, 0) ///< Set when 
-Wa,--no-warn is enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.
 CODEGENOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain
  ///< inline line tables.
-CODEGENOPT(StackClashProtector, 1, 0) ///< Set when -fstack-clash-protection 
is enabled.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is 
enabled.
 CODEGENOPT(NoInfsFPMath  , 1, 0) ///< Assume FP arguments, results not 
+-Inf.
 CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP 
zero

diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td 
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 95cb81f09922..20b49605eb6f 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -239,10 +239,6 @@ def note_invalid_subexpr_in_const_expr : Note<
 let CategoryName = "Inline Assembly Issue" in {
   def err_asm_invalid_type_in_input : Error<
 "invalid type %0 in asm input for constraint '%1'">;
-
-  def warn_stack_clash_protection_inline_asm : Warning<
-"Unable to protect inline asm that clobbers stack pointer against stack 
clash">,
-InGroup>;
 }
 
 // Sema && Serialization

diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index cb5aabdc468f..3a8e35524695 10064

[clang] 74734e8 - Fix docs and comments for max_tokens_total pragma

2020-02-07 Thread Hans Wennborg via cfe-commits

Author: Hans Wennborg
Date: 2020-02-07T11:37:14+01:00
New Revision: 74734e809ac778beb01776ee207643184c09c2a0

URL: 
https://github.com/llvm/llvm-project/commit/74734e809ac778beb01776ee207643184c09c2a0
DIFF: 
https://github.com/llvm/llvm-project/commit/74734e809ac778beb01776ee207643184c09c2a0.diff

LOG: Fix docs and comments for max_tokens_total pragma

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/lib/Parse/ParsePragma.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 2dcedcb60340..a2bc29986a07 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1178,7 +1178,7 @@ the token limit, which can be set in three ways:
which works like and overrides the ``-fmax-tokens=`` flag:
 
.. code-block: c++
-  #pragma clang max_file_tokens 1234
+  #pragma clang max_tokens_total 1234
 
 These limits can be helpful in limiting code growth through included files.
 

diff  --git a/clang/lib/Parse/ParsePragma.cpp b/clang/lib/Parse/ParsePragma.cpp
index f2b0d39496d2..34232437528b 100644
--- a/clang/lib/Parse/ParsePragma.cpp
+++ b/clang/lib/Parse/ParsePragma.cpp
@@ -3336,7 +3336,7 @@ void 
PragmaMaxTokensHereHandler::HandlePragma(Preprocessor &PP,
   }
 }
 
-// Handle '#pragma clang max_file_tokens 12345'.
+// Handle '#pragma clang max_tokens_total 12345'.
 void PragmaMaxTokensTotalHandler::HandlePragma(Preprocessor &PP,
PragmaIntroducer Introducer,
Token &Tok) {



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


[PATCH] D73350: [analyzer] Small StreamChecker refactoring (NFC).

2020-02-07 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc4f0f8ec41fd: [analyzer] Small StreamChecker refactoring 
(NFC). (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73350

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


Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -135,31 +135,32 @@
 }
 
 void StreamChecker::evalFopen(const CallEvent &Call, CheckerContext &C) const {
-  ProgramStateRef state = C.getState();
-  SValBuilder &svalBuilder = C.getSValBuilder();
+  ProgramStateRef State = C.getState();
+  SValBuilder &SVB = C.getSValBuilder();
   const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+
   auto *CE = dyn_cast_or_null(Call.getOriginExpr());
   if (!CE)
 return;
 
-  DefinedSVal RetVal =
-  svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
-  .castAs();
-  state = state->BindExpr(CE, C.getLocationContext(), RetVal);
+  DefinedSVal RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
+   .castAs();
+  SymbolRef RetSym = RetVal.getAsSymbol();
+  assert(RetSym && "RetVal must be a symbol here.");
+
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
 
-  ConstraintManager &CM = C.getConstraintManager();
   // Bifurcate the state into two: one with a valid FILE* pointer, the other
   // with a NULL.
-  ProgramStateRef stateNotNull, stateNull;
-  std::tie(stateNotNull, stateNull) = CM.assumeDual(state, RetVal);
+  ProgramStateRef StateNotNull, StateNull;
+  std::tie(StateNotNull, StateNull) =
+  C.getConstraintManager().assumeDual(State, RetVal);
 
-  SymbolRef Sym = RetVal.getAsSymbol();
-  assert(Sym && "RetVal must be a symbol here.");
-  stateNotNull = stateNotNull->set(Sym, StreamState::getOpened());
-  stateNull = stateNull->set(Sym, StreamState::getOpenFailed());
+  StateNotNull = StateNotNull->set(RetSym, 
StreamState::getOpened());
+  StateNull = StateNull->set(RetSym, StreamState::getOpenFailed());
 
-  C.addTransition(stateNotNull);
-  C.addTransition(stateNull);
+  C.addTransition(StateNotNull);
+  C.addTransition(StateNull);
 }
 
 void StreamChecker::evalFreopen(const CallEvent &Call,
@@ -228,8 +229,6 @@
 
   if (!C.isDifferent() && StateChanged)
 C.addTransition(State);
-
-  return;
 }
 
 void StreamChecker::checkArgNullStream(const CallEvent &Call, CheckerContext 
&C,


Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -135,31 +135,32 @@
 }
 
 void StreamChecker::evalFopen(const CallEvent &Call, CheckerContext &C) const {
-  ProgramStateRef state = C.getState();
-  SValBuilder &svalBuilder = C.getSValBuilder();
+  ProgramStateRef State = C.getState();
+  SValBuilder &SVB = C.getSValBuilder();
   const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+
   auto *CE = dyn_cast_or_null(Call.getOriginExpr());
   if (!CE)
 return;
 
-  DefinedSVal RetVal =
-  svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
-  .castAs();
-  state = state->BindExpr(CE, C.getLocationContext(), RetVal);
+  DefinedSVal RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount())
+   .castAs();
+  SymbolRef RetSym = RetVal.getAsSymbol();
+  assert(RetSym && "RetVal must be a symbol here.");
+
+  State = State->BindExpr(CE, C.getLocationContext(), RetVal);
 
-  ConstraintManager &CM = C.getConstraintManager();
   // Bifurcate the state into two: one with a valid FILE* pointer, the other
   // with a NULL.
-  ProgramStateRef stateNotNull, stateNull;
-  std::tie(stateNotNull, stateNull) = CM.assumeDual(state, RetVal);
+  ProgramStateRef StateNotNull, StateNull;
+  std::tie(StateNotNull, StateNull) =
+  C.getConstraintManager().assumeDual(State, RetVal);
 
-  SymbolRef Sym = RetVal.getAsSymbol();
-  assert(Sym && "RetVal must be a symbol here.");
-  stateNotNull = stateNotNull->set(Sym, StreamState::getOpened());
-  stateNull = stateNull->set(Sym, StreamState::getOpenFailed());
+  StateNotNull = StateNotNull->set(RetSym, StreamState::getOpened());
+  StateNull = StateNull->set(RetSym, StreamState::getOpenFailed());
 
-  C.addTransition(stateNotNull);
-  C.addTransition(stateNull);
+  C.addTransition(StateNotNull);
+  C.addTransition(StateNull);
 }
 
 void StreamChecker::evalFreopen(const CallEvent &Call,
@@ -228,8 +229,6 @@
 
   if (!C.isDifferent() && StateChanged)
 C.addTransition(State);
-
-  return;
 }
 
 void StreamChecker::checkArgNullStream(const CallEvent &Call, CheckerContext &C,
_

[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

2020-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 3 inline comments as done.
martong added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:537-551
+  // The format is as follows:
   //{ "function name",
-  //  { spec:
+  //  { variant0:
   //{ argument types list, ... },
-  //return type, purity, { range set list:
+  //return type, purity, { specification list:
   //  { range list:
   //{ argument index, within or out of, {{from, to}, ...} },

NoQ wrote:
> I suspect that this comment would need a lot more updates.
Could you please elaborate? Do you mean to add comments e.g. to 
`ArgumentCondition` and the rest below? Or to rewrite the above comment?



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:553
   //}
+  using Summary = FunctionSummaryTy;
+  auto ArgumentCondition = [](ArgNoTy ArgNo, ValueRangeKindTy Kind,

NoQ wrote:
> There seems to be a lot of inconsistency in the use of `T`, `Ty`, and 
> lack-of-suffix.
> 
> I'd prefer to have one of them reserved for `QualType`s (eg., `IntTy`).
Ok, I've renamed every type with the `Ty` suffix to not have any suffix. `Ty` 
is reserved for variables now whose type is QualType, e.g. IntTy.
Removed the `T` suffix too from the lambdas that can generate summaries (e.g. 
`Getc`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897



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


[PATCH] D73891: [RISCV] Support experimental/unratified extensions

2020-02-07 Thread Simon Cook via Phabricator via cfe-commits
simoncook planned changes to this revision.
simoncook added a comment.

With the update to D69987  adding the `Zvqmac` 
predicate, it seems both the `b` and `v` extensions have Z extensions that also 
need supporting using this method, I'll update this to also support Z shortly, 
but don't expect the general method to change, just adding the same version 
check elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73891



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


[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

2020-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 243117.
martong marked an inline comment as done.
martong added a comment.

- Rename several types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897

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

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -9,7 +9,7 @@
 // This checker improves modeling of a few simple library functions.
 // It does not generate warnings.
 //
-// This checker provides a specification format - `FunctionSummaryTy' - and
+// This checker provides a specification format - `Summary' - and
 // contains descriptions of some library functions in this format. Each
 // specification contains a list of branches for splitting the program state
 // upon call, and range constraints on argument and return-value symbols that
@@ -21,7 +21,7 @@
 // consider standard C function `ispunct(int x)', which returns a non-zero value
 // iff `x' is a punctuation character, that is, when `x' is in range
 //   ['!', '/']   [':', '@']  U  ['[', '\`']  U  ['{', '~'].
-// `FunctionSummaryTy' provides only two branches for this function. However,
+// `Summary' provides only two branches for this function. However,
 // any attempt to describe this range with if-statements in the body farm
 // would result in many more branches. Because each branch needs to be analyzed
 // independently, this significantly reduces performance. Additionally,
@@ -31,7 +31,7 @@
 // was not consciously intended, and therefore it might have been unreachable.
 //
 // This checker uses eval::Call for modeling "pure" functions, for which
-// their `FunctionSummaryTy' is a precise model. This avoids unnecessary
+// their `Summary' is a precise model. This avoids unnecessary
 // invalidation passes. Conflicts with other checkers are unlikely because
 // if the function has no other effects, other checkers would probably never
 // want to improve upon the modeling done by this checker.
@@ -64,49 +64,48 @@
   /// Below is a series of typedefs necessary to define function specs.
   /// We avoid nesting types here because each additional qualifier
   /// would need to be repeated in every function spec.
-  struct FunctionSummaryTy;
+  struct Summary;
 
   /// Specify how much the analyzer engine should entrust modeling this function
   /// to us. If he doesn't, he performs additional invalidations.
-  enum InvalidationKindTy { NoEvalCall, EvalCallAsPure };
+  enum InvalidationKind { NoEvalCall, EvalCallAsPure };
 
-  /// A pair of ValueRangeKindTy and IntRangeVectorTy would describe a range
+  /// A pair of ValueRangeKind and IntRangeVector would describe a range
   /// imposed on a particular argument or return value symbol.
   ///
   /// Given a range, should the argument stay inside or outside this range?
   /// The special `ComparesToArgument' value indicates that we should
   /// impose a constraint that involves other argument or return value symbols.
-  enum ValueRangeKindTy { OutOfRange, WithinRange, ComparesToArgument };
+  enum ValueRangeKind { OutOfRange, WithinRange, ComparesToArgument };
 
   // The universal integral type to use in value range descriptions.
   // Unsigned to make sure overflows are well-defined.
-  typedef uint64_t RangeIntTy;
+  typedef uint64_t RangeInt;
 
   /// Normally, describes a single range constraint, eg. {{0, 1}, {3, 4}} is
   /// a non-negative integer, which less than 5 and not equal to 2. For
   /// `ComparesToArgument', holds information about how exactly to compare to
   /// the argument.
-  typedef std::vector> IntRangeVectorTy;
+  typedef std::vector> IntRangeVector;
 
   /// A reference to an argument or return value by its number.
   /// ArgNo in CallExpr and CallEvent is defined as Unsigned, but
   /// obviously uint32_t should be enough for all practical purposes.
-  typedef uint32_t ArgNoTy;
-  static const ArgNoTy Ret = std::numeric_limits::max();
+  typedef uint32_t ArgNo;
+  static const ArgNo Ret = std::numeric_limits::max();
 
   /// Incapsulates a single range on a single symbol within a branch.
   class ValueRange {
-ArgNoTy ArgNo; // Argument to which we apply the range.
-ValueRangeKindTy Kind; // Kind of range definition.
-IntRangeVectorTy Args; // Polymorphic arguments.
+ArgNo ArgN;  // Argument to which we apply the range.
+ValueRangeKind Kind; // Kind of range definition.
+IntRangeVector Args; // Polymorphic arguments.
 
   public:
-ValueRange(ArgNoTy ArgNo, ValueRangeKindTy Kind,
-   const IntRangeVectorTy &Args)
-: ArgNo(ArgNo), Kind(Kind), Args(Args) {}
+ValueRange(ArgNo ArgN, ValueRangeKind Kind, const IntRangeVector &Args)
+: ArgN(A

[PATCH] D74146: [SytemZ] Disable vector ABI when using option -march=arch[8|9|10]

2020-02-07 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74146



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


[PATCH] D74116: [Sema][C++] Strawman patch to propagate conversion type in order to specialize the diagnostics

2020-02-07 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

In D74116#1863186 , @rjmccall wrote:

> Hmm.  The alternative approach, I suppose, would be to recognize that we're 
> about to emit a generic error and just recheck assignment constraints to 
> recompute the AssignConvertType.


Indeed. It will need less changes around the code base but also means checks 
will be repeated.


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

https://reviews.llvm.org/D74116



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


[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-02-07 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

If more discussion is needed it is better to use "Discourse" instead of this 
review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705



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


[PATCH] D74168: [CMake] Make EXCLUDE_FROM_ALL an argument to add_lit_testsuite

2020-02-07 Thread Konstantin Schwarz via Phabricator via cfe-commits
kschwarz added inline comments.



Comment at: llvm/test/CMakeLists.txt:171
 
+if(LLVM_BUILD_TOOLS)
+  set(exclude_from_check_all "EXCLUDE_FROM_CHECK_ALL")

Hi @JDevlieghere, we've noticed that with this patch check-llvm isn't added to 
check-all anymore by default. Is this the intended behaviour?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74168



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


[PATCH] D71433: [analyzer] CERT: POS34-C

2020-02-07 Thread Zurab Tsinadze via Phabricator via cfe-commits
zukatsinadze updated this revision to Diff 243127.
zukatsinadze added a comment.

Addressed new inline comments.


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

https://reviews.llvm.org/D71433

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/lib/StaticAnalyzer/Checkers/AllocationState.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
  clang/test/Analysis/cert/pos34-c.cpp

Index: clang/test/Analysis/cert/pos34-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/pos34-c.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.pos.34c\
+// RUN:  -verify %s
+
+// Examples from the CERT rule's page.
+// https://wiki.sei.cmu.edu/confluence/x/6NYxBQ
+
+#include "../Inputs/system-header-simulator.h"
+void free(void *memblock);
+void *malloc(size_t size);
+int putenv(char *);
+int snprintf(char *str, size_t size, const char *format, ...);
+
+namespace test_auto_var_used_bad {
+
+int volatile_memory1(const char *var) {
+  char env[1024];
+  int retval = snprintf(env, sizeof(env), "TEST=%s", var);
+  if (retval < 0 || (size_t)retval >= sizeof(env)) {
+/* Handle error */
+  }
+
+  return putenv(env);
+  // expected-warning@-1 {{'putenv' function should not be called with auto variables}}
+}
+
+} // namespace test_auto_var_used_bad
+
+namespace test_auto_var_used_good {
+
+int test_static(const char *var) {
+  static char env[1024];
+
+  int retval = snprintf(env, sizeof(env), "TEST=%s", var);
+  if (retval < 0 || (size_t)retval >= sizeof(env)) {
+/* Handle error */
+  }
+
+  return putenv(env);
+}
+
+int test_heap_memory(const char *var) {
+  static char *oldenv;
+  const char *env_format = "TEST=%s";
+  const size_t len = strlen(var) + strlen(env_format);
+  char *env = (char *)malloc(len);
+  if (env == NULL) {
+return -1;
+  }
+  if (putenv(env) != 0) { // no-warning: env was dynamically allocated.
+free(env);
+return -1;
+  }
+  if (oldenv != NULL) {
+free(oldenv); /* avoid memory leak */
+  }
+  oldenv = env;
+  return 0;
+}
+
+} // namespace test_auto_var_used_good
Index: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.pos.34c\
+// RUN:  -verify %s
+
+#include "../Inputs/system-header-simulator.h"
+void free(void *memblock);
+void *malloc(size_t size);
+int putenv(char *);
+int rand();
+
+namespace test_auto_var_used_good {
+
+extern char *ex;
+int test_extern() {
+  return putenv(ex); // no-warning: extern storage class.
+}
+
+void foo(void) {
+  char *buffer = (char *)"huttah!";
+  if (rand() % 2 == 0) {
+buffer = (char *)malloc(5);
+strcpy(buffer, "woot");
+  }
+  putenv(buffer);
+}
+
+void bar(void) {
+  char *buffer = (char *)malloc(5);
+  strcpy(buffer, "woot");
+
+  if (rand() % 2 == 0) {
+free(buffer);
+buffer = (char *)"blah blah blah";
+  }
+  putenv(buffer);
+}
+
+void baz() {
+  char env[] = "NAME=value";
+  // TODO: False Positive
+  putenv(env);
+  // expected-warning@-1 {{'putenv' function should not be called with auto variables}}
+
+  /*
+DO SOMETHING
+  */
+
+  putenv((char *)"NAME=anothervalue");
+}
+
+} // namespace test_auto_var_used_good
Index: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
===
--- clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
+++ clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
@@ -9,13 +9,18 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 
 // Common strings used for the "category" of many static analyzer issues.
-namespace clang { namespace ento { namespace categories {
+namespace clang {
+namespace ento {
+namespace categories {
 
-const char * const CoreFoundationObjectiveC = "Core Foundation/Objective-C";
-const char * const LogicError = "Logic error";
-const char * const MemoryRefCount =
-  "Memory (Core Foundation/Objective-C/OSObject)";
-const char * const MemoryError = "Memory error";
-const char * const UnixAPI = "Unix API";
-const char * const CXXObjectLifecycle = "C++ object lifecycle";
-}}}
+const char *const CoreFoundationObjectiveC = "Core Foundation/Objective-C";
+const char *const LogicError = "Logic error";
+const char *const MemoryRefCount =
+"Memory (Core Foundation/Objective-C/OSObject)";
+const char *const MemoryError = "Memory error";
+const char *const UnixAPI = "Unix API

[PATCH] D71433: [analyzer] CERT: POS34-C

2020-02-07 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment.

In D71433#1808316 , @Charusso wrote:

> In D71433#1784238 , @NoQ wrote:
>
> > Currently the check may warn on non-bugs of the following kind:
> >
> >   void foo() {
> > char env[] = "NAME=value";
> > putenv(env);
> > doStuff();
> > putenv("NAME=anothervalue");
> >   }
> >
>
>
> That could be the next round as a follow-up patch as the next semester starts 
> in February [...]


Well, the next semester is about to start. Could you implement that request 
please?


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

https://reviews.llvm.org/D71433



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


[PATCH] D71433: [analyzer] CERT: POS34-C

2020-02-07 Thread Zurab Tsinadze via Phabricator via cfe-commits
zukatsinadze added a comment.

In D71433#1863638 , @Charusso wrote:

> In D71433#1808316 , @Charusso wrote:
>
> > In D71433#1784238 , @NoQ wrote:
> >
> > > Currently the check may warn on non-bugs of the following kind:
> > >
> > >   void foo() {
> > > char env[] = "NAME=value";
> > > putenv(env);
> > > doStuff();
> > > putenv("NAME=anothervalue");
> > >   }
> > >
> >
> >
> > That could be the next round as a follow-up patch as the next semester 
> > starts in February [...]
>
>
> Well, the next semester is about to start. Could you implement that request 
> please?


Sure. I plan to start working on it next week.


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

https://reviews.llvm.org/D71433



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


[clang] 64bc627 - clang-cl: Parse new MSVC flags /Qspectre-load and /Qspectre-load-cf

2020-02-07 Thread Hans Wennborg via cfe-commits

Author: Hans Wennborg
Date: 2020-02-07T13:00:52+01:00
New Revision: 64bc627b8878dd77fc3a85007e2ced0a515c77d3

URL: 
https://github.com/llvm/llvm-project/commit/64bc627b8878dd77fc3a85007e2ced0a515c77d3
DIFF: 
https://github.com/llvm/llvm-project/commit/64bc627b8878dd77fc3a85007e2ced0a515c77d3.diff

LOG: clang-cl: Parse new MSVC flags /Qspectre-load and /Qspectre-load-cf

See 
https://github.com/MicrosoftDocs/cpp-docs/commit/2fdf0ba0bf8d3875c754776ca1084654135cb710

Added: 


Modified: 
clang/include/clang/Driver/CLCompatOptions.td
clang/test/Driver/cl-options.c

Removed: 




diff  --git a/clang/include/clang/Driver/CLCompatOptions.td 
b/clang/include/clang/Driver/CLCompatOptions.td
index 90be1d3f3c04..561746d931ed 100644
--- a/clang/include/clang/Driver/CLCompatOptions.td
+++ b/clang/include/clang/Driver/CLCompatOptions.td
@@ -452,6 +452,8 @@ def _SLASH_Qpar : CLFlag<"Qpar">;
 def _SLASH_Qpar_report : CLJoined<"Qpar-report">;
 def _SLASH_Qsafe_fp_loads : CLFlag<"Qsafe_fp_loads">;
 def _SLASH_Qspectre : CLFlag<"Qspectre">;
+def _SLASH_Qspectre_load : CLFlag<"Qspectre-load">;
+def _SLASH_Qspectre_load_cf : CLFlag<"Qspectre-load-cf">;
 def _SLASH_Qvec_report : CLJoined<"Qvec-report">;
 def _SLASH_u : CLFlag<"u">;
 def _SLASH_V : CLFlag<"V">;

diff  --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index b5dd667943d2..f230caa936be 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -464,6 +464,8 @@
 // RUN: /Qpar-report:1 \
 // RUN: /Qsafe_fp_loads \
 // RUN: /Qspectre \
+// RUN: /Qspectre-load \
+// RUN: /Qspectre-load-cf \
 // RUN: /Qvec-report:2 \
 // RUN: /u \
 // RUN: /V \



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


[clang] 6064f42 - [OpenCL] Restrict addr space conversions in nested pointers

2020-02-07 Thread Anastasia Stulova via cfe-commits

Author: Anastasia Stulova
Date: 2020-02-07T12:04:35Z
New Revision: 6064f426a18304e16b51cc79e74c9c2d55ef5a9c

URL: 
https://github.com/llvm/llvm-project/commit/6064f426a18304e16b51cc79e74c9c2d55ef5a9c
DIFF: 
https://github.com/llvm/llvm-project/commit/6064f426a18304e16b51cc79e74c9c2d55ef5a9c.diff

LOG: [OpenCL] Restrict addr space conversions in nested pointers

Address space conversion changes pointer representation.
This commit disallows such conversions when they are not
legal i.e. for the nested pointers even with compatible
address spaces. Because the address space conversion in
the nested levels can't be generated to modify the pointers
correctly. The behavior implemented is as follows:

- Any implicit conversions of nested pointers with different
  address spaces is rejected.
- Any conversion of address spaces in nested pointers in safe
  casts (e.g. const_cast or static_cast) is rejected.
- Conversion in low level C-style or reinterpret_cast is accepted
  but with a warning (this aligns with OpenCL C behavior).

Fixes PR39674

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

Added: 
clang/test/SemaOpenCLCXX/address-space-castoperators.cl

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
clang/test/SemaOpenCL/address-spaces.cl
clang/test/SemaOpenCLCXX/address-space-deduction.cl
clang/test/SemaOpenCLCXX/address-space-references.cl

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 504c9057107b..104a2a575481 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6760,6 +6760,10 @@ def err_bad_cxx_cast_scalar_to_vector_
diff erent_size : Error<
 def err_bad_cxx_cast_vector_to_vector_
diff erent_size : Error<
   "%select{||reinterpret_cast||C-style cast|}0 from vector %1 "
   "to vector %2 of 
diff erent size">;
+def warn_bad_cxx_cast_nested_pointer_addr_space : Warning<
+  "%select{reinterpret_cast|C-style cast}0 from %1 to %2 "
+  "changes address space of nested pointers">,
+  InGroup;
 def err_bad_lvalue_to_rvalue_cast : Error<
   "cannot cast from lvalue of type %1 to rvalue reference type %2; types are "
   "not compatible">;

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index a905ebc67305..7a8cbca1e3f1 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -2311,6 +2311,24 @@ static TryCastResult TryReinterpretCast(Sema &Self, 
ExprResult &SrcExpr,
 return SuccessResult;
   }
 
+  // Diagnose address space conversion in nested pointers.
+  QualType DestPtee = DestType->getPointeeType().isNull()
+  ? DestType->getPointeeType()
+  : DestType->getPointeeType()->getPointeeType();
+  QualType SrcPtee = SrcType->getPointeeType().isNull()
+ ? SrcType->getPointeeType()
+ : SrcType->getPointeeType()->getPointeeType();
+  while (!DestPtee.isNull() && !SrcPtee.isNull()) {
+if (DestPtee.getAddressSpace() != SrcPtee.getAddressSpace()) {
+  Self.Diag(OpRange.getBegin(),
+diag::warn_bad_cxx_cast_nested_pointer_addr_space)
+  << CStyle << SrcType << DestType << SrcExpr.get()->getSourceRange();
+  break;
+}
+DestPtee = DestPtee->getPointeeType();
+SrcPtee = SrcPtee->getPointeeType();
+  }
+
   // C++ 5.2.10p7: A pointer to an object can be explicitly converted to
   //   a pointer to an object of 
diff erent type.
   // Void pointers are not specified, but supported by every compiler out 
there.

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 9b89bac0db39..858e7ae34a7f 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -3180,7 +3180,7 @@ static bool isNonTrivialObjCLifetimeConversion(Qualifiers 
FromQuals,
 /// FromType and \p ToType is permissible, given knowledge about whether every
 /// outer layer is const-qualified.
 static bool isQualificationConversionStep(QualType FromType, QualType ToType,
-  bool CStyle,
+  bool CStyle, bool IsTopLevel,
   bool &PreviousToQualsIncludeConst,
   bool &ObjCLifetimeConversion) {
   Qualifiers FromQuals = FromType.getQualifiers();
@@ -3217,11 +3217,15 @@ static bool isQualificationConversionStep(QualType 
FromType, QualType ToType,
   if (!CStyle && !ToQuals.compatiblyIncludes(FromQuals))
 return false;
 
-  // For a C-style cast, just require the address spaces to overlap.
-  // FIXME: Does "superset" also imply the representation of a pointer is the

[PATCH] D73360: [OpenCL] Restrict address space conversions in nested pointers

2020-02-07 Thread Anastasia Stulova via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6064f426a183: [OpenCL] Restrict addr space conversions in 
nested pointers (authored by Anastasia).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D73360?vs=242312&id=243132#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73360

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  clang/test/SemaOpenCL/address-spaces.cl
  clang/test/SemaOpenCLCXX/address-space-castoperators.cl
  clang/test/SemaOpenCLCXX/address-space-deduction.cl
  clang/test/SemaOpenCLCXX/address-space-references.cl

Index: clang/test/SemaOpenCLCXX/address-space-references.cl
===
--- clang/test/SemaOpenCLCXX/address-space-references.cl
+++ clang/test/SemaOpenCLCXX/address-space-references.cl
@@ -13,3 +13,16 @@
 void foo() {
   bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
 }
+
+// Test addr space conversion with nested pointers
+
+extern void nestptr(int *&); // expected-note {{candidate function not viable: no known conversion from '__global int *__private' to '__generic int *__generic &__private' for 1st argument}}
+extern void nestptr_const(int * const &); // expected-note {{candidate function not viable: cannot pass pointer to address space '__constant' as a pointer to address space '__generic' in 1st argument}}
+int test_nestptr(__global int *glob, __constant int *cons, int* gen) {
+  nestptr(glob); // expected-error{{no matching function for call to 'nestptr'}}
+  // Addr space conversion first occurs on a temporary.
+  nestptr_const(glob);
+  // No legal conversion between disjoint addr spaces.
+  nestptr_const(cons); // expected-error{{no matching function for call to 'nestptr_const'}}
+  return *(*cons ? glob : gen);
+}
Index: clang/test/SemaOpenCLCXX/address-space-deduction.cl
===
--- clang/test/SemaOpenCLCXX/address-space-deduction.cl
+++ clang/test/SemaOpenCLCXX/address-space-deduction.cl
@@ -105,7 +105,7 @@
 
 __kernel void k() {
   __local float x[2];
-  __local float(*p)[2];
+  float(*p)[2];
   t1(x);
   t2(&x);
   t3(&x);
Index: clang/test/SemaOpenCLCXX/address-space-castoperators.cl
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/address-space-castoperators.cl
@@ -0,0 +1,12 @@
+//RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -ast-dump -verify | FileCheck %s
+
+void nester_ptr() {
+  local int * * locgen;
+  constant int * * congen;
+  int * * gengen;
+
+  gengen = const_cast(locgen); //expected-error{{const_cast from '__local int *__generic *' to '__generic int *__generic *' is not allowed}}
+  gengen = static_cast(locgen); //expected-error{{static_cast from '__local int *__generic *' to '__generic int *__generic *' is not allowed}}
+// CHECK-NOT: AddressSpaceConversion
+  gengen = reinterpret_cast(locgen); //expected-warning{{reinterpret_cast from '__local int *__generic *' to '__generic int *__generic *' changes address space of nested pointers}}
+}
Index: clang/test/SemaOpenCL/address-spaces.cl
===
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -198,9 +198,7 @@
   ll = gg;   // expected-error {{assigning to '__local int *__private *' from incompatible type '__global int *__private *__private'}}
   ll = l;// expected-error {{assigning to '__local int *__private *' from incompatible type '__local int *__private'; take the address with &}}
   ll = gg_f; // expected-error {{assigning to '__local int *__private *' from incompatible type '__global float *__private *__private'}}
-  // FIXME: The below becomes a reinterpret_cast, and therefore does not emit an error
-  // even though the address space mismatches in the nested pointers.
-  ll = (__local int * __private *)gg;
+  ll = (__local int *__private *)gg; //expected-warning{{C-style cast from '__global int *__private *' to '__local int *__private *' changes address space of nested pointers}}
 
   gg_f = g;  // expected-error {{assigning to '__global float *__private *' from incompatible type '__global int *__private'}}
   gg_f = gg; // expected-error {{assigning to '__global float *__private *' from incompatible type '__global int *__private *__private'}}
Index: clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
===
--- clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
+++ clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
@@ -513,17 +513,37 @@
 #endif
 
   // Case 3: 

[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

2020-02-07 Thread 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 is amazing. LGTM, granted that @NoQ is happy with the patch as well.




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:33
 //
 // This checker uses eval::Call for modeling "pure" functions, for which
 // their `FunctionSummaryTy' is a precise model. This avoids unnecessary

If we put pure in quotes, we might as well go the extra mile and quickly 
explain what that means.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:42-49
 // The following standard C functions are currently supported:
 //
 //   fgetc  getline   isdigit   isupper
 //   fread  isalnum   isgraph   isxdigit
 //   fwrite isalpha   islower   read
 //   getc   isascii   isprint   write
 //   getcharisblank   ispunct

I would prefer to just have a checker option that could print out the currently 
modeled function rather than these lines of a recipe for outdated comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:214-215
 
   // The map of all functions supported by the checker. It is initialized
   // lazily, and it doesn't change after initialization.
+  mutable llvm::StringMap FunctionSummaryMap;

But why? This isn't important for this patch, so feel free to leave as-is, but 
still.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897



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


[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

After tests are added and `clang-format-diff.py` is ran, this would be an 
amazing patch, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74131



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


[PATCH] D74214: [clang-tidy] Fix PR#34798 'readability-braces-around-statements breaks statements containing braces.'

2020-02-07 Thread Karasev Nikita via Phabricator via cfe-commits
f00kat created this revision.
f00kat added reviewers: aaron.ballman, alexfh.
f00kat added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.

In method 'BracesAroundStatementsCheck::checkStmt' the call 
'Lexer::makeFileCharRange(line 253)' return different EndSourceLocation for 
different statements.

  CharSourceRange FileRange = Lexer::makeFileCharRange(
CharSourceRange::getTokenRange(S->getSourceRange()), SM,
Context->getLangOpts());

For example

  class c {};
  c test()
  {
if (true)
return {};
if (true)
bool b[2] = { true, true };
  }

In case

  return {};

FileRange.getEnd() will be ';'
In case

  bool b[2] = { true, true };

FileRange.getEnd() will be '\r'

Before call 'findEndLocation' we do this

  const auto FREnd = FileRange.getEnd().getLocWithOffset(-1);

so 'findEndLocation' received '}' in the first case and ';' in the second what 
breaks the logic of 'findEndLocation'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74214

Files:
  clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp
@@ -204,3 +204,28 @@
   // CHECK-FIXES-NEXT: {{^  ;$}}
   // CHECK-FIXES-NEXT: {{^}$}}
 }
+
+class X {};
+X test_initListExpr()
+{
+  if (cond("x")) return {};
+  else return {};
+  // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: statement should be inside 
braces
+  // CHECK-FIXES: if (cond("x")) { return {};
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES: return {};
+  // CHECK-FIXES: }
+
+  if (cond("y1"))
+if (cond("y2") && cond("y3"))
+  return {};
+else
+  return {};
+  // CHECK-MESSAGES: :[[@LINE-2]]:24: warning: statement should be inside 
braces
+  // CHECK-FIXES: if (cond("y1")) {
+  // CHECK-FIXES: if (cond("y2") && cond("y3")) {
+  // CHECK-FIXES: return {};
+  // CHECK-FIXES: } else {
+  // CHECK-FIXES: return {};
+  // CHECK-FIXES: }
+}
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -18,19 +18,32 @@
 namespace readability {
 namespace {
 
-tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM,
-const ASTContext *Context) {
-  Token Tok;
+bool tryGetTokenKind(SourceLocation Loc, const SourceManager &SM,
+ const ASTContext *Context, tok::TokenKind &Kind) {
+  Kind = tok::NUM_TOKENS;
   SourceLocation Beginning =
   Lexer::GetBeginningOfToken(Loc, SM, Context->getLangOpts());
+
+  Token Tok;
   const bool Invalid =
   Lexer::getRawToken(Beginning, Tok, SM, Context->getLangOpts());
-  assert(!Invalid && "Expected a valid token.");
+  if (!Invalid) {
+Kind = Tok.getKind();
+  }
+
+  return !Invalid;
+}
+
+tok::TokenKind getTokenKind(SourceLocation Loc, const SourceManager &SM,
+const ASTContext *Context) {
+  tok::TokenKind Kind;
+  if (tryGetTokenKind(Loc, SM, Context, Kind)) {
+return Kind;
+  }
 
-  if (Invalid)
-return tok::NUM_TOKENS;
+  assert(false && "Expected a valid token.");
 
-  return Tok.getKind();
+  return tok::NUM_TOKENS;
 }
 
 SourceLocation forwardSkipWhitespaceAndComments(SourceLocation Loc,
@@ -58,8 +71,21 @@
   // Loc points to the beginning of the last (non-comment non-ws) token
   // before end or ';'.
   assert(Loc.isValid());
-  bool SkipEndWhitespaceAndComments = true;
   tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
+
+  // We need to check that it is not 'InitListExpr' which ends with 
+  // the tokens '};' because it will break the following analysis
+  tok::TokenKind NextTokKind;
+  SourceLocation NextLoc =
+  Lexer::getLocForEndOfToken(Loc, 0, SM, Context->getLangOpts());
+  if (tryGetTokenKind(NextLoc, SM, Context, NextTokKind)) {
+if (TokKind == tok::r_brace && NextTokKind == tok::semi) {
+  Loc = NextLoc;
+  TokKind = NextTokKind;
+}
+  }
+
+  bool SkipEndWhitespaceAndComments = true;
   if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
   TokKind == tok::r_brace) {
 // If we are at ";" or "}", we found the last token. We could use as well


Index: clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-braces-around-statements.cpp
++

[PATCH] D73720: [Analyzer] Use note tags to track container begin and and changes

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:731
+  }
+  return C.getNoteTag([Text, Name](BugReport &BR) -> std::string {
+  SmallString<256> Msg;

baloghadamsoftware wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > You'll need to check whether the container is actually of interest 
> > > > > > to the bug report. We don't want notes to be added about changes to 
> > > > > > irrelevant containers.
> > > > > > 
> > > > > > You can use a combination of "Report `BR` was emitted by one of the 
> > > > > > iterator checkers" and "The memory region of the container is 
> > > > > > marked as interesting" (while also actually marking it as 
> > > > > > interesting in the checker).
> > > > > > 
> > > > > > Ideally we should instead make a new generic storage inside the 
> > > > > > `BugReport` object, in order to pass down the interesting 
> > > > > > information from the call site of `emitReport` ("Hi, i'm an 
> > > > > > iterator checker who emitted this report and i'm interested in 
> > > > > > changes made to the size of this container").
> > > > > Are you sure in this? I already wondered how it works so I added a 
> > > > > test that checks one container and changes another one and there were 
> > > > > no note tags displayed for the one we did not check but change. See 
> > > > > the last test.
> > > > That's because you didn't do
> > > > ```lang=c++
> > > >   V2.cbegin();
> > > >   V2.cend();
> > > > ```
> > > > in the beginning.
> > > A similar conversation sparked up recently in between @boga95, @steakhal 
> > > and me regarding reporting taintedness. Bug reports are fine up to the 
> > > point where (in reverse) the first propagation happens, but finding out 
> > > which value tainted the one that caused the report isn't handled at the 
> > > moment. One idea was to mark the initial (again, in reverse) value as 
> > > interesting, create a `NoteTag` at the point of propagation, where we 
> > > should know which value was the cause of the spread, mark that 
> > > interesting as well, etc.
> > > 
> > > If `NoteTag`s only emit a message when the concerning value is 
> > > interesting, this should theoretically solve that problem. I guess you 
> > > could say that we're propagating interestingness in reverse.
> > > 
> > > I'm not immediately sure if this idea was ever mentioned or implemented 
> > > here.
> > Yes, that's the intended solution to such problems. `trackExpressionValue` 
> > works similarly, just with assignments instead of taint propagations. And 
> > in both cases note tags are a much more straightforward solution to the 
> > problem.
> Yes, you are right. My problem now is that how to mark interesting when 
> debugging? I I filter for interesting containers only, I lose my ability to 
> debug. Should I create a debug function just for marking a container as 
> interesting. Or is there such function already?
I'm not immediately sure how interetingness ties into debugging, what specific 
scenario are you thinking about?


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

https://reviews.llvm.org/D73720



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


[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision.
Szelethus added a comment.
This revision now requires changes to proceed.

I wouldn't like to see reports emitted by a checker that resides in 
`apiModeling`. Could we create a new one? Some checkers, like the 
`IteratorChecker`, `MallocChecker` and `CStringChecker` implement a variety of 
user-facing checkers within the same class, that is also an option, if creating 
a new checker class is too much of a hassle.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:697-699
+  // The behavior is undefined if the value of the argument is not
+  // representable as unsigned char or is not equal to EOF. See e.g. C99
+  // 7.4.1.2 The isalpha function (p: 181-182).

This is true for the rest of the summaries as well, but shouldn't we retrieve 
the `unsigned char` size from `ASTContext`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73898



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


[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

A portion of my concerns are answered by this patch: D72035 
.




Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:103-132
   struct FunctionData {
 FunctionData() = delete;
 FunctionData(const FunctionData &) = default;
 FunctionData(FunctionData &&) = default;
 FunctionData &operator=(const FunctionData &) = delete;
 FunctionData &operator=(FunctionData &&) = delete;
 

Szelethus wrote:
> I know this isn't really relevant, but isn't this redundant with 
> `CallDescription`?
Ah, okay, so `CallDescription` doesn't really have the `FunctionDecl`, but this 
still feels like a duplication of functionalities.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71524



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


[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Hmm, have you branched off of D71524 ? If so, 
this patch should definitely land first.




Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:165
   /// Given a pointer argument, return the value it points to.
-  static Optional getPointedToSVal(CheckerContext &C, const Expr *Arg);
+  static Optional getPointeeOf(CheckerContext &C, const Expr *Arg);
 

Nice!



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:385
+unsigned getNumArgs(const CallEvent &Call) {
+  return Call.getNumArgs() + static_cast(isa(Call));
 }

NoQ wrote:
> steakhal wrote:
> > I'm not sure why should we adjust (//workaround//) the number of arguments 
> > of `CXXInstanceCall`s calls, can someone explain it to me?
> > 
> > The same question raised for `getArg` too. 
> Remove this :)
> 
> I think this is about this inconsistency with operator calls where one of 
> {decl, expr} treats `this` as an argument, but the other doesn't. `CallEvent` 
> automatically accounts for that (see `getAdjustedParameterIndex()` and 
> `getASTArgumentIndex()` as they're overridden in various sub-classes of 
> `CallEvent`).
I have some thoughts about this here: D71524#1859435. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72035



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


[PATCH] D73742: [Clang][Driver] After default -fintegrated-cc1, fix report_fatal_error no longer generates preprocessed source + reproducer.sh

2020-02-07 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments.



Comment at: clang/tools/driver/cc1_main.cpp:73
   // defined as an internal software error.  Otherwise, exit with status 1.
-  exit(GenCrashDiag ? 70 : 1);
+  llvm::sys::Process::Exit(GenCrashDiag ? 70 : 1);
 }

I don't think it matters (yet?) but we should probably also use 
llvm::sys::Process::Exit() in cc1as_main.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73742



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


[PATCH] D71433: [analyzer] CERT: POS34-C

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.

This is a very neat checker, the source code reads so easily, we might as well 
put it as the official CERT rule description.

I think adding the non-compliant and compliant code examples would be nice. I 
also wrote some inline comments, but I'm fine with leaving them for a later 
patch. LGTM, granted that @NoQ and @Charusso are happy.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h:16-22
+extern const char *const CoreFoundationObjectiveC;
+extern const char *const LogicError;
+extern const char *const MemoryRefCount;
+extern const char *const MemoryError;
+extern const char *const UnixAPI;
+extern const char *const CXXObjectLifecycle;
+extern const char *const SecurityError;

We could turns these into `llvm::StringLiteral`s one day.



Comment at: clang/lib/StaticAnalyzer/Checkers/AllocationState.h:28-30
+/// \returns The MallocBugVisitor.
+std::unique_ptr getMallocBRVisitor(SymbolRef Sym);
+

Is this deadcode now?



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:3383-3386
+std::unique_ptr getMallocBRVisitor(SymbolRef Sym) {
+  return std::make_unique(Sym);
+}
+

And this?



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:50
+  StringRef ErrorMsg =
+  "'putenv' function should not be called with auto variables";
+  ExplodedNode *N = C.generateErrorNode();

How about `The 'putenv' function should not be called with arguments that have 
automatic storage`. But I guess we could leave wordsmithing for later.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:55
+  // Track the argument.
+  if (isa(MSR)) {
+bugreporter::trackExpressionValue(Report->getErrorNode(), ArgExpr, 
*Report);

This should always be true, since we bail out a couple lines up if it isn't, 
right?


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

https://reviews.llvm.org/D71433



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


[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

2020-02-07 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 243144.
steakhal added a comment.

- Tests added.
- Clang-format-diff applied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74131

Files:
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/test/Analysis/debug-exprinspection-istainted.c

Index: clang/test/Analysis/debug-exprinspection-istainted.c
===
--- /dev/null
+++ clang/test/Analysis/debug-exprinspection-istainted.c
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-checker=alpha.security.taint
+
+int scanf(const char *restrict format, ...);
+void clang_analyzer_isTainted(char);
+void clang_analyzer_isTainted_any_suffix(char);
+
+void foo() {
+  char buf[32] = "";
+  clang_analyzer_isTainted(buf[0]);// expected-warning {{NO}}
+  clang_analyzer_isTainted_any_suffix(buf[0]); // expected-warning {{NO}}
+  scanf("%s", buf);
+  clang_analyzer_isTainted(buf[0]);// expected-warning {{YES}}
+  clang_analyzer_isTainted_any_suffix(buf[0]); // expected-warning {{YES}}
+
+  int tainted_value = buf[0]; // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "Taint.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/SValExplainer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -46,6 +47,7 @@
   void analyzerHashDump(const CallExpr *CE, CheckerContext &C) const;
   void analyzerDenote(const CallExpr *CE, CheckerContext &C) const;
   void analyzerExpress(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerIsTainted(const CallExpr *CE, CheckerContext &C) const;
 
   typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *,
  CheckerContext &C) const;
@@ -73,26 +75,34 @@
 
   // These checks should have no effect on the surrounding environment
   // (globals should not be invalidated, etc), hence the use of evalCall.
-  FnCheck Handler = llvm::StringSwitch(C.getCalleeName(CE))
-.Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval)
-.Case("clang_analyzer_checkInlined",
-  &ExprInspectionChecker::analyzerCheckInlined)
-.Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
-.Case("clang_analyzer_warnIfReached",
-  &ExprInspectionChecker::analyzerWarnIfReached)
-.Case("clang_analyzer_warnOnDeadSymbol",
-  &ExprInspectionChecker::analyzerWarnOnDeadSymbol)
-.StartsWith("clang_analyzer_explain", &ExprInspectionChecker::analyzerExplain)
-.StartsWith("clang_analyzer_dump", &ExprInspectionChecker::analyzerDump)
-.Case("clang_analyzer_getExtent", &ExprInspectionChecker::analyzerGetExtent)
-.Case("clang_analyzer_printState",
-  &ExprInspectionChecker::analyzerPrintState)
-.Case("clang_analyzer_numTimesReached",
-  &ExprInspectionChecker::analyzerNumTimesReached)
-.Case("clang_analyzer_hashDump", &ExprInspectionChecker::analyzerHashDump)
-.Case("clang_analyzer_denote", &ExprInspectionChecker::analyzerDenote)
-.Case("clang_analyzer_express", &ExprInspectionChecker::analyzerExpress)
-.Default(nullptr);
+  FnCheck Handler =
+  llvm::StringSwitch(C.getCalleeName(CE))
+  .Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval)
+  .Case("clang_analyzer_checkInlined",
+&ExprInspectionChecker::analyzerCheckInlined)
+  .Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
+  .Case("clang_analyzer_warnIfReached",
+&ExprInspectionChecker::analyzerWarnIfReached)
+  .Case("clang_analyzer_warnOnDeadSymbol",
+&ExprInspectionChecker::analyzerWarnOnDeadSymbol)
+  .StartsWith("clang_analyzer_explain",
+  &ExprInspectionChecker::analyzerExplain)
+  .StartsWith("clang_analyzer_dump",
+  &ExprInspectionChecker::analyzerDump)
+  .Case("clang_analyzer_getExtent",
+&ExprInspectionChecker::analyzerGetExtent)
+  .Case("clang_analyzer_printState",
+&ExprInspectionChecker::analyzerPrintState)
+  .Case("clang_analyzer_numTimesReached",
+&ExprInspectionChecker::analyzerNumTimesReached)
+  .Case("clang_analyzer_hashDump",
+&ExprInspectionChecker::analyzerHash

[PATCH] D74131: [analyzer][taint] Add isTainted debug expression inspection check

2020-02-07 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 243146.
steakhal added a comment.

Clarified example usage, especially in contrast of eg.: `debug.TaintTest` 
checker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74131

Files:
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/test/Analysis/debug-exprinspection-istainted.c

Index: clang/test/Analysis/debug-exprinspection-istainted.c
===
--- /dev/null
+++ clang/test/Analysis/debug-exprinspection-istainted.c
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-checker=alpha.security.taint
+
+int scanf(const char *restrict format, ...);
+void clang_analyzer_isTainted(char);
+void clang_analyzer_isTainted_any_suffix(char);
+
+void foo() {
+  char buf[32] = "";
+  clang_analyzer_isTainted(buf[0]);// expected-warning {{NO}}
+  clang_analyzer_isTainted_any_suffix(buf[0]); // expected-warning {{NO}}
+  scanf("%s", buf);
+  clang_analyzer_isTainted(buf[0]);// expected-warning {{YES}}
+  clang_analyzer_isTainted_any_suffix(buf[0]); // expected-warning {{YES}}
+
+  int tainted_value = buf[0]; // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "Taint.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Checkers/SValExplainer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -46,6 +47,7 @@
   void analyzerHashDump(const CallExpr *CE, CheckerContext &C) const;
   void analyzerDenote(const CallExpr *CE, CheckerContext &C) const;
   void analyzerExpress(const CallExpr *CE, CheckerContext &C) const;
+  void analyzerIsTainted(const CallExpr *CE, CheckerContext &C) const;
 
   typedef void (ExprInspectionChecker::*FnCheck)(const CallExpr *,
  CheckerContext &C) const;
@@ -73,26 +75,34 @@
 
   // These checks should have no effect on the surrounding environment
   // (globals should not be invalidated, etc), hence the use of evalCall.
-  FnCheck Handler = llvm::StringSwitch(C.getCalleeName(CE))
-.Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval)
-.Case("clang_analyzer_checkInlined",
-  &ExprInspectionChecker::analyzerCheckInlined)
-.Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
-.Case("clang_analyzer_warnIfReached",
-  &ExprInspectionChecker::analyzerWarnIfReached)
-.Case("clang_analyzer_warnOnDeadSymbol",
-  &ExprInspectionChecker::analyzerWarnOnDeadSymbol)
-.StartsWith("clang_analyzer_explain", &ExprInspectionChecker::analyzerExplain)
-.StartsWith("clang_analyzer_dump", &ExprInspectionChecker::analyzerDump)
-.Case("clang_analyzer_getExtent", &ExprInspectionChecker::analyzerGetExtent)
-.Case("clang_analyzer_printState",
-  &ExprInspectionChecker::analyzerPrintState)
-.Case("clang_analyzer_numTimesReached",
-  &ExprInspectionChecker::analyzerNumTimesReached)
-.Case("clang_analyzer_hashDump", &ExprInspectionChecker::analyzerHashDump)
-.Case("clang_analyzer_denote", &ExprInspectionChecker::analyzerDenote)
-.Case("clang_analyzer_express", &ExprInspectionChecker::analyzerExpress)
-.Default(nullptr);
+  FnCheck Handler =
+  llvm::StringSwitch(C.getCalleeName(CE))
+  .Case("clang_analyzer_eval", &ExprInspectionChecker::analyzerEval)
+  .Case("clang_analyzer_checkInlined",
+&ExprInspectionChecker::analyzerCheckInlined)
+  .Case("clang_analyzer_crash", &ExprInspectionChecker::analyzerCrash)
+  .Case("clang_analyzer_warnIfReached",
+&ExprInspectionChecker::analyzerWarnIfReached)
+  .Case("clang_analyzer_warnOnDeadSymbol",
+&ExprInspectionChecker::analyzerWarnOnDeadSymbol)
+  .StartsWith("clang_analyzer_explain",
+  &ExprInspectionChecker::analyzerExplain)
+  .StartsWith("clang_analyzer_dump",
+  &ExprInspectionChecker::analyzerDump)
+  .Case("clang_analyzer_getExtent",
+&ExprInspectionChecker::analyzerGetExtent)
+  .Case("clang_analyzer_printState",
+&ExprInspectionChecker::analyzerPrintState)
+  .Case("clang_analyzer_numTimesReached",
+&ExprInspectionChecker::analyzerNumTimesReached)
+  .Case("clang_analyzer_hashDump",
+

[PATCH] D74216: [clang-rename] Fix the missing template constructors.

2020-02-07 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: kbobyrev.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.

When renaming a class with template constructors, we are missing the
occurrences of the template constructors, because getUSRsForDeclaration doesn't
give USRs of the templated constructors (they are not in the normal `ctors()`
method).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74216

Files:
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
  clang/test/clang-rename/TemplateCtor.cpp


Index: clang/test/clang-rename/TemplateCtor.cpp
===
--- /dev/null
+++ clang/test/clang-rename/TemplateCtor.cpp
@@ -0,0 +1,10 @@
+class Foo { // CHECK: class Bar {
+public:
+  template 
+  Foo(); // CHECK: Bar();
+
+  template 
+  Foo(Foo &); // CHECK: Bar(Bar &);
+};
+
+// RUN: clang-rename -offset=6 -new-name=Bar %s -- | sed 's,//.*,,' | 
FileCheck %s
Index: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -135,6 +135,13 @@
 
 for (const auto *CtorDecl : RecordDecl->ctors())
   USRSet.insert(getUSRForDecl(CtorDecl));
+// Add template constructor decls, they are not in ctors() unfortunately.
+if (RecordDecl->hasUserDeclaredConstructor())
+  for (const auto *MD : RecordDecl->decls())
+if (const auto *FTD = dyn_cast(MD))
+  if (const auto *Ctor =
+  dyn_cast(FTD->getTemplatedDecl()))
+USRSet.insert(getUSRForDecl(Ctor));
 
 USRSet.insert(getUSRForDecl(RecordDecl->getDestructor()));
 USRSet.insert(getUSRForDecl(RecordDecl));
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -137,6 +137,17 @@
 };
   )cpp",
 
+  // Rename template class constructor.
+   R"cpp(
+class [[F^oo]] {
+  template
+  [[Foo]]();
+
+  template
+  [[Foo]](T t);
+};
+  )cpp",
+
   // Class in template argument.
   R"cpp(
 class [[F^oo]] {};


Index: clang/test/clang-rename/TemplateCtor.cpp
===
--- /dev/null
+++ clang/test/clang-rename/TemplateCtor.cpp
@@ -0,0 +1,10 @@
+class Foo { // CHECK: class Bar {
+public:
+  template 
+  Foo(); // CHECK: Bar();
+
+  template 
+  Foo(Foo &); // CHECK: Bar(Bar &);
+};
+
+// RUN: clang-rename -offset=6 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -135,6 +135,13 @@
 
 for (const auto *CtorDecl : RecordDecl->ctors())
   USRSet.insert(getUSRForDecl(CtorDecl));
+// Add template constructor decls, they are not in ctors() unfortunately.
+if (RecordDecl->hasUserDeclaredConstructor())
+  for (const auto *MD : RecordDecl->decls())
+if (const auto *FTD = dyn_cast(MD))
+  if (const auto *Ctor =
+  dyn_cast(FTD->getTemplatedDecl()))
+USRSet.insert(getUSRForDecl(Ctor));
 
 USRSet.insert(getUSRForDecl(RecordDecl->getDestructor()));
 USRSet.insert(getUSRForDecl(RecordDecl));
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -137,6 +137,17 @@
 };
   )cpp",
 
+  // Rename template class constructor.
+   R"cpp(
+class [[F^oo]] {
+  template
+  [[Foo]]();
+
+  template
+  [[Foo]](T t);
+};
+  )cpp",
+
   // Class in template argument.
   R"cpp(
 class [[F^oo]] {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71433: [analyzer] CERT: POS34-C

2020-02-07 Thread Zurab Tsinadze via Phabricator via cfe-commits
zukatsinadze marked 2 inline comments as done.
zukatsinadze added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:3383-3386
+std::unique_ptr getMallocBRVisitor(SymbolRef Sym) {
+  return std::make_unique(Sym);
+}
+

Szelethus wrote:
> And this?
Forgot to delete. Thanks


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

https://reviews.llvm.org/D71433



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


[PATCH] D71433: [analyzer] CERT: POS34-C

2020-02-07 Thread Zurab Tsinadze via Phabricator via cfe-commits
zukatsinadze updated this revision to Diff 243149.
zukatsinadze marked an inline comment as done.
zukatsinadze added a comment.

- Removed dead code.
- Removed unnecessary if condition.
- Changed error phrasing.




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

https://reviews.llvm.org/D71433

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
  clang/test/Analysis/cert/pos34-c.cpp

Index: clang/test/Analysis/cert/pos34-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/pos34-c.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.pos.34c\
+// RUN:  -verify %s
+
+// Examples from the CERT rule's page.
+// https://wiki.sei.cmu.edu/confluence/x/6NYxBQ
+
+#include "../Inputs/system-header-simulator.h"
+void free(void *memblock);
+void *malloc(size_t size);
+int putenv(char *);
+int snprintf(char *str, size_t size, const char *format, ...);
+
+namespace test_auto_var_used_bad {
+
+int volatile_memory1(const char *var) {
+  char env[1024];
+  int retval = snprintf(env, sizeof(env), "TEST=%s", var);
+  if (retval < 0 || (size_t)retval >= sizeof(env)) {
+/* Handle error */
+  }
+
+  return putenv(env);
+  // expected-warning@-1 {{The 'putenv' function should not be called with arguments that have automatic storage}}
+}
+
+} // namespace test_auto_var_used_bad
+
+namespace test_auto_var_used_good {
+
+int test_static(const char *var) {
+  static char env[1024];
+
+  int retval = snprintf(env, sizeof(env), "TEST=%s", var);
+  if (retval < 0 || (size_t)retval >= sizeof(env)) {
+/* Handle error */
+  }
+
+  return putenv(env);
+}
+
+int test_heap_memory(const char *var) {
+  static char *oldenv;
+  const char *env_format = "TEST=%s";
+  const size_t len = strlen(var) + strlen(env_format);
+  char *env = (char *)malloc(len);
+  if (env == NULL) {
+return -1;
+  }
+  if (putenv(env) != 0) { // no-warning: env was dynamically allocated.
+free(env);
+return -1;
+  }
+  if (oldenv != NULL) {
+free(oldenv); /* avoid memory leak */
+  }
+  oldenv = env;
+  return 0;
+}
+
+} // namespace test_auto_var_used_good
Index: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/pos34-c-fp-suppression.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.pos.34c\
+// RUN:  -verify %s
+
+#include "../Inputs/system-header-simulator.h"
+void free(void *memblock);
+void *malloc(size_t size);
+int putenv(char *);
+int rand();
+
+namespace test_auto_var_used_good {
+
+extern char *ex;
+int test_extern() {
+  return putenv(ex); // no-warning: extern storage class.
+}
+
+void foo(void) {
+  char *buffer = (char *)"huttah!";
+  if (rand() % 2 == 0) {
+buffer = (char *)malloc(5);
+strcpy(buffer, "woot");
+  }
+  putenv(buffer);
+}
+
+void bar(void) {
+  char *buffer = (char *)malloc(5);
+  strcpy(buffer, "woot");
+
+  if (rand() % 2 == 0) {
+free(buffer);
+buffer = (char *)"blah blah blah";
+  }
+  putenv(buffer);
+}
+
+void baz() {
+  char env[] = "NAME=value";
+  // TODO: False Positive
+  putenv(env);
+  // expected-warning@-1 {{The 'putenv' function should not be called with arguments that have automatic storage}}
+
+  /*
+DO SOMETHING
+  */
+
+  putenv((char *)"NAME=anothervalue");
+}
+
+} // namespace test_auto_var_used_good
Index: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
===
--- clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
+++ clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
@@ -9,13 +9,18 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 
 // Common strings used for the "category" of many static analyzer issues.
-namespace clang { namespace ento { namespace categories {
+namespace clang {
+namespace ento {
+namespace categories {
 
-const char * const CoreFoundationObjectiveC = "Core Foundation/Objective-C";
-const char * const LogicError = "Logic error";
-const char * const MemoryRefCount =
-  "Memory (Core Foundation/Objective-C/OSObject)";
-const char * const MemoryError = "Memory error";
-const char * const UnixAPI = "Unix API";
-const char * const CXXObjectLifecycle = "C++ object lifecycle";
-}}}
+const char *const CoreFoundationObjectiveC = "Core Foundation/Objective-C";
+const char *const LogicError = "Logic error";
+const char *const MemoryRefCount =
+"Memory (Core Foundation/Objective-C/OSObject)";
+const char *const MemoryError = "Memory

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

2020-02-07 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I've filed this bug against this patch: 
https://bugs.llvm.org/show_bug.cgi?id=44823

We've discovered in cases where multiple TUs are invoked in a single command 
line that the memory between calls isn't cleaned up like it was previously.  
This results in really high memory use from a compilation.

Experimentation has shown that removing the addition of the -disable-free flag 
from the CC1Command command lines fixes the issue.  However, I believe we 
should only remove that in cases where it is NOT the last CC1 command (since 
the disable-free has some pretty solid performance improvements).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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


[PATCH] D73644: [Mips] Add intrinsics for 4-byte and 8-byte MSA loads/stores.

2020-02-07 Thread Mirko Brkusanin via Phabricator via cfe-commits
mbrkusanin updated this revision to Diff 243153.

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

https://reviews.llvm.org/D73644

Files:
  clang/include/clang/Basic/BuiltinsMips.def
  clang/lib/Headers/msa.h
  clang/lib/Sema/SemaChecking.cpp
  llvm/include/llvm/IR/IntrinsicsMips.td
  llvm/lib/Target/Mips/MipsISelLowering.cpp
  llvm/lib/Target/Mips/MipsISelLowering.h
  llvm/lib/Target/Mips/MipsMSAInstrInfo.td
  llvm/lib/Target/Mips/MipsSEISelDAGToDAG.cpp
  llvm/test/CodeGen/Mips/msa/ldr_str.ll

Index: llvm/test/CodeGen/Mips/msa/ldr_str.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Mips/msa/ldr_str.ll
@@ -0,0 +1,224 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -march=mips -mcpu=mips32r5 -mattr=+msa,+fp64 -O0 < %s | FileCheck %s --check-prefix=MIPS32R5-EB
+; RUN: llc -march=mipsel   -mcpu=mips32r5 -mattr=+msa,+fp64 -O0 < %s | FileCheck %s --check-prefix=MIPS32R5-EL
+; RUN: llc -march=mips -mcpu=mips32r6 -mattr=+msa,+fp64 -O0 < %s | FileCheck %s --check-prefix=MIPS32R6-EB
+; RUN: llc -march=mipsel   -mcpu=mips32r6 -mattr=+msa,+fp64 -O0 < %s | FileCheck %s --check-prefix=MIPS32R6-EL
+; RUN: llc -march=mips64   -mcpu=mips64r6 -mattr=+msa,+fp64 -O0 < %s | FileCheck %s --check-prefix=MIPS64R6
+; RUN: llc -march=mips64el -mcpu=mips64r6 -mattr=+msa,+fp64 -O0 < %s | FileCheck %s --check-prefix=MIPS64R6
+
+; Test intrinsics for 4-byte and 8-byte MSA load and stores.
+
+define void @llvm_mips_ldr_d_test(<2 x i64>* %val, i8* %ptr) nounwind {
+; MIPS32R5-EB-LABEL: llvm_mips_ldr_d_test:
+; MIPS32R5-EB:   # %bb.0: # %entry
+; MIPS32R5-EB-NEXT:# implicit-def: $at
+; MIPS32R5-EB-NEXT:lwr $1, 23($5)
+; MIPS32R5-EB-NEXT:lwl $1, 20($5)
+; MIPS32R5-EB-NEXT:# implicit-def: $v0
+; MIPS32R5-EB-NEXT:lwr $2, 19($5)
+; MIPS32R5-EB-NEXT:lwl $2, 16($5)
+; MIPS32R5-EB-NEXT:fill.w $w0, $1
+; MIPS32R5-EB-NEXT:insert.w $w0[1], $2
+; MIPS32R5-EB-NEXT:st.d $w0, 0($4)
+; MIPS32R5-EB-NEXT:jr $ra
+; MIPS32R5-EB-NEXT:nop
+;
+; MIPS32R5-EL-LABEL: llvm_mips_ldr_d_test:
+; MIPS32R5-EL:   # %bb.0: # %entry
+; MIPS32R5-EL-NEXT:# implicit-def: $at
+; MIPS32R5-EL-NEXT:lwr $1, 16($5)
+; MIPS32R5-EL-NEXT:lwl $1, 19($5)
+; MIPS32R5-EL-NEXT:# implicit-def: $v0
+; MIPS32R5-EL-NEXT:lwr $2, 20($5)
+; MIPS32R5-EL-NEXT:lwl $2, 23($5)
+; MIPS32R5-EL-NEXT:fill.w $w0, $1
+; MIPS32R5-EL-NEXT:insert.w $w0[1], $2
+; MIPS32R5-EL-NEXT:st.d $w0, 0($4)
+; MIPS32R5-EL-NEXT:jr $ra
+; MIPS32R5-EL-NEXT:nop
+;
+; MIPS32R6-EB-LABEL: llvm_mips_ldr_d_test:
+; MIPS32R6-EB:   # %bb.0: # %entry
+; MIPS32R6-EB-NEXT:lw $1, 20($5)
+; MIPS32R6-EB-NEXT:lw $2, 16($5)
+; MIPS32R6-EB-NEXT:fill.w $w0, $1
+; MIPS32R6-EB-NEXT:insert.w $w0[1], $2
+; MIPS32R6-EB-NEXT:st.d $w0, 0($4)
+; MIPS32R6-EB-NEXT:jrc $ra
+;
+; MIPS32R6-EL-LABEL: llvm_mips_ldr_d_test:
+; MIPS32R6-EL:   # %bb.0: # %entry
+; MIPS32R6-EL-NEXT:lw $1, 16($5)
+; MIPS32R6-EL-NEXT:lw $2, 20($5)
+; MIPS32R6-EL-NEXT:fill.w $w0, $1
+; MIPS32R6-EL-NEXT:insert.w $w0[1], $2
+; MIPS32R6-EL-NEXT:st.d $w0, 0($4)
+; MIPS32R6-EL-NEXT:jrc $ra
+;
+; MIPS64R6-LABEL: llvm_mips_ldr_d_test:
+; MIPS64R6:   # %bb.0: # %entry
+; MIPS64R6-NEXT:ld $1, 16($5)
+; MIPS64R6-NEXT:fill.d $w0, $1
+; MIPS64R6-NEXT:st.d $w0, 0($4)
+; MIPS64R6-NEXT:jrc $ra
+entry:
+  %0 = tail call <2 x i64> @llvm.mips.ldr.d(i8* %ptr, i32 16)
+  store <2 x i64> %0, <2 x i64>* %val
+  ret void
+}
+
+declare <2 x i64> @llvm.mips.ldr.d(i8*, i32) nounwind
+
+define void @llvm_mips_ldrq_w_test(<4 x i32>* %val, i8* %ptr) nounwind {
+; MIPS32R5-EB-LABEL: llvm_mips_ldrq_w_test:
+; MIPS32R5-EB:   # %bb.0: # %entry
+; MIPS32R5-EB-NEXT:# implicit-def: $at
+; MIPS32R5-EB-NEXT:lwr $1, 19($5)
+; MIPS32R5-EB-NEXT:lwl $1, 16($5)
+; MIPS32R5-EB-NEXT:fill.w $w0, $1
+; MIPS32R5-EB-NEXT:st.w $w0, 0($4)
+; MIPS32R5-EB-NEXT:jr $ra
+; MIPS32R5-EB-NEXT:nop
+;
+; MIPS32R5-EL-LABEL: llvm_mips_ldrq_w_test:
+; MIPS32R5-EL:   # %bb.0: # %entry
+; MIPS32R5-EL-NEXT:# implicit-def: $at
+; MIPS32R5-EL-NEXT:lwr $1, 16($5)
+; MIPS32R5-EL-NEXT:lwl $1, 19($5)
+; MIPS32R5-EL-NEXT:fill.w $w0, $1
+; MIPS32R5-EL-NEXT:st.w $w0, 0($4)
+; MIPS32R5-EL-NEXT:jr $ra
+; MIPS32R5-EL-NEXT:nop
+;
+; MIPS32R6-EB-LABEL: llvm_mips_ldrq_w_test:
+; MIPS32R6-EB:   # %bb.0: # %entry
+; MIPS32R6-EB-NEXT:lw $1, 16($5)
+; MIPS32R6-EB-NEXT:fill.w $w0, $1
+; MIPS32R6-EB-NEXT:st.w $w0, 0($4)
+; MIPS32R6-EB-NEXT:jrc $ra
+;
+; MIPS32R6-EL-LABEL: llvm_mips_ldrq_w_test:
+; MIPS32R6-EL:   # %bb.0: # %entry
+; MIPS32R6-EL-NEXT:lw $1, 16($5)
+; MIPS32R6-EL-NEXT:fill.w $w0, $1
+; MIPS32R6-EL-NEXT:st.w $w0, 0($4)
+; MIPS32R6-EL-NEXT:jrc $ra
+;
+; MIPS64R6-LABEL: llvm_mips_ldrq_w_test:
+; MIPS64R6:   # %bb.0: # %entry
+; MIPS64R6-NEXT:lw $1

[PATCH] D73644: [Mips] Add intrinsics for 4-byte and 8-byte MSA loads/stores.

2020-02-07 Thread Mirko Brkusanin via Phabricator via cfe-commits
mbrkusanin added a comment.

Rebase.

Not yet, a proposal was made to both GCC and LLVM and as far as I can tell no 
work was done on that yet. If we accept these names I'll let them know so we 
end up with matching names.

As for 4/8 byte loads, in case of having them implemented as **ld** plus some 
extra instructions, I don't really see the point about making sure those other 
vector elements have same value as first. So if we ignore those we remain with 
only **ld**. In that case we can just not implement these loads and just have 
the user use `__builtin_msa_ld_w` and `__builtin_msa_ld_d` instead. But if we 
do decide to implement them it would make more sense to have them only read 4/8 
bytes instead of all 16. That way you can use both since **ld** is already 
available.


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

https://reviews.llvm.org/D73644



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


[PATCH] D73644: [Mips] Add intrinsics for 4-byte and 8-byte MSA loads/stores.

2020-02-07 Thread Mirko Brkusanin via Phabricator via cfe-commits
mbrkusanin added a comment.

Not yet, a proposal was made to both GCC and LLVM and as far as I can tell no 
work was done on GCC yet. If we accept these names I'll let them know so we end 
up with matching names.

As for 4/8 byte loads, in case of having them implemented as **ld** plus some 
extra instructions, I don't really see the point about making sure those other 
vector elements have same value as first. So if we ignore those we remain with 
only **ld**. In that case we can just not implement these loads and just have 
the user use `__builtin_msa_ld_w` and `__builtin_msa_ld_d` instead. But if we 
do decide to implement them it would make more sense to have them only read 4/8 
bytes instead of all 16. That way you can use both since **ld** is already 
available.


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

https://reviews.llvm.org/D73644



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


[PATCH] D73543: [clang] Add support for __builtin_memcpy_inline

2020-02-07 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment.

Anything else @efriedma ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73543



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


[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

2020-02-07 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3414-3416
+def fsycl : Flag<["-"], "fsycl">, Group, Flags<[NoArgumentUnused, 
CoreOption]>,
   HelpText<"Enable SYCL kernels compilation for device">;
+def fno_sycl : Flag<["-"], "fno-sycl">, Group, 
Flags<[NoArgumentUnused, CoreOption]>,

These flags should not be ignored, `NoArgumentUnused` should be applied to this 
flags.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5304-5306
+  // Forward -sycl-std option to -cc1
+  Args.AddLastArg(CmdArgs, options::OPT_sycl_std_EQ);
+

This code is not required, you already forwarded `sycl-std` to the frontend 
earlier



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2549
+  // but also those using the SYCL API
+  if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+Opts.setSYCLVersion(

I think processing of `sycl-std` in the frontend also must be controlled by 
some high-level option, like `-fsycl` or something like this. Without this 
`-fsycl`-like option this `std` option also must be ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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


[clang] ea9166b - [OPENMP50]Add parsing/sema for acq_rel clause.

2020-02-07 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-02-07T09:21:10-05:00
New Revision: ea9166b5a838d788a4ec0c9ddf0c83b09f49cfe4

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

LOG: [OPENMP50]Add parsing/sema for acq_rel clause.

Added basic support (representation + parsing/sema/(de)serialization)
for acq_rel clause in flush/atomic directives.

Added: 


Modified: 
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/OpenMPKinds.def
clang/include/clang/Sema/Sema.h
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtProfile.cpp
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/OpenMP/atomic_ast_print.cpp
clang/test/OpenMP/atomic_messages.cpp
clang/test/OpenMP/flush_ast_print.cpp
clang/test/OpenMP/flush_messages.cpp
clang/tools/libclang/CIndex.cpp

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index 3ef504b70937..f0dddf729309 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -1875,6 +1875,46 @@ class OMPSeqCstClause : public OMPClause {
   }
 };
 
+/// This represents 'acq_rel' clause in the '#pragma omp atomic|flush'
+/// directives.
+///
+/// \code
+/// #pragma omp flush acq_rel
+/// \endcode
+/// In this example directive '#pragma omp flush' has 'acq_rel' clause.
+class OMPAcqRelClause : public OMPClause {
+public:
+  /// Build 'ack_rel' clause.
+  ///
+  /// \param StartLoc Starting location of the clause.
+  /// \param EndLoc Ending location of the clause.
+  OMPAcqRelClause(SourceLocation StartLoc, SourceLocation EndLoc)
+  : OMPClause(OMPC_acq_rel, StartLoc, EndLoc) {}
+
+  /// Build an empty clause.
+  OMPAcqRelClause()
+  : OMPClause(OMPC_acq_rel, SourceLocation(), SourceLocation()) {}
+
+  child_range children() {
+return child_range(child_iterator(), child_iterator());
+  }
+
+  const_child_range children() const {
+return const_child_range(const_child_iterator(), const_child_iterator());
+  }
+
+  child_range used_children() {
+return child_range(child_iterator(), child_iterator());
+  }
+  const_child_range used_children() const {
+return const_child_range(const_child_iterator(), const_child_iterator());
+  }
+
+  static bool classof(const OMPClause *T) {
+return T->getClauseKind() == OMPC_acq_rel;
+  }
+};
+
 /// This represents clause 'private' in the '#pragma omp ...' directives.
 ///
 /// \code

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index 55245039e84f..d8e6fb6e0254 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -3121,6 +3121,11 @@ bool 
RecursiveASTVisitor::VisitOMPSeqCstClause(OMPSeqCstClause *) {
   return true;
 }
 
+template 
+bool RecursiveASTVisitor::VisitOMPAcqRelClause(OMPAcqRelClause *) {
+  return true;
+}
+
 template 
 bool RecursiveASTVisitor::VisitOMPThreadsClause(OMPThreadsClause *) {
   return true;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 104a2a575481..bd5bfa3e122b 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9656,6 +9656,8 @@ def note_omp_atomic_capture: Note<
   "%select{expected assignment expression|expected compound statement|expected 
exactly two expression statements|expected in right hand side of the first 
expression}0">;
 def err_omp_atomic_several_clauses : Error<
   "directive '#pragma omp atomic' cannot contain more than one 'read', 
'write', 'update' or 'capture' clause">;
+def err_omp_atomic_several_mem_order_clauses : Error<
+  "directive '#pragma omp atomic' cannot contain more than one 'seq_cst' or 
'acq_rel' clause">;
 def note_omp_atomic_previous_clause : Note<
   "'%0' clause used here">;
 def err_omp_target_contains_not_only_teams : Error<
@@ -9894,6 +9896,10 @@ def err_omp_one_defaultmap_each_category: Error<
 def err_omp_lastprivate_conditional_non_scalar : Error<
   "expected list item of scalar type in 'lastprivate' clause with 
'conditional' modifier"
   >;
+def err_omp_flush_order_clause_and_list : Error<
+  "'flush' directive with memory order clause '%0' cannot have the list">;
+def note_omp_flush_order_clause_here : Note<
+  "memory order clause '%0' is specified here">;
 } // end of OpenMP category
 
 let CategoryName = "Related Result Typ

[clang] 75f09b5 - Re-land "[Clang][Driver] Remove -M group options ..." and "[Clang] Avoid crashing when generating crash diagnostics when '#pragma clang __debug ..."

2020-02-07 Thread Alexandre Ganea via cfe-commits

Author: Alexandre Ganea
Date: 2020-02-07T09:51:09-05:00
New Revision: 75f09b54429bee17a96e2ba7a2ac0f0a8a7f7e74

URL: 
https://github.com/llvm/llvm-project/commit/75f09b54429bee17a96e2ba7a2ac0f0a8a7f7e74
DIFF: 
https://github.com/llvm/llvm-project/commit/75f09b54429bee17a96e2ba7a2ac0f0a8a7f7e74.diff

LOG: Re-land "[Clang][Driver] Remove -M group options ..." and "[Clang] Avoid 
crashing when generating crash diagnostics when '#pragma clang __debug ..."

This re-lands commits f41ec709d9d388dc43469e6ac7f51b6313f7e4af 
(https://reviews.llvm.org/D74076)
and commit 5fedc2b410853a6aef05e8edf19ebfc4e071e28f 
(https://reviews.llvm.org/D74070)

The previous build break was caused by '#pragma clang __debug llvm_unreachable' 
used in a non-assert build. Move it to a separate test in 
crash-report-with-asserts.c.

Added: 
clang/test/Driver/crash-report-with-asserts.c

Modified: 
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Lex/PreprocessorOptions.h
clang/lib/Driver/Compilation.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Lex/Pragma.cpp
clang/test/Driver/crash-report.c

Removed: 




diff  --git a/clang/include/clang/Driver/CC1Options.td 
b/clang/include/clang/Driver/CC1Options.td
index 0d0b05f8961c..733a1080be52 100644
--- a/clang/include/clang/Driver/CC1Options.td
+++ b/clang/include/clang/Driver/CC1Options.td
@@ -866,6 +866,8 @@ def detailed_preprocessing_record : Flag<["-"], 
"detailed-preprocessing-record">
   HelpText<"include a detailed record of preprocessing actions">;
 def setup_static_analyzer : Flag<["-"], "setup-static-analyzer">,
   HelpText<"Set up preprocessor for static analyzer (done automatically when 
static analyzer is run).">;
+def disable_pragma_debug_crash : Flag<["-"], "disable-pragma-debug-crash">,
+  HelpText<"Disable any #pragma clang __debug that can lead to crashing 
behavior. This is meant for testing.">;
 
 
//===--===//
 // OpenCL Options

diff  --git a/clang/include/clang/Lex/PreprocessorOptions.h 
b/clang/include/clang/Lex/PreprocessorOptions.h
index c281cd51e266..c551f87e0d7b 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -189,6 +189,9 @@ class PreprocessorOptions {
   /// Set up preprocessor for RunAnalysis action.
   bool SetUpStaticAnalyzer = false;
 
+  /// Prevents intended crashes when using #pragma clang __debug. For testing.
+  bool DisablePragmaDebugCrash = false;
+
 public:
   PreprocessorOptions() : PrecompiledPreambleBytes(0, false) {}
 

diff  --git a/clang/lib/Driver/Compilation.cpp 
b/clang/lib/Driver/Compilation.cpp
index 25aec3690f21..52477576b2eb 100644
--- a/clang/lib/Driver/Compilation.cpp
+++ b/clang/lib/Driver/Compilation.cpp
@@ -258,14 +258,23 @@ void Compilation::initCompilationForDiagnostics() {
 
   // Remove any user specified output.  Claim any unclaimed arguments, so as
   // to avoid emitting warnings about unused args.
-  OptSpecifier OutputOpts[] = { options::OPT_o, options::OPT_MD,
-options::OPT_MMD };
+  OptSpecifier OutputOpts[] = {
+  options::OPT_o,  options::OPT_MD, options::OPT_MMD, options::OPT_M,
+  options::OPT_MM, options::OPT_MF, options::OPT_MG,  options::OPT_MJ,
+  options::OPT_MQ, options::OPT_MT, options::OPT_MV};
   for (unsigned i = 0, e = llvm::array_lengthof(OutputOpts); i != e; ++i) {
 if (TranslatedArgs->hasArg(OutputOpts[i]))
   TranslatedArgs->eraseArg(OutputOpts[i]);
   }
   TranslatedArgs->ClaimAllArgs();
 
+  // Force re-creation of the toolchain Args, otherwise our modifications just
+  // above will have no effect.
+  for (auto Arg : TCArgs)
+if (Arg.second != TranslatedArgs)
+  delete Arg.second;
+  TCArgs.clear();
+
   // Redirect stdout/stderr to /dev/null.
   Redirects = {None, {""}, {""}};
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 65039ac64b5a..9e2279e90b7e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4750,6 +4750,11 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
  : "-");
   }
 
+  // Give the gen diagnostics more chances to succeed, by avoiding intentional
+  // crashes.
+  if (D.CCGenDiagnostics)
+CmdArgs.push_back("-disable-pragma-debug-crash");
+
   bool UseSeparateSections = isUseSeparateSections(Triple);
 
   if (Args.hasFlag(options::OPT_ffunction_sections,

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index d1d9a773f959..17e190627881 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3471,6 +3471,7 @@ static void ParsePreprocessorArgs(PreprocessorOpt

Re: patch via mailing list: Use getLocation() in too few/many arguments diagnostic

2020-02-07 Thread John Marshall via cfe-commits
Ping. I am a newcomer to Clang so don't know who might be appropriate reviewers 
to CC, so I've CCed a couple of general people from clang/CODE_OWNERS.TXT who 
may be able to forward as appropriate.

Thanks,

John


On 20 Jan 2020, at 16:09, John Marshall wrote:
> 
> This small patch improves the diagnostics when calling a function with the 
> wrong number of arguments. For example, given
> 
>   int
>   foo(int a, int b);
> 
>   int bar() { return foo(1); }
> 
> The existing compiler shows the error message and a note for "foo declared 
> here" showing only the "int" line. Ideally it would show the line containing 
> the word "foo" instead, which it does after this patch. Also if the function 
> declaration starts with some incidental macro, a trace of macro expansion is 
> shown which is likely irrelevant. See for example 
> https://github.com/samtools/htslib/issues/1013 and PR#23564.
> 
> I have not contributed to LLVM before and I am unfamiliar with the difference 
> between getBeginLoc() and getLocation() and the implications of using each. 
> However this patch does fix PR#23564 and perhaps this fix would be mostly a 
> no-brainer for those who are familiar with the code and these two location 
> functions in particular?

commit e8e73a04dd4274441541cc5fdc553cc4b26c00c3
Author: John Marshall 
Date:   Mon Jan 20 14:58:14 2020 +

Use getLocation() in too few/many arguments diagnostic

Use the more accurate location when emitting the location of the
function being called's prototype in diagnostics emitted when calling
a function with an incorrect number of arguments.

In particular, avoids showing a trace of irrelevant macro expansions
for "MY_EXPORT static int AwesomeFunction(int, int);". Fixes PR#23564.

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ffe72c98356..b9d7024f083 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5194,7 +5194,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
 
   // Emit the location of the prototype.
   if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
-Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
+Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
 
   return true;
 }
@@ -5239,7 +5239,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
 
   // Emit the location of the prototype.
   if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
-Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
+Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
 
   // This deletes the extra arguments.
   Call->shrinkNumArgs(NumParams);

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


[PATCH] D74222: [AArch64][SVE] Add mul/mla/mls lane & dup intrinsics

2020-02-07 Thread Kerry McLaughlin via Phabricator via cfe-commits
kmclaughlin created this revision.
kmclaughlin added reviewers: c-rhodes, sdesmalen, dancgr, efriedma.
Herald added subscribers: psnobl, rkruppe, hiraditya, kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: LLVM.

Implements the following intrinsics:

- @llvm.aarch64.sve.dup
- @llvm.aarch64.sve.mul.lane
- @llvm.aarch64.sve.mla.lane
- @llvm.aarch64.sve.mls.lane


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74222

Files:
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-scalar-to-vec.ll
  llvm/test/CodeGen/AArch64/sve2-intrinsics-int-mul-lane.ll

Index: llvm/test/CodeGen/AArch64/sve2-intrinsics-int-mul-lane.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve2-intrinsics-int-mul-lane.ll
@@ -0,0 +1,119 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve2 < %s | FileCheck %s
+
+;
+; MUL
+;
+
+define  @mul_lane_d( %a,  %b) {
+; CHECK-LABEL: mul_lane_d:
+; CHECK: mul z0.d, z0.d, z1.d[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mul.lane.nxv2i64( %a,
+ %b,
+i32 1)
+  ret  %out
+}
+
+define  @mul_lane_s( %a,  %b) {
+; CHECK-LABEL: mul_lane_s:
+; CHECK: mul z0.s, z0.s, z1.s[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mul.lane.nxv4i32( %a,
+ %b,
+i32 1)
+  ret  %out
+}
+
+define  @mul_lane_h( %a,  %b) {
+; CHECK-LABEL: mul_lane_h:
+; CHECK: mul z0.h, z0.h, z1.h[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mul.lane.nxv8i16( %a,
+ %b,
+i32 1)
+  ret  %out
+}
+
+;
+; MLA
+;
+
+define  @mla_lane_d( %a,  %b,  %c) {
+; CHECK-LABEL: mla_lane_d:
+; CHECK: mla z0.d, z1.d, z2.d[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mla.lane.nxv2i64( %a,
+ %b,
+ %c,
+i32 1)
+  ret  %out
+}
+
+define  @mla_lane_s( %a,  %b,  %c) {
+; CHECK-LABEL: mla_lane_s:
+; CHECK: mla z0.s, z1.s, z2.s[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mla.lane.nxv4i32( %a,
+ %b,
+ %c,
+i32 1)
+  ret  %out
+}
+
+define  @mla_lane_h( %a,  %b,  %c) {
+; CHECK-LABEL: mla_lane_h:
+; CHECK: mla z0.h, z1.h, z2.h[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mla.lane.nxv8i16( %a,
+ %b,
+ %c,
+i32 1)
+  ret  %out
+}
+
+;
+; MLS
+;
+
+define  @mls_lane_d( %a,  %b,  %c) {
+; CHECK-LABEL: mls_lane_d:
+; CHECK: mls z0.d, z1.d, z2.d[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mls.lane.nxv2i64( %a,
+ %b,
+ %c,
+i32 1)
+  ret  %out
+}
+
+define  @mls_lane_s( %a,  %b,  %c) {
+; CHECK-LABEL: mls_lane_s:
+; CHECK: mls z0.s, z1.s, z2.s[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mls.lane.nxv4i32( %a,
+ %b,
+ %c,
+i32 1)
+  ret  %out
+}
+
+define  @mls_lane_h( %a,  %b,  %c) {
+; CHECK-LABEL: mls_lane_h:
+; CHECK: mls z0.h, z1.h, z2.h[1]
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.mls.lane.nxv8i16( %a,
+ %b,
+ %c,
+i32 1)
+  ret  %out
+}
+
+declare  @llvm.aarch64.sve.mul.lane.nxv8i16(, , i32)
+declare  @llvm.aarch64.sve.mul.lane.nxv4i32(, , i32)
+declare  @llvm.aarch64.sve.mul.lane.nxv2i64(, , i32)
+declare  @llvm.aarch64.sve.mla.lane.nxv8i16(, , , i32)
+declare  @llvm.aarch64.sve.mla.lane.nxv4i32(, , , i32)
+declare  @llvm.aarch64.sve.

Re: patch via mailing list: Use getLocation() in too few/many arguments diagnostic

2020-02-07 Thread Hubert Tong via cfe-commits
I think this looks okay. I think Richard or Aaron might be able to provide
a more informed opinion.

-- HT

On Fri, Feb 7, 2020 at 10:06 AM John Marshall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Ping. I am a newcomer to Clang so don't know who might be appropriate
> reviewers to CC, so I've CCed a couple of general people from
> clang/CODE_OWNERS.TXT who may be able to forward as appropriate.
>
> Thanks,
>
> John
>
>
> On 20 Jan 2020, at 16:09, John Marshall wrote:
> >
> > This small patch improves the diagnostics when calling a function with
> the wrong number of arguments. For example, given
> >
> >   int
> >   foo(int a, int b);
> >
> >   int bar() { return foo(1); }
> >
> > The existing compiler shows the error message and a note for "foo
> declared here" showing only the "int" line. Ideally it would show the line
> containing the word "foo" instead, which it does after this patch. Also if
> the function declaration starts with some incidental macro, a trace of
> macro expansion is shown which is likely irrelevant. See for example
> https://github.com/samtools/htslib/issues/1013 and PR#23564.
> >
> > I have not contributed to LLVM before and I am unfamiliar with the
> difference between getBeginLoc() and getLocation() and the implications of
> using each. However this patch does fix PR#23564 and perhaps this fix would
> be mostly a no-brainer for those who are familiar with the code and these
> two location functions in particular?
>
> commit e8e73a04dd4274441541cc5fdc553cc4b26c00c3
> Author: John Marshall 
> Date:   Mon Jan 20 14:58:14 2020 +
>
> Use getLocation() in too few/many arguments diagnostic
>
> Use the more accurate location when emitting the location of the
> function being called's prototype in diagnostics emitted when calling
> a function with an incorrect number of arguments.
>
> In particular, avoids showing a trace of irrelevant macro expansions
> for "MY_EXPORT static int AwesomeFunction(int, int);". Fixes PR#23564.
>
> diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
> index ffe72c98356..b9d7024f083 100644
> --- a/clang/lib/Sema/SemaExpr.cpp
> +++ b/clang/lib/Sema/SemaExpr.cpp
> @@ -5194,7 +5194,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr
> *Fn,
>
>// Emit the location of the prototype.
>if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
> -Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
> +Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
>
>return true;
>  }
> @@ -5239,7 +5239,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr
> *Fn,
>
>// Emit the location of the prototype.
>if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
> -Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
> +Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
>
>// This deletes the extra arguments.
>Call->shrinkNumArgs(NumParams);
>
> ___
> 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] D73520: [analyzer] BugReporterVisitors: Refactor and documentation

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Thanks! Bug report generation seems far less of a mess than is used to be :)




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:44
 
-/// BugReporterVisitors are used to add custom diagnostics along a path.
+/// BugReporterVisitors are used to add custom diagnostics along a \emoji bug
+/// path. They also could serve false positive suppression.

I suspect `\emoji` wasn't intentional here, given that it needs another 
argument :^)


Repository:
  rC Clang

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

https://reviews.llvm.org/D73520



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


[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

2020-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 243167.
martong marked 6 inline comments as done.
martong added a comment.

- Describe what pure means


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897

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

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -9,7 +9,7 @@
 // This checker improves modeling of a few simple library functions.
 // It does not generate warnings.
 //
-// This checker provides a specification format - `FunctionSummaryTy' - and
+// This checker provides a specification format - `Summary' - and
 // contains descriptions of some library functions in this format. Each
 // specification contains a list of branches for splitting the program state
 // upon call, and range constraints on argument and return-value symbols that
@@ -21,7 +21,7 @@
 // consider standard C function `ispunct(int x)', which returns a non-zero value
 // iff `x' is a punctuation character, that is, when `x' is in range
 //   ['!', '/']   [':', '@']  U  ['[', '\`']  U  ['{', '~'].
-// `FunctionSummaryTy' provides only two branches for this function. However,
+// `Summary' provides only two branches for this function. However,
 // any attempt to describe this range with if-statements in the body farm
 // would result in many more branches. Because each branch needs to be analyzed
 // independently, this significantly reduces performance. Additionally,
@@ -30,13 +30,13 @@
 // which may lead to false positives because considering this particular path
 // was not consciously intended, and therefore it might have been unreachable.
 //
-// This checker uses eval::Call for modeling "pure" functions, for which
-// their `FunctionSummaryTy' is a precise model. This avoids unnecessary
-// invalidation passes. Conflicts with other checkers are unlikely because
-// if the function has no other effects, other checkers would probably never
-// want to improve upon the modeling done by this checker.
+// This checker uses eval::Call for modeling pure functions (functions without
+// side effets), for which their `Summary' is a precise model. This avoids
+// unnecessary invalidation passes. Conflicts with other checkers are unlikely
+// because if the function has no other effects, other checkers would probably
+// never want to improve upon the modeling done by this checker.
 //
-// Non-"pure" functions, for which only partial improvement over the default
+// Non-pure functions, for which only partial improvement over the default
 // behavior is expected, are modeled via check::PostCall, non-intrusively.
 //
 // The following standard C functions are currently supported:
@@ -64,49 +64,48 @@
   /// Below is a series of typedefs necessary to define function specs.
   /// We avoid nesting types here because each additional qualifier
   /// would need to be repeated in every function spec.
-  struct FunctionSummaryTy;
+  struct Summary;
 
   /// Specify how much the analyzer engine should entrust modeling this function
   /// to us. If he doesn't, he performs additional invalidations.
-  enum InvalidationKindTy { NoEvalCall, EvalCallAsPure };
+  enum InvalidationKind { NoEvalCall, EvalCallAsPure };
 
-  /// A pair of ValueRangeKindTy and IntRangeVectorTy would describe a range
+  /// A pair of ValueRangeKind and IntRangeVector would describe a range
   /// imposed on a particular argument or return value symbol.
   ///
   /// Given a range, should the argument stay inside or outside this range?
   /// The special `ComparesToArgument' value indicates that we should
   /// impose a constraint that involves other argument or return value symbols.
-  enum ValueRangeKindTy { OutOfRange, WithinRange, ComparesToArgument };
+  enum ValueRangeKind { OutOfRange, WithinRange, ComparesToArgument };
 
   // The universal integral type to use in value range descriptions.
   // Unsigned to make sure overflows are well-defined.
-  typedef uint64_t RangeIntTy;
+  typedef uint64_t RangeInt;
 
   /// Normally, describes a single range constraint, eg. {{0, 1}, {3, 4}} is
   /// a non-negative integer, which less than 5 and not equal to 2. For
   /// `ComparesToArgument', holds information about how exactly to compare to
   /// the argument.
-  typedef std::vector> IntRangeVectorTy;
+  typedef std::vector> IntRangeVector;
 
   /// A reference to an argument or return value by its number.
   /// ArgNo in CallExpr and CallEvent is defined as Unsigned, but
   /// obviously uint32_t should be enough for all practical purposes.
-  typedef uint32_t ArgNoTy;
-  static const ArgNoTy Ret = std::numeric_limits::max();
+  typedef uint32_t ArgNo;
+  static const ArgNo Ret = std::numeric_limits::m

[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

2020-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:33
 //
 // This checker uses eval::Call for modeling "pure" functions, for which
 // their `FunctionSummaryTy' is a precise model. This avoids unnecessary

Szelethus wrote:
> If we put pure in quotes, we might as well go the extra mile and quickly 
> explain what that means.
Ok, I removed the quotes and added the meaning of pure in parenthesis.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:42-49
 // The following standard C functions are currently supported:
 //
 //   fgetc  getline   isdigit   isupper
 //   fread  isalnum   isgraph   isxdigit
 //   fwrite isalpha   islower   read
 //   getc   isascii   isprint   write
 //   getcharisblank   ispunct

Szelethus wrote:
> I would prefer to just have a checker option that could print out the 
> currently modeled function rather than these lines of a recipe for outdated 
> comments.
Yes I agree, especially because I am planning to add a plethora of new 
functions in the future. I think that would be the appropriate time to 
implement the checker option.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:214-215
 
   // The map of all functions supported by the checker. It is initialized
   // lazily, and it doesn't change after initialization.
+  mutable llvm::StringMap FunctionSummaryMap;

Szelethus wrote:
> But why? This isn't important for this patch, so feel free to leave as-is, 
> but still.
I suppose this is because we cannot have a reference to the `ASTContext` in the 
constructor of the Checker. We can get that only via any of the checker 
callbacks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897



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


[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

2020-02-07 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

In D67983#1863019 , @arphaman wrote:

> @jyknight @rjmccall I'm not sure this change is 100% fine. For example, the 
> following code no longer compiles with ARC:
>
>   @protocol Delegate
>   @end
>  
>   @interface X 
>  
>   @end
>  
>   @interface Y
>   @property id d;
>   @end
>  
>   @implementation X
>  
>   + (void)foo:(Y *)y with:(X*)x {
>y.d = self; // error: assigning to 'id' from incompatible type 
> 'const Class'
>y.d = x; // fine
>   }
>  
>   @end
>


Your error looks correct to me -- "self" in a classmethod is not an instance, 
but the class itself. And while instances of X implement "Delegate", the Class 
does not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67983



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


Re: patch via mailing list: Use getLocation() in too few/many arguments diagnostic

2020-02-07 Thread Aaron Ballman via cfe-commits
Thank you for the patch -- I think the changes look reasonable, but it
should come with some test cases as well. Source location stuff is a
bit onerous to try to test, but I think the best approach would be to
add a new test that uses FileCheck instead of -verify so that you can
validate the source location's line and column are as expected in the
note. Once you have such a test (and have verified that no other tests
fail with your changes), I'm happy to commit on your behalf.

~Aaron

On Fri, Feb 7, 2020 at 10:23 AM Hubert Tong
 wrote:
>
> I think this looks okay. I think Richard or Aaron might be able to provide a 
> more informed opinion.
>
> -- HT
>
> On Fri, Feb 7, 2020 at 10:06 AM John Marshall via cfe-commits 
>  wrote:
>>
>> Ping. I am a newcomer to Clang so don't know who might be appropriate 
>> reviewers to CC, so I've CCed a couple of general people from 
>> clang/CODE_OWNERS.TXT who may be able to forward as appropriate.
>>
>> Thanks,
>>
>> John
>>
>>
>> On 20 Jan 2020, at 16:09, John Marshall wrote:
>> >
>> > This small patch improves the diagnostics when calling a function with the 
>> > wrong number of arguments. For example, given
>> >
>> >   int
>> >   foo(int a, int b);
>> >
>> >   int bar() { return foo(1); }
>> >
>> > The existing compiler shows the error message and a note for "foo declared 
>> > here" showing only the "int" line. Ideally it would show the line 
>> > containing the word "foo" instead, which it does after this patch. Also if 
>> > the function declaration starts with some incidental macro, a trace of 
>> > macro expansion is shown which is likely irrelevant. See for example 
>> > https://github.com/samtools/htslib/issues/1013 and PR#23564.
>> >
>> > I have not contributed to LLVM before and I am unfamiliar with the 
>> > difference between getBeginLoc() and getLocation() and the implications of 
>> > using each. However this patch does fix PR#23564 and perhaps this fix 
>> > would be mostly a no-brainer for those who are familiar with the code and 
>> > these two location functions in particular?
>>
>> commit e8e73a04dd4274441541cc5fdc553cc4b26c00c3
>> Author: John Marshall 
>> Date:   Mon Jan 20 14:58:14 2020 +
>>
>> Use getLocation() in too few/many arguments diagnostic
>>
>> Use the more accurate location when emitting the location of the
>> function being called's prototype in diagnostics emitted when calling
>> a function with an incorrect number of arguments.
>>
>> In particular, avoids showing a trace of irrelevant macro expansions
>> for "MY_EXPORT static int AwesomeFunction(int, int);". Fixes PR#23564.
>>
>> diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
>> index ffe72c98356..b9d7024f083 100644
>> --- a/clang/lib/Sema/SemaExpr.cpp
>> +++ b/clang/lib/Sema/SemaExpr.cpp
>> @@ -5194,7 +5194,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
>>
>>// Emit the location of the prototype.
>>if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
>> -Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
>> +Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
>>
>>return true;
>>  }
>> @@ -5239,7 +5239,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
>>
>>// Emit the location of the prototype.
>>if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
>> -Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
>> +Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
>>
>>// This deletes the extra arguments.
>>Call->shrinkNumArgs(NumParams);
>>
>> ___
>> 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] D74070: [Clang] Don't let gen crash diagnostics fail when '#pragma clang __debug crash' is used

2020-02-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

Relanded as rG75f09b54429bee17a96e2ba7a2ac0f0a8a7f7e74 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74070



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


[PATCH] D73536: [analyzer][taint] Remove taint from symbolic expressions if used in comparisons

2020-02-07 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

You cannot always have constant bounds. E.g. a dynamically allocated array size 
might depend on a variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73536



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


[PATCH] D74076: [Clang][Driver] Remove -M group options before generating crash diagnostics

2020-02-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea added a comment.

Relanded as rG75f09b54429bee17a96e2ba7a2ac0f0a8a7f7e74 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74076



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


[PATCH] D73897: [analyzer] StdLibraryFunctionsChecker refactor: remove macros

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:42-49
 // The following standard C functions are currently supported:
 //
 //   fgetc  getline   isdigit   isupper
 //   fread  isalnum   isgraph   isxdigit
 //   fwrite isalpha   islower   read
 //   getc   isascii   isprint   write
 //   getcharisblank   ispunct

martong wrote:
> Szelethus wrote:
> > I would prefer to just have a checker option that could print out the 
> > currently modeled function rather than these lines of a recipe for outdated 
> > comments.
> Yes I agree, especially because I am planning to add a plethora of new 
> functions in the future. I think that would be the appropriate time to 
> implement the checker option.
Totally agreed! Thank you for the cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73897



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


[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

2020-02-07 Thread Alexey Bader via Phabricator via cfe-commits
bader updated this revision to Diff 243173.
bader marked 2 inline comments as done.
bader added a comment.

Applied code review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Driver/sycl.c
  clang/test/Preprocessor/sycl-macro.cpp

Index: clang/test/Preprocessor/sycl-macro.cpp
===
--- clang/test/Preprocessor/sycl-macro.cpp
+++ clang/test/Preprocessor/sycl-macro.cpp
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -E -dM | FileCheck %s
-// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefix=CHECK-SYCL %s
+// RUN: %clang_cc1 %s -fsycl -sycl-std=2015 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -sycl-std=1.2.1 -E -dM | FileCheck --check-prefix=CHECK-SYCL-STD %s
+// RUN: %clang_cc1 %s -fsycl-is-device -E -dM | FileCheck --check-prefixes=CHECK-SYCL %s
 
 // CHECK-NOT:#define __SYCL_DEVICE_ONLY__ 1
+// CHECK-NOT:#define CL_SYCL_LANGUAGE_VERSION 121
+// CHECK-SYCL-STD:#define CL_SYCL_LANGUAGE_VERSION 121
 // CHECK-SYCL:#define __SYCL_DEVICE_ONLY__ 1
Index: clang/test/Driver/sycl.c
===
--- clang/test/Driver/sycl.c
+++ clang/test/Driver/sycl.c
@@ -1,10 +1,20 @@
 // RUN: %clang -### -fsycl -c %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clang -### -fno-sycl -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
 // RUN: %clangxx -### -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### -fsycl -fno-sycl %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 // RUN: %clangxx -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang_cl -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### -fsycl %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang_cl -### %s 2>&1 | FileCheck %s --check-prefix=DISABLED
 
 // ENABLED: "-cc1"{{.*}} "-fsycl-is-device"
+// ENABLED-SAME: "-sycl-std={{[-.sycl0-9]+}}"
 // DISABLED-NOT: "-fsycl-is-device"
+// DISABLED-NOT: "-sycl-std="
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -450,6 +450,16 @@
 if (LangOpts.FastRelaxedMath)
   Builder.defineMacro("__FAST_RELAXED_MATH__");
   }
+
+  // SYCL Version is set to a value when building SYCL applications
+  switch (LangOpts.getSYCLVersion()) {
+  case LangOptions::SYCLVersionList::SYCL_2015:
+Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
+break;
+  case LangOptions::SYCLVersionList::Undefined:
+break;
+  }
+
   // Not "standard" per se, but available even with the -undef flag.
   if (LangOpts.AsmPreprocessor)
 Builder.defineMacro("__ASSEMBLER__");
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2544,6 +2544,26 @@
   LangStd = OpenCLLangStd;
   }
 
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {
+// -sycl-std applies to any SYCL source, not only those containing kernels,
+// but also those using the SYCL API
+if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
+  Opts.setSYCLVersion(
+  llvm::StringSwitch(A->getValue())
+  .Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+ LangOptions::SYCLVersionList::SYCL_2015)
+  .Default(LangOptions::SYCLVersionList::Undefined));
+
+  if (Opts.getSYCLVersion() == LangOptions::SYCLVersionList::Undefined) {
+// User has passed an invalid value to the flag, this is an error
+Diags.Report(diag::err_drv_invalid_value)
+<< A->getAsString(Args) << A->getValue();
+  }
+}
+  }
+
   Opts.IncludeDefaultHead

[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

2020-02-07 Thread Alexey Bader via Phabricator via cfe-commits
bader marked an inline comment as done.
bader added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3414-3416
+def fsycl : Flag<["-"], "fsycl">, Group, Flags<[NoArgumentUnused, 
CoreOption]>,
   HelpText<"Enable SYCL kernels compilation for device">;
+def fno_sycl : Flag<["-"], "fno-sycl">, Group, 
Flags<[NoArgumentUnused, CoreOption]>,

ABataev wrote:
> These flags should not be ignored, `NoArgumentUnused` should be applied to 
> this flags.
Do you mean "`NoArgumentUnused` should **not** be applied"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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


[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D72705#1863499 , @balazske wrote:

> If more discussion is needed it is better to use "Discourse" instead of this 
> review?


I've been meaning to suggest that we use either that or discord for casual 
chatting (for simple questions etc), but given that we already have a working 
prototype, I think it'd be better to keep the conversation here for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705



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


[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

2020-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

In D73898#1863710 , @Szelethus wrote:

> I wouldn't like to see reports emitted by a checker that resides in 
> `apiModeling`. Could we create a new one? Some checkers, like the 
> `IteratorChecker`, `MallocChecker` and `CStringChecker` implement a variety 
> of user-facing checkers within the same class, that is also an option, if 
> creating a new checker class is too much of a hassle.


Yes, we could split the warning emitting part to a new checker. My concern with 
that is in that case we would have the argument constraining part in 
checkPostCall still in this checker, because that is part of the modelling. And 
actually it makes sense to apply the argument constraints only if we know for 
sure that they are not violated. The violation then would  be checked in the 
new checker, this seems a bit awkward to me. Because checking the violation of 
the constraints and applying the constraints seems to be a cohesive action to 
me. I mean it would not even make sense to turn off the warning checker, 
because then we'd be applying the constraints blindly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73898



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


[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

2020-02-07 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3414-3416
+def fsycl : Flag<["-"], "fsycl">, Group, Flags<[NoArgumentUnused, 
CoreOption]>,
   HelpText<"Enable SYCL kernels compilation for device">;
+def fno_sycl : Flag<["-"], "fno-sycl">, Group, 
Flags<[NoArgumentUnused, CoreOption]>,

bader wrote:
> ABataev wrote:
> > These flags should not be ignored, `NoArgumentUnused` should be applied to 
> > this flags.
> Do you mean "`NoArgumentUnused` should **not** be applied"?
Yes, missed `not`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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


[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-02-07 Thread Steve Canon via Phabricator via cfe-commits
scanon added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2077
+getDataLayout().getABITypeAlignment(getTypes().ConvertType(RetTy)))
+  SRETAttrs.addAlignmentAttr(Align);
 ArgAttrs[IRFunctionArgs.getSRetArgNo()] =

rjmccall wrote:
> Why only when under-aligned?  Just to avoid churning tests?  I think we 
> should apply this unconditionally.
On mainstream architectures today, there's rarely a benefit to knowing about 
higher alignment (e.g. MOVUPS is just as fast as MOVAPS if the address is 
actually aligned), so we won't see significant perf wins from preserving 
over-alignment in most cases, but it also doesn't cost us anything AFAICT and 
could deliver wins in some specific cases (e.g. AVX on SNB and IVB, where I 
think we split underaligned 256b stores into two 128b chunks).

So, yeah, I think we ought to simply unconditionally add the alignment to the 
sret.


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

https://reviews.llvm.org/D74183



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


[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

2020-02-07 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2547
 
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);

`-fno-sycl` should not be passed to frontend. Just use `hasFlag(OPT_fsycl)`.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

This option also must be controlled by `-fsycl`:
```
Opts.SYCLIsDevice =  Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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


[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

2020-02-07 Thread Alexey Bader via Phabricator via cfe-commits
bader marked an inline comment as done.
bader added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

ABataev wrote:
> This option also must be controlled by `-fsycl`:
> ```
> Opts.SYCLIsDevice =  Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
> 
> ```
Does it really has to? This logic is already present in the driver and it makes 
front-end tests verbose `%clang_cc1 -fsycl -fsycl-is-device`.
Can `-fsycl-is-device` imply `-fsycl`?
Looking how CUDA/OpenMP options are handled, not all of them are processed 
using this pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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


[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

2020-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong marked an inline comment as done.
martong added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:697-699
+  // The behavior is undefined if the value of the argument is not
+  // representable as unsigned char or is not equal to EOF. See e.g. C99
+  // 7.4.1.2 The isalpha function (p: 181-182).

Szelethus wrote:
> This is true for the rest of the summaries as well, but shouldn't we retrieve 
> the `unsigned char` size from `ASTContext`?
Yes this is a good idea. I will do this.

What bothers me really much, however, is that we should handle EOF in a 
platform dependent way as well ... and I have absolutely no idea how to do that 
given that is defined by a macro in a platform specific header file. I am 
desperately in need for help and ideas about how could we get the value of EOF 
for the analysed platform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73898



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


[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

2020-02-07 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 243178.
balazske added a comment.

Adding a simplified version.

- The check is only for "EOFOrNegative" case.
- Long list of functions is removed.
- Not checking for "access in condition".
- Removed the warning if value is unused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72705

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/error-return.c

Index: clang/test/Analysis/error-return.c
===
--- /dev/null
+++ clang/test/Analysis/error-return.c
@@ -0,0 +1,103 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.unix.ErrorReturn -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+FILE *file();
+
+void test_EOFOrNeg_LT() {
+  int X = fputs("str", file());
+  if (X < 0) {
+  }
+}
+
+void test_EOFOrNeg_LT_Wrong1() {
+  int X = fputs("str", file());
+  if (X < 1) { // expected-warning{{Use of return value that was not checked}}
+  }
+}
+
+void test_EOFOrNeg_LT_Wrong2() {
+  int X = fputs("str", file());
+  if (X < -1) { // expected-warning{{Use of return value that was not checked}}
+  }
+}
+
+void test_EOFOrNeg_GT() {
+  int X = fputs("str", file());
+  if (0 > X) {
+  }
+}
+
+void test_EOFOrNeg_EQ() {
+  int X = fputs("str", file());
+  if (X == -1) {
+  }
+}
+
+void test_EOFOrNeg_EQ_Wrong() {
+  int X = fputs("str", file());
+  if (X == 0) { // expected-warning{{Use of return value that was not checked}}
+  }
+}
+
+void test_EOFOrNeg_NE() {
+  int X = fputs("str", file());
+  if (X != -1) {
+  }
+}
+
+void test_EOFOrNeg_LNot() {
+  int X = fputs("str", file());
+  if (!X) { // expected-warning{{Use of return value that was not checked}}
+  }
+}
+
+void test_EOFOrNeg_If() {
+  int X = fputs("str", file());
+  // FIXME: This should be detected.
+  if (X) {
+  }
+}
+
+void test_EOFOrNeg_Call_If() {
+  // FIXME: This should be detected.
+  if (fputs("str", file())) {
+  }
+}
+
+void test_EOFOrNeg_Use() {
+  int X = fputs("str", file());
+  int Y = X + 1; // expected-warning{{Use of return value that was not checked}}
+}
+
+void test_EOFOrNeg_UseSyscall() {
+  int X = fputs("str", file());
+  fakeSystemHeaderCallIntVal(X); // expected-warning{{Use of return value that was not checked}}
+}
+
+void test_EOFOrNeg_UseSyscall_NoVar() {
+  fakeSystemHeaderCallIntVal(fputs("str", file())); // expected-warning{{Use of return value that was not checked}}
+}
+
+void unknown1(int);
+
+void test_EOFOrNeg_EscapeFunction() {
+  int X = fputs("str", file());
+  unknown1(X);
+  int Y = X + 1;
+}
+
+int GlobalInt;
+
+void test_EOFOrNeg_EscapeAssignment() {
+  GlobalInt = fputs("str", file());
+  int X = GlobalInt;
+}
+
+void test_EOFOrNeg_LT_NoErrorAfterCheck() {
+  int X = fputs("str", file());
+  if (X < 0) {
+  }
+  if (X < 1) {
+  }
+}
Index: clang/test/Analysis/Inputs/system-header-simulator.h
===
--- clang/test/Analysis/Inputs/system-header-simulator.h
+++ clang/test/Analysis/Inputs/system-header-simulator.h
@@ -53,6 +53,7 @@
  int (*)(void *, const char *, int),
  fpos_t (*)(void *, fpos_t, int),
  int (*)(void *));
+int fputs(const char *restrict, FILE *restrict);
 
 int sqlite3_bind_text_my(int, const char*, int n, void(*)(void*));
 
@@ -82,6 +83,7 @@
 //The following are fake system header functions for generic testing.
 void fakeSystemHeaderCallInt(int *);
 void fakeSystemHeaderCallIntPtr(int **);
+void fakeSystemHeaderCallIntVal(int);
 
 // Some data strauctures may hold onto the pointer and free it later.
 void fake_insque(void *, void *);
@@ -112,4 +114,4 @@
 #define NULL __DARWIN_NULL
 #endif
 
-#define offsetof(t, d) __builtin_offsetof(t, d)
\ No newline at end of file
+#define offsetof(t, d) __builtin_offsetof(t, d)
Index: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp
@@ -0,0 +1,482 @@
+//===-- ErrorReturnChecker.cpp *- 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 ErrorReturnChecker, a builtin checker that checks for
+// error checking of certain C API function return values.
+// This check is taken from SEI CERT ERR33-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors
+//
+// About the 

[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

2020-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

In D73898#1864066 , @martong wrote:

> In D73898#1863710 , @Szelethus wrote:
>
> > I wouldn't like to see reports emitted by a checker that resides in 
> > `apiModeling`. Could we create a new one? Some checkers, like the 
> > `IteratorChecker`, `MallocChecker` and `CStringChecker` implement a variety 
> > of user-facing checkers within the same class, that is also an option, if 
> > creating a new checker class is too much of a hassle.
>
>
> ... And actually it makes sense to apply the argument constraints only if we 
> know for sure that they are not violated. ...


What I mean by that is that we must do over-approximation if the argument is 
symbolic. I.e. we presume that the constraints do hold otherwise the program 
would be ill-formed and there is no point to continue the analysis on this 
path. It is very similar to what we do in case of the DivZero or the NullDeref 
Checkers: if there is no violation (no warning) and the variable is symbolic 
then we constrain the value by the condition. E.g. in DivZero::checkPreStmt we 
have:

  // If we get here, then the denom should not be zero. We abandon the implicit
  // zero denom case for now.
  C.addTransition(stateNotZero);

Strictly speaking, these transitions should be part of the modeling then in 
this sense (and they should be in PostStmt?). Still they are not separated into 
a different checker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73898



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


[PATCH] D73898: [analyzer] StdLibraryFunctionsChecker: Add argument constraints

2020-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

> What I mean by that is that we must do over-approximation if the argument is 
> symbolic. I.e. we presume that the constraints do hold otherwise the program 
> would be ill-formed and there is no point to continue the analysis on this 
> path.

Sorry, that's actually under-approximation because we elide paths.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73898



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


[PATCH] D73701: [clang] fix linkage of nested lambda

2020-02-07 Thread Philippe Daouadi via Phabricator via cfe-commits
blastrock added a comment.

Thanks for the reviews! I do not have commit access, so please merge this 
whenever you can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73701



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


[PATCH] D74229: [Clang] Cover '#pragma clang __debug overflow_stack' in tests

2020-02-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done.
aganea added inline comments.



Comment at: clang/lib/Lex/Pragma.cpp:1148
 
+#ifdef __clang__
+  __attribute__((optnone))

Would it be better if we had something like `LLVM_[DISABLE|ENABLE]_OPT` in 
`Compiler.h` instead of the #ifdefs? That would map to `#pragma clang optimize 
[off|on]` (and corresponding #pragma on MSVC/GCC). Note that there's no way I 
know of to disable optimizations on a per-function basis on MSVC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74229



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


[PATCH] D74229: [Clang] Cover '#pragma clang __debug overflow_stack' in tests

2020-02-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea created this revision.
aganea added reviewers: rnk, hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
aganea marked an inline comment as done.
aganea added inline comments.



Comment at: clang/lib/Lex/Pragma.cpp:1148
 
+#ifdef __clang__
+  __attribute__((optnone))

Would it be better if we had something like `LLVM_[DISABLE|ENABLE]_OPT` in 
`Compiler.h` instead of the #ifdefs? That would map to `#pragma clang optimize 
[off|on]` (and corresponding #pragma on MSVC/GCC). Note that there's no way I 
know of to disable optimizations on a per-function basis on MSVC.


Previously, `#pragma clang __debug overflow_stack` wasn't really working, the 
function was simply looping and not overflowing.
I tested this in various configurations, with Clang 9, MSVC 2017/2019 and GCC 9 
on Ubuntu.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74229

Files:
  clang/lib/Lex/Pragma.cpp
  clang/test/Driver/crash-report.c


Index: clang/test/Driver/crash-report.c
===
--- clang/test/Driver/crash-report.c
+++ clang/test/Driver/crash-report.c
@@ -21,12 +21,20 @@
 // RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
 // RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
 
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1  \
+// RUN:  CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1 \
+// RUN:  not %clang %s @%t.rsp -DOVERFLOW 2>&1 | FileCheck %s
+// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
+// RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
+
 // REQUIRES: crash-recovery
 
 #ifdef PARSER
 #pragma clang __debug parser_crash
 #elif CRASH
 #pragma clang __debug crash
+#elif OVERFLOW
+#pragma clang __debug overflow_stack
 #endif
 
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -1145,16 +1145,24 @@
 /*IsReinject=*/false);
   }
 
+#ifdef __clang__
+  __attribute__((optnone))
+#elif __GNUC__
+  __attribute__((optimize("O0")))
+#elif _MSC_VER
 // Disable MSVC warning about runtime stack overflow.
-#ifdef _MSC_VER
-#pragma warning(disable : 4717)
+#pragma warning(disable : 4717)
+#pragma optimize("", off)
 #endif
-  static void DebugOverflowStack(void (*P)() = nullptr) {
-void (*volatile Self)(void(*P)()) = DebugOverflowStack;
-Self(reinterpret_cast(Self));
-  }
-#ifdef _MSC_VER
-#pragma warning(default : 4717)
+  static void
+  DebugOverflowStack(void (*P)() = nullptr, unsigned *V = nullptr) {
+unsigned A[16384]{1};
+void (*volatile Self)(void (*P)(), unsigned *) = DebugOverflowStack;
+Self(reinterpret_cast(Self), A);
+  }
+#if !defined(__clang__) && defined(_MSC_VER)
+#pragma optimize("", on)
+#pragma warning(default : 4717)
 #endif
 };
 


Index: clang/test/Driver/crash-report.c
===
--- clang/test/Driver/crash-report.c
+++ clang/test/Driver/crash-report.c
@@ -21,12 +21,20 @@
 // RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
 // RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
 
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1  \
+// RUN:  CC_PRINT_HEADERS=1 CC_LOG_DIAGNOSTICS=1 \
+// RUN:  not %clang %s @%t.rsp -DOVERFLOW 2>&1 | FileCheck %s
+// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s
+// RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s
+
 // REQUIRES: crash-recovery
 
 #ifdef PARSER
 #pragma clang __debug parser_crash
 #elif CRASH
 #pragma clang __debug crash
+#elif OVERFLOW
+#pragma clang __debug overflow_stack
 #endif
 
 // CHECK: Preprocessed source(s) and associated run script(s) are located at:
Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -1145,16 +1145,24 @@
 /*IsReinject=*/false);
   }
 
+#ifdef __clang__
+  __attribute__((optnone))
+#elif __GNUC__
+  __attribute__((optimize("O0")))
+#elif _MSC_VER
 // Disable MSVC warning about runtime stack overflow.
-#ifdef _MSC_VER
-#pragma warning(disable : 4717)
+#pragma warning(disable : 4717)
+#pragma optimize("", off)
 #endif
-  static void DebugOverflowStack(void (*P)() = nullptr) {
-void (*volatile Self)(void(*P)()) = DebugOverflowStack;
-Self(reinterpret_cast(Self));
-  }
-#ifdef _MSC_VER
-#pragma warning(default : 4717)
+  static void
+  DebugOverflowStack(void (*P)() = nullptr, unsigned *V = nullptr) {
+unsigned A[16384]{1};
+void (*volatile Self)(void (*P)(), unsigned *) = DebugOverflowStack;
+Self(reinterpret_cast(S

[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

2020-02-07 Thread Diogo N. Sampaio via Phabricator via cfe-commits
dnsampaio updated this revision to Diff 243197.
dnsampaio added a comment.

Added opt-out flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72932

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGRecordLayout.h
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/aapcs-bitfield.c
  clang/test/CodeGen/bitfield-2.c

Index: clang/test/CodeGen/bitfield-2.c
===
--- clang/test/CodeGen/bitfield-2.c
+++ clang/test/CodeGen/bitfield-2.c
@@ -1,3 +1,4 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -O3 -o %t.opt.ll %s \
 // RUN:   -fdump-record-layouts > %t.dump.txt
 // RUN: FileCheck -check-prefix=CHECK-RECORD < %t.dump.txt %s
@@ -14,7 +15,7 @@
 // CHECK-RECORD:   LLVMType:%struct.s0 = type { [3 x i8] }
 // CHECK-RECORD:   IsZeroInitializable:1
 // CHECK-RECORD:   BitFields:[
-// CHECK-RECORD: 
+// CHECK-RECORD: 
-// CHECK-RECORD: 
+// CHECK-RECORD: 
+// CHECK-RECORD: 
-// CHECK-RECORD: 
+// CHECK-RECORD: 
 
 struct __attribute__((packed)) s9 {
Index: clang/test/CodeGen/aapcs-bitfield.c
===
--- clang/test/CodeGen/aapcs-bitfield.c
+++ clang/test/CodeGen/aapcs-bitfield.c
@@ -1,8 +1,12 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
-// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 | FileCheck %s -check-prefix=LE
-// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 | FileCheck %s -check-prefix=BE
-// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad | FileCheck %s -check-prefixes=LE,LENUMLOADS
-// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad | FileCheck %s -check-prefixes=BE,BENUMLOADS
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 -fno-AAPCSBitfieldWidth | FileCheck %s -check-prefix=LE
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 -fno-AAPCSBitfieldWidth | FileCheck %s -check-prefix=BE
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad -fno-AAPCSBitfieldWidth | FileCheck %s -check-prefixes=LE,LENUMLOADS
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad -fno-AAPCSBitfieldWidth | FileCheck %s -check-prefixes=BE,BENUMLOADS
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 | FileCheck %s -check-prefix=LEWIDTH
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 | FileCheck %s -check-prefix=BEWIDTH
+// RUN: %clang_cc1 -triple armv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad | FileCheck %s -check-prefixes=LEWIDTH,LEWIDTHNUM
+// RUN: %clang_cc1 -triple armebv8-none-linux-eabi %s -emit-llvm -o - -O3 -fAAPCSBitfieldLoad | FileCheck %s -check-prefixes=BEWIDTH,BEWIDTHNUM
 
 struct st0 {
   short c : 7;
@@ -24,6 +28,22 @@
 // BE-NEXT:[[BF_ASHR:%.*]] = ashr i8 [[BF_LOAD]], 1
 // BE-NEXT:[[CONV:%.*]] = sext i8 [[BF_ASHR]] to i32
 // BE-NEXT:ret i32 [[CONV]]
+// LEWIDTH-LABEL: @st0_check_load(
+// LEWIDTH-NEXT:  entry:
+// LEWIDTH-NEXT:[[TMP0:%.*]] = getelementptr [[STRUCT_ST0:%.*]], %struct.st0* [[M:%.*]], i32 0, i32 0
+// LEWIDTH-NEXT:[[BF_LOAD:%.*]] = load i8, i8* [[TMP0]], align 2
+// LEWIDTH-NEXT:[[BF_SHL:%.*]] = shl i8 [[BF_LOAD]], 1
+// LEWIDTH-NEXT:[[BF_ASHR:%.*]] = ashr exact i8 [[BF_SHL]], 1
+// LEWIDTH-NEXT:[[CONV:%.*]] = sext i8 [[BF_ASHR]] to i32
+// LEWIDTH-NEXT:ret i32 [[CONV]]
+//
+// BEWIDTH-LABEL: @st0_check_load(
+// BEWIDTH-NEXT:  entry:
+// BEWIDTH-NEXT:[[TMP0:%.*]] = getelementptr [[STRUCT_ST0:%.*]], %struct.st0* [[M:%.*]], i32 0, i32 0
+// BEWIDTH-NEXT:[[BF_LOAD:%.*]] = load i8, i8* [[TMP0]], align 2
+// BEWIDTH-NEXT:[[BF_ASHR:%.*]] = ashr i8 [[BF_LOAD]], 1
+// BEWIDTH-NEXT:[[CONV:%.*]] = sext i8 [[BF_ASHR]] to i32
+// BEWIDTH-NEXT:ret i32 [[CONV]]
 //
 int st0_check_load(struct st0 *m) {
   return m->c;
@@ -46,6 +66,23 @@
 // BE-NEXT:[[BF_SET:%.*]] = or i8 [[BF_CLEAR]], 2
 // BE-NEXT:store i8 [[BF_SET]], i8* [[TMP0]], align 2
 // BE-NEXT:ret void
+// LEWIDTH-LABEL: @st0_check_store(
+// LEWIDTH-NEXT:  entry:
+// LEWIDTH-NEXT:[[TMP0:%.*]] = getelementptr [[STRUCT_ST0:%.*]], %struct.st0* [[M:%.*]], i32 0, i32 0
+// LEWIDTH-NEXT:[[BF_LOAD:%.*]] = load i8, i8* [[TMP0]], align 2
+// LEWIDTH-NEXT:[[BF_CLEAR:%.*]] = and i8 [[BF_LOAD]], -128
+// LEWIDTH-NEXT:[[BF_SET:%.*]] = or i8 [[BF_CLEAR]], 1
+// LEWIDTH-NEXT:store i8 [[BF_SET]], i8* [[TMP0]], align 2
+// LEWIDTH-NEXT:ret void
+//
+// BEWIDTH-

[PATCH] D73916: [clang] Add `forceReload` clangd extension to 'textDocument/didChange'

2020-02-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Thanks for this. It makes me a bit sad but I don't really see a clean way to 
make this "just work".




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:653
 
-  Server->addDocument(File, *Contents, WantDiags);
+  Server->addDocument(File, *Contents, WantDiags, ForceReload);
 }

Rather than plumbing this down many levels as yet-another-bool-param, can we 
shove it into either ParseInputs or ParseOptions?
e.g. `bool ParseInputs::AllowCached = true;` and set it to false here?

It's kind of a stretch for the API as we won't make use of it everywhere that 
ParseInputs is used, but I think it's better than adding a niche boolean param 
everywhere.



Comment at: clang-tools-extra/clangd/Protocol.h:656
+
+  /// Force the preamble to be rebuilt for this version of the file (only when
+  /// present and set to true). This is a workaround for how our heuristics for

Nit: the preamble is an implementation detail here, an even higher-level 
description would be e.g.

Force a complete rebuild of the file, ignoring all cached state. Slow!
This is useful to defeat clangd's assumption that missing headers stay missing.



Comment at: clang-tools-extra/clangd/Protocol.h:661
+  /// This is a clangd extension.
+  llvm::Optional forceReload;
 };

just `bool forceReload = false;` - we don't bother modelling this as a tristate 
if there's a well-defined defaut.

(Though I'm sure there are places where we are inconsistent...)



Comment at: clang-tools-extra/clangd/Protocol.h:661
+  /// This is a clangd extension.
+  llvm::Optional forceReload;
 };

sammccall wrote:
> just `bool forceReload = false;` - we don't bother modelling this as a 
> tristate if there's a well-defined defaut.
> 
> (Though I'm sure there are places where we are inconsistent...)
somehow "rebuild" seems clearer than "reload" to me, because we're not being 
very specific about what we're loading.

Also `allowCache` (with inverted sense) might be clearer.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:435
 
 std::shared_ptr OldPreamble =
 getPossiblyStalePreamble();

no need to pass a separate parameter through to buildPreamble - if you're 
forcing a preamble rebuild then just don't bother getting the old preamble. 
OldPreamble is null and you get the behaviour you want.

(The logging will be slightly off - "first preamble" - but I don't think it's 
worth fixing)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73916



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


[PATCH] D74116: [Sema][C++] Strawman patch to propagate conversion type in order to specialize the diagnostics

2020-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

This shouldn't be an expensive function call, and being a little inefficient on 
the error path to get better diagnostics is a good trade-off.


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

https://reviews.llvm.org/D74116



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


[PATCH] D74229: [Clang] Cover '#pragma clang __debug overflow_stack' in tests

2020-02-07 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea planned changes to this revision.
aganea added a comment.

Withdrawing this, I found an crash in Debug build. I think this can't work 
without doing all the crash processing in a separate, pre-allocated thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74229



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


[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

2020-02-07 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2548
+  Opts.SYCL = Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false);
+  Opts.SYCLIsDevice = Args.hasArg(options::OPT_fsycl_is_device);
+  if (Opts.SYCL || Opts.SYCLIsDevice) {

bader wrote:
> ABataev wrote:
> > This option also must be controlled by `-fsycl`:
> > ```
> > Opts.SYCLIsDevice =  Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
> > 
> > ```
> Does it really has to? This logic is already present in the driver and it 
> makes front-end tests verbose `%clang_cc1 -fsycl -fsycl-is-device`.
> Can `-fsycl-is-device` imply `-fsycl`?
> Looking how CUDA/OpenMP options are handled, not all of them are processed 
> using this pattern.
In general, this is how we handle it in OpenMP. Cuda works differently, because 
it has its own kind of files (.cu) and Cuda is triggered by the language switch 
(-x cu). Seems to me, you're using something close to OpenMP model, no? Or do 
you want to define your own language kind just like Cuda?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857



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


[PATCH] D74234: [clang] [NFC] Change boolean SuppressUserConversions parameters to an enum

2020-02-07 Thread Logan Smith via Phabricator via cfe-commits
logan-5 created this revision.
logan-5 added a reviewer: rsmith.
logan-5 added a project: clang.
Herald added a subscriber: cfe-commits.
logan-5 added a child revision: D74238: [clang] [clang] Improve diagnostic note 
for implicit conversion sequences that would work if more than one implicit 
user-defined conversion were allowed..

This is groundwork for a subsequent patch that adds a third mode for searching 
for user conversions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74234

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp

Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1317,7 +1317,7 @@
 /// is not an option. See TryImplicitConversion for more information.
 static ImplicitConversionSequence
 TryUserDefinedConversion(Sema &S, Expr *From, QualType ToType,
- bool SuppressUserConversions,
+ Sema::UserDefinedConversions UserConversions,
  AllowedExplicit AllowExplicit,
  bool InOverloadResolution,
  bool CStyle,
@@ -1325,7 +1325,7 @@
  bool AllowObjCConversionOnExplicit) {
   ImplicitConversionSequence ICS;
 
-  if (SuppressUserConversions) {
+  if (UserConversions == Sema::UserDefinedConversions::Suppress) {
 // We're not in the case above, so there is no conversion that
 // we can perform.
 ICS.setBad(BadConversionSequence::no_conversion, From, ToType);
@@ -1410,8 +1410,8 @@
 /// but will instead return an implicit conversion sequence of kind
 /// "BadConversion".
 ///
-/// If @p SuppressUserConversions, then user-defined conversions are
-/// not permitted.
+/// If @p UserConversions is UserDefinedConversions::Suppress, then 
+/// user-defined conversions are not permitted.
 /// If @p AllowExplicit, then explicit user-defined conversions are
 /// permitted.
 ///
@@ -1420,7 +1420,7 @@
 /// be initialized with __strong id* or __weak id* arguments.
 static ImplicitConversionSequence
 TryImplicitConversion(Sema &S, Expr *From, QualType ToType,
-  bool SuppressUserConversions,
+  Sema::UserDefinedConversions UserConversions,
   AllowedExplicit AllowExplicit,
   bool InOverloadResolution,
   bool CStyle,
@@ -1467,7 +1467,7 @@
 return ICS;
   }
 
-  return TryUserDefinedConversion(S, From, ToType, SuppressUserConversions,
+  return TryUserDefinedConversion(S, From, ToType, UserConversions,
   AllowExplicit, InOverloadResolution, CStyle,
   AllowObjCWritebackConversion,
   AllowObjCConversionOnExplicit);
@@ -1475,12 +1475,12 @@
 
 ImplicitConversionSequence
 Sema::TryImplicitConversion(Expr *From, QualType ToType,
-bool SuppressUserConversions,
+Sema::UserDefinedConversions UserConversions,
 AllowedExplicit AllowExplicit,
 bool InOverloadResolution,
 bool CStyle,
 bool AllowObjCWritebackConversion) {
-  return ::TryImplicitConversion(*this, From, ToType, SuppressUserConversions,
+  return ::TryImplicitConversion(*this, From, ToType, UserConversions,
  AllowExplicit, InOverloadResolution, CStyle,
  AllowObjCWritebackConversion,
  /*AllowObjCConversionOnExplicit=*/false);
@@ -1513,7 +1513,7 @@
 CheckObjCBridgeRelatedConversions(From->getBeginLoc(), ToType,
   From->getType(), From);
   ICS = ::TryImplicitConversion(*this, From, ToType,
-/*SuppressUserConversions=*/false,
+UserDefinedConversions::Allow,
 AllowExplicit ? AllowedExplicit::All
   : AllowedExplicit::None,
 /*InOverloadResolution=*/false,
@@ -3334,17 +3334,19 @@
 if (Usable) {
   // If the first argument is (a reference to) the target type,
   // suppress conversions.
-  bool SuppressUserConversions = isFirstArgumentCompatibleWithType(
-  S.Context, Info.Constructor, ToType);
+  Sema::UserDefinedConversions UserConversions = 
+ isFirstArgumentCompatibleWithType(S.Context, Info.Constructor, ToType)
+? Sema::UserDefinedConversions::Suppress
+: Sema::UserDefinedConversions::Allow;
   if (Info.Construc

[PATCH] D74238: [clang] [clang] Improve diagnostic note for implicit conversion sequences that would work if more than one implicit user-defined conversion were allowed.

2020-02-07 Thread Logan Smith via Phabricator via cfe-commits
logan-5 created this revision.
logan-5 added a reviewer: rsmith.
logan-5 added a project: clang.
Herald added a subscriber: cfe-commits.

The current text of the 'note' diagnostic for bad conversions is confusing in 
the presence of user-defined conversion operations: https://godbolt.org/z/zrgeHH
For the first error, the conversion function is simply pointed at in a note 
without any further explanation. For the second error, the final note claims 
there is no conversion from X2 to Y2, even though it is plainly visible in the 
source code. The reason for the error is that only one implicit user-defined 
conversion may be applied in these circumstances, but the error message is 
misleading to those who may not know this rule.

This is determined by searching for these illegal conversions in 
CompleteNonViableCandidate after overload resolution fails.

Depends on D74234 . Also depends on D74235 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74238

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/drs/dr0xx.cpp
  clang/test/SemaCXX/address-space-ctor.cpp
  clang/test/SemaCXX/constructor-initializer.cpp
  clang/test/SemaCXX/user-defined-conversions.cpp

Index: clang/test/SemaCXX/user-defined-conversions.cpp
===
--- clang/test/SemaCXX/user-defined-conversions.cpp
+++ clang/test/SemaCXX/user-defined-conversions.cpp
@@ -97,3 +97,51 @@
 a = b; // expected-error{{calling a private constructor of class 'rdar10202900::A'}}
   }
 }
+
+namespace more_than_one {
+
+namespace std_example {
+struct X {
+  operator int();
+};
+
+struct Y {
+  operator X(); // expected-note{{converting from 'more_than_one::std_example::Y' to 'more_than_one::std_example::X' would apply more than one implicit user-defined conversion}}
+};
+
+Y a;
+int b = a; // expected-error{{no viable conversion}}
+int c = X(a);
+}
+
+namespace ctors {
+struct X {};
+struct Y {
+  Y(X);
+};
+struct Z { // expected-note 2{{candidate constructor (the implicit}}
+  Z(Y);   // expected-note{{converting from 'more_than_one::ctors::X' to 'more_than_one::ctors::Y' would apply more than one implicit user-defined conversion}}
+};
+
+X x;
+Z z = x; // expected-error{{no viable conversion}}
+Z z2 = Y(x);
+}
+
+namespace ops {
+
+struct Y;
+struct X {
+  operator Y() const; // expected-note {{converting from 'more_than_one::ops::X' to 'more_than_one::ops::Y' would apply more than one implicit user-defined conversion}}
+};
+struct Y {};
+struct Z { // expected-note 2{{candidate constructor (the implicit}}
+  Z(Y);   // expected-note{{converting from 'more_than_one::ops::X' to 'more_than_one::ops::Y' would apply more than one implicit user-defined conversion}}
+};
+
+X x;
+Z z = x; // expected-error{{no viable conversion}}
+Z z2 = static_cast(x);
+}
+} // namespace more_than_one
+
Index: clang/test/SemaCXX/constructor-initializer.cpp
===
--- clang/test/SemaCXX/constructor-initializer.cpp
+++ clang/test/SemaCXX/constructor-initializer.cpp
@@ -306,7 +306,7 @@
 namespace PR10758 {
 struct A;
 struct B {
-  B (A const &); // expected-note 2 {{candidate constructor not viable: no known conversion from 'const PR10758::B' to 'const PR10758::A &' for 1st argument}}
+  B (A const &); // expected-note 2 {{candidate constructor not viable: converting from 'const PR10758::B' to 'const PR10758::A &' would apply more than one implicit user-defined conversion}}
   B (B &); // expected-note 2 {{candidate constructor not viable: 1st argument ('const PR10758::B') would lose const qualifier}}
 };
 struct A {
Index: clang/test/SemaCXX/address-space-ctor.cpp
===
--- clang/test/SemaCXX/address-space-ctor.cpp
+++ clang/test/SemaCXX/address-space-ctor.cpp
@@ -6,8 +6,8 @@
   int i;
 };
 
-//expected-note@-5{{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const MyType &' for 1st argument}}
-//expected-note@-6{{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'MyType &&' for 1st argument}}
+//expected-note@-5{{candidate constructor (the implicit copy constructor) not viable: converting from 'int' to 'const MyType &' would apply more than one implicit user-defined conversion}}
+//expected-note@-6{{candidate constructor (the implicit move constructor) not viable: converting from 'int' to 'MyType &&' would apply more than one implicit user-defined conversion}}
 //expected-note@-6{{candidate constructor ignored: cannot be used to construct an object in address space '__attribute__((address_space(10)))'}}
 //expected-note@-8{{candidate constructor ignored: cannot be used to construct an obje

[PATCH] D74009: [clang] Improve diagnostic note for implicit conversions that are disallowed because they involve more than one user-defined conversion.

2020-02-07 Thread Logan Smith via Phabricator via cfe-commits
logan-5 abandoned this revision.
logan-5 added a comment.

I split this patch into multiple, and reworked the implementation. The new 
stuff lives here and here and here: D74238  
D74234  D74235 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74009



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


[PATCH] D74235: [clang] [NFC] Rename Sema::DiagnoseMultipleUserConversion to DiagnoseAmbiguousUserConversion

2020-02-07 Thread Logan Smith via Phabricator via cfe-commits
logan-5 created this revision.
logan-5 added a reviewer: rsmith.
logan-5 added a project: clang.
Herald added a subscriber: cfe-commits.
logan-5 added a child revision: D74238: [clang] [clang] Improve diagnostic note 
for implicit conversion sequences that would work if more than one implicit 
user-defined conversion were allowed..

This is to avoid confusion in an upcoming patch that uses the term "multiple 
user-defined conversions" to refer to (invalid) implicit conversion sequences 
that involve applying more than one user conversion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74235

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaOverload.cpp


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3606,7 +3606,7 @@
 }
 
 bool
-Sema::DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType) {
+Sema::DiagnoseAmbiguousUserDefinedConversion(Expr *From, QualType ToType) {
   ImplicitConversionSequence ICS;
   OverloadCandidateSet CandidateSet(From->getExprLoc(),
 OverloadCandidateSet::CSK_Normal);
@@ -5465,7 +5465,7 @@
   if (!ICS.isBad())
 return PerformImplicitConversion(From, Context.BoolTy, ICS, AA_Converting);
 
-  if (!DiagnoseMultipleUserDefinedConversion(From, Context.BoolTy))
+  if (!DiagnoseAmbiguousUserDefinedConversion(From, Context.BoolTy))
 return Diag(From->getBeginLoc(), diag::err_typecheck_bool_condition)
<< From->getType() << From->getSourceRange();
   return ExprError();
@@ -5579,7 +5579,7 @@
 break;
   case ImplicitConversionSequence::AmbiguousConversion:
   case ImplicitConversionSequence::BadConversion:
-if (!S.DiagnoseMultipleUserDefinedConversion(From, T))
+if (!S.DiagnoseAmbiguousUserDefinedConversion(From, T))
   return S.Diag(From->getBeginLoc(),
 diag::err_typecheck_converted_constant_expression)
  << From->getType() << From->getSourceRange() << T;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3011,7 +3011,7 @@
  bool CStyle, bool &ObjCLifetimeConversion);
   bool IsFunctionConversion(QualType FromType, QualType ToType,
 QualType &ResultTy);
-  bool DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType);
+  bool DiagnoseAmbiguousUserDefinedConversion(Expr *From, QualType ToType);
   bool isSameOrCompatibleFunctionType(CanQualType Param, CanQualType Arg);
 
   ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3606,7 +3606,7 @@
 }
 
 bool
-Sema::DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType) {
+Sema::DiagnoseAmbiguousUserDefinedConversion(Expr *From, QualType ToType) {
   ImplicitConversionSequence ICS;
   OverloadCandidateSet CandidateSet(From->getExprLoc(),
 OverloadCandidateSet::CSK_Normal);
@@ -5465,7 +5465,7 @@
   if (!ICS.isBad())
 return PerformImplicitConversion(From, Context.BoolTy, ICS, AA_Converting);
 
-  if (!DiagnoseMultipleUserDefinedConversion(From, Context.BoolTy))
+  if (!DiagnoseAmbiguousUserDefinedConversion(From, Context.BoolTy))
 return Diag(From->getBeginLoc(), diag::err_typecheck_bool_condition)
<< From->getType() << From->getSourceRange();
   return ExprError();
@@ -5579,7 +5579,7 @@
 break;
   case ImplicitConversionSequence::AmbiguousConversion:
   case ImplicitConversionSequence::BadConversion:
-if (!S.DiagnoseMultipleUserDefinedConversion(From, T))
+if (!S.DiagnoseAmbiguousUserDefinedConversion(From, T))
   return S.Diag(From->getBeginLoc(),
 diag::err_typecheck_converted_constant_expression)
  << From->getType() << From->getSourceRange() << T;
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3011,7 +3011,7 @@
  bool CStyle, bool &ObjCLifetimeConversion);
   bool IsFunctionConversion(QualType FromType, QualType ToType,
 QualType &ResultTy);
-  bool DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType);
+  bool DiagnoseAmbiguousUserDefinedConversion(Expr *From, QualType ToType);
   bool isSameOrCompatibleFunctionType(CanQualType Param, CanQualType Arg);
 
   ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lis

[clang] 2926917 - [clang] Fix linkage of nested lambdas.

2020-02-07 Thread Michael Liao via cfe-commits

Author: Michael Liao
Date: 2020-02-07T13:24:21-05:00
New Revision: 2926917f430d705f084813b63a40fafc61872524

URL: 
https://github.com/llvm/llvm-project/commit/2926917f430d705f084813b63a40fafc61872524
DIFF: 
https://github.com/llvm/llvm-project/commit/2926917f430d705f084813b63a40fafc61872524.diff

LOG: [clang] Fix linkage of nested lambdas.

patch from Philippe Daouadi 

This is an attempt to fix
[PR#44368](https://bugs.llvm.org/show_bug.cgi?id=44368)

This effectively reverts [D1783](https://reviews.llvm.org/D1783). It
doesn't break the current tests and fixes the test that this commit
adds.

We now decide of a lambda linkage only depending on the visibility of
its parent context.

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

Added: 


Modified: 
clang/lib/AST/Decl.cpp
clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp

Removed: 




diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 0d30f64b992e..216137bf74f9 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1318,19 +1318,6 @@ LinkageInfo LinkageComputer::getLVForLocalDecl(const 
NamedDecl *D,
  LV.isVisibilityExplicit());
 }
 
-static inline const CXXRecordDecl*
-getOutermostEnclosingLambda(const CXXRecordDecl *Record) {
-  const CXXRecordDecl *Ret = Record;
-  while (Record && Record->isLambda()) {
-Ret = Record;
-if (!Record->getParent()) break;
-// Get the Containing Class of this Lambda Class
-Record = dyn_cast_or_null(
-  Record->getParent()->getParent());
-  }
-  return Ret;
-}
-
 LinkageInfo LinkageComputer::computeLVForDecl(const NamedDecl *D,
   LVComputationKind computation,
   bool IgnoreVarTypeLinkage) {
@@ -1396,25 +1383,9 @@ LinkageInfo LinkageComputer::computeLVForDecl(const 
NamedDecl *D,
   return getInternalLinkageFor(D);
 }
 
-// This lambda has its linkage/visibility determined:
-//  - either by the outermost lambda if that lambda has no mangling
-//number.
-//  - or by the parent of the outer most lambda
-// This prevents infinite recursion in settings such as nested lambdas
-// used in NSDMI's, for e.g.
-//  struct L {
-//int t{};
-//int t2 = ([](int a) { return [](int b) { return b; };})(t)(t);
-//  };
-const CXXRecordDecl *OuterMostLambda =
-getOutermostEnclosingLambda(Record);
-if (OuterMostLambda->hasKnownLambdaInternalLinkage() ||
-!OuterMostLambda->getLambdaManglingNumber())
-  return getInternalLinkageFor(D);
-
 return getLVForClosure(
-  OuterMostLambda->getDeclContext()->getRedeclContext(),
-  OuterMostLambda->getLambdaContextDecl(), computation);
+  Record->getDeclContext()->getRedeclContext(),
+  Record->getLambdaContextDecl(), computation);
   }
 
   break;

diff  --git a/clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp 
b/clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
index 9a449874cb85..6b45645c9e2d 100644
--- a/clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
+++ b/clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - 
%s -fexceptions -std=c++11 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - 
%s -fexceptions -std=c++14 | FileCheck --check-prefixes=CHECK,CXX14 %s
 
 // CHECK-LABEL: define void @_ZN19non_inline_function3fooEv()
 // CHECK-LABEL: define internal void 
@"_ZZN19non_inline_function3fooEvENK3$_0clEi"(%class.anon
@@ -51,3 +52,18 @@ inline int foo() {
 }
 int use = foo();
 }
+
+#if __cplusplus >= 201402L
+// CXX14-LABEL: define internal void 
@"_ZZZN32lambda_capture_in_generic_lambda3fooIiEEDavENKUlT_E_clIZNS_L1fEvE3$_1EEDaS1_ENKUlvE_clEv"
+namespace lambda_capture_in_generic_lambda {
+template  auto foo() {
+  return [](auto func) {
+[func] { func(); }();
+  };
+}
+static void f() {
+  foo()([] { });
+}
+void f1() { f(); }
+}
+#endif



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


[PATCH] D73701: [clang] fix linkage of nested lambda

2020-02-07 Thread Michael Liao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2926917f430d: [clang] Fix linkage of nested lambdas. 
(authored by hliao).

Changed prior to commit:
  https://reviews.llvm.org/D73701?vs=241713&id=243224#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73701

Files:
  clang/lib/AST/Decl.cpp
  clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp


Index: clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
===
--- clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
+++ clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - 
%s -fexceptions -std=c++11 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - 
%s -fexceptions -std=c++14 | FileCheck --check-prefixes=CHECK,CXX14 %s
 
 // CHECK-LABEL: define void @_ZN19non_inline_function3fooEv()
 // CHECK-LABEL: define internal void 
@"_ZZN19non_inline_function3fooEvENK3$_0clEi"(%class.anon
@@ -51,3 +52,18 @@
 }
 int use = foo();
 }
+
+#if __cplusplus >= 201402L
+// CXX14-LABEL: define internal void 
@"_ZZZN32lambda_capture_in_generic_lambda3fooIiEEDavENKUlT_E_clIZNS_L1fEvE3$_1EEDaS1_ENKUlvE_clEv"
+namespace lambda_capture_in_generic_lambda {
+template  auto foo() {
+  return [](auto func) {
+[func] { func(); }();
+  };
+}
+static void f() {
+  foo()([] { });
+}
+void f1() { f(); }
+}
+#endif
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1318,19 +1318,6 @@
  LV.isVisibilityExplicit());
 }
 
-static inline const CXXRecordDecl*
-getOutermostEnclosingLambda(const CXXRecordDecl *Record) {
-  const CXXRecordDecl *Ret = Record;
-  while (Record && Record->isLambda()) {
-Ret = Record;
-if (!Record->getParent()) break;
-// Get the Containing Class of this Lambda Class
-Record = dyn_cast_or_null(
-  Record->getParent()->getParent());
-  }
-  return Ret;
-}
-
 LinkageInfo LinkageComputer::computeLVForDecl(const NamedDecl *D,
   LVComputationKind computation,
   bool IgnoreVarTypeLinkage) {
@@ -1396,25 +1383,9 @@
   return getInternalLinkageFor(D);
 }
 
-// This lambda has its linkage/visibility determined:
-//  - either by the outermost lambda if that lambda has no mangling
-//number.
-//  - or by the parent of the outer most lambda
-// This prevents infinite recursion in settings such as nested lambdas
-// used in NSDMI's, for e.g.
-//  struct L {
-//int t{};
-//int t2 = ([](int a) { return [](int b) { return b; };})(t)(t);
-//  };
-const CXXRecordDecl *OuterMostLambda =
-getOutermostEnclosingLambda(Record);
-if (OuterMostLambda->hasKnownLambdaInternalLinkage() ||
-!OuterMostLambda->getLambdaManglingNumber())
-  return getInternalLinkageFor(D);
-
 return getLVForClosure(
-  OuterMostLambda->getDeclContext()->getRedeclContext(),
-  OuterMostLambda->getLambdaContextDecl(), computation);
+  Record->getDeclContext()->getRedeclContext(),
+  Record->getLambdaContextDecl(), computation);
   }
 
   break;


Index: clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
===
--- clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
+++ clang/test/CodeGenCXX/lambda-expressions-nested-linkage.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - %s -fexceptions -std=c++11 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - %s -fexceptions -std=c++14 | FileCheck --check-prefixes=CHECK,CXX14 %s
 
 // CHECK-LABEL: define void @_ZN19non_inline_function3fooEv()
 // CHECK-LABEL: define internal void @"_ZZN19non_inline_function3fooEvENK3$_0clEi"(%class.anon
@@ -51,3 +52,18 @@
 }
 int use = foo();
 }
+
+#if __cplusplus >= 201402L
+// CXX14-LABEL: define internal void @"_ZZZN32lambda_capture_in_generic_lambda3fooIiEEDavENKUlT_E_clIZNS_L1fEvE3$_1EEDaS1_ENKUlvE_clEv"
+namespace lambda_capture_in_generic_lambda {
+template  auto foo() {
+  return [](auto func) {
+[func] { func(); }();
+  };
+}
+static void f() {
+  foo()([] { });
+}
+void f1() { f(); }
+}
+#endif
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1318,19 +1318,6 @@
  LV.isVisibilityExplicit());
 }
 
-static inline const CXXReco

[PATCH] D73701: [clang] fix linkage of nested lambda

2020-02-07 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

In D73701#1864136 , @blastrock wrote:

> Thanks for the reviews! I do not have commit access, so please merge this 
> whenever you can.


done in 2926917f430 
. thanks 
for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73701



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


[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-02-07 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2077
+getDataLayout().getABITypeAlignment(getTypes().ConvertType(RetTy)))
+  SRETAttrs.addAlignmentAttr(Align);
 ArgAttrs[IRFunctionArgs.getSRetArgNo()] =

scanon wrote:
> rjmccall wrote:
> > Why only when under-aligned?  Just to avoid churning tests?  I think we 
> > should apply this unconditionally.
> On mainstream architectures today, there's rarely a benefit to knowing about 
> higher alignment (e.g. MOVUPS is just as fast as MOVAPS if the address is 
> actually aligned), so we won't see significant perf wins from preserving 
> over-alignment in most cases, but it also doesn't cost us anything AFAICT and 
> could deliver wins in some specific cases (e.g. AVX on SNB and IVB, where I 
> think we split underaligned 256b stores into two 128b chunks).
> 
> So, yeah, I think we ought to simply unconditionally add the alignment to the 
> sret.
@rjmccall, are you seeing a reason to add the attribute when the implicit one 
is correct (neither over-aligned nor under-aligned)?  If so, it seems to me 
like the added noise would make the IR harder to read.


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

https://reviews.llvm.org/D74183



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


[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2077
+getDataLayout().getABITypeAlignment(getTypes().ConvertType(RetTy)))
+  SRETAttrs.addAlignmentAttr(Align);
 ArgAttrs[IRFunctionArgs.getSRetArgNo()] =

dexonsmith wrote:
> scanon wrote:
> > rjmccall wrote:
> > > Why only when under-aligned?  Just to avoid churning tests?  I think we 
> > > should apply this unconditionally.
> > On mainstream architectures today, there's rarely a benefit to knowing 
> > about higher alignment (e.g. MOVUPS is just as fast as MOVAPS if the 
> > address is actually aligned), so we won't see significant perf wins from 
> > preserving over-alignment in most cases, but it also doesn't cost us 
> > anything AFAICT and could deliver wins in some specific cases (e.g. AVX on 
> > SNB and IVB, where I think we split underaligned 256b stores into two 128b 
> > chunks).
> > 
> > So, yeah, I think we ought to simply unconditionally add the alignment to 
> > the sret.
> @rjmccall, are you seeing a reason to add the attribute when the implicit one 
> is correct (neither over-aligned nor under-aligned)?  If so, it seems to me 
> like the added noise would make the IR harder to read.
Well, first, I think we're going to end up needing an alignment there in all 
cases eventually because of opaque pointer types.  But I also think it's just 
cleaner and more testable to add the attribute in all cases instead of leaving 
it off when the IR type happens to have the right alignment, which can be very 
sensitive to the target.


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

https://reviews.llvm.org/D74183



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


[clang] 4a1a069 - Support -fstack-clash-protection for x86

2020-02-07 Thread via cfe-commits

Author: serge_sans_paille
Date: 2020-02-07T19:54:39+01:00
New Revision: 4a1a0690ad6813a4c8cdb8dc20ea6337aa1f61e0

URL: 
https://github.com/llvm/llvm-project/commit/4a1a0690ad6813a4c8cdb8dc20ea6337aa1f61e0
DIFF: 
https://github.com/llvm/llvm-project/commit/4a1a0690ad6813a4c8cdb8dc20ea6337aa1f61e0.diff

LOG: Support -fstack-clash-protection for x86

Implement protection against the stack clash attack [0] through inline stack
probing.

Probe stack allocation every PAGE_SIZE during frame lowering or dynamic
allocation to make sure the page guard, if any, is touched when touching the
stack, in a similar manner to GCC[1].

This extends the existing `probe-stack' mechanism with a special value 
`inline-asm'.
Technically the former uses function call before stack allocation while this
patch provides inlined stack probes and chunk allocation.

Only implemented for x86.

[0] https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt
[1] https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00556.html

This a recommit of 39f50da2a357a8f685b3540246c5d762734e035f with correct option
flags set.

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

Added: 
clang/test/CodeGen/stack-clash-protection.c
clang/test/Driver/stack-clash-protection.c
llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
llvm/test/CodeGen/X86/stack-clash-large.ll
llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
llvm/test/CodeGen/X86/stack-clash-medium.ll
llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
llvm/test/CodeGen/X86/stack-clash-small.ll
llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Modified: 
clang/docs/ClangCommandLineReference.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/include/clang/Basic/TargetInfo.h
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/X86.h
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/lib/Target/X86/X86CallFrameOptimization.cpp
llvm/lib/Target/X86/X86FrameLowering.cpp
llvm/lib/Target/X86/X86FrameLowering.h
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.h
llvm/lib/Target/X86/X86InstrCompiler.td
llvm/lib/Target/X86/X86InstrInfo.td

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 24c8ee2bc9ef..609e7fa66c00 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -1917,6 +1917,10 @@ Use a strong heuristic to apply stack protectors to 
functions
 
 Emit section containing metadata on function stack sizes
 
+.. option:: -fstack-clash-protection, -fno-stack-clash-protection
+
+Instrument stack allocation to prevent stack clash attacks (x86, non-Windows 
only).
+
 .. option:: -fstandalone-debug, -fno-limit-debug-info, -fno-standalone-debug
 
 Emit full debug info for all types used by the program

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cb8d73932bf6..856effd19718 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -61,6 +61,10 @@ New Compiler Flags
 --
 
 
+- -fstack-clash-protection will provide a protection against the stack clash
+  attack for x86 architecture through automatic probing of each page of
+  allocated stack.
+
 Deprecated Compiler Flags
 -
 

diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 21e391912584..48c0df49e32d 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -150,6 +150,7 @@ CODEGENOPT(NoWarn, 1, 0) ///< Set when 
-Wa,--no-warn is enabled.
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.
 CODEGENOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain
  ///< inline line tables.
+CODEGENOPT(StackClashProtector, 1, 0) ///< Set when -fstack-clash-protection 
is enabled.
 CODEGENOPT(NoImplicitFloat   , 1, 0) ///< Set when -mno-implicit-float is 
enabled.
 CODEGENOPT(NoInfsFPMath  , 1, 0) ///< Assume FP arguments, results not 
+-Inf.
 CODEGENOPT(NoSignedZeros , 1, 0) ///< Allow ignoring the signedness of FP 
zero

diff  --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td 
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 20b49605eb6f..95cb81f09922 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clan

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2077
+getDataLayout().getABITypeAlignment(getTypes().ConvertType(RetTy)))
+  SRETAttrs.addAlignmentAttr(Align);
 ArgAttrs[IRFunctionArgs.getSRetArgNo()] =

rjmccall wrote:
> dexonsmith wrote:
> > scanon wrote:
> > > rjmccall wrote:
> > > > Why only when under-aligned?  Just to avoid churning tests?  I think we 
> > > > should apply this unconditionally.
> > > On mainstream architectures today, there's rarely a benefit to knowing 
> > > about higher alignment (e.g. MOVUPS is just as fast as MOVAPS if the 
> > > address is actually aligned), so we won't see significant perf wins from 
> > > preserving over-alignment in most cases, but it also doesn't cost us 
> > > anything AFAICT and could deliver wins in some specific cases (e.g. AVX 
> > > on SNB and IVB, where I think we split underaligned 256b stores into two 
> > > 128b chunks).
> > > 
> > > So, yeah, I think we ought to simply unconditionally add the alignment to 
> > > the sret.
> > @rjmccall, are you seeing a reason to add the attribute when the implicit 
> > one is correct (neither over-aligned nor under-aligned)?  If so, it seems 
> > to me like the added noise would make the IR harder to read.
> Well, first, I think we're going to end up needing an alignment there in all 
> cases eventually because of opaque pointer types.  But I also think it's just 
> cleaner and more testable to add the attribute in all cases instead of 
> leaving it off when the IR type happens to have the right alignment, which 
> can be very sensitive to the target.
In general, I think frontends should *never* be leaving it up to LLVM to infer 
alignment based on IR types, and this is part-and-parcel with that.


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

https://reviews.llvm.org/D74183



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


[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-02-07 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2077
+getDataLayout().getABITypeAlignment(getTypes().ConvertType(RetTy)))
+  SRETAttrs.addAlignmentAttr(Align);
 ArgAttrs[IRFunctionArgs.getSRetArgNo()] =

rjmccall wrote:
> rjmccall wrote:
> > dexonsmith wrote:
> > > scanon wrote:
> > > > rjmccall wrote:
> > > > > Why only when under-aligned?  Just to avoid churning tests?  I think 
> > > > > we should apply this unconditionally.
> > > > On mainstream architectures today, there's rarely a benefit to knowing 
> > > > about higher alignment (e.g. MOVUPS is just as fast as MOVAPS if the 
> > > > address is actually aligned), so we won't see significant perf wins 
> > > > from preserving over-alignment in most cases, but it also doesn't cost 
> > > > us anything AFAICT and could deliver wins in some specific cases (e.g. 
> > > > AVX on SNB and IVB, where I think we split underaligned 256b stores 
> > > > into two 128b chunks).
> > > > 
> > > > So, yeah, I think we ought to simply unconditionally add the alignment 
> > > > to the sret.
> > > @rjmccall, are you seeing a reason to add the attribute when the implicit 
> > > one is correct (neither over-aligned nor under-aligned)?  If so, it seems 
> > > to me like the added noise would make the IR harder to read.
> > Well, first, I think we're going to end up needing an alignment there in 
> > all cases eventually because of opaque pointer types.  But I also think 
> > it's just cleaner and more testable to add the attribute in all cases 
> > instead of leaving it off when the IR type happens to have the right 
> > alignment, which can be very sensitive to the target.
> In general, I think frontends should *never* be leaving it up to LLVM to 
> infer alignment based on IR types, and this is part-and-parcel with that.
> I think we're going to end up needing an alignment there in all cases 
> eventually because of opaque pointer types.

That's a great point.  In that case I don't have a strong opinion.


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

https://reviews.llvm.org/D74183



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


[PATCH] D68720: Support -fstack-clash-protection for x86

2020-02-07 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

This breaks check-clang on mac and win: http://45.33.8.238/mac/7485/step_7.txt 
http://45.33.8.238/win/7753/step_7.txt

Please revert and then investigate asynchronously, unless the fix is obvious.




Comment at: clang/test/CodeGen/stack-clash-protection.c:2
+// check interaction between -fstack-clash-protection and dynamic allocation 
schemes
+// RUN: %clang -target x86_64 -O0 -o %t.out %s -fstack-clash-protection && 
%t.out
+

Tests should compile binaries and then run them if at all possible. This is 
impossible in cross-build scenarios and in general we try hard to have unit 
tests, not integration tests. Check that this produces the IR you expect, and 
have an llvm-level test that the IR produces the assembly you expect, and if 
you must, an lld level test that checks that the generated elf file looks like 
you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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


[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

2020-02-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington updated this revision to Diff 243243.
erik.pilkington marked 2 inline comments as done.
erik.pilkington added a comment.

Disable the optimization for non-trivially destructible types.


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

https://reviews.llvm.org/D74094

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h
  clang/test/CodeGen/lifetime-call-temp.c
  clang/test/CodeGenCXX/stack-reuse-miscompile.cpp

Index: clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
===
--- clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ clang/test/CodeGenCXX/stack-reuse-miscompile.cpp
@@ -26,6 +26,8 @@
 // CHECK: [[T2:%.*]] = alloca %class.T, align 4
 // CHECK: [[T3:%.*]] = alloca %class.T, align 4
 //
+// CHECK: [[AGG:%.*]] = alloca %class.S, align 4
+//
 // FIXME: We could defer starting the lifetime of the return object of concat
 // until the call.
 // CHECK: [[T1i8:%.*]] = bitcast %class.T* [[T1]] to i8*
@@ -37,8 +39,15 @@
 //
 // CHECK: [[T3i8:%.*]] = bitcast %class.T* [[T3]] to i8*
 // CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T3i8]])
+//
+// CHECK: [[AGGi8:%.*]] = bitcast %class.S* [[AGG]] to i8*
+// CHECK: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[AGGi8]])
+//
 // CHECK: [[T5:%.*]] = call %class.T* @_ZN1TC1E1S(%class.T* [[T3]], [2 x i32] %{{.*}})
 //
+// CHECK: [[AGGi8:%.*]] = bitcast %class.S* {{.*}} to i8*
+// CHECK: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[AGGi8]])
+//
 // CHECK: call void @_ZNK1T6concatERKS_(%class.T* sret [[T1]], %class.T* [[T2]], %class.T* dereferenceable(16) [[T3]])
 // CHECK: [[T6:%.*]] = call i8* @_ZNK1T3strEv(%class.T* [[T1]])
 //
Index: clang/test/CodeGen/lifetime-call-temp.c
===
--- /dev/null
+++ clang/test/CodeGen/lifetime-call-temp.c
@@ -0,0 +1,83 @@
+// RUN: %clang -cc1  -triple x86_64-apple-macos -O1 -disable-llvm-passes %s -S -emit-llvm -o - | FileCheck %s --implicit-check-not=llvm.lifetime
+// RUN: %clang -cc1 -xc++ -std=c++17 -triple x86_64-apple-macos -O1 -disable-llvm-passes %s -S -emit-llvm -o - | FileCheck %s --implicit-check-not=llvm.lifetime --check-prefix=CHECK --check-prefix=CXX
+// RUN: %clang -cc1 -xobjective-c-triple x86_64-apple-macos -O1 -disable-llvm-passes %s -S -emit-llvm -o - | FileCheck %s --implicit-check-not=llvm.lifetime --check-prefix=CHECK --check-prefix=OBJC
+
+typedef struct { int x[100]; } aggregate;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+void takes_aggregate(aggregate);
+aggregate gives_aggregate();
+
+// CHECK-LABEL: define void @t1
+void t1() {
+  takes_aggregate(gives_aggregate());
+
+  // CHECK: [[AGGTMP:%.*]] = alloca %struct.aggregate, align 8
+  // CHECK: [[CAST:%.*]] = bitcast %struct.aggregate* [[AGGTMP]] to i8*
+  // CHECK: call void @llvm.lifetime.start.p0i8(i64 400, i8* [[CAST]])
+  // CHECK: call void{{.*}} @gives_aggregate(%struct.aggregate* sret [[AGGTMP]])
+  // CHECK: call void @takes_aggregate(%struct.aggregate* byval(%struct.aggregate) align 8 [[AGGTMP]])
+  // CHECK: [[CAST:%.*]] = bitcast %struct.aggregate* [[AGGTMP]] to i8*
+  // CHECK: call void @llvm.lifetime.end.p0i8(i64 400, i8* [[CAST]])
+}
+
+// CHECK: declare {{.*}}llvm.lifetime.start
+// CHECK: declare {{.*}}llvm.lifetime.end
+
+#ifdef __cplusplus
+// CXX: define void @t2
+void t2() {
+  struct S {
+S(aggregate) {}
+  };
+  S{gives_aggregate()};
+
+  // CXX: [[AGG:%.*]] = alloca %struct.aggregate
+  // CXX: call void @llvm.lifetime.start.p0i8(i64 400, i8*
+  // CXX: call void @gives_aggregate(%struct.aggregate* sret [[AGG]])
+  // CXX: call void @_ZZ2t2EN1SC1E9aggregate(%struct.S* {{.*}}, %struct.aggregate* byval(%struct.aggregate) align 8 [[AGG]])
+  // CXX: call void @llvm.lifetime.end.p0i8(i64 400, i8*
+}
+
+struct Dtor {
+  ~Dtor();
+};
+
+void takes_dtor(Dtor);
+Dtor gives_dtor();
+
+// CXX: define void @t3
+void t3() {
+  takes_dtor(gives_dtor());
+
+  // CXX-NOT @llvm.lifetime
+  // CXX: ret void
+}
+
+#endif
+
+#ifdef __OBJC__
+
+@interface X
+-m:(aggregate)x;
+@end
+
+// OBJC: define void @t4
+void t4(X *x) {
+  [x m: gives_aggregate()];
+
+  // OBJC: [[AGG:%.*]] = alloca %struct.aggregate
+  // OBJC: call void @llvm.lifetime.start.p0i8(i64 400, i8*
+  // OBJC: call void{{.*}} @gives_aggregate(%struct.aggregate* sret [[AGGTMP]])
+  // OBJC: call {{.*}}@objc_msgSend
+  // OBJC: call void @llvm.lifetime.end.p0i8(i64 400, i8*
+}
+
+#endif
+
+#ifdef __cplusplus
+}
+#endif
Index: clang/lib/CodeGen/CGCall.h
===
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -283,6 +283,11 @@
 llvm::Instruction *IsActiveIP;
   };
 
+  struct EndLifetimeInfo {
+llvm::Value *Addr;
+llvm::Value *Size;
+  };
+
   void add(RValue rvalue, QualType type) { push_back(CallArg(rvalue, type)); }
 
   void addUncopiedAggregate(LValue LV, QualType type) {
@@ -299,6 +304,

[PATCH] D73967: Implement _ExtInt as an extended int type specifier.

2020-02-07 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 243248.
erichkeane added a comment.

Remove conversions between _ExtInt until WG14 decides what should be done with 
them.


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

https://reviews.llvm.org/D73967

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenTBAA.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/CodeGenTypes.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGen/ext-int-sanitizer.cpp
  clang/test/CodeGen/ext-int.cpp
  clang/test/CodeGenCXX/debug-info-template-partial-specialization.cpp
  clang/test/CodeGenOpenCL/ext-int-shift.cl
  clang/test/SemaCXX/ext-int.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1804,6 +1804,8 @@
 DEFAULT_TYPELOC_IMPL(SubstTemplateTypeParm, Type)
 DEFAULT_TYPELOC_IMPL(SubstTemplateTypeParmPack, Type)
 DEFAULT_TYPELOC_IMPL(Auto, Type)
+DEFAULT_TYPELOC_IMPL(ExtInt, Type)
+DEFAULT_TYPELOC_IMPL(DependentExtInt, Type)
 
 bool CursorVisitor::VisitCXXRecordDecl(CXXRecordDecl *D) {
   // Visit the nested-name-specifier, if present.
Index: clang/test/SemaCXX/ext-int.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/ext-int.cpp
@@ -0,0 +1,266 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct HasExtInt {
+  _ExtInt(Bounds) b;
+  unsigned _ExtInt(Bounds) b2;
+};
+
+// Delcaring variables:
+_ExtInt(33) Declarations(_ExtInt(48) &Param) { // Useable in params and returns.
+  short _ExtInt(43) a; // expected-error {{'short _ExtInt' is invalid}}
+  _ExtInt(43) long b;  // expected-error {{'long _ExtInt' is invalid}}
+
+  // These should all be fine:
+  const _ExtInt(5) c = 3;
+  const unsigned _ExtInt(5) d; // expected-error {{default initialization of an object of const type 'const unsigned _ExtInt(5)'}}
+  unsigned _ExtInt(5) e = 5;
+  _ExtInt(5) unsigned f;
+
+  _ExtInt(-3) g; // expected-error{{signed _ExtInt must have a size of at least 2}}
+  _ExtInt(0) h; // expected-error{{signed _ExtInt must have a size of at least 2}}
+  _ExtInt(1) i; // expected-error{{signed _ExtInt must have a size of at least 2}}
+  _ExtInt(2) j;;
+  unsigned _ExtInt(0) k;// expected-error{{unsigned _ExtInt must have a size of at least 1}}
+  unsigned _ExtInt(1) l;
+  signed _ExtInt(1) m; // expected-error{{signed _ExtInt must have a size of at least 2}}
+
+  constexpr _ExtInt(6) n = 33; // expected-warning{{implicit conversion from 'int' to 'const _ExtInt(6)' changes value from 33 to -31}}
+  constexpr _ExtInt(7) o = 33;
+
+  // Check LLVM imposed max size.
+  _ExtInt(0xFF) p; // expected-error {{signed _ExtInt of sizes greater than 16777215 not supported}}
+  unsigned _ExtInt(0xFF) q; // expected-error {{unsigned _ExtInt of sizes greater than 16777215 not supported}}
+
+// Ensure template params are instantiated correctly.
+  // expected-error@5{{signed _ExtInt must have a size of at least 2}}
+  // expected-error@6{{unsigned _ExtInt must have a size of at least 1}}
+  // expected-note@+1{{in instantiation of template class }}
+  HasExtInt<-1> r;
+  // expected-error@5{{signed _ExtInt must have a size of at least 2}}
+  // expected-error@6{{unsigned _ExtInt must have a size of at least 1}}
+  // expected-note@+1{{in instanti

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

2020-02-07 Thread Francis Visoiu Mistrih via Phabricator via cfe-commits
thegameg added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3687
 
-  args.add(EmitAnyExprToTemp(E), type);
 }

Is there any other use of `EmitAnyExprToTemp` that can benefit from this?


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

https://reviews.llvm.org/D74094



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


[PATCH] D73916: [clang] Add `forceReload` clangd extension to 'textDocument/didChange'

2020-02-07 Thread David Goldman via Phabricator via cfe-commits
dgoldman updated this revision to Diff 243251.
dgoldman marked 5 inline comments as done.
dgoldman added a comment.

- Refactor to use `forceRebuild` and `ParseOptions`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73916

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -615,6 +615,46 @@
   ASSERT_FALSE(DoUpdate(OtherSourceContents));
 }
 
+TEST_F(TUSchedulerTests, ForceRebuild) {
+  TUScheduler S(CDB, optsForTest(), captureDiags());
+
+  auto Source = testPath("foo.cpp");
+  auto Header = testPath("foo.h");
+
+  auto SourceContents = R"cpp(
+  #include "foo.h"
+  int b = a;
+)cpp";
+
+  // Return value indicates if the updated callback was received.
+  auto DoUpdate = [&](std::string Contents, bool ForceRebuild) -> bool {
+std::atomic Updated(false);
+Updated = false;
+ParseInputs Inputs = getInputs(Source, std::string(Contents));
+Inputs.Opts.ForceRebuild = ForceRebuild;
+updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
+[&Updated](std::vector) { Updated = true; });
+bool UpdateFinished = S.blockUntilIdle(timeoutSeconds(10));
+if (!UpdateFinished)
+  ADD_FAILURE() << "Updated has not finished in one second. Threading bug?";
+return Updated;
+  };
+
+  // Update the source contents, which should trigger an initial build with
+  // the header file missing.
+  ASSERT_TRUE(DoUpdate(SourceContents, /*ForceRebuild*/false));
+
+  // Add the header file.
+  Files[Header] = "int a;";
+  Timestamps[Header] = time_t(1);
+
+  // The addition of the missing header file shouldn't trigger a rebuild since
+  // we don't track missing files.
+  ASSERT_FALSE(DoUpdate(SourceContents, /*ForceRebuild*/false)); 
+
+  // Forcing the reload should should cause a rebuild, though.
+  ASSERT_TRUE(DoUpdate(SourceContents, /*ForceRebuild*/true)); 
+}
 TEST_F(TUSchedulerTests, NoChangeDiags) {
   TUScheduler S(CDB, optsForTest(), captureDiags());
 
@@ -721,7 +761,8 @@
   TUScheduler S(CDB, optsForTest(), captureDiags());
   std::vector Diagnostics;
   updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
-  WantDiagnostics::Yes, [&](std::vector D) {
+  WantDiagnostics::Yes,
+  [&](std::vector D) {
 Diagnostics = std::move(D);
 Ready.notify();
   });
@@ -745,7 +786,8 @@
   TUScheduler S(CDB, optsForTest(), captureDiags());
   std::vector Diagnostics;
   updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
-  WantDiagnostics::Yes, [&](std::vector D) {
+  WantDiagnostics::Yes,
+  [&](std::vector D) {
 Diagnostics = std::move(D);
 Ready.notify();
   });
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -432,7 +432,8 @@
 }
 
 std::shared_ptr OldPreamble =
-getPossiblyStalePreamble();
+Inputs.Opts.ForceRebuild ? std::shared_ptr()
+ : getPossiblyStalePreamble();
 std::shared_ptr NewPreamble = buildPreamble(
 FileName, *Invocation, OldPreamble, OldCommand, Inputs,
 StorePreambleInMemory,
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -652,6 +652,12 @@
   /// either they will be provided for this version or some subsequent one.
   /// This is a clangd extension.
   llvm::Optional wantDiagnostics;
+
+  /// Force a complete rebuild of the file, ignoring all cached state. Slow!
+  /// Disabled by default. This is useful to defeat clangd's assumption that
+  /// missing headers stay missing.
+  /// This is a clangd extension.
+  bool forceRebuild;
 };
 bool fromJSON(const llvm::json::Value &, DidChangeTextDocumentParams &);
 
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -430,7 +430,10 @@
 
 bool fromJSON(const llvm::json::Value &Params, DidChangeTextDocumentParams &R) {
   llvm::json::ObjectMapp

[PATCH] D73916: [clang] Add `forceReload` clangd extension to 'textDocument/didChange'

2020-02-07 Thread David Goldman via Phabricator via cfe-commits
dgoldman marked an inline comment as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:653
 
-  Server->addDocument(File, *Contents, WantDiags);
+  Server->addDocument(File, *Contents, WantDiags, ForceReload);
 }

sammccall wrote:
> Rather than plumbing this down many levels as yet-another-bool-param, can we 
> shove it into either ParseInputs or ParseOptions?
> e.g. `bool ParseInputs::AllowCached = true;` and set it to false here?
> 
> It's kind of a stretch for the API as we won't make use of it everywhere that 
> ParseInputs is used, but I think it's better than adding a niche boolean 
> param everywhere.
Done, this cleaned up the tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73916



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


[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

2020-02-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3687
 
-  args.add(EmitAnyExprToTemp(E), type);
 }

thegameg wrote:
> Is there any other use of `EmitAnyExprToTemp` that can benefit from this?
Right now, the only other user of that function is 
`CodeGenFunction::EmitStmtExprLValue`, which doesn't seem all that interesting. 
There are a lot of places that do create temporary allocas that would be worth 
looking into in the future though. 


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

https://reviews.llvm.org/D74094



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


  1   2   >