Re: Invalid _Static_assert expanded from HASH_CALLBACKS_CHECK

[Tim Deegan <tim@xen.org>]

Hi,

At 08:45 +0200 on 31 May (1622450756), Jan Beulich wrote:
> On 28.05.2021 17:44, Tim Deegan wrote:
> > Hi,
> > 
> > At 10:58 +0200 on 25 May (1621940330), Jan Beulich wrote:
> >> On 24.05.2021 06:29, Roberto Bagnara wrote:
> >>> I stumbled upon parsing errors due to invalid uses of
> >>> _Static_assert expanded from HASH_CALLBACKS_CHECK where
> >>> the tested expression is not constant, as mandated by
> >>> the C standard.
> >>>
> >>> Judging from the following comment, there is partial awareness
> >>> of the fact this is an issue:
> >>>
> >>> #ifndef __clang__ /* At least some versions dislike some of the uses. */
> >>> #define HASH_CALLBACKS_CHECK(mask) \
> >>>      BUILD_BUG_ON((mask) > (1U << ARRAY_SIZE(callbacks)) - 1)
> >>>
> >>> Indeed, this is not a fault of Clang: the point is that some
> >>> of the expansions of this macro are not C.  Moreover,
> >>> the fact that GCC sometimes accepts them is not
> >>> something we can rely upon:
> > 
> > Well, that is unfortunate - especially since the older ad-hoc
> > compile-time assertion macros handled this kind of thing pretty well.
> > Why when I were a lad &c &c. :)
> 
> So I have to admit I don't understand: The commit introducing
> HASH_CALLBACKS_CHECK() (90629587e16e "x86/shadow: replace stale
> literal numbers in hash_{vcpu,domain}_foreach()") did not replace
> any prior compile-time checking. Hence I wonder what you're
> referring to (and hence what alternative ways of dealing with the
> situation there might be that I'm presently not seeing).

Sorry, I wasn't clear.  Before there was compiler support for
compile-time assertions, people used horrible macros that expanded to
things like int x[(p)?0:-1].  (I don't remember which exact flavour we
had in Xen.)  Those worked fine with static consts because the
predicates only had to be compile-time constant in practice, but now
they have to be constant in principle too.

So I don't think there was a better way of adding these assertions in
90629587e16e, I'm just generally grumbling that the official
compile-time assertions are not quite as useful as the hacks they
replaced.

And I am definitely *not* suggesting that we go back to those kind of
hacks just to get around the compiler's insistence on the letter of
the law. :)

Cheers,

Tim.