[Lldb-commits] [PATCH] D69016: [lldb] move more things from python to cmake

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

mgorny added a comment.

I'm not sure if I like the usage of `POST_BUILD` stuff (it provides less 
control than separate targets) but overall this seems a good change. Replacing 
~250 lines of reinventing the wheel with ~20 lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69016



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

[Lldb-commits] [PATCH] D68910: python path should be platform-dependent

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D68910#1710147 , @hhb wrote:

> This is fine for me. Actually it doesn't make any difference for all 
> platforms I tried... Out of curiosity, is it possible to share the 
> implementation of get_python_lib() in openSUSE? Thanks.


https://build.opensuse.org/package/show/devel:languages:python:Factory/python3-base
 is the sources of the package. F00102-lib64.patch is probably what you're 
interested in.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68910



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

[Lldb-commits] [PATCH] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D68541#1710031 , @clayborg wrote:

> Looks good!


Can you please ACK also the other patches linked above that this one depends 
on? Or can I take the discussion here as being enough?

> With a little work, "gui" mode can really become great. I hope to see more 
> patches? Happy to discuss next stops on the mailing list if you are 
> interested!

That's kind of the plan. I rather miss a variables view in gdb tui and I've 
decided that it should be simpler to implement a command prompt in lldb gui 
than a variables view in gdb tui. But I'm still evaluating, I've got other 
things on my plate, only so much time and it's a question if it'd be really 
just a little work to make lldb meet my needs (and I've already run into few 
lame problems such as one of those harmless signals like SIGTSTP simply killing 
the debugging without any useful feedback). We'll see how this goes.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68541



___
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

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D68943#1709998 , @grimar wrote:

> Ok, I was able to debug it finally.
>
> I think we should add a .symtab in a few more cases implicitly.
>  For example, when we have a SHT_RELA/SHT_REL section that has an empty Link,
>  i.e. it refers to .symtab by default and we should provide it.
>  There are other cases, like when Link just explicitly refers to .symtab.
>
> Here is a patch based on this one.
>  It is unpolished, but shows the idea. With it all yaml2obj tests pass.
>
> F10276544: patch.diff 


Nice! Can you post a differential for the yaml2obj change? I believe the 
differential can focus on lldb and remove yaml2obj changes (kwk will still get 
credited for the test cleanups).


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] D68968: [android/process info] Introduce bundle id

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: jingham.
labath added subscribers: danalbert, jingham.
labath added a comment.

Introducing a "bundle" identifier as a first class concept sounds reasonable to 
me, particularly if that concept can be applied to more than one platform. But 
since we're talking about iOS, it would be good to have the apple folks sign 
off on this idea too (+@jingham).

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.

In D68968#1709706 , @clayborg wrote:

> I would like to see the m_executable always contain the path to "app_process" 
> binary. Arg0 is currently used when spawning via posix_spawn or fork/exec to 
> control the name of the binary that is actually passed on the command line. 
> For example you might specify "clang++" as your binary. If we resolved this 
> to a binary we would find it resolves to "clang++@ -> clang", but we want to 
> pass in clang++ as the arg0 so the compiler knows to  run in C++ mode. So I 
> would like Arg0 to be the actual argument that the client would like to be 
> passed to the executable. Then m_bundle_id can be used by the platform 
> launching code to launch the app. Also, by having m_executable set to 
> "app_process", we can create a valid target in LLDB using the ProcessInfo and 
> then launch/attach.


FWIW, even in the previous implementation the idea was the "executable" field 
would point to the real exe (or be empty, if we can't locate it). And the argv 
array would still contain the best approximation of the arguments process was 
invoked with -- only an approximation, because on linux it's possible (and 
relatively common) for a process to overwrite it's argv[0] to alter the "ps" 
output. This is what we were trying to emulate by having `GetName` return 
argv[0] if the executable path was unknown.




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());

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?



Comment at: lldb/source/Utility/ProcessInfo.cpp:45-46
 const char *ProcessInfo::GetName() const {
+  if (m_executable.GetFilename().IsEmpty() && !m_bundle_id.empty())
+return m_bundle_id.c_str();
   return m_executable.GetFilename().GetCString();

What should be the order of preference here? It seems like it would be more 
natural to use "bundle id" as the "name" even if the executable path happens to 
be available/readable.


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] D69008: Add arm64_32 support

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

(looks good to me)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69008



___
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

[Fangrui Song via Phabricator via lldb-commits]

MaskRay 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];

You may use `("Parameter " + Twine(Index)).str()` to get rid of the 
"llvm/Support/FormatVariadic.h" dependency.


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] D68549: make ConstString allocate memory in non-tiny chunks

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yeah, allocating 256MB sounds a bit too much, particularly for lldb-server 
(ideally, I'd remove ConstString from lldb-server completely, but that's a 
different story). The reason for 256 string pools was to avoid/reduce locking 
contention when accessing the pool, but that does make the power-of-2 scaling 
in BumpPtrAllocator kick in too slowly.

Maybe we could reduce the initial size by an order of magnitude here? 
Allocating 25MB (or whatever is the closest power of 2) does not sound like too 
much, and it should still be much better than 4kb. As an alternative, we could 
make the scaling in BumpPtrAllocator faster/configurable. I also doubt that the 
number of string pools was arrived at very scientifically, so it may be 
possible to tune that number too (though, without any data back it up, it seems 
to me that reducing the slab size should have less impact than reducing the 
pool count).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68549



___
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

[George Rimar via Phabricator via lldb-commits]

grimar added a comment.

In D68943#1710525 , @MaskRay wrote:

> In D68943#1709998 , @grimar wrote:
>
> > Ok, I was able to debug it finally.
> >
> > I think we should add a .symtab in a few more cases implicitly.
> >  For example, when we have a SHT_RELA/SHT_REL section that has an empty 
> > Link,
> >  i.e. it refers to .symtab by default and we should provide it.
> >  There are other cases, like when Link just explicitly refers to .symtab.
> >
> > Here is a patch based on this one.
> >  It is unpolished, but shows the idea. With it all yaml2obj tests pass.
> >
> > F10276544: patch.diff 
>
>
> Nice! Can you post a differential for the yaml2obj change? I believe the 
> differential can focus on lldb and remove yaml2obj changes (kwk will still 
> get credited for the test cleanups).


Yes. It needs polishing + fixes for obj2yaml (I do not think it compiled for me 
at all). Also probably makes sence to add tests for obj2yaml too, I need to 
take a look what it does
when the object doesn't have symbol table with such change. I'll try to suggest 
a patch today.


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] D68980: [LLDB] [test] Allow specifying the full arch for msvc/clang-cl in build.py

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo updated this revision to Diff 225171.
mstorsjo retitled this revision from "[LLDB] [test] Pass 
--target=-windows-msvc to clang-cl" to "[LLDB] [test] Allow specifying 
the full arch for msvc/clang-cl in build.py".
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Extended the `--arch` option to build.py, to allow it to take a proper 
architecture name instead of just "32" and "64", when used with msvc or 
clang-cl.


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

https://reviews.llvm.org/D68980

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


Index: lldb/test/Shell/helper/build.py
===
--- lldb/test/Shell/helper/build.py
+++ lldb/test/Shell/helper/build.py
@@ -4,6 +4,7 @@
 
 import argparse
 import os
+import re
 import signal
 import subprocess
 import sys
@@ -26,7 +27,6 @@
 dest='arch',
 required=True,
 default='host',
-choices=['32', '64', 'host'],
 help='Specify the architecture to target.')
 
 parser.add_argument('--compiler',
@@ -277,7 +277,14 @@
 def __init__(self, toolchain_type, args):
 Builder.__init__(self, toolchain_type, args, '.obj')
 
-self.msvc_arch_str = 'x86' if self.arch == '32' else 'x64'
+if self.arch == '32' or re.match(r'i\d86', self.arch):
+self.msvc_arch_str = 'x86'
+elif self.arch == '64' or self.arch == 'host' or self.arch == 'x86_64':
+self.msvc_arch_str = 'x64'
+elif self.arch == 'aarch64' or self.arch == 'arm64':
+self.msvc_arch_str = 'arm64'
+elif re.match(r'^arm', self.arch):
+self.msvc_arch_str = 'arm'
 
 if toolchain_type == 'msvc':
 # Make sure we're using the appropriate toolchain for the desired
@@ -553,7 +560,10 @@
 
 args.append(self.compiler)
 if self.toolchain_type == 'clang-cl':
-args.append('-m' + self.arch)
+if self.arch == '32' or self.arch == '64':
+args.append('-m' + self.arch)
+elif self.arch != 'host':
+args.append('--target=%s-windows-msvc' % (self.arch))
 
 if self.opt == 'none':
 args.append('/Od')
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,7 @@
 // clang-format off
 // REQUIRES: lld
 
-// RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib -o %t.exe -- %s 
+// RUN: %build --compiler=clang-cl --arch=i386 --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
 
Index: lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/disassembly.cpp
@@ -2,7 +2,7 @@
 // REQUIRES: lld
 
 // Test that we can show disassembly and source.
-// RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s 
+// RUN: %build --compiler=clang-cl --arch=x86_64 --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
 


Index: lldb/test/Shell/helper/build.py
===
--- lldb/test/Shell/helper/build.py
+++ lldb/test/Shell/helper/build.py
@@ -4,6 +4,7 @@
 
 import argparse
 import os
+import re
 import signal
 import subprocess
 import sys
@@ -26,7 +27,6 @@
 dest='arch',
 required=True,
 default='host',
-choices=['32', '64', 'host'],
 help='Specify the architecture to target.')
 
 parser.add_argument('--compiler',
@@ -277,7 +277,14 @@
 def __init__(self, toolchain_type, args):
 Builder.__init__(self, toolchain_type, args, '.obj')
 
-self.msvc_arch_str = 'x86' if self.arch == '32' else 'x64'
+if self.arch == '32' or re.match(r'i\d86', self.arch):
+self.msvc_arch_str = 'x86'
+elif self.arch == '64' or self.arch == 'host' or self.arch == 'x86_64':
+self.msvc_arch_str = 'x64'
+elif self.arch == 'aarch64' or self.arch == 'arm64':
+self.msvc_arch_str = 'arm64'
+elif re.match(r'^arm', self.arch):
+self.msvc_arch_str = 'arm'
 
 if toolchain_type == 'msvc':
 # Make sure we're using the appropriate toolchain for the desired
@@ -553,7 +560,10 @

[Lldb-commits] [PATCH] D68549: make ConstString allocate memory in non-tiny chunks

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

Oh, I somehow completely missed the fact that there are 256 of those :-/. In 
that case my numbers are way too much. Can you easily benchmark with different 
numbers? I think something like 131072 for BumpPtrAllocator (to still be large 
enough for the mmap threshold) could be reasonable, the size for StringPool can 
go down to 256 or maybe can be simply dropped.

In D68549#1710293 , @joerg wrote:

> I'm a bit puzzled by the need for this change.


I could measure a difference in lldb symbol load times with the change (and it 
was enough to measure just with a stopwatch). And there's no "need" for the 
change, the logic was that it should improve situations that have many debug 
symbols and shouldn't matter for situations with few debug symbols. The 
assumption was that in realistic scenarios nobody would care that a debugging 
session needs 300KiB of memory instead of 100KiB. If I try to debug either a 
small test app or the whole clang binary, I can see a difference but nothing 
I'd care about in practice. What I care about though is that LLDB spends a 
significant portion of its startup time in ConstString.

In D68549#1710293 , @joerg wrote:

> The bump allocator already has logic to do power-of-two scaling for the 
> allocation, so I wonder why it doesn't work properly here.


As already said, it does work, eventually, but it's not power-of-two and so it 
takes its time before it figures out that it doesn't need to hammer on the 
allocation routines so much.

> Anyway, I think we either make the slab allocation logic smarter than just 
> creating 1MiB slabs and trust the kernel/libc to not actually allocate that 
> much real memory (maybe something like the std::vector growth model) or we 
> fix the fact that we always have 256 string pools.

So, in what realistic scenarios is this change a problem? Maybe it'd be enough 
to just have #if defined(ANDROID) || defined(ARM) || defined(WHATEVER) and not 
overcomplicate this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68549



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

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.




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

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())`.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp:827
+
+  const char *script =
+  "from inspect import signature, Parameter, ismethod \n"

Make this a raw string? Among other benefits, it means clang-format will not 
reflow it weirdly...



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);

Likewise, should most of these `takeErrors` be `cantFail` too ?



Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:339
+#if PY_MAJOR_VERSION < 3
+PyObject *obj = PyObject_CallFunction(m_py_obj, const_cast(format),
+  PythonFormat::get(t)...);

Maybe it's time for a utility function like `py2_const_cast(format)` ?



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);

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))`


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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The code itself looks fine to me. Jim, is this ok from a 
scripting/compatibility/whatnot perspective?




Comment at: 
lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py:25
 
+if sys.version_info[:2] >= (3, 3) or sys.version_info.major < 3:
+# Test a bunch of different kinds of python callables with

Do we have to worry about 3.0<=python<=3.3 actually? Unlike 2.7, these have 
actually been EOLed already, and I would expect/hope that anyone who is not 
stuck at 2.7 has upgraded past them..



Comment at: lldb/scripts/Python/python-wrapper.swig:701-702
+if (!argc) {
+llvm::consumeError(argc.takeError());
+return false;
+}

There is a possibility to shorten this to `return 
errorToBool(argc.takeError())` though I am undecided whether that makes things 
clearer or not, so I am mainly writing this to make you aware of that 
possibility. I'll leave that up to you to decide whether to use it...



