[Lldb-commits] [PATCH] D68444: remove a smattering of isolated, unnecessary uses of FILE*

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

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.

There a a few call sites that use FILE* which are easy to 
fix without disrupting anything else.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68444

Files:
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Expression/REPL.cpp


Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -423,7 +423,7 @@
   .SetBaseLineNumber(m_code.GetSize() + 1);
 }
 if (extra_line) {
-  fprintf(output_sp->GetFile().GetStream(), "\n");
+  output_sp->GetFile().Printf("\n");
 }
   }
 }
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -328,10 +328,9 @@
   prompt = GetPrompt();
 
 if (prompt && prompt[0]) {
-  FILE *out = GetOutputFILE();
-  if (out) {
-::fprintf(out, "%s", prompt);
-::fflush(out);
+  if (m_output_sp) {
+m_output_sp->GetFile().Printf("%s", prompt);
+m_output_sp->Flush();
   }
 }
   }
@@ -490,10 +489,11 @@
   // Show line numbers if we are asked to
   std::string line;
   if (m_base_line_number > 0 && GetIsInteractive()) {
-FILE *out = GetOutputFILE();
-if (out)
-  ::fprintf(out, "%u%s", m_base_line_number + 
(uint32_t)lines.GetSize(),
-GetPrompt() == nullptr ? " " : "");
+if (m_output_sp) {
+  m_output_sp->GetFile().Printf(
+  "%u%s", m_base_line_number + (uint32_t)lines.GetSize(),
+  GetPrompt() == nullptr ? " " : "");
+}
   }
 
   m_curr_line_idx = lines.GetSize();
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -467,10 +467,8 @@
 
 sb_interpreter.HandleCommand(command, result, false);
 
-if (GetErrorFileHandle() != nullptr)
-  result.PutError(GetErrorFile());
-if (GetOutputFileHandle() != nullptr)
-  result.PutOutput(GetOutputFile());
+result.PutError(m_opaque_sp->GetErrorStream().GetFileSP());
+result.PutOutput(m_opaque_sp->GetOutputStream().GetFileSP());
 
 if (!m_opaque_sp->GetAsyncExecution()) {
   SBProcess process(GetCommandInterpreter().GetProcess());


Index: lldb/source/Expression/REPL.cpp
===
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -423,7 +423,7 @@
   .SetBaseLineNumber(m_code.GetSize() + 1);
 }
 if (extra_line) {
-  fprintf(output_sp->GetFile().GetStream(), "\n");
+  output_sp->GetFile().Printf("\n");
 }
   }
 }
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -328,10 +328,9 @@
   prompt = GetPrompt();
 
 if (prompt && prompt[0]) {
-  FILE *out = GetOutputFILE();
-  if (out) {
-::fprintf(out, "%s", prompt);
-::fflush(out);
+  if (m_output_sp) {
+m_output_sp->GetFile().Printf("%s", prompt);
+m_output_sp->Flush();
   }
 }
   }
@@ -490,10 +489,11 @@
   // Show line numbers if we are asked to
   std::string line;
   if (m_base_line_number > 0 && GetIsInteractive()) {
-FILE *out = GetOutputFILE();
-if (out)
-  ::fprintf(out, "%u%s", m_base_line_number + (uint32_t)lines.GetSize(),
-GetPrompt() == nullptr ? " " : "");
+if (m_output_sp) {
+  m_output_sp->GetFile().Printf(
+  "%u%s", m_base_line_number + (uint32_t)lines.GetSize(),
+  GetPrompt() == nullptr ? " " : "");
+}
   }
 
   m_curr_line_idx = lines.GetSize();
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -467,10 +467,8 @@
 
 sb_interpreter.HandleCommand(command, result, false);
 
