Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread Pavel Labath via lldb-dev
I wasn't using type units (those don't work at all right now).

I've done a bit of digging, and i found this patch
 which explicitly disables template
member function parsing (though it seems it didn't really work before
either). The patch contains a quite long explanation of why is this
not working. I can't say I understand all of it (this is getting a bit
out of my league), but the core of the issue seems to be that when we
start to mix classes from two CU which have different sets of
instantiations in a single expression, things quickly go south because
the recycled clang ASTs from the two dwarf versions do not match.

For better or worse, it seems gdb is having similar issues as well, as
I couldn't get it to grok my member template expressions either..

On Thu, 14 Jun 2018 at 19:47, David Blaikie  wrote:
>
> oh, awesome.
>
> Were you using type units? (I imagine that'd make the situation worse - since 
> the way clang emits DWARF for a type with a member function template implicit 
> specialization is to emit the type unit without any mention of this, and to 
> emit the implicit specialization declaration into the stub type in the CU 
> (that references the type unit)) Without type units I'd be pretty surprised 
> if you couldn't call the implicit specialization at least from the CU in 
> which it was instantiated.
>
> On Thu, Jun 14, 2018 at 11:41 AM Pavel Labath  wrote:
>>
>> On Thu, 14 Jun 2018 at 19:29, Pavel Labath  wrote:
>> >
>> > On Thu, 14 Jun 2018 at 19:26, David Blaikie  wrote:
>> > >
>> > >
>> > >
>> > > On Thu, Jun 14, 2018 at 11:24 AM Pavel Labath  wrote:
>> > >>
>> > >> On Thu, 14 Jun 2018 at 17:58, Greg Clayton  wrote:
>> > >> >
>> > >> >
>> > >> >
>> > >> > On Jun 14, 2018, at 9:36 AM, Adrian Prantl  wrote:
>> > >> >
>> > >> >
>> > >> >
>> > >> > On Jun 14, 2018, at 7:01 AM, Pavel Labath via llvm-dev 
>> > >> >  wrote:
>> > >> >
>> > >> > Thank you all. I am going to try to reply to all comments in a single 
>> > >> > email.
>> > >> >
>> > >> > Regarding the  .apple_objc idea, I am afraid the situation is not as
>> > >> > simple as just flipping a switch.
>> > >> >
>> > >> >
>> > >> > Jonas is currently working on adding the support for DWARF5-style 
>> > >> > Objective-C accelerator tables to LLVM/LLDB/dsymutil. Based on the 
>> > >> > assumption that DWARF 4 and earlier are unaffected by any of this, I 
>> > >> > don't think it's necessary to spend any effort of making the 
>> > >> > transition smooth. I'm fine with having Objective-C on DWARF 5 broken 
>> > >> > on trunk for two weeks until Jonas is done adding Objective-C support 
>> > >> > to the DWARF 5 implementation.
>> > >>
>> > >> Ideally, I would like to enable the accelerator tables (possibly with
>> > >> a different version number or something) on DWARF 4 too (on non-apple
>> > >> targets only). The reason for this is that their absence if causing
>> > >> large slowdowns when debugging on non-apple platforms, and I wouldn't
>> > >> want to wait for dwarf 5 for that to go away (I mean no disrespect to
>> > >> Paul and DWARF 5 effort in general, but even if all of DWARF 5 in llvm
>> > >> was done tomorrow, there would still be lldb, which hasn't even begun
>> > >> to look at this version).
>> > >>
>> > >> That said, if you are working on the Objective C support right now,
>> > >> then I am happy to wait two weeks or so that we have a full
>> > >> implementation from the get-go.
>> > >>
>> > >> > But, other options may be possible as well. What's not clear to me is
>> > >> > whether these tables couldn't be replaced by extra information in the
>> > >> > .debug_info section. It seems to me that these tables are trying to
>> > >> > work around the issue that there is no straight way to go from a
>> > >> > DW_TAG_structure type DIE describing an ObjC class to it's methods. If
>> > >> > these methods (their forward declarations) were be present as children
>> > >> > of the type DIE (as they are for c++ classes), then these tables may
>> > >> > not be necessary. But maybe (probably) that has already been
>> > >> > considered and deemed infeasible for some reason. In any case this
>> > >> > seemed like a thing best left for people who actually work on ObjC
>> > >> > support to figure out.
>> > >> >
>> > >> >
>> > >> > That's really a question for Greg or Jim — I don't know why the 
>> > >> > current representation has the Objective-C methods outside of the 
>> > >> > structs. One reason might be that an interface's implementation can 
>> > >> > define more methods than are visible in its public interface in the 
>> > >> > header file, but we already seem to be aware of this and mark the 
>> > >> > implementation with DW_AT_APPLE_objc_complete_type. I also am not 
>> > >> > sure that this is the *only* reason for the objc accelerator table. 
>> > >> > But I'd like to learn.
>> > >>
>> > >> My observation was based on studying lldb code. The only place where
>> > >> the objc table is used is in the 

Re: [lldb-dev] clang::VersionTuple

2018-06-15 Thread Pavel Labath via lldb-dev
Hello again,

Just a quick update on the state of this.

 I've managed to move VersionTuple from clang to llvm. I've also
created  to switch over our version
handling to that class.

Could I interest anyone in taking a quick look at the patch?

pl

