Re: [XEN PATCH][for-4.19 1/9] xen/include: add macro LOWEST_POW2

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

On 06/10/2023 11:29, Julien Grall wrote:
> Hi,
> 
> On 06/10/2023 09:26, Nicola Vetrini wrote:
>> The purpose of this macro is to encapsulate the well-known expression
>> 'x & -x', that in 2's complement architectures on unsigned integers 
>> will
>> give 2^ffs(x), where ffs(x) is the position of the lowest set bit in 
>> x.
>> 
>> A deviation for ECLAIR is also introduced.
> 
> Can you explain why this is a deviation in ECLAIR rather than one with
> /* SAF-* */ (or whichever name we decide to rename to)? Is this
> because the code is correct from MISRA perspective but the tool is
> confused?
> 

The code does violate Rule 10.1 (a unary minus applied to an unsigned 
value is
deemed inappropriate by MISRA), but rather than changing a whole lot of 
places where this
construct is used (mainly in x86 code), the reasoning is that it makes 
more sense to isolate
it and justify its presence by the fact that on 2's complement 
architectures the result is
indeed correct.

>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>>   automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
>>   xen/include/xen/macros.h                         | 6 ++++--
>>   2 files changed, 10 insertions(+), 2 deletions(-)
>> 
>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
>> b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> index d8170106b449..016164643105 100644
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -274,6 +274,12 @@ still non-negative."
>>   -config=MC3R1.R10.1,etypes+={safe, 
>> "stmt(operator(logical)||node(conditional_operator||binary_conditional_operator))", 
>> "dst_type(ebool||boolean)"}
>>   -doc_end
>>   +-doc_begin="The macro LOWEST_POW2 encapsulates a well-known pattern 
>> to obtain the value
>> +2^ffs(x) for unsigned integers on two's complement architectures
>> +(all the architectures supported by Xen satisfy this requirement)."
>> +-config=MC3R1.R10.1,reports+={safe, 
>> "any_area(any_loc(any_exp(macro(^LOWEST_POW2$))))"}
>> +-doc_end
>> +
>>   ### Set 3 ###
>>     #
>> diff --git a/xen/include/xen/macros.h b/xen/include/xen/macros.h
>> index d0caae7db298..bb9a1c9a53d0 100644
>> --- a/xen/include/xen/macros.h
>> +++ b/xen/include/xen/macros.h
>> @@ -8,8 +8,10 @@
>>   #define DIV_ROUND(n, d) (((n) + (d) / 2) / (d))
>>   #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
>>   -#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
>> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>> +#define LOWEST_POW2(x) ((x) & -(x))
>> +
>> +#define MASK_EXTR(v, m) (((v) & (m)) / LOWEST_POW2(m))
>> +#define MASK_INSR(v, m) (((v) * LOWEST_POW2(m)) & (m))
>>     #define count_args_(dot, a1, a2, a3, a4, a5, a6, a7, a8, x, ...) x
>>   #define count_args(args...) \

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)