[Rpm-maint] rpm4.14 makes perl-RPM4 testsuite to segfault

[Thierry Vignaud]

On 5 October 2017 at 12:03, Panu Matilainen <pmatilai at redhat.com> wrote:
>>>>>>>> Yeah, I'm getting segfaults all the way to rpm 4.11.x, didn't test
>>>>>>>> earlier because this already shows it's not a regression in 4.14.x
>>>>>>>> but
>>>>>>>> something else. A bug in perl-RPM4 perhaps, as compiling it with -Og
>>>>>>>> makes
>>>>>>>> the crash go away, other optimization levels make it blow up with
>>>>>>>> different
>>>>>>>> levels of spectacular. I dont see anything obvious in there but that
>>>>>>>> doesn't
>>>>>>>> mean much, I know diddly about perl and its extensions.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I ran it with some added debugging on rpm side which I'm more
>>>>>>> familiar
>>>>>>> with, and the crash happens because a totally garbage pointer is
>>>>>>> passed
>>>>>>> to
>>>>>>> headerFree(). Well indeed, it was passing the address of the header
>>>>>>> pointer
>>>>>>> variable as the header itself into the callback, and when you try do
>>>>>>> stuff
>>>>>>> with that, well...
>>>>>>>
>>>>>>> This fixes it:
>>>>>>>
>>>>>>> diff --git a/src/RPM4.xs b/src/RPM4.xs
>>>>>>> index 04c65ee..6604477 100644
>>>>>>> --- a/src/RPM4.xs
>>>>>>> +++ b/src/RPM4.xs
>>>>>>> @@ -246,7 +246,7 @@ static void *
>>>>>>>                 s_what = "INST_START";
>>>>>>>                 if (h) {
>>>>>>>                     mXPUSHs(newSVpv("header", 0));
>>>>>>> -                mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
>>>>>>> &h));
>>>>>>> +                mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
>>>>>>> h));
>>>>>>>     #ifdef HDRPMMEM
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Oh and you'll want to fix the debug printf too, even though it's
>>>>>> obviously
>>>>>> harmless (but then useless for debugging):
>>>>>>
>>>>>>>                     PRINTF_NEW(bless_header, &h, -1);
>>>>>>
>>>>>>
>>>>>>
>>>>>>                                                 ^^
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Blech, one of those days...
>>>>>
>>>>> The above is closer to mark but still not quite right and will crash
>>>>> too,
>>>>> only in a different way because the refcount is wrong. Here's the real
>>>>> deal:
>>>>>
>>>>> diff --git a/src/RPM4.xs b/src/RPM4.xs
>>>>> index 04c65ee..f7cfd33 100644
>>>>> --- a/src/RPM4.xs
>>>>> +++ b/src/RPM4.xs
>>>>> @@ -246,9 +246,9 @@ static void *
>>>>>                s_what = "INST_START";
>>>>>                if (h) {
>>>>>                    mXPUSHs(newSVpv("header", 0));
>>>>> -                mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header, &h));
>>>>> +                mXPUSHs(sv_setref_pv(newSVpvs(""), bless_header,
>>>>> headerLink(h)));
>>>>>    #ifdef HDRPMMEM
>>>>> -                PRINTF_NEW(bless_header, &h, -1);
>>>>> +                PRINTF_NEW(bless_header, h, -1);
>>>>>    #endif
>>>>>                }
>>>>>            break;
>>>>>
>>>>>           - Panu -
>>>>>
>>>>
>>>> Thanks
>>>> Now remains the issue with several builds on the same spec object
>>>>
>>>
>>> Yeah, I was just about to write about that.
>>>
>>> I totally misunderstood the case initially (that's what happens if you
>>> try
>>> to understand the world by peeking through a keyhole), I thought your
>>> testcase was looking at the number of files by itself and finding one
>>> more
>>> than expected. When the error actually comes from rpm itself.
>>
>>
>> Yeah we had a misunderstanding there :-)
>>
>>> That file entries get messed when reusing the same spec is probably an
>>> ages
>>> old issue, it's just that until now rpm didn't sanity check the file
>>> data.
>>>
>>> The right thing to do is to reload the spec, I doubt it ever *really*
>>> worked
>>> correctly.
>>
>>
>> OK I'll commit my fix in the VC then.
>> There's still one last testsuite failure with RPM4 when verifying a
>> rpmdb after rebuilding it :
>>
>> $ LC_ALL=C perl -Iblib/lib -Iblib/arch t/05transaction.t 2>&1|grep -A2
>> -B36 "not ok"
>> error: rpmdb: Enhancename: No such file or directory
>> error: db5 error(2) from db->verify: No such file or directory
>> (...)
>> error: rpmdb: Name: No such file or directory
>> error: db5 error(2) from db->verify: No such file or directory
>> not ok 4 - Verify empty
>> #   Failed test 'Verify empty'
>> #   at t/05transaction.t line 24.
>>
>> The initdb creates the proper files, but after rebuilddb, there's only
>> "Packages" (+ the __db.000 env)
>> This used to work.
>> Any idea?
>>
>
> Yeah, see
> https://github.com/rpm-software-management/rpm/commit/b62d85d78b07d6d43ca26fd7cda489c636a5796b
> :)
>
> (and yes that'll be in 4.14.0 too)
>
>         - Panu -

Thanks.