On Wed, 9 May 2018 at 08:49, Pavel Labath  wrote:
>
> Thank you all for the feedback. I'll get on with this when I find some
> spare time. I will send a patch which will show the final look of the code,
> before I start moving VersionTuple into llvm.
>
> cheers,
> pl
> On Tue, 8 May 2018 at 19:46, Frédéric Riss via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
>
>
> > On May 8, 2018, at 11:37 AM, Greg Clayton  wrote:
>
> > I was referring to the Swift and Apple internal branches. They tend to
> lock down against older llvm and clang repositories so when we put changes
> in llvm or clang that are required for LLDB, it makes merging a bit tougher
> in those cases. Again, I am not affected by this, just trying to watch out
> for you guys.
>
>
> > I understand and I appreciate it, I was just worried that I’m missing
> something obvious. We branch LLDB at the same time as LLVM so that’s not
> actually an issue. Of course, it might cause merge conflicts or make it
> harder to cherry-pick patches, but that's just living downstream.
>
> > Fred
>
> > Greg
>
>
> > On May 8, 2018, at 11:35 AM, Greg Clayton  wrote:
>
> > I'm good if Apple is good.
>
> > On May 8, 2018, at 11:31 AM, Frédéric Riss  wrote:
>
>
>
> > On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
>
>
> > On May 8, 2018, at 9:47 AM, Zachary Turner  wrote:
>
> > We don’t want the lowest levels of lldb to depend on clang. If this is
> useful we should move it from clang to llvm and use llvm::VersionTuple
>
>
> > I agree, though this move will cause merging issues for many that have
> repositories that link against older llvm/clang. Doesn't affect me anymore,
> but Apple will be affected.
>
>
> > I’m not sure I understand what issues you’re referring to, we don’t link
> new LLDBs to old clangs (and even if we did, it wouldn’t be something the
> that drives community decisions).
>
> > Fred
>
> > Greg
>
> > On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
> >> No issues from me.
>
> >> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >> >
> >> > While moving Args around, I noticed that we have a bunch of
> >> > functions/classes that pass/store version numbers as a triplet of
> integers
> >> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper
> class
> >> > for that when I noticed clang::VersionTuple, which is pretty much what
> I
> >> > wanted out of the box.
> >> >
> >> > Now there are small differences between this class, and what we have
> now:
> >> > it has an extra fourth "build" field, and it uses only 31 bits to
> represent
> >> >  the values. None of these seem to matter (particularly as we are
> >> > converting our representation into this struct in some places) that
> much,
> >> > but before I go through the trouble of pulling this class into llvm
> >> > (although technically possible, it seems wrong to pull a clang
> dependency
> >> > at such a low level), I wanted to make sure we are able to use it.
> >> >
> >> > Do you see any reason why we could not replace our version triplets
> with
> >> > clang::VersionTuple ?
> >> >
> >> > cheers,
> >> > pl
> >> > ___
> >> > lldb-dev mailing list
> >> > lldb-dev@lists.llvm.org
> >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
> >> ___
> >> lldb-dev mailing list
> >> lldb-dev@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
>
>
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread Greg Clayton via lldb-dev
To elaborate a bit more on the issue that is detailed in 
https://reviews.llvm.org/rL260308:

There are many clang AST contexts that are used in LLDB:
- one for each lldb_private::Module that contains type definitions as we know 
them in the module and its symbol vendor
- one for each expression
- one for results of expressions in the lldb_private::Target

As we run expressions we end up copying classes around between the Module ASTs 
and the expression and Target ASTs. If a class has templated functions, they 
will only be in the DWARF is a specialization was created and used. If you have 
a class that looks like:

class A {
   A();
void Foo(T t);
};


And then you have main.cpp that has a "double" and "int" specialization, the 
class definition in DWARF looks like:

class A {
   A();
void Foo(int t);
void Foo(double t);
};

In another source file, say foo.cpp, if its use of class A doesn't specialize 
Foo, we have a class definition in DWARF that looks like:

class A {
   A();
};

With the C++ ODR rules, we can pick any of the "class A" definitions whose 
qualified name matches ("::A") and has the same decl file + decl line. So when 
parsing "class A", the DWARF parser will use the accelerator tables to find all 
instances of "class A", and it will pick on and use it and this will become the 
one true definition for "class A". This is because DWARF is only emitted for 
template functions when there is a specialization, that mean any definition of 
"class A" might or might not include any definition for " A::Foo(T 
t);". When we copy types between ASTs, everything is fine if the classes match 
(no copy needs to be made), but things go wrong if things don't match and this 
causes expression errors. 

Some ways to fix this:
1 - anytime we need _any_ C++ class, we must dig up all definitions and check 
_all_ DW_TAG_subprogram DIEs within the class to check if any functions have 
templates and use the class with the most specializations
2 - have DWARF actually emit the template function info all the time as a type 
T, not a specialization, so we always have the full definition
3 - have some accelerator table that explicitly points us to all 
specializations of class methods given a class name

Solution #1 would cause us to dig through all definitions of all C++ classes 
all the time when parsing DWARF to check if definitions of the classes had 
template methods. And we would need to find the class that has the most 
template methods. This would cause us to parse much more of the debug info all 
of the time and cause increased memory consumption and performance regressions.

Solution #2: not sure if DWARF even supports generic template definitions where 
the template isn't specialized. And, how would we be able to tell DWARF that 
emits only specialized templates vs one that has generic definitions...

Solution #3 will require compiler changes.

So this is another vote to support the ability for a given class to be able to 
locate all of its functions, kind of like we need for Objective C where the 
class definition doesn't contain all of methods, so we have the .apple_objc 
section that provides this mapping for us. We would need something similar for 
C++.

So maybe a possible solution is some sort of section that can specify all of 
the DIEs related to a class that are not contained in the class hierarchy 
itself. This would work for Objective C and for C++.

Thoughts?

Greg

