Skip to contentSkip to conversation

arm64: sysreg: fix sparse warnings

PATCH New Sergey Matyukevich 4 participants 10 messages Nov 9, 2018 – Nov 16, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Specify correct type for the constants to avoid
the following sparse complaints:

./arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0xffffffffffffffff is so big it is unsigned long ./arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0xffffffffffffffff is so big it is unsigned long

---
 arch/arm64/include/asm/sysreg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/include/asm/sysreg.h +2 -2
index 0c909c4a932f..e025aaa35178 100644
@@ -468,7 +468,7 @@
 			 SCTLR_ELx_SA     | SCTLR_ELx_I    | SCTLR_ELx_WXN | \
 			 SCTLR_ELx_DSSBS | ENDIAN_CLEAR_EL2 | SCTLR_EL2_RES0)
 
-#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffff
+#if (SCTLR_EL2_SET ^ SCTLR_EL2_CLEAR) != 0xffffffffffffffffULL
 #error "Inconsistent SCTLR_EL2 set/clear bits"
 #endif
 
@@ -509,7 +509,7 @@
 			 SCTLR_EL1_UMA | SCTLR_ELx_WXN     | ENDIAN_CLEAR_EL1 |\
 			 SCTLR_ELx_DSSBS | SCTLR_EL1_NTWI  | SCTLR_EL1_RES0)
 
-#if (SCTLR_EL1_SET ^ SCTLR_EL1_CLEAR) != 0xffffffffffffffff
+#if (SCTLR_EL1_SET ^ SCTLR_EL1_CLEAR) != 0xffffffffffffffffULL
 #error "Inconsistent SCTLR_EL1 set/clear bits"
 #endif
 
Signature · 1 line
2.11.0
#1 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 10 lines · Sergey Matyukevich

Specify correct type for the constants to avoid
the following sparse complaints:

./arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0xffffffffffffffff is so big it is unsigned long ./arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0xffffffffffffffff is so big it is unsigned long

---
 arch/arm64/include/asm/sysreg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

I thought this was fixed in newer versions of sparse so that it treats AArch64 as 64-bit? [see sparse commit 49d56b6969d2f]

Will

#2 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 7 lines · Will Deacon
Quoted 6 lines · Sergey Matyukevich

Specify correct type for the constants to avoid
the following sparse complaints:

./arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0xffffffffffffffff is so big it is unsigned long ./arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0xffffffffffffffff is so big it is unsigned long

Quoted 7 lines
Quoted 4 lines
---
 arch/arm64/include/asm/sysreg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

I thought this was fixed in newer versions of sparse so that it treats AArch64 as 64-bit? [see sparse commit 49d56b6969d2f]

I am using up-to-date sparse. However this warning still appears, not sure why. Maybe sparse treats such constants as unsigned int ?

Meanwhile it looks like I chose wrong type: it is enough to use UL instead of ULL to make sparse happy. In fact warning is clear about it.

Regards,
Sergey

#3 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 16 lines · Sergey Matyukevich
Quoted 7 lines · Will Deacon
Quoted 6 lines · Sergey Matyukevich

Specify correct type for the constants to avoid
the following sparse complaints:

./arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0xffffffffffffffff is so big it is unsigned long ./arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0xffffffffffffffff is so big it is unsigned long

Quoted 7 lines
Quoted 4 lines
---
 arch/arm64/include/asm/sysreg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

I thought this was fixed in newer versions of sparse so that it treats AArch64 as 64-bit? [see sparse commit 49d56b6969d2f]

Quoted 6 lines

I am using up-to-date sparse. However this warning still appears, not sure why. Maybe sparse treats such constants as unsigned int ?

Meanwhile it looks like I chose wrong type: it is enough to use UL instead of ULL to make sparse happy. In fact warning is clear about it.

