date:20210629

[Lldb-commits] [lldb] fe1874d - [lldb/Interpreter] Add setting to set session transcript save directory

2021-06-29 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2021-06-29T10:54:29+02:00
New Revision: fe1874dd2dd99c9811db515a2957e2a42f9f6868

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

LOG: [lldb/Interpreter] Add setting to set session transcript save directory

This patch introduces a new interpreter setting
`interpreter.save-session-directory` so the user can specify a directory
where the session transcripts will be saved.

If not set, the session transcript are saved on a temporary file.

rdar://72902842

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/InterpreterProperties.td
lldb/test/API/commands/session/save/TestSessionSave.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index a8475ca610463..6430773de1b64 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -493,6 +493,9 @@ class CommandInterpreter : public Broadcaster,
   bool GetSaveSessionOnQuit() const;
   void SetSaveSessionOnQuit(bool enable);
 
+  FileSpec GetSaveSessionDirectory() const;
+  void SetSaveSessionDirectory(llvm::StringRef path);
+
   bool GetEchoCommands() const;
   void SetEchoCommands(bool enable);
 

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 2e07ff5703ff2..68e8edfc90583 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -160,6 +160,16 @@ void CommandInterpreter::SetSaveSessionOnQuit(bool enable) 
{
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, enable);
 }
 
