[Lldb-commits] [PATCH] D59579: Use list comprehension instead of map/filter to prepare Python2/3 compat

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59579

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/scripts/analyze-project-deps.py
  lldb/scripts/swig_bot_lib/local.py
  lldb/utils/lui/lldbutil.py
  lldb/utils/vim-lldb/python-vim-lldb/lldb_controller.py

Index: lldb/utils/vim-lldb/python-vim-lldb/lldb_controller.py
===
--- lldb/utils/vim-lldb/python-vim-lldb/lldb_controller.py
+++ lldb/utils/vim-lldb/python-vim-lldb/lldb_controller.py
@@ -102,8 +102,8 @@
 pass
 
 if result.GetSize() > 0:
-results = filter(None, [result.GetStringAtIndex(x)
-for x in range(result.GetSize())])
+results = [_f for _f in [result.GetStringAtIndex(x)
+for x in range(result.GetSize())] if _f]
 return results
 else:
 return []
Index: lldb/utils/lui/lldbutil.py
===
--- lldb/utils/lui/lldbutil.py
+++ lldb/utils/lui/lldbutil.py
@@ -84,7 +84,7 @@
 return None
 
 packed = struct.pack(fmt, val)
-return bytearray(map(ord, packed))
+return bytearray(ord(x) for x in packed)
 
 
 def bytearray_to_int(bytes, bytesize):
@@ -706,7 +706,7 @@
 def GetFuncName(i):
 return thread.GetFrameAtIndex(i).GetFunctionName()
 
-return map(GetFuncName, range(thread.GetNumFrames()))
+return [GetFuncName(i) for i in range(thread.GetNumFrames())]
 
 
 def get_symbol_names(thread):
@@ -716,7 +716,7 @@
 def GetSymbol(i):
 return thread.GetFrameAtIndex(i).GetSymbol().GetName()
 
-return map(GetSymbol, range(thread.GetNumFrames()))
+return [GetSymbol(i) for i in range(thread.GetNumFrames())]
 
 
 def get_pc_addresses(thread):
@@ -726,7 +726,7 @@
 def GetPCAddress(i):
 return thread.GetFrameAtIndex(i).GetPCAddress()
 
-return map(GetPCAddress, range(thread.GetNumFrames()))
+return [GetPCAddress(i) for in in range(thread.GetNumFrames())]
 
 
 def get_filenames(thread):
@@ -737,7 +737,7 @@
 return thread.GetFrameAtIndex(
 i).GetLineEntry().GetFileSpec().GetFilename()
 
-return map(GetFilename, range(thread.GetNumFrames()))
+return [GetFilename(i) for i in range(thread.GetNumFrames())]
 
 
 def get_line_numbers(thread):
@@ -747,7 +747,7 @@
 def GetLineNumber(i):
 return thread.GetFrameAtIndex(i).GetLineEntry().GetLine()
 
-return map(GetLineNumber, range(thread.GetNumFrames()))
+return [GetLineNumber(i) for i in range(thread.GetNumFrames())]
 
 
 def get_module_names(thread):
@@ -758,7 +758,7 @@
 return thread.GetFrameAtIndex(
 i).GetModule().GetFileSpec().GetFilename()
 
-return map(GetModuleName, range(thread.GetNumFrames()))
+return [GetModuleName(i) for i in range(thread.GetNumFrames())]
 
 
 def get_stack_frames(thread):
@@ -768,7 +768,7 @@
 def GetStackFrame(i):
 return thread.GetFrameAtIndex(i)
 
-return map(GetStackFrame, range(thread.GetNumFrames()))
+return [GetStackFrame(i) for i in range(thread.GetNumFrames())]
 
 
 def print_stacktrace(thread, string_buffer=False):
Index: lldb/scripts/swig_bot_lib/local.py
===
--- lldb/scripts/swig_bot_lib/local.py
+++ lldb/scripts/swig_bot_lib/local.py
@@ -54,10 +54,8 @@
 full_path = os.path.normpath(os.path.join(src_root, subfolder))
 candidates = [os.path.normpath(os.path.join(full_path, f))
   for f in os.listdir(full_path)]
-actual = filter(
-lambda f: os.path.isfile(f) and os.path.splitext(f)[1] == ext,
-candidates)
-return (subfolder, map(lambda f: os.path.basename(f), actual))
+actual = [f for f in candidates if os.path.isfile(f) and os.path.splitext(f)[1] == ext]
+return (subfolder, [os.path.basename(f) for f in actual])
 archive_entries = map(filter_func, filters)
 else:
 for (root, dirs, files) in os.walk(src_root):
Index: lldb/scripts/analyze-project-deps.py
===
--- lldb/scripts/analyze-project-deps.py
+++ lldb/scripts/analyze-project-deps.py
@@ -65,7 +65,7 @@
 for (base, dirs, files) in os.walk(inc_dir):
 dir = os.path.basename(base)
 relative = os.path.relpath(base, inc_dir)
-inc_files = filter(lambda x : os.path.splitext(x)[1] in [".h"], files)
+inc_files = [x for x in files if os.path.splitext(x)[1] in [".h"]]
 relative = relative.replace("\\", "/")
 for inc in inc_files:
 inc_path = os.path.join(base, inc)
@@ -74,7 +74,7 @@
 for (base, dirs, files) in os.walk(src_dir)

[Lldb-commits] [PATCH] D59581: Python 2/3 compat: urllib

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: llvm-commits, lldb-commits, jdoerfert.
Herald added projects: LLDB, LLVM.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59581

Files:
  lld/utils/benchmark.py


Index: lld/utils/benchmark.py
===
--- lld/utils/benchmark.py
+++ lld/utils/benchmark.py
@@ -13,8 +13,13 @@
 import json
 import datetime
 import argparse
-import urllib
-import urllib2
+try:
+from urllib.parse import urlencode
+from urllib.request import urlopen, Request
+except ImportError:
+from urllib import urlencode
+from urllib2 import urlopen, Request
+
 
 parser = argparse.ArgumentParser()
 parser.add_argument('benchmark_directory')
@@ -126,8 +131,8 @@
 return json.dumps(ret, sort_keys=True, indent=4)
 
 def submitToServer(data):
-data2 = urllib.urlencode({ 'input_data' : data }).encode('ascii')
-urllib2.urlopen(urllib2.Request(args.url, data2))
+data2 = urlencode({ 'input_data' : data }).encode('ascii')
+urlopen(Request(args.url, data2))
 
 os.chdir(args.benchmark_directory)
 data = buildLntJson(getBenchmarks())


Index: lld/utils/benchmark.py
===
--- lld/utils/benchmark.py
+++ lld/utils/benchmark.py
@@ -13,8 +13,13 @@
 import json
 import datetime
 import argparse
-import urllib
-import urllib2
+try:
+from urllib.parse import urlencode
+from urllib.request import urlopen, Request
+except ImportError:
+from urllib import urlencode
+from urllib2 import urlopen, Request
+
 
 parser = argparse.ArgumentParser()
 parser.add_argument('benchmark_directory')
@@ -126,8 +131,8 @@
 return json.dumps(ret, sort_keys=True, indent=4)
 
 def submitToServer(data):
-data2 = urllib.urlencode({ 'input_data' : data }).encode('ascii')
-urllib2.urlopen(urllib2.Request(args.url, data2))
+data2 = urlencode({ 'input_data' : data }).encode('ascii')
+urlopen(Request(args.url, data2))
 
 os.chdir(args.benchmark_directory)
 data = buildLntJson(getBenchmarks())
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59583: python2/3 compat: exceptions

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59583

Files:
  lldb/examples/python/crashlog.py


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -284,7 +284,7 @@
 try:
 plist_root = plistlib.readPlistFromString(s)
 except:
