xiaobai created this revision.
xiaobai added reviewers: aprantl, labath, JDevlieghere, clayborg.
Herald added subscribers: jdoerfert, srhines.

This behavior was originally added in rL252264 
<https://reviews.llvm.org/rL252264> (git commit 76a7f365da)
in order to be extra careful with handling platforms like watchos and tvos.
However, as far as triples go, those two (and others) are treated as OSes and
not environments, so that should not really apply here.

Additionally, this behavior is incorrect and can lead to incorrect ArchSpecs.
Because android is specified as an environment and not an OS, not propogating
the environment can lead to modules and targets being misidentified.


https://reviews.llvm.org/D58664

Files:
  include/lldb/Utility/ArchSpec.h
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp

Index: unittests/Utility/ArchSpecTest.cpp
===================================================================
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -134,22 +134,46 @@
 }
 
 TEST(ArchSpecTest, MergeFrom) {
-  ArchSpec A;
-  ArchSpec B("x86_64-pc-linux");
-
-  EXPECT_FALSE(A.IsValid());
-  ASSERT_TRUE(B.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
-
-  A.MergeFrom(B);
-  ASSERT_TRUE(A.IsValid());
-  EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
-  EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
-  EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
-  EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  {
+    ArchSpec A;
+    ArchSpec B("x86_64-pc-linux");
+
+    EXPECT_FALSE(A.IsValid());
+    ASSERT_TRUE(B.IsValid());
+    EXPECT_EQ(llvm::Triple::ArchType::x86_64, B.GetTriple().getArch());
+    EXPECT_EQ(llvm::Triple::VendorType::PC, B.GetTriple().getVendor());
+    EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+    EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, B.GetCore());
+
+    A.MergeFrom(B);
+    ASSERT_TRUE(A.IsValid());
+    EXPECT_EQ(llvm::Triple::ArchType::x86_64, A.GetTriple().getArch());
+    EXPECT_EQ(llvm::Triple::VendorType::PC, A.GetTriple().getVendor());
+    EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+    EXPECT_EQ(ArchSpec::eCore_x86_64_x86_64, A.GetCore());
+  }
+  {
+    ArchSpec A("aarch64-unknown-unknown-unknown");
+    ArchSpec B("aarch64-unknown-linux-android");
+
+    EXPECT_TRUE(A.IsValid());
+    EXPECT_TRUE(B.IsValid());
+
+    EXPECT_EQ(llvm::Triple::ArchType::aarch64, B.GetTriple().getArch());
+    EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+              B.GetTriple().getVendor());
+    EXPECT_EQ(llvm::Triple::OSType::Linux, B.GetTriple().getOS());
+    EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+              B.GetTriple().getEnvironment());
+
+    A.MergeFrom(B);
+    EXPECT_EQ(llvm::Triple::ArchType::aarch64, A.GetTriple().getArch());
+    EXPECT_EQ(llvm::Triple::VendorType::UnknownVendor,
+              A.GetTriple().getVendor());
+    EXPECT_EQ(llvm::Triple::OSType::Linux, A.GetTriple().getOS());
+    EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
+              A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
@@ -232,3 +256,58 @@
   EXPECT_FALSE(ArchSpec());
   EXPECT_TRUE(ArchSpec("x86_64-pc-linux"));
 }
+
+TEST(ArchSpecTest, TripleComponentsWereSpecified) {
+  {
+    ArchSpec A("x86-unknown-unknown");
+    ASSERT_FALSE(A.TripleVendorWasSpecified());
+    ASSERT_FALSE(A.TripleOSWasSpecified());
+    ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+    ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+    ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+    ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+    ArchSpec A("arm-pc-linux-android");
+    ASSERT_TRUE(A.TripleVendorWasSpecified());
+    ASSERT_TRUE(A.TripleOSWasSpecified());
+    ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+    ASSERT_FALSE(A.TripleVendorIsUnspecifiedUnknown());
+    ASSERT_FALSE(A.TripleOSIsUnspecifiedUnknown());
+    ASSERT_FALSE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+    ArchSpec A("aarch64-unknown-linux-android");
+    ASSERT_TRUE(A.TripleVendorWasSpecified());
+    ASSERT_TRUE(A.TripleOSWasSpecified());
+    ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+    ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+    ASSERT_FALSE(A.TripleOSIsUnspecifiedUnknown());
+    ASSERT_FALSE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+    ArchSpec A("");
+    ASSERT_FALSE(A.TripleVendorWasSpecified());
+    ASSERT_FALSE(A.TripleOSWasSpecified());
+    ASSERT_FALSE(A.TripleEnvironmentWasSpecified());
+
+    ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+    ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+    ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+  {
+    ArchSpec A("arm---");
+    // Note: llvm::Triple::normalize interprets empty-string as "unknown"
+    // instead of unspecified.
+    ASSERT_TRUE(A.TripleVendorWasSpecified());
+    ASSERT_TRUE(A.TripleOSWasSpecified());
+    ASSERT_TRUE(A.TripleEnvironmentWasSpecified());
+
+    ASSERT_TRUE(A.TripleVendorIsUnspecifiedUnknown());
+    ASSERT_TRUE(A.TripleOSIsUnspecifiedUnknown());
+    ASSERT_TRUE(A.TripleEnvironmentIsUnspecifiedUnknown());
+  }
+}
Index: source/Utility/ArchSpec.cpp
===================================================================
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -903,10 +903,9 @@
     if (other.GetCore() != eCore_uknownMach64)
       UpdateCore();
   }
-  if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
-      !TripleVendorWasSpecified()) {
-    if (other.TripleVendorWasSpecified())
-      GetTriple().setEnvironment(other.GetTriple().getEnvironment());
+  if (TripleEnvironmentIsUnspecifiedUnknown() &&
+      !other.TripleEnvironmentIsUnspecifiedUnknown()) {
+    GetTriple().setEnvironment(other.GetTriple().getEnvironment());
   }
   // If this and other are both arm ArchSpecs and this ArchSpec is a generic
   // "some kind of arm" spec but the other ArchSpec is a specific arm core,
Index: include/lldb/Utility/ArchSpec.h
===================================================================
--- include/lldb/Utility/ArchSpec.h
+++ include/lldb/Utility/ArchSpec.h
@@ -377,19 +377,19 @@
   }
 
   bool TripleVendorIsUnspecifiedUnknown() const {
-    return m_triple.getVendor() == llvm::Triple::UnknownVendor &&
-           m_triple.getVendorName().empty();
+    return m_triple.getVendor() == llvm::Triple::UnknownVendor;
   }
 
   bool TripleOSWasSpecified() const { return !m_triple.getOSName().empty(); }
 
-  bool TripleEnvironmentWasSpecified() const {
-    return !m_triple.getEnvironmentName().empty();
+  bool TripleOSIsUnspecifiedUnknown() const {
+    return m_triple.getOS() == llvm::Triple::UnknownOS;
   }
 
-  bool TripleOSIsUnspecifiedUnknown() const {
-    return m_triple.getOS() == llvm::Triple::UnknownOS &&
-           m_triple.getOSName().empty();
+  bool TripleEnvironmentWasSpecified() const { return m_triple.hasEnvironment(); }
+
+  bool TripleEnvironmentIsUnspecifiedUnknown() const {
+    return m_triple.getEnvironment() == llvm::Triple::UnknownEnvironment;
   }
 
   //------------------------------------------------------------------
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to