[Lldb-commits] [PATCH] D61659: Fix bug in ArchSpec::MergeFrom

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, zturner, jingham.

Previous ArchSpec tests didn't catch this bug since we never tested just the OS 
being out of date. Fixed the bug and covered this with a test that would catch 
this.

This was found when trying to load a core file where the core file was an ELF 
file with just the e_machine for architeture and where the ELF header had no OS 
set in the OSABI field of the e_ident. It wasn't merging the architecture with 
the target architecture correctly.


https://reviews.llvm.org/D61659

Files:
  source/Utility/ArchSpec.cpp
  unittests/Utility/ArchSpecTest.cpp


Index: unittests/Utility/ArchSpecTest.cpp
===================================================================
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -10,6 +10,7 @@
 
 #include "lldb/Utility/ArchSpec.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/BinaryFormat/ELF.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -174,6 +175,31 @@
     EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
               A.GetTriple().getEnvironment());
   }
+  {
+    ArchSpec A, B;
+    A.SetArchitecture(eArchTypeELF, llvm::ELF::EM_ARM,
+                      LLDB_INVALID_CPUTYPE, llvm::ELF::ELFOSABI_NONE);
+    B.SetArchitecture(eArchTypeELF, llvm::ELF::EM_ARM,
+                      LLDB_INVALID_CPUTYPE, llvm::ELF::ELFOSABI_LINUX);
+
+    EXPECT_TRUE(A.IsValid());
+    EXPECT_TRUE(B.IsValid());
+    
+    EXPECT_EQ(llvm::Triple::ArchType::arm, 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::UnknownEnvironment,
+              B.GetTriple().getEnvironment());
+    
+    A.MergeFrom(B);
+    EXPECT_EQ(llvm::Triple::ArchType::arm, 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::UnknownEnvironment,
+              A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
Index: source/Utility/ArchSpec.cpp
===================================================================
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -859,7 +859,7 @@
 void ArchSpec::MergeFrom(const ArchSpec &other) {
   if (!TripleVendorWasSpecified() && other.TripleVendorWasSpecified())
     GetTriple().setVendor(other.GetTriple().getVendor());
-  if (!TripleOSWasSpecified() && other.TripleVendorWasSpecified())
+  if (!TripleOSWasSpecified() && other.TripleOSWasSpecified())
     GetTriple().setOS(other.GetTriple().getOS());
   if (GetTriple().getArch() == llvm::Triple::UnknownArch) {
     GetTriple().setArch(other.GetTriple().getArch());



Index: unittests/Utility/ArchSpecTest.cpp
===================================================================
--- unittests/Utility/ArchSpecTest.cpp
+++ unittests/Utility/ArchSpecTest.cpp
@@ -10,6 +10,7 @@
 
 #include "lldb/Utility/ArchSpec.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/BinaryFormat/ELF.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -174,6 +175,31 @@
     EXPECT_EQ(llvm::Triple::EnvironmentType::Android,
               A.GetTriple().getEnvironment());
   }
+  {
+    ArchSpec A, B;
+    A.SetArchitecture(eArchTypeELF, llvm::ELF::EM_ARM,
+                      LLDB_INVALID_CPUTYPE, llvm::ELF::ELFOSABI_NONE);
+    B.SetArchitecture(eArchTypeELF, llvm::ELF::EM_ARM,
+                      LLDB_INVALID_CPUTYPE, llvm::ELF::ELFOSABI_LINUX);
+
+    EXPECT_TRUE(A.IsValid());
+    EXPECT_TRUE(B.IsValid());
+    
+    EXPECT_EQ(llvm::Triple::ArchType::arm, 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::UnknownEnvironment,
+              B.GetTriple().getEnvironment());
+    
+    A.MergeFrom(B);
+    EXPECT_EQ(llvm::Triple::ArchType::arm, 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::UnknownEnvironment,
+              A.GetTriple().getEnvironment());
+  }
 }
 
 TEST(ArchSpecTest, MergeFromMachOUnknown) {
Index: source/Utility/ArchSpec.cpp
===================================================================
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -859,7 +859,7 @@
 void ArchSpec::MergeFrom(const ArchSpec &other) {
   if (!TripleVendorWasSpecified() && other.TripleVendorWasSpecified())
     GetTriple().setVendor(other.GetTriple().getVendor());
-  if (!TripleOSWasSpecified() && other.TripleVendorWasSpecified())
+  if (!TripleOSWasSpecified() && other.TripleOSWasSpecified())
     GetTriple().setOS(other.GetTriple().getOS());
   if (GetTriple().getArch() == llvm::Triple::UnknownArch) {
     GetTriple().setArch(other.GetTriple().getArch());

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