[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprAgg.cpp:964
+RHS = CGF.EmitAnyExpr(E->getRHS()).getAggregatePointer();
+break;
+  case TEK_Complex:

It looks like we don't actually support any aggregate types here, which I think 
is fine because comparing those types is only sensible for things like calls.  
If you do want to pave the way for that, or (probably more usefully) for 
supporting complex types, you should make EmitCompare take the operands as 
RValues and just use EmitAnyExpr here without paying any attention to the 
evaluation kind.



Comment at: lib/CodeGen/CGExprAgg.cpp:1000
+  assert(CmpInfo.Record->isTriviallyCopyable() &&
+ "cannot copy non-trivially copyable aggregate");
+

This assertion should really be further up in this function, because you're 
already relying on this quite heavily by this point.


https://reviews.llvm.org/D45476



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

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

[David CARLIER via Phabricator via cfe-commits]

devnexen added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1485
 
+  state = CheckOverlap(C, state, CE->getArg(2), Dst, srcExpr);
+

NoQ wrote:
> This crashes on the old tests for the checker. I guess that's because the 
> normal `strcpy()` doesn't have three arguments (it counts from 0).
True I forgot those cases.


https://reviews.llvm.org/D45177



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

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

[David CARLIER via Phabricator via cfe-commits]

devnexen updated this revision to Diff 145352.

https://reviews.llvm.org/D45177

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/bsd-string.c

Index: test/Analysis/bsd-string.c
===
--- /dev/null
+++ test/Analysis/bsd-string.c
@@ -0,0 +1,40 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
+
+#define NULL ((void *)0)
+
+typedef __typeof(sizeof(int)) size_t;
+size_t strlcpy(char *dst, const char *src, size_t n);
+size_t strlcat(char *dst, const char *src, size_t n);
+void clang_analyzer_eval(int);
+
+void f1() {
+  char overlap[] = "123456789";
+  strlcpy(overlap, overlap + 1, 3); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void f2() {
+  char buf[5];
+  strlcpy(buf, "abcd", sizeof(buf)); // expected-no-warning
+  strlcat(buf, "efgh", sizeof(buf)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+}
+
+void f3() {
+  char dst[2];
+  const char *src = "abdef";
+  strlcpy(dst, src, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+}
+
+void f4() {
+  strlcpy(NULL, "abcdef", 6); // expected-warning{{Null pointer argument in call to string copy function}}
+}
+
+void f5() {
+  strlcat(NULL, "abcdef", 6); // expected-warning{{Null pointer argument in call to string copy function}}
+}
+
+void f6() {
+  char buf[8];
+  strlcpy(buf, "abc", 3);
+  size_t len = strlcat(buf, "defg", 4);
+  clang_analyzer_eval(len == 7); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -97,14 +97,17 @@
   void evalStrcpy(CheckerContext , const CallExpr *CE) const;
   void evalStrncpy(CheckerContext , const CallExpr *CE) const;
   void evalStpcpy(CheckerContext , const CallExpr *CE) const;
+  void evalStrlcpy(CheckerContext , const CallExpr *CE) const;
   void evalStrcpyCommon(CheckerContext ,
 const CallExpr *CE,
 bool returnEnd,
 bool isBounded,
-bool isAppending) const;
+bool isAppending,
+bool returnPtr = true) const;
 
   void evalStrcat(CheckerContext , const CallExpr *CE) const;
   void evalStrncat(CheckerContext , const CallExpr *CE) const;
+  void evalStrlcat(CheckerContext , const CallExpr *CE) const;
 
   void evalStrcmp(CheckerContext , const CallExpr *CE) const;
   void evalStrncmp(CheckerContext , const CallExpr *CE) const;
@@ -1393,6 +1396,18 @@
/* isAppending = */ false);
 }
 
+void CStringChecker::evalStrlcpy(CheckerContext , const CallExpr *CE) const {
+  if (CE->getNumArgs() < 3)
+return;
+
+  // char *strlcpy(char *dst, const char *src, size_t n);
+  evalStrcpyCommon(C, CE,
+   /* returnEnd = */ true,
+   /* isBounded = */ true,
+   /* isAppending = */ false,
+   /* returnPtr = */ false);
+}
+
 void CStringChecker::evalStrcat(CheckerContext , const CallExpr *CE) const {
   if (CE->getNumArgs() < 2)
 return;
@@ -1415,9 +1430,21 @@
/* isAppending = */ true);
 }
 
+void CStringChecker::evalStrlcat(CheckerContext , const CallExpr *CE) const {
+  if (CE->getNumArgs() < 3)
+return;
+
+  //char *strlcat(char *s1, const char *s2, size_t n);
+  evalStrcpyCommon(C, CE,
+   /* returnEnd = */ false,
+   /* isBounded = */ true,
+   /* isAppending = */ true,
+   /* returnPtr = */ false);
+}
+
 void CStringChecker::evalStrcpyCommon(CheckerContext , const CallExpr *CE,
   bool returnEnd, bool isBounded,
-  bool isAppending) const {
+  bool isAppending, bool returnPtr) const {
   CurrentFunctionDescription = "string copy function";
   ProgramStateRef state = C.getState();
   const LocationContext *LCtx = C.getLocationContext();
@@ -1455,6 +1482,11 @@
   SVal maxLastElementIndex = UnknownVal();
   const char *boundWarning = nullptr;
 
+  state = CheckOverlap(C, state, isBounded ? CE->getArg(2) : CE->getArg(1), Dst, srcExpr);
+
+  if (!state)
+return;
+
   // If the function is strncpy, strncat, etc... it is bounded.
   if (isBounded) {
 // Get the max number of characters to copy.
@@ -1658,35 +1690,41 @@
 finalStrLength = amountCopied;
   }
 
-  // The final result of the function will either be a pointer past the last
-  // copied element, or a pointer to the start of the destination buffer.
-  SVal Result = (returnEnd ? UnknownVal() : DstVal);
+  SVal Result;
+
+  if (returnPtr) {
+// The final 

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/ComparisonCategories.h:71
+  /// standard library. The key is a value of ComparisonCategoryResult.
+  mutable llvm::DenseMap Objects;
+

EricWF wrote:
> rjmccall wrote:
> > We expect this map to have at least two of the seven result types, and 
> > probably three or four, right?  It should probably just be an array; it'll 
> > both be faster and use less memory.
> > 
> > (The similar map in `ComparisonCategories` is different because we expect 
> > it to be empty in most translation units.)
> Ack.
> 
> Do you want `std::array` or something slightly more conservative like 
> `llvm::SmallVector`?
std::array is definitely better here.



Comment at: lib/AST/ComparisonCategories.cpp:85
+  return nullptr;
+}
+

EricWF wrote:
> rjmccall wrote:
> > This method is returning a pointer to an entry of a DenseMap.  The 
> > resulting pointer is then treated as a stable key in a set on Sema.  That 
> > pointer will be dangling if the DenseMap needs to grow beyond its original 
> > allocation.
> > 
> > I would suggest perhaps storing a fixed-size array of pointers to 
> > ComparisonCategoryInfos that you allocate on-demand.
> Woops! Thanks for the correction. I'm so used to STL node-based maps I 
> assumed the keys were stable.
> 
> I'll use a bitset, and index into it using the `ComparisonCategoryType` 
> enumerators as indexes.
Sounds good.



Comment at: lib/CodeGen/CGExprAgg.cpp:971
+  auto EmitCmpRes = [&](const VarDecl *VD) {
+return CGF.CGM.GetAddrOfGlobalVar(VD);
+  };

EricWF wrote:
> rjmccall wrote:
> > EricWF wrote:
> > > rsmith wrote:
> > > > Perhaps directly emit the constant value here rather than the address 
> > > > of the global? I think we should consider what IR we want to see coming 
> > > > out of Clang, and I don't think that IR should contain loads from 
> > > > globals to get the small constant integer that is the value of the 
> > > > conversion result.
> > > > 
> > > > I think it would be reasonable for us to say that we require the 
> > > > standard library types to contain exactly one non-static data member of 
> > > > integral type, and for us to form a select between the relevant integer 
> > > > values here. We really have no need to support all possible 
> > > > implementations of these types, and we can revisit this if some other 
> > > > standard library implementation ships types that don't follow that 
> > > > pattern. (If we find such a standard library, we could emit multiple 
> > > > selects, or a first-class aggregate select, or whatever generates the 
> > > > best code at -O0.)
> > > I agree emitting the value would be better, and that most STL 
> > > implementations should implement the types using only one non-static 
> > > member.
> > > However, note that the specification for `partial_ordering` is described 
> > > in terms of two non-static data members, so it seems possible an STL 
> > > implementation might implement in that way.
> > > 
> > > Would it be appropriate to do this as a smaller follow up patch?
> > While it would be nice if we could special-case the case of a class with a 
> > single integral field all the way through the various uses of it, IRGen is 
> > not at all set up to try to take advantage of that.  You will need to store 
> > your integral result into the dest slot here anyway.  That makes me suspect 
> > that there's just no value in trying to produce a scalar select before 
> > doing that store instead of branching to pick one of two stores.
> > 
> > Also, I know there's this whole clever argument for why we can get away 
> > with lazily finding this comparison-result type and its static members in 
> > translation units that are just deserializing a spaceship operator.  Just 
> > to make me feel better, though, please actually check here dynamically that 
> > the assumptions we're relying on actually hold and that we've found an 
> > appropriate variable for the comparison result and it does have an 
> > initializer.  It is fine to generate an atrocious diagnostic if we see a 
> > failure, but let's please not crash just because something weird and 
> > unexpected happened with module import.
> > While it would be nice if we could special-case the case of a class with a 
> > single integral field all the way through the various uses of it, IRGen is 
> > not at all set up to try to take advantage of that. You will need to store 
> > your integral result into the dest slot here anyway. That makes me suspect 
> > that there's just no value in trying to produce a scalar select before 
> > doing that store instead of branching to pick one of two stores.
> 
> I went ahead and did it anyway, though I suspect you might be right. I'll 
> need to look into it further. (In particular if we avoid ODR uses and hence 
> can avoid emitting the inline variable definitions).
> 
> > 

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 12 inline comments as done.
EricWF added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:9825
+LHS = S.ImpCastExprToType(LHS.get(), IntType, CK_IntegralCast);
+RHS = S.ImpCastExprToType(RHS.get(), IntType, CK_IntegralCast);
+LHSType = RHSType = IntType;

EricWF wrote:
> rjmccall wrote:
> > I believe this will assert if the underlying type of the enum is `bool`.
> Ack. It does indeed.
> 
> What's the correct way to promote the boolean to an integer?
I seem to have solved the problem for enums with an underlying type of bool by 
first performing LValueToRValue conversions, followed by a conversion to `int` 
instead of `bool`.

Does that sound reasonable?



Comment at: lib/Sema/SemaExpr.cpp:9955
+  // and direction polls
+  return buildResultTy(ComparisonCategoryType::StrongEquality);
+

rjmccall wrote:
> Should we generate a tautological warning about comparisons on `nullptr_t` 
> that aren't the result of some kind of macro/template expansion?
Probably? But we don't currently do it for equality operators, so perhaps this 
could be done in a follow up patch which adds the diagnostic for both equality 
and three-way comparisons?


https://reviews.llvm.org/D45476



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF marked 18 inline comments as done.
EricWF added inline comments.



Comment at: include/clang/AST/ASTContext.h:1986
+  /// This object needs to be initialized by Sema the first time it checks
+  /// a three-way comparison.
+  ComparisonCategories CompCategories;

rjmccall wrote:
> Is this comment accurate?  Because if your test case with a deserialized 
> comparison operator is supposed to work, we certainly don't get that.
Woops. No, it is not accurate. It's a remnant of an earlier attempt. I'll 
remove it.



Comment at: include/clang/AST/ComparisonCategories.h:71
+  /// standard library. The key is a value of ComparisonCategoryResult.
+  mutable llvm::DenseMap Objects;
+

rjmccall wrote:
> We expect this map to have at least two of the seven result types, and 
> probably three or four, right?  It should probably just be an array; it'll 
> both be faster and use less memory.
> 
> (The similar map in `ComparisonCategories` is different because we expect it 
> to be empty in most translation units.)
Ack.

Do you want `std::array` or something slightly more conservative like 
`llvm::SmallVector`?



Comment at: lib/AST/ComparisonCategories.cpp:85
+  return nullptr;
+}
+

rjmccall wrote:
> This method is returning a pointer to an entry of a DenseMap.  The resulting 
> pointer is then treated as a stable key in a set on Sema.  That pointer will 
> be dangling if the DenseMap needs to grow beyond its original allocation.
> 
> I would suggest perhaps storing a fixed-size array of pointers to 
> ComparisonCategoryInfos that you allocate on-demand.
Woops! Thanks for the correction. I'm so used to STL node-based maps I assumed 
the keys were stable.

I'll use a bitset, and index into it using the `ComparisonCategoryType` 
enumerators as indexes.



Comment at: lib/CodeGen/CGExprAgg.cpp:971
+  auto EmitCmpRes = [&](const VarDecl *VD) {
+return CGF.CGM.GetAddrOfGlobalVar(VD);
+  };

rjmccall wrote:
> EricWF wrote:
> > rsmith wrote:
> > > Perhaps directly emit the constant value here rather than the address of 
> > > the global? I think we should consider what IR we want to see coming out 
> > > of Clang, and I don't think that IR should contain loads from globals to 
> > > get the small constant integer that is the value of the conversion result.
> > > 
> > > I think it would be reasonable for us to say that we require the standard 
> > > library types to contain exactly one non-static data member of integral 
> > > type, and for us to form a select between the relevant integer values 
> > > here. We really have no need to support all possible implementations of 
> > > these types, and we can revisit this if some other standard library 
> > > implementation ships types that don't follow that pattern. (If we find 
> > > such a standard library, we could emit multiple selects, or a first-class 
> > > aggregate select, or whatever generates the best code at -O0.)
> > I agree emitting the value would be better, and that most STL 
> > implementations should implement the types using only one non-static member.
> > However, note that the specification for `partial_ordering` is described in 
> > terms of two non-static data members, so it seems possible an STL 
> > implementation might implement in that way.
> > 
> > Would it be appropriate to do this as a smaller follow up patch?
> While it would be nice if we could special-case the case of a class with a 
> single integral field all the way through the various uses of it, IRGen is 
> not at all set up to try to take advantage of that.  You will need to store 
> your integral result into the dest slot here anyway.  That makes me suspect 
> that there's just no value in trying to produce a scalar select before doing 
> that store instead of branching to pick one of two stores.
> 
> Also, I know there's this whole clever argument for why we can get away with 
> lazily finding this comparison-result type and its static members in 
> translation units that are just deserializing a spaceship operator.  Just to 
> make me feel better, though, please actually check here dynamically that the 
> assumptions we're relying on actually hold and that we've found an 
> appropriate variable for the comparison result and it does have an 
> initializer.  It is fine to generate an atrocious diagnostic if we see a 
> failure, but let's please not crash just because something weird and 
> unexpected happened with module import.
> While it would be nice if we could special-case the case of a class with a 
> single integral field all the way through the various uses of it, IRGen is 
> not at all set up to try to take advantage of that. You will need to store 
> your integral result into the dest slot here anyway. That makes me suspect 
> that there's just no value in trying to produce a scalar select before doing 
> that store 

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

In https://reviews.llvm.org/D44934#1088771, @NoQ wrote:

> Hmm, ok, it seems that i've just changed the API in 
> https://reviews.llvm.org/D46368, and i should have thought about this use 
> case. Well, at least i have some background understanding of these problems 
> now. Sorry for not keeping eye on this problem.
>
> In https://reviews.llvm.org/D44934#1051002, @NoQ wrote:
>
> > Why do you need separate code for null and non-null character? The 
> > function's semantics doesn't seem to care.
>
>
> I guess i can answer myself here:
>
>   int32_t x;
>   memset(, 1, sizeof(int32_t));
>   clang_analyzer_eval(x == 0x1010101); // should be TRUE
>
>
> I really doubt that we support this case.
>
> So, yeah, zero character is indeed special.


Thank you, Artem! I did not consider this common situation.  This patch does 
not really support this situation, in this patch the value of `x` will be 1, 
it's not correct!

> And since zero character is special, i guess we could use the new 
> `bindDefaultZero()` API, and perform a simple invalidation in the non-zero 
> character case.

Agree with you. The core problem here is that there is no perfect way to `bind 
default` for non-zero value, not the string length stuff. So **invalidate the 
destination buffer** but still **set string length** for non-zero character is 
OK. Right?

> @MTC, would this be enough for your use case? Or is it really important for 
> you to support the non-zero character case? Cause my example is fairly 
> artificial, and probably i'm worrying too much about it. If nobody really 
> codes that way, i'm fine with supporting the non-zero character case via a 
> simple default binding. In this case we should merge `bindDefaultZero()` with 
> your `overwriteRegion()` (i'd prefer your function name, but please keep the 
> empty class check).

Based on my limited programming experience, I think `memset` with non-zero 
character is common. However given that we can't handle non-zero character 
binding problems very well, we should now support only zero character. After 
all, IMHO,  correctness of semantic is also important  for the analyzer.

I will update this patch to only handle zero character case. In the future, as 
I am more familiar with the `RegionStore`, I will solve the problem of non-zero 
character binding.


Repository:
  rC Clang

https://reviews.llvm.org/D44934



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

[PATCH] D46489: [HIP] Let assembler output bitcode for amdgcn

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think the right solution here is to make a CompileJobAction with type 
TY_LLVM_BC in the first place.  You should get the advice of a driver expert, 
though.


https://reviews.llvm.org/D46489



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

[PATCH] D46475: [HIP] Set proper triple and offload kind for the toolchain

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Driver/Driver.cpp:554
+   }) ||
+  IsHIP) {
 const ToolChain *HostTC = C.getSingleOffloadToolChain();

It seems to me that it wouldn't be too hard to do your TODO here; it's 
basically just checking for HIP (and remembering that you saw it) in the any_of 
function, right?


https://reviews.llvm.org/D46475



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

[PATCH] D46489: [HIP] Let assembler output bitcode for amdgcn

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

amdgcn does not support linking of object files, therefore let
assembler output bitcode for HIP for amdgcn.


https://reviews.llvm.org/D46489

Files:
  lib/Driver/ToolChains/Clang.cpp


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3213,7 +3213,11 @@
 CmdArgs.push_back("-P");
 }
   } else if (isa(JA)) {
-CmdArgs.push_back("-emit-obj");
+// AMDGCN does not support linking obj files.
+if (IsHIP && Triple.getArch() == llvm::Triple::amdgcn)
+  CmdArgs.push_back("-emit-llvm-bc");
+else
+  CmdArgs.push_back("-emit-obj");
 
 CollectArgsForIntegratedAssembler(C, Args, CmdArgs, D);
 


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3213,7 +3213,11 @@
 CmdArgs.push_back("-P");
 }
   } else if (isa(JA)) {
-CmdArgs.push_back("-emit-obj");
+// AMDGCN does not support linking obj files.
+if (IsHIP && Triple.getArch() == llvm::Triple::amdgcn)
+  CmdArgs.push_back("-emit-llvm-bc");
+else
+  CmdArgs.push_back("-emit-obj");
 
 CollectArgsForIntegratedAssembler(C, Args, CmdArgs, D);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46473: [HIP] Let clang-offload-bundler support HIP

[John McCall via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D46473



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

[PATCH] D46472: [HIP] Support offloading by linkscript

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

The IRGen aspects of this seem reasonable, modulo a comment change.  I don't 
feel qualified to judge the driver change.




Comment at: lib/CodeGen/CGCUDANV.cpp:317
+  if (GpuBinaryFileName.empty() && !IsHIP)
 return nullptr;
 

Is this filename string only used for CUDA?  If so, please rename it to make 
that clear, or at least add a comment.


https://reviews.llvm.org/D46472



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

[PATCH] D46471: [HIP] Add hip offload kind

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Otherwise LGTM.




Comment at: lib/Driver/Compilation.cpp:201
+  // not compiled again if there are already failures. It is OK to abort the
+  // CUDA pipeline on errors.
+  if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP))

Mentioning only CUDA in the second clause makes me wonder whether it's *only* 
okay to abort a CUDA pipeline, not a HIP one.  That is presumably not your 
intent.  You could just drop "CUDA" there.


https://reviews.llvm.org/D46471



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

[PATCH] D46487: [HIP] Diagnose unsupported host triple

[John McCall via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D46487



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

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Peter Collingbourne via Phabricator via cfe-commits]

pcc accepted this revision.
pcc added a comment.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D46464



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

[PATCH] D45174: non-zero-length bit-fields should make a class non-empty

[John McCall via Phabricator via cfe-commits]

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

Okay, LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D45174



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

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll:14
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc 
-fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml.thin.0.yaml | FileCheck %s -check-prefix=YAML
+

pcc wrote:
> This file isn't named correctly according to the `-foptimization-record-file` 
> flag, right? Looks like the easy fix would be to pass -1 as the task 
> identifier to thinBackend, but it would probably be worth looking more 
> closely at some point at how we name these extra files in LTO.
I realized after cleaning out old test outputs that this causes an issue with 
the handling of save-temps for ThinLTO distributed backends. Previously they 
were adding the task ID of "0", and this became unsigned -1 after this change. 
I sent an LLVM patch D46488 to change that so no Task ID is added to the path 
if it is -1, and updated the test in this clang patch.


Repository:
  rC Clang

https://reviews.llvm.org/D46464



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

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 145348.
tejohnson added a comment.

Update test for change to pass -1 as the Task ID for distributed backends,
and to reflect companion llvm change (https://reviews.llvm.org/D46488).


Repository:
  rC Clang

https://reviews.llvm.org/D46464

Files:
  lib/CodeGen/BackendUtil.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
  test/CodeGen/thinlto_backend.ll

Index: test/CodeGen/thinlto_backend.ll
===
--- test/CodeGen/thinlto_backend.ll
+++ test/CodeGen/thinlto_backend.ll
@@ -29,13 +29,13 @@
 
 ; Ensure f2 was imported. Check for all 3 flavors of -save-temps[=cwd|obj].
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj
-; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: llvm-dis %t1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
 ; RUN: mkdir -p %T/dir1
 ; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd)
