[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py:43
+if region.GetRegionEnd() > illegal_address:
+illegal_address = region.GetRegionEnd()
 

I'm confused as to what the logic is here now.

You:
* read from address 0x0 to fill in error
* walk the memory regions until the highest one, making the end of that the 
illegal address
* assume that the error value from reading 0x0 is the same as you'd get from 
this new illegal address
...

Or am I missing something and the ptr was just left in accidentally.

(I would suggest you could jump to the last region in the list straight away 
but I don't think we actually require them to be sorted, plus sometimes you get 
multiple regions with the same base)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126109

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

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py:43
+if region.GetRegionEnd() > illegal_address:
+illegal_address = region.GetRegionEnd()
 

DavidSpickett wrote:
> I'm confused as to what the logic is here now.
> 
> You:
> * read from address 0x0 to fill in error
> * walk the memory regions until the highest one, making the end of that the 
> illegal address
> * assume that the error value from reading 0x0 is the same as you'd get from 
> this new illegal address
> ...
> 
> Or am I missing something and the ptr was just left in accidentally.
> 
> (I would suggest you could jump to the last region in the list straight away 
> but I don't think we actually require them to be sorted, plus sometimes you 
> get multiple regions with the same base)
> 
(same base but different end addresses I mean)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126109

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

[Lldb-commits] [PATCH] D126217: [lldb] Improve TestAppleSimulatorOSType.py error message

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: augusto2112, jingham.
Herald added a project: All.
JDevlieghere requested review of this revision.

Show a more informative error message when we fail to parse the simctl output. 
It was inspired by https://reviews.llvm.org/D109336 which got reverted because 
we didn't want to silently fail.


https://reviews.llvm.org/D126217

Files:
  lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py


Index: lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
===
--- lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
+++ lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
@@ -17,9 +17,13 @@
 
 def check_simulator_ostype(self, sdk, platform_name, 
arch=platform.machine()):
 cmd = ['xcrun', 'simctl', 'list', '-j', 'devices']
-self.trace(' '.join(cmd))
+cmd_str = ' '.join(cmd)
+self.trace(cmd_str)
 sim_devices_str = subprocess.check_output(cmd).decode("utf-8")
-sim_devices = json.loads(sim_devices_str)['devices']
+try:
+sim_devices = json.loads(sim_devices_str)['devices']
+except json.decoder.JSONDecodeError:
+self.fail("Could not parse '{}' output. Authorization 
denied?".format(cmd_str))
 # Find an available simulator for the requested platform
 deviceUDID = None
 deviceRuntime = None


Index: lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
===
--- lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
+++ lldb/test/API/tools/lldb-server/TestAppleSimulatorOSType.py
@@ -17,9 +17,13 @@
 
 def check_simulator_ostype(self, sdk, platform_name, arch=platform.machine()):
 cmd = ['xcrun', 'simctl', 'list', '-j', 'devices']
-self.trace(' '.join(cmd))
+cmd_str = ' '.join(cmd)
+self.trace(cmd_str)
 sim_devices_str = subprocess.check_output(cmd).decode("utf-8")
-sim_devices = json.loads(sim_devices_str)['devices']
+try:
+sim_devices = json.loads(sim_devices_str)['devices']
+except json.decoder.JSONDecodeError:
+self.fail("Could not parse '{}' output. Authorization denied?".format(cmd_str))
 # Find an available simulator for the requested platform
 deviceUDID = None
 deviceRuntime = None
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c30a8c8 - [lldb] Fix should_skip_simulator_test decorator

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-05-23T09:33:51-07:00
New Revision: c30a8c80837608ccb190aecb84a723ca00dd87df

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

LOG: [lldb] Fix should_skip_simulator_test decorator

Currently simulator tests get skipped when the reported platform is
macosx rather than darwin. Update the decorator to match both.

Added: 


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

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 8f636024abe91..467530fdfd7e7 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -378,8 +378,8 @@ def apple_simulator_test(platform):
 The SDK identifiers for simulators are iphonesimulator, appletvsimulator, 
watchsimulator
 """
 def should_skip_simulator_test():
-if lldbplatformutil.getHostPlatform() != 'darwin':
-return "simulator tests are run only on darwin hosts"
+if lldbplatformutil.getHostPlatform() not in ['darwin', 'macosx']:
+return "simulator tests are run only on darwin hosts."
 try:
 DEVNULL = open(os.devnull, 'w')
 output = subprocess.check_output(["xcodebuild", "-showsdks"], 
stderr=DEVNULL).decode("utf-8")



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

[Lldb-commits] [lldb] 46c1f77 - Add [opt] suffix to optimized stack frame in lldb-vscode

[Jeffrey Tan via lldb-commits]

Author: Jeffrey Tan
Date: 2022-05-23T10:03:13-07:00
New Revision: 46c1f77e160a63726e312c0550157ee451bacd46

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

LOG: Add [opt] suffix to optimized stack frame in lldb-vscode

To help user identify optimized code This diff adds a "[opt]" suffix to
optimized stack frames in lldb-vscode. This provides consistent experience
as command line lldb.

It also adds a new "optimized" attribute to DAP stack frame object so that
it is easy to identify from telemetry than parsing trailing "[opt]".

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

Added: 
lldb/test/API/tools/lldb-vscode/optimized/Makefile
lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
lldb/test/API/tools/lldb-vscode/optimized/main.cpp

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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/optimized/Makefile 
b/lldb/test/API/tools/lldb-vscode/optimized/Makefile
new file mode 100644
index ..685b2606b55a
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/optimized/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -O3 -glldb
+include Makefile.rules

diff  --git a/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py 
b/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
new file mode 100644
index ..862fe632a51b
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -0,0 +1,35 @@
+"""
+Test lldb-vscode variables/stackTrace request for optimized code
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_optimized(lldbvscode_testcase.VSCodeTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+def test_stack_frame_name(self):
+''' Test optimized frame has special name suffix.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 1')
+lines = [breakpoint_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)
+leaf_frame = self.vscode.get_stackFrame(frameIndex=0)
+self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
+parent_frame = self.vscode.get_stackFrame(frameIndex=1)
+self.assertTrue(parent_frame['name'].endswith(' [opt]'))

diff  --git a/lldb/test/API/tools/lldb-vscode/optimized/main.cpp 
b/lldb/test/API/tools/lldb-vscode/optimized/main.cpp
new file mode 100644
index ..24ace4e4196f
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/optimized/main.cpp
@@ -0,0 +1,16 @@
+#include 
+#include 
+
+int foo(int x, int y) {
+  printf("Got input %d, %d\n", x, y);
+  return x + y + 3; // breakpoint 1
+}
+
+int main(int argc, char const *argv[]) {
+  int optimized = argc > 1 ? std::stoi(argv[1]) : 0;
+
+  printf("argc: %d, optimized: %d\n", argc, optimized);
+  int result = foo(argc, 20);
+  printf("result: %d\n", result); // breakpoint 2
+  return 0;
+}

diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 55d734e65fb5..cd8d8861d98a 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -751,7 +751,19 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
   llvm::json::Object object;
   int64_t frame_id = MakeVSCodeFrameID(frame);
   object.try_emplace("id", frame_id);
-  EmplaceSafeString(object, "name", frame.GetFunctionName());
+
+  std::string frame_name;
+  const char *func_name = frame.GetFunctionName();
+  if (func_name)
+frame_name = func_name;
+  else
+frame_name = "";
+  bool is_optimized = frame.GetFunction().GetIsOptimized();
+  if (is_optimized)
+frame_name += " [opt]";
+  EmplaceSafeString(object, "name", frame_name);
+  object.try_emplace("optimized", is_optimized);
+
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));
 



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

[Lldb-commits] [PATCH] D126013: Add [opt] suffix to optimized stack frame in lldb-vscode

[jeffrey tan via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG46c1f77e160a: Add [opt] suffix to optimized stack frame in 
lldb-vscode (authored by yinghuitan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126013

Files:
  lldb/test/API/tools/lldb-vscode/optimized/Makefile
  lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
  lldb/test/API/tools/lldb-vscode/optimized/main.cpp
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -751,7 +751,19 @@
   llvm::json::Object object;
   int64_t frame_id = MakeVSCodeFrameID(frame);
   object.try_emplace("id", frame_id);
-  EmplaceSafeString(object, "name", frame.GetFunctionName());
+
+  std::string frame_name;
+  const char *func_name = frame.GetFunctionName();
+  if (func_name)
+frame_name = func_name;
+  else
+frame_name = "";
+  bool is_optimized = frame.GetFunction().GetIsOptimized();
+  if (is_optimized)
+frame_name += " [opt]";
+  EmplaceSafeString(object, "name", frame_name);
+  object.try_emplace("optimized", is_optimized);
+
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));
 
Index: lldb/test/API/tools/lldb-vscode/optimized/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/main.cpp
@@ -0,0 +1,16 @@
+#include 
+#include 
+
+int foo(int x, int y) {
+  printf("Got input %d, %d\n", x, y);
+  return x + y + 3; // breakpoint 1
+}
+
+int main(int argc, char const *argv[]) {
+  int optimized = argc > 1 ? std::stoi(argv[1]) : 0;
+
+  printf("argc: %d, optimized: %d\n", argc, optimized);
+  int result = foo(argc, 20);
+  printf("result: %d\n", result); // breakpoint 2
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -0,0 +1,35 @@
+"""
+Test lldb-vscode variables/stackTrace request for optimized code
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_optimized(lldbvscode_testcase.VSCodeTestCaseBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+def test_stack_frame_name(self):
+''' Test optimized frame has special name suffix.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 1')
+lines = [breakpoint_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)
+leaf_frame = self.vscode.get_stackFrame(frameIndex=0)
+self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
+parent_frame = self.vscode.get_stackFrame(frameIndex=1)
+self.assertTrue(parent_frame['name'].endswith(' [opt]'))
Index: lldb/test/API/tools/lldb-vscode/optimized/Makefile
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -O3 -glldb
+include Makefile.rules


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -751,7 +751,19 @@
   llvm::json::Object object;
   int64_t frame_id = MakeVSCodeFrameID(frame);
   object.try_emplace("id", frame_id);
-  EmplaceSafeString(object, "name", frame.GetFunctionName());
+
+  std::string frame_name;
+  const char *func_name = frame.GetFunctionName();
+  if (func_name)
+frame_name = func_name;
+  else
+frame_name = "";
+  bool is_optimized = frame.GetFunction().GetIsOptimized();
+  if (is_optimized)
+frame_name += " [opt]";
+  EmplaceSafeString(object, "name", frame_name);
+  object.try_emplace("optimized", is_optimized);
+
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));
 
Index: lldb/test/API/tools/lldb-vscode/optimized/main.cpp
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/optimized/main.cpp
@@ -0,0 +1,16 @@
+#include 
+#include 
+
+int foo(int x, int y) {
+  printf("Got input %d, %d\n", x, y

[Lldb-commits] [lldb] 0fe220a - Show error message for optimized variables

[Jeffrey Tan via lldb-commits]

Author: Jeffrey Tan
Date: 2022-05-23T10:04:58-07:00
New Revision: 0fe220a33179723b7b366c2b398bac3c3b4216ec

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

LOG: Show error message for optimized variables

This fixes an issue that optimized variable error message is not shown to end
users in lldb-vscode.

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

Added: 


Modified: 
lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
lldb/tools/lldb-vscode/JSONUtils.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py 
b/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
index 862fe632a51b..a5e40d03abe4 100644
--- a/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
+++ b/lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -33,3 +33,22 @@ def test_stack_frame_name(self):
 self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
 parent_frame = self.vscode.get_stackFrame(frameIndex=1)
 self.assertTrue(parent_frame['name'].endswith(' [opt]'))
+
+@skipIfWindows
+@skipIfRemote
+def test_optimized_variable(self):
+''' Test optimized variable value contains error.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 2')
+lines = [breakpoint_line]
+# Set breakpoint in the thread function so we can step the threads
+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)
+optimized_variable = self.vscode.get_local_variable('optimized')
+
+self.assertTrue(optimized_variable['value'].startswith('";
+  } else if (!value.empty()) {
 strm << value;
 if (!summary.empty())
   strm << ' ' << summary;



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

[Lldb-commits] [PATCH] D126014: Show error message for optimized variables

[jeffrey tan 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 rG0fe220a33179: Show error message for optimized variables 
(authored by yinghuitan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126014

Files:
  lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -136,10 +136,13 @@
   llvm::StringRef value = v.GetValue();
   llvm::StringRef summary = v.GetSummary();
   llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+  lldb::SBError error = v.GetError();
 
   std::string result;
   llvm::raw_string_ostream strm(result);
-  if (!value.empty()) {
+  if (!error.Success()) {
+strm << "";
+  } else if (!value.empty()) {
 strm << value;
 if (!summary.empty())
   strm << ' ' << summary;
Index: lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
===
--- lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
+++ lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -33,3 +33,22 @@
 self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
 parent_frame = self.vscode.get_stackFrame(frameIndex=1)
 self.assertTrue(parent_frame['name'].endswith(' [opt]'))
+
+@skipIfWindows
+@skipIfRemote
+def test_optimized_variable(self):
+''' Test optimized variable value contains error.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 2')
+lines = [breakpoint_line]
+# Set breakpoint in the thread function so we can step the threads
+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)
+optimized_variable = self.vscode.get_local_variable('optimized')
+
+self.assertTrue(optimized_variable['value'].startswith('Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -136,10 +136,13 @@
   llvm::StringRef value = v.GetValue();
   llvm::StringRef summary = v.GetSummary();
   llvm::StringRef type_name = v.GetType().GetDisplayTypeName();
+  lldb::SBError error = v.GetError();
 
   std::string result;
   llvm::raw_string_ostream strm(result);
-  if (!value.empty()) {
+  if (!error.Success()) {
+strm << "";
+  } else if (!value.empty()) {
 strm << value;
 if (!summary.empty())
   strm << ' ' << summary;
Index: lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
===
--- lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
+++ lldb/test/API/tools/lldb-vscode/optimized/TestVSCode_optimized.py
@@ -33,3 +33,22 @@
 self.assertTrue(leaf_frame['name'].endswith(' [opt]'))
 parent_frame = self.vscode.get_stackFrame(frameIndex=1)
 self.assertTrue(parent_frame['name'].endswith(' [opt]'))
+
+@skipIfWindows
+@skipIfRemote
+def test_optimized_variable(self):
+''' Test optimized variable value contains error.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.cpp'
+breakpoint_line = line_number(source, '// breakpoint 2')
+lines = [breakpoint_line]
+# Set breakpoint in the thread function so we can step the threads
+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)
+optimized_variable = self.vscode.get_local_variable('optimized')
+
+self.assertTrue(optimized_variable['value'].startswith('___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126225: Fix lldb-vscode frame test failure

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan created this revision.
yinghuitan added reviewers: clayborg, labath, jingham, kusmour, aadsm.
Herald added a project: All.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previous patch (https://reviews.llvm.org/D126013) added a new "optimized"
attribute to DAP stack frame this caused some tests, like
lldb-vscode/coreFile/TestVSCode_coreFile.py
to fail because the tests explicitly check for all attributes.

To fix the test failure I decided to remove this attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126225

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -765,7 +765,6 @@
   if (is_optimized)
 frame_name += " [opt]";
   EmplaceSafeString(object, "name", frame_name);
-  object.try_emplace("optimized", is_optimized);
 
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -765,7 +765,6 @@
   if (is_optimized)
 frame_name += " [opt]";
   EmplaceSafeString(object, "name", frame_name);
-  object.try_emplace("optimized", is_optimized);
 
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py:43
+if region.GetRegionEnd() > illegal_address:
+illegal_address = region.GetRegionEnd()
 

DavidSpickett wrote:
> DavidSpickett wrote:
> > I'm confused as to what the logic is here now.
> > 
> > You:
> > * read from address 0x0 to fill in error
> > * walk the memory regions until the highest one, making the end of that the 
> > illegal address
> > * assume that the error value from reading 0x0 is the same as you'd get 
> > from this new illegal address
> > ...
> > 
> > Or am I missing something and the ptr was just left in accidentally.
> > 
> > (I would suggest you could jump to the last region in the list straight 
> > away but I don't think we actually require them to be sorted, plus 
> > sometimes you get multiple regions with the same base)
> > 
> (same base but different end addresses I mean)
Hello! Thank you for the review!

I was initializing `illegal_address` to 0 as a way to, well, initialize the 
value. I suppose that I could have initialized it to `None` and then used that 
a special value in the loop. In fact, now that I think about it, that's the 
tact that I will take in a second version of this patch. 

But, yes, you basically have the rest correct: I walk through all the memory 
regions and find the highest address. Then, when I have that address (which I 
know is not in a valid memory region), I will use that as the address to set 
for the illegal breakpoint.

Does that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126109

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

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py:43
+if region.GetRegionEnd() > illegal_address:
+illegal_address = region.GetRegionEnd()
 

hawkinsw wrote:
> DavidSpickett wrote:
> > DavidSpickett wrote:
> > > I'm confused as to what the logic is here now.
> > > 
> > > You:
> > > * read from address 0x0 to fill in error
> > > * walk the memory regions until the highest one, making the end of that 
> > > the illegal address
> > > * assume that the error value from reading 0x0 is the same as you'd get 
> > > from this new illegal address
> > > ...
> > > 
> > > Or am I missing something and the ptr was just left in accidentally.
> > > 
> > > (I would suggest you could jump to the last region in the list straight 
> > > away but I don't think we actually require them to be sorted, plus 
> > > sometimes you get multiple regions with the same base)
> > > 
> > (same base but different end addresses I mean)
> Hello! Thank you for the review!
> 
> I was initializing `illegal_address` to 0 as a way to, well, initialize the 
> value. I suppose that I could have initialized it to `None` and then used 
> that a special value in the loop. In fact, now that I think about it, that's 
> the tact that I will take in a second version of this patch. 
> 
> But, yes, you basically have the rest correct: I walk through all the memory 
> regions and find the highest address. Then, when I have that address (which I 
> know is not in a valid memory region), I will use that as the address to set 
> for the illegal breakpoint.
> 
> Does that make sense?
And, woah, I now realize what you meant! So sorry!

Yes, `ptr` was leftover. I am so sorry!

Standby for the next version of this patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126109

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

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw updated this revision to Diff 431427.
hawkinsw added a comment.

Updating based on DavidSpickett helpful feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126109

Files:
  
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py


Index: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
===
--- 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
+++ 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
@@ -27,14 +27,26 @@
   "Set a breakpoint here",
   lldb.SBFileSpec("main.c"))
 
-# Now see if we can read from 0.  If I can't do that, I don't
-# have a good way to know what an illegal address is...
-error = lldb.SBError()
 
-ptr = process.ReadPointerFromMemory(0x0, error)
 
-if not error.Success():
-bkpt = target.BreakpointCreateByAddress(0x0)
+# illegal_address will hold (optionally) an address that, if
+# used as a breakpoint, will generate an unresolved breakpoint.
+illegal_address = None
+
+# Walk through all the memory regions in the process and
+# find an address that is invalid.
+regions = process.GetMemoryRegions()
+for region_idx in range(regions.GetSize()):
+region = lldb.SBMemoryRegionInfo()
+regions.GetMemoryRegionAtIndex(region_idx, region)
+if illegal_address == None or \
+region.GetRegionEnd() > illegal_address:
+illegal_address = region.GetRegionEnd()
+
+if illegal_address != None:
+# Now, set a breakpoint at the address we know is illegal.
+bkpt = target.BreakpointCreateByAddress(illegal_address)
+# Verify that breakpoint is not resolved.
 for bp_loc in bkpt:
 self.assertEquals(bp_loc.IsResolved(), False)
 else:


Index: lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
===
--- lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
+++ lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py
@@ -27,14 +27,26 @@
   "Set a breakpoint here",
   lldb.SBFileSpec("main.c"))
 
-# Now see if we can read from 0.  If I can't do that, I don't
-# have a good way to know what an illegal address is...
-error = lldb.SBError()
 
-ptr = process.ReadPointerFromMemory(0x0, error)
 
-if not error.Success():
-bkpt = target.BreakpointCreateByAddress(0x0)
+# illegal_address will hold (optionally) an address that, if
+# used as a breakpoint, will generate an unresolved breakpoint.
+illegal_address = None
+
+# Walk through all the memory regions in the process and
+# find an address that is invalid.
+regions = process.GetMemoryRegions()
+for region_idx in range(regions.GetSize()):
+region = lldb.SBMemoryRegionInfo()
+regions.GetMemoryRegionAtIndex(region_idx, region)
+if illegal_address == None or \
+region.GetRegionEnd() > illegal_address:
+illegal_address = region.GetRegionEnd()
+
+if illegal_address != None:
+# Now, set a breakpoint at the address we know is illegal.
+bkpt = target.BreakpointCreateByAddress(illegal_address)
+# Verify that breakpoint is not resolved.
 for bp_loc in bkpt:
 self.assertEquals(bp_loc.IsResolved(), False)
 else:
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

[Will Hawkins via Phabricator via lldb-commits]

hawkinsw added a comment.

@DavidSpickett Thank you for the review! I hope that this 2nd version of the 
patch addresses your helpful comments!

Will


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126109

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

[Lldb-commits] [lldb] 0da230f - [lldb] Improve formatting of dlopen error messages (NFC)

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2022-05-23T10:57:40-07:00
New Revision: 0da230ff44399f74dc9bf05c8f212e7fc22079fa

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

LOG: [lldb] Improve formatting of dlopen error messages (NFC)

Ensure there's a space between "utility" and "function", and also makes
it easier to grep/search for "utility function".

While making this change, I also re-formatted the other dlopen error messages
(with clang-format). This fix other instances of spaces missing between words,
and makes each of these strings fit a single line, making them greppable.

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

Added: 


Modified: 
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp 
b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 9a2f0972f9993..fb0d39ea6f866 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -613,9 +613,9 @@ 
PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
   std::move(expr), dlopen_wrapper_name, eLanguageTypeC_plus_plus, exe_ctx);
   if (!utility_fn_or_error) {
 std::string error_str = llvm::toString(utility_fn_or_error.takeError());
-error.SetErrorStringWithFormat("dlopen error: could not create utility"
-   "function: %s",
-   error_str.c_str());
+error.SetErrorStringWithFormat(
+"dlopen error: could not create utility function: %s",
+error_str.c_str());
 return nullptr;
   }
   std::unique_ptr dlopen_utility_func_up =
@@ -650,8 +650,9 @@ 
PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
   do_dlopen_function = dlopen_utility_func_up->MakeFunctionCaller(
   clang_void_pointer_type, arguments, exe_ctx.GetThreadSP(), 
utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not make function"
-   "caller: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not make function caller: %s",
+utility_error.AsCString());
 return nullptr;
   }
   
@@ -717,8 +718,9 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process 
*process,
permissions,
utility_error);
   if (path_addr == LLDB_INVALID_ADDRESS) {
-error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-"for path: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not allocate memory for path: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
 
@@ -730,8 +732,9 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process 
*process,
 
   process->WriteMemory(path_addr, path.c_str(), path_len, utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not write path string:"
-" %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not write path string: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
   
@@ -742,8 +745,9 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process 
*process,
   permissions,
   utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-"for path: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not allocate memory for path: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
   
@@ -791,9 +795,9 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process 
*process,
   permissions,
   utility_error);
 if (path_array_addr == LLDB_INVALID_ADDRESS) {
-  error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-  "for path array: %s", 
-  utility_error.AsCString());
+  error.SetErrorStringWithFormat(
+  "dlopen error: could not allocate memory for path array: %s",
+  utility_error.AsCString());
   return LLDB_INVALID_IMAGE_TOKEN;
 }
 
@@ -807,8 +811,9 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process 
*process

[Lldb-commits] [PATCH] D126078: [lldb] Improve formatting of dlopen error messages (NFC)

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0da230ff4439: [lldb] Improve formatting of dlopen error 
messages (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126078

Files:
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -613,9 +613,9 @@
   std::move(expr), dlopen_wrapper_name, eLanguageTypeC_plus_plus, exe_ctx);
   if (!utility_fn_or_error) {
 std::string error_str = llvm::toString(utility_fn_or_error.takeError());
-error.SetErrorStringWithFormat("dlopen error: could not create utility"
-   "function: %s",
-   error_str.c_str());
+error.SetErrorStringWithFormat(
+"dlopen error: could not create utility function: %s",
+error_str.c_str());
 return nullptr;
   }
   std::unique_ptr dlopen_utility_func_up =
@@ -650,8 +650,9 @@
   do_dlopen_function = dlopen_utility_func_up->MakeFunctionCaller(
   clang_void_pointer_type, arguments, exe_ctx.GetThreadSP(), utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not make function"
-   "caller: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not make function caller: %s",
+utility_error.AsCString());
 return nullptr;
   }
   
@@ -717,8 +718,9 @@
permissions,
utility_error);
   if (path_addr == LLDB_INVALID_ADDRESS) {
-error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-"for path: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not allocate memory for path: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
 
@@ -730,8 +732,9 @@
 
   process->WriteMemory(path_addr, path.c_str(), path_len, utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not write path string:"
-" %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not write path string: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
   
@@ -742,8 +745,9 @@
   permissions,
   utility_error);
   if (utility_error.Fail()) {
-error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-"for path: %s", utility_error.AsCString());
+error.SetErrorStringWithFormat(
+"dlopen error: could not allocate memory for path: %s",
+utility_error.AsCString());
 return LLDB_INVALID_IMAGE_TOKEN;
   }
   
@@ -791,9 +795,9 @@
   permissions,
   utility_error);
 if (path_array_addr == LLDB_INVALID_ADDRESS) {
-  error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-  "for path array: %s", 
-  utility_error.AsCString());
+  error.SetErrorStringWithFormat(
+  "dlopen error: could not allocate memory for path array: %s",
+  utility_error.AsCString());
   return LLDB_INVALID_IMAGE_TOKEN;
 }
 
@@ -807,8 +811,9 @@
  path_array.size(), utility_error);
 
 if (utility_error.Fail()) {
-  error.SetErrorStringWithFormat("dlopen error: could not write path array:"
- " %s", utility_error.AsCString());
+  error.SetErrorStringWithFormat(
+  "dlopen error: could not write path array: %s",
+  utility_error.AsCString());
   return LLDB_INVALID_IMAGE_TOKEN;
 }
 // Now make spaces in the target for the buffer.  We need to add one for
@@ -819,9 +824,9 @@
   permissions,
   utility_error);
 if (buffer_addr == LLDB_INVALID_ADDRESS) {
-  error.SetErrorStringWithFormat("dlopen error: could not allocate memory"
-  "for buffer: %s", 
-  utility_error.AsCString());
+  error.SetErrorStringWithFormat(
+  "dlopen error: could not allocate memory for buffer: %s",
+  utility_error.AsCString());
   return LLDB_INVALID_IMAGE_TO

[Lldb-commits] [lldb] 760298a - [lldb] Specify aguments of `image list`

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2022-05-23T11:00:48-07:00
New Revision: 760298adc264f9c1029d93ab38711c131e19a2f4

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

LOG: [lldb] Specify aguments of `image list`

Register positional argument details in `CommandObjectTargetModulesList`.

I recently learned that `image list` takes a module name, but the help info
does not indicate this. With this change, `help image list` will show that it
accepts zero or more module names.

This makes it easier to get info about specific modules, without having to
find/grep through the full image list.

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp
lldb/test/API/commands/help/TestHelp.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index f42588b673a46..7789698c1d678 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -50,6 +50,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Timer.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
 
 #include "llvm/ADT/ScopeExit.h"
@@ -2901,8 +2902,10 @@ class CommandObjectTargetModulesList : public 
CommandObjectParsed {
   CommandObjectTargetModulesList(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "target modules list",
-"List current executable and dependent shared library images.",
-"target modules list []") {}
+"List current executable and dependent shared library images.") {
+CommandArgumentData module_arg{eArgTypeShlibName, eArgRepeatStar};
+m_arguments.push_back({module_arg});
+  }
 
   ~CommandObjectTargetModulesList() override = default;
 

diff  --git a/lldb/test/API/commands/help/TestHelp.py 
b/lldb/test/API/commands/help/TestHelp.py
index e3363e870e5db..09e7a32a854ca 100644
--- a/lldb/test/API/commands/help/TestHelp.py
+++ b/lldb/test/API/commands/help/TestHelp.py
@@ -104,6 +104,13 @@ def test_help_image_du_line_should_work(self):
 self.expect("help image du line", substrs=[
 'Dump the line table for one or more compilation units'])
 
+@no_debug_info_test
+def test_help_image_list_shows_positional_args(self):
+"""Command 'help image list' should describe positional args."""
+# 'image' is an alias for 'target modules'.
+self.expect("help image list", substrs=[
+' [...]'])
+
 @no_debug_info_test
 def test_help_target_variable_syntax(self):
 """Command 'help target variable' should display  ..."""



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

[Lldb-commits] [PATCH] D125154: [lldb] Specify aguments of `image list`

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG760298adc264: [lldb] Specify aguments of `image list` 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125154

Files:
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/test/API/commands/help/TestHelp.py


Index: lldb/test/API/commands/help/TestHelp.py
===
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -104,6 +104,13 @@
 self.expect("help image du line", substrs=[
 'Dump the line table for one or more compilation units'])
 
+@no_debug_info_test
+def test_help_image_list_shows_positional_args(self):
+"""Command 'help image list' should describe positional args."""
+# 'image' is an alias for 'target modules'.
+self.expect("help image list", substrs=[
+' [...]'])
+
 @no_debug_info_test
 def test_help_target_variable_syntax(self):
 """Command 'help target variable' should display  ..."""
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -50,6 +50,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Timer.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
 
 #include "llvm/ADT/ScopeExit.h"
@@ -2901,8 +2902,10 @@
   CommandObjectTargetModulesList(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "target modules list",
-"List current executable and dependent shared library images.",
-"target modules list []") {}
+"List current executable and dependent shared library images.") {
+CommandArgumentData module_arg{eArgTypeShlibName, eArgRepeatStar};
+m_arguments.push_back({module_arg});
+  }
 
   ~CommandObjectTargetModulesList() override = default;
 


Index: lldb/test/API/commands/help/TestHelp.py
===
--- lldb/test/API/commands/help/TestHelp.py
+++ lldb/test/API/commands/help/TestHelp.py
@@ -104,6 +104,13 @@
 self.expect("help image du line", substrs=[
 'Dump the line table for one or more compilation units'])
 
+@no_debug_info_test
+def test_help_image_list_shows_positional_args(self):
+"""Command 'help image list' should describe positional args."""
+# 'image' is an alias for 'target modules'.
+self.expect("help image list", substrs=[
+' [...]'])
+
 @no_debug_info_test
 def test_help_target_variable_syntax(self):
 """Command 'help target variable' should display  ..."""
Index: lldb/source/Commands/CommandObjectTarget.cpp
===
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -50,6 +50,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/Timer.h"
+#include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-private-enumerations.h"
 
 #include "llvm/ADT/ScopeExit.h"
@@ -2901,8 +2902,10 @@
   CommandObjectTargetModulesList(CommandInterpreter &interpreter)
   : CommandObjectParsed(
 interpreter, "target modules list",
-"List current executable and dependent shared library images.",
-"target modules list []") {}
+"List current executable and dependent shared library images.") {
+CommandArgumentData module_arg{eArgTypeShlibName, eArgRepeatStar};
+m_arguments.push_back({module_arg});
+  }
 
   ~CommandObjectTargetModulesList() override = default;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] f0a61c2 - Remove `friend` classes from TypeCategoryMap

[Jorge Gorbe Moya via lldb-commits]

Author: Jorge Gorbe Moya
Date: 2022-05-23T11:31:53-07:00
New Revision: f0a61c2ce2afec200967c59d45181c7a9fbe2c3f

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

LOG: Remove `friend` classes from TypeCategoryMap

As far as I can tell, the only thing those friend classes access is the
`ValueSP` typedef.

Given that this is a map-ish class, with "Map" in its name, it doesn't
seem like a stretch to make `KeyType`, `ValueType` and `ValueSP` public.
More so when the public methods of the class have `KeyType` and
`ValueSP` arguments and clearly `ValueSP` needs to be accessed from the
outside.

`friend` complicates local reasoning about who can access private
members, which is valuable in a class like this that has every method
locking a mutex to prevent concurrent access.

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/TypeCategoryMap.h

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/TypeCategoryMap.h 
b/lldb/include/lldb/DataFormatters/TypeCategoryMap.h
index 4dbca29db066d..45dbb306aa758 100644
--- a/lldb/include/lldb/DataFormatters/TypeCategoryMap.h
+++ b/lldb/include/lldb/DataFormatters/TypeCategoryMap.h
@@ -23,13 +23,13 @@
 namespace lldb_private {
 class TypeCategoryMap {
 private:
-  typedef ConstString KeyType;
-  typedef TypeCategoryImpl ValueType;
-  typedef ValueType::SharedPointer ValueSP;
   typedef std::list ActiveCategoriesList;
   typedef ActiveCategoriesList::iterator ActiveCategoriesIterator;
 
 public:
+  typedef ConstString KeyType;
+  typedef TypeCategoryImpl ValueType;
+  typedef ValueType::SharedPointer ValueSP;
   typedef std::map MapType;
   typedef MapType::iterator MapIterator;
   typedef std::function ForEachCallback;
@@ -103,9 +103,6 @@ class TypeCategoryMap {
   ActiveCategoriesList &active_list() { return m_active_categories; }
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
-
-  friend class FormattersContainer;
-  friend class FormatManager;
 };
 } // namespace lldb_private
 



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

[Lldb-commits] [PATCH] D126103: Remove `friend` classes from TypeCategoryMap

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf0a61c2ce2af: Remove `friend` classes from TypeCategoryMap 
(authored by jgorbe).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126103

Files:
  lldb/include/lldb/DataFormatters/TypeCategoryMap.h


Index: lldb/include/lldb/DataFormatters/TypeCategoryMap.h
===
--- lldb/include/lldb/DataFormatters/TypeCategoryMap.h
+++ lldb/include/lldb/DataFormatters/TypeCategoryMap.h
@@ -23,13 +23,13 @@
 namespace lldb_private {
 class TypeCategoryMap {
 private:
-  typedef ConstString KeyType;
-  typedef TypeCategoryImpl ValueType;
-  typedef ValueType::SharedPointer ValueSP;
   typedef std::list ActiveCategoriesList;
   typedef ActiveCategoriesList::iterator ActiveCategoriesIterator;
 
 public:
+  typedef ConstString KeyType;
+  typedef TypeCategoryImpl ValueType;
+  typedef ValueType::SharedPointer ValueSP;
   typedef std::map MapType;
   typedef MapType::iterator MapIterator;
   typedef std::function ForEachCallback;
@@ -103,9 +103,6 @@
   ActiveCategoriesList &active_list() { return m_active_categories; }
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
-
-  friend class FormattersContainer;
-  friend class FormatManager;
 };
 } // namespace lldb_private
 


Index: lldb/include/lldb/DataFormatters/TypeCategoryMap.h
===
--- lldb/include/lldb/DataFormatters/TypeCategoryMap.h
+++ lldb/include/lldb/DataFormatters/TypeCategoryMap.h
@@ -23,13 +23,13 @@
 namespace lldb_private {
 class TypeCategoryMap {
 private:
-  typedef ConstString KeyType;
-  typedef TypeCategoryImpl ValueType;
-  typedef ValueType::SharedPointer ValueSP;
   typedef std::list ActiveCategoriesList;
   typedef ActiveCategoriesList::iterator ActiveCategoriesIterator;
 
 public:
+  typedef ConstString KeyType;
+  typedef TypeCategoryImpl ValueType;
+  typedef ValueType::SharedPointer ValueSP;
   typedef std::map MapType;
   typedef MapType::iterator MapIterator;
   typedef std::function ForEachCallback;
@@ -103,9 +103,6 @@
   ActiveCategoriesList &active_list() { return m_active_categories; }
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
-
-  friend class FormattersContainer;
-  friend class FormatManager;
 };
 } // namespace lldb_private
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126225: Fix lldb-vscode frame test failure

[Greg Clayton via Phabricator via lldb-commits]

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

Since this "optimized" value isn't part of the actual JSON schema for the stack 
frames, and since we aren't using it in the IDE, best to remove this until it 
is either in the protocol definition or used by the IDE.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126225

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

[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added reviewers: labath, JDevlieghere.
Herald added a subscriber: pengfei.
Herald added a project: All.
jgorbe requested review of this revision.
Herald added a project: LLDB.

I've noticed (what looked like) deadlocking behavior while running some
Python code that registers categories and formatters from a separate
thread. I ran lldb and my scripts through Helgrind and got (among other
unrelated reports) the report below.

In summary, there are two mutexes (`TypeCategoryMap::m_map_mutex` and
`FormatManager::m_language_categories_mutex`) that sometimes get
acquired in different order. This change attempts to fix that by
reducing the scope of the locks that appear in the Helgrind report.

- `TypeCategoryMap::Add` locks `m_map_mutex` and keeps holding it while 
notifying `listener`. The listener class shouldn't touch any data in 
`TypeCategoryMap` (because everything is private), except by using the class' 
own methods, and all of them acquire the mutex on entry.

  This change makes it release the lock after `m_map_mutex` is updated, but 
before the listener is invoked.

- `FormatManager::GetCategoryForLanguage` does a map lookup (holding 
`m_language_categories_mutex`) and, if the lookup fails, constructs a new 
`LanguageCategory`. This also results in listeners being called while still 
holding the mutex.

  This patch changes it so that the lock is only held during initial lookup. If 
lookup fails, the new category will be created and only then we'll acquire the 
lock again to insert the new category into the map.

Here's the Helgrind report for reference:

  ==3574084== 
  ==3574084==
  ==3574084== Thread #6: lock order "0x1472C070 before 0x1472C110" violated
  ==3574084==
  ==3574084== Observed (incorrect) order is: acquisition of lock at 0x1472C110
  ==3574084==at 0x484777F: mutex_lock_WRK (hg_intercepts.c:942)
  ==3574084==by 0xADFF5E2: __gthread_mutex_lock(pthread_mutex_t*) 
(bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:749)
  ==3574084==by 0xADFF5B4: __gthread_recursive_mutex_lock(pthread_mutex_t*) 
(bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:811)
  ==3574084==by 0xAE00D34: std::recursive_mutex::lock() 
(bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/mutex:108)
  ==3574084==by 0xADFFEB2: 
std::lock_guard::lock_guard(std::recursive_mutex&) 
(bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_mutex.h:229)
  ==3574084==by 0xB2F1E04: 
lldb_private::TypeCategoryMap::Add(lldb_private::ConstString, 
std::shared_ptr const&) 
(llvm/lldb/source/DataFormatters/TypeCategoryMap.cpp:28)
  ==3574084==by 0xB2D8A57: 
lldb_private::FormatManager::GetCategory(lldb_private::ConstString, bool) 
(llvm/lldb/source/DataFormatters/FormatManager.cpp:423)
  ==3574084==by 0xB2D380B: 
lldb_private::DataVisualization::Categories::GetCategory(lldb_private::ConstString,
 std::shared_ptr&, bool) 
(llvm/lldb/source/DataFormatters/DataVisualization.cpp:79)
  ==3574084==by 0xAE47DFD: lldb::SBDebugger::CreateCategory(char const*) 
(llvm/lldb/source/API/SBDebugger.cpp:1535)
  ==3574084==by 0xAF7BA0B: _wrap_SBDebugger_CreateCategory(_object*, 
_object*) (LLDBWrapPython.cpp:24738)
  ==3574084==by 0x14E77483: ??? (in 
/usr/lib/x86_64-linux-gnu/libpython3.9.so.1.0)
  ==3574084==by 0x14E33A07: _PyObject_MakeTpCall (in 
/usr/lib/x86_64-linux-gnu/libpython3.9.so.1.0)
  ==3574084==
  ==3574084==  followed by a later acquisition of lock at 0x1472C070
  ==3574084==at 0x484777F: mutex_lock_WRK (hg_intercepts.c:942)
  ==3574084==by 0xADFF5E2: __gthread_mutex_lock(pthread_mutex_t*) 
(bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:749)
  ==3574084==by 0xADFF5B4: __gthread_recursive_mutex_lock(pthread_mutex_t*) 
(bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:811)
  ==3574084==by 0xAE00D34: std::recursive_mutex::lock() 
(bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/mutex:108)
  ==3574084==by 0xADFFEB2: 
std::lock_guard::lock_guard(std::recursive_mutex&) 
(bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_mutex.h:229)
  ==3574084==by 0xB2D6DDA: lldb_private::FormatManager::Changed() 
(llvm/lldb/source/DataFormatters/FormatManager.cpp:118)
  ==3574084==by 0xB2F1E46: 
lldb_private::TypeCategoryMap::Add(lldb_private::ConstString, 
std::shared_ptr const&) 
(llvm/lldb/source/DataFormatters/TypeCategoryMap.cpp:31)
  ==3574084==by 0xB2D8A57: 
lldb_private::FormatManager::GetCategory(lldb_private::ConstString, bool) 
(llvm/lldb/source/DataFormatters/FormatManager.cpp:423)
  ==3574084==by 0xB2D380B: 
lldb_private::DataVisualization::Categories::GetCategory(lldb_private::ConstString,
 std::shared_p

[Lldb-commits] [lldb] a1d4903 - Fix lldb-vscode frame test failure

[Jeffrey Tan via lldb-commits]

Author: Jeffrey Tan
Date: 2022-05-23T14:04:37-07:00
New Revision: a1d490319a9e2196786f6504e0c447ee0a1dad71

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

LOG: Fix lldb-vscode frame test failure

Previous patch (https://reviews.llvm.org/D126013) added a new "optimized"
attribute to DAP stack frame this caused some tests, like
lldb-vscode/coreFile/TestVSCode_coreFile.py
to fail because the tests explicitly check for all attributes.

To fix the test failure I decided to remove this attribute.

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

Added: 


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

Removed: 




diff  --git a/lldb/tools/lldb-vscode/JSONUtils.cpp 
b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 1fb4c00a8255..4bc965e61b81 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -765,7 +765,6 @@ llvm::json::Value CreateStackFrame(lldb::SBFrame &frame) {
   if (is_optimized)
 frame_name += " [opt]";
   EmplaceSafeString(object, "name", frame_name);
-  object.try_emplace("optimized", is_optimized);
 
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));



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

[Lldb-commits] [PATCH] D126225: Fix lldb-vscode frame test failure

[jeffrey tan via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1d490319a9e: Fix lldb-vscode frame test failure (authored 
by yinghuitan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126225

Files:
  lldb/tools/lldb-vscode/JSONUtils.cpp


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -765,7 +765,6 @@
   if (is_optimized)
 frame_name += " [opt]";
   EmplaceSafeString(object, "name", frame_name);
-  object.try_emplace("optimized", is_optimized);
 
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));


Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -765,7 +765,6 @@
   if (is_optimized)
 frame_name += " [opt]";
   EmplaceSafeString(object, "name", frame_name);
-  object.try_emplace("optimized", is_optimized);
 
   int64_t disasm_line = 0;
   object.try_emplace("source", CreateSource(frame, disasm_line));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

In D122974#3481269 , @llunak wrote:

> In D122974#3480686 , @dblaikie 
> wrote:
>
>> In D122974#3424654 , @JDevlieghere 
>> wrote:
>>
>>> 
>
> ...
>
>>>   struct HashedStringRef {
>>> const llvm::StringRef str;
>>> const unsigned full_hash;
>>> const uint8_t hash; 
>>> HashedStringRef(llvm::StringRef s) : str(s), full_hash(djbHash(str)), 
>>> hash(hash(str)) {}
>>>   }
>
> ...
>
>> The external code shouldn't be able to create their own (ctor 
>> private/protected, etc) - the only way to get one is to call `StringMap` to 
>> produce one.
>
> But what if I don't want StringMap to compute the hash at all? There's still 
> that 10-15% of CPU time spent in djbHash() when debug info uses exactly[*] 
> that (and LLDB's index cache could be modified to store it). Which means it 
> can be useful for StringMap to allow accepting precomputed hash, and then 
> what purpose will that HashedStringRef have?

If that happens, yeah, there's probably no point protecting this API - though 
I'd be more cautious of that change/any change that supports serializing the 
hash as this could end up causing StringMap to have to have stability 
guarantees that might be unfavorable for other uses (see ABSL maps that 
intentionally randomize/reseed each instance to ensure users aren't depending 
on undocumented guarantees - this gives them the flexibility to update the hash 
algorithm with something better in the future without breaking users who might 
be serializing the hashes/relying on iteration order/etc)

If we want a structure that can use a stable hash - it might be at that point 
that we move the hash support entirely out of StringMap and make it pluggable, 
with the default implementation doing djbHash as before, and even the new 
"stable" one doing that too, but doing it explicitly/documenting what 
guarantees it requires (stability within a major version? Across major 
versions?)

& then I guess what the API looks like is still an open question - perhaps the 
default trait (the one without any stability guarantees) could have a private 
implementation and expose that to StringMap via friendship. The stable 
implementation can have a public implementation for hashing & then an API like 
the one proposed where they can be passed into StringMap (yeah, without any of 
the handle safety previously proposed) - and assert when it's passed in that it 
matches what the trait provides (under EXPENSIVE_CHECKS, probably).


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

https://reviews.llvm.org/D122974

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

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: JDevlieghere, labath, chaoren, clayborg, kastiglione.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

One of the weaknesses of the signal handling model in lldb is that we access 
the information about signals from the process.  That's good, since different 
OS'es and even different OS versions might have more or fewer signals, or they 
may be renumbered, etc.

However, it means you can't set signal handling behaviors in your .lldbinit, 
and if you set them in one run they don't persist on re-run.  That's been 
somewhat annoying over the years.

This adds a "DummySignals" map to the target that holds signals as 
std::strings, and the user intentions.  I called them DummySignals because if 
you set them in an .lldbinit, they will go into the Dummy Target, and then will 
prime new targets.  They work like all the other Dummy Target elements, if you 
add a handling with no Targets, they go to the dummy target then to your 
target.  If you add them when there's a real target, it will only go into that 
target.

Just like before, if you run `process handle` when you have a process, the 
UnixSignals values will be changed, but the change will also be recorded in the 
DummySignals map so we can apply it on re-run.

I also added a `-c` option to "process handle" which allows you to clear the 
setting from your target (and with `-d` from the dummy target).
I also added a `-t` option to "process handle" to show you just the changes you 
made to the default setting values.

Since I wanted you to also  be able to "reset to the default" I had to change 
the UnixSignals class so it recorded the default value as well as the current 
value.

There are a couple of pieces of this that aren't done yet, but the patch was 
getting pretty big.

1. No SB API.  I'll add an SBTarget::HandleSignal to set the handling in the 
dummy target.
2. When you add a signal with no process, we can't spell-check the signal name 
for you.  The current patch emits a warning on Launch or Attach if there was a 
signal whose handling is configured but the signal name wasn't recognized.

I don't plan to do #2 right now.  You would have to have the AddSignal add the 
signal name to a global pool, and then construct all the Platforms and get them 
to make their UnixSignals so the registry would happen.  Then you could compare 
the signal against this list.

I'm not even sure that being able to spell check the signals is worth having to 
construct all the platforms to build this registry.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126259

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/UnixSignals.h
  lldb/packages/Python/lldbsuite/test/lldbutil.py
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/Options.td
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/test/API/commands/process/handle/Makefile
  lldb/test/API/commands/process/handle/TestProcessHandle.py
  lldb/test/API/commands/process/handle/main.cpp
  lldb/test/API/functionalities/signal/raise/TestRaise.py
  lldb/unittests/Signals/UnixSignalsTest.cpp

Index: lldb/unittests/Signals/UnixSignalsTest.cpp
===
--- lldb/unittests/Signals/UnixSignalsTest.cpp
+++ lldb/unittests/Signals/UnixSignalsTest.cpp
@@ -53,6 +53,29 @@
   EXPECT_EQ(LLDB_INVALID_SIGNAL_NUMBER, signals.GetNextSignalNumber(16));
 }
 
+TEST(UnixSignalsTest, Reset) {
+  TestSignals signals;
+  bool stop_val = signals.GetShouldStop(2);
+  bool notify_val   = signals.GetShouldNotify(2);
+  bool suppress_val = signals.GetShouldSuppress(2);
+  
+  // Change two, then reset one and make sure only that one was reset:
+  EXPECT_EQ(true, signals.SetShouldNotify(2, !notify_val));
+  EXPECT_EQ(true, signals.SetShouldSuppress(2, !suppress_val));
+  EXPECT_EQ(true, signals.ResetSignal(2, false, true, false));
+  EXPECT_EQ(stop_val, signals.GetShouldStop(2));
+  EXPECT_EQ(notify_val, signals.GetShouldStop(2));
+  EXPECT_EQ(!suppress_val, signals.GetShouldNotify(2));
+  
+  // Make sure reset with no arguments resets them all:
+  EXPECT_EQ(true, signals.SetShouldSuppress(2, !suppress_val));
+  EXPECT_EQ(true, signals.SetShouldNotify(2, !notify_val));
+  EXPECT_EQ(true, signals.ResetSignal(2));
+  EXPECT_EQ(stop_val, signals.GetShouldStop(2));
+  EXPECT_EQ(notify_val, signals.GetShouldNotify(2));
+  EXPECT_EQ(suppress_val, signals.GetShouldSuppress(2));
+}
+
 TEST(UnixSignalsTest, GetInfo) {
   TestSignals signals;
 
Index: lldb/test/API/functionalities/signal/raise/TestRaise.py
===
--- lldb/test/API/functionalities/signal/raise/TestRaise.py
+++ lldb/test/API/functionalities/signal/raise/TestRaise.py
@@ -36,6 +36,10 @@
 
 def launch(self, target,

[Lldb-commits] [PATCH] D126260: [lldb/crashlog] Add support for Application Specific Backtraces

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

For an exception crashlog, the thread backtracesaren't usually very helpful
and instead, developpers look at the "Application Specific Backtrace" that
was generated by `objc_exception_throw`.

LLDB could already parse and symbolicate these Application Specific Backtraces
for regular textual-based crashlog, so this patch adds support to parse them
in JSON crashlogs.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126260

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/scripted_process/crashlog_scripted_process.py


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -140,6 +140,8 @@
 self.idx = self.backing_thread.index
 self.tid = self.backing_thread.id
 self.name = self.backing_thread.name
+if self.backing_thread.app_specific_backtrace:
+self.name = "Application Specific Backtrace - " + str(self.idx)
 self.queue = self.backing_thread.queue
 self.has_crashed = (self.scripted_process.crashed_thread_idx == 
self.idx)
 self.create_stackframes()
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -449,6 +449,8 @@
 self.parse_images(self.data['usedImages'])
 self.parse_main_image(self.data)
 self.parse_threads(self.data['threads'])
+if 'asiBacktraces' in self.data:
+self.parse_app_specific_backtraces(self.data['asiBacktraces'])
 self.parse_errors(self.data)
 thread = self.crashlog.threads[self.crashlog.crashed_thread_idx]
 reason = self.parse_crash_reason(self.data['exception'])
@@ -560,6 +562,37 @@
 self.crashlog.threads.append(thread)
 idx += 1
 
+def parse_app_specific_backtraces(self, json_app_specific_bts):
+def parse_asi_backtrace(self, thread, bt):
+frame_regex = re.compile(r'^([0-9]+)' r'\s'# id
+r'+(.+?)'r'\s+'   # img_name
+r'(0x[0-9a-fA-F]{7}[0-9a-fA-F]+)' # addr
+r' +(.*)' # offs
+)
+
+for line in bt.split('\n'):
+frame_match = frame_regex.search(line)
+if not frame_match:
+print("error: Couldn't parse Application Specific 
Backtrace.")
+return False
+
+(frame_id, frame_img_name,
+frame_addr, frame_ofs) = frame_match.groups()
+
+thread.add_ident(frame_img_name)
+if frame_img_name not in self.crashlog.idents:
+self.crashlog.idents.append(frame_img_name)
+thread.frames.append(self.crashlog.Frame(int(frame_id), int(
+frame_addr, 0), frame_ofs))
+
+return True
+
+for idx, backtrace in enumerate(json_app_specific_bts):
+thread = self.crashlog.Thread(idx, True)
+thread.queue = "Application Specific Backtraces"
+if parse_asi_backtrace(self, thread, backtrace):
+self.crashlog.threads.append(thread)
+
 def parse_thread_registers(self, json_thread_state, prefix=None):
 registers = dict()
 for key, state in json_thread_state.items():


Index: lldb/examples/python/scripted_process/crashlog_scripted_process.py
===
--- lldb/examples/python/scripted_process/crashlog_scripted_process.py
+++ lldb/examples/python/scripted_process/crashlog_scripted_process.py
@@ -140,6 +140,8 @@
 self.idx = self.backing_thread.index
 self.tid = self.backing_thread.id
 self.name = self.backing_thread.name
+if self.backing_thread.app_specific_backtrace:
+self.name = "Application Specific Backtrace - " + str(self.idx)
 self.queue = self.backing_thread.queue
 self.has_crashed = (self.scripted_process.crashed_thread_idx == self.idx)
 self.create_stackframes()
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -449,6 +449,8 @@
 self.parse_images(self.data['usedImages'])
 self.parse_main_image(self.data)
 self.parse_threads(self.data['threads'])
+

[Lldb-commits] [PATCH] D126260: [lldb/crashlog] Add support for Application Specific Backtraces

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Test? :-)




Comment at: lldb/examples/python/crashlog.py:567-570
+frame_regex = re.compile(r'^([0-9]+)' r'\s'# id
+r'+(.+?)'r'\s+'   # img_name
+r'(0x[0-9a-fA-F]{7}[0-9a-fA-F]+)' # addr
+r' +(.*)' # offs

Do we have an existing regex for this from the textual parser?



Comment at: lldb/examples/python/crashlog.py:576
+if not frame_match:
+print("error: Couldn't parse Application Specific 
Backtrace.")
+return False

For consistency with the other errors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126260

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

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Target/Target.h:1421
+protected:
+  struct DummySignalValues {
+LazyBool pass = eLazyBoolCalculate;

We should make UnixSignals::Signal a public class and then just save a 
std::vector of those inside this class. That class doesn't contain a signal 
number, so we should be able to re-use it here and avoid creating a new class 
that mimic what is contains. 



Comment at: lldb/include/lldb/Target/Target.h:1429
+  };
+  using DummySignalElement = std::pair;
+  static bool UpdateSignalFromDummy(lldb::UnixSignalsSP signals_sp, 

Maybe store a std::vector objects filled out as much as 
possible here? Then we don't need the DummySignalElement type at all.



Comment at: lldb/include/lldb/Target/Target.h:1533-1536
+  std::map m_dummy_signals;/// These are used 
to
+  /// set the signal state when you don't have a process and more usefully 
+  /// in the Dummy target where you can't know exactly what signals you 
+  /// will have.

Switch to std::vector< UnixSignal::Signal> here and then we can prime the 
UnixSignals with the vector?



Comment at: lldb/include/lldb/Target/UnixSignals.h:120
 bool m_suppress : 1, m_stop : 1, m_notify : 1;
+bool m_default_suppress : 1, m_default_stop : 1, m_default_notify : 1;
 

If we store a vector in the target, can we reset the 
default values from another "Signal" struct instead of saving it here? Not a 
big deal if not, but just wondering.



Comment at: lldb/include/lldb/Target/UnixSignals.h:126
 ~Signal() = default;
+void Reset(bool reset_stop, bool reset_notify, bool reset_suppress);
   };

To follow up with he above comment, this _could_ become:
```
void Reset(const UnixSignals::Signal &signal);
```
we can find the UnixSignals::Signal object for this signal first in the target 
and pass it down into here?



Comment at: lldb/source/Commands/Options.td:754
+  def process_handle_dummy : Option<"dummy", "d">, Group<2>,
+Desc<"Also clear the values in the dummy target so they won't be inherited 
by new targets.">;
 }

Do we need the "Also" in the documentation here? Is this option only available 
when used with another option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126259

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

[Lldb-commits] [PATCH] D126267: [trace][intelpt] Support system-wide tracing [13] - Add context switch decoding

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added a reviewer: jj10306.
Herald added subscribers: mgrang, mgorny.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Add the logic that parses all cpu context switch traces and produces blocks 
of continuous executions, which will be later used to assign intel pt subtraces 
to threads and to identify gaps. This logic can also identify if the context 
switch trace is malformed.
- The continuous executions blocks are able to indicate when there were some 
contention issues when producing the context switch trace. See the inline 
comments for more information.
- Update the 'dump info' command to show information and stats related to the 
multicore decoding flow, including timing about context switch decoding.
- Add the logic to conver nanoseconds to TSCs.
- Fix a bug when returning the context switches. Now they data returned makes 
sense and even empty traces can be returned from lldb-server.
- Finish the necessary bits for loading and saving a multi-core trace bundle 
from disk.
- Change some size_t to uint64_t for compatibility with 32 bit systems.

Tested by saving a trace session of a program that sleeps 100 times, it was 
able to produce the following 'dump info' text:

  (lldb) trace load /tmp/trace3/trace.json  
 (lldb) thread trace dump info  
Trace technology: 
intel-pt
  
  thread #1: tid = 4192415
Total number of instructions: 1
  
Memory usage:
  Total approximate memory usage (excluding raw trace): 2.51 KiB
  Average memory usage per instruction (excluding raw trace): 2573.00 bytes
  
Timing for this thread:
  
Timing for global tasks:
  Context switch trace decoding: 0.00s
  
Events:
  Number of instructions with events: 0
  Number of individual events: 0
  
Multi-core decoding:
  Total number of continuous executions found: 2499
  Number of continuous executions for this thread: 102
  
Errors:
  Number of TSC decoding errors: 0


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126267

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/TraceGDBRemotePackets.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/Perf.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TaskTimer.cpp
  lldb/source/Plugins/Trace/intel-pt/TaskTimer.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/TraceGDBRemotePackets.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceLoad.py
  
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -233,5 +233,6 @@
 # We must have captured the context switch of when the target resumed
 self.assertTrue(found_non_empty_context_switch)
 
+self.expect("thread trace dump instructions", substrs=['unimplemented'])
 
 self.traceStopProcess()
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -43,8 +43,8 @@
 Total approximate memory usage (excluding raw trace): 1.27 KiB
 Average memory usage per instruction (excluding raw trace): 61.76 bytes
 
-  Timing:
-Decoding instructions: ''', '''s
+  Timing for this thread:
+Decoding instructions: ''', '''
 
   Events:
 Number of instructions with events: 1
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -45,8 +45,8 @@
 Total approximate memory usage (exc

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D122974#3532822 , @dblaikie wrote:

> If we want a structure that can use a stable hash

Not for D124704 . It doesn't make sense to 
require a stable hash algorithm for an internal cache file. All that it 
requires is that StringMap provides the hash value for a string, which it does 
with the public hash(). If that implementation changes, the EXPENSIVE_CHECKS 
assert will find that when unittesting the LLDB parts, and in that case the 
versioned cache file can have its version increased and problem solved. Even in 
case StringMap implementation changes in a way that no longer makes it feasible 
to store the precomputed hash value it's simpler to just dump the optimization.

So no need to overcomplicate this. As for the case of somebody else trying to 
rely on the stability of the hash, I can solve that by adding a comment that 
says it's not stable (and then that somebody else can go to the lengths of 
making it stable if needed).


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

https://reviews.llvm.org/D122974

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

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

[Luboš Luňák via Phabricator via lldb-commits]

llunak updated this revision to Diff 431597.
llunak added a comment.

Added documentation for StringMap::hash(), including an explicit comment saying 
that the implementation is not guaranteed to be stable.


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

https://reviews.llvm.org/D122974

Files:
  lldb/source/Utility/ConstString.cpp
  llvm/include/llvm/ADT/StringMap.h
  llvm/lib/Support/StringMap.cpp

Index: llvm/lib/Support/StringMap.cpp
===
--- llvm/lib/Support/StringMap.cpp
+++ llvm/lib/Support/StringMap.cpp
@@ -11,7 +11,6 @@
 //===--===//
 
 #include "llvm/ADT/StringMap.h"
-#include "llvm/Support/DJB.h"
 #include "llvm/Support/MathExtras.h"
 
 using namespace llvm;
@@ -80,11 +79,14 @@
 /// specified bucket will be non-null.  Otherwise, it will be null.  In either
 /// case, the FullHashValue field of the bucket will be set to the hash value
 /// of the string.
-unsigned StringMapImpl::LookupBucketFor(StringRef Name) {
+unsigned StringMapImpl::LookupBucketFor(StringRef Name,
+uint32_t FullHashValue) {
+#ifdef EXPENSIVE_CHECKS
+  assert(FullHashValue == hash(Name));
+#endif
   // Hash table unallocated so far?
   if (NumBuckets == 0)
 init(16);
-  unsigned FullHashValue = djbHash(Name, 0);
   unsigned BucketNo = FullHashValue & (NumBuckets - 1);
   unsigned *HashTable = getHashTable(TheTable, NumBuckets);
 
@@ -136,10 +138,12 @@
 /// FindKey - Look up the bucket that contains the specified key. If it exists
 /// in the map, return the bucket number of the key.  Otherwise return -1.
 /// This does not modify the map.
-int StringMapImpl::FindKey(StringRef Key) const {
+int StringMapImpl::FindKey(StringRef Key, uint32_t FullHashValue) const {
   if (NumBuckets == 0)
 return -1; // Really empty table?
-  unsigned FullHashValue = djbHash(Key, 0);
+#ifdef EXPENSIVE_CHECKS
+  assert(FullHashValue == hash(Key));
+#endif
   unsigned BucketNo = FullHashValue & (NumBuckets - 1);
   unsigned *HashTable = getHashTable(TheTable, NumBuckets);
 
Index: llvm/include/llvm/ADT/StringMap.h
===
--- llvm/include/llvm/ADT/StringMap.h
+++ llvm/include/llvm/ADT/StringMap.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringMapEntry.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Support/AllocatorBase.h"
+#include "llvm/Support/DJB.h"
 #include "llvm/Support/PointerLikeTypeTraits.h"
 #include 
 #include 
@@ -60,12 +61,20 @@
   /// specified bucket will be non-null.  Otherwise, it will be null.  In either
   /// case, the FullHashValue field of the bucket will be set to the hash value
   /// of the string.
-  unsigned LookupBucketFor(StringRef Key);
+  unsigned LookupBucketFor(StringRef Key) {
+return LookupBucketFor(Key, hash(Key));
+  }
+
+  /// Overload that explicitly takes precomputed hash(Key).
+  unsigned LookupBucketFor(StringRef Key, uint32_t FullHashValue);
 
   /// FindKey - Look up the bucket that contains the specified key. If it exists
   /// in the map, return the bucket number of the key.  Otherwise return -1.
   /// This does not modify the map.
-  int FindKey(StringRef Key) const;
+  int FindKey(StringRef Key) const { return FindKey(Key, hash(Key)); }
+
+  /// Overload that explicitly takes precomputed hash(Key).
+  int FindKey(StringRef Key, uint32_t FullHashValue) const;
 
   /// RemoveKey - Remove the specified StringMapEntry from the table, but do not
   /// delete it.  This aborts if the value isn't in the table.
@@ -94,6 +103,13 @@
   bool empty() const { return NumItems == 0; }
   unsigned size() const { return NumItems; }
 
+  /// Returns the hash value that will be used for the given string.
+  /// This allows precomputing the value and passing it explicitly
+  /// to some of the functions.
+  /// The implementation of this function is not guaranteed to be stable
+  /// and may change.
+  static uint32_t hash(StringRef Key) { return llvm::djbHash(Key, 0); }
+
   void swap(StringMapImpl &Other) {
 std::swap(TheTable, Other.TheTable);
 std::swap(NumBuckets, Other.NumBuckets);
@@ -215,15 +231,19 @@
   StringMapKeyIterator(end()));
   }
 
-  iterator find(StringRef Key) {
-int Bucket = FindKey(Key);
+  iterator find(StringRef Key) { return find(Key, hash(Key)); }
+
+  iterator find(StringRef Key, uint32_t FullHashValue) {
+int Bucket = FindKey(Key, FullHashValue);
 if (Bucket == -1)
   return end();
 return iterator(TheTable + Bucket, true);
   }
 
-  const_iterator find(StringRef Key) const {
-int Bucket = FindKey(Key);
+  const_iterator find(StringRef Key) const { return find(Key, hash(Key)); }
+
+  const_iterator find(StringRef Key, uint32_t FullHashValue) const {
+int Bucket = FindKey(Key, FullHashValue);
 if (Bucket == -1)
   return end();
 return const_iterator(TheTable + Buck