Re: [GIT PULL] ext4 changes for the 6.4 merge window

[Nick Desaulniers]

On Wed, Apr 26, 2023 at 10:03 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Apr 24, 2023 at 9:18 PM Theodore Ts'o <tytso@xxxxxxx> wrote:
> >
> > Please note that after merging the mm and ext4 trees you will need to
> > apply the patch found here[1].
> >
> > [1] https://lore.kernel.org/r/20230419120923.3152939-1-willy@xxxxxxxxxxxxx
> >
> > This is due to a patch in the mm tree, "mm: return an ERR_PTR from
> > __filemap_get_folio" changing that function to returning an ERR_PTR
> > instead of returning NULL on an error.
>
> Side note, itr would be wonderful if we could mark the places that
> return an error pointer as returning "nonnull", and catch things like
> this automatically at build time where people compare an error pointer
> to NULL.

That's what clang's _Nonnull attribute does (with -Wnullability-extension).
https://godbolt.org/z/6jo1zbMd9
But it's not toolchain portable, at the moment.  Would require changes
to clang to use the GNU C __attribute__ syntax, too (which I'm not
against adding support for).

>
> Howeder, it sadly turns out that compilers have gotten this completely wrong.
>
> gcc apparently completely screwed things up, and "nonnull" is not a
> warning aid, it's a "you can remove tests against NULL silently".
>
> And clang does seem to have taken the same approach with
> "returns_nonnull", which is really really sad, considering that
> apparently they got it right for "_Nonnull" for function arguments
> (where it's documented to cause a warning if you pass in a NULL
> argument, rather than cause the compiler to generate sh*t buggy code)

Heh, I just had this conversation maybe within the past month with
Bionic (Android's libc) developers.

Yeah, the nonnull attributes != _Nonnull "attributes." (Quotes because
IIUC _Nonnull doesn't use the __attribute__ GNU C extension syntax).
My understanding (which may be wrong) is that nonnull is implemented
for compatibility with GCC, while _Nonnull was likely implemented by
Apple (my guess; did not check) (so compatibility with GNU C
__attribute__ syntax probably wasn't considered in code review).

https://clang.llvm.org/docs/AttributeReference.html#nullability-attributes

The Bionic developers are deploying _Nonnull throughout the codebase
and intentionally not using nonnull which is dangerous (a teammate
used the term "Developer Hostile Behavior"). nonnull has implications
on codegen, _Nonnull only affects diagnostics.

https://android-review.googlesource.com/q/owner:zijunzhao@xxxxxxxxxx+Nullability

For examples. Works on return types, too.  So _Nonnull can be used on
return types rather than returns_nonnull.

>
> Compiler people who think that "undefined behavior is a good way to
> implement optimizations" are a menace, and should be shunned. They are
> paste-eaters of the worst kind.

Thanks! :-*

>
> Is there any chance that somebody could hit compiler people with a big
> clue-bat, and say "undefined behavior is not a feature, it's a bug",
> and try to make them grow up?

Good. I can feel your anger. Strike me down with all of your hatred,
and your journey to the dark side will be complete.  Your hate has
made you powerful.  Let the hate flow through you!

>
> Adding some clang people to the participants, since they at least seem
> to have *almost* gotten it right.
>
>             Linus



-- 
Thanks,
~Nick Desaulniers