[Lldb-commits] [lldb] 8ee2257 - [lldb/Test] Fix missing yaml2obj in Xcode standalone build.

2020-07-10 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-07-10T21:34:56-07:00
New Revision: 8ee225744f109b19e7d2412cbc50d4586991d8cf

URL: 
https://github.com/llvm/llvm-project/commit/8ee225744f109b19e7d2412cbc50d4586991d8cf
DIFF: 
https://github.com/llvm/llvm-project/commit/8ee225744f109b19e7d2412cbc50d4586991d8cf.diff

LOG: [lldb/Test] Fix missing yaml2obj in Xcode standalone build.

Rather than trying to find the yaml2obj from dotest we should pass it in
like we do for dsymutil and FileCheck.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/configuration.py
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/packages/Python/lldbsuite/test/dotest_args.py
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/CMakeLists.txt
lldb/test/API/lit.cfg.py
lldb/test/API/lit.site.cfg.py.in
lldb/utils/lldb-dotest/CMakeLists.txt
lldb/utils/lldb-dotest/lldb-dotest.in

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/configuration.py 
b/lldb/packages/Python/lldbsuite/test/configuration.py
index f05152253c75..ca2786446300 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -57,6 +57,9 @@
 # Path to the FileCheck testing tool. Not optional.
 filecheck = None
 
+# Path to the yaml2obj tool. Not optional.
+yaml2obj = None
+
 # The arch might dictate some specific CFLAGS to be passed to the toolchain to 
build
 # the inferior programs.  The global variable cflags_extras provides a hook to 
do
 # just that.
@@ -163,6 +166,13 @@ def get_filecheck_path():
 if filecheck and os.path.lexists(filecheck):
 return filecheck
 
+def get_yaml2obj_path():
+"""
+Get the path to the yaml2obj tool.
+"""
+if yaml2obj and os.path.lexists(yaml2obj):
+return yaml2obj
+
 def is_reproducer_replay():
 """
 Returns true when dotest is being replayed from a reproducer. Never use

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index f9975b27c475..8238168d0fb6 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -272,13 +272,17 @@ def parseOptionsAndInitTestdirs():
 configuration.dsymutil = seven.get_command_output(
 'xcrun -find -toolchain default dsymutil')
 
+
+# The lldb-dotest script produced by the CMake build passes in a path to a
+# working FileCheck and yaml2obj binary. So does one specific Xcode
+# project target. However, when invoking dotest.py directly, a valid
+# --filecheck and --yaml2obj option needs to be given.
 if args.filecheck:
-# The lldb-dotest script produced by the CMake build passes in a path
-# to a working FileCheck binary. So does one specific Xcode project
-# target. However, when invoking dotest.py directly, a valid 
--filecheck
-# option needs to be given.
 configuration.filecheck = os.path.abspath(args.filecheck)
 
+if args.yaml2obj:
+configuration.yaml2obj = os.path.abspath(args.yaml2obj)
+
 if not configuration.get_filecheck_path():
 logging.warning('No valid FileCheck executable; some tests may 
fail...')
 logging.warning('(Double-check the --filecheck argument to dotest.py)')

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py 
b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index ff2ac5a47ea5..d6f59efdf28b 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -51,7 +51,7 @@ def create_parser():
suggestions: do not 
lump the "-A arch1 -A arch2" together such that the -E option applies to only 
one of the architectures'''))
 
 group.add_argument('--dsymutil', metavar='dsymutil', dest='dsymutil', 
help=textwrap.dedent('Specify which dsymutil to use.'))
-
+group.add_argument('--yaml2obj', metavar='yaml2obj', dest='yaml2obj', 
help=textwrap.dedent('Specify which yaml2obj binary to use.'))
 group.add_argument('--filecheck', metavar='filecheck', dest='filecheck', 
help=textwrap.dedent('Specify which FileCheck binary to use.'))
 
 # Test filtering options

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index ddb79de0ab32..29561d4794be 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1633,20 +1633,6 @@ def findBuiltClang(self):
 
 return os.environ["CC"]
 
-def findYaml2obj(self):
-"""
-Get the path to the yaml2obj executable, which can be used to create
-test object files from easy to write yaml instructions.
-
-Throws an Exception if the executable cannot be found.
-"""
-# Tries to 

[Lldb-commits] [PATCH] D83600: Add a decorator to skip tests when running under Rosetta

2020-07-10 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

Good, one minor suggestion.




Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:560
+return False
+return platform.uname()[5] == "arm" and self.getArchitecture() == 
"x86_64"
+return skipTestIfFn(is_running_rosetta, bugnumber)(func)

There might be a more direct way of checking this, maybe `machine()`. We used 
`platform().uname()` for internal limitations & machine not always returning 
the right string, but it should be fixed on Beta 1 (and beyond) for sure


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

https://reviews.llvm.org/D83600



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


[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

2020-07-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

Hi, your git commit contains extra Phabricator tags. You can drop `Reviewers:` 
`Subscribers:` `Tags:` and the text `Summary:` from the git commit with the 
following script:

  arcfilter () {
  arc amend
  git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} 
/Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: 
/,"");print}' | git commit --amend --date=now -F -
  }

`Reviewed By: ` is considered important by some people. Please keep the tag. 
(`--date=now` is my personal preference (author dates are usually not useful. 
Using committer dates can make log almost monotonic in time))

`llvm/utils/git/pre-push.py` can validate the message does not include unneeded 
tags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477



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


[Lldb-commits] [PATCH] D83600: Add a decorator to skip tests when running under Rosetta

2020-07-10 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision.
aprantl added reviewers: davide, friss.

This allows skipping a test when running the testsuite on macOS under the 
Rosetta translation layer.


https://reviews.llvm.org/D83600

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -552,6 +552,14 @@
 return skipTestIfFn(are_sb_headers_missing)(func)
 
 
+def skipIfRosetta(func, bugnumber=None):
+"""Skip a test when running the testsuite on macOS under the Rosetta 
translation layer."""
+def is_running_rosetta(self):
+if not lldbplatformutil.getPlatform() in ['darwin', 'macosx']:
+return False
+return platform.uname()[5] == "arm" and self.getArchitecture() == 
"x86_64"
+return skipTestIfFn(is_running_rosetta, bugnumber)(func)
+
 def skipIfiOSSimulator(func):
 """Decorate the item to skip tests that should be skipped on the iOS 
