diff mbox series

perf: enum overflow in uapi/linux/perf_event.h

Message ID a70172863fa7d3c138f788b18d36524bd45b0a73.1536326078.git.christophe.leroy@c-s.fr (mailing list archive)
State Rejected
Headers show
Series perf: enum overflow in uapi/linux/perf_event.h | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied
snowpatch_ozlabs/checkpatch success Test checkpatch on branch next
snowpatch_ozlabs/build-ppc64le success Test build-ppc64le on branch next
snowpatch_ozlabs/build-ppc64be success Test build-ppc64be on branch next
snowpatch_ozlabs/build-ppc64e success Test build-ppc64e on branch next
snowpatch_ozlabs/build-ppc32 success Test build-ppc32 on branch next

Commit Message

Christophe Leroy Sept. 7, 2018, 1:27 p.m. UTC
On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is
out of scope. The following sparse warning is encountered:

  CHECK   arch/powerpc/kernel/process.c
./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)

This patch changes it to a #define

Fixes: 6cbc304f2f360 ("perf/x86/intel: Fix unwind errors from PEBS entries (mk-II)")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 include/uapi/linux/perf_event.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peter Zijlstra Sept. 7, 2018, 1:42 p.m. UTC | #1
On Fri, Sep 07, 2018 at 01:27:19PM +0000, Christophe Leroy wrote:
> On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is
> out of scope. The following sparse warning is encountered:
> 
>   CHECK   arch/powerpc/kernel/process.c
> ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)

Urgh... what compiler is that? I've not seen anything like that from the
build bots.
Christophe Leroy Sept. 7, 2018, 1:50 p.m. UTC | #2
On 09/07/2018 01:42 PM, Peter Zijlstra wrote:
> On Fri, Sep 07, 2018 at 01:27:19PM +0000, Christophe Leroy wrote:
>> On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is
>> out of scope. The following sparse warning is encountered:
>>
>>    CHECK   arch/powerpc/kernel/process.c
>> ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> 
> Urgh... what compiler is that? I've not seen anything like that from the
> build bots.
> 

[root@pc16082vm linux-powerpc]# sparse --version
0.5.2

[root@pc16082vm linux-powerpc]# ppc-linux-gcc --version
ppc-linux-gcc (GCC) 5.4.0
Peter Zijlstra Sept. 7, 2018, 1:58 p.m. UTC | #3
On Fri, Sep 07, 2018 at 01:50:18PM +0000, Christophe Leroy wrote:
> 
> 
> On 09/07/2018 01:42 PM, Peter Zijlstra wrote:
> > On Fri, Sep 07, 2018 at 01:27:19PM +0000, Christophe Leroy wrote:
> > > On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is
> > > out of scope. The following sparse warning is encountered:
> > > 
> > >    CHECK   arch/powerpc/kernel/process.c
> > > ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> > 
> > Urgh... what compiler is that? I've not seen anything like that from the
> > build bots.
> > 
> 
> [root@pc16082vm linux-powerpc]# sparse --version
> 0.5.2
> 
> [root@pc16082vm linux-powerpc]# ppc-linux-gcc --version
> ppc-linux-gcc (GCC) 5.4.0

Ah, that's a sparse warning. But does your GCC agree? The thing is,
sparse uses the C enum spec, but I suspect GCC uses the C++ enum spec
and it all works fine.
Peter Zijlstra Sept. 7, 2018, 2:13 p.m. UTC | #4
On Fri, Sep 07, 2018 at 03:58:17PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 07, 2018 at 01:50:18PM +0000, Christophe Leroy wrote:
> > 
> > 
> > On 09/07/2018 01:42 PM, Peter Zijlstra wrote:
> > > On Fri, Sep 07, 2018 at 01:27:19PM +0000, Christophe Leroy wrote:
> > > > On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is
> > > > out of scope. The following sparse warning is encountered:
> > > > 
> > > >    CHECK   arch/powerpc/kernel/process.c
> > > > ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> > > 
> > > Urgh... what compiler is that? I've not seen anything like that from the
> > > build bots.
> > > 
> > 
> > [root@pc16082vm linux-powerpc]# sparse --version
> > 0.5.2
> > 
> > [root@pc16082vm linux-powerpc]# ppc-linux-gcc --version
> > ppc-linux-gcc (GCC) 5.4.0
> 
> Ah, that's a sparse warning. But does your GCC agree? The thing is,
> sparse uses the C enum spec, but I suspect GCC uses the C++ enum spec
> and it all works fine.

What does the below proglet print on your PPC32 box? I suspect the
output will be:

400000000000

and all is well.

---
#include <stdio.h>

enum ponies {
	big = 1ULL<<46,
};

int main(int argc, char **argv)
{
	unsigned long long val = big;
	printf("%Lx\n", val);
	return 0;
}
Christophe Leroy Sept. 7, 2018, 2:15 p.m. UTC | #5
Le 07/09/2018 à 15:58, Peter Zijlstra a écrit :
> On Fri, Sep 07, 2018 at 01:50:18PM +0000, Christophe Leroy wrote:
>>
>>
>> On 09/07/2018 01:42 PM, Peter Zijlstra wrote:
>>> On Fri, Sep 07, 2018 at 01:27:19PM +0000, Christophe Leroy wrote:
>>>> On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is
>>>> out of scope. The following sparse warning is encountered:
>>>>
>>>>     CHECK   arch/powerpc/kernel/process.c
>>>> ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
>>>
>>> Urgh... what compiler is that? I've not seen anything like that from the
>>> build bots.
>>>
>>
>> [root@pc16082vm linux-powerpc]# sparse --version
>> 0.5.2
>>
>> [root@pc16082vm linux-powerpc]# ppc-linux-gcc --version
>> ppc-linux-gcc (GCC) 5.4.0
> 
> Ah, that's a sparse warning. But does your GCC agree? The thing is,
> sparse uses the C enum spec, but I suspect GCC uses the C++ enum spec
> and it all works fine.
> 

