All of lore.kernel.org
 help / color / mirror / Atom feed
* sparse v0.6.4
@ 2021-09-06 18:53 Ramsay Jones
  2021-09-07  6:14 ` Luc Van Oostenryck
  2022-01-15  4:25 ` Randy Dunlap
  0 siblings, 2 replies; 6+ messages in thread
From: Ramsay Jones @ 2021-09-06 18:53 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: Sparse Mailing-list, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]


Hi Luc,

I have tested the new release, without issue, in the normal way on the
usual platforms (32- & 64-bit Linux, 64-bit cygwin).

[Have you posted to the sparse mailing-list yet? I think my subscription
has lapsed or something! I recently had to re-subscribe to the git
mailing-list as well. :( ]

Sorry for being tardy, but about 3 months ago a sparse issue came up on
the git mailing-list (see [1]). Take a look at this:

  $ cat -n junk.c
       1	
       2	static void func(int x)
       3	{
       4		switch (x) {
       5			default:
       6		}
       7	}
  $ sparse junk.c
  junk.c:6:9: error: Expected ; at end of statement
  junk.c:6:9: error: got }
  junk.c:8:0: error: Expected } at end of compound statement
  junk.c:8:0: error: got end-of-input
  junk.c:8:0: error: Expected } at end of function
  junk.c:8:0: error: got end-of-input
  $ 

Note: a case label that doesn't label a statement is not valid C, so what
is the problem? Well, for me, the implication of the email exchange was
that gcc seems to accept it without problem, except (with gcc 9.3.0):

  $ gcc junk.c
  junk.c: In function ‘func’:
  junk.c:5:3: error: label at end of compound statement
      5 |   default:
        |   ^~~~~~~
  $ 

... it doesn't for me!

