* Re: [PATCH] arm64: sysreg: fix sparse warnings
[not found] ` <20181110170311.GA7700@brain-police>
@ 2018-11-10 17:54 ` Luc Van Oostenryck
2018-11-12 16:32 ` Ramsay Jones
0 siblings, 1 reply; 6+ messages in thread
From: Luc Van Oostenryck @ 2018-11-10 17:54 UTC (permalink / raw)
To: Will Deacon
Cc: Sergey Matyukevich, Suzuki K Poulose, Marc Zyngier,
Catalin Marinas, linux-sparse, linux-arm-kernel
On Sat, Nov 10, 2018 at 05:03:21PM +0000, Will Deacon wrote:
> On Sat, Nov 10, 2018 at 02:16:47PM +0300, Sergey Matyukevich wrote:
> > On Sat, Nov 10, 2018 at 12:16:16AM +0000, Will Deacon wrote:
> > > On Fri, Nov 09, 2018 at 11:47:47PM +0300, Sergey Matyukevich wrote:
> > > > 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
> > > >
> > > > Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> > > > ---
> > > > 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.
>
> 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: sysreg: fix sparse warnings
2018-11-10 17:54 ` [PATCH] arm64: sysreg: fix sparse warnings Luc Van Oostenryck
@ 2018-11-12 16:32 ` Ramsay Jones
2018-11-14 22:25 ` Luc Van Oostenryck
0 siblings, 1 reply; 6+ messages in thread
From: Ramsay Jones @ 2018-11-12 16:32 UTC (permalink / raw)
To: Luc Van Oostenryck, Will Deacon
Cc: Sergey Matyukevich, Suzuki K Poulose, Marc Zyngier,
Catalin Marinas, linux-sparse, linux-arm-kernel
On 10/11/2018 17:54, Luc Van Oostenryck wrote:
> On Sat, Nov 10, 2018 at 05:03:21PM +0000, Will Deacon wrote:
>> On Sat, Nov 10, 2018 at 02:16:47PM +0300, Sergey Matyukevich wrote:
>>> On Sat, Nov 10, 2018 at 12:16:16AM +0000, Will Deacon wrote:
>>>> On Fri, Nov 09, 2018 at 11:47:47PM +0300, Sergey Matyukevich wrote:
>>>>> 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
>>>>>
>>>>> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
>>>>> ---
>>>>> 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.
>>
>> 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.
>
> 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).
> 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!).
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: sysreg: fix sparse warnings
2018-11-12 16:32 ` Ramsay Jones
@ 2018-11-14 22:25 ` Luc Van Oostenryck
2018-11-15 8:35 ` Sergey Matyukevich
0 siblings, 1 reply; 6+ messages in thread
From: Luc Van Oostenryck @ 2018-11-14 22:25 UTC (permalink / raw)
To: Ramsay Jones
Cc: Sergey Matyukevich, Suzuki K Poulose, Marc Zyngier,
Catalin Marinas, Will Deacon, linux-sparse, linux-arm-kernel
On Mon, Nov 12, 2018 at 04:32:31PM +0000, Ramsay Jones wrote:
>
>
> On 10/11/2018 17:54, Luc Van Oostenryck wrote:
> > On Sat, Nov 10, 2018 at 05:03:21PM +0000, Will Deacon wrote:
> >> On Sat, Nov 10, 2018 at 02:16:47PM +0300, Sergey Matyukevich wrote:
> >>> On Sat, Nov 10, 2018 at 12:16:16AM +0000, Will Deacon wrote:
> >>>> On Fri, Nov 09, 2018 at 11:47:47PM +0300, Sergey Matyukevich wrote:
> >>>>> 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
> >>>>>
> >>>>> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> >>>>> ---
> >>>>> 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.
> >>
> >> 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.
> >
> > 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.
> (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.
> > 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.
> > 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.
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: sysreg: fix sparse warnings
2018-11-14 22:25 ` Luc Van Oostenryck
@ 2018-11-15 8:35 ` Sergey Matyukevich
2018-11-16 10:34 ` Luc Van Oostenryck
0 siblings, 1 reply; 6+ messages in thread
From: Sergey Matyukevich @ 2018-11-15 8:35 UTC (permalink / raw)
To: Luc Van Oostenryck
Cc: Ramsay Jones, Suzuki K Poulose, Marc Zyngier, Catalin Marinas,
Will Deacon, linux-sparse, linux-arm-kernel
> > >>>>> 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
> > >>>>>
> > >>>>> Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>
> > >>>>> ---
> > >>>>> 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.
> > >>
> > >> 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.
> > >
> > > 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.
>
> > (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.
>
> > > 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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: sysreg: fix sparse warnings
2018-11-15 8:35 ` Sergey Matyukevich
@ 2018-11-16 10:34 ` Luc Van Oostenryck
2018-11-16 13:54 ` Sergey Matyukevich
0 siblings, 1 reply; 6+ messages in thread
From: Luc Van Oostenryck @ 2018-11-16 10:34 UTC (permalink / raw)
To: Sergey Matyukevich
Cc: Ramsay Jones, Suzuki K Poulose, Marc Zyngier, Catalin Marinas,
Will Deacon, linux-sparse, linux-arm-kernel
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: sysreg: fix sparse warnings
2018-11-16 10:34 ` Luc Van Oostenryck
@ 2018-11-16 13:54 ` Sergey Matyukevich
0 siblings, 0 replies; 6+ messages in thread
From: Sergey Matyukevich @ 2018-11-16 13:54 UTC (permalink / raw)
To: Luc Van Oostenryck
Cc: Ramsay Jones, Suzuki K Poulose, Marc Zyngier, Catalin Marinas,
Will Deacon, linux-sparse, linux-arm-kernel
On Fri, Nov 16, 2018 at 11:34:00AM +0100, Luc Van Oostenryck wrote:
> > 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.
Thanks for clarifiation! I will send v2.
Regards,
Sergey
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-16 13:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20181109204747.10008-1-geomatsi@gmail.com>
[not found] ` <20181110001615.GA5665@brain-police>
[not found] ` <20181110111647.GA3511@speedy.hunter>
[not found] ` <20181110170311.GA7700@brain-police>
2018-11-10 17:54 ` [PATCH] arm64: sysreg: fix sparse warnings Luc Van Oostenryck
2018-11-12 16:32 ` Ramsay Jones
2018-11-14 22:25 ` Luc Van Oostenryck
2018-11-15 8:35 ` Sergey Matyukevich
2018-11-16 10:34 ` Luc Van Oostenryck
2018-11-16 13:54 ` Sergey Matyukevich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).