sparse: new feature " multiple initializers" has false positives on MODULE_ALIAS

PATCH In review Christian Borntraeger 5 participants 20 messages Jan 22, 2015 – Feb 12, 2015

Subsystems (2)

Documentation Sparse checker

lore

Share & tools

lore

Christian Borntraeger

author

2015-01-22 20:31 UTC #0 lore raw

Linus, Christopher,

Commit 0f25c6a78e08fdc15af5e599d836fa24349c042f ("Add warning about duplicate initializers") has a false positive on arch/s390/kvm/kvm-s390.c

CHECK arch/s390/kvm/kvm-s390.c
arch/s390/kvm/kvm-s390.c:1823:1: error: symbol '__UNIQUE_ID_alias__COUNTER__' has multiple initializers (originally initialized at arch/s390/kvm/kvm-s390.c:1822)

1822: MODULE_ALIAS_MISCDEV(KVM_MINOR);
1823: MODULE_ALIAS("devname:kvm");

Preprocessing with gcc gives
static const char __UNIQUE_ID_alias0[] __attribute__((__used__)) __attribute__((section(".modinfo"), unused, aligned(1))) = "alias" "=" "char-major-" "10" "-" "232"; static const char __UNIQUE_ID_alias1[] __attribute__((__used__)) __attribute__((section(".modinfo"), unused, aligned(1))) = "alias" "=" "devname:kvm";

so alias0 and alias1 instead of __COUNTER__.
I never heard of __COUNTER__ before, so I guess its some gcc magic that sparse should mimic..

Christian

Christopher Li

maintainer

2015-01-23 16:40 UTC #1 lore raw

Quoted 6 lines · Christian Borntraeger

Linus, Christopher,
Commit 0f25c6a78e08fdc15af5e599d836fa24349c042f ("Add warning about duplicate initializers") has a false positive on arch/s390/kvm/kvm-s390.c
CHECK arch/s390/kvm/kvm-s390.c
arch/s390/kvm/kvm-s390.c:1823:1: error: symbol '__UNIQUE_ID_alias__COUNTER__' has multiple initializers (originally initialized at arch/s390/kvm/kvm-s390.c:1822)

Search the "__COUNTER__" macro shows that:
https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html

__COUNTER__This macro expands to sequential integral values starting from 0. In conjunction with the ## operator, this provides a convenient means to generate unique identifiers. Care must be taken to ensure that __COUNTER__ is not expanded prior to inclusion of precompiled headers which use it. Otherwise, the precompiled headers will not be used.

I think sparse haven't implement the __COUNTER__ macro. That is why it emit the error on duplicate entry.

Chris

Luc Van Oostenryck

2015-01-23 22:23 UTC #2 lore raw

PATCH 1/1 Teach sparse about the __COUNTER__ predefined macro

Quoted 19 lines · Christopher Li

Quoted 6 lines · Christian Borntraeger
Linus, Christopher,
Commit 0f25c6a78e08fdc15af5e599d836fa24349c042f ("Add warning about duplicate initializers") has a false positive on arch/s390/kvm/kvm-s390.c
CHECK arch/s390/kvm/kvm-s390.c
arch/s390/kvm/kvm-s390.c:1823:1: error: symbol '__UNIQUE_ID_alias__COUNTER__' has multiple initializers (originally initialized at arch/s390/kvm/kvm-s390.c:1822)
Search the "__COUNTER__" macro shows that:
https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
__COUNTER__This macro expands to sequential integral values starting from 0. In conjunction with the ## operator, this provides a convenient means to generate unique identifiers. Care must be taken to ensure that __COUNTER__ is not expanded prior to inclusion of precompiled headers which use it. Otherwise, the precompiled headers will not be used.

Quoted 5 lines

I think sparse haven't implement the __COUNTER__ macro. That is why it emit the error on duplicate entry.
Chris
--

The following patch should fix that.

Luc

Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.

Signed-off-by Luc Van Oostenryck <[email protected]>

---
 ident-list.h                          |  1 +
 pre-process.c                         |  4 ++++
 validation/preprocessor/__COUNTER__.c | 12 ++++++++++++
 3 files changed, 17 insertions(+)
 create mode 100644 validation/preprocessor/__COUNTER__.c

ident-list.h +1 -0

index d5a145f8..b65b667d 100644
@@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
 __IDENT(__func___ident, "__func__", 0);
 __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
 __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
+__IDENT(__COUNTER___ident, "__COUNTER__", 0);
 
 /* Sparse commands */
 IDENT_RESERVED(__context__);

pre-process.c +4 -0

index 1aa3d2c4..316247ac 100644
@@ -181,6 +181,10 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		static int counter;
+
+		replace_with_integer(token, counter++);
 	}
 	return 1;
 }

validation/preprocessor/__COUNTER__.c new file +12 -0

new file mode 100644
index 00000000..98187ee6
@@ -0,0 +1,12 @@
+__COUNTER__
+__COUNTER__
+/*
+ * check-name: __COUNTER__ #1
+ * check-command: sparse -E $file
+ *
+ * check-output-start
+
+0
+1
+ * check-output-end
+ */

Sam Ravnborg

Acked-by on v1 1/1

2015-01-23 22:33 UTC #3 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Quoted 20 lines · Luc Van Oostenryck

