to NN or not to NN, or how to NN

[Jarkko Hietaniemi]

(Re: https://rt.perl.org/Ticket/Display.html?id=125570, please read
through it first)

Attempting summary first at technical level:

(1) At least under gcc the the __attribute__((nonnull(..)) does NOT mean
"magically make the code work if NULL passed in", even for values of
"work" meaning "insert code that assert-fails on NULL". In fact, we
explicitly insert our own ASSERTs (and by a porting test, enforce
and verify them) to do the assert-fails.

What the attribute *really* means is (quoting Aristotle): "a *promise
from the programmer to the compiler* that the function will never be
called with NULL arguments." And what this means is that any code
that tests the pointer, or goes through it, can be elided by the
optimizer because we just promised, via the attribute, that this
code makes no sense. This means that e.g. any sanity checks
testing the argument against NULL-ness will be removed.
As also pointed out by Aristotle, there is no inter-procedural
flow analysis done to see where NULLs might come in. (For a library
like libperl, that would be quite a feat.)

(2) bulk88 pointed out that the exact behavior on badly behaving
pointers (like NULL) is undefined, which is defined (haha) to mean
that the implementation (of compiler, OS, CPU) is free to to do what
they want. A fair point, though I don't know what we can do here
portably, beyond adding special code for each platform where we
know exactly what to do.

I am in the camp "the attribute is doing us no good", and should not
be generated, under any build configuration. It's not doing what
one would think it's doing, and again as Aristotle points out,
is no different from using #ifdef DEBUG/DEBUGGING.

I am also in the even more aggressive camp "why are we using the ASSERTs
only in DEBUGGING builds?" I mean, is accidentally derefing a NULL
somehow more acceptable in production builds? This I expect to be
of too extreme fringe for most.

[bulk88]

Post by Jarkko Hietaniemi
(2) bulk88 pointed out that the exact behavior on badly behaving
pointers (like NULL) is undefined, which is defined (haha) to mean
that the implementation (of compiler, OS, CPU) is free to to do what
they want. A fair point, though I don't know what we can do here
portably, beyond adding special code for each platform where we
know exactly what to do.

Perl should has tests to prove that derefing NULL stops execution and
shows the user that there was a failure. A couple other C fatal runtime
errors should also be tested. See attached rough patch. HPUX perl uses a
CC flag to make derefing NULL SEGV instead of evaluate to 0, since the
"successful evaluate to 0" is insane in traditional C world. one
complement integers, and 9 and 7 bit chars would be other harmful CC
architectures.

Post by Jarkko Hietaniemi
I am in the camp "the attribute is doing us no good", and should not
be generated, under any build configuration. It's not doing what
one would think it's doing, and again as Aristotle points out,
is no different from using #ifdef DEBUG/DEBUGGING.

The optimization I think is very important.

destroybig(sv) {
if(sv->flags == 123) {
destroyextra(sv->body.extra);
}
SvREFCNT_dec(sv);
}

If destroybig's sv argument is marked NN, GCC will remove the NULL test
in SvREFCNT_dec when SvREFCNT_dec is inlined into destroybig(). A
computer can find cases of automatic optimization to SvREFCNT_dec_NN
better than a human hunting and pecking for it by eye (try understanding
what "tryAMAGICunTARGETlist(meth, jump)" does, it 1 or 2 screenfuls of
code, or what about cop.h's 1 screenful big macros?).

Post by Jarkko Hietaniemi
I am also in the even more aggressive camp "why are we using the ASSERTs
only in DEBUGGING builds?" I mean, is accidentally derefing a NULL
somehow more acceptable in production builds? This I expect to be
of too extreme fringe for most.

Reliable SEGVing of refering of NULL I think should be relied on. Perl
already SEGVs intentionally in 1 place
http://perl5.git.perl.org/perl.git/blob/749f0eb1485a7bb7561d71f9539d9b7655363136:/win32/vmem.h#l200


Any segv of an address < ~0x200 is easy to diagnose. In Perlese its
"Can't call method "foo" on an undefined value at -e line 1." In C its
SIGSEGV. undef/NULL means object/struct not there.

"Bizarre copy of %s in %s" is as equally vague as a SEGV that requires a
C debugger,

The only SEGVs that are very hard to debug is is SEGV on addr 0x3778C8
since that isnt "object not created", that is memory corruption or use
after free corruption.

[Aristotle Pagaltzis]

Post by bulk88
The optimization I think is very important.

What does that mean in numbers? (And let me state up front that care
about cycles a lot more than I care about bytes, even if I care about
both.)

Post by bulk88
If destroybig's sv argument is marked NN, GCC will remove the NULL
test in SvREFCNT_dec when SvREFCNT_dec is inlined into destroybig().
A computer can find cases of automatic optimization to SvREFCNT_dec_NN
better than a human hunting and pecking for it by eye (try
understanding what "tryAMAGICunTARGETlist(meth, jump)" does, it 1 or
2 screenfuls of code, or what about cop.h's 1 screenful big macros?).

That would be great if the NN attribute led every call site of a non-
inlined NN function to have an assert added, so that NN-marked functions
down through their inlined callees could elide the checks safely.

It doesn’t, which turns this into a case of

#11963 It's easy to get the *wrong* answer in O(1) time.

Regards,

[Ed Avis]

I have sometimes used http://www.splint.org/ to add not-null annotations
to C programs. It is mentioned in perlhacktips. However, its standard
behaviour is to take all pointers as declared 'not null' unless they are
annotated with /*@null@*/. Sprinkling that through the codebase might be
a step too far.

[Nicholas Clark]

Post by Jarkko Hietaniemi
(Re: https://rt.perl.org/Ticket/Display.html?id=125570, please read
through it first)
(1) At least under gcc the the __attribute__((nonnull(..)) does NOT mean
"magically make the code work if NULL passed in", even for values of
"work" meaning "insert code that assert-fails on NULL". In fact, we
explicitly insert our own ASSERTs (and by a porting test, enforce
and verify them) to do the assert-fails.
What the attribute *really* means is (quoting Aristotle): "a *promise
from the programmer to the compiler* that the function will never be
called with NULL arguments." And what this means is that any code
that tests the pointer, or goes through it, can be elided by the
optimizer because we just promised, via the attribute, that this
code makes no sense. This means that e.g. any sanity checks
testing the argument against NULL-ness will be removed.
As also pointed out by Aristotle, there is no inter-procedural
flow analysis done to see where NULLs might come in. (For a library
like libperl, that would be quite a feat.)

In theory this thing should be useful. It ought to help in situations like
ours where there's macro rich code which contains a bunch of NULL checks, but
it's used in functions where the call path ensures that that parameter isn't
NULL.

But, in practice as I think I'd said privately to various people over the
past 5 years, but I don't think I'd mailed the list, I see it as a design
flaw that *only* function parameters can get this designation. It ought to
be be possible to also annotate variables and structure members as not-NULL,
and so have your not-NULL-ness be analysed, and faulted.

What gcc offers right now is rather like having function parameters be allowed
to be declared const, with various optimisations then made on that basis,
but

1) variables and structure elements not being permitted to be declared const
2) *and* constness silently being cast away by the compiler

and that's self-evidently awesome and all the best compilers do that.

Post by Jarkko Hietaniemi
(2) bulk88 pointed out that the exact behavior on badly behaving
pointers (like NULL) is undefined, which is defined (haha) to mean
that the implementation (of compiler, OS, CPU) is free to to do what
they want. A fair point, though I don't know what we can do here
portably, beyond adding special code for each platform where we
know exactly what to do.
I am in the camp "the attribute is doing us no good", and should not
be generated, under any build configuration. It's not doing what
one would think it's doing, and again as Aristotle points out,
is no different from using #ifdef DEBUG/DEBUGGING.

I'm coming to that view.

The intent of the asserts was on the

1) assumption that the code generation would be useful
2) the above observation that it's only 1/3rd finished in gcc
3) that we have good enough test coverage to find the bad annotations

