ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of rearrangements")
introduced new violations on previously clean rules 20.9 and 20.12.
The first is introduced because CONFIG_CC_IS_CLANG in xen/self-tests.h is not
defined in the configuration under analysis. Using "defined()" instead avoids
relying on the preprocessor's behaviour upon encountering an undedfined identifier
and addresses the violation.
The violation of Rule 20.12 is due to "val" being used both as an ordinary argument
in macro RUNTIME_CHECK, and as a stringification operator.
No functional change.
Fixes: ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of rearrangements")
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
automation/eclair_analysis/ECLAIR/deviations.ecl | 2 +-
xen/include/xen/self-tests.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index f29db9e08248..e2653f77eb2c 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -473,7 +473,7 @@ deliberate."
-doc_begin="Uses of a macro parameter for ordinary expansion and as an operand
to the # or ## operators within the following macros are deliberate, to provide
useful diagnostic messages to the user."
--config=MC3R1.R20.12,macros+={deliberate, "name(ASSERT||BUILD_BUG_ON||BUILD_BUG_ON_ZERO)"}
+-config=MC3R1.R20.12,macros+={deliberate, "name(ASSERT||BUILD_BUG_ON||BUILD_BUG_ON_ZERO||RUNTIME_CHECK)"}
-doc_end
-doc_begin="The helper macro GENERATE_CASE may use a macro parameter for ordinary
diff --git a/xen/include/xen/self-tests.h b/xen/include/xen/self-tests.h
index 8410db7aaaae..42a4cc4d17fe 100644
--- a/xen/include/xen/self-tests.h
+++ b/xen/include/xen/self-tests.h
@@ -16,7 +16,7 @@
* Clang < 8 can't fold constants through static inlines, causing this to
* fail. Simply skip it for incredibly old compilers.
*/
-#if !CONFIG_CC_IS_CLANG || CONFIG_CLANG_VERSION >= 80000
+#if !defined(CONFIG_CC_IS_CLANG) || CONFIG_CLANG_VERSION >= 80000
#define COMPILE_CHECK(fn, val, res) \
do { \
typeof(fn(val)) real = fn(val); \
--
2.34.1On 01/06/2024 11:16 am, Nicola Vetrini wrote:
> ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of rearrangements")
> introduced new violations on previously clean rules 20.9 and 20.12.
>
> The first is introduced because CONFIG_CC_IS_CLANG in xen/self-tests.h is not
> defined in the configuration under analysis. Using "defined()" instead avoids
> relying on the preprocessor's behaviour upon encountering an undedfined identifier
> and addresses the violation.
>
> The violation of Rule 20.12 is due to "val" being used both as an ordinary argument
> in macro RUNTIME_CHECK, and as a stringification operator.
>
> No functional change.
>
> Fixes: ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of rearrangements")
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Thankyou for this patch. I'd seen that I'd broken something. (Entirely
my fault - I've done a lot of testing in Gitlab for the series, but
never manually ran the Eclair jobs. I'll try to remember better next time.)
One question though.
https://gitlab.com/xen-project/xen/-/jobs/6994213979 says:
Failure: 1 regressions found for clean guidelines
service MC3R1.R20.9: (required) All identifiers used in the
controlling expression of `#if' or `#elif' preprocessing directives
shall be #define'd before evaluation:
violation: 1
While there is a report for 20.12, it's not clean yet (so the first
sentence wants adjusting), and RUNTIME_CHECK doesn't show up newly in it.
So, while I agree that RUNTIME_CHECK() should be included in the 20.12
exclusions, why is current Gitlab Run not reporting it?
~Andrew
On 2024-06-01 14:47, Andrew Cooper wrote:
> On 01/06/2024 11:16 am, Nicola Vetrini wrote:
>> ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of
>> rearrangements")
>> introduced new violations on previously clean rules 20.9 and 20.12.
>>
>> The first is introduced because CONFIG_CC_IS_CLANG in xen/self-tests.h
>> is not
>> defined in the configuration under analysis. Using "defined()" instead
>> avoids
>> relying on the preprocessor's behaviour upon encountering an
>> undedfined identifier
>> and addresses the violation.
>>
>> The violation of Rule 20.12 is due to "val" being used both as an
>> ordinary argument
>> in macro RUNTIME_CHECK, and as a stringification operator.
>>
>> No functional change.
>>
>> Fixes: ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead
>> of rearrangements")
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>
> Thankyou for this patch. I'd seen that I'd broken something.
> (Entirely
> my fault - I've done a lot of testing in Gitlab for the series, but
> never manually ran the Eclair jobs. I'll try to remember better next
> time.)
>
> One question though.
> https://gitlab.com/xen-project/xen/-/jobs/6994213979 says:
>
> Failure: 1 regressions found for clean guidelines
> service MC3R1.R20.9: (required) All identifiers used in the
> controlling expression of `#if' or `#elif' preprocessing directives
> shall be #define'd before evaluation:
> violation: 1
>
> While there is a report for 20.12, it's not clean yet (so the first
> sentence wants adjusting), and RUNTIME_CHECK doesn't show up newly in
> it.
>
> So, while I agree that RUNTIME_CHECK() should be included in the 20.12
> exclusions, why is current Gitlab Run not reporting it?
>
> ~Andrew
While Rule 20.12 wasn't clean on x86, but it was on arm, so in the arm64
run it is reported as such
https://gitlab.com/xen-project/xen/-/jobs/6994213980
with the other patches in the series the rule should be clean on both
architectures, so this imbalance should disappear rather shortly.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
On 01/06/2024 1:58 pm, Nicola Vetrini wrote:
> On 2024-06-01 14:47, Andrew Cooper wrote:
>> On 01/06/2024 11:16 am, Nicola Vetrini wrote:
>>> ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of
>>> rearrangements")
>>> introduced new violations on previously clean rules 20.9 and 20.12.
>>>
>>> The first is introduced because CONFIG_CC_IS_CLANG in
>>> xen/self-tests.h is not
>>> defined in the configuration under analysis. Using "defined()"
>>> instead avoids
>>> relying on the preprocessor's behaviour upon encountering an
>>> undedfined identifier
>>> and addresses the violation.
>>>
>>> The violation of Rule 20.12 is due to "val" being used both as an
>>> ordinary argument
>>> in macro RUNTIME_CHECK, and as a stringification operator.
>>>
>>> No functional change.
>>>
>>> Fixes: ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure
>>> ahead of rearrangements")
>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>
>> Thankyou for this patch. I'd seen that I'd broken something. (Entirely
>> my fault - I've done a lot of testing in Gitlab for the series, but
>> never manually ran the Eclair jobs. I'll try to remember better next
>> time.)
>>
>> One question though.
>> https://gitlab.com/xen-project/xen/-/jobs/6994213979 says:
>>
>> Failure: 1 regressions found for clean guidelines
>> service MC3R1.R20.9: (required) All identifiers used in the
>> controlling expression of `#if' or `#elif' preprocessing directives
>> shall be #define'd before evaluation:
>> violation: 1
>>
>> While there is a report for 20.12, it's not clean yet (so the first
>> sentence wants adjusting), and RUNTIME_CHECK doesn't show up newly in
>> it.
>>
>> So, while I agree that RUNTIME_CHECK() should be included in the 20.12
>> exclusions, why is current Gitlab Run not reporting it?
>>
>> ~Andrew
>
> While Rule 20.12 wasn't clean on x86, but it was on arm, so in the
> arm64 run it is reported as such
>
> https://gitlab.com/xen-project/xen/-/jobs/6994213980
>
> with the other patches in the series the rule should be clean on both
> architectures, so this imbalance should disappear rather shortly.
>
Thanks - I'd just found the ARM report and was replying - I'll rebase
onto this thread.
I still don't understand why the violation doesn't show up on the x86
build. Specifically, why it's missing from here:
https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/xen/ECLAIR_normal/staging/X86_64/6994213979/prev/PROJECT.ecd;/by_service/MC3R1.R20.12.html
From the ARM report, one is xen/lib/generic-ffsl.c:61 and checking with
the x86 build log, we are building generic-ffsl.c too (which is expected.)
~Andrew
On 2024-06-01 15:08, Andrew Cooper wrote:
> On 01/06/2024 1:58 pm, Nicola Vetrini wrote:
>> On 2024-06-01 14:47, Andrew Cooper wrote:
>>> On 01/06/2024 11:16 am, Nicola Vetrini wrote:
>>>> ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of
>>>> rearrangements")
>>>> introduced new violations on previously clean rules 20.9 and 20.12.
>>>>
>>>> The first is introduced because CONFIG_CC_IS_CLANG in
>>>> xen/self-tests.h is not
>>>> defined in the configuration under analysis. Using "defined()"
>>>> instead avoids
>>>> relying on the preprocessor's behaviour upon encountering an
>>>> undedfined identifier
>>>> and addresses the violation.
>>>>
>>>> The violation of Rule 20.12 is due to "val" being used both as an
>>>> ordinary argument
>>>> in macro RUNTIME_CHECK, and as a stringification operator.
>>>>
>>>> No functional change.
>>>>
>>>> Fixes: ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure
>>>> ahead of rearrangements")
>>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>>
>>> Thankyou for this patch. I'd seen that I'd broken something.
>>> (Entirely
>>> my fault - I've done a lot of testing in Gitlab for the series, but
>>> never manually ran the Eclair jobs. I'll try to remember better next
>>> time.)
>>>
>>> One question though.
>>> https://gitlab.com/xen-project/xen/-/jobs/6994213979 says:
>>>
>>> Failure: 1 regressions found for clean guidelines
>>> service MC3R1.R20.9: (required) All identifiers used in the
>>> controlling expression of `#if' or `#elif' preprocessing directives
>>> shall be #define'd before evaluation:
>>> violation: 1
>>>
>>> While there is a report for 20.12, it's not clean yet (so the first
>>> sentence wants adjusting), and RUNTIME_CHECK doesn't show up newly in
>>> it.
>>>
>>> So, while I agree that RUNTIME_CHECK() should be included in the
>>> 20.12
>>> exclusions, why is current Gitlab Run not reporting it?
>>>
>>> ~Andrew
>>
>> While Rule 20.12 wasn't clean on x86, but it was on arm, so in the
>> arm64 run it is reported as such
>>
>> https://gitlab.com/xen-project/xen/-/jobs/6994213980
>>
>> with the other patches in the series the rule should be clean on both
>> architectures, so this imbalance should disappear rather shortly.
>>
>
> Thanks - I'd just found the ARM report and was replying - I'll rebase
> onto this thread.
>
> I still don't understand why the violation doesn't show up on the x86
> build. Specifically, why it's missing from here:
> https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/xen/ECLAIR_normal/staging/X86_64/6994213979/prev/PROJECT.ecd;/by_service/MC3R1.R20.12.html
>
Note the "prev" here in the URL: this means you're looking at the
analysis run before your series was merged (on 03147e6837ff045db)
this is the latest run for x86 on staging:
https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/xen/ECLAIR_normal/staging/X86_64/6994213979/PROJECT.ecd;/by_service/MC3R1.R20.12.html
> From the ARM report, one is xen/lib/generic-ffsl.c:61 and checking with
> the x86 build log, we are building generic-ffsl.c too (which is
> expected.)
>
> ~Andrew
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
On 01/06/2024 2:52 pm, Nicola Vetrini wrote:
> On 2024-06-01 15:08, Andrew Cooper wrote:
>> On 01/06/2024 1:58 pm, Nicola Vetrini wrote:
>>> On 2024-06-01 14:47, Andrew Cooper wrote:
>>>> On 01/06/2024 11:16 am, Nicola Vetrini wrote:
>>>>> ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure ahead of
>>>>> rearrangements")
>>>>> introduced new violations on previously clean rules 20.9 and 20.12.
>>>>>
>>>>> The first is introduced because CONFIG_CC_IS_CLANG in
>>>>> xen/self-tests.h is not
>>>>> defined in the configuration under analysis. Using "defined()"
>>>>> instead avoids
>>>>> relying on the preprocessor's behaviour upon encountering an
>>>>> undedfined identifier
>>>>> and addresses the violation.
>>>>>
>>>>> The violation of Rule 20.12 is due to "val" being used both as an
>>>>> ordinary argument
>>>>> in macro RUNTIME_CHECK, and as a stringification operator.
>>>>>
>>>>> No functional change.
>>>>>
>>>>> Fixes: ea59e7d780d9 ("xen/bitops: Cleanup and new infrastructure
>>>>> ahead of rearrangements")
>>>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>>>
>>>> Thankyou for this patch. I'd seen that I'd broken something.
>>>> (Entirely
>>>> my fault - I've done a lot of testing in Gitlab for the series, but
>>>> never manually ran the Eclair jobs. I'll try to remember better next
>>>> time.)
>>>>
>>>> One question though.
>>>> https://gitlab.com/xen-project/xen/-/jobs/6994213979 says:
>>>>
>>>> Failure: 1 regressions found for clean guidelines
>>>> service MC3R1.R20.9: (required) All identifiers used in the
>>>> controlling expression of `#if' or `#elif' preprocessing directives
>>>> shall be #define'd before evaluation:
>>>> violation: 1
>>>>
>>>> While there is a report for 20.12, it's not clean yet (so the first
>>>> sentence wants adjusting), and RUNTIME_CHECK doesn't show up newly in
>>>> it.
>>>>
>>>> So, while I agree that RUNTIME_CHECK() should be included in the 20.12
>>>> exclusions, why is current Gitlab Run not reporting it?
>>>>
>>>> ~Andrew
>>>
>>> While Rule 20.12 wasn't clean on x86, but it was on arm, so in the
>>> arm64 run it is reported as such
>>>
>>> https://gitlab.com/xen-project/xen/-/jobs/6994213980
>>>
>>> with the other patches in the series the rule should be clean on both
>>> architectures, so this imbalance should disappear rather shortly.
>>>
>>
>> Thanks - I'd just found the ARM report and was replying - I'll rebase
>> onto this thread.
>>
>> I still don't understand why the violation doesn't show up on the x86
>> build. Specifically, why it's missing from here:
>> https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/xen/ECLAIR_normal/staging/X86_64/6994213979/prev/PROJECT.ecd;/by_service/MC3R1.R20.12.html
>>
>>
>
> Note the "prev" here in the URL: this means you're looking at the
> analysis run before your series was merged (on 03147e6837ff045db)
>
> this is the latest run for x86 on staging:
>
> https://saas.eclairit.com:3787/fs/var/local/eclair/xen-project.ecdf/xen-project/xen/ECLAIR_normal/staging/X86_64/6994213979/PROJECT.ecd;/by_service/MC3R1.R20.12.html
Oh - thankyou for explaining.
~Andrew
© 2016 - 2024 Red Hat, Inc.