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

[Panu Matilainen]

On 10/05/2017 12:34 PM, Thierry Vignaud wrote:
> On 5 October 2017 at 10:15, 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.

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.

	- Panu -