Comment at: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:634
+ * accept an arbitrary number */
+int max_positional_args;
 /* the number of positional arguments, including optional ones,

Make this unsigned, and add a symbolic constant (static constexpr unsigned) for 
the "varargs" value.

I've also considered making the vararg-ness more explicit via 
`Optional` but (U)INTMAX is actually a pretty good value for the 
varargs and it enables you to handle this case with a single comparison, so the 
explicitness is probably not worth it.


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] D69016: [lldb] move more things from python to cmake

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: JDevlieghere.
labath accepted this revision.
labath added a subscriber: JDevlieghere.
labath added a comment.
This revision is now accepted and ready to land.

In D69016#1710380 , @davide wrote:

> What are you trying to accomplish here?


I believe the goal is to delete the custom python script completely and rely on 
cmake to do this instead. Now that we no longer have two build systems to worry 
about, I think this is a very good idea and it simplifies the code a lot. I 
believe that at least @JDevlieghere was also in favour of this direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69016



___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thank you guys for jumping onto this. This will be very useful in lldb.


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] D68541: Implement 'up' and 'down' shortcuts in lldb gui

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/gui/basicdebug/TestGuiBasicDebug.py:31
+#  which is not necessarily the order on the screen.)
+self.child.timeout = 2
+self.child.send("s") # step

llunak wrote:
> labath wrote:
> > What's the reason for that? It'd be best to not mess with the timeout in 
> > the test, and particularly to not set such a small time out as it will 
> > likely lead to flakyness on slow/heavily loaded machines.
> I added it because writing and debugging the unittest without this was a 
> pain, but fair enough.
> 
Yeah, the pexpect tests leave a lot to be desired, and there are ways to avoid 
waiting for the entire timeout period when the output does not match (in the 
common cases), but it needs someone willing to implement it.

The deficiencies of pexpect become particularly obivous in the "gui" tests as 
there the raw stream of bytes coming out of the pty is way too low-level to be 
able to match it reliably. If we take the `[^\r\n]` pattern above, it encodes a 
lot of assumptions about how lldb and ncurses choose to print to the screen -- 
ncurses could perfectly well decide to insert some line breaks after printing 
the source code, but then go back (via ansi cursor movement codes), and print 
the stop reason. The result would be exactly the same as far as the user is 
concerned, but the test would fail.

For testing the gui mode, I think we should build (or reuse, if something 
suitable exists) a very simple "terminal emulator" which "understands" cursor 
movement codes (and maybe some other stuff). Then we could assert that the 
contents of the line as the user would see it instead of how it happened to be 
printed out.

Anyway, I don't think you have to do that now, but it is something to think 
about, should you want to invest more heavily into the lldb "gui".


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68541



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

[Lldb-commits] [PATCH] D68980: [LLDB] [test] Allow specifying the full arch for msvc/clang-cl in build.py

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Having proper "target" support is one way to go, though I am disappointed to 
see it go this way, particularly as nobody really knows why invoking clang-cl 
directly should not work, and so the main motivation seems to be a fear of 
potentially breaking something, without understanding what that "something" is, 
or whether it even exists. The reason I am disappointed is because this script 
is a fairly big departure from the way most lit tests in llvm work, and it 
wasn't ever really fully embraced by the lldb community (as can be seen by the 
amount of %clangxx tests that could be using it, but aren't). And while I do 
think it has some benefits, I don't think it should be used as a replacement 
for all clang-cl invocations. If it turns out that there having a script 
wrapping clang-cl is a necessity (which I doubt), then I'd rather see a 
different python script (possibly reusing parts of this one) which just sets up 
the necessary environment, but otherwise forwards arguments to clang-cl 
verbatim. That would enable one to invoke clang-cl with any fancy argument he 
wants (and this could be hidden under %clang-cl), while leaving this script 
with the job of figuring out how to build a working host executable with a 
random compiler.

Anyway, if we're going down this path, then I think it should be done properly:

- having "host" mean "x64" is just wrong. It should be the "host"
- "32" and "64" sound like they could be useful, so they can stay if they are 
needed, but they should mean "the 32-bit flavour of 'host'", not "i386".
- anything else specifies an exact target, and it should probably be a full 
triple, not just the architecture
- using an unsupported builder/target (gcc) should be an error


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

https://reviews.llvm.org/D68980



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

[Lldb-commits] [PATCH] D68549: make ConstString allocate memory in non-tiny chunks

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

Thinking more about this, maybe this is really not the right place and the 
change should be done in BumpPtrAllocator? To me it seems rather unreasonable 
that it would double the allocation size only after 128 allocations. Surely by 
the time it has already allocated 128*4096=0.5MiB memory it could have decided 
to raise the allocation size much sooner?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68549



___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Looks pretty good, just two quick comments.




Comment at: 
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py:15-17
+exe_name = "TestProcess"  # Must match Makefile
+
+

You don't need this. This was present in the other test because it was (also) 
attaching by name, but when attaching by pid, you can do with the default 
"a.out".



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1941
+process_info.GetArguments().Clear();
+process_info.SetArg0(" ");
+break;

Why a single space? I can understand an empty string, but a `" "` seems very 
arbitrary...


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] D68549: make ConstString allocate memory in non-tiny chunks

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68549#1710744 , @llunak wrote:

> Thinking more about this, maybe this is really not the right place and the 
> change should be done in BumpPtrAllocator? To me it seems rather unreasonable 
> that it would double the allocation size only after 128 allocations. Surely 
> by the time it has already allocated 128*4096=0.5MiB memory it could have 
> decided to raise the allocation size much sooner?


That is my impression as well, but beware that BumpPtrAllocator has many users, 
and a they are generally more performance-sensitive and better tuned than lldb, 
so it may turn out that this value actually *is* the sweet spot for something. 
Nevertheless, I think it's worth asking around or trying to send a patch.

However, independently of that, I think it would make sense to raise the 
initial allocation size in ConstString somewhat (just not straight to 1 (256) 
meg).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68549



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

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.


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] D68963: delete SWIG typemaps for FILE*

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:655
+  auto new_file = PythonFile::FromFile(file, mode);
+  if (!new_file)
+return false;

lawrence_danna wrote:
> labath wrote:
> > You'll need to handle the error here. (And it would be better to replace 
> > auto with Expected as that would make it obvious that the error 
> > needs to be handled.)
> thanks,   I don't know why I can't remember to do that.
Yeah, I don't blame you. The Expected behavior is unlike anything else I've 
seen, and I myself am still not convinced that such a draconian way of 
enforcing error checks is in order. However, it is here, and I definitely think 
it's better than passing around "Status" and T objects separately, like old 
lldb APIs do.

What you could do is help yourself and reviewers notice that and tune down the 
"auto" dial (that's also an llvm policy) and spell out the Expected type. 
Otherwise the type can be confused with `Optional` (I wish we could say 
`Expected`), which does *not* require checks...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68963



___
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 created this revision.
mstorsjo added reviewers: labath, stella.stamenova, aleksandr.urakov, amccarth.
Herald added subscribers: JDevlieghere, teemperor, abidh, kristof.beyls.
Herald added a project: LLDB.

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.


Repository:
  rLLDB LLDB

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] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It would be super-awesome if this worked, as this is exactly how the DWARF 
moral equivalents of these tests (e.g. 
test/Shell/SymbolFile/DWARF/debug-types.test) work. Let's see what Stella says 
about this.


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] D68134: [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api

[Martin Storsjö via Phabricator via lldb-commits]

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

Does anyone want to ack this one after updating it to not need changes to tests 
(as far as I know), by using new demangler options?


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

https://reviews.llvm.org/D68134



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, amccarth.
Herald added subscribers: mgrang, mgorny.

The goal of this refactor is to enable ProcessMinidump to take into
account the loaded modules and their sections when computing the
permissions of various ranges of memory, as discussed in D66638 
.

This patch moves some of the responsibility for computing the ranges
from MinidumpParser into ProcessMinidump. MinidumpParser still does the
parsing, but ProcessMinidump becomes responsible for answering the
actual queries about memory ranges. This will enable it (in a follow-up
patch) to augment the information obtained from the parser with data
obtained from actual object files.

The changes in the actual code are fairly straight-forward and just
involve moving code around. MinidumpParser::GetMemoryRegions is renamed
to BuildMemoryRegions to emphasize that it does no caching. The only new
thing is the additional bool flag returned from this function. This
indicates whether the returned regions describe all memory mapped into
the target process. Data obtained from /proc/maps and the MemoryInfoList
stream is considered to be exhaustive. Data obtained from Memory(64)List
is not. This will be used to determine whether we need to augment the
data or not.

This reshuffle means that it is no longer possible/easy to test some of
this code via unit tests, as constructing a ProcessMinidump instance is
hard. Instead, I update the unit tests to only test the parsing of the
actual data, and test the answering of queries through a lit test using
the "memory region" command. The patch also includes some tweaks to the
MemoryRegion class to make the unit tests easier to write.


https://reviews.llvm.org/D69035

Files:
  include/lldb/Target/MemoryRegionInfo.h
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpParser.h
  source/Plugins/Process/minidump/ProcessMinidump.cpp
  source/Plugins/Process/minidump/ProcessMinidump.h
  source/Target/CMakeLists.txt
  source/Target/MemoryRegionInfo.cpp
  test/Shell/Minidump/memory-region.yaml
  unittests/Process/minidump/MinidumpParserTest.cpp

Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- unittests/Process/minidump/MinidumpParserTest.cpp
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -332,34 +332,6 @@
   EXPECT_FALSE(parser->FindMemoryRange(0x7ffe + 4096).hasValue());
 }
 
-void check_region(MinidumpParser &parser, lldb::addr_t addr, lldb::addr_t start,
-  lldb::addr_t end, MemoryRegionInfo::OptionalBool read,
-  MemoryRegionInfo::OptionalBool write,
-  MemoryRegionInfo::OptionalBool exec,
-  MemoryRegionInfo::OptionalBool mapped,
-  ConstString name = ConstString()) {
-  SCOPED_TRACE(addr);
-  auto range_info = parser.GetMemoryRegionInfo(addr);
-  EXPECT_EQ(start, range_info.GetRange().GetRangeBase());
-  EXPECT_EQ(end, range_info.GetRange().GetRangeEnd());
-  EXPECT_EQ(read, range_info.GetReadable());
-  EXPECT_EQ(write, range_info.GetWritable());
-  EXPECT_EQ(exec, range_info.GetExecutable());
-  EXPECT_EQ(mapped, range_info.GetMapped());
-  EXPECT_EQ(name, range_info.GetName());
-}
-
-// Same as above function where addr == start
-void check_region(MinidumpParser &parser, lldb::addr_t start, lldb::addr_t end,
-  MemoryRegionInfo::OptionalBool read,
-  MemoryRegionInfo::OptionalBool write,
-  MemoryRegionInfo::OptionalBool exec,
-  MemoryRegionInfo::OptionalBool mapped,
-  ConstString name = ConstString()) {
-  check_region(parser, start, start, end, read, write, exec, mapped, name);
-}
-
-
 constexpr auto yes = MemoryRegionInfo::eYes;
 constexpr auto no = MemoryRegionInfo::eNo;
 constexpr auto unknown = MemoryRegionInfo::eDontKnow;
@@ -378,17 +350,7 @@
 Type:[  ]
   - Base Address:0x0001
 Allocation Protect: [ PAGE_READ_WRITE ]
-Region Size: 0x0001
-State:   [ MEM_COMMIT ]
-Type:[ MEM_MAPPED ]
-  - Base Address:0x0002
-Allocation Protect: [ PAGE_READ_WRITE ]
-Region Size: 0x0001
-State:   [ MEM_COMMIT ]
-Type:[ MEM_MAPPED ]
-  - Base Address:0x0003
-Allocation Protect: [ PAGE_READ_WRITE ]
-Region Size: 0x1000
+Region Size: 0x00021000
 State:   [ MEM_COMMIT ]
 Type:[ MEM_MAPPED ]
   - Base Address:0x0004
@@ -413,21 +375,20 @@
 )"),
 llvm::Succeeded());
 
-  check_region(*parser, 0x, 0x0001, no, no, no, no);
-  check_region(*parser, 0x0001, 0x0002, yes, yes, no, yes);
-  check_region(*p

[Lldb-commits] [PATCH] D68549: make ConstString allocate memory in non-tiny chunks

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

(Seems like my previous comment was cut off in the middle for some reason?)

We could also just let the allocator take a parameter so that is increases the 
growth size to do it every Nth slab (N=1 for us) and set a slightly larger 
starting size.

By the way, if I look at the flame graph for lldb starting up and setting a 
break point in an LLDB debug binary 
, it seems that on the 
benchmark server we spent less than 0.5% of the startup time in that allocator 
function and much more time in that other ConstString overhead (the 
double-hashing to find the StringPool, the different locking). I'm curious why 
the allocator logic is so disproportionally slow on your setup (0.5% vs 
10-20%). The only real work we do in the allocator is calling malloc, so I 
assume calling malloc is much more expensive on your system?

> Can you easily benchmark with different numbers?

Sadly not, but just running LLDB to attach to a LLDB debug build and running 
this lldb command list 

 should replicate the most important benchmark.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68549



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

[Lldb-commits] [PATCH] D69008: Add arm64_32 support

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Thanks for looking it over, Pavel.  It touches lots of different files, but the 
changes are so tiny in most cases, I didn't know who to set as a reviewer. ;)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69008



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

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

[Pavel Labath via Phabricator via lldb-commits]

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

Actually, it looks like the "follow-up" patch will be more complicated than I 
had hoped for, and so I am starting to wonder whether we shouldn't just have 
the unwinder check for section permissions himself (which would be a 
one-liner). I'll need to investigate more, so you may want to hold off 
reviewing this.


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] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[George Rimar via Phabricator via lldb-commits]

grimar added a comment.

I've posted a change for yaml2obj/obj2yaml here: 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] D68549: make ConstString allocate memory in non-tiny chunks

[Luboš Luňák via Phabricator via lldb-commits]

llunak added a comment.

In D68549#1710986 , @teemperor wrote:

> We could also just let the allocator take a parameter so that is increases 
> the growth size to do it every Nth slab (N=1 for us) and set a slightly 
> larger starting size.


Heh, I wasn't exactly planning on getting into hacking base LLVM classes with 
this seemingly trivial commit. Oh well, a patch is at 
https://reviews.llvm.org/D69050 . If it gets accepted, I'll change ConstString 
to use BumpPtrAllocatorImpl and change 
the StringPool construction back to not having an explicit default.

> By the way, if I look at the flame graph for lldb starting up and setting a 
> break point in an LLDB debug binary 
> , it seems that on 
> the benchmark server we spent less than 0.5% of the startup time in that 
> allocator function and much more time in that other ConstString overhead (the 
> double-hashing to find the StringPool, the different locking). I'm curious 
> why the allocator logic is so disproportionally slow on your setup (0.5% vs 
> 10-20%). The only real work we do in the allocator is calling malloc, so I 
> assume calling malloc is much more expensive on your system?

It is more expensive, relatively speaking. I used -gpubnames, so my profile 
wastes no time in ManualDWARFIndex. And if your "LLDB debug binary" implies 
non-optimized, then I tested with an optimized build, which of course changes 
things. And finally I tested with debug LibreOffice build, which means tons of 
debug data, more than with Clang or LLDB, but processed quickly thanks to 
-gpubnames. Apparently at that point malloc, kernel memory handling, etc. costs 
can become measurable *shrug*. The cost of ConstString hashing etc. is even 
larger, but there I don't see as simple solution as tweaking the allocator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68549



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

[Lldb-commits] [PATCH] D69016: [lldb] move more things from python to cmake

[Haibo Huang via Phabricator via lldb-commits]

hhb added a comment.

In D69016#1710466 , @mgorny wrote:

> I'm not sure if I like the usage of `POST_BUILD` stuff (it provides less 
> control than separate targets) but overall this seems a good change. 
> Replacing ~250 lines of reinventing the wheel with ~20 lines.


Agreed. After I move all the things to cmake, it will be a good idea to 
refactor POST_BUILD to separate targets. And make sure all file dependencies 
are correct. For now I want to keep the same semantic.

In D69016#1710716 , @labath wrote:

> In D69016#1710380 , @davide wrote:
>
> > What are you trying to accomplish here?
>
>
> I believe the goal is to delete the custom python script completely and rely 
> on cmake to do this instead. Now that we no longer have two build systems to 
> worry about, I think this is a very good idea and it simplifies the code a 
> lot. I believe that at least @JDevlieghere was also in favour of this 
> direction.


Besides that, I want to make sure cross compiling is correct. In the old python 
script, we only detect the build platform, and make decision based on that. 
That's obviously wrong for cross compiling...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69016



___
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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

IIUC, the only external change in this patch is that before if you implemented 
your Python command using a class with an `__call__` method, it would have to 
be the signature that took exe_ctx.   Since this is now switching off of the 
number of arguments (less self) you could make an `__call__` form that didn't 
take the extra argument.  I certainly want to discourage that, since the form 
without the exe_ctx will do the wrong thing in some contexts (e.g. if the 
command is used in breakpoint callbacks or stop-hooks).  It looks like that 
would be easy to prevent, you know that you've found __call__ right above where 
you check.  So I think it would be better to explicitly disallow __call__forms 
that don't take the exe_ctx.

This is different from the extra_args cases in the breakpoint callbacks and 
thread plans and so forth.  In those cases, if you don't need extra_args, 
there's no good reason why you should have to have them.  But in the case of 
Python commands, it's actually more correct to get the state you are operating 
on from the exe_ctx.  Otherwise you have to fall back on the currently selected 
process/thread/frame/etc, and there are contexts (like when you have multiple 
targets all hitting breakpoints simultaneously) where it does not make sense to 
have the currently selected state track what process/thread/frame are pertinent 
to the operation of a breakpoint command or stop hook.  So I do want to gently 
discourage this form of command.


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] D68961: Add support for DW_AT_export_symbols for anonymous structs

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

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)


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

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

[walter erquinigo via Phabricator via lldb-commits]

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



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1941
+process_info.GetArguments().Clear();
+process_info.SetArg0(" ");
+break;

labath wrote:
> Why a single space? I can understand an empty string, but a `" "` seems very 
> arbitrary...
that's indeed a typo...


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] D68671: Add the ability to pass extra args to a Python breakpoint command function

[Jim Ingham via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/Interpreter/ScriptInterpreter.h:469
+  
+  virtual int GetNumArgumentsForCallable(const char *callable_name) { 
+return -1;

labath wrote:
> jingham wrote:
> > labath wrote:
> > > jingham wrote:
> > > > jingham wrote:
> > > > > labath wrote:
> > > > > > labath wrote:
> > > > > > > lawrence_danna wrote:
> > > > > > > > jingham wrote:
> > > > > > > > > lawrence_danna wrote:
> > > > > > > > > > labath wrote:
> > > > > > > > > > > jingham wrote:
> > > > > > > > > > > > lawrence_danna wrote:
> > > > > > > > > > > > > labath wrote:
> > > > > > > > > > > > > > In light of varargs functions (`*args, **kwargs`), 
> > > > > > > > > > > > > > which are fairly popular in python, the concept of 
> > > > > > > > > > > > > > "number of arguments of a callable" does not seem 
> > > > > > > > > > > > > > that well defined. The current implementation seems 
> > > > > > > > > > > > > > to return the number of fixed arguments, which 
> > > > > > > > > > > > > > might be fine, but I think this behavior should be 
> > > > > > > > > > > > > > documented. Also, it would be good to modernize 
> > > > > > > > > > > > > > this function signature -- have it take a 
> > > > > > > > > > > > > > StringRef, and return a `Expected` -- 
> > > > > > > > > > > > > > ongoing work by @lawrence_danna will make it 
> > > > > > > > > > > > > > possible to return errors from the python 
> > > > > > > > > > > > > > interpreter, and this will make it possible to 
> > > > > > > > > > > > > > display those, instead of just guessing that this 
> > > > > > > > > > > > > > is because the callable was not found (it could in 
> > > > > > > > > > > > > > fact be because the named thing is not a callable, 
> > > > > > > > > > > > > > of because resolving the name produced an 
> > > > > > > > > > > > > > exception, ...).
> > > > > > > > > > > > > I just took a look at 
> > > > > > > > > > > > > PythonCallable::GetNumArguments() and it's horribly 
> > > > > > > > > > > > > broken.  
> > > > > > > > > > > > > 
> > > > > > > > > > > > > It doesn't even work for the simplest test case I 
> > > > > > > > > > > > > could think of.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > ```  
> > > > > > > > > > > > > auto builtins = PythonModule::Import("builtins");
> > > > > > > > > > > > > ASSERT_THAT_EXPECTED(builtins, llvm::Succeeded());
> > > > > > > > > > > > > auto hex = 
> > > > > > > > > > > > > As(builtins.get().GetAttribute("hex"));
> > > > > > > > > > > > > ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());  
> > > > > > > > > > > > > EXPECT_EQ(hex.get().GetNumArguments().count, 1u);
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > 
> > > > > > > > > > > > > we should really re-write it to use inspect.signature.
> > > > > > > > > > > > Interesting.  We use it for free functions (what you 
> > > > > > > > > > > > pass to the -F option of `breakpoint command add`) and 
> > > > > > > > > > > > for the __init__ and __call__ method in the little 
> > > > > > > > > > > > classes you can make up for scripted thread plans and 
> > > > > > > > > > > > for the class version of Python implemented 
> > > > > > > > > > > > command-line commands.  We have tests for telling 3 
> > > > > > > > > > > > vrs. 4 vrs. not enough or too many, and they all pass.  
> > > > > > > > > > > > So it seems to work in the cases we currently need it 
> > > > > > > > > > > > to work for...
> > > > > > > > > > > > 
> > > > > > > > > > > > "inspect.signature" is python 3 only, and the python 2 
> > > > > > > > > > > > equivalent is deprecated.  So it will take a little 
> > > > > > > > > > > > fancy footwork to use it in the transition period.
> > > > > > > > > > > lol :)
> > > > > > > > > > > 
> > > > > > > > > > > I would actually say that we should try not to use this 
> > > > > > > > > > > function(ality) wherever possible. Making decisions based 
> > > > > > > > > > > on the number of arguments the thing you're about to call 
> > > > > > > > > > > takes sounds weird. I don't want to get too involved in 
> > > > > > > > > > > this, but I was designing this, I'd just say that if one 
> > > > > > > > > > > tries to pass arguments to the callback then the callback 
> > > > > > > > > > > MUST take three arguments (or we'll abort processing the 
> > > > > > > > > > > breakpoint command). If he wants his function to be 
> > > > > > > > > > > called both with arguments and without, he can just add a 
> > > > > > > > > > > default value to the third argument. (And if his function 
> > > > > > > > > > > takes two arguments, but he still tries to pass 
> > > > > > > > > > > something... well, it's his own fault).
> > > > > > > > > > > 
> > > > > > > > > > > Anyway, feel free to ignore this comment, but I felt like 
> > > > > > > > > > > I had to say something. :)
> > > > > > > > > > I completely agree with Pavel.Inspecting a function 
> > > > > > > > 

[Lldb-commits] [lldb] r375024 - [lldb] move more things from python to cmake

[Haibo Huang via lldb-commits]

Author: hhb
Date: Wed Oct 16 11:00:21 2019
New Revision: 375024

URL: http://llvm.org/viewvc/llvm-project?rev=375024&view=rev
Log:
[lldb] move more things from python to cmake

Summary: Move the copy of six.py, lldb.py and macosx/heap

Reviewers: labath

Subscribers: mgorny, lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/CMakeLists.txt
lldb/trunk/scripts/Python/finishSwigPythonLLDB.py

Modified: lldb/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=375024&r1=375023&r2=375024&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Wed Oct 16 11:00:21 2019
@@ -208,6 +208,7 @@ if (NOT LLDB_DISABLE_PYTHON)
   # Add a Post-Build Event to copy over Python files and create the symlink
   # to liblldb.so for the Python API(hardlink on Windows).
   add_custom_target(finish_swig ALL
+COMMAND ${CMAKE_COMMAND} -E make_directory ${lldb_python_build_path}
 COMMAND
   ${PYTHON_EXECUTABLE} 
${LLDB_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
 --srcRoot=${LLDB_SOURCE_DIR}
@@ -224,6 +225,28 @@ if (NOT LLDB_DISABLE_PYTHON)
 DEPENDS ${lldb_scripts_dir}/lldb.py
 COMMENT "Python script sym-linking LLDB Python API")
 
+  if(NOT LLDB_USE_SYSTEM_SIX)
+add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
+  COMMAND ${CMAKE_COMMAND} -E copy
+"${LLDB_SOURCE_DIR}/third_party/Python/module/six/six.py"
+"${lldb_python_build_path}/../six.py")
+  endif()
+
+  add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
+COMMAND ${CMAKE_COMMAND} -E copy
+  "${lldb_scripts_dir}/lldb.py"
+  "${lldb_python_build_path}/__init__.py")
+
+  if(APPLE)
+SET(lldb_python_heap_dir "${lldb_python_build_path}/macosx/heap")
+add_custom_command(TARGET finish_swig POST_BUILD VERBATIM
+  COMMAND ${CMAKE_COMMAND} -E make_directory ${lldb_python_heap_dir}
+  COMMAND ${CMAKE_COMMAND} -E copy
+"${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap/heap_find.cpp"
+"${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap/Makefile"
+${lldb_python_heap_dir})
+  endif()
+
   function(create_relative_symlink target dest_file output_dir output_name)
 get_filename_component(dest_file ${dest_file} ABSOLUTE)
 get_filename_component(output_dir ${output_dir} ABSOLUTE)

Modified: lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/finishSwigPythonLLDB.py?rev=375024&r1=375023&r2=375024&view=diff
==
--- lldb/trunk/scripts/Python/finishSwigPythonLLDB.py (original)
+++ lldb/trunk/scripts/Python/finishSwigPythonLLDB.py Wed Oct 16 11:00:21 2019
@@ -74,56 +74,6 @@ strMsgCopySixPy = "Copying six.py from '
 strErrMsgCopySixPyFailed = "Unable to copy '%s' to '%s'"
 
 
-def is_debug_interpreter():
-return hasattr(sys, 'gettotalrefcount')
-
-#++---
-# Details:  Copy files needed by lldb/macosx/heap.py to build libheap.dylib.
-# Args: vDictArgs   - (R) Program input parameters.
-#   vstrFrameworkPythonDir  - (R) Python framework directory.
-# Returns:  Bool - True = function success, False = failure.
-#   Str - Error description on task failure.
-# Throws:   None.
-#--
-
-
-def macosx_copy_file_for_heap(vDictArgs, vstrFrameworkPythonDir):
-dbg = utilsDebug.CDebugFnVerbose(
-"Python script macosx_copy_file_for_heap()")
-bOk = True
-strMsg = ""
-
-eOSType = utilsOsType.determine_os_type()
-if eOSType != utilsOsType.EnumOsType.Darwin:
-return (bOk, strMsg)
-
-strHeapDir = os.path.join(vstrFrameworkPythonDir, "macosx", "heap")
-strHeapDir = os.path.normcase(strHeapDir)
-if os.path.exists(strHeapDir) and os.path.isdir(strHeapDir):
-return (bOk, strMsg)
-
-os.makedirs(strHeapDir)
-
-strRoot = os.path.normpath(vDictArgs["--srcRoot"])
-strSrc = os.path.join(
-strRoot,
-"examples",
-"darwin",
-"heap_find",
-"heap",
-"heap_find.cpp")
-shutil.copy(strSrc, strHeapDir)
-strSrc = os.path.join(
-strRoot,
-"examples",
-"darwin",
-"heap_find",
-"heap",
-"Makefile")
-shutil.copy(strSrc, strHeapDir)
-
-return (bOk, strMsg)
-
 #++---
 # Details:  Create Python packages and Python __init__ files.
 # Args: vDictArgs   - (R) Program input parameters.
@@ -198,142 +148,6 @@ def create_py_pkg(
 
 return (bOk, strMsg)
 
-#++---
-# Details:  Copy the lldb.py file into the lldb package directory and rename
-#  

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

[Haibo Huang via Phabricator via lldb-commits]

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

Format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69019

Files:
  lldb/CMakeLists.txt
  lldb/scripts/Python/createPythonInit.py
  lldb/scripts/Python/finishSwigPythonLLDB.py
  lldb/scripts/finishSwigWrapperClasses.py
  lldb/scripts/utilsArgsParse.py
  lldb/scripts/utilsDebug.py
  lldb/scripts/utilsOsType.py

Index: lldb/scripts/utilsOsType.py
===
--- lldb/scripts/utilsOsType.py
+++ /dev/null
@@ -1,103 +0,0 @@
-""" Utility module to determine the OS Python running on
-
---
-File: utilsOsType.py
-
-Overview:   Python module to supply functions and an enumeration to
-help determine the platform type, bit size and OS currently
-being used.
---
-
-"""
-
-# Python modules:
-import sys  # Provide system information
-
-# Third party modules:
-
-# In-house modules:
-
-# Instantiations:
-
-# Enumerations:
-#-
-# Details:  Class to implement a 'C' style enumeration type.
-# Gotchas:  None.
-# Authors:  Illya Rudkin 28/11/2013.
-# Changes:  None.
-#--
-if sys.version_info.major >= 3:
-from enum import Enum
-
-class EnumOsType(Enum):
-Unknown = 0
-Darwin = 1
-FreeBSD = 2
-Linux = 3
-NetBSD = 4
-OpenBSD = 5
-Windows = 6
-kFreeBSD = 7
-else:
-class EnumOsType(object):
-values = ["Unknown",
-  "Darwin",
-  "FreeBSD",
-  "Linux",
-  "NetBSD",
-  "OpenBSD",
-  "Windows",
-  "kFreeBSD"]
-
-class __metaclass__(type):
-#++
-# Details:  Fn acts as an enumeration.
-# Args: vName - (R) Enumeration to match.
-# Returns:  Int - Matching enumeration/index.
-# Throws:   None.
-#--
-
-def __getattr__(cls, vName):
-return cls.values.index(vName)
-
-#++---
-# Details:  Reverse fast lookup of the values list.
-# Args: vI - (R) Index / enumeration.
-# Returns:  Str - text description matching enumeration.
-# Throws:   None.
-#--
-def name_of(cls, vI):
-return EnumOsType.values[vI]
-
-#-
-#-
-#-
-
-#++---
-# Details:  Determine what operating system is currently running on.
-# Args: None.
-# Returns:  EnumOsType - The OS type being used ATM.
-# Throws:   None.
-#--
-
-
-def determine_os_type():
-eOSType = EnumOsType.Unknown
-
-strOS = sys.platform
-if strOS == "darwin":
-eOSType = EnumOsType.Darwin
-elif strOS.startswith("freebsd"):
-eOSType = EnumOsType.FreeBSD
-elif strOS.startswith("linux"):
-eOSType = EnumOsType.Linux
-elif strOS.startswith("netbsd"):
-eOSType = EnumOsType.NetBSD
-elif strOS.startswith("openbsd"):
-eOSType = EnumOsType.OpenBSD
-elif strOS == "win32":
-eOSType = EnumOsType.Windows
-elif strOS.startswith("gnukfreebsd"):
-eOSType = EnumOsType.kFreeBSD
-
-return eOSType
Index: lldb/scripts/utilsDebug.py
===
--- lldb/scripts/utilsDebug.py
+++ /dev/null
@@ -1,125 +0,0 @@
-""" Utility module to help debug Python scripts
-
---
-File: utilsDebug.py
-
-Overview:  Python module to supply functions to help debug Python
-   scripts.
-Gotchas:   None.
-Copyright: None.
---
-"""
-
-# Python modules:
-import sys
-
-# Third party modules:
-
-# In-house modules:
-
-# Instantiations:
-
-#-
-# Details: Class to implement simple stack function trace. Instantiation the
-#  class as the first function you want to trace. Example:
-#  obj = utilsDebug.CDebugFnVerbose("validate_arguments()")
-# Gotchas: This class will not work in properly in a multi-threaded
-#  environment.
-# Authors: Illya Rudkin 28/11/2013.
-# Changes: None.
-#--
-
-
-class CD

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

[Haibo Huang via Phabricator via lldb-commits]

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

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69019

Files:
  lldb/CMakeLists.txt
  lldb/scripts/Python/createPythonInit.py
  lldb/scripts/Python/finishSwigPythonLLDB.py
  lldb/scripts/finishSwigWrapperClasses.py
  lldb/scripts/utilsArgsParse.py
  lldb/scripts/utilsDebug.py
  lldb/scripts/utilsOsType.py

Index: lldb/scripts/utilsOsType.py
===
--- lldb/scripts/utilsOsType.py
+++ /dev/null
@@ -1,103 +0,0 @@
-""" Utility module to determine the OS Python running on
-
---
-File: utilsOsType.py
-
-Overview:   Python module to supply functions and an enumeration to
-help determine the platform type, bit size and OS currently
-being used.
---
-
-"""
-
-# Python modules:
-import sys  # Provide system information
-
-# Third party modules:
-
-# In-house modules:
-
-# Instantiations:
-
-# Enumerations:
-#-
-# Details:  Class to implement a 'C' style enumeration type.
-# Gotchas:  None.
-# Authors:  Illya Rudkin 28/11/2013.
-# Changes:  None.
-#--
-if sys.version_info.major >= 3:
-from enum import Enum
-
-class EnumOsType(Enum):
-Unknown = 0
-Darwin = 1
-FreeBSD = 2
-Linux = 3
-NetBSD = 4
-OpenBSD = 5
-Windows = 6
-kFreeBSD = 7
-else:
-class EnumOsType(object):
-values = ["Unknown",
-  "Darwin",
-  "FreeBSD",
-  "Linux",
-  "NetBSD",
-  "OpenBSD",
-  "Windows",
-  "kFreeBSD"]
-
-class __metaclass__(type):
-#++
-# Details:  Fn acts as an enumeration.
-# Args: vName - (R) Enumeration to match.
-# Returns:  Int - Matching enumeration/index.
-# Throws:   None.
-#--
-
-def __getattr__(cls, vName):
-return cls.values.index(vName)
-
-#++---
-# Details:  Reverse fast lookup of the values list.
-# Args: vI - (R) Index / enumeration.
-# Returns:  Str - text description matching enumeration.
-# Throws:   None.
-#--
-def name_of(cls, vI):
-return EnumOsType.values[vI]
-
-#-
-#-
-#-
-
-#++---
-# Details:  Determine what operating system is currently running on.
-# Args: None.
-# Returns:  EnumOsType - The OS type being used ATM.
-# Throws:   None.
-#--
-
-
-def determine_os_type():
-eOSType = EnumOsType.Unknown
-
-strOS = sys.platform
-if strOS == "darwin":
-eOSType = EnumOsType.Darwin
-elif strOS.startswith("freebsd"):
-eOSType = EnumOsType.FreeBSD
-elif strOS.startswith("linux"):
-eOSType = EnumOsType.Linux
-elif strOS.startswith("netbsd"):
-eOSType = EnumOsType.NetBSD
-elif strOS.startswith("openbsd"):
-eOSType = EnumOsType.OpenBSD
-elif strOS == "win32":
-eOSType = EnumOsType.Windows
-elif strOS.startswith("gnukfreebsd"):
-eOSType = EnumOsType.kFreeBSD
-
-return eOSType
Index: lldb/scripts/utilsDebug.py
===
--- lldb/scripts/utilsDebug.py
+++ /dev/null
@@ -1,125 +0,0 @@
-""" Utility module to help debug Python scripts
-
---
-File: utilsDebug.py
-
-Overview:  Python module to supply functions to help debug Python
-   scripts.
-Gotchas:   None.
-Copyright: None.
---
-"""
-
-# Python modules:
-import sys
-
-# Third party modules:
-
-# In-house modules:
-
-# Instantiations:
-
-#-
-# Details: Class to implement simple stack function trace. Instantiation the
-#  class as the first function you want to trace. Example:
-#  obj = utilsDebug.CDebugFnVerbose("validate_arguments()")
-# Gotchas: This class will not work in properly in a multi-threaded
-#  environment.
-# Authors: Illya Rudkin 28/11/2013.
-# Changes: None.
-#--
-
-
-class CD

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

[walter erquinigo via Phabricator via lldb-commits]

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

addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68293

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/packages/Python/lldbsuite/test/commands/platform/process/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
  lldb/packages/Python/lldbsuite/test/commands/platform/process/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -227,13 +227,11 @@
 }
 
 if (verbose || show_args) {
+  s.PutCString(m_arg0);
   const uint32_t argc = m_arguments.GetArgumentCount();
-  if (argc > 0) {
-for (uint32_t i = 0; i < argc; i++) {
-  if (i > 0)
-s.PutChar(' ');
-  s.PutCString(m_arguments.GetArgumentAtIndex(i));
-}
+  for (uint32_t i = 0; i < argc; i++) {
+s.PutChar(' ');
+s.PutCString(m_arguments.GetArgumentAtIndex(i));
   }
 } else {
   s.PutCString(GetName());
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1181,6 +1181,15 @@
   proc_info.GetEffectiveUserID(), proc_info.GetEffectiveGroupID());
   response.PutCString("name:");
   response.PutStringAsRawHex8(proc_info.GetExecutableFile().GetCString());
+
+  response.PutChar(';');
+  response.PutCString("args:");
+  response.PutStringAsRawHex8(proc_info.GetArg0());
+  for (auto &arg : proc_info.GetArguments()) {
+response.PutChar('-');
+response.PutStringAsRawHex8(arg.ref());
+  }
+
   response.PutChar(';');
   const ArchSpec &proc_arch = proc_info.GetArchitecture();
   if (proc_arch.IsValid()) {
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
@@ -1927,6 +1927,26 @@
 std::string name;
 extractor.GetHexByteString(name);
 process_info.GetExecutableFile().SetFile(name, FileSpec::Style::native);
+  } else if (name.equals("args")) {
+llvm::StringRef encoded_args(value), hex_arg;
+
+bool is_arg0 = true;
+while (!encoded_args.empty()) {
+  std::tie(hex_arg, encoded_args) = encoded_args.split('-');
+  std::string arg;
+  StringExtractor extractor(hex_arg);
+  if (extractor.GetHexByteString(arg) * 2 != hex_arg.size()) {
+// In case of wrong encoding, we discard all the arguments
+process_info.GetArguments().Clear();
+process_info.SetArg0("");
+break;
+  }
+  if (is_arg0)
+process_info.SetArg0(arg);
+  else
+process_info.GetArguments().AppendArgument(arg);
+  is_arg0 = false;
+}
   } else if (name.equals("cputype")) {
 value.getAsInteger(0, cpu);
   } else if (name.equals("cpusubtype")) {
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
@@ -5,6 +5,8 @@
 from lldbsuite.test.decorators import *
 from gdbclientutils import *
 
+def hexlify(string):
+return binascii.hexlify(string.encode()).decode()
 
 class TestPlatformClient(GDBRemoteTestBase):
 
@@ -12,22 +14,52 @@
 """Test connecting to a remote linux platform"""
 
 class MyResponder(MockGDBServerResponder):
+def __init__(self):
+MockGDBServerResponder.__init__(self)
+self.currentQsProc = 0
+self.all_users = False
+
 def qfProcessInfo(self, packet):
 if "all_users:1" in packet:
-return "pid:10;ppid:1;uid:1;gid:1;euid:1;egid:1;name:" + binascii.hexlify("/a/test_process".encode()).decode() + ";"
+self.all_users = True
+name = hexlify("/a/test_process")
+args = "-".join(map(hex

[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 added inline comments.



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 \

Why is lld-link not specified as %lld_link?


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] D69058: [test] Add a .clang-format file for the shell test.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added subscribers: teemperor, abidh, dexonsmith, mehdi_amini.
Herald added a project: LLDB.

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.


Repository:
  rLLDB LLDB

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] [PATCH] D69016: [lldb] move more things from python to cmake

[Haibo Huang via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d4a40751ff3: [lldb] move more things from python to cmake 
(authored by hhb).

Changed prior to commit:
  https://reviews.llvm.org/D69016?vs=225151&id=225270#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69016

Files:
  lldb/CMakeLists.txt
  lldb/scripts/Python/finishSwigPythonLLDB.py

Index: lldb/scripts/Python/finishSwigPythonLLDB.py
===
--- lldb/scripts/Python/finishSwigPythonLLDB.py
+++ lldb/scripts/Python/finishSwigPythonLLDB.py
@@ -74,56 +74,6 @@
 strErrMsgCopySixPyFailed = "Unable to copy '%s' to '%s'"
 
 
-def is_debug_interpreter():
-return hasattr(sys, 'gettotalrefcount')
-
-#++---
-# Details:  Copy files needed by lldb/macosx/heap.py to build libheap.dylib.
-# Args: vDictArgs   - (R) Program input parameters.
-#   vstrFrameworkPythonDir  - (R) Python framework directory.
-# Returns:  Bool - True = function success, False = failure.
-#   Str - Error description on task failure.
-# Throws:   None.
-#--
-
-
-def macosx_copy_file_for_heap(vDictArgs, vstrFrameworkPythonDir):
-dbg = utilsDebug.CDebugFnVerbose(
-"Python script macosx_copy_file_for_heap()")
-bOk = True
-strMsg = ""
-
-eOSType = utilsOsType.determine_os_type()
-if eOSType != utilsOsType.EnumOsType.Darwin:
-return (bOk, strMsg)
-
-strHeapDir = os.path.join(vstrFrameworkPythonDir, "macosx", "heap")
-strHeapDir = os.path.normcase(strHeapDir)
-if os.path.exists(strHeapDir) and os.path.isdir(strHeapDir):
-return (bOk, strMsg)
-
-os.makedirs(strHeapDir)
-
-strRoot = os.path.normpath(vDictArgs["--srcRoot"])
-strSrc = os.path.join(
-strRoot,
-"examples",
-"darwin",
-"heap_find",
-"heap",
-"heap_find.cpp")
-shutil.copy(strSrc, strHeapDir)
-strSrc = os.path.join(
-strRoot,
-"examples",
-"darwin",
-"heap_find",
-"heap",
-"Makefile")
-shutil.copy(strSrc, strHeapDir)
-
-return (bOk, strMsg)
-
 #++---
 # Details:  Create Python packages and Python __init__ files.
 # Args: vDictArgs   - (R) Program input parameters.
@@ -198,142 +148,6 @@
 
 return (bOk, strMsg)
 
-#++---
-# Details:  Copy the lldb.py file into the lldb package directory and rename
-#   to __init_.py.
-# Args: vDictArgs   - (R) Program input parameters.
-#   vstrFrameworkPythonDir  - (R) Python framework directory.
-#   vstrCfgBldDir   - (R) Config directory path.
-# Returns:  Bool - True = function success, False = failure.
-#   Str - Error description on task failure.
-# Throws:   None.
-#--
-
-
-def copy_lldbpy_file_to_lldb_pkg_dir(
-vDictArgs,
-vstrFrameworkPythonDir,
-vstrCfgBldDir):
-dbg = utilsDebug.CDebugFnVerbose(
-"Python script copy_lldbpy_file_to_lldb_pkg_dir()")
-bOk = True
-bDbg = "-d" in vDictArgs
-strMsg = ""
-
-strSrc = os.path.join(vstrCfgBldDir, "lldb.py")
-strSrc = os.path.normcase(strSrc)
-strDst = os.path.join(vstrFrameworkPythonDir, "__init__.py")
-strDst = os.path.normcase(strDst)
-
-if not os.path.exists(strSrc):
-strMsg = strErrMsgLLDBPyFileNotNotFound % strSrc
-return (bOk, strMsg)
-
-try:
-if bDbg:
-print((strMsgCopyLLDBPy % (strSrc, strDst)))
-shutil.copyfile(strSrc, strDst)
-except IOError as e:
-bOk = False
-strMsg = "I/O error(%d): %s %s" % (e.errno,
-   e.strerror, strErrMsgCpLldbpy)
-if e.errno == 2:
-strMsg += " Src:'%s' Dst:'%s'" % (strSrc, strDst)
-except:
-bOk = False
-strMsg = strErrMsgUnexpected % sys.exec_info()[0]
-
-return (bOk, strMsg)
-
-
-def copy_six(vDictArgs, vstrFrameworkPythonDir):
-dbg = utilsDebug.CDebugFnVerbose("Python script copy_six()")
-bDbg = "-d" in vDictArgs
-bOk = True
-strMsg = ""
-site_packages_dir = os.path.dirname(vstrFrameworkPythonDir)
-six_module_filename = "six.py"
-src_file = os.path.join(
-vDictArgs['--srcRoot'],
-"third_party",
-"Python",
-"module",
-"six",
-six_module_filename)
-src_file = os.path.normpath(src_file)
-target = os.path.join(site_packages_dir, six_module_filename)
-
-if bDbg:
-print((strMsgCopySixPy % (src_file, target)))
-try:
-shutil.copyfile(src_file, target)
-except:
-bOk = False
-strMsg = strErrMsgCopySixPyFailed

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

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.


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

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

[walter erquinigo via Phabricator via lldb-commits]

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



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


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] D69031: [LLDB] [test] Use %clang_cl instead of build.py in a few tests

[Martin Storsjö via Phabricator via lldb-commits]

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



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 \

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.


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] [lldb] r375029 - [android/process list] support showing process arguments

[Walter Erquinigo via lldb-commits]

Author: wallace
Date: Wed Oct 16 11:47:05 2019
New Revision: 375029

URL: http://llvm.org/viewvc/llvm-project?rev=375029&view=rev
Log:
[android/process list] support showing process arguments

Summary:
The qfProcessInfo and qsProcessInfo packets currently don't set the processes' 
arguments, however the platform process list -v command tries to print it.
In this diff I'm adding the arguments as part of the packet, and now the 
command shows the arguments just like on mac.

On Mac:

5071  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /usr/libexec/secd
5031  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /usr/libexec/secinitd
5011  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /usr/libexec/languageassetd --firstLogin
4971  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /usr/libexec/trustd --agent
4961  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /usr/libexec/lsd
4941  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /System/Library/Frameworks/CoreTelephony.framework/Support/CommCenter -L
4911  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /usr/sbin/distnoted agent
4891  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /usr/libexec/UserEventAgent (Aqua)
4841  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /usr/sbin/cfprefsd agent
4831  wallace1876110778 wallace1876110778 x86_64-apple-macosx   
   /System/Library/Frameworks/LocalAuthentication.framework/Support/coreauthd
On android:

1561   1016   root   0 0  
aarch64-unknown-linux-android  /system/bin/ip6tables-restore--noflush -w -v
1805   9821000   1000  1000 
 android:drmService
1811   98210189  10189 10189
 com.qualcomm.embms:remote
1999   1  1000   1000  1000   
aarch64-unknown-linux-android  /system/bin/tlc_serverCCM
2332   98210038  10038 10038
 com.android.systemui
2378   9831053   1053  1053 
 webview_zygote
2448   9825013   5013  5013 
 com.sec.location.nsflp2
2465   98210027  10027 10027
 com.google.android.gms.persistent

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

Added:
lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/
lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/Makefile

lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
lldb/trunk/packages/Python/lldbsuite/test/commands/platform/process/main.cpp
Modified:
lldb/trunk/docs/lldb-gdb-remote.txt

lldb/trunk/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py

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

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Utility/ProcessInfo.cpp

Modified: lldb/trunk/docs/lldb-gdb-remote.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/lldb-gdb-remote.txt?rev=375029&r1=375028&r2=375029&view=diff
==
--- lldb/trunk/docs/lldb-gdb-remote.txt (original)
+++ lldb/trunk/docs/lldb-gdb-remote.txt Wed Oct 16 11:47:05 2019
@@ -1,15 +1,15 @@
 LLDB has added new GDB server packets to better support multi-threaded and
 remote debugging. Why? Normally you need to start the correct GDB and the
 correct GDB server when debugging. If you have mismatch, then things go wrong
-very quickly. LLDB makes extensive use of the GDB remote protocol and we 
+very quickly. LLDB makes extensive use of the GDB remote protocol and we
 wanted to make sure that the experience was a bit more dynamic where we can
 discover information about a remote target with having to know anything up
-front. We also ran into performance issues with the existing GDB remote 
+front. We also ran into performance issues with the existing GDB remote
 protocol that can be overcome when using a reliable communications layer.
 Some packets improve performance, others allow for remote process launching
 (if you have an OS), and others allow us to dynamically figure out what
 registers a thread might have. Again with GDB, both sides pre-agree on how the
-registers will look (how many, their register number,name and offsets). We 
+registers will look (how many, their register number,name and offsets). We
 prefer to be able to dynamically determine what kind of architecture, OS and
 vendor we a

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

[walter erquinigo via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG48a50ee0344d: [android/process list] support showing process 
arguments (authored by wallace).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68293

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/packages/Python/lldbsuite/test/commands/platform/process/Makefile
  
lldb/packages/Python/lldbsuite/test/commands/platform/process/TestProcessList.py
  lldb/packages/Python/lldbsuite/test/commands/platform/process/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -227,13 +227,11 @@
 }
 
 if (verbose || show_args) {
+  s.PutCString(m_arg0);
   const uint32_t argc = m_arguments.GetArgumentCount();
-  if (argc > 0) {
-for (uint32_t i = 0; i < argc; i++) {
-  if (i > 0)
-s.PutChar(' ');
-  s.PutCString(m_arguments.GetArgumentAtIndex(i));
-}
+  for (uint32_t i = 0; i < argc; i++) {
+s.PutChar(' ');
+s.PutCString(m_arguments.GetArgumentAtIndex(i));
   }
 } else {
   s.PutCString(GetName());
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1185,6 +1185,15 @@
   proc_info.GetEffectiveUserID(), proc_info.GetEffectiveGroupID());
   response.PutCString("name:");
   response.PutStringAsRawHex8(proc_info.GetExecutableFile().GetCString());
+
+  response.PutChar(';');
+  response.PutCString("args:");
+  response.PutStringAsRawHex8(proc_info.GetArg0());
+  for (auto &arg : proc_info.GetArguments()) {
+response.PutChar('-');
+response.PutStringAsRawHex8(arg.ref());
+  }
+
   response.PutChar(';');
   const ArchSpec &proc_arch = proc_info.GetArchitecture();
   if (proc_arch.IsValid()) {
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
@@ -1927,6 +1927,26 @@
 std::string name;
 extractor.GetHexByteString(name);
 process_info.GetExecutableFile().SetFile(name, FileSpec::Style::native);
+  } else if (name.equals("args")) {
+llvm::StringRef encoded_args(value), hex_arg;
+
+bool is_arg0 = true;
+while (!encoded_args.empty()) {
+  std::tie(hex_arg, encoded_args) = encoded_args.split('-');
+  std::string arg;
+  StringExtractor extractor(hex_arg);
+  if (extractor.GetHexByteString(arg) * 2 != hex_arg.size()) {
+// In case of wrong encoding, we discard all the arguments
+process_info.GetArguments().Clear();
+process_info.SetArg0("");
+break;
+  }
+  if (is_arg0)
+process_info.SetArg0(arg);
+  else
+process_info.GetArguments().AppendArgument(arg);
+  is_arg0 = false;
+}
   } else if (name.equals("cputype")) {
 value.getAsInteger(0, cpu);
   } else if (name.equals("cpusubtype")) {
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
@@ -5,6 +5,8 @@
 from lldbsuite.test.decorators import *
 from gdbclientutils import *
 
+def hexlify(string):
+return binascii.hexlify(string.encode()).decode()
 
 class TestPlatformClient(GDBRemoteTestBase):
 
@@ -12,22 +14,52 @@
 """Test connecting to a remote linux platform"""
 
 class MyResponder(MockGDBServerResponder):
+def __init__(self):
+MockGDBServerResponder.__init__(self)
+self.currentQsProc = 0
+self.all_users = False
+
 def qfProcessInfo(self, packet):
 if "all_users:1" in packet:
-return "pid:10;ppid:1;uid:1;gid:1;euid:1;egid:1;name:" + binascii.hexlify("/a/test_process".encode()).decode() + ";"
+self.all_users = True
+  

[Lldb-commits] [lldb] r375032 - Add arm64_32 support to lldb, an ILP32 codegen

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Wed Oct 16 12:14:49 2019
New Revision: 375032

URL: http://llvm.org/viewvc/llvm-project?rev=375032&view=rev
Log:
Add arm64_32 support to lldb, an ILP32 codegen 
that runs on arm64 ISA targets, specifically 
Apple watches.


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

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

lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-function/TestCallStdStringFunction.py

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/char/TestExprsChar.py

lldb/trunk/packages/Python/lldbsuite/test/commands/expression/persist_objc_pointeetype/TestPersistObjCPointeeType.py

lldb/trunk/packages/Python/lldbsuite/test/commands/register/register/register_command/TestRegisters.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsindexpath/TestDataFormatterNSIndexPath.py
lldb/trunk/packages/Python/lldbsuite/test/lldbplatformutil.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/watchpoint/TestWatchpointIter.py
lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
lldb/trunk/source/Host/macosx/objcxx/Host.mm
lldb/trunk/source/Host/macosx/objcxx/HostInfoMacOSX.mm
lldb/trunk/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp
lldb/trunk/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp
lldb/trunk/source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp

lldb/trunk/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/trunk/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp
lldb/trunk/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/trunk/source/Plugins/Process/Utility/StopInfoMachException.cpp

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

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp
lldb/trunk/source/Symbol/CompactUnwindInfo.cpp
lldb/trunk/source/Target/Platform.cpp
lldb/trunk/source/Target/Thread.cpp
lldb/trunk/source/Utility/ArchSpec.cpp
lldb/trunk/tools/compact-unwind/compact-unwind-dumper.c
lldb/trunk/tools/debugserver/source/DNB.cpp
lldb/trunk/unittests/Utility/ArchSpecTest.cpp

Modified: lldb/trunk/include/lldb/Utility/ArchSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/ArchSpec.h?rev=375032&r1=375031&r2=375032&view=diff
==
--- lldb/trunk/include/lldb/Utility/ArchSpec.h (original)
+++ lldb/trunk/include/lldb/Utility/ArchSpec.h Wed Oct 16 12:14:49 2019
@@ -122,6 +122,7 @@ public:
 eCore_thumbv7em,
 eCore_arm_arm64,
 eCore_arm_armv8,
+eCore_arm_arm64_32,
 eCore_arm_aarch64,
 
 eCore_mips32,

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py?rev=375032&r1=375031&r2=375032&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/arm/breakpoint-it/TestBreakpointIt.py 
Wed Oct 16 12:14:49 2019
@@ -17,7 +17,7 @@ class TestBreakpointIt(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 @skipIf(archs=no_match(["arm"]))
-@skipIf(archs=["arm64", "arm64e"])
+@skipIf(archs=["arm64", "arm64e", "arm64_32"])
 def test_false(self):
 self.build()
 exe = self.getBuildArtifact("a.out")
@@ -31,7 +31,7 @@ class TestBreakpointIt(TestBase):
 "Breakpoint does not get hit")
 
 @skipIf(archs=no_match(["arm"]))
-@skipIf(archs=["arm64", "arm64e"])
+@skipIf(archs=["arm64", "arm64e", "arm64_32"])
 def test_true(self):
 self.build()
 exe = self.getBuildArtifact("a.out")

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-function/TestCallStdStringFunction.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/commands/expression/call-function/TestCallStdStringFunction.py?rev=375032&r1=375031&r2=375032&view=diff
===

[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 225284.
lawrence_danna added a comment.

added tests for __call__ objects, and a fix for python2


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, 1);
 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, 2);
 EXPECT_EQ(arginfo.get().has_varargs, false);
-EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
   {
@@ -667,15 +667,26 @@
 auto arginfo = lambda.GetArgInfo();
 ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
 EXPECT_EQ(arginfo.get().count, 2);
+EXPECT_EQ(arginfo.get().max_positional_args, INT_MAX);
 EXPECT_EQ(arginfo.get().has_varargs, true);
-EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
   {
 const char *script = "class Foo: \n"
  "  def bar(self, x):\n"
  " return x \n"
+ "  @classmethod \n"
+ "  def classbar(cls, x):\n"
+ " return x \n"
+ "  @staticmethod \n"
+ "  def staticbar(x):\n"
+ " return x \n"
+ "  def __call__(self, x): \n"
+ " return x \n"
+ "obj = Foo() \n"
  "bar_bound   = Foo().bar \n"
+ "bar_class   = Foo().classbar \n"
+ "bar_static  = Foo().staticbar \n"
  "bar_unbound = Foo.bar \n";
 PyObject *o =
 PyRun_String(script, Py_file_input, globals.get(), globals.get());
@@ -687,16 +698,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, 1);
 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, 2);
+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, 1);
+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, 1);
+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, 1);
 EXPECT_EQ(arginfo.get().has_varargs, false);
-EXPECT_EQ(arginfo.get().is_bound_method, false);
   }
 
 #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
@@ -710,9 +742,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, 1);
 EXPECT_EQ(arginfo.get().has_varargs, false);
-EXPECT_EQ(arginfo.get().is_bound_method, fal

[Lldb-commits] [PATCH] D69008: Add arm64_32 support

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda closed this revision.
jasonmolenda added a comment.

committed in r375032 where I accidentally copied in the wrong phabracator URL, 
so lol.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D69008



___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



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 \

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.


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] [lldb] r375034 - [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api. NFCI.

[Martin Storsjo via lldb-commits]

Author: mstorsjo
Date: Wed Oct 16 12:39:56 2019
New Revision: 375034

URL: http://llvm.org/viewvc/llvm-project?rev=375034&view=rev
Log:
[LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api. 
NFCI.

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

Modified:
lldb/trunk/source/Core/Mangled.cpp

Modified: lldb/trunk/source/Core/Mangled.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Mangled.cpp?rev=375034&r1=375033&r2=375034&view=diff
==
--- lldb/trunk/source/Core/Mangled.cpp (original)
+++ lldb/trunk/source/Core/Mangled.cpp Wed Oct 16 12:39:56 2019
@@ -8,13 +8,6 @@
 
 #include "lldb/Core/Mangled.h"
 
-#if defined(_WIN32)
-#include "lldb/Host/windows/windows.h"
-
-#include 
-#pragma comment(lib, "dbghelp.lib")
-#endif
-
 #include "lldb/Core/RichManglingContext.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
@@ -39,25 +32,6 @@
 #include 
 using namespace lldb_private;
 
-#if defined(_MSC_VER)
-static DWORD safeUndecorateName(const char *Mangled, char *Demangled,
-DWORD DemangledLength) {
-  static std::mutex M;
-  std::lock_guard Lock(M);
-  return ::UnDecorateSymbolName(
-  Mangled, Demangled, DemangledLength,
-  UNDNAME_NO_ACCESS_SPECIFIERS |   // Strip public, private, protected
-   // keywords
-  UNDNAME_NO_ALLOCATION_LANGUAGE | // Strip __thiscall, __stdcall,
-   // etc keywords
-  UNDNAME_NO_THROW_SIGNATURES |// Strip throw() specifications
-  UNDNAME_NO_MEMBER_TYPE | // Strip virtual, static, etc
-   // specifiers
-  UNDNAME_NO_MS_KEYWORDS   // Strip all MS extension keywords
-  );
-}
-#endif
-
 static inline Mangled::ManglingScheme cstring_mangling_scheme(const char *s) {
   if (s) {
 if (s[0] == '?')
@@ -200,28 +174,20 @@ void Mangled::SetValue(ConstString name)
 
 // Local helpers for different demangling implementations.
 static char *GetMSVCDemangledStr(const char *M) {
-#if defined(_MSC_VER)
-  const size_t demangled_length = 2048;
-  char *demangled_cstr = static_cast(::malloc(demangled_length));
-  ::ZeroMemory(demangled_cstr, demangled_length);
-  DWORD result = safeUndecorateName(M, demangled_cstr, demangled_length);
+  char *demangled_cstr = llvm::microsoftDemangle(
+  M, nullptr, nullptr, nullptr,
+  llvm::MSDemangleFlags(llvm::MSDF_NoAccessSpecifier |
+llvm::MSDF_NoCallingConvention |
+llvm::MSDF_NoMemberType));
 
   if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) 
{
 if (demangled_cstr && demangled_cstr[0])
   LLDB_LOGF(log, "demangled msvc: %s -> \"%s\"", M, demangled_cstr);
 else
-  LLDB_LOGF(log, "demangled msvc: %s -> error: 0x%lu", M, result);
+  LLDB_LOGF(log, "demangled msvc: %s -> error", M);
   }
 
-  if (result != 0) {
-return demangled_cstr;
-  } else {
-::free(demangled_cstr);
-return nullptr;
-  }
-#else
-  return nullptr;
-#endif
+  return demangled_cstr;
 }
 
 static char *GetItaniumDemangledStr(const char *M) {


___
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 added a comment.

In D69014#1711400 , @jingham wrote:

> IIUC, the only external change in this patch is that before if you 
> implemented your Python command using a class with an `__call__` method, it 
> would have to be the signature that took exe_ctx.   Since this is now 
> switching off of the number of arguments (less self) you could make an 
> `__call__` form that didn't take the extra argument.  I certainly want to 
> discourage that, since the form without the exe_ctx will do the wrong thing 
> in some contexts (e.g. if the command is used in breakpoint callbacks or 
> stop-hooks).  It looks like that would be easy to prevent, you know that 
> you've found __call__ right above where you check.  So I think it would be 
> better to explicitly disallow __call__forms that don't take the exe_ctx.
>
> This is different from the extra_args cases in the breakpoint callbacks and 
> thread plans and so forth.  In those cases, if you don't need extra_args, 
> there's no good reason why you should have to have them.  But in the case of 
> Python commands, it's actually more correct to get the state you are 
> operating on from the exe_ctx.  Otherwise you have to fall back on the 
> currently selected process/thread/frame/etc, and there are contexts (like 
> when you have multiple targets all hitting breakpoints simultaneously) where 
> it does not make sense to have the currently selected state track what 
> process/thread/frame are pertinent to the operation of a breakpoint command 
> or stop hook.  So I do want to gently discourage this form of command.


Right now what happens on trunk is that commands implemented with `__call__` 
just don't work at all on any version of python with either 4 or 5 arguments.

If you look at my latest update, you can see I fix a bug in the old 
implementation `GetNumArguments` where it looks for `im_self` on the original 
object, not the `__call__`.   I fixed it to look at the `__call__` attribute, 
but that is not even entirely correct either, because the python data model 
states that `__call__` is one of the magic methods that must be set on the 
//class//, and cannot be overridden for individual objects.   Oh wait, no, that 
was the python 2.7 data model, it looks like they changed it in 3 so `foo()` 
really is synonymous with `foo.__call__()`

My point here is that python callables are complicated.   They are code.   
Their implementation is liable to change, both because Python itself will 
change, and because users will change what they put in them.I understand 
the backwards-compatibility argument for why we should inspect them, but even 
so, we should inspect them //as little as possible//.  Ideally the only 
inspection we'll ever do is to ask "can this callable take  N arguments?".   
And when we ask that question we should ask it in the documented way, not by 
peeking inside method wrappers and python code objects and trying to understand 
them.


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] D68134: [LLDB] Use the llvm microsoft demangler instead of the windows dbghelp api

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG13993a6f8681: [LLDB] Use the llvm microsoft demangler 
instead of the windows dbghelp api. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68134

Files:
  lldb/source/Core/Mangled.cpp


Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -8,13 +8,6 @@
 
 #include "lldb/Core/Mangled.h"
 
-#if defined(_WIN32)
-#include "lldb/Host/windows/windows.h"
-
-#include 
-#pragma comment(lib, "dbghelp.lib")
-#endif
-
 #include "lldb/Core/RichManglingContext.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
@@ -39,25 +32,6 @@
 #include 
 using namespace lldb_private;
 
-#if defined(_MSC_VER)
-static DWORD safeUndecorateName(const char *Mangled, char *Demangled,
-DWORD DemangledLength) {
-  static std::mutex M;
-  std::lock_guard Lock(M);
-  return ::UnDecorateSymbolName(
-  Mangled, Demangled, DemangledLength,
-  UNDNAME_NO_ACCESS_SPECIFIERS |   // Strip public, private, protected
-   // keywords
-  UNDNAME_NO_ALLOCATION_LANGUAGE | // Strip __thiscall, __stdcall,
-   // etc keywords
-  UNDNAME_NO_THROW_SIGNATURES |// Strip throw() specifications
-  UNDNAME_NO_MEMBER_TYPE | // Strip virtual, static, etc
-   // specifiers
-  UNDNAME_NO_MS_KEYWORDS   // Strip all MS extension keywords
-  );
-}
-#endif
-
 static inline Mangled::ManglingScheme cstring_mangling_scheme(const char *s) {
   if (s) {
 if (s[0] == '?')
@@ -200,28 +174,20 @@
 
 // Local helpers for different demangling implementations.
 static char *GetMSVCDemangledStr(const char *M) {
-#if defined(_MSC_VER)
-  const size_t demangled_length = 2048;
-  char *demangled_cstr = static_cast(::malloc(demangled_length));
-  ::ZeroMemory(demangled_cstr, demangled_length);
-  DWORD result = safeUndecorateName(M, demangled_cstr, demangled_length);
+  char *demangled_cstr = llvm::microsoftDemangle(
+  M, nullptr, nullptr, nullptr,
+  llvm::MSDemangleFlags(llvm::MSDF_NoAccessSpecifier |
+llvm::MSDF_NoCallingConvention |
+llvm::MSDF_NoMemberType));
 
   if (Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DEMANGLE)) 
{
 if (demangled_cstr && demangled_cstr[0])
   LLDB_LOGF(log, "demangled msvc: %s -> \"%s\"", M, demangled_cstr);
 else
-  LLDB_LOGF(log, "demangled msvc: %s -> error: 0x%lu", M, result);
+  LLDB_LOGF(log, "demangled msvc: %s -> error", M);
   }
 
-  if (result != 0) {
-return demangled_cstr;
-  } else {
-::free(demangled_cstr);
-return nullptr;
-  }
-#else
-  return nullptr;
-#endif
+  return demangled_cstr;
 }
 
 static char *GetItaniumDemangledStr(const char *M) {


Index: lldb/source/Core/Mangled.cpp
===
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -8,13 +8,6 @@
 
 #include "lldb/Core/Mangled.h"
 
-#if defined(_WIN32)
-#include "lldb/Host/windows/windows.h"
-
-#include 
-#pragma comment(lib, "dbghelp.lib")
-#endif
-
 #include "lldb/Core/RichManglingContext.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
@@ -39,25 +32,6 @@
 #include 
 using namespace lldb_private;
 
-#if defined(_MSC_VER)
-static DWORD safeUndecorateName(const char *Mangled, char *Demangled,
-DWORD DemangledLength) {
-  static std::mutex M;
-  std::lock_guard Lock(M);
-  return ::UnDecorateSymbolName(
-  Mangled, Demangled, DemangledLength,
-  UNDNAME_NO_ACCESS_SPECIFIERS |   // Strip public, private, protected
-   // keywords
-  UNDNAME_NO_ALLOCATION_LANGUAGE | // Strip __thiscall, __stdcall,
-   // etc keywords
-  UNDNAME_NO_THROW_SIGNATURES |// Strip throw() specifications
-  UNDNAME_NO_MEMBER_TYPE | // Strip virtual, static, etc
-   // specifiers
-  UNDNAME_NO_MS_KEYWORDS   // Strip all MS extension keywords
-  );
-}
-#endif
-
 static inline Mangled::ManglingScheme cstring_mangling_scheme(const char *s) {
   if (s) {
 if (s[0] == '?')
@@ -200,28 +174,20 @@
 
 // Local helpers for different demangling implementations.
 static char *GetMSVCDemangledStr(const char *M) {
-#if defined(_MSC_VER)
-  const size_t demangled_length = 2048;
-  char *demangled_cstr = static_cast(::malloc(demangled_length));
-  ::ZeroMemory(demangled_cstr, demangled_length);
-  DWORD result = safeUndecorateName(M, demangled_

[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 added a comment.

fixed


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] 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 225297.
lawrence_danna added a comment.

comment


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,15 +667,27 @@
 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);
   }
 
   {
 const char *script = "class Foo: \n"
  "  def bar(self, x):\n"
  " return x \n"
+ "  @classmethod \n"
+ "  def classbar(cls, x):\n"
+ " return x \n"
+ "  @staticmethod \n"
+ "  def staticbar(x):\n"
+ " return x \n"
+ "  def __call__(self, x): \n"
+ " return x \n"
+ "obj = Foo() \n"
  "bar_bound   = Foo().bar \n"
+ "bar_class   = Foo().classbar \n"
+ "bar_static  = Foo().staticbar \n"
  "bar_unbound = Foo.bar \n";
 PyObject *o =
 PyRun_String(script, Py_file_input, globals.get(), globals.get());
@@ -687,16 +699,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
@@ -710,9 +743,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, false);
-EXPECT_EQ(arginfo.get().is_bound_method, f

[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 225296.
lawrence_danna marked 3 inline comments as done.
lawrence_danna added a comment.

review fixes


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,15 +667,27 @@
 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);
   }
 
   {
 const char *script = "class Foo: \n"
  "  def bar(self, x):\n"
  " return x \n"
+ "  @classmethod \n"
+ "  def classbar(cls, x):\n"
+ " return x \n"
+ "  @staticmethod \n"
+ "  def staticbar(x):\n"
+ " return x \n"
+ "  def __call__(self, x): \n"
+ " return x \n"
+ "obj = Foo() \n"
  "bar_bound   = Foo().bar \n"
+ "bar_class   = Foo().classbar \n"
+ "bar_static  = Foo().staticbar \n"
  "bar_unbound = Foo.bar \n";
 PyObject *o =
 PyRun_String(script, Py_file_input, globals.get(), globals.get());
@@ -687,16 +699,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
@@ -710,9 +743,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, f

[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 225302.
lawrence_danna marked 8 inline comments as done.
lawrence_danna added a comment.

review fixes


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,92 @@
 }
   }
 }
+
+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,*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);
+  }
+
+  {
+const char *script = "class Foo: \n"
+ "  def bar(self, x):\n"
+ " return x \n"
+ "bar_bound   = Foo().bar \n"
+ "bar_unbound = Foo.bar \n";
+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/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -309,20 +309,35 @@
   static llvm::Error exception(const char *s = nullptr) {
 return llvm::make_error(s);
   }
+  static llvm::Error keyError() {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "key not in dict");
+  }
+
+#if PY_MAJOR_VERSION < 3
+  static char *py2_const_cast(const char *s) { return const_cast(s); }
+#else
+  static const char *py2_const_cast(const

[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 225303.
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,15 +667,27 @@
 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);
   }
 
   {
 const char *script = "class Foo: \n"
  "  def bar(self, x):\n"
  " return x \n"
+ "  @classmethod \n"
+ "  def classbar(cls, x):\n"
+ " return x \n"
+ "  @staticmethod \n"
+ "  def staticbar(x):\n"
+ " return x \n"
+ "  def __call__(self, x): \n"
+ " return x \n"
+ "obj = Foo() \n"
  "bar_bound   = Foo().bar \n"
+ "bar_class   = Foo().classbar \n"
+ "bar_static  = Foo().staticbar \n"
  "bar_unbound = Foo.bar \n";
 PyObject *o =
 PyRun_String(script, Py_file_input, globals.get(), globals.get());
@@ -687,16 +699,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
@@ -710,9 +743,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, false);
-EXPECT_EQ(arginfo.get().is_bound_method, f

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

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

lawrence_danna added inline comments.



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

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')
```



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);

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.



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);

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.


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 added a comment.

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.


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 added a comment.

Also,  @jingham, only the `#if` branch for python2 has easy access to the 
`__call__` attribute.   We could test for it in python3 but it wouldn't do what 
you're thinking it would:

  In [1]: def f(): 
 ...: return 1 
 ...:   

   
  
  In [2]: f.__call__

   
  Out[2]: 
  
  In [3]: type(f).__call__  

   
  Out[3]: 




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

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

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

fixup: VAR_KEYWORD


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,117 @@
 }
   }
 }
+
+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().max_positional_args, 2u);
+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().max_positional_args,
+  PythonCallable::ArgInfo::UNBOUNDED);
+EXPECT_EQ(arginfo.get().has_varargs, true);
+  }
+
+  {
+const char *script = "class Foo: \n"
+ "  def bar(self, x):\n"
+ " return x \n"
+ "bar_bound   = Foo().bar \n"
+ "bar_unbound = Foo.bar \n";
+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());
+   

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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

As Greg said, iOS (and macOS as well, though less directly) have the notion of 
bundleID.  At present, lldb doesn't directly use/figure out the bundle ID, 
though it could either from the binary itself or from debugserver, which does 
have to know that.  As far as I know we always also have the path, the bundle 
ID is inferred from the App package the binary path points to.  So that 
complication should not be present on iOS.

Anyway, I don't see any problem with fetching and displaying the bundleID.

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.


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] r375060 - [Reproducer] Add LoadBuffer<> helper (NFC)

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct 16 17:01:57 2019
New Revision: 375060

URL: http://llvm.org/viewvc/llvm-project?rev=375060&view=rev
Log:
[Reproducer] Add LoadBuffer<> helper (NFC)

Introduce a helper method named LoadBuffer in the Loader to abstract
reading a reproducer file from disk.

Modified:
lldb/trunk/include/lldb/Utility/Reproducer.h
lldb/trunk/source/Commands/CommandObjectReproducer.cpp

Modified: lldb/trunk/include/lldb/Utility/Reproducer.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Reproducer.h?rev=375060&r1=375059&r2=375060&view=diff
==
--- lldb/trunk/include/lldb/Utility/Reproducer.h (original)
+++ lldb/trunk/include/lldb/Utility/Reproducer.h Wed Oct 16 17:01:57 2019
@@ -302,6 +302,15 @@ public:
 return GetRoot().CopyByAppendingPathComponent(T::file);
   }
 
+  template  llvm::Expected LoadBuffer() {
+FileSpec file = GetFile();
+llvm::ErrorOr> buffer =
+llvm::vfs::getRealFileSystem()->getBufferForFile(file.GetPath());
+if (!buffer)
+  return llvm::errorCodeToError(buffer.getError());
+return (*buffer)->getBuffer().str();
+  }
+
   llvm::Error LoadIndex();
 
   const FileSpec &GetRoot() const { return m_root; }

Modified: lldb/trunk/source/Commands/CommandObjectReproducer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectReproducer.cpp?rev=375060&r1=375059&r2=375060&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectReproducer.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectReproducer.cpp Wed Oct 16 17:01:57 
2019
@@ -265,19 +265,12 @@ protected:
   return true;
 }
 case eReproducerProviderVersion: {
-  FileSpec version_file = loader->GetFile();
-
-  // Load the version info into a buffer.
-  ErrorOr> buffer =
-  vfs::getRealFileSystem()->getBufferForFile(version_file.GetPath());
-  if (!buffer) {
-SetError(result, errorCodeToError(buffer.getError()));
+  Expected version = loader->LoadBuffer();
+  if (!version) {
+SetError(result, version.takeError());
 return false;
   }
-
-  // Return the version string.
-  StringRef version = (*buffer)->getBuffer();
-  result.AppendMessage(version.str());
+  result.AppendMessage(*version);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return true;
 }


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

[Lldb-commits] [lldb] r375059 - [Reproducer] Capture the debugger's working directory

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct 16 17:01:53 2019
New Revision: 375059

URL: http://llvm.org/viewvc/llvm-project?rev=375059&view=rev
Log:
[Reproducer] Capture the debugger's working directory

This patch extends the reproducer to capture the debugger's current
working directory. This information will be used later to set the
current working directory of the VFS.

Added:
lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test
Modified:
lldb/trunk/include/lldb/Utility/Reproducer.h
lldb/trunk/source/Utility/Reproducer.cpp

Modified: lldb/trunk/include/lldb/Utility/Reproducer.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Reproducer.h?rev=375059&r1=375058&r2=375059&view=diff
==
--- lldb/trunk/include/lldb/Utility/Reproducer.h (original)
+++ lldb/trunk/include/lldb/Utility/Reproducer.h Wed Oct 16 17:01:53 2019
@@ -132,6 +132,27 @@ public:
   static char ID;
 };
 
+/// Provider for the LLDB current working directroy.
+///
+/// When the reproducer is kept, it writes lldb's current working directory to
+/// a file named cwd.txt in the reproducer root.
+class WorkingDirectoryProvider : public Provider {
+public:
+  WorkingDirectoryProvider(const FileSpec &directory) : Provider(directory) {
+llvm::SmallString<128> cwd;
+if (std::error_code EC = llvm::sys::fs::current_path(cwd))
+  return;
+m_cwd = cwd.str();
+  }
+  struct Info {
+static const char *name;
+static const char *file;
+  };
+  void Keep() override;
+  std::string m_cwd;
+  static char ID;
+};
+
 class DataRecorder {
 public:
   DataRecorder(const FileSpec &filename, std::error_code &ec)

Modified: lldb/trunk/source/Utility/Reproducer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Reproducer.cpp?rev=375059&r1=375058&r2=375059&view=diff
==
--- lldb/trunk/source/Utility/Reproducer.cpp (original)
+++ lldb/trunk/source/Utility/Reproducer.cpp Wed Oct 16 17:01:53 2019
@@ -144,7 +144,9 @@ static FileSpec MakeAbsolute(FileSpec fi
 }
 
 Generator::Generator(FileSpec root)
-: m_root(MakeAbsolute(std::move(root))), m_done(false) {}
+: m_root(MakeAbsolute(std::move(root))), m_done(false) {
+  GetOrCreate();
+}
 
 Generator::~Generator() {}
 
@@ -281,6 +283,15 @@ void VersionProvider::Keep() {
   os << m_version << "\n";
 }
 
+void WorkingDirectoryProvider::Keep() {
+  FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file);
+  std::error_code ec;
+  llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text);
+  if (ec)
+return;
+  os << m_cwd << "\n";
+}
+
 llvm::raw_ostream *ProcessGDBRemoteProvider::GetHistoryStream() {
   FileSpec history_file = GetRoot().CopyByAppendingPathComponent(Info::file);
 
@@ -330,6 +341,7 @@ char FileProvider::ID = 0;
 char ProcessGDBRemoteProvider::ID = 0;
 char ProviderBase::ID = 0;
 char VersionProvider::ID = 0;
+char WorkingDirectoryProvider::ID = 0;
 const char *CommandProvider::Info::file = "command-interpreter.yaml";
 const char *CommandProvider::Info::name = "command-interpreter";
 const char *FileProvider::Info::file = "files.yaml";
@@ -338,3 +350,5 @@ const char *ProcessGDBRemoteProvider::In
 const char *ProcessGDBRemoteProvider::Info::name = "gdb-remote";
 const char *VersionProvider::Info::file = "version.txt";
 const char *VersionProvider::Info::name = "version";
+const char *WorkingDirectoryProvider::Info::file = "cwd.txt";
+const char *WorkingDirectoryProvider::Info::name = "cwd";

Added: lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test?rev=375059&view=auto
==
--- lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test (added)
+++ lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test Wed Oct 16 17:01:53 
2019
@@ -0,0 +1,11 @@
+# This tests relative capture paths.
+
+# RUN: echo "CHECK: %t" > %t.check
+
+# RUN: rm -rf %t.repro
+# RUN: mkdir -p %t.repro
+# RUN: mkdir -p %t
+# RUN: cd %t
+# RUN: %clang %S/Inputs/simple.c -g -o %t/reproducer.out
+# RUN: %lldb -x -b -s %S/Inputs/FileCapture.in --capture --capture-path 
%t.repro %t/reproducer.out
+# RUN: cat %t.repro/cwd.txt | FileCheck %t.check


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

[Lldb-commits] [lldb] r375061 - [Reproducer] Support dumping the reproducer CWD

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct 16 17:02:00 2019
New Revision: 375061

URL: http://llvm.org/viewvc/llvm-project?rev=375061&view=rev
Log:
[Reproducer] Support dumping the reproducer CWD

Add support for dumping the current working directory with
`reproducer dump -p cwd`.

Added:
lldb/trunk/test/Shell/Reproducer/Inputs/WorkingDir.in
Modified:
lldb/trunk/source/Commands/CommandObjectReproducer.cpp
lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test

Modified: lldb/trunk/source/Commands/CommandObjectReproducer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectReproducer.cpp?rev=375061&r1=375060&r2=375061&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectReproducer.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectReproducer.cpp Wed Oct 16 17:02:00 
2019
@@ -9,8 +9,8 @@
 #include "CommandObjectReproducer.h"
 
 #include "lldb/Host/OptionParser.h"
-#include "lldb/Utility/Reproducer.h"
 #include "lldb/Utility/GDBRemote.h"
+#include "lldb/Utility/Reproducer.h"
 
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -27,6 +27,7 @@ enum ReproducerProvider {
   eReproducerProviderFiles,
   eReproducerProviderGDB,
   eReproducerProviderVersion,
+  eReproducerProviderWorkingDirectory,
   eReproducerProviderNone
 };
 
@@ -52,6 +53,11 @@ static constexpr OptionEnumValueElement
 "Version",
 },
 {
+eReproducerProviderWorkingDirectory,
+"cwd",
+"Working Directory",
+},
+{
 eReproducerProviderNone,
 "none",
 "None",
@@ -274,6 +280,17 @@ protected:
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return true;
 }
+case eReproducerProviderWorkingDirectory: {
+  Expected cwd =
+  loader->LoadBuffer();
+  if (!cwd) {
+SetError(result, cwd.takeError());
+return false;
+  }
+  result.AppendMessage(*cwd);
+  result.SetStatus(eReturnStatusSuccessFinishResult);
+  return true;
+}
 case eReproducerProviderCommands: {
   // Create a new command loader.
   std::unique_ptr command_loader =
@@ -320,7 +337,7 @@ protected:
 return false;
   }
 
-  for (GDBRemotePacket& packet : packets) {
+  for (GDBRemotePacket &packet : packets) {
 packet.Dump(result.GetOutputStream());
   }
 

Added: lldb/trunk/test/Shell/Reproducer/Inputs/WorkingDir.in
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/Reproducer/Inputs/WorkingDir.in?rev=375061&view=auto
==
--- lldb/trunk/test/Shell/Reproducer/Inputs/WorkingDir.in (added)
+++ lldb/trunk/test/Shell/Reproducer/Inputs/WorkingDir.in Wed Oct 16 17:02:00 
2019
@@ -0,0 +1,4 @@
+run
+reproducer status
+reproducer dump -p cwd
+reproducer generate

Modified: lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test?rev=375061&r1=375060&r2=375061&view=diff
==
--- lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test (original)
+++ lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test Wed Oct 16 17:02:00 
2019
@@ -7,5 +7,7 @@
 # RUN: mkdir -p %t
 # RUN: cd %t
 # RUN: %clang %S/Inputs/simple.c -g -o %t/reproducer.out
-# RUN: %lldb -x -b -s %S/Inputs/FileCapture.in --capture --capture-path 
%t.repro %t/reproducer.out
+# RUN: %lldb -x -b -s %S/Inputs/WorkingDir.in --capture --capture-path 
%t.repro %t/reproducer.out
+
 # RUN: cat %t.repro/cwd.txt | FileCheck %t.check
+# RUN: %lldb --replay %t.repro | FileCheck %t.check


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

[Lldb-commits] [lldb] r375062 - Revert "make ConstString allocate memory in non-tiny chunks"

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Wed Oct 16 17:02:32 2019
New Revision: 375062

URL: http://llvm.org/viewvc/llvm-project?rev=375062&view=rev
Log:
Revert "make ConstString allocate memory in non-tiny chunks"

As discussed in https://reviews.llvm.org/D68549, the actual issue
here seems to be that the BumpPtrAllocator is growing far too slow
because of the 256 different StringPools used as the backend for ConstString.
At the same time the original patch made ConstString allocate memory in
256MiB slabs for the same reason, meaning that the RSS usage of LLDB increased
by a few hundred MiB for all users without bringing any noticeable speedup
for most of them.

Modified:
lldb/trunk/source/Utility/ConstString.cpp

Modified: lldb/trunk/source/Utility/ConstString.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ConstString.cpp?rev=375062&r1=375061&r2=375062&view=diff
==
--- lldb/trunk/source/Utility/ConstString.cpp (original)
+++ lldb/trunk/source/Utility/ConstString.cpp Wed Oct 16 17:02:32 2019
@@ -31,10 +31,7 @@ using namespace lldb_private;
 class Pool {
 public:
   typedef const char *StringPoolValueType;
-  // BumpPtrAllocator allocates in 4KiB chunks, any larger C++ project is going
-  // to have megabytes of symbols, so allocate in larger chunks.
-  typedef llvm::BumpPtrAllocatorImpl Allocator;
-  typedef llvm::StringMap
+  typedef llvm::StringMap
   StringPool;
   typedef llvm::StringMapEntry StringPoolEntryType;
 
@@ -155,9 +152,7 @@ protected:
 
   struct PoolEntry {
 mutable llvm::sys::SmartRWMutex m_mutex;
-// StringMap by default starts with 16 buckets, any larger project is
-// going to have many symbols, so start with a larger value.
-StringPool m_string_map = StringPool( 65536 );
+StringPool m_string_map;
   };
 
   std::array m_string_pools;


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

[Lldb-commits] [lldb] r375064 - [Reproducer] Set the working directory in the VFS

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct 16 17:24:37 2019
New Revision: 375064

URL: http://llvm.org/viewvc/llvm-project?rev=375064&view=rev
Log:
[Reproducer] Set the working directory in the VFS

Now that the VFS knows how to deal with virtual working directories, we
can set the current working directory to the one we recorded during
reproducer capture. This ensures that relative paths are resolved
correctly during replay.

Modified:
lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test

Modified: lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Initialization/SystemInitializerCommon.cpp?rev=375064&r1=375063&r2=375064&view=diff
==
--- lldb/trunk/source/Initialization/SystemInitializerCommon.cpp (original)
+++ lldb/trunk/source/Initialization/SystemInitializerCommon.cpp Wed Oct 16 
17:24:37 2019
@@ -78,6 +78,13 @@ llvm::Error SystemInitializerCommon::Ini
 } else {
   FileSystem::Initialize();
 }
+if (llvm::Expected cwd =
+loader->LoadBuffer()) {
+  
FileSystem::Instance().GetVirtualFileSystem()->setCurrentWorkingDirectory(
+  *cwd);
+} else {
+  return cwd.takeError();
+}
   } else if (repro::Generator *g = r.GetGenerator()) {
 repro::VersionProvider &vp = g->GetOrCreate();
 vp.SetVersion(lldb_private::GetVersion());

Modified: lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test?rev=375064&r1=375063&r2=375064&view=diff
==
--- lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test (original)
+++ lldb/trunk/test/Shell/Reproducer/TestWorkingDir.test Wed Oct 16 17:24:37 
2019
@@ -1,13 +1,17 @@
-# This tests relative capture paths.
+# This tests that the reproducer can deal with relative files. We create a
+# binary in a subdirectory and pass its relative path to LLDB. The subdirectory
+# is removed before replay so that it only exists in the reproducer's VFS.
 
 # RUN: echo "CHECK: %t" > %t.check
 
 # RUN: rm -rf %t.repro
 # RUN: mkdir -p %t.repro
 # RUN: mkdir -p %t
+# RUN: mkdir -p %t/binary
 # RUN: cd %t
-# RUN: %clang %S/Inputs/simple.c -g -o %t/reproducer.out
-# RUN: %lldb -x -b -s %S/Inputs/WorkingDir.in --capture --capture-path 
%t.repro %t/reproducer.out
+# RUN: %clang %S/Inputs/simple.c -g -o binary/reproducer.out
+# RUN: %lldb -x -b -s %S/Inputs/WorkingDir.in --capture --capture-path 
%t.repro binary/reproducer.out
+# RUN: rm -rf %t/binary
 
 # RUN: cat %t.repro/cwd.txt | FileCheck %t.check
 # RUN: %lldb --replay %t.repro | FileCheck %t.check


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

[Lldb-commits] [lldb] r375068 - [CMake] Make it possible to set the RPATH in add_lldb_exectable.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct 16 17:50:39 2019
New Revision: 375068

URL: http://llvm.org/viewvc/llvm-project?rev=375068&view=rev
Log:
[CMake] Make it possible to set the RPATH in add_lldb_exectable.

Make it possible to pass a build and install RPATH to
add_lldb_executable instead of having to call lldb_setup_rpaths after
the fact.

This fixes a real issue where setting an install RPATH with
lldb_setup_rpaths would only affect the symroot installation component.
Given that lldb_setup_rpaths sets a target property I would expect this
to be orthogonal to installation components. Regardless, it makes sense
to integrate this functionality in add_lldb_exectable.

Modified:
lldb/trunk/cmake/modules/AddLLDB.cmake

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=375068&r1=375067&r2=375068&view=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Wed Oct 16 17:50:39 2019
@@ -148,7 +148,7 @@ function(add_lldb_executable name)
   cmake_parse_arguments(ARG
 "GENERATE_INSTALL"
 "INSTALL_PREFIX;ENTITLEMENTS"
-"LINK_LIBS;CLANG_LIBS;LINK_COMPONENTS"
+"LINK_LIBS;CLANG_LIBS;LINK_COMPONENTS;BUILD_RPATH;INSTALL_RPATH"
 ${ARGN}
 )
 
@@ -175,13 +175,26 @@ function(add_lldb_executable name)
   endif()
   set_target_properties(${name} PROPERTIES FOLDER "lldb executables")
 
+  if (ARG_BUILD_RPATH)
+set_target_properties(${name} PROPERTIES BUILD_RPATH "${ARG_BUILD_RPATH}")
+  endif()
+
+  if (ARG_INSTALL_RPATH)
+set_target_properties(${name} PROPERTIES
+  BUILD_WITH_INSTALL_RPATH OFF
+  INSTALL_RPATH "${ARG_INSTALL_RPATH}")
+  endif()
+
   if(ARG_GENERATE_INSTALL)
 set(install_dest bin)
 if(ARG_INSTALL_PREFIX)
   set(install_dest ${ARG_INSTALL_PREFIX})
 endif()
 install(TARGETS ${name} COMPONENT ${name}
-RUNTIME DESTINATION ${install_dest})
+RUNTIME DESTINATION ${install_dest}
+LIBRARY DESTINATION ${install_dest}
+BUNDLE DESTINATION ${install_dest}
+FRAMEWORK DESTINATION ${install_dest})
 if (NOT CMAKE_CONFIGURATION_TYPES)
   add_llvm_install_targets(install-${name}
DEPENDS ${name}


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

[Lldb-commits] [lldb] r375072 - SBCommandReturnObject: change LLDB_RECORD_METHOD(..., FILE *, ...) to use LLDB_RECORD_DUMMY

[Fangrui Song via lldb-commits]

Author: maskray
Date: Wed Oct 16 18:28:07 2019
New Revision: 375072

URL: http://llvm.org/viewvc/llvm-project?rev=375072&view=rev
Log:
SBCommandReturnObject: change LLDB_RECORD_METHOD(..., FILE *, ...) to use 
LLDB_RECORD_DUMMY

POSIX says FILE is a typedef to a structure containing information about
a file. The structure is unspecified, i.e. it may be an incomplete type, as is 
the case on musl
(`struct _IO_FILE` is an implementation detail that is not exposed).

`LLDB_RECORD_METHOD(..., (FILE *), ...)` transitively uses sizeof(FILE)
and requires the structure to be complete.  Change it to
LLDB_RECORD_DUMMY to fix the build failure on musl (regression of
D57475).

Reviewed By: JDevlieghere, labath, lawrence_danna

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

Modified:
lldb/trunk/source/API/SBCommandReturnObject.cpp

Modified: lldb/trunk/source/API/SBCommandReturnObject.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBCommandReturnObject.cpp?rev=375072&r1=375071&r2=375072&view=diff
==
--- lldb/trunk/source/API/SBCommandReturnObject.cpp (original)
+++ lldb/trunk/source/API/SBCommandReturnObject.cpp Wed Oct 16 18:28:07 2019
@@ -116,7 +116,7 @@ size_t SBCommandReturnObject::GetErrorSi
 }
 
 size_t SBCommandReturnObject::PutOutput(FILE *fh) {
-  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
+  LLDB_RECORD_DUMMY(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
   if (fh) {
 size_t num_bytes = GetOutputSize();
 if (num_bytes)
@@ -141,8 +141,7 @@ size_t SBCommandReturnObject::PutOutput(
 }
 
 size_t SBCommandReturnObject::PutError(FILE *fh) {
-  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutError, (FILE *), fh);
-
+  LLDB_RECORD_DUMMY(size_t, SBCommandReturnObject, PutError, (FILE *), fh);
   if (fh) {
 size_t num_bytes = GetErrorSize();
 if (num_bytes)
@@ -255,31 +254,31 @@ bool SBCommandReturnObject::GetDescripti
 }
 
 void SBCommandReturnObject::SetImmediateOutputFile(FILE *fh) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateOutputFile,
- (FILE *), fh);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateOutputFile,
+(FILE *), fh);
 
   SetImmediateOutputFile(fh, false);
 }
 
 void SBCommandReturnObject::SetImmediateErrorFile(FILE *fh) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateErrorFile,
- (FILE *), fh);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateErrorFile,
+(FILE *), fh);
 
   SetImmediateErrorFile(fh, false);
 }
 
 void SBCommandReturnObject::SetImmediateOutputFile(FILE *fh,
bool transfer_ownership) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateOutputFile,
- (FILE *, bool), fh, transfer_ownership);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateOutputFile,
+(FILE *, bool), fh, transfer_ownership);
   FileSP file = std::make_shared(fh, transfer_ownership);
   ref().SetImmediateOutputFile(file);
 }
 
 void SBCommandReturnObject::SetImmediateErrorFile(FILE *fh,
   bool transfer_ownership) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateErrorFile,
- (FILE *, bool), fh, transfer_ownership);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateErrorFile,
+(FILE *, bool), fh, transfer_ownership);
   FileSP file = std::make_shared(fh, transfer_ownership);
   ref().SetImmediateErrorFile(file);
 }


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

[Lldb-commits] [PATCH] D68872: SBCommandReturnObject: change LLDB_RECORD_METHOD(..., FILE *, ...) to use LLDB_RECORD_DUMMY

[Fangrui Song via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG56ee31964f5a: SBCommandReturnObject: change 
LLDB_RECORD_METHOD(..., FILE *, ...) to use… (authored by MaskRay).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68872?vs=225155&id=225338#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68872

Files:
  lldb/source/API/SBCommandReturnObject.cpp


Index: lldb/source/API/SBCommandReturnObject.cpp
===
--- lldb/source/API/SBCommandReturnObject.cpp
+++ lldb/source/API/SBCommandReturnObject.cpp
@@ -116,7 +116,7 @@
 }
 
 size_t SBCommandReturnObject::PutOutput(FILE *fh) {
-  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
+  LLDB_RECORD_DUMMY(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
   if (fh) {
 size_t num_bytes = GetOutputSize();
 if (num_bytes)
@@ -141,8 +141,7 @@
 }
 
 size_t SBCommandReturnObject::PutError(FILE *fh) {
-  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutError, (FILE *), fh);
-
+  LLDB_RECORD_DUMMY(size_t, SBCommandReturnObject, PutError, (FILE *), fh);
   if (fh) {
 size_t num_bytes = GetErrorSize();
 if (num_bytes)
@@ -255,31 +254,31 @@
 }
 
 void SBCommandReturnObject::SetImmediateOutputFile(FILE *fh) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateOutputFile,
- (FILE *), fh);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateOutputFile,
+(FILE *), fh);
 
   SetImmediateOutputFile(fh, false);
 }
 
 void SBCommandReturnObject::SetImmediateErrorFile(FILE *fh) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateErrorFile,
- (FILE *), fh);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateErrorFile,
+(FILE *), fh);
 
   SetImmediateErrorFile(fh, false);
 }
 
 void SBCommandReturnObject::SetImmediateOutputFile(FILE *fh,
bool transfer_ownership) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateOutputFile,
- (FILE *, bool), fh, transfer_ownership);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateOutputFile,
+(FILE *, bool), fh, transfer_ownership);
   FileSP file = std::make_shared(fh, transfer_ownership);
   ref().SetImmediateOutputFile(file);
 }
 
 void SBCommandReturnObject::SetImmediateErrorFile(FILE *fh,
   bool transfer_ownership) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateErrorFile,
- (FILE *, bool), fh, transfer_ownership);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateErrorFile,
+(FILE *, bool), fh, transfer_ownership);
   FileSP file = std::make_shared(fh, transfer_ownership);
   ref().SetImmediateErrorFile(file);
 }


Index: lldb/source/API/SBCommandReturnObject.cpp
===
--- lldb/source/API/SBCommandReturnObject.cpp
+++ lldb/source/API/SBCommandReturnObject.cpp
@@ -116,7 +116,7 @@
 }
 
 size_t SBCommandReturnObject::PutOutput(FILE *fh) {
-  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
+  LLDB_RECORD_DUMMY(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
   if (fh) {
 size_t num_bytes = GetOutputSize();
 if (num_bytes)
@@ -141,8 +141,7 @@
 }
 
 size_t SBCommandReturnObject::PutError(FILE *fh) {
-  LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutError, (FILE *), fh);
-
+  LLDB_RECORD_DUMMY(size_t, SBCommandReturnObject, PutError, (FILE *), fh);
   if (fh) {
 size_t num_bytes = GetErrorSize();
 if (num_bytes)
@@ -255,31 +254,31 @@
 }
 
 void SBCommandReturnObject::SetImmediateOutputFile(FILE *fh) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateOutputFile,
- (FILE *), fh);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateOutputFile,
+(FILE *), fh);
 
   SetImmediateOutputFile(fh, false);
 }
 
 void SBCommandReturnObject::SetImmediateErrorFile(FILE *fh) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateErrorFile,
- (FILE *), fh);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateErrorFile,
+(FILE *), fh);
 
   SetImmediateErrorFile(fh, false);
 }
 
 void SBCommandReturnObject::SetImmediateOutputFile(FILE *fh,
bool transfer_ownership) {
-  LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateOutputFile,
- (FILE *, bool), fh, transfer_ownership);
+  LLDB_RECORD_DUMMY(void, SBCommandReturnObject, SetImmediateOutputFile,
+(FILE *, bool), fh, t

[Lldb-commits] [lldb] r375073 - delete SWIG typemaps for FILE*

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Wed Oct 16 18:35:22 2019
New Revision: 375073

URL: http://llvm.org/viewvc/llvm-project?rev=375073&view=rev
Log:
delete SWIG typemaps for FILE*

Summary:
The SWIG typemaps for FILE* are no longer used, so
this patch deletes them.

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

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

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

Modified: lldb/trunk/include/lldb/Host/File.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/File.h?rev=375073&r1=375072&r2=375073&view=diff
==
--- lldb/trunk/include/lldb/Host/File.h (original)
+++ lldb/trunk/include/lldb/Host/File.h Wed Oct 16 18:35:22 2019
@@ -136,20 +136,6 @@ public:
   /// ENOTSUP, success, or another error.
   virtual Status GetFileSpec(FileSpec &file_spec) const;
 
-  /// DEPRECATED! Extract the underlying FILE* and reset this File without 
closing it.
-  ///
-  /// This is only here to support legacy SB interfaces that need to convert 
scripting
-  /// language objects into FILE* streams.   That conversion is inherently 
sketchy and
-  /// doing so may cause the stream to be leaked.
-  ///
-  /// After calling this the File will be reset to its original state.  It 
will be
-  /// invalid and it will not hold on to any resources.
-  ///
-  /// \return
-  /// The underlying FILE* stream from this File, if one exists and can be 
extracted,
-  /// nullptr otherwise.
-  virtual FILE *TakeStreamAndClear();
-
   /// Get underlying OS file descriptor for this file, or kInvalidDescriptor.
   /// If the descriptor is valid, then it may be used directly for I/O
   /// However, the File may also perform it's own buffering, so avoid using
@@ -413,7 +399,6 @@ public:
   Status Close() override;
   WaitableHandle GetWaitableHandle() override;
   Status GetFileSpec(FileSpec &file_spec) const override;
-  FILE *TakeStreamAndClear() override;
   int GetDescriptor() const override;
   FILE *GetStream() override;
   off_t SeekFromStart(off_t offset, Status *error_ptr = nullptr) override;

Modified: lldb/trunk/scripts/Python/python-typemaps.swig
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/python-typemaps.swig?rev=375073&r1=375072&r2=375073&view=diff
==
--- lldb/trunk/scripts/Python/python-typemaps.swig (original)
+++ lldb/trunk/scripts/Python/python-typemaps.swig Wed Oct 16 18:35:22 2019
@@ -451,74 +451,6 @@ bool SetNumberFromPyObject(doubl
   }
 }
 
-// FIXME both of these paths wind up calling fdopen() with no provision for 
ever calling
-// fclose() on the result.  SB interfaces that use FILE* should be deprecated 
for scripting
-// use and this typemap should eventually be removed.
-%typemap(in) FILE * {
-   using namespace lldb_private;
-   if ($input == Py_None)
-  $1 = nullptr;
-   else if (!lldb_private::PythonFile::Check($input)) {
-  int fd = PyObject_AsFileDescriptor($input);
-  if (fd < 0 || PyErr_Occurred())
-return nullptr;
-  PythonObject py_input(PyRefType::Borrowed, $input);
-  PythonString py_mode = 
py_input.GetAttributeValue("mode").AsType();
-  if (!py_mode.IsValid() || PyErr_Occurred())
-return nullptr;
-FILE *f;
-if ((f = fdopen(fd, py_mode.GetString().str().c_str(
-  $1 = f;
-else {
-  PyErr_SetString(PyExc_TypeError, strerror(errno));
-  return nullptr;
-}
-   }
-   else
-   {
-  PythonFile py_file(PyRefType::Borrowed, $input);
-  lldb::FileUP file = py_file.GetUnderlyingFile();
-  if (!file)
- return nullptr;
-  $1 = file->TakeStreamAndClear();
-}
-}
-
-%typemap(out) FILE * {
-   // TODO: This is gross. We should find a way to fetch the mode flags from 
the
-   // lldb_private::File object.
-   char mode[4] = {0};
-%#ifdef __APPLE__
-   int i = 0;
-   if ($1)
-   {
-   short flags = $1->_flags;
-
-   if (flags & __SRD)
-  mode[i++] = 'r';
-   else if (flags & __SWR)
-  mode[i++] = 'w';
-   else // if (flags & __SRW)
-  mode[i++] = 'a';
-}
-%#else
-   // There's no portable way to get the mode here. We just return a mode which
-   // permits both reads and writes and count on the operating system to return
-   // an error when an invalid operation is attempted.
-   mode[0] = 'r';
-   mode[1] = '+';
-%#endif
-   using namespace lldb_priv

[Lldb-commits] [PATCH] D68963: delete SWIG typemaps for FILE*

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

This revision was automatically updated to reflect the committed changes.
lawrence_danna marked an inline comment as done.
Closed by commit rG0f783599a4c6: delete SWIG typemaps for FILE* (authored by 
lawrence_danna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68963

Files:
  lldb/include/lldb/Host/File.h
  lldb/scripts/Python/python-typemaps.swig
  lldb/source/Host/common/File.cpp
  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
@@ -585,8 +585,9 @@
   auto file = FileSystem::Instance().Open(FileSpec(FileSystem::DEV_NULL),
   File::eOpenOptionRead);
   ASSERT_THAT_EXPECTED(file, llvm::Succeeded());
-  PythonFile py_file(*file.get(), "r");
-  EXPECT_TRUE(PythonFile::Check(py_file.get()));
+  auto py_file = PythonFile::FromFile(*file.get(), "r");
+  ASSERT_THAT_EXPECTED(py_file, llvm::Succeeded());
+  EXPECT_TRUE(PythonFile::Check(py_file.get().get()));
 }
 
 TEST_F(PythonDataObjectsTest, TestObjectAttributes) {
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -651,10 +651,15 @@
 
   PythonDictionary &sys_module_dict = GetSysModuleDictionary();
 
+  auto new_file = PythonFile::FromFile(file, mode);
+  if (!new_file) {
+llvm::consumeError(new_file.takeError());
+return false;
+  }
+
   save_file = sys_module_dict.GetItemForKey(PythonString(py_name));
 
-  PythonFile new_file(file, mode);
-  sys_module_dict.SetItemForKey(PythonString(py_name), new_file);
+  sys_module_dict.SetItemForKey(PythonString(py_name), new_file.get());
   return true;
 }
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -637,20 +637,6 @@
   static llvm::Expected FromFile(File &file,
  const char *mode = nullptr);
 
-  // FIXME delete this after FILE* typemaps are deleted
-  // and ScriptInterpreterPython is fixed
-  PythonFile(File &file, const char *mode = nullptr) {
-auto f = FromFile(file, mode);
-if (f)
-  *this = std::move(f.get());
-else {
-  Reset();
-  llvm::consumeError(f.takeError());
-}
-  }
-
-  lldb::FileUP GetUnderlyingFile() const;
-
   llvm::Expected ConvertToFile(bool borrowed = false);
   llvm::Expected
   ConvertToFileForcingUseOfScriptingIOMethods(bool borrowed = false);
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -837,25 +837,6 @@
 #endif
 }
 
-FileUP PythonFile::GetUnderlyingFile() const {
-  if (!IsValid())
-return nullptr;
-
-  // We don't own the file descriptor returned by this function, make sure the
-  // File object knows about that.
-  PythonString py_mode = GetAttributeValue("mode").AsType();
-  auto options = File::GetOptionsFromMode(py_mode.GetString());
-  if (!options) {
-llvm::consumeError(options.takeError());
-return nullptr;
-  }
-  auto file = std::unique_ptr(new NativeFile(
-  PyObject_AsFileDescriptor(m_py_obj), options.get(), false));
-  if (!file->IsValid())
-return nullptr;
-  return file;
-}
-
 namespace {
 class GIL {
 public:
Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -117,8 +117,6 @@
   return std::error_code(ENOTSUP, std::system_category());
 }
 
-FILE *File::TakeStreamAndClear() { return nullptr; }
-
 int File::GetDescriptor() const { return kInvalidDescriptor; }
 
 FILE *File::GetStream() { return nullptr; }
@@ -331,18 +329,6 @@
   return error;
 }
 
-FILE *NativeFile::TakeStreamAndClear() {
-  FILE *stream = GetStream();
-  m_stream = NULL;
-  m_descriptor = kInvalidDescriptor;
-  m_options = OpenOptions();
-  m_own_stream = false;
-  m_own_descriptor = false;
-  m_is_interactive = m_supports_colo

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

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

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69080

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -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,13 +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


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,13 +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
___
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 225350.
lawrence_danna added a comment.

include move version too


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;
@@ -249,14 +237,10 @@
 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));
 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