[Lldb-commits] [PATCH] D104067: [lldb] Decouple ObjCLanguage from Symtab

2021-06-22 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104067

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

[Lldb-commits] [lldb] 35cf5b1 - [lldb] Bumb Clang version requirement for TestBasicEntryValues.py to 11

2021-06-22 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2021-06-22T11:58:05+02:00
New Revision: 35cf5b109769ceb356a9013ca0c0f60fbd230080

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

LOG: [lldb] Bumb Clang version requirement for TestBasicEntryValues.py to 11

The test only passes with Clang>=11 so adjust the decorator.

Failure output for Clang 10 is:

--- FileCheck trace (code=1) ---
FileCheck main.cpp -check-prefix=FUNC1-GNU

FileCheck input:
  Address: a.out[0x00401127] (a.out.PT_LOAD[1]..text + 263)
  Summary: a.out`func1(int&) + 23 at main.cpp:25:1
   Module: file = 
"functionalities/param_entry_vals/basic_entry_values/BasicEntryValues_GNU.test_dwo/a.out",
 arch = "x86_64"
  CompileUnit: id = {0x}, file = 
"functionalities/param_entry_vals/basic_entry_values/main.cpp", language = 
"c++11"
 Function: id = {0x410a}, name = "func1(int&)", mangled = 
"_Z5func1Ri", range = [0x00401110-0x00401129)
 FuncType: id = {0x410a}, byte-size = 0, decl = main.cpp:13, 
compiler_type = "void (int &)"
   Blocks: id = {0x410a}, range = [0x00401110-0x00401129)
LineEntry: [0x00401127-0x00401130): 
functionalities/param_entry_vals/basic_entry_values/main.cpp:25:1
   Symbol: id = {0x002c}, range = 
[0x00401110-0x00401129), name="func1(int&)", 
mangled="_Z5func1Ri"

FileCheck output:

functionalities/param_entry_vals/basic_entry_values/main.cpp:23:16: error: 
FUNC1-GNU: expected string not found in input
 // FUNC1-GNU: name = "sink", type = "int &", location = DW_OP_GNU_entry_value

Added: 


Modified: 

lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
 
b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
index 4f294f555ec83..4b9a814764158 100644
--- 
a/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
+++ 
b/lldb/test/API/functionalities/param_entry_vals/basic_entry_values/TestBasicEntryValues.py
@@ -4,7 +4,7 @@
 
 supported_archs = ["x86_64", "aarch64"]
 decorators = [skipIf(archs=no_match(supported_archs)),
- skipIf(compiler="clang", compiler_version=['<', '10.0']),
+ skipIf(compiler="clang", compiler_version=['<', '11.0']),
  skipUnlessHasCallSiteInfo,
  skipIf(dwarf_version=['<', '4'])]
 



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

[Lldb-commits] [lldb] 98e2b1a - [lldb] Adjust Clang version requirements for tail_call_frames tests

2021-06-22 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2021-06-22T12:22:15+02:00
New Revision: 98e2b1a8dd8ff03b6289a40a16fe749834257494

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

LOG: [lldb] Adjust Clang version requirements for tail_call_frames tests

Those tests are all failing for older Clang versions. This is adding the
respective test decorators for the passing Clang versions to get the recently
revived matrix bot green.

Added: 


Modified: 

lldb/test/API/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py

lldb/test/API/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py

lldb/test/API/functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py

lldb/test/API/functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py

lldb/test/API/functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py

lldb/test/API/functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py

lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py
 
b/lldb/test/API/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py
index 9772ceea989ad..45346a42c2370 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/cross_dso/TestCrossDSOTailCalls.py
@@ -15,7 +15,7 @@ class TestCrossDSOTailCalls(TestBase):
 def setUp(self):
 TestBase.setUp(self)
 
-@skipIf(compiler="clang", compiler_version=['<', '8.0'])
+@skipIf(compiler="clang", compiler_version=['<', '10.0'])
 @skipIf(dwarf_version=['<', '4'])
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 @expectedFailureAll(archs=['arm', 'aarch64'], bugnumber="llvm.org/PR44561")

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py
 
b/lldb/test/API/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py
index 223b7c79a3661..48e1c86d42bdb 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/cross_object/TestCrossObjectTailCalls.py
@@ -15,7 +15,7 @@ class TestCrossObjectTailCalls(TestBase):
 def setUp(self):
 TestBase.setUp(self)
 
-@skipIf(compiler="clang", compiler_version=['<', '8.0'])
+@skipIf(compiler="clang", compiler_version=['<', '10.0'])
 @skipIf(dwarf_version=['<', '4'])
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr26265")
 @expectedFailureAll(archs=['arm', 'aarch64'], bugnumber="llvm.org/PR44561")

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
index 699263e7150cb..55c718db6d243 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py
@@ -2,7 +2,8 @@
 from lldbsuite.test import decorators
 
 decor = [decorators.skipUnlessHasCallSiteInfo,
- decorators.skipIf(dwarf_version=['<', '4'])]
+ decorators.skipIf(dwarf_version=['<', '4']),
+ decorators.skipIf(compiler="clang", compiler_version=['<', '11.0'])]
 lldbinline.MakeInlineTest(__file__, globals(), name="DisambiguateCallSite_V5",
 build_dict=dict(CFLAGS_EXTRAS="-O2 -glldb"), decorators=decor)
 lldbinline.MakeInlineTest(__file__, globals(), name="DisambiguateCallSite_GNU",

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
index 80ed07992f1ed..e4b41185f8c7f 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py
@@ -2,7 +2,8 @@
 from lldbsuite.test import decorators
 
 decor = [decorators.skipUnlessHasCallSiteInfo,
- decorators.skipIf(dwarf_version=['<', '4'

[Lldb-commits] [PATCH] D104697: [lldb] Remove more redundant SetStatus(eReturnStatusFailed)

2021-06-22 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett created this revision.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Mostly by converting uses of GetErrorStream to AppendError,
so that the call to SetStatus is implicit.

Some remain where it isn't certain that you'll have a message
to set, or you want the output to be on stdout.

One place in CommandObjectWatchpoint previously didn't set
the status to failed at all. However it's pretty obvious
that it should do so.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104697

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectVersion.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Interpreter/CommandObject.cpp

Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -219,7 +219,6 @@
   // A process that is not running is considered paused.
   if (GetFlags().Test(eCommandProcessMustBeLaunched)) {
 result.AppendError("Process must exist.");
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
 } else {
@@ -239,7 +238,6 @@
   case eStateUnloaded:
 if (GetFlags().Test(eCommandProcessMustBeLaunched)) {
   result.AppendError("Process must be launched.");
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 break;
@@ -249,7 +247,6 @@
 if (GetFlags().Test(eCommandProcessMustBePaused)) {
   result.AppendError("Process is running.  Use 'process interrupt' to "
  "pause execution.");
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
   }
@@ -351,7 +348,6 @@
   Status error(group_options.NotifyOptionParsingFinished(&exe_ctx));
   if (error.Fail()) {
 result.AppendError(error.AsCString());
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
   return true;
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -941,11 +941,11 @@
 } else {
   const char *error_cstr = error.AsCString(nullptr);
   if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any variable "
-   "expression path that matches '%s'\n",
-   command.GetArgumentAtIndex(0));
+result.AppendErrorWithFormat("unable to find any variable "
+ "expression path that matches '%s'",
+ command.GetArgumentAtIndex(0));
   return false;
 }
 
@@ -1065,10 +1065,8 @@
 // If no argument is present, issue an error message.  There's no way to
 // set a watchpoint.
 if (raw_command.trim().empty()) {
-  result.GetErrorStream().Printf("error: required argument missing; "
- "specify an expression to evaluate into "
- "the address to watch for\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("required argument missing; specify an expression "
+ "to evaluate into the address to watch for");
   return false;
 }
 
@@ -1095,12 +1093,10 @@
 ExpressionResults expr_result =
 target->EvaluateExpression(expr, frame, valobj_sp, options);
 if (expr_result != eExpressionCompleted) {
-  result.GetErrorStream().Printf(
-  "error: expression evaluation of address to watch failed\n");
-  result.GetErrorStream() << "expression evaluated: \n" << expr << "\n";
+  result.AppendError("expression evaluation of address to watch failed");
+  result.AppendErrorWithFormat("expression evaluated: \n%s", expr.data());
   if (valobj_sp && !valobj_sp->GetError().Success())
-result.GetErrorStream() << valobj_sp->GetError().AsCString() << "\n";
-  result.SetStatus(eReturnStatusFailed);
+result.AppendError(valobj_sp->GetError().AsCString());
   return false;
 }
 
@@ -1108,9 +1104,7 @@
 bool success = false;
 addr = valobj_sp->GetValueAsUnsigned(0, &success);
 if (!success) {
-  result.GetErrorStream().Printf(
-  "error: expression did not evaluate to an address\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("expression did not evaluate to an address");
   return false;
 }
 
Index: lldb/source/Commands/CommandObjectVersion.cpp
=

[Lldb-commits] [PATCH] D104697: [lldb] Remove more redundant SetStatus(eReturnStatusFailed)

2021-06-22 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Commands/CommandObjectReproducer.cpp:141
 static void SetError(CommandReturnObject &result, Error err) {
-  result.GetErrorStream().Printf("error: %s\n",
- toString(std::move(err)).c_str());
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError(toString(std::move(err)).c_str());
 }

You can drop the `c_str()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104697

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

