[lldb-dev] RFC: -flimit-debug-info + frame variable

[Greg Clayton via lldb-dev]

> On Jul 21, 2020, at 9:27 AM, Pavel Labath <pavel at labath.sk> wrote:
> 
> On 20/07/2020 23:25, Jim Ingham wrote:
>> It seems like you are having to work hard in the ValueObject system because you don’t want to use single AST Type for the ValueObject’s type.  Seems like it be much simpler if you could cons up a complete type in the ScratchASTContext, and then use the underlying TypeSystem to do the layout computation.
>> 
>> Preserving the full type in the scratch context also avoids other problems.  For instance, suppose module A has a class that has an opaque reference to a type B.  There is a full definition of B in modules B and C.  If you make up a ValueObject for an object of type A resolving the full type to the one in Module B you can get into trouble.  Suppose the next user step is over the dlclose of module B.  When the local variable goes to see if it has changed, it will stumble across a type reference to a module that’s no longer present in the program.  And if somebody calls RemoveOrphanedModules it won’t even be in the shared module cache.
>> 
>> You can try to finesse this by saying you can choose the type from the defining module so it can’t go away.  But a) I don’t think you can know that for non-virtual classes in C++ and I don’t think you guarantee you can know how to do that for any given language.
>> 
>> I wonder if it wouldn’t be a better approach to build up a full compiler-type by importing the types you find into the scratch AST context.  That way you know they can’t go away.   And since you still have a full CompilerType for the variable, you can let the languages tell you where to find children based on their knowledge of the types.
>> 
> 
> I do see the attractiveness of constructing of a full compiler type. The
> reason I am hesitant to go that way, because it seems to me that this
> would negate the two main benefits of the frame variable command over
> the expression evaluator: a) it's fast; b) it's less likely to crash.
> 
> And while I don't think it will be as slow or as crashy as the
> expression evaluator, the usage of the ast importer will force a lot
> more types to be parsed than are strictly needed for this functionality.
> And the insertion of all potentially conflicting types from different
> modules into a single ast context is also somewhat worrying.

I agree here. Frame variable should do as little as possible when dealing with a ValueObject and its type, so only completing the parts of the type we know are transparent it a good approach.
> 
> The dlclose issue is an interesting one. Presumably, we could ensure
> that the module does not go away by storing a module shared (or weak?)
> pointer somewhere inside the value object. BTW, how does this work with
> ValueObject casts right now? If I cast a ValueObject to a CompilerType
> belonging to a different module, does anything ensure this module does
> not go away? Or when dereferencing a pointer to an type which is not
> complete in the current module?

I am not sure dlclose is a problem, the module won't usually be cleaned up. And that shared library shouldn't have the definition we need and be able to be unloaded IIUC how the -flimit-debug-info stuff works.

> 
> I'm hoping that this stuff won't be "hard work". I haven't prototyped
> the code yet, but I am hoping to keep this lookup code in under 200 LOC.
> And as Greg points out, there are ways to put this stuff into the type
> system -- I'm just not sure whether that is needed given that the
> ValueObject class is the only user of the GetIndexOfChildMemberWithName
> interface. The whole function is pretty clearly designed with
> ValueObject::GetChildMemberWithName in mind.

We should be able to code it into ValueObject, or maybe just into TypeSystem base class?
> 
> Another thing I like about this approach is that it will mostly use the
> same code path for the limit and no-limit debug info scenarios. OTOH,
> I'm pretty sure we would want to use the scratch context thingy only for
> types that are really not complete in their own modules, which would
> leave the scratch context method as a fairly complex, but rarely
> exercised path.
> 
> pl