Re: [GIT PULL] bcachefs

[Linus Torvalds <torvalds@linux-foundation.org>]

On Wed, 6 Sept 2023 at 13:02, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> And guess what happens when you have (unsigned char)-1? It does *not*
> cast back to -1.

Side note: again, this may be one of those "it works in practice",
because if we have -fshort-enums, I think 'enum
btree_node_locked_type' in turn ends up being represented as a 'signed
char', because that's the smallest simple type that can fit all those
values.

I don't think gcc ever uses less than that (ie while a six_lock_type
could fit in two bits, it's still going to be considered at least a
8-bit value in practice).

So we may have 'enum six_lock_type' essentially being 'unsigned char',
and when the code does

    mark_btree_node_locked(trans, path, 0, BTREE_NODE_UNLOCKED);

that BTREE_NODE_UNLOCKED value might actually be 255.

And then when it's cast to 'enum btree_node_locked_type' in the inline
function, the 255 will be cast to 'signed char', and we'll end up
compatible with '(enum btree_node_locked_type)-1' again.

So it's one of those things that are seriously wrong to do, but might
generate the expected code anyway.

Unless the compiler adds any other sanity checks, like UBSAN or
something, that actually uses the exact range of the enums.

It could happen even without UBSAN, if the compiler ends up going "I
can see that the original value came from a 'enum six_lock_type', so I
know the original value can't be signed, so any comparison with
BTREE_NODE_UNLOCKED can never be true.

But again, I suspect that in practice this all just happens to work.
That doesn't make it right.

               Linus