[Lldb-commits] [PATCH] D69468: [LLDB][breakpoints] ArgInfo::count -> ArgInfo::max_positional_args

2019-10-27 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/scripts/Python/python-wrapper.swig:64
 
+unsigned max_positional_args = PythonCallable::ArgInfo::UNBOUNDED;
+if (auto arg_info = pfunc.GetArgInfo()) {

Is there any case where fetching the argument info will fail, but we still can 
successfully call the target object? Should we just bail out here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69468



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

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

2019-10-27 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

Which Python version are you using? I've just hacked Python 2.7 a bit and found 
that the failing `close()` is actually `fclose()` on `FILE*` object with fd=0. 
I can reproduce `EBADF` only when the same fd is closed twice, so I suspect 
that something else is closing stdin first, then some extra Python file created 
for stdin is being closed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737



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

[Lldb-commits] [PATCH] D69468: [LLDB][breakpoints] ArgInfo::count -> ArgInfo::max_positional_args

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna marked an inline comment as done.
lawrence_danna added inline comments.



Comment at: lldb/scripts/Python/python-wrapper.swig:64
 
+unsigned max_positional_args = PythonCallable::ArgInfo::UNBOUNDED;
+if (auto arg_info = pfunc.GetArgInfo()) {

labath wrote:
> Is there any case where fetching the argument info will fail, but we still 
> can successfully call the target object? Should we just bail out here?
probably not? My thinking is that since there's no meaningful way to return 
an error from this function we may as well try to call it and let the exception 
get logged.   But I dunno.Should I change it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69468



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

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

2019-10-27 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

I've just tried on Linux and it seems that Python closes the same descriptors. 
So somehow closing stdin works here on Linux, and fails on NetBSD. Curious 
enough, with a simple test I can verify that both NetBSD and Linux return EBADF 
when trying to close stdin twice. So unless there's another reason EBADF is 
returned here, it seems that stdin is closed somewhere earlier on NetBSD, and 
the same thing doesn't happen on Linux.

However, I think this is only a symptom of a more generic problem. I don't 
think we should really be creating a second system of file objects outside 
Python, and injecting it into Python file objects like this. I have a bad 
feeling for e.g. creating low-level a new file object for stdin, and then 
having Python destroy it alongside its own stdin object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737



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

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna added a comment.

In D68737#1722556 , @mgorny wrote:

> Which Python version are you using?


3.7.I'll try it with 2.7.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737



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

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna added a comment.

In D68737#1722255 , @mgorny wrote:

> However, I think this is only a symptom of a more generic problem. I don't 
> think we should really be creating a second system of file objects outside 
> Python, and injecting it into Python file objects like this. I have a bad 
> feeling for e.g. creating low-level a new file object for stdin, and then 
> having Python destroy it alongside its own stdin object.


What do you mean "a second system of file objects outside Python, and injecting 
it into Python file objects"?   That's not what it does.We have 
`lldb_private::File` proxies for python files, not the other way around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737



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

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

2019-10-27 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

In D68737#1722575 , @lawrence_danna 
wrote:

> In D68737#1722255 , @mgorny wrote:
>
> > However, I think this is only a symptom of a more generic problem. I don't 
> > think we should really be creating a second system of file objects outside 
> > Python, and injecting it into Python file objects like this. I have a bad 
> > feeling for e.g. creating low-level a new file object for stdin, and then 
> > having Python destroy it alongside its own stdin object.
>
>
> What do you mean "a second system of file objects outside Python, and 
> injecting it into Python file objects"?   That's not what it does.We have 
> `lldb_private::File` proxies for python files, not the other way around.


Hmm, I guess I've been reading that code wrong. I guess I'm out of clues then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737



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

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna added a comment.

In D68737#1722620 , @mgorny wrote:

> In D68737#1722575 , @lawrence_danna 
> wrote:
>
> > In D68737#1722255 , @mgorny wrote:
> >
> > > However, I think this is only a symptom of a more generic problem. I 
> > > don't think we should really be creating a second system of file objects 
> > > outside Python, and injecting it into Python file objects like this. I 
> > > have a bad feeling for e.g. creating low-level a new file object for 
> > > stdin, and then having Python destroy it alongside its own stdin object.
> >
> >
> > What do you mean "a second system of file objects outside Python, and 
> > injecting it into Python file objects"?   That's not what it does.We 
> > have `lldb_private::File` proxies for python files, not the other way 
> > around.
>
>
> Hmm, I guess I've been reading that code wrong. I guess I'm out of clues then.


I can reproduce it with python 2.7.I'll post a fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737



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

[Lldb-commits] [PATCH] D68737: SBFile::GetFile: convert SBFile back into python native files.

2019-10-27 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

Great, thanks! So it's not just my computer going crazy after all ;-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68737



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

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna created this revision.
lawrence_danna added reviewers: labath, mgorny.
Herald added a subscriber: krytarowski.
Herald added a project: LLDB.

Here's another instance where we were calling fflush on an input 
stream, which is illegal on NetBSD.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69488

Files:
  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
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+static char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+static const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+static char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+static const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, ::fflush);
+  // the lldb_private::File still owns it.   N

[Lldb-commits] [lldb] 40b0fa7 - [LLDB][formatters] ArgInfo::count -> ArgInfo::max_positional_args

2019-10-27 Thread Lawrence D'Anna via lldb-commits 

Author: Lawrence D'Anna
Date: 2019-10-27T16:01:46-07:00
New Revision: 40b0fa7ef2123866b2252ef6990040c2707cabe4

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

LOG: [LLDB][formatters] ArgInfo::count -> ArgInfo::max_positional_args

Summary:
Move breakpoints from the old, bad ArgInfo::count to the new, better
ArgInfo::max_positional_args.   Soon ArgInfo::count will be no more.

This functionality is tested in `TestFormatters.py`, 
`TestDataFormatterSynthVal.py`,
`TestDataFormatterSynthType.py`.

You may notice that the old code was passing 0 arguments when count was 1, and 
passing
1 argument when count is 2.

This is no longer necessary because max_positional_args counts the self pointer
correctly.

Reviewers: labath, jingham, JDevlieghere

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/scripts/Python/python-wrapper.swig

Removed: 




diff  --git a/lldb/scripts/Python/python-wrapper.swig 
b/lldb/scripts/Python/python-wrapper.swig
index b7af34221934..5e9a2ba1367c 100644
--- a/lldb/scripts/Python/python-wrapper.swig
+++ b/lldb/scripts/Python/python-wrapper.swig
@@ -495,11 +495,17 @@ LLDBSwigPython_CalculateNumChildren
 if (!pfunc.IsAllocated())
 return 0;
 
+auto arg_info = pfunc.GetArgInfo();
+if (!arg_info) {
+llvm::consumeError(arg_info.takeError());
+return 0;
+}
+
 PythonObject result;
-auto argc = pfunc.GetNumArguments();
-if (argc.count == 1)
+
+if (arg_info.get().max_positional_args < 1)
 result = pfunc();
-else if (argc.count == 2)
+else
 result = pfunc(PythonInteger(max));
 
 if (!result.IsAllocated())
@@ -511,13 +517,13 @@ LLDBSwigPython_CalculateNumChildren
 
 size_t ret_val = int_result.GetInteger();
 
-if (PyErr_Occurred())
+if (PyErr_Occurred()) //FIXME use Expected to catch python exceptions
 {
 PyErr_Print();
 PyErr_Clear();
 }
 
-if (argc.count == 1)
+if (arg_info.get().max_positional_args < 1)
 ret_val = std::min(ret_val, static_cast(max));
 
 return ret_val;



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

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna updated this revision to Diff 226595.
lawrence_danna added a comment.

py2_const_cast shouldn't be static as a free function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69488

Files:
  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
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, ::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for tha

[Lldb-commits] [PATCH] D69019: [lldb] move package generation from python to cmake

2019-10-27 Thread Haibo Huang via Phabricator via lldb-commits 
hhb added a comment.

In D69019#1722063 , @labath wrote:

> Right. I see what you mean.
>
> But... does this have to happen at build time? Since the list of files is 
> already known at configuration time, you should be able to generate the files 
> in the "cmake" step (but still leave the copying for the build step, so that 
> any changes to the files are reflected in rebuilds).
>
> (I'm not insisting on that -- I think that the current patch is already much 
> better than what we had before. If you think that the python script is better 
> than that, feel free to say so.)


That could be done... Except that the path of these files can be determined at 
build time, at least for multi-configuration generators. So we may need to 
write them to a temporary path at configuration time, and copy them to the 
right Debug / Release directory at build time.

Yea let's use python for now... 😊


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69019



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

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna updated this revision to Diff 226597.
lawrence_danna added a comment.

found another one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69488

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -310,7 +310,7 @@
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }
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
@@ -845,6 +845,7 @@
 def i(sbf):
 for i in range(10):
 f = sbf.GetFile()
