[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:305
 if (is_trivially_serializable::value)
-  return;
+  return const_cast(t);
 // We need to make a copy as the original object might go out of scope.

JDevlieghere wrote:
> labath wrote:
> > JDevlieghere wrote:
> > > labath wrote:
> > > > What's up with the `const_cast` ? Should this maybe take a `T &t` 
> > > > argument and let `T` be deduced as `const U` when needed?
> > > I need to decode the template instantiation error for the details, but we 
> > > need to return a non-const T. 
> > It would be good to at least know the reason for that, because const_casts 
> > smell..
> The problem is that you cannot cannot bind a non-const lvalue reference of 
> type `T&` to an rvalue of type `T`.
The usual solution to that is to use "universal" references (` T 
&&t`) plus careful sprinkling with `std::forward`. Would that work here?

Or a separate overload for const and non-const references?



Comment at: lldb/source/API/SBReproducerPrivate.h:80-85
+  FileSpec file = l->GetFile();
+  static auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
+  if (!error_or_file)
+return {};
+  static ReplayData r((*error_or_file)->getBuffer());
+  return {r.GetDeserializer(), r.GetRegistry()};

JDevlieghere wrote:
> labath wrote:
> > Could this be done in the initialization code somewhere (inside 
> > `Reproducer::Initialize`, I guess)? We could avoid static variables and get 
> > better error handling that way...
> Not if we want to keep the `SBProvider` in API instead of Utility. 
Right, of course.

Soo... do it one level higher then? Initialize the object in 
SBReproducer::(Passive)Replay, and use it from here?


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

https://reviews.llvm.org/D77602



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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

In D75750#1971446 , @labath wrote:

> In D75750#1967019 , @fche2 wrote:
>
> > >> So it might be good to have the SymbolVendors use one or more 
> > >> SymbolServer plug-ins.
> > > 
> > > I don't believe we have anything that would require all modules in a 
> > > given target (or whatever) to use the same symbol vendor type.  [...]
> >
> > Just for clarity, is someone proposing to undertake such a rework of that 
> > infrastructure?  It sounds like this is becoming a prerequisite for 
> > Konrad's patch, but if no one's actually doing it, that means Konrad's work 
> > is on hold indefinitely.  Is that the intent?
>
>
> Yes, I believe that is becoming a prerequisite. I believe Konrad is willing 
> to try to implement that, but I have advised him to hold on a bit until the 
> exact details are hashed out.


@labath, I'm not really keen on implementing the architectural changes that you 
mentioned because it will take ages when I do that. And the cross-platform bit 
makes me nervous as well. Initially I hoped we might be able to integrate my 
work and improve on the architecture later. Then we're not fighting on too many 
fronts at the same time?

Shall we maybe move the discussion about the architectural changes to lldb-dev 
instead of this patch? @clayborg @labath @jingham @jankratochvil ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

While getting rid of this would be great, this (the manual dwarf index) is also 
very performance sensitive code which has been optimized and re-optimized a 
number of times. So, I would want to see a benchmark to ensure this does not 
affect performance adversely. A good benchmark would be to take a release 
(without asserts) build of lldb and debug build of clang (without accelerator 
tables) and then run something like `time opt/lldb dbg/clang -o "br set -n 
nonexisting_function" -b` a couple of times. The test should be run on an elf 
binary because the mac debug info model is sufficiently different to skew the 
results, even if you do manage to disable the accelerator tables.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:24-30
+static void TaskMapOverInt(size_t end,
+   const llvm::function_ref &func) {
+  llvm::ThreadPool pool;
+  for (size_t i = 0; i < end; i++)
+pool.async(func, i);
+  pool.wait();
+}

This *is* simpler than the previous implementation, but it may have different 
performance characteristics. The previous implementation was optimized for 
small workloads where the overhead of enqueuing a task was not negligible 
compared to the size of the job itself.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:90-113
+  TaskMapOverInt(units_to_index.size(), extract_fn);
 
   // Now create a task runner that can index each DWARF unit in a
   // separate thread so we can index quickly.
 
-  TaskMapOverInt(0, units_to_index.size(), parser_fn);
+  TaskMapOverInt(units_to_index.size(), parser_fn);
 

All of this should use the same ThreadPool instance to avoid re-creating the 
pool threads a couple of times.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D78337



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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

lldb-dev is indeed a better place for the architectural discussion. However, 
moving the discussion there does not automatically unblock this patch. "get 
something in now and improve the architecture later" almost never works out in 
practice. In fact I would say that adding debuginfod is a good way to cement 
the status quo. The situation around finding symbols is messy enough already 
because one needs to understand the funky mac symbol searching mechanism, which 
is pretty much impossible without a mac machine. After debuginfod, one will 
need to understand both, and have a linux machine with some debuginfod setup. 
The set of such people is likely to be empty of a long time...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk resigned from this revision.
kwk added a comment.
This revision now requires review to proceed.

I resign because I think @labath made some good points that I cannot argue 
about.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D78337



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

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

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

All tests pass.  I first thought that the 
`lldb/test/Shell/Reproducer/TestGDBRemoteRepro.test` test didn't work but it 
seems to be flaky.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:90
   // unit refers to another and the indexes accesses those DIEs.
-  TaskMapOverInt(0, units_to_index.size(), extract_fn);
+  TaskMapOverInt(units_to_index.size(), extract_fn);
 

labath wrote:
> All of this should use the same ThreadPool instance to avoid re-creating the 
> pool threads a couple of times.
@JDevlieghere I assume you've removed the first of the three parameters to 
`TaskMapOverInt` because it was always `0` anyways? If not, shouldn't this be 
easily changeable?

```lang=c++

static void TaskMapOverInt(size_t idx, size_t end,
   const llvm::function_ref &func) {
  llvm::ThreadPool pool;
  for (; idx < end; idx++)
pool.async(func, idx);
  pool.wait();
}
```


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D78337



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

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:24-30
+static void TaskMapOverInt(size_t end,
+   const llvm::function_ref &func) {
+  llvm::ThreadPool pool;
+  for (size_t i = 0; i < end; i++)
+pool.async(func, i);
+  pool.wait();
+}

labath wrote:
> This *is* simpler than the previous implementation, but it may have different 
> performance characteristics. The previous implementation was optimized for 
> small workloads where the overhead of enqueuing a task was not negligible 
> compared to the size of the job itself.
(I'm not saying this should be changed back. llvm's thread pool creates threads 
differently, and that is likely to have been the main cause of overhead, so 
this optimization may not matter anymore. However, it would be good to verify 
that.)


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D78337



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

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Hmm... well, I like how this (active&passive) is unified at the template level. 
It still feels like there's too much code to implement this thing (coming from 
the duplication for const, non-const and static variants), but I'm not sure 
what can be done about that. I'll need to ponder this a bit more




Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:995
+
+template  struct invoke_char_ptr;
+template 

