* [PATCH] dissect: add support for _Generic
@ 2020-07-28 18:35 Alexey Gladkov
2020-07-28 19:49 ` Oleg Nesterov
0 siblings, 1 reply; 11+ messages in thread
From: Alexey Gladkov @ 2020-07-28 18:35 UTC (permalink / raw)
To: linux-sparse; +Cc: Oleg Nesterov, Luc Van Oostenryck
No special support needed for _Generic, so just suppress the warning
about unknown type.
Before:
$ ./test-dissect validation/generic-functions.c
FILE: validation/generic-functions.c
13:1 def f testf void ( ... )
13:1 testf def . v a float
validation/generic-functions.c:13:1: warning: bad expr->type: 31
13:1 testf -r- . v a float
14:1 def f testd void ( ... )
14:1 testd def . v a double
validation/generic-functions.c:14:1: warning: bad expr->type: 31
14:1 testd -r- . v a double
15:1 def f testl void ( ... )
15:1 testl def . v a long double
validation/generic-functions.c:15:1: warning: bad expr->type: 31
15:1 testl -r- . v a long double
After:
$ ./test-dissect validation/generic-functions.c
FILE: validation/generic-functions.c
13:1 def f testf void ( ... )
13:1 testf def . v a float
13:1 testf -r- . v a float
14:1 def f testd void ( ... )
14:1 testd def . v a double
14:1 testd -r- . v a double
15:1 def f testl void ( ... )
15:1 testl def . v a long double
15:1 testl -r- . v a long double
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
dissect.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/dissect.c b/dissect.c
index ccb7897b..b494f93c 100644
--- a/dissect.c
+++ b/dissect.c
@@ -342,6 +342,7 @@ again:
case EXPR_TYPE: // [struct T]; Why ???
case EXPR_VALUE:
case EXPR_FVALUE:
+ case EXPR_GENERIC:
break; case EXPR_LABEL:
ret = &label_ctype;
--
2.25.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: add support for _Generic
2020-07-28 18:35 [PATCH] dissect: add support for _Generic Alexey Gladkov
@ 2020-07-28 19:49 ` Oleg Nesterov
2020-07-28 23:10 ` Luc Van Oostenryck
0 siblings, 1 reply; 11+ messages in thread
From: Oleg Nesterov @ 2020-07-28 19:49 UTC (permalink / raw)
To: Alexey Gladkov; +Cc: linux-sparse, Luc Van Oostenryck
On 07/28, Alexey Gladkov wrote:
>
> No special support needed for _Generic,
Hmm. I am already sleeping and didn't read the _Generic code yet... but
shouldn't dissect() inspect ->control/map/def?
That said,
> so just suppress the warning
> about unknown type.
probably better than nothing, lets shut up the warning first.
Oleg.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: add support for _Generic
2020-07-28 19:49 ` Oleg Nesterov
@ 2020-07-28 23:10 ` Luc Van Oostenryck
2020-07-29 11:28 ` Oleg Nesterov
2020-07-30 15:09 ` [PATCH] dissect: support _Generic() a bit more Oleg Nesterov
0 siblings, 2 replies; 11+ messages in thread
From: Luc Van Oostenryck @ 2020-07-28 23:10 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Alexey Gladkov, linux-sparse
On Tue, Jul 28, 2020 at 09:49:38PM +0200, Oleg Nesterov wrote:
> On 07/28, Alexey Gladkov wrote:
> >
> > No special support needed for _Generic,
>
> Hmm. I am already sleeping and didn't read the _Generic code yet... but
> shouldn't dissect() inspect ->control/map/def?
>
> That said,
>
> > so just suppress the warning
> > about unknown type.
>
> probably better than nothing, lets shut up the warning first.
OK, since there is some urgency, I applied it directly but
my first reaction was also "eh, you can't just ignore
EXPR_GENERIC / pretend it's one of the top-level expression".
OTOH, I wonder what can be done without first evaluating
(the type of) the controlling expression and the types of the map
(if I understand correctly, evaluation is avoided in dissect).
-- Luc
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: add support for _Generic
2020-07-28 23:10 ` Luc Van Oostenryck
@ 2020-07-29 11:28 ` Oleg Nesterov
2020-07-29 14:50 ` Luc Van Oostenryck
2020-07-30 15:09 ` [PATCH] dissect: support _Generic() a bit more Oleg Nesterov
1 sibling, 1 reply; 11+ messages in thread
From: Oleg Nesterov @ 2020-07-29 11:28 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
On 07/29, Luc Van Oostenryck wrote:
>
> OTOH, I wonder what can be done without first evaluating
> (the type of) the controlling expression and the types of the map
> (if I understand correctly, evaluation is avoided in dissect).
Yes. I'll try to think a bit more, but so far I think I'll simply
send the patch below.
Test-case:
void func(void)
{
_Generic(a,
int: b,
void: c,
default: d,
) = e;
}
output:
1:6 def f func void ( ... )
3:18 func --- v a bad type
4:33 func -w- v b bad type
5:33 func -w- v c bad type
6:33 func -w- v d bad type
7:13 func -r- v e bad type
Of course, technically this is wrong, it looks as if all 3 variables are
modified. But not that bad imo, dissect doesn't even try to be "precise",
and this output still looks useful for the indexing/etc.
Oleg.
--- a/dissect.c
+++ b/dissect.c
@@ -342,7 +342,6 @@ again:
case EXPR_TYPE: // [struct T]; Why ???
case EXPR_VALUE:
case EXPR_FVALUE:
- case EXPR_GENERIC:
break; case EXPR_LABEL:
ret = &label_ctype;
@@ -472,6 +471,17 @@ again:
} while ((expr = expr->down));
}
+ break; case EXPR_GENERIC: {
+ struct type_expression *map;
+
+ do_expression(U_VOID, expr->control);
+
+ for (map = expr->map; map; map = map->next)
+ ret = do_expression(mode, map->expr);
+ if (expr->def)
+ ret = do_expression(mode, expr->def);
+ }
+
break; case EXPR_SYMBOL:
ret = report_symbol(mode, expr);
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: add support for _Generic
2020-07-29 11:28 ` Oleg Nesterov
@ 2020-07-29 14:50 ` Luc Van Oostenryck
2020-07-30 15:08 ` Oleg Nesterov
0 siblings, 1 reply; 11+ messages in thread
From: Luc Van Oostenryck @ 2020-07-29 14:50 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Alexey Gladkov, linux-sparse
On Wed, Jul 29, 2020 at 01:28:02PM +0200, Oleg Nesterov wrote:
> On 07/29, Luc Van Oostenryck wrote:
> >
> > OTOH, I wonder what can be done without first evaluating
> > (the type of) the controlling expression and the types of the map
> > (if I understand correctly, evaluation is avoided in dissect).
>
> Yes. I'll try to think a bit more, but so far I think I'll simply
> send the patch below.
...
> Of course, technically this is wrong, it looks as if all 3 variables are
> modified. But not that bad imo, dissect doesn't even try to be "precise",
> and this output still looks useful for the indexing/etc.
>
> --- a/dissect.c
> +++ b/dissect.c
> @@ -342,7 +342,6 @@ again:
> case EXPR_TYPE: // [struct T]; Why ???
> case EXPR_VALUE:
> case EXPR_FVALUE:
> - case EXPR_GENERIC:
>
> break; case EXPR_LABEL:
> ret = &label_ctype;
> @@ -472,6 +471,17 @@ again:
> } while ((expr = expr->down));
> }
>
> + break; case EXPR_GENERIC: {
> + struct type_expression *map;
> +
> + do_expression(U_VOID, expr->control);
> +
> + for (map = expr->map; map; map = map->next)
> + ret = do_expression(mode, map->expr);
> + if (expr->def)
> + ret = do_expression(mode, expr->def);
> + }
> +
> break; case EXPR_SYMBOL:
> ret = report_symbol(mode, expr);
> }
Yes, that should do the 'walking'. The returned type will just be
quite arbitrary, but I don't know how much it matters.
-- Luc
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: add support for _Generic
2020-07-29 14:50 ` Luc Van Oostenryck
@ 2020-07-30 15:08 ` Oleg Nesterov
2020-07-30 20:00 ` Luc Van Oostenryck
0 siblings, 1 reply; 11+ messages in thread
From: Oleg Nesterov @ 2020-07-30 15:08 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
On 07/29, Luc Van Oostenryck wrote:
>
> > + break; case EXPR_GENERIC: {
> > + struct type_expression *map;
> > +
> > + do_expression(U_VOID, expr->control);
> > +
> > + for (map = expr->map; map; map = map->next)
> > + ret = do_expression(mode, map->expr);
> > + if (expr->def)
> > + ret = do_expression(mode, expr->def);
> > + }
> > +
> > break; case EXPR_SYMBOL:
> > ret = report_symbol(mode, expr);
> > }
>
> Yes, that should do the 'walking'.
OK, I am sending this stupid patch. Better than nothing.
> The returned type will just be
> quite arbitrary, but I don't know how much it matters.
Of course. And this is not good. For example:
void func(void)
{
struct B *b; struct C *c; struct D *d;
_Generic(a,
int: b,
void*: c,
default: d
) ->mem++;
}
output:
1:6 def f func void ( ... )
3:18 func def . v b struct B *
3:31 func def . v c struct C *
3:44 func def . v d struct D *
4:18 func --- v a bad type
5:33 func --m . v b struct B *
6:33 func --m . v c struct C *
7:33 func --m . v d struct D *
8:11 func -m- m D.mem bad type
But I do not know how to improve it without serious complications, and
(so far) I think it doesn't worth the effort.
Oleg.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] dissect: support _Generic() a bit more
2020-07-28 23:10 ` Luc Van Oostenryck
2020-07-29 11:28 ` Oleg Nesterov
@ 2020-07-30 15:09 ` Oleg Nesterov
2020-07-30 20:05 ` Luc Van Oostenryck
1 sibling, 1 reply; 11+ messages in thread
From: Oleg Nesterov @ 2020-07-30 15:09 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
Change do_expression(EXPR_GENERIC) to inspect expr->control/map/def.
The is the minimal "better than nothing" change, technically incorrect
but still useful for the indexing.
Example:
void func(void)
{
_Generic(a,
int: b,
void: c,
default: d,
) = e;
}
output:
1:6 def f func void ( ... )
3:18 func --- v a bad type
4:33 func -w- v b bad type
5:33 func -w- v c bad type
6:33 func -w- v d bad type
7:13 func -r- v e bad type
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
dissect.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/dissect.c b/dissect.c
index a633c8bf..fd09707d 100644
--- a/dissect.c
+++ b/dissect.c
@@ -342,7 +342,6 @@ again:
case EXPR_TYPE: // [struct T]; Why ???
case EXPR_VALUE:
case EXPR_FVALUE:
- case EXPR_GENERIC:
break; case EXPR_LABEL:
ret = &label_ctype;
@@ -472,6 +471,17 @@ again:
} while ((expr = expr->down));
}
+ break; case EXPR_GENERIC: {
+ struct type_expression *map;
+
+ do_expression(U_VOID, expr->control);
+
+ for (map = expr->map; map; map = map->next)
+ ret = do_expression(mode, map->expr);
+ if (expr->def)
+ ret = do_expression(mode, expr->def);
+ }
+
break; case EXPR_SYMBOL:
ret = report_symbol(mode, expr);
}
--
2.25.1.362.g51ebf55
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: add support for _Generic
2020-07-30 15:08 ` Oleg Nesterov
@ 2020-07-30 20:00 ` Luc Van Oostenryck
2020-07-31 14:43 ` Oleg Nesterov
0 siblings, 1 reply; 11+ messages in thread
From: Luc Van Oostenryck @ 2020-07-30 20:00 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Alexey Gladkov, linux-sparse
On Thu, Jul 30, 2020 at 05:08:37PM +0200, Oleg Nesterov wrote:
> On 07/29, Luc Van Oostenryck wrote:
> > The returned type will just be
> > quite arbitrary, but I don't know how much it matters.
>
> Of course. And this is not good. For example:
>
> void func(void)
> {
> struct B *b; struct C *c; struct D *d;
> _Generic(a,
> int: b,
> void*: c,
> default: d
> ) ->mem++;
> }
>
> output:
>
> 1:6 def f func void ( ... )
> 3:18 func def . v b struct B *
> 3:31 func def . v c struct C *
> 3:44 func def . v d struct D *
> 4:18 func --- v a bad type
> 5:33 func --m . v b struct B *
> 6:33 func --m . v c struct C *
> 7:33 func --m . v d struct D *
> 8:11 func -m- m D.mem bad type
>
> But I do not know how to improve it without serious complications, and
Are you thinking about calling evaluate_symbol_list() or about
something else? What kind of complications?
> (so far) I think it doesn't worth the effort.
Yes, _Generic() clearly makes things a bit more complicated here.
Same for __auto_type, which is not yet used by the kernel but will
probably be soon.
-- Luc
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: support _Generic() a bit more
2020-07-30 15:09 ` [PATCH] dissect: support _Generic() a bit more Oleg Nesterov
@ 2020-07-30 20:05 ` Luc Van Oostenryck
0 siblings, 0 replies; 11+ messages in thread
From: Luc Van Oostenryck @ 2020-07-30 20:05 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Alexey Gladkov, linux-sparse
On Thu, Jul 30, 2020 at 05:09:58PM +0200, Oleg Nesterov wrote:
> Change do_expression(EXPR_GENERIC) to inspect expr->control/map/def.
> The is the minimal "better than nothing" change, technically incorrect
> but still useful for the indexing.
Thanks. Applied & pushed.
-- Luc
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: add support for _Generic
2020-07-30 20:00 ` Luc Van Oostenryck
@ 2020-07-31 14:43 ` Oleg Nesterov
2020-07-31 16:13 ` Luc Van Oostenryck
0 siblings, 1 reply; 11+ messages in thread
From: Oleg Nesterov @ 2020-07-31 14:43 UTC (permalink / raw)
To: Luc Van Oostenryck; +Cc: Alexey Gladkov, linux-sparse
On 07/30, Luc Van Oostenryck wrote:
>
> On Thu, Jul 30, 2020 at 05:08:37PM +0200, Oleg Nesterov wrote:
> > On 07/29, Luc Van Oostenryck wrote:
> > > The returned type will just be
> > > quite arbitrary, but I don't know how much it matters.
> >
> > Of course. And this is not good. For example:
> >
> > void func(void)
> > {
> > struct B *b; struct C *c; struct D *d;
> > _Generic(a,
> > int: b,
> > void*: c,
> > default: d
> > ) ->mem++;
> > }
> >
> > output:
> >
> > 1:6 def f func void ( ... )
> > 3:18 func def . v b struct B *
> > 3:31 func def . v c struct C *
> > 3:44 func def . v d struct D *
> > 4:18 func --- v a bad type
> > 5:33 func --m . v b struct B *
> > 6:33 func --m . v c struct C *
> > 7:33 func --m . v d struct D *
> > 8:11 func -m- m D.mem bad type
> >
> > But I do not know how to improve it without serious complications, and
>
> Are you thinking about calling evaluate_symbol_list()
I meant, it is not simple to teach dissect() to handle this case correctly.
It understand the types, but for example it doesn't even try to distinguish
"int" and "float".
And I would like to avoid evaluate_expression/etc.
> or about
> something else?
And something else. See the example above, this code is incomplete and in this
case evaluate can't help. Ideally dissect should also report the (possible) usage
of B.mem and C.mem.
> > (so far) I think it doesn't worth the effort.
>
> Yes, _Generic() clearly makes things a bit more complicated here.
> Same for __auto_type,
Yes, but hopefully dissect needs much more simple changes to handle __auto_type.
Thanks,
Oleg.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dissect: add support for _Generic
2020-07-31 14:43 ` Oleg Nesterov
@ 2020-07-31 16:13 ` Luc Van Oostenryck
0 siblings, 0 replies; 11+ messages in thread
From: Luc Van Oostenryck @ 2020-07-31 16:13 UTC (permalink / raw)
To: Oleg Nesterov; +Cc: Alexey Gladkov, linux-sparse
On Fri, Jul 31, 2020 at 04:43:01PM +0200, Oleg Nesterov wrote:
> On 07/30, Luc Van Oostenryck wrote:
> >
> > On Thu, Jul 30, 2020 at 05:08:37PM +0200, Oleg Nesterov wrote:
> > > But I do not know how to improve it without serious complications, and
> >
> > Are you thinking about calling evaluate_symbol_list()
>
> I meant, it is not simple to teach dissect() to handle this case correctly.
> It understand the types, but for example it doesn't even try to distinguish
> "int" and "float".
OK, that's fine. It's just like using a super type like 'scalar'
or 'basetype' or something.
> And I would like to avoid evaluate_expression/etc.
>
> > or about
> > something else?
>
> And something else. See the example above, this code is incomplete and in this
> case evaluate can't help. Ideally dissect should also report the (possible) usage
> of B.mem and C.mem.
OK, I begin to understand. You want your own type evaluation with
its own rules. evaluate_expression() and friends would indeed not help.
But I'm afraid that, once _Generic() will be used more extensively
in macros, it will create a lot of these 'possible usages' that would,
in fact, be irrelevant to the code analyzed, possibly with an explosion
of combinations.
> > > (so far) I think it doesn't worth the effort.
> >
> > Yes, _Generic() clearly makes things a bit more complicated here.
> > Same for __auto_type,
>
> Yes, but hopefully dissect needs much more simple changes to handle __auto_type.
Yes, it should.
Thanks for the reply.
--Luc
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-07-31 16:13 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28 18:35 [PATCH] dissect: add support for _Generic Alexey Gladkov
2020-07-28 19:49 ` Oleg Nesterov
2020-07-28 23:10 ` Luc Van Oostenryck
2020-07-29 11:28 ` Oleg Nesterov
2020-07-29 14:50 ` Luc Van Oostenryck
2020-07-30 15:08 ` Oleg Nesterov
2020-07-30 20:00 ` Luc Van Oostenryck
2020-07-31 14:43 ` Oleg Nesterov
2020-07-31 16:13 ` Luc Van Oostenryck
2020-07-30 15:09 ` [PATCH] dissect: support _Generic() a bit more Oleg Nesterov
2020-07-30 20:05 ` 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).