Re: [PATCH 1/6] x86/cpu-policy: Drop build time cross-checks of featureset sizes

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/05/2023 3:28 pm, Jan Beulich wrote:
> On 09.05.2023 15:04, Andrew Cooper wrote:
>> On 08/05/2023 7:47 am, Jan Beulich wrote:
>>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic 
>>>> for
>>>> code which looks like:
>>>>
>>>>   uint32_t foo[1] = { 1, 2, 3 };
>>>>
>>>> However, GCC 12 at least does now warn for this:
>>>>
>>>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>>>     884 | uint32_t foo[1] = { 1, 2, 3 };
>>>>         |                        ^
>>>>   foo.c:1:24: note: (near initialization for 'foo')
>>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>>> the arrays in question don't have explicit dimensions at their
>>> definition sites, and hence they derive their dimensions from their
>>> initializers. So the build-time-checks are about the arrays in fact
>>> obtaining the right dimensions, i.e. the initializers being suitable.
>>>
>>> With the core part of the reasoning not being applicable, I'm afraid I
>>> can't even say "okay with an adjusted description".
>> Now I'm extra confused.
>>
>> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
>> when I was expecting one, and there was a bug in the original featureset
>> work caused by this going wrong.
>>
>> But godbolt seems to agree that even GCC 4.1 notices.
>>
>> Maybe it was some other error (C file not seeing the header properly?)
>> which disappeared across the upstream review?
> Or maybe, by mistake, too few initializer fields? But what exactly it
> was probably doesn't matter. If this patch is to stay (see below), some
> different description will be needed anyway (or the change be folded
> into the one actually invalidating those BUILD_BUG_ON()s).
>
>> Either way, these aren't appropriate, and need deleting before patch 5,
>> because the check is no longer valid when a featureset can be longer
>> than the autogen length.
> Well, they need deleting if we stick to the approach chosen there right
> now. If we switched to my proposed alternative, they better would stay.

Given that all versions of GCC do warn, I don't see any justification
for them to stay.

i.e. this should be committed, even if the commit message says "no idea
what they were taken originally, but they're superfluous in the logic as
it exists today".

~Andrew