[Lldb-commits] [PATCH] D69646: [LLDB] [PECOFF] Fix error handling for executables that object::createBinary error out on

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added reviewers: labath, amccarth, aleksandr.urakov.
Herald added subscribers: JDevlieghere, abidh.
Herald added a project: LLDB.

llvm::object::createBinary returns an Expected<>, which requires not only 
checking the object for success, but also requires consuming the Error, if one 
was set.

For another similar existing case, move consuming of the Error object to a 
standalone std::string variable. If the Error only is consumed within a 
LLDB_LOGF() statement, it actually doesn't get consumed unless that log channel 
is enabled.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D69646

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml


Index: lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
===================================================================
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
@@ -0,0 +1,80 @@
+## Test that errors in loading an exe doesn't crash lldb.
+
+# RUN: yaml2obj %s > %t.exe
+# RUN: %lldb %t.exe 2>&1 | FileCheck %s
+
+# CHECK: error: '{{.*}}' doesn't contain any {{.*}} platform architectures
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4096
+  ImageBase:       1073741824
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:       IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+    RelativeVirtualAddress: 12345678
+    Size:            100
+  ImportTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ResourceTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ExceptionTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  CertificateTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  BaseRelocationTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  Debug:
+    RelativeVirtualAddress: 0
+    Size:            0
+  Architecture:
+    RelativeVirtualAddress: 0
+    Size:            0
+  GlobalPtr:
+    RelativeVirtualAddress: 0
+    Size:            0
+  TlsTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  LoadConfigTable:
+    RelativeVirtualAddress: 0
+    Size:            0
+  BoundImport:
+    RelativeVirtualAddress: 0
+    Size:            0
+  IAT:
+    RelativeVirtualAddress: 0
+    Size:            0
+  DelayImportDescriptor:
+    RelativeVirtualAddress: 0
+    Size:            0
+  ClrRuntimeHeader:
+    RelativeVirtualAddress: 0
+    Size:            0
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  4096
+    VirtualSize:     1
+    SectionData:     C3
+symbols:         []
+...
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -167,9 +167,18 @@
   if (!data_sp || !ObjectFilePECOFF::MagicBytesMatch(data_sp))
     return initial_count;
 
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
+
   auto binary = llvm::object::createBinary(file.GetPath());
-  if (!binary)
+
+  if (!binary) {
+    std::string msg = errorToErrorCode(binary.takeError()).message();
+    LLDB_LOGF(log,
+              "ObjectFilePECOFF::GetModuleSpecifications() - failed to create binary "
+              "for file (%s): %s",
+              file ? file.GetPath().c_str() : "<NULL>", msg.c_str());
     return initial_count;
+  }
 
   if (!binary->getBinary()->isCOFF() &&
       !binary->getBinary()->isCOFFImportFile())
@@ -242,11 +251,11 @@
 
   auto binary = llvm::object::createBinary(m_file.GetPath());
   if (!binary) {
+    std::string msg = errorToErrorCode(binary.takeError()).message();
     LLDB_LOGF(log,
               "ObjectFilePECOFF::CreateBinary() - failed to create binary "
               "for file (%s): %s",
-              m_file ? m_file.GetPath().c_str() : "<NULL>",
-              errorToErrorCode(binary.takeError()).message().c_str());
+              m_file ? m_file.GetPath().c_str() : "<NULL>", msg.c_str());
     return false;
   }
 

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