[Lldb-commits] [lldb] 39ea3ce - [lldb] Disable TestSimulatorPlatform.py because it's causing a SIGHUP

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-04-21T20:00:51-07:00
New Revision: 39ea3ceda31cd02ef7e29bbeca74271feed3fc53

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

LOG: [lldb] Disable TestSimulatorPlatform.py because it's causing a SIGHUP

Ever since Dave Zarzycki's patch to sort test start times based on prior
test timing data (https://reviews.llvm.org/D98179) the test suite aborts
with a SIGHUP. I don't believe his patch is to blame, but rather
uncovers an preexisting issue by making test runs more deterministic.

I was able to narrow down the issue to TestSimulatorPlatform.py. The
issue also manifests itself on the standalone bot on GreenDragon [1].
This patch disables the test until we can figure this out.

[1] http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-standalone/

rdar://76995109

Added: 


Modified: 
lldb/test/API/macosx/simulator/TestSimulatorPlatform.py

Removed: 




diff  --git a/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py 
b/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
index 3deb37742b18..301919dea3b2 100644
--- a/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
+++ b/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
@@ -42,7 +42,7 @@ def check_debugserver(self, log, expected_platform, 
expected_version):
 if expected_version:
 self.assertEquals(aout_info['min_version_os_sdk'], 
expected_version)
 
-
+@skipIf(bugnumber="rdar://76995109")
 def run_with(self, arch, os, vers, env, expected_load_command):
 env_list = [env] if env else []
 triple = '-'.join([arch, 'apple', os + vers] + env_list)



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

[Lldb-commits] [lldb] 875654f - Fix VSCode/TestOptions.test

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-04-21T15:46:26-07:00
New Revision: 875654f897ac01ad91a1a5f5dc23d07ac548a0e0

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

LOG: Fix VSCode/TestOptions.test

Found by https://lab.llvm.org/buildbot/#/builders/96/builds/6936

Added: 


Modified: 
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 95074bb1c984..5f9e96bf6f50 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3158,6 +3158,11 @@ int main(int argc, char *argv[]) {
   llvm::ArrayRef ArgsArr = llvm::makeArrayRef(argv + 1, argc);
   llvm::opt::InputArgList input_args = T.ParseArgs(ArgsArr, MAI, MAC);
 
+  if (input_args.hasArg(OPT_help)) {
+printHelp(T, llvm::sys::path::filename(argv[0]));
+return EXIT_SUCCESS;
+  }
+
   if (llvm::opt::Arg *target_arg = input_args.getLastArg(OPT_launch_target)) {
 if (llvm::opt::Arg *comm_file = input_args.getLastArg(OPT_comm_file)) {
   int target_args_pos = argc;
@@ -3189,11 +3194,6 @@ int main(int argc, char *argv[]) {
 
   int portno = -1;
 
-  if (input_args.hasArg(OPT_help)) {
-printHelp(T, llvm::sys::path::filename(argv[0]));
-return EXIT_SUCCESS;
-  }
-
   if (auto *arg = input_args.getLastArg(OPT_port)) {
 auto optarg = arg->getValue();
 char *remainder;



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

[Lldb-commits] [lldb] c4a83c4 - Fix TestVSCode_runInTerminal

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-04-21T15:20:47-07:00
New Revision: c4a83c4e69f1c858df787723df4923c41a23e00d

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

LOG: Fix TestVSCode_runInTerminal

It failed in https://lab.llvm.org/buildbot/#/builders/68/builds/10912

And it was caused due to https://reviews.llvm.org/rG64f47c1e58a1

Added: 


Modified: 
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index b810bf738526..95074bb1c984 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -3149,9 +3149,6 @@ int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  // stdout/stderr redirection to the IDE's console
-  int new_stdout_fd = SetupStdoutStderrRedirection();
-
   llvm::SmallString<256> program_path(argv[0]);
   llvm::sys::fs::make_absolute(program_path);
   g_vsc.debug_adaptor_path = program_path.str().str();
@@ -3178,6 +3175,9 @@ int main(int argc, char *argv[]) {
 }
   }
 
+  // stdout/stderr redirection to the IDE's console
+  int new_stdout_fd = SetupStdoutStderrRedirection();
+
   // Initialize LLDB first before we do anything.
   lldb::SBDebugger::Initialize();
 



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

[Lldb-commits] [lldb] c9a0754 - [lldb-vscode] Distinguish shadowed variables in the scopes request

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-04-21T15:09:39-07:00
New Revision: c9a0754b443ba73623574db880a7b0b14826fedb

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

LOG: [lldb-vscode] Distinguish shadowed variables in the scopes request

VSCode doesn't render multiple variables with the same name in the variables 
view. It only renders one of them. This is a situation that happens often when 
there are shadowed variables.
The nodejs debugger solves this by adding a number suffix to the variable, e.g. 
"x", "x2", "x3" are the different x variables in nested blocks.

In this patch I'm doing something similar, but the suffix is " @ 
), e.g. "x @ main.cpp:17", "x @ main.cpp:21". The fallback 
would be an address if the source and line information is not present, which 
should be rare.

This fix is only needed for globals and locals. Children of variables don't 
suffer of this problem.

When there are shadowed variables
{F16182150}

Without shadowed variables
{F16182152}

Modifying these variables through the UI works

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
lldb/test/API/tools/lldb-vscode/variables/main.cpp
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/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 5f5e4b4021073..0a55fc0ead1e4 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -85,7 +85,6 @@ def verify_breakpoint_hit(self, breakpoint_ids):
 # the right breakpoint matches and not worry about the actual
 # location.
 description = body['description']
-print("description: %s" % (description))
 for breakpoint_id in breakpoint_ids:
 match_desc = 'breakpoint %s.' % (breakpoint_id)
 if match_desc in description:

diff  --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py 
b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
index 12fa3e1d30572..765cfbe6ed5a1 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -110,6 +110,9 @@ def test_scopes_variables_setVariable_evaluate(self):
 'y': {'equals': {'type': 'int', 'value': '22'}},
 'buffer': {'children': buffer_children}
 }
+},
+'x': {
+'equals': {'type': 'int'}
 }
 }
 verify_globals = {
@@ -221,3 +224,61 @@ def test_scopes_variables_setVariable_evaluate(self):
 value = response['body']['variables'][0]['value']
 self.assertEqual(value, '111',
 'verify pt.x got set to 111 (111 != %s)' % (value))
+
+# We check shadowed variables and that a new get_local_variables 
request
+# gets the right data
+breakpoint2_line = line_number(source, '// breakpoint 2')
+lines = [breakpoint2_line]
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(len(breakpoint_ids), len(lines),
+"expect correct number of breakpoints")
+self.continue_to_breakpoints(breakpoint_ids)
+
+verify_locals['argc']['equals']['value'] = '123'
+verify_locals['pt']['children']['x']['equals']['value'] = '111'
+verify_locals['x @ main.cpp:17'] = {'equals': {'type': 'int', 'value': 
'89'}}
+verify_locals['x @ main.cpp:19'] = {'equals': {'type': 'int', 'value': 
'42'}}
+verify_locals['x @ main.cpp:21'] = {'equals': {'type': 'int', 'value': 
'72'}}
+
+self.verify_variables(verify_locals, self.vscode.get_local_variables())
+
+# Now we verify that we correctly change the name of a variable with 
and without 
diff erentiator suffix
+self.assertFalse(self.vscode.request_setVariable(1, "x2", 
9)['success'])
+self.assertFalse(self.vscode.request_setVariable(1, "x @ main.cpp:0", 
9)['success'])
+
+self.assertTrue(self.vscode.request_setVariable(1, "x @ main.cpp:17", 
17)['success'])
+self.assertTrue(self.vscode.request_setVariable(1, "x @ main.cpp:19", 
19)['success'])
+self.assertTrue(self.vscode.request_setVariable(1, "x @ main.cpp:21", 

[Lldb-commits] [PATCH] D99989: [lldb-vscode] Distinguish shadowed variables in the scopes request

[walter erquinigo via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc9a0754b443b: [lldb-vscode] Distinguish shadowed variables 
in the scopes request (authored by wallace).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D99989?vs=336945=339403#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99989

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/test/API/tools/lldb-vscode/variables/main.cpp
  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
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "llvm/ADT/ArrayRef.h"
@@ -2716,7 +2717,9 @@
   // This is a reference to the containing variable/scope
   const auto variablesReference =
   GetUnsigned(arguments, "variablesReference", 0);
-  const auto name = GetString(arguments, "name");
+  llvm::StringRef name = GetString(arguments, "name");
+  bool is_duplicated_variable_name = name.find(" @") != llvm::StringRef::npos;
+
   const auto value = GetString(arguments, "value");
   // Set success to false just in case we don't find the variable by name
   response.try_emplace("success", false);
@@ -2758,14 +2761,10 @@
   break;
 }
 
-// Find the variable by name in the correct scope and hope we don't have
-// multiple variables with the same name. We search backwards because
-// the list of variables has the top most variables first and variables
-// in deeper scopes are last. This means we will catch the deepest
-// variable whose name matches which is probably what the user wants.
 for (int64_t i = end_idx - 1; i >= start_idx; --i) {
-  auto curr_variable = g_vsc.variables.GetValueAtIndex(i);
-  llvm::StringRef variable_name(curr_variable.GetName());
+  lldb::SBValue curr_variable = g_vsc.variables.GetValueAtIndex(i);
+  std::string variable_name = CreateUniqueVariableNameForDisplay(
+  curr_variable, is_duplicated_variable_name);
   if (variable_name == name) {
 variable = curr_variable;
 if (curr_variable.MightHaveChildren())
@@ -2774,6 +2773,9 @@
   }
 }
   } else {
+// This is not under the globals or locals scope, so there are no duplicated
+// names.
+
 // We have a named item within an actual variable so we need to find it
 // withing the container variable by name.
 const int64_t var_idx = VARREF_TO_VARIDX(variablesReference);
@@ -2810,6 +2812,8 @@
   EmplaceSafeString(body, "message", std::string(error.GetCString()));
 }
 response["success"] = llvm::json::Value(success);
+  } else {
+response["success"] = llvm::json::Value(false);
   }
 
   response.try_emplace("body", std::move(body));
@@ -2925,12 +2929,26 @@
   break;
 }
 const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
+
+// We first find out which variable names are duplicated
+std::unordered_map variable_name_counts;
+for (auto i = start_idx; i < end_idx; ++i) {
+  lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
+  if (!variable.IsValid())
+break;
+  variable_name_counts[variable.GetName()]++;
+}
+
+// Now we construct the result with unique display variable names
 for (auto i = start_idx; i < end_idx; ++i) {
   lldb::SBValue variable = g_vsc.variables.GetValueAtIndex(i);
+  const char *name = variable.GetName();
+
   if (!variable.IsValid())
 break;
-  variables.emplace_back(
-  CreateVariable(variable, VARIDX_TO_VARREF(i), i, hex));
+  variables.emplace_back(CreateVariable(variable, VARIDX_TO_VARREF(i), i,
+hex,
+variable_name_counts[name] > 1));
 }
   } else {
 // We are expanding a variable that has children, so we will return its
Index: lldb/tools/lldb-vscode/JSONUtils.h
===
--- lldb/tools/lldb-vscode/JSONUtils.h
+++ lldb/tools/lldb-vscode/JSONUtils.h
@@ -399,6 +399,14 @@
 /// definition outlined by Microsoft.
 llvm::json::Value CreateThreadStopped(lldb::SBThread , uint32_t stop_id);
 
+/// VSCode can't display two variables with the same name, so we need to
+/// distinguish them by using a suffix.
+///
+/// If the source and line information is present, we use it as the suffix.
+/// Otherwise, we fallback to the variable address or register 

[Lldb-commits] [lldb] 64f47c1 - [lldb-vscode] redirect stderr/stdout to the IDE's console

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-04-21T14:48:48-07:00
New Revision: 64f47c1e58a10de160ce3fb3afbc50c0243e2977

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

LOG: [lldb-vscode] redirect stderr/stdout to the IDE's console

In certain occasions times, like when LLDB is initializing and
evaluating the .lldbinit files, it tries to print to stderr and stdout
directly. This confuses the IDE with malformed data, as it talks to
lldb-vscode using stdin and stdout following the JSON RPC protocol. This
ends up terminating the debug session with the user unaware of what's
going on. There might be other situations in which this can happen, and
they will be harder to debug than the .lldbinit case.

After several discussions with @clayborg, @yinghuitan and @aadsm, we
realized that the best course of action is to simply redirect stdout and
stderr to the console, without modifying LLDB itself. This will prove to
be resilient to future bugs or features.

I made the simplest possible redirection logic I could come up with. It
only works for POSIX, and to make it work with Windows should be merely
changing pipe and dup2 for the windows equivalents like _pipe and _dup2.
Sadly I don't have a Windows machine, so I'll do it later once my office
reopens, or maybe someone else can do it.

I'm intentionally not adding a stop-redirecting logic, as I don't see it
useful for the lldb-vscode case (why would we want to do that, really?).

I added a test.

Note: this is a simpler version of D80659. I first tried to implement a
RIIA version of it, but it was problematic to manage the state of the
thread and reverting the redirection came with some non trivial
complexities, like what to do with unflushed data after the debug
session has finished on the IDE's side.

Added: 
lldb/test/API/tools/lldb-vscode/console/TestVSCode_redirection_to_console.py
lldb/tools/lldb-vscode/OutputRedirector.cpp
lldb/tools/lldb-vscode/OutputRedirector.h

Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/tools/lldb-vscode/CMakeLists.txt
lldb/tools/lldb-vscode/IOStream.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index d073e3692dd30..5f5e4b4021073 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -9,18 +9,18 @@ class VSCodeTestCaseBase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
-def create_debug_adaptor(self):
+def create_debug_adaptor(self, lldbVSCodeEnv=None):
 '''Create the Visual Studio Code debug adaptor'''
 self.assertTrue(os.path.exists(self.lldbVSCodeExec),
 'lldb-vscode must exist')
 log_file_path = self.getBuildArtifact('vscode.txt')
 self.vscode = vscode.DebugAdaptor(
 executable=self.lldbVSCodeExec, init_commands=self.setUpCommands(),
-log_file=log_file_path)
+log_file=log_file_path, env=lldbVSCodeEnv)
 
-def build_and_create_debug_adaptor(self):
+def build_and_create_debug_adaptor(self, lldbVSCodeEnv=None):
 self.build()
-self.create_debug_adaptor()
+self.create_debug_adaptor(lldbVSCodeEnv)
 
 def set_source_breakpoints(self, source_path, lines, condition=None,
hitCondition=None):
@@ -343,11 +343,12 @@ def build_and_launch(self, program, args=None, cwd=None, 
env=None,
  stopCommands=None, exitCommands=None,
  terminateCommands=None, sourcePath=None,
  debuggerRoot=None, runInTerminal=False,
- disconnectAutomatically=True, postRunCommands=None):
+ disconnectAutomatically=True, postRunCommands=None,
+ lldbVSCodeEnv=None):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and launch the process.
 '''
-self.build_and_create_debug_adaptor()
+self.build_and_create_debug_adaptor(lldbVSCodeEnv)
 self.assertTrue(os.path.exists(program), 'executable must exist')
 
 return self.launch(program, args, cwd, env, stopOnEntry, disableASLR,

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 02e9b65e9afe2..9eeebdbf76091 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ 

[Lldb-commits] [lldb] 12a2507 - Fix TestVSCode_launch test

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-04-21T14:33:34-07:00
New Revision: 12a25076463d0b04b535d07982d7420971bcea3e

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

LOG: Fix TestVSCode_launch test

Broken in https://lab.llvm.org/buildbot/#/builders/96/builds/6933

We don't really need to run this test on arm, but would be worth fixing
it later.

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py 
b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
index 6f65a7250f1e6..c40e7e2dd4aa7 100644
--- a/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ b/lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -293,6 +293,7 @@ def test_environment(self):
 
 @skipIfWindows
 @skipIfRemote
+@skipIf(archs=["arm", "aarch64"]) # failed run 
https://lab.llvm.org/buildbot/#/builders/96/builds/6933
 def test_commands(self):
 '''
 Tests the "initCommands", "preRunCommands", "stopCommands",



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

[Lldb-commits] [PATCH] D99974: [lldb-vscode] redirect stderr/stdout to the IDE's console

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 339385.
wallace added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99974

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/console/TestVSCode_redirection_to_console.py
  lldb/tools/lldb-vscode/CMakeLists.txt
  lldb/tools/lldb-vscode/IOStream.cpp
  lldb/tools/lldb-vscode/OutputRedirector.cpp
  lldb/tools/lldb-vscode/OutputRedirector.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
@@ -57,6 +57,7 @@
 
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
+#include "OutputRedirector.h"
 
 #if defined(_WIN32)
 #ifndef PATH_MAX
@@ -3090,10 +3091,49 @@
 #endif
 }
 
+/// used only by TestVSCode_redirection_to_console.py
+void redirection_test() {
+  printf("stdout message\n");
+  fprintf(stderr, "stderr message\n");
+  fflush(stdout);
+  fflush(stderr);
+}
+
+/// Redirect stdout and stderr fo the IDE's console output.
+///
+/// Errors in this operation will be printed to the log file and the IDE's
+/// console output as well.
+///
+/// \return
+/// A fd pointing to the original stdout.
+int SetupStdoutStderrRedirection() {
+  int new_stdout_fd = dup(fileno(stdout));
+  auto stdout_err_redirector_callback = [&](llvm::StringRef data) {
+g_vsc.SendOutput(OutputType::Console, data);
+  };
+
+  for (int fd : {fileno(stdout), fileno(stderr)}) {
+if (llvm::Error err = RedirectFd(fd, stdout_err_redirector_callback)) {
+  std::string error_message = llvm::toString(std::move(err));
+  if (g_vsc.log)
+*g_vsc.log << error_message << std::endl;
+  stdout_err_redirector_callback(error_message);
+}
+  }
+
+  /// used only by TestVSCode_redirection_to_console.py
+  if (getenv("LLDB_VSCODE_TEST_STDOUT_STDERR_REDIRECTION") != nullptr)
+redirection_test();
+  return new_stdout_fd;
+}
+
 int main(int argc, char *argv[]) {
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
+  // stdout/stderr redirection to the IDE's console
+  int new_stdout_fd = SetupStdoutStderrRedirection();
+
   llvm::SmallString<256> program_path(argv[0]);
   llvm::sys::fs::make_absolute(program_path);
   g_vsc.debug_adaptor_path = program_path.str().str();
@@ -3163,9 +3203,9 @@
 }
   } else {
 g_vsc.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
-g_vsc.output.descriptor =
-StreamDescriptor::from_file(fileno(stdout), false);
+g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
   }
