Fix bug in ArchSpec::MergeFrom

[clayborg]

Summary

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.

[xiaobai]

Ah, this was my bad. Thanks for taking care of this.

unittests/Utility/ArchSpecTest.cpp

188–193 ↗

(On Diff #198538)

I think it might be a good idea to assert A's values here to show that nothing changed except the OS after the MergeFrom call.

lldb/trunk/source/Utility/ArchSpec.cpp

return !normalized_triple.getArchName().empty() &&
          normalized_triple.getOSName().empty() &&
          normalized_triple.getVendorName().empty() &&
          normalized_triple.getEnvironmentName().empty();
 }
 
 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());
 
     // MachO unknown64 isn't really invalid as the debugger can still obtain
     // information from the binary, e.g. line tables. As such, we don't update
     // the core here.
     if (other.GetCore() != eCore_uknownMach64)

lldb/trunk/unittests/Utility/ArchSpecTest.cpp

 //===-- ArchSpecTest.cpp ----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "gtest/gtest.h"
 
 #include "lldb/Utility/ArchSpec.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/BinaryFormat/ELF.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 TEST(ArchSpecTest, TestParseMachCPUDashSubtypeTripleSimple) {
 
   // Success conditions.  Valid cpu/subtype combinations using both - and .
   ArchSpec AS;
TEST(ArchSpecTest, MergeFrom) {
     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());
   }
+  {
+    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) {
   class MyArchSpec : public ArchSpec {
   public:
     MyArchSpec() {
       this->SetTriple("unknown-mach-64");
       this->m_core = ArchSpec::eCore_uknownMach64;