+self.assertEqual(f.mode, "w")
 yield f
 sbf = lldb.SBFile.Create(f, borrow=True)
 yield sbf


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const ch

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna updated this revision to Diff 226599.
lawrence_danna added a comment.

and another


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69488

Files:
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/source/Host/common/File.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+inline const char *py2_const_cast(const char *s) { return s; }
+#endif
+
 enum class PyInitialValue { Invalid, Empty };
 
 template  struct PythonFormat;
@@ -309,16 +317,6 @@
 
   StructuredData::ObjectSP CreateStructuredObject() const;
 
-protected:
-
-#if PY_MAJOR_VERSION < 3
-  // The python 2 API declares some arguments as char* that should
-  // be const char *, but it doesn't actually modify them.
-  static char *py2_const_cast(const char *s) { return const_cast(s); }
-#else
-  static const char *py2_const_cast(const char *s) { return s; }
-#endif
-
 public:
   template 
   llvm::Expected CallMethod(const char *name,
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1502,12 +1502,19 @@
   file_obj = PyFile_FromFd(file.GetDescriptor(), nullptr, mode, -1, nullptr,
"ignore", nullptr, 0);
 #else
-  // Read through the Python source, doesn't seem to modify these strings
-  char *cmode = const_cast(mode);
   // We pass ::flush instead of ::fclose here so we borrow the FILE* --
-  // the lldb_private::File still owns it.
-  file_obj =
-  PyFile_FromFile(file.GetStream(), const_cast(""), cmode, 
::fflush);
+  // the lldb_private::File still owns it.   NetBSD does not allow
+  // input files to be flushed, so we have to check for that case too.
+  int (*closer)(FILE *);
+  auto opts = file.GetOptions();
+  if (!opts)
+return opts.takeError();
+  if (opts.get() & File::eOpenOptionWrite)
+closer = ::fflush;
+  else
+closer = [](FILE *) { return 0; };
+  file_obj = PyFile_FromFile(file.GetStream(), py2_const_cast(""),
+ py2_const_cast(mode), closer);
 #endif
 
   if (!file_obj)
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -310,7 +310,7 @@
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
-} else {
+} else if (m_options & eOpenOptionWrite) {
   if (::fflush(m_stream) == EOF)
 error.SetErrorToErrno();
 }
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
@@ -845,11 +845,16 @@
 def i(sbf):
 for i in range(10):
 f = sbf.GetFile()
