develooper Front page | perl.perl5.porters | Postings from February 2020

Re: Proposed new policy and approach to handling fatal exceptions inbackports: -DFATAL_BACKPORTS_(?:DIS)?ALLOWED

Thread Previous
From:
demerphq
Date:
February 10, 2020 09:09
Subject:
Re: Proposed new policy and approach to handling fatal exceptions inbackports: -DFATAL_BACKPORTS_(?:DIS)?ALLOWED
Message ID:
CANgJU+U6QF=rkuHk_cmF4LeHUdPB=+5jB_+7TywQG9EHrZYpvA@mail.gmail.com
On Mon, 10 Feb 2020 at 09:52, Steve Hay <steve.m.hay@googlemail.com> wrote:

> On Mon, 10 Feb 2020 at 06:38, demerphq <demerphq@gmail.com> wrote:
>
>> I believe that our approach to changes that introduce new fatal
>> exceptions is flawed. As far as I understand the rules we cannot introduce
>> new fatal exceptions. This means that a whole host of fixes for bugs in
>> things like the regex engine can not be backported.
>>
>> [...]
>> Where we find an issue that triggers segfaults, illegal reads, or
>> infinite loops which we solve by introducing a new exception we backport
>> the patch with a define guard like FATAL_BACKPORTS_ALLOWED. This would
>> allow people that want the protections (like $work would) can add
>> -DFATAL_BACKPORTS_ALLOWED to their build args.  We could also do this the
>> other way round, and assume people *want* such fixes but provide for a
>> define flag like -DFATAL_BACKPORTS_DISALLOWED, which would disable such
>> fixes.  I would prefer the latter, but be more than satisfied with the
>> former.
>>
>>
>>
> I think this is a good proposal in principle, especially in its more
> conservative FATAL_BACKPORTS_ALLLOWED form, but I worry about how messy
> backporting some patches might get. In the case of the two under discussion
> (ac3afc4b35 and perl5-security #125) the code changes are small and
> localized and easily wrapped in a new #define, but for some patches there
> may be various changes strewn around several files and it could get messy,
> i.e. error-prone to do, which is a worry for a maint release.
>
> We would also need some way to test for the #define from test scripts
> because most of these kinds of patches add tests to check that the fix is
> working, which obviously need to be skipped if the new code isn't compiled
> in.
>

Very good point. I hadn't thought of that part. I guess we would need to
expose the define in Config somehow.


>
> There is also then an extra overhead on making a maint release because we
> would need to test builds with and without the new #define (whichever way
> around it is done) to be sure that the manually-crafted addition of the
> #define and test skips has all been done correctly, especially since it
> won't be a simple matter of cherry-picking something from blead that is
> already proven to be good.
>

Also a good point.

Maybe we could add the flag even in blead, but make it versioned, and in
blead and versions later than the blead it was added to the versioned
define would be true for all versions.

EG, for these patches we would do something like

#if defined(FATAL_BACKPORTS_ALLOWED) || PERL_VERSION >= 5_31
#define FATAL_BACKPORTS_ALLOWED_5_30
#endif

#ifdef FATAL_BACKPORTS_ALLOWED_5_30

...code...

#endif

But with a PERL_VERSION check that actually works. ;-)

Then the patch and test code, etc would be the same in blead or later, eg,
the test code could use the same guards, etc.


>
> My feeling is that this is something that needs further thought and
> discussion before we decide whether to do it. I'm wary of jumping straight
> into this in the next few weeks for 5.30.2.
>

Totally understand. I just want to get some discussion going here so we
have a better and more flexible strategy for the future and reduce conflict
in the community over contradictory but legitimate points of view.

cheers,
Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Thread Previous


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About