+FileSpec CommandInterpreter::GetSaveSessionDirectory() const {
+  const uint32_t idx = ePropertySaveSessionDirectory;
+  return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr, idx);
+}
+
+void CommandInterpreter::SetSaveSessionDirectory(llvm::StringRef path) {
+  const uint32_t idx = ePropertySaveSessionDirectory;
+  m_collection_sp->SetPropertyAtIndexAsString(nullptr, idx, path);
+}
+
 bool CommandInterpreter::GetEchoCommands() const {
   const uint32_t idx = ePropertyEchoCommands;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
@@ -2925,9 +2935,15 @@ bool CommandInterpreter::SaveTranscript(
 std::string now = llvm::to_string(std::chrono::system_clock::now());
 std::replace(now.begin(), now.end(), ' ', '_');
 const std::string file_name = "lldb_session_" + now + ".log";
-FileSpec tmp = HostInfo::GetGlobalTempDir();
-tmp.AppendPathComponent(file_name);
-output_file = tmp.GetPath();
+
+FileSpec save_location = GetSaveSessionDirectory();
+
+if (!save_location)
+  save_location = HostInfo::GetGlobalTempDir();
+
+FileSystem::Instance().Resolve(save_location);
+save_location.AppendPathComponent(file_name);
+output_file = save_location.GetPath();
   }
 
   auto error_out = [&](llvm::StringRef error_message, std::string description) 
{

diff  --git a/lldb/source/Interpreter/InterpreterProperties.td 
b/lldb/source/Interpreter/InterpreterProperties.td
index 1148c1b01def5..1c6f0206c489d 100644
--- a/lldb/source/Interpreter/InterpreterProperties.td
+++ b/lldb/source/Interpreter/InterpreterProperties.td
@@ -13,6 +13,9 @@ let Definition = "interpreter" in {
 Global,
 DefaultFalse,
 Desc<"If true, LLDB will save the session's transcripts before quitting.">;
+  def SaveSessionDirectory: Property<"save-session-directory", "FileSpec">,
+DefaultStringValue<"">,
+Desc<"A path where LLDB will save the session's transcripts. This is 
particularly useful when you can't set the session file, for example when using 
`save-session-on-quit`.">;
   def StopCmdSourceOnError: Property<"stop-command-source-on-error", 
"Boolean">,
 Global,
 DefaultTrue,

diff  --git a/lldb/test/API/commands/session/save/TestSessionSave.py 
b/lldb/test/API/commands/session/save/TestSessionSave.py
index ec244a42efcac..e144ed19d1c50 100644
--- a/lldb/test/API/commands/session/save/TestSessionSave.py
+++ b/lldb/test/API/commands/session/save/TestSessionSave.py
@@ -1,7 +1,7 @@
 """
 Test the session save feature
 """
-
+import os
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -72,3 +72,20 @@ def test_session_save(self):
   lines = raw.splitlines()[:-1]
   for line in lines:
 self.assertIn(line, content)
+
+td = tempfile.TemporaryDirectory()
+res = lldb.SBCommandReturnObject()
+interpreter.HandleCommand('settings set 
inte

[Lldb-commits] [lldb] d6b6461 - [lldb/Interpreter] Fix session-save-on-quit when using ^D

2021-06-29 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2021-06-29T10:54:29+02:00
New Revision: d6b64612bd92cda8b53ef348fb578983124c600f

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

LOG: [lldb/Interpreter] Fix session-save-on-quit when using ^D

Previously, when `interpreter.save-session-on-quit` was enabled, lldb
would save the session transcript only when running the `quit` command.

This patch changes that so the transcripts are saved when the debugger
object is destroyed if the setting is enabled.

rdar://72902650

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Commands/CommandObjectQuit.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/test/API/commands/session/save/TestSessionSave.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectQuit.cpp 
b/lldb/source/Commands/CommandObjectQuit.cpp
index e4de347870753..6ac04290f603f 100644
--- a/lldb/source/Commands/CommandObjectQuit.cpp
+++ b/lldb/source/Commands/CommandObjectQuit.cpp
@@ -101,8 +101,5 @@ bool CommandObjectQuit::DoExecute(Args &command, 
CommandReturnObject &result) {
   m_interpreter.BroadcastEvent(event_type);
   result.SetStatus(eReturnStatusQuit);
 
-  if (m_interpreter.GetSaveSessionOnQuit())
-m_interpreter.SaveTranscript(result);
-
   return true;
 }

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 735e6f43ac69f..12210ed541bc1 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Host/Terminal.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/OptionValueSInt64.h"
@@ -604,6 +605,17 @@ void Debugger::Destroy(DebuggerSP &debugger_sp) {
   if (!debugger_sp)
 return;
 
+  CommandInterpreter &cmd_interpreter = debugger_sp->GetCommandInterpreter();
+
+  if (cmd_interpreter.GetSaveSessionOnQuit()) {
+CommandReturnObject result(debugger_sp->GetUseColor());
+cmd_interpreter.SaveTranscript(result);
+if (result.Succeeded())
+  debugger_sp->GetOutputStream() << result.GetOutputData() << '\n';
+else
+  debugger_sp->GetErrorStream() << result.GetErrorData() << '\n';
+  }
+
   debugger_sp->Clear();
 
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index 68e8edfc90583..00e9ccb762c32 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2974,6 +2974,7 @@ bool CommandInterpreter::SaveTranscript(
 return error_out("Unable to write to destination file",
  "Bytes written do not match transcript size.");
 
+  result.SetStatus(eReturnStatusSuccessFinishNoResult);
   result.AppendMessageWithFormat("Session's transcripts saved to %s\n",
  output_file->c_str());
 

diff  --git a/lldb/test/API/commands/session/save/TestSessionSave.py 
b/lldb/test/API/commands/session/save/TestSessionSave.py
index e144ed19d1c50..2e25047e501a7 100644
--- a/lldb/test/API/commands/session/save/TestSessionSave.py
+++ b/lldb/test/API/commands/session/save/TestSessionSave.py
@@ -2,6 +2,8 @@
 Test the session save feature
 """
 import os
+import tempfile
+
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -57,7 +59,6 @@ def test_session_save(self):
 self.assertFalse(res.Succeeded())
 raw += self.raw_transcript_builder(cmd, res)
 
-import tempfile
 tf = tempfile.NamedTemporaryFile()
 output_file = tf.name
 
@@ -89,3 +90,37 @@ def test_session_save(self):
   lines = raw.splitlines()[:-1]
   for line in lines:
 self.assertIn(line, content)
+
+@skipIfWindows
+@skipIfReproducer
+@no_debug_info_test
+def test_session_save_on_quit(self):
+raw = ""
+interpreter = self.dbg.GetCommandInterpreter()
+
+td = tempfile.TemporaryDirectory()
+
+settings = [
+  'settings set interpreter.echo-commands true',
+  'settings set interpreter.echo-comment-commands true',
+  'settings set interpreter.stop-command-source-on-error false',
+  'settings set interpreter.save-session-on-quit true',
+  'settings set interpreter.save-session-directory ' + td.name,
+]
+
+for setting in settings:
+  res = lldb.SBCommandReturnObject()
+  interpreter.HandleCommand(setting, res)

[Lldb-commits] [PATCH] D105030: [lldb/Interpreter] Add setting to set session transcript save directory

2021-06-29 Thread Med Ismail Bennani 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 rGfe1874dd2dd9: [lldb/Interpreter] Add setting to set session 
transcript save directory (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105030

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/InterpreterProperties.td
  lldb/test/API/commands/session/save/TestSessionSave.py

Index: lldb/test/API/commands/session/save/TestSessionSave.py
===
--- lldb/test/API/commands/session/save/TestSessionSave.py
+++ lldb/test/API/commands/session/save/TestSessionSave.py
@@ -1,7 +1,7 @@
 """
 Test the session save feature
 """
-
+import os
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -72,3 +72,20 @@
   lines = raw.splitlines()[:-1]
   for line in lines:
 self.assertIn(line, content)
+
+td = tempfile.TemporaryDirectory()
+res = lldb.SBCommandReturnObject()
+interpreter.HandleCommand('settings set interpreter.save-session-directory ' + td.name, res)
+self.assertTrue(res.Succeeded())
+
+res = lldb.SBCommandReturnObject()
+interpreter.HandleCommand('session save', res)
+self.assertTrue(res.Succeeded())
+raw += self.raw_transcript_builder(cmd, res)
+
+with open(os.path.join(td.name, os.listdir(td.name)[0]), "r") as file:
+  content = file.read()
+  # Exclude last line, since session won't record it's own output
+  lines = raw.splitlines()[:-1]
+  for line in lines:
+self.assertIn(line, content)
Index: lldb/source/Interpreter/InterpreterProperties.td
===
--- lldb/source/Interpreter/InterpreterProperties.td
+++ lldb/source/Interpreter/InterpreterProperties.td
@@ -13,6 +13,9 @@
 Global,
 DefaultFalse,
 Desc<"If true, LLDB will save the session's transcripts before quitting.">;
+  def SaveSessionDirectory: Property<"save-session-directory", "FileSpec">,
+DefaultStringValue<"">,
+Desc<"A path where LLDB will save the session's transcripts. This is particularly useful when you can't set the session file, for example when using `save-session-on-quit`.">;
   def StopCmdSourceOnError: Property<"stop-command-source-on-error", "Boolean">,
 Global,
 DefaultTrue,
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -160,6 +160,16 @@
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, enable);
 }
 
+FileSpec CommandInterpreter::GetSaveSessionDirectory() const {
+  const uint32_t idx = ePropertySaveSessionDirectory;
+  return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr, idx);
+}
+
+void CommandInterpreter::SetSaveSessionDirectory(llvm::StringRef path) {
+  const uint32_t idx = ePropertySaveSessionDirectory;
+  m_collection_sp->SetPropertyAtIndexAsString(nullptr, idx, path);
+}
+
 bool CommandInterpreter::GetEchoCommands() const {
   const uint32_t idx = ePropertyEchoCommands;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
@@ -2925,9 +2935,15 @@
 std::string now = llvm::to_string(std::chrono::system_clock::now());
 std::replace(now.begin(), now.end(), ' ', '_');
 const std::string file_name = "lldb_session_" + now + ".log";
-FileSpec tmp = HostInfo::GetGlobalTempDir();
-tmp.AppendPathComponent(file_name);
-output_file = tmp.GetPath();
+
+FileSpec save_location = GetSaveSessionDirectory();
+
+if (!save_location)
+  save_location = HostInfo::GetGlobalTempDir();
+
+FileSystem::Instance().Resolve(save_location);
+save_location.AppendPathComponent(file_name);
+output_file = save_location.GetPath();
   }
 
   auto error_out = [&](llvm::StringRef error_message, std::string description) {
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -493,6 +493,9 @@
   bool GetSaveSessionOnQuit() const;
   void SetSaveSessionOnQuit(bool enable);
 
+  FileSpec GetSaveSessionDirectory() const;
+  void SetSaveSessionDirectory(llvm::StringRef path);
+
   bool GetEchoCommands() const;
   void SetEchoCommands(bool enable);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105038: [lldb/Interpreter] Fix session-save-on-quit when using ^D

2021-06-29 Thread Med Ismail Bennani 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 rGd6b64612bd92: [lldb/Interpreter] Fix session-save-on-quit 
when using ^D (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105038

Files:
  lldb/source/Commands/CommandObjectQuit.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/test/API/commands/session/save/TestSessionSave.py

Index: lldb/test/API/commands/session/save/TestSessionSave.py
===
--- lldb/test/API/commands/session/save/TestSessionSave.py
+++ lldb/test/API/commands/session/save/TestSessionSave.py
@@ -2,6 +2,8 @@
 Test the session save feature
 """
 import os
+import tempfile
+
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -57,7 +59,6 @@
 self.assertFalse(res.Succeeded())
 raw += self.raw_transcript_builder(cmd, res)
 
-import tempfile
 tf = tempfile.NamedTemporaryFile()
 output_file = tf.name
 
@@ -89,3 +90,37 @@
   lines = raw.splitlines()[:-1]
   for line in lines:
 self.assertIn(line, content)
+
+@skipIfWindows
+@skipIfReproducer
+@no_debug_info_test
+def test_session_save_on_quit(self):
+raw = ""
+interpreter = self.dbg.GetCommandInterpreter()
+
+td = tempfile.TemporaryDirectory()
+
+settings = [
+  'settings set interpreter.echo-commands true',
+  'settings set interpreter.echo-comment-commands true',
+  'settings set interpreter.stop-command-source-on-error false',
+  'settings set interpreter.save-session-on-quit true',
+  'settings set interpreter.save-session-directory ' + td.name,
+]
+
+for setting in settings:
+  res = lldb.SBCommandReturnObject()
+  interpreter.HandleCommand(setting, res)
+  raw += self.raw_transcript_builder(setting, res)
+
+self.dbg.Destroy(self.dbg)
+
+with open(os.path.join(td.name, os.listdir(td.name)[0]), "r") as file:
+  content = file.read()
+  # Exclude last line, since session won't record it's own output
+  lines = raw.splitlines()[:-1]
+  for line in lines:
+self.assertIn(line, content)
+
+
+
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2974,6 +2974,7 @@
 return error_out("Unable to write to destination file",
  "Bytes written do not match transcript size.");
 
+  result.SetStatus(eReturnStatusSuccessFinishNoResult);
   result.AppendMessageWithFormat("Session's transcripts saved to %s\n",
  output_file->c_str());
 
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Host/Terminal.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/OptionValueSInt64.h"
@@ -604,6 +605,17 @@
   if (!debugger_sp)
 return;
 
+  CommandInterpreter &cmd_interpreter = debugger_sp->GetCommandInterpreter();
+
+  if (cmd_interpreter.GetSaveSessionOnQuit()) {
+CommandReturnObject result(debugger_sp->GetUseColor());
+cmd_interpreter.SaveTranscript(result);
+if (result.Succeeded())
+  debugger_sp->GetOutputStream() << result.GetOutputData() << '\n';
+else
+  debugger_sp->GetErrorStream() << result.GetErrorData() << '\n';
+  }
+
   debugger_sp->Clear();
 
   if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
Index: lldb/source/Commands/CommandObjectQuit.cpp
===
--- lldb/source/Commands/CommandObjectQuit.cpp
+++ lldb/source/Commands/CommandObjectQuit.cpp
@@ -101,8 +101,5 @@
   m_interpreter.BroadcastEvent(event_type);
   result.SetStatus(eReturnStatusQuit);
 
-  if (m_interpreter.GetSaveSessionOnQuit())
-m_interpreter.SaveTranscript(result);
-
   return true;
 }
___
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-29 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

omjavaid added a comment.

This rev has caused multiple test failure on LLDB Arm/AArch64 buildbots.

https://lab.llvm.org/buildbot/#/builders/17/builds/8504
https://lab.llvm.org/buildbot/#/builders/96/builds/9110
https://lab.llvm.org/buildbot/#/builders/96/builds/9111


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-29 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

This broke the windows lldb bot and the follow up changes did not fix it:

https://lab.llvm.org/buildbot/#/builders/83/builds/7748

Can you have a look?


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] c000323 - [lldb] Skip TestPairFromStdModule for now

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


Author: Raphael Isemann
Date: 2021-06-29T19:01:11+02:00
New Revision: c00032321a6ae26f9c8056d024e262abf342631e

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

LOG: [lldb] Skip TestPairFromStdModule for now

I didn't get around to fix this change and the original commit itself seems
fine, so this looks like an existing LLDB/Clang bug that was just uncovered
by this change. Skipping while I'm investigating.

Added: 


Modified: 

lldb/test/API/commands/expression/import-std-module/pair/TestPairFromStdModule.py

Removed: 




diff  --git 
a/lldb/test/API/commands/expression/import-std-module/pair/TestPairFromStdModule.py
 
b/lldb/test/API/commands/expression/import-std-module/pair/TestPairFromStdModule.py
index d0449a4e7800d..121b6e7420349 100644
--- 
a/lldb/test/API/commands/expression/import-std-module/pair/TestPairFromStdModule.py
+++ 
b/lldb/test/API/commands/expression/import-std-module/pair/TestPairFromStdModule.py
@@ -13,6 +13,10 @@ class TestCase(TestBase):
 
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
+# FIXME: This regressed in 69d5a6662115499198ebfa07a081e98a6ce4b915
+# but needs further investigation for what underlying Clang/LLDB bug can't
+# handle that code change.
+@skipIf
 def test(self):
 self.build()
 



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

[Lldb-commits] [PATCH] D105133: [lldb] Fix Recognizer/assert.test with glibc-2.33.9000-31.fc35.x86_64

2021-06-29 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil created this revision.
jankratochvil added reviewers: teemperor, mib.
jankratochvil added a project: LLDB.
Herald added a subscriber: JDevlieghere.
jankratochvil requested review of this revision.

While on regular Linux system (Fedora 34 GA, not updated):

  * thread #1, name = '1', stop reason = hit program assert
  frame #0: 0x77e242a2 libc.so.6`raise + 322
  frame #1: 0x77e0d8a4 libc.so.6`abort + 278
  frame #2: 0x77e0d789 libc.so.6`__assert_fail_base.cold + 15
  frame #3: 0x77e1ca16 libc.so.6`__assert_fail + 70
* frame #4: 0x004011bd 1`main at assert.c:7:3

On Fedora 35 pre-release one gets:

  * thread #1, name = '1', stop reason = signal SIGABRT
* frame #0: 0x77e48ee3 libc.so.6`pthread_kill@GLIBC_2.2.5 + 67
  frame #1: 0x77dfb986 libc.so.6`raise + 22
  frame #2: 0x77de5806 libc.so.6`abort + 230
  frame #3: 0x77de571b libc.so.6`__assert_fail_base.cold + 15
  frame #4: 0x77df4646 libc.so.6`__assert_fail + 70
  frame #5: 0x004011bd 1`main at assert.c:7:3

The question is whether there should be included also a variant without the 
symbol versioning (`@GLIBC_2.2.5`). I heard some distros (=non-Fedora) may not 
use it but I am not sure with it. They can fix it after they have the newer 
glibc themselves. BTW this is a version when ABI of the function was changed so 
it will not change in the future (unless the ABI changes again but then its 
handling should be reevaluated anyway).
I did not write a testcase as one needs the specific glibc. An artificial test 
would just copy the changed source.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105133

Files:
  lldb/source/Target/AssertFrameRecognizer.cpp


Index: lldb/source/Target/AssertFrameRecognizer.cpp
===
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -45,6 +45,7 @@
 location.symbols.push_back(ConstString("raise"));
 location.symbols.push_back(ConstString("__GI_raise"));
 location.symbols.push_back(ConstString("gsignal"));
+location.symbols.push_back(ConstString("pthread_kill@GLIBC_2.2.5"));
 break;
   default:
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
@@ -112,7 +113,7 @@
   if (!GetAssertLocation(os, location))
 return RecognizedStackFrameSP();
 
-  const uint32_t frames_to_fetch = 5;
+  const uint32_t frames_to_fetch = 6;
   const uint32_t last_frame_index = frames_to_fetch - 1;
   StackFrameSP prev_frame_sp = nullptr;
 


Index: lldb/source/Target/AssertFrameRecognizer.cpp
===
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -45,6 +45,7 @@
 location.symbols.push_back(ConstString("raise"));
 location.symbols.push_back(ConstString("__GI_raise"));
 location.symbols.push_back(ConstString("gsignal"));
+location.symbols.push_back(ConstString("pthread_kill@GLIBC_2.2.5"));
 break;
   default:
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
@@ -112,7 +113,7 @@
   if (!GetAssertLocation(os, location))
 return RecognizedStackFrameSP();
 
-  const uint32_t frames_to_fetch = 5;
+  const uint32_t frames_to_fetch = 6;
   const uint32_t last_frame_index = frames_to_fetch - 1;
   StackFrameSP prev_frame_sp = nullptr;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] aaba371 - [clang][PATCH][nfc] Refactor TargetInfo::adjust to pass DiagnosticsEngine to allow diagnostics on target-unsupported options

2021-06-29 Thread Melanie Blower via lldb-commits


Author: Melanie Blower
Date: 2021-06-29T13:26:23-04:00
New Revision: aaba37187fda7f5a7fdc4c1e6129cbaaa1bbf709

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

LOG: [clang][PATCH][nfc] Refactor TargetInfo::adjust to pass DiagnosticsEngine 
to allow diagnostics on target-unsupported options

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/lib/Basic/TargetInfo.cpp
clang/lib/Basic/Targets/AMDGPU.cpp
clang/lib/Basic/Targets/AMDGPU.h
clang/lib/Basic/Targets/PPC.cpp
clang/lib/Basic/Targets/PPC.h
clang/lib/Basic/Targets/SPIR.h
clang/lib/Basic/Targets/WebAssembly.cpp
clang/lib/Basic/Targets/WebAssembly.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Interpreter/Interpreter.cpp
clang/tools/clang-import-test/clang-import-test.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index d59bad30e7428..20f6afa76cbb3 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1162,7 +1162,7 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   /// Apply changes to the target information with respect to certain
   /// language options which change the target configuration and adjust
   /// the language based on the target options where applicable.
-  virtual void adjust(LangOptions &Opts);
+  virtual void adjust(DiagnosticsEngine &Diags, LangOptions &Opts);
 
   /// Adjust target options based on codegen options.
   virtual void adjustTargetOptions(const CodeGenOptions &CGOpts,

diff  --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index e73b4a3a40c74..4c2859e5eda7f 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -346,7 +346,7 @@ bool TargetInfo::isTypeSigned(IntType T) {
 /// Apply changes to the target information with respect to certain
 /// language options which change the target configuration and adjust
 /// the language based on the target options where applicable.
-void TargetInfo::adjust(LangOptions &Opts) {
+void TargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts) {
   if (Opts.NoBitFieldTypeAlign)
 UseBitFieldTypeAlignment = false;
 

diff  --git a/clang/lib/Basic/Targets/AMDGPU.cpp 
b/clang/lib/Basic/Targets/AMDGPU.cpp
index 595132e2e70ba..fac786dbcf9e2 100644
--- a/clang/lib/Basic/Targets/AMDGPU.cpp
+++ b/clang/lib/Basic/Targets/AMDGPU.cpp
@@ -358,8 +358,8 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple 
&Triple,
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
 }
 
-void AMDGPUTargetInfo::adjust(LangOptions &Opts) {
-  TargetInfo::adjust(Opts);
+void AMDGPUTargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts) {
+  TargetInfo::adjust(Diags, Opts);
   // ToDo: There are still a few places using default address space as private
   // address space in OpenCL, which needs to be cleaned up, then Opts.OpenCL
   // can be removed from the following line.

diff  --git a/clang/lib/Basic/Targets/AMDGPU.h 
b/clang/lib/Basic/Targets/AMDGPU.h
index fe5c61c6ba2bb..244a6e0446905 100644
--- a/clang/lib/Basic/Targets/AMDGPU.h
+++ b/clang/lib/Basic/Targets/AMDGPU.h
@@ -93,7 +93,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public 
TargetInfo {
 
   void setAddressSpaceMap(bool DefaultIsPrivate);
 
-  void adjust(LangOptions &Opts) override;
+  void adjust(DiagnosticsEngine &Diags, LangOptions &Opts) override;
 
   uint64_t getPointerWidthV(unsigned AddrSpace) const override {
 if (isR600(getTriple()))

diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 6860b5e5d02fa..d431dda970222 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -614,10 +614,10 @@ void 
PPCTargetInfo::fillValidCPUList(SmallVectorImpl &Values) const {
   Values.append(std::begin(ValidCPUNames), std::end(ValidCPUNames));
 }
 
-void PPCTargetInfo::adjust(LangOptions &Opts) {
+void PPCTargetInfo::adjust(DiagnosticsEngine &Diags, LangOptions &Opts) {
   if (HasAltivec)
 Opts.AltiVec = 1;
-  TargetInfo::adjust(Opts);
+  TargetInfo::adjust(Diags, Opts);
   if (LongDoubleFormat != &llvm::APFloat::IEEEdouble())
 LongDoubleFormat = Opts.PPCIEEELongDouble
? &llvm::APFloat::IEEEquad()

diff  --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
index 554f2174fee00..18ee1194c759d 100644
--- a/clang/lib/Basic/Targets/PPC.h
+++ b/clang/lib/Basic/Targets/PPC.h
@@ -89,7 +89

[Lldb-commits] [PATCH] D104729: [clang][PATCH][nfc] Refactor TargetInfo::adjust to pass DiagnosticsEngine to allow diagnostics on target-unsupported options

2021-06-29 Thread Melanie Blower via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaaba37187fda: [clang][PATCH][nfc] Refactor 
TargetInfo::adjust to pass DiagnosticsEngine to… (authored by mibintc).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104729

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Interpreter/Interpreter.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -704,7 +704,7 @@
   if (!instance->hasTarget())
 return nullptr;
 
-  instance->getTarget().adjust(instance->getLangOpts());
+  instance->getTarget().adjust(*diagnostics_engine, instance->getLangOpts());
 
   if (!action->BeginSourceFile(*instance,
instance->getFrontendOpts().Inputs[0]))
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -658,7 +658,8 @@
   //
   // FIXME: We shouldn't need to do this, the target should be immutable once
   // created. This complexity should be lifted elsewhere.
-  m_compiler->getTarget().adjust(m_compiler->getLangOpts());
+  m_compiler->getTarget().adjust(m_compiler->getDiagnostics(),
+		 m_compiler->getLangOpts());
 
   // 6. Set up the diagnostic buffer for reporting errors
 
Index: clang/tools/clang-import-test/clang-import-test.cpp
===
--- clang/tools/clang-import-test/clang-import-test.cpp
+++ clang/tools/clang-import-test/clang-import-test.cpp
@@ -208,7 +208,7 @@
   TargetInfo *TI = TargetInfo::CreateTargetInfo(
   Ins->getDiagnostics(), Ins->getInvocation().TargetOpts);
   Ins->setTarget(TI);
-  Ins->getTarget().adjust(Ins->getLangOpts());
+  Ins->getTarget().adjust(Ins->getDiagnostics(), Ins->getLangOpts());
   Ins->createFileManager();
   Ins->createSourceManager(Ins->getFileManager());
   Ins->createPreprocessor(TU_Complete);
Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -110,7 +110,7 @@
"Initialization failed. "
"Target is missing");
 
-  Clang->getTarget().adjust(Clang->getLangOpts());
+  Clang->getTarget().adjust(Clang->getDiagnostics(), Clang->getLangOpts());
 
   return std::move(Clang);
 }
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -142,7 +142,7 @@
   // Inform the target of the language options.
   // FIXME: We shouldn't need to do this, the target should be immutable once
   // created. This complexity should be lifted elsewhere.
-  getTarget().adjust(getLangOpts());
+  getTarget().adjust(getDiagnostics(), getLangOpts());
 
   // Adjust target options based on codegen options.
   getTarget().adjustTargetOptions(getCodeGenOpts(), getTargetOpts());
@@ -457,7 +457,7 @@
   getSourceManager(), *HeaderInfo, *this,
   /*IdentifierInfoLookup=*/nullptr,
   /*OwnsHeaderSearch=*/true, TUKind);
-  getTarget().adjust(getLangOpts());
+  getTarget().adjust(getDiagnostics(), getLangOpts());
   PP->Initialize(getTarget(), getAuxTarget());
 
   if (PPOpts.DetailedRecord)
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -588,7 +588,7 @@
 //
 // FIXME: We shouldn't need to do this, the target should be immutable once
 // created. This complexity should be lifted elsewhere.
-Target->adjust(LangOpt);
+Target->adjust(PP.getDiagnostics(), LangOpt);
 
 // Initialize the preprocessor.
 PP.Initialize(*Target);
Index

[Lldb-commits] [PATCH] D105136: [lldb Look for the mangled objc_copyRealizedClassList_nolock symbol name

2021-06-29 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added a reviewer: jingham.
JDevlieghere requested review of this revision.

When we check if the `objc_copyRealizedClassList_nolock` we need to check for 
the mangled symbol rather than than the unmangled one as we did for the locking 
objc_copyRealizedClassList which was C exported.


https://reviews.llvm.org/D105136

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -668,7 +668,7 @@
   static const ConstString g_gdb_object_getClass("gdb_object_getClass");
   m_has_object_getClass = HasSymbol(g_gdb_object_getClass);
   static const ConstString g_objc_copyRealizedClassList(
-  "objc_copyRealizedClassList_nolock");
+  "_ZL33objc_copyRealizedClassList_nolockPj");
   m_has_objc_copyRealizedClassList = HasSymbol(g_objc_copyRealizedClassList);
 
   RegisterObjCExceptionRecognizer(process);


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -668,7 +668,7 @@
   static const ConstString g_gdb_object_getClass("gdb_object_getClass");
   m_has_object_getClass = HasSymbol(g_gdb_object_getClass);
   static const ConstString g_objc_copyRealizedClassList(
-  "objc_copyRealizedClassList_nolock");
+  "_ZL33objc_copyRealizedClassList_nolockPj");
   m_has_objc_copyRealizedClassList = HasSymbol(g_objc_copyRealizedClassList);
 
   RegisterObjCExceptionRecognizer(process);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105136: [lldb Look for the mangled objc_copyRealizedClassList_nolock symbol name

2021-06-29 Thread Jim Ingham via Phabricator via lldb-commits

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

LGTM


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

https://reviews.llvm.org/D105136

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

[Lldb-commits] [lldb] 71be4db - [lldb] Check for the mangled symbol name for objc_copyRealizedClassList_nolock

2021-06-29 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2021-06-29T10:58:35-07:00
New Revision: 71be4db05bbdcc8a9bbe01f54cf273b530327ec7

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

LOG: [lldb] Check for the mangled symbol name for 
objc_copyRealizedClassList_nolock

When we check whether the Objective-C SPI is available, we need to check
for the mangled symbol name. Unlike `objc_copyRealizedClassList`, which
is C exported, the `nolock` variant is not.

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

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index a3a0827cfe65a..2ea7640ed737b 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -668,7 +668,7 @@ AppleObjCRuntimeV2::AppleObjCRuntimeV2(Process *process,
   static const ConstString g_gdb_object_getClass("gdb_object_getClass");
   m_has_object_getClass = HasSymbol(g_gdb_object_getClass);
   static const ConstString g_objc_copyRealizedClassList(
-  "objc_copyRealizedClassList_nolock");
+  "_ZL33objc_copyRealizedClassList_nolockPj");
   m_has_objc_copyRealizedClassList = HasSymbol(g_objc_copyRealizedClassList);
 
   RegisterObjCExceptionRecognizer(process);



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

[Lldb-commits] [PATCH] D105136: [lldb] Check for the mangled symbol name for objc_copyRealizedClassList_nolock

2021-06-29 Thread Jonas Devlieghere 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 rG71be4db05bbd: [lldb] Check for the mangled symbol name for 
objc_copyRealizedClassList_nolock (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105136

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -668,7 +668,7 @@
   static const ConstString g_gdb_object_getClass("gdb_object_getClass");
   m_has_object_getClass = HasSymbol(g_gdb_object_getClass);
   static const ConstString g_objc_copyRealizedClassList(
-  "objc_copyRealizedClassList_nolock");
+  "_ZL33objc_copyRealizedClassList_nolockPj");
   m_has_objc_copyRealizedClassList = HasSymbol(g_objc_copyRealizedClassList);
 
   RegisterObjCExceptionRecognizer(process);


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -668,7 +668,7 @@
   static const ConstString g_gdb_object_getClass("gdb_object_getClass");
   m_has_object_getClass = HasSymbol(g_gdb_object_getClass);
   static const ConstString g_objc_copyRealizedClassList(
-  "objc_copyRealizedClassList_nolock");
+  "_ZL33objc_copyRealizedClassList_nolockPj");
   m_has_objc_copyRealizedClassList = HasSymbol(g_objc_copyRealizedClassList);
 
   RegisterObjCExceptionRecognizer(process);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105133: [lldb] Fix Recognizer/assert.test with glibc-2.33.9000-31.fc35.x86_64

2021-06-29 Thread Florian Weimer via Phabricator via lldb-commits

fweimer added inline comments.



Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:48
 location.symbols.push_back(ConstString("gsignal"));
+location.symbols.push_back(ConstString("pthread_kill@GLIBC_2.2.5"));
 break;

The symbol version is architecture-dependent (not every architecture uses 
`GLIBC_2.2.5`), and there is also another `GLIBC_2.34` symbol version even on 
x86-64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105133

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

[Lldb-commits] [PATCH] D105133: [lldb] Fix Recognizer/assert.test with glibc-2.33.9000-31.fc35.x86_64

2021-06-29 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

Another possibility could be to add a `bool is_regex` to the `SymbolLocation` 
struct and set it to `true` for Linux, then in `RegisterAssertFrameRecognizer` 
if that flag is set, build a `RegularExpression` out  of the every symbol 
string in the struct and call the `FrameRecognizerManager::AddRecognizer` 
overload that uses `RegularExpressionSP` arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105133

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

[Lldb-commits] [PATCH] D105133: [lldb] Fix Recognizer/assert.test with glibc-2.33.9000-31.fc35.x86_64

2021-06-29 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil updated this revision to Diff 355327.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105133

Files:
  lldb/source/Target/AssertFrameRecognizer.cpp


Index: lldb/source/Target/AssertFrameRecognizer.cpp
===
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -22,6 +22,10 @@
 struct SymbolLocation {
   FileSpec module_spec;
   std::vector symbols;
+
+  // Both module_spec and symbols are regular expressions. In such case all
+  // symbols are matched with their trailing @VER symbol version stripped.
+  bool is_regex = false;
 };
 
 /// Fetches the abort frame location depending on the current platform.
@@ -45,6 +49,8 @@
 location.symbols.push_back(ConstString("raise"));
 location.symbols.push_back(ConstString("__GI_raise"));
 location.symbols.push_back(ConstString("gsignal"));
+location.symbols.push_back(ConstString("pthread_kill"));
+location.is_regex = true;
 break;
   default:
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
@@ -93,9 +99,27 @@
   if (!GetAbortLocation(os, location))
 return;
 
+  if (!location.is_regex) {
+target.GetFrameRecognizerManager().AddRecognizer(
+std::make_shared(),
+location.module_spec.GetFilename(), location.symbols,
+/*first_instruction_only*/ false);
+return;
+  }
+  std::string regex = "^(";
+  for (auto it = location.symbols.cbegin(); it != location.symbols.cend();
+   ++it) {
+if (it != location.symbols.cbegin())
+  regex += '|';
+regex += it->GetStringRef();
+  }
+  // Strip the trailing @VER symbol version.
+  regex += ")(@.*)?$";
   target.GetFrameRecognizerManager().AddRecognizer(
-  StackFrameRecognizerSP(new AssertFrameRecognizer()),
-  location.module_spec.GetFilename(), location.symbols,
+  std::make_shared(),
+  std::make_shared(
+  location.module_spec.GetFilename().GetStringRef()),
+  std::make_shared(std::move(regex)),
   /*first_instruction_only*/ false);
 }
 
@@ -112,7 +136,7 @@
   if (!GetAssertLocation(os, location))
 return RecognizedStackFrameSP();
 
-  const uint32_t frames_to_fetch = 5;
+  const uint32_t frames_to_fetch = 6;
   const uint32_t last_frame_index = frames_to_fetch - 1;
   StackFrameSP prev_frame_sp = nullptr;
 


Index: lldb/source/Target/AssertFrameRecognizer.cpp
===
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -22,6 +22,10 @@
 struct SymbolLocation {
   FileSpec module_spec;
   std::vector symbols;
+
+  // Both module_spec and symbols are regular expressions. In such case all
+  // symbols are matched with their trailing @VER symbol version stripped.
+  bool is_regex = false;
 };
 
 /// Fetches the abort frame location depending on the current platform.
@@ -45,6 +49,8 @@
 location.symbols.push_back(ConstString("raise"));
 location.symbols.push_back(ConstString("__GI_raise"));
 location.symbols.push_back(ConstString("gsignal"));
+location.symbols.push_back(ConstString("pthread_kill"));
+location.is_regex = true;
 break;
   default:
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
@@ -93,9 +99,27 @@
   if (!GetAbortLocation(os, location))
 return;
 
+  if (!location.is_regex) {
+target.GetFrameRecognizerManager().AddRecognizer(
+std::make_shared(),
+location.module_spec.GetFilename(), location.symbols,
+/*first_instruction_only*/ false);
+return;
+  }
+  std::string regex = "^(";
+  for (auto it = location.symbols.cbegin(); it != location.symbols.cend();
+   ++it) {
+if (it != location.symbols.cbegin())
+  regex += '|';
+regex += it->GetStringRef();
+  }
+  // Strip the trailing @VER symbol version.
+  regex += ")(@.*)?$";
   target.GetFrameRecognizerManager().AddRecognizer(
-  StackFrameRecognizerSP(new AssertFrameRecognizer()),
-  location.module_spec.GetFilename(), location.symbols,
+  std::make_shared(),
+  std::make_shared(
+  location.module_spec.GetFilename().GetStringRef()),
+  std::make_shared(std::move(regex)),
   /*first_instruction_only*/ false);
 }
 
@@ -112,7 +136,7 @@
   if (!GetAssertLocation(os, location))
 return RecognizedStackFrameSP();
 
-  const uint32_t frames_to_fetch = 5;
+  const uint32_t frames_to_fetch = 6;
   const uint32_t last_frame_index = frames_to_fetch - 1;
   StackFrameSP prev_frame_sp = nullptr;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D105133: [lldb] Fix Recognizer/assert.test with glibc-2.33.9000-31.fc35.x86_64

2021-06-29 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil added inline comments.



Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:115
+regex += it->GetStringRef();
+  }
+  // Strip the trailing @VER symbol version.

There would be nice `llvm::join`. But it wants `Elem.size()` while 
`ConstString` here provides `Elem.GetLength()`. I did not try to extend 
`StringExtras.h`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105133

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

[Lldb-commits] [PATCH] D105133: [lldb] Fix Recognizer/assert.test with glibc-2.33.9000-31.fc35.x86_64

2021-06-29 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

LGTM!




Comment at: lldb/source/Target/AssertFrameRecognizer.cpp:115
+regex += it->GetStringRef();
+  }
+  // Strip the trailing @VER symbol version.

jankratochvil wrote:
> There would be nice `llvm::join`. But it wants `Elem.size()` while 
> `ConstString` here provides `Elem.GetLength()`. I did not try to extend 
> `StringExtras.h`.
> 
That's unfortunate indeed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105133

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

[Lldb-commits] [PATCH] D105133: [lldb] Fix Recognizer/assert.test with glibc-2.33.9000-31.fc35.x86_64

2021-06-29 Thread Jan Kratochvil via Phabricator via lldb-commits

jankratochvil updated this revision to Diff 355333.
jankratochvil added a comment.
This revision is now accepted and ready to land.

Unfortunately `AddRecognizer` requires both or none `module` and `symbols` as 
regexes. And we cannot make `module` a regex as `GetFileSpec().FileEquals` 
would not work then. So I have just escaped the regex.
It looks a bit fishy as `StackFrameRecognizer` uses 
`entry.module_regexp->Execute` ignoring case insensitivity on OSX but then I do 
not target that platform myself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105133

Files:
  lldb/source/Target/AssertFrameRecognizer.cpp


Index: lldb/source/Target/AssertFrameRecognizer.cpp
===
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -22,6 +22,10 @@
 struct SymbolLocation {
   FileSpec module_spec;
   std::vector symbols;
+
+  // The symbols are regular expressions. In such case all symbols are matched
+  // with their trailing @VER symbol version stripped.
+  bool symbols_are_regex = false;
 };
 
 /// Fetches the abort frame location depending on the current platform.
@@ -45,6 +49,8 @@
 location.symbols.push_back(ConstString("raise"));
 location.symbols.push_back(ConstString("__GI_raise"));
 location.symbols.push_back(ConstString("gsignal"));
+location.symbols.push_back(ConstString("pthread_kill"));
+location.symbols_are_regex = true;
 break;
   default:
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
@@ -93,9 +99,33 @@
   if (!GetAbortLocation(os, location))
 return;
 
+  if (!location.symbols_are_regex) {
+target.GetFrameRecognizerManager().AddRecognizer(
+std::make_shared(),
+location.module_spec.GetFilename(), location.symbols,
+/*first_instruction_only*/ false);
+return;
+  }
+  std::string module_re = "^";
+  for (char c : location.module_spec.GetFilename().GetStringRef()) {
+if (c == '.')
+  module_re += '\\';
+module_re += c;
+  }
+  module_re += '$';
+  std::string symbol_re = "^(";
+  for (auto it = location.symbols.cbegin(); it != location.symbols.cend();
+   ++it) {
+if (it != location.symbols.cbegin())
+  symbol_re += '|';
+symbol_re += it->GetStringRef();
+  }
+  // Strip the trailing @VER symbol version.
+  symbol_re += ")(@.*)?$";
   target.GetFrameRecognizerManager().AddRecognizer(
-  StackFrameRecognizerSP(new AssertFrameRecognizer()),
-  location.module_spec.GetFilename(), location.symbols,
+  std::make_shared(),
+  std::make_shared(std::move(module_re)),
+  std::make_shared(std::move(symbol_re)),
   /*first_instruction_only*/ false);
 }
 
@@ -112,7 +142,7 @@
   if (!GetAssertLocation(os, location))
 return RecognizedStackFrameSP();
 
-  const uint32_t frames_to_fetch = 5;
+  const uint32_t frames_to_fetch = 6;
   const uint32_t last_frame_index = frames_to_fetch - 1;
   StackFrameSP prev_frame_sp = nullptr;
 


Index: lldb/source/Target/AssertFrameRecognizer.cpp
===
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -22,6 +22,10 @@
 struct SymbolLocation {
   FileSpec module_spec;
   std::vector symbols;
+
+  // The symbols are regular expressions. In such case all symbols are matched
+  // with their trailing @VER symbol version stripped.
+  bool symbols_are_regex = false;
 };
 
 /// Fetches the abort frame location depending on the current platform.
@@ -45,6 +49,8 @@
 location.symbols.push_back(ConstString("raise"));
 location.symbols.push_back(ConstString("__GI_raise"));
 location.symbols.push_back(ConstString("gsignal"));
+location.symbols.push_back(ConstString("pthread_kill"));
+location.symbols_are_regex = true;
 break;
   default:
 Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));
