[Lldb-commits] [lldb] r341511 - Re-instate a bit of code that was commented out in r188246 which

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Wed Sep  5 17:55:27 2018
New Revision: 341511

URL: http://llvm.org/viewvc/llvm-project?rev=341511&view=rev
Log:
Re-instate a bit of code that was commented out in r188246 which
reads an ObjectFileMachO's string table in one chunk.  Originally
this was commented out because binaries in the system's shared cache
all share a mega-string table and so reading the entire mega-strtab
for each binary was a performance problem.

In the reinstated code, I add a check that the binary we're reading
from memory is not in the shared cache (there isn't a constant in
<mach-o/loader.h> for this bit yet; we hardcode the value in one
other place in ObjectFileMachO alread).  For binaries that we're
reading out of memory that are NOT in the shared cache, reading 
the string table in one chunk is a big performance improvement.

Also have debugserver send up the flags value for binaries in its
response to the jGetLoadedDynamicLibrariesInfos request.

NFC.

<rdar://problem/33604496> 


Modified:
    lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
    lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm

Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=341511&r1=341510&r2=341511&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Wed Sep  5 17:55:27 2018
@@ -2331,14 +2331,6 @@ size_t ObjectFileMachO::ParseSymtab() {
             if (nlist_data_sp)
               nlist_data.SetData(nlist_data_sp, 0,
                                  nlist_data_sp->GetByteSize());
-            // Load strings individually from memory when loading from memory
-            // since shared cache string tables contain strings for all symbols
-            // from all shared cached libraries DataBufferSP strtab_data_sp
-            // (ReadMemory (process_sp, strtab_addr,
-            // strtab_data_byte_size));
-            // if (strtab_data_sp)
-            //    strtab_data.SetData (strtab_data_sp, 0,
-            //    strtab_data_sp->GetByteSize());
             if (m_dysymtab.nindirectsyms != 0) {
               const addr_t indirect_syms_addr = linkedit_load_addr +
                                                 m_dysymtab.indirectsymoff -
@@ -2350,6 +2342,22 @@ size_t ObjectFileMachO::ParseSymtab() {
                 indirect_symbol_index_data.SetData(
                     indirect_syms_data_sp, 0,
                     indirect_syms_data_sp->GetByteSize());
+              // If this binary is outside the shared cache, 
+              // cache the string table.
+              // Binaries in the shared cache all share a giant string table, and
+              // we can't share the string tables across multiple ObjectFileMachO's,
+              // so we'd end up re-reading this mega-strtab for every binary
+              // in the shared cache - it would be a big perf problem.
+              // For binaries outside the shared cache, it's faster to read the
+              // entire strtab at once instead of piece-by-piece as we process
+              // the nlist records.
+              if ((m_header.flags & 0x80000000u) == 0) {
+                DataBufferSP strtab_data_sp (ReadMemory (process_sp, strtab_addr, 
+                      strtab_data_byte_size));
+                if (strtab_data_sp) {
+                  strtab_data.SetData (strtab_data_sp, 0, strtab_data_sp->GetByteSize());
+                }
+              }
             }
           }
           if (memory_module_load_level >=

Modified: lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm?rev=341511&r1=341510&r2=341511&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm (original)
+++ lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm Wed Sep  5 17:55:27 2018
@@ -803,6 +803,8 @@ JSONGenerator::ObjectSP MachProcess::For
         (uint32_t)image_infos[i].macho_info.mach_header.cpusubtype);
     mach_header_dict_sp->AddIntegerItem(
         "filetype", image_infos[i].macho_info.mach_header.filetype);
+    mach_header_dict_sp->AddIntegerItem ("flags", 
+                         image_infos[i].macho_info.mach_header.flags);
 
     //          DynamicLoaderMacOSX doesn't currently need these fields, so
     //          don't send them.
@@ -810,8 +812,6 @@ JSONGenerator::ObjectSP MachProcess::For
     //            image_infos[i].macho_info.mach_header.ncmds);
     //            mach_header_dict_sp->AddIntegerItem ("sizeofcmds",
     //            image_infos[i].macho_info.mach_header.sizeofcmds);
-    //            mach_header_dict_sp->AddIntegerItem ("flags",
-    //            image_infos[i].macho_info.mach_header.flags);
     image_info_dict_sp->AddItem("mach_header", mach_header_dict_sp);
 
     JSONGenerator::ArraySP segments_sp(new JSONGenerator::Array());