Experience is suggesting that assumption 3 *is* good enough, at least for the
core, because we've not hit a failed assert in years.

But as others (Aristotle?) observed that's not something we can guarantee
(or "guarantee" or empirically "prove" correct) for library code.

Which would mean that we ought not have Not Null declarations on any API
function.


I did a test on dromedary.

Two build trees, same config.sh, confirmed that all object files were byte
for byte identical (apart from perl.o, __DATE__)


Did this, and re-ran embed.pl, to get rid of all __attribute__nonnull__:

diff --git a/regen/embed.pl b/regen/embed.pl
index 07438de..d882c0d 100755
--- a/regen/embed.pl
+++ b/regen/embed.pl
@@ -206,7 +206,7 @@ my ($embed, $core, $ext, $api) = setup_embed();
$prefix, $pat, $args;
}
}
- if ( @nonnull ) {
+ if ( 0 && @nonnull ) {
my @pos = map { $has_context ? "pTHX_$_" : $_ } @nonnull;
push @attrs, map { sprintf( "__attribute__nonnull__(%s)", $_ ) } @pos;
}


Total object file size goes from 3607011 to 3608299
So it *is* making a difference.

What I can't tell (obviously) is whether it's a difference that matters.
Is this in hot code?

There isn't anything I'm comfortable to use as a benchmark.
What is Dave using to figure out (and measure) potential optimisations for
booking.com?


Hence I'm wondering, what the benefit of this is.

We have accurate information currently about which function parameters are
never NULL. In machine readable form. With tokens at all the right places
in the C code.

So it would be possible to programmatically convert all the lines such as

PERL_ARGS_ASSERT_DO_SPAWN_NOWAIT;

into

if (!cmd)
Perl_croak(aTHX_ "Perl_do_spawn_nowait called with NULL cmd");


(we know the function name, whether the function has aTHX_ available, and
the command name, and where in the C source code to s/.../.../)


at which point, I believe, we've given the C compiler's optmimiser the same
information as the current not_null attribute does, which means that if its
optimiser is competent, it can propagate that through and do exactly the same
dead code elimination.

But not just for gcc :-)

