* [PATCH] univ-init: scalar initializer needs some additional checks
@ 2020-06-02 16:33 Luc Van Oostenryck
2020-06-03 1:01 ` Ramsay Jones
0 siblings, 1 reply; 4+ messages in thread
From: Luc Van Oostenryck @ 2020-06-02 16:33 UTC (permalink / raw)
To: linux-sparse; +Cc: Ramsay Jones, Luc Van Oostenryck
Currently, -Wno-universal-initializer is simply implemented
by simply replacing '{ 0 }' by '{ }'.
However, this is a bit too simple when it concerns scalars
initialized with '{ 0 }' because:
* sparse & GCC issued warnings for empty scalar initializers
* initializing a pointer with '{ }' is extra bad.
So, restore the old behaviour for scalar initializers.
This is done by leaving '{ 0 }' as-is at parse time and changing
it as '{ }' only at evaluation time for compound initializers.
Fixes: 537e3e2daebd37d69447e65535fc94e82b38fc18
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
evaluate.c | 3 +++
expression.h | 1 +
parse.c | 15 ++++++++-------
validation/Wuniv-init-ko.c | 16 ++++++++++++++++
validation/Wuniv-init-ok.c | 18 ++++++++++++++++++
5 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index 8d2e68692a48..16553eb3481b 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2608,6 +2608,9 @@ static void handle_list_initializer(struct expression *expr,
struct expression *e, *last = NULL, *top = NULL, *next;
int jumped = 0;
+ if (expr->zero_init)
+ expr->expr_list = NULL;
+
FOR_EACH_PTR(expr->expr_list, e) {
struct expression **v;
struct symbol *type;
diff --git a/expression.h b/expression.h
index 64aa1fc23309..07fe8502e15e 100644
--- a/expression.h
+++ b/expression.h
@@ -159,6 +159,7 @@ DECLARE_ALLOCATOR(type_expression);
struct expression {
enum expression_type type:8;
unsigned flags:8;
+ unsigned zero_init:1;
int op;
struct position pos;
struct symbol *ctype;
diff --git a/parse.c b/parse.c
index 687c8c0c235c..9569efdc68b3 100644
--- a/parse.c
+++ b/parse.c
@@ -2783,13 +2783,6 @@ static struct token *initializer_list(struct expression_list **list, struct toke
{
struct expression *expr;
- // '{ 0 }' is equivalent to '{ }' unless wanting all possible
- // warnings about using '0' to initialize a null-pointer.
- if (!Wuniversal_initializer) {
- if (match_token_zero(token) && match_op(token->next, '}'))
- token = token->next;
- }
-
for (;;) {
token = single_initializer(&expr, token);
if (!expr)
@@ -2807,6 +2800,14 @@ struct token *initializer(struct expression **tree, struct token *token)
if (match_op(token, '{')) {
struct expression *expr = alloc_expression(token->pos, EXPR_INITIALIZER);
*tree = expr;
+ if (!Wuniversal_initializer) {
+ struct token *next = token->next;
+ // '{ 0 }' is equivalent to '{ }' except for some
+ // warnings, like using 0 to initialize a null-pointer.
+ if (match_token_zero(next) && match_op(next->next, '}'))
+ expr->zero_init = 1;
+ }
+
token = initializer_list(&expr->expr_list, token->next);
return expect(token, '}', "at end of initializer");
}
diff --git a/validation/Wuniv-init-ko.c b/validation/Wuniv-init-ko.c
index 315c211a5db6..bd77a0af55bd 100644
--- a/validation/Wuniv-init-ko.c
+++ b/validation/Wuniv-init-ko.c
@@ -4,11 +4,27 @@ struct s {
static struct s s = { 0 };
+static int a = { 0 };
+static int b = { };
+static int c = { 1, 2 };
+static struct s *ptr = { 0 };
+
+struct o {
+ struct i {
+ int a;
+ };
+};
+
+static struct o o = { 0 };
/*
* check-name: univ-init-ko
*
* check-error-start
Wuniv-init-ko.c:6:23: warning: Using plain integer as NULL pointer
+Wuniv-init-ko.c:8:16: error: invalid initializer
+Wuniv-init-ko.c:9:16: error: invalid initializer
+Wuniv-init-ko.c:10:26: warning: Using plain integer as NULL pointer
+Wuniv-init-ko.c:18:23: warning: missing braces around initializer
* check-error-end
*/
diff --git a/validation/Wuniv-init-ok.c b/validation/Wuniv-init-ok.c
index c39647517323..1f0c3dcb0c02 100644
--- a/validation/Wuniv-init-ok.c
+++ b/validation/Wuniv-init-ok.c
@@ -4,8 +4,26 @@ struct s {
static struct s s = { 0 };
+static int a = { 0 };
+static int b = { };
+static int c = { 1, 2 };
+static struct s *ptr = { 0 };
+
+struct o {
+ struct i {
+ int a;
+ };
+};
+
+static struct o o = { 0 };
/*
* check-name: univ-init-ok
* check-command: sparse -Wno-universal-initializer $file
+ *
+ * check-error-start
+Wuniv-init-ok.c:8:16: error: invalid initializer
+Wuniv-init-ok.c:9:16: error: invalid initializer
+Wuniv-init-ok.c:10:26: warning: Using plain integer as NULL pointer
+ * check-error-end
*/
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] univ-init: scalar initializer needs some additional checks
2020-06-02 16:33 [PATCH] univ-init: scalar initializer needs some additional checks Luc Van Oostenryck
@ 2020-06-03 1:01 ` Ramsay Jones
2020-06-03 13:47 ` Ramsay Jones
2020-06-03 21:16 ` Luc Van Oostenryck
0 siblings, 2 replies; 4+ messages in thread
From: Ramsay Jones @ 2020-06-03 1:01 UTC (permalink / raw)
To: Luc Van Oostenryck, linux-sparse
On 02/06/2020 17:33, Luc Van Oostenryck wrote:
> Currently, -Wno-universal-initializer is simply implemented
> by simply replacing '{ 0 }' by '{ }'.
>
> However, this is a bit too simple when it concerns scalars
> initialized with '{ 0 }' because:
> * sparse & GCC issued warnings for empty scalar initializers
> * initializing a pointer with '{ }' is extra bad.
>
> So, restore the old behaviour for scalar initializers.
> This is done by leaving '{ 0 }' as-is at parse time and changing
> it as '{ }' only at evaluation time for compound initializers.
>
I applied this patch just now and everything worked fine. In addition,
the tests from my patch also passed, once I had remembered to add the
-Wno-universal-initializer to the 'check-command' - because I do not
have the patch which changes the default for that warning.
The only thing which gave me pause ...
> Fixes: 537e3e2daebd37d69447e65535fc94e82b38fc18
> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
> ---
> evaluate.c | 3 +++
> expression.h | 1 +
> parse.c | 15 ++++++++-------
> validation/Wuniv-init-ko.c | 16 ++++++++++++++++
> validation/Wuniv-init-ok.c | 18 ++++++++++++++++++
> 5 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/evaluate.c b/evaluate.c
> index 8d2e68692a48..16553eb3481b 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -2608,6 +2608,9 @@ static void handle_list_initializer(struct expression *expr,
> struct expression *e, *last = NULL, *top = NULL, *next;
> int jumped = 0;
>
> + if (expr->zero_init)
> + expr->expr_list = NULL;
... was the potential memory leak here. (OK it wouldn't be a
huge leak, but still!).
ATB,
Ramsay Jones
> +
> FOR_EACH_PTR(expr->expr_list, e) {
> struct expression **v;
> struct symbol *type;
> diff --git a/expression.h b/expression.h
> index 64aa1fc23309..07fe8502e15e 100644
> --- a/expression.h
> +++ b/expression.h
> @@ -159,6 +159,7 @@ DECLARE_ALLOCATOR(type_expression);
> struct expression {
> enum expression_type type:8;
> unsigned flags:8;
> + unsigned zero_init:1;
> int op;
> struct position pos;
> struct symbol *ctype;
> diff --git a/parse.c b/parse.c
> index 687c8c0c235c..9569efdc68b3 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -2783,13 +2783,6 @@ static struct token *initializer_list(struct expression_list **list, struct toke
> {
> struct expression *expr;
>
> - // '{ 0 }' is equivalent to '{ }' unless wanting all possible
> - // warnings about using '0' to initialize a null-pointer.
> - if (!Wuniversal_initializer) {
> - if (match_token_zero(token) && match_op(token->next, '}'))
> - token = token->next;
> - }
> -
> for (;;) {
> token = single_initializer(&expr, token);
> if (!expr)
> @@ -2807,6 +2800,14 @@ struct token *initializer(struct expression **tree, struct token *token)
> if (match_op(token, '{')) {
> struct expression *expr = alloc_expression(token->pos, EXPR_INITIALIZER);
> *tree = expr;
> + if (!Wuniversal_initializer) {
> + struct token *next = token->next;
> + // '{ 0 }' is equivalent to '{ }' except for some
> + // warnings, like using 0 to initialize a null-pointer.
> + if (match_token_zero(next) && match_op(next->next, '}'))
> + expr->zero_init = 1;
> + }
> +
> token = initializer_list(&expr->expr_list, token->next);
> return expect(token, '}', "at end of initializer");
> }
> diff --git a/validation/Wuniv-init-ko.c b/validation/Wuniv-init-ko.c
> index 315c211a5db6..bd77a0af55bd 100644
> --- a/validation/Wuniv-init-ko.c
> +++ b/validation/Wuniv-init-ko.c
> @@ -4,11 +4,27 @@ struct s {
>
>
> static struct s s = { 0 };
> +static int a = { 0 };
> +static int b = { };
> +static int c = { 1, 2 };
> +static struct s *ptr = { 0 };
> +
> +struct o {
> + struct i {
> + int a;
> + };
> +};
> +
> +static struct o o = { 0 };
>
> /*
> * check-name: univ-init-ko
> *
> * check-error-start
> Wuniv-init-ko.c:6:23: warning: Using plain integer as NULL pointer
> +Wuniv-init-ko.c:8:16: error: invalid initializer
> +Wuniv-init-ko.c:9:16: error: invalid initializer
> +Wuniv-init-ko.c:10:26: warning: Using plain integer as NULL pointer
> +Wuniv-init-ko.c:18:23: warning: missing braces around initializer
> * check-error-end
> */
> diff --git a/validation/Wuniv-init-ok.c b/validation/Wuniv-init-ok.c
> index c39647517323..1f0c3dcb0c02 100644
> --- a/validation/Wuniv-init-ok.c
> +++ b/validation/Wuniv-init-ok.c
> @@ -4,8 +4,26 @@ struct s {
>
>
> static struct s s = { 0 };
> +static int a = { 0 };
> +static int b = { };
> +static int c = { 1, 2 };
> +static struct s *ptr = { 0 };
> +
> +struct o {
> + struct i {
> + int a;
> + };
> +};
> +
> +static struct o o = { 0 };
>
> /*
> * check-name: univ-init-ok
> * check-command: sparse -Wno-universal-initializer $file
> + *
> + * check-error-start
> +Wuniv-init-ok.c:8:16: error: invalid initializer
> +Wuniv-init-ok.c:9:16: error: invalid initializer
> +Wuniv-init-ok.c:10:26: warning: Using plain integer as NULL pointer
> + * check-error-end
> */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] univ-init: scalar initializer needs some additional checks
2020-06-03 1:01 ` Ramsay Jones
@ 2020-06-03 13:47 ` Ramsay Jones
2020-06-03 21:16 ` Luc Van Oostenryck
1 sibling, 0 replies; 4+ messages in thread
From: Ramsay Jones @ 2020-06-03 13:47 UTC (permalink / raw)
To: Luc Van Oostenryck, linux-sparse
On 03/06/2020 02:01, Ramsay Jones wrote:
> On 02/06/2020 17:33, Luc Van Oostenryck wrote:
[snip]
> I applied this patch just now and everything worked fine. In addition,
> the tests from my patch also passed, once I had remembered to add the
> -Wno-universal-initializer to the 'check-command' - because I do not
> have the patch which changes the default for that warning.
>
> The only thing which gave me pause ...
>
>> Fixes: 537e3e2daebd37d69447e65535fc94e82b38fc18
>> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
>> ---
>> evaluate.c | 3 +++
>> expression.h | 1 +
>> parse.c | 15 ++++++++-------
>> validation/Wuniv-init-ko.c | 16 ++++++++++++++++
>> validation/Wuniv-init-ok.c | 18 ++++++++++++++++++
>> 5 files changed, 46 insertions(+), 7 deletions(-)
>>
>> diff --git a/evaluate.c b/evaluate.c
>> index 8d2e68692a48..16553eb3481b 100644
>> --- a/evaluate.c
>> +++ b/evaluate.c
>> @@ -2608,6 +2608,9 @@ static void handle_list_initializer(struct expression *expr,
>> struct expression *e, *last = NULL, *top = NULL, *next;
>> int jumped = 0;
>>
>> + if (expr->zero_init)
>> + expr->expr_list = NULL;
>
> ... was the potential memory leak here. (OK it wouldn't be a
> huge leak, but still!).
Heh, as soon as my head hit the pillow I realised that, due to extensive
use of memory pools/arenas, this is a rather silly comment! ;-)
[Ah, well, it was 2am!]
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] univ-init: scalar initializer needs some additional checks
2020-06-03 1:01 ` Ramsay Jones
2020-06-03 13:47 ` Ramsay Jones
@ 2020-06-03 21:16 ` Luc Van Oostenryck
1 sibling, 0 replies; 4+ messages in thread
From: Luc Van Oostenryck @ 2020-06-03 21:16 UTC (permalink / raw)
To: Ramsay Jones; +Cc: linux-sparse
On Wed, Jun 03, 2020 at 02:01:07AM +0100, Ramsay Jones wrote:
>
> I applied this patch just now and everything worked fine. In addition,
> the tests from my patch also passed, once I had remembered to add the
> -Wno-universal-initializer to the 'check-command' - because I do not
> have the patch which changes the default for that warning.
I should have added that this patch was meant to be applied before
the one for the default :(
> The only thing which gave me pause ...
>
> > diff --git a/evaluate.c b/evaluate.c
> > index 8d2e68692a48..16553eb3481b 100644
> > --- a/evaluate.c
> > +++ b/evaluate.c
> > @@ -2608,6 +2608,9 @@ static void handle_list_initializer(struct expression *expr,
> > struct expression *e, *last = NULL, *top = NULL, *next;
> > int jumped = 0;
> >
> > + if (expr->zero_init)
> > + expr->expr_list = NULL;
>
> ... was the potential memory leak here. (OK it wouldn't be a
> huge leak, but still!).
Well yes [replying to your other mail too). It doesn't matter much here
but it's also easy to free the list, which is what I've done.
Thanks for giving a look at all of this.
Both patches are now applied.
-- Luc
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-03 21:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-02 16:33 [PATCH] univ-init: scalar initializer needs some additional checks Luc Van Oostenryck
2020-06-03 1:01 ` Ramsay Jones
2020-06-03 13:47 ` Ramsay Jones
2020-06-03 21:16 ` Luc Van Oostenryck
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).