subject:"\\\\\\\[Lldb\\\\\\\-commits\\\\\\\] \\\\\\\[PATCH\\\\\\\] D79554\\\\\\\: \\\\\\\[lldb\\\\\\\/Dataformatter\\\\\\\] Add support to CF\\\\\\\{Dictionary, Set\\\\\\\}Ref types"

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-07-07 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib added a comment.

@labath Thanks for reporting that crash. D83327 
 should fix the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-07-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:690-694
+  if (!valobj && synthetic_array_member)
+valobj = GetSyntheticValue()
+ ->GetChildAtIndex(synthetic_index, synthetic_array_member)
+ .get();
+

I'm getting a crash here when attempting to dereference incomplete c(++) types. 
The simplest (albeit contrived) repro is:
```
$ cat /tmp/a.c 
struct incomplete;

struct incomplete *var = (struct incomplete *)0xdead;

int main() {}
$ clang /tmp/a.c -o /tmp/a.out -g
$ lldb /tmp/a.out -o "target variable var[0]"
(lldb) target create "/tmp/a.out"
Current executable set to '/tmp/a.out' (x86_64).
(lldb) target variable var[0]
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash 
backtrace.
Stack dump:
0.  Program arguments: lldb /tmp/a.out -o target variable var[0] 
 #0 0x561dcd331cea llvm::sys::PrintStackTrace(llvm::raw_ostream&) 
(lldb+0x29cea)
 #1 0x561dcd32ffc4 llvm::sys::RunSignalHandlers() (lldb+0x27fc4)
 #2 0x561dcd330108 SignalHandler(int) (lldb+0x28108)
 #3 0x7f62b35175b0 __restore_rt (/lib64/libpthread.so.0+0x135b0)
 #4 0x7f62ae9235fc lldb_private::ValueObject::CreateChildAtIndex(unsigned 
long, bool, int) (../lib/liblldb.so.11git+0xb985fc)
 #5 0x7f62ae923e2b 
lldb_private::ValueObject::GetSyntheticArrayMember(unsigned long, bool) 
(../lib/liblldb.so.11git+0xb98e2b)
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-15 Thread Med Ismail Bennani via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
mib marked 4 inline comments as done.
Closed by commit rG0eba9de71e2a: [lldb/Dataformatter] Add support to 
CF{Dictionary,Set}Ref types (authored by mib).

Changed prior to commit:
  https://reviews.llvm.org/D79554?vs=264038=264334#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  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
@@ -482,8 +482,7 @@
   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);
+  CFDictionaryRef cfDictionaryRef = (__bridge CFDictionaryRef)nsDictionary;
 
   NSAttributedString *attrString =
   [[NSAttributedString alloc] initWithString:@"hello world from foo"
@@ -542,6 +541,7 @@
   [nsmutableset addObject:str4];
   NSSet *nscfSet =
   CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
+  CFSetRef cfSetRef = (__bridge CFSetRef)nscfSet;
 
   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
@@ -32,7 +32,7 @@
 '(NSDictionary *) nscfDictionary = ',
 ' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
-' 3 key/value pairs',
+' 2 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
 ' 21 key/value pairs',
 '(CFArrayRef) cfarray_ref = ',
@@ -57,10 +57,23 @@
 
 
 self.expect(
-  'frame var nscfSet',
+'frame variable -d run-target *cfDictionaryRef',
+patterns=[
+'\(const __CFDictionary\) \*cfDictionaryRef =',
+'key = 0x.* @"foo"',
+'value = 0x.* @"foo"',
+'key = 0x.* @"bar"',
+'value = 0x.* @"bar"',
+])
+
+
+self.expect(
+  'frame var nscfSet cfSetRef',
   substrs=[
   '(NSSet *) nscfSet = ',
   '2 elements',
+  '(CFSetRef) cfSetRef = ',
+  '2 elements',
   ])
 
 self.expect(
@@ -72,6 +85,14 @@
 ])
 
 self.expect(
+  'frame variable -d run-target *cfSetRef',
+  patterns=[
+  '\(const __CFSet\) \*cfSetRef =',
+  '\[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
@@ -448,6 +448,10 @@
 appkit_flags);
   AddCXXSummary(objc_category_sp,
 lldb_private::formatters::NSDictionarySummaryProvider,
+"NSDictionary summary provider", ConstString("__CFDictionary"),
+appkit_flags);
+  AddCXXSummary(objc_category_sp,
+lldb_private::formatters::NSDictionarySummaryProvider,
 "NSDictionary summary provider",
 ConstString("CFMutableDictionaryRef"), appkit_flags);
 
@@ -468,6 +472,9 @@
 "__NSCFSet summary", ConstString("__NSCFSet"), appkit_flags);
   AddCXXSummary(objc_category_sp,
 lldb_private::formatters::NSSetSummaryProvider,
+"__CFSet summary", ConstString("__CFSet"), appkit_flags);
+  AddCXXSummary(objc_category_sp,
+lldb_private::formatters::NSSetSummaryProvider,

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-15 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:2839-2861
+// In case of C opaque pointers, use the pointee type and try to
+// recreate a new ValueObjectChild using it.
+if (!m_deref_valobj) {
+  if (Language::LanguageIsCFamily(GetPreferredDisplayLanguage())) {
+if (HasSyntheticValue()) {
+
+  child_compiler_type = compiler_type.GetPointeeType();

Can this be merged with the code above? It seems like the only difference is 
the way it which we get the CompilerType.



Comment at: lldb/source/Core/ValueObject.cpp:2842
+if (!m_deref_valobj) {
+  if (Language::LanguageIsCFamily(GetPreferredDisplayLanguage())) {
+if (HasSyntheticValue()) {

What's the reason for limiting this to the C family? It seems like one could 
want to have a pretty printer for incomplete types in any language...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-15 Thread Raphael Isemann via Phabricator via lldb-commits

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

IMHO this looks good now. Maybe add a comment to the commit that this adds 
support for incomplete types in ValueObjects.

I think @jingham should also take a look at this to make sure this makes sense.




Comment at: lldb/source/Core/ValueObjectSyntheticFilter.cpp:56
   SetName(parent.GetName());
-  CopyValueData(m_parent);
+  if (m_parent->GetCompilerType().IsCompleteType())
+CopyValueData(m_parent);

Maybe comment this line that copying the data of an incomplete type doesn't 
make sense as it has no (byte) size. (Previously this worked as our made-up 
type was empty and had a size of 0 bytes.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-14 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 264038.
mib edited the summary of this revision.
mib added a comment.

Instead of creating a new empty struct type, this new implementation will use 
the opaque pointer's pointee type to create the new `ValueObjectChild`.
This makes the previously introduced helper function in `TypeSystem` and 
`TypeSystemClang` useless, so I got rid of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectSyntheticFilter.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  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
@@ -482,8 +482,7 @@
   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);
+  CFDictionaryRef cfDictionaryRef = (__bridge CFDictionaryRef)nsDictionary;
 
   NSAttributedString *attrString =
   [[NSAttributedString alloc] initWithString:@"hello world from foo"
@@ -542,6 +541,7 @@
   [nsmutableset addObject:str4];
   NSSet *nscfSet =
   CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
+  CFSetRef cfSetRef = (__bridge CFSetRef)nscfSet;
 
   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
@@ -32,7 +32,7 @@
 '(NSDictionary *) nscfDictionary = ',
 ' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
-' 3 key/value pairs',
+' 2 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
 ' 21 key/value pairs',
 '(CFArrayRef) cfarray_ref = ',
@@ -57,10 +57,23 @@
 
 
 self.expect(
-  'frame var nscfSet',
+'frame variable -d run-target *cfDictionaryRef',
+patterns=[
+'\(const __CFDictionary\) \*cfDictionaryRef =',
+'key = 0x.* @"foo"',
+'value = 0x.* @"foo"',
+'key = 0x.* @"bar"',
+'value = 0x.* @"bar"',
+])
+
+
+self.expect(
+  'frame var nscfSet cfSetRef',
   substrs=[
   '(NSSet *) nscfSet = ',
   '2 elements',
+  '(CFSetRef) cfSetRef = ',
+  '2 elements',
   ])
 
 self.expect(
@@ -71,6 +84,14 @@
   '\[1\] = 0x.* @".*"',
 ])
 
+self.expect(
+  'frame variable -d run-target *cfSetRef',
+  patterns=[
+  '\(const __CFSet\) \*cfSetRef =',
+  '\[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
@@ -446,6 +446,10 @@
 lldb_private::formatters::NSDictionarySummaryProvider,
 "NSDictionary summary provider", ConstString("CFDictionaryRef"),
 appkit_flags);
+  AddCXXSummary(objc_category_sp,
+lldb_private::formatters::NSDictionarySummaryProvider,
+"NSDictionary summary provider", ConstString("__CFDictionary"),
+appkit_flags);
   AddCXXSummary(objc_category_sp,
 lldb_private::formatters::NSDictionarySummaryProvider,
 "NSDictionary summary provider",
@@ -466,6 +470,9 @@
   AddCXXSummary(objc_category_sp,
 lldb_private::formatters::NSSetSummaryProvider,
 "__NSCFSet summary", ConstString("__NSCFSet"), appkit_flags);
+

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-11 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D79554#2026999 , @teemperor wrote:

> I don't see a huge problem with things like `__lldb_autogen_nspair` as it's a 
> single obviously generated type hardcoded into LLDB. But this really generic 
> approach here can generate all kind of structs whenever some formatter 
> decides that some random incomplete record has a synthetic value.
>
> But in any case, I think we're kind of talking about different problems here. 
> We do have the following type:
>
>   TypedefType 0x7fc9cd04f440 'CFDictionaryRef' sugar
>   |-Typedef 0x7fc9cd04f3e0 'CFDictionaryRef'
>   `-PointerType 0x7fc9cd04f3a0 'const struct __CFDictionary *'
> `-QualType 0x7fc9cd04f381 'const struct __CFDictionary' const
>   `-RecordType 0x7fc9cd04f380 'struct __CFDictionary'
> `-CXXRecord 0x7fc9cd04f2d0 '__CFDictionary'
>
>
> So we do have an underlying type for `*CFDictionaryRef` which is just the 
> `__CFDictionary` record. The problem is just that this is an incomplete type. 
> So we could make a ValueObject that has an incomplete struct type which seems 
> to work for me just fine? We probably need to adjust the formatter for this 
> prints an actual empty value: `(const __CFDictionary) *dict = {}`


That's exactly what I was thinking. As the CompilerType abstraction is already 
capable of representing incomplete types, and our goal is to construct 
synthetic children ( == ValueObjects) for these types, then it seems reasonable 
for ValueObjects to operate on these incomplete types. I mean, one won't be 
able to do much with them, but it should be possible to take their address and 
then grovel in that memory, or cast it to something else (and I assume that's 
what these pretty printers will be doing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-08 Thread Raphael Isemann via Phabricator via lldb-commits

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

I don't see a huge problem with things like `__lldb_autogen_nspair` as it's a 
single obviously generated type hardcoded into LLDB. But this really generic 
approach here can generate all kind of structs whenever some formatter decides 
that some random incomplete record has a synthetic value.

But in any case, I think we're kind of talking about different problems here. 
We do have the following type:

  TypedefType 0x7fc9cd04f440 'CFDictionaryRef' sugar
  |-Typedef 0x7fc9cd04f3e0 'CFDictionaryRef'
  `-PointerType 0x7fc9cd04f3a0 'const struct __CFDictionary *'