So, I decided just to improve the error message issued by sparse. However,
that caused a moment of deja vu for me - hadn't you already fixed this
same issue? Having found your commit 0d6bb7e1 ("handle more graciously
labels with no statement", 2020-10-26), I realized that your fix only applied
for regular labels. The attached patch was the result of extending your
solution to case labels, like so:

  $ ./sparse junk.c
  junk.c:6:9: warning: statement expected after case label
  $ 

Note, just like your earlier commit, this issues a warning, rather than an
error (which it should probably be). I wrote this patch back in June, and
then forgot about it. :( [well, it was only lightly tested (testsuite and
one run over git), no tests, no commit message and it should probably be
an error!]

About a month ago, I noticed that gcc 11.2 had been released and the
release notes mentioned "Labels may appear before declarations and at the
end of a compound statement."  This, in turn, caused me to check my
current draft C2x document (dated December 11, 2020), which includes the
following:'N2508 Free Positioning of Labels Inside Compound Statements'.

It just so happens that, last night, I updated my cygwin installation and
the version of gcc went from 10.2 to 11.2. I think you can probably guess
what comes next:

  $ gcc -c junk.c
  $

  $ gcc -c -pedantic junk.c
  junk.c: In function ‘func’:
  junk.c:5:17: warning: label at end of compound statement [-Wpedantic]
      5 |                 default:
        |                 ^~~~~~~
  $ 

[Note: I tried several -std=cxx, but none of them even warned without
-pedantic]

So, for now, the standard does not allow these constructs, but the next
standard (whenever that will be) will.

ATB,
Ramsay Jones

[1] https://lore.kernel.org/git/xmqqr1hlqd5v.fsf@gitster.g/

[-- Attachment #2: 0001-parse-warn-about-a-case-label-on-empty-statement.patch --]
[-- Type: text/x-patch, Size: 890 bytes --]

From 39f65942a4047a391ca8323ae109ae3c1fe14bd9 Mon Sep 17 00:00:00 2001
From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Mon, 6 Sep 2021 19:00:57 +0100
Subject: [PATCH] parse: warn about a 'case label' on empty statement

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 parse.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/parse.c b/parse.c
index bc1c0602..9f2a3cdf 100644
--- a/parse.c
+++ b/parse.c
@@ -2329,6 +2329,11 @@ static inline struct token *case_statement(struct token *token, struct statement
 	stmt->type = STMT_CASE;
 	token = expect(token, ':', "after default/case");
 	add_case_statement(stmt);
+	if (match_op(token, '}')) {
+		warning(token->pos, "statement expected after case label");
+		stmt->case_statement = alloc_statement(token->pos, STMT_NONE);
+		return token;
+	}
 	return statement(token, &stmt->case_statement);
 }
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: sparse v0.6.4
  2021-09-06 18:53 sparse v0.6.4 Ramsay Jones
@ 2021-09-07  6:14 ` Luc Van Oostenryck
  2022-01-15  4:25 ` Randy Dunlap
  1 sibling, 0 replies; 6+ messages in thread
From: Luc Van Oostenryck @ 2021-09-07  6:14 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Sparse Mailing-list, Junio C Hamano

On Mon, Sep 06, 2021 at 07:53:19PM +0100, Ramsay Jones wrote:
> 
> Hi Luc,
> 
> I have tested the new release, without issue, in the normal way on the
> usual platforms (32- & 64-bit Linux, 64-bit cygwin).

Nice. Thank you.
 
> [Have you posted to the sparse mailing-list yet? I think my subscription
> has lapsed or something! I recently had to re-subscribe to the git
> mailing-list as well. :( ]

Yes, it was posted on the ML (archived as
https://lore.kernel.org/linux-sparse/20210906042111.lhoq7egtpmw3jcv6@mail/ )
I used to be unsubscribed from various kernel.org mailing list relatively
often (but I think I haven't been the last 2 years or so, at least from the
sparse ML).

> 
> Sorry for being tardy, but about 3 months ago a sparse issue came up on
> the git mailing-list (see [1]). Take a look at this:
> 
>   $ cat -n junk.c
>        1	
>        2	static void func(int x)
>        3	{
>        4		switch (x) {
>        5			default:
>        6		}
>        7	}
>   $ sparse junk.c
>   junk.c:6:9: error: Expected ; at end of statement
>   junk.c:6:9: error: got }
>   junk.c:8:0: error: Expected } at end of compound statement
>   junk.c:8:0: error: got end-of-input
>   junk.c:8:0: error: Expected } at end of function
>   junk.c:8:0: error: got end-of-input
>   $ 
> 
> Note: a case label that doesn't label a statement is not valid C, so what
> is the problem? Well, for me, the implication of the email exchange was
> that gcc seems to accept it without problem, except (with gcc 9.3.0):
> 
>   $ gcc junk.c
>   junk.c: In function ‘func’:
>   junk.c:5:3: error: label at end of compound statement
>       5 |   default:
>         |   ^~~~~~~
>   $ 
> 
> ... it doesn't for me!
> 
> So, I decided just to improve the error message issued by sparse. However,
> that caused a moment of deja vu for me - hadn't you already fixed this
> same issue? Having found your commit 0d6bb7e1 ("handle more graciously
> labels with no statement", 2020-10-26), I realized that your fix only applied
> for regular labels.

I vaguely remember there was some complications for switch labels but I can't
remember the details.

> The attached patch was the result of extending your
> solution to case labels, like so:
> 
>   $ ./sparse junk.c
>   junk.c:6:9: warning: statement expected after case label
>   $ 
> 
> Note, just like your earlier commit, this issues a warning, rather than an
> error (which it should probably be).

I tend more and more to use errors only for the cases where Sparse can't
handle the following correctly anymore.

> I wrote this patch back in June, and
> then forgot about it. :( [well, it was only lightly tested (testsuite and
> one run over git), no tests, no commit message and it should probably be
> an error!]
> 
> About a month ago, I noticed that gcc 11.2 had been released and the
> release notes mentioned "Labels may appear before declarations and at the
> end of a compound statement."  This, in turn, caused me to check my
> current draft C2x document (dated December 11, 2020), which includes the
> following:'N2508 Free Positioning of Labels Inside Compound Statements'.
> 
> It just so happens that, last night, I updated my cygwin installation and
> the version of gcc went from 10.2 to 11.2. I think you can probably guess
> what comes next:
> 
>   $ gcc -c junk.c
>   $
> 
>   $ gcc -c -pedantic junk.c
>   junk.c: In function ‘func’:
>   junk.c:5:17: warning: label at end of compound statement [-Wpedantic]
>       5 |                 default:
>         |                 ^~~~~~~
>   $ 
> 
> [Note: I tried several -std=cxx, but none of them even warned without
> -pedantic]
> 
> So, for now, the standard does not allow these constructs, but the next
> standard (whenever that will be) will.

Yes, I'm aware of this but IIRC the motivation was related to the mixing
of statements and declarations. The next standard is planned for 2023,
meanwhile I don't have a strong opinion about this error/warning (but
I'm very fine making things coherent between switch and regular labels).

Best regards,
-- Luc

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sparse v0.6.4
  2021-09-06 18:53 sparse v0.6.4 Ramsay Jones
  2021-09-07  6:14 ` Luc Van Oostenryck
@ 2022-01-15  4:25 ` Randy Dunlap
  2022-05-21 13:54   ` Luc Van Oostenryck
  2022-06-14  0:17   ` [PATCH] predefine __ATOMIC_ACQUIRE & friends as weak Luc Van Oostenryck
  1 sibling, 2 replies; 6+ messages in thread
From: Randy Dunlap @ 2022-01-15  4:25 UTC (permalink / raw)
  To: Ramsay Jones, Luc Van Oostenryck; +Cc: Sparse Mailing-list

Hi Luc,

On 9/6/21 11:53, Ramsay Jones wrote:
> I have tested the new release, without issue, in the normal way on the
> usual platforms (32- & 64-bit Linux, 64-bit cygwin).
> 
> [Have you posted to the sparse mailing-list yet? I think my subscription
> has lapsed or something! I recently had to re-subscribe to the git
> mailing-list as well. :( ]

Odd, I didn't receive it either. No big deal, but I am just looking
at a kernel build issue when using sparse v0.6.4.

It seems that $subject version adds builtins for ATOMIC_ACQUIRE and
ATOMIC_RELEASE, but GCC has those too, so there are a few thousand
(OK, I didn't count them - I killed it quickly) of these: (e.g.)


..  CHECK   ../init/do_mounts_initrd.c
.command-line: note: in included file:
builtin:1:9: sparse: warning: preprocessor token __ATOMIC_ACQUIRE redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: warning: preprocessor token __ATOMIC_SEQ_CST redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: warning: preprocessor token __ATOMIC_ACQ_REL redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: warning: preprocessor token __ATOMIC_RELEASE redefined
builtin:0:0: sparse: this was the original definition

Any suggestions for how to avoid these warnings?

thanks.
-- 
~Randy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sparse v0.6.4
  2022-01-15  4:25 ` Randy Dunlap
@ 2022-05-21 13:54   ` Luc Van Oostenryck
  2022-05-21 15:51     ` Randy Dunlap
  2022-06-14  0:17   ` [PATCH] predefine __ATOMIC_ACQUIRE & friends as weak Luc Van Oostenryck
  1 sibling, 1 reply; 6+ messages in thread
From: Luc Van Oostenryck @ 2022-05-21 13:54 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: Ramsay Jones, Sparse Mailing-list

On Fri, Jan 14, 2022 at 08:25:31PM -0800, Randy Dunlap wrote:
> Hi Luc,

Hi Randy,

Apologies for this reply delayed for much too long.
 
> It seems that $subject version adds builtins for ATOMIC_ACQUIRE and
> ATOMIC_RELEASE, but GCC has those too, so there are a few thousand
> (OK, I didn't count them - I killed it quickly) of these: (e.g.)

Yes indeed, it was one of the things added in this release.
 
> ..  CHECK   ../init/do_mounts_initrd.c
> .command-line: note: in included file:
> builtin:1:9: sparse: warning: preprocessor token __ATOMIC_ACQUIRE redefined
> builtin:0:0: sparse: this was the original definition
> builtin:1:9: sparse: warning: preprocessor token __ATOMIC_SEQ_CST redefined
> builtin:0:0: sparse: this was the original definition
> builtin:1:9: sparse: warning: preprocessor token __ATOMIC_ACQ_REL redefined
> builtin:0:0: sparse: this was the original definition
> builtin:1:9: sparse: warning: preprocessor token __ATOMIC_RELEASE redefined
> builtin:0:0: sparse: this was the original definition
> 
> Any suggestions for how to avoid these warnings?

Do you still see these? If yes, can you tell a bit about your setup and
the exact command line used?

I'm asking because I never saw a problem with this, same for the test bots.
It should be exactly the same as for any other predefined value.
However, as predefined, they should all be reported from 'builtin:0:0:'
and the 'builtin:1:9:' look as if those are from a '-include <somefile.h>'
on the command line.

Best regards,
-- Luc

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sparse v0.6.4
  2022-05-21 13:54   ` Luc Van Oostenryck
@ 2022-05-21 15:51     ` Randy Dunlap
  0 siblings, 0 replies; 6+ messages in thread
From: Randy Dunlap @ 2022-05-21 15:51 UTC (permalink / raw)
  To: Luc Van Oostenryck; +Cc: Ramsay Jones, Sparse Mailing-list



On 5/21/22 06:54, Luc Van Oostenryck wrote:
> On Fri, Jan 14, 2022 at 08:25:31PM -0800, Randy Dunlap wrote:
>> Hi Luc,
> 
> Hi Randy,
> 
> Apologies for this reply delayed for much too long.

Hey Luc,

>  
>> It seems that $subject version adds builtins for ATOMIC_ACQUIRE and
>> ATOMIC_RELEASE, but GCC has those too, so there are a few thousand
>> (OK, I didn't count them - I killed it quickly) of these: (e.g.)
> 
> Yes indeed, it was one of the things added in this release.
>  
>> ..  CHECK   ../init/do_mounts_initrd.c
>> .command-line: note: in included file:
>> builtin:1:9: sparse: warning: preprocessor token __ATOMIC_ACQUIRE redefined
>> builtin:0:0: sparse: this was the original definition
>> builtin:1:9: sparse: warning: preprocessor token __ATOMIC_SEQ_CST redefined
>> builtin:0:0: sparse: this was the original definition
>> builtin:1:9: sparse: warning: preprocessor token __ATOMIC_ACQ_REL redefined
>> builtin:0:0: sparse: this was the original definition
>> builtin:1:9: sparse: warning: preprocessor token __ATOMIC_RELEASE redefined
>> builtin:0:0: sparse: this was the original definition
>>
>> Any suggestions for how to avoid these warnings?
> 
> Do you still see these? If yes, can you tell a bit about your setup and
> the exact command line used?

No, I no longer see these warnings. :)

> I'm asking because I never saw a problem with this, same for the test bots.
> It should be exactly the same as for any other predefined value.
> However, as predefined, they should all be reported from 'builtin:0:0:'
> and the 'builtin:1:9:' look as if those are from a '-include <somefile.h>'
> on the command line.
> 
> Best regards,
> -- Luc

Thanks.

-- 
~Randy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] predefine __ATOMIC_ACQUIRE & friends as weak
  2022-01-15  4:25 ` Randy Dunlap
  2022-05-21 13:54   ` Luc Van Oostenryck
@ 2022-06-14  0:17   ` Luc Van Oostenryck
  1 sibling, 0 replies; 6+ messages in thread
From: Luc Van Oostenryck @ 2022-06-14  0:17 UTC (permalink / raw)
  To: linux-sparse; +Cc: Luc Van Oostenryck, Randy Dunlap, kernel test robot

From: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

In kernel's arch/mips/Makefile the whole content of gcc's -dM is used
for CHECKFLAGS. This conflict with some macros also defined internally:
	builtin:1:9: warning: preprocessor token __ATOMIC_ACQUIRE redefined
	builtin:0:0: this was the original definition

Fix this by using a weak define for these macros.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 predefine.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/predefine.c b/predefine.c
index 98e38a04579d..5b0f0caf4c35 100644
--- a/predefine.c
+++ b/predefine.c
@@ -179,12 +179,12 @@ void predefined_macros(void)
 	if (arch_target->has_int128)
 		predefined_sizeof("INT128", "", 128);
 
-	predefine("__ATOMIC_RELAXED", 0, "0");
-	predefine("__ATOMIC_CONSUME", 0, "1");
-	predefine("__ATOMIC_ACQUIRE", 0, "3");
-	predefine("__ATOMIC_RELEASE", 0, "4");
-	predefine("__ATOMIC_ACQ_REL", 0, "7");
-	predefine("__ATOMIC_SEQ_CST", 0, "8");
+	predefine("__ATOMIC_RELAXED", 1, "0");
+	predefine("__ATOMIC_CONSUME", 1, "1");
+	predefine("__ATOMIC_ACQUIRE", 1, "3");
+	predefine("__ATOMIC_RELEASE", 1, "4");
+	predefine("__ATOMIC_ACQ_REL", 1, "7");
+	predefine("__ATOMIC_SEQ_CST", 1, "8");
 
 	predefine("__ORDER_LITTLE_ENDIAN__", 1, "1234");
 	predefine("__ORDER_BIG_ENDIAN__", 1, "4321");
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-06-14  0:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-06 18:53 sparse v0.6.4 Ramsay Jones
2021-09-07  6:14 ` Luc Van Oostenryck
2022-01-15  4:25 ` Randy Dunlap
2022-05-21 13:54   ` Luc Van Oostenryck
2022-05-21 15:51     ` Randy Dunlap
2022-06-14  0:17   ` [PATCH] predefine __ATOMIC_ACQUIRE & friends as weak Luc Van Oostenryck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.