> On Jun 15, 2018, at 3:34 AM, Pavel Labath  wrote:
> 
> I wasn't using type units (those don't work at all right now).
> 
> I've done a bit of digging, and i found this patch
>  which explicitly disables template
> member function parsing (though it seems it didn't really work before
> either). The patch contains a quite long explanation of why is this
> not working. I can't say I understand all of it (this is getting a bit
> out of my league), but the core of the issue seems to be that when we
> start to mix classes from two CU which have different sets of
> instantiations in a single expression, things quickly go south because
> the recycled clang ASTs from the two dwarf versions do not match.
> 
> For better or worse, it seems gdb is having similar issues as well, as
> I couldn't get it to grok my member template expressions either..
> 
> On Thu, 14 Jun 2018 at 19:47, David Blaikie  wrote:
>> 
>> oh, awesome.
>> 
>> Were you using type units? (I imagine that'd make the situation worse - 
>> since the way clang emits DWARF for a type with a member function template 
>> implicit specialization is to emit the type unit without any mention of 
>> this, and to emit the implicit specialization declaration into the stub type 
>> in the CU (that references the type unit)) Without type units I'd be pretty 
>> surprised if you couldn't call the implicit specialization at least from the 
>> CU in which it was instantiated.
>> 
>> On Thu, Jun 14, 2018 at 11:41 AM Pavel Labath  wrote:
>>> 
>>> On T

Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread via lldb-dev
> From: Greg Clayton [mailto:clayb...@gmail.com] 
>
> ...
> If a class has templated functions, they will only be in the DWARF is a
> specialization was created and used. If you have a class that looks like:
>
> class A {
>   A();
>    void Foo(T t);
> };
>
> And then you have main.cpp that has a "double" and "int" specialization,
> the class definition in DWARF looks like:
>
> class A {
>   A();
>    void Foo(int t);
>    void Foo(double t);
> };
>
> In another source file, say foo.cpp, if its use of class A doesn't
> specialize Foo, we have a class definition in DWARF that looks like:
>
> class A {
>   A();
> };
>

I think it would be more instructive to think about a case where 
main.cpp had Foo and foo.cpp had Foo.

> With the C++ ODR rules, we can pick any of the "class A" definitions
> whose qualified name matches ("::A") and has the same decl file + 
> decl line. So when parsing "class A", the DWARF parser will use the
> accelerator tables to find all instances of "class A", and it will
> pick on and use it and this will become the one true definition for
> "class A".

FTR, the Sony debugger finds them all and then merges them into the one
true definition, because there's no promise that any one description is
a superset of the rest.

> This is because DWARF is only emitted for template functions when
> there is a specialization, that mean any definition of "class A" might
> or might not include any definition for " A::Foo(T t);".

It's not just template functions, you know; the implicit ctors/dtors
might or might not be in any given description.  Those are also only
instantiated and described in CUs that require them.

> When we copy types between ASTs, everything is fine if the classes
> match (no copy needs to be made), but things go wrong if things
> don't match and this causes expression errors. 
>
> Some ways to fix this:
> 1 - anytime we need _any_ C++ class, we must dig up all definitions
> and check _all_ DW_TAG_subprogram DIEs within the class to check if
> any functions have templates and use the class with the most
> specializations

This still fails in my "more instructive" case.  The accelerator table
does make it fast to find all the classes with the same name, and you
can then merge them.

> 2 - have DWARF actually emit the template function info all the time
> as a type T, not a specialization, so we always have the full definition

Hm.  You mean a subroutine_type with template_parameter children that
don't have actual values?  That would give you a pattern, but not tell
you what/where definitions exist.  I don't see how that can help?

> 3 - have some accelerator table that explicitly points us to all
> specializations of class methods given a class name

So you want an accelerator to tell you what bits you need to pick up
from the various descriptions in order to construct the overall
superset definition, which would be a little cheaper than parsing
each entire class description (which you can already find through
the existing accelerator tables).  I can see that.

> Solution #1 would cause us to dig through all definitions of all C++
> classes all the time when parsing DWARF to check if definitions of
> the classes had template methods. And we would need to find the class
> that has the most template methods. This would cause us to parse much
> more of the debug info all of the time and cause increased memory
> consumption and performance regressions.

It would be cheap to put a flag on the class DIE that tells you there
are template methods to go look for.  Then you incur the cost only
when necessary.  And the accelerator table makes it fast to find the
other class descriptions.

> Solution #2: not sure if DWARF even supports generic template
> definitions where the template isn't specialized. And, how would we
> be able to tell DWARF that emits only specialized templates vs one
> that has generic definitions...

Right, DWARF today doesn't do that, although as I mentioned earlier
conjuring up a subroutine_type with template parameters would not
appear to be any more helpful than a simple flag on the class.

>
> Solution #3 will require compiler changes.

Well, so does #2.

> So this is another vote to support the ability for a given class
> to be able to locate all of its functions, kind of like we need
> for Objective C where the class definition doesn't contain all of
> methods, so we have the .apple_objc section that provides this
> mapping for us. We would need something similar for C++.
>
> So maybe a possible solution is some sort of section that can
> specify all of the DIEs related to a class that are not contained
> in the class hierarchy itself. This would work for Objective C
> and for C++.
>
> Thoughts?
>
> Greg

So, would it be helpful to have a flag on the class that tells you
whether there are method instantiations to go look for?  My
impression is that templated class methods are unusual so it would
save the performance cost a lot of the time right there.

I don'

Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread Greg Clayton via lldb-dev


> On Jun 15, 2018, at 9:23 AM,  
>  wrote:
> 
>> From: Greg Clayton [mailto:clayb...@gmail.com] 
>> 
>> ...
>> If a class has templated functions, they will only be in the DWARF is a
>> specialization was created and used. If you have a class that looks like:
>> 
>> class A {
>>A();
>> void Foo(T t);
>> };
>> 
>> And then you have main.cpp that has a "double" and "int" specialization,
>> the class definition in DWARF looks like:
>> 
>> class A {
>>A();
>> void Foo(int t);
>> void Foo(double t);
>> };
>> 
>> In another source file, say foo.cpp, if its use of class A doesn't
>> specialize Foo, we have a class definition in DWARF that looks like:
>> 
>> class A {
>>A();
>> };
>> 
> 
> I think it would be more instructive to think about a case where 
> main.cpp had Foo and foo.cpp had Foo.