-if (GetErrorFileHandle() != nullptr)
-  result.PutError(GetErrorFile());
-if (GetOutputFileHandle() != nullptr)
-  result.PutOutput(GetOutputFile());
+result.PutError(m_opaque_sp->GetErrorStream().GetFileSP());
+result.PutOutput(m_opaque_sp->GetOutputStream().GetFileSP());
 
 if (!m_opaque_sp->GetAsyncExecution()) {
   SBProcess process(GetCommandInterpreter().GetProcess());
___
lldb-commits 

[Lldb-commits] [PATCH] D68433: SBFile: add a bunch of tests that should eventually work.

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

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

cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68433

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig

Index: lldb/scripts/Python/python-typemaps.swig
===
--- lldb/scripts/Python/python-typemaps.swig
+++ lldb/scripts/Python/python-typemaps.swig
@@ -440,19 +440,19 @@
   $1 = nullptr;
else if (!lldb_private::PythonFile::Check($input)) {
   int fd = PyObject_AsFileDescriptor($input);
+  if (fd < 0 || PyErr_Occurred())
+return nullptr;
   PythonObject py_input(PyRefType::Borrowed, $input);
   PythonString py_mode = py_input.GetAttributeValue("mode").AsType();
-
-  if (-1 != fd && py_mode.IsValid()) {
- FILE *f;
- if ((f = fdopen(fd, py_mode.GetString().str().c_str(
-$1 = f;
- else
-PyErr_SetString(PyExc_TypeError, strerror(errno));
-  } else {
- PyErr_SetString(PyExc_TypeError,"not a file-like object");
- return nullptr;
-  }
+  if (!py_mode.IsValid() || PyErr_Occurred())
+return nullptr;
+FILE *f;
+if ((f = fdopen(fd, py_mode.GetString().str().c_str(
+  $1 = f;
+else {
+  PyErr_SetString(PyExc_TypeError, strerror(errno));
+  return nullptr;
+}
}
else
{
Index: lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -180,6 +180,19 @@
 self.assertIn("is not a valid command", f.read())
 
 
+@add_test_categories(['pyapi'])
+def test_legacy_file_error(self):
+debugger = self.debugger
+with open(self.out_filename, 'w') as f:
+debugger.SetErrorFileHandle(f, False)
+self.handleCmd('lolwut', check=False, collect_result=False)
+debugger.GetErrorFileHandle().write('FOOBAR\n')
+with open(self.out_filename, 'r') as f:
+errors = f.read()
+self.assertTrue(re.search(r'error:.*lolwut', errors))
+self.assertTrue(re.search(r'FOOBAR', errors))
+
+
 @add_test_categories(['pyapi'])
 def test_sbfile_type_errors(self):
 sbf = lldb.SBFile()
@@ -269,6 +282,17 @@
 self.assertTrue(re.search(r'Show a list of all debugger commands', f.read()))
 
 
+@add_test_categories(['pyapi'])
+def test_help(self):
+debugger = self.debugger
+with open(self.out_filename, 'w') as f:
+status = debugger.SetOutputFile(lldb.SBFile(f))
+self.assertTrue(status.Success())
+self.handleCmd("help help", check=False, collect_result=False)
+with open(self.out_filename, 'r') as f:
+self.assertIn('Show a list of all debugger commands', f.read())
+
+
 @add_test_categories(['pyapi'])
 def test_immediate(self):
 with open(self.out_filename, 'w') as f:
@@ -278,15 +302,44 @@
 interpreter.HandleCommand("help help", ret)
 # make sure the file wasn't closed early.
 f.write("\nQUUX\n")
-
 ret = None # call destructor and flush streams
-
 with open(self.out_filename, 'r') as f:
 output = f.read()
 self.assertTrue(re.search(r'Show a list of all debugger commands', output))
 self.assertTrue(re.search(r'QUUX', output))
 
 
+@add_test_categories(['pyapi'])
+@skipIf(True) # FIXME need SBFile interfaces on SBCommandReturnObject
+def test_immediate_string(self):
+f = io.StringIO()
+ret = lldb.SBCommandReturnObject()
+ret.SetImmediateOutputFile(f)
+interpreter = self.debugger.GetCommandInterpreter()
+interpreter.HandleCommand("help help", ret)
+# make sure the file wasn't closed early.
+f.write("\nQUUX\n")
+ret = None # call destructor and flush streams
+output = f.getvalue()
+self.assertTrue(re.search(r'Show a list of all debugger commands', output))
+self.assertTrue(re.search(r'QUUX', output))
+
+
+@add_test_categories(['pyapi'])
+@skipIf(True) # FIXME need SBFile interfaces on SBCommandReturnObject
+def test_immediate_sbfile_string(self):
+f = io.StringIO()
+ret = lldb.SBCommandReturnObject()
+ret.SetImmediateOutputFile(lldb.SBFile(f))
+interpreter = self.debugger.GetCommandInterpreter()
+interpreter.HandleCommand("help help", ret)
+output = f.getvalue()
+ret = None # call destructor and flush streams
+# sbfile default 

[Lldb-commits] [PATCH] D68434: SBFile support in SBCommandReturnObject

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

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

cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68434

Files:
  lldb/include/lldb/API/SBCommandReturnObject.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/interface/SBCommandReturnObject.i
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -11,6 +11,7 @@
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBHostOS.h"
 #include "lldb/API/SBLanguageRuntime.h"
 #include "lldb/API/SBReproducer.h"
@@ -499,16 +500,16 @@
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result);
   if (m_option_data.m_debug_mode) {
-result.PutError(m_debugger.GetErrorFileHandle());
-result.PutOutput(m_debugger.GetOutputFileHandle());
+result.PutError(m_debugger.GetErrorFile());
+result.PutOutput(m_debugger.GetOutputFile());
   }
 
   // Source the local .lldbinit file if it exists and we're allowed to source.
   // Here we want to always print the return object because it contains the
   // warning and instructions to load local lldbinit files.
   sb_interpreter.SourceInitFileInCurrentWorkingDirectory(result);
-  result.PutError(m_debugger.GetErrorFileHandle());
-  result.PutOutput(m_debugger.GetOutputFileHandle());
+  result.PutError(m_debugger.GetErrorFile());
+  result.PutOutput(m_debugger.GetOutputFile());
 
   // We allow the user to specify an exit code when calling quit which we will
   // return when exiting.
@@ -574,8 +575,8 @@
   }
 
   if (m_option_data.m_debug_mode) {
-result.PutError(m_debugger.GetErrorFileHandle());
-result.PutOutput(m_debugger.GetOutputFileHandle());
+result.PutError(m_debugger.GetErrorFile());
+result.PutOutput(m_debugger.GetOutputFile());
   }
 
   const bool handle_events = true;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -893,8 +893,9 @@
   ::setbuf(outfile_handle, nullptr);
 
 result->SetImmediateOutputFile(
-debugger.GetOutputFile().GetStream());
-result->SetImmediateErrorFile(debugger.GetErrorFile().GetStream());
+debugger.GetOutputStream().GetFileSP());
+result->SetImmediateErrorFile(
+debugger.GetErrorStream().GetFileSP());
   }
 }
   }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -468,9 +468,9 @@
 sb_interpreter.HandleCommand(command, result, false);
 
 if (GetErrorFileHandle() != nullptr)
-  result.PutError(GetErrorFileHandle());
+  result.PutError(GetErrorFile());
 if (GetOutputFileHandle() != nullptr)
-  result.PutOutput(GetOutputFileHandle());
+  result.PutOutput(GetOutputFile());
 
 if (!m_opaque_sp->GetAsyncExecution()) {
   SBProcess process(GetCommandInterpreter().GetProcess());
Index: lldb/source/API/SBCommandReturnObject.cpp
===
--- lldb/source/API/SBCommandReturnObject.cpp
+++ lldb/source/API/SBCommandReturnObject.cpp
@@ -10,6 +10,7 @@
 #include "SBReproducerPrivate.h"
 #include "Utils.h"
 #include "lldb/API/SBError.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Utility/ConstString.h"
@@ -98,7 +99,6 @@
 
 size_t SBCommandReturnObject::PutOutput(FILE *fh) {
   LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
-
   if (fh) {
 size_t num_bytes = GetOutputSize();
 if (num_bytes)
@@ -107,6 +107,21 @@
   return 0;
 }
 
+size_t SBCommandReturnObject::PutOutput(FileSP file_sp) {
+  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (FileSP),
+ file_sp);
+  if (!file_sp)
+return 0;
+  return file_sp->Printf("%s", GetOutput());
+}
+
+size_t SBCommandReturnObject::PutOutput(SBFile file) {
+  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (SBFile), file);
+  if (!file.m_opaque_sp)
+return 0;
+  return file.m_opaque_sp->Printf("%s", 

[Lldb-commits] [PATCH] D68442: [lldb] Unifying lldb python path

[Haibo Huang via Phabricator via lldb-commits]

hhb updated this revision to Diff 223135.
hhb added a comment.

Fix description..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68442

Files:
  lldb/CMakeLists.txt
  lldb/scripts/CMakeLists.txt
  lldb/scripts/Python/finishSwigPythonLLDB.py
  lldb/scripts/finishSwigWrapperClasses.py
  lldb/scripts/get_relative_lib_dir.py
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -48,8 +48,7 @@
 
 protected:
   static void ComputePythonDirForApple(llvm::SmallVectorImpl );
-  static void ComputePythonDirForPosix(llvm::SmallVectorImpl );
-  static void ComputePythonDirForWindows(llvm::SmallVectorImpl );
+  static void ComputePythonDir(llvm::SmallVectorImpl );
 };
 } // namespace lldb_private
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -305,39 +305,20 @@
   auto rend = llvm::sys::path::rend(path_ref);
   auto framework = std::find(rbegin, rend, "LLDB.framework");
   if (framework == rend) {
-ComputePythonDirForPosix(path);
+ComputePythonDir(path);
 return;
   }
   path.resize(framework - rend);
   llvm::sys::path::append(path, style, "LLDB.framework", "Resources", "Python");
 }
 
-void ScriptInterpreterPython::ComputePythonDirForPosix(
+void ScriptInterpreterPython::ComputePythonDir(
 llvm::SmallVectorImpl ) {
-  auto style = llvm::sys::path::Style::posix;
-#if defined(LLDB_PYTHON_RELATIVE_LIBDIR)
   // Build the path by backing out of the lib dir, then building with whatever
   // the real python interpreter uses.  (e.g. lib for most, lib64 on RHEL
-  // x86_64).
-  llvm::sys::path::remove_filename(path, style);
-  llvm::sys::path::append(path, style, LLDB_PYTHON_RELATIVE_LIBDIR);
-#else
-  llvm::sys::path::append(path, style,
-  "python" + llvm::Twine(PY_MAJOR_VERSION) + "." +
-  llvm::Twine(PY_MINOR_VERSION),
-  "site-packages");
-#endif
-}
-
-void ScriptInterpreterPython::ComputePythonDirForWindows(
-llvm::SmallVectorImpl ) {
-  auto style = llvm::sys::path::Style::windows;
-  llvm::sys::path::remove_filename(path, style);
-  llvm::sys::path::append(path, style, "lib", "site-packages");
-
-  // This will be injected directly through FileSpec.GetDirectory().SetString(),
-  // so we need to normalize manually.
-  std::replace(path.begin(), path.end(), '\\', '/');
+  // x86_64, or bin on Windows).
+  llvm::sys::path::remove_filename(path);
+  llvm::sys::path::append(path, LLDB_PYTHON_RELATIVE_LIBDIR);
 }
 
 FileSpec ScriptInterpreterPython::GetPythonDir() {
@@ -350,11 +331,10 @@
 
 #if defined(__APPLE__)
 ComputePythonDirForApple(path);
-#elif defined(_WIN32)
-ComputePythonDirForWindows(path);
 #else
-ComputePythonDirForPosix(path);
+ComputePythonDir(path);
 #endif
+llvm::sys::path::native(path);
 spec.GetDirectory().SetString(path);
 return spec;
   }();
Index: lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
@@ -1,15 +1,7 @@
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
-  # Call a python script to gather the arch-specific libdir for
-  # modules like the lldb module.
-  execute_process(
-COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/../../../../scripts/get_relative_lib_dir.py
-RESULT_VARIABLE get_libdir_status
-OUTPUT_VARIABLE relative_libdir
-)
-  if (get_libdir_status EQUAL 0)
-add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${relative_libdir}")
-  endif()
+if(NOT LLDB_PYTHON_RELATIVE_PATH)
+  message(FATAL_ERROR "LLDB_PYTHON_RELATIVE_PATH is not set yet.")
 endif()
+add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${LLDB_PYTHON_RELATIVE_PATH}")
 
 add_lldb_library(lldbPluginScriptInterpreterPython PLUGIN
   PythonDataObjects.cpp
Index: lldb/scripts/get_relative_lib_dir.py
===
--- lldb/scripts/get_relative_lib_dir.py
+++ /dev/null
@@ -1,44 +0,0 @@
-import distutils.sysconfig
-import os
-import platform
-import re
-import sys
-
-
-def get_python_relative_libdir():
-"""Returns the appropropriate python 

[Lldb-commits] [PATCH] D68442: [lldb] Remove unused variables.

[Haibo Huang via Phabricator via lldb-commits]

hhb created this revision.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

Fixes the comment in https://reviews.llvm.org/D67993

[lldb] Unifying lldb python path

There are 3 places where python site-package path is calculated
independently:

1. finishSwigPythonLLDB.py where files are written to site-packages.

2. lldb/scripts/CMakeLists.txt where site-packages are installed.

3. ScriptInterpreterPython.cpp where site-packages are added to

PYTHONPATH.

This change creates the path once and use it everywhere. So that they
will not go out of sync.

Also it provides a chance for cross compile users to specify the right
path for site-packages.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68442

Files:
  lldb/CMakeLists.txt
  lldb/scripts/CMakeLists.txt
  lldb/scripts/Python/finishSwigPythonLLDB.py
  lldb/scripts/finishSwigWrapperClasses.py
  lldb/scripts/get_relative_lib_dir.py
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -48,8 +48,7 @@
 
 protected:
   static void ComputePythonDirForApple(llvm::SmallVectorImpl );
-  static void ComputePythonDirForPosix(llvm::SmallVectorImpl );
-  static void ComputePythonDirForWindows(llvm::SmallVectorImpl );
+  static void ComputePythonDir(llvm::SmallVectorImpl );
 };
 } // namespace lldb_private
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -304,39 +304,20 @@
   auto rend = llvm::sys::path::rend(path_ref);
   auto framework = std::find(rbegin, rend, "LLDB.framework");
   if (framework == rend) {
-ComputePythonDirForPosix(path);
+ComputePythonDir(path);
 return;
   }
   path.resize(framework - rend);
   llvm::sys::path::append(path, style, "LLDB.framework", "Resources", "Python");
 }
 
-void ScriptInterpreterPython::ComputePythonDirForPosix(
+void ScriptInterpreterPython::ComputePythonDir(
 llvm::SmallVectorImpl ) {
-  auto style = llvm::sys::path::Style::posix;
-#if defined(LLDB_PYTHON_RELATIVE_LIBDIR)
   // Build the path by backing out of the lib dir, then building with whatever
   // the real python interpreter uses.  (e.g. lib for most, lib64 on RHEL
-  // x86_64).
-  llvm::sys::path::remove_filename(path, style);
-  llvm::sys::path::append(path, style, LLDB_PYTHON_RELATIVE_LIBDIR);
-#else
-  llvm::sys::path::append(path, style,
-  "python" + llvm::Twine(PY_MAJOR_VERSION) + "." +
-  llvm::Twine(PY_MINOR_VERSION),
-  "site-packages");
-#endif
-}
-
-void ScriptInterpreterPython::ComputePythonDirForWindows(
-llvm::SmallVectorImpl ) {
-  auto style = llvm::sys::path::Style::windows;
-  llvm::sys::path::remove_filename(path, style);
-  llvm::sys::path::append(path, style, "lib", "site-packages");
-
-  // This will be injected directly through FileSpec.GetDirectory().SetString(),
-  // so we need to normalize manually.
-  std::replace(path.begin(), path.end(), '\\', '/');
+  // x86_64, or bin on Windows).
+  llvm::sys::path::remove_filename(path);
+  llvm::sys::path::append(path, LLDB_PYTHON_RELATIVE_LIBDIR);
 }
 
 FileSpec ScriptInterpreterPython::GetPythonDir() {
@@ -349,11 +330,10 @@
 
 #if defined(__APPLE__)
 ComputePythonDirForApple(path);
-#elif defined(_WIN32)
-ComputePythonDirForWindows(path);
 #else
-ComputePythonDirForPosix(path);
+ComputePythonDir(path);
 #endif
+llvm::sys::path::native(path);
 spec.GetDirectory().SetString(path);
 return spec;
   }();
Index: lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
@@ -1,15 +1,7 @@
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
-  # Call a python script to gather the arch-specific libdir for
-  # modules like the lldb module.
-  execute_process(
-COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/../../../../scripts/get_relative_lib_dir.py
-RESULT_VARIABLE get_libdir_status
-OUTPUT_VARIABLE relative_libdir
-)
-  if (get_libdir_status EQUAL 0)
-add_definitions(-DLLDB_PYTHON_RELATIVE_LIBDIR="${relative_libdir}")
-  endif()
+if(NOT LLDB_PYTHON_RELATIVE_PATH)
+  message(FATAL_ERROR "LLDB_PYTHON_RELATIVE_PATH is 

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

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

aadsm added a comment.

I ended reverting this because of SymbolFile/Breakpad/symtab.test. It seems 
that in this test we add symbols after the synthetic entry point symbol is 
added creating confusion.
I think I'll need to change the code that adds symbols in Symtab to explicitly 
check if we're overlapping the synthetic entry point and reduce the entry point 
or remove it in the case it overlaps at the beginning.


Repository:
  rL LLVM

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] [lldb] r373686 - [Python] Remove unused variable

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu Oct  3 18:38:57 2019
New Revision: 373686

URL: http://llvm.org/viewvc/llvm-project?rev=373686=rev
Log:
[Python] Remove unused variable

warning: unused variable 'py_func_obj' [-Wunused-variable]
  PyObject *py_func_obj = m_py_obj;

Modified:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

Modified: 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp?rev=373686=373685=373686=diff
==
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
(original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
Thu Oct  3 18:38:57 2019
@@ -891,7 +891,6 @@ PythonCallable::ArgInfo PythonCallable::
   ArgInfo result = {0, false, false, false};
   if (!IsValid())
 return result;
-  PyObject *py_func_obj = m_py_obj;
 
   PythonObject __init__ = GetAttributeValue("__init__");
   if (__init__.IsValid() ) {


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

[Lldb-commits] [lldb] r373685 - Properly handle instantiation-dependent array bounds.

[Richard Smith via lldb-commits]

Author: rsmith
Date: Thu Oct  3 18:25:59 2019
New Revision: 373685

URL: http://llvm.org/viewvc/llvm-project?rev=373685=rev
Log:
Properly handle instantiation-dependent array bounds.

We previously failed to treat an array with an instantiation-dependent
but not value-dependent bound as being an instantiation-dependent type.
We now track the array bound expression as part of a constant array type
if it's an instantiation-dependent expression.

Modified:
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=373685=373684=373685=diff
==
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Thu Oct  3 18:25:59 2019
@@ -2180,7 +2180,7 @@ CompilerType ClangASTContext::CreateArra
   } else {
 return CompilerType(this, ast->getConstantArrayType(
  ClangUtil::GetQualType(element_type),
- ap_element_count,
+ ap_element_count, nullptr,
  clang::ArrayType::Normal, 0)
   .getAsOpaquePtr());
   }
@@ -4469,7 +4469,7 @@ CompilerType ClangASTContext::GetArrayTy
 return CompilerType(
 this, ast_ctx
   ->getConstantArrayType(
-  qual_type, llvm::APInt(64, size),
+  qual_type, llvm::APInt(64, size), nullptr,
   clang::ArrayType::ArraySizeModifier::Normal, 0)
   .getAsOpaquePtr());
   else


___
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]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373680: Explicitly set entry point arch when its thumb 
(authored by aadsm, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68069?vs=222967=223121#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68069

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

Index: lldb/trunk/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
===
--- lldb/trunk/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
+++ lldb/trunk/unittests/ObjectFile/ELF/TestObjectFileELF.cpp
@@ -172,3 +172,129 @@
   Uuid.SetFromStringRef("1b8a73ac238390e32a7ff4ac8ebe4d6a41ecf5c9", 20);
   EXPECT_EQ(Spec.GetUUID(), Uuid);
 }
+
+TEST_F(ObjectFileELFTest, GetSymtab_NoSymEntryPointArmThumbAddressClass) {
+  /*
+  // nosym-entrypoint-arm-thumb.s
+  .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
+  _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:   

[Lldb-commits] [lldb] r373679 - Python3 doesn't seem to allow you to tell whether an object is a class

[Jim Ingham via lldb-commits]

Author: jingham
Date: Thu Oct  3 16:57:34 2019
New Revision: 373679

URL: http://llvm.org/viewvc/llvm-project?rev=373679=rev
Log:
Python3 doesn't seem to allow you to tell whether an object is a class

PyClass_Check and everything it relied on seems gone from Python3.7.  So
I won't check whether it is a class first...

Also cleaned up a couple of warnings.

Modified:
lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

Modified: lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h?rev=373679=373678=373679=diff
==
--- lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h 
(original)
+++ lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h Thu 
Oct  3 16:57:34 2019
@@ -27,7 +27,7 @@ public:
   int class_option = 'C',
   int key_option = 'k', 
   int value_option = 'v',
-  char *class_long_option = "python-class",
+  const char *class_long_option = "python-class",
   const char *key_long_option = "python-class-key",
   const char *value_long_option = "python-class-value",
   bool required = false);

Modified: lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp?rev=373679=373678=373679=diff
==
--- lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp (original)
+++ lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp Thu Oct  3 
16:57:34 2019
@@ -18,7 +18,7 @@ OptionGroupPythonClassWithDict::OptionGr
  int class_option,
  int key_option, 
  int value_option,
- char *class_long_option,
+ const char *class_long_option,
  const char *key_long_option,
  const char *value_long_option,
  bool required) {
@@ -77,7 +77,6 @@ Status OptionGroupPythonClassWithDict::S
 llvm::StringRef option_arg,
 ExecutionContext *execution_context) {
   Status error;
-  const int short_option = m_option_definition[option_idx].short_option;
   switch (option_idx) {
   case 0: {
 m_class_name.assign(option_arg);

Modified: 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp?rev=373679=373678=373679=diff
==
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
(original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
Thu Oct  3 16:57:34 2019
@@ -892,8 +892,6 @@ PythonCallable::ArgInfo PythonCallable::
   if (!IsValid())
 return result;
   PyObject *py_func_obj = m_py_obj;
-  if (!PyClass_Check(m_py_obj))
-return result;
 
   PythonObject __init__ = GetAttributeValue("__init__");
   if (__init__.IsValid() ) {


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

[Lldb-commits] [PATCH] D68434: SBFile support in SBCommandReturnObject

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

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.
lawrence_danna added a parent revision: D68433: SBFile: add a bunch of tests 
that should eventually work..

This patch add SBFile interfaces to SBCommandReturnObject, and
removes the internal callers of its FILE* interfaces.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68434

Files:
  lldb/include/lldb/API/SBCommandReturnObject.h
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/interface/SBCommandReturnObject.i
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/API/SBDebugger.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -11,6 +11,7 @@
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBHostOS.h"
 #include "lldb/API/SBLanguageRuntime.h"
 #include "lldb/API/SBReproducer.h"
@@ -499,16 +500,16 @@
   SBCommandReturnObject result;
   sb_interpreter.SourceInitFileInHomeDirectory(result);
   if (m_option_data.m_debug_mode) {
-result.PutError(m_debugger.GetErrorFileHandle());
-result.PutOutput(m_debugger.GetOutputFileHandle());
+result.PutError(m_debugger.GetErrorFile());
+result.PutOutput(m_debugger.GetOutputFile());
   }
 
   // Source the local .lldbinit file if it exists and we're allowed to source.
   // Here we want to always print the return object because it contains the
   // warning and instructions to load local lldbinit files.
   sb_interpreter.SourceInitFileInCurrentWorkingDirectory(result);
-  result.PutError(m_debugger.GetErrorFileHandle());
-  result.PutOutput(m_debugger.GetOutputFileHandle());
+  result.PutError(m_debugger.GetErrorFile());
+  result.PutOutput(m_debugger.GetOutputFile());
 
   // We allow the user to specify an exit code when calling quit which we will
   // return when exiting.
@@ -574,8 +575,8 @@
   }
 
   if (m_option_data.m_debug_mode) {
-result.PutError(m_debugger.GetErrorFileHandle());
-result.PutOutput(m_debugger.GetOutputFileHandle());
+result.PutError(m_debugger.GetErrorFile());
+result.PutOutput(m_debugger.GetOutputFile());
   }
 
   const bool handle_events = true;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -893,8 +893,9 @@
   ::setbuf(outfile_handle, nullptr);
 
 result->SetImmediateOutputFile(
-debugger.GetOutputFile().GetStream());
-result->SetImmediateErrorFile(debugger.GetErrorFile().GetStream());
+debugger.GetOutputStream().GetFileSP());
+result->SetImmediateErrorFile(
+debugger.GetErrorStream().GetFileSP());
   }
 }
   }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -468,9 +468,9 @@
 sb_interpreter.HandleCommand(command, result, false);
 
 if (GetErrorFileHandle() != nullptr)
-  result.PutError(GetErrorFileHandle());
+  result.PutError(GetErrorFile());
 if (GetOutputFileHandle() != nullptr)
-  result.PutOutput(GetOutputFileHandle());
+  result.PutOutput(GetOutputFile());
 
 if (!m_opaque_sp->GetAsyncExecution()) {
   SBProcess process(GetCommandInterpreter().GetProcess());
Index: lldb/source/API/SBCommandReturnObject.cpp
===
--- lldb/source/API/SBCommandReturnObject.cpp
+++ lldb/source/API/SBCommandReturnObject.cpp
@@ -10,6 +10,7 @@
 #include "SBReproducerPrivate.h"
 #include "Utils.h"
 #include "lldb/API/SBError.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBStream.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Utility/ConstString.h"
@@ -98,7 +99,6 @@
 
 size_t SBCommandReturnObject::PutOutput(FILE *fh) {
   LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
-
   if (fh) {
 size_t num_bytes = GetOutputSize();
 if (num_bytes)
@@ -107,6 +107,21 @@
   return 0;
 }
 
+size_t SBCommandReturnObject::PutOutput(FileSP file_sp) {
+  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (FileSP),
+ file_sp);
+  if (!file_sp)
+return 0;
+  return file_sp->Printf("%s", GetOutput());
+}
+
+size_t 

[Lldb-commits] [PATCH] D68433: SBFile: add a bunch of tests that should eventually work.

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

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.
lawrence_danna added a parent revision: D68188: allow arbitrary python streams 
to be converted to SBFile.

It's really annoying and confusing to have to keep referring
back to earlier versions of this SBFile work to find the
tests that need to be added for each patch, and not duplicate
them with new tests.

This patch just imports all my tests.   A bunch of them don't
work yet, so they are marked to be skipped.   They'll be
unmarked as I fix them.

One of these tests will actually trip an assert in the SWIG
code now instead of just failing, so I'm fixing that here too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68433

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig

Index: lldb/scripts/Python/python-typemaps.swig
===
--- lldb/scripts/Python/python-typemaps.swig
+++ lldb/scripts/Python/python-typemaps.swig
@@ -440,19 +440,19 @@
   $1 = nullptr;
else if (!lldb_private::PythonFile::Check($input)) {
   int fd = PyObject_AsFileDescriptor($input);
+  if (fd < 0 || PyErr_Occurred())
+return nullptr;
   PythonObject py_input(PyRefType::Borrowed, $input);
   PythonString py_mode = py_input.GetAttributeValue("mode").AsType();
-
-  if (-1 != fd && py_mode.IsValid()) {
- FILE *f;
- if ((f = fdopen(fd, py_mode.GetString().str().c_str(
-$1 = f;
- else
-PyErr_SetString(PyExc_TypeError, strerror(errno));
-  } else {
- PyErr_SetString(PyExc_TypeError,"not a file-like object");
- return nullptr;
-  }
+  if (!py_mode.IsValid() || PyErr_Occurred())
+return nullptr;
+FILE *f;
+if ((f = fdopen(fd, py_mode.GetString().str().c_str(
+  $1 = f;
+else {
+  PyErr_SetString(PyExc_TypeError, strerror(errno));
+  return nullptr;
+}
}
else
{
Index: lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -180,6 +180,19 @@
 self.assertIn("is not a valid command", f.read())
 
 
+@add_test_categories(['pyapi'])
+def test_legacy_file_error(self):
+debugger = self.debugger
+with open(self.out_filename, 'w') as f:
+debugger.SetErrorFileHandle(f, False)
+self.handleCmd('lolwut', check=False, collect_result=False)
+debugger.GetErrorFileHandle().write('FOOBAR\n')
+with open(self.out_filename, 'r') as f:
+errors = f.read()
+self.assertTrue(re.search(r'error:.*lolwut', errors))
+self.assertTrue(re.search(r'FOOBAR', errors))
+
+
 @add_test_categories(['pyapi'])
 def test_sbfile_type_errors(self):
 sbf = lldb.SBFile()
@@ -269,6 +282,17 @@
 self.assertTrue(re.search(r'Show a list of all debugger commands', f.read()))
 
 
+@add_test_categories(['pyapi'])
+def test_help(self):
+debugger = self.debugger
+with open(self.out_filename, 'w') as f:
+status = debugger.SetOutputFile(lldb.SBFile(f))
+self.assertTrue(status.Success())
+self.handleCmd("help help", check=False, collect_result=False)
+with open(self.out_filename, 'r') as f:
+self.assertIn('Show a list of all debugger commands', f.read())
+
+
 @add_test_categories(['pyapi'])
 def test_immediate(self):
 with open(self.out_filename, 'w') as f:
@@ -278,15 +302,44 @@
 interpreter.HandleCommand("help help", ret)
 # make sure the file wasn't closed early.
 f.write("\nQUUX\n")
-
 ret = None # call destructor and flush streams
-
 with open(self.out_filename, 'r') as f:
 output = f.read()
 self.assertTrue(re.search(r'Show a list of all debugger commands', output))
 self.assertTrue(re.search(r'QUUX', output))
 
 
+@add_test_categories(['pyapi'])
+@skipIf(True) # FIXME need SBFile interfaces on SBCommandReturnObject
+def test_immediate_string(self):
+f = io.StringIO()
+ret = lldb.SBCommandReturnObject()
+ret.SetImmediateOutputFile(f)
+interpreter = self.debugger.GetCommandInterpreter()
+interpreter.HandleCommand("help help", ret)
+# make sure the file wasn't closed early.
+f.write("\nQUUX\n")
+ret = None # call destructor and flush streams
+output = f.getvalue()
+self.assertTrue(re.search(r'Show a list of all debugger commands', output))
+

[Lldb-commits] [PATCH] D67988: [lldb] clean up lldb/scripts a little bit

[Haibo Huang via Phabricator via lldb-commits]

hhb updated this revision to Diff 223117.
hhb added a comment.

Fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67988

Files:
  lldb/scripts/Python/finishSwigPythonLLDB.py
  lldb/scripts/finishSwigWrapperClasses.py

Index: lldb/scripts/finishSwigWrapperClasses.py
===
--- lldb/scripts/finishSwigWrapperClasses.py
+++ lldb/scripts/finishSwigWrapperClasses.py
@@ -5,8 +5,6 @@
 
 Overview:   Python script(s) to finish off the SWIG Python C++ Script
 Bridge wrapper code on the Windows/LINUX/OSX platform.
-The Python scripts are equivalent to the shell script (.sh)
-files.
 We use SWIG to create a C++ file containing the appropriate
 wrapper classes and functions for each scripting language,
 before liblldb is built (thus the C++ file can be compiled
@@ -293,11 +291,9 @@
 
 # Iterate script directory find any script language directories
 for scriptLang in listDirs:
-# __pycache__ is a magic directory in Python 3 that holds .pyc files
-if scriptLang != "__pycache__" and scriptLang != "swig_bot_lib":
-dbg.dump_text("Executing language script for \'%s\'" % scriptLang)
-nResult, strStatusMsg = run_post_process(
-scriptLang, strFinishFileName, vDictArgs)
+dbg.dump_text("Executing language script for \'%s\'" % scriptLang)
+nResult, strStatusMsg = run_post_process(
+scriptLang, strFinishFileName, vDictArgs)
 if nResult < 0:
 break
 
Index: lldb/scripts/Python/finishSwigPythonLLDB.py
===
--- lldb/scripts/Python/finishSwigPythonLLDB.py
+++ lldb/scripts/Python/finishSwigPythonLLDB.py
@@ -5,8 +5,6 @@
 
 Overview:   Python script(s) to post process SWIG Python C++ Script
 Bridge wrapper code on the Windows/LINUX/OSX platform.
-The Python scripts are equivalent to the shell script (.sh)
-files.
 For the Python script interpreter (external to liblldb) to
 be able to import and use the lldb module, there must be
 two files, lldb.py and _lldb.so, that it can find. lldb.py
@@ -599,7 +597,8 @@
 
 #++---
 # Details:  Determine where to put the files. Retrieve the directory path for
-#   Python's dist_packages/ site_package folder on a Windows platform.
+#   Python's dist_packages / site_package folder when build for traditional
+#   unix layout.
 # Args: vDictArgs   - (R) Program input parameters.
 # Returns:  Bool - True = function success, False = failure.
 #   Str - Python Framework directory path.
@@ -608,13 +607,14 @@
 #--
 
 
-def get_framework_python_dir_windows(vDictArgs):
+def get_framework_python_dir_traditional(vDictArgs):
 dbg = utilsDebug.CDebugFnVerbose(
-"Python script get_framework_python_dir_windows()")
+"Python script get_framework_python_dir_traditional()")
 bOk = True
 strWkDir = ""
 strErrMsg = ""
 
+dbg.dump_text("Built by LLVM")
 # We are being built by LLVM, so use the PYTHON_INSTALL_DIR argument,
 # and append the python version directory to the end of it.  Depending
 # on the system other stuff may need to be put here as well.
@@ -640,7 +640,7 @@
 
 #++---
 # Details:  Retrieve the directory path for Python's dist_packages/
-#   site_package folder on a UNIX style platform.
+#   site_package folder for macos framework.
 # Args: vDictArgs   - (R) Program input parameters.
 # Returns:  Bool - True = function success, False = failure.
 #   Str - Python Framework directory path.
@@ -649,32 +649,27 @@
 #--
 
 
-def get_framework_python_dir_other_platforms(vDictArgs):
+def get_framework_python_dir_macos_framework(vDictArgs):
 dbg = utilsDebug.CDebugFnVerbose(
-"Python script get_framework_python_dir_other_platform()")
+"Python script get_framework_python_dir_macos_framework()")
 bOk = True
 strWkDir = ""
 strErrMsg = ""
 bDbg = "-d" in vDictArgs
 
-bMakeFileCalled = "-m" in vDictArgs
-if bMakeFileCalled:
-dbg.dump_text("Built by LLVM")
-return get_framework_python_dir_windows(vDictArgs)
+dbg.dump_text("Built by XCode")
+# We are being built by XCode, so all the lldb Python files can go
+# into the LLDB.framework/Resources/Python subdirectory.
+strWkDir = vDictArgs["--targetDir"]
+strWkDir = os.path.join(strWkDir, "LLDB.framework")
+if os.path.exists(strWkDir):
+if bDbg:
+

[Lldb-commits] [lldb] r373677 - Forgot to change the header guards on OptionGroupPythonClassWithDict.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Thu Oct  3 16:32:42 2019
New Revision: 373677

URL: http://llvm.org/viewvc/llvm-project?rev=373677=rev
Log:
Forgot to change the header guards on OptionGroupPythonClassWithDict.

I think that's what is confusing the modules build on the bots.

Modified:
lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp

Modified: lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h?rev=373677=373676=373677=diff
==
--- lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h 
(original)
+++ lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h Thu 
Oct  3 16:32:42 2019
@@ -6,9 +6,10 @@
 //
 
//===--===//
 
-#ifndef liblldb_OptionGroupString_h_
-#define liblldb_OptionGroupString_h_
+#ifndef liblldb_OptionGroupPythonClassWithDict_h_
+#define liblldb_OptionGroupPythonClassWithDict_h_
 
+#include "lldb/lldb-types.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Utility/StructuredData.h"
 
@@ -61,4 +62,4 @@ protected:
 
 } // namespace lldb_private
 
-#endif // liblldb_OptionGroupString_h_
+#endif // liblldb_OptionGroupPythonClassWithDict_h_

Modified: lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp?rev=373677=373676=373677=diff
==
--- lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp (original)
+++ lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp Thu Oct  3 
16:32:42 2019
@@ -1,4 +1,4 @@
-//===-- OptionGroupKeyValue.cpp --*- C++ 
-*-===//
+//===-- OptionGroupPythonClassWithDict.cpp 
--*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.


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

[Lldb-commits] [PATCH] D68422: [DWARFASTParserClang] Factor out structure-like type parsing, NFC

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

Thank you. I've just realized that Phab's "Raw Diff" mode 
(https://reviews.llvm.org/F10156020) renders the diff in a much clearer way. I 
suspect reviewing the raw diff will be much easier than reviewing what Phab has 
rendered here.


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

https://reviews.llvm.org/D68422



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

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

[Jim Ingham via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373675: Pass an SBStructuredData to scripted ThreadPlans on 
use. (authored by jingham, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68366?vs=223096=223115#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68366

Files:
  lldb/trunk/include/lldb/API/SBStructuredData.h
  lldb/trunk/include/lldb/API/SBThread.h
  lldb/trunk/include/lldb/API/SBThreadPlan.h
  lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/trunk/include/lldb/Target/Thread.h
  lldb/trunk/include/lldb/Target/ThreadPlanPython.h
  lldb/trunk/packages/Python/lldbsuite/test/commands/help/TestHelp.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  lldb/trunk/scripts/Python/python-wrapper.swig
  lldb/trunk/scripts/interface/SBThread.i
  lldb/trunk/scripts/interface/SBThreadPlan.i
  lldb/trunk/source/API/SBThread.cpp
  lldb/trunk/source/API/SBThreadPlan.cpp
  lldb/trunk/source/Commands/CommandObjectThread.cpp
  lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/trunk/source/Target/Thread.cpp
  lldb/trunk/source/Target/ThreadPlanPython.cpp
  lldb/trunk/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/trunk/scripts/Python/python-wrapper.swig
===
--- lldb/trunk/scripts/Python/python-wrapper.swig
+++ lldb/trunk/scripts/Python/python-wrapper.swig
@@ -250,6 +250,7 @@
 (
 const char *python_class_name,
 const char *session_dictionary_name,
+lldb_private::StructuredDataImpl *args_impl,
 std::string _string,
 const lldb::ThreadPlanSP& thread_plan_sp
 )
@@ -279,7 +280,23 @@
 if (!tp_arg.IsAllocated())
 Py_RETURN_NONE;
 
-PythonObject result = pfunc(tp_arg, dict);
+PythonObject result = {};
+size_t init_num_args = pfunc.GetNumInitArguments().count;
+if (init_num_args == 3) {
+if (args_impl != nullptr) {
+   error_string.assign("args passed, but __init__ does not take an args dictionary");
+   Py_RETURN_NONE;
+}
+result = pfunc(tp_arg, dict);
+} else if (init_num_args = 4) {
+lldb::SBStructuredData *args_value = new lldb::SBStructuredData(args_impl);
+PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(args_value));
+result = pfunc(tp_arg, args_arg, dict);
+} else {
+error_string.assign("wrong number of arguments in __init__, should be 1 or 2 (not including self & dict)");
+Py_RETURN_NONE;
+}
+
 // FIXME: At this point we should check that the class we found supports all the methods
 // that we need.
 
Index: lldb/trunk/scripts/interface/SBThread.i
===
--- lldb/trunk/scripts/interface/SBThread.i
+++ lldb/trunk/scripts/interface/SBThread.i
@@ -254,6 +254,11 @@
 StepUsingScriptedThreadPlan (const char *script_class_name, bool resume_immediately);
 
 SBError
+StepUsingScriptedThreadPlan(const char *script_class_name,
+lldb::SBStructuredData _data,
+bool resume_immediately);
+
+SBError
 JumpToLine (lldb::SBFileSpec _spec, uint32_t line);
 
 void
Index: lldb/trunk/scripts/interface/SBThreadPlan.i
===
--- lldb/trunk/scripts/interface/SBThreadPlan.i
+++ lldb/trunk/scripts/interface/SBThreadPlan.i
@@ -109,6 +109,14 @@
 SBThreadPlan
 QueueThreadPlanForStepScripted(const char *script_class_name);
 
+SBThreadPlan
+QueueThreadPlanForStepScripted(const char *script_class_name,
+   SBError );
+SBThreadPlan
+QueueThreadPlanForStepScripted(const char *script_class_name,
+   SBStructuredData _data,
+   SBError );
+
 
 protected:
 friend class SBBreakpoint;
Index: lldb/trunk/include/lldb/API/SBThread.h
===
--- lldb/trunk/include/lldb/API/SBThread.h
+++ lldb/trunk/include/lldb/API/SBThread.h
@@ -122,6 +122,10 @@
   SBError StepUsingScriptedThreadPlan(const char *script_class_name,
   bool resume_immediately);
 
+  SBError StepUsingScriptedThreadPlan(const char *script_class_name,
+  lldb::SBStructuredData _data,
+

[Lldb-commits] [lldb] r373675 - Pass an SBStructuredData to scripted ThreadPlans on use.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Thu Oct  3 15:50:18 2019
New Revision: 373675

URL: http://llvm.org/viewvc/llvm-project?rev=373675=rev
Log:
Pass an SBStructuredData to scripted ThreadPlans on use.

This will allow us to write reusable scripted ThreadPlans, since
you can use key/value pairs with known keys in the plan to parametrize
its behavior.

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

Modified:
lldb/trunk/include/lldb/API/SBStructuredData.h
lldb/trunk/include/lldb/API/SBThread.h
lldb/trunk/include/lldb/API/SBThreadPlan.h
lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
lldb/trunk/include/lldb/Target/Thread.h
lldb/trunk/include/lldb/Target/ThreadPlanPython.h
lldb/trunk/packages/Python/lldbsuite/test/commands/help/TestHelp.py

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

lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
lldb/trunk/scripts/Python/python-wrapper.swig
lldb/trunk/scripts/interface/SBThread.i
lldb/trunk/scripts/interface/SBThreadPlan.i
lldb/trunk/source/API/SBThread.cpp
lldb/trunk/source/API/SBThreadPlan.cpp
lldb/trunk/source/Commands/CommandObjectThread.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/trunk/source/Target/Thread.cpp
lldb/trunk/source/Target/ThreadPlanPython.cpp
lldb/trunk/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Modified: lldb/trunk/include/lldb/API/SBStructuredData.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBStructuredData.h?rev=373675=373674=373675=diff
==
--- lldb/trunk/include/lldb/API/SBStructuredData.h (original)
+++ lldb/trunk/include/lldb/API/SBStructuredData.h Thu Oct  3 15:50:18 2019
@@ -91,6 +91,8 @@ protected:
   friend class SBTraceOptions;
   friend class SBDebugger;
   friend class SBTarget;
+  friend class SBThread;
+  friend class SBThreadPlan;
 
   StructuredDataImplUP m_impl_up;
 };

Modified: lldb/trunk/include/lldb/API/SBThread.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBThread.h?rev=373675=373674=373675=diff
==
--- lldb/trunk/include/lldb/API/SBThread.h (original)
+++ lldb/trunk/include/lldb/API/SBThread.h Thu Oct  3 15:50:18 2019
@@ -122,6 +122,10 @@ public:
   SBError StepUsingScriptedThreadPlan(const char *script_class_name,
   bool resume_immediately);
 
+  SBError StepUsingScriptedThreadPlan(const char *script_class_name,
+  lldb::SBStructuredData _data,
+  bool resume_immediately);
+
   SBError JumpToLine(lldb::SBFileSpec _spec, uint32_t line);
 
   void RunToAddress(lldb::addr_t addr);

Modified: lldb/trunk/include/lldb/API/SBThreadPlan.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBThreadPlan.h?rev=373675=373674=373675=diff
==
--- lldb/trunk/include/lldb/API/SBThreadPlan.h (original)
+++ lldb/trunk/include/lldb/API/SBThreadPlan.h Thu Oct  3 15:50:18 2019
@@ -28,6 +28,9 @@ public:
 
   SBThreadPlan(lldb::SBThread , const char *class_name);
 
+  SBThreadPlan(lldb::SBThread , const char *class_name, 
+   lldb::SBStructuredData _data);
+
   ~SBThreadPlan();
 
   explicit operator bool() const;
@@ -100,6 +103,9 @@ public:
   SBThreadPlan QueueThreadPlanForStepScripted(const char *script_class_name);
   SBThreadPlan QueueThreadPlanForStepScripted(const char *script_class_name,
   SBError );
+  SBThreadPlan QueueThreadPlanForStepScripted(const char *script_class_name,
+  lldb::SBStructuredData 
_data,
+  SBError );
 
 private:
   friend class SBBreakpoint;

Modified: lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h?rev=373675=373674=373675=diff
==
--- lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h (original)
+++ lldb/trunk/include/lldb/Interpreter/ScriptInterpreter.h Thu Oct  3 15:50:18 
2019
@@ -208,6 +208,7 @@ public:
 
   virtual StructuredData::ObjectSP
   CreateScriptedThreadPlan(const char *class_name,
+   StructuredDataImpl *args_data,
std::string _str,
lldb::ThreadPlanSP 

[Lldb-commits] [PATCH] D68422: [DWARFASTParserClang] Factor out structure-like type parsing, NFC

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Thank you for doing this, this looks like an excellent start from a quick 
review.


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

https://reviews.llvm.org/D68422



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

[Lldb-commits] [PATCH] D68363: Segregate the Python class + key/value dictionary into a separate OptionGroup

[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 rL373673: Break out the Python class  key/value options 
into a separate OptionGroup. (authored by jingham, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68363?vs=222938=223112#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68363

Files:
  lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py
  lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
  lldb/trunk/source/Commands/Options.td
  lldb/trunk/source/Interpreter/CMakeLists.txt
  lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp
  lldb/trunk/source/Interpreter/Options.cpp

Index: lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
===
--- lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
+++ lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
@@ -0,0 +1,64 @@
+//===-- OptionGroupPythonClassWithDict.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_OptionGroupString_h_
+#define liblldb_OptionGroupString_h_
+
+#include "lldb/Interpreter/Options.h"
+#include "lldb/Utility/StructuredData.h"
+
+namespace lldb_private {
+
+// Use this Option group if you have a python class that implements some
+// Python extension point, and you pass a SBStructuredData to the class 
+// __init__ method.  
+// class_option specifies the class name
+// the key and value options are read in in pairs, and a 
+// StructuredData::Dictionary is constructed with those pairs.
+class OptionGroupPythonClassWithDict : public OptionGroup {
+public:
+  OptionGroupPythonClassWithDict(const char *class_use,
+  int class_option = 'C',
+  int key_option = 'k', 
+  int value_option = 'v',
+  char *class_long_option = "python-class",
+  const char *key_long_option = "python-class-key",
+  const char *value_long_option = "python-class-value",
+  bool required = false);
+  
+  ~OptionGroupPythonClassWithDict() override;
+
+  llvm::ArrayRef GetDefinitions() override {
+return llvm::ArrayRef(m_option_definition);
+  }
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+ExecutionContext *execution_context) override;
+  Status SetOptionValue(uint32_t, const char *, ExecutionContext *) = delete;
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override;
+  Status OptionParsingFinished(ExecutionContext *execution_context) override;
+  
+  const StructuredData::DictionarySP GetStructuredData() {
+return m_dict_sp;
+  }
+  const std::string () {
+return m_class_name;
+  }
+
+protected:
+  std::string m_class_name;
+  std::string m_current_key;
+  StructuredData::DictionarySP m_dict_sp;
+  std::string m_class_usage_text, m_key_usage_text, m_value_usage_text;
+  OptionDefinition m_option_definition[3];
+};
+
+} // namespace lldb_private
+
+#endif // liblldb_OptionGroupString_h_
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py
@@ -7,6 +7,7 @@
   def __init__(self, bkpt, extra_args, dict):
   self.bkpt = bkpt
   self.extra_args = extra_args
+
   Resolver.func_list = []
   Resolver.got_files = 0
 
@@ -15,6 +16,8 @@
   sym_item = self.extra_args.GetValueForKey("symbol")
   if sym_item.IsValid():
   sym_name = sym_item.GetStringValue(1000)
+  else:
+  print("Didn't have a value for key 'symbol'")
 
   if sym_ctx.compile_unit.IsValid():
   Resolver.got_files = 1
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
===
--- 

[Lldb-commits] [lldb] r373673 - Break out the Python class & key/value options into a separate OptionGroup.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Thu Oct  3 15:18:51 2019
New Revision: 373673

URL: http://llvm.org/viewvc/llvm-project?rev=373673=rev
Log:
Break out the Python class & key/value options into a separate OptionGroup.

Use this in the scripted breakpoint command.  Added some tests for parsing
the key/value options.  This uncovered a bug in handling parsing errors 
mid-line.
I also fixed that bug.

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

Added:
lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
lldb/trunk/source/Interpreter/OptionGroupPythonClassWithDict.cpp
Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/resolver.py
lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
lldb/trunk/source/Commands/Options.td
lldb/trunk/source/Interpreter/CMakeLists.txt
lldb/trunk/source/Interpreter/Options.cpp

Added: lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h?rev=373673=auto
==
--- lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h (added)
+++ lldb/trunk/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h Thu 
Oct  3 15:18:51 2019
@@ -0,0 +1,64 @@
+//===-- OptionGroupPythonClassWithDict.h 
-*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef liblldb_OptionGroupString_h_
+#define liblldb_OptionGroupString_h_
+
+#include "lldb/Interpreter/Options.h"
+#include "lldb/Utility/StructuredData.h"
+
+namespace lldb_private {
+
+// Use this Option group if you have a python class that implements some
+// Python extension point, and you pass a SBStructuredData to the class 
+// __init__ method.  
+// class_option specifies the class name
+// the key and value options are read in in pairs, and a 
+// StructuredData::Dictionary is constructed with those pairs.
+class OptionGroupPythonClassWithDict : public OptionGroup {
+public:
+  OptionGroupPythonClassWithDict(const char *class_use,
+  int class_option = 'C',
+  int key_option = 'k', 
+  int value_option = 'v',
+  char *class_long_option = "python-class",
+  const char *key_long_option = "python-class-key",
+  const char *value_long_option = "python-class-value",
+  bool required = false);
+  
+  ~OptionGroupPythonClassWithDict() override;
+
+  llvm::ArrayRef GetDefinitions() override {
+return llvm::ArrayRef(m_option_definition);
+  }
+
+  Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+ExecutionContext *execution_context) override;
+  Status SetOptionValue(uint32_t, const char *, ExecutionContext *) = delete;
+
+  void OptionParsingStarting(ExecutionContext *execution_context) override;
+  Status OptionParsingFinished(ExecutionContext *execution_context) override;
+  
+  const StructuredData::DictionarySP GetStructuredData() {
+return m_dict_sp;
+  }
+  const std::string () {
+return m_class_name;
+  }
+
+protected:
+  std::string m_class_name;
+  std::string m_current_key;
+  StructuredData::DictionarySP m_dict_sp;
+  std::string m_class_usage_text, m_key_usage_text, m_value_usage_text;
+  OptionDefinition m_option_definition[3];
+};
+
+} // namespace lldb_private
+
+#endif // liblldb_OptionGroupString_h_

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py?rev=373673=373672=373673=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
 Thu Oct  3 15:18:51 2019
@@ -38,6 +38,12 @@ class TestScriptedResolver(TestBase):
 self.build()
 self.do_test_cli()
 
+def test_bad_command_lines(self):
+"""Make sure we get appropriate errors when we give invalid key/value
+   options"""
+self.build()
+self.do_test_bad_options()
+
 def setUp(self):
 # Call super's setUp().
 

[Lldb-commits] [PATCH] D68291: [process list] make the TRIPLE column wider

[walter erquinigo via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373670: [process list] make the TRIPLE column wider 
(authored by wallace, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68291?vs=222921=223105#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68291

Files:
  lldb/trunk/source/Utility/ProcessInfo.cpp
  lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp


Index: lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
===
--- lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
+++ lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
@@ -67,15 +67,15 @@
   ProcessInstanceInfo::DumpTableHeader(s, show_args, verbose);
   info.DumpAsTableRow(s, resolver, show_args, verbose);
   EXPECT_STREQ(
-  R"(PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE  
 ARGUMENTS
-== == == == == == 
 
-47 0  user1  group3 2  4  x86_64-pc-linux  

+  R"(PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE  
   ARGUMENTS
+== == == == == == 
== 
+47 0  user1  group3 2  4  x86_64-pc-linux  
  
 )",
   s.GetData());
 }
 
 TEST(ProcessInstanceInfo, DumpTable_invalidUID) {
-  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
+  ProcessInstanceInfo info("a.out", ArchSpec("aarch64-unknown-linux-android"), 
47);
 
   DummyUserIDResolver resolver;
   StreamString s;
@@ -85,9 +85,9 @@
   ProcessInstanceInfo::DumpTableHeader(s, show_args, verbose);
   info.DumpAsTableRow(s, resolver, show_args, verbose);
   EXPECT_STREQ(
-  R"(PIDPARENT USER   TRIPLE   NAME
-== == ==  
-47 0 x86_64-pc-linux  a.out
+  R"(PIDPARENT USER   TRIPLE NAME
+== == == == 

+47 0 aarch64-unknown-linux-android  a.out
 )",
   s.GetData());
 }
Index: lldb/trunk/source/Utility/ProcessInfo.cpp
===
--- lldb/trunk/source/Utility/ProcessInfo.cpp
+++ lldb/trunk/source/Utility/ProcessInfo.cpp
@@ -169,13 +169,15 @@
 
   if (verbose) {
 s.Printf("PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE 
"
- "  %s\n",
+ "%s\n",
  label);
-s.PutCString("== == == == == == "
- " \n");
+s.PutCString(
+"== == == == == == "
+"== \n");
   } else {
-s.Printf("PIDPARENT USER   TRIPLE   %s\n", label);
-s.PutCString("== == ==  "
+s.Printf("PIDPARENT USER   TRIPLE %s\n",
+ label);
+s.PutCString("== == == == "
  "\n");
   }
 }
@@ -216,12 +218,12 @@
 ::GetEffectiveGroupID,
 ::GetGroupName);
 
-  s.Printf("%-24s ", arch_strm.GetData());
+  s.Printf("%-30s ", arch_strm.GetData());
 } else {
   print(::EffectiveUserIDIsValid,
 ::GetEffectiveUserID,
 ::GetUserName);
-  s.Printf("%-24s ", arch_strm.GetData());
+  s.Printf("%-30s ", arch_strm.GetData());
 }
 
 if (verbose || show_args) {


Index: lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
===
--- lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
+++ lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
@@ -67,15 +67,15 @@
   ProcessInstanceInfo::DumpTableHeader(s, show_args, verbose);
   info.DumpAsTableRow(s, resolver, show_args, verbose);
   EXPECT_STREQ(
-  R"(PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE   ARGUMENTS
-== == == == == ==  
-47 0  user1  group3 2  4  x86_64-pc-linux  
+  R"(PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE ARGUMENTS
+== == == == == == 

[Lldb-commits] [lldb] r373670 - [process list] make the TRIPLE column wider

[Walter Erquinigo via lldb-commits]

Author: wallace
Date: Thu Oct  3 14:57:01 2019
New Revision: 373670

URL: http://llvm.org/viewvc/llvm-project?rev=373670=rev
Log:
[process list] make the TRIPLE column wider

Summary:
Now that `process list` works better on the android platform, the arch 
aarch64-unknown-linux-android appears quite often.
The existing printed width of the TRIPLE column is not long enough, which 
doesn't look okay.
E.g.
```
1561   1016aarch64-unknown-linux-android ip6tables-restore
1999   1   aarch64-unknown-linux-android tlc_server
2332   982  com.android.systemui
2378   983  webview_zygote
```

Now, after adding 6 spaces, it looks better

```
PIDPARENT USER   TRIPLE NAME
== == == == 

...
1561   1016  aarch64-unknown-linux-android  ip6tables-restore
1999   1 aarch64-unknown-linux-android  tlc_server
2332   982  com.android.systemui
2378   983  webview_zygote
2448   982  com.sec.location.nsflp2
```

Reviewers: clayborg, labath, xiaobai, aadsm

Reviewed By: labath

Subscribers: srhines, kristof.beyls, lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/source/Utility/ProcessInfo.cpp
lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp

Modified: lldb/trunk/source/Utility/ProcessInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ProcessInfo.cpp?rev=373670=373669=373670=diff
==
--- lldb/trunk/source/Utility/ProcessInfo.cpp (original)
+++ lldb/trunk/source/Utility/ProcessInfo.cpp Thu Oct  3 14:57:01 2019
@@ -169,13 +169,15 @@ void ProcessInstanceInfo::DumpTableHeade
 
   if (verbose) {
 s.Printf("PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE 
"
- "  %s\n",
+ "%s\n",
  label);
-s.PutCString("== == == == == == "
- " \n");
+s.PutCString(
+"== == == == == == "
+"== \n");
   } else {
-s.Printf("PIDPARENT USER   TRIPLE   %s\n", label);
-s.PutCString("== == ==  "
+s.Printf("PIDPARENT USER   TRIPLE %s\n",
+ label);
+s.PutCString("== == == == "
  "\n");
   }
 }
@@ -216,12 +218,12 @@ void ProcessInstanceInfo::DumpAsTableRow
 ::GetEffectiveGroupID,
 ::GetGroupName);
 
-  s.Printf("%-24s ", arch_strm.GetData());
+  s.Printf("%-30s ", arch_strm.GetData());
 } else {
   print(::EffectiveUserIDIsValid,
 ::GetEffectiveUserID,
 ::GetUserName);
-  s.Printf("%-24s ", arch_strm.GetData());
+  s.Printf("%-30s ", arch_strm.GetData());
 }
 
 if (verbose || show_args) {

Modified: lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp?rev=373670=373669=373670=diff
==
--- lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp (original)
+++ lldb/trunk/unittests/Utility/ProcessInstanceInfoTest.cpp Thu Oct  3 
14:57:01 2019
@@ -67,15 +67,15 @@ TEST(ProcessInstanceInfo, DumpTable) {
   ProcessInstanceInfo::DumpTableHeader(s, show_args, verbose);
   info.DumpAsTableRow(s, resolver, show_args, verbose);
   EXPECT_STREQ(
-  R"(PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE  
 ARGUMENTS
-== == == == == == 
 
-47 0  user1  group3 2  4  x86_64-pc-linux  

+  R"(PIDPARENT USER   GROUP  EFF USER   EFF GROUP  TRIPLE  
   ARGUMENTS
+== == == == == == 
== 
+47 0  user1  group3 2  4  x86_64-pc-linux  
  
 )",
   s.GetData());
 }
 
 TEST(ProcessInstanceInfo, DumpTable_invalidUID) {
-  ProcessInstanceInfo info("a.out", ArchSpec("x86_64-pc-linux"), 47);
+  ProcessInstanceInfo info("a.out", ArchSpec("aarch64-unknown-linux-android"), 
47);
 
   DummyUserIDResolver resolver;
   StreamString s;
@@ 

[Lldb-commits] [PATCH] D67993: [lldb] Calculate relative path for symbol links

[Haibo Huang via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373668: [lldb] Calculate relative path for symbol links 
(authored by hhb, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67993?vs=221612=223102#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67993

Files:
  lldb/trunk/scripts/Python/finishSwigPythonLLDB.py


Index: lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
===
--- lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
+++ lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
@@ -365,7 +365,6 @@
 # Throws:   None.
 #--
 
-
 def make_symlink(
 vDictArgs,
 vstrFrameworkPythonDir,
@@ -377,27 +376,15 @@
 bDbg = "-d" in vDictArgs
 strTarget = os.path.join(vstrFrameworkPythonDir, vstrTargetFile)
 strTarget = os.path.normcase(strTarget)
-strSrc = ""
+strPrefix = vDictArgs['--prefix']
 
 os.chdir(vstrFrameworkPythonDir)
 bMakeFileCalled = "-m" in vDictArgs
 eOSType = utilsOsType.determine_os_type()
-if not bMakeFileCalled:
-strBuildDir = os.path.join("..", "..", "..")
-else:
-# Resolve vstrSrcFile path relatively the build directory
-if eOSType == utilsOsType.EnumOsType.Windows:
-# On a Windows platform the vstrFrameworkPythonDir looks like:
-# llvm\\build\\Lib\\site-packages\\lldb
-strBuildDir = os.path.join("..", "..", "..")
-else:
-# On a UNIX style platform the vstrFrameworkPythonDir looks like:
-# llvm/build/lib/python2.7/site-packages/lldb
-strBuildDir = os.path.join("..", "..", "..", "..")
-strSrc = os.path.normcase(os.path.join(strBuildDir, vstrSrcFile))
-
-return make_symlink_native(vDictArgs, strSrc, strTarget)
 
+strSrc = os.path.normcase(os.path.join(strPrefix, vstrSrcFile))
+strRelSrc = os.path.relpath(strSrc, os.path.dirname(strTarget))
+return make_symlink_native(vDictArgs, strRelSrc, strTarget)
 
 #++---
 # Details:  Make the symbolic that the script bridge for Python will need in


Index: lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
===
--- lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
+++ lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
@@ -365,7 +365,6 @@
 # Throws:   None.
 #--
 
-
 def make_symlink(
 vDictArgs,
 vstrFrameworkPythonDir,
@@ -377,27 +376,15 @@
 bDbg = "-d" in vDictArgs
 strTarget = os.path.join(vstrFrameworkPythonDir, vstrTargetFile)
 strTarget = os.path.normcase(strTarget)
-strSrc = ""
+strPrefix = vDictArgs['--prefix']
 
 os.chdir(vstrFrameworkPythonDir)
 bMakeFileCalled = "-m" in vDictArgs
 eOSType = utilsOsType.determine_os_type()
-if not bMakeFileCalled:
-strBuildDir = os.path.join("..", "..", "..")
-else:
-# Resolve vstrSrcFile path relatively the build directory
-if eOSType == utilsOsType.EnumOsType.Windows:
-# On a Windows platform the vstrFrameworkPythonDir looks like:
-# llvm\\build\\Lib\\site-packages\\lldb
-strBuildDir = os.path.join("..", "..", "..")
-else:
-# On a UNIX style platform the vstrFrameworkPythonDir looks like:
-# llvm/build/lib/python2.7/site-packages/lldb
-strBuildDir = os.path.join("..", "..", "..", "..")
-strSrc = os.path.normcase(os.path.join(strBuildDir, vstrSrcFile))
-
-return make_symlink_native(vDictArgs, strSrc, strTarget)
 
+strSrc = os.path.normcase(os.path.join(strPrefix, vstrSrcFile))
+strRelSrc = os.path.relpath(strSrc, os.path.dirname(strTarget))
+return make_symlink_native(vDictArgs, strRelSrc, strTarget)
 
 #++---
 # Details:  Make the symbolic that the script bridge for Python will need in
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

[Jim Ingham via Phabricator via lldb-commits]

jingham marked 4 inline comments as done.
jingham added inline comments.



Comment at: lldb/include/lldb/Target/ThreadPlanPython.h:35
+  ThreadPlanPython(Thread , const char *class_name, 
+   StructuredDataImpl *args_data);
   ~ThreadPlanPython() override;

JDevlieghere wrote:
> Why do we need the StructuredDataImpl and not the StructuredData?
I figured out that I had to do it this way when I implemented the same feature 
as part of the ScriptedBreakpointResolvers.  This change is largely copied from 
that.  But I don't remember why now.  

I'll go back and figure that out again if you are really curious, but since 
this is following a successful precedent, I'd rather not do that right now...



Comment at: lldb/source/API/SBThreadPlan.cpp:409
+
+  if (m_opaque_sp) {
+Status plan_status;

JDevlieghere wrote:
> Swap this and have an early return?
Every other function in this file is written this way.  I don't want to fix 
them piecemeal, and doing them all here would make this commit too hard to read.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68366



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

[Lldb-commits] [PATCH] D68366: Parametrize scripted ThreadPlans using SBStructuredData

[Jim Ingham via Phabricator via lldb-commits]

jingham updated this revision to Diff 223096.
jingham added a comment.

Added the missing REGISTER macros


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68366

Files:
  lldb/include/lldb/API/SBStructuredData.h
  lldb/include/lldb/API/SBThread.h
  lldb/include/lldb/API/SBThreadPlan.h
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlanPython.h
  lldb/packages/Python/lldbsuite/test/commands/help/TestHelp.py
  lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
  
lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  lldb/scripts/Python/python-wrapper.swig
  lldb/scripts/interface/SBThread.i
  lldb/scripts/interface/SBThreadPlan.i
  lldb/source/API/SBThread.cpp
  lldb/source/API/SBThreadPlan.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlanPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -95,6 +95,7 @@
 
 extern "C" void *LLDBSwigPythonCreateScriptedThreadPlan(
 const char *python_class_name, const char *session_dictionary_name,
+StructuredDataImpl *args_data,
 std::string _string,
 const lldb::ThreadPlanSP _plan_sp) {
   return nullptr;
Index: lldb/source/Target/ThreadPlanPython.cpp
===
--- lldb/source/Target/ThreadPlanPython.cpp
+++ lldb/source/Target/ThreadPlanPython.cpp
@@ -25,10 +25,11 @@
 
 // ThreadPlanPython
 
-ThreadPlanPython::ThreadPlanPython(Thread , const char *class_name)
+ThreadPlanPython::ThreadPlanPython(Thread , const char *class_name, 
+   StructuredDataImpl *args_data)
 : ThreadPlan(ThreadPlan::eKindPython, "Python based Thread Plan", thread,
  eVoteNoOpinion, eVoteNoOpinion),
-  m_class_name(class_name), m_did_push(false) {
+  m_class_name(class_name), m_args_data(args_data), m_did_push(false) {
   SetIsMasterPlan(true);
   SetOkayToDiscard(true);
   SetPrivate(false);
@@ -65,7 +66,8 @@
.GetScriptInterpreter();
 if (script_interp) {
   m_implementation_sp = script_interp->CreateScriptedThreadPlan(
-  m_class_name.c_str(), m_error_str, this->shared_from_this());
+  m_class_name.c_str(), m_args_data, m_error_str, 
+  this->shared_from_this());
 }
   }
 }
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Interpreter/OptionValueFileSpecList.h"
@@ -1482,9 +1483,18 @@
 }
 
 lldb::ThreadPlanSP Thread::QueueThreadPlanForStepScripted(
-bool abort_other_plans, const char *class_name, bool stop_other_threads,
+bool abort_other_plans, const char *class_name, 
+StructuredData::ObjectSP extra_args_sp,  bool stop_other_threads,
 Status ) {
-  ThreadPlanSP thread_plan_sp(new ThreadPlanPython(*this, class_name));
+
+  StructuredDataImpl *extra_args_impl = nullptr; 
+  if (extra_args_sp) {
+extra_args_impl = new StructuredDataImpl();
+extra_args_impl->SetObjectSP(extra_args_sp);
+  }
+
+  ThreadPlanSP thread_plan_sp(new ThreadPlanPython(*this, class_name, 
+   extra_args_impl));
 
   status = QueueThreadPlan(thread_plan_sp, abort_other_plans);
   return thread_plan_sp;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -78,6 +78,7 @@
 
   StructuredData::ObjectSP
   CreateScriptedThreadPlan(const char *class_name,
+   StructuredDataImpl *args_data,
std::string _str,
lldb::ThreadPlanSP thread_plan) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

[Lldb-commits] [PATCH] D68370: Componentize lldb/scripts to use with LLVM_DISTRIBUTION_COMPONENTS

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

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

Rename lldb-scripts to lldb-python-scripts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68370

Files:
  lldb/scripts/CMakeLists.txt


Index: lldb/scripts/CMakeLists.txt
===
--- lldb/scripts/CMakeLists.txt
+++ lldb/scripts/CMakeLists.txt
@@ -69,5 +69,14 @@
 OUTPUT_STRIP_TRAILING_WHITESPACE)
 
   # Install the LLDB python module
-  install(DIRECTORY ${SWIG_PYTHON_DIR}/ DESTINATION ${SWIG_INSTALL_DIR})
+  add_custom_target(lldb-python-scripts)
+  add_dependencies(lldb-python-scripts finish_swig)
+  install(DIRECTORY ${SWIG_PYTHON_DIR}/
+DESTINATION ${SWIG_INSTALL_DIR}
+COMPONENT lldb-scripts)
+  if (NOT LLVM_ENABLE_IDE)
+add_llvm_install_targets(install-lldb-python-scripts
+ COMPONENT lldb-python-scripts
+ DEPENDS lldb-python-scripts)
+  endif()
 endif()


Index: lldb/scripts/CMakeLists.txt
===
--- lldb/scripts/CMakeLists.txt
+++ lldb/scripts/CMakeLists.txt
@@ -69,5 +69,14 @@
 OUTPUT_STRIP_TRAILING_WHITESPACE)
 
   # Install the LLDB python module
-  install(DIRECTORY ${SWIG_PYTHON_DIR}/ DESTINATION ${SWIG_INSTALL_DIR})
+  add_custom_target(lldb-python-scripts)
+  add_dependencies(lldb-python-scripts finish_swig)
+  install(DIRECTORY ${SWIG_PYTHON_DIR}/
+DESTINATION ${SWIG_INSTALL_DIR}
+COMPONENT lldb-scripts)
+  if (NOT LLVM_ENABLE_IDE)
+add_llvm_install_targets(install-lldb-python-scripts
+ COMPONENT lldb-python-scripts
+ DEPENDS lldb-python-scripts)
+  endif()
 endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68422: [DWARFASTParserClang] Factor out structure-like type parsing, NFC

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: labath, shafik, teemperor.
Herald added a subscriber: aprantl.
Herald added a reviewer: jdoerfert.

Split out the logic to parse structure-like types into a separate
function, in an attempt to reduce the complexity of ParseTypeFromDWARF.

Note: Phab seems to be mis-rendering the diff. I'd be happy to move this
review to the mailing list to make things easier.

Inspired by discussion in https://reviews.llvm.org/D68130.


https://reviews.llvm.org/D68422

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -15,7 +15,9 @@
 #include "llvm/ADT/SmallVector.h"
 
 #include "DWARFASTParser.h"
+#include "DWARFDIE.h"
 #include "DWARFDefines.h"
+#include "DWARFFormValue.h"
 #include "lldb/Core/ClangForward.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Symbol/ClangASTContext.h"
@@ -29,6 +31,8 @@
 class DWARFDebugInfoEntry;
 class SymbolFileDWARF;
 
+struct ParsedDWARFTypeAttributes;
+
 class DWARFASTParserClang : public DWARFASTParser {
 public:
   DWARFASTParserClang(lldb_private::ClangASTContext );
@@ -122,6 +126,11 @@
bool is_signed, uint32_t enumerator_byte_size,
const DWARFDIE _die);
 
+  /// Parse a structure, class, or union type DIE.
+  lldb::TypeSP ParseStructureLikeDIE(const DWARFDIE ,
+ ParsedDWARFTypeAttributes ,
+ lldb_private::Log *log);
+
   lldb_private::Type *GetTypeForDIE(const DWARFDIE );
 
   clang::Decl *GetClangDeclForDIE(const DWARFDIE );
@@ -142,6 +151,14 @@
 
   void LinkDeclToDIE(clang::Decl *decl, const DWARFDIE );
 
+  /// If \p type_sp is valid, calculate and set its symbol context scope, and
+  /// update the type list for its backing symbol file.
+  ///
+  /// Returns \p type_sp.
+  lldb::TypeSP
+  UpdateSymbolContextScopeForType(const lldb_private::SymbolContext ,
+  const DWARFDIE , lldb::TypeSP type_sp);
+
   lldb::TypeSP ParseTypeFromDWO(const DWARFDIE , lldb_private::Log *log);
 
   // Return true if this type is a declaration to a type in an external
@@ -149,4 +166,37 @@
   lldb::ModuleSP GetModuleForType(const DWARFDIE );
 };
 
+/// Parsed form of all attributes that are relevant for type reconstruction.
+/// Some attributes are relevant for all kinds of types (declaration), while
+/// others are only meaningful to a specific type (is_virtual)
+struct ParsedDWARFTypeAttributes {
+  explicit ParsedDWARFTypeAttributes(const DWARFDIE );
+
+  lldb::AccessType accessibility = lldb::eAccessNone;
+  bool is_artificial = false;
+  bool is_complete_objc_class = false;
+  bool is_explicit = false;
+  bool is_forward_declaration = false;
+  bool is_inline = false;
+  bool is_scoped_enum = false;
+  bool is_vector = false;
+  bool is_virtual = false;
+  clang::StorageClass storage = clang::SC_None;
+  const char *mangled_name = nullptr;
+  lldb_private::ConstString name;
+  lldb_private::Declaration decl;
+  DWARFDIE object_pointer;
+  DWARFFormValue abstract_origin;
+  DWARFFormValue containing_type;
+  DWARFFormValue signature;
+  DWARFFormValue specification;
+  DWARFFormValue type;
+  lldb::LanguageType class_language = lldb::eLanguageTypeUnknown;
+  llvm::Optional byte_size;
+  size_t calling_convention = llvm::dwarf::DW_CC_normal;
+  uint32_t bit_stride = 0;
+  uint32_t byte_stride = 0;
+  uint32_t encoding = 0;
+};
+
 #endif // SymbolFileDWARF_DWARFASTParserClang_h_
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -9,7 +9,6 @@
 #include 
 
 #include "DWARFASTParserClang.h"
-#include "DWARFDIE.h"
 #include "DWARFDebugInfo.h"
 #include "DWARFDeclContext.h"
 #include "DWARFDefines.h"
@@ -232,42 +231,7 @@
   }
 }
 
-namespace {
-/// Parsed form of all attributes that are relevant for type reconstruction.
-/// Some attributes are relevant for all kinds of types (declaration), while
-/// others are only meaningful to a specific type (is_virtual)
-struct ParsedTypeAttributes {
-  explicit ParsedTypeAttributes(const DWARFDIE );
-
-  AccessType accessibility = eAccessNone;
-  bool is_artificial = false;
-  bool is_complete_objc_class = false;
-  bool is_explicit = false;
-  bool is_forward_declaration = false;
-  bool is_inline = false;
-  bool is_scoped_enum = false;
-  bool is_vector = false;
-  bool is_virtual = false;
-  clang::StorageClass storage = clang::SC_None;
-  const char *mangled_name = nullptr;
-  

[Lldb-commits] [lldb] r373662 - [test] Disable TestCustomShell on Linux

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu Oct  3 13:49:55 2019
New Revision: 373662

URL: http://llvm.org/viewvc/llvm-project?rev=373662=rev
Log:
[test] Disable TestCustomShell on Linux

ShellExpandArguments is unimplemented on Linux. I need to come up with
another way to test this on Linux.

Modified:
lldb/trunk/lit/Host/TestCustomShell.test

Modified: lldb/trunk/lit/Host/TestCustomShell.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Host/TestCustomShell.test?rev=373662=373661=373662=diff
==
--- lldb/trunk/lit/Host/TestCustomShell.test (original)
+++ lldb/trunk/lit/Host/TestCustomShell.test Thu Oct  3 13:49:55 2019
@@ -1,5 +1,9 @@
+# This test applies to POSIX.
 # UNSUPPORTED: system-windows
 
+# FIXME: ShellExpandArguments is unimplemented on Linux.
+# UNSUPPORTED: system-linux
+
 # RUN: %clang %S/Inputs/simple.c -g -o %t.out
 # RUN: SHELL=bogus %lldb %t.out -b -o 'run' 2>&1 | FileCheck %s --check-prefix 
ERROR
 # RUN: env -i %lldb %t.out -b -o 'run' 2>&1 | FileCheck %s


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

[Lldb-commits] [PATCH] D68299: [JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationServerLLGS

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

In D68299#1692503 , @labath wrote:

> > FYI, we've root caused some internal lldb asan failures to this patch. I 
> > don't have a repro yet (my lldb tests seem to be failing locally for 
> > unrelated reasons)
>
> This should be fixed with r373572.


Thanks!

> BTW, are you sure those reasons were unrelated? If you have had nearly every 
> test failing then it was very likely due to this. (The reason only a couple 
> of tests failed for you in asan mode is because our CI does not run a large 
> chunk of the lldb tests... :/ )

Yes, tests are still failing locally for me, e.g. 
`Driver/TestConvenienceVariables.test` in `ninja check-lldb-driver` fails:

  Command Output (stderr):
  --
  Traceback (most recent call last):
File "", line 1, in 
  ImportError: No module named lldb.embedded_interpreter
  Traceback (most recent call last):
File "", line 1, in 
  NameError: name 'run_one_line' is not defined
  Traceback (most recent call last):
File "", line 1, in 
  NameError: name 'run_one_line' is not defined
  Traceback (most recent call last):
File "", line 1, in 
  NameError: name 'run_one_line' is not defined
  Traceback (most recent call last):
File "", line 1, in 
  NameError: name 'run_one_line' is not defined
  Traceback (most recent call last):
File "", line 1, in 
  NameError: name 'run_one_line' is not defined
  Traceback (most recent call last):
File "", line 1, in 
  NameError: name 'run_one_line' is not defined
  error: python failed attempting to evaluate 'print(lldb.debugger)'

I think it's a python-related misconfiguration somewhere about not finding the 
lldb module, but I'm not sure when it started -- these tests used to pass, but 
I nuked my cmake tree and apparently didn't reconfigure it correctly.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68299



___
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 marked 2 inline comments as done.
lawrence_danna added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1238-1245
+// OwnedPythonFile::IsValid() chains into Base::IsValid()
+// File::IsValid() is false by default, but for the following classes
+// we want the file to be considered valid as long as the python bits
+// are valid.
+class PresumptivelyValidFile : public File {
+public:
+  bool IsValid() const override { return true; };

labath wrote:
> lawrence_danna wrote:
> > labath wrote:
> > > lawrence_danna wrote:
> > > > labath wrote:
> > > > > How about if OwnedPythonFile defines `IsValid()` as 
> > > > > `IsPythonObjectValid() || Base::IsValid()`. Then PythonIOFile could 
> > > > > redefine it to be simply `IsPythonObjectValid()` without the need for 
> > > > > the extra class?
> > > > If I did that then a SimplePythonFile would still be valid if the file 
> > > > was closed on the python side... seems like the wrong behavior.
> > > Sorry, I meant &&: `IsPythonObjectValid() && Base::IsValid()`. Basically, 
> > > I'm trying to see if there's a reasonable way to reduce the number of 
> > > classes floating around, and this `PresumptivelyValidFile` seems like it 
> > > could be avoided...
> > If i did it that way, with `&&` and no `PresumptivelyValidFile` then the 
> > python IO files would chain into the base `File` class and say they're 
> > invalid.
> > 
> > That's how I coded it up at first, I didn't notice I needed 
> > `PresumptivelyValidFile` until I saw the python IO files coming up as 
> > invalid.
> Yeah, but that's where the second part of this idea comes in. The python IO 
> files can re-override `IsValid()` so that it does *not* chain into the base 
> class, and just calls `IsPythonObjectValid()` from `OwnedPythonFile`. That 
> arrangement seems to make sense to me, though I am willing to be convinced 
> otherwise.
oh, yea that works.


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] D68366: Parametrize scripted ThreadPlans using SBStructuredData

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Target/ThreadPlanPython.h:35
+  ThreadPlanPython(Thread , const char *class_name, 
+   StructuredDataImpl *args_data);
   ~ThreadPlanPython() override;

Why do we need the StructuredDataImpl and not the StructuredData?



Comment at: lldb/source/API/SBThread.cpp:985
   bool resume_immediately) {
   LLDB_RECORD_METHOD(lldb::SBError, SBThread, StepUsingScriptedThreadPlan,
+ (const char *, lldb::SBStructuredData &, bool), 

It appears this is missing a corresponding LLDB_REGISTER macro.



Comment at: lldb/source/API/SBThreadPlan.cpp:78
+   lldb::SBStructuredData _data) {
+  LLDB_RECORD_CONSTRUCTOR(SBThreadPlan, (lldb::SBThread &, const char *,
+ SBStructuredData &),

Same here



Comment at: lldb/source/API/SBThreadPlan.cpp:404
+ SBError ) {
+  LLDB_RECORD_METHOD(lldb::SBThreadPlan, SBThreadPlan,
+ QueueThreadPlanForStepScripted,

This one is fine it seems.



Comment at: lldb/source/API/SBThreadPlan.cpp:409
+
+  if (m_opaque_sp) {
+Status plan_status;

Swap this and have an early return?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68366



___
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 updated this revision to Diff 223080.
lawrence_danna added a comment.

rm class PresumptivelyValidFile


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,20 +972,19 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
+  assert(!PyErr_Occurred());
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
-  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
-return false;
-  if (!object_type.HasAttribute("fileno"))
+  if (!PyObject_IsSubclass(object_type.get(), io_base_class.get())) {
+PyErr_Clear();
 return false;
+  }
 
   return true;
 #endif
@@ -1031,4 +1039,439 @@
   return file;
 }
 
+namespace {
+class GIL {
+public:
+  GIL() {
+

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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1238-1245
+// OwnedPythonFile::IsValid() chains into Base::IsValid()
+// File::IsValid() is false by default, but for the following classes
+// we want the file to be considered valid as long as the python bits
+// are valid.
+class PresumptivelyValidFile : public File {
+public:
+  bool IsValid() const override { return true; };

lawrence_danna wrote:
> labath wrote:
> > lawrence_danna wrote:
> > > labath wrote:
> > > > How about if OwnedPythonFile defines `IsValid()` as 
> > > > `IsPythonObjectValid() || Base::IsValid()`. Then PythonIOFile could 
> > > > redefine it to be simply `IsPythonObjectValid()` without the need for 
> > > > the extra class?
> > > If I did that then a SimplePythonFile would still be valid if the file 
> > > was closed on the python side... seems like the wrong behavior.
> > Sorry, I meant &&: `IsPythonObjectValid() && Base::IsValid()`. Basically, 
> > I'm trying to see if there's a reasonable way to reduce the number of 
> > classes floating around, and this `PresumptivelyValidFile` seems like it 
> > could be avoided...
> If i did it that way, with `&&` and no `PresumptivelyValidFile` then the 
> python IO files would chain into the base `File` class and say they're 
> invalid.
> 
> That's how I coded it up at first, I didn't notice I needed 
> `PresumptivelyValidFile` until I saw the python IO files coming up as invalid.
Yeah, but that's where the second part of this idea comes in. The python IO 
files can re-override `IsValid()` so that it does *not* chain into the base 
class, and just calls `IsPythonObjectValid()` from `OwnedPythonFile`. That 
arrangement seems to make sense to me, though I am willing to be convinced 
otherwise.


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] D68366: Parametrize scripted ThreadPlans using SBStructuredData

[Adrian Prantl via Phabricator via lldb-commits]

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

From a purely mechanical point of view, this lgtm.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68366



___
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.

fixed


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] D68188: allow arbitrary python streams to be converted to SBFile

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

lawrence_danna updated this revision to Diff 223075.
lawrence_danna marked an inline comment as done.
lawrence_danna added a comment.

semicolons


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,20 +972,19 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
+  assert(!PyErr_Occurred());
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
-  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
-return false;
-  if (!object_type.HasAttribute("fileno"))
+  if (!PyObject_IsSubclass(object_type.get(), io_base_class.get())) {
+PyErr_Clear();
 return false;
+  }
 
   return true;
 #endif
@@ -1031,4 +1039,441 @@
   return file;
 }
 
+namespace {
+class GIL 

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

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

lawrence_danna added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1238-1245
+// OwnedPythonFile::IsValid() chains into Base::IsValid()
+// File::IsValid() is false by default, but for the following classes
+// we want the file to be considered valid as long as the python bits
+// are valid.
+class PresumptivelyValidFile : public File {
+public:
+  bool IsValid() const override { return true; };

labath wrote:
> lawrence_danna wrote:
> > labath wrote:
> > > How about if OwnedPythonFile defines `IsValid()` as 
> > > `IsPythonObjectValid() || Base::IsValid()`. Then PythonIOFile could 
> > > redefine it to be simply `IsPythonObjectValid()` without the need for the 
> > > extra class?
> > If I did that then a SimplePythonFile would still be valid if the file was 
> > closed on the python side... seems like the wrong behavior.
> Sorry, I meant &&: `IsPythonObjectValid() && Base::IsValid()`. Basically, I'm 
> trying to see if there's a reasonable way to reduce the number of classes 
> floating around, and this `PresumptivelyValidFile` seems like it could be 
> avoided...
If i did it that way, with `&&` and no `PresumptivelyValidFile` then the python 
IO files would chain into the base `File` class and say they're invalid.

That's how I coded it up at first, I didn't notice I needed 
`PresumptivelyValidFile` until I saw the python IO files coming up as invalid.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1278
+class BinaryPythonFile : public PythonIOFile {
+  friend class PythonFile;
+

labath wrote:
> lawrence_danna wrote:
> > labath wrote:
> > > What is this needed for? I don't see the PythonFile class doing anything 
> > > funny (and it probably shouldn't).
> > python API functions expect you to check for an exception with 
> > PyErr_Occured() and clear it before calling into them again.   If you don't 
> > things usually work out OK, except on a debug build in which an assert will 
> > trip.
> Are you sure you're looking at the right place (the comments tend to move 
> around as you reorganize code)? I was asking why is PythonFile a friend of 
> the BinaryPythonFile class. That doesn't seem relevant to the PyErr_Occured 
> checks...
oh, yea it got moved around. fixed.


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] D68188: allow arbitrary python streams to be converted to SBFile

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

lawrence_danna updated this revision to Diff 223074.
lawrence_danna marked 5 inline comments as done.
lawrence_danna added a comment.

remove unnecessary friends


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,20 +972,19 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
+  assert(!PyErr_Occurred());
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
-  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
-return false;
-  if (!object_type.HasAttribute("fileno"))
+  if (!PyObject_IsSubclass(object_type.get(), io_base_class.get())) {
+PyErr_Clear();
 return false;
+  }
 
   return true;
 #endif
@@ -1031,4 +1039,441 @@
   return file;
 }
 

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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1238-1245
+// OwnedPythonFile::IsValid() chains into Base::IsValid()
+// File::IsValid() is false by default, but for the following classes
+// we want the file to be considered valid as long as the python bits
+// are valid.
+class PresumptivelyValidFile : public File {
+public:
+  bool IsValid() const override { return true; };

lawrence_danna wrote:
> labath wrote:
> > How about if OwnedPythonFile defines `IsValid()` as `IsPythonObjectValid() 
> > || Base::IsValid()`. Then PythonIOFile could redefine it to be simply 
> > `IsPythonObjectValid()` without the need for the extra class?
> If I did that then a SimplePythonFile would still be valid if the file was 
> closed on the python side... seems like the wrong behavior.
Sorry, I meant &&: `IsPythonObjectValid() && Base::IsValid()`. Basically, I'm 
trying to see if there's a reasonable way to reduce the number of classes 
floating around, and this `PresumptivelyValidFile` seems like it could be 
avoided...



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1251
+  PythonIOFile(const PythonFile , bool borrowed)
+  : OwnedPythonFile(file, borrowed){};
+

lawrence_danna wrote:
> labath wrote:
> > no semicolon
> huh?
There's no need to put a semicolon after the body of the constructor.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1278
+class BinaryPythonFile : public PythonIOFile {
+  friend class PythonFile;
+

lawrence_danna wrote:
> labath wrote:
> > What is this needed for? I don't see the PythonFile class doing anything 
> > funny (and it probably shouldn't).
> python API functions expect you to check for an exception with 
> PyErr_Occured() and clear it before calling into them again.   If you don't 
> things usually work out OK, except on a debug build in which an assert will 
> trip.
Are you sure you're looking at the right place (the comments tend to move 
around as you reorganize code)? I was asking why is PythonFile a friend of the 
BinaryPythonFile class. That doesn't seem relevant to the PyErr_Occured 
checks...


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] 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/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1255-1264
+  Status Close() override {
+assert(m_py_obj);
+GIL takeGIL;
+if (m_borrowed)
+  return Flush();
+Take(PyObject_CallMethod(m_py_obj, "close", "()"));
+if (PyErr_Occurred())

labath wrote:
> Hmm... how exactly does this differ from the implementation in 
> `OwnedPythonFile`? Is it only that it does not chain into the base class? 
> Could we use the same trick as for IsValid to reduce the duplication?
they actually have different behavior.   The OwnedPythonFile version chains 
into the base class, but the PythonIOFile version also flushes the stream on 
the //python// side if the file is borrowed.   The idea is that closing any 
file should flush the buffer you've been writing too even if the file is 
borrowed.   That way you don't get any weird surprises where you "close" a file 
and nothing gets written.


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] D68188: allow arbitrary python streams to be converted to SBFile

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

lawrence_danna updated this revision to Diff 223072.
lawrence_danna marked 3 inline comments as done.
lawrence_danna added a comment.

group all the PythonFile support together in the source file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,20 +972,19 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
+  assert(!PyErr_Occurred());
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
-  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
-return false;
-  if (!object_type.HasAttribute("fileno"))
+  if (!PyObject_IsSubclass(object_type.get(), io_base_class.get())) {
+PyErr_Clear();
 return false;
+  }
 
   return true;
 #endif
@@ -1031,4 

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

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

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



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1238-1245
+// OwnedPythonFile::IsValid() chains into Base::IsValid()
+// File::IsValid() is false by default, but for the following classes
+// we want the file to be considered valid as long as the python bits
+// are valid.
+class PresumptivelyValidFile : public File {
+public:
+  bool IsValid() const override { return true; };

labath wrote:
> How about if OwnedPythonFile defines `IsValid()` as `IsPythonObjectValid() || 
> Base::IsValid()`. Then PythonIOFile could redefine it to be simply 
> `IsPythonObjectValid()` without the need for the extra class?
If I did that then a SimplePythonFile would still be valid if the file was 
closed on the python side... seems like the wrong behavior.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1251
+  PythonIOFile(const PythonFile , bool borrowed)
+  : OwnedPythonFile(file, borrowed){};
+

labath wrote:
> no semicolon
huh?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1278
+class BinaryPythonFile : public PythonIOFile {
+  friend class PythonFile;
+

labath wrote:
> What is this needed for? I don't see the PythonFile class doing anything 
> funny (and it probably shouldn't).
python API functions expect you to check for an exception with PyErr_Occured() 
and clear it before calling into them again.   If you don't things usually work 
out OK, except on a debug build in which an assert will trip.


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] D68188: allow arbitrary python streams to be converted to SBFile

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

lawrence_danna added a comment.

cleanup


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] D68188: allow arbitrary python streams to be converted to SBFile

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

lawrence_danna updated this revision to Diff 223071.
lawrence_danna marked 2 inline comments as done.
lawrence_danna added a comment.

cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,20 +972,19 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
+  assert(!PyErr_Occurred());
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
-  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
-return false;
-  if (!object_type.HasAttribute("fileno"))
+  if (!PyObject_IsSubclass(object_type.get(), io_base_class.get())) {
+PyErr_Clear();
 return false;
+  }
 
   return true;
 #endif
@@ -1031,4 +1039,444 @@
   return file;
 }
 
+namespace {
+class GIL {

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

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

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

use LLDB_LOGF and add a logging test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,20 +972,19 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
+  assert(!PyErr_Occurred());
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
-  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
-return false;
-  if (!object_type.HasAttribute("fileno"))
+  if (!PyObject_IsSubclass(object_type.get(), io_base_class.get())) {
+PyErr_Clear();
 return false;
+  }
 
   return true;
 #endif
@@ -1031,4 +1039,444 @@
   return file;
 }
 
+namespace {
+class GIL {
+public:
+  GIL() {

[Lldb-commits] [PATCH] D68293: [android/process list] support showing process arguments

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 223061.
wallace added a comment.

- Encoding differently the arguments. Now it shouldn't have any corner cases
- I'm also sending from the server all the args, including arg0
- In the client, when displaying the process name, I'm using arg0 as fallback 
if the exe path is empty
- Fixed formatting

I still need to finish testing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68293

Files:
  lldb/source/Host/linux/Host.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -39,6 +39,8 @@
 }
 
 const char *ProcessInfo::GetName() const {
+  if (m_executable.GetFilename().IsEmpty())
+return GetArguments().GetArgumentAtIndex(0);
   return m_executable.GetFilename().GetCString();
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1176,6 +1176,18 @@
   response.PutCString("name:");
   response.PutStringAsRawHex8(proc_info.GetExecutableFile().GetCString());
   response.PutChar(';');
+  response.PutCString("args:");
+  response.PutStringAsRawHex8(proc_info.GetArg0());
+  // for (auto it = proc_info.GetArguments().begin(); it !=
+  // proc_info.GetArguments().end(); ++it) {
+  //  response.PutChar('-');
+  // response.PutStringAsRawHex8(it->c_str());
+  //
+  for (auto  : proc_info.GetArguments()) {
+response.PutChar('-');
+response.PutStringAsRawHex8(arg.c_str());
+  }
+  response.PutChar(';');
   const ArchSpec _arch = proc_info.GetArchitecture();
   if (proc_arch.IsValid()) {
 const llvm::Triple _triple = proc_arch.GetTriple();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1199,10 +1199,9 @@
 if (!value.getAsInteger(0, pointer_byte_size))
   ++num_keys_decoded;
   } else if (name.equals("os_version") ||
- name.equals(
- "version")) // Older debugserver binaries used the
- // "version" key instead of
- // "os_version"...
+ name.equals("version")) // Older debugserver binaries used
+ // the "version" key instead of
+ // "os_version"...
   {
 if (!m_os_version.tryParse(value))
   ++num_keys_decoded;
@@ -1927,6 +1926,28 @@
 std::string name;
 extractor.GetHexByteString(name);
 process_info.GetExecutableFile().SetFile(name, FileSpec::Style::native);
+  } else if (name.equals("args")) {
+StringExtractor extractor(value);
+std::stringstream stream(value);
+
+auto extractOneArg = [&](std::string ) {
+  std::string hex_arg;
+  if (!getline(stream, hex_arg, '-'))
+return false;
+  extractor = StringExtractor(hex_arg);
+  extractor.GetHexByteString(arg);
+  return true;
+};
+
+std::string arg;
+if (!extractOneArg(arg))
+  continue;
+process_info.SetArg0(arg);
+process_info.GetArguments().AppendArgument(arg);
+
+while (extractOneArg(arg)) {
+  process_info.GetArguments().AppendArgument(arg);
+}
   } else if (name.equals("cputype")) {
 value.getAsInteger(0, cpu);
   } else if (name.equals("cpusubtype")) {
@@ -2067,7 +2088,7 @@
  !vendor_name.empty()) {
 llvm::Triple triple(llvm::Twine("-") + vendor_name + "-" + os_name);
 if (!environment.empty())
-triple.setEnvironmentName(environment);
+  triple.setEnvironmentName(environment);
 
 assert(triple.getObjectFormat() != llvm::Triple::UnknownObjectFormat);
 assert(triple.getObjectFormat() != llvm::Triple::Wasm);
@@ -2580,7 +2601,7 @@
  * The reply from '?' packet could be as simple as 'S05'. There is no packet
  * which can
  * give us pid and/or tid. Assume pid=tid=1 in such cases.
-*/
+ */
 if (response.IsUnsupportedResponse() && IsConnected()) {
   m_curr_tid = 1;
   return true;
@@ -2753,7 +2774,7 @@
  * be as 

[Lldb-commits] [PATCH] D68316: [Host] Return the user's shell from GetDefaultShell

[Jonas Devlieghere 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 rL373644: [Host] Return the users shell from 
GetDefaultShell (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68316?vs=222882=223060#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68316

Files:
  lldb/trunk/lit/Host/Inputs/simple.c
  lldb/trunk/lit/Host/TestCustomShell.test
  lldb/trunk/source/Host/posix/HostInfoPosix.cpp


Index: lldb/trunk/source/Host/posix/HostInfoPosix.cpp
===
--- lldb/trunk/source/Host/posix/HostInfoPosix.cpp
+++ lldb/trunk/source/Host/posix/HostInfoPosix.cpp
@@ -52,15 +52,19 @@
 };
 } // namespace
 
-llvm::Optional PosixUserIDResolver::DoGetUserName(id_t uid) {
+struct PasswdEntry {
+  std::string username;
+  std::string shell;
+};
+
+static llvm::Optional GetPassword(id_t uid) {
 #ifdef USE_GETPWUID
   // getpwuid_r is missing from android-9
-  // UserIDResolver provides some thread safety by making sure noone calls this
-  // function concurrently, but using getpwuid is ultimately not thread-safe as
-  // we don't know who else might be calling it.
-  struct passwd *user_info_ptr = ::getpwuid(uid);
-  if (user_info_ptr)
-return std::string(user_info_ptr->pw_name);
+  // The caller should provide some thread safety by making sure no one calls
+  // this function concurrently, because using getpwuid is ultimately not
+  // thread-safe as we don't know who else might be calling it.
+  if (auto *user_info_ptr = ::getpwuid(uid))
+return PasswdEntry{user_info_ptr->pw_name, user_info_ptr->pw_shell};
 #else
   struct passwd user_info;
   struct passwd *user_info_ptr = _info;
@@ -69,12 +73,18 @@
   if (::getpwuid_r(uid, _info, user_buffer, user_buffer_size,
_info_ptr) == 0 &&
   user_info_ptr) {
-return std::string(user_info_ptr->pw_name);
+return PasswdEntry{user_info_ptr->pw_name, user_info_ptr->pw_shell};
   }
 #endif
   return llvm::None;
 }
 
+llvm::Optional PosixUserIDResolver::DoGetUserName(id_t uid) {
+  if (llvm::Optional password = GetPassword(uid))
+return password->username;
+  return llvm::None;
+}
+
 llvm::Optional PosixUserIDResolver::DoGetGroupName(id_t gid) {
 #ifndef __ANDROID__
   char group_buffer[PATH_MAX];
@@ -113,7 +123,13 @@
 
 uint32_t HostInfoPosix::GetEffectiveGroupID() { return getegid(); }
 
-FileSpec HostInfoPosix::GetDefaultShell() { return FileSpec("/bin/sh"); }
+FileSpec HostInfoPosix::GetDefaultShell() {
+  if (const char *v = ::getenv("SHELL"))
+return FileSpec(v);
+  if (llvm::Optional password = GetPassword(::geteuid()))
+return FileSpec(password->shell);
+  return FileSpec("/bin/sh");
+}
 
 bool HostInfoPosix::ComputeSupportExeDirectory(FileSpec _spec) {
   return ComputePathRelativeToLibrary(file_spec, "/bin");
Index: lldb/trunk/lit/Host/Inputs/simple.c
===
--- lldb/trunk/lit/Host/Inputs/simple.c
+++ lldb/trunk/lit/Host/Inputs/simple.c
@@ -0,0 +1 @@
+int main(int argc, char const *argv[]) { return 0; }
Index: lldb/trunk/lit/Host/TestCustomShell.test
===
--- lldb/trunk/lit/Host/TestCustomShell.test
+++ lldb/trunk/lit/Host/TestCustomShell.test
@@ -0,0 +1,8 @@
+# UNSUPPORTED: system-windows
+
+# RUN: %clang %S/Inputs/simple.c -g -o %t.out
+# RUN: SHELL=bogus %lldb %t.out -b -o 'run' 2>&1 | FileCheck %s --check-prefix 
ERROR
+# RUN: env -i %lldb %t.out -b -o 'run' 2>&1 | FileCheck %s
+
+# ERROR: error: shell expansion failed
+# CHECK-NOT: error: shell expansion failed


Index: lldb/trunk/source/Host/posix/HostInfoPosix.cpp
===
--- lldb/trunk/source/Host/posix/HostInfoPosix.cpp
+++ lldb/trunk/source/Host/posix/HostInfoPosix.cpp
@@ -52,15 +52,19 @@
 };
 } // namespace
 
-llvm::Optional PosixUserIDResolver::DoGetUserName(id_t uid) {
+struct PasswdEntry {
+  std::string username;
+  std::string shell;
+};
+
+static llvm::Optional GetPassword(id_t uid) {
 #ifdef USE_GETPWUID
   // getpwuid_r is missing from android-9
-  // UserIDResolver provides some thread safety by making sure noone calls this
-  // function concurrently, but using getpwuid is ultimately not thread-safe as
-  // we don't know who else might be calling it.
-  struct passwd *user_info_ptr = ::getpwuid(uid);
-  if (user_info_ptr)
-return std::string(user_info_ptr->pw_name);
+  // The caller should provide some thread safety by making sure no one calls
+  // this function concurrently, because using getpwuid is ultimately not
+  // thread-safe as we don't know who else might be calling it.
+  if (auto *user_info_ptr = 

[Lldb-commits] [lldb] r373644 - [Host] Return the user's shell from GetDefaultShell

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu Oct  3 11:29:01 2019
New Revision: 373644

URL: http://llvm.org/viewvc/llvm-project?rev=373644=rev
Log:
[Host] Return the user's shell from GetDefaultShell

LLDB handles shell expansion by running lldb-argdumper under a shell.
Currently, this is always /bin/sh on POSIX. This potentially leads to
different behavior between lldb and the user's current shell. Here's an
example of different expansions between shells:

$ /bin/bash -c 'echo -config={Options:[{key:foo_key,value:foo_value}]}'
-config={Options:[key:foo_key]} -config={Options:[value:foo_value]}

$ /bin/zsh -c 'echo -config={Options:[{key:foo_key,value:foo_value}]}'
zsh:1: no matches found: -config={Options:[key:foo_key]}

$ /bin/sh -c 'echo -config={Options:[{key:foo_key,value:foo_value}]}'
-config={Options:[key:foo_key]} -config={Options:[value:foo_value]}

$ /bin/fish -c 'echo -config={Options:[{key:foo_key,value:foo_value}]}'
-config=Options:[key:foo_key] -config=Options:[value:foo_value]

To reduce surprises, this patch returns the user's current shell. It
first looks at the SHELL environment variable. If that isn't set, it'll
ask for the user's default shell. Only if that fails, we'll fallback to
/bin/sh, which should always be available.

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

Added:
lldb/trunk/lit/Host/
lldb/trunk/lit/Host/Inputs/
lldb/trunk/lit/Host/Inputs/simple.c
lldb/trunk/lit/Host/TestCustomShell.test
Modified:
lldb/trunk/source/Host/posix/HostInfoPosix.cpp

Added: lldb/trunk/lit/Host/Inputs/simple.c
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Host/Inputs/simple.c?rev=373644=auto
==
--- lldb/trunk/lit/Host/Inputs/simple.c (added)
+++ lldb/trunk/lit/Host/Inputs/simple.c Thu Oct  3 11:29:01 2019
@@ -0,0 +1 @@
+int main(int argc, char const *argv[]) { return 0; }

Added: lldb/trunk/lit/Host/TestCustomShell.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Host/TestCustomShell.test?rev=373644=auto
==
--- lldb/trunk/lit/Host/TestCustomShell.test (added)
+++ lldb/trunk/lit/Host/TestCustomShell.test Thu Oct  3 11:29:01 2019
@@ -0,0 +1,8 @@
+# UNSUPPORTED: system-windows
+
+# RUN: %clang %S/Inputs/simple.c -g -o %t.out
+# RUN: SHELL=bogus %lldb %t.out -b -o 'run' 2>&1 | FileCheck %s --check-prefix 
ERROR
+# RUN: env -i %lldb %t.out -b -o 'run' 2>&1 | FileCheck %s
+
+# ERROR: error: shell expansion failed
+# CHECK-NOT: error: shell expansion failed

Modified: lldb/trunk/source/Host/posix/HostInfoPosix.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/HostInfoPosix.cpp?rev=373644=373643=373644=diff
==
--- lldb/trunk/source/Host/posix/HostInfoPosix.cpp (original)
+++ lldb/trunk/source/Host/posix/HostInfoPosix.cpp Thu Oct  3 11:29:01 2019
@@ -52,15 +52,19 @@ protected:
 };
 } // namespace
 
-llvm::Optional PosixUserIDResolver::DoGetUserName(id_t uid) {
+struct PasswdEntry {
+  std::string username;
+  std::string shell;
+};
+
+static llvm::Optional GetPassword(id_t uid) {
 #ifdef USE_GETPWUID
   // getpwuid_r is missing from android-9
-  // UserIDResolver provides some thread safety by making sure noone calls this
-  // function concurrently, but using getpwuid is ultimately not thread-safe as
-  // we don't know who else might be calling it.
-  struct passwd *user_info_ptr = ::getpwuid(uid);
-  if (user_info_ptr)
-return std::string(user_info_ptr->pw_name);
+  // The caller should provide some thread safety by making sure no one calls
+  // this function concurrently, because using getpwuid is ultimately not
+  // thread-safe as we don't know who else might be calling it.
+  if (auto *user_info_ptr = ::getpwuid(uid))
+return PasswdEntry{user_info_ptr->pw_name, user_info_ptr->pw_shell};
 #else
   struct passwd user_info;
   struct passwd *user_info_ptr = _info;
@@ -69,12 +73,18 @@ llvm::Optional PosixUserIDR
   if (::getpwuid_r(uid, _info, user_buffer, user_buffer_size,
_info_ptr) == 0 &&
   user_info_ptr) {
-return std::string(user_info_ptr->pw_name);
+return PasswdEntry{user_info_ptr->pw_name, user_info_ptr->pw_shell};
   }
 #endif
   return llvm::None;
 }
 
+llvm::Optional PosixUserIDResolver::DoGetUserName(id_t uid) {
+  if (llvm::Optional password = GetPassword(uid))
+return password->username;
+  return llvm::None;
+}
+
 llvm::Optional PosixUserIDResolver::DoGetGroupName(id_t gid) {
 #ifndef __ANDROID__
   char group_buffer[PATH_MAX];
@@ -113,7 +123,13 @@ uint32_t HostInfoPosix::GetEffectiveUser
 
 uint32_t HostInfoPosix::GetEffectiveGroupID() { return getegid(); }
 
-FileSpec HostInfoPosix::GetDefaultShell() { return FileSpec("/bin/sh"); }
+FileSpec HostInfoPosix::GetDefaultShell() {
+  if (const char *v = ::getenv("SHELL"))
+return FileSpec(v);
+  if 

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

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

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

assert instead of raise


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,20 +972,19 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
+  assert(!PyErr_Occurred());
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
-  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
-return false;
-  if (!object_type.HasAttribute("fileno"))
+  if (!PyObject_IsSubclass(object_type.get(), io_base_class.get())) {
+PyErr_Clear();
 return false;
+  }
 
   return true;
 #endif
@@ -1031,4 +1039,445 @@
   return file;
 }
 
+namespace {
+class GIL {
+public:
+  GIL() {
+m_state 

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

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

lawrence_danna updated this revision to Diff 223056.
lawrence_danna marked an inline comment as done.
lawrence_danna added a comment.

anonymous namespace, and a windows fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,20 +972,19 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
   PythonObject io_base_class = io_dict.GetItemForKey(PythonString("IOBase"));
+  assert(!PyErr_Occurred());
 
   PythonObject object_type(PyRefType::Owned, PyObject_Type(py_obj));
 
-  if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
-return false;
-  if (!object_type.HasAttribute("fileno"))
+  if (!PyObject_IsSubclass(object_type.get(), io_base_class.get())) {
+PyErr_Clear();
 return false;
+  }
 
   return true;
 #endif
@@ -1031,4 +1039,445 @@
   return file;
 

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

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

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

no_debug_info_test -> NO_DEBUG_INFO_TESTCASE


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,9 +972,7 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
@@ -975,8 +982,6 @@
 
   if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
 return false;
-  if (!object_type.HasAttribute("fileno"))
-return false;
 
   return true;
 #endif
@@ -1031,4 +1036,429 @@
   return file;
 }
 
+class GIL {
+public:
+  GIL() {
+m_state = PyGILState_Ensure();
+assert(!PyErr_Occurred());
+  }
+  ~GIL() { PyGILState_Release(m_state); }
+
+protected:
+  PyGILState_STATE m_state;
+};
+
+const char *PythonException::toCString() const {
+  if (m_repr_bytes) {
+return 

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

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

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

We need to obsessively check PyErr_Occurred() before calling
into the next python API, even when we'd just catch it a 
little later.   Python APIs will generally work if you don't,
but it's undefined behavior to do it and debug builds of python
will assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,9 +972,7 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
@@ -975,8 +982,6 @@
 
   if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
 return false;
-  if (!object_type.HasAttribute("fileno"))
-return false;
 
   return true;
 #endif
@@ -1031,4 +1036,429 @@
   return file;
 }
 
+class GIL {
+public:
+  GIL() {
+m_state = PyGILState_Ensure();
+

[Lldb-commits] [PATCH] D68361: [dsymutil] Tablegenify option parsing

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373622: [dsymutil] Tablegenify option parsing (authored by 
JDevlieghere, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D68361?vs=222934=223042#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68361

Files:
  llvm/trunk/test/tools/dsymutil/cmdline.test
  llvm/trunk/tools/dsymutil/CMakeLists.txt
  llvm/trunk/tools/dsymutil/Options.td
  llvm/trunk/tools/dsymutil/dsymutil.cpp

Index: llvm/trunk/test/tools/dsymutil/cmdline.test
===
--- llvm/trunk/test/tools/dsymutil/cmdline.test
+++ llvm/trunk/test/tools/dsymutil/cmdline.test
@@ -1,21 +1,19 @@
 RUN: dsymutil -help 2>&1 | FileCheck --check-prefix=HELP %s
 HELP: OVERVIEW: manipulate archived DWARF debug symbol files.
-HELP: USAGE: dsymutil{{[^ ]*}} [options] 
+HELP: USAGE: {{.*}}dsymutil{{[^ ]*}} [options] 
 HELP-NOT: -reverse-iterate
-HELP: Color Options
-HELP: -color
-HELP: Specific Options:
+HELP: Dsymutil Options:
 HELP: -accelerator
-HELP: -arch=
+HELP: -arch 
 HELP: -dump-debug-map
 HELP: -flat
 HELP: -minimize
 HELP: -no-odr
 HELP: -no-output
 HELP: -no-swiftmodule-timestamp
-HELP: -num-threads=
-HELP: -o=
-HELP: -oso-prepend-path=
+HELP: -num-threads 
+HELP: -oso-prepend-path 
+HELP: -o 
 HELP: -papertrail
 HELP: -symbol-map
 HELP: -symtab
Index: llvm/trunk/tools/dsymutil/Options.td
===
--- llvm/trunk/tools/dsymutil/Options.td
+++ llvm/trunk/tools/dsymutil/Options.td
@@ -0,0 +1,146 @@
+include "llvm/Option/OptParser.td"
+
+class F: Flag<["--", "-"], name>;
+
+def grp_general : OptionGroup<"Dsymutil">, HelpText<"Dsymutil Options">;
+
+def help: F<"help">,
+  HelpText<"Prints this help output.">,
+  Group;
+def: Flag<["-"], "h">,
+  Alias,
+  HelpText<"Alias for --help">,
+  Group;
+
+def version: F<"version">,
+  HelpText<"Prints the dsymutil version.">,
+  Group;
+def: Flag<["-"], "v">,
+  Alias,
+  HelpText<"Alias for --version">,
+  Group;
+
+def verbose: F<"verbose">,
+  HelpText<"Enable verbose mode.">,
+  Group;
+
+def verify: F<"verify">,
+  HelpText<"Run the DWARF verifier on the linked DWARF debug info.">,
+  Group;
+
+def no_output: F<"no-output">,
+  HelpText<"Do the link in memory, but do not emit the result file.">,
+  Group;
+
+def no_swiftmodule_timestamp: F<"no-swiftmodule-timestamp">,
+  HelpText<"Don't check timestamp for swiftmodule files.">,
+  Group;
+
+def no_odr: F<"no-odr">,
+  HelpText<"Do not use ODR (One Definition Rule) for type uniquing.">,
+  Group;
+
+def dump_debug_map: F<"dump-debug-map">,
+  HelpText<"Parse and dump the debug map to standard output. Not DWARF link will take place.">,
+  Group;
+
+def yaml_input: F<"y">,
+  HelpText<"Treat the input file is a YAML debug map rather than a binary.">,
+  Group;
+
+def papertrail: F<"papertrail">,
+  HelpText<"Embed warnings in the linked DWARF debug info.">,
+  Group;
+
+def assembly: F<"S">,
+  HelpText<"Output textual assembly instead of a binary dSYM companion file.">,
+  Group;
+
+def symtab: F<"symtab">,
+  HelpText<"Dumps the symbol table found in executable or object file(s) and exits.">,
+  Group;
+def: Flag<["-"], "s">,
+  Alias,
+  HelpText<"Alias for --symtab">,
+  Group;
+
+def flat: F<"flat">,
+  HelpText<"Produce a flat dSYM file (not a bundle).">,
+  Group;
+def: Flag<["-"], "f">,
+  Alias,
+  HelpText<"Alias for --flat">,
+  Group;
+
+def minimize: F<"minimize">,
+  HelpText<"When used when creating a dSYM file with Apple accelerator tables, "
+   "this option will suppress the emission of the .debug_inlines, "
+   ".debug_pubnames, and .debug_pubtypes sections since dsymutil "
+   "has better equivalents: .apple_names and .apple_types. When used in "
+   "conjunction with --update option, this option will cause redundant "
+   "accelerator tables to be removed.">,
+  Group;
+def: Flag<["-"], "z">,
+  Alias,
+  HelpText<"Alias for --minimize">,
+  Group;
+
+def update: F<"update">,
+  HelpText<"Updates existing dSYM files to contain the latest accelerator tables and other DWARF optimizations.">,
+  Group;
+def: Flag<["-"], "u">,
+  Alias,
+  HelpText<"Alias for --update">,
+  Group;
+
+def output: Separate<["--", "-"], "o">,
+  MetaVarName<"">,
+  HelpText<"Specify the output file. Defaults to .dwarf">,
+  Group;
+def: Separate<["-"], "out">,
+  Alias,
+  HelpText<"Alias for --o">,
+  Group;
+def: Joined<["--", "-"], "out=">, Alias;
+def: Joined<["--", "-"], "o=">, Alias;
+
+def oso_prepend_path: Separate<["--", "-"], "oso-prepend-path">,
+  MetaVarName<"">,
+  HelpText<"Specify a directory to prepend to the paths of object files.">,
+  Group;
+def: Joined<["--", "-"], "oso-prepend-path=">, Alias;
+
+def symbolmap: Separate<["--", "-"], "symbol-map">,
+  MetaVarName<"">,
+  HelpText<"Updates the existing dSYMs 

[Lldb-commits] [PATCH] D68361: [dsymutil] Tablegenify option parsing

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D68361#1692988 , @thegameg wrote:

> I noticed a bunch of explicit `llvm::` prefixes like `llvm::Error`, 
> `llvm::StringRef`, etc. Did you intentionally leave that there?


No, that's definitely unintentional. I'll fix the inconsistency in a follow-up 
commit! :-)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68361



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

[Lldb-commits] [PATCH] D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

[Gabor Marton via Phabricator via lldb-commits]

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

LGTM!


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

https://reviews.llvm.org/D68326



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

Re: [Lldb-commits] [lldb] r373572 - Fix a use-after-free in GDBRemoteCommunicationServerLLGS

[Jonas Devlieghere via lldb-commits]

Thanks Pavel!

On Thu, Oct 3, 2019 at 12:57 AM Pavel Labath via lldb-commits
 wrote:
>
> Author: labath
> Date: Thu Oct  3 00:59:26 2019
> New Revision: 373572
>
> URL: http://llvm.org/viewvc/llvm-project?rev=373572=rev
> Log:
> Fix a use-after-free in GDBRemoteCommunicationServerLLGS
>
> Although it's called "GetString", StreamString::GetString actually
> returns a StringRef. Creating a json object with a StringRef does not
> make a copy, which means the StringRef will be dangling as soon as the
> underlying stream is destroyed. Add a .str() to force the json object to
> hold a copy of the string.
>
> This fixes nearly every test on linux.
>
> Modified:
> 
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>
> Modified: 
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=373572=373571=373572=diff
> ==
> --- 
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>  (original)
> +++ 
> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
>  Thu Oct  3 00:59:26 2019
> @@ -462,7 +462,8 @@ GetRegistersAsJSON(NativeThreadProtocol
>  WriteRegisterValueInHexFixedWidth(stream, reg_ctx, *reg_info_p,
>_value, lldb::eByteOrderBig);
>
> -register_object.try_emplace(llvm::to_string(reg_num), 
> stream.GetString());
> +register_object.try_emplace(llvm::to_string(reg_num),
> +stream.GetString().str());
>}
>
>return register_object;
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68361: [dsymutil] Tablegenify option parsing

[Frederic Riss via Phabricator via lldb-commits]

friss accepted this revision.
friss added a comment.

This seems a little heavyweight, but it reads nicely and gets rid of global 
state. Go for it.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68361



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

[Lldb-commits] [PATCH] D68361: [dsymutil] Tablegenify option parsing

[Francis Visoiu Mistrih via Phabricator via lldb-commits]

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

I noticed a bunch of explicit `llvm::` prefixes like `llvm::Error`, 
`llvm::StringRef`, etc. Did you intentionally leave that there?

Otherwise, this LGTM, thanks for the nice cleanup!




Comment at: llvm/tools/dsymutil/dsymutil.cpp:161
+"standard input cannot be used as input for a dSYM update.",
+inconvertibleErrorCode());
+  }

Would it make sense for all these error codes to be 
`std::errc::invalid_argument`?



Comment at: llvm/tools/dsymutil/dsymutil.cpp:233
+  getInputs(Args, Options.LinkOptions.Update)) {
+Options.InputFiles = *InputFiles;
+  } else {

`=  std::move(*InputFiles);`?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68361



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

[Lldb-commits] [PATCH] D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 223007.
teemperor marked 2 inline comments as done.
teemperor added a comment.

- Addressed feedback (Thanks Gabor, Adrian & Shafik!)


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

https://reviews.llvm.org/D68326

Files:
  clang/include/clang/AST/ExternalASTMerger.h
  clang/lib/AST/ExternalASTMerger.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/TestLibCxxModernTypeLookup.py
  
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/libcxx/main.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -180,108 +180,20 @@
   return ret;
 }
 
-namespace {
-/// This class walks an AST and ensures that all DeclContexts defined inside the
-/// current source file are properly complete.
-///
-/// This is used to ensure that persistent types defined in the current source
-/// file migrate completely to the persistent AST context before they are
-/// reused.  If that didn't happen, it would be impoossible to complete them
-/// because their origin would be gone.
-///
-/// The stragtegy used by this class is to check the SourceLocation (to be
-/// specific, the FileID) and see if it's the FileID for the current expression.
-/// Alternate strategies could include checking whether an ExternalASTMerger,
-/// set up to not have the current context as a source, can find an original for
-/// the type.
-class Completer : public clang::RecursiveASTVisitor {
-private:
-  /// Used to import Decl contents
-  clang::ASTImporter _exporter;
-  /// The file that's going away
-  clang::FileID m_file;
-  /// Visited Decls, to avoid cycles
-  llvm::DenseSet m_completed;
-
-  bool ImportAndCheckCompletable(clang::Decl *decl) {
-llvm::Expected imported_decl = m_exporter.Import(decl);
-if (!imported_decl) {
-  llvm::consumeError(imported_decl.takeError());
-  return false;
-}
-if (m_completed.count(decl))
-  return false;
-if (!llvm::isa(decl))
-  return false;
-const clang::SourceLocation loc = decl->getLocation();
-if (!loc.isValid())
-  return false;
-const clang::FileID file =
-m_exporter.getFromContext().getSourceManager().getFileID(loc);
-if (file != m_file)
-  return false;
-// We are assuming the Decl was parsed in this very expression, so it
-// should not have external storage.
-lldbassert(!llvm::cast(decl)->hasExternalLexicalStorage());
-return true;
-  }
-
-  void Complete(clang::Decl *decl) {
-m_completed.insert(decl);
-auto *decl_context = llvm::cast(decl);
-llvm::Expected imported_decl = m_exporter.Import(decl);
-if (!imported_decl) {
-  llvm::consumeError(imported_decl.takeError());
-  return;
-}
-m_exporter.CompleteDecl(decl);
-for (Decl *child : decl_context->decls())
-  if (ImportAndCheckCompletable(child))
-Complete(child);
-  }
-
-  void MaybeComplete(clang::Decl *decl) {
-if (ImportAndCheckCompletable(decl))
-  Complete(decl);
-  }
-
-public:
-  Completer(clang::ASTImporter , clang::FileID file)
-  : m_exporter(exporter), m_file(file) {}
-
-  // Implements the RecursiveASTVisitor's core API.  It is called on each Decl
-  // that the RecursiveASTVisitor encounters, and returns true if the traversal
-  // should continue.
-  bool VisitDecl(clang::Decl *decl) {
-MaybeComplete(decl);
-return true;
-  }
-};
-} // namespace
-
-static void CompleteAllDeclContexts(clang::ASTImporter ,
-clang::FileID file, clang::QualType root) {
-  clang::QualType canonical_type = root.getCanonicalType();
-  if (clang::TagDecl *tag_decl = canonical_type->getAsTagDecl()) {
-Completer(exporter, file).TraverseDecl(tag_decl);
-  } else if (auto interface_type = llvm::dyn_cast(
- canonical_type.getTypePtr())) {
-Completer(exporter, file).TraverseDecl(interface_type->getDecl());
-  } else {
-Completer(exporter, file).TraverseType(canonical_type);
-  }
-}
-
 static clang::QualType ExportAllDeclaredTypes(
-clang::ExternalASTMerger , clang::ASTContext ,
-clang::FileManager _file_manager,
+clang::ExternalASTMerger _merger, clang::ExternalASTMerger ,
+clang::ASTContext , clang::FileManager _file_manager,
 const clang::ExternalASTMerger::OriginMap _origin_map,
 clang::FileID file, clang::QualType root) {
+  // Mark the source as temporary to make sure all declarations from the
+  // AST are exported. Also add the parent_merger as the merger into the
+  // source AST so that the merger can track back any declarations from
+  // the persistent ASTs we used as sources.
   

[Lldb-commits] [PATCH] D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST

[Raphael Isemann via Phabricator via lldb-commits]

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



Comment at: clang/lib/AST/DeclBase.cpp:95
  DeclContext *Parent, std::size_t Extra) {
+  if (!(!Parent || >getParentASTContext() == )) {
+llvm::errs() << Parent << " | " << >getParentASTContext()

martong wrote:
> Left over debug printout?
Whoops, thanks.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:183
 
-namespace {
-/// This class walks an AST and ensures that all DeclContexts defined inside 
the

shafik wrote:
> So we are removing this b/c we are now doing a minimal import from the 
> temporary source and then in the next patch you will change that?
We always did a minimal import here and the completer just tried to turn this a 
non-minimal import by iterating over the AST and then copying a bunch of 
declarations over. It currently only seems to cause errors in the existing 
tests. Also, we anyway don't test the feature it's actually supposed to 
implement anywhere in LLLDB as this is a bug in the original LLDB. So this just 
removes this code until we can properly implement and test it (which will 
hopefully just be turning off the MinimalImport when importing from temporary 
sources).


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68326



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

[Lldb-commits] [PATCH] D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect.

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Ok, let's try this once more. I took another look at the patch, and I have some 
more tiny comments, but nothing too major.

I'm sorry this took so long. When the topic of parsing both symbol sections 
came up, I was happy that it was implemented in a way that applies outside of 
minidebuginfo too, because it seemed like it could be a useful source of 
additional information. However, that turned into a huge rabbit hole, which 
isn't really relevant for the thing you're trying to accomplish. If we've 
managed to come this far without needing to consult both symbol tables, I think 
we'll manage to wait a couple years longer.




Comment at: lldb/lit/Modules/ELF/minidebuginfo-no-lzma.yaml:4
+# This test checks that a warning is printed when we're trying
+# to decompress a. gnu_debugdata section when no LZMA support was compiled in.
+

s/a. /a ./



Comment at: lldb/source/Host/common/LZMA.cpp:1
+//===--- Compression.cpp - Compression implementation 
-===//
+//

Fix header.



Comment at: lldb/source/Host/common/LZMA.cpp:69-72
+  if (InputBuffer.size() == 0)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "size of xz-compressed blob cannot be 0");
+

This is redundant, as you're comparing the size against LZMA_STREAM_HEADER_SIZE 
anyway.



Comment at: lldb/source/Host/common/LZMA.cpp:84
+  lzma_ret xzerr = lzma_stream_footer_decode(
+  , InputBuffer.data() + InputBuffer.size() - 
LZMA_STREAM_HEADER_SIZE);
+  if (xzerr != LZMA_OK) {

Maybe `InputBuffer.take_back(LZMA_STREAM_HEADER_SIZE).data()` ?



Comment at: lldb/source/Host/common/LZMA.cpp:104-105
+  lzma_index_buffer_decode(, , nullptr,
+   InputBuffer.data() + InputBuffer.size() -
+   LZMA_STREAM_HEADER_SIZE - 
opts.backward_size,
+   , InputBuffer.size());

same here.



Comment at: lldb/source/Host/common/LZMA.cpp:125-128
+  if (InputBuffer.empty())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "size of xz-compressed blob cannot be 0");
+

This is redundant, as getUncompressedSize checks that anyway.



Comment at: lldb/source/Host/common/LZMA.cpp:137
+  // Decompress xz buffer to buffer.
+  uint64_t memlimit(UINT64_MAX);
+  size_t inpos = 0;

` = UINT64_MAX;`. This looks too much like a function declaration, and is not 
consistent with the code below.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1854
+  gdd_objfile_section_list->FindSectionByType(
+  eSectionTypeELFSymbolTable, true)) {
+SectionSP module_section_sp = unified_section_list.FindSectionByType(

jankratochvil wrote:
> I miss here comparing that the sections inside `.gnu_debugdata` file match 
> sections in the outer file:
> ```
> jankratochvil: "hm I was under the impression that section number should 
> remain the same in regular and split debug files" - maybe you are right but 
> then we could just also compare the two section tables and if they differ 
> then reject whole .gnu_debugdata.  And then we do not have to compare section 
> names but it is enough to compare section indexes.
> labath: "right but then we could just also compare the two section tables and 
> if they differ then reject whole .gnu_debugdata."
> labath: I would like that ^
> ```
> Also I do not see here handling `.strtab`, why does it work without moving 
> also `.strtab`?
I'd leave that out for now. This is similar stuff to what is already being done 
in SymbolVendorELF, which should also probably check for section matches, and 
would be best handled together.

The reason that this works without moving `.strtab` is because the parsing is 
still done within the context of the owning object file. This whole section 
merging business is a pretty big mess, and in dire need of some TLC, but @kwk 
is not making that worse, so I wouldn't want to deal with that here...



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2725
+// minidebuginfo case we parse .dynsym when there's a .gnu_debuginfo
+// section, nomatter if .symtab was already parsed or not.
+if (!symtab ||

You can add that this is because minidebuginfo normally removes the symtab 
symbols which have their matching dynsym counterparts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791



___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D68354: [platform process list] add a flag for showing the processes of all users

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think the fact that all of our options *must* have a short version is a 
pretty serious deficiency in our option parsing system. It results in a lot of 
unintuitive options that nobody uses because they can't guess what they stand 
for. But anyway, `-x` seems as good as any, as all the good alternatives are 
already taken.

For testing this, I think the best option would be something similar to the 
gdb-client suite (test/testcases/functionalities/gdb_remote_client), which 
allows you to mock the remote end of a connection. It is currently geared 
toward talking to the gdb server (and not the platform), but as the underlying 
protocol is the same, I am hoping that it can be repurposed to this use case 
fairly easily. I am imagining a flow where you create a mock server, configure 
it to respond appropriately to any packet that lldb issues, and then run e.g. 
"platform select remote-linux", "platform connect connect://localhost:%d", 
"platform proces list --all-users".

Setting that up is a bit overkill for a simple patch as this one, but as it 
looks like you're going to be doing a lot of work in this area in the near 
future, I think it makes sense to set up some testing infrastructure early.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68354



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

[Lldb-commits] [PATCH] D68289: [lldb-server/android] Show more processes by relaxing some checks

[Pavel Labath via Phabricator via lldb-commits]

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

Looks good. I don't think there's a reasonable way to test this, as these will 
fail only when running stuff under a different user, or on a system with a 
paranoid security module.




Comment at: lldb/source/Host/linux/Host.cpp:193
   auto BufferOrError = getProcFile(pid, "environ");
-  if (!BufferOrError)
-return false;
+  if (!BufferOrError) {
+return;

We usually don't put braces around short statements like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289



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

[Lldb-commits] [PATCH] D68291: [process list] make the TRIPLE column wider

[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. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68291



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

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D67994#1691641 , @shafik wrote:

> In D67994#1683934 , @labath wrote:
>
> > IOW, I was not saying you should use "image dump ast" to write the test you 
> > wanted to write. I was merely saying that we should try to make "lldb-test 
> > -dump-ast" use the same dumping code as "image dump ast" does (assuming the 
> > latter outputs the kind of data that you need, but it seems to me that it 
> > does...). "image dump ast" can remain lazy, and only show the things that 
> > have been parsed so far (which is also useful sometimes), while "lldb-test" 
> > can do whatever it takes to parse everything (I would hope that is merely 
> > calling Module::ParseAllDebugSymbols).
>
>
> In order to reuse the functionality we would need to import the complete 
> clang ASTs to the scratch AST context and that would end up being a lot more 
> work then the current approach. So I stuck with the current approach.


I am sorry, all of these ast contexts are still fairly confusing to me, so I am 
not sure what this actually means. Can you elaborate?


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

https://reviews.llvm.org/D67994



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

[Lldb-commits] [PATCH] D67994: Modify lldb-test to print out ASTs from symbol file

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Maybe this is my fault since I'm the one who introduced the first bunch of 
arguments here IIRC, but anyway, I have a feeling all of these dumping options 
are getting out of hand. Looking at the argument list, you'd expect that 
-dump-ast, and -dump-clang-ast do something completely different, but they 
actually print the exact same AST, only one allows you to print a subset of it, 
while the other one doesn't.

Given that the ast dumping is currently incompatible with all/most of the other 
dumping options anyway, maybe we should turn over a clean slate, and implement 
a new subcommand specifically dedicated to dumping clang ast (as opposed to the 
internal lldb representations of types/functions/..., which is what the 
"symbols" subcommand started out as, and which is what most of its options are 
dedicated to).

OR, and this would-be super cool, if it was actually possible, we could try to 
make ast-dumping compatible with the existing searching options.  Currently, if 
you type `lldb-test symbols -find=type -name=foo` it will search for types 
named "foo", and print some summary of the lldb object representing that type. 
What if we made it so that adding `-dump-ast` to that command caused the tool 
to dump the *AST* of the types it has found (e.g., in addition to the previous 
summary)? I think that would make the behavior of the tool most natural to a 
new user, but I am not sure whether that would actually fit in with your goals 
here...


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

https://reviews.llvm.org/D67994



___
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.

Ok, I think that the major issues are out of the way now. I've done a more 
careful pass over the file, and I think this is looking pretty good. I am 
particularly happy with how you've implemented (and tested) the interaction 
with python exceptions. A bunch of additional comments inline, but none really 
that important.




Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:347-348
+
+@add_test_categories(['pyapi'])
+@no_debug_info_test
+def test_sbfile_write_borrowed(self):

Actually, I'm pretty sure that this category annotation is unnecessary, as the 
`python_api` contains a `.categories` file specifying this category. Also, the 
`@no_debug_info_test` thingy might be better achieved by setting 
`NO_DEBUG_INFO_TESTCASE = True` class property.



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:477-478
+status = self.debugger.SetOutputFile(sbf)
+if status.Fail():
+raise Exception(status)
+self.handleCmd('script 2+2')

`self.assertTrue(status.Succes())`. If you're doing this because you want a 
better error message or something, feel free to implement something like 
`assertSuccess` or the like. I'd be happy to use that, as I want better error 
messages too.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1039
 
+class GIL {
+public:

All of these file-local classes (unless you choose to move them to a separate 
file) should go into an anonymous namespace. See 
 on the 
recommended way to do that.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1069-1071
+  if (log) {
+log->Printf("%s failed with exception: %s", caller, toCString());
+  }

We're using LLDB_LOG or LLDB_LOGF these days. The first one is prefereed if you 
have complex values that can't be displayed by printf natively (e.g. StringRef).



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1176
+
+// A SimplyPythonFile is a OwnedPythonFile that just does all I/O as
+// a NativeFile

s/Simply/Simple



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1180-1182
+  SimplePythonFile(int fd, uint32_t options, const PythonFile ,
+   bool borrowed)
+  : OwnedPythonFile(file, borrowed, fd, options, false){};

This reordering of arguments is confusing. Can we make the two orders match?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1238-1245
+// OwnedPythonFile::IsValid() chains into Base::IsValid()
+// File::IsValid() is false by default, but for the following classes
+// we want the file to be considered valid as long as the python bits
+// are valid.
+class PresumptivelyValidFile : public File {
+public:
+  bool IsValid() const override { return true; };

How about if OwnedPythonFile defines `IsValid()` as `IsPythonObjectValid() || 
Base::IsValid()`. Then PythonIOFile could redefine it to be simply 
`IsPythonObjectValid()` without the need for the extra class?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1251
+  PythonIOFile(const PythonFile , bool borrowed)
+  : OwnedPythonFile(file, borrowed){};
+

no semicolon



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1255-1264
+  Status Close() override {
+assert(m_py_obj);
+GIL takeGIL;
+if (m_borrowed)
+  return Flush();
+Take(PyObject_CallMethod(m_py_obj, "close", "()"));
+if (PyErr_Occurred())

Hmm... how exactly does this differ from the implementation in 
`OwnedPythonFile`? Is it only that it does not chain into the base class? Could 
we use the same trick as for IsValid to reduce the duplication?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1278
+class BinaryPythonFile : public PythonIOFile {
+  friend class PythonFile;
+

What is this needed for? I don't see the PythonFile class doing anything funny 
(and it probably shouldn't).



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:1397
+
+llvm::Expected PythonFile::ConvertToFileForcingUseOfScriptingIOMethods(
+bool borrowed) {

I was struggling to find this method, as I was looking it next to the other 
PythonFile methods. I get why it's here, but I think it's confusing to have 
PythonFile be split this way. Could you move the other classes so that they are 
declared before all of PythonFile methods. Or, given that 

[Lldb-commits] [PATCH] D68376: [lldb] Fix that 'ninja clean' breaks the build by deleting debugserver_vers.c

[Pavel Labath via Phabricator via lldb-commits]

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

This paragraph from that bug is interesting too:

  According to the 3.11 documentation for the GENERATED source file property, 
it should be used to indicate "Is this source file generated as part of the 
build or CMake process."  Earlier documentation does not mention "CMake 
process".  I was originally using it because "This information is then used to 
exempt the file from any existence or validity checks.", otherwise CMake would 
complain a about the missing source file.

Unfortunately, the cmake maintainer did not circle back to answer that, so we 
don't know the official cmake position, but my interpretation of this paragraph 
is the same as that of OP (i.e., that setting this property is the right thing 
to do, and the fact that it does not work is a bug in ninja or its cmake 
generator).

That said, this clearly does not work at the moment, so removing it is the best 
workaround we have.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68376



___
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.

BTW, I've had to add (or rather, extend) a fairly ugly hack in r373573 in order 
to get the new tests running on non-darwin platforms. The reason for that is 
that the FILE* out typemap needs to get the "mode" of the object in order to 
construct the python wrapper.

My hope is that we can be improved once you're finished your work here. For 
instance by reimplementing `GetOutputFileHandle` on top of `GetOutputFile` in 
swig, as the latter will (hopefully) be able to get the mode of a file 
correctly. Does that sound reasonable, or am I totally off-track here?


Repository:
  rL LLVM

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] [lldb] r373573 - "Fix" TestFileHandle.py on non-darwin platforms

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Oct  3 01:44:33 2019
New Revision: 373573

URL: http://llvm.org/viewvc/llvm-project?rev=373573=rev
Log:
"Fix" TestFileHandle.py on non-darwin platforms

This test exposed a very long standing issue that the python file
objects returned by the FILE* typemap were unusable on non-darwin
platforms. The reason they work on darwin is that they rely on a
non-standard extension to fetch the "mode" of a FILE* object. On other
platforms, this code was #ifdefed out, and so we were returning an empty
mode.

As there's no portable way to get this information, I just change the
non-darwin path to return "r+", which should permit both reading and
writing operations on the object. If the underlying file descriptor
turns out to be incompatible with this mode, the operating system should
return EBADF (or equivalent), instead of the "file not open for XXX"
error from python.

Modified:
lldb/trunk/scripts/Python/python-typemaps.swig

Modified: lldb/trunk/scripts/Python/python-typemaps.swig
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/python-typemaps.swig?rev=373573=373572=373573=diff
==
--- lldb/trunk/scripts/Python/python-typemaps.swig (original)
+++ lldb/trunk/scripts/Python/python-typemaps.swig Thu Oct  3 01:44:33 2019
@@ -406,6 +406,8 @@ bool SetNumberFromPyObject(doubl
 }
 
 %typemap(out) FILE * {
+   // TODO: This is gross. We should find a way to fetch the mode flags from 
the
+   // lldb_private::File object.
char mode[4] = {0};
 %#ifdef __APPLE__
int i = 0;
@@ -420,6 +422,12 @@ bool SetNumberFromPyObject(doubl
else // if (flags & __SRW)
   mode[i++] = 'a';
 }
+%#else
+   // There's no portable way to get the mode here. We just return a mode which
+   // permits both reads and writes and count on the operating system to return
+   // an error when an invalid operation is attempted.
+   mode[0] = 'r';
+   mode[1] = '+';
 %#endif
using namespace lldb_private;
NativeFile file($1, false);


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

[Lldb-commits] [PATCH] D68299: [JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationServerLLGS

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

> FYI, we've root caused some internal lldb asan failures to this patch. I 
> don't have a repro yet (my lldb tests seem to be failing locally for 
> unrelated reasons)

This should be fixed with r373572.

BTW, are you sure those reasons were unrelated? If you have had nearly every 
test failing then it was very likely due to this. (The reason only a couple of 
tests failed for you in asan mode is because our CI does not run a large chunk 
of the lldb tests... :/ )


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68299



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

[Lldb-commits] [PATCH] D68376: [lldb] Fix that 'ninja clean' breaks the build by deleting debugserver_vers.c

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: JDevlieghere, labath.
Herald added subscribers: lldb-commits, abidh, mgorny.
Herald added a project: LLDB.

We mark debugserver_vers.c as a generated file in CMake. This means that when 
we run `ninja clean` we end up deleting that file,
but any following `ninja` invocation will fail due to the file missing. The 
file can't be generated as `ninja` doesn't know it has to
rerun CMake to create the file. Turns out that marking the output of 
configure_file as generated is wrong as explained in this bug report:
https://gitlab.kitware.com/cmake/cmake/issues/18032

This patch just removes that property. The only side effect of this seems to be 
that this file maybe shows up in your IDE when
opening our CMake project, but that seems like a small sacrifice.

This patch can be quickly tested by running `ninja clean ; ninja 
lldbDebugserverCommon`. Before this patch the build will fail
due to debugserver_vers.c missing.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68376

Files:
  lldb/tools/debugserver/source/CMakeLists.txt


Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -136,7 +136,6 @@
   )
 
 set(DEBUGSERVER_VERS_GENERATED_FILE 
${CMAKE_CURRENT_BINARY_DIR}/debugserver_vers.c)
-set_source_files_properties(${DEBUGSERVER_VERS_GENERATED_FILE} PROPERTIES 
GENERATED 1)
 configure_file(debugserver_vers.c.in
${DEBUGSERVER_VERS_GENERATED_FILE} @ONLY)
 


Index: lldb/tools/debugserver/source/CMakeLists.txt
===
--- lldb/tools/debugserver/source/CMakeLists.txt
+++ lldb/tools/debugserver/source/CMakeLists.txt
@@ -136,7 +136,6 @@
   )
 
 set(DEBUGSERVER_VERS_GENERATED_FILE ${CMAKE_CURRENT_BINARY_DIR}/debugserver_vers.c)
-set_source_files_properties(${DEBUGSERVER_VERS_GENERATED_FILE} PROPERTIES GENERATED 1)
 configure_file(debugserver_vers.c.in
${DEBUGSERVER_VERS_GENERATED_FILE} @ONLY)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r373572 - Fix a use-after-free in GDBRemoteCommunicationServerLLGS

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu Oct  3 00:59:26 2019
New Revision: 373572

URL: http://llvm.org/viewvc/llvm-project?rev=373572=rev
Log:
Fix a use-after-free in GDBRemoteCommunicationServerLLGS

Although it's called "GetString", StreamString::GetString actually
returns a StringRef. Creating a json object with a StringRef does not
make a copy, which means the StringRef will be dangling as soon as the
underlying stream is destroyed. Add a .str() to force the json object to
hold a copy of the string.

This fixes nearly every test on linux.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=373572=373571=373572=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
 Thu Oct  3 00:59:26 2019
@@ -462,7 +462,8 @@ GetRegistersAsJSON(NativeThreadProtocol
 WriteRegisterValueInHexFixedWidth(stream, reg_ctx, *reg_info_p,
   _value, lldb::eByteOrderBig);
 
-register_object.try_emplace(llvm::to_string(reg_num), stream.GetString());
+register_object.try_emplace(llvm::to_string(reg_num),
+stream.GetString().str());
   }
 
   return register_object;


___
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 updated this revision to Diff 222970.
lawrence_danna added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.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
@@ -84,14 +84,19 @@
 
   PythonObject(const PythonObject ) : m_py_obj(nullptr) { Reset(rhs); }
 
+  PythonObject(PythonObject &) {
+m_py_obj = rhs.m_py_obj;
+rhs.m_py_obj = nullptr;
+  }
+
   virtual ~PythonObject() { Reset(); }
 
   void Reset() {
 // Avoid calling the virtual method since it's not necessary
 // to actually validate the type of the PyObject if we're
 // just setting to null.
-if (Py_IsInitialized())
-  Py_XDECREF(m_py_obj);
+if (m_py_obj && Py_IsInitialized())
+  Py_DECREF(m_py_obj);
 m_py_obj = nullptr;
   }
 
@@ -123,7 +128,7 @@
 // an owned reference by incrementing it.  If it is an owned
 // reference (for example the caller allocated it with PyDict_New()
 // then we must *not* increment it.
-if (Py_IsInitialized() && type == PyRefType::Borrowed)
+if (m_py_obj && Py_IsInitialized() && type == PyRefType::Borrowed)
   Py_XINCREF(m_py_obj);
   }
 
@@ -467,8 +472,38 @@
   void Reset(File , const char *mode);
 
   lldb::FileUP GetUnderlyingFile() const;
+
+  llvm::Expected ConvertToFile(bool borrowed = false);
+  llvm::Expected
+  ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
 };
 
+class PythonException : public llvm::ErrorInfo {
+private:
+  PyObject *m_exception_type, *m_exception, *m_traceback;
+  PyObject *m_repr_bytes;
+
+public:
+  static char ID;
+  const char *toCString() const;
+  PythonException(const char *caller);
+  void Restore();
+  ~PythonException();
+  void log(llvm::raw_ostream ) const override;
+  std::error_code convertToErrorCode() const override;
+};
+
+template  T unwrapOrSetPythonException(llvm::Expected expected) {
+  if (expected)
+return expected.get();
+  llvm::handleAllErrors(
+  expected.takeError(), [](PythonException ) { E.Restore(); },
+  [](const llvm::ErrorInfoBase ) {
+PyErr_SetString(PyExc_Exception, E.message().c_str());
+  });
+  return T();
+}
+
 } // namespace lldb_private
 
 #endif
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/Stream.h"
 
 #include "llvm/ADT/StringSwitch.h"
@@ -33,6 +34,14 @@
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
 
+template  static T Take(PyObject * obj) {
+return T(PyRefType::Owned, obj);
+}
+
+template  static T Retain(PyObject * obj) {
+  return T(PyRefType::Borrowed, obj);
+}
+
 // PythonObject
 
 void PythonObject::Dump(Stream ) const {
@@ -963,9 +972,7 @@
   // first-class object type anymore.  `PyFile_FromFd` is just a thin wrapper
   // over `io.open()`, which returns some object derived from `io.IOBase`. As a
   // result, the only way to detect a file in Python 3 is to check whether it
-  // inherits from `io.IOBase`.  Since it is possible for non-files to also
-  // inherit from `io.IOBase`, we additionally verify that it has the `fileno`
-  // attribute, which should guarantee that it is backed by the file system.
+  // inherits from `io.IOBase`.
   PythonObject io_module(PyRefType::Owned, PyImport_ImportModule("io"));
   PythonDictionary io_dict(PyRefType::Borrowed,
PyModule_GetDict(io_module.get()));
@@ -975,8 +982,6 @@
 
   if (1 != PyObject_IsSubclass(object_type.get(), io_base_class.get()))
 return false;
-  if (!object_type.HasAttribute("fileno"))
-return false;
 
   return true;
 #endif
@@ -1031,4 +1036,408 @@
   return file;
 }
 
+class GIL {
+public:
+  GIL() { m_state = PyGILState_Ensure(); }
+  ~GIL() { PyGILState_Release(m_state); }
+
+protected:
+  PyGILState_STATE m_state;
+};
+
+const char *PythonException::toCString() const {
+  if (m_repr_bytes) {
+return PyBytes_AS_STRING(m_repr_bytes);
+  } else {
+return "unknown exception";
+  }
+}
+

[Lldb-commits] [PATCH] D68370: Componentize lldb/scripts to use with LLVM_DISTRIBUTION_COMPONENTS

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/scripts/CMakeLists.txt:72
   # Install the LLDB python module
-  install(DIRECTORY ${SWIG_PYTHON_DIR}/ DESTINATION ${SWIG_INSTALL_DIR})
+  add_custom_target(lldb-scripts)
+  add_dependencies(lldb-scripts finish_swig)

Maybe add `python` somewhere in the name. Technically, the whole thing looks 
like it was designed to support more swig targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68370



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