Quoted 19 lines · Christopher Li
Quoted 6 lines · Christian Borntraeger
Linus, Christopher,
Commit 0f25c6a78e08fdc15af5e599d836fa24349c042f ("Add warning about duplicate initializers") has a false positive on arch/s390/kvm/kvm-s390.c
CHECK arch/s390/kvm/kvm-s390.c
arch/s390/kvm/kvm-s390.c:1823:1: error: symbol '__UNIQUE_ID_alias__COUNTER__' has multiple initializers (originally initialized at arch/s390/kvm/kvm-s390.c:1822)
Search the "__COUNTER__" macro shows that:
https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
__COUNTER__This macro expands to sequential integral values starting from 0. In conjunction with the ## operator, this provides a convenient means to generate unique identifiers. Care must be taken to ensure that __COUNTER__ is not expanded prior to inclusion of precompiled headers which use it. Otherwise, the precompiled headers will not be used.

Quoted 8 lines

Quoted 5 lines
I think sparse haven't implement the __COUNTER__ macro. That is why it emit the error on duplicate entry.
Chris
--
The following patch should fix that.

Seems we were working on this in parallel :-)

Quoted 8 lines

Luc
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
Signed-off-by Luc Van Oostenryck <[email protected]>

Acked-by Sam Ravnborg <[email protected]>

Sam

[email protected]

2015-01-23 22:39 UTC #4 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

In reply to Luc Van Oostenryck

Quoted 5 lines · Luc Van Oostenryck

Quoted 2 lines · Christopher Li
I think sparse haven't implement the __COUNTER__ macro. That is why it emit the error on duplicate entry.
The following patch should fix that.

[...]

Quoted 3 lines

Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
Signed-off-by Luc Van Oostenryck <[email protected]>

One issue below.

pre-process.c static int expand_one_symbol(struct token **list) · line 187

Quoted 10 lines

pre-process.c +4 -0

@@ -181,6 +181,10 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		static int counter;
+
+		replace_with_integer(token, counter++);

This should not use a static counter. GCC and Sparse can run over more than one file in one invocation:

$ head test1.c test2.c
==> test1.c <==
__COUNTER__
__COUNTER__

==> test2.c <==
__COUNTER__
__COUNTER__

$ gcc -E test1.c test2.c
# 1 "test1.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test1.c"
0
1
# 1 "test2.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test2.c"
0
1

Notice that the second file starts with __COUNTER__ at 0 again.

The counter *should* keep counting through include files, but needs to reset before starting a new top-level compile.

- Josh Triplett

Luc Van Oostenryck

2015-01-23 23:59 UTC #5 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Quoted 54 lines · On Fri, Jan 23, 2015 at 02:38:56PM -0800, [email protected] wrote:

Quoted 5 lines · Luc Van Oostenryck
Quoted 2 lines · Christopher Li
I think sparse haven't implement the __COUNTER__ macro. That is why it emit the error on duplicate entry.
The following patch should fix that.
[...]
Quoted 3 lines
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
Signed-off-by Luc Van Oostenryck <[email protected]>
One issue below.
pre-process.c static int expand_one_symbol(struct token **list) · line 187
Quoted 10 lines
pre-process.c +4 -0
@@ -181,6 +181,10 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		static int counter;
+
+		replace_with_integer(token, counter++);
This should not use a static counter. GCC and Sparse can run over more than one file in one invocation:
$ head test1.c test2.c
==> test1.c <==
__COUNTER__
__COUNTER__
==> test2.c <==
__COUNTER__
__COUNTER__
$ gcc -E test1.c test2.c
# 1 "test1.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test1.c"
0
1
# 1 "test2.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "test2.c"
0
1

Quoted 7 lines

Notice that the second file starts with __COUNTER__ at 0 again.
The counter *should* keep counting through include files, but needs to reset before starting a new top-level compile.
- Josh Triplett
--

Yes, indeed.
Thanks for bringing to my attention.

Here is a new version of the patch taking care of that.

Luc

---
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.

This macro expands to sequential integral values starting from 0, and this for each top-level source file.

Signed-off-by Luc Van Oostenryck <[email protected]>

---
 ident-list.h                       |  1 +
 pre-process.c                      |  4 ++++
 validation/preprocessor/counter1.c | 12 ++++++++++++
 validation/preprocessor/counter2.c | 14 ++++++++++++++
 validation/preprocessor/counter2.h |  1 +
 validation/preprocessor/counter3.c | 13 +++++++++++++
 6 files changed, 45 insertions(+)
 create mode 100644 validation/preprocessor/counter1.c
 create mode 100644 validation/preprocessor/counter2.c
 create mode 100644 validation/preprocessor/counter2.h
 create mode 100644 validation/preprocessor/counter3.c

ident-list.h +1 -0

index d5a145f8..b65b667d 100644
@@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
 __IDENT(__func___ident, "__func__", 0);
 __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
 __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
+__IDENT(__COUNTER___ident, "__COUNTER__", 0);
 
 /* Sparse commands */
 IDENT_RESERVED(__context__);

pre-process.c +4 -0

index 1aa3d2c4..601e0f26 100644
@@ -45,6 +45,7 @@
 #include "scope.h"
 
 static int false_nesting = 0;
+static int counter_macro;		// __COUNTER__ expansion
 
 #define INCLUDEPATHS 300
 const char *includepath[INCLUDEPATHS+1] = {

@@ -181,6 +182,8 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		replace_with_integer(token, counter_macro++);
 	}
 	return 1;
 }

@@ -1882,6 +1885,7 @@ static void init_preprocessor(void)
 		sym->normal = 0;
 	}
 
+	counter_macro = 0;
 }
 
 static void handle_preprocessor_line(struct stream *stream, struct token **line, struct token *start)

validation/preprocessor/counter1.c new file +12 -0

