[Lldb-commits] [PATCH] D67222: [Windows] Added support of watchpoints to `NativeProcessWindows`

[Aleksandr Urakov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373300: [Windows] Added support of watchpoints to 
`NativeProcessWindows` (authored by aleksandr.urakov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67222?vs=221671=222545#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67222

Files:
  lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp
  
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h
  
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp
  
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h
  
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp
  
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h
  lldb/trunk/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp

Index: lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
+++ lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
@@ -431,13 +431,34 @@
   ExceptionResult result = ExceptionResult::SendToApplication;
   switch (record.GetExceptionCode()) {
   case DWORD(STATUS_SINGLE_STEP):
-  case STATUS_WX86_SINGLE_STEP:
-StopThread(record.GetThreadID(), StopReason::eStopReasonTrace);
+  case STATUS_WX86_SINGLE_STEP: {
+uint32_t wp_id = LLDB_INVALID_INDEX32;
+if (NativeThreadWindows *thread = GetThreadByID(record.GetThreadID())) {
+  NativeRegisterContextWindows _ctx = thread->GetRegisterContext();
+  Status error =
+  reg_ctx.GetWatchpointHitIndex(wp_id, record.GetExceptionAddress());
+  if (error.Fail())
+LLDB_LOG(log,
+ "received error while checking for watchpoint hits, pid = "
+ "{0}, error = {1}",
+ thread->GetID(), error);
+  if (wp_id != LLDB_INVALID_INDEX32) {
+addr_t wp_addr = reg_ctx.GetWatchpointAddress(wp_id);
+addr_t wp_hit_addr = reg_ctx.GetWatchpointHitAddress(wp_id);
+std::string desc =
+formatv("{0} {1} {2}", wp_addr, wp_id, wp_hit_addr).str();
+StopThread(record.GetThreadID(), StopReason::eStopReasonWatchpoint,
+   desc);
+  }
+}
+if (wp_id == LLDB_INVALID_INDEX32)
+  StopThread(record.GetThreadID(), StopReason::eStopReasonTrace);
+
 SetState(eStateStopped, true);
 
 // Continue the debugger.
 return ExceptionResult::MaskException;
-
+  }
   case DWORD(STATUS_BREAKPOINT):
   case STATUS_WX86_BREAKPOINT:
 if (FindSoftwareBreakpoint(record.GetExceptionAddress())) {
@@ -513,8 +534,16 @@
 
 void NativeProcessWindows::OnCreateThread(const HostThread _thread) {
   llvm::sys::ScopedLock lock(m_mutex);
-  m_threads.push_back(
-  std::make_unique(*this, new_thread));
+
+  auto thread = std::make_unique(*this, new_thread);
+  thread->GetRegisterContext().ClearAllHardwareWatchpoints();
+  for (const auto  : GetWatchpointMap()) {
+const NativeWatchpoint  = pair.second;
+thread->SetWatchpoint(wp.m_addr, wp.m_size, wp.m_watch_flags,
+  wp.m_hardware);
+  }
+
+  m_threads.push_back(std::move(thread));
 }
 
 void NativeProcessWindows::OnExitThread(lldb::tid_t thread_id,
Index: lldb/trunk/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp
===
--- lldb/trunk/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp
+++ lldb/trunk/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp
@@ -132,11 +132,30 @@
 
 Status NativeThreadWindows::SetWatchpoint(lldb::addr_t addr, size_t size,
   uint32_t watch_flags, bool hardware) {
-  return Status("unimplemented.");
+  if (!hardware)
+return Status("not implemented");
+  if (m_state == eStateLaunching)
+return Status();
+  Status error = RemoveWatchpoint(addr);
+  if (error.Fail())
+return error;
+  uint32_t wp_index =
+  m_reg_context_up->SetHardwareWatchpoint(addr, size, watch_flags);
+  if (wp_index == LLDB_INVALID_INDEX32)
+return Status("Setting hardware watchpoint failed.");
+  m_watchpoint_index_map.insert({addr, wp_index});
+  return Status();
 }
 
 Status NativeThreadWindows::RemoveWatchpoint(lldb::addr_t addr) {
-  return Status("unimplemented");
+  auto wp = m_watchpoint_index_map.find(addr);
+  if (wp == m_watchpoint_index_map.end())
+return Status();
+  uint32_t wp_index = wp->second;
+  m_watchpoint_index_map.erase(wp);
+  if 

[Lldb-commits] [PATCH] D67222: [Windows] Added support of watchpoints to `NativeProcessWindows`

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov added a comment.

Thank you! I've added a switch in a manner similar to one that Zachary has 
chosen for SymbolFilePDB/NativePDB plugins here: D68258 



Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67222



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

[Lldb-commits] [lldb] r373300 - [Windows] Added support of watchpoints to `NativeProcessWindows`

[Aleksandr Urakov via lldb-commits]

Author: aleksandr.urakov
Date: Mon Sep 30 22:52:16 2019
New Revision: 373300

URL: http://llvm.org/viewvc/llvm-project?rev=373300=rev
Log:
[Windows] Added support of watchpoints to `NativeProcessWindows`

Summary: This patch adds support of watchpoints to the new 
`NativeProcessWindows` plugin. The same tests as in D67168 pass with these 
changes when the old plugin is turned off, so they will cover this 
functionality when the old plugin is gone.

Reviewers: asmith, amccarth, stella.stamenova, labath

Reviewed By: labath

Subscribers: labath, jfb, JDevlieghere, lldb-commits, leonid.mashinskiy

Tags: #lldb

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

Modified:
lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp

lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp

lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.h

lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.cpp

lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_i386.h

lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.cpp

lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_x86_64.h
lldb/trunk/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp

Modified: 
lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp?rev=373300=373299=373300=diff
==
--- lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp 
Mon Sep 30 22:52:16 2019
@@ -431,13 +431,34 @@ NativeProcessWindows::OnDebugException(b
   ExceptionResult result = ExceptionResult::SendToApplication;
   switch (record.GetExceptionCode()) {
   case DWORD(STATUS_SINGLE_STEP):
-  case STATUS_WX86_SINGLE_STEP:
-StopThread(record.GetThreadID(), StopReason::eStopReasonTrace);
+  case STATUS_WX86_SINGLE_STEP: {
+uint32_t wp_id = LLDB_INVALID_INDEX32;
+if (NativeThreadWindows *thread = GetThreadByID(record.GetThreadID())) {
+  NativeRegisterContextWindows _ctx = thread->GetRegisterContext();
+  Status error =
+  reg_ctx.GetWatchpointHitIndex(wp_id, record.GetExceptionAddress());
+  if (error.Fail())
+LLDB_LOG(log,
+ "received error while checking for watchpoint hits, pid = "
+ "{0}, error = {1}",
+ thread->GetID(), error);
+  if (wp_id != LLDB_INVALID_INDEX32) {
+addr_t wp_addr = reg_ctx.GetWatchpointAddress(wp_id);
+addr_t wp_hit_addr = reg_ctx.GetWatchpointHitAddress(wp_id);
+std::string desc =
+formatv("{0} {1} {2}", wp_addr, wp_id, wp_hit_addr).str();
+StopThread(record.GetThreadID(), StopReason::eStopReasonWatchpoint,
+   desc);
+  }
+}
+if (wp_id == LLDB_INVALID_INDEX32)
+  StopThread(record.GetThreadID(), StopReason::eStopReasonTrace);
+
 SetState(eStateStopped, true);
 
 // Continue the debugger.
 return ExceptionResult::MaskException;
-
+  }
   case DWORD(STATUS_BREAKPOINT):
   case STATUS_WX86_BREAKPOINT:
 if (FindSoftwareBreakpoint(record.GetExceptionAddress())) {
@@ -513,8 +534,16 @@ NativeProcessWindows::OnDebugException(b
 
 void NativeProcessWindows::OnCreateThread(const HostThread _thread) {
   llvm::sys::ScopedLock lock(m_mutex);
-  m_threads.push_back(
-  std::make_unique(*this, new_thread));
+
+  auto thread = std::make_unique(*this, new_thread);
+  thread->GetRegisterContext().ClearAllHardwareWatchpoints();
+  for (const auto  : GetWatchpointMap()) {
+const NativeWatchpoint  = pair.second;
+thread->SetWatchpoint(wp.m_addr, wp.m_size, wp.m_watch_flags,
+  wp.m_hardware);
+  }
+
+  m_threads.push_back(std::move(thread));
 }
 
 void NativeProcessWindows::OnExitThread(lldb::tid_t thread_id,

Modified: 
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp?rev=373300=373299=373300=diff
==
--- 
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_WoW64.cpp
 Mon Sep 30 22:52:16 2019
@@ -56,12 +56,14 @@ CreateRegisterInfoInterface(const ArchSp
   return new RegisterContextWindows_i386(target_arch);
 }
 
-static Status GetWoW64ThreadContextHelper(lldb::thread_t thread_handle,
-  PWOW64_CONTEXT 

[Lldb-commits] [PATCH] D68258: [Windows] Introduce a switch for the `lldb-server` mode on Windows

[Aleksandr Urakov via Phabricator via lldb-commits]

aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: labath, amccarth, asmith, stella.stamenova.
aleksandr.urakov added a project: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.

This patch introduces a switch, based on the environment variable 
`LLDB_USE_LLDB_SERVER`, to determine whether to use the `ProcessWindows` plugin 
(the old way) or the `lldb-server` way for debugging on Windows.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68258

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp


Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -81,13 +81,24 @@
   return ProcessSP(new ProcessWindows(target_sp, listener_sp));
 }
 
-void ProcessWindows::Initialize() {
-  static llvm::once_flag g_once_flag;
+static bool ShouldUseLLDBServer() {
+  llvm::StringRef use_lldb_server = ::getenv("LLDB_USE_LLDB_SERVER");
+  return use_lldb_server.equals_lower("on") ||
+ use_lldb_server.equals_lower("yes") ||
+ use_lldb_server.equals_lower("1") ||
+ use_lldb_server.equals_lower("true");
+}
 
-  llvm::call_once(g_once_flag, []() {
-PluginManager::RegisterPlugin(GetPluginNameStatic(),
-  GetPluginDescriptionStatic(), 
CreateInstance);
-  });
+void ProcessWindows::Initialize() {
+  if (!ShouldUseLLDBServer()) {
+static llvm::once_flag g_once_flag;
+
+llvm::call_once(g_once_flag, []() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(),
+CreateInstance);
+});
+  }
 }
 
 void ProcessWindows::Terminate() {}


Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -81,13 +81,24 @@
   return ProcessSP(new ProcessWindows(target_sp, listener_sp));
 }
 
-void ProcessWindows::Initialize() {
-  static llvm::once_flag g_once_flag;
+static bool ShouldUseLLDBServer() {
+  llvm::StringRef use_lldb_server = ::getenv("LLDB_USE_LLDB_SERVER");
+  return use_lldb_server.equals_lower("on") ||
+ use_lldb_server.equals_lower("yes") ||
+ use_lldb_server.equals_lower("1") ||
+ use_lldb_server.equals_lower("true");
+}
 
-  llvm::call_once(g_once_flag, []() {
-PluginManager::RegisterPlugin(GetPluginNameStatic(),
-  GetPluginDescriptionStatic(), CreateInstance);
-  });
+void ProcessWindows::Initialize() {
+  if (!ShouldUseLLDBServer()) {
+static llvm::once_flag g_once_flag;
+
+llvm::call_once(g_once_flag, []() {
+  PluginManager::RegisterPlugin(GetPluginNameStatic(),
+GetPluginDescriptionStatic(),
+CreateInstance);
+});
+  }
 }
 
 void ProcessWindows::Terminate() {}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68171: Remove unused "append" parameter from FindTypes API

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

This broke the windows bot. A couple of other issues have shown up and been 
fixed since, but this change is still causing a break:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/9373


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68171



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

[Lldb-commits] [PATCH] D68188: allow arbitrary python streams to be converted to SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D68188#1687532 , @labath wrote:

> It seems to be that instead of subclassing a fully-functional File class, it 
> would be better to create an abstract class, that both `File` and the new 
> python thingy could implement. That would also create a natural place to 
> write down the FILE* conversion semantics and other stuff we talked about at 
> lldb-dev. I don't have any opinion as to whether the name `File` should 
> denote the new abstract class, or the real thing, but it should most likely 
> be done as a separate patch.
>
> What do you think about that?


I could do it that way, but I have some reservations about it.In this 
version the class hierarchy looks like this:

File
 |
   SimplePythonFile +
|   |
  TextPythonFile BinaryPythonFile

If we added an abstract base class, I guess it would look like this:

AbstractFile
 ||
File  PythonFile
 | | | |
   SimplePythonFile  | |
 | |
+ ---+ |  
|  |
  TextPythonFile BinaryPythonFile

Does a more complicated class hierarchy like that really solve a problem or 
make the code easier to understand?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188



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

[Lldb-commits] [PATCH] D68181: SBDebugger::SetInputFile, SetOutputFile, etc.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 222538.
lawrence_danna added a comment.

assertions instead of exceptions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68181

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Core/Debugger.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/interface/SBDebugger.i
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -954,6 +954,8 @@
 PythonFile::~PythonFile() {}
 
 bool PythonFile::Check(PyObject *py_obj) {
+  if (!py_obj)
+return false;
 #if PY_MAJOR_VERSION < 3
   return PyFile_Check(py_obj);
 #else
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -821,31 +821,22 @@
 
 repro::DataRecorder *Debugger::GetInputRecorder() { return m_input_recorder; }
 
-void Debugger::SetInputFileHandle(FILE *fh, bool tranfer_ownership,
-  repro::DataRecorder *recorder) {
+void Debugger::SetInputFile(FileSP file_sp, repro::DataRecorder *recorder) {
+  assert(file_sp && file_sp->IsValid());
   m_input_recorder = recorder;
-
-  m_input_file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!m_input_file_sp->IsValid())
-m_input_file_sp = std::make_shared(stdin, false);
-
+  m_input_file_sp = file_sp;
   // Save away the terminal state if that is relevant, so that we can restore
   // it in RestoreInputState.
   SaveInputTerminalState();
 }
 
-void Debugger::SetOutputFileHandle(FILE *fh, bool tranfer_ownership) {
-  FileSP file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!file_sp->IsValid())
-file_sp = std::make_shared(stdout, false);
+void Debugger::SetOutputFile(FileSP file_sp) {
+  assert(file_sp && file_sp->IsValid());
   m_output_stream_sp = std::make_shared(file_sp);
-
 }
 
-void Debugger::SetErrorFileHandle(FILE *fh, bool tranfer_ownership) {
-  FileSP file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!file_sp->IsValid())
-file_sp = std::make_shared(stderr, false);
+void Debugger::SetErrorFile(FileSP file_sp) {
+  assert(file_sp && file_sp->IsValid());
   m_error_stream_sp = std::make_shared(file_sp);
 }
 
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -18,6 +18,7 @@
 #include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBEvent.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
@@ -285,49 +286,96 @@
 m_opaque_sp->GetCommandInterpreter().SkipAppInitFiles(b);
 }
 
-// Shouldn't really be settable after initialization as this could cause lots
-// of problems; don't want users trying to switch modes in the middle of a
-// debugging session.
 void SBDebugger::SetInputFileHandle(FILE *fh, bool transfer_ownership) {
   LLDB_RECORD_METHOD(void, SBDebugger, SetInputFileHandle, (FILE *, bool), fh,
  transfer_ownership);
+  SetInputFile(std::make_shared(fh, transfer_ownership));
+}
 
-  if (!m_opaque_sp)
-return;
+// Shouldn't really be settable after initialization as this could cause lots
+// of problems; don't want users trying to switch modes in the middle of a
+// debugging session.
+SBError SBDebugger::SetInputFile(SBFile file) {
+  LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (SBFile), file);
+
+  SBError error;
+  if (!m_opaque_sp) {
+error.ref().SetErrorString("invalid debugger");
+return error;
+  }
 
   repro::DataRecorder *recorder = nullptr;
   if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
 recorder = g->GetOrCreate().GetNewDataRecorder();
 
+  FileSP file_sp = file.m_opaque_sp;
+
   static std::unique_ptr loader =
   repro::CommandLoader::Create(repro::Reproducer::Instance().GetLoader());
   if (loader) {
-llvm::Optional file = loader->GetNextFile();
-fh = file ? FileSystem::Instance().Fopen(file->c_str(), "r") : nullptr;
+llvm::Optional nextfile = loader->GetNextFile();
+FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
+: nullptr;
+// FIXME Jonas Devlieghere: shouldn't this error be propagated out to the
+// reproducer somehow if fh is NULL?
+if (fh) {
+  file_sp = std::make_shared(fh, true);
+}
+  }
+
+  if (!file_sp || 

[Lldb-commits] [PATCH] D68181: SBDebugger::SetInputFile, SetOutputFile, etc.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 222537.
lawrence_danna added a comment.

pushed validity checks out to the SB layer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68181

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Core/Debugger.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/interface/SBDebugger.i
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -954,6 +954,8 @@
 PythonFile::~PythonFile() {}
 
 bool PythonFile::Check(PyObject *py_obj) {
+  if (!py_obj)
+return false;
 #if PY_MAJOR_VERSION < 3
   return PyFile_Check(py_obj);
 #else
Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -821,31 +821,22 @@
 
 repro::DataRecorder *Debugger::GetInputRecorder() { return m_input_recorder; }
 
-void Debugger::SetInputFileHandle(FILE *fh, bool tranfer_ownership,
-  repro::DataRecorder *recorder) {
+void Debugger::SetInputFile(FileSP file_sp, repro::DataRecorder *recorder) {
+  assert(file_sp && file_sp->IsValid());
   m_input_recorder = recorder;
-
-  m_input_file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!m_input_file_sp->IsValid())
-m_input_file_sp = std::make_shared(stdin, false);
-
+  m_input_file_sp = file_sp;
   // Save away the terminal state if that is relevant, so that we can restore
   // it in RestoreInputState.
   SaveInputTerminalState();
 }
 
-void Debugger::SetOutputFileHandle(FILE *fh, bool tranfer_ownership) {
-  FileSP file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!file_sp->IsValid())
-file_sp = std::make_shared(stdout, false);
+void Debugger::SetOutputFile(FileSP file_sp) {
+  assert(file_sp && file_sp->IsValid());
   m_output_stream_sp = std::make_shared(file_sp);
-
 }
 
-void Debugger::SetErrorFileHandle(FILE *fh, bool tranfer_ownership) {
-  FileSP file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!file_sp->IsValid())
-file_sp = std::make_shared(stderr, false);
+void Debugger::SetErrorFile(FileSP file_sp) {
+  assert(file_sp && file_sp->IsValid());
   m_error_stream_sp = std::make_shared(file_sp);
 }
 
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -18,6 +18,7 @@
 #include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBEvent.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
@@ -285,49 +286,96 @@
 m_opaque_sp->GetCommandInterpreter().SkipAppInitFiles(b);
 }
 
-// Shouldn't really be settable after initialization as this could cause lots
-// of problems; don't want users trying to switch modes in the middle of a
-// debugging session.
 void SBDebugger::SetInputFileHandle(FILE *fh, bool transfer_ownership) {
   LLDB_RECORD_METHOD(void, SBDebugger, SetInputFileHandle, (FILE *, bool), fh,
  transfer_ownership);
+  SetInputFile(std::make_shared(fh, transfer_ownership));
+}
 
-  if (!m_opaque_sp)
-return;
+// Shouldn't really be settable after initialization as this could cause lots
+// of problems; don't want users trying to switch modes in the middle of a
+// debugging session.
+SBError SBDebugger::SetInputFile(SBFile file) {
+  LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (SBFile), file);
+
+  SBError error;
+  if (!m_opaque_sp) {
+error.ref().SetErrorString("invalid debugger");
+return error;
+  }
 
   repro::DataRecorder *recorder = nullptr;
   if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
 recorder = g->GetOrCreate().GetNewDataRecorder();
 
+  FileSP file_sp = file.m_opaque_sp;
+
   static std::unique_ptr loader =
   repro::CommandLoader::Create(repro::Reproducer::Instance().GetLoader());
   if (loader) {
-llvm::Optional file = loader->GetNextFile();
-fh = file ? FileSystem::Instance().Fopen(file->c_str(), "r") : nullptr;
+llvm::Optional nextfile = loader->GetNextFile();
+FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
+: nullptr;
+// FIXME Jonas Devlieghere: shouldn't this error be propagated out to the
+// reproducer somehow if fh is NULL?
+if (fh) {
+  file_sp = std::make_shared(fh, true);
+}
+  }
+
+  if (!file_sp || 

[Lldb-commits] [PATCH] D68160: File::Clear() -> File::TakeStreamAndClear()

[Lawrence D'Anna via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
lawrence_danna marked an inline comment as done.
Closed by commit rL373285: File::Clear() - File::TakeStreamAndClear() 
(authored by lawrence_danna, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68160?vs=222459=222532#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68160

Files:
  lldb/trunk/include/lldb/Host/File.h
  lldb/trunk/scripts/Python/python-typemaps.swig
  lldb/trunk/source/Host/common/File.cpp


Index: lldb/trunk/source/Host/common/File.cpp
===
--- lldb/trunk/source/Host/common/File.cpp
+++ lldb/trunk/source/Host/common/File.cpp
@@ -162,13 +162,16 @@
   return error;
 }
 
-void File::Clear() {
-  m_stream = nullptr;
+FILE *File::TakeStreamAndClear() {
+  FILE *stream = GetStream();
+  m_stream = NULL;
   m_descriptor = kInvalidDescriptor;
   m_options = 0;
   m_own_stream = false;
+  m_own_descriptor = false;
   m_is_interactive = m_supports_colors = m_is_real_terminal =
   eLazyBoolCalculate;
+  return stream;
 }
 
 Status File::GetFileSpec(FileSpec _spec) const {
Index: lldb/trunk/scripts/Python/python-typemaps.swig
===
--- lldb/trunk/scripts/Python/python-typemaps.swig
+++ lldb/trunk/scripts/Python/python-typemaps.swig
@@ -372,6 +372,9 @@
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+// FIXME both of these paths wind up calling fdopen() with no provision for 
ever calling
+// fclose() on the result.  SB interfaces that use FILE* should be deprecated 
for scripting
+// use and this typemap should eventually be removed.
 %typemap(in) FILE * {
using namespace lldb_private;
if ($input == Py_None)
@@ -398,9 +401,7 @@
   lldb::FileUP file = py_file.GetUnderlyingFile();
   if (!file)
  return nullptr;
-  $1 = file->GetStream();
-  if ($1)
- file->Clear();
+  $1 = file->TakeStreamAndClear();
 }
 }
 
Index: lldb/trunk/include/lldb/Host/File.h
===
--- lldb/trunk/include/lldb/Host/File.h
+++ lldb/trunk/include/lldb/Host/File.h
@@ -120,7 +120,19 @@
 
   Status Close() override;
 
-  void Clear();
+  /// DEPRECATED! Extract the underlying FILE* and reset this File without 
closing it.
+  ///
+  /// This is only here to support legacy SB interfaces that need to convert 
scripting
+  /// language objects into FILE* streams.   That conversion is inherently 
sketchy and
+  /// doing so may cause the stream to be leaked.
+  ///
+  /// After calling this the File will be reset to its original state.  It 
will be
+  /// invalid and it will not hold on to any resources.
+  ///
+  /// \return
+  /// The underlying FILE* stream from this File, if one exists and can be 
extracted,
+  /// nullptr otherwise.
+  FILE *TakeStreamAndClear();
 
   int GetDescriptor() const;
 


Index: lldb/trunk/source/Host/common/File.cpp
===
--- lldb/trunk/source/Host/common/File.cpp
+++ lldb/trunk/source/Host/common/File.cpp
@@ -162,13 +162,16 @@
   return error;
 }
 
-void File::Clear() {
-  m_stream = nullptr;
+FILE *File::TakeStreamAndClear() {
+  FILE *stream = GetStream();
+  m_stream = NULL;
   m_descriptor = kInvalidDescriptor;
   m_options = 0;
   m_own_stream = false;
+  m_own_descriptor = false;
   m_is_interactive = m_supports_colors = m_is_real_terminal =
   eLazyBoolCalculate;
+  return stream;
 }
 
 Status File::GetFileSpec(FileSpec _spec) const {
Index: lldb/trunk/scripts/Python/python-typemaps.swig
===
--- lldb/trunk/scripts/Python/python-typemaps.swig
+++ lldb/trunk/scripts/Python/python-typemaps.swig
@@ -372,6 +372,9 @@
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+// FIXME both of these paths wind up calling fdopen() with no provision for ever calling
+// fclose() on the result.  SB interfaces that use FILE* should be deprecated for scripting
+// use and this typemap should eventually be removed.
 %typemap(in) FILE * {
using namespace lldb_private;
if ($input == Py_None)
@@ -398,9 +401,7 @@
   lldb::FileUP file = py_file.GetUnderlyingFile();
   if (!file)
  return nullptr;
-  $1 = file->GetStream();
-  if ($1)
- file->Clear();
+  $1 = file->TakeStreamAndClear();
 }
 }
 
Index: lldb/trunk/include/lldb/Host/File.h
===
--- lldb/trunk/include/lldb/Host/File.h
+++ lldb/trunk/include/lldb/Host/File.h
@@ -120,7 +120,19 @@
 
   Status Close() override;
 
-  void Clear();
+  /// DEPRECATED! Extract the underlying FILE* and reset this File without closing it.
+  ///
+ 

[Lldb-commits] [lldb] r373285 - File::Clear() -> File::TakeStreamAndClear()

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Mon Sep 30 18:05:02 2019
New Revision: 373285

URL: http://llvm.org/viewvc/llvm-project?rev=373285=rev
Log:
File::Clear() -> File::TakeStreamAndClear()

Summary:
File::Clear() is an ugly function.  It's only used in one place,
which is the swig typemaps for FILE*.   This patch refactors and
renames that function to make it clear what it's really for and
why nobody else should use it.

Both File::TakeStreamAndClear() and the FILE* typemaps will be
removed in later patches after a suitable replacement is in place.

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/Host/File.h
lldb/trunk/scripts/Python/python-typemaps.swig
lldb/trunk/source/Host/common/File.cpp

Modified: lldb/trunk/include/lldb/Host/File.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/File.h?rev=373285=373284=373285=diff
==
--- lldb/trunk/include/lldb/Host/File.h (original)
+++ lldb/trunk/include/lldb/Host/File.h Mon Sep 30 18:05:02 2019
@@ -120,7 +120,19 @@ public:
 
   Status Close() override;
 
-  void Clear();
+  /// DEPRECATED! Extract the underlying FILE* and reset this File without 
closing it.
+  ///
+  /// This is only here to support legacy SB interfaces that need to convert 
scripting
+  /// language objects into FILE* streams.   That conversion is inherently 
sketchy and
+  /// doing so may cause the stream to be leaked.
+  ///
+  /// After calling this the File will be reset to its original state.  It 
will be
+  /// invalid and it will not hold on to any resources.
+  ///
+  /// \return
+  /// The underlying FILE* stream from this File, if one exists and can be 
extracted,
+  /// nullptr otherwise.
+  FILE *TakeStreamAndClear();
 
   int GetDescriptor() const;
 

Modified: lldb/trunk/scripts/Python/python-typemaps.swig
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/python-typemaps.swig?rev=373285=373284=373285=diff
==
--- lldb/trunk/scripts/Python/python-typemaps.swig (original)
+++ lldb/trunk/scripts/Python/python-typemaps.swig Mon Sep 30 18:05:02 2019
@@ -372,6 +372,9 @@ bool SetNumberFromPyObject(doubl
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+// FIXME both of these paths wind up calling fdopen() with no provision for 
ever calling
+// fclose() on the result.  SB interfaces that use FILE* should be deprecated 
for scripting
+// use and this typemap should eventually be removed.
 %typemap(in) FILE * {
using namespace lldb_private;
if ($input == Py_None)
@@ -398,9 +401,7 @@ bool SetNumberFromPyObject(doubl
   lldb::FileUP file = py_file.GetUnderlyingFile();
   if (!file)
  return nullptr;
-  $1 = file->GetStream();
-  if ($1)
- file->Clear();
+  $1 = file->TakeStreamAndClear();
 }
 }
 

Modified: lldb/trunk/source/Host/common/File.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/File.cpp?rev=373285=373284=373285=diff
==
--- lldb/trunk/source/Host/common/File.cpp (original)
+++ lldb/trunk/source/Host/common/File.cpp Mon Sep 30 18:05:02 2019
@@ -162,13 +162,16 @@ Status File::Close() {
   return error;
 }
 
-void File::Clear() {
-  m_stream = nullptr;
+FILE *File::TakeStreamAndClear() {
+  FILE *stream = GetStream();
+  m_stream = NULL;
   m_descriptor = kInvalidDescriptor;
   m_options = 0;
   m_own_stream = false;
+  m_own_descriptor = false;
   m_is_interactive = m_supports_colors = m_is_real_terminal =
   eLazyBoolCalculate;
+  return stream;
 }
 
 Status File::GetFileSpec(FileSpec _spec) const {


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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

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



Comment at: lldb/scripts/Python/python-typemaps.swig:464
 }
+
+// These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i,

`swig/LICENSE` says the following, so it should be fine to copy and paste these.

```
The SWIG library and examples, under the Lib and Examples top level 
directories, are distributed under the following terms:

  You may copy, modify, distribute, and make derivative works based on
  this software, in source code or object code form, without
  restriction. If you distribute the software to others, you may do
  so according to the terms of your choice. This software is offered as
  is, without warranty of any kind.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793



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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 222528.
lawrence_danna added a comment.

add reproducer instrumentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793

Files:
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBError.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Host/File.h
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/scripts/lldb.swig
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBFile.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -466,8 +466,6 @@
   void Reset(PyRefType type, PyObject *py_obj) override;
   void Reset(File , const char *mode);
 
-  static uint32_t GetOptionsFromMode(llvm::StringRef mode);
-
   lldb::FileUP GetUnderlyingFile() const;
 };
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -949,7 +949,6 @@
 
 PythonFile::PythonFile(File , const char *mode) { Reset(file, mode); }
 
-
 PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
 
 PythonFile::~PythonFile() {}
@@ -1014,22 +1013,6 @@
 #endif
 }
 
-uint32_t PythonFile::GetOptionsFromMode(llvm::StringRef mode) {
-  if (mode.empty())
-return 0;
-
-  return llvm::StringSwitch(mode.str())
-  .Case("r", File::eOpenOptionRead)
-  .Case("w", File::eOpenOptionWrite)
-  .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend |
- File::eOpenOptionCanCreate)
-  .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
-  .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
-  File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
-  .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
-  File::eOpenOptionAppend | File::eOpenOptionCanCreate)
-  .Default(0);
-}
 
 FileUP PythonFile::GetUnderlyingFile() const {
   if (!IsValid())
@@ -1038,7 +1021,7 @@
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType();
-  auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
+  auto options = File::GetOptionsFromMode(py_mode.GetString());
   auto file = std::make_unique(PyObject_AsFileDescriptor(m_py_obj),
  options, false);
   if (!file->IsValid())
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -68,6 +68,20 @@
   return nullptr;
 }
 
+uint32_t File::GetOptionsFromMode(llvm::StringRef mode) {
+  return llvm::StringSwitch(mode)
+  .Case("r", File::eOpenOptionRead)
+  .Case("w", File::eOpenOptionWrite)
+  .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend |
+ File::eOpenOptionCanCreate)
+  .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
+  .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
+  File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
+  .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
+  File::eOpenOptionAppend | File::eOpenOptionCanCreate)
+  .Default(0);
+}
+
 int File::kInvalidDescriptor = -1;
 FILE *File::kInvalidStream = nullptr;
 
@@ -143,9 +157,14 @@
 
 Status File::Close() {
   Status error;
-  if (StreamIsValid() && m_own_stream) {
-if (::fclose(m_stream) == EOF)
-  error.SetErrorToErrno();
+  if (StreamIsValid()) {
+if (m_own_stream) {
+  if (::fclose(m_stream) == EOF)
+error.SetErrorToErrno();
+} else {
+  if (::fflush(m_stream) == EOF)
+error.SetErrorToErrno();
+}
   }
 
   if (DescriptorIsValid() && m_own_descriptor) {
Index: lldb/source/API/SBReproducer.cpp
===
--- lldb/source/API/SBReproducer.cpp
+++ lldb/source/API/SBReproducer.cpp
@@ -52,6 +52,7 @@
   RegisterMethods(R);
   RegisterMethods(R);
   

[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex

[Jim Ingham via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373280: Allow the internal-state-thread free access to the 
TargetAPI mutex. (authored by jingham, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68174?vs=74=222527#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68174

Files:
  lldb/trunk/include/lldb/Target/Process.h
  lldb/trunk/include/lldb/Target/Target.h
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/source/Target/Target.cpp

Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -4129,3 +4129,10 @@
 module_list = event_data->m_module_list;
   return module_list;
 }
+
+std::recursive_mutex ::GetAPIMutex() { 
+  if (GetProcessSP() && GetProcessSP()->CurrentThreadIsPrivateStateThread())
+return m_private_mutex;
+  else
+return m_mutex;
+}
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -5538,6 +5538,12 @@
 return m_public_run_lock;
 }
 
+bool Process::CurrentThreadIsPrivateStateThread()
+{
+  return m_private_state_thread.EqualsThread(Host::GetCurrentThread());
+}
+
+
 void Process::Flush() {
   m_thread_list.Flush();
   m_extended_thread_list.Flush();
Index: lldb/trunk/include/lldb/Target/Target.h
===
--- lldb/trunk/include/lldb/Target/Target.h
+++ lldb/trunk/include/lldb/Target/Target.h
@@ -535,7 +535,7 @@
 
   static const lldb::TargetPropertiesSP ();
 
-  std::recursive_mutex () { return m_mutex; }
+  std::recursive_mutex ();
 
   void DeleteCurrentProcess();
 
@@ -1288,6 +1288,12 @@
   lldb::PlatformSP m_platform_sp; ///< The platform for this target.
   std::recursive_mutex m_mutex; ///< An API mutex that is used by the lldb::SB*
 /// classes make the SB interface thread safe
+  /// When the private state thread calls SB API's - usually because it is 
+  /// running OS plugin or Python ThreadPlan code - it should not block on the
+  /// API mutex that is held by the code that kicked off the sequence of events
+  /// that led us to run the code.  We hand out this mutex instead when we 
+  /// detect that code is running on the private state thread.
+  std::recursive_mutex m_private_mutex; 
   Arch m_arch;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).
Index: lldb/trunk/include/lldb/Target/Process.h
===
--- lldb/trunk/include/lldb/Target/Process.h
+++ lldb/trunk/include/lldb/Target/Process.h
@@ -2272,6 +2272,8 @@
   void ClearPreResumeAction(PreResumeActionCallback callback, void *baton);
 
   ProcessRunLock ();
+  
+  bool CurrentThreadIsPrivateStateThread();
 
   virtual Status SendEventData(const char *data) {
 Status return_error("Sending an event is not supported for this process.");
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
@@ -1,7 +1,10 @@
 #include 
 
 void foo() {
-  printf("Set a breakpoint here.\n");
+  int foo = 10; 
+  printf("%d\n", foo); // Set a breakpoint here. 
+  foo = 20;
+  printf("%d\n", foo);
 }
 
 int main() {
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
@@ -35,3 +35,38 @@
 
 def queue_child_thread_plan(self):
 return self.thread_plan.QueueThreadPlanForStepScripted("Steps.StepOut")
+
+# This plan does a step-over until a variable changes value.
+class StepUntil(StepWithChild):
+def __init__(self, thread_plan, dict):
+self.frame = thread_plan.GetThread().frames[0]
+self.target = thread_plan.GetThread().GetProcess().GetTarget()
+self.value = self.frame.FindVariable("foo")
+

[Lldb-commits] [lldb] r373280 - Allow the internal-state-thread free access to the TargetAPI mutex.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Mon Sep 30 17:47:25 2019
New Revision: 373280

URL: http://llvm.org/viewvc/llvm-project?rev=373280=rev
Log:
Allow the internal-state-thread free access to the TargetAPI mutex.

It is always doing work on behalf of another thread that presumably
has the mutex, so if it is calling SB API's it should have free access
to the mutex.  This is the same decision as we made earlier with the
process RunLock.

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

Modified:
lldb/trunk/include/lldb/Target/Process.h
lldb/trunk/include/lldb/Target/Target.h

lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
lldb/trunk/source/Target/Process.cpp
lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=373280=373279=373280=diff
==
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Sep 30 17:47:25 2019
@@ -2272,6 +2272,8 @@ public:
   void ClearPreResumeAction(PreResumeActionCallback callback, void *baton);
 
   ProcessRunLock ();
+  
+  bool CurrentThreadIsPrivateStateThread();
 
   virtual Status SendEventData(const char *data) {
 Status return_error("Sending an event is not supported for this process.");

Modified: lldb/trunk/include/lldb/Target/Target.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=373280=373279=373280=diff
==
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Mon Sep 30 17:47:25 2019
@@ -535,7 +535,7 @@ public:
 
   static const lldb::TargetPropertiesSP ();
 
-  std::recursive_mutex () { return m_mutex; }
+  std::recursive_mutex ();
 
   void DeleteCurrentProcess();
 
@@ -1288,6 +1288,12 @@ protected:
   lldb::PlatformSP m_platform_sp; ///< The platform for this target.
   std::recursive_mutex m_mutex; ///< An API mutex that is used by the lldb::SB*
 /// classes make the SB interface thread safe
+  /// When the private state thread calls SB API's - usually because it is 
+  /// running OS plugin or Python ThreadPlan code - it should not block on the
+  /// API mutex that is held by the code that kicked off the sequence of events
+  /// that led us to run the code.  We hand out this mutex instead when we 
+  /// detect that code is running on the private state thread.
+  std::recursive_mutex m_private_mutex; 
   Arch m_arch;
   ModuleList m_images; ///< The list of images for this process (shared
/// libraries and anything dynamically loaded).

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py?rev=373280=373279=373280=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
 Mon Sep 30 17:47:25 2019
@@ -35,3 +35,38 @@ class StepScripted(StepWithChild):
 
 def queue_child_thread_plan(self):
 return self.thread_plan.QueueThreadPlanForStepScripted("Steps.StepOut")
+
+# This plan does a step-over until a variable changes value.
+class StepUntil(StepWithChild):
+def __init__(self, thread_plan, dict):
+self.frame = thread_plan.GetThread().frames[0]
+self.target = thread_plan.GetThread().GetProcess().GetTarget()
+self.value = self.frame.FindVariable("foo")
+StepWithChild.__init__(self, thread_plan)
+
+def queue_child_thread_plan(self):
+le = self.frame.GetLineEntry()
+start_addr = le.GetStartAddress() 
+start = start_addr.GetLoadAddress(self.target)
+end = le.GetEndAddress().GetLoadAddress(self.target)
+print("Stepping from 0x%x to 0x%x (0x%x)"%(start, end, end - start))
+return self.thread_plan.QueueThreadPlanForStepOverRange(start_addr,
+end - start)
+
+def should_stop(self, event):
+if not self.child_thread_plan.IsPlanComplete():
+return False
+
+# If we've stepped out of this frame, stop.
+if not self.frame.IsValid():
+return True
+
+if not self.value.IsValid():
+return True
+
+print("Got next value: %d"%(self.value.GetValueAsUnsigned()))
+if not self.value.GetValueDidChange():

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

[Vedant Kumar via Phabricator via lldb-commits]

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

Sweet! Does this 'automatically' fix the 'llvm-argdumper has issues escaping 
JSON-ified input' issue we discussed in person?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68248



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

[Lldb-commits] [lldb] r373277 - [Docs] Document lldb-instr

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Sep 30 17:12:47 2019
New Revision: 373277

URL: http://llvm.org/viewvc/llvm-project?rev=373277=rev
Log:
[Docs] Document lldb-instr

This adds some information on how to instrument the API classes.

Modified:
lldb/trunk/docs/resources/sbapi.rst

Modified: lldb/trunk/docs/resources/sbapi.rst
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/resources/sbapi.rst?rev=373277=373276=373277=diff
==
--- lldb/trunk/docs/resources/sbapi.rst (original)
+++ lldb/trunk/docs/resources/sbapi.rst Mon Sep 30 17:12:47 2019
@@ -53,3 +53,43 @@ file, and adding documentation and the P
 decidedly low-tech way, by maintaining the two files in parallel. That
 simplifies the build process, but it does mean that if you add a method to the
 C++ API's for an SB class, you have to copy the interface to the .i file.
+
+API Instrumentation
+---
+
+The reproducer infrastructure requires API methods to be instrumented so that
+they can be captured and replayed. Instrumentation consists of two macros,
+``LLDB_REGISTER`` and ``LLDB_RECORD``. Both can be automatically generated with
+the ``lldb-instr`` utility.
+
+To add instrumentation for a given file, pass it to the ``lldb-instr`` tool.
+Like other clang-based tools it requires a compilation database
+(``compile_commands.json``) to be present in the current working directory.
+
+::
+
+./bin/lldb-instr /path/to/lldb/source/API/SBDebugger.cpp
+
+
+The tool will automatically insert ``LLDB_RECORD`` macros inline, however you
+will need to run ``clang-format`` over the processed file, as the tool
+(intentionally) makes no attempt to get that right.
+
+The ``LLDB_REGISTER`` macros are printed to standard out between curly braces.
+You'll have to copy-paste those into the corresponding `RegsiterMethods`
+function in the implementation file. This function is fully specialized in the
+corresponding type.
+
+::
+
+  template <> void RegisterMethods(Registry ) {
+...
+  }
+
+
+When adding a new class, you'll also have to add a call to ``RegisterMethods``
+in the ``SBRegistry`` constructor.
+
+The tool can be used incrementally. However, it will ignore existing macros
+even if their signature is wrong. It will only generate a ``LLDB_REGISTER`` if
+it emitted a corresponding ``LLDB_RECORD`` macro.


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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 222522.
lawrence_danna added a comment.

flush fix for python2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793

Files:
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBError.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Host/File.h
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/scripts/lldb.swig
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -466,8 +466,6 @@
   void Reset(PyRefType type, PyObject *py_obj) override;
   void Reset(File , const char *mode);
 
-  static uint32_t GetOptionsFromMode(llvm::StringRef mode);
-
   lldb::FileUP GetUnderlyingFile() const;
 };
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -949,7 +949,6 @@
 
 PythonFile::PythonFile(File , const char *mode) { Reset(file, mode); }
 
-
 PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
 
 PythonFile::~PythonFile() {}
@@ -1014,22 +1013,6 @@
 #endif
 }
 
-uint32_t PythonFile::GetOptionsFromMode(llvm::StringRef mode) {
-  if (mode.empty())
-return 0;
-
-  return llvm::StringSwitch(mode.str())
-  .Case("r", File::eOpenOptionRead)
-  .Case("w", File::eOpenOptionWrite)
-  .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend |
- File::eOpenOptionCanCreate)
-  .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
-  .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
-  File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
-  .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
-  File::eOpenOptionAppend | File::eOpenOptionCanCreate)
-  .Default(0);
-}
 
 FileUP PythonFile::GetUnderlyingFile() const {
   if (!IsValid())
@@ -1038,7 +1021,7 @@
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType();
-  auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
+  auto options = File::GetOptionsFromMode(py_mode.GetString());
   auto file = std::make_unique(PyObject_AsFileDescriptor(m_py_obj),
  options, false);
   if (!file->IsValid())
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -68,6 +68,20 @@
   return nullptr;
 }
 
+uint32_t File::GetOptionsFromMode(llvm::StringRef mode) {
+  return llvm::StringSwitch(mode)
+  .Case("r", File::eOpenOptionRead)
+  .Case("w", File::eOpenOptionWrite)
+  .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend |
+ File::eOpenOptionCanCreate)
+  .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
+  .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
+  File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
+  .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
+  File::eOpenOptionAppend | File::eOpenOptionCanCreate)
+  .Default(0);
+}
+
 int File::kInvalidDescriptor = -1;
 FILE *File::kInvalidStream = nullptr;
 
@@ -143,9 +157,14 @@
 
 Status File::Close() {
   Status error;
-  if (StreamIsValid() && m_own_stream) {
-if (::fclose(m_stream) == EOF)
-  error.SetErrorToErrno();
+  if (StreamIsValid()) {
+if (m_own_stream) {
+  if (::fclose(m_stream) == EOF)
+error.SetErrorToErrno();
+} else {
+  if (::fflush(m_stream) == EOF)
+error.SetErrorToErrno();
+}
   }
 
   if (DescriptorIsValid() && m_own_descriptor) {
Index: lldb/source/API/SBFile.cpp
===
--- /dev/null
+++ lldb/source/API/SBFile.cpp
@@ -0,0 +1,76 @@
+//===-- SBFile.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 

[Lldb-commits] [PATCH] D68188: allow arbitrary python streams to be converted to SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna marked 2 inline comments as done.
lawrence_danna added inline comments.



Comment at: lldb/include/lldb/API/SBFile.h:16-21
+/* These tags make no difference at the c++ level, but
+ * when the constructors are called from python they control
+ * how python files are converted by SWIG into FileSP */
+struct FileBorrow {};
+struct FileForceScriptingIO {};
+struct FileBorrowAndForceScriptingIO {};

labath wrote:
> I don't think we have anything similar to this in any of our SB classes. It 
> might be better to avoid it. Could the same thing be achieved e.g. with a 
> static factory function (`static SBFile SBFile::Create(FileSP file_sp, bool 
> borrow, bool force_scripting_io)`) ?
I don't think it can be done that way, because we want those bools to control 
how the python object is translated into a C++ object, so we need a different 
swig wrapper for each possibility, which means we need a different c++ function 
to wrap  for each possibility.

One way around this would be to have SWIG translate the python object into an 
intermediate object that just takes a reference, and then perform further 
translation inside the SB layer.But we can't do that either, because 
scripting support is not part of the base LLDB library, it's a plugin.And 
in fact Xcode ships two versions of the plugin, one for python2 and one for 
python3.The SB needs to be agnostic about different versions of python, so 
it can't do the translation itself.   


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188



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

[Lldb-commits] [PATCH] D68181: SBDebugger::SetInputFile, SetOutputFile, etc.

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 222520.
lawrence_danna added a comment.

propagated updates from parent


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68181

Files:
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Core/Debugger.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/interface/SBDebugger.i
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp

Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -821,32 +821,42 @@
 
 repro::DataRecorder *Debugger::GetInputRecorder() { return m_input_recorder; }
 
-void Debugger::SetInputFileHandle(FILE *fh, bool tranfer_ownership,
-  repro::DataRecorder *recorder) {
+Status Debugger::SetInputFile(FileSP file_sp, repro::DataRecorder *recorder) {
+  Status error;
   m_input_recorder = recorder;
 
-  m_input_file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!m_input_file_sp->IsValid())
+  if (!file_sp || !file_sp->IsValid()) {
 m_input_file_sp = std::make_shared(stdin, false);
+error.SetErrorString("invalid file");
+  } else {
+m_input_file_sp = file_sp;
+  }
 
   // Save away the terminal state if that is relevant, so that we can restore
   // it in RestoreInputState.
   SaveInputTerminalState();
+
+  return error;
 }
 
-void Debugger::SetOutputFileHandle(FILE *fh, bool tranfer_ownership) {
-  FileSP file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!file_sp->IsValid())
+Status Debugger::SetOutputFile(FileSP file_sp) {
+  Status error;
+  if (!file_sp || !file_sp->IsValid()) {
 file_sp = std::make_shared(stdout, false);
+error.SetErrorString("invalid file");
+  }
   m_output_stream_sp = std::make_shared(file_sp);
-
+  return error;
 }
 
-void Debugger::SetErrorFileHandle(FILE *fh, bool tranfer_ownership) {
-  FileSP file_sp = std::make_shared(fh, tranfer_ownership);
-  if (!file_sp->IsValid())
+Status Debugger::SetErrorFile(FileSP file_sp) {
+  Status error;
+  if (!file_sp || !file_sp->IsValid()) {
 file_sp = std::make_shared(stderr, false);
+error.SetErrorString("invalid file");
+  }
   m_error_stream_sp = std::make_shared(file_sp);
+  return error;
 }
 
 void Debugger::SaveInputTerminalState() {
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -18,6 +18,7 @@
 #include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBEvent.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBListener.h"
 #include "lldb/API/SBProcess.h"
@@ -304,30 +305,92 @@
   if (loader) {
 llvm::Optional file = loader->GetNextFile();
 fh = file ? FileSystem::Instance().Fopen(file->c_str(), "r") : nullptr;
+transfer_ownership = true;
   }
 
-  m_opaque_sp->SetInputFileHandle(fh, transfer_ownership, recorder);
+  m_opaque_sp->SetInputFile(std::make_shared(fh, transfer_ownership),
+recorder);
+}
+
+SBError SBDebugger::SetInputFile(SBFile file) {
+  LLDB_RECORD_METHOD(SBError, SBDebugger, SetInputFile, (SBFile), file);
+
+  SBError error;
+  if (!m_opaque_sp) {
+error.ref().SetErrorString("invalid debugger");
+return error;
+  }
+
+  repro::DataRecorder *recorder = nullptr;
+  if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
+recorder = g->GetOrCreate().GetNewDataRecorder();
+
+  FileSP file_sp = file.m_opaque_sp;
+
+  static std::unique_ptr loader =
+  repro::CommandLoader::Create(repro::Reproducer::Instance().GetLoader());
+  if (loader) {
+llvm::Optional nextfile = loader->GetNextFile();
+FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
+: nullptr;
+if (fh) {
+  file_sp = std::make_shared(fh, true);
+}
+  }
+
+  error.SetError(m_opaque_sp->SetInputFile(file_sp, recorder));
+  return error;
 }
 
 void SBDebugger::SetOutputFileHandle(FILE *fh, bool transfer_ownership) {
   LLDB_RECORD_METHOD(void, SBDebugger, SetOutputFileHandle, (FILE *, bool), fh,
  transfer_ownership);
 
-  if (m_opaque_sp)
-m_opaque_sp->SetOutputFileHandle(fh, transfer_ownership);
+  if (m_opaque_sp) {
+m_opaque_sp->SetOutputFile(std::make_shared(fh, transfer_ownership));
+  }
+}
+
+SBError SBDebugger::SetOutputFile(SBFile file) {
+  LLDB_RECORD_METHOD(SBError, SBDebugger, SetOutputFile, (SBFile file), file);
+  SBError error;
+  if (!m_opaque_sp) {
+error.ref().SetErrorString("invalid debugger");
+return error;
+  }
+  if (!file) {
+error.ref().SetErrorString("invalid file");
+return error;
+  }
+  

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

[Jonas Devlieghere via Phabricator via lldb-commits]

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



Comment at: 
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp:387
   std::string expected_packet1 =
-  R"(jTraceStart:{"buffersize" : 8192,"metabuffersize" : 8192,"params" :)";
+  R"(jTraceStart:{"buffersize":8192,"metabuffersize":8192,"params":)";
   std::string expected_packet2 =

shafik wrote:
> Removing the spaces, is this just a formatting change?
Yes


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68248



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

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: 
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp:387
   std::string expected_packet1 =
-  R"(jTraceStart:{"buffersize" : 8192,"metabuffersize" : 8192,"params" :)";
+  R"(jTraceStart:{"buffersize":8192,"metabuffersize":8192,"params":)";
   std::string expected_packet2 =

Removing the spaces, is this just a formatting change?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68248



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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793



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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 222516.
lawrence_danna marked 7 inline comments as done.
lawrence_danna added a comment.

fixed according to reviewer comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793

Files:
  lldb/include/lldb/API/LLDB.h
  lldb/include/lldb/API/SBDefines.h
  lldb/include/lldb/API/SBError.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Host/File.h
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/scripts/lldb.swig
  lldb/source/API/CMakeLists.txt
  lldb/source/API/SBFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -466,8 +466,6 @@
   void Reset(PyRefType type, PyObject *py_obj) override;
   void Reset(File , const char *mode);
 
-  static uint32_t GetOptionsFromMode(llvm::StringRef mode);
-
   lldb::FileUP GetUnderlyingFile() const;
 };
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -949,7 +949,6 @@
 
 PythonFile::PythonFile(File , const char *mode) { Reset(file, mode); }
 
-
 PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); }
 
 PythonFile::~PythonFile() {}
@@ -1014,22 +1013,6 @@
 #endif
 }
 
-uint32_t PythonFile::GetOptionsFromMode(llvm::StringRef mode) {
-  if (mode.empty())
-return 0;
-
-  return llvm::StringSwitch(mode.str())
-  .Case("r", File::eOpenOptionRead)
-  .Case("w", File::eOpenOptionWrite)
-  .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend |
- File::eOpenOptionCanCreate)
-  .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
-  .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
-  File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
-  .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
-  File::eOpenOptionAppend | File::eOpenOptionCanCreate)
-  .Default(0);
-}
 
 FileUP PythonFile::GetUnderlyingFile() const {
   if (!IsValid())
@@ -1038,7 +1021,7 @@
   // We don't own the file descriptor returned by this function, make sure the
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType();
-  auto options = PythonFile::GetOptionsFromMode(py_mode.GetString());
+  auto options = File::GetOptionsFromMode(py_mode.GetString());
   auto file = std::make_unique(PyObject_AsFileDescriptor(m_py_obj),
  options, false);
   if (!file->IsValid())
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -68,6 +68,20 @@
   return nullptr;
 }
 
+uint32_t File::GetOptionsFromMode(llvm::StringRef mode) {
+  return llvm::StringSwitch(mode)
+  .Case("r", File::eOpenOptionRead)
+  .Case("w", File::eOpenOptionWrite)
+  .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend |
+ File::eOpenOptionCanCreate)
+  .Case("r+", File::eOpenOptionRead | File::eOpenOptionWrite)
+  .Case("w+", File::eOpenOptionRead | File::eOpenOptionWrite |
+  File::eOpenOptionCanCreate | File::eOpenOptionTruncate)
+  .Case("a+", File::eOpenOptionRead | File::eOpenOptionWrite |
+  File::eOpenOptionAppend | File::eOpenOptionCanCreate)
+  .Default(0);
+}
+
 int File::kInvalidDescriptor = -1;
 FILE *File::kInvalidStream = nullptr;
 
@@ -143,9 +157,14 @@
 
 Status File::Close() {
   Status error;
-  if (StreamIsValid() && m_own_stream) {
-if (::fclose(m_stream) == EOF)
-  error.SetErrorToErrno();
+  if (StreamIsValid()) {
+if (m_own_stream) {
+  if (::fclose(m_stream) == EOF)
+error.SetErrorToErrno();
+} else {
+  if (::fflush(m_stream) == EOF)
+error.SetErrorToErrno();
+}
   }
 
   if (DescriptorIsValid() && m_own_descriptor) {
Index: lldb/source/API/SBFile.cpp
===
--- /dev/null
+++ lldb/source/API/SBFile.cpp
@@ -0,0 +1,76 @@
+//===-- SBFile.cpp --*- C++ -*-===//
+//

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, mgorny, xiaobai, aprantl, clayborg.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

This patch replaces the hand-rolled JSON emission with LLVM's JSON library.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68248

Files:
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Utility/StructuredData.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -384,9 +384,9 @@
 
   // Since the line is exceeding 80 characters.
   std::string expected_packet1 =
-  R"(jTraceStart:{"buffersize" : 8192,"metabuffersize" : 8192,"params" :)";
+  R"(jTraceStart:{"buffersize":8192,"metabuffersize":8192,"params":)";
   std::string expected_packet2 =
-  R"( {"psb" : 1,"tracetech" : "intel-pt"},"threadid" : 35,"type" : 1})";
+  R"({"psb":1,"tracetech":"intel-pt"},"threadid":35,"type":1})";
   HandlePacket(server, (expected_packet1 + expected_packet2), "1");
   ASSERT_TRUE(error.Success());
   ASSERT_EQ(result.get(), 1u);
@@ -409,8 +409,7 @@
 return client.SendStopTracePacket(trace_id, thread_id);
   });
 
-  const char *expected_packet =
-  R"(jTraceStop:{"threadid" : 35,"traceid" : 3})";
+  const char *expected_packet = R"(jTraceStop:{"threadid":35,"traceid":3})";
   HandlePacket(server, expected_packet, "OK");
   ASSERT_TRUE(result.get().Success());
 
@@ -435,8 +434,8 @@
   });
 
   std::string expected_packet1 =
-  R"(jTraceBufferRead:{"buffersize" : 32,"offset" : 0,"threadid" : 35,)";
-  std::string expected_packet2 = R"("traceid" : 3})";
+  R"(jTraceBufferRead:{"buffersize":32,"offset":0,"threadid":35,)";
+  std::string expected_packet2 = R"("traceid":3})";
   HandlePacket(server, expected_packet1+expected_packet2, "123456");
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(buffer.size(), 3u);
@@ -467,8 +466,8 @@
   });
 
   std::string expected_packet1 =
-  R"(jTraceMetaRead:{"buffersize" : 32,"offset" : 0,"threadid" : 35,)";
-  std::string expected_packet2 = R"("traceid" : 3})";
+  R"(jTraceMetaRead:{"buffersize":32,"offset":0,"threadid":35,)";
+  std::string expected_packet2 = R"("traceid":3})";
   HandlePacket(server, expected_packet1+expected_packet2, "123456");
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(buffer.size(), 3u);
@@ -497,11 +496,10 @@
   });
 
   const char *expected_packet =
-  R"(jTraceConfigRead:{"threadid" : 35,"traceid" : 3})";
+  R"(jTraceConfigRead:{"threadid":35,"traceid":3})";
   std::string response1 =
-  R"({"buffersize" : 8192,"params" : {"psb" : 1,"tracetech" : "intel-pt"})";
-  std::string response2 =
-  R"(],"metabuffersize" : 8192,"threadid" : 35,"type" : 1}])";
+  R"({"buffersize":8192,"params":{"psb":1,"tracetech":"intel-pt"})";
+  std::string response2 = R"(],"metabuffersize":8192,"threadid":35,"type":1}])";
   HandlePacket(server, expected_packet, response1+response2);
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(options.getTraceBufferSize(), 8192u);
Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -11,7 +11,6 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Status.h"
-#include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -21,6 +20,7 @@
 #include 
 
 using namespace lldb_private;
+using namespace llvm;
 
 // Functions that use a JSONParser to parse JSON into StructuredData
 static StructuredData::ObjectSP ParseJSONValue(JSONParser _parser);
@@ -181,98 +181,48 @@
 }
 
 void StructuredData::Object::DumpToStdout(bool pretty_print) const {
-  StreamString stream;
-  Dump(stream, pretty_print);
-  llvm::outs() << stream.GetString();
+  json::OStream stream(llvm::outs(), pretty_print);
+  Serialize(stream);
 }
 
-void StructuredData::Array::Dump(Stream , bool pretty_print) const {
-  bool first = true;
-  s << "[";
-  if (pretty_print) {
-s << "\n";
-s.IndentMore();
-  }
+void StructuredData::Array::Serialize(json::OStream ) const {
+  s.arrayBegin();
   for (const auto _sp : m_items) {
-if (first) {
-  first = false;
-} else {
-  s << ",";
-  if (pretty_print)
-s << "\n";
-}
-
-if (pretty_print)
-  s.Indent();
-

[Lldb-commits] [lldb] r373267 - [StackFrameList][DFS] Turn a few raw pointers into references, NFC

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Mon Sep 30 14:20:14 2019
New Revision: 373267

URL: http://llvm.org/viewvc/llvm-project?rev=373267=rev
Log:
[StackFrameList][DFS] Turn a few raw pointers into references, NFC

Modified:
lldb/trunk/source/Symbol/Function.cpp
lldb/trunk/source/Target/StackFrameList.cpp

Modified: lldb/trunk/source/Symbol/Function.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Function.cpp?rev=373267=373266=373267=diff
==
--- lldb/trunk/source/Symbol/Function.cpp (original)
+++ lldb/trunk/source/Symbol/Function.cpp Mon Sep 30 14:20:14 2019
@@ -173,6 +173,7 @@ void CallEdge::ParseSymbolFileAndResolve
 
 Function *CallEdge::GetCallee(ModuleList ) {
   ParseSymbolFileAndResolve(images);
+  assert(resolved && "Did not resolve lazy callee");
   return lazy_callee.def;
 }
 

Modified: lldb/trunk/source/Target/StackFrameList.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrameList.cpp?rev=373267=373266=373267=diff
==
--- lldb/trunk/source/Target/StackFrameList.cpp (original)
+++ lldb/trunk/source/Target/StackFrameList.cpp Mon Sep 30 14:20:14 2019
@@ -286,15 +286,15 @@ static void FindInterveningFrames(Functi
 
 DFS(Function *end, ModuleList ) : end(end), images(images) {}
 
-void search(Function *first_callee, std::vector ) {
+void search(Function _callee, std::vector ) {
   dfs(first_callee);
   if (!ambiguous)
 path = std::move(solution_path);
 }
 
-void dfs(Function *callee) {
+void dfs(Function ) {
   // Found a path to the target function.
-  if (callee == end) {
+  if ( == end) {
 if (solution_path.empty())
   solution_path = active_path;
 else
@@ -306,19 +306,19 @@ static void FindInterveningFrames(Functi
   // there's more than one way to reach a target. This errs on the side of
   // caution: it conservatively stops searching when some solutions are
   // still possible to save time in the average case.
-  if (!visited_nodes.insert(callee).second) {
+  if (!visited_nodes.insert().second) {
 ambiguous = true;
 return;
   }
 
   // Search the calls made from this callee.
-  active_path.push_back(callee);
-  for (CallEdge  : callee->GetTailCallingEdges()) {
+  active_path.push_back();
+  for (CallEdge  : callee.GetTailCallingEdges()) {
 Function *next_callee = edge.GetCallee(images);
 if (!next_callee)
   continue;
 
-dfs(next_callee);
+dfs(*next_callee);
 if (ambiguous)
   return;
   }
@@ -326,7 +326,7 @@ static void FindInterveningFrames(Functi
 }
   };
 
-  DFS(, images).search(first_callee, path);
+  DFS(, images).search(*first_callee, path);
 }
 
 /// Given that \p next_frame will be appended to the frame list, synthesize


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

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 222490.
aadsm added a comment.

Add lit test to check dissassembly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68069

Files:
  lldb/lit/SymbolFile/dissassemble-entry-point.s
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp

Index: lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -172,3 +172,131 @@
   Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9", 20);
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
+
+TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
+  /*
+  // nosym-entrypoint-arm-thumb.s
+  .global _Start
+  .thumb_func
+  _start:
+  mov r0, #42
+  mov r7, #1
+  svc #0
+  // arm-linux-androideabi-as nosym-entrypoint-arm-thumb.s
+  //   -o nosym-entrypoint-arm-thumb.o
+  // arm-linux-androideabi-ld nosym-entrypoint-arm-thumb.o
+  //   -o nosym-entrypoint-arm-thumb -e 0x8075 -s
+  */
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+  Entry:   0x8075
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x8074
+AddressAlign:0x0002
+Content: 2A20012700DF
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x9000
+AddressAlign:0x0001
+Content: ''
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x9000
+AddressAlign:0x0001
+  - Name:.note.gnu.gold-version
+Type:SHT_NOTE
+AddressAlign:0x0004
+Content: 040009000400474E5500676F6C6420312E313100
+  - Name:.ARM.attributes
+Type:SHT_ARM_ATTRIBUTES
+AddressAlign:0x0001
+Content: '41130061656162690001090006020901'
+...
+)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  ModuleSpec spec{FileSpec(ExpectedFile->name())};
+  spec.GetSymbolFileSpec().SetFile(ExpectedFile->name(),
+   FileSpec::Style::native);
+  auto module_sp = std::make_shared(spec);
+
+  auto entry_point_addr = module_sp->GetObjectFile()->GetEntryPointAddress();
+  ASSERT_TRUE(entry_point_addr.GetOffset() & 1);
+  // Decrease the offsite by 1 to make it into a breakable address since this
+  // is Thumb.
+  entry_point_addr.SetOffset(entry_point_addr.GetOffset() - 1);
+  ASSERT_EQ(entry_point_addr.GetAddressClass(),
+AddressClass::eCodeAlternateISA);
+}
+
+TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmAddressClass) {
+  /*
+  // nosym-entrypoint-arm.s
+  .global _Start
+  _start:
+  movs r0, #42
+  movs r7, #1
+  svc #0
+  // arm-linux-androideabi-as nosym-entrypoint-arm.s
+  //   -o nosym-entrypoint-arm.o
+  // arm-linux-androideabi-ld nosym-entrypoint-arm.o
+  //   -o nosym-entrypoint-arm -e 0x8074 -s
+  */
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:ELFDATA2LSB
+  Type:ET_EXEC
+  Machine: EM_ARM
+  Flags:   [ EF_ARM_SOFT_FLOAT, EF_ARM_EABI_VER5 ]
+  Entry:   0x8074
+Sections:
+  - Name:.text
+Type:SHT_PROGBITS
+Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
+Address: 0x8074
+AddressAlign:0x0004
+Content: 2A00A0E30170A0E300EF
+  - Name:.data
+Type:SHT_PROGBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x9000
+AddressAlign:0x0001
+Content: ''
+  - Name:.bss
+Type:SHT_NOBITS
+Flags:   [ SHF_WRITE, SHF_ALLOC ]
+Address: 0x9000
+AddressAlign:0x0001
+  - Name:.note.gnu.gold-version
+Type:SHT_NOTE
+AddressAlign:0x0004
+Content: 040009000400474E5500676F6C6420312E313100
+  - Name:.ARM.attributes
+Type:SHT_ARM_ATTRIBUTES
+AddressAlign:0x0001
+Content: '41130061656162690001090006010801'
+...
+)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 222479.
kwk added a comment.

- Fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/merge-symbols.yaml
  lldb/lit/Modules/lit.local.cfg
  lldb/lit/helper/toolchain.py
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -12,6 +12,7 @@
 #include 
 
 #include 
+#include 
 
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
@@ -55,7 +56,7 @@
 /// This class provides a generic ELF (32/64 bit) reader plugin implementing
 /// the ObjectFile protocol.
 class ObjectFileELF : public lldb_private::ObjectFile {
-public:
+public:  
   // Static Functions
   static void Initialize();
 
@@ -184,6 +185,8 @@
 
   typedef std::map
   FileAddressToAddressClassMap;
+  
+  typedef std::unordered_set UniqueElfSymbolColl;
 
   /// Version of this reader common to all plugins based on this class.
   static const uint32_t m_plugin_version = 1;
@@ -278,7 +281,8 @@
   /// will parse the symbols only once.  Returns the number of symbols parsed.
   unsigned ParseSymbolTable(lldb_private::Symtab *symbol_table,
 lldb::user_id_t start_id,
-lldb_private::Section *symtab);
+lldb_private::Section *symtab,
+UniqueElfSymbolColl _elf_symbols);
 
   /// Helper routine for ParseSymbolTable().
   unsigned ParseSymbols(lldb_private::Symtab *symbol_table,
@@ -286,7 +290,8 @@
 lldb_private::SectionList *section_list,
 const size_t num_symbols,
 const lldb_private::DataExtractor _data,
-const lldb_private::DataExtractor _data);
+const lldb_private::DataExtractor _data,
+UniqueElfSymbolColl _elf_symbols);
 
   /// Scans the relocation entries and adds a set of artificial symbols to the
   /// given symbol table for each PLT slot.  Returns the number of symbols
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1871,7 +1871,8 @@
  SectionList *section_list,
  const size_t num_symbols,
  const DataExtractor _data,
- const DataExtractor _data) {
+ const DataExtractor _data,
+ UniqueElfSymbolColl _elf_symbols) {
   ELFSymbol symbol;
   lldb::offset_t offset = 0;
 
@@ -2196,20 +2197,28 @@
 symbol_size_valid,  // Symbol size is valid
 has_suffix, // Contains linker annotations?
 flags); // Symbol flags.
-symtab->AddSymbol(dc_symbol);
+
+NamedELFSymbol needle(symbol, ConstString(symbol_ref),
+  symbol_section_sp.get() ? symbol_section_sp->GetName()
+  : ConstString());
+if (unique_elf_symbols.insert(needle).second) {
+  symtab->AddSymbol(dc_symbol);
+}
   }
   return i;
 }
 
-unsigned ObjectFileELF::ParseSymbolTable(Symtab *symbol_table,
- user_id_t start_id,
- lldb_private::Section *symtab) {
+unsigned
+ObjectFileELF::ParseSymbolTable(Symtab *symbol_table, user_id_t start_id,
+lldb_private::Section *symtab,
+UniqueElfSymbolColl _elf_symbols) {
   if (symtab->GetObjectFile() != this) {
 // If the symbol table section is owned by a different object file, have it
 // do the parsing.
 ObjectFileELF *obj_file_elf =
 static_cast(symtab->GetObjectFile());
-return obj_file_elf->ParseSymbolTable(symbol_table, start_id, symtab);
+return obj_file_elf->ParseSymbolTable(symbol_table, start_id, symtab,
+  unique_elf_symbols);
   }
 
   // Get section list for this object file.
@@ -2237,7 +2246,7 @@
   size_t num_symbols = symtab_data.GetByteSize() / symtab_hdr->sh_entsize;
 
   return ParseSymbols(symbol_table, start_id, section_list, num_symbols,
-   

[Lldb-commits] [lldb] r373250 - Try to update Windows unit test for API change.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Sep 30 12:38:52 2019
New Revision: 373250

URL: http://llvm.org/viewvc/llvm-project?rev=373250=rev
Log:
Try to update Windows unit test for API change.

Modified:
lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Modified: lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp?rev=373250=373249=373250=diff
==
--- lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp Mon Sep 30 
12:38:52 2019
@@ -355,8 +355,8 @@ TEST_F(SymbolFilePDBTests, TestSimpleCla
   llvm::pdb::IPDBSession  = symfile->GetPDBSession();
   llvm::DenseSet searched_files;
   TypeMap results;
-  EXPECT_EQ(1u, symfile->FindTypes(ConstString("Class"), nullptr, false, 0,
-   searched_files, results));
+  symfile->FindTypes(ConstString("Class"), nullptr, false, 0, searched_files,
+ results);
   EXPECT_EQ(1u, results.GetSize());
   lldb::TypeSP udt_type = results.GetTypeAtIndex(0);
   EXPECT_EQ(ConstString("Class"), udt_type->GetName());
@@ -385,8 +385,8 @@ TEST_F(SymbolFilePDBTests, TestNestedCla
   llvm::dyn_cast_or_null(_ast_ctx_or_err.get());
   EXPECT_NE(nullptr, clang_ast_ctx);
 
-  EXPECT_EQ(1u, symfile->FindTypes(ConstString("Class"), nullptr, false, 0,
-   searched_files, results));
+  symfile->FindTypes(ConstString("Class"), nullptr, false, 0, searched_files,
+ results);
   EXPECT_EQ(1u, results.GetSize());
 
   auto Class = results.GetTypeAtIndex(0);
@@ -404,9 +404,8 @@ TEST_F(SymbolFilePDBTests, TestNestedCla
   // compiler type for both, but `FindTypes` may return more than one type
   // (with the same compiler type) because the symbols have different IDs.
   auto ClassCompilerDeclCtx = CompilerDeclContext(clang_ast_ctx, ClassDeclCtx);
-  EXPECT_LE(1u, symfile->FindTypes(ConstString("NestedClass"),
-   , false, 0,
-   searched_files, results));
+  symfile->FindTypes(ConstString("NestedClass"), , false,
+ 0, searched_files, results);
   EXPECT_LE(1u, results.GetSize());
 
   lldb::TypeSP udt_type = results.GetTypeAtIndex(0);
@@ -450,8 +449,8 @@ TEST_F(SymbolFilePDBTests, TestClassInNa
   auto ns_namespace = symfile->FindNamespace(ConstString("NS"), nullptr);
   EXPECT_TRUE(ns_namespace.IsValid());
 
-  EXPECT_EQ(1u, symfile->FindTypes(ConstString("NSClass"), _namespace, 
false,
-   0, searched_files, results));
+  symfile->FindTypes(ConstString("NSClass"), _namespace, false,
+ 0, searched_files, results);
   EXPECT_EQ(1u, results.GetSize());
 
   lldb::TypeSP udt_type = results.GetTypeAtIndex(0);
@@ -476,8 +475,8 @@ TEST_F(SymbolFilePDBTests, TestEnumTypes
   const char *EnumsToCheck[] = {"Enum", "ShortEnum"};
   for (auto Enum : EnumsToCheck) {
 TypeMap results;
-EXPECT_EQ(1u, symfile->FindTypes(ConstString(Enum), nullptr, false, 0,
- searched_files, results));
+symfile->FindTypes(ConstString(Enum), nullptr, false, 0,
+   searched_files, results);
 EXPECT_EQ(1u, results.GetSize());
 lldb::TypeSP enum_type = results.GetTypeAtIndex(0);
 EXPECT_EQ(ConstString(Enum), enum_type->GetName());
@@ -525,8 +524,8 @@ TEST_F(SymbolFilePDBTests, TestTypedefs)
"VariadicFuncPointerTypedef"};
   for (auto Typedef : TypedefsToCheck) {
 TypeMap results;
-EXPECT_EQ(1u, symfile->FindTypes(ConstString(Typedef), nullptr, false, 0,
- searched_files, results));
+symfile->FindTypes(ConstString(Typedef), nullptr, false, 0,
+   searched_files, results);
 EXPECT_EQ(1u, results.GetSize());
 lldb::TypeSP typedef_type = results.GetTypeAtIndex(0);
 EXPECT_EQ(ConstString(Typedef), typedef_type->GetName());
@@ -571,17 +570,17 @@ TEST_F(SymbolFilePDBTests, TestMaxMatche
   llvm::DenseSet searched_files;
   TypeMap results;
   const ConstString name("ClassTypedef");
-  uint32_t num_results =
-  symfile->FindTypes(name, nullptr, false, 0, searched_files, results);
+  symfile->FindTypes(name, nullptr, false, 0, searched_files, results);
   // Try to limit ourselves from 1 to 10 results, otherwise we could be doing
   // this thousands of times.
   // The idea is just to make sure that for a variety of values, the number of
   // limited results always
   // comes out to the number we are expecting.
-  uint32_t iterations = std::min(num_results, 10u);
+  uint32_t iterations = std::min(results.GetSize(), 10u);
+  uint32_t num_results = results.GetSize();
   for (uint32_t i = 1; i <= iterations; ++i) {
-uint32_t num_limited_results =
- 

[Lldb-commits] [PATCH] D68169: Remove size_t return parameter from FindTypes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 222476.

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

https://reviews.llvm.org/D68169

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/ModuleList.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/TypeList.h
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/source/Symbol/SymbolFile.cpp

Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -139,18 +139,14 @@
   return;
 }
 
-uint32_t SymbolFile::FindTypes(
+void SymbolFile::FindTypes(
 ConstString name, const CompilerDeclContext *parent_decl_ctx,
 uint32_t max_matches,
 llvm::DenseSet _symbol_files,
-TypeMap ) {
-  return 0;
-}
+TypeMap ) {}
 
-size_t SymbolFile::FindTypes(llvm::ArrayRef pattern,
- LanguageSet languages, TypeMap ) {
-  return 0;
-}
+void SymbolFile::FindTypes(llvm::ArrayRef pattern,
+   LanguageSet languages, TypeMap ) {}
 
 void SymbolFile::AssertModuleLock() {
   // The code below is too expensive to leave enabled in release builds. It's
Index: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
===
--- lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -71,9 +71,9 @@
 lldb::SymbolContextItem resolve_scope,
 lldb_private::SymbolContext ) override;
 
-  size_t GetTypes(lldb_private::SymbolContextScope *sc_scope,
-  lldb::TypeClass type_mask,
-  lldb_private::TypeList _list) override;
+  void GetTypes(lldb_private::SymbolContextScope *sc_scope,
+lldb::TypeClass type_mask,
+lldb_private::TypeList _list) override;
 
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
Index: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
===
--- lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
+++ lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
@@ -47,11 +47,9 @@
   return new SymbolFileSymtab(std::move(objfile_sp));
 }
 
-size_t SymbolFileSymtab::GetTypes(SymbolContextScope *sc_scope,
-  TypeClass type_mask,
-  lldb_private::TypeList _list) {
-  return 0;
-}
+void SymbolFileSymtab::GetTypes(SymbolContextScope *sc_scope,
+TypeClass type_mask,
+lldb_private::TypeList _list) {}
 
 SymbolFileSymtab::SymbolFileSymtab(ObjectFileSP objfile_sp)
 : SymbolFile(std::move(objfile_sp)), m_source_indexes(), m_func_indexes(),
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -125,23 +125,23 @@
 
   void AddSymbols(lldb_private::Symtab ) override;
 
-  uint32_t
+  void
   FindTypes(lldb_private::ConstString name,
 const lldb_private::CompilerDeclContext *parent_decl_ctx,
 uint32_t max_matches,
 llvm::DenseSet _symbol_files,
 lldb_private::TypeMap ) override;
 
-  size_t FindTypes(llvm::ArrayRef pattern,
-   lldb_private::LanguageSet languages,
-   lldb_private::TypeMap ) override;
+  void FindTypes(llvm::ArrayRef pattern,
+ lldb_private::LanguageSet languages,
+ lldb_private::TypeMap ) override;
 
   void FindTypesByRegex(const lldb_private::RegularExpression ,
 uint32_t 

[Lldb-commits] [PATCH] D68169: Remove size_t return parameter from FindTypes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 222473.
aprantl retitled this revision from "Fix a regression in FindTypes" to "Remove 
size_t return parameter from FindTypes".
aprantl edited the summary of this revision.
aprantl added a reviewer: labath.
Herald added subscribers: dexonsmith, mehdi_amini.

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

https://reviews.llvm.org/D68169

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Core/ModuleList.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/TypeList.h
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
  lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
  lldb/source/Symbol/SymbolFile.cpp

Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -139,18 +139,14 @@
   return;
 }
 
-uint32_t SymbolFile::FindTypes(
+void SymbolFile::FindTypes(
 ConstString name, const CompilerDeclContext *parent_decl_ctx,
 uint32_t max_matches,
 llvm::DenseSet _symbol_files,
-TypeMap ) {
-  return 0;
-}
+TypeMap ) {}
 
-size_t SymbolFile::FindTypes(llvm::ArrayRef pattern,
- LanguageSet languages, TypeMap ) {
-  return 0;
-}
+void SymbolFile::FindTypes(llvm::ArrayRef pattern,
+   LanguageSet languages, TypeMap ) {}
 
 void SymbolFile::AssertModuleLock() {
   // The code below is too expensive to leave enabled in release builds. It's
Index: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
===
--- lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -71,9 +71,9 @@
 lldb::SymbolContextItem resolve_scope,
 lldb_private::SymbolContext ) override;
 
-  size_t GetTypes(lldb_private::SymbolContextScope *sc_scope,
-  lldb::TypeClass type_mask,
-  lldb_private::TypeList _list) override;
+  void GetTypes(lldb_private::SymbolContextScope *sc_scope,
+lldb::TypeClass type_mask,
+lldb_private::TypeList _list) override;
 
   // PluginInterface protocol
   lldb_private::ConstString GetPluginName() override;
Index: lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
===
--- lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
+++ lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
@@ -47,11 +47,9 @@
   return new SymbolFileSymtab(std::move(objfile_sp));
 }
 
-size_t SymbolFileSymtab::GetTypes(SymbolContextScope *sc_scope,
-  TypeClass type_mask,
-  lldb_private::TypeList _list) {
-  return 0;
-}
+void SymbolFileSymtab::GetTypes(SymbolContextScope *sc_scope,
+TypeClass type_mask,
+lldb_private::TypeList _list) {}
 
 SymbolFileSymtab::SymbolFileSymtab(ObjectFileSP objfile_sp)
 : SymbolFile(std::move(objfile_sp)), m_source_indexes(), m_func_indexes(),
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -125,23 +125,23 @@
 
   void AddSymbols(lldb_private::Symtab ) override;
 
-  uint32_t
+  void
   FindTypes(lldb_private::ConstString name,
 const lldb_private::CompilerDeclContext *parent_decl_ctx,
 uint32_t max_matches,
 llvm::DenseSet _symbol_files,
 lldb_private::TypeMap ) override;
 
-  size_t FindTypes(llvm::ArrayRef pattern,
-   lldb_private::LanguageSet languages,
-   lldb_private::TypeMap ) override;
+  

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

This looks fine to me, it doesn't look like you are leaving out anything useful.

Note, I think just to enforce the "default constructor" discipline, there's a 
test in the test suite 
(python_api/default-constructor/TestDefaultConstructorForAPIObjects.py) that 
has a file per class that just makes a default constructed object and calls all 
of its methods to make sure they don't crash.  According to that pattern your 
test_sbfile_invalid test should actually go there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793



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

[Lldb-commits] [PATCH] D68160: File::Clear() -> File::TakeStreamAndClear()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna updated this revision to Diff 222459.
lawrence_danna added a comment.

review fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68160

Files:
  lldb/include/lldb/Host/File.h
  lldb/scripts/Python/python-typemaps.swig
  lldb/source/Host/common/File.cpp


Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -162,13 +162,16 @@
   return error;
 }
 
-void File::Clear() {
-  m_stream = nullptr;
+FILE *File::TakeStreamAndClear() {
+  FILE *stream = GetStream();
+  m_stream = NULL;
   m_descriptor = kInvalidDescriptor;
   m_options = 0;
   m_own_stream = false;
+  m_own_descriptor = false;
   m_is_interactive = m_supports_colors = m_is_real_terminal =
   eLazyBoolCalculate;
+  return stream;
 }
 
 Status File::GetFileSpec(FileSpec _spec) const {
Index: lldb/scripts/Python/python-typemaps.swig
===
--- lldb/scripts/Python/python-typemaps.swig
+++ lldb/scripts/Python/python-typemaps.swig
@@ -372,6 +372,9 @@
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+// FIXME both of these paths wind up calling fdopen() with no provision for 
ever calling
+// fclose() on the result.  SB interfaces that use FILE* should be deprecated 
for scripting
+// use and this typemap should eventually be removed.
 %typemap(in) FILE * {
using namespace lldb_private;
if ($input == Py_None)
@@ -398,9 +401,7 @@
   lldb::FileUP file = py_file.GetUnderlyingFile();
   if (!file)
  return nullptr;
-  $1 = file->GetStream();
-  if ($1)
- file->Clear();
+  $1 = file->TakeStreamAndClear();
 }
 }
 
Index: lldb/include/lldb/Host/File.h
===
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -120,7 +120,19 @@
 
   Status Close() override;
 
-  void Clear();
+  /// DEPRECATED! Extract the underlying FILE* and reset this File without 
closing it.
+  ///
+  /// This is only here to support legacy SB interfaces that need to convert 
scripting
+  /// language objects into FILE* streams.   That conversion is inherently 
sketchy and
+  /// doing so may cause the stream to be leaked.
+  ///
+  /// After calling this the File will be reset to its original state.  It 
will be
+  /// invalid and it will not hold on to any resources.
+  ///
+  /// \return
+  /// The underlying FILE* stream from this File, if one exists and can be 
extracted,
+  /// nullptr otherwise.
+  FILE *TakeStreamAndClear();
 
   int GetDescriptor() const;
 


Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -162,13 +162,16 @@
   return error;
 }
 
-void File::Clear() {
-  m_stream = nullptr;
+FILE *File::TakeStreamAndClear() {
+  FILE *stream = GetStream();
+  m_stream = NULL;
   m_descriptor = kInvalidDescriptor;
   m_options = 0;
   m_own_stream = false;
+  m_own_descriptor = false;
   m_is_interactive = m_supports_colors = m_is_real_terminal =
   eLazyBoolCalculate;
+  return stream;
 }
 
 Status File::GetFileSpec(FileSpec _spec) const {
Index: lldb/scripts/Python/python-typemaps.swig
===
--- lldb/scripts/Python/python-typemaps.swig
+++ lldb/scripts/Python/python-typemaps.swig
@@ -372,6 +372,9 @@
   $1 = $1 || PyCallable_Check(reinterpret_cast($input));
 }
 
+// FIXME both of these paths wind up calling fdopen() with no provision for ever calling
+// fclose() on the result.  SB interfaces that use FILE* should be deprecated for scripting
+// use and this typemap should eventually be removed.
 %typemap(in) FILE * {
using namespace lldb_private;
if ($input == Py_None)
@@ -398,9 +401,7 @@
   lldb::FileUP file = py_file.GetUnderlyingFile();
   if (!file)
  return nullptr;
-  $1 = file->GetStream();
-  if ($1)
- file->Clear();
+  $1 = file->TakeStreamAndClear();
 }
 }
 
Index: lldb/include/lldb/Host/File.h
===
--- lldb/include/lldb/Host/File.h
+++ lldb/include/lldb/Host/File.h
@@ -120,7 +120,19 @@
 
   Status Close() override;
 
-  void Clear();
+  /// DEPRECATED! Extract the underlying FILE* and reset this File without closing it.
+  ///
+  /// This is only here to support legacy SB interfaces that need to convert scripting
+  /// language objects into FILE* streams.   That conversion is inherently sketchy and
+  /// doing so may cause the stream to be leaked.
+  ///
+  /// After calling this the File will be reset to its original state.  It will be
+  /// invalid and it will not hold on to any resources.
+  ///
+  /// \return
+  ///  

[Lldb-commits] [PATCH] D68160: File::Clear() -> File::TakeStreamAndClear()

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna marked 2 inline comments as done.
lawrence_danna added inline comments.



Comment at: lldb/source/Host/common/File.cpp:166-167
+FILE *File::TakeStreamAndClear() {
+  GetStream();
+  FILE *stream = m_stream;
+  m_stream = NULL;

labath wrote:
> `FILE *stream = GetStream()` ?
yea that's better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68160



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

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

It would be good as Pavel and Davide suggest to write a test directly for the 
JSON parser.  Doing so in the C++ Unit test seems the most convenient, but 
that's up to you.  But it would also be good to add a test for the particular 
"breakpoint write" -> "breakpoint read" scenario that uncovered this bug.  
There are already some tests for that in the 
functionalities/breakpoints/serialize test case, so it should be easy to add 
one there.


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

https://reviews.llvm.org/D68179



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

[Lldb-commits] [lldb] r373233 - [test] Make TestBasicEntryValuesX86_64 run on Linux as well as Darwin

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Mon Sep 30 10:11:46 2019
New Revision: 373233

URL: http://llvm.org/viewvc/llvm-project?rev=373233=rev
Log:
[test] Make TestBasicEntryValuesX86_64 run on Linux as well as Darwin

I think a reasonable plan here is to add support for OSes following the
SysV ABI one by one, watching the bots as we go.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py?rev=373233=373232=373233=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
 Mon Sep 30 10:11:46 2019
@@ -1,8 +1,12 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
+from lldbsuite.test import lldbplatformutil
+
+supported_platforms = ["linux"]
+supported_platforms.extend(lldbplatformutil.getDarwinOSTriples())
 
 lldbinline.MakeInlineTest(__file__, globals(),
-[decorators.skipUnlessDarwin,
+[decorators.skipUnlessPlatform(supported_platforms),
  decorators.skipUnlessArch('x86_64'),
  decorators.skipUnlessHasCallSiteInfo,
  decorators.skipIf(dwarf_version=['<', '4'])])


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

[Lldb-commits] [lldb] r373229 - [lldb][NFC] Updating test to reflect made by D67966

[Shafik Yaghmour via lldb-commits]

Author: shafik
Date: Mon Sep 30 09:57:35 2019
New Revision: 373229

URL: http://llvm.org/viewvc/llvm-project?rev=373229=rev
Log:
[lldb][NFC] Updating test to reflect made by D67966

Summary:
D67966 changes the output when dumping DWARF expressions and this updates 
basic_entry_values_x86_64 test to reflect this change.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp?rev=373229=373228=373229=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
 Mon Sep 30 09:57:35 2019
@@ -31,7 +31,7 @@ void func1(int , int x) {
   DESTROY_RSI;
 
   //% self.filecheck("image lookup -va $pc", "main.cpp", 
"-check-prefix=FUNC1-DESC")
-  // FUNC1-DESC: name = "x", type = "int", location = DW_OP_entry_value( rsi)
+  // FUNC1-DESC: name = "x", type = "int", location = 
DW_OP_entry_value(DW_OP_reg4 RSI)
 
   ++sink;
 }


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

[Lldb-commits] [PATCH] D68171: Remove unused "append" parameter from FindTypes API

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373224: Remove unused append parameter from 
FindTypes API (authored by adrian, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68171?vs=69=222453#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68171

Files:
  lldb/trunk/include/lldb/Core/Module.h
  lldb/trunk/include/lldb/Symbol/SymbolFile.h
  lldb/trunk/source/Core/Module.cpp
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/trunk/source/Symbol/SymbolFile.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp

Index: lldb/trunk/source/Core/Module.cpp
===
--- lldb/trunk/source/Core/Module.cpp
+++ lldb/trunk/source/Core/Module.cpp
@@ -941,13 +941,13 @@
 
 size_t Module::FindTypes_Impl(
 ConstString name, const CompilerDeclContext *parent_decl_ctx,
-bool append, size_t max_matches,
+size_t max_matches,
 llvm::DenseSet _symbol_files,
 TypeMap ) {
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
   if (SymbolFile *symbols = GetSymbolFile())
-return symbols->FindTypes(name, parent_decl_ctx, append, max_matches,
+return symbols->FindTypes(name, parent_decl_ctx, max_matches,
   searched_symbol_files, types);
   return 0;
 }
@@ -955,12 +955,10 @@
 size_t Module::FindTypesInNamespace(ConstString type_name,
 const CompilerDeclContext *parent_decl_ctx,
 size_t max_matches, TypeList _list) {
-  const bool append = true;
   TypeMap types_map;
   llvm::DenseSet searched_symbol_files;
-  size_t num_types =
-  FindTypes_Impl(type_name, parent_decl_ctx, append, max_matches,
- searched_symbol_files, types_map);
+  size_t num_types = FindTypes_Impl(type_name, parent_decl_ctx, max_matches,
+searched_symbol_files, types_map);
   if (num_types > 0) {
 SymbolContext sc;
 sc.module_sp = shared_from_this();
@@ -988,7 +986,6 @@
   const char *type_name_cstr = name.GetCString();
   llvm::StringRef type_scope;
   llvm::StringRef type_basename;
-  const bool append = true;
   TypeClass type_class = eTypeClassAny;
   TypeMap typesmap;
 
@@ -1001,7 +998,7 @@
 exact_match = type_scope.consume_front("::");
 
 ConstString type_basename_const_str(type_basename);
-if (FindTypes_Impl(type_basename_const_str, nullptr, append, max_matches,
+if (FindTypes_Impl(type_basename_const_str, nullptr, max_matches,
searched_symbol_files, typesmap)) {
   typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class,
  exact_match);
@@ -1013,13 +1010,13 @@
 if (type_class != eTypeClassAny && !type_basename.empty()) {
   // The "type_name_cstr" will have been modified if we have a valid type
   // class prefix (like "struct", "class", "union", "typedef" etc).
-  FindTypes_Impl(ConstString(type_basename), nullptr, append, UINT_MAX,
+  FindTypes_Impl(ConstString(type_basename), nullptr, UINT_MAX,
  searched_symbol_files, typesmap);
   typesmap.RemoveMismatchedTypes(type_scope, type_basename, type_class,
  exact_match);
   num_matches = typesmap.GetSize();
 } else {
-  num_matches = FindTypes_Impl(name, nullptr, append, UINT_MAX,
+  num_matches = FindTypes_Impl(name, nullptr, UINT_MAX,
searched_symbol_files, typesmap);
   if (exact_match) {
 std::string name_str(name.AsCString(""));
@@ -1038,11 +1035,11 @@
 }
 
 size_t Module::FindTypes(llvm::ArrayRef pattern,
- LanguageSet languages, bool append, TypeMap ) {
+ LanguageSet languages, TypeMap ) {
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
   if (SymbolFile *symbols = GetSymbolFile())
-return symbols->FindTypes(pattern, languages, append, types);
+return 

[Lldb-commits] [lldb] r373224 - Remove unused "append" parameter from FindTypes API

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Mon Sep 30 09:42:28 2019
New Revision: 373224

URL: http://llvm.org/viewvc/llvm-project?rev=373224=rev
Log:
Remove unused "append" parameter from FindTypes API

I noticed that SymbolFileDWARFDebugMap::FindTypes was implementing it
incorrectly (passing append=false in a for-loop to recursive calls to
FindTypes would yield only the very last set of results), but instead
of fixing it, removing it seemed like an even better option.

rdar://problem/54412692

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

Modified:
lldb/trunk/include/lldb/Core/Module.h
lldb/trunk/include/lldb/Symbol/SymbolFile.h
lldb/trunk/source/Core/Module.cpp
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/trunk/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/trunk/source/Symbol/SymbolFile.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/include/lldb/Core/Module.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Module.h?rev=373224=373223=373224=diff
==
--- lldb/trunk/include/lldb/Core/Module.h (original)
+++ lldb/trunk/include/lldb/Core/Module.h Mon Sep 30 09:42:28 2019
@@ -460,7 +460,7 @@ public:
   /// specify a DeclContext and a language for the type being searched
   /// for.
   size_t FindTypes(llvm::ArrayRef pattern,
-   LanguageSet languages, bool append, TypeMap );
+   LanguageSet languages, TypeMap );
 
   lldb::TypeSP FindFirstType(const SymbolContext ,
  ConstString type_name, bool exact_match);
@@ -1076,7 +1076,7 @@ private:
 
   size_t FindTypes_Impl(
   ConstString name, const CompilerDeclContext *parent_decl_ctx,
-  bool append, size_t max_matches,
+  size_t max_matches,
   llvm::DenseSet _symbol_files,
   TypeMap );
 

Modified: lldb/trunk/include/lldb/Symbol/SymbolFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/SymbolFile.h?rev=373224=373223=373224=diff
==
--- lldb/trunk/include/lldb/Symbol/SymbolFile.h (original)
+++ lldb/trunk/include/lldb/Symbol/SymbolFile.h Mon Sep 30 09:42:28 2019
@@ -190,15 +190,14 @@ public:
  SymbolContextList _list);
   virtual uint32_t
   FindTypes(ConstString name, const CompilerDeclContext *parent_decl_ctx,
-bool append, uint32_t max_matches,
+uint32_t max_matches,
 llvm::DenseSet _symbol_files,
 TypeMap );
 
   /// Find types specified by a CompilerContextPattern.
   /// \param languagesOnly return results in these languages.
   virtual size_t FindTypes(llvm::ArrayRef pattern,
-   LanguageSet languages, bool append,
-   TypeMap );
+   LanguageSet languages, TypeMap );
 
   virtual void
   GetMangledNamesForFunction(const std::string _qualified_name,

Modified: lldb/trunk/source/Core/Module.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Module.cpp?rev=373224=373223=373224=diff
==
--- lldb/trunk/source/Core/Module.cpp (original)
+++ lldb/trunk/source/Core/Module.cpp Mon Sep 30 09:42:28 2019
@@ -941,13 +941,13 @@ void Module::FindAddressesForLine(const
 
 size_t Module::FindTypes_Impl(
 ConstString name, const CompilerDeclContext *parent_decl_ctx,
-bool append, size_t max_matches,
+size_t max_matches,
 llvm::DenseSet _symbol_files,
 TypeMap ) {
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
   if (SymbolFile *symbols = GetSymbolFile())
-return symbols->FindTypes(name, parent_decl_ctx, append, max_matches,
+return symbols->FindTypes(name, parent_decl_ctx, max_matches,
   searched_symbol_files, types);
   return 0;
 }
@@ -955,12 +955,10 @@ size_t Module::FindTypes_Impl(
 size_t Module::FindTypesInNamespace(ConstString type_name,
 const CompilerDeclContext *parent_decl_ctx,
 size_t max_matches, TypeList _list) {
-  const bool append = true;
   TypeMap 

[Lldb-commits] [PATCH] D68007: [lldb] Move swig call from python code to cmake

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/trunk/scripts/CMakeLists.txt:5
+  ${LLDB_SOURCE_DIR}/include/lldb/API/*.h
+  ${LLDB_SOURCE_DIR}/include/lldb/*.h
 )

xiaobai wrote:
> hhb wrote:
> > xiaobai wrote:
> > > Doesn't this now include the `lldb-private` headers now? Is that intended?
> > Oh sorry. Will send a change to fix. Is there an easy way to exclude a 
> > pattern in file() call?
> You can use `EXCLUDE` but I'm not sure that works with `file(GLOB)`. It might 
> be better just to enumerate all the headers you want to use instead of 
> globbing.
+1


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68007



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

ping


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D68171: Remove unused "append" parameter from FindTypes API

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.

拾


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

https://reviews.llvm.org/D68171



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

[Lldb-commits] [PATCH] D68169: Fix a regression in FindTypes

[Adrian Prantl via Phabricator via lldb-commits]

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

Pavel suggested to just remove the return value altogether, which would be even 
better. I'll give that a try.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68169



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

[Lldb-commits] [PATCH] D68096: ProcessMinidump: Suppress reporting stop for signal '0'

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet marked 2 inline comments as done.
JosephTremoulet added a comment.

In D68096#1687790 , @labath wrote:

> It doesn't look like it should be too hard to add yaml support for the 
> exceptions stream -- it should only be a matter of adapting the patterns 
> already used for other stream to work for this particular case. Could you 
> give a go at that?


Yes, will do, thanks for the pointers.




Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:258-265
+uint32_t signo = m_active_exception->exception_record.exception_code;
+
+if (signo == 0) {
+  // Artifically inject a SIGSTOP so that we'll find a stop reason
+  // when we process the stop event.
+  signo = GetUnixSignals()->GetSignalNumberFromName("SIGSTOP");
+}

labath wrote:
> After some investigation, I re-learned about the DumpRequested constant, 
> which is already checked for in this code (and it is tested 
> (TestMiniDumpNew.py:test_snapshow_minidump) and works/does not hang). It 
> seems to me like it would make sense to treat this case the same way as 
> DumpRequested, and just don't return any stop info (instead of returning a 
> stop info with signal 0 or SIGSTOP). IOW, you would just put `if (signo==0) 
> return;` here. Eventually we could apply the same change to process elf core 
> as well..
> 
> WDYT?
SGTM, and seems to work for my use case.  I wasn't sure though whether to leave 
the `if (signo==0) return` under `if (arch.GetTripe().getOS() == 
llvm::Triple::Linux)`.  Originally I'd put logic here just because I was 
copying logic from Process**Elf**Core.  It looks like the Apple case here would 
maybe similarly have an issue with exception code zero (?), but the `else` case 
(Windows?) doesn't reference the exception_code at all so I'd hesitate to apply 
this change there... I've left it under the check for Linux, but happy to move 
it if that seems wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68096



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

[Lldb-commits] [PATCH] D68096: ProcessMinidump: inject SIGSTOP on Linux if no thread has a signal

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 222444.
JosephTremoulet added a comment.

- Review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68096

Files:
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp


Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -215,6 +215,9 @@
 
   m_thread_list = m_minidump_parser->GetThreads();
   m_active_exception = m_minidump_parser->GetExceptionStream();
+
+  SetUnixSignals(UnixSignals::Create(GetArchitecture()));
+
   ReadModuleList();
 
   llvm::Optional pid = m_minidump_parser->GetPid();
@@ -234,6 +237,7 @@
 Status ProcessMinidump::DoDestroy() { return Status(); }
 
 void ProcessMinidump::RefreshStateAfterStop() {
+
   if (!m_active_exception)
 return;
 
@@ -250,8 +254,15 @@
   ArchSpec arch = GetArchitecture();
 
   if (arch.GetTriple().getOS() == llvm::Triple::Linux) {
+uint32_t signo = m_active_exception->exception_record.exception_code;
+
+if (signo == 0) {
+  // No stop.
+  return;
+}
+
 stop_info = StopInfo::CreateStopReasonWithSignal(
-*stop_thread, m_active_exception->exception_record.exception_code);
+*stop_thread, signo);
   } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) {
 stop_info = StopInfoMachException::CreateStopReasonWithMachException(
 *stop_thread, m_active_exception->exception_record.exception_code, 2,


Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -215,6 +215,9 @@
 
   m_thread_list = m_minidump_parser->GetThreads();
   m_active_exception = m_minidump_parser->GetExceptionStream();
+
+  SetUnixSignals(UnixSignals::Create(GetArchitecture()));
+
   ReadModuleList();
 
   llvm::Optional pid = m_minidump_parser->GetPid();
@@ -234,6 +237,7 @@
 Status ProcessMinidump::DoDestroy() { return Status(); }
 
 void ProcessMinidump::RefreshStateAfterStop() {
+
   if (!m_active_exception)
 return;
 
@@ -250,8 +254,15 @@
   ArchSpec arch = GetArchitecture();
 
   if (arch.GetTriple().getOS() == llvm::Triple::Linux) {
+uint32_t signo = m_active_exception->exception_record.exception_code;
+
+if (signo == 0) {
+  // No stop.
+  return;
+}
+
 stop_info = StopInfo::CreateStopReasonWithSignal(
-*stop_thread, m_active_exception->exception_record.exception_code);
+*stop_thread, signo);
   } else if (arch.GetTriple().getVendor() == llvm::Triple::Apple) {
 stop_info = StopInfoMachException::CreateStopReasonWithMachException(
 *stop_thread, m_active_exception->exception_record.exception_code, 2,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

I'm just back from vacation.  I agree with Pavel that we need to hear more from 
Jason at this point.  I'm still very interested in helping this land in some 
form.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67347



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

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 222441.
kwk added a comment.

- include test code in .c test file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/merge-symbols.yaml
  lldb/lit/Modules/lit.local.cfg
  lldb/lit/helper/toolchain.py
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -12,6 +12,7 @@
 #include 
 
 #include 
+#include 
 
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
@@ -55,7 +56,7 @@
 /// This class provides a generic ELF (32/64 bit) reader plugin implementing
 /// the ObjectFile protocol.
 class ObjectFileELF : public lldb_private::ObjectFile {
-public:
+public:  
   // Static Functions
   static void Initialize();
 
@@ -184,6 +185,8 @@
 
   typedef std::map
   FileAddressToAddressClassMap;
+  
+  typedef std::unordered_set UniqueElfSymbolColl;
 
   /// Version of this reader common to all plugins based on this class.
   static const uint32_t m_plugin_version = 1;
@@ -278,7 +281,8 @@
   /// will parse the symbols only once.  Returns the number of symbols parsed.
   unsigned ParseSymbolTable(lldb_private::Symtab *symbol_table,
 lldb::user_id_t start_id,
-lldb_private::Section *symtab);
+lldb_private::Section *symtab,
+UniqueElfSymbolColl _elf_symbols);
 
   /// Helper routine for ParseSymbolTable().
   unsigned ParseSymbols(lldb_private::Symtab *symbol_table,
@@ -286,7 +290,8 @@
 lldb_private::SectionList *section_list,
 const size_t num_symbols,
 const lldb_private::DataExtractor _data,
-const lldb_private::DataExtractor _data);
+const lldb_private::DataExtractor _data,
+UniqueElfSymbolColl _elf_symbols);
 
   /// Scans the relocation entries and adds a set of artificial symbols to the
   /// given symbol table for each PLT slot.  Returns the number of symbols
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1871,7 +1871,8 @@
  SectionList *section_list,
  const size_t num_symbols,
  const DataExtractor _data,
- const DataExtractor _data) {
+ const DataExtractor _data,
+ UniqueElfSymbolColl _elf_symbols) {
   ELFSymbol symbol;
   lldb::offset_t offset = 0;
 
@@ -2196,20 +2197,28 @@
 symbol_size_valid,  // Symbol size is valid
 has_suffix, // Contains linker annotations?
 flags); // Symbol flags.
-symtab->AddSymbol(dc_symbol);
+
+NamedELFSymbol needle(symbol, ConstString(symbol_ref),
+  symbol_section_sp.get() ? symbol_section_sp->GetName()
+  : ConstString());
+if (unique_elf_symbols.insert(needle).second) {
+  symtab->AddSymbol(dc_symbol);
+}
   }
   return i;
 }
 
-unsigned ObjectFileELF::ParseSymbolTable(Symtab *symbol_table,
- user_id_t start_id,
- lldb_private::Section *symtab) {
+unsigned
+ObjectFileELF::ParseSymbolTable(Symtab *symbol_table, user_id_t start_id,
+lldb_private::Section *symtab,
+UniqueElfSymbolColl _elf_symbols) {
   if (symtab->GetObjectFile() != this) {
 // If the symbol table section is owned by a different object file, have it
 // do the parsing.
 ObjectFileELF *obj_file_elf =
 static_cast(symtab->GetObjectFile());
-return obj_file_elf->ParseSymbolTable(symbol_table, start_id, symtab);
+return obj_file_elf->ParseSymbolTable(symbol_table, start_id, symtab,
+  unique_elf_symbols);
   }
 
   // Get section list for this object file.
@@ -2237,7 +2246,7 @@
   size_t num_symbols = symtab_data.GetByteSize() / symtab_hdr->sh_entsize;
 
   return ParseSymbols(symbol_table, start_id, section_list, 

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 222427.
kwk added a comment.

- Added YAML test to merge symbols


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/load-from-dynsym-alone.test
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
  lldb/lit/Modules/ELF/merge-symbols.yaml
  lldb/lit/helper/toolchain.py
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -12,6 +12,7 @@
 #include 
 
 #include 
+#include 
 
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
@@ -55,7 +56,7 @@
 /// This class provides a generic ELF (32/64 bit) reader plugin implementing
 /// the ObjectFile protocol.
 class ObjectFileELF : public lldb_private::ObjectFile {
-public:
+public:  
   // Static Functions
   static void Initialize();
 
@@ -184,6 +185,8 @@
 
   typedef std::map
   FileAddressToAddressClassMap;
+  
+  typedef std::unordered_set UniqueElfSymbolColl;
 
   /// Version of this reader common to all plugins based on this class.
   static const uint32_t m_plugin_version = 1;
@@ -278,7 +281,8 @@
   /// will parse the symbols only once.  Returns the number of symbols parsed.
   unsigned ParseSymbolTable(lldb_private::Symtab *symbol_table,
 lldb::user_id_t start_id,
-lldb_private::Section *symtab);
+lldb_private::Section *symtab,
+UniqueElfSymbolColl _elf_symbols);
 
   /// Helper routine for ParseSymbolTable().
   unsigned ParseSymbols(lldb_private::Symtab *symbol_table,
@@ -286,7 +290,8 @@
 lldb_private::SectionList *section_list,
 const size_t num_symbols,
 const lldb_private::DataExtractor _data,
-const lldb_private::DataExtractor _data);
+const lldb_private::DataExtractor _data,
+UniqueElfSymbolColl _elf_symbols);
 
   /// Scans the relocation entries and adds a set of artificial symbols to the
   /// given symbol table for each PLT slot.  Returns the number of symbols
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1871,7 +1871,8 @@
  SectionList *section_list,
  const size_t num_symbols,
  const DataExtractor _data,
- const DataExtractor _data) {
+ const DataExtractor _data,
+ UniqueElfSymbolColl _elf_symbols) {
   ELFSymbol symbol;
   lldb::offset_t offset = 0;
 
@@ -2196,20 +2197,27 @@
 symbol_size_valid,  // Symbol size is valid
 has_suffix, // Contains linker annotations?
 flags); // Symbol flags.
-symtab->AddSymbol(dc_symbol);
+
+NamedELFSymbol needle(symbol, ConstString(symbol_ref),
+  ConstString(symbol_section_sp->GetName()));
+if (unique_elf_symbols.insert(needle).second) {
+  symtab->AddSymbol(dc_symbol);
+}
   }
   return i;
 }
 
-unsigned ObjectFileELF::ParseSymbolTable(Symtab *symbol_table,
- user_id_t start_id,
- lldb_private::Section *symtab) {
+unsigned
+ObjectFileELF::ParseSymbolTable(Symtab *symbol_table, user_id_t start_id,
+lldb_private::Section *symtab,
+UniqueElfSymbolColl _elf_symbols) {
   if (symtab->GetObjectFile() != this) {
 // If the symbol table section is owned by a different object file, have it
 // do the parsing.
 ObjectFileELF *obj_file_elf =
 static_cast(symtab->GetObjectFile());
-return obj_file_elf->ParseSymbolTable(symbol_table, start_id, symtab);
+return obj_file_elf->ParseSymbolTable(symbol_table, start_id, symtab,
+  unique_elf_symbols);
   }
 
   // Get section list for this object file.
@@ -2237,7 +2245,7 @@
   size_t num_symbols = symtab_data.GetByteSize() / symtab_hdr->sh_entsize;
 
   return ParseSymbols(symbol_table, start_id, section_list, 

[Lldb-commits] [PATCH] D67966: Use llvm for dumping DWARF expressions

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373208: Use llvm for dumping DWARF expressions (authored by 
labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67966

Files:
  lldb/trunk/include/lldb/Utility/DataExtractor.h
  lldb/trunk/lit/SymbolFile/DWARF/debug_loc.s
  lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s
  lldb/trunk/source/Expression/DWARFExpression.cpp
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/trunk/source/Symbol/ClangASTContext.cpp
  lldb/trunk/unittests/Utility/ScalarTest.cpp

Index: lldb/trunk/unittests/Utility/ScalarTest.cpp
===
--- lldb/trunk/unittests/Utility/ScalarTest.cpp
+++ lldb/trunk/unittests/Utility/ScalarTest.cpp
@@ -16,7 +16,9 @@
 #include "llvm/Testing/Support/Error.h"
 
 using namespace lldb_private;
-using namespace llvm;
+using llvm::APInt;
+using llvm::Failed;
+using llvm::Succeeded;
 
 template 
 bool checkInequality(T c1, T c2) {
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -86,7 +86,6 @@
 #include "llvm/Support/raw_ostream.h"
 
 #define DEBUGSERVER_BASENAME "debugserver"
-using namespace llvm;
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
Index: lldb/trunk/source/Expression/DWARFExpression.cpp
===
--- lldb/trunk/source/Expression/DWARFExpression.cpp
+++ lldb/trunk/source/Expression/DWARFExpression.cpp
@@ -88,413 +88,10 @@
lldb::offset_t length,
lldb::DescriptionLevel level,
ABI *abi) const {
-  if (!m_data.ValidOffsetForDataOfSize(offset, length))
-return;
-  const lldb::offset_t start_offset = offset;
-  const lldb::offset_t end_offset = offset + length;
-
-  // An operation within a DWARF expression may contain a sub-expression. The
-  // motivating example for this is DW_OP_entry_value. Keep track of where each
-  // each sub-expression ends.
-  std::vector ends_of_subexprs;
-
-  // "Finish" (i.e. print the closing right-parens) for sub-expressions up to
-  // the specified \p op_offset.
-  auto finish_subexpressions_to = [&](const lldb::offset_t op_offset) {
-while (!ends_of_subexprs.empty() && op_offset >= ends_of_subexprs.back()) {
-  ends_of_subexprs.pop_back();
-  s->Printf(")");
-  if (!ends_of_subexprs.empty())
-s->Printf(" ");
-}
-  };
-
-  while (m_data.ValidOffset(offset) && offset < end_offset) {
-const lldb::offset_t op_offset = offset;
-const uint8_t op = m_data.GetU8();
-finish_subexpressions_to(op_offset);
-
-switch (level) {
-default:
-  break;
-
-case lldb::eDescriptionLevelBrief:
-  if (op_offset > start_offset)
-s->PutChar(' ');
-  break;
-
-case lldb::eDescriptionLevelFull:
-case lldb::eDescriptionLevelVerbose:
-  if (op_offset > start_offset)
-s->EOL();
-  s->Indent();
-  if (level == lldb::eDescriptionLevelFull)
-break;
-  // Fall through for verbose and print offset and DW_OP prefix..
-  s->Printf("0x%8.8" PRIx64 ": %s", op_offset,
-op >= DW_OP_APPLE_uninit ? "DW_OP_APPLE_" : "DW_OP_");
-  break;
-}
-
-switch (op) {
-case DW_OP_addr:
-  *s << "DW_OP_addr(" << m_data.GetAddress() << ") ";
-  break; // 0x03 1 address
-case DW_OP_deref:
-  *s << "DW_OP_deref";
-  break; // 0x06
-case DW_OP_const1u:
-  s->Printf("DW_OP_const1u(0x%2.2x)", m_data.GetU8());
-  break; // 0x08 1 1-byte constant
-case DW_OP_const1s:
-  s->Printf("DW_OP_const1s(0x%2.2x)", m_data.GetU8());
-  break; // 0x09 1 1-byte constant
-case DW_OP_const2u:
-  s->Printf("DW_OP_const2u(0x%4.4x)", m_data.GetU16());
-  break; // 0x0a 1 2-byte constant
-case DW_OP_const2s:
-  s->Printf("DW_OP_const2s(0x%4.4x)", m_data.GetU16());
-  break; // 0x0b 1 2-byte constant
-case DW_OP_const4u:
-  s->Printf("DW_OP_const4u(0x%8.8x)", m_data.GetU32());
-  break; // 0x0c 1 4-byte constant
-case DW_OP_const4s:
-  s->Printf("DW_OP_const4s(0x%8.8x)", m_data.GetU32());
-  break; // 0x0d 1 4-byte constant
-case DW_OP_const8u:
-  s->Printf("DW_OP_const8u(0x%16.16" PRIx64 ")", m_data.GetU64());
-  break; // 0x0e 1 8-byte constant
-case DW_OP_const8s:
-  s->Printf("DW_OP_const8s(0x%16.16" PRIx64 ")", m_data.GetU64());
-  break; // 0x0f 1 8-byte constant
-case DW_OP_constu:
-  

[Lldb-commits] [lldb] r373208 - Use llvm for dumping DWARF expressions

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Sep 30 06:44:17 2019
New Revision: 373208

URL: http://llvm.org/viewvc/llvm-project?rev=373208=rev
Log:
Use llvm for dumping DWARF expressions

Summary:
It uses the new ability of ABI plugins to vend llvm::MCRegisterInfo
structs (which is what is needed to turn dwarf register numbers into
strings).

Reviewers: JDevlieghere, aprantl, jasonmolenda

Subscribers: tatyana-krasnukha, lldb-commits

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

Modified:
lldb/trunk/include/lldb/Utility/DataExtractor.h
lldb/trunk/lit/SymbolFile/DWARF/debug_loc.s
lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s
lldb/trunk/source/Expression/DWARFExpression.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp
lldb/trunk/unittests/Utility/ScalarTest.cpp

Modified: lldb/trunk/include/lldb/Utility/DataExtractor.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/DataExtractor.h?rev=373208=373207=373208=diff
==
--- lldb/trunk/include/lldb/Utility/DataExtractor.h (original)
+++ lldb/trunk/include/lldb/Utility/DataExtractor.h Mon Sep 30 06:44:17 2019
@@ -14,6 +14,7 @@
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/DataExtractor.h"
 
 #include 
 #include 
@@ -995,6 +996,11 @@ public:
 return {GetDataStart(), size_t(GetByteSize())};
   }
 
+  llvm::DataExtractor GetAsLLVM() const {
+return {GetData(), GetByteOrder() == lldb::eByteOrderLittle,
+uint8_t(GetAddressByteSize())};
+  }
+
 protected:
   // Member variables
   const uint8_t *m_start; ///< A pointer to the first byte of data.

Modified: lldb/trunk/lit/SymbolFile/DWARF/debug_loc.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/debug_loc.s?rev=373208=373207=373208=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/debug_loc.s (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/debug_loc.s Mon Sep 30 06:44:17 2019
@@ -9,12 +9,12 @@
 # RUN:   | FileCheck %s
 
 # CHECK-LABEL: image lookup -v -a 0
-# CHECK: Variable: {{.*}}, name = "x0", type = "int", location = rdi,
+# CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg5 
RDI,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
 # CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 
 # CHECK-LABEL: image lookup -v -a 2
-# CHECK: Variable: {{.*}}, name = "x0", type = "int", location = rax,
+# CHECK: Variable: {{.*}}, name = "x0", type = "int", location = DW_OP_reg0 
RAX,
 # CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
 # CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 

Modified: lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s?rev=373208=373207=373208=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/dwarf5_locations.s Mon Sep 30 06:44:17 2019
@@ -7,7 +7,7 @@
 # RUN: lldb-test symbols %t | FileCheck %s
 
 # CHECK: Variable{0x7fff0011}, name = "color"
-# CHECK-SAME: location = DW_OP_addrx(0x0)
+# CHECK-SAME: location = DW_OP_addrx 0x0
 
 .text
 .section.debug_str,"MS",@progbits,1

Modified: lldb/trunk/source/Expression/DWARFExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DWARFExpression.cpp?rev=373208=373207=373208=diff
==
--- lldb/trunk/source/Expression/DWARFExpression.cpp (original)
+++ lldb/trunk/source/Expression/DWARFExpression.cpp Mon Sep 30 06:44:17 2019
@@ -88,413 +88,10 @@ void DWARFExpression::DumpLocation(Strea
lldb::offset_t length,
lldb::DescriptionLevel level,
ABI *abi) const {
-  if (!m_data.ValidOffsetForDataOfSize(offset, length))
-return;
-  const lldb::offset_t start_offset = offset;
-  const lldb::offset_t end_offset = offset + length;
-
-  // An operation within a DWARF expression may contain a sub-expression. The
-  // motivating example for this is DW_OP_entry_value. Keep track of where each
-  // each sub-expression ends.
-  std::vector ends_of_subexprs;
-
-  // "Finish" (i.e. print the closing right-parens) for sub-expressions up to
-  // the specified \p op_offset.
-  auto finish_subexpressions_to = [&](const lldb::offset_t op_offset) {
-while (!ends_of_subexprs.empty() && op_offset >= ends_of_subexprs.back()) {
-  ends_of_subexprs.pop_back();
-  s->Printf(")");
-  if (!ends_of_subexprs.empty())
-s->Printf(" ");
-}
-  

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@labath I did prepare some YAML file but apparently `yaml2obj` isn't meant to 
deal with this properly. Instead I get an Error like this: `yaml2obj: error: 
repeated symbol name: 'main'`. It looks like symbols from the `Symbols:` part 
of the YAML file are just added by name to a map. Changing `yaml2obj` for this 
seems a bit too heavy right now. If you're okay I'll go with a few more c 
programs if I can pull them off.

Here's the part hat causes the error:

  template  void ELFState::buildSymbolIndexes() {
auto Build = [this](ArrayRef V, NameToIdxMap ) {
  for (size_t I = 0, S = V.size(); I < S; ++I) {
const ELFYAML::Symbol  = V[I];
if (!Sym.Name.empty() && !Map.addName(Sym.Name, I + 1))
  reportError("repeated symbol name: '" + Sym.Name + "'");
  }
};
  
Build(Doc.Symbols, SymN2I);
Build(Doc.DynamicSymbols, DynSymN2I);
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390



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

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm afraid I don't have much to add beyond what I've already said. I think 
@jasonmolenda should make the call as to how to move forward here. If the 
chosen direction requires any changes in how the breakpad unwind info handled, 
I'm happy to give you a hand there.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67347



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

[Lldb-commits] [lldb] r373206 - [lldb][NFC][modern-type-lookup] Remove while(false) behind if() {}

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Mon Sep 30 06:08:08 2019
New Revision: 373206

URL: http://llvm.org/viewvc/llvm-project?rev=373206=rev
Log:
[lldb][NFC][modern-type-lookup] Remove while(false) behind if() {}

This was originally a 'do { ... } while (false);' like in the rest
of the function, but the do was refactored into an 'if' without
also removing the trailing 'while(false);'

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp?rev=373206=373205=373206=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp Mon Sep 
30 06:08:08 2019
@@ -129,8 +129,6 @@ void ClangASTSource::InstallASTContext(c
  *scratch_ast_context->getFileManager(),
  scratch_ast_context->GetOriginMap()});
 }
-while (false)
-  ;
 
 m_merger_up =
 std::make_unique(target, sources);


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

[Lldb-commits] [PATCH] D67222: [Windows] Added support of watchpoints to `NativeProcessWindows`

[Pavel Labath via Phabricator via lldb-commits]

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

Cool, maybe we should have a setting/environment variable/something to control 
which plugin is used. That would make it easier to switch between the two 
implementations.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67222



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

[Lldb-commits] [PATCH] D67589: Fix crash on SBCommandReturnObject & assignment

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/api/command-return-object/Makefile:1
+MAKE_DSYM := NO
+

jankratochvil wrote:
> jankratochvil wrote:
> > labath wrote:
> > > I'm pretty sure this line is not needed.
> > Removed in current testsuite: rL373061
> So I do not understand how `MAKE_DSYM` works but global removal of `MAKE_DSYM 
> := NO` in rL373061 broke OSX so I have reverted it by rL373110. Keeping 
> `MAKE_DSYM := NO` stays omitted for this testcase.
Yeah, the test which failed is doing pretty funky stuff, so I can imagine it 
can get hurt by removing MAKE_DSYM. Omitting it for this test should be safe 
though. (Probably all other tests except TestFunctionStarts.py too, but I don't 
think you have to bother with that.)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67589



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

[Lldb-commits] [PATCH] D68134: [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

In D68134#1687518 , @mstorsjo wrote:

> In D68134#1687500 , @labath wrote:
>
> > I'm not sure what failed here exactly, but there are some places in lldb 
> > that parse the demangled names. These might get confused by additional 
> > things appearing in the name. Though it's possible to also fix that, so the 
> > main question might be: what is the name we want to display to the users? I 
> > guess it would be the best if this matched what is displayed by other tools 
> > ?
>
>
> In this case, the output becomes the same as what has been chosen to be used 
> by the common llvm demangler, which probably should be a good choice in 
> general.


That makes sense to me, but I am not really a windows person. Maybe wait a 
while to see if any of the real "windows people" have any thoughts.

> 
> 
>> As for tests, you should at least be able to run the tests in the regular 
>> "host" setup, right ?
> 
> In principle, but I normally don't run windows except in a very underpowered 
> VM for testing things, and building there is no fun. I can spin up some more 
> powerful VMs for some better testing though.
> 
> In this case, much of the functionality of these tests require having the MS 
> DIA SDK available (which also implies building in MSVC/clang-cl mode, not 
> mingw mode), for reading PDB files. Probably just another step to do, but I 
> don't have it set up right now.

So, you're developing windows arm64 support, without even a real windows x86 
around? That's brave. :)


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

https://reviews.llvm.org/D68134



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

[Lldb-commits] [lldb] r373201 - [lldb] Reland 370734: Test 'frame select -r' and fix that INT32_MIN breaks the option parser

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Mon Sep 30 05:49:32 2019
New Revision: 373201

URL: http://llvm.org/viewvc/llvm-project?rev=373201=rev
Log:
[lldb] Reland 370734: Test 'frame select -r' and fix that INT32_MIN breaks the 
option parser

The problem with r370734 was that it removed the code for resetting the options 
in
OptionParsingStarting. This caused that once a 'frame select -r ...' command 
was executed,
we kept the relative index argument for all following 'frame select ...' 
invocations (even
the ones with an absolute index as they are the same command object). See 
rdar://55791276.

This relands the patch but keeps the code that resets the command options 
before execution.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
lldb/trunk/source/Commands/CommandObjectFrame.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py?rev=373201=373200=373201=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
 Mon Sep 30 05:49:32 2019
@@ -23,12 +23,55 @@ class TestFrameSelect(TestBase):
 
 self.expect("frame select -r -1", error=True, substrs=["Already at the 
bottom of the stack."])
 self.expect("frame select -r -2147483647", error=True, 
substrs=["Already at the bottom of the stack."])
+self.expect("frame select -r -2147483648", error=True, 
substrs=["error: invalid frame offset argument '-2147483648'"])
+self.expect("frame select -r -2147483649", error=True, 
substrs=["error: invalid frame offset argument '-2147483649'"])
 
 self.expect("frame select -r 1", substrs=["nested2() at"])
 self.expect("frame select -r -2", substrs=["nested3() at"])
 self.expect("frame select -r 1", substrs=["nested2() at"])
 self.expect("frame select -r -2147483647", substrs=["nested3() at"])
 self.expect("frame select -r 1", substrs=["nested2() at"])
+self.expect("frame select -r -2147483648", error=True, 
substrs=["error: invalid frame offset argument '-2147483648'"])
+self.expect("frame select -r -2147483649", error=True, 
substrs=["error: invalid frame offset argument '-2147483649'"])
 
 self.expect("frame select -r 100")
 self.expect("frame select -r 1", error=True, substrs=["Already at the 
top of the stack."])
+
+@no_debug_info_test
+@skipIfWindows
+def test_mixing_relative_and_abs(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+"// Set break point at this line.", lldb.SBFileSpec("main.cpp"))
+
+# The function associated with each frame index can change depending
+# on the function calling main (e.g. `start`), so this only tests that
+# the frame index number is correct. We test the actual functions
+# in the relative test.
+
+# Jump to the top of the stack.
+self.expect("frame select 0", substrs=["frame #0"])
+
+# Run some relative commands.
+self.expect("up", substrs=["frame #1"])
+self.expect("frame select -r 1", substrs=["frame #2"])
+self.expect("frame select -r -1", substrs=["frame #1"])
+
+# Test that absolute indices still work.
+self.expect("frame select 2", substrs=["frame #2"])
+self.expect("frame select 1", substrs=["frame #1"])
+self.expect("frame select 3", substrs=["frame #3"])
+self.expect("frame select 0", substrs=["frame #0"])
+self.expect("frame select 1", substrs=["frame #1"])
+
+# Run some other relative frame select commands.
+self.expect("down", substrs=["frame #0"])
+self.expect("frame select -r 1", substrs=["frame #1"])
+self.expect("frame select -r -1", substrs=["frame #0"])
+
+# Test that absolute indices still work.
+self.expect("frame select 2", substrs=["frame #2"])
+self.expect("frame select 1", substrs=["frame #1"])
+self.expect("frame select 3", substrs=["frame #3"])
+self.expect("frame select 0", substrs=["frame #0"])

Modified: lldb/trunk/source/Commands/CommandObjectFrame.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectFrame.cpp?rev=373201=373200=373201=diff
==
--- lldb/trunk/source/Commands/CommandObjectFrame.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectFrame.cpp Mon Sep 30 05:49:32 2019
@@ -246,13 +246,15 @@ public:
   Status error;
   const int short_option = m_getopt_table[option_idx].val;
   switch (short_option) {
-  case 'r':
-if 

[Lldb-commits] [PATCH] D68210: Object/minidump: Add support for the MemoryInfoList stream

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: amccarth, jhenderson, clayborg.
Herald added a project: LLVM.

This patch adds the definitions of the constants and structures
necessary to interpret the MemoryInfoList minidump stream, as well as
the object::MinidumpFile interface to access the stream.

While the code is fairly simple, there is one important deviation from
the other minidump streams, which is worth calling out explicitly.
Unlike other "List" streams, the size of the records inside
MemoryInfoList stream is not known statically. Instead it is described
in the stream header. This makes it impossible to return
ArrayRef from the accessor method, as it is done with other
streams. Instead, I create an iterator class, which can be parameterized
by the runtime size of the structure, and return
iterator_range instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D68210

Files:
  include/llvm/BinaryFormat/Minidump.h
  include/llvm/BinaryFormat/MinidumpConstants.def
  include/llvm/Object/Minidump.h
  lib/Object/Minidump.cpp
  unittests/Object/MinidumpTest.cpp

Index: unittests/Object/MinidumpTest.cpp
===
--- unittests/Object/MinidumpTest.cpp
+++ unittests/Object/MinidumpTest.cpp
@@ -511,3 +511,180 @@
 EXPECT_EQ(0x00090807u, MD.Memory.RVA);
   }
 }
+
+TEST(MinidumpFile, getMemoryInfoList) {
+  std::vector OneEntry{
+  // Header
+  'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  32, 0, 0, 0,  // StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, TimeDateStamp
+  0, 0, 0, 0, 0, 0, 0, 0,   // Flags
+// Stream Directory
+  16, 0, 0, 0, 64, 0, 0, 0, // Type, DataSize,
+  44, 0, 0, 0,  // RVA
+  // MemoryInfoListHeader
+  16, 0, 0, 0, 48, 0, 0, 0, // SizeOfHeader, SizeOfEntry
+  1, 0, 0, 0, 0, 0, 0, 0,   // NumberOfEntries
+  // MemoryInfo
+  0, 1, 2, 3, 4, 5, 6, 7,   // BaseAddress
+  8, 9, 0, 1, 2, 3, 4, 5,   // AllocationBase
+  16, 0, 0, 0, 6, 7, 8, 9,  // AllocationProtect, Reserved0
+  0, 1, 2, 3, 4, 5, 6, 7,   // RegionSize
+  0, 16, 0, 0, 32, 0, 0, 0, // State, Protect
+  0, 0, 2, 0, 8, 9, 0, 1,   // Type, Reserved1
+  };
+
+  // Same as before, but the list header is larger.
+  std::vector BiggerHeader{
+  // Header
+  'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  32, 0, 0, 0,  // StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, TimeDateStamp
+  0, 0, 0, 0, 0, 0, 0, 0,   // Flags
+// Stream Directory
+  16, 0, 0, 0, 68, 0, 0, 0, // Type, DataSize,
+  44, 0, 0, 0,  // RVA
+  // MemoryInfoListHeader
+  20, 0, 0, 0, 48, 0, 0, 0, // SizeOfHeader, SizeOfEntry
+  1, 0, 0, 0, 0, 0, 0, 0,   // NumberOfEntries
+  0, 0, 0, 0,   // ???
+  // MemoryInfo
+  0, 1, 2, 3, 4, 5, 6, 7,   // BaseAddress
+  8, 9, 0, 1, 2, 3, 4, 5,   // AllocationBase
+  16, 0, 0, 0, 6, 7, 8, 9,  // AllocationProtect, Reserved0
+  0, 1, 2, 3, 4, 5, 6, 7,   // RegionSize
+  0, 16, 0, 0, 32, 0, 0, 0, // State, Protect
+  0, 0, 2, 0, 8, 9, 0, 1,   // Type, Reserved1
+  };
+
+  // Same as before, but the entry is larger.
+  std::vector BiggerEntry{
+  // Header
+  'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+  1, 0, 0, 0,   // NumberOfStreams,
+  32, 0, 0, 0,  // StreamDirectoryRVA
+  0, 1, 2, 3, 4, 5, 6, 7,   // Checksum, TimeDateStamp
+  0, 0, 0, 0, 0, 0, 0, 0,   // Flags
+// Stream Directory
+  16, 0, 0, 0, 68, 0, 0, 0, // Type, DataSize,
+  44, 0, 0, 0,  // RVA
+  // MemoryInfoListHeader
+  16, 0, 0, 0, 52, 0, 0, 0, // SizeOfHeader, SizeOfEntry
+  1, 0, 0, 0, 0, 0, 0, 0,   // NumberOfEntries
+  // MemoryInfo
+  0, 1, 2, 3, 4, 5, 6, 7,   // BaseAddress
+  8, 9, 0, 1, 2, 3, 4, 5,   // AllocationBase
+  16, 0, 0, 0, 6, 7, 8, 9,  // AllocationProtect, Reserved0
+  0, 1, 2, 3, 4, 5, 6, 7,   // RegionSize
+  0, 16, 0, 0, 32, 0, 0, 0, // State, Protect
+  0, 0, 2, 0, 8, 9, 0, 1,   // Type, Reserved1
+  0, 0, 0, 0,   // ???
+  };
+
+  for (ArrayRef Data : {OneEntry, BiggerHeader, BiggerEntry}) {
+auto ExpectedFile = create(Data);
+ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+const MinidumpFile  = **ExpectedFile;
+auto ExpectedInfo = File.getMemoryInfoList();
+ASSERT_THAT_EXPECTED(ExpectedInfo, Succeeded());
+

[Lldb-commits] [PATCH] D68096: ProcessMinidump: inject SIGSTOP on Linux if no thread has a signal

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks for cleaning this up, and for sending the minidump. Looking at the dump, 
I am pretty sure that the problem is the lack of proper exception stream 
support in yaml2obj. The stream contains a reference to a thread context, and 
since yaml2obj does not understand this, it copies the offsets verbatim, and so 
they end up pointing to random garbage after the yaml round trip.

It doesn't look like it should be too hard to add yaml support for the 
exceptions stream -- it should only be a matter of adapting the patterns 
already used for other stream to work for this particular case. Could you give 
a go at that? Alternatively, you can wait a bit until I finish with the 
MemoryInfoList stream. After that, I can squeeze some time to do the Exception 
stream too.




Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:219
+
+  if (arch.GetTriple().isOSLinux())
+SetUnixSignals(UnixSignals::Create(GetArchitecture()));

I think you can create this unconditionally.



Comment at: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp:258-265
+uint32_t signo = m_active_exception->exception_record.exception_code;
+
+if (signo == 0) {
+  // Artifically inject a SIGSTOP so that we'll find a stop reason
+  // when we process the stop event.
+  signo = GetUnixSignals()->GetSignalNumberFromName("SIGSTOP");
+}

After some investigation, I re-learned about the DumpRequested constant, which 
is already checked for in this code (and it is tested 
(TestMiniDumpNew.py:test_snapshow_minidump) and works/does not hang). It seems 
to me like it would make sense to treat this case the same way as 
DumpRequested, and just don't return any stop info (instead of returning a stop 
info with signal 0 or SIGSTOP). IOW, you would just put `if (signo==0) return;` 
here. Eventually we could apply the same change to process elf core as well..

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68096



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

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 222387.
kwk marked an inline comment as done.
kwk added a comment.

- typo: dynmic -> dynamic
- Applied changes requested in review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390

Files:
  lldb/lit/Modules/ELF/Inputs/load-from-dynsym-alone.c
  lldb/lit/Modules/ELF/Inputs/load-symtab-and-dynsym.c
  lldb/lit/Modules/ELF/load-from-dynsym-alone.test
  lldb/lit/Modules/ELF/load-symtab-and-dynsym.test
  lldb/lit/helper/toolchain.py
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp
  lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -12,6 +12,7 @@
 #include 
 
 #include 
+#include 
 
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Utility/ArchSpec.h"
@@ -55,7 +56,7 @@
 /// This class provides a generic ELF (32/64 bit) reader plugin implementing
 /// the ObjectFile protocol.
 class ObjectFileELF : public lldb_private::ObjectFile {
-public:
+public:  
   // Static Functions
   static void Initialize();
 
@@ -184,6 +185,8 @@
 
   typedef std::map
   FileAddressToAddressClassMap;
+  
+  typedef std::unordered_set UniqueElfSymbolColl;
 
   /// Version of this reader common to all plugins based on this class.
   static const uint32_t m_plugin_version = 1;
@@ -278,7 +281,8 @@
   /// will parse the symbols only once.  Returns the number of symbols parsed.
   unsigned ParseSymbolTable(lldb_private::Symtab *symbol_table,
 lldb::user_id_t start_id,
-lldb_private::Section *symtab);
+lldb_private::Section *symtab,
+UniqueElfSymbolColl _elf_symbols);
 
   /// Helper routine for ParseSymbolTable().
   unsigned ParseSymbols(lldb_private::Symtab *symbol_table,
@@ -286,7 +290,8 @@
 lldb_private::SectionList *section_list,
 const size_t num_symbols,
 const lldb_private::DataExtractor _data,
-const lldb_private::DataExtractor _data);
+const lldb_private::DataExtractor _data,
+UniqueElfSymbolColl _elf_symbols);
 
   /// Scans the relocation entries and adds a set of artificial symbols to the
   /// given symbol table for each PLT slot.  Returns the number of symbols
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1871,7 +1871,8 @@
  SectionList *section_list,
  const size_t num_symbols,
  const DataExtractor _data,
- const DataExtractor _data) {
+ const DataExtractor _data,
+ UniqueElfSymbolColl _elf_symbols) {
   ELFSymbol symbol;
   lldb::offset_t offset = 0;
 
@@ -2196,20 +2197,27 @@
 symbol_size_valid,  // Symbol size is valid
 has_suffix, // Contains linker annotations?
 flags); // Symbol flags.
-symtab->AddSymbol(dc_symbol);
+
+NamedELFSymbol needle(symbol, ConstString(symbol_ref),
+  ConstString(symbol_section_sp->GetName()));
+if (unique_elf_symbols.insert(needle).second) {
+  symtab->AddSymbol(dc_symbol);
+}
   }
   return i;
 }
 
-unsigned ObjectFileELF::ParseSymbolTable(Symtab *symbol_table,
- user_id_t start_id,
- lldb_private::Section *symtab) {
+unsigned
+ObjectFileELF::ParseSymbolTable(Symtab *symbol_table, user_id_t start_id,
+lldb_private::Section *symtab,
+UniqueElfSymbolColl _elf_symbols) {
   if (symtab->GetObjectFile() != this) {
 // If the symbol table section is owned by a different object file, have it
 // do the parsing.
 ObjectFileELF *obj_file_elf =
 static_cast(symtab->GetObjectFile());
-return obj_file_elf->ParseSymbolTable(symbol_table, start_id, symtab);
+return obj_file_elf->ParseSymbolTable(symbol_table, start_id, symtab,
+  unique_elf_symbols);
   }
 
   // Get section list for this object file.
@@ -2237,7 +2245,7 @@
   size_t num_symbols = symtab_data.GetByteSize() / symtab_hdr->sh_entsize;
 
   return ParseSymbols(symbol_table, 

[Lldb-commits] [PATCH] D67390: [LLDB][ELF] Load both, .symtab and .dynsym sections

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked 8 inline comments as done.
kwk added a comment.

In D67390#1685484 , @labath wrote:

> This looks fairly ok to me, but it could use a more explicit test of the 
> symbol uniqueing code. Right, now I believe the two tests you added check 
> that the symbols are _not_ uniqued. (They're also a bit hard to follow due to 
> the whole
>
>   objcopy business). Could you create a test with a yaml file which will 
> contain various edge cases relevant to this code. I'm thinking of stuff like 
> "a dynsym and a symtab symbol at the same address, but a different name", "a 
> dynsym and symtab symbols with identical names, but different addresses", 
> etc. Then just run "image dump symtab" on that file to check what we have 
> parsed?


I'll give my best to implement this today.

> I am also surprised that you weren't able to just use a Section* (instead of 
> the name) for the uniqueing. I'd expect that all symbols (even those from the 
> separate object file) should be resolved to the sections in the main object. 
> I see that this isn't the case, but I am surprised that this isn't causing 
> any problems. Anyway, as things seem to be working as they are now, we can 
> leave that for another day.

Okay.

> In D67390#1685313 , @kwk wrote:
> 
>> Before I used the bare symbol name with stripped `@VERSION` suffix. Now I've 
>> changed the implementation of `NamedELFSymbol` to include the `@VERSION` 
>> suffix and the tests pass.
> 
> 
> Interesting. I'm pretty sure that the symbol count is irrelevant for that 
> test (it just wants to know if it is there), so we can change that if needed. 
> However, having the uniqueing include the `@VERSION` sounds right to me, so 
> if that makes the test happy too then, great.

Yes, I hoped so. Thank you. Please await another revision of this patch with 
the tests requested.




Comment at: lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp:371-373
+  r.st_name = st_name;
+  return elf::ELFSymbol::operator==(r) &&
+ st_name_string == rhs.st_name_string;

labath wrote:
> kwk wrote:
> > clayborg wrote:
> > > I would almost just manually compare only the things we care about here. 
> > > Again, what about st_shndx when comparing a symbol from the main symbol 
> > > table and one from the .gnu_debugdata symbol table. Are those section 
> > > indexes guaranteed to match? I would think not. 
> > @clayborg I explicitly only compare what we care about and therefore always 
> > set the name index to  be the same.
> I'll echo @clayborg here. This business with copying the ELFSymbol and 
> clearing some fields is confusing. Do you even need the ELFSymbol::operator== 
> for anything? If not I'd just delete that, and have the derived version 
> compare all fields.
> 
> Also, it would be nice if the operator== and hash function definitions were 
> next to each other. Can you just forward declare the std::hash stuff in the 
> header, and have the implementation be next to this?
> I'll echo @clayborg here. This business with copying the ELFSymbol and 
> clearing some fields is confusing.

I've cleared up the documentation now and it is exactly the way I like it. 
Every entity deals with it's own business (respects its own fields when 
comparing). I find it pretty dirty to compare fields from the base struct in a 
derived one. The way I compare fields from the base struct is minimally 
invasive.

> Do you even need the ELFSymbol::operator== for anything?

Yes, when you want to compare ELFSymbols. I know that I don't do that 
explicitly but I the function only deals with fields from the entity itself and 
they don't spill into any derived structure (with the exception of explicitly 
ignoring fields).

> If not I'd just delete that, and have the derived version compare all fields.

No because I call it explcitly from the derived NamedELFSymbol.

> Also, it would be nice if the operator== and hash function definitions were 
> next to each other. Can you just forward declare the std::hash stuff in the 
> header, and have the implementation be next to this?

I've found a compromise that is even more appealing to me. The ELFSymbol and 
NamedELFSymbol structs now have a `hash` function which contains the 
implementation next to the one of `operator==()`. This `hash` is called in the 
specialization which remains in the same location as before; the reason being 
that I didn't find a way do define something in the `std::` namespace when I'm 
in the `elf::` namespace.





Comment at: lldb/source/Plugins/ObjectFile/ELF/ELFHeader.h:446
+std::size_t h2 = std::hash()(s.st_name_string.AsCString());
+std::size_t h3 = std::hash()(s.st_section_name_string.AsCString());
+return llvm::hash_combine(h1, h2, h3);

jankratochvil wrote:
> I find better to rather define `std::hash` (or provide 
> `ConstString::Hasher` which I do in my DWZ patchset).

[Lldb-commits] [PATCH] D68140: [lldb][clang][modern-type-lookup] Use ASTImporterSharedState in ExternalASTMerger

[Raphael Isemann via Phabricator via lldb-commits]

teemperor marked 2 inline comments as done.
teemperor added inline comments.



Comment at: clang/include/clang/AST/ExternalASTMerger.h:92
   ImporterTarget Target;
+  std::shared_ptr SharedState;
 

shafik wrote:
> Can you add a comment explaining what this is and why we need it and how it 
> relates to the `ASTImpoter`.
> 
> It is not obvious just looking the local changes what effect adding this has.
Added a comment how we used the shared state in all created ASTImporters, but 
the exact effects of having the shared state are explained in the class itself 
(e.g. helping the lookup in some cases).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68140



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

[Lldb-commits] [lldb] r373194 - [lldb] Partly revert 370734: Test 'frame select -r' and fix that INT32_MIN breaks the option parser

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Mon Sep 30 02:00:23 2019
New Revision: 373194

URL: http://llvm.org/viewvc/llvm-project?rev=373194=rev
Log:
[lldb] Partly revert 370734: Test 'frame select -r' and fix that INT32_MIN 
breaks the option parser

This somehow caused that 'frame select X' ends up being interpreted as 'frame 
select -r 1' when 'up' or 'down'
were run before 'frame select X'. See rdar://55791276.
Partly reverting to unbreak master. The changes that aren't reverted are the 
generic 'frame select -r' tests
that are obviously NFC and test existing behavior.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
lldb/trunk/source/Commands/CommandObjectFrame.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py?rev=373194=373193=373194=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/frame/select/TestFrameSelect.py
 Mon Sep 30 02:00:23 2019
@@ -23,16 +23,12 @@ class TestFrameSelect(TestBase):
 
 self.expect("frame select -r -1", error=True, substrs=["Already at the 
bottom of the stack."])
 self.expect("frame select -r -2147483647", error=True, 
substrs=["Already at the bottom of the stack."])
-self.expect("frame select -r -2147483648", error=True, 
substrs=["error: invalid frame offset argument '-2147483648'"])
-self.expect("frame select -r -2147483649", error=True, 
substrs=["error: invalid frame offset argument '-2147483649'"])
 
 self.expect("frame select -r 1", substrs=["nested2() at"])
 self.expect("frame select -r -2", substrs=["nested3() at"])
 self.expect("frame select -r 1", substrs=["nested2() at"])
 self.expect("frame select -r -2147483647", substrs=["nested3() at"])
 self.expect("frame select -r 1", substrs=["nested2() at"])
-self.expect("frame select -r -2147483648", error=True, 
substrs=["error: invalid frame offset argument '-2147483648'"])
-self.expect("frame select -r -2147483649", error=True, 
substrs=["error: invalid frame offset argument '-2147483649'"])
 
 self.expect("frame select -r 100")
 self.expect("frame select -r 1", error=True, substrs=["Already at the 
top of the stack."])

Modified: lldb/trunk/source/Commands/CommandObjectFrame.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectFrame.cpp?rev=373194=373193=373194=diff
==
--- lldb/trunk/source/Commands/CommandObjectFrame.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectFrame.cpp Mon Sep 30 02:00:23 2019
@@ -246,15 +246,13 @@ public:
   Status error;
   const int short_option = m_getopt_table[option_idx].val;
   switch (short_option) {
-case 'r': {
-int32_t offset = 0;
-if (option_arg.getAsInteger(0, offset) || offset == INT32_MIN) {
+  case 'r':
+if (option_arg.getAsInteger(0, relative_frame_offset)) {
+  relative_frame_offset = INT32_MIN;
   error.SetErrorStringWithFormat("invalid frame offset argument '%s'",
  option_arg.str().c_str());
-} else
-  relative_frame_offset = offset;
+}
 break;
-  }
 
   default:
 llvm_unreachable("Unimplemented option");
@@ -263,13 +261,15 @@ public:
   return error;
 }
 
-void OptionParsingStarting(ExecutionContext *execution_context) override {}
-
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+  relative_frame_offset = INT32_MIN;
+}
+
 llvm::ArrayRef GetDefinitions() override {
   return llvm::makeArrayRef(g_frame_select_options);
 }
 
-llvm::Optional relative_frame_offset;
+int32_t relative_frame_offset;
   };
 
   CommandObjectFrameSelect(CommandInterpreter )
@@ -307,15 +307,15 @@ protected:
 Thread *thread = m_exe_ctx.GetThreadPtr();
 
 uint32_t frame_idx = UINT32_MAX;
-if (m_options.relative_frame_offset.hasValue()) {
+if (m_options.relative_frame_offset != INT32_MIN) {
   // The one and only argument is a signed relative frame index
   frame_idx = thread->GetSelectedFrameIndex();
   if (frame_idx == UINT32_MAX)
 frame_idx = 0;
 
-  if (*m_options.relative_frame_offset < 0) {
-if (static_cast(frame_idx) >= 
-*m_options.relative_frame_offset)
-  frame_idx += *m_options.relative_frame_offset;
+  if (m_options.relative_frame_offset < 0) {
+if (static_cast(frame_idx) >= 
-m_options.relative_frame_offset)
+  frame_idx += m_options.relative_frame_offset;
 else {
   

[Lldb-commits] [PATCH] D68140: [lldb][clang][modern-type-lookup] Use ASTImporterSharedState in ExternalASTMerger

[Raphael Isemann via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373193: [lldb][clang][modern-type-lookup] Use 
ASTImporterSharedState in… (authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68140?vs=222167=222374#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68140

Files:
  cfe/trunk/include/clang/AST/ExternalASTMerger.h
  cfe/trunk/lib/AST/ExternalASTMerger.cpp
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp

Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+USE_LIBCPP := 1
+include Makefile.rules
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
@@ -0,0 +1,6 @@
+#include 
+
+int main(int argc, char **argv) {
+  std::pair pair = std::make_pair(1, 2L);
+  return pair.first; // Set break point at this line.
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
@@ -0,0 +1,20 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxModernTypeLookup(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(["libc++"])
+def test(self):
+self.build()
+
+# Activate modern-type-lookup.
+self.runCmd("settings set target.experimental.use-modern-type-lookup true")
+
+lldbutil.run_to_source_breakpoint(self,
+"// Set break point at this line.", lldb.SBFileSpec("main.cpp"))
+
+# Test a few simple expressions that should still work with modern-type-lookup.
+self.expect("expr pair", substrs=["(std::", "pair SharedState;
 
 public:
   ExternalASTMerger(const ImporterTarget ,
Index: cfe/trunk/lib/AST/ExternalASTMerger.cpp
===
--- cfe/trunk/lib/AST/ExternalASTMerger.cpp
+++ cfe/trunk/lib/AST/ExternalASTMerger.cpp
@@ -107,11 +107,13 @@
   LazyASTImporter(ExternalASTMerger &_Parent, ASTContext ,
   FileManager , ASTContext ,
   FileManager ,
-  const ExternalASTMerger::OriginMap &_FromOrigins)
+  const ExternalASTMerger::OriginMap &_FromOrigins,
+  std::shared_ptr SharedState)
   : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager,
-/*MinimalImport=*/true),
+/*MinimalImport=*/true, SharedState),
 Parent(_Parent), Reverse(FromContext, FromFileManager, ToContext,
- ToFileManager, /*MinimalImport=*/true), FromOrigins(_FromOrigins) {}
+ ToFileManager, /*MinimalImport=*/true),
+FromOrigins(_FromOrigins) {}
 
   /// Whenever a DeclContext is imported, ensure that ExternalASTSource's origin
   /// map is kept up to date.  Also set the appropriate flags.
@@ -314,6 +316,8 @@
 
 ExternalASTMerger::ExternalASTMerger(const ImporterTarget ,
  llvm::ArrayRef Sources) : LogStream(::nulls()), Target(Target) {
+  SharedState = std::make_shared(
+  *Target.AST.getTranslationUnitDecl());
   AddSources(Sources);
 }
 
@@ -321,7 +325,7 @@
   for (const ImporterSource  : Sources) {
 assert( != );
 Importers.push_back(std::make_unique(
-*this, Target.AST, Target.FM, S.AST, S.FM, S.OM));
+*this, Target.AST, Target.FM, S.AST, S.FM, S.OM, SharedState));
   }
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [lldb] r373193 - [lldb][clang][modern-type-lookup] Use ASTImporterSharedState in ExternalASTMerger

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Mon Sep 30 01:52:16 2019
New Revision: 373193

URL: http://llvm.org/viewvc/llvm-project?rev=373193=rev
Log:
[lldb][clang][modern-type-lookup] Use ASTImporterSharedState in 
ExternalASTMerger

Summary:
The ExternalASTMerger should use the ASTImporterSharedState. This allows it to
handle std::pair in LLDB (but the rest of libc++ is still work in progress).

Reviewers: martong, shafik, a.sidorin

Subscribers: rnkovacs, christof, JDevlieghere, lldb-commits

Tags: #lldb

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

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/

lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile

lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile?rev=373193=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/Makefile
 Mon Sep 30 01:52:16 2019
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+USE_LIBCPP := 1
+include Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py?rev=373193=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
 Mon Sep 30 01:52:16 2019
@@ -0,0 +1,20 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxModernTypeLookup(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(["libc++"])
+def test(self):
+self.build()
+
+# Activate modern-type-lookup.
+self.runCmd("settings set target.experimental.use-modern-type-lookup 
true")
+
+lldbutil.run_to_source_breakpoint(self,
+"// Set break point at this line.", lldb.SBFileSpec("main.cpp"))
+
+# Test a few simple expressions that should still work with 
modern-type-lookup.
+self.expect("expr pair", substrs=["(std::", "pairhttp://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp?rev=373193=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
 Mon Sep 30 01:52:16 2019
@@ -0,0 +1,6 @@
+#include 
+
+int main(int argc, char **argv) {
+  std::pair pair = std::make_pair(1, 2L);
+  return pair.first; // Set break point at this line.
+}


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

[Lldb-commits] [PATCH] D68188: allow arbitrary python streams to be converted to SBFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I haven't looked at the patch in detail, but I have one high-level question. It 
seems to be that instead of subclassing a fully-functional File class, it would 
be better to create an abstract class, that both `File` and the new python 
thingy could implement. That would also create a natural place to write down 
the FILE* conversion semantics and other stuff we talked about at lldb-dev. I 
don't have any opinion as to whether the name `File` should denote the new 
abstract class, or the real thing, but it should most likely be done as a 
separate patch.

What do you think about that?




Comment at: lldb/include/lldb/API/SBFile.h:16-21
+/* These tags make no difference at the c++ level, but
+ * when the constructors are called from python they control
+ * how python files are converted by SWIG into FileSP */
+struct FileBorrow {};
+struct FileForceScriptingIO {};
+struct FileBorrowAndForceScriptingIO {};

I don't think we have anything similar to this in any of our SB classes. It 
might be better to avoid it. Could the same thing be achieved e.g. with a 
static factory function (`static SBFile SBFile::Create(FileSP file_sp, bool 
borrow, bool force_scripting_io)`) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188



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

[Lldb-commits] [PATCH] D68134: [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api

[Martin Storsjö via Phabricator via lldb-commits]

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

In D68134#1687500 , @labath wrote:

> In D68134#1687031 , @mstorsjo wrote:
>
> > In D68134#1686970 , @thakis wrote:
> >
> > > We can add flags for omitting access specifiers etc if it's critical for 
> > > lldb. Or maybe we can just change the test that caused the revert.
> >
> >
> > Yeah I doubt it's critical to maintain the exact same form as before, but I 
> > need to get the tests running in my cross compile setup to verify exactly 
> > how to update them.
>
>
> I'm not sure what failed here exactly, but there are some places in lldb that 
> parse the demangled names. These might get confused by additional things 
> appearing in the name. Though it's possible to also fix that, so the main 
> question might be: what is the name we want to display to the users? I guess 
> it would be the best if this matched what is displayed by other tools ?


In this case, the output becomes the same as what has been chosen to be used by 
the common llvm demangler, which probably should be a good choice in general. 
In this case, a few more identifiers are added. Or I guess it can be discussed 
if that one should be changed (or add options to it, as @thakis said was 
possible).

> As for tests, you should at least be able to run the tests in the regular 
> "host" setup, right ?

In principle, but I normally don't run windows except in a very underpowered VM 
for testing things, and building there is no fun. I can spin up some more 
powerful VMs for some better testing though.

In this case, much of the functionality of these tests require having the MS 
DIA SDK available (which also implies building in MSVC/clang-cl mode, not mingw 
mode), for reading PDB files. Probably just another step to do, but I don't 
have it set up right now.


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

https://reviews.llvm.org/D68134



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

[Lldb-commits] [PATCH] D68181: SBDebugger::SetInputFile, SetOutputFile, etc.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks mostly fine to me. Just a couple of questions inline...




Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:243-244
+status = debugger.SetOutputFile(outsbf)
+if status.Fail():
+raise Exception(status)
+

`self.assertTrue(status.Success())`



Comment at: lldb/source/API/SBDebugger.cpp:296-313
   if (!m_opaque_sp)
 return;
 
   repro::DataRecorder *recorder = nullptr;
   if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator())
 recorder = g->GetOrCreate().GetNewDataRecorder();
 

Any chance we could make this method simply delegate to the SBFile version ?



Comment at: lldb/source/Core/Debugger.cpp:828-833
+  if (!file_sp || !file_sp->IsValid()) {
 m_input_file_sp = std::make_shared(stdin, false);
+error.SetErrorString("invalid file");
+  } else {
+m_input_file_sp = file_sp;
+  }

Could we make it so that the validity of the file_sp argument is checked at the 
SB level? That way you wouldn't need the Status result here. Same goes for 
stdout and stderr methods.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68181



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

[Lldb-commits] [PATCH] D68134: [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo updated this revision to Diff 222358.
mstorsjo edited the summary of this revision.
mstorsjo added a reviewer: thakis.
mstorsjo added a comment.

I tried to update the testcases based on the log output 
(http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/9306/steps/test/logs/stdio).

Even after fudging the tests to somehow run on windows, both of the tests that 
failed seem to require the DIA SDK which I don't have readily available (for 
reading PDB files; there's some native support for PDB files but I'm not sure 
what the state of it is and running those tests with it).


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

https://reviews.llvm.org/D68134

Files:
  lldb/lit/SymbolFile/PDB/udt-layout.test
  lldb/source/Core/Mangled.cpp
  lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Index: lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -615,6 +615,6 @@
 
   SymbolContext sc;
   EXPECT_TRUE(sc_list.GetContextAtIndex(0, sc));
-  EXPECT_STREQ("int foo(int)",
+  EXPECT_STREQ("int __cdecl foo(int)",
sc.GetFunctionName(Mangled::ePreferDemangled).AsCString());
 }
Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -8,13 +8,6 @@
 
 #include "lldb/Core/Mangled.h"
 
-#if defined(_WIN32)
-#include "lldb/Host/windows/windows.h"
-
-#include 
-#pragma comment(lib, "dbghelp.lib")
-#endif
-
 #include "lldb/Core/RichManglingContext.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
@@ -39,25 +32,6 @@
 #include 
 using namespace lldb_private;
 
-#if defined(_MSC_VER)
-static DWORD safeUndecorateName(const char *Mangled, char *Demangled,
-DWORD DemangledLength) {
-  static std::mutex M;
-  std::lock_guard Lock(M);
-  return ::UnDecorateSymbolName(
-  Mangled, Demangled, DemangledLength,
-  UNDNAME_NO_ACCESS_SPECIFIERS |   // Strip public, private, protected
-   // keywords
-  UNDNAME_NO_ALLOCATION_LANGUAGE | // Strip __thiscall, __stdcall,
-   // etc keywords
-  UNDNAME_NO_THROW_SIGNATURES |// Strip throw() specifications
-  UNDNAME_NO_MEMBER_TYPE | // Strip virtual, static, etc
-   // specifiers
-  UNDNAME_NO_MS_KEYWORDS   // Strip all MS extension keywords
-  );
-}
-#endif
-
 static inline Mangled::ManglingScheme cstring_mangling_scheme(const char *s) {
   if (s) {
 if (s[0] == '?')
@@ -218,28 +192,16 @@
 
 // Local helpers for different demangling implementations.
 static char *GetMSVCDemangledStr(const char *M) {
-#if defined(_MSC_VER)
-  const size_t demangled_length = 2048;
-  char *demangled_cstr = static_cast(::malloc(demangled_length));
-  ::ZeroMemory(demangled_cstr, demangled_length);
-  DWORD result = safeUndecorateName(M, demangled_cstr, demangled_length);
+  char *demangled_cstr = llvm::microsoftDemangle(M, nullptr, nullptr, nullptr);
 
   if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) {
 if (demangled_cstr && demangled_cstr[0])
   LLDB_LOGF(log, "demangled msvc: %s -> \"%s\"", M, demangled_cstr);
 else
-  LLDB_LOGF(log, "demangled msvc: %s -> error: 0x%lu", M, result);
+  LLDB_LOGF(log, "demangled msvc: %s -> error", M);
   }
 
-  if (result != 0) {
-return demangled_cstr;
-  } else {
-::free(demangled_cstr);
-return nullptr;
-  }
-#else
-  return nullptr;
-#endif
+  return demangled_cstr;
 }
 
 static char *GetItaniumDemangledStr(const char *M) {
Index: lldb/lit/SymbolFile/PDB/udt-layout.test
===
--- lldb/lit/SymbolFile/PDB/udt-layout.test
+++ lldb/lit/SymbolFile/PDB/udt-layout.test
@@ -2,7 +2,7 @@
 RUN: %build --compiler=clang-cl --output=%t.exe %S/Inputs/UdtLayoutTest.cpp
 RUN: %lldb -b -s %S/Inputs/UdtLayoutTest.script -- %t.exe | FileCheck %s
 
-CHECK:(int) int C::abc = 123
+CHECK:(int) public: static int C::abc = 123
 CHECK:(List [16]) ls = {
 CHECK:  [15] = {
 CHECK:Prev = 0x
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68134: [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68134#1687031 , @mstorsjo wrote:

> In D68134#1686970 , @thakis wrote:
>
> > We can add flags for omitting access specifiers etc if it's critical for 
> > lldb. Or maybe we can just change the test that caused the revert.
>
>
> Yeah I doubt it's critical to maintain the exact same form as before, but I 
> need to get the tests running in my cross compile setup to verify exactly how 
> to update them.


I'm not sure what failed here exactly, but there are some places in lldb that 
parse the demangled names. These might get confused by additional things 
appearing in the name. Though it's possible to also fix that, so the main 
question might be: what is the name we want to display to the users? I guess it 
would be the best if this matched what is displayed by other tools ?

As for tests, you should at least be able to run the tests in the regular 
"host" setup, right ?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68134



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

[Lldb-commits] [PATCH] D68134: [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

(See CPlusPlusLanguage::MethodName for one such parsing instance.)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68134



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

[Lldb-commits] [PATCH] D67793: new api class: SBFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks. This looks good to me, but given that this is going to become a stable 
api that well need to maintain for a looong time, I'd like to get signoff from 
someone else as well. @jingham maybe ?




Comment at: lldb/include/lldb/API/SBDefines.h:95
 class LLDB_API SBUnixSignals;
+class LLDB_API SBFile;
 

Sort this list ?



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:27
+
+def handle_command(debugger, cmd, raise_on_fail=True, collect_result=True):
+

Please add a comment to explain why is this needed (I guess that has something 
to do with needing more control over the command output stream), because 
superficially it looks like you're reimplementing `TestBase.runCmd` here.



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:37-38
+
+if collect_result and raise_on_fail and not ret.Succeeded():
+raise Exception
+

Maybe instead of `raise_on_fail` this should be called `check` (in analogy to 
runCmd function), and instead of raising an exception you could assert that 
ret.Succeeded() is true?



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:52
+try:
+with open('output', 'w') as f:
+debugger.SetOutputFileHandle(f, False)

I believe all of these will end up writing to the source tree. Could you try 
replacing that with `self.getBuildArtifact("output")` ?



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:127-132
+
+
+
+
+
+

delete



Comment at: lldb/scripts/interface/SBFile.i:36
+
+bool IsValid() const;
+

add operator bool here. I'm not sure if swig handles operator!, but if it does, 
then we should add that too.



Comment at: lldb/source/API/SBFile.cpp:1
+//===-- SBFile.cpp --*- C++ -*-===//
+//

I think you'll need to add the reproducer boilerplate to this file. I think it 
should be possible to do that automatically via lldb-instr, but I've never done 
that. @JDevlieghere should be able to help here.



Comment at: lldb/source/Host/common/File.cpp:72-74
+  if (mode.empty())
+return 0;
+  return llvm::StringSwitch(mode.str())

I know you're just copying, but could you remove the `.str()` call the 
`.empty() check, as neither of them are really needed here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793



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

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68179#1686893 , @davide wrote:

> This needs a test. You can either write a python one as the ones in 
> `test/testcases` or a lit-style based as the ones in `lit/`.


Another option might be a c++ unit test for the JSON class (see 
`unittests/Utility/JSONTest.cpp`).


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

https://reviews.llvm.org/D68179



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

[Lldb-commits] [PATCH] D68171: Remove unused "append" parameter from FindTypes API

[Pavel Labath via Phabricator via lldb-commits]

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

Nuke it from orbit. It's the only way to be sure. I'd also consider removing 
the size_t return value. If some user really needs that, he can always 
implement the counting himself.


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

https://reviews.llvm.org/D68171



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

[Lldb-commits] [PATCH] D68069: Explicitly set entry point arch when it's thumb

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68069#1686199 , @aadsm wrote:

> >   Not everyone has both of these things enabled (particularly having lld is 
> > less common), but they are things that one _can_ enable no matter what is 
> > his host or target architecture, so I am not worried by that. And as tests 
> > like these become more common, I expect more people will start having these 
> > enabled by default.
>
> Will the tests running on the build bots have lld, arm enabled?


All of the current "buildbot" bots have it. We even have an "experimental" arm 
buildbot running on arm hardware, but that one is red all the time :(. 
"Greendragon" don't have lld iirc, but one day I might try to convince them to 
enable it too. :)

In D68069#1686711 , @aadsm wrote:

> >   (substituting llvm-mc for as and ld.lld for linker).
>
> I was not able to figure out how to generate the object file with llvm-mc 
> correctly. I've tried a few variations of the triple (e.g.: `armv7-eabi`) but 
> I never end up with the same code that I have in the assembly: `# RUN: 
> llvm-mc -triple=thumbv7 %s -filetype=obj -o %t.o`. Any idea here?


Changing the input to:

  movs r0, #42
  movs r7, #1
  svc #0

seems to do the trick to me (this how objdump disassembles the gnu as produced 
binary). I guess that's because "mov" does not name a specific instruction and 
as and llvm-mc choose to interpret it differently. I think a case could be made 
for changing llvm-mc to pick the other (shorter) form by default, but that's 
probably not relevant now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68069



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

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68130#1686184 , @vsk wrote:

> Great find. The changes in this patch makes sense to me locally, but I'm 
> having trouble picking up on the context necessary to confidently 'lgtm'. + 
> @JDevlieghere & @labath to get some more experienced people.


I am not very familiar with this code either, but it seems fairly safe and 
straightforward to me...

> I'd love to see the big switch in ParseTypeFromDWARF broken up into small, 
> well-commented functions bit-by-bit -- when that's done, I think I'll have a 
> much better chance at reviewing changes. If folks agree that that's a 
> reasonable refactor, I'd be happy to send a few patches (perhaps starting 
> with the DW_TAG_subroutine_type handling).

+100 to that. I've tried to reduce the function down a bit by splitting the 
dwarf parsing stuff into the separate `ParsedTypeAttributes` class, but the 
amount of code left is still far too much for a single function. If it's 
getting in the way, feel free to undo/refactor the attribute parsing code...


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

https://reviews.llvm.org/D68130



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

[Lldb-commits] [PATCH] D68160: File::Clear() -> File::TakeStreamAndClear()

[Pavel Labath via Phabricator via lldb-commits]

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

Sounds good. If this were a more permanent interface, I'd argue that the 
function should be called `TakeStream` (in analogy with 
`llvm::Expected::takeError`), but for a temporary thing that does not matter...




Comment at: lldb/source/Host/common/File.cpp:166-167
+FILE *File::TakeStreamAndClear() {
+  GetStream();
+  FILE *stream = m_stream;
+  m_stream = NULL;

`FILE *stream = GetStream()` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68160



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