-; RUN: llvm-dis %T/dir1/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: llvm-dis %T/dir1/*1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
 ; RUN: mkdir -p %T/dir2
 ; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps)
-; RUN: llvm-dis %T/dir2/*1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; RUN: llvm-dis %T/dir2/*1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
 ; CHECK-IMPORT: define available_externally void @f2()
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s
 ; CHECK-OBJ: T f1
Index: test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
===
--- /dev/null
+++ test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
@@ -0,0 +1,47 @@
+; Test to ensure -fdiagnostics-show-hotness and -fsave-optimization-record
+; work when invoking the ThinLTO backend path.
+; RUN: opt -module-summary -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+
+; First try with pass remarks to stderr
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+
+; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 300)
+
+; Next try YAML pass remarks file
+; RUN: rm -f %t2.opt.yaml
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml | FileCheck %s -check-prefix=YAML
+
+; YAML: --- !Passed
+; YAML-NEXT: Pass:inline
+; YAML-NEXT: Name:Inlined
+; YAML-NEXT: Function:main
+; YAML-NEXT: Hotness: 300
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee:  tinkywinky
+; YAML-NEXT:   - String:  ' inlined into '
+; YAML-NEXT:   - Caller:  main
+; YAML-NEXT:   - String:  ' with cost='
+; YAML-NEXT:   - Cost:'0'
+; YAML-NEXT:   - String:  ' (threshold='
+; YAML-NEXT:   - Threshold:   '337'
+; YAML-NEXT:   - String:  ')'
+; YAML-NEXT: ...
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+declare i32 @patatino()
+
+define i32 @tinkywinky() {
+  %a = call i32 @patatino()
+  ret i32 %a
+}
+
+define i32 @main() !prof !0 {
+  %i = call i32 @tinkywinky()
+  ret i32 %i
+}
+
+!0 = !{!"function_entry_count", i64 300}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1075,7 +1075,9 @@
   bool UsingProfile = UsingSampleProfile ||
   (Opts.getProfileUse() != CodeGenOptions::ProfileNone);
 
-  if (Opts.DiagnosticsWithHotness && !UsingProfile)
+  if (Opts.DiagnosticsWithHotness && !UsingProfile &&
+  // An IR file will contain PGO as metadata
+  IK.getLanguage() != InputKind::LLVM_IR)
 Diags.Report(diag::warn_drv_diagnostics_hotness_requires_pgo)
 << "-fdiagnostics-show-hotness";
 
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -1136,6 +1136,8 @@
   Conf.SampleProfile = std::move(SampleProfile);
   Conf.UseNewPM = CGOpts.ExperimentalNewPassManager;
   Conf.DebugPassManager = CGOpts.DebugPassManager;
+  Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
+  Conf.RemarksFilename = CGOpts.OptRecordFile;
   switch (Action) {
   case Backend_EmitNothing:
 Conf.PreCodeGenModuleHook = [](size_t Task, const Module ) {
@@ -1159,7 +1161,7 @@
 break;
   }
   if (Error E = thinBackend(
-  Conf, 0, 

[PATCH] D46487: [HIP] Diagnose unsupported host triple

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

https://reviews.llvm.org/D46487

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/Driver.cpp
  test/Driver/cuda-bad-arch.cu


Index: test/Driver/cuda-bad-arch.cu
===
--- test/Driver/cuda-bad-arch.cu
+++ test/Driver/cuda-bad-arch.cu
@@ -2,6 +2,7 @@
 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
 
 // RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=compute_20 -c %s 
2>&1 \
 // RUN: | FileCheck -check-prefix BAD %s
@@ -25,9 +26,12 @@
 // RUN: %clang -### -target x86_64-linux-gnu -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix OK %s
 
-// We don't allow using NVPTX for host compilation.
+// We don't allow using NVPTX/AMDGCN for host compilation.
 // RUN: %clang -### --cuda-host-only -target nvptx-nvidia-cuda -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix HOST_NVPTX %s
+// RUN: %clang -### --cuda-host-only -target amdgcn-amd-amdhsa -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix HOST_AMDGCN %s
 
 // OK-NOT: error: Unsupported CUDA gpu architecture
-// HOST_NVPTX: error: unsupported use of NVPTX for host compilation.
+// HOST_NVPTX: error: unsupported architecture 'nvptx' for host compilation.
+// HOST_AMDGCN: error: unsupported architecture 'amdgcn' for host compilation.
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2331,11 +2331,13 @@
 
   const ToolChain *HostTC = 
C.getSingleOffloadToolChain();
   assert(HostTC && "No toolchain for host compilation.");
-  if (HostTC->getTriple().isNVPTX()) {
-// We do not support targeting NVPTX for host compilation. Throw
+  if (HostTC->getTriple().isNVPTX() ||
+  HostTC->getTriple().getArch() == llvm::Triple::amdgcn) {
+// We do not support targeting NVPTX/AMDGCN for host compilation. Throw
 // an error and abort pipeline construction early so we don't trip
 // asserts that assume device-side compilation.
-C.getDriver().Diag(diag::err_drv_cuda_nvptx_host);
+C.getDriver().Diag(diag::err_drv_cuda_host_arch)
+<< HostTC->getTriple().getArchName();
 return true;
   }
 
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -40,7 +40,7 @@
   "but installation at %3 is %4.  Use --cuda-path to specify a different CUDA "
   "install, pass a different GPU arch with --cuda-gpu-arch, or pass "
   "--no-cuda-version-check.">;
-def err_drv_cuda_nvptx_host : Error<"unsupported use of NVPTX for host 
compilation.">;
+def err_drv_cuda_host_arch : Error<"unsupported architecture '%0' for host 
compilation.">;
 def err_drv_invalid_thread_model_for_target : Error<
   "invalid thread model '%0' in '%1' for this target">;
 def err_drv_invalid_linker_name : Error<


Index: test/Driver/cuda-bad-arch.cu
===
--- test/Driver/cuda-bad-arch.cu
+++ test/Driver/cuda-bad-arch.cu
@@ -2,6 +2,7 @@
 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
 
 // RUN: %clang -### -target x86_64-linux-gnu --cuda-gpu-arch=compute_20 -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix BAD %s
@@ -25,9 +26,12 @@
 // RUN: %clang -### -target x86_64-linux-gnu -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix OK %s
 
-// We don't allow using NVPTX for host compilation.
+// We don't allow using NVPTX/AMDGCN for host compilation.
 // RUN: %clang -### --cuda-host-only -target nvptx-nvidia-cuda -c %s 2>&1 \
 // RUN: | FileCheck -check-prefix HOST_NVPTX %s
+// RUN: %clang -### --cuda-host-only -target amdgcn-amd-amdhsa -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix HOST_AMDGCN %s
 
 // OK-NOT: error: Unsupported CUDA gpu architecture
-// HOST_NVPTX: error: unsupported use of NVPTX for host compilation.
+// HOST_NVPTX: error: unsupported architecture 'nvptx' for host compilation.
+// HOST_AMDGCN: error: unsupported architecture 'amdgcn' for host compilation.
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2331,11 +2331,13 @@
 
   const ToolChain *HostTC = C.getSingleOffloadToolChain();
   assert(HostTC && "No toolchain for host compilation.");
-  if (HostTC->getTriple().isNVPTX()) {
-// We do not support targeting NVPTX for host compilation. Throw
+  if (HostTC->getTriple().isNVPTX() ||
+  HostTC->getTriple().getArch() == llvm::Triple::amdgcn) {
+// We do not support targeting NVPTX/AMDGCN for host compilation. Throw
 // 

r331578 - Fix a couple places that immediately called operator-> on the result of dyn_cast.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Fri May  4 18:58:26 2018
New Revision: 331578

URL: http://llvm.org/viewvc/llvm-project?rev=331578=rev
Log:
Fix a couple places that immediately called operator-> on the result of 
dyn_cast.

It looks like it safe to just use cast for both cases.

Modified:
cfe/trunk/lib/AST/QualTypeNames.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Modified: cfe/trunk/lib/AST/QualTypeNames.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/QualTypeNames.cpp?rev=331578=331577=331578=diff
==
--- cfe/trunk/lib/AST/QualTypeNames.cpp (original)
+++ cfe/trunk/lib/AST/QualTypeNames.cpp Fri May  4 18:58:26 2018
@@ -408,7 +408,7 @@ QualType getFullyQualifiedType(QualType
 // Get the qualifiers.
 Qualifiers Quals = QT.getQualifiers();
 
-QT = dyn_cast(QT.getTypePtr())->desugar();
+QT = cast(QT.getTypePtr())->desugar();
 
 // Add back the qualifiers.
 QT = Ctx.getQualifiedType(QT, Quals);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=331578=331577=331578=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri May  4 
18:58:26 2018
@@ -483,7 +483,7 @@ private:
   if (!(isa(R) || isa(R)))
 return false; // Pattern-matching failed.
   Subregions.push_back(R);
-  R = dyn_cast(R)->getSuperRegion();
+  R = cast(R)->getSuperRegion();
 }
 bool IndirectReference = !Subregions.empty();
 


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

[libcxx] r331575 - [libcxx] [test] Fix MSVC x64 truncation warning.

[Stephan T. Lavavej via cfe-commits]

Author: stl_msft
Date: Fri May  4 18:40:24 2018
New Revision: 331575

URL: http://llvm.org/viewvc/llvm-project?rev=331575=rev
Log:
[libcxx] [test] Fix MSVC x64 truncation warning.

warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of 
data

Requesting post-commit review.

Modified:

libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.formatted/istream_extractors/streambuf.pass.cpp

Modified: 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.formatted/istream_extractors/streambuf.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.formatted/istream_extractors/streambuf.pass.cpp?rev=331575=331574=331575=diff
==
--- 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.formatted/istream_extractors/streambuf.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.formatted/istream_extractors/streambuf.pass.cpp
 Fri May  4 18:40:24 2018
@@ -50,7 +50,7 @@ protected:
 str_.resize(str_.capacity());
 base::setp(const_cast(str_.data()),
const_cast(str_.data() + str_.size()));
-base::pbump(n+1);
+base::pbump(static_cast(n+1));
 }
 return ch;
 }


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

[libcxx] r331576 - [libcxx] [test] Fix whitespace, NFC.

[Stephan T. Lavavej via cfe-commits]

Author: stl_msft
Date: Fri May  4 18:40:27 2018
New Revision: 331576

URL: http://llvm.org/viewvc/llvm-project?rev=331576=rev
Log:
[libcxx] [test] Fix whitespace, NFC.

Strip trailing whitespace and untabify.

Modified:

libcxx/trunk/test/std/iterators/stream.iterators/ostreambuf.iterator/ostreambuf.iter.ops/failed.pass.cpp

Modified: 
libcxx/trunk/test/std/iterators/stream.iterators/ostreambuf.iterator/ostreambuf.iter.ops/failed.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/iterators/stream.iterators/ostreambuf.iterator/ostreambuf.iter.ops/failed.pass.cpp?rev=331576=331575=331576=diff
==
--- 
libcxx/trunk/test/std/iterators/stream.iterators/ostreambuf.iterator/ostreambuf.iter.ops/failed.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/iterators/stream.iterators/ostreambuf.iterator/ostreambuf.iter.ops/failed.pass.cpp
 Fri May  4 18:40:27 2018
@@ -19,23 +19,23 @@
 
 template  >
 struct my_streambuf : public std::basic_streambuf {
-   typedef typename std::basic_streambuf::int_type  int_type;
-   typedef typename std::basic_streambuf::char_type char_type;
-   
-   my_streambuf() {}
-   int_type sputc(char_type) { return Traits::eof(); }
-   };
+typedef typename std::basic_streambuf::int_type  int_type;
+typedef typename std::basic_streambuf::char_type char_type;
+
+my_streambuf() {}
+int_type sputc(char_type) { return Traits::eof(); }
+};
 
 int main()
 {
 {
-   my_streambuf buf;
+my_streambuf buf;
 std::ostreambuf_iterator i();
 i = 'a';
 assert(i.failed());
 }
 {
-   my_streambuf buf;
+my_streambuf buf;
 std::ostreambuf_iterator i();
 i = L'a';
 assert(i.failed());


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

[PATCH] D43928: [analyzer] Correctly measure array size in security.insecureAPI.strcpy

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Sorry, i completely forgot about this one :(

I think this patch needs `lit` tests, eg. tell the analyzer to analyze a simple 
strcpy() call on any `-target` with non-8-bit chars and see if it's still 
crashes or behaves incorrectly.




Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
 if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  auto ArraySize = BR.getContext().getTypeSizeInChars(Array).getQuantity();
   if (const auto *String = dyn_cast(Source)) {

leanil wrote:
> NoQ wrote:
> > I suggest not using `auto` here, because it makes it harder to understand 
> > integer promotions (potential overflows or sign extensions) in the 
> > comparison.
> That's true. Should it be `CharUnits::QuantityType` then?
I think you should still `getQuantity()`, and then explicitly cast it to a type 
that's compatible with `String->getLength()`, so that there were no implicit 
integer casts around `>=`.


https://reviews.llvm.org/D43928



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

[PATCH] D46485: Add python tool to dump and construct header maps

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno created this revision.
bruno added a reviewer: rsmith.
Herald added a subscriber: mgorny.

Header maps are binary files used by Xcode, which are used to map
header names or paths to other locations. Clang has support for
those since its inception, but there's not a lot of header map
testing around.

Since it's a binary format, testing becomes pretty much brittle
and its hard to even know what's inside if you don't have the
appropriate tools.

Add a python based tool that allows creating and dumping header
maps based on a json description of those. While here, rewrite
tests to use the tool and remove the binary files from the tree.

This tool was written by Daniel Dunbar.

rdar://problem/39994722


Repository:
  rC Clang

https://reviews.llvm.org/D46485

Files:
  CMakeLists.txt
  test/CMakeLists.txt
  test/Preprocessor/Inputs/headermap-rel/foo.hmap
  test/Preprocessor/Inputs/headermap-rel/foo.hmap.json
  test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap
  test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap.json
  test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
  test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
  test/Preprocessor/headermap-rel.c
  test/Preprocessor/headermap-rel2.c
  test/Preprocessor/nonportable-include-with-hmap.c
  utils/hmaptool/CMakeLists.txt
  utils/hmaptool/hmaptool

Index: utils/hmaptool/hmaptool
===
--- /dev/null
+++ utils/hmaptool/hmaptool
@@ -0,0 +1,293 @@
+#!/usr/bin/env python
+
+import json
+import optparse
+import os
+import struct
+import sys
+
+###
+
+k_header_magic_LE = 'pamh'
+k_header_magic_BE = 'hmap'
+
+def hmap_hash(str):
+"""hash(str) -> int
+
+Apply the "well-known" headermap hash function.
+"""
+
+return sum((ord(c.lower()) * 13
+for c in str), 0)
+
+class HeaderMap(object):
+@staticmethod
+def frompath(path):
+with open(path, 'rb') as f:
+magic = f.read(4)
+if magic == k_header_magic_LE:
+endian_code = '<'
+elif magic == k_header_magic_BE:
+endian_code = '>'
+else:
+raise SystemExit("error: %s: not a headermap" % (
+path,))
+
+# Read the header information.
+header_fmt = endian_code + 'HH'
+header_size = struct.calcsize(header_fmt)
+data = f.read(header_size)
+if len(data) != header_size:
+raise SystemExit("error: %s: truncated headermap header" % (
+path,))
+
+(version, reserved, strtable_offset, num_entries,
+ num_buckets, max_value_len) = struct.unpack(header_fmt, data)
+
+if version != 1:
+raise SystemExit("error: %s: unknown headermap version: %r" % (
+path, version))
+if reserved != 0:
+raise SystemExit("error: %s: invalid reserved value in header" % (
+path,))
+
+# The number of buckets must be a power of two.
+if num_buckets == 0 or (num_buckets & num_buckets - 1) != 0:
+raise SystemExit("error: %s: invalid number of buckets" % (
+path,))
+
+# Read all of the buckets.
+bucket_fmt = endian_code + 'III'
+bucket_size = struct.calcsize(bucket_fmt)
+buckets_data = f.read(num_buckets * bucket_size)
+if len(buckets_data) != num_buckets * bucket_size:
+raise SystemExit("error: %s: truncated headermap buckets" % (
+path,))
+buckets = [struct.unpack(bucket_fmt,
+ buckets_data[i*bucket_size:(i+1)*bucket_size])
+   for i in range(num_buckets)]
+
+# Read the string table; the format doesn't explicitly communicate the
+# size of the string table (which is dumb), so assume it is the rest of
+# the file.
+f.seek(0, 2)
+strtable_size = f.tell() - strtable_offset
+f.seek(strtable_offset)
+
+strtable = f.read(strtable_size)
+if len(strtable) != strtable_size:
+raise SystemExit("error: %s: unable to read complete string table"%(
+path,))
+if strtable[-1] != '\0':
+raise SystemExit("error: %s: invalid string table in headermap" % (
+path,))
+
+return HeaderMap(num_entries, buckets, strtable)
+
+def __init__(self, num_entries, buckets, strtable):
+self.num_entries = num_entries
+self.buckets = buckets
+self.strtable = strtable
+
+def get_string(self, idx):
+if idx >= len(self.strtable):
+raise SystemExit("error: %s: invalid string index" % (
+path,))
+end_idx = 

[PATCH] D45177: CStringChecker, check strlcpy/strlcat

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1485
 
+  state = CheckOverlap(C, state, CE->getArg(2), Dst, srcExpr);
+

This crashes on the old tests for the checker. I guess that's because the 
normal `strcpy()` doesn't have three arguments (it counts from 0).


https://reviews.llvm.org/D45177



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/ASTContext.h:1986
+  /// This object needs to be initialized by Sema the first time it checks
+  /// a three-way comparison.
+  ComparisonCategories CompCategories;

Is this comment accurate?  Because if your test case with a deserialized 
comparison operator is supposed to work, we certainly don't get that.



Comment at: include/clang/AST/ComparisonCategories.h:71
+  /// standard library. The key is a value of ComparisonCategoryResult.
+  mutable llvm::DenseMap Objects;
+

We expect this map to have at least two of the seven result types, and probably 
three or four, right?  It should probably just be an array; it'll both be 
faster and use less memory.

(The similar map in `ComparisonCategories` is different because we expect it to 
be empty in most translation units.)



Comment at: include/clang/AST/ComparisonCategories.h:206
+  const ASTContext 
+  mutable llvm::DenseMap Data;
+  mutable NamespaceDecl *StdNS = nullptr;

Please add a comment explaining what the keys are.



Comment at: lib/AST/ComparisonCategories.cpp:85
+  return nullptr;
+}
+

This method is returning a pointer to an entry of a DenseMap.  The resulting 
pointer is then treated as a stable key in a set on Sema.  That pointer will be 
dangling if the DenseMap needs to grow beyond its original allocation.

I would suggest perhaps storing a fixed-size array of pointers to 
ComparisonCategoryInfos that you allocate on-demand.



Comment at: lib/CodeGen/CGExprAgg.cpp:971
+  auto EmitCmpRes = [&](const VarDecl *VD) {
+return CGF.CGM.GetAddrOfGlobalVar(VD);
+  };

EricWF wrote:
> rsmith wrote:
> > Perhaps directly emit the constant value here rather than the address of 
> > the global? I think we should consider what IR we want to see coming out of 
> > Clang, and I don't think that IR should contain loads from globals to get 
> > the small constant integer that is the value of the conversion result.
> > 
> > I think it would be reasonable for us to say that we require the standard 
> > library types to contain exactly one non-static data member of integral 
> > type, and for us to form a select between the relevant integer values here. 
> > We really have no need to support all possible implementations of these 
> > types, and we can revisit this if some other standard library 
> > implementation ships types that don't follow that pattern. (If we find such 
> > a standard library, we could emit multiple selects, or a first-class 
> > aggregate select, or whatever generates the best code at -O0.)
> I agree emitting the value would be better, and that most STL implementations 
> should implement the types using only one non-static member.
> However, note that the specification for `partial_ordering` is described in 
> terms of two non-static data members, so it seems possible an STL 
> implementation might implement in that way.
> 
> Would it be appropriate to do this as a smaller follow up patch?
While it would be nice if we could special-case the case of a class with a 
single integral field all the way through the various uses of it, IRGen is not 
at all set up to try to take advantage of that.  You will need to store your 
integral result into the dest slot here anyway.  That makes me suspect that 
there's just no value in trying to produce a scalar select before doing that 
store instead of branching to pick one of two stores.

Also, I know there's this whole clever argument for why we can get away with 
lazily finding this comparison-result type and its static members in 
translation units that are just deserializing a spaceship operator.  Just to 
make me feel better, though, please actually check here dynamically that the 
assumptions we're relying on actually hold and that we've found an appropriate 
variable for the comparison result and it does have an initializer.  It is fine 
to generate an atrocious diagnostic if we see a failure, but let's please not 
crash just because something weird and unexpected happened with module import.



Comment at: lib/Sema/SemaDeclCXX.cpp:8929
+return QualType();
+  }
+

Alright, works for me.



Comment at: lib/Sema/SemaDeclCXX.cpp:8956
+  // the cache and return the newly cached value.
+  FullyCheckedComparisonCategories.insert(Info);
+  return Info->getType();

I think you should probably do this insertion above (perhaps instead of the 
original `count` check) so that you don't dump 100 diagnostics on the user if 
they've got a malformed stdlib.



Comment at: lib/Sema/SemaExpr.cpp:9816
+RHS = S.ImpCastExprToType(RHS.get(), Type, CK_BitCast);
+  } else {
+// C++2a [expr.spaceship]p4

rsmith wrote:
> EricWF wrote:
> > 

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:2512
+  if (D && isa(D) &&
+  cast(D)->isCopyOrMoveConstructor())
+F->addParamAttr(1, llvm::Attribute::NoAlias);

Why does it matter whether it's a copy constructor?  The standard text you're 
quoting doesn't seem to make that distinction.


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ok, i personally think this patch should go in. I think it makes it clear 
enough that an unsuspecting user would suspect something by reading the flag, 
and makes the feature hard enough to discover. And i'm happy to support anybody 
who's going to work on this stuff.

-

~*sorry for hijacking review with this, we should move this conversation to the 
list*~

In https://reviews.llvm.org/D46159#1088768, @pfultz2 wrote:

> I am definitely interested in working on getting some of the checkers stable, 
> and have already started some work on expanding the ConversionChecker, but 
> would like user feedback as I move forward.


Yay! I would most likely have a chance to help out with final evaluation of 
checkers before they move out of alpha.

In https://reviews.llvm.org/D46159#1088768, @pfultz2 wrote:

> Some like the Conversion checker doesn't seem to have any issues(because its 
> fairly limited at this point).


Last time i tried it, it was very hard to understand positives produced by the 
checker. It seems that it needs to implement a relatively sophisticated "bug 
visitor" in order to explain why does it think that a certain conversion would 
overflow by highlighting the respective spot somewhere in the middle of the 
path. It is especially important when the value comes from an inlined function 
because the analyzer wouldn't draw paths through all inlined functions (it 
would have been overwhelming) unless something interesting happens in there.

Then somebody would need to have a look at intentional overflows and if it's 
possible to avoid them.

In https://reviews.llvm.org/D46159#1088768, @pfultz2 wrote:

> Others like StreamChecker probably just needs some simple fixes.


Hmm, at a glance, it looks almost empty.

1. Again, it needs a bug visitor to highlight where the file was open. 
Otherwise many reports may become impossible to understand.
2. It is eagerly splitting states upon file open, which doubles remaining 
analysis time on every file open. This needs to be deferred until an actual 
branch is found in the code.
3. It needs a pointer escape callback, in order to work with wrappers around 
open/close functions. This needs to be complemented with a certain knowledge 
about the standard library, to see what functions should not cause escapes. 
SimpleStreamChecker has an example.
4. If it's ever to support `open()`/`close()`, it would, apart from other 
things, also need an "integer escape" callback , which will require a bit of 
digging into the analyzer core.

Pt.4 is mostly about feature work, but Pt.1–3 are must-fix. This isn't an 
overwhelming amount of work, but it will take some time.

Otherwise, yeah, it's a good simple check to have. It could be expanded to 
support a variety of weird APIs such as `fdopen()` but it's also a checker 
that's hard to get wrong. PthreadLockChecker is also a great candidate with 
similar problems; i have some unfinished work on that on here on the 
phabricator.


https://reviews.llvm.org/D46159



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 145332.
AntonBikineev added a comment.
Herald added subscribers: aheejin, sbc100.

I've moved setting noalias-attribute down to IR-function creation. This is 
needed in the context of emitting a constructor call when the definition of the 
constructor is not available (and clang emits an IR-constructor-declaration). 
@lebedev.ri @aaron.ballman I've also adjusted existing tests. There a quite a 
few of them, so it seems there is no reason of having a specific test case.


Repository:
  rC Clang

https://reviews.llvm.org/D46441

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CXX/except/except.spec/p14-ir.cpp
  test/CodeGenCXX/constructor-init.cpp
  test/CodeGenCXX/copy-constructor-synthesis-2.cpp
  test/CodeGenCXX/copy-constructor-synthesis.cpp
  test/CodeGenCXX/dllexport-members.cpp
  test/CodeGenCXX/dllimport-members.cpp
  test/CodeGenCXX/implicit-copy-constructor.cpp
  test/CodeGenCXX/pod-member-memcpys.cpp
  test/CodeGenCXX/wasm-args-returns.cpp
  test/CodeGenObjCXX/implicit-copy-constructor.mm

Index: test/CodeGenObjCXX/implicit-copy-constructor.mm
===
--- test/CodeGenObjCXX/implicit-copy-constructor.mm
+++ test/CodeGenObjCXX/implicit-copy-constructor.mm
@@ -41,7 +41,7 @@
   D d2(d);
 }
 
-// CHECK-LABEL: define linkonce_odr void @_ZN1DC1ERS_(%struct.D* %this, %struct.D* dereferenceable({{[0-9]+}})) unnamed_addr
+// CHECK-LABEL: define linkonce_odr void @_ZN1DC1ERS_(%struct.D* %this, %struct.D* noalias dereferenceable({{[0-9]+}})) unnamed_addr
 // CHECK: call void @_ZN1AC1Ev
 // CHECK: call void @_ZN1CC2ERS_1A
 // CHECK: call void @_ZN1AD1Ev
Index: test/CodeGenCXX/wasm-args-returns.cpp
===
--- test/CodeGenCXX/wasm-args-returns.cpp
+++ test/CodeGenCXX/wasm-args-returns.cpp
@@ -44,7 +44,7 @@
 test(copy_ctor);
 // CHECK: define void @_Z7forward9copy_ctor(%struct.copy_ctor* noalias sret %{{.*}}, %struct.copy_ctor* %{{.*}})
 //
-// CHECK: declare %struct.copy_ctor* @_ZN9copy_ctorC1ERKS_(%struct.copy_ctor* returned, %struct.copy_ctor* dereferenceable(8))
+// CHECK: declare %struct.copy_ctor* @_ZN9copy_ctorC1ERKS_(%struct.copy_ctor* returned, %struct.copy_ctor* noalias dereferenceable(8))
 //
 // CHECK: define void @_Z14test_copy_ctorv()
 // CHECK: %[[tmp:.*]] = alloca %struct.copy_ctor, align 8
@@ -62,7 +62,7 @@
 test(aligned_copy_ctor);
 // CHECK: define void @_Z7forward17aligned_copy_ctor(%struct.aligned_copy_ctor* noalias sret %{{.*}}, %struct.aligned_copy_ctor* %{{.*}})
 //
-// CHECK: declare %struct.aligned_copy_ctor* @_ZN17aligned_copy_ctorC1ERKS_(%struct.aligned_copy_ctor* returned, %struct.aligned_copy_ctor* dereferenceable(16))
+// CHECK: declare %struct.aligned_copy_ctor* @_ZN17aligned_copy_ctorC1ERKS_(%struct.aligned_copy_ctor* returned, %struct.aligned_copy_ctor* noalias dereferenceable(16))
 //
 // CHECK: define void @_Z22test_aligned_copy_ctorv()
 // CHECK: %[[tmp:.*]] = alloca %struct.aligned_copy_ctor, align 16
Index: test/CodeGenCXX/pod-member-memcpys.cpp
===
--- test/CodeGenCXX/pod-member-memcpys.cpp
+++ test/CodeGenCXX/pod-member-memcpys.cpp
@@ -181,51 +181,51 @@
 
 CALL_CC(PackedMembers)
 // PackedMembers copy-assignment:
-// CHECK-LABEL: define linkonce_odr void @_ZN13PackedMembersC2ERKS_(%struct.PackedMembers* %this, %struct.PackedMembers* dereferenceable({{[0-9]+}}))
+// CHECK-LABEL: define linkonce_odr void @_ZN13PackedMembersC2ERKS_(%struct.PackedMembers* %this, %struct.PackedMembers* noalias dereferenceable({{[0-9]+}}))
 // CHECK: call void @_ZN6NonPODC1ERKS_
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 1 {{.*}} align 1 {{.*}}i64 16, i1 {{.*}})
 // CHECK: ret void
 
 CALL_CC(BitfieldMember2)
 // BitfieldMember2 copy-constructor:
-// CHECK-2-LABEL: define linkonce_odr void @_ZN15BitfieldMember2C2ERKS_(%struct.BitfieldMember2* %this, %struct.BitfieldMember2* dereferenceable({{[0-9]+}}))
+// CHECK-2-LABEL: define linkonce_odr void @_ZN15BitfieldMember2C2ERKS_(%struct.BitfieldMember2* %this, %struct.BitfieldMember2* noalias dereferenceable({{[0-9]+}}))
 // CHECK-2: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 {{.*}}i64 16, i1 false)
 // CHECK-2: call void @_ZN6NonPODC1ERKS_
 // CHECK-2: ret void
 
 CALL_CC(BitfieldMember3)
 // BitfieldMember3 copy-constructor:
-// CHECK-LABEL: define linkonce_odr void @_ZN15BitfieldMember3C2ERKS_(%struct.BitfieldMember3* %this, %struct.BitfieldMember3* dereferenceable({{[0-9]+}}))
+// CHECK-LABEL: define linkonce_odr void @_ZN15BitfieldMember3C2ERKS_(%struct.BitfieldMember3* %this, %struct.BitfieldMember3* noalias dereferenceable({{[0-9]+}}))
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 8 {{.*}} align 8 {{.*}}i64 8, i1 false)
 // CHECK: ret void
 
 CALL_CC(ReferenceMember)
 // ReferenceMember copy-constructor:
-// CHECK-LABEL: define linkonce_odr void 

LLVM buildmaster will be restarted tonight

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster will be restarted after 6PM Pacific time today.

Thanks

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

[PATCH] D46155: Add warning flag -Wordered-compare-function-pointers.

[Eli Friedman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331570: Add warning flag 
-Wordered-compare-function-pointers. (authored by efriedma, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D46155

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Misc/warning-flags.c


Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (77):
+CHECK: Warnings without flags (76):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -31,7 +31,6 @@
 CHECK-NEXT:   ext_template_arg_extra_parens
 CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
-CHECK-NEXT:   ext_typecheck_ordered_comparison_of_function_pointers
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
 CHECK-NEXT:   pp_invalid_string_literal
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5932,7 +5932,8 @@
 def err_typecheck_ordered_comparison_of_pointer_and_zero : Error<
   "ordered comparison between pointer and zero (%0 and %1)">;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
-  "ordered comparison of function pointers (%0 and %1)">;
+  "ordered comparison of function pointers (%0 and %1)">,
+  InGroup>;
 def ext_typecheck_comparison_of_fptr_to_void : Extension<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def err_typecheck_comparison_of_fptr_to_void : Error<


Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (77):
+CHECK: Warnings without flags (76):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -31,7 +31,6 @@
 CHECK-NEXT:   ext_template_arg_extra_parens
 CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
-CHECK-NEXT:   ext_typecheck_ordered_comparison_of_function_pointers
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
 CHECK-NEXT:   pp_invalid_string_literal
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5932,7 +5932,8 @@
 def err_typecheck_ordered_comparison_of_pointer_and_zero : Error<
   "ordered comparison between pointer and zero (%0 and %1)">;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
-  "ordered comparison of function pointers (%0 and %1)">;
+  "ordered comparison of function pointers (%0 and %1)">,
+  InGroup>;
 def ext_typecheck_comparison_of_fptr_to_void : Extension<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def err_typecheck_comparison_of_fptr_to_void : Error<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331570 - Add warning flag -Wordered-compare-function-pointers.

[Eli Friedman via cfe-commits]

Author: efriedma
Date: Fri May  4 17:09:51 2018
New Revision: 331570

URL: http://llvm.org/viewvc/llvm-project?rev=331570=rev
Log:
Add warning flag -Wordered-compare-function-pointers.

The C standard doesn't allow comparisons like "f1 < f2" (where f1 and f2
are function pointers), but we allow them as an extension.  Add a
warning flag to control this warning.

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


Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/test/Misc/warning-flags.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=331570=331569=331570=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri May  4 17:09:51 
2018
@@ -5932,7 +5932,8 @@ def ext_typecheck_ordered_comparison_of_
 def err_typecheck_ordered_comparison_of_pointer_and_zero : Error<
   "ordered comparison between pointer and zero (%0 and %1)">;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
-  "ordered comparison of function pointers (%0 and %1)">;
+  "ordered comparison of function pointers (%0 and %1)">,
+  InGroup>;
 def ext_typecheck_comparison_of_fptr_to_void : Extension<
   "equality comparison between function pointer and void pointer (%0 and %1)">;
 def err_typecheck_comparison_of_fptr_to_void : Error<

Modified: cfe/trunk/test/Misc/warning-flags.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/warning-flags.c?rev=331570=331569=331570=diff
==
--- cfe/trunk/test/Misc/warning-flags.c (original)
+++ cfe/trunk/test/Misc/warning-flags.c Fri May  4 17:09:51 2018
@@ -18,7 +18,7 @@ This test serves two purposes:
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (77):
+CHECK: Warnings without flags (76):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -31,7 +31,6 @@ CHECK-NEXT:   ext_plain_complex
 CHECK-NEXT:   ext_template_arg_extra_parens
 CHECK-NEXT:   ext_typecheck_comparison_of_pointer_integer
 CHECK-NEXT:   ext_typecheck_cond_incompatible_operands
-CHECK-NEXT:   ext_typecheck_ordered_comparison_of_function_pointers
 CHECK-NEXT:   ext_typecheck_ordered_comparison_of_pointer_integer
 CHECK-NEXT:   ext_using_undefined_std
 CHECK-NEXT:   pp_invalid_string_literal


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

[PATCH] D44934: [analyzer] Improve the modeling of `memset()`.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm, ok, it seems that i've just changed the API in 
https://reviews.llvm.org/D46368, and i should have thought about this use case. 
Well, at least i have some background understanding of these problems now. 
Sorry for not keeping eye on this problem.

In https://reviews.llvm.org/D44934#1051002, @NoQ wrote:

> Why do you need separate code for null and non-null character? The function's 
> semantics doesn't seem to care.


I guess i can answer myself here:

  int32_t x;
  memset(, 1, sizeof(int32_t));
  clang_analyzer_eval(x == 0x1010101); // should be TRUE

I really doubt that we support this case.

So, yeah, zero character is indeed special.

And since zero character is special, i guess we could use the new 
`bindDefaultZero()` API, and perform a simple invalidation in the non-zero 
character case. @MTC, would this be enough for your use case? Or is it really 
important for you to support the non-zero character case? Cause my example is 
fairly artificial, and probably i'm worrying too much about it. If nobody 
really codes that way, i'm fine with supporting the non-zero character case via 
a simple default binding. In this case we should merge `bindDefaultZero()` with 
your `overwriteRegion()` (i'd prefer your function name, but please keep the 
empty class check).




Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:148
+  ProgramStateRef NewState = makeWithStore(NewStore);
+  return Mgr.getOwningEngine()
+ ? Mgr.getOwningEngine()->processRegionChange(NewState, R, LCtx)

MTC wrote:
> a.sidorin wrote:
> > Do clients of `overwriteRegion` really need to call checkers?
> It is possible that my understanding of the engine is not enough, I think we 
> need to call `processRangeChange()` for memory change. `memset()` will 
> overwrite the memory region, so I think there is a need to call this API.
> 
> What do you think?
Well, we need to make sure we don't notify checkers recursively. In `bindLoc()` 
we have the `notifyChanges` parameter, i guess we need to have something 
similar here as well, if we really need to notify checkers.

Technically, yeah, we need to notify checkers. Like, if you memset() a string 
to `'\0'`, string checker needs to know that its length has also become 0. 
Wait, we already are in the string checker, and we're already doing that. I 
guess it's not that important at the moment, so i'd rather delay it until we 
need it, and see if we have shared checker states available earlier and if they 
help.

Also `getOwningEngine()` is never null.



Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:118
   const LocationContext *LCtx,
   bool notifyChanges) const {
   ProgramStateManager  = getStateManager();

Note: `notifyChanges` declared here :)


Repository:
  rC Clang

https://reviews.llvm.org/D44934



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

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

> In this sense bug reports against abandoned alpha checkers (which are, 
> unfortunately, the majority) aren't very useful. In most cases it's just too 
> easy to see how broken they are.

Although the majority are like that, not all of them are. Some like the 
Conversion checker doesn't seem to have any issues(because its fairly limited 
at this point). Others like StreamChecker probably just needs some simple 
fixes. Maybe we can move the alpha checks that have lots of issues and no 
contributions for awhile to another category such as abandoned.

> But if you are interested in a particular checker and want to work on it to 
> make sure it's stable, we'd be glad to help, so please contact us on the 
> mailing list.

I am definitely interested in working on getting some of the checkers stable, 
and have already started some work on expanding the ConversionChecker, but 
would like user feedback as I move forward.


https://reviews.llvm.org/D46159



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc accepted this revision.
pcc added a comment.

LGTM


https://reviews.llvm.org/D46403



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D46403#1088759, @pcc wrote:

> We should be installing `compiler-rt/lib/cfi/cfi_blacklist.txt`, no?


Oh, I see. This is already taken care of, then.

@cmtice This looks fine to me, but please wait for another +1 from someone more 
familiar with cfi.


https://reviews.llvm.org/D46403



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

We should be installing `compiler-rt/lib/cfi/cfi_blacklist.txt`, no?


https://reviews.llvm.org/D46403



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Caroline Tice via Phabricator via cfe-commits]

cmtice added a comment.

vsk:  Are you asking me to put together a cfi blacklist to ship in the resource 
directory in the default install as part of this code review?  Or is that 
something you want to see in a different code reivew?


https://reviews.llvm.org/D46403



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGExprAgg.cpp:971
+  auto EmitCmpRes = [&](const VarDecl *VD) {
+return CGF.CGM.GetAddrOfGlobalVar(VD);
+  };

rsmith wrote:
> Perhaps directly emit the constant value here rather than the address of the 
> global? I think we should consider what IR we want to see coming out of 
> Clang, and I don't think that IR should contain loads from globals to get the 
> small constant integer that is the value of the conversion result.
> 
> I think it would be reasonable for us to say that we require the standard 
> library types to contain exactly one non-static data member of integral type, 
> and for us to form a select between the relevant integer values here. We 
> really have no need to support all possible implementations of these types, 
> and we can revisit this if some other standard library implementation ships 
> types that don't follow that pattern. (If we find such a standard library, we 
> could emit multiple selects, or a first-class aggregate select, or whatever 
> generates the best code at -O0.)
I agree emitting the value would be better, and that most STL implementations 
should implement the types using only one non-static member.
However, note that the specification for `partial_ordering` is described in 
terms of two non-static data members, so it seems possible an STL 
implementation might implement in that way.

Would it be appropriate to do this as a smaller follow up patch?


https://reviews.llvm.org/D45476



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

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D46464



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

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Teresa Johnson via Phabricator via cfe-commits]

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

Fixed the output file as suggested. Also note that this test will fail and 
can't go in until after https://reviews.llvm.org/D46387


Repository:
  rC Clang

https://reviews.llvm.org/D46464



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

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

rsmith added inline comments.



Comment at: lib/AST/ExprConstant.cpp:8829
+  return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+  });

It'd be clearer to call `VisitBinCmp` rather than `VisitBinaryOperator`.



Comment at: lib/CodeGen/CGExprAgg.cpp:971
+  auto EmitCmpRes = [&](const VarDecl *VD) {
+return CGF.CGM.GetAddrOfGlobalVar(VD);
+  };

Perhaps directly emit the constant value here rather than the address of the 
global? I think we should consider what IR we want to see coming out of Clang, 
and I don't think that IR should contain loads from globals to get the small 
constant integer that is the value of the conversion result.

I think it would be reasonable for us to say that we require the standard 
library types to contain exactly one non-static data member of integral type, 
and for us to form a select between the relevant integer values here. We really 
have no need to support all possible implementations of these types, and we can 
revisit this if some other standard library implementation ships types that 
don't follow that pattern. (If we find such a standard library, we could emit 
multiple selects, or a first-class aggregate select, or whatever generates the 
best code at -O0.)


https://reviews.llvm.org/D45476



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

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 145321.
tejohnson added a comment.

Address comments


Repository:
  rC Clang

https://reviews.llvm.org/D46464

Files:
  lib/CodeGen/BackendUtil.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll


Index: test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
===
--- /dev/null
+++ test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
@@ -0,0 +1,47 @@
+; Test to ensure -fdiagnostics-show-hotness and -fsave-optimization-record
+; work when invoking the ThinLTO backend path.
+; RUN: opt -module-summary -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+
+; First try with pass remarks to stderr
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm 
-pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+
+; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 
300)
+
+; Next try YAML pass remarks file
+; RUN: rm -f %t2.opt.yaml
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc 
-fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml | FileCheck %s -check-prefix=YAML
+
+; YAML: --- !Passed
+; YAML-NEXT: Pass:inline
+; YAML-NEXT: Name:Inlined
+; YAML-NEXT: Function:main
+; YAML-NEXT: Hotness: 300
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee:  tinkywinky
+; YAML-NEXT:   - String:  ' inlined into '
+; YAML-NEXT:   - Caller:  main
+; YAML-NEXT:   - String:  ' with cost='
+; YAML-NEXT:   - Cost:'0'
+; YAML-NEXT:   - String:  ' (threshold='
+; YAML-NEXT:   - Threshold:   '337'
+; YAML-NEXT:   - String:  ')'
+; YAML-NEXT: ...
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+declare i32 @patatino()
+
+define i32 @tinkywinky() {
+  %a = call i32 @patatino()
+  ret i32 %a
+}
+
+define i32 @main() !prof !0 {
+  %i = call i32 @tinkywinky()
+  ret i32 %i
+}
+
+!0 = !{!"function_entry_count", i64 300}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1075,7 +1075,9 @@
   bool UsingProfile = UsingSampleProfile ||
   (Opts.getProfileUse() != CodeGenOptions::ProfileNone);
 
-  if (Opts.DiagnosticsWithHotness && !UsingProfile)
+  if (Opts.DiagnosticsWithHotness && !UsingProfile &&
+  // An IR file will contain PGO as metadata
+  IK.getLanguage() != InputKind::LLVM_IR)
 Diags.Report(diag::warn_drv_diagnostics_hotness_requires_pgo)
 << "-fdiagnostics-show-hotness";
 
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -1136,6 +1136,8 @@
   Conf.SampleProfile = std::move(SampleProfile);
   Conf.UseNewPM = CGOpts.ExperimentalNewPassManager;
   Conf.DebugPassManager = CGOpts.DebugPassManager;
+  Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
+  Conf.RemarksFilename = CGOpts.OptRecordFile;
   switch (Action) {
   case Backend_EmitNothing:
 Conf.PreCodeGenModuleHook = [](size_t Task, const Module ) {
@@ -1159,7 +1161,7 @@
 break;
   }
   if (Error E = thinBackend(
-  Conf, 0, AddStream, *M, *CombinedIndex, ImportList,
+  Conf, -1, AddStream, *M, *CombinedIndex, ImportList,
   ModuleToDefinedGVSummaries[M->getModuleIdentifier()], ModuleMap)) {
 handleAllErrors(std::move(E), [&](ErrorInfoBase ) {
   errs() << "Error running ThinLTO backend: " << EIB.message() << '\n';


Index: test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
===
--- /dev/null
+++ test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
@@ -0,0 +1,47 @@
+; Test to ensure -fdiagnostics-show-hotness and -fsave-optimization-record
+; work when invoking the ThinLTO backend path.
+; RUN: opt -module-summary -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+
+; First try with pass remarks to stderr
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+
+; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 300)
+
+; Next try YAML pass remarks file
+; RUN: rm -f %t2.opt.yaml
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml | FileCheck %s -check-prefix=YAML
+
+; YAML: --- !Passed
+; YAML-NEXT: Pass:inline
+; YAML-NEXT: Name:Inlined
+; YAML-NEXT: Function:main
+; YAML-NEXT: Hotness: 300
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee:  tinkywinky
+; YAML-NEXT:   - String:

[PATCH] D46187: [Analyzer] getRegisteredCheckers(): handle debug checkers too.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

It seems that you're using debug checkers of the analyzer to gain some free 
tools for exploring the source code (such as displaying a call graph) for free, 
right?

I believe we could also benefit from a method of extracting the analyzer's 
`clang -cc1` run-line from clang-tidy. This would allow arbitrary debugging 
over a single analyzer invocation.


Repository:
  rC Clang

https://reviews.llvm.org/D46187



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

[PATCH] D46176: Add SourceManagerForFile helper which sets up SourceManager and dependencies for a single file with code snippet

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Thanks for the comment and suggestion!

I've changed this to `SourceManagerForFile` and move it to SourceManager.h, 
which seems to be a more natural fit.  `SourceManagerForFile` sounds like a 
sensible name to me, but I'm guess there might be a better name...


Repository:
  rC Clang

https://reviews.llvm.org/D46176



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

[PATCH] D46176: Pull helper class Environment from clangFormat into libToolingCore [NFC]

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 145318.
ioeric added a comment.

- Create SourceManagerForFile instead of Environment; put it in SourceManager.h 
which seems to be a better place.


Repository:
  rC Clang

https://reviews.llvm.org/D46176

Files:
  include/clang/Basic/SourceManager.h
  lib/Basic/SourceManager.cpp
  lib/Format/Format.cpp
  lib/Format/SortJavaScriptImports.cpp
  lib/Format/TokenAnalyzer.cpp
  lib/Format/TokenAnalyzer.h

Index: lib/Format/TokenAnalyzer.h
===
--- lib/Format/TokenAnalyzer.h
+++ lib/Format/TokenAnalyzer.h
@@ -37,44 +37,24 @@
 class Environment {
 public:
   Environment(SourceManager , FileID ID, ArrayRef Ranges)
-  : ID(ID), CharRanges(Ranges.begin(), Ranges.end()), SM(SM),
-  FirstStartColumn(0),
-  NextStartColumn(0),
-  LastStartColumn(0) {}
-
-  Environment(FileID ID, std::unique_ptr FileMgr,
-  std::unique_ptr VirtualSM,
-  std::unique_ptr Diagnostics,
-  const std::vector ,
-  unsigned FirstStartColumn,
-  unsigned NextStartColumn,
-  unsigned LastStartColumn)
-  : ID(ID), CharRanges(CharRanges.begin(), CharRanges.end()),
-SM(*VirtualSM), 
-FirstStartColumn(FirstStartColumn),
-NextStartColumn(NextStartColumn),
-LastStartColumn(LastStartColumn),
-FileMgr(std::move(FileMgr)),
-VirtualSM(std::move(VirtualSM)), Diagnostics(std::move(Diagnostics)) {}
+  : SM(SM), ID(ID), CharRanges(Ranges.begin(), Ranges.end()),
+FirstStartColumn(0), NextStartColumn(0), LastStartColumn(0) {}
 
   // This sets up an virtual file system with file \p FileName containing the
   // fragment \p Code. Assumes that \p Code starts at \p FirstStartColumn,
   // that the next lines of \p Code should start at \p NextStartColumn, and
   // that \p Code should end at \p LastStartColumn if it ends in newline.
   // See also the documentation of clang::format::internal::reformat.
-  static std::unique_ptr
-  CreateVirtualEnvironment(StringRef Code, StringRef FileName,
-   ArrayRef Ranges,
-   unsigned FirstStartColumn = 0,
-   unsigned NextStartColumn = 0,
-   unsigned LastStartColumn = 0);
+  Environment(StringRef Code, StringRef FileName,
+  ArrayRef Ranges, unsigned FirstStartColumn = 0,
+  unsigned NextStartColumn = 0, unsigned LastStartColumn = 0);
 
   FileID getFileID() const { return ID; }
 
-  ArrayRef getCharRanges() const { return CharRanges; }
-
   const SourceManager () const { return SM; }
 
+  ArrayRef getCharRanges() const { return CharRanges; }
+
   // Returns the column at which the fragment of code managed by this
   // environment starts.
   unsigned getFirstStartColumn() const { return FirstStartColumn; }
@@ -88,19 +68,17 @@
   unsigned getLastStartColumn() const { return LastStartColumn; }
 
 private:
+  std::unique_ptr VirtualSM;
+
+  // This refers to either a SourceManager provided by users or VirtualSM
+  // created for a single file.
+  SourceManager 
   FileID ID;
+
   SmallVector CharRanges;
-  SourceManager 
   unsigned FirstStartColumn;
   unsigned NextStartColumn;
   unsigned LastStartColumn;
-
-  // The order of these fields are important - they should be in the same order
-  // as they are created in `CreateVirtualEnvironment` so that they can be
-  // deleted in the reverse order as they are created.
-  std::unique_ptr FileMgr;
-  std::unique_ptr VirtualSM;
-  std::unique_ptr Diagnostics;
 };
 
 class TokenAnalyzer : public UnwrappedLineConsumer {
Index: lib/Format/TokenAnalyzer.cpp
===
--- lib/Format/TokenAnalyzer.cpp
+++ lib/Format/TokenAnalyzer.cpp
@@ -34,48 +34,21 @@
 namespace clang {
 namespace format {
 
-// This sets up an virtual file system with file \p FileName containing \p
-// Code.
-std::unique_ptr
-Environment::CreateVirtualEnvironment(StringRef Code, StringRef FileName,
-  ArrayRef Ranges,
-  unsigned FirstStartColumn,
-  unsigned NextStartColumn,
-  unsigned LastStartColumn) {
-  // This is referenced by `FileMgr` and will be released by `FileMgr` when it
-  // is deleted.
-  IntrusiveRefCntPtr InMemoryFileSystem(
-  new vfs::InMemoryFileSystem);
-  // This is passed to `SM` as reference, so the pointer has to be referenced
-  // in `Environment` so that `FileMgr` can out-live this function scope.
-  std::unique_ptr FileMgr(
-  new FileManager(FileSystemOptions(), InMemoryFileSystem));
-  // This is passed to `SM` as reference, so the pointer has to be referenced
-  // by `Environment` due to the same reason above.
-  std::unique_ptr Diagnostics(new DiagnosticsEngine(
-  

r331565 - [analyzer] Remove untested code in evalLoad.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri May  4 16:01:10 2018
New Revision: 331565

URL: http://llvm.org/viewvc/llvm-project?rev=331565=rev
Log:
[analyzer] Remove untested code in evalLoad.

No functional change intended.

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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=331565=331564=331565=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri May  4 16:01:10 2018
@@ -2897,43 +2897,6 @@ void ExprEngine::evalLoad(ExplodedNodeSe
   const ProgramPointTag *tag,
   QualType LoadTy) {
   assert(!location.getAs() && "location cannot be a NonLoc.");
-
-  // Are we loading from a region?  This actually results in two loads; one
-  // to fetch the address of the referenced value and one to fetch the
-  // referenced value.
-  if (const auto *TR =
-dyn_cast_or_null(location.getAsRegion())) {
-
-QualType ValTy = TR->getValueType();
-if (const ReferenceType *RT = ValTy->getAs()) {
-  static SimpleProgramPointTag
- loadReferenceTag(TagProviderName, "Load Reference");
-  ExplodedNodeSet Tmp;
-  evalLoadCommon(Tmp, NodeEx, BoundEx, Pred, state,
- location, ,
- getContext().getPointerType(RT->getPointeeType()));
-
-  // Perform the load from the referenced value.
-  for (const auto I : Tmp) {
-state = I->getState();
-location = state->getSVal(BoundEx, I->getLocationContext());
-evalLoadCommon(Dst, NodeEx, BoundEx, I, state, location, tag, LoadTy);
-  }
-  return;
-}
-  }
-
-  evalLoadCommon(Dst, NodeEx, BoundEx, Pred, state, location, tag, LoadTy);
-}
-
-void ExprEngine::evalLoadCommon(ExplodedNodeSet ,
-const Expr *NodeEx,
-const Expr *BoundEx,
-ExplodedNode *Pred,
-ProgramStateRef state,
-SVal location,
-const ProgramPointTag *tag,
-QualType LoadTy) {
   assert(NodeEx);
   assert(BoundEx);
   // Evaluate the location (checks for bad dereferences).


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

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

[Shuai Wang via Phabricator via cfe-commits]

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

I've reverted the handling of DeclRefExpr to volatile after rethinking about it.

The purpose of this analyzer is to find whether the given Expr is mutated 
within a scope, but doesn't really care about external changes. In other words 
we're trying to find mutations explicitly written within the specified scope.




Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88
+
+const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
+  // LHS of any assignment operators.

aaron.ballman wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > shuaiwang wrote:
> > > > aaron.ballman wrote:
> > > > > Should this also consider a DeclRefExpr to a volatile-qualified 
> > > > > variable as a direct mutation?
> > > > > 
> > > > > What about using `Expr::HasSideEffect()`?
> > > > Good catch about DeclRefExpr to volatile.
> > > > 
> > > > `HasSideEffects` means something different. Here find*Mutation means 
> > > > find a Stmt **in ancestors** that mutates the given Expr. 
> > > > `HasSideEffects` IIUC means whether anything is mutated by the Expr but 
> > > > doesn't care about what exactly is mutated.
> > > May I ask the exact semantics for `volatile`? I would add the test to my 
> > > check, too.
> > > 
> > > It is possible to write `const volatile int m = 42;`, but if i understand 
> > > correctly `m` could still change by hardware, or would this be UB?
> > > If the `const` is missing, one must always assume external modification, 
> > > right?
> > > HasSideEffects means something different. Here find*Mutation means find a 
> > > Stmt in ancestors that mutates the given Expr. HasSideEffects IIUC means 
> > > whether anything is mutated by the Expr but doesn't care about what 
> > > exactly is mutated.
> > 
> > What I am getting at is that the code in `findDirectMutation()` seems to go 
> > through a lot of effort looking for expressions that have side effects and 
> > I was wondering if we could leverage that call instead of duplicating the 
> > effort.
> > May I ask the exact semantics for volatile? I would add the test to my 
> > check, too.
> 
> Basically, volatile objects can change in ways unknown to the implementation, 
> so a volatile read must always perform an actual read (because the object's 
> value may have changed since the last read) and a volatile write must always 
> be performed as well (because writing a value may have further side effects 
> if the object is backed by some piece of interesting hardware).
> 
> > It is possible to write const volatile int m = 42;, but if i understand 
> > correctly m could still change by hardware, or would this be UB?
> 
> That could still be changed by hardware, and it would not be UB. For 
> instance, the variable might be referencing some hardware sensor and so you 
> can only read the values in (hence it's const) but the values may be changed 
> out from under you (hence, it's not constant).
> 
> > If the const is missing, one must always assume external modification, 
> > right?
> 
> You have to assume external modification regardless.
Unfortunately I don't think HasSideEffects help in here. We're finding one 
particular side effect that is mutating the specified Expr so we need to have 
explicitly `equalsNode(Exp)` for all the matches. `HasSideEffects` can simply 
claim `f(a, b)` or even `g()` has side effects (given `void f(Foo&, const 
Bar&)`, `void g()`) while we need to require a link to the specific Expr we're 
analyzing (i.e. `f(a, b)` is mutating `a` but not `b`)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679



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

[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang updated this revision to Diff 145313.
shuaiwang added a comment.

Revert "DeclRefExpr to volatile handling" part of last update.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45679

Files:
  clang-tidy/utils/CMakeLists.txt
  clang-tidy/utils/ExprMutationAnalyzer.cpp
  clang-tidy/utils/ExprMutationAnalyzer.h
  unittests/clang-tidy/CMakeLists.txt
  unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -0,0 +1,554 @@
+//===-- ExprMutationAnalyzerTest.cpp - clang-tidy -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../clang-tidy/utils/ExprMutationAnalyzer.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+using namespace clang::ast_matchers;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::StartsWith;
+using ::testing::Values;
+
+namespace {
+
+using ExprMatcher = internal::Matcher;
+using StmtMatcher = internal::Matcher;
+
+ExprMatcher declRefTo(StringRef Name) {
+  return declRefExpr(to(namedDecl(hasName(Name;
+}
+
+StmtMatcher withEnclosingCompound(ExprMatcher Matcher) {
+  return expr(Matcher, hasAncestor(compoundStmt().bind("stmt"))).bind("expr");
+}
+
+bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  const auto *const E = selectFirst("expr", Results);
+  return utils::ExprMutationAnalyzer(S, >getASTContext()).isMutated(E);
+}
+
+SmallVector
+mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
+  const auto *const S = selectFirst("stmt", Results);
+  SmallVector Chain;
+  utils::ExprMutationAnalyzer Analyzer(S, >getASTContext());
+  for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
+const Stmt *By = Analyzer.findMutation(E);
+std::string buffer;
+llvm::raw_string_ostream stream(buffer);
+By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
+Chain.push_back(StringRef(stream.str()).trim().str());
+E = dyn_cast(By);
+  }
+  return Chain;
+}
+
+std::string removeSpace(std::string s) {
+  s.erase(std::remove_if(s.begin(), s.end(),
+ [](char c) { return std::isspace(c); }),
+  s.end());
+  return s;
+}
+
+} // namespace
+
+TEST(ExprMutationAnalyzerTest, Trivial) {
+  const auto AST = tooling::buildASTFromCode("void f() { int x; x; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
+class AssignmentTest : public ::testing::TestWithParam {};
+
+TEST_P(AssignmentTest, AssignmentModifies) {
+  const std::string ModExpr = "x " + GetParam() + " 10";
+  const auto AST =
+  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
+Values("=", "+=", "-=", "*=", "/=", "%=", "&=", "|=",
+   "^=", "<<=", ">>="), );
+
+class IncDecTest : public ::testing::TestWithParam {};
+
+TEST_P(IncDecTest, IncDecModifies) {
+  const std::string ModExpr = GetParam();
+  const auto AST =
+  tooling::buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
+Values("++x", "--x", "x++", "x--"), );
+
+TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
+  const auto AST = tooling::buildASTFromCode(
+  "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+}
+
+TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
+  const auto AST = tooling::buildASTFromCode(
+  "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }");
+  const auto Results =
+  match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll:14
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc 
-fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml.thin.0.yaml | FileCheck %s -check-prefix=YAML
+

This file isn't named correctly according to the `-foptimization-record-file` 
flag, right? Looks like the easy fix would be to pass -1 as the task identifier 
to thinBackend, but it would probably be worth looking more closely at some 
point at how we name these extra files in LTO.


Repository:
  rC Clang

https://reviews.llvm.org/D46464



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

r331563 - [analyzer] Invalidate union regions properly. Don't hesitate to load later.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri May  4 15:19:32 2018
New Revision: 331563

URL: http://llvm.org/viewvc/llvm-project?rev=331563=rev
Log:
[analyzer] Invalidate union regions properly. Don't hesitate to load later.

We weren't invalidating our unions correctly. The previous behavior in
invalidateRegionsWorker::VisitCluster() was to direct-bind an UnknownVal
to the union (at offset 0).

For that reason we were never actually loading default bindings from our unions,
because there never was any default binding to load, and the value
that is presumed when there's no default binding to load
is usually completely incorrect (eg. UndefinedVal for stack unions).

The new behavior is to default-bind a conjured symbol (of irrelevant type)
to the union that's being invalidated, similarly to what we do for structures
and classes. Then it becomes safe to load the value properly.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/test/Analysis/unions.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=331563=331562=331563=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Fri May  4 15:19:32 2018
@@ -230,11 +230,6 @@ Optional RegionBindingsRef::getDir
 }
 
 Optional RegionBindingsRef::getDefaultBinding(const MemRegion *R) const {
-  if (R->isBoundable())
-if (const TypedValueRegion *TR = dyn_cast(R))
-  if (TR->getValueType()->isUnionType())
-return UnknownVal();
-
   return Optional::create(lookup(R, BindingKey::Default));
 }
 
@@ -1099,7 +1094,7 @@ void invalidateRegionsWorker::VisitClust
 return;
   }
 
-  if (T->isStructureOrClassType()) {
+  if (T->isRecordType()) {
 // Invalidate the region by setting its default value to
 // conjured symbol. The type of the symbol is irrelevant.
 DefinedOrUnknownSVal V = svalBuilder.conjureSymbolVal(baseR, Ex, LCtx,

Modified: cfe/trunk/test/Analysis/unions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unions.cpp?rev=331563=331562=331563=diff
==
--- cfe/trunk/test/Analysis/unions.cpp (original)
+++ cfe/trunk/test/Analysis/unions.cpp Fri May  4 15:19:32 2018
@@ -79,8 +79,7 @@ namespace PR17596 {
 IntOrString vv;
 vv.i = 5;
 uu = vv;
-// FIXME: Should be true.
-clang_analyzer_eval(uu.i == 5); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(uu.i == 5); // expected-warning{{TRUE}}
   }
 
   void testInvalidation() {
@@ -106,3 +105,20 @@ namespace PR17596 {
 clang_analyzer_eval(uu.s[0] == 'a'); // expected-warning{{UNKNOWN}}
   }
 }
+
+namespace assume_union_contents {
+union U {
+  int x;
+};
+
+U get();
+
+void test() {
+  U u = get();
+  int y = 0;
+  if (u.x)
+y = 1;
+  if (u.x)
+y = 1 / y; // no-warning
+}
+} // end namespace assume_union_contents


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

[PATCH] D45241: [analyzer] Invalidate union regions properly. Don't hesitate to load the default binding later.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331563: [analyzer] Invalidate union regions properly. 
Dont hesitate to load later. (authored by dergachev, committed by ).
Herald added a subscriber: baloghadamsoftware.

Repository:
  rC Clang

https://reviews.llvm.org/D45241

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/unions.cpp


Index: test/Analysis/unions.cpp
===
--- test/Analysis/unions.cpp
+++ test/Analysis/unions.cpp
@@ -79,8 +79,7 @@
 IntOrString vv;
 vv.i = 5;
 uu = vv;
-// FIXME: Should be true.
-clang_analyzer_eval(uu.i == 5); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(uu.i == 5); // expected-warning{{TRUE}}
   }
 
   void testInvalidation() {
@@ -106,3 +105,20 @@
 clang_analyzer_eval(uu.s[0] == 'a'); // expected-warning{{UNKNOWN}}
   }
 }
+
+namespace assume_union_contents {
+union U {
+  int x;
+};
+
+U get();
+
+void test() {
+  U u = get();
+  int y = 0;
+  if (u.x)
+y = 1;
+  if (u.x)
+y = 1 / y; // no-warning
+}
+} // end namespace assume_union_contents
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -230,11 +230,6 @@
 }
 
 Optional RegionBindingsRef::getDefaultBinding(const MemRegion *R) const {
-  if (R->isBoundable())
-if (const TypedValueRegion *TR = dyn_cast(R))
-  if (TR->getValueType()->isUnionType())
-return UnknownVal();
-
   return Optional::create(lookup(R, BindingKey::Default));
 }
 
@@ -1099,7 +1094,7 @@
 return;
   }
 
-  if (T->isStructureOrClassType()) {
+  if (T->isRecordType()) {
 // Invalidate the region by setting its default value to
 // conjured symbol. The type of the symbol is irrelevant.
 DefinedOrUnknownSVal V = svalBuilder.conjureSymbolVal(baseR, Ex, LCtx,


Index: test/Analysis/unions.cpp
===
--- test/Analysis/unions.cpp
+++ test/Analysis/unions.cpp
@@ -79,8 +79,7 @@
 IntOrString vv;
 vv.i = 5;
 uu = vv;
-// FIXME: Should be true.
-clang_analyzer_eval(uu.i == 5); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(uu.i == 5); // expected-warning{{TRUE}}
   }
 
   void testInvalidation() {
@@ -106,3 +105,20 @@
 clang_analyzer_eval(uu.s[0] == 'a'); // expected-warning{{UNKNOWN}}
   }
 }
+
+namespace assume_union_contents {
+union U {
+  int x;
+};
+
+U get();
+
+void test() {
+  U u = get();
+  int y = 0;
+  if (u.x)
+y = 1;
+  if (u.x)
+y = 1 / y; // no-warning
+}
+} // end namespace assume_union_contents
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -230,11 +230,6 @@
 }
 
 Optional RegionBindingsRef::getDefaultBinding(const MemRegion *R) const {
-  if (R->isBoundable())
-if (const TypedValueRegion *TR = dyn_cast(R))
-  if (TR->getValueType()->isUnionType())
-return UnknownVal();
-
   return Optional::create(lookup(R, BindingKey::Default));
 }
 
@@ -1099,7 +1094,7 @@
 return;
   }
 
-  if (T->isStructureOrClassType()) {
+  if (T->isRecordType()) {
 // Invalidate the region by setting its default value to
 // conjured symbol. The type of the symbol is irrelevant.
 DefinedOrUnknownSVal V = svalBuilder.conjureSymbolVal(baseR, Ex, LCtx,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331562 - [analyzer] pr36458: Fix retrieved value cast for symbolic void pointers.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri May  4 15:11:12 2018
New Revision: 331562

URL: http://llvm.org/viewvc/llvm-project?rev=331562=rev
Log:
[analyzer] pr36458: Fix retrieved value cast for symbolic void pointers.

C allows us to write any bytes into any memory region. When loading weird bytes
from memory regions of known types, the analyzer is required to make sure that
the loaded value makes sense by casting it to an appropriate type.

Fix such cast for loading values that represent void pointers from non-void
pointer type places.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
cfe/trunk/test/Analysis/casts.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=331562=331561=331562=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Fri May  4 15:11:12 2018
@@ -378,6 +378,20 @@ SVal StoreManager::CastRetrievedVal(SVal
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy));
+
   return svalBuilder.dispatchCast(V, castTy);
 }
 

Modified: cfe/trunk/test/Analysis/casts.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.c?rev=331562=331561=331562=diff
==
--- cfe/trunk/test/Analysis/casts.c (original)
+++ cfe/trunk/test/Analysis/casts.c Fri May  4 15:11:12 2018
@@ -149,3 +149,25 @@ void multiDimensionalArrayPointerCasts()
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // 
expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastUIntPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned u;
+  int *x;
+  (*((int *)())) = (int)
+  *x = 1;
+  clang_analyzer_eval(u == 1); // expected-warning{{TRUE}}
+}
+
+void testCastVoidPtrToIntPtrThroughUIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)(unsigned *)getVoidPtr();
+  *x = 1; // no-crash
+}


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

[PATCH] D46415: [analyzer] pr36458: Fix retrieved value cast for symbolic void pointers.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC331562: [analyzer] pr36458: Fix retrieved value cast for 
symbolic void pointers. (authored by dergachev, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D46415

Files:
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/casts.c


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -149,3 +149,25 @@
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // 
expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastUIntPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned u;
+  int *x;
+  (*((int *)())) = (int)
+  *x = 1;
+  clang_analyzer_eval(u == 1); // expected-warning{{TRUE}}
+}
+
+void testCastVoidPtrToIntPtrThroughUIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)(unsigned *)getVoidPtr();
+  *x = 1; // no-crash
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -378,6 +378,20 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy));
+
   return svalBuilder.dispatchCast(V, castTy);
 }
 


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -149,3 +149,25 @@
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastUIntPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned u;
+  int *x;
+  (*((int *)())) = (int)
+  *x = 1;
+  clang_analyzer_eval(u == 1); // expected-warning{{TRUE}}
+}
+
+void testCastVoidPtrToIntPtrThroughUIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)(unsigned *)getVoidPtr();
+  *x = 1; // no-crash
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -378,6 +378,20 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy));
+
   return svalBuilder.dispatchCast(V, castTy);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46368: [analyzer] pr18953: Zeroing constructors, store binding extents, ASTRecordLayouts.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> We could work around that by invalidating object contents every time we 
> detect something fishy. The only reasonable detection i can come up with 
> would be to detect if we have a coinciding binding (i.e. what the assertion 
> was previously protecting us from), eg.:
> 
>   if (B.getDefaultBinding(R) || B.getDirectBinding(R))
> V = UnknownVal();
> 
> 
> This detection is imperfect - there are tests that would still fail. In 
> particular it won't fix the `ASTRecordLayout` problem. And i also suspect 
> that it really destroys a lot more good data than bad data. So i'll be 
> experimenting with that now. In the current version of the patch i remove the 
> special case handling and simply bind the default value blindly even if i 
> know that `RegionStore` can't handle it precisely.

Committed as is for now. Didn't notice any difference between the three 
approaches on my C++ testing grounds.


Repository:
  rL LLVM

https://reviews.llvm.org/D46368



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

[PATCH] D46368: [analyzer] pr18953: Zeroing constructors, store binding extents, ASTRecordLayouts.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL331561: [analyzer] pr18953: Split C++ zero-initialization 
from default initialization. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46368?vs=144937=145304#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46368

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
  cfe/trunk/test/Analysis/ctor.mm

Index: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
@@ -65,10 +65,6 @@
   return MRMgr.getElementRegion(EleTy, idx, Base, svalBuilder.getContext());
 }
 
-StoreRef StoreManager::BindDefault(Store store, const MemRegion *R, SVal V) {
-  return StoreRef(store, *this);
-}
-
 const ElementRegion *StoreManager::GetElementZeroRegion(const SubRegion *R,
 QualType T) {
   NonLoc idx = svalBuilder.makeZeroArrayIndex();
Index: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -409,8 +409,22 @@
 
   RegionBindingsRef bind(RegionBindingsConstRef B, Loc LV, SVal V);
 
-  // BindDefault is only used to initialize a region with a default value.
-  StoreRef BindDefault(Store store, const MemRegion *R, SVal V) override {
+  // BindDefaultInitial is only used to initialize a region with
+  // a default value.
+  StoreRef BindDefaultInitial(Store store, const MemRegion *R,
+  SVal V) override {
+RegionBindingsRef B = getRegionBindings(store);
+// Use other APIs when you have to wipe the region that was initialized
+// earlier.
+assert(!(B.getDefaultBinding(R) || B.getDirectBinding(R)) &&
+   "Double initialization!");
+B = B.addBinding(BindingKey::Make(R, BindingKey::Default), V);
+return StoreRef(B.asImmutableMap().getRootWithoutRetain(), *this);
+  }
+
+  // BindDefaultZero is used for zeroing constructors that may accidentally
+  // overwrite existing bindings.
+  StoreRef BindDefaultZero(Store store, const MemRegion *R) override {
 // FIXME: The offsets of empty bases can be tricky because of
 // of the so called "empty base class optimization".
 // If a base class has been optimized out
@@ -420,24 +434,14 @@
 // and trying to infer them from offsets/alignments
 // seems to be error-prone and non-trivial because of the trailing padding.
 // As a temporary mitigation we don't create bindings for empty bases.
-if (R->getKind() == MemRegion::CXXBaseObjectRegionKind &&
-cast(R)->getDecl()->isEmpty())
-  return StoreRef(store, *this);
+if (const auto *BR = dyn_cast(R))
+  if (BR->getDecl()->isEmpty())
+return StoreRef(store, *this);
 
 RegionBindingsRef B = getRegionBindings(store);
-assert(!B.lookup(R, BindingKey::Direct));
-
-BindingKey Key = BindingKey::Make(R, BindingKey::Default);
-if (B.lookup(Key)) {
-  const SubRegion *SR = cast(R);
-  assert(SR->getAsOffset().getOffset() ==
- SR->getSuperRegion()->getAsOffset().getOffset() &&
- "A default value must come from a super-region");
-  B = removeSubRegionBindings(B, SR);
-} else {
-  B = B.addBinding(Key, V);
-}
-
+SVal V = svalBuilder.makeZeroVal(Ctx.CharTy);
+B = removeSubRegionBindings(B, cast(R));
+B = B.addBinding(BindingKey::Make(R, BindingKey::Default), V);
 return StoreRef(B.asImmutableMap().getRootWithoutRetain(), *this);
   }
 
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -375,9 +375,6 @@
  I != E; ++I) {
   ProgramStateRef State = (*I)->getState();
   if (CE->requiresZeroInitialization()) {
-// Type of the zero doesn't matter.
-SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy);
-
 // FIXME: Once we properly handle constructors in new-expressions, we'll
 // need to invalidate the region before setting a default value, to make
 // sure there aren't any 

r331560 - [X86] Correct the attributes on the incssp and rdssp builtins to only have 'nothrow'

[Craig Topper via cfe-commits]

Author: ctopper
Date: Fri May  4 14:56:43 2018
New Revision: 331560

URL: http://llvm.org/viewvc/llvm-project?rev=331560=rev
Log:
[X86] Correct the attributes on the incssp and rdssp builtins to only have 
'nothrow'

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/include/clang/Basic/BuiltinsX86_64.def

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=331560=331559=331560=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Fri May  4 14:56:43 2018
@@ -656,8 +656,8 @@ TARGET_BUILTIN(__builtin_ia32_xsavec, "v
 TARGET_BUILTIN(__builtin_ia32_xsaves, "vv*ULLi", "n", "xsaves")
 
 // SHSTK
-TARGET_BUILTIN(__builtin_ia32_incsspd, "vUi", "u", "shstk")
-TARGET_BUILTIN(__builtin_ia32_rdsspd, "UiUi", "Un", "shstk")
+TARGET_BUILTIN(__builtin_ia32_incsspd, "vUi", "n", "shstk")
+TARGET_BUILTIN(__builtin_ia32_rdsspd, "UiUi", "n", "shstk")
 TARGET_BUILTIN(__builtin_ia32_saveprevssp, "v", "n", "shstk")
 TARGET_BUILTIN(__builtin_ia32_rstorssp, "vv*", "n", "shstk")
 TARGET_BUILTIN(__builtin_ia32_wrssd, "vUiv*", "n", "shstk")

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86_64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86_64.def?rev=331560=331559=331560=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86_64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86_64.def Fri May  4 14:56:43 2018
@@ -66,8 +66,8 @@ TARGET_BUILTIN(__builtin_ia32_xsaveopt64
 TARGET_BUILTIN(__builtin_ia32_xrstors64, "vv*ULLi", "n", "xsaves")
 TARGET_BUILTIN(__builtin_ia32_xsavec64, "vv*ULLi", "n", "xsavec")
 TARGET_BUILTIN(__builtin_ia32_xsaves64, "vv*ULLi", "n", "xsaves")
-TARGET_BUILTIN(__builtin_ia32_incsspq, "vULLi", "u", "shstk")
-TARGET_BUILTIN(__builtin_ia32_rdsspq, "ULLiULLi", "Un", "shstk")
+TARGET_BUILTIN(__builtin_ia32_incsspq, "vULLi", "n", "shstk")
+TARGET_BUILTIN(__builtin_ia32_rdsspq, "ULLiULLi", "n", "shstk")
 TARGET_BUILTIN(__builtin_ia32_wrssq, "vULLiv*", "n", "shstk")
 TARGET_BUILTIN(__builtin_ia32_wrussq, "vULLiv*", "n", "shstk")
 TARGET_BUILTIN(__builtin_ia32_addcarryx_u64, "UcUcULLiULLiULLi*", "n", "adx")


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

r331561 - [analyzer] pr18953: Split C++ zero-initialization from default initialization.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri May  4 14:56:51 2018
New Revision: 331561

URL: http://llvm.org/viewvc/llvm-project?rev=331561=rev
Log:
[analyzer] pr18953: Split C++ zero-initialization from default initialization.

The bindDefault() API of the ProgramState allows setting a default value
for reads from memory regions that were not preceded by writes.

It was used for implementing C++ zeroing constructors (i.e. default constructors
that boil down to setting all fields of the object to 0).

Because differences between zeroing consturctors and other forms of default
initialization have been piling up (in particular, zeroing constructors can be
called multiple times over the same object, probably even at the same offset,
requiring a careful and potentially slow cleanup of previous bindings in the
RegionStore), we split the API in two: bindDefaultInitial() for modeling
initial values and bindDefaultZero() for modeling zeroing constructors.

This fixes a few assertion failures from which the investigation originated.

The imperfect protection from both inability of the RegionStore to support
binding extents and lack of information in ASTRecordLayout has been loosened
because it's, well, imperfect, and it is unclear if it fixing more than it
was breaking.

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp
cfe/trunk/test/Analysis/ctor.mm

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=331561=331560=331561=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
Fri May  4 14:56:51 2018
@@ -242,7 +242,18 @@ public:
 
   ProgramStateRef bindLoc(SVal location, SVal V, const LocationContext *LCtx) 
const;
 
-  ProgramStateRef bindDefault(SVal loc, SVal V, const LocationContext *LCtx) 
const;
+  /// Initializes the region of memory represented by \p loc with an initial
+  /// value. Once initialized, all values loaded from any sub-regions of that
+  /// region will be equal to \p V, unless overwritten later by the program.
+  /// This method should not be used on regions that are already initialized.
+  /// If you need to indicate that memory contents have suddenly become unknown
+  /// within a certain region of memory, consider invalidateRegions().
+  ProgramStateRef bindDefaultInitial(SVal loc, SVal V,
+ const LocationContext *LCtx) const;
+
+  /// Performs C++ zero-initialization procedure on the region of memory
+  /// represented by \p loc.
+  ProgramStateRef bindDefaultZero(SVal loc, const LocationContext *LCtx) const;
 
   ProgramStateRef killBinding(Loc LV) const;
 

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h?rev=331561=331560=331561=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h Fri May  
4 14:56:51 2018
@@ -107,7 +107,15 @@ public:
   ///   by \c val bound to the location given for \c loc.
   virtual StoreRef Bind(Store store, Loc loc, SVal val) = 0;
 
-  virtual StoreRef BindDefault(Store store, const MemRegion *R, SVal V);
+  /// Return a store with the specified value bound to all sub-regions of the
+  /// region. The region must not have previous bindings. If you need to
+  /// invalidate existing bindings, consider invalidateRegions().
+  virtual StoreRef BindDefaultInitial(Store store, const MemRegion *R,
+  SVal V) = 0;
+
+  /// Return a store with in which all values within the given region are
+  /// reset to zero. This method is allowed to overwrite previous bindings.
+  virtual StoreRef BindDefaultZero(Store store, const MemRegion *R) = 0;
 
   /// \brief Create a new store with the specified binding removed.
   /// \param ST the original store, that is the basis for the new store.

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=331561=331560=331561=diff

r331559 - [X86] Fix some inconsistent formatting in the first line of our intrinsics headers.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Fri May  4 14:45:25 2018
New Revision: 331559

URL: http://llvm.org/viewvc/llvm-project?rev=331559=rev
Log:
[X86] Fix some inconsistent formatting in the first line of our intrinsics 
headers.

Some were too long and some were too short.

Modified:
cfe/trunk/lib/Headers/avx512pfintrin.h
cfe/trunk/lib/Headers/avx512vlbitalgintrin.h
cfe/trunk/lib/Headers/avx512vlcdintrin.h
cfe/trunk/lib/Headers/avx512vpopcntdqintrin.h
cfe/trunk/lib/Headers/avx512vpopcntdqvlintrin.h
cfe/trunk/lib/Headers/cetintrin.h
cfe/trunk/lib/Headers/clflushoptintrin.h
cfe/trunk/lib/Headers/pkuintrin.h
cfe/trunk/lib/Headers/xsavecintrin.h
cfe/trunk/lib/Headers/xsaveintrin.h
cfe/trunk/lib/Headers/xsaveoptintrin.h
cfe/trunk/lib/Headers/xsavesintrin.h
cfe/trunk/lib/Headers/xtestintrin.h

Modified: cfe/trunk/lib/Headers/avx512pfintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512pfintrin.h?rev=331559=331558=331559=diff
==
--- cfe/trunk/lib/Headers/avx512pfintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512pfintrin.h Fri May  4 14:45:25 2018
@@ -1,4 +1,4 @@
-/*===- avx512pfintrin.h - PF intrinsics --===
+/*===- avx512pfintrin.h - PF intrinsics ===
  *
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy

Modified: cfe/trunk/lib/Headers/avx512vlbitalgintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vlbitalgintrin.h?rev=331559=331558=331559=diff
==
--- cfe/trunk/lib/Headers/avx512vlbitalgintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512vlbitalgintrin.h Fri May  4 14:45:25 2018
@@ -1,4 +1,4 @@
-/*===- avx512vlbitalgintrin.h - BITALG intrinsics 
--===
+/*=== avx512vlbitalgintrin.h - BITALG intrinsics ---===
  *
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy

Modified: cfe/trunk/lib/Headers/avx512vlcdintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vlcdintrin.h?rev=331559=331558=331559=diff
==
--- cfe/trunk/lib/Headers/avx512vlcdintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512vlcdintrin.h Fri May  4 14:45:25 2018
@@ -1,4 +1,4 @@
-/*=== avx512vlcdintrin.h - AVX512VL and AVX512CD intrinsics 
---===
+/*=== avx512vlcdintrin.h - AVX512VL and AVX512CD intrinsics ===
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal

Modified: cfe/trunk/lib/Headers/avx512vpopcntdqintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vpopcntdqintrin.h?rev=331559=331558=331559=diff
==
--- cfe/trunk/lib/Headers/avx512vpopcntdqintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512vpopcntdqintrin.h Fri May  4 14:45:25 2018
@@ -1,5 +1,4 @@
-/*===- avx512vpopcntdqintrin.h - AVX512VPOPCNTDQ intrinsics
- *--===
+/*===- avx512vpopcntdqintrin.h - AVX512VPOPCNTDQ intrinsics-===
  *
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy

Modified: cfe/trunk/lib/Headers/avx512vpopcntdqvlintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vpopcntdqvlintrin.h?rev=331559=331558=331559=diff
==
--- cfe/trunk/lib/Headers/avx512vpopcntdqvlintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512vpopcntdqvlintrin.h Fri May  4 14:45:25 2018
@@ -1,5 +1,4 @@
-/*===- avx512vpopcntdqintrin.h - AVX512VPOPCNTDQ intrinsics
- *--===
+/*=== avx512vpopcntdqintrin.h - AVX512VPOPCNTDQ intrinsics -===
  *
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy

Modified: cfe/trunk/lib/Headers/cetintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/cetintrin.h?rev=331559=331558=331559=diff
==
--- cfe/trunk/lib/Headers/cetintrin.h (original)
+++ cfe/trunk/lib/Headers/cetintrin.h Fri May  4 14:45:25 2018
@@ -1,4 +1,4 @@
-/*=== cetintrin.h - CET intrinsic ===
+/*=== cetintrin.h - CET intrinsic --===
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal

Modified: cfe/trunk/lib/Headers/clflushoptintrin.h
URL: 

[PATCH] D46224: [analyzer] pr37209: Fix casts of glvalues to references.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> i'll most likely try to investigate it more carefully, or at least see if it 
> causes regressions on large codebases, before committing

Seemed fine, no visible change. I hope this means that i pattern-matched 
carefully enough.


Repository:
  rC Clang

https://reviews.llvm.org/D46224



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

r331558 - [analyzer] pr37209: Fix casts of glvalues to references.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri May  4 14:39:25 2018
New Revision: 331558

URL: http://llvm.org/viewvc/llvm-project?rev=331558=rev
Log:
[analyzer] pr37209: Fix casts of glvalues to references.

Many glvalue expressions aren't of their respective reference type -
they are simply glvalues of their value type.

This was causing problems when we were trying to obtain type of the original
expression while evaluating certain glvalue bit-casts.

Fixed by artificially forging a reference type to provide to the casting
procedure.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/test/Analysis/casts.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=331558=331557=331558=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Fri May  4 14:39:25 2018
@@ -257,6 +257,13 @@ ProgramStateRef ExprEngine::handleLValue
 ProgramStateRef state, const Expr* Ex, const LocationContext* LCtx,
 QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr,
 ExplodedNode* Pred) {
+  if (T->isLValueReferenceType()) {
+assert(!CastE->getType()->isLValueReferenceType());
+ExTy = getContext().getLValueReferenceType(ExTy);
+  } else if (T->isRValueReferenceType()) {
+assert(!CastE->getType()->isRValueReferenceType());
+ExTy = getContext().getRValueReferenceType(ExTy);
+  }
   // Delegate to SValBuilder to process.
   SVal OrigV = state->getSVal(Ex, LCtx);
   SVal V = svalBuilder.evalCast(OrigV, T, ExTy);

Modified: cfe/trunk/test/Analysis/casts.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/casts.cpp?rev=331558=331557=331558=diff
==
--- cfe/trunk/test/Analysis/casts.cpp (original)
+++ cfe/trunk/test/Analysis/casts.cpp Fri May  4 14:39:25 2018
@@ -21,3 +21,17 @@ void intAsBoolAsSwitchCondition(int c) {
   break;
   }
 }
+
+int *(char *p) {
+  return (int *&)*(int *)p;
+}
+bool testCastToIntPtrLValueRef(char *p, int *s) {
+  return castToIntPtrLValueRef(p) != s; // no-crash
+}
+
+int *&(char *p) {
+  return (int *&&)*(int *)p;
+}
+bool testCastToIntPtrRValueRef(char *p, int *s) {
+  return castToIntPtrRValueRef(p) != s; // no-crash
+}


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

[PATCH] D46224: [analyzer] pr37209: Fix casts of glvalues to references.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC331558: [analyzer] pr37209: Fix casts of glvalues to 
references. (authored by dergachev, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D46224

Files:
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Analysis/casts.cpp


Index: test/Analysis/casts.cpp
===
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -21,3 +21,17 @@
   break;
   }
 }
+
+int *(char *p) {
+  return (int *&)*(int *)p;
+}
+bool testCastToIntPtrLValueRef(char *p, int *s) {
+  return castToIntPtrLValueRef(p) != s; // no-crash
+}
+
+int *&(char *p) {
+  return (int *&&)*(int *)p;
+}
+bool testCastToIntPtrRValueRef(char *p, int *s) {
+  return castToIntPtrRValueRef(p) != s; // no-crash
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -257,6 +257,13 @@
 ProgramStateRef state, const Expr* Ex, const LocationContext* LCtx,
 QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr,
 ExplodedNode* Pred) {
+  if (T->isLValueReferenceType()) {
+assert(!CastE->getType()->isLValueReferenceType());
+ExTy = getContext().getLValueReferenceType(ExTy);
+  } else if (T->isRValueReferenceType()) {
+assert(!CastE->getType()->isRValueReferenceType());
+ExTy = getContext().getRValueReferenceType(ExTy);
+  }
   // Delegate to SValBuilder to process.
   SVal OrigV = state->getSVal(Ex, LCtx);
   SVal V = svalBuilder.evalCast(OrigV, T, ExTy);


Index: test/Analysis/casts.cpp
===
--- test/Analysis/casts.cpp
+++ test/Analysis/casts.cpp
@@ -21,3 +21,17 @@
   break;
   }
 }
+
+int *(char *p) {
+  return (int *&)*(int *)p;
+}
+bool testCastToIntPtrLValueRef(char *p, int *s) {
+  return castToIntPtrLValueRef(p) != s; // no-crash
+}
+
+int *&(char *p) {
+  return (int *&&)*(int *)p;
+}
+bool testCastToIntPtrRValueRef(char *p, int *s) {
+  return castToIntPtrRValueRef(p) != s; // no-crash
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -257,6 +257,13 @@
 ProgramStateRef state, const Expr* Ex, const LocationContext* LCtx,
 QualType T, QualType ExTy, const CastExpr* CastE, StmtNodeBuilder& Bldr,
 ExplodedNode* Pred) {
+  if (T->isLValueReferenceType()) {
+assert(!CastE->getType()->isLValueReferenceType());
+ExTy = getContext().getLValueReferenceType(ExTy);
+  } else if (T->isRValueReferenceType()) {
+assert(!CastE->getType()->isRValueReferenceType());
+ExTy = getContext().getRValueReferenceType(ExTy);
+  }
   // Delegate to SValBuilder to process.
   SVal OrigV = state->getSVal(Ex, LCtx);
   SVal V = svalBuilder.evalCast(OrigV, T, ExTy);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45174: non-zero-length bit-fields should make a class non-empty

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

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

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D45174



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

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:258-260
+Report->addNote(NoteBuf,
+PathDiagnosticLocation::create(FieldChain.getEndOfChain(),
+   
Context.getSourceManager()));

NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > Aha, ok, got it, so you're putting the warning at the end of the 
> > > constructor and squeezing all uninitialized fields into a single warning 
> > > by presenting them as notes.
> > > 
> > > Because for now notes aren't supported everywhere (and aren't always 
> > > supported really well), i guess it'd be necessary to at least provide an 
> > > option to make note-less reports before the checker is enabled by default 
> > > everywhere. In this case notes contain critical pieces of information and 
> > > as such cannot be really discarded.
> > > 
> > > I'm not sure that notes are providing a better user experience here than 
> > > simply saying that "field x.y->z is never initialized". With notes, the 
> > > user will have to scroll around (at least in scan-build) to find which 
> > > field is uninitialized.
> > > 
> > > Notes will probably also not decrease the number of reports too much 
> > > because in most cases there will be just one uninitialized field. Though 
> > > i agree that when there is more than one report, the user will be likely 
> > > to deal with all fields at once rather than separately.
> > > 
> > > So it's a bit wonky.
> > > 
> > > We might want to enable one mode or another in the checker depending on 
> > > the analyzer output flag. Probably in the driver 
> > > (`RenderAnalyzerOptions()`).
> > > 
> > > It'd be a good idea to land the checker into alpha first and fix this in 
> > > a follow-up patch because this review is already overweight.
> > > [...]i guess it'd be necessary to at least provide an option to make 
> > > note-less reports before the checker is enabled by default everywhere. In 
> > > this case notes contain critical pieces of information and as such cannot 
> > > be really discarded.
> > This can already be achieved with `-analyzer-config notes-as-events=true`. 
> > There no good reason however not to make an additional flag that would emit 
> > warnings instead of notes.
> > > I'm not sure that notes are providing a better user experience here than 
> > > simply saying that "field x.y->z is never initialized". With notes, the 
> > > user will have to scroll around (at least in scan-build) to find which 
> > > field is uninitialized.
> > This checker had a previous implementation that emitted a warning for each 
> > uninitialized field, instead of squeezing them in a single warning per 
> > constructor call. One of the major reasons behind rewriting it was that it 
> > emitted so many of them that it was difficult to differentiate which 
> > warning belonged to which constructor call. Also, in the case of the 
> > LLVM/Clang project, the warnings from this checker alone would be in the 
> > thousands.
> > > Notes will probably also not decrease the number of reports too much 
> > > because in most cases there will be just one uninitialized field.
> > While this is true to some extent, it's not uncommon that a single 
> > constructor call leaves 7 uninitialized -- in the LLVM/Clang project I 
> > found one that had over 30!
> > Since its practically impossible to determine whether this was a 
> > performance enhancement or just poor programming, the few cases where it is 
> > intentional, an object would emit many more warnings would make  make 
> > majority of the findings (where an object truly had only 1 or 2 fields 
> > uninit) a lot harder to spot in my opinion.
> > 
> > In general, I think one warning per constructor call makes the best user 
> > experience, but a temporary solution should be implemented until there's 
> > better support for notes.
> > 
> > I agree, this should be a topic of a follow-up patch.
> > 
> > This can already be achieved with `-analyzer-config notes-as-events=true`.
> 
> Yep. But the resulting user experience seems pretty bad to me. It was a good 
> quick proof-of-concept trick to test things, but the fact that notes aren't 
> path pieces is way too apparent in case of this checker. So i guess this flag 
> was a bad idea.
> 
> > Also, in the case of the LLVM/Clang project, the warnings from this checker 
> > alone would be in the thousands.
> 
> This makes me a bit worried, i wonder what's the reason why does the checker 
> shout so loudly on a codebase that doesn't seem to have actual uninitialized 
> value bugs all over the place.
> 
> Are any of these duplicates found in the same constructor for the same field 
> in different translation units? Suppose we discard the duplicates (if any) 
> and warn exactly once (across the whole LLVM) for every uninitialized field 
> (which is probably more than once per constructor). Then I wonder:
> 1. How many 

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

[Artem Dergachev via Phabricator via cfe-commits]

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

Yep, looks good. This limitation to `DeclRefExpr`s was super conservative.

I'll follow up regarding the bugprone category in a week or so on the mailing 
lists. It's been a long-standing historical decision that the Analyzer, for the 
sake of good user experience, should only warn on real bugs and have low false 
positive rates, but there definitely is a desire for more advanced users to 
opt-in into reasonable lint / coding standard checks, so we really need some 
room for them, and a way of discriminating them from checks that are simply bad 
due to large amounts of inevitable false positives.

In https://reviews.llvm.org/D46081#1079231, @whisperity wrote:

> While I understand extending the analyzer to cover more is a good approach, 
> there is `-Wconversion` which seemingly covers this -- or at least the 
> trivial case(?):


Yeah, this check historically was an attempt on a quieter and at the same time 
more powerful alternative to `-Wconversion`.


https://reviews.llvm.org/D46081



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

[PATCH] D45212: [HIP] Let CUDA toolchain support HIP language mode and amdgpu

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 145297.
yaxunl added a comment.

clean up code and separate action builder to another review.


https://reviews.llvm.org/D45212

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h

Index: lib/Driver/ToolChains/Cuda.h
===
--- lib/Driver/ToolChains/Cuda.h
+++ lib/Driver/ToolChains/Cuda.h
@@ -127,6 +127,25 @@
 };
 
 } // end namespace NVPTX
+
+namespace AMDGCN {
+// Runs llvm-link/opt/llc/lld, which links multiple LLVM bitcode, together with
+// device library, then compiles it to ISA in a shared object.
+class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
+public:
+  Linker(const ToolChain )
+  : Tool("AMDGCN::Linker", "amdgcn-link", TC, RF_Full,
+ llvm::sys::WEM_UTF8, "--options-file") {}
+
+  bool hasIntegratedCPP() const override { return false; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput) const override;
+};
+
+} // end namespace AMDGCN
 } // end namespace tools
 
 namespace toolchains {
@@ -150,9 +169,7 @@
  llvm::opt::ArgStringList ,
  Action::OffloadKind DeviceOffloadKind) const override;
 
-  // Never try to use the integrated assembler with CUDA; always fork out to
-  // ptxas.
-  bool useIntegratedAs() const override { return false; }
+  bool useIntegratedAs() const override;
   bool isCrossCompiling() const override { return true; }
   bool isPICDefault() const override { return false; }
   bool isPIEDefault() const override { return false; }
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -304,6 +304,157 @@
   return NoDebug;
 }
 
+static bool addBCLib(Compilation , const ArgList ,
+ ArgStringList , ArgStringList LibraryPaths,
+ StringRef BCName) {
+  std::string FullName;
+  bool FoundLibDevice = false;
+  for (std::string LibraryPath : LibraryPaths) {
+SmallString<128> Path(LibraryPath);
+llvm::sys::path::append(Path, BCName);
+FullName = Args.MakeArgString(Path);
+if (llvm::sys::fs::exists(FullName.c_str())) {
+  FoundLibDevice = true;
+  break;
+}
+  }
+  if (!FoundLibDevice)
+C.getDriver().Diag(diag::err_drv_no_such_file) << BCName;
+  CmdArgs.push_back(Args.MakeArgString(FullName));
+  return FoundLibDevice;
+}
+
+// For amdgcn the inputs of the linker job are device bitcode and output is
+// object file. It calls llvm-link, opt, llc, then lld steps.
+void AMDGCN::Linker::ConstructJob(Compilation , const JobAction ,
+  const InputInfo ,
+  const InputInfoList ,
+  const ArgList ,
+  const char *LinkingOutput) const {
+
+  assert(StringRef(JA.getOffloadingArch()).startswith("gfx") &&
+ " unless gfx processor, backend should be clang");
+  const auto  =
+  static_cast(getToolChain());
+  assert(TC.getTriple().getArch() == llvm::Triple::amdgcn && "Wrong platform");
+
+  // Construct llvm-link command.
+  ArgStringList CmdArgs;
+  // Add the input bc's created by compile step.
+  for (InputInfoList::const_iterator it = Inputs.begin(), ie = Inputs.end();
+   it != ie; ++it) {
+const InputInfo  = *it;
+CmdArgs.push_back(II.getFilename());
+  }
+
+  std::string GFXNAME = JA.getOffloadingArch();
+
+  ArgStringList LibraryPaths;
+
+  // Find in --hip-device-lib-path and HIP_LIBRARY_PATH.
+  for (auto Arg : Args) {
+if (Arg->getSpelling() == "--hip-device-lib-path=") {
+  LibraryPaths.push_back(Args.MakeArgString(Arg->getValue()));
+}
+  }
+
+  addDirectoryList(Args, LibraryPaths, "-L", "HIP_DEVICE_LIB_PATH");
+
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "libhiprt.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "opencl.amdgcn.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "ockl.amdgcn.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "irif.amdgcn.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "ocml.amdgcn.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "oclc_finite_only_off.amdgcn.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "oclc_daz_opt_off.amdgcn.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths,
+   "oclc_correctly_rounded_sqrt_on.amdgcn.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "oclc_unsafe_math_off.amdgcn.bc");
+  addBCLib(C, Args, CmdArgs, LibraryPaths, "hc.amdgcn.bc");
+  // Drop gfx in GFXNAME.
+  addBCLib(C, Args, CmdArgs, LibraryPaths,
+   (Twine("oclc_isa_version_") + StringRef(GFXNAME).drop_front(3) +
+".amdgcn.bc")
+   .str());
+
+  // Add an intermediate output file which 

[PATCH] D46476: [HIP] Add action builder

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

To support separate compile/link and linking across device IR in different 
source files,
a new HIP action builder is introduced. Basically it compiles/links host and 
device
code separately, and embed fat binary in host linking stage through link script.


https://reviews.llvm.org/D46476

Files:
  lib/Driver/Driver.cpp
  test/Driver/cuda-phases.cu

Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -7,195 +7,242 @@
 // REQUIRES: clang-driver
 // REQUIRES: powerpc-registered-target
 // REQUIRES: nvptx-registered-target
-
+// REQUIRES: amdgpu-registered-target
 //
 // Test single gpu architecture with complete compilation.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s 2>&1 \
-// RUN: | FileCheck -check-prefix=BIN %s
-// BIN-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (host-cuda)
-// BIN-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-cuda)
-// BIN-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, cuda-cpp-output, (device-cuda, sm_30)
-// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, sm_30)
-// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, sm_30)
-// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, sm_30)
-// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P7]]}, object
-// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P6]]}, assembler
-// BIN-DAG: [[P10:[0-9]+]]: linker, {[[P8]], [[P9]]}, cuda-fatbin, (device-cuda)
-// BIN-DAG: [[P11:[0-9]+]]: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {[[P2]]}, "device-cuda (nvptx64-nvidia-cuda)" {[[P10]]}, ir
-// BIN-DAG: [[P12:[0-9]+]]: backend, {[[P11]]}, assembler, (host-cuda)
-// BIN-DAG: [[P13:[0-9]+]]: assembler, {[[P12]]}, object, (host-cuda)
-// BIN-DAG: [[P14:[0-9]+]]: linker, {[[P13]]}, image, (host-cuda)
+// RUN: | FileCheck -check-prefixes=BIN,BIN_NV %s
+// RUN: %clang -x hip -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 %s 2>&1 \
+// RUN: | FileCheck -check-prefixes=BIN,BIN_AMD %s
+// BIN_NV-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:cuda]], (host-[[T]])
+// BIN_AMD-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:hip]], (host-[[T]])
+// BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (host-[[T]])
+// BIN-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-[[T]])
+// BIN_NV-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T]], (device-[[T]], [[ARCH:sm_30]])
+// BIN_AMD-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T]], (device-[[T]], [[ARCH:gfx803]])
+// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, [[T]]-cpp-output, (device-[[T]], [[ARCH]])
+// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-[[T]], [[ARCH]])
+// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-[[T]], [[ARCH]])
+// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-[[T]], [[ARCH]])
+// BIN_NV-DAG: [[P8:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE:nvptx64-nvidia-cuda]]:[[ARCH]])" {[[P7]]}, object
+// BIN_NV-DAG: [[P9:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE]]:[[ARCH]])" {[[P6]]}, assembler
+// BIN_NV-DAG: [[P10:[0-9]+]]: linker, {[[P8]], [[P9]]}, cuda-fatbin, (device-[[T]])
+// BIN_NV-DAG: [[P11:[0-9]+]]: offload, "host-[[T]] (powerpc64le-ibm-linux-gnu)" {[[P2]]}, "device-[[T]] ([[TRIPLE]])" {[[P10]]}, ir
+// BIN_NV-DAG: [[P12:[0-9]+]]: backend, {[[P11]]}, assembler, (host-[[T]])
+// BIN_AMD-DAG: [[P12:[0-9]+]]: backend, {[[P2]]}, assembler, (host-[[T]])
+// BIN-DAG: [[P13:[0-9]+]]: assembler, {[[P12]]}, object, (host-[[T]])
+// BIN-DAG: [[P14:[0-9]+]]: linker, {[[P13]]}, image, (host-[[T]])
+// BIN_AMD-DAG: [[P15:[0-9]+]]: linker, {[[P7]]}, image, (device-[[T]], [[ARCH]])
+// BIN_AMD-DAG: [[P16:[0-9]+]]: offload, "host-[[T]] (powerpc64le-ibm-linux-gnu)" {[[P14]]},
+// BIN_AMD-DAG-SAME:  "device-[[T]] ([[TRIPLE:amdgcn-amd-amdhsa]]:[[ARCH]])" {[[P15]]}, object
 
 //
 // Test single gpu architecture up to the assemble phase.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s -S 2>&1 \
-// RUN: | FileCheck -check-prefix=ASM %s
-// ASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// ASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (device-cuda, sm_30)
-// ASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-cuda, sm_30)
-// ASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-cuda, sm_30)
-// ASM-DAG: [[P4:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P3]]}, assembler
-// ASM-DAG: [[P5:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, 

[PATCH] D46475: [HIP] Set proper triple and offload kind for the toolchain

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

Also introduce --hip-link option to indicate HIP for linking.


https://reviews.llvm.org/D46475

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


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -537,24 +537,42 @@
   InputList ) {
 
   //
-  // CUDA
+  // CUDA/HIP
   //
   // We need to generate a CUDA toolchain if any of the inputs has a CUDA type.
-  if (llvm::any_of(Inputs, [](std::pair ) 
{
-return types::isCuda(I.first);
-  })) {
+  // ToDo: Handle mixed CUDA/HIP input files and -x hip option. Diagnose
+  //   CUDA on amdgcn and HIP on nvptx.
+  bool IsHIP =
+  (C.getInputArgs().hasArg(options::OPT_x) &&
+   StringRef(C.getInputArgs().getLastArg(options::OPT_x)->getValue()) ==
+   "hip") ||
+  C.getInputArgs().hasArg(options::OPT_hip_link);
+  if (llvm::any_of(Inputs,
+   [](std::pair ) {
+ return types::isCuda(I.first);
+   }) ||
+  IsHIP) {
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
-llvm::Triple CudaTriple(HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda"
- : "nvptx-nvidia-cuda");
-// Use the CUDA and host triples as the key into the ToolChains map, 
because
-// the device toolchain we create depends on both.
+StringRef DeviceTripleStr;
+auto OFK = IsHIP ? Action::OFK_HIP : Action::OFK_Cuda;
+if (IsHIP) {
+  // HIP is only supported on amdgcn.
+  DeviceTripleStr = "amdgcn-amd-amdhsa";
+} else {
+  // CUDA is only supported on nvptx.
+  DeviceTripleStr = HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda"
+ : "nvptx-nvidia-cuda";
+}
+llvm::Triple CudaTriple(DeviceTripleStr);
+// Use the CUDA/HIP and host triples as the key into the ToolChains map,
+// because the device toolchain we create depends on both.
 auto  = ToolChains[CudaTriple.str() + "/" + HostTriple.str()];
 if (!CudaTC) {
   CudaTC = llvm::make_unique(
-  *this, CudaTriple, *HostTC, C.getInputArgs(), Action::OFK_Cuda);
+  *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);
 }
-C.addOffloadDeviceToolChain(CudaTC.get(), Action::OFK_Cuda);
+C.addOffloadDeviceToolChain(CudaTC.get(), OFK);
   }
 
   //
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -555,6 +555,8 @@
   HelpText<"Do not include PTX for the follwing GPU architecture (e.g. sm_35) 
or 'all'. May be specified more than once.">;
 def cuda_gpu_arch_EQ : Joined<["--"], "cuda-gpu-arch=">, Flags<[DriverOption]>,
   HelpText<"CUDA GPU architecture (e.g. sm_35).  May be specified more than 
once.">;
+def hip_link : Flag<["--"], "hip-link">,
+  HelpText<"Link clang-offload-bundler bundles for HIP">;
 def no_cuda_gpu_arch_EQ : Joined<["--"], "no-cuda-gpu-arch=">, 
Flags<[DriverOption]>,
   HelpText<"Remove GPU architecture (e.g. sm_35) from the list of GPUs to 
compile for. "
"'all' resets the list to its default value.">;


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -537,24 +537,42 @@
   InputList ) {
 
   //
-  // CUDA
+  // CUDA/HIP
   //
   // We need to generate a CUDA toolchain if any of the inputs has a CUDA type.
-  if (llvm::any_of(Inputs, [](std::pair ) {
-return types::isCuda(I.first);
-  })) {
+  // ToDo: Handle mixed CUDA/HIP input files and -x hip option. Diagnose
+  //   CUDA on amdgcn and HIP on nvptx.
+  bool IsHIP =
+  (C.getInputArgs().hasArg(options::OPT_x) &&
+   StringRef(C.getInputArgs().getLastArg(options::OPT_x)->getValue()) ==
+   "hip") ||
+  C.getInputArgs().hasArg(options::OPT_hip_link);
+  if (llvm::any_of(Inputs,
+   [](std::pair ) {
+ return types::isCuda(I.first);
+   }) ||
+  IsHIP) {
 const ToolChain *HostTC = C.getSingleOffloadToolChain();
 const llvm::Triple  = HostTC->getTriple();
-llvm::Triple CudaTriple(HostTriple.isArch64Bit() ? "nvptx64-nvidia-cuda"
- : "nvptx-nvidia-cuda");
-// Use the CUDA and host triples as the key into the ToolChains map, because
-// the device toolchain we create depends on both.
+StringRef DeviceTripleStr;
+auto OFK = IsHIP ? Action::OFK_HIP : 

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D46159#1088050, @alexfh wrote:

> As folks noted here, some users prefer to use clang-tidy as a frontend for 
> the static analyzer. If this helps them test experimental CSA features and 
> CSA maintainers are willing to accept bug reports and potentially patches 
> from this category of users, I don't want to create obstacles, as long as all 
> experimental features need to be explicitly enabled.


+my imho thing: i totally don't mind that category of users :)

But just one thing - as of the current (for the last few years) balance between 
1.5 – 2 maintainers being blessed with really impressing amount of 
contributors, we really really prefer maintainance help (eg. someone who would 
be able to take responsibility of making sure a certain check is complete and 
moved out of alpha - test it thoroughly, address known issues, keep maintaining 
it after it is declared stable) versus random improvements and experiments on 
which time is being spent without any visible movement towards making the check 
actually available to the users in the nearest future.

In this sense bug reports against abandoned alpha checkers (which are, 
unfortunately, the majority) aren't very useful. In most cases it's just too 
easy to see how broken they are. But if you are interested in a particular 
checker and want to work on it to make sure it's stable, we'd be glad to help, 
so please contact us on the mailing list.

That's kinda where we are these days.


https://reviews.llvm.org/D46159



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

[PATCH] D46473: [HIP] Let clang-offload-bundler support HIP

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

When bundle/unbundle intermediate files for HIP, there may be multiple
sub archs, therefore BoundArch needs to be included in the target
and output file names for clang-offload-bundler.


https://reviews.llvm.org/D46473

Files:
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -969,11 +969,11 @@
 getOffloadKindAndTriple(Target, Kind, Triple);
 
 bool KindIsValid = !Kind.empty();
-KindIsValid = KindIsValid &&
-  StringSwitch(Kind)
-  .Case("host", true)
-  .Case("openmp", true)
-  .Default(false);
+KindIsValid = KindIsValid && StringSwitch(Kind)
+ .Case("host", true)
+ .Case("openmp", true)
+ .Case("hip", true)
+ .Default(false);
 
 bool TripleIsValid = !Triple.empty();
 llvm::Triple T(Triple);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5543,6 +5543,10 @@
 Triples += Action::GetOffloadKindName(CurKind);
 Triples += '-';
 Triples += CurTC->getTriple().normalize();
+if (CurKind == Action::OFK_HIP && CurDep->getOffloadingArch()) {
+  Triples += '-';
+  Triples += CurDep->getOffloadingArch();
+}
   }
   CmdArgs.push_back(TCArgs.MakeArgString(Triples));
 
@@ -5612,6 +5616,11 @@
 Triples += Action::GetOffloadKindName(Dep.DependentOffloadKind);
 Triples += '-';
 Triples += Dep.DependentToolChain->getTriple().normalize();
+if (Dep.DependentOffloadKind == Action::OFK_HIP &&
+!Dep.DependentBoundArch.empty()) {
+  Triples += '-';
+  Triples += Dep.DependentBoundArch;
+}
   }
 
   CmdArgs.push_back(TCArgs.MakeArgString(Triples));
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -3726,9 +3726,12 @@
   UI.DependentToolChain->getTriple().normalize(),
   /*CreatePrefixForHost=*/true);
   auto CurI = InputInfo(
-  UA, GetNamedOutputPath(C, *UA, BaseInput, UI.DependentBoundArch,
- /*AtTopLevel=*/false, MultipleArchs,
- OffloadingPrefix),
+  UA,
+  GetNamedOutputPath(C, *UA, BaseInput, UI.DependentBoundArch,
+ /*AtTopLevel=*/false,
+ MultipleArchs ||
+ UI.DependentOffloadKind == Action::OFK_HIP,
+ OffloadingPrefix),
   BaseInput);
   // Save the unbundling result.
   UnbundlingResults.push_back(CurI);