`-QualType 0x7fc9cd04f381 'const struct __CFDictionary' const
  `-RecordType 0x7fc9cd04f380 'struct __CFDictionary'
`-CXXRecord 0x7fc9cd04f2d0 '__CFDictionary'

So we do have an underlying type for `*CFDictionaryRef` which is just the 
`__CFDictionary` record. The problem is just that this is an incomplete type. 
So we could make a ValueObject that has an incomplete struct type which seems 
to work for me just fine? We probably need to adjust the formatter for this 
prints an actual empty value: `(const __CFDictionary) *dict = {}`

Comment at: lldb/source/Core/ValueObject.cpp:2853
+ConstString g___lldb_opaque_ptr_type(
+compiler_type.GetTypeName().AsCString());
+

teemperor wrote:
> This is the name of the typedef, not the underlying incomplete struct. So 
> this creates a new empty record with the name of the typedef which seems 
> weird. If we fake that some type is actually empty instead of incomplete, 
> then I think the underlying struct makes more sense to emulate. Also this 
> variable name is kinda weird with it's `g` prefix even though it's not a 
> global or a static.
This is still the name of the typedef:
```
lang=c++
child_compiler_type =
type_system->GetEmptyStructType(compiler_type.GetTypeName());
```

So we end up with an AST like we get from here:
```
lang=c++
typedef NSDictionary* CFDictionaryRef;
struct CFDictionaryRef {}; // conflict
```

This should at least have some kind of `__lldb_made_up_type_unique_stringy_` 
prefix in the generated name to make this not ambiguous.

Also a log statement would be useful that we know when these types are created 
when someone reports a bug.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554

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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-08 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib updated this revision to Diff 262823.
mib edited the summary of this revision.
mib added a comment.

- Added a virtual method to create an empty struct type 
`TypeSystem::GetEmptyStructType` (if someone can think of a better name, I'm 
open to suggestions)
- Refactored `ValueObject::Dereference`
- Reverted the NFC changes in `ValueObject::CreateChildAtIndex`
- Added test for C++ incomplete type with data-formatter (WIP)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554

Files:
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/TypeSystem.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py
  lldb/test/API/functionalities/data-formatter/data-formatter-objc/main.m
  
lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/Makefile
  
lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/TestDataFormatterOpaquePtr.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.cc
  
lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.h
  
lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.py

Index: lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.py
@@ -0,0 +1,42 @@
+import lldb
+import lldb.formatters
+import lldb.formatters.synth as synth
+
+def ReadOpaquePointerAddress(var):
+error = lldb.SBError()
+
+ptr = var.GetChildMemberWithName('pImpl')
+
+if not ptr or not ptr.IsValid():
+  error.SetErrorString('Invalid pointer.')
+  return error, None
+
+return error, ptr.GetValue()
+
+def ReadOpaquePointerValue(var):
+  error = lldb.SBError()
+
+  ptr = var.GetChildAtIndex(0)
+
+  if not ptr or not ptr.IsValid():
+error.SetErrorString('Invalid pointer.')
+return error, None
+  
+  return error, ptr.GetValue;
+
+def OpaquePtrSummaryProvider(value, _):
+error, addr = ReadOpaquePointerAddress(value)
+if error.Fail():
+  return "{} has no value!".format(addr)
+return "{} contains a value!".format(addr)
+
+class OpaquePtrSyntheticChildProvider(synth.PythonObjectSyntheticChildProvider):
+def __init__(self, value, dict):
+  synth.PythonObjectSyntheticChildProvider.__init__(self, value, dict)
+
+def make_children(self):
+  error, value = ReadOpaquePointerValue(self.value)
+  
+  if error.Fail():
+return []
+  return [('value', value)]
Index: lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.h
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.h
@@ -0,0 +1,8 @@
+class Opaque {
+  class Impl;
+  Impl *pImpl;
+
+public:
+  Opaque(int);
+  ~Opaque();
+};
Index: lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.cc
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/opaque_ptr.cc
@@ -0,0 +1,9 @@
+#include "opaque_ptr.h"
+
+class Opaque::Impl {
+  int value; // private data
+public:
+  Impl(int value) : value(value) {}
+};
+Opaque::Opaque(int value) : pImpl{new Impl(value)} {}
+Opaque::~Opaque() = default;
Index: lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/main.cpp
@@ -0,0 +1,9 @@
+#include "opaque_ptr.h"
+
+typedef Opaque *OpaquePtr;
+
+int main() {
+  Opaque base(7);
+  OpaquePtr ptr = 
+  return 0; // Set breakpoint here.
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/TestDataFormatterOpaquePtr.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-opaque-ptr/TestDataFormatterOpaquePtr.py
@@ -0,0 +1,51 @@
+"""
+Test lldb data formatter subsystem.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class OpaquePtrDataFormatterTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def setUp(self):
+# Call super's setUp().
+

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Jim Ingham via Phabricator via lldb-commits

jingham added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:2846
+  if (llvm::isa(compiler_type.GetTypeSystem())) {
+if (HasSyntheticValue()) {
+  TargetSP target_sp = GetTargetSP();

mib wrote:
> friss wrote:
> > I am understanding correctly that this condition will only trigger when 
> > there's a synthetic child provider for the type? In other words, if we have 
> > an opaque pointer that we don't know how to display, it will still cause an 
> > error, right?
> Yes, from my understanding, if the opaque pointer doesn't have any synthetic 
> child provider, it won't have a synthetic value and lldb should error saying 
> `error: dereference failed`.
We should add a test to make sure this stays true.  



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Med Ismail Bennani via Phabricator via lldb-commits

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



Comment at: lldb/source/Core/ValueObject.cpp:52
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+

friss wrote:
> You don't want to introduce a dependency between Core and a plugin. What you 
> need here might need to be introduced as a new abstract functionality on the 
> TypeSystem base class.
I'll change this before updating the differential with the fixes.



Comment at: lldb/source/Core/ValueObject.cpp:2846
+  if (llvm::isa(compiler_type.GetTypeSystem())) {
+if (HasSyntheticValue()) {
+  TargetSP target_sp = GetTargetSP();

friss wrote:
> I am understanding correctly that this condition will only trigger when 
> there's a synthetic child provider for the type? In other words, if we have 
> an opaque pointer that we don't know how to display, it will still cause an 
> error, right?
Yes, from my understanding, if the opaque pointer doesn't have any synthetic 
child provider, it won't have a synthetic value and lldb should error saying 
`error: dereference failed`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

The way the ValueObject code works w.r.t. Synthetic child providers is that you 
first look up and make a ValueObject for the actual value the user requested, 
(for instance a ValueObjectVariable or a ValueObjectChild or a 
ValueObjectConstResult for expressions, etc.), then you hand that to the 
printer.  The printer will look at whether the current settings prefer 
Synthetic Values, and if they do they ask the actual value if it has any 
Synthetic children.  So when you do something like:

(lldb) frame var *opaque_ptr

we first have to make a ValueObject for *opaque_ptr (in this case the 
dereference is a ValueObjectChild of the ValueObjectVariable representing the 
variable `opaque_ptr` and return that to the printer.

But you can't currently make a ValueObject that doesn't have a type, and the 
problem here is that we don't have a type for * of the opaque pointer.  Making 
an empty struct definition and returning a ValueObject with that struct 
definition is one fairly straightforward way of doing that.  And we already 
inject custom types in other places to manage synthetic children (look for 
`__lldb_autogen_nspair` for instance).  So this seemed the most straightforward 
choice.

Note, you could also make a new ValueObject subclass for these purposes: 
ValueObjectOpaqueDereference to forward the Synthetic Children.  The 
ValueObject base class doesn't require a type so you could make such a thing. 
But it would somehow fake handing out the type so you'd have to be careful how 
you did that.  Then its job would be to hand out the synthetic children.  If 
people really hate making the fake structure we could go this way instead, but 
it seems better to use an extant facility if we can, and "fake handing out the 
type" seems to me at least as dangerous as injecting synthesized types into the 
TypeSystem.  The latter we already do, the former we don't.

I'm not happy with the notion of just hard-coding this to CF types or making it 
about bridged types at all.  It is not uncommon to have a client library that 
vends opaque pointers, but you've figured out what some of the fields are.  One 
solution to debugging this scenario is to introduce another shadow type, either 
in lldb or actually in your code, and cast types to that when printing.  But 
that means you can't use "frame var" since it doesn't support casting, and you 
have to remember to cast every time in the expression parser which is annoying. 
 if you made a synthetic child provider, and if what Ismail is trying to get 
working here works generally, then once you've made the provider, you can just 
print * of the pointer, and it will work.  So I think this should be a general 
facility for opaque pointer types with synthetic child providers.  So I'd much 
prefer to keep this a general facility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik added a comment.

I have to agree w/ Pavel here that I am not crazy about creating creating an 
empty struct here, it is not clear why there is no other way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Med Ismail Bennani via Phabricator via lldb-commits

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



Comment at: lldb/source/Core/ValueObject.cpp:665
   bool child_is_deref_of_parent = false;
+  CompilerType compiler_type = GetCompilerType();
   uint64_t language_flags = 0;

teemperor wrote:
> This change (and the one below) don't seem to be some NFC refactoring? Not 
> sure why this was refactored as `compiler_type` is only ever used once where 
> we previously called `GetCompilerType()`?
This is a remaining from a previous change that I ended up not removing. I'll 
change it back the way it was.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:59-60
+
+self.expect(
+'frame variable -d run-target *cfDictionaryRef',
+patterns=[

friss wrote:
> I must be missing something obvious, but it seems like that patch doesn't 
> register a formatter for CFDictionaryRef. Was it already there but 
> non-functional?
The string summary worked but not the synthetic child provider.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

I don't really know that ValueObject code, but I wish we could just pretend 
that the dereferenced type of X is a type named Y or something like that and 
then just map *Ref classes to their non-opaque bridged types when dereferenced. 
No idea if that's possible though.

Anyway, if we go with the fake empty struct approach then I wish we could just 
use the (fixed?) list 

 of these special bridged structs and hard-code them into some ObjC plugin or 
something like that. If those few selected classes are manually completed as 
empty structs when we find them in the debug info then I think that's a 
manageable workaround to get this running.




Comment at: lldb/source/Core/ValueObject.cpp:665
   bool child_is_deref_of_parent = false;
+  CompilerType compiler_type = GetCompilerType();
   uint64_t language_flags = 0;

This change (and the one below) don't seem to be some NFC refactoring? Not sure 
why this was refactored as `compiler_type` is only ever used once where we 
previously called `GetCompilerType()`?



Comment at: lldb/source/Core/ValueObject.cpp:2853
+ConstString g___lldb_opaque_ptr_type(
+compiler_type.GetTypeName().AsCString());
+

This is the name of the typedef, not the underlying incomplete struct. So this 
creates a new empty record with the name of the typedef which seems weird. If 
we fake that some type is actually empty instead of incomplete, then I think 
the underlying struct makes more sense to emulate. Also this variable name is 
kinda weird with it's `g` prefix even though it's not a global or a static.



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:58
 
+
+self.expect(

Trailing spaces (that Phabricator somehow doesn't properly highlight :/)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Pavel Labath via Phabricator via lldb-commits

labath added a reviewer: teemperor.
labath added a comment.

The ValueObject change looks very worrying to me. The ability to pretty-print 
incomplete types seems pretty useful, but the implementation of that looks a 
bit... scary. I mean you're not even checking whether the incomplete type is a 
struct or an enum. And the effect of forcefully creating a empty struct has 
impact outside of the pretty-printer. Maybe it would be better if we thought 
the valueobject/pretty-printer machinery to work with incomplete types?

Raphael, what do you make of all of this?

Also would it be possible to test the ValueObject functionality on a c++ test 
(an incomplete c++ type with a custom pretty printer), so that test can run 
every where?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Frederic Riss via Phabricator via lldb-commits

friss added inline comments.



Comment at: lldb/source/Core/ValueObject.cpp:52
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+

You don't want to introduce a dependency between Core and a plugin. What you 
need here might need to be introduced as a new abstract functionality on the 
TypeSystem base class.



Comment at: lldb/source/Core/ValueObject.cpp:2846
+  if (llvm::isa(compiler_type.GetTypeSystem())) {
+if (HasSyntheticValue()) {
+  TargetSP target_sp = GetTargetSP();

I am understanding correctly that this condition will only trigger when there's 
a synthetic child provider for the type? In other words, if we have an opaque 
pointer that we don't know how to display, it will still cause an error, right?



Comment at: 
lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:59-60
+
+self.expect(
+'frame variable -d run-target *cfDictionaryRef',
+patterns=[

I must be missing something obvious, but it seems like that patch doesn't 
register a formatter for CFDictionaryRef. Was it already there but 
non-functional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554



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

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-06 Thread 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 a subscriber: lldb-commits.

This patch improves data formatting for CFDictionaryRef and CFSetRef.
It uses the same data-formatter as NSCFDictionaries and NSCFSets introduced
previously (D78396 ) but did require some 
adjustments in Core::ValueObject.

Since the "Ref" types are opaque pointers to the actual CF containers, if the
value object has a synthetic value, lldb will create an empty record type to
create the new ValueObjectChild needed to dereference the original ValueObject.
This allows the "Ref" types to behaves the same as CF containers when used with
the `frame variable` command, the SBAPI or in Xcode's variable inspector.

rdar://53104287

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79554

Files:
  lldb/source/Core/ValueObject.cpp
  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
@@ -482,8 +482,7 @@
   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);
+  CFDictionaryRef cfDictionaryRef = (__bridge CFDictionaryRef)nsDictionary;
 
   NSAttributedString *attrString =
   [[NSAttributedString alloc] initWithString:@"hello world from foo"
@@ -542,6 +541,7 @@
   [nsmutableset addObject:str4];
   NSSet *nscfSet =
   CFBridgingRelease(CFSetCreate(nil, (void *)cfValues, 2, nil));
+  CFSetRef cfSetRef = (__bridge CFSetRef)nscfSet;
 
   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
@@ -32,7 +32,7 @@
 '(NSDictionary *) nscfDictionary = ',
 ' 4 key/value pairs',
 '(CFDictionaryRef) cfDictionaryRef = ',
-' 3 key/value pairs',
+' 2 key/value pairs',
 '(NSDictionary *) newMutableDictionary = ',
 ' 21 key/value pairs',
 '(CFArrayRef) cfarray_ref = ',
@@ -55,12 +55,25 @@
 'value = 0x.* @"quux"',
 ])
 
+
+self.expect(
+'frame variable -d run-target *cfDictionaryRef',
+patterns=[
+'\(CFDictionaryRef\) \*cfDictionaryRef =',
+'key = 0x.* @"foo"',
+'value = 0x.* @"foo"',
+'key = 0x.* @"bar"',
+'value = 0x.* @"bar"',
+])
+
 
 self.expect(
-  'frame var nscfSet',
+  'frame var nscfSet cfSetRef',
   substrs=[
   '(NSSet *) nscfSet = ',
   '2 elements',
+  '(CFSetRef) cfSetRef = ',
+  '2 elements',
   ])
 
 self.expect(
@@ -70,6 +83,14 @@
   '\[0\] = 0x.* @".*"',
   '\[1\] = 0x.* @".*"',
 ])
+
+self.expect(
+  'frame variable -d run-target *cfSetRef',
+  patterns=[
+  '\(CFSetRef\) \*cfSetRef =',
+  '\[0\] = 0x.* @".*"',
+  '\[1\] = 0x.* @".*"',
+])
 
 self.expect(
 'frame variable iset1 iset2 imset',
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -610,6 +610,10 @@
   lldb_private::formatters::NSSetSyntheticFrontEndCreator,
   "__NSCFSet synthetic children", ConstString("__NSCFSet"),
   ScriptedSyntheticChildren::Flags());
+  AddCXXSynthetic(objc_category_sp,
+  lldb_private::formatters::NSSetSyntheticFrontEndCreator,
+  "CFSetRef synthetic children", ConstString("CFSetRef"),
+

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

18 matches

Site Navigation

Mail list logo

Footer information