r309725 - Thread Safety Analysis: fix assert_capability.

[Josh Gao via cfe-commits]

Author: jmgao
Date: Tue Aug  1 12:18:05 2017
New Revision: 309725

URL: http://llvm.org/viewvc/llvm-project?rev=309725&view=rev
Log:
Thread Safety Analysis: fix assert_capability.

Summary:
Previously, the assert_capability attribute was completely ignored by
thread safety analysis.

Reviewers: delesley, rnk

Reviewed By: delesley

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=309725&r1=309724&r2=309725&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Aug  1 12:18:05 2017
@@ -2138,7 +2138,7 @@ def AssertCapability : InheritableAttr {
   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
   let DuplicatesAllowedWhileMerging = 1;
-  let Args = [ExprArgument<"Expr">];
+  let Args = [VariadicExprArgument<"Args">];
   let Accessors = [Accessor<"isShared",
 [GNU<"assert_shared_capability">,
  CXX11<"clang", "assert_shared_capability">]>];

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=309725&r1=309724&r2=309725&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Tue Aug  1 12:18:05 2017
@@ -1735,8 +1735,23 @@ void BuildLockset::handleCall(Expr *Exp,
 CapExprSet AssertLocks;
 Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
 for (const auto &AssertLock : AssertLocks)
-  Analyzer->addLock(FSet, llvm::make_unique(
-  AssertLock, LK_Shared, Loc, false, true),
+  Analyzer->addLock(FSet,
+llvm::make_unique(
+AssertLock, LK_Shared, Loc, false, true),
+ClassifyDiagnostic(A));
+break;
+  }
+
+  case attr::AssertCapability: {
+AssertCapabilityAttr *A = cast(At);
+CapExprSet AssertLocks;
+Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
+for (const auto &AssertLock : AssertLocks)
+  Analyzer->addLock(FSet,
+llvm::make_unique(
+AssertLock,
+A->isShared() ? LK_Shared : LK_Exclusive, Loc,
+false, true),
 ClassifyDiagnostic(A));
 break;
   }

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=309725&r1=309724&r2=309725&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Aug  1 12:18:05 2017
@@ -5686,8 +5686,12 @@ static void handleCapabilityAttr(Sema &S
 
 static void handleAssertCapabilityAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
+  SmallVector Args;
+  if (!checkLockFunAttrCommon(S, D, Attr, Args))
+return;
+
   D->addAttr(::new (S.Context) AssertCapabilityAttr(Attr.getRange(), S.Context,
-Attr.getArgAsExpr(0),
+Args.data(), Args.size(),
 Attr.getAttributeSpellingListIndex()));
 }
 

Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=309725&r1=309724&r2=309725&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Tue Aug  1 12:18:05 
2017
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety 
-Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety 
-Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions 
-DUSE_ASSERT_CAPABILITY=0 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety 
-Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions 
-DUSE_ASSERT_CAPABILITY=1 %s
 
 // FIXME: should also run  %clang_cc1 -fsyntax-only -verify -Wthread-safety 
-std=c++11 -Wc++98-compat %s
 // FIXME: should also run  %clang_cc1 -fsyntax-only -verify -Wthread-safety %s

r309731 - Revert "Thread Safety Analysis: fix assert_capability."

[Josh Gao via cfe-commits]

Author: jmgao
Date: Tue Aug  1 12:53:31 2017
New Revision: 309731

URL: http://llvm.org/viewvc/llvm-project?rev=309731&view=rev
Log:
Revert "Thread Safety Analysis: fix assert_capability."

This reverts commit rL309725.

Broke test/Sema/attr-capabilities.c.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=309731&r1=309730&r2=309731&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Aug  1 12:53:31 2017
@@ -2138,7 +2138,7 @@ def AssertCapability : InheritableAttr {
   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
   let DuplicatesAllowedWhileMerging = 1;
-  let Args = [VariadicExprArgument<"Args">];
+  let Args = [ExprArgument<"Expr">];
   let Accessors = [Accessor<"isShared",
 [GNU<"assert_shared_capability">,
  CXX11<"clang", "assert_shared_capability">]>];

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=309731&r1=309730&r2=309731&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Tue Aug  1 12:53:31 2017
@@ -1735,23 +1735,8 @@ void BuildLockset::handleCall(Expr *Exp,
 CapExprSet AssertLocks;
 Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
 for (const auto &AssertLock : AssertLocks)
-  Analyzer->addLock(FSet,
-llvm::make_unique(
-AssertLock, LK_Shared, Loc, false, true),
-ClassifyDiagnostic(A));
-break;
-  }
-
-  case attr::AssertCapability: {
-AssertCapabilityAttr *A = cast(At);
-CapExprSet AssertLocks;
-Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
-for (const auto &AssertLock : AssertLocks)
-  Analyzer->addLock(FSet,
-llvm::make_unique(
-AssertLock,
-A->isShared() ? LK_Shared : LK_Exclusive, Loc,
-false, true),
+  Analyzer->addLock(FSet, llvm::make_unique(
+  AssertLock, LK_Shared, Loc, false, true),
 ClassifyDiagnostic(A));
 break;
   }

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=309731&r1=309730&r2=309731&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Aug  1 12:53:31 2017
@@ -5686,12 +5686,8 @@ static void handleCapabilityAttr(Sema &S
 
 static void handleAssertCapabilityAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
-  SmallVector Args;
-  if (!checkLockFunAttrCommon(S, D, Attr, Args))
-return;
-
   D->addAttr(::new (S.Context) AssertCapabilityAttr(Attr.getRange(), S.Context,
-Args.data(), Args.size(),
+Attr.getArgAsExpr(0),
 Attr.getAttributeSpellingListIndex()));
 }
 

Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=309731&r1=309730&r2=309731&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Tue Aug  1 12:53:31 
2017
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety 
-Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions 
-DUSE_ASSERT_CAPABILITY=0 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety 
-Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions 
-DUSE_ASSERT_CAPABILITY=1 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety 
-Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions %s
 
 // FIXME: should also run  %clang_cc1 -fsyntax-only -verify -Wthread-safety 
-std=c++11 -Wc++98-compat %s
 // FIXME: should also run  %clang_cc1 -fsyntax-only -verify -Wthread-safety %s
@@ -14,15 +13,8 @@
 #define ACQUIRED_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__)))
 #define EXCLUSIVE_LOCK_FUNCTION(...)
__attribute__((exclu

r310402 - Reland "Thread Safety Analysis: fix assert_capability."

[Josh Gao via cfe-commits]

Author: jmgao
Date: Tue Aug  8 12:44:34 2017
New Revision: 310402

URL: http://llvm.org/viewvc/llvm-project?rev=310402&view=rev
Log:
Reland "Thread Safety Analysis: fix assert_capability."

Delete the test that was broken by rL309725, and add it back in a
follow up commit. Also, improve the tests a bit.

Reviewers: delesley, aaron.ballman

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Sema/attr-capabilities.c
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=310402&r1=310401&r2=310402&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Aug  8 12:44:34 2017
@@ -2138,7 +2138,7 @@ def AssertCapability : InheritableAttr {
   let TemplateDependent = 1;
   let ParseArgumentsAsUnevaluated = 1;
   let DuplicatesAllowedWhileMerging = 1;
-  let Args = [ExprArgument<"Expr">];
+  let Args = [VariadicExprArgument<"Args">];
   let Accessors = [Accessor<"isShared",
 [GNU<"assert_shared_capability">,
  CXX11<"clang", "assert_shared_capability">]>];

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=310402&r1=310401&r2=310402&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Tue Aug  8 12:44:34 2017
@@ -1735,8 +1735,23 @@ void BuildLockset::handleCall(Expr *Exp,
 CapExprSet AssertLocks;
 Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
 for (const auto &AssertLock : AssertLocks)
-  Analyzer->addLock(FSet, llvm::make_unique(
-  AssertLock, LK_Shared, Loc, false, true),
+  Analyzer->addLock(FSet,
+llvm::make_unique(
+AssertLock, LK_Shared, Loc, false, true),
+ClassifyDiagnostic(A));
+break;
+  }
+
+  case attr::AssertCapability: {
+AssertCapabilityAttr *A = cast(At);
+CapExprSet AssertLocks;
+Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD);
+for (const auto &AssertLock : AssertLocks)
+  Analyzer->addLock(FSet,
+llvm::make_unique(
+AssertLock,
+A->isShared() ? LK_Shared : LK_Exclusive, Loc,
+false, true),
 ClassifyDiagnostic(A));
 break;
   }

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=310402&r1=310401&r2=310402&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Aug  8 12:44:34 2017
@@ -5700,8 +5700,12 @@ static void handleCapabilityAttr(Sema &S
 
 static void handleAssertCapabilityAttr(Sema &S, Decl *D,
const AttributeList &Attr) {
+  SmallVector Args;
+  if (!checkLockFunAttrCommon(S, D, Attr, Args))
+return;
+
   D->addAttr(::new (S.Context) AssertCapabilityAttr(Attr.getRange(), S.Context,
-Attr.getArgAsExpr(0),
+Args.data(), Args.size(),
 Attr.getAttributeSpellingListIndex()));
 }
 

Modified: cfe/trunk/test/Sema/attr-capabilities.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-capabilities.c?rev=310402&r1=310401&r2=310402&view=diff
==
--- cfe/trunk/test/Sema/attr-capabilities.c (original)
+++ cfe/trunk/test/Sema/attr-capabilities.c Tue Aug  8 12:44:34 2017
@@ -37,9 +37,6 @@ void Func6(void) __attribute__((requires
 void Func7(void) __attribute__((assert_capability(GUI))) {}
 void Func8(void) __attribute__((assert_shared_capability(GUI))) {}
 
-void Func9(void) __attribute__((assert_capability())) {} // expected-error 
{{'assert_capability' attribute takes one argument}}
-void Func10(void) __attribute__((assert_shared_capability())) {} // 
expected-error {{'assert_shared_capability' attribute takes one argument}}
-
 void Func11(void) __attribute__((acquire_capability(GUI))) {}
 void Func12(void) __attribute__((acquire_shared_capability(GUI))) {}
 

Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL: 
http://llvm.org/viewvc/llvm-project/c

r310403 - Thread Safety Analysis: warn on nonsensical attributes.

[Josh Gao via cfe-commits]

Author: jmgao
Date: Tue Aug  8 12:44:35 2017
New Revision: 310403

URL: http://llvm.org/viewvc/llvm-project?rev=310403&view=rev
Log:
Thread Safety Analysis: warn on nonsensical attributes.

Add warnings in cases where an implicit `this` argument is expected to
attributes because either `this` doesn't exist because the attribute is
on a free function, or because `this` is on a type that doesn't have a
corresponding capability/lockable/scoped_lockable attribute.

Reviewers: delesley, aaron.ballman

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Sema/attr-capabilities.c
cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=310403&r1=310402&r2=310403&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug  8 12:44:35 
2017
@@ -2932,6 +2932,16 @@ def warn_thread_attribute_decl_not_locka
   "%0 attribute can only be applied in a context annotated "
   "with 'capability(\"mutex\")' attribute">,
   InGroup, DefaultIgnore;
+def warn_thread_attribute_noargs_not_lockable : Warning<
+  "%0 attribute requires type annotated with 'capability' attribute; "
+  "type here is %1">,
+  InGroup, DefaultIgnore;
+def warn_thread_attribute_noargs_not_method : Warning<
+  "%0 attribute without arguments can only be applied to a method of a class">,
+  InGroup, DefaultIgnore;
+def warn_thread_attribute_noargs_static_method : Warning<
+  "%0 attribute without arguments cannot be applied to a static method">,
+  InGroup, DefaultIgnore;
 def warn_thread_attribute_decl_not_pointer : Warning<
   "%0 only applies to pointer types; type here is %1">,
   InGroup, DefaultIgnore;

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=310403&r1=310402&r2=310403&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Aug  8 12:44:35 2017
@@ -480,7 +480,7 @@ static const RecordType *getRecordType(Q
   return nullptr;
 }
 
-static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
+template  static bool checkRecordTypeForAttr(Sema &S, QualType Ty) 
{
   const RecordType *RT = getRecordType(Ty);
 
   if (!RT)
@@ -497,7 +497,7 @@ static bool checkRecordTypeForCapability
 
   // Check if the record itself has a capability.
   RecordDecl *RD = RT->getDecl();
-  if (RD->hasAttr())
+  if (RD->hasAttr())
 return true;
 
   // Else check if any base classes have a capability.
@@ -505,14 +505,14 @@ static bool checkRecordTypeForCapability
 CXXBasePaths BPaths(false, false);
 if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) {
   const auto *Type = BS->getType()->getAs();
-  return Type->getDecl()->hasAttr();
+  return Type->getDecl()->hasAttr();
 }, BPaths))
   return true;
   }
   return false;
 }
 
-static bool checkTypedefTypeForCapability(QualType Ty) {
+template  static bool checkTypedefTypeForAttr(QualType Ty) {
   const auto *TD = Ty->getAs();
   if (!TD)
 return false;
@@ -521,19 +521,27 @@ static bool checkTypedefTypeForCapabilit
   if (!TN)
 return false;
 
-  return TN->hasAttr();
+  return TN->hasAttr();
 }
 
-static bool typeHasCapability(Sema &S, QualType Ty) {
-  if (checkTypedefTypeForCapability(Ty))
+template  static bool typeHasAttr(Sema &S, QualType Ty) {
+  if (checkTypedefTypeForAttr(Ty))
 return true;
 
-  if (checkRecordTypeForCapability(S, Ty))
+  if (checkRecordTypeForAttr(S, Ty))
 return true;
 
   return false;
 }
 
+static bool typeHasCapability(Sema &S, QualType Ty) {
+  return typeHasAttr(S, Ty);
+}
+
+static bool typeHasScopedLockable(Sema &S, QualType Ty) {
+  return typeHasAttr(S, Ty);
+}
+
 static bool isCapabilityExpr(Sema &S, const Expr *Ex) {
   // Capability expressions are simple expressions involving the boolean logic
   // operators &&, || or !, a simple DeclRefExpr, CastExpr or a ParenExpr. Once
@@ -570,6 +578,8 @@ static void checkAttrArgsAreCapabilityOb
SmallVectorImpl &Args,
int Sidx = 0,
bool ParamIdxOk = false) {
+  bool TriedParam = false;
+
   for (unsigned Idx = Sidx; Idx < Attr.getNumArgs(); ++Idx) {
 Expr *ArgExp = Attr.getArgAsExpr(Idx);
 
@@ -610,15 +620,18 @@ static void checkAttrArgsAreCapabilityOb
 const RecordType *RT = getRecordType(ArgTy);
 
 // Now check if we index into a record type function param.
-i

Re: r310403 - Thread Safety Analysis: warn on nonsensical attributes.

[Josh Gao via cfe-commits]

Sorry for the breakage, I'm reverting the patch that caused this now.

On Fri, Aug 11, 2017 at 12:36 AM, NAKAMURA Takumi 
wrote:

> It causes failure in check-libcxx with just-built clang.
>
> http://bb.pgr.jp/builders/bootstrap-clang-libcxx-lld-i686-linux/builds/758
>
>
> On Wed, Aug 9, 2017 at 4:45 AM Josh Gao via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: jmgao
>> Date: Tue Aug  8 12:44:35 2017
>> New Revision: 310403
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=310403&view=rev
>> Log:
>> Thread Safety Analysis: warn on nonsensical attributes.
>>
>> Add warnings in cases where an implicit `this` argument is expected to
>> attributes because either `this` doesn't exist because the attribute is
>> on a free function, or because `this` is on a type that doesn't have a
>> corresponding capability/lockable/scoped_lockable attribute.
>>
>> Reviewers: delesley, aaron.ballman
>>
>> Differential Revision: https://reviews.llvm.org/D36237
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> cfe/trunk/test/Sema/attr-capabilities.c
>> cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
>> DiagnosticSemaKinds.td?rev=310403&r1=310402&r2=310403&view=diff
>> 
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug  8
>> 12:44:35 2017
>> @@ -2932,6 +2932,16 @@ def warn_thread_attribute_decl_not_locka
>>"%0 attribute can only be applied in a context annotated "
>>"with 'capability(\"mutex\")' attribute">,
>>InGroup, DefaultIgnore;
>> +def warn_thread_attribute_noargs_not_lockable : Warning<
>> +  "%0 attribute requires type annotated with 'capability' attribute; "
>> +  "type here is %1">,
>> +  InGroup, DefaultIgnore;
>> +def warn_thread_attribute_noargs_not_method : Warning<
>> +  "%0 attribute without arguments can only be applied to a method of a
>> class">,
>> +  InGroup, DefaultIgnore;
>> +def warn_thread_attribute_noargs_static_method : Warning<
>> +  "%0 attribute without arguments cannot be applied to a static method">,
>> +  InGroup, DefaultIgnore;
>>  def warn_thread_attribute_decl_not_pointer : Warning<
>>"%0 only applies to pointer types; type here is %1">,
>>InGroup, DefaultIgnore;
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
>> SemaDeclAttr.cpp?rev=310403&r1=310402&r2=310403&view=diff
>> 
>> ==
>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Aug  8 12:44:35 2017
>> @@ -480,7 +480,7 @@ static const RecordType *getRecordType(Q
>>return nullptr;
>>  }
>>
>> -static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
>> +template  static bool checkRecordTypeForAttr(Sema &S,
>> QualType Ty) {
>>const RecordType *RT = getRecordType(Ty);
>>
>>if (!RT)
>> @@ -497,7 +497,7 @@ static bool checkRecordTypeForCapability
>>
>>// Check if the record itself has a capability.
>>RecordDecl *RD = RT->getDecl();
>> -  if (RD->hasAttr())
>> +  if (RD->hasAttr())
>>  return true;
>>
>>// Else check if any base classes have a capability.
>> @@ -505,14 +505,14 @@ static bool checkRecordTypeForCapability
>>  CXXBasePaths BPaths(false, false);
>>  if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &)
>> {
>>const auto *Type = BS->getType()->getAs();
>> -  return Type->getDecl()->hasAttr();
>> +  return Type->getDecl()->hasAttr();
>>  }, BPaths))
>>return true;
>>}
>>return false;
>>  }
>>
>> -static bool checkTypedefTypeForCapability(QualType Ty) {
>> +template  static bool checkTypedefTypeForAttr(QualType Ty) {
>>const auto *TD = Ty->getAs();
>>if (!TD)
>>  return false;
>> @@ -521,1

r310698 - Revert "Thread Safety Analysis: warn on nonsensical attributes."

[Josh Gao via cfe-commits]

Author: jmgao
Date: Fri Aug 11 00:54:35 2017
New Revision: 310698

URL: http://llvm.org/viewvc/llvm-project?rev=310698&view=rev
Log:
Revert "Thread Safety Analysis: warn on nonsensical attributes."

This reverts commit rL310403, which caused spurious warnings in libc++,
because it didn't properly handle templated scoped lockable types.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Sema/attr-capabilities.c
cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=310698&r1=310697&r2=310698&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Aug 11 00:54:35 
2017
@@ -2935,16 +2935,6 @@ def warn_thread_attribute_decl_not_locka
   "%0 attribute can only be applied in a context annotated "
   "with 'capability(\"mutex\")' attribute">,
   InGroup, DefaultIgnore;
-def warn_thread_attribute_noargs_not_lockable : Warning<
-  "%0 attribute requires type annotated with 'capability' attribute; "
-  "type here is %1">,
-  InGroup, DefaultIgnore;
-def warn_thread_attribute_noargs_not_method : Warning<
-  "%0 attribute without arguments can only be applied to a method of a class">,
-  InGroup, DefaultIgnore;
-def warn_thread_attribute_noargs_static_method : Warning<
-  "%0 attribute without arguments cannot be applied to a static method">,
-  InGroup, DefaultIgnore;
 def warn_thread_attribute_decl_not_pointer : Warning<
   "%0 only applies to pointer types; type here is %1">,
   InGroup, DefaultIgnore;

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=310698&r1=310697&r2=310698&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Aug 11 00:54:35 2017
@@ -480,7 +480,7 @@ static const RecordType *getRecordType(Q
   return nullptr;
 }
 
-template  static bool checkRecordTypeForAttr(Sema &S, QualType Ty) 
{
+static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
   const RecordType *RT = getRecordType(Ty);
 
   if (!RT)
@@ -497,7 +497,7 @@ template  static bool checkR
 
   // Check if the record itself has a capability.
   RecordDecl *RD = RT->getDecl();
-  if (RD->hasAttr())
+  if (RD->hasAttr())
 return true;
 
   // Else check if any base classes have a capability.
@@ -505,14 +505,14 @@ template  static bool checkR
 CXXBasePaths BPaths(false, false);
 if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) {
   const auto *Type = BS->getType()->getAs();
-  return Type->getDecl()->hasAttr();
+  return Type->getDecl()->hasAttr();
 }, BPaths))
   return true;
   }
   return false;
 }
 
-template  static bool checkTypedefTypeForAttr(QualType Ty) {
+static bool checkTypedefTypeForCapability(QualType Ty) {
   const auto *TD = Ty->getAs();
   if (!TD)
 return false;
@@ -521,27 +521,19 @@ template  static bool checkT
   if (!TN)
 return false;
 
-  return TN->hasAttr();
+  return TN->hasAttr();
 }
 
-template  static bool typeHasAttr(Sema &S, QualType Ty) {
-  if (checkTypedefTypeForAttr(Ty))
+static bool typeHasCapability(Sema &S, QualType Ty) {
+  if (checkTypedefTypeForCapability(Ty))
 return true;
 
-  if (checkRecordTypeForAttr(S, Ty))
+  if (checkRecordTypeForCapability(S, Ty))
 return true;
 
   return false;
 }
 
-static bool typeHasCapability(Sema &S, QualType Ty) {
-  return typeHasAttr(S, Ty);
-}
-
-static bool typeHasScopedLockable(Sema &S, QualType Ty) {
-  return typeHasAttr(S, Ty);
-}
-
 static bool isCapabilityExpr(Sema &S, const Expr *Ex) {
   // Capability expressions are simple expressions involving the boolean logic
   // operators &&, || or !, a simple DeclRefExpr, CastExpr or a ParenExpr. Once
@@ -578,8 +570,6 @@ static void checkAttrArgsAreCapabilityOb
SmallVectorImpl &Args,
int Sidx = 0,
bool ParamIdxOk = false) {
-  bool TriedParam = false;
-
   for (unsigned Idx = Sidx; Idx < Attr.getNumArgs(); ++Idx) {
 Expr *ArgExp = Attr.getArgAsExpr(Idx);
 
@@ -620,18 +610,15 @@ static void checkAttrArgsAreCapabilityOb
 const RecordType *RT = getRecordType(ArgTy);
 
 // Now check if we index into a record type function param.
-if (!RT && ParamIdxOk) {
+if(!RT && ParamIdxOk) {
   FunctionDecl *FD = dyn_cast(D);
   IntegerLiteral *IL = dyn_cast(ArgExp);
-  if (FD && IL) {
-// Don't emit free function warnings if an index was given.
-TriedParam = tru

Re: r310403 - Thread Safety Analysis: warn on nonsensical attributes.

[Josh Gao via cfe-commits]

Reverted in r310698.

On Fri, Aug 11, 2017 at 12:51 AM, Josh Gao  wrote:

> Sorry for the breakage, I'm reverting the patch that caused this now.
>
> On Fri, Aug 11, 2017 at 12:36 AM, NAKAMURA Takumi 
> wrote:
>
>> It causes failure in check-libcxx with just-built clang.
>>
>> http://bb.pgr.jp/builders/bootstrap-clang-libcxx-lld-i686-
>> linux/builds/758
>>
>>
>> On Wed, Aug 9, 2017 at 4:45 AM Josh Gao via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: jmgao
>>> Date: Tue Aug  8 12:44:35 2017
>>> New Revision: 310403
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=310403&view=rev
>>> Log:
>>> Thread Safety Analysis: warn on nonsensical attributes.
>>>
>>> Add warnings in cases where an implicit `this` argument is expected to
>>> attributes because either `this` doesn't exist because the attribute is
>>> on a free function, or because `this` is on a type that doesn't have a
>>> corresponding capability/lockable/scoped_lockable attribute.
>>>
>>> Reviewers: delesley, aaron.ballman
>>>
>>> Differential Revision: https://reviews.llvm.org/D36237
>>>
>>> Modified:
>>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>> cfe/trunk/test/Sema/attr-capabilities.c
>>> cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>>> Basic/DiagnosticSemaKinds.td?rev=310403&r1=310402&r2=310403&view=diff
>>> 
>>> ==
>>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Aug  8
>>> 12:44:35 2017
>>> @@ -2932,6 +2932,16 @@ def warn_thread_attribute_decl_not_locka
>>>"%0 attribute can only be applied in a context annotated "
>>>"with 'capability(\"mutex\")' attribute">,
>>>InGroup, DefaultIgnore;
>>> +def warn_thread_attribute_noargs_not_lockable : Warning<
>>> +  "%0 attribute requires type annotated with 'capability' attribute; "
>>> +  "type here is %1">,
>>> +  InGroup, DefaultIgnore;
>>> +def warn_thread_attribute_noargs_not_method : Warning<
>>> +  "%0 attribute without arguments can only be applied to a method of a
>>> class">,
>>> +  InGroup, DefaultIgnore;
>>> +def warn_thread_attribute_noargs_static_method : Warning<
>>> +  "%0 attribute without arguments cannot be applied to a static
>>> method">,
>>> +  InGroup, DefaultIgnore;
>>>  def warn_thread_attribute_decl_not_pointer : Warning<
>>>"%0 only applies to pointer types; type here is %1">,
>>>InGroup, DefaultIgnore;
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD
>>> eclAttr.cpp?rev=310403&r1=310402&r2=310403&view=diff
>>> 
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Aug  8 12:44:35 2017
>>> @@ -480,7 +480,7 @@ static const RecordType *getRecordType(Q
>>>return nullptr;
>>>  }
>>>
>>> -static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
>>> +template  static bool checkRecordTypeForAttr(Sema &S,
>>> QualType Ty) {
>>>const RecordType *RT = getRecordType(Ty);
>>>
>>>if (!RT)
>>> @@ -497,7 +497,7 @@ static bool checkRecordTypeForCapability
>>>
>>>// Check if the record itself has a capability.
>>>RecordDecl *RD = RT->getDecl();
>>> -  if (RD->hasAttr())
>>> +  if (RD->hasAttr())
>>>  return true;
>>>
>>>// Else check if any base classes have a capability.
>>> @@ -505,14 +505,14 @@ static bool checkRecordTypeForCapability
>>>  CXXBasePaths BPaths(false, false);
>>>  if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath
>>> &) {
>>>const auto *Type = BS->getType()->getAs();
>>> -

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 32736.
jmgao added a comment.

Uploading diff with arcanist.


http://reviews.llvm.org/D12181

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/sanitize-trap-function.c

Index: test/CodeGen/sanitize-trap-function.c
===
--- /dev/null
+++ test/CodeGen/sanitize-trap-function.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero | FileCheck %s -check-prefix=NONE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo | FileCheck %s -check-prefix=TRAP -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+
+int f(int x, int y) {
+  // CHECK: call void @llvm.trap() #[[N:[0-9]+]],
+  // CHECK-NEXT: unreachable
+
+  // NONE-NOT: trap-func-name
+  // TRAP: attributes #[[N]] {{.+}} "trap-func-name"="foo"
+  // SANITIZE: attributes #[[N]] {{.+}} "trap-func-name"="bar"
+  return x / y;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -504,6 +504,7 @@
 << Args.getLastArg(OPT_mthread_model)->getAsString(Args)
 << Opts.ThreadModel;
   Opts.TrapFuncName = Args.getLastArgValue(OPT_ftrap_function_EQ);
+  Opts.SanitizeTrapFuncName = Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
   Opts.UseInitArray = Args.hasArg(OPT_fuse_init_array);
 
   Opts.FunctionSections = Args.hasFlag(OPT_ffunction_sections,
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2886,11 +2886,16 @@
 
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
+  void EmitSanitizeTrapCheck(llvm::Value *Checked);
   void EmitTrapCheck(llvm::Value *Checked);
+  void EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName);
 
   /// \brief Emit a call to trap or debugtrap and attach function attribute
   /// "trap-func-name" if specified.
+  llvm::CallInst *EmitSanitizeTrapCall(llvm::Intrinsic::ID IntrID);
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
+  llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID,
+   const std::string &TrapFuncName);
 
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -2383,7 +2383,7 @@
   : SanitizerKind::UnsignedIntegerOverflow;
   EmitBinOpCheck(std::make_pair(NotOverflow, Kind), Ops);
 } else
-  CGF.EmitTrapCheck(Builder.CreateNot(overflow));
+  CGF.EmitSanitizeTrapCheck(Builder.CreateNot(overflow));
 return result;
   }
 
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2301,7 +2301,7 @@
   }
 
   if (TrapCond)
-EmitTrapCheck(TrapCond);
+EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)
 return;
 
@@ -2381,16 +2381,27 @@
   EmitBlock(Cont);
 }
 
+void CodeGenFunction::EmitSanitizeTrapCheck(llvm::Value *Checked) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
+return EmitTrapCheck(Checked, CGM.getCodeGenOpts().SanitizeTrapFuncName);
+  }
+  return EmitTrapCheck(Checked);
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
+  return EmitTrapCheck(Checked, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per
   // function to save on code size.
   if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
 TrapBB = createBasicBlock("trap");
 Builder.CreateCondBr(Checked, Cont, TrapBB);
 EmitBlock(TrapBB);
-llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
+llvm::CallInst *TrapCall = EmitTrapCall(llvm::In

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao marked 8 inline comments as done.


Comment at: tools/clang/lib/CodeGen/CGExpr.cpp:2388
@@ +2387,3 @@
+  }
+  return EmitTrapCheck(Checked);
+}

samsonov wrote:
> This is confusing. So, you have the following behavior whenever you need to 
> emit a check for `-fsanitize-trap=foo`:
>   # Emit a call to `-fsanitize-trap-function`, if it's specified
>   # Otherwise, emit a call to `-ftrap-function`, if it's specified
>   # Otherwise, emit a trap instruction.
> 
> Do you really want it? Don't you need to skip (2) instead? If you do, you 
> should carefully document it.
> 
> 
My interpretation of the current `-ftrap-function` help text ("Issue call to 
specified function rather than a trap instruction") is that if 
`-ftrap-function` is specified, clang should never emit a trap instruction if 
defined. Does this seem reasonable?

I added a blurb to UsersManual.rst saying that it falls back to 
`-ftrap-function` if not specified.


Comment at: tools/clang/lib/CodeGen/CGExpr.cpp:2422
@@ -2404,1 +2421,3 @@
+
 llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
+  return EmitTrapCall(IntrID, CGM.getCodeGenOpts().TrapFuncName);

samsonov wrote:
> Isn't it easier to just kill this function, and always pass in 
> CGM.getCodeGenOpts().TrapFuncName where applicable?
It's used in a few other places (`__builtin_trap`, `__builtin_debugtrap`, fall 
off the end of a function in -O0), so I think it's probably easier (and less 
error prone) to keep a copy of the function that fetches the TrapFuncName 
itself.


http://reviews.llvm.org/D12181



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

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 32755.
jmgao marked 2 inline comments as done.
jmgao added a comment.

Address comments


http://reviews.llvm.org/D12181

Files:
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/sanitize-trap-function.c

Index: test/CodeGen/sanitize-trap-function.c
===
--- /dev/null
+++ test/CodeGen/sanitize-trap-function.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero | FileCheck %s -check-prefix=NONE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo | FileCheck %s -check-prefix=TRAP -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+
+int f(int x, int y) {
+  // CHECK: call void @llvm.trap() #[[N:[0-9]+]],
+  // CHECK-NEXT: unreachable
+
+  // NONE-NOT: trap-func-name
+  // TRAP: attributes #[[N]] {{.+}} "trap-func-name"="foo"
+  // SANITIZE: attributes #[[N]] {{.+}} "trap-func-name"="bar"
+  return x / y;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -681,6 +681,7 @@
   parseSanitizerKinds("-fsanitize-trap=",
   Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
   Opts.SanitizeTrap);
+  Opts.SanitizeTrapFuncName = Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
 
   Opts.CudaGpuBinaryFileNames =
   Args.getAllArgValues(OPT_fcuda_include_gpubinary);
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2885,13 +2885,26 @@
  ArrayRef DynamicArgs);
 
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
+  /// conditional branch to it, for the -fsanitize checks.
+  void EmitSanitizeTrapCheck(llvm::Value *Checked);
+
+  /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
 
+  /// \brief Create a basic block that will call the specified trap function,
+  /// and emit a conditional branch to it.
+  void EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName);
+
   /// \brief Emit a call to trap or debugtrap and attach function attribute
-  /// "trap-func-name" if specified.
+  /// "trap-func-name" if -ftrap-function is specified.
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
 
+  /// \brief Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if specified.
+  llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID,
+   const std::string &TrapFuncName);
+
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void EmitNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2301,7 +2301,7 @@
   }
 
   if (TrapCond)
-EmitTrapCheck(TrapCond);
+EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)
 return;
 
@@ -2381,16 +2381,27 @@
   EmitBlock(Cont);
 }
 