Any difference is the problem here for clang ASTs, so it just matters that they 
are different. We make a CXXRecordDecl in the clang AST and if we see even one 
specialization, then we add the generic version to the CXXRecordDecl and we are 
good to go. So the main.cpp had Foo and foo.cpp had Foo is 
actually fine. If I make a CXXRecordDecl from either of these then the two 
definitions match since the clang AST CXXRecordDecl just needs to have the 
templated function declaration.

> 
>> With the C++ ODR rules, we can pick any of the "class A" definitions
>> whose qualified name matches ("::A") and has the same decl file + 
>> decl line. So when parsing "class A", the DWARF parser will use the
>> accelerator tables to find all instances of "class A", and it will
>> pick on and use it and this will become the one true definition for
>> "class A".
> 
> FTR, the Sony debugger finds them all and then merges them into the one
> true definition, because there's no promise that any one description is
> a superset of the rest.

At the expense of parsing every definition for a class within each file.
> 
>> This is because DWARF is only emitted for template functions when
>> there is a specialization, that mean any definition of "class A" might
>> or might not include any definition for " A::Foo(T t);".
> 
> It's not just template functions, you know; the implicit ctors/dtors
> might or might not be in any given description.  Those are also only
> instantiated and described in CUs that require them.

We don't care about those as those are marked as DW_AT_artificial and we leave 
those out of the clang AST Context CXXRecordDecl because they are implicit and 
the compiler can add those back in if needed since it knows if a class can have 
the constructors implicitly created.

> 
>> When we copy types between ASTs, everything is fine if the classes
>> match (no copy needs to be made), but things go wrong if things
>> don't match and this causes expression errors. 
>> 
>> Some ways to fix this:
>> 1 - anytime we need _any_ C++ class, we must dig up all definitions
>> and check _all_ DW_TAG_subprogram DIEs within the class to check if
>> any functions have templates and use the class with the most
>> specializations
> 
> This still fails in my "more instructive" case.  The accelerator table
> does make it fast to find all the classes with the same name, and you
> can then merge them.

That is a lot of DWARF parsing and logic to try and figure out what the full 
set of DW_TAG_subprograms are.

> 
>> 2 - have DWARF actually emit the template function info all the time
>> as a type T, not a specialization, so we always have the full definition
> 
> Hm.  You mean a subroutine_type with template_parameter children that
> don't have actual values?  That would give you a pattern, but not tell
> you what/where definitions exist.  I don't see how that can help?

Right now DWARF does only specializations, so DWARF would need to be extended 
to be able to specify the template details without requiring a specialization. 
Not easy for sure.

> 
>> 3 - have some accelerator table that explicitly points us to all
>> specializations of class methods given a class name
> 
> So you want an accelerator to tell you what bits you need to pick up
> from the various descriptions in order to construct the overall
> superset definition, which would be a little cheaper than parsing
> each entire class description (which you can already find through
> the existing accelerator tables).  I can see that.

Yes. This is the most appealing to me as well.

> 
>> Solution #1 would cause us to dig through all definitions of all C++
>> classes all the time when parsing DWARF to check if definitions of
>> the classes had template methods. And we would need to find the class
>> that has the most template methods. This would cause us to parse much
>> more of the debug info all of the time and cause increased memory
>> consumption and performance regressions.
> 
> It would be cheap to put a flag on the class DIE that tells you there
> are template methods to go look for.  Then you incur the cost only
> when necessary.  And the accelerator table makes it

Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread via lldb-dev


> -Original Message-
> From: Greg Clayton [mailto:clayb...@gmail.com]
> Sent: Friday, June 15, 2018 12:46 PM
> To: Robinson, Paul
> Cc: lab...@google.com; dblai...@gmail.com; apra...@apple.com;
> echri...@google.com; jdevliegh...@apple.com; llvm-...@lists.llvm.org;
> jan.kratoch...@redhat.com; lldb-dev@lists.llvm.org; Jim Ingham
> Subject: Re: [llvm-dev] [lldb-dev] Adding DWARF5 accelerator table support
> to llvm
> 
> 
> 
> > On Jun 15, 2018, at 9:23 AM, 
>  wrote:
> >
> >> From: Greg Clayton [mailto:clayb...@gmail.com]
> >>
> >> ...
> >> If a class has templated functions, they will only be in the DWARF is a
> >> specialization was created and used. If you have a class that looks
> like:
> >>
> >> class A {
> >>A();
> >> void Foo(T t);
> >> };
> >>
> >> And then you have main.cpp that has a "double" and "int"
> specialization,
> >> the class definition in DWARF looks like:
> >>
> >> class A {
> >>A();
> >> void Foo(int t);
> >> void Foo(double t);
> >> };
> >>
> >> In another source file, say foo.cpp, if its use of class A doesn't
> >> specialize Foo, we have a class definition in DWARF that looks like:
> >>
> >> class A {
> >>A();
> >> };
> >>
> >
> > I think it would be more instructive to think about a case where
> > main.cpp had Foo and foo.cpp had Foo.
> 
> Any difference is the problem here for clang ASTs, so it just matters that
> they are different. We make a CXXRecordDecl in the clang AST and if we see
> even one specialization, then we add the generic version to the
> CXXRecordDecl and we are good to go. So the main.cpp had Foo and
> foo.cpp had Foo is actually fine. If I make a CXXRecordDecl from
> either of these then the two definitions match since the clang AST
> CXXRecordDecl just needs to have the templated function declaration.