Ah yes, sorry. I misread the patch the first time around and thought you were changing a UL suffix to a ULL suffix. So actually, I think this is just a case of sparse getting confused about this constant because it's actually going to get parsed by the pre-processor, which says:

  | It [the preprocessor parsing #if] carries out all calculations in the
  | widest integer type known to the compiler; on most machines supported by
  | GCC this is 64 bits

so I still think this is a false positive. Adding Luc in case he has any ideas.

Will

#4 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 29 lines · Will Deacon
Quoted 16 lines · Sergey Matyukevich
Quoted 7 lines · Will Deacon
Quoted 6 lines · Sergey Matyukevich

Specify correct type for the constants to avoid
the following sparse complaints:

./arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0xffffffffffffffff is so big it is unsigned long ./arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0xffffffffffffffff is so big it is unsigned long

Quoted 7 lines
Quoted 4 lines
---
 arch/arm64/include/asm/sysreg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

I thought this was fixed in newer versions of sparse so that it treats AArch64 as 64-bit? [see sparse commit 49d56b6969d2f]

Quoted 6 lines

I am using up-to-date sparse. However this warning still appears, not sure why. Maybe sparse treats such constants as unsigned int ?

Meanwhile it looks like I chose wrong type: it is enough to use UL instead of ULL to make sparse happy. In fact warning is clear about it.

Ah yes, sorry. I misread the patch the first time around and thought you were changing a UL suffix to a ULL suffix. So actually, I think this is just a case of sparse getting confused about this constant because it's actually going to get parsed by the pre-processor, which says:

Quoted 6 lines
  | It [the preprocessor parsing #if] carries out all calculations in the
  | widest integer type known to the compiler; on most machines supported by
  | GCC this is 64 bits

so I still think this is a false positive. Adding Luc in case he has any ideas.

Well, I think the warning is 'correct' (and the message is quite clear about the 'possible' problem) but probably not useful enough. In fact, some months ago, it was agreed that sparse will only issue this warning when -Wpedantic is set but this flag is not yet handled.

I'll look at this next week.

-- Luc

#5 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 34 lines · Luc Van Oostenryck
Quoted 33 lines · Will Deacon
Quoted 16 lines · Sergey Matyukevich
Quoted 7 lines · Will Deacon
Quoted 6 lines · Sergey Matyukevich

Specify correct type for the constants to avoid
the following sparse complaints:

./arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0xffffffffffffffff is so big it is unsigned long ./arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0xffffffffffffffff is so big it is unsigned long

Quoted 7 lines
Quoted 4 lines
---
 arch/arm64/include/asm/sysreg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

I thought this was fixed in newer versions of sparse so that it treats AArch64 as 64-bit? [see sparse commit 49d56b6969d2f]

Quoted 6 lines

I am using up-to-date sparse. However this warning still appears, not sure why. Maybe sparse treats such constants as unsigned int ?

Meanwhile it looks like I chose wrong type: it is enough to use UL instead of ULL to make sparse happy. In fact warning is clear about it.

Ah yes, sorry. I misread the patch the first time around and thought you were changing a UL suffix to a ULL suffix. So actually, I think this is just a case of sparse getting confused about this constant because it's actually going to get parsed by the pre-processor, which says:

  | It [the preprocessor parsing #if] carries out all calculations in the
  | widest integer type known to the compiler; on most machines supported by
  | GCC this is 64 bits
Quoted 5 lines
Quoted 2 lines

so I still think this is a false positive. Adding Luc in case he has any ideas.

Well, I think the warning is 'correct' (and the message is quite clear about the 'possible' problem) but probably not useful enough.

yes, the warning is correct. I think the message is quite clear, so would be happy to hear any possible improvements! ;-)

(Also, correcting the source, in order to suppress these warnings, should take mere seconds - for each such warning, of course).

Quoted 2 lines

In fact, some months ago, it was agreed that sparse will only issue this warning when -Wpedantic is set but this flag is not yet handled.

I have patches, but I have been waiting for the 'official repo' issue to get resolved. (If you remember, I was building on top of the current official repo and continually merging to the master branch of your github sparse.git - but I got bored of doing that!).

Quoted 1 line

I'll look at this next week.

I stopped work on my patches months ago, after doing just enough to enable me to work on git without tearing my hair out on Linux Mint 19 (or Ubuntu 18.04, Fedora 27+ etc.). ;-)

I was working on -Wsystem-headers, so this warning (along with some other __extension__ warnings in the system headers) do not cause me any further issues on git. (that would not help on the kernel, of course).

[BTW, I have been building/testing/running the master branch of
git://github.com/lucvoo/sparse.git for months now, without seeing any regressions.]

ATB,
Ramsay Jones

#6 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 37 lines · Ramsay Jones
Quoted 34 lines · Luc Van Oostenryck
Quoted 33 lines · Will Deacon
Quoted 16 lines · Sergey Matyukevich
Quoted 7 lines · Will Deacon
Quoted 6 lines · Sergey Matyukevich

Specify correct type for the constants to avoid
the following sparse complaints:

./arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0xffffffffffffffff is so big it is unsigned long ./arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0xffffffffffffffff is so big it is unsigned long

Quoted 7 lines
Quoted 4 lines
---
 arch/arm64/include/asm/sysreg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

I thought this was fixed in newer versions of sparse so that it treats AArch64 as 64-bit? [see sparse commit 49d56b6969d2f]

Quoted 6 lines

I am using up-to-date sparse. However this warning still appears, not sure why. Maybe sparse treats such constants as unsigned int ?

Meanwhile it looks like I chose wrong type: it is enough to use UL instead of ULL to make sparse happy. In fact warning is clear about it.

Ah yes, sorry. I misread the patch the first time around and thought you were changing a UL suffix to a ULL suffix. So actually, I think this is just a case of sparse getting confused about this constant because it's actually going to get parsed by the pre-processor, which says:

  | It [the preprocessor parsing #if] carries out all calculations in the
  | widest integer type known to the compiler; on most machines supported by
  | GCC this is 64 bits
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

so I still think this is a false positive. Adding Luc in case he has any ideas.

Well, I think the warning is 'correct' (and the message is quite clear about the 'possible' problem) but probably not useful enough.

yes, the warning is correct. I think the message is quite clear, so would be happy to hear any possible improvements! ;-)

Oh, I was not talking about the message but, like Linus explained, that since no bits are lost here (and sparse has another warning in case some bits would be lost) and nothing is incorrect, the warning is not really interesting.

Quoted 2 lines

(Also, correcting the source, in order to suppress these warnings, should take mere seconds - for each such warning, of course).

Yes, sure, it was even the origin of the thread.

Quoted 7 lines
Quoted 2 lines

In fact, some months ago, it was agreed that sparse will only issue this warning when -Wpedantic is set but this flag is not yet handled.

I have patches, but I have been waiting for the 'official repo' issue to get resolved. (If you remember, I was building on top of the current official repo and continually merging to the master branch of your github sparse.git - but I got bored of doing that!).

Yes, I understand. This should be solved now.

Quoted 8 lines
Quoted 1 line

I'll look at this next week.

I stopped work on my patches months ago, after doing just enough to enable me to work on git without tearing my hair out on Linux Mint 19 (or Ubuntu 18.04, Fedora 27+ etc.). ;-)

I was working on -Wsystem-headers, so this warning (along with some other __extension__ warnings in the system headers) do not cause me

Interesting. I'll be glad to review them.

Quoted 5 lines

any further issues on git. (that would not help on the kernel, of course).

[BTW, I have been building/testing/running the master branch of
git://github.com/lucvoo/sparse.git for months now, without seeing any regressions.]

That's always good to know. Thank you.

-- Luc

#7 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 51 lines
Quoted 29 lines
Quoted 29 lines
Quoted 29 lines
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines

the following sparse complaints:

./arch/arm64/include/asm/sysreg.h:471:42: warning: constant 0xffffffffffffffff is so big it is unsigned long ./arch/arm64/include/asm/sysreg.h:512:42: warning: constant 0xffffffffffffffff is so big it is unsigned long

Quoted 7 lines
Quoted 4 lines
---
 arch/arm64/include/asm/sysreg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

I thought this was fixed in newer versions of sparse so that it treats AArch64 as 64-bit? [see sparse commit 49d56b6969d2f]

Quoted 6 lines

I am using up-to-date sparse. However this warning still appears, not sure why. Maybe sparse treats such constants as unsigned int ?

Meanwhile it looks like I chose wrong type: it is enough to use UL instead of ULL to make sparse happy. In fact warning is clear about it.

Ah yes, sorry. I misread the patch the first time around and thought you were changing a UL suffix to a ULL suffix. So actually, I think this is just a case of sparse getting confused about this constant because it's actually going to get parsed by the pre-processor, which says:

  | It [the preprocessor parsing #if] carries out all calculations in the
  | widest integer type known to the compiler; on most machines supported by
  | GCC this is 64 bits
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

so I still think this is a false positive. Adding Luc in case he has any ideas.

Well, I think the warning is 'correct' (and the message is quite clear about the 'possible' problem) but probably not useful enough.

yes, the warning is correct. I think the message is quite clear, so would be happy to hear any possible improvements! ;-)

Oh, I was not talking about the message but, like Linus explained, that since no bits are lost here (and sparse has another warning in case some bits would be lost) and nothing is incorrect, the warning is not really interesting.

Quoted 2 lines

(Also, correcting the source, in order to suppress these warnings, should take mere seconds - for each such warning, of course).

Yes, sure, it was even the origin of the thread.

Quoted 3 lines
Quoted 2 lines

In fact, some months ago, it was agreed that sparse will only issue this warning when -Wpedantic is set but this flag is not yet handled.

Quoted 6 lines
Quoted 4 lines

I have patches, but I have been waiting for the 'official repo' issue to get resolved. (If you remember, I was building on top of the current official repo and continually merging to the master branch of your github sparse.git - but I got bored of doing that!).

Yes, I understand. This should be solved now.

Hi,

Could you please clarify what is your resolution regarding that warning. I pulled all the changes from sparse repository on kernel.org. However the same warning is still here when building modules with C=2 option.

Regards,
Sergey

#8 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 5 lines

Hi,

Could you please clarify what is your resolution regarding that warning. I pulled all the changes from sparse repository on kernel.org. However the same warning is still here when building modules with C=2 option.

Hi,

Yes, the warning is still there since it's maybe not desired but is at least legit. For the short term, I think the best is your original patch (but using UL as the prefix). I would be glad to ack such a patch (I tried to answer in the original thread but I never found it although I should be subscribed to the linux-arm-kernel mailing list).

For the slightly longer term, I'll need to see how Ramsay's patches can also be used for kernel code.

Best regards,
-- Luc

#9 lore raw

PATCH 1/1 arm64: sysreg: fix sparse warnings

Quoted 15 lines · Luc Van Oostenryck
Quoted 5 lines

Hi,

Could you please clarify what is your resolution regarding that warning. I pulled all the changes from sparse repository on kernel.org. However the same warning is still here when building modules with C=2 option.

Hi,

Yes, the warning is still there since it's maybe not desired but is at least legit. For the short term, I think the best is your original patch (but using UL as the prefix). I would be glad to ack such a patch (I tried to answer in the original thread but I never found it although I should be subscribed to the linux-arm-kernel mailing list).

Quoted 2 lines

For the slightly longer term, I'll need to see how Ramsay's patches can also be used for kernel code.

Thanks for clarifiation! I will send v2.

Regards,
Sergey