Linux-Sparse Archive on lore.kernel.org
 help / color / Atom feed
* [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	[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	[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, back to index

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

Linux-Sparse Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sparse/0 linux-sparse/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sparse linux-sparse/ https://lore.kernel.org/linux-sparse \
		linux-sparse@vger.kernel.org
	public-inbox-index linux-sparse

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sparse


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git