@@ -93,9 +99,33 @@
   if (!GetAbortLocation(os, location))
 return;
 
+  if (!location.symbols_are_regex) {
+target.GetFrameRecognizerManager().AddRecognizer(
+std::make_shared(),
+location.module_spec.GetFilename(), location.symbols,
+/*first_instruction_only*/ false);
+return;
+  }
+  std::string module_re = "^";
+  for (char c : location.module_spec.GetFilename().GetStringRef()) {
+if (c == '.')
+  module_re += '\\';
+module_re += c;
+  }
+  module_re += '$';
+  std::string symbol_re = "^(";
+  for (auto it = location.symbols.cbegin(); it != location.symbols.cend();
+   ++it) {
+if (it != location.symbols.cbegin())
+  symbol_re += '|';
+symbol_re += it->GetStringRef();
+  }
+  // Strip the trailing @VER symbol version.
+  symbol_re += ")(@.*)?$";
   target.GetFrameRecognizerManager().AddRecognizer(
-  StackFrameRecognizerSP(new AssertFrameRecognizer())

[Lldb-commits] [lldb] bb2cfca - Revert D104488 and friends since it broke the windows bot

2021-06-29 Thread Stella Stamenova via lldb-commits


Author: Stella Stamenova
Date: 2021-06-29T12:58:55-07:00
New Revision: bb2cfca2f3237d7f722e95d4cab9f3d71f728c9c

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

LOG: Revert D104488 and friends since it broke the windows bot

Reverts commits:
"Fix failing tests after https://reviews.llvm.org/D104488.";
"Fix buildbot failure after https://reviews.llvm.org/D104488.";
"Create synthetic symbol names on demand to improve memory consumption and 
startup times."

This series of commits broke the windows lldb bot and then failed to fix all of 
the failing tests.

Added: 


Modified: 
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
lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
lldb/test/Shell/SymbolFile/Breakpad/symtab.test

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index dc83565c7db52..1e29cf53b78b3 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -712,6 +712,8 @@ class ObjectFile : public 
std::enable_shared_from_this,
   /// false otherwise.
   bool SetModulesArchitecture(const ArchSpec &new_arch);
 
+  ConstString GetNextSyntheticSymbolName();
+
   static lldb::DataBufferSP MapFileData(const FileSpec &file, uint64_t Size,
 uint64_t Offset);
 

diff  --git a/lldb/include/lldb/Symbol/Symbol.h 
b/lldb/include/lldb/Symbol/Symbol.h
index be3e8abefa490..3abe3114863de 100644
--- a/lldb/include/lldb/Symbol/Symbol.h
+++ b/lldb/include/lldb/Symbol/Symbol.h
@@ -113,20 +113,14 @@ class Symbol : public SymbolContextScope {
   lldb::LanguageType GetLanguage() const {
 // TODO: See if there is a way to determine the language for a symbol
 // somehow, for now just return our best guess
-return GetMangled().GuessLanguage();
+return m_mangled.GuessLanguage();
   }
 
   void SetID(uint32_t uid) { m_uid = uid; }
 
-  Mangled &GetMangled() {
-SynthesizeNameIfNeeded();
-return m_mangled;
-  }
+  Mangled &GetMangled() { return m_mangled; }
 
-  const Mangled &GetMangled() const {
-SynthesizeNameIfNeeded();
-return m_mangled;
-  }
+  const Mangled &GetMangled() const { return m_mangled; }
 
   ConstString GetReExportedSymbolName() const;
 
@@ -172,9 +166,9 @@ class Symbol : public SymbolContextScope {
   bool IsTrampoline() const;
 
   bool IsIndirect() const;
-
+  
   bool IsWeak() const { return m_is_weak; }
-
+  
   void SetIsWeak (bool b) { m_is_weak = b; }
 
   bool GetByteSizeIsValid() const { return m_size_is_valid; }
@@ -229,10 +223,6 @@ class Symbol : public SymbolContextScope {
 
   bool ContainsFileAddress(lldb::addr_t file_addr) const;
 
-  static llvm::StringRef GetSyntheticSymbolPrefix() {
-return "___lldb_unnamed_symbol";
-  }
-
 protected:
   // This is the internal guts of ResolveReExportedSymbol, it assumes
   // reexport_name is not null, and that module_spec is valid.  We track the
@@ -243,8 +233,6 @@ class Symbol : public SymbolContextScope {
   lldb_private::ModuleSpec &module_spec,
   lldb_private::ModuleList &seen_modules) const;
 
-  void SynthesizeNameIfNeeded() const;
-
   uint32_t m_uid =
   UINT32_MAX;   // User ID (usually the original symbol table 
index)
   uint16_t m_type_data = 0; // data specific to m_type
@@ -270,7 +258,7 @@ class Symbol : public SymbolContextScope {
  // doing name lookups
   m_is_weak : 1,
   m_type : 6;// Values from the lldb::SymbolType enum.
-  mutable Mangled m_mangled; // uniqued symbol name/mangled name pair
+  Mangled m_mangled; // uniqued symbol name/mangled name pair
   AddressRange m_addr_range; // Contains the value, or the section offset
  // address when the value is an address in a
  // section, and the size (if any)

diff  --git a/lldb/include/lldb/Symbol/Symtab.h 
b/lldb/include/lldb/Symbol/Symtab.h
index e1ad0dfd2eb8d..fbfa3a5e0cec7 100644
--- a/lldb/include/lldb/Symbol/Symtab.h
+++ b/lldb/include/lldb/Symbol/Symtab.h
@@ -219,26 +219,6 @@ class Symtab {
 return false;
   }
 
-  /// A helper function that looks up full function names.
-  ///
-  /// 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

[Lldb-commits] [lldb] 9952d59 - [lldb] Fix globals-bss.cpp which was broken in https://reviews.llvm.org/D105055

2021-06-29 Thread Stella Stamenova via lldb-commits


Author: Stella Stamenova
Date: 2021-06-29T13:39:18-07:00
New Revision: 9952d591ccc49cbcbf9c89d5191e6111c44703a6

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

LOG: [lldb] Fix globals-bss.cpp which was broken in 
https://reviews.llvm.org/D105055

-S replaced -s, so the test needs to be updated to use the new option

Added: 


Modified: 
lldb/test/Shell/SymbolFile/NativePDB/globals-bss.cpp

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/globals-bss.cpp 
b/lldb/test/Shell/SymbolFile/NativePDB/globals-bss.cpp
index 3744a2837055d..9c65c26499cd1 100644
--- a/lldb/test/Shell/SymbolFile/NativePDB/globals-bss.cpp
+++ b/lldb/test/Shell/SymbolFile/NativePDB/globals-bss.cpp
@@ -4,7 +4,7 @@
 // Make sure we can read variables from BSS
 // RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
 // RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
-// RUN: llvm-readobj -s %t.exe | FileCheck --check-prefix=BSS %s
+// RUN: llvm-readobj -S %t.exe | FileCheck --check-prefix=BSS %s
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/globals-bss.lldbinit 2>&1 | FileCheck %s
 



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

[Lldb-commits] [lldb] c8a9c78 - [lldb] Fix debug_loc.s which was broken after https://reviews.llvm.org/D103502

2021-06-29 Thread Stella Stamenova via lldb-commits


Author: Stella Stamenova
Date: 2021-06-29T13:54:48-07:00
New Revision: c8a9c78e170e3b972041b301a50d0456afe83d10

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

LOG: [lldb] Fix debug_loc.s which was broken after 
https://reviews.llvm.org/D103502

An empty location is now printed as 

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s 
b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
index 3d78469f6306d..5b8d1bc328559 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
@@ -14,11 +14,11 @@
 
 # CHECK-LABEL: image lookup -v -a 0
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg5 
RDI,
-# CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
+# CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
 
 # CHECK-LABEL: image lookup -v -a 2
 # CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg0 
RAX,
-# CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
+# CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
 # CHECK: Variable: {{.*}}, name = "x3", type = "int", location = DW_OP_reg1 
RDX,
 
 # CHECK-LABEL: image dump symfile



___
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-29 Thread Omar Emara via Phabricator via lldb-commits

OmarEmaraDev updated this revision to Diff 355364.
OmarEmaraDev added a comment.
This revision is now accepted and ready to land.

- Rewrite internal field navigation.
- Rewrite form window drawing. Form delegate no longer have drawing routines.
- Add global error messages.
- Add action bar. Form delegate can now define as many arbitrary actions as 
needed.
- Make action button scrollable.
- Add support for composite fields.
- Add backward tab navigation.


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
@@ -71,6 +71,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 using namespace lldb;
@@ -85,6 +86,8 @@
 #define KEY_RETURN 10
 #define KEY_ESCAPE 27
 
+#define KEY_SHIFT_TAB (KEY_MAX + 1)
+
 namespace curses {
 class Menu;
 class MenuDelegate;
@@ -336,93 +339,52 @@
   int m_first_visible_line;
 };
 
-class Window {
+// A surface is an abstraction for something than can be drawn on. The surface
+// have a width, a height, a cursor position, and a multitude of drawing
+// operations. This type should be sub-classed to get an actually useful ncurses
+// object, such as a Window, SubWindow, Pad, or a SubPad.
+class Surface {
 public:
-  Window(const char *name)
-  : m_name(name), m_window(nullptr), m_panel(nullptr), m_parent(nullptr),
-m_subwindows(), m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(false),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {}
+  Surface() : m_window(nullptr) {}
 
-  Window(const char *name, WINDOW *w, bool del = true)
-  : m_name(name), m_window(nullptr), m_panel(nullptr), m_parent(nullptr),
-m_subwindows(), m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(del),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {
-if (w)
-  Reset(w);
-  }
+  WINDOW *get() { return m_window; }
 
-  Window(const char *name, const Rect &bounds)
-  : m_name(name), m_window(nullptr), m_parent(nullptr), m_subwindows(),
-m_delegate_sp(), m_curr_active_window_idx(UINT32_MAX),
-m_prev_active_window_idx(UINT32_MAX), m_delete(true),
-m_needs_update(true), m_can_activate(true), m_is_subwin(false) {
-Reset(::newwin(bounds.size.height, bounds.size.width, bounds.origin.y,
-   bounds.origin.y));
-  }
+  operator WINDOW *() { return m_window; }
 
-  virtual ~Window() {
-RemoveSubWindows();
-Reset();
+  // Copy a region of the surface to another surface.
+  void CopyToSurface(Surface &target, Point source_origin, Point target_origin,
+ Size size) {
+::copywin(m_window, target.get(), source_origin.y, source_origin.x,
+  target_origin.y, target_origin.x,
+  target_origin.y + size.height - 1,
+  target_origin.x + size.width - 1, false);
   }
 
-  void Reset(WINDOW *w = nullptr, bool del = true) {
-if (m_window == w)
-  return;
-
-if (m_panel) {
-  ::del_panel(m_panel);
-  m_panel = nullptr;
-}
-if (m_window && m_delete) {
-  ::delwin(m_window);
-  m_window = nullptr;
-  m_delete = false;
-}
-if (w) {
-  m_window = w;
-  m_panel = ::new_panel(m_window);
-  m_delete = del;
-}
-  }
+  int GetCursorX() const { return getcurx(m_window); }
+  int GetCursorY() const { return getcury(m_window); }
+  void MoveCursor(int x, int y) { ::wmove(m_window, y, x); }
 
   void AttributeOn(attr_t attr) { ::wattron(m_window, attr); }
   void AttributeOff(attr_t attr) { ::wattroff(m_window, attr); }
-  void Box(chtype v_char = ACS_VLINE, chtype h_char = ACS_HLINE) {
-::box(m_window, v_char, h_char);
-  }
-  void Clear() { ::wclear(m_window); }
-  void Erase() { ::werase(m_window); }
-  Rect GetBounds() const {
-return Rect(GetParentOrigin(), GetSize());
-  } // Get the rectangle in our parent window
-  int GetChar() { return ::wgetch(m_window); }
-  int GetCursorX() const { return getcurx(m_window); }
-  int GetCursorY() const { return getcury(m_window); }
-  Rect GetFrame() const {
-return Rect(Point(), GetSize());
-  } // Get our rectangle in our own coordinate system
-  Point GetParentOrigin() const { return Point(GetParentX(), GetParentY()); }
-  Size GetSize() const { return Size(GetWidth(), GetHeight()); }
-  int GetParentX() const { return getparx(m_window); }
-  int GetParentY() const { return getpary(m_window); }
+
   int GetMaxX() const { return getmaxx(m_window); }
   int GetMaxY() const { return getmaxy(m_window); }
   int GetWidth() const { return GetMaxX(); }
   int GetHeight() const {

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

2021-06-29 Thread Omar Emara via Phabricator via lldb-commits

OmarEmaraDev planned changes to this revision.
OmarEmaraDev added a comment.

- Scrolling was temporarily removed from the patch. It was causing issues with 
fields that change in size. I will reimplement it as contextual scrolling 
directly.
- Action buttons weren't moved to the window border as discussed. The window 
border is already highlighted when the form is active, which makes highlighting 
and navigating fields not user friendly. Action buttons are now scrollable 
though, which solves the issue of space.


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] D104405: Change PathMappingList::FindFile to return an optional result (NFC)

2021-06-29 Thread Adrian Prantl 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 rGa346372200e7: Change PathMappingList::FindFile to return an 
optional result (NFC) (authored by aprantl).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D104405?vs=353700&id=355379#toc

Repository:
  rG LLVM Github Monorepo

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
@@ -194,16 +194,16 @@
   return false;
 }
 
-bool PathMappingList::FindFile(const FileSpec &orig_spec,
-   FileSpec &new_spec) const {
+llvm::Optional
+PathMappingList::FindFile(const FileSpec &orig_spec) const {
   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();
 
   for (auto entry : m_pairs) {
@@ -228,15 +228,15 @@
   continue;
 
 if (orig_ref.consume_front(prefix_ref)) {
+  FileSpec new_spec;
   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,9 @@
 
 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;
+// Check target specific source remappings (i.e., the
+// target.source-map setting), then fall back to the module
+// specific remapping (i.e., the .dSYM remapping dictionary).
+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
-  ///

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

2021-06-29 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2021-06-29T15:10:46-07:00
New Revision: a346372200e7b2b99631bd90691678d5ca03fdd1

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

LOG: Change PathMappingList::FindFile to return an optional result (NFC)

This is an NFC modernization refactoring that replaces the combination
of a bool return + reference argument, with an Optional return value.

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

Added: 


Modified: 
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

Removed: 




diff  --git a/lldb/include/lldb/Target/PathMappingList.h 
b/lldb/include/lldb/Target/PathMappingList.h
index 5d8e2a1b4d242..46d7a427d3071 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -90,14 +90,9 @@ class PathMappingList {
   /// \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;
 

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 6502518f9247f..af7128496812d 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1598,7 +1598,11 @@ bool Module::MatchesModuleSpec(const ModuleSpec 
&module_ref) {
 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,

diff  --git a/lldb/source/Core/SourceManager.cpp 
b/lldb/source/Core/SourceManager.cpp
index 691bda6592e2e..3d51d42897c23 100644
--- a/lldb/source/Core/SourceManager.cpp
+++ b/lldb/source/Core/SourceManager.cpp
@@ -441,13 +441,17 @@ void SourceManager::File::CommonInitializer(const 
FileSpec &file_spec,
   }
   // 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;
+// Check target specific source remappings (i.e., the
+// target.source-map setting), then fall back to the module
+// specific remapping (i.e., the .dSYM remapping dictionary).
+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);
 }
   }

diff  --git a/lldb/source/Symbol/LineEntry.cpp 
b/lldb/source/Symbol/LineEntry.cpp
index 58bf8509a9589..1b2801cd03683 100644
--- a/lldb/source/Symbol/LineEntry.cpp
+++ b/lldb/source/Symbol/LineEntry.cpp
@@ -252,9 +252,9 @@ AddressRange LineEntry::GetSameLineContiguousAddressRange(
 
 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;
   }
 }

diff  --git a/lldb/source/Target/PathMappingList.cpp 
b/lldb/source/Target/PathMappingList.cpp
index b6dbf551ea57d..f9d415bcf15d7 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -194,16 +194,16 @@ bool PathMappingLis

[Lldb-commits] [PATCH] D104406: Express PathMappingList::FindFile() in terms of PathMappingList.h::RemapPath() (NFC)

2021-06-29 Thread Adrian Prantl 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 rG302b1b971809: Express PathMappingList::FindFile() in terms 
of PathMappingList::RemapPath() (authored by aprantl).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D104406?vs=352492&id=355382#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104406

Files:
  lldb/source/Target/PathMappingList.cpp


Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -194,48 +194,12 @@
   return false;
 }
 
-llvm::Optional
-PathMappingList::FindFile(const FileSpec &orig_spec) const {
-  if (m_pairs.empty())
-return {};
-
-  std::string orig_path = orig_spec.GetPath();
-
-  if (orig_path.empty())
-return {};
-
-  bool orig_is_relative = orig_spec.IsRelative();
 
-  for (auto entry : m_pairs) {
-llvm::StringRef orig_ref(orig_path);
-llvm::StringRef prefix_ref = entry.first.GetStringRef();
-if (orig_ref.size() < prefix_ref.size())
-  continue;
-// We consider a relative prefix or one of just "." to
-// mean "only apply to relative paths".
-bool prefix_is_relative = false;
-
-if (prefix_ref == ".") {
-  prefix_is_relative = true;
-  // Remove the "." since it will have been removed from the
-  // FileSpec paths already.
-  prefix_ref = prefix_ref.drop_front();
-} else {
-  FileSpec prefix_spec(prefix_ref, FileSpec::Style::native);
-  prefix_is_relative = prefix_spec.IsRelative();
-}
-if (prefix_is_relative != orig_is_relative)
-  continue;
+llvm::Optional PathMappingList::FindFile(const FileSpec &orig_spec) 
const {
+  if (auto remapped = RemapPath(orig_spec.GetPath()))
+if (FileSystem::Instance().Exists(*remapped))
+  return remapped;
 
-if (orig_ref.consume_front(prefix_ref)) {
-  FileSpec new_spec;
-  new_spec.SetFile(entry.second.GetCString(), FileSpec::Style::native);
-  new_spec.AppendPathComponent(orig_ref);
-  if (FileSystem::Instance().Exists(new_spec))
-return new_spec;
-}
-  }
-  
   return {};
 }
 


Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -194,48 +194,12 @@
   return false;
 }
 
-llvm::Optional
-PathMappingList::FindFile(const FileSpec &orig_spec) const {
-  if (m_pairs.empty())
-return {};
-
-  std::string orig_path = orig_spec.GetPath();
-
-  if (orig_path.empty())
-return {};
-
-  bool orig_is_relative = orig_spec.IsRelative();
 
-  for (auto entry : m_pairs) {
-llvm::StringRef orig_ref(orig_path);
-llvm::StringRef prefix_ref = entry.first.GetStringRef();
-if (orig_ref.size() < prefix_ref.size())
-  continue;
-// We consider a relative prefix or one of just "." to
-// mean "only apply to relative paths".
-bool prefix_is_relative = false;
-
-if (prefix_ref == ".") {
-  prefix_is_relative = true;
-  // Remove the "." since it will have been removed from the
-  // FileSpec paths already.
-  prefix_ref = prefix_ref.drop_front();
-} else {
-  FileSpec prefix_spec(prefix_ref, FileSpec::Style::native);
-  prefix_is_relative = prefix_spec.IsRelative();
-}
-if (prefix_is_relative != orig_is_relative)
-  continue;
+llvm::Optional PathMappingList::FindFile(const FileSpec &orig_spec) const {
+  if (auto remapped = RemapPath(orig_spec.GetPath()))
+if (FileSystem::Instance().Exists(*remapped))
+  return remapped;
 
-if (orig_ref.consume_front(prefix_ref)) {
-  FileSpec new_spec;
-  new_spec.SetFile(entry.second.GetCString(), FileSpec::Style::native);
-  new_spec.AppendPathComponent(orig_ref);
-  if (FileSystem::Instance().Exists(new_spec))
-return new_spec;
-}
-  }
-  
   return {};
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 302b1b9 - Express PathMappingList::FindFile() in terms of PathMappingList::RemapPath()

2021-06-29 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2021-06-29T15:14:31-07:00
New Revision: 302b1b97180907011aae610b9f51d4b9186c9821

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

LOG: Express PathMappingList::FindFile() in terms of 
PathMappingList::RemapPath()

NFC.

This patch replaces the function body FindFile() with a call to
RemapPath(), since the two functions implement the same functionality.

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

Added: 


Modified: 
lldb/source/Target/PathMappingList.cpp

Removed: 




diff  --git a/lldb/source/Target/PathMappingList.cpp 
b/lldb/source/Target/PathMappingList.cpp
index f9d415bcf15d..8a8cc1c8ab9b 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -194,48 +194,12 @@ bool PathMappingList::ReverseRemapPath(const FileSpec 
&file, FileSpec &fixed) co
   return false;
 }
 
-llvm::Optional
-PathMappingList::FindFile(const FileSpec &orig_spec) const {
-  if (m_pairs.empty())
-return {};
-
-  std::string orig_path = orig_spec.GetPath();
-
-  if (orig_path.empty())
-return {};
-
-  bool orig_is_relative = orig_spec.IsRelative();
 
-  for (auto entry : m_pairs) {
-llvm::StringRef orig_ref(orig_path);
-llvm::StringRef prefix_ref = entry.first.GetStringRef();
-if (orig_ref.size() < prefix_ref.size())
-  continue;
-// We consider a relative prefix or one of just "." to
-// mean "only apply to relative paths".
-bool prefix_is_relative = false;
-
-if (prefix_ref == ".") {
-  prefix_is_relative = true;
-  // Remove the "." since it will have been removed from the
-  // FileSpec paths already.
-  prefix_ref = prefix_ref.drop_front();
-} else {
-  FileSpec prefix_spec(prefix_ref, FileSpec::Style::native);
-  prefix_is_relative = prefix_spec.IsRelative();
-}
-if (prefix_is_relative != orig_is_relative)
-  continue;
+llvm::Optional PathMappingList::FindFile(const FileSpec &orig_spec) 
const {
+  if (auto remapped = RemapPath(orig_spec.GetPath()))
+if (FileSystem::Instance().Exists(*remapped))
+  return remapped;
 
-if (orig_ref.consume_front(prefix_ref)) {
-  FileSpec new_spec;
-  new_spec.SetFile(entry.second.GetCString(), FileSpec::Style::native);
-  new_spec.AppendPathComponent(orig_ref);
-  if (FileSystem::Instance().Exists(new_spec))
-return new_spec;
-}
-  }
-  
   return {};
 }
 



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

[Lldb-commits] [lldb] a0e1b11 - Modernize Module::RemapFile to return an Optional (NFC)

2021-06-29 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2021-06-29T15:19:31-07:00
New Revision: a0e1b11fac7a1599faec21d13fae45c8571de02c

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

LOG: Modernize Module::RemapFile to return an Optional (NFC)

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

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

Added: 


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

Removed: 




diff  --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index a9ec1e890dabd..dd7100c4616c3 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -850,13 +850,10 @@ class Module : public 
std::enable_shared_from_this,
   /// \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.

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index af7128496812d..fb805353e47b8 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1605,14 +1605,11 @@ bool Module::FindSourceFile(const FileSpec &orig_spec,
   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) {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index ca02d64709e89..6549e4fca0f95 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -240,9 +240,12 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP &module,
   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 @@ static void MakeAbsoluteAndRemap(FileSpec &file_spec, 
DWARFUnit &dwarf_cu,
   // 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) 
{



___
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-29 Thread Adrian Prantl 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 rGa0e1b11fac7a: Modernize Module::RemapFile to return an 
Optional (NFC) (authored by aprantl).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

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_f

[Lldb-commits] [PATCH] D104407: Improve path remapping in cross-debugging scenarios

2021-06-29 Thread Adrian Prantl 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 rG21e013303bb7: Improve path remapping in cross-debugging 
scenarios (authored by aprantl).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D104407?vs=352497&id=355385#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104407

Files:
  lldb/source/Target/PathMappingList.cpp
  lldb/unittests/Target/PathMappingListTest.cpp


Index: lldb/unittests/Target/PathMappingListTest.cpp
===
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -6,9 +6,9 @@
 //
 
//===--===//
 
-#include "llvm/ADT/ArrayRef.h"
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -19,6 +19,8 @@
   FileSpec original;
   FileSpec remapped;
   Matches(const char *o, const char *r) : original(o), remapped(r) {}
+  Matches(const char *o, llvm::sys::path::Style style, const char *r)
+  : original(o, style), remapped(r) {}
 };
 } // namespace
 
@@ -112,3 +114,27 @@
   };
   TestPathMappings(map, matches, fails);
 }
+
+#ifndef _WIN32
+TEST(PathMappingListTest, CrossPlatformTests) {
+  PathMappingList map;
+  map.Append(ConstString(R"(C:\old)"), ConstString("/new"), false);
+  Matches matches[] = {
+{R"(C:\old)", llvm::sys::path::Style::windows, "/new"},
+{R"(C:\old\)", llvm::sys::path::Style::windows, "/new"},
+{R"(C:\old\foo\.)", llvm::sys::path::Style::windows, "/new/foo"},
+{R"(C:\old\foo.c)", llvm::sys::path::Style::windows, "/new/foo.c"},
+{R"(C:\old\foo.c\.)", llvm::sys::path::Style::windows, "/new/foo.c"},
+{R"(C:\old\.\foo.c)", llvm::sys::path::Style::windows, "/new/foo.c"},
+  };
+  ConstString fails[] = {
+ConstString("/foo"),
+ConstString("/"),
+ConstString("foo.c"),
+ConstString("./foo.c"),
+ConstString("../foo.c"),
+ConstString("../bar/foo.c"),
+  };
+  TestPathMappings(map, matches, fails);
+}
+#endif
Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -152,6 +152,18 @@
   return false;
 }
 
+/// Append components to path, applying style.
+static void AppendPathComponents(FileSpec &path, llvm::StringRef components,
+ llvm::sys::path::Style style) {
+auto component = llvm::sys::path::begin(components, style);
+auto e = llvm::sys::path::end(components);
+while (component != e &&
+llvm::sys::path::is_separator(*component->data(), style))
+  ++component;
+for (; component != e; ++component)
+  path.AppendPathComponent(*component);
+}
+
 llvm::Optional
 PathMappingList::RemapPath(llvm::StringRef path) const {
   if (m_pairs.empty() || path.empty())
@@ -175,7 +187,9 @@
 continue;
 }
 FileSpec remapped(it.second.GetStringRef());
-remapped.AppendPathComponent(path);
+auto orig_style = FileSpec::GuessPathStyle(prefix).getValueOr(
+llvm::sys::path::Style::native);
+AppendPathComponents(remapped, path, orig_style);
 return remapped;
   }
   return {};
@@ -187,8 +201,11 @@
   for (const auto &it : m_pairs) {
 if (!path_ref.consume_front(it.second.GetStringRef()))
   continue;
-fixed.SetFile(it.first.GetStringRef(), FileSpec::Style::native);
-fixed.AppendPathComponent(path_ref);
+auto orig_file = it.first.GetStringRef();
+auto orig_style = FileSpec::GuessPathStyle(orig_file).getValueOr(
+llvm::sys::path::Style::native);
+fixed.SetFile(orig_file, orig_style);
+AppendPathComponents(fixed, path_ref, orig_style);
 return true;
   }
   return false;


Index: lldb/unittests/Target/PathMappingListTest.cpp
===
--- lldb/unittests/Target/PathMappingListTest.cpp
+++ lldb/unittests/Target/PathMappingListTest.cpp
@@ -6,9 +6,9 @@
 //
 //===--===//
 
-#include "llvm/ADT/ArrayRef.h"
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -19,6 +19,8 @@
   FileSpec original;
   FileSpec remapped;
   Matches(const char *o, const char *r) : original(o), remapped(r) {}
+  Matches(const char *o, llvm::sys::path::Style style, const char *r)
+  : original(o, style), remapped(r) {}
 };
 } // namespace
 
@@ -112,3 +114,27 @@
   };
   TestPathMappings(map, matches, fails);
 }
+
+#ifndef _WIN32
+TEST(PathMappingListTest, CrossPlatformTests) {
+  PathMappingList

[Lldb-commits] [lldb] 21e0133 - Improve path remapping in cross-debugging scenarios

2021-06-29 Thread Adrian Prantl via lldb-commits


Author: Adrian Prantl
Date: 2021-06-29T15:27:01-07:00
New Revision: 21e013303bb7d0dbb9106283af0fb966fe45af42

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

LOG: Improve path remapping in cross-debugging scenarios

This patch implements a slight improvement when debugging across
platforms and remapping source paths that are in a non-native
format. See the unit test for examples.

rdar://79205675

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

Added: 


Modified: 
lldb/source/Target/PathMappingList.cpp
lldb/unittests/Target/PathMappingListTest.cpp

Removed: 




diff  --git a/lldb/source/Target/PathMappingList.cpp 
b/lldb/source/Target/PathMappingList.cpp
index 8a8cc1c8ab9be..86b0ad5f933ff 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -152,6 +152,18 @@ bool PathMappingList::RemapPath(ConstString path,
   return false;
 }
 
+/// Append components to path, applying style.
+static void AppendPathComponents(FileSpec &path, llvm::StringRef components,
+ llvm::sys::path::Style style) {
+auto component = llvm::sys::path::begin(components, style);
+auto e = llvm::sys::path::end(components);
+while (component != e &&
+llvm::sys::path::is_separator(*component->data(), style))
+  ++component;
+for (; component != e; ++component)
+  path.AppendPathComponent(*component);
+}
+
 llvm::Optional
 PathMappingList::RemapPath(llvm::StringRef path) const {
   if (m_pairs.empty() || path.empty())
@@ -175,7 +187,9 @@ PathMappingList::RemapPath(llvm::StringRef path) const {
 continue;
 }
 FileSpec remapped(it.second.GetStringRef());
-remapped.AppendPathComponent(path);
+auto orig_style = FileSpec::GuessPathStyle(prefix).getValueOr(
+llvm::sys::path::Style::native);
+AppendPathComponents(remapped, path, orig_style);
 return remapped;
   }
   return {};
@@ -187,8 +201,11 @@ bool PathMappingList::ReverseRemapPath(const FileSpec 
&file, FileSpec &fixed) co
   for (const auto &it : m_pairs) {
 if (!path_ref.consume_front(it.second.GetStringRef()))
   continue;
-fixed.SetFile(it.first.GetStringRef(), FileSpec::Style::native);
-fixed.AppendPathComponent(path_ref);
+auto orig_file = it.first.GetStringRef();
+auto orig_style = FileSpec::GuessPathStyle(orig_file).getValueOr(
+llvm::sys::path::Style::native);
+fixed.SetFile(orig_file, orig_style);
+AppendPathComponents(fixed, path_ref, orig_style);
 return true;
   }
   return false;

diff  --git a/lldb/unittests/Target/PathMappingListTest.cpp 
b/lldb/unittests/Target/PathMappingListTest.cpp
index 66fd97c17f624..90b6f1134a2b6 100644
--- a/lldb/unittests/Target/PathMappingListTest.cpp
+++ b/lldb/unittests/Target/PathMappingListTest.cpp
@@ -6,9 +6,9 @@
 //
 
//===--===//
 
-#include "llvm/ADT/ArrayRef.h"
 #include "lldb/Target/PathMappingList.h"
 #include "lldb/Utility/FileSpec.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "gtest/gtest.h"
 #include 
 
@@ -19,6 +19,8 @@ struct Matches {
   FileSpec original;
   FileSpec remapped;
   Matches(const char *o, const char *r) : original(o), remapped(r) {}
+  Matches(const char *o, llvm::sys::path::Style style, const char *r)
+  : original(o, style), remapped(r) {}
 };
 } // namespace
 
@@ -112,3 +114,27 @@ TEST(PathMappingListTest, RemapRoot) {
   };
   TestPathMappings(map, matches, fails);
 }
+
+#ifndef _WIN32
+TEST(PathMappingListTest, CrossPlatformTests) {
+  PathMappingList map;
+  map.Append(ConstString(R"(C:\old)"), ConstString("/new"), false);
+  Matches matches[] = {
+{R"(C:\old)", llvm::sys::path::Style::windows, "/new"},
+{R"(C:\old\)", llvm::sys::path::Style::windows, "/new"},
+{R"(C:\old\foo\.)", llvm::sys::path::Style::windows, "/new/foo"},
+{R"(C:\old\foo.c)", llvm::sys::path::Style::windows, "/new/foo.c"},
+{R"(C:\old\foo.c\.)", llvm::sys::path::Style::windows, "/new/foo.c"},
+{R"(C:\old\.\foo.c)", llvm::sys::path::Style::windows, "/new/foo.c"},
+  };
+  ConstString fails[] = {
+ConstString("/foo"),
+ConstString("/"),
+ConstString("foo.c"),
+ConstString("./foo.c"),
+ConstString("../foo.c"),
+ConstString("../bar/foo.c"),
+  };
+  TestPathMappings(map, matches, fails);
+}
+#endif



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

[Lldb-commits] [PATCH] D103210: [lldb] Introduce Language::MethodNameInfo

2021-06-29 Thread Alex Langford via Phabricator via lldb-commits

bulbazord abandoned this revision.
bulbazord added a comment.

Not going with this change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103210

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

[Lldb-commits] [PATCH] D104914: [lldb] Correct format of qMemTags type field

2021-06-29 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

omjavaid added inline comments.



Comment at: 
lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py:22
 """ Test that qMemTags packets are parsed correctly and/or rejected. 
"""
 
 self.build()

Missing isAArch64MTE check here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104914

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

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

2021-06-29 Thread Greg Clayton via Phabricator via lldb-commits

clayborg created this revision.
clayborg added reviewers: labath, aprantl, JDevlieghere, jingham, wallace.
Herald added subscribers: kristof.beyls, emaste.
clayborg requested review of this revision.
Herald added subscribers: lldb-commits, MaskRay.
Herald added a project: LLDB.

This fix was created after profiling the target creation of a large C/C++/ObjC 
application that contained almost 4,000,000 redacted symbol names. The symbol 
table parsing code was creating names for each of these synthetic symbols and 
adding them to the name indexes. The code was also adding the object file 
basename to the end of the symbol name which doesn't allow symbols from 
different shared libraries to share the names in the constant string pool.

Prior to this fix this was creating 180MB of "___lldb_unnamed_symbol" symbol 
names and was taking a long time to generate each name, add them to the string 
pool and then add each of these names to the name index.

This patch fixes the issue by:

- not adding a name to synthetic symbols at creation time, and allows name to 
be dynamically generated when accessed
- doesn't add synthetic symbol names to the name indexes, but catches this 
special case as name lookup time. Users won't typically set breakpoints or 
lookup these synthetic names, but support was added to do the lookup in case it 
does happen
- removes the object file baseanme from the generated names to allow the names 
to be shared in the constant string pool

Prior to this fix the startup times for a large application was:
12.5 seconds (cold file caches)
8.5 seconds (warm file caches)

After this fix:
9.7 seconds (cold file caches)
5.7 seconds (warm file caches)

The names of the symbols are auto generated by appending the symbol's UserID to 
the end of the "___lldb_unnamed_symbol" string and is only done when the name 
is requested from a synthetic symbol if it has no name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105160

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
  lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
  lldb/test/Shell/SymbolFile/Breakpad/symtab.test

Index: lldb/test/Shell/SymbolFile/Breakpad/symtab.test
===
--- lldb/test/Shell/SymbolFile/Breakpad/symtab.test
+++ lldb/test/Shell/SymbolFile/Breakpad/symtab.test
@@ -5,7 +5,7 @@
 # CHECK-LABEL: (lldb) image dump symtab symtab.out
 # CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 5:
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
-# CHECK: [0]  0  SX Code0x00400x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}$$symtab.out
+# CHECK: [0]  0  SX Code0x00400x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}
 # CHECK: [1]  0   X Code0x004000b00x000c 0x f1_func
 # CHECK: [2]  0   X Code0x004000a00x000d 0x func_only
 # CHECK: [3]  0   X Code0x004000c00x0010 0x f2
Index: lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
+++ lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
@@ -3,8 +3,8 @@
 
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
 # CHECK: [0]  1 SourceFile  0x0x 0x0004 -
-# CHECK: [1]  2  SX Code0x002011800x0010 0x ___lldb_unnamed_symbol1$${{.*}}
-# CHECK: [2]  3  SX Code0x002011900x0006 0x ___lldb_unnamed_symbol2$${{.*}}
+# CHECK: [1]  2  SX Code0x002011800x0010 0x ___lldb_unnamed_symbol{{[0-9]*}}
+# CHECK: [2]  3  SX Code0x002011900x0006 0x ___lldb_unnamed_symbol{{[0-9]*}}
 
 --- !ELF
 FileHeader:
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -301,7 +301,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

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

2021-06-29 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

@stella.stamenova all test suite failures should be fixed now. This is a redo 
on https://reviews.llvm.org/D104488 which was reverted in less that a day...




Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2829
+/*section_sp=*/section_sp,
+/*offset=*/0,
+/*size=*/0, // FDE can span multiple symbols so don't use its size.

This is a bug that I tried to fix by putting "entry_point_addr.GetOffset()" 
instead of zero here. Restoring to zero to make sure that:

  lldb-shell :: SymbolFile/dissassemble-entry-point.s
  lldb-unit :: 
ObjectFile/ELF/./ObjectFileELFTests.exe/ObjectFileELFTest.GetSymtab_NoSymEntryPointArmThumbAddressClass

don't fail. I will fix this bug in another patch.



Comment at: lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml:6-7
 # CHECK: [0]  1 SourceFile  0x 
   0x 0x0004 -
-# CHECK: [1]  2  SX Code0x00201180 
   0x0010 0x ___lldb_unnamed_symbol1$${{.*}}
-# CHECK: [2]  3  SX Code0x00201190 
   0x0006 0x ___lldb_unnamed_symbol2$${{.*}}
+# CHECK: [1]  2  SX Code0x00201180 
   0x0010 0x ___lldb_unnamed_symbol{{[0-9]*}}
+# CHECK: [2]  3  SX Code0x00201190 
   0x0006 0x ___lldb_unnamed_symbol{{[0-9]*}}
 

This fixes the following previously failing test:

  lldb-shell :: ObjectFile/ELF/eh_frame-symbols.yaml




Comment at: lldb/test/Shell/SymbolFile/Breakpad/symtab.test:8
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address
   Size   Flags  Name
-# CHECK: [0]  0  SX Code0x0040 
   0x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}$$symtab.out
