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



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

Reply via email to