Post by Jarkko Hietaniemi
I am also in the even more aggressive camp "why are we using the ASSERTs
only in DEBUGGING builds?" I mean, is accidentally derefing a NULL
somehow more acceptable in production builds? This I expect to be
of too extreme fringe for most.

I don't know. I'd not thought about that.
IIRC I was the one who changed the perl.h assert() macro from something
that called croak() [yes, pretty] into something that would actually DIE DIE
DIE. And not get trapped by eval (wah!)

[and tomorrow I'm off on holiday with no Internet coverage for the next 11
days, so I'm not going to be in a position to reply to any replies any time
soon]

Nicholas Clark

[bulk88]

Post by Nicholas Clark

Post by Jarkko Hietaniemi
I am in the camp "the attribute is doing us no good", and should not
be generated, under any build configuration. It's not doing what
one would think it's doing, and again as Aristotle points out,
is no different from using #ifdef DEBUG/DEBUGGING.

I'm coming to that view.
The intent of the asserts was on the
1) assumption that the code generation would be useful
2) the above observation that it's only 1/3rd finished in gcc
3) that we have good enough test coverage to find the bad annotations

Bad annotations crash, every perl platform should be crashing from
derefing NULL. Failing a NULL assert vs SEGV doesn't save me any time
since I still need to get a stack trace to do anything about the SEGV. I
assume everyone in the room knows how to use a stepping debugger and how
to obtain a stack trace in C. The NULL asserts are slightly helpful when
a bug reporter who thinks C is the 3rd letter of the alphabet reports a
SEGV, but I think even then, without a repro by P5P, it wont get fixed.
If P5P can repro the SEGV, they can get a C debugger stack trace, which
goes back to the experience still requiring a C debugger to fix the
bug/SEGV.

