[Lldb-commits] [PATCH] D120607: [lldb/test] Re-enable TestEvents.py on Darwin and fix crashes

2022-02-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added a reviewer: JDevlieghere.
mib added a project: LLDB.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch re-enables TestEvents.py on Darwin and fixes some crashes
that were happening due to an undefined method.

I ran it 100 times locally with the following command and it passed
every the time:

  for i in {1..100}; do print $i/100; ./bin/lldb-dotest -p TestEvents.py 2>&1 | 
rg PASSED; if [ "$?" -eq "1" ]; then break; fi; done

Let's see if it still fails non-deterministically on the bots and
eventually also re-enable it on linux.

rdar://37037235

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120607

Files:
  lldb/test/API/python_api/event/TestEvents.py


Index: lldb/test/API/python_api/event/TestEvents.py
===
--- lldb/test/API/python_api/event/TestEvents.py
+++ lldb/test/API/python_api/event/TestEvents.py
@@ -13,7 +13,6 @@
 
 
 @skipIfLinux   # llvm.org/pr25924, sometimes generating SIGSEGV
-@skipIfDarwin
 class EventAPITestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
@@ -171,9 +170,9 @@
 # Let's only try at most 3 times to retrieve any kind of event.
 while not count > 3:
 if listener.WaitForEvent(5, event):
-self.trace("Got a valid event:", event)
-self.trace("Event data flavor:", event.GetDataFlavor())
-self.trace("Event type:", 
lldbutil.state_type_to_str(event.GetType()))
+self.context.trace("Got a valid event:", event)
+self.context.trace("Event data flavor:", 
event.GetDataFlavor())
+self.context.trace("Event type:", 
lldbutil.state_type_to_str(event.GetType()))
 listener.Clear()
 return
 count = count + 1
@@ -187,6 +186,7 @@
 
 # Let's start the listening thread to retrieve the event.
 my_thread = MyListeningThread()
+my_thread.context = self
 my_thread.start()
 
 # Wait until the 'MyListeningThread' terminates.
@@ -256,7 +256,7 @@
 class MyListeningThread(threading.Thread):
 
 def run(self):
-self.trace("Running MyListeningThread:", self)
+self.context.trace("Running MyListeningThread:", self)
 
 # Regular expression pattern for the event description.
 pattern = re.compile("data = {.*, state = (.*)}$")
@@ -266,7 +266,7 @@
 while True:
 if listener.WaitForEvent(5, event):
 desc = lldbutil.get_description(event)
-self.trace("Event description:", desc)
+self.context.trace("Event description:", desc)
 match = pattern.search(desc)
 if not match:
 break


Index: lldb/test/API/python_api/event/TestEvents.py
===
--- lldb/test/API/python_api/event/TestEvents.py
+++ lldb/test/API/python_api/event/TestEvents.py
@@ -13,7 +13,6 @@
 
 
 @skipIfLinux   # llvm.org/pr25924, sometimes generating SIGSEGV
-@skipIfDarwin
 class EventAPITestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
@@ -171,9 +170,9 @@
 # Let's only try at most 3 times to retrieve any kind of event.
 while not count > 3:
 if listener.WaitForEvent(5, event):
-self.trace("Got a valid event:", event)
-self.trace("Event data flavor:", event.GetDataFlavor())
-self.trace("Event type:", lldbutil.state_type_to_str(event.GetType()))
+self.context.trace("Got a valid event:", event)
+self.context.trace("Event data flavor:", event.GetDataFlavor())
+self.context.trace("Event type:", lldbutil.state_type_to_str(event.GetType()))
 listener.Clear()
 return
 count = count + 1
@@ -187,6 +186,7 @@
 
 # Let's start the listening thread to retrieve the event.
 my_thread = MyListeningThread()
+my_thread.context = self
 my_thread.start()
 
 # Wait until the 'MyListeningThread' terminates.
@@ -256,7 +256,7 @@
 class MyListeningThread(threading.Thread):
 
 def run(self):
-self.trace("Running MyListeningThread:", self)
+self.context.trace("Running MyListeningThread:", self)
 
 # Regular expression pattern for the event description.
 pattern = re.compile("data = {.*, state = (.*)}$")
@@ -266,7 +266,7 @@
 while True:
 

[Lldb-commits] [lldb] 2dc6e90 - [lldb/Host] Fix crash in FileSystem::IsLocal

2022-02-25 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-02-25T18:33:31-08:00
New Revision: 2dc6e906b09deb092c15a726c06d0ecbeec1eb18

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

LOG: [lldb/Host] Fix crash in FileSystem::IsLocal

This checks `m_fs` before dereferencing it to access its`isLocal` method.

rdar://67410058

Signed-off-by: Med Ismail Bennani 

Added: 


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

Removed: 




