[PATCH] dissect: add support for _Generic

[PATCH] dissect: add support for _Generic

[Alexey Gladkov]

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

Re: [PATCH] dissect: add support for _Generic

[Oleg Nesterov]

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.

Re: [PATCH] dissect: add support for _Generic

[Luc Van Oostenryck]

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

Re: [PATCH] dissect: add support for _Generic

[Oleg Nesterov]

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);
 	}

Re: [PATCH] dissect: add support for _Generic

[Luc Van Oostenryck]

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

Re: [PATCH] dissect: add support for _Generic

[Oleg Nesterov]

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.

[PATCH] dissect: support _Generic() a bit more

[Oleg Nesterov]

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

Re: [PATCH] dissect: add support for _Generic

[Luc Van Oostenryck]

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

Re: [PATCH] dissect: support _Generic() a bit more

[Luc Van Oostenryck]

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 

Re: [PATCH] dissect: add support for _Generic

[Oleg Nesterov]

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.

Re: [PATCH] dissect: add support for _Generic

[Luc Van Oostenryck]

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