On 01.03.2021 17:03, Ian Jackson wrote:
> Jan Beulich writes ("[PATCH 2/2][4.15?] x86: fix build when NR_CPUS == 1"):
>> In this case the compiler is recognizing that no valid array indexes
>> remain (in x2apic_cluster()'s access to per_cpu(cpu_2_logical_apicid,
>> ...)), but oddly enough isn't really consistent about the checking it
>> does (see the code comment).
> ...
>> - if (this_cpu == cpu || x2apic_cluster(this_cpu) !=
>> x2apic_cluster(cpu))
>> + if ( this_cpu == cpu )
>> + continue;
>> + /*
>> + * Guard in particular against the compiler suspecting out-of-bounds
>> + * array accesses below when NR_CPUS=1 (oddly enough with gcc 10 it
>> + * is the 1st of these alone which actually helps, not the 2nd, nor
>> + * are both required together there).
>> + */
>> + BUG_ON(this_cpu >= NR_CPUS);
>> + BUG_ON(cpu >= NR_CPUS);
>> + if ( x2apic_cluster(this_cpu) != x2apic_cluster(cpu) )
>> continue;
>
> Is there some particular reason for not putting the BUG_ON before the
> if test ? That would avoid the refactoring.
I want it to be as close as possible to the place where the issue
is. I also view the refactoring as a good thing, since it allows
a style correction as a side effect.
> Of course putting it in next to the array indexing would address that
> too.
See my earlier reply to Roger's similar remark (which still was
along the lines of what I've said above).
Jan