diff  --git a/lldb/source/Host/common/FileSystem.cpp 
b/lldb/source/Host/common/FileSystem.cpp
index 1e4a24abe3017..26a98fa0a4ec4 100644
--- a/lldb/source/Host/common/FileSystem.cpp
+++ b/lldb/source/Host/common/FileSystem.cpp
@@ -192,7 +192,8 @@ bool FileSystem::IsDirectory(const FileSpec _spec) 
const {
 
 bool FileSystem::IsLocal(const Twine ) const {
   bool b = false;
-  m_fs->isLocal(path, b);
+  if (m_fs)
+m_fs->isLocal(path, b);
   return b;
 }
 



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


[Lldb-commits] [PATCH] D120599: Reland "[lldb/test] Fix TestProgressReporting.py race issue with the event listener"

2022-02-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG425880ed35ee: Reland [lldb/test] Fix 
TestProgressReporting.py race issue with the event… (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120599

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/source/API/SBDebugger.cpp
  lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py

Index: lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
===
--- lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -17,41 +17,42 @@
 TestBase.setUp(self)
 self.progress_events = []
 
-def fetch_events(self, test_broadcaster):
-listener = lldb.SBListener("lldb.progress.listener")
-listener.StartListeningForEvents(test_broadcaster,
- self.eBroadcastBitStopProgressThread)
-
-progress_broadcaster = self.dbg.GetBroadcaster()
-progress_broadcaster.AddListener(listener, lldb.SBDebugger.eBroadcastBitProgress)
-
+def fetch_events(self):
 event = lldb.SBEvent()
 
 done = False
 while not done:
-if listener.WaitForEvent(1, event):
+if self.listener.WaitForEvent(1, event):
 event_mask = event.GetType();
-if event.BroadcasterMatchesRef(test_broadcaster):
+if event.BroadcasterMatchesRef(self.test_broadcaster):
 if event_mask & self.eBroadcastBitStopProgressThread:
 done = True;
-elif event.BroadcasterMatchesRef(progress_broadcaster):
-message = lldb.SBDebugger().GetProgressFromEvent(event, 0, 0, 0, False);
+elif event.BroadcasterMatchesRef(self.progress_broadcaster):
+ret_args = lldb.SBDebugger().GetProgressFromEvent(event);
+self.assertGreater(len(ret_args), 1)
+
+message = ret_args[0]
 if message:
 self.progress_events.append((message, event))
 
-@skipUnlessDarwin
 def test_dwarf_symbol_loading_progress_report(self):
 """Test that we are able to fetch dwarf symbol loading progress events"""
 self.build()
 
-test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
-listener_thread = threading.Thread(target=self.fetch_events,
-   args=[test_broadcaster])
+self.listener = lldb.SBListener("lldb.progress.listener")
+self.test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
+self.listener.StartListeningForEvents(self.test_broadcaster,
+  self.eBroadcastBitStopProgressThread)
+
+self.progress_broadcaster = self.dbg.GetBroadcaster()
+self.progress_broadcaster.AddListener(self.listener, lldb.SBDebugger.eBroadcastBitProgress)
+
+listener_thread = threading.Thread(target=self.fetch_events)
 listener_thread.start()
 
 lldbutil.run_to_source_breakpoint(self, 'break here', lldb.SBFileSpec('main.c'))
 
-test_broadcaster.BroadcastEventByType(self.eBroadcastBitStopProgressThread)
+self.test_broadcaster.BroadcastEventByType(self.eBroadcastBitStopProgressThread)
 listener_thread.join()
 
-self.assertTrue(len(self.progress_events) > 0)
+self.assertGreater(len(self.progress_events), 0)
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -151,8 +151,7 @@
  uint64_t ,
  uint64_t ,
  bool _debugger_specific) {
-  LLDB_INSTRUMENT_VA(event, progress_id, completed, total,
- is_debugger_specific);
+  LLDB_INSTRUMENT_VA(event);
   const Debugger::ProgressEventData *progress_data =
   Debugger::ProgressEventData::GetEventDataFromEvent(event.get());
   if (progress_data == nullptr)
Index: lldb/bindings/interface/SBDebugger.i
===
--- lldb/bindings/interface/SBDebugger.i
+++ lldb/bindings/interface/SBDebugger.i
@@ -123,14 +123,11 @@
 };
 
 
-%apply uint64_t& INOUT { uint64_t& progress_id };
-%apply uint64_t& INOUT { uint64_t& completed };
-%apply uint64_t& INOUT { uint64_t& total };
-%apply bool& INOUT { bool& is_debugger_specific };
 static const char *GetProgressFromEvent(const lldb::SBEvent ,
-uint64_t _id,
-  

[Lldb-commits] [PATCH] D120598: [lldb/crashlog] Fix scripted_crashlog_json.test failure

2022-02-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbadb6e2730a9: [lldb/crashlog] Fix 
scripted_crashlog_json.test failure (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120598

Files:
  lldb/examples/python/crashlog.py
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips


Index: 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
===
--- 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
+++ 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
@@ -21,8 +21,8 @@
   "procExitAbsTime" : 6478056175721,
   "translated" : false,
   "cpuType" : "ARM-64",
-  "procName" : "scripted_crashlog_json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "zsh",
   "parentPid" : 82132,
   "coalitionName" : "com.apple.Terminal",
@@ -47,8 +47,9 @@
 "base" : 4372692992,
 "size" : 16384,
 "uuid" : "b928ee77-9429-334f-ac88-41440bb3d4c7",
-"path" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
-"name" : "scripted_crashlog_json.test.tmp.out"
+"uuid" : "@UUID@",
+"path" : "@EXEC@",
+"name" : "@NAME@"
   },
   {
 "source" : "P",
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
@@ -21,8 +21,8 @@
   "incident" : "FA21DF23-3344-4E45-BF27-4B8E63B7012B",
   "pid" : 72932,
   "cpuType" : "X86-64",
-  "procName" : "json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "fish",
   "parentPid" : 67002,
   "coalitionName" : "io.alacritty",
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -989,7 +989,10 @@
 result.PutCString("error: python exception: %s" % e)
 return
 
-target = crashlog.create_target()
+if debugger.GetNumTargets() > 0:
+target = debugger.GetTargetAtIndex(0)
+else:
+target = crashlog.create_target()
 if not target:
 result.PutCString("error: couldn't create target")
 return


Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
@@ -21,8 +21,8 @@
   "procExitAbsTime" : 6478056175721,
   "translated" : false,
   "cpuType" : "ARM-64",
-  "procName" : "scripted_crashlog_json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "zsh",
   "parentPid" : 82132,
   "coalitionName" : "com.apple.Terminal",
@@ -47,8 +47,9 @@
 "base" : 4372692992,
 "size" : 16384,
 "uuid" : "b928ee77-9429-334f-ac88-41440bb3d4c7",
-"path" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
-"name" : "scripted_crashlog_json.test.tmp.out"
+"uuid" : "@UUID@",
+"path" : "@EXEC@",
+"name" : "@NAME@"
   },
   {
 "source" : "P",
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
@@ -21,8 +21,8 @@
   "incident" : "FA21DF23-3344-4E45-BF27-4B8E63B7012B",
   "pid" : 72932,
   "cpuType" : "X86-64",
-  "procName" : "json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "fish",
   "parentPid" : 67002,
   "coalitionName" : "io.alacritty",
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -989,7 +989,10 @@
 result.PutCString("error: python exception: %s" % e)
 return
 
-target = crashlog.create_target()
+if debugger.GetNumTargets() > 0:
+target = debugger.GetTargetAtIndex(0)
+else:
+target = crashlog.create_target()
 if not target:
 result.PutCString("error: couldn't create target")
 return

[Lldb-commits] [lldb] 425880e - Reland "[lldb/test] Fix TestProgressReporting.py race issue with the event listener"

2022-02-25 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-02-25T17:20:39-08:00
New Revision: 425880ed35ee399d8f1af0aac60100e4901f289f

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

LOG: Reland "[lldb/test] Fix TestProgressReporting.py race issue with the event 
listener"

This patch relands commit 3e3e79a9e4c378b59f5f393f556e6a84edcd8898, and
fixes the memory sanitizer issue described in D120284, by removing the
output arguments from the LLDB_INSTRUMENT_VA invocation.

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/bindings/interface/SBDebugger.i
lldb/source/API/SBDebugger.cpp
lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py

Removed: 




diff  --git a/lldb/bindings/interface/SBDebugger.i 
b/lldb/bindings/interface/SBDebugger.i
index 3790857b8ab61..0ef1766a50c6b 100644
--- a/lldb/bindings/interface/SBDebugger.i
+++ b/lldb/bindings/interface/SBDebugger.i
@@ -123,14 +123,11 @@ public:
 };
 
 
-%apply uint64_t& INOUT { uint64_t& progress_id };
-%apply uint64_t& INOUT { uint64_t& completed };
-%apply uint64_t& INOUT { uint64_t& total };
-%apply bool& INOUT { bool& is_debugger_specific };
 static const char *GetProgressFromEvent(const lldb::SBEvent ,
-uint64_t _id,
-uint64_t , uint64_t ,
-bool _debugger_specific);
+uint64_t ,
+uint64_t ,
+uint64_t ,
+bool );
 
 SBBroadcaster GetBroadcaster();
 

diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 1582c538fa255..c3d1a9817c5e4 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -151,8 +151,7 @@ const char *SBDebugger::GetProgressFromEvent(const 
lldb::SBEvent ,
  uint64_t ,
  uint64_t ,
  bool _debugger_specific) {
-  LLDB_INSTRUMENT_VA(event, progress_id, completed, total,
- is_debugger_specific);
+  LLDB_INSTRUMENT_VA(event);
   const Debugger::ProgressEventData *progress_data =
   Debugger::ProgressEventData::GetEventDataFromEvent(event.get());
   if (progress_data == nullptr)

diff  --git 
a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py 
b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
index b9d9953539c11..79ef4e3f9f861 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -17,41 +17,42 @@ def setUp(self):
 TestBase.setUp(self)
 self.progress_events = []
 
-def fetch_events(self, test_broadcaster):
-listener = lldb.SBListener("lldb.progress.listener")
-listener.StartListeningForEvents(test_broadcaster,
- self.eBroadcastBitStopProgressThread)
-
-progress_broadcaster = self.dbg.GetBroadcaster()
-progress_broadcaster.AddListener(listener, 
lldb.SBDebugger.eBroadcastBitProgress)
-
+def fetch_events(self):
 event = lldb.SBEvent()
 
 done = False
 while not done:
-if listener.WaitForEvent(1, event):
+if self.listener.WaitForEvent(1, event):
 event_mask = event.GetType();
-if event.BroadcasterMatchesRef(test_broadcaster):
+if event.BroadcasterMatchesRef(self.test_broadcaster):
 if event_mask & self.eBroadcastBitStopProgressThread:
 done = True;
-elif event.BroadcasterMatchesRef(progress_broadcaster):
-message = lldb.SBDebugger().GetProgressFromEvent(event, 0, 
0, 0, False);
+elif event.BroadcasterMatchesRef(self.progress_broadcaster):
+ret_args = lldb.SBDebugger().GetProgressFromEvent(event);
+self.assertGreater(len(ret_args), 1)
+
+message = ret_args[0]
 if message:
 self.progress_events.append((message, event))
 
-@skipUnlessDarwin
 def test_dwarf_symbol_loading_progress_report(self):
 """Test that we are able to fetch dwarf symbol loading progress 
events"""
 self.build()
 
-test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
-listener_thread = threading.Thread(target=self.fetch_events,
-

[Lldb-commits] [lldb] badb6e2 - [lldb/crashlog] Fix scripted_crashlog_json.test failure

2022-02-25 Thread Med Ismail Bennani via lldb-commits

Author: Med Ismail Bennani
Date: 2022-02-25T17:20:35-08:00
New Revision: badb6e2730a970b5ee94f0adb77c2de741ebbf68

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

LOG: [lldb/crashlog] Fix scripted_crashlog_json.test failure

This patch should fix the test failure on scripted_crashlog_json.test.

The failure is happening because crash reporter will obfuscate the
executable path in the crashlog, if it is located inside the user's
home directory and replace it with `/USER/*/` as a placeholder.

To fix that, we can patch the placeholder with the executable path
before loading the crashlog in lldb.

This also fixes a bug where we would create another target when loading
the crashlog in a scripted process, even if lldb already had a target
for it. Now, crashlog will only create a target if there is none in lldb.

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/examples/python/crashlog.py
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 8c20fa71d3e2a..9a669b50d2523 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -989,7 +989,10 @@ def load_crashlog_in_scripted_process(debugger, 
crash_log_file):
 result.PutCString("error: python exception: %s" % e)
 return
 
-target = crashlog.create_target()
+if debugger.GetNumTargets() > 0:
+target = debugger.GetTargetAtIndex(0)
+else:
+target = crashlog.create_target()
 if not target:
 result.PutCString("error: couldn't create target")
 return

diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
index af72bba8c6d7c..cc2f16c016147 100644
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
+++ b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
@@ -21,8 +21,8 @@
   "incident" : "FA21DF23-3344-4E45-BF27-4B8E63B7012B",
   "pid" : 72932,
   "cpuType" : "X86-64",
-  "procName" : "json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "fish",
   "parentPid" : 67002,
   "coalitionName" : "io.alacritty",

diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
index 02dbbb51f4a66..5abbc80493355 100644
--- 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
+++ 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
@@ -21,8 +21,8 @@
   "procExitAbsTime" : 6478056175721,
   "translated" : false,
   "cpuType" : "ARM-64",
-  "procName" : "scripted_crashlog_json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "zsh",
   "parentPid" : 82132,
   "coalitionName" : "com.apple.Terminal",
@@ -47,8 +47,9 @@
 "base" : 4372692992,
 "size" : 16384,
 "uuid" : "b928ee77-9429-334f-ac88-41440bb3d4c7",
-"path" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
-"name" : "scripted_crashlog_json.test.tmp.out"
+"uuid" : "@UUID@",
+"path" : "@EXEC@",
+"name" : "@NAME@"
   },
   {
 "source" : "P",



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


[Lldb-commits] [PATCH] D120598: [lldb/crashlog] Fix scripted_crashlog_json.test failure

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120598

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


[Lldb-commits] [PATCH] D120599: Reland "[lldb/test] Fix TestProgressReporting.py race issue with the event listener"

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Ship it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120599

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


[Lldb-commits] [PATCH] D120595: [WIP][trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces

2022-02-25 Thread walter erquinigo via Phabricator via lldb-commits
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

you got the flow correct. I left comments regarding specific details, but good 
job overall!!




Comment at: lldb/docs/lldb-gdb-remote.txt:454-479
+//}],
+//... other parameters specific to the provided tracing type
 //  }
 //
 // NOTES
 //   - "traceThreads" includes all thread traced by both "process tracing" and
 // "thread tracing".

don't forget to f



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:51
+  int64_t m_time_shift;
+  int64_t m_time_zero;
+};

jj10306 wrote:
> What is the suggested way to serialize a u64 into JSON? I thought of two 
> approaches:
> 1. Store two separate u32
> 2. store the integer as a string
> 
> @wallace wdyt?
you don't need to use int64_t here. You can use the real types here and do the 
conversion in the fromJson and toJson methods



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:55
+/// TSC to nanosecond conversion.
+struct TscConverter {
+  virtual ~TscConverter() = default;

TscToWallTimeConverter is a better name



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:55
+/// TSC to nanosecond conversion.
+struct TscConverter {
+  virtual ~TscConverter() = default;

wallace wrote:
> TscToWallTimeConverter is a better name
use a class and not a struct. A struct has all its member public and we don't 
want that



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:58
+  /// Convert TSC value to nanoseconds.
+  virtual uint64_t TscToNanos(uint64_t tsc) = 0;
+  virtual llvm::json::Value toJSON() = 0;

ToNanos() is also okay. Up to you



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:63
+/// Conversion based on values define in perf_event_mmap_page.
+struct PerfTscConverter : TscConverter {
+  PerfTscConverter() = default;





Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:64-66
+  PerfTscConverter() = default;
+  PerfTscConverter(uint32_t time_mult, uint16_t time_shift, uint64_t 
time_zero) :
+m_perf_conversion{time_mult, time_shift, static_cast(time_zero)} 
{}

you can get rid of the constructors a



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:70
+
+  PerfTscConversionValues m_perf_conversion;
+};

you don't need this struct. You should just put the 3 fields here.



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:86
+  /// `nullptr` if no TscConverter exists.
+  std::shared_ptr tsc_converter;
+};

it's weird for a bag of data to contain a field transmitted by json called 
coverter. Just call it TimestampCounterRate. Because that's what it is. Then 
each TimestampCounterRate subclass can have a ToNanos(uint64_t tsc) method that 
converts to that.
Your documentation should specific if this ToNanos method returns the since the 
start of the computer, or since the start of the program, or anything else



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:89
+
+bool fromJSON(const llvm::json::Value , std::shared_ptr 
_converter, llvm::json::Path path);
+

Create a typedef in this file called TimestampCounterRateSP for readability. 
You can use tsc_rate as your variable names



Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.cpp:45
 
+std::shared_ptr IntelPTManager::tsc_converter = nullptr;
+

remove it from here



Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.cpp:257-259
+  if (m_mmap_meta->cap_user_time_zero) {
+IntelPTManager::tsc_converter = 
std::make_shared(PerfTscConverter(m_mmap_meta->time_mult, 
m_mmap_meta->time_shift, m_mmap_meta->time_zero));
+  }

notice the case is which g_tsc_rate is not None but nullptr. That means that 
the computation was performed but no rate was found, so the computation won't 
happen again. We have to make the code in that way because it's possible that 
checking for some rates is not a no-op. E.g. you can run a little 10ms program 
and calculate the difference in TSC to get an estimated rate.

Besides that, put all of this in a new function



Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.h:238
 
+  static std::shared_ptr tsc_converter;
+

static llvm::Optional g_tsc_rate; // by default this is 
None



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:22
 #include "llvm/ADT/None.h"
+#include 
 

jj10306 wrote:
> 
memory should be defined as a new include block between lines 9 and 11. That's 
just the pattern we 

[Lldb-commits] [PATCH] D120599: Reland "[lldb/test] Fix TestProgressReporting.py race issue with the event listener"

2022-02-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: labath, JDevlieghere, MForster.
mib added a project: LLDB.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch relands commit 3e3e79a9e4c378b59f5f393f556e6a84edcd8898 
, and
fixes the memory sanitizer issue described in D120284 
, by removing the
output arguments from the LLDB_INSTRUMENT_VA invocation.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120599

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/source/API/SBDebugger.cpp
  lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py

Index: lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
===
--- lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -17,41 +17,42 @@
 TestBase.setUp(self)
 self.progress_events = []
 
-def fetch_events(self, test_broadcaster):
-listener = lldb.SBListener("lldb.progress.listener")
-listener.StartListeningForEvents(test_broadcaster,
- self.eBroadcastBitStopProgressThread)
-
-progress_broadcaster = self.dbg.GetBroadcaster()
-progress_broadcaster.AddListener(listener, lldb.SBDebugger.eBroadcastBitProgress)
-
+def fetch_events(self):
 event = lldb.SBEvent()
 
 done = False
 while not done:
-if listener.WaitForEvent(1, event):
+if self.listener.WaitForEvent(1, event):
 event_mask = event.GetType();
-if event.BroadcasterMatchesRef(test_broadcaster):
+if event.BroadcasterMatchesRef(self.test_broadcaster):
 if event_mask & self.eBroadcastBitStopProgressThread:
 done = True;
-elif event.BroadcasterMatchesRef(progress_broadcaster):
-message = lldb.SBDebugger().GetProgressFromEvent(event, 0, 0, 0, False);
+elif event.BroadcasterMatchesRef(self.progress_broadcaster):
+ret_args = lldb.SBDebugger().GetProgressFromEvent(event);
+self.assertGreater(len(ret_args), 1)
+
+message = ret_args[0]
 if message:
 self.progress_events.append((message, event))
 
-@skipUnlessDarwin
 def test_dwarf_symbol_loading_progress_report(self):
 """Test that we are able to fetch dwarf symbol loading progress events"""
 self.build()
 
-test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
-listener_thread = threading.Thread(target=self.fetch_events,
-   args=[test_broadcaster])
+self.listener = lldb.SBListener("lldb.progress.listener")
+self.test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
+self.listener.StartListeningForEvents(self.test_broadcaster,
+  self.eBroadcastBitStopProgressThread)
+
+self.progress_broadcaster = self.dbg.GetBroadcaster()
+self.progress_broadcaster.AddListener(self.listener, lldb.SBDebugger.eBroadcastBitProgress)
+
+listener_thread = threading.Thread(target=self.fetch_events)
 listener_thread.start()
 
 lldbutil.run_to_source_breakpoint(self, 'break here', lldb.SBFileSpec('main.c'))
 
-test_broadcaster.BroadcastEventByType(self.eBroadcastBitStopProgressThread)
+self.test_broadcaster.BroadcastEventByType(self.eBroadcastBitStopProgressThread)
 listener_thread.join()
 
-self.assertTrue(len(self.progress_events) > 0)
+self.assertGreater(len(self.progress_events), 0)
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -151,8 +151,7 @@
  uint64_t ,
  uint64_t ,
  bool _debugger_specific) {
-  LLDB_INSTRUMENT_VA(event, progress_id, completed, total,
- is_debugger_specific);
+  LLDB_INSTRUMENT_VA(event);
   const Debugger::ProgressEventData *progress_data =
   Debugger::ProgressEventData::GetEventDataFromEvent(event.get());
   if (progress_data == nullptr)
Index: lldb/bindings/interface/SBDebugger.i
===
--- lldb/bindings/interface/SBDebugger.i
+++ lldb/bindings/interface/SBDebugger.i
@@ -123,14 +123,11 @@
 };
 
 
-%apply uint64_t& INOUT { uint64_t& progress_id };
-%apply uint64_t& INOUT { uint64_t& completed 

[Lldb-commits] [PATCH] D120598: [lldb/crashlog] Fix scripted_crashlog_json.test failure

2022-02-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added reviewers: JDevlieghere, jingham.
mib added a project: LLDB.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch should fix the test failure on scripted_crashlog_json.test.

The failure is caused because crash reporter will obfuscated the
executable path in the crashlog, if it's located inside the user's home
directory and replace it with `/USER/*/` as a placeholder.

To fix that, we can patch the placeholder with the executable path
before loading the crashlog in lldb.

This also fixes a bug where we would create another target when loading
the crashlog in a scripted process, even if lldb already had a target
for it. Now, crashlog will only create a target if there is none in lldb.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120598

Files:
  lldb/examples/python/crashlog.py
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips


Index: 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
===
--- 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
+++ 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
@@ -21,8 +21,8 @@
   "procExitAbsTime" : 6478056175721,
   "translated" : false,
   "cpuType" : "ARM-64",
-  "procName" : "scripted_crashlog_json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "zsh",
   "parentPid" : 82132,
   "coalitionName" : "com.apple.Terminal",
@@ -47,8 +47,9 @@
 "base" : 4372692992,
 "size" : 16384,
 "uuid" : "b928ee77-9429-334f-ac88-41440bb3d4c7",
-"path" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
-"name" : "scripted_crashlog_json.test.tmp.out"
+"uuid" : "@UUID@",
+"path" : "@EXEC@",
+"name" : "@NAME@"
   },
   {
 "source" : "P",
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
@@ -21,8 +21,8 @@
   "incident" : "FA21DF23-3344-4E45-BF27-4B8E63B7012B",
   "pid" : 72932,
   "cpuType" : "X86-64",
-  "procName" : "json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "fish",
   "parentPid" : 67002,
   "coalitionName" : "io.alacritty",
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -989,7 +989,10 @@
 result.PutCString("error: python exception: %s" % e)
 return
 
-target = crashlog.create_target()
+if debugger.GetNumTargets() > 0:
+target = debugger.GetTargetAtIndex(0)
+else:
+target = crashlog.create_target()
 if not target:
 result.PutCString("error: couldn't create target")
 return


Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/scripted_crashlog.ips
@@ -21,8 +21,8 @@
   "procExitAbsTime" : 6478056175721,
   "translated" : false,
   "cpuType" : "ARM-64",
-  "procName" : "scripted_crashlog_json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "zsh",
   "parentPid" : 82132,
   "coalitionName" : "com.apple.Terminal",
@@ -47,8 +47,9 @@
 "base" : 4372692992,
 "size" : 16384,
 "uuid" : "b928ee77-9429-334f-ac88-41440bb3d4c7",
-"path" : "\/Users\/USER\/*\/scripted_crashlog_json.test.tmp.out",
-"name" : "scripted_crashlog_json.test.tmp.out"
+"uuid" : "@UUID@",
+"path" : "@EXEC@",
+"name" : "@NAME@"
   },
   {
 "source" : "P",
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/Inputs/a.out.ips
@@ -21,8 +21,8 @@
   "incident" : "FA21DF23-3344-4E45-BF27-4B8E63B7012B",
   "pid" : 72932,
   "cpuType" : "X86-64",
-  "procName" : "json.test.tmp.out",
-  "procPath" : "\/Users\/USER\/*\/json.test.tmp.out",
+  "procName" : "@NAME@",
+  "procPath" : "@EXEC@",
   "parentProc" : "fish",
   "parentPid" : 67002,
   "coalitionName" : "io.alacritty",
Index: 

[Lldb-commits] [PATCH] D120595: [trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces

2022-02-25 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments.



Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:51
+  int64_t m_time_shift;
+  int64_t m_time_zero;
+};

What is the suggested way to serialize a u64 into JSON? I thought of two 
approaches:
1. Store two separate u32
2. store the integer as a string

@wallace wdyt?



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:22
 #include "llvm/ADT/None.h"
+#include 
 





Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:188
+  /// TSC to nano converter. nullptr if no conversion exists.
+  std::shared_ptr m_tsc_converter;
 };

Not sure `TraceIntelPT` is the correct spot to cache the converter on the 
client side. It will depend on how this new logic is integrated with the 
decoders, but curious to get your initial feedback on this.



Comment at: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp:75-76
+  bool base_bool = o && fromJSON(value, (TraceGetStateResponse &)packet, path) 
&& o.map("tsc_conversion", packet.tsc_converter);
+  // call perftscconverter
+  // get an instance of perftscconverter and wrap it in optionalhttps://reviews.llvm.org/D120595/new/

https://reviews.llvm.org/D120595

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


[Lldb-commits] [PATCH] D120594: Improve error messages for command that need a stopped process

2022-02-25 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

In D120594#3346730 , @jasonmolenda 
wrote:

> In D120594#3346722 , @JDevlieghere 
> wrote:
>
>> Nit: can we use a colon instead of dash? We have a bunch of places already 
>> where we're "concatenating" errors with a colon so I'd like to keep that 
>> consistent.
>
> Using a dash is definitely the kind of thing I would do, but I don't think I 
> did here?  I'm really not wedded to my specific wording in this patch.

Ah sorry, I missed them down in the CommandObjectDisassemler changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120594

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


[Lldb-commits] [PATCH] D120594: Improve error messages for command that need a stopped process

2022-02-25 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

In D120594#3346722 , @JDevlieghere 
wrote:

> Nit: can we use a colon instead of dash? We have a bunch of places already 
> where we're "concatenating" errors with a colon so I'd like to keep that 
> consistent.

Using a dash is definitely the kind of thing I would do, but I don't think I 
did here?  I'm really not wedded to my specific wording in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120594

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


[Lldb-commits] [PATCH] D120594: Improve error messages for command that need a stopped process

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Nit: can we use a colon instead of dash? We have a bunch of places already 
where we're "concatenating" errors with a colon so I'd like to keep that 
consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120594

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


[Lldb-commits] [PATCH] D120595: [trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces

2022-02-25 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 created this revision.
jj10306 added reviewers: wallace, davidca.
jj10306 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Begin implementing Tsc to timestamp conversion for IntelPT traces:

- Add TscConverter and stub out first implementor (PerfTscConverter)
- Add tsc_converter to TraceIntelPT and update DoRefreshLiveProcess to 
accomodate different trace implementation's TraceGetState responses

This is a work in progress, creating diff now to get feedback on initial 
changes before proceeding with decoder and schema changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120595

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
  lldb/source/Plugins/Process/Linux/IntelPTManager.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp

Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "lldb/Utility/TraceIntelPTGDBRemotePackets.h"
+#include "lldb/Utility/TraceGDBRemotePackets.h"
 
 using namespace llvm;
 using namespace llvm::json;
@@ -43,4 +44,55 @@
   return base;
 }
 
+uint64_t PerfTscConverter::TscToNanos(uint64_t tsc) {
+  // conversion logic here
+  return 1;
+}
+
+bool fromJSON(const llvm::json::Value , std::shared_ptr _converter, llvm::json::Path path) {
+  std::string tsc_converter_kind;
+  ObjectMapper o(value, path);
+
+  bool ret = o.map("kind", tsc_converter_kind);
+
+  if (tsc_converter_kind == "perf") {
+std::shared_ptr perf_tsc_converter_sp = std::make_shared(PerfTscConverter());
+ret = ret && o.map("time_mult", perf_tsc_converter_sp->m_perf_conversion.m_time_mult) && o.map("time_shift", perf_tsc_converter_sp->m_perf_conversion.m_time_shift) && o.map("time_zero", perf_tsc_converter_sp->m_perf_conversion.m_time_zero);
+// should we check ret here?
+tsc_converter = perf_tsc_converter_sp;
+  } else if (tsc_converter_kind == "freq") {
+// TODO
+tsc_converter = nullptr;
+  } else {
+tsc_converter = nullptr;
+  }
+  return ret;
+}
+
+bool fromJSON(const json::Value , TraceIntelPTGetStateResponse , Path path) {
+  ObjectMapper o(value, path);
+  bool base_bool = o && fromJSON(value, (TraceGetStateResponse &)packet, path) && o.map("tsc_conversion", packet.tsc_converter);
+  // call perftscconverter
+  // get an instance of perftscconverter and wrap it in optionaltoJSON();
+base.getAsObject()->try_emplace("tsc_conversion", std::move(tsc_converter_json));
+  }
+  return base;
+}
 } // namespace lldb_private
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Target/SectionLoadList.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/Stream.h"
+#include "lldb/Utility/TraceGDBRemotePackets.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -187,28 +188,19 @@
   m_stop_id = new_stop_id;
   m_live_thread_data.clear();
 
-  Expected json_string = GetLiveProcessState();
-  if (!json_string) {
-DoRefreshLiveProcessState(json_string.takeError());
-return;
-  }
-  Expected live_process_state =
-  json::parse(*json_string, "TraceGetStateResponse");
-  if (!live_process_state) {
-DoRefreshLiveProcessState(live_process_state.takeError());
-return;
-  }
+  std::unique_ptr live_process_state = DoRefreshLiveProcessState(GetLiveProcessState());
 
-  for (const TraceThreadState _state :
-   live_process_state->tracedThreads) {
-for (const TraceBinaryData  : thread_state.binaryData)
-  m_live_thread_data[thread_state.tid][item.kind] = item.size;
-  }
+  if (live_process_state) {
+for (const TraceThreadState _state :
+live_process_state->tracedThreads) {
+  for (const TraceBinaryData  : thread_state.binaryData)
+m_live_thread_data[thread_state.tid][item.kind] = item.size;
+}
 
-  for (const TraceBinaryData  : live_process_state->processBinaryData)
-m_live_process_data[item.kind] = item.size;
+for (const TraceBinaryData  : live_process_state->processBinaryData)
+  m_live_process_data[item.kind] = item.size;
+  }
 
-  DoRefreshLiveProcessState(std::move(live_process_state));
 }
 
 uint32_t Trace::GetStopID() {
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ 

[Lldb-commits] [PATCH] D120594: Improve error messages for command that need a stopped process

2022-02-25 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda created this revision.
jasonmolenda added a reviewer: jingham.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

I don't think lldb's error messaging is great when attached to a process -- the 
command interpreter's commands are asynchronous -- and the user doesn't realize 
that it is currently executing.  They resume execution and they can type more 
commands while the inferior process is executing.  This leads to people getting 
messages like

(lldb) c
Process 82784 resuming
(lldb) si
error: invalid thread
(lldb) dis
error: Cannot disassemble around the current function without a selected frame.

(lldb) reg read pc
error: invalid thread
(lldb) p/x $pc
error: Process is running.  Use 'process interrupt' to pause execution.
(lldb)

That last one was good, but the rest are mystifying unless you work on lldb and 
know how the command requirements are expressed/tested.

This patch changes the error messages, and for the disassembler, adds cases 
where we have a Process and where we don't (in a way that is messy but all 3 
instances print slightly different error messages).