Index: tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -969,11 +969,11 @@
 getOffloadKindAndTriple(Target, Kind, Triple);
 
 bool KindIsValid = !Kind.empty();
-KindIsValid = KindIsValid &&
-  StringSwitch(Kind)
-  .Case("host", true)
-  .Case("openmp", true)
-  .Default(false);
+KindIsValid = KindIsValid && StringSwitch(Kind)
+ .Case("host", true)
+ .Case("openmp", true)
+ .Case("hip", true)
+ .Default(false);
 
 bool TripleIsValid = !Triple.empty();
 llvm::Triple T(Triple);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5543,6 +5543,10 @@
 Triples += Action::GetOffloadKindName(CurKind);
 Triples += '-';
 Triples += CurTC->getTriple().normalize();
+if (CurKind == Action::OFK_HIP && CurDep->getOffloadingArch()) {
+  Triples += '-';
+  Triples += CurDep->getOffloadingArch();
+}
   }
   CmdArgs.push_back(TCArgs.MakeArgString(Triples));
 
@@ -5612,6 +5616,11 @@
 Triples += Action::GetOffloadKindName(Dep.DependentOffloadKind);
 Triples += '-';
 Triples += Dep.DependentToolChain->getTriple().normalize();
+if (Dep.DependentOffloadKind == Action::OFK_HIP &&
+!Dep.DependentBoundArch.empty()) {
+  Triples += '-';
+  Triples += Dep.DependentBoundArch;
+}
   }
 
   