this forward decl is superfluous.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D77759



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

Re: [Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to regenerate this test, if needed.

[Robinson, Paul via lldb-commits]

Hi Davide,

I reread the review, and I see I was confused by two things:
(1) the name of the test is static_scope.s even though what it's testing is the 
scope of a var with a const_value, nothing to do with `static`;
(2) there's this comment in the review: https://reviews.llvm.org/D77698#1973122 
"what's the DWARF'y way to find out whether a variable is static?" suggesting 
you did care about static-scope variables.

I suggest renaming the test to better reflect what it's testing (which has 
nothing to do with the scope of static vars).

Re your question, DWARF doesn't explicitly identify variables as static.  
Variables have scopes, which might be local or might be file-level or might be 
global.  Globals are distinguished from file-level locals with the external 
flag.  Locals and file-level vars simply are in the appropriate scope.  A 
static local should look exactly like a non-static local, except that its 
location expression(s) would not be frame-based.  Static locals are not 
exciting, from a DWARF viewpoint.  (LLVM historically has had trouble putting 
them in the correct scope, but that is a compiler problem not a DWARF problem.)

I hope this resolves the questions/confusion.
Thanks,
--paulr

From: Davidino Italiano 
Sent: Thursday, April 16, 2020 5:26 PM
To: Robinson, Paul 
Cc: lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to 
regenerate this test, if needed.




On Apr 16, 2020, at 2:01 PM, Robinson, Paul 
mailto:paul.robin...@sony.com>> wrote:




-Original Message-
From: lldb-commits 
mailto:lldb-commits-boun...@lists.llvm.org>>
 On Behalf Of
