[Lldb-commits] [PATCH] D17449: Handle the case when a variable is only valid in part of the enclosing scope

[Greg Clayton via lldb-commits]

clayborg added a comment.

In http://reviews.llvm.org/D17449#359717, @tberghammer wrote:

> We have 2 (+1) independent dimension here:
>
> 1. List of ranges where the variable is in scope
> 2. List of ranges where the variable is available
> 3. Format of the location list (it is more of an implementation details because of a format change in dwarf5)
>
>   To represent all of the possible combination for a variable we have to handle all dimension separately. I think we have several options:
> 4. Treat the scope as a top level parameter of a variable (same way as we treat the enclosing lexical block) the way I implemented it and if we are worrying about the memory implication then a possible optimization is to say that an empty location list means that the scope is the same as the enclosing block (This way we will add 2 pointer to each Variable in the general case what will be a negligible size increase).
> 5. Make DWARFExpression store 2 list of ranges (1 for the locations and 1 for the scope). I think it is a very similar implementation then the current one but with storing the data in a wrong location so I am strongly against this option.
> 6. Try to make DWARFExpression smart enough to handle the 2 layer of the location expressions (I think this is you suggestion). I think to implement this we need the following changes:
>   - Change DWARFExpression to store a RangeMap<lldb::addr_t, lldb::addr_t, DataExtractor> for the location list instead of just a DataExtractor because we need some data format what can be modified instead of the current implementation where DWARFExpression stores only a DataExtractor referencing a blob of data.
>   - Teach SymbolFileDWARF about the details of the DWARFExpression format so it can modify it.
>   - Abuse the DWARFExpression a bit so it can return some additional value next to the result of the evaluation for the DWARFExpression.
>
>     Based on the steps I listed above I don't think it is reasonable to merge the scope information into the DWARFExpression especially if we consider that dwarf expressions are used for several other thing then describing the location of variables. If we don't want to add the scoping information to the Variable class then we can go with a plan like this (I don't really like it):
> 7. Create a new DWARFVariableLocationDescription class
> 8. Remove all location list support from the DWARFExpression class so it really just evaluates dwarf expressions
> 9. In the new class store a map from address range to (dwarf expression + bitfield) where the bitfield value tell if the variable is in/out of scope and if it has/hasn't have location information
>
>   I think in one side this would be a fairly clean abstraction and it would also simplify the DWARFExpression class but on the other side I am almost certain then it would have more memory and performance impact then my current implementation (especially if the memory optimization I mentioned is applied) because it will have to create a range map from each dwarf expression and store it in memory.
>
>   All in all I don't really share your concern regarding memory impact, but I am happy to optimize it to the point where we use only 3 extra pointer (1 std::vector) compared to the original situation in the common (C/C++) case. Looking into the other options I don't think any of them is any better in terms of memory/performance and they will complicate the use of dwarf expressions all over lldb (the DWARFVariableLocationDescription class would make things cleaner but I don't see any good way to implement it without a higher memory penalty then my current implementation)


I can see your point. I don't want to change the DWARFExpression class to just contain a list of ranges and have DWARFExpression _only_ parse single DWARF location expressions because this will cause us to have to parse the information and pull out the addresses just so we can put it into a variable that may never get displayed and this is work we don't need to do. It is actually stored in the DWARF (if the DW_AT_locations has a reference it becomes an offset into the .debug_loc section which contains data already encoded with start address, end address, block, repeat until terminate), so lets not change DWARFExpression.

How about we add the in scope range list that was previously added to the variable to the DWARFExpression class instead? Just in case we have things other that a variable that has a DW_AT_location or DW_AT_data_member_location that also use the DW_AT_start_scope. Then you can ask your DWARFExpression if it is in scope before evaluating it. Then if the in scope range list is empty, it is in scope. Then we check if we have a location list, if we do, make sure the address is in one of those ranges and if not the variable is not available. Else find the right location expression and evaluate it. How does that sound?


http://reviews.llvm.org/D17449