[PATCH] D46472: [HIP] Support offloading by linkscript

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

To support linking device code in different source files, it is necessary to
embed fat binary at host linking stage.

This patch emits an external symbol for fat binary in host codegen, then
embed the fat binary by lld through a link script.


https://reviews.llvm.org/D46472

Files:
  include/clang/Driver/Options.td
  lib/CodeGen/CGCUDANV.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CommonArgs.h
  lib/Driver/ToolChains/Gnu.cpp
  test/CodeGenCUDA/device-stub.cu

Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -1,22 +1,22 @@
 // RUN: echo "GPU binary would be here" > %t
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-include-gpubinary %t -o - \
-// RUN:   | FileCheck %s --check-prefixes=ALL,NORDC,CUDA
+// RUN:   | FileCheck %s --check-prefixes=ALL,NORDC,CUDA,CUDANORDC
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-include-gpubinary %t -o -  -DNOGLOBALS \
-// RUN:   | FileCheck %s -check-prefix=NOGLOBALS
+// RUN:   | FileCheck %s -check-prefixes=NOGLOBALS,CUDANOGLOBALS
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-rdc -fcuda-include-gpubinary %t -o - \
-// RUN:   | FileCheck %s --check-prefixes=ALL,RDC,CUDA
+// RUN:   | FileCheck %s --check-prefixes=ALL,RDC,CUDA,CUDARDC
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - \
 // RUN:   | FileCheck %s -check-prefix=NOGPUBIN
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-include-gpubinary %t -o - -x hip\
 // RUN:   | FileCheck %s --check-prefixes=ALL,NORDC,HIP
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-include-gpubinary %t -o -  -DNOGLOBALS -x hip \
-// RUN:   | FileCheck %s -check-prefix=NOGLOBALS
+// RUN:   | FileCheck %s -check-prefixes=NOGLOBALS,HIPNOGLOBALS
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-rdc -fcuda-include-gpubinary %t -o - -x hip \
 // RUN:   | FileCheck %s --check-prefixes=ALL,RDC,HIP