+void CodeGenFunction::EmitSanitizeTrapCheck(llvm::Value *Checked) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
+return EmitTrapCheck(Checked, CGM.getCodeGenOpts().SanitizeTrapFuncName);
+  }
+  return EmitTrapCheck(Checked);
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
+  return EmitTrapCheck(Checked, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per
   // function to save on code size.
   if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
 TrapBB = createBasicBlock("trap");
 Builder.CreateCondBr(Checked, Cont, TrapBB);
 EmitBlock(TrapBB);
-llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
+llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrins

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 32756.
jmgao added a comment.

Improve comment


http://reviews.llvm.org/D12181

Files:
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/sanitize-trap-function.c

Index: test/CodeGen/sanitize-trap-function.c
===
--- /dev/null
+++ test/CodeGen/sanitize-trap-function.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero | FileCheck %s -check-prefix=NONE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo | FileCheck %s -check-prefix=TRAP -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+
+int f(int x, int y) {
+  // CHECK: call void @llvm.trap() #[[N:[0-9]+]],
+  // CHECK-NEXT: unreachable
+
+  // NONE-NOT: trap-func-name
+  // TRAP: attributes #[[N]] {{.+}} "trap-func-name"="foo"
+  // SANITIZE: attributes #[[N]] {{.+}} "trap-func-name"="bar"
+  return x / y;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -681,6 +681,7 @@
   parseSanitizerKinds("-fsanitize-trap=",
   Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
   Opts.SanitizeTrap);
+  Opts.SanitizeTrapFuncName = Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
 
   Opts.CudaGpuBinaryFileNames =
   Args.getAllArgValues(OPT_fcuda_include_gpubinary);
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2885,13 +2885,26 @@
  ArrayRef DynamicArgs);
 
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
+  /// conditional branch to it, for the -fsanitize checks.
+  void EmitSanitizeTrapCheck(llvm::Value *Checked);
+
+  /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
 
+  /// \brief Create a basic block that will call the specified trap function,
+  /// and emit a conditional branch to it.
+  void EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName);
+
   /// \brief Emit a call to trap or debugtrap and attach function attribute