new file mode 100644
index 00000000..98187ee6
@@ -0,0 +1,12 @@
+__COUNTER__
+__COUNTER__
+/*
+ * check-name: __COUNTER__ #1
+ * check-command: sparse -E $file
+ *
+ * check-output-start
+
+0
+1
+ * check-output-end
+ */

validation/preprocessor/counter2.c new file +14 -0

new file mode 100644
index 00000000..9883b682
@@ -0,0 +1,14 @@
+__FILE__ __COUNTER__
+#include <counter2.h>
+__FILE__ __COUNTER__
+/*
+ * check-name: __COUNTER__ #2
+ * check-command: sparse -Ipreprocessor -E $file
+ *
+ * check-output-start
+
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */

validation/preprocessor/counter2.h new file +1 -0

new file mode 100644
index 00000000..447b70ab
@@ -0,0 +1 @@
+__FILE__ __COUNTER__

validation/preprocessor/counter3.c new file +13 -0

new file mode 100644
index 00000000..1449b2d1
@@ -0,0 +1,13 @@
+/*
+ * check-name: __COUNTER__ #3
+ * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
+ *
+ * check-output-start
+
+0
+1
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */

Josh Triplett

Reviewed-by

2015-01-24 01:30 UTC #6 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Quoted 6 lines · Luc Van Oostenryck

Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
This macro expands to sequential integral values starting from 0, and this for each top-level source file.
Signed-off-by Luc Van Oostenryck <[email protected]>

counter3.c seems like a bit of an abuse of the test suite framework, but I don't have a better suggestion.

Reviewed-by Josh Triplett <[email protected]>

Quoted 121 lines

 ident-list.h                       |  1 +
 pre-process.c                      |  4 ++++
 validation/preprocessor/counter1.c | 12 ++++++++++++
 validation/preprocessor/counter2.c | 14 ++++++++++++++
 validation/preprocessor/counter2.h |  1 +
 validation/preprocessor/counter3.c | 13 +++++++++++++
 6 files changed, 45 insertions(+)
 create mode 100644 validation/preprocessor/counter1.c
 create mode 100644 validation/preprocessor/counter2.c
 create mode 100644 validation/preprocessor/counter2.h
 create mode 100644 validation/preprocessor/counter3.c

ident-list.h +1 -0

index d5a145f8..b65b667d 100644
@@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
 __IDENT(__func___ident, "__func__", 0);
 __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
 __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
+__IDENT(__COUNTER___ident, "__COUNTER__", 0);
 
 /* Sparse commands */
 IDENT_RESERVED(__context__);

pre-process.c +4 -0

index 1aa3d2c4..601e0f26 100644
@@ -45,6 +45,7 @@
 #include "scope.h"
 
 static int false_nesting = 0;
+static int counter_macro;		// __COUNTER__ expansion
 
 #define INCLUDEPATHS 300
 const char *includepath[INCLUDEPATHS+1] = {

@@ -181,6 +182,8 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		replace_with_integer(token, counter_macro++);
 	}
 	return 1;
 }

@@ -1882,6 +1885,7 @@ static void init_preprocessor(void)
 		sym->normal = 0;
 	}
 
+	counter_macro = 0;
 }
 
 static void handle_preprocessor_line(struct stream *stream, struct token **line, struct token *start)

validation/preprocessor/counter1.c new file +12 -0

new file mode 100644
index 00000000..98187ee6
@@ -0,0 +1,12 @@
+__COUNTER__
+__COUNTER__
+/*
+ * check-name: __COUNTER__ #1
+ * check-command: sparse -E $file
+ *
+ * check-output-start
+
+0
+1
+ * check-output-end
+ */

validation/preprocessor/counter2.c new file +14 -0

new file mode 100644
index 00000000..9883b682
@@ -0,0 +1,14 @@
+__FILE__ __COUNTER__
+#include <counter2.h>
+__FILE__ __COUNTER__
+/*
+ * check-name: __COUNTER__ #2
+ * check-command: sparse -Ipreprocessor -E $file
+ *
+ * check-output-start
+
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */

validation/preprocessor/counter2.h new file +1 -0

new file mode 100644
index 00000000..447b70ab
@@ -0,0 +1 @@
+__FILE__ __COUNTER__

validation/preprocessor/counter3.c new file +13 -1

new file mode 100644
index 00000000..1449b2d1
@@ -0,0 +1,13 @@
+/*
+ * check-name: __COUNTER__ #3
+ * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
+ *
+ * check-output-start
+
+0
+1
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */
--

To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Luc Van Oostenryck

2015-01-24 11:27 UTC #7 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Quoted 6 lines · Josh Triplett

Quoted 5 lines · Luc Van Oostenryck
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
This macro expands to sequential integral values starting from 0, and this for each top-level source file.

Quoted 6 lines

Quoted 1 line
Signed-off-by Luc Van Oostenryck <[email protected]>
counter3.c seems like a bit of an abuse of the test suite framework, but I don't have a better suggestion.
Reviewed-by Josh Triplett <[email protected]>

Yes, I know ...

Would the following change to the test-suite (introducing tags to separate input sections) and the corresponding test be more OK ?

Luc

validation/test-suite +9 -0

index df5a7c60..97d4dd40 100755
@@ -102,6 +102,15 @@ do_test()
 	fi
 	test_name=$last_result
 