There may be suggestions or other ideas for how these could be phrased.  I 
avoided the term "live process" because I was thinking of corefile debugging, 
but "stopped process" may make people think "well I killed the process, it's 
stopped now, why can't I disassemble" lol.  I'm trying to think less like an 
lldb programmer and more like an lldb user, and I think these terms get the 
point across:

(lldb) c
Process 82784 resuming
(lldb) si
error: Command requires a process which is currently stopped.
(lldb) dis
error: Cannot disassemble around the current function without the process being 
stopped.
(lldb) reg read pc
error: Command requires a process which is currently stopped.
(lldb)

lldb/test/Shell/Commands/command-disassemble.s will need to be updated with the 
final messaging too, before this could be landed.  But I suspect there may be 
some suggestions for edits.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120594

Files:
  lldb/include/lldb/Interpreter/CommandObject.h
  lldb/source/Commands/CommandObjectDisassemble.cpp

Index: lldb/source/Commands/CommandObjectDisassemble.cpp
===
--- lldb/source/Commands/CommandObjectDisassemble.cpp
+++ lldb/source/Commands/CommandObjectDisassemble.cpp
@@ -278,11 +278,20 @@
 
 llvm::Expected>
 CommandObjectDisassemble::GetCurrentFunctionRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "function without a selected frame.\n");
+if (process) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Cannot disassemble around the current "
+  "function without the process being stopped.\n");
+} else {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Cannot disassemble around the current "
+ "function without a selected frame -"
+ "no currently running process.\n");
+}
   }
   SymbolContext sc(
   frame->GetSymbolContext(eSymbolContextFunction | eSymbolContextSymbol));
@@ -301,11 +310,20 @@
 
 llvm::Expected>
 CommandObjectDisassemble::GetCurrentLineRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "line without a selected frame.\n");
+if (process) {
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Cannot disassemble around the current "
+  "function without the process being stopped.\n");
+} else {
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Cannot disassemble around the current "
+ "line without a selected frame -"
+ "no currently running process.\n");
+}
   }
 
   LineEntry pc_line_entry(
@@ -361,11 +379,20 @@
 
 llvm::Expected>
 CommandObjectDisassemble::GetPCRanges() {
+  Process *process = m_exe_ctx.GetProcessPtr();
   StackFrame *frame = m_exe_ctx.GetFramePtr();
   if (!frame) {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Cannot disassemble around the current "
-   "PC without a selected frame.\n");

[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

Yup, that works too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120284

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


[Lldb-commits] [PATCH] D120593: [Support] Allow the ability to change WithColor's auto detection function

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 411533.

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

https://reviews.llvm.org/D120593

Files:
  lldb/tools/driver/Driver.cpp
  llvm/include/llvm/Support/WithColor.h
  llvm/lib/Support/WithColor.cpp

Index: llvm/lib/Support/WithColor.cpp
===
--- llvm/lib/Support/WithColor.cpp
+++ llvm/lib/Support/WithColor.cpp
@@ -33,6 +33,9 @@
 static ManagedStatic, CreateUseColor> UseColor;
 void llvm::initWithColorOptions() { *UseColor; }
 
+WithColor::AutoDetectFunctionType WithColor::AutoDetectFunction =
+WithColor::defaultAutoDetectFunction();
+
 WithColor::WithColor(raw_ostream , HighlightColor Color, ColorMode Mode)
 : OS(OS), Mode(Mode) {
   // Detect color from terminal type unless the user passed the --color option.
@@ -127,8 +130,7 @@
   case ColorMode::Disable:
 return false;
   case ColorMode::Auto:
-return *UseColor == cl::BOU_UNSET ? OS.has_colors()
-  : *UseColor == cl::BOU_TRUE;
+return AutoDetectFunction(OS);
   }
   llvm_unreachable("All cases handled above.");
 }
@@ -159,3 +161,15 @@
 WithColor::warning() << Info.message() << '\n';
   });
 }
+
+WithColor::AutoDetectFunctionType WithColor::defaultAutoDetectFunction() {
+  return [](const raw_ostream ) {
+return *UseColor == cl::BOU_UNSET ? OS.has_colors()
+  : *UseColor == cl::BOU_TRUE;
+  };
+}
+
+void WithColor::setAutoDetectFunction(
+AutoDetectFunctionType NewAutoDetectFunction) {
+  AutoDetectFunction = std::move(NewAutoDetectFunction);
+}
Index: llvm/include/llvm/Support/WithColor.h
===
--- llvm/include/llvm/Support/WithColor.h
+++ llvm/include/llvm/Support/WithColor.h
@@ -11,6 +11,8 @@
 
 #include "llvm/Support/raw_ostream.h"
 
+#include 
+
 namespace llvm {
 
 class Error;
@@ -54,6 +56,9 @@
   raw_ostream 
   ColorMode Mode;
 
+  using AutoDetectFunctionType = std::function;
+  static AutoDetectFunctionType AutoDetectFunction;
+
 public:
   /// To be used like this: WithColor(OS, HighlightColor::String) << "text";
   /// @param OS The output stream
@@ -132,6 +137,13 @@
   /// Implement default handling for Warning.
   /// Print "warning: " to stderr.
   static void defaultWarningHandler(Error Warning);
+
+  /// Retrieve the default color auto detection function.
+  static AutoDetectFunctionType defaultAutoDetectFunction();
+
+  /// Change the global auto detection function.
+  static void
+  setAutoDetectFunction(AutoDetectFunctionType NewAutoDetectFunction);
 };
 
 } // end namespace llvm
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -186,6 +186,13 @@
   m_debugger.SkipLLDBInitFiles(false);
   m_debugger.SkipAppInitFiles(false);
 
+  if (args.hasArg(OPT_no_use_colors)) {
+m_debugger.SetUseColor(false);
+WithColor::setAutoDetectFunction(
+[](const llvm::raw_ostream ) { return false; });
+m_option_data.m_debug_mode = true;
+  }
+
   if (args.hasArg(OPT_version)) {
 m_option_data.m_print_version = true;
   }
@@ -227,11 +234,6 @@
   m_debugger.GetInstanceName());
   }
 
-  if (args.hasArg(OPT_no_use_colors)) {
-m_debugger.SetUseColor(false);
-m_option_data.m_debug_mode = true;
-  }
-
   if (auto *arg = args.getLastArg(OPT_file)) {
 auto arg_value = arg->getValue();
 SBFileSpec file(arg_value);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D120593: [Support] Allow the ability to change WithColor's auto detection function

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, RKSimon, jdenny.
Herald added subscribers: dexonsmith, hiraditya.
JDevlieghere requested review of this revision.
Herald added a project: LLVM.

`WithColor` has an "auto detection mode" which looks whether the corresponding  
whether the corresponding cl::opt is enabled or not. While this is great when 
opting into cl::opt, it's not so great for downstream users of this utility, 
which might have their own competing options to enable or disable colors. The 
`WithColor` constructor takes a color mode, but the big benefit of the class 
are its static `error` and `warning` helpers and default error handlers.

In order to allow users of this utility to enable or disable colors globally, 
this patch adds the ability to specify a global auto detection function. By 
default, the auto detection function behaves the way that it does today. The 
benefit of this patch lies in that it can be overwritten. In addition to a 
ability to change the auto detection function, I've also made it possible to 
get your hands on the default auto detection function, so you swap it back if 
if you so desire.

This patch allow downstream users (like LLDB) to globally disable colors with 
its own command line flag.


https://reviews.llvm.org/D120593

Files:
  lldb/tools/driver/Driver.cpp
  llvm/include/llvm/Support/WithColor.h
  llvm/lib/Support/WithColor.cpp

Index: llvm/lib/Support/WithColor.cpp
===
--- llvm/lib/Support/WithColor.cpp
+++ llvm/lib/Support/WithColor.cpp
@@ -33,6 +33,12 @@
 static ManagedStatic, CreateUseColor> UseColor;
 void llvm::initWithColorOptions() { *UseColor; }
 
+WithColor::AutoDetectFunctionType WithColor::AutoDetectFunction =
+[](const raw_ostream ) {
+  return *UseColor == cl::BOU_UNSET ? OS.has_colors()
+: *UseColor == cl::BOU_TRUE;
+};
+
 WithColor::WithColor(raw_ostream , HighlightColor Color, ColorMode Mode)
 : OS(OS), Mode(Mode) {
   // Detect color from terminal type unless the user passed the --color option.
@@ -127,8 +133,7 @@
   case ColorMode::Disable:
 return false;
   case ColorMode::Auto:
-return *UseColor == cl::BOU_UNSET ? OS.has_colors()
-  : *UseColor == cl::BOU_TRUE;
+return AutoDetectFunction(OS);
   }
   llvm_unreachable("All cases handled above.");
 }
@@ -159,3 +164,15 @@
 WithColor::warning() << Info.message() << '\n';
   });
 }
+
+WithColor::AutoDetectFunctionType WithColor::defaultAutoDetectFunction() {
+  return [](const raw_ostream ) {
+return *UseColor == cl::BOU_UNSET ? OS.has_colors()
+  : *UseColor == cl::BOU_TRUE;
+  };
+}
+
+void WithColor::setAutoDetectFunction(
+AutoDetectFunctionType NewAutoDetectFunction) {
+  AutoDetectFunction = std::move(NewAutoDetectFunction);
+}
Index: llvm/include/llvm/Support/WithColor.h
===
--- llvm/include/llvm/Support/WithColor.h
+++ llvm/include/llvm/Support/WithColor.h
@@ -11,6 +11,8 @@
 
 #include "llvm/Support/raw_ostream.h"
 
+#include 
+
 namespace llvm {
 
 class Error;
@@ -54,6 +56,9 @@
   raw_ostream 
   ColorMode Mode;
 
+  using AutoDetectFunctionType = std::function;
+  static AutoDetectFunctionType AutoDetectFunction;
+
 public:
   /// To be used like this: WithColor(OS, HighlightColor::String) << "text";
   /// @param OS The output stream
@@ -132,6 +137,13 @@
   /// Implement default handling for Warning.
   /// Print "warning: " to stderr.
   static void defaultWarningHandler(Error Warning);
+
+  /// Retrieve the default color auto detection function.
+  static AutoDetectFunctionType defaultAutoDetectFunction();
+
+  /// Change the global auto detection function.
+  static void
+  setAutoDetectFunction(AutoDetectFunctionType NewAutoDetectFunction);
 };
 
 } // end namespace llvm
Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -186,6 +186,13 @@
   m_debugger.SkipLLDBInitFiles(false);
   m_debugger.SkipAppInitFiles(false);
 
+  if (args.hasArg(OPT_no_use_colors)) {
+m_debugger.SetUseColor(false);
+WithColor::setAutoDetectFunction(
+[](const llvm::raw_ostream ) { return false; });
+m_option_data.m_debug_mode = true;
+  }
+
   if (args.hasArg(OPT_version)) {
 m_option_data.m_print_version = true;
   }
@@ -227,11 +234,6 @@
   m_debugger.GetInstanceName());
   }
 
-  if (args.hasArg(OPT_no_use_colors)) {
-m_debugger.SetUseColor(false);
-m_option_data.m_debug_mode = true;
-  }
-
   if (auto *arg = args.getLastArg(OPT_file)) {
 auto arg_value = arg->getValue();
 SBFileSpec file(arg_value);
___
lldb-commits mailing list

[Lldb-commits] [PATCH] D120578: [lldb] Implement ObjectFile::GetCStrFromSection

2022-02-25 Thread Andreu Carminati via Phabricator via lldb-commits
andcarminati added inline comments.



Comment at: lldb/source/Symbol/ObjectFile.cpp:559
+   lldb::offset_t section_offset) const {
+  offset_t offset = section->GetOffset() + section_offset;
+  return m_data.GetCStr();

Wouldn't it be better to use 
```
section->GetFileOffset()
``` 
to access m_data? As this object contains data loaded from the file. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120578

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


[Lldb-commits] [lldb] 2ce6bc6 - [lldb] Fix check for TARGET_OS_IPHONE

2022-02-25 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2022-02-25T13:24:39-08:00
New Revision: 2ce6bc61e876f4e7918952a565488d737ce647f6

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

LOG: [lldb] Fix check for TARGET_OS_IPHONE

Instead of checking whether TARGET_OS_IPHONE is set to 1, the current
code just check the existence of TARGET_OS_IPHONE, which either always
succeeds or always fails, depending on whether you have
TargetConditionals.h included.

Added: 


Modified: 
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index a50803df58e7c..8676371b963e4 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -84,6 +84,10 @@
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/ScopedPrinter.h"
 
+#if defined(__APPLE__)
+#include 
+#endif
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -434,7 +438,7 @@ void CommandInterpreter::Initialize() {
   if (cmd_obj_sp) {
 alias_arguments_vector_sp = std::make_shared();
 #if defined(__APPLE__)
-#if defined(TARGET_OS_IPHONE)
+#if TARGET_OS_IPHONE
 AddAlias("r", cmd_obj_sp, "--");
 AddAlias("run", cmd_obj_sp, "--");
 #else



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


[Lldb-commits] [PATCH] D120578: [lldb] Implement ObjectFile::GetCStrFromSection

2022-02-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I don't think this implementation is sound. If you look at the implementation 
of `ReadSectionData` you'll see that there is a lot more involved in reading 
data from a section than just incrementing a pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120578

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


[Lldb-commits] [PATCH] D120578: [lldb] Implement ObjectFile::GetCStrFromSection

2022-02-25 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 added inline comments.



Comment at: lldb/include/lldb/Symbol/ObjectFile.h:677
+  const char *GetCStrFromSection(Section *section,
+ lldb::offset_t section_offset) const;
+

aprantl wrote:
> Should we call it `GetCStringFromSection`, or even better 
> `ReadCStringFromSection`?
I agree that `ReadCStringFromSection` sounds better.



Comment at: lldb/source/Symbol/ObjectFile.cpp:558
+ObjectFile::GetCStrFromSection(Section *section,
+   lldb::offset_t section_offset) const {
+  offset_t offset = section->GetOffset() + section_offset;

aprantl wrote:
> The fact that it's a C string seems to be not relevant to the function. It 
> seems to just convert a file address to a load address?
What do you mean? This function calls `m_data.GetCStr` and returns the result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120578

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


[Lldb-commits] [PATCH] D120578: [lldb] Implement ObjectFile::GetCStrFromSection

2022-02-25 Thread Adrian Prantl via Phabricator via lldb-commits
aprantl added inline comments.



Comment at: lldb/include/lldb/Symbol/ObjectFile.h:676
 
+  const char *GetCStrFromSection(Section *section,
+ lldb::offset_t section_offset) const;

Can you add a doxygen comment for the method?



Comment at: lldb/include/lldb/Symbol/ObjectFile.h:677
+  const char *GetCStrFromSection(Section *section,
+ lldb::offset_t section_offset) const;
+

Should we call it `GetCStringFromSection`, or even better 
`ReadCStringFromSection`?



Comment at: lldb/source/Symbol/ObjectFile.cpp:558
+ObjectFile::GetCStrFromSection(Section *section,
+   lldb::offset_t section_offset) const {
+  offset_t offset = section->GetOffset() + section_offset;

The fact that it's a C string seems to be not relevant to the function. It 
seems to just convert a file address to a load address?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120578

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


[Lldb-commits] [PATCH] D120578: [lldb] Implement ObjectFile::GetCStrFromSection

2022-02-25 Thread Augusto Noronha via Phabricator via lldb-commits
augusto2112 created this revision.
augusto2112 added reviewers: aprantl, JDevlieghere.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120578

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Symbol/ObjectFile.cpp


Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -553,6 +553,13 @@
   section_data);
 }
 
+const char *
+ObjectFile::GetCStrFromSection(Section *section,
+   lldb::offset_t section_offset) const {
+  offset_t offset = section->GetOffset() + section_offset;
+  return m_data.GetCStr();
+}
+
 bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
 FileSpec _file,
 ConstString _object,
Index: lldb/include/lldb/Symbol/ObjectFile.h
===
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -673,6 +673,9 @@
   virtual size_t ReadSectionData(Section *section,
  DataExtractor _data);
 
+  const char *GetCStrFromSection(Section *section,
+ lldb::offset_t section_offset) const;
+
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.


Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -553,6 +553,13 @@
   section_data);
 }
 
+const char *
+ObjectFile::GetCStrFromSection(Section *section,
+   lldb::offset_t section_offset) const {
+  offset_t offset = section->GetOffset() + section_offset;
+  return m_data.GetCStr();
+}
+
 bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
 FileSpec _file,
 ConstString _object,
Index: lldb/include/lldb/Symbol/ObjectFile.h
===
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -673,6 +673,9 @@
   virtual size_t ReadSectionData(Section *section,
  DataExtractor _data);
 
+  const char *GetCStrFromSection(Section *section,
+ lldb::offset_t section_offset) const;
+
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D120292: [lldb] Add lldb.find helper function

2022-02-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

Jim's suggestion made me think that if we make a subcommand for this, it would 
be nice to have some completion handling. I know it would probably require you 
to move away from the python function but this is just a suggestion for later 
improvements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120292

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


[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

2022-02-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D120284#3345994 , @JDevlieghere 
wrote:

>> I'm not entirely sure what's the best fix here. @JDevlieghere, what do you 
>> think? Can we just remove the output arguments from the LLDB_INSTRUMENT_VA 
>> invocation (given how logging their entry values is pretty useless)?
>
> Yup, the way I dealt with that for the reproducers was initialize them before 
> the macro, but no point in doing that purely for logging. Let's just remove 
> the macro.
>
> I'll need to think of a way to avoid `lldb-instr` putting it back. It will 
> ignore functions that start with a macro, so maybe we can add a NOOP macro 
> "LLDB_NO_INSTRUMENT" or something?

I thought we could just replace `LLDB_INSTRUMENT_VA(event, progress_id, 
completed, total, is_debugger_specific);` with  `LLDB_INSTRUMENT_VA(event);`

Is that not sufficient to dissuade the tool?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120284

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


[Lldb-commits] [PATCH] D120292: [lldb] Add lldb.find helper function

2022-02-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

In D120292#3346030 , @JDevlieghere 
wrote:

> I like Jim's suggestion

Same same !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120292

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


[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

2022-02-25 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib added a comment.

In D120284#3345994 , @JDevlieghere 
wrote:

>> I'm not entirely sure what's the best fix here. @JDevlieghere, what do you 
>> think? Can we just remove the output arguments from the LLDB_INSTRUMENT_VA 
>> invocation (given how logging their entry values is pretty useless)?
>
> Yup, the way I dealt with that for the reproducers was initialize them before 
> the macro, but no point in doing that purely for logging. Let's just remove 
> the macro.
>
> I'll need to think of a way to avoid `lldb-instr` putting it back. It will 
> ignore functions that start with a macro, so maybe we can add a NOOP macro 
> "LLDB_NO_INSTRUMENT" or something?

Will `lldb-instr` try to override the macro if not all the function arguments 
are passed to the LLDB_INSTRUMENT_VA invocation ? If this is not the case, it 
should be fine for `SBDebugger::GetProgressFromEvent` since it still takes an 
SBEvent argument, that will remain in the LLDB_INSTRUMENT_VA invocation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120284

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


[Lldb-commits] [PATCH] D120292: [lldb] Add lldb.find helper function

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

I like Jim's suggestion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120292

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


[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

> I'm not entirely sure what's the best fix here. @JDevlieghere, what do you 
> think? Can we just remove the output arguments from the LLDB_INSTRUMENT_VA 
> invocation (given how logging their entry values is pretty useless)?

Yup, the way I dealt with that for the reproducers was initialize them before 
the macro, but no point in doing that purely for logging. Let's just remove the 
macro.

I'll need to think of a way to avoid `lldb-instr` putting it back. It will 
ignore functions that start with a macro, so maybe we can add a NOOP macro 
"LLDB_NO_INSTRUMENT" or something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120284

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


[Lldb-commits] [PATCH] D119146: [lldb/Platform] Prepare decouple instance and plugin names

2022-02-25 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision.
JDevlieghere added a comment.

(Still) LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119146

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


[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

2022-02-25 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu updated this revision to Diff 411446.
zequanwu marked 5 inline comments as done.
zequanwu added a comment.

update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963

Files:
  lldb/include/lldb/Core/Address.h
  lldb/include/lldb/Expression/DWARFExpression.h
  lldb/include/lldb/Symbol/Variable.h
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Address.cpp
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_loclists_base.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc_and_loclists.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwo.s
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_loclists-dwp.s
  lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
  lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test

Index: lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
+++ lldb/test/Shell/SymbolFile/DWARF/x86/unused-inlined-params.test
@@ -14,35 +14,35 @@
 # at the inlined function entry.
 image lookup -v -s break_at_inlined_f_in_main
 # CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_consts +42
-# CHECK: name = "unused2", type = "int", location = 
-# CHECK: name = "partial", type = "int", location = DW_OP_reg4 RSI
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0011, 0x0014) -> DW_OP_consts +42
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
+# CHECK: name = "partial", type = "int", valid ranges = , location = [0x0011, 0x0019) -> DW_OP_reg4 RSI
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
 
 # Show variables outsid of the live range of the 'partial' parameter
 # and verify that the output is as expected.
 image lookup -v -s break_at_inlined_f_in_main_between_printfs
 # CHECK-LABEL: image lookup -v -s break_at_inlined_f_in_main_between_printfs
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_reg3 RBX
-# CHECK: name = "unused2", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0014, 0x001e) -> DW_OP_reg3 RBX
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
 # Note: image lookup does not show variables outside of their
 #   location, so |partial| is missing here.
 # CHECK-NOT: partial
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
 
 # Check that we show parameters even if all of them are compiled away.
 image lookup -v -s  break_at_inlined_g_in_main
 # CHECK-LABEL: image lookup -v -s  break_at_inlined_g_in_main
-# CHECK: name = "unused", type = "int", location = 
+# CHECK: name = "unused", type = "int", valid ranges = , location = 
 
 # Check that even the other inlined instance of f displays the correct
 # parameters.
 image lookup -v -s  break_at_inlined_f_in_other
 # CHECK-LABEL: image lookup -v -s  break_at_inlined_f_in_other
-# CHECK: name = "unused1", type = "void *", location = 
-# CHECK: name = "used", type = "int", location = DW_OP_consts +1
-# CHECK: name = "unused2", type = "int", location = 
-# CHECK: name = "partial", type = "int", location =  DW_OP_consts +2
-# CHECK: name = "unused3", type = "int", location = 
+# CHECK: name = "unused1", type = "void *", valid ranges = , location = 
+# CHECK: name = "used", type = "int", valid ranges = , location = [0x0001, 0x000b) -> DW_OP_consts +1
+# CHECK: name = "unused2", type = "int", valid ranges = , location = 
+# CHECK: name = "partial", type = "int", valid ranges = , location = [0x0001, 0x0006) -> DW_OP_consts +2
+# CHECK: name = "unused3", type = "int", valid ranges = , location = 
Index: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
+++ lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s
@@ -12,7 +12,7 @@
 # CHECK-LABEL: image lookup -v -n F1
 # CHECK: CompileUnit: id = {0x0001}, file = "1.c", language = ""
 # CHECK: Function: {{.*}}, name = "F1", range = [0x0001-0x0002)
-# CHECK: Variable: {{.*}}, name = "x", type = "int", location = DW_OP_reg1 RDX
+# CHECK: Variable: {{.*}}, name = 

[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

2022-02-25 Thread Michael Forster via Phabricator via lldb-commits
MForster added a comment.

> The backtrace is pretty much useless

Yes, sorry, that's all I had. Just added some evidence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120284

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


[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

2022-02-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

The backtrace is pretty much useless, but what is happening is that the 
LLDB_INSTRUMENT_VA macro inside SBDebugger::GetProgressFromEvent is trying to 
log the uninitialized output variable, which angers the gods of sanitation.

I'm not entirely sure what's the best fix here. @JDevlieghere, what do you 
think? Can we just remove the output arguments from the LLDB_INSTRUMENT_VA 
invocation (given how logging their entry values is pretty useless)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120284

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


[Lldb-commits] [lldb] 4729a72 - Revert "[lldb/test] Fix TestProgressReporting.py race issue with the event listener"

2022-02-25 Thread Michael Forster via lldb-commits

Author: Michael Forster
Date: 2022-02-25T13:07:49Z
New Revision: 4729a72acec31b46e167b16f50efacab9d16da9c

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

LOG: Revert "[lldb/test] Fix TestProgressReporting.py race issue with the event 
listener"

This reverts commit 3e3e79a9e4c378b59f5f393f556e6a84edcd8898.

MemorySanitizer: use-of-uninitialized-value

Added: 


Modified: 
lldb/bindings/interface/SBDebugger.i
lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py

Removed: 




diff  --git a/lldb/bindings/interface/SBDebugger.i 
b/lldb/bindings/interface/SBDebugger.i
index 0ef1766a50c6b..3790857b8ab61 100644
--- a/lldb/bindings/interface/SBDebugger.i
+++ b/lldb/bindings/interface/SBDebugger.i
@@ -123,11 +123,14 @@ public:
 };
 
 
+%apply uint64_t& INOUT { uint64_t& progress_id };
+%apply uint64_t& INOUT { uint64_t& completed };
+%apply uint64_t& INOUT { uint64_t& total };
+%apply bool& INOUT { bool& is_debugger_specific };
 static const char *GetProgressFromEvent(const lldb::SBEvent ,
-uint64_t ,
-uint64_t ,
-uint64_t ,
-bool );
+uint64_t _id,
+uint64_t , uint64_t ,
+bool _debugger_specific);
 
 SBBroadcaster GetBroadcaster();
 

diff  --git 
a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py 
b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
index 79ef4e3f9f861..b9d9953539c11 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -17,42 +17,41 @@ def setUp(self):
 TestBase.setUp(self)
 self.progress_events = []
 
-def fetch_events(self):
+def fetch_events(self, test_broadcaster):
+listener = lldb.SBListener("lldb.progress.listener")
+listener.StartListeningForEvents(test_broadcaster,
+ self.eBroadcastBitStopProgressThread)
+
+progress_broadcaster = self.dbg.GetBroadcaster()
+progress_broadcaster.AddListener(listener, 
lldb.SBDebugger.eBroadcastBitProgress)
+
 event = lldb.SBEvent()
 
 done = False
 while not done:
-if self.listener.WaitForEvent(1, event):
+if listener.WaitForEvent(1, event):
 event_mask = event.GetType();
-if event.BroadcasterMatchesRef(self.test_broadcaster):
+if event.BroadcasterMatchesRef(test_broadcaster):
 if event_mask & self.eBroadcastBitStopProgressThread:
 done = True;
-elif event.BroadcasterMatchesRef(self.progress_broadcaster):
-ret_args = lldb.SBDebugger().GetProgressFromEvent(event);
-self.assertGreater(len(ret_args), 1)
-
-message = ret_args[0]
+elif event.BroadcasterMatchesRef(progress_broadcaster):
+message = lldb.SBDebugger().GetProgressFromEvent(event, 0, 
0, 0, False);
 if message:
 self.progress_events.append((message, event))
 
+@skipUnlessDarwin
 def test_dwarf_symbol_loading_progress_report(self):
 """Test that we are able to fetch dwarf symbol loading progress 
events"""
 self.build()
 
-self.listener = lldb.SBListener("lldb.progress.listener")
-self.test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
-self.listener.StartListeningForEvents(self.test_broadcaster,
-  
self.eBroadcastBitStopProgressThread)
-
-self.progress_broadcaster = self.dbg.GetBroadcaster()
-self.progress_broadcaster.AddListener(self.listener, 
lldb.SBDebugger.eBroadcastBitProgress)
-
-listener_thread = threading.Thread(target=self.fetch_events)
+test_broadcaster = lldb.SBBroadcaster('lldb.broadcaster.test')
+listener_thread = threading.Thread(target=self.fetch_events,
+   args=[test_broadcaster])
 listener_thread.start()
 
 lldbutil.run_to_source_breakpoint(self, 'break here', 
lldb.SBFileSpec('main.c'))
 
-
self.test_broadcaster.BroadcastEventByType(self.eBroadcastBitStopProgressThread)
+
test_broadcaster.BroadcastEventByType(self.eBroadcastBitStopProgressThread)
 listener_thread.join()
 
-self.assertGreater(len(self.progress_events), 0)
+

[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

2022-02-25 Thread Michael Forster via Phabricator via lldb-commits
MForster added a comment.

This fails under MSAN:

  ==3096==WARNING: MemorySanitizer: use-of-uninitialized-value
  #0 0x7f0e7899ce18  
(/build/work/73ffa3b6f1cbc2218b7b71e21a3dd1e2e53b/google3/runfiles/google3/third_party/py/lldb/dotest_wrapper.par/@0x3a0b9000+0x2e3e18)
 (BuildId: 4c3940d9ff00226e685dcfb93f4b4753)
  #1 0x7f0e7898fcc8  
(/build/work/73ffa3b6f1cbc2218b7b71e21a3dd1e2e53b/google3/runfiles/google3/third_party/py/lldb/dotest_wrapper.par/@0x3a0b9000+0x2d6cc8)
 (BuildId: 4c3940d9ff00226e685dcfb93f4b4753)
  #2 0x7f0e78973001  
(/build/work/73ffa3b6f1cbc2218b7b71e21a3dd1e2e53b/google3/runfiles/google3/third_party/py/lldb/dotest_wrapper.par/@0x3a0b9000+0x2ba001)
 (BuildId: 4c3940d9ff00226e685dcfb93f4b4753)
  #3 0x7f0e78cd1f10  
(/build/work/73ffa3b6f1cbc2218b7b71e21a3dd1e2e53b/google3/runfiles/google3/third_party/py/lldb/dotest_wrapper.par/@0x3a0b9000+0x618f10)
 (BuildId: 4c3940d9ff00226e685dcfb93f4b4753)
  #4 0x558eaf3ecc70 in cfunction_call_varargs 
third_party/python_runtime/v3_7/Objects/call.c:770:18
  #5 0x558eaf3ecc70 in PyCFunction_Call 
third_party/python_runtime/v3_7/Objects/call.c:786:16
  #6 0x558eaf685ff8 in do_call_core 
third_party/python_runtime/v3_7/Python/ceval.c:4641:9
  #7 0x558eaf685ff8 in _PyEval_EvalFrameDefault 
third_party/python_runtime/v3_7/Python/ceval.c:3191:22
  #8 0x558eaf6a0691 in PyEval_EvalFrameEx 
third_party/python_runtime/v3_7/Python/ceval.c:547:12
  #9 0x558eaf6a0691 in _PyEval_EvalCodeWithName 
third_party/python_runtime/v3_7/Python/ceval.c:3930:14
  #10 0x558eaf3ebaa3 in _PyFunction_FastCallKeywords 
third_party/python_runtime/v3_7/Objects/call.c:433:12
  #11 0x558eaf69db2d in call_function 
third_party/python_runtime/v3_7/Python/ceval.c:4616:17
  #12 0x558eaf685844 in _PyEval_EvalFrameDefault 
third_party/python_runtime/v3_7/Python/ceval.c:3093:23
  #13 0x558eaf67d594 in PyEval_EvalFrameEx 
third_party/python_runtime/v3_7/Python/ceval.c:547:12
  #14 0x558eaf3ed230 in function_code_fastcall 
third_party/python_runtime/v3_7/Objects/call.c:283:14
  #15 0x558eaf3ea160 in _PyFunction_FastCallDict 
third_party/python_runtime/v3_7/Objects/call.c
  #16 0x558eaf3e99a4 in _PyObject_FastCallDict 
third_party/python_runtime/v3_7/Objects/call.c:98:16
  #17 0x558eaf3ef739 in _PyObject_Call_Prepend 
third_party/python_runtime/v3_7/Objects/call.c:906:14
  #18 0x558eaf3f462b in method_call 
third_party/python_runtime/v3_7/Objects/classobject.c:309:12
  #19 0x558eaf3ec51d in PyObject_Call 
third_party/python_runtime/v3_7/Objects/call.c:245:18
  #20 0x558eaf685d63 in do_call_core 
third_party/python_runtime/v3_7/Python/ceval.c:4645:16
  #21 0x558eaf685d63 in _PyEval_EvalFrameDefault 
third_party/python_runtime/v3_7/Python/ceval.c:3191:22
  #22 0x558eaf67d594 in PyEval_EvalFrameEx 
third_party/python_runtime/v3_7/Python/ceval.c:547:12
  #23 0x558eaf3ed230 in function_code_fastcall 
third_party/python_runtime/v3_7/Objects/call.c:283:14
  #24 0x558eaf3ebb23 in _PyFunction_FastCallKeywords 
third_party/python_runtime/v3_7/Objects/call.c
  #25 0x558eaf69db2d in call_function 
third_party/python_runtime/v3_7/Python/ceval.c:4616:17
  #26 0x558eaf685808 in _PyEval_EvalFrameDefault 
third_party/python_runtime/v3_7/Python/ceval.c:3110:23
  #27 0x558eaf67d594 in PyEval_EvalFrameEx 
third_party/python_runtime/v3_7/Python/ceval.c:547:12
  #28 0x558eaf3ed230 in function_code_fastcall 
third_party/python_runtime/v3_7/Objects/call.c:283:14
  #29 0x558eaf3ebb23 in _PyFunction_FastCallKeywords 
third_party/python_runtime/v3_7/Objects/call.c
  #30 0x558eaf69db2d in call_function 
third_party/python_runtime/v3_7/Python/ceval.c:4616:17
  #31 0x558eaf685808 in _PyEval_EvalFrameDefault 
third_party/python_runtime/v3_7/Python/ceval.c:3110:23
  #32 0x558eaf67d594 in PyEval_EvalFrameEx 
third_party/python_runtime/v3_7/Python/ceval.c:547:12
  #33 0x558eaf3ed230 in function_code_fastcall 
third_party/python_runtime/v3_7/Objects/call.c:283:14
  #34 0x558eaf3ea160 in _PyFunction_FastCallDict 
third_party/python_runtime/v3_7/Objects/call.c
  #35 0x558eaf3e99a4 in _PyObject_FastCallDict 
third_party/python_runtime/v3_7/Objects/call.c:98:16
  #36 0x558eaf3ef739 in _PyObject_Call_Prepend 
third_party/python_runtime/v3_7/Objects/call.c:906:14
  #37 0x558eaf3f462b in method_call 
third_party/python_runtime/v3_7/Objects/classobject.c:309:12
  #38 0x558eaf3ec51d in PyObject_Call 
third_party/python_runtime/v3_7/Objects/call.c:245:18
  #39 0x558eaecfdc9a in t_bootstrap 
third_party/python_runtime/v3_7/Modules/_threadmodule.c:994:11
  #40 0x558eaf77ac94 in pythread_wrapper 
third_party/python_runtime/v3_7/Python/thread_pthread.h:174:5
  #41 0x7f0e7bf0eb54 in start_thread 
(/usr/grte/v5/lib64/libpthread.so.0+0xbb54) (BuildId: 
64752de50ebd1a108f4b3f8d0d7e1a13)
  #42 0x7f0e7be43f7e in clone (/usr/grte/v5/lib64/libc.so.6+0x13cf7e) 
(BuildId: 

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

2022-02-25 Thread Andreu Carminati via Phabricator via lldb-commits
andcarminati added inline comments.



Comment at: lldb/tools/driver/Driver.cpp:673-674
+static void sigtstp_handler(int signo) {
   if (g_driver != nullptr)
 g_driver->GetDebugger().SaveInputTerminalState();
 

labath wrote:
> JDevlieghere wrote:
> > andcarminati wrote:
> > > JDevlieghere wrote:
> > > > labath wrote:
> > > > > JDevlieghere wrote:
> > > > > > I see an opportunity for a little RAII helper.
> > > > > What kind of a helper did you have in mind? Practically the entire 
> > > > > function consists of setup and teardown in preparation for the 
> > > > > `raise(signo)` call. If I wanted to be fancy I could put all of that 
> > > > > in a helper, but I don't think that would make it cleaner. Plus, we 
> > > > > also need to be careful about the functions we call from a signal 
> > > > > handler, and I really don't know whether e.g. `llvm::make_scope_exit` 
> > > > > is guaranteed to not allocate (heap) memory.
> > > > I was only referring to the Save/RestoreInputTerminalState() part of 
> > > > this function. Something like:
> > > > 
> > > > ```
> > > > class TerminalStateRAII() {
> > > > public:
> > > >   TerminalStateRAII(Driver* driver) : driver(m_driver) {
> > > > if (m_driver)
> > > >   m_driver->GetDebugger().SaveInputTerminalState();
> > > >   }
> > > > 
> > > >   ~SignalHelper() {
> > > > if (m_driver)
> > > >   m_driver->GetDebugger().SaveInputTerminalState();
> > > >   }
> > > > 
> > > > private:
> > > >   Driver* m_driver;
> > > > };
> > > > ```
> > > > 
> > > > Obviously, this isn't at all important, just something that came to 
> > > > mind.
> > > I think this is a good idea to reduce code duplication. Another approach:
> > > 
> > > ```
> > > class TerminalStateRAII() {
> > > public:
> > >   TerminalStateRAII(Driver* driver) : driver(m_driver) {
> > > SaveInputTerminalState();
> > >   }
> > > 
> > >   ~TerminalStateRAII() {
> > > SaveInputTerminalState();
> > >   }
> > > 
> > > private:
> > >   Driver* m_driver;
> > >   void SaveInputTerminalState(){
> > > if (m_driver)
> > >   m_driver->GetDebugger().SaveInputTerminalState();
> > >   }
> > > };
> > > 
> > > ```
> > That's a typo on my part, the destructor needs to call 
> > `RestoreInputTerminalState` (as opposed to `SaveInputTerminalState`).
> Ok, I see. If this was a more complex function (e.g. multiple return points), 
> then I'd agree, but this function is really simple and linear (just like a 
> signal handler should be). I am not convinced by the "code duplication" 
> argument -- the way I see it, the helper class replaces 4 lines of (simple) 
> code with ~15 lines of boilerplate. And this function is literally the only 
> caller of Save/RestoreInputTerminalState.
Considering this argument:

> Ok, I see. If this was a more complex function (e.g. multiple return points), 
> then I'd agree, but this function is really simple and linear (just like a 
> signal handler should be). 

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120320

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


[Lldb-commits] [lldb] cc7bf4a - [LLDB] XFAIL TestUnambiguousTailCalls.py for Arm/Linux

2022-02-25 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2022-02-25T17:09:50+05:00
New Revision: cc7bf4afeefc6d3951871dcb091873243a563646

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

LOG: [LLDB] XFAIL TestUnambiguousTailCalls.py for Arm/Linux

This patch marks TestUnambiguousTailCalls.py as XFAIL on Arm/Linux.
Test started failing after 3c4ed02698afec021c6bca80740d1e58e3ee019e.

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

Added: 


Modified: 

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

Removed: 




diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
 
b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
index 19aad2ab1ec32..cef500f0e7754 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
@@ -2,6 +2,7 @@
 from lldbsuite.test import decorators
 
 decor = [decorators.skipUnlessHasCallSiteInfo,
+ decorators.skipIf(archs=['arm'],oslist=["linux"]),
  decorators.skipIf(dwarf_version=['<', '4']),
  decorators.skipIf(compiler="clang", compiler_version=['<', '11.0'])]
 lldbinline.MakeInlineTest(__file__, globals(), name="UnambiguousTailCalls_V5",



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


[Lldb-commits] [PATCH] D119146: [lldb/Platform] Decouple instance and plugin names

2022-02-25 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 411366.
labath added a comment.

Reduce the scope of the patch to GetName interface changes. Given the direction
of the discourse thread, it seems unlikely that we will be able to determine the
instance name at object creation time.

We will likely need a setter which will be called when the class is associated
with a debugger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119146

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBPlatform.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Interpreter/OptionGroupPlatform.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
  lldb/source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -242,7 +242,7 @@
 platform_set.end()) {
   if (!platform_set.empty())
 error_strm.PutCString(", ");
-  error_strm.PutCString(the_platform_sp->GetName().GetCString());
+  error_strm.PutCString(the_platform_sp->GetName());
   platform_set.insert(the_platform_sp.get());
 }
   }
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2895,11 +2895,10 @@
   if (platform_sp) {
 GetTarget().SetPlatform(platform_sp);
 GetTarget().SetArchitecture(platform_arch);
-LLDB_LOGF(log,
-  "Process::%s switching platform to %s and architecture "
-  "to %s based on info from attach",
-  __FUNCTION__, platform_sp->GetName().AsCString(""),
-  platform_arch.GetTriple().getTriple().c_str());
+LLDB_LOG(log,
+ "switching platform to {0} and architecture to {1} based on "
+ "info from attach",
+ platform_sp->GetName(), platform_arch.GetTriple().getTriple());
   }
 } else if (!process_arch.IsValid()) {
   ProcessInstanceInfo process_info;
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -280,7 +280,7 @@
 
 std::lock_guard guard(GetPlatformListMutex());
 for (const auto _sp : GetPlatformList()) {
-  if (platform_sp->GetName() == name)
+  if (platform_sp->GetName() == name.GetStringRef())
 return platform_sp;
 }
   }
@@ -786,8 +786,6 @@
   }
 }
 
-ConstString Platform::GetName() { return ConstString(GetPluginName()); }
-
 const char *Platform::GetHostname() {
   if (IsHost())
 return "127.0.0.1";
@@ -1728,7 +1726,7 @@
 
 FileSpec Platform::GetModuleCacheRoot() {
   auto dir_spec = GetGlobalPlatformProperties().GetModuleCacheDirectory();
-  dir_spec.AppendPathComponent(GetName().AsCString());
+  dir_spec.AppendPathComponent(GetPluginName());
   return dir_spec;
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2404,9 +2404,8 @@
   m_public_state.GetValue() != eStateRunning) {
 PlatformSP platform_sp = GetTarget().GetPlatform();
 
-if (platform_sp && platform_sp->GetName() &&
-platform_sp->GetName().GetStringRef() ==
-PlatformRemoteiOS::GetPluginNameStatic()) {
+if (platform_sp && platform_sp->GetPluginName() ==
+   PlatformRemoteiOS::GetPluginNameStatic()) {
   if (m_destroy_tried_resuming) {
 if (log)
   log->PutCString("ProcessGDBRemote::DoDestroy() - Tried resuming to "
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -24,7 +24,7 @@
 
 class PlatformDarwin : public PlatformPOSIX {
 public:
-  PlatformDarwin(bool is_host);
+  using PlatformPOSIX::PlatformPOSIX;
 
   ~PlatformDarwin() override;
 
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- 

[Lldb-commits] [PATCH] D114627: [lldb] add new overload for SymbolFile::FindTypes that accepts a scope

2022-02-25 Thread Lasse Folger via Phabricator via lldb-commits
lassefolger added a comment.

I'm still planning to look into this just couldn't find the time this week and 
the last. Sorry for the delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114627

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


[Lldb-commits] [lldb] 317a2b1 - [LLDB] Remove XFAIL from minidebuginfo-set-and-hit-breakpoint.test

2022-02-25 Thread Muhammad Omair Javaid via lldb-commits

Author: Muhammad Omair Javaid
Date: 2022-02-25T15:55:05+05:00
New Revision: 317a2b1f6004708b28969c47b48526f6378c4e64

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

LOG: [LLDB] Remove XFAIL from minidebuginfo-set-and-hit-breakpoint.test

This patch removes XFAIL from minidebuginfo-set-and-hit-breakpoint.test.
Test started passing after 3c4ed02698afec021c6bca80740d1e58e3ee019e.

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

Added: 


Modified: 
lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test

Removed: 




diff  --git 
a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test 
b/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test
index f361ccb585783..a9650614674ea 100644
--- a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test
+++ b/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test
@@ -1,4 +1,3 @@
-# XFAIL: target-arm && linux-gnu
 # REQUIRES: system-linux, lzma, xz
 
 # We want to keep the symbol "multiplyByThree" in the .dynamic section and not



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


[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

2022-02-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/tools/driver/Driver.cpp:673-674
+static void sigtstp_handler(int signo) {
   if (g_driver != nullptr)
 g_driver->GetDebugger().SaveInputTerminalState();
 

JDevlieghere wrote:
> andcarminati wrote:
> > JDevlieghere wrote:
> > > labath wrote:
> > > > JDevlieghere wrote:
> > > > > I see an opportunity for a little RAII helper.
> > > > What kind of a helper did you have in mind? Practically the entire 
> > > > function consists of setup and teardown in preparation for the 
> > > > `raise(signo)` call. If I wanted to be fancy I could put all of that in 
> > > > a helper, but I don't think that would make it cleaner. Plus, we also 
> > > > need to be careful about the functions we call from a signal handler, 
> > > > and I really don't know whether e.g. `llvm::make_scope_exit` is 
> > > > guaranteed to not allocate (heap) memory.
> > > I was only referring to the Save/RestoreInputTerminalState() part of this 
> > > function. Something like:
> > > 
> > > ```
> > > class TerminalStateRAII() {
> > > public:
> > >   TerminalStateRAII(Driver* driver) : driver(m_driver) {
> > > if (m_driver)
> > >   m_driver->GetDebugger().SaveInputTerminalState();
> > >   }
> > > 
> > >   ~SignalHelper() {
> > > if (m_driver)
> > >   m_driver->GetDebugger().SaveInputTerminalState();
> > >   }
> > > 
> > > private:
> > >   Driver* m_driver;
> > > };
> > > ```
> > > 
> > > Obviously, this isn't at all important, just something that came to mind.
> > I think this is a good idea to reduce code duplication. Another approach:
> > 
> > ```
> > class TerminalStateRAII() {
> > public:
> >   TerminalStateRAII(Driver* driver) : driver(m_driver) {
> > SaveInputTerminalState();
> >   }
> > 
> >   ~TerminalStateRAII() {
> > SaveInputTerminalState();
> >   }
> > 
> > private:
> >   Driver* m_driver;
> >   void SaveInputTerminalState(){
> > if (m_driver)
> >   m_driver->GetDebugger().SaveInputTerminalState();
> >   }
> > };
> > 
> > ```
> That's a typo on my part, the destructor needs to call 
> `RestoreInputTerminalState` (as opposed to `SaveInputTerminalState`).
Ok, I see. If this was a more complex function (e.g. multiple return points), 
then I'd agree, but this function is really simple and linear (just like a 
signal handler should be). I am not convinced by the "code duplication" 
argument -- the way I see it, the helper class replaces 4 lines of (simple) 
code with ~15 lines of boilerplate. And this function is literally the only 
caller of Save/RestoreInputTerminalState.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120320

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


[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

2022-02-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/source/Core/Address.cpp:739
-  s->PutCString(", location = ");
-  var_sp->DumpLocationForAddress(s, *this);
-  s->PutCString(", decl = ");

zequanwu wrote:
> labath wrote:
> > This place was the only caller of Variable::DumpLocationForAddress. What if 
> > we, instead of duplicating its logic here, just change the function to do 
> > what we want?
> > The interface could be as simple as `Variable::DumpLocations(Stream&, 
> > Address)` where, if one provides an invalid Address, then all locations get 
> > dumped, and one can request a specific location by providing a concrete 
> > address.
> > We might even do something similar for the DWARFExpression class, and avoid 
> > the filter callbacks completely.
> I just moved the filter inside `Variable::DumpLocations`, because we still 
> need to pass the filter callback to `DWARFExpression::DumpLocaitons` inside 
> `Variable::DumpLocaitons` to choose dumping all ranges + locations or just 
> one range location. 
Looking at it now, I think it'd be better to inline the callback all the way 
into DWARFExpression::DumpLocations. The callback is more flexible, but I'm not 
convinced that we need that flexibility, and the interface of the callback is 
pretty complex (two bool return values). I believe it would be sufficient if 
the function accepted an extra address argument (as an addr_t probably), and 
used an invalid address (LLDB_INVALID_ADDRESS) to mean "dump all locations".



Comment at: lldb/source/Expression/DWARFExpression.cpp:2684
+  }
+  bool is_first = true;
+  auto callback = [&](llvm::DWARFLocationExpression loc) -> bool {

llvm::ListSeparator separator;



Comment at: lldb/source/Expression/DWARFExpression.cpp:2692-2693
+   m_data.GetAddressByteSize());
+  if (!is_first)
+s->PutCString(", ");
+  s->PutCString("[");

s->AsRawOstream() << separator;



Comment at: lldb/source/Symbol/Variable.cpp:449
+bool Variable::DumpLocations(Stream *s, lldb::DescriptionLevel level,
+ lldb::addr_t func_load_addr, ABI *abi,
+ const Address ) {

Can we keep `abi` and `func_load_addr` computations in this function? (It 
probably was easier to make them arguments starting from the last version of 
your patch, but putting the code here would reduce the diff from HEAD and also 
make the interface simpler).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119963

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


[Lldb-commits] [PATCH] D120517: PlatformMacOSX should be activated for lldb built to run on an iOS etc device

2022-02-25 Thread Jason Molenda via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcd2ba23efb3e: PlatformMacOSX should be activated for lldb 
built to run on an iOS etc device (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120517

Files:
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp


Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -137,15 +137,21 @@
   std::vector result;
 #if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
   // macOS for ARM64 support both native and translated x86_64 processes
-  ARMGetSupportedArchitectures(result, llvm::Triple::MacOSX);
 
-  // We can't use x86GetSupportedArchitectures() because it uses
-  // the system architecture for some of its return values and also
-  // has a 32bits variant.
-  result.push_back(ArchSpec("x86_64-apple-macosx"));
-  result.push_back(ArchSpec("x86_64-apple-ios-macabi"));
-  result.push_back(ArchSpec("arm64-apple-ios-macabi"));
-  result.push_back(ArchSpec("arm64e-apple-ios-macabi"));
+  // When cmdline lldb is run on iOS, watchOS, etc, it is still
+  // using "PlatformMacOSX".
+  llvm::Triple::OSType host_os = GetHostOSType();
+  ARMGetSupportedArchitectures(result, host_os);
+
+  if (host_os == llvm::Triple::MacOSX) {
+// We can't use x86GetSupportedArchitectures() because it uses
+// the system architecture for some of its return values and also
+// has a 32bits variant.
+result.push_back(ArchSpec("x86_64-apple-macosx"));
+result.push_back(ArchSpec("x86_64-apple-ios-macabi"));
+result.push_back(ArchSpec("arm64-apple-ios-macabi"));
+result.push_back(ArchSpec("arm64e-apple-ios-macabi"));
+  }
 #else
   x86GetSupportedArchitectures(result);
   result.push_back(ArchSpec("x86_64-apple-ios-macabi"));
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -174,6 +174,9 @@
   static std::string FindComponentInPath(llvm::StringRef path,
  llvm::StringRef component);
 
+  // The OSType where lldb is running.
+  static llvm::Triple::OSType GetHostOSType();
+
   std::string m_developer_directory;
   llvm::StringMap m_sdk_path;
   std::mutex m_sdk_path_mutex;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1328,3 +1328,23 @@
 return FileSpec(FindComponentInPath(fspec.GetPath(), "CommandLineTools"));
   return {};
 }
+
+llvm::Triple::OSType PlatformDarwin::GetHostOSType() {
+#if !defined(__APPLE__)
+  return llvm::Triple::MacOSX;
+#else
+#if TARGET_OS_OSX
+  return llvm::Triple::MacOSX;
+#elif TARGET_OS_IOS
+  return llvm::Triple::IOS;
+#elif TARGET_OS_WATCH
+  return llvm::Triple::WatchOS;
+#elif TARGET_OS_TV
+  return llvm::Triple::TvOS;
+#elif TARGET_OS_BRIDGE
+  return llvm::Triple::BridgeOS;
+#else
+#error "LLDB being compiled for an unrecognized Darwin OS"
+#endif
+#endif // __APPLE__
+}


Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -137,15 +137,21 @@
   std::vector result;
 #if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
   // macOS for ARM64 support both native and translated x86_64 processes
-  ARMGetSupportedArchitectures(result, llvm::Triple::MacOSX);
 
-  // We can't use x86GetSupportedArchitectures() because it uses
-  // the system architecture for some of its return values and also
-  // has a 32bits variant.
-  result.push_back(ArchSpec("x86_64-apple-macosx"));
-  result.push_back(ArchSpec("x86_64-apple-ios-macabi"));
-  result.push_back(ArchSpec("arm64-apple-ios-macabi"));
-  result.push_back(ArchSpec("arm64e-apple-ios-macabi"));
+  // When cmdline lldb is run on iOS, watchOS, etc, it is still
+  // using "PlatformMacOSX".
+  llvm::Triple::OSType host_os = GetHostOSType();
+  ARMGetSupportedArchitectures(result, host_os);
+
+  if (host_os == llvm::Triple::MacOSX) {
+// We can't use x86GetSupportedArchitectures() because it uses
+// the system architecture for some of its return values and also
+// has a 32bits variant.
+result.push_back(ArchSpec("x86_64-apple-macosx"));
+

[Lldb-commits] [lldb] cd2ba23 - PlatformMacOSX should be activated for lldb built to run on an iOS etc device

2022-02-25 Thread Jason Molenda via lldb-commits

Author: Jason Molenda
Date: 2022-02-25T00:55:54-08:00
New Revision: cd2ba23efb3e282d86cf7a94952f0bf61677fa7f

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

LOG: PlatformMacOSX should be activated for lldb built to run on an iOS etc 
device

In the changes Jonas made in https://reviews.llvm.org/D117340 , a
small oversight was that PlatformMacOSX (despite the name) is active
for any native Darwin operating system, where lldb and the target
process are running on the same system. This patch uses compile-time
checks to return the appropriate OSType for the OS lldb is being
compiled to, so the "host" platform will correctly be selected when
lldb & the inferior are both running on that OS. And a small change
to PlatformMacOSX::GetSupportedArchitectures which adds additional
recognized triples when running on macOS but not other native Darwin
systems.

Differential Revision: https://reviews.llvm.org/D120517
rdar://89247060

Added: 


Modified: 
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 4b3ac0bf06838..706f3ae71bb3c 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1328,3 +1328,23 @@ FileSpec 
PlatformDarwin::GetCurrentCommandLineToolsDirectory() {
 return FileSpec(FindComponentInPath(fspec.GetPath(), "CommandLineTools"));
   return {};
 }
+
+llvm::Triple::OSType PlatformDarwin::GetHostOSType() {
+#if !defined(__APPLE__)
+  return llvm::Triple::MacOSX;
+#else
+#if TARGET_OS_OSX
+  return llvm::Triple::MacOSX;
+#elif TARGET_OS_IOS
+  return llvm::Triple::IOS;
+#elif TARGET_OS_WATCH
+  return llvm::Triple::WatchOS;
+#elif TARGET_OS_TV
+  return llvm::Triple::TvOS;
+#elif TARGET_OS_BRIDGE
+  return llvm::Triple::BridgeOS;
+#else
+#error "LLDB being compiled for an unrecognized Darwin OS"
+#endif
+#endif // __APPLE__
+}

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
index 57617ae58c89d..7d5968c48f254 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -174,6 +174,9 @@ class PlatformDarwin : public PlatformPOSIX {
   static std::string FindComponentInPath(llvm::StringRef path,
  llvm::StringRef component);
 
+  // The OSType where lldb is running.
+  static llvm::Triple::OSType GetHostOSType();
+
   std::string m_developer_directory;
   llvm::StringMap m_sdk_path;
   std::mutex m_sdk_path_mutex;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
index 9639e1b0b2cb4..17bfb52e6a041 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -137,15 +137,21 @@ std::vector 
PlatformMacOSX::GetSupportedArchitectures() {
   std::vector result;
 #if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
   // macOS for ARM64 support both native and translated x86_64 processes
-  ARMGetSupportedArchitectures(result, llvm::Triple::MacOSX);
 
-  // We can't use x86GetSupportedArchitectures() because it uses
-  // the system architecture for some of its return values and also
-  // has a 32bits variant.
-  result.push_back(ArchSpec("x86_64-apple-macosx"));
-  result.push_back(ArchSpec("x86_64-apple-ios-macabi"));
-  result.push_back(ArchSpec("arm64-apple-ios-macabi"));
-  result.push_back(ArchSpec("arm64e-apple-ios-macabi"));
+  // When cmdline lldb is run on iOS, watchOS, etc, it is still
+  // using "PlatformMacOSX".
+  llvm::Triple::OSType host_os = GetHostOSType();
+  ARMGetSupportedArchitectures(result, host_os);
+
+  if (host_os == llvm::Triple::MacOSX) {
+// We can't use x86GetSupportedArchitectures() because it uses
+// the system architecture for some of its return values and also
+// has a 32bits variant.
+result.push_back(ArchSpec("x86_64-apple-macosx"));
+result.push_back(ArchSpec("x86_64-apple-ios-macabi"));
+result.push_back(ArchSpec("arm64-apple-ios-macabi"));
+result.push_back(ArchSpec("arm64e-apple-ios-macabi"));
+  }
 #else
   x86GetSupportedArchitectures(result);
   result.push_back(ArchSpec("x86_64-apple-ios-macabi"));



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