-print(("Got exception: ", sys.exc_value, " handling 
dsymForUUID output: \n", s)) 
+print(("Got exception: ", sys.exc_info()[1], " 
handling dsymForUUID output: \n", s))
 raise
 if plist_root:
 plist = plist_root[uuid_str]


Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -284,7 +284,7 @@
 try:
 plist_root = plistlib.readPlistFromString(s)
 except:
-print(("Got exception: ", sys.exc_value, " handling dsymForUUID output: \n", s)) 
+print(("Got exception: ", sys.exc_info()[1], " handling dsymForUUID output: \n", s))
 raise
 if plist_root:
 plist = plist_root[uuid_str]
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59584: python 2/3 compat: commands vs subprocess

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59584

Files:
  lldb/examples/python/delta.py
  lldb/examples/python/gdbremote.py
  lldb/examples/python/globals.py
  lldb/examples/python/memory.py
  lldb/examples/python/performance.py
  lldb/examples/python/process_events.py
  lldb/examples/python/stacks.py
  lldb/examples/python/types.py
  lldb/scripts/verify_api.py

Index: lldb/scripts/verify_api.py
===
--- lldb/scripts/verify_api.py
+++ lldb/scripts/verify_api.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 
-import commands
+import subprocess
 import optparse
 import os
 import os.path
@@ -11,7 +11,7 @@
 def extract_exe_symbol_names(arch, exe_path, match_str):
 command = 'dsymutil --arch %s -s "%s" | grep "%s" | colrm 1 69' % (
 arch, exe_path, match_str)
-(command_exit_status, command_output) = commands.getstatusoutput(command)
+(command_exit_status, command_output) = subprocess.getstatusoutput(command)
 if command_exit_status == 0:
 if command_output:
 return command_output[0:-1].split("'\n")
Index: lldb/examples/python/types.py
===
--- lldb/examples/python/types.py
+++ lldb/examples/python/types.py
@@ -9,7 +9,6 @@
 #   (lldb) command script import /path/to/cmdtemplate.py
 #--
 
-import commands
 from __future__ import print_function
 
 import platform
@@ -18,6 +17,11 @@
 import signal
 import sys
 
+if sys.version_info.major == 2:
+import commands as subprocess
+else:
+import subprocess
+
 try:
 # Just try for LLDB in case PYTHONPATH is already correctly setup
 import lldb
@@ -27,7 +31,7 @@
 platform_system = platform.system()
 if platform_system == 'Darwin':
 # On Darwin, try the currently selected Xcode directory
-xcode_dir = commands.getoutput("xcode-select --print-path")
+xcode_dir = subprocess.getoutput("xcode-select --print-path")
 if xcode_dir:
 lldb_python_dirs.append(
 os.path.realpath(
@@ -54,7 +58,6 @@
 print("error: couldn't locate the 'lldb' module, please set PYTHONPATH correctly")
 sys.exit(1)
 
-import commands
 import optparse
 import shlex
 import time
Index: lldb/examples/python/stacks.py
===
--- lldb/examples/python/stacks.py
+++ lldb/examples/python/stacks.py
@@ -1,7 +1,6 @@
 #!/usr/bin/python
 from __future__ import print_function
 import lldb
-import commands
 import optparse
 import shlex
 
Index: lldb/examples/python/process_events.py
===
--- lldb/examples/python/process_events.py
+++ lldb/examples/python/process_events.py
@@ -8,7 +8,6 @@
 #   export PYTHONPATH=/Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/Resources/Python
 #--
 
-import commands
 from __future__ import print_function
 
 import optparse
@@ -16,6 +15,11 @@
 import platform
 import sys
 
+if sys.version_info.major == 2:
+import commands as subprocess
+else:
+import subprocess
+
 #--
 # Code that auto imports LLDB
 #--
@@ -28,7 +32,7 @@
 platform_system = platform.system()
 if platform_system == 'Darwin':
 # On Darwin, try the currently selected Xcode directory
-xcode_dir = commands.getoutput("xcode-select --print-path")
+xcode_dir = subprocess.getoutput("xcode-select --print-path")
 if xcode_dir:
 lldb_python_dirs.append(
 os.path.realpath(
Index: lldb/examples/python/performance.py
===
--- lldb/examples/python/performance.py
+++ lldb/examples/python/performance.py
@@ -8,7 +8,6 @@
 #   export PYTHONPATH=/Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/Resources/Python
 #--
 
-import commands
 from __future__ import print_function
 
 import optparse
@@ -20,6 +19,11 @@
 import time
 import types
 
+if sys.version_info.major == 2:
+import commands as subprocess
+else:
+import subprocess
+
 #--
 # Code that auto imports LLDB
 #--
@@ -32,7 +36,7 @@
 platform_system = platform.system()
 if platform_system == 'Darwin':
 # On Darwin, try the currently selected Xcode directory
-xcode_d

[Lldb-commits] [PATCH] D59582: Python 2/3 compat: StringIO

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59582

Files:
  lldb/examples/customization/bin-utils/binutils.py
  lldb/examples/python/mach_o.py
  lldb/utils/git-svn/convert.py
  lldb/utils/lui/lldbutil.py
  lldb/utils/misc/grep-svn-log.py
  lldb/utils/sync-source/syncsource.py
  lldb/utils/test/disasm.py
  lldb/utils/test/run-until-faulted.py

Index: lldb/utils/test/run-until-faulted.py
===
--- lldb/utils/test/run-until-faulted.py
+++ lldb/utils/test/run-until-faulted.py
@@ -32,7 +32,6 @@
 
 
 def do_lldb_launch_loop(lldb_command, exe, exe_options):
-from cStringIO import StringIO
 import pexpect
 import time
 
Index: lldb/utils/test/disasm.py
===
--- lldb/utils/test/disasm.py
+++ lldb/utils/test/disasm.py
@@ -39,7 +39,7 @@
 func,
 mc,
 mc_options):
-from cStringIO import StringIO
+from io import StringIO
 import pexpect
 
 gdb_prompt = "\r\n\(gdb\) "
Index: lldb/utils/sync-source/syncsource.py
===
--- lldb/utils/sync-source/syncsource.py
+++ lldb/utils/sync-source/syncsource.py
@@ -11,7 +11,7 @@
 """
 
 import argparse
-import cStringIO
+import io
 import importlib
 import json
 import os.path
@@ -89,11 +89,11 @@
 # preserving the line count.
 regex = re.compile(r"#.*$")
 
-comment_stripped_file = cStringIO.StringIO()
+comment_stripped_file = io.StringIO()
 with open(filename, "r") as json_file:
 for line in json_file:
 comment_stripped_file.write(regex.sub("", line))
-return json.load(cStringIO.StringIO(comment_stripped_file.getvalue()))
+return json.load(io.StringIO(comment_stripped_file.getvalue()))
 
 
 def find_appropriate_rcfile(options):
Index: lldb/utils/misc/grep-svn-log.py
===
--- lldb/utils/misc/grep-svn-log.py
+++ lldb/utils/misc/grep-svn-log.py
@@ -13,7 +13,7 @@
 import fileinput
 import re
 import sys
-import StringIO
+import io
 
 # Separator string for "svn log -v" output.
 separator = '-' * 72
@@ -23,7 +23,7 @@
 svn log -v | grep-svn-log.py '^   D.+why_are_you_missing.h'"""
 
 
-class Log(StringIO.StringIO):
+class Log(io.StringIO):
 """Simple facade to keep track of the log content."""
 
 def __init__(self):
Index: lldb/utils/lui/lldbutil.py
===
--- lldb/utils/lui/lldbutil.py
+++ lldb/utils/lui/lldbutil.py
@@ -17,7 +17,7 @@
 import lldb
 import os
 import sys
-import StringIO
+import io
 
 # ===
 # Utilities for locating/checking executable programs
@@ -52,7 +52,7 @@
 
 It returns the disassembly content in a string object.
 """
-buf = StringIO.StringIO()
+buf = io.StringIO()
 insts = function_or_symbol.GetInstructions(target)
 for i in insts:
 print(i, file=buf)
@@ -776,7 +776,7 @@
 def print_stacktrace(thread, string_buffer=False):
 """Prints a simple stack trace of this thread."""
 
-output = StringIO.StringIO() if string_buffer else sys.stdout
+output = io.StringIO() if string_buffer else sys.stdout
 target = thread.GetProcess().GetTarget()
 
 depth = thread.GetNumFrames()
@@ -819,7 +819,7 @@
 def print_stacktraces(process, string_buffer=False):
 """Prints the stack traces of all the threads."""
 
-output = StringIO.StringIO() if string_buffer else sys.stdout
+output = io.StringIO() if string_buffer else sys.stdout
 
 print("Stack traces for " + str(process), file=output)
 
@@ -879,7 +879,7 @@
 def print_registers(frame, string_buffer=False):
 """Prints all the register sets of the frame."""
 
-output = StringIO.StringIO() if string_buffer else sys.stdout
+output = io.StringIO() if string_buffer else sys.stdout
 
 print("Register sets for " + str(frame), file=output)
 
@@ -962,7 +962,7 @@
 
 def format(self, value, buffer=None, indent=0):
 if not buffer:
-output = StringIO.StringIO()
+output = io.StringIO()
 else:
 output = buffer
 # If there is a summary, it suffices.
@@ -992,7 +992,7 @@
 
 def format(self, value, buffer=None):
 if not buffer:
-output = StringIO.StringIO()
+output = io.StringIO()
 else:
 output = buffer
 
@@ -1019,7 +1019,7 @@
 
 def format(self, value, buffer=None):
 if not buffer:
-output = StringIO.StringIO()
+output = io.StringIO()
 else:
 output = buffer
 
Index: lldb/utils/git-svn/convert.py

[Lldb-commits] [PATCH] D59585: python 2/3 compat: int vs long

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59585

Files:
  lldb/examples/python/jump.py


Index: lldb/examples/python/jump.py
===
--- lldb/examples/python/jump.py
+++ lldb/examples/python/jump.py
@@ -80,7 +80,7 @@
 if (mo is not None):
 matched = True
 # print "Matched "
-address = long(mo.group(1), base=0)
+address = int(mo.group(1), base=0)
 breakpoint = target.BreakpointCreateByAddress(address)
 
 if (not matched):


Index: lldb/examples/python/jump.py
===
--- lldb/examples/python/jump.py
+++ lldb/examples/python/jump.py
@@ -80,7 +80,7 @@
 if (mo is not None):
 matched = True
 # print "Matched "
-address = long(mo.group(1), base=0)
+address = int(mo.group(1), base=0)
 breakpoint = target.BreakpointCreateByAddress(address)
 
 if (not matched):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59587: Use explicit loop instead of map for Python 2/3 compat

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

The output of map is not used, so using a list comprehension or an explicit 
call to list looks awkward.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59587

Files:
  lldb/scripts/Xcode/build-llvm.py


Index: lldb/scripts/Xcode/build-llvm.py
===
--- lldb/scripts/Xcode/build-llvm.py
+++ lldb/scripts/Xcode/build-llvm.py
@@ -239,7 +239,8 @@
 
 
 def all_check_out_if_needed():
-map(check_out_if_needed, XCODE_REPOSITORIES())
+for r in XCODE_REPOSITORIES():
+check_out_if_needed(r)
 
 
 def should_build_llvm():
@@ -263,7 +264,8 @@
 
 
 def setup_source_symlinks():
-map(setup_source_symlink, XCODE_REPOSITORIES())
+for r in XCODE_REPOSITORIES():
+setup_source_symlink(r)
 
 
 def setup_build_symlink():


Index: lldb/scripts/Xcode/build-llvm.py
===
--- lldb/scripts/Xcode/build-llvm.py
+++ lldb/scripts/Xcode/build-llvm.py
@@ -239,7 +239,8 @@
 
 
 def all_check_out_if_needed():
-map(check_out_if_needed, XCODE_REPOSITORIES())
+for r in XCODE_REPOSITORIES():
+check_out_if_needed(r)
 
 
 def should_build_llvm():
@@ -263,7 +264,8 @@
 
 
 def setup_source_symlinks():
-map(setup_source_symlink, XCODE_REPOSITORIES())
+for r in XCODE_REPOSITORIES():
+setup_source_symlink(r)
 
 
 def setup_build_symlink():
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59586: Python 2/3 compat: tkinter

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59586

Files:
  lldb/examples/python/lldbtk.py


Index: lldb/examples/python/lldbtk.py
===
--- lldb/examples/python/lldbtk.py
+++ lldb/examples/python/lldbtk.py
@@ -1,10 +1,15 @@
 #!/usr/bin/python
+from __future__ import print_function
 
 import lldb
 import shlex
 import sys
-from Tkinter import *
-import ttk
+try:
+from tkinter import *
+import tkinter.ttk as ttk
+except ImportError:
+from Tkinter import *
+import ttk
 
 
 class ValueTreeItemDelegate(object):


Index: lldb/examples/python/lldbtk.py
===
--- lldb/examples/python/lldbtk.py
+++ lldb/examples/python/lldbtk.py
@@ -1,10 +1,15 @@
 #!/usr/bin/python
+from __future__ import print_function
 
 import lldb
 import shlex
 import sys
-from Tkinter import *
-import ttk
+try:
+from tkinter import *
+import tkinter.ttk as ttk
+except ImportError:
+from Tkinter import *
+import ttk
 
 
 class ValueTreeItemDelegate(object):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59588: Python 2/3 compat: iteritems vs items

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59588

Files:
  lldb/examples/python/pytracer.py


Index: lldb/examples/python/pytracer.py
===
--- lldb/examples/python/pytracer.py
+++ lldb/examples/python/pytracer.py
@@ -339,7 +339,7 @@
 
 def print_keyword_args(**kwargs):
 # kwargs is a dict of the keyword args passed to the function
-for key, value in kwargs.iteritems():
+for key, value in kwargs.items():
 print("%s = %s" % (key, value))
 
 


Index: lldb/examples/python/pytracer.py
===
--- lldb/examples/python/pytracer.py
+++ lldb/examples/python/pytracer.py
@@ -339,7 +339,7 @@
 
 def print_keyword_args(**kwargs):
 # kwargs is a dict of the keyword args passed to the function
-for key, value in kwargs.iteritems():
+for key, value in kwargs.items():
 print("%s = %s" % (key, value))
 
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59590: Python 2/3 compat: queue vs Queue

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59590

Files:
  lldb/utils/lui/lui.py
  lldb/utils/lui/sandbox.py


Index: lldb/utils/lui/sandbox.py
===
--- lldb/utils/lui/sandbox.py
+++ lldb/utils/lui/sandbox.py
@@ -14,7 +14,10 @@
 import signal
 import sys
 
-import Queue
+try:
+import queue
+except ImportError:
+import Queue as queue
 
 import cui
 
@@ -62,7 +65,7 @@
 
 def main(screen):
 global event_queue
-event_queue = Queue.Queue()
+event_queue = queue.Queue()
 
 sandbox = SandboxUI(screen, event_queue)
 sandbox.eventLoop()
Index: lldb/utils/lui/lui.py
===
--- lldb/utils/lui/lui.py
+++ lldb/utils/lui/lui.py
@@ -18,7 +18,10 @@
 import signal
 import sys
 
-import Queue
+try:
+import queue
+except ImportError:
+import Queue as queue
 
 import debuggerdriver
 import cui
@@ -126,7 +129,7 @@
 signal.signal(signal.SIGINT, sigint_handler)
 
 global event_queue
-event_queue = Queue.Queue()
+event_queue = queue.Queue()
 
 global debugger
 debugger = lldb.SBDebugger.Create()


Index: lldb/utils/lui/sandbox.py
===
--- lldb/utils/lui/sandbox.py
+++ lldb/utils/lui/sandbox.py
@@ -14,7 +14,10 @@
 import signal
 import sys
 
-import Queue
+try:
+import queue
+except ImportError:
+import Queue as queue
 
 import cui
 
@@ -62,7 +65,7 @@
 
 def main(screen):
 global event_queue
-event_queue = Queue.Queue()
+event_queue = queue.Queue()
 
 sandbox = SandboxUI(screen, event_queue)
 sandbox.eventLoop()
Index: lldb/utils/lui/lui.py
===
--- lldb/utils/lui/lui.py
+++ lldb/utils/lui/lui.py
@@ -18,7 +18,10 @@
 import signal
 import sys
 
-import Queue
+try:
+import queue
+except ImportError:
+import Queue as queue
 
 import debuggerdriver
 import cui
@@ -126,7 +129,7 @@
 signal.signal(signal.SIGINT, sigint_handler)
 
 global event_queue
-event_queue = Queue.Queue()
+event_queue = queue.Queue()
 
 global debugger
 debugger = lldb.SBDebugger.Create()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59591: Python 2/3 compat: unichr vs chr

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59591

Files:
  lldb/examples/summaries/cocoa/CFString.py


Index: lldb/examples/summaries/cocoa/CFString.py
===
--- lldb/examples/summaries/cocoa/CFString.py
+++ lldb/examples/summaries/cocoa/CFString.py
@@ -11,6 +11,10 @@
 import lldb.runtime.objc.objc_runtime
 import lldb.formatters.Logger
 
+try:
+unichr
+except NameError:
+unichr = chr
 
 def CFString_SummaryProvider(valobj, dict):
 logger = lldb.formatters.Logger.Logger()


Index: lldb/examples/summaries/cocoa/CFString.py
===
--- lldb/examples/summaries/cocoa/CFString.py
+++ lldb/examples/summaries/cocoa/CFString.py
@@ -11,6 +11,10 @@
 import lldb.runtime.objc.objc_runtime
 import lldb.formatters.Logger
 
+try:
+unichr
+except NameError:
+unichr = chr
 
 def CFString_SummaryProvider(valobj, dict):
 logger = lldb.formatters.Logger.Logger()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59589: Python 2/3 compat: str vs basestring

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added subscribers: lldb-commits, jdoerfert, arphaman.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59589

Files:
  lldb/examples/summaries/cocoa/CFArray.py
  lldb/examples/summaries/cocoa/CFBag.py
  lldb/examples/summaries/cocoa/CFBinaryHeap.py
  lldb/examples/summaries/cocoa/CFDictionary.py
  lldb/examples/summaries/cocoa/NSData.py
  lldb/examples/summaries/cocoa/NSIndexSet.py
  lldb/examples/summaries/cocoa/NSMachPort.py
  lldb/examples/summaries/cocoa/NSSet.py

Index: lldb/examples/summaries/cocoa/NSSet.py
===
--- lldb/examples/summaries/cocoa/NSSet.py
+++ lldb/examples/summaries/cocoa/NSSet.py
@@ -13,6 +13,11 @@
 import CFBag
 import lldb.formatters.Logger
 
+try:
+basestring
+except NameError:
+basestring = str
+
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
Index: lldb/examples/summaries/cocoa/NSMachPort.py
===
--- lldb/examples/summaries/cocoa/NSMachPort.py
+++ lldb/examples/summaries/cocoa/NSMachPort.py
@@ -13,6 +13,11 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
+try:
+basestring
+except NameError:
+basestring =str
+
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
Index: lldb/examples/summaries/cocoa/NSIndexSet.py
===
--- lldb/examples/summaries/cocoa/NSIndexSet.py
+++ lldb/examples/summaries/cocoa/NSIndexSet.py
@@ -13,6 +13,11 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
+try:
+basestring
+except NameError:
+basestring = str
+
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
Index: lldb/examples/summaries/cocoa/NSData.py
===
--- lldb/examples/summaries/cocoa/NSData.py
+++ lldb/examples/summaries/cocoa/NSData.py
@@ -13,6 +13,11 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
+try:
+basestring
+except NameError:
+basestring = str
+
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
Index: lldb/examples/summaries/cocoa/CFDictionary.py
===
--- lldb/examples/summaries/cocoa/CFDictionary.py
+++ lldb/examples/summaries/cocoa/CFDictionary.py
@@ -13,6 +13,11 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
+try:
+basestring
+except NameError
+basestring = str
+
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
Index: lldb/examples/summaries/cocoa/CFBinaryHeap.py
===
--- lldb/examples/summaries/cocoa/CFBinaryHeap.py
+++ lldb/examples/summaries/cocoa/CFBinaryHeap.py
@@ -13,6 +13,11 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
+try:
+basestring
+except NameError:
+basestring = str
+
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
Index: lldb/examples/summaries/cocoa/CFBag.py
===
--- lldb/examples/summaries/cocoa/CFBag.py
+++ lldb/examples/summaries/cocoa/CFBag.py
@@ -13,6 +13,11 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
+try:
+basestring
+except NameError:
+basestring = str
+
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
Index: lldb/examples/summaries/cocoa/CFArray.py
===
--- lldb/examples/summaries/cocoa/CFArray.py
+++ lldb/examples/summaries/cocoa/CFArray.py
@@ -13,6 +13,11 @@
 import lldb.formatters.metrics
 import lldb.formatters.Logger
 
+try:
+basestring
+except NameError:
+basestring = str
+
 statistics = lldb.formatters.metrics.Metrics()
 statistics.add_metric('invalid_isa')
 statistics.add_metric('invalid_pointer')
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Paul Robinson via Phabricator via lldb-commits]

probinson added a comment.

I remember LLVM's DWARFContext being irritating to deal with when I was doing 
the DWARF 5 stuff.  The exact details have been swapped out of my memory, but I 
suspect it had something to do with needing to know whether I was in DWO mode 
or normal mode in order to snag the right section for some sort of 
cross-section lookup.  This is because DWARFContext is the dumping ground for 
all the sections, regardless of what kind of file we're looking at, and it's up 
to the user of its interfaces to know what mode it's in.
I will admit that having one dumping ground is convenient in some ways, 
especially when we were looking at object files that contained a mix of both 
normal and .dwo sections, but IIRC we don't emit mixed-mode object files 
anymore.

Not objecting to this patch, but it was the most convenient place to raise this 
particular grump, and something to think about as you progress.


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[James Henderson via Phabricator via lldb-commits]

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

LGTM. I don't really know much about the minidump stuff, but the general 
structure looks fine to me.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59482



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

[Lldb-commits] [PATCH] D59579: Use list comprehension instead of map/filter to prepare Python2/3 compat

[Pavel Labath via Phabricator via lldb-commits]

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

I think this should be fine, modulo the typo. TBH, I was not even aware some of 
these files existed. The ones that are already used by people on a daily basis 
should by already python3-ready.




Comment at: lldb/utils/lui/lldbutil.py:729
 
-return map(GetPCAddress, range(thread.GetNumFrames()))
+return [GetPCAddress(i) for in in range(thread.GetNumFrames())]
 

typo (`in in`)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59579



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:28
+
+  void SetDwarfData(const DWARFDataExtractor &dwarf);
+

Don't want to block progress here over this, but I just wanted to say that my 
impression was that this DwarfData business was a relict from the time when we 
re not mmapping the input object files. These days, we always mmap the input, 
and so I don't believe that should be necessary because ObjectFileMachO will 
automatically perform the DataBuffer slicing that you're doing here manually as 
a part of ReadSectionData.

Maybe one of the Apple folks could check whether this is really needed, because 
if it isn't, it will make your job easier here, as well as produce an API that 
is more similar to its llvm counterpart.


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Pavel Labath via Phabricator via lldb-commits]

labath added reviewers: markmentovai, amccarth.
labath added a comment.

Thanks for the review.

I've added some extra people who should be familiar with the minidump format 
(in addition to @clayborg, and @zturner, who should also have some knowledge of 
it), just in case they have any thoughts/suggestions about this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59482



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

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:27
+struct Stream {
+  enum class StreamKind {
+Hex,

Maybe "Encoding" might be a better name?



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:28
+  enum class StreamKind {
+Hex,
+SystemInfo,

Maybe "Binary" instead of "Hex"? Or maybe "RawBytes"?



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:48
+/// fallback when no other stream kind is suitable.
+struct HexStream : public Stream {
+  yaml::BinaryRef Content;

Name "ByteStream" maybe? Does hex make sense?



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:50
+  yaml::BinaryRef Content;
+  yaml::Hex32 Size;
+

Is the "Size" necessary here? Does yaml::BinaryRef not contain a size?



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:81
+/// /proc/ file on linux).
+struct TextStream : public Stream {
+  BlockStringRef Text;

Maybe further specify the content like "UTF8Stream" or "ASCIIStream"? Not sure 
what the encoding is expected to be, but might be better to indicate the exact 
text encoding here?



Comment at: lib/ObjectYAML/MinidumpYAML.cpp:132
+std::unique_ptr Stream::create(StreamType Type) {
+  StreamKind Kind = getKind(Type);
+  switch (Kind) {

This is where encoding might be a bit more clear?
```
Encoding E = getEncoding(Type);
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59482



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[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/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:17
+static const DWARFDataExtractor *LoadOrGetSection(
+Module &module, const llvm::Optional 
&mapped_dwarf_data,
+SectionType section_type, llvm::Optional &extractor) {

is "mapped_dwarf_data" actually the entire file mmap'ed? If so rename this to 
"mapped_file_data"?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:40
+
+  module.GetObjectFile()->ReadSectionData(section_sp.get(), *extractor);
+  return extractor.getPointer();

Use:

```
  lldb::offset_t Section::GetSectionData(DataExtractor &data);
```
We might want to make the ObjectFile::ReadSectionData private so people don't 
use it and add a doxygen comment to use the above API in Section?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:46-49
+void DWARFContext::SetDwarfData(const DWARFDataExtractor &dwarf) {
+  m_dwarf_data = dwarf;
+}
+

Again, is this the entire file that is mapped?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:21
+  Module &m_module;
+  llvm::Optional m_dwarf_data;
+

What does m_dwarf_data contain? The entire file? Just the __DWARF segment for 
mach-o?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:28
+
+  void SetDwarfData(const DWARFDataExtractor &dwarf);
+

labath wrote:
> Don't want to block progress here over this, but I just wanted to say that my 
> impression was that this DwarfData business was a relict from the time when 
> we re not mmapping the input object files. These days, we always mmap the 
> input, and so I don't believe that should be necessary because 
> ObjectFileMachO will automatically perform the DataBuffer slicing that you're 
> doing here manually as a part of ReadSectionData.
> 
> Maybe one of the Apple folks could check whether this is really needed, 
> because if it isn't, it will make your job easier here, as well as produce an 
> API that is more similar to its llvm counterpart.
We don't always mmap. We mmap if the file is local. We read the entire file if 
the file is on a network drive. If we don't do this and we mmap a NFS file, and 
that mount goes away, we crash the next time we access a page that hasn't been 
paged in. So we don't know if stuff is actually mmap'ed or not.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:41
+  void SetDwarfData(SymbolFileDWARF *dwarf2Data,
+lldb_private::DWARFContext *dwarf_context);
 

Store as a reference maybe? We must assume that the DWARFContext will be alive 
as long as this class is around. We would need to supply this in the 
constructor then. Is that possible?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:70
   SymbolFileDWARF *m_dwarf2Data;
+  lldb_private::DWARFContext *m_dwarf_context;
   CompileUnitColl m_compile_units;

Be nice if this can be a reference and be set in the ctor



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:690-691
static_cast(this));
 if (get_debug_info_data().GetByteSize() > 0) {
   m_info.reset(new DWARFDebugInfo());
   if (m_info) {

```
m_info.reset(new DWARFDebugInfo(m_dwarf_context));
```



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:693
   if (m_info) {
-m_info->SetDwarfData(this);
+m_info->SetDwarfData(this, &m_dwarf_context);
   }

revert


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59580: Use compatible print statements for Python2/3

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Thanks for your interest in the subject of Python! I converted everything I was 
able to find looking at our test suite, it seems that we lack coverage for 
these scripts.
Did you end up testing them with both py 2 and py 3 or just applied a tool on 
them?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59580



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:28
+
+  void SetDwarfData(const DWARFDataExtractor &dwarf);
+

clayborg wrote:
> labath wrote:
> > Don't want to block progress here over this, but I just wanted to say that 
> > my impression was that this DwarfData business was a relict from the time 
> > when we re not mmapping the input object files. These days, we always mmap 
> > the input, and so I don't believe that should be necessary because 
> > ObjectFileMachO will automatically perform the DataBuffer slicing that 
> > you're doing here manually as a part of ReadSectionData.
> > 
> > Maybe one of the Apple folks could check whether this is really needed, 
> > because if it isn't, it will make your job easier here, as well as produce 
> > an API that is more similar to its llvm counterpart.
> We don't always mmap. We mmap if the file is local. We read the entire file 
> if the file is on a network drive. If we don't do this and we mmap a NFS 
> file, and that mount goes away, we crash the next time we access a page that 
> hasn't been paged in. So we don't know if stuff is actually mmap'ed or not.
Yeah, I wasn't being fully correct here, but this distinction does not really 
matter here. Even if we don't mmap, we will end up with a full image of the 
file in memory, so anybody can slice any chunk of that file as he sees fit. 
Since ObjectFileMachO::ReadSectionData already implements this slicing, there's 
no advantage in slicing manually here.


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

[Greg Clayton via Phabricator via lldb-commits]

clayborg marked an inline comment as done.
clayborg added inline comments.



Comment at: 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py:533
+self.assertEqual(2, len(modules))
+self.verify_module(modules[0], "/tmp/a", None)
+self.verify_module(modules[1], "/tmp/b", None)

serge-sans-paille wrote:
> labath wrote:
> > Am I right in thinking that if I have an object file called `/tmp/a` on my 
> > system, then lldb will pick it up (and it's build-id), causing this test to 
> > fail ? If that's the case then it would be better to use some path which is 
> > less likely to exist.
> I second this. https://docs.python.org/3/library/tempfile.html provides ways 
> to create such temporary names.
These have strict UUID values that must match and the value is hard coded into 
the minidump files. So even if there is such a file, the UUID won't match.


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

https://reviews.llvm.org/D59433



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Zachary Turner via Phabricator via lldb-commits]

zturner marked 5 inline comments as done.
zturner added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:17
+static const DWARFDataExtractor *LoadOrGetSection(
+Module &module, const llvm::Optional 
&mapped_dwarf_data,
+SectionType section_type, llvm::Optional &extractor) {

clayborg wrote:
> is "mapped_dwarf_data" actually the entire file mmap'ed? If so rename this to 
> "mapped_file_data"?
It's the contents of the `__DWARF` segment on MachO, unless my reading of the 
old code is wrong.

In the old code, `SymbolFileDWARF` would try to read this segment, and if it 
existed set it to `SymbolFileDWARF::m_dwarf_data`.  Then, the 
`ReadCachedSectionData` function would first check if this member had data in 
it, and if so read the data from there instead of from the ObjectFile.

In this modified version of the patch, I call `SetDwarfData` with the contents 
of this section, then pass it through to this function so that we can perform 
the same logic.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:40
+
+  module.GetObjectFile()->ReadSectionData(section_sp.get(), *extractor);
+  return extractor.getPointer();

clayborg wrote:
> Use:
> 
> ```
>   lldb::offset_t Section::GetSectionData(DataExtractor &data);
> ```
> We might want to make the ObjectFile::ReadSectionData private so people don't 
> use it and add a doxygen comment to use the above API in Section?
Yes, I like that better.  Thanks for pointing it out.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:46-49
+void DWARFContext::SetDwarfData(const DWARFDataExtractor &dwarf) {
+  m_dwarf_data = dwarf;
+}
+

clayborg wrote:
> Again, is this the entire file that is mapped?
As mentioned elsewhere, this is the contents of `__DWARF`, which I didn't 
understand the purpose of, so I was hesitant to make any drastic changes.  
Perhaps you could clarify the significance of it now though and perhaps suggest 
an alternative approach to what we're doing here.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:28
+
+  void SetDwarfData(const DWARFDataExtractor &dwarf);
+

labath wrote:
> clayborg wrote:
> > labath wrote:
> > > Don't want to block progress here over this, but I just wanted to say 
> > > that my impression was that this DwarfData business was a relict from the 
> > > time when we re not mmapping the input object files. These days, we 
> > > always mmap the input, and so I don't believe that should be necessary 
> > > because ObjectFileMachO will automatically perform the DataBuffer slicing 
> > > that you're doing here manually as a part of ReadSectionData.
> > > 
> > > Maybe one of the Apple folks could check whether this is really needed, 
> > > because if it isn't, it will make your job easier here, as well as 
> > > produce an API that is more similar to its llvm counterpart.
> > We don't always mmap. We mmap if the file is local. We read the entire file 
> > if the file is on a network drive. If we don't do this and we mmap a NFS 
> > file, and that mount goes away, we crash the next time we access a page 
> > that hasn't been paged in. So we don't know if stuff is actually mmap'ed or 
> > not.
> Yeah, I wasn't being fully correct here, but this distinction does not really 
> matter here. Even if we don't mmap, we will end up with a full image of the 
> file in memory, so anybody can slice any chunk of that file as he sees fit. 
> Since ObjectFileMachO::ReadSectionData already implements this slicing, 
> there's no advantage in slicing manually here.
The parameter here represents the content of the `__DWARF` data segment on 
MachO.  I don't really know what this is for or the implications of removing 
it, so for now I left it identical to the original code with NFC, and planned 
on addressing this in the future.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:70
   SymbolFileDWARF *m_dwarf2Data;
+  lldb_private::DWARFContext *m_dwarf_context;
   CompileUnitColl m_compile_units;

clayborg wrote:
> Be nice if this can be a reference and be set in the ctor
Fair enough, I kept it as a pointer for parity with `SymbolFileDWARF* 
m_dwarf2Data`, but a reference is definitely better.


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py:533
+self.assertEqual(2, len(modules))
+self.verify_module(modules[0], "/tmp/a", None)
+self.verify_module(modules[1], "/tmp/b", None)

clayborg wrote:
> serge-sans-paille wrote:
> > labath wrote:
> > > Am I right in thinking that if I have an object file called `/tmp/a` on 
> > > my system, then lldb will pick it up (and it's build-id), causing this 
> > > test to fail ? If that's the case then it would be better to use some 
> > > path which is less likely to exist.
> > I second this. https://docs.python.org/3/library/tempfile.html provides 
> > ways to create such temporary names.
> These have strict UUID values that must match and the value is hard coded 
> into the minidump files. So even if there is such a file, the UUID won't 
> match.
That is true for other files, but is it true about this one? I specifically 
highlighted this instance one because it tests the uuid-not-present case (in 
which case I believe lldb will interpret that as matching any object file).


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

https://reviews.llvm.org/D59433



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

We can get rid of the m_dwarf_data all together. As Pavel suggested, we used to 
not mmap the entire file, and now we do most of the time. The 
Section::GetSectionData() will ref count the mmap data in the data extractor 
and use the mmap already, so we should only support grabbing the contents via 
the Section::GetSectionData() and don't propagate this legacy code in the new 
DWARFContext.


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:17
+static const DWARFDataExtractor *LoadOrGetSection(
+Module &module, const llvm::Optional 
&mapped_dwarf_data,
+SectionType section_type, llvm::Optional &extractor) {

zturner wrote:
> clayborg wrote:
> > is "mapped_dwarf_data" actually the entire file mmap'ed? If so rename this 
> > to "mapped_file_data"?
> It's the contents of the `__DWARF` segment on MachO, unless my reading of the 
> old code is wrong.
> 
> In the old code, `SymbolFileDWARF` would try to read this segment, and if it 
> existed set it to `SymbolFileDWARF::m_dwarf_data`.  Then, the 
> `ReadCachedSectionData` function would first check if this member had data in 
> it, and if so read the data from there instead of from the ObjectFile.
> 
> In this modified version of the patch, I call `SetDwarfData` with the 
> contents of this section, then pass it through to this function so that we 
> can perform the same logic.
remove "mapped_dwarf_data" arg



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:34-38
+  if (mapped_dwarf_data.hasValue()) {
+extractor->SetData(*mapped_dwarf_data, section_sp->GetOffset(),
+   section_sp->GetFileSize());
+return extractor.getPointer();
+  }

remove



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:46-49
+void DWARFContext::SetDwarfData(const DWARFDataExtractor &dwarf) {
+  m_dwarf_data = dwarf;
+}
+

zturner wrote:
> clayborg wrote:
> > Again, is this the entire file that is mapped?
> As mentioned elsewhere, this is the contents of `__DWARF`, which I didn't 
> understand the purpose of, so I was hesitant to make any drastic changes.  
> Perhaps you could clarify the significance of it now though and perhaps 
> suggest an alternative approach to what we're doing here.
remove



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:50
+
+const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() {
+  return LoadOrGetSection(m_module, m_dwarf_data, 
eSectionTypeDWARFDebugAranges,

rename to "getArangesData()"? Would it be better to return an 
Optional?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:21
+  Module &m_module;
+  llvm::Optional m_dwarf_data;
+

clayborg wrote:
> What does m_dwarf_data contain? The entire file? Just the __DWARF segment for 
> mach-o?
Remove this now that we don't need it since Section::GetSectionData() will do 
the right thing already.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:28
+
+  void SetDwarfData(const DWARFDataExtractor &dwarf);
+

zturner wrote:
> labath wrote:
> > clayborg wrote:
> > > labath wrote:
> > > > Don't want to block progress here over this, but I just wanted to say 
> > > > that my impression was that this DwarfData business was a relict from 
> > > > the time when we re not mmapping the input object files. These days, we 
> > > > always mmap the input, and so I don't believe that should be necessary 
> > > > because ObjectFileMachO will automatically perform the DataBuffer 
> > > > slicing that you're doing here manually as a part of ReadSectionData.
> > > > 
> > > > Maybe one of the Apple folks could check whether this is really needed, 
> > > > because if it isn't, it will make your job easier here, as well as 
> > > > produce an API that is more similar to its llvm counterpart.
> > > We don't always mmap. We mmap if the file is local. We read the entire 
> > > file if the file is on a network drive. If we don't do this and we mmap a 
> > > NFS file, and that mount goes away, we crash the next time we access a 
> > > page that hasn't been paged in. So we don't know if stuff is actually 
> > > mmap'ed or not.
> > Yeah, I wasn't being fully correct here, but this distinction does not 
> > really matter here. Even if we don't mmap, we will end up with a full image 
> > of the file in memory, so anybody can slice any chunk of that file as he 
> > sees fit. Since ObjectFileMachO::ReadSectionData already implements this 
> > slicing, there's no advantage in slicing manually here.
> The parameter here represents the content of the `__DWARF` data segment on 
> MachO.  I don't really know what this is for or the implications of removing 
> it, so for now I left it identical to the original code with NFC, and planned 
> on addressing this in the future.
We can get rid of this after thinking about this.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:404-407
+if (section) {
   m_obj_file->ReadSectionData(section, m_dwarf_data);
+  m_dwarf_context.SetDwarfData(m_dwarf_data);
+}

Get rid of m_dwarf_data now that we don't need it? That will make subsequent 
patches easier.


===

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Pavel Labath via Phabricator via lldb-commits]

labath marked 8 inline comments as done.
labath added inline comments.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:27
+struct Stream {
+  enum class StreamKind {
+Hex,

clayborg wrote:
> Maybe "Encoding" might be a better name?
On it's own I agree Encoding would be better, but the term `Kind` is already 
used by the EFLYAML implementation, so I think it's better to be consistent 
with that.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:28
+  enum class StreamKind {
+Hex,
+SystemInfo,

clayborg wrote:
> Maybe "Binary" instead of "Hex"? Or maybe "RawBytes"?
Makes sense. In fact ELF already uses the name `RawContent` for the equivalent 
functionality, so I went with that. I also changed `Text` to `TextContent`.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:50
+  yaml::BinaryRef Content;
+  yaml::Hex32 Size;
+

clayborg wrote:
> Is the "Size" necessary here? Does yaml::BinaryRef not contain a size?
This is another bit I stole from ELFYAML. The idea is that you can specify only 
the size, in case you don't actually care about the content, or specify the 
size+content if you only care about the initial few bytes of the section. 
However, it looks like I didn't finish copying the idea, so I was missing the 
bits that check that `Size >= Content.size()` and the bit which pads the size 
when serializing. Now I fix that and add some additional tests.



Comment at: include/llvm/ObjectYAML/MinidumpYAML.h:81
+/// /proc/ file on linux).
+struct TextStream : public Stream {
+  BlockStringRef Text;

clayborg wrote:
> Maybe further specify the content like "UTF8Stream" or "ASCIIStream"? Not 
> sure what the encoding is expected to be, but might be better to indicate the 
> exact text encoding here?
Unfortunately, I don't think there is a well-defined encoding here. I think the 
files will be mostly ASCII, but for instance the encoding of the "filename" 
part of /proc/PID/maps can really be anything. So, I think we should just keep 
this bit deliberately vague.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59482



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

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 191508.
labath marked 2 inline comments as done.
labath added a comment.

- rename Text/Hex streams
- fix Hex stream size handling and add some tests for it


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59482

Files:
  include/llvm/ObjectYAML/MinidumpYAML.h
  include/llvm/ObjectYAML/ObjectYAML.h
  lib/ObjectYAML/CMakeLists.txt
  lib/ObjectYAML/MinidumpYAML.cpp
  lib/ObjectYAML/ObjectYAML.cpp
  tools/yaml2obj/CMakeLists.txt
  tools/yaml2obj/yaml2minidump.cpp
  tools/yaml2obj/yaml2obj.cpp
  tools/yaml2obj/yaml2obj.h
  unittests/ObjectYAML/CMakeLists.txt
  unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- /dev/null
+++ unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -0,0 +1,207 @@
+//===- MinidumpYAMLTest.cpp - Tests for Minidump<->YAML code --===//
+//
+// 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 "llvm/ObjectYAML/MinidumpYAML.h"
+#include "llvm/Object/Minidump.h"
+#include "llvm/ObjectYAML/ObjectYAML.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::minidump;
+
+static Expected>
+toBinary(StringRef Yaml) {
+  SmallString<0> Binary;
+  raw_svector_ostream OS(Binary);
+  if (Error E = MinidumpYAML::writeAsBinary(Yaml, OS))
+return std::move(E);
+
+  return object::MinidumpFile::create(MemoryBufferRef(Binary, "Binary"));
+}
+
+TEST(MinidumpYAML, Basic) {
+  auto ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  ARM64
+Platform ID: Linux
+CSD Version RVA: 0x01020304
+CPU:
+  CPUID:   0x05060708
+  - Type:LinuxMaps
+Text: |
+  400d9000-400db000 r-xp  b3:04 227/system/bin/app_process
+  400db000-400dc000 r--p 1000 b3:04 227/system/bin/app_process
+
+  - Type:LinuxAuxv
+Content: DEADBEEFBAADF00D)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(3u, File.streams().size());
+
+  EXPECT_EQ(StreamType::SystemInfo, File.streams()[0].Type);
+  auto ExpectedSysInfo = File.getSystemInfo();
+  ASSERT_THAT_EXPECTED(ExpectedSysInfo, Succeeded());
+  const SystemInfo &SysInfo = *ExpectedSysInfo;
+  EXPECT_EQ(ProcessorArchitecture::ARM64, SysInfo.ProcessorArch);
+  EXPECT_EQ(OSPlatform::Linux, SysInfo.PlatformId);
+  EXPECT_EQ(0x01020304u, SysInfo.CSDVersionRVA);
+  EXPECT_EQ(0x05060708u, SysInfo.CPU.Arm.CPUID);
+
+  EXPECT_EQ(StreamType::LinuxMaps, File.streams()[1].Type);
+  EXPECT_EQ("400d9000-400db000 r-xp  b3:04 227"
+"/system/bin/app_process\n"
+"400db000-400dc000 r--p 1000 b3:04 227"
+"/system/bin/app_process\n",
+toStringRef(*File.getRawStream(StreamType::LinuxMaps)));
+
+  EXPECT_EQ(StreamType::LinuxAuxv, File.streams()[2].Type);
+  EXPECT_EQ((ArrayRef{0xDE, 0xAD, 0xBE, 0xEF, 0xBA, 0xAD, 0xF0, 0x0D}),
+File.getRawStream(StreamType::LinuxAuxv));
+}
+
+TEST(MinidumpYAML, RawContent) {
+  // Size shorter than content.
+  auto ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:LinuxAuxv
+Size:7
+Content: DEADBEEFBAADF00D)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
+
+  ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:LinuxAuxv
+Size:9
+Content: DEADBEEFBAADF00D)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  EXPECT_EQ(
+  (ArrayRef{0xDE, 0xAD, 0xBE, 0xEF, 0xBA, 0xAD, 0xF0, 0x0D, 0x00}),
+  File.getRawStream(StreamType::LinuxAuxv));
+}
+
+TEST(MinidumpYAML, X86SystemInfo) {
+  // Vendor ID too short.
+  auto ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  X86
+Platform ID: Linux
+CPU:
+  Vendor ID:   LLVMLLVMLLV
+  Version Info:0x01020304
+  Feature Info:0x05060708
+  AMD Extended Features: 0x09000102)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
+
+  // Vendor ID too long.
+  ExpectedFile = toBinary(R"(
+--- !minidump
+Streams:
+  - Type:SystemInfo
+Processor Arch:  X86
+Platform ID: Linux
+CPU:
+  Vendor ID:   LLVMLLVMLLVML
+  Version Info:0x01020304
+  Feature Info:0x05060708
+  AMD Extended Features: 0x09000102)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Failed());
+
+  ExpectedFile = toBinary(R"(
+--- !mini

[Lldb-commits] [PATCH] D59482: [ObjectYAML] Add basic minidump generation support

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

LGTM


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59482



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Zachary Turner via Phabricator via lldb-commits]

zturner updated this revision to Diff 191510.
zturner marked an inline comment as done.
zturner added a comment.

Updated based on suggestions, including removal of the `m_dwarf_data` member 
variable which holds the contents of the `__DWARF` segment on MachO.


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

https://reviews.llvm.org/D59562

Files:
  lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -33,12 +33,6 @@
   if (section_list) {
 SectionSP section_sp(section_list->FindSectionByType(sect_type, true));
 if (section_sp) {
-  // See if we memory mapped the DWARF segment?
-  if (m_dwarf_data.GetByteSize()) {
-data.SetData(m_dwarf_data, section_sp->GetOffset(),
- section_sp->GetFileSize());
-return;
-  }
 
   if (m_obj_file->ReadSectionData(section_sp.get(), data) != 0)
 return;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -30,6 +30,7 @@
 #include "lldb/Utility/RangeMap.h"
 #include "lldb/lldb-private.h"
 
+#include "DWARFContext.h"
 #include "DWARFDataExtractor.h"
 #include "DWARFDefines.h"
 #include "DWARFIndex.h"
@@ -227,7 +228,6 @@
 
   virtual const lldb_private::DWARFDataExtractor &get_debug_abbrev_data();
   virtual const lldb_private::DWARFDataExtractor &get_debug_addr_data();
-  const lldb_private::DWARFDataExtractor &get_debug_aranges_data();
   const lldb_private::DWARFDataExtractor &get_debug_frame_data();
   virtual const lldb_private::DWARFDataExtractor &get_debug_info_data();
   const lldb_private::DWARFDataExtractor &get_debug_line_data();
@@ -458,11 +458,10 @@
   llvm::once_flag m_dwp_symfile_once_flag;
   std::unique_ptr m_dwp_symfile;
 
-  lldb_private::DWARFDataExtractor m_dwarf_data;
+  lldb_private::DWARFContext m_context;
 
   DWARFDataSegment m_data_debug_abbrev;
   DWARFDataSegment m_data_debug_addr;
-  DWARFDataSegment m_data_debug_aranges;
   DWARFDataSegment m_data_debug_frame;
   DWARFDataSegment m_data_debug_info;
   DWARFDataSegment m_data_debug_line;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -355,12 +355,13 @@
   << 32), // Used by SymbolFileDWARFDebugMap to
   // when this class parses .o files to
   // contain the .o file index/ID
-  m_debug_map_module_wp(), m_debug_map_symfile(NULL), m_data_debug_abbrev(),
-  m_data_debug_aranges(), m_data_debug_frame(), m_data_debug_info(),
-  m_data_debug_line(), m_data_debug_macro(), m_data_debug_loc(),
-  m_data_debug_ranges(), m_data_debug_rnglists(), m_data_debug_str(),
-  m_data_apple_names(), m_data_apple_types(), m_data_apple_namespaces(),
-  m_abbr(), m_info(), m_line(), m_fetched_external_modules(false),
+  m_debug_map_module_wp(), m_debug_map_symfile(NULL),
+  m_context(*objfile->GetModule()), m_data_debug_abbrev(),
+  m_data_debug_frame(), m_data_debug_info(), m_data_debug_line(),
+  m_data_debug_macro(), m_data_debug_loc(), m_data_debug_ranges(),
+  m_data_debug_rnglists(), m_data_debug_str(), m_data_apple_names(),
+  m_data_apple_types(), m_data_apple_namespaces(), m_abbr(), m_info(),
+  m_line(), m_fetched_external_modules(false),
   m_supports_DW_AT_APPLE_objc_complete_type(eLazyBoolCalculate), m_ranges(),
   m_unique_ast_type_map() {}
 
@@ -394,15 +395,6 @@
 
 void SymbolFileDWARF::InitializeObject() {
   Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
-  ModuleSP module_sp(m_obj_file->GetModule());
-  if (module_sp) {
-const SectionList *section_list = module_sp->GetSectionList();
-Section *section =
-section_list->FindSectionByName(GetDWARFMachOSegmentName()).get();
-
-if (section)
-  m_obj_file->ReadSectionData(section, m_dwarf_data);
-  }
 
   if (!GetGlobalPluginProperties()->IgnoreFileIndexes()) {
 DWARFDataExtractor apple_names, apple_namespaces

[Lldb-commits] [lldb] r356573 - Fix UUID decoding from minidump files

[Greg Clayton via lldb-commits]

Author: gclayton
Date: Wed Mar 20 09:50:17 2019
New Revision: 356573

URL: http://llvm.org/viewvc/llvm-project?rev=356573&view=rev
Log:
Fix UUID decoding from minidump files

This patch fixes:

UUIDs now don't include the age field from a PDB70 when the age is zero. Prior 
to this they would incorrectly contain the zero age which stopped us from being 
able to match up the UUID with real files.
UUIDs for Apple targets get the first 32 bit value and next two 16 bit values 
swapped. Breakpad incorrectly swaps these values when it creates darwin 
minidump files, so this must be undone so we can match up symbol files with the 
minidump modules.
UUIDs that are all zeroes are treated as invalid UUIDs. Breakpad will always 
save out a UUID, even if one wasn't available. This caused all files that have 
UUID values of zero to be uniqued to the first module that had a zero UUID. We 
now don't fill in the UUID if it is all zeroes.
Added tests for PDB70 and ELF build ID based CvRecords.

Differential Revision: https://reviews.llvm.org/D59433


Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-16.dmp
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-20.dmp
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-no-age.dmp
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-with-age.dmp
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-zero-uuids.dmp
   (with props)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/macos-arm-uuids-no-age.dmp
   (with props)
Modified:
lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py?rev=356573&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
 Wed Mar 20 09:50:17 2019
@@ -0,0 +1,121 @@
+"""
+Test basics of Minidump debugging.
+"""
+
+from __future__ import print_function
+from six import iteritems
+
+import shutil
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MiniDumpUUIDTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+super(MiniDumpUUIDTestCase, self).setUp()
+self._initial_platform = lldb.DBG.GetSelectedPlatform()
+
+def tearDown(self):
+lldb.DBG.SetSelectedPlatform(self._initial_platform)
+super(MiniDumpUUIDTestCase, self).tearDown()
+
+def verify_module(self, module, verify_path, verify_uuid):
+uuid = module.GetUUIDString()
+self.assertEqual(verify_path, module.GetFileSpec().fullpath)
+self.assertEqual(verify_uuid, uuid)
+
+def test_zero_uuid_modules(self):
+"""
+Test multiple modules having a MINIDUMP_MODULE.CvRecord that is 
valid,
+but contains a PDB70 value whose age is zero and whose UUID values 
are 
+all zero. Prior to a fix all such modules would be duplicated to 
the
+first one since the UUIDs claimed to be valid and all zeroes. Now 
we
+ensure that the UUID is not valid for each module and that we have
+each of the modules in the target after loading the core
+"""
+self.dbg.CreateTarget(None)
+self.target = self.dbg.GetSelectedTarget()
+self.process = self.target.LoadCore("linux-arm-zero-uuids.dmp")
+modules = self.target.modules
+self.assertEqual(2, len(modules))
+self.verify_module(modules[0], "/file/does/not/exist/a", None)
+self.verify_module(modules[1], "/file/does/not/exist/b", None)
+
+def test_uuid_modules_no_age(self):
+"""
+Test multiple modules having a MINIDUMP_MODULE.CvRecord that is 
valid,
+and contains a PDB70 value whose age is zero and whose UUID values 
are 
+valid. Ensure we decode the UUID and don't include the age field 
in the UUID.
+"""
+self.dbg.CreateTarget(None)
+self.target = self.dbg.GetSelectedTarget()
+self.process = self.target.LoadCore("linux-arm-uuids-no-age.dmp")
+   

[Lldb-commits] [PATCH] D59433: Fix UUID decoding from minidump files.

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356573: Fix UUID decoding from minidump files (authored by 
gclayton, committed by ).
Herald added subscribers: llvm-commits, jdoerfert.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D59433?vs=190901&id=191516#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59433

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-16.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-elf-build-id-20.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-no-age.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-uuids-with-age.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-arm-zero-uuids.dmp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/macos-arm-uuids-no-age.dmp
  lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp

Index: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
===
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -165,21 +165,44 @@
   static_cast(static_cast(*signature));
 
   if (cv_signature == CvSignature::Pdb70) {
-// PDB70 record
 const CvRecordPdb70 *pdb70_uuid = nullptr;
 Status error = consumeObject(cv_record, pdb70_uuid);
-if (!error.Fail()) {
-  auto arch = GetArchitecture();
-  // For Apple targets we only need a 16 byte UUID so that we can match
-  // the UUID in the Module to actual UUIDs from the built binaries. The
-  // "Age" field is zero in breakpad minidump files for Apple targets, so
-  // we restrict the UUID to the "Uuid" field so we have a UUID we can use
-  // to match.
-  if (arch.GetTriple().getVendor() == llvm::Triple::Apple)
-return UUID::fromData(pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
-  else
-return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+if (error.Fail())
+  return UUID();
+// If the age field is not zero, then include the entire pdb70_uuid struct
+if (pdb70_uuid->Age != 0)
+  return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+
+// Many times UUIDs are all zeroes. This can cause more than one module
+// to claim it has a valid UUID of all zeroes and causes the files to all
+// merge into the first module that claims this valid zero UUID.
+bool all_zeroes = true;
+for (size_t i = 0; all_zeroes && i < sizeof(pdb70_uuid->Uuid); ++i)
+  all_zeroes = pdb70_uuid->Uuid[i] == 0;
+if (all_zeroes)
+  return UUID();
+
+if (GetArchitecture().GetTriple().getVendor() == llvm::Triple::Apple) {
+  // Breakpad incorrectly byte swaps the first 32 bit and next 2 16 bit
+  // values in the UUID field. Undo this so we can match things up
+  // with our symbol files
+  uint8_t apple_uuid[16];
+  // Byte swap the first 32 bits
+  apple_uuid[0] = pdb70_uuid->Uuid[3];
+  apple_uuid[1] = pdb70_uuid->Uuid[2];
+  apple_uuid[2] = pdb70_uuid->Uuid[1];
+  apple_uuid[3] = pdb70_uuid->Uuid[0];
+  // Byte swap the next 16 bit value
+  apple_uuid[4] = pdb70_uuid->Uuid[5];
+  apple_uuid[5] = pdb70_uuid->Uuid[4];
+  // Byte swap the next 16 bit value
+  apple_uuid[6] = pdb70_uuid->Uuid[7];
+  apple_uuid[7] = pdb70_uuid->Uuid[6];
+  for (size_t i = 8; i < sizeof(pdb70_uuid->Uuid); ++i)
+apple_uuid[i] = pdb70_uuid->Uuid[i];
+  return UUID::fromData(apple_uuid, sizeof(apple_uuid));
 }
+return UUID::fromData(pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
   } else if (cv_signature == CvSignature::ElfBuildId)
 return UUID::fromData(cv_record);
 
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpUUID.py
@@ -0,0 +1,121 @@
+"""
+Test basics of Minidump debugging.
+"""
+
+from __future__ import print_function
+from six import iteritems
+
+import shutil
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MiniDumpUUIDTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCAS

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, JDevlieghere, zturner.
Herald added subscribers: ki.stfu, emaste.
Herald added a project: LLDB.

This is WIP on adding EINTR handling all over the place, via 
`llvm::sys::RetryAfterSignal()`. If you don't like this approach, please lemme 
know ASAP ;-).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D59606

Files:
  lldb/source/Core/Disassembler.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/MainLoop.cpp
  lldb/source/Host/common/PseudoTerminal.cpp
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Host/posix/DomainSocket.cpp
  lldb/source/Host/posix/PipePosix.cpp
  lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
  lldb/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
  lldb/source/Plugins/Process/Linux/ProcessorTrace.h
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/tools/darwin-debug/darwin-debug.cpp
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/PseudoTerminal.cpp
  lldb/tools/debugserver/source/RNBSocket.cpp
  lldb/tools/debugserver/source/debugserver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-mi/MIUtilFileStd.cpp
  lldb/tools/lldb-perf/common/clang/lldb_perf_clang.cpp
  lldb/tools/lldb-vscode/IOStream.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -41,6 +41,7 @@
 #include 
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -90,7 +91,8 @@
 } else {
   listen(sockfd, 5);
   socklen_t clilen = sizeof(cli_addr);
-  newsockfd = accept(sockfd, (struct sockaddr *)&cli_addr, &clilen);
+  newsockfd = llvm::sys::RetryAfterSignal(-1, accept,
+  sockfd, (struct sockaddr *)&cli_addr, &clilen);
   if (newsockfd < 0)
 if (g_vsc.log)
   *g_vsc.log << "error: accept (" << strerror(errno) << ")"
@@ -99,7 +101,7 @@
 #if defined(_WIN32)
 closesocket(sockfd);
 #else
-close(sockfd);
+llvm::sys::RetryAfterSignal(-1, close, sockfd);
 #endif
   }
   return newsockfd;
Index: lldb/tools/lldb-vscode/IOStream.cpp
===
--- lldb/tools/lldb-vscode/IOStream.cpp
+++ lldb/tools/lldb-vscode/IOStream.cpp
@@ -20,6 +20,8 @@
 #include 
 #include 
 
+#include "llvm/Support/Errno.h"
+
 using namespace lldb_vscode;
 
 StreamDescriptor::StreamDescriptor() {}
@@ -36,10 +38,10 @@
 #if defined(_WIN32)
 ::closesocket(m_socket);
 #else
-::close(m_socket);
+llvm::sys::RetryAfterSignal(-1, ::close, m_socket);
 #endif
   else
-::close(m_fd);
+llvm::sys::RetryAfterSignal(-1, ::close, m_fd);
 }
 
 StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) {
Index: lldb/tools/lldb-perf/common/clang/lldb_perf_clang.cpp
===
--- lldb/tools/lldb-perf/common/clang/lldb_perf_clang.cpp
+++ lldb/tools/lldb-perf/common/clang/lldb_perf_clang.cpp
@@ -13,6 +13,7 @@
 #include "lldb-perf/lib/Timer.h"
 #include "lldb-perf/lib/Xcode.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Errno.h"
 #include 
 #include 
 #include 
@@ -124,7 +125,7 @@
 }
 )";
 write(fd, source_content, strlen(source_content));
-close(fd);
+llvm::sys::RetryAfterSignal(-1, close, fd);
   } else {
 const char *error_cstr = strerror(errno);
 fprintf(stderr,
Index: lldb/tools/lldb-mi/MIUtilFileStd.cpp
===
--- lldb/tools/lldb-mi/MIUtilFileStd.cpp
+++ lldb/tools/lldb-mi/MIUtilFileStd.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/FileSystem.h"
 
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Errno.h"
 
 //++
 //
@@ -195,7 +196,7 @@
   if (m_pFileHandle == nullptr)
 return;
 
-  ::fclose(m_pFileHandle);
+  llvm::sys::RetryAfterSignal(EOF, ::fclose, m_pFileHandle);
   m_pFileHandle = nullptr;
   // m_bFileError = false; Do not reset as want to remain until next attempt at
   // open or create
@@ -228,7 +229,7 @@
   FILE *pTmp = nullptr;
   pTmp = ::fopen(vFileNamePath.c_str(), "wb");
   if (pTmp != nullptr) {
-::fclose(pTmp);
+llvm::sys::RetryAfterSignal(EOF, ::fclose, pTmp);
 return true;
   }
 
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -19,6 +19,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Errno.h"
 #include "llvm/Support/F

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The "problem" I have with this is that (IIRC) retrying close(2) on EINTR is the 
*wrong* thing to do on linux (because the fd will be closed anyway, and so we 
may end up closing someone else's file the second time around). I'll try to dig 
up more info about that tomorrow.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59606



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

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D59606#1436796 , @labath wrote:

> The "problem" I have with this is that (IIRC) retrying close(2) on EINTR is 
> the *wrong* thing to do on linux (because the fd will be closed anyway, and 
> so we may end up closing someone else's file the second time around). I'll 
> try to dig up more info about that tomorrow.


Just found http://alobbs.com/post/54503240599/close-and-eintr and indeed you're 
right. While I find that behavior really silly and would consider it a bug, I 
see that POSIX considers it 'undefined' whether the file is closed or not. I 
guess I'll have to be more careful when deciding which syscalls to wrap.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59606



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

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Hmm, I also see that LLVM has signal-safe 
`Process::SafelyCloseFileDescriptor()`. Should I use that, or just ignore 
potential issues with `close()`?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59606



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

[Lldb-commits] [lldb] r356592 - Remove the unused return value in ASTImporter::Imported [NFC]

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Wed Mar 20 12:00:25 2019
New Revision: 356592

URL: http://llvm.org/viewvc/llvm-project?rev=356592&view=rev
Log:
Remove the unused return value in ASTImporter::Imported [NFC]

Summary:
`ASTImporter::Imported` currently returns a Decl, but that return value is not 
used by the ASTImporter (or anywhere else)
nor is it documented.

Reviewers: balazske, martong, a.sidorin, shafik

Reviewed By: balazske, martong

Subscribers: rnkovacs, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D59595

Modified:
lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
lldb/trunk/source/Symbol/ClangASTImporter.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTImporter.h?rev=356592&r1=356591&r2=356592&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTImporter.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTImporter.h Wed Mar 20 12:00:25 2019
@@ -262,7 +262,7 @@ private:
 
 void ImportDefinitionTo(clang::Decl *to, clang::Decl *from);
 
-clang::Decl *Imported(clang::Decl *from, clang::Decl *to) override;
+void Imported(clang::Decl *from, clang::Decl *to) override;
 
 clang::Decl *GetOriginalDecl(clang::Decl *To) override;
 

Modified: lldb/trunk/source/Symbol/ClangASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTImporter.cpp?rev=356592&r1=356591&r2=356592&view=diff
==
--- lldb/trunk/source/Symbol/ClangASTImporter.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTImporter.cpp Wed Mar 20 12:00:25 2019
@@ -937,7 +937,7 @@ void ClangASTImporter::Minion::ImportDef
   }
 }
 
-clang::Decl *ClangASTImporter::Minion::Imported(clang::Decl *from,
+void ClangASTImporter::Minion::Imported(clang::Decl *from,
 clang::Decl *to) {
   ClangASTMetrics::RegisterClangImport();
 
@@ -1096,8 +1096,6 @@ clang::Decl *ClangASTImporter::Minion::I
   }
 }
   }
-
-  return clang::ASTImporter::Imported(from, to);
 }
 
 clang::Decl *ClangASTImporter::Minion::GetOriginalDecl(clang::Decl *To) {


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

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 191553.
mgorny added a comment.

Got rid of `close()` and `fclose()` wrapping, for now.


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

https://reviews.llvm.org/D59606

Files:
  lldb/source/Host/common/Socket.cpp
  lldb/source/Host/common/TCPSocket.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Host/posix/DomainSocket.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/tools/darwin-debug/darwin-debug.cpp
  lldb/tools/debugserver/source/RNBSocket.cpp
  lldb/tools/debugserver/source/debugserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -41,6 +41,7 @@
 #include 
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -90,7 +91,8 @@
 } else {
   listen(sockfd, 5);
   socklen_t clilen = sizeof(cli_addr);
-  newsockfd = accept(sockfd, (struct sockaddr *)&cli_addr, &clilen);
+  newsockfd = llvm::sys::RetryAfterSignal(-1, accept,
+  sockfd, (struct sockaddr *)&cli_addr, &clilen);
   if (newsockfd < 0)
 if (g_vsc.log)
   *g_vsc.log << "error: accept (" << strerror(errno) << ")"
Index: lldb/tools/debugserver/source/debugserver.cpp
===
--- lldb/tools/debugserver/source/debugserver.cpp
+++ lldb/tools/debugserver/source/debugserver.cpp
@@ -681,8 +681,9 @@
 saddr_un.sun_path[sizeof(saddr_un.sun_path) - 1] = '\0';
 saddr_un.sun_len = SUN_LEN(&saddr_un);
 
-if (::connect(s, (struct sockaddr *)&saddr_un,
-  static_cast(SUN_LEN(&saddr_un))) < 0) {
+if (llvm::sys::RetryAfterSignal(-1, ::connect, s,
+  (struct sockaddr *)&saddr_un,
+  static_cast(SUN_LEN(&saddr_un))) < 0) {
   perror("error: connect (socket, &saddr_un, saddr_un_len)");
   exit(1);
 }
Index: lldb/tools/debugserver/source/RNBSocket.cpp
===
--- lldb/tools/debugserver/source/RNBSocket.cpp
+++ lldb/tools/debugserver/source/RNBSocket.cpp
@@ -25,6 +25,7 @@
 #include 
 
 #include "lldb/Host/SocketAddress.h"
+#include "llvm/Support/Errno.h"
 
 #ifdef WITH_LOCKDOWN
 #include "lockdown.h"
@@ -169,7 +170,8 @@
   lldb_private::SocketAddress &addr_in = socket_pair->second;
   lldb_private::SocketAddress accept_addr;
   socklen_t sa_len = accept_addr.GetMaxLength();
-  m_fd = ::accept(sock_fd, &accept_addr.sockaddr(), &sa_len);
+  m_fd = llvm::sys::RetryAfterSignal(-1, ::accept,
+  sock_fd, &accept_addr.sockaddr(), &sa_len);
 
   if (m_fd == -1) {
 err.SetError(errno, DNBError::POSIX);
@@ -230,7 +232,8 @@
 
 address.SetPort(port);
 
-if (-1 == ::connect(m_fd, &address.sockaddr(), address.GetLength())) {
+if (-1 == llvm::sys::RetryAfterSignal(-1, ::connect, m_fd,
+  &address.sockaddr(), address.GetLength())) {
   Disconnect(false);
   continue;
 }
Index: lldb/tools/darwin-debug/darwin-debug.cpp
===
--- lldb/tools/darwin-debug/darwin-debug.cpp
+++ lldb/tools/darwin-debug/darwin-debug.cpp
@@ -39,6 +39,8 @@
 
 #include 
 
+#include "llvm/Support/Errno.h"
+
 #ifndef _POSIX_SPAWN_DISABLE_ASLR
 #define _POSIX_SPAWN_DISABLE_ASLR 0x0100
 #endif
@@ -287,7 +289,8 @@
   saddr_un.sun_path[sizeof(saddr_un.sun_path) - 1] = '\0';
   saddr_un.sun_len = SUN_LEN(&saddr_un);
 
-  if (::connect(s, (struct sockaddr *)&saddr_un, SUN_LEN(&saddr_un)) < 0) {
+  if (llvm::sys::RetryAfterSignal(-1, ::connect, s,
+(struct sockaddr *)&saddr_un, SUN_LEN(&saddr_un)) < 0) {
 perror("error: connect (socket, &saddr_un, saddr_un_len)");
 exit(1);
   }
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -21,6 +21,7 @@
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/Errno.h"
 
 #include 
 
@@ -39,7 +40,7 @@
 
 void PythonObject::Dump(Stream &strm) const {
   if (m_py_obj) {
-FILE *file = ::tmpfile();
+FILE *file = llvm::sys::RetryAfterSignal(nullptr, ::tmpfile);
 if (file) {
   ::PyObject_Print(m_py_obj, file, 0);
   const long length = ftell(file);
@@ -47,7 +48,8 @@
 ::rewind(file);
 std::vector file_contents(length, '\0');
 const size_t length_read =
-::fread(file_contents.data(), 1, file_contents.size(), file);
+llvm::sys::RetryAfterSignal(0, ::fread,
+file_c

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

LGTM as long as the test suite is happy


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

https://reviews.llvm.org/D59606



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:24
+  // forward.
+  extractor.emplace();
+

Don't we want to put an empty DWARFDataExtractor in here? Maybe this code 
should be:

```
const SectionList *section_list = module.GetSectionList();
  if (!section_list)
return nullptr;

  auto section_sp = section_list->FindSectionByType(section_type, true);
  if (!section_sp)
return nullptr;

  extractor.emplace();
  if (section_sp->GetSectionData(*extractor) == 0)
extractor = llvm::None;
  return extractor.getPointer();
```



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:40
+
+const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() {
+  return LoadOrGetSection(m_module, eSectionTypeDWARFDebugAranges,

Why do we care about "getOrLoadArangesData()"? What not just "getArangesData()"?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:552
+  data.Clear();
+  m_obj_file->ReadSectionData(section_sp.get(), data);
 }

```
if (m_obj_file->ReadSectionData(section_sp.get(), data) == 0)
  data.Clear();
```


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:24
+  // forward.
+  extractor.emplace();
+

clayborg wrote:
> Don't we want to put an empty DWARFDataExtractor in here? Maybe this code 
> should be:
> 
> ```
> const SectionList *section_list = module.GetSectionList();
>   if (!section_list)
> return nullptr;
> 
>   auto section_sp = section_list->FindSectionByType(section_type, true);
>   if (!section_sp)
> return nullptr;
> 
>   extractor.emplace();
>   if (section_sp->GetSectionData(*extractor) == 0)
> extractor = llvm::None;
>   return extractor.getPointer();
> ```
Or use a local DWARFDataExtractor and move it into "extractor" if we succeed?

```
DWARFDataExtractor data;
if (section_sp->GetSectionData(data) > 0)
extractor = std::move(data);
```


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Zachary Turner via Phabricator via lldb-commits]

zturner marked 2 inline comments as done.
zturner added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:50
+
+const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() {
+  return LoadOrGetSection(m_module, m_dwarf_data, 
eSectionTypeDWARFDebugAranges,

clayborg wrote:
> rename to "getArangesData()"? Would it be better to return an 
> Optional?
I actually prefer to keep the name as `getOrLoad`.  `get` functions often sound 
like they don't modify anything, but this way it's clear that lazy evaluation 
will occur, which might be important to the caller for some reason or another.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:24
+  // forward.
+  extractor.emplace();
+

clayborg wrote:
> clayborg wrote:
> > Don't we want to put an empty DWARFDataExtractor in here? Maybe this code 
> > should be:
> > 
> > ```
> > const SectionList *section_list = module.GetSectionList();
> >   if (!section_list)
> > return nullptr;
> > 
> >   auto section_sp = section_list->FindSectionByType(section_type, true);
> >   if (!section_sp)
> > return nullptr;
> > 
> >   extractor.emplace();
> >   if (section_sp->GetSectionData(*extractor) == 0)
> > extractor = llvm::None;
> >   return extractor.getPointer();
> > ```
> Or use a local DWARFDataExtractor and move it into "extractor" if we succeed?
> 
> ```
> DWARFDataExtractor data;
> if (section_sp->GetSectionData(data) > 0)
> extractor = std::move(data);
> ```
The suggested code here has a bug.  If you say 

```
extractor = llvm::None;
return extractor.getPointer();
```

it will assert, because there is no pointer, it's uninitialized.  When i write 
`extractor.emplace();` it initializes it with a default constructed 
`DataExtractor`, the idea being that if we try this function and it fails for 
any reason, we should not try the function again, because it already failed 
once, and so we should just "cache" the fact that it failed and immediately 
return.  By having a default constructed `DataExtractor` and changing the fast 
path exit condition to also return null in the case where the size is 0, we 
ensure that we only ever do work once, regardless of whether that work fails or 
succeeds.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:40
+
+const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() {
+  return LoadOrGetSection(m_module, eSectionTypeDWARFDebugAranges,

clayborg wrote:
> Why do we care about "getOrLoadArangesData()"? What not just 
> "getArangesData()"?
I like to emphasize in the name that the function might do work.  People are 
trained to think of `get` functions as read-only, but it's not the case here 
and so the idea is to make sure that nobody can misinterpret the behavior.


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:552
+  data.Clear();
+  m_obj_file->ReadSectionData(section_sp.get(), data);
 }

clayborg wrote:
> ```
> if (m_obj_file->ReadSectionData(section_sp.get(), data) == 0)
>   data.Clear();
> ```
What do you think about this one? This is my last nit


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Zachary Turner via Phabricator via lldb-commits]

zturner marked an inline comment as done.
zturner added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:552
+  data.Clear();
+  m_obj_file->ReadSectionData(section_sp.get(), data);
 }

clayborg wrote:
> clayborg wrote:
> > ```
> > if (m_obj_file->ReadSectionData(section_sp.get(), data) == 0)
> >   data.Clear();
> > ```
> What do you think about this one? This is my last nit
I did see that one, but I was having trouble figuring out how the behavior is 
different from what I already wrote.  The only way I could see it being 
different is if `ReadSectionData` would actually write data there even when it 
failed.  That would be strange though, do you know if it can happen?

The way I wrote it, it clears first, and then if the function fails / returns 
0, I would expect it to be unchanged.

Happy to change it if there's actually a difference I'm overlooking, but if 
they're the same then I think the branchless version is slightly easier to read.


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Greg Clayton via Phabricator via lldb-commits]

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

The suggestion just avoids the Clear() call at the expense of a branch. No big 
deal. Fine either way.


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

https://reviews.llvm.org/D59562



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

[Lldb-commits] [lldb] r356612 - Introduce DWARFContext.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Wed Mar 20 13:49:25 2019
New Revision: 356612

URL: http://llvm.org/viewvc/llvm-project?rev=356612&view=rev
Log:
Introduce DWARFContext.

LLVM's DWARF parsing library has a class called DWARFContext which holds
all of the various DWARF data sections and lots of other information.
LLDB's on the other hand stores all of this directly in SymbolFileDWARF
/ SymbolFileDWARFDwo and passes this interface around through the
parsing library. Obviously this is incompatible with a world where the
low level interface does not depend on the high level interface, so we
need to move towards a model similar to LLVM's - i.e. all of the context
needed for low level parsing should be in a single class, and that class
gets passed around.

This patch is a small incremental step towards achieving this. The
interface and internals deviate from LLVM's for technical reasons, but
the high level idea is the same. The goal is, eventually, to remove all
occurrences of SymbolFileDWARF from the low level parsing code.

For now I've chosen a very simple section - the .debug_aranges section
to move into DWARFContext while leaving everything else unchanged. In
the short term this is a bit confusing because now the information you
need might come from either of 2 different locations. But it's a huge
refactor to do this all at once and runs a much higher risk of breaking
things. So I think it would be wise to do this in very small pieces.

TL;DR - No functional change

Differential Revision: https://reviews.llvm.org/D59562

Added:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt?rev=356612&r1=356611&r2=356612&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt Wed Mar 20 
13:49:25 2019
@@ -7,6 +7,7 @@ add_lldb_library(lldbPluginSymbolFileDWA
   DWARFAttribute.cpp
   DWARFBaseDIE.cpp
   DWARFCompileUnit.cpp
+  DWARFContext.cpp
   DWARFDataExtractor.cpp
   DWARFDebugAbbrev.cpp
   DWARFDebugAranges.cpp

Added: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp?rev=356612&view=auto
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp (added)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp Wed Mar 20 
13:49:25 2019
@@ -0,0 +1,43 @@
+//===-- DWARFContext.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 "DWARFContext.h"
+
+#include "lldb/Core/Section.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static const DWARFDataExtractor *
+LoadOrGetSection(Module &module, SectionType section_type,
+ llvm::Optional &extractor) {
+  if (extractor.hasValue())
+return extractor->GetByteSize() > 0 ? extractor.getPointer() : nullptr;
+
+  // Initialize to an empty extractor so that we always take the fast path 
going
+  // forward.
+  extractor.emplace();
+
+  const SectionList *section_list = module.GetSectionList();
+  if (!section_list)
+return nullptr;
+
+  auto section_sp = section_list->FindSectionByType(section_type, true);
+  if (!section_sp)
+return nullptr;
+
+  section_sp->GetSectionData(*extractor);
+  return extractor.getPointer();
+}
+
+DWARFContext::DWARFContext(Module &module) : m_module(module) {}
+
+const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() {
+  return LoadOrGetSection(m_module, eSectionTypeDWARFDebugAranges,
+  m_data_debug_aranges);
+}

Added: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h?rev=356612&view=auto
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h (added)
+++ lldb/trunk/source/Plugins/SymbolFile

[Lldb-commits] [PATCH] D59562: [SymbolFileDWARF] Introduce DWARFContext

[Zachary Turner via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356612: Introduce DWARFContext. (authored by zturner, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59562?vs=191510&id=191572#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59562

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFContext.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -19,6 +19,10 @@
 #include "lldb/lldb-private.h"
 #include "llvm/Support/Error.h"
 
+namespace lldb_private {
+class DWARFContext;
+}
+
 typedef std::multimap
 CStringToDIEMap;
 typedef CStringToDIEMap::iterator CStringToDIEMapIter;
@@ -32,7 +36,7 @@
   const dw_offset_t next_offset,
   const uint32_t depth, void *userData);
 
-  DWARFDebugInfo();
+  explicit DWARFDebugInfo(lldb_private::DWARFContext &context);
   void SetDwarfData(SymbolFileDWARF *dwarf2Data);
 
   size_t GetNumCompileUnits();
@@ -62,6 +66,7 @@
   // Member variables
   //--
   SymbolFileDWARF *m_dwarf2Data;
+  lldb_private::DWARFContext &m_context;
   CompileUnitColl m_compile_units;
   std::unique_ptr
   m_cu_aranges_up; // A quick address to compile unit table
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -355,12 +355,13 @@
   << 32), // Used by SymbolFileDWARFDebugMap to
   // when this class parses .o files to
   // contain the .o file index/ID
-  m_debug_map_module_wp(), m_debug_map_symfile(NULL), m_data_debug_abbrev(),
-  m_data_debug_aranges(), m_data_debug_frame(), m_data_debug_info(),
-  m_data_debug_line(), m_data_debug_macro(), m_data_debug_loc(),
-  m_data_debug_ranges(), m_data_debug_rnglists(), m_data_debug_str(),
-  m_data_apple_names(), m_data_apple_types(), m_data_apple_namespaces(),
-  m_abbr(), m_info(), m_line(), m_fetched_external_modules(false),
+  m_debug_map_module_wp(), m_debug_map_symfile(NULL),
+  m_context(*objfile->GetModule()), m_data_debug_abbrev(),
+  m_data_debug_frame(), m_data_debug_info(), m_data_debug_line(),
+  m_data_debug_macro(), m_data_debug_loc(), m_data_debug_ranges(),
+  m_data_debug_rnglists(), m_data_debug_str(), m_data_apple_names(),
+  m_data_apple_types(), m_data_apple_namespaces(), m_abbr(), m_info(),
+  m_line(), m_fetched_external_modules(false),
   m_supports_DW_AT_APPLE_objc_complete_type(eLazyBoolCalculate), m_ranges(),
   m_unique_ast_type_map() {}
 
@@ -394,15 +395,6 @@
 
 void SymbolFileDWARF::InitializeObject() {
   Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
-  ModuleSP module_sp(m_obj_file->GetModule());
-  if (module_sp) {
-const SectionList *section_list = module_sp->GetSectionList();
-Section *section =
-section_list->FindSectionByName(GetDWARFMachOSegmentName()).get();
-
-if (section)
-  m_obj_file->ReadSectionData(section, m_dwarf_data);
-  }
 
   if (!GetGlobalPluginProperties()->IgnoreFileIndexes()) {
 DWARFDataExtractor apple_names, apple_namespaces, apple_types, apple_objc;
@@ -549,19 +541,15 @@
   DWARFDataExtractor &data) {
   ModuleSP module_sp(m_obj_file->GetModule());
   const SectionList *section_list = module_sp->GetSectionList();
-  if (section_list) {
-SectionSP section_sp(section_list->FindSectionByType(sect_type, true));
-if (section_sp) {
-  // See if we memory mapped the DWARF segment?
-  if (m_dwarf_data.GetByteSize()) {
-data.SetData(m_dwarf_data, section_sp->GetOffset(),
- section_sp->GetFileSize());
-  } else {
-if (m_obj_file->ReadSectionData(section_sp.get(), data) == 0)
-  data.Clear();
-  }
-}
-  }
+  if (!section_list)
+return;
+
+  SectionSP section_sp(section_lis

[Lldb-commits] [PATCH] D59611: Move the rest of the section loading over to DWARFContext

[Zachary Turner via Phabricator via lldb-commits]

zturner created this revision.
zturner added reviewers: clayborg, JDevlieghere, labath, aprantl.
Herald added a subscriber: jdoerfert.

This gets all of the remaining sections except those that are DWO-dependent.  
That will be a bit harder to do since we'll somehow have to get knowledge of 
DWO-ness into the `DWARFContext`.

This is mostly mechanical now after D59562 .


https://reviews.llvm.org/D59611

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -26,7 +26,7 @@
   GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit) override;
 
   lldb_private::DWARFExpression::LocationListFormat
-  GetLocationListFormat() const override;
+  GetLocationListFormat() override;
 
   size_t GetObjCMethodDIEOffsets(lldb_private::ConstString class_name,
  DIEArray &method_die_offsets) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -147,7 +147,7 @@
 }
 
 DWARFExpression::LocationListFormat
-SymbolFileDWARFDwo::GetLocationListFormat() const {
+SymbolFileDWARFDwo::GetLocationListFormat() {
   return DWARFExpression::SplitDwarfLocationList;
 }
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -228,23 +228,9 @@
 
   virtual const lldb_private::DWARFDataExtractor &get_debug_abbrev_data();
   virtual const lldb_private::DWARFDataExtractor &get_debug_addr_data();
-  const lldb_private::DWARFDataExtractor &get_debug_frame_data();
   virtual const lldb_private::DWARFDataExtractor &get_debug_info_data();
-  const lldb_private::DWARFDataExtractor &get_debug_line_data();
-  const lldb_private::DWARFDataExtractor &get_debug_line_str_data();
-  const lldb_private::DWARFDataExtractor &get_debug_macro_data();
-  const lldb_private::DWARFDataExtractor &get_debug_loc_data();
-  const lldb_private::DWARFDataExtractor &get_debug_loclists_data();
-  const lldb_private::DWARFDataExtractor &get_debug_ranges_data();
-  const lldb_private::DWARFDataExtractor &get_debug_rnglists_data();
   virtual const lldb_private::DWARFDataExtractor &get_debug_str_data();
   virtual const lldb_private::DWARFDataExtractor &get_debug_str_offsets_data();
-  const lldb_private::DWARFDataExtractor &get_debug_types_data();
-  const lldb_private::DWARFDataExtractor &get_apple_names_data();
-  const lldb_private::DWARFDataExtractor &get_apple_types_data();
-  const lldb_private::DWARFDataExtractor &get_apple_namespaces_data();
-  const lldb_private::DWARFDataExtractor &get_apple_objc_data();
-  const lldb_private::DWARFDataExtractor &get_gnu_debugaltlink();
 
   DWARFDebugAbbrev *DebugAbbrev();
 
@@ -258,8 +244,6 @@
 
   const DWARFDebugRangesBase *DebugRanges() const;
 
-  const lldb_private::DWARFDataExtractor &DebugLocData();
-
   static bool SupportedVersion(uint16_t version);
 
   DWARFDIE
@@ -285,7 +269,7 @@
 uint32_t cu_idx);
 
   virtual lldb_private::DWARFExpression::LocationListFormat
-  GetLocationListFormat() const;
+  GetLocationListFormat();
 
   lldb::ModuleSP GetDWOModule(lldb_private::ConstString name);
 
@@ -302,6 +286,7 @@
   virtual std::unique_ptr
   GetDwoSymbolFileForCompileUnit(DWARFUnit &dwarf_cu,
  const DWARFDebugInfoEntry &cu_die);
+  lldb_private::DWARFContext *GetDWARFContext();
 
   // For regular SymbolFileDWARF instances the method returns nullptr,
   // for the instances of the subclass SymbolFi

[Lldb-commits] [PATCH] D59611: Move the rest of the section loading over to DWARFContext

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Use reference to DWARFContext instead of pointers? Apply to entire patch and 
good to go


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

https://reviews.llvm.org/D59611



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

[Lldb-commits] [lldb] r356625 - Update DWARF files.

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Wed Mar 20 15:57:33 2019
New Revision: 356625

URL: http://llvm.org/viewvc/llvm-project?rev=356625&view=rev
Log:
Update DWARF files.

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=356625&r1=356624&r2=356625&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Wed Mar 20 15:57:33 2019
@@ -251,6 +251,7 @@
4CD44D4220B777850003557C /* DWARFBaseDIE.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 4CD44D4020B777850003557C /* DWARFBaseDIE.cpp */; 
};
268900D713353E6F00698AC0 /* DWARFCallFrameInfo.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 26BC7F1710F1B8EC00F91463 /* 
DWARFCallFrameInfo.cpp */; };
268900B813353E5F00698AC0 /* DWARFCompileUnit.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 260C89B710F57C5600BB2B04 /* 
DWARFCompileUnit.cpp */; };
+   AF5B97DB2242FC2F002D3F2C /* DWARFContext.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF5B97D92242FC27002D3F2C /* DWARFContext.cpp */; 
};
266E82971B8CE3AC008FCA06 /* DWARFDIE.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 266E82961B8CE3AC008FCA06 /* DWARFDIE.cpp */; };
26AB92121819D74600E63F3E /* DWARFDataExtractor.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 26AB92101819D74600E63F3E /* 
DWARFDataExtractor.cpp */; };
268900B913353E5F00698AC0 /* DWARFDebugAbbrev.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 260C89B910F57C5600BB2B04 /* 
DWARFDebugAbbrev.cpp */; };
@@ -259,8 +260,6 @@
268900BC13353E5F00698AC0 /* DWARFDebugInfo.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 260C89BF10F57C5600BB2B04 /* DWARFDebugInfo.cpp 
*/; };
268900BD13353E5F00698AC0 /* DWARFDebugInfoEntry.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = 260C89C110F57C5600BB2B04 /* 
DWARFDebugInfoEntry.cpp */; };
268900BE13353E5F00698AC0 /* DWARFDebugLine.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 260C89C310F57C5600BB2B04 /* DWARFDebugLine.cpp 
*/; };
-   268900BF13353E5F00698AC0 /* DWARFDebugMacinfo.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 260C89C510F57C5600BB2B04 /* 
DWARFDebugMacinfo.cpp */; };
-   268900C013353E5F00698AC0 /* DWARFDebugMacinfoEntry.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 260C89C710F57C5600BB2B04 /* 
DWARFDebugMacinfoEntry.cpp */; };
23D4007D1C2101F2000C3885 /* DWARFDebugMacro.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 23E77CD61C20F29F007192AD /* DWARFDebugMacro.cpp 
*/; };
268900C313353E5F00698AC0 /* DWARFDebugRanges.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 260C89CD10F57C5600BB2B04 /* 
DWARFDebugRanges.cpp */; };
26B1EFAE154638AF00E2DAC7 /* DWARFDeclContext.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 26B1EFAC154638AF00E2DAC7 /* 
DWARFDeclContext.cpp */; };
@@ -1758,6 +1757,8 @@
26BC7C5910F1B6E900F91463 /* DWARFCallFrameInfo.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
DWARFCallFrameInfo.h; path = include/lldb/Symbol/DWARFCallFrameInfo.h; 
sourceTree = ""; };
260C89B710F57C5600BB2B04 /* DWARFCompileUnit.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = DWARFCompileUnit.cpp; sourceTree = ""; };
260C89B810F57C5600BB2B04 /* DWARFCompileUnit.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
DWARFCompileUnit.h; sourceTree = ""; };
+   AF5B97D92242FC27002D3F2C /* DWARFContext.cpp */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = 
DWARFContext.cpp; sourceTree = ""; };
+   AF5B97DA2242FC27002D3F2C /* DWARFContext.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DWARFContext.h; 
sourceTree = ""; };
266E82961B8CE3AC008FCA06 /* DWARFDIE.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = DWARFDIE.cpp; sourceTree = ""; };
266E82951B8CE346008FCA06 /* DWARFDIE.h */ = {isa = 
PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DWARFDIE.h; 
sourceTree = ""; };
26AB92101819D74600E63F3E /* DWARFDataExtractor.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
path = DWARFDataExtractor.cpp; sourceTree = ""; };
@@ -1774,10 +1775,6 @@
260C89C210F57C5600BB2B04 /* DWARFDebugInfoEntry.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = 
DWARFDebugInfoEntry.h; sourceTree = ""; };
260C89C

[Lldb-commits] [lldb] r356626 - Change the logging on ptrace(PT_KILL) in MachProcess::Kill to log

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Wed Mar 20 15:59:16 2019
New Revision: 356626

URL: http://llvm.org/viewvc/llvm-project?rev=356626&view=rev
Log:
Change the logging on ptrace(PT_KILL) in MachProcess::Kill to log
if LOG_PROCESS is enabled or if there was an error making that call.

 

Modified:
lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm

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=356626&r1=356625&r2=356626&view=diff
==
--- lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm (original)
+++ lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm Wed Mar 20 
15:59:16 2019
@@ -1290,9 +1290,11 @@ bool MachProcess::Kill(const struct time
   ::ptrace(PT_KILL, m_pid, 0, 0);
   DNBError err;
   err.SetErrorToErrno();
-  DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Kill() DoSIGSTOP() ::ptrace "
-"(PT_KILL, pid=%u, 0, 0) => 0x%8.8x (%s)",
-   m_pid, err.Status(), err.AsString());
+  if (DNBLogCheckLogBit(LOG_PROCESS) || err.Fail()) {
+err.LogThreaded("MachProcess::Kill() DoSIGSTOP() ::ptrace "
+"(PT_KILL, pid=%u, 0, 0) => 0x%8.8x (%s)",
+m_pid, err.Status(), err.AsString());
+  }
   m_thread_actions = DNBThreadResumeActions(eStateRunning, 0);
   PrivateResume();
 


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

[Lldb-commits] [lldb] r356638 - [Reproducers] Log inconsistencies during replay (NFC)

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Mar 20 18:57:33 2019
New Revision: 356638

URL: http://llvm.org/viewvc/llvm-project?rev=356638&view=rev
Log:
[Reproducers] Log inconsistencies during replay (NFC)

Make debugging of the GDB remote packet aspect of reproducers easier by
logging both requests and replies. This enables some sanity checking
during replay.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp?rev=356638&r1=356637&r2=356638&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp 
Wed Mar 20 18:57:33 2019
@@ -45,7 +45,7 @@ void GDBRemoteCommunicationHistory::AddP
   m_packets[idx].bytes_transmitted = bytes_transmitted;
   m_packets[idx].packet_idx = m_total_packet_count;
   m_packets[idx].tid = llvm::get_threadid();
-  if (m_stream && type == ePacketTypeRecv)
+  if (m_stream)
 m_packets[idx].Serialize(*m_stream);
 }
 
@@ -62,7 +62,7 @@ void GDBRemoteCommunicationHistory::AddP
   m_packets[idx].bytes_transmitted = bytes_transmitted;
   m_packets[idx].packet_idx = m_total_packet_count;
   m_packets[idx].tid = llvm::get_threadid();
-  if (m_stream && type == ePacketTypeRecv)
+  if (m_stream)
 m_packets[idx].Serialize(*m_stream);
 }
 

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp?rev=356638&r1=356637&r2=356638&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 Wed Mar 20 18:57:33 2019
@@ -30,6 +30,26 @@ using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
 
+static bool unexpected(llvm::StringRef expected, llvm::StringRef actual) {
+  // The 'expected' string contains the raw data, including the leading $ and
+  // trailing checksum. The 'actual' string contains only the packet's content.
+  if (expected.contains(actual))
+return false;
+  if (expected == "+" || actual == "+")
+return false;
+  // Contains a PID which might be different.
+  if (expected.contains("vAttach"))
+return false;
+  // Contains a ascii-hex-path.
+  if (expected.contains("QSetSTD"))
+return false;
+  // Contains environment values.
+  if (expected.contains("QEnvironment"))
+return false;
+
+  return true;
+}
+
 GDBRemoteCommunicationReplayServer::GDBRemoteCommunicationReplayServer()
 : GDBRemoteCommunication("gdb-remote.server",
  "gdb-remote.server.rx_packet"),
@@ -87,15 +107,31 @@ GDBRemoteCommunicationReplayServer::GetP
 m_send_acks = false;
   }
 
+  Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   while (!m_packet_history.empty()) {
 // Pop last packet from the history.
 GDBRemoteCommunicationHistory::Entry entry = m_packet_history.back();
 m_packet_history.pop_back();
 
-// We only care about what we received from the server. Skip everything
-// the client sent.
-if (entry.type != GDBRemoteCommunicationHistory::ePacketTypeRecv)
+if (entry.type == GDBRemoteCommunicationHistory::ePacketTypeSend) {
+  if (unexpected(entry.packet.data, packet.GetStringRef())) {
+LLDB_LOG(log,
+ "GDBRemoteCommunicationReplayServer expected packet: '{}'\n",
+ entry.packet.data);
+LLDB_LOG(log,
+ "GDBRemoteCommunicationReplayServer actual packet: '{}'\n",
+ packet.GetStringRef());
+  }
   continue;
+}
+
+if (entry.type == GDBRemoteCommunicationHistory::ePacketTypeInvalid) {
+  LLDB_LOG(
+  log,
+  "GDBRemoteCommunicationReplayServer skipped invalid packet: '{}'\n",
+  packet.GetStringRef());
+  continue;
+}
 
 return SendRawPacketNoLock(entry.packet.data, true);
   }


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

[Lldb-commits] [lldb] r356643 - [Reproducers] Properly handle QEnvironment packets

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Mar 20 21:08:31 2019
New Revision: 356643

URL: http://llvm.org/viewvc/llvm-project?rev=356643&view=rev
Log:
[Reproducers] Properly handle QEnvironment packets

On Linux, a QEnvironment packet is sent for every environment variable.
This breaks replay when the number of environment variables is different
then during capture. The solution is to always reply with OK.

Modified:
lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp

Modified: lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test?rev=356643&r1=356642&r2=356643&view=diff
==
--- lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test (original)
+++ lldb/trunk/lit/Reproducer/TestGDBRemoteRepro.test Wed Mar 20 21:08:31 2019
@@ -9,7 +9,7 @@
 # RUN: rm -rf %t.repro
 # RUN: %clang %S/Inputs/simple.c -g -o %t.out
 # RUN: %lldb -x -b -s %S/Inputs/GDBRemoteCapture.in --capture --capture-path 
%t.repro %t.out | FileCheck %s --check-prefix CHECK --check-prefix CAPTURE
-# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix CHECK 
--check-prefix REPLAY
+# RUN: env FOO=BAR %lldb --replay %t.repro | FileCheck %s --check-prefix CHECK 
--check-prefix REPLAY
 
 # CHECK: Breakpoint 1
 # CHECK: Process {{.*}} stopped

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp?rev=356643&r1=356642&r2=356643&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
 Wed Mar 20 21:08:31 2019
@@ -107,6 +107,13 @@ GDBRemoteCommunicationReplayServer::GetP
 m_send_acks = false;
   }
 
+  // A QEnvironment packet is sent for every environment variable. If the
+  // number of environment variables is different during replay, the replies
+  // become out of sync.
+  if (packet.GetStringRef().find("QEnvironment") == 0) {
+return SendRawPacketNoLock("$OK#9a", true);
+  }
+
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   while (!m_packet_history.empty()) {
 // Pop last packet from the history.
@@ -122,6 +129,14 @@ GDBRemoteCommunicationReplayServer::GetP
  "GDBRemoteCommunicationReplayServer actual packet: '{}'\n",
  packet.GetStringRef());
   }
+
+  // Ignore QEnvironment packets as they're handled earlier.
+  if (entry.packet.data.find("QEnvironment") == 1) {
+assert(m_packet_history.back().type ==
+   GDBRemoteCommunicationHistory::ePacketTypeRecv);
+m_packet_history.pop_back();
+  }
+
   continue;
 }
 


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

[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Robert O'Callahan via Phabricator via lldb-commits]

rocallahan added a comment.
Herald added a project: LLVM.

Ping?


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54747



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