-  /// "trap-func-name" if specified.
+  /// "trap-func-name" if -ftrap-function is specified.
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
 
+  /// \brief Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if nonempty.
+  llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID,
+   const std::string &TrapFuncName);
+
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void EmitNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2301,7 +2301,7 @@
   }
 
   if (TrapCond)
-EmitTrapCheck(TrapCond);
+EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)
 return;
 
@@ -2381,16 +2381,27 @@
   EmitBlock(Cont);
 }
 
+void CodeGenFunction::EmitSanitizeTrapCheck(llvm::Value *Checked) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
+return EmitTrapCheck(Checked, CGM.getCodeGenOpts().SanitizeTrapFuncName);
+  }
+  return EmitTrapCheck(Checked);
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
+  return EmitTrapCheck(Checked, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per
   // function to save on code size.
   if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
 TrapBB = createBasicBlock("trap");
 Builder.CreateCondBr(Checked, Cont, TrapBB);
 EmitBlock(TrapBB);
-llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
+llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap, TrapFuncName);
 TrapCall->se

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao added a comment.

In http://reviews.llvm.org/D12181#229467, @rsmith wrote:

> In http://reviews.llvm.org/D12181#229358, @rsmith wrote:
>
> > Can you please give a brief description of the motivation for this change? 
> > When would it be appropriate to use this rather than `-ftrap-function`?
>
>
> I'd still like an answer to this. It's not clear to me what the purpose of 
> this is, and why you'd want a custom runtime hook for sanitizer traps but not 
> other traps. The only time we emit a trap using `-ftrap-function` where there 
> is no corresponding sanitizer is for `__builtin_trap()`; is the intention 
> that you still want a normal trap for that builtin?


