[Lldb-commits] [PATCH] D69014: [LLDB] bugfix: command script add -f doesn't work for some callables

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I also think we shouldn't go out of our way (and I consider any kind of 
introspection as "going out of our way") to forbid calling with a "deprecated" 
signature for some callables even though there's technically no backward 
compatibility to worry about (because it was not possible to call those 
callables in the past). For "gentle discouragement" I'd rather just print a 
warning whenever we call any callable with a deprecated signature. That warning 
can explain the problem and how to fix it (and it's strength can be ramped up 
over time if we want to eventually remove these).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69014



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

[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68995#1711831 , @lawrence_danna 
wrote:

> In D68995#1710594 , @labath wrote:
>
> > Thanks for jumping onto this. Apart from the inline comments, I have one 
> > high level question: What are the cases that the old method is wrong for? 
> > Was it just builtin functions, or are there other cases too? Is it possible 
> > to fix it to work for builtings too? (okay, those were three questions)
> >
> > What I am wondering is how important it is to maintain two methods for 
> > fetching the argument information. Builtin functions are not likely to be 
> > used as lldb callbacks, so if it's just those, it may be sufficient to just 
> > leave a TODO to use the new method once we are in a position to require 
> > python>=3.3.
>
>
> Looks like the old implementation also doesn't work for class or static 
> methods, or for objects with `__call__` method.I added a bunch of tests 
> for script commands with various callable types in the followon patch.


Ok, I see... Would it be hard/impossible to fix that using the previous method, 
so that we have a unified implementation? Or would that result in a bunch of 
ifdefs too ? I'm sorry if that's obvious, but I don't know much about the C 
python api..




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:58-61
+  auto utf8 = str.AsUTF8();
+  if (!utf8)
+return utf8.takeError();
+  return utf8.get();

lawrence_danna wrote:
> labath wrote:
> > Btw, what are the reasons that this can fail? The string object was created 
> > two lines above, so we definitely know it's a string. If the call cannot 
> > fail in this particular circumstance, then a one can express that (and make 
> > the code simpler) with `return cantFail(str.AsUTF8())`.
> It is possible!
> 
> ```
> chr(0xDFFF).encode('utf8')
> ```
Right. Thanks for explaining.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:864-891
+auto function = As(globals.GetItem("get_arg_info"));
+if (!function)
+  return function.takeError();
+get_arg_info = std::move(function.get());
+  }
+
+  Expected pyarginfo = get_arg_info.Call(*this);

lawrence_danna wrote:
> labath wrote:
> > Likewise, should most of these `takeErrors` be `cantFail` too ?
> Yea, i guess given that we control the script there's really no reasonable 
> way these can fail if the first check doesn't.
Yes, that's what I was thinking.



Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:317
+
+#if PY_MAJOR_VERSION < 3
+  static char *py2_const_cast(const char *s) { return const_cast(s); }

Could you also add a comment that this is because py2 apis accept strings as 
"char *", and that one should be careful to not use it if the function can 
actually modify the string.



Comment at: 
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp:695
+EXPECT_EQ(arginfo.get().max_positional_args,
+  PythonCallable::ArgInfo::UNBOUNDED);
+EXPECT_EQ(arginfo.get().has_varargs, true);

I don't see this defined anywhere. I guess it leaked from the other patch?



Comment at: 
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp:700
+  {
+const char *script = "class Foo: \n"
+ "  def bar(self, x):\n"

Raw string too?



Comment at: 
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp:643-647
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);

lawrence_danna wrote:
> labath wrote:
> > If you implemented `operator==` and `operator<<(raw_ostream&)` for the 
> > ArgInfo class, you could write this as 
> > `EXPECT_THAT_EXPECTED(lambda.GetArgInfo(), llvm::HasValue(ArgInfo(1, false, 
> > false))`
> eh, do i have to?I actually like having them each on their own line.   It 
> makes the patches nicer when members are added or removed, and i'd like to 
> remove all the members except max_positional_args.
No, you don't. These are mostly fine. The main thing I don't like is when 
people then try to "reduce duplication" and create a function like 
`compare_arginfo`, with the EXPECTs inside. Then, when the checks fail, the 
error message points you to that function and you're left guessing which of the 
compare invocations did this fail for. The only thing that can be said here is 
that if the ASSERT_THAT_EXPECTED macro fails it will terminate the execution of 
all the subsequent checks too, whereas if this was just one EXPECT call, then 
you'd be able to see results of all other tests too (and maybe notice a 
pa

[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

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

lawrence_danna marked 3 inline comments as done.
lawrence_danna added a comment.

In D68995#1712387 , @labath wrote:

> In D68995#1711831 , @lawrence_danna 
> wrote:
>
> > In D68995#1710594 , @labath wrote:
> >
> > > Thanks for jumping onto this. Apart from the inline comments, I have one 
> > > high level question: What are the cases that the old method is wrong for? 
> > > Was it just builtin functions, or are there other cases too? Is it 
> > > possible to fix it to work for builtings too? (okay, those were three 
> > > questions)
> > >
> > > What I am wondering is how important it is to maintain two methods for 
> > > fetching the argument information. Builtin functions are not likely to be 
> > > used as lldb callbacks, so if it's just those, it may be sufficient to 
> > > just leave a TODO to use the new method once we are in a position to 
> > > require python>=3.3.
> >
> >
> > Looks like the old implementation also doesn't work for class or static 
> > methods, or for objects with `__call__` method.I added a bunch of tests 
> > for script commands with various callable types in the followon patch.
>
>
> Ok, I see... Would it be hard/impossible to fix that using the previous 
> method, so that we have a unified implementation? Or would that result in a 
> bunch of ifdefs too ? I'm sorry if that's obvious, but I don't know much 
> about the C python api..


I did fix the previous method for python2, at least for the callable types I 
could think of to test.

There must be some way to get something resembling the old method to work for 
python3, because `inspect` does it.   But it would be harder to get right, and 
a lot less likely to keep being correct in future versions of python.And I 
don't think it's possible to have a single implementation like that that works 
for both 2 and 3 without ifdefs.

I was thinking that since python2 is EOL in like two months anyway, the best 
thing to do is to just keep the old implementation around until we drop python 
2 support, and then delete it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68995



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

[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

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

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

fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68995

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -20,6 +20,7 @@
 #include "PythonTestSuite.h"
 
 using namespace lldb_private;
+using namespace lldb_private::python;
 
 class PythonDataObjectsTest : public PythonTestSuite {
 public:
@@ -626,3 +627,116 @@
 }
   }
 }
+
+TEST_F(PythonDataObjectsTest, TestCallable) {
+
+  PythonDictionary globals(PyInitialValue::Empty);
+  auto builtins = PythonModule::BuiltinsModule();
+  llvm::Error error = globals.SetItem("__builtins__", builtins);
+  ASSERT_FALSE(error);
+
+  {
+PyObject *o = PyRun_String("lambda x : x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y=0: x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y=0, **kw: x", Py_eval_input,
+   globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y,*a: x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, true);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y,*a,**kw: x", Py_eval_input,
+   globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, true);
+  }
+
+  {
+const char *script = R"(
+class Foo:
+  def bar(self, x):
+ return x
+bar_bound   = Foo().bar
+bar_unbound = Foo.bar
+)";
+PyObject *o =
+PyRun_String(script, Py_file_input, globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+Take(o);
+
+auto bar_bound = As(globals.GetItem("bar_bound"));
+ASSERT_THAT_EXPECTED(bar_bound, llvm::Succeeded());
+auto arginfo = bar_bound.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2); // FIXME, wrong
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, true);
+
+auto bar_unbound = As(globals.GetItem("bar_unbound"));
+ASSERT_THAT_EXPECTED(bar_unbound, llvm::Succeeded());
+arginfo = bar_unbound.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+
+  // the old implementation of GetArgInfo just doesn't work on builtins.
+
+  {
+auto builtins = PythonModule::BuiltinsModule();
+auto hex = As(builtins.GetAttribute("hex"));
+ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());
+auto arginfo = hex.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+#endif
+}
\ No newline at end of file
Index: lldb/source/Plugins/ScriptInterpreter/Pytho

[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

[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/D68995/new/

https://reviews.llvm.org/D68995



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

[Lldb-commits] [PATCH] D69014: [LLDB] bugfix: command script add -f doesn't work for some callables

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

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

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69014

Files:
  
lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
  lldb/packages/Python/lldbsuite/test/commands/command/script/callables.py
  lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
  lldb/scripts/Python/python-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -643,8 +643,8 @@
 auto arginfo = lambda.GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().max_positional_args, 1u);
 EXPECT_EQ(arginfo.get().has_varargs, false);
-EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
   {
@@ -655,8 +655,8 @@
 auto arginfo = lambda.GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().max_positional_args, 2u);
 EXPECT_EQ(arginfo.get().has_varargs, false);
-EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
   {
@@ -667,6 +667,7 @@
 auto arginfo = lambda.GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().max_positional_args, 2u);
 EXPECT_EQ(arginfo.get().has_varargs, false);
   }
 
@@ -678,8 +679,9 @@
 auto arginfo = lambda.GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().max_positional_args,
+  PythonCallable::ArgInfo::UNBOUNDED);
 EXPECT_EQ(arginfo.get().has_varargs, true);
-EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
   {
@@ -690,6 +692,8 @@
 auto arginfo = lambda.GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().max_positional_args,
+  PythonCallable::ArgInfo::UNBOUNDED);
 EXPECT_EQ(arginfo.get().has_varargs, true);
   }
 
@@ -698,7 +702,18 @@
 class Foo:
   def bar(self, x):
  return x
+  @classmethod
+  def classbar(cls, x):
+ return x
+  @staticmethod
+  def staticbar(x):
+ return x
+  def __call__(self, x):
+ return x
+obj = Foo()
 bar_bound   = Foo().bar
+bar_class   = Foo().classbar
+bar_static  = Foo().staticbar
 bar_unbound = Foo.bar
 )";
 PyObject *o =
@@ -711,16 +726,37 @@
 auto arginfo = bar_bound.get().GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 2); // FIXME, wrong
+EXPECT_EQ(arginfo.get().max_positional_args, 1u);
 EXPECT_EQ(arginfo.get().has_varargs, false);
-EXPECT_EQ(arginfo.get().is_bound_method, true);
 
 auto bar_unbound = As(globals.GetItem("bar_unbound"));
 ASSERT_THAT_EXPECTED(bar_unbound, llvm::Succeeded());
 arginfo = bar_unbound.get().GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().max_positional_args, 2u);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+
+auto bar_class = As(globals.GetItem("bar_class"));
+ASSERT_THAT_EXPECTED(bar_class, llvm::Succeeded());
+arginfo = bar_class.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().max_positional_args, 1u);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+
+auto bar_static = As(globals.GetItem("bar_static"));
+ASSERT_THAT_EXPECTED(bar_static, llvm::Succeeded());
+arginfo = bar_static.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().max_positional_args, 1u);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+
+auto obj = As(globals.GetItem("obj"));
+ASSERT_THAT_EXPECTED(obj, llvm::Succeeded());
+arginfo = obj.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().max_positional_args, 1u);
 EXPECT_EQ(arginfo.get().has_varargs, false);
-EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
 #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
@@ -734,9 +770,9 @@
 auto arginfo = hex.get().GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().max_positional_args, 1u);
 EXPECT_EQ(arginfo.get().has_varargs, fal

[Lldb-commits] [PATCH] D68968: [android/process info] Introduce bundle id

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68968#1712018 , @jingham wrote:

> I don't think it would be good to make it hard to see the actual path (if you 
> have it) as well as the bundle ID.  If you are working on system components, 
> you want to know that you are running some hand-built copy of a binary and 
> not the pre-installed one.  So we want to make it easy to see "which thing 
> with this bundleID am I actually running".  Other than that this seems fine 
> to me.


Thanks, Jim. This part is interesting. On android the application "path" is 
mostly uninteresting, because it will always be "/system/bin/app_proces" (as 
the "app" is really just a shared library loaded into that process), and AFAIK 
it is not even possible to run an application (an least on an unrooted phone) 
from a different location.

What would you say is the right way to display this in the "platform process 
list" output? Would it be ok if in non-verbose mode we displayed only one of 
these things (bundle id, path base name, argv[0], probably in that order), and 
then for the non-verbose mode listed each thing separately?




Comment at: lldb/source/Host/linux/Host.cpp:220-222
+  if (process_info.GetNameAsStringRef().empty() &&
+  !process_info.GetArg0().empty()) {
+process_info.SetBundleID(process_info.GetArg0());

wallace wrote:
> labath wrote:
> > How sure are we that the app processes are the only ones which have the exe 
> > link unreadable? Will that be true on all phones or just the recent ones 
> > with all the selinux stuff?
> > 
> > I was hoping that there is some more reliable way of fetching this 
> > information, but there doesn't seem to be anything except [[ 
> > https://developer.android.com/reference/android/app/ActivityManager.RunningAppProcessInfo.html
> >  | ActivityManager.RunningAppProcessInfo ]], which I don't know if it can 
> > be accessed from lldb, either host- or device-side.
> > 
> > @danalbert, any ideas here?
> Another option I've just discovered is to invoke `pm list packages` to get 
> the list of all apks and if an Arg0 is in that list, then it's a package.
> That seems good enough
That should prevent us accidentally setting an incorrect bundle id, but it does 
not prevent a process from deliberately changing its argv[0] to the name of 
some other installed package. That seems suboptimal, particularly if we're 
later going to use start using the bundle id for other than just display 
purposes (e.g. for issuing "am kill" commands). So, I am still wondering if we 
shouldn't go back to using argv[0] for the purpose of "process list", and leave 
the "bundle id" field for cases where we can get this information from a more 
reliable source (e.g. reading it from the apk, or fetching it from the android 
package manager, etc). What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68968



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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

[Pavel Labath via Phabricator via lldb-commits]

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

I like where this is going.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:235-245
   PythonObject &operator=(const PythonObject &other) {
 Reset(PyRefType::Borrowed, other.get());
 return *this;
   }
 
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject &&other) {
 Reset();

You can consider simplifying this further down to a "universal"/"sink" 
`operator=(PythonObject other)`. Since the object is really just a pointer, the 
extra object being created won't hurt (in fact, the removal of `&`-indirection 
might make things faster).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080



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

[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

[Pavel Labath via Phabricator via lldb-commits]

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

In D68995#1712396 , @lawrence_danna 
wrote:

> >> Looks like the old implementation also doesn't work for class or static 
> >> methods, or for objects with `__call__` method.I added a bunch of 
> >> tests for script commands with various callable types in the followon 
> >> patch.
> > 
> > Ok, I see... Would it be hard/impossible to fix that using the previous 
> > method, so that we have a unified implementation? Or would that result in a 
> > bunch of ifdefs too ? I'm sorry if that's obvious, but I don't know much 
> > about the C python api..
>
> I did fix the previous method for python2, at least for the callable types I 
> could think of to test.
>
> There must be some way to get something resembling the old method to work for 
> python3, because `inspect` does it.   But it would be harder to get right, 
> and a lot less likely to keep being correct in future versions of python.
> And I don't think it's possible to have a single implementation like that 
> that works for both 2 and 3 without ifdefs.


Right, that's what I was thinking/fearing. I think I am satisfied with that 
explanation. Thanks for explaining.

> I was thinking that since python2 is EOL in like two months anyway, the best 
> thing to do is to just keep the old implementation around until we drop 
> python 2 support, and then delete it.

Yeah, though I don't know if that means that lldb will drop support for it 
straight away. I haven't heard of any plans like that, though the next weeks 
dev meeting will probably be a good place to talk about it. Will you be there 
by any chance?




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:822
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+static const char *get_arg_info_script = R"(
+from inspect import signature, Parameter, ismethod

`static const char get_arg_info_script[]` (one pointer more efficient)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68995



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

[Lldb-commits] [PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk abandoned this revision.
kwk added a comment.

Abandoning for D69041 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943



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

[Lldb-commits] [PATCH] D69100: COFF: Create a separate "section" for the file header

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: amccarth, mstorsjo.

In an attempt to ensure that every part of the module's memory image is
accounted for, D56537  created a special 
"container section" spanning the
entire image. While that seemed reasonable at the time (and it still
mostly does), it did create a problem of what to put as the "file size"
of the section, because the image is not continuous on disk, as we
generally assume (which is why I put zero there). Additionally, this
arrangement makes it unclear what kind of permissions should be assigned
to that section (which is what my next patch does).

To get around these, this patch partially reverts D56537 
, and goes back
to top-level sections. Instead, what I do is create a new "section" for
the object file header, which is also being loaded into memory, though
its not considered to be a section in the strictest sense. This makes it
possible to correctly assign file size section, and we can later assign
permissions to it as well.


https://reviews.llvm.org/D69100

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  test/Shell/ObjectFile/PECOFF/export-dllfunc.yaml
  test/Shell/ObjectFile/PECOFF/sections.yaml
  test/Shell/ObjectFile/PECOFF/subsections.yaml

Index: test/Shell/ObjectFile/PECOFF/sections.yaml
===
--- test/Shell/ObjectFile/PECOFF/sections.yaml
+++ test/Shell/ObjectFile/PECOFF/sections.yaml
@@ -2,36 +2,36 @@
 # RUN: lldb-test object-file %t | FileCheck %s
 
 
-# CHECK:  Showing 1 sections
+# CHECK:  Showing 3 sections
 # CHECK-NEXT:   Index: 0
 # CHECK-NEXT:   ID: 0x
-# CHECK-NEXT:   Name:
-# CHECK-NEXT:   Type: container
+# CHECK-NEXT:   Name: header
+# CHECK-NEXT:   Type: regular
 # CHECK-NEXT:   Permissions: ---
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x4000
-# CHECK-NEXT:   VM size: 12288
-# CHECK-NEXT:   File size: 0
-# CHECK-NEXT:   Showing 2 subsections
-# CHECK-NEXT: Index: 0
-# CHECK-NEXT: ID: 0x1
-# CHECK-NEXT: Name: .text
-# CHECK-NEXT: Type: code
-# CHECK-NEXT: Permissions: ---
-# CHECK-NEXT: Thread specific: no
-# CHECK-NEXT: VM address: 0x40001000
-# CHECK-NEXT: VM size: 64
-# CHECK-NEXT: File size: 512
+# CHECK-NEXT:   VM size: 512
+# CHECK-NEXT:   File size: 512
 # CHECK-EMPTY: 
-# CHECK-NEXT: Index: 1
-# CHECK-NEXT: ID: 0x2
-# CHECK-NEXT: Name: .data
-# CHECK-NEXT: Type: data
-# CHECK-NEXT: Permissions: ---
-# CHECK-NEXT: Thread specific: no
-# CHECK-NEXT: VM address: 0x40002000
-# CHECK-NEXT: VM size: 64
-# CHECK-NEXT: File size: 512
+# CHECK-NEXT:   Index: 1
+# CHECK-NEXT:   ID: 0x1
+# CHECK-NEXT:   Name: .text
+# CHECK-NEXT:   Type: code
+# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x40001000
+# CHECK-NEXT:   VM size: 64
+# CHECK-NEXT:   File size: 512
+# CHECK-EMPTY:
+# CHECK-NEXT:   Index: 2
+# CHECK-NEXT:   ID: 0x2
+# CHECK-NEXT:   Name: .data
+# CHECK-NEXT:   Type: data
+# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Thread specific: no
+# CHECK-NEXT:   VM address: 0x40002000
+# CHECK-NEXT:   VM size: 64
+# CHECK-NEXT:   File size: 512
 
 
 --- !COFF
Index: test/Shell/ObjectFile/PECOFF/export-dllfunc.yaml
===
--- test/Shell/ObjectFile/PECOFF/export-dllfunc.yaml
+++ test/Shell/ObjectFile/PECOFF/export-dllfunc.yaml
@@ -11,20 +11,24 @@
 # UUID should not be empty if the module is built with debug info.
 # BASIC-CHECK-DAG: UUID: {{[0-9A-F]{7,}[0-9A-F]}}-{{.*}}
 
-# BASIC-CHECK: Showing 3 subsections
+# BASIC-CHECK: Showing 4 sections
+#
 # BASIC-CHECK:  Index: 0
+# BASIC-CHECK:  Name: header
+#
+# BASIC-CHECK:  Index: 1
 # BASIC-CHECK:  Name: .text
 # BASIC-CHECK:  Type: code
 # BASIC-CHECK:  VM size: 22
 # BASIC-CHECK:  File size: 512
 #
-# BASIC-CHECK:  Index: 1
+# BASIC-CHECK:  Index: 2
 # BASIC-CHECK:  Name: .rdata
 # BASIC-CHECK:  Type: data
 # BASIC-CHECK:  VM size: {{.}}
 # BASIC-CHECK:  File size: 512
 #
-# BASIC-CHECK:  Index: 2
+# BASIC-CHECK:  Index: 3
 # BASIC-CHECK:  Name: .pdata
 # BASIC-CHECK:  Type: data
 # BASIC-CHECK:  VM size: 12
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -790,13 +790,15 @@
   if (module_sp) {
 std::lock_guard guard(module_sp->GetMutex());
 
-SectionSP image_sp = std::make_shared(
-module_sp, this, ~user_id_t(0), ConstString(), eSectionTypeContainer,
-m_coff_header_opt.image_base, m_coff_header_opt.image_size,
-/*file_offset*/ 0, /*file_size*/ 0, m_coff_header_opt.sect_alignment,
+SectionSP header_sp = std::make_shared(
+module_sp, this, ~user_i

[Lldb-commits] [PATCH] D69035: minidump: Refactor memory region computation code

[Pavel Labath via Phabricator via lldb-commits]

labath requested review of this revision.
labath added a comment.

With some additional COFF tweaks (D69100 , 
D69102 ), the next patch doesn't turn out that 
bad (though I'm still happy to accept "unwinder should just check section 
permissions instead of us translating them to memory regions" as an answer).


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

https://reviews.llvm.org/D69035



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

[Lldb-commits] [PATCH] D69102: COFF: Set section permissions

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: amccarth, mstorsjo.
labath added a parent revision: D69100: COFF: Create a separate "section" for 
the file header.
labath marked an inline comment as done.
labath added inline comments.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:800
 /*flags*/ 0);
+header_sp->SetPermissions(ePermissionsReadable);
 m_sections_up->AddSection(header_sp);

Are these the right permissions for the header?


This enables us to reason about whether a given address can be
executable, for instance during unwinding.


https://reviews.llvm.org/D69102

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  test/Shell/ObjectFile/PECOFF/sections.yaml


Index: test/Shell/ObjectFile/PECOFF/sections.yaml
===
--- test/Shell/ObjectFile/PECOFF/sections.yaml
+++ test/Shell/ObjectFile/PECOFF/sections.yaml
@@ -7,7 +7,7 @@
 # CHECK-NEXT:   ID: 0x
 # CHECK-NEXT:   Name: header
 # CHECK-NEXT:   Type: regular
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x4000
 # CHECK-NEXT:   VM size: 512
@@ -17,7 +17,7 @@
 # CHECK-NEXT:   ID: 0x1
 # CHECK-NEXT:   Name: .text
 # CHECK-NEXT:   Type: code
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r-x
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40001000
 # CHECK-NEXT:   VM size: 64
@@ -27,7 +27,7 @@
 # CHECK-NEXT:   ID: 0x2
 # CHECK-NEXT:   Name: .data
 # CHECK-NEXT:   Type: data
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40002000
 # CHECK-NEXT:   VM size: 64
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -797,6 +797,7 @@
 /*file_offset*/ 0, m_coff_header_opt.header_size,
 m_coff_header_opt.sect_alignment,
 /*flags*/ 0);
+header_sp->SetPermissions(ePermissionsReadable);
 m_sections_up->AddSection(header_sp);
 unified_section_list.AddSection(header_sp);
 
@@ -919,6 +920,15 @@
   m_coff_header_opt.sect_alignment, // Section alignment
   m_sect_headers[idx].flags));  // Flags for this section
 
+  uint32_t permissions = 0;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_EXECUTE)
+permissions |= ePermissionsExecutable;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_READ)
+permissions |= ePermissionsReadable;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_WRITE)
+permissions |= ePermissionsWritable;
+  section_sp->SetPermissions(permissions);
+
   m_sections_up->AddSection(section_sp);
   unified_section_list.AddSection(section_sp);
 }


