<div dir="ltr">Hi Dylan,<div><br></div><div>That fix seems to work so I've committed it as r283387. Let me know if that fixes your issue.</div><div><br></div><div>Cheers,</div><div>Lang.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 4, 2016 at 4:27 PM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Hi Dylan,</div><div><br></div><div>Ahh, I see the problem. Apparently we changed the invariant to "Archive children (even sentinels) must have parents", and we now distinguish sentinels by their null data members. We never saw a segfault from that change because we never default construct a child_iterator and all all other calls to the Child constructor (including the ones you mentioned above) actually pass in a parent.</div><div><br></div><div>I think the solution is to make the Child constructor that you referenced check the Parent value and only compute the Size argument value for the ArchiveMemberHeader if Parent is non-null. I'm testing that fix now.</div><div><br></div><div>Cheers,</div><div>Lang.</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 4, 2016 at 3:30 PM, Dylan McKay <span dir="ltr"><<a href="mailto:dylanmckay34@gmail.com" target="_blank">dylanmckay34@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="m_-2719351549944184555m_-6097388697050377896markdown-here-wrapper"><p style="margin:0px 0px 1.2em!important">Here’s one of the places we create a Archive::Child with all nulls</p>
<p style="margin:0px 0px 1.2em!important"><a href="https://github.com/llvm-mirror/llvm/blob/master/lib/Object/Archive.cpp#L771" target="_blank">Archive.cpp:771</a></p>
<p style="margin:0px 0px 1.2em!important">Archive::child_iterator Archive::child_end() const {<br> return child_iterator(Child(this, nullptr, nullptr), nullptr);<br>}</p>
<p style="margin:0px 0px 1.2em!important">If you look at the constructor in question</p>
<p style="margin:0px 0px 1.2em!important"><a href="https://github.com/llvm-mirror/llvm/blob/master/lib/Object/Archive.cpp#L308" target="_blank">Archive.cpp:308</a>.</p>
<p style="margin:0px 0px 1.2em!important">Archive::Child::Child(const Archive <em>Parent, const char </em>Start, Error *Err)<br> : Parent(Parent), Header(Parent, Start, Parent->getData().size() -<br> (Start - Parent->getData().data()), Err) {</p>
<p style="margin:0px 0px 1.2em!important"> /// …<br>}</p>
<p style="margin:0px 0px 1.2em!important">We are always dereferencing the given parent, even when it is null.</p>
<div title="MDH:SGVyZSdzIG9uZSBvZiB0aGUgcGxhY2VzIHdlIGNyZWF0ZSBhIEFyY2hpdmU6OkNoaWxkIHdpdGgg
YWxsIG51bGxzPGRpdj48YnI+PC9kaXY+PGRpdj5bQXJjaGl2ZS5jcHA6NzcxXShodHRwczovL2dp
dGh1Yi5jb20vbGx2bS1taXJyb3IvbGx2bS9ibG9iL21hc3Rlci9saWIvT2JqZWN0L0FyY2hpdmUu
Y3BwI0w3NzEpPGJyPjwvZGl2PjxkaXY+PGJyPjwvZGl2PjxkaXY+PGRpdj5BcmNoaXZlOjpjaGls
ZF9pdGVyYXRvciBBcmNoaXZlOjpjaGlsZF9lbmQoKSBjb25zdCB7PC9kaXY+PGRpdj4mbmJzcDsg
cmV0dXJuIGNoaWxkX2l0ZXJhdG9yKENoaWxkKHRoaXMsIG51bGxwdHIsIG51bGxwdHIpLCBudWxs
cHRyKTs8L2Rpdj48ZGl2Pn08L2Rpdj48L2Rpdj48ZGl2Pjxicj48L2Rpdj48ZGl2PklmIHlvdSBs
b29rIGF0IHRoZSBjb25zdHJ1Y3RvciBpbiBxdWVzdGlvbjwvZGl2PjxkaXY+PGJyPjwvZGl2Pjxk
aXY+W0FyY2hpdmUuY3BwOjMwOF0oaHR0cHM6Ly9naXRodWIuY29tL2xsdm0tbWlycm9yL2xsdm0v
YmxvYi9tYXN0ZXIvbGliL09iamVjdC9BcmNoaXZlLmNwcCNMMzA4KS48YnI+PC9kaXY+PGRpdj48
YnI+PC9kaXY+PGRpdj48ZGl2PkFyY2hpdmU6OkNoaWxkOjpDaGlsZChjb25zdCBBcmNoaXZlICpQ
YXJlbnQsIGNvbnN0IGNoYXIgKlN0YXJ0LCBFcnJvciAqRXJyKTwvZGl2PjxkaXY+Jm5ic3A7ICZu
YnNwOyA6IFBhcmVudChQYXJlbnQpLCBIZWFkZXIoUGFyZW50LCBTdGFydCwgUGFyZW50LSZndDtn
ZXREYXRhKCkuc2l6ZSgpIC08L2Rpdj48ZGl2PiZuYnNwOyAmbmJzcDsgJm5ic3A7ICZuYnNwOyAm
bmJzcDsgJm5ic3A7ICZuYnNwOyAmbmJzcDsgJm5ic3A7ICZuYnNwOyAmbmJzcDsgJm5ic3A7ICZu
YnNwOyAmbmJzcDsgJm5ic3A7KFN0YXJ0IC0gUGFyZW50LSZndDtnZXREYXRhKCkuZGF0YSgpKSwg
RXJyKSB7PC9kaXY+PC9kaXY+PGRpdj4mbmJzcDsgLy8vIC4uLjwvZGl2PjxkaXY+fTwvZGl2Pjxk
aXY+PGJyPjwvZGl2PjxkaXY+V2UgYXJlIGFsd2F5cyBkZXJlZmVyZW5jaW5nIHRoZSBnaXZlbiBw
YXJlbnQsIGV2ZW4gd2hlbiBpdCBpcyBudWxsLjwvZGl2Pg==" style="height:0;width:0;max-height:0;max-width:0;overflow:hidden;font-size:0em;padding:0;margin:0"></div></div></div><div class="m_-2719351549944184555HOEnZb"><div class="m_-2719351549944184555h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 5, 2016 at 8:36 AM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Dylan,<div><br></div><div>Where exactly are you seeing the segfault? Are you passing a non-null Data field? Archive::Child values can be sentinels (all null fields) or concrete children (both data and a parent), but you can't create an Archive::Child with a data pointer but no parent.</div><div><br></div><div>- Lang.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-2719351549944184555m_-6097388697050377896h5">On Tue, Oct 4, 2016 at 11:01 AM, Kevin Enderby via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="m_-2719351549944184555m_-6097388697050377896h5"><div style="word-wrap:break-word">Hi Dylan,<div><br></div><div>I worked with Lang Hames on this back in July when this change was made. I don’t recall the exact details off the top of my head, but one key points of the thinking was you could not use this without handling the Error. I’ll seem to recall the only case one of the default constructors was allowed was for a sentinel value.</div><div><br></div><div>I’ll try to grab Lang today and try to look at this and get back to you. Sorry the details are not fresh on my mind as we did make these changes over 2 months ago.</div><div><br></div><div>Kev</div><div><div class="m_-2719351549944184555m_-6097388697050377896m_5542592793295111450h5"><div><br><div><blockquote type="cite"><div>On Oct 3, 2016, at 6:48 PM, Dylan McKay <<a href="mailto:dylanmckay34@gmail.com" target="_blank">dylanmckay34@gmail.com</a>> wrote:</div><br class="m_-2719351549944184555m_-6097388697050377896m_5542592793295111450m_-4939382976372582487Apple-interchange-newline"><div><div dir="ltr"><div class="m_-2719351549944184555m_-6097388697050377896m_5542592793295111450m_-4939382976372582487markdown-here-wrapper"><p style="margin:0px 0px 1.2em!important">r276686 introduced a regression into the object archive writer.</p><p style="margin:0px 0px 1.2em!important"><a href="http://276686/" target="_blank">r276686</a> on Phabricator</p><p style="margin:0px 0px 1.2em!important">It causes the constructor <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">Archive::Child(Parent, Data, StartOfFile)</code> to segfault if it is given a parent equalling NULL.</p><p style="margin:0px 0px 1.2em!important">LLVM does this in a few places. The most prominent example is the <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">Archive::child_iterator</code> default constructor. This will never execute without causing a segfault.</p><p style="margin:0px 0px 1.2em!important">I’m in the process of updating Rust to LLVM 4.0. We are currently using <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">Archive::child_iterator</code>, and it causes us to segfault when writing object files.</p>
<div title="MDH:cjI3NjY4NiBpbnRyb2R1Y2VkIGEgcmVncmVzc2lvbiBpbnRvIHRoZSBvYmplY3QgYXJjaGl2ZSB3
cml0ZXIuPGRpdj48YnI+PC9kaXY+PGRpdj5bcjI3NjY4Nl0oMjc2Njg2KSZuYnNwO29uIFBoYWJy
aWNhdG9yPGJyPjxkaXY+PGJyPjwvZGl2PjxkaXY+SXQgY2F1c2VzIHRoZSBjb25zdHJ1Y3RvciBg
QXJjaGl2ZTo6Q2hpbGQoUGFyZW50LCBEYXRhLCBTdGFydE9mRmlsZSlgIHRvIHNlZ2ZhdWx0IGlm
IGl0IGlzIGdpdmVuIGEgcGFyZW50IGVxdWFsbGluZyBOVUxMLjwvZGl2PjxkaXY+PGJyPjwvZGl2
PjxkaXY+TExWTSBkb2VzIHRoaXMgaW4gYSBmZXcgcGxhY2VzLiBUaGUgbW9zdCBwcm9taW5lbnQg
ZXhhbXBsZSBpcyB0aGUgYEFyY2hpdmU6OmNoaWxkX2l0ZXJhdG9yYCBkZWZhdWx0IGNvbnN0cnVj
dG9yLiBUaGlzIHdpbGwgbmV2ZXIgZXhlY3V0ZSB3aXRob3V0IGNhdXNpbmcgYSBzZWdmYXVsdC48
L2Rpdj48ZGl2Pjxicj48L2Rpdj48ZGl2PjxkaXY+SSdtIGluIHRoZSBwcm9jZXNzIG9mIHVwZGF0
aW5nIFJ1c3QgdG8gTExWTSA0LjAuIFdlIGFyZSBjdXJyZW50bHkgdXNpbmcgYEFyY2hpdmU6OmNo
aWxkX2l0ZXJhdG9yYCwgYW5kIGl0IGNhdXNlcyB1cyB0byBzZWdmYXVsdCB3aGVuIHdyaXRpbmcg
b2JqZWN0IGZpbGVzLjwvZGl2PjxkaXY+PGJyPjwvZGl2PjwvZGl2PjwvZGl2Pg==" style="height:0;width:0;max-height:0;max-width:0;overflow:hidden;font-size:0em;padding:0;margin:0"></div></div></div>
</div></blockquote></div><br></div></div></div></div><br></div></div>______________________________<wbr>_________________<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/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>