Davide Italiano via lldb-commits
Sent: Thursday, April 16, 2020 4:32 PM
To: lldb-commits@lists.llvm.org
Subject: [Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to
regenerate this test, if needed.


Author: Davide Italiano
Date: 2020-04-16T13:31:32-07:00
New Revision: 1fae85a8534ec51ca893899314bd244b3e9684c7

URL: 
https://github.com/llvm/llvm-
project/commit/1fae85a8534ec51ca893899314bd244b3e9684c7
DIFF: 
https://github.com/llvm/llvm-
project/commit/1fae85a8534ec51ca893899314bd244b3e9684c7.diff

LOG: [DWARF] Add instructions to regenerate this test, if needed.

Added:


Modified:
   lldb/test/Shell/SymbolFile/DWARF/static_scope.s

Removed:



##
##
diff  --git a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
index 84a69e08ecfc..02d497ac9ccb 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/static_scope.s
@@ -3,6 +3,15 @@

# REQUIRES: x86

+# Original test case (for future reference), compiled with:
+# $ clang-10 -g -Og test.c -o test
+# $ cat test.c
+# volatile int a;
+# main() {
+#   int b = 3;

Did you mean this to be `static`?
--paulr


No. Read the the review that introduced this test.


+#   a;
+# }
+
# RUN: llvm-mc -triple=x86_64-apple-macosx10.15.0 -filetype=obj %s > %t.o
# RUN: lldb-test symbols %t.o | FileCheck %s




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

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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a subscriber: fche.
kwk added a comment.

In D75750#1988330 , @labath wrote:

> lldb-dev is indeed a better place for the architectural discussion. However, 
> moving the discussion there does not automatically unblock this patch. "get 
> something in now and improve the architecture later" almost never works out 
> in practice. In fact I would say that adding debuginfod is a good way to 
> cement the status quo.


I get that, but hear me out...

> The situation around finding symbols is messy enough already

The way in which I integrated debuginfod for now is just to find source files 
and not yet symbols. That being said. I don't fear the status quo so much. The 
status quo is probably worse for symbols than it is for source files, don't you 
think? So with *all* the CMake integration, the hosting inside 
`lldb/include/lldb/Host/DebugInfoD.h` and your beloved test case,

testcase 


I think it is fair to say that at least some work is there that can be taken 
into LLDB. **As long** as I fix the retrieval of the module in 
`SourceManager::File::CommonInitializer`. As suggested by @jankratochvil either 
here or on IRC, I would like to give it a shot and try to pass down the correct 
module to this function. I'd say, let's see if this function can be passed a 
Module and if the changes are worth it. The whole part for retrieving debug 
information can come when the architectural changes are done. But then it's a 
piece of cake to extend `lldb/include/lldb/Host/DebugInfoD.h` with the right 
methods to call the debuginfod client lib.

> because one needs to understand the funky mac symbol searching mechanism, 
> which is pretty much impossible without a mac machine.

I'm setting up my old mac to compile LLDB and I guess @jankratochvil might soon 
also have his own Mac. This at least puts us in a position where we can verify 
some of our changes.

> After debuginfod, one will need to understand both, and have a linux machine 
> with some debuginfod setup. The set of such people is likely to be empty of a 
> long time...

I'm not sure if I understand you correctly but to me the *setup* is just to 
point to a machine with *your* or a hosted server. At least for OS binaries 
@fche2 @fche  (which is the correct one?) is making some effort to have those 
debuginfos and source files available and setup. That is a great start for most 
embedded systems with not much disk space to install all debug information I 
guess. Correct my if this is a wrong anticipation. Sure, I mean it will take a 
while before LLDB with debuginfod will make it into a distribution but hey, 
time flies by.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

The current plan discussed with @kwk is to create the new `SymbolServer` 
abstract superclass and some its inherited implementation and move there the 
appropriate parts of existing `lldb/source/Symbol/LocateSymbolFile.cpp`. 
Current `SymbolVendor` implementations would then iterate new `SymbolServer`s 
by the existing `LocateExecutableSymbolFile` function. That may be enough for a 
patch of its own.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750



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

[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

[Shafik Yaghmour via Phabricator via lldb-commits]

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

LGTM

Thank you! This looks like a nice update to the functionality.


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

https://reviews.llvm.org/D78329



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

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/unittests/DataFormatter/StringPrinterTests.cpp:1
+//===-- StringPrinterTests.cpp --*- C++ 
-*-===//
+//

drop the redundant C++ marker


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843



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

[Lldb-commits] [lldb] 681466f - Allow lldb-test to combine -find with -dump-clang-ast

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-04-17T11:01:20-07:00
New Revision: 681466f5e6412350a0b066791450e72325c2c074

URL: 
https://github.com/llvm/llvm-project/commit/681466f5e6412350a0b066791450e72325c2c074
DIFF: 
https://github.com/llvm/llvm-project/commit/681466f5e6412350a0b066791450e72325c2c074.diff

LOG: Allow lldb-test to combine -find with -dump-clang-ast

This patch threads an lldb::DescriptionLevel through the typesystem to
allow dumping the full Clang AST (level=verbose) of any lldb::Type in
addition to the human-readable source description (default
level=full). This type dumping interface is currently not exposed
through the SBAPI.

The application is to let lldb-test dump the clang AST of search
results. I need this to test lazy type completion of clang types in
subsequent patches.

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

Added: 


Modified: 
lldb/include/lldb/Symbol/CompilerType.h
lldb/include/lldb/Symbol/Type.h
lldb/include/lldb/Symbol/TypeMap.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/source/Symbol/CompilerType.cpp
lldb/source/Symbol/Type.cpp
lldb/source/Symbol/TypeMap.cpp
lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h
lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
lldb/tools/lldb-test/lldb-test.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index b0a7953190f8..280966a327ec 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -371,9 +371,15 @@ class CompilerType {
size_t data_byte_size);
 
   /// Dump to stdout.
-  void DumpTypeDescription() const;
-
-  void DumpTypeDescription(Stream *s) const;
+  void DumpTypeDescription(lldb::DescriptionLevel level =
+   lldb::eDescriptionLevelFull) const;
+
+  /// Print a description of the type to a stream. The exact implementation
+  /// varies, but the expectation is that eDescriptionLevelFull returns a
+  /// source-like representation of the type, whereas eDescriptionLevelVerbose
+  /// does a dump of the underlying AST if applicable.
+  void DumpTypeDescription(Stream *s, lldb::DescriptionLevel level =
+  lldb::eDescriptionLevelFull) const;
   /// \}
 
   bool GetValueAsScalar(const DataExtractor &data, lldb::offset_t data_offset,

diff  --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index dfff30029168..8735d016bb22 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -103,7 +103,8 @@ class Type : public std::enable_shared_from_this, 
public UserID {
   // they get an error.
   Type();
 
-  void Dump(Stream *s, bool show_context);
+  void Dump(Stream *s, bool show_context,
+lldb::DescriptionLevel level = lldb::eDescriptionLevelFull);
 
   void DumpTypeName(Stream *s);
 

diff  --git a/lldb/include/lldb/Symbol/TypeMap.h 
b/lldb/include/lldb/Symbol/TypeMap.h
index dd9dbc69f404..67bb65b5faec 100644
--- a/lldb/include/lldb/Symbol/TypeMap.h
+++ b/lldb/include/lldb/Symbol/TypeMap.h
@@ -26,7 +26,8 @@ class TypeMap {
 
   void Clear();
 
-  void Dump(Stream *s, bool show_context);
+  void Dump(Stream *s, bool show_context,
+lldb::DescriptionLevel level = lldb::eDescriptionLevelFull);
 
   TypeMap FindTypes(ConstString name);
 

diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h 
b/lldb/include/lldb/Symbol/TypeSystem.h
index ba2bbfaf4650..e188f29354b8 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -374,11 +374,18 @@ class TypeSystem : public PluginInterface {
  uint32_t bitfield_bit_offset,
  ExecutionContextScope *exe_scope) = 0;
 
-  virtual void
-  DumpTypeDescription(lldb::opaque_compiler_type_t type) = 0; // Dump to stdout
-
-  virtual void DumpTypeDescription(lldb::opaque_compiler_type_t type,
-   Stream *s) = 0;
+  /// Dump the type to stdout.
+  virtual void DumpTypeDescription(
+  lldb::opaque_compiler_type_t type,
+  lldb::DescriptionLevel level = lldb::eDescriptionLevelFull) = 0;
+
+  /// Print a description of the type to a stream. The exact implementation
+  /// varies, but the expectation is that eDescriptionLevelFull returns a
+  /// source-like representation of the type, whereas eDescriptionLevelVerbose
+  /// does a dump of the underlying AST if applicable.
+  virtual void DumpTypeDescription(
+  lldb::opaque_compiler_type_t type, Stream *s,
+  lldb::DescriptionLevel level = lldb::eDescriptionLevelFull) = 0;
 
   // TODO: These methods appear unused. Should they be removed?
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeS

[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

[Adrian Prantl via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG681466f5e641: Allow lldb-test to combine -find with 
-dump-clang-ast (authored by aprantl).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78329

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/Type.h
  lldb/include/lldb/Symbol/TypeMap.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Symbol/Type.cpp
  lldb/source/Symbol/TypeMap.cpp
  lldb/test/Shell/SymbolFile/DWARF/Inputs/ModuleOwnership/A.h
  lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -169,10 +169,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 DumpClangAST(
+"dump-clang-ast",
+cl::desc("Dump clang AST restored from symbols. When used on its own this "
+ "will dump the entire AST of all loaded symbols. When combined "
+ "with -find, it changes the presentation of the search results "
+ "from pretty-printing the types to an AST dump."),
+cl::sub(SymbolsSubcommand));
 
 static cl::opt Verify("verify", cl::desc("Verify symbol information."),
 cl::sub(SymbolsSubcommand));
@@ -192,7 +195,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 dumpEntireClangAST(lldb_private::Module &Module);
 static Error verify(lldb_private::Module &Module);
 
 static Expected getAction();
@@ -404,6 +407,10 @@
   return List.GetVariableAtIndex(0)->GetDeclContext();
 }
 
+static lldb::DescriptionLevel GetDescriptionLevel() {
+  return opts::symbols::DumpClangAST ? eDescriptionLevelVerbose : eDescriptionLevelFull;
+}
+
 Error opts::symbols::findFunctions(lldb_private::Module &Module) {
   SymbolFile &Symfile = *Module.GetSymbolFile();
   SymbolContextList List;
@@ -534,7 +541,12 @@
 
   outs() << formatv("Found {0} types:\n", Map.GetSize());
   StreamString Stream;
-  Map.Dump(&Stream, false);
+  // Resolve types to force-materialize typedef types.
+  Map.ForEach([&](TypeSP &type) {
+type->GetFullCompilerType();
+return false;
+  });
+  Map.Dump(&Stream, false, GetDescriptionLevel());
   outs() << Stream.GetData() << "\n";
   return Error::success();
 }
@@ -615,7 +627,7 @@
   return Error::success();
 }
 
-Error opts::symbols::dumpClangAST(lldb_private::Module &Module) {
+Error opts::symbols::dumpEntireClangAST(lldb_private::Module &Module) {
   Module.ParseAllDebugSymbols();
 
   SymbolFile *symfile = Module.GetSymbolFile();
@@ -719,13 +731,17 @@
   }
 
   if (DumpClangAST) {
-if (Find != FindType::None)
-  return make_string_error("Cannot both search and dump clang AST.");
-if (Regex || !Context.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 (Find == FindType::None) {
+  if (Regex || !Context.empty() || !File.empty() || Line != 0)
+return make_string_error(
+"-regex, -context, -name, -file and -line options are not "
+"applicable for dumping the entire clang AST. Either combine with "
+"-find, or use -dump-clang-ast as a standalone option.");
+  return dumpEntireClangAST;
+}
+if (Find != FindType::Type)
+  return make_string_error("This combination of -dump-clang-ast and -find "
+   " is not yet implemented.");
   }
 
   if (Regex && !Context.empty())
Index: lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
===
--- lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
+++ lldb/test/Shell/SymbolFile/DWARF/module-ownership.mm
@@ -1,17 +1,22 @@
 // RUN: %clang --target=x86_64-apple-macosx -g -gmodules \
 // RUN:-fmodules -fmodules-cache-path=%t.cache \
 // RUN:-c -o %t.o %s -I%S/Inputs
-// RUN: lldb-test symbols -dump-clang-ast %t.o | FileCheck %s
 // Verify that the owning module information from DWARF is preserved in the AST.
 

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 258390.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

- Remove const_cast
- Initialize instrumentation data in `SBReproducer.cpp`


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

https://reviews.llvm.org/D77602

Files:
  lldb/include/lldb/Utility/Reproducer.h
  lldb/include/lldb/Utility/ReproducerInstrumentation.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/API/SBReproducer.cpp
  lldb/source/API/SBReproducerPrivate.h
  lldb/source/Utility/Reproducer.cpp
  lldb/source/Utility/ReproducerInstrumentation.cpp
  lldb/unittests/Utility/ReproducerInstrumentationTest.cpp

Index: lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
===
--- lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
+++ lldb/unittests/Utility/ReproducerInstrumentationTest.cpp
@@ -52,23 +52,62 @@
 
 static llvm::Optional g_registry;
 static llvm::Optional g_serializer;
+static llvm::Optional g_deserializer;
 
-inline InstrumentationData GetTestInstrumentationData() {
+class TestInstrumentationData : public InstrumentationData {
+public:
+  TestInstrumentationData() : InstrumentationData() {}
+  TestInstrumentationData(Serializer &serializer, Registry ®istry)
+  : InstrumentationData(serializer, registry) {}
+  TestInstrumentationData(Deserializer &deserializer, Registry ®istry)
+  : InstrumentationData(deserializer, registry) {}
+};
+
+inline TestInstrumentationData GetTestInstrumentationData() {
+  assert(!(g_serializer && g_deserializer));
   if (g_serializer)
-return InstrumentationData(*g_serializer, *g_registry);
-  return InstrumentationData();
+return TestInstrumentationData(*g_serializer, *g_registry);
+  if (g_deserializer)
+return TestInstrumentationData(*g_deserializer, *g_registry);
+  return TestInstrumentationData();
 }
 
 class TestInstrumentationDataRAII {
-public:
+private:
   TestInstrumentationDataRAII(llvm::raw_string_ostream &os) {
 g_registry.emplace();
 g_serializer.emplace(os);
+g_deserializer.reset();
   }
 
-  ~TestInstrumentationDataRAII() {
+  TestInstrumentationDataRAII(llvm::StringRef buffer) {
+g_registry.emplace();
+g_serializer.reset();
+g_deserializer.emplace(buffer);
+  }
+
+  friend std::unique_ptr
+  std::make_unique(llvm::raw_string_ostream &os);
+  friend std::unique_ptr
+  std::make_unique(llvm::StringRef &buffer);
+
+public:
+  ~TestInstrumentationDataRAII() { Reset(); }
+
+  void Reset() {
 g_registry.reset();
 g_serializer.reset();
+g_deserializer.reset();
+  }
+
+  static std::unique_ptr
+  GetRecordingData(llvm::raw_string_ostream &os) {
+return std::make_unique(os);
+  }
+
+  static std::unique_ptr
+  GetReplayData(llvm::StringRef buffer) {
+return std::make_unique(buffer);
   }
 };
 
@@ -95,11 +134,17 @@
   InstrumentedFoo(const InstrumentedFoo &foo);
   InstrumentedFoo &operator=(const InstrumentedFoo &foo);
   void A(int a);
+  int GetA();
   void B(int &b) const;
+  int &GetB();
   int C(float *c);
+  float GetC();
   int D(const char *d) const;
+  void GetD(char *buffer, size_t length);
   static void E(double e);
+  double GetE();
   static int F();
+  bool GetF();
   void Validate() override;
    }
   virtual bool IsA(Class c) override { return c == Class::Foo; }
@@ -182,35 +227,71 @@
   m_a = a;
 }
 
+int InstrumentedFoo::GetA() {
+  LLDB_RECORD_METHOD_NO_ARGS(int, InstrumentedFoo, GetA);
+
+  return m_a;
+}
+
 void InstrumentedFoo::B(int &b) const {
   LLDB_RECORD_METHOD_CONST(void, InstrumentedFoo, B, (int &), b);
   m_called++;
   m_b = b;
 }
 
+int &InstrumentedFoo::GetB() {
+  LLDB_RECORD_METHOD_NO_ARGS(int &, InstrumentedFoo, GetB);
+
+  return m_b;
+}
+
 int InstrumentedFoo::C(float *c) {
   LLDB_RECORD_METHOD(int, InstrumentedFoo, C, (float *), c);
   m_c = *c;
   return 1;
 }
 
+float InstrumentedFoo::GetC() {
+  LLDB_RECORD_METHOD_NO_ARGS(float, InstrumentedFoo, GetC);
+
+  return m_c;
+}
+
 int InstrumentedFoo::D(const char *d) const {
   LLDB_RECORD_METHOD_CONST(int, InstrumentedFoo, D, (const char *), d);
   m_d = std::string(d);
   return 2;
 }
 
+void InstrumentedFoo::GetD(char *buffer, size_t length) {
+  LLDB_RECORD_METHOD(void, InstrumentedFoo, GetD, (char *, size_t), buffer,
+ length);
+  ::snprintf(buffer, length, "%s", m_d.c_str());
+}
+
 void InstrumentedFoo::E(double e) {
   LLDB_RECORD_STATIC_METHOD(void, InstrumentedFoo, E, (double), e);
   g_e = e;
 }
 
+double InstrumentedFoo::GetE() {
+  LLDB_RECORD_METHOD_NO_ARGS(double, InstrumentedFoo, GetE);
+
+  return g_e;
+}
+
 int InstrumentedFoo::F() {
   LLDB_RECORD_STATIC_METHOD_NO_ARGS(int, InstrumentedFoo, F);
   g_f = true;
   return 3;
 }
 
+bool InstrumentedFoo::GetF() {
+  LLDB_RECORD_METHOD_NO_ARGS(bool, InstrumentedFoo, GetF);
+
+  return g_f;
+}
+
 void InstrumentedFoo::Validate() {
   LLDB_RECORD_METHOD_NO_ARGS(void, InstrumentedFoo, Validate);
   EXPECT_

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: friss, davide, jingham.
mib added a project: LLDB.
Herald added subscribers: lldb-commits, mgorny.
mib edited the summary of this revision.
mib updated this revision to Diff 258402.
mib added a comment.

Reformat patch.


[lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

This patch improves data formatting for CoreFoundation containers:
CFDictionary and CFSet.

These data formatters make the containers and their children appear in Xcode's
variables view (and on the command line) without having to expand the
data structure.

Previous implementation (D48450 ) only 
supported showing the container's element count.

  (lldb) frame var dict
  (__NSCFDictionary *) dict = 0x0001004062b0 2 key/value pairs
  
  (lldb) frame var set
  (__NSCFSet *) set = 0x000100406330 2 elements

Now the variable can be dereferenced to dispaly the container's elements:

  (lldb) frame var *dict
  (__NSCFDictionary) *dict = {
[0] = {
  key = 0x00014050 @"123"
  value = 0x00014090 @"456"
}
[1] = {
  key = 0x00014030 @"abc"
  value = 0x00014070 @"def"
}
  }
  
  (lldb) frame var *set
  (__NSCFSet) *set = {
[0] = 0x00014050 @"123"
[1] = 0x00014030 @"abc"
  }

rdar://39882287

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78396

Files:
  lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp
  lldb/source/Plugins/Language/ObjC/CFBasicHash.h
  lldb/source/Plugins/Language/ObjC/CMakeLists.txt
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.h
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
@@ -376,9 +376,10 @@
 	[newMutableDictionary setObject:@"foo" forKey:@"bar19"];
 	[newMutableDictionary setObject:@"foo" forKey:@"bar20"];
 
-	id cfKeys[2] = { @"foo", @"bar", @"baz", @"quux" };
-	id cfValues[2] = { @"foo", @"bar", @"baz", @"quux" };
+	id cfKeys[4] = { @"foo", @"bar", @"baz", @"quux" };
+	id cfValues[4] = { @"foo", @"bar", @"baz", @"quux" };
 	NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
+  NSDictionary *nscfDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 4, nil, nil));
 	CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
 
 	NSAttributedString* attrString = [[NSAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
@@ -412,6 +413,7 @@
 	NSSet* nsset = [[NSSet alloc] initWithObjects:str1,str2,str3,nil];
 	NSSet *nsmutableset = [[NSMutableSet alloc] initWithObjects:str1,str2,str3,nil];
 	[nsmutableset addObject:str4];
+  NSSet* nscfSet = CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
 
 	CFDataRef data_ref = CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
 
Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
@@ -20,6 +20,7 @@
 self.appkit_tester_impl(self.nscontainers_data_formatter_commands)
 
 def nscontainers_data_formatter_commands(self):
+
 self.expect(
 'frame variable newArray nsDictionary newDictionary nscfDictionary cfDictionaryRef newMutableDictionary cfarray_ref mutable_array_ref',
 substrs=[
@@ -29,6 +30,8 @@
 ' 2 key/value pairs',
 '(NSDictionary *) newDictionary = ',
 ' 12 key/value pairs',
+'(NSDictionary *) nscfDictionary = ',
+' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
 ' 3 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
@@ -39,6 +42,36 @@
 ' @"11 elements"',
 ])
 
+self.expect(
+'frame variable -d run-target *nscfDictionary',
+patterns=[
+'\(__NSCFDictionary\) \*nscfDictionary =',
+'

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 258402.
mib added a comment.

Reformat patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396

Files:
  lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp
  lldb/source/Plugins/Language/ObjC/CFBasicHash.h
  lldb/source/Plugins/Language/ObjC/CMakeLists.txt
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.h
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
@@ -376,9 +376,10 @@
 	[newMutableDictionary setObject:@"foo" forKey:@"bar19"];
 	[newMutableDictionary setObject:@"foo" forKey:@"bar20"];
 
-	id cfKeys[2] = { @"foo", @"bar", @"baz", @"quux" };
-	id cfValues[2] = { @"foo", @"bar", @"baz", @"quux" };
+	id cfKeys[4] = { @"foo", @"bar", @"baz", @"quux" };
+	id cfValues[4] = { @"foo", @"bar", @"baz", @"quux" };
 	NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
+  NSDictionary *nscfDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 4, nil, nil));
 	CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
 
 	NSAttributedString* attrString = [[NSAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
@@ -412,6 +413,7 @@
 	NSSet* nsset = [[NSSet alloc] initWithObjects:str1,str2,str3,nil];
 	NSSet *nsmutableset = [[NSMutableSet alloc] initWithObjects:str1,str2,str3,nil];
 	[nsmutableset addObject:str4];
+  NSSet* nscfSet = CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
 
 	CFDataRef data_ref = CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
 
Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
@@ -20,6 +20,7 @@
 self.appkit_tester_impl(self.nscontainers_data_formatter_commands)
 
 def nscontainers_data_formatter_commands(self):
+
 self.expect(
 'frame variable newArray nsDictionary newDictionary nscfDictionary cfDictionaryRef newMutableDictionary cfarray_ref mutable_array_ref',
 substrs=[
@@ -29,6 +30,8 @@
 ' 2 key/value pairs',
 '(NSDictionary *) newDictionary = ',
 ' 12 key/value pairs',
+'(NSDictionary *) nscfDictionary = ',
+' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
 ' 3 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
@@ -39,6 +42,36 @@
 ' @"11 elements"',
 ])
 
+self.expect(
+'frame variable -d run-target *nscfDictionary',
+patterns=[
+'\(__NSCFDictionary\) \*nscfDictionary =',
+'key = 0x.* @"foo"',
+'value = 0x.* @"foo"',
+'key = 0x.* @"bar"',
+'value = 0x.* @"bar"',
+'key = 0x.* @"baz"',
+'value = 0x.* @"baz"',
+'key = 0x.* @"quux"',
+'value = 0x.* @"quux"',
+])
+
+
+self.expect(
+  'frame var nscfSet',
+  substrs=[
+  '(NSSet *) nscfSet = ',
+  '2 elements',
+  ])
+  
+self.expect(
+  'frame variable -d run-target *nscfSet',
+  patterns=[
+  '\(__NSCFSet\) \*nscfSet =',
+  '\[0\] = 0x.* @".*"',
+  '\[1\] = 0x.* @".*"',
+])
+  
 self.expect(
 'frame variable iset1 iset2 imset',
 substrs=['4 indexes', '512 indexes', '10 indexes'])
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -606,6 +606,11 @@
   lldb_private::formatters::NSSetSyntheticFrontEndCreator,

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

Thanks for taking care of this. It's a lot of work. First round of comments.




Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:12
+
+CFBasicHash::~CFBasicHash() { m_address = LLDB_INVALID_ADDRESS; }
+

Why do you need this? Can't you just use the default destructor?



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:18-19
+  return m_ht_32;
+  return m_ht_64;
+  return false;
+}

I'm under the impression the second return is never hit.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:23-24
+bool CFBasicHash::Update(addr_t addr, ExecutionContextRef exe_ctx_rf) {
+  if (addr == LLDB_INVALID_ADDRESS)
+return false;
+

Can this ever happen? What if `addr` is `0` instead?



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:28-29
+  m_exe_ctx_ref = exe_ctx_rf;
+  m_ptr_size =
+  m_exe_ctx_ref.GetTargetSP()->GetArchitecture().GetAddressByteSize();
+  m_byte_order = m_exe_ctx_ref.GetTargetSP()->GetArchitecture().GetByteOrder();

Thanks for doing this, as it will work on remote devices. We really need to 
check the test passes on arm64 before committing.



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:35-71
+
+size = sizeof(__CFBasicHash::RuntimeBase);
+size += sizeof(__CFBasicHash::Bits);
+
+DataBufferHeap buffer(size, 0);
+m_exe_ctx_ref.GetProcessSP()->ReadMemory(addr, buffer.GetBytes(), size,
+ error);

These two pieces of code, `m_ptr_size == 4` and `m_ptr_size == 8` are 
surprisingly similar. I'm really worried we might have a bug in one of them and 
miss the other. Is there anything we can do to share the code? [e.g. 
templatize].



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:139-140
+
+  // Unsupported architecure
+  return false;
+}

This could be an `lldb_assert` or `unreachable`. We really shouldn't be here if 
ptrsize != 8 or ptrsize != 4, unless there's an egregious bug.



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:101
 namespace formatters {
+#pragma mark NSDictionaryI
 class NSDictionaryISyntheticFrontEnd : public SyntheticChildrenFrontEnd {

Why?



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:145
 
+#pragma mark NSCFDictionary
+class NSCFDictionarySyntheticFrontEnd : public SyntheticChildrenFrontEnd {

Again, why?



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:178-188
+private:
+  // Prime numbers. Values above 100 have been adjusted up so that the
+  // malloced block size will be just below a multiple of 512; values
+  // above 1200 have been adjusted up to just below a multiple of 4096.
+  constexpr static const uintptr_t NSDictionaryCapacities[] = {
+  0,3,6, 11,19,32,   52,
+  85,   118,  155,   237,   390,   672,  1065,

Maybe a reference to the foundation header where these are defined, if public.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Vedant Kumar via Phabricator via lldb-commits]

vsk marked 8 inline comments as done.
vsk added inline comments.



Comment at: lldb/source/DataFormatters/StringPrinter.cpp:173
+  unsigned escaped_len;
+  const unsigned max_buffer_size = 7;
+  uint8_t *data = new uint8_t[max_buffer_size];

shafik wrote:
> `constexpr`
Cool, I learned that only 'constexpr' guarantees that you have a compile-time 
constant. Fixed here and below.



Comment at: lldb/unittests/DataFormatter/StringPrinterTests.cpp:109
+  EXPECT_TRUE(matches({"\0", 1}, QUOTE("")));
+  EXPECT_TRUE(matches("\a", QUOTE("\\a")));
+  EXPECT_TRUE(matches("\b", QUOTE("\\u{8}")));

shafik wrote:
> We have [raw string 
> literals](https://en.cppreference.com/w/cpp/language/string_literal) since 
> C++11:
> 
> ```
> EXPECT_TRUE(matches("\a", R"("\a")"));
> ```
I personally find the current version much clearer, but can change this if 
others feel strongly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843



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

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Vedant Kumar via Phabricator via lldb-commits]

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



Comment at: lldb/source/DataFormatters/StringPrinter.cpp:405
-template <>
-bool StringPrinter::ReadStringAndDumpToStream<
-StringPrinter::StringElementType::ASCII>(

This is what I mean. The whole ReadStringAndDumpToStream specialization 
can be deleted and replaced with a call to DumpEncodedBufferToStream, as the 
UTF & ASCII code paths are really similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843



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

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 258419.
vsk marked an inline comment as done.
vsk added a comment.

Address review feedback:

- Use standard gtest operators (EXPECT_EQ)
- constexpr/std::move cleanup, comment cleanups, sprintf length fix

Also, I took the opportunity to further unify the ASCII and UTF8 handling --
this lets us delete a bunch of duplicated code. PTAL, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77843

Files:
  lldb/include/lldb/DataFormatters/StringPrinter.h
  lldb/include/lldb/Target/Language.h
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Plugins/Language/ObjC/NSString.cpp
  lldb/source/Target/Language.cpp
  lldb/unittests/DataFormatter/CMakeLists.txt
  lldb/unittests/DataFormatter/StringPrinterTests.cpp

Index: lldb/unittests/DataFormatter/StringPrinterTests.cpp
===
--- /dev/null
+++ lldb/unittests/DataFormatter/StringPrinterTests.cpp
@@ -0,0 +1,160 @@
+//===-- StringPrinterTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/DataFormatters/StringPrinter.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/Endian.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+using lldb_private::formatters::StringPrinter;
+using llvm::Optional;
+using llvm::StringRef;
+
+#define QUOTE(x) std::string("\"" x "\"")
+
+/// Format \p input according to the options specified in the template params,
+/// then check whether the result is equal to \p reference. If not, dump the
+/// expeted vs. actual results.
+template 
+static Optional format(StringRef input) {
+  StreamString out;
+  StringPrinter::ReadBufferAndDumpToStreamOptions opts;
+  opts.SetStream(&out);
+  opts.SetSourceSize(input.size());
+  opts.SetNeedsZeroTermination(true);
+  opts.SetEscapeNonPrintables(true);
+  opts.SetIgnoreMaxLength(false);
+  opts.SetEscapeStyle(escape_style);
+  DataExtractor extractor(input.data(), input.size(),
+  endian::InlHostByteOrder(), sizeof(void *));
+  opts.SetData(extractor);
+  const bool success = StringPrinter::ReadBufferAndDumpToStream(opts);
+  if (!success)
+return llvm::None;
+  return out.GetString().str();
+}
+
+// Test ASCII formatting for C++. This behaves exactly like UTF8 formatting for
+// C++, although that's questionable (see FIXME in StringPrinter.cpp).
+TEST(StringPrinterTests, CxxASCII) {
+  auto fmt = [](StringRef str) {
+return format(str);
+  };
+
+  // Special escapes.
+  EXPECT_EQ(fmt({"\0", 1}), QUOTE(""));
+  EXPECT_EQ(fmt("\a"), QUOTE("\\a"));
+  EXPECT_EQ(fmt("\b"), QUOTE("\\b"));
+  EXPECT_EQ(fmt("\f"), QUOTE("\\f"));
+  EXPECT_EQ(fmt("\n"), QUOTE("\\n"));
+  EXPECT_EQ(fmt("\r"), QUOTE("\\r"));
+  EXPECT_EQ(fmt("\t"), QUOTE("\\t"));
+  EXPECT_EQ(fmt("\v"), QUOTE("\\v"));
+  EXPECT_EQ(fmt("\""), QUOTE("\\\""));
+  EXPECT_EQ(fmt("\'"), QUOTE("'"));
+  EXPECT_EQ(fmt("\\"), QUOTE(""));
+
+  // Printable characters.
+  EXPECT_EQ(fmt("'"), QUOTE("'"));
+  EXPECT_EQ(fmt("a"), QUOTE("a"));
+  EXPECT_EQ(fmt("Z"), QUOTE("Z"));
+  EXPECT_EQ(fmt("🥑"), QUOTE("🥑"));
+
+  // Octal (\nnn), hex (\xnn), extended octal (\u or \U).
+  EXPECT_EQ(fmt("\uD55C"), QUOTE("한"));
+  EXPECT_EQ(fmt("\U00010348"), QUOTE("𐍈"));
+
+  // FIXME: These strings are all rejected, but shouldn't be AFAICT. LLDB finds
+  // that these are not valid utf8 sequences, but that's OK, the raw values
+  // should still be printed out.
+  EXPECT_NE(fmt("\376"), QUOTE("\\xfe")); // \376 is 254 in decimal.
+  EXPECT_NE(fmt("\xfe"), QUOTE("\\xfe")); // \xfe is 254 in decimal.
+}
+
+// Test UTF8 formatting for C++.
+TEST(StringPrinterTests, CxxUTF8) {
+  auto fmt = [](StringRef str) {
+return format(str);
+  };
+
+  // Special escapes.
+  EXPECT_EQ(fmt({"\0", 1}), QUOTE(""));
+  EXPECT_EQ(fmt("\a"), QUOTE("\\a"));
+  EXPECT_EQ(fmt("\b"), QUOTE("\\b"));
+  EXPECT_EQ(fmt("\f"), QUOTE("\\f"));
+  EXPECT_EQ(fmt("\n"), QUOTE("\\n"));
+  EXPECT_EQ(fmt("\r"), QUOTE("\\r"));
+  EXPECT_EQ(fmt("\t"), QUOTE("\\t"));
+  EXPECT_EQ(fmt("\v"), QUOTE("\\v"));
+  EXPECT_EQ(fmt("\""), QUOTE("\\\""));
+  EXPECT_EQ(fmt("\'"), QUOTE("'"));
+  EXPECT_EQ(fmt("\\"), QUOTE(""));
+
+  // Printable characters.
+  EXPECT_EQ(fmt("'"), QUOTE("'"));
+  EXPECT_EQ(fmt("a"), QUOTE("a"));
+  EXPECT_EQ(fmt("Z"), QUOTE("Z"));
+  EXPECT_EQ(fmt("🥑"), QUOTE("🥑"));
+
+  // Octal (\nnn), hex (\xnn), extended octal (\u or \U

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:35-71
+
+size = sizeof(__CFBasicHash::RuntimeBase);
+size += sizeof(__CFBasicHash::Bits);
+
+DataBufferHeap buffer(size, 0);
+m_exe_ctx_ref.GetProcessSP()->ReadMemory(addr, buffer.GetBytes(), size,
+ error);

davide wrote:
> These two pieces of code, `m_ptr_size == 4` and `m_ptr_size == 8` are 
> surprisingly similar. I'm really worried we might have a bug in one of them 
> and miss the other. Is there anything we can do to share the code? [e.g. 
> templatize].
Indeed, they're very similar and I already tried using templates (and SFINAE) 
to make it more generic, however I couldn't achieve that.

Since the remote architecture might be different from lldb's, we can't use 
macros to generate the underlying struct with the right size. So, I decided to 
template the structure used by CF, and have one of each architecture as a class 
attribute (look at CFBasicHash.h:114). 

Basically it's a tradeoff I chose voluntarily: I preferred having the 
CFBasicHash class handle the architecture to only expose one CFBasicHash object 
in the CFDictionary and CFSet data-formatter, rather than having two 
CFBasicHash objects templated for each ptr_size and have all the logic 
duplicated for each different architecture AND each data formatters.

If you can see a better way to do it, please let me know :) 



Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:178-188
+private:
+  // Prime numbers. Values above 100 have been adjusted up so that the
+  // malloced block size will be just below a multiple of 512; values
+  // above 1200 have been adjusted up to just below a multiple of 4096.
+  constexpr static const uintptr_t NSDictionaryCapacities[] = {
+  0,3,6, 11,19,32,   52,
+  85,   118,  155,   237,   390,   672,  1065,

davide wrote:
> Maybe a reference to the foundation header where these are defined, if public.
It is not in a public header, that's why I copied the explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396



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

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 258431.
mib marked 2 inline comments as done.
mib added a comment.

Address Davide's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396

Files:
  lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp
  lldb/source/Plugins/Language/ObjC/CFBasicHash.h
  lldb/source/Plugins/Language/ObjC/CMakeLists.txt
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.h
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m

Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
@@ -376,10 +376,12 @@
 	[newMutableDictionary setObject:@"foo" forKey:@"bar19"];
 	[newMutableDictionary setObject:@"foo" forKey:@"bar20"];
 
-	id cfKeys[2] = { @"foo", @"bar", @"baz", @"quux" };
-	id cfValues[2] = { @"foo", @"bar", @"baz", @"quux" };
-	NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
-	CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
+id cfKeys[4] = {@"foo", @"bar", @"baz", @"quux"};
+id cfValues[4] = {@"foo", @"bar", @"baz", @"quux"};
+NSDictionary *nsDictionary = CFBridgingRelease(CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 2, nil, nil));
+NSDictionary *nscfDictionary = CFBridgingRelease(CFDictionaryCreate(
+nil, (void *)cfKeys, (void *)cfValues, 4, nil, nil));
+CFDictionaryRef cfDictionaryRef = CFDictionaryCreate(nil, (void *)cfKeys, (void *)cfValues, 3, nil, nil);
 
 	NSAttributedString* attrString = [[NSAttributedString alloc] initWithString:@"hello world from foo" attributes:newDictionary];
 	[attrString isEqual:nil];
@@ -412,8 +414,10 @@
 	NSSet* nsset = [[NSSet alloc] initWithObjects:str1,str2,str3,nil];
 	NSSet *nsmutableset = [[NSMutableSet alloc] initWithObjects:str1,str2,str3,nil];
 	[nsmutableset addObject:str4];
+NSSet *nscfSet =
+CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
 
-	CFDataRef data_ref = CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
+CFDataRef data_ref = CFDataCreate(kCFAllocatorDefault, [immutableData bytes], 5);
 
 	CFMutableDataRef mutable_data_ref = CFDataCreateMutable(kCFAllocatorDefault, 8);
 	CFDataAppendBytes(mutable_data_ref, [mutableData bytes], 5);
Index: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
@@ -20,6 +20,7 @@
 self.appkit_tester_impl(self.nscontainers_data_formatter_commands)
 
 def nscontainers_data_formatter_commands(self):
+
 self.expect(
 'frame variable newArray nsDictionary newDictionary nscfDictionary cfDictionaryRef newMutableDictionary cfarray_ref mutable_array_ref',
 substrs=[
@@ -29,6 +30,8 @@
 ' 2 key/value pairs',
 '(NSDictionary *) newDictionary = ',
 ' 12 key/value pairs',
+'(NSDictionary *) nscfDictionary = ',
+' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
 ' 3 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
@@ -39,6 +42,36 @@
 ' @"11 elements"',
 ])
 
+self.expect(
+'frame variable -d run-target *nscfDictionary',
+patterns=[
+'\(__NSCFDictionary\) \*nscfDictionary =',
+'key = 0x.* @"foo"',
+'value = 0x.* @"foo"',
+'key = 0x.* @"bar"',
+'value = 0x.* @"bar"',
+'key = 0x.* @"baz"',
+'value = 0x.* @"baz"',
+'key = 0x.* @"quux"',
+'value = 0x.* @"quux"',
+])
+
+
+self.expect(
+  'frame var nscfSet',
+  substrs=[
+  '(NSSet *) nscfSet = ',
+  '2 elements',
+  ])
+  
+self.expect(
+  'frame variable -d run-target *nscfSet',
+  patterns=[
+ 

[Lldb-commits] [lldb] eef9cb1 - [lldb] [testsuite] Fix TestFixIts.py on Linux

[Jan Kratochvil via lldb-commits]

Author: Jan Kratochvil
Date: 2020-04-18T08:32:12+02:00
New Revision: eef9cb1628898adb5652a32eb95b53c544de0d6f

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

LOG: [lldb] [testsuite] Fix TestFixIts.py on Linux

Since D77214 there is a testsuite regression for TestFixIts.py
on Fedora 31 x86_64.
File 
"/home/jkratoch/redhat/llvm-monorepo/lldb/test/API/commands/expression/fixits/TestFixIts.py",
 line 148, in test_with_target
  self.assertEquals(value.GetError().GetCString(), "error: No value")
  AssertionError: 'error: error: Multiple internal symbols found for \'d\'\nid 
= {0x0d2a}, ran [truncated]... != 'error: No value'

That is because Fedora glibc incl. libm.so contains also ELF debug
symbols and there exists a 'd' symbol:
  (gdb) p d
  $1 = {i = {0, 1076887552}, d = 16}
  (gdb) p &d
  $2 = (const number *) 0x778e8bc0 
  (gdb) info sym 0x778e8bc0
  d in section .rodata of /lib64/libm.so.6

  $ nm /lib64/libm.so.6 |grep ' d$'
  000bfbc0 r d
  000caa20 r d
  000caa20 r d
  000caa20 r d

  glibc-build$ for i in `find -name "*.o"`;do nm 2>/dev/null $i|grep ' d$' && 
echo $i;done
  0080 r d
  ./math/s_atan-fma4.o
  0080 r d
  ./math/s_atan-avx.o
  0080 r d
  ./math/s_atan.o

Added: 


Modified: 
lldb/test/API/commands/expression/fixits/TestFixIts.py

Removed: 




diff  --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py 
b/lldb/test/API/commands/expression/fixits/TestFixIts.py
index 9ce5415b732e..9907c2557788 100644
--- a/lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -107,11 +107,11 @@ def test_with_multiple_retries(self):
 struct S1 : public T {
   using T::TypeDef;
   int f() {
-Data d;
-d.m = 123;
+Data data;
+data.m = 123;
 // The first error as the using above requires a 'typename '.
 // Will trigger a Fix-It that puts 'typename' in the right place.
-typename S1::TypeDef i = &d;
+typename S1::TypeDef i = &data;
 // i has the type "Data *", so this should be i.m.
 // The second run will change the . to -> via the Fix-It.
 return i.m;



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