The goal is to be able to give a useful fsanitize-specific error message 
("fsanitize trap occurred"), while not lying and saying this for non-sanitize 
traps.


http://reviews.llvm.org/D12181



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

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 32768.
jmgao added a comment.

Make option fit in 80 cols


http://reviews.llvm.org/D12181

Files:
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/sanitize-trap-function.c

Index: test/CodeGen/sanitize-trap-function.c
===
--- /dev/null
+++ test/CodeGen/sanitize-trap-function.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero | FileCheck %s -check-prefix=NONE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo | FileCheck %s -check-prefix=TRAP -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+
+int f(int x, int y) {
+  // CHECK: call void @llvm.trap() #[[N:[0-9]+]],
+  // CHECK-NEXT: unreachable
+
+  // NONE-NOT: trap-func-name
+  // TRAP: attributes #[[N]] {{.+}} "trap-func-name"="foo"
+  // SANITIZE: attributes #[[N]] {{.+}} "trap-func-name"="bar"
+  return x / y;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -681,6 +681,7 @@
   parseSanitizerKinds("-fsanitize-trap=",
   Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
   Opts.SanitizeTrap);
+  Opts.SanitizeTrapFuncName = Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
 
   Opts.CudaGpuBinaryFileNames =
   Args.getAllArgValues(OPT_fcuda_include_gpubinary);
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2885,13 +2885,26 @@
  ArrayRef DynamicArgs);
 
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
+  /// conditional branch to it, for the -fsanitize checks.
+  void EmitSanitizeTrapCheck(llvm::Value *Checked);
+
+  /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
 
