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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Sounds reasonable, but you'll also need to update the test in 
ProcessInstanceInfoTest.cpp. Maybe also change/add an entry to that test so 
that it includes this long android triple while you're inside?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68291



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Including the args sounds like a good idea, but I don't think the chosen 
encoding scheme is very good. The encoding done in `GetCommandString` is very 
naive and not reversible. Since you're hex-encoding the result anyway, and the 
nul character cannot be present inside an argument you could use it to delimit 
the individual arguments (i.e. `666f6f00626172` -> `{"foo", "bar"}`). Or, you 
could look at how the `$A` packet transmits arguments, and implement something 
similar.

Also, you should write a test for this. I think it should be possible to run a 
process with known arguments, and then verify that they show up in "platform 
process list" output. Alternatively, the client part of the protocol can also 
be tested via c++ unit tests (see unittests/Process/gdb-remote). The advantage 
of that approach is that you're able to inject invalid packets and test the 
handling of those. In an ideal world, we'd have both kinds of these tests, but 
we're pretty far from that, so I'd settle for at least one of them. :)




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:613-615
+if (SendPacketAndWaitForResponse(
+"jGetLoadedDynamicLibrariesInfos:", response, false) ==
+PacketResult::Success) {

When you're running clang format, please make sure it only formats the lines 
that you have modified. I've you're using the standard git-clang-format 
integration, this should be as simple as `git clang-format HEAD^`. Please 
revert these unrelated changes.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1944-1948
+if (!process_info.GetExecutableFile().GetPath().empty()) {
+  process_info.SetArguments(
+  Args(process_info.GetExecutableFile().GetPath() + args_command),
+  true);
+}

I don't fully understand the what this does. Is it trying to set the executable 
name as argv[0]? Wouldn't it be better to send the argv[0] separately (together 
with the rest of the args array), just in case the process has a special 
argv[0].


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] D68299: [JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationServerLLGS

[Pavel Labath via Phabricator via lldb-commits]

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

Cool. Thanks for doing this.


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

https://reviews.llvm.org/D68299



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

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

[Pavel Labath via Phabricator via lldb-commits]

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

LGTM, with the usual question/caveat.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68304



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

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

[Pavel Labath via Phabricator via lldb-commits]

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

LG, though I'm not sure whether using `using namespace llvm` is such a good 
idea. There's a fair number of classes with identical names in llvm and 
lldb_private namespaces. As we start using more of llvm, we will start hitting 
conflicts. E.g., I've had to handle a bunch of conflicts when I made 
lldb_private::DataExtractor convertible to llvm::DataExtractor (which made the 
second name visible nearly everywhere). For this particular case a namespace 
alias (`namespace json = llvm::json`) might be a better option.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68301



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

[Lldb-commits] [PATCH] D68305: [JSON] Remove Utility/JSON.{h|cpp}

[Pavel Labath via Phabricator via lldb-commits]

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

Yay. This turned to be easier than I expected.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68305



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

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

[Pavel Labath via Phabricator via lldb-commits]

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

LG, with the same caveat about about namespaces.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68302



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

[Lldb-commits] [PATCH] D68312: [gdb-remote] process properly effective uid

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Someone also didn't write a test for that. :) Let's not make the same mistake. 
The same two options I mentioned in D68293  
should apply here too, though for this case, I would slightly prefer the c++ 
unit test route, as it will isolate you from the weirdness of the host 
platforms and their uid handling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68312



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

[Lldb-commits] [PATCH] D68314: [process info] Remove assert in DoGetGroupName

[Pavel Labath via Phabricator via lldb-commits]

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

I am happy to accept this patch as-is, but since you seem to be on a quest to 
improve the process listing I thought I'd mention that `getgrgid_r` seems to be 
available on android since API level 24, and `getgrgid` since pretty much 
forever (I think it may not have been exposed in old ndk releases). So, you 
could massage this code a bit to enable group name resolution on android too 
(but you don't have to).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68314



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/lit/Host/TestCustomShell.test:5
+# RUN: SHELL=bogus %lldb %t.out -b -o 'run' 2>&1 | FileCheck %s
+# CHECK: error: shell expansion failed

JDevlieghere wrote:
> JDevlieghere wrote:
> > friss wrote:
> > > Is there a reliable way to check that the expansion we get in lldb 
> > > matches the one in the shell? For example, could we have the program dump 
> > > its arguments once without lldb and match them against the lldb output?
> > > 
> > > I guess the zsh example that errors out in your description makes this 
> > > hard?  
> > You could run lldb-argdumper under lldb and compare the output to running 
> > it under different shells. Do you think it's reasonable to assume that at 
> > least `/bin/bash` and `/bin/zsh` are available?
> Assuming we can find something that expands differently without throwing an 
> error in bash and zsh.
I don't think you can assume zsh is universally available. If you wanted to do 
something like, the safest option would be to write a mock shell, which 
implements some dumb substitution (s/FOO/BAR/) and otherwise forwards to the 
real shell. But, I wouldn't say that is required, as that is not even testing 
what this patch is changing. A simpler but still interesting test might be to 
unset the SHELL variable to ensure that the getpwuid path is at least executed 
(as one can't really check it's result reasonably).



Comment at: lldb/source/Host/posix/HostInfoPosix.cpp:119
+return FileSpec(v);
+  if (const char *v = ::getpwuid(::geteuid())->pw_shell)
+return FileSpec(v);

friss wrote:
> the dereference seems a little careless. getpwuid can fail and return NULL.
Also, getpwuid is not thread safe. There's already code in this very file which 
chooses between getpwuid and getpwuid_r, but it is hidden inside 
`PosixUserIDResolver::DoGetUserName`. If you factor that out into a helper 
function, you'll be able to always use the best available API.


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

https://reviews.llvm.org/D68316



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

[Lldb-commits] [PATCH] D68289: [lldb-server/android] Show more processes and package name when necessary

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It seems to me that the Host class is too low level to be encoding some of the 
android specifics into it. I think it would be better to handle these things 
higher up. Please see inline comments for details.




Comment at: lldb/source/Host/linux/Host.cpp:178-185
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
+#if defined(__ANDROID__)
+// On android we fallback to Arg0, which commonly is an apk package name.
+if (!process_info.GetArg0().empty()) {
+  ExePath = process_info.GetArg0();

Why does this fail on android exactly? Is it some seccomp thingy? I'm surprised 
that we're allowed to read `/proc/cmdline`, but not the `/proc/exe` link..

Anyway, I don't think that pretending that "com.my.app" is an executable file 
name is a good idea. Maybe it would be better to implement something on the 
other end. I.e., we print out argv[0]  if the file name is not present?



Comment at: lldb/source/Host/linux/Host.cpp:213-218
+#if defined(__ANDROID__)
+  // It's okay if we can't get the environment on android
+  return true;
+#else
+  return false;
+#endif

I think we can just make this non-android specific and say that we're ok with 
returning the results if we were unable to fetch the environment. For instance, 
you could make the caller not fail hard (but just log or something) if this 
returns false.



Comment at: lldb/source/Host/linux/Host.cpp:254-260
+#if defined(__ANDROID__)
+// On android each apk has its own user, so it's better to display all
+// users instead of the current one.
+true;
+#else
+match_info.GetMatchAllUsers();
+#endif

Just from the perspective of the FindProcesses API, I don't think it's 
reasonable for it to unilaterally ignore the explicit request to limit the set 
of processes to return. Ideally, this decision would be made higher up 
(somewhere near the code that actually enables us to attach to processes of 
other users), and communicated here via the `GetMatchAllUsers` flag. However, 
that code doesn't exist yet. Maybe we could drop this bit for now? Or implement 
a flag to the "platform process list" which would allow one to see all 
processes? Disregarding android, it may still be interesting to see all 
processes of some remote system, even if one cannot attach to them. And later, 
once we're actually able to attach to processes of other "users" we can set it 
up so that this flag becomes the default for android targets?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289



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

[Lldb-commits] [PATCH] D68317: factor out an abstract base class for File

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Makes sense to me, just delete the custom make_shared stuff.




Comment at: lldb/include/lldb/Host/File.h:377-381
+  template 
+  static std::shared_ptr make_shared(Args... args) {
+return std::static_pointer_cast(
+std::make_shared(args...));
+  }

Please delete this stuff, and just use regular std::make_shared. (BTW, 
shared_ptr is implicitly convertible to shared_ptr)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68317



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

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

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

LLVM reasoning for why to go with `std::vector`: 
http://llvm.org/docs/ProgrammersManual.html#set-like-containers-std-set-smallset-setvector-etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390



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

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

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: shafik, martong.
Herald added subscribers: lldb-commits, JDevlieghere, abidh, christof, 
rnkovacs, aprantl.
Herald added a project: LLDB.

As we figured out in D67803 , importing 
declarations from a temporary ASTContext that were originally from a persistent 
ASTContext
causes a bunch of duplicated declarations where we end up having declarations 
in the target AST that have no associated ASTImporter that
can complete them.

I haven't figured out how/if we can solve this in the current way we do things 
in LLDB, but in the modern-type-lookup this is solvable
as we have a saner architecture with the ExternalASTMerger. As we can 
(hopefully) make modern-type-lookup the default mode in the future,
I would say we try fixing this issue here. As we don't use the hack that was 
reinstated in D67803  during 
modern-type-lookup, the test case for this
is essentially just printing any kind of container in `std::` as we would 
otherwise run into the issue that required a hack like D67803 
.

What this patch is doing in essence is that instead of importing a declaration 
from a temporary ASTContext, we instead check if the
declaration originally came from a persistent ASTContext (e.g. the debug 
information) and we directly import from there. The ExternalASTMerger
is already connected with ASTImporters to these different sources, so this 
patch is essentially just two parts:

1. Mark our temporary ASTContext/ImporterSource as temporary when we import 
from the expression AST.
2. If the ExternalASTMerger sees we import from the expression AST, instead of 
trying to import these temporary declarations, check if we

can instead import from the persistent ASTContext that is already connected. 
This ensures that all records from the persistent source actually
come from the persistent source and are minimally imported in a way that allows 
them to be completed later on in the target AST.

The next step is to run the ASTImporter for these temporary expressions with 
the MinimalImport mode disabled, but that's a follow up patch.

This patch fixes most test failures with modern-type-lookup enabled by default 
(down to 73 failing tests, which includes the 22 import-std-module tests
which need special treatment).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68326

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

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -180,108 +180,20 @@
   return ret;
 }
 
-namespace {
-/// This class walks an AST and ensures that all DeclContexts defined inside the
-/// current source file are properly complete.
-///
-/// This is used to ensure that persistent types defined in the current source
-/// file migrate completely to the persistent AST context before they are
-/// reused.  If that didn't happen, it would be impoossible to complete them
-/// because their origin would be gone.
-///
-/// The stragtegy used by this class is to check the SourceLocation (to be
-/// specific, the FileID) and see if it's the FileID for the current expression.
-/// Alternate strategies could include checking whether an ExternalASTMerger,
-/// set up to not have the current context as a source, can find an original for
-/// the type.
-class Completer : public clang::RecursiveASTVisitor {
-private:
-  /// Used to import Decl contents
-  clang::ASTImporter &m_exporter;
-  /// The file that's going away
-  clang::FileID m_file;
-  /// Visited Decls, to avoid cycles
-  llvm::DenseSet m_completed;
-
-  bool ImportAndCheckCompletable(clang::Decl *decl) {
-llvm::Expected imported_decl = m_exporter.Import(decl);
-if (!imported_decl) {
-  llvm::consumeError(imported_decl.takeError());
-  return false;
-}
-if (m_completed.count(decl))
-  return false;
-if (!llvm::isa(decl))
-  return false;
-const clang::SourceLocation loc = decl->getLocation();
-if (!loc.isValid())
-  return false;
-const clang::FileID file =
-m_exporter.getFromContext().getSourceManager().getFileID(loc);
-if (file != m_file)
-  return false;
-// We are assuming the Decl was parsed in this very expression, so it
-// should not have external storage.
-lldbassert(!llvm::cast(decl)->hasExternalLexicalStorage());
-return true;
-  }
-

[Lldb-commits] [PATCH] D68278: Fix evaluation of nested classes with parent from other CU

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Just a few nitpicks about some minor typos, otherwise this LGTM. Thanks for the 
patch! I assume you need someone to commit this for you?




Comment at: 
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py:2
+"""
+Test that expression evaluator can access members of nested classes even if
+the parents of the nested classes were imported from another compilation unit.

missing 'the' between 'that' and 'expression evaluator'. Also a missing 'a' 
between 'of' and 'nested'.



Comment at: 
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp:11
+  // WrapperB refers to the Inner and Outer DIEs from the other.cpp CU.
+  // It is important that WrapperB is only foward-declared in shared.h.
+  WrapperB* b = foo();

Nit pick: 'foward' -> 'forward'.


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

https://reviews.llvm.org/D68278



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Hmm... I like your solution of the typemap problem, but this CRTP class seems 
to be way more complicated than needed. There shouldn't be any need for CRTP as 
we already have regular dynamic dispatch via virtual methods. Also, I don't 
think the forwarding should be needed if you're going the template route. How 
about something like this instead?

  template
  class OwningPythonFile : public Base {
Status Close() { /* close magic here*/; }
// no need to forward anything -- inheritance handles that
  };
  
  class TextPythonFile: public OwningPythonFile {
// override methods as usual
  };
  // same for BinaryPythonFile

Then depending on what you need, you create either a NativeFile, 
OwningPythonFile, or a Text/BinaryPythonFile. How does that sound?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188



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

[Lldb-commits] [PATCH] D68278: Fix evaluation of nested classes with parent from other CU

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added inline comments.



Comment at: 
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py:2
+"""
+Test that expression evaluator can access members of nested classes even if
+the parents of the nested classes were imported from another compilation unit.

teemperor wrote:
> missing 'the' between 'that' and 'expression evaluator'. Also a missing 'a' 
> between 'of' and 'nested'.
Fixing "the expression evaluator"; "of a nested classes" does not sound right, 
though.


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

https://reviews.llvm.org/D68278



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

[Lldb-commits] [PATCH] D68278: Fix evaluation of nested classes with parent from other CU

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin updated this revision to Diff 222803.
jarin marked 5 inline comments as done.
jarin added a comment.

Fixed the nits, thanks for the careful review!

I will indeed need someone to submit this for me. Thanks in advance :-)


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

https://reviews.llvm.org/D68278

Files:
  
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/Makefile
  
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
  
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
  
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/other.cpp
  
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/shared.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -690,6 +690,8 @@
 type_sp = unique_ast_entry_up->m_type_sp;
 if (type_sp) {
   dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+  LinkDeclContextToDIE(
+  GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die);
   return type_sp;
 }
   }
Index: packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/shared.h
===
--- /dev/null
+++ packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/shared.h
@@ -0,0 +1,17 @@
+struct OuterX {
+  template
+  struct Inner {
+int oX_inner = 42;
+  };
+};
+
+struct OuterY {
+  template
+  struct Inner {
+typename OuterX::Inner oY_inner;
+  };
+};
+
+struct WrapperB;
+
+WrapperB* foo();
Index: packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/other.cpp
===
--- /dev/null
+++ packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/other.cpp
@@ -0,0 +1,10 @@
+#include "shared.h"
+
+struct WrapperB {
+  OuterY y;
+  OuterX x;
+};
+
+WrapperB* foo() {
+  return new WrapperB();
+}
Index: packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
===
--- /dev/null
+++ packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
@@ -0,0 +1,22 @@
+#include "shared.h"
+
+struct WrapperA {
+  OuterY::Inner y;
+};
+
+int main() {
+  // WrapperA refers to the Inner and Outer class DIEs from this CU.
+  WrapperA a;
+  // WrapperB refers to the Inner and Outer DIEs from the other.cpp CU.
+  // It is important that WrapperB is only forward-declared in shared.h.
+  WrapperB* b = foo();
+
+  // Evaluating 'b' here will parse other.cpp's DIEs for all
+  // the Inner and Outer classes from shared.h.
+  //
+  // Evaluating 'a' here will find and reuse the already-parsed
+  // versions of the Inner and Outer classes. In the associated test
+  // we make sure that we can still resolve all the types properly
+  // by evaluating 'a.y.oY_inner.oX_inner'.
+  return 0;  // break here
+}
Index: packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
===
--- /dev/null
+++ packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
@@ -0,0 +1,29 @@
+"""
+Test that the expression evaluator can access members of nested classes even if
+the parents of the nested classes were imported from another compilation unit.
+"""
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestNestedClassWithParentInAnotherCU(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def test_nested_class_with_parent_in_another_cu(self):
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+self.build()
+(_, _, thread, _) = lldbutil.run_to_source_breakpoint(self, "// break here", self.main_source_file)
+frame = thread.GetSelectedFrame()
+# Parse the DIEs of the parent classes and the nested classes from
+# other.cpp's CU.
+warmup_result = frame.EvaluateExpression("b")
+self.assertTrue(warmup_result.IsValid())
+# Inspect fields of the nested classes. This will reuse the types that
+# were parsed during the evaluation above. By accessing a chain of
+# fields, we try to verify that all the DIEs, reused types and
+# declaration contexts were wired properly into lldb's parser's state.
+expr_result = frame.EvaluateExpression("a.y.oY_inner.oX_inner")
+   

[Lldb-commits] [PATCH] D68278: Fix evaluation of nested classes with parent from other CU

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added inline comments.



Comment at: 
packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py:2
+"""
+Test that expression evaluator can access members of nested classes even if
+the parents of the nested classes were imported from another compilation unit.

jarin wrote:
> teemperor wrote:
> > missing 'the' between 'that' and 'expression evaluator'. Also a missing 'a' 
> > between 'of' and 'nested'.
> Fixing "the expression evaluator"; "of a nested classes" does not sound 
> right, though.
Whoops, right, my bad.


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

https://reviews.llvm.org/D68278



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

[Lldb-commits] [lldb] r373457 - [lldb][NFC] Create the ASTContext in ClangASTContext exactly once.

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Wed Oct  2 05:26:08 2019
New Revision: 373457

URL: http://llvm.org/viewvc/llvm-project?rev=373457&view=rev
Log:
[lldb][NFC] Create the ASTContext in ClangASTContext exactly once.

Reason for this patch is the Ssame reason as for the previous patches:
Having a ClangASTContext and being able to switch the associated ASTContext 
isn't
a use case we have (or should have), so let's simplify all this code.
This way it becomes clearer in what order we initialize data structures.

The DWARFASTParserClangTests changes are necessary as the test is using
a ClangASTContext but relied on the fact that no called function ever calls
getASTContext() on our ClangASTContext (as that would create the ASTContext).
As we now always create the ASTContext the fact that we had an uninitialized
FileSystem made the test crash.

Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/source/Symbol/ClangASTContext.cpp
lldb/trunk/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=373457&r1=373456&r2=373457&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Wed Oct  2 05:26:08 2019
@@ -53,6 +53,7 @@ public:
 
   // Constructors and Destructors
   explicit ClangASTContext(llvm::StringRef triple = "");
+  explicit ClangASTContext(ArchSpec arch);
 
   /// Constructs a ClangASTContext that uses an existing ASTContext internally.
   /// Useful when having an existing ASTContext created by Clang.
@@ -114,10 +115,6 @@ public:
 
   const char *GetTargetTriple();
 
-  void SetTargetTriple(llvm::StringRef target_triple);
-
-  void SetArchitecture(const ArchSpec &arch);
-
   void SetExternalSource(
   llvm::IntrusiveRefCntPtr &ast_source_up);
 
@@ -1008,11 +1005,14 @@ private:
   // For ClangASTContext only
   ClangASTContext(const ClangASTContext &);
   const ClangASTContext &operator=(const ClangASTContext &);
+  /// Creates the internal ASTContext.
+  void CreateASTContext();
+  void SetTargetTriple(llvm::StringRef target_triple);
 };
 
 class ClangASTContextForExpressions : public ClangASTContext {
 public:
-  ClangASTContextForExpressions(Target &target);
+  ClangASTContextForExpressions(Target &target, ArchSpec arch);
 
   ~ClangASTContextForExpressions() override = default;
 

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=373457&r1=373456&r2=373457&view=diff
==
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Wed Oct  2 05:26:08 2019
@@ -526,6 +526,17 @@ ClangASTContext::ClangASTContext(llvm::S
 : TypeSystem(TypeSystem::eKindClang) {
   if (!target_triple.empty())
 SetTargetTriple(target_triple);
+  // The caller didn't pass an ASTContext so create a new one for this
+  // ClangASTContext.
+  CreateASTContext();
+}
+
+ClangASTContext::ClangASTContext(ArchSpec arch)
+: TypeSystem(TypeSystem::eKindClang) {
+  SetTargetTriple(arch.GetTriple().str());
+  // The caller didn't pass an ASTContext so create a new one for this
+  // ClangASTContext.
+  CreateASTContext();
 }
 
 ClangASTContext::ClangASTContext(ASTContext &existing_ctxt)
@@ -575,26 +586,21 @@ lldb::TypeSystemSP ClangASTContext::Crea
   }
 
   if (module) {
-std::shared_ptr ast_sp(new ClangASTContext);
-if (ast_sp) {
-  ast_sp->SetArchitecture(fixed_arch);
-}
+std::shared_ptr ast_sp(
+new ClangASTContext(fixed_arch));
 return ast_sp;
   } else if (target && target->IsValid()) {
 std::shared_ptr ast_sp(
-new ClangASTContextForExpressions(*target));
-if (ast_sp) {
-  ast_sp->SetArchitecture(fixed_arch);
-  ast_sp->m_scratch_ast_source_up.reset(
-  new ClangASTSource(target->shared_from_this()));
-  lldbassert(ast_sp->getFileManager());
-  ast_sp->m_scratch_ast_source_up->InstallASTContext(
-  *ast_sp->getASTContext(), *ast_sp->getFileManager(), true);
-  llvm::IntrusiveRefCntPtr proxy_ast_source(
-  ast_sp->m_scratch_ast_source_up->CreateProxy());
-  ast_sp->SetExternalSource(proxy_ast_source);
-  return ast_sp;
-}
+new ClangASTContextForExpressions(*target, fixed_arch));
+ast_sp->m_scratch_ast_source_up.reset(
+new ClangASTSource(target->shared_from_this()));
+lldbassert(ast_sp->getFileManager());
+ast_sp->m_scratch_ast_source_up->InstallASTContext(
+*ast_sp->getASTContext(), *ast_sp->getFileManager(), true);
+llvm:

[Lldb-commits] [lldb] r373460 - [lldb][NFC] Remove ClangASTContext::Clear

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Wed Oct  2 05:38:04 2019
New Revision: 373460

URL: http://llvm.org/viewvc/llvm-project?rev=373460&view=rev
Log:
[lldb][NFC] Remove ClangASTContext::Clear

We now only use this function directly after initialization. As Clear()
resets the ASTContext back to its initial state, this is just a no-op.
There are no other users for this and we no longer can set the ASTContext
after construction, so Clear has no useful purpose anymore. It's also
mostly copy-pasted from Finalize().

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

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=373460&r1=373459&r2=373460&view=diff
==
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Wed Oct  2 05:38:04 2019
@@ -111,8 +111,6 @@ public:
   void setSema(clang::Sema *s);
   clang::Sema *getSema() { return m_sema; }
 
-  void Clear();
-
   const char *GetTargetTriple();
 
   void SetExternalSource(

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=373460&r1=373459&r2=373460&view=diff
==
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Wed Oct  2 05:38:04 2019
@@ -660,18 +660,6 @@ void ClangASTContext::Finalize() {
   m_scratch_ast_source_up.reset();
 }
 
-void ClangASTContext::Clear() {
-  m_language_options_up.reset();
-  m_source_manager_up.reset();
-  m_diagnostics_engine_up.reset();
-  m_target_options_rp.reset();
-  m_target_info_up.reset();
-  m_identifier_table_up.reset();
-  m_selector_table_up.reset();
-  m_builtins_up.reset();
-  m_pointer_byte_size = 0;
-}
-
 void ClangASTContext::setSema(Sema *s) {
   // Ensure that the new sema actually belongs to our ASTContext.
   assert(s == nullptr || &s->getASTContext() == m_ast_up.get());
@@ -683,7 +671,6 @@ const char *ClangASTContext::GetTargetTr
 }
 
 void ClangASTContext::SetTargetTriple(llvm::StringRef target_triple) {
-  Clear();
   m_target_triple = target_triple.str();
 }
 


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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Thanks. The code looks fine now, but it doesn't look like you've addressed my 
comments in the test (or at least, they didn't make it into the uploaded 
version). Also, I don't think the long comment before the code in ObjectFileELF 
really reflects what the code does right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68069



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

[Lldb-commits] [PATCH] D68270: DWARFDebugLoc: Add a function to get the address range of an entry

[Pavel Labath via Phabricator via lldb-commits]

labath planned changes to this revision.
labath marked an inline comment as done.
labath added a comment.

Thanks for the quick feedback. I didn't realize that the range list code 
handles some of this stuff already (I didn't look at it -- I guess I 
should've). I'll try to play around with this a bit, and then figure out what 
to do.

Regarding the iterator api: it has occurred to me that such an api would be 
more suitable (in fact, lldb's current api already has that), but since the 
location lists are generally small, I did not want to make a big deal out of 
it. But now that you mention it, I'll definitely consider it...


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68270



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

[Lldb-commits] [lldb] r373470 - [lldb] Fix evaluation of nested classes with parent from other CU

[Raphael Isemann via lldb-commits]

Author: teemperor
Date: Wed Oct  2 06:46:17 2019
New Revision: 373470

URL: http://llvm.org/viewvc/llvm-project?rev=373470&view=rev
Log:
[lldb] Fix evaluation of nested classes with parent from other CU

This makes sure that we associate DIEs that are imported from other CUs with 
the appropriate decl context.

Without this fix, nested classes can be dumped directly into their CU context 
if their parent was imported from another CU.

Reviewed By: teemperor, labath

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

Patch by Jaroslav Sevcik!

Added:

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/Makefile

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/other.cpp

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/shared.h
Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/Makefile?rev=373470&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/Makefile
 Wed Oct  2 06:46:17 2019
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp other.cpp
+
+include Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py?rev=373470&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
 Wed Oct  2 06:46:17 2019
@@ -0,0 +1,29 @@
+"""
+Test that the expression evaluator can access members of nested classes even if
+the parents of the nested classes were imported from another compilation unit.
+"""
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestNestedClassWithParentInAnotherCU(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def test_nested_class_with_parent_in_another_cu(self):
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+self.build()
+(_, _, thread, _) = lldbutil.run_to_source_breakpoint(self, "// break 
here", self.main_source_file)
+frame = thread.GetSelectedFrame()
+# Parse the DIEs of the parent classes and the nested classes from
+# other.cpp's CU.
+warmup_result = frame.EvaluateExpression("b")
+self.assertTrue(warmup_result.IsValid())
+# Inspect fields of the nested classes. This will reuse the types that
+# were parsed during the evaluation above. By accessing a chain of
+# fields, we try to verify that all the DIEs, reused types and
+# declaration contexts were wired properly into lldb's parser's state.
+expr_result = frame.EvaluateExpression("a.y.oY_inner.oX_inner")
+self.assertTrue(expr_result.IsValid())
+self.assertEqual(expr_result.GetValue(), "42")

Added: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp?rev=373470&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
 Wed Oct  2 06:46:17 2019
@@ -0,0 +1,22 @@
+#include "shared.h"
+
+struct WrapperA {
+  OuterY::Inner y;
+};
+
+int main() {
+  // WrapperA refers to the Inner and Outer class DIEs from this CU.
+  WrapperA a;
+  // WrapperB refers to the Inner and Outer DIEs from the other.cpp CU.
+  // It is important that WrapperB is only forward-declared in shared.h.
+  WrapperB* b = foo();
+
+  // Evaluatin

[Lldb-commits] [PATCH] D68278: Fix evaluation of nested classes with parent from other CU

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373470: [lldb] Fix evaluation of nested classes with parent 
from other CU (authored by teemperor, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68278?vs=222803&id=222820#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68278

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/Makefile
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/other.cpp
  
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/shared.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -691,6 +691,8 @@
 type_sp = unique_ast_entry_up->m_type_sp;
 if (type_sp) {
   dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
+  LinkDeclContextToDIE(
+  GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die), die);
   return type_sp;
 }
   }
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/main.cpp
@@ -0,0 +1,22 @@
+#include "shared.h"
+
+struct WrapperA {
+  OuterY::Inner y;
+};
+
+int main() {
+  // WrapperA refers to the Inner and Outer class DIEs from this CU.
+  WrapperA a;
+  // WrapperB refers to the Inner and Outer DIEs from the other.cpp CU.
+  // It is important that WrapperB is only forward-declared in shared.h.
+  WrapperB* b = foo();
+
+  // Evaluating 'b' here will parse other.cpp's DIEs for all
+  // the Inner and Outer classes from shared.h.
+  //
+  // Evaluating 'a' here will find and reuse the already-parsed
+  // versions of the Inner and Outer classes. In the associated test
+  // we make sure that we can still resolve all the types properly
+  // by evaluating 'a.y.oY_inner.oX_inner'.
+  return 0;  // break here
+}
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/TestNestedClassWithParentInAnotherCU.py
@@ -0,0 +1,29 @@
+"""
+Test that the expression evaluator can access members of nested classes even if
+the parents of the nested classes were imported from another compilation unit.
+"""
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestNestedClassWithParentInAnotherCU(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+def test_nested_class_with_parent_in_another_cu(self):
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+self.build()
+(_, _, thread, _) = lldbutil.run_to_source_breakpoint(self, "// break here", self.main_source_file)
+frame = thread.GetSelectedFrame()
+# Parse the DIEs of the parent classes and the nested classes from
+# other.cpp's CU.
+warmup_result = frame.EvaluateExpression("b")
+self.assertTrue(warmup_result.IsValid())
+# Inspect fields of the nested classes. This will reuse the types that
+# were parsed during the evaluation above. By accessing a chain of
+# fields, we try to verify that all the DIEs, reused types and
+# declaration contexts were wired properly into lldb's parser's state.
+expr_result = frame.EvaluateExpression("a.y.oY_inner.oX_inner")
+self.assertTrue(expr_result.IsValid())
+self.assertEqual(expr_result.GetValue(), "42")
Index: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/other.cpp
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/nested-class-other-compilation-unit/other.cpp
+++ lldb/trunk/packages/Python

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

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

With [modern-type-lookup], we completely evade the use of 
`ASTImporterDelegate`? That would be a wonderful thing to use only the the 
ExternalASTMerger on a long term...

> ...  a bunch of duplicated declarations

This popped a few thoughts into my head:
One way to discover duplicated declarations is to set the ODRViolation handling 
strategy to "conservative" in clang::ASTImporter. However, minimal imported 
decls will be structurally non-equivalent with those which are imported in 
normal mode. Thus, it may make sense only if the normal import mode is used 
exclusively.

> The next step is to run the ASTImporter for these temporary expressions with 
> the MinimalImport mode disabled, but that's a follow up patch.

👍 I think this is a very good direction.




Comment at: clang/include/clang/AST/ExternalASTMerger.h:120
+  /// declaration. If any ASTImporter did import the given declaration,
+  /// then this functions returns the declaration that FromD was imported from.
+  /// Returns nullptr if no ASTImporter did import import FromD.

typo: `functions` -> `function`
`FromD` -> `D` ?



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

Left over debug printout?



Comment at: clang/lib/AST/ExternalASTMerger.cpp:108
+  /// from.
+  llvm::DenseMap ToOrigin;
+  /// @see ExternalASTMerger::ImporterSource::Merger

I was thinking about that `ToOrigin` could be in the SharedState, but then I 
realized it is better to have it here, because the merger is the one which 
encapsulates the handling of several importers.



Comment at: clang/lib/AST/ExternalASTMerger.cpp:141
+// that doesn't cause having minimally imported declarations in the target
+// ASTContext that no connected ASTImporter has imported (and can 
complete).
+//

This line/sentence is hard to parse for me.
I get this part: 

```
This way the ExternalASTMerger can safely do a minimal import that doesn't 
cause having minimally imported declarations in the target ASTContext.
```

But this I don't: 
```
that no connected ASTImporter has imported.
```



Comment at: clang/lib/AST/ExternalASTMerger.cpp:149
+// the other (persistent) ASTImporter to this (temporary) ASTImporter.
+// The steps can be visualized like this:
+//

:+1:
I like this approach.



Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68326



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

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

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

lawrence_danna added a comment.

In D68188#1691129 , @labath wrote:

> Hmm... I like your solution of the typemap problem, but this CRTP class seems 
> to be way more complicated than needed. There shouldn't be any need for CRTP 
> as we already have regular dynamic dispatch via virtual methods. Also, I 
> don't think the forwarding should be needed if you're going the template 
> route. How about something like this instead?
>
>   template
>   class OwningPythonFile : public Base {
> Status Close() { /* close magic here*/; }
> // no need to forward anything -- inheritance handles that
>   };
>  
>   class TextPythonFile: public OwningPythonFile {
> // override methods as usual
>   };
>   // same for BinaryPythonFile
>
>
> Then depending on what you need, you create either a NativeFile, 
> OwningPythonFile, or a Text/BinaryPythonFile. How does that sound?


Next step is add type maps to convert these things back to python.  Without the 
CRTP it can’t just check for a single OwnedPythonfile base class — it’ll have 
to check for all three.  Is that worse than using CRTP?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188



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

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

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

lawrence_danna added a comment.

@labath any more comments on this one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68188#1691337 , @lawrence_danna 
wrote:

> In D68188#1691129 , @labath wrote:
>
> > Then depending on what you need, you create either a NativeFile, 
> > OwningPythonFile, or a Text/BinaryPythonFile. How does that 
> > sound?
>
>
> Next step is add type maps to convert these things back to python.  Without 
> the CRTP it can’t just check for a single OwnedPythonfile base class — it’ll 
> have to check for all three.  Is that worse than using CRTP?


Hmm... hard to say without knowing more details, but I would say that being 
worse than CRTP is quite hard. :) Also, Text and Binary python file will have 
OwningPythonFile as their base, so you can check for those together. I'd 
say we go with this first, and then revisit if needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188



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

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

[Pavel Labath via Phabricator via lldb-commits]

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

Nah, this looks fine to me now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67793



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

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk abandoned this revision.
kwk added a comment.

Here's the relevant transcript from #lldb@otfc for why this change is abandoned.

  [10/02/19 15:22:25]  labath: Is it acceptable for you?  "BTW 
given how this unique-ness of symbols turns out to be non-trivial is it really 
needed?  Because for regular .symtab we can ignore .dynsym (as current LLDB 
does) and for .gnu_debugdata->.symtab we can just concatenate it with .dynsym 
and there will be no symbol duplicates anyway."
  [10/02/19 15:22:25]  Otherwise it is either a big code 
complication or a big performance regression and it is not really needed for 
anything.
  [10/02/19 15:27:42]  yeah, i suppose that would work
  [10/02/19 15:28:11]  then you just need to make sure the merging does 
not kick in for non-gnu_debugdata sections
  [10/02/19 15:28:30]  a simple check on the existing of this section 
might suffice
  [10/02/19 15:29:39]  it seems to me that it would be useful to have 
information from both sources available, but it is true that nobody has really 
needed that so far
  [10/02/19 15:36:11]  Thanks.
  [10/02/19 15:36:22]  kkleine: ^^^ Let's call it a win-win. :-)
  [10/02/19 16:14:27]  labath, jankratochvil I wonder if we the 
merging with .gnu_debugdata needs to be as "clever" as it is today and if it is 
enough to just add symbols without checking for uniqueness. That would simplify 
things even more.
  [10/02/19 16:16:32]  I think just add it without checks.
  [10/02/19 16:16:53]  works for me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67390



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

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

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

aadsm added a comment.

I actually just completely missed the first comment on the .s test and forgot 
about the comment 😇.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68069



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

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

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth added a comment.

In D68258#1690757 , @aleksandr.urakov 
wrote:

> In D68258#1690756 , 
> @aleksandr.urakov wrote:
>
> > I've made it in the way similar to Zachary have made for the 
> > `SymbolFileNativePDB` plugin. An environment variable could be more 
> > convenient e.g. to run a bunch of tests using the `lldb-test` option.
>


The environment variable for using the native PDB was always intended to be 
temporary, since the native PDB reader would eventually be the only PDB reader.

In general, I find environment variables harder to manage (especially on 
Windows) than command line options.  They're invisible global variables that 
compound all the environmental issues that make building and testing so flaky 
and hard to diagnose.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D68258#1691485 , @amccarth wrote:

> The environment variable for using the native PDB was always intended to be 
> temporary, since the native PDB reader would eventually be the only PDB 
> reader.


I don't know whether that's good or bad, but I think this variable is precisely 
the same kind of "temporary" as the other one. :)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68258



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

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk planned changes to this revision.
kwk added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2711-2731
 Section *symtab =
 section_list->FindSectionByType(eSectionTypeELFSymbolTable, 
true).get();
-if (!symtab) {
-  // The symtab section is non-allocable and can be stripped, so if it
-  // doesn't exist then use the dynsym section which should always be
-  // there.
-  symtab =
-  section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true)
-  .get();
-}
 if (symtab) {
   m_symtab_up.reset(new Symtab(symtab->GetObjectFile()));
   symbol_id += ParseSymbolTable(m_symtab_up.get(), symbol_id, symtab);
 }
 

kwk wrote:
> labath wrote:
> > Let's put this bit into a separate patch. As I said over IRC, I view this 
> > bit as functionally independent of the debugdata stuff (it's definitely 
> > independent in it's current form, as it will kick in for non-debugdata 
> > files too, which I think is fine).
> Done in D67390.
We agreed to not have this other patch. See D67390#1691424.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791



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

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

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: clang/include/clang/AST/ExternalASTMerger.h:87
 const OriginMap &OM;
+/// True iff the source only exists temporary, i.e. it will be removed from
+/// the ExternalASTMerger during the life time of the ExternalASTMerger.

super-nit: there's supposed to be a comma before and after "i.e." and "e.g." :-)



Comment at: clang/lib/AST/ExternalASTMerger.cpp:171
+// Check that we never end up in the current Importer again.
+assert(&PersistentCtx != &getFromContext());
+assert(&OtherImporter != this);

&& "error message" ... this will help with debugging in a couple of months!


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68326



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

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 222845.
kwk marked 12 inline comments as done.
kwk added a comment.

- Change logic and comment for when .dynsym is parsed
- Use different pattern for error checking
- use less auto
- Remove required system-linux test feature from LZMA tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791

Files:
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/LZMA.h
  lldb/lit/CMakeLists.txt
  lldb/lit/Modules/ELF/Inputs/minidebuginfo-main.c
  lldb/lit/Modules/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/lit/Modules/ELF/minidebuginfo-find-symbols.yaml
  lldb/lit/Modules/ELF/minidebuginfo-no-lzma.yaml
  lldb/lit/Modules/ELF/minidebuginfo-set-and-hit-breakpoint.test
  lldb/lit/lit.cfg.py
  lldb/lit/lit.site.cfg.py.in
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/LZMA.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -208,6 +208,10 @@
   /// Collection of symbols from the dynamic table.
   DynamicSymbolColl m_dynamic_symbols;
 
+  /// Object file parsed from .gnu_debugdata section (\sa
+  /// GetGnuDebugDataObjectFile())
+  std::shared_ptr m_gnu_debug_data_object_file;
+
   /// List of file specifications corresponding to the modules (shared
   /// libraries) on which this object file depends.
   mutable std::unique_ptr m_filespec_up;
@@ -383,6 +387,14 @@
   lldb_private::UUID &uuid);
 
   bool AnySegmentHasPhysicalAddress();
+  
+  /// Takes the .gnu_debugdata and returns the decompressed object file that is
+  /// stored within that section.
+  ///
+  /// \returns either the decompressed object file stored within the
+  /// .gnu_debugdata section or \c nullptr if an error occured or if there's no
+  /// section with that name.
+  std::shared_ptr GetGnuDebugDataObjectFile();
 };
 
 #endif // liblldb_ObjectFileELF_h_
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -18,6 +18,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Host/LZMA.h"
 #include "lldb/Symbol/DWARFCallFrameInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/SectionLoadList.h"
@@ -1842,6 +1843,70 @@
   // unified section list.
   if (GetType() != eTypeDebugInfo)
 unified_section_list = *m_sections_up;
+  
+  // If there's a .gnu_debugdata section, we'll try to read the .symtab that's
+  // embedded in there and replace the one in the original object file (if any).
+  // If there's none in the orignal object file, we add it to it.
+  if (auto gdd_obj_file = GetGnuDebugDataObjectFile()) {
+if (auto gdd_objfile_section_list = gdd_obj_file->GetSectionList()) {
+  if (SectionSP symtab_section_sp =
+  gdd_objfile_section_list->FindSectionByType(
+  eSectionTypeELFSymbolTable, true)) {
+SectionSP module_section_sp = unified_section_list.FindSectionByType(
+eSectionTypeELFSymbolTable, true);
+if (module_section_sp)
+  unified_section_list.ReplaceSection(module_section_sp->GetID(),
+  symtab_section_sp);
+else
+  unified_section_list.AddSection(symtab_section_sp);
+  }
+}
+  }  
+}
+
+std::shared_ptr ObjectFileELF::GetGnuDebugDataObjectFile() {
+  if (m_gnu_debug_data_object_file != nullptr)
+return m_gnu_debug_data_object_file;
+
+  SectionSP section =
+  GetSectionList()->FindSectionByName(ConstString(".gnu_debugdata"));
+  if (!section)
+return nullptr;
+
+  if (!lldb_private::lzma::isAvailable()) {
+GetModule()->ReportWarning(
+"No LZMA support found for reading .gnu_debugdata section");
+return nullptr;
+  }
+
+  // Uncompress the data
+  DataExtractor data;
+  section->GetSectionData(data);
+  llvm::SmallVector uncompressedData;
+  auto err = lldb_private::lzma::uncompress(data.GetData(), uncompressedData);
+  if (err) {
+GetModule()->ReportWarning(
+"An error occurred while decompression the section %s: %s",
+section->GetName().AsCString(), llvm::toString(std::move(err)).c_str());
+return nullptr;
+  }
+
+  // Construct ObjectFileELF object from decompressed buffer
+  DataBufferSP gdd_data_buf(
+  new DataBufferHeap(uncompressedData.data(), uncompressedData.size()));
+  auto fspec = GetFileSpec().CopyByAppendingPathComponent(
+  llvm::StringRef("gnu_debugdata"));
+ 

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk requested review of this revision.
kwk added a comment.
This revision is now accepted and ready to land.

Tests did pass so this change is ready for review @labath @jankratochvil .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791



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

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

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

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

Update comment and test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68069

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

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

[Lldb-commits] [PATCH] D68317: factor out an abstract base class for File

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

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

rm NativeFile::make_shared


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68317

Files:
  lldb/include/lldb/Host/File.h
  lldb/scripts/Python/python-typemaps.swig
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBFile.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/StreamFile.cpp
  lldb/source/Host/common/File.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  
lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
  lldb/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Target/Process.cpp
  lldb/unittests/Host/FileTest.cpp

Index: lldb/unittests/Host/FileTest.cpp
===
--- lldb/unittests/Host/FileTest.cpp
+++ lldb/unittests/Host/FileTest.cpp
@@ -31,7 +31,7 @@
   FILE *stream = fdopen(fd, "r");
   ASSERT_TRUE(stream);
 
-  File file(stream, true);
+  NativeFile file(stream, true);
   EXPECT_EQ(file.GetWaitableHandle(), fd);
 }
 
@@ -46,7 +46,7 @@
   llvm::FileRemover remover(name);
   ASSERT_GE(fd, 0);
 
-  File file(fd, File::eOpenOptionWrite, true);
+  NativeFile file(fd, File::eOpenOptionWrite, true);
   ASSERT_TRUE(file.IsValid());
 
   FILE *stream = file.GetStream();
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4422,9 +4422,9 @@
 
 protected:
   Process *m_process;
-  File m_read_file;  // Read from this file (usually actual STDIN for LLDB
-  File m_write_file; // Write to this file (usually the master pty for getting
- // io to debuggee)
+  NativeFile m_read_file;  // Read from this file (usually actual STDIN for LLDB
+  NativeFile m_write_file; // Write to this file (usually the master pty for
+   // getting io to debuggee)
   Pipe m_pipe;
   std::atomic m_is_running{false};
 };
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -719,9 +719,9 @@
 
   PythonDictionary &sys_module_dict = GetSysModuleDictionary();
   if (sys_module_dict.IsValid()) {
-File in_file(in, false);
-File out_file(out, false);
-File err_file(err, false);
+NativeFile in_file(in, false);
+NativeFile out_file(out, false);
+NativeFile err_file(err, false);
 
 lldb::FileSP in_sp;
 lldb::StreamFileSP out_sp;
Index: lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -1024,8 +1024,8 @@
   // File object knows about that.
   PythonString py_mode = GetAttributeValue("mode").AsType();
   auto options = File::GetOptionsFromMode(py_mode.GetString());
-  auto file = std::make_unique(PyObject_AsFileDescriptor(m_py_obj),
- options, false);
+  auto file = std::unique_ptr(
+  new NativeFile(PyObject_AsFileDescriptor(m_py_obj), options, false));
   if (!file->IsValid())
 return nullptr;
   return file;
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
@@ -546,7 +546,7 @@
   int err = -1;
   int save_errno = 0;
   if (fd >= 0) {
-File file(fd, 0, true);
+NativeFile file(fd, 0, true);
 Status error = file.Close();
 err = 0;
 save_errno = error.GetError();
@@ -577,7 +577,7 @@
   }
 
   std::string buffer(count, 0);
-  File file(fd, File::eOpenOptionRead, false);
+  NativeFile file(fd, File::eOpenOptionRead, false);
   Status error = file.Read(static_cast(&buffer[0]), count, offset);
   const ssize_t bytes_read = error.Success() ? count : -1;
   const int save_errno = error.GetError();
@@ -609,7 +609,7 @@
 if (packet.GetChar() == ',') {
   std::string buffer;
   if (packet.GetEscapedBinaryData(buffer)) {
-File file(f

[Lldb-commits] [PATCH] D68317: factor out an abstract base class for File

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

lawrence_danna added inline comments.



Comment at: lldb/include/lldb/Host/File.h:377-381
+  template 
+  static std::shared_ptr make_shared(Args... args) {
+return std::static_pointer_cast(
+std::make_shared(args...));
+  }

labath wrote:
> Please delete this stuff, and just use regular std::make_shared. (BTW, 
> shared_ptr is implicitly convertible to shared_ptr)
Huh, I swear when i tried that yesterday the compiler was saying it wasn't 
implicitly convertible. Now, it is as you say.   Must have been gremlins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68317



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

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

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

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

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/include/lldb/Host/File.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

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

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: clang/include/clang/AST/ExternalASTMerger.h:95
   public:
-ImporterSource(ASTContext &_AST, FileManager &_FM, const OriginMap &_OM)
-: AST(_AST), FM(_FM), OM(_OM) {}
+ImporterSource(ASTContext &_AST, FileManager &_FM, const OriginMap &_OM,
+   bool _Temporary = false, ExternalASTMerger *Merger = 
nullptr)

Identifiers the begin with an underscore and followed by a capital letter are 
reserved see [lex.name/p3.1](http://eel.is/c++draft/lex.name#3.1):

>Each identifier that contains a double underscore __ or begins with an 
>underscore followed by an uppercase letter is reserved to the implementation 
>for any use.



Comment at: clang/lib/AST/ExternalASTMerger.cpp:141
+// that doesn't cause having minimally imported declarations in the target
+// ASTContext that no connected ASTImporter has imported (and can 
complete).
+//

martong wrote:
> This line/sentence is hard to parse for me.
> I get this part: 
> 
> ```
> This way the ExternalASTMerger can safely do a minimal import that doesn't 
> cause having minimally imported declarations in the target ASTContext.
> ```
> 
> But this I don't: 
> ```
> that no connected ASTImporter has imported.
> ```
I agree this is hard to parse, although I am happy that you are explaining the 
rationale.



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

So we are removing this b/c we are now doing a minimal import from the 
temporary source and then in the next patch you will change that?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68326



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

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 222861.
shafik retitled this revision from "[WIP] Modify lldb-test to print out ASTs 
from symbol file" to "Modify lldb-test to print out ASTs from symbol file".
shafik added a comment.

- Updated approach based on comment
- Pushed clang ast manipulation into `ClangASTContext.cpp`
- Created a new option in lldb-test to keep the new functionality separate from 
the old use called `dumpClangAST`
- Cleaned up checking DWARF type by using `isType()`


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

https://reviews.llvm.org/D67994

Files:
  include/lldb/Symbol/ClangASTContext.h
  source/Core/Module.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Symbol/ClangASTContext.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -42,6 +42,7 @@
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+
 #include 
 #include 
 
@@ -159,6 +160,13 @@
 static cl::opt DumpAST("dump-ast",
  cl::desc("Dump AST restored from symbols."),
  cl::sub(SymbolsSubcommand));
+static cl::opt
+DumpClangAST("dump-clang-ast",
+ cl::desc("Dump clang AST restored from symbols."),
+ cl::sub(SymbolsSubcommand));
+static cl::opt
+SymbolName("symbol-name", cl::desc("Symbol to dump complete AST for"),
+   cl::sub(SymbolsSubcommand));
 
 static cl::opt Verify("verify", cl::desc("Verify symbol information."),
 cl::sub(SymbolsSubcommand));
@@ -178,6 +186,7 @@
 static Error findVariables(lldb_private::Module &Module);
 static Error dumpModule(lldb_private::Module &Module);
 static Error dumpAST(lldb_private::Module &Module);
+static Error dumpClangAST(lldb_private::Module &Module);
 static Error verify(lldb_private::Module &Module);
 
 static Expected getAction();
@@ -524,11 +533,11 @@
 Error opts::symbols::dumpAST(lldb_private::Module &Module) {
   Module.ParseAllDebugSymbols();
 
-  auto symfile = Module.GetSymbolFile();
+  SymbolFile *symfile = Module.GetSymbolFile();
   if (!symfile)
 return make_string_error("Module has no symbol file.");
 
-  auto type_system_or_err =
+  llvm::Expected type_system_or_err =
   symfile->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
   if (!type_system_or_err)
 return make_string_error("Can't retrieve ClangASTContext");
@@ -542,7 +551,7 @@
   if (!ast_ctx)
 return make_string_error("Can't retrieve AST context.");
 
-  auto tu = ast_ctx->getTranslationUnitDecl();
+  clang::TranslationUnitDecl *tu = ast_ctx->getTranslationUnitDecl();
   if (!tu)
 return make_string_error("Can't retrieve translation unit declaration.");
 
@@ -551,6 +560,30 @@
   return Error::success();
 }
 
+Error opts::symbols::dumpClangAST(lldb_private::Module &Module) {
+  Module.ParseAllDebugSymbols();
+
+  SymbolFile *symfile = Module.GetSymbolFile();
+  if (!symfile)
+return make_string_error("Module has no symbol file.");
+
+  llvm::Expected type_system_or_err =
+  symfile->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
+  if (!type_system_or_err)
+return make_string_error("Can't retrieve ClangASTContext");
+
+  auto *clang_ast_ctx =
+  llvm::dyn_cast_or_null(&type_system_or_err.get());
+  if (!clang_ast_ctx)
+return make_string_error("Retrieved TypeSystem was not a ClangASTContext");
+
+  StreamString Stream;
+  clang_ast_ctx->DumpFromSymbols(Stream, SymbolName);
+  outs() << Stream.GetData() << "\n";
+
+  return Error::success();
+}
+
 Error opts::symbols::verify(lldb_private::Module &Module) {
   SymbolFile *symfile = Module.GetSymbolFile();
   if (!symfile)
@@ -629,6 +662,17 @@
 return dumpAST;
   }
 
+  if (DumpClangAST) {
+if (Find != FindType::None)
+  return make_string_error("Cannot both search and dump clang AST.");
+if (Regex || !Context.empty() || !Name.empty() || !File.empty() ||
+Line != 0)
+  return make_string_error(
+  "-regex, -context, -name, -file and -line options are not "
+  "applicable for dumping clang AST.");
+return dumpClangAST;
+  }
+
   if (Regex && !Context.empty())
 return make_string_error(
 "Cannot search using both regular expressions and context.");
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -9163,6 +9163,43 @@
   tu->dump(s.AsRawOstream());
 }
 
+void ClangASTContext::DumpFromSymbols(Stream &s, llvm::StringRef symbol_name) {
+  SymbolFile *symfile = GetSymbolFile();
+
+  if (!symfile)
+return;
+
+  lldb_private::TypeList type_list;
+  size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
+
+  for (size_t i = 0; i < n

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D67994#1683934 , @labath wrote:

> In D67994#1683440 , @shafik wrote:
>
> > I believe this is due to us being lazy as to when we import.
>
>
> Yes, but doesn't calling `Module::ParseAllDebugSymbols` force us to parse 
> everything?  "image dump ast" does dump only the things that have been 
> parsed, but that doesn't mean it lldb-test needs to do that too.
>
> IOW, I was not saying you should use "image dump ast" to write the test you 
> wanted to write. I was merely saying that we should try to make "lldb-test 
> -dump-ast" use the same dumping code as "image dump ast" does (assuming the 
> latter outputs the kind of data that you need, but it seems to me that it 
> does...). "image dump ast" can remain lazy, and only show the things that 
> have been parsed so far (which is also useful sometimes), while "lldb-test" 
> can do whatever it takes to parse everything (I would hope that is merely 
> calling Module::ParseAllDebugSymbols).


In order to reuse the functionality we would need to import the complete clang 
ASTs to the scratch AST context and that would end up being a lot more work 
then the current approach. So I stuck with the current approach.


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

https://reviews.llvm.org/D67994



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

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik marked 7 inline comments as done.
shafik added a comment.

I ended up splitting out the functionality into a new method `dumpClangAST` it 
looks PDB is not as lazy as DWARF and there are several PDB tests already using 
the current `dumpAST` as it is.




Comment at: tools/lldb-test/lldb-test.cpp:552-579
+
+lldb_private::TypeList type_list;
+size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
+printf( "Type list size: %zu\n", ntypes);
+
+for( size_t i = 0; i < ntypes; ++i) {
+auto type = type_list.GetTypeAtIndex(i);

shafik wrote:
> clayborg wrote:
> > I know that there is already clang AST stuff in this file, but it seems 
> > like anything in this file should be just passed to the type system for 
> > dumping? This code could be:
> > 
> > ```
> > lldb_private::TypeList type_list;
> > size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
> > printf( "Type list size: %zu\n", ntypes);
> > for( size_t i = 0; i < ntypes; ++i)
> >   type_list.GetTypeAtIndex(i)->DumpTypeValue(...);
> > ```
> > 
> > Better yet this entire function could just become:
> > 
> > ```
> > Error opts::symbols::dumpAST(lldb_private::Module &Module) {
> >   Module.ParseAllDebugSymbols();
> > 
> >   auto symfile = Module.GetSymbolFile();
> >   if (!symfile)
> > return make_string_error("Module has no symbol file.");
> > 
> >   auto type_system_or_err =
> >   symfile->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
> >   if (type_system_or_err)
> > type_system_or_err->DumpAST(...);
> >   else
> > return make_string_error("Can't retrieve TypeSystem");
> > }
> > ```
> > And all clang AST specific stuff can be removed from this binary? Tests 
> > would need to be updated.
> > 
> If we do stick with this approach pushing the `DumpAST(...)` into 
> `TypeSystem` seems reasonable.
It looked like pushing this into `ClangASTContext` made more sense. I am happy 
to bikeshed naming though.


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

https://reviews.llvm.org/D67994



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

[Lldb-commits] [PATCH] D68289: [lldb-server/android] Show more processes and package name when necessary

[walter erquinigo via Phabricator via lldb-commits]

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



Comment at: lldb/source/Host/linux/Host.cpp:178-185
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
+#if defined(__ANDROID__)
+// On android we fallback to Arg0, which commonly is an apk package name.
+if (!process_info.GetArg0().empty()) {
+  ExePath = process_info.GetArg0();

labath wrote:
> Why does this fail on android exactly? Is it some seccomp thingy? I'm 
> surprised that we're allowed to read `/proc/cmdline`, but not the `/proc/exe` 
> link..
> 
> Anyway, I don't think that pretending that "com.my.app" is an executable file 
> name is a good idea. Maybe it would be better to implement something on the 
> other end. I.e., we print out argv[0]  if the file name is not present?
From what I've been seeing, /proc/cmdline is always available, even for root 
processes. On the other hand, /proc/exe is only available for the user that is 
seeing it. In fact, if you use run-as, you can see the contents of /proc/exe, 
but that would involve making too many slow calls (run-as is slow).

I do think that having "com.my.app" as executable name is okay, because android 
doesn't let you interact with an apk by any other means, unless you root your 
device. When using android commands, "com.my.app" is just like any other 
process.

I don't have any strong opinions though, so I'm okay with following your 
suggestion if you prefer so.

Handling the android case from the host by doing exePath =  argv[0] would look 
like this:

- the Arg0 fallback is done inside the client when the exe path is empty 
- a special additional handling would be required for cases when lldb is 
running on Android. With the approach of this diff, that case is covered.
- the current function, in the case of android, returns a possible empty exe 
path. For anything not android, empty exe paths are not allowed


what do you think?



Comment at: lldb/source/Host/linux/Host.cpp:213-218
+#if defined(__ANDROID__)
+  // It's okay if we can't get the environment on android
+  return true;
+#else
+  return false;
+#endif

labath wrote:
> I think we can just make this non-android specific and say that we're ok with 
> returning the results if we were unable to fetch the environment. For 
> instance, you could make the caller not fail hard (but just log or something) 
> if this returns false.
agree!



Comment at: lldb/source/Host/linux/Host.cpp:254-260
+#if defined(__ANDROID__)
+// On android each apk has its own user, so it's better to display all
+// users instead of the current one.
+true;
+#else
+match_info.GetMatchAllUsers();
+#endif

labath wrote:
> Just from the perspective of the FindProcesses API, I don't think it's 
> reasonable for it to unilaterally ignore the explicit request to limit the 
> set of processes to return. Ideally, this decision would be made higher up 
> (somewhere near the code that actually enables us to attach to processes of 
> other users), and communicated here via the `GetMatchAllUsers` flag. However, 
> that code doesn't exist yet. Maybe we could drop this bit for now? Or 
> implement a flag to the "platform process list" which would allow one to see 
> all processes? Disregarding android, it may still be interesting to see all 
> processes of some remote system, even if one cannot attach to them. And 
> later, once we're actually able to attach to processes of other "users" we 
> can set it up so that this flag becomes the default for android targets?
this is a good plan. I'll implement that flag for now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289



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

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

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: include/lldb/Symbol/ClangASTContext.h:895
 
+  /// Dump clang AST types from the symbol table
+  ///

nit: `.` at the end



Comment at: include/lldb/Symbol/ClangASTContext.h:896
+  /// Dump clang AST types from the symbol table
+  ///
+  /// \param[in] s

What is "the symbol table" in this context? Does ClangASTContext have access to 
one?



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3059
+
+if (isType(dwarf_tag) && tag != DW_TAG_subrange_type)
+  ParseType(sc, die, &type_is_new);

comment explaining why not subrange type?



Comment at: source/Symbol/ClangASTContext.cpp:9166
 
+void ClangASTContext::DumpFromSymbols(Stream &s, llvm::StringRef symbol_name) {
+  SymbolFile *symfile = GetSymbolFile();

Ah .. so should it be called DumpFromSymbolFile() or LookupAndDump()?




Comment at: source/Symbol/ClangASTContext.cpp:9173
+  lldb_private::TypeList type_list;
+  size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
+

This API with the return value doesn't exit any more.



Comment at: source/Symbol/ClangASTContext.cpp:9182
+
+s << type->GetName().AsCString() << "\n";
+

the AsCString() part seems to be bad for the performance.. is there a variant 
that returns a StringRef instead?



Comment at: source/Symbol/ClangASTContext.cpp:9184
+
+if (clang::CXXRecordDecl *record_decl =
+
GetAsCXXRecordDecl(type->GetFullCompilerType().GetOpaqueQualType()))

A switch over the kind would be more efficient and potentially nicer to read?



Comment at: source/Symbol/ClangASTContext.cpp:9195
+ GetAsEnumDecl(type->GetFullCompilerType()))
+  enum_decl->dump(s.AsRawOstream());
+else {

is an EnumDecl not a TagDecl?


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

https://reviews.llvm.org/D67994



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

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

[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/lit/SymbolFile/dissassemble-entry-point.s:10
+
+.global _Start
+_start:

This .global _Start is also unneeded, as it does not even match the actual 
_start symbol below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68069



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

[Lldb-commits] [lldb] r373497 - [JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationServerLLGS

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct  2 11:02:23 2019
New Revision: 373497

URL: http://llvm.org/viewvc/llvm-project?rev=373497&view=rev
Log:
[JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationServerLLGS

This patch replaces the LLDB's JSON implementation with the one from
LLVM in GDBRemoteCommunicationServerLLGS.

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

Modified:

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

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=373497&r1=373496&r2=373497&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
 Wed Oct  2 11:02:23 2019
@@ -32,7 +32,6 @@
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/Endian.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegisterValue.h"
@@ -40,6 +39,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/UriParser.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include "ProcessGDBRemote.h"
@@ -402,19 +402,21 @@ static void WriteRegisterValueInHexFixed
   }
 }
 
-static JSONObject::SP GetRegistersAsJSON(NativeThreadProtocol &thread) {
+static llvm::Expected
+GetRegistersAsJSON(NativeThreadProtocol &thread) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
 
   NativeRegisterContext& reg_ctx = thread.GetRegisterContext();
 
-  JSONObject::SP register_object_sp = std::make_shared();
+  json::Object register_object;
 
 #ifdef LLDB_JTHREADSINFO_FULL_REGISTER_SET
   // Expedite all registers in the first register set (i.e. should be GPRs)
   // that are not contained in other registers.
   const RegisterSet *reg_set_p = reg_ctx_sp->GetRegisterSet(0);
   if (!reg_set_p)
-return nullptr;
+return llvm::make_error("failed to get registers",
+   llvm::inconvertibleErrorCode());
   for (const uint32_t *reg_num_p = reg_set_p->registers;
*reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
 uint32_t reg_num = *reg_num_p;
@@ -460,12 +462,10 @@ static JSONObject::SP GetRegistersAsJSON
 WriteRegisterValueInHexFixedWidth(stream, reg_ctx, *reg_info_p,
   ®_value, lldb::eByteOrderBig);
 
-register_object_sp->SetObject(
-llvm::to_string(reg_num),
-std::make_shared(stream.GetString()));
+register_object.try_emplace(llvm::to_string(reg_num), stream.GetString());
   }
 
-  return register_object_sp;
+  return register_object;
 }
 
 static const char *GetStopReasonString(StopReason stop_reason) {
@@ -492,11 +492,11 @@ static const char *GetStopReasonString(S
   return nullptr;
 }
 
-static JSONArray::SP GetJSONThreadsInfo(NativeProcessProtocol &process,
-bool abridged) {
+static llvm::Expected
+GetJSONThreadsInfo(NativeProcessProtocol &process, bool abridged) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
 
-  JSONArray::SP threads_array_sp = std::make_shared();
+  json::Array threads_array;
 
   // Ensure we can get info on the given thread.
   uint32_t thread_idx = 0;
@@ -510,7 +510,8 @@ static JSONArray::SP GetJSONThreadsInfo(
 struct ThreadStopInfo tid_stop_info;
 std::string description;
 if (!thread->GetStopReason(tid_stop_info, description))
-  return nullptr;
+  return llvm::make_error(
+  "failed to get stop reason", llvm::inconvertibleErrorCode());
 
 const int signum = tid_stop_info.details.signal.signo;
 if (log) {
@@ -522,50 +523,49 @@ static JSONArray::SP GetJSONThreadsInfo(
 tid_stop_info.reason, tid_stop_info.details.exception.type);
 }
 
-JSONObject::SP thread_obj_sp = std::make_shared();
-threads_array_sp->AppendObject(thread_obj_sp);
+json::Object thread_obj;
 
 if (!abridged) {
-  if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread))
-thread_obj_sp->SetObject("registers", registers_sp);
+  if (llvm::Expected registers =
+  GetRegistersAsJSON(*thread)) {
+thread_obj.try_emplace("registers", std::move(*registers));
+  } else {
+return registers.takeError();
+  }
 }
 
-thread_obj_sp->SetObject("tid", std::make_shared(tid));
+thread_obj.try_emplace("tid", static_cast(tid));
+
 if (signum != 0)
-  thread_obj_sp->SetObject("signal", std::make_shared(signum));
+  thread_obj.try_emplace("signal", signum);
 
 const std::string thread_name

[Lldb-commits] [lldb] r373498 - [JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationClient

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct  2 11:02:26 2019
New Revision: 373498

URL: http://llvm.org/viewvc/llvm-project?rev=373498&view=rev
Log:
[JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationClient

This patch replaces the LLDB's JSON implementation with the one from
LLVM in GDBRemoteCommunicationClient.

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

Modified:

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

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=373498&r1=373497&r2=373498&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
Wed Oct  2 11:02:26 2019
@@ -23,7 +23,6 @@
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
@@ -35,14 +34,15 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/JSON.h"
 
 #if defined(HAVE_LIBCOMPRESSION)
 #include 
 #endif
 
 using namespace lldb;
-using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
 using namespace std::chrono;
 
 // GDBRemoteCommunicationClient constructor
@@ -3609,21 +3609,21 @@ ParseModuleSpec(StructuredData::Dictiona
 llvm::Optional>
 GDBRemoteCommunicationClient::GetModulesInfo(
 llvm::ArrayRef module_file_specs, const llvm::Triple &triple) {
+  namespace json = llvm::json;
+
   if (!m_supports_jModulesInfo)
 return llvm::None;
 
-  JSONArray::SP module_array_sp = std::make_shared();
+  json::Array module_array;
   for (const FileSpec &module_file_spec : module_file_specs) {
-JSONObject::SP module_sp = std::make_shared();
-module_array_sp->AppendObject(module_sp);
-module_sp->SetObject(
-"file", std::make_shared(module_file_spec.GetPath(false)));
-module_sp->SetObject("triple",
- std::make_shared(triple.getTriple()));
+module_array.push_back(
+json::Object{{"file", module_file_spec.GetPath(false)},
+ {"triple", triple.getTriple()}});
   }
   StreamString unescaped_payload;
   unescaped_payload.PutCString("jModulesInfo:");
-  module_array_sp->Write(unescaped_payload);
+  unescaped_payload.AsRawOstream() << std::move(module_array);
+
   StreamGDBRemote payload;
   payload.PutEscapedBytes(unescaped_payload.GetString().data(),
   unescaped_payload.GetSize());


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

[Lldb-commits] [lldb] r373499 - [JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationServerPlatform

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct  2 11:02:29 2019
New Revision: 373499

URL: http://llvm.org/viewvc/llvm-project?rev=373499&view=rev
Log:
[JSON] Use LLVM's library for encoding JSON in 
GDBRemoteCommunicationServerPlatform

This patch replaces the LLDB's JSON implementation with the one from
LLVM in GDBRemoteCommunicationServerPlatform.

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

Modified:

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

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=373499&r1=373498&r2=373499&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 Wed Oct  2 11:02:29 2019
@@ -18,6 +18,7 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Threading.h"
 
 #include "lldb/Host/Config.h"
@@ -28,7 +29,6 @@
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/GDBRemote.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StructuredData.h"
@@ -37,8 +37,8 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 using namespace lldb;
-using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
 
 // GDBRemoteCommunicationServerPlatform constructor
 GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
@@ -215,22 +215,21 @@ GDBRemoteCommunicationServerPlatform::Ha
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerPlatform::Handle_qQueryGDBServer(
 StringExtractorGDBRemote &packet) {
+  namespace json = llvm::json;
+
   if (m_pending_gdb_server.pid == LLDB_INVALID_PROCESS_ID)
 return SendErrorResponse(4);
 
-  JSONObject::SP server_sp = std::make_shared();
-  server_sp->SetObject("port",
-   
std::make_shared(m_pending_gdb_server.port));
+  json::Object server{{"port", m_pending_gdb_server.port}};
+
   if (!m_pending_gdb_server.socket_name.empty())
-server_sp->SetObject(
-"socket_name",
-
std::make_shared(m_pending_gdb_server.socket_name.c_str()));
+server.try_emplace("socket_name", m_pending_gdb_server.socket_name);
 
-  JSONArray server_list;
-  server_list.AppendObject(server_sp);
+  json::Array server_list;
+  server_list.push_back(std::move(server));
 
   StreamGDBRemote response;
-  server_list.Write(response);
+  response.AsRawOstream() << std::move(server_list);
 
   StreamGDBRemote escaped_response;
   escaped_response.PutEscapedBytes(response.GetString().data(),


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

[Lldb-commits] [lldb] r373501 - [JSON] Remove Utility/JSON.{h|cpp}

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct  2 11:02:36 2019
New Revision: 373501

URL: http://llvm.org/viewvc/llvm-project?rev=373501&view=rev
Log:
[JSON] Remove Utility/JSON.{h|cpp}

This patch is the final step in my quest to get rid of the JSON parser
in LLDB. Vedant's coverage report [1] shows that it was mostly untested.
Furthermore, the LLVM implementation has a much nicer API and using it
means one less thing to maintain for LLDB.

[1] http://lab.llvm.org:8080/coverage/coverage-reports/index.html

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

Removed:
lldb/trunk/include/lldb/Utility/JSON.h
lldb/trunk/source/Utility/JSON.cpp
lldb/trunk/unittests/Utility/JSONTest.cpp
Modified:
lldb/trunk/source/Utility/CMakeLists.txt
lldb/trunk/unittests/Utility/CMakeLists.txt

Removed: lldb/trunk/include/lldb/Utility/JSON.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/JSON.h?rev=373500&view=auto
==
--- lldb/trunk/include/lldb/Utility/JSON.h (original)
+++ lldb/trunk/include/lldb/Utility/JSON.h (removed)
@@ -1,283 +0,0 @@
-//===-JSON.h *- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef utility_JSON_h_
-#define utility_JSON_h_
-
-#include "lldb/Utility/StringExtractor.h"
-
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-namespace lldb_private {
-class Stream;
-
-class JSONValue {
-public:
-  virtual void Write(Stream &s) = 0;
-
-  typedef std::shared_ptr SP;
-
-  enum class Kind { String, Number, True, False, Null, Object, Array };
-
-  JSONValue(Kind k) : m_kind(k) {}
-
-  Kind GetKind() const { return m_kind; }
-
-  virtual ~JSONValue() = default;
-
-private:
-  const Kind m_kind;
-};
-
-class JSONString : public JSONValue {
-public:
-  JSONString();
-  JSONString(const char *s);
-  JSONString(const std::string &s);
-
-  JSONString(const JSONString &s) = delete;
-  JSONString &operator=(const JSONString &s) = delete;
-
-  void Write(Stream &s) override;
-
-  typedef std::shared_ptr SP;
-
-  std::string GetData() { return m_data; }
-
-  static bool classof(const JSONValue *V) {
-return V->GetKind() == JSONValue::Kind::String;
-  }
-
-  ~JSONString() override = default;
-
-private:
-  static std::string json_string_quote_metachars(const std::string &);
-
-  std::string m_data;
-};
-
-class JSONNumber : public JSONValue {
-public:
-  typedef std::shared_ptr SP;
-
-  // We cretae a constructor for all integer and floating point type with using
-  // templates and
-  // SFINAE to avoid having ambiguous overloads because of the implicit type
-  // promotion. If we
-  // would have constructors only with int64_t, uint64_t and double types then
-  // constructing a JSONNumber from an int32_t (or any other similar type)
-  // would fail to compile.
-
-  template ::value &&
-std::is_unsigned::value>::type * = nullptr>
-  explicit JSONNumber(T u)
-  : JSONValue(JSONValue::Kind::Number), m_data_type(DataType::Unsigned) {
-m_data.m_unsigned = u;
-  }
-
-  template ::value &&
-std::is_signed::value>::type * = 
nullptr>
-  explicit JSONNumber(T s)
-  : JSONValue(JSONValue::Kind::Number), m_data_type(DataType::Signed) {
-m_data.m_signed = s;
-  }
-
-  template ::value>::type * = 
nullptr>
-  explicit JSONNumber(T d)
-  : JSONValue(JSONValue::Kind::Number), m_data_type(DataType::Double) {
-m_data.m_double = d;
-  }
-
-  ~JSONNumber() override = default;
-
-  JSONNumber(const JSONNumber &s) = delete;
-  JSONNumber &operator=(const JSONNumber &s) = delete;
-
-  void Write(Stream &s) override;
-
-  uint64_t GetAsUnsigned() const;
-
-  int64_t GetAsSigned() const;
-
-  double GetAsDouble() const;
-
-  static bool classof(const JSONValue *V) {
-return V->GetKind() == JSONValue::Kind::Number;
-  }
-
-private:
-  enum class DataType : uint8_t { Unsigned, Signed, Double } m_data_type;
-
-  union {
-uint64_t m_unsigned;
-int64_t m_signed;
-double m_double;
-  } m_data;
-};
-
-class JSONTrue : public JSONValue {
-public:
-  JSONTrue();
-
-  JSONTrue(const JSONTrue &s) = delete;
-  JSONTrue &operator=(const JSONTrue &s) = delete;
-
-  void Write(Stream &s) override;
-
-  typedef std::shared_ptr SP;
-
-  static bool classof(const JSONValue *V) {
-return V->GetKind() == JSONValue::Kind::True;
-  }
-
-  ~JSONTrue() override = default;
-};
-
-class JSONFalse : public JSONValue {
-public:
-  JSONFalse();
-
-  JSONFalse(const JSONFalse &s) = delete;
-  JSONFalse &operator=(const JSONFalse &s) = delete;
-
-  void Write(Stream &s) override;
-
-  typedef std::shared_ptr SP;
-
- 

[Lldb-commits] [lldb] r373500 - [JSON] Use LLVM's library for encoding JSON in GDBRemoteCommunicationServerCommon

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Wed Oct  2 11:02:32 2019
New Revision: 373500

URL: http://llvm.org/viewvc/llvm-project?rev=373500&view=rev
Log:
[JSON] Use LLVM's library for encoding JSON in 
GDBRemoteCommunicationServerCommon

This patch replaces the LLDB's JSON implementation with the one from
LLVM in GDBRemoteCommunicationServerCommon.

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

Modified:

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

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=373500&r1=373499&r2=373500&view=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
 Wed Oct  2 11:02:32 2019
@@ -30,11 +30,12 @@
 #include "lldb/Target/Platform.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/GDBRemote.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StructuredData.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/JSON.h"
 
 #include "ProcessGDBRemoteLog.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
@@ -43,11 +44,10 @@
 #include "lldb/Host/android/HostInfoAndroid.h"
 #endif
 
-#include "llvm/ADT/StringSwitch.h"
 
 using namespace lldb;
-using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
 
 #ifdef __ANDROID__
 const static uint32_t g_default_packet_timeout_sec = 20; // seconds
@@ -1120,6 +1120,8 @@ GDBRemoteCommunicationServerCommon::Hand
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_jModulesInfo(
 StringExtractorGDBRemote &packet) {
+  namespace json = llvm::json;
+
   packet.SetFilePos(::strlen("jModulesInfo:"));
 
   StructuredData::ObjectSP object_sp = 
StructuredData::ParseJSON(packet.Peek());
@@ -1130,7 +1132,7 @@ GDBRemoteCommunicationServerCommon::Hand
   if (!packet_array)
 return SendErrorResponse(2);
 
-  JSONArray::SP response_array_sp = std::make_shared();
+  json::Array response_array;
   for (size_t i = 0; i < packet_array->GetSize(); ++i) {
 StructuredData::Dictionary *query =
 packet_array->GetItemAtIndex(i)->GetAsDictionary();
@@ -1148,27 +1150,22 @@ GDBRemoteCommunicationServerCommon::Hand
 const auto file_offset = matched_module_spec.GetObjectOffset();
 const auto file_size = matched_module_spec.GetObjectSize();
 const auto uuid_str = matched_module_spec.GetUUID().GetAsString("");
-
 if (uuid_str.empty())
   continue;
-
-JSONObject::SP response = std::make_shared();
-response_array_sp->AppendObject(response);
-response->SetObject("uuid", std::make_shared(uuid_str));
-response->SetObject(
-"triple",
-std::make_shared(
-matched_module_spec.GetArchitecture().GetTriple().getTriple()));
-response->SetObject("file_path",
-std::make_shared(
-matched_module_spec.GetFileSpec().GetPath()));
-response->SetObject("file_offset",
-std::make_shared(file_offset));
-response->SetObject("file_size", std::make_shared(file_size));
+const auto triple_str =
+matched_module_spec.GetArchitecture().GetTriple().getTriple();
+const auto file_path = matched_module_spec.GetFileSpec().GetPath();
+
+json::Object response{{"uuid", uuid_str},
+  {"triple", triple_str},
+  {"file_path", file_path},
+  {"file_offset", static_cast(file_offset)},
+  {"file_size", static_cast(file_size)}};
+response_array.push_back(std::move(response));
   }
 
   StreamString response;
-  response_array_sp->Write(response);
+  response.AsRawOstream() << std::move(response_array);
   StreamGDBRemote escaped_response;
   escaped_response.PutEscapedBytes(response.GetString().data(),
response.GetSize());


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

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

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373498: [JSON] Use LLVM's library for encoding JSON in 
GDBRemoteCommunicationClient (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68301?vs=222710&id=222877#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68301

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


Index: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
@@ -35,14 +34,15 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/JSON.h"
 
 #if defined(HAVE_LIBCOMPRESSION)
 #include 
 #endif
 
 using namespace lldb;
-using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
 using namespace std::chrono;
 
 // GDBRemoteCommunicationClient constructor
@@ -3609,21 +3609,21 @@
 llvm::Optional>
 GDBRemoteCommunicationClient::GetModulesInfo(
 llvm::ArrayRef module_file_specs, const llvm::Triple &triple) {
+  namespace json = llvm::json;
+
   if (!m_supports_jModulesInfo)
 return llvm::None;
 
-  JSONArray::SP module_array_sp = std::make_shared();
+  json::Array module_array;
   for (const FileSpec &module_file_spec : module_file_specs) {
-JSONObject::SP module_sp = std::make_shared();
-module_array_sp->AppendObject(module_sp);
-module_sp->SetObject(
-"file", std::make_shared(module_file_spec.GetPath(false)));
-module_sp->SetObject("triple",
- std::make_shared(triple.getTriple()));
+module_array.push_back(
+json::Object{{"file", module_file_spec.GetPath(false)},
+ {"triple", triple.getTriple()}});
   }
   StreamString unescaped_payload;
   unescaped_payload.PutCString("jModulesInfo:");
-  module_array_sp->Write(unescaped_payload);
+  unescaped_payload.AsRawOstream() << std::move(module_array);
+
   StreamGDBRemote payload;
   payload.PutEscapedBytes(unescaped_payload.GetString().data(),
   unescaped_payload.GetSize());


Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -23,7 +23,6 @@
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
@@ -35,14 +34,15 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/JSON.h"
 
 #if defined(HAVE_LIBCOMPRESSION)
 #include 
 #endif
 
 using namespace lldb;
-using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
 using namespace std::chrono;
 
 // GDBRemoteCommunicationClient constructor
@@ -3609,21 +3609,21 @@
 llvm::Optional>
 GDBRemoteCommunicationClient::GetModulesInfo(
 llvm::ArrayRef module_file_specs, const llvm::Triple &triple) {
+  namespace json = llvm::json;
+
   if (!m_supports_jModulesInfo)
 return llvm::None;
 
-  JSONArray::SP module_array_sp = std::make_shared();
+  json::Array module_array;
   for (const FileSpec &module_file_spec : module_file_specs) {
-JSONObject::SP module_sp = std::make_shared();
-module_array_sp->AppendObject(module_sp);
-module_sp->SetObject(
-"file", std::make_shared(module_file_spec.GetPath(false)));
-module_sp->SetObject("triple",
- std::make_shared(triple.getTriple()));
+module_array.push_back(
+json::Object{{"file", module_file_spec.GetPath(false)},
+ {"triple", triple.getTriple()}});
   }
   StreamString unescaped_payload;
   unescaped_payload.PutCString("jModulesInfo:");
-  module_array_sp->Write(unescaped_payload);
+  unescaped_payload.AsRawOstream() << std::move(module_array);
+
   StreamGDBRemote payload;
   payload.PutEscapedBytes(unescaped_payload.GetString().data(),
   unescaped_payload.GetSize());

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

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373497: [JSON] Use LLVM's library for encoding JSON in 
GDBRemoteCommunicationServerLLGS (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68299?vs=222706&id=222876#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68299

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

Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -32,7 +32,6 @@
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/Endian.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegisterValue.h"
@@ -40,6 +39,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/UriParser.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include "ProcessGDBRemote.h"
@@ -402,19 +402,21 @@
   }
 }
 
-static JSONObject::SP GetRegistersAsJSON(NativeThreadProtocol &thread) {
+static llvm::Expected
+GetRegistersAsJSON(NativeThreadProtocol &thread) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
 
   NativeRegisterContext& reg_ctx = thread.GetRegisterContext();
 
-  JSONObject::SP register_object_sp = std::make_shared();
+  json::Object register_object;
 
 #ifdef LLDB_JTHREADSINFO_FULL_REGISTER_SET
   // Expedite all registers in the first register set (i.e. should be GPRs)
   // that are not contained in other registers.
   const RegisterSet *reg_set_p = reg_ctx_sp->GetRegisterSet(0);
   if (!reg_set_p)
-return nullptr;
+return llvm::make_error("failed to get registers",
+   llvm::inconvertibleErrorCode());
   for (const uint32_t *reg_num_p = reg_set_p->registers;
*reg_num_p != LLDB_INVALID_REGNUM; ++reg_num_p) {
 uint32_t reg_num = *reg_num_p;
@@ -460,12 +462,10 @@
 WriteRegisterValueInHexFixedWidth(stream, reg_ctx, *reg_info_p,
   ®_value, lldb::eByteOrderBig);
 
-register_object_sp->SetObject(
-llvm::to_string(reg_num),
-std::make_shared(stream.GetString()));
+register_object.try_emplace(llvm::to_string(reg_num), stream.GetString());
   }
 
-  return register_object_sp;
+  return register_object;
 }
 
 static const char *GetStopReasonString(StopReason stop_reason) {
@@ -492,11 +492,11 @@
   return nullptr;
 }
 
-static JSONArray::SP GetJSONThreadsInfo(NativeProcessProtocol &process,
-bool abridged) {
+static llvm::Expected
+GetJSONThreadsInfo(NativeProcessProtocol &process, bool abridged) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
 
-  JSONArray::SP threads_array_sp = std::make_shared();
+  json::Array threads_array;
 
   // Ensure we can get info on the given thread.
   uint32_t thread_idx = 0;
@@ -510,7 +510,8 @@
 struct ThreadStopInfo tid_stop_info;
 std::string description;
 if (!thread->GetStopReason(tid_stop_info, description))
-  return nullptr;
+  return llvm::make_error(
+  "failed to get stop reason", llvm::inconvertibleErrorCode());
 
 const int signum = tid_stop_info.details.signal.signo;
 if (log) {
@@ -522,50 +523,49 @@
 tid_stop_info.reason, tid_stop_info.details.exception.type);
 }
 
-JSONObject::SP thread_obj_sp = std::make_shared();
-threads_array_sp->AppendObject(thread_obj_sp);
+json::Object thread_obj;
 
 if (!abridged) {
-  if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread))
-thread_obj_sp->SetObject("registers", registers_sp);
+  if (llvm::Expected registers =
+  GetRegistersAsJSON(*thread)) {
+thread_obj.try_emplace("registers", std::move(*registers));
+  } else {
+return registers.takeError();
+  }
 }
 
-thread_obj_sp->SetObject("tid", std::make_shared(tid));
+thread_obj.try_emplace("tid", static_cast(tid));
+
 if (signum != 0)
-  thread_obj_sp->SetObject("signal", std::make_shared(signum));
+  thread_obj.try_emplace("signal", signum);
 
 const std::string thread_name = thread->GetName();
 if (!thread_name.empty())
-  thread_obj_sp->SetObject("name",
-   std::make_shared(thread_name));
+  thread_obj.try_emplace("name", thread_name);
 
-if (const char *stop_reason_str = GetStopReasonString(tid_stop_info.reason))
-  thread_obj_sp->S

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

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373499: [JSON] Use LLVM's library for encoding JSON in… 
(authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68302?vs=222711&id=222878#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68302

Files:
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp


Index: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -18,6 +18,7 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Threading.h"
 
 #include "lldb/Host/Config.h"
@@ -28,7 +29,6 @@
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/GDBRemote.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StructuredData.h"
@@ -37,8 +37,8 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 using namespace lldb;
-using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
 
 // GDBRemoteCommunicationServerPlatform constructor
 GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
@@ -215,22 +215,21 @@
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerPlatform::Handle_qQueryGDBServer(
 StringExtractorGDBRemote &packet) {
+  namespace json = llvm::json;
+
   if (m_pending_gdb_server.pid == LLDB_INVALID_PROCESS_ID)
 return SendErrorResponse(4);
 
-  JSONObject::SP server_sp = std::make_shared();
-  server_sp->SetObject("port",
-   
std::make_shared(m_pending_gdb_server.port));
+  json::Object server{{"port", m_pending_gdb_server.port}};
+
   if (!m_pending_gdb_server.socket_name.empty())
-server_sp->SetObject(
-"socket_name",
-
std::make_shared(m_pending_gdb_server.socket_name.c_str()));
+server.try_emplace("socket_name", m_pending_gdb_server.socket_name);
 
-  JSONArray server_list;
-  server_list.AppendObject(server_sp);
+  json::Array server_list;
+  server_list.push_back(std::move(server));
 
   StreamGDBRemote response;
-  server_list.Write(response);
+  response.AsRawOstream() << std::move(server_list);
 
   StreamGDBRemote escaped_response;
   escaped_response.PutEscapedBytes(response.GetString().data(),


Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -18,6 +18,7 @@
 #include 
 
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Threading.h"
 
 #include "lldb/Host/Config.h"
@@ -28,7 +29,6 @@
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/GDBRemote.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StructuredData.h"
@@ -37,8 +37,8 @@
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 using namespace lldb;
-using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
 
 // GDBRemoteCommunicationServerPlatform constructor
 GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
@@ -215,22 +215,21 @@
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerPlatform::Handle_qQueryGDBServer(
 StringExtractorGDBRemote &packet) {
+  namespace json = llvm::json;
+
   if (m_pending_gdb_server.pid == LLDB_INVALID_PROCESS_ID)
 return SendErrorResponse(4);
 
-  JSONObject::SP server_sp = std::make_shared();
-  server_sp->SetObject("port",
-   std::make_shared(m_pending_gdb_server.port));
+  json::Object server{{"port", m_pending_gdb_server.port}};
+
   if (!m_pending_gdb_server.socket_name.empty())
-server_sp->SetObject(
-"socket_name",
-std::make_shared(m_pending_gdb_server.socket_name.c_str()));
+server.try_emplace("socket_name", m_pending_gdb_server.socket_name);
 
-  JSONArray server_list;
-  server_list.AppendObject(server_sp);
+  json::Array server_list;
+  server_list.push_back(std::move(server));
 
   StreamGDBRemote response;
-  server_list.Write(response);
+  response.AsRawOstream() << std::move(server_list);
 
   StreamGDBRemote escaped_response;
   esc

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

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373500: [JSON] Use LLVM's library for encoding JSON in… 
(authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68304?vs=222715&id=222879#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68304

Files:
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp


Index: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -30,11 +30,12 @@
 #include "lldb/Target/Platform.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/GDBRemote.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StructuredData.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/JSON.h"
 
 #include "ProcessGDBRemoteLog.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
@@ -43,11 +44,10 @@
 #include "lldb/Host/android/HostInfoAndroid.h"
 #endif
 
-#include "llvm/ADT/StringSwitch.h"
 
 using namespace lldb;
-using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
 
 #ifdef __ANDROID__
 const static uint32_t g_default_packet_timeout_sec = 20; // seconds
@@ -1120,6 +1120,8 @@
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_jModulesInfo(
 StringExtractorGDBRemote &packet) {
+  namespace json = llvm::json;
+
   packet.SetFilePos(::strlen("jModulesInfo:"));
 
   StructuredData::ObjectSP object_sp = 
StructuredData::ParseJSON(packet.Peek());
@@ -1130,7 +1132,7 @@
   if (!packet_array)
 return SendErrorResponse(2);
 
-  JSONArray::SP response_array_sp = std::make_shared();
+  json::Array response_array;
   for (size_t i = 0; i < packet_array->GetSize(); ++i) {
 StructuredData::Dictionary *query =
 packet_array->GetItemAtIndex(i)->GetAsDictionary();
@@ -1148,27 +1150,22 @@
 const auto file_offset = matched_module_spec.GetObjectOffset();
 const auto file_size = matched_module_spec.GetObjectSize();
 const auto uuid_str = matched_module_spec.GetUUID().GetAsString("");
-
 if (uuid_str.empty())
   continue;
-
-JSONObject::SP response = std::make_shared();
-response_array_sp->AppendObject(response);
-response->SetObject("uuid", std::make_shared(uuid_str));
-response->SetObject(
-"triple",
-std::make_shared(
-matched_module_spec.GetArchitecture().GetTriple().getTriple()));
-response->SetObject("file_path",
-std::make_shared(
-matched_module_spec.GetFileSpec().GetPath()));
-response->SetObject("file_offset",
-std::make_shared(file_offset));
-response->SetObject("file_size", std::make_shared(file_size));
+const auto triple_str =
+matched_module_spec.GetArchitecture().GetTriple().getTriple();
+const auto file_path = matched_module_spec.GetFileSpec().GetPath();
+
+json::Object response{{"uuid", uuid_str},
+  {"triple", triple_str},
+  {"file_path", file_path},
+  {"file_offset", static_cast(file_offset)},
+  {"file_size", static_cast(file_size)}};
+response_array.push_back(std::move(response));
   }
 
   StreamString response;
-  response_array_sp->Write(response);
+  response.AsRawOstream() << std::move(response_array);
   StreamGDBRemote escaped_response;
   escaped_response.PutEscapedBytes(response.GetString().data(),
response.GetSize());


Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -30,11 +30,12 @@
 #include "lldb/Target/Platform.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/GDBRemote.h"
-#include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StructuredData.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Support/JSON.h"
 
 #include "ProcessGDBRemoteLog.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
@@ -43,11 +44,10 @@
 #include "lldb/Host/android/HostInfoAndroid.h"
 #endif
 
-#include "llvm/ADT/StringSwitch.h"
 
 using names

[Lldb-commits] [PATCH] D68305: [JSON] Remove Utility/JSON.{h|cpp}

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373501: [JSON] Remove Utility/JSON.{h|cpp} (authored by 
JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68305?vs=222716&id=222880#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68305

Files:
  lldb/trunk/include/lldb/Utility/JSON.h
  lldb/trunk/source/Utility/CMakeLists.txt
  lldb/trunk/source/Utility/JSON.cpp
  lldb/trunk/unittests/Utility/CMakeLists.txt
  lldb/trunk/unittests/Utility/JSONTest.cpp

Index: lldb/trunk/unittests/Utility/CMakeLists.txt
===
--- lldb/trunk/unittests/Utility/CMakeLists.txt
+++ lldb/trunk/unittests/Utility/CMakeLists.txt
@@ -11,7 +11,6 @@
   EventTest.cpp
   FileSpecTest.cpp
   FlagsTest.cpp
-  JSONTest.cpp
   ListenerTest.cpp
   LogTest.cpp
   NameMatchesTest.cpp
Index: lldb/trunk/source/Utility/CMakeLists.txt
===
--- lldb/trunk/source/Utility/CMakeLists.txt
+++ lldb/trunk/source/Utility/CMakeLists.txt
@@ -27,7 +27,6 @@
   FileSpec.cpp
   GDBRemote.cpp
   IOObject.cpp
-  JSON.cpp
   LLDBAssert.cpp
   Listener.cpp
   Log.cpp
Index: lldb/trunk/include/lldb/Utility/JSON.h
===
--- lldb/trunk/include/lldb/Utility/JSON.h
+++ lldb/trunk/include/lldb/Utility/JSON.h
@@ -1,283 +0,0 @@
-//===-JSON.h *- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef utility_JSON_h_
-#define utility_JSON_h_
-
-#include "lldb/Utility/StringExtractor.h"
-
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-namespace lldb_private {
-class Stream;
-
-class JSONValue {
-public:
-  virtual void Write(Stream &s) = 0;
-
-  typedef std::shared_ptr SP;
-
-  enum class Kind { String, Number, True, False, Null, Object, Array };
-
-  JSONValue(Kind k) : m_kind(k) {}
-
-  Kind GetKind() const { return m_kind; }
-
-  virtual ~JSONValue() = default;
-
-private:
-  const Kind m_kind;
-};
-
-class JSONString : public JSONValue {
-public:
-  JSONString();
-  JSONString(const char *s);
-  JSONString(const std::string &s);
-
-  JSONString(const JSONString &s) = delete;
-  JSONString &operator=(const JSONString &s) = delete;
-
-  void Write(Stream &s) override;
-
-  typedef std::shared_ptr SP;
-
-  std::string GetData() { return m_data; }
-
-  static bool classof(const JSONValue *V) {
-return V->GetKind() == JSONValue::Kind::String;
-  }
-
-  ~JSONString() override = default;
-
-private:
-  static std::string json_string_quote_metachars(const std::string &);
-
-  std::string m_data;
-};
-
-class JSONNumber : public JSONValue {
-public:
-  typedef std::shared_ptr SP;
-
-  // We cretae a constructor for all integer and floating point type with using
-  // templates and
-  // SFINAE to avoid having ambiguous overloads because of the implicit type
-  // promotion. If we
-  // would have constructors only with int64_t, uint64_t and double types then
-  // constructing a JSONNumber from an int32_t (or any other similar type)
-  // would fail to compile.
-
-  template ::value &&
-std::is_unsigned::value>::type * = nullptr>
-  explicit JSONNumber(T u)
-  : JSONValue(JSONValue::Kind::Number), m_data_type(DataType::Unsigned) {
-m_data.m_unsigned = u;
-  }
-
-  template ::value &&
-std::is_signed::value>::type * = nullptr>
-  explicit JSONNumber(T s)
-  : JSONValue(JSONValue::Kind::Number), m_data_type(DataType::Signed) {
-m_data.m_signed = s;
-  }
-
-  template ::value>::type * = nullptr>
-  explicit JSONNumber(T d)
-  : JSONValue(JSONValue::Kind::Number), m_data_type(DataType::Double) {
-m_data.m_double = d;
-  }
-
-  ~JSONNumber() override = default;
-
-  JSONNumber(const JSONNumber &s) = delete;
-  JSONNumber &operator=(const JSONNumber &s) = delete;
-
-  void Write(Stream &s) override;
-
-  uint64_t GetAsUnsigned() const;
-
-  int64_t GetAsSigned() const;
-
-  double GetAsDouble() const;
-
-  static bool classof(const JSONValue *V) {
-return V->GetKind() == JSONValue::Kind::Number;
-  }
-
-private:
-  enum class DataType : uint8_t { Unsigned, Signed, Double } m_data_type;
-
-  union {
-uint64_t m_unsigned;
-int64_t m_signed;
-double m_double;
-  } m_data;
-};
-
-class JSONTrue : public JSONValue {
-public:
-  JSONTrue();
-
-  JSONTrue(const JSONTrue &s) = delete;
-  JSONTrue &operator=(const JSONTrue &s) = delete;
-
-  void Write(Stream 

[Lldb-commits] [PATCH] D68289: [lldb-server/android] Show more processes and package name when necessary

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Host/linux/Host.cpp:178-185
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
+#if defined(__ANDROID__)
+// On android we fallback to Arg0, which commonly is an apk package name.
+if (!process_info.GetArg0().empty()) {
+  ExePath = process_info.GetArg0();

wallace wrote:
> labath wrote:
> > Why does this fail on android exactly? Is it some seccomp thingy? I'm 
> > surprised that we're allowed to read `/proc/cmdline`, but not the 
> > `/proc/exe` link..
> > 
> > Anyway, I don't think that pretending that "com.my.app" is an executable 
> > file name is a good idea. Maybe it would be better to implement something 
> > on the other end. I.e., we print out argv[0]  if the file name is not 
> > present?
> From what I've been seeing, /proc/cmdline is always available, even for root 
> processes. On the other hand, /proc/exe is only available for the user that 
> is seeing it. In fact, if you use run-as, you can see the contents of 
> /proc/exe, but that would involve making too many slow calls (run-as is slow).
> 
> I do think that having "com.my.app" as executable name is okay, because 
> android doesn't let you interact with an apk by any other means, unless you 
> root your device. When using android commands, "com.my.app" is just like any 
> other process.
> 
> I don't have any strong opinions though, so I'm okay with following your 
> suggestion if you prefer so.
> 
> Handling the android case from the host by doing exePath =  argv[0] would 
> look like this:
> 
> - the Arg0 fallback is done inside the client when the exe path is empty 
> - a special additional handling would be required for cases when lldb is 
> running on Android. With the approach of this diff, that case is covered.
> - the current function, in the case of android, returns a possible empty exe 
> path. For anything not android, empty exe paths are not allowed
> 
> 
> what do you think?
Having "com.my.app" as an executable (more like, process)  name is probably 
fine. However, having that as an executable _path_ (of type FileSpec!) is 
streching it too far, I believe. If this function was returning some 
higher-level concept of a process, which had a "name" as a separate property, 
then putting argv[0] there would be fine. However, for something of type 
FileSpec, I think it's better to have nothing rather than something that is 
definitely _not_ a file.

> the Arg0 fallback is done inside the client when the exe path is empty
agreed, and this doesn't need to be android-specific

> a special additional handling would be required for cases when lldb is 
> running on Android. With the approach of this diff, that case is covered.
I am not sure what you mean by that. Can you elaborate?

> the current function, in the case of android, returns a possible empty exe 
> path. For anything not android, empty exe paths are not allowed
yes, and I think we should, like for the case of environment, just permit a 
ProcessInstanceInfo result with no executable path, regardless of the OS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289



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

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

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

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

get rid of CRTP gobbledygook


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68188

Files:
  lldb/include/lldb/API/SBFile.h
  lldb/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
  lldb/scripts/Python/python-typemaps.swig
  lldb/scripts/interface/SBFile.i
  lldb/source/API/SBFile.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

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

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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 222882.

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

https://reviews.llvm.org/D68316

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


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


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

[Lldb-commits] [lldb] r373507 - [ObjectFileMachO] Catch up with FileDesc changes.

[Davide Italiano via lldb-commits]

Author: davide
Date: Wed Oct  2 12:20:15 2019
New Revision: 373507

URL: http://llvm.org/viewvc/llvm-project?rev=373507&view=rev
Log:
[ObjectFileMachO] Catch up with FileDesc changes.

This didn't show up because nobody built __arm64__ in a while.



Modified:
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=373507&r1=373506&r2=373507&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Wed Oct  2 
12:20:15 2019
@@ -2554,8 +2554,8 @@ size_t ObjectFileMachO::ParseSymtab() {
   "dyld_shared_cache_", /* DYLD_SHARED_CACHE_BASE_NAME */
   header_arch.GetArchitectureName(), ".development");
 
-  FileSpec dsc_nondevelopment_filespec(dsc_path, false);
-  FileSpec dsc_development_filespec(dsc_path_development, false);
+  FileSpec dsc_nondevelopment_filespec(dsc_path);
+  FileSpec dsc_development_filespec(dsc_path_development);
   FileSpec dsc_filespec;
 
   UUID dsc_uuid;
@@ -3001,7 +3001,7 @@ size_t ObjectFileMachO::ParseSymtab() {
   // second is the directory for the source
   // file so you end up with a path that looks
   // like "/tmp/src//tmp/src/"
-  FileSpec so_dir(so_path, false);
+  FileSpec so_dir(so_path);
   if (!FileSystem::Instance().Exists(so_dir)) {
 so_dir.SetFile(
 &full_so_path[double_slash_pos + 1],


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

[Lldb-commits] [lldb] r373508 - [ObjectFileMachO] FileSpec::SetFile() now takes the style as arg.

[Davide Italiano via lldb-commits]

Author: davide
Date: Wed Oct  2 12:20:18 2019
New Revision: 373508

URL: http://llvm.org/viewvc/llvm-project?rev=373508&view=rev
Log:
[ObjectFileMachO] FileSpec::SetFile() now takes the style as arg.

Another block that's only compiled on __arm64__ and wasn't
updated.



Modified:
lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=373508&r1=373507&r2=373508&view=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
+++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Wed Oct  2 
12:20:18 2019
@@ -3005,7 +3005,7 @@ size_t ObjectFileMachO::ParseSymtab() {
   if (!FileSystem::Instance().Exists(so_dir)) {
 so_dir.SetFile(
 &full_so_path[double_slash_pos + 1],
-false);
+FileSpec::Style::native);
 if (FileSystem::Instance().Exists(so_dir)) 
{
   // Trim off the incorrect path
   full_so_path.erase(0,


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

[Lldb-commits] [lldb] r373509 - [RegisterContextDarwin_arm64] Include the headers for getsysctlbyname.

[Davide Italiano via lldb-commits]

Author: davide
Date: Wed Oct  2 12:20:21 2019
New Revision: 373509

URL: http://llvm.org/viewvc/llvm-project?rev=373509&view=rev
Log:
[RegisterContextDarwin_arm64] Include the headers for getsysctlbyname.

This code is only used under __arm64__, use the correct guard.



Modified:
lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp

Modified: 
lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp?rev=373509&r1=373508&r2=373509&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp 
Wed Oct  2 12:20:21 2019
@@ -25,6 +25,11 @@
 
 #include 
 
+#if defined(__APPLE__) && (defined(__arm64__) || defined(__aarch64__))
+#include 
+#include 
+#endif
+
 // Support building against older versions of LLVM, this macro was added
 // recently.
 #ifndef LLVM_EXTENSION


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

[Lldb-commits] [lldb] r373510 - [ARM64] XPC services are unsupported on device.

[Davide Italiano via lldb-commits]

Author: davide
Date: Wed Oct  2 12:20:24 2019
New Revision: 373510

URL: http://llvm.org/viewvc/llvm-project?rev=373510&view=rev
Log:
[ARM64] XPC services are unsupported on device.

While around, clean up support for a 8 years old OS.



Modified:
lldb/trunk/source/Host/macosx/objcxx/Host.mm

Modified: lldb/trunk/source/Host/macosx/objcxx/Host.mm
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/objcxx/Host.mm?rev=373510&r1=373509&r2=373510&view=diff
==
--- lldb/trunk/source/Host/macosx/objcxx/Host.mm (original)
+++ lldb/trunk/source/Host/macosx/objcxx/Host.mm Wed Oct  2 12:20:24 2019
@@ -10,8 +10,8 @@
 
 #include 
 
-#if !defined(MAC_OS_X_VERSION_10_7) || 
\
-MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
+// On device doesn't have supporty for XPC.
+#if defined(__APPLE__) && (defined(__arm64__) || defined(__aarch64__))
 #define NO_XPC_SERVICES 1
 #endif
 


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

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

[walter erquinigo via Phabricator via lldb-commits]

wallace created this revision.
wallace added reviewers: labath, xiaobai, aadsm, clayborg.
Herald added subscribers: lldb-commits, atanasyan, kristof.beyls, arichardson, 
sdardis.
Herald added a project: LLDB.

For context: https://reviews.llvm.org/D68293

We need a way to show all the processes on android regardless of the user id. 
When you run `platform process list`, you only see the processes with the same 
user as the user that launched lldb-server. However, it's quite useful to see 
all the processes, though, and it will lay a foundation for full apk debugging 
support from lldb.

Before:

  PIDPARENT USER   TRIPLE   NAME
  == == ==  
  3234   1 aarch64-unknown-linux-android adbd
  8034   3234  aarch64-unknown-linux-android sh
  9096   3234  aarch64-unknown-linux-android sh
  9098   9096  aarch64-unknown-linux-android lldb-server
  (lldb) ^D

Now:

  (lldb) platform process list -x
  205 matching processes were found on "remote-android"
  PIDPARENT USER   TRIPLE   NAME
  == == ==  
  1  0  init
  5241  init
  5251  init
  5311  ueventd
  5681  logd
  5691 aarch64-unknown-linux-android servicemanager
  5701 aarch64-unknown-linux-android hwservicemanager
  5711 aarch64-unknown-linux-android vndservicemanager
  5771 aarch64-unknown-linux-android qseecomd
  580577   aarch64-unknown-linux-android qseecomd
  ...
  23816  979
com.android.providers.calendar
  24600  979com.verizon.mips.services
  27888  979com.hualai
  28043  2378   
com.android.chrome:sandboxed_process0
  31449  979com.att.shm
  31779  979com.samsung.android.authfw
  31846  979
com.samsung.android.server.iris
  32014  979
com.samsung.android.MtpApplication
  32045  979com.samsung.InputEventApp


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68354

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp


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
@@ -2176,8 +2176,7 @@
   if (match_info.GetProcessInfo().EffectiveGroupIDIsValid())
 packet.Printf("egid:%u;",
   match_info.GetProcessInfo().GetEffectiveGroupID());
-  if (match_info.GetProcessInfo().EffectiveGroupIDIsValid())
-packet.Printf("all_users:%u;", match_info.GetMatchAllUsers() ? 1 : 0);
+  packet.Printf("all_users:%u;", match_info.GetMatchAllUsers() ? 1 : 0);
   if (match_info.GetProcessInfo().GetArchitecture().IsValid()) {
 const ArchSpec &match_arch =
 match_info.GetProcessInfo().GetArchitecture();
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -602,6 +602,9 @@
   def platform_process_list_show_args : Option<"show-args", "A">,
 GroupRange<1, 6>,
 Desc<"Show process arguments instead of the process executable basename.">;
+  def platform_process_list_all_users: Option<"all-users", "x">,
+GroupRange<1,6>,
+Desc<"Show processes matching all user IDs.">;
   def platform_process_list_verbose : Option<"verbose", "v">, GroupRange<1, 6>,
 Desc<"Enable verbose output.">;
 }
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1264,6 +1264,10 @@
 verbose = true;
 break;
 
+  case 'x':
+match_info.SetMatchAllUsers(true);
+break;
+
   default:
 llvm_unreachable("Unimplemented option");
   }


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRem

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

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.



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

I miss here comparing that the sections inside `.gnu_debugdata` file match 
sections in the outer file:
```
jankratochvil: "hm I was under the impression that section number should 
remain the same in regular and split debug files" - maybe you are right but 
then we could just also compare the two section tables and if they differ then 
reject whole .gnu_debugdata.  And then we do not have to compare section names 
but it is enough to compare section indexes.
labath: "right but then we could just also compare the two section tables and 
if they differ then reject whole .gnu_debugdata."
labath: I would like that ^
```
Also I do not see here handling `.strtab`, why does it work without moving also 
`.strtab`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66791



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

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

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Btw, I couldn't find a better short command than 'x', but if you have a better 
suggestion, i'd be happy to accept it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68354



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

[Lldb-commits] [PATCH] D68289: [lldb-server/android] Show more processes and package name when necessary

[walter erquinigo via Phabricator via lldb-commits]

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

simplify this diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/Options.td
  lldb/source/Host/linux/Host.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

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
@@ -2176,8 +2176,7 @@
   if (match_info.GetProcessInfo().EffectiveGroupIDIsValid())
 packet.Printf("egid:%u;",
   match_info.GetProcessInfo().GetEffectiveGroupID());
-  if (match_info.GetProcessInfo().EffectiveGroupIDIsValid())
-packet.Printf("all_users:%u;", match_info.GetMatchAllUsers() ? 1 : 0);
+  packet.Printf("all_users:%u;", match_info.GetMatchAllUsers() ? 1 : 0);
   if (match_info.GetProcessInfo().GetArchitecture().IsValid()) {
 const ArchSpec &match_arch =
 match_info.GetProcessInfo().GetArchitecture();
Index: lldb/source/Host/linux/Host.cpp
===
--- lldb/source/Host/linux/Host.cpp
+++ lldb/source/Host/linux/Host.cpp
@@ -144,68 +144,80 @@
   }
 }
 
-static bool GetProcessAndStatInfo(::pid_t pid,
-  ProcessInstanceInfo &process_info,
-  ProcessState &State, ::pid_t &tracerpid) {
-  tracerpid = 0;
-  process_info.Clear();
+static bool GetProcessArgs(::pid_t pid, ProcessInstanceInfo &process_info) {
+  auto BufferOrError = getProcFile(pid, "cmdline");
+  if (!BufferOrError)
+return false;
+  std::unique_ptr Cmdline = std::move(*BufferOrError);
 
+  llvm::StringRef Arg0, Rest;
+  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
+  process_info.SetArg0(Arg0);
+  while (!Rest.empty()) {
+llvm::StringRef Arg;
+std::tie(Arg, Rest) = Rest.split('\0');
+process_info.GetArguments().AppendArgument(Arg);
+  }
+  return true;
+}
+
+static void GetExePathAndArch(::pid_t pid, ProcessInstanceInfo &process_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+  std::string ExePath(PATH_MAX, '\0');
 
   // We can't use getProcFile here because proc/[pid]/exe is a symbolic link.
   llvm::SmallString<64> ProcExe;
   (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
-  std::string ExePath(PATH_MAX, '\0');
 
   ssize_t len = readlink(ProcExe.c_str(), &ExePath[0], PATH_MAX);
-  if (len <= 0) {
+  if (len > 0) {
+ExePath.resize(len);
+  } else {
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
   }
-  ExePath.resize(len);
-
   // If the binary has been deleted, the link name has " (deleted)" appended.
   // Remove if there.
   llvm::StringRef PathRef = ExePath;
   PathRef.consume_back(" (deleted)");
 
-  process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  if (!PathRef.empty()) {
+process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  }
+}
 
+static void GetProcessEnviron(::pid_t pid, ProcessInstanceInfo &process_info) {
   // Get the process environment.
   auto BufferOrError = getProcFile(pid, "environ");
-  if (!BufferOrError)
-return false;
+  if (!BufferOrError) {
+return;
+  }
   std::unique_ptr Environ = std::move(*BufferOrError);
-
-  // Get the command line used to start the process.
-  BufferOrError = getProcFile(pid, "cmdline");
-  if (!BufferOrError)
-return false;
-  std::unique_ptr Cmdline = std::move(*BufferOrError);
-
-  // Get User and Group IDs and get tracer pid.
-  if (!GetStatusInfo(pid, process_info, State, tracerpid))
-return false;
-
-  process_info.SetProcessID(pid);
-  process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
-
   llvm::StringRef Rest = Environ->getBuffer();
   while (!Rest.empty()) {
 llvm::StringRef Var;
 std::tie(Var, Rest) = Rest.split('\0');
 process_info.GetEnvironment().insert(Var);
   }
+}
 
-  llvm::StringRef Arg0;
-  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
-  process_info.SetArg0(Arg0);
-  while (!Rest.empty()) {
-llvm::StringRef Arg;
-std::tie(Arg, Rest) = Rest.split('\0');
-process_info.GetArguments().AppendArgument(Arg);
-  }
+static bool GetProcessAndStatInfo(::pid_t pid,
+  ProcessInstanceInfo &process_info,
+  ProcessState &State, ::pid_t &tracerpid) {
+  tracerpid = 0;
+  process_info.Clear();
+
+  process_info.SetProcessID(pid);
+
+  GetExePa

[Lldb-commits] [PATCH] D68289: [lldb-server/android] Show more processes and package name when necessary

[walter erquinigo via Phabricator via lldb-commits]

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

i'm learning arc...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289

Files:
  lldb/source/Host/linux/Host.cpp

Index: lldb/source/Host/linux/Host.cpp
===
--- lldb/source/Host/linux/Host.cpp
+++ lldb/source/Host/linux/Host.cpp
@@ -144,68 +144,80 @@
   }
 }
 
-static bool GetProcessAndStatInfo(::pid_t pid,
-  ProcessInstanceInfo &process_info,
-  ProcessState &State, ::pid_t &tracerpid) {
-  tracerpid = 0;
-  process_info.Clear();
+static bool GetProcessArgs(::pid_t pid, ProcessInstanceInfo &process_info) {
+  auto BufferOrError = getProcFile(pid, "cmdline");
+  if (!BufferOrError)
+return false;
+  std::unique_ptr Cmdline = std::move(*BufferOrError);
 
+  llvm::StringRef Arg0, Rest;
+  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
+  process_info.SetArg0(Arg0);
+  while (!Rest.empty()) {
+llvm::StringRef Arg;
+std::tie(Arg, Rest) = Rest.split('\0');
+process_info.GetArguments().AppendArgument(Arg);
+  }
+  return true;
+}
+
+static void GetExePathAndArch(::pid_t pid, ProcessInstanceInfo &process_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+  std::string ExePath(PATH_MAX, '\0');
 
   // We can't use getProcFile here because proc/[pid]/exe is a symbolic link.
   llvm::SmallString<64> ProcExe;
   (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
-  std::string ExePath(PATH_MAX, '\0');
 
   ssize_t len = readlink(ProcExe.c_str(), &ExePath[0], PATH_MAX);
-  if (len <= 0) {
+  if (len > 0) {
+ExePath.resize(len);
+  } else {
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
   }
-  ExePath.resize(len);
-
   // If the binary has been deleted, the link name has " (deleted)" appended.
   // Remove if there.
   llvm::StringRef PathRef = ExePath;
   PathRef.consume_back(" (deleted)");
 
-  process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  if (!PathRef.empty()) {
+process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  }
+}
 
+static void GetProcessEnviron(::pid_t pid, ProcessInstanceInfo &process_info) {
   // Get the process environment.
   auto BufferOrError = getProcFile(pid, "environ");
-  if (!BufferOrError)
-return false;
+  if (!BufferOrError) {
+return;
+  }
   std::unique_ptr Environ = std::move(*BufferOrError);
-
-  // Get the command line used to start the process.
-  BufferOrError = getProcFile(pid, "cmdline");
-  if (!BufferOrError)
-return false;
-  std::unique_ptr Cmdline = std::move(*BufferOrError);
-
-  // Get User and Group IDs and get tracer pid.
-  if (!GetStatusInfo(pid, process_info, State, tracerpid))
-return false;
-
-  process_info.SetProcessID(pid);
-  process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
-
   llvm::StringRef Rest = Environ->getBuffer();
   while (!Rest.empty()) {
 llvm::StringRef Var;
 std::tie(Var, Rest) = Rest.split('\0');
 process_info.GetEnvironment().insert(Var);
   }
+}
 
-  llvm::StringRef Arg0;
-  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
-  process_info.SetArg0(Arg0);
-  while (!Rest.empty()) {
-llvm::StringRef Arg;
-std::tie(Arg, Rest) = Rest.split('\0');
-process_info.GetArguments().AppendArgument(Arg);
-  }
+static bool GetProcessAndStatInfo(::pid_t pid,
+  ProcessInstanceInfo &process_info,
+  ProcessState &State, ::pid_t &tracerpid) {
+  tracerpid = 0;
+  process_info.Clear();
+
+  process_info.SetProcessID(pid);
+
+  GetExePathAndArch(pid, process_info);
+  GetProcessArgs(pid, process_info);
+  GetProcessEnviron(pid, process_info);
+
+  // Get User and Group IDs and get tracer pid.
+  if (!GetStatusInfo(pid, process_info, State, tracerpid))
+return false;
 
   return true;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

I removed all android-specific logic from this diff, and relaxed most of the 
checks inside GetProcessAndStatInfo so that more processes are displayed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289



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

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

[walter erquinigo via Phabricator via lldb-commits]

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

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68289

Files:
  lldb/source/Host/linux/Host.cpp

Index: lldb/source/Host/linux/Host.cpp
===
--- lldb/source/Host/linux/Host.cpp
+++ lldb/source/Host/linux/Host.cpp
@@ -144,68 +144,79 @@
   }
 }
 
-static bool GetProcessAndStatInfo(::pid_t pid,
-  ProcessInstanceInfo &process_info,
-  ProcessState &State, ::pid_t &tracerpid) {
-  tracerpid = 0;
-  process_info.Clear();
+static void GetProcessArgs(::pid_t pid, ProcessInstanceInfo &process_info) {
+  auto BufferOrError = getProcFile(pid, "cmdline");
+  if (!BufferOrError)
+return;
+  std::unique_ptr Cmdline = std::move(*BufferOrError);
+
+  llvm::StringRef Arg0, Rest;
+  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
+  process_info.SetArg0(Arg0);
+  while (!Rest.empty()) {
+llvm::StringRef Arg;
+std::tie(Arg, Rest) = Rest.split('\0');
+process_info.GetArguments().AppendArgument(Arg);
+  }
+}
 
+static void GetExePathAndArch(::pid_t pid, ProcessInstanceInfo &process_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+  std::string ExePath(PATH_MAX, '\0');
 
   // We can't use getProcFile here because proc/[pid]/exe is a symbolic link.
   llvm::SmallString<64> ProcExe;
   (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
-  std::string ExePath(PATH_MAX, '\0');
 
   ssize_t len = readlink(ProcExe.c_str(), &ExePath[0], PATH_MAX);
-  if (len <= 0) {
+  if (len > 0) {
+ExePath.resize(len);
+  } else {
 LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
  Status(errno, eErrorTypePOSIX));
-return false;
+ExePath.resize(0);
   }
-  ExePath.resize(len);
-
   // If the binary has been deleted, the link name has " (deleted)" appended.
   // Remove if there.
   llvm::StringRef PathRef = ExePath;
   PathRef.consume_back(" (deleted)");
 
-  process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  if (!PathRef.empty()) {
+process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
+process_info.SetArchitecture(GetELFProcessCPUType(PathRef));
+  }
+}
 
+static void GetProcessEnviron(::pid_t pid, ProcessInstanceInfo &process_info) {
   // Get the process environment.
   auto BufferOrError = getProcFile(pid, "environ");
-  if (!BufferOrError)
-return false;
+  if (!BufferOrError) {
+return;
+  }
   std::unique_ptr Environ = std::move(*BufferOrError);
-
-  // Get the command line used to start the process.
-  BufferOrError = getProcFile(pid, "cmdline");
-  if (!BufferOrError)
-return false;
-  std::unique_ptr Cmdline = std::move(*BufferOrError);
-
-  // Get User and Group IDs and get tracer pid.
-  if (!GetStatusInfo(pid, process_info, State, tracerpid))
-return false;
-
-  process_info.SetProcessID(pid);
-  process_info.GetExecutableFile().SetFile(PathRef, FileSpec::Style::native);
-
   llvm::StringRef Rest = Environ->getBuffer();
   while (!Rest.empty()) {
 llvm::StringRef Var;
 std::tie(Var, Rest) = Rest.split('\0');
 process_info.GetEnvironment().insert(Var);
   }
+}
 
-  llvm::StringRef Arg0;
-  std::tie(Arg0, Rest) = Cmdline->getBuffer().split('\0');
-  process_info.SetArg0(Arg0);
-  while (!Rest.empty()) {
-llvm::StringRef Arg;
-std::tie(Arg, Rest) = Rest.split('\0');
-process_info.GetArguments().AppendArgument(Arg);
-  }
+static bool GetProcessAndStatInfo(::pid_t pid,
+  ProcessInstanceInfo &process_info,
+  ProcessState &State, ::pid_t &tracerpid) {
+  tracerpid = 0;
+  process_info.Clear();
+
+  process_info.SetProcessID(pid);
+
+  GetExePathAndArch(pid, process_info);
+  GetProcessArgs(pid, process_info);
+  GetProcessEnviron(pid, process_info);
+
+  // Get User and Group IDs and get tracer pid.
+  if (!GetStatusInfo(pid, process_info, State, tracerpid))
+return false;
 
   return true;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

FYI, we've root caused some internal lldb asan failures to this patch. I don't 
have a repro yet (my lldb tests seem to be failing locally for unrelated 
reasons), but e.g. `Register/x86-64-read.test` is failing to read all the 
non-general purpose registers:

  (lldb) register read --all
  General Purpose Registers:
 rax = 0x7fffd660
 ... etc. ...
  
  Floating Point Registers:
  42 registers were unavailable.
  
  Advanced Vector Extensions:
  16 registers were unavailable.
  
  Memory Protection Extensions:
  6 registers were unavailable.
  
  (lldb) process continue
  error: Failed to resume process: Resume timed out..

These tests are passing in all other modes, including other sanitizer modes


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68299



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

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

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 222921.
wallace added a comment.
Herald added a subscriber: srhines.

updated the unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68291

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


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

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

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


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

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

[Alex Cameron via Phabricator via lldb-commits]

tetsuo-cpp updated this revision to Diff 222923.
tetsuo-cpp added a comment.

Rebased onto trunk.


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

https://reviews.llvm.org/D68179

Files:
  lldb/tools/debugserver/source/JSON.cpp
  lldb/tools/debugserver/source/JSON.h


Index: lldb/tools/debugserver/source/JSON.h
===
--- lldb/tools/debugserver/source/JSON.h
+++ lldb/tools/debugserver/source/JSON.h
@@ -292,6 +292,8 @@
   JSONValue::SP ParseJSONValue();
 
 protected:
+  JSONValue::SP ParseJSONValue(const std::string &value, const Token &token);
+
   JSONValue::SP ParseJSONObject();
 
   JSONValue::SP ParseJSONArray();
Index: lldb/tools/debugserver/source/JSON.cpp
===
--- lldb/tools/debugserver/source/JSON.cpp
+++ lldb/tools/debugserver/source/JSON.cpp
@@ -516,13 +516,16 @@
   std::string value;
   std::string key;
   while (true) {
-JSONValue::SP value_sp = ParseJSONValue();
+JSONParser::Token token = GetToken(value);
+if (token == JSONParser::Token::ArrayEnd)
+  return JSONValue::SP(array_up.release());
+JSONValue::SP value_sp = ParseJSONValue(value, token);
 if (value_sp)
   array_up->AppendObject(value_sp);
 else
   break;
 
-JSONParser::Token token = GetToken(value);
+token = GetToken(value);
 if (token == JSONParser::Token::Comma) {
   continue;
 } else if (token == JSONParser::Token::ArrayEnd) {
@@ -537,6 +540,11 @@
 JSONValue::SP JSONParser::ParseJSONValue() {
   std::string value;
   const JSONParser::Token token = GetToken(value);
+  return ParseJSONValue(value, token);
+}
+
+JSONValue::SP JSONParser::ParseJSONValue(const std::string &value,
+ const Token &token) {
   switch (token) {
   case JSONParser::Token::ObjectStart:
 return ParseJSONObject();


Index: lldb/tools/debugserver/source/JSON.h
===
--- lldb/tools/debugserver/source/JSON.h
+++ lldb/tools/debugserver/source/JSON.h
@@ -292,6 +292,8 @@
   JSONValue::SP ParseJSONValue();
 
 protected:
+  JSONValue::SP ParseJSONValue(const std::string &value, const Token &token);
+
   JSONValue::SP ParseJSONObject();
 
   JSONValue::SP ParseJSONArray();
Index: lldb/tools/debugserver/source/JSON.cpp
===
--- lldb/tools/debugserver/source/JSON.cpp
+++ lldb/tools/debugserver/source/JSON.cpp
@@ -516,13 +516,16 @@
   std::string value;
   std::string key;
   while (true) {
-JSONValue::SP value_sp = ParseJSONValue();
+JSONParser::Token token = GetToken(value);
+if (token == JSONParser::Token::ArrayEnd)
+  return JSONValue::SP(array_up.release());
+JSONValue::SP value_sp = ParseJSONValue(value, token);
 if (value_sp)
   array_up->AppendObject(value_sp);
 else
   break;
 
-JSONParser::Token token = GetToken(value);
+token = GetToken(value);
 if (token == JSONParser::Token::Comma) {
   continue;
 } else if (token == JSONParser::Token::ArrayEnd) {
@@ -537,6 +540,11 @@
 JSONValue::SP JSONParser::ParseJSONValue() {
   std::string value;
   const JSONParser::Token token = GetToken(value);
+  return ParseJSONValue(value, token);
+}
+
+JSONValue::SP JSONParser::ParseJSONValue(const std::string &value,
+ const Token &token) {
   switch (token) {
   case JSONParser::Token::ObjectStart:
 return ParseJSONObject();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 222935.
shafik marked 14 inline comments as done.
shafik added a comment.

Updates based on comments:

- Remove use of AsCString()
- Fixed use of old API
- other minor issues


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

https://reviews.llvm.org/D67994

Files:
  include/lldb/Symbol/ClangASTContext.h
  source/Core/Module.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Symbol/ClangASTContext.cpp
  tools/lldb-test/lldb-test.cpp

Index: tools/lldb-test/lldb-test.cpp
===
--- tools/lldb-test/lldb-test.cpp
+++ tools/lldb-test/lldb-test.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+
 #include 
 #include 
 
@@ -169,6 +170,13 @@
 static cl::opt DumpAST("dump-ast",
  cl::desc("Dump AST restored from symbols."),
  cl::sub(SymbolsSubcommand));
+static cl::opt
+DumpClangAST("dump-clang-ast",
+ cl::desc("Dump clang AST restored from symbols."),
+ cl::sub(SymbolsSubcommand));
+static cl::opt
+SymbolName("symbol-name", cl::desc("Symbol to dump complete AST for"),
+   cl::sub(SymbolsSubcommand));
 
 static cl::opt Verify("verify", cl::desc("Verify symbol information."),
 cl::sub(SymbolsSubcommand));
@@ -188,6 +196,7 @@
 static Error findVariables(lldb_private::Module &Module);
 static Error dumpModule(lldb_private::Module &Module);
 static Error dumpAST(lldb_private::Module &Module);
+static Error dumpClangAST(lldb_private::Module &Module);
 static Error verify(lldb_private::Module &Module);
 
 static Expected getAction();
@@ -581,11 +590,11 @@
 Error opts::symbols::dumpAST(lldb_private::Module &Module) {
   Module.ParseAllDebugSymbols();
 
-  auto symfile = Module.GetSymbolFile();
+  SymbolFile *symfile = Module.GetSymbolFile();
   if (!symfile)
 return make_string_error("Module has no symbol file.");
 
-  auto type_system_or_err =
+  llvm::Expected type_system_or_err =
   symfile->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
   if (!type_system_or_err)
 return make_string_error("Can't retrieve ClangASTContext");
@@ -599,7 +608,7 @@
   if (!ast_ctx)
 return make_string_error("Can't retrieve AST context.");
 
-  auto tu = ast_ctx->getTranslationUnitDecl();
+  clang::TranslationUnitDecl *tu = ast_ctx->getTranslationUnitDecl();
   if (!tu)
 return make_string_error("Can't retrieve translation unit declaration.");
 
@@ -608,6 +617,30 @@
   return Error::success();
 }
 
+Error opts::symbols::dumpClangAST(lldb_private::Module &Module) {
+  Module.ParseAllDebugSymbols();
+
+  SymbolFile *symfile = Module.GetSymbolFile();
+  if (!symfile)
+return make_string_error("Module has no symbol file.");
+
+  llvm::Expected type_system_or_err =
+  symfile->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
+  if (!type_system_or_err)
+return make_string_error("Can't retrieve ClangASTContext");
+
+  auto *clang_ast_ctx =
+  llvm::dyn_cast_or_null(&type_system_or_err.get());
+  if (!clang_ast_ctx)
+return make_string_error("Retrieved TypeSystem was not a ClangASTContext");
+
+  StreamString Stream;
+  clang_ast_ctx->DumpFromSymbolFile(Stream, SymbolName);
+  outs() << Stream.GetData() << "\n";
+
+  return Error::success();
+}
+
 Error opts::symbols::verify(lldb_private::Module &Module) {
   SymbolFile *symfile = Module.GetSymbolFile();
   if (!symfile)
@@ -686,6 +719,17 @@
 return dumpAST;
   }
 
+  if (DumpClangAST) {
+if (Find != FindType::None)
+  return make_string_error("Cannot both search and dump clang AST.");
+if (Regex || !Context.empty() || !Name.empty() || !File.empty() ||
+Line != 0)
+  return make_string_error(
+  "-regex, -context, -name, -file and -line options are not "
+  "applicable for dumping clang AST.");
+return dumpClangAST;
+  }
+
   if (Regex && !Context.empty())
 return make_string_error(
 "Cannot search using both regular expressions and context.");
Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -8992,6 +8992,45 @@
   tu->dump(s.AsRawOstream());
 }
 
+void ClangASTContext::DumpFromSymbolFile(Stream &s,
+ llvm::StringRef symbol_name) {
+  SymbolFile *symfile = GetSymbolFile();
+
+  if (!symfile)
+return;
+
+  lldb_private::TypeList type_list;
+  symfile->GetTypes(nullptr, eTypeClassAny, type_list);
+  size_t ntypes = type_list.GetSize();
+
+  for (size_t i = 0; i < ntypes; ++i) {
+TypeSP type = type_list.GetTypeAtIndex(i);
+
+if (!symbol_name.empty())
+  if (symbol_name.compare(type->GetName().GetStringRef()) != 0)
+continue;
+
+s << type->Ge

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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: friss, aprantl, thegameg.
Herald added subscribers: llvm-commits, mgorny.
Herald added projects: LLDB, LLVM.

This patch reimplements command line option parsing in dsymutil with Tablegen 
and libOption. The main motivation for this change is to prevent clashes with 
other cl::opt options defined in llvm. Although it's a bit more heavyweight, it 
has some nice advantages such as no global static initializers and better 
separation between the code and the option definitions.

I also used this opportunity to improve how dsymutil deals with incompatible 
options. Instead of having checks spread across the code, everything is now 
grouped together in `verifyOptions`. The fact that the options are no longer 
global means that we need to pass them around a bit more, but I think it's 
worth the tradeoff.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68361

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

Index: llvm/tools/dsymutil/dsymutil.cpp
===
--- llvm/tools/dsymutil/dsymutil.cpp
+++ llvm/tools/dsymutil/dsymutil.cpp
@@ -20,12 +20,16 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/DebugInfo/DIContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFVerifier.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/MachO.h"
+#include "llvm/Option/Arg.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
@@ -43,137 +47,227 @@
 #include 
 
 using namespace llvm;
-using namespace llvm::cl;
 using namespace llvm::dsymutil;
 using namespace object;
 
-static OptionCategory DsymCategory("Specific Options");
-static opt Help("h", desc("Alias for -help"), Hidden);
-static opt Version("v", desc("Alias for -version"), Hidden);
-
-static list InputFiles(Positional, OneOrMore,
-desc(""), cat(DsymCategory));
-
-static opt
-OutputFileOpt("o",
-  desc("Specify the output file. default: .dwarf"),
-  value_desc("filename"), cat(DsymCategory));
-static alias OutputFileOptA("out", desc("Alias for -o"),
-aliasopt(OutputFileOpt));
-
-static opt OsoPrependPath(
-"oso-prepend-path",
-desc("Specify a directory to prepend to the paths of object files."),
-value_desc("path"), cat(DsymCategory));
-
-static opt Assembly(
-"S",
-desc("Output textual assembly instead of a binary dSYM companion file."),
-init(false), cat(DsymCategory), cl::Hidden);
-
-static opt DumpStab(
-"symtab",
-desc("Dumps the symbol table found in executable or object file(s) and\n"
- "exits."),
-init(false), cat(DsymCategory));
-static alias DumpStabA("s", desc("Alias for --symtab"), aliasopt(DumpStab));
-
-static opt FlatOut("flat",
- desc("Produce a flat dSYM file (not a bundle)."),
- init(false), cat(DsymCategory));
-static alias FlatOutA("f", desc("Alias for --flat"), aliasopt(FlatOut));
-
-static opt Minimize(
-"minimize",
-desc("When used when creating a dSYM file with Apple accelerator tables,\n"
- "this option will suppress the emission of the .debug_inlines, \n"
- ".debug_pubnames, and .debug_pubtypes sections since dsymutil \n"
- "has better equivalents: .apple_names and .apple_types. When used in\n"
- "conjunction with --update option, this option will cause redundant\n"
- "accelerator tables to be removed."),
-init(false), cat(DsymCategory));
-static alias MinimizeA("z", desc("Alias for --minimize"), aliasopt(Minimize));
-
-static opt Update(
-"update",
-desc("Updates existing dSYM files to contain the latest accelerator\n"
- "tables and other DWARF optimizations."),
-init(false), cat(DsymCategory));
-static alias UpdateA("u", desc("Alias for --update"), aliasopt(Update));
-
-static opt SymbolMap(
-"symbol-map",
-desc("Updates the existing dSYMs inplace using symbol map specified."),
-value_desc("bcsymbolmap"), cat(DsymCategory));
-
-static cl::opt AcceleratorTable(
-"accelerator", cl::desc("Output accelerator tables."),
-cl::values(clEnumValN(AccelTableKind::Default, "Default",
-  "Default for input."),
-   clEnumValN(AccelTableKind::Apple, "Apple", "Apple"),
-   clEnumValN(AccelTableKind::Dwarf, "Dwarf", "DWARF")),
-cl::init(AccelTableKind::Default), cat(DsymCategory));
-
-static opt NumThreads(
-"num-threads",
-desc("Specifies the maximum number (n) of simultaneous threads

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

[Alex Cameron via Phabricator via lldb-commits]

tetsuo-cpp added a comment.

Thank you for the suggestions!

I'm assuming that since only the `debugserver` portion of this change is left, 
I should write a test specifically for that. I had a quick look and it wasn't 
obvious how to do that but I will spend some more time on it this weekend.

@jingham
Regarding the idea of a serialization test, I've only ever triggered the bug by 
manually adding an empty array to some breakpoint JSON. I'm not sure how the 
reporter stumbled across the issue "naturally". Is it ok to have a 
serialization test where the test itself edits the JSON after it has been 
written? Or would it be better if I find some scenario where LLDB writes JSON 
with an empty array and trigger the bug by reading it back in?


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

https://reviews.llvm.org/D68179



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

[Lldb-commits] [PATCH] D67831: [lldb] Get the TargetAPI lock in SBProcess::IsInstrumentationRuntimePresen

[Jim Ingham via Phabricator via lldb-commits]

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

LGTM


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D67831



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

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

[Jim Ingham via Phabricator via lldb-commits]

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

I want to reuse these options (break set's -P -k -v) in another command (thread 
step-scripted) in a future commit, so I broke them off into their own 
OptionGroup.  I made the short options settable because you can never tell 
whether there's going to be the same three open slots in every command.  For 
instance, "thread step-scripted" uses -C not -P, which will be more correct 
if/when we have another script interpreter.  But -C was already taken for 
"break set"...

I added some tests for the parsing of these options.  In the course of adding 
those tests I discovered that if an OptionGroup returns an error, and the 
option is not the last option in the line, the error is ignored.  I fixed that 
as well - that's the change to Options.cpp


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68363

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

Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -1380,6 +1380,10 @@
? nullptr
: OptionParser::GetOptionArgument(),
execution_context);
+  // If the Option setting returned an error, we should stop parsing
+  // and return the error.
+  if (error.Fail())
+break;  
 } else {
   error.SetErrorStringWithFormat("invalid option with value '%i'", val);
 }
Index: lldb/source/Interpreter/CMakeLists.txt
===
--- lldb/source/Interpreter/CMakeLists.txt
+++ lldb/source/Interpreter/CMakeLists.txt
@@ -20,6 +20,7 @@
   OptionGroupBoolean.cpp
   OptionGroupFile.cpp
   OptionGroupFormat.cpp
+  OptionGroupPythonClassWithDict.cpp
   OptionGroupOutputFile.cpp
   OptionGroupPlatform.cpp
   OptionGroupString.cpp
Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -163,17 +163,6 @@
 "match against all source files, pass the \"--all-files\" option.">;
   def breakpoint_set_all_files : Option<"all-files", "A">, Group<9>,
 Desc<"All files are searched for source pattern matches.">;
-  def breakpoint_set_python_class : Option<"python-class", "P">, Group<11>,
-Arg<"PythonClass">, Required, Desc<"The name of the class that implement "
-"a scripted breakpoint.">;
-  def breakpoint_set_python_class_key : Option<"python-class-key", "k">,
-Group<11>, Arg<"None">,
-Desc<"The key for a key/value pair passed to the class that implements a "
-"scripted breakpoint.  Can be specified more than once.">;
-  def breakpoint_set_python_class_value : Option<"python-class-value", "v">,
-Group<11>, Arg<"None">, Desc<"The value for the previous key in the pair "
-"passed to the class that implements a scripted breakpoint. "
-"Can be specified more than once.">;
   def breakpoint_set_language_exception : Option<"language-exception", "E">,
 Group<10>, Arg<"Language">, Required,
 Desc<"Set the breakpoint on exceptions thrown by the specified language "
Index: lldb/source/Commands/CommandObjectBreakpoint.cpp
===
--- lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -16,6 +16,7 @@
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionArgParser.h"
+#include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
 #include "lldb/Interpreter/OptionValueBoolean.h"
 #include "lldb/Interpreter/OptionValueString.h"
 #include "lldb/Interpreter/OptionValueUInt64.h"
@@ -241,12 +242,16 @@
 interpreter, "breakpoint set",
 "Sets a breakpoint or set of breakpoints in the executable.",
 "breakpoint set "),
+m_python_class_options("scripted breakpoint", 'P'),
 m_bp_opts(), m_options() {
   // We're picking up all the normal options, commands and disable.
+  m_all_options.Append(&m_python_class_options, LLDB_OPT_SET_1,
+   LLDB_OPT_SET_11);
   m_all_options.Append(&m_bp_opts, 
LLDB_OPT_SET_1 | LLDB_OPT_SET_3 | LLDB_OPT_SET_4, 
LLDB_OPT_SET_ALL);
-  m_all_options.Append(&m_dum

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

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added reviewers: labath, JDevlieghere, aprantl.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
jingham added a parent revision: D68363: Segregate the Python class + key/value 
dictionary into a separate OptionGroup.

It is trivial to write a ScriptedThreadPlan to do "step until the variable 
'foo' changes value".  It is equally easy to write one that does "step until 
the variable 'bar' changes value".  But you would have to write a different 
Python class for each of these tasks, since there's no way to pass parameters 
into the ThreadPlan on creation.

This patch adds that ability.  You can say:

(lldb) thread step-scripted -C Module.ThreadPlan -k variable_name -v foo

lldb will package the key and value into an SBStructuredData dictionary, and 
pass that to the __init__ method of the ThreadPlan class.  Then the ThreadPlan 
can fetch the value for that key from the dictionary and watch that variable.

I also added an SB API that allows you to pass any SBStructuredData to 
SBThread.StepUsingScriptedThreadPlan, if you need to pass in something more 
complex than a simple key/value dictionary.

This revision depends on:

https://reviews.llvm.org/D68363

Not sure who to put on to review this, most of the folks who worked actively on 
the PythonObjects aren't really active anymore...  I added folks who touched 
some of the relevant files recently...


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68366

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

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

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

[Jim Ingham via Phabricator via lldb-commits]

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

Missed a bit that removed m_class_name from the old command options (and didn't 
use the new option.)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D68366

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

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

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

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

aadsm created this revision.
aadsm added reviewers: labath, xiaobai, clayborg, lanza.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.

I'd like to install lldb using the install-distribution target with 
LLVM_DISTRIBUTION_COMPONENTS but this is currently not possible as the 
lldb/scripts do not provide any component we can use and install the python 
scripts.
For this effect I created an lldb-scripts target and added the 
install-lldb-scripts llvm install target.

I tested with:
cmake ... -DLLVM_ENABLE_PROJECTS="clang;lldb" 
-DLLVM_DISTRIBUTION_COMPONENTS="lldb;liblldb;lldb-scripts" ...
DESTDIR=... ninja install-distribution

Then checked with bin/lldb -x -o 'script import lldb'

Question:
Since the lldb component is basically useless without at least liblldb would it 
make sense to make it depending on liblldb and this new lldb-scripts components?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68370

Files:
  lldb/scripts/CMakeLists.txt


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


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

[Lldb-commits] [lldb] r373562 - new api class: SBFile

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Wed Oct  2 21:01:07 2019
New Revision: 373562

URL: http://llvm.org/viewvc/llvm-project?rev=373562&view=rev
Log:
new api class: SBFile

Summary:
SBFile is a scripting API wrapper for lldb_private::File

This is the first step in a project to enable arbitrary python
io.IOBase file objects -- including those that override the read()
and write() methods -- to be used as the main debugger IOStreams.

Currently this is impossible because python file objects must first
be converted into FILE* streams by SWIG in order to be passed into
the debugger.

full prototype: https://github.com/smoofra/llvm-project/tree/files

Reviewers: JDevlieghere, jasonmolenda, zturner, jingham, labath

Reviewed By: labath

Subscribers: labath, mgorny, lldb-commits

Tags: #lldb

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

Added:
lldb/trunk/include/lldb/API/SBFile.h
lldb/trunk/scripts/interface/SBFile.i
lldb/trunk/source/API/SBFile.cpp
Modified:
lldb/trunk/include/lldb/API/LLDB.h
lldb/trunk/include/lldb/API/SBDefines.h
lldb/trunk/include/lldb/API/SBError.h
lldb/trunk/include/lldb/Host/File.h

lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py

lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
lldb/trunk/scripts/Python/python-typemaps.swig
lldb/trunk/scripts/lldb.swig
lldb/trunk/source/API/CMakeLists.txt
lldb/trunk/source/API/SBReproducer.cpp
lldb/trunk/source/Host/common/File.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

Modified: lldb/trunk/include/lldb/API/LLDB.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/LLDB.h?rev=373562&r1=373561&r2=373562&view=diff
==
--- lldb/trunk/include/lldb/API/LLDB.h (original)
+++ lldb/trunk/include/lldb/API/LLDB.h Wed Oct  2 21:01:07 2019
@@ -13,8 +13,8 @@
 #include "lldb/API/SBAttachInfo.h"
 #include "lldb/API/SBBlock.h"
 #include "lldb/API/SBBreakpoint.h"
-#include "lldb/API/SBBreakpointName.h"
 #include "lldb/API/SBBreakpointLocation.h"
+#include "lldb/API/SBBreakpointName.h"
 #include "lldb/API/SBBroadcaster.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBCommandReturnObject.h"
@@ -28,6 +28,7 @@
 #include "lldb/API/SBEvent.h"
 #include "lldb/API/SBExecutionContext.h"
 #include "lldb/API/SBExpressionOptions.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/API/SBFileSpec.h"
 #include "lldb/API/SBFileSpecList.h"
 #include "lldb/API/SBFrame.h"

Modified: lldb/trunk/include/lldb/API/SBDefines.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBDefines.h?rev=373562&r1=373561&r2=373562&view=diff
==
--- lldb/trunk/include/lldb/API/SBDefines.h (original)
+++ lldb/trunk/include/lldb/API/SBDefines.h Wed Oct  2 21:01:07 2019
@@ -41,6 +41,7 @@ class LLDB_API SBEvent;
 class LLDB_API SBEventList;
 class LLDB_API SBExecutionContext;
 class LLDB_API SBExpressionOptions;
+class LLDB_API SBFile;
 class LLDB_API SBFileSpec;
 class LLDB_API SBFileSpecList;
 class LLDB_API SBFrame;

Modified: lldb/trunk/include/lldb/API/SBError.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBError.h?rev=373562&r1=373561&r2=373562&view=diff
==
--- lldb/trunk/include/lldb/API/SBError.h (original)
+++ lldb/trunk/include/lldb/API/SBError.h Wed Oct  2 21:01:07 2019
@@ -70,6 +70,7 @@ protected:
   friend class SBTrace;
   friend class SBValue;
   friend class SBWatchpoint;
+  friend class SBFile;
 
   lldb_private::Status *get();
 

Added: lldb/trunk/include/lldb/API/SBFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBFile.h?rev=373562&view=auto
==
--- lldb/trunk/include/lldb/API/SBFile.h (added)
+++ lldb/trunk/include/lldb/API/SBFile.h Wed Oct  2 21:01:07 2019
@@ -0,0 +1,38 @@
+//===-- SBFile.h *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SBFile_h_
+#define LLDB_SBFile_h_
+
+#include "lldb/API/SBDefines.h"
+
+namespace lldb {
+
+class LLDB_API SBFile {
+public:
+  SBFile();
+  SBFile(FILE *file, bool transfer_ownership);
+  SBFile(int fd, const char *mode, bool transfer_ownership);
+  ~SBFile();
+
+  SBError Read(uint8_t *buf, size_t num_bytes, size_t *bytes_read);
+  SBError Write(const uint8_t *buf, size_t

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

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373562: new api class: SBFile (authored by lawrence_danna, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D67793?vs=222661&id=222960#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67793

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

Index: lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
+++ lldb/trunk/packages/Python/lldbsuite/test/python_api/default-constructor/TestDefaultConstructorForAPIObjects.py
@@ -194,6 +194,20 @@
 
 @add_test_categories(['pyapi'])
 @no_debug_info_test
+def test_SBFile(self):
+sbf = lldb.SBFile()
+self.assertFalse(sbf.IsValid())
+self.assertFalse(bool(sbf))
+e, n = sbf.Write(b'foo')
+self.assertTrue(e.Fail())
+self.assertEqual(n, 0)
+buffer = bytearray(100)
+e, n = sbf.Read(buffer)
+self.assertEqual(n, 0)
+self.assertTrue(e.Fail())
+
+@add_test_categories(['pyapi'])
+@no_debug_info_test
 def test_SBInstruction(self):
 obj = lldb.SBInstruction()
 if self.TraceOn():
Index: lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -0,0 +1,150 @@
+"""
+Test lldb Python API for file handles.
+"""
+
+from __future__ import print_function
+
+import os
+import io
+import re
+import sys
+from contextlib import contextmanager
+
+import lldb
+from lldbsuite.test import  lldbtest
+from lldbsuite.test.decorators import add_test_categories, no_debug_info_test
+
+
+def readStrippedLines(f):
+def i():
+for line in f:
+line = line.strip()
+if line:
+yield line
+return list(i())
+
+
+class FileHandleTestCase(lldbtest.TestBase):
+
+mydir = lldbtest.Base.compute_mydir(__file__)
+
+# The way this class interacts with the debugger is different
+# than normal.   Most of these test cases will mess with the
+# debugger I/O streams, so we want a fresh debugger for each
+# test so those mutations don't interfere with each other.
+#
+# Also, the way normal tests evaluate debugger commands is
+# by using a SBCommandInterpreter directly, which captures
+# the output in a result object.   For many of tests tests
+# we want the debugger to write the  output directly to
+# its I/O streams like it would have done interactively.
+#
+# For this reason we also define handleCmd() here, even though
+# it is similar to runCmd().
+
+def setUp(self):
+super(FileHandleTestCase, self).setUp()
+self.debugger = lldb.SBDebugger.Create()
+self.out_filename = self.getBuildArtifact('output')
+self.in_filename = self.getBuildArtifact('input')
+
+def tearDown(self):
+lldb.SBDebugger.Destroy(self.debugger)
+super(FileHandleTestCase, self).tearDown()
+for name in (self.out_filename, self.in_filename):
+if os.path.exists(name):
+os.unlink(name)
+
+# Similar to runCmd(), but this uses the per-test debugger, and it
+# supports, letting the debugger just print the results instead
+# of collecting them.
+def handleCmd(self, cmd, check=True, collect_result=True):
+assert not check or collect_result
+ret = lldb.SBCommandReturnObject()
+if collect_result:
+interpreter = self.debugger.GetCommandInterpreter()
+interpreter.HandleCommand(cmd, ret)
+else:
+self.debugger.HandleCommand(cmd)
+if collect_result an

[Lldb-commits] [lldb] r373563 - SBDebugger::SetInputFile, SetOutputFile, etc.

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Wed Oct  2 21:04:48 2019
New Revision: 373563

URL: http://llvm.org/viewvc/llvm-project?rev=373563&view=rev
Log:
SBDebugger::SetInputFile, SetOutputFile, etc.

Summary:
Add new methods to SBDebugger to set IO files as SBFiles instead of
as FILE* streams.

In future commits, the FILE* methods will be deprecated and these
will become the primary way to set the debugger I/O streams.

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/API/SBDebugger.h
lldb/trunk/include/lldb/API/SBFile.h
lldb/trunk/include/lldb/Core/Debugger.h

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

Modified: lldb/trunk/include/lldb/API/SBDebugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBDebugger.h?rev=373563&r1=373562&r2=373563&view=diff
==
--- lldb/trunk/include/lldb/API/SBDebugger.h (original)
+++ lldb/trunk/include/lldb/API/SBDebugger.h Wed Oct  2 21:04:48 2019
@@ -88,6 +88,18 @@ public:
 
   FILE *GetErrorFileHandle();
 
+  SBError SetInputFile(SBFile file);
+
+  SBError SetOutputFile(SBFile file);
+
+  SBError SetErrorFile(SBFile file);
+
+  SBFile GetInputFile();
+
+  SBFile GetOutputFile();
+
+  SBFile GetErrorFile();
+
   void SaveInputTerminalState();
 
   void RestoreInputTerminalState();

Modified: lldb/trunk/include/lldb/API/SBFile.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBFile.h?rev=373563&r1=373562&r2=373563&view=diff
==
--- lldb/trunk/include/lldb/API/SBFile.h (original)
+++ lldb/trunk/include/lldb/API/SBFile.h Wed Oct  2 21:04:48 2019
@@ -14,6 +14,8 @@
 namespace lldb {
 
 class LLDB_API SBFile {
+  friend class SBDebugger;
+
 public:
   SBFile();
   SBFile(FILE *file, bool transfer_ownership);
@@ -31,6 +33,7 @@ public:
 
 private:
   FileSP m_opaque_sp;
+  SBFile(FileSP file_sp);
 };
 
 } // namespace lldb

Modified: lldb/trunk/include/lldb/Core/Debugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=373563&r1=373562&r2=373563&view=diff
==
--- lldb/trunk/include/lldb/Core/Debugger.h (original)
+++ lldb/trunk/include/lldb/Core/Debugger.h Wed Oct  2 21:04:48 2019
@@ -132,12 +132,11 @@ public:
 
   repro::DataRecorder *GetInputRecorder();
 
-  void SetInputFileHandle(FILE *fh, bool tranfer_ownership,
-  repro::DataRecorder *recorder = nullptr);
+  void SetInputFile(lldb::FileSP file, repro::DataRecorder *recorder = 
nullptr);
 
-  void SetOutputFileHandle(FILE *fh, bool tranfer_ownership);
+  void SetOutputFile(lldb::FileSP file);
 
-  void SetErrorFileHandle(FILE *fh, bool tranfer_ownership);
+  void SetErrorFile(lldb::FileSP file);
 
   void SaveInputTerminalState();
 

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py?rev=373563&r1=373562&r2=373563&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
 Wed Oct  2 21:04:48 2019
@@ -12,9 +12,19 @@ from contextlib import contextmanager
 
 import lldb
 from lldbsuite.test import  lldbtest
-from lldbsuite.test.decorators import add_test_categories, no_debug_info_test
+from lldbsuite.test.decorators import (
+add_test_categories, no_debug_info_test, skipIf)
 
 
+@contextmanager
+def replace_stdout(new):
+old = sys.stdout
+sys.stdout = new
+try:
+yield
+finally:
+sys.stdout = old
+
 def readStrippedLines(f):
 def i():
 for line in f:
@@ -66,6 +76,8 @@ class FileHandleTestCase(lldbtest.TestBa
 interpreter.HandleCommand(cmd, ret)
 else:
 self.debugger.HandleCommand(cmd)
+self.debugger.GetOutputFile().Flush()
+self.debugger.GetErrorFile().Flush()
 if collect_result and check:
 self.assertTrue(ret.Succeeded())
 return ret.GetOutput()
@@ -97,6 +109,19 @@ class FileHandleTestCase(lldbtest.TestBa
 with open(self.out_filename, 'r') as f:
 self.assertIn('deadbeef', f.read())
 
+@add_test_categories(['pyapi'])
+@no_debug_info_test
+def

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

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373563: SBDebugger::SetInputFile, SetOutputFile, etc. 
(authored by lawrence_danna, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68181?vs=222664&id=222961#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68181

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

Index: lldb/trunk/scripts/interface/SBDebugger.i
===
--- lldb/trunk/scripts/interface/SBDebugger.i
+++ lldb/trunk/scripts/interface/SBDebugger.i
@@ -183,6 +183,24 @@
 FILE *
 GetErrorFileHandle ();
 
+SBError
+SetInputFile (SBFile file);
+
+SBError
+SetOutputFile (SBFile file);
+
+SBError
+SetErrorFile (SBFile file);
+
+SBFile
+GetInputFile ();
+
+SBFile
+GetOutputFile ();
+
+SBFile
+GetErrorFile ();
+
 lldb::SBCommandInterpreter
 GetCommandInterpreter ();
 
Index: lldb/trunk/include/lldb/Core/Debugger.h
===
--- lldb/trunk/include/lldb/Core/Debugger.h
+++ lldb/trunk/include/lldb/Core/Debugger.h
@@ -132,12 +132,11 @@
 
   repro::DataRecorder *GetInputRecorder();
 
-  void SetInputFileHandle(FILE *fh, bool tranfer_ownership,
-  repro::DataRecorder *recorder = nullptr);
+  void SetInputFile(lldb::FileSP file, repro::DataRecorder *recorder = nullptr);
 
-  void SetOutputFileHandle(FILE *fh, bool tranfer_ownership);
+  void SetOutputFile(lldb::FileSP file);
 
-  void SetErrorFileHandle(FILE *fh, bool tranfer_ownership);
+  void SetErrorFile(lldb::FileSP file);
 
   void SaveInputTerminalState();
 
Index: lldb/trunk/include/lldb/API/SBFile.h
===
--- lldb/trunk/include/lldb/API/SBFile.h
+++ lldb/trunk/include/lldb/API/SBFile.h
@@ -14,6 +14,8 @@
 namespace lldb {
 
 class LLDB_API SBFile {
+  friend class SBDebugger;
+
 public:
   SBFile();
   SBFile(FILE *file, bool transfer_ownership);
@@ -31,6 +33,7 @@
 
 private:
   FileSP m_opaque_sp;
+  SBFile(FileSP file_sp);
 };
 
 } // namespace lldb
Index: lldb/trunk/include/lldb/API/SBDebugger.h
===
--- lldb/trunk/include/lldb/API/SBDebugger.h
+++ lldb/trunk/include/lldb/API/SBDebugger.h
@@ -88,6 +88,18 @@
 
   FILE *GetErrorFileHandle();
 
+  SBError SetInputFile(SBFile file);
+
+  SBError SetOutputFile(SBFile file);
+
+  SBError SetErrorFile(SBFile file);
+
+  SBFile GetInputFile();
+
+  SBFile GetOutputFile();
+
+  SBFile GetErrorFile();
+
   void SaveInputTerminalState();
 
   void RestoreInputTerminalState();
Index: lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
+++ lldb/trunk/packages/Python/lldbsuite/test/python_api/file_handle/TestFileHandle.py
@@ -12,9 +12,19 @@
 
 import lldb
 from lldbsuite.test import  lldbtest
-from lldbsuite.test.decorators import add_test_categories, no_debug_info_test
+from lldbsuite.test.decorators import (
+add_test_categories, no_debug_info_test, skipIf)
 
 
+@contextmanager
+def replace_stdout(new):
+old = sys.stdout
+sys.stdout = new
+try:
+yield
+finally:
+sys.stdout = old
+
 def readStrippedLines(f):
 def i():
 for line in f:
@@ -66,6 +76,8 @@
 interpreter.HandleCommand(cmd, ret)
 else:
 self.debugger.HandleCommand(cmd)
+self.debugger.GetOutputFile().Flush()
+self.debugger.GetErrorFile().Flush()
 if collect_result and check:
 self.assertTrue(ret.Succeeded())
 return ret.GetOutput()
@@ -97,6 +109,19 @@
 with open(self.out_filename, 'r') as f:
 self.assertIn('deadbeef', f.read())
 
+@add_test_categories(['pyapi'])
+@no_debug_info_test
+def test_legacy_file_err_with_get(self):
+with open(self.out_filename, 'w') as f:
+self.debugger.SetErrorFileHandle(f, False)
+self.handleCmd('lolwut', check=False, collect_result=False)
+self.debugger.GetErrorFileHandle().write('FOOBAR\n')
+lldb.SBDebugger.Destroy(self.debugger)
+with open(self.out_filename, 'r') as f:
+errors = f.read()
+   

[Lldb-commits] [lldb] r373564 - factor out an abstract base class for File

[Lawrence D'Anna via lldb-commits]

Author: lawrence_danna
Date: Wed Oct  2 21:31:46 2019
New Revision: 373564

URL: http://llvm.org/viewvc/llvm-project?rev=373564&view=rev
Log:
factor out an abstract base class for File

Summary:
This patch factors out File as an abstract base
class and moves most of its actual functionality into
a subclass called NativeFile.   In the next patch,
I'm going to be adding subclasses of File that
don't necessarily have any connection to actual OS files,
so they will not inherit from NativeFile.

This patch was split out as a prerequisite for
https://reviews.llvm.org/D68188

Reviewers: JDevlieghere, jasonmolenda, labath

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

Modified:
lldb/trunk/include/lldb/Host/File.h
lldb/trunk/scripts/Python/python-typemaps.swig
lldb/trunk/source/API/SBDebugger.cpp
lldb/trunk/source/API/SBFile.cpp
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Core/StreamFile.cpp
lldb/trunk/source/Host/common/File.cpp
lldb/trunk/source/Host/common/FileSystem.cpp
lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/trunk/source/Target/Process.cpp
lldb/trunk/unittests/Host/FileTest.cpp

Modified: lldb/trunk/include/lldb/Host/File.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/File.h?rev=373564&r1=373563&r2=373564&view=diff
==
--- lldb/trunk/include/lldb/Host/File.h (original)
+++ lldb/trunk/include/lldb/Host/File.h Wed Oct  2 21:31:46 2019
@@ -22,10 +22,12 @@
 namespace lldb_private {
 
 /// \class File File.h "lldb/Host/File.h"
-/// A file class.
+/// An abstract base class for files.
 ///
-/// A file class that divides abstracts the LLDB core from host file
-/// functionality.
+/// Files will often be NativeFiles, which provides a wrapper
+/// around host OS file functionality.   But it
+/// is also possible to subclass file to provide objects that have file
+/// or stream functionality but are not backed by any host OS file.
 class File : public IOObject {
 public:
   static int kInvalidDescriptor;
@@ -49,77 +51,80 @@ public:
   };
 
   static mode_t ConvertOpenOptionsForPOSIXOpen(uint32_t open_options);
+  static uint32_t GetOptionsFromMode(llvm::StringRef mode);
+  static bool DescriptorIsValid(int descriptor) { return descriptor >= 0; };
 
   File()
-  : IOObject(eFDTypeFile), m_descriptor(kInvalidDescriptor),
-m_own_descriptor(false), m_stream(kInvalidStream), m_options(0),
-m_own_stream(false), m_is_interactive(eLazyBoolCalculate),
-m_is_real_terminal(eLazyBoolCalculate),
-m_supports_colors(eLazyBoolCalculate) {}
-
-  File(FILE *fh, bool transfer_ownership)
-  : IOObject(eFDTypeFile), m_descriptor(kInvalidDescriptor),
-m_own_descriptor(false), m_stream(fh), m_options(0),
-m_own_stream(transfer_ownership), m_is_interactive(eLazyBoolCalculate),
+  : IOObject(eFDTypeFile), m_is_interactive(eLazyBoolCalculate),
 m_is_real_terminal(eLazyBoolCalculate),
-m_supports_colors(eLazyBoolCalculate) {}
+m_supports_colors(eLazyBoolCalculate){};
 
-  File(int fd, uint32_t options, bool transfer_ownership)
-  : IOObject(eFDTypeFile), m_descriptor(fd),
-m_own_descriptor(transfer_ownership), m_stream(kInvalidStream),
-m_options(options), m_own_stream(false),
-m_is_interactive(eLazyBoolCalculate),
-m_is_real_terminal(eLazyBoolCalculate) {}
-
-  /// Destructor.
+  /// Read bytes from a file from the current file position into buf.
   ///
-  /// The destructor is virtual in case this class is subclassed.
-  ~File() override;
-
-  bool IsValid() const override {
-return DescriptorIsValid() || StreamIsValid();
-  }
-
-  /// Convert to pointer operator.
+  /// NOTE: This function is NOT thread safe. Use the read function
+  /// that takes an "off_t &offset" to ensure correct operation in multi-
+  /// threaded environments.
   ///
-  /// This allows code to check a File object to see if it contains anything
-  /// valid using code such as:
+  /// \param[out] buf
   ///
-  /// \code
-  /// File file(...);
-  /// if (file)
-  /// { ...
-  /// \endcode
+  /// \param[in,out] num_bytes.
+  ///Pass in the size of buf.  Read will pass out the number
+  ///of bytes read.   Zero bytes read with no error indicates
+  ///EOF.
   ///
   /// \return
-  /// A pointer to this

[Lldb-commits] [PATCH] D68317: factor out an abstract base class for File

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rL373564: factor out an abstract base class for File (authored 
by lawrence_danna, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68317?vs=222851&id=222962#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D68317

Files:
  lldb/trunk/include/lldb/Host/File.h
  lldb/trunk/scripts/Python/python-typemaps.swig
  lldb/trunk/source/API/SBDebugger.cpp
  lldb/trunk/source/API/SBFile.cpp
  lldb/trunk/source/Core/Debugger.cpp
  lldb/trunk/source/Core/StreamFile.cpp
  lldb/trunk/source/Host/common/File.cpp
  lldb/trunk/source/Host/common/FileSystem.cpp
  lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  
lldb/trunk/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
  lldb/trunk/source/Plugins/Process/Darwin/NativeProcessDarwin.cpp
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/unittests/Host/FileTest.cpp

Index: lldb/trunk/unittests/Host/FileTest.cpp
===
--- lldb/trunk/unittests/Host/FileTest.cpp
+++ lldb/trunk/unittests/Host/FileTest.cpp
@@ -31,7 +31,7 @@
   FILE *stream = fdopen(fd, "r");
   ASSERT_TRUE(stream);
 
-  File file(stream, true);
+  NativeFile file(stream, true);
   EXPECT_EQ(file.GetWaitableHandle(), fd);
 }
 
@@ -46,7 +46,7 @@
   llvm::FileRemover remover(name);
   ASSERT_GE(fd, 0);
 
-  File file(fd, File::eOpenOptionWrite, true);
+  NativeFile file(fd, File::eOpenOptionWrite, true);
   ASSERT_TRUE(file.IsValid());
 
   FILE *stream = file.GetStream();
Index: lldb/trunk/source/API/SBDebugger.cpp
===
--- lldb/trunk/source/API/SBDebugger.cpp
+++ lldb/trunk/source/API/SBDebugger.cpp
@@ -289,7 +289,7 @@
 void SBDebugger::SetInputFileHandle(FILE *fh, bool transfer_ownership) {
   LLDB_RECORD_METHOD(void, SBDebugger, SetInputFileHandle, (FILE *, bool), fh,
  transfer_ownership);
-  SetInputFile(std::make_shared(fh, transfer_ownership));
+  SetInputFile((FileSP)std::make_shared(fh, transfer_ownership));
 }
 
 // Shouldn't really be settable after initialization as this could cause lots
@@ -319,7 +319,7 @@
 // FIXME Jonas Devlieghere: shouldn't this error be propagated out to the
 // reproducer somehow if fh is NULL?
 if (fh) {
-  file_sp = std::make_shared(fh, true);
+  file_sp = std::make_shared(fh, true);
 }
   }
 
@@ -335,7 +335,7 @@
 void SBDebugger::SetOutputFileHandle(FILE *fh, bool transfer_ownership) {
   LLDB_RECORD_METHOD(void, SBDebugger, SetOutputFileHandle, (FILE *, bool), fh,
  transfer_ownership);
-  SetOutputFile(std::make_shared(fh, transfer_ownership));
+  SetOutputFile((FileSP)std::make_shared(fh, transfer_ownership));
 }
 
 SBError SBDebugger::SetOutputFile(SBFile file) {
@@ -356,7 +356,7 @@
 void SBDebugger::SetErrorFileHandle(FILE *fh, bool transfer_ownership) {
   LLDB_RECORD_METHOD(void, SBDebugger, SetErrorFileHandle, (FILE *, bool), fh,
  transfer_ownership);
-  SetErrorFile(std::make_shared(fh, transfer_ownership));
+  SetErrorFile((FileSP)std::make_shared(fh, transfer_ownership));
 }
 
 SBError SBDebugger::SetErrorFile(SBFile file) {
Index: lldb/trunk/source/API/SBFile.cpp
===
--- lldb/trunk/source/API/SBFile.cpp
+++ lldb/trunk/source/API/SBFile.cpp
@@ -21,14 +21,14 @@
 SBFile::SBFile() { LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBFile); }
 
 SBFile::SBFile(FILE *file, bool transfer_ownership) {
-  m_opaque_sp = std::make_shared(file, transfer_ownership);
+  m_opaque_sp = std::make_shared(file, transfer_ownership);
 }
 
 SBFile::SBFile(int fd, const char *mode, bool transfer_owndership) {
   LLDB_RECORD_CONSTRUCTOR(SBFile, (int, const char *, bool), fd, mode,
   transfer_owndership);
   auto options = File::GetOptionsFromMode(mode);
-  m_opaque_sp = std::make_shared(fd, options, transfer_owndership);
+  m_opaque_sp = std::make_shared(fd, options, transfer_owndership);
 }
 
 SBError SBFile::Read(uint8_t *buf, size_t num_bytes, size_t *bytes_read) {
Index: lldb/trunk/source/Core/StreamFile.cpp
===
--- lldb/trunk/source/Core/StreamFile.cpp
+++ lldb/trunk/source/Core/StreamFile.cpp
@@ -25,11 +25,11 @@
 
 StreamFile::StreamFile(int fd, bool transfer_ownership) : Stream() {
   m_file_sp =
-  std::make_shared(fd, File::eO

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

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

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

Remove .global _Start


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68069

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

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

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

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

mgorny added inline comments.



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68370



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