+self.assertEqual(f.mode, "w")
 yield f
 sbf = lldb.SBFile.Create(f, borrow=True)
 yield sbf
 sbf.Write(str(i).encode('ascii') + b"\n")
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.
+while files:
+files.pop()
 with open(self.out_filename, 'r') as f:
 self.assertEqual(list(range(10)), list(map(int, 
f.read().strip().split(
 


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -189,6 +189,14 @@
  "key not in dict");
 }
 
+#if PY_MAJOR_VERSION < 3
+// The python 2 API declares some arguments as char* that should
+// be const char *, but it doesn't actually modify them.
+inline char *py2_const_cast(const char *s) {

[Lldb-commits] [PATCH] D69469: [LLDB][formatters] ArgInfo::count -> ArgInfo::max_positional_args

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG40b0fa7ef212: [LLDB][formatters] ArgInfo::count -> 
ArgInfo::max_positional_args (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69469

Files:
  lldb/scripts/Python/python-wrapper.swig


Index: lldb/scripts/Python/python-wrapper.swig
===
--- lldb/scripts/Python/python-wrapper.swig
+++ lldb/scripts/Python/python-wrapper.swig
@@ -495,11 +495,17 @@
 if (!pfunc.IsAllocated())
 return 0;
 
+auto arg_info = pfunc.GetArgInfo();
+if (!arg_info) {
+llvm::consumeError(arg_info.takeError());
+return 0;
+}
+
 PythonObject result;
-auto argc = pfunc.GetNumArguments();
-if (argc.count == 1)
+
+if (arg_info.get().max_positional_args < 1)
 result = pfunc();
-else if (argc.count == 2)
+else
 result = pfunc(PythonInteger(max));
 
 if (!result.IsAllocated())
@@ -511,13 +517,13 @@
 
 size_t ret_val = int_result.GetInteger();
 
-if (PyErr_Occurred())
+if (PyErr_Occurred()) //FIXME use Expected to catch python exceptions
 {
 PyErr_Print();
 PyErr_Clear();
 }
 
-if (argc.count == 1)
+if (arg_info.get().max_positional_args < 1)
 ret_val = std::min(ret_val, static_cast(max));
 
 return ret_val;


Index: lldb/scripts/Python/python-wrapper.swig
===
--- lldb/scripts/Python/python-wrapper.swig
+++ lldb/scripts/Python/python-wrapper.swig
@@ -495,11 +495,17 @@
 if (!pfunc.IsAllocated())
 return 0;
 
+auto arg_info = pfunc.GetArgInfo();
+if (!arg_info) {
+llvm::consumeError(arg_info.takeError());
+return 0;
+}
+
 PythonObject result;
-auto argc = pfunc.GetNumArguments();
-if (argc.count == 1)
+
+if (arg_info.get().max_positional_args < 1)
 result = pfunc();
-else if (argc.count == 2)
+else
 result = pfunc(PythonInteger(max));
 
 if (!result.IsAllocated())
@@ -511,13 +517,13 @@
 
 size_t ret_val = int_result.GetInteger();
 
-if (PyErr_Occurred())
+if (PyErr_Occurred()) //FIXME use Expected to catch python exceptions
 {
 PyErr_Print();
 PyErr_Clear();
 }
 
-if (argc.count == 1)
+if (arg_info.get().max_positional_args < 1)
 ret_val = std::min(ret_val, static_cast(max));
 
 return ret_val;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

2019-10-27 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

I'm sorry but I won't be able to test it until later today. Visually, looks 
good though.




Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:854
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.

For the record, this doesn't really guarantee specific order of destruction. 
Generally, in Python code you shouldn't rely on destructors being called at all 
and close files manually. That's why `with` is commonly used with files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69488



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

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna marked 2 inline comments as done.
lawrence_danna added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:854
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.

mgorny wrote:
> For the record, this doesn't really guarantee specific order of destruction. 
> Generally, in Python code you shouldn't rely on destructors being called at 
> all and close files manually. That's why `with` is commonly used with files.
The point is not to rely on the file being closed at a certain time.   They 
point is that each file added to the list is a borrowed reference to the 
previous one, and we should not allow those references to become dangling by 
letting the older ones go to zero-references before the younger ones.

Now that I say it, I wonder if it was a bad idea to expose this kind of C++ 
object lifetime semantics to python programs.
I put in `GetFile` mainly so that in the case that a SBFile was a proxy of a 
python file, you could get the python file back.

It might be better to restrict it to that case, and return None instead of a 
borrowed file in other cases.

On the other hand, there's value in having a consistent API.  Under the current 
semantics this just works:

```
print("foo",   file=debugger.GetOutputFile().GetFile())
```

Overall I think I'd lean towards keeping GetFile the way it is, but I can see 
the argument for restricting it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69488



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

[Lldb-commits] [PATCH] D69488: [LLDB][Python] fix another fflush issue on NetBSD

2019-10-27 Thread Lawrence D'Anna via Phabricator via lldb-commits 
lawrence_danna marked an inline comment as done.
lawrence_danna added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py:854
 files = list(i(sbf))
+# delete them in reverse order, again because each is a borrow
+# of the previous.

lawrence_danna wrote:
> mgorny wrote:
> > For the record, this doesn't really guarantee specific order of 
> > destruction. Generally, in Python code you shouldn't rely on destructors 
> > being called at all and close files manually. That's why `with` is commonly 
> > used with files.
> The point is not to rely on the file being closed at a certain time.   They 
> point is that each file added to the list is a borrowed reference to the 
> previous one, and we should not allow those references to become dangling by 
> letting the older ones go to zero-references before the younger ones.
> 
> Now that I say it, I wonder if it was a bad idea to expose this kind of C++ 
> object lifetime semantics to python programs.
> I put in `GetFile` mainly so that in the case that a SBFile was a proxy of a 
> python file, you could get the python file back.
> 
> It might be better to restrict it to that case, and return None instead of a 
> borrowed file in other cases.
> 
> On the other hand, there's value in having a consistent API.  Under the 
> current semantics this just works:
> 
> ```
> print("foo",   file=debugger.GetOutputFile().GetFile())
> ```
> 
> Overall I think I'd lean towards keeping GetFile the way it is, but I can see 
> the argument for restricting it.
On second second thought, there's a better way.   We don't need to expose the 
borrow semantics, and we don't need to restrict GetFile like that.  We can 
do the same thing File.cpp does when you ask it for a FILE* on a borrowed FD -- 
that is, use dup and create one that python can truly own.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69488



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

[Lldb-commits] [PATCH] D69425: [lldb] Fix broken -D option for breakpoint set command

2019-10-27 Thread Martin Svensson via Phabricator via lldb-commits 
poya added a comment.

Sure, I'll try to add a test in test/Shell/Breakpoint once I get the tests up 
and running on my machine.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69425



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