+  /// \brief Create a basic block that will call the specified trap function,
+  /// and emit a conditional branch to it.
+  void EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName);
+
   /// \brief Emit a call to trap or debugtrap and attach function attribute
-  /// "trap-func-name" if specified.
+  /// "trap-func-name" if -ftrap-function is specified.
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
 
+  /// \brief Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if nonempty.
+  llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID,
+   const std::string &TrapFuncName);
+
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void EmitNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2301,7 +2301,7 @@
   }
 
   if (TrapCond)
-EmitTrapCheck(TrapCond);
+EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)
 return;
 
@@ -2381,16 +2381,27 @@
   EmitBlock(Cont);
 }
 
+void CodeGenFunction::EmitSanitizeTrapCheck(llvm::Value *Checked) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
+return EmitTrapCheck(Checked, CGM.getCodeGenOpts().SanitizeTrapFuncName);
+  }
+  return EmitTrapCheck(Checked);
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
+  return EmitTrapCheck(Checked, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per
   // function to save on code size.
   if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
 TrapBB = createBasicBlock("trap");
 Builder.CreateCondBr(Checked, Cont, TrapBB);
 EmitBlock(TrapBB);
-llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
+llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap, TrapFuncName);
 T

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 32769.
jmgao added a comment.

Doc fix, 80 col


http://reviews.llvm.org/D12181

Files:
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/sanitize-trap-function.c

Index: test/CodeGen/sanitize-trap-function.c
===
--- /dev/null
+++ test/CodeGen/sanitize-trap-function.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero | FileCheck %s -check-prefix=NONE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo | FileCheck %s -check-prefix=TRAP -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+
+int f(int x, int y) {
+  // CHECK: call void @llvm.trap() #[[N:[0-9]+]],
+  // CHECK-NEXT: unreachable
+
+  // NONE-NOT: trap-func-name
+  // TRAP: attributes #[[N]] {{.+}} "trap-func-name"="foo"
+  // SANITIZE: attributes #[[N]] {{.+}} "trap-func-name"="bar"
+  return x / y;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -681,6 +681,7 @@
   parseSanitizerKinds("-fsanitize-trap=",
   Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
   Opts.SanitizeTrap);
+  Opts.SanitizeTrapFuncName = Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
 
   Opts.CudaGpuBinaryFileNames =
   Args.getAllArgValues(OPT_fcuda_include_gpubinary);
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2885,13 +2885,26 @@
  ArrayRef DynamicArgs);
 
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
+  /// conditional branch to it, for the -fsanitize checks.
+  void EmitSanitizeTrapCheck(llvm::Value *Checked);
+
+  /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
 
+  /// \brief Create a basic block that will call the specified trap function,
+  /// and emit a conditional branch to it.
+  void EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName);
+
   /// \brief Emit a call to trap or debugtrap and attach function attribute
-  /// "trap-func-name" if specified.
+  /// "trap-func-name" if -ftrap-function is specified.
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
 
+  /// \brief Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if nonempty.
+  llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID,
+   const std::string &TrapFuncName);
+
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void EmitNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2301,7 +2301,7 @@
   }
 
   if (TrapCond)
-EmitTrapCheck(TrapCond);
+EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)
 return;
 
@@ -2381,16 +2381,27 @@
   EmitBlock(Cont);
 }
 
+void CodeGenFunction::EmitSanitizeTrapCheck(llvm::Value *Checked) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
+return EmitTrapCheck(Checked, CGM.getCodeGenOpts().SanitizeTrapFuncName);
+  }
+  return EmitTrapCheck(Checked);
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
+  return EmitTrapCheck(Checked, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per
   // function to save on code size.
   if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
 TrapBB = createBasicBlock("trap");
 Builder.CreateCondBr(Checked, Cont, TrapBB);
 EmitBlock(TrapBB);
-llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap);
+llvm::CallInst *TrapCall = EmitTrapCall(llvm::Intrinsic::trap, TrapFuncName);
 TrapCall->se

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 32776.
jmgao marked 2 inline comments as done.
jmgao added a comment.

Make methods private


http://reviews.llvm.org/D12181

Files:
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/sanitize-trap-function.c

Index: test/CodeGen/sanitize-trap-function.c
===
--- /dev/null
+++ test/CodeGen/sanitize-trap-function.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero | FileCheck %s -check-prefix=NONE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo | FileCheck %s -check-prefix=TRAP -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+
+int f(int x, int y) {
+  // CHECK: call void @llvm.trap() #[[N:[0-9]+]],
+  // CHECK-NEXT: unreachable
+
+  // NONE-NOT: trap-func-name
+  // TRAP: attributes #[[N]] {{.+}} "trap-func-name"="foo"
+  // SANITIZE: attributes #[[N]] {{.+}} "trap-func-name"="bar"
+  return x / y;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -681,6 +681,7 @@
   parseSanitizerKinds("-fsanitize-trap=",
   Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
   Opts.SanitizeTrap);
+  Opts.SanitizeTrapFuncName = Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
 
   Opts.CudaGpuBinaryFileNames =
   Args.getAllArgValues(OPT_fcuda_include_gpubinary);
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2885,13 +2885,28 @@
  ArrayRef DynamicArgs);
 
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
+  /// conditional branch to it, for the -fsanitize checks.
+  void EmitSanitizeTrapCheck(llvm::Value *Checked);
+
+  /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
 
   /// \brief Emit a call to trap or debugtrap and attach function attribute