Post by Nicholas Clark
Experience is suggesting that assumption 3 *is* good enough, at least for the
core, because we've not hit a failed assert in years.
But as others (Aristotle?) observed that's not something we can guarantee
(or "guarantee" or empirically "prove" correct) for library code.

Perl 5 isn't academically designed on principles of formal methods.
Perl's extensive use of unions, casting, and function pointers means it
becomes hard to trace anything more than 1 or 2 layers statically.

Post by Nicholas Clark
Which would mean that we ought not have Not Null declarations on any API
function.
I did a test on dromedary.
Two build trees, same config.sh, confirmed that all object files were byte
for byte identical (apart from perl.o, __DATE__)
diff --git a/regen/embed.pl b/regen/embed.pl
index 07438de..d882c0d 100755
--- a/regen/embed.pl
+++ b/regen/embed.pl
@@ -206,7 +206,7 @@ my ($embed, $core, $ext, $api) = setup_embed();
$prefix, $pat, $args;
}
}
}
Total object file size goes from 3607011 to 3608299
So it *is* making a difference.

Using "gcc version 4.9.2 (i686-posix-sjlj, built by strawberryperl.com
project)" and JHI's un-nn branch,
http://perl5.git.perl.org/perl.git/commit/f904452c6f4bf7f687ef25de67acfaf0c79e84e8


unnn perl523.dll .text section size 14BF08, perl523.dll, file size 1698KB

NN (1 commit before JHI's branch at
http://perl5.git.perl.org/perl.git/commit/749f0eb1485a7bb7561d71f9539d9b7655363136
) perl523.dll .text section size 14E5F8, perl523.dll file size 1708KB

Post by Nicholas Clark
What I can't tell (obviously) is whether it's a difference that matters.
Is this in hot code?

See attached files, 1 is raw data, other is some analysis by eye of why
the sizes are different. Summary is NN successfully removes branches in
a number of places, but it is poorly tested by GCC devs, and it blocks
random calling convention for static functions optimization, atleast on
x86-32. Blocking random calling convention for static optimization costs
alot more machine code than is saved by branch remove, but this is a
GCC bug remember. Maybe in older or newer GCCs they fixed the problem
that random calling conventions dont get applied to static funcs
declared with NN. Maybe clang dont suffer such bugs like GCC and for it
NN will win in machine code size and file size. Testing clang is more
difficult for me so I am not going to do it unless someone is interested.

The raw data doesn't include any static functions, so any wins or losses
in static functions aren't mentioned in unndiff.diff file.

Post by Nicholas Clark
There isn't anything I'm comfortable to use as a benchmark.
What is Dave using to figure out (and measure) potential optimisations for
booking.com?
Hence I'm wondering, what the benefit of this is.
We have accurate information currently about which function parameters are
never NULL. In machine readable form. With tokens at all the right places
in the C code.
So it would be possible to programmatically convert all the lines such as
PERL_ARGS_ASSERT_DO_SPAWN_NOWAIT;
into
if (!cmd)
Perl_croak(aTHX_ "Perl_do_spawn_nowait called with NULL cmd");
(we know the function name, whether the function has aTHX_ available, and
the command name, and where in the C source code to s/.../.../)
at which point, I believe, we've given the C compiler's optmimiser the same
information as the current not_null attribute does, which means that if its
optimiser is competent, it can propagate that through and do exactly the same
dead code elimination.
But not just for gcc :-)

Why would

Post by Nicholas Clark
if (!cmd)
Perl_croak(aTHX_ "Perl_do_spawn_nowait called with NULL cmd");

ever optimize away without NN attribute on var cmd?