@@ -64,15 +64,17 @@
 // * constant unnamed string with the kernel name
 // ALL: private unnamed_addr constant{{.*}}kernelfunc{{.*}}\00"
 // * constant unnamed string with GPU binary
-// ALL: private unnamed_addr constant{{.*GPU binary would be here.*}}\00"
-// NORDC-SAME: section ".nv_fatbin", align 8
-// RDC-SAME: section "__nv_relfatbin", align 8
+// HIP: @[[FATBIN:__hip_fatbin]] = external constant i8, section ".hip_fatbin"
+// CUDA: @[[FATBIN:.*]] = private unnamed_addr constant{{.*GPU binary would be here.*}}\00",
+// CUDANORDC-SAME: section ".nv_fatbin", align 8
+// CUDARDC-SAME: section "__nv_relfatbin", align 8
 // * constant struct that wraps GPU binary
-// CUDA: @__[[PREFIX:cuda]]_fatbin_wrapper = internal constant
-// CUDA-SAME: { i32, i32, i8*, i8* }
-// HIP: @__[[PREFIX:hip]]_fatbin_wrapper = internal constant
-// HIP-SAME:  { i32, i32, i8*, i8* }
-// ALL-SAME: { i32 1180844977, i32 1, {{.*}}, i8* null }
+// ALL: @__[[PREFIX:cuda|hip]]_fatbin_wrapper = internal constant
+// ALL-SAME: { i32, i32, i8*, i8* }
+// ALL-SAME: { i32 1180844977, i32 1,
+// CUDA-SAME: i8* getelementptr inbounds ({{.*}}@[[FATBIN]], i64 0, i64 0),
+// HIP-SAME:  i8* @[[FATBIN]],
+// ALL-SAME: i8* null }
 // ALL-SAME: section ".nvFatBinSegment"
 // * variable to save GPU binary handle after initialization
 // NORDC: @__[[PREFIX]]_gpubin_handle = internal global i8** null
