labath created this revision.
labath added reviewers: amccarth, markmentovai.
Herald added a project: LLDB.

This code (recently responsible for a unaligned access sanitizer
failure) claims that the string table offset zero should result in an
empty string.

I cannot find any mention of this detail in the Microsoft COFF
documentation, and the llvm COFF parser also does not handle offset zero
specially. This code was introduced in 0076e7159, which also does not go
into specifics, citing "various bugfixes".

Given that this is obviously a hack, and does not cause tests to fail, I
think we should just delete it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83881

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -543,12 +543,6 @@
   if (m_data.ValidOffsetForDataOfSize(offset, size))
     return DataExtractor(m_data, offset, size);
 
-  if (m_file) {
-    // A bit of a hack, but we intend to write to this buffer, so we can't
-    // mmap it.
-    auto buffer_sp = MapFileData(m_file, size, offset);
-    return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
-  }
   ProcessSP process_sp(m_process_wp.lock());
   DataExtractor data;
   if (process_sp) {
@@ -652,12 +646,6 @@
           DataExtractor strtab_data = ReadImageData(
               m_coff_header.symoff + symbol_data_size, strtab_size);
 
-          // First 4 bytes should be zeroed after strtab_size has been read,
-          // because it is used as offset 0 to encode a NULL string.
-          uint32_t *strtab_data_start = const_cast<uint32_t *>(
-              reinterpret_cast<const uint32_t *>(strtab_data.GetDataStart()));
-          ::memset(&strtab_data_start[0], 0, sizeof(uint32_t));
-
           offset = 0;
           std::string symbol_name;
           Symbol *symbols = m_symtab_up->Resize(num_syms);


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -543,12 +543,6 @@
   if (m_data.ValidOffsetForDataOfSize(offset, size))
     return DataExtractor(m_data, offset, size);
 
-  if (m_file) {
-    // A bit of a hack, but we intend to write to this buffer, so we can't
-    // mmap it.
-    auto buffer_sp = MapFileData(m_file, size, offset);
-    return DataExtractor(buffer_sp, GetByteOrder(), GetAddressByteSize());
-  }
   ProcessSP process_sp(m_process_wp.lock());
   DataExtractor data;
   if (process_sp) {
@@ -652,12 +646,6 @@
           DataExtractor strtab_data = ReadImageData(
               m_coff_header.symoff + symbol_data_size, strtab_size);
 
-          // First 4 bytes should be zeroed after strtab_size has been read,
-          // because it is used as offset 0 to encode a NULL string.
-          uint32_t *strtab_data_start = const_cast<uint32_t *>(
-              reinterpret_cast<const uint32_t *>(strtab_data.GetDataStart()));
-          ::memset(&strtab_data_start[0], 0, sizeof(uint32_t));
-
           offset = 0;
           std::string symbol_name;
           Symbol *symbols = m_symtab_up->Resize(num_syms);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to