[Lldb-commits] [PATCH] D83731: Add Debug Info Size to Symbol Status

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:40-41
         self.assertEqual('Symbols not found.', program_module['symbolStatus'])
-        symbol_path = self.getBuildArtifact("a.out")
-        self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" "%s"' % (program, symbol_path)))
+        symbols_path = self.getBuildArtifact("a.out")
+        self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" "%s"' % (program, symbols_path)))
 
----------------
revert renaming and there will be no diffs here


================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:47-48
             return 'Symbols loaded.' == program_module['symbolStatus']
-        self.waitUntil(checkSymbolsLoaded)
 
+        self.waitUntil(checkSymbolsLoaded)
         active_modules = self.vscode.get_active_modules()
----------------
revert and there will be no diffs here


================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:53
         self.assertEqual(program, program_module['path'])
         self.assertEqual('Symbols loaded.', program_module['symbolStatus'])
         self.assertIn('symbolFilePath', program_module)
----------------
We need a comment here if this is for Mac only stating something like:

```
# On darwin, if we add the unstripped executable as the symbol file, it will end up using the executable and the .o files as the debug info. We won't see any debug information size for this case since all of the debug info sections are in the .o files.
```


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:333
+  llvm::StringRef section_name(section.GetName());
+  if (section_name.startswith(".debug") || section_name.startswith("__debug"))
+    debug_info_size += section.GetFileByteSize();
----------------
Need to also check for ".apple" and "__apple" for the Apple DWARF accelerator tables.


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:336
+  size_t num_sub_sections = section.GetNumSubSections();
+  std::ofstream mylog;
+  for (size_t i = 0; i < num_sub_sections; i++) {
----------------
remove mylog stuff




================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:338-339
+  for (size_t i = 0; i < num_sub_sections; i++) {
+    // lldb::SBSection sub_section = section.GetSubSectionAtIndex(i);
+    // debug_info_size += sub_section.GetFileByteSize();
+    mylog.open("sec_name.txt");
----------------
remove


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:340-342
+    mylog.open("sec_name.txt");
+    mylog << num_sub_sections;
+    mylog << section_name.data();
----------------
remove mylog stuff


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:353-354
+  for (size_t i = 0; i < num_sections; i++) {
+    lldb::SBSection section = module.GetSectionAtIndex(i);
+    debug_info_size += GetDebugInfoSizeInSection(section);
+  }
----------------
Remove local "section":

```
debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731