[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 233923.
serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

Take comment into account.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-nobuiltin.c
  clang/test/CodeGen/memcpy-nobuiltin.inc

Index: clang/test/CodeGen/memcpy-nobuiltin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.inc
@@ -0,0 +1,19 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
+#ifdef WITH_SELF_REFERENCE_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  if (n != 0)
+memcpy(dest, from, n);
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -UWITH_DECL | FileCheck --check-prefix=CHECK-NO-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_SELF_REFERENCE_DECL | FileCheck --check-prefix=CHECK-SELF-REF-DECL %s
+//
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+// CHECK-SELF-REF-DECL: @llvm.memcpy
+//
+#include 
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+
+  static char buffer[1];
+  memcpy(buffer, from, 2); // expected-warning {{'memcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr())
  F->setSection(SA->getName());
 
+  if (FD->isInlineBuiltinDeclaration()) {
+F->addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
 // A replaceable global allocation function does not act like a builtin by
 // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4596,8 +4596,15 @@
 }
 
 static CGCallee EmitDirectCallee(CodeGenFunction , const FunctionDecl *FD) {
+
   if (auto builtinID = FD->getBuiltinID()) {
-return CGCallee::forBuiltin(builtinID, FD);
+// Replaceable builtin provide their own implementation of a builtin. Unless
+// we are in the builtin implementation itself, don't call the actual
+// builtin. If we are in the builtin implementation, avoid trivial infinite
+// recursion.
+if (!FD->isInlineBuiltinDeclaration() ||
+CGF.CurFn->getName() == FD->getName())
+  return CGCallee::forBuiltin(builtinID, FD);
   }
 
   llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, FD);
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,24 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isInlineBuiltinDeclaration() const {
+  if (!getBuiltinID())
+return false;
+
+  const FunctionDecl *Definition;
+  if (hasBody(Definition)) {
+
+if (!Definition->isInlineSpecified())
+  return false;
+
+const SourceManager  = getASTContext().getSourceManager();
+SourceLocation SL = Definition->getLocation();
+if (SL.isValid())
+  return SM.isInSystemHeader(SL);
+  }
+  return false;
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides an inline implementation of a builtin.
+  bool isInlineBuiltinDeclaration() const;
+
   /// Determine whether this is a destroying operator delete.
   bool isDestroyingOperatorDelete() const;
 

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 233924.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-nobuiltin.c
  clang/test/CodeGen/memcpy-nobuiltin.inc

Index: clang/test/CodeGen/memcpy-nobuiltin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.inc
@@ -0,0 +1,19 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
+#ifdef WITH_SELF_REFERENCE_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  if (n != 0)
+memcpy(dest, from, n);
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -UWITH_DECL | FileCheck --check-prefix=CHECK-NO-DECL %s
+// RUN: %clang_cc1 -verify -S -emit-llvm -o- %s -isystem %S -DWITH_SELF_REFERENCE_DECL | FileCheck --check-prefix=CHECK-SELF-REF-DECL %s
+//
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+// CHECK-SELF-REF-DECL: @llvm.memcpy
+//
+#include 
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+
+  static char buffer[1];
+  memcpy(buffer, from, 2); // expected-warning {{'memcpy' will always overflow; destination buffer has size 1, but size argument is 2}}
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr())
  F->setSection(SA->getName());
 
+  if (FD->isInlineBuiltinDeclaration()) {
+F->addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
 // A replaceable global allocation function does not act like a builtin by
 // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4596,8 +4596,15 @@
 }
 
 static CGCallee EmitDirectCallee(CodeGenFunction , const FunctionDecl *FD) {
+
   if (auto builtinID = FD->getBuiltinID()) {
-return CGCallee::forBuiltin(builtinID, FD);
+// Replaceable builtin provide their own implementation of a builtin. Unless
+// we are in the builtin implementation itself, don't call the actual
+// builtin. If we are in the builtin implementation, avoid trivial infinite
+// recursion.
+if (!FD->isInlineBuiltinDeclaration() ||
+CGF.CurFn->getName() == FD->getName())
+  return CGCallee::forBuiltin(builtinID, FD);
   }
 
   llvm::Constant *calleePtr = EmitFunctionDeclPointer(CGF.CGM, FD);
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,14 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isInlineBuiltinDeclaration() const {
+  if (!getBuiltinID())
+return false;
+
+  const FunctionDecl *Definition;
+  return hasBody(Definition) && Definition->isInlineSpecified();
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides an inline implementation of a builtin.
+  bool isInlineBuiltinDeclaration() const;
+
   /// Determine whether this is a destroying operator delete.
   bool isDestroyingOperatorDelete() const;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71510: [clang][checkers] Added new checker 'error-return-checker'. (WIP)

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, mgorny.
Herald added a project: clang.

Currently work-in-progress.
This should check if the return values from certain C API functions
are checked for error. The list of functions is included.
A check for zero (or non-zero) error return value is used for every function.
The code is to be extended to work with other types of error conditions
and better bug report construction (at least indicate the place of the
function).

This check should implement SEI CERT C Coding Standard rule
"ERR33-C. Detect and handle standard library errors".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71510

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

Index: clang/test/Analysis/error-return.c
===
--- /dev/null
+++ clang/test/Analysis/error-return.c
@@ -0,0 +1,364 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.unix.ErrorReturn -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+/*
+Functions from CERT ERR33-C that should be checked for error:
+
+void *aligned_alloc( size_t alignment, size_t size );
+errno_t asctime_s(char *buf, rsize_t bufsz, const struct tm *time_ptr);
+int at_quick_exit( void (*func)(void) );
+int atexit( void (*func)(void) );
+void* bsearch( const void *key, const void *ptr, size_t count, size_t size,
+   int (*comp)(const void*, const void*) );
+void* bsearch_s( const void *key, const void *ptr, rsize_t count, rsize_t size,
+ int (*comp)(const void *, const void *, void *),
+ void *context );
+wint_t btowc( int c );
+size_t c16rtomb( char * restrict s, char16_t c16, mbstate_t * restrict ps );
+size_t c32rtomb( char * restrict s, char32_t c32, mbstate_t * restrict ps );
+void* calloc( size_t num, size_t size );
+clock_t clock(void);
+int cnd_broadcast( cnd_t *cond );
+int cnd_init( cnd_t* cond );
+int cnd_signal( cnd_t *cond );
+int cnd_timedwait( cnd_t* restrict cond, mtx_t* restrict mutex,
+   const struct timespec* restrict time_point );
+int cnd_wait( cnd_t* cond, mtx_t* mutex );
+errno_t ctime_s(char *buffer, rsize_t bufsz, const time_t *time);
+int fclose( FILE *stream );
+int fflush( FILE *stream );
+int fgetc( FILE *stream );
+int fgetpos( FILE *restrict stream, fpos_t *restrict pos );
+char *fgets( char *restrict str, int count, FILE *restrict stream );
+wint_t fgetwc( FILE *stream );
+FILE *fopen( const char *restrict filename, const char *restrict mode );
+errno_t fopen_s(FILE *restrict *restrict streamptr,
+const char *restrict filename,
+const char *restrict mode);
+int fprintf( FILE *restrict stream, const char *restrict format, ... );
+int fprintf_s(FILE *restrict stream, const char *restrict format, ...);
+int fputc( int ch, FILE *stream );
+int fputs( const char *restrict str, FILE *restrict stream );
+wint_t fputwc( wchar_t ch, FILE *stream );
+int fputws( const wchar_t * restrict str, FILE * restrict stream );
+size_t fread( void *restrict buffer, size_t size, size_t count,
+  FILE *restrict stream );
+FILE *freopen( const char *restrict filename, const char *restrict mode,
+   FILE *restrict stream );
+errno_t freopen_s(FILE *restrict *restrict newstreamptr,
+  const char *restrict filename, const char *restrict mode,
+  FILE *restrict stream);
+int fscanf( FILE *restrict stream, const char *restrict format, ... );
+int fscanf_s(FILE *restrict stream, const char *restrict format, ...);
+int fseek( FILE *stream, long offset, int origin );
+int fsetpos( FILE *stream, const fpos_t *pos );
+long ftell( FILE *stream );
+int fwprintf( FILE *restrict stream,
+  const wchar_t *restrict format, ... );
+int fwprintf_s( FILE *restrict stream,
+const wchar_t *restrict format, ...);
+size_t fwrite( const void *restrict buffer, size_t size, size_t count,
+   FILE *restrict stream ); // more exact error return: < count
+int fwscanf( FILE *restrict stream,
+ const wchar_t *restrict format, ... );
+int fwscanf_s( FILE *restrict stream,
+   const wchar_t *restrict format, ...);
+int getc( FILE *stream );
+int getchar(void);
+char *getenv( const char *name );
+errno_t getenv_s( size_t *restrict len, char *restrict value,
+  rsize_t valuesz, const char *restrict name );
+char *gets_s( char *str, rsize_t n );
+wint_t getwc( FILE *stream );
+wint_t getwchar(void);
+struct tm *gmtime( const time_t *time );
+struct tm *gmtime_s(const time_t *restrict time, struct tm *restrict result);
+struct tm *localtime( const time_t *time );

[PATCH] D71510: [clang][checkers] Added new checker 'error-return-checker'. (WIP)

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Code is to be reformatted later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71510



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

[clang] c2f1831 - Move ASTRecordReader into its own header; NFC.

[John McCall via cfe-commits]

Author: John McCall
Date: 2019-12-14T03:28:23-05:00
New Revision: c2f18315ff53006e44afe065368019e41cb98053

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

LOG: Move ASTRecordReader into its own header; NFC.

AbstractBasicReader.h has quite a few dependencies already,
and that's only likely to increase.  Meanwhile, ASTRecordReader
is really an implementation detail of the ASTReader that is only
used in a small number of places.

I've kept it in a public header for the use of projects like Swift
that might want to plug in to Clang's serialization framework.

I've also moved OMPClauseReader into an implementation file,
although it can't be made private because of friendship.

Added: 
clang/include/clang/Serialization/ASTRecordReader.h

Modified: 
clang/include/clang/Serialization/ASTReader.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/StaticAnalyzer/Frontend/ModelInjector.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTReader.h 
b/clang/include/clang/Serialization/ASTReader.h
index 3f321f03d966..e74bf00e0872 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -13,26 +13,16 @@
 #ifndef LLVM_CLANG_SERIALIZATION_ASTREADER_H
 #define LLVM_CLANG_SERIALIZATION_ASTREADER_H
 
-#include "clang/AST/AbstractBasicReader.h"
-#include "clang/AST/DeclCXX.h"
-#include "clang/AST/DeclObjC.h"
-#include "clang/AST/DeclarationName.h"
-#include "clang/AST/NestedNameSpecifier.h"
-#include "clang/AST/OpenMPClause.h"
-#include "clang/AST/TemplateBase.h"
-#include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/IdentifierTable.h"
-#include "clang/Basic/Module.h"
 #include "clang/Basic/OpenCLOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Version.h"
 #include "clang/Lex/ExternalPreprocessorSource.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PreprocessingRecord.h"
-#include "clang/Lex/Token.h"
 #include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Sema/IdentifierResolver.h"
 #include "clang/Serialization/ASTBitCodes.h"
@@ -40,9 +30,6 @@
 #include "clang/Serialization/ModuleFile.h"
 #include "clang/Serialization/ModuleFileExtension.h"
 #include "clang/Serialization/ModuleManager.h"
-#include "llvm/ADT/APFloat.h"
-#include "llvm/ADT/APInt.h"
-#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -58,8 +45,6 @@
 #include "llvm/ADT/iterator.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Bitstream/BitstreamReader.h"
-#include "llvm/Support/Casting.h"
-#include "llvm/Support/Endian.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/VersionTuple.h"
@@ -83,6 +68,7 @@ class ASTReader;
 class ASTRecordReader;
 class CXXTemporary;
 class Decl;
+class DeclarationName;
 class DeclaratorDecl;
 class DeclContext;
 class EnumDecl;
@@ -112,9 +98,8 @@ class SourceManager;
 class Stmt;
 class SwitchCase;
 class TargetOptions;
-class TemplateParameterList;
+class Token;
 class TypedefNameDecl;
-class TypeSourceInfo;
 class ValueDecl;
 class VarDecl;
 
@@ -2280,341 +2265,6 @@ class ASTReader
   bool isProcessingUpdateRecords() { return ProcessingUpdateRecords; }
 };
 
-/// An object for streaming information from a record.
-class ASTRecordReader
-: public serialization::DataStreamBasicReader {
-  using ModuleFile = serialization::ModuleFile;
-
-  ASTReader *Reader;
-  ModuleFile *F;
-  unsigned Idx = 0;
-  ASTReader::RecordData Record;
-
-  using RecordData = ASTReader::RecordData;
-  using RecordDataImpl = ASTReader::RecordDataImpl;
-
-public:
-  /// Construct an ASTRecordReader that uses the default encoding scheme.
-  ASTRecordReader(ASTReader , ModuleFile )
-: DataStreamBasicReader(Reader.getContext()), Reader(), F() {}
-
-  /// Reads a record with id AbbrevID from Cursor, resetting the
-  /// internal state.
-  Expected readRecord(llvm::BitstreamCursor ,
-unsigned AbbrevID);
-
-  /// Is this a module file for a module (rather than a PCH or similar).
-  bool isModule() const { return F->isModule(); }
-
-  /// Retrieve the AST context that this AST reader supplements.
-  ASTContext () { return Reader->getContext(); }
-
-  /// The current position in this record.
-  unsigned getIdx() const { return Idx; }
-
-  /// The length of this record.
-  size_t size() const { return Record.size(); }
-
-  /// An arbitrary index in this record.
-  const uint64_t [](size_t N) { return Record[N]; }
-
-  /// Returns the last 

[PATCH] D71374: Improve support of GNU mempcpy

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@xbolva00 : validation is ok: 
https://github.com/serge-sans-paille/llvm-project/pull/5/checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71374



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323
+Result = Builder.CreateIntrinsic(
+Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), 
Args.IntType},
+{SrcForMask, NegatedMask}, nullptr, "aligned_result");

Is sufficient amount of passes, analyses know about this intrinsic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

ping @rsmith


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

https://reviews.llvm.org/D63960



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Reverted in f0604e73a4daa35a10eb17a998657d6c4bd0e971 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[PATCH] D71345: [clangd] Fall back to selecting token-before-cursor if token-after-cursor fails.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks compile on most bots in the "clang" section on 
http://lab.llvm.org:8011/console , probably the ones that use gcc as host 
compiler.

Here's an example:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-selfhost-neon/builds/2714/steps/build%20stage%201/logs/stdio


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71345



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

[clang-tools-extra] 687e98d - Fix build with older (still supported) gcc versions.

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2019-12-14T10:13:15-05:00
New Revision: 687e98d294c4f77e8b431adb7d86dfba5ab84645

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

LOG: Fix build with older (still supported) gcc versions.

Older gccs can't handle multiline raw string literals in
macro parameters.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/FormattedStringTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp 
b/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
index 7d57be61f0b2..89ffab6aac0c 100644
--- a/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FormattedStringTests.cpp
@@ -120,16 +120,20 @@ TEST(Document, Separators) {
   D.addParagraph().appendText("foo");
   D.addCodeBlock("test");
   D.addParagraph().appendText("bar");
-  EXPECT_EQ(D.asMarkdown(), R"md(foo
+
+  const char ExpectedMarkdown[] = R"md(foo
 ```cpp
 test
 ```
-bar)md");
-  EXPECT_EQ(D.asPlainText(), R"pt(foo
+bar)md";
+  EXPECT_EQ(D.asMarkdown(), ExpectedMarkdown);
+
+  const char ExpectedText[] = R"pt(foo
 
 test
 
-bar)pt");
+bar)pt";
+  EXPECT_EQ(D.asPlainText(), ExpectedText);
 }
 
 TEST(Document, Spacer) {



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

[PATCH] D71503: New warning on for-loops that never run because condition is false on the first iteration

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for working on this!
This seems to be missing tests.


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

https://reviews.llvm.org/D71503



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Alexander Richardson via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323
+Result = Builder.CreateIntrinsic(
+Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), 
Args.IntType},
+{SrcForMask, NegatedMask}, nullptr, "aligned_result");

lebedev.ri wrote:
> Is sufficient amount of passes, analyses know about this intrinsic?
Good question. In the simple test cases that I looked at the code generation 
was equivalent. 

In our fork we still use ptrtoint+inttoptr since I implemented them before the 
new intrinsic existed. But since the ptrmask instrinsic exists now I thought 
I'd use it for upstreaming.
I'll investigate if this results in worse codegen for more complex uses.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D69475: [clang] Provide better fix-it on exception spec error

[Jeremy Demeule via Phabricator via cfe-commits]

jdemeule added a comment.

Kindly ping reviewer.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69475



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

[PATCH] D69282: [RFC] Add a VCS conflict marker format printing on tooling::ReplacementError

[Jeremy Demeule via Phabricator via cfe-commits]

jdemeule added a comment.

Kindly ping reviewers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69282



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

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

[Pablo Martin-Gomez via Phabricator via cfe-commits]

Bouska created this revision.
Bouska added reviewers: djasper, klimek, krasimir.
Bouska added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes bug #44192

When clang-format is run with option AllowShortBlocksOnASingleLine, it is 
expected to either succeed in putting the short block with its control 
statement on a single line or fail and leave the block as is. When brace 
wrapping after control statement is activated, if the block + the control 
statement length is superior to column limit but the block alone is not, 
clang-format puts the block in two lines: one for the control statement and one 
for the block. This patch removes this unexpected behaviour. Current unittests 
are updated to check for this behaviour.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71512

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -583,7 +583,7 @@
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
-   "  ff();\n"
+   "  ff();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true) { //\n"
@@ -659,7 +659,7 @@
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
-   "  ff();\n"
+   "  ff();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
@@ -721,7 +721,9 @@
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
 "  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
-"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"  {\\\n"
+"RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier;   \\\n"
+"  }\n"
 "X;",
 format("#define A \\\n"
"   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -327,21 +327,6 @@
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;
 }
-// Try to merge either empty or one-line block if is precedeed by control
-// statement token
-if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
-I != AnnotatedLines.begin() &&
-I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  unsigned MergedLines = 0;
-  if (Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never) {
-MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
-// If we managed to merge the block, discard the first merged line
-// since we are merging starting from I.
-if (MergedLines > 0)
-  --MergedLines;
-  }
-  return MergedLines;
-}
 // Don't merge block with left brace wrapped after ObjC special blocks
 if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
 I[-1]->First->is(tok::at) && I[-1]->First->Next) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -583,7 +583,7 @@
   verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
-   "  ff();\n"
+   "  ff();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true) { //\n"
@@ -659,7 +659,7 @@
   verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
"{\n"
-   "  ff();\n"
+   "  ff();\n"
"}",
AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
@@ -721,7 +721,9 @@
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
 "  if 

[PATCH] D71507: [perf-training] Make training data location configurable

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71507



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

[PATCH] D71414: [clangd] Introduce codeblocks

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FormattedStringTests.cpp:126
+test
+```
+bar)md");

Older (but still supported) gccs can't handle multiline raw strings in macro 
arguments, see e.g. 
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/21230/steps/ninja%20check%201/logs/stdio

I fixed this for you in 687e98d294c4f77e. It's been broken for 3 days, please 
watch bots and your inbox after committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71414



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323
+Result = Builder.CreateIntrinsic(
+Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), 
Args.IntType},
+{SrcForMask, NegatedMask}, nullptr, "aligned_result");

arichardson wrote:
> lebedev.ri wrote:
> > Is sufficient amount of passes, analyses know about this intrinsic?
> Good question. In the simple test cases that I looked at the code generation 
> was equivalent. 
> 
> In our fork we still use ptrtoint+inttoptr since I implemented them before 
> the new intrinsic existed. But since the ptrmask instrinsic exists now I 
> thought I'd use it for upstreaming.
> I'll investigate if this results in worse codegen for more complex uses.
> 
(TLDR: before producing it in more cases in clang, i think it should be first 
ensured
that everything in middle-end is fully aware of said intrinsic. (i.e. using it 
vs it's
exploded form results in no differences in final assembly on a sufficient test 
coverage))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D71320: [IR] Split out target specific intrinsic enums into separate headers

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I think this broke the reverse-iteration bot: 
http://lab.llvm.org:8011/builders/reverse-iteration/builds/15619

In D71320#1782204 , @rnk wrote:

> In D71320#1780805 , @thakis wrote:
>
> > Any reason these are called .h? All our other tablegen outputs are called 
> > .inc.
>
>
> Yes, they have header guards, they are not textual. Most other tablegen 
> outputs are intended to be used with some kind of xmacro pattern, where the 
> includer sets a macro before including the file. I could've structured things 
> so that there is:
>
> - a per-target .inc file in build/include/llvm/IR/
> - a per-target .h file in llvm/include/llvm/IR, not generated
>
>   But I felt that it was less boilerplate and code to have tablegen splat out 
> the .h file and include that from .cpp files directly.


Makes sense, but it's unusual.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71320



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

Re: [clang] c2f1831 - Move ASTRecordReader into its own header; NFC.

[Nico Weber via cfe-commits]

One of the commits in this series, likely
https://github.com/llvm/llvm-project/commit/d505e57cc273750541ec8bbce2065b8b87c99ad6
(which for some reason doesn't show up on cfe-commits?)
broke Modules/merge-lifetime-extended-temporary.cpp when using a clang
built at r365097 as host compiler. (Full error log:
http://45.33.8.238/linux/5720/step_7.txt , also copied to the bottom of
this mail for indexing.)

I tried the test with a few other clangs as host compilers and everything
passes (I tried 362913, 369647, and the 9.0 release clang), so this very
likely isn't worth looking into more. But in case others see this failure,
maybe this note helps them.

Nico



Full error:

FAIL: Clang :: Modules/merge-lifetime-extended-temporary.cpp (6905 of 16551)
 TEST 'Clang ::
Modules/merge-lifetime-extended-temporary.cpp' FAILED 
Script:
--
: 'RUN: at line 1';
/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang -cc1
-internal-isystem
/usr/local/google/home/thakis/src/llvm-project/out/gn/lib/clang/10.0.0/include
-nostdsysteminc -fmodules -fimplicit-module-maps
-fmodules-cache-path=/usr/local/google/home/thakis/src/llvm-project/out/gn/obj/clang/test/Modules/Output/merge-lifetime-extended-temporary.cpp.tmp
-x c++
-I/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/Inputs/merge-lifetime-extended-temporary
-verify -std=c++11
/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/merge-lifetime-extended-temporary.cpp
-DORDER=1
: 'RUN: at line 2';
/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang -cc1
-internal-isystem
/usr/local/google/home/thakis/src/llvm-project/out/gn/lib/clang/10.0.0/include
-nostdsysteminc -fmodules -fimplicit-module-maps
-fmodules-cache-path=/usr/local/google/home/thakis/src/llvm-project/out/gn/obj/clang/test/Modules/Output/merge-lifetime-extended-temporary.cpp.tmp
-x c++
-I/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/Inputs/merge-lifetime-extended-temporary
-verify -std=c++11
/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/merge-lifetime-extended-temporary.cpp
-DORDER=2
--
Exit Code: 134

Command Output (stderr):
--
clang: ../../clang/include/clang/AST/Type.h:666: const
clang::ExtQualsTypeCommonBase *clang::QualType::getCommonPtr() const:
Assertion `!isNull() && "Cannot retrieve a NULL type pointer"' failed.
Stack dump:
0. Program arguments:
/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang -cc1
-internal-isystem
/usr/local/google/home/thakis/src/llvm-project/out/gn/lib/clang/10.0.0/include
-nostdsysteminc -fmodules -fimplicit-module-maps
-fmodules-cache-path=/usr/local/google/home/thakis/src/llvm-project/out/gn/obj/clang/test/Modules/Output/merge-lifetime-extended-temporary.cpp.tmp
-x c++
-I/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/Inputs/merge-lifetime-extended-temporary
-verify -std=c++11
/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/merge-lifetime-extended-temporary.cpp
-DORDER=1
1.
/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/merge-lifetime-extended-temporary.cpp:13:24:
current parser token '=='
 #0 0x03e57cbd PrintStackTraceSignalHandler(void*)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x3e57cbd)
 #1 0x03e55a0e llvm::sys::RunSignalHandlers()
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x3e55a0e)
 #2 0x03e57e7c SignalHandler(int)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x3e57e7c)
 #3 0x7fa76931f3a0 __restore_rt
(/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
 #4 0x7fa7689e6cfb raise
/build/glibc-XAwaOT/glibc-2.28/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x7fa7689d18ad abort
/build/glibc-XAwaOT/glibc-2.28/stdlib/abort.c:81:7
 #6 0x7fa7689d177f get_sysdep_segment_value
/build/glibc-XAwaOT/glibc-2.28/intl/loadmsgcat.c:509:8
 #7 0x7fa7689d177f _nl_load_domain
/build/glibc-XAwaOT/glibc-2.28/intl/loadmsgcat.c:970:34
 #8 0x7fa7689df542 (/lib/x86_64-linux-gnu/libc.so.6+0x2f542)
 #9 0x02fce25a clang::ASTContext::getCanonicalType(clang::QualType)
const
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x2fce25a)
#10 0x0455ebff
clang::ASTContext::getLValueReferenceType(clang::QualType, bool) const
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x455ebff)
#11 0x054064a5 clang::ASTReader::readTypeRecord(unsigned int)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x54064a5)
#12 0x053fb305 clang::ASTReader::GetType(unsigned int)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x53fb305)
#13 0x0544a0fe
clang::ASTDeclReader::VisitValueDecl(clang::ValueDecl*)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x544a0fe)
#14 0x0544a260
clang::ASTDeclReader::VisitDeclaratorDecl(clang::DeclaratorDecl*)

Re: [clang] c2f1831 - Move ASTRecordReader into its own header; NFC.

[John McCall via cfe-commits]

On 14 Dec 2019, at 14:37, Nico Weber wrote:

One of the commits in this series, likely
https://github.com/llvm/llvm-project/commit/d505e57cc273750541ec8bbce2065b8b87c99ad6
(which for some reason doesn't show up on cfe-commits?)
broke Modules/merge-lifetime-extended-temporary.cpp when using a clang
built at r365097 as host compiler. (Full error log:
http://45.33.8.238/linux/5720/step_7.txt , also copied to the bottom 
of

this mail for indexing.)

I tried the test with a few other clangs as host compilers and 
everything
passes (I tried 362913, 369647, and the 9.0 release clang), so this 
very
likely isn't worth looking into more. But in case others see this 
failure,

maybe this note helps them.


Weird.  Thanks for letting me know.

John.



Nico



Full error:

FAIL: Clang :: Modules/merge-lifetime-extended-temporary.cpp (6905 of 
16551)

 TEST 'Clang ::
Modules/merge-lifetime-extended-temporary.cpp' FAILED 


Script:
--
: 'RUN: at line 1';
/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang -cc1
-internal-isystem
/usr/local/google/home/thakis/src/llvm-project/out/gn/lib/clang/10.0.0/include
-nostdsysteminc -fmodules -fimplicit-module-maps
-fmodules-cache-path=/usr/local/google/home/thakis/src/llvm-project/out/gn/obj/clang/test/Modules/Output/merge-lifetime-extended-temporary.cpp.tmp
-x c++
-I/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/Inputs/merge-lifetime-extended-temporary
-verify -std=c++11
/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/merge-lifetime-extended-temporary.cpp
-DORDER=1
: 'RUN: at line 2';
/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang -cc1
-internal-isystem
/usr/local/google/home/thakis/src/llvm-project/out/gn/lib/clang/10.0.0/include
-nostdsysteminc -fmodules -fimplicit-module-maps
-fmodules-cache-path=/usr/local/google/home/thakis/src/llvm-project/out/gn/obj/clang/test/Modules/Output/merge-lifetime-extended-temporary.cpp.tmp
-x c++
-I/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/Inputs/merge-lifetime-extended-temporary
-verify -std=c++11
/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/merge-lifetime-extended-temporary.cpp
-DORDER=2
--
Exit Code: 134

Command Output (stderr):
--
clang: ../../clang/include/clang/AST/Type.h:666: const
clang::ExtQualsTypeCommonBase *clang::QualType::getCommonPtr() const:
Assertion `!isNull() && "Cannot retrieve a NULL type pointer"' failed.
Stack dump:
0. Program arguments:
/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang -cc1
-internal-isystem
/usr/local/google/home/thakis/src/llvm-project/out/gn/lib/clang/10.0.0/include
-nostdsysteminc -fmodules -fimplicit-module-maps
-fmodules-cache-path=/usr/local/google/home/thakis/src/llvm-project/out/gn/obj/clang/test/Modules/Output/merge-lifetime-extended-temporary.cpp.tmp
-x c++
-I/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/Inputs/merge-lifetime-extended-temporary
-verify -std=c++11
/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/merge-lifetime-extended-temporary.cpp
-DORDER=1
1.
/usr/local/google/home/thakis/src/llvm-project/clang/test/Modules/merge-lifetime-extended-temporary.cpp:13:24:
current parser token '=='
 #0 0x03e57cbd PrintStackTraceSignalHandler(void*)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x3e57cbd)
 #1 0x03e55a0e llvm::sys::RunSignalHandlers()
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x3e55a0e)
 #2 0x03e57e7c SignalHandler(int)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x3e57e7c)
 #3 0x7fa76931f3a0 __restore_rt
(/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
 #4 0x7fa7689e6cfb raise
/build/glibc-XAwaOT/glibc-2.28/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
 #5 0x7fa7689d18ad abort
/build/glibc-XAwaOT/glibc-2.28/stdlib/abort.c:81:7
 #6 0x7fa7689d177f get_sysdep_segment_value
/build/glibc-XAwaOT/glibc-2.28/intl/loadmsgcat.c:509:8
 #7 0x7fa7689d177f _nl_load_domain
/build/glibc-XAwaOT/glibc-2.28/intl/loadmsgcat.c:970:34
 #8 0x7fa7689df542 (/lib/x86_64-linux-gnu/libc.so.6+0x2f542)
 #9 0x02fce25a 
clang::ASTContext::getCanonicalType(clang::QualType)

const
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x2fce25a)
#10 0x0455ebff
clang::ASTContext::getLValueReferenceType(clang::QualType, bool) const
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x455ebff)
#11 0x054064a5 clang::ASTReader::readTypeRecord(unsigned int)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x54064a5)
#12 0x053fb305 clang::ASTReader::GetType(unsigned int)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x53fb305)
#13 0x0544a0fe
clang::ASTDeclReader::VisitValueDecl(clang::ValueDecl*)
(/usr/local/google/home/thakis/src/llvm-project/out/gn/bin/clang+0x544a0fe)
#14 0x0544a260

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

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze marked 4 inline comments as done.
zukatsinadze added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:765
+  HelpText<"Finds calls to the `putenv` function which pass a pointer to "
+   "an automatic variable as the argument. (CERT POS 34C)">,
+  Documentation;

Charusso wrote:
> I would write ##`putenv`## -> `'putenv'` and the CERT rule-number should be 
> clear from that point so you could emit it.
Oops. Forgot this one. Will fix it later.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp:45-52
+  if (const auto *DRE = dyn_cast(ArgExpr->IgnoreImpCasts()))
+if (const auto *VD = dyn_cast(DRE->getDecl()))
+  IsPossiblyAutoVar = isa(VD) && isa(MSR);
+
+  if (!IsPossiblyAutoVar &&
+  (isa(MSR) || isa(MSR) ||
+   isa(MSR) ||

NoQ wrote:
> Simply check whether the memory space is a stack memory space. This should be 
> a one-liner.
I could not get rid of `isPossiblyAutoVar` and `GlobalInternalSpaceRegion`. 



Comment at: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp:15
+int volatile_memory1(char *a) {
+  return putenv(a);
+  // expected-warning@-1 {{'putenv' function should not be called with auto 
variables}}

I need `isPossiblyAutoVar` for this type. 



Comment at: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp:20
+void volatile_memory2(char *a) {
+  char *buff = (char *)"hello";
+  putenv(buff);

And `GlobalInternalSpaceRegion` for this.


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

https://reviews.llvm.org/D71433



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

[clang] 357e64e - [cxx_status] Fix paper number for "Concept auto" paper.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-14T14:53:05-08:00
New Revision: 357e64e95267de3dfc64b5563dec2df84e6cce0e

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

LOG: [cxx_status] Fix paper number for "Concept auto" paper.

Added: 


Modified: 
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index ffac33387960..47f58727def8 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -890,7 +890,7 @@ C++2a implementation status
 https://wg21.link/p1084r2;>P1084R2
   
   
-https://wg21.link/p1141r2;>P1414R2
+https://wg21.link/p1141r2;>P1141R2
   

 https://wg21.link/p0848r3;>P0848R3



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

[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.

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

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

Only a few non-nit comments; if you feel confident addressing those, please 
feel free to commit after doing so. (If you'd like another round of review, let 
me know.) Thanks!




Comment at: clang/include/clang/AST/DeclTemplate.h:796-799
+  template 
+  typename SpecEntryTraits::DeclType*
   findSpecializationImpl(llvm::FoldingSetVector ,
+ void *, ProfileArguments&&... ProfileArgs);

Formatting nits:

`typename ...ProfileArguments`
`ProfileArguments &&...ProfileArgs`



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2507
+def note_could_not_normalize_argument_substitution_failed : Note<
+  "while substituting into %0 %1; make sure no invalid expressions/types form "
+  "in concept arguments">;

Consider adding "during constraint normalization" before the semicolon.

Giving advice here ("make sure [...]") isn't consistent with our normal 
diagnostic style, which is to present facts. I wonder if we can rephrase this 
as a fact? Something like "while substituting into %0 %1; constraint 
normalization is not a SFINAE context" would work -- though I don't think we 
mention SFINAE in any diagnostics yet either, and I'd prefer not to, if 
possible.

Maybe just "while substituting into %0 %1 during constraint normalization" is 
enough; the fact that we're issuing an error for a substitution failure seems 
to strongly imply that substitution failure in this context is an error. What 
do you think?



Comment at: clang/include/clang/Sema/Sema.h:6036
+  void DiagnoseRedeclarationConstraintMismatch(const TemplateParameterList 
*Old,
+  const TemplateParameterList 
*New);
+

Nit: underindented.



Comment at: clang/lib/AST/DeclTemplate.cpp:432
 ClassTemplateDecl::findPartialSpecialization(ArrayRef Args,
- void *) {
-  return findSpecializationImpl(getPartialSpecializations(), Args, InsertPos);
+TemplateParameterList *TPL, void *) {
+  return findSpecializationImpl(getPartialSpecializations(), InsertPos, Args,

Nit: parameters after a newline should start in the same column as the first 
parameter. (Either flow `Args` onto a new line or indent these parameters to 
line up with it.)



Comment at: clang/lib/AST/DeclTemplate.cpp:445-463
+if (const auto *NTTP = dyn_cast(D)) {
+  ID.AddInteger(NTTP->getDepth());
+  ID.AddInteger(NTTP->getIndex());
+  ID.AddBoolean(NTTP->isParameterPack());
+  NTTP->getType().getCanonicalType().Profile(ID);
+  continue;
+}

Should we profile the kind of the template parameter (non-type/type/template) 
here? The profiling for the different kinds seems like it might not actually 
collide in practice, but that looks like it's happening by luck rather than by 
design.

Conversely, can we remove the profiling of the depth and index? That seems like 
it should be implied by the position of the parameter in the profiling data.



Comment at: clang/lib/Sema/SemaConcept.cpp:417
+  AtomicConstraint(Sema , const Expr *ConstraintExpr) :
+  ConstraintExpr{ConstraintExpr} { };
+

Nit: we generally don't use direct-list-initialization. Use parens here.



Comment at: clang/lib/Sema/SemaConcept.cpp:528-569
+struct OccurringTemplateParameterFinder :
+RecursiveASTVisitor {
+  llvm::SmallBitVector 
+
+  OccurringTemplateParameterFinder(llvm::SmallBitVector )
+  : OccurringIndices(OccurringIndices) { }
+

You can use `Sema::MarkUsedTemplateParameters` for this.

... oh, except that it's broken for this case and has a FIXME to walk the full 
expression in `OnlyDeduced = false` mode:

```
  // FIXME: if !OnlyDeduced, we have to walk the whole subexpression to
  // find other occurrences of template parameters.
```

Oops. Please either move this into `MarkUsedTemplateParameters` and call that 
from here, or at least add a FIXME here to remind us to do so.



Comment at: clang/lib/Sema/SemaTemplate.cpp:10521-10526
+
+// p1
+//   template-parameter:
+// ...
+// parameter-declaration
+// p2

Presumably this is unintentional / left behind from editing?



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2505
+TemplateArgumentLoc
+Sema::getIdentityTemplateArgumentLoc(Decl *TemplateParm) {
+  if (auto *TTP = dyn_cast(TemplateParm))

Is there any code that can be shared between this and 
`ASTContext::getInjectedTemplateArg`? (I think probably not, because that 
creates pack expansions and we don't want them here, but it seems worth 
considering.)



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2514
+  auto 

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

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze added a comment.

In D71433#1784238 , @NoQ wrote:

> Thanks! This looks like a simple and efficient check. I have one overall 
> suggestion.
>
> Currently the check may warn on non-bugs of the following kind:
>
>   void foo() {
> char env[] = "NAME=value";
> putenv(env);
> doStuff();
> putenv("NAME=anothervalue");
>   }
>
>
> I.e., it's obviously harmless if the local variable pointer is removed from 
> the environment before it goes out of scope. Can we instead warn when the 
> *last* `putenv()` on the execution path through the current stack frame is a 
> local variable (that goes out of scope at the end of the stack frame)?
>
> That'd allow the checker to be enabled by default, which will give a lot more 
> users access to it. Otherwise we'll have to treat it as an opt-in check and 
> users will only enable it when they know about it, which is much less 
> usefulness.


@NoQ I like the idea, but I am not really sure how to do that. I started 
working on Static Analyzer just lask week.


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

https://reviews.llvm.org/D71433



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

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

rsmith added a comment.

To me these seem sufficiently useful to be worth having. Please add 
documentation for them.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2913
+def warn_alignment_builtin_useless : Warning<
+  "%select{aligning a value|checking whether a value is aligned}0 to 1 byte is"
+   " %select{a no-op|always true}0">, InGroup;

"the result of checking whether [...] is always true" would be better.



Comment at: clang/lib/AST/ExprConstant.cpp:10546-10548
+  if (!E->getArg(0)->EvaluateAsInt(ExprResult, Info.Ctx))
+return false;
+  Val = ExprResult.Val.getInt();

rsmith wrote:
> These should be evaluated in the same `EvalInfo` as the enclosing evaluation, 
> so they can use (eg) the values of local state that's already been computed 
> in a constexpr function.
There are also cases where the first argument is a pointer for which we should 
be able to constant-fold `__builtin_is_aligned` (if we can evaluate it to a 
specific offset within a specific object, and we know the alignment for that 
object as a whole). See the handling for `__builtin_assume_aligned`, and factor 
out the common part.



Comment at: clang/lib/AST/ExprConstant.cpp:10546-10550
+  if (!E->getArg(0)->EvaluateAsInt(ExprResult, Info.Ctx))
+return false;
+  Val = ExprResult.Val.getInt();
+  if (!E->getArg(1)->EvaluateAsInt(ExprResult, Info.Ctx))
+return false;

These should be evaluated in the same `EvalInfo` as the enclosing evaluation, 
so they can use (eg) the values of local state that's already been computed in 
a constexpr function.



Comment at: clang/lib/AST/ExprConstant.cpp:10553
+  if (Alignment < 0)
+return false;
+  // XXX: can this ever happen? Will we end up here even if Sema gives an 
error?

You need to produce a diagnostic on failure.



Comment at: clang/lib/AST/ExprConstant.cpp:10558
+return false;
+  // Ensure both values have the same bit width so that we don't assert later.
+  *ValWidth = Val.getBitWidth();

Would it make more sense to convert integer arguments to `intptr_t` when 
checking the call to the builtin?



Comment at: clang/lib/Sema/SemaChecking.cpp:222
+  QualType SrcTy = Source->getType();
+  // Should also be able to use it with arrays (but not functions!)
+  bool IsArrayToPointerDecay =

You should actually decay the argument to a pointer here. More generally, the 
right pattern to use here is:

1) Compute the parameter type you want the builtin to have for this call, then
2) Perform a copy-initialization of a parameter with that type from the 
argument, then
3) Update TheCall's corresponding argument to the converted form.

You can find this pattern in various builtin checking functions in this file if 
you want something to crib from. (Search for `InitializeParameter`.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D71433#1784920 , @zukatsinadze 
wrote:

> @NoQ I like the idea, but I am not really sure how to do that. I started 
> working on Static Analyzer just lask week.


Let's get the initial attempt right first, and delay this for the next patch. 
You could accomplish this by keeping track of the last `putenv()` in a program 
state trait and moving the warning in `checkEndFunction()`.




Comment at: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp:15
+int volatile_memory1(char *a) {
+  return putenv(a);
+  // expected-warning@-1 {{'putenv' function should not be called with auto 
variables}}

zukatsinadze wrote:
> I need `isPossiblyAutoVar` for this type. 
This test is pretty questionable. There is no indication in the code that `a` 
points to an automatic variable.



Comment at: clang/test/Analysis/cert/pos34-c-fp-suppression.cpp:20
+void volatile_memory2(char *a) {
+  char *buff = (char *)"hello";
+  putenv(buff);

zukatsinadze wrote:
> And `GlobalInternalSpaceRegion` for this.
This test is wrong. `"hello"` is not an automatic variable.


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

https://reviews.llvm.org/D71433



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

Re: [clang] 5708f2d - [clang] Fix modules build after addition of TypeBitCodes.def

[John McCall via cfe-commits]

On 14 Dec 2019, at 12:25, Raphael Isemann via cfe-commits wrote:

Author: Raphael Isemann
Date: 2019-12-14T18:24:53+01:00
New Revision: 5708f2daf7386ef5f1ec54db4bda2b48bbcbe934

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


LOG: [clang] Fix modules build after addition of TypeBitCodes.def

In revision 139006ceb641f038a2b19cac1174316e57004ed6 the Serialization
folder got its first def file 'TypeBitCodes.def'. This broke the
modules build as this .def file was not textually included but 
implicitly

converted into a module due to our umbrella directive.

This patch fixes this by explicitly marking the .def file as textual.


Sorry, thanks!

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

[PATCH] D71026: Fix LLVM_ENABLE_MODULES=ON + BUILD_SHARED_LIBS=ON build

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

rsmith added a comment.

Please can you describe the problem that this is fixing? Without that, it's 
hard to know whether this is the appropriate fix. Is there a modules buildbot 
that's failing, that we could look at the output from?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71026



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

[PATCH] D71508: [DebugInfo] Duplicate file names in debug info

[David Stenberg via Phabricator via cfe-commits]

dstenb added a comment.

It should be possible to create a test for this using something like:

  RUN: rm -rf %t && mkdir %t
  RUN: cp %s %t/filename.c
  RUN: cd %t
  RUN: clang ./filename.c [...]




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71508



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

[PATCH] D71512: [clang-format] Fix short block when braking after control statement

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:586
   verifyFormat("if (true) {\n"
-   "  ff();\n"
"}",

any reason you are changing existing tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71512



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

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

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze updated this revision to Diff 233946.
zukatsinadze added a comment.

Addressed most of the inline comments.


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

https://reviews.llvm.org/D71433

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

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

[PATCH] D43357: [Concepts] Function trailing requires clauses

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

rsmith added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:179-180
 def err_unexpected_semi : Error<"unexpected ';' before %0">;
+def err_expected_primary_got_unary : Error<
+  "expected primary expression before %0; did you forget parentheses?">;
+def note_potential_bin_op_in_constraint_logical_or : Note<

I think the "did you forget parentheses?" part here may be irritating to users 
who didn't know they needed parentheses at all -- it may be read as patronizing 
by some. Can we rephrase as "parentheses are required around a top-level unary 
%0 expression in a requires clause" or something like that?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2515
+  "'%0' in the two declarations is not considered equivalent - move it to a "
+  "concept and reference it from here:">;
+def note_ambiguous_atomic_constraints_second : Note<"and here">;

Don't use a colon to indicate a location. Diagnostics are formatted in many 
different ways and this won't always make sense. Also, where possible, don't 
pretty-print expressions in diagnostics; underline the source range instead.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2516
+  "concept and reference it from here:">;
+def note_ambiguous_atomic_constraints_second : Note<"and here">;
 

Similarly, this note presentation won't always make sense. Can we somehow 
rephrase these notes so they're more presenting the facts and less telling the 
user what to do?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3886-3895
+def note_ovl_candidate_constraints_not_satisfied : Note<
+"candidate %select{function|function|constructor|"
+"function|function|constructor|"
+"constructor (the implicit default constructor)|"
+"constructor (the implicit copy constructor)|"
+"constructor (the implicit move constructor)|"
+"function (the implicit copy assignment operator)|"

Don't repeat this `%select`; use `%sub{select_ovl_candidate_kind}` instead. 
(The enum and the appropriate `%select` have already changed at least twice 
since this was copy-pasted...)



Comment at: clang/include/clang/Parse/Parser.h:1652
 prec::Level MinPrec);
-  ExprResult ParseCastExpression(bool isUnaryExpression,
+  /// CastParseOption - Control what ParseCastExpression will parse.
+  enum CastParseOption {

Don't repeat the name of the entity in the doc comment. (This is an old style 
that we're phasing out; new code shouldn't do it.)



Comment at: clang/include/clang/Parse/Parser.h:1653
+  /// CastParseOption - Control what ParseCastExpression will parse.
+  enum CastParseOption {
+AnyCastExpr = 0,

We would usually call this sort of enumeration `SomethingKind` not 
`SomethingOption`.



Comment at: clang/include/clang/Parse/Parser.h:1658
+  };
+  ExprResult ParseCastExpression(CastParseOption ExprType,
  bool isAddressOfOperand,

Please don't use `Type` as the name of something that's not a language-level 
type.



Comment at: clang/include/clang/Parse/Parser.h:2700
+  void InitCXXThisScopeForDeclaratorIfRelevant(const Declarator ,
+  const DeclSpec , llvm::Optional );
   bool ParseRefQualifier(bool ,

Line-wrapped parameters should start on the same column as the first parameter.



Comment at: clang/lib/Parse/ParseDecl.cpp:2068-2069
 
   if (ParseAsmAttributesAfterDeclarator(D))
 return nullptr;
 

We should check with GCC to see which side of the requires-clause they expect 
this.

For the second and subsequent declarations, we expect the asm label before the 
requires clause; here we parse the asm label after the requires clause.



Comment at: clang/lib/Parse/ParseDecl.cpp:2257-2258
+  //declarator requires-clause
+  if (Tok.is(tok::kw_requires))
+ParseTrailingRequiresClause(D);
+

We already parsed this for the first declarator in the group. Do we 
accidentally permit two requires clauses on the first declarator?



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3799-3810
+if (D.isFunctionDeclarator()) {
+  auto  = D.getFunctionTypeInfo();
+  if (FTI.Params)
+for (auto  : ArrayRef(FTI.Params,
+
FTI.NumParams)){
+  auto *ParamDecl = cast(Param.Param);
+  if (ParamDecl->getIdentifier())

This scope-building code should be in `Sema`, not in the parser. (Consider 
adding an `ActOnStartTrailingRequiresClause` / 
`ActOnFinishTrailingRequiresClause` pair.)



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3801
+  auto  = D.getFunctionTypeInfo();
+   

[PATCH] D71513: [compiler-rt] [test] Disable MPROTECT on two builtin tests

[Michał Górny via Phabricator via cfe-commits]

mgorny created this revision.
mgorny added reviewers: krytarowski, rnk, samsonov, howard.hinnant.
Herald added subscribers: llvm-commits, dberris.
Herald added a project: LLVM.

Introduce a new %paxctl_nomprotect substitution and call it on two
builtin tests that do not work with MPROTECT on NetBSD.  This
substitution evaluates to no-op command (':') on other systems.


https://reviews.llvm.org/D71513

Files:
  compiler-rt/test/builtins/Unit/clear_cache_test.c
  compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
  compiler-rt/test/lit.common.cfg.py


Index: compiler-rt/test/lit.common.cfg.py
===
--- compiler-rt/test/lit.common.cfg.py
+++ compiler-rt/test/lit.common.cfg.py
@@ -504,3 +504,6 @@
  "test", "sanitizer_common", "netbsd_commands")
   config.netbsd_noaslr_prefix = ('sh ' +
  os.path.join(nb_commands_dir, 
'run_noaslr.sh'))
+  config.substitutions.append( ('%paxctl_nomprotect', '/usr/sbin/paxctl +m') )
+else:
+  config.substitutions.append( ('%paxctl_nomprotect', ':') )
Index: compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
===
--- compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
+++ compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
@@ -1,5 +1,5 @@
 // REQUIRES: native-run
-// RUN: %clang_builtins %s %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %paxctl_nomprotect %t && %run %t
 // REQUIRES: librt_has_enable_execute_stack
 //===-- enable_execute_stack_test.c - Test __enable_execute_stack 
--===//
 //
Index: compiler-rt/test/builtins/Unit/clear_cache_test.c
===
--- compiler-rt/test/builtins/Unit/clear_cache_test.c
+++ compiler-rt/test/builtins/Unit/clear_cache_test.c
@@ -1,6 +1,6 @@
 // REQUIRES: native-run
 // UNSUPPORTED: arm, aarch64
-// RUN: %clang_builtins %s %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %paxctl_nomprotect %t && %run %t
 // REQUIRES: librt_has_clear_cache
 //===-- clear_cache_test.c - Test clear_cache 
-===//
 //


Index: compiler-rt/test/lit.common.cfg.py
===
--- compiler-rt/test/lit.common.cfg.py
+++ compiler-rt/test/lit.common.cfg.py
@@ -504,3 +504,6 @@
  "test", "sanitizer_common", "netbsd_commands")
   config.netbsd_noaslr_prefix = ('sh ' +
  os.path.join(nb_commands_dir, 'run_noaslr.sh'))
+  config.substitutions.append( ('%paxctl_nomprotect', '/usr/sbin/paxctl +m') )
+else:
+  config.substitutions.append( ('%paxctl_nomprotect', ':') )
Index: compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
===
--- compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
+++ compiler-rt/test/builtins/Unit/enable_execute_stack_test.c
@@ -1,5 +1,5 @@
 // REQUIRES: native-run
-// RUN: %clang_builtins %s %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %paxctl_nomprotect %t && %run %t
 // REQUIRES: librt_has_enable_execute_stack
 //===-- enable_execute_stack_test.c - Test __enable_execute_stack --===//
 //
Index: compiler-rt/test/builtins/Unit/clear_cache_test.c
===
--- compiler-rt/test/builtins/Unit/clear_cache_test.c
+++ compiler-rt/test/builtins/Unit/clear_cache_test.c
@@ -1,6 +1,6 @@
 // REQUIRES: native-run
 // UNSUPPORTED: arm, aarch64
-// RUN: %clang_builtins %s %librt -o %t && %run %t
+// RUN: %clang_builtins %s %librt -o %t && %paxctl_nomprotect %t && %run %t
 // REQUIRES: librt_has_clear_cache
 //===-- clear_cache_test.c - Test clear_cache -===//
 //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2c59c4f - [perf-training] Make training data location configurable

[Shoaib Meenai via cfe-commits]

Author: Shoaib Meenai
Date: 2019-12-14T09:46:41-08:00
New Revision: 2c59c4ffb9c111f8d87a65839697d03fc485c51c

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

LOG: [perf-training] Make training data location configurable

We may wish to keep the PGO training data outside the repository. Add a
CMake variable to allow referencing an external lit testsuite.

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

Added: 


Modified: 
clang/utils/perf-training/CMakeLists.txt
clang/utils/perf-training/lit.cfg
clang/utils/perf-training/lit.site.cfg.in
clang/utils/perf-training/order-files.lit.cfg
clang/utils/perf-training/order-files.lit.site.cfg.in

Removed: 




diff  --git a/clang/utils/perf-training/CMakeLists.txt 
b/clang/utils/perf-training/CMakeLists.txt
index 39f9a4ca3c13..25bb7f49a5ef 100644
--- a/clang/utils/perf-training/CMakeLists.txt
+++ b/clang/utils/perf-training/CMakeLists.txt
@@ -10,6 +10,10 @@ endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
+set(CLANG_PGO_TRAINING_DATA "${CMAKE_CURRENT_SOURCE_DIR}" CACHE PATH
+  "The path to a lit testsuite containing samples for PGO and order file 
generation"
+  )
+
 if(LLVM_BUILD_INSTRUMENTED)
   configure_lit_site_cfg(
 ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in

diff  --git a/clang/utils/perf-training/lit.cfg 
b/clang/utils/perf-training/lit.cfg
index 671d44f83b94..67a42345da46 100644
--- a/clang/utils/perf-training/lit.cfg
+++ b/clang/utils/perf-training/lit.cfg
@@ -27,7 +27,7 @@ config.clang = lit.util.which('clang', 
config.clang_tools_dir).replace('\\', '/'
 config.name = 'Clang Perf Training'
 config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.S', 
'.modulemap']
 
-cc1_wrapper = '%s %s/perf-helper.py cc1' % (config.python_exe, 
config.test_source_root)
+cc1_wrapper = '%s %s/perf-helper.py cc1' % (config.python_exe, 
config.perf_helper_dir)
 
 use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
 config.test_format = lit.formats.ShTest(use_lit_shell == "0")

diff  --git a/clang/utils/perf-training/lit.site.cfg.in 
b/clang/utils/perf-training/lit.site.cfg.in
index 66683bcd5286..340a0e909b10 100644
--- a/clang/utils/perf-training/lit.site.cfg.in
+++ b/clang/utils/perf-training/lit.site.cfg.in
@@ -3,8 +3,9 @@
 import sys
 
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
+config.perf_helper_dir = "@CMAKE_CURRENT_SOURCE_DIR@"
 config.test_exec_root = "@CMAKE_CURRENT_BINARY_DIR@"
-config.test_source_root = "@CMAKE_CURRENT_SOURCE_DIR@"
+config.test_source_root = "@CLANG_PGO_TRAINING_DATA@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_exe = "@PYTHON_EXECUTABLE@"
 

diff  --git a/clang/utils/perf-training/order-files.lit.cfg 
b/clang/utils/perf-training/order-files.lit.cfg
index 93904ec84a41..03c605c01c56 100644
--- a/clang/utils/perf-training/order-files.lit.cfg
+++ b/clang/utils/perf-training/order-files.lit.cfg
@@ -28,8 +28,8 @@ config.clang = os.path.realpath(lit.util.which('clang', 
config.clang_tools_dir))
 config.name = 'Clang Perf Training'
 config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.S', 
'.modulemap']
 
-dtrace_wrapper = '%s %s/perf-helper.py dtrace' % (config.python_exe, 
config.test_source_root)
-dtrace_wrapper_cc1 = '%s %s/perf-helper.py dtrace --cc1' % (config.python_exe, 
config.test_source_root)
+dtrace_wrapper = '%s %s/perf-helper.py dtrace' % (config.python_exe, 
config.perf_helper_dir)
+dtrace_wrapper_cc1 = '%s %s/perf-helper.py dtrace --cc1' % (config.python_exe, 
config.perf_helper_dir)
 
 if 'darwin' in config.target_triple:
 lit_config.note('using DTrace oneshot probe')

diff  --git a/clang/utils/perf-training/order-files.lit.site.cfg.in 
b/clang/utils/perf-training/order-files.lit.site.cfg.in
index 0490a217ad04..87406dbaf9a6 100644
--- a/clang/utils/perf-training/order-files.lit.site.cfg.in
+++ b/clang/utils/perf-training/order-files.lit.site.cfg.in
@@ -3,8 +3,9 @@
 import sys
 
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
+config.perf_helper_dir = "@CMAKE_CURRENT_SOURCE_DIR@"
 config.test_exec_root = "@CMAKE_CURRENT_BINARY_DIR@"
-config.test_source_root = "@CMAKE_CURRENT_SOURCE_DIR@"
+config.test_source_root = "@CLANG_PGO_TRAINING_DATA@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_exe = "@PYTHON_EXECUTABLE@"
 



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D70469#1779906 , @xazax.hun wrote:

> In D70469#1778609 , @NoQ wrote:
>
> > What about `__attribute__((acquire_handle("fuchsia")))`
>
>
> I would by fine with this as well. Aaron?


I would also be fine with that. I don't have a strong opinion on whether to use 
a string literal or an identifier as the argument, but I slightly swing towards 
string literal so you could do something like `"C++ Core Guideline"` where 
there are spaces or punctuators.




Comment at: clang/lib/Sema/SemaType.cpp:7424
+ ParsedAttr ) {
+  if (CurType->isFunctionType()) {
+State.getSema().Diag(Attr.getLoc(), diag::warn_type_attribute_wrong_type)

NoQ wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > xazax.hun wrote:
> > > > aaron.ballman wrote:
> > > > > aaron.ballman wrote:
> > > > > > Is this correct, or should it be `if (!CurType->isFunctionType())`?
> > > > > I see now that this is only incorrect for the acquire attribute, but 
> > > > > not the other two.
> > > > I think we might consider it incorrect for the acquire as well. Because 
> > > > if we let the user put acquire on the function, it is ambiguous where 
> > > > to put the annotation. If we do not let them put on the function they 
> > > > are always forced to put on the return type to express this. But in 
> > > > case this kind of ambiguity is a feature and not a bug I can change the 
> > > > behavior.
> > > I guess I was considering the semantics as being acquire on a function 
> > > type applies to the non-void return value being the handle and release on 
> > > a function type applies to the only parameter being passed to the 
> > > function.
> > > 
> > > I don't think it makes sense to apply acquire or release semantics to a 
> > > handle type itself because that's not really part of the type identity 
> > > itself (it's not an "acquiring handle" or a "releasing handle" -- I would 
> > > think it's *both* at the same time), so I think that's been throwing me 
> > > for a loop with the latest round of patches here. What I was imagining 
> > > was that you should mark any place a handle is created or released, which 
> > > works almost-fine for parameters (because you can mark the parameter 
> > > declarations with an attribute). It doesn't quite work for function 
> > > pointers, however, because those don't have parameters. e.g., `void 
> > > (*fp)([[clang::acquire_handle]] int &)` won't do what you expect, I 
> > > believe. However, for functions which return the acquired handle, there 
> > > is no good place to write the attribute except on the function itself 
> > > (because, again, it's not a property of the return type, but of the 
> > > specific function's return type). For those cases, I imagine you want the 
> > > function *type* to be what codifies that a handle is returned, because 
> > > then it will work with function pointers. For *using* a handle, I'm not 
> > > certain what the benefit is to having an attribute on each use -- that 
> > > seems likely for a user to forget to add an annotation somewhere and get 
> > > incorrect behavior. I would normally suggest that the attribute be 
> > > applied to the type declaration of the handle (e.g., through a typedef), 
> > > but if you want this to work with handles coming from system headers 
> > > (like file descriptors or sockets, etc) then will this approach work?
> > Hmm. The problem is that a function might do multiple things with multiple 
> > handles, for example in Fuchsia there are functions that replacing a handle 
> > with another one. So I am afraid naively putting the attributes on the 
> > function type might end up being ambiguous sometimes. I totally agree with 
> > your argument about the annotation not being part of the type identity. If 
> > we do want to put this on the function type maybe we could do something 
> > similar to `nonnull` and enumerate parameter indices? What do you think? 
> > 
> > The `use` annotation might be a bit misleading. The user does not 
> > necessarily need to annotate all uses.
> > Let's consider the following function and a call:
> > ```
> > void f(int *handle);
> > close_handle(handle);
> > f(); // What will this do?
> > ```
> > 
> > The function `f` would be free to do anything, like store the address of 
> > the handle in a global data structure, overwrite the closed handle and so 
> > on. The purpose of the `use` annotation is to tell the analyzer that none 
> > of this will happen, so we can safely diagnose a use after release. So the 
> > original idea was to annotate those functions that are guaranteed to never 
> > close, overwrite etc the handle.
> > For unannotated functions the analyzer will be conservative and assume that 
> > the code is OK. 
> I heard that type attributes are relatively scary and require a lot of work 
> to implement, and 

[clang] 5708f2d - [clang] Fix modules build after addition of TypeBitCodes.def

[Raphael Isemann via cfe-commits]

Author: Raphael Isemann
Date: 2019-12-14T18:24:53+01:00
New Revision: 5708f2daf7386ef5f1ec54db4bda2b48bbcbe934

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

LOG: [clang] Fix modules build after addition of TypeBitCodes.def

In revision 139006ceb641f038a2b19cac1174316e57004ed6 the Serialization
folder got its first def file 'TypeBitCodes.def'. This broke the
modules build as this .def file was not textually included but implicitly
converted into a module due to our umbrella directive.

This patch fixes this by explicitly marking the .def file as textual.

Added: 


Modified: 
clang/include/clang/module.modulemap

Removed: 




diff  --git a/clang/include/clang/module.modulemap 
b/clang/include/clang/module.modulemap
index 2cbe865bce87..b3e2108d3fa6 100644
--- a/clang/include/clang/module.modulemap
+++ b/clang/include/clang/module.modulemap
@@ -114,7 +114,15 @@ module Clang_Parse { requires cplusplus umbrella "Parse" 
module * { export * } }
 module Clang_Rewrite { requires cplusplus umbrella "Rewrite/Core" module * { 
export * } }
 module Clang_RewriteFrontend { requires cplusplus umbrella "Rewrite/Frontend" 
module * { export * } }
 module Clang_Sema { requires cplusplus umbrella "Sema" module * { export * } }
-module Clang_Serialization { requires cplusplus umbrella "Serialization" 
module * { export * } }
+
+module Clang_Serialization {
+  requires cplusplus
+  umbrella "Serialization"
+
+  textual header "Serialization/TypeBitCodes.def"
+
+  module * { export * }
+}
 
 module Clang_StaticAnalyzer_Core {
   requires cplusplus



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

[PATCH] D71507: [perf-training] Make training data location configurable

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c59c4ffb9c1: [perf-training] Make training data location 
configurable (authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71507

Files:
  clang/utils/perf-training/CMakeLists.txt
  clang/utils/perf-training/lit.cfg
  clang/utils/perf-training/lit.site.cfg.in
  clang/utils/perf-training/order-files.lit.cfg
  clang/utils/perf-training/order-files.lit.site.cfg.in


Index: clang/utils/perf-training/order-files.lit.site.cfg.in
===
--- clang/utils/perf-training/order-files.lit.site.cfg.in
+++ clang/utils/perf-training/order-files.lit.site.cfg.in
@@ -3,8 +3,9 @@
 import sys
 
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
+config.perf_helper_dir = "@CMAKE_CURRENT_SOURCE_DIR@"
 config.test_exec_root = "@CMAKE_CURRENT_BINARY_DIR@"
-config.test_source_root = "@CMAKE_CURRENT_SOURCE_DIR@"
+config.test_source_root = "@CLANG_PGO_TRAINING_DATA@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_exe = "@PYTHON_EXECUTABLE@"
 
Index: clang/utils/perf-training/order-files.lit.cfg
===
--- clang/utils/perf-training/order-files.lit.cfg
+++ clang/utils/perf-training/order-files.lit.cfg
@@ -28,8 +28,8 @@
 config.name = 'Clang Perf Training'
 config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.S', 
'.modulemap']
 
-dtrace_wrapper = '%s %s/perf-helper.py dtrace' % (config.python_exe, 
config.test_source_root)
-dtrace_wrapper_cc1 = '%s %s/perf-helper.py dtrace --cc1' % (config.python_exe, 
config.test_source_root)
+dtrace_wrapper = '%s %s/perf-helper.py dtrace' % (config.python_exe, 
config.perf_helper_dir)
+dtrace_wrapper_cc1 = '%s %s/perf-helper.py dtrace --cc1' % (config.python_exe, 
config.perf_helper_dir)
 
 if 'darwin' in config.target_triple:
 lit_config.note('using DTrace oneshot probe')
Index: clang/utils/perf-training/lit.site.cfg.in
===
--- clang/utils/perf-training/lit.site.cfg.in
+++ clang/utils/perf-training/lit.site.cfg.in
@@ -3,8 +3,9 @@
 import sys
 
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
+config.perf_helper_dir = "@CMAKE_CURRENT_SOURCE_DIR@"
 config.test_exec_root = "@CMAKE_CURRENT_BINARY_DIR@"
-config.test_source_root = "@CMAKE_CURRENT_SOURCE_DIR@"
+config.test_source_root = "@CLANG_PGO_TRAINING_DATA@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_exe = "@PYTHON_EXECUTABLE@"
 
Index: clang/utils/perf-training/lit.cfg
===
--- clang/utils/perf-training/lit.cfg
+++ clang/utils/perf-training/lit.cfg
@@ -27,7 +27,7 @@
 config.name = 'Clang Perf Training'
 config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.S', 
'.modulemap']
 
-cc1_wrapper = '%s %s/perf-helper.py cc1' % (config.python_exe, 
config.test_source_root)
+cc1_wrapper = '%s %s/perf-helper.py cc1' % (config.python_exe, 
config.perf_helper_dir)
 
 use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
 config.test_format = lit.formats.ShTest(use_lit_shell == "0")
Index: clang/utils/perf-training/CMakeLists.txt
===
--- clang/utils/perf-training/CMakeLists.txt
+++ clang/utils/perf-training/CMakeLists.txt
@@ -10,6 +10,10 @@
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
+set(CLANG_PGO_TRAINING_DATA "${CMAKE_CURRENT_SOURCE_DIR}" CACHE PATH
+  "The path to a lit testsuite containing samples for PGO and order file 
generation"
+  )
+
 if(LLVM_BUILD_INSTRUMENTED)
   configure_lit_site_cfg(
 ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in


Index: clang/utils/perf-training/order-files.lit.site.cfg.in
===
--- clang/utils/perf-training/order-files.lit.site.cfg.in
+++ clang/utils/perf-training/order-files.lit.site.cfg.in
@@ -3,8 +3,9 @@
 import sys
 
 config.clang_tools_dir = "@CLANG_TOOLS_DIR@"
+config.perf_helper_dir = "@CMAKE_CURRENT_SOURCE_DIR@"
 config.test_exec_root = "@CMAKE_CURRENT_BINARY_DIR@"
-config.test_source_root = "@CMAKE_CURRENT_SOURCE_DIR@"
+config.test_source_root = "@CLANG_PGO_TRAINING_DATA@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_exe = "@PYTHON_EXECUTABLE@"
 
Index: clang/utils/perf-training/order-files.lit.cfg
===
--- clang/utils/perf-training/order-files.lit.cfg
+++ clang/utils/perf-training/order-files.lit.cfg
@@ -28,8 +28,8 @@
 config.name = 'Clang Perf Training'
 config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.S', '.modulemap']
 
-dtrace_wrapper = '%s %s/perf-helper.py dtrace' % (config.python_exe, config.test_source_root)