Index: test/Shell/ObjectFile/PECOFF/sections.yaml
===
--- test/Shell/ObjectFile/PECOFF/sections.yaml
+++ test/Shell/ObjectFile/PECOFF/sections.yaml
@@ -7,7 +7,7 @@
 # CHECK-NEXT:   ID: 0x
 # CHECK-NEXT:   Name: header
 # CHECK-NEXT:   Type: regular
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x4000
 # CHECK-NEXT:   VM size: 512
@@ -17,7 +17,7 @@
 # CHECK-NEXT:   ID: 0x1
 # CHECK-NEXT:   Name: .text
 # CHECK-NEXT:   Type: code
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r-x
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40001000
 # CHECK-NEXT:   VM size: 64
@@ -27,7 +27,7 @@
 # CHECK-NEXT:   ID: 0x2
 # CHECK-NEXT:   Name: .data
 # CHECK-NEXT:   Type: data
-# CHECK-NEXT:   Permissions: ---
+# CHECK-NEXT:   Permissions: r--
 # CHECK-NEXT:   Thread specific: no
 # CHECK-NEXT:   VM address: 0x40002000
 # CHECK-NEXT:   VM size: 64
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -797,6 +797,7 @@
 /*file_offset*/ 0, m_coff_header_opt.header_size,
 m_coff_header_opt.sect_alignment,
 /*flags*/ 0);
+header_sp->SetPermissions(ePermissionsReadable);
 m_sections_up->AddSection(header_sp);
 unified_section_list.AddSection(header_sp);
 
@@ -919,6 +920,15 @@
   m_coff_header_opt.sect_alignment, // Section alignment
   m_sect_headers[idx].flags));  // Flags for this section
 
+  uint32_t permissions = 0;
+  if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_MEM_EXECUTE)
+permissions |= ePermissionsExecutable;
+  if (m_sect_headers[idx

[Lldb-commits] [PATCH] D69102: COFF: Set section permissions

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:800
 /*flags*/ 0);
+header_sp->SetPermissions(ePermissionsReadable);
 m_sections_up->AddSection(header_sp);

Are these the right permissions for the header?


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

https://reviews.llvm.org/D69102



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

[Lldb-commits] [PATCH] D69105: minidump: Create memory regions from the sections of loaded modules

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: amccarth, clayborg.
Herald added a subscriber: mgrang.
labath added parent revisions: D69102: COFF: Set section permissions, D69035: 
minidump: Refactor memory region computation code.

Not all minidumps contain information about memory permissions. However,
it is still important to know which regions of memory contain
potentially executable code. This is particularly important for
unwinding on win32, as the default unwind method there relies on
scanning the stack for things which "look like" code pointers.

This patch enables ProcessMinidump to reconstruct the likely permissions
of memory regions using the sections of loaded object files. It only
does this if we don't have a better source (memory info list stream, or
linux /proc/maps) for this information, and only if the information in
the object files does not conflict with the information in the minidump.

Theoretically that last bit could be improved, since the permissions
obtained from the MemoryList streams is also only a very rough guess,
but it did not seem worthwhile to complicate the implementation because
of that because there will generally be no overlap in practice as the
MemoryList will contain the stack contents and not any module data.

The patch adds a test checking that the module section permissions are
entered into the memory region list, and also a test which demonstrate
that now the unwinder is able to correctly find return addresses even in
minidumps without memory info list streams.

There's one TODO left in this patch, which is that the "memory region"
output does not give any indication about the "don't know" values of
memory region permissions (it just prints them as if they permission bit
was set). I address this in a follow up.


https://reviews.llvm.org/D69105

Files:
  source/Plugins/Process/minidump/ProcessMinidump.cpp
  test/Shell/Minidump/Inputs/basic-elf.yaml
  test/Shell/Minidump/memory-region-from-module.yaml
  test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml

Index: test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml
===
--- /dev/null
+++ test/Shell/SymbolFile/Breakpad/unwind-via-stack-win-no-memory-info.yaml
@@ -0,0 +1,133 @@
+# RUN: yaml2obj --docnum=1 %s > %t.dmp
+# RUN: yaml2obj --docnum=2 %s > %t.exe
+# RUN: %lldb -c %t.dmp %t.exe \
+# RUN:   -o "target symbols add %S/Inputs/unwind-via-raSearch.syms" \
+# RUN:   -o "thread backtrace" -b | FileCheck %s
+
+# CHECK-LABEL: thread backtrace
+# CHECK: frame #0: 0x000b1092 unwind-via-stack-win.exe`many_pointer_args
+# CHECK: frame #1: 0x000b1079 unwind-via-stack-win.exe`call_many + 105
+# CHECK: frame #2: 0x000b1085 unwind-via-stack-win.exe`main + 5
+# CHECK: frame #3: 0x77278494 kernel32.dll
+# CHECK-NOT: frame
+
+--- !minidump
+Streams:
+  - Type:ThreadList
+Threads:
+  - Thread Id:   0x290C
+Priority Class:  0x0020
+Environment Block: 0x00A98000
+Context: 3F0001007F022B0053002B002B0080100B0080100B50A90080100B0080100B00E4FECF0092100B002300440301007CFECF002B007F02801F0200144E3D6C2100EA1E00F4C000E507FC012CE3C014D8E20201E507880F401D839DC60140007F00880F401D0A00900F401D0100EA1E9808E5077F009008E50799016002E5072CABC87708346474B423010044E3C014200020532777A80F401D4F346474D00F401D6F378CCC5C4CD5013AFCD72F90E3C01418CE3470B423B80F401DC00F401DC80F401DD00F401D
+Stack:
+  Start of Memory Range: 0x00CFFE78
+  Content: 79100B100B100B100B100B100B100B100B100B100B100B100B100B100B100B100B100B100B100B100B100B0085100B0094842777
+  - Type:ModuleList
+Modules:
+  - Base of Image:   0x000B1000
+Size of Image:   0x4000
+Module Name: 'unwind-via-stack-win.exe'
+CodeVi

[Lldb-commits] [PATCH] D69106: MemoryRegion: Print "don't know" permission values as such

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, jingham.
labath added a parent revision: D69105: minidump: Create memory regions from 
the sections of loaded modules.

The permissions in a memory region have ternary states (yes, no, don't
know), but the memory region command only prints in binary, treating
"don't know" as "yes", which is particularly confusing as for instance
the unwinder will treat an unknown value as "no".

This patch makes is so that we distinguish all three states when
printing the values, using "?" to indicate the lack of information. It
is implemented via a special argument to the format provider for the
OptionalBool enumeration.


https://reviews.llvm.org/D69106

Files:
  include/lldb/Target/MemoryRegionInfo.h
  source/Commands/CommandObjectMemory.cpp
  source/Target/MemoryRegionInfo.cpp
  test/Shell/Minidump/memory-region-from-module.yaml

Index: test/Shell/Minidump/memory-region-from-module.yaml
===
--- test/Shell/Minidump/memory-region-from-module.yaml
+++ test/Shell/Minidump/memory-region-from-module.yaml
@@ -19,8 +19,7 @@
 # ALL: [0x-0x4000) ---
 # ALL-LABEL: (lldb) memory region 0x4000
 # CHECK1: [0x4000-0x40b0) r-x {{.*}}memory-region-from-module.exe PT_LOAD[0]
-# TODO: This output does not give any indication that the region is only "potentially" writable.
-# CHECK2: [0x4000-0x4010) rwx
+# CHECK2: [0x4000-0x4010) r??
 # ALL-LABEL: (lldb) memory region 0x5000
 # ALL: [0x5000-0x505c) rw- {{.*}}memory-region-from-module.exe PT_LOAD[1]
 # ALL-LABEL: (lldb) memory region 0x6000
Index: source/Target/MemoryRegionInfo.cpp
===
--- source/Target/MemoryRegionInfo.cpp
+++ source/Target/MemoryRegionInfo.cpp
@@ -8,13 +8,33 @@
 
 #include "lldb/Target/MemoryRegionInfo.h"
 
+using namespace lldb_private;
+
 llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &OS,
 const MemoryRegionInfo &Info) {
-  return OS << llvm::formatv("MemoryRegionInfo([{0}, {1}), {2}, {3}, {4}, {5}, "
- "`{6}`, {7}, {8})",
+  return OS << llvm::formatv("MemoryRegionInfo([{0}, {1}), {2:r}{3:w}{4:x}, "
+ "{5}, `{6}`, {7}, {8})",
  Info.GetRange().GetRangeBase(),
  Info.GetRange().GetRangeEnd(), Info.GetReadable(),
  Info.GetWritable(), Info.GetExecutable(),
  Info.GetMapped(), Info.GetName(), Info.GetFlash(),
  Info.GetBlocksize());
 }
+
+void llvm::format_provider::format(
+const MemoryRegionInfo::OptionalBool &B, raw_ostream &OS,
+StringRef Options) {
+  assert(Options.size() <= 1);
+  bool Empty = Options.empty();
+  switch (B) {
+  case lldb_private::MemoryRegionInfo::eNo:
+OS << (Empty ? "no" : "-");
+return;
+  case lldb_private::MemoryRegionInfo::eYes:
+OS << (Empty ? "yes" : Options);
+return;
+  case lldb_private::MemoryRegionInfo::eDontKnow:
+OS << (Empty ? "don't know" : "?");
+return;
+  }
+}
Index: source/Commands/CommandObjectMemory.cpp
===
--- source/Commands/CommandObjectMemory.cpp
+++ source/Commands/CommandObjectMemory.cpp
@@ -1730,15 +1730,12 @@
   section_name = section_sp->GetName();
 }
   }
-  result.AppendMessageWithFormat(
-  "[0x%16.16" PRIx64 "-0x%16.16" PRIx64 ") %c%c%c%s%s%s%s\n",
+  result.AppendMessageWithFormatv(
+  "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}\n",
   range_info.GetRange().GetRangeBase(),
-  range_info.GetRange().GetRangeEnd(),
-  range_info.GetReadable() ? 'r' : '-',
-  range_info.GetWritable() ? 'w' : '-',
-  range_info.GetExecutable() ? 'x' : '-',
-  name ? " " : "", name.AsCString(""),
-  section_name ? " " : "", section_name.AsCString(""));
+  range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
+  range_info.GetWritable(), range_info.GetExecutable(),
+  name ? " " : "", name, section_name ? " " : "", section_name);
   m_prev_end_addr = range_info.GetRange().GetRangeEnd();
   result.SetStatus(eReturnStatusSuccessFinishResult);
 } else {
Index: include/lldb/Target/MemoryRegionInfo.h
===
--- include/lldb/Target/MemoryRegionInfo.h
+++ include/lldb/Target/MemoryRegionInfo.h
@@ -133,21 +133,13 @@
 
 namespace llvm {
 template <>
+/// If Options is empty, prints a textual representation of the value. If
+/// Options is a single character, it uses that character for the "yes" value,
+/

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 225432.
tatyana-krasnukha added a comment.
Herald added a subscriber: wuzish.

Removed any ARC-specific logic from the ProcessGDBRemote.cpp.

It seems, there is nothing to test now;)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55718

Files:
  include/lldb/Utility/ArchSpec.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Target/Platform.cpp
  source/Target/Thread.cpp
  source/Utility/ArchSpec.cpp


Index: include/lldb/Utility/ArchSpec.h
===
--- include/lldb/Utility/ArchSpec.h
+++ include/lldb/Utility/ArchSpec.h
@@ -183,6 +183,8 @@
 eCore_uknownMach32,
 eCore_uknownMach64,
 
+eCore_arc, // little endian ARC
+
 kNumCores,
 
 kCore_invalid,
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -214,6 +214,7 @@
  ArchSpec::eCore_uknownMach32, "unknown-mach-32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::UnknownArch,
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"}
 };
 
 // Ensure that we have an entry in the g_core_definitions for each core. If you
@@ -436,6 +437,8 @@
  ArchSpec::eMIPSSubType_mips64r6el, 0xu, 0xu}, // 
mips64r6el
 {ArchSpec::eCore_hexagon_generic, llvm::ELF::EM_HEXAGON,
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
+{ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu }, // ARC
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -2060,6 +2060,7 @@
 case llvm::Triple::ppc64le:
 case llvm::Triple::systemz:
 case llvm::Triple::hexagon:
+case llvm::Triple::arc:
   m_unwinder_up.reset(new UnwindLLDB(*this));
   break;
 
Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -1822,6 +1822,12 @@
 trap_opcode_size = sizeof(g_aarch64_opcode);
   } break;
 
+  case llvm::Triple::arc: {
+static const uint8_t g_hex_opcode[] = { 0xff, 0x7f };
+trap_opcode = g_hex_opcode;
+trap_opcode_size = sizeof(g_hex_opcode);
+  } break;
+
   // TODO: support big-endian arm and thumb trap codes.
   case llvm::Triple::arm: {
 // The ARM reference recommends the use of 0xe7fddefe and 0xdefe but the
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -339,7 +339,7 @@
   // not, we assume no limit
 
   // build the qSupported packet
-  std::vector features = {"xmlRegisters=i386,arm,mips"};
+  std::vector features = {"xmlRegisters=i386,arm,mips,arc"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {


Index: include/lldb/Utility/ArchSpec.h
===
--- include/lldb/Utility/ArchSpec.h
+++ include/lldb/Utility/ArchSpec.h
@@ -183,6 +183,8 @@
 eCore_uknownMach32,
 eCore_uknownMach64,
 
+eCore_arc, // little endian ARC
+
 kNumCores,
 
 kCore_invalid,
Index: source/Utility/ArchSpec.cpp
===
--- source/Utility/ArchSpec.cpp
+++ source/Utility/ArchSpec.cpp
@@ -214,6 +214,7 @@
  ArchSpec::eCore_uknownMach32, "unknown-mach-32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::UnknownArch,
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"}
 };
 
 // Ensure that we have an entry in the g_core_definitions for each core. If you
@@ -436,6 +437,8 @@
  ArchSpec::eMIPSSubType_mips64r6el, 0xu, 0xu}, // mips64r6el
 {ArchSpec::eCore_hexagon_generic, llvm::ELF::EM_HEXAGON,
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
+{ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu }, // ARC
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -2060,6 +2060,7 @@
 case llvm::Triple::ppc64le:
 case llvm::Triple::systemz:
 case llvm::Triple::hexagon:
+case llvm::Triple::arc:
   m_unwind

[Lldb-commits] [PATCH] D55724: [ARC] Add SystemV ABI

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 225433.
tatyana-krasnukha added a comment.
Herald added a subscriber: JDevlieghere.

Rebased on the current trunk.

Updated according to the last revision of D55718 
.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D55724

Files:
  source/API/SystemInitializerFull.cpp
  source/Plugins/ABI/CMakeLists.txt
  source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp
  source/Plugins/ABI/SysV-arc/ABISysV_arc.h
  source/Plugins/ABI/SysV-arc/CMakeLists.txt

Index: source/Plugins/ABI/SysV-arc/CMakeLists.txt
===
--- source/Plugins/ABI/SysV-arc/CMakeLists.txt
+++ source/Plugins/ABI/SysV-arc/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_lldb_library(lldbPluginABISysV_arc PLUGIN
+  ABISysV_arc.cpp
+
+  LINK_LIBS
+lldbCore
+lldbSymbol
+lldbTarget
+lldbPluginProcessUtility
+  LINK_COMPONENTS
+Support
+  )
Index: source/Plugins/ABI/SysV-arc/ABISysV_arc.h
===
--- source/Plugins/ABI/SysV-arc/ABISysV_arc.h
+++ source/Plugins/ABI/SysV-arc/ABISysV_arc.h
@@ -0,0 +1,106 @@
+//===-- ArchitectureArc.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_ABISysV_arc_h_
+#define liblldb_ABISysV_arc_h_
+
+// Other libraries and framework includes
+#include 
+
+// Project includes
+#include "lldb/Target/ABI.h"
+#include "lldb/lldb-private.h"
+
+class ABISysV_arc : public lldb_private::ABI {
+public:
+  ~ABISysV_arc() override = default;
+
+  size_t GetRedZoneSize() const override;
+
+  bool PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+  lldb::addr_t functionAddress,
+  lldb::addr_t returnAddress,
+  llvm::ArrayRef args) const override;
+
+  // Special thread plan for GDB style non-jit function calls.
+  bool
+  PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+ lldb::addr_t functionAddress, lldb::addr_t returnAddress,
+ llvm::Type &prototype,
+ llvm::ArrayRef args) const override;
+
+  bool GetArgumentValues(lldb_private::Thread &thread,
+ lldb_private::ValueList &values) const override;
+
+  lldb_private::Status
+  SetReturnValueObject(lldb::StackFrameSP &frame_sp,
+   lldb::ValueObjectSP &new_value) override;
+
+  lldb::ValueObjectSP
+  GetReturnValueObjectImpl(lldb_private::Thread &thread,
+   lldb_private::CompilerType &type) const override;
+
+  // Specialized to work with llvm IR types.
+  lldb::ValueObjectSP GetReturnValueObjectImpl(lldb_private::Thread &thread,
+   llvm::Type &type) const override;
+
+  bool
+  CreateFunctionEntryUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool CreateDefaultUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
+
+  bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
+// Stack call frame address must be 4 byte aligned.
+return (cfa & 0x3ull) == 0;
+  }
+
+  bool CodeAddressIsValid(lldb::addr_t pc) override {
+// Code addresse must be 2 byte aligned.
+return (pc & 1ull) == 0;
+  }
+
+  const lldb_private::RegisterInfo *
+  GetRegisterInfoArray(uint32_t &count) override;
+
+  //--
+  // Static Functions
+  //--
+
+  static void Initialize();
+
+  static void Terminate();
+
+  static lldb::ABISP CreateInstance(lldb::ProcessSP process_sp,
+const lldb_private::ArchSpec &arch);
+
+  static lldb_private::ConstString GetPluginNameStatic();
+
+  //--
+  // PluginInterface protocol
+  //--
+
+  lldb_private::ConstString GetPluginName() override;
+
+  uint32_t GetPluginVersion() override;
+
+private:
+  lldb::ValueObjectSP
+  GetReturnValueObjectSimple(lldb_private::Thread &thread,
+ lldb_private::CompilerType &ast_type) const;
+
+  bool IsRegisterFileReduced(lldb_private::RegisterContext ®_ctx) const;
+
+  using lldb_private::ABI::ABI; // Call CreateInstance instead.
+
+  using ConfigurationFlags = llvm::Optional;
+  mutable ConfigurationFlags m_is_reg_file_reduced;
+};
+
+#endif // liblldb_ABISysV_ar