-  /// "trap-func-name" if specified.
+  /// "trap-func-name" if -ftrap-function is specified.
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
 
+private:
+  /// \brief Create a basic block that will call the specified trap function,
+  /// and emit a conditional branch to it.
+  void EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName);
+
+  /// \brief Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if nonempty.
+  llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID,
+   const std::string &TrapFuncName);
+
+public:
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void EmitNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2301,7 +2301,7 @@
   }
 
   if (TrapCond)
-EmitTrapCheck(TrapCond);
+EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)
 return;
 
@@ -2381,16 +2381,31 @@
   EmitBlock(Cont);
 }
 
+void CodeGenFunction::EmitSanitizeTrapCheck(llvm::Value *Checked) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
+return EmitTrapCheck(Checked, CGM.getCodeGenOpts().SanitizeTrapFuncName);
+  }
+  return EmitTrapCheck(Checked);
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
+  return EmitTrapCheck(Checked, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
+return EmitTrapCall(IntrID, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap down to just one per
   // function to save on code size.
   if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
 TrapBB = createBasicBlock("trap");
 Builder.CreateCondBr(Ch

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 32788.
jmgao marked 7 inline comments as done.
jmgao added a comment.

clang-format, remove \brief from modified doxygen comments.


http://reviews.llvm.org/D12181

Files:
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/sanitize-trap-function.c

Index: test/CodeGen/sanitize-trap-function.c
===
--- /dev/null
+++ test/CodeGen/sanitize-trap-function.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero | FileCheck %s -check-prefix=NONE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo | FileCheck %s -check-prefix=TRAP -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+
+int f(int x, int y) {
+  // CHECK: call void @llvm.trap() #[[N:[0-9]+]],
+  // CHECK-NEXT: unreachable
+
+  // NONE-NOT: trap-func-name
+  // TRAP: attributes #[[N]] {{.+}} "trap-func-name"="foo"
+  // SANITIZE: attributes #[[N]] {{.+}} "trap-func-name"="bar"
+  return x / y;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -681,6 +681,8 @@
   parseSanitizerKinds("-fsanitize-trap=",
   Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
   Opts.SanitizeTrap);
+  Opts.SanitizeTrapFuncName =
+  Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
 
   Opts.CudaGpuBinaryFileNames =
   Args.getAllArgValues(OPT_fcuda_include_gpubinary);
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2884,14 +2884,29 @@
  StringRef CheckName, ArrayRef StaticArgs,
  ArrayRef DynamicArgs);
 
+  /// Create a basic block that will call the trap intrinsic and emit a
+  /// conditional branch to it, for the -fsanitize checks.
+  void EmitSanitizeTrapCheck(llvm::Value *Checked);
+
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
 
-  /// \brief Emit a call to trap or debugtrap and attach function attribute
-  /// "trap-func-name" if specified.
+  /// Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if -ftrap-function is specified.
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
 
+ private:
+  /// \brief Create a basic block that will call the specified trap function,
+  /// and emit a conditional branch to it.
+  void EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName);
+
+  /// \brief Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if nonempty.
+  llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID,
+   const std::string &TrapFuncName);
+
+ public:
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void EmitNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2300,8 +2300,7 @@
 Cond = Cond ? Builder.CreateAnd(Cond, Check) : Check;
   }
 
-  if (TrapCond)
-EmitTrapCheck(TrapCond);
+  if (TrapCond) EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)
 return;
 
@@ -2381,16 +2380,33 @@
   EmitBlock(Cont);
 }
 
+void CodeGenFunction::EmitSanitizeTrapCheck(llvm::Value *Checked) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
+return EmitTrapCheck(Checked, CGM.getCodeGenOpts().SanitizeTrapFuncName);
+  }
+  return EmitTrapCheck(Checked);
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
+  return EmitTrapCheck(Checked, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
+  return EmitTrapCall(IntrID, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
+const std::string &TrapFuncName) {
   llvm::BasicBlock *

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 32833.
jmgao added a comment.

Remove more `\brief`s


http://reviews.llvm.org/D12181

Files:
  docs/UsersManual.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/sanitize-trap-function.c

Index: test/CodeGen/sanitize-trap-function.c
===
--- /dev/null
+++ test/CodeGen/sanitize-trap-function.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero | FileCheck %s -check-prefix=NONE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo | FileCheck %s -check-prefix=TRAP -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=integer-divide-by-zero -fsanitize-trap=integer-divide-by-zero -ftrap-function=foo -fsanitize-trap-function=bar | FileCheck %s -check-prefix=SANITIZE -check-prefix=CHECK
+
+int f(int x, int y) {
+  // CHECK: call void @llvm.trap() #[[N:[0-9]+]],
+  // CHECK-NEXT: unreachable
+
+  // NONE-NOT: trap-func-name
+  // TRAP: attributes #[[N]] {{.+}} "trap-func-name"="foo"
+  // SANITIZE: attributes #[[N]] {{.+}} "trap-func-name"="bar"
+  return x / y;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -681,6 +681,8 @@
   parseSanitizerKinds("-fsanitize-trap=",
   Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
   Opts.SanitizeTrap);
+  Opts.SanitizeTrapFuncName =
+  Args.getLastArgValue(OPT_fsanitize_trap_function_EQ);
 
   Opts.CudaGpuBinaryFileNames =
   Args.getAllArgValues(OPT_fcuda_include_gpubinary);
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2884,14 +2884,29 @@
  StringRef CheckName, ArrayRef StaticArgs,
  ArrayRef DynamicArgs);
 
+  /// Create a basic block that will call the trap intrinsic and emit a
+  /// conditional branch to it, for the -fsanitize checks.
+  void EmitSanitizeTrapCheck(llvm::Value *Checked);
+
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
 
-  /// \brief Emit a call to trap or debugtrap and attach function attribute
-  /// "trap-func-name" if specified.
+  /// Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if -ftrap-function is specified.
   llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID);
 
+ private:
+  /// Create a basic block that will call the specified trap function, and emit
+  /// a conditional branch to it.
+  void EmitTrapCheck(llvm::Value *Checked, const std::string &TrapFuncName);
+
+  /// Emit a call to trap or debugtrap and attach function attribute
+  /// "trap-func-name" if nonempty.
+  llvm::CallInst *EmitTrapCall(llvm::Intrinsic::ID IntrID,
+   const std::string &TrapFuncName);
+
+ public:
   /// \brief Create a check for a function parameter that may potentially be
   /// declared as non-null.
   void EmitNonNullArgCheck(RValue RV, QualType ArgType, SourceLocation ArgLoc,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -2300,8 +2300,7 @@
 Cond = Cond ? Builder.CreateAnd(Cond, Check) : Check;
   }
 
-  if (TrapCond)
-EmitTrapCheck(TrapCond);
+  if (TrapCond) EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)
 return;
 
@@ -2381,16 +2380,33 @@
   EmitBlock(Cont);
 }
 
+void CodeGenFunction::EmitSanitizeTrapCheck(llvm::Value *Checked) {
+  if (!CGM.getCodeGenOpts().SanitizeTrapFuncName.empty()) {
+return EmitTrapCheck(Checked, CGM.getCodeGenOpts().SanitizeTrapFuncName);
+  }
+  return EmitTrapCheck(Checked);
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
+  return EmitTrapCheck(Checked, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
+  return EmitTrapCall(IntrID, CGM.getCodeGenOpts().TrapFuncName);
+}
+
+void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
+const std::string &TrapFuncName) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   // If we're optimizing, collapse all calls to trap dow

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao marked an inline comment as done.


Comment at: lib/CodeGen/CGExpr.cpp:2303
@@ -2302,4 +2302,3 @@
 
-  if (TrapCond)
-EmitTrapCheck(TrapCond);
+  if (TrapCond) EmitSanitizeTrapCheck(TrapCond);
   if (!FatalCond && !RecoverableCond)

Yes


http://reviews.llvm.org/D12181



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

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao marked 4 inline comments as done.
jmgao added a comment.

http://reviews.llvm.org/D12181



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

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao added a comment.

Ping, I think @samsonov was waiting on @rsmith's feedback on the following:

In http://reviews.llvm.org/D12181#229493, @jmgao wrote:

