| Message ID | 20200728183507.422662-1-gladkov.alexey@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | dissect: add support for _Generic | expand |
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.
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
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); }
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
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.
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
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.
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
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;
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(+)