Regression in object archive writer

[Lang Hames via llvm-commits]

Hi Dylan,

That fix seems to work so I've committed it as r283387. Let me know if that
fixes your issue.

Cheers,
Lang.

On Tue, Oct 4, 2016 at 4:27 PM, Lang Hames <lhames at gmail.com> wrote:

> 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/20161005/0a59ac65/attachment.html>