* Re: [PATCH net-next] net: can: Fix compiling warning [not found] ` <6e1c5aa0-8ed3-eec3-a34d-867ea8f54e9d@hartkopp.net> @ 2019-08-07 10:50 ` Dan Carpenter 2019-08-12 17:19 ` Kees Cook 0 siblings, 1 reply; 3+ messages in thread From: Dan Carpenter @ 2019-08-07 10:50 UTC (permalink / raw) To: Oliver Hartkopp, Patrick Bellasi, linux-sparse Cc: Mao Wenan, davem, netdev, linux-kernel, kernel-janitors, Ingo Molnar On Tue, Aug 06, 2019 at 06:41:44PM +0200, Oliver Hartkopp wrote: > I compiled the code (the original version), but I do not get that "Should it > be static?" warning: > > user@box:~/net-next$ make C=1 > CALL scripts/checksyscalls.sh > CALL scripts/atomic/check-atomics.sh > DESCEND objtool > CHK include/generated/compile.h > CHECK net/can/af_can.c > ./include/linux/sched.h:609:43: error: bad integer constant expression > ./include/linux/sched.h:609:73: error: invalid named zero-width bitfield > `value' > ./include/linux/sched.h:610:43: error: bad integer constant expression > ./include/linux/sched.h:610:67: error: invalid named zero-width bitfield > `bucket_id' > CC [M] net/can/af_can.o The sched.h errors suppress Sparse warnings so it's broken/useless now. The code looks like this: include/linux/sched.h 613 struct uclamp_se { 614 unsigned int value : bits_per(SCHED_CAPACITY_SCALE); 615 unsigned int bucket_id : bits_per(UCLAMP_BUCKETS); 616 unsigned int active : 1; 617 unsigned int user_defined : 1; 618 }; bits_per() is zero and Sparse doesn't like zero sized bitfields. regards, dan carpenter ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: can: Fix compiling warning 2019-08-07 10:50 ` [PATCH net-next] net: can: Fix compiling warning Dan Carpenter @ 2019-08-12 17:19 ` Kees Cook 2019-08-13 12:48 ` Dan Carpenter 0 siblings, 1 reply; 3+ messages in thread From: Kees Cook @ 2019-08-12 17:19 UTC (permalink / raw) To: Dan Carpenter Cc: Oliver Hartkopp, Patrick Bellasi, linux-sparse, Mao Wenan, davem, netdev, linux-kernel, kernel-janitors, Ingo Molnar On Wed, Aug 07, 2019 at 01:50:42PM +0300, Dan Carpenter wrote: > On Tue, Aug 06, 2019 at 06:41:44PM +0200, Oliver Hartkopp wrote: > > I compiled the code (the original version), but I do not get that "Should it > > be static?" warning: > > > > user@box:~/net-next$ make C=1 > > CALL scripts/checksyscalls.sh > > CALL scripts/atomic/check-atomics.sh > > DESCEND objtool > > CHK include/generated/compile.h > > CHECK net/can/af_can.c > > ./include/linux/sched.h:609:43: error: bad integer constant expression > > ./include/linux/sched.h:609:73: error: invalid named zero-width bitfield > > `value' > > ./include/linux/sched.h:610:43: error: bad integer constant expression > > ./include/linux/sched.h:610:67: error: invalid named zero-width bitfield > > `bucket_id' > > CC [M] net/can/af_can.o > > The sched.h errors suppress Sparse warnings so it's broken/useless now. > The code looks like this: > > include/linux/sched.h > 613 struct uclamp_se { > 614 unsigned int value : bits_per(SCHED_CAPACITY_SCALE); > 615 unsigned int bucket_id : bits_per(UCLAMP_BUCKETS); > 616 unsigned int active : 1; > 617 unsigned int user_defined : 1; > 618 }; > > bits_per() is zero and Sparse doesn't like zero sized bitfields. I just noticed these sparse warnings too -- what's happening here? Are they _supposed_ to be 0-width fields? It doesn't look like it to me: CONFIG_UCLAMP_BUCKETS_COUNT=5 ... #define UCLAMP_BUCKETS CONFIG_UCLAMP_BUCKETS_COUNT ... unsigned int bucket_id : bits_per(UCLAMP_BUCKETS); I would expect this to be 3 bits wide. ... Looks like gcc agrees: struct uclamp_se { unsigned int value:11; /* 0: 0 4 */ unsigned int bucket_id:3; /* 0:11 4 */ ... So this is a sparse issue? -- Kees Cook ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] net: can: Fix compiling warning 2019-08-12 17:19 ` Kees Cook @ 2019-08-13 12:48 ` Dan Carpenter 0 siblings, 0 replies; 3+ messages in thread From: Dan Carpenter @ 2019-08-13 12:48 UTC (permalink / raw) To: Kees Cook, Nicolai Stange Cc: Oliver Hartkopp, Patrick Bellasi, linux-sparse, Mao Wenan, davem, netdev, linux-kernel, kernel-janitors, Ingo Molnar On Mon, Aug 12, 2019 at 10:19:27AM -0700, Kees Cook wrote: > On Wed, Aug 07, 2019 at 01:50:42PM +0300, Dan Carpenter wrote: > > On Tue, Aug 06, 2019 at 06:41:44PM +0200, Oliver Hartkopp wrote: > > > I compiled the code (the original version), but I do not get that "Should it > > > be static?" warning: > > > > > > user@box:~/net-next$ make C=1 > > > CALL scripts/checksyscalls.sh > > > CALL scripts/atomic/check-atomics.sh > > > DESCEND objtool > > > CHK include/generated/compile.h > > > CHECK net/can/af_can.c > > > ./include/linux/sched.h:609:43: error: bad integer constant expression > > > ./include/linux/sched.h:609:73: error: invalid named zero-width bitfield > > > `value' > > > ./include/linux/sched.h:610:43: error: bad integer constant expression > > > ./include/linux/sched.h:610:67: error: invalid named zero-width bitfield > > > `bucket_id' > > > CC [M] net/can/af_can.o > > > > The sched.h errors suppress Sparse warnings so it's broken/useless now. > > The code looks like this: > > > > include/linux/sched.h > > 613 struct uclamp_se { > > 614 unsigned int value : bits_per(SCHED_CAPACITY_SCALE); > > 615 unsigned int bucket_id : bits_per(UCLAMP_BUCKETS); > > 616 unsigned int active : 1; > > 617 unsigned int user_defined : 1; > > 618 }; > > > > bits_per() is zero and Sparse doesn't like zero sized bitfields. > > I just noticed these sparse warnings too -- what's happening here? Are > they _supposed_ to be 0-width fields? It doesn't look like it to me: I'm sorr, I don't even know what code I was looking at before. I think my cscope database was stale? You're right. Sparse doesn't think it's zero, it knows that it is 11 and 3. What's happening is that it's failing the test in in bad_integer_constant_expression(): if (!(expr->flags & CEF_ICE)) The ICE in CEF_ICE stands for Integer Constant Expression. The rule here is that enums are not constant expressions in c99. See the explanation in commit 274c154704db ("constexpr: introduce additional expression constness tracking flags"). I don't think the CEF_ICE is set properly in evaluate_conditional_expression(). If conditional is constant and it's true and the ->cond_true expression is constant then the result should be constant as well. It shouldn't matter if the cond_false is constant. But instead it is ANDing all three sub expressions: expr->flags = (expr->conditional->flags & (*true)->flags & expr->cond_false->flags & ~CEF_CONST_MASK); Or actually in this case it's doing: if (expr->conditional->flags & (CEF_ACE | CEF_ADDR)) expr->flags = (*true)->flags & expr->cond_false->flags & ~CEF_CONST_MASK; But it's the same problem because it's should ignore cond_false. regards, dan carpenter ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-13 12:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20190802033643.84243-1-maowenan@huawei.com>
[not found] ` <0050efdb-af9f-49b9-8d83-f574b3d46a2e@hartkopp.net>
[not found] ` <20190806135231.GJ1974@kadam>
[not found] ` <6e1c5aa0-8ed3-eec3-a34d-867ea8f54e9d@hartkopp.net>
2019-08-07 10:50 ` [PATCH net-next] net: can: Fix compiling warning Dan Carpenter
2019-08-12 17:19 ` Kees Cook
2019-08-13 12:48 ` Dan Carpenter
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).