@@ -136,9 +138,10 @@
 // There should be no __[[PREFIX]]_register_globals if we have no
 // device-side globals, but we still need to register GPU binary.
 // Skip GPU binary string first.
-// NOGLOBALS: @0 = private unnamed_addr constant{{.*}}
+// CUDANOGLOBALS: @{{.*}} = private unnamed_addr constant{{.*}}
+// HIPNOGLOBALS: @{{.*}} = external constant{{.*}}
 // NOGLOBALS-NOT: define internal void @__{{.*}}_register_globals
-// NOGLOBALS: define internal void @__[[PREFIX:.*]]_module_ctor
+// NOGLOBALS: define internal void @__[[PREFIX:cuda|hip]]_module_ctor
 // NOGLOBALS: call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
 // NOGLOBALS-NOT: call void @__[[PREFIX]]_register_globals
 // NOGLOBALS: define internal void @__[[PREFIX]]_module_dtor
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -535,6 +535,10 @@
   // Add OpenMP offloading linker script args if required.
   AddOpenMPLinkerScript(getToolChain(), C, Output, Inputs, Args, CmdArgs, JA);
 
+  // Add HIP offloading linker script args if required.
+  AddHIPLinkerScript(getToolChain(), C, Output, Inputs, Args, CmdArgs, JA,
+ *this);
+
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 
Index: lib/Driver/ToolChains/CommonArgs.h

[PATCH] D46471: [HIP] Add hip offload kind

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

There are quite differences in HIP action builder and action job creation,
which justifies to define a separate offload kind.


https://reviews.llvm.org/D46471

Files:
  include/clang/Driver/Action.h
  lib/Driver/Action.cpp
  lib/Driver/Compilation.cpp
  lib/Driver/ToolChains/Clang.cpp

Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -132,6 +132,11 @@
   else if (JA.isDeviceOffloading(Action::OFK_Cuda))
 Work(*C.getSingleOffloadToolChain());
 
