[Lldb-commits] [lldb] r282648 - Fix an issue where libc++ changed the type information we get for std::map::iterator, rendering LLDB unable to display elements vended by an iterator

[Enrico Granata via lldb-commits]

> On Sep 28, 2016, at 5:58 PM, Zachary Turner <zturner at google.com> wrote:
> 
> 
> 
> On Wed, Sep 28, 2016 at 4:02 PM Enrico Granata via lldb-commits <lldb-commits at lists.llvm.org <mailto:lldb-commits at lists.llvm.org>> wrote:
> 
> -
> +
> +  static ConstString g___i_("__i_");
> +
>    // this must be a ValueObject* because it is a child of the ValueObject we are
>    // producing children for
>    // it if were a ValueObjectSP, we would end up with a loop (iterator ->
> @@ -281,6 +287,57 @@ bool lldb_private::formatters::LibCxxMap
>                                     SyntheticChildrenTraversal::None),
>                         nullptr)
>                     .get();
> +
> +  if (!m_pair_ptr) {
> +    m_pair_ptr = valobj_sp->GetValueForExpressionPath(".__i_.__ptr_", nullptr, nullptr, nullptr,
> +                                                      ValueObject::GetValueForExpressionPathOptions()
> +                                                      .DontCheckDotVsArrowSyntax()
> +                                                      .SetSyntheticChildrenTraversal(
> +                                                                                     ValueObject::GetValueForExpressionPathOptions::
> +                                                                                     SyntheticChildrenTraversal::None),
> +                                                      nullptr)
> +    .get();
> +    if (m_pair_ptr) {
> +      auto __i_(valobj_sp->GetChildMemberWithName(g___i_, true));
> Is there any reason we need to use such weird variable names?  This looks like undefined behavior to me.  Variables with more than one adjacent underscore are reserved.

I am naming the variable the same as the corresponding variable in the type I am formatting. Future me is usually thankful when I do that.. it helps keep track of what ValueObject matches what child element; in this case, our iterators have an __i_ child

>  
> +      lldb::TemplateArgumentKind kind;
> +      if (!__i_) {
> +        m_pair_ptr = nullptr;
> +        return false;
> +      }
> +      CompilerType pair_type(__i_->GetCompilerType().GetTemplateArgument(0, kind));
> +      std::string name; uint64_t bit_offset_ptr; uint32_t bitfield_bit_size_ptr; bool is_bitfield_ptr;
> +      pair_type = pair_type.GetFieldAtIndex(0, name, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
> +      if (!pair_type) {
> +        m_pair_ptr = nullptr;
> +        return false;
> +      }
> +
> +      auto addr(m_pair_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS));
> +      m_pair_ptr = nullptr;
> +      if (addr && addr!=LLDB_INVALID_ADDRESS) {
> Can you do an early return here?

Like instead of if (foo) { stuff }
if (!foo) return;
stuff

Sure. It didn't seem that deeply nested to be worth it, but I see no reason why I couldn't..

>  
> +        ClangASTContext *ast_ctx = llvm::dyn_cast_or_null<ClangASTContext>(pair_type.GetTypeSystem());
> +        if (!ast_ctx)
> +          return false;
> +        CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(ConstString(), {
> +          {"ptr0",ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
> +          {"ptr1",ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
> +          {"ptr2",ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
> +          {"cw",ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
> +          {"payload",pair_type}
> +        });
> +        DataBufferSP buffer_sp(new DataBufferHeap(tree_node_type.GetByteSize(nullptr),0));
> +        ProcessSP process_sp(target_sp->GetProcessSP());
> +        Error error;
> +        process_sp->ReadMemory(addr, buffer_sp->GetBytes(), buffer_sp->GetByteSize(), error);
> +        if (error.Fail())
> +          return false;
> +        DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(), process_sp->GetAddressByteSize());
> +        auto pair_sp = CreateValueObjectFromData("pair", extractor, valobj_sp->GetExecutionContextRef(), tree_node_type);
> +        if (pair_sp)
> +          m_pair_sp = pair_sp->GetChildAtIndex(4,true);
> +      }
> +    }
> +  }
> 
>    return false;
>  }
> @@ -293,9 +350,11 @@ size_t lldb_private::formatters::LibCxxM
>  lldb::ValueObjectSP
>  lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::GetChildAtIndex(
>      size_t idx) {
> -  if (!m_pair_ptr)
> -    return lldb::ValueObjectSP();
> -  return m_pair_ptr->GetChildAtIndex(idx, true);
> +  if (m_pair_ptr)
> +    return m_pair_ptr->GetChildAtIndex(idx, true);
> +  if (m_pair_sp)
> +    return m_pair_sp->GetChildAtIndex(idx, true);
> +  return lldb::ValueObjectSP();
>  }
> 
>  bool lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
> 
> Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h?rev=282648&r1=282647&r2=282648&view=diff <http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h?rev=282648&r1=282647&r2=282648&view=diff>
> ==============================================================================
> --- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h (original)
> +++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.h Wed Sep 28 17:53:16 2016
> @@ -81,6 +81,7 @@ public:
> 
>  private:
>    ValueObject *m_pair_ptr;
> +  lldb::ValueObjectSP m_pair_sp;
>  };
> 
>  SyntheticChildrenFrontEnd *
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org <mailto:lldb-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits>

Thanks,
- Enrico
📩 egranata@.com ☎️ 27683

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160929/87c62bd8/attachment-0001.html>