+
   uint32_t packet_idx = 0;
   while (!g_vsc.sent_terminated_event) {
 llvm::json::Object object;
Index: lldb/tools/lldb-vscode/OutputRedirector.h
===
--- /dev/null
+++ lldb/tools/lldb-vscode/OutputRedirector.h
@@ -0,0 +1,28 @@
+//===-- OutputRedirector.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#ifndef LLDB_TOOLS_LLDB_VSCODE_OUTPUT_REDIRECTOR_H
+#define LLDB_TOOLS_LLDB_VSCODE_OUTPUT_REDIRECTOR_H
+
+#include 
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+namespace lldb_vscode {
+
+/// Redirects the output of a given file descriptor to a callback.
+///
+/// \return
+/// \a Error::success if the redirection was set up correctly, or an error
+/// otherwise.
+llvm::Error RedirectFd(int fd, std::function callback);
+
+} // namespace lldb_vscode
+
+#endif // LLDB_TOOLS_LLDB_VSCODE_OUTPUT_REDIRECTOR_H
Index: lldb/tools/lldb-vscode/OutputRedirector.cpp
===
--- /dev/null
+++ lldb/tools/lldb-vscode/OutputRedirector.cpp
@@ -0,0 +1,56 @@
+//===-- OutputRedirector.cpp ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#if !defined(_WIN32)
+#include 
+#endif
+
+#include "OutputRedirector.h"
+
+using namespace llvm;
+
+namespace lldb_vscode {
+
+Error RedirectFd(int fd, std::function callback) {
+#if !defined(_WIN32)
+  int 

[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D96236#2705715 , @dblaikie wrote:

> 

(similar idea)

In D96236#2705920 , @clayborg wrote:

> 



> Is there really not way to ask the contained "U" DWARFUnit member for the 
> main unit?

There is no such way because one DWARFUnit can be used by multiple (in practice 
by two - DW_AT_language C plus C++) main units. You did ask it in 
D73206#1837138 .

> Can we add a new "DWARFUnit *MU;" to the DWARFDie class?

Been there, done that. It is the option 1 from text below. It cannot be applied 
(it was against the DWZ patch that time), just for an idea how it did work: 
https://people.redhat.com/jkratoch/dwz-DWARFUnitPair-2019-10-30.patch

   class DWARFBaseDIE {
  -  DWARFUnit *m_cu;
  +  DWARFUnitPair m_cu;
 DWARFDebugInfoEntry *m_die;
   };
  +class DWARFUnitPair {
  ...
  +  DWARFUnit *m_cu;
  +  // For non-DWZ setups it is either equal to 'm_cu' or nullptr if 'm_cu' is 
a DWARFTypeUnit.
  +  DWARFCompileUnit *m_main_cu;
  +};

It was discussed in D73206  where was 
originally a different initial comment:

-

[...] proposing multiple possibilities how to satisfy DWZ's need for 
`DWARFDIE::m_main_cu`, could you choose one?

1. `DWARFDIE` 16B->24B does not matter as `DWARFDIE` is not stored anywhere - 
maybe one could use this patch in such case.
2. PointerUnion for DWARFDIE::m_cu so that DWARFDIE remains 16B without DWZ.

  class DWARFBaseDIE {
llvm::PointerUnion m_cu;
DWARFDebugInfoEntry *m_die;
  };
  class DWARFUnitPair {`
DWARFUnit *m_cu;
DWARFCompileUnit *m_main_cu;
  };

3. (removed as I do not find nowadays enough to store just `DW_LANG_*` instead 
of the main unit pointer.)

-

The option 2 is fine as the double dereference performance impact (if any) 
affects only LLDB run on DWZ systems. And still LLDB with this little 
performance hit is much faster than the only existing debugger GDB for DWZ 
systems.

@labath did disagree in D73206#1836018 
 regarding the merge with LLVM DWARF. 
I cannot much talk about the merge as I do not know much the LLVM counterpart 
and so I haven't yet made any plan in my mind how to merge it.

Thanks for the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96236

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

[Lldb-commits] [PATCH] D100340: [lldb-vscode] Add postRunCommands

[walter erquinigo via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG79fbbeb41280: [lldb-vscode] Add postRunCommands (authored by 
wallace).

Changed prior to commit:
  https://reviews.llvm.org/D100340?vs=336942=339374#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100340

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -161,6 +161,11 @@
 "description": "Commands executed just before the program is launched.",
 "default": []
 			},
+			"postRunCommands": {
+"type": "array",
+"description": "Commands executed just as soon as the program is successfully launched when it's in a stopped state prior to any automatic continuation.",
+"default": []
+			},
 			"launchCommands": {
 "type": "array",
 "description": "Custom commands that are executed instead of launching a process. A target will be created with the launch arguments prior to executing these commands. The commands may optionally create a new target and must perform a launch. A valid process must exist after these commands complete or the \"launch\" will fail.",
@@ -237,6 +242,11 @@
 "description": "Commands executed just before the program is attached to.",
 "default": []
 			},
+			"postRunCommands": {
+"type": "array",
+"description": "Commands executed just as soon as the program is successfully attached when it's in a stopped state prior to any automatic continuation.",
+"default": []
+			},
 			"stopCommands": {
 "type": "array",
 "description": "Commands executed each time the program stops.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -570,6 +570,8 @@
   llvm::StringRef core_file = GetString(arguments, "coreFile");
   g_vsc.stop_at_entry =
   core_file.empty() ? GetBoolean(arguments, "stopOnEntry", false) : true;
+  std::vector postRunCommands =
+  GetStrings(arguments, "postRunCommands");
   const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
 
   // This is a hack for loading DWARF in .o files on Mac where the .o files
@@ -638,12 +640,14 @@
   if (error.Fail()) {
 response["success"] = llvm::json::Value(false);
 EmplaceSafeString(response, "message", std::string(error.GetCString()));
+  } else {
+g_vsc.RunLLDBCommands("Running postRunCommands:", postRunCommands);
   }
+
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
   if (error.Success()) {
 SendProcessEvent(Attach);
 g_vsc.SendJSON(CreateEventObject("initialized"));
-// SendThreadStoppedEvent();
   }
 }
 
@@ -1608,6 +1612,8 @@
   g_vsc.exit_commands = GetStrings(arguments, "exitCommands");
   g_vsc.terminate_commands = GetStrings(arguments, "terminateCommands");
   auto launchCommands = GetStrings(arguments, "launchCommands");
+  std::vector postRunCommands =
+  GetStrings(arguments, "postRunCommands");
   g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
   const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
 
@@ -1689,7 +1695,10 @@
   if (error.Fail()) {
 response["success"] = llvm::json::Value(false);
 EmplaceSafeString(response, "message", std::string(error.GetCString()));
+  } else {
+g_vsc.RunLLDBCommands("Running postRunCommands:", postRunCommands);
   }
+
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 
   if (g_vsc.is_attach)
Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -313,12 +313,14 @@
 program = self.getBuildArtifact("a.out")
 initCommands = ['target list', 'platform list']
 preRunCommands = ['image list a.out', 'image dump sections a.out']
+postRunCommands = ['help trace', 'help process trace']
 stopCommands = ['frame variable', 'bt']
 exitCommands = ['expr 2+3', 'expr 3+4']
 terminateCommands = ['expr 4+2']
 self.build_and_launch(program,
   initCommands=initCommands,

[Lldb-commits] [lldb] 79fbbeb - [lldb-vscode] Add postRunCommands

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2021-04-21T13:51:30-07:00
New Revision: 79fbbeb41280ed170a387ad925c5397c0af32364

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

LOG: [lldb-vscode] Add postRunCommands

This diff ass postRunCommands, which are the counterpart of the preRunCommands. 
TThey will be executed right after the target is launched or attached 
correctly, which means that the targets can assume that the target is running.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/attach/TestVSCode_attach.py
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
lldb/tools/lldb-vscode/lldb-vscode.cpp
lldb/tools/lldb-vscode/package.json

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index 7ddb3164cb3db..d073e3692dd30 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -251,7 +251,8 @@ def continue_to_exit(self, exitCode=0):
 def attach(self, program=None, pid=None, waitFor=None, trace=None,
initCommands=None, preRunCommands=None, stopCommands=None,
exitCommands=None, attachCommands=None, coreFile=None,
-   disconnectAutomatically=True, terminateCommands=None):
+   disconnectAutomatically=True, terminateCommands=None,
+   postRunCommands=None):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and attach to the process.
 '''
@@ -271,7 +272,7 @@ def cleanup():
 initCommands=initCommands, preRunCommands=preRunCommands,
 stopCommands=stopCommands, exitCommands=exitCommands,
 attachCommands=attachCommands, terminateCommands=terminateCommands,
-coreFile=coreFile)
+coreFile=coreFile, postRunCommands=postRunCommands)
 if not (response and response['success']):
 self.assertTrue(response['success'],
 'attach failed (%s)' % (response['message']))
@@ -283,7 +284,7 @@ def launch(self, program=None, args=None, cwd=None, 
env=None,
stopCommands=None, exitCommands=None, terminateCommands=None,
sourcePath=None, debuggerRoot=None, launchCommands=None,
sourceMap=None, disconnectAutomatically=True, 
runInTerminal=False,
-   expectFailure=False):
+   expectFailure=False, postRunCommands=None):
 '''Sending launch request to vscode
 '''
 
@@ -319,7 +320,8 @@ def cleanup():
 launchCommands=launchCommands,
 sourceMap=sourceMap,
 runInTerminal=runInTerminal,
-expectFailure=expectFailure)
+expectFailure=expectFailure,
+postRunCommands=postRunCommands)
 
 if expectFailure:
 return response
@@ -341,7 +343,7 @@ def build_and_launch(self, program, args=None, cwd=None, 
env=None,
  stopCommands=None, exitCommands=None,
  terminateCommands=None, sourcePath=None,
  debuggerRoot=None, runInTerminal=False,
- disconnectAutomatically=True):
+ disconnectAutomatically=True, postRunCommands=None):
 '''Build the default Makefile target, create the VSCode debug adaptor,
and launch the process.
 '''
@@ -352,4 +354,5 @@ def build_and_launch(self, program, args=None, cwd=None, 
env=None,
 disableSTDIO, shellExpandArguments, trace,
 initCommands, preRunCommands, stopCommands, exitCommands,
 terminateCommands, sourcePath, debuggerRoot, 
runInTerminal=runInTerminal,
-disconnectAutomatically=disconnectAutomatically)
+disconnectAutomatically=disconnectAutomatically,
+postRunCommands=postRunCommands)

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 926a63aeb239e..02e9b65e9afe2 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -494,7 +494,7 @@ def request_attach(self, program=None, pid=None, 
waitFor=None, trace=None,
initCommands=None, preRunCommands=None,
 

[Lldb-commits] [lldb] 5d1c43f - [lldb] Use the compiler from the SDK in simulator tests

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-04-21T13:22:58-07:00
New Revision: 5d1c43f333c2327be61604dc90ea675f0d1e6913

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

LOG: [lldb] Use the compiler from the SDK in simulator tests

Use the clang compiler from the SDK to build the simulator test programs
to ensure we pick up the correct libc++.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbutil.py
lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index 06a84c2554000..5fff3726a65e6 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -95,6 +95,11 @@ def get_xcode_sdk_root(sdk):
 ]).rstrip().decode('utf-8')
 
 
+def get_xcode_clang(sdk):
+return subprocess.check_output(['xcrun', '-sdk', sdk, '-f', 'clang'
+]).rstrip().decode("utf-8")
+
+
 # ===
 # Disassembly for an SBFunction or an SBSymbol object
 # ===

diff  --git a/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py 
b/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
index 8a68f8e410968..3deb37742b18d 100644
--- a/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
+++ b/lldb/test/API/macosx/simulator/TestSimulatorPlatform.py
@@ -57,10 +57,12 @@ def run_with(self, arch, os, vers, env, 
expected_load_command):
 version_min = '-m{}-version-min={}'.format(os, vers)
 
 sdk_root = lldbutil.get_xcode_sdk_root(sdk)
+clang = lldbutil.get_xcode_clang(sdk)
 
 self.build(
 dictionary={
 'ARCH': arch,
+'CC': clang,
 'ARCH_CFLAGS': '-target {} {}'.format(triple, version_min),
 'SDKROOT': sdk_root
 })

diff  --git a/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py 
b/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
index dc5785fd49a9e..01942344e0e10 100644
--- a/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
+++ b/lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
@@ -51,6 +51,7 @@ def check_simulator_ostype(self, sdk, platform_name, 
arch=platform.machine()):
 exe_name = 'test_simulator_platform_{}'.format(platform_name)
 sdkroot = lldbutil.get_xcode_sdk_root(sdk)
 vers = lldbutil.get_xcode_sdk_version(sdk)
+clang = lldbutil.get_xcode_clang(sdk)
 
 # Older versions of watchOS (<7.0) only support i386
 if platform_name == 'watchos':
@@ -63,6 +64,7 @@ def check_simulator_ostype(self, sdk, platform_name, 
arch=platform.machine()):
 self.build(
 dictionary={
 'EXE': exe_name,
+'CC': clang,
 'SDKROOT': sdkroot.strip(),
 'ARCH': arch,
 'ARCH_CFLAGS': '-target {} {}'.format(triple, version_min),



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

Re: [Lldb-commits] [lldb] cd64273 - [lldb/ELF] Fix IDs of synthetic eh_frame symbols

[Shafik Yaghmour via lldb-commits]

Having side effects in the argument to a function call just feels like a bad 
idea in general even if it seems obviously correct here e.g. 

  ++last_symbol_id,

Can’t we just do:

  uint64_t last_symbol_id =
  num_symbols ? symbol_table->SymbolAtIndex(num_symbols - 1)->GetID() + 1: 
1;

Thank you

> On Apr 21, 2021, at 2:25 AM, Pavel Labath via lldb-commits 
>  wrote:
> 
> 
> Author: Pavel Labath
> Date: 2021-04-21T11:24:43+02:00
> New Revision: cd64273f5ed39ec697ff1e20a1fe25ebd3502629
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/cd64273f5ed39ec697ff1e20a1fe25ebd3502629
> DIFF: 
> https://github.com/llvm/llvm-project/commit/cd64273f5ed39ec697ff1e20a1fe25ebd3502629.diff
> 
> LOG: [lldb/ELF] Fix IDs of synthetic eh_frame symbols
> 
> The code used the total number of symbols to create a symbol ID for the
> synthetic symbols. This is not correct because the IDs of real symbols
> can be higher than their total number, as we do not add all symbols (and
> in particular, we never add symbol zero, which is not a real symbol).
> 
> This meant we could have symbols with duplicate IDs, which caused
> problems if some relocations were referring to the duplicated IDs. This
> was the cause of the failure of the test D97786.
> 
> This patch fixes the code to use the ID of the highest (last) symbol
> instead.
> 
> Added: 
>lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
> 
> Modified: 
>lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> 
> Removed: 
> 
> 
> 
> 
> diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
> b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> index f30ed427f8535..6e94ab97992a1 100644
> --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
> @@ -2901,8 +2901,11 @@ void ObjectFileELF::ParseUnwindSymbols(Symtab 
> *symbol_table,
>   // recalculate the index first.
>   std::vector new_symbols;
> 
> -  eh_frame->ForEachFDEEntries([this, symbol_table, section_list, 
> _symbols](
> -  lldb::addr_t file_addr, uint32_t size, dw_offset_t) {
> +  size_t num_symbols = symbol_table->GetNumSymbols();
> +  uint64_t last_symbol_id =
> +  num_symbols ? symbol_table->SymbolAtIndex(num_symbols - 1)->GetID() : 
> 0;
> +  eh_frame->ForEachFDEEntries([&](lldb::addr_t file_addr, uint32_t size,
> +  dw_offset_t) {
> Symbol *symbol = symbol_table->FindSymbolAtFileAddress(file_addr);
> if (symbol) {
>   if (!symbol->GetByteSizeIsValid()) {
> @@ -2915,21 +2918,20 @@ void ObjectFileELF::ParseUnwindSymbols(Symtab 
> *symbol_table,
>   if (section_sp) {
> addr_t offset = file_addr - section_sp->GetFileAddress();
> const char *symbol_name = GetNextSyntheticSymbolName().GetCString();
> -uint64_t symbol_id = symbol_table->GetNumSymbols();
> Symbol eh_symbol(
> -symbol_id,   // Symbol table index.
> -symbol_name, // Symbol name.
> -eSymbolTypeCode, // Type of this symbol.
> -true,// Is this globally visible?
> -false,   // Is this symbol debug info?
> -false,   // Is this symbol a trampoline?
> -true,// Is this symbol artificial?
> -section_sp,  // Section in which this symbol is defined or 
> null.
> -offset,  // Offset in section or symbol value.
> -0, // Size:  Don't specify the size as an FDE can
> -false, // Size is valid: cover multiple symbols.
> -false, // Contains linker annotations?
> -0);// Symbol flags.
> +++last_symbol_id, // Symbol table index.
> +symbol_name,  // Symbol name.
> +eSymbolTypeCode,  // Type of this symbol.
> +true, // Is this globally visible?
> +false,// Is this symbol debug info?
> +false,// Is this symbol a trampoline?
> +true, // Is this symbol artificial?
> +section_sp, // Section in which this symbol is defined or null.
> +offset, // Offset in section or symbol value.
> +0,  // Size:  Don't specify the size as an FDE 
> can
> +false,  // Size is valid: cover multiple symbols.
> +false,  // Contains linker annotations?
> +0); // Symbol flags.
> new_symbols.push_back(eh_symbol);
>   }
> }
> 
> diff  --git a/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml 
> b/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
> new file mode 100644
> index 0..6178a45de1b59
> --- /dev/null
> +++ b/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
> @@ -0,0 +1,32 @@
> +# RUN: yaml2obj %s >%t
> +# RUN: %lldb %t -o "image dump 

[Lldb-commits] [PATCH] D100977: [lldb] Use forward type in pointer-to-member

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Were you able to reduce the reproducer you got to a test? If it's really the 
same bug as in D100180  then I guess a 
similar test case should trigger the issue (e.g., a class with a 
pointer-to-member member that references some type that is currently being 
parsed?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100977

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

[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb5e11f460b1: [lldb] Fix 
RichManglingContext::FromCxxMethodName() leak (authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100795

Files:
  lldb/include/lldb/Core/RichManglingContext.h
  lldb/source/Core/RichManglingContext.cpp


Index: lldb/source/Core/RichManglingContext.cpp
===
--- lldb/source/Core/RichManglingContext.cpp
+++ lldb/source/Core/RichManglingContext.cpp
@@ -19,7 +19,12 @@
 using namespace lldb_private;
 
 // RichManglingContext
-void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();
+}
+
+void RichManglingContext::ResetCxxMethodParser() {
   // If we want to support parsers for other languages some day, we need a
   // switch here to delete the correct parser type.
   if (m_cxx_method_parser.hasValue()) {
@@ -27,6 +32,10 @@
 delete get(m_cxx_method_parser);
 m_cxx_method_parser.reset();
   }
+}
+
+void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+  ResetCxxMethodParser();
 
   assert(new_provider != None && "Only reset to a valid provider");
   m_provider = new_provider;
Index: lldb/include/lldb/Core/RichManglingContext.h
===
--- lldb/include/lldb/Core/RichManglingContext.h
+++ lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.
@@ -77,6 +77,9 @@
 
   /// Members for ItaniumPartialDemangler
   llvm::ItaniumPartialDemangler m_ipd;
+  /// Note: m_ipd_buf is a raw pointer due to being resized by realloc via
+  /// ItaniumPartialDemangler. It should be managed with malloc/free, not
+  /// new/delete.
   char *m_ipd_buf;
   size_t m_ipd_buf_size;
 
@@ -86,6 +89,9 @@
   /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
   llvm::Any m_cxx_method_parser;
 
+  /// Clean up memory when using PluginCxxLanguage
+  void ResetCxxMethodParser();
+
   /// Clean up memory and set a new info provider for this instance.
   void ResetProvider(InfoProvider new_provider);
 


Index: lldb/source/Core/RichManglingContext.cpp
===
--- lldb/source/Core/RichManglingContext.cpp
+++ lldb/source/Core/RichManglingContext.cpp
@@ -19,7 +19,12 @@
 using namespace lldb_private;
 
 // RichManglingContext
-void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();
+}
+
+void RichManglingContext::ResetCxxMethodParser() {
   // If we want to support parsers for other languages some day, we need a
   // switch here to delete the correct parser type.
   if (m_cxx_method_parser.hasValue()) {
@@ -27,6 +32,10 @@
 delete get(m_cxx_method_parser);
 m_cxx_method_parser.reset();
   }
+}
+
+void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+  ResetCxxMethodParser();
 
   assert(new_provider != None && "Only reset to a valid provider");
   m_provider = new_provider;
Index: lldb/include/lldb/Core/RichManglingContext.h
===
--- lldb/include/lldb/Core/RichManglingContext.h
+++ lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.
@@ -77,6 +77,9 @@
 
   /// Members for ItaniumPartialDemangler
   llvm::ItaniumPartialDemangler m_ipd;
+  /// Note: m_ipd_buf is a raw pointer due to being resized by realloc via
+  /// ItaniumPartialDemangler. It should be managed with malloc/free, not
+  /// new/delete.
   char *m_ipd_buf;
   size_t m_ipd_buf_size;
 
@@ -86,6 +89,9 @@
   /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
   llvm::Any m_cxx_method_parser;
 
+  /// Clean up memory when using PluginCxxLanguage
+  void ResetCxxMethodParser();
+
   /// Clean up memory and set a new info provider for this instance.
   void ResetProvider(InfoProvider new_provider);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] eb5e11f - [lldb] Fix RichManglingContext::FromCxxMethodName() leak

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2021-04-21T12:32:08-07:00
New Revision: eb5e11f460b1bcace77127de00b8bd15d409ea3a

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

LOG: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

`RichManglingContext::FromCxxMethodName` allocates a m_cxx_method_parser, but 
never deletes it.

This fixes a `-DLLVM_USE_SANITIZER=Leaks` failure.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/include/lldb/Core/RichManglingContext.h
lldb/source/Core/RichManglingContext.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/RichManglingContext.h 
b/lldb/include/lldb/Core/RichManglingContext.h
index 68f80e73b7241..61616d9ac27a4 100644
--- a/lldb/include/lldb/Core/RichManglingContext.h
+++ b/lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@ class RichManglingContext {
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.
@@ -77,6 +77,9 @@ class RichManglingContext {
 
   /// Members for ItaniumPartialDemangler
   llvm::ItaniumPartialDemangler m_ipd;
+  /// Note: m_ipd_buf is a raw pointer due to being resized by realloc via
+  /// ItaniumPartialDemangler. It should be managed with malloc/free, not
+  /// new/delete.
   char *m_ipd_buf;
   size_t m_ipd_buf_size;
 
@@ -86,6 +89,9 @@ class RichManglingContext {
   /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
   llvm::Any m_cxx_method_parser;
 
+  /// Clean up memory when using PluginCxxLanguage
+  void ResetCxxMethodParser();
+
   /// Clean up memory and set a new info provider for this instance.
   void ResetProvider(InfoProvider new_provider);
 

diff  --git a/lldb/source/Core/RichManglingContext.cpp 
b/lldb/source/Core/RichManglingContext.cpp
index 2094d96acd7c0..2dcb1407e6c74 100644
--- a/lldb/source/Core/RichManglingContext.cpp
+++ b/lldb/source/Core/RichManglingContext.cpp
@@ -19,7 +19,12 @@ using namespace lldb;
 using namespace lldb_private;
 
 // RichManglingContext
-void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();
+}
+
+void RichManglingContext::ResetCxxMethodParser() {
   // If we want to support parsers for other languages some day, we need a
   // switch here to delete the correct parser type.
   if (m_cxx_method_parser.hasValue()) {
@@ -27,6 +32,10 @@ void RichManglingContext::ResetProvider(InfoProvider 
new_provider) {
 delete get(m_cxx_method_parser);
 m_cxx_method_parser.reset();
   }
+}
+
+void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+  ResetCxxMethodParser();
 
   assert(new_provider != None && "Only reset to a valid provider");
   m_provider = new_provider;



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

[Lldb-commits] [PATCH] D100208: [lldb] [Process/Linux] Report fork/vfork stop reason

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

mgorny updated this revision to Diff 339350.
mgorny marked 5 inline comments as done.
mgorny added a comment.

Implemented all the requests, modulo claim that PID and TID are duplicate ;-). 
Also found cases for `bool()` with `Extension` stuff.


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

https://reviews.llvm.org/D100208

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
  lldb/test/API/tools/lldb-server/main.cpp

Index: lldb/test/API/tools/lldb-server/main.cpp
===
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -216,7 +216,13 @@
 
   sig_result = signal(SIGSEGV, signal_handler);
   if (sig_result == SIG_ERR) {
-fprintf(stderr, "failed to set SIGUSR1 handler: errno=%d\n", errno);
+fprintf(stderr, "failed to set SIGSEGV handler: errno=%d\n", errno);
+exit(1);
+  }
+
+  sig_result = signal(SIGCHLD, SIG_IGN);
+  if (sig_result == SIG_ERR) {
+fprintf(stderr, "failed to set SIGCHLD handler: errno=%d\n", errno);
 exit(1);
   }
 #endif
@@ -293,6 +299,14 @@
   else if (arg == "swap_chars")
 func_p = swap_chars;
   func_p();
+#if !defined(_WIN32)
+} else if (arg == "fork") {
+  if (fork() == 0)
+_exit(0);
+} else if (arg == "vfork") {
+  if (vfork() == 0)
+_exit(0);
+#endif
 } else if (consume_front(arg, "thread:new")) {
 threads.push_back(std::thread(thread_func, nullptr));
 } else if (consume_front(arg, "thread:print-ids")) {
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -0,0 +1,58 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def fork_and_detach_test(self, variant):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=[variant])
+self.add_qSupported_packets(["{}-events+".format(variant)])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+fork_regex = "[$]T.*;{}:p([0-9a-f]*)[.]([0-9a-f]*).*".format(variant)
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": fork_regex,
+ "capture": {1: "pid", 2: "tid"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+pid = int(ret["pid"], 16)
+self.reset_test_sequence()
+
+# detach the forked child
+self.test_sequence.add_log_lines([
+"read packet: $D;{:x}#00".format(pid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+@skipIf(oslist=no_match(["linux"]))
+def test_fork(self):
+self.fork_and_detach_test("fork")
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": r"[$]W00#.*"},
+], True)
+self.expect_gdbremote_sequence()
+
+@skipIf(oslist=no_match(["linux"]))
+def test_vfork(self):
+self.fork_and_detach_test("vfork")
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": r"[$]T.*vforkdone.*"},
+"read packet: $c#00",
+{"direction": "send", "regex": r"[$]W00#.*"},
+], True)
+self.expect_gdbremote_sequence()
Index: lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
===
--- lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
+++ lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
@@ -85,6 +85,10 @@
 
   void SetStoppedByTrace();
 
+  void SetStoppedByFork(bool is_vfork, lldb::pid_t child_pid);
+
+  void SetStoppedByVForkDone();
+
   void SetStoppedWithNoReason();
 
   void SetStoppedByProcessorTrace(llvm::StringRef description);
Index: lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ 

[Lldb-commits] [PATCH] D100243: [LLDB][GUI] Expand selected thread tree item by default

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:2212
+item[i].Expand();
 }
 return;

yeah, we need one frame item to be selected at all times so it indicates the 
current thread/frame that is selected within LLDB. 

So a great solution would be to expand the selected thread and show all frames 
within the selected thread. Add code to select the current frame after as we 
can re-use this code as we step along in our program. If we add a command line 
window that allows LLDB commands, if the user types "frame select 12", the view 
should update to indicate which frame is selected. Also if the user types 
"thread select 5", the view should update accordingly.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100243

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

[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Much of this patch requires all places that took a DWARFDie object now also 
must take an extra parameter. Is there really no way to have DWARFDie be able 
to discover the main unit via its contained DWARFUnit?  DWARFDie objects are 
transient, so they can contain more information that just what they currently 
contain:

  class DWARFDie {
DWARFUnit *U = nullptr;
const DWARFDebugInfoEntry *Die = nullptr;
  };

Can we add a new "DWARFUnit *MU;" to the DWARFDie class? Is there really not 
way to ask the contained "U" DWARFUnit member for the main unit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96236

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

[Lldb-commits] [PATCH] D100208: [lldb] [Process/Linux] Report fork/vfork stop reason

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

mgorny added a comment.

In D100208#2705455 , @labath wrote:

> This seems reasonable.
>
> Why don't we have tests that detach from the parent process (and maybe 
> continue the child)? Does that already work? (or in general, what is expected 
> to work after this patch?)

It doesn't, we need D100261  for that. Right 
now `current_process` will become `nullptr` if we detach the parent.

That said, technically we could test that after detaching the parent we get an 
error from other requests ;-).


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

https://reviews.llvm.org/D100208

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

[Lldb-commits] [PATCH] D91963: [lldb] [test/Register] Initial tests for regsets in core dumps [WIP]

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe414ede2cd54: [lldb] [test/Register] Initial tests for 
regsets in core dumps (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91963

Files:
  lldb/test/Shell/Register/Core/Inputs/strip-coredump.py
  lldb/test/Shell/Register/Core/Inputs/x86-32-freebsd.core
  lldb/test/Shell/Register/Core/Inputs/x86-32-gp.check
  lldb/test/Shell/Register/Core/Inputs/x86-32-linux.core
  lldb/test/Shell/Register/Core/Inputs/x86-32-netbsd.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-freebsd.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-gp-hixmm.check
  lldb/test/Shell/Register/Core/Inputs/x86-64-linux.core
  lldb/test/Shell/Register/Core/Inputs/x86-64-netbsd.core
  lldb/test/Shell/Register/Core/Inputs/x86-core-dump.cpp
  lldb/test/Shell/Register/Core/Inputs/x86-fp.check
  lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
  lldb/test/Shell/Register/Core/x86-32-freebsd-gp.test
  lldb/test/Shell/Register/Core/x86-32-linux-addr.test
  lldb/test/Shell/Register/Core/x86-32-linux-fp.test
  lldb/test/Shell/Register/Core/x86-32-linux-gp.test
  lldb/test/Shell/Register/Core/x86-64-freebsd-addr.test
  lldb/test/Shell/Register/Core/x86-64-freebsd-fp.test
  lldb/test/Shell/Register/Core/x86-64-freebsd-gp.test
  lldb/test/Shell/Register/Core/x86-64-linux-addr.test
  lldb/test/Shell/Register/Core/x86-64-linux-fp.test
  lldb/test/Shell/Register/Core/x86-64-linux-gp.test
  lldb/test/Shell/Register/Core/x86-64-netbsd-addr.test
  lldb/test/Shell/Register/Core/x86-64-netbsd-fp.test
  lldb/test/Shell/Register/Core/x86-64-netbsd-gp.test

Index: lldb/test/Shell/Register/Core/x86-64-netbsd-gp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-netbsd-gp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-netbsd.core | FileCheck %p/Inputs/x86-64-gp-hixmm.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-64-netbsd-fp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-netbsd-fp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-netbsd.core | FileCheck %p/Inputs/x86-fp.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-64-netbsd-addr.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-netbsd-addr.test
@@ -0,0 +1,18 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-netbsd.core | FileCheck %s
+
+register read --all
+# CHECK-DAG: rip = 0x00400c47
+# CHECK-DAG: rflags = 0x00010212
+# CHECK-DAG: cs = 0x0047
+# CHECK-DAG: fs = 0x
+# CHECK-DAG: gs = 0x
+# CHECK-DAG: ss = 0x003f
+# CHECK-DAG: ds = 0x0023
+# CHECK-DAG: es = 0x0023
+
+# CHECK-DAG: fiseg = 0x
+# CHECK-DAG: fioff = 0x00400c06
+# CHECK-DAG: fip = 0x00400c06
+# CHECK-DAG: foseg = 0x
+# CHECK-DAG: fooff = 0xfff93078
+# CHECK-DAG: fdp = 0xfff93078
Index: lldb/test/Shell/Register/Core/x86-64-linux-gp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-linux-gp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux.core | FileCheck %p/Inputs/x86-64-gp-hixmm.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-64-linux-fp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-linux-fp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux.core | FileCheck %p/Inputs/x86-fp.check
+
+register read --all
Index: lldb/test/Shell/Register/Core/x86-64-linux-addr.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-linux-addr.test
@@ -0,0 +1,18 @@
+# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux.core | FileCheck %s
+
+register read --all
+# CHECK-DAG: rip = 0x004012db
+# CHECK-DAG: rflags = 0x00010246
+# CHECK-DAG: cs = 0x0033
+# CHECK-DAG: fs = 0x
+# CHECK-DAG: gs = 0x
+# CHECK-DAG: ss = 0x002b
+# CHECK-DAG: ds = 0x
+# CHECK-DAG: es = 0x
+
+# CHECK-DAG: fiseg = 0x
+# CHECK-DAG: fioff = 0x0040129a
+# CHECK-DAG: fip = 0x0040129a
+# CHECK-DAG: foseg = 0x7ffd
+# CHECK-DAG: fooff = 0x547cb5f8
+# CHECK-DAG: fdp = 0x7ffd547cb5f8
Index: lldb/test/Shell/Register/Core/x86-64-freebsd-gp.test
===
--- /dev/null
+++ lldb/test/Shell/Register/Core/x86-64-freebsd-gp.test
@@ -0,0 +1,3 @@
+# RUN: %lldb -b -s %s -c 

[Lldb-commits] [lldb] e414ede - [lldb] [test/Register] Initial tests for regsets in core dumps

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-21T19:41:09+02:00
New Revision: e414ede2cd54b03f3ff7d547132f06d1b836e5bb

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

LOG: [lldb] [test/Register] Initial tests for regsets in core dumps

Add initial tests for reading register sets from core dumps.  This
includes a C++ program to write registers and dump core, resulting core
dumps for Linux, FreeBSD and NetBSD, and the tests to verify them.

The tests are split into generic part, verifying user-specified register
values, and coredump-specific tests that verify memory addresses that
differ for every dump.

At this moment, all platforms support GPRs and FPRs up to XMM for amd64
target.  The i386 target does not work on NetBSD at all, and is missing
FPRs entirely on FreeBSD.

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

Added: 
lldb/test/Shell/Register/Core/Inputs/strip-coredump.py
lldb/test/Shell/Register/Core/Inputs/x86-32-freebsd.core
lldb/test/Shell/Register/Core/Inputs/x86-32-gp.check
lldb/test/Shell/Register/Core/Inputs/x86-32-linux.core
lldb/test/Shell/Register/Core/Inputs/x86-32-netbsd.core
lldb/test/Shell/Register/Core/Inputs/x86-64-freebsd.core
lldb/test/Shell/Register/Core/Inputs/x86-64-gp-hixmm.check
lldb/test/Shell/Register/Core/Inputs/x86-64-linux.core
lldb/test/Shell/Register/Core/Inputs/x86-64-netbsd.core
lldb/test/Shell/Register/Core/Inputs/x86-core-dump.cpp
lldb/test/Shell/Register/Core/Inputs/x86-fp.check
lldb/test/Shell/Register/Core/x86-32-freebsd-addr.test
lldb/test/Shell/Register/Core/x86-32-freebsd-gp.test
lldb/test/Shell/Register/Core/x86-32-linux-addr.test
lldb/test/Shell/Register/Core/x86-32-linux-fp.test
lldb/test/Shell/Register/Core/x86-32-linux-gp.test
lldb/test/Shell/Register/Core/x86-64-freebsd-addr.test
lldb/test/Shell/Register/Core/x86-64-freebsd-fp.test
lldb/test/Shell/Register/Core/x86-64-freebsd-gp.test
lldb/test/Shell/Register/Core/x86-64-linux-addr.test
lldb/test/Shell/Register/Core/x86-64-linux-fp.test
lldb/test/Shell/Register/Core/x86-64-linux-gp.test
lldb/test/Shell/Register/Core/x86-64-netbsd-addr.test
lldb/test/Shell/Register/Core/x86-64-netbsd-fp.test
lldb/test/Shell/Register/Core/x86-64-netbsd-gp.test

Modified: 


Removed: 




diff  --git a/lldb/test/Shell/Register/Core/Inputs/strip-coredump.py 
b/lldb/test/Shell/Register/Core/Inputs/strip-coredump.py
new file mode 100755
index 0..8908c835641fd
--- /dev/null
+++ b/lldb/test/Shell/Register/Core/Inputs/strip-coredump.py
@@ -0,0 +1,69 @@
+#!/usr/bin/env python
+# Strip unnecessary data from a core dump to reduce its size.
+#
+# 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
+
+import argparse
+import os.path
+import sys
+import tempfile
+
+from elftools.elf.elffile import ELFFile
+
+
+def strip_non_notes(elf, inf, outf):
+next_segment_offset = min(x.header.p_offset for x in elf.iter_segments())
+copy_segments = filter(lambda x: x.header.p_type == "PT_NOTE",
+   elf.iter_segments())
+
+# first copy the headers
+inf.seek(0)
+outf.write(inf.read(next_segment_offset))
+
+for seg in copy_segments:
+assert seg.header.p_filesz > 0
+
+inf.seek(seg.header.p_offset)
+# fill the area between last write and new offset with zeros
+outf.seek(seg.header.p_offset)
+
+# now copy the segment
+outf.write(inf.read(seg.header.p_filesz))
+
+
+def main():
+argp = argparse.ArgumentParser()
+action = argp.add_mutually_exclusive_group(required=True)
+action.add_argument("--strip-non-notes",
+action="store_const",
+const=strip_non_notes,
+dest="action",
+help="Strip all segments except for notes")
+argp.add_argument("elf",
+  help="ELF file to strip (in place)",
+  nargs='+')
+args = argp.parse_args()
+
+for path in args.elf:
+with open(path, "rb") as f:
+elf = ELFFile(f)
+# we do not support copying the section table now
+assert elf.num_sections() == 0
+
+tmpf = tempfile.NamedTemporaryFile(dir=os.path.dirname(path),
+   delete=False)
+try:
+args.action(elf, f, tmpf)
+except:
+os.unlink(tmpf.name)
+raise
+else:
+os.rename(tmpf.name, path)
+
+return 0
+
+
+if __name__ == "__main__":
+sys.exit(main())

[Lldb-commits] [PATCH] D91963: [lldb] [test/Register] Initial tests for regsets in core dumps [WIP]

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

mgorny added a comment.

I'll try to keep this in mind. I expect to have some vacation time ~1 month 
from now, so maybe then ;-).


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

https://reviews.llvm.org/D91963

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

[Lldb-commits] [PATCH] D100977: Use forward type in pointer-to-member

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a reviewer: shafik.
emrekultursay requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This change is similar in spirit to the change at:
https://reviews.llvm.org/rG34c697c85e9d0af11a72ac4df5578aac94a627b3

It fixes the problem where the layout of a type was being accessed
while its base classes were not populated yet; which caused an
incorrect layout to be produced and cached.

Bug: 50054


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100977

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1357,7 +1357,7 @@
   dwarf->ResolveTypeUID(attrs.containing_type.Reference(), true);
 
   CompilerType pointee_clang_type = pointee_type->GetForwardCompilerType();
-  CompilerType class_clang_type = class_type->GetLayoutCompilerType();
+  CompilerType class_clang_type = class_type->GetForwardCompilerType();
 
   CompilerType clang_type = TypeSystemClang::CreateMemberPointerType(
   class_clang_type, pointee_clang_type);


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1357,7 +1357,7 @@
   dwarf->ResolveTypeUID(attrs.containing_type.Reference(), true);
 
   CompilerType pointee_clang_type = pointee_type->GetForwardCompilerType();
-  CompilerType class_clang_type = class_type->GetLayoutCompilerType();
+  CompilerType class_clang_type = class_type->GetForwardCompilerType();
 
   CompilerType clang_type = TypeSystemClang::CreateMemberPointerType(
   class_clang_type, pointee_clang_type);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100965: [lldb] Refactor (FileSpec+Line) to SourceLocationSpec (NFCI)

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

It seems like most of the places where you use the SourceLocationSpec, you also 
pass "check_inlines" and "exact".  Those seem to me natural parts of a source 
location search specification class, and including them in the Spec would clean 
up the API's even further.  Not necessary if it gets ugly for some reason, but 
it would look nicer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

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

[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D98370#2704597 , @werat wrote:

> In D98370#2686515 , @jingham wrote:
>
>> If persisting already persistent variables is indeed the only failure case, 
>> then I wonder if it wouldn't be more straightforward to just see if the 
>> ValueObject is already a persistent variable and have Persist just return 
>> the incoming variable.
>
> Persisting already persistent variables is not the only valid use case, one 
> might also want to persist variables created via 
> `SBTarget::CreateValueFromData()`.

Sure.  But but when I was poking around at it a little bit, it seems like the 
other use cases already work, and the only one that was failing was the case 
where you call persist on a persistent variable.  If that is really true, then 
maybe we should fix the failing case directly.

> I guess it is currently not clear from the API, but I would expect 
> `SBValue::Persist()` to produces a new value every time:
>
>   auto v1 = value.Persist();
>   auto v2 = value.Persist();
>   
>   assert(v1.GetName() != v1.GetName())  // modulo comparing char*...

Not sure why?  The API is a request:  "I made a variable somehow, and I would 
like you to make it persist so I can use its value later on even if the 
underlying data has changed."  Why do you care whether you get a copy of an 
already persistent or just a shared value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98370

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

[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

If this is going to be plumbed through everywhere (ie: a DWARFDIE is mostly 
useless without both its lexical DWARFUnit and its imported-into DWARFUnit) 
then maybe it makes more sense to add this DWARFUnit* to the DWARFBaseDie type 
alongside the lexical DWARFUnit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96236

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

[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added inline comments.



Comment at: lldb/source/Core/RichManglingContext.cpp:23
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();

dblaikie wrote:
> JDevlieghere wrote:
> > shafik wrote:
> > > rupprecht wrote:
> > > > JDevlieghere wrote:
> > > > > Instead of managing memory by hand, can we use a `unique_ptr` 
> > > > > instead?
> > > > The buffer here is created by `malloc`, and from a cursory reading of 
> > > > `processIPDStrResult`, can be passed to other methods that call 
> > > > `realloc` on it. It should be paired with `free`, not `delete`. You 
> > > > could put that in a `unique_ptr`, but when you go 
> > > > down that road, I think it's probably simpler to leave as-is. (You'd 
> > > > also have to take care to always manually `.release()` it when updating 
> > > > it to the realloc'd value, because you don't want to delete the 
> > > > pre-realloc'd buffer).
> > > > 
> > > > (Note: this line is pulled from the original `~RichManglingContext()` 
> > > > definition in the header. I didn't write it so I can't defend it that 
> > > > well :) )
> > > I didn't suggest this b/c it was clear it was a quick fix and it seemed a 
> > > reach to ask for that in this fix.
> > Thanks for the explanation! That does sound like a bit of overkill. If it's 
> > not already documented, would it be useful to leave that as a comment 
> > somewhere? 
> (I think for this patch, makes sense not to try to address the 
> ownership/allocation model because it is non-trivial, as you've mentioned - 
> but longer term, it might be worth revisiting it at some point - as it has a 
> non-trivial & not well enforced boundary in terms of the allocation scheme 
> shared between RichManglingContext and ItaniumPartialDemangler. It'd be good 
> if that boundary were more clear rather than requiring malloc'd memory to be 
> passed in, and requiring the caller to free it - some kind of abstraction 
> over the memory ownership would probably be good to have)
Added a comment to the header. Agree about the longer term fixes on the API 
boundaries -- D100800 is a leak fix for that API, so it's clearly an API with 
sharp edges.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100795

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

[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht updated this revision to Diff 339264.
rupprecht marked 4 inline comments as done.
rupprecht added a comment.

- Add comments about memory management for m_ipd_buf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100795

Files:
  lldb/include/lldb/Core/RichManglingContext.h
  lldb/source/Core/RichManglingContext.cpp


Index: lldb/source/Core/RichManglingContext.cpp
===
--- lldb/source/Core/RichManglingContext.cpp
+++ lldb/source/Core/RichManglingContext.cpp
@@ -19,7 +19,12 @@
 using namespace lldb_private;
 
 // RichManglingContext
-void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();
+}
+
+void RichManglingContext::ResetCxxMethodParser() {
   // If we want to support parsers for other languages some day, we need a
   // switch here to delete the correct parser type.
   if (m_cxx_method_parser.hasValue()) {
@@ -27,6 +32,10 @@
 delete get(m_cxx_method_parser);
 m_cxx_method_parser.reset();
   }
+}
+
+void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+  ResetCxxMethodParser();
 
   assert(new_provider != None && "Only reset to a valid provider");
   m_provider = new_provider;
Index: lldb/include/lldb/Core/RichManglingContext.h
===
--- lldb/include/lldb/Core/RichManglingContext.h
+++ lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.
@@ -77,6 +77,9 @@
 
   /// Members for ItaniumPartialDemangler
   llvm::ItaniumPartialDemangler m_ipd;
+  /// Note: m_ipd_buf is a raw pointer due to being resized by realloc via
+  /// ItaniumPartialDemangler. It should be managed with malloc/free, not
+  /// new/delete.
   char *m_ipd_buf;
   size_t m_ipd_buf_size;
 
@@ -86,6 +89,9 @@
   /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
   llvm::Any m_cxx_method_parser;
 
+  /// Clean up memory when using PluginCxxLanguage
+  void ResetCxxMethodParser();
+
   /// Clean up memory and set a new info provider for this instance.
   void ResetProvider(InfoProvider new_provider);
 


Index: lldb/source/Core/RichManglingContext.cpp
===
--- lldb/source/Core/RichManglingContext.cpp
+++ lldb/source/Core/RichManglingContext.cpp
@@ -19,7 +19,12 @@
 using namespace lldb_private;
 
 // RichManglingContext
-void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();
+}
+
+void RichManglingContext::ResetCxxMethodParser() {
   // If we want to support parsers for other languages some day, we need a
   // switch here to delete the correct parser type.
   if (m_cxx_method_parser.hasValue()) {
@@ -27,6 +32,10 @@
 delete get(m_cxx_method_parser);
 m_cxx_method_parser.reset();
   }
+}
+
+void RichManglingContext::ResetProvider(InfoProvider new_provider) {
+  ResetCxxMethodParser();
 
   assert(new_provider != None && "Only reset to a valid provider");
   m_provider = new_provider;
Index: lldb/include/lldb/Core/RichManglingContext.h
===
--- lldb/include/lldb/Core/RichManglingContext.h
+++ lldb/include/lldb/Core/RichManglingContext.h
@@ -29,7 +29,7 @@
 m_ipd_buf[0] = '\0';
   }
 
-  ~RichManglingContext() { std::free(m_ipd_buf); }
+  ~RichManglingContext();
 
   /// Use the ItaniumPartialDemangler to obtain rich mangling information from
   /// the given mangled name.
@@ -77,6 +77,9 @@
 
   /// Members for ItaniumPartialDemangler
   llvm::ItaniumPartialDemangler m_ipd;
+  /// Note: m_ipd_buf is a raw pointer due to being resized by realloc via
+  /// ItaniumPartialDemangler. It should be managed with malloc/free, not
+  /// new/delete.
   char *m_ipd_buf;
   size_t m_ipd_buf_size;
 
@@ -86,6 +89,9 @@
   /// dependency. Instead keep a llvm::Any and cast it on-access in the cpp.
   llvm::Any m_cxx_method_parser;
 
+  /// Clean up memory when using PluginCxxLanguage
+  void ResetCxxMethodParser();
+
   /// Clean up memory and set a new info provider for this instance.
   void ResetProvider(InfoProvider new_provider);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100208: [lldb] [Process/Linux] Report fork/vfork stop reason

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Note that you don't actually have to use/extend this particular inferior for 
these tests. Feel free to create special purpose inferiors, if that's a better 
fit for the test, just like we do with "normal" tests...


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

https://reviews.llvm.org/D100208

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

[Lldb-commits] [PATCH] D100208: [lldb] [Process/Linux] Report fork/vfork stop reason

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This seems reasonable.

Why don't we have tests that detach from the parent process (and maybe continue 
the child)? Does that already work? (or in general, what is expected to work 
after this patch?)




Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:932
+
+std::unique_ptr child_process_up{child_process};
+Extension expected_ext = is_vfork ? Extension::vfork : Extension::fork;

Store directly into unique_ptr above.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:938-940
+parent_thread->SetStoppedByVFork(child_pid);
+  else
+parent_thread->SetStoppedByFork(child_pid);

I'd consider merging the two functions into `SetStoppedByFork(is_vfork, 
child_pid)`



Comment at: lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp:401-402
+  m_stop_info.reason = StopReason::eStopReasonFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_pid;
+}

duplicated



Comment at: lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp:409-410
+  m_stop_info.reason = StopReason::eStopReasonVFork;
+  m_stop_info.details.fork.child_pid = child_pid;
+  m_stop_info.details.fork.child_tid = child_pid;
+}

ditto



Comment at: lldb/test/API/tools/lldb-server/main.cpp:322-327
+} else if (!std::strcmp(argv[i], FORK_COMMAND)) {
+  if (fork() == 0)
+_exit(0);
+} else if (!std::strcmp(argv[i], VFORK_COMMAND)) {
+  if (vfork() == 0)
+_exit(0);

I guess these would need to be ifdefed too.


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

https://reviews.llvm.org/D100208

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

[Lldb-commits] [PATCH] D100962: [lldb/Utility] Add SourceLocationSpec class (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:184
+  FileSpec m_file_spec;
+  uint32_t m_line;
+  llvm::Optional m_column;

mib wrote:
> JDevlieghere wrote:
> > Are there situations where the line is optional too? 
> Technically, `m_line` could be equal to 0 (if there is no line information) 
> but that would basically mean this class is a `FileSpec` container.
> 
> I guess I can add an assert in the constructor checking if it's not null.
Indeed. Another option would be to make it a precondition that a 
SourceLocationSpec is always valid, assuming that unlike the column number, the 
file and line are usually valid. One way to do that is by adding asserts to the 
ctor or if this class is often created from user input, then I'd make the ctor 
private and add a static `CreateSoureLocationSpec` factory, that checks our 
preconditions and returns an Expected/Optional. That has the advantage of not 
having the change all the call sites for the few scenarios where those are 
actually invalid. This has the advantage of paying the price upfront when 
creating the LocationSpec rather than having to check in every place it's used. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100962

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

[Lldb-commits] [PATCH] D100965: [lldb] Refactor (FileSpec+Line) to SourceLocationSpec (NFCI)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Nice, it's great to see functions taking less arguments for a change ;-)




Comment at: lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h:12-14
 #include "lldb/Breakpoint/BreakpointResolver.h"
 
+#include "lldb/Utility/SourceLocationSpec.h"





Comment at: lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h:32
+  BreakpointResolverFileLine(const lldb::BreakpointSP ,
+ const SourceLocationSpec location_spec,
+ lldb::addr_t offset, bool check_inlines,

Was this meant to be a const ref? There's no point in making objects passed by 
value const, as they're already copied in and this just means that if the 
function wants to modify them, they have to make a second copy while the 
function already has its own copy. 



Comment at: lldb/include/lldb/Core/AddressResolverFileLine.h:45-48
+  SourceLocationSpec m_src_location_spec; // This contains the file spec and
+  // line we are looking for.
   bool m_inlines; // This determines whether the resolver looks for inlined
   // functions or not.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

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

[Lldb-commits] [PATCH] D100962: [lldb/Utility] Add SourceLocationSpec class (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:184
+  FileSpec m_file_spec;
+  uint32_t m_line;
+  llvm::Optional m_column;

JDevlieghere wrote:
> Are there situations where the line is optional too? 
Technically, `m_line` could be equal to 0 (if there is no line information) but 
that would basically mean this class is a `FileSpec` container.

I guess I can add an assert in the constructor checking if it's not null.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100962

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

[Lldb-commits] [PATCH] D100208: [lldb] [Process/Linux] Report fork/vfork stop reason

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

mgorny updated this revision to Diff 339257.
mgorny retitled this revision from "[lldb] [Process/Linux] Report fork/vfork 
stop reason to client" to "[lldb] [Process/Linux] Report fork/vfork stop 
reason".
mgorny edited the summary of this revision.
mgorny added a comment.

Now includes lldb-server tests.

@labath, please let me know if I should merge this with previous patches, or if 
it's fine to commit in split.


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

https://reviews.llvm.org/D100208

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeThreadLinux.h
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
  lldb/test/API/tools/lldb-server/main.cpp

Index: lldb/test/API/tools/lldb-server/main.cpp
===
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -28,6 +28,8 @@
 static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
 static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
 static const char *const GET_HEAP_ADDRESS_COMMAND = "get-heap-address-hex:";
+static const char *const FORK_COMMAND = "fork";
+static const char *const VFORK_COMMAND = "vfork";
 
 static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
 static const char *const CALL_FUNCTION_PREFIX = "call-function:";
@@ -225,7 +227,13 @@
 
   sig_result = signal(SIGSEGV, signal_handler);
   if (sig_result == SIG_ERR) {
-fprintf(stderr, "failed to set SIGUSR1 handler: errno=%d\n", errno);
+fprintf(stderr, "failed to set SIGSEGV handler: errno=%d\n", errno);
+exit(1);
+  }
+
+  sig_result = signal(SIGCHLD, SIG_IGN);
+  if (sig_result == SIG_ERR) {
+fprintf(stderr, "failed to set SIGCHLD handler: errno=%d\n", errno);
 exit(1);
   }
 #endif
@@ -311,6 +319,12 @@
   }
   if (func_p)
 func_p();
+} else if (!std::strcmp(argv[i], FORK_COMMAND)) {
+  if (fork() == 0)
+_exit(0);
+} else if (!std::strcmp(argv[i], VFORK_COMMAND)) {
+  if (vfork() == 0)
+_exit(0);
 } else if (std::strstr(argv[i], THREAD_PREFIX)) {
   // Check if we're creating a new thread.
   if (std::strstr(argv[i] + strlen(THREAD_PREFIX), THREAD_COMMAND_NEW)) {
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -0,0 +1,58 @@
+import gdbremote_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def fork_and_detach_test(self, variant):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=[variant])
+self.add_qSupported_packets(["{}-events+".format(variant)])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+fork_regex = "[$]T.*;{}:p([0-9a-f]*)[.]([0-9a-f]*).*".format(variant)
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": fork_regex,
+ "capture": {1: "pid", 2: "tid"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+pid = int(ret["pid"], 16)
+self.reset_test_sequence()
+
+# detach the forked child
+self.test_sequence.add_log_lines([
+"read packet: $D;{:x}#00".format(pid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+@skipIf(oslist=no_match(["linux"]))
+def test_fork(self):
+self.fork_and_detach_test("fork")
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": r"[$]W00#.*"},
+], True)
+self.expect_gdbremote_sequence()
+
+@skipIf(oslist=no_match(["linux"]))
+def test_vfork(self):
+self.fork_and_detach_test("vfork")
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": r"[$]T.*vforkdone.*"},
+"read packet: $c#00",
+{"direction": "send", "regex": r"[$]W00#.*"},
+], True)
+self.expect_gdbremote_sequence()
Index: lldb/source/Plugins/Process/Linux/NativeThreadLinux.h

[Lldb-commits] [PATCH] D100962: [lldb/Utility] Add SourceLocationSpec class (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:12-19
+#include 
+
+#include "lldb/Utility/FileSpec.h"
+
+#include "llvm/ADT/Optional.h"
+
+#include 





Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:90
+
+  /// Convert to pointer operator.
+  ///





Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:126
+  /// and making both line and column value the empty string.
+  void Clear();
+

The class looks mostly immutable (which is good) so I think we can omit this.



Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:153
+
+  static bool Equal(const SourceLocationSpec , const SourceLocationSpec ,
+bool full);





Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:160
+  /// file_spec and line number. An empty \a pattern matches everything.
+  static bool Match(const SourceLocationSpec ,
+const SourceLocationSpec );

Would it make sense to merge this into Equal and replace the bool with an enum 
value that represents 

 - file + line must match, 
 - file + line + col must match,
 - file + line + col must match if lhs has it?



Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:178
+
+  bool HasColumn() const { return m_column.hasValue() && *m_column != 0; }
+

If 0 is not a valid column, we should enforce that rather than having two ways 
of representing it. Can we add an assert to the ctor and make it a precondition 
for the class? 



Comment at: lldb/include/lldb/Utility/SourceLocationSpec.h:184
+  FileSpec m_file_spec;
+  uint32_t m_line;
+  llvm::Optional m_column;

Are there situations where the line is optional too? 



Comment at: lldb/unittests/Utility/SourceLocationSpecTest.cpp:1-2
+//===-- SourceLocationSpecTest.cpp
+//--===//
+//




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100962

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

[Lldb-commits] [PATCH] D100965: [lldb] Refactor (FileSpec+Line) to SourceLocationSpec (NFCI)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 339255.
mib edited reviewers, added: teemperor, jingham; removed: jdoerfert.
Herald added a reviewer: jdoerfert.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100965

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Core/AddressResolverFileLine.h
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/include/lldb/Symbol/LineTable.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBThread.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
  lldb/source/Core/AddressResolverFileLine.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/LineTable.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp

Index: lldb/unittests/Symbol/TestLineEntry.cpp
===
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -64,9 +64,10 @@
   if (sc_comp_units.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No comp unit found on the test object.");
+
   sc_comp_units[0].comp_unit->ResolveSymbolContext(
-  file_spec, line, check_inlines, exact, eSymbolContextLineEntry,
-  sc_line_entries);
+  SourceLocationSpec(file_spec, line), check_inlines, exact,
+  eSymbolContextLineEntry, sc_line_entries);
   if (sc_line_entries.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No line entry found on the test object.");
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -97,10 +97,9 @@
   return type_system_or_err;
 }
 
-uint32_t SymbolFile::ResolveSymbolContext(const FileSpec _spec,
-  uint32_t line, bool check_inlines,
-  lldb::SymbolContextItem resolve_scope,
-  SymbolContextList _list) {
+uint32_t SymbolFile::ResolveSymbolContext(
+const SourceLocationSpec _location_spec, bool check_inlines,
+lldb::SymbolContextItem resolve_scope, SymbolContextList _list) {
   return 0;
 }
 
Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -304,7 +304,8 @@
 
 uint32_t LineTable::FindLineEntryIndexByFileIndex(
 uint32_t start_idx, const std::vector _indexes,
-uint32_t line, bool exact, LineEntry *line_entry_ptr) {
+SourceLocationSpec src_location_spec, bool exact,
+LineEntry *line_entry_ptr) {
 
   const size_t count = m_entries.size();
   size_t best_match = UINT32_MAX;
@@ -324,6 +325,8 @@
 // after and
 // if they're not in the same function, don't return a match.
 
+uint32_t line = src_location_spec.GetLine();
+
 if (m_entries[idx].line < line) {
   continue;
 } else if (m_entries[idx].line == line) {
@@ -346,10 +349,9 @@
   return UINT32_MAX;
 }
 
-uint32_t LineTable::FindLineEntryIndexByFileIndex(uint32_t start_idx,
-  uint32_t file_idx,
-  uint32_t line, bool exact,
-  LineEntry *line_entry_ptr) {
+uint32_t LineTable::FindLineEntryIndexByFileIndex(
+uint32_t start_idx, uint32_t file_idx, SourceLocationSpec src_location_spec,
+bool exact, LineEntry *line_entry_ptr) {
   const size_t count = m_entries.size();
   size_t best_match = UINT32_MAX;
 
@@ -368,6 +370,8 @@
 // after and
 // if they're not in the same function, don't return a match.
 
+uint32_t line = src_location_spec.GetLine();
+
 if (m_entries[idx].line < line) {
   continue;
 } else if (m_entries[idx].line == line) {
Index: lldb/source/Symbol/CompileUnit.cpp
===
--- lldb/source/Symbol/CompileUnit.cpp
+++ 

[Lldb-commits] [PATCH] D100965: [lldb] Refactor (FileSpec+Line) to SourceLocationSpec (NFCI)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
mib requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.

This patch refactors a good part of the code base turning FileSpec+Line
pairs into a SourceLocationSpec argument.

This change is required for a following patch that will add handling of the
column line information when doing symbol resolution.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100965

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Core/AddressResolverFileLine.h
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/include/lldb/Symbol/LineTable.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBThread.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
  lldb/source/Core/AddressResolverFileLine.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/LineTable.cpp
  lldb/source/Symbol/SymbolFile.cpp
  lldb/unittests/Symbol/TestLineEntry.cpp

Index: lldb/unittests/Symbol/TestLineEntry.cpp
===
--- lldb/unittests/Symbol/TestLineEntry.cpp
+++ lldb/unittests/Symbol/TestLineEntry.cpp
@@ -64,9 +64,10 @@
   if (sc_comp_units.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No comp unit found on the test object.");
+
   sc_comp_units[0].comp_unit->ResolveSymbolContext(
-  file_spec, line, check_inlines, exact, eSymbolContextLineEntry,
-  sc_line_entries);
+  SourceLocationSpec(file_spec, line), check_inlines, exact,
+  eSymbolContextLineEntry, sc_line_entries);
   if (sc_line_entries.GetSize() == 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No line entry found on the test object.");
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -97,10 +97,9 @@
   return type_system_or_err;
 }
 
-uint32_t SymbolFile::ResolveSymbolContext(const FileSpec _spec,
-  uint32_t line, bool check_inlines,
-  lldb::SymbolContextItem resolve_scope,
-  SymbolContextList _list) {
+uint32_t SymbolFile::ResolveSymbolContext(
+const SourceLocationSpec _location_spec, bool check_inlines,
+lldb::SymbolContextItem resolve_scope, SymbolContextList _list) {
   return 0;
 }
 
Index: lldb/source/Symbol/LineTable.cpp
===
--- lldb/source/Symbol/LineTable.cpp
+++ lldb/source/Symbol/LineTable.cpp
@@ -304,7 +304,8 @@
 
 uint32_t LineTable::FindLineEntryIndexByFileIndex(
 uint32_t start_idx, const std::vector _indexes,
-uint32_t line, bool exact, LineEntry *line_entry_ptr) {
+SourceLocationSpec src_location_spec, bool exact,
+LineEntry *line_entry_ptr) {
 
   const size_t count = m_entries.size();
   size_t best_match = UINT32_MAX;
@@ -324,6 +325,8 @@
 // after and
 // if they're not in the same function, don't return a match.
 
+uint32_t line = src_location_spec.GetLine();
+
 if (m_entries[idx].line < line) {
   continue;
 } else if (m_entries[idx].line == line) {
@@ -346,10 +349,9 @@
   return UINT32_MAX;
 }
 
-uint32_t LineTable::FindLineEntryIndexByFileIndex(uint32_t start_idx,
-  uint32_t file_idx,
-  uint32_t line, bool exact,
-  LineEntry *line_entry_ptr) {
+uint32_t LineTable::FindLineEntryIndexByFileIndex(
+uint32_t start_idx, uint32_t file_idx, SourceLocationSpec src_location_spec,
+bool exact, LineEntry *line_entry_ptr) {
   const size_t count = m_entries.size();
   size_t best_match = UINT32_MAX;
 
@@ -368,6 +370,8 @@
 // after and
 // if they're not in the same function, don't return a match.
 
+uint32_t line = src_location_spec.GetLine();
+
   

[Lldb-commits] [lldb] 55ee541 - [lldb/test] Clean up TestThreadSpecificBpPlusCondition inferior

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-21T17:37:30+02:00
New Revision: 55ee541653a817c77a6b5b837c4b8cc50ed3ae0e

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

LOG: [lldb/test] Clean up TestThreadSpecificBpPlusCondition inferior

The test had a race that could cause two threads to end up with the same
"thread local" value. I believe this would not cause the test to fail,
but it could cause it to succeed even when the functionality is broken.

The new implementation removes this uncertainty, and removes a lot of
cruft left over from the time this test was written using pthreads.

Added: 


Modified: 

lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/TestThreadSpecificBpPlusCondition.py

lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/TestThreadSpecificBpPlusCondition.py
 
b/lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/TestThreadSpecificBpPlusCondition.py
index 325252f9ae00..95499883aa7a 100644
--- 
a/lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/TestThreadSpecificBpPlusCondition.py
+++ 
b/lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/TestThreadSpecificBpPlusCondition.py
@@ -19,7 +19,6 @@ class ThreadSpecificBreakPlusConditionTestCase(TestBase):
 @skipIfDarwin
 # hits break in another thread in testrun
 @add_test_categories(['pyapi'])
-@expectedFailureAll(oslist=['ios', 'watchos', 'tvos', 'bridgeos'], 
archs=['armv7', 'armv7k'], bugnumber='rdar://problem/34563348') # Two threads 
seem to end up with the same my_value when built for armv7.
 @expectedFlakeyNetBSD
 def test_python(self):
 """Test that we obey thread conditioned breakpoints."""

diff  --git 
a/lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/main.cpp
 
b/lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/main.cpp
index af8ab84157fd..beceb09e5b9c 100644
--- 
a/lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/main.cpp
+++ 
b/lldb/test/API/functionalities/thread/thread_specific_break_plus_condition/main.cpp
@@ -2,38 +2,22 @@
 #include 
 #include 
 
-void *
-thread_function (void *thread_marker)
-{
-int keep_going = 1; 
-int my_value = *((int *)thread_marker);
-int counter = 0;
-
-while (counter < 20)
-{
-counter++; // Break here in thread body.
-std::this_thread::sleep_for(std::chrono::microseconds(10));
-}
-return NULL;
+void thread_function(int my_value) {
+  int counter = 0;
+  while (counter < 20) {
+counter++; // Break here in thread body.
+std::this_thread::sleep_for(std::chrono::microseconds(10));
+  }
 }
 
+int main() {
+  std::vector threads;
 
-int 
-main ()
-{
-std::vector threads;
-
-int thread_value = 0;
-int i;
-
-for (i = 0; i < 10; i++)
-{
-thread_value += 1;
-threads.push_back(std::thread(thread_function, _value));
-}
+  for (int i = 0; i < 10; i++)
+threads.push_back(std::thread(thread_function, threads.size() + 1));
 
-for (i = 0; i < 10; i++)
-threads[i].join();
+  for (std::thread  : threads)
+t.join();
 
-return 0;
+  return 0;
 }



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

[Lldb-commits] [PATCH] D100964: [lldb][WIP-ish] Add a assertPacketResponse utility to lldb-server tests

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.
Herald added a subscriber: JDevlieghere.

I was going to wait until this is fully functional, but since you're about to
write some lldb-server tests, I figured it's a good time to get some feedback.

The main part that's missing is the ability to wait for some output to be
produced. I'd like to replace that with something completely different as the
current mechanism relies on sleeps.

But if you find it useful in the current form, I could clean it up and check in 
pretty quickly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100964

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

[Lldb-commits] [PATCH] D100964: [lldb][WIP-ish] Add a assertPacketResponse utility to lldb-server tests

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added a reviewer: mgorny.
labath requested review of this revision.
Herald added a project: LLDB.

The goal is to replace the awkward add_log_lines dance.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100964

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  lldb/test/API/tools/lldb-server/main.cpp

Index: lldb/test/API/tools/lldb-server/main.cpp
===
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -177,6 +177,7 @@
   std::lock_guard lock(g_print_mutex);
   printf("thread %" PRIx64 ": past SIGSEGV\n", get_thread_id());
 }
+return nullptr;
   }
 
   int sleep_seconds_remaining = 60;
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -25,77 +25,44 @@
 mydir = TestBase.compute_mydir(__file__)
 
 def test_thread_suffix_supported(self):
-server = self.connect_to_debug_monitor()
-self.assertIsNotNone(server)
-
+self.connect_to_debug_monitor()
 self.do_handshake()
-self.test_sequence.add_log_lines(
-["lldb-server <  26> read packet: $QThreadSuffixSupported#e4",
- "lldb-server <   6> send packet: $OK#9a"],
-True)
-
-self.expect_gdbremote_sequence()
-
+self.assertPacketResponse(b"QThreadSuffixSupported", b"OK")
 
 def test_list_threads_in_stop_reply_supported(self):
-server = self.connect_to_debug_monitor()
-self.assertIsNotNone(server)
-
+self.connect_to_debug_monitor()
 self.do_handshake()
-self.test_sequence.add_log_lines(
-["lldb-server <  27> read packet: $QListThreadsInStopReply#21",
- "lldb-server <   6> send packet: $OK#9a"],
-True)
-self.expect_gdbremote_sequence()
+self.assertPacketResponse(b"QListThreadsInStopReply", b"OK")
 
 def test_c_packet_works(self):
 self.build()
-procs = self.prep_debug_monitor_and_inferior()
-self.test_sequence.add_log_lines(
-["read packet: $c#63",
- "send packet: $W00#00"],
-True)
-
-self.expect_gdbremote_sequence()
+self.prep_debug_monitor_and_inferior()
+self.assertPacketResponse(b"c", b"W00")
 
 @skipIfWindows # No pty support to test any inferior output
 def test_inferior_print_exit(self):
 self.build()
-procs = self.prep_debug_monitor_and_inferior(
-inferior_args=["hello, world"])
-self.test_sequence.add_log_lines(
-["read packet: $vCont;c#a8",
- {"type": "output_match", "regex": self.maybe_strict_output_regex(r"hello, world\r\n")},
- "send packet: $W00#00"],
-True)
+self.prep_debug_monitor_and_inferior(inferior_args=["hello, world"])
 
-context = self.expect_gdbremote_sequence()
-self.assertIsNotNone(context)
+self.assertPacketResponse(b"vCont;c", b"W00")
+self.assertRegex(self._server.consume_accumulated_output(),
+self.maybe_strict_output_regex(br"hello, world\r\n"))
 
 def test_first_launch_stop_reply_thread_matches_first_qC(self):
 self.build()
-procs = self.prep_debug_monitor_and_inferior()
-self.test_sequence.add_log_lines(["read packet: $qC#00",
-  {"direction": "send",
-   "regex": r"^\$QC([0-9a-fA-F]+)#",
-   "capture": {1: "thread_id_QC"}},
-  "read packet: $?#00",
-  {"direction": "send",
-  "regex": r"^\$T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+)",
-  "capture": {1: "thread_id_?"}}],
- True)
-context = self.expect_gdbremote_sequence()
-self.assertEqual(context.get("thread_id_QC"), context.get("thread_id_?"))
+self.prep_debug_monitor_and_inferior()
+
+self.assertPacketResponse(b"qC", re.compile(b"QC(?P[0-9a-fA-F]+)"))
+self.assertPacketResponse(b"?",
+re.compile(b"T[0-9a-fA-F]{2}thread:(?P[0-9a-fA-F]+)"))
+self.assertEqual(self.captures["QC"], self.captures["q"])
 
 def test_attach_commandline_continue_app_exits(self):
 self.build()
 self.set_inferior_startup_attach()
 procs = self.prep_debug_monitor_and_inferior()
-self.test_sequence.add_log_lines(
-["read packet: $vCont;c#a8",
- "send packet: $W00#00"],
- 

[Lldb-commits] [PATCH] D100962: [lldb/Utility] Add SourceLocationSpec class (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: JDevlieghere, jingham.
mib added a project: LLDB.
Herald added a subscriber: mgorny.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch introduces a new utility class, SourceLocationSpec.

A source location specifier class holds a FileSpec object with
line and column information, the last one being optional.

It describes a specific location in a source file and allows the user
to perfom checks and comparaisons between multiple instances of that class.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100962

Files:
  lldb/include/lldb/Utility/SourceLocationSpec.h
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/SourceLocationSpec.cpp
  lldb/unittests/Utility/CMakeLists.txt
  lldb/unittests/Utility/SourceLocationSpecTest.cpp

Index: lldb/unittests/Utility/SourceLocationSpecTest.cpp
===
--- /dev/null
+++ lldb/unittests/Utility/SourceLocationSpecTest.cpp
@@ -0,0 +1,210 @@
+//===-- SourceLocationSpecTest.cpp
+//--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Utility/SourceLocationSpec.h"
+
+using namespace lldb_private;
+
+TEST(SourceLocationSpecTest, FileLineColumnComponents) {
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  const uint32_t line = 19;
+  const uint16_t column = 4;
+  SourceLocationSpec without_column(fs, line);
+  EXPECT_EQ(fs, without_column.GetFileSpec());
+  EXPECT_EQ(line, without_column.GetLine());
+  EXPECT_FALSE(without_column.HasColumn());
+  EXPECT_EQ(llvm::None, without_column.GetColumn());
+  EXPECT_STREQ("/foo/bar:19", without_column.GetCString());
+
+  SourceLocationSpec with_column(fs, 19, 4);
+  EXPECT_TRUE(with_column.HasColumn());
+  EXPECT_EQ(column, with_column.GetColumn());
+  EXPECT_STREQ("/foo/bar:19:4", with_column.GetCString());
+}
+
+TEST(SourceLocationSpecTest, Equal) {
+  auto Eq = [](SourceLocationSpec a, SourceLocationSpec b, bool full) {
+return SourceLocationSpec::Equal(a, b, full);
+  };
+
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  FileSpec other_fs("/foo/baz", FileSpec::Style::posix);
+
+  EXPECT_TRUE(Eq(SourceLocationSpec(fs, 4), SourceLocationSpec(fs, 4), true));
+  EXPECT_TRUE(Eq(SourceLocationSpec(fs, 4), SourceLocationSpec(fs, 4), false));
+
+  EXPECT_FALSE(
+  Eq(SourceLocationSpec(fs, 4), SourceLocationSpec(other_fs, 4), true));
+  EXPECT_FALSE(
+  Eq(SourceLocationSpec(fs, 4), SourceLocationSpec(other_fs, 4), false));
+
+  EXPECT_TRUE(
+  Eq(SourceLocationSpec(fs, 4, 19), SourceLocationSpec(fs, 4, 19), true));
+  EXPECT_TRUE(
+  Eq(SourceLocationSpec(fs, 4, 19), SourceLocationSpec(fs, 4, 19), false));
+
+  EXPECT_FALSE(Eq(SourceLocationSpec(fs, 4, 19),
+  SourceLocationSpec(other_fs, 4, 19), true));
+  EXPECT_FALSE(Eq(SourceLocationSpec(fs, 4, 19),
+  SourceLocationSpec(other_fs, 4, 19), false));
+
+  EXPECT_FALSE(
+  Eq(SourceLocationSpec(fs, 4), SourceLocationSpec(fs, 4, 19), true));
+  EXPECT_TRUE(
+  Eq(SourceLocationSpec(fs, 4), SourceLocationSpec(fs, 4, 19), false));
+
+  EXPECT_FALSE(Eq(SourceLocationSpec(fs, 0), SourceLocationSpec(fs, 4), true));
+  EXPECT_FALSE(Eq(SourceLocationSpec(fs, 0), SourceLocationSpec(fs, 4), false));
+
+  EXPECT_FALSE(
+  Eq(SourceLocationSpec(fs, 0), SourceLocationSpec(fs, 4, 19), true));
+  EXPECT_FALSE(
+  Eq(SourceLocationSpec(fs, 0), SourceLocationSpec(fs, 4, 19), false));
+
+  EXPECT_FALSE(
+  Eq(SourceLocationSpec(fs, 4, 96), SourceLocationSpec(fs, 4, 19), true));
+  EXPECT_TRUE(
+  Eq(SourceLocationSpec(fs, 4, 96), SourceLocationSpec(fs, 4, 19), false));
+}
+
+TEST(SourceLocationSpecTest, Match) {
+  auto Match = [](SourceLocationSpec a, SourceLocationSpec b) {
+return SourceLocationSpec::Match(a, b);
+  };
+
+  FileSpec fs("/foo/bar", FileSpec::Style::posix);
+  FileSpec other_fs("/foo/baz", FileSpec::Style::posix);
+
+  EXPECT_TRUE(Match(SourceLocationSpec(fs, 4), SourceLocationSpec(fs, 4)));
+
+  EXPECT_FALSE(
+  Match(SourceLocationSpec(fs, 4), SourceLocationSpec(other_fs, 4)));
+  EXPECT_FALSE(Match(SourceLocationSpec(fs, 4), SourceLocationSpec(fs, 19)));
+  EXPECT_FALSE(
+  Match(SourceLocationSpec(fs, 4), SourceLocationSpec(other_fs, 19)));
+
+  EXPECT_TRUE(
+  Match(SourceLocationSpec(fs, 4, 19), SourceLocationSpec(fs, 4, 19)));
+  EXPECT_TRUE(
+  Match(SourceLocationSpec(fs, 4, 19), SourceLocationSpec(fs, 4, 96)));
+
+  EXPECT_FALSE(Match(SourceLocationSpec(fs, 4, 19),
+ SourceLocationSpec(other_fs, 4, 96)));
+  

[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

That's sure fine, thanks for the reply. Great there is at least the plan to 
review it in the future. I understand you are busy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96236

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

[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I would love to do something about this, but currently I have no idea when I 
would be able to take on a review of this size. I know it's not what you wanted 
to hear. Sorry. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96236

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

[Lldb-commits] [PATCH] D91963: [lldb] [test/Register] Initial tests for regsets in core dumps [WIP]

[Pavel Labath via Phabricator via lldb-commits]

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

I like this.

I should note that, in the mean time, obj2yaml support for program headers has 
been greatly improved, to the point where one may actually be able to generate 
a reasonably-looking core file from yaml. However, obj2yaml support is lagging 
behind a bit, so one could not produce an initial yaml file and would have to 
start from scratch instead. I would love to get the missing bits of 
functionality into yaml2obj so this could all be yaml-ified, but I think this 
is pretty cool nonetheless...


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

https://reviews.llvm.org/D91963

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

[Lldb-commits] [lldb] 953f580 - [lldb/test] Modernize lldb-server test inferior

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-21T17:02:41+02:00
New Revision: 953f580b904989af11a69a2fbcb046cf0e10cdc5

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

LOG: [lldb/test] Modernize lldb-server test inferior

Avoid c string manipulation by introducing a StringRef-like
consume_front operation.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/main.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/main.cpp 
b/lldb/test/API/tools/lldb-server/main.cpp
index f719e4bc52f8..f1f576ab09bf 100644
--- a/lldb/test/API/tools/lldb-server/main.cpp
+++ b/lldb/test/API/tools/lldb-server/main.cpp
@@ -20,23 +20,6 @@
 #include 
 #include 
 
-static const char *const RETVAL_PREFIX = "retval:";
-static const char *const SLEEP_PREFIX = "sleep:";
-static const char *const STDERR_PREFIX = "stderr:";
-static const char *const SET_MESSAGE_PREFIX = "set-message:";
-static const char *const PRINT_MESSAGE_COMMAND = "print-message:";
-static const char *const GET_DATA_ADDRESS_PREFIX = "get-data-address-hex:";
-static const char *const GET_STACK_ADDRESS_COMMAND = "get-stack-address-hex:";
-static const char *const GET_HEAP_ADDRESS_COMMAND = "get-heap-address-hex:";
-
-static const char *const GET_CODE_ADDRESS_PREFIX = "get-code-address-hex:";
-static const char *const CALL_FUNCTION_PREFIX = "call-function:";
-
-static const char *const THREAD_PREFIX = "thread:";
-static const char *const THREAD_COMMAND_NEW = "new";
-static const char *const THREAD_COMMAND_PRINT_IDS = "print-ids";
-static const char *const THREAD_COMMAND_SEGFAULT = "segfault";
-
 static const char *const PRINT_PID_COMMAND = "print-pid";
 
 static bool g_print_thread_ids = false;
@@ -202,6 +185,14 @@ static void *thread_func(void *arg) {
   return nullptr;
 }
 
+static bool consume_front(std::string , const std::string ) {
+  if (str.find(front) != 0)
+return false;
+
+  str = str.substr(front.size());
+  return true;
+}
+
 int main(int argc, char **argv) {
   lldb_enable_attach();
 
@@ -232,15 +223,16 @@ int main(int argc, char **argv) {
 
   // Process command line args.
   for (int i = 1; i < argc; ++i) {
-if (std::strstr(argv[i], STDERR_PREFIX)) {
+std::string arg = argv[i];
+if (consume_front(arg, "stderr:")) {
   // Treat remainder as text to go to stderr.
-  fprintf(stderr, "%s\n", (argv[i] + strlen(STDERR_PREFIX)));
-} else if (std::strstr(argv[i], RETVAL_PREFIX)) {
+  fprintf(stderr, "%s\n", arg.c_str());
+} else if (consume_front(arg, "retval:")) {
   // Treat as the return value for the program.
-  return_value = std::atoi(argv[i] + strlen(RETVAL_PREFIX));
-} else if (std::strstr(argv[i], SLEEP_PREFIX)) {
+  return_value = std::atoi(arg.c_str());
+} else if (consume_front(arg, "sleep:")) {
   // Treat as the amount of time to have this process sleep (in seconds).
-  int sleep_seconds_remaining = std::atoi(argv[i] + strlen(SLEEP_PREFIX));
+  int sleep_seconds_remaining = std::atoi(arg.c_str());
 
   // Loop around, sleeping until all sleep time is used up.  Note that
   // signals will cause sleep to end early with the number of seconds
@@ -248,32 +240,31 @@ int main(int argc, char **argv) {
   std::this_thread::sleep_for(
   std::chrono::seconds(sleep_seconds_remaining));
 
-} else if (std::strstr(argv[i], SET_MESSAGE_PREFIX)) {
+} else if (consume_front(arg, "set-message:")) {
   // Copy the contents after "set-message:" to the g_message buffer.
   // Used for reading inferior memory and verifying contents match
   // expectations.
-  strncpy(g_message, argv[i] + strlen(SET_MESSAGE_PREFIX),
-  sizeof(g_message));
+  strncpy(g_message, arg.c_str(), sizeof(g_message));
 
   // Ensure we're null terminated.
   g_message[sizeof(g_message) - 1] = '\0';
 
-} else if (std::strstr(argv[i], PRINT_MESSAGE_COMMAND)) {
+} else if (consume_front(arg, "print-message:")) {
   std::lock_guard lock(g_print_mutex);
   printf("message: %s\n", g_message);
-} else if (std::strstr(argv[i], GET_DATA_ADDRESS_PREFIX)) {
+} else if (consume_front(arg, "get-data-address-hex:")) {
   volatile void *data_p = nullptr;
 
-  if (std::strstr(argv[i] + strlen(GET_DATA_ADDRESS_PREFIX), "g_message"))
+  if (arg == "g_message")
 data_p = _message[0];
-  else if (std::strstr(argv[i] + strlen(GET_DATA_ADDRESS_PREFIX), "g_c1"))
+  else if (arg == "g_c1")
 data_p = _c1;
-  else if (std::strstr(argv[i] + strlen(GET_DATA_ADDRESS_PREFIX), "g_c2"))
+  else if (arg == "g_c2")
 data_p = _c2;
 
   std::lock_guard lock(g_print_mutex);
   printf("data address: %p\n", data_p);
-} else if (std::strstr(argv[i], GET_HEAP_ADDRESS_COMMAND)) {
+  

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

mgorny updated this revision to Diff 339217.
mgorny marked an inline comment as done.
mgorny added a comment.

Remove the test, update the API to pass `std::unique_ptr<>` instance instead of 
ref.


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

https://reviews.llvm.org/D100191

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===
--- lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -25,6 +25,13 @@
   MOCK_METHOD2(ProcessStateChanged,
void(NativeProcessProtocol *Process, StateType State));
   MOCK_METHOD1(DidExec, void(NativeProcessProtocol *Process));
+  MOCK_METHOD2(NewSubprocessImpl,
+   void(NativeProcessProtocol *parent_process,
+std::unique_ptr _process));
+  void NewSubprocess(NativeProcessProtocol *parent_process,
+ std::unique_ptr child_process) {
+NewSubprocessImpl(parent_process, child_process);
+  }
 };
 
 // NB: This class doesn't use the override keyword to avoid
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -378,9 +378,7 @@
 return eServerPacketType_C;
 
   case 'D':
-if (packet_size == 1)
-  return eServerPacketType_D;
-break;
+return eServerPacketType_D;
 
   case 'g':
 return eServerPacketType_g;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -78,6 +78,10 @@
 
   void DidExec(NativeProcessProtocol *process) override;
 
+  void
+  NewSubprocess(NativeProcessProtocol *parent_process,
+std::unique_ptr child_process) override;
+
   Status InitializeConnection(std::unique_ptr connection);
 
 protected:
@@ -89,7 +93,8 @@
   NativeProcessProtocol *m_current_process;
   NativeProcessProtocol *m_continue_process;
   std::recursive_mutex m_debugged_process_mutex;
-  std::unique_ptr m_debugged_process_up;
+  std::unordered_map>
+  m_debugged_processes;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -249,14 +249,14 @@
 
   {
 std::lock_guard guard(m_debugged_process_mutex);
-assert(!m_debugged_process_up && "lldb-server creating debugged "
- "process but one already exists");
+assert(m_debugged_processes.empty() && "lldb-server creating debugged "
+   "process but one already exists");
 auto process_or =
 m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
 if (!process_or)
   return Status(process_or.takeError());
-m_debugged_process_up = std::move(*process_or);
-m_continue_process = m_current_process = m_debugged_process_up.get();
+m_continue_process = m_current_process = process_or->get();
+m_debugged_processes[m_current_process->GetID()] = std::move(*process_or);
   }
 
   SetEnabledExtensions(*m_current_process);
@@ -273,10 +273,10 @@
 LLDB_LOG(log,
  "pid = {0}: setting up stdout/stderr redirection via $O "
  "gdb-remote commands",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
 
 // Setup stdout/stderr mapping from inferior to $O
-auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
+auto terminal_fd = m_current_process->GetTerminalFileDescriptor();
 if (terminal_fd >= 0) {
   LLDB_LOGF(log,
 "ProcessGDBRemoteCommunicationServerLLGS::%s setting "
@@ -295,12 +295,12 @@
 LLDB_LOG(log,
  "pid = {0} skipping stdout/stderr redirection via $O: inferior "
  "will communicate over client-provided file descriptors",
- m_debugged_process_up->GetID());
+ m_current_process->GetID());
   }
 
   printf("Launched '%s' as process %" PRIu64 "...\n",
  

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

[Raphael Isemann via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe2039142f6b1: Some FormatEntity.cpp cleanup and unit testing 
(authored by nealsid, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98153

Files:
  lldb/include/lldb/Core/FormatEntity.h
  lldb/source/Core/FormatEntity.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/FormatEntityTest.cpp

Index: lldb/unittests/Core/FormatEntityTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/FormatEntityTest.cpp
@@ -0,0 +1,159 @@
+//===-- FormatEntityTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/FormatEntity.h"
+#include "lldb/Utility/Status.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+using Definition = FormatEntity::Entry::Definition;
+using Entry = FormatEntity::Entry;
+
+TEST(FormatEntityTest, DefinitionConstructionNameAndType) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid);
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, nullptr);
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(d.data, 0UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameAndString) {
+  Definition d("foo", "string");
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, "string");
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::EscapeCode);
+  EXPECT_EQ(d.data, 0UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameTypeData) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
+
+  EXPECT_EQ(d.name, "foo");
+  EXPECT_EQ(d.string, nullptr);
+  EXPECT_EQ(d.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(d.data, 33UL);
+  EXPECT_EQ(d.num_children, 0UL);
+  EXPECT_EQ(d.children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+TEST(FormatEntityTest, DefinitionConstructionNameTypeChildren) {
+  Definition d("foo", FormatEntity::Entry::Type::Invalid, 33);
+  Definition parent("parent", FormatEntity::Entry::Type::Invalid, 1, );
+  EXPECT_EQ(parent.name, "parent");
+  EXPECT_EQ(parent.string, nullptr);
+  EXPECT_EQ(parent.type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(parent.num_children, 1UL);
+  EXPECT_EQ(parent.children, );
+  EXPECT_FALSE(parent.keep_separator);
+
+  EXPECT_EQ(parent.children[0].name, "foo");
+  EXPECT_EQ(parent.children[0].string, nullptr);
+  EXPECT_EQ(parent.children[0].type, FormatEntity::Entry::Type::Invalid);
+  EXPECT_EQ(parent.children[0].data, 33UL);
+  EXPECT_EQ(parent.children[0].num_children, 0UL);
+  EXPECT_EQ(parent.children[0].children, nullptr);
+  EXPECT_FALSE(d.keep_separator);
+}
+
+constexpr llvm::StringRef lookupStrings[] = {
+"${addr.load}",
+"${addr.file}",
+"${ansi.fg.black}",
+"${ansi.fg.red}",
+"${ansi.fg.green}",
+"${ansi.fg.yellow}",
+"${ansi.fg.blue}",
+"${ansi.fg.purple}",
+"${ansi.fg.cyan}",
+"${ansi.fg.white}",
+"${ansi.bg.black}",
+"${ansi.bg.red}",
+"${ansi.bg.green}",
+"${ansi.bg.yellow}",
+"${ansi.bg.blue}",
+"${ansi.bg.purple}",
+"${ansi.bg.cyan}",
+"${ansi.bg.white}",
+"${file.basename}",
+"${file.dirname}",
+"${file.fullpath}",
+"${frame.index}",
+"${frame.pc}",
+"${frame.fp}",
+"${frame.sp}",
+"${frame.flags}",
+"${frame.no-debug}",
+"${frame.reg.*}",
+"${frame.is-artificial}",
+"${function.id}",
+"${function.name}",
+"${function.name-without-args}",
+"${function.name-with-args}",
+"${function.mangled-name}",
+"${function.addr-offset}",
+"${function.concrete-only-addr-offset-no-padding}",
+"${function.line-offset}",
+"${function.pc-offset}",
+"${function.initial-function}",
+"${function.changed}",
+"${function.is-optimized}",
+"${line.file.basename}",
+"${line.file.dirname}",
+"${line.file.fullpath}",
+"${line.number}",
+"${line.column}",
+"${line.start-addr}",
+"${line.end-addr}",
+"${module.file.basename}",
+"${module.file.dirname}",
+"${module.file.fullpath}",
+"${process.id}",
+"${process.name}",
+"${process.file.basename}",
+"${process.file.dirname}",
+"${process.file.fullpath}",
+"${script.frame}",
+"${script.process}",
+"${script.target}",
+

[Lldb-commits] [lldb] e203914 - Some FormatEntity.cpp cleanup and unit testing

[Raphael Isemann via lldb-commits]

Author: Neal (nealsid)
Date: 2021-04-21T15:12:59+02:00
New Revision: e2039142f6b196e9a491c8e0b18faa033c4d9f10

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

LOG: Some FormatEntity.cpp cleanup and unit testing

Just fixing a few things I noticed as I am working on another feature for format
strings in the prompt: forward decls, adding constexpr constructors, various
checks, and unit tests for FormatEntity::Parse and new Definition constructors,
etc.

Reviewed By: teemperor

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

Added: 
lldb/unittests/Core/FormatEntityTest.cpp

Modified: 
lldb/include/lldb/Core/FormatEntity.h
lldb/source/Core/FormatEntity.cpp
lldb/unittests/Core/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Core/FormatEntity.h 
b/lldb/include/lldb/Core/FormatEntity.h
index 91999f64ab5ff..ca16e02c54a5a 100644
--- a/lldb/include/lldb/Core/FormatEntity.h
+++ b/lldb/include/lldb/Core/FormatEntity.h
@@ -9,9 +9,6 @@
 #ifndef LLDB_CORE_FORMATENTITY_H
 #define LLDB_CORE_FORMATENTITY_H
 
-#include "lldb/Utility/CompletionRequest.h"
-#include "lldb/Utility/FileSpec.h"
-#include "lldb/Utility/Status.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-types.h"
 #include 
@@ -23,12 +20,16 @@
 
 namespace lldb_private {
 class Address;
+class CompletionRequest;
 class ExecutionContext;
+class FileSpec;
+class Status;
 class Stream;
 class StringList;
 class SymbolContext;
 class ValueObject;
 }
+
 namespace llvm {
 class StringRef;
 }
@@ -103,20 +104,52 @@ class FormatEntity {
 };
 
 struct Definition {
+  /// The name/string placeholder that corresponds to this definition.
   const char *name;
-  const char *string; // Insert this exact string into the output
-  Entry::Type type;
-  uint64_t data;
-  uint32_t num_children;
-  Definition *children; // An array of "num_children" Definition entries,
-  bool keep_separator;
+  /// Insert this exact string into the output
+  const char *string = nullptr;
+  /// Entry::Type corresponding to this definition.
+  const Entry::Type type;
+  /// Data that is returned as the value of the format string.
+  const uint64_t data = 0;
+  /// The number of children of this node in the tree of format strings.
+  const uint32_t num_children = 0;
+  /// An array of "num_children" Definition entries.
+  const Definition *children = nullptr;
+  /// Whether the separator is kept during parsing or not.  It's used
+  /// for entries with parameters.
+  const bool keep_separator = false;
+
+  constexpr Definition(const char *name, const FormatEntity::Entry::Type t)
+  : name(name), type(t) {}
+
+  constexpr Definition(const char *name, const char *string)
+  : name(name), string(string), type(Entry::Type::EscapeCode) {}
+
+  constexpr Definition(const char *name, const FormatEntity::Entry::Type t,
+   const uint64_t data)
+  : name(name), string(nullptr), type(t), data(data) {}
+
+  constexpr Definition(const char *name, const FormatEntity::Entry::Type t,
+   const uint64_t num_children,
+   const Definition *children,
+   const bool keep_separator = false)
+  : name(name), type(t), num_children(num_children), 
children(children),
+keep_separator(keep_separator) {}
 };
 
+template 
+static constexpr Definition
+DefinitionWithChildren(const char *name, const FormatEntity::Entry::Type t,
+   const Definition ()[N],
+   bool keep_separator = false) {
+  return Definition(name, t, N, children, keep_separator);
+}
+
 Entry(Type t = Type::Invalid, const char *s = nullptr,
   const char *f = nullptr)
-: string(s ? s : ""), printf_format(f ? f : ""), children(),
-  definition(nullptr), type(t), fmt(lldb::eFormatDefault), number(0),
-  deref(false) {}
+: string(s ? s : ""), printf_format(f ? f : ""), children(), type(t),
+  fmt(lldb::eFormatDefault), number(0), deref(false) {}
 
 Entry(llvm::StringRef s);
 Entry(char ch);
@@ -133,7 +166,6 @@ class FormatEntity {
   string.clear();
   printf_format.clear();
   children.clear();
-  definition = nullptr;
   type = Type::Invalid;
   fmt = lldb::eFormatDefault;
   number = 0;
@@ -157,8 +189,6 @@ class FormatEntity {
   }
   if (children != rhs.children)
 return false;
-  if (definition != rhs.definition)
-return false;
   if (type != rhs.type)
 return false;
   if (fmt != rhs.fmt)
@@ -171,7 +201,6 @@ class FormatEntity {
 

[Lldb-commits] [PATCH] D98153: Some FormatEntity.cpp cleanup and unit testing

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.

Thanks for updating and thanks for the patch! I can land it right now.




Comment at: lldb/unittests/Core/FormatEntityTest.cpp:154
+TEST(FormatEntity, LookupAllEntriesInTree) {
+  for (const auto  : lookupStrings) {
+Entry e;

nealsid wrote:
> teemperor wrote:
> > teemperor wrote:
> > > You could just use `llvm::StringRef` here (it's shorter and more 
> > > descriptive than the `auto`).
> > I actually meant `llvm::StringRef` as the whole type (StringRef references 
> > are not really useful as StringRef is already a lightweight String 
> > reference)
> Done - kept it const since StringRef appears to depend on that for some of 
> its methods.  I was looking through StringRef (and the use of optionals for 
> something else) and as far as I understand, which could be entirely wrong, 
> it's meant to provide a single interface over unowned character data whether 
> it comes from a std::string or char*, and thought using string_view would be 
> good, as well - is moving to C++17 on the roadmap? I know it's one of those 
> things that everyone is in favor of and wants to do but nobody has time for :)
> 
It's on the roadmap but not sure when that will happen. I think that depends on 
when the interest in having C++17 features is bigger than the pain it will 
inflict on people stuck on outdated runtime environments that need 
bootstrapping for C++17.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98153

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

[Lldb-commits] [PATCH] D100196: [lldb] [gdb-remote server] Introduce new stop reasons for fork and vfork

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

mgorny added a comment.

For other readers: as discussed in D100191 , 
I've stripped client part of this (except for trivial enumerations). The server 
part will be used in tests but the stop reasons won't be reported to regular 
clients as they don't report `fork-events` support.


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

https://reviews.llvm.org/D100196

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

[Lldb-commits] [PATCH] D76964: Fix an issue where the IgnoreName function was not allowing "Class" to be looked up inside a namespace or other class.

[Raphael Isemann via Phabricator via lldb-commits]

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

I think this review got kind of stuck. I think the only missing thing is 
simplifying the `self.dbg.CreateTarget` call in the test, but otherwise this 
looks good to me. I'm just going to accept this as the bit of cleanup doesn't 
need another revision.

(Sorry for the delay, I kept clicking on this review and just saw unaddressed 
comments so I thought this wasn't done yet).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76964

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

[Lldb-commits] [PATCH] D100196: [lldb] [gdb-remote server] Introduce new stop reasons for fork and vfork

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

mgorny updated this revision to Diff 339198.
mgorny retitled this revision from "[lldb] Introduce new stop reasons for fork 
and vfork" to "[lldb] [gdb-remote server] Introduce new stop reasons for fork 
and vfork".
mgorny edited the summary of this revision.
mgorny added a comment.

@labath, does this subset look better?


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

https://reviews.llvm.org/D100196

Files:
  lldb/bindings/interface/SBThread.i
  lldb/bindings/interface/SBThreadPlan.i
  lldb/docs/python_api_enums.rst
  lldb/examples/python/performance.py
  lldb/include/lldb/API/SBThread.h
  lldb/include/lldb/API/SBThreadPlan.h
  lldb/include/lldb/Host/Debug.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/packages/Python/lldbsuite/test/lldbutil.py
  lldb/source/API/SBThread.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/Thread.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp
  lldb/tools/lldb-vscode/LLDBUtils.cpp

Index: lldb/tools/lldb-vscode/LLDBUtils.cpp
===
--- lldb/tools/lldb-vscode/LLDBUtils.cpp
+++ lldb/tools/lldb-vscode/LLDBUtils.cpp
@@ -56,6 +56,9 @@
   case lldb::eStopReasonException:
   case lldb::eStopReasonExec:
   case lldb::eStopReasonProcessorTrace:
+  case lldb::eStopReasonFork:
+  case lldb::eStopReasonVFork:
+  case lldb::eStopReasonVForkDone:
 return true;
   case lldb::eStopReasonThreadExiting:
   case lldb::eStopReasonInvalid:
Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -877,6 +877,15 @@
   case lldb::eStopReasonExec:
 body.try_emplace("reason", "entry");
 break;
+  case lldb::eStopReasonFork:
+body.try_emplace("reason", "fork");
+break;
+  case lldb::eStopReasonVFork:
+body.try_emplace("reason", "vfork");
+break;
+  case lldb::eStopReasonVForkDone:
+body.try_emplace("reason", "vforkdone");
+break;
   case lldb::eStopReasonThreadExiting:
   case lldb::eStopReasonInvalid:
   case lldb::eStopReasonNone:
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1679,6 +1679,12 @@
 return "exception";
   case eStopReasonExec:
 return "exec";
+  case eStopReasonFork:
+return "fork";
+  case eStopReasonVFork:
+return "vfork";
+  case eStopReasonVForkDone:
+return "vfork done";
   case eStopReasonPlanComplete:
 return "plan complete";
   case eStopReasonThreadExiting:
Index: lldb/source/Target/StackFrameList.cpp
===
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -131,6 +131,9 @@
   case eStopReasonWatchpoint:
   case eStopReasonException:
   case eStopReasonExec:
+  case eStopReasonFork:
+  case eStopReasonVFork:
+  case eStopReasonVForkDone:
   case eStopReasonSignal:
 // In all these cases we want to stop in the deepest frame.
 m_current_inlined_pc = curr_pc;
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -822,6 +822,9 @@
 case eStopReasonWatchpoint:
 case eStopReasonException:
 case eStopReasonExec:
+case eStopReasonFork:
+case eStopReasonVFork:
+case eStopReasonVForkDone:
 case eStopReasonThreadExiting:
 case eStopReasonInstrumentation:
 case eStopReasonProcessorTrace:
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -659,6 +659,12 @@
 return "exec";
   case eStopReasonProcessorTrace:
 return "processor trace";
+  case eStopReasonFork:
+return "fork";
+  case eStopReasonVFork:
+return "vfork";
+  case eStopReasonVForkDone:
+return "vforkdone";
   case eStopReasonInstrumentation:
   case eStopReasonInvalid:
   case eStopReasonPlanComplete:
@@ -934,6 +940,14 @@
 }
   }
 
+  // Include child process PID/TID for forks.
+  if (tid_stop_info.reason == eStopReasonFork ||
+  tid_stop_info.reason == eStopReasonVFork) {
+response.Printf("%s:p%" PRIx64 ".%" PRIx64 ";", reason_str,
+tid_stop_info.details.fork.child_pid,
+tid_stop_info.details.fork.child_tid);
+  }
+
   return SendPacketNoLock(response.GetString());
 }
 
Index: lldb/source/API/SBThread.cpp

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

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

mgorny updated this revision to Diff 339193.
mgorny marked an inline comment as done.
mgorny added a comment.

Implemented the suggested changes, including `bool()` use in 
`SetEnabledExtensions()`. Removed the client announcements of `fork-events` and 
`vfork-events` support.

I've left the server parts of `fork-events` and `vfork-events`. It won't 
trigger until both the process plugin and the client indicates support too.


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

https://reviews.llvm.org/D100153

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -103,6 +103,8 @@
   bool m_thread_suffix_supported = false;
   bool m_list_threads_in_stop_reply = false;
 
+  NativeProcessProtocol::Extension m_extensions_supported = {};
+
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
@@ -264,6 +266,9 @@
   llvm::Expected ReadTid(StringExtractorGDBRemote ,
   bool allow_all, lldb::pid_t default_pid);
 
+  // Call SetEnabledExtensions() with appropriate flags on the process.
+  void SetEnabledExtensions(NativeProcessProtocol );
+
   // For GDBRemoteCommunicationServerLLGS only
   GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) =
   delete;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -259,6 +259,8 @@
 m_continue_process = m_current_process = m_debugged_process_up.get();
   }
 
+  SetEnabledExtensions(*m_current_process);
+
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as
   // needed. llgs local-process debugging may specify PTY paths, which will
   // make these file actions non-null process launch -i/e/o will also make
@@ -327,6 +329,7 @@
   }
   m_debugged_process_up = std::move(*process_or);
   m_continue_process = m_current_process = m_debugged_process_up.get();
+  SetEnabledExtensions(*m_current_process);
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_up->GetTerminalFileDescriptor();
@@ -3557,7 +3560,7 @@
 
 std::vector GDBRemoteCommunicationServerLLGS::HandleFeatures(
 const llvm::ArrayRef client_features) {
-  auto ret =
+  std::vector ret =
   GDBRemoteCommunicationServerCommon::HandleFeatures(client_features);
   ret.insert(ret.end(), {
 "QThreadSuffixSupported+", "QListThreadsInStopReply+",
@@ -3566,5 +3569,36 @@
 "QPassSignals+", "qXfer:auxv:read+", "qXfer:libraries-svr4:read+",
 #endif
   });
+
+  // check for client features
+  using Extension = NativeProcessProtocol::Extension;
+  m_extensions_supported = {};
+  for (llvm::StringRef x : client_features)
+m_extensions_supported |= llvm::StringSwitch(x)
+  .Case("fork-events+", Extension::fork)
+  .Case("vfork-events+", Extension::vfork)
+  .Default({});
+  m_extensions_supported &= m_process_factory.GetSupportedExtensions();
+
+  // report only if actually supported
+  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::fork))
+ret.push_back("fork-events+");
+  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::vfork))
+ret.push_back("vfork-events+");
+
+  if (m_debugged_process_up)
+SetEnabledExtensions(*m_debugged_process_up);
   return ret;
 }
+
+void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
+NativeProcessProtocol ) {
+  NativeProcessProtocol::Extension flags = {};
+  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::fork))
+flags |= NativeProcessProtocol::Extension::fork;
+  if (bool(m_extensions_supported & NativeProcessProtocol::Extension::vfork))
+flags |= NativeProcessProtocol::Extension::vfork;
+
+  assert(!bool(flags & ~m_process_factory.GetSupportedExtensions()));
+  process.SetEnabledExtensions(flags);
+}
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ 

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D100191#2704748 , @mgorny wrote:

> In D100191#2704681 , @labath wrote:
>
>> In D100191#2704492 , @mgorny wrote:
>>
>>> In D100191#2704403 , @labath 
>>> wrote:
>>>
 Let's identify the set of patches needed to make this testable via the 
 lldb-server suite (this one, D100153 , 
 D100208  (or equivalent for some other 
 os), and what else?) and test that?
>>>
>>> In its current form, D100208  relies at 
>>> least on D100196  as well. I suppose we 
>>> might get away without other patches for now. My logic is that as long as 
>>> client doesn't indicate fork support, the regular LLDB behavior won't 
>>> change. We can mock-enable `fork-events` in the server tests to get things 
>>> rolling, unless I'm missing something.
>>
>> Sounds great.
>>
>>> I think we could avoid merging D100196  
>>> if I split D100208  into two parts, the 
>>> earlier part just calling `NewSubprocess()` without actually reporting 
>>> stop. This won't be really functional (or used in real sessions) but should 
>>> suffice for testing.
>>
>> I'm not sure how would that work -- you'd still have to provide a way to 
>> notify the client (test) about the new process and its pid, so that the test 
>> can assert something meaningful. Let's just keep D100196 
>> , but leave out the client specific parts 
>> -- just keep it about adding the new enums and relevant server code (trivial 
>> case switches are fine).
>
> Do you mean the `DidFoo()` methods, or the whole `StopInfo` stuff along with 
> the logic in `ProcessGDBRemote`?

All of it.

> If the latter, should I add some asserts for the unhandled stop reasons or 
> just ignore them for now?

Nah, just ignore it completely lldb-server will not be sending those as long as 
the client does not advertise support. It's possible lldb will misbehave if a 
broken/evil server sends reasons like these, but that's not a problem for this 
patch.


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

https://reviews.llvm.org/D100191

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

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

mgorny added a comment.

In D100191#2704681 , @labath wrote:

> In D100191#2704492 , @mgorny wrote:
>
>> In D100191#2704403 , @labath wrote:
>>
>>> Let's identify the set of patches needed to make this testable via the 
>>> lldb-server suite (this one, D100153 , 
>>> D100208  (or equivalent for some other 
>>> os), and what else?) and test that?
>>
>> In its current form, D100208  relies at 
>> least on D100196  as well. I suppose we 
>> might get away without other patches for now. My logic is that as long as 
>> client doesn't indicate fork support, the regular LLDB behavior won't 
>> change. We can mock-enable `fork-events` in the server tests to get things 
>> rolling, unless I'm missing something.
>
> Sounds great.
>
>> I think we could avoid merging D100196  if 
>> I split D100208  into two parts, the 
>> earlier part just calling `NewSubprocess()` without actually reporting stop. 
>> This won't be really functional (or used in real sessions) but should 
>> suffice for testing.
>
> I'm not sure how would that work -- you'd still have to provide a way to 
> notify the client (test) about the new process and its pid, so that the test 
> can assert something meaningful. Let's just keep D100196 
> , but leave out the client specific parts 
> -- just keep it about adding the new enums and relevant server code (trivial 
> case switches are fine).

Do you mean the `DidFoo()` methods, or the whole `StopInfo` stuff along with 
the logic in `ProcessGDBRemote`? If the latter, should I add some asserts for 
the unhandled stop reasons or just ignore them for now?


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

https://reviews.llvm.org/D100191

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

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D100191#2704492 , @mgorny wrote:

> In D100191#2704403 , @labath wrote:
>
>> Let's identify the set of patches needed to make this testable via the 
>> lldb-server suite (this one, D100153 , 
>> D100208  (or equivalent for some other 
>> os), and what else?) and test that?
>
> In its current form, D100208  relies at 
> least on D100196  as well. I suppose we 
> might get away without other patches for now. My logic is that as long as 
> client doesn't indicate fork support, the regular LLDB behavior won't change. 
> We can mock-enable `fork-events` in the server tests to get things rolling, 
> unless I'm missing something.

Sounds great.

> I think we could avoid merging D100196  if 
> I split D100208  into two parts, the 
> earlier part just calling `NewSubprocess()` without actually reporting stop. 
> This won't be really functional (or used in real sessions) but should suffice 
> for testing.

I'm not sure how would that work -- you'd still have to provide a way to notify 
the client (test) about the new process and its pid, so that the test can 
assert something meaningful. Let's just keep D100196 
, but leave out the client specific parts -- 
just keep it about adding the new enums and relevant server code (trivial case 
switches are fine).

> To be honest, I really like to keep these patches small, even if it means it 
> takes 2 or 3 patches to make a test. I would prefer just adding the test to 
> the last patch in series.

I don't mind the multiple patches. These are a bit on the smaller side, but 
they are definitely easier to review than a single giant patch). It does create 
confusion though, since that is not the usual mode of operation around here.


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

https://reviews.llvm.org/D100191

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

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3562-3567
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)

mgorny wrote:
> mgorny wrote:
> > labath wrote:
> > > Maybe drop the `== Extension::[v]fork` part?
> > Can't do that, `enum class` doesn't convert to `bool`. In fact, I tried a 
> > few more or less crazy ideas to make this work, and none worked ;-).
> The next best thing I was able to do is check `!= 
> NativeProcessProtocol::Extension()` which is a little bit shorter. Maybe I 
> could try adding `operator==` and `!=` against, say, `nullptr` or some 
> special constant?
An explicit bool cast works though, and is less fancy. Maybe if we combine this 
the `m_enabled_extensions` idea, we could do something like:
```
using Extension = NativeProcessProtocol::Extension;
Extension client_extensions{};
for (StringRef x : client_features)
  client_extensions |= llvm::StringSwitch(x).Case("fork-events+", 
Extension::fork).Case("vfork-events+", Extension::vfork).Default({});
m_enabled_extensions = client_extensions & 
m_process_factory.GetSupportedExtensions();
if (bool(m_enabled_extensions & Extension::fork))
  ret.push_back("fork-events+");
if (bool(m_enabled_extensions & Extension::vfork))
  ret.push_back("vfork-events+");
```


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

https://reviews.llvm.org/D100153

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

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

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

mgorny marked 4 inline comments as done.
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3562-3567
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)

mgorny wrote:
> labath wrote:
> > Maybe drop the `== Extension::[v]fork` part?
> Can't do that, `enum class` doesn't convert to `bool`. In fact, I tried a few 
> more or less crazy ideas to make this work, and none worked ;-).
The next best thing I was able to do is check `!= 
NativeProcessProtocol::Extension()` which is a little bit shorter. Maybe I 
could try adding `operator==` and `!=` against, say, `nullptr` or some special 
constant?


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

https://reviews.llvm.org/D100153

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

[Lldb-commits] [PATCH] D98370: [lldb] Fix SBValue::Persist() for constant values

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

In D98370#2686515 , @jingham wrote:

> If persisting already persistent variables is indeed the only failure case, 
> then I wonder if it wouldn't be more straightforward to just see if the 
> ValueObject is already a persistent variable and have Persist just return the 
> incoming variable.

Persisting already persistent variables is not the only valid use case, one 
might also want to persist variables created via 
`SBTarget::CreateValueFromData()`.

I guess it is currently not clear from the API, but I would expect 
`SBValue::Persist()` to produces a new value every time:

  auto v1 = value.Persist();
  auto v2 = value.Persist();
  
  assert(v1.GetName() != v1.GetName())  // modulo comparing char*...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98370

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

[Lldb-commits] [PATCH] D100418: [lldb] [MainLoop] Support multiple callbacks per signal

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG08ce2ba51803: [lldb] [MainLoop] Support multiple callbacks 
per signal (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100418

Files:
  lldb/include/lldb/Host/MainLoop.h
  lldb/source/Host/common/MainLoop.cpp
  lldb/unittests/Host/MainLoopTest.cpp

Index: lldb/unittests/Host/MainLoopTest.cpp
===
--- lldb/unittests/Host/MainLoopTest.cpp
+++ lldb/unittests/Host/MainLoopTest.cpp
@@ -152,4 +152,49 @@
   killer.join();
   ASSERT_EQ(1u, callback_count);
 }
+
+// Test that two callbacks can be registered for the same signal
+// and unregistered independently.
+TEST_F(MainLoopTest, TwoSignalCallbacks) {
+  MainLoop loop;
+  Status error;
+  int callback2_count = 0;
+  int callback3_count = 0;
+
+  auto handle = loop.RegisterSignal(SIGUSR1, make_callback(), error);
+  ASSERT_TRUE(error.Success());
+
+  {
+// Run a single iteration with two callbacks enabled.
+auto handle2 = loop.RegisterSignal(
+SIGUSR1, [&](MainLoopBase ) { ++callback2_count; }, error);
+ASSERT_TRUE(error.Success());
+
+kill(getpid(), SIGUSR1);
+ASSERT_TRUE(loop.Run().Success());
+ASSERT_EQ(1u, callback_count);
+ASSERT_EQ(1u, callback2_count);
+ASSERT_EQ(0u, callback3_count);
+  }
+
+  {
+// Make sure that remove + add new works.
+auto handle3 = loop.RegisterSignal(
+SIGUSR1, [&](MainLoopBase ) { ++callback3_count; }, error);
+ASSERT_TRUE(error.Success());
+
+kill(getpid(), SIGUSR1);
+ASSERT_TRUE(loop.Run().Success());
+ASSERT_EQ(2u, callback_count);
+ASSERT_EQ(1u, callback2_count);
+ASSERT_EQ(1u, callback3_count);
+  }
+
+  // Both extra callbacks should be unregistered now.
+  kill(getpid(), SIGUSR1);
+  ASSERT_TRUE(loop.Run().Success());
+  ASSERT_EQ(3u, callback_count);
+  ASSERT_EQ(1u, callback2_count);
+  ASSERT_EQ(1u, callback3_count);
+}
 #endif
Index: lldb/source/Host/common/MainLoop.cpp
===
--- lldb/source/Host/common/MainLoop.cpp
+++ lldb/source/Host/common/MainLoop.cpp
@@ -302,13 +302,15 @@
   error.SetErrorString("Signal polling is not supported on this platform.");
   return nullptr;
 #else
-  if (m_signals.find(signo) != m_signals.end()) {
-error.SetErrorStringWithFormat("Signal %d already monitored.", signo);
-return nullptr;
+  auto signal_it = m_signals.find(signo);
+  if (signal_it != m_signals.end()) {
+auto callback_it = signal_it->second.callbacks.insert(
+signal_it->second.callbacks.end(), callback);
+return SignalHandleUP(new SignalHandle(*this, signo, callback_it));
   }
 
   SignalInfo info;
-  info.callback = callback;
+  info.callbacks.push_back(callback);
   struct sigaction new_action;
   new_action.sa_sigaction = 
   new_action.sa_flags = SA_SIGINFO;
@@ -338,9 +340,10 @@
 _action.sa_mask, _set);
   assert(ret == 0 && "pthread_sigmask failed");
   info.was_blocked = sigismember(_set, signo);
-  m_signals.insert({signo, info});
+  auto insert_ret = m_signals.insert({signo, info});
 
-  return SignalHandleUP(new SignalHandle(*this, signo));
+  return SignalHandleUP(new SignalHandle(
+  *this, signo, insert_ret.first->second.callbacks.begin()));
 #endif
 }
 
@@ -350,13 +353,19 @@
   assert(erased);
 }
 
-void MainLoop::UnregisterSignal(int signo) {
+void MainLoop::UnregisterSignal(int signo,
+std::list::iterator callback_it) {
 #if SIGNAL_POLLING_UNSUPPORTED
   Status("Signal polling is not supported on this platform.");
 #else
   auto it = m_signals.find(signo);
   assert(it != m_signals.end());
 
+  it->second.callbacks.erase(callback_it);
+  // Do not remove the signal handler unless all callbacks have been erased.
+  if (!it->second.callbacks.empty())
+return;
+
   sigaction(signo, >second.old_action, nullptr);
 
   sigset_t set;
@@ -398,8 +407,14 @@
 
 void MainLoop::ProcessSignal(int signo) {
   auto it = m_signals.find(signo);
-  if (it != m_signals.end())
-it->second.callback(*this); // Do the work
+  if (it != m_signals.end()) {
+// The callback may actually register/unregister signal handlers,
+// so we need to create a copy first.
+llvm::SmallVector callbacks_to_run{
+it->second.callbacks.begin(), it->second.callbacks.end()};
+for (auto  : callbacks_to_run)
+  x(*this); // Do the work
+  }
 }
 
 void MainLoop::ProcessReadObject(IOObject::WaitableHandle handle) {
Index: lldb/include/lldb/Host/MainLoop.h
===
--- lldb/include/lldb/Host/MainLoop.h
+++ lldb/include/lldb/Host/MainLoop.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/MainLoopBase.h"
 #include 

[Lldb-commits] [lldb] 08ce2ba - [lldb] [MainLoop] Support multiple callbacks per signal

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-04-21T12:18:20+02:00
New Revision: 08ce2ba518031425643ce3a4a0476f770c9b8dcd

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

LOG: [lldb] [MainLoop] Support multiple callbacks per signal

Support registering multiple callbacks for a single signal.  This is
necessary to support multiple co-existing native process instances, with
separate SIGCHLD handlers.

The system signal handler is registered on first request, additional
callback are added on subsequent requests.  The system signal handler
is removed when last callback is unregistered.

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

Added: 


Modified: 
lldb/include/lldb/Host/MainLoop.h
lldb/source/Host/common/MainLoop.cpp
lldb/unittests/Host/MainLoopTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/MainLoop.h 
b/lldb/include/lldb/Host/MainLoop.h
index 9ca5040b60a89..06785bbdbe246 100644
--- a/lldb/include/lldb/Host/MainLoop.h
+++ b/lldb/include/lldb/Host/MainLoop.h
@@ -13,6 +13,7 @@
 #include "lldb/Host/MainLoopBase.h"
 #include "llvm/ADT/DenseMap.h"
 #include 
+#include 
 
 #if !HAVE_PPOLL && !HAVE_SYS_EVENT_H && !defined(__ANDROID__)
 #define SIGNAL_POLLING_UNSUPPORTED 1
@@ -68,7 +69,7 @@ class MainLoop : public MainLoopBase {
 protected:
   void UnregisterReadObject(IOObject::WaitableHandle handle) override;
 
-  void UnregisterSignal(int signo);
+  void UnregisterSignal(int signo, std::list::iterator callback_it);
 
 private:
   void ProcessReadObject(IOObject::WaitableHandle handle);
@@ -76,14 +77,16 @@ class MainLoop : public MainLoopBase {
 
   class SignalHandle {
   public:
-~SignalHandle() { m_mainloop.UnregisterSignal(m_signo); }
+~SignalHandle() { m_mainloop.UnregisterSignal(m_signo, m_callback_it); }
 
   private:
-SignalHandle(MainLoop , int signo)
-: m_mainloop(mainloop), m_signo(signo) {}
+SignalHandle(MainLoop , int signo,
+ std::list::iterator callback_it)
+: m_mainloop(mainloop), m_signo(signo), m_callback_it(callback_it) {}
 
 MainLoop _mainloop;
 int m_signo;
+std::list::iterator m_callback_it;
 
 friend class MainLoop;
 SignalHandle(const SignalHandle &) = delete;
@@ -91,7 +94,7 @@ class MainLoop : public MainLoopBase {
   };
 
   struct SignalInfo {
-Callback callback;
+std::list callbacks;
 #if HAVE_SIGACTION
 struct sigaction old_action;
 #endif

diff  --git a/lldb/source/Host/common/MainLoop.cpp 
b/lldb/source/Host/common/MainLoop.cpp
index 02cabbc93550a..cdcb052217960 100644
--- a/lldb/source/Host/common/MainLoop.cpp
+++ b/lldb/source/Host/common/MainLoop.cpp
@@ -302,13 +302,15 @@ MainLoop::RegisterSignal(int signo, const Callback 
, Status ) {
   error.SetErrorString("Signal polling is not supported on this platform.");
   return nullptr;
 #else
-  if (m_signals.find(signo) != m_signals.end()) {
-error.SetErrorStringWithFormat("Signal %d already monitored.", signo);
-return nullptr;
+  auto signal_it = m_signals.find(signo);
+  if (signal_it != m_signals.end()) {
+auto callback_it = signal_it->second.callbacks.insert(
+signal_it->second.callbacks.end(), callback);
+return SignalHandleUP(new SignalHandle(*this, signo, callback_it));
   }
 
   SignalInfo info;
-  info.callback = callback;
+  info.callbacks.push_back(callback);
   struct sigaction new_action;
   new_action.sa_sigaction = 
   new_action.sa_flags = SA_SIGINFO;
@@ -338,9 +340,10 @@ MainLoop::RegisterSignal(int signo, const Callback 
, Status ) {
 _action.sa_mask, _set);
   assert(ret == 0 && "pthread_sigmask failed");
   info.was_blocked = sigismember(_set, signo);
-  m_signals.insert({signo, info});
+  auto insert_ret = m_signals.insert({signo, info});
 
-  return SignalHandleUP(new SignalHandle(*this, signo));
+  return SignalHandleUP(new SignalHandle(
+  *this, signo, insert_ret.first->second.callbacks.begin()));
 #endif
 }
 
@@ -350,13 +353,19 @@ void 
MainLoop::UnregisterReadObject(IOObject::WaitableHandle handle) {
   assert(erased);
 }
 
-void MainLoop::UnregisterSignal(int signo) {
+void MainLoop::UnregisterSignal(int signo,
+std::list::iterator callback_it) {
 #if SIGNAL_POLLING_UNSUPPORTED
   Status("Signal polling is not supported on this platform.");
 #else
   auto it = m_signals.find(signo);
   assert(it != m_signals.end());
 
+  it->second.callbacks.erase(callback_it);
+  // Do not remove the signal handler unless all callbacks have been erased.
+  if (!it->second.callbacks.empty())
+return;
+
   sigaction(signo, >second.old_action, nullptr);
 
   sigset_t set;
@@ -398,8 +407,14 @@ Status MainLoop::Run() {
 
 void MainLoop::ProcessSignal(int signo) {
   auto it = m_signals.find(signo);
-  if (it 

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

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

mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3562-3567
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)

labath wrote:
> Maybe drop the `== Extension::[v]fork` part?
Can't do that, `enum class` doesn't convert to `bool`. In fact, I tried a few 
more or less crazy ideas to make this work, and none worked ;-).



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:102-103
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+

labath wrote:
> I am wondering if this should just be `NativeProcessProtocol::Extension 
> m_enabled_extensions;` Can we think of an extension that would belong to 
> `NativeProcessProtocol::Extension`, but we would not want to store its state 
> in the GDBRemoteCommunicationServerLLGS class?
I can't think of any right now and I suppose even if there were one, there 
would probably be no big loss in using `Extension` here.


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

https://reviews.llvm.org/D100153

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

[Lldb-commits] [PATCH] D100153: [lldb] [Process] Introduce protocol extension support API

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:389
+m_enabled_extensions = flags;
+return llvm::Error::success();
+  }

mgorny wrote:
> labath wrote:
> > Are you sure that returning success is the best "default" behavior? Maybe 
> > the base implementation should always return an error (as it does not 
> > implement any extensions)? Or return success, only if one enables an empty 
> > set of extensions?
> I'm not sure whether we need error handling here at all.
> 
> The current impl doesn't do anything but setting an instance var here. The 
> original impl controlled events to `PTRACE_SETOPTIONS` but in the end I've 
> decided to enable tracing fork/vfork unconditionally, and just using 
> extensions to decide whether to handle it locally or defer to client.
> 
> I suppose it might make sense to review other patches first and get back to 
> this one once we're sure what we need.
I think I agree with you. Let's just drop error handling altogether. Maybe add 
something like `assert(features && ~m_process_factory.GetSupportedExtensions() 
== {})` to `GDBRemoteCommunicationServerLLGS::SetEnabledExtensions`.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3562-3567
+(process_features & NativeProcessProtocol::Extension::fork) ==
+NativeProcessProtocol::Extension::fork)
+  m_fork_events_supported = true;
+else if (x == "vfork-events+" &&
+ (process_features & NativeProcessProtocol::Extension::vfork) ==
+ NativeProcessProtocol::Extension::vfork)

Maybe drop the `== Extension::[v]fork` part?



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:102-103
 
+  bool m_fork_events_supported = false;
+  bool m_vfork_events_supported = false;
+

I am wondering if this should just be `NativeProcessProtocol::Extension 
m_enabled_extensions;` Can we think of an extension that would belong to 
`NativeProcessProtocol::Extension`, but we would not want to store its state in 
the GDBRemoteCommunicationServerLLGS class?


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

https://reviews.llvm.org/D100153

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

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

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

mgorny added a comment.

In D100191#2704403 , @labath wrote:

> Let's identify the set of patches needed to make this testable via the 
> lldb-server suite (this one, D100153 , 
> D100208  (or equivalent for some other os), 
> and what else?) and test that?

In its current form, D100208  relies at least 
on D100196  as well. I suppose we might get 
away without other patches for now. My logic is that as long as client doesn't 
indicate fork support, the regular LLDB behavior won't change. We can 
mock-enable `fork-events` in the server tests to get things rolling, unless I'm 
missing something.

I think we could avoid merging D100196  if I 
split D100208  into two parts, the earlier 
part just calling `NewSubprocess()` without actually reporting stop. This won't 
be really functional (or used in real sessions) but should suffice for testing.

To be honest, I really like to keep these patches small, even if it means it 
takes 2 or 3 patches to make a test. I would prefer just adding the test to the 
last patch in series.


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

https://reviews.llvm.org/D100191

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

[Lldb-commits] [lldb] cd64273 - [lldb/ELF] Fix IDs of synthetic eh_frame symbols

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-04-21T11:24:43+02:00
New Revision: cd64273f5ed39ec697ff1e20a1fe25ebd3502629

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

LOG: [lldb/ELF] Fix IDs of synthetic eh_frame symbols

The code used the total number of symbols to create a symbol ID for the
synthetic symbols. This is not correct because the IDs of real symbols
can be higher than their total number, as we do not add all symbols (and
in particular, we never add symbol zero, which is not a real symbol).

This meant we could have symbols with duplicate IDs, which caused
problems if some relocations were referring to the duplicated IDs. This
was the cause of the failure of the test D97786.

This patch fixes the code to use the ID of the highest (last) symbol
instead.

Added: 
lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml

Modified: 
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index f30ed427f8535..6e94ab97992a1 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2901,8 +2901,11 @@ void ObjectFileELF::ParseUnwindSymbols(Symtab 
*symbol_table,
   // recalculate the index first.
   std::vector new_symbols;
 
-  eh_frame->ForEachFDEEntries([this, symbol_table, section_list, _symbols](
-  lldb::addr_t file_addr, uint32_t size, dw_offset_t) {
+  size_t num_symbols = symbol_table->GetNumSymbols();
+  uint64_t last_symbol_id =
+  num_symbols ? symbol_table->SymbolAtIndex(num_symbols - 1)->GetID() : 0;
+  eh_frame->ForEachFDEEntries([&](lldb::addr_t file_addr, uint32_t size,
+  dw_offset_t) {
 Symbol *symbol = symbol_table->FindSymbolAtFileAddress(file_addr);
 if (symbol) {
   if (!symbol->GetByteSizeIsValid()) {
@@ -2915,21 +2918,20 @@ void ObjectFileELF::ParseUnwindSymbols(Symtab 
*symbol_table,
   if (section_sp) {
 addr_t offset = file_addr - section_sp->GetFileAddress();
 const char *symbol_name = GetNextSyntheticSymbolName().GetCString();
-uint64_t symbol_id = symbol_table->GetNumSymbols();
 Symbol eh_symbol(
-symbol_id,   // Symbol table index.
-symbol_name, // Symbol name.
-eSymbolTypeCode, // Type of this symbol.
-true,// Is this globally visible?
-false,   // Is this symbol debug info?
-false,   // Is this symbol a trampoline?
-true,// Is this symbol artificial?
-section_sp,  // Section in which this symbol is defined or 
null.
-offset,  // Offset in section or symbol value.
-0, // Size:  Don't specify the size as an FDE can
-false, // Size is valid: cover multiple symbols.
-false, // Contains linker annotations?
-0);// Symbol flags.
+++last_symbol_id, // Symbol table index.
+symbol_name,  // Symbol name.
+eSymbolTypeCode,  // Type of this symbol.
+true, // Is this globally visible?
+false,// Is this symbol debug info?
+false,// Is this symbol a trampoline?
+true, // Is this symbol artificial?
+section_sp, // Section in which this symbol is defined or null.
+offset, // Offset in section or symbol value.
+0,  // Size:  Don't specify the size as an FDE can
+false,  // Size is valid: cover multiple symbols.
+false,  // Contains linker annotations?
+0); // Symbol flags.
 new_symbols.push_back(eh_symbol);
   }
 }

diff  --git a/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml 
b/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
new file mode 100644
index 0..6178a45de1b59
--- /dev/null
+++ b/lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
@@ -0,0 +1,32 @@
+# RUN: yaml2obj %s >%t
+# RUN: %lldb %t -o "image dump symtab" -b | FileCheck %s
+
+# CHECK: Index   UserID DSX TypeFile Address/Value Load Address
   Size   Flags  Name
+# CHECK: [0]  1 SourceFile  0x 
   0x 0x0004 -
+# CHECK: [1]  2  SX Code0x00201180 
   0x0010 0x ___lldb_unnamed_symbol1$${{.*}}
+# CHECK: [2]  3  SX Code0x00201190 
   0x0006 0x ___lldb_unnamed_symbol2$${{.*}}
+
+--- !ELF

[Lldb-commits] [PATCH] D100191: [lldb] [llgs] Support owning and detaching extra processes

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D100191#2692882 , @mgorny wrote:

> In D100191#2689489 , @labath wrote:
>
>> We should also start thinking about tests. I suppose the smallest piece of 
>> functionality that could be usefully tested (with a lldb-server test) is 
>> debugging a process that forks, stopping after the fork, and detaching from 
>> the child. Shall we try making that work first?
>
> How about using a `MockProcess` to unittest server's behavior wrt getting 
> `NewSubprocess()` and `Detach()` calls?

I have mixed feelings about that. On one hand, these kinds of tests really 
enable you to test scenarios (often to do with various borderline and error 
conditions) which would not be testable in other ways. And they can run on any 
platform. However, you're not actually testing the error conditions, and I also 
have to put a bunch of other things on the other tip of the scale:

- this test is not a very good example of how tests should be written -- it 
doesn't test the public interface of the class, and it doesn't even set it up 
in a very realistic way. For example, the class does not have a connection 
object initialized. I'm guessing you cannot check the result of the `Handle_D` 
function (which is directly touched by this patch, and it is at least close to 
the public API), because it would always return an error due to a missing 
connection. All of this makes the test very fragile
- writing this test wouldn't "absolve" us of the need to write a test at a 
higher level, which would test the integration with a real process class. All 
of the checks you do here could be done at the lldb-server test level, and 
within the existing test framework (you couldn't check that `m_current_process` 
is null, but you could for example check that a `g` packet returns an error, 
which should be close enough).

So overall, I think that David was right that this patch is too small to be 
tested independently. Though I didn't say that explicitly, this is what I was 
also getting at with the "minimal usefully testable functionality" remark. So, 
while I hate to waste the effort you put into this test (and I'm very sorry 
that my slow response made you waste it), I don't think it would be wise to 
waste more of it in trying to make the test look good.

Let's identify the set of patches needed to make this testable via the 
lldb-server suite (this one, D100153 , 
D100208  (or equivalent for some other os), 
and what else?) and test that?




Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:226
+NewSubprocess(NativeProcessProtocol *parent_process,
+  std::unique_ptr _process) = 0;
   };

mgorny wrote:
> labath wrote:
> > That way, the delegate _must_ do something with the child process.
> Indeed, it must. Unfortunately, this breaks `MockDelegate`:
> 
> ```
> /home/mgorny/git/llvm-project/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h:
>  In member function ‘virtual testing::internal::Function::Result 
> lldb_private::MockDelegate::NewSubprocess(testing::internal::Function::Argument1,
>  testing::internal::Function std::unique_ptr)>::Argument2)’:
> /home/mgorny/git/llvm-project/llvm/utils/unittest/googlemock/include/gmock/gmock-generated-function-mockers.h:405:73:
>  error: use of deleted function ‘std::unique_ptr<_Tp, _Dp>::unique_ptr(const 
> std::unique_ptr<_Tp, _Dp>&) [with _Tp = lldb_private::NativeProcessProtocol; 
> _Dp = std::default_delete]’
>   405 | return GMOCK_MOCKER_(2, constness, Method).Invoke(gmock_a1, 
> gmock_a2); \
>   |   
>   ^
> /home/mgorny/git/llvm-project/llvm/utils/unittest/googlemock/include/gmock/gmock-generated-function-mockers.h:679:30:
>  note: in expansion of macro ‘GMOCK_METHOD2_’
>   679 | #define MOCK_METHOD2(m, ...) GMOCK_METHOD2_(, , , m, __VA_ARGS__)
>   |  ^~
> /home/mgorny/git/llvm-project/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h:28:3:
>  note: in expansion of macro ‘MOCK_METHOD2’
>28 |   MOCK_METHOD2(NewSubprocess,
>   |   ^~~~
> [...]
> ```
> 
> and then there are a few pages of errors.
Right. I see. In that case, I think we should fix the problem inside 
MockDelegate. The simplest way to do that is to add a forwarding method which 
repackages the arguments in a gmock-friendly way. Something like this ought to 
work:
```
class MockDelegate {
  MOCK_METHOD2(NewSubprocessImpl,
   void(NativeProcessProtocol *parent_process,
std::unique_ptr _proces /*or 
NativeProcessProtocol &, or whatever works best*/));
  void NewSubprocess(NativeProcessProtocol *parent_process,
std::unique_ptr child_proces) { 
NewSubprocessImpl(parent_process, child_process); }

[Lldb-commits] [PATCH] D100931: Set binaries-loaded/unloaded breakpoint on a name, not an address on macOS

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
jasonmolenda requested review of this revision.

I don't know what I was thinking when I wrote this; setting this on an addr_t 
just removes information that lldb could use to avoid unnecessary breakpoint 
setting evaluations when new libraries are loaded.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100931

Files:
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp


Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -344,28 +344,26 @@
 bool DynamicLoaderMacOS::SetNotificationBreakpoint() {
   if (m_break_id == LLDB_INVALID_BREAK_ID) {
 ConstString g_symbol_name("_dyld_debugger_notification");
-const Symbol *symbol = nullptr;
 ModuleSP dyld_sp(GetDYLDModule());
 if (dyld_sp) {
-  symbol = dyld_sp->FindFirstSymbolWithNameAndType(g_symbol_name,
-   eSymbolTypeCode);
-}
-if (symbol &&
-(symbol->ValueIsAddress() || symbol->GetAddressRef().IsValid())) {
-  addr_t symbol_address =
-  
symbol->GetAddressRef().GetOpcodeLoadAddress(_process->GetTarget());
-  if (symbol_address != LLDB_INVALID_ADDRESS) {
-bool internal = true;
-bool hardware = false;
-Breakpoint *breakpoint =
-m_process->GetTarget()
-.CreateBreakpoint(symbol_address, internal, hardware)
-.get();
-breakpoint->SetCallback(DynamicLoaderMacOS::NotifyBreakpointHit, this,
-true);
-breakpoint->SetBreakpointKind("shared-library-event");
-m_break_id = breakpoint->GetID();
-  }
+  bool internal = true;
+  bool hardware = false;
+  LazyBool skip_prologue = eLazyBoolNo;
+  FileSpecList *source_files = nullptr;
+  FileSpecList dyld_filelist;
+  dyld_filelist.Append(dyld_sp->GetObjectFile()->GetFileSpec());
+
+  Breakpoint *breakpoint =
+  m_process->GetTarget()
+  .CreateBreakpoint(_filelist, source_files,
+"_dyld_debugger_notification",
+eFunctionNameTypeFull, eLanguageTypeC, 0,
+skip_prologue, internal, hardware)
+  .get();
+  breakpoint->SetCallback(DynamicLoaderMacOS::NotifyBreakpointHit, this,
+  true);
+  breakpoint->SetBreakpointKind("shared-library-event");
+  m_break_id = breakpoint->GetID();
 }
   }
   return m_break_id != LLDB_INVALID_BREAK_ID;


Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
===
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -344,28 +344,26 @@
 bool DynamicLoaderMacOS::SetNotificationBreakpoint() {
   if (m_break_id == LLDB_INVALID_BREAK_ID) {
 ConstString g_symbol_name("_dyld_debugger_notification");
-const Symbol *symbol = nullptr;
 ModuleSP dyld_sp(GetDYLDModule());
 if (dyld_sp) {
-  symbol = dyld_sp->FindFirstSymbolWithNameAndType(g_symbol_name,
-   eSymbolTypeCode);
-}
-if (symbol &&
-(symbol->ValueIsAddress() || symbol->GetAddressRef().IsValid())) {
-  addr_t symbol_address =
-  symbol->GetAddressRef().GetOpcodeLoadAddress(_process->GetTarget());
-  if (symbol_address != LLDB_INVALID_ADDRESS) {
-bool internal = true;
-bool hardware = false;
-Breakpoint *breakpoint =
-m_process->GetTarget()
-.CreateBreakpoint(symbol_address, internal, hardware)
-.get();
-breakpoint->SetCallback(DynamicLoaderMacOS::NotifyBreakpointHit, this,
-true);
-breakpoint->SetBreakpointKind("shared-library-event");
-m_break_id = breakpoint->GetID();
-  }
+  bool internal = true;
+  bool hardware = false;
+  LazyBool skip_prologue = eLazyBoolNo;
+  FileSpecList *source_files = nullptr;
+  FileSpecList dyld_filelist;
+  dyld_filelist.Append(dyld_sp->GetObjectFile()->GetFileSpec());
+
+  Breakpoint *breakpoint =
+  m_process->GetTarget()
+  .CreateBreakpoint(_filelist, source_files,
+"_dyld_debugger_notification",
+eFunctionNameTypeFull, eLanguageTypeC, 0,
+skip_prologue, internal, hardware)
+  .get();
+  

[Lldb-commits] [PATCH] D100898: [CMake][lldb] add_lldb_library's functions require default visibility

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Is there a way to address this through the preprocessor? We already have the 
`LLDB_API` macro, which expands to `__declspec(dllexport/import)` on windows. 
Would expanding it do `__attribute__((visibility("default")))` (or something) 
on other platforms help with anything?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100898

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