Add Debug Info Size to Symbol Status

[• aelitashen]

Summary

If a module has debug info, the size of debug symbol will be displayed after the Symbols Loaded Message for each module in the VScode modules view.{F12335461}

[wallace]

Please include a test case

lldb/tools/lldb-vscode/JSONUtils.cpp

398–409

Move this logic to another function, so that this function is simpler

403

debug_info is int, thus, if you do debug_info/10, then the result with be an int rounded down. If you cast it to double, you'll have a double without decimals. The correct way to do is to do double(debug_info) / 10.

419

This is a method, therefore it should start with a verb

[clayborg]

See inlined comments. You will also need to fix the test for this since it will now fail as the "symbolStatus" now contains the size.

lldb/tools/lldb-vscode/JSONUtils.cpp

389–396

Move these lines to a static function above this function:

static uint64_t GetDebugInfoSize(lldb::SBModule module) {
  uint64_t debug_info = 0;
  size_t num_sections = module.GetNumSections();
  for (int i = 0; i < (int)num_sections; i++) {
    lldb::SBSection section = module.GetSectionAtIndex(i);
    debug_info += DebugInfoInSection(section);
  }
  return debug_info;
}

419

Rename this as Walter commented and make this function static. You will need to move it above the function above otherwise the compiler won't see it:

static uint64_t GetDebugInfoSize(lldb::SBSection section) {

420

You need to check if the section is a debug info section by checking if it starts with ".debug_" or "__debug":

uint64_t debug_info_size = 0;
llvm::StringRef section_name(section.GetName());
if ((section_name.startswith(".debug") || section_name.startswith("__debug"))
  debug_info_size += section.GetFileByteSize();

422

Use "size_t" as the type for i and remove (int) cast.

423–424

This will add the section size of all sections that are contained in a section. We don't want this. We want to recursively call this function and have it compare the section name as mentioned above in the inline comments:

debug_info_size += GetDebugInfoSize(section.GetSubSectionAtIndex(i));

lldb/tools/lldb-vscode/JSONUtils.h

446 ↗

(On Diff #277613)

This isn't a JSON related function. Just make this a static function in the file that uses it and nothing in a header file.

[clayborg]

lldb/tools/lldb-vscode/JSONUtils.cpp

398

increase size to 32 just in case we get really big debug info later..

400

Use the PRIu64 macro here to make sure this works on all platforms and the units are wrong here, should just be "B" instead of "KB". Also use snprintf for safety:

snprintf(debug_info_size, sizeof(debug_info_size), " (%"  PRIu64 "B)", debug_info);

PRIu64 is a macro that will expand to a string that will always match a uint64_t. The three strings (" (%", PRIu64, and "B)") will get combined by the compiler into a single format string.

402

no need to multiply the debug_info by 10 here. You will want to make a local double so we don't convert back to an integer:

double kb = (double)debug_info/1024.0;

403

Just use the local double above "kb" and use snprintf:

snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fKB)", kb);

405

Don't multiply by 10 and use a local double:

double mb = (double)debug_info/(1024.0*1024.0);

406

Use local double and use snprintf:

snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fMB)", mb);

408

double gb = (double)debug_info/(1024.0*1024.0*1024.0);

409

Use local and snprintf:

snprintf(debug_info_size, sizeof(debug_info_size), " (%.1fGB)", gb);

426

return debug_info_size;

[aprantl]

lldb/tools/lldb-vscode/JSONUtils.cpp

400

Could we avoid snprintf altogether by switching to http://llvm.org/doxygen/classllvm_1_1raw__string__ostream.html ?

[• aelitashen]

Add Test for Debug Info Size

[clayborg]

lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

36–38

revert renaming and there will be no diffs here

43–55

revert and there will be no diffs here

58–59

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.

lldb/tools/lldb-vscode/JSONUtils.cpp

335

Need to also check for ".apple" and "__apple" for the Apple DWARF accelerator tables.

338

remove mylog stuff

340–341

remove

342–344

remove mylog stuff

355–356

Remove local "section":

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

[• aelitashen]

The tests added are only for macOS, Walter will help me write tests on Linux. This update optimize the code and apply git clang format.

[wallace]

lldb/tools/lldb-vscode/JSONUtils.cpp

335–337

you can merge these two ifs

[clayborg]

So you should revert any clang format changes that are not in modified lines of source, mostly revert a lot of lines in lldb-vscode.cpp.

lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

20

Name should be more descriptive. Maybe "setup_test_common" is a better name

20

So all of these tests can re-use this function if we switch it up a bit. Here is what I was thinking:

def run_test(self, symbol_basename, expect_debug_info_size):
    program_basename = "a.out.stripped"
    program = self.getBuildArtifact(program_basename)
    self.build_and_launch(program)
    functions = ['foo']
    breakpoint_ids = self.set_function_breakpoints(functions)
    self.assertEquals(len(breakpoint_ids), len(functions), 'expect one breakpoint')
    self.continue_to_breakpoints(breakpoint_ids)
    active_modules = self.vscode.get_active_modules()
    program_module = active_modules[program_basename]
    self.assertIn(program_basename, active_modules, '%s module is in active modules' % (program_basename))
    self.assertIn('name', program_module, 'make sure name is in module')
    self.assertEqual(program_basename, program_module['name'])
    self.assertIn('path', program_module, 'make sure path is in module')
    self.assertEqual(program, program_module['path'])
    self.assertTrue('symbolFilePath' not in program_module, 'Make sure a.out.stripped has no debug info')
    self.assertEqual('Symbols not found.', program_module['symbolStatus'])
    symbols_path = self.getBuildArtifact(symbol_basename)
    self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" "%s"' % (program, symbols_path)))

    def checkSymbolsLoaded():
        active_modules = self.vscode.get_active_modules()
        program_module = active_modules[program_basename]
        return 'Symbols loaded.' == program_module['symbolStatus']

    def checkSymbolsLoadedWithSize():
        active_modules = self.vscode.get_active_modules()
        program_module = active_modules[program_basename]
        symbolsStatus = program_module['symbolStatus']
        symbol_regex = re.compile(r"Symbols loaded. \([0-9]+(\.[0-9]*)?[KMG]?B\)")
        return symbol_regex.match(program_module['symbolStatus'])
            
    if expect_debug_info_size:
        self.waitUntil(checkSymbolsLoadedWithSize)
    else:
        self.waitUntil(checkSymbolsLoaded)

Then your tests would be:

@skipIfWindows
@skipIfRemote    
def test_module_event(self):
    # Mac or linux.

    # On mac, if we load a.out as our symbol file, we will use DWARF with .o files and we will
    # have debug symbols, but we won't see any debug info size because all of the DWARF
    # sections are in .o files.

    # On other platforms, we expect a.out to have debug info, so we will expect a size.
    expect_debug_info_size = platform.system() != 'Darwin'
    return run_test("a.out", expect_debug_info_size)

@skipIfWindows
@skipUnlessDarwin
@skipIfRemote    
def test_module_event_dsym(self):
    # Darwin only test with dSYM file.

    # On mac, if we load a.out.dSYM as our symbol file, we will have debug symbols and we
    # will have DWARF sections added to the module, so we will expect a size.
    return run_test("a.out.dSYM", True)

This should cover both mac and non windows correctly.

21

add a space after program:

program = self.getBuildArtifact(program_basename)

32

Remove @skipUnlessDarwin here. This should work on linux.

60

wrap comment to 80 chars

lldb/tools/lldb-vscode/JSONUtils.cpp

337

yes, merge the if statements

[wallace]

@clayborg, the tests will fail on linux because the Makefile is expecting a .dsym file. I think it's okay if she does it just for Darwin and then I update the test to cover also linux, as I have my linux already well set up.

[• aelitashen]

Create help function in tests for code re-use. Take care of both macOS and Linux system.

[• aelitashen]

Merge Codes and Improve Readability

[• aelitashen]

Merge if statements in GetDebugInfoSizeInSection()

[wallace]

The logic looks very good, just some final comments and the code will be high quality

lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

83

With you current Makefile, this test will fail on Linux, as the Makefile will expect a .dsym file to be created. Simply put @ skipUnlessDarwin back here, and add this comment

#TODO: Update the Makefile so that this test runs on Linux

Once this is committed, I'll work on make this test pass on Linux

84–90

The common way to write function comments is with ''', like this

'''
  Mac or linux.

  On mac, if we load a.out as our symbol file, we will use DWARF with .o files and we will
  have debug symbols, but we won't see any debug info size because all of the DWARF
  sections are in .o files.

  On other platforms, we expect a.out to have debug info, so we will expect a size.
'''

That way you don't need to type that many #

98–101

same here about the comment

lldb/tools/lldb-vscode/JSONUtils.cpp

335

apply the format change it suggests, i.e start the second line with ||

355

ConvertDebugInfoSizeToString is a better name

356–370

a more modern way to implement this is

#include <sstream>
#include <iomanip>
 ...
std::ostringstream oss;
oss << "(";
oss << std::fixed << std::setprecision(1);

if (debug_info < 1024) {
  oss << debug_info << "B";
} else if (debug_info < 1024 * 1024) {
  double kb = double(debug_info) / 1024.0;
  oss << kb << "KB";
} else if (debug_info < 1024 * 1024 * 1024) {
  double mb = double(debug_info) / (1024.0 * 1024.0);
  oss << mb << "MB";
} else {
  double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
  oss << gb << "GB";;
}
oss << ")";
return oss.str();

It's actually safer, as you don't need to specify the array size of your debug_info_size_buffer

391–392

symbol_str += ConvertDebugInfoSizeToString(debug_info);

is more concise

[• aelitashen]

Add TODO comment for Linux, Use ostringstream for Debug Info Size message.

[wallace]

lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

82

remove this whitespace

[• aelitashen]

Remove white space for test_module_event

[clayborg]

Just a space before the '(' of the debug info size.

lldb/tools/lldb-vscode/JSONUtils.cpp

357

Need as space before the '(' character as we append this string to the "Symbols loaded." string.

[• aelitashen]

Add a white space in Symbols loaded info.

[clayborg]

Looks good!

[• aelitashen]

Fix the accidentally removed test contents

[clayborg]

Looks like there is an indentation issue in the test. See inline comments.

lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

72–78

Unindent everything after the self.waitUntil()? Otherwise we are not testing the program, symbolFilePath and addressRange on symbols with size.

[• aelitashen]

lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

72–78

When unindenting these codes, the dsym test for darwin fails as the symbol paths don't match. One is using dysm path, one is using a.out.path.

[clayborg]

lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

72–78

Then we need to fix this function so that it does work. One thing to note in the dSYM case: dSYM files are bundles which means it is a directory that contains a file within it. So you might specify "/tmp/a.out.dSYM" as the path, but end up with "/tmp/a.out.dSYM/Context/Resources/DWARF/a.out" as the symbol path. So you could switch the symbol file path to use assertIn:

self.assertIn(symbols_path, program_module['symbolFilePath'])

[• aelitashen]

Fix run_test to cover every system case and attributes

clang/tools/clang-format/git-clang-format

-#!/usr/bin/env python
-#
-#===- git-clang-format - ClangFormat Git Integration ---------*- python -*--===#
-#
-# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-# See https://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-#
-#===------------------------------------------------------------------------===#
-
-r"""
-clang-format git integration
-============================
-
-This file provides a clang-format integration for git. Put it somewhere in your
-path and ensure that it is executable. Then, "git clang-format" will invoke
-clang-format on the changes in current files or a specific commit.
-
-For further details, run:
-git clang-format -h
-
-Requires Python 2.7 or Python 3
-"""
-
-from __future__ import absolute_import, division, print_function
-import argparse
-import collections
-import contextlib
-import errno
-import os
-import re
-import subprocess
-import sys
-
-usage = 'git clang-format [OPTIONS] [<commit>] [<commit>] [--] [<file>...]'
-
-desc = '''
-If zero or one commits are given, run clang-format on all lines that differ
-between the working directory and <commit>, which defaults to HEAD.  Changes are
-only applied to the working directory.
-
-If two commits are given (requires --diff), run clang-format on all lines in the
-second <commit> that differ from the first <commit>.
-
-The following git-config settings set the default of the corresponding option:
-  clangFormat.binary
-  clangFormat.commit
-  clangFormat.extensions
-  clangFormat.style
-'''
-
-# Name of the temporary index file in which save the output of clang-format.
-# This file is created within the .git directory.
-temp_index_basename = 'clang-format-index'
-
-
-Range = collections.namedtuple('Range', 'start, count')
-
-
-def main():
-  config = load_git_config()
-
-  # In order to keep '--' yet allow options after positionals, we need to
-  # check for '--' ourselves.  (Setting nargs='*' throws away the '--', while
-  # nargs=argparse.REMAINDER disallows options after positionals.)
-  argv = sys.argv[1:]
-  try:
-    idx = argv.index('--')
-  except ValueError:
-    dash_dash = []
-  else:
-    dash_dash = argv[idx:]
-    argv = argv[:idx]
-
-  default_extensions = ','.join([
-      # From clang/lib/Frontend/FrontendOptions.cpp, all lower case
-      'c', 'h',  # C
-      'm',  # ObjC
-      'mm',  # ObjC++
-      'cc', 'cp', 'cpp', 'c++', 'cxx', 'hh', 'hpp', 'hxx',  # C++
-      'cu',  # CUDA
-      # Other languages that clang-format supports
-      'proto', 'protodevel',  # Protocol Buffers
-      'java',  # Java
-      'js',  # JavaScript
-      'ts',  # TypeScript
-      'cs',  # C Sharp
-      ])
-
-  p = argparse.ArgumentParser(
-    usage=usage, formatter_class=argparse.RawDescriptionHelpFormatter,
-    description=desc)
-  p.add_argument('--binary',
-                 default=config.get('clangformat.binary', 'clang-format'),
-                 help='path to clang-format'),
-  p.add_argument('--commit',
-                 default=config.get('clangformat.commit', 'HEAD'),
-                 help='default commit to use if none is specified'),
-  p.add_argument('--diff', action='store_true',
-                 help='print a diff instead of applying the changes')
-  p.add_argument('--extensions',
-                 default=config.get('clangformat.extensions',
-                                    default_extensions),
-                 help=('comma-separated list of file extensions to format, '
-                       'excluding the period and case-insensitive')),
-  p.add_argument('-f', '--force', action='store_true',
-                 help='allow changes to unstaged files')
-  p.add_argument('-p', '--patch', action='store_true',
-                 help='select hunks interactively')
-  p.add_argument('-q', '--quiet', action='count', default=0,
-                 help='print less information')
-  p.add_argument('--style',
-                 default=config.get('clangformat.style', None),
-                 help='passed to clang-format'),
-  p.add_argument('-v', '--verbose', action='count', default=0,
-                 help='print extra information')
-  # We gather all the remaining positional arguments into 'args' since we need
-  # to use some heuristics to determine whether or not <commit> was present.
-  # However, to print pretty messages, we make use of metavar and help.
-  p.add_argument('args', nargs='*', metavar='<commit>',
-                 help='revision from which to compute the diff')
-  p.add_argument('ignored', nargs='*', metavar='<file>...',
-                 help='if specified, only consider differences in these files')
-  opts = p.parse_args(argv)
-
-  opts.verbose -= opts.quiet
-  del opts.quiet
-
-  commits, files = interpret_args(opts.args, dash_dash, opts.commit)
-  if len(commits) > 1:
-    if not opts.diff:
-      die('--diff is required when two commits are given')
-  else:
-    if len(commits) > 2:
-      die('at most two commits allowed; %d given' % len(commits))
-  changed_lines = compute_diff_and_extract_lines(commits, files)
-  if opts.verbose >= 1:
-    ignored_files = set(changed_lines)
-  filter_by_extension(changed_lines, opts.extensions.lower().split(','))
-  if opts.verbose >= 1:
-    ignored_files.difference_update(changed_lines)
-    if ignored_files:
-      print('Ignoring changes in the following files (wrong extension):')
-      for filename in ignored_files:
-        print('    %s' % filename)
-    if changed_lines:
-      print('Running clang-format on the following files:')
-      for filename in changed_lines:
-        print('    %s' % filename)
-  if not changed_lines:
-    print('no modified files to format')
-    return
-  # The computed diff outputs absolute paths, so we must cd before accessing
-  # those files.
-  cd_to_toplevel()
-  if len(commits) > 1:
-    old_tree = commits[1]
-    new_tree = run_clang_format_and_save_to_tree(changed_lines,
-                                                 revision=commits[1],
-                                                 binary=opts.binary,
-                                                 style=opts.style)
-  else:
-    old_tree = create_tree_from_workdir(changed_lines)
-    new_tree = run_clang_format_and_save_to_tree(changed_lines,
-                                                 binary=opts.binary,
-                                                 style=opts.style)
-  if opts.verbose >= 1:
-    print('old tree: %s' % old_tree)
-    print('new tree: %s' % new_tree)
-  if old_tree == new_tree:
-    if opts.verbose >= 0:
-      print('clang-format did not modify any files')
-  elif opts.diff:
-    print_diff(old_tree, new_tree)
-  else:
-    changed_files = apply_changes(old_tree, new_tree, force=opts.force,
-                                  patch_mode=opts.patch)
-    if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
-      print('changed files:')
-      for filename in changed_files:
-        print('    %s' % filename)
-
-
-def load_git_config(non_string_options=None):
-  """Return the git configuration as a dictionary.
-
-  All options are assumed to be strings unless in `non_string_options`, in which
-  is a dictionary mapping option name (in lower case) to either "--bool" or
-  "--int"."""
-  if non_string_options is None:
-    non_string_options = {}
-  out = {}
-  for entry in run('git', 'config', '--list', '--null').split('\0'):
-    if entry:
-      if '\n' in entry:
-        name, value = entry.split('\n', 1)
-      else:
-        # A setting with no '=' ('\n' with --null) is implicitly 'true'
-        name = entry
-        value = 'true'
-      if name in non_string_options:
-        value = run('git', 'config', non_string_options[name], name)
-      out[name] = value
-  return out
-
-
-def interpret_args(args, dash_dash, default_commit):
-  """Interpret `args` as "[commits] [--] [files]" and return (commits, files).
-
-  It is assumed that "--" and everything that follows has been removed from
-  args and placed in `dash_dash`.
-
-  If "--" is present (i.e., `dash_dash` is non-empty), the arguments to its
-  left (if present) are taken as commits.  Otherwise, the arguments are checked
-  from left to right if they are commits or files.  If commits are not given,
-  a list with `default_commit` is used."""
-  if dash_dash:
-    if len(args) == 0:
-      commits = [default_commit]
-    else:
-      commits = args
-    for commit in commits:
-      object_type = get_object_type(commit)
-      if object_type not in ('commit', 'tag'):
-        if object_type is None:
-          die("'%s' is not a commit" % commit)
-        else:
-          die("'%s' is a %s, but a commit was expected" % (commit, object_type))
-    files = dash_dash[1:]
-  elif args:
-    commits = []
-    while args:
-      if not disambiguate_revision(args[0]):
-        break
-      commits.append(args.pop(0))
-    if not commits:
-      commits = [default_commit]
-    files = args
-  else:
-    commits = [default_commit]
-    files = []
-  return commits, files
-
-
-def disambiguate_revision(value):
-  """Returns True if `value` is a revision, False if it is a file, or dies."""
-  # If `value` is ambiguous (neither a commit nor a file), the following
-  # command will die with an appropriate error message.
-  run('git', 'rev-parse', value, verbose=False)
-  object_type = get_object_type(value)
-  if object_type is None:
-    return False
-  if object_type in ('commit', 'tag'):
-    return True
-  die('`%s` is a %s, but a commit or filename was expected' %
-      (value, object_type))
-
-
-def get_object_type(value):
-  """Returns a string description of an object's type, or None if it is not
-  a valid git object."""
-  cmd = ['git', 'cat-file', '-t', value]
-  p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-  stdout, stderr = p.communicate()
-  if p.returncode != 0:
-    return None
-  return convert_string(stdout.strip())
-
-
-def compute_diff_and_extract_lines(commits, files):
-  """Calls compute_diff() followed by extract_lines()."""
-  diff_process = compute_diff(commits, files)
-  changed_lines = extract_lines(diff_process.stdout)
-  diff_process.stdout.close()
-  diff_process.wait()
-  if diff_process.returncode != 0:
-    # Assume error was already printed to stderr.
-    sys.exit(2)
-  return changed_lines
-
-
-def compute_diff(commits, files):
-  """Return a subprocess object producing the diff from `commits`.
-
-  The return value's `stdin` file object will produce a patch with the
-  differences between the working directory and the first commit if a single
-  one was specified, or the difference between both specified commits, filtered
-  on `files` (if non-empty).  Zero context lines are used in the patch."""
-  git_tool = 'diff-index'
-  if len(commits) > 1:
-    git_tool = 'diff-tree'
-  cmd = ['git', git_tool, '-p', '-U0'] + commits + ['--']
-  cmd.extend(files)
-  p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
-  p.stdin.close()
-  return p
-
-
-def extract_lines(patch_file):
-  """Extract the changed lines in `patch_file`.
-
-  The return value is a dictionary mapping filename to a list of (start_line,
-  line_count) pairs.
-
-  The input must have been produced with ``-U0``, meaning unidiff format with
-  zero lines of context.  The return value is a dict mapping filename to a
-  list of line `Range`s."""
-  matches = {}
-  for line in patch_file:
-    line = convert_string(line)
-    match = re.search(r'^\+\+\+\ [^/]+/(.*)', line)
-    if match:
-      filename = match.group(1).rstrip('\r\n')
-    match = re.search(r'^@@ -[0-9,]+ \+(\d+)(,(\d+))?', line)
-    if match:
-      start_line = int(match.group(1))
-      line_count = 1
-      if match.group(3):
-        line_count = int(match.group(3))
-      if line_count > 0:
-        matches.setdefault(filename, []).append(Range(start_line, line_count))
-  return matches
-
-
-def filter_by_extension(dictionary, allowed_extensions):
-  """Delete every key in `dictionary` that doesn't have an allowed extension.
-
-  `allowed_extensions` must be a collection of lowercase file extensions,
-  excluding the period."""
-  allowed_extensions = frozenset(allowed_extensions)
-  for filename in list(dictionary.keys()):
-    base_ext = filename.rsplit('.', 1)
-    if len(base_ext) == 1 and '' in allowed_extensions:
-        continue
-    if len(base_ext) == 1 or base_ext[1].lower() not in allowed_extensions:
-      del dictionary[filename]
-
-
-def cd_to_toplevel():
-  """Change to the top level of the git repository."""
-  toplevel = run('git', 'rev-parse', '--show-toplevel')
-  os.chdir(toplevel)
-
-
-def create_tree_from_workdir(filenames):
-  """Create a new git tree with the given files from the working directory.
-
-  Returns the object ID (SHA-1) of the created tree."""
-  return create_tree(filenames, '--stdin')
-
-
-def run_clang_format_and_save_to_tree(changed_lines, revision=None,
-                                      binary='clang-format', style=None):
-  """Run clang-format on each file and save the result to a git tree.
-
-  Returns the object ID (SHA-1) of the created tree."""
-  def iteritems(container):
-      try:
-          return container.iteritems() # Python 2
-      except AttributeError:
-          return container.items() # Python 3
-  def index_info_generator():
-    for filename, line_ranges in iteritems(changed_lines):
-      if revision:
-        git_metadata_cmd = ['git', 'ls-tree',
-                            '%s:%s' % (revision, os.path.dirname(filename)),
-                            os.path.basename(filename)]
-        git_metadata = subprocess.Popen(git_metadata_cmd, stdin=subprocess.PIPE,
-                                        stdout=subprocess.PIPE)
-        stdout = git_metadata.communicate()[0]
-        mode = oct(int(stdout.split()[0], 8))
-      else:
-        mode = oct(os.stat(filename).st_mode)
-      # Adjust python3 octal format so that it matches what git expects
-      if mode.startswith('0o'):
-          mode = '0' + mode[2:]
-      blob_id = clang_format_to_blob(filename, line_ranges,
-                                     revision=revision,
-                                     binary=binary,
-                                     style=style)
-      yield '%s %s\t%s' % (mode, blob_id, filename)
-  return create_tree(index_info_generator(), '--index-info')
-
-
-def create_tree(input_lines, mode):
-  """Create a tree object from the given input.
-
-  If mode is '--stdin', it must be a list of filenames.  If mode is
-  '--index-info' is must be a list of values suitable for "git update-index
-  --index-info", such as "<mode> <SP> <sha1> <TAB> <filename>".  Any other mode
-  is invalid."""
-  assert mode in ('--stdin', '--index-info')
-  cmd = ['git', 'update-index', '--add', '-z', mode]
-  with temporary_index_file():
-    p = subprocess.Popen(cmd, stdin=subprocess.PIPE)
-    for line in input_lines:
-      p.stdin.write(to_bytes('%s\0' % line))
-    p.stdin.close()
-    if p.wait() != 0:
-      die('`%s` failed' % ' '.join(cmd))
-    tree_id = run('git', 'write-tree')
-    return tree_id
-
-
-def clang_format_to_blob(filename, line_ranges, revision=None,
-                         binary='clang-format', style=None):
-  """Run clang-format on the given file and save the result to a git blob.
-
-  Runs on the file in `revision` if not None, or on the file in the working
-  directory if `revision` is None.
-
-  Returns the object ID (SHA-1) of the created blob."""
-  clang_format_cmd = [binary]
-  if style:
-    clang_format_cmd.extend(['-style='+style])
-  clang_format_cmd.extend([
-      '-lines=%s:%s' % (start_line, start_line+line_count-1)
-      for start_line, line_count in line_ranges])
-  if revision:
-    clang_format_cmd.extend(['-assume-filename='+filename])
-    git_show_cmd = ['git', 'cat-file', 'blob', '%s:%s' % (revision, filename)]
-    git_show = subprocess.Popen(git_show_cmd, stdin=subprocess.PIPE,
-                                stdout=subprocess.PIPE)
-    git_show.stdin.close()
-    clang_format_stdin = git_show.stdout
-  else:
-    clang_format_cmd.extend([filename])
-    git_show = None
-    clang_format_stdin = subprocess.PIPE
-  try:
-    clang_format = subprocess.Popen(clang_format_cmd, stdin=clang_format_stdin,
-                                    stdout=subprocess.PIPE)
-    if clang_format_stdin == subprocess.PIPE:
-      clang_format_stdin = clang_format.stdin
-  except OSError as e:
-    if e.errno == errno.ENOENT:
-      die('cannot find executable "%s"' % binary)
-    else:
-      raise
-  clang_format_stdin.close()
-  hash_object_cmd = ['git', 'hash-object', '-w', '--path='+filename, '--stdin']
-  hash_object = subprocess.Popen(hash_object_cmd, stdin=clang_format.stdout,
-                                 stdout=subprocess.PIPE)
-  clang_format.stdout.close()
-  stdout = hash_object.communicate()[0]
-  if hash_object.returncode != 0:
-    die('`%s` failed' % ' '.join(hash_object_cmd))
-  if clang_format.wait() != 0:
-    die('`%s` failed' % ' '.join(clang_format_cmd))
-  if git_show and git_show.wait() != 0:
-    die('`%s` failed' % ' '.join(git_show_cmd))
-  return convert_string(stdout).rstrip('\r\n')
-
-
[e-mail]
-def temporary_index_file(tree=None):
-  """Context manager for setting GIT_INDEX_FILE to a temporary file and deleting
-  the file afterward."""
-  index_path = create_temporary_index(tree)
-  old_index_path = os.environ.get('GIT_INDEX_FILE')
-  os.environ['GIT_INDEX_FILE'] = index_path
-  try:
-    yield
-  finally:
-    if old_index_path is None:
-      del os.environ['GIT_INDEX_FILE']
-    else:
-      os.environ['GIT_INDEX_FILE'] = old_index_path
-    os.remove(index_path)
-
-
-def create_temporary_index(tree=None):
-  """Create a temporary index file and return the created file's path.
-
-  If `tree` is not None, use that as the tree to read in.  Otherwise, an
-  empty index is created."""
-  gitdir = run('git', 'rev-parse', '--git-dir')
-  path = os.path.join(gitdir, temp_index_basename)
-  if tree is None:
-    tree = '--empty'
-  run('git', 'read-tree', '--index-output='+path, tree)
-  return path
-
-
-def print_diff(old_tree, new_tree):
-  """Print the diff between the two trees to stdout."""
-  # We use the porcelain 'diff' and not plumbing 'diff-tree' because the output
-  # is expected to be viewed by the user, and only the former does nice things
-  # like color and pagination.
-  #
-  # We also only print modified files since `new_tree` only contains the files
-  # that were modified, so unmodified files would show as deleted without the
-  # filter.
-  subprocess.check_call(['git', 'diff', '--diff-filter=M', old_tree, new_tree,
-                         '--'])
-
-
-def apply_changes(old_tree, new_tree, force=False, patch_mode=False):
-  """Apply the changes in `new_tree` to the working directory.
-
-  Bails if there are local changes in those files and not `force`.  If
-  `patch_mode`, runs `git checkout --patch` to select hunks interactively."""
-  changed_files = run('git', 'diff-tree', '--diff-filter=M', '-r', '-z',
-                      '--name-only', old_tree,
-                      new_tree).rstrip('\0').split('\0')
-  if not force:
-    unstaged_files = run('git', 'diff-files', '--name-status', *changed_files)
-    if unstaged_files:
-      print('The following files would be modified but '
-                'have unstaged changes:', file=sys.stderr)
-      print(unstaged_files, file=sys.stderr)
-      print('Please commit, stage, or stash them first.', file=sys.stderr)
-      sys.exit(2)
-  if patch_mode:
-    # In patch mode, we could just as well create an index from the new tree
-    # and checkout from that, but then the user will be presented with a
-    # message saying "Discard ... from worktree".  Instead, we use the old
-    # tree as the index and checkout from new_tree, which gives the slightly
-    # better message, "Apply ... to index and worktree".  This is not quite
-    # right, since it won't be applied to the user's index, but oh well.
-    with temporary_index_file(old_tree):
-      subprocess.check_call(['git', 'checkout', '--patch', new_tree])
-    index_tree = old_tree
-  else:
-    with temporary_index_file(new_tree):
-      run('git', 'checkout-index', '-a', '-f')
-  return changed_files
-
-
-def run(*args, **kwargs):
-  stdin = kwargs.pop('stdin', '')
-  verbose = kwargs.pop('verbose', True)
-  strip = kwargs.pop('strip', True)
-  for name in kwargs:
-    raise TypeError("run() got an unexpected keyword argument '%s'" % name)
-  p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                       stdin=subprocess.PIPE)
-  stdout, stderr = p.communicate(input=stdin)
-
-  stdout = convert_string(stdout)
-  stderr = convert_string(stderr)
-
-  if p.returncode == 0:
-    if stderr:
-      if verbose:
-        print('`%s` printed to stderr:' % ' '.join(args), file=sys.stderr)
-      print(stderr.rstrip(), file=sys.stderr)
-    if strip:
-      stdout = stdout.rstrip('\r\n')
-    return stdout
-  if verbose:
-    print('`%s` returned %s' % (' '.join(args), p.returncode), file=sys.stderr)
-  if stderr:
-    print(stderr.rstrip(), file=sys.stderr)
-  sys.exit(2)
-
-
-def die(message):
-  print('error:', message, file=sys.stderr)
-  sys.exit(2)
-
-
-def to_bytes(str_input):
-    # Encode to UTF-8 to get binary data.
-    if isinstance(str_input, bytes):
-        return str_input
-    return str_input.encode('utf-8')
-
-
-def to_string(bytes_input):
-    if isinstance(bytes_input, str):
-        return bytes_input
-    return bytes_input.encode('utf-8')
-
-
-def convert_string(bytes_input):
-    try:
-        return to_string(bytes_input.decode('utf-8'))
-    except AttributeError: # 'str' object has no attribute 'decode'.
-        return str(bytes_input)
-    except UnicodeError:
-        return str(bytes_input)
-
-if __name__ == '__main__':
-  main()

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py

def set_function_breakpoints(self, functions, condition=None,
         if response is None:
             return []
         breakpoints = response['body']['breakpoints']
         breakpoint_ids = []
         for breakpoint in breakpoints:
             breakpoint_ids.append('%i' % (breakpoint['id']))
         return breakpoint_ids
 
-    def waitUntil(self, condition):
-        while True:
-            if condition():
-                break
+    def waitUntil(self, condition_callback):
+        for _ in range(20):
+            if condition_callback():
+                return True
             time.sleep(0.5)
+        return False
 
     def verify_breakpoint_hit(self, breakpoint_ids):
         '''Wait for the process we are debugging to stop, and verify we hit
            any breakpoint location in the "breakpoint_ids" array.
            "breakpoint_ids" should be a list of breakpoint ID strings
            (["1", "2"]). The return value from self.set_source_breakpoints()
            or self.set_function_breakpoints() can be passed to this function'''
         stopped_events = self.vscode.wait_for_stopped()

lldb/test/API/tools/lldb-vscode/module/Makefile

 DYLIB_NAME := foo
 DYLIB_CXX_SOURCES := foo.cpp
 CXX_SOURCES := main.cpp
 
-all: a.out.stripped
+LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"
+USE_LIBDL :=1
 
 include Makefile.rules
 
-a.out.stripped: a.out.dSYM
+all: a.out.stripped
+
+a.out.stripped:
   strip -o a.out.stripped a.out
+
 ifneq "$(CODESIGN)" ""
   $(CODESIGN) -fs - a.out.stripped
-endif
+endif
 No newline at end of file

lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py

 """
 Test lldb-vscode setBreakpoints request
 """
 
 from __future__ import print_function
 
 import unittest2
 import vscode
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
+import re
 
 class TestVSCode_module(lldbvscode_testcase.VSCodeTestCaseBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-
-    @skipIfWindows
-    @skipUnlessDarwin
-    @skipIfRemote
-    def test_modules_event(self):
+    def run_test(self, symbol_basename, expect_debug_info_size):
         program_basename = "a.out.stripped"
-        program= self.getBuildArtifact(program_basename)
+        program = self.getBuildArtifact(program_basename)
         self.build_and_launch(program)
         functions = ['foo']
         breakpoint_ids = self.set_function_breakpoints(functions)
-        self.assertEquals(len(breakpoint_ids), len(functions),
-                        'expect one breakpoint')
+        self.assertEquals(len(breakpoint_ids), len(functions), 'expect one breakpoint')
         self.continue_to_breakpoints(breakpoint_ids)
         active_modules = self.vscode.get_active_modules()
-        self.assertIn(program_basename, active_modules, '%s module is in active modules' % (program_basename))
         program_module = active_modules[program_basename]
+        self.assertIn(program_basename, active_modules, '%s module is in active modules' % (program_basename))
         self.assertIn('name', program_module, 'make sure name is in module')
         self.assertEqual(program_basename, program_module['name'])
         self.assertIn('path', program_module, 'make sure path is in module')
         self.assertEqual(program, program_module['path'])
         self.assertTrue('symbolFilePath' not in program_module, 'Make sure a.out.stripped has no debug info')
         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(symbol_basename)
+        self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" "%s"' % (program, symbols_path)))
 
         def checkSymbolsLoaded():
             active_modules = self.vscode.get_active_modules()
             program_module = active_modules[program_basename]
             return 'Symbols loaded.' == program_module['symbolStatus']
-        self.waitUntil(checkSymbolsLoaded)
 
+        def checkSymbolsLoadedWithSize():
+            active_modules = self.vscode.get_active_modules()
+            program_module = active_modules[program_basename]
+            symbolsStatus = program_module['symbolStatus']
+            symbol_regex = re.compile(r"Symbols loaded. \([0-9]+(\.[0-9]*)?[KMG]?B\)")
+            return symbol_regex.match(program_module['symbolStatus'])
+                
+        if expect_debug_info_size:
+            self.waitUntil(checkSymbolsLoadedWithSize)
+        else:
+            self.waitUntil(checkSymbolsLoaded)
         active_modules = self.vscode.get_active_modules()
         program_module = active_modules[program_basename]
         self.assertEqual(program_basename, program_module['name'])
         self.assertEqual(program, program_module['path'])
-        self.assertEqual('Symbols loaded.', program_module['symbolStatus'])
         self.assertIn('symbolFilePath', program_module)
-        self.assertEqual(symbol_path, program_module['symbolFilePath'])
+        self.assertIn(symbols_path, program_module['symbolFilePath'])
         self.assertIn('addressRange', program_module)
 
     @skipIfWindows
     @skipUnlessDarwin
-    @skipIfRemote
+    @skipIfRemote  
+    #TODO: Update the Makefile so that this test runs on Linux
+    def test_module_event(self):
+        '''
+            Mac or linux.
+
+            On mac, if we load a.out as our symbol file, we will use DWARF with .o files and we will
+            have debug symbols, but we won't see any debug info size because all of the DWARF
+            sections are in .o files.
+
+            On other platforms, we expect a.out to have debug info, so we will expect a size.
+            expect_debug_info_size = platform.system() != 'Darwin'
+            return self.run_test("a.out", expect_debug_info_size)
+        '''
+        expect_debug_info_size = platform.system() != 'Darwin'
+        return self.run_test("a.out", expect_debug_info_size)
+
+    @skipIfWindows
+    @skipUnlessDarwin
+    @skipIfRemote    
+    def test_module_event_dsym(self):
+        '''
+            Darwin only test with dSYM file.
+
+            On mac, if we load a.out.dSYM as our symbol file, we will have debug symbols and we
+            will have DWARF sections added to the module, so we will expect a size.
+            return self.run_test("a.out.dSYM", True)
+        '''
+        return self.run_test("a.out.dSYM", True)
+
+    @skipIfWindows
+    @skipUnlessDarwin
+    @skipIfRemote
     def test_compile_units(self):
-        program= self.getBuildArtifact("a.out")
+        program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
         source = "main.cpp"
         main_source_path = self.getSourcePath(source)
         breakpoint1_line = line_number(source, '// breakpoint 1')
         lines = [breakpoint1_line]
         breakpoint_ids = self.set_source_breakpoints(source, lines)
         self.continue_to_breakpoints(breakpoint_ids)
         moduleId = self.vscode.get_active_modules()['a.out']['id']
         response = self.vscode.request_getCompileUnits(moduleId)
         self.assertTrue(response['body'])
         self.assertTrue(len(response['body']['compileUnits']) == 1,
                         'Only one source file should exist')
         self.assertTrue(response['body']['compileUnits'][0]['compileUnitPath'] == main_source_path,
                         'Real path to main.cpp matches')
 

lldb/tools/lldb-vscode/JSONUtils.cpp

 //===-- JSONUtils.cpp -------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <algorithm>
+#include <iomanip>
+#include <sstream>
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBBreakpointLocation.h"
if (bp_addr.IsValid()) {
     object.try_emplace("source", CreateSource(line_entry));
   }
   // We try to add request_line as a fallback
   if (request_line)
     object.try_emplace("line", *request_line);
   return llvm::json::Value(std::move(object));
 }
 
+static uint64_t GetDebugInfoSizeInSection(lldb::SBSection section) {
+  uint64_t debug_info_size = 0;
+  llvm::StringRef section_name(section.GetName());
+  if (section_name.startswith(".debug") || section_name.startswith("__debug") ||
+      section_name.startswith(".apple") || section_name.startswith("__apple"))
+    debug_info_size += section.GetFileByteSize();
+  size_t num_sub_sections = section.GetNumSubSections();
+  for (size_t i = 0; i < num_sub_sections; i++) {
+    debug_info_size +=
+        GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static uint64_t GetDebugInfoSize(lldb::SBModule module) {
+  uint64_t debug_info_size = 0;
+  size_t num_sections = module.GetNumSections();
+  for (size_t i = 0; i < num_sections; i++) {
+    debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
+  std::ostringstream oss;
+  oss << " (";
+  oss << std::fixed << std::setprecision(1);
+
+  if (debug_info < 1024) {
+    oss << debug_info << "B";
+  } else if (debug_info < 1024 * 1024) {
+    double kb = double(debug_info) / 1024.0;
+    oss << kb << "KB";
+  } else if (debug_info < 1024 * 1024 * 1024) {
+    double mb = double(debug_info) / (1024.0 * 1024.0);
+    oss << mb << "MB";
+  } else {
+    double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
+    oss << gb << "GB";
+    ;
+  }
+  oss << ")";
+  return oss.str();
+}
 llvm::json::Value CreateModule(lldb::SBModule &module) {
   llvm::json::Object object;
   if (!module.IsValid())
     return llvm::json::Value(std::move(object));
   const char *uuid = module.GetUUIDString();
   object.try_emplace("id", uuid ? std::string(uuid) : std::string(""));
   object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
   char module_path_arr[PATH_MAX];
   module.GetFileSpec().GetPath(module_path_arr, sizeof(module_path_arr));
   std::string module_path(module_path_arr);
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
-    object.try_emplace("symbolStatus", "Symbols loaded.");
+    std::string symbol_str = "Symbols loaded.";
+    uint64_t debug_info = GetDebugInfoSize(module);
+    if (debug_info > 0) {
+      symbol_str += ConvertDebugInfoSizeToString(debug_info);
+    }
+    object.try_emplace("symbolStatus", symbol_str);
     char symbol_path_arr[PATH_MAX];
-    module.GetSymbolFileSpec().GetPath(symbol_path_arr, sizeof(symbol_path_arr));
+    module.GetSymbolFileSpec().GetPath(symbol_path_arr,
+                                       sizeof(symbol_path_arr));
     std::string symbol_path(symbol_path_arr);
     object.try_emplace("symbolFilePath", symbol_path);
   } else {
     object.try_emplace("symbolStatus", "Symbols not found.");
   }
   std::string loaded_addr = std::to_string(
       module.GetObjectFileHeaderAddress().GetLoadAddress(g_vsc.target));
   object.try_emplace("addressRange", loaded_addr);
   std::string version_str;
   uint32_t version_nums[3];
-  uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t));
-  for (uint32_t i=0; i<num_versions; ++i) {
+  uint32_t num_versions =
+      module.GetVersion(version_nums, sizeof(version_nums) / sizeof(uint32_t));
+  for (uint32_t i = 0; i < num_versions; ++i) {
     if (!version_str.empty())
       version_str += ".";
     version_str += std::to_string(version_nums[i]);
   }
   if (!version_str.empty())
     object.try_emplace("version", version_str);
   return llvm::json::Value(std::move(object));
 }
 
 void AppendBreakpoint(lldb::SBBreakpoint &bp, llvm::json::Array &breakpoints,
                       llvm::Optional<llvm::StringRef> request_path,
                       llvm::Optional<uint32_t> request_line) {
   breakpoints.emplace_back(CreateBreakpoint(bp, request_path, request_line));
 }
 
 // "Event": {
 //   "allOf": [ { "$ref": "#/definitions/ProtocolMessage" }, {
 //     "type": "object",
 //     "description": "Server-initiated event.",
 //     "properties": {
 //       "type": {
 //         "type": "string",
 //         "enum": [ "event" ]
 //       },
 //       "event": {