[Lldb-commits] [lldb] r375122 - [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via lldb-commits]

Author: tkrasnukha
Date: Thu Oct 17 08:16:21 2019
New Revision: 375122

URL: http://llvm.org/viewvc/llvm-project?rev=375122&view=rev
Log:
[ARC] Basic support in gdb-remote process plugin

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

Modified:
lldb/trunk/include/lldb/Utility/ArchSpec.h

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Target/Platform.cpp
lldb/trunk/source/Target/Thread.cpp
lldb/trunk/source/Utility/ArchSpec.cpp

Modified: lldb/trunk/include/lldb/Utility/ArchSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/ArchSpec.h?rev=375122&r1=375121&r2=375122&view=diff
==
--- lldb/trunk/include/lldb/Utility/ArchSpec.h (original)
+++ lldb/trunk/include/lldb/Utility/ArchSpec.h Thu Oct 17 08:16:21 2019
@@ -184,6 +184,8 @@ public:
 eCore_uknownMach32,
 eCore_uknownMach64,
 
+eCore_arc, // little endian ARC
+
 kNumCores,
 
 kCore_invalid,

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=375122&r1=375121&r2=375122&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
Thu Oct 17 08:16:21 2019
@@ -339,7 +339,7 @@ void GDBRemoteCommunicationClient::GetRe
   // not, we assume no limit
 
   // build the qSupported packet
-  std::vector features = {"xmlRegisters=i386,arm,mips"};
+  std::vector features = {"xmlRegisters=i386,arm,mips,arc"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {

Modified: lldb/trunk/source/Target/Platform.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Platform.cpp?rev=375122&r1=375121&r2=375122&view=diff
==
--- lldb/trunk/source/Target/Platform.cpp (original)
+++ lldb/trunk/source/Target/Platform.cpp Thu Oct 17 08:16:21 2019
@@ -1823,6 +1823,12 @@ size_t Platform::GetSoftwareBreakpointTr
 trap_opcode_size = sizeof(g_aarch64_opcode);
   } break;
 
+  case llvm::Triple::arc: {
+static const uint8_t g_hex_opcode[] = { 0xff, 0x7f };
+trap_opcode = g_hex_opcode;
+trap_opcode_size = sizeof(g_hex_opcode);
+  } break;
+
   // TODO: support big-endian arm and thumb trap codes.
   case llvm::Triple::arm: {
 // The ARM reference recommends the use of 0xe7fddefe and 0xdefe but the

Modified: lldb/trunk/source/Target/Thread.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Thread.cpp?rev=375122&r1=375121&r2=375122&view=diff
==
--- lldb/trunk/source/Target/Thread.cpp (original)
+++ lldb/trunk/source/Target/Thread.cpp Thu Oct 17 08:16:21 2019
@@ -2061,6 +2061,7 @@ Unwind *Thread::GetUnwinder() {
 case llvm::Triple::ppc64le:
 case llvm::Triple::systemz:
 case llvm::Triple::hexagon:
+case llvm::Triple::arc:
   m_unwinder_up.reset(new UnwindLLDB(*this));
   break;
 

Modified: lldb/trunk/source/Utility/ArchSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ArchSpec.cpp?rev=375122&r1=375121&r2=375122&view=diff
==
--- lldb/trunk/source/Utility/ArchSpec.cpp (original)
+++ lldb/trunk/source/Utility/ArchSpec.cpp Thu Oct 17 08:16:21 2019
@@ -216,6 +216,7 @@ static const CoreDefinition g_core_defin
  ArchSpec::eCore_uknownMach32, "unknown-mach-32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::UnknownArch,
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"}
 };
 
 // Ensure that we have an entry in the g_core_definitions for each core. If you
@@ -442,6 +443,8 @@ static const ArchDefinitionEntry g_elf_a
  ArchSpec::eMIPSSubType_mips64r6el, 0xu, 0xu}, // 
mips64r6el
 {ArchSpec::eCore_hexagon_generic, llvm::ELF::EM_HEXAGON,
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
+{ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu }, // ARC
 };
 
 static const ArchDefinition g_elf_arch_def = {


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

[Lldb-commits] [lldb] r375123 - [ARC] Add SystemV ABI

[Tatyana Krasnukha via lldb-commits]

Author: tkrasnukha
Date: Thu Oct 17 08:18:03 2019
New Revision: 375123

URL: http://llvm.org/viewvc/llvm-project?rev=375123&view=rev
Log:
[ARC] Add SystemV ABI

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

Added:
lldb/trunk/source/Plugins/ABI/SysV-arc/
lldb/trunk/source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp
lldb/trunk/source/Plugins/ABI/SysV-arc/ABISysV_arc.h
lldb/trunk/source/Plugins/ABI/SysV-arc/CMakeLists.txt
Modified:
lldb/trunk/source/API/SystemInitializerFull.cpp
lldb/trunk/source/Plugins/ABI/CMakeLists.txt

Modified: lldb/trunk/source/API/SystemInitializerFull.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SystemInitializerFull.cpp?rev=375123&r1=375122&r2=375123&view=diff
==
--- lldb/trunk/source/API/SystemInitializerFull.cpp (original)
+++ lldb/trunk/source/API/SystemInitializerFull.cpp Thu Oct 17 08:18:03 2019
@@ -24,6 +24,7 @@
 #include "Plugins/ABI/MacOSX-arm/ABIMacOSX_arm.h"
 #include "Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.h"
 #include "Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.h"
+#include "Plugins/ABI/SysV-arc/ABISysV_arc.h"
 #include "Plugins/ABI/SysV-arm/ABISysV_arm.h"
 #include "Plugins/ABI/SysV-arm64/ABISysV_arm64.h"
 #include "Plugins/ABI/SysV-hexagon/ABISysV_hexagon.h"
@@ -137,6 +138,8 @@ SystemInitializerFull::~SystemInitialize
 #define LLDB_PROCESS_ARM(op)   
\
   ABIMacOSX_arm::op(); 
\
   ABISysV_arm::op();
+#define LLDB_PROCESS_ARC(op)   
\
+  ABISysV_arc::op();
 #define LLDB_PROCESS_Hexagon(op) ABISysV_hexagon::op();
 #define LLDB_PROCESS_Mips(op)  
\
   ABISysV_mips::op();  
\
@@ -152,7 +155,6 @@ SystemInitializerFull::~SystemInitialize
   ABIWindows_x86_64::op();
 
 #define LLDB_PROCESS_AMDGPU(op)
-#define LLDB_PROCESS_ARC(op)
 #define LLDB_PROCESS_AVR(op)
 #define LLDB_PROCESS_BPF(op)
 #define LLDB_PROCESS_Lanai(op)

Modified: lldb/trunk/source/Plugins/ABI/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/CMakeLists.txt?rev=375123&r1=375122&r2=375123&view=diff
==
--- lldb/trunk/source/Plugins/ABI/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/ABI/CMakeLists.txt Thu Oct 17 08:18:03 2019
@@ -6,6 +6,9 @@ if ("ARM" IN_LIST LLVM_TARGETS_TO_BUILD)
   add_subdirectory(MacOSX-arm)
   add_subdirectory(SysV-arm)
 endif()
+if ("ARC" IN_LIST LLVM_TARGETS_TO_BUILD)
+  add_subdirectory(SysV-arc)
+endif()
 if ("Hexagon" IN_LIST LLVM_TARGETS_TO_BUILD)
   add_subdirectory(SysV-hexagon)
 endif()

Added: lldb/trunk/source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp?rev=375123&view=auto
==
--- lldb/trunk/source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp (added)
+++ lldb/trunk/source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp Thu Oct 17 08:18:03 
2019
@@ -0,0 +1,614 @@
+//===-- ABISysV_arc.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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ABISysV_arc.h"
+
+// C Includes
+// C++ Includes
+#include 
+#include 
+#include 
+
+// Other libraries and framework includes
+#include "llvm/ADT/Triple.h"
+#include "llvm/IR/DerivedTypes.h"
+#include "llvm/Support/MathExtras.h"
+
+#include "lldb/Core/Module.h"
+#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Value.h"
+#include "lldb/Core/ValueObjectConstResult.h"
+#include "lldb/Core/ValueObjectMemory.h"
+#include "lldb/Core/ValueObjectRegister.h"
+#include "lldb/Symbol/UnwindPlan.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/RegisterValue.h"
+#include "lldb/Utility/Status.h"
+
+#define DEFINE_REG_NAME(reg_num)  ConstString(#reg_num).GetCString()
+#define DEFINE_REG_NAME_STR(reg_name) ConstString(reg_name).GetCString()
+
+// The ABI is not a source of such information as size, offset, encoding, etc.
+// of a register. Just provides correct dwarf and eh_frame numbers.
+
+#define DEFINE_GENERIC_REGISTER_STUB(dwarf_num, str_name, generic_num)\
+  {   \
+DEFINE_REG_NAME(dwarf_num), DEFINE_REG_NAME_STR(str_name),  

[Lldb-commits] [PATCH] D69106: MemoryRegion: Print "don't know" permission values as such

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks good. Do we maybe want to use "unknown" instead of "don't know" when 
printing out the long for of a MemoryRegionInfo::OptionalBool? Or maybe "???"?




Comment at: test/Shell/Minidump/memory-region-from-module.yaml:22
 # CHECK1: [0x4000-0x40b0) r-x 
{{.*}}memory-region-from-module.exe PT_LOAD[0]
-# TODO: This output does not give any indication that the region is only 
"potentially" writable.
-# CHECK2: [0x4000-0x4010) rwx
+# CHECK2: [0x4000-0x4010) r??
 # ALL-LABEL: (lldb) memory region 0x5000

How do we get two responses for a single address here?


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

https://reviews.llvm.org/D69106



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

[Lldb-commits] [PATCH] D55724: [ARC] Add SystemV ABI

[Tatyana Krasnukha via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG92e498d58cf4: [ARC] Add SystemV ABI (authored by 
tatyana-krasnukha).

Changed prior to commit:
  https://reviews.llvm.org/D55724?vs=225433&id=225438#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55724

Files:
  lldb/source/API/SystemInitializerFull.cpp
  lldb/source/Plugins/ABI/CMakeLists.txt
  lldb/source/Plugins/ABI/SysV-arc/ABISysV_arc.cpp
  lldb/source/Plugins/ABI/SysV-arc/ABISysV_arc.h
  lldb/source/Plugins/ABI/SysV-arc/CMakeLists.txt

Index: lldb/source/Plugins/ABI/SysV-arc/CMakeLists.txt
===
--- /dev/null
+++ lldb/source/Plugins/ABI/SysV-arc/CMakeLists.txt
@@ -0,0 +1,11 @@
+add_lldb_library(lldbPluginABISysV_arc PLUGIN
+  ABISysV_arc.cpp
+
+  LINK_LIBS
+lldbCore
+lldbSymbol
+lldbTarget
+lldbPluginProcessUtility
+  LINK_COMPONENTS
+Support
+  )
Index: lldb/source/Plugins/ABI/SysV-arc/ABISysV_arc.h
===
--- /dev/null
+++ lldb/source/Plugins/ABI/SysV-arc/ABISysV_arc.h
@@ -0,0 +1,106 @@
+//===-- ArchitectureArc.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_ABISysV_arc_h_
+#define liblldb_ABISysV_arc_h_
+
+// Other libraries and framework includes
+#include 
+
+// Project includes
+#include "lldb/Target/ABI.h"
+#include "lldb/lldb-private.h"
+
+class ABISysV_arc : public lldb_private::ABI {
+public:
+  ~ABISysV_arc() override = default;
+
+  size_t GetRedZoneSize() const override;
+
+  bool PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+  lldb::addr_t functionAddress,
+  lldb::addr_t returnAddress,
+  llvm::ArrayRef args) const override;
+
+  // Special thread plan for GDB style non-jit function calls.
+  bool
+  PrepareTrivialCall(lldb_private::Thread &thread, lldb::addr_t sp,
+ lldb::addr_t functionAddress, lldb::addr_t returnAddress,
+ llvm::Type &prototype,
+ llvm::ArrayRef args) const override;
+
+  bool GetArgumentValues(lldb_private::Thread &thread,
+ lldb_private::ValueList &values) const override;
+
+  lldb_private::Status
+  SetReturnValueObject(lldb::StackFrameSP &frame_sp,
+   lldb::ValueObjectSP &new_value) override;
+
+  lldb::ValueObjectSP
+  GetReturnValueObjectImpl(lldb_private::Thread &thread,
+   lldb_private::CompilerType &type) const override;
+
+  // Specialized to work with llvm IR types.
+  lldb::ValueObjectSP GetReturnValueObjectImpl(lldb_private::Thread &thread,
+   llvm::Type &type) const override;
+
+  bool
+  CreateFunctionEntryUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool CreateDefaultUnwindPlan(lldb_private::UnwindPlan &unwind_plan) override;
+
+  bool RegisterIsVolatile(const lldb_private::RegisterInfo *reg_info) override;
+
+  bool CallFrameAddressIsValid(lldb::addr_t cfa) override {
+// Stack call frame address must be 4 byte aligned.
+return (cfa & 0x3ull) == 0;
+  }
+
+  bool CodeAddressIsValid(lldb::addr_t pc) override {
+// Code addresse must be 2 byte aligned.
+return (pc & 1ull) == 0;
+  }
+
+  const lldb_private::RegisterInfo *
+  GetRegisterInfoArray(uint32_t &count) override;
+
+  //--
+  // Static Functions
+  //--
+
+  static void Initialize();
+
+  static void Terminate();
+
+  static lldb::ABISP CreateInstance(lldb::ProcessSP process_sp,
+const lldb_private::ArchSpec &arch);
+
+  static lldb_private::ConstString GetPluginNameStatic();
+
+  //--
+  // PluginInterface protocol
+  //--
+
+  lldb_private::ConstString GetPluginName() override;
+
+  uint32_t GetPluginVersion() override;
+
+private:
+  lldb::ValueObjectSP
+  GetReturnValueObjectSimple(lldb_private::Thread &thread,
+ lldb_private::CompilerType &ast_type) const;
+
+  bool IsRegisterFileReduced(lldb_private::RegisterContext ®_ctx) const;
+
+  using lldb_private::ABI::ABI; // Call CreateInstance instead.
+
+  using RegisterFileFlag = llvm::Optional;
+  mutable RegisterFileFlag m_is_reg_file_reduced;
+};
+
+#endif // liblldb_ABISysV_arc_h_
Index: ll

[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

[Tatyana Krasnukha via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfaf6b2543e47: [ARC] Basic support in gdb-remote process 
plugin (authored by tatyana-krasnukha).

Changed prior to commit:
  https://reviews.llvm.org/D55718?vs=225432&id=225437#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D55718

Files:
  lldb/include/lldb/Utility/ArchSpec.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Utility/ArchSpec.cpp


Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -216,6 +216,7 @@
  ArchSpec::eCore_uknownMach32, "unknown-mach-32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::UnknownArch,
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"}
 };
 
 // Ensure that we have an entry in the g_core_definitions for each core. If you
@@ -442,6 +443,8 @@
  ArchSpec::eMIPSSubType_mips64r6el, 0xu, 0xu}, // 
mips64r6el
 {ArchSpec::eCore_hexagon_generic, llvm::ELF::EM_HEXAGON,
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
+{ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu }, // ARC
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -2061,6 +2061,7 @@
 case llvm::Triple::ppc64le:
 case llvm::Triple::systemz:
 case llvm::Triple::hexagon:
+case llvm::Triple::arc:
   m_unwinder_up.reset(new UnwindLLDB(*this));
   break;
 
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1823,6 +1823,12 @@
 trap_opcode_size = sizeof(g_aarch64_opcode);
   } break;
 
+  case llvm::Triple::arc: {
+static const uint8_t g_hex_opcode[] = { 0xff, 0x7f };
+trap_opcode = g_hex_opcode;
+trap_opcode_size = sizeof(g_hex_opcode);
+  } break;
+
   // TODO: support big-endian arm and thumb trap codes.
   case llvm::Triple::arm: {
 // The ARM reference recommends the use of 0xe7fddefe and 0xdefe but the
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
@@ -339,7 +339,7 @@
   // not, we assume no limit
 
   // build the qSupported packet
-  std::vector features = {"xmlRegisters=i386,arm,mips"};
+  std::vector features = {"xmlRegisters=i386,arm,mips,arc"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {
Index: lldb/include/lldb/Utility/ArchSpec.h
===
--- lldb/include/lldb/Utility/ArchSpec.h
+++ lldb/include/lldb/Utility/ArchSpec.h
@@ -184,6 +184,8 @@
 eCore_uknownMach32,
 eCore_uknownMach64,
 
+eCore_arc, // little endian ARC
+
 kNumCores,
 
 kCore_invalid,


Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -216,6 +216,7 @@
  ArchSpec::eCore_uknownMach32, "unknown-mach-32"},
 {eByteOrderLittle, 8, 4, 4, llvm::Triple::UnknownArch,
  ArchSpec::eCore_uknownMach64, "unknown-mach-64"},
+{eByteOrderLittle, 4, 2, 4, llvm::Triple::arc, ArchSpec::eCore_arc, "arc"}
 };
 
 // Ensure that we have an entry in the g_core_definitions for each core. If you
@@ -442,6 +443,8 @@
  ArchSpec::eMIPSSubType_mips64r6el, 0xu, 0xu}, // mips64r6el
 {ArchSpec::eCore_hexagon_generic, llvm::ELF::EM_HEXAGON,
  LLDB_INVALID_CPUTYPE, 0xu, 0xu}, // HEXAGON
+{ArchSpec::eCore_arc, llvm::ELF::EM_ARC_COMPACT2, LLDB_INVALID_CPUTYPE,
+ 0xu, 0xu }, // ARC
 };
 
 static const ArchDefinition g_elf_arch_def = {
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -2061,6 +2061,7 @@
 case llvm::Triple::ppc64le:
 case llvm::Triple::systemz:
 case llvm::Triple::hexagon:
+case llvm::Triple::arc:
   m_unwinder_up.reset(new UnwindLLDB(*this));
   break;
 
Index: lldb/source/Target/Platform.cpp

[Lldb-commits] [PATCH] D69100: COFF: Create a separate "section" for the file header

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:797
+m_coff_header_opt.header_size,
+/*file_offset*/ 0, m_coff_header_opt.header_size,
+m_coff_header_opt.sect_alignment,

dropped ///*file_size*///


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

https://reviews.llvm.org/D69100



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

[Lldb-commits] [lldb] r375127 - Fix an inverted condition in test.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Thu Oct 17 08:41:17 2019
New Revision: 375127

URL: http://llvm.org/viewvc/llvm-project?rev=375127&view=rev
Log:
Fix an inverted condition in test.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py?rev=375127&r1=375126&r2=375127&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
 Thu Oct 17 08:41:17 2019
@@ -31,9 +31,9 @@ class GdbRemoteTestCaseBase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
-_TIMEOUT_SECONDS = 120 * (1 if ('ASAN_OPTIONS' in os.environ) else 10)
-_READ_TIMEOUT=   5 * (1 if ('ASAN_OPTIONS' in os.environ) else 10)
-_WAIT_TIMEOUT=   3 * (1 if ('ASAN_OPTIONS' in os.environ) else 10)
+_TIMEOUT_SECONDS = 120 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
+_READ_TIMEOUT=   5 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
+_WAIT_TIMEOUT=   3 * (10 if ('ASAN_OPTIONS' in os.environ) else 1)
 
 _GDBREMOTE_KILL_PACKET = "$k#6b"
 


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

[Lldb-commits] [PATCH] D69100: COFF: Create a separate "section" for the file header

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:794
+SectionSP header_sp = std::make_shared(
+module_sp, this, ~user_id_t(0), ConstString("header"),
+eSectionTypeOther, m_coff_header_opt.image_base,

maybe name this "COFF header" or "PECOFF header"?


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

https://reviews.llvm.org/D69100



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

[Lldb-commits] [PATCH] D69105: minidump: Create memory regions from the sections of loaded modules

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:348-349
+  bool is_complete;
+  std::tie(*m_memory_regions, is_complete) =
+  m_minidump_parser->BuildMemoryRegions();
+

Might be nice to just assign memory regions and then ask the minidump parser 
what its source of memory regions was? I am thinking of:

```
m_memory_regions = m_minidump_parser->BuildMemoryRegions();
switch (m_minidump_parser->GetMemoryRegionsSource()) {
  case MemoryInfoList:
  case LinuxProcMaps:
break;
  case MemoryList:
// Memory list is not always a exhaustive list of memory regions in a 
process...
// Insert code below the "if (is_complete)" in switch?
```

Actually thinking of this a bit more, we should really just do this work inside 
"m_minidump_parser->BuildMemoryRegions();". The minidump itself can then make 
the call and we don't need to return the "is_complete". Otherwise if anyone 
else uses the info from "m_minidump_parser->BuildMemoryRegions();" they would 
have to do the same thing as this code which would be bad. 



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

https://reviews.llvm.org/D69105



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

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

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

This is causing a failure on the Windows Bot:

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68293



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

[Lldb-commits] [PATCH] D69114: Disable TestProcessList on windows

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added a reviewer: stella.stamenova.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
wallace edited the summary of this revision.
wallace added reviewers: clayborg, aadsm, labath.

`platform process list -v` on windows doesn't show all the process arguments, 
making this test useless for that platform.
Windows trunk is broken as well


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69114

Files:
  
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py


Index: 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
@@ -19,6 +19,7 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows # platform proces list -v on windows doesn't show all 
process the arguments
 def test_process_list_with_args(self):
 """Test process list show process args"""
 self.build()


Index: lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
===
--- lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
+++ lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
@@ -19,6 +19,7 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows # platform proces list -v on windows doesn't show all process the arguments
 def test_process_list_with_args(self):
 """Test process list show process args"""
 self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Stella Stamenova via Phabricator via lldb-commits]

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

The tests passed in my setup. After you commit this, please monitor the windows 
Buildbot (it currently has a couple of failures, so you will have to check it 
and not rely on an email).




Comment at: lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp:6
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \

labath wrote:
> mstorsjo wrote:
> > stella.stamenova wrote:
> > > Why is lld-link not specified as %lld_link?
> > Because that's how the lit substitutions are set up currently, and that's 
> > how existing tests write it. The substitutions are defined here:
> > https://github.com/llvm/llvm-project/blob/master/llvm/utils/lit/lit/llvm/config.py#L410
> > https://github.com/llvm/llvm-project/blob/master/llvm/utils/lit/lit/llvm/config.py#L492
> > 
> > Not sure if it's just because these have randomly happened to diverge, or 
> > if the percent for clang indicates that it's not just replaced with a 
> > resolved path, but also might include a few preset options.
> > Not sure if it's just because these have randomly happened to diverge, or 
> > if the percent for clang indicates that it's not just replaced with a 
> > resolved path, but also might include a few preset options.
> 
> I think that might have been the intention at some point, but I wouldn't 
> count on it being applied consistently.
If I recall correctly, this was one of the problems that build.py was trying to 
fix - because lld-link is not %lld_link sometimes it just called lld-link. It 
seems to be working though, so maybe we can use it as-is.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69031



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

[Lldb-commits] [PATCH] D69114: Disable TestProcessList on windows

[Stella Stamenova via Phabricator via lldb-commits]

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

Please also file a bug and reference it in the skip statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69114



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

[Lldb-commits] [PATCH] D69114: Disable TestProcessList on windows

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

oh girl, i need to get a bugzilla account. I hope i can get it soon. If it 
takes too long I can send another patch with the updated comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69114



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

[Lldb-commits] [PATCH] D69114: Disable TestProcessList on windows

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I filed a bug for you, please reference it and submit: 
https://bugs.llvm.org/show_bug.cgi?id=43702 (we can update the bug description 
later).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69114



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

Re: [Lldb-commits] [PATCH] D68968: [android/process info] Introduce bundle id

[Jim Ingham via lldb-commits]

> On Oct 17, 2019, at 1:35 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> In D68968#1712018 , @jingham wrote:
> 
>> I don't think it would be good to make it hard to see the actual path (if 
>> you have it) as well as the bundle ID.  If you are working on system 
>> components, you want to know that you are running some hand-built copy of a 
>> binary and not the pre-installed one.  So we want to make it easy to see 
>> "which thing with this bundleID am I actually running".  Other than that 
>> this seems fine to me.
> 
> 
> Thanks, Jim. This part is interesting. On android the application "path" is 
> mostly uninteresting, because it will always be "/system/bin/app_proces" (as 
> the "app" is really just a shared library loaded into that process), and 
> AFAIK it is not even possible to run an application (an least on an unrooted 
> phone) from a different location.

On iOS you can run an app from anywhere.  There's a registration process so the 
app launcher knows how to respond to "tell app-bundle foo to do something".   
But other than that I don't think there are any significant restrictions on 
where it can be.

> 
> What would you say is the right way to display this in the "platform process 
> list" output? Would it be ok if in non-verbose mode we displayed only one of 
> these things (bundle id, path base name, argv[0], probably in that order), 
> and then for the non-verbose mode listed each thing separately?
> 

If the second "non-verbose" in the sentence above was supposed to be "verbose" 
then this seems good to me.  Even though the location is not so restricted, 
Xcode installs binaries for you and the there's no way to control where they 
go, so for the vast majority of users seeing the bundle id would be fine.

Jim


> 
> 
> 
> Comment at: lldb/source/Host/linux/Host.cpp:220-222
> +  if (process_info.GetNameAsStringRef().empty() &&
> +  !process_info.GetArg0().empty()) {
> +process_info.SetBundleID(process_info.GetArg0());
> 
> wallace wrote:
>> labath wrote:
>>> How sure are we that the app processes are the only ones which have the exe 
>>> link unreadable? Will that be true on all phones or just the recent ones 
>>> with all the selinux stuff?
>>> 
>>> I was hoping that there is some more reliable way of fetching this 
>>> information, but there doesn't seem to be anything except [[ 
>>> https://developer.android.com/reference/android/app/ActivityManager.RunningAppProcessInfo.html
>>>  | ActivityManager.RunningAppProcessInfo ]], which I don't know if it can 
>>> be accessed from lldb, either host- or device-side.
>>> 
>>> @danalbert, any ideas here?
>> Another option I've just discovered is to invoke `pm list packages` to get 
>> the list of all apks and if an Arg0 is in that list, then it's a package.
>> That seems good enough
> That should prevent us accidentally setting an incorrect bundle id, but it 
> does not prevent a process from deliberately changing its argv[0] to the name 
> of some other installed package. That seems suboptimal, particularly if we're 
> later going to use start using the bundle id for other than just display 
> purposes (e.g. for issuing "am kill" commands). So, I am still wondering if 
> we shouldn't go back to using argv[0] for the purpose of "process list", and 
> leave the "bundle id" field for cases where we can get this information from 
> a more reliable source (e.g. reading it from the apk, or fetching it from the 
> android package manager, etc). What do you think?
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D68968/new/
> 
> https://reviews.llvm.org/D68968
> 
> 
> 

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

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[Joseph Tremoulet via Phabricator via lldb-commits]

