A better initializing the OSType in elf object file's arch_spec

[trixirt]

Summary

Setting the OSType in the ArchSpec triple is needed to correctly setup up
the register context plugin. ArchSpec::SetArchitecture, for Mach-O only, sets
the OSType. For ELF it was left to the ObjectFileELF to fill in the missing
OSType.

This change moves the ObjectFileELF logic into ArchSpec.

A new optional 'os' parameter has been added to SetArchitecture.
For ELF, this value is the from the ELF header.e_ident[EI_OSABI].
The default value is 0 or ELFOSABI_NONE.

The real work of determining the OSType was done by the ObjectFileELF
helper function GetOsFromOSABI. This logic has been moved SetArchitecture.

GetOsFromOSABI has been commented as being deprectated.
It is left in to support asserts.

For ELF the vendor value returned from SetArchitecture should be UnknownVendor.
An unneeded resetting in ObjectFileELF has been removed and replaced with
an assert.

This fixes a problem reading a core file on FreeBSD/ARM because the spec triple
was arm-unknown-unknown

[clayborg]

See inlined comments.

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

1400 ↗

(On Diff #24473)

After this line can you make sure that if the OS or vendor is unknown that it is left as an unspecified unknown?

assert(arch_spec.GetTriple().getOS() != llvm::Triple::UnknownOS || arch_spec.TripleOSWasSpecified() == false);
assert(arch_spec.GetTriple().getVendor() != llvm::Triple::UnknownVendor || arch_spec.TripleVendorWasSpecified() == false);

If the above assert asserts when you run this, you will need to do one of:

arch_spec.GetTriple().setOSName(llvm::StringRef());
arch_spec.GetTriple().setVendorName(llvm::StringRef());

This will clear the name, but leave the enum value as UnknownOS or UnknownVendor.

This will make sure that any "unknown" values are said to be "unspecified unknowns" so that the arch will effectively be one of

<arch>-*-*
<arch>-*-<os>

If the triple contains "unknown" as the string value in their triple, plug-ins won't be able to wildcard match the ELF file when needed. See the implementations of ArchSpec::TripleVendorWasSpecified() and ArchSpec::TripleOSWasSpecified() to see how we do this, but a quick sum is if the triple has a enum value of unknown and no string value, it is a "unspecified unknown" and if the enum is unknown and the string value is "unknown" then is is a "specified unknown". Some plug-ins will match the specified unknown values and also allow wildcard matching when the unknown isn't specified. It would be nice to eventually have llvm::Triple support for "none" for both the OS and vendor so we can specifiy "no OS" and "no vendor" for things like baseboard binaries, but right now we use "specified unknown" as the marker for "no OS" and "no vendor".

[trixirt]

I believe that an 'unknown' vendor is normal for some triples.
For example i386-unknown-freebsd10.1

Tom

[clayborg]

Ok, just make sure that the unknown is wanted. I would say that GetOsFromOSABI() should be renamed to UpdateOSAndVendorFromOSABI() and it should take the ArchSpec and set the vendor and OS correctly. Specified unknowns are OK when they are desired, but if this function doesn't recognize the OSABI and it doesn't set anything, they should be left and unspecified unknowns.

[emaste]

See also https://svnweb.freebsd.org/base?view=revision&revision=278909 for a workaround in the FreeBSD tree for ELF core file arch setting

[trixirt]

It seems like GetOsFromOSABI's purpose is to compensate for
ArchSpec::SetArchitecture. I think it would be better to improve
SetArchitecture. I will post a change showing how this could be done
shorty.
Tom

[trixirt]

Move ObjectFileELF::GetOsFromOSABI functionality to ArchSpec::SetArchitecture

[clayborg]

Looks fine.

lldb/trunk/include/lldb/Core/ArchSpec.h

public:
     /// x64-pc-windows-msvc, then merging that triple into this one
     /// will result in the triple i386-pc-windows-msvc.
     ///
     //------------------------------------------------------------------
     void
     MergeFrom(const ArchSpec &other);
     
     //------------------------------------------------------------------
-    /// Change the architecture object type and CPU type.
+    /// Change the architecture object type, CPU type and OS type.
     ///
     /// @param[in] arch_type The object type of this ArchSpec.
     ///
     /// @param[in] cpu The required CPU type.
     ///
-    /// @return True if the object and CPU type were successfully set.
+    /// @param[in] os The optional OS type
+    /// The default value of 0 was choosen to from the ELF spec value
+    /// ELFOSABI_NONE.  ELF is the only one using this parameter.  If another
+    /// format uses this parameter and 0 does not work, use a value over
+    /// 255 because in the ELF header this is value is only a byte.
+    ///
+    /// @return True if the object, and CPU were successfully set.
+    ///
+    /// As a side effect, the vendor value is usually set to unknown.
+    /// The exections are
+    ///   aarch64-apple-ios
+    ///   arm-apple-ios
+    ///   thumb-apple-ios
+    ///   x86-apple-
+    ///   x86_64-apple-
+    ///
+    /// As a side effect, the os value is usually set to unknown
+    /// The exceptions are
+    ///   *-*-aix
+    ///   aarch64-apple-ios
+    ///   arm-apple-ios
+    ///   thumb-apple-ios
+    ///   powerpc-apple-darwin
+    ///   *-*-freebsd
+    ///   *-*-linux
+    ///   *-*-netbsd
+    ///   *-*-openbsd
+    ///   *-*-solaris
     //------------------------------------------------------------------
     bool
     SetArchitecture (ArchitectureType arch_type, 
                      uint32_t cpu,
-                     uint32_t sub);
+                     uint32_t sub,
+                     uint32_t os = 0);
 
     //------------------------------------------------------------------
     /// Returns the byte order for the architecture specification.
     ///
     /// @return The endian enumeration for the current endianness of
     ///     the architecture specification
     //------------------------------------------------------------------
     lldb::ByteOrder

