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

[Panu Matilainen]

On 10/05/2017 01:00 PM, Thierry Vignaud wrote:
> On 5 October 2017 at 11:40, 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 -