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

[David Blaikie via llvm-commits]

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/f905c49c/attachment.html>