labath created this revision.

MergeFrom was updating the architecture if the target triple did not
have it set. However, it was leaving the core field as invalid. This
resulted in assertion failures in core file tests as a missing core
meant we were unable to compute the address byte size properly.

Add a unit test for the new behaviour.


https://reviews.llvm.org/D32221

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


Index: unittests/Core/ArchSpecTest.cpp
===================================================================
--- unittests/Core/ArchSpecTest.cpp
+++ unittests/Core/ArchSpecTest.cpp
@@ -134,3 +134,22 @@
   AS = ArchSpec();
   EXPECT_FALSE(AS.SetTriple(""));
 }
+
+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());
+}
Index: source/Core/ArchSpec.cpp
===================================================================
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -834,19 +834,7 @@
 
 bool ArchSpec::SetTriple(const llvm::Triple &triple) {
   m_triple = triple;
-
-  llvm::StringRef arch_name(m_triple.getArchName());
-  const CoreDefinition *core_def = FindCoreDefinition(arch_name);
-  if (core_def) {
-    m_core = core_def->core;
-    // Set the byte order to the default byte order for an architecture.
-    // This can be modified if needed for cases when cores handle both
-    // big and little endian
-    m_byte_order = core_def->default_byte_order;
-  } else {
-    Clear();
-  }
-
+  UpdateCore();
   return IsValid();
 }
 
@@ -994,8 +982,10 @@
     GetTriple().setVendor(other.GetTriple().getVendor());
   if (TripleOSIsUnspecifiedUnknown() && !other.TripleOSIsUnspecifiedUnknown())
     GetTriple().setOS(other.GetTriple().getOS());
-  if (GetTriple().getArch() == llvm::Triple::UnknownArch)
+  if (GetTriple().getArch() == llvm::Triple::UnknownArch) {
     GetTriple().setArch(other.GetTriple().getArch());
+    UpdateCore();
+  }
   if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
       !TripleVendorWasSpecified()) {
     if (other.TripleVendorWasSpecified())
@@ -1190,6 +1180,20 @@
   return false;
 }
 
+void ArchSpec::UpdateCore() {
+  llvm::StringRef arch_name(m_triple.getArchName());
+  const CoreDefinition *core_def = FindCoreDefinition(arch_name);
+  if (core_def) {
+    m_core = core_def->core;
+    // Set the byte order to the default byte order for an architecture.
+    // This can be modified if needed for cases when cores handle both
+    // big and little endian
+    m_byte_order = core_def->default_byte_order;
+  } else {
+    Clear();
+  }
+}
+
 
//===----------------------------------------------------------------------===//
 // Helper methods.
 
Index: include/lldb/Core/ArchSpec.h
===================================================================
--- include/lldb/Core/ArchSpec.h
+++ include/lldb/Core/ArchSpec.h
@@ -625,6 +625,7 @@
 
 protected:
   bool IsEqualTo(const ArchSpec &rhs, bool exact_match) const;
+  void UpdateCore();
 
   llvm::Triple m_triple;
   Core m_core = kCore_invalid;


Index: unittests/Core/ArchSpecTest.cpp
===================================================================
--- unittests/Core/ArchSpecTest.cpp
+++ unittests/Core/ArchSpecTest.cpp
@@ -134,3 +134,22 @@
   AS = ArchSpec();
   EXPECT_FALSE(AS.SetTriple(""));
 }
+
+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());
+}
Index: source/Core/ArchSpec.cpp
===================================================================
--- source/Core/ArchSpec.cpp
+++ source/Core/ArchSpec.cpp
@@ -834,19 +834,7 @@
 
 bool ArchSpec::SetTriple(const llvm::Triple &triple) {
   m_triple = triple;
-
-  llvm::StringRef arch_name(m_triple.getArchName());
-  const CoreDefinition *core_def = FindCoreDefinition(arch_name);
-  if (core_def) {
-    m_core = core_def->core;
-    // Set the byte order to the default byte order for an architecture.
-    // This can be modified if needed for cases when cores handle both
-    // big and little endian
-    m_byte_order = core_def->default_byte_order;
-  } else {
-    Clear();
-  }
-
+  UpdateCore();
   return IsValid();
 }
 
@@ -994,8 +982,10 @@
     GetTriple().setVendor(other.GetTriple().getVendor());
   if (TripleOSIsUnspecifiedUnknown() && !other.TripleOSIsUnspecifiedUnknown())
     GetTriple().setOS(other.GetTriple().getOS());
-  if (GetTriple().getArch() == llvm::Triple::UnknownArch)
+  if (GetTriple().getArch() == llvm::Triple::UnknownArch) {
     GetTriple().setArch(other.GetTriple().getArch());
+    UpdateCore();
+  }
   if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
       !TripleVendorWasSpecified()) {
     if (other.TripleVendorWasSpecified())
@@ -1190,6 +1180,20 @@
   return false;
 }
 
+void ArchSpec::UpdateCore() {
+  llvm::StringRef arch_name(m_triple.getArchName());
+  const CoreDefinition *core_def = FindCoreDefinition(arch_name);
+  if (core_def) {
+    m_core = core_def->core;
+    // Set the byte order to the default byte order for an architecture.
+    // This can be modified if needed for cases when cores handle both
+    // big and little endian
+    m_byte_order = core_def->default_byte_order;
+  } else {
+    Clear();
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // Helper methods.
 
Index: include/lldb/Core/ArchSpec.h
===================================================================
--- include/lldb/Core/ArchSpec.h
+++ include/lldb/Core/ArchSpec.h
@@ -625,6 +625,7 @@
 
 protected:
   bool IsEqualTo(const ArchSpec &rhs, bool exact_match) const;
+  void UpdateCore();
 
   llvm::Triple m_triple;
   Core m_core = kCore_invalid;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to