+	# grab the input sections
+	input_nr=1
+	while grep -q "input-file-$input_nr-start" "$file"; do
+		sed -n "/input-file-$input_nr-start/,/input-file-$input_nr-end/p" "$file" \
+			| grep -v input-file > "$file".input$input_nr
+		eval "file$input_nr=$file.input$input_nr"
+		input_nr=$(($input_nr + 1))
+	done
+
 	# does the test provide a specific command ?
 	cmd=`eval echo $default_path/$default_cmd`
 	get_value "check-command" $file

Documentation/test-suite +7 -0

index 6c4f24f6..34b38696 100644
@@ -32,6 +32,10 @@ check-output-start / check-output-end (optional)
 check-known-to-fail (optional)
 	Mark the test as being known to fail.
 
+input-file-1-start / input-file-1-end, / input-file-2-start / ... (optional)
+	The input files of check-command lies between those tags.
+	It's only needed when the test needs several distincts input files.
+
 
 	Using test-suite
 	~~~~~~~~~~~~~~~~

@@ -58,6 +62,9 @@ name:
 cmd:
 	check-command value. If no cmd is provided, it defaults to
 	"sparse $file".
+	If the "input-file-1-start/..." tags are used those files are to be
+	referenced with "$file1", ... and the command need to be something like:
+	"sparse $file1 $file2"
 
 The output of the test-suite format command can be redirected into the
 test case to create a test-suite formated file.

validation/preprocessor/counter3.c +20 -0

index e69de29b..76635e82 100644
@@ -0,0 +1,20 @@
+/* input-file-1-start */
+__COUNTER__
+__COUNTER__
+/* input-file-1-end */
+
+/* input-file-2-start */
+__COUNTER__
+/* input-file-2-end */
+
+/*
+ * check-name: __COUNTER__ #3
+ * check-command: sparse -E $file1 $file2
+ *
+ * check-output-start
+
+0
+1
+0
+ * check-output-end
+ */

Josh Triplett

2015-01-24 20:19 UTC #8 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Quoted 12 lines · Luc Van Oostenryck

Quoted 11 lines · Josh Triplett
Quoted 6 lines · Luc Van Oostenryck
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
This macro expands to sequential integral values starting from 0, and this for each top-level source file.
Signed-off-by Luc Van Oostenryck <[email protected]>
counter3.c seems like a bit of an abuse of the test suite framework, but I don't have a better suggestion.

Quoted 6 lines

Quoted 1 line
Reviewed-by Josh Triplett <[email protected]>
Yes, I know ...
Would the following change to the test-suite (introducing tags to separate input sections) and the corresponding test be more OK ?

Interesting idea! That would also allow consolidating tests that require include files into a single test file, if it's possible to #include "$file1"; for instance, pragma-once.c could avoid recursing if the test fails.

I'll leave it to others to decide if this seems like a direction the test suite should expand to cover, or if for this single case counter3.c should just include other tests as your previous patch did.

- Josh Triplett

Luc Van Oostenryck

2015-01-24 20:39 UTC #9 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Quoted 25 lines · Josh Triplett

Quoted 12 lines · Luc Van Oostenryck
Quoted 11 lines · Josh Triplett
Quoted 6 lines · Luc Van Oostenryck
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
This macro expands to sequential integral values starting from 0, and this for each top-level source file.
Signed-off-by Luc Van Oostenryck <[email protected]>
counter3.c seems like a bit of an abuse of the test suite framework, but I don't have a better suggestion.
Quoted 6 lines
Quoted 1 line
Reviewed-by Josh Triplett <[email protected]>
Yes, I know ...
Would the following change to the test-suite (introducing tags to separate input sections) and the corresponding test be more OK ?
Interesting idea! That would also allow consolidating tests that require include files into a single test file, if it's possible to #include "$file1"; for instance, pragma-once.c could avoid recursing if the test fails.

Quoted 6 lines

I'll leave it to others to decide if this seems like a direction the test suite should expand to cover, or if for this single case counter3.c should just include other tests as your previous patch did.
- Josh Triplett
--

Yes, it's fairly easy to add. In fact ... yesterday I had a version that did that but I removed it because I found it a bit hackish and it was not needed.

Luc

Christian Borntraeger

author Tested-by

2015-01-25 20:12 UTC #10 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

In reply to Luc Van Oostenryck

Am 24.01.2015 um 00:59 schrieb Luc Van Oostenryck:
[...]

Quoted 8 lines

Yes, indeed.
Thanks for bringing to my attention.
Here is a new version of the patch taking care of that.
Luc

Quoted 7 lines

---
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.
This macro expands to sequential integral values starting from 0, and this for each top-level source file.
Signed-off-by Luc Van Oostenryck <[email protected]>

Tested-by Christian Borntraeger <[email protected]>

Quoted 119 lines

---
 ident-list.h                       |  1 +
 pre-process.c                      |  4 ++++
 validation/preprocessor/counter1.c | 12 ++++++++++++
 validation/preprocessor/counter2.c | 14 ++++++++++++++
 validation/preprocessor/counter2.h |  1 +
 validation/preprocessor/counter3.c | 13 +++++++++++++
 6 files changed, 45 insertions(+)
 create mode 100644 validation/preprocessor/counter1.c
 create mode 100644 validation/preprocessor/counter2.c
 create mode 100644 validation/preprocessor/counter2.h
 create mode 100644 validation/preprocessor/counter3.c

ident-list.h +1 -0

index d5a145f8..b65b667d 100644
@@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
 __IDENT(__func___ident, "__func__", 0);
 __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
 __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
+__IDENT(__COUNTER___ident, "__COUNTER__", 0);
 
 /* Sparse commands */
 IDENT_RESERVED(__context__);

pre-process.c +4 -0