Got it, good to know.
You don't actually care what instantiations exist? That seems odd.

> 
> >
> >> With the C++ ODR rules, we can pick any of the "class A" definitions
> >> whose qualified name matches ("::A") and has the same decl file +
> >> decl line. So when parsing "class A", the DWARF parser will use the
> >> accelerator tables to find all instances of "class A", and it will
> >> pick on and use it and this will become the one true definition for
> >> "class A".
> >
> > FTR, the Sony debugger finds them all and then merges them into the one
> > true definition, because there's no promise that any one description is
> > a superset of the rest.
> 
> At the expense of parsing every definition for a class within each file.
> >
> >> This is because DWARF is only emitted for template functions when
> >> there is a specialization, that mean any definition of "class A" might
> >> or might not include any definition for " A::Foo(T t);".
> >
> > It's not just template functions, you know; the implicit ctors/dtors
> > might or might not be in any given description.  Those are also only
> > instantiated and described in CUs that require them.
> 
> We don't care about those as those are marked as DW_AT_artificial and we
> leave those out of the clang AST Context CXXRecordDecl because they are
> implicit and the compiler can add those back in if needed since it knows
> if a class can have the constructors implicitly created.
> 
> >
> >> When we copy types between ASTs, everything is fine if the classes
> >> match (no copy needs to be made), but things go wrong if things
> >> don't match and this causes expression errors.
> >>
> >> Some ways to fix this:
> >> 1 - anytime we need _any_ C++ class, we must dig up all definitions
> >> and check _all_ DW_TAG_subprogram DIEs within the class to check if
> >> any functions have templates and use the class with the most
> >> specializations
> >
> > This still fails in my "more instructive" case.  The accelerator table
> > does make it fast to find all the classes with the same name, and you
> > can then merge them.
> 
> That is a lot of DWARF parsing and logic to try and figure out what the
> full set of DW_TAG_subprograms are.
> 
> >
> >> 2 - have DWARF actually emit the template function info all the time
> >> as a type T, not a specialization, so we always have the full
> definition
> >
> > Hm.  You mean a subroutine_type with template_parameter children that
> > don't have actual values?  That would give you a pattern, but not tell
> > you what/where definitions exist.  I don't see how that can help?
> 
> Right now DWARF does only specializations, so DWARF would need to be
> extended to be able to specify the template details without requiring a
> specialization. Not easy for sure.
> 
> >
> >> 3 - have some accelerator table that explicitly points us to all
> >> specializations of class methods given a class name
> >
> > So you want an accelerator to tell you what bits you need to pick up
> > from the various descriptions in order to construct the overall
> > superset definition, which would be a little cheaper than parsing
> > each entire class description (which you can already find thro

Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread Greg Clayton via lldb-dev


> On Jun 15, 2018, at 10:40 AM,  
>  wrote:
> 
> 
> 
>> -Original Message-
>> From: Greg Clayton [mailto:clayb...@gmail.com ]
>> Sent: Friday, June 15, 2018 12:46 PM
>> To: Robinson, Paul
>> Cc: lab...@google.com ; dblai...@gmail.com 
>> ; apra...@apple.com ;
>> echri...@google.com ; jdevliegh...@apple.com 
>> ; llvm-...@lists.llvm.org 
>> ;
>> jan.kratoch...@redhat.com ; 
>> lldb-dev@lists.llvm.org ; Jim Ingham
>> Subject: Re: [llvm-dev] [lldb-dev] Adding DWARF5 accelerator table support
>> to llvm
>> 
>> 
>> 
>>> On Jun 15, 2018, at 9:23 AM, 
>>  wrote:
>>> 
 From: Greg Clayton [mailto:clayb...@gmail.com]
 
 ...
 If a class has templated functions, they will only be in the DWARF is a
 specialization was created and used. If you have a class that looks
>> like:
 
 class A {
   A();
void Foo(T t);
 };
 
 And then you have main.cpp that has a "double" and "int"
>> specialization,
 the class definition in DWARF looks like:
 
 class A {
   A();
void Foo(int t);
void Foo(double t);
 };
 
 In another source file, say foo.cpp, if its use of class A doesn't
 specialize Foo, we have a class definition in DWARF that looks like:
 
 class A {
   A();
 };
 
>>> 
>>> I think it would be more instructive to think about a case where
>>> main.cpp had Foo and foo.cpp had Foo.
>> 
>> Any difference is the problem here for clang ASTs, so it just matters that
>> they are different. We make a CXXRecordDecl in the clang AST and if we see
>> even one specialization, then we add the generic version to the
>> CXXRecordDecl and we are good to go. So the main.cpp had Foo and
>> foo.cpp had Foo is actually fine. If I make a CXXRecordDecl from
>> either of these then the two definitions match since the clang AST
>> CXXRecordDecl just needs to have the templated function declaration.
> 
> Got it, good to know.
> You don't actually care what instantiations exist? That seems odd.

The compiler will use the generic type and ask where the specialized version 
lives. For the template method type, we will in the right stuff when we make 
the function type, but the class itself just has the generic definition. 
> 
>> 
>>> 
 With the C++ ODR rules, we can pick any of the "class A" definitions
 whose qualified name matches ("::A") and has the same decl file +
 decl line. So when parsing "class A", the DWARF parser will use the
 accelerator tables to find all instances of "class A", and it will
 pick on and use it and this will become the one true definition for
 "class A".
>>> 
>>> FTR, the Sony debugger finds them all and then merges them into the one
>>> true definition, because there's no promise that any one description is
>>> a superset of the rest.
>> 
>> At the expense of parsing every definition for a class within each file.
>>> 
 This is because DWARF is only emitted for template functions when
 there is a specialization, that mean any definition of "class A" might
 or might not include any definition for " A::Foo(T t);".
>>> 
>>> It's not just template functions, you know; the implicit ctors/dtors
>>> might or might not be in any given description.  Those are also only
>>> instantiated and described in CUs that require them.
>> 
>> We don't care about those as those are marked as DW_AT_artificial and we
>> leave those out of the clang AST Context CXXRecordDecl because they are
>> implicit and the compiler can add those back in if needed since it knows
>> if a class can have the constructors implicitly created.
>> 
>>> 
 When we copy types between ASTs, everything is fine if the classes
 match (no copy needs to be made), but things go wrong if things
 don't match and this causes expression errors.
 
 Some ways to fix this:
 1 - anytime we need _any_ C++ class, we must dig up all definitions
 and check _all_ DW_TAG_subprogram DIEs within the class to check if
 any functions have templates and use the class with the most
 specializations
>>> 
>>> This still fails in my "more instructive" case.  The accelerator table
>>> does make it fast to find all the classes with the same name, and you
>>> can then merge them.
>> 
>> That is a lot of DWARF parsing and logic to try and figure out what the
>> full set of DW_TAG_subprograms are.
>> 
>>> 
 2 - have DWARF actually emit the template function info all the time
 as a type T, not a specialization, so we always have the full
>> definition
>>> 
>>> Hm.  You mean a subroutine_type with template_parameter children that
>>> don't have actual values?  That would give you a pattern, but not tell
>>> you what/where definitions exist.  I 

Re: [lldb-dev] clang::VersionTuple

2018-06-15 Thread Jim Ingham via lldb-dev


> On Jun 15, 2018, at 3:44 AM, Pavel Labath via lldb-dev 
>  wrote:
> 
> Hello again,
> 
> Just a quick update on the state of this.
> 
> I've managed to move VersionTuple from clang to llvm. I've also
> created  to switch over our version
> handling to that class.
> 
> Could I interest anyone in taking a quick look at the patch?


Somehow I can’t log into Phabricator from home so I can’t comment right now, 
but I took a look.  

In some of your changes in the SB files you do:

  if (PlatformSP platform_sp = GetSP())
version = platform_sp->GetOSVersion();

I don’t like putting initializers in if statements like this because I always 
have to think twice about whether you meant “==“.  Moreover, all of the 
surrounding code does it differently:

  PlatformSP platform_sp = GetSP()
  if (platform_sp)
version = platform_sp->GetOSVersion();

so switching to the other form in a couple of places only kinda forces the 
double-take.  But that’s a little nit.

Given that clang::VersionTuple::tryParse does what the hand-parsing in place 
(which I’m assuming it does or you wouldn’t have used it) this looks fine and a 
nice cleanup to me.

Jim


> 
> pl
> 
> On Wed, 9 May 2018 at 08:49, Pavel Labath  wrote:
>> 
>> Thank you all for the feedback. I'll get on with this when I find some
>> spare time. I will send a patch which will show the final look of the code,
>> before I start moving VersionTuple into llvm.
>> 
>> cheers,
>> pl
>> On Tue, 8 May 2018 at 19:46, Frédéric Riss via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 11:37 AM, Greg Clayton  wrote:
>> 
>>> I was referring to the Swift and Apple internal branches. They tend to
>> lock down against older llvm and clang repositories so when we put changes
>> in llvm or clang that are required for LLDB, it makes merging a bit tougher
>> in those cases. Again, I am not affected by this, just trying to watch out
>> for you guys.
>> 
>> 
>>> I understand and I appreciate it, I was just worried that I’m missing
>> something obvious. We branch LLDB at the same time as LLVM so that’s not
>> actually an issue. Of course, it might cause merge conflicts or make it
>> harder to cherry-pick patches, but that's just living downstream.
>> 
>>> Fred
>> 
>>> Greg
>> 
>> 
>>> On May 8, 2018, at 11:35 AM, Greg Clayton  wrote:
>> 
>>> I'm good if Apple is good.
>> 
>>> On May 8, 2018, at 11:31 AM, Frédéric Riss  wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 9:47 AM, Zachary Turner  wrote:
>> 
>>> We don’t want the lowest levels of lldb to depend on clang. If this is
>> useful we should move it from clang to llvm and use llvm::VersionTuple
>> 
>> 
>>> I agree, though this move will cause merging issues for many that have
>> repositories that link against older llvm/clang. Doesn't affect me anymore,
>> but Apple will be affected.
>> 
>> 
>>> I’m not sure I understand what issues you’re referring to, we don’t link
>> new LLDBs to old clangs (and even if we did, it wouldn’t be something the
>> that drives community decisions).
>> 
>>> Fred
>> 
>>> Greg
>> 
>>> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> 
 No issues from me.
>> 
> On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
> 
> While moving Args around, I noticed that we have a bunch of
> functions/classes that pass/store version numbers as a triplet of
>> integers
> (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper
>> class
> for that when I noticed clang::VersionTuple, which is pretty much what
>> I
> wanted out of the box.
> 
> Now there are small differences between this class, and what we have
>> now:
> it has an extra fourth "build" field, and it uses only 31 bits to
>> represent
> the values. None of these seem to matter (particularly as we are
> converting our representation into this struct in some places) that
>> much,
> but before I go through the trouble of pulling this class into llvm
> (although technically possible, it seems wrong to pull a clang
>> dependency
> at such a low level), I wanted to make sure we are able to use it.
> 
> Do you see any reason why we could not replace our version triplets
>> with
> clang::VersionTuple ?
> 
> cheers,
> pl
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>> 
 ___
 lldb-dev mailing list
 lldb-dev@lists.llvm.org
 http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>> 
>> 
>>> ___
>>> lldb-dev mailing list
>>> lldb-dev@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/

Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread via lldb-dev
gc> Solution #1 would cause us to dig through all definitions of all C++
gc> classes all the time when parsing DWARF to check if definitions of
gc> the classes had template methods. And we would need to find the class
gc> that has the most template methods. This would cause us to parse much
gc> more of the debug info all of the time and cause increased memory
gc> consumption and performance regressions.

pr> It would be cheap to put a flag on the class DIE that tells you there
pr> are template methods to go look for.  Then you incur the cost only
pr> when necessary.  And the accelerator table makes it fast to find the
pr> other class descriptions.

gc> That is a fine solution. But we still run into the problem where we don't
gc> know if the DWARF knows about that flag. If we do a flag, it would be nice
gc> if it were mandatory on all classes to indicate support for the flag. But
gc> this would be a fine solution and not hard to implement.

pr> So what you really want is not a flag, but a count, so you can tell when
pr> you've found all the different templates.  If the count is zero, there's
pr> nothing to look for.  If the count is two, you look around at all the
pr> various definitions of the class until you find two different templates,
pr> then you stop.  If there's no count attribute, your producer doesn't 
pr> know you want this information and you do it the hard way.  Or, we've
pr> invented a way to describe the templates directly in the class.
pr>
pr> How's that?

gc> that would work fine.

I filed PR37816 to track this idea.

The other ideas:

 - accelerator to point to the actual instantiations
 - emitting template definitions not just instantiations

would be trickier to define and harder to implement correctly.
I won't say they can't be done, but somebody else would have to do
the heavy lifting here, unless it turns out that our debugger folks
like the idea.

--paulr

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


Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread David Blaikie via lldb-dev
How do you handle name lookup for nested classes? They have the same
problem (they don't appear in all definitions) - don't appear in all
descriptions of the outer/parent class. (in theory we could ensure there's
always at least a declaration of the nested class - but we don't even do
that if the nested class is unused)

Is it just the case that Clang doesn't mind you adding a new nested class
but it does mind you adding a new member function template? If so, maybe we
could change Clang to support adding new member function templates instead
of extending DWARF?

On Fri, Jun 15, 2018 at 11:59 AM  wrote:

> gc> Solution #1 would cause us to dig through all definitions of all C++
> gc> classes all the time when parsing DWARF to check if definitions of
> gc> the classes had template methods. And we would need to find the class
> gc> that has the most template methods. This would cause us to parse much
> gc> more of the debug info all of the time and cause increased memory
> gc> consumption and performance regressions.
>
> pr> It would be cheap to put a flag on the class DIE that tells you there
> pr> are template methods to go look for.  Then you incur the cost only
> pr> when necessary.  And the accelerator table makes it fast to find the
> pr> other class descriptions.
>
> gc> That is a fine solution. But we still run into the problem where we
> don't
> gc> know if the DWARF knows about that flag. If we do a flag, it would be
> nice
> gc> if it were mandatory on all classes to indicate support for the flag.
> But
> gc> this would be a fine solution and not hard to implement.
>
> pr> So what you really want is not a flag, but a count, so you can tell
> when
> pr> you've found all the different templates.  If the count is zero,
> there's
> pr> nothing to look for.  If the count is two, you look around at all the
> pr> various definitions of the class until you find two different
> templates,
> pr> then you stop.  If there's no count attribute, your producer doesn't
> pr> know you want this information and you do it the hard way.  Or, we've
> pr> invented a way to describe the templates directly in the class.
> pr>
> pr> How's that?
>
> gc> that would work fine.
>
> I filed PR37816 to track this idea.
>
> The other ideas:
>
>  - accelerator to point to the actual instantiations
>  - emitting template definitions not just instantiations
>
> would be trickier to define and harder to implement correctly.
> I won't say they can't be done, but somebody else would have to do
> the heavy lifting here, unless it turns out that our debugger folks
> like the idea.
>
> --paulr
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


[lldb-dev] multiple NameErrors on lldb startup, doesn't seem to affect lldb performance

2018-06-15 Thread Peter Rowat via lldb-dev
Here is what happens:


 prowat% lldb modTS
(lldb) target create "modTS"
Traceback (most recent call last):
  File "", line 1, in 
  File 
"/Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/Resources/Python/lldb/__init__.py",
 line 98, in 
import six
ImportError: No module named six
Traceback (most recent call last):
  File "", line 1, in 
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "", line 1, in 
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
  File "", line 1, in 
NameError: name 'run_one_line' is not defined
Traceback (most recent call last):
..
..
..  (at least 100 repetitions) ..
..
..
Traceback (most recent call last):
  File "", line 1, in 
NameError: name 'run_one_line' is not defined
Current executable set to 'modTS' (x86_64).
(lldb)

===
Exactly the same output appears when lldb is started by:
==
(lldb) process attach -n modTS

==
 prowat% lldb -v
lldb-902.0.79.7
  Swift-4.1
===

This is a nuisance but LLDB is still usable.
 

OSX 10.13
Peter 



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


Re: [lldb-dev] [llvm-dev] Adding DWARF5 accelerator table support to llvm

2018-06-15 Thread Pavel Labath via lldb-dev
On Fri, 15 Jun 2018 at 20:14, David Blaikie  wrote:
>
> How do you handle name lookup for nested classes? They have the same problem 
> (they don't appear in all definitions) - don't appear in all descriptions of 
> the outer/parent class. (in theory we could ensure there's always at least a 
> declaration of the nested class - but we don't even do that if the nested 
> class is unused)
>
> Is it just the case that Clang doesn't mind you adding a new nested class but 
> it does mind you adding a new member function template? If so, maybe we could 
> change Clang to support adding new member function templates instead of 
> extending DWARF?


I was thinking about the same thing. It seems to me that this could be
viewed as a deficiency of our implementation of dwarf parsing. (It's a
pretty understandable deficiency, given that we are based on clang
(compiler), and it thinks of the types in the same way as C++ does --
incomplete; or complete with template members and all). However, these
template member functions should not impact anything "important" in
the class (data member layout, vtables, ...) so one could conceivably
have an implementation which allows member addition on the fly. And in
this case the existing accelerator tables would work perfectly -- we
would get a query "does this class have method X", we would look at
the accel table, and it would point us straight to X. However, I have
no idea how hard would it be to fit this scheme into the existing
clang/lldb design.



That said, having DWARF be able to represent the template member
functions in an abstract way also sounds like nice thing to have from
a debug info format.

pl
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


[lldb-dev] I'm going to sort the two big lists of files in the lldb xcode project file later today

2018-06-15 Thread Jason Molenda via lldb-dev
We maintain a few different branches of the lldb sources, e.g. to add swift 
support, and the xcode project files have diverged over time from llvm.org to 
our github repositories making git merging a real pain.  The two biggest 
sources of pain are the BuildFiles and FileReferences sections of the project 
file, where we've managed to get the files in completely different orders 
across our different branches.

I threw together a quick script to sort the files in these sections, and to 
segregate the swift etc files that aren't on llvm.org into their own sections 
so they don't cause merge headaches as often.

If anyone is maintaining a fork/branch of lldb where they've added files to the 
xcode project file, this sorting will be need hand cleanup from them too.  I 
suspect that everyone outside apple would be using cmake and ignoring the xcode 
project files -- but if this is a problem, please let me know and I'll delay.

Right now my cleanup script looks like this

#! /usr/bin/ruby
#


## Sort the BuildFile and FileReference sections of an Xcode project file,
## putting Apple/github-local files at the front to avoid merge conflicts.
#
## Run this in a directory with a project.pbxproj file.  The sorted version
## is printed on standard output.
#


# Files with these words in the names will be sorted into a separate section;
# they are only present in some repositories and so having them intermixed 
# can lead to merge failures.
segregated_filenames = ["Swift", "repl", "RPC"]

if !File.exists?("project.pbxproj")
puts "ERROR: project.pbxproj does not exist."
exit(1)
end

beginning  = Array.new   # All lines before "PBXBuildFile section"
files  = Array.new   # PBXBuildFile section lines -- sort these
middle = Array.new   # All lines between PBXBuildFile and PBXFileReference sections
refs   = Array.new   # PBXFileReference section lines -- sort these
ending = Array.new   # All lines after PBXFileReference section

all_lines = File.readlines 'project.pbxproj'

state = 1 # "begin"
all_lines.each do |l|
l.chomp
if state == 1 && l =~ /Begin PBXBuildFile section/
beginning.push(l)
state = 2
next
end
if state == 2 && l =~ /End PBXBuildFile section/
middle.push(l)
state = 3
next
end
if state == 3 && l =~ /Begin PBXFileReference section/
middle.push(l)
state = 4
next
end
if state == 4 && l =~ /End PBXFileReference section/
ending.push(l)
state = 5
next
end

if state == 1
beginning.push(l)
elsif state == 2
files.push(l)
elsif state == 3
middle.push(l)
elsif state == 4
refs.push(l)
else
ending.push(l)
end
end

# Sort FILES by the filename, putting swift etc in front

# key is filename
# value is array of text lines for that filename in the FILES text
# (libraries like libz.dylib seem to occur multiple times, probably
# once each for different targets).

files_by_filename = Hash.new { |k, v| k[v] = Array.new }

files.each do |l|
# 2669421A1A6DC2AC0063BE93 /* MICmdCmdTarget.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 266941941A6DC2AC0063BE93 /* MICmdCmdTarget.cpp */; };

if l =~ /^\s+([A-F0-9]{24})\s+\/\*\s+(.*?)\sin.*?\*\/.*?fileRef = ([A-F0-9]{24})\s.*$/
uuid = $1
filename = $2
fileref = $3
files_by_filename[filename].push(l)
end
end

# clear the FILES array

files = Array.new

# add the lines in sorted order.  First swift/etc, then everything else.

segregated_filenames.each do |keyword|
filenames = files_by_filename.keys
filenames.select {|l| l.include?(keyword) }.sort.each do |fn|
# re-add all the lines for the filename FN to our FILES array that we'll
# be outputting.
files_by_filename[fn].sort.each do |l|
files.push(l)
end
files_by_filename.delete(fn)
end
end

# All segregated filenames have been added to the FILES output array.
# Now add all the other lines, sorted by filename.

files_by_filename.keys.sort.each do |fn|
files_by_filename[fn].sort.each do |l|
files.push(l)
end
end

# Sort REFS by the filename, putting swift etc in front

refs_by_filename = Hash.new { |k, v| k[v] = Array.new }
refs.each do |l|
# 2611FF12142D83060017FEA3 /* SBValue.i */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.c.preprocessed; path = SBValue.i; sourceTree = ""; };

if l =~ /^\s+([A-F0-9]{24})\s+\/\*\s+(.*?)\s\*\/.*$/
uuid = $1
filename = $2
refs_by_filename[filename].push(l)
end
end

# clear the refs array

refs = Array.new

# add the lines in sorted order.  First swift/etc, then everything else.


segregated_filenames.each do |keyword|
filenames = refs_by_filename.keys
filenames.select {|l| l.include?(keyword) }.sort.each do |fn|
# re-add all the lines for the filename FN to our refs array that we'll