evaluate: check static storage duration objects' intializers' constness

RFC PATCH Superseded Nicolai Stange 3 participants 8 messages Jul 22, 2015 – Dec 8, 2016 Sparse checker

lore

Share & tools

lore

Nicolai Stange

author

2015-07-22 23:19 UTC #0 lore raw

RFC PATCH 7/13 evaluate: check static storage duration objects' intializers' constness

Initializers of static storage duration objects shall be constant expressions [6.7.8(4)].

Warn if that requirement is not met.

Identify static storage duration objects by having either of MOD_TOPLEVEL or MOD_STATIC set.

Check an initializer's constness at the lowest possible subobject level, i.e. at the level of the "assignment-expression" production in [6.7.8].

For compound objects, make handle_list_initializer() pass the surrounding object's storage duration modifiers down to handle_simple_initializer() at subobject initializer evaluation.

Signed-off-by Nicolai Stange <[email protected]>

---
 evaluate.c                  |  25 +++++++++-
 validation/constexpr-init.c | 109 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 validation/constexpr-init.c

evaluate.c +24 -1

index c38b893..a65cc0c 100644
@@ -2466,6 +2466,7 @@ static void handle_list_initializer(struct expression *expr,
 {
 	struct expression *e, *last = NULL, *top = NULL, *next;
 	int jumped = 0;
+	unsigned long old_modifiers;
 
 	FOR_EACH_PTR(expr->expr_list, e) {
 		struct expression **v;

@@ -2520,8 +2521,21 @@ found:
 		else
 			v = &top->ident_expression;
 
-		if (handle_simple_initializer(v, 1, lclass, top->ctype))
+		/*
+		 * Temporarily copy storage modifiers down from
+		 * surrounding type such that
+		 * handle_simple_initializer() can check
+		 * initializations of subobjects with static storage
+		 * duration.
+		 */
+		old_modifiers = top->ctype->ctype.modifiers;
+		top->ctype->ctype.modifiers =
+			old_modifiers | (ctype->ctype.modifiers & MOD_STORAGE);
+		if (handle_simple_initializer(v, 1, lclass, top->ctype)) {
+			top->ctype->ctype.modifiers = old_modifiers;
 			continue;
+		}
+		top->ctype->ctype.modifiers = old_modifiers;
 
 		if (!(lclass & TYPE_COMPOUND)) {
 			warning(e->pos, "bogus scalar initializer");

@@ -2631,6 +2645,15 @@ static int handle_simple_initializer(struct expression **ep, int nested,
 		if (!evaluate_expression(e))
 			return 1;
 		compatible_assignment_types(e, ctype, ep, "initializer");
+		/*
+		 * Initializers for static storage duration objects
+		 * shall be constant expressions or a string literal [6.7.8(4)].
+		 */
+		if ((ctype->ctype.modifiers & (MOD_TOPLEVEL | MOD_STATIC)) &&
+			!(e->flags & (EXPR_FLAG_ARITH_CONST_EXPR
+					| EXPR_FLAG_ADDR_CONST_EXPR)))
+			warning(e->pos, "initializer for static storage duration object is not a constant expression");
+
 		return 1;
 	}

validation/constexpr-init.c new file +109 -0

109 lines not on this page · raw message

Signature · 1 line

2.4.5

Luc Van Oostenryck

2016-01-09 18:04 UTC #1 lore raw

Quoted 8 lines · Nicolai Stange

Initializers of static storage duration objects shall be constant expressions [6.7.8(4)].
Warn if that requirement is not met.
Identify static storage duration objects by having either of MOD_TOPLEVEL or MOD_STATIC set.

Quoted 7 lines

Check an initializer's constness at the lowest possible subobject level, i.e. at the level of the "assignment-expression" production in [6.7.8].
For compound objects, make handle_list_initializer() pass the surrounding object's storage duration modifiers down to handle_simple_initializer() at subobject initializer evaluation.

This patch makes validation/{builtin_bswap,choose_expr}.c fail. Of course, it's directly related to the purpose of the patch but then the test should be adapted.

Yours,
Luc

Nicolai Stange

author

2016-01-09 22:28 UTC #2 lore raw

Quoted 13 lines · Luc Van Oostenryck

Quoted 12 lines · Nicolai Stange
Initializers of static storage duration objects shall be constant expressions [6.7.8(4)].
Warn if that requirement is not met.
Identify static storage duration objects by having either of MOD_TOPLEVEL or MOD_STATIC set.
Check an initializer's constness at the lowest possible subobject level, i.e. at the level of the "assignment-expression" production in [6.7.8].

Quoted 9 lines

Quoted 3 lines
For compound objects, make handle_list_initializer() pass the surrounding object's storage duration modifiers down to handle_simple_initializer() at subobject initializer evaluation.
This patch makes validation/{builtin_bswap,choose_expr}.c fail. Of course, it's directly related to the purpose of the patch but then the test should be adapted.

Yes, you are absolutely right. However, as mentioned in this RFC series' cover letter, I decided to leave these two failers as is "for the moment". Certainly this is anything but best practice and I can only apologize for sending you half (well 97%) baken patches -- and promise to never do it again...

Luc Van Oostenryck

2016-01-11 18:02 UTC #3 lore raw

Quoted 20 lines · Nicolai Stange

Quoted 18 lines · Luc Van Oostenryck
Quoted 15 lines · Nicolai Stange
Initializers of static storage duration objects shall be constant expressions [6.7.8(4)].
Warn if that requirement is not met.
Identify static storage duration objects by having either of MOD_TOPLEVEL or MOD_STATIC set.
Check an initializer's constness at the lowest possible subobject level, i.e. at the level of the "assignment-expression" production in [6.7.8].
For compound objects, make handle_list_initializer() pass the surrounding object's storage duration modifiers down to handle_simple_initializer() at subobject initializer evaluation.

Quoted 7 lines

Quoted 4 lines
This patch makes validation/{builtin_bswap,choose_expr}.c fail. Of course, it's directly related to the purpose of the patch but then the test should be adapted.
Yes, you are absolutely right. However, as mentioned in this RFC series' cover letter, I decided to leave these two failers as is "for the moment".

It's fine then.
I just wanted to be sure that you was aware of it.

Quoted 3 lines

Certainly this is anything but best practice and I can only apologize for sending you half (well 97%) baken patches -- and promise to never do it again...

Personally, I think that drafts are very fine.
They're the basis on which we, developers, can exchange ideas. And your patches are far from drafts, they are already finely coocked.

But just to be sure to avoid any misunderstanding:
you know that I'm not the maintainer, just a reviewer. Right?

Yours,
Luc

Nicolai Stange

author

2016-01-11 18:15 UTC #4 lore raw

Quoted 36 lines · Luc Van Oostenryck

Quoted 20 lines · Nicolai Stange
Quoted 18 lines · Luc Van Oostenryck
Quoted 15 lines · Nicolai Stange
Initializers of static storage duration objects shall be constant expressions [6.7.8(4)].
Warn if that requirement is not met.
Identify static storage duration objects by having either of MOD_TOPLEVEL or MOD_STATIC set.
Check an initializer's constness at the lowest possible subobject level, i.e. at the level of the "assignment-expression" production in [6.7.8].
For compound objects, make handle_list_initializer() pass the surrounding object's storage duration modifiers down to handle_simple_initializer() at subobject initializer evaluation.
Quoted 7 lines
Quoted 4 lines
This patch makes validation/{builtin_bswap,choose_expr}.c fail. Of course, it's directly related to the purpose of the patch but then the test should be adapted.
Yes, you are absolutely right. However, as mentioned in this RFC series' cover letter, I decided to leave these two failers as is "for the moment".
It's fine then.
I just wanted to be sure that you was aware of it.
Quoted 3 lines
Certainly this is anything but best practice and I can only apologize for sending you half (well 97%) baken patches -- and promise to never do it again...

Quoted 6 lines

Personally, I think that drafts are very fine.
They're the basis on which we, developers, can exchange ideas. And your patches are far from drafts, they are already finely coocked.
But just to be sure to avoid any misunderstanding:
you know that I'm not the maintainer, just a reviewer. Right?

Yes, I know that:
https://sparse.wiki.kernel.org/index.php/Main_Page

Btw, this remembers me of the fact that Josh Triplett is still listed as a maintainer in sparse.1

Either of {web,man}page is wrong about that. Or both are incomplete and we have actually got two maintainers here?

Josh Triplett

2016-01-11 19:28 UTC #5 lore raw

Quoted 5 lines · Nicolai Stange

Btw, this remembers me of the fact that Josh Triplett is still listed as a maintainer in sparse.1
Either of {web,man}page is wrong about that. Or both are incomplete and we have actually got two maintainers here?

I'm definitely not the maintainer anymore. I just hang around and participate sporadically. :)

Luc Van Oostenryck

2016-12-08 04:49 UTC #6 lore raw

PATCH 1/1 Update maintainers in the manpage

The manpage still mentioned Josh Triplett as the maintainer while Christopher is the maintainer since 2009.

CC Christopher Li <[email protected]>

CC Josh Triplett <[email protected]>

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

---
 sparse.1 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

sparse.1 +1 -1

index ccae55cc..4aeb0039 100644
@@ -364,4 +364,4 @@ https://sparse.wiki.kernel.org/
 [email protected]
 .
 .SH MAINTAINER
-Josh Triplett <[email protected]>
+Christopher Li <[email protected]>

Signature · 1 line

2.10.2

Josh Triplett

Reviewed-by on v1 1/1

2016-12-08 05:43 UTC #7 lore raw

Re: [PATCH] Update maintainers in the manpage

Quoted 6 lines · Luc Van Oostenryck

The manpage still mentioned Josh Triplett as the maintainer while Christopher is the maintainer since 2009.
CC Christopher Li <[email protected]>
CC Josh Triplett <[email protected]>
Signed-off-by Luc Van Oostenryck <[email protected]>

Reviewed-by Josh Triplett <[email protected]>

Quoted 16 lines

 sparse.1 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

sparse.1 +1 -1

index ccae55cc..4aeb0039 100644
@@ -364,4 +364,4 @@ https://sparse.wiki.kernel.org/
 [email protected]
 .
 .SH MAINTAINER
-Josh Triplett <[email protected]>
+Christopher Li <[email protected]>

Signature · 1 line

2.10.2