+  if (JA.isHostOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
+  else if (JA.isDeviceOffloading(Action::OFK_HIP))
+Work(*C.getSingleOffloadToolChain());
+
   if (JA.isHostOffloading(Action::OFK_OpenMP)) {
 auto TCs = C.getOffloadToolChains();
 for (auto II = TCs.first, IE = TCs.second; II != IE; ++II)
@@ -3099,13 +3104,14 @@
   // Check number of inputs for sanity. We need at least one input.
   assert(Inputs.size() >= 1 && "Must have at least one input.");
   const InputInfo  = Inputs[0];
-  // CUDA compilation may have multiple inputs (source file + results of
+  // CUDA/HIP compilation may have multiple inputs (source file + results of
   // device-side compilations). OpenMP device jobs also take the host IR as a
   // second input. All other jobs are expected to have exactly one
   // input.
   bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
+  bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
-  assert((IsCuda || (IsOpenMPDevice && Inputs.size() == 2) ||
+  assert((IsCuda || IsHIP || (IsOpenMPDevice && Inputs.size() == 2) ||
   Inputs.size() == 1) &&
  "Unable to handle multiple inputs.");
 
@@ -3117,10 +3123,10 @@
   bool IsWindowsMSVC = RawTriple.isWindowsMSVCEnvironment();
   bool IsIAMCU = RawTriple.isOSIAMCU();
 
-  // Adjust IsWindowsXYZ for CUDA compilations.  Even when compiling in device
-  // mode (i.e., getToolchain().getTriple() is NVPTX, not Windows), we need to
-  // pass Windows-specific flags to cc1.
-  if (IsCuda) {
+  // Adjust IsWindowsXYZ for CUDA/HIP compilations.  Even when compiling in
+  // device mode (i.e., getToolchain().getTriple() is NVPTX/AMDGCN, not
+  // Windows), we need to pass Windows-specific flags to cc1.
+  if (IsCuda || IsHIP) {
 IsWindowsMSVC |= AuxTriple && AuxTriple->isWindowsMSVCEnvironment();
 IsWindowsGNU |= AuxTriple && AuxTriple->isWindowsGNUEnvironment();
 IsWindowsCygnus |= AuxTriple && AuxTriple->isWindowsCygwinEnvironment();
@@ -3144,18 +3150,21 @@
 Args.ClaimAllArgs(options::OPT_MJ);
   }
 
-  if (IsCuda) {
-// We have to pass the triple of the host if compiling for a CUDA device and
-// vice-versa.
+  if (IsCuda || IsHIP) {
+// We have to pass the triple of the host if compiling for a CUDA/HIP device
+// and vice-versa.
 std::string NormalizedTriple;
-if (JA.isDeviceOffloading(Action::OFK_Cuda))
+if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+JA.isDeviceOffloading(Action::OFK_HIP))
   NormalizedTriple = C.getSingleOffloadToolChain()
  ->getTriple()
  .normalize();
 else
-  NormalizedTriple = C.getSingleOffloadToolChain()
- ->getTriple()
- .normalize();
+  NormalizedTriple =
+  (IsCuda ? C.getSingleOffloadToolChain()
+  : C.getSingleOffloadToolChain())
+  ->getTriple()
+  .normalize();
 
 CmdArgs.push_back("-aux-triple");
 CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -196,10 +196,10 @@
   if (FailingCommands.empty())
 return false;
 
-  // CUDA can have the same input source code compiled multiple times so do not
-  // compiled again if there are already failures. It is OK to abort the CUDA
-  // pipeline on errors.
-  if (A->isOffloading(Action::OFK_Cuda))
+  // CUDA/HIP can have the same input source code compiled multiple times so do
+  // not compiled again if there are already failures. It is OK to abort the
+  // CUDA pipeline on errors.
+  if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP))
 return true;
 
   for (const auto  : FailingCommands)
Index: lib/Driver/Action.cpp
===
--- lib/Driver/Action.cpp
+++ lib/Driver/Action.cpp
@@ -96,16 +96,23 @@
 return "device-cuda";
   case OFK_OpenMP:
 return "device-openmp";
+  case OFK_HIP:
+return "device-hip";
 
 // TODO: Add other programming models here.
   }
 
   if (!ActiveOffloadKindMask)
 return {};
 
   

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC331556: [analyzer] Treat more const variables and fields as 
known contants. (authored by dergachev, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45774

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Core/SValBuilder.cpp
  test/Analysis/globals.cpp

Index: test/Analysis/globals.cpp
===
--- test/Analysis/globals.cpp
+++ test/Analysis/globals.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+
+static const unsigned long long scull = 0;
+void static_int()
+{
+*(int*)scull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull = 0;
+void const_int()
+{
+*(int*)cull = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+static int * const spc = 0;
+void static_ptr()
+{
+*spc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc = 0;
+void const_ptr()
+{
+*pc = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+const unsigned long long cull_nonnull = 4;
+void nonnull_int()
+{
+*(int*)(cull_nonnull - 4) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const pc_nonnull = (int*)sizeof(int);
+void nonnull_ptr()
+{
+*(pc_nonnull - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const constcast = const_cast((int*)sizeof(int));
+void cast1()
+{
+*(constcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const recast = reinterpret_cast(sizeof(int));
+void cast2()
+{
+*(recast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const staticcast = static_cast((int*)sizeof(int));
+void cast3()
+{
+*(staticcast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct Foo { int a; };
+Foo * const dyncast = dynamic_cast((Foo*)sizeof(Foo));
+void cast4()
+{
+// Do not handle dynamic_cast for now, because it may change the pointer value.
+(dyncast - 1)->a = 0; // no-warning
+}
+
+typedef int * const intptrconst;
+int * const funccast = intptrconst(sizeof(int));
+void cast5()
+{
+*(funccast - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S1
+{
+int * p;
+};
+const S1 s1 = {
+.p = (int*)sizeof(int)
+};
+void conststruct()
+{
+*(s1.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S2
+{
+int * const p;
+};
+S2 s2 = {
+.p = (int*)sizeof(int)
+};
+void constfield()
+{
+*(s2.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+int * const parr[1] = { (int*)sizeof(int) };
+void constarr()
+{
+*(parr[0] - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
+
+struct S3
+{
+int * p = (int*)sizeof(int);
+};
+void recordinit()
+{
+S3 s3;
+*(s3.p - 1) = 0; // expected-warning{{Dereference of null pointer}}
+}
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -119,7 +119,7 @@
 
   if (T->isNullPtrType())
 return makeZeroVal(T);
-  
+
   if (!SymbolManager::canSymbolicate(T))
 return UnknownVal();
 
@@ -328,12 +328,19 @@
   case Stmt::CXXNullPtrLiteralExprClass:
 return makeNull();
 
+  case Stmt::CStyleCastExprClass:
+  case Stmt::CXXFunctionalCastExprClass:
+  case Stmt::CXXConstCastExprClass:
+  case Stmt::CXXReinterpretCastExprClass:
+  case Stmt::CXXStaticCastExprClass:
   case Stmt::ImplicitCastExprClass: {
 const auto *CE = cast(E);
 switch (CE->getCastKind()) {
 default:
   break;
 case CK_ArrayToPointerDecay:
+case CK_IntegralToPointer:
+case CK_NoOp:
 case CK_BitCast: {
   const Expr *SE = CE->getSubExpr();
   Optional Val = getConstantVal(SE);
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1606,7 +1606,7 @@
   const MemRegion* superR = R->getSuperRegion();
 
   // Check if the region is an element region of a string literal.
-  if (const StringRegion *StrR=dyn_cast(superR)) {
+  if (const StringRegion *StrR = dyn_cast(superR)) {
 // FIXME: Handle loads from strings where the literal is treated as
 // an integer, e.g., *((unsigned int*)"hello")
 QualType T = Ctx.getAsArrayType(StrR->getValueType())->getElementType();
@@ -1629,6 +1629,27 @@
   char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
   return svalBuilder.makeIntVal(c, T);
 }
+  } else if (const VarRegion *VR = dyn_cast(superR)) {
+// Check if the containing array is const and has an initialized value.
+const VarDecl *VD = VR->getDecl();
+// Either the array or the array element has to be 

r331556 - [analyzer] Treat more const variables and fields as known contants.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Fri May  4 13:52:39 2018
New Revision: 331556

URL: http://llvm.org/viewvc/llvm-project?rev=331556=rev
Log:
[analyzer] Treat more const variables and fields as known contants.

When loading from a variable or a field that is declared as constant,
the analyzer will try to inspect its initializer and constant-fold it.
Upon success, the analyzer would skip normal load and return the respective
constant.

The new behavior also applies to fields/elements of brace-initialized structures
and arrays.

Patch by Rafael Stahl!

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

Added:
cfe/trunk/test/Analysis/globals.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=331556=331555=331556=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Fri May  4 13:52:39 2018
@@ -1606,7 +1606,7 @@ SVal RegionStoreManager::getBindingForEl
   const MemRegion* superR = R->getSuperRegion();
 
   // Check if the region is an element region of a string literal.
-  if (const StringRegion *StrR=dyn_cast(superR)) {
+  if (const StringRegion *StrR = dyn_cast(superR)) {
 // FIXME: Handle loads from strings where the literal is treated as
 // an integer, e.g., *((unsigned int*)"hello")
 QualType T = Ctx.getAsArrayType(StrR->getValueType())->getElementType();
@@ -1629,6 +1629,27 @@ SVal RegionStoreManager::getBindingForEl
   char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
   return svalBuilder.makeIntVal(c, T);
 }
+  } else if (const VarRegion *VR = dyn_cast(superR)) {
+// Check if the containing array is const and has an initialized value.
+const VarDecl *VD = VR->getDecl();
+// Either the array or the array element has to be const.
+if (VD->getType().isConstQualified() || 
R->getElementType().isConstQualified()) {
+  if (const Expr *Init = VD->getInit()) {
+if (const auto *InitList = dyn_cast(Init)) {
+  // The array index has to be known.
+  if (auto CI = R->getIndex().getAs()) {
+int64_t i = CI->getValue().getSExtValue();
+// Return unknown value if index is out of bounds.
+if (i < 0 || i >= InitList->getNumInits())
+  return UnknownVal();
+
+if (const Expr *ElemInit = InitList->getInit(i))
+  if (Optional V = svalBuilder.getConstantVal(ElemInit))
+return *V;
+  }
+}
+  }
+}
   }
 
   // Check for loads from a code text region.  For such loads, just give up.
@@ -1678,7 +1699,28 @@ SVal RegionStoreManager::getBindingForFi
   if (const Optional  = B.getDirectBinding(R))
 return *V;
 
-  QualType Ty = R->getValueType();
+  // Is the field declared constant and has an in-class initializer?
+  const FieldDecl *FD = R->getDecl();
+  QualType Ty = FD->getType();
+  if (Ty.isConstQualified())
+if (const Expr *Init = FD->getInClassInitializer())
+  if (Optional V = svalBuilder.getConstantVal(Init))
+return *V;
+
+  // If the containing record was initialized, try to get its constant value.
+  const MemRegion* superR = R->getSuperRegion();
+  if (const auto *VR = dyn_cast(superR)) {
+const VarDecl *VD = VR->getDecl();
+QualType RecordVarTy = VD->getType();
+// Either the record variable or the field has to be const qualified.
+if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
+  if (const Expr *Init = VD->getInit())
+if (const auto *InitList = dyn_cast(Init))
+  if (const Expr *FieldInit = InitList->getInit(FD->getFieldIndex()))
+if (Optional V = svalBuilder.getConstantVal(FieldInit))
+  return *V;
+  }
+
   return getBindingForFieldOrElementCommon(B, R, Ty);
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=331556=331555=331556=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Fri May  4 13:52:39 2018
@@ -119,7 +119,7 @@ SValBuilder::getRegionValueSymbolVal(con
 
   if (T->isNullPtrType())
 return makeZeroVal(T);
-  
+
   if (!SymbolManager::canSymbolicate(T))
 return UnknownVal();
 
@@ -328,12 +328,19 @@ Optional SValBuilder::getConstantV
   case Stmt::CXXNullPtrLiteralExprClass:
 return makeNull();
 
+  case Stmt::CStyleCastExprClass:
+  case Stmt::CXXFunctionalCastExprClass:
+  case Stmt::CXXConstCastExprClass:
+  case Stmt::CXXReinterpretCastExprClass:
+  case Stmt::CXXStaticCastExprClass:
   case 

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Two questions for the future:

- Should we skip the initializer binding for local variables (and 
fields/elements of local variables) entirely? Cause we can load from them 
anyway. And keeping the Store small might be good for performance.

- Just in case, do you accidentally plan supporting nested initializer lists?


https://reviews.llvm.org/D45774



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

[PATCH] D46464: [ThinLTO] Support opt remarks options with distributed ThinLTO backends

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added a reviewer: pcc.
Herald added subscribers: eraman, inglorion, mehdi_amini.

Passes down the necessary code ge options to the LTO Config to enable
-fdiagnostics-show-hotness and -fsave-optimization-record in the ThinLTO
backend for a distributed build.

Also, remove warning about not having PGO when the input is IR.


Repository:
  rC Clang

https://reviews.llvm.org/D46464

Files:
  lib/CodeGen/BackendUtil.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll


Index: test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
===
--- /dev/null
+++ test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
@@ -0,0 +1,47 @@
+; Test to ensure -fdiagnostics-show-hotness and -fsave-optimization-record
+; work when invoking the ThinLTO backend path.
+; RUN: opt -module-summary -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+
+; First try with pass remarks to stderr
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm 
-pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+
+; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 
300)
+
+; Next try YAML pass remarks file
+; RUN: rm -f %t2.opt.yaml.thin.0.yaml
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc 
-fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml.thin.0.yaml | FileCheck %s -check-prefix=YAML
+
+; YAML: --- !Passed
+; YAML-NEXT: Pass:inline
+; YAML-NEXT: Name:Inlined
+; YAML-NEXT: Function:main
+; YAML-NEXT: Hotness: 300
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee:  tinkywinky
+; YAML-NEXT:   - String:  ' inlined into '
+; YAML-NEXT:   - Caller:  main
+; YAML-NEXT:   - String:  ' with cost='
+; YAML-NEXT:   - Cost:'0'
+; YAML-NEXT:   - String:  ' (threshold='
+; YAML-NEXT:   - Threshold:   '337'
+; YAML-NEXT:   - String:  ')'
+; YAML-NEXT: ...
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+declare i32 @patatino()
+
+define i32 @tinkywinky() {
+  %a = call i32 @patatino()
+  ret i32 %a
+}
+
+define i32 @main() !prof !0 {
+  %i = call i32 @tinkywinky()
+  ret i32 %i
+}
+
+!0 = !{!"function_entry_count", i64 300}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1075,7 +1075,9 @@
   bool UsingProfile = UsingSampleProfile ||
   (Opts.getProfileUse() != CodeGenOptions::ProfileNone);
 
-  if (Opts.DiagnosticsWithHotness && !UsingProfile)
+  if (Opts.DiagnosticsWithHotness && !UsingProfile &&
+  // An IR file will contain PGO as metadata
+  IK.getLanguage() != InputKind::LLVM_IR)
 Diags.Report(diag::warn_drv_diagnostics_hotness_requires_pgo)
 << "-fdiagnostics-show-hotness";
 
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -1136,6 +1136,8 @@
   Conf.SampleProfile = std::move(SampleProfile);
   Conf.UseNewPM = CGOpts.ExperimentalNewPassManager;
   Conf.DebugPassManager = CGOpts.DebugPassManager;
+  Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
+  Conf.RemarksFilename = CGOpts.OptRecordFile;
   switch (Action) {
   case Backend_EmitNothing:
 Conf.PreCodeGenModuleHook = [](size_t Task, const Module ) {


Index: test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
===
--- /dev/null
+++ test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
@@ -0,0 +1,47 @@
+; Test to ensure -fdiagnostics-show-hotness and -fsave-optimization-record
+; work when invoking the ThinLTO backend path.
+; RUN: opt -module-summary -o %t.o %s
+; RUN: llvm-lto -thinlto -o %t %t.o
+
+; First try with pass remarks to stderr
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -mllvm -pass-remarks=inline -fdiagnostics-show-hotness -o %t2.o -c 2>&1 | FileCheck %s
+
+; CHECK: tinkywinky inlined into main with cost=0 (threshold=337) (hotness: 300)
+
+; Next try YAML pass remarks file
+; RUN: rm -f %t2.opt.yaml.thin.0.yaml
+; RUN: %clang -O2 -x ir %t.o -fthinlto-index=%t.thinlto.bc -fsave-optimization-record -fdiagnostics-show-hotness -o %t2.o -c
+; RUN: cat %t2.opt.yaml.thin.0.yaml | FileCheck %s -check-prefix=YAML
+
+; YAML: --- !Passed
+; YAML-NEXT: Pass:inline
+; YAML-NEXT: Name:Inlined
+; YAML-NEXT: Function:main
+; YAML-NEXT: Hotness: 300
+; YAML-NEXT: Args:
+; YAML-NEXT:   - Callee:  tinkywinky
+; YAML-NEXT:   - String:  ' inlined into '
+; YAML-NEXT:   - Caller:  main
+; YAML-NEXT:   - String:

[PATCH] D46452: [sanitizer] Don't add --export-dynamic for Myriad

[Walter Lee via Phabricator via cfe-commits]

waltl created this revision.
waltl added reviewers: vitalybuka, eugenis, alekseyshl.
Herald added a subscriber: llvm-commits.

This is to work around a bug in some versions of gnu ld, where
--export-dynamic implies -shared even if -static is explicitly given.
Myriad supports static linking only, so --export-dynamic is never
needed.


Repository:
  rL LLVM

https://reviews.llvm.org/D46452

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


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -703,7 +703,7 @@
   }
   // If there is a static runtime with no dynamic list, force all the symbols
   // to be dynamic to be sure we export sanitizer interface functions.
-  if (AddExportDynamic)
+  if (AddExportDynamic && TC.getTriple().getVendor() != llvm::Triple::Myriad)
 CmdArgs.push_back("--export-dynamic");
 
   const SanitizerArgs  = TC.getSanitizerArgs();


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -703,7 +703,7 @@
   }
   // If there is a static runtime with no dynamic list, force all the symbols
   // to be dynamic to be sure we export sanitizer interface functions.
-  if (AddExportDynamic)
+  if (AddExportDynamic && TC.getTriple().getVendor() != llvm::Triple::Myriad)
 CmdArgs.push_back("--export-dynamic");
 
   const SanitizerArgs  = TC.getSanitizerArgs();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 145266.
cmtice added a comment.

Added comment to change in source code.


https://reviews.llvm.org/D46403

Files:
  lib/Driver/SanitizerArgs.cpp
  test/Driver/fsanitize-blacklist.c


Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -62,4 +62,8 @@
 // CHECK-ONLY-FIRST-DISABLED: -fsanitize-blacklist={{.*}}.second
 // CHECK-ONLY_FIRST-DISABLED-NOT: good
 
+// If cfi_blacklist.txt cannot be found in the resource dir, driver should 
fail.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -resource-dir=/dev/null 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-CFI-BLACKLIST
+// CHECK-MISSING-CFI-BLACKLIST: error: no such file or directory: 
'{{.*}}/share/cfi_blacklist.txt'
+
 // DELIMITERS: {{^ *"}}
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -115,6 +115,10 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  // If cfi_blacklist.txt cannot be found in the resource dir, driver
+  // should fail.
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 


Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -62,4 +62,8 @@
 // CHECK-ONLY-FIRST-DISABLED: -fsanitize-blacklist={{.*}}.second
 // CHECK-ONLY_FIRST-DISABLED-NOT: good
 
+// If cfi_blacklist.txt cannot be found in the resource dir, driver should fail.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -resource-dir=/dev/null %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-CFI-BLACKLIST
+// CHECK-MISSING-CFI-BLACKLIST: error: no such file or directory: '{{.*}}/share/cfi_blacklist.txt'
+
 // DELIMITERS: {{^ *"}}
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -115,6 +115,10 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  // If cfi_blacklist.txt cannot be found in the resource dir, driver
+  // should fail.
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45774: [analyzer] cover more cases where a Loc can be bound to constants

[Artem Dergachev via Phabricator via cfe-commits]

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

Yup, thanks! I'll commit.


https://reviews.llvm.org/D45774



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

[PATCH] D46450: [Driver] Add mips_Features_Group to Options to improve documentation sorting

[Vince Del Vecchio via Phabricator via cfe-commits]

vdelvecc created this revision.
vdelvecc added reviewers: sdardis, atanasyan, rsmith.
Herald added subscribers: cfe-commits, arichardson.

Move all of the MIPS-only options into a new m_mips_Features_Group.  Nearly all 
other targets have most target-specific options grouped, but MIPS does not.

The primary benefits are that the options will be listed together (and thus 
identifiable as MIPS-specific even if they have no help string) in the 
ClangCommandLineReference, and that Options.td is a bit more organized.

A secondary benefit is that a custom version of clang can more easily 
hide/disable groups of options for unsupported targets.


Repository:
  rC Clang

https://reviews.llvm.org/D46450

Files:
  include/clang/Driver/Options.td

Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -143,14 +143,16 @@
 // These are explicitly handled.
 def m_hexagon_Features_HVX_Group : OptionGroup<"">,
Group, DocName<"Hexagon">;
+def m_mips_Features_Group : OptionGroup<"">,
+Group, DocName<"MIPS">;
 def m_ppc_Features_Group : OptionGroup<"">,
Group, DocName<"PowerPC">;
 def m_wasm_Features_Group : OptionGroup<"">,
 Group, DocName<"WebAssembly">;
 def m_x86_Features_Group : OptionGroup<"">,
Group, Flags<[CoreOption]>, DocName<"X86">;
 
-def m_libc_Group : OptionGroup<"">, Group,
+def m_libc_Group : OptionGroup<"">, Group,
Flags<[HelpHidden]>;
 
 def O_Group : OptionGroup<"">, Group,
@@ -2092,127 +2094,135 @@
 def mno_pie_copy_relocations : Flag<["-"], "mno-pie-copy-relocations">, Group;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at function entry (x86 only)">,
   Flags<[CC1Option]>, Group;
-def mips16 : Flag<["-"], "mips16">, Group;
-def mno_mips16 : Flag<["-"], "mno-mips16">, Group;
-def mmicromips : Flag<["-"], "mmicromips">, Group;
-def mno_micromips : Flag<["-"], "mno-micromips">, Group;
-def mxgot : Flag<["-"], "mxgot">, Group;
-def mno_xgot : Flag<["-"], "mno-xgot">, Group;
-def mldc1_sdc1 : Flag<["-"], "mldc1-sdc1">, Group;
-def mno_ldc1_sdc1 : Flag<["-"], "mno-ldc1-sdc1">, Group;
-def mcheck_zero_division : Flag<["-"], "mcheck-zero-division">, Group;
+def mips16 : Flag<["-"], "mips16">, Group;
+def mno_mips16 : Flag<["-"], "mno-mips16">, Group;
+def mmicromips : Flag<["-"], "mmicromips">, Group;
+def mno_micromips : Flag<["-"], "mno-micromips">, Group;
+def mxgot : Flag<["-"], "mxgot">, Group;
+def mno_xgot : Flag<["-"], "mno-xgot">, Group;
+def mldc1_sdc1 : Flag<["-"], "mldc1-sdc1">, Group;
+def mno_ldc1_sdc1 : Flag<["-"], "mno-ldc1-sdc1">, Group;
+def mcheck_zero_division : Flag<["-"], "mcheck-zero-division">,
+   Group;
 def mno_check_zero_division : Flag<["-"], "mno-check-zero-division">,
-  Group;
-def mcompact_branches_EQ : Joined<["-"], "mcompact-branches=">, Group;
+  Group;
+def mcompact_branches_EQ : Joined<["-"], "mcompact-branches=">,
+   Group;
 def mbranch_likely : Flag<["-"], "mbranch-likely">, Group,
   IgnoredGCCCompat;
 def mno_branch_likely : Flag<["-"], "mno-branch-likely">, Group,
   IgnoredGCCCompat;
 def mindirect_jump_EQ : Joined<["-"], "mindirect-jump=">,
-  Group,
+  Group,
   HelpText<"Change indirect jump instructions to inhibit speculation">;
-def mdsp : Flag<["-"], "mdsp">, Group;
-def mno_dsp : Flag<["-"], "mno-dsp">, Group;
-def mdspr2 : Flag<["-"], "mdspr2">, Group;
-def mno_dspr2 : Flag<["-"], "mno-dspr2">, Group;
-def msingle_float : Flag<["-"], "msingle-float">, Group;
-def mdouble_float : Flag<["-"], "mdouble-float">, Group;
-def mmadd4 : Flag<["-"], "mmadd4">, Group,
+def mdsp : Flag<["-"], "mdsp">, Group;
+def mno_dsp : Flag<["-"], "mno-dsp">, Group;
+def mdspr2 : Flag<["-"], "mdspr2">, Group;
+def mno_dspr2 : Flag<["-"], "mno-dspr2">, Group;
+def msingle_float : Flag<["-"], "msingle-float">, Group;
+def mdouble_float : Flag<["-"], "mdouble-float">, Group;
+def mmadd4 : Flag<["-"], "mmadd4">, Group,
   HelpText<"Enable the generation of 4-operand madd.s, madd.d and related instructions.">;
-def mno_madd4 : Flag<["-"], "mno-madd4">, Group,
+def mno_madd4 : Flag<["-"], "mno-madd4">, Group,
   HelpText<"Disable the generation of 4-operand madd.s, madd.d and related instructions.">;
-def mmsa : Flag<["-"], "mmsa">, Group,
+def mmsa : Flag<["-"], "mmsa">, Group,
   HelpText<"Enable MSA ASE (MIPS only)">;
-def mno_msa : Flag<["-"], "mno-msa">, Group,
+def mno_msa : Flag<["-"], "mno-msa">, Group,
   HelpText<"Disable MSA ASE (MIPS only)">;
-def mmt : Flag<["-"], "mmt">, Group,
+def mmt : Flag<["-"], "mmt">, Group,
   HelpText<"Enable MT ASE (MIPS only)">;
-def mno_mt : Flag<["-"], "mno-mt">, Group,
+def mno_mt : Flag<["-"], "mno-mt">, Group,
   

[PATCH] D46449: remove references to nonexistent PragmaTable interface

[Mostyn Bramley-Moore via Phabricator via cfe-commits]

mostynb created this revision.
Herald added a subscriber: cfe-commits.

There does not appear to be a PragmaTable interface in the repository, either 
on tip-of-tree or historically.

Let's remove these references.


Repository:
  rC Clang

https://reviews.llvm.org/D46449

Files:
  clang/include/clang/Lex/Pragma.h
  clang/lib/Lex/Pragma.cpp


Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -7,7 +7,7 @@
 //
 
//===--===//
 //
-// This file implements the PragmaHandler/PragmaTable interfaces and implements
+// This file implements the PragmaHandler interface and implements
 // pragma related methods of the Preprocessor class.
 //
 
//===--===//
Index: clang/include/clang/Lex/Pragma.h
===
--- clang/include/clang/Lex/Pragma.h
+++ clang/include/clang/Lex/Pragma.h
@@ -7,7 +7,7 @@
 //
 
//===--===//
 //
-// This file defines the PragmaHandler and PragmaTable interfaces.
+// This file defines the PragmaHandler interface.
 //
 
//===--===//
 


Index: clang/lib/Lex/Pragma.cpp
===
--- clang/lib/Lex/Pragma.cpp
+++ clang/lib/Lex/Pragma.cpp
@@ -7,7 +7,7 @@
 //
 //===--===//
 //
-// This file implements the PragmaHandler/PragmaTable interfaces and implements
+// This file implements the PragmaHandler interface and implements
 // pragma related methods of the Preprocessor class.
 //
 //===--===//
Index: clang/include/clang/Lex/Pragma.h
===
--- clang/include/clang/Lex/Pragma.h
+++ clang/include/clang/Lex/Pragma.h
@@ -7,7 +7,7 @@
 //
 //===--===//
 //
-// This file defines the PragmaHandler and PragmaTable interfaces.
+// This file defines the PragmaHandler interface.
 //
 //===--===//
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

[Sterling Augustine via cfe-commits]

Committed as r331552.

On Fri, May 4, 2018 at 12:43 PM, Mikhail Ramalho via Phabricator <
revi...@reviews.llvm.org> wrote:

> mikhail.ramalho updated this revision to Diff 145255.
> mikhail.ramalho added a comment.
>
> Fixed the test case.
>
>
> https://reviews.llvm.org/D36610
>
> Files:
>   include/clang/AST/QualTypeNames.h
>   lib/AST/QualTypeNames.cpp
>   unittests/Tooling/QualTypeNamesTest.cpp
>
>
> Index: unittests/Tooling/QualTypeNamesTest.cpp
> ===
> --- unittests/Tooling/QualTypeNamesTest.cpp
> +++ unittests/Tooling/QualTypeNamesTest.cpp
> @@ -26,9 +26,13 @@
>  std::string ExpectedName =
>  ExpectedQualTypeNames.lookup(VD->getNameAsString());
>  if (ExpectedName != "") {
> -  std::string ActualName =
> -  TypeName::getFullyQualifiedName(VD->getType(), *Context,
> -  WithGlobalNsPrefix);
> +  PrintingPolicy Policy(Context->getPrintingPolicy());
> +  Policy.SuppressScope = false;
> +  Policy.AnonymousTagLocations = true;
> +  Policy.PolishForDeclaration = true;
> +  Policy.SuppressUnwrittenScope = true;
> +  std::string ActualName = TypeName::getFullyQualifiedName(
> +  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
>if (ExpectedName != ActualName) {
>  // A custom message makes it much easier to see what declaration
>  // failed compared to EXPECT_EQ.
> @@ -217,6 +221,26 @@
>"  }\n"
>"}\n"
>);
> +
> +  TypeNameVisitor AnonStrucs;
> +  AnonStrucs.ExpectedQualTypeNames["a"] = "short";
> +  AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
> +  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
> +  "input.cc:2:27)";
> +  AnonStrucs.ExpectedQualTypeNames["b"] = "short";
> +  AnonStrucs.ExpectedQualTypeNames["un_in_st_2"] =
> +  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
> +  "input.cc:5:27)";
> +  AnonStrucs.ExpectedQualTypeNames["anon_st"] =
> +  "struct (anonymous struct at input.cc:1:1)";
> +  AnonStrucs.runOver(R"(struct {
> +  union {
> +short a;
> +  } un_in_st_1;
> +  union {
> +short b;
> +  } un_in_st_2;
> +} anon_st;)");
>  }
>
>  }  // end anonymous namespace
> Index: lib/AST/QualTypeNames.cpp
> ===
> --- lib/AST/QualTypeNames.cpp
> +++ lib/AST/QualTypeNames.cpp
> @@ -452,12 +452,8 @@
>
>  std::string getFullyQualifiedName(QualType QT,
>const ASTContext ,
> +  const PrintingPolicy ,
>bool WithGlobalNsPrefix) {
> -  PrintingPolicy Policy(Ctx.getPrintingPolicy());
> -  Policy.SuppressScope = false;
> -  Policy.AnonymousTagLocations = false;
> -  Policy.PolishForDeclaration = true;
> -  Policy.SuppressUnwrittenScope = true;
>QualType FQQT = getFullyQualifiedType(QT, Ctx, WithGlobalNsPrefix);
>return FQQT.getAsString(Policy);
>  }
> Index: include/clang/AST/QualTypeNames.h
> ===
> --- include/clang/AST/QualTypeNames.h
> +++ include/clang/AST/QualTypeNames.h
> @@ -72,6 +72,7 @@
>  /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
>  /// specifier "::" will be prepended to the fully qualified name.
>  std::string getFullyQualifiedName(QualType QT, const ASTContext ,
> +  const PrintingPolicy ,
>bool WithGlobalNsPrefix = false);
>
>  /// \brief Generates a QualType that can be used to name the same type
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r331552 - Allow modifying the PrintingPolicy for fully qualified names.

[Sterling Augustine via cfe-commits]

Author: saugustine
Date: Fri May  4 13:12:39 2018
New Revision: 331552

URL: http://llvm.org/viewvc/llvm-project?rev=331552=rev
Log:
Allow modifying the PrintingPolicy for fully qualified names.

Author: mikhail.rama...@gmail.com


Modified:
cfe/trunk/include/clang/AST/QualTypeNames.h
cfe/trunk/lib/AST/QualTypeNames.cpp
cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp

Modified: cfe/trunk/include/clang/AST/QualTypeNames.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/QualTypeNames.h?rev=331552=331551=331552=diff
==
--- cfe/trunk/include/clang/AST/QualTypeNames.h (original)
+++ cfe/trunk/include/clang/AST/QualTypeNames.h Fri May  4 13:12:39 2018
@@ -72,6 +72,7 @@ namespace TypeName {
 /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
 /// specifier "::" will be prepended to the fully qualified name.
 std::string getFullyQualifiedName(QualType QT, const ASTContext ,
+  const PrintingPolicy ,
   bool WithGlobalNsPrefix = false);
 
 /// \brief Generates a QualType that can be used to name the same type

Modified: cfe/trunk/lib/AST/QualTypeNames.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/QualTypeNames.cpp?rev=331552=331551=331552=diff
==
--- cfe/trunk/lib/AST/QualTypeNames.cpp (original)
+++ cfe/trunk/lib/AST/QualTypeNames.cpp Fri May  4 13:12:39 2018
@@ -452,12 +452,8 @@ QualType getFullyQualifiedType(QualType
 
 std::string getFullyQualifiedName(QualType QT,
   const ASTContext ,
+  const PrintingPolicy ,
   bool WithGlobalNsPrefix) {
-  PrintingPolicy Policy(Ctx.getPrintingPolicy());
-  Policy.SuppressScope = false;
-  Policy.AnonymousTagLocations = false;
-  Policy.PolishForDeclaration = true;
-  Policy.SuppressUnwrittenScope = true;
   QualType FQQT = getFullyQualifiedType(QT, Ctx, WithGlobalNsPrefix);
   return FQQT.getAsString(Policy);
 }

Modified: cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp?rev=331552=331551=331552=diff
==
--- cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/QualTypeNamesTest.cpp Fri May  4 13:12:39 2018
@@ -26,9 +26,13 @@ struct TypeNameVisitor : TestVisitorgetNameAsString());
 if (ExpectedName != "") {
-  std::string ActualName =
-  TypeName::getFullyQualifiedName(VD->getType(), *Context,
-  WithGlobalNsPrefix);
+  PrintingPolicy Policy(Context->getPrintingPolicy());
+  Policy.SuppressScope = false;
+  Policy.AnonymousTagLocations = true;
+  Policy.PolishForDeclaration = true;
+  Policy.SuppressUnwrittenScope = true;
+  std::string ActualName = TypeName::getFullyQualifiedName(
+  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
   if (ExpectedName != ActualName) {
 // A custom message makes it much easier to see what declaration
 // failed compared to EXPECT_EQ.
@@ -217,6 +221,26 @@ TEST(QualTypeNameTest, getFullyQualified
   "  }\n"
   "}\n"
   );
+
+  TypeNameVisitor AnonStrucs;
+  AnonStrucs.ExpectedQualTypeNames["a"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:2:27)";
+  AnonStrucs.ExpectedQualTypeNames["b"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_2"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:5:27)";
+  AnonStrucs.ExpectedQualTypeNames["anon_st"] =
+  "struct (anonymous struct at input.cc:1:1)";
+  AnonStrucs.runOver(R"(struct {
+  union {
+short a;
+  } un_in_st_1;
+  union {
+short b;
+  } un_in_st_2;
+} anon_st;)");
 }
 
 }  // end anonymous namespace


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

[PATCH] D46415: [analyzer] pr36458: Fix retrieved value cast for symbolic void pointers.

[Artem Dergachev via Phabricator via cfe-commits]

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

Fix test names. Add one more test, just to make sure it works.


https://reviews.llvm.org/D46415

Files:
  lib/StaticAnalyzer/Core/Store.cpp
  test/Analysis/casts.c


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -149,3 +149,25 @@
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // 
expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastUIntPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned u;
+  int *x;
+  (*((int *)())) = (int)
+  *x = 1;
+  clang_analyzer_eval(u == 1); // expected-warning{{TRUE}}
+}
+
+void testCastVoidPtrToIntPtrThroughUIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)(unsigned *)getVoidPtr();
+  *x = 1; // no-crash
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -378,6 +378,20 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy));
+
   return svalBuilder.dispatchCast(V, castTy);
 }
 


Index: test/Analysis/casts.c
===
--- test/Analysis/casts.c
+++ test/Analysis/casts.c
@@ -149,3 +149,25 @@
 
   clang_analyzer_eval(*((char *)y1) == *((char *) y3)); // expected-warning{{TRUE}}
 }
+
+void *getVoidPtr();
+
+void testCastVoidPtrToIntPtrThroughIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)getVoidPtr();
+  *x = 1; // no-crash
+}
+
+void testCastUIntPtrToIntPtrThroughIntTypedAssignment() {
+  unsigned u;
+  int *x;
+  (*((int *)())) = (int)
+  *x = 1;
+  clang_analyzer_eval(u == 1); // expected-warning{{TRUE}}
+}
+
+void testCastVoidPtrToIntPtrThroughUIntTypedAssignment() {
+  int *x;
+  (*((int *)())) = (int)(unsigned *)getVoidPtr();
+  *x = 1; // no-crash
+}
Index: lib/StaticAnalyzer/Core/Store.cpp
===
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -378,6 +378,20 @@
   if (castTy.isNull() || V.isUnknownOrUndef())
 return V;
 
+  // When retrieving symbolic pointer and expecting a non-void pointer,
+  // wrap them into element regions of the expected type if necessary.
+  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
+  // make sure that the retrieved value makes sense, because there's no other
+  // cast in the AST that would tell us to cast it to the correct pointer type.
+  // We might need to do that for non-void pointers as well.
+  // FIXME: We really need a single good function to perform casts for us
+  // correctly every time we need it.
+  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  if (SR->getSymbol()->getType().getCanonicalType() !=
+  castTy.getCanonicalType())
+return loc::MemRegionVal(castRegion(SR, castTy));
+
   return svalBuilder.dispatchCast(V, castTy);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho updated this revision to Diff 145255.
mikhail.ramalho added a comment.

Fixed the test case.


https://reviews.llvm.org/D36610

Files:
  include/clang/AST/QualTypeNames.h
  lib/AST/QualTypeNames.cpp
  unittests/Tooling/QualTypeNamesTest.cpp


Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -26,9 +26,13 @@
 std::string ExpectedName =
 ExpectedQualTypeNames.lookup(VD->getNameAsString());
 if (ExpectedName != "") {
-  std::string ActualName =
-  TypeName::getFullyQualifiedName(VD->getType(), *Context,
-  WithGlobalNsPrefix);
+  PrintingPolicy Policy(Context->getPrintingPolicy());
+  Policy.SuppressScope = false;
+  Policy.AnonymousTagLocations = true;
+  Policy.PolishForDeclaration = true;
+  Policy.SuppressUnwrittenScope = true;
+  std::string ActualName = TypeName::getFullyQualifiedName(
+  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
   if (ExpectedName != ActualName) {
 // A custom message makes it much easier to see what declaration
 // failed compared to EXPECT_EQ.
@@ -217,6 +221,26 @@
   "  }\n"
   "}\n"
   );
+
+  TypeNameVisitor AnonStrucs;
+  AnonStrucs.ExpectedQualTypeNames["a"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:2:27)";
+  AnonStrucs.ExpectedQualTypeNames["b"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_2"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:5:27)";
+  AnonStrucs.ExpectedQualTypeNames["anon_st"] =
+  "struct (anonymous struct at input.cc:1:1)";
+  AnonStrucs.runOver(R"(struct {
+  union {
+short a;
+  } un_in_st_1;
+  union {
+short b;
+  } un_in_st_2;
+} anon_st;)");
 }
 
 }  // end anonymous namespace
Index: lib/AST/QualTypeNames.cpp
===
--- lib/AST/QualTypeNames.cpp
+++ lib/AST/QualTypeNames.cpp
@@ -452,12 +452,8 @@
 
 std::string getFullyQualifiedName(QualType QT,
   const ASTContext ,
+  const PrintingPolicy ,
   bool WithGlobalNsPrefix) {
-  PrintingPolicy Policy(Ctx.getPrintingPolicy());
-  Policy.SuppressScope = false;
-  Policy.AnonymousTagLocations = false;
-  Policy.PolishForDeclaration = true;
-  Policy.SuppressUnwrittenScope = true;
   QualType FQQT = getFullyQualifiedType(QT, Ctx, WithGlobalNsPrefix);
   return FQQT.getAsString(Policy);
 }
Index: include/clang/AST/QualTypeNames.h
===
--- include/clang/AST/QualTypeNames.h
+++ include/clang/AST/QualTypeNames.h
@@ -72,6 +72,7 @@
 /// \param[in] WithGlobalNsPrefix - If true, then the global namespace
 /// specifier "::" will be prepended to the fully qualified name.
 std::string getFullyQualifiedName(QualType QT, const ASTContext ,
+  const PrintingPolicy ,
   bool WithGlobalNsPrefix = false);
 
 /// \brief Generates a QualType that can be used to name the same type


Index: unittests/Tooling/QualTypeNamesTest.cpp
===
--- unittests/Tooling/QualTypeNamesTest.cpp
+++ unittests/Tooling/QualTypeNamesTest.cpp
@@ -26,9 +26,13 @@
 std::string ExpectedName =
 ExpectedQualTypeNames.lookup(VD->getNameAsString());
 if (ExpectedName != "") {
-  std::string ActualName =
-  TypeName::getFullyQualifiedName(VD->getType(), *Context,
-  WithGlobalNsPrefix);
+  PrintingPolicy Policy(Context->getPrintingPolicy());
+  Policy.SuppressScope = false;
+  Policy.AnonymousTagLocations = true;
+  Policy.PolishForDeclaration = true;
+  Policy.SuppressUnwrittenScope = true;
+  std::string ActualName = TypeName::getFullyQualifiedName(
+  VD->getType(), *Context, Policy, WithGlobalNsPrefix);
   if (ExpectedName != ActualName) {
 // A custom message makes it much easier to see what declaration
 // failed compared to EXPECT_EQ.
@@ -217,6 +221,26 @@
   "  }\n"
   "}\n"
   );
+
+  TypeNameVisitor AnonStrucs;
+  AnonStrucs.ExpectedQualTypeNames["a"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_1"] =
+  "union (anonymous struct at input.cc:1:1)::(anonymous union at "
+  "input.cc:2:27)";
+  AnonStrucs.ExpectedQualTypeNames["b"] = "short";
+  AnonStrucs.ExpectedQualTypeNames["un_in_st_2"] =
+  "union (anonymous 

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

One problem with this direction is that clang doesn't ship a cfi blacklist in 
its default install, so, this leaves cfi users with stock toolchains to fend 
for themselves. I think it'd be a good idea to ship a basic cfi blacklist in 
clang's resource dir to avoid inadvertent breakage.


https://reviews.llvm.org/D46403



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

[PATCH] D46439: Fix incorrect packed aligned structure layout

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

rsmith added a comment.

This looks fine, but needs a test.




Comment at: lib/Sema/SemaDecl.cpp:15576
 
+// Handle attributes before checking for alignas underalignment.
+if (Attr)

More generally: "before checking the layout".



Comment at: lib/Sema/SemaDecl.cpp:15651
 }
   } else {
 ObjCIvarDecl **ClsFields =

Do we need to do any attribute processing in this Objective-C interface / 
implementation / category case?


Repository:
  rC Clang

https://reviews.llvm.org/D46439



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D45476#1088189, @rsmith wrote:

> This is significantly more complexity than we need. We're talking about 
> constexpr variables here, so we just need a `VarDecl* -- you can then ask 
> that declaration for its evaluated value and emit that directly.


If these variables are required to satisfy the requirements for that, then I 
agree that that would be the simplest solution.


https://reviews.llvm.org/D45476



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D45476#1088180, @EricWF wrote:

> OK, As I see it, we have two choices:
>
> (1) Stash the expressions in Sema and import them like
>
> In https://reviews.llvm.org/D45476#1088047, @rjmccall wrote:
>
> > I'm sorry, but this is just not true.  The formation rules for a copy 
> > constructor are laid
> >  out in [class.copy]p2, and they explicitly allow default arguments.
>
>
> Don't apologize because I'm wrong :-P. Thanks for the correction.


It's been a thorn in my side quite a bit, too. :)

>>> Second, in the synopsis for the STL types, no constructors are declared. 
>>> Although I don't
>>>  think the standard spells it out anywhere, I believe this forbids the 
>>> types from having user
>>>  defined constructors (though perhaps not user-declared explicitly 
>>> defaulted constructors.
>>>  For example adding a user defined destructor would be non-conforming since 
>>> it makes
>>>  the types non-literal).
>> 
>> That would certainly be helpful, but I don't think it's true; in general the 
>> standard describes
>>  what things are guaranteed to work with library types, but it doesn't 
>> generally constrain
>>  the existence of other operations.
> 
> I think this is somewhat of a moot point, but I think the constraint is in 
> `[member.functions]p2`:
> 
>> For a non-virtual member function described in the C++ standard library, an 
>> implementation may declare
>>  a different set of member function signatures, provided that any call to 
>> the member function that would
>>  select an overload from the set of declarations described in this document 
>> behaves as if that overload were selected.
> 
> My argument is that because the class synopsis for the comparison category 
> types doesn't describe or declare
>  the copy constructor, so the implementation *isn't*  free to declare it 
> differently.

The type has to allow itself to be constructed with an l-value (whether const 
or not)
of its own type in order to be CopyConstructible.  However, that's just a 
statement
about the *signatures* of the type's constructors; it doesn't say anything 
about whether
those constructors are user-defined, nor does it limit what other constructors 
might
be provided as long as they don't somehow prevent copy-construction from 
succeeding.
(And in somewhat obscure cases, constructing a T with an l-value of type T won't
even select a copy constructor, and that's legal under the standard, too.)  All 
that matters,
absent requirements about T being an aggregate or trivially-copyable type, is 
that the
construction is well-formed and that it has the effect of copying the value.

Anyway, I agree that this is moot.

John.


https://reviews.llvm.org/D45476



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: lib/Driver/SanitizerArgs.cpp:118
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;

vsk wrote:
> CFI can be enabled with other sanitizers, such as ubsan. I think you'll need 
> 'Mask & CFI' here. It'd be great to have a test for this case, too (i.e 
> -fsanitize=cfi,undefined -resource-dir=/dev/null should also give you this 
> diagnostic).
Sorry, I misread the surrounding code. Your check is fine as-is.


https://reviews.llvm.org/D46403



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

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: lib/Driver/SanitizerArgs.cpp:118
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;

CFI can be enabled with other sanitizers, such as ubsan. I think you'll need 
'Mask & CFI' here. It'd be great to have a test for this case, too (i.e 
-fsanitize=cfi,undefined -resource-dir=/dev/null should also give you this 
diagnostic).



Comment at: test/Driver/fsanitize-blacklist.c:65
 
+// If cfi_blacklist.txt cannot be found in the resource dir, driver should 
fail.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -resource-dir=/dev/null 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-CFI-BLACKLIST

It'd be more helpful for readers if this comment were in the source, at the 
point where the diagnostic is emitted.


https://reviews.llvm.org/D46403



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

[PATCH] D46446: [c++17] Fix assertion on synthesizing deduction guides after a fatal error.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added a reviewer: rsmith.

After a fatal error Sema::InstantiatingTemplate doesn't allow further
instantiation and doesn't push a CodeSynthesisContext. When we tried to
synthesize implicit deduction guides from constructors we hit the
assertion

> Assertion failed: (!CodeSynthesisContexts.empty() && "Cannot perform an 
> instantiation without some context on the " "instantiation stack"), function 
> SubstType, file clang/lib/Sema/SemaTemplateInstantiate.cpp, line 1580.

Fix by avoiding deduction guide synthesis if InstantiatingTemplate is invalid.

rdar://problem/39051732


https://reviews.llvm.org/D46446

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp


Index: clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp
@@ -0,0 +1,16 @@
+// RUN: not %clang_cc1 -std=c++17 -fsyntax-only -ferror-limit 1 %s 2>&1 | 
FileCheck %s
+
+#error Error 1
+#error Error 2
+// CHECK: fatal error: too many errors emitted, stopping now
+
+namespace rdar39051732 {
+
+  template struct A {
+template  A(T&, ...);
+  };
+  // Deduction guide triggers constructor instantiation.
+  template A(const T&, const T&) -> A;
+
+}
+
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1935,6 +1935,8 @@
   // FIXME: Add a kind for this to give more meaningful diagnostics. But can
   // this substitution process actually fail?
   InstantiatingTemplate BuildingDeductionGuides(*this, Loc, Template);
+  if (BuildingDeductionGuides.isInvalid())
+return;
 
   // Convert declared constructors into deduction guide templates.
   // FIXME: Skip constructors for which deduction must necessarily fail (those


Index: clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/instantiate-after-fatal-cxx17.cpp
@@ -0,0 +1,16 @@
+// RUN: not %clang_cc1 -std=c++17 -fsyntax-only -ferror-limit 1 %s 2>&1 | FileCheck %s
+
+#error Error 1
+#error Error 2
+// CHECK: fatal error: too many errors emitted, stopping now
+
+namespace rdar39051732 {
+
+  template struct A {
+template  A(T&, ...);
+  };
+  // Deduction guide triggers constructor instantiation.
+  template A(const T&, const T&) -> A;
+
+}
+
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -1935,6 +1935,8 @@
   // FIXME: Add a kind for this to give more meaningful diagnostics. But can
   // this substitution process actually fail?
   InstantiatingTemplate BuildingDeductionGuides(*this, Loc, Template);
+  if (BuildingDeductionGuides.isInvalid())
+return;
 
   // Convert declared constructors into deduction guide templates.
   // FIXME: Skip constructors for which deduction must necessarily fail (those
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46374: Add support for ObjC property name to be a single acronym.

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Looks good to me.


Repository:
  rL LLVM

https://reviews.llvm.org/D46374



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

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

rsmith added a comment.

This is significantly more complexity than we need. We're talking about 
constexpr variables here, so we just need a `VarDecl* -- you can then ask that 
declaration for its evaluated value and emit that directly.


https://reviews.llvm.org/D45476



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

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

OK, As I see it, we have two choices:

(1) Stash the expressions in Sema and import them like

In https://reviews.llvm.org/D45476#1088047, @rjmccall wrote:

>




> Ah.  If you want to be able to find this thing without a Sema around in order 
> to
>  handle deserialized expressions by just caching things in the ASTContext, 
> yes,
>  I agree that it would be difficult to build a `CXXConstructExpr` correctly.  
> I don't
>  fully understand the goal of avoiding serialization here, though.

Perhaps I don't fully understand the goal of avoiding serialization either.

>>> STLs *frequently* make use of default arguments on copy constructors (for
>>>  allocators). I agree that it’s unlikely that that would happen here, but
>>>  that’s precisely because it’s unlikely that this type would ever be
>>>  non-trivial.
>> 
>> A couple of points. First, when I say copy constructor, I mean the special 
>> member, which
>>  cannot have default arguments,
> 
> I'm sorry, but this is just not true.  The formation rules for a copy 
> constructor are laid
>  out in [class.copy]p2, and they explicitly allow default arguments.

Don't apologize because I'm wrong :-P. Thanks for the correction.

>> Second, in the synopsis for the STL types, no constructors are declared. 
>> Although I don't
>>  think the standard spells it out anywhere, I believe this forbids the types 
>> from having user
>>  defined constructors (though perhaps not user-declared explicitly defaulted 
>> constructors.
>>  For example adding a user defined destructor would be non-conforming since 
>> it makes
>>  the types non-literal).
> 
> That would certainly be helpful, but I don't think it's true; in general the 
> standard describes
>  what things are guaranteed to work with library types, but it doesn't 
> generally constrain
>  the existence of other operations.

I think this is somewhat of a moot point, but I think the constraint is in 
`[member.functions]p2`:

> For a non-virtual member function described in the C++ standard library, an 
> implementation may declare
>  a different set of member function signatures, provided that any call to the 
> member function that would
>  select an overload from the set of declarations described in this document 
> behaves as if that overload were selected.

My argument is that because the class synopsis for the comparison category 
types doesn't describe or declare
the copy constructor, so the implementation *isn't*  free to declare it 
differently.


https://reviews.llvm.org/D45476



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

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 updated this revision to Diff 145247.
sbc100 added a comment.

- revert


Repository:
  rCXX libc++

https://reviews.llvm.org/D46443

Files:
  include/cstdalign
  test/libcxx/min_max_macros.sh.cpp
  test/libcxx/utilities/any/size_and_alignment.pass.cpp


Index: test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -14,6 +14,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 int main()
 {
Index: test/libcxx/min_max_macros.sh.cpp
===
--- test/libcxx/min_max_macros.sh.cpp
+++ test/libcxx/min_max_macros.sh.cpp
@@ -70,6 +70,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: include/cstdalign
===
--- /dev/null
+++ include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN


Index: test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -14,6 +14,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 int main()
 {
Index: test/libcxx/min_max_macros.sh.cpp
===
--- test/libcxx/min_max_macros.sh.cpp
+++ test/libcxx/min_max_macros.sh.cpp
@@ -70,6 +70,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: include/cstdalign
===
--- /dev/null
+++ include/cstdalign
@@ -0,0 +1,33 @@
+// -*- C++ -*-
+//===--- cstdalign ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
+/*
+cstdalign synopsis
+
+Macros:
+
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
+
+*/
+
+#include <__config>
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_CSTDALIGN
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46403: [CFI] Force LLVM to die if the implicit blacklist files cannot be found.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 145243.
cmtice added a comment.

Updated the error to only occur for CFI blacklist, and added test case.


https://reviews.llvm.org/D46403

Files:
  lib/Driver/SanitizerArgs.cpp
  test/Driver/fsanitize-blacklist.c


Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -62,4 +62,8 @@
 // CHECK-ONLY-FIRST-DISABLED: -fsanitize-blacklist={{.*}}.second
 // CHECK-ONLY_FIRST-DISABLED-NOT: good
 
+// If cfi_blacklist.txt cannot be found in the resource dir, driver should 
fail.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -resource-dir=/dev/null 
%s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-CFI-BLACKLIST
+// CHECK-MISSING-CFI-BLACKLIST: error: no such file or directory: 
'{{.*}}/share/cfi_blacklist.txt'
+
 // DELIMITERS: {{^ *"}}
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -115,6 +115,8 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 


Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -62,4 +62,8 @@
 // CHECK-ONLY-FIRST-DISABLED: -fsanitize-blacklist={{.*}}.second
 // CHECK-ONLY_FIRST-DISABLED-NOT: good
 
+// If cfi_blacklist.txt cannot be found in the resource dir, driver should fail.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -resource-dir=/dev/null %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-CFI-BLACKLIST
+// CHECK-MISSING-CFI-BLACKLIST: error: no such file or directory: '{{.*}}/share/cfi_blacklist.txt'
+
 // DELIMITERS: {{^ *"}}
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -115,6 +115,8 @@
 llvm::sys::path::append(Path, "share", BL.File);
 if (llvm::sys::fs::exists(Path))
   BlacklistFiles.push_back(Path.str());
+else if (BL.Mask == CFI)
+  D.Diag(clang::diag::err_drv_no_such_file) << Path;
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46443: Add missing cstdalign header

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: cfe-commits, christof, aheejin.

Repository:
  rCXX libc++

https://reviews.llvm.org/D46443

Files:
  include/cassert
  include/cstdalign
  test/libcxx/min_max_macros.sh.cpp
  test/libcxx/utilities/any/size_and_alignment.pass.cpp


Index: test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -14,6 +14,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 int main()
 {
Index: test/libcxx/min_max_macros.sh.cpp
===
--- test/libcxx/min_max_macros.sh.cpp
+++ test/libcxx/min_max_macros.sh.cpp
@@ -70,6 +70,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: include/cstdalign
===
--- include/cstdalign
+++ include/cstdalign
@@ -1,25 +1,33 @@
 // -*- C++ -*-
-//===-- cassert 
---===//
+//===--- cstdalign 
===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 
//===--===//
 
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
 /*
-cassert synopsis
+cstdalign synopsis
 
 Macros:
 
-assert
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
 
 */
 
 #include <__config>
-#include 
+#include 
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
+
+#endif  // _LIBCPP_CSTDALIGN
Index: include/cassert
===
--- include/cassert
+++ include/cassert
@@ -8,6 +8,9 @@
 //
 
//===--===//
 
+#ifndef _LIBCPP_CASSERT
+#define _LIBCPP_CASSERT
+
 /*
 cassert synopsis
 
@@ -23,3 +26,5 @@
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
+
+#endif  // _LIBCPP_CASSERT


Index: test/libcxx/utilities/any/size_and_alignment.pass.cpp
===
--- test/libcxx/utilities/any/size_and_alignment.pass.cpp
+++ test/libcxx/utilities/any/size_and_alignment.pass.cpp
@@ -14,6 +14,7 @@
 // Check that the size and alignment of any are what we expect.
 
 #include 
+#include 
 
 int main()
 {
Index: test/libcxx/min_max_macros.sh.cpp
===
--- test/libcxx/min_max_macros.sh.cpp
+++ test/libcxx/min_max_macros.sh.cpp
@@ -70,6 +70,8 @@
 TEST_MACROS();
 #include 
 TEST_MACROS();
+#include 
+TEST_MACROS();
 #include 
 TEST_MACROS();
 #include 
Index: include/cstdalign
===
--- include/cstdalign
+++ include/cstdalign
@@ -1,25 +1,33 @@
 // -*- C++ -*-
-//===-- cassert ---===//
+//===--- cstdalign ===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===--===//
 
+#ifndef _LIBCPP_CSTDALIGN
+#define _LIBCPP_CSTDALIGN
+
 /*
-cassert synopsis
+cstdalign synopsis
 
 Macros:
 
-assert
+   alignof
+   alignas
+__alignof_is_defined
+__alignas_is_defined
 
 */
 
 #include <__config>
-#include 
+#include 
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
+
+#endif  // _LIBCPP_CSTDALIGN
Index: include/cassert
===
--- include/cassert
+++ include/cassert
@@ -8,6 +8,9 @@
 //
 //===--===//
 
+#ifndef _LIBCPP_CASSERT
+#define _LIBCPP_CASSERT
+
 /*
 cassert synopsis
 
@@ -23,3 +26,5 @@
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
+
+#endif  // _LIBCPP_CASSERT
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46431: [x86] Introduce the pconfig intrinsic

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D46431



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

[PATCH] D46435: [x86] Introduce the encl[u|s|v] intrinsics

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D46435



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