[llvm] r305229 - Fix a null pointer dereference in llvm-pdbutil pretty.

[Zachary Turner via llvm-commits]

Because the test that tests everything would have been a superset of this
test, making this redundant. Not a big deal I suppose but that was the
thinking
On Mon, Jun 19, 2017 at 9:57 AM David Blaikie <dblaikie at gmail.com> wrote:

> On Mon, Jun 19, 2017 at 9:53 AM Zachary Turner <zturner at google.com> wrote:
>
>> Yea, i had put this off because i was planning on adding a test where I
>> hand constructed a yaml file that would systematically test every possible
>> symbol. To add a test for this now would require editing some source by
>> hand, compiling with msvc, and checking in the updated pdb in binary. Do
>> you think I should just go ahead and do that, or work on the every symbol
>> test?
>>
>
> Not sure I quite follow - if the YAML based testing can hit this, why
> would it need to wait for a YAML test that tests everything? Could add one
> that tests this then add all the other cases to it in a follow-up?
>
> But if it's coming 'soon', either way's probably OK. Would be good to
> followup on this thread with the revision number of the test case for
> posterity. :)
>
> - Dave
>
>
>> On Mon, Jun 19, 2017 at 9:28 AM David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> Test coverage?
>>>
>>> On Mon, Jun 12, 2017 at 1:47 PM Zachary Turner via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: zturner
>>>> Date: Mon Jun 12 15:46:35 2017
>>>> New Revision: 305229
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=305229&view=rev
>>>> Log:
>>>> Fix a null pointer dereference in llvm-pdbutil pretty.
>>>>
>>>> Static data members were causing a problem because I mistakenly
>>>> assumed all members would affect a class's layout and so the
>>>> Layout member would be non-null.
>>>>
>>>> Modified:
>>>>     llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp
>>>>     llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp
>>>>
>>>> Modified: llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp?rev=305229&r1=305228&r2=305229&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp (original)
>>>> +++ llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp Mon Jun 12 15:46:35 2017
>>>> @@ -181,13 +181,14 @@ void UDTLayoutBase::initializeChildren(c
>>>>        if (Data->getDataKind() == PDB_DataKind::Member)
>>>>          Members.push_back(std::move(Data));
>>>>        else
>>>> -        Other.push_back(std::move(Child));
>>>> +        Other.push_back(std::move(Data));
>>>>      } else if (auto VT = unique_dyn_cast<PDBSymbolTypeVTable>(Child))
>>>>        VTables.push_back(std::move(VT));
>>>>      else if (auto Func = unique_dyn_cast<PDBSymbolFunc>(Child))
>>>>        Funcs.push_back(std::move(Func));
>>>> -    else
>>>> +    else {
>>>>        Other.push_back(std::move(Child));
>>>> +    }
>>>>    }
>>>>
>>>>    // We don't want to have any re-allocations in the list of bases, so
>>>> make
>>>>
>>>> Modified:
>>>> llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp?rev=305229&r1=305228&r2=305229&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp
>>>> (original)
>>>> +++ llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp
>>>> Mon Jun 12 15:46:35 2017
>>>> @@ -151,21 +151,21 @@ bool PrettyClassLayoutGraphicalDumper::s
>>>>  }
>>>>
>>>>  void PrettyClassLayoutGraphicalDumper::dump(const PDBSymbolData
>>>> &Symbol) {
>>>> -  assert(CurrentItem != nullptr);
>>>> -
>>>> -  DataMemberLayoutItem &Layout =
>>>> -      static_cast<DataMemberLayoutItem &>(*CurrentItem);
>>>> -
>>>>    VariableDumper VarDumper(Printer);
>>>>    VarDumper.start(Symbol, ClassOffsetZero);
>>>>
>>>> -  if (Layout.hasUDTLayout() && shouldRecurse()) {
>>>> -    uint32_t ChildOffsetZero = ClassOffsetZero +
>>>> Layout.getOffsetInParent();
>>>> -    Printer.Indent();
>>>> -    PrettyClassLayoutGraphicalDumper TypeDumper(Printer,
>>>> RecursionLevel + 1,
>>>> -                                                ChildOffsetZero);
>>>> -    TypeDumper.start(Layout.getUDTLayout());
>>>> -    Printer.Unindent();
>>>> +  if (CurrentItem != nullptr) {
>>>> +    DataMemberLayoutItem &Layout =
>>>> +        static_cast<DataMemberLayoutItem &>(*CurrentItem);
>>>> +
>>>> +    if (Layout.hasUDTLayout() && shouldRecurse()) {
>>>> +      uint32_t ChildOffsetZero = ClassOffsetZero +
>>>> Layout.getOffsetInParent();
>>>> +      Printer.Indent();
>>>> +      PrettyClassLayoutGraphicalDumper TypeDumper(Printer,
>>>> RecursionLevel + 1,
>>>> +                                                  ChildOffsetZero);
>>>> +      TypeDumper.start(Layout.getUDTLayout());
>>>> +      Printer.Unindent();
>>>> +    }
>>>>    }
>>>>
>>>>    DumpedAnything = true;
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170619/fefa45ba/attachment.html>