Simulator."""
 def is_ios_simulator():


Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -552,6 +552,14 @@
 return skipTestIfFn(are_sb_headers_missing)(func)
 
 
+def skipIfRosetta(func, bugnumber=None):
+"""Skip a test when running the testsuite on macOS under the Rosetta translation layer."""
+def is_running_rosetta(self):
+if not lldbplatformutil.getPlatform() in ['darwin', 'macosx']:
+return False
+return platform.uname()[5] == "arm" and self.getArchitecture() == "x86_64"
+return skipTestIfFn(is_running_rosetta, bugnumber)(func)
+
 def skipIfiOSSimulator(func):
 """Decorate the item to skip tests that should be skipped on the iOS Simulator."""
 def is_ios_simulator():
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D83582: Fix nesting of #ifdef

2020-07-10 Thread Adrian Prantl via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG851cc2f8f60a: Fix nesting of #ifdef (authored by aprantl).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83582

Files:
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm


Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -780,11 +780,11 @@
 if (DeploymentInfo deployment_info = GetDeploymentInfo(lc, load_cmds_p)) {
   // Simulator support. If the platform is ambiguous, use the dyld info.
   if (deployment_info.maybe_simulator) {
-// If dyld doesn't return a platform, use a heuristic.
-#if (defined(__x86_64__) || defined(__i386__))
-// If we are running on Intel macOS, it is safe to assume
-// this is really a back-deploying simulator binary.
 if (deployment_info.maybe_simulator) {
+#if (defined(__x86_64__) || defined(__i386__))
+  // If dyld doesn't return a platform, use a heuristic.
+  // If we are running on Intel macOS, it is safe to assume
+  // this is really a back-deploying simulator binary.
   switch (deployment_info.platform) {
   case PLATFORM_IOS:
 deployment_info.platform = PLATFORM_IOSSIMULATOR;
@@ -797,12 +797,12 @@
 break;
   }
 #else
-// On an Apple Silicon macOS host, there is no
-// ambiguity. The only binaries that use legacy load
-// commands are back-deploying native iOS binaries. All
-// simulator binaries use the newer, unambiguous
-// LC_BUILD_VERSION load commands.
-deployment_info.maybe_simulator = false;
+  // On an Apple Silicon macOS host, there is no
+  // ambiguity. The only binaries that use legacy load
+  // commands are back-deploying native iOS binaries. All
+  // simulator binaries use the newer, unambiguous
+  // LC_BUILD_VERSION load commands.
+  deployment_info.maybe_simulator = false;
 #endif
 }
   }


Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -780,11 +780,11 @@
 if (DeploymentInfo deployment_info = GetDeploymentInfo(lc, load_cmds_p)) {
   // Simulator support. If the platform is ambiguous, use the dyld info.
   if (deployment_info.maybe_simulator) {
-// If dyld doesn't return a platform, use a heuristic.
-#if (defined(__x86_64__) || defined(__i386__))
-// If we are running on Intel macOS, it is safe to assume
-// this is really a back-deploying simulator binary.
 if (deployment_info.maybe_simulator) {
+#if (defined(__x86_64__) || defined(__i386__))
+  // If dyld doesn't return a platform, use a heuristic.
+  // If we are running on Intel macOS, it is safe to assume
+  // this is really a back-deploying simulator binary.
   switch (deployment_info.platform) {
   case PLATFORM_IOS:
 deployment_info.platform = PLATFORM_IOSSIMULATOR;
@@ -797,12 +797,12 @@
 break;
   }
 #else
-// On an Apple Silicon macOS host, there is no
-// ambiguity. The only binaries that use legacy load
-// commands are back-deploying native iOS binaries. All
-// simulator binaries use the newer, unambiguous
-// LC_BUILD_VERSION load commands.
-deployment_info.maybe_simulator = false;
+  // On an Apple Silicon macOS host, there is no
+  // ambiguity. The only binaries that use legacy load
+  // commands are back-deploying native iOS binaries. All
+  // simulator binaries use the newer, unambiguous
+  // LC_BUILD_VERSION load commands.
+  deployment_info.maybe_simulator = false;
 #endif
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 851cc2f - Fix nesting of #ifdef

2020-07-10 Thread Adrian Prantl via lldb-commits

Author: Adrian Prantl
Date: 2020-07-10T17:13:46-07:00
New Revision: 851cc2f8f60ad86a8af4f57d23df0c930efdc4da

URL: 
https://github.com/llvm/llvm-project/commit/851cc2f8f60ad86a8af4f57d23df0c930efdc4da
DIFF: 
https://github.com/llvm/llvm-project/commit/851cc2f8f60ad86a8af4f57d23df0c930efdc4da.diff

LOG: Fix nesting of #ifdef

This fixes a compile error when building for an arm64 host.

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

Added: 


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

Removed: 




diff  --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm 
b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 4bf5305cd67b..8a35f605daa3 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -780,11 +780,11 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary 
*options,
 if (DeploymentInfo deployment_info = GetDeploymentInfo(lc, load_cmds_p)) {
   // Simulator support. If the platform is ambiguous, use the dyld info.
   if (deployment_info.maybe_simulator) {
-// If dyld doesn't return a platform, use a heuristic.
-#if (defined(__x86_64__) || defined(__i386__))
-// If we are running on Intel macOS, it is safe to assume
-// this is really a back-deploying simulator binary.
 if (deployment_info.maybe_simulator) {
+#if (defined(__x86_64__) || defined(__i386__))
+  // If dyld doesn't return a platform, use a heuristic.
+  // If we are running on Intel macOS, it is safe to assume
+  // this is really a back-deploying simulator binary.
   switch (deployment_info.platform) {
   case PLATFORM_IOS:
 deployment_info.platform = PLATFORM_IOSSIMULATOR;
@@ -797,12 +797,12 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary 
*options,
 break;
   }
 #else
-// On an Apple Silicon macOS host, there is no
-// ambiguity. The only binaries that use legacy load
-// commands are back-deploying native iOS binaries. All
-// simulator binaries use the newer, unambiguous
-// LC_BUILD_VERSION load commands.
-deployment_info.maybe_simulator = false;
+  // On an Apple Silicon macOS host, there is no
+  // ambiguity. The only binaries that use legacy load
+  // commands are back-deploying native iOS binaries. All
+  // simulator binaries use the newer, unambiguous
+  // LC_BUILD_VERSION load commands.
+  deployment_info.maybe_simulator = false;
 #endif
 }
   }



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


[Lldb-commits] [lldb] c60216d - Revert "[lldb-vscode] Fix TestVSCode_module"

2020-07-10 Thread Walter Erquinigo via lldb-commits

Author: Walter Erquinigo
Date: 2020-07-10T17:07:07-07:00
New Revision: c60216db15132401ff60c08ccef899321f63b6b6

URL: 
https://github.com/llvm/llvm-project/commit/c60216db15132401ff60c08ccef899321f63b6b6
DIFF: 
https://github.com/llvm/llvm-project/commit/c60216db15132401ff60c08ccef899321f63b6b6.diff

LOG: Revert "[lldb-vscode] Fix TestVSCode_module"
This reverts commit 881af6eb0030986876d3b80668193e5c3c04a87c.

Revert "[lldb-vscode] Add Compile Unit List to Modules View"
This reverts commit 03ef61033ff5e1e40518f14f642e4ad8d686974c.

Revert "[lldb-vscode] Add Support for Module Event"
This reverts commit f7f80159753ba725f7e32529fcc369bc358efbb3.

The debian buildbot has reported issues with the modules test.
http://lab.llvm.org:8011/builders/lldb-x86_64-debian/builds/13767/steps/test/logs/stdio

Reverting it for now.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/JSONUtils.h
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 
lldb/test/API/tools/lldb-vscode/module/Makefile
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
lldb/test/API/tools/lldb-vscode/module/foo.cpp
lldb/test/API/tools/lldb-vscode/module/foo.h
lldb/test/API/tools/lldb-vscode/module/main.cpp



diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 6b1c1c961b54..1ad168e794cf 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -113,7 +113,6 @@ def __init__(self, recv, send, init_commands):
 self.initialize_body = None
 self.thread_stop_reasons = {}
 self.breakpoint_events = []
-self.module_events = {}
 self.sequence = 1
 self.threads = None
 self.recv_thread.start()
@@ -134,9 +133,6 @@ def validate_response(cls, command, response):
 if command['seq'] != response['request_seq']:
 raise ValueError('seq mismatch in response')
 
-def get_active_modules(self):
-return self.module_events
-
 def get_output(self, category, timeout=0.0, clear=True):
 self.output_condition.acquire()
 output = None
@@ -222,15 +218,6 @@ def handle_recv_packet(self, packet):
 self.breakpoint_events.append(packet)
 # no need to add 'breakpoint' event packets to our packets list
 return keepGoing
-elif event == 'module':
-reason = body['reason']
-if (reason == 'new' or reason == 'changed'):
-self.module_events[body['module']['name']] = body['module']
-elif reason == 'removed':
-if body['module']['name'] in self.module_events:
-self.module_events.pop(body['module']['name'])
-return keepGoing
-
 elif packet_type == 'response':
 if packet['command'] == 'disconnect':
 keepGoing = False
@@ -760,16 +747,6 @@ def request_setFunctionBreakpoints(self, names, 
condition=None,
 }
 return self.send_recv(command_dict)
 
-def request_getCompileUnits(self, moduleId):
-args_dict = {'moduleId': moduleId}
-command_dict = {
-'command': 'getCompileUnits',
-'type': 'request',
-'arguments': args_dict
-}
-response = self.send_recv(command_dict)
-return response
-
 def request_completions(self, text):
 args_dict = {
 'text': text,

diff  --git a/lldb/test/API/tools/lldb-vscode/module/Makefile 
b/lldb/test/API/tools/lldb-vscode/module/Makefile
deleted file mode 100644
index 1fb944b13893..
--- a/lldb/test/API/tools/lldb-vscode/module/Makefile
+++ /dev/null
@@ -1,13 +0,0 @@
-DYLIB_NAME := foo
-DYLIB_CXX_SOURCES := foo.cpp
-CXX_SOURCES := main.cpp
-
-all: a.out.stripped
-
-include Makefile.rules
-
-a.out.stripped: a.out.dSYM
-   strip -o a.out.stripped a.out
-ifneq "$(CODESIGN)" ""
-   $(CODESIGN) -fs - a.out.stripped
-endif

diff  --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py 
b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
deleted file mode 100644
index 1a5cadb22bc6..
--- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ /dev/null
@@ -1,65 +0,0 @@
-"""
-Test lldb-vscode setBreakpoints request
-"""
-
-from __future__ import print_function
-
-import unittest2
-import vscode
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-import lldbvscode_testcase
-
-
-class TestVSCode_module(lldbvscode_testcase.VSCodeTestCaseBase):
-
-mydir = 

[Lldb-commits] [PATCH] D82477: [lldb-vscode] Add Support for Module Event

2020-07-10 Thread Yifan Shen via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf7f80159753b: [lldb-vscode] Add Support for Module Event 
(authored by aelitashen, committed by Walter Erquinigo 
walterme...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/module/Makefile
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/test/API/tools/lldb-vscode/module/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  lldb/tools/lldb-vscode/VSCode.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
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Option/Arg.h"
@@ -434,6 +435,30 @@
 g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
   }
 }
+  } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
+if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
+event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
+event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
+  int num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
+  for (int i = 0; i < num_modules; i++) {
+auto module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
+auto module_event = CreateEventObject("module");
+llvm::json::Value module_value = CreateModule(module);
+llvm::json::Object body;
+if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) {
+  body.try_emplace("reason", "new");
+} else if (event_mask &
+lldb::SBTarget::eBroadcastBitModulesUnloaded) {
+  body.try_emplace("reason", "removed");
+} else if (event_mask &
+lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
+  body.try_emplace("reason", "changed");
+}
+body.try_emplace("module", module_value);
+module_event.try_emplace("body", std::move(body));
+g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
+  }
+}
   } else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
 if (event_mask & eBroadcastBitStopEventThread) {
   done = true;
Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -358,6 +358,11 @@
 lldb::SBTarget::eBroadcastBitBreakpointChanged);
 listener.StartListeningForEvents(this->broadcaster,
  eBroadcastBitStopEventThread);
+listener.StartListeningForEvents(
+  this->target.GetBroadcaster(),
+  lldb::SBTarget::eBroadcastBitModulesLoaded |
+  lldb::SBTarget::eBroadcastBitModulesUnloaded |
+  lldb::SBTarget::eBroadcastBitSymbolsLoaded);
   }
 }
 
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -13,6 +13,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include "VSCodeForward.h"
+#include "lldb/API/SBModule.h"
 
 namespace lldb_vscode {
 
@@ -237,6 +238,16 @@
  llvm::Optional request_path = llvm::None,
  llvm::Optional request_line = llvm::None);
 
+/// Converts a LLDB module to a VS Code DAP module for use in "modules" events.
+///
+/// \param[in] module
+/// A LLDB module object to convert into a JSON value
+///
+/// \return
+/// A "Module" JSON object with that follows the formal JSON
+/// definition outlined by Microsoft.
+llvm::json::Value CreateModule(lldb::SBModule );
+
 /// Create a "Event" JSON object using \a event_name as the event name
 ///
 /// \param[in] event_name
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -327,6 +327,41 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateModule(lldb::SBModule ) {
+  llvm::json::Object object;
+  if (!module.IsValid())
+return llvm::json::Value(std::move(object));
+  object.try_emplace("id", std::string(module.GetUUIDString()));
+  object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
+  char module_path_arr[PATH_MAX];
+  module.GetFileSpec().GetPath(module_path_arr, sizeof(module_path_arr));
+  

[Lldb-commits] [PATCH] D83072: [lldb-vscode] Add Compile Unit List to Modules View

2020-07-10 Thread Yifan Shen via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG03ef61033ff5: [lldb-vscode] Add Compile Unit List to Modules 
View (authored by aelitashen, committed by Walter Erquinigo 
walterme...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83072

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/JSONUtils.h
  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
@@ -1174,6 +1174,72 @@
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+// "getCompileUnitsRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Compile Unit request; value of command field is
+// 'getCompileUnits'.",
+// "properties": {
+//   "command": {
+// "type": "string",
+// "enum": [ "getCompileUnits" ]
+//   },
+//   "arguments": {
+// "$ref": "#/definitions/getCompileUnitRequestArguments"
+//   }
+// },
+// "required": [ "command", "arguments" ]
+//   }]
+// },
+// "getCompileUnitsRequestArguments": {
+//   "type": "object",
+//   "description": "Arguments for 'getCompileUnits' request.",
+//   "properties": {
+// "moduleId": {
+//   "type": "string",
+//   "description": "The ID of the module."
+// }
+//   },
+//   "required": [ "moduleId" ]
+// },
+// "getCompileUnitsResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to 'getCompileUnits' request.",
+// "properties": {
+//   "body": {
+// "description": "Response to 'getCompileUnits' request. Array of
+// paths of compile units."
+//   }
+// }
+//   }]
+// }
+
+void request_getCompileUnits(const llvm::json::Object ) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  lldb::SBProcess process = g_vsc.target.GetProcess();
+  llvm::json::Object body;
+  llvm::json::Array units;
+  auto arguments = request.getObject("arguments");
+  std::string module_id = std::string(GetString(arguments, "moduleId"));
+  int num_modules = g_vsc.target.GetNumModules();
+  for (int i = 0; i < num_modules; i++) {
+auto curr_module = g_vsc.target.GetModuleAtIndex(i);
+if (module_id == curr_module.GetUUIDString()) {
+  int num_units = curr_module.GetNumCompileUnits();
+  for (int j = 0; j < num_units; j++) {
+auto curr_unit = curr_module.GetCompileUnitAtIndex(j);\
+units.emplace_back(CreateCompileUnit(curr_unit));\
+  }
+  body.try_emplace("compileUnits", std::move(units));
+  break;
+}
+  }
+  response.try_emplace("body", std::move(body));
+  g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+}
+
 // "InitializeRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 // "type": "object",
@@ -2759,6 +2825,7 @@
   REQUEST_CALLBACK(disconnect),
   REQUEST_CALLBACK(evaluate),
   REQUEST_CALLBACK(exceptionInfo),
+  REQUEST_CALLBACK(getCompileUnits),
   REQUEST_CALLBACK(initialize),
   REQUEST_CALLBACK(launch),
   REQUEST_CALLBACK(next),
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -441,6 +441,8 @@
 llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
  int64_t varID, bool format_hex);
 
+llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit);
+
 } // namespace lldb_vscode
 
 #endif
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -937,4 +937,13 @@
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit) {
+  llvm::json::Object object;
+  char unit_path_arr[PATH_MAX];
+  unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr));
+  std::string unit_path(unit_path_arr);
+  object.try_emplace("compileUnitPath", unit_path);
+  return llvm::json::Value(std::move(object));
+}
+
 } // namespace lldb_vscode
Index: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
===
--- lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -46,4 +46,22 @@
 self.assertIn('symbolFilePath', program_module)
 

[Lldb-commits] [lldb] f7f8015 - [lldb-vscode] Add Support for Module Event

2020-07-10 Thread Walter Erquinigo via lldb-commits

Author: Yifan Shen
Date: 2020-07-10T16:50:59-07:00
New Revision: f7f80159753ba725f7e32529fcc369bc358efbb3

URL: 
https://github.com/llvm/llvm-project/commit/f7f80159753ba725f7e32529fcc369bc358efbb3
DIFF: 
https://github.com/llvm/llvm-project/commit/f7f80159753ba725f7e32529fcc369bc358efbb3.diff

LOG: [lldb-vscode] Add Support for Module Event

Summary:
Whenever a module is created, removed or changed, lldb-vscode is now sending an 
event that can be interpreted by the IDE so that modules can be rendered in the 
IDE, like the tree view in this screenshot

{F12229758}

Reviewers: wallace, clayborg, kusmour, aadsm

Reviewed By: clayborg

Subscribers: cfe-commits, labath, lldb-commits

Tags: #lldb, #clang

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

Added: 
lldb/test/API/tools/lldb-vscode/module/Makefile
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
lldb/test/API/tools/lldb-vscode/module/main.cpp

Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/JSONUtils.h
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 1ad168e794cf..93a44ccce215 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -113,6 +113,7 @@ def __init__(self, recv, send, init_commands):
 self.initialize_body = None
 self.thread_stop_reasons = {}
 self.breakpoint_events = []
+self.module_events = {}
 self.sequence = 1
 self.threads = None
 self.recv_thread.start()
@@ -133,6 +134,9 @@ def validate_response(cls, command, response):
 if command['seq'] != response['request_seq']:
 raise ValueError('seq mismatch in response')
 
+def get_active_modules(self):
+return self.module_events
+
 def get_output(self, category, timeout=0.0, clear=True):
 self.output_condition.acquire()
 output = None
@@ -218,6 +222,15 @@ def handle_recv_packet(self, packet):
 self.breakpoint_events.append(packet)
 # no need to add 'breakpoint' event packets to our packets list
 return keepGoing
+elif event == 'module':
+reason = body['reason']
+if (reason == 'new' or reason == 'changed'):
+self.module_events[body['module']['name']] = body['module']
+elif reason == 'removed':
+if body['module']['name'] in self.module_events:
+self.module_events.pop(body['module']['name'])
+return keepGoing
+
 elif packet_type == 'response':
 if packet['command'] == 'disconnect':
 keepGoing = False

diff  --git a/lldb/test/API/tools/lldb-vscode/module/Makefile 
b/lldb/test/API/tools/lldb-vscode/module/Makefile
new file mode 100644
index ..04c9fda1c388
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/module/Makefile
@@ -0,0 +1,11 @@
+CXX_SOURCES := main.cpp
+
+all:a.out.stripped
+
+include Makefile.rules
+
+a.out.stripped: a.out.dSYM
+   strip -o a.out.stripped a.out
+ifneq "$(CODESIGN)" ""
+   $(CODESIGN) -fs - a.out.stripped
+endif
\ No newline at end of file

diff  --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py 
b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
new file mode 100644
index ..480826e46ef9
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -0,0 +1,49 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_module(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_modules_event(self):
+program_basename = "a.out.stripped"
+program= self.getBuildArtifact(program_basename)
+self.build_and_launch(program)
+source = "main.cpp"
+main_source_path = self.getSourcePath(source)
+functions = ['main']
+breakpoint_ids = self.set_function_breakpoints(functions)
+self.assertEquals(len(breakpoint_ids), len(functions),
+'expect one breakpoint')
+self.continue_to_breakpoints(breakpoint_ids)
+active_modules = self.vscode.get_active_modules()
+self.assertIn(program_basename, active_modules, '%s module is in 
active modules' % (program_basename))
+

[Lldb-commits] [lldb] 881af6e - [lldb-vscode] Fix TestVSCode_module

2020-07-10 Thread Walter Erquinigo via lldb-commits

Author: Walter Erquinigo
Date: 2020-07-10T16:50:59-07:00
New Revision: 881af6eb0030986876d3b80668193e5c3c04a87c

URL: 
https://github.com/llvm/llvm-project/commit/881af6eb0030986876d3b80668193e5c3c04a87c
DIFF: 
https://github.com/llvm/llvm-project/commit/881af6eb0030986876d3b80668193e5c3c04a87c.diff

LOG: [lldb-vscode] Fix TestVSCode_module

For some reason this works on the original author's machine, but not on my. So 
I'm using a safer approach of using an unstripped dynamic library to place 
breakpoints on. The author was placing a breakpoint on the main symbol of a 
stripped library and for some reason it worked on their machine, but it 
shouldn't have...

Offender diff: D82477

Added: 
lldb/test/API/tools/lldb-vscode/module/foo.cpp
lldb/test/API/tools/lldb-vscode/module/foo.h

Modified: 
lldb/test/API/tools/lldb-vscode/module/Makefile
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
lldb/test/API/tools/lldb-vscode/module/main.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/module/Makefile 
b/lldb/test/API/tools/lldb-vscode/module/Makefile
index 04c9fda1c388..1fb944b13893 100644
--- a/lldb/test/API/tools/lldb-vscode/module/Makefile
+++ b/lldb/test/API/tools/lldb-vscode/module/Makefile
@@ -1,6 +1,8 @@
+DYLIB_NAME := foo
+DYLIB_CXX_SOURCES := foo.cpp
 CXX_SOURCES := main.cpp
 
-all:a.out.stripped
+all: a.out.stripped
 
 include Makefile.rules
 
@@ -8,4 +10,4 @@ a.out.stripped: a.out.dSYM
strip -o a.out.stripped a.out
 ifneq "$(CODESIGN)" ""
$(CODESIGN) -fs - a.out.stripped
-endif
\ No newline at end of file
+endif

diff  --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py 
b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
index 1de461ef9efa..1a5cadb22bc6 100644
--- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -20,9 +20,7 @@ def test_modules_event(self):
 program_basename = "a.out.stripped"
 program= self.getBuildArtifact(program_basename)
 self.build_and_launch(program)
-source = "main.cpp"
-main_source_path = self.getSourcePath(source)
-functions = ['main']
+functions = ['foo']
 breakpoint_ids = self.set_function_breakpoints(functions)
 self.assertEquals(len(breakpoint_ids), len(functions),
 'expect one breakpoint')
@@ -37,7 +35,7 @@ def test_modules_event(self):
 self.assertTrue('symbolFilePath' not in program_module, 'Make sure 
a.out.stripped has no debug info')
 self.assertEqual('Symbols not found.', program_module['symbolStatus'])
 symbol_path = self.getBuildArtifact("a.out")
-response = self.vscode.request_evaluate('`%s' % ('target symbols add 
-s "%s" "%s"' % (program, symbol_path)))
+self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" 
"%s"' % (program, symbol_path)))
 active_modules = self.vscode.get_active_modules()
 program_module = active_modules[program_basename]
 self.assertEqual(program_basename, program_module['name'])

diff  --git a/lldb/test/API/tools/lldb-vscode/module/foo.cpp 
b/lldb/test/API/tools/lldb-vscode/module/foo.cpp
new file mode 100644
index ..9dba85a9ccca
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/module/foo.cpp
@@ -0,0 +1,3 @@
+int foo() {
+return 12;
+}

diff  --git a/lldb/test/API/tools/lldb-vscode/module/foo.h 
b/lldb/test/API/tools/lldb-vscode/module/foo.h
new file mode 100644
index ..5d5f8f0c9e78
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/module/foo.h
@@ -0,0 +1 @@
+int foo();

diff  --git a/lldb/test/API/tools/lldb-vscode/module/main.cpp 
b/lldb/test/API/tools/lldb-vscode/module/main.cpp
index 62bcb78696b0..4ff2b2360eb9 100644
--- a/lldb/test/API/tools/lldb-vscode/module/main.cpp
+++ b/lldb/test/API/tools/lldb-vscode/module/main.cpp
@@ -1,3 +1,6 @@
+#include "foo.h"
+
 int main(int argc, char const *argv[]) {
+  foo();
   return 0; // breakpoint 1
 }



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


[Lldb-commits] [lldb] 03ef610 - [lldb-vscode] Add Compile Unit List to Modules View

2020-07-10 Thread Walter Erquinigo via lldb-commits

Author: Yifan Shen
Date: 2020-07-10T16:50:59-07:00
New Revision: 03ef61033ff5e1e40518f14f642e4ad8d686974c

URL: 
https://github.com/llvm/llvm-project/commit/03ef61033ff5e1e40518f14f642e4ad8d686974c
DIFF: 
https://github.com/llvm/llvm-project/commit/03ef61033ff5e1e40518f14f642e4ad8d686974c.diff

LOG: [lldb-vscode] Add Compile Unit List to Modules View

Summary: User can expand and check compile unit list for the modules that have 
debug info.

Reviewers: wallace, clayborg

Reviewed By: clayborg

Subscribers: aprantl, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/JSONUtils.h
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 93a44ccce215..6b1c1c961b54 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -760,6 +760,16 @@ def request_setFunctionBreakpoints(self, names, 
condition=None,
 }
 return self.send_recv(command_dict)
 
+def request_getCompileUnits(self, moduleId):
+args_dict = {'moduleId': moduleId}
+command_dict = {
+'command': 'getCompileUnits',
+'type': 'request',
+'arguments': args_dict
+}
+response = self.send_recv(command_dict)
+return response
+
 def request_completions(self, text):
 args_dict = {
 'text': text,

diff  --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py 
b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
index 480826e46ef9..1de461ef9efa 100644
--- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -46,4 +46,22 @@ def test_modules_event(self):
 self.assertIn('symbolFilePath', program_module)
 self.assertEqual(symbol_path, program_module['symbolFilePath'])
 self.assertIn('addressRange', program_module)
-
\ No newline at end of file
+
+def test_compile_units(self):
+program= self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = "main.cpp"
+main_source_path = self.getSourcePath(source)
+breakpoint1_line = line_number(source, '// breakpoint 1')
+lines = [breakpoint1_line]
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.continue_to_breakpoints(breakpoint_ids)
+moduleId = self.vscode.get_active_modules()['a.out']['id']
+response = self.vscode.request_getCompileUnits(moduleId)
+print(response['body'])
+self.assertTrue(response['body'])
+self.assertTrue(len(response['body']['compileUnits']) == 1,
+'Only one source file should exist')
+self.assertTrue(response['body']['compileUnits'][0]['compileUnitPath'] 
== main_source_path, 
+'Real path to main.cpp matches')
+ 
\ No newline at end of file

diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 1bb9480c510a..86c29fb23811 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -937,4 +937,13 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t 
variablesReference,
   return llvm::json::Value(std::move(object));
 }
 
+llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit) {
+  llvm::json::Object object;
+  char unit_path_arr[PATH_MAX];
+  unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr));
+  std::string unit_path(unit_path_arr);
+  object.try_emplace("compileUnitPath", unit_path);
+  return llvm::json::Value(std::move(object));
+}
+
 } // namespace lldb_vscode

diff  --git a/lldb/tools/lldb-vscode/JSONUtils.h 
b/lldb/tools/lldb-vscode/JSONUtils.h
index 71aa87e3d523..e2ccfdb1fb2b 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.h
+++ b/lldb/tools/lldb-vscode/JSONUtils.h
@@ -441,6 +441,8 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread 
, uint32_t stop_id);
 llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
  int64_t varID, bool format_hex);
 
+llvm::json::Value CreateCompileUnit(lldb::SBCompileUnit unit);
+
 } // namespace lldb_vscode
 
 #endif

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index cdaaa896d4fe..27ee832677d7 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1174,6 +1174,72 @@ void 

[Lldb-commits] [PATCH] D79147: Switch to using -debug-info-kind=constructor as default (from =limited)

2020-07-10 Thread Amy Huang via Phabricator via lldb-commits
akhuang updated this revision to Diff 276778.
akhuang added a comment.

fix one more test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79147

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang-g-opts.c
  clang/test/Driver/cuda-dwarf-2.cu
  clang/test/Driver/debug-options-as.c
  clang/test/Driver/debug-options.c
  clang/test/Driver/integrated-as.s
  clang/test/Driver/myriad-toolchain.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/split-debug.c
  lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp

Index: lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
===
--- lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
+++ lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
@@ -106,6 +106,7 @@
 int main() {
   MemberTest::Base B1;
   B1.Get();
+  MemberTest::Class C1;
   MemberTest::Class::StaticMemberFunc(1, 10, 2);
   return 0;
 }
Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -68,18 +68,18 @@
 // RUN: FileCheck -check-prefix=CHECK-NOINLINE-WITHOUT-SPLIT < %t %s
 //
 // CHECK-NOINLINE-WITHOUT-SPLIT: "-fno-split-dwarf-inlining"
-// CHECK-NOINLINE-WITHOUT-SPLIT: "-debug-info-kind=limited"
+// CHECK-NOINLINE-WITHOUT-SPLIT: "-debug-info-kind=constructor"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gmlt -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-GMLT < %t %s
 //
-// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=limited"
+// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=constructor"
 // CHECK-SPLIT-WITH-GMLT: "-split-dwarf-output"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-NOINL < %t %s
 //
-// CHECK-SPLIT-WITH-NOINL: "-debug-info-kind=limited"
+// CHECK-SPLIT-WITH-NOINL: "-debug-info-kind=constructor"
 // CHECK-SPLIT-WITH-NOINL: "-split-dwarf-output"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -gmlt -fsplit-dwarf-inlining -S -### %s 2> %t
@@ -92,7 +92,7 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -gmlt -gsplit-dwarf -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-GMLT < %t %s
 //
-// CHECK-SPLIT-OVER-GMLT: "-debug-info-kind=limited"
+// CHECK-SPLIT-OVER-GMLT: "-debug-info-kind=constructor"
 // CHECK-SPLIT-OVER-GMLT: "-split-dwarf-file"
 // CHECK-SPLIT-OVER-GMLT: "-split-dwarf-output"
 
@@ -117,6 +117,6 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf=split -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
 //
-// CHECK-SPLIT-OVER-G0: "-debug-info-kind=limited"
+// CHECK-SPLIT-OVER-G0: "-debug-info-kind=constructor"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-file"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-output"
Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -241,7 +241,7 @@
 
 // HAS_DEBUG-NOT: warning: debug
 // HAS_DEBUG: "-triple" "nvptx64-nvidia-cuda"
-// HAS_DEBUG-SAME: "-debug-info-kind={{limited|line-tables-only}}"
+// HAS_DEBUG-SAME: "-debug-info-kind={{constructor|line-tables-only}}"
 // HAS_DEBUG-SAME: "-dwarf-version=2"
 // HAS_DEBUG-SAME: "-fopenmp-is-device"
 // HAS_DEBUG: ptxas
Index: clang/test/Driver/myriad-toolchain.c
===
--- clang/test/Driver/myriad-toolchain.c
+++ clang/test/Driver/myriad-toolchain.c
@@ -83,7 +83,7 @@
 // NOSTDLIB-NOT: "-lc"
 
 // RUN: %clang -### -c -g %s -target sparc-myriad 2>&1 | FileCheck -check-prefix=G_SPARC %s
-// G_SPARC: "-debug-info-kind=limited" "-dwarf-version=2"
+// G_SPARC: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -c %s -target sparc-myriad-rtems -fuse-init-array 2>&1 \
 // RUN: | FileCheck -check-prefix=USE-INIT-ARRAY %s
Index: clang/test/Driver/integrated-as.s
===
--- clang/test/Driver/integrated-as.s
+++ clang/test/Driver/integrated-as.s
@@ -27,19 +27,19 @@
 // XA_INCLUDE2: "-Ifoo_dir"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-4 -gdwarf-2 2>&1 | FileCheck --check-prefix=DWARF2 %s
-// DWARF2: "-debug-info-kind=limited" "-dwarf-version=2"
+// DWARF2: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-3 2>&1 | FileCheck --check-prefix=DWARF3 %s
-// DWARF3: "-debug-info-kind=limited" "-dwarf-version=3"
+// DWARF3: "-debug-info-kind=constructor" "-dwarf-version=3"
 
 // RUN: %clang -### -target x86_64--- -c 

[Lldb-commits] [PATCH] D79147: Switch to using -debug-info-kind=constructor as default (from =limited)

2020-07-10 Thread Amy Huang via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG227db86a1b7d: Switch to using -debug-info-kind=constructor 
as default (from =limited) (authored by akhuang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79147

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/clang-g-opts.c
  clang/test/Driver/cuda-dwarf-2.cu
  clang/test/Driver/debug-options-as.c
  clang/test/Driver/debug-options.c
  clang/test/Driver/integrated-as.s
  clang/test/Driver/myriad-toolchain.c
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/split-debug.c
  lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp

Index: lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
===
--- lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
+++ lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp
@@ -106,6 +106,7 @@
 int main() {
   MemberTest::Base B1;
   B1.Get();
+  MemberTest::Class C1;
   MemberTest::Class::StaticMemberFunc(1, 10, 2);
   return 0;
 }
Index: clang/test/Driver/split-debug.c
===
--- clang/test/Driver/split-debug.c
+++ clang/test/Driver/split-debug.c
@@ -68,18 +68,18 @@
 // RUN: FileCheck -check-prefix=CHECK-NOINLINE-WITHOUT-SPLIT < %t %s
 //
 // CHECK-NOINLINE-WITHOUT-SPLIT: "-fno-split-dwarf-inlining"
-// CHECK-NOINLINE-WITHOUT-SPLIT: "-debug-info-kind=limited"
+// CHECK-NOINLINE-WITHOUT-SPLIT: "-debug-info-kind=constructor"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gmlt -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-GMLT < %t %s
 //
-// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=limited"
+// CHECK-SPLIT-WITH-GMLT: "-debug-info-kind=constructor"
 // CHECK-SPLIT-WITH-GMLT: "-split-dwarf-output"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -fno-split-dwarf-inlining -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-WITH-NOINL < %t %s
 //
-// CHECK-SPLIT-WITH-NOINL: "-debug-info-kind=limited"
+// CHECK-SPLIT-WITH-NOINL: "-debug-info-kind=constructor"
 // CHECK-SPLIT-WITH-NOINL: "-split-dwarf-output"
 
 // RUN: %clang -target x86_64-unknown-linux-gnu -gsplit-dwarf -gmlt -fsplit-dwarf-inlining -S -### %s 2> %t
@@ -92,7 +92,7 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -gmlt -gsplit-dwarf -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-GMLT < %t %s
 //
-// CHECK-SPLIT-OVER-GMLT: "-debug-info-kind=limited"
+// CHECK-SPLIT-OVER-GMLT: "-debug-info-kind=constructor"
 // CHECK-SPLIT-OVER-GMLT: "-split-dwarf-file"
 // CHECK-SPLIT-OVER-GMLT: "-split-dwarf-output"
 
@@ -117,6 +117,6 @@
 // RUN: %clang -target x86_64-unknown-linux-gnu -g0 -gsplit-dwarf=split -S -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-SPLIT-OVER-G0 < %t %s
 //
-// CHECK-SPLIT-OVER-G0: "-debug-info-kind=limited"
+// CHECK-SPLIT-OVER-G0: "-debug-info-kind=constructor"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-file"
 // CHECK-SPLIT-OVER-G0: "-split-dwarf-output"
Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -241,7 +241,7 @@
 
 // HAS_DEBUG-NOT: warning: debug
 // HAS_DEBUG: "-triple" "nvptx64-nvidia-cuda"
-// HAS_DEBUG-SAME: "-debug-info-kind={{limited|line-tables-only}}"
+// HAS_DEBUG-SAME: "-debug-info-kind={{constructor|line-tables-only}}"
 // HAS_DEBUG-SAME: "-dwarf-version=2"
 // HAS_DEBUG-SAME: "-fopenmp-is-device"
 // HAS_DEBUG: ptxas
Index: clang/test/Driver/myriad-toolchain.c
===
--- clang/test/Driver/myriad-toolchain.c
+++ clang/test/Driver/myriad-toolchain.c
@@ -83,7 +83,7 @@
 // NOSTDLIB-NOT: "-lc"
 
 // RUN: %clang -### -c -g %s -target sparc-myriad 2>&1 | FileCheck -check-prefix=G_SPARC %s
-// G_SPARC: "-debug-info-kind=limited" "-dwarf-version=2"
+// G_SPARC: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -c %s -target sparc-myriad-rtems -fuse-init-array 2>&1 \
 // RUN: | FileCheck -check-prefix=USE-INIT-ARRAY %s
Index: clang/test/Driver/integrated-as.s
===
--- clang/test/Driver/integrated-as.s
+++ clang/test/Driver/integrated-as.s
@@ -27,19 +27,19 @@
 // XA_INCLUDE2: "-Ifoo_dir"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-4 -gdwarf-2 2>&1 | FileCheck --check-prefix=DWARF2 %s
-// DWARF2: "-debug-info-kind=limited" "-dwarf-version=2"
+// DWARF2: "-debug-info-kind=constructor" "-dwarf-version=2"
 
 // RUN: %clang -### -target x86_64--- -c -integrated-as %s -gdwarf-3 2>&1 | FileCheck --check-prefix=DWARF3 %s
-// DWARF3: "-debug-info-kind=limited" "-dwarf-version=3"
+// 

[Lldb-commits] [PATCH] D79147: Switch to using -debug-info-kind=constructor as default (from =limited)

2020-07-10 Thread Fangrui Song via Phabricator via lldb-commits
MaskRay added a comment.

Hi, your git commit contains extra Phabricator tags. You can drop `Reviewers:` 
`Subscribers:` `Tags:` and the text `Summary:` from the git commit with the 
following script:

  arcfilter () {
  arc amend
  git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} 
/Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ {sub(/^Summary: 
/,"");print}' | git commit --amend --date=now -F -
  }

`Reviewed By: ` is considered important by some people. Please keep the tag. 
(`--date=now` is my personal preference (author dates are usually not useful. 
Using committer dates can make log almost monotonic in time))

`llvm/utils/git/pre-push.py` can validate the message does not include unneeded 
tags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79147



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


[Lldb-commits] [PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

2020-07-10 Thread Michele Scandale via Phabricator via lldb-commits
michele.scandale added a comment.

In D83454#2144386 , @JDevlieghere 
wrote:

> In D83454#2142719 , 
> @michele.scandale wrote:
>
> > I tested locally the standalone build of Clang with D83426 
> > .
> >  I just want to make sure that we all agree this is right way moving 
> > forward.
>
>
> Yep, with the target exported this is the right thing to do.


Great.

I don't have commit rights. Could someone land this on my behalf?

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83454



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


[Lldb-commits] [lldb] 169c832 - [ldb/Reproducers] Add YamlRecorder and MultiProvider

2020-07-10 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-07-10T12:48:22-07:00
New Revision: 169c83208f37f8e5539b1386030d9f3e4b15f16a

URL: 
https://github.com/llvm/llvm-project/commit/169c83208f37f8e5539b1386030d9f3e4b15f16a
DIFF: 
https://github.com/llvm/llvm-project/commit/169c83208f37f8e5539b1386030d9f3e4b15f16a.diff

LOG: [ldb/Reproducers] Add YamlRecorder and MultiProvider

This patch does several things that are all closely related:

 - It introduces a new YamlRecorder as a counterpart to the existing
   DataRecorder. As the name suggests the former serializes data as yaml
   while the latter uses raw texts or bytes.

 - It introduces a new MultiProvider base class which can be backed by
   either a DataRecorder or a YamlRecorder.

 - It reimplements the CommandProvider in terms of the new
   MultiProvider.

Finally, it adds unit testing coverage for the MultiProvider, a naive
YamlProvider built on top of the new YamlRecorder and the existing
MutliLoader.

Differential revision: https://reviews.llvm.org/D83441

Added: 


Modified: 
lldb/include/lldb/Utility/Reproducer.h
lldb/source/API/SBDebugger.cpp
lldb/source/Utility/Reproducer.cpp
lldb/unittests/Utility/ReproducerTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/Reproducer.h 
b/lldb/include/lldb/Utility/Reproducer.h
index 5d810460a0c5..ab673e5e0019 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -159,6 +159,9 @@ class WorkingDirectoryProvider : public 
Provider {
   static char ID;
 };
 
+/// The recorder is a small object handed out by a provider to record data. It
+/// is commonly used in combination with a MultiProvider which is meant to
+/// record information for multiple instances of the same source of data.
 class AbstractRecorder {
 protected:
   AbstractRecorder(const FileSpec , std::error_code )
@@ -181,6 +184,7 @@ class AbstractRecorder {
   bool m_record;
 };
 
+/// Recorder that records its data as text to a file.
 class DataRecorder : public AbstractRecorder {
 public:
   DataRecorder(const FileSpec , std::error_code )
@@ -199,24 +203,88 @@ class DataRecorder : public AbstractRecorder {
   }
 };
 
-class CommandProvider : public Provider {
+/// Recorder that records its data as YAML to a file.
+class YamlRecorder : public AbstractRecorder {
+public:
+  YamlRecorder(const FileSpec , std::error_code )
+  : AbstractRecorder(filename, ec) {}
+
+  static llvm::Expected>
+  Create(const FileSpec );
+
+  template  void Record(const T ) {
+if (!m_record)
+  return;
+llvm::yaml::Output yout(m_os);
+// The YAML traits are defined as non-const because they are used for
+// serialization and deserialization. The cast is safe because
+// serialization doesn't modify the object.
+yout << const_cast(t);
+m_os.flush();
+  }
+};
+
+/// The MultiProvider is a provider that hands out recorder which can be used
+/// to capture data for 
diff erent instances of the same object. The recorders
+/// can be passed around or stored as an instance member.
+///
+/// The Info::file for the MultiProvider contains an index of files for every
+/// recorder. Use the MultiLoader to read the index and get the individual
+/// files.
+template 
+class MultiProvider : public repro::Provider {
+public:
+  MultiProvider(const FileSpec ) : Provider(directory) {}
+
+  T *GetNewRecorder() {
+std::size_t i = m_recorders.size() + 1;
+std::string filename = (llvm::Twine(V::Info::name) + llvm::Twine("-") +
+llvm::Twine(i) + llvm::Twine(".yaml"))
+   .str();
+auto recorder_or_error =
+T::Create(this->GetRoot().CopyByAppendingPathComponent(filename));
+if (!recorder_or_error) {
+  llvm::consumeError(recorder_or_error.takeError());
+  return nullptr;
+}
+
+m_recorders.push_back(std::move(*recorder_or_error));
+return m_recorders.back().get();
+  }
+
+  void Keep() override {
+std::vector files;
+for (auto  : m_recorders) {
+  recorder->Stop();
+  files.push_back(recorder->GetFilename().GetPath());
+}
+
+FileSpec file = 
this->GetRoot().CopyByAppendingPathComponent(V::Info::file);
+std::error_code ec;
+llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text);
+if (ec)
+  return;
+llvm::yaml::Output yout(os);
+yout << files;
+  }
+
+  void Discard() override { m_recorders.clear(); }
+
+private:
+  std::vector> m_recorders;
+};
+
+class CommandProvider : public MultiProvider {
 public:
   struct Info {
 static const char *name;
 static const char *file;
   };
 
-  CommandProvider(const FileSpec ) : Provider(directory) {}
-
-  DataRecorder *GetNewDataRecorder();
-
-  void Keep() override;
-  void Discard() override;
+  CommandProvider(const FileSpec )
+  : MultiProvider(directory) {}
 
   static char ID;
-
-private:
-  std::vector> 

[Lldb-commits] [PATCH] D83441: [ldb/Reproducers] Add YamlRecorder and MultiProvider

2020-07-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
JDevlieghere marked 5 inline comments as done.
Closed by commit rG169c83208f37: [ldb/Reproducers] Add YamlRecorder and 
MultiProvider (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D83441?vs=276597=277133#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83441

Files:
  lldb/include/lldb/Utility/Reproducer.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Utility/Reproducer.cpp
  lldb/unittests/Utility/ReproducerTest.cpp

Index: lldb/unittests/Utility/ReproducerTest.cpp
===
--- lldb/unittests/Utility/ReproducerTest.cpp
+++ lldb/unittests/Utility/ReproducerTest.cpp
@@ -9,6 +9,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
 
@@ -31,8 +32,25 @@
   static char ID;
 };
 
+class YamlMultiProvider
+: public MultiProvider {
+public:
+  struct Info {
+static const char *name;
+static const char *file;
+  };
+
+  YamlMultiProvider(const FileSpec ) : MultiProvider(directory) {}
+
+  static char ID;
+};
+
 const char *DummyProvider::Info::name = "dummy";
 const char *DummyProvider::Info::file = "dummy.yaml";
+const char *YamlMultiProvider::Info::name = "mutli";
+const char *YamlMultiProvider::Info::file = "mutli.yaml";
+char DummyProvider::ID = 0;
+char YamlMultiProvider::ID = 0;
 
 class DummyReproducer : public Reproducer {
 public:
@@ -42,7 +60,25 @@
   using Reproducer::SetReplay;
 };
 
-char DummyProvider::ID = 0;
+struct YamlData {
+  YamlData() : i(-1) {}
+  YamlData(int i) : i(i) {}
+  int i;
+};
+
+inline bool operator==(const YamlData , const YamlData ) {
+  return LHS.i == RHS.i;
+}
+
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(YamlData)
+
+namespace llvm {
+namespace yaml {
+template <> struct MappingTraits {
+  static void mapping(IO , YamlData ) { io.mapRequired("i", Y.i); };
+};
+} // namespace yaml
+} // namespace llvm
 
 TEST(ReproducerTest, SetCapture) {
   DummyReproducer reproducer;
@@ -144,3 +180,83 @@
   auto _alt = generator.GetOrCreate();
   EXPECT_EQ(, _alt);
 }
+
+TEST(GeneratorTest, YamlMultiProvider) {
+  SmallString<128> root;
+  std::error_code ec = llvm::sys::fs::createUniqueDirectory("reproducer", root);
+  ASSERT_FALSE(static_cast(ec));
+
+  auto cleanup = llvm::make_scope_exit(
+  [&] { EXPECT_FALSE(llvm::sys::fs::remove_directories(root.str())); });
+
+  YamlData data0(0);
+  YamlData data1(1);
+  YamlData data2(2);
+  YamlData data3(3);
+
+  {
+DummyReproducer reproducer;
+EXPECT_THAT_ERROR(reproducer.SetCapture(FileSpec(root.str())), Succeeded());
+
+auto  = *reproducer.GetGenerator();
+auto *provider = generator.Create();
+ASSERT_NE(nullptr, provider);
+
+auto *recorder = provider->GetNewRecorder();
+ASSERT_NE(nullptr, recorder);
+recorder->Record(data0);
+recorder->Record(data1);
+
+recorder = provider->GetNewRecorder();
+ASSERT_NE(nullptr, recorder);
+recorder->Record(data2);
+recorder->Record(data3);
+
+generator.Keep();
+  }
+
+  {
+DummyReproducer reproducer;
+EXPECT_THAT_ERROR(reproducer.SetReplay(FileSpec(root.str())), Succeeded());
+
+auto  = *reproducer.GetLoader();
+std::unique_ptr> multi_loader =
+repro::MultiLoader::Create();
+
+// Read the first file.
+{
+  llvm::Optional file = multi_loader->GetNextFile();
+  EXPECT_TRUE(static_cast(file));
+
+  auto buffer = llvm::MemoryBuffer::getFile(*file);
+  EXPECT_TRUE(static_cast(buffer));
+
+  yaml::Input yin((*buffer)->getBuffer());
+  std::vector data;
+  yin >> data;
+
+  ASSERT_EQ(data.size(), 2U);
+  EXPECT_THAT(data, testing::ElementsAre(data0, data1));
+}
+
+// Read the second file.
+{
+  llvm::Optional file = multi_loader->GetNextFile();
+  EXPECT_TRUE(static_cast(file));
+
+  auto buffer = llvm::MemoryBuffer::getFile(*file);
+  EXPECT_TRUE(static_cast(buffer));
+
+  yaml::Input yin((*buffer)->getBuffer());
+  std::vector data;
+  yin >> data;
+
+  ASSERT_EQ(data.size(), 2U);
+  EXPECT_THAT(data, testing::ElementsAre(data2, data3));
+}
+
+// There is no third file.
+llvm::Optional file = multi_loader->GetNextFile();
+EXPECT_FALSE(static_cast(file));
+  }
+}
Index: lldb/source/Utility/Reproducer.cpp
===
--- lldb/source/Utility/Reproducer.cpp
+++ lldb/source/Utility/Reproducer.cpp
@@ -272,40 +272,15 @@
   return std::move(recorder);
 }
 
-DataRecorder *CommandProvider::GetNewDataRecorder() {
-  std::size_t i = m_data_recorders.size() + 1;
-  std::string filename = (llvm::Twine(Info::name) + llvm::Twine("-") +
-  llvm::Twine(i) + 

[Lldb-commits] [PATCH] D83582: Fix nesting of #ifdef

2020-07-10 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D83582



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


[Lldb-commits] [PATCH] D83545: [lldb/dotest] Remove the "xunit" result formatter

2020-07-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Yup, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83545



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


[Lldb-commits] [PATCH] D83582: Fix nesting of #ifdef

2020-07-10 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl updated this revision to Diff 277122.
aprantl added a comment.

Also move comment.


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

https://reviews.llvm.org/D83582

Files:
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm


Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -780,11 +780,11 @@
 if (DeploymentInfo deployment_info = GetDeploymentInfo(lc, load_cmds_p)) {
   // Simulator support. If the platform is ambiguous, use the dyld info.
   if (deployment_info.maybe_simulator) {
-// If dyld doesn't return a platform, use a heuristic.
-#if (defined(__x86_64__) || defined(__i386__))
-// If we are running on Intel macOS, it is safe to assume
-// this is really a back-deploying simulator binary.
 if (deployment_info.maybe_simulator) {
+#if (defined(__x86_64__) || defined(__i386__))
+  // If dyld doesn't return a platform, use a heuristic.
+  // If we are running on Intel macOS, it is safe to assume
+  // this is really a back-deploying simulator binary.
   switch (deployment_info.platform) {
   case PLATFORM_IOS:
 deployment_info.platform = PLATFORM_IOSSIMULATOR;
@@ -797,12 +797,12 @@
 break;
   }
 #else
-// On an Apple Silicon macOS host, there is no
-// ambiguity. The only binaries that use legacy load
-// commands are back-deploying native iOS binaries. All
-// simulator binaries use the newer, unambiguous
-// LC_BUILD_VERSION load commands.
-deployment_info.maybe_simulator = false;
+  // On an Apple Silicon macOS host, there is no
+  // ambiguity. The only binaries that use legacy load
+  // commands are back-deploying native iOS binaries. All
+  // simulator binaries use the newer, unambiguous
+  // LC_BUILD_VERSION load commands.
+  deployment_info.maybe_simulator = false;
 #endif
 }
   }


Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -780,11 +780,11 @@
 if (DeploymentInfo deployment_info = GetDeploymentInfo(lc, load_cmds_p)) {
   // Simulator support. If the platform is ambiguous, use the dyld info.
   if (deployment_info.maybe_simulator) {
-// If dyld doesn't return a platform, use a heuristic.
-#if (defined(__x86_64__) || defined(__i386__))
-// If we are running on Intel macOS, it is safe to assume
-// this is really a back-deploying simulator binary.
 if (deployment_info.maybe_simulator) {
+#if (defined(__x86_64__) || defined(__i386__))
+  // If dyld doesn't return a platform, use a heuristic.
+  // If we are running on Intel macOS, it is safe to assume
+  // this is really a back-deploying simulator binary.
   switch (deployment_info.platform) {
   case PLATFORM_IOS:
 deployment_info.platform = PLATFORM_IOSSIMULATOR;
@@ -797,12 +797,12 @@
 break;
   }
 #else
-// On an Apple Silicon macOS host, there is no
-// ambiguity. The only binaries that use legacy load
-// commands are back-deploying native iOS binaries. All
-// simulator binaries use the newer, unambiguous
-// LC_BUILD_VERSION load commands.
-deployment_info.maybe_simulator = false;
+  // On an Apple Silicon macOS host, there is no
+  // ambiguity. The only binaries that use legacy load
+  // commands are back-deploying native iOS binaries. All
+  // simulator binaries use the newer, unambiguous
+  // LC_BUILD_VERSION load commands.
+  deployment_info.maybe_simulator = false;
 #endif
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D83582: Fix nesting of #ifdef

2020-07-10 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl created this revision.
aprantl added a reviewer: friss.
Herald added a subscriber: kristof.beyls.

This fixes a compile error when building for an arm64 host.


https://reviews.llvm.org/D83582

Files:
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm


Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -781,10 +781,10 @@
   // Simulator support. If the platform is ambiguous, use the dyld info.
   if (deployment_info.maybe_simulator) {
 // If dyld doesn't return a platform, use a heuristic.
-#if (defined(__x86_64__) || defined(__i386__))
 // If we are running on Intel macOS, it is safe to assume
 // this is really a back-deploying simulator binary.
 if (deployment_info.maybe_simulator) {
+#if (defined(__x86_64__) || defined(__i386__))
   switch (deployment_info.platform) {
   case PLATFORM_IOS:
 deployment_info.platform = PLATFORM_IOSSIMULATOR;
@@ -797,12 +797,12 @@
 break;
   }
 #else
-// On an Apple Silicon macOS host, there is no
-// ambiguity. The only binaries that use legacy load
-// commands are back-deploying native iOS binaries. All
-// simulator binaries use the newer, unambiguous
-// LC_BUILD_VERSION load commands.
-deployment_info.maybe_simulator = false;
+  // On an Apple Silicon macOS host, there is no
+  // ambiguity. The only binaries that use legacy load
+  // commands are back-deploying native iOS binaries. All
+  // simulator binaries use the newer, unambiguous
+  // LC_BUILD_VERSION load commands.
+  deployment_info.maybe_simulator = false;
 #endif
 }
   }


Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -781,10 +781,10 @@
   // Simulator support. If the platform is ambiguous, use the dyld info.
   if (deployment_info.maybe_simulator) {
 // If dyld doesn't return a platform, use a heuristic.
-#if (defined(__x86_64__) || defined(__i386__))
 // If we are running on Intel macOS, it is safe to assume
 // this is really a back-deploying simulator binary.
 if (deployment_info.maybe_simulator) {
+#if (defined(__x86_64__) || defined(__i386__))
   switch (deployment_info.platform) {
   case PLATFORM_IOS:
 deployment_info.platform = PLATFORM_IOSSIMULATOR;
@@ -797,12 +797,12 @@
 break;
   }
 #else
-// On an Apple Silicon macOS host, there is no
-// ambiguity. The only binaries that use legacy load
-// commands are back-deploying native iOS binaries. All
-// simulator binaries use the newer, unambiguous
-// LC_BUILD_VERSION load commands.
-deployment_info.maybe_simulator = false;
+  // On an Apple Silicon macOS host, there is no
+  // ambiguity. The only binaries that use legacy load
+  // commands are back-deploying native iOS binaries. All
+  // simulator binaries use the newer, unambiguous
+  // LC_BUILD_VERSION load commands.
+  deployment_info.maybe_simulator = false;
 #endif
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] ecfa01e - [lldb] on s390x fix override issue

2020-07-10 Thread Konrad Kleine via lldb-commits

Author: Konrad Kleine
Date: 2020-07-10T15:11:49-04:00
New Revision: ecfa01e956a49ed349683f1cfcfbbec423114b68

URL: 
https://github.com/llvm/llvm-project/commit/ecfa01e956a49ed349683f1cfcfbbec423114b68
DIFF: 
https://github.com/llvm/llvm-project/commit/ecfa01e956a49ed349683f1cfcfbbec423114b68.diff

LOG: [lldb] on s390x fix override issue

Summary:
This fixes an override issue by marking a function as const so that the
signature maps to the signature of the function in the base class.

This is the original error:

In file included from 
/root/llvm/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp:11:
/root/llvm/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h:79:10:
 error: 'size_t 
lldb_private::process_linux::NativeRegisterContextLinux_s390x::GetGPRSize()' 
marked 'override', but does not override
   79 |   size_t GetGPRSize() override { return sizeof(m_regs); }
  |  ^~

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h 
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
index 69714e400113..5ed78810a18b 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@ class NativeRegisterContextLinux_s390x : public 
NativeRegisterContextLinux {
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return _regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return _fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 



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


[Lldb-commits] [PATCH] D83580: [lldb] on s390x fix override issue

2020-07-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGecfa01e956a4: [lldb] on s390x fix override issue (authored 
by kwk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83580

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return _regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return _fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return _regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return _fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D83580: [lldb] on s390x fix override issue

2020-07-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk added a comment.

In D83580#2144871 , @serge-sans-paille 
wrote:

> Possible followup: shouldn't all the other `Get*` be flagged as `const` too?


Thank you @serge-sans-paille . You're right but I'm unsure if all getters are 
actually const or not. And I think one needs to figure this out for all 
overrides as well. And right now I don't want to break another arch ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83580



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


[Lldb-commits] [PATCH] D83580: [lldb] on s390x fix override issue

2020-07-10 Thread serge via Phabricator via lldb-commits
serge-sans-paille added a comment.

Possible followup: shouldn't all the other `Get*` be flagged as `const` too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83580



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


[Lldb-commits] [PATCH] D83580: [lldb] on s390x fix override issue

2020-07-10 Thread serge via Phabricator via lldb-commits
serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.

Looks obviously good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83580



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


[Lldb-commits] [PATCH] D83580: [lldb] on s390x fix override issue

2020-07-10 Thread Konrad Wilhelm Kleine via Phabricator via lldb-commits
kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This fixes an override issue by marking a function as const so that the
signature maps to the signature of the function in the base class.

This is the original error:

In file included from 
/root/llvm/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp:11:
/root/llvm/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h:79:10:
 error: 'size_t 
lldb_private::process_linux::NativeRegisterContextLinux_s390x::GetGPRSize()' 
marked 'override', but does not override

  79 |   size_t GetGPRSize() override { return sizeof(m_regs); }
 |  ^~


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83580

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return _regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return _fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return _regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return _fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-10 Thread Jim Ingham via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe337350be9d6: This is a refinement on 
96601ec28b7efe5abf3479a1aa91bcedb235bbbd. The intent of… (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83450

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectVariable.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectVariable.cpp

Index: lldb/source/Core/ValueObjectVariable.cpp
===
--- lldb/source/Core/ValueObjectVariable.cpp
+++ lldb/source/Core/ValueObjectVariable.cpp
@@ -242,9 +242,64 @@
   m_resolved_value.SetContext(Value::eContextTypeInvalid, nullptr);
 }
   }
+  
   return m_error.Success();
 }
 
+void ValueObjectVariable::DoUpdateChildrenAddressType(ValueObject ) {
+  Value::ValueType value_type = valobj.GetValue().GetValueType();
+  ExecutionContext exe_ctx(GetExecutionContextRef());
+  Process *process = exe_ctx.GetProcessPtr();
+  const bool process_is_alive = process && process->IsAlive();
+  const uint32_t type_info = valobj.GetCompilerType().GetTypeInfo();
+  const bool is_pointer_or_ref =
+  (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
+
+  switch (value_type) {
+  case Value::eValueTypeFileAddress:
+// If this type is a pointer, then its children will be considered load
+// addresses if the pointer or reference is dereferenced, but only if
+// the process is alive.
+//
+// There could be global variables like in the following code:
+// struct LinkedListNode { Foo* foo; LinkedListNode* next; };
+// Foo g_foo1;
+// Foo g_foo2;
+// LinkedListNode g_second_node = { _foo2, NULL };
+// LinkedListNode g_first_node = { _foo1, _second_node };
+//
+// When we aren't running, we should be able to look at these variables
+// using the "target variable" command. Children of the "g_first_node"
+// always will be of the same address type as the parent. But children
+// of the "next" member of LinkedListNode will become load addresses if
+// we have a live process, or remain a file address if it was a file
+// address.
+if (process_is_alive && is_pointer_or_ref)
+  valobj.SetAddressTypeOfChildren(eAddressTypeLoad);
+else
+  valobj.SetAddressTypeOfChildren(eAddressTypeFile);
+break;
+  case Value::eValueTypeHostAddress:
+// Same as above for load addresses, except children of pointer or refs
+// are always load addresses. Host addresses are used to store freeze
+// dried variables. If this type is a struct, the entire struct
+// contents will be copied into the heap of the
+// LLDB process, but we do not currently follow any pointers.
+if (is_pointer_or_ref)
+  valobj.SetAddressTypeOfChildren(eAddressTypeLoad);
+else
+  valobj.SetAddressTypeOfChildren(eAddressTypeHost);
+break;
+  case Value::eValueTypeLoadAddress:
+  case Value::eValueTypeScalar:
+  case Value::eValueTypeVector:
+valobj.SetAddressTypeOfChildren(eAddressTypeLoad);
+break;
+  }
+}
+
+
+
 bool ValueObjectVariable::IsInScope() {
   const ExecutionContextRef _ctx_ref = GetExecutionContextRef();
   if (exe_ctx_ref.HasFrameRef()) {
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -140,58 +140,6 @@
 // Destructor
 ValueObject::~ValueObject() {}
 
-void ValueObject::UpdateChildrenAddressType() {
-  Value::ValueType value_type = m_value.GetValueType();
-  ExecutionContext exe_ctx(GetExecutionContextRef());
-  Process *process = exe_ctx.GetProcessPtr();
-  const bool process_is_alive = process && process->IsAlive();
-  const uint32_t type_info = GetCompilerType().GetTypeInfo();
-  const bool is_pointer_or_ref =
-  (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
-
-  switch (value_type) {
-  case Value::eValueTypeFileAddress:
-// If this type is a pointer, then its children will be considered load
-// addresses if the pointer or reference is dereferenced, but only if
-// the process is alive.
-//
-// There could be global variables like in the following code:
-// struct LinkedListNode { Foo* foo; LinkedListNode* next; };
-// Foo g_foo1;
-// Foo g_foo2;
-// LinkedListNode g_second_node = { _foo2, NULL };
-// LinkedListNode g_first_node = { _foo1, _second_node };
-//
-// When we aren't running, we should be able to look at these variables
-// using the "target variable" command. Children of the "g_first_node"
-// always will be of the same address type as the parent. But children
-// of the "next" member of LinkedListNode will become load addresses if
-// we have a live process, or remain a file address if it was a 

[Lldb-commits] [lldb] e337350 - This is a refinement on 96601ec28b7efe5abf3479a1aa91bcedb235bbbd. The intent of that change was to do the same work for the computation of the locations of the children

2020-07-10 Thread Jim Ingham via lldb-commits

Author: Jim Ingham
Date: 2020-07-10T11:11:59-07:00
New Revision: e337350be9d6efd72027c331f95ef33df61fdc43

URL: 
https://github.com/llvm/llvm-project/commit/e337350be9d6efd72027c331f95ef33df61fdc43
DIFF: 
https://github.com/llvm/llvm-project/commit/e337350be9d6efd72027c331f95ef33df61fdc43.diff

LOG: This is a refinement on 96601ec28b7efe5abf3479a1aa91bcedb235bbbd. The 
intent of that change was to do the same work for the computation of the 
locations of the children of ValueObjectVariable as was done for the root 
ValueObjectVariable. This original patch did that by moving the computation 
from ValueObjectVariable to ValueObject. That fixed the problem but caused a 
handful of swift-lldb testsuite failures and a crash or two.

The problem is that synthetic value objects can sometimes represent objects in 
target memory, and other times they might be made up wholly in lldb memory, 
with pointers from one synthetic object to another, and so the 
ValueObjectVariable computation was not appropriate.

This patch delegates the computation to the root of the ValueObject in 
question. That solves the problem for ValueObjectVariable while not messing up 
the computation for ValueObjectConstResult or ValueObjectSynthetic.

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

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/Core/ValueObjectVariable.h
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectVariable.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index 7c8a01beea56..0080368fd996 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -963,9 +963,14 @@ class ValueObject : public UserID {
 
   void SetPreferredDisplayLanguageIfNeeded(lldb::LanguageType);
 
+protected:
+  virtual void DoUpdateChildrenAddressType(ValueObject ) { return; };
+
 private:
   virtual CompilerType MaybeCalculateCompleteType();
-  void UpdateChildrenAddressType();
+  void UpdateChildrenAddressType() {
+GetRoot()->DoUpdateChildrenAddressType(*this);
+  }
 
   lldb::ValueObjectSP GetValueForExpressionPath_Impl(
   llvm::StringRef expression_cstr,

diff  --git a/lldb/include/lldb/Core/ValueObjectVariable.h 
b/lldb/include/lldb/Core/ValueObjectVariable.h
index a0417e5e7d85..b7e262574a14 100644
--- a/lldb/include/lldb/Core/ValueObjectVariable.h
+++ b/lldb/include/lldb/Core/ValueObjectVariable.h
@@ -67,6 +67,8 @@ class ValueObjectVariable : public ValueObject {
 
 protected:
   bool UpdateValue() override;
+  
+  void DoUpdateChildrenAddressType(ValueObject ) override;
 
   CompilerType GetCompilerTypeImpl() override;
 

diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index f13ad63dfb61..3a775b07e5e1 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -140,58 +140,6 @@ ValueObject::ValueObject(ExecutionContextScope *exe_scope,
 // Destructor
 ValueObject::~ValueObject() {}
 
-void ValueObject::UpdateChildrenAddressType() {
-  Value::ValueType value_type = m_value.GetValueType();
-  ExecutionContext exe_ctx(GetExecutionContextRef());
-  Process *process = exe_ctx.GetProcessPtr();
-  const bool process_is_alive = process && process->IsAlive();
-  const uint32_t type_info = GetCompilerType().GetTypeInfo();
-  const bool is_pointer_or_ref =
-  (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
-
-  switch (value_type) {
-  case Value::eValueTypeFileAddress:
-// If this type is a pointer, then its children will be considered load
-// addresses if the pointer or reference is dereferenced, but only if
-// the process is alive.
-//
-// There could be global variables like in the following code:
-// struct LinkedListNode { Foo* foo; LinkedListNode* next; };
-// Foo g_foo1;
-// Foo g_foo2;
-// LinkedListNode g_second_node = { _foo2, NULL };
-// LinkedListNode g_first_node = { _foo1, _second_node };
-//
-// When we aren't running, we should be able to look at these variables
-// using the "target variable" command. Children of the "g_first_node"
-// always will be of the same address type as the parent. But children
-// of the "next" member of LinkedListNode will become load addresses if
-// we have a live process, or remain a file address if it was a file
-// address.
-if (process_is_alive && is_pointer_or_ref)
-  SetAddressTypeOfChildren(eAddressTypeLoad);
-else
-  SetAddressTypeOfChildren(eAddressTypeFile);
-break;
-  case Value::eValueTypeHostAddress:
-// Same as above for load addresses, except children of pointer or refs
-// are always load addresses. Host addresses are used to store freeze
-// dried variables. If this type is a struct, the entire struct
-// contents will be copied into the heap of 

[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

In D83450#2143631 , @labath wrote:

> Right. It does bug me a little that this has no test, but I don't think it's 
> the worst that could happen. And you're the one who's going to have to debug 
> this all over again if the next value object change breaks swift again.


Thanks!

> 
> 
> In D83450#2142114 , @jingham wrote:
> 
>> I'm planning on continuing to dig into this stuff, and hopefully come up 
>> with something more coherent.  But your fix is needed for 
>> ValueObjectVariables to avoid crashes when debugging optimized code, and so 
>> I need a short-term way to get it to work but not cause problems for 
>> Synthetic values.  This fix is mostly correct.  , I think if you were 
>> being more rigorous about this direction of fix, you would have 
>> UpdateChildrenAddressType walk back up the parent hierarchy asking at each 
>> level whether the parent knows where its pointer children should be found.  
>> But that's adding complexity that doesn't solve any actual problem, and we'd 
>> probably end up ripping it out again in a better solution.
> 
> 
> Yes, I've been wondering if something like that wasn't needed, but I wasn't 
> sure how all the parent-child relationships work...

The reason for not doing it that way immediately is that I see no reason why 
the entity creating a subordinate ValueObject can't know where the ValueObject 
it is creating will find its data and the data for anything that reaches 
outside the immediate Value, when it creates it.  So adding more complexity to 
UpdateChildrenAddressType just makes the whole system more Ptolemaic, and it's 
got way too many epicycles as it stands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83450



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


[Lldb-commits] [PATCH] D83454: [CMake] Make `intrinsics_gen` dependency unconditional.

2020-07-10 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D83454#2142719 , @michele.scandale 
wrote:

> I tested locally the standalone build of Clang with D83426 
> .
>  I just want to make sure that we all agree this is right way moving forward.


Yep, with the target exported this is the right thing to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83454



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


[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

In D83446#2143464 , @labath wrote:

> In D83446#2142564 , @JDevlieghere 
> wrote:
>
> > Seems like my thoughts got lost a bit with all the inline replies: we can 
> > solve this particular issue by making `process connect` block in 
> > synchronous mode. The fact that that's not happening today is a bug beyond 
> > the reproducers. I don't think we should change the current behavior in 
> > asynchronous mode. I think it's probably worthwhile to do that even if we 
> > decide to add extra synchronization for the reproducers in which case this 
> > would no longer be an example of a problem solved by the current patch.
>
>
> I'm starting to understand why you say "process connect" is not 
> blocking/synchronous. Although it is waiting for the "stopped" event to be 
> sent to the public queue (here 
> ),
>  it is not waiting for it to be processed by the public queue. The "process 
> continue" command OTOH, is kind of waiting for the public queue processing to 
> finish. I say only "kind of" because it does not actually process it on the 
> _real_ public queue -- instead it "hijacks" the events and processes then on 
> its own little event loop 
> .
>  The part that I got wrong was that I was assuming that the hijacked event 
> loop would rebroadcast the stop event to the real public queue. In fact, it 
> doesn't, which means that the stop event processing is completely finished by 
> the time that the "process continue" command returns.
>
> It seems reasonable that "process connect" (and it's SB equivalent) should 
> behave the same way. This would also explain why so many of the "gdb-client" 
> tests need to use `lldbutil.expect_state_changes` when connecting to the mock 
> server, even though they are running in synchronous mode (the only other 
> tests using this function explicitly choose to use the async mode).
>
> For async mode, we can say that the user needs to ensure that the event is 
> handled before he can issue other commands -- this is the same situation as 
> with "process continue". Of course, in the asynch case, the reproducers are 
> "the user" and they will need to do something about this. But maybe we don't 
> need to cross that bridge right now?


The "synchronous" mode contract is that by the time the command completes, 
there should be no more event work to be done.  That's why the event should be 
consumed and not rebroadcast.  It seems to me "process connect" is just 
breaking the synch mode contract.

I think reproducers in truly async mode (particularly where you can have 
multiple processes each with different listeners) is going to be challenging 
for reproducers.  But sync mode shouldn't be that hard, provided it is 
implemented correctly everywhere.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D83446



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


[Lldb-commits] [PATCH] D83433: Fix how we handle bit-fields for Objective-C when creating an AST

2020-07-10 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

(Also the commit description should be updated again).


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

https://reviews.llvm.org/D83433



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


[Lldb-commits] [PATCH] D83433: Fix how we handle bit-fields for Objective-C when creating an AST

2020-07-10 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

So for the test case that this fixes I get:

  x i 8
  f1 b15 12 (DWARF 0x0)
  f2 b4 13 (DWARF 0x7)
  f3 b4 14 (DWARF 0x3)

For the 2/4/4 bitfield case I get:

  x i 8
  f1 b2 12 (DWARF 0x0)
  f2 b4 12 (DWARF 0x2)
  f3 b4 12 (DWARF 0x6)

Anyway, Adrian's and Jim's point seem right. DWARF contains the bit offset from 
the start of the 'container' (it seems ObjC just groups bitfields together by 
some algorithm). If I look at the code for printing bitfields it seems like we 
expect to get the *bit* offset from the layout we store and we get the *byte* 
offset from the runtime. And then we combine those two into the actual offset 
we have to read. The values we get from the example above seem to match that 
and get the expected result, so I think that's right.

Regarding the second test: Running the test (You need to add the `test_` prefix 
first) I get a pretty long warning that we try to get the size of the interface 
without an execution context:

  warning: trying to determine the size of type @interface HasBitfield2 : 
NSObject{
  unsigned int x;
  unsigned int field1;
  unsigned int field2;
  unsigned int field3;
  }
  @end
  without a valid ExecutionContext. this is not reliable. please file a bug 
against LLDB.
  backtrace:
  0  liblldb.11.0.0git.dylib 0x0001070146b5 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
  1  liblldb.11.0.0git.dylib 0x000106f16a1b 
lldb_private::TypeSystemClang::GetBitSize(void*, 
lldb_private::ExecutionContextScope*) + 827
  2  liblldb.11.0.0git.dylib 0x000106ba0421 
lldb_private::CompilerType::GetByteSize(lldb_private::ExecutionContextScope*) 
const + 33
  3  liblldb.11.0.0git.dylib 0x0001079bcd68 
IRForTarget::CreateResultVariable(llvm::Function&) + 2760
  4  liblldb.11.0.0git.dylib 0x0001079c30d0 
IRForTarget::runOnModule(llvm::Module&) + 928
  5  liblldb.11.0.0git.dylib 0x0001079a15a4 
lldb_private::ClangExpressionParser::PrepareForExecution(unsigned long long&, 
unsigned long long&, std::__1::shared_ptr&, 
lldb_private::ExecutionContext&, bool&, lldb_private::ExecutionPolicy) + 1572
  6  liblldb.11.0.0git.dylib 0x0001079b723a 
lldb_private::ClangUserExpression::Parse(lldb_private::DiagnosticManager&, 
lldb_private::ExecutionContext&, lldb_private::ExecutionPolicy, bool, bool) + 
1002
  7  liblldb.11.0.0git.dylib 0x000106b31e77 
lldb_private::UserExpression::Evaluate(lldb_private::ExecutionContext&, 
lldb_private::EvaluateExpressionOptions const&, llvm::StringRef, 
llvm::StringRef, std::__1::shared_ptr&, 
lldb_private::Status&, std::__1::basic_string, std::__1::allocator >*, 
lldb_private::ValueObject*) + 2375
  8  liblldb.11.0.0git.dylib 0x000106c358e5 
lldb_private::Target::EvaluateExpression(llvm::StringRef, 
lldb_private::ExecutionContextScope*, 
std::__1::shared_ptr&, 
lldb_private::EvaluateExpressionOptions const&, std::__1::basic_string, std::__1::allocator >*, 
lldb_private::ValueObject*) + 885

In general the semantics here seem kinda off. The result of the expression is  
`HasBitfield2` which I don't think is something we really have in Obj-C as it 
seems like a stack-allocated `HasBitfields2` instance. Changing the expression 
to return `HasBitfield2 *` instead and then forcing that we dereference Obj-C 
pointers fixes the second case for me. (FWIW, doing `frame var *hb2` is also 
working and is doing essentially the same thing as it just prints the children 
of the pointer). Fixing that can just be a follow up PR.

So I would say removing the sanity check here seems fine. We maybe want to have 
a specific Obj-C sanity check but I'm not even sure how this would look like. 
Maybe figuring out what's the maximum 'container size' we can have in Obj-C and 
checking that offset + size is below that makes sense, but otherwise I don't 
think we can do a lot. So I think my previous comment that this is ready to go 
beside some minor changes is IMHO the way to go.




Comment at: lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py:24
+@expectedFailureAll()
+def ExpressionOnObject(self):
+self.build()

This method isn't a test at the moment as it doesn't have a `test` prefix in 
the name. E.g. `test_ExpressionOnObject` is the way to make this a test that 
actually runs. Currently it's just a normal method that is expected to be run 
by other `test_` methods.


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

https://reviews.llvm.org/D83433



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


[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks. It was fairly easy to reproduce the problem. I have a proposed fix in 
D83552 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83306



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


[Lldb-commits] [PATCH] D83552: [lldb/test] Do a better job at setting (DY)LD_LIBRARY_PATH

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: davide, jingham.
Herald added a project: LLDB.

registerSharedLibrariesWithTarget was setting the library path
environment variable to the process build directory, but the function is
also accepting libraries in other directories (in which case they won't
be found automatically).

This patch makes the function set the path variable correctly for these
libraries too. This enables us to remove the code for setting the path
variable in TestWeakSymbols.py, which was working only accidentally --
it was relying on the fact that

  launch_info.SetEnvironmentEntries(..., append=True)

would not overwrite the path variable it has set, but that is going to
change with D83306 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83552

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py


Index: lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py
===
--- lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py
+++ lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py
@@ -48,10 +48,6 @@
 
 launch_info = lldb.SBLaunchInfo(None)
 launch_info.SetWorkingDirectory(self.getBuildDir())
-# We have to point to the hidden directory to pick up the
-# version of the dylib without the weak symbols:
-env_expr = self.platformContext.shlib_environment_var + "=" + 
hidden_dir
-launch_info.SetEnvironmentEntries([env_expr], True)
 
 (self.target, _, thread, _) = lldbutil.run_to_source_breakpoint(
   self, "Set a breakpoint here",
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1928,8 +1928,7 @@
 shlib_prefix = self.platformContext.shlib_prefix
 shlib_extension = '.' + self.platformContext.shlib_extension
 
-working_dir = self.get_process_working_directory()
-environment = ['%s=%s' % (shlib_environment_var, working_dir)]
+dirs = []
 # Add any shared libraries to our target if remote so they get
 # uploaded into the working directory on the remote side
 for name in shlibs:
@@ -1952,6 +1951,7 @@
 # Make sure we found the local shared library in the above code
 self.assertTrue(os.path.exists(local_shlib_path))
 
+
 # Add the shared library to our target
 shlib_module = target.AddModule(local_shlib_path, None, None, None)
 if lldb.remote_platform:
@@ -1961,8 +1961,15 @@
 os.path.basename(local_shlib_path))
 shlib_module.SetRemoteInstallFileSpec(
 lldb.SBFileSpec(remote_shlib_path, False))
+dir_to_add = self.get_process_working_directory()
+else:
+dir_to_add = os.path.dirname(local_shlib_path)
+
+if dir_to_add not in dirs:
+dirs.append(dir_to_add)
 
-return environment
+env_value = self.platformContext.shlib_path_separator.join(dirs)
+return ['%s=%s' % (shlib_environment_var, env_value)]
 
 def registerSanitizerLibrariesWithTarget(self, target):
 runtimes = []


Index: lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py
===
--- lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py
+++ lldb/test/API/commands/expression/weak_symbols/TestWeakSymbols.py
@@ -48,10 +48,6 @@
 
 launch_info = lldb.SBLaunchInfo(None)
 launch_info.SetWorkingDirectory(self.getBuildDir())
-# We have to point to the hidden directory to pick up the
-# version of the dylib without the weak symbols:
-env_expr = self.platformContext.shlib_environment_var + "=" + hidden_dir
-launch_info.SetEnvironmentEntries([env_expr], True)
 
 (self.target, _, thread, _) = lldbutil.run_to_source_breakpoint(
   self, "Set a breakpoint here",
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1928,8 +1928,7 @@
 shlib_prefix = self.platformContext.shlib_prefix
 shlib_extension = '.' + self.platformContext.shlib_extension
 
-working_dir = self.get_process_working_directory()
-environment = ['%s=%s' % (shlib_environment_var, working_dir)]
+dirs = []
 # Add any shared libraries to our target if remote so they get
 # uploaded into the working 

[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

PS: Don't worry about the harbormaster failures too much. It's very picky these 
days...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83425



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


[Lldb-commits] [PATCH] D83425: [lldb] add printing of stdout compile errors to lldbsuite

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test_event/build_exception.py:8
 self.build_error = called_process_error.lldb_extensions.get(
-"stderr_content", "")
+"stderr_content", "").decode("utf-8") + 
called_process_error.lldb_extensions.get("stdout_content","").decode("utf-8")
 

This `.decode` call will be incorrect on python3 if `lldb_extensions` ever does 
not contain the `stderr_content` field. (That's because `""`, a string object, does not have a `decode` method -- only 
`bytes` objects can be decoded in python3.

By the looks of things, stderr_content will always be filled in, so you should 
be able to just replace this with direct member access 
(`called_process_error.lldb_extensions.stderr_content.decode(...)`).

Also, if I'm not mistaken, this will just concatenate stdout and stderr without 
any sort of delimiters, or caring about the order in which they are produced. 
That could end up being confusing because the compilers stderr will come before 
the compiler command which wrote it. If we're not going to be merging the two 
streams in the proper sequence (I described one way in 
), then it would be better to prefix 
each stream with it's own header (like you've done in 
),
 so that it is clear that one is looking at two independent streams.



Comment at: lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py:253-269
 def _handle_error_build(self, test_event):
 """Handles a test error.
 @param test_event the test event to handle.
 """
 message = self._replace_invalid_xml(test_event["issue_message"])
 build_issue_description = self._replace_invalid_xml(
 BuildError.format_build_error(

I believe this file is pretty much unused now. I've created D83545 to remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83425



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


[Lldb-commits] [PATCH] D83545: [lldb/dotest] Remove the "xunit" result formatter

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added a reviewer: JDevlieghere.
Herald added a project: LLDB.

My understanding is that this was added to make dotest interact well
with the GreenDragon bots, back when dotest was the main test driver.
Now that everything goes through lit (which has its own xunit
formatter), it seems largely irrelevant.

There are more cleanups that can be done after removing this be done
here, but this should be enough to test the waters.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83545

Files:
  lldb/packages/Python/lldbsuite/test/configuration.py
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/packages/Python/lldbsuite/test/dotest_args.py
  lldb/packages/Python/lldbsuite/test_event/formatter/__init__.py
  lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py

Index: lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py
===
--- lldb/packages/Python/lldbsuite/test_event/formatter/xunit.py
+++ /dev/null
@@ -1,595 +0,0 @@
-"""
-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
-
-Provides an xUnit ResultsFormatter for integrating the LLDB
-test suite with the Jenkins xUnit aggregator and other xUnit-compliant
-test output processors.
-"""
-from __future__ import absolute_import
-from __future__ import print_function
-
-# System modules
-import re
-import sys
-import xml.sax.saxutils
-
-# Third-party modules
-import six
-
-# Local modules
-from ..event_builder import EventBuilder
-from ..build_exception import BuildError
-from .results_formatter import ResultsFormatter
-
-
-class XunitFormatter(ResultsFormatter):
-"""Provides xUnit-style formatted output.
-"""
-
-# Result mapping arguments
-RM_IGNORE = 'ignore'
-RM_SUCCESS = 'success'
-RM_FAILURE = 'failure'
-RM_PASSTHRU = 'passthru'
-
-@staticmethod
-def _build_illegal_xml_regex():
-"""Constructs a regex to match all illegal xml characters.
-
-Expects to be used against a unicode string."""
-# Construct the range pairs of invalid unicode characters.
-illegal_chars_u = [
-(0x00, 0x08), (0x0B, 0x0C), (0x0E, 0x1F), (0x7F, 0x84),
-(0x86, 0x9F), (0xFDD0, 0xFDDF), (0xFFFE, 0x)]
-
-# For wide builds, we have more.
-if sys.maxunicode >= 0x1:
-illegal_chars_u.extend(
-[(0x1FFFE, 0x1), (0x2FFFE, 0x2), (0x3FFFE, 0x3),
- (0x4FFFE, 0x4), (0x5FFFE, 0x5), (0x6FFFE, 0x6),
- (0x7FFFE, 0x7), (0x8FFFE, 0x8), (0x9FFFE, 0x9),
- (0xAFFFE, 0xA), (0xBFFFE, 0xB), (0xCFFFE, 0xC),
- (0xDFFFE, 0xD), (0xEFFFE, 0xE), (0xE, 0xF),
- (0x10FFFE, 0x10)])
-
-# Build up an array of range expressions.
-illegal_ranges = [
-"%s-%s" % (six.unichr(low), six.unichr(high))
-for (low, high) in illegal_chars_u]
-
-# Compile the regex
-return re.compile(six.u('[%s]') % six.u('').join(illegal_ranges))
-
-@staticmethod
-def _quote_attribute(text):
-"""Returns the given text in a manner safe for usage in an XML attribute.
-
-@param text the text that should appear within an XML attribute.
-@return the attribute-escaped version of the input text.
-"""
-return xml.sax.saxutils.quoteattr(text)
-
-def _replace_invalid_xml(self, str_or_unicode):
-"""Replaces invalid XML characters with a '?'.
-
-@param str_or_unicode a string to replace invalid XML
-characters within.  Can be unicode or not.  If not unicode,
-assumes it is a byte string in utf-8 encoding.
-
-@returns a utf-8-encoded byte string with invalid
-XML replaced with '?'.
-"""
-# Get the content into unicode
-if isinstance(str_or_unicode, str):
-# If we hit decoding errors due to data corruption, replace the
-# invalid characters with U+FFFD REPLACEMENT CHARACTER.
-unicode_content = str_or_unicode.decode('utf-8', 'replace')
-else:
-unicode_content = str_or_unicode
-return self.invalid_xml_re.sub(
-six.u('?'), unicode_content).encode('utf-8')
-
-@classmethod
-def arg_parser(cls):
-"""@return arg parser used to parse formatter-specific options."""
-parser = super(XunitFormatter, cls).arg_parser()
-
-# These are valid choices for results mapping.
-results_mapping_choices = [
-XunitFormatter.RM_IGNORE,
-XunitFormatter.RM_SUCCESS,
-XunitFormatter.RM_FAILURE,
-XunitFormatter.RM_PASSTHRU]
-parser.add_argument(
-"--assert-on-unknown-events",
-  

[Lldb-commits] [PATCH] D83441: [ldb/Reproducers] Add YamlRecorder and MultiProvider

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This seems fine. A bit more doxygen wouldn't hurt...




Comment at: lldb/source/Utility/Reproducer.cpp:10
 #include "lldb/Utility/Reproducer.h"
+#include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBAssert.h"

It doesn't look like this is actually needed.



Comment at: lldb/unittests/Utility/ReproducerTest.cpp:65-66
+struct YamlData {
+  YamlData() : i(-1){};
+  YamlData(int i) : i(i){};
+  int i;

superfluous semicolons



Comment at: lldb/unittests/Utility/ReproducerTest.cpp:74-75
+
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(YamlData)
+LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(std::vector)
+

Are both of these needed? I would expect the first one to be enough...



Comment at: lldb/unittests/Utility/ReproducerTest.cpp:240-242
+  ASSERT_EQ(data.size(), 2U);
+  EXPECT_EQ(data[0], data0);
+  EXPECT_EQ(data[1], data1);

`EXPECT_THAT(data, testing::ElementsAre(data0, data1));`


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

https://reviews.llvm.org/D83441



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


[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Right. It does bug me a little that this has no test, but I don't think it's 
the worst that could happen. And you're the one who's going to have to debug 
this all over again if the next value object change breaks swift again.

In D83450#2142114 , @jingham wrote:

> I'm planning on continuing to dig into this stuff, and hopefully come up with 
> something more coherent.  But your fix is needed for ValueObjectVariables to 
> avoid crashes when debugging optimized code, and so I need a short-term way 
> to get it to work but not cause problems for Synthetic values.  This fix is 
> mostly correct.  , I think if you were being more rigorous about this 
> direction of fix, you would have UpdateChildrenAddressType walk back up the 
> parent hierarchy asking at each level whether the parent knows where its 
> pointer children should be found.  But that's adding complexity that doesn't 
> solve any actual problem, and we'd probably end up ripping it out again in a 
> better solution.


Yes, I've been wondering if something like that wasn't needed, but I wasn't 
sure how all the parent-child relationships work...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83450



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


[Lldb-commits] [PATCH] D83512: [lldb/Module] Allow for the creation of memory-only modules

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I like this.

The coff thingy is unfortunate, but it's not a hard limitation. It's just that 
nobody cared about making this work from memory (until now), and (re)loading 
the file from disk was the easiest solution. If my d372a8e8 
 sticks, 
then I think we should be able to load coff files from memory too. I would like 
if all tests loaded the files from memory, because it makes the code simpler, 
but maybe that does not need to be done here -- that can definitely be handled 
separately.

As for the testing strategy, I think that some dedicated tests would be nice in 
any case, if for nothing else, then for testing some of the edge cases (opening 
a corrupted data buffer, opening a buffer when a file with the given name 
exists on disk, etc.).




Comment at: lldb/include/lldb/Core/ModuleSpec.h:40
 m_uuid(uuid), m_object_name(), m_object_offset(0),
 m_object_size(FileSystem::Instance().GetByteSize(file_spec)),
+m_source_mappings(), m_data(data) {}

I guess this GetByteSize call should also not fire if `data` is not set. In 
fact I'm not sure it should fire at all. I think at one point I tried to remove 
this and didn't get any failures on linux. If mac is the same then maybe we 
could delete it unconditionally.



Comment at: lldb/include/lldb/Core/ModuleSpec.h:55
 m_object_mod_time(rhs.m_object_mod_time),
 m_source_mappings(rhs.m_source_mappings) {}
 

You missed the copy constructor. I think both of these could be `= default`, or 
just omitted completely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83512



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


[Lldb-commits] [lldb] d372a8e - [lldb/pecoff] Use a different llvm createBinary overload for parsing

2020-07-10 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-07-10T11:57:11+02:00
New Revision: d372a8e8bce266bb4043e6a0bfd76c7e5bf457a5

URL: 
https://github.com/llvm/llvm-project/commit/d372a8e8bce266bb4043e6a0bfd76c7e5bf457a5
DIFF: 
https://github.com/llvm/llvm-project/commit/d372a8e8bce266bb4043e6a0bfd76c7e5bf457a5.diff

LOG: [lldb/pecoff] Use a different llvm createBinary overload for parsing

Change the code the use the version which accepts a memory buffer,
instead of the one taking a file name.

This ensures we are not loading the file into memory twice
(ObjectFilePECOFF also loads a copy), reducing our memory footprint, as
well as enabling additional goodies in the future, like being able to
open files which don't exist on disk (D83512).

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp 
b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 7d0f4535bf9e..d2227bde47e9 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -55,15 +55,12 @@ struct CVInfoPdb70 {
   llvm::support::ulittle32_t Age;
 };
 
-static UUID GetCoffUUID(llvm::object::COFFObjectFile *coff_obj) {
-  if (!coff_obj)
-return UUID();
-
+static UUID GetCoffUUID(llvm::object::COFFObjectFile _obj) {
   const llvm::codeview::DebugInfo *pdb_info = nullptr;
   llvm::StringRef pdb_file;
 
   // This part is similar with what has done in minidump parser.
-  if (!coff_obj->getDebugPDBInfo(pdb_info, pdb_file) && pdb_info) {
+  if (!coff_obj.getDebugPDBInfo(pdb_info, pdb_file) && pdb_info) {
 if (pdb_info->PDB70.CVSignature == llvm::OMF::Signature::PDB70) {
   using llvm::support::endian::read16be;
   using llvm::support::endian::read32be;
@@ -172,7 +169,10 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
 
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
 
-  auto binary = llvm::object::createBinary(file.GetPath());
+  if (DataBufferSP full_sp = MapFileData(file, -1, file_offset))
+data_sp = std::move(full_sp);
+  auto binary = llvm::object::createBinary(llvm::MemoryBufferRef(
+  toStringRef(data_sp->GetData()), file.GetFilename().GetStringRef()));
 
   if (!binary) {
 LLDB_LOG_ERROR(log, binary.takeError(),
@@ -180,18 +180,15 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
 return initial_count;
   }
 
-  if (!binary->getBinary()->isCOFF() &&
-  !binary->getBinary()->isCOFFImportFile())
+  auto *COFFObj = llvm::dyn_cast(binary->get());
+  if (!COFFObj)
 return initial_count;
 
-  auto COFFObj =
-llvm::cast(binary->getBinary());
-
   ModuleSpec module_spec(file);
   ArchSpec  = module_spec.GetArchitecture();
   lldb_private::UUID  = module_spec.GetUUID();
   if (!uuid.IsValid())
-uuid = GetCoffUUID(COFFObj);
+uuid = GetCoffUUID(*COFFObj);
 
   switch (COFFObj->getMachine()) {
   case MachineAmd64:
@@ -244,12 +241,13 @@ lldb::SymbolType ObjectFilePECOFF::MapSymbolType(uint16_t 
coff_symbol_type) {
 }
 
 bool ObjectFilePECOFF::CreateBinary() {
-  if (m_owningbin)
+  if (m_binary)
 return true;
 
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_OBJECT));
 
-  auto binary = llvm::object::createBinary(m_file.GetPath());
+  auto binary = llvm::object::createBinary(llvm::MemoryBufferRef(
+  toStringRef(m_data.GetData()), m_file.GetFilename().GetStringRef()));
   if (!binary) {
 LLDB_LOG_ERROR(log, binary.takeError(),
"Failed to create binary for file ({1}): {0}", m_file);
@@ -257,19 +255,14 @@ bool ObjectFilePECOFF::CreateBinary() {
   }
 
   // Make sure we only handle COFF format.
-  if (!binary->getBinary()->isCOFF() &&
-  !binary->getBinary()->isCOFFImportFile())
+  m_binary =
+  llvm::unique_dyn_cast(std::move(*binary));
+  if (!m_binary)
 return false;
 
-  m_owningbin = OWNBINType(std::move(*binary));
-  LLDB_LOGF(log,
-"%p ObjectFilePECOFF::CreateBinary() module = %p (%s), file = "
-"%s, binary = %p (Bin = %p)",
-static_cast(this), static_cast(GetModule().get()),
-GetModule()->GetSpecificationDescription().c_str(),
-m_file ? m_file.GetPath().c_str() : "",
-static_cast(m_owningbin.getPointer()),
-static_cast(m_owningbin->getBinary()));
+  LLDB_LOG(log, "this = {0}, module = {1} ({2}), file = {3}, binary = {4}",
+   this, GetModule().get(), GetModule()->GetSpecificationDescription(),
+   m_file.GetPath(), m_binary.get());
   return true;
 }
 
@@ -281,7 +274,7 @@ ObjectFilePECOFF::ObjectFilePECOFF(const lldb::ModuleSP 
_sp,
lldb::offset_t length)
 : ObjectFile(module_sp, file, file_offset, length, data_sp, data_offset),
   m_dos_header(), m_coff_header(), 

[Lldb-commits] [PATCH] D79699: Add ptrace register access for AArch64 SVE registers

2020-07-10 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 276964.
omjavaid added a comment.

This update incorporates suggestion from last iteration and also rebases after 
merger of register sets and register infos in RegisterInfosPOSIX_arm64.
Also I have posted a separate patch D83541  to 
make lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h platform 
independent in order to use them with elf-core.

@labath what do you think ?


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

https://reviews.llvm.org/D79699

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
@@ -0,0 +1,5 @@
+int main() {
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #5.\n\t");
+  return 0; // Set a break point here.
+}
\ No newline at end of file
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -0,0 +1,142 @@
+"""
+Test the AArch64 SVE registers.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class RegisterCommandsTestCase(TestBase):
+
+def check_sve_register_size(self, set, name, expected):
+reg_value = set.GetChildMemberWithName(name)
+self.assertTrue(reg_value.IsValid(),
+'Verify we have a register named "%s"' % (name))
+self.assertEqual(reg_value.GetByteSize(), expected,
+ 'Verify "%s" == %i' % (name, expected))
+
+mydir = TestBase.compute_mydir(__file__)
+# @skipIf
+
+def test_sve_registers_configuration(self):
+"""Test AArch64 SVE registers size configuration."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+thread = process.GetThreadAtIndex(0)
+currentFrame = thread.GetFrameAtIndex(0)
+
+has_sve = False
+for registerSet in currentFrame.GetRegisters():
+if 'Scalable Vector Extension Registers' in registerSet.GetName():
+has_sve = True
+
+if not has_sve:
+self.skipTest('SVE registers must be supported.')
+
+registerSets = process.GetThreadAtIndex(
+0).GetFrameAtIndex(0).GetRegisters()
+
+sve_registers = registerSets.GetValueAtIndex(2)
+
+vg_reg = sve_registers.GetChildMemberWithName("vg")
+
+vg_reg_value = sve_registers.GetChildMemberWithName(
+"vg").GetValueAsUnsigned()
+
+z_reg_size = vg_reg_value * 8
+
+p_reg_size = z_reg_size / 8
+
+for i in range(32):
+self.check_sve_register_size(
+sve_registers, 'z%i' % (i), z_reg_size)
+
+for i in range(16):
+self.check_sve_register_size(
+sve_registers, 'p%i' % (i), p_reg_size)
+
+self.check_sve_register_size(sve_registers, 'ffr', p_reg_size)
+
+mydir = TestBase.compute_mydir(__file__)
+@no_debug_info_test
+# @skipIf
+def test_sve_registers_read_write(self):
+"""Test AArch64 SVE registers read and write."""
+self.build()
+self.line = line_number('main.c', '// Set a break point here.')
+
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1)
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stop reason = breakpoint 1."])
+
+target = 

[Lldb-commits] [PATCH] D77047: AArch64 SVE register infos and core file support

2020-07-10 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid updated this revision to Diff 276963.
omjavaid added a comment.

This update incorporates suggestion from last iteration and also rebases after 
merger to registersets and register infos in RegisterInfosPOSIX_arm64.
Also I have posted a separate patch D83541  to 
make lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h platform 
independent in order to use them with elf-core.

@labath what do you think ?


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

https://reviews.llvm.org/D77047

Files:
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_arm64_sve.h
  lldb/source/Plugins/Process/Utility/lldb-arm64-register-enums.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
  lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.core

Index: lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
===
--- /dev/null
+++ lldb/test/API/functionalities/postmortem/elf-core/linux-aarch64-sve.c
@@ -0,0 +1,24 @@
+// compile with -march=armv8-a+sve on compatible aarch64 compiler
+// linux-aarch64-sve.core was generated by: aarch64-linux-gnu-gcc-8
+// commandline: -march=armv8-a+sve -nostdlib -static -g linux-aarch64-sve.c
+static void bar(char *boom) {
+  char F = 'b';
+  asm volatile("ptrue p0.s\n\t");
+  asm volatile("fcpy  z0.s, p0/m, #7.5\n\t");
+  asm volatile("ptrue p1.s\n\t");
+  asm volatile("fcpy  z1.s, p1/m, #11.5\n\t");
+  asm volatile("ptrue p3.s\n\t");
+  asm volatile("fcpy  z3.s, p3/m, #15.5\n\t");
+
+  *boom = 47; // Frame bar
+}
+
+static void foo(char *boom, void (*boomer)(char *)) {
+  char F = 'f';
+  boomer(boom); // Frame foo
+}
+
+void _start(void) {
+  char F = '_';
+  foo(0, bar); // Frame _start
+}
Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -269,7 +269,7 @@
 self.dbg.DeleteTarget(target)
 
 @skipIf(triple='^mips')
-@skipIfLLVMTargetMissing("AArch64")
+#@skipIfLLVMTargetMissing("AArch64")
 def test_aarch64_regs(self):
 # check 64 bit ARM core files
 target = self.dbg.CreateTarget(None)
@@ -323,6 +323,47 @@
 
 self.expect("register read --all")
 
+#@skipIf(triple='^mips')
+#@skipIfLLVMTargetMissing("AArch64")
+def test_aarch64_sve_regs(self):
+# check 64 bit ARM core files
+target = self.dbg.CreateTarget(None)
+self.assertTrue(target, VALID_TARGET)
+process = target.LoadCore("linux-aarch64-sve.core")
+
+values = {}
+values["fp"] = "0xfc1ff4f0"
+values["lr"] = "0x00400170"
+values["sp"] = "0xfc1ff4d0"
+values["pc"] = "0x0040013c"
+values["v0"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40}"
+values["v1"] = "{0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41}"
+values["v2"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["v3"] = "{0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41}"
+values["s0"] = "7.5"
+values["s1"] = "11.5"
+values["s2"] = "0"
+values["s3"] = "15.5"
+values["d0"] = "65536.0158538818"
+values["d1"] = "1572864.25476074"
+values["d2"] = "0"
+values["d3"] = "25165828.0917969"
+values["vg"] = "0x0004"
+values["z0"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40 0x00 0x00 0xf0 0x40}"
+values["z1"] = "{0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41 0x00 0x00 0x38 0x41}"
+values["z2"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+values["z3"] = "{0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 0x00 0x00 0x78 0x41 

[Lldb-commits] [PATCH] D83541: Remove Linux sysroot dependencies of SVE PT macros

2020-07-10 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added subscribers: kristof.beyls, tschuett.
Herald added a reviewer: rengolin.

SVE elf note data requires SVE PT macros for reading writing data. Same macros 
are used by Linux ptrace SVE register access. 
This patch makes necessary changes to 
lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h in order to 
make them sysroot independent.


https://reviews.llvm.org/D83541

Files:
  lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h


Index: lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
===
--- lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
+++ lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
@@ -9,12 +9,17 @@
 #ifndef lldb_LinuxPTraceDefines_arm64sve_h
 #define lldb_LinuxPTraceDefines_arm64sve_h
 
+struct _aarch64_context {
+  uint16_t magic;
+  uint16_t size;
+};
+
 #define SVE_MAGIC 0x53564501
 
 struct sve_context {
-  struct _aarch64_ctx head;
-  __u16 vl;
-  __u16 __reserved[3];
+  struct _aarch64_context head;
+  uint16_t vl;
+  uint16_t __reserved[3];
 };
 
 /*
@@ -92,8 +97,8 @@
  * Additional data might be appended in the future.
  */
 
-#define SVE_SIG_ZREG_SIZE(vq) ((__u32)(vq)*SVE_VQ_BYTES)
-#define SVE_SIG_PREG_SIZE(vq) ((__u32)(vq) * (SVE_VQ_BYTES / 8))
+#define SVE_SIG_ZREG_SIZE(vq) ((uint32_t)(vq)*SVE_VQ_BYTES)
+#define SVE_SIG_PREG_SIZE(vq) ((uint32_t)(vq) * (SVE_VQ_BYTES / 8))
 #define SVE_SIG_FFR_SIZE(vq) SVE_SIG_PREG_SIZE(vq)
 
 #define SVE_SIG_REGS_OFFSET
\
@@ -123,12 +128,12 @@
 /* SVE/FP/SIMD state (NT_ARM_SVE) */
 
 struct user_sve_header {
-  __u32 size; /* total meaningful regset content in bytes */
-  __u32 max_size; /* maxmium possible size for this thread */
-  __u16 vl;   /* current vector length */
-  __u16 max_vl;   /* maximum possible vector length */
-  __u16 flags;
-  __u16 __reserved;
+  uint32_t size; /* total meaningful regset content in bytes */
+  uint32_t max_size; /* maxmium possible size for this thread */
+  uint16_t vl;   /* current vector length */
+  uint16_t max_vl;   /* maximum possible vector length */
+  uint16_t flags;
+  uint16_t __reserved;
 };
 
 /* Definitions for user_sve_header.flags: */
@@ -206,8 +211,8 @@
 #define SVE_PT_SVE_ZREG_SIZE(vq) SVE_SIG_ZREG_SIZE(vq)
 #define SVE_PT_SVE_PREG_SIZE(vq) SVE_SIG_PREG_SIZE(vq)
 #define SVE_PT_SVE_FFR_SIZE(vq) SVE_SIG_FFR_SIZE(vq)
-#define SVE_PT_SVE_FPSR_SIZE sizeof(__u32)
-#define SVE_PT_SVE_FPCR_SIZE sizeof(__u32)
+#define SVE_PT_SVE_FPSR_SIZE sizeof(uint32_t)
+#define SVE_PT_SVE_FPCR_SIZE sizeof(uint32_t)
 
 #define __SVE_SIG_TO_PT(offset)
\
   ((offset)-SVE_SIG_REGS_OFFSET + SVE_PT_REGS_OFFSET)


Index: lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
===
--- lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
+++ lldb/source/Plugins/Process/Linux/LinuxPTraceDefines_arm64sve.h
@@ -9,12 +9,17 @@
 #ifndef lldb_LinuxPTraceDefines_arm64sve_h
 #define lldb_LinuxPTraceDefines_arm64sve_h
 
+struct _aarch64_context {
+  uint16_t magic;
+  uint16_t size;
+};
+
 #define SVE_MAGIC 0x53564501
 
 struct sve_context {
-  struct _aarch64_ctx head;
-  __u16 vl;
-  __u16 __reserved[3];
+  struct _aarch64_context head;
+  uint16_t vl;
+  uint16_t __reserved[3];
 };
 
 /*
@@ -92,8 +97,8 @@
  * Additional data might be appended in the future.
  */
 
-#define SVE_SIG_ZREG_SIZE(vq) ((__u32)(vq)*SVE_VQ_BYTES)
-#define SVE_SIG_PREG_SIZE(vq) ((__u32)(vq) * (SVE_VQ_BYTES / 8))
+#define SVE_SIG_ZREG_SIZE(vq) ((uint32_t)(vq)*SVE_VQ_BYTES)
+#define SVE_SIG_PREG_SIZE(vq) ((uint32_t)(vq) * (SVE_VQ_BYTES / 8))
 #define SVE_SIG_FFR_SIZE(vq) SVE_SIG_PREG_SIZE(vq)
 
 #define SVE_SIG_REGS_OFFSET\
@@ -123,12 +128,12 @@
 /* SVE/FP/SIMD state (NT_ARM_SVE) */
 
 struct user_sve_header {
-  __u32 size; /* total meaningful regset content in bytes */
-  __u32 max_size; /* maxmium possible size for this thread */
-  __u16 vl;   /* current vector length */
-  __u16 max_vl;   /* maximum possible vector length */
-  __u16 flags;
-  __u16 __reserved;
+  uint32_t size; /* total meaningful regset content in bytes */
+  uint32_t max_size; /* maxmium possible size for this thread */
+  uint16_t vl;   /* current vector length */
+  uint16_t max_vl;   /* maximum possible vector length */
+  uint16_t flags;
+  uint16_t __reserved;
 };
 
 /* Definitions for user_sve_header.flags: */
@@ -206,8 +211,8 @@
 #define SVE_PT_SVE_ZREG_SIZE(vq) SVE_SIG_ZREG_SIZE(vq)
 #define SVE_PT_SVE_PREG_SIZE(vq) SVE_SIG_PREG_SIZE(vq)
 #define SVE_PT_SVE_FFR_SIZE(vq) SVE_SIG_FFR_SIZE(vq)
-#define SVE_PT_SVE_FPSR_SIZE sizeof(__u32)
-#define SVE_PT_SVE_FPCR_SIZE sizeof(__u32)
+#define SVE_PT_SVE_FPSR_SIZE 

[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-10 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D83446#2142564 , @JDevlieghere 
wrote:

> Seems like my thoughts got lost a bit with all the inline replies: we can 
> solve this particular issue by making `process connect` block in synchronous 
> mode. The fact that that's not happening today is a bug beyond the 
> reproducers. I don't think we should change the current behavior in 
> asynchronous mode. I think it's probably worthwhile to do that even if we 
> decide to add extra synchronization for the reproducers in which case this 
> would no longer be an example of a problem solved by the current patch.


I'm starting to understand why you say "process connect" is not 
blocking/synchronous. Although it is waiting for the "stopped" event to be sent 
to the public queue (here 
),
 it is not waiting for it to be processed by the public queue. The "process 
continue" command OTOH, is kind of waiting for the public queue processing to 
finish. I say only "kind of" because it does not actually process it on the 
_real_ public queue -- instead it "hijacks" the events and processes then on 
its own little event loop 
.
 The part that I got wrong was that I was assuming that the hijacked event loop 
would rebroadcast the stop event to the real public queue. In fact, it doesn't, 
which means that the stop event processing is completely finished by the time 
that the "process continue" command returns.

It seems reasonable that "process connect" (and it's SB equivalent) should 
behave the same way. This would also explain why so many of the "gdb-client" 
tests need to use `lldbutil.expect_state_changes` when connecting to the mock 
server, even though they are running in synchronous mode (the only other tests 
using this function explicitly choose to use the async mode).

For async mode, we can say that the user needs to ensure that the event is 
handled before he can issue other commands -- this is the same situation as 
with "process continue". Of course, in the asynch case, the reproducers are 
"the user" and they will need to do something about this. But maybe we don't 
need to cross that bridge right now?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D83446



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


[Lldb-commits] [lldb] a65da5f - [LLDB] Update AArch64 Dwarf and EH frame register numbers

2020-07-10 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2020-07-10T11:45:39+05:00
New Revision: a65da5f5924fbb7bad28bbb397e3e9a94959df4c

URL: 
https://github.com/llvm/llvm-project/commit/a65da5f5924fbb7bad28bbb397e3e9a94959df4c
DIFF: 
https://github.com/llvm/llvm-project/commit/a65da5f5924fbb7bad28bbb397e3e9a94959df4c.diff

LOG: [LLDB] Update AArch64 Dwarf and EH frame register numbers

This patch updates ARM64_ehframe_Registers.h and ARM64_DWARF_Registers.h
with latest register numbers in line with AArch64 SVE support.

For refernce take a look at "DWARF for the ARM® 64-bit Architecture (AArch64)
with SVE support" manual from Arm.
Version used: abi_sve_aadwarf_100985__00_en.pdf

Added: 


Modified: 
lldb/source/Utility/ARM64_DWARF_Registers.h
lldb/source/Utility/ARM64_ehframe_Registers.h

Removed: 




diff  --git a/lldb/source/Utility/ARM64_DWARF_Registers.h 
b/lldb/source/Utility/ARM64_DWARF_Registers.h
index 1bb2ba071f95..ed8ff722088d 100644
--- a/lldb/source/Utility/ARM64_DWARF_Registers.h
+++ b/lldb/source/Utility/ARM64_DWARF_Registers.h
@@ -51,7 +51,31 @@ enum {
   sp = x31,
   pc = 32,
   cpsr = 33,
-  // 34-63 reserved
+  // 34-45 reserved
+
+  // 64-bit SVE Vector granule pseudo register
+  vg = 46,
+
+  // VG ́8-bit SVE first fault register
+  ffr = 47,
+
+  // VG x ́8-bit SVE predicate registers
+  p0 = 48,
+  p1,
+  p2,
+  p3,
+  p4,
+  p5,
+  p6,
+  p7,
+  p8,
+  p9,
+  p10,
+  p11,
+  p12,
+  p13,
+  p14,
+  p15,
 
   // V0-V31 (128 bit vector registers)
   v0 = 64,
@@ -85,9 +109,41 @@ enum {
   v28,
   v29,
   v30,
-  v31
+  v31,
 
-  // 96-127 reserved
+  // VG ́64-bit SVE vector registers
+  z0 = 96,
+  z1,
+  z2,
+  z3,
+  z4,
+  z5,
+  z6,
+  z7,
+  z8,
+  z9,
+  z10,
+  z11,
+  z12,
+  z13,
+  z14,
+  z15,
+  z16,
+  z17,
+  z18,
+  z19,
+  z20,
+  z21,
+  z22,
+  z23,
+  z24,
+  z25,
+  z26,
+  z27,
+  z28,
+  z29,
+  z30,
+  z31
 };
 
 } // namespace arm64_dwarf

diff  --git a/lldb/source/Utility/ARM64_ehframe_Registers.h 
b/lldb/source/Utility/ARM64_ehframe_Registers.h
index 3e7baf98e148..c235891ec015 100644
--- a/lldb/source/Utility/ARM64_ehframe_Registers.h
+++ b/lldb/source/Utility/ARM64_ehframe_Registers.h
@@ -49,10 +49,34 @@ enum {
   lr, // aka x30
   sp, // aka x31 aka wzr
   pc, // value is 32
-  cpsr
-};
+  cpsr,
+  // 34-45 reserved
 
-enum {
+  // 64-bit SVE Vector granule pseudo register
+  vg = 46,
+
+  // VG ́8-bit SVE first fault register
+  ffr = 47,
+
+  // VG x ́8-bit SVE predicate registers
+  p0 = 48,
+  p1,
+  p2,
+  p3,
+  p4,
+  p5,
+  p6,
+  p7,
+  p8,
+  p9,
+  p10,
+  p11,
+  p12,
+  p13,
+  p14,
+  p15,
+
+  // V0-V31 (128 bit vector registers)
   v0 = 64,
   v1,
   v2,
@@ -84,7 +108,41 @@ enum {
   v28,
   v29,
   v30,
-  v31 // 95
+  v31,
+
+  // VG ́64-bit SVE vector registers
+  z0 = 96,
+  z1,
+  z2,
+  z3,
+  z4,
+  z5,
+  z6,
+  z7,
+  z8,
+  z9,
+  z10,
+  z11,
+  z12,
+  z13,
+  z14,
+  z15,
+  z16,
+  z17,
+  z18,
+  z19,
+  z20,
+  z21,
+  z22,
+  z23,
+  z24,
+  z25,
+  z26,
+  z27,
+  z28,
+  z29,
+  z30,
+  z31
 };
 }
 



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