> In http://reviews.llvm.org/D12181#229467, @rsmith wrote:
>
> > In http://reviews.llvm.org/D12181#229358, @rsmith wrote:
> >
> > > Can you please give a brief description of the motivation for this 
> > > change? When would it be appropriate to use this rather than 
> > > `-ftrap-function`?
> >
> >
> > I'd still like an answer to this. It's not clear to me what the purpose of 
> > this is, and why you'd want a custom runtime hook for sanitizer traps but 
> > not other traps. The only time we emit a trap using `-ftrap-function` where 
> > there is no corresponding sanitizer is for `__builtin_trap()`; is the 
> > intention that you still want a normal trap for that builtin?
>
>
> The goal is to be able to give a useful fsanitize-specific error message 
> ("fsanitize trap occurred"), while not lying and saying this for non-sanitize 
> traps.





http://reviews.llvm.org/D12181



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

Re: [PATCH] D12181: [sanitizer] Add -fsanitize-trap-function.

[Josh Gao via cfe-commits]

jmgao added a comment.

With #1, it seems unfortunate to not be able to distinguish between a sanitize 
inserted __builtin_trap and code manually calling it. (Would there be an 
-fsanitize-trap=trap? :-)
With #2, we're worried about the generated code being noticeably worse in the 
unexceptional case than running without the sanitizers.

Compiling the following snippet with -O3 -fsanitize=unsigned-integer-overflow 
-fomit-frame-pointer and additional arguments generates:

  unsigned foo(unsigned a, unsigned b, unsigned c, unsigned d) { return a + b + 
c + d; }

no additional arguments

  foo:
push{r4, r5, r6, r7, r8, lr}
mov r5, r2
mov r2, r1
mov r1, r0
mov r0, #1
mov r8, r3
mov r4, #1
add r6, r1, r2
cmp r6, r1
movhs   r0, #0
cmp r0, #0
bne .LBB0_4
  .LBB0_1:
add r7, r6, r5
cmp r7, r6
movhs   r4, #0
cmp r4, #0
bne .LBB0_5
  .LBB0_2:
add r5, r7, r8
mov r0, #1
cmp r5, r7
movhs   r0, #0
cmp r0, #0
bne .LBB0_6
  .LBB0_3:
mov r0, r5
pop {r4, r5, r6, r7, r8, lr}
bx  lr
  .LBB0_4:
  

-fsanitize-trap=unsigned-integer-overflow

  foo:
add r1, r0, r1
mov r12, #1
cmp r1, r0
mov r0, #1
movhs   r0, #0
cmp r0, #0
bne .LBB0_3
  @ BB#1:
add r2, r1, r2
cmp r2, r1
movhs   r12, #0
cmp r12, #0
bne .LBB0_3
  @ BB#2:
add r0, r2, r3
mov r1, #1
cmp r0, r2
movhs   r1, #0
cmp r1, #0
bxeqlr
  .LBB0_3:
.long   3892305662  @ trap

-fsanitize-trap=unsigned-integer-overflow -fsanitize-trap-function=sanitize_trap

  foo:
push{r11, lr}  ; Not quite perfect, but still better
add r1, r0, r1
mov r12, #1
cmp r1, r0
mov r0, #1
movhs   r0, #0
cmp r0, #0
bne .LBB0_3
  @ BB#1:
add r2, r1, r2
cmp r2, r1
movhs   r12, #0
cmp r12, #0
bne .LBB0_3
  @ BB#2:
add r0, r2, r3
mov r1, #1
cmp r0, r2
movhs   r1, #0
cmp r1, #0
popeq   {r11, lr}
bxeqlr
  .LBB0_3:
bl  sanitize_trap(PLT)


http://reviews.llvm.org/D12181



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

[PATCH] D21163: Strip Android version when looking up toolchain paths.

[Josh Gao via cfe-commits]

jmgao created this revision.
jmgao added reviewers: srhines, danalbert.
jmgao added a subscriber: cfe-commits.
Herald added subscribers: srhines, danalbert, tberghammer, aemerson.

Android target triples can include a version number in the abi field
(e.g. 'aarch64-linux-android21'), used for checking for availability.
However, the driver was searching for toolchain binaries using the
passed in triple as a prefix.

http://reviews.llvm.org/D21163