One experiment is changing PERL_ARGS_ASSERT_DO_SPAWN_NOWAIT to call
ASSUME(). IDK how effective ASSUME is with GCC and clang vs
__attribute__((nonnull(a))). If you think about it,
__attribute__((nonnull(a))) can be implemented by a CC as a wrapper
around ASSUME, but there is no guarentee it is implemented that way by a
CC. VC doesn't know nonnull but does know about ASSUME. Any supposed
problems with nonnull optimizing away asserts will happen with ASSUME,
so no SEGVs are solved. Perl's ASSUME macro calls assert macro if
DEBUGGING or a CC doesn't know what assume() is. What libc assert macro
expands to is not perl ASSUME's problem but other perl .c/.sh/.h/.pm
code's problem.

I would be hesitant to make all assert()s be ASSUME()s without verifying
the effect of doing that. Since passing huge statements might confuse
the CC optimizer more than it helps.

[Aristotle Pagaltzis]

The NULL asserts are slightly helpful when a bug reporter who thinks
C is the 3rd letter of the alphabet reports a SEGV, but I think even
then, without a repro by P5P, it wont get fixed. If P5P can repro the
SEGV, they can get a C debugger stack trace, which goes back to the
experience still requiring a C debugger to fix the bug/SEGV.

Asserting on NULL helps prevent sideways propagation of errors:

A NULL that wanders into code which expects non-NULL may slip through
unnoticed in that instance due to some constellation of conditionals
that lead to it not being dereferenced that time.

Then somewhere up the call chain this NULL (which was already a bug but
has been hidden by circumstances) gets passed down some other call chain
and it ends up dereferenced there. Sure, you’ll get a crash and then a
backtrace from *that*, but that doesn’t point out the real problem, so
now you have to debug your way back to where the NULL really happened.

It would be much more helpful if that NULL had blown up as soon as it
made its way to *any* NULL-sensitive code, regardless of circumstances.

Why would

Post by Nicholas Clark
if (!cmd)
Perl_croak(aTHX_ "Perl_do_spawn_nowait called with NULL cmd");

ever optimize away without NN attribute on var cmd?

Well you would have to use an assert(cmd) there. But after that, the
compiler is in a position to know that the value of cmd in code that
follows the assert must always be valid, so it can then reorder code,
remove branches, etc, based on that knowledge.

Compilers already do such analysis and already do such optimisations.

Whether any of them does this particular one (or might in the future)
is the question.

One experiment is changing PERL_ARGS_ASSERT_DO_SPAWN_NOWAIT to call
ASSUME().

Yes. For compilers who need this assistance, that would make sense to
place after the assert.

Regards,

[Tony Cook]

Post by bulk88

Post by Nicholas Clark

Post by Jarkko Hietaniemi
I am in the camp "the attribute is doing us no good", and should not
be generated, under any build configuration. It's not doing what
one would think it's doing, and again as Aristotle points out,
is no different from using #ifdef DEBUG/DEBUGGING.

I'm coming to that view.
The intent of the asserts was on the
1) assumption that the code generation would be useful
2) the above observation that it's only 1/3rd finished in gcc
3) that we have good enough test coverage to find the bad annotations

Bad annotations crash, every perl platform should be crashing from
derefing NULL. Failing a NULL assert vs SEGV doesn't save me any
time since I still need to get a stack trace to do anything about
the SEGV. I assume everyone in the room knows how to use a stepping
debugger and how to obtain a stack trace in C. The NULL asserts are
slightly helpful when a bug reporter who thinks C is the 3rd letter
of the alphabet reports a SEGV, but I think even then, without a
repro by P5P, it wont get fixed. If P5P can repro the SEGV, they can
get a C debugger stack trace, which goes back to the experience
still requiring a C debugger to fix the bug/SEGV.

