[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D55134#3224339 , @ychen wrote:

> FWIW, ASTUnit seems to save an effective triple while the current AST uses 
> the nominal triple. (for example, `armv7a-xx-xx-eabihf` vs 
> `armv7a-xx-xx-eabi`). I'm not sure if there is a good solution other than 
> using heuristics to allow some differences.

@ychen, thanks for the notice! I hope we'll have time to address the issue soon.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55134

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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.
Herald added a reviewer: Szelethus.
Herald added a subscriber: steakhal.

FWIW, ASTUnit seems to save an effective triple while the current AST uses the 
nominal triple. (for example, `armv7a-xx-xx-eabihf` vs `armv7a-xx-xx-eabi`). 
I'm not sure if there is a good solution other than using heuristics to allow 
some differences.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55134

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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL348610: [CTU] Add triple/lang mismatch handling (authored by 
martong, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55134?vs=176687=177228#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55134

Files:
  cfe/trunk/include/clang/Basic/DiagnosticCrossTUKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
  cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
  cfe/trunk/test/Analysis/ctu-different-triples.cpp
  cfe/trunk/test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: cfe/trunk/test/Analysis/ctu-different-triples.cpp
===
--- cfe/trunk/test/Analysis/ctu-different-triples.cpp
+++ cfe/trunk/test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp.externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_analyze_cc1 -triple powerpc64-montavista-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: cfe/trunk/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- cfe/trunk/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ cfe/trunk/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp.externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
===
--- cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
+++ cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
@@ -33,6 +33,7 @@
 namespace cross_tu {
 
 namespace {
+
 #define DEBUG_TYPE "CrossTranslationUnit"
 STATISTIC(NumGetCTUCalled, "The # of getCTUDefinition function called");
 STATISTIC(
@@ -41,6 +42,37 @@
 STATISTIC(NumGetCTUSuccess,
   "The # of getCTUDefinition successfully returned the "
   "requested function's body");
+STATISTIC(NumTripleMismatch, "The # of triple mismatches");
+STATISTIC(NumLangMismatch, "The # of language mismatches");
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
 
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
@@ -65,6 +97,10 @@
   return "Failed to load external AST source.";
 case 

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gábor Horváth via Phabricator via cfe-commits]

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

LG!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> We should probably prefer %clang_analyze_cc1 to %clang_cc1 -analyze here too.

Ok, changed that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176687.
martong added a comment.

- Use clang_analyze_cc1


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_analyze_cc1 -triple powerpc64-montavista-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,31 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const llvm::Triple  = Context.getTargetInfo().getTriple();
+  const llvm::Triple  =
+  Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  // known parts only.
+  if 

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176686.
martong marked an inline comment as done.
martong added a comment.

- Add a case to emitCrossTUDiagnostics


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only \
+// RUN:   -analyze -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,31 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const llvm::Triple  = Context.getTargetInfo().getTriple();
+  const llvm::Triple  =
+  Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext 

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong marked 2 inline comments as done.
martong added inline comments.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+// diagnostics.
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();

xazax.hun wrote:
> martong wrote:
> > xazax.hun wrote:
> > > I think we should not emit an error here. It should be up to the caller 
> > > (the user of the library) to decide if she wants to handle this as an 
> > > error, warnings, or just suppress these kinds of problems. I would rather 
> > > extend `emitCrossTUDiagnostics` as a shorthand for the user if emitting 
> > > an error is the desired behavior.
> >  I would prefer to exploit the capabilities of the `DiagEngine`, instead of 
> > extending the interface of `CrossTranslationUnitContext`.
> > By using a `DiagGroup` we can upgrade the warning to be an error, so I just 
> > changed it to be like that.
> You do not need the extend the interface. There is already a function for 
> that below. Also, the user might not want to display a warning either but do 
> something else, like generate a log message. 
Ah, okay, I missed that. Now I added a case to that function. Still, I think 
having a `DiagGroup` is useful anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

We should probably prefer `%clang_analyze_cc1` to `%clang_cc1 -analyze` here 
too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+// diagnostics.
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();

martong wrote:
> xazax.hun wrote:
> > I think we should not emit an error here. It should be up to the caller 
> > (the user of the library) to decide if she wants to handle this as an 
> > error, warnings, or just suppress these kinds of problems. I would rather 
> > extend `emitCrossTUDiagnostics` as a shorthand for the user if emitting an 
> > error is the desired behavior.
>  I would prefer to exploit the capabilities of the `DiagEngine`, instead of 
> extending the interface of `CrossTranslationUnitContext`.
> By using a `DiagGroup` we can upgrade the warning to be an error, so I just 
> changed it to be like that.
You do not need the extend the interface. There is already a function for that 
below. Also, the user might not want to display a warning either but do 
something else, like generate a log message. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176672.
martong added a comment.

- Forgot to rename err_ctu_incompat_triple -> warn_ctu_incompat_triple


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only \
+// RUN:   -analyze -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const auto  = Context.getTargetInfo().getTriple();
+  const auto  = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: include/clang/Basic/DiagnosticCrossTUKinds.td:19
+
+def err_ctu_incompat_triple : Error<
+  "imported AST from '%0' had been generated for a different target, current: 
%1, imported: %2">;

xazax.hun wrote:
> I am not sure if we want this to be an error. The analysis could continue 
> without any problems if we do not actually merge the two ASTs. So maybe 
> having a warning only is better. My concern is that once we have this error, 
> there would be no way to analyze mixed language (C/C++) projects cleanly 
> using CTU.
Okay, I agree, I changed it to be a warning by default.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+// diagnostics.
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();

xazax.hun wrote:
> I think we should not emit an error here. It should be up to the caller (the 
> user of the library) to decide if she wants to handle this as an error, 
> warnings, or just suppress these kinds of problems. I would rather extend 
> `emitCrossTUDiagnostics` as a shorthand for the user if emitting an error is 
> the desired behavior.
 I would prefer to exploit the capabilities of the `DiagEngine`, instead of 
extending the interface of `CrossTranslationUnitContext`.
By using a `DiagGroup` we can upgrade the warning to be an error, so I just 
changed it to be like that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176671.
martong marked 4 inline comments as done.
martong added a comment.

- Diagnose a warning (which may be upgradable to an error)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,22 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only \
+// RUN:   -analyze -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -Werror=ctu \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const auto  = Context.getTargetInfo().getTriple();
+  const auto  = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are 

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun requested changes to this revision.
xazax.hun added inline comments.
This revision now requires changes to proceed.



Comment at: include/clang/Basic/DiagnosticCrossTUKinds.td:19
+
+def err_ctu_incompat_triple : Error<
+  "imported AST from '%0' had been generated for a different target, current: 
%1, imported: %2">;

I am not sure if we want this to be an error. The analysis could continue 
without any problems if we do not actually merge the two ASTs. So maybe having 
a warning only is better. My concern is that once we have this error, there 
would be no way to analyze mixed language (C/C++) projects cleanly using CTU.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+// diagnostics.
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();

I think we should not emit an error here. It should be up to the caller (the 
user of the library) to decide if she wants to handle this as an error, 
warnings, or just suppress these kinds of problems. I would rather extend 
`emitCrossTUDiagnostics` as a shorthand for the user if emitting an error is 
the desired behavior.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176584.
martong added a comment.

- Break long RUN lines


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,21 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only \
+// RUN:   -analyze -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir \
+// RUN:   -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const auto  = Context.getTargetInfo().getTriple();
+  const auto  = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  // known parts only.
+  if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
+// TODO: Pass the SourceLocation of the CallExpression for 

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Aleksei Sidorin via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> Hah. Do we support CTU for other languages, like ObjC and ObjC++? Can this be 
> an issue there?

I really don't know. We never tried it, our focus is on C and C++ for now. 
Unfortunately, there is nobody with ObjC/C++ knowledge and (more importantly) 
with interest to try CTU and report any errors.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks for the review! I have updated the patch according to your comments.




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:31
 
+namespace llvm {
+// Same as Triple's equality operator, but we check a field only if that is

a_sidorin wrote:
> Why don't we place it into the anon namespace just below?
Ok, I moved it into the unnamed namespace below.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:35
+bool hasEqualKnownFields(const Triple , const Triple ) {
+  return ((Lhs.getArch() != Triple::UnknownArch &&
+   Rhs.getArch() != Triple::UnknownArch)

a_sidorin wrote:
> This has to be split, probably with early returns. Example:
> ```if (Lhs.getArch() != Triple::UnknownArch && Rhs.getArch() != 
> Triple::UnknownArch &&
>   Lhs.getArch() != Rhs.getArch()
>   return false;
> ...```
Ok, I split that up with early returns.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:59
+}
+}
+

a_sidorin wrote:
> Szelethus wrote:
> > `// end of namespace llvm`
> `// end namespace llvm` is much more common.
Moved to the unnamed namespace.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176417.
martong marked 13 inline comments as done.
martong added a comment.

- Address review comments


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,16 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -32,6 +32,36 @@
 namespace cross_tu {
 
 namespace {
+
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const llvm::Triple , const llvm::Triple ) {
+  using llvm::Triple;
+  if (Lhs.getArch() != Triple::UnknownArch &&
+  Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
+return false;
+  if (Lhs.getSubArch() != Triple::NoSubArch &&
+  Rhs.getSubArch() != Triple::NoSubArch &&
+  Lhs.getSubArch() != Rhs.getSubArch())
+return false;
+  if (Lhs.getVendor() != Triple::UnknownVendor &&
+  Rhs.getVendor() != Triple::UnknownVendor &&
+  Lhs.getVendor() != Rhs.getVendor())
+return false;
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
+  Lhs.getOS() != Rhs.getOS())
+return false;
+  if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Rhs.getEnvironment() != Triple::UnknownEnvironment &&
+  Lhs.getEnvironment() != Rhs.getEnvironment())
+return false;
+  if (Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Rhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+  Lhs.getObjectFormat() != Rhs.getObjectFormat())
+return false;
+  return true;
+}
+
 // FIXME: This class is will be removed after the transition to llvm::Error.
 class IndexErrorCategory : public std::error_category {
 public:
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const auto  = Context.getTargetInfo().getTriple();
+  const auto  = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  // known parts only.
+  if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
+// TODO: Pass the SourceLocation of the CallExpression for more precise
+// diagnostics.
+

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,
Please find my comments inline.

> Hah. Do we support CTU for other languages, like ObjC and ObjC++? Can this be 
> an issue there?

That's a good question. In Samsung, CTU hasn't been tested on ObjC code. 
Upstream CTU supports only FunctionDecls as well so its ObjC status is 
questionable..




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:31
 
+namespace llvm {
+// Same as Triple's equality operator, but we check a field only if that is

Why don't we place it into the anon namespace just below?



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:35
+bool hasEqualKnownFields(const Triple , const Triple ) {
+  return ((Lhs.getArch() != Triple::UnknownArch &&
+   Rhs.getArch() != Triple::UnknownArch)

This has to be split, probably with early returns. Example:
```if (Lhs.getArch() != Triple::UnknownArch && Rhs.getArch() != 
Triple::UnknownArch &&
  Lhs.getArch() != Rhs.getArch()
  return false;
...```



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:47
+  : true) &&
+ ((Lhs.getOS() != Triple::UnknownOS && Rhs.getOS() != 
Triple::UnknownOS)
+  ? Lhs.getOS() == Rhs.getOS()

We can use `Triple::isOSUnknown()` instead.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:59
+}
+}
+

Szelethus wrote:
> `// end of namespace llvm`
`// end namespace llvm` is much more common.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:203
 
+  const auto& TripleTo = Context.getTargetInfo().getTriple();
+  const auto& TripleFrom = Unit->getASTContext().getTargetInfo().getTriple();

Reference char should stick to the variable, not to the type.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:211
+// TODO pass the SourceLocation of the CallExpression for more precise
+// diagnostics
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)

Comments should end with a dot. Same below.



Comment at: test/Analysis/ctu-unknown-parts-in-triples.cpp:8
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze 
-analyzer-checker=core,debug.ExprInspection  -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir 
-verify %s
+

Two spaces after `ExprInspection`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Hah. Do we support CTU for other languages, like ObjC and ObjC++? Can this be 
an issue there?




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:59
+}
+}
+

`// end of namespace llvm`



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:210
+  if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
+// TODO pass the SourceLocation of the CallExpression for more precise
+// diagnostics

`TODO: `



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:214
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();
+// TODO Add statistics here
+return llvm::make_error(index_error_code::triple_mismatch);

Use punctuation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134



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

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 176172.
martong added a comment.

- Add lit tests


Repository:
  rC Clang

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

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/ctu-different-triples.cpp
  test/Analysis/ctu-unknown-parts-in-triples.cpp

Index: test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -0,0 +1,16 @@
+// We do not expect any error when one part of the triple is unknown, but other
+// known parts are equal.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection  -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir -verify %s
+
+// expected-no-diagnostics
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: test/Analysis/ctu-different-triples.cpp
===
--- /dev/null
+++ test/Analysis/ctu-different-triples.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple powerpc64-montavista-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir -verify %s
+
+// We expect an error in this file, but without a location.
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+
+int f(int);
+
+int main() {
+  return f(5);
+}
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -28,6 +28,36 @@
 #include 
 #include 
 
+namespace llvm {
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const Triple , const Triple ) {
+  return ((Lhs.getArch() != Triple::UnknownArch &&
+   Rhs.getArch() != Triple::UnknownArch)
+  ? Lhs.getArch() == Rhs.getArch()
+  : true) &&
+ ((Lhs.getSubArch() != Triple::NoSubArch &&
+   Rhs.getSubArch() != Triple::NoSubArch)
+  ? Lhs.getSubArch() == Rhs.getSubArch()
+  : true) &&
+ ((Lhs.getVendor() != Triple::UnknownVendor &&
+   Rhs.getVendor() != Triple::UnknownVendor)
+  ? Lhs.getVendor() == Rhs.getVendor()
+  : true) &&
+ ((Lhs.getOS() != Triple::UnknownOS && Rhs.getOS() != Triple::UnknownOS)
+  ? Lhs.getOS() == Rhs.getOS()
+  : true) &&
+ ((Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+   Rhs.getEnvironment() != Triple::UnknownEnvironment)
+  ? Lhs.getEnvironment() == Rhs.getEnvironment()
+  : true) &&
+ ((Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+   Rhs.getObjectFormat() != Triple::UnknownObjectFormat)
+  ? Lhs.getObjectFormat() == Rhs.getObjectFormat()
+  : true);
+}
+}
+
 namespace clang {
 namespace cross_tu {
 
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const auto& TripleTo = Context.getTargetInfo().getTriple();
+  const auto& TripleFrom = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  // known parts only.
+  if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
+// TODO pass the SourceLocation of the CallExpression for more precise
+// diagnostics
+

[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: xazax.hun, a_sidorin.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.

We introduce a strict policy for C++ CTU. It can work across TUs only if
the C++ dialects are the same. We neither allow C vs C++ CTU.  We do this
because the same constructs might be represented with different properties in
the corresponding AST nodes or even the nodes might be completely different (a
struct will be RecordDecl in C, but it will be a CXXRectordDecl in C++, thus it
may cause certain assertions during cast operations).


Repository:
  rC Clang

https://reviews.llvm.org/D55134

Files:
  include/clang/Basic/DiagnosticCrossTUKinds.td
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp

Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -28,6 +28,36 @@
 #include 
 #include 
 
+namespace llvm {
+// Same as Triple's equality operator, but we check a field only if that is
+// known in both instances.
+bool hasEqualKnownFields(const Triple , const Triple ) {
+  return ((Lhs.getArch() != Triple::UnknownArch &&
+   Rhs.getArch() != Triple::UnknownArch)
+  ? Lhs.getArch() == Rhs.getArch()
+  : true) &&
+ ((Lhs.getSubArch() != Triple::NoSubArch &&
+   Rhs.getSubArch() != Triple::NoSubArch)
+  ? Lhs.getSubArch() == Rhs.getSubArch()
+  : true) &&
+ ((Lhs.getVendor() != Triple::UnknownVendor &&
+   Rhs.getVendor() != Triple::UnknownVendor)
+  ? Lhs.getVendor() == Rhs.getVendor()
+  : true) &&
+ ((Lhs.getOS() != Triple::UnknownOS && Rhs.getOS() != Triple::UnknownOS)
+  ? Lhs.getOS() == Rhs.getOS()
+  : true) &&
+ ((Lhs.getEnvironment() != Triple::UnknownEnvironment &&
+   Rhs.getEnvironment() != Triple::UnknownEnvironment)
+  ? Lhs.getEnvironment() == Rhs.getEnvironment()
+  : true) &&
+ ((Lhs.getObjectFormat() != Triple::UnknownObjectFormat &&
+   Rhs.getObjectFormat() != Triple::UnknownObjectFormat)
+  ? Lhs.getObjectFormat() == Rhs.getObjectFormat()
+  : true);
+}
+}
+
 namespace clang {
 namespace cross_tu {
 
@@ -55,6 +85,10 @@
   return "Failed to load external AST source.";
 case index_error_code::failed_to_generate_usr:
   return "Failed to generate USR.";
+case index_error_code::triple_mismatch:
+  return "Triple mismatch";
+case index_error_code::lang_mismatch:
+  return "Language mismatch";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -166,6 +200,30 @@
   assert(>getFileManager() ==
  >getASTContext().getSourceManager().getFileManager());
 
+  const auto& TripleTo = Context.getTargetInfo().getTriple();
+  const auto& TripleFrom = Unit->getASTContext().getTargetInfo().getTriple();
+  // The imported AST had been generated for a different target.
+  // Some parts of the triple in the loaded ASTContext can be unknown while the
+  // very same parts in the target ASTContext are known. Thus we check for the
+  // known parts only.
+  if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
+// TODO pass the SourceLocation of the CallExpression for more precise
+// diagnostics
+Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();
+// TODO Add statistics here
+return llvm::make_error(index_error_code::triple_mismatch);
+  }
+
+  const auto& LangTo = Context.getLangOpts();
+  const auto& LangFrom = Unit->getASTContext().getLangOpts();
+  // FIXME: Currenty we do not support CTU across C++ and C and across
+  // different dialects of C++.
+  if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
+// TODO Add statistics here.
+return llvm::make_error(index_error_code::lang_mismatch);
+  }
+
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
   if (const FunctionDecl *ResultDecl =
   findFunctionInDeclContext(TU, LookupFnName))
Index: include/clang/CrossTU/CrossTranslationUnit.h
===
--- include/clang/CrossTU/CrossTranslationUnit.h
+++ include/clang/CrossTU/CrossTranslationUnit.h
@@ -41,7 +41,9 @@
   missing_definition,
   failed_import,
   failed_to_get_external_ast,
-  failed_to_generate_usr
+  failed_to_generate_usr,
+  triple_mismatch,
+  lang_mismatch
 };
 
 class IndexError : public llvm::ErrorInfo {
Index: include/clang/Basic/DiagnosticCrossTUKinds.td
===
--- include/clang/Basic/DiagnosticCrossTUKinds.td
+++ include/clang/Basic/DiagnosticCrossTUKinds.td
@@ -15,4 +15,7 @@
 
 def err_multiple_def_index : Error<
   "multiple definitions