index 1aa3d2c4..601e0f26 100644
@@ -45,6 +45,7 @@
 #include "scope.h"
 
 static int false_nesting = 0;
+static int counter_macro;		// __COUNTER__ expansion
 
 #define INCLUDEPATHS 300
 const char *includepath[INCLUDEPATHS+1] = {

@@ -181,6 +182,8 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		replace_with_integer(token, counter_macro++);
 	}
 	return 1;
 }

@@ -1882,6 +1885,7 @@ static void init_preprocessor(void)
 		sym->normal = 0;
 	}
 
+	counter_macro = 0;
 }
 
 static void handle_preprocessor_line(struct stream *stream, struct token **line, struct token *start)

validation/preprocessor/counter1.c new file +12 -0

new file mode 100644
index 00000000..98187ee6
@@ -0,0 +1,12 @@
+__COUNTER__
+__COUNTER__
+/*
+ * check-name: __COUNTER__ #1
+ * check-command: sparse -E $file
+ *
+ * check-output-start
+
+0
+1
+ * check-output-end
+ */

validation/preprocessor/counter2.c new file +14 -0

new file mode 100644
index 00000000..9883b682
@@ -0,0 +1,14 @@
+__FILE__ __COUNTER__
+#include <counter2.h>
+__FILE__ __COUNTER__
+/*
+ * check-name: __COUNTER__ #2
+ * check-command: sparse -Ipreprocessor -E $file
+ *
+ * check-output-start
+
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */

validation/preprocessor/counter2.h new file +1 -0

new file mode 100644
index 00000000..447b70ab
@@ -0,0 +1 @@
+__FILE__ __COUNTER__

validation/preprocessor/counter3.c new file +13 -0

new file mode 100644
index 00000000..1449b2d1
@@ -0,0 +1,13 @@
+/*
+ * check-name: __COUNTER__ #3
+ * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
+ *
+ * check-output-start
+
+0
+1
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */

Christian Borntraeger

author

2015-01-28 20:39 UTC #11 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Am 25.01.2015 um 21:12 schrieb Christian Borntraeger:

Quoted 13 lines

Am 24.01.2015 um 00:59 schrieb Luc Van Oostenryck:
[...]
Quoted 11 lines
Yes, indeed.
Thanks for bringing to my attention.
Here is a new version of the patch taking care of that.
Luc
---
Subject: [PATCH] Teach sparse about the __COUNTER__ predefined macro.

Quoted 6 lines

Quoted 4 lines
This macro expands to sequential integral values starting from 0, and this for each top-level source file.
Signed-off-by Luc Van Oostenryck <[email protected]>
Tested-by Christian Borntraeger <[email protected]>

Christoper,
can you apply that fix as well?

Thans

Quoted 121 lines

Quoted 120 lines

---
 ident-list.h                       |  1 +
 pre-process.c                      |  4 ++++
 validation/preprocessor/counter1.c | 12 ++++++++++++
 validation/preprocessor/counter2.c | 14 ++++++++++++++
 validation/preprocessor/counter2.h |  1 +
 validation/preprocessor/counter3.c | 13 +++++++++++++
 6 files changed, 45 insertions(+)
 create mode 100644 validation/preprocessor/counter1.c
 create mode 100644 validation/preprocessor/counter2.c
 create mode 100644 validation/preprocessor/counter2.h
 create mode 100644 validation/preprocessor/counter3.c

ident-list.h +1 -0

index d5a145f8..b65b667d 100644
@@ -108,6 +108,7 @@ __IDENT(__TIME___ident, "__TIME__", 0);
 __IDENT(__func___ident, "__func__", 0);
 __IDENT(__FUNCTION___ident, "__FUNCTION__", 0);
 __IDENT(__PRETTY_FUNCTION___ident, "__PRETTY_FUNCTION__", 0);
+__IDENT(__COUNTER___ident, "__COUNTER__", 0);
 
 /* Sparse commands */
 IDENT_RESERVED(__context__);

pre-process.c +4 -0

index 1aa3d2c4..601e0f26 100644
@@ -45,6 +45,7 @@
 #include "scope.h"
 
 static int false_nesting = 0;
+static int counter_macro;		// __COUNTER__ expansion
 
 #define INCLUDEPATHS 300
 const char *includepath[INCLUDEPATHS+1] = {

@@ -181,6 +182,8 @@ static int expand_one_symbol(struct token **list)
 			time(&t);
 		strftime(buffer, 9, "%T", localtime(&t));
 		replace_with_string(token, buffer);
+	} else if (token->ident == &__COUNTER___ident) {
+		replace_with_integer(token, counter_macro++);
 	}
 	return 1;
 }

@@ -1882,6 +1885,7 @@ static void init_preprocessor(void)
 		sym->normal = 0;
 	}
 
+	counter_macro = 0;
 }
 
 static void handle_preprocessor_line(struct stream *stream, struct token **line, struct token *start)

validation/preprocessor/counter1.c new file +12 -0

new file mode 100644
index 00000000..98187ee6
@@ -0,0 +1,12 @@
+__COUNTER__
+__COUNTER__
+/*
+ * check-name: __COUNTER__ #1
+ * check-command: sparse -E $file
+ *
+ * check-output-start
+
+0
+1
+ * check-output-end
+ */

validation/preprocessor/counter2.c new file +14 -0

new file mode 100644
index 00000000..9883b682
@@ -0,0 +1,14 @@
+__FILE__ __COUNTER__
+#include <counter2.h>
+__FILE__ __COUNTER__
+/*
+ * check-name: __COUNTER__ #2
+ * check-command: sparse -Ipreprocessor -E $file
+ *
+ * check-output-start
+
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */

validation/preprocessor/counter2.h new file +1 -0

new file mode 100644
index 00000000..447b70ab
@@ -0,0 +1 @@
+__FILE__ __COUNTER__

validation/preprocessor/counter3.c new file +13 -0

new file mode 100644
index 00000000..1449b2d1
@@ -0,0 +1,13 @@
+/*
+ * check-name: __COUNTER__ #3
+ * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
+ *
+ * check-output-start
+
+0
+1
+"preprocessor/counter2.c" 0
+"preprocessor/counter2.h" 1
+"preprocessor/counter2.c" 2
+ * check-output-end
+ */

Christopher Li

maintainer

2015-02-02 05:17 UTC #12 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

In reply to Luc Van Oostenryck

Quoted 4 lines · Luc Van Oostenryck

Thanks for bringing to my attention.
Here is a new version of the patch taking care of that.

I like Sam's commit message better. So I merge a version using his commit message.

The branch for review is here:
https://git.kernel.org/cgit/devel/sparse/chrisl/sparse.git/log/?h=review-counter

If there is no objects, I will merge it to master.

Chris

Luc Van Oostenryck

2015-02-04 02:38 UTC #13 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Quoted 10 lines · Christopher Li

Quoted 4 lines · Luc Van Oostenryck
Thanks for bringing to my attention.
Here is a new version of the patch taking care of that.
I like Sam's commit message better. So I merge a version using his commit message.

Quoted 7 lines

The branch for review is here:
https://git.kernel.org/cgit/devel/sparse/chrisl/sparse.git/log/?h=review-counter
If there is no objects, I will merge it to master.
Chris
--

Fine for me but Josh had a remark about the third test file (validation/preprocessor/counter3.c) abusing a bit the test framework and I shared his feeling.

Here is a patch serie fixing this abuse by extending the test framework.

1/3) test-suite: add support for tests case involving several input files 2/3) test-suite: allow filename expansion of the input sections
3/3) test-suite: consolidate tests that require include files into single test files

 Documentation/test-suite                 | 11 +++++++++++
 validation/.gitignore                    |  1 +
 validation/pragma-once.c                 | 13 ++++++++++++-
 validation/preprocessor/counter2.c       | 16 +++++++++++-----
 validation/preprocessor/counter2.h       |  1 -
 validation/preprocessor/counter3.c       | 15 +++++++++++----
 validation/preprocessor/preprocessor20.c | 18 +++++++++++++++---
 validation/preprocessor/preprocessor20.h |  6 ------
 validation/test-suite                    | 11 +++++++++++
 validation/test-suite-file-expansion.c   | 22 ++++++++++++++++++++++
 10 files changed, 94 insertions(+), 20 deletions(-)

Christopher Li

maintainer

2015-02-12 20:16 UTC #14 lore raw

Re: [PATCH] Teach sparse about the __COUNTER__ predefined macro

Quoted 6 lines · Luc Van Oostenryck

Fine for me but Josh had a remark about the third test file (validation/preprocessor/counter3.c) abusing a bit the test framework and I shared his feeling.
Here is a patch serie fixing this abuse by extending the test framework.

I check in a version with very minor fix for the counter3.c hack. Counter3.c does a few things unclean, top of which is, it does not invoke counter3.c at all. It is invoking counter1.c and counter2.c.

I modify coutner3.c to include the couter2.c, that will solve the problem that counter3.c not using its own source code.

Any way, the change is pushed.

Thanks

Chris

Luc Van Oostenryck

2015-02-04 02:46 UTC #15 lore raw

PATCH 1/3 test-suite: add support for tests case involving several input files

In reply to Christopher Li

These input files, instead of being stored in real files and thus polluting/confusing
the test framework are now placed in a serie of sections inside an unique test file. These sections are delimited by new tags: input-file-1-start / input-file-1-end, / input-file-2-start/ ...

Signed-off-by Luc Van Oostenryck <[email protected]>

Inspired-by Josh Triplett <[email protected]>

---
 Documentation/test-suite           |  7 +++++++
 validation/.gitignore              |  1 +
 validation/preprocessor/counter3.c | 15 +++++++++++----
 validation/test-suite              |  9 +++++++++
 4 files changed, 28 insertions(+), 4 deletions(-)

Documentation/test-suite +7 -0

index 6c4f24f6..e2ce7003 100644
@@ -32,6 +32,10 @@ check-output-start / check-output-end (optional)
 check-known-to-fail (optional)
 	Mark the test as being known to fail.
 
+input-file-1-start / input-file-1-end, / input-file-2-start/ ... (optional)
+	The input files of check-command lies between those two tags.
+	It's only needed when a test requires several files.
+
 
 	Using test-suite
 	~~~~~~~~~~~~~~~~

@@ -58,6 +62,9 @@ name:
 cmd:
 	check-command value. If no cmd is provided, it defaults to
 	"sparse $file".
+	If the "input-file-1-start/..." tags are used those files are to be
+	referenced with "$file1", ... and the command need to be something like:
+	"sparse $file1 $file2"
 
 The output of the test-suite format command can be redirected into the
 test case to create a test-suite formated file.

validation/.gitignore +1 -0

index 77276ba4..2a5f114b 100644
@@ -2,3 +2,4 @@
 *.diff
 *.got
 *.expected
+*.input[1-9]

validation/preprocessor/counter3.c +11 -4