On POSIX(ish) systems a failed assert() generates a signal, which
produces a core dump (if they're enabled) and caught by the debugger.

On Win32 I suspect you'd have to breakpoint abort() to get a useful
backtrace from an assertion failure.

Tony

[Craig A. Berry]

Post by Tony Cook

Post by bulk88

Post by Nicholas Clark

Post by Jarkko Hietaniemi
I am in the camp "the attribute is doing us no good", and should not
be generated, under any build configuration. It's not doing what
one would think it's doing, and again as Aristotle points out,
is no different from using #ifdef DEBUG/DEBUGGING.

I'm coming to that view.
The intent of the asserts was on the
1) assumption that the code generation would be useful
2) the above observation that it's only 1/3rd finished in gcc
3) that we have good enough test coverage to find the bad annotations

Bad annotations crash, every perl platform should be crashing from
derefing NULL. Failing a NULL assert vs SEGV doesn't save me any
time since I still need to get a stack trace to do anything about
the SEGV. I assume everyone in the room knows how to use a stepping
debugger and how to obtain a stack trace in C. The NULL asserts are
slightly helpful when a bug reporter who thinks C is the 3rd letter
of the alphabet reports a SEGV, but I think even then, without a
repro by P5P, it wont get fixed. If P5P can repro the SEGV, they can
get a C debugger stack trace, which goes back to the experience
still requiring a C debugger to fix the bug/SEGV.

