Re: [Tinycc-devel] Broken commit e460f7dbb216

[grischka]

On 30.07.2022 12:45, Vincent Lefevre wrote:
> I changed the & to && because it is more correct and makes more sense,
> but note that this is not really redundant because n is signed and
> n & (n - 1) is not portable when n = 0 or 1 if two's complement isn't
> necessarily used, because 0 may have several representations (well,
> tcc is not designed to work on such exotic platforms, but who knows
> in the future... and moreover, because of this general portability
> issue, compilers may issue warnings).

Sorry, what?  Are you saying that (n & (n - 1)) with n == 1 -> (1 & 0)
and with n == 0 -> (0 & -1) does not evaluate to 0 in both cases always
necessarily, in C?

> > As to the "n < 0 ||" clause by the way. the C-standards seem to say:
> >
> >    "Alignments are represented as values of the type size_t. Valid alignments
> >     include only fundamental alignments, plus an additional implementation-
> >     defined set of values, which may be empty.  Every valid alignment value
> >     shall be a nonnegative integral power of two."
> >
> > Well, when "size_t" means unsigned, how could it be not "nonnegative"
> > then ?!?
>
> I don't understand what you mean here. The fact is that in the code,
> n is of type int, not size_t. So the case n < 0 needs to be taken
> into account. Note that n comes from
>
>                    n = expr_const();
>
> thus may be negative.

Seems I have misunderstood what they are trying to say.  It seems that by
   "nonnegative integral power of two"
they mean any powers 0..n of two, as in
    2 ^ 0..n
as opposed to
    2 ^ -1.2
for example.

That hasn't to do much with "int n = expr_const();" in tcc though because
the 'n' there does not mean "2 ^ n", just as _Alignas(1) does not mean
to align at 2 ^ 1.  It means to align at 2 ^ 0 = 1.

Therfor the combination

    if (n < 0 || ...
        tcc_error("alignment must be a positive power of two");

still seems misleading.  "_Alignas(0x80000000) int x;" is not an
invalid declaration, per se.

> BTW, the condition seems incomplete because large values are forbidden.
> ISO C17 says in 6.2.8p2:
>
>    A fundamental alignment is a valid alignment less than or equal to
>    _Alignof (max_align_t).
>
> and in 6.7.5p3 (a constraint):
>
>    The constant expression shall be an integer constant expression.
>    It shall evaluate to a valid fundamental alignment, or to a valid
>    extended alignment supported by the implementation for an object
>    of the storage duration (if any) being declared, or to zero.

Then I think we can still accept larger values as "extended alignments
supported by implementation".  (Where really large values would crash
"the implementation" rather efficiently though).

--- grischka