lldb/trunk/source/Core/ArchSpec.cpp

if (GetTriple().getOS() == llvm::Triple::UnknownOS && !TripleOSWasSpecified())
         GetTriple().setOS(other.GetTriple().getOS());
     if (GetTriple().getArch() == llvm::Triple::UnknownArch)
         GetTriple().setArch(other.GetTriple().getArch());
     if (GetTriple().getEnvironment() == llvm::Triple::UnknownEnvironment)
         GetTriple().setEnvironment(other.GetTriple().getEnvironment());
 }
 
 bool
-ArchSpec::SetArchitecture (ArchitectureType arch_type, uint32_t cpu, uint32_t sub)
+ArchSpec::SetArchitecture (ArchitectureType arch_type, uint32_t cpu, uint32_t sub, uint32_t os)
 {
     m_core = kCore_invalid;
     bool update_triple = true;
     const ArchDefinition *arch_def = FindArchDefinition(arch_type);
     if (arch_def)
     {
         const ArchDefinitionEntry *arch_def_entry = FindArchDefinitionEntry (arch_def, cpu, sub);
         if (arch_def_entry)
if (arch_def)
                             // anything in the first
                             break;
 
                         default:
                             m_triple.setOS (llvm::Triple::MacOSX);
                             break;
                     }
                 }
+                else if (arch_type == eArchTypeELF)
+                {
+                    llvm::Triple::OSType ostype;
+                    switch (os)
+                    {
+                        case llvm::ELF::ELFOSABI_AIX:      ostype = llvm::Triple::OSType::AIX; break;
+                        case llvm::ELF::ELFOSABI_FREEBSD:  ostype = llvm::Triple::OSType::FreeBSD; break;
+                        case llvm::ELF::ELFOSABI_GNU:      ostype = llvm::Triple::OSType::Linux; break;
+                        case llvm::ELF::ELFOSABI_NETBSD:   ostype = llvm::Triple::OSType::NetBSD; break;
+                        case llvm::ELF::ELFOSABI_OPENBSD:  ostype = llvm::Triple::OSType::OpenBSD; break;
+                        case llvm::ELF::ELFOSABI_SOLARIS:  ostype = llvm::Triple::OSType::Solaris; break;
+                        default:
+                            ostype = llvm::Triple::OSType::UnknownOS;
+                    }
+                    m_triple.setOS (ostype);
+                    m_triple.setVendor (llvm::Triple::UnknownVendor);
+                }
                 // Fall back onto setting the machine type if the arch by name failed...
                 if (m_triple.getArch () == llvm::Triple::UnknownArch)
                     m_triple.setArch (core_def->machine);
             }
         }
     }
     CoreUpdated(update_triple);
     return IsValid();

lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

switch (osabi_byte)
         _MAKE_OSABI_CASE(ELFOSABI_ARM);
         _MAKE_OSABI_CASE(ELFOSABI_STANDALONE);
         default:
             return "<unknown-osabi>";
     }
 #undef _MAKE_OSABI_CASE
 }
 