On POSIX(ish) systems a failed assert() generates a signal, which
produces a core dump (if they're enabled) and caught by the debugger.
On Win32 I suspect you'd have to breakpoint abort() to get a useful
backtrace from an assertion failure.

According to POSIX, a failed assert will give you "the text of the
argument, the name of the source file, the source file line number,
and the name of the enclosing function." It's true that some of the
time you will also need to know what called the enclosing function,
but not all of the time.

[Tony Cook]

Post by Craig A. Berry

Post by Tony Cook
On POSIX(ish) systems a failed assert() generates a signal, which
produces a core dump (if they're enabled) and caught by the debugger.
On Win32 I suspect you'd have to breakpoint abort() to get a useful
backtrace from an assertion failure.

According to POSIX, a failed assert will give you "the text of the
argument, the name of the source file, the source file line number,
and the name of the enclosing function." It's true that some of the
time you will also need to know what called the enclosing function,
but not all of the time.

After an assertion failure reports that information it calls abort(),
which raises SIGABRT, which produces a core/debugger break.

From a debugging point of view both assert() and dereffing NULL (on
systems where that SIGSEGV's) are as easy as the other to produce a
backtrace from.

Unless you're on Win32, which is where I expect most of bulk88's
experience comes from.

Tony

[Reini Urban]

(Re: https://rt.perl.org/Ticket/Display.html?id=125570, please read through it first)
(1) At least under gcc the the __attribute__((nonnull(..)) does NOT mean
"magically make the code work if NULL passed in", even for values of "work" meaning "insert code that assert-fails on NULL". In fact, we
explicitly insert our own ASSERTs (and by a porting test, enforce
and verify them) to do the assert-fails.
What the attribute *really* means is (quoting Aristotle): "a *promise
from the programmer to the compiler* that the function will never be
called with NULL arguments." And what this means is that any code
that tests the pointer, or goes through it, can be elided by the
optimizer because we just promised, via the attribute, that this
code makes no sense. This means that e.g. any sanity checks
testing the argument against NULL-ness will be removed.
As also pointed out by Aristotle, there is no inter-procedural
flow analysis done to see where NULLs might come in. (For a library
like libperl, that would be quite a feat.)
(2) bulk88 pointed out that the exact behavior on badly behaving
pointers (like NULL) is undefined, which is defined (haha) to mean
that the implementation (of compiler, OS, CPU) is free to to do what
they want. A fair point, though I don't know what we can do here
portably, beyond adding special code for each platform where we
know exactly what to do.
I am in the camp "the attribute is doing us no good", and should not
be generated, under any build configuration. It's not doing what
one would think it's doing, and again as Aristotle points out,
is no different from using #ifdef DEBUG/DEBUGGING.
I am also in the even more aggressive camp "why are we using the ASSERTs
only in DEBUGGING builds?" I mean, is accidentally derefing a NULL
somehow more acceptable in production builds? This I expect to be
of too extreme fringe for most.

Yes.
Anecdote from parrot, where we used nonnull attributes heavily:

It is very dangerous to let NULL slip through, as the optimizer omits
all checks at run-time, no asserts are in place without “DEBUGGING”.
I spent a few days finding a crazy bug with NN, just to find out that NULL
was passed to a nonnull attribute, and the optimizer removed the
if (!param) {} block. Even if the check is there, the compiler will remove it.

It is a very good attribute to speed up -O2, but the dead-code elimination
should be visible somewhere. It is not.

Reini

[Jarkko Hietaniemi]

Post by Reini Urban
It is a very good attribute to speed up -O2, but the dead-code elimination
should be visible somewhere. It is not.

Right. If the "hey, you promised the foo would be non-null, so I just
went ahead and removed lines 789-796" would be somehow more prominent,
the effect would be less insidious.

As a side note: there's a related, but different, optimization for
pointer use, removing repeated checks of pointers (and where
I think it's sane and okay):

if (ptr) {
... block 1 ...
}
... block 2 ... /* Code not changing the ptr ... */
if (ptr) { /* This check here makes no sense. */
... block 3 ...
}

Unless a cosmic ray hits the CPU, the ptr is going to be the same.
(Also, risk messing up the CPU branch prediction.) Just inline the
block 3 straight after block 2.

True, for defensive/paranoid programming we could protect every
access to the pointers (and if using macros/inlines, it might
easily be happening). But when optimizing the full code flow,
one check at the top should be enough. (Reminds me of an old joke
of a marketdroid: "our new CPU is so fast, it requires three
HALTs to come to a full stop"...)

Also, looks like static analyzers like Coverity are getting smart
enough to detect and moan about such cases.

[Jarkko Hietaniemi]

My read on the discussion is that the __attribute__((nonnull(...)) could
and should indeed go. (Hey, I admit I was prebiased on this.)
Unless good argumentation tells me otherwise, in few days I'll just
go ahead and do that.

[bulk88]

Post by Jarkko Hietaniemi
My read on the discussion is that the __attribute__((nonnull(...)) could
and should indeed go. (Hey, I admit I was prebiased on this.)
Unless good argumentation tells me otherwise, in few days I'll just
go ahead and do that.

Did you read
http://www.nntp.perl.org/group/perl.perl5.porters/2015/08/msg229949.html ?

[Jarkko Hietaniemi]

Post by bulk88

Post by Jarkko Hietaniemi
My read on the discussion is that the __attribute__((nonnull(...)) could
and should indeed go. (Hey, I admit I was prebiased on this.)
Unless good argumentation tells me otherwise, in few days I'll just
go ahead and do that.

Did you read
http://www.nntp.perl.org/group/perl.perl5.porters/2015/08/msg229949.html ?

Yes. But I don't quite understand your point.

I am not suggesting to do anything about the asserts, one way or
another, at this point.

Just removing the non-null attributes.

[bulk88]

Post by Jarkko Hietaniemi
Yes. But I don't quite understand your point.
I am not suggesting to do anything about the asserts, one way or
another, at this point.
Just removing the non-null attributes.

Removing the non-null attribute increases the amount of GCC 4.9.2 -O2
branches. You are deoptimizing the code.

[Jarkko Hietaniemi]

Post by bulk88

Post by Jarkko Hietaniemi
Yes. But I don't quite understand your point.
I am not suggesting to do anything about the asserts, one way or
another, at this point.
Just removing the non-null attributes.

Removing the non-null attribute increases the amount of GCC 4.9.2 -O2
branches. You are deoptimizing the code.

Sigh. I give up.

[Peter Rabbitson]

Post by Jarkko Hietaniemi

Post by bulk88

Post by Jarkko Hietaniemi
Yes. But I don't quite understand your point.
I am not suggesting to do anything about the asserts, one way or
another, at this point.
Just removing the non-null attributes.

Removing the non-null attribute increases the amount of GCC 4.9.2 -O2
branches. You are deoptimizing the code.

Sigh. I give up.

Please don't give up. Daniel has many rather far-fetched ideas wrt code
(de-)optimizations, which (ideas) are not mainstream by any measure.

Yes, Jarkko is deoptimizing the code. He is also turning it into
something that wouldn't be a first-order target for security
researchers. In other words undoing the vulnerable mess we have with any
gcc 4.8+.

Jarkko: *PLEASE* get the patchset together. There are very few people
with the necessary expertise to execute this cleanly.

[bulk88]

Post by Peter Rabbitson
Please don't give up. Daniel has many rather far-fetched ideas wrt code
(de-)optimizations, which (ideas) are not mainstream by any measure.

So what are your ideas? I'd love to hear them.

[Ivan Pozdeev]

Post by Jarkko Hietaniemi
(Re: https://rt.perl.org/Ticket/Display.html?id=125570, please read
through it first)
(1) At least under gcc the the __attribute__((nonnull(..)) does NOT mean
"magically make the code work if NULL passed in", even for values of
"work" meaning "insert code that assert-fails on NULL". In fact, we
explicitly insert our own ASSERTs (and by a porting test, enforce
and verify them) to do the assert-fails.
What the attribute *really* means is (quoting Aristotle): "a *promise
from the programmer to the compiler* that the function will never be
called with NULL arguments." And what this means is that any code
that tests the pointer, or goes through it, can be elided by the
optimizer because we just promised, via the attribute, that this
code makes no sense.

According to
https://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Function-Attributes.html,
it means:

If the compiler determines that a null pointer is passed in an argument slot
marked as non-null, and the -Wnonnull option is enabled, a warning is issued.
The compiler may also choose to make optimizations based on the knowledge that
certain function arguments will not be null.

I.e. the primary purpose is code verification rather than optimization.

Post by Jarkko Hietaniemi
(2) bulk88 pointed out that the exact behavior on badly behaving
pointers (like NULL) is undefined, which is defined (haha) to mean
that the implementation (of compiler, OS, CPU) is free to to do what
they want. A fair point, though I don't know what we can do here
portably, beyond adding special code for each platform where we
know exactly what to do.
I am in the camp "the attribute is doing us no good", and should not
be generated, under any build configuration. It's not doing what
one would think it's doing, and again as Aristotle points out,
is no different from using #ifdef DEBUG/DEBUGGING.
I am also in the even more aggressive camp "why are we using the ASSERTs
only in DEBUGGING builds?" I mean, is accidentally derefing a NULL
somehow more acceptable in production builds? This I expect to be
of too extreme fringe for most.

[Aristotle Pagaltzis]

Post by Ivan Pozdeev
According to
https://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Function-Attributes.html,
If the compiler determines that a null pointer is passed in an
argument slot marked as non-null, and the -Wnonnull option is
enabled, a warning is issued. The compiler may also choose to make
optimizations based on the knowledge that certain function
arguments will not be null.
I.e. the primary purpose is code verification rather than optimization.

Please read the preceding conversation. The fact that it its actual
effect is different from what you would naïvely expect from a surface
reading of the docs (though strictly speaking it’s still in accordance
with them) is the entire point of this controversy. Just quoting the
docs means you missed the point entirely.

Regards,

[Aristotle Pagaltzis]

Post by bulk88
Removing the non-null attribute increases the amount of GCC 4.9.2 -O2
branches. You are deoptimizing the code.

Correct. As he ought to.

Post by bulk88
Sigh. I give up.

Please don’t.

Regards,