+# CHECK: [0]  0  SX Code0x0040 
   0x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}
 # CHECK: [1]  0   X Code0x004000b0 
   0x000c 0x f1_func

This fixes the following previously failing test:

  lldb-shell :: SymbolFile/Breakpad/symtab.test



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105160

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

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

2021-06-29 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

In D105160#2848805 , @clayborg wrote:

> @stella.stamenova all test suite failures should be fixed now. This is a redo 
> on https://reviews.llvm.org/D104488 which was reverted in less that a day...

@clayborg : Thanks! We ended up with several more failing tests because of 
issues that were committed while the bot was red and was not sending 
notifications. It's always good to keep things green, so that issues don't pile 
up. I'm sorry I didn't get to it sooner.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105160

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

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

2021-06-29 Thread walter erquinigo via Phabricator via lldb-commits

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

Crossing fingers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105160

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

[Lldb-commits] [PATCH] D105034: [lldb/lua] Add scripted watchpoints for Lua

2021-06-29 Thread Pedro Tammela via Phabricator via lldb-commits

tammela added a comment.

Clang format complained a bit, please fix those.




Comment at: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test:15
+# CHECK: val=nil
+# CHECK: Process {{[0-9]+}} exited

More tests cases would be nice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105034

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

[Lldb-commits] [lldb] c8164d0 - Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-29 Thread Greg Clayton via lldb-commits