JosephTremoulet updated this revision to Diff 225459.
JosephTremoulet marked an inline comment as done.
JosephTremoulet added a comment.

- Rebase
- Use Twine instead of formatv


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68657

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/linux-x86_64.yaml
  llvm/include/llvm/ObjectYAML/MinidumpYAML.h
  llvm/lib/ObjectYAML/MinidumpEmitter.cpp
  llvm/lib/ObjectYAML/MinidumpYAML.cpp
  llvm/test/tools/obj2yaml/basic-minidump.yaml
  llvm/test/tools/yaml2obj/minidump-exception-missing-parameter.yaml
  llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp

Index: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
+++ llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp
@@ -139,3 +139,200 @@
   (ArrayRef{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}),
   makeArrayRef(SysInfo.CPU.Other.ProcessorFeatures));
 }
+
+// Test that we can parse a normal-looking ExceptionStream.
+TEST(MinidumpYAML, ExceptionStream) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x7
+Exception Record:
+  Exception Code:  0x23
+  Exception Flags: 0x5
+  Exception Record: 0x0102030405060708
+  Exception Address: 0x0a0b0c0d0e0f1011
+  Number of Parameters: 2
+  Parameter 0: 0x22
+  Parameter 1: 0x24
+Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected ExpectedStream =
+  File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(2u, Exception.NumberParameters);
+  EXPECT_EQ(0x22u, Exception.ExceptionInformation[0]);
+  EXPECT_EQ(0x24u, Exception.ExceptionInformation[1]);
+
+  Expected> ExpectedContext =
+  File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+   0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an exception stream with no ExceptionInformation.
+TEST(MinidumpYAML, ExceptionStream_NoParameters) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x7
+Exception Record:
+  Exception Code:  0x23
+  Exception Flags: 0x5
+  Exception Record: 0x0102030405060708
+  Exception Address: 0x0a0b0c0d0e0f1011
+Thread Context:  3DeadBeefDefacedABadCafe)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  object::MinidumpFile &File = **ExpectedFile;
+
+  ASSERT_EQ(1u, File.streams().size());
+
+  Expected ExpectedStream =
+  File.getExceptionStream();
+
+  ASSERT_THAT_EXPECTED(ExpectedStream, Succeeded());
+
+  const minidump::ExceptionStream &Stream = *ExpectedStream;
+  EXPECT_EQ(0x7u, Stream.ThreadId);
+  const minidump::Exception &Exception = Stream.ExceptionRecord;
+  EXPECT_EQ(0x23u, Exception.ExceptionCode);
+  EXPECT_EQ(0x5u, Exception.ExceptionFlags);
+  EXPECT_EQ(0x0102030405060708u, Exception.ExceptionRecord);
+  EXPECT_EQ(0x0a0b0c0d0e0f1011u, Exception.ExceptionAddress);
+  EXPECT_EQ(0u, Exception.NumberParameters);
+
+  Expected> ExpectedContext =
+  File.getRawData(Stream.ThreadContext);
+  ASSERT_THAT_EXPECTED(ExpectedContext, Succeeded());
+  EXPECT_EQ((ArrayRef{0x3d, 0xea, 0xdb, 0xee, 0xfd, 0xef, 0xac, 0xed,
+   0xab, 0xad, 0xca, 0xfe}),
+*ExpectedContext);
+}
+
+// Test that we can parse an ExceptionStream where the stated number of
+// parameters is greater than the actual size of the ExceptionInformation
+// array.
+TEST(MinidumpYAML, ExceptionStream_TooManyParameters) {
+  SmallString<0> Storage;
+  auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+  - Type:Exception
+Thread ID:  0x8
+Exception Record:
+  Exception Code: 0
+  Number of Parameters: 16
+  Parameter 0: 0x0
+  Parameter 1: 0xff
+  Parameter 2: 0xee
+  Parameter 3: 0xdd
+  Parameter 4: 0xcc
+  Parameter 5: 0xbb
+  Parameter 6: 0xaa
+  Parameter 7: 0x99
+  Parameter 8: 0x88
+  Parameter 9: 0x77
+  Parameter 10: 0x66
+  Parameter 11: 0x55
+   

[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[Joseph Tremoulet via Phabricator via lldb-commits]

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



Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:389
+  for (size_t Index = 0; Index < Exception.MaxParameters; ++Index) {
+SmallString<16> Name = formatv("Parameter {0}", Index);
+support::ulittle64_t &Field = Exception.ExceptionInformation[Index];

JosephTremoulet wrote:
> MaskRay wrote:
> > You may use `("Parameter " + Twine(Index)).str()` to get rid of the 
> > "llvm/Support/FormatVariadic.h" dependency.
> Is #including FormatVariadic.h a bad thing?  I did it this way to avoid heap 
> allocation.
...finally realized that Twine avoids the heap too :).  Thanks for the 
suggestion, updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68657



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

[Lldb-commits] [PATCH] D69119: Modernize the rest of the Find.* API (NFC)

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

+1


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

https://reviews.llvm.org/D69119



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

[Lldb-commits] [lldb] r375144 - Disable TestProcessList on windows

[Walter Erquinigo via lldb-commits]

Author: wallace
Date: Thu Oct 17 10:53:44 2019
New Revision: 375144

URL: http://llvm.org/viewvc/llvm-project?rev=375144&view=rev
Log:
Disable TestProcessList on windows

Summary: `platform process list -v` on windows doesn't show all the process 
arguments, making this test useless for that platform

Reviewers: stella.stamenova

Subscribers: lldb-commits

Tags: #lldb

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py?rev=375144&r1=375143&r2=375144&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
 Thu Oct 17 10:53:44 2019
@@ -19,6 +19,7 @@ class ProcessListTestCase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows # https://bugs.llvm.org/show_bug.cgi?id=43702
 def test_process_list_with_args(self):
 """Test process list show process args"""
 self.build()


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

[Lldb-commits] [PATCH] D69114: Disable TestProcessList on windows

[walter erquinigo via Phabricator via lldb-commits]

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

added bug link in the comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69114

Files:
  
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py


Index: 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
@@ -19,6 +19,7 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows # https://bugs.llvm.org/show_bug.cgi?id=43702
 def test_process_list_with_args(self):
 """Test process list show process args"""
 self.build()


Index: lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
===
--- lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
+++ lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
@@ -19,6 +19,7 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows # https://bugs.llvm.org/show_bug.cgi?id=43702
 def test_process_list_with_args(self):
 """Test process list show process args"""
 self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69114: Disable TestProcessList on windows

[walter erquinigo 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 rGe19dfa6745f6: Disable TestProcessList on windows (authored 
by wallace).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69114

Files:
  
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py


Index: 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
@@ -19,6 +19,7 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows # https://bugs.llvm.org/show_bug.cgi?id=43702
 def test_process_list_with_args(self):
 """Test process list show process args"""
 self.build()


Index: lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
===
--- lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
+++ lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
@@ -19,6 +19,7 @@
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfWindows # https://bugs.llvm.org/show_bug.cgi?id=43702
 def test_process_list_with_args(self):
 """Test process list show process args"""
 self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r375146 - [Reproducer] Surface error if setting the cwd fails

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu Oct 17 10:58:44 2019
New Revision: 375146

URL: http://llvm.org/viewvc/llvm-project?rev=375146&view=rev
Log:
[Reproducer] Surface error if setting the cwd fails

Make sure that we surface an error if setting the current working
directory fails during replay.

Modified:
lldb/trunk/source/Initialization/SystemInitializerCommon.cpp

Modified: lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Initialization/SystemInitializerCommon.cpp?rev=375146&r1=375145&r2=375146&view=diff
==
--- lldb/trunk/source/Initialization/SystemInitializerCommon.cpp (original)
+++ lldb/trunk/source/Initialization/SystemInitializerCommon.cpp Thu Oct 17 
10:58:44 2019
@@ -80,8 +80,13 @@ llvm::Error SystemInitializerCommon::Ini
 }
 if (llvm::Expected cwd =
 loader->LoadBuffer()) {
-  
FileSystem::Instance().GetVirtualFileSystem()->setCurrentWorkingDirectory(
-  *cwd);
+  cwd->erase(std::remove_if(cwd->begin(), cwd->end(), std::iscntrl),
+ cwd->end());
+  if (std::error_code ec = FileSystem::Instance()
+   .GetVirtualFileSystem()
+   ->setCurrentWorkingDirectory(*cwd)) {
+return llvm::errorCodeToError(ec);
+  }
 } else {
   return cwd.takeError();
 }


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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

[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.h:235-245
   PythonObject &operator=(const PythonObject &other) {
 Reset(PyRefType::Borrowed, other.get());
 return *this;
   }
 
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject &&other) {
 Reset();

labath wrote:
> You can consider simplifying this further down to a "universal"/"sink" 
> `operator=(PythonObject other)`. Since the object is really just a pointer, 
> the extra object being created won't hurt (in fact, the removal of 
> `&`-indirection might make things faster).
wouldn't that result in an extra retain and release every time a PythonObject 
was copied instead of referenced or moved?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080



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

[Lldb-commits] [PATCH] D69119: Modernize the rest of the Find.* API (NFC)

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

yay




Comment at: lldb/tools/lldb-test/lldb-test.cpp:439-448
+List.Clear();
+Symfile.FindFunctions(RE, true, List);
   } else {
 Expected ContextOr = getDeclContext(Symfile);
 if (!ContextOr)
   return ContextOr.takeError();
 CompilerDeclContext *ContextPtr =

I'm pretty sure these Clears are not needed.


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

https://reviews.llvm.org/D69119



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

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

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Thu Oct 17 11:16:50 2019
New Revision: 375151

URL: http://llvm.org/viewvc/llvm-project?rev=375151&view=rev
Log:
[lldb] Don't emit artificial constructor declarations as global functions

Summary:
When we have a artificial constructor DIE, we currently create from that a 
global function with the name of that class.
That ends up causing a bunch of funny errors such as "must use 'struct' tag to 
refer to type 'Foo' in this scope" when
doing `Foo f`. Also causes that constructing a class via `Foo()` actually just 
calls that global function.

The fix is that when we have an artificial method decl, we always treat it as 
handled even if we don't create a CXXMethodDecl
for it (which we never do for artificial methods at the moment).

Fixes rdar://55757491 and probably some other radars.

Reviewers: aprantl, vsk, shafik

Reviewed By: aprantl

Subscribers: jingham, shafik, labath, JDevlieghere, lldb-commits

Tags: #lldb

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

Added:

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp
Modified:

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py?rev=375151&r1=375150&r2=375151&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
 Thu Oct 17 11:16:50 2019
@@ -49,3 +49,7 @@ class ExprCommandCallOverriddenMethod(Te
 
 # Test calling the base class.
 self.expect("expr realbase.foo()", substrs=["1"])
+
+# Test with locally constructed instances.
+self.expect("expr Base().foo()", substrs=["1"])
+self.expect("expr Derived().foo()", substrs=["2"])

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp?rev=375151&r1=375150&r2=375151&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp
 Thu Oct 17 11:16:50 2019
@@ -11,6 +11,7 @@ public:
 
 int main() {
   Base realbase;
+  realbase.foo();
   Derived d;
   Base *b = &d;
   return 0; // Set breakpoint here

Added: 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py?rev=375151&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
 Thu Oct 17 11:16:50 2019
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), None)

Added: 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp?rev=375151&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp
 Thu Oct 17 11:16:50 2019
@@ -0,0 +1,10 @@
+struct Foo {
+  // Triggers that we emit an artificial constructor for Foo.
+  virtual ~Foo() = default;
+};
+
+

[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

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

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

char * -> char[]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68995

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -20,6 +20,7 @@
 #include "PythonTestSuite.h"
 
 using namespace lldb_private;
+using namespace lldb_private::python;
 
 class PythonDataObjectsTest : public PythonTestSuite {
 public:
@@ -626,3 +627,116 @@
 }
   }
 }
+
+TEST_F(PythonDataObjectsTest, TestCallable) {
+
+  PythonDictionary globals(PyInitialValue::Empty);
+  auto builtins = PythonModule::BuiltinsModule();
+  llvm::Error error = globals.SetItem("__builtins__", builtins);
+  ASSERT_FALSE(error);
+
+  {
+PyObject *o = PyRun_String("lambda x : x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y=0: x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y=0, **kw: x", Py_eval_input,
+   globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y,*a: x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, true);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y,*a,**kw: x", Py_eval_input,
+   globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, true);
+  }
+
+  {
+const char *script = R"(
+class Foo:
+  def bar(self, x):
+ return x
+bar_bound   = Foo().bar
+bar_unbound = Foo.bar
+)";
+PyObject *o =
+PyRun_String(script, Py_file_input, globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+Take(o);
+
+auto bar_bound = As(globals.GetItem("bar_bound"));
+ASSERT_THAT_EXPECTED(bar_bound, llvm::Succeeded());
+auto arginfo = bar_bound.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2); // FIXME, wrong
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, true);
+
+auto bar_unbound = As(globals.GetItem("bar_unbound"));
+ASSERT_THAT_EXPECTED(bar_unbound, llvm::Succeeded());
+arginfo = bar_unbound.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+
+  // the old implementation of GetArgInfo just doesn't work on builtins.
+
+  {
+auto builtins = PythonModule::BuiltinsModule();
+auto hex = As(builtins.GetAttribute("hex"));
+ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());
+auto arginfo = hex.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+#endif
+}
\ No newline at end of file
Index: lldb/source/Plugins/ScriptInterp

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

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6237c9fe6ce9: [lldb] Don't emit artificial constructor 
declarations as global functions (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68130

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp
  
lldb/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1007,8 +1007,11 @@
   is_attr_used, attrs.is_artificial);
 
   type_handled = cxx_method_decl != NULL;