Ah yes, it seems that GCC is happy. So sparse should be fixed instead ?

Anyway, is it really correct to put this constant inside that enum, 
after PERF_SAMPLE_MAX  ?

Christophe
Peter Zijlstra Sept. 7, 2018, 2:23 p.m. UTC | #6
On Fri, Sep 07, 2018 at 04:15:33PM +0200, Christophe LEROY wrote:

> Ah yes, it seems that GCC is happy. So sparse should be fixed instead ?

Ideally, yes.

> Anyway, is it really correct to put this constant inside that enum, after
> PERF_SAMPLE_MAX  ?

It is a bit of a hack, agreed. What we do is use the top bit of that
word (u64) for some internal state. By placing it there (after MAX) we
ensure it is not available for userspace (trying to set it will return
in -EINVAL) and by keeping it in the enum we know that bit is
unavailable for future use.

I have a patch queued that puts a little comment on that:

  https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/commit/?h=perf/urgent&id=34cad593c9ea350a1811ab718e64b36e5cde870c

(url is not stable, as I regenerate that git tree from quilt every so
often, but it should probably last the day).
Luc Van Oostenryck Sept. 7, 2018, 6:43 p.m. UTC | #7
On Fri, Sep 07, 2018 at 04:15:33PM +0200, Christophe LEROY wrote:
> Le 07/09/2018 à 15:58, Peter Zijlstra a écrit :
> > On Fri, Sep 07, 2018 at 01:50:18PM +0000, Christophe Leroy wrote:
> > > 
> > > 
> > > On 09/07/2018 01:42 PM, Peter Zijlstra wrote:
> > > > On Fri, Sep 07, 2018 at 01:27:19PM +0000, Christophe Leroy wrote:
> > > > > On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is
> > > > > out of scope. The following sparse warning is encountered:
> > > > > 
> > > > >     CHECK   arch/powerpc/kernel/process.c
> > > > > ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> > > > 
> > > > Urgh... what compiler is that? I've not seen anything like that from the
> > > > build bots.
> > > > 
> > > 
> > > [root@pc16082vm linux-powerpc]# sparse --version
> > > 0.5.2
> > > 
> > > [root@pc16082vm linux-powerpc]# ppc-linux-gcc --version
> > > ppc-linux-gcc (GCC) 5.4.0
> > 
> > Ah, that's a sparse warning. But does your GCC agree? The thing is,
> > sparse uses the C enum spec, but I suspect GCC uses the C++ enum spec
> > and it all works fine.

Sparse is a bit weird about the exact underlying type used for enums.

> Ah yes, it seems that GCC is happy. So sparse should be fixed instead ?

I'll investigate (I suppose the same is given on x86-32).

-- Luc
Luc Van Oostenryck Sept. 7, 2018, 11:55 p.m. UTC | #8
On Fri, Sep 07, 2018 at 08:43:59PM +0200, Luc Van Oostenryck wrote:
> On Fri, Sep 07, 2018 at 04:15:33PM +0200, Christophe LEROY wrote:
> > Le 07/09/2018 à 15:58, Peter Zijlstra a écrit :
> > > On Fri, Sep 07, 2018 at 01:50:18PM +0000, Christophe Leroy wrote:
> > > > 
> > > > 
> > > > On 09/07/2018 01:42 PM, Peter Zijlstra wrote:
> > > > > On Fri, Sep 07, 2018 at 01:27:19PM +0000, Christophe Leroy wrote:
> > > > > > On PPC32, enums are 32 bits, so __PERF_SAMPLE_CALLCHAIN_EARLY is
> > > > > > out of scope. The following sparse warning is encountered:
> > > > > > 
> > > > > >     CHECK   arch/powerpc/kernel/process.c
> > > > > > ./include/uapi/linux/perf_event.h:147:56: warning: cast truncates bits from constant value (8000000000000000 becomes 0)
> > > > > 
> > > > > Urgh... what compiler is that? I've not seen anything like that from the
> > > > > build bots.
> > > > > 
> > > > 
> > > > [root@pc16082vm linux-powerpc]# sparse --version
> > > > 0.5.2
> > > > 
> > > > [root@pc16082vm linux-powerpc]# ppc-linux-gcc --version
> > > > ppc-linux-gcc (GCC) 5.4.0
> > > 
> > > Ah, that's a sparse warning. But does your GCC agree? The thing is,
> > > sparse uses the C enum spec, but I suspect GCC uses the C++ enum spec
> > > and it all works fine.
> 
> Sparse is a bit weird about the exact underlying type used for enums.
> 
> > Ah yes, it seems that GCC is happy. So sparse should be fixed instead ?
> 
> I'll investigate (I suppose the same is given on x86-32).


It's definitively a bug in sparse. A relatively nasty one and which
open a can of worms. Fortunately, I had already looked at these
problems in May, I just didn't had the time to push the patches.

-- Luc
diff mbox series

Patch

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index eeb787b1c53c..27c7842bc86a 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -143,10 +143,10 @@  enum perf_event_sample_format {
 	PERF_SAMPLE_PHYS_ADDR			= 1U << 19,
 
 	PERF_SAMPLE_MAX = 1U << 20,		/* non-ABI */
-
-	__PERF_SAMPLE_CALLCHAIN_EARLY		= 1ULL << 63,
 };
 
+#define __PERF_SAMPLE_CALLCHAIN_EARLY		(1ULL << 63)
+
 /*
  * values to program into branch_sample_type when PERF_SAMPLE_BRANCH is set
  *