Author: Greg Clayton
Date: 2021-06-29T17:44:33-07:00
New Revision: c8164d0276b97679e80db01adc860271ab4a5d11

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

LOG: Create synthetic symbol names on demand to improve memory consumption and 
startup times.

This fix was created after profiling the target creation of a large C/C++/ObjC 
application that contained almost 4,000,000 redacted symbol names. The symbol 
table parsing code was creating names for each of these synthetic symbols and 
adding them to the name indexes. The code was also adding the object file 
basename to the end of the symbol name which doesn't allow symbols from 
different shared libraries to share the names in the constant string pool.

Prior to this fix this was creating 180MB of "___lldb_unnamed_symbol" symbol 
names and was taking a long time to generate each name, add them to the string 
pool and then add each of these names to the name index.

This patch fixes the issue by:
- not adding a name to synthetic symbols at creation time, and allows name to 
be dynamically generated when accessed
- doesn't add synthetic symbol names to the name indexes, but catches this 
special case as name lookup time. Users won't typically set breakpoints or 
lookup these synthetic names, but support was added to do the lookup in case it 
does happen
- removes the object file baseanme from the generated names to allow the names 
to be shared in the constant string pool

Prior to this fix the startup times for a large application was:
12.5 seconds (cold file caches)
8.5 seconds (warm file caches)