[Lldb-commits] [PATCH] D104697: [lldb] Remove more redundant SetStatus(eReturnStatusFailed)

2021-06-22 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett updated this revision to Diff 353598.
DavidSpickett added a comment.

Remove c_str() in Reproducer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104697

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectVersion.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Interpreter/CommandObject.cpp

Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -219,7 +219,6 @@
   // A process that is not running is considered paused.
   if (GetFlags().Test(eCommandProcessMustBeLaunched)) {
 result.AppendError("Process must exist.");
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
 } else {
@@ -239,7 +238,6 @@
   case eStateUnloaded:
 if (GetFlags().Test(eCommandProcessMustBeLaunched)) {
   result.AppendError("Process must be launched.");
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 break;
@@ -249,7 +247,6 @@
 if (GetFlags().Test(eCommandProcessMustBePaused)) {
   result.AppendError("Process is running.  Use 'process interrupt' to "
  "pause execution.");
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
   }
@@ -351,7 +348,6 @@
   Status error(group_options.NotifyOptionParsingFinished(&exe_ctx));
   if (error.Fail()) {
 result.AppendError(error.AsCString());
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
   return true;
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -941,11 +941,11 @@
 } else {
   const char *error_cstr = error.AsCString(nullptr);
   if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any variable "
-   "expression path that matches '%s'\n",
-   command.GetArgumentAtIndex(0));
+result.AppendErrorWithFormat("unable to find any variable "
+ "expression path that matches '%s'",
+ command.GetArgumentAtIndex(0));
   return false;
 }
 
@@ -1065,10 +1065,8 @@
 // If no argument is present, issue an error message.  There's no way to
 // set a watchpoint.
 if (raw_command.trim().empty()) {
-  result.GetErrorStream().Printf("error: required argument missing; "
- "specify an expression to evaluate into "
- "the address to watch for\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("required argument missing; specify an expression "
+ "to evaluate into the address to watch for");
   return false;
 }
 
@@ -1095,12 +1093,10 @@
 ExpressionResults expr_result =
 target->EvaluateExpression(expr, frame, valobj_sp, options);
 if (expr_result != eExpressionCompleted) {
-  result.GetErrorStream().Printf(
-  "error: expression evaluation of address to watch failed\n");
-  result.GetErrorStream() << "expression evaluated: \n" << expr << "\n";
+  result.AppendError("expression evaluation of address to watch failed");
+  result.AppendErrorWithFormat("expression evaluated: \n%s", expr.data());
   if (valobj_sp && !valobj_sp->GetError().Success())
-result.GetErrorStream() << valobj_sp->GetError().AsCString() << "\n";
-  result.SetStatus(eReturnStatusFailed);
+result.AppendError(valobj_sp->GetError().AsCString());
   return false;
 }
 
@@ -1108,9 +1104,7 @@
 bool success = false;
 addr = valobj_sp->GetValueAsUnsigned(0, &success);
 if (!success) {
-  result.GetErrorStream().Printf(
-  "error: expression did not evaluate to an address\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("expression did not evaluate to an address");
   return false;
 }
 
Index: lldb/source/Commands/CommandObjectVersion.cpp
===
--- lldb/source/Commands/CommandObjectVersion.cpp
+++ lldb/source/Commands/CommandObjectVersion.cpp
@@ -28,7 +28,6 @@
 result.SetStatus(eReturnStatusSuccessFinishResult);
   } else {
 result.AppendError("the version command takes no arguments.");
-res

[Lldb-commits] [lldb] 28058d4 - [LLDB] Skip TestExitDuringExpression on aarch64/linux buildbot

2021-06-22 Thread Muhammad Omair Javaid via lldb-commits 

Author: Muhammad Omair Javaid
Date: 2021-06-22T16:22:48+05:00
New Revision: 28058d4cd10dac8129a5d5760597e832ea6eef81

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

LOG: [LLDB] Skip TestExitDuringExpression on aarch64/linux buildbot

TestExitDuringExpression test_exit_before_one_thread_no_unwind fails
sporadically on both Arm and AArch64 linux buildbots. This seems like
manifesting itself on a fully loaded machine. I have not found a reliable
timeout value so marking it skip for now.

Added: 


Modified: 

lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
 
b/lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
index dafc0a967605c..97bf151b60a74 100644
--- 
a/lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
+++ 
b/lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
@@ -23,6 +23,7 @@ def test_exit_before_one_thread_unwind(self):
 self.exiting_expression_test(True, True)
 
 @skipIfWindows
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"], 
bugnumber="llvm.org/pr48414")
 @expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48414")
 @expectedFailureNetBSD
 def test_exit_before_one_thread_no_unwind(self):
@@ -109,4 +110,3 @@ def exiting_expression_test(self, before_one_thread_timeout 
, unwind):
 ret_val_value = ret_val.GetValueAsSigned(error)
 self.assertTrue(error.Success(), "Got ret_val's value")
 self.assertEqual(ret_val_value, 10, "We put the right value in 
ret_val")
-



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

[Lldb-commits] [lldb] c462048 - [lldb][NFC] Use SubsystemRAII in XcodeSDKModuleTests

2021-06-22 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2021-06-22T13:41:01+02:00
New Revision: c462048cc4c088f6d12e6a55c1ceb54fd731a2b3

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

LOG: [lldb][NFC] Use SubsystemRAII in XcodeSDKModuleTests

Added: 


Modified: 
lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Removed: 




diff  --git a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp 
b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
index 287143a0e338b..9fb4602a41e3d 100644
--- a/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp
@@ -21,14 +21,7 @@ using namespace lldb_private;
 #ifdef __APPLE__
 namespace {
 class XcodeSDKModuleTests : public testing::Test {
-  void SetUp() override {
-HostInfoBase::Initialize();
-PlatformMacOSX::Initialize();
-  }
-  void TearDown() override {
-PlatformMacOSX::Terminate();
-HostInfoBase::Terminate();
-  }
+  SubsystemRAII subsystems;
 };
 } // namespace
 



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

[Lldb-commits] [lldb] 48e2d3a - [lldb][NFC] Remove an outdated comment in HostInfoBase

2021-06-22 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2021-06-22T16:48:17+02:00
New Revision: 48e2d3a5c23f7a11972c30f92d48d85a6a6bf1bd

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

LOG: [lldb][NFC] Remove an outdated comment in HostInfoBase

We should *never* use static local variables in this file as this makes
unittesting the plugin code impossible (and this whole 'testing' thing has
turned out to be rather useful so far).

Added: 


Modified: 
lldb/source/Host/common/HostInfoBase.cpp

Removed: 




diff  --git a/lldb/source/Host/common/HostInfoBase.cpp 
b/lldb/source/Host/common/HostInfoBase.cpp
index d923a40cab3bb..a6239a3208bce 100644
--- a/lldb/source/Host/common/HostInfoBase.cpp
+++ b/lldb/source/Host/common/HostInfoBase.cpp
@@ -31,12 +31,7 @@ using namespace lldb;
 using namespace lldb_private;
 
 namespace {
-// The HostInfoBaseFields is a work around for windows not supporting static
-// variables correctly in a thread safe way. Really each of the variables in
-// HostInfoBaseFields should live in the functions in which they are used and
-// each one should be static, but the work around is in place to avoid this
-// restriction. Ick.
-
+/// Contains the state of the HostInfoBase plugin.
 struct HostInfoBaseFields {
   ~HostInfoBaseFields() {
 if (FileSystem::Instance().Exists(m_lldb_process_tmp_dir)) {



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

[Lldb-commits] [PATCH] D104697: [lldb] Remove more redundant SetStatus(eReturnStatusFailed)

2021-06-22 Thread David Spickett 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 rGa8dd7094d364: [lldb] Remove more redundant 
SetStatus(eReturnStatusFailed) (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104697

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectVersion.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Interpreter/CommandObject.cpp

Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -219,7 +219,6 @@
   // A process that is not running is considered paused.
   if (GetFlags().Test(eCommandProcessMustBeLaunched)) {
 result.AppendError("Process must exist.");
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
 } else {
@@ -239,7 +238,6 @@
   case eStateUnloaded:
 if (GetFlags().Test(eCommandProcessMustBeLaunched)) {
   result.AppendError("Process must be launched.");
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
 break;
@@ -249,7 +247,6 @@
 if (GetFlags().Test(eCommandProcessMustBePaused)) {
   result.AppendError("Process is running.  Use 'process interrupt' to "
  "pause execution.");
-  result.SetStatus(eReturnStatusFailed);
   return false;
 }
   }
@@ -351,7 +348,6 @@
   Status error(group_options.NotifyOptionParsingFinished(&exe_ctx));
   if (error.Fail()) {
 result.AppendError(error.AsCString());
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
   return true;
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -941,11 +941,11 @@
 } else {
   const char *error_cstr = error.AsCString(nullptr);
   if (error_cstr)
-result.GetErrorStream().Printf("error: %s\n", error_cstr);
+result.AppendError(error_cstr);
   else
-result.GetErrorStream().Printf("error: unable to find any variable "
-   "expression path that matches '%s'\n",
-   command.GetArgumentAtIndex(0));
+result.AppendErrorWithFormat("unable to find any variable "
+ "expression path that matches '%s'",
+ command.GetArgumentAtIndex(0));
   return false;
 }
 
@@ -1065,10 +1065,8 @@
 // If no argument is present, issue an error message.  There's no way to
 // set a watchpoint.
 if (raw_command.trim().empty()) {
-  result.GetErrorStream().Printf("error: required argument missing; "
- "specify an expression to evaluate into "
- "the address to watch for\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("required argument missing; specify an expression "
+ "to evaluate into the address to watch for");
   return false;
 }
 
@@ -1095,12 +1093,10 @@
 ExpressionResults expr_result =
 target->EvaluateExpression(expr, frame, valobj_sp, options);
 if (expr_result != eExpressionCompleted) {
-  result.GetErrorStream().Printf(
-  "error: expression evaluation of address to watch failed\n");
-  result.GetErrorStream() << "expression evaluated: \n" << expr << "\n";
+  result.AppendError("expression evaluation of address to watch failed");
+  result.AppendErrorWithFormat("expression evaluated: \n%s", expr.data());
   if (valobj_sp && !valobj_sp->GetError().Success())
-result.GetErrorStream() << valobj_sp->GetError().AsCString() << "\n";
-  result.SetStatus(eReturnStatusFailed);
+result.AppendError(valobj_sp->GetError().AsCString());
   return false;
 }
 
@@ -1108,9 +1104,7 @@
 bool success = false;
 addr = valobj_sp->GetValueAsUnsigned(0, &success);
 if (!success) {
-  result.GetErrorStream().Printf(
-  "error: expression did not evaluate to an address\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("expression did not evaluate to an address");
   return false;
 }
 
Index: lldb/source/Commands/CommandObjectVersion.cpp
===
--- lldb/source/Commands/CommandObjectVersion.cpp
+++ lldb/source/Commands/CommandObjectVersion.cpp
@@ -28,7 +28,6 @@
 re

[Lldb-commits] [lldb] a8dd709 - [lldb] Remove more redundant SetStatus(eReturnStatusFailed)

2021-06-22 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2021-06-22T15:28:28Z
New Revision: a8dd7094d364f6fb4921627a36b920e5098e88c3

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

LOG: [lldb] Remove more redundant SetStatus(eReturnStatusFailed)

Mostly by converting uses of GetErrorStream to AppendError,
so that the call to SetStatus is implicit.

Some remain where it isn't certain that you'll have a message
to set, or you want the output to be on stdout.

One place in CommandObjectWatchpoint previously didn't set
the status to failed at all. However it's pretty obvious
that it should do so.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectVersion.cpp
lldb/source/Commands/CommandObjectWatchpoint.cpp
lldb/source/Interpreter/CommandObject.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectPlatform.cpp 
b/lldb/source/Commands/CommandObjectPlatform.cpp
index 16fe05a400fd0..fab1ddc9e99ee 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -820,10 +820,8 @@ class CommandObjectPlatformGetFile : public 
CommandObjectParsed {
   bool DoExecute(Args &args, CommandReturnObject &result) override {
 // If the number of arguments is incorrect, issue an error message.
 if (args.GetArgumentCount() != 2) {
-  result.GetErrorStream().Printf("error: required arguments missing; "
- "specify both the source and destination "
- "file paths\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("required arguments missing; specify both the "
+ "source and destination file paths");
   return false;
 }
 
@@ -894,10 +892,8 @@ class CommandObjectPlatformGetSize : public 
CommandObjectParsed {
   bool DoExecute(Args &args, CommandReturnObject &result) override {
 // If the number of arguments is incorrect, issue an error message.
 if (args.GetArgumentCount() != 1) {
-  result.GetErrorStream().Printf("error: required argument missing; "
- "specify the source file path as the only 
"
- "argument\n");
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError("required argument missing; specify the source file "
+ "path as the only argument");
   return false;
 }
 

diff  --git a/lldb/source/Commands/CommandObjectReproducer.cpp 
b/lldb/source/Commands/CommandObjectReproducer.cpp
index c8b2639344927..50e24d6469aa2 100644
--- a/lldb/source/Commands/CommandObjectReproducer.cpp
+++ b/lldb/source/Commands/CommandObjectReproducer.cpp
@@ -138,9 +138,7 @@ llvm::Expected static ReadFromYAML(StringRef filename) {
 }
 
 static void SetError(CommandReturnObject &result, Error err) {
-  result.GetErrorStream().Printf("error: %s\n",
- toString(std::move(err)).c_str());
-  result.SetStatus(eReturnStatusFailed);
+  result.AppendError(toString(std::move(err)));
 }
 
 /// Create a loader from the given path if specified. Otherwise use the current

diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index e3591e713ea8e..84670707941d1 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -318,7 +318,6 @@ class CommandObjectTargetCreate : public 
CommandObjectParsed {
 
   if (!target_sp) {
 result.AppendError(error.AsCString());
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
 
@@ -342,7 +341,6 @@ class CommandObjectTargetCreate : public 
CommandObjectParsed {
   Status err = platform_sp->PutFile(file_spec, remote_file);
   if (err.Fail()) {
 result.AppendError(err.AsCString());
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
 }
@@ -357,7 +355,6 @@ class CommandObjectTargetCreate : public 
CommandObjectParsed {
   Status err = platform_sp->GetFile(remote_file, file_spec);
   if (err.Fail()) {
 result.AppendError(err.AsCString());
-result.SetStatus(eReturnStatusFailed);
 return false;
   }
 } else {
@@ -851,9 +848,8 @@ class CommandObjectTargetVariable : public 
CommandObjectParsed {
 }
 
 if (matches == 0) {
-  result.GetErrorStream().Printf(
-  "

[Lldb-commits] [PATCH] D104404: Change PathMappingList::RemapPath to return an optional result (NFC)

2021-06-22 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/source/Core/Module.cpp:1604
 
 bool Module::RemapSourceFile(llvm::StringRef path,
  std::string &new_path) const {

shafik wrote:
> Why not also have this return an `Optional`?
That would be perfectly reasonable. I'll add a review for that patch.


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

https://reviews.llvm.org/D104404

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

[Lldb-commits] [PATCH] D104724: Modernize Module::RemapFile to return an Optional

2021-06-22 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl created this revision.
aprantl added reviewers: shafik, JDevlieghere.
aprantl requested review of this revision.

This addresses feedback raised in https://reviews.llvm.org/D104404.


https://reviews.llvm.org/D104724

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -240,9 +240,12 @@
   const size_t number_of_files = prologue.FileNames.size();
   for (size_t idx = first_file; idx <= number_of_files; ++idx) {
 std::string remapped_file;
-if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style))
-  if (!module->RemapSourceFile(llvm::StringRef(*file_path), remapped_file))
+if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style)) {
+  if (auto remapped = module->RemapSourceFile(llvm::StringRef(*file_path)))
+remapped_file = *remapped;
+  else
 remapped_file = std::move(*file_path);
+}
 
 // Unconditionally add an entry, so the indices match up.
 support_files.EmplaceBack(remapped_file, style);
@@ -681,9 +684,8 @@
   // files are NFS mounted.
   file_spec.MakeAbsolute(dwarf_cu.GetCompilationDirectory());
 
-  std::string remapped_file;
-  if (module_sp->RemapSourceFile(file_spec.GetPath(), remapped_file))
-file_spec.SetFile(remapped_file, FileSpec::Style::native);
+  if (auto remapped_file = module_sp->RemapSourceFile(file_spec.GetPath()))
+file_spec.SetFile(*remapped_file, FileSpec::Style::native);
 }
 
 lldb::CompUnitSP SymbolFileDWARF::ParseCompileUnit(DWARFCompileUnit &dwarf_cu) 
{
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1605,14 +1605,11 @@
   return false;
 }
 
-bool Module::RemapSourceFile(llvm::StringRef path,
- std::string &new_path) const {
+llvm::Optional Module::RemapSourceFile(llvm::StringRef path) 
const {
   std::lock_guard guard(m_mutex);
-  if (auto remapped = m_source_mappings.RemapPath(path)) {
-new_path = remapped->GetPath();
-return true;
-  }
-  return false;
+  if (auto remapped = m_source_mappings.RemapPath(path))
+return remapped->GetPath();
+  return {};
 }
 
 void Module::RegisterXcodeSDK(llvm::StringRef sdk_name, llvm::StringRef 
sysroot) {
Index: lldb/include/lldb/Core/Module.h
===
--- lldb/include/lldb/Core/Module.h
+++ lldb/include/lldb/Core/Module.h
@@ -850,13 +850,10 @@
   /// \param[in] path
   /// The original source file path to try and remap.
   ///
-  /// \param[out] new_path
-  /// The newly remapped filespec that is may or may not exist.
-  ///
   /// \return
-  /// /b true if \a path was successfully located and \a new_path
-  /// is filled in with a new source path, \b false otherwise.
-  bool RemapSourceFile(llvm::StringRef path, std::string &new_path) const;
+  /// The newly remapped filespec that is may or may not exist if
+  /// \a path was successfully located.
+  llvm::Optional RemapSourceFile(llvm::StringRef path) const;
   bool RemapSourceFile(const char *, std::string &) const = delete;
 
   /// Update the ArchSpec to a more specific variant.


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -240,9 +240,12 @@
   const size_t number_of_files = prologue.FileNames.size();
   for (size_t idx = first_file; idx <= number_of_files; ++idx) {
 std::string remapped_file;
-if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style))
-  if (!module->RemapSourceFile(llvm::StringRef(*file_path), remapped_file))
+if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style)) {
+  if (auto remapped = module->RemapSourceFile(llvm::StringRef(*file_path)))
+remapped_file = *remapped;
+  else
 remapped_file = std::move(*file_path);
+}
 
 // Unconditionally add an entry, so the indices match up.
 support_files.EmplaceBack(remapped_file, style);
@@ -681,9 +684,8 @@
   // files are NFS mounted.
   file_spec.MakeAbsolute(dwarf_cu.GetCompilationDirectory());
 
-  std::string remapped_file;
-  if (module_sp->RemapSourceFile(file_spec.GetPath(), remapped_file))
-file_spec.SetFile(remapped_file, FileSpec::Style::native);
+  if (auto remapped_file = module_sp->RemapSourceFile(file_spec.GetPath()))
+file_spec.SetFile(*remapped_file, FileSpec::Style::native);
 }
 
 lldb::CompUnitSP SymbolFileDWARF::ParseCompileUnit(DWARFCompil

[Lldb-commits] [PATCH] D104405: Change PathMappingList::FindFile to return an optional result (NFC)

2021-06-22 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl updated this revision to Diff 353700.
aprantl added a comment.

Update to the correct patch


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

https://reviews.llvm.org/D104405

Files:
  lldb/include/lldb/Target/PathMappingList.h
  lldb/source/Core/Module.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Symbol/LineEntry.cpp
  lldb/source/Target/PathMappingList.cpp

Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -193,15 +193,16 @@
   return false;
 }
 
-bool PathMappingList::FindFile(const FileSpec &orig_spec,
-   FileSpec &new_spec) const {
+llvm::Optional
+PathMappingList::FindFile(const FileSpec &orig_spec) const {
+  FileSpec new_spec;
   if (m_pairs.empty())
-return false;
+return {};
   
   std::string orig_path = orig_spec.GetPath();
 
   if (orig_path.empty())
-return false;
+return {};
   
   bool orig_is_relative = orig_spec.IsRelative();
 
@@ -230,12 +231,12 @@
   new_spec.SetFile(entry.second.GetCString(), FileSpec::Style::native);
   new_spec.AppendPathComponent(orig_ref);
   if (FileSystem::Instance().Exists(new_spec))
-return true;
+return new_spec;
 }
   }
   
   new_spec.Clear();
-  return false;
+  return {};
 }
 
 bool PathMappingList::Replace(ConstString path,
Index: lldb/source/Symbol/LineEntry.cpp
===
--- lldb/source/Symbol/LineEntry.cpp
+++ lldb/source/Symbol/LineEntry.cpp
@@ -252,9 +252,8 @@
 
 void LineEntry::ApplyFileMappings(lldb::TargetSP target_sp) {
   if (target_sp) {
-// Apply any file remappings to our file
-FileSpec new_file_spec;
-if (target_sp->GetSourcePathMap().FindFile(original_file, new_file_spec))
-  file = new_file_spec;
+// Apply any file remappings to our file.
+if (auto new_file_spec = target_sp->GetSourcePathMap().FindFile(original_file))
+  file = *new_file_spec;
   }
 }
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -441,13 +441,17 @@
   }
   // Try remapping if m_file_spec does not correspond to an existing file.
   if (!FileSystem::Instance().Exists(m_file_spec)) {
-FileSpec new_file_spec;
 // Check target specific source remappings first, then fall back to
 // modules objects can have individual path remappings that were
 // detected when the debug info for a module was found. then
-if (target->GetSourcePathMap().FindFile(m_file_spec, new_file_spec) ||
-target->GetImages().FindSourceFile(m_file_spec, new_file_spec)) {
-  m_file_spec = new_file_spec;
+auto remapped = target->GetSourcePathMap().FindFile(m_file_spec);
+if (!remapped) {
+  FileSpec new_spec;
+  if (target->GetImages().FindSourceFile(m_file_spec, new_spec))
+remapped = new_spec;
+}
+if (remapped) {
+  m_file_spec = *remapped;
   m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
 }
   }
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1598,7 +1598,11 @@
 bool Module::FindSourceFile(const FileSpec &orig_spec,
 FileSpec &new_spec) const {
   std::lock_guard guard(m_mutex);
-  return m_source_mappings.FindFile(orig_spec, new_spec);
+  if (auto remapped = m_source_mappings.FindFile(orig_spec)) {
+new_spec = *remapped;
+return true;
+  }
+  return false;
 }
 
 bool Module::RemapSourceFile(llvm::StringRef path,
Index: lldb/include/lldb/Target/PathMappingList.h
===
--- lldb/include/lldb/Target/PathMappingList.h
+++ lldb/include/lldb/Target/PathMappingList.h
@@ -90,14 +90,9 @@
   /// \param[in] orig_spec
   /// The original source file path to try and remap.
   ///
-  /// \param[out] new_spec
-  /// The newly remapped filespec that is guaranteed to exist.
-  ///
   /// \return
-  /// /b true if \a orig_spec was successfully located and
-  /// \a new_spec is filled in with an existing file spec,
-  /// \b false otherwise.
-  bool FindFile(const FileSpec &orig_spec, FileSpec &new_spec) const;
+  /// The newly remapped filespec that is guaranteed to exist.
+  llvm::Optional FindFile(const FileSpec &orig_spec) const;
 
   uint32_t FindIndexForPath(ConstString path) const;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-22 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

Some comments on comments...




Comment at: lldb/include/lldb/Symbol/Symtab.h:224
+  ///
+  /// We started not always generating symbol names for synthetic symbols and
+  /// we want to be able to lookup synthetic symbols by name when needed. This

This comment is hard to read.  I think it's mostly because you describe the 
implementation before the reason for it.

Maybe this would be clearer like:

"We generate unique names for synthetic symbols so that users can look them up 
by name when needed.  But because doing so is uncommon in normal debugger use, 
we trade off some performance at lookup time for faster symbol table building 
by detecting these symbols and generating their names lazily, rather than 
adding them to the normal symbol indexes.  This function does the job of first 
consulting the indexes, and if that fails checking whether the symbol has the 
synthetic symbol prefix and generating the correct synthetic name if it does.





Comment at: lldb/source/Symbol/Symbol.cpp:575
+// identify individual symbols so we give them a unique name. The name
+// starts with the synthetic symbol prefix and are followed by a unique
+// number. Typically the UserID is a real symbol is the symbol table

are -> is

or maybe:

starts with the synthetic symbol prefix, followed by a unique number



Comment at: lldb/source/Symbol/Symbol.cpp:576
+// starts with the synthetic symbol prefix and are followed by a unique
+// number. Typically the UserID is a real symbol is the symbol table
+// index of the symbol in the object file's symbol table(s). Synthetic

is -> of so this reads:

Typically the UserID of a real symbol is ...



Comment at: lldb/source/Symbol/Symbol.cpp:578
+// index of the symbol in the object file's symbol table(s). Synthetic
+// UserID values tend to be numbers that are higher than the last symbol
+// index in the symbol table. The synthetic name for the same synthetic

I don't think you need the implementation detail here, you are stating policy.  
Starting from "Typically" I think something like the following is more direct:

Typically the UserID of a real symbol is the symbol table index of the symbol 
in the object file's symbol table(s), so it will be the same every time you 
read in the object file.  We want the same persistence for synthetic symbols so 
that users can identify them across multiple debug sessions, to understand 
crashes in those symbols and to reliably set breakpoints on them.



Comment at: lldb/source/Symbol/Symtab.cpp:628
+  // Synthetic symbol names are not added to the name indexes, but they start
+  // with a prefix and end with a the symbol UserID, so allow users to find
+  // these without having to add them to the name indexes. These queries will

so -> to


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104488

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

[Lldb-commits] [lldb] 709f818 - [lldb] Add missing string include to lldb-server's main

2021-06-22 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2021-06-22T19:49:10+02:00
New Revision: 709f8186a45e28c0640c999a8b779433dc0eb525

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

LOG: [lldb] Add missing string include to lldb-server's main

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 f113304cfd628..c02c991300fc4 100644
--- a/lldb/test/API/tools/lldb-server/main.cpp
+++ b/lldb/test/API/tools/lldb-server/main.cpp
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 



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

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-22 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg updated this revision to Diff 353715.
clayborg added a comment.

Fixed comments per Jim Ingham's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104488

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Symbol/Symbol.h
  lldb/include/lldb/Symbol/Symtab.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Symbol/ObjectFile.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Symbol/Symtab.cpp

Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -295,7 +295,7 @@
   // the trampoline symbols to be searchable by name we can remove this and
   // then possibly add a new bool to any of the Symtab functions that
   // lookup symbols by name to indicate if they want trampolines.
-  if (symbol->IsTrampoline())
+  if (symbol->IsTrampoline() || symbol->IsSynthetic())
 continue;
 
   // If the symbol's name string matched a Mangled::ManglingScheme, it is
@@ -618,6 +618,36 @@
   }
 }
 
+uint32_t Symtab::GetNameIndexes(ConstString symbol_name,
+std::vector &indexes) {
+  auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
+  const uint32_t count = name_to_index.GetValues(symbol_name, indexes);
+  if (count)
+return count;
+  // Synthetic symbol names are not added to the name indexes, but they start
+  // with a prefix and end with a the symbol UserID. This allows users to find
+  // these symbols without having to add them to the name indexes. These
+  // queries will not happen very often since the names don't mean anything, so
+  // performance is not paramount in this case.
+  llvm::StringRef name = symbol_name.GetStringRef();
+  // String the synthetic prefix if the name starts with it.
+  if (!name.consume_front(Symbol::GetSyntheticSymbolPrefix()))
+return 0; // Not a synthetic symbol name
+
+  // Extract the user ID from the symbol name
+  user_id_t uid = 0;
+  if (getAsUnsignedInteger(name, /*Radix=*/10, uid))
+return 0; // Failed to extract the user ID as an integer
+  Symbol *symbol = FindSymbolByID(uid);
+  if (symbol == nullptr)
+return 0;
+  const uint32_t symbol_idx = GetIndexForSymbol(symbol);
+  if (symbol_idx == UINT32_MAX)
+return 0;
+  indexes.push_back(symbol_idx);
+  return 1;
+}
+
 uint32_t Symtab::AppendSymbolIndexesWithName(ConstString symbol_name,
  std::vector &indexes) {
   std::lock_guard guard(m_mutex);
@@ -627,8 +657,7 @@
 if (!m_name_indexes_computed)
   InitNameIndexes();
 
-auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
-return name_to_index.GetValues(symbol_name, indexes);
+return GetNameIndexes(symbol_name, indexes);
   }
   return 0;
 }
@@ -645,10 +674,9 @@
 if (!m_name_indexes_computed)
   InitNameIndexes();
 
-auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
 std::vector all_name_indexes;
 const size_t name_match_count =
-name_to_index.GetValues(symbol_name, all_name_indexes);
+GetNameIndexes(symbol_name, all_name_indexes);
 for (size_t i = 0; i < name_match_count; ++i) {
   if (CheckSymbolAtIndex(all_name_indexes[i], symbol_debug_type,
  symbol_visibility))
Index: lldb/source/Symbol/Symbol.cpp
===
--- lldb/source/Symbol/Symbol.cpp
+++ lldb/source/Symbol/Symbol.cpp
@@ -56,8 +56,8 @@
   m_size_is_synthesized(false),
   m_size_is_valid(size_is_valid || range.GetByteSize() > 0),
   m_demangled_is_synthesized(false),
-  m_contains_linker_annotations(contains_linker_annotations), 
-  m_is_weak(false), m_type(type), m_mangled(mangled), m_addr_range(range), 
+  m_contains_linker_annotations(contains_linker_annotations),
+  m_is_weak(false), m_type(type), m_mangled(mangled), m_addr_range(range),
   m_flags(flags) {}
 
 Symbol::Symbol(const Symbol &rhs)
@@ -119,7 +119,7 @@
 }
 
 ConstString Symbol::GetDisplayName() const {
-  return m_mangled.GetDisplayDemangledName();
+  return GetMangled().GetDisplayDemangledName();
 }
 
 ConstString Symbol::GetReExportedSymbolName() const {
@@ -202,7 +202,7 @@
   s->Printf(", value = 0x%16.16" PRIx64,
 m_addr_range.GetBaseAddress().GetOffset());
   }
-  ConstString demangled = m_mangled.GetDemangledName();
+  ConstString demangled = GetMangled().GetDemangledName();
   if (demangled)
 s->Printf(", name=\"%s\"", demangled.AsCString());
   if (m_mangled.GetMangledName())
@@ -218,7 +218,7 @@
   // Make sure the size of the symbol is up to date before dumping
   GetByteSize();
 
-  ConstString name = m_mangled.GetName(name_preference

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-22 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104488

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

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-22 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Anyone have any other comments? Or is this good to go?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104488

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

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

2021-06-22 Thread Omar Emara via Phabricator via lldb-commits 
OmarEmaraDev updated this revision to Diff 353783.
OmarEmaraDev added a comment.

- Remove Field type and use FieldDelegate directly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104395

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp

Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -392,6 +392,12 @@
   void Box(chtype v_char = ACS_VLINE, chtype h_char = ACS_HLINE) {
 ::box(m_window, v_char, h_char);
   }
+  void VerticalLine(int n, chtype v_char = ACS_VLINE) {
+::wvline(m_window, v_char, n);
+  }
+  void HorizontalLine(int n, chtype h_char = ACS_HLINE) {
+::whline(m_window, h_char, n);
+  }
   void Clear() { ::wclear(m_window); }
   void Erase() { ::werase(m_window); }
   Rect GetBounds() const {
@@ -674,6 +680,36 @@
   AttributeOff(attr);
   }
 
+  void DrawBox(const Rect &bounds, chtype v_char = ACS_VLINE,
+   chtype h_char = ACS_HLINE) {
+MoveCursor(bounds.origin.x, bounds.origin.y);
+VerticalLine(bounds.size.height);
+HorizontalLine(bounds.size.width);
+PutChar(ACS_ULCORNER);
+
+MoveCursor(bounds.origin.x + bounds.size.width - 1, bounds.origin.y);
+VerticalLine(bounds.size.height);
+PutChar(ACS_URCORNER);
+
+MoveCursor(bounds.origin.x, bounds.origin.y + bounds.size.height - 1);
+HorizontalLine(bounds.size.width);
+PutChar(ACS_LLCORNER);
+
+MoveCursor(bounds.origin.x + bounds.size.width - 1,
+   bounds.origin.y + bounds.size.height - 1);
+PutChar(ACS_LRCORNER);
+  }
+
+  void DrawTitledBox(const Rect &bounds, const char *title,
+ chtype v_char = ACS_VLINE, chtype h_char = ACS_HLINE) {
+DrawBox(bounds, v_char, h_char);
+int title_offset = 2;
+MoveCursor(bounds.origin.x + title_offset, bounds.origin.y);
+PutChar('[');
+PutCString(title, bounds.size.width - title_offset);
+PutChar(']');
+  }
+
   virtual void Draw(bool force) {
 if (m_delegate_sp && m_delegate_sp->WindowDelegateDraw(*this, force))
   return;
@@ -869,6 +905,627 @@
   const Window &operator=(const Window &) = delete;
 };
 
+/
+// Forms
+/
+
+class FieldDelegate {
+public:
+  virtual ~FieldDelegate() = default;
+
+  virtual Rect FieldDelegateGetBounds() = 0;
+
+  virtual void FieldDelegateDraw(Window &window, bool is_active) = 0;
+
+  virtual HandleCharResult FieldDelegateHandleChar(int key) {
+return eKeyNotHandled;
+  }
+
+  void FieldDelegateSetPageIndex(int page_index) { m_page_index = page_index; }
+
+  int FieldDelegateGetPageIndex() { return m_page_index; }
+
+protected:
+  // The index of the page this field belongs to.
+  int m_page_index;
+};
+
+typedef std::shared_ptr FieldDelegateSP;
+
+class TextFieldDelegate : public FieldDelegate {
+public:
+  TextFieldDelegate(const char *label, int width, Point origin,
+const char *content)
+  : m_label(label), m_width(width), m_origin(origin), m_cursor_position(0),
+m_first_visibile_char(0) {
+if (content)
+  m_content = content;
+assert(m_width > 2);
+  }
+
+  // Get the bounding box of the field. The text field has a height of 3, 2
+  // lines for borders and 1 for the content.
+  Rect FieldDelegateGetBounds() override {
+return Rect(m_origin, Size(m_width, 3));
+  }
+
+  // Get the start X position of the content in window space, without the
+  // borders.
+  int GetX() { return m_origin.x + 1; }
+
+  // Get the start Y position of the content in window space, without the
+  // borders.
+  int GetY() { return m_origin.y + 1; }
+
+  // Get the effective width of the field, without the borders.
+  int GetEffectiveWidth() { return m_width - 2; }
+
+  // Get the cursor position in window space.
+  int GetCursorWindowXPosition() {
+return GetX() + m_cursor_position - m_first_visibile_char;
+  }
+
+  int GetContentLength() { return m_content.length(); }
+
+  void FieldDelegateDraw(Window &window, bool is_active) override {
+// Draw label box.
+window.DrawTitledBox(FieldDelegateGetBounds(), m_label.c_str());
+
+// Draw content.
+window.MoveCursor(GetX(), GetY());
+const char *text = m_content.c_str() + m_first_visibile_char;
+window.PutCString(text, GetEffectiveWidth());
+
+// Highlight the cursor.
+window.MoveCursor(GetCursorWindowXPosition(), GetY());
+if (is_active)
+  window.AttributeOn(A_REVERSE);
+if (m_cursor_position == GetContentLength())
+  // Cursor is past the last character. Highlight an empty space.
+  window.PutChar(' ');
+else
+  window.PutChar(m_content[m_cursor_position]);
+if (is_active)
+  window.AttributeOff(A_REVERSE);
+  }
+
+  // The cursor is allowed to move one character past the string.
+  // m_cursor_position is in range [0, GetContentLength()].
+  v

[Lldb-commits] [PATCH] D104404: Change PathMappingList::RemapPath to return an optional result (NFC)

2021-06-22 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

Patch for `Module` is in https://reviews.llvm.org/D104724.


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

https://reviews.llvm.org/D104404

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

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

2021-06-22 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Just a few questions on how buttons should be handled since we are making new 
sweeping changes! See inlined comments and let me know what you think




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1396-1405
+  // 
+  // | |
+  // | |
+  // | Form elements here. |
+  // | |
+  // |...  |
+  // |-|

Should we put a cancel button and "Submit" button in the window's bottom line 
to save space? Any errors could be popped up with a modal dialog when the user 
tries to submit. The help for a form could specify that "Esc" will cancel the 
dialog and we wouldn't have to write it into the bottom of the window.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1438
+
+window.DrawTitleBox(window.GetName(), "Press Esc to cancel");
+

As mentioned above, it might be cleaner to put the "Cancel" and "Submit" into 
the bottom line of the dialog box to save space?



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1450
+// Draw the centered submit button.
+const char *button_text = "[Submit]";
+int x = (window.GetWidth() - sizeof(button_text) - 1) / 2;

Maybe we want to add a ButtonDelegate class and each window can add a list of 
button delegates. Then the window draw code would know to draw them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104395

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

[Lldb-commits] [PATCH] D102993: [lldb] Disable minimal import mode for RecordDecls that back FieldDecls

2021-06-22 Thread Jordan Rupprecht via Phabricator via lldb-commits 
rupprecht added a comment.

In D102993#2811902 , @rupprecht wrote:

> In D102993#2811337 , @teemperor 
> wrote:
>
>> Sure I can take a look, but I don't see the immediate problem when looking 
>> at the backtrace.
>>
>> I assume it takes some time to reduce the bug in Chrome? If you could get me 
>> the `dump()` output for the Decls `D`, `(Decl*)ToDC`, `FromRD`, `ToD`, 
>> `(Decl*)ToD->getLexicalDeclContext()` and whether the left or the right side 
>> of the `&&` is true/false then maybe it becomes more obvious what's going 
>> wrong here.
>
> This is probably not going to be useful:
>
> - `D->dump()`: `FieldDecl 0x169993f9fbd0 <>  __r_ 
> 'std::__compressed_pair, 
> std::allocator >::__rep, std::allocator >'`
> - `((Decl*)ToDC)->dump()` - prints:
>
>   a decl that inherits DeclContext isn't handled
>   UNREACHABLE executed at llvm-project/clang/lib/AST/DeclBase.cpp:928!

`ToDC->getDeclKindName()` is "ClassTemplateSpecialization". Does that explain 
the unreachable error message? Maybe it needs a special case somewhere?

> - `FromRD->dump()` prints something wy to long to include here :)
>
>   ClassTemplateSpecializationDecl 0x169983fc2b98 <>  sloc> class basic_string definition
>   |-DefinitionData standard_layout has_user_declared_ctor 
> can_const_default_init
>   | |-DefaultConstructor exists non_trivial user_provided
>   | |-CopyConstructor non_trivial user_declared has_const_param 
> needs_overload_resolution implicit_has_const_param
>   | |-MoveConstructor exists non_trivial user_declared 
>   | |-CopyAssignment non_trivial has_const_param user_declared 
> needs_overload_resolution implicit_has_const_param
>   | |-MoveAssignment exists non_trivial user_declared
>   | `-Destructor non_trivial user_declared
>   |-private 'std::__basic_string_common'
>   |-TemplateArgument type 'char'
>   | `-BuiltinType 0x1695c0305860 'char'
>   ...
>
>
>
> - `((Decl*)ToD)->getLexicalDeclContext()` errors: `error: Execution was 
> interrupted, reason: signal SIGSEGV: address access protected (fault address: 
> 0x55e26400).` (ToD is nonnull: 0x1699897a5e00)
>
> I'm running out of time today & I'm off tomorrow, but I should be able to 
> post something on Monday. I have a suspicion it's dependent on whatever flags 
> chrome is building with, so I'll try to figure that out too.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102993

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