<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 19, 2017 at 9:53 AM Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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?<br></blockquote><div><br>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?<br><br>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. :)<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div dir="ltr">On Mon, Jun 19, 2017 at 9:28 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Test coverage?</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 12, 2017 at 1:47 PM Zachary Turner via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: zturner<br>
Date: Mon Jun 12 15:46:35 2017<br>
New Revision: 305229<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=305229&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=305229&view=rev</a><br>
Log:<br>
Fix a null pointer dereference in llvm-pdbutil pretty.<br>
<br>
Static data members were causing a problem because I mistakenly<br>
assumed all members would affect a class's layout and so the<br>
Layout member would be non-null.<br>
<br>
Modified:<br>
llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp<br>
llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp<br>
<br>
Modified: llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp?rev=305229&r1=305228&r2=305229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp?rev=305229&r1=305228&r2=305229&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp (original)<br>
+++ llvm/trunk/lib/DebugInfo/PDB/UDTLayout.cpp Mon Jun 12 15:46:35 2017<br>
@@ -181,13 +181,14 @@ void UDTLayoutBase::initializeChildren(c<br>
if (Data->getDataKind() == PDB_DataKind::Member)<br>
Members.push_back(std::move(Data));<br>
else<br>
- Other.push_back(std::move(Child));<br>
+ Other.push_back(std::move(Data));<br>
} else if (auto VT = unique_dyn_cast<PDBSymbolTypeVTable>(Child))<br>
VTables.push_back(std::move(VT));<br>
else if (auto Func = unique_dyn_cast<PDBSymbolFunc>(Child))<br>
Funcs.push_back(std::move(Func));<br>
- else<br>
+ else {<br>
Other.push_back(std::move(Child));<br>
+ }<br>
}<br>
<br>
// We don't want to have any re-allocations in the list of bases, so make<br>
<br>
Modified: llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp?rev=305229&r1=305228&r2=305229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp?rev=305229&r1=305228&r2=305229&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp (original)<br>
+++ llvm/trunk/tools/llvm-pdbutil/PrettyClassLayoutGraphicalDumper.cpp Mon Jun 12 15:46:35 2017<br>
@@ -151,21 +151,21 @@ bool PrettyClassLayoutGraphicalDumper::s<br>
}<br>
<br>
void PrettyClassLayoutGraphicalDumper::dump(const PDBSymbolData &Symbol) {<br>
- assert(CurrentItem != nullptr);<br>
-<br>
- DataMemberLayoutItem &Layout =<br>
- static_cast<DataMemberLayoutItem &>(*CurrentItem);<br>
-<br>
VariableDumper VarDumper(Printer);<br>
VarDumper.start(Symbol, ClassOffsetZero);<br>
<br>
- if (Layout.hasUDTLayout() && shouldRecurse()) {<br>
- uint32_t ChildOffsetZero = ClassOffsetZero + Layout.getOffsetInParent();<br>
- Printer.Indent();<br>
- PrettyClassLayoutGraphicalDumper TypeDumper(Printer, RecursionLevel + 1,<br>
- ChildOffsetZero);<br>
- TypeDumper.start(Layout.getUDTLayout());<br>
- Printer.Unindent();<br>
+ if (CurrentItem != nullptr) {<br>
+ DataMemberLayoutItem &Layout =<br>
+ static_cast<DataMemberLayoutItem &>(*CurrentItem);<br>
+<br>
+ if (Layout.hasUDTLayout() && shouldRecurse()) {<br>
+ uint32_t ChildOffsetZero = ClassOffsetZero + Layout.getOffsetInParent();<br>
+ Printer.Indent();<br>
+ PrettyClassLayoutGraphicalDumper TypeDumper(Printer, RecursionLevel + 1,<br>
+ ChildOffsetZero);<br>
+ TypeDumper.start(Layout.getUDTLayout());<br>
+ Printer.Unindent();<br>
+ }<br>
}<br>
<br>
DumpedAnything = true;<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div></div>