index 1449b2d1..76635e82 100644
@@ -1,13 +1,20 @@
+/* input-file-1-start */
+__COUNTER__
+__COUNTER__
+/* input-file-1-end */
+
+/* input-file-2-start */
+__COUNTER__
+/* input-file-2-end */
+
 /*
  * check-name: __COUNTER__ #3
- * check-command: sparse -Ipreprocessor -E preprocessor/counter1.c preprocessor/counter2.c
+ * check-command: sparse -E $file1 $file2
  *
  * check-output-start
 
 0
 1
-"preprocessor/counter2.c" 0
-"preprocessor/counter2.h" 1
-"preprocessor/counter2.c" 2
+0
  * check-output-end
  */

validation/test-suite +9 -0

index df5a7c60..97d4dd40 100755
@@ -102,6 +102,15 @@ do_test()
 	fi
 	test_name=$last_result
 
+	# grab the input sections
+	input_nr=1
+	while grep -q "input-file-$input_nr-start" "$file"; do
+		sed -n "/input-file-$input_nr-start/,/input-file-$input_nr-end/p" "$file" \
+			| grep -v input-file > "$file".input$input_nr
+		eval "file$input_nr=$file.input$input_nr"
+		input_nr=$(($input_nr + 1))
+	done
+
 	# does the test provide a specific command ?
 	cmd=`eval echo $default_path/$default_cmd`
 	get_value "check-command" $file

Signature · 1 line

2.2.0

Christopher Li

maintainer

2015-02-06 15:02 UTC #16 lore raw

Re: [PATCH 1/3] test-suite: add support for tests case involving several input files

Quoted 4 lines · Luc Van Oostenryck

These input files, instead of being stored in real files and thus polluting/confusing
the test framework are now placed in a serie of sections inside an unique test file. These sections are delimited by new tags: input-file-1-start / input-file-1-end, / input-file-2-start/ ...

Sorry for the late review. Can I summarize the polluting/confusing problem as:
If there is more than one C file was used in one test case, and test suit wll invoke each of the C file as a separate test case, not a good thing.

I think merging it into one test file then split them by the test suit is too complicated.
It is actually easier to maintain them as separate files.

I think the simpler way would be just teach the test suit, some file was mean to be
part of a separate test case. Just don't invoke it as stand alone test. We can add some rules like, embed a tag or simply reflect that in the file name.

It will be a smaller change to the test suit as well.

Thanks

Chris

Luc Van Oostenryck

2015-02-04 02:49 UTC #17 lore raw

PATCH 2/3 test-suite: allow filename expansion of the input sections

In reply to Christopher Li

Signed-off-by Luc Van Oostenryck <[email protected]>

---
 Documentation/test-suite               |  4 ++++
 validation/test-suite                  |  2 ++
 validation/test-suite-file-expansion.c | 22 ++++++++++++++++++++++
 3 files changed, 28 insertions(+)
 create mode 100644 validation/test-suite-file-expansion.c

Documentation/test-suite +4 -0

index e2ce7003..86bee335 100644
@@ -35,6 +35,10 @@ check-known-to-fail (optional)
 input-file-1-start / input-file-1-end, / input-file-2-start/ ... (optional)
 	The input files of check-command lies between those two tags.
 	It's only needed when a test requires several files.
+	The '$file1', '$file2', ... strings are special. They will be expanded
+	at run time to the name of each of these files. These strings can be used
+	inside these input sections themselves, the check-command or the
+	check-output section.
 
 
 	Using test-suite

validation/test-suite +2 -0

index 97d4dd40..8b0cd5e4 100755
@@ -106,6 +106,7 @@ do_test()
 	input_nr=1
 	while grep -q "input-file-$input_nr-start" "$file"; do
 		sed -n "/input-file-$input_nr-start/,/input-file-$input_nr-end/p" "$file" \
+			| sed "s:\\\$file\\([1-9]\\):$(basename $file).input\\1:" \
 			| grep -v input-file > "$file".input$input_nr
 		eval "file$input_nr=$file.input$input_nr"
 		input_nr=$(($input_nr + 1))

@@ -136,6 +137,7 @@ do_test()
 
 	# grab the expected output
 	sed -n '/check-output-start/,/check-output-end/p' $file \
+		| sed "s:\\\$file\\([1-9]\\):$file.input\\1:" \
 		| grep -v check-output > "$file".output.expected
 	sed -n '/check-error-start/,/check-error-end/p' $file \
 		| grep -v check-error > "$file".error.expected

validation/test-suite-file-expansion.c new file +22 -0

new file mode 100644
index 00000000..fc4ff8d5
@@ -0,0 +1,22 @@
+/* input-file-1-start */
+const char file = __FILE__;
+/* input-file-1-end */
+
+/* input-file-2-start */
+int val = 0;
+#include "$file1"
+const char file = __FILE__;
+/* input-file-2-end */
+
+/*
+ * check-name: test-suite $file* expansion
+ * check-command: sparse -E $file1 $file2
+ *
+ * check-output-start
+
+const char file = "$file1";
+int val = 0;
+const char file = "$file1";
+const char file = "$file2";
+ * check-output-end
+ */

Signature · 1 line

2.2.0

Luc Van Oostenryck

2015-02-04 03:11 UTC #18 lore raw

PATCH 2/3 test-suite: allow filename expansion of the input sections

In reply to Christopher Li

Signed-off-by Luc Van Oostenryck <[email protected]>

---
 Documentation/test-suite               |  4 ++++
 validation/test-suite                  |  2 ++
 validation/test-suite-file-expansion.c | 22 ++++++++++++++++++++++
 3 files changed, 28 insertions(+)
 create mode 100644 validation/test-suite-file-expansion.c