After this fix:
9.7 seconds (cold file caches)
5.7 seconds (warm file caches)

The names of the symbols are auto generated by appending the symbol's UserID to 
the end of the "___lldb_unnamed_symbol" string and is only done when the name 
is requested from a synthetic symbol if it has no name.

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

Added: 


Modified: 
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
lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
lldb/test/Shell/SymbolFile/Breakpad/symtab.test

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index 1e29cf53b78b3..dc83565c7db52 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -712,8 +712,6 @@ class ObjectFile : public 
std::enable_shared_from_this,
   /// false otherwise.
   bool SetModulesArchitecture(const ArchSpec &new_arch);
 
-  ConstString GetNextSyntheticSymbolName();
-
   static lldb::DataBufferSP MapFileData(const FileSpec &file, uint64_t Size,
 uint64_t Offset);
 

diff  --git a/lldb/include/lldb/Symbol/Symbol.h 
b/lldb/include/lldb/Symbol/Symbol.h
index 3abe3114863de..be3e8abefa490 100644
--- a/lldb/include/lldb/Symbol/Symbol.h
+++ b/lldb/include/lldb/Symbol/Symbol.h
@@ -113,14 +113,20 @@ class Symbol : public SymbolContextScope {
   lldb::LanguageType GetLanguage() const {
 // TODO: See if there is a way to determine the language for a symbol
 // somehow, for now just return our best guess
-return m_mangled.GuessLanguage();
+return GetMangled().GuessLanguage();
   }
 
   void SetID(uint32_t uid) { m_uid = uid; }
 
-  Mangled &GetMangled() { return m_mangled; }
+  Mangled &GetMangled() {
+SynthesizeNameIfNeeded();
+return m_mangled;
+  }
 
-  const Mangled &GetMangled() const { return m_mangled; }
+  const Mangled &GetMangled() const {
+SynthesizeNameIfNeeded();
+return m_mangled;
+  }
 
   ConstString GetReExportedSymbolName() const;
 
@@ -166,9 +172,9 @@ class Symbol : public SymbolContextScope {
   bool IsTrampoline() const;
 
   bool IsIndirect() const;
-  
+
   bool IsWeak() const { return m_is_weak; }
-  
+
   void SetIsWeak (bool b) { m_is_weak = b; }
 
   bool GetByteSizeIsValid() const { return m_size_is_valid; }
@@ -223,6 +229,10 @@ class Symbol : public SymbolContextScope {
 
   bool ContainsFileAddress(lldb::addr_t file_addr) const;
 
+  static llvm::StringRef GetSyntheticSymbolPrefix() {
+return "___lldb_unnamed_symbol";
+  }
+
 protected:
   // This is the internal guts of ResolveReExportedSymbol, it assumes
   // reexport_name is not null, and that module_spec is valid.  We track the
@@ -233,6 +243,8 @@ class Symbol : public SymbolContextScope {
   lldb_private::ModuleSpec &module_spec,
   lldb_private::ModuleList &seen_modules) const;
 
+  void Syn

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

2021-06-29 Thread Greg Clayton 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 rGc8164d0276b9: Create synthetic symbol names on demand to 
improve memory consumption and… (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105160

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
  lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
  lldb/test/Shell/SymbolFile/Breakpad/symtab.test

Index: lldb/test/Shell/SymbolFile/Breakpad/symtab.test
===
--- lldb/test/Shell/SymbolFile/Breakpad/symtab.test
+++ lldb/test/Shell/SymbolFile/Breakpad/symtab.test
@@ -5,7 +5,7 @@
 # CHECK-LABEL: (lldb) image dump symtab symtab.out
 # CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 5:
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
-# CHECK: [0]  0  SX Code0x00400x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}$$symtab.out
+# CHECK: [0]  0  SX Code0x00400x00b0 0x ___lldb_unnamed_symbol{{[0-9]*}}
 # CHECK: [1]  0   X Code0x004000b00x000c 0x f1_func
 # CHECK: [2]  0   X Code0x004000a00x000d 0x func_only
 # CHECK: [3]  0   X Code0x004000c00x0010 0x f2
Index: lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
===
--- lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
+++ lldb/test/Shell/ObjectFile/ELF/eh_frame-symbols.yaml
@@ -3,8 +3,8 @@
 
 # CHECK: Index   UserID DSX TypeFile Address/Value Load Address   Size   Flags  Name
 # CHECK: [0]  1 SourceFile  0x0x 0x0004 -
-# CHECK: [1]  2  SX Code0x002011800x0010 0x ___lldb_unnamed_symbol1$${{.*}}
-# CHECK: [2]  3  SX Code0x002011900x0006 0x ___lldb_unnamed_symbol2$${{.*}}
+# CHECK: [1]  2  SX Code0x002011800x0010 0x ___lldb_unnamed_symbol{{[0-9]*}}
+# CHECK: [2]  3  SX Code0x002011900x0006 0x ___lldb_unnamed_symbol{{[0-9]*}}
 
 --- !ELF
 FileHeader:
Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -301,7 +301,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
@@ -628,6 +628,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;
+  index

[Lldb-commits] [lldb] 43f6dad - Fix buildbot compile error for https://reviews.llvm.org/D105160.

2021-06-29 Thread Greg Clayton via lldb-commits


Author: Greg Clayton
Date: 2021-06-29T18:03:25-07:00
New Revision: 43f6dad2344247976d5777f56a1fc29e39c6c717

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

LOG: Fix buildbot compile error for https://reviews.llvm.org/D105160.

Added: 


Modified: 
lldb/source/Symbol/Symtab.cpp

Removed: 




diff  --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index d859d8e25129..89e75c28cb9b 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -645,7 +645,7 @@ uint32_t Symtab::GetNameIndexes(ConstString symbol_name,
 return 0; // Not a synthetic symbol name
 
   // Extract the user ID from the symbol name
-  user_id_t uid = 0;
+  unsigned long long uid = 0;
   if (getAsUnsignedInteger(name, /*Radix=*/10, uid))
 return 0; // Failed to extract the user ID as an integer
   Symbol *symbol = FindSymbolByID(uid);



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

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

2021-06-29 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan created this revision.
yinghuitan added reviewers: clayborg, wallace.
Herald added a subscriber: jfb.
yinghuitan requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

VScode now sends a "scopes" DAP request immediately after any expression 
evaluation. 
This scopes request would clear and invalidate any non-scoped expandable 
variables in g_vsc.variables, causing later "variables" request to return empty 
result.
The symptom is that any expandable variables in VScode watch window/debug 
console UI to return empty content.

This diff fixes this issue by only clearing the expandable variables at process 
continue time. To achieve this, we have to repopulate all scoped variables 
during context switch for each "scopes" request without clearing global 
expandable variables. 
So the PR puts the scoped variables into its own locals/globals/registers; and 
all expandable variables into separate "expandableVariables" list.
Also, instead of using the variable index for "variableReference", it generates 
a new variableReference id each time as the key of "expandableVariables".

As a further new feature, this PR adds a new "expandablePermanentVariables" 
which has the lifetime of debug session. Any expandable variables from debug 
console
are added into this list. This enables users to snapshot expanable old variable 
in debug console and compare with new variables if desire.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105166

Files:
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -107,6 +107,25 @@
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
+// Debuggee will continue from stopped state.
+void willContinue() {
+  g_vsc.variableHolder.clear();
+}
+
+lldb::SBValueList* getScopeRef(int64_t variablesReference) {
+  assert(VARREF_IS_SCOPE(variablesReference));
+  switch (variablesReference) {
+case VARREF_LOCALS:
+  return &g_vsc.variableHolder.locals;
+case VARREF_GLOBALS:
+  return &g_vsc.variableHolder.globals;
+case VARREF_REGS:
+  return &g_vsc.variableHolder.registers;
+default:
+  return nullptr;
+}
+}
+
 SOCKET AcceptConnection(int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
@@ -727,6 +746,7 @@
   // Remember the thread ID that caused the resume so we can set the
   // "threadCausedFocus" boolean value in the "stopped" events.
   g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+  willContinue();
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);
@@ -1215,9 +1235,8 @@
   EmplaceSafeString(body, "type",
 value_typename ? value_typename : NO_TYPENAME);
   if (value.MightHaveChildren()) {
-auto variablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
-g_vsc.variables.Append(value);
-body.try_emplace("variablesReference", variablesReference);
+auto variableReference = g_vsc.variableHolder.insertNewExpandableVariable(value, /*isPermanent*/context == "repl");
+body.try_emplace("variablesReference", variableReference);
   } else {
 body.try_emplace("variablesReference", (int64_t)0);
   }
@@ -1769,6 +1788,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+willContinue();
 thread.StepOver();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -1895,20 +1915,15 @@
 frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
 frame.GetThread().SetSelectedFrame(frame.GetFrameID());
   }
-  g_vsc.variables.Clear();
-  g_vsc.variables.Append(frame.GetVariables(true,   // arguments
+  g_vsc.variableHolder.locals = frame.GetVariables(true,   // arguments
 true,   // locals
 false,  // statics
-true)); // in_scope_only
-  g_vsc.num_locals = g_vsc.variables.GetSize();
-  g_vsc.variables.Append(frame.GetVariables(false,  // arguments
+true); // in_scope_only
+  g_vsc.variableHolder.globals = frame.GetVariables(false,  // arguments
 false,  // locals
 true,   // statics
-true)); // in_scope_only
-  g_vsc.num_globals = g_vsc.variables.GetSize() - (g_vsc.num_locals);
-  g_vsc.variables.Append(frame.GetReg

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

2021-06-29 Thread Greg Clayton via Phabricator via lldb-commits

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

Mostly LLDB and LLVM coding guideline conventions that need to be followed. 
Many comments and code suggestions. But there are some code changes like having 
unique IDs for temp variable references and permanent ones.

Main issues are:

- LLVM coding guidelines can't go over 80 columns, must wrap. If you use 
Phabricator it would fix these for you
- LLDB variable names are lower case with underscores between words 
("isPermanent" should be "is_permament")
- LLDB methods are camel case and start with upper case letter 
("getNewVariableReference" should be "GetNewVariableReference")
- LLVM coding guidelines want no braces on single line if/else
- LLVM coding guidelines for inline variable names are to use inline C comments 
with equal sign after variable name: "/*is_permanent=*/true"




Comment at: lldb/tools/lldb-vscode/VSCode.cpp:33
 VSCode::VSCode()
-: variables(), broadcaster("lldb-vscode"), num_regs(0), num_locals(0),
-  num_globals(0), log(),
+: broadcaster("lldb-vscode"), log(),
   exception_breakpoints(

as long as we are changing this, we don't need to default construct anything



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:384-387
+  scopes.emplace_back(CreateScope("Locals", VARREF_LOCALS, 
g_vsc.variableHolder.locals.GetSize(), false));
   scopes.emplace_back(
-  CreateScope("Globals", VARREF_GLOBALS, num_globals, false));
-  scopes.emplace_back(CreateScope("Registers", VARREF_REGS, num_regs, false));
+  CreateScope("Globals", VARREF_GLOBALS, 
g_vsc.variableHolder.globals.GetSize(), false));
+  scopes.emplace_back(CreateScope("Registers", VARREF_REGS, 
g_vsc.variableHolder.registers.GetSize(), false));

over 80 character column limit



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:529
 
+void VariablesHolder::clear() {
+  this->locals.Clear();

LLDB function naming is camel case with upper case first letter.



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:530-533
+  this->locals.Clear();
+  this->globals.Clear();
+  this->registers.Clear();
+  this->expandableVariables.clear();





Comment at: lldb/tools/lldb-vscode/VSCode.cpp:537-539
+  const int64_t newVariableReference = isPermanent ? ((1ll << 
PermanentVariableReferenceTagBit) | nextVariableReferenceSeed) : 
nextVariableReferenceSeed;
+  ++nextVariableReferenceSeed;
+  return newVariableReference;

80 columns max and use separate var ref indexes



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:542-544
+bool VariablesHolder::isPermanentVariableReference(int64_t variableReference) {
+  return (variableReference & (1ll << PermanentVariableReferenceTagBit)) != 0;
+}

fix LLDB coding guidelines



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:546
+
+lldb::SBValue VariablesHolder::getVariableFromVariableReference(int64_t value) 
{
+  lldb::SBValue variable;





Comment at: lldb/tools/lldb-vscode/VSCode.cpp:548-553
+  auto isPermanent = isPermanentVariableReference(value);
+  if (isPermanent) {
+variable = expandablePermanentVariables.find(value)->second;
+  } else {
+variable = expandableVariables.find(value)->second;
+  }

Follow LLVM coding guidelines where there are no braces on single statement 
if/else, lldb var names



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:557-565
+int64_t VariablesHolder::insertNewExpandableVariable(lldb::SBValue 
expandableVariable, bool isPermanent) {
+  auto newVariablesReferences = getNewVariableRefence(isPermanent);
+  if (isPermanent) {
+expandablePermanentVariables.insert(std::make_pair(newVariablesReferences, 
expandableVariable));
+  } else {
+expandableVariables.insert(std::make_pair(newVariablesReferences, 
expandableVariable));
+  }

LLDB coding guidelines and 80 column limit



Comment at: lldb/tools/lldb-vscode/VSCode.h:84
 
+struct VariablesHolder {
+  // Bit tag bit to tell if a variableReference is inside 
expandablePermanentVariables or not.

Rename



Comment at: lldb/tools/lldb-vscode/VSCode.h:86
+  // Bit tag bit to tell if a variableReference is inside 
expandablePermanentVariables or not.
+  static constexpr int PermanentVariableReferenceTagBit = 32;
+

We should just define this as a mask instead of a bit number as we always use 
it as a mask.



Comment at: lldb/tools/lldb-vscode/VSCode.h:92
+
+  int64_t nextVariableReferenceSeed{VARREF_FIRST_VAR_IDX};
+  llvm::DenseMap expandableVariables;

LLDB naming conventions, and we should have two IDs, one for permanent and one 
for temp



Comment at: lldb/tools/lldb-vscode/VSCode.h:93
+  int64

[Lldb-commits] [PATCH] D105034: [lldb/lua] Add scripted watchpoints for Lua

2021-06-29 Thread Siger Young via Phabricator via lldb-commits

siger-young updated this revision to Diff 355428.
siger-young added a comment.

This patch mainly adds two extra tests for Lua scripted watchpoints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105034

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test


Index: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -6,10 +6,28 @@
 r
 watchpoint set variable val
 watchpoint command add -s lua
-print("val="..tostring(frame:FindVariable("val"):GetValue()))
+print("val=" .. tostring(frame:FindVariable("val"):GetValue()))
 quit
 c
 # CHECK: val=1
 # CHECK: val=2
-# CHECK: val=nil
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set variable val
+watchpoint modify 1 -c "(val == 1)"
+watchpoint command add -s lua
+print("conditional watchpoint")
+wp:SetEnabled(false)
+quit
+c
+# CHECK: conditional watchpoint
+# CHECK-NOT: conditional watchpoint
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set expr 0x00
+watchpoint command add -s lua
+print("never triggers")
+quit
+c
+# CHECK-NOT: never triggers
 # CHECK: Process {{[0-9]+}} exited
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -117,8 +117,8 @@
   io_handler.SetIsDone(true);
 } break;
 case eIOHandlerWatchpoint: {
-  auto *wp_options = static_cast(
-  io_handler.GetUserData());
+  auto *wp_options =
+  static_cast(io_handler.GetUserData());
   m_script_interpreter.SetWatchpointCommandCallback(wp_options,
 data.c_str());
   io_handler.SetIsDone(true);
@@ -304,9 +304,8 @@
   debugger.GetScriptInterpreter(true, eScriptLanguageLua));
   Lua &lua = lua_interpreter->GetLua();
 
-  llvm::Expected BoolOrErr = lua.CallWatchpointCallback(baton,
-  stop_frame_sp,
-  wp_sp);
+  llvm::Expected BoolOrErr =
+  lua.CallWatchpointCallback(baton, stop_frame_sp, wp_sp);
   if (llvm::Error E = BoolOrErr.takeError()) {
 debugger.GetErrorStream() << toString(std::move(E));
 return true;
@@ -325,8 +324,7 @@
 }
 
 void ScriptInterpreterLua::CollectDataForWatchpointCommandCallback(
-WatchpointOptions *wp_options,
-CommandReturnObject &result) {
+WatchpointOptions *wp_options, CommandReturnObject &result) {
   IOHandlerSP io_handler_sp(
   new IOHandlerLuaInterpreter(m_debugger, *this, eIOHandlerWatchpoint));
   io_handler_sp->SetUserData(wp_options);


Index: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -6,10 +6,28 @@
 r
 watchpoint set variable val
 watchpoint command add -s lua
-print("val="..tostring(frame:FindVariable("val"):GetValue()))
+print("val=" .. tostring(frame:FindVariable("val"):GetValue()))
 quit
 c
 # CHECK: val=1
 # CHECK: val=2
-# CHECK: val=nil
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set variable val
+watchpoint modify 1 -c "(val == 1)"
+watchpoint command add -s lua
+print("conditional watchpoint")
+wp:SetEnabled(false)
+quit
+c
+# CHECK: conditional watchpoint
+# CHECK-NOT: conditional watchpoint
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set expr 0x00
+watchpoint command add -s lua
+print("never triggers")
+quit
+c
+# CHECK-NOT: never triggers
 # CHECK: Process {{[0-9]+}} exited
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -117,8 +117,8 @@
   io_handler.SetIsDone(true);
 } break;
 case eIOHandlerWatchpoint: {
-  auto *wp_options = static_cast(
-  io_handler.GetUserData());
+  auto *wp_options =
+  static_cast(io_handler.GetUserData());
   m_script_interpreter.SetWatchpointCommandCallback(wp_options,
 data.c_str());
   io_handler.SetIsDone(true);
@@ -304,9 +304,8 @@
   debugger.GetScriptInterpreter(true, eScriptLanguageLua));
   Lua &lua = lua_interpreter->GetLua();
 
-  llvm::Expected BoolOrErr = lua.CallWatchpointCa

[Lldb-commits] [PATCH] D105034: [lldb/lua] Add scripted watchpoints for Lua

2021-06-29 Thread Siger Young via Phabricator via lldb-commits

siger-young updated this revision to Diff 355429.
siger-young added a comment.

Fix some linting and add extra tests for Lua scripted watchpoints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105034

Files:
  lldb/bindings/lua/lua-swigsafecast.swig
  lldb/bindings/lua/lua-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
  lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
  lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

Index: lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
@@ -30,6 +30,11 @@
   return false;
 }
 
+extern "C" llvm::Expected LLDBSwigLuaWatchpointCallbackFunction(
+lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp) {
+  return false;
+}
+
 #if _MSC_VER
 #pragma warning (pop)
 #endif
Index: lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
===
--- lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -1,9 +1,33 @@
 # REQUIRES: lua
 # XFAIL: system-netbsd
-# RUN: echo "int main() { return 0; }" | %clang_host -x c - -o %t
+# RUN: echo "int main() { int val = 1; val++; return 0; }" | %clang_host -x c - -g -o %t
 # RUN: %lldb -s %s --script-language lua %t 2>&1 | FileCheck %s
 b main
 r
-watchpoint set expr 0x0
+watchpoint set variable val
 watchpoint command add -s lua
-# CHECK: error: This script interpreter does not support watchpoint callbacks
+print("val=" .. tostring(frame:FindVariable("val"):GetValue()))
+quit
+c
+# CHECK: val=1
+# CHECK: val=2
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set variable val
+watchpoint modify 1 -c "(val == 1)"
+watchpoint command add -s lua
+print("conditional watchpoint")
+wp:SetEnabled(false)
+quit
+c
+# CHECK: conditional watchpoint
+# CHECK-NOT: conditional watchpoint
+# CHECK: Process {{[0-9]+}} exited
+r
+watchpoint set expr 0x00
+watchpoint command add -s lua
+print("never triggers")
+quit
+c
+# CHECK-NOT: never triggers
+# CHECK: Process {{[0-9]+}} exited
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -9,6 +9,7 @@
 #ifndef liblldb_ScriptInterpreterLua_h_
 #define liblldb_ScriptInterpreterLua_h_
 
+#include "lldb/Breakpoint/WatchpointOptions.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
 #include "lldb/Utility/Status.h"
@@ -61,6 +62,10 @@
  lldb::user_id_t break_id,
  lldb::user_id_t break_loc_id);
 
+  static bool WatchpointCallbackFunction(void *baton,
+ StoppointCallbackContext *context,
+ lldb::user_id_t watch_id);
+
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
 
@@ -75,9 +80,16 @@
   std::vector &bp_options_vec,
   CommandReturnObject &result) override;
 
+  void
+  CollectDataForWatchpointCommandCallback(WatchpointOptions *wp_options,
+  CommandReturnObject &result) override;
+
   Status SetBreakpointCommandCallback(BreakpointOptions *bp_options,
   const char *command_body_text) override;
 
+  void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
+const char *command_body_text) override;
+
   Status SetBreakpointCommandCallbackFunction(
   BreakpointOptions *bp_options, const char *function_name,
   StructuredData::ObjectSP extra_args_sp) override;
@@ -89,6 +101,10 @@
   Status RegisterBreakpointCallback(BreakpointOptions *bp_options,
 const char *command_body_text,
 StructuredData::ObjectSP extra_args_sp);
+
+  Status RegisterWatchpointCallback(WatchpointOptions *wp_options,
+const char *command_body_text,
+StructuredData::ObjectSP extra_args_sp);
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterprete

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

2021-06-29 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan updated this revision to Diff 355435.
yinghuitan marked 37 inline comments as done.
yinghuitan added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105166

Files:
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -107,6 +107,19 @@
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
+lldb::SBValueList *GetTopLevelScope(int64_t variablesReference) {
+  switch (variablesReference) {
+  case VARREF_LOCALS:
+return &g_vsc.variables.locals;
+  case VARREF_GLOBALS:
+return &g_vsc.variables.globals;
+  case VARREF_REGS:
+return &g_vsc.variables.registers;
+  default:
+return nullptr;
+  }
+}
+
 SOCKET AcceptConnection(int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
@@ -727,6 +740,7 @@
   // Remember the thread ID that caused the resume so we can set the
   // "threadCausedFocus" boolean value in the "stopped" events.
   g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+  g_vsc.willContinue();
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);
@@ -1215,9 +1229,9 @@
   EmplaceSafeString(body, "type",
 value_typename ? value_typename : NO_TYPENAME);
   if (value.MightHaveChildren()) {
-auto variablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
-g_vsc.variables.Append(value);
-body.try_emplace("variablesReference", variablesReference);
+auto variableReference = g_vsc.variables.InsertNewExpandableVariable(
+value, /*is_permanent=*/context == "repl");
+body.try_emplace("variablesReference", variableReference);
   } else {
 body.try_emplace("variablesReference", (int64_t)0);
   }
@@ -1769,6 +1783,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepOver();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -1895,20 +1910,15 @@
 frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
 frame.GetThread().SetSelectedFrame(frame.GetFrameID());
   }
-  g_vsc.variables.Clear();
-  g_vsc.variables.Append(frame.GetVariables(true,   // arguments
-true,   // locals
-false,  // statics
-true)); // in_scope_only
-  g_vsc.num_locals = g_vsc.variables.GetSize();
-  g_vsc.variables.Append(frame.GetVariables(false,  // arguments
-false,  // locals
-true,   // statics
-true)); // in_scope_only
-  g_vsc.num_globals = g_vsc.variables.GetSize() - (g_vsc.num_locals);
-  g_vsc.variables.Append(frame.GetRegisters());
-  g_vsc.num_regs =
-  g_vsc.variables.GetSize() - (g_vsc.num_locals + g_vsc.num_globals);
+  g_vsc.variables.locals = frame.GetVariables(/*arguments=*/true,
+  /*locals=*/true,
+  /*statics=*/false,
+  /*in_scope_only=*/true);
+  g_vsc.variables.globals = frame.GetVariables(/*arguments=*/false,
+   /*locals=*/false,
+   /*statics=*/true,
+   /*in_scope_only=*/true);
+  g_vsc.variables.registers = frame.GetRegisters();
   body.try_emplace("scopes", g_vsc.CreateTopLevelScopes());
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2523,6 +2533,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepInto();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -2575,6 +2586,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepOut();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -2750,37 +2762,19 @@
   // of the variable within the g_vsc.variables list.
   const auto id_val

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

2021-06-29 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan updated this revision to Diff 355436.
yinghuitan marked an inline comment as done.
yinghuitan added a comment.

Rerun linter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105166

Files:
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -107,6 +107,19 @@
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
+lldb::SBValueList *GetTopLevelScope(int64_t variablesReference) {
+  switch (variablesReference) {
+  case VARREF_LOCALS:
+return &g_vsc.variables.locals;
+  case VARREF_GLOBALS:
+return &g_vsc.variables.globals;
+  case VARREF_REGS:
+return &g_vsc.variables.registers;
+  default:
+return nullptr;
+  }
+}
+
 SOCKET AcceptConnection(int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
@@ -727,6 +740,7 @@
   // Remember the thread ID that caused the resume so we can set the
   // "threadCausedFocus" boolean value in the "stopped" events.
   g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+  g_vsc.willContinue();
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);
@@ -1215,9 +1229,9 @@
   EmplaceSafeString(body, "type",
 value_typename ? value_typename : NO_TYPENAME);
   if (value.MightHaveChildren()) {
-auto variablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
-g_vsc.variables.Append(value);
-body.try_emplace("variablesReference", variablesReference);
+auto variableReference = g_vsc.variables.InsertNewExpandableVariable(
+value, /*is_permanent=*/context == "repl");
+body.try_emplace("variablesReference", variableReference);
   } else {
 body.try_emplace("variablesReference", (int64_t)0);
   }
@@ -1769,6 +1783,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepOver();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -1895,20 +1910,15 @@
 frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
 frame.GetThread().SetSelectedFrame(frame.GetFrameID());
   }
-  g_vsc.variables.Clear();
-  g_vsc.variables.Append(frame.GetVariables(true,   // arguments
-true,   // locals
-false,  // statics
-true)); // in_scope_only
-  g_vsc.num_locals = g_vsc.variables.GetSize();
-  g_vsc.variables.Append(frame.GetVariables(false,  // arguments
-false,  // locals
-true,   // statics
-true)); // in_scope_only
-  g_vsc.num_globals = g_vsc.variables.GetSize() - (g_vsc.num_locals);
-  g_vsc.variables.Append(frame.GetRegisters());
-  g_vsc.num_regs =
-  g_vsc.variables.GetSize() - (g_vsc.num_locals + g_vsc.num_globals);
+  g_vsc.variables.locals = frame.GetVariables(/*arguments=*/true,
+  /*locals=*/true,
+  /*statics=*/false,
+  /*in_scope_only=*/true);
+  g_vsc.variables.globals = frame.GetVariables(/*arguments=*/false,
+   /*locals=*/false,
+   /*statics=*/true,
+   /*in_scope_only=*/true);
+  g_vsc.variables.registers = frame.GetRegisters();
   body.try_emplace("scopes", g_vsc.CreateTopLevelScopes());
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2523,6 +2533,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepInto();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -2575,6 +2586,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepOut();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -2750,37 +2762,19 @@
   // of the variable within the g_vsc.variables list.
   const auto id_value = GetUnsi

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

2021-06-29 Thread jeffrey tan via Phabricator via lldb-commits

yinghuitan updated this revision to Diff 355437.
yinghuitan added a comment.

rebase to latest master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105166

Files:
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -107,6 +107,19 @@
 
 enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch };
 
+lldb::SBValueList *GetTopLevelScope(int64_t variablesReference) {
+  switch (variablesReference) {
+  case VARREF_LOCALS:
+return &g_vsc.variables.locals;
+  case VARREF_GLOBALS:
+return &g_vsc.variables.globals;
+  case VARREF_REGS:
+return &g_vsc.variables.registers;
+  default:
+return nullptr;
+  }
+}
+
 SOCKET AcceptConnection(int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
@@ -727,6 +740,7 @@
   // Remember the thread ID that caused the resume so we can set the
   // "threadCausedFocus" boolean value in the "stopped" events.
   g_vsc.focus_tid = GetUnsigned(arguments, "threadId", LLDB_INVALID_THREAD_ID);
+  g_vsc.willContinue();
   lldb::SBError error = process.Continue();
   llvm::json::Object body;
   body.try_emplace("allThreadsContinued", true);
@@ -1215,9 +1229,9 @@
   EmplaceSafeString(body, "type",
 value_typename ? value_typename : NO_TYPENAME);
   if (value.MightHaveChildren()) {
-auto variablesReference = VARIDX_TO_VARREF(g_vsc.variables.GetSize());
-g_vsc.variables.Append(value);
-body.try_emplace("variablesReference", variablesReference);
+auto variableReference = g_vsc.variables.InsertNewExpandableVariable(
+value, /*is_permanent=*/context == "repl");
+body.try_emplace("variablesReference", variableReference);
   } else {
 body.try_emplace("variablesReference", (int64_t)0);
   }
@@ -1769,6 +1783,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepOver();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -1895,20 +1910,15 @@
 frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
 frame.GetThread().SetSelectedFrame(frame.GetFrameID());
   }
-  g_vsc.variables.Clear();
-  g_vsc.variables.Append(frame.GetVariables(true,   // arguments
-true,   // locals
-false,  // statics
-true)); // in_scope_only
-  g_vsc.num_locals = g_vsc.variables.GetSize();
-  g_vsc.variables.Append(frame.GetVariables(false,  // arguments
-false,  // locals
-true,   // statics
-true)); // in_scope_only
-  g_vsc.num_globals = g_vsc.variables.GetSize() - (g_vsc.num_locals);
-  g_vsc.variables.Append(frame.GetRegisters());
-  g_vsc.num_regs =
-  g_vsc.variables.GetSize() - (g_vsc.num_locals + g_vsc.num_globals);
+  g_vsc.variables.locals = frame.GetVariables(/*arguments=*/true,
+  /*locals=*/true,
+  /*statics=*/false,
+  /*in_scope_only=*/true);
+  g_vsc.variables.globals = frame.GetVariables(/*arguments=*/false,
+   /*locals=*/false,
+   /*statics=*/true,
+   /*in_scope_only=*/true);
+  g_vsc.variables.registers = frame.GetRegisters();
   body.try_emplace("scopes", g_vsc.CreateTopLevelScopes());
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
@@ -2523,6 +2533,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepInto();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -2575,6 +2586,7 @@
 // Remember the thread ID that caused the resume so we can set the
 // "threadCausedFocus" boolean value in the "stopped" events.
 g_vsc.focus_tid = thread.GetThreadID();
+g_vsc.willContinue();
 thread.StepOut();
   } else {
 response["success"] = llvm::json::Value(false);
@@ -2750,37 +2762,19 @@
   // of the variable within the g_vsc.variables list.
   const auto id_value = GetUnsigned(arguments, "id", UINT64_MAX);

[Lldb-commits] [PATCH] D104281: [lldb][docs] Add reference docs for Lua scripting

2021-06-29 Thread Siger Young via Phabricator via lldb-commits

siger-young updated this revision to Diff 355439.
siger-young added a comment.

Rebase two commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104281

Files:
  lldb/docs/conf.py
  lldb/docs/index.rst
  lldb/docs/use/lua-reference.rst
  lldb/docs/use/python-reference.rst
  lldb/docs/use/scripting-reference.rst

Index: lldb/docs/use/scripting-reference.rst
===
--- lldb/docs/use/scripting-reference.rst
+++ lldb/docs/use/scripting-reference.rst
@@ -1,23 +1,30 @@
-Python Reference
-
+Scripting Reference
+===
 
-The entire LLDB API is available as Python functions through a script bridging
-interface. This means the LLDB API's can be used directly from python either
-interactively or to build python apps that provide debugger features.
-
-Additionally, Python can be used as a programmatic interface within the lldb
-command interpreter (we refer to this for brevity as the embedded interpreter).
+Python can be used as a programmatic interface within the LLDB command
+interpreter (we refer to this for brevity as the embedded interpreter).
 Of course, in this context it has full access to the LLDB API - with some
 additional conveniences we will call out in the FAQ.
 
+Additionally, the entire LLDB API is available as Python functions through
+a script bridging interface. This means the LLDB API's can be used directly
+from Python either interactively or to build Python apps that provide debugger
+features.
+
+At present, Lua is experimentally available as an embedded interpreter in LLDB.
+Since some scripting features are still unavailable in Lua, there is no language
+switch tab on code blocks of unsupported features. If you want to use Lua
+interpreter, you should ensure that your LLDB is compiled with Lua support.
+See :doc:`Building ` for instructions. 
+
 .. contents::
:local:
 
-Documentation
---
+Built-in Documentation
+--
 
-The LLDB API is contained in a python module named lldb. A useful resource when
-writing Python extensions is the lldb Python classes reference guide.
+The LLDB API is contained in a Python module named ``lldb``. A useful resource
+when writing Python extensions is the `lldb` Python classes reference guide.
 
 The documentation is also accessible in an interactive debugger session with
 the following command:
@@ -54,7 +61,8 @@
   |
...
 
-Or you can get help using any python object, here we use the lldb.process
+
+Or you can get help using any Python object, here we use the lldb.process
 object which is a global variable in the lldb module which represents the
 currently selected process:
 
@@ -74,25 +82,53 @@
   |
...
 
-Embedded Python Interpreter
+Embedded Script Interpreter
 ---
 
-The embedded python interpreter can be accessed in a variety of ways from
-within LLDB. The easiest way is to use the lldb command script with no
-arguments at the lldb command prompt:
+The embedded script interpreter can be accessed in a variety of ways from
+within LLDB. The easiest way is to use the LLDB command ``script`` with no
+arguments at the LLDB command prompt.
+
+By default, the embedded script interpreter is Python. To choose the language
+you prefer, you can specify default language via ``--script-language`` options
+when launching LLDB:
+
+::
+
+   $ lldb --script-language lua
+
+or pass a parameter to LLDB command ``script``.
 
 ::
 
-   (lldb) script
-   Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
-   >>> 2+3
-   5
-   >>> hex(12345)
-   '0x3039'
-   >>>
+   (lldb) script -l lua --
+
+Language features are all available in the embedded script interpreter:
+
+.. tabs::
+   .. code-tab:: python
+
+  (lldb) script
+  Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
+  >>> 2+3
+  5
+  >>> hex(12345)
+  '0x3039'
+  >>>
+
+   .. code-tab:: lua
+  :emphasize-lines: 1
+
+  (lldb) script -l lua --
+  >>> t = { 1, 2, 3 }
+  >>> print(t[1])
+  1
+  >>> print(t[1] + t[2])
+  3
+  >>>
 
-This drops you into the embedded python interpreter. When running under the
-script command, lldb sets some convenience variables that give you quick access
+This drops you into the embedded script interpreter. When running under the
+script command, LLDB sets some convenience variables that give you quick access
 to the currently selected entities that characterize the program and debugger
 state. In each case, if there is no currently selected entity of the
 appropriate type, the variable's IsValid method will return false. These
@@ -138,60 +174,84 @@
 
 Moreover, they are only defined and meaningful while in the interactive Python
 interpreter. There is no guarantee on their value in any other situation, hence
-you should not use them when

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

2021-06-29 Thread Greg Clayton via Phabricator via lldb-commits

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:531
 
+void Variables::Clear() {
+  locals.Clear();





Comment at: lldb/tools/lldb-vscode/VSCode.cpp:535
+  registers.Clear();
+  expandable_variables.clear();
+}

Do we want to set "next_temporary_var_ref" to VARREF_FIRST_VAR_IDX here too? Ok 
if we don't.



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:538
+
+int64_t Variables::GetNewVariableRefence(bool is_permanent) {
+  if (is_permanent)

rename



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:549
+
+lldb::SBValue Variables::GetVariableFromVariableReference(int64_t var_ref) {
+  lldb::SBValue variable;

rename to "GetVariable" to help with 80 column limit and make const;



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:550-555
+  lldb::SBValue variable;
+  if (IsPermanentVariableReference(var_ref))
+variable = expandable_permanent_variables.find(var_ref)->second;
+  else
+variable = expandable_variables.find(var_ref)->second;
+  return variable;

We need to guard against the variable reference not existing in the map to make 
sure we never crash:



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:558
+
+int64_t Variables::InsertNewExpandableVariable(lldb::SBValue 
expandableVariable,
+   bool is_permanent) {

var name for LLDB and rename to shorter name.



Comment at: lldb/tools/lldb-vscode/VSCode.cpp:560
+   bool is_permanent) {
+  auto newVariablesReferences = GetNewVariableRefence(is_permanent);
+  if (is_permanent)

rename newVariablesReferences to "var_ref"



Comment at: lldb/tools/lldb-vscode/VSCode.h:94
+  int64_t next_permanent_var_ref{VARREF_FIRST_VAR_IDX};
+  llvm::DenseMap expandable_variables;
+  llvm::DenseMap expandable_permanent_variables;

rename to "temporary_variables" and add a comment



Comment at: lldb/tools/lldb-vscode/VSCode.h:95
+  llvm::DenseMap expandable_variables;
+  llvm::DenseMap expandable_permanent_variables;
+

rename to "permanent_variables" and add comment



Comment at: lldb/tools/lldb-vscode/VSCode.h:99
+  /// expandable_permanent_variables.
+  bool IsPermanentVariableReference(int64_t variableReference);
+

lldb variable name, and make static since it doesn't require any ivar access



Comment at: lldb/tools/lldb-vscode/VSCode.h:110
+  /// Insert a new \p expandableVariable.
+  int64_t InsertNewExpandableVariable(lldb::SBValue expandableVariable,
+  bool is_permanent);

Fix LLDB variable name



Comment at: lldb/tools/lldb-vscode/VSCode.h:114
+  /// Clear all scope variables and non-permanent expandable variables.
+  void Clear();
+};

clear sounds like you are clearing everything in the class. I would name this 
willContinue() like the others



Comment at: lldb/tools/lldb-vscode/VSCode.h:238
+  /// Debuggee will continue from stopped state.
+  void willContinue() { variables.Clear(); }
+

Correct camel case for LLDB function names


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105166

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

[Lldb-commits] [PATCH] D105166: Fix expression evaluation result expansion in lldb-vscode

2021-06-29 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

We will need to add tests for this as well to ensure it doesn't regress. Tests 
should be added to: 
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py

I would add a new test function into the TestVSCode_variables class:

  @skipIfWindows
  @skipIfRemote
  def test_scopes_and_evaluate(self):

You can probably use the same test program as 
test_scopes_variables_setVariable_evaluate. We will want to:

- send a "scopes" packet
- send a "evaluate" packet with some structure as the expression and "repl" as 
the source
- send a "scopes packet
- make sure we can expand the "evaluate" we did prior to running the second 
"scopes"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105166

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

53 matches

Mail list logo