Regression in object archive writer

[Dylan McKay via llvm-commits]

That fixes it

Cheers,
Dylan

On Thu, Oct 6, 2016 at 10:30 AM, Lang Hames <lhames at gmail.com> wrote:

> 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/20161006/759838c4/attachment.html>