Documentation/test-suite +4 -0

index e2ce7003..86bee335 100644
@@ -35,6 +35,10 @@ check-known-to-fail (optional)
 input-file-1-start / input-file-1-end, / input-file-2-start/ ... (optional)
 	The input files of check-command lies between those two tags.
 	It's only needed when a test requires several files.
+	The '$file1', '$file2', ... strings are special. They will be expanded
+	at run time to the name of each of these files. These strings can be used
+	inside these input sections themselves, the check-command or the
+	check-output section.
 
 
 	Using test-suite

validation/test-suite +2 -0

index 97d4dd40..8b0cd5e4 100755
@@ -106,6 +106,7 @@ do_test()
 	input_nr=1
 	while grep -q "input-file-$input_nr-start" "$file"; do
 		sed -n "/input-file-$input_nr-start/,/input-file-$input_nr-end/p" "$file" \
+			| sed "s:\\\$file\\([1-9]\\):$(basename $file).input\\1:" \
 			| grep -v input-file > "$file".input$input_nr
 		eval "file$input_nr=$file.input$input_nr"
 		input_nr=$(($input_nr + 1))

@@ -136,6 +137,7 @@ do_test()
 
 	# grab the expected output
 	sed -n '/check-output-start/,/check-output-end/p' $file \
+		| sed "s:\\\$file\\([1-9]\\):$file.input\\1:" \
 		| grep -v check-output > "$file".output.expected
 	sed -n '/check-error-start/,/check-error-end/p' $file \
 		| grep -v check-error > "$file".error.expected

validation/test-suite-file-expansion.c new file +22 -0

new file mode 100644
index 00000000..fc4ff8d5
@@ -0,0 +1,22 @@
+/* input-file-1-start */
+const char file = __FILE__;
+/* input-file-1-end */
+
+/* input-file-2-start */
+int val = 0;
+#include "$file1"
+const char file = __FILE__;
+/* input-file-2-end */
+
+/*
+ * check-name: test-suite $file* expansion
+ * check-command: sparse -E $file1 $file2
+ *
+ * check-output-start
+
+const char file = "$file1";
+int val = 0;
+const char file = "$file1";
+const char file = "$file2";
+ * check-output-end
+ */

Signature · 1 line

2.2.0

Luc Van Oostenryck

2015-02-04 02:51 UTC #19 lore raw

PATCH 3/3 test-suite: consolidate tests that require include files into single test files

In reply to Christopher Li

Signed-off-by Luc Van Oostenryck <[email protected]>

---
 validation/pragma-once.c                 | 13 ++++++++++++-
 validation/preprocessor/counter2.c       | 16 +++++++++++-----
 validation/preprocessor/counter2.h       |  1 -
 validation/preprocessor/preprocessor20.c | 18 +++++++++++++++---
 validation/preprocessor/preprocessor20.h |  6 ------
 5 files changed, 38 insertions(+), 16 deletions(-)
 delete mode 100644 validation/preprocessor/counter2.h
 delete mode 100644 validation/preprocessor/preprocessor20.h

validation/pragma-once.c +12 -1

index 5e8b8254..83c0f9f9 100644
@@ -1,5 +1,16 @@
+/* input-file-1-start */
 #pragma once
-#include "pragma-once.c"
+
+struct s {
+	int i;
+};
+/* input-file-1-end */
+
+/* input-file-2-start */
+#include "$file1"
+#include "$file1"
+/* input-file-2-end */
 /*
  * check-name: #pragma once
+ * check-command: sparse $file2
  */

validation/preprocessor/counter2.c +11 -5

index 9883b682..fa15d954 100644
@@ -1,14 +1,20 @@
+/* input-file-1-start */
 __FILE__ __COUNTER__
-#include <counter2.h>
+/* input-file-1-end */
+
+/* input-file-2-start */
 __FILE__ __COUNTER__
+#include "$file1"
+__FILE__ __COUNTER__
+/* input-file-2-end */
 /*
  * check-name: __COUNTER__ #2
- * check-command: sparse -Ipreprocessor -E $file
+ * check-command: sparse -E $file2
  *
  * check-output-start
 
-"preprocessor/counter2.c" 0
-"preprocessor/counter2.h" 1
-"preprocessor/counter2.c" 2
+"$file2" 0
+"$file1" 1
+"$file2" 2
  * check-output-end
  */

validation/preprocessor/counter2.h deleted +0 -1

deleted file mode 100644
index 447b70ab..00000000
@@ -1 +0,0 @@
-__FILE__ __COUNTER__

validation/preprocessor/preprocessor20.c +15 -3

index 90e93f37..679a9737 100644
@@ -1,10 +1,22 @@
-#include "preprocessor20.h"
+/* input-file-1-start */
+#ifdef X
+B
+#endif
+#ifndef Y
+A
+#endif
+/* input-file-1-end */
+
+/* input-file-2-start */
+#include "$file1"
 #define X
 #define Y
-#include "preprocessor20.h"
+#include "$file1"
+/* input-file-2-end */
+
 /*
  * check-name: Preprocessor #20
- * check-command: sparse -E $file
+ * check-command: sparse -E $file2
  *
  * check-output-start

validation/preprocessor/preprocessor20.h deleted +0 -6

deleted file mode 100644
index 322c543a..00000000
@@ -1,6 +0,0 @@
-#ifdef X
-B
-#endif
-#ifndef Y
-A
-#endif

Signature · 1 line

2.2.0