Re: Preview: portable dumper

[Robert Pluim <rpluim@gmail.com>]

Daniel Colascione <dancol@dancol.org> writes:

> On 02/20/2018 10:09 AM, Robert Pluim wrote:
>> Daniel Colascione <dancol@dancol.org> writes:
>>
>>> On 02/20/2018 09:28 AM, Paul Eggert wrote:
>>>> On 02/20/2018 09:19 AM, Daniel Colascione wrote:
>>>>> Do you get an assertion failure if you enable assertion checking?
>>>>> If we're seeing this kind of problem from the compiler expectation
>>>>> alone, I'll definitely revert that change.
>>>>
>>>> In my experience, eassume (and 'assume') shouldn't be used for
>>>> complicated expressions, as GCC is too-easily confused by
>>>> them. eassume should be used only for expressions where the
>>>> assumption really does help the compiler, either by generating
>>>> significantly-better code or by pacifying a false alarm.
>>>
>>> I'd hoped things had gotten better, and some local testing seemed to
>>> suggest it was safe to use these days. Apparently it's not, which is a
>>> shame. It'd be nice, but not urgent, to track down which specific
>>> assumptions caused what I think must be miscompilations so we could
>>> report them upstream.
>>
>> The guilty change is:
>>
>> --- a/src/pdumper.c
>> +++ b/src/pdumper.c
>> @@ -607,7 +607,7 @@ dump_set_have_current_referrer (struct dump_context *ctx, bool have)
>>   static void
>>   DUMP_CLEAR_REFERRER (struct dump_context *ctx)
>>   {
>> -  eassert (ctx->have_current_referrer);
>> +  eassume (ctx->have_current_referrer);
>>     dump_set_have_current_referrer (ctx, false);
>>     if (dump_tracking_referrers_p (ctx))
>>       ctx->current_referrer = Qnil;
>>
>> If I revert just that hunk, the build succeeds.
>
> Wow: thanks for tracking that down.
>

splitpatch + git bisect run can do wonders :-)

> What on earth? I mean, we're instructing the compiler to assume
> something that's both trivial and true. What GCC version is this? If
> it's recent, I think it's probably worth reporting.

$ gcc --version
gcc-5.real (Ubuntu 5.4.0-6ubuntu1~16.04.6) 5.4.0 20160609

I have a gcc-8-ish somewhere that I can rev up to see if it has the
same problem, but probably not soon.

Robert