+  // Artificial methods are always handled even when don't
+  // create a new declaration for them.
+  type_handled |= attrs.is_artificial;
 
-  if (type_handled) {
+  if (cxx_method_decl) {
 LinkDeclContextToDIE(
 ClangASTContext::GetAsDeclContext(cxx_method_decl),
 die);
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp
===
--- /dev/null
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp
@@ -0,0 +1,10 @@
+struct Foo {
+  // Triggers that we emit an artificial constructor for Foo.
+  virtual ~Foo() = default;
+};
+
+int main() {
+  Foo f;
+  // Try to construct foo in our expression.
+  return 0; //%self.expect("expr Foo()", substrs=["(Foo) $0 = {}"])
+}
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
===
--- /dev/null
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(), None)
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/main.cpp
@@ -11,6 +11,7 @@
 
 int main() {
   Base realbase;
+  realbase.foo();
   Derived d;
   Base *b = &d;
   return 0; // Set breakpoint here
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
@@ -49,3 +49,7 @@
 
 # Test calling the base class.
 self.expect("expr realbase.foo()", substrs=["1"])
+
+# Test with locally constructed instances.
+self.expect("expr Base().foo()", substrs=["1"])
+self.expect("expr Derived().foo()", substrs=["2"])


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1007,8 +1007,11 @@
   is_attr_used, attrs.is_artificial);
 
   type_handled = cxx_method_decl != NULL;
+  // Artificial methods are always handled even when don't
+  // create a new declaration for them.
+  type_handled |= attrs.is_artificial;
 
-  if (type_handled) {
+  if (cxx_method_decl) {
 LinkDeclContextToDIE(
 ClangASTContext::GetAsDeclContext(cxx_method_decl),
 die);
Index: lldb/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/main.cpp

[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

[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/D68995/new/

https://reviews.llvm.org/D68995



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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:235-245
   PythonObject &operator=(const PythonObject &other) {
 Reset(PyRefType::Borrowed, other.get());
 return *this;
   }
 
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject &&other) {
 Reset();

lawrence_danna wrote:
> labath wrote:
> > You can consider simplifying this further down to a "universal"/"sink" 
> > `operator=(PythonObject other)`. Since the object is really just a pointer, 
> > the extra object being created won't hurt (in fact, the removal of 
> > `&`-indirection might make things faster).
> wouldn't that result in an extra retain and release every time a PythonObject 
> was copied instead of referenced or moved?
No, it shouldn't, because the temporary PythonObject will be move-constructed 
(== no refcount traffic), if the operator= is called with an xvalue (if the rhs 
was not an xvalue, then you wouldn't end up calling the `&&` overload anyway). 
Then you can move the temporary object into *this, and avoid refcount traffic 
again.

So, there is an additional PythonObject created, but it's move-constructed if 
possible, which should be efficient, if I understand these classes correctly. 
This is the recommended practice (at least by some) when you don't want to 
squeeze every last nanosecond of performance..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080



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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

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

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



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:235-245
   PythonObject &operator=(const PythonObject &other) {
 Reset(PyRefType::Borrowed, other.get());
 return *this;
   }
 
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject &&other) {
 Reset();

labath wrote:
> lawrence_danna wrote:
> > labath wrote:
> > > You can consider simplifying this further down to a "universal"/"sink" 
> > > `operator=(PythonObject other)`. Since the object is really just a 
> > > pointer, the extra object being created won't hurt (in fact, the removal 
> > > of `&`-indirection might make things faster).
> > wouldn't that result in an extra retain and release every time a 
> > PythonObject was copied instead of referenced or moved?
> No, it shouldn't, because the temporary PythonObject will be move-constructed 
> (== no refcount traffic), if the operator= is called with an xvalue (if the 
> rhs was not an xvalue, then you wouldn't end up calling the `&&` overload 
> anyway). Then you can move the temporary object into *this, and avoid 
> refcount traffic again.
> 
> So, there is an additional PythonObject created, but it's move-constructed if 
> possible, which should be efficient, if I understand these classes correctly. 
> This is the recommended practice (at least by some) when you don't want to 
> squeeze every last nanosecond of performance..
How do you move the temporary object into *this, if you only have 
`operator=(PythonObject other)` to assign with?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080



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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:235-245
   PythonObject &operator=(const PythonObject &other) {
 Reset(PyRefType::Borrowed, other.get());
 return *this;
   }
 
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject &&other) {
 Reset();

lawrence_danna wrote:
> labath wrote:
> > lawrence_danna wrote:
> > > labath wrote:
> > > > You can consider simplifying this further down to a "universal"/"sink" 
> > > > `operator=(PythonObject other)`. Since the object is really just a 
> > > > pointer, the extra object being created won't hurt (in fact, the 
> > > > removal of `&`-indirection might make things faster).
> > > wouldn't that result in an extra retain and release every time a 
> > > PythonObject was copied instead of referenced or moved?
> > No, it shouldn't, because the temporary PythonObject will be 
> > move-constructed (== no refcount traffic), if the operator= is called with 
> > an xvalue (if the rhs was not an xvalue, then you wouldn't end up calling 
> > the `&&` overload anyway). Then you can move the temporary object into 
> > *this, and avoid refcount traffic again.
> > 
> > So, there is an additional PythonObject created, but it's move-constructed 
> > if possible, which should be efficient, if I understand these classes 
> > correctly. This is the recommended practice (at least by some) when you 
> > don't want to squeeze every last nanosecond of performance..
> How do you move the temporary object into *this, if you only have 
> `operator=(PythonObject other)` to assign with?
In case that wasn't clear, the idea is to replace two operator= overloads with 
a single universal one taking a temporary. The advantage of that is less 
opportunities to implement move/copy incorrectly. The cost is one temporary 
move-constructed object more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080



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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

[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.h:235-245
   PythonObject &operator=(const PythonObject &other) {
 Reset(PyRefType::Borrowed, other.get());
 return *this;
   }
 
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject &&other) {
 Reset();

labath wrote:
> lawrence_danna wrote:
> > labath wrote:
> > > lawrence_danna wrote:
> > > > labath wrote:
> > > > > You can consider simplifying this further down to a 
> > > > > "universal"/"sink" `operator=(PythonObject other)`. Since the object 
> > > > > is really just a pointer, the extra object being created won't hurt 
> > > > > (in fact, the removal of `&`-indirection might make things faster).
> > > > wouldn't that result in an extra retain and release every time a 
> > > > PythonObject was copied instead of referenced or moved?
> > > No, it shouldn't, because the temporary PythonObject will be 
> > > move-constructed (== no refcount traffic), if the operator= is called 
> > > with an xvalue (if the rhs was not an xvalue, then you wouldn't end up 
> > > calling the `&&` overload anyway). Then you can move the temporary object 
> > > into *this, and avoid refcount traffic again.
> > > 
> > > So, there is an additional PythonObject created, but it's 
> > > move-constructed if possible, which should be efficient, if I understand 
> > > these classes correctly. This is the recommended practice (at least by 
> > > some) when you don't want to squeeze every last nanosecond of 
> > > performance..
> > How do you move the temporary object into *this, if you only have 
> > `operator=(PythonObject other)` to assign with?
> In case that wasn't clear, the idea is to replace two operator= overloads 
> with a single universal one taking a temporary. The advantage of that is less 
> opportunities to implement move/copy incorrectly. The cost is one temporary 
> move-constructed object more.
so does that amount to just deleting the copy-assign, and keeping the 
move-assign how it is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080



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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:235-245
   PythonObject &operator=(const PythonObject &other) {
 Reset(PyRefType::Borrowed, other.get());
 return *this;
   }
 
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject &&other) {
 Reset();

lawrence_danna wrote:
> labath wrote:
> > lawrence_danna wrote:
> > > labath wrote:
> > > > lawrence_danna wrote:
> > > > > labath wrote:
> > > > > > You can consider simplifying this further down to a 
> > > > > > "universal"/"sink" `operator=(PythonObject other)`. Since the 
> > > > > > object is really just a pointer, the extra object being created 
> > > > > > won't hurt (in fact, the removal of `&`-indirection might make 
> > > > > > things faster).
> > > > > wouldn't that result in an extra retain and release every time a 
> > > > > PythonObject was copied instead of referenced or moved?
> > > > No, it shouldn't, because the temporary PythonObject will be 
> > > > move-constructed (== no refcount traffic), if the operator= is called 
> > > > with an xvalue (if the rhs was not an xvalue, then you wouldn't end up 
> > > > calling the `&&` overload anyway). Then you can move the temporary 
> > > > object into *this, and avoid refcount traffic again.
> > > > 
> > > > So, there is an additional PythonObject created, but it's 
> > > > move-constructed if possible, which should be efficient, if I 
> > > > understand these classes correctly. This is the recommended practice 
> > > > (at least by some) when you don't want to squeeze every last nanosecond 
> > > > of performance..
> > > How do you move the temporary object into *this, if you only have 
> > > `operator=(PythonObject other)` to assign with?
> > In case that wasn't clear, the idea is to replace two operator= overloads 
> > with a single universal one taking a temporary. The advantage of that is 
> > less opportunities to implement move/copy incorrectly. The cost is one 
> > temporary move-constructed object more.
> so does that amount to just deleting the copy-assign, and keeping the 
> move-assign how it is?
> How do you move the temporary object into *this, if you only have 
> operator=(PythonObject other) to assign with?

You need to do it manually, like the current `&&` overload does, but you don't 
also need to implement the copy semantics in the const& overload.

> so does that amount to just deleting the copy-assign, and keeping the 
> move-assign how it is?
Almost. The implementation of move-assign would remain the same, but you'd drop 
the `&&` (otherwise you'd lose the ability to copy-assign) from the signature. 
I.e., 
```
PythonObject &operator=(PythonObject other) {
Reset();
m_py_obj = std::exchange(other.m_py_obj, nullptr); // I just learned of 
this today so I have to show off.
return *this;
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080



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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

[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.h:235-245
   PythonObject &operator=(const PythonObject &other) {
 Reset(PyRefType::Borrowed, other.get());
 return *this;
   }
 
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject &&other) {
 Reset();

labath wrote:
> lawrence_danna wrote:
> > labath wrote:
> > > lawrence_danna wrote:
> > > > labath wrote:
> > > > > lawrence_danna wrote:
> > > > > > labath wrote:
> > > > > > > You can consider simplifying this further down to a 
> > > > > > > "universal"/"sink" `operator=(PythonObject other)`. Since the 
> > > > > > > object is really just a pointer, the extra object being created 
> > > > > > > won't hurt (in fact, the removal of `&`-indirection might make 
> > > > > > > things faster).
> > > > > > wouldn't that result in an extra retain and release every time a 
> > > > > > PythonObject was copied instead of referenced or moved?
> > > > > No, it shouldn't, because the temporary PythonObject will be 
> > > > > move-constructed (== no refcount traffic), if the operator= is called 
> > > > > with an xvalue (if the rhs was not an xvalue, then you wouldn't end 
> > > > > up calling the `&&` overload anyway). Then you can move the temporary 
> > > > > object into *this, and avoid refcount traffic again.
> > > > > 
> > > > > So, there is an additional PythonObject created, but it's 
> > > > > move-constructed if possible, which should be efficient, if I 
> > > > > understand these classes correctly. This is the recommended practice 
> > > > > (at least by some) when you don't want to squeeze every last 
> > > > > nanosecond of performance..
> > > > How do you move the temporary object into *this, if you only have 
> > > > `operator=(PythonObject other)` to assign with?
> > > In case that wasn't clear, the idea is to replace two operator= overloads 
> > > with a single universal one taking a temporary. The advantage of that is 
> > > less opportunities to implement move/copy incorrectly. The cost is one 
> > > temporary move-constructed object more.
> > so does that amount to just deleting the copy-assign, and keeping the 
> > move-assign how it is?
> > How do you move the temporary object into *this, if you only have 
> > operator=(PythonObject other) to assign with?
> 
> You need to do it manually, like the current `&&` overload does, but you 
> don't also need to implement the copy semantics in the const& overload.
> 
> > so does that amount to just deleting the copy-assign, and keeping the 
> > move-assign how it is?
> Almost. The implementation of move-assign would remain the same, but you'd 
> drop the `&&` (otherwise you'd lose the ability to copy-assign) from the 
> signature. I.e., 
> ```
> PythonObject &operator=(PythonObject other) {
> Reset();
> m_py_obj = std::exchange(other.m_py_obj, nullptr); // I just learned of 
> this today so I have to show off.
> return *this;
>   }
> ```
oooh, i get it.   It didn't occur to me that you could treat other as a 
rvalue without passing it as one in the parameter list, but of course you can 
because if it's pass-by-value then it's just a local variable by the time 
operator= gets its hands on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080



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

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

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

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

universal assignment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -323,8 +323,8 @@
   PythonList list(PyRefType::Owned, py_list);
 
   PythonObject list_items[list_size];
-  list_items[0].Reset(PythonInteger(long_value0));
-  list_items[1].Reset(PythonString(string_value1));
+  list_items[0] = PythonInteger(long_value0);
+  list_items[1] = PythonString(string_value1);
 
   for (unsigned i = 0; i < list_size; ++i)
 list.SetItemAtIndex(i, list_items[i]);
@@ -469,10 +469,10 @@
   PythonObject py_keys[dict_entries];
   PythonObject py_values[dict_entries];
 
-  py_keys[0].Reset(PythonString(key_0));
-  py_keys[1].Reset(PythonInteger(key_1));
-  py_values[0].Reset(PythonInteger(value_0));
-  py_values[1].Reset(PythonString(value_1));
+  py_keys[0] = PythonString(key_0);
+  py_keys[1] = PythonInteger(key_1);
+  py_values[0] = PythonInteger(value_0);
+  py_values[1] = PythonString(value_1);
 
   PyObject *py_dict = PyDict_New();
   EXPECT_TRUE(PythonDictionary::Check(py_dict));
@@ -509,10 +509,10 @@
   PythonString keys[dict_entries];
   PythonObject values[dict_entries];
 
-  keys[0].Reset(PythonString(key_0));
-  keys[1].Reset(PythonString(key_1));
-  values[0].Reset(PythonInteger(value_0));
-  values[1].Reset(PythonString(value_1));
+  keys[0] = PythonString(key_0);
+  keys[1] = PythonString(key_1);
+  values[0] = PythonInteger(value_0);
+  values[1] = PythonString(value_1);
 
   PythonDictionary dict(PyInitialValue::Empty);
   for (int i = 0; i < 2; ++i)
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -182,7 +182,8 @@
 Reset(type, py_obj);
   }
 
-  PythonObject(const PythonObject &rhs) : m_py_obj(nullptr) { Reset(rhs); }
+  PythonObject(const PythonObject &rhs)
+  : PythonObject(PyRefType::Borrowed, rhs.m_py_obj) {}
 
   PythonObject(PythonObject &&rhs) {
 m_py_obj = rhs.m_py_obj;
@@ -197,19 +198,6 @@
 m_py_obj = nullptr;
   }
 
-  void Reset(const PythonObject &rhs) {
-if (!rhs.IsValid())
-  Reset();
-else
-  Reset(PyRefType::Borrowed, rhs.m_py_obj);
-  }
-
-  // PythonObject is implicitly convertible to PyObject *, which will call the
-  // wrong overload.  We want to explicitly disallow this, since a PyObject
-  // *always* owns its reference.  Therefore the overload which takes a
-  // PyRefType doesn't make sense, and the copy constructor should be used.
-  void Reset(PyRefType type, const PythonObject &ref) = delete;
-
   void Reset(PyRefType type, PyObject *py_obj) {
 if (py_obj == m_py_obj)
   return;
@@ -244,19 +232,9 @@
 return result;
   }
 
-  PythonObject &operator=(const PythonObject &other) {
-Reset(PyRefType::Borrowed, other.get());
-return *this;
-  }
-
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject other) {
 Reset();
-m_py_obj = other.m_py_obj;
-other.m_py_obj = nullptr;
-  }
-
-  PythonObject &operator=(PythonObject &&other) {
-Reset(std::move(other));
+m_py_obj = std::exchange(other.m_py_obj, nullptr);
 return *this;
   }
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -410,7 +410,7 @@
 llvm::consumeError(s.takeError());
 Reset();
   } else {
-PythonObject::Reset(std::move(s.get()));
+*this = std::move(s.get());
   }
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r375156 - [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Martin Storsjo via lldb-commits]

Author: mstorsjo
Date: Thu Oct 17 12:22:50 2019
New Revision: 375156

URL: http://llvm.org/viewvc/llvm-project?rev=375156&view=rev
Log:
[LLDB] [test] Use %clang_cl instead of build.py in a few tests

This allows explicitly specifying the intended target architecture,
for tests that aren't supposed to be executed, and that don't
require MSVC headers or libraries to be available.

(These tests already implicitly assumed to be built for x86; one
didn't specify anything, assuming x86_64, while the other specified
--arch=32, which only picks the 32 bit variant of the default target
architecture).

Join two comment lines in disassembly.cpp, to keep row numbers
checked in the test unchanged.

This fixes running check-lldb on arm linux.

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

Modified:
lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp
lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp

Modified: lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp?rev=375156&r1=375155&r2=375156&view=diff
==
--- lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp (original)
+++ lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp Thu Oct 17 
12:22:50 2019
@@ -2,12 +2,12 @@
 // REQUIRES: lld
 
 // Test that we can show disassembly and source.
-// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s 
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/disassembly.lldbinit | FileCheck %s
 
-// Some context lines before
-// the function.
+// Some context lines before the function.
 
 int foo() { return 42; }
 

Modified: 
lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp?rev=375156&r1=375155&r2=375156&view=diff
==
--- lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp 
(original)
+++ lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp 
Thu Oct 17 12:22:50 2019
@@ -1,7 +1,8 @@
 // clang-format off
 // REQUIRES: lld
 
-// RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib -o %t.exe -- %s 
+// RUN: %clang_cl --target=i386-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/function-types-calling-conv.lldbinit | FileCheck %s
 


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

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG95980409e653: [LLDB] [test] Use %clang_cl instead of 
build.py in a few tests (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031

Files:
  lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
  lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
@@ -1,7 +1,8 @@
 // clang-format off
 // REQUIRES: lld
 
-// RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib -o %t.exe -- %s 
+// RUN: %clang_cl --target=i386-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/function-types-calling-conv.lldbinit | FileCheck %s
 
Index: lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
@@ -2,12 +2,12 @@
 // REQUIRES: lld
 
 // Test that we can show disassembly and source.
-// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s 
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/disassembly.lldbinit | FileCheck %s
 
-// Some context lines before
-// the function.
+// Some context lines before the function.
 
 int foo() { return 42; }
 


Index: lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
@@ -1,7 +1,8 @@
 // clang-format off
 // REQUIRES: lld
 
-// RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib -o %t.exe -- %s 
+// RUN: %clang_cl --target=i386-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/function-types-calling-conv.lldbinit | FileCheck %s
 
Index: lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
@@ -2,12 +2,12 @@
 // REQUIRES: lld
 
 // Test that we can show disassembly and source.
-// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s 
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/disassembly.lldbinit | FileCheck %s
 
-// Some context lines before
-// the function.
+// Some context lines before the function.
 
 int foo() { return 42; }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68968: [android/process info] Introduce bundle id

[Dan Albert via Phabricator via lldb-commits]

danalbert added a subscriber: enh.
danalbert added a comment.

In D68968#1710520 , @labath wrote:

> Independently, I am wondering if there's a better way to link the process id 
> to a bundle. Using argv[0] might be ok if we're using it just for display 
> purposes, but if we're going to be doing other stuff based on that 
> identifier, it would be better to get it from a more reliable source. 
> Unfortunately, I was not able to find a more "reasonable source", but maybe 
> @danalbert has an idea.


@enh might




Comment at: lldb/source/Host/linux/Host.cpp:220-222
+  if (process_info.GetNameAsStringRef().empty() &&
+  !process_info.GetArg0().empty()) {
+process_info.SetBundleID(process_info.GetArg0());

labath wrote:
> wallace wrote:
> > labath wrote:
> > > How sure are we that the app processes are the only ones which have the 
> > > exe link unreadable? Will that be true on all phones or just the recent 
> > > ones with all the selinux stuff?
> > > 
> > > I was hoping that there is some more reliable way of fetching this 
> > > information, but there doesn't seem to be anything except [[ 
> > > https://developer.android.com/reference/android/app/ActivityManager.RunningAppProcessInfo.html
> > >  | ActivityManager.RunningAppProcessInfo ]], which I don't know if it can 
> > > be accessed from lldb, either host- or device-side.
> > > 
> > > @danalbert, any ideas here?
> > Another option I've just discovered is to invoke `pm list packages` to get 
> > the list of all apks and if an Arg0 is in that list, then it's a package.
> > That seems good enough
> That should prevent us accidentally setting an incorrect bundle id, but it 
> does not prevent a process from deliberately changing its argv[0] to the name 
> of some other installed package. That seems suboptimal, particularly if we're 
> later going to use start using the bundle id for other than just display 
> purposes (e.g. for issuing "am kill" commands). So, I am still wondering if 
> we shouldn't go back to using argv[0] for the purpose of "process list", and 
> leave the "bundle id" field for cases where we can get this information from 
> a more reliable source (e.g. reading it from the apk, or fetching it from the 
> android package manager, etc). What do you think?
@enh 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68968



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

[Lldb-commits] [PATCH] D69133: eliminate nontrivial Reset(...) from TypedPythonObject

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

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, clayborg, labath, jingham.
Herald added a project: LLDB.
lawrence_danna added a parent revision: D69080: eliminate one form of 
PythonObject::Reset().

This deletes `Reset(...)`, except for the no-argument form `Reset()`
from `TypedPythonObject`, and therefore from `PythonString`, `PythonList`, 
etc.

It updates the various callers to use assignment, `As<>`, `Take<>`, 
and `Retain<>`, as appropriate.

followon to https://reviews.llvm.org/D69080


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69133

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -27,8 +27,7 @@
   void SetUp() override {
 PythonTestSuite::SetUp();
 
-PythonString sys_module("sys");
-m_sys_module.Reset(PyRefType::Owned, PyImport_Import(sys_module.get()));
+m_sys_module = unwrapIgnoringErrors(PythonModule::Import("sys"));
 m_main_module = PythonModule::MainModule();
 m_builtins_module = PythonModule::BuiltinsModule();
   }
@@ -70,13 +69,10 @@
   PythonDictionary dict(PyInitialValue::Empty);
 
   PyObject *new_dict = PyDict_New();
-  dict.Reset(PyRefType::Owned, new_dict);
+  dict = Take(new_dict);
   EXPECT_EQ(new_dict, dict.get());
 
-  dict.Reset(PyRefType::Owned, nullptr);
-  EXPECT_EQ(nullptr, dict.get());
-
-  dict.Reset(PyRefType::Owned, PyDict_New());
+  dict = Take(PyDict_New());
   EXPECT_NE(nullptr, dict.get());
   dict.Reset();
   EXPECT_EQ(nullptr, dict.get());
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -55,6 +55,7 @@
 
 using namespace lldb;
 using namespace lldb_private;
+using namespace lldb_private::python;
 
 // Defined in the SWIG source file
 #if PY_MAJOR_VERSION >= 3
@@ -765,19 +766,16 @@
   if (!main_dict.IsValid())
 return m_session_dict;
 
-  PythonObject item = main_dict.GetItemForKey(PythonString(m_dictionary_name));
-  m_session_dict.Reset(PyRefType::Borrowed, item.get());
+  m_session_dict = unwrapIgnoringErrors(
+  As(main_dict.GetItem(m_dictionary_name)));
   return m_session_dict;
 }
 
 PythonDictionary &ScriptInterpreterPythonImpl::GetSysModuleDictionary() {
   if (m_sys_module_dict.IsValid())
 return m_sys_module_dict;
-
-  PythonObject sys_module(PyRefType::Borrowed, PyImport_AddModule("sys"));
-  if (sys_module.IsValid())
-m_sys_module_dict.Reset(PyRefType::Borrowed,
-PyModule_GetDict(sys_module.get()));
+  PythonModule sys_module = unwrapIgnoringErrors(PythonModule::Import("sys"));
+  m_sys_module_dict = sys_module.GetDictionary();
   return m_sys_module_dict;
 }
 
@@ -1053,9 +1051,8 @@
   PythonDictionary locals = GetSessionDictionary();
 
   if (!locals.IsValid()) {
-locals.Reset(
-PyRefType::Owned,
-PyObject_GetAttrString(globals.get(), m_dictionary_name.c_str()));
+locals = unwrapIgnoringErrors(
+As(globals.GetAttribute(m_dictionary_name)));
   }
 
   if (!locals.IsValid())
@@ -1204,9 +1201,8 @@
   PythonDictionary locals = GetSessionDictionary();
 
   if (!locals.IsValid())
-locals.Reset(
-PyRefType::Owned,
-PyObject_GetAttrString(globals.get(), m_dictionary_name.c_str()));
+locals = unwrapIgnoringErrors(
+As(globals.GetAttribute(m_dictionary_name)));
 
   if (!locals.IsValid())
 locals = globals;
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -332,6 +332,10 @@
 return python::Take(obj);
   }
 
+  llvm::Expected GetAttribute(const std::string &name) const {
+return GetAttribute(name.c_str());
+  }
+
   llvm::Expected IsTrue() {
 if (!m_py_obj)
   return nullDeref();
@@ -394,8 +398,12 @@
 
   using PythonObject::Reset;
 
-  void Reset(PyRefType type, PyObject *py_obj) {
-Reset();
+  void Reset() { PythonObject::Reset(); }
+
+  void Reset(PyRefType type, PyObject *py_obj) = delete;
+
+  TypedPythonObject(PyRefType type, PyObject *py_obj) {
+m_py_obj = nullptr;
 if (!py_obj)
   return;
 T::Convert(type, py_obj);
@@ -405,8 +413,6 @@
 

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Either this or https://reviews.llvm.org/D69076 broke the lldb-cmake bot: 
http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/2706/

  Script:
  --
  : 'RUN: at line 4';   
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/clang 
--driver-mode=cl -isysroot 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
 
-fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/lldb-test-build.noindex/module-cache-clang/lldb-shell
 --target=i386-windows-msvc -Od -Z7 -c 
/Fo/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/test/SymbolFile/NativePDB/Output/function-types-calling-conv.cpp.tmp.obj
 -- 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
  : 'RUN: at line 5';   
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/lld-link 
-debug:full -nodefaultlib -entry:main 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/test/SymbolFile/NativePDB/Output/function-types-calling-conv.cpp.tmp.obj
 
-out:/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/test/SymbolFile/NativePDB/Output/function-types-calling-conv.cpp.tmp.exe
 
-pdb:/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/test/SymbolFile/NativePDB/Output/function-types-calling-conv.cpp.tmp.pdb
  : 'RUN: at line 6';   env LLDB_USE_NATIVE_PDB_READER=1 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/lldb 
--no-lldbinit -S 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/test/Shell/lit-lldb-init
 -f 
/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/test/SymbolFile/NativePDB/Output/function-types-calling-conv.cpp.tmp.exe
 -s  
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/Shell/SymbolFile/NativePDB/Inputs/function-types-calling-conv.lldbinit
 | /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/FileCheck 
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/lldb/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  clang-10: warning: unknown argument ignored in clang-cl: '-isysroot' 
[-Wunknown-argument]
  clang-10: warning: unknown argument ignored in clang-cl: 
'-fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/lldb-test-build.noindex/module-cache-clang/lldb-shell'
 [-Wunknown-argument]
  clang-10: error: cannot specify 
'/Fo/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/test/SymbolFile/NativePDB/Output/function-types-calling-conv.cpp.tmp.obj'
 when compiling multiple source files
  clang-10: warning: 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk:
 'linker' input unused [-Wunused-command-line-argument]
  
  --


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031



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

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Probably this commit. Can you please revert or fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031



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

[Lldb-commits] [lldb] r375163 - Revert "[LLDB] [test] Use %clang_cl instead of build.py in a few tests"

[Martin Storsjo via lldb-commits]

Author: mstorsjo
Date: Thu Oct 17 13:14:19 2019
New Revision: 375163

URL: http://llvm.org/viewvc/llvm-project?rev=375163&view=rev
Log:
Revert "[LLDB] [test] Use %clang_cl instead of build.py in a few tests"

This reverts SVN r375156, as it seems to have broken tests when run
on macOS: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/2706/console

Modified:
lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp
lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp

Modified: lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp?rev=375163&r1=375162&r2=375163&view=diff
==
--- lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp (original)
+++ lldb/trunk/test/Shell/SymbolFile/NativePDB/disassembly.cpp Thu Oct 17 
13:14:19 2019
@@ -2,12 +2,12 @@
 // REQUIRES: lld
 
 // Test that we can show disassembly and source.
-// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
-// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
+// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s 
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/disassembly.lldbinit | FileCheck %s
 
-// Some context lines before the function.
+// Some context lines before
+// the function.
 
 int foo() { return 42; }
 

Modified: 
lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp?rev=375163&r1=375162&r2=375163&view=diff
==
--- lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp 
(original)
+++ lldb/trunk/test/Shell/SymbolFile/NativePDB/function-types-calling-conv.cpp 
Thu Oct 17 13:14:19 2019
@@ -1,8 +1,7 @@
 // clang-format off
 // REQUIRES: lld
 
-// RUN: %clang_cl --target=i386-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
-// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
+// RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib -o %t.exe -- %s 
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
 // RUN: %p/Inputs/function-types-calling-conv.lldbinit | FileCheck %s
 


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

[Lldb-commits] [PATCH] D69058: [test] Add a .clang-format file for the shell test.

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM, thanks!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69058



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

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

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

Unfortunately, this is failing on the windows bot:

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68130



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

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D69031#1713532 , @aprantl wrote:

> Probably this commit. Can you please revert or fix?


Sorry about this, reverted it for now.

I think the reason might be that %clang_cl ends up expanding to something 
(maybe `-isysroot`) that clang-cl doesn't parse properly but ends up parsing as 
an input filename, will try to reproduce it locally on macOS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031



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

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 225508.
shafik added a comment.

Minor fixes.


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

https://reviews.llvm.org/D68961

Files:
  include/lldb/Symbol/ClangASTContext.h
  packages/Python/lldbsuite/test/python_api/type/TestTypeList.py
  packages/Python/lldbsuite/test/python_api/type/main.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  source/Symbol/ClangASTContext.cpp
  test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp

Index: test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
===
--- /dev/null
+++ test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
@@ -0,0 +1,19 @@
+// Test to verify we are corectly generating anonymous flags when parsing
+// anonymous class and unnamed structs from DWARF to the a clang AST node.
+
+// RUN: %clang++ -g -c -o %t.o %s
+// RUN: lldb-test symbols -dump-clang-ast %t.o | FileCheck %s
+
+struct A {
+  struct {
+int x;
+  };
+  struct {
+int y;
+  } C;
+} a;
+
+// CHECK: A::(anonymous struct)
+// CHECK: |-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
+// CHECK: A::(anonymous struct)
+// CHECK: |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -1346,11 +1346,9 @@
 
 #pragma mark Structure, Unions, Classes
 
-CompilerType ClangASTContext::CreateRecordType(DeclContext *decl_ctx,
-   AccessType access_type,
-   const char *name, int kind,
-   LanguageType language,
-   ClangASTMetadata *metadata) {
+CompilerType ClangASTContext::CreateRecordType(
+DeclContext *decl_ctx, AccessType access_type, const char *name, int kind,
+LanguageType language, ClangASTMetadata *metadata, bool exports_symbols) {
   ASTContext *ast = getASTContext();
   assert(ast != nullptr);
 
@@ -1401,10 +1399,7 @@
 // Anonymous classes is a GNU/MSVC extension that clang supports. It
 // requires the anonymous class be embedded within a class. So the new
 // heuristic verifies this condition.
-//
-// FIXME: An unnamed class within a class is also wrongly recognized as an
-// anonymous struct.
-if (isa(decl_ctx))
+if (isa(decl_ctx) && exports_symbols)
   decl->setAnonymousStructOrUnion(true);
   }
 
@@ -8988,7 +8983,7 @@
 TypeSP type = type_list.GetTypeAtIndex(i);
 
 if (!symbol_name.empty())
-  if (symbol_name.compare(type->GetName().GetStringRef()) != 0)
+  if (symbol_name != type->GetName().GetStringRef())
 continue;
 
 s << type->GetName().AsCString() << "\n";
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -181,6 +181,7 @@
   bool is_scoped_enum = false;
   bool is_vector = false;
   bool is_virtual = false;
+  bool exports_symbols = false;
   clang::StorageClass storage = clang::SC_None;
   const char *mangled_name = nullptr;
   lldb_private::ConstString name;
Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -347,6 +347,9 @@
 case DW_AT_GNU_vector:
   is_vector = form_value.Boolean();
   break;
+case DW_AT_export_symbols:
+  exports_symbols = form_value.Boolean();
+  break;
 }
   }
 }
@@ -1543,7 +1546,7 @@
   clang_type_was_created = true;
   clang_type = m_ast.CreateRecordType(
   decl_ctx, attrs.accessibility, attrs.name.GetCString(), tag_decl_kind,
-  attrs.class_language, &metadata);
+  attrs.class_language, &metadata, attrs.exports_symbols);
 }
   }
 
Index: packages/Python/lldbsuite/test/python_api/type/main.cpp
===
--- packages/Python/lldbsuite/test/python_api/type/main.cpp
+++ packages/Python/lldbsuite/test/python_api/type/main.cpp
@@ -15,6 +15,14 @@
 TASK_TYPE_1,
 TASK_TYPE_2
 } type;
+// This struct is anonymous b/c it does not have a name
+// and it is not unnamed class.
+// Anonymous classes are a GNU extension.
+struct {
+  int y;
+};
+// This struct is an unnamed class see [class.pre]p1
+// http://eel.is/

[Lldb-commits] [PATCH] D68961: Add support for DW_AT_export_symbols for anonymous structs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D68961#1711537 , @labath wrote:

> In D68961#1711407 , @shafik wrote:
>
> > In D68961#1709708 , @clayborg 
> > wrote:
> >
> > > Have many compilers supported DW_AT_export_symbols for a while now? If 
> > > not, are there any serious issues introduced here that would change 
> > > debugger behavior if this attribute is not emitted by a compiler? Or is 
> > > this a new fix in clang that was recently introduced in order to fix an 
> > > issue when debugging in lldb?
> >
> >
> > We don't except any regressions for code compiled with older compilers. We 
> > are fixing the case that unnamed classes are identified as anonymous. The 
> > anonymous classes cases should be caught in older revisions in 
> > `ClangASTContext::AddFieldToRecordType` which does a 
> > `Rec->setAnonymousStructOrUnion(true)` for those cases.
>
>
> Would it make sense to write a test in asm then? That way it would be obvious 
> exactly what kind of debug info is being tested, and you could ensure both 
> old&new compiler behavior is covered.


The `TestTypeList.py` covers the relevant cases of both anonymous class and an 
unnamed class. I ran `check-lldb` with the feature for emitting 
`DW_AT_export_symbols` disabled and did not see any regressions in this test or 
others except `

We also have the lldb-cmake-matrix 
 bot which 
runs the test suite using older versions of clang which is there for exactly 
this purpose to catch regressions due to features not supported by older 
compiler versions. Which would catch any regressions here.


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

https://reviews.llvm.org/D68961



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

Re: [Lldb-commits] [PATCH] D68968: [android/process info] Introduce bundle id

[enh via lldb-commits]

nothing i know of. +yabinc might have dealt with this with simpleperf...

On Thu, Oct 17, 2019 at 12:54 PM Dan Albert via Phabricator
 wrote:
>
> danalbert added a subscriber: enh.
> danalbert added a comment.
>
> In D68968#1710520 , @labath wrote:
>
> > Independently, I am wondering if there's a better way to link the process 
> > id to a bundle. Using argv[0] might be ok if we're using it just for 
> > display purposes, but if we're going to be doing other stuff based on that 
> > identifier, it would be better to get it from a more reliable source. 
> > Unfortunately, I was not able to find a more "reasonable source", but maybe 
> > @danalbert has an idea.
>
>
> @enh might
>
>
>
> 
> Comment at: lldb/source/Host/linux/Host.cpp:220-222
> +  if (process_info.GetNameAsStringRef().empty() &&
> +  !process_info.GetArg0().empty()) {
> +process_info.SetBundleID(process_info.GetArg0());
> 
> labath wrote:
> > wallace wrote:
> > > labath wrote:
> > > > How sure are we that the app processes are the only ones which have the 
> > > > exe link unreadable? Will that be true on all phones or just the recent 
> > > > ones with all the selinux stuff?
> > > >
> > > > I was hoping that there is some more reliable way of fetching this 
> > > > information, but there doesn't seem to be anything except [[ 
> > > > https://developer.android.com/reference/android/app/ActivityManager.RunningAppProcessInfo.html
> > > >  | ActivityManager.RunningAppProcessInfo ]], which I don't know if it 
> > > > can be accessed from lldb, either host- or device-side.
> > > >
> > > > @danalbert, any ideas here?
> > > Another option I've just discovered is to invoke `pm list packages` to 
> > > get the list of all apks and if an Arg0 is in that list, then it's a 
> > > package.
> > > That seems good enough
> > That should prevent us accidentally setting an incorrect bundle id, but it 
> > does not prevent a process from deliberately changing its argv[0] to the 
> > name of some other installed package. That seems suboptimal, particularly 
> > if we're later going to use start using the bundle id for other than just 
> > display purposes (e.g. for issuing "am kill" commands). So, I am still 
> > wondering if we shouldn't go back to using argv[0] for the purpose of 
> > "process list", and leave the "bundle id" field for cases where we can get 
> > this information from a more reliable source (e.g. reading it from the apk, 
> > or fetching it from the android package manager, etc). What do you think?
> @enh
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D68968/new/
>
> https://reviews.llvm.org/D68968
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r375170 - Adapt Windows test to API change.

[Adrian Prantl via lldb-commits]

Author: adrian
Date: Thu Oct 17 13:51:55 2019
New Revision: 375170

URL: http://llvm.org/viewvc/llvm-project?rev=375170&view=rev
Log:
Adapt Windows test to API change.

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

Modified: lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp?rev=375170&r1=375169&r2=375170&view=diff
==
--- lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp Thu Oct 17 
13:51:55 2019
@@ -603,9 +603,8 @@ TEST_F(SymbolFilePDBTests, TestFindSymbo
   lldb::ModuleSP module = std::make_shared(fspec, aspec);
 
   SymbolContextList sc_list;
-  EXPECT_EQ(1u,
-module->FindSymbolsWithNameAndType(ConstString("?foo@@YAHH@Z"),
-   lldb::eSymbolTypeAny, sc_list));
+  module->FindSymbolsWithNameAndType(ConstString("?foo@@YAHH@Z"),
+ lldb::eSymbolTypeAny, sc_list);
   EXPECT_EQ(1u, sc_list.GetSize());
 
   SymbolContext sc;


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

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D69031#1713060 , @stella.stamenova 
wrote:

> The tests passed in my setup. After you commit this, please monitor the 
> windows Buildbot (it currently has a couple of failures, so you will have to 
> check it and not rely on an email).


I reverted this due to issues when run on macOS, but it did seem to pass 
without adding any new failures to the windows bot, fwiw.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031



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

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

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Yeah, seems like constructing objects in expressions isn't implemented on 
Windows. I'm not sure if there is a reliable way to test that constructors 
aren't shadowed by these global functions if constructors themselves don't work 
on Windows, but I filed llvm.org/pr43707 for the underlying bug and x-failed 
the tests. Will push in a few minutes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68130



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

Re: [Lldb-commits] [PATCH] D68968: [android/process info] Introduce bundle id

[Yabin Cui via lldb-commits]

simpleperf reads argv[0] from /proc//cmdline for display purposes. If
not for display, a package name is expected from user inputs.

On Thu, Oct 17, 2019 at 1:33 PM enh  wrote:

> nothing i know of. +yabinc might have dealt with this with simpleperf...
>
> On Thu, Oct 17, 2019 at 12:54 PM Dan Albert via Phabricator
>  wrote:
> >
> > danalbert added a subscriber: enh.
> > danalbert added a comment.
> >
> > In D68968#1710520 , @labath
> wrote:
> >
> > > Independently, I am wondering if there's a better way to link the
> process id to a bundle. Using argv[0] might be ok if we're using it just
> for display purposes, but if we're going to be doing other stuff based on
> that identifier, it would be better to get it from a more reliable source.
> Unfortunately, I was not able to find a more "reasonable source", but maybe
> @danalbert has an idea.
> >
> >
> > @enh might
> >
> >
> >
> > 
> > Comment at: lldb/source/Host/linux/Host.cpp:220-222
> > +  if (process_info.GetNameAsStringRef().empty() &&
> > +  !process_info.GetArg0().empty()) {
> > +process_info.SetBundleID(process_info.GetArg0());
> > 
> > labath wrote:
> > > wallace wrote:
> > > > labath wrote:
> > > > > How sure are we that the app processes are the only ones which
> have the exe link unreadable? Will that be true on all phones or just the
> recent ones with all the selinux stuff?
> > > > >
> > > > > I was hoping that there is some more reliable way of fetching this
> information, but there doesn't seem to be anything except [[
> https://developer.android.com/reference/android/app/ActivityManager.RunningAppProcessInfo.html
> | ActivityManager.RunningAppProcessInfo ]], which I don't know if it can be
> accessed from lldb, either host- or device-side.
> > > > >
> > > > > @danalbert, any ideas here?
> > > > Another option I've just discovered is to invoke `pm list packages`
> to get the list of all apks and if an Arg0 is in that list, then it's a
> package.
> > > > That seems good enough
> > > That should prevent us accidentally setting an incorrect bundle id,
> but it does not prevent a process from deliberately changing its argv[0] to
> the name of some other installed package. That seems suboptimal,
> particularly if we're later going to use start using the bundle id for
> other than just display purposes (e.g. for issuing "am kill" commands). So,
> I am still wondering if we shouldn't go back to using argv[0] for the
> purpose of "process list", and leave the "bundle id" field for cases where
> we can get this information from a more reliable source (e.g. reading it
> from the apk, or fetching it from the android package manager, etc). What
> do you think?
> > @enh
> >
> >
> > Repository:
> >   rG LLVM Github Monorepo
> >
> > CHANGES SINCE LAST ACTION
> >   https://reviews.llvm.org/D68968/new/
> >
> > https://reviews.llvm.org/D68968
> >
> >
> >
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69119: Modernize the rest of the Find.* API (NFC)

[Sterling Augustine via Phabricator via lldb-commits]

saugustine added a comment.

This breaks the build due to 
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1355:8: error: unused 
variable 'old_size' [-Werror,-Wunused-variable]

I will check in a fix shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69119



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

[Lldb-commits] [lldb] r375172 - [test] Add a .clang-format file for the shell test.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Thu Oct 17 14:23:35 2019
New Revision: 375172

URL: http://llvm.org/viewvc/llvm-project?rev=375172&view=rev
Log:
[test] Add a .clang-format file for the shell test.

The API tests have a .clang-format file that disables formatting
altogether. While this is needed for some tests, it also leads to
inconsistency between test files. The shell tests suffer from a similar
problem: a test with a source-file extension (.c, .cpp) will get
formatted, potentially breaking up lines and leading to invalid RUN
commands.

Rather than completely disabling formatting here, I propose to not
enforce a line limit instead. That way tests will be consistent, but you
can still have long run commands (as is not uncommon in LLVM either) and
use breakpoints with patters that extend beyond 80 cols.

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

Added:
lldb/trunk/test/Shell/.clang-format

Added: lldb/trunk/test/Shell/.clang-format
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/.clang-format?rev=375172&view=auto
==
--- lldb/trunk/test/Shell/.clang-format (added)
+++ lldb/trunk/test/Shell/.clang-format Thu Oct 17 14:23:35 2019
@@ -0,0 +1,3 @@
+BasedOnStyle: LLVM
+# We don't want clang-format to introduce line breaks in RUN commands.
+ColumnLimit: 999


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

[Lldb-commits] [lldb] r375173 - [lldb] X-fail tests that use constructors in expressions on Windows

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Thu Oct 17 14:27:26 2019
New Revision: 375173

URL: http://llvm.org/viewvc/llvm-project?rev=375173&view=rev
Log:
[lldb] X-fail tests that use constructors in expressions on Windows

These tests were testing a bug related to constructors. It seems that
on Windows the expression command can't construct objects (or at least,
call their constructor explicitly which is required for the tests), so
this is just x-failing them until Windows actually supports constructor calls.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py?rev=375173&r1=375172&r2=375173&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
 Thu Oct 17 14:27:26 2019
@@ -53,3 +53,20 @@ class ExprCommandCallOverriddenMethod(Te
 # Test with locally constructed instances.
 self.expect("expr Base().foo()", substrs=["1"])
 self.expect("expr Derived().foo()", substrs=["2"])
+
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr43707")
+def test_call_on_temporary(self):
+"""Test calls to overridden methods in derived classes."""
+self.build()
+
+# Set breakpoint in main and run exe
+self.runCmd("file " + self.getBuildArtifact("a.out"),
+CURRENT_EXECUTABLE_SET)
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", self.line, num_expected_locations=-1, 
loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Test with locally constructed instances.
+self.expect("expr Base().foo()", substrs=["1"])
+self.expect("expr Derived().foo()", substrs=["2"])

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py?rev=375173&r1=375172&r2=375173&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/ignore-artificial-constructors/TestIgnoreArtificialConstructors.py
 Thu Oct 17 14:27:26 2019
@@ -1,4 +1,5 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-lldbinline.MakeInlineTest(__file__, globals(), None)
+lldbinline.MakeInlineTest(__file__, globals(), [lldbinline.expectedFailureAll(
+oslist=["windows"], bugnumber="llvm.org/pr43707")])


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

[Lldb-commits] [PATCH] D69058: [test] Add a .clang-format file for the shell test.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0f6c6434cc4: [test] Add a .clang-format file for the shell 
test. (authored by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69058

Files:
  lldb/test/Shell/.clang-format


Index: lldb/test/Shell/.clang-format
===
--- /dev/null
+++ lldb/test/Shell/.clang-format
@@ -0,0 +1,3 @@
+BasedOnStyle: LLVM
+# We don't want clang-format to introduce line breaks in RUN commands.
+ColumnLimit: 999


Index: lldb/test/Shell/.clang-format
===
--- /dev/null
+++ lldb/test/Shell/.clang-format
@@ -0,0 +1,3 @@
+BasedOnStyle: LLVM
+# We don't want clang-format to introduce line breaks in RUN commands.
+ColumnLimit: 999
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r375174 - (NFC) Delete variable made unused by llvm-svn: 375160

[Sterling Augustine via lldb-commits]

Author: saugustine
Date: Thu Oct 17 14:40:12 2019
New Revision: 375174

URL: http://llvm.org/viewvc/llvm-project?rev=375174&view=rev
Log:
(NFC) Delete variable made unused by llvm-svn: 375160

Reviewers: aprantl

Subscribers: llvm-commits

Tags: #llvm

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp?rev=375174&r1=375173&r2=375174&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp Thu Oct 17 
14:40:12 2019
@@ -1352,7 +1352,6 @@ void SymbolFilePDB::FindFunctions(const
   if (!regex.IsValid())
 return;
 
-  auto old_size = sc_list.GetSize();
   CacheFunctionNames();
 
   std::set resolved_ids;


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

[Lldb-commits] [PATCH] D69143: (NFC) Delete variable made unused by llvm-svn: 375160

[Sterling Augustine 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 rGbbbc873f83e4: (NFC) Delete variable made unused by llvm-svn: 
375160 (authored by saugustine).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69143

Files:
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp


Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1352,7 +1352,6 @@
   if (!regex.IsValid())
 return;
 
-  auto old_size = sc_list.GetSize();
   CacheFunctionNames();
 
   std::set resolved_ids;


Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1352,7 +1352,6 @@
   if (!regex.IsValid())
 return;
 
-  auto old_size = sc_list.GetSize();
   CacheFunctionNames();
 
   std::set resolved_ids;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D69143: (NFC) Delete variable made unused by llvm-svn: 375160

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69143



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

[Lldb-commits] [lldb] r375181 - clean up the implementation of PythonCallable::GetNumArguments

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Thu Oct 17 15:22:06 2019
New Revision: 375181

URL: http://llvm.org/viewvc/llvm-project?rev=375181&view=rev
Log:
clean up the implementation of PythonCallable::GetNumArguments

Summary:
The current implementation of PythonCallable::GetNumArguments
is not exception safe, has weird semantics, and is just plain
incorrect for some kinds of functions.

Python 3.3 introduces inspect.signature, which lets us easily
query for function signatures in a sane and documented way.

This patch leaves the old implementation in place for < 3.3,
but uses inspect.signature for modern pythons.   It also leaves
the old weird semantics in place, but with FIXMEs grousing about
it.   We should update the callers and fix the semantics in a
subsequent patch.It also adds some tests.

Reviewers: JDevlieghere, clayborg, labath, jingham

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.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=375181&r1=375180&r2=375181&view=diff
==
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
(original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
Thu Oct 17 15:22:06 2019
@@ -31,6 +31,7 @@
 using namespace lldb_private;
 using namespace lldb;
 using namespace lldb_private::python;
+using llvm::cantFail;
 using llvm::Error;
 using llvm::Expected;
 
@@ -47,6 +48,20 @@ Expected python::As
+Expected python::As(Expected &&obj) {
+  if (!obj)
+return obj.takeError();
+  PyObject *str_obj = PyObject_Str(obj.get().get());
+  if (!obj)
+return llvm::make_error();
+  auto str = Take(str_obj);
+  auto utf8 = str.AsUTF8();
+  if (!utf8)
+return utf8.takeError();
+  return utf8.get();
+}
+
 void StructuredPythonObject::Serialize(llvm::json::OStream &s) const {
   s.value(llvm::formatv("Python Obj: {0:X}", GetValue()).str());
 }
@@ -657,16 +672,66 @@ PythonList PythonDictionary::GetKeys() c
 }
 
 PythonObject PythonDictionary::GetItemForKey(const PythonObject &key) const {
-  if (IsAllocated() && key.IsValid())
-return PythonObject(PyRefType::Borrowed,
-PyDict_GetItem(m_py_obj, key.get()));
-  return PythonObject();
+  auto item = GetItem(key);
+  if (!item) {
+llvm::consumeError(item.takeError());
+return PythonObject();
+  }
+  return std::move(item.get());
+}
+
+Expected
+PythonDictionary::GetItem(const PythonObject &key) const {
+  if (!IsValid())
+return nullDeref();
+#if PY_MAJOR_VERSION >= 3
+  PyObject *o = PyDict_GetItemWithError(m_py_obj, key.get());
+  if (PyErr_Occurred())
+return exception();
+#else
+  PyObject *o = PyDict_GetItem(m_py_obj, key.get());
+#endif
+  if (!o)
+return keyError();
+  return Retain(o);
+}
+
+Expected PythonDictionary::GetItem(const char *key) const {
+  if (!IsValid())
+return nullDeref();
+  PyObject *o = PyDict_GetItemString(m_py_obj, key);
+  if (PyErr_Occurred())
+return exception();
+  if (!o)
+return keyError();
+  return Retain(o);
+}
+
+Error PythonDictionary::SetItem(const PythonObject &key,
+const PythonObject &value) const {
+  if (!IsValid() || !value.IsValid())
+return nullDeref();
+  int r = PyDict_SetItem(m_py_obj, key.get(), value.get());
+  if (r < 0)
+return exception();
+  return Error::success();
+}
+
+Error PythonDictionary::SetItem(const char *key,
+const PythonObject &value) const {
+  if (!IsValid() || !value.IsValid())
+return nullDeref();
+  int r = PyDict_SetItemString(m_py_obj, key, value.get());
+  if (r < 0)
+return exception();
+  return Error::success();
 }
 
 void PythonDictionary::SetItemForKey(const PythonObject &key,
  const PythonObject &value) {
-  if (IsAllocated() && key.IsValid() && value.IsValid())
-PyDict_SetItem(m_py_obj, key.get(), value.get());
+  Error error = SetItem(key, value);
+  if (error)
+llvm::consumeError(std::move(error));
 }
 
 StructuredData::DictionarySP
@@ -736,23 +801,89 @@ bool PythonCallable::Check(PyObject *py_
 }
 
 PythonCallable::ArgInfo PythonCallable::GetNumInitArguments() const {
-  ArgInfo result = {0, false, false, false};
-  if (!IsValid())
-return result;
-
-  PythonObject __init__ = GetAttributeValue("__init__");
-  if (__init__.IsValid() ) {
-auto __init_callable__ = __init__.AsType();
-if (__init_callable__.IsValid())
-  return __init_callable__.GetNumArguments();
+  auto argin

[Lldb-commits] [lldb] r375182 - eliminate one form of PythonObject::Reset()

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Thu Oct 17 15:22:09 2019
New Revision: 375182

URL: http://llvm.org/viewvc/llvm-project?rev=375182&view=rev
Log:
eliminate one form of PythonObject::Reset()

Summary:
I'd like to eliminate all forms of Reset() and all public constructors
on these objects, so the only way to make them is with Take<> and Retain<>
and the only way to copy or move them is with actual c++ copy, move, or
assignment.

This is a simple place to start.

Reviewers: JDevlieghere, clayborg, labath, jingham

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.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=375182&r1=375181&r2=375182&view=diff
==
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
(original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp 
Thu Oct 17 15:22:09 2019
@@ -410,7 +410,7 @@ void PythonString::SetString(llvm::Strin
 llvm::consumeError(s.takeError());
 Reset();
   } else {
-PythonObject::Reset(std::move(s.get()));
+*this = std::move(s.get());
   }
 }
 

Modified: lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h?rev=375182&r1=375181&r2=375182&view=diff
==
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h 
(original)
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h Thu 
Oct 17 15:22:09 2019
@@ -182,7 +182,8 @@ public:
 Reset(type, py_obj);
   }
 
-  PythonObject(const PythonObject &rhs) : m_py_obj(nullptr) { Reset(rhs); }
+  PythonObject(const PythonObject &rhs)
+  : PythonObject(PyRefType::Borrowed, rhs.m_py_obj) {}
 
   PythonObject(PythonObject &&rhs) {
 m_py_obj = rhs.m_py_obj;
@@ -197,19 +198,6 @@ public:
 m_py_obj = nullptr;
   }
 
-  void Reset(const PythonObject &rhs) {
-if (!rhs.IsValid())
-  Reset();
-else
-  Reset(PyRefType::Borrowed, rhs.m_py_obj);
-  }
-
-  // PythonObject is implicitly convertible to PyObject *, which will call the
-  // wrong overload.  We want to explicitly disallow this, since a PyObject
-  // *always* owns its reference.  Therefore the overload which takes a
-  // PyRefType doesn't make sense, and the copy constructor should be used.
-  void Reset(PyRefType type, const PythonObject &ref) = delete;
-
   void Reset(PyRefType type, PyObject *py_obj) {
 if (py_obj == m_py_obj)
   return;
@@ -244,19 +232,9 @@ public:
 return result;
   }
 
-  PythonObject &operator=(const PythonObject &other) {
-Reset(PyRefType::Borrowed, other.get());
-return *this;
-  }
-
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject other) {
 Reset();
-m_py_obj = other.m_py_obj;
-other.m_py_obj = nullptr;
-  }
-
-  PythonObject &operator=(PythonObject &&other) {
-Reset(std::move(other));
+m_py_obj = std::exchange(other.m_py_obj, nullptr);
 return *this;
   }
 

Modified: 
lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp?rev=375182&r1=375181&r2=375182&view=diff
==
--- lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp 
(original)
+++ lldb/trunk/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp 
Thu Oct 17 15:22:09 2019
@@ -323,8 +323,8 @@ TEST_F(PythonDataObjectsTest, TestPython
   PythonList list(PyRefType::Owned, py_list);
 
   PythonObject list_items[list_size];
-  list_items[0].Reset(PythonInteger(long_value0));
-  list_items[1].Reset(PythonString(string_value1));
+  list_items[0] = PythonInteger(long_value0);
+  list_items[1] = PythonString(string_value1);
 
   for (unsigned i = 0; i < list_size; ++i)
 list.SetItemAtIndex(i, list_items[i]);
@@ -469,10 +469,10 @@ TEST_F(PythonDataObjectsTest, TestPython
   PythonObject py_keys[dict_entries];
   PythonObject py_values[dict_entries];
 
-  py_keys[0].Reset(PythonString(key_0));
-  py_keys[1].Reset(PythonInteger(key_1));
-  py_values[0].Reset(PythonInteger(value_0));
-  py_values[1].Reset(PythonString(value_1));
+  py_keys[0] = PythonString(key_0);
+  py_keys[1] = PythonInteger(key_1);
+  py_values[0] = PythonInteger(value_0);
+  p

[Lldb-commits] [PATCH] D68995: clean up the implementation of PythonCallable::GetNumArguments

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc86a6acaee55: clean up the implementation of 
PythonCallable::GetNumArguments (authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68995

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -20,6 +20,7 @@
 #include "PythonTestSuite.h"
 
 using namespace lldb_private;
+using namespace lldb_private::python;
 
 class PythonDataObjectsTest : public PythonTestSuite {
 public:
@@ -626,3 +627,116 @@
 }
   }
 }
+
+TEST_F(PythonDataObjectsTest, TestCallable) {
+
+  PythonDictionary globals(PyInitialValue::Empty);
+  auto builtins = PythonModule::BuiltinsModule();
+  llvm::Error error = globals.SetItem("__builtins__", builtins);
+  ASSERT_FALSE(error);
+
+  {
+PyObject *o = PyRun_String("lambda x : x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y=0: x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y=0, **kw: x", Py_eval_input,
+   globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y,*a: x", Py_eval_input, globals.get(),
+   globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, true);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+  {
+PyObject *o = PyRun_String("lambda x,y,*a,**kw: x", Py_eval_input,
+   globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+auto lambda = Take(o);
+auto arginfo = lambda.GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, true);
+  }
+
+  {
+const char *script = R"(
+class Foo:
+  def bar(self, x):
+ return x
+bar_bound   = Foo().bar
+bar_unbound = Foo.bar
+)";
+PyObject *o =
+PyRun_String(script, Py_file_input, globals.get(), globals.get());
+ASSERT_FALSE(o == NULL);
+Take(o);
+
+auto bar_bound = As(globals.GetItem("bar_bound"));
+ASSERT_THAT_EXPECTED(bar_bound, llvm::Succeeded());
+auto arginfo = bar_bound.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2); // FIXME, wrong
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, true);
+
+auto bar_unbound = As(globals.GetItem("bar_unbound"));
+ASSERT_THAT_EXPECTED(bar_unbound, llvm::Succeeded());
+arginfo = bar_unbound.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+
+  // the old implementation of GetArgInfo just doesn't work on builtins.
+
+  {
+auto builtins = PythonModule::BuiltinsModule();
+auto hex = As(builtins.GetAttribute("hex"));
+ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());
+auto arginfo = hex.get().GetArgInfo();
+ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+EXPECT_EQ(arginfo.get().count, 1);
+EXPECT_EQ(arginfo.get().has_varargs, false);
+EXPECT_EQ(arginfo.get().is_bound_method, false);
+  }
+
+#endif
+}
\ No newline at end

[Lldb-commits] [PATCH] D69080: eliminate one form of PythonObject::Reset()

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG03819d1c80ad: eliminate one form of PythonObject::Reset() 
(authored by lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69080

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -323,8 +323,8 @@
   PythonList list(PyRefType::Owned, py_list);
 
   PythonObject list_items[list_size];
-  list_items[0].Reset(PythonInteger(long_value0));
-  list_items[1].Reset(PythonString(string_value1));
+  list_items[0] = PythonInteger(long_value0);
+  list_items[1] = PythonString(string_value1);
 
   for (unsigned i = 0; i < list_size; ++i)
 list.SetItemAtIndex(i, list_items[i]);
@@ -469,10 +469,10 @@
   PythonObject py_keys[dict_entries];
   PythonObject py_values[dict_entries];
 
-  py_keys[0].Reset(PythonString(key_0));
-  py_keys[1].Reset(PythonInteger(key_1));
-  py_values[0].Reset(PythonInteger(value_0));
-  py_values[1].Reset(PythonString(value_1));
+  py_keys[0] = PythonString(key_0);
+  py_keys[1] = PythonInteger(key_1);
+  py_values[0] = PythonInteger(value_0);
+  py_values[1] = PythonString(value_1);
 
   PyObject *py_dict = PyDict_New();
   EXPECT_TRUE(PythonDictionary::Check(py_dict));
@@ -509,10 +509,10 @@
   PythonString keys[dict_entries];
   PythonObject values[dict_entries];
 
-  keys[0].Reset(PythonString(key_0));
-  keys[1].Reset(PythonString(key_1));
-  values[0].Reset(PythonInteger(value_0));
-  values[1].Reset(PythonString(value_1));
+  keys[0] = PythonString(key_0);
+  keys[1] = PythonString(key_1);
+  values[0] = PythonInteger(value_0);
+  values[1] = PythonString(value_1);
 
   PythonDictionary dict(PyInitialValue::Empty);
   for (int i = 0; i < 2; ++i)
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -182,7 +182,8 @@
 Reset(type, py_obj);
   }
 
-  PythonObject(const PythonObject &rhs) : m_py_obj(nullptr) { Reset(rhs); }
+  PythonObject(const PythonObject &rhs)
+  : PythonObject(PyRefType::Borrowed, rhs.m_py_obj) {}
 
   PythonObject(PythonObject &&rhs) {
 m_py_obj = rhs.m_py_obj;
@@ -197,19 +198,6 @@
 m_py_obj = nullptr;
   }
 
-  void Reset(const PythonObject &rhs) {
-if (!rhs.IsValid())
-  Reset();
-else
-  Reset(PyRefType::Borrowed, rhs.m_py_obj);
-  }
-
-  // PythonObject is implicitly convertible to PyObject *, which will call the
-  // wrong overload.  We want to explicitly disallow this, since a PyObject
-  // *always* owns its reference.  Therefore the overload which takes a
-  // PyRefType doesn't make sense, and the copy constructor should be used.
-  void Reset(PyRefType type, const PythonObject &ref) = delete;
-
   void Reset(PyRefType type, PyObject *py_obj) {
 if (py_obj == m_py_obj)
   return;
@@ -244,19 +232,9 @@
 return result;
   }
 
-  PythonObject &operator=(const PythonObject &other) {
-Reset(PyRefType::Borrowed, other.get());
-return *this;
-  }
-
-  void Reset(PythonObject &&other) {
+  PythonObject &operator=(PythonObject other) {
 Reset();
-m_py_obj = other.m_py_obj;
-other.m_py_obj = nullptr;
-  }
-
-  PythonObject &operator=(PythonObject &&other) {
-Reset(std::move(other));
+m_py_obj = std::exchange(other.m_py_obj, nullptr);
 return *this;
   }
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -410,7 +410,7 @@
 llvm::consumeError(s.takeError());
 Reset();
   } else {
-PythonObject::Reset(std::move(s.get()));
+*this = std::move(s.get());
   }
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68968: [android/process info] Introduce bundle id

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Thanks for your feedback and to @clayborg  for an offline discussion we had.

I'm going to add three new attributes to the ProcessInfo class, with 
corresponding parameters in the gdb-remote packet

- display_name: This is a custom display name that the server can set for a 
given process. This will supersede any other other attribute in the 
GetProcessName call. There are interesting cases that are worth mentioning for 
Android:
  - Kernel threads and system processes are displayed like [kworker/u:0] 
[khubd] by ps on Android. At least on modern devices ps.c identify those 
processes as the ones with unreadable /proc//cmdline and unreadable 
/proc//exe. The name is gotten from /proc//stat. There's some generic 
information about this here 
https://unix.stackexchange.com/questions/22121/what-do-the-brackets-around-processes-mean.
 In this case both the actual executable and args are not known, so it's better 
to leave those fields empty and store the display name in this new field.
  - Custom apk process names: apks can launch processes and subprocesses with 
custom names. The custom name has the form com.example.app:custom_name. It's 
always that way and it's documented here 
https://developer.android.com/guide/topics/manifest/application-element#proc 
(go to the android:process section). A given apk can have several subprocesses, 
each one with a different custom name, e.g. com.samsung.services:Core, 
com.samsung.services:Monitor, etc. In this case, the package name is known and 
a display_name field would be useful to store the entire custom name.

- bundle_id: This will be exactly the apk package name or the iOS bundle name 
inferred by the server. I plan to make a best effort guess for modern android 
devices, leaving old devices as cases to implement as needed. I plan to use `pm 
list packages` for this, given that this is the only reliable way to get all 
the apk names. Besides, I'll check /proc/stat for the process name and not 
arg0. It seems that /stat is not modifiable the same way arg0 is.

- bundle_path: On android this is the path the apk path. This will also be 
useful for iOS, because app bundles are also stored somewhere. Fortunately, the 
command `pm list packages -f`, shows the list of apks along with their 
corresponding paths on disk. This command doesn't work on old devices, but 
still, I wouldn't prioritize them now.

For the `platform process list` command, I'll make the GetProcessName function 
look for the actual name from these sources in the given order: display_name -> 
bundle_id -> exe path
In the case of the --verbose mode, I'll show all bundle information and display 
name fields


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68968



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

[Lldb-commits] [lldb] r375187 - [lldb][NFC] Fix typo in DWARFASTParserClang.cpp

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Thu Oct 17 16:11:32 2019
New Revision: 375187

URL: http://llvm.org/viewvc/llvm-project?rev=375187&view=rev
Log:
[lldb][NFC] Fix typo in DWARFASTParserClang.cpp

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=375187&r1=375186&r2=375187&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Thu Oct 
17 16:11:32 2019
@@ -1007,8 +1007,8 @@ TypeSP DWARFASTParserClang::ParseTypeFro
   is_attr_used, attrs.is_artificial);
 
   type_handled = cxx_method_decl != NULL;
-  // Artificial methods are always handled even when don't
-  // create a new declaration for them.
+  // Artificial methods are always handled even when we
+  // don't create a new declaration for them.
   type_handled |= attrs.is_artificial;
 
   if (cxx_method_decl) {


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

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: jfb, nickdesaulniers, friss, JDevlieghere.
Herald added subscribers: dexonsmith, hiraditya, mgorny.
Herald added a project: LLVM.

Occasionally, during test teardown, there is a write to a closed pipe in of
LLDB's handful of IPC channels. Sometimes the communication is inherently
unreliable, so LLDB tries to avoid being killed due to SIGPIPE. Actually, it
explicitly calls `signal(SIGPIPE, SIG_IGN)`.  However, LLVM's default SIGPIPE
behavior is to exit with IO_ERR. Opt LLDB out of that.

I expect that this will resolve some LLDB test suite flakiness (tests
randomly failing with IO_ERR) that we've seen since r344372.

rdar://55750240


https://reviews.llvm.org/D69148

Files:
  lldb/tools/driver/Driver.cpp
  llvm/include/llvm/Support/Signals.h
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/Signals.inc
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/SignalsTest.cpp

Index: llvm/unittests/Support/SignalsTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/SignalsTest.cpp
@@ -0,0 +1,65 @@
+//- unittests/Support/SignalsTest.cpp - Signal handling test =//
+//
+// 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
+//
+//===--===//
+
+#if !defined(_WIN32)
+#include 
+#include 
+#endif // !defined(_WIN32)
+
+#include "llvm/Support/Signals.h"
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+#if !defined(_WIN32)
+void NoOpSigHandler(void *) {}
+
+TEST(SignalTest, EnableExitOnSIGPIPE) {
+  // Register default signal handlers.
+  sys::AddSignalHandler(NoOpSigHandler, nullptr);
+
+  // Create unidirectional read/write pipes.
+  int fds[2];
+  int err = pipe(fds);
+  if (err != 0)
+return; // If we can't make pipes, this isn't testing anything.
+
+  // Close the read pipe.
+  close(fds[0]);
+
+  // Attempt to write to the write pipe. Expect exit(IOERR).
+  const void *buf = (const void *)&fds;
+
+  // LLVM's default handler should catch SIGPIPE and exit with IOERR.
+  EXPECT_EXIT(write(fds[1], buf, 1), ::testing::ExitedWithCode(EX_IOERR), "");
+}
+
+TEST(SignalTest, DisableExitOnSIGPIPE) {
+  // Register default signal handlers.
+  sys::AddSignalHandler(NoOpSigHandler, nullptr);
+
+  // Disable exit-on-SIGPIPE.
+  sys::SetExitOnFailedPipeWrite(false);
+
+  // Create unidirectional read/write pipes.
+  int fds[2];
+  int err = pipe(fds);
+  if (err != 0)
+return; // If we can't make pipes, this isn't testing anything.
+
+  // Close the read pipe.
+  close(fds[0]);
+
+  // Attempt to write to the write pipe.
+  const void *buf = (const void *)&fds;
+
+  // No handler should be installed: we expect kill-by-SIGPIPE.
+  EXPECT_EXIT(write(fds[1], buf, 1), ::testing::KilledBySignal(SIGPIPE), "");
+}
+#endif // !defined(_WIN32)
Index: llvm/unittests/Support/CMakeLists.txt
===
--- llvm/unittests/Support/CMakeLists.txt
+++ llvm/unittests/Support/CMakeLists.txt
@@ -60,6 +60,7 @@
   ScaledNumberTest.cpp
   SourceMgrTest.cpp
   SpecialCaseListTest.cpp
+  SignalsTest.cpp
   StringPool.cpp
   SwapByteOrderTest.cpp
   SymbolRemappingReaderTest.cpp
Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -560,6 +560,9 @@
   // Unimplemented.
 }
 
+void llvm::sys::SetExitOnFailedPipeWrite(bool) {
+  // Unimplemented.
+}
 
 /// Add a function to be called when a signal is delivered to the process. The
 /// handler can have a cookie passed to it to identify what instance of the
Index: llvm/lib/Support/Unix/Signals.inc
===
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -88,6 +88,7 @@
 ATOMIC_VAR_INIT(nullptr);
 static std::atomic InfoSignalFunction =
 ATOMIC_VAR_INIT(nullptr);
+static std::atomic ExitOnSIGPIPE = ATOMIC_VAR_INIT(true);
 
 namespace {
 /// Signal-safe removal of files.
@@ -362,7 +363,7 @@
 return OldInterruptFunction();
 
   // Send a special return code that drivers can check for, from sysexits.h.
-  if (Sig == SIGPIPE)
+  if (Sig == SIGPIPE && ExitOnSIGPIPE)
 exit(EX_IOERR);
 
   raise(Sig);   // Execute the default handler.
@@ -403,6 +404,10 @@
   RegisterHandlers();
 }
 
+void llvm::sys::SetExitOnFailedPipeWrite(bool ExitOnFailedWrite) {
+  ExitOnSIGPIPE.store(ExitOnFailedWrite);
+}
+
 // The public API
 bool llvm::sys::RemoveFileOnSignal(StringRef Filename,
std::string* ErrMsg) {
Index: llvm/include/llvm/Support/Signals.h
==

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[JF Bastien via Phabricator via lldb-commits]

jfb added inline comments.



Comment at: lldb/tools/driver/Driver.cpp:864
+  // in LLDB leaves behind temporary objects).
+  llvm::sys::SetExitOnFailedPipeWrite(/*ExitOnFailedWrite=*/false);
+

Could this instead pass a lambda, like other signal handlers? So sigpipe would 
call any user-specified lambda, and by default would instead call exit.


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

https://reviews.llvm.org/D69148



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

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Vedant Kumar via Phabricator via lldb-commits]

vsk planned changes to this revision.
vsk marked an inline comment as done.
vsk added inline comments.



Comment at: lldb/tools/driver/Driver.cpp:864
+  // in LLDB leaves behind temporary objects).
+  llvm::sys::SetExitOnFailedPipeWrite(/*ExitOnFailedWrite=*/false);
+

jfb wrote:
> Could this instead pass a lambda, like other signal handlers? So sigpipe 
> would call any user-specified lambda, and by default would instead call exit.
Ah, sure. That would make things a bit more customizable.


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

https://reviews.llvm.org/D69148



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

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 225546.
vsk edited the summary of this revision.
vsk added a comment.

- Allow setting a SIGPIPE handler.


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

https://reviews.llvm.org/D69148

Files:
  lldb/tools/driver/Driver.cpp
  llvm/include/llvm/Support/Signals.h
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/Signals.inc
  llvm/unittests/Support/CMakeLists.txt
  llvm/unittests/Support/SignalsTest.cpp

Index: llvm/unittests/Support/SignalsTest.cpp
===
--- /dev/null
+++ llvm/unittests/Support/SignalsTest.cpp
@@ -0,0 +1,65 @@
+//- unittests/Support/SignalsTest.cpp - Signal handling test =//
+//
+// 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
+//
+//===--===//
+
+#if !defined(_WIN32)
+#include 
+#include 
+#endif // !defined(_WIN32)
+
+#include "llvm/Support/Signals.h"
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+#if !defined(_WIN32)
+void NoOpSigHandler(void *) {}
+
+TEST(SignalTest, EnableExitOnSIGPIPE) {
+  // Register default signal handlers.
+  sys::AddSignalHandler(NoOpSigHandler, nullptr);
+
+  // Create unidirectional read/write pipes.
+  int fds[2];
+  int err = pipe(fds);
+  if (err != 0)
+return; // If we can't make pipes, this isn't testing anything.
+
+  // Close the read pipe.
+  close(fds[0]);
+
+  // Attempt to write to the write pipe. Expect exit(IOERR).
+  const void *buf = (const void *)&fds;
+
+  // LLVM's default handler should catch SIGPIPE and exit with IOERR.
+  EXPECT_EXIT(write(fds[1], buf, 1), ::testing::ExitedWithCode(EX_IOERR), "");
+}
+
+TEST(SignalTest, DisableExitOnSIGPIPE) {
+  // Register default signal handlers.
+  sys::AddSignalHandler(NoOpSigHandler, nullptr);
+
+  // Disable exit-on-SIGPIPE.
+  sys::SetPipeSignalFunction(nullptr);
+
+  // Create unidirectional read/write pipes.
+  int fds[2];
+  int err = pipe(fds);
+  if (err != 0)
+return; // If we can't make pipes, this isn't testing anything.
+
+  // Close the read pipe.
+  close(fds[0]);
+
+  // Attempt to write to the write pipe.
+  const void *buf = (const void *)&fds;
+
+  // No handler should be installed: we expect kill-by-SIGPIPE.
+  EXPECT_EXIT(write(fds[1], buf, 1), ::testing::KilledBySignal(SIGPIPE), "");
+}
+#endif // !defined(_WIN32)
Index: llvm/unittests/Support/CMakeLists.txt
===
--- llvm/unittests/Support/CMakeLists.txt
+++ llvm/unittests/Support/CMakeLists.txt
@@ -58,6 +58,7 @@
   ReverseIterationTest.cpp
   ReplaceFileTest.cpp
   ScaledNumberTest.cpp
+  SignalsTest.cpp
   SourceMgrTest.cpp
   SpecialCaseListTest.cpp
   StringPool.cpp
Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -560,6 +560,9 @@
   // Unimplemented.
 }
 
+void llvm::sys::SetPipeSignalFunction(void (*Handler)()) {
+  // Unimplemented.
+}
 
 /// Add a function to be called when a signal is delivered to the process. The
 /// handler can have a cookie passed to it to identify what instance of the
Index: llvm/lib/Support/Unix/Signals.inc
===
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -82,12 +82,18 @@
 static RETSIGTYPE SignalHandler(int Sig);  // defined below.
 static RETSIGTYPE InfoSignalHandler(int Sig);  // defined below.
 
+static void DefaultPipeSignalFunction() {
+  exit(EX_IOERR);
+}
+
 using SignalHandlerFunctionType = void (*)();
 /// The function to call if ctrl-c is pressed.
 static std::atomic InterruptFunction =
 ATOMIC_VAR_INIT(nullptr);
 static std::atomic InfoSignalFunction =
 ATOMIC_VAR_INIT(nullptr);
+static std::atomic PipeSignalFunction =
+ATOMIC_VAR_INIT(DefaultPipeSignalFunction);
 
 namespace {
 /// Signal-safe removal of files.
@@ -363,7 +369,8 @@
 
   // Send a special return code that drivers can check for, from sysexits.h.
   if (Sig == SIGPIPE)
-exit(EX_IOERR);
+if (SignalHandlerFunctionType CurrentPipeFunction = PipeSignalFunction)
+  CurrentPipeFunction();
 
   raise(Sig);   // Execute the default handler.
   return;
@@ -403,6 +410,11 @@
   RegisterHandlers();
 }
 
+void llvm::sys::SetPipeSignalFunction(void (*Handler)()) {
+  PipeSignalFunction.exchange(Handler);
+  RegisterHandlers();
+}
+
 // The public API
 bool llvm::sys::RemoveFileOnSignal(StringRef Filename,
std::string* ErrMsg) {
Index: llvm/include/llvm/Support/Signals.h
===
--- llvm/include/llvm/Support/Sign

[Lldb-commits] [PATCH] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Check out  `lldb/test/Shell/helper/toolchain.py`, you probably need to filter 
out some options for clang_cl specifically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69031



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

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[JF Bastien via Phabricator via lldb-commits]

jfb accepted this revision.
jfb added a subscriber: jordan_rose.
jfb added a comment.
This revision is now accepted and ready to land.

More of an FYI, @jordan_rose might be interested in this.


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

https://reviews.llvm.org/D69148



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

[Lldb-commits] [PATCH] D69148: Disable exit-on-SIGPIPE in lldb

[Jordan Rose via Phabricator via lldb-commits]

jordan_rose added inline comments.



Comment at: llvm/lib/Support/Unix/Signals.inc:372
   if (Sig == SIGPIPE)
-exit(EX_IOERR);
+if (SignalHandlerFunctionType CurrentPipeFunction = PipeSignalFunction)
+  CurrentPipeFunction();

Should it be doing the same sort of `exchange` as the interrupt function?


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

https://reviews.llvm.org/D69148



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

[Lldb-commits] [PATCH] D69153: convert LLDBSwigPythonCallTypeScript to ArgInfo::max_positional_args

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

lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, clayborg, labath, jingham.
Herald added a project: LLDB.
lawrence_danna added a parent revision: D69014: [LLDB] bugfix: command script 
add -f doesn't work for some callables.

This patch converts another user of ArgInfo::count over to 
use ArgInfo::max_positional_args instead.   I also add a test
to make sure both documented signatures for python type formatters 
work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69153

Files:
  
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
  
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/formatters.py
  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
@@ -163,14 +163,19 @@
 }
 
 PythonObject result;
-auto argc = pfunc.GetNumArguments();
-// if the third argument is supported, or varargs are allowed
+auto argc = pfunc.GetArgInfo();
+if (!argc) {
+llvm::consumeError(argc.takeError());
+return false;
+}
+
 PythonObject value_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_value));
 PythonObject options_arg(PyRefType::Owned, 
SBTypeToSWIGWrapper(sb_options));
-if (argc.count == 3 || argc.has_varargs)
-result = pfunc(value_arg,dict,options_arg);
-else
+
+if (argc.get().max_positional_args < 3)
 result = pfunc(value_arg,dict);
+else
+result = pfunc(value_arg,dict,options_arg);
 
 retval = result.Str().GetString().str();
 
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/formatters.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/formatters.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/formatters.py
@@ -1,3 +1,5 @@
+import lldb
+
 def foo_SummaryProvider(valobj, dict):
 a = valobj.GetChildMemberWithName('a')
 a_ptr = valobj.GetChildMemberWithName('a_ptr')
@@ -15,3 +17,8 @@
 ', i_ptr = ' + str(i_ptr.GetValueAsUnsigned(0)) + ' -> ' + 
str(i_ptr.Dereference().GetValueAsUnsigned(0)) + \
 ', b_ref = ' + str(b_ref.GetValueAsUnsigned(0)) + \
 ', h = ' + str(h.GetValueAsUnsigned(0)) + ' , k = ' + 
str(k.GetValueAsUnsigned(0))
+
+def foo_SummaryProvider3(valobj, dict, options):
+if not isinstance(options, lldb.SBTypeSummaryOptions):
+raise Exception()
+return foo_SummaryProvider(valobj, dict) + ", WITH_OPTS"
\ No newline at end of file
Index: 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
===
--- 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
+++ 
lldb/packages/Python/lldbsuite/test/commands/expression/formatters/TestFormatters.py
@@ -74,6 +74,21 @@
 # EXPR-TYPES-NEW-FOO-NEXT:   }
 # EXPR-TYPES-NEW-FOO-NEXT: }
 
+
+self.runCmd("type summary add -F formatters.foo_SummaryProvider3 foo")
+self.filecheck("expression foo1", __file__, 
'-check-prefix=EXPR-FOO1opts')
+# EXPR-FOO1opts: (foo) $
+# EXPR-FOO1opts-SAME: a = 12
+# EXPR-FOO1opts-SAME: a_ptr = {{[0-9]+}} -> 13
+# EXPR-FOO1opts-SAME: i = 24
+# EXPR-FOO1opts-SAME: i_ptr = {{[0-9]+}} -> 25
+# EXPR-FOO1opts-SAME: b_ref = {{[0-9]+}}
+# EXPR-FOO1opts-SAME: h = 27
+# EXPR-FOO1opts-SAME: k = 29
+# EXPR-FOO1opts-SAME: WITH_OPTS
+
+self.runCmd("type summary delete foo")
+
 self.runCmd("type summary add -F formatters.foo_SummaryProvider foo")
 
 self.expect("expression new int(12)",


Index: lldb/scripts/Python/python-wrapper.swig
===
--- lldb/scripts/Python/python-wrapper.swig
+++ lldb/scripts/Python/python-wrapper.swig
@@ -163,14 +163,19 @@
 }
 
 PythonObject result;
-auto argc = pfunc.GetNumArguments();
-// if the third argument is supported, or varargs are allowed
+auto argc = pfunc.GetArgInfo();
+if (!argc) {
+llvm::consumeError(argc.takeError());
+return false;
+}
+
 PythonObject value_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_value));
 PythonObject options_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_options));
-if (argc.count == 3 || argc.has_varargs)
-result = pfunc(value_arg,dict,options_arg);
-else
+
+if (argc.get().max_positional_args < 3)
 result = pfunc(value_arg,dict);
+else
+result = pfunc(value_arg,dict,options_arg);
 
 retval = result.Str().GetString().str();
 
Index: lldb/packages/Python/lldbsuite/test/commands/expression/formatters/formatters.py
===

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

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

lawrence_danna added a comment.

In D67793#1692544 , @labath wrote:

> 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?


oops, sorry I missed this.   Are you happy with how it all turned out?


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