Handle the case when a variable is only valid in part of the enclosing scope

[tberghammer]

Summary

Handle the case when a variable is only valid in part of the enclosing scope

DWARF stores this information in the DW_AT_start_scope attribute. This
CL add support for this attribute and also changes the functions
displaying frame variables to only display the variables currently in
scope.

Note: This CL is part of an effort to make LLDB capable of debugging Java code JIT-ed by the android runtime

[clayborg]

DWARF already has support for this using location lists. I would rather see this done by having the DWARF parser modify the DWARF expression to be a location list based. I don't mean have the compiler change, but just how we store the location in the variable. This would mean the ONLY change will be in the DWARF parser. We don't need to track any ranges or anything in the variable this way.

So if you have a variable:

DW_TAG_subprogram:
DW_AT_name("main")
DW_AT_low_pc(0x1000)
DW_AT_high_pc(0x2000)

    DW_TAG_variable:
    DW_AT_name("a")
    DW_AT_start_scope (0x20)
    DW_AT_location (0x11 0x22 0x33) (location is described by a block of DW_OP_expression bytes)

Then we would make a location list expression where the DWARFExpression's type is switched from:

NonLocationList with bytes of 0x11, 0x22, 0x33

over to a DWARFExpression whose type is:

RegularLocationList with [0x1020-0x2000): 0x11, 0x22, 0x33

If that doesn't make sense let me know. If we do this, then the ONLY change is in the DWARF parser since we already support location list based expressions.

[tberghammer]

There is a (minor) difference between a variable being out of scope and having no location information. Being out of scope means that the variable hasn't declared yet while having no location information means that the variable is declared, it is in scope, but it is optimized out so the debugger can't display it's value. I introduced this new dimension of data to properly represent the difference between the 2 scenario.

In case of C/C++ the difference is very small (and I am not aware of any C/C++ compiler emitting DW_AT_start_scope) but based on our understanding of the dwarf spec it is valid if a compiler emits no lexical blocks at all and specifies the scope of all variable inside a function body using DW_AT_start_scope attributes (we use this method in the android java runtime because the full scoping information is not available). In this scenario the difference between being out of scope and being optimized out is significant because we can have two variable with the same name inside a function (same lexical block) with different scoping information what will ensure that the variable names are unique at any given point of the function (assuming the dwarf information is correct).

If we want to represent this information with using a dwarf expression then we need to create a new expression value what I think is a much more intrusive change as it will modify we handle the standard dwarf expressions. In my opinion it is a much better option to introduce the scoping information as a high level concept the way I implemented it so all dwarf information (possibly) generated by the compilers can be handled correctly.

Let me know what do you think.

[clayborg]

I would rather do this by modifying the expression locations if at all possible. I would like to not having to modify each variable to contain a RangeList as this is extra data per variable. I would be fine if we just add a new enumeration to the DWARFExpression like StartScopeLocationList which is the same as "RegularLocationList". The contents would be the same (start address + end address followed by location expression block), but it would mean that the locations indicate that the variable is in scope (instead of just not available). Then we just need to modify Variable::IsInScope() to check what it did before, and if the DWARFExpression is StartScopeLocationList, just make sure we have a location that contains the current address. Does that sound acceptable? I am also OK with modifying the DWARFExpression class to be able to return an enumeration when the DWARFExpression is evaluated so it can return things like:

eDWARFExpressionSuccess
eDWARFExpressionVariableNotAvailable
eDWARFExpressionOutOfScope
eDWARFExpressionError

Then RegularLocationList would return eDWARFExpressionVariableNotAvailable when there is no location that the variable is available, and StartScopeLocationList would return eDWARFExpressionOutOfScope.

[tberghammer]

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:

• 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).
• 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.
• 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):

• Create a new DWARFVariableLocationDescription class
• Remove all location list support from the DWARFExpression class so it really just evaluates dwarf expressions
• 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)

[clayborg]

In 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:

• 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).
• 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.
• 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):

• Create a new DWARFVariableLocationDescription class
• Remove all location list support from the DWARFExpression class so it really just evaluates dwarf expressions
• 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?

[tberghammer]

I can live with that but I still think the scope information belongs to the variable and not to the DWARF expression representing it's location. As far as I see keeping the information inside the variable have lower memory impact (assuming an empty list means full scope) because we have less Variable object then DWARFExpression object and it isn't complicate other use cases of DWARFExpression where scope isn't used at all.

