compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

LGTM outside of the PE header check.



================
Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:1035
+  // llvm::object::COFFObjectFile::getSectionSize().
+  if (m_binary->getDOSHeader())
+    return std::min(section->GetByteSize(), section->GetFileSize());
----------------
Can we use `m_binary->getPEHeader() || m_binary->getPE32Header()` here instead? 
 We are technically ensuring that this is a linked (PE) binary rather than an 
object file.  While a DOS header is present, it is not an absolute requirement 
(theoretically, it is practically never going to change).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157059/new/

https://reviews.llvm.org/D157059

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

Reply via email to