Regression in object archive writer

[Lang Hames via llvm-commits]

Hi Dylan,

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.

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.

Cheers,
Lang.

On Tue, Oct 4, 2016 at 3:30 PM, Dylan McKay <dylanmckay34 at gmail.com> wrote:

> Here’s one of the places we create a Archive::Child with all nulls
>
> Archive.cpp:771
> <https://github.com/llvm-mirror/llvm/blob/master/lib/Object/Archive.cpp#L771>
>
> Archive::child_iterator Archive::child_end() const {
> return child_iterator(Child(this, nullptr, nullptr), nullptr);
> }
>
> If you look at the constructor in question
>
> Archive.cpp:308
> <https://github.com/llvm-mirror/llvm/blob/master/lib/Object/Archive.cpp#L308>
> .
>
> Archive::Child::Child(const Archive *Parent, const char *Start, Error
> *Err)
> : Parent(Parent), Header(Parent, Start, Parent->getData().size() -
> (Start - Parent->getData().data()), Err) {
>
> /// …
> }
>
> We are always dereferencing the given parent, even when it is null.
> ​
>
> On Wed, Oct 5, 2016 at 8:36 AM, Lang Hames <lhames at gmail.com> wrote:
>
>> Hi Dylan,
>>
>> 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.
>>
>> - Lang.
>>
>>
>> On Tue, Oct 4, 2016 at 11:01 AM, Kevin Enderby via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Hi Dylan,
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> Kev
>>>
>>> On Oct 3, 2016, at 6:48 PM, Dylan McKay <dylanmckay34 at gmail.com> wrote:
>>>
>>> r276686 introduced a regression into the object archive writer.
>>>
>>> r276686 <http://276686/> on Phabricator
>>>
>>> It causes the constructor Archive::Child(Parent, Data, StartOfFile) to
>>> segfault if it is given a parent equalling NULL.
>>>
>>> LLVM does this in a few places. The most prominent example is the
>>> Archive::child_iterator default constructor. This will never execute
>>> without causing a segfault.
>>>
>>> I’m in the process of updating Rust to LLVM 4.0. We are currently using
>>> Archive::child_iterator, and it causes us to segfault when writing
>>> object files.
>>> ​
>>>
>>>
>>>
>>> _______________________________________________
>>> 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/20161004/7ca54ae3/attachment-0001.html>