So all in all I can move the information to the DWARFExpression but I think keeping them in the Variable class is better. Let me know what do you think.

[clayborg]

In D17449#359874, @tberghammer wrote:

I can live with that but I still think the scope information belongs to the variable and not to the DWARF expression representing it's location. As far as I see keeping the information inside the variable have lower memory impact (assuming an empty list means full scope) because we have less Variable object then DWARFExpression object and it isn't complicate other use cases of DWARFExpression where scope isn't used at all.

So all in all I can move the information to the DWARFExpression but I think keeping them in the Variable class is better. Let me know what do you think.

I would vote for modifying the DWARFExpression only because we might eventually have other DIEs that have locations and start scope attributes that might use DWARFExpressions. But I don't see a problem with either if you really feel strongly...

[tberghammer]

I added the memory optimization to represent the case when no DW_AT_start_scope is specified as an empty range map. This way the memory impact for C/C++ will be only an empty std::vector per variable.

Regarding the location of the code I don't want to add it to DWARFExpression because dwarf expressions are used for much more thing then variable locations on their own. If an other DIE needs this information in the future then I would suggest to introduce a new class (don't have any good name for it) containing a DWARFExpression representing the location and a RangeMap representing the scope information.

In long term (don't have time from for it) I would also want to split DWARFExpression to 2 class where one of them know how to evaluate a dwarf expression and can represent only a single dwarf expression. Next to it I want to introduce a new DWARFLocationList class what knows about the location list part with no knowledge about the DWARF opcodes. The 2 class can be closely integrated so we don't need to parse anything upfront while we are keeping the 2 concept separated.

include/lldb/Core/RangeMap.h

public:
 
         ~RangeArray() = default;
 
         void
         Append (const Entry &entry)
         {
             m_entries.push_back (entry);
         }
-        
+        void
+        Append (B base, S size)
+        {
+            m_entries.emplace_back(base, size);
+        }
+
         bool
         RemoveEntrtAtIndex (uint32_t idx)
         {
             if (idx < m_entries.size())
             {
                 m_entries.erase (m_entries.begin() + idx);
                 return true;
             }

include/lldb/Symbol/Block.h

public:
     /// @return
     ///     A variable list shared pointer that contains all variables
     ///     for this block.
     //------------------------------------------------------------------
     uint32_t
     AppendBlockVariables (bool can_create,
                           bool get_child_block_variables,
                           bool stop_if_child_block_is_inlined_function,
+                          const std::function<bool(Variable*)>& filter,
                           VariableList *variable_list);
-                          
     //------------------------------------------------------------------
     /// Appends the variables from this block, and optionally from all
     /// parent blocks, to \a variable_list.
     ///
     /// @param[in] can_create
     ///     If \b true, the variables can be parsed if they already
     ///     haven't been, else the current state of the block will be
     ///     returned. Passing \b true for this parameter can be used
public:
     ///     All variables in this block, and optionally all parent
     ///     blocks will be added to this list.
     ///
     /// @return
     ///     The number of variable that were appended to \a
     ///     variable_list.
     //------------------------------------------------------------------
     uint32_t
-    AppendVariables (bool can_create, 
-                     bool get_parent_variables, 
+    AppendVariables (bool can_create,
+                     bool get_parent_variables,
                      bool stop_if_block_is_inlined_function,
+                     const std::function<bool(Variable*)>& filter,
                      VariableList *variable_list);
 
     //------------------------------------------------------------------
     /// Get const accessor for any inlined function information.
     ///
     /// @return
     ///     A const pointer to any inlined function information, or nullptr
     ///     if this is a regular block.

include/lldb/Symbol/Variable.h

 #define liblldb_Variable_h_
 
 #include <memory>
 #include <vector>
 
 #include "lldb/lldb-private.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/Core/Mangled.h"
+#include "lldb/Core/RangeMap.h"
 #include "lldb/Core/UserID.h"
 #include "lldb/Expression/DWARFExpression.h"
 #include "lldb/Symbol/Declaration.h"
 
 namespace lldb_private {
 
 class Variable : public UserID,
     public std::enable_shared_from_this<Variable>
 {
 public:
+    typedef RangeArray<lldb::addr_t, lldb::addr_t, 1> RangeList;
+    typedef RangeList::Entry Range;
+
     //------------------------------------------------------------------
     // Constructors and Destructors
     //------------------------------------------------------------------
     Variable (lldb::user_id_t uid,
               const char *name, 
               const char *mangled,  // The mangled or fully qualified name of the variable.
               const lldb::SymbolFileTypeSP &symfile_type_sp,
               lldb::ValueType scope,
               SymbolContextScope *owner_scope,
+              const RangeList& scope_range,
               Declaration* decl,
               const DWARFExpression& location,
               bool external,
               bool artificial,
               bool static_member = false);
 
     virtual
     ~Variable();
AutoComplete (const ExecutionContext &exe_ctx,
                   StringList &matches,
                   bool &word_complete);
 
     CompilerDeclContext
     GetDeclContext ();
 
     CompilerDecl
     GetDecl ();
+
 protected:
     ConstString m_name;                 // The basename of the variable (no namespaces)
     Mangled m_mangled;                  // The mangled name of the variable
     lldb::SymbolFileTypeSP m_symfile_type_sp;   // The type pointer of the variable (int, struct, class, etc)
     lldb::ValueType m_scope;            // global, parameter, local
     SymbolContextScope *m_owner_scope;  // The symbol file scope that this variable was defined in
+    RangeList m_scope_range;            // The list of ranges inside the owner's scope where this variable is valid
     Declaration m_declaration;          // Declaration location for this item.
     DWARFExpression m_location;         // The location of this variable that can be fed to DWARFExpression::Evaluate()
     uint8_t m_external:1,               // Visible outside the containing compile unit?
             m_artificial:1,             // Non-zero if the variable is not explicitly declared in source
             m_loc_is_const_data:1,      // The m_location expression contains the constant variable value data, not a DWARF location
             m_static_member:1;          // Non-zero if variable is static member of a class or struct.
 private:
     Variable(const Variable& rhs);
     Variable& operator=(const Variable& rhs);
 };
 
 } // namespace lldb_private
 
 #endif  // liblldb_Variable_h_

source/API/SBBlock.cpp

 }
 
 void
 SBBlock::AppendVariables (bool can_create, bool get_parent_variables, lldb_private::VariableList *var_list)
 {
     if (IsValid())
     {
         bool show_inline = true;
-        m_opaque_ptr->AppendVariables (can_create, get_parent_variables, show_inline, var_list);
+        m_opaque_ptr->AppendVariables (can_create,
+                                       get_parent_variables,
+                                       show_inline,
+                                       [](Variable*) { return true; },
+                                       var_list);
     }
 }
 
 SBBlock
 SBBlock::GetParent ()
 {
     SBBlock sb_block;
     if (m_opaque_ptr)

source/API/SBFrame.cpp

if (target && process)
                 SymbolContext sc (frame->GetSymbolContext (eSymbolContextBlock));
 
                 if (sc.block)
                 {
                     const bool can_create = true;
                     const bool get_parent_variables = true;
                     const bool stop_if_block_is_inlined_function = true;
 
-                    if (sc.block->AppendVariables (can_create, 
+                    if (sc.block->AppendVariables (can_create,
                                                    get_parent_variables,
                                                    stop_if_block_is_inlined_function,
+                                                   [frame](Variable* v) { return v->IsInScope(frame); },
                                                    &variable_list))
                     {
                         var_sp = variable_list.FindVariable (ConstString(name));
                     }
                 }
 
                 if (var_sp)
                 {
if (target && process)
                         const bool can_create = true;
                         const bool get_parent_variables = true;
                         const bool stop_if_block_is_inlined_function = true;
 
                         if (sc.block)
                             sc.block->AppendVariables(can_create,
                                                       get_parent_variables,
                                                       stop_if_block_is_inlined_function,
+                                                      [frame](Variable* v) { return v->IsInScope(frame); },
                                                       &variable_list);
                         if (value_type == eValueTypeVariableGlobal)
                         {
                             const bool get_file_globals = true;
                             VariableList *frame_vars = frame->GetVariableList(get_file_globals);
                             if (frame_vars)
                                 frame_vars->AppendVariablesIfUnique(variable_list);
                         }

source/Core/Address.cpp

case DumpStyleDetailedSymbolContext:
                 
                 if (sc.block)
                 {
                     bool can_create = true;
                     bool get_parent_variables = true;
                     bool stop_if_block_is_inlined_function = false;
                     VariableList variable_list;
                     sc.block->AppendVariables (can_create,
-                                               get_parent_variables, 
-                                               stop_if_block_is_inlined_function, 
+                                               get_parent_variables,
+                                               stop_if_block_is_inlined_function,
+                                               [](Variable*) { return true; },
                                                &variable_list);
-                    
                     const size_t num_variables = variable_list.GetSize();
                     for (size_t var_idx = 0; var_idx < num_variables; ++var_idx)
                     {
                         Variable *var = variable_list.GetVariableAtIndex (var_idx).get();
                         if (var && var->LocationIsValidForAddress (*this))
                         {
                             s->Indent();
                             s->Printf ("   Variable: id = {0x%8.8" PRIx64 "}, name = \"%s\"",

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if ((tag == DW_TAG_variable) ||
             uint32_t i;
             DWARFFormValue type_die_form;
             DWARFExpression location(die.GetCU());
             bool is_external = false;
             bool is_artificial = false;
             bool location_is_const_value_data = false;
             bool has_explicit_location = false;
             DWARFFormValue const_value;
+            Variable::RangeList scope_ranges;
             //AccessType accessibility = eAccessNone;
 
             for (i=0; i<num_attributes; ++i)
             {
                 dw_attr_t attr = attributes.AttributeAtIndex(i);
                 DWARFFormValue form_value;
                 
                 if (attributes.ExtractFormValueAtIndex(i, form_value))
if ((tag == DW_TAG_variable) ||
                         break;
                     case DW_AT_specification:
                     {
                         DWARFDebugInfo* debug_info = DebugInfo();
                         if (debug_info)
                             spec_die = debug_info->GetDIE(DIERef(form_value));
                         break;
                     }
+                    case DW_AT_start_scope:
+                    {
+                        if (form_value.Form() == DW_FORM_sec_offset)
+                        {
+                            DWARFRangeList dwarf_scope_ranges;
+                            const DWARFDebugRanges* debug_ranges = DebugRanges();
+                            debug_ranges->FindRanges(form_value.Unsigned(), dwarf_scope_ranges);
+
+                            // All DW_AT_start_scope are relative to the base address of the
+                            // compile unit. We add the compile unit base address to make
+                            // sure all the addresses are properly fixed up.
+                            for (size_t i = 0, count = dwarf_scope_ranges.GetSize(); i < count; ++i)
+                            {
+                                const DWARFRangeList::Entry& range = dwarf_scope_ranges.GetEntryRef(i);
+                                scope_ranges.Append(range.GetRangeBase() + die.GetCU()->GetBaseAddress(),
+                                                    range.GetByteSize());
+                            }
+                        }
+                        else
+                        {
+                            // TODO: Handle the case when DW_AT_start_scope have form constant. The 
+                            // dwarf spec is a bit ambiguous about what is the expected behavior in
+                            // case the enclosing block have a non coninious address range and the
+                            // DW_AT_start_scope entry have a form constant. 
+                            GetObjectFile()->GetModule()->ReportWarning ("0x%8.8" PRIx64 ": DW_AT_start_scope has unsupported form type (0x%x)\n",
+                                                                         die.GetID(),
+                                                                         form_value.Form());
+                        }
+
+                        scope_ranges.Sort();
+                        scope_ranges.CombineConsecutiveRanges();
+                    }
                     case DW_AT_artificial:      is_artificial = form_value.Boolean(); break;
                     case DW_AT_accessibility:   break; //accessibility = DW_ACCESS_to_AccessType(form_value.Unsigned()); break;
                     case DW_AT_declaration:
                     case DW_AT_description:
                     case DW_AT_endianity:
                     case DW_AT_segment:
-                    case DW_AT_start_scope:
                     case DW_AT_visibility:
                     default:
                     case DW_AT_abstract_origin:
                     case DW_AT_sibling:
                         break;
                     }
                 }
             }
if ((tag == DW_TAG_variable) ||
                     symbol_context_scope = sc.comp_unit;
                     break;
                 }
             }
 
             if (symbol_context_scope)
             {
                 SymbolFileTypeSP type_sp(new SymbolFileType(*this, DIERef(type_die_form).GetUID()));
-                
                 if (const_value.Form() && type_sp && type_sp->GetType())
                     location.CopyOpcodeData(const_value.Unsigned(), type_sp->GetType()->GetByteSize(), die.GetCU()->GetAddressByteSize());
-                
+                // If no scope_ranges were specified then copy them over from the parent block
+                Block* context_block = symbol_context_scope->CalculateSymbolContextBlock();
+                if (scope_ranges.IsEmpty() && context_block)
+                {
+                    for (size_t i = 0, count = context_block->GetNumRanges(); i < count; ++i)
+                    {
+                        AddressRange range;
+                        if (context_block->GetRangeAtIndex(i, range))
+                            scope_ranges.Append(range.GetBaseAddress().GetFileAddress(),
+                                                range.GetByteSize());
+                    }
+                }
+
                 var_sp.reset (new Variable (die.GetID(),
-                                            name, 
+                                            name,
                                             mangled,
                                             type_sp,
-                                            scope, 
-                                            symbol_context_scope, 
-                                            &decl, 
-                                            location, 
-                                            is_external, 
+                                            scope,
+                                            symbol_context_scope,
+                                            scope_ranges,
+                                            &decl,
+                                            location,
+                                            is_external,
                                             is_artificial,
                                             is_static_member));
                 
                 var_sp->SetLocationIsConstantValueData (location_is_const_value_data);
             }
             else
             {
                 // Not ready to parse this variable yet. It might be a global

source/Symbol/Block.cpp

Block::GetBlockVariableList (bool can_create)
     }
     return m_variable_list_sp;
 }
 
 uint32_t
 Block::AppendBlockVariables (bool can_create,
                              bool get_child_block_variables,
                              bool stop_if_child_block_is_inlined_function,
+                             const std::function<bool(Variable*)>& filter,
                              VariableList *variable_list)
 {
     uint32_t num_variables_added = 0;
     VariableList *block_var_list = GetBlockVariableList (can_create).get();
     if (block_var_list)
     {
-        num_variables_added += block_var_list->GetSize();
-        variable_list->AddVariables (block_var_list);
+        for (size_t i = 0; i < block_var_list->GetSize(); ++i)
+        {
+            VariableSP variable = block_var_list->GetVariableAtIndex(i);
+            if (filter(variable.get()))
+            {
+                num_variables_added++;
+                variable_list->AddVariable(variable);
+            }
+        }
     }
-    
     if (get_child_block_variables)
     {
         collection::const_iterator pos, end = m_children.end();
         for (pos = m_children.begin(); pos != end; ++pos)
         {
             Block *child_block = pos->get();
             if (stop_if_child_block_is_inlined_function == false || 
                 child_block->GetInlinedFunctionInfo() == nullptr)
             {
                 num_variables_added += child_block->AppendBlockVariables (can_create,
                                                                           get_child_block_variables,
                                                                           stop_if_child_block_is_inlined_function,
+                                                                          filter,
                                                                           variable_list);
             }
         }
     }
     return num_variables_added;
 }
 
 uint32_t
 Block::AppendVariables 
 (
     bool can_create, 
     bool get_parent_variables, 
     bool stop_if_block_is_inlined_function,
+    const std::function<bool(Variable*)>& filter,
     VariableList *variable_list
 )
 {
     uint32_t num_variables_added = 0;
     VariableListSP variable_list_sp(GetBlockVariableList(can_create));
 
     bool is_inlined_function = GetInlinedFunctionInfo() != nullptr;
-    if (variable_list_sp.get())
+    if (variable_list_sp)
+    {
+        for (size_t i = 0; i < variable_list_sp->GetSize(); ++i)
         {
-        num_variables_added = variable_list_sp->GetSize();
-        variable_list->AddVariables(variable_list_sp.get());
+            VariableSP variable = variable_list_sp->GetVariableAtIndex(i);
+            if (filter(variable.get()))
+            {
+                num_variables_added++;
+                variable_list->AddVariable(variable);
+            }
+        }
     }
-    
     if (get_parent_variables)
     {
         if (stop_if_block_is_inlined_function && is_inlined_function)
             return num_variables_added;
             
         Block* parent_block = GetParent();
         if (parent_block)
-            num_variables_added += parent_block->AppendVariables (can_create, get_parent_variables, stop_if_block_is_inlined_function, variable_list);
+            num_variables_added += parent_block->AppendVariables(can_create,
+                                                                 get_parent_variables,
+                                                                 stop_if_block_is_inlined_function,
+                                                                 filter,
+                                                                 variable_list);
     }
     return num_variables_added;
 }
 
 CompilerDeclContext
 Block::GetDeclContext()
 {
     ModuleSP module_sp = CalculateSymbolContextModule ();

source/Symbol/Variable.cpp

 // Variable constructor
 //----------------------------------------------------------------------
 Variable::Variable (lldb::user_id_t uid,
                     const char *name,
                     const char *mangled,  // The mangled or fully qualified name of the variable.
                     const lldb::SymbolFileTypeSP &symfile_type_sp,
                     ValueType scope,
                     SymbolContextScope *context,
+                    const RangeList& scope_range,
                     Declaration* decl_ptr,
                     const DWARFExpression& location,
                     bool external,
                     bool artificial,
                     bool static_member) :
     UserID(uid),
     m_name(name),
     m_mangled (ConstString(mangled)),
     m_symfile_type_sp(symfile_type_sp),
     m_scope(scope),
     m_owner_scope(context),
+    m_scope_range(scope_range),
     m_declaration(decl_ptr),
     m_location(location),
     m_external(external),
     m_artificial(artificial),
     m_static_member(static_member)
 {
 }
 
case eValueTypeVariableLocal:
         {
             // We don't have a location list, we just need to see if the block
             // that this variable was defined in is currently
             Block *deepest_frame_block = frame->GetSymbolContext(eSymbolContextBlock).block;
             if (deepest_frame_block)
             {
                 SymbolContext variable_sc;
                 CalculateSymbolContext (&variable_sc);
+
                 // Check for static or global variable defined at the compile unit 
                 // level that wasn't defined in a block
                 if (variable_sc.block == nullptr)
-                    return true;    
+                    return true;
 
-                if (variable_sc.block == deepest_frame_block)
-                    return true;
-                return variable_sc.block->Contains (deepest_frame_block);
+                // Check if the variable is valid in the current block
+                if (variable_sc.block != deepest_frame_block &&
+                    !variable_sc.block->Contains(deepest_frame_block))
+                {
+                    return false;
+                }
+
+                addr_t file_address = frame->GetFrameCodeAddress().GetFileAddress();
+                return m_scope_range.FindEntryThatContains(file_address) != nullptr;
             }
         }
         break;
 
     default:
         break;
     }
     return false;

source/Target/StackFrame.cpp

if (m_flags.IsClear(RESOLVED_VARIABLES))
         Block *frame_block = GetFrameBlock();
         
         if (frame_block)
         {
             const bool get_child_variables = true;
             const bool can_create = true;
             const bool stop_if_child_block_is_inlined_function = true;
             m_variable_list_sp.reset(new VariableList());
-            frame_block->AppendBlockVariables(can_create, get_child_variables, stop_if_child_block_is_inlined_function, m_variable_list_sp.get());
+            frame_block->AppendBlockVariables(can_create,
+                                              get_child_variables,
+                                              stop_if_child_block_is_inlined_function,
+                                              [this](Variable* v) { return v->IsInScope(this); },
+                                              m_variable_list_sp.get());
         }
     }
     
     if (m_flags.IsClear(RESOLVED_GLOBAL_VARIABLES) &&
         get_file_globals)
     {
         m_flags.Set(RESOLVED_GLOBAL_VARIABLES);
         
StackFrame::GetInScopeVariableList (bool get_file_globals)
     if (m_sc.block)
     {
         const bool can_create = true;
         const bool get_parent_variables = true;
         const bool stop_if_block_is_inlined_function = true;
         m_sc.block->AppendVariables (can_create, 
                                      get_parent_variables,
                                      stop_if_block_is_inlined_function,
+                                     [this](Variable* v) { return v->IsInScope(this); },
                                      var_list_sp.get());
     }
                      
     if (m_sc.comp_unit && get_file_globals)
     {
         VariableListSP global_variable_list_sp (m_sc.comp_unit->GetVariableList(true));
         if (global_variable_list_sp)
             var_list_sp->AddVariables (global_variable_list_sp.get());