+//
+// WARNING : This function is being deprecated
+// It's functionality has moved to ArchSpec::SetArchitecture
+// This function is only being kept to validate the move.
+//
+// TODO : Remove this function
 static bool
 GetOsFromOSABI (unsigned char osabi_byte, llvm::Triple::OSType &ostype)
 {
     switch (osabi_byte)
     {
         case ELFOSABI_AIX:      ostype = llvm::Triple::OSType::AIX; break;
         case ELFOSABI_FREEBSD:  ostype = llvm::Triple::OSType::FreeBSD; break;
         case ELFOSABI_GNU:      ostype = llvm::Triple::OSType::Linux; break;
if (ObjectFileELF::MagicBytesMatch(data_sp, 0, data_sp->GetByteSize()))
         {
             if (data_sp)
             {
                 ModuleSpec spec (file);
 
                 const uint32_t sub_type = subTypeFromElfHeader(header);
                 spec.GetArchitecture().SetArchitecture(eArchTypeELF,
                                                        header.e_machine,
-                                                       sub_type);
+                                                       sub_type,
+                                                       header.e_ident[EI_OSABI]);
 
                 if (spec.GetArchitecture().IsValid())
                 {
                     llvm::Triple::OSType ostype;
-                    // First try to determine the OS type from the OSABI field in the elf header.
+                    llvm::Triple::VendorType vendor;
+                    llvm::Triple::OSType spec_ostype = spec.GetArchitecture ().GetTriple ().getOS ();
 
                     if (log)
                         log->Printf ("ObjectFileELF::%s file '%s' module OSABI: %s", __FUNCTION__, file.GetPath ().c_str (), OSABIAsCString (header.e_ident[EI_OSABI]));
-                    if (GetOsFromOSABI (header.e_ident[EI_OSABI], ostype) && ostype != llvm::Triple::OSType::UnknownOS)
-                    {
-                        spec.GetArchitecture ().GetTriple ().setOS (ostype);
 
-                        // Also clear the vendor so we don't end up with situations like
-                        // x86_64-apple-FreeBSD.
-                        spec.GetArchitecture ().GetTriple ().setVendor (llvm::Triple::VendorType::UnknownVendor);
+                    // SetArchitecture should have set the vendor to unknown
+                    vendor = spec.GetArchitecture ().GetTriple ().getVendor ();
+                    assert(vendor == llvm::Triple::UnknownVendor);
 
+                    //
+                    // Validate it is ok to remove GetOsFromOSABI
+                    GetOsFromOSABI (header.e_ident[EI_OSABI], ostype);
+                    assert(spec_ostype == ostype);
+                    if (spec_ostype != llvm::Triple::OSType::UnknownOS)
+                    {
                         if (log)
                             log->Printf ("ObjectFileELF::%s file '%s' set ELF module OS type from ELF header OSABI.", __FUNCTION__, file.GetPath ().c_str ());
                     }
 
                     // Try to get the UUID from the section list. Usually that's at the end, so
                     // map the file in if we don't have it already.
                     size_t section_header_end = header.e_shoff + header.e_shnum * header.e_shentsize;
                     if (section_header_end > data_sp->GetByteSize())
ObjectFileELF::GetSectionHeaderInfo(SectionHeaderColl &section_headers,
     // Don't reparse the section headers if we already did that.
     if (!section_headers.empty())
         return section_headers.size();
 
     // Only initialize the arch_spec to okay defaults if they're not already set.
     // We'll refine this with note data as we parse the notes.
     if (arch_spec.GetTriple ().getOS () == llvm::Triple::OSType::UnknownOS)
     {
+        llvm::Triple::OSType ostype;
+        llvm::Triple::OSType spec_ostype;
         const uint32_t sub_type = subTypeFromElfHeader(header);
-        arch_spec.SetArchitecture (eArchTypeELF, header.e_machine, sub_type);
+        arch_spec.SetArchitecture (eArchTypeELF, header.e_machine, sub_type, header.e_ident[EI_OSABI]);
+        //
+        // Validate if it is ok to remove GetOsFromOSABI
+        GetOsFromOSABI (header.e_ident[EI_OSABI], ostype);
+        spec_ostype = arch_spec.GetTriple ().getOS ();
+        assert(spec_ostype == ostype);
     }
 
     // If there are no section headers we are done.
     if (header.e_shnum == 0)
         return 0;
 
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_MODULES));