[lldb] Use the win32 OS when determining the architecture of a PECOFF obj file

[wallace]

Summary

When creating the ArchSpec of a PECOFF file, if the subsystem field in the header indicates a windows subsystem, we use Win32 as the OS.

[wallace]

ping

[zturner]

This doesn't seem right. PowerPC and Thumb on Windows? Greg, does OSX use any of this PECOFF code?

[clayborg]

I know we have uses of PECOFF for EFI, but our EFI guys actually run a converter to convert their PECOFF file over into mach-o and they use those when debugging in LLDB. So I don' think this change will affect anything on our end. But just to be safe, is there anything else that you can look at inside the PECOFF file to make sure the OS should be set to windows? Any hints in the imports/exports? I can ask our EFI guys for one of their PECOFF files if needed.

[wallace]

Well, I just need to set the correct OS when creating the ArchSpec so that it works well in conjunction with https://reviews.llvm.org/D24283. If I get it somehow from Target, would that be good enough?

I was assuming that PECOFF was win-only and I have no idea of what else to look for in the PECOFF file to determine the OS.

[zturner]

coff contains a subsystem field. Take a look at llvm/Support/COFF.h then search, for example, for IMAGE_SUBSYSTEM_EFI_APPLICATION. This is part of an enum called WindowsSubsystem. This field should exist in all PE/COFF files. I don't know if LLDB parses it. If it does you can just it. If not you might have to add some logic to extract this field out yourself. I agree it probably won't affect anything since this code isn't used by much outside of Windows, but I would rather the code be correct.

[wallace]

omg, i feel ashamed for not noticing that when i was reading the documentation, I'll do as you say, thanks!

[clayborg]

Go ahead and changes this for now, we'll see if we run into problems. I believe PECOFF is really only used on Windows right now, so we can worry about this when and if we run into problem.

[clayborg]

Sorry, I missed Zach's comment. If we can set this correctly then lets try.

[wallace]

using the subsystem field

[wallace]

clang-format

[wallace]

I was able to use that field and saw the field IMAGE_SUBSYSTEM_WINDOWS_GUI in the libraries I was loading

[zturner]

I would pass llvm::Triple::UnknownOS instead of the constant 0, but otherwise this looks fine. I'll commit this later today.

[clayborg]

Looks good.

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

973

Use llvm::Triple::UnknownOS instead of 0 here and this will be good to go.

[wallace]

use llvm::Triple::UnknownOS

[wallace]

thanks!

[wallace]

fix typo

[zturner]

ok let me try that patch first.

[wallace]

this was commited by zturner

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

public:
 
   //------------------------------------------------------------------
   // PluginInterface protocol
   //------------------------------------------------------------------
   lldb_private::ConstString GetPluginName() override;
 
   uint32_t GetPluginVersion() override;
 
+  bool IsWindowsSubsystem();
+
 protected:
   bool NeedsEndianSwap() const;
 
   typedef struct dos_header { // DOS .EXE header
     uint16_t e_magic;         // Magic number
     uint16_t e_cblp;          // Bytes on last page of file
     uint16_t e_cp;            // Pages in file
     uint16_t e_crlc;          // Relocations

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

void ObjectFilePECOFF::DumpSectionHeaders(Stream *s) {
   SectionHeaderCollIter pos, end = m_sect_headers.end();
 
   for (pos = m_sect_headers.begin(); pos != end; ++pos, ++idx) {
     s->Printf("[%2u] ", idx);
     ObjectFilePECOFF::DumpSectionHeader(s, *pos);
   }
 }
 
+bool ObjectFilePECOFF::IsWindowsSubsystem() {
+  switch (m_coff_header_opt.subsystem) {
+  case llvm::COFF::IMAGE_SUBSYSTEM_NATIVE:
+  case llvm::COFF::IMAGE_SUBSYSTEM_WINDOWS_GUI:
+  case llvm::COFF::IMAGE_SUBSYSTEM_WINDOWS_CUI:
+  case llvm::COFF::IMAGE_SUBSYSTEM_NATIVE_WINDOWS:
+  case llvm::COFF::IMAGE_SUBSYSTEM_WINDOWS_CE_GUI:
+  case llvm::COFF::IMAGE_SUBSYSTEM_XBOX:
+  case llvm::COFF::IMAGE_SUBSYSTEM_WINDOWS_BOOT_APPLICATION:
+    return true;
+  default:
+    return false;
+  }
+}
+
 bool ObjectFilePECOFF::GetArchitecture(ArchSpec &arch) {
   uint16_t machine = m_coff_header.machine;
   switch (machine) {
   case llvm::COFF::IMAGE_FILE_MACHINE_AMD64:
   case llvm::COFF::IMAGE_FILE_MACHINE_I386:
   case llvm::COFF::IMAGE_FILE_MACHINE_POWERPC:
   case llvm::COFF::IMAGE_FILE_MACHINE_POWERPCFP:
   case llvm::COFF::IMAGE_FILE_MACHINE_ARM:
   case llvm::COFF::IMAGE_FILE_MACHINE_ARMNT:
   case llvm::COFF::IMAGE_FILE_MACHINE_THUMB:
-    arch.SetArchitecture(eArchTypeCOFF, machine, LLDB_INVALID_CPUTYPE);
+    arch.SetArchitecture(eArchTypeCOFF, machine, LLDB_INVALID_CPUTYPE,
+                         IsWindowsSubsystem() ? llvm::Triple::Win32
+                                              : llvm::Triple::UnknownOS);
     return true;
   default:
     break;
   }
   return false;
 }
 
 ObjectFile::Type ObjectFilePECOFF::CalculateType() {