Files:
  lib/Driver/Driver.cpp

Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2283,7 +2283,15 @@
 const char *Tool, const ToolChain &TC,
 SmallVectorImpl &Names) const {
   // FIXME: Needs a better variable than DefaultTargetTriple
-  Names.emplace_back(DefaultTargetTriple + "-" + Tool);
+  StringRef Triple = DefaultTargetTriple;
+
+  // On Android, the target triple can include a version number that needs to
+  // be stripped.
+  if (TC.getTriple().isAndroid()) {
+Triple = Triple.rtrim("0123456789");
+  }
+
+  Names.emplace_back((Triple + "-" + Tool).str());
   Names.emplace_back(Tool);
 
   // Allow the discovery of tools prefixed with LLVM's default target triple.


Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2283,7 +2283,15 @@
 const char *Tool, const ToolChain &TC,
 SmallVectorImpl &Names) const {
   // FIXME: Needs a better variable than DefaultTargetTriple
-  Names.emplace_back(DefaultTargetTriple + "-" + Tool);
+  StringRef Triple = DefaultTargetTriple;
+
+  // On Android, the target triple can include a version number that needs to
+  // be stripped.
+  if (TC.getTriple().isAndroid()) {
+Triple = Triple.rtrim("0123456789");
+  }
+
+  Names.emplace_back((Triple + "-" + Tool).str());
   Names.emplace_back(Tool);
 
   // Allow the discovery of tools prefixed with LLVM's default target triple.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21163: Strip Android version when looking up toolchain paths.

[Josh Gao via cfe-commits]

jmgao updated this revision to Diff 60294.
jmgao added a comment.

Add test.


http://reviews.llvm.org/D21163

Files:
  lib/Driver/Driver.cpp
  test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld
  test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe
  test/Driver/android-triple-version.c

Index: test/Driver/android-triple-version.c
===
--- /dev/null
+++ test/Driver/android-triple-version.c
@@ -0,0 +1,10 @@
+// Android's target triples can contain a version number in the environment
+// field (e.g. arm-linux-androideabi9).
+// Make sure that any version is stripped when finding toolchain binaries.
+
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi %s 2>&1 | FileCheck %s
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi9 %s 2>&1 | FileCheck %s
+
+// CHECK: arm-linux-androideabi-ld
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2343,7 +2343,15 @@
 const char *Tool, const ToolChain &TC,
 SmallVectorImpl &Names) const {
   // FIXME: Needs a better variable than DefaultTargetTriple
-  Names.emplace_back(DefaultTargetTriple + "-" + Tool);
+  StringRef Triple = DefaultTargetTriple;
+
+  // On Android, the target triple can include a version number that needs to
+  // be stripped.
+  if (TC.getTriple().isAndroid()) {
+Triple = Triple.rtrim("0123456789");
+  }
+
+  Names.emplace_back((Triple + "-" + Tool).str());
   Names.emplace_back(Tool);
 
   // Allow the discovery of tools prefixed with LLVM's default target triple.


Index: test/Driver/android-triple-version.c
===
--- /dev/null
+++ test/Driver/android-triple-version.c
@@ -0,0 +1,10 @@
+// Android's target triples can contain a version number in the environment
+// field (e.g. arm-linux-androideabi9).
+// Make sure that any version is stripped when finding toolchain binaries.
+
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi %s 2>&1 | FileCheck %s
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi9 %s 2>&1 | FileCheck %s
+
+// CHECK: arm-linux-androideabi-ld
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2343,7 +2343,15 @@
 const char *Tool, const ToolChain &TC,
 SmallVectorImpl &Names) const {
   // FIXME: Needs a better variable than DefaultTargetTriple
-  Names.emplace_back(DefaultTargetTriple + "-" + Tool);
+  StringRef Triple = DefaultTargetTriple;
+
+  // On Android, the target triple can include a version number that needs to
+  // be stripped.
+  if (TC.getTriple().isAndroid()) {
+Triple = Triple.rtrim("0123456789");
+  }
+
+  Names.emplace_back((Triple + "-" + Tool).str());
   Names.emplace_back(Tool);
 
   // Allow the discovery of tools prefixed with LLVM's default target triple.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21163: Strip Android version when looking up toolchain paths.

[Josh Gao via cfe-commits]

jmgao added a comment.

Thanks for the review!


http://reviews.llvm.org/D21163



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

Re: [PATCH] D21163: Strip Android version when looking up toolchain paths.

[Josh Gao via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL272413: Strip Android version when looking up toolchain 
paths. (authored by jmgao).

Changed prior to commit:
  http://reviews.llvm.org/D21163?vs=60294&id=60379#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D21163

Files:
  cfe/trunk/lib/Driver/Driver.cpp
  
cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld
  
cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe
  cfe/trunk/test/Driver/android-triple-version.c

Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -2343,7 +2343,15 @@
 const char *Tool, const ToolChain &TC,
 SmallVectorImpl &Names) const {
   // FIXME: Needs a better variable than DefaultTargetTriple
-  Names.emplace_back(DefaultTargetTriple + "-" + Tool);
+  StringRef Triple = DefaultTargetTriple;
+
+  // On Android, the target triple can include a version number that needs to
+  // be stripped.
+  if (TC.getTriple().isAndroid()) {
+Triple = Triple.rtrim("0123456789");
+  }
+
+  Names.emplace_back((Triple + "-" + Tool).str());
   Names.emplace_back(Tool);
 
   // Allow the discovery of tools prefixed with LLVM's default target triple.
Index: cfe/trunk/test/Driver/android-triple-version.c
===
--- cfe/trunk/test/Driver/android-triple-version.c
+++ cfe/trunk/test/Driver/android-triple-version.c
@@ -0,0 +1,10 @@
+// Android's target triples can contain a version number in the environment
+// field (e.g. arm-linux-androideabi9).
+// Make sure that any version is stripped when finding toolchain binaries.
+
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi %s 2>&1 | FileCheck %s
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi9 %s 2>&1 | FileCheck %s
+
+// CHECK: arm-linux-androideabi-ld


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -2343,7 +2343,15 @@
 const char *Tool, const ToolChain &TC,
 SmallVectorImpl &Names) const {
   // FIXME: Needs a better variable than DefaultTargetTriple
-  Names.emplace_back(DefaultTargetTriple + "-" + Tool);
+  StringRef Triple = DefaultTargetTriple;
+
+  // On Android, the target triple can include a version number that needs to
+  // be stripped.
+  if (TC.getTriple().isAndroid()) {
+Triple = Triple.rtrim("0123456789");
+  }
+
+  Names.emplace_back((Triple + "-" + Tool).str());
   Names.emplace_back(Tool);
 
   // Allow the discovery of tools prefixed with LLVM's default target triple.
Index: cfe/trunk/test/Driver/android-triple-version.c
===
--- cfe/trunk/test/Driver/android-triple-version.c
+++ cfe/trunk/test/Driver/android-triple-version.c
@@ -0,0 +1,10 @@
+// Android's target triples can contain a version number in the environment
+// field (e.g. arm-linux-androideabi9).
+// Make sure that any version is stripped when finding toolchain binaries.
+
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi %s 2>&1 | FileCheck %s
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi9 %s 2>&1 | FileCheck %s
+
+// CHECK: arm-linux-androideabi-ld
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r272413 - Strip Android version when looking up toolchain paths.

[Josh Gao via cfe-commits]

Author: jmgao
Date: Fri Jun 10 13:30:33 2016
New Revision: 272413

URL: http://llvm.org/viewvc/llvm-project?rev=272413&view=rev
Log:
Strip Android version when looking up toolchain paths.

Summary:
Android target triples can include a version number in the abi field
(e.g. 'aarch64-linux-android21'), used for checking for availability.
However, the driver was searching for toolchain binaries using the
passed in triple as a prefix.

Reviewers: srhines, danalbert, t.p.northover

Subscribers: t.p.northover, aemerson, tberghammer, danalbert, srhines, 
cfe-commits

Differential Revision: http://reviews.llvm.org/D21163

Added:
cfe/trunk/test/Driver/Inputs/android_triple_version/
cfe/trunk/test/Driver/Inputs/android_triple_version/bin/

cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld
   (with props)

cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe
   (with props)
cfe/trunk/test/Driver/android-triple-version.c
Modified:
cfe/trunk/lib/Driver/Driver.cpp

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=272413&r1=272412&r2=272413&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Fri Jun 10 13:30:33 2016
@@ -2343,7 +2343,15 @@ void Driver::generatePrefixedToolNames(
 const char *Tool, const ToolChain &TC,
 SmallVectorImpl &Names) const {
   // FIXME: Needs a better variable than DefaultTargetTriple
-  Names.emplace_back(DefaultTargetTriple + "-" + Tool);
+  StringRef Triple = DefaultTargetTriple;
+
+  // On Android, the target triple can include a version number that needs to
+  // be stripped.
+  if (TC.getTriple().isAndroid()) {
+Triple = Triple.rtrim("0123456789");
+  }
+
+  Names.emplace_back((Triple + "-" + Tool).str());
   Names.emplace_back(Tool);
 
   // Allow the discovery of tools prefixed with LLVM's default target triple.

Added: 
cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld?rev=272413&view=auto
==
(empty)

Propchange: 
cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld
--
svn:executable = *

Added: 
cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe?rev=272413&view=auto
==
(empty)

Propchange: 
cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe
--
svn:executable = *

Added: cfe/trunk/test/Driver/android-triple-version.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-triple-version.c?rev=272413&view=auto
==
--- cfe/trunk/test/Driver/android-triple-version.c (added)
+++ cfe/trunk/test/Driver/android-triple-version.c Fri Jun 10 13:30:33 2016
@@ -0,0 +1,10 @@
+// Android's target triples can contain a version number in the environment
+// field (e.g. arm-linux-androideabi9).
+// Make sure that any version is stripped when finding toolchain binaries.
+
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi %s 2>&1 | FileCheck %s
+// RUN: env "PATH=%S/Inputs/android_triple_version/bin" \
+// RUN: %clang -### -target arm-linux-androideabi9 %s 2>&1 | FileCheck %s
+
+// CHECK: arm-linux-androideabi-ld


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

Re: r272413 - Strip Android version when looking up toolchain paths.

[Josh Gao via cfe-commits]

Sorry, I got some other buildbot failure emails in which the test didn't
fail, and I assumed they were all the same. I'll be more careful in the
future.

-Josh
On Jun 10, 2016 10:04 PM, "Chandler Carruth"  wrote:

> This broke several bots that have now been red most of the day. Here are
> recent builds that show the failure:
> http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/4611
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/6331
> http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/15622
>
> I suspect this is because using PATH in this way as part of a test doesn't
> work well, but I'll let you look at what the best option is for fixing
> this. For now I've reverted it in
>
> Please watch the bots when committing in the future.
>
> -Chandler
>
> On Fri, Jun 10, 2016 at 11:37 AM Josh Gao via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: jmgao
>> Date: Fri Jun 10 13:30:33 2016
>> New Revision: 272413
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=272413&view=rev
>> Log:
>> Strip Android version when looking up toolchain paths.
>>
>> Summary:
>> Android target triples can include a version number in the abi field
>> (e.g. 'aarch64-linux-android21'), used for checking for availability.
>> However, the driver was searching for toolchain binaries using the
>> passed in triple as a prefix.
>>
>> Reviewers: srhines, danalbert, t.p.northover
>>
>> Subscribers: t.p.northover, aemerson, tberghammer, danalbert, srhines,
>> cfe-commits
>>
>> Differential Revision: http://reviews.llvm.org/D21163
>>
>> Added:
>> cfe/trunk/test/Driver/Inputs/android_triple_version/
>> cfe/trunk/test/Driver/Inputs/android_triple_version/bin/
>>
>> cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld
>>  (with props)
>>
>> cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe
>>  (with props)
>> cfe/trunk/test/Driver/android-triple-version.c
>> Modified:
>> cfe/trunk/lib/Driver/Driver.cpp
>>
>> Modified: cfe/trunk/lib/Driver/Driver.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=272413&r1=272412&r2=272413&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Driver/Driver.cpp (original)
>> +++ cfe/trunk/lib/Driver/Driver.cpp Fri Jun 10 13:30:33 2016
>> @@ -2343,7 +2343,15 @@ void Driver::generatePrefixedToolNames(
>>  const char *Tool, const ToolChain &TC,
>>  SmallVectorImpl &Names) const {
>>// FIXME: Needs a better variable than DefaultTargetTriple
>> -  Names.emplace_back(DefaultTargetTriple + "-" + Tool);
>> +  StringRef Triple = DefaultTargetTriple;
>> +
>> +  // On Android, the target triple can include a version number that
>> needs to
>> +  // be stripped.
>> +  if (TC.getTriple().isAndroid()) {
>> +Triple = Triple.rtrim("0123456789");
>> +  }
>> +
>> +  Names.emplace_back((Triple + "-" + Tool).str());
>>Names.emplace_back(Tool);
>>
>>// Allow the discovery of tools prefixed with LLVM's default target
>> triple.
>>
>> Added:
>> cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld?rev=272413&view=auto
>>
>> ==
>> (empty)
>>
>> Propchange:
>> cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld
>>
>> --
>> svn:executable = *
>>
>> Added:
>> cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe?rev=272413&view=auto
>>
>> ==
>> (empty)
>>
>> Propchange:
>> cfe/trunk/test/Driver/Inputs/android_triple_version/bin/arm-linux-androideabi-ld.exe
>>
>> --
>> svn:executable = *
>>
>> Added: cfe/trunk/test/Driver/android-triple-v