Coccinelle archive on lore.kernel.org
 help / color / Atom feed
* [Cocci] Using a macro for variable attributes
@ 2020-04-28 10:11 Paul Chaignon
  2020-04-28 17:44 ` Julia Lawall
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Chaignon @ 2020-04-28 10:11 UTC (permalink / raw)
  To: cocci; +Cc: paul.chaignon

Hi all,

I am working on a small semantic patch to annotate specific variables in
our codebase with __attribute__((aligned(8))). The following program works
fine.

  @r@
  expression e1, e2;
  identifier x;
  @@
  (
    struct \(icmphdr\|icmp6hdr\) x
  + __attribute__((aligned(8)))
    ;
  |
    struct \(icmphdr\|icmp6hdr\) x
  + __attribute__((aligned(8)))
    = ...;
  )
    ... when exists
    ctx_load_bytes(e1, e2, &x, ...)

However, when I replace __attribute__((aligned(8))) with our internal
macro __align_stack_8, it fails with the following error:

  plus: parse error:
    File "/home/paul/cilium/contrib/coccinelle/aligned.cocci", line 7, column 2, charpos = 77
    around = '__align_stack_8',
    whole content = + __align_stack_8

I've tried adding '#define __align_stack_8' in a file passed with
--macro-file, without success. Is this a known limitation for macros or
am I missing something?

Thanks,
Paul
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] Using a macro for variable attributes
  2020-04-28 10:11 [Cocci] Using a macro for variable attributes Paul Chaignon
@ 2020-04-28 17:44 ` Julia Lawall
  2020-04-28 18:17   ` Paul Chaignon
  0 siblings, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2020-04-28 17:44 UTC (permalink / raw)
  To: Paul Chaignon; +Cc: cocci, paul.chaignon



On Tue, 28 Apr 2020, Paul Chaignon wrote:

> Hi all,
>
> I am working on a small semantic patch to annotate specific variables in
> our codebase with __attribute__((aligned(8))). The following program works
> fine.
>
>   @r@
>   expression e1, e2;
>   identifier x;
>   @@
>   (
>     struct \(icmphdr\|icmp6hdr\) x
>   + __attribute__((aligned(8)))
>     ;
>   |
>     struct \(icmphdr\|icmp6hdr\) x
>   + __attribute__((aligned(8)))
>     = ...;
>   )
>     ... when exists
>     ctx_load_bytes(e1, e2, &x, ...)
>
> However, when I replace __attribute__((aligned(8))) with our internal
> macro __align_stack_8, it fails with the following error:
>
>   plus: parse error:
>     File "/home/paul/cilium/contrib/coccinelle/aligned.cocci", line 7, column 2, charpos = 77
>     around = '__align_stack_8',
>     whole content = + __align_stack_8
>
> I've tried adding '#define __align_stack_8' in a file passed with
> --macro-file, without success. Is this a known limitation for macros or
> am I missing something?

Try adding the "metavariable" declaration:

attribute name __align_stack_8;

julia

>
> Thanks,
> Paul
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] Using a macro for variable attributes
  2020-04-28 17:44 ` Julia Lawall
@ 2020-04-28 18:17   ` Paul Chaignon
  2020-04-28 18:41     ` Julia Lawall
  2020-04-30 15:40     ` Jaskaran Singh
  0 siblings, 2 replies; 7+ messages in thread
From: Paul Chaignon @ 2020-04-28 18:17 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, paul.chaignon

Thanks for the quick answer!

On Tue, Apr 28, 2020 at 07:44:15PM +0200, Julia Lawall wrote:
> 
> 
> On Tue, 28 Apr 2020, Paul Chaignon wrote:
> 
> > Hi all,
> >
> > I am working on a small semantic patch to annotate specific variables in
> > our codebase with __attribute__((aligned(8))). The following program works
> > fine.
> >
> >   @r@
> >   expression e1, e2;
> >   identifier x;
> >   @@
> >   (
> >     struct \(icmphdr\|icmp6hdr\) x
> >   + __attribute__((aligned(8)))
> >     ;
> >   |
> >     struct \(icmphdr\|icmp6hdr\) x
> >   + __attribute__((aligned(8)))
> >     = ...;
> >   )
> >     ... when exists
> >     ctx_load_bytes(e1, e2, &x, ...)
> >
> > However, when I replace __attribute__((aligned(8))) with our internal
> > macro __align_stack_8, it fails with the following error:
> >
> >   plus: parse error:
> >     File "/home/paul/cilium/contrib/coccinelle/aligned.cocci", line 7, column 2, charpos = 77
> >     around = '__align_stack_8',
> >     whole content = + __align_stack_8
> >
> > I've tried adding '#define __align_stack_8' in a file passed with
> > --macro-file, without success. Is this a known limitation for macros or
> > am I missing something?
> 
> Try adding the "metavariable" declaration:
> 
> attribute name __align_stack_8;

Awesome, that worked. And I think I understand: undeclared identifiers are by
default considered symbols, leading to the parse error.

Unfortunately, my semantic patch now leads to the following changes:

  -	struct icmphdr icmphdr __align_stack_8;
  +	struct icmphdr icmphdr __align_stack_8 __align_stack_8;

I would normally add a first case to my conjunction to match on
already-present attributes, but Coccinelle can't match on attributes yet.
Any workaround?

Paul
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] Using a macro for variable attributes
  2020-04-28 18:17   ` Paul Chaignon
@ 2020-04-28 18:41     ` Julia Lawall
  2020-04-28 18:58       ` Paul Chaignon
  2020-04-30 15:40     ` Jaskaran Singh
  1 sibling, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2020-04-28 18:41 UTC (permalink / raw)
  To: Paul Chaignon; +Cc: cocci, paul.chaignon



On Tue, 28 Apr 2020, Paul Chaignon wrote:

> Thanks for the quick answer!
>
> On Tue, Apr 28, 2020 at 07:44:15PM +0200, Julia Lawall wrote:
> >
> >
> > On Tue, 28 Apr 2020, Paul Chaignon wrote:
> >
> > > Hi all,
> > >
> > > I am working on a small semantic patch to annotate specific variables in
> > > our codebase with __attribute__((aligned(8))). The following program works
> > > fine.
> > >
> > >   @r@
> > >   expression e1, e2;
> > >   identifier x;
> > >   @@
> > >   (
> > >     struct \(icmphdr\|icmp6hdr\) x
> > >   + __attribute__((aligned(8)))
> > >     ;
> > >   |
> > >     struct \(icmphdr\|icmp6hdr\) x
> > >   + __attribute__((aligned(8)))
> > >     = ...;
> > >   )
> > >     ... when exists
> > >     ctx_load_bytes(e1, e2, &x, ...)
> > >
> > > However, when I replace __attribute__((aligned(8))) with our internal
> > > macro __align_stack_8, it fails with the following error:
> > >
> > >   plus: parse error:
> > >     File "/home/paul/cilium/contrib/coccinelle/aligned.cocci", line 7, column 2, charpos = 77
> > >     around = '__align_stack_8',
> > >     whole content = + __align_stack_8
> > >
> > > I've tried adding '#define __align_stack_8' in a file passed with
> > > --macro-file, without success. Is this a known limitation for macros or
> > > am I missing something?
> >
> > Try adding the "metavariable" declaration:
> >
> > attribute name __align_stack_8;
>
> Awesome, that worked. And I think I understand: undeclared identifiers are by
> default considered symbols, leading to the parse error.
>
> Unfortunately, my semantic patch now leads to the following changes:
>
>   -	struct icmphdr icmphdr __align_stack_8;
>   +	struct icmphdr icmphdr __align_stack_8 __align_stack_8;
>
> I would normally add a first case to my conjunction to match on
> already-present attributes, but Coccinelle can't match on attributes yet.
> Any workaround?

Jaskaran is working on it.

A hackish solution would be to put a position variable on the variable
name and put a position variable on the ; and then use python to see if
they are not adjacent to each other...

Do you get a lot of occurrences of the problem?  In the short term it
could be simpler to just clean it up by hand.  It should be easy to search
for at least.

julia


>
> Paul
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] Using a macro for variable attributes
  2020-04-28 18:41     ` Julia Lawall
@ 2020-04-28 18:58       ` Paul Chaignon
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Chaignon @ 2020-04-28 18:58 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci, paul.chaignon

On Tue, Apr 28, 2020 at 08:41:08PM +0200, Julia Lawall wrote:
> 
> 
> On Tue, 28 Apr 2020, Paul Chaignon wrote:
> 
> > Thanks for the quick answer!
> >
> > On Tue, Apr 28, 2020 at 07:44:15PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Tue, 28 Apr 2020, Paul Chaignon wrote:
> > >
> > > > Hi all,
> > > >
> > > > I am working on a small semantic patch to annotate specific variables in
> > > > our codebase with __attribute__((aligned(8))). The following program works
> > > > fine.
> > > >
> > > >   @r@
> > > >   expression e1, e2;
> > > >   identifier x;
> > > >   @@
> > > >   (
> > > >     struct \(icmphdr\|icmp6hdr\) x
> > > >   + __attribute__((aligned(8)))
> > > >     ;
> > > >   |
> > > >     struct \(icmphdr\|icmp6hdr\) x
> > > >   + __attribute__((aligned(8)))
> > > >     = ...;
> > > >   )
> > > >     ... when exists
> > > >     ctx_load_bytes(e1, e2, &x, ...)
> > > >
> > > > However, when I replace __attribute__((aligned(8))) with our internal
> > > > macro __align_stack_8, it fails with the following error:
> > > >
> > > >   plus: parse error:
> > > >     File "/home/paul/cilium/contrib/coccinelle/aligned.cocci", line 7, column 2, charpos = 77
> > > >     around = '__align_stack_8',
> > > >     whole content = + __align_stack_8
> > > >
> > > > I've tried adding '#define __align_stack_8' in a file passed with
> > > > --macro-file, without success. Is this a known limitation for macros or
> > > > am I missing something?
> > >
> > > Try adding the "metavariable" declaration:
> > >
> > > attribute name __align_stack_8;
> >
> > Awesome, that worked. And I think I understand: undeclared identifiers are by
> > default considered symbols, leading to the parse error.
> >
> > Unfortunately, my semantic patch now leads to the following changes:
> >
> >   -	struct icmphdr icmphdr __align_stack_8;
> >   +	struct icmphdr icmphdr __align_stack_8 __align_stack_8;
> >
> > I would normally add a first case to my conjunction to match on
> > already-present attributes, but Coccinelle can't match on attributes yet.
> > Any workaround?
> 
> Jaskaran is working on it.

Great! I'll keep a watch on new releases :-)

> A hackish solution would be to put a position variable on the variable
> name and put a position variable on the ; and then use python to see if
> they are not adjacent to each other...
> 
> Do you get a lot of occurrences of the problem?  In the short term it
> could be simpler to just clean it up by hand.  It should be easy to search
> for at least.

We don't have a lot of variables that need this annotation overall, but
I'm hoping to run the semantic patch as part of our CI pipeline.

In the medium term, I think I can just keep my current version (as above
but extended with positions and a Python error message). Because it
doesn't try to add missing attributes, it doesn't need the attribute name
and doesn't have the double annotation problem.

Thanks a lot for your help!

Cheers,
Paul

> 
> julia
> 
> 
> >
> > Paul
> >
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] Using a macro for variable attributes
  2020-04-28 18:17   ` Paul Chaignon
  2020-04-28 18:41     ` Julia Lawall
@ 2020-04-30 15:40     ` Jaskaran Singh
  2020-05-01 13:45       ` Paul Chaignon
  1 sibling, 1 reply; 7+ messages in thread
From: Jaskaran Singh @ 2020-04-30 15:40 UTC (permalink / raw)
  To: Paul Chaignon, Julia Lawall; +Cc: cocci, paul.chaignon

On Tue, 2020-04-28 at 20:17 +0200, Paul Chaignon wrote:
> Thanks for the quick answer!
> 
> On Tue, Apr 28, 2020 at 07:44:15PM +0200, Julia Lawall wrote:
> > 
> > On Tue, 28 Apr 2020, Paul Chaignon wrote:
> > 
> > > Hi all,
> > > 
> > > I am working on a small semantic patch to annotate specific
> > > variables in
> > > our codebase with __attribute__((aligned(8))). The following
> > > program works
> > > fine.
> > > 
> > >   @r@
> > >   expression e1, e2;
> > >   identifier x;
> > >   @@
> > >   (
> > >     struct \(icmphdr\|icmp6hdr\) x
> > >   + __attribute__((aligned(8)))
> > >     ;
> > >   |
> > >     struct \(icmphdr\|icmp6hdr\) x
> > >   + __attribute__((aligned(8)))
> > >     = ...;
> > >   )
> > >     ... when exists
> > >     ctx_load_bytes(e1, e2, &x, ...)
> > > 
> > > However, when I replace __attribute__((aligned(8))) with our
> > > internal
> > > macro __align_stack_8, it fails with the following error:
> > > 
> > >   plus: parse error:
> > >     File "/home/paul/cilium/contrib/coccinelle/aligned.cocci",
> > > line 7, column 2, charpos = 77
> > >     around = '__align_stack_8',
> > >     whole content = + __align_stack_8
> > > 
> > > I've tried adding '#define __align_stack_8' in a file passed with
> > > --macro-file, without success. Is this a known limitation for
> > > macros or
> > > am I missing something?
> > 
> > Try adding the "metavariable" declaration:
> > 
> > attribute name __align_stack_8;
> 
> Awesome, that worked. And I think I understand: undeclared
> identifiers are by
> default considered symbols, leading to the parse error.
> 
> Unfortunately, my semantic patch now leads to the following changes:
> 
>   -	struct icmphdr icmphdr __align_stack_8;
>   +	struct icmphdr icmphdr __align_stack_8 __align_stack_8;
> 

Hi Paul,

Just FYI if you want to avoid the double attribute problem there,
disable the optional_attributes isomorphism for the rule where you add
the __align_stack_8 attribute. Example:

@disable optional_attributes@
attribute name __align_stack_8;
@@

 int foo
+	__align_stack_8
	= 2;

With this, any attributes that you don't specify in your SmPL won't be
matched in source code either.

> I would normally add a first case to my conjunction to match on
> already-present attributes, but Coccinelle can't match on attributes
> yet.

Granted, attribute matching on Coccinelle's master branch is far from
perfect, but I think it should work for declarations/assignments.
Hopefully I can get these patches out soon :)

Cheers,
Jaskaran.

> Any workaround?
> 
> Paul
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Cocci] Using a macro for variable attributes
  2020-04-30 15:40     ` Jaskaran Singh
@ 2020-05-01 13:45       ` Paul Chaignon
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Chaignon @ 2020-05-01 13:45 UTC (permalink / raw)
  To: Jaskaran Singh; +Cc: cocci, paul.chaignon

On Thu, Apr 30, 2020 at 09:10:17PM +0530, Jaskaran Singh wrote:
> On Tue, 2020-04-28 at 20:17 +0200, Paul Chaignon wrote:
> > Thanks for the quick answer!
> > 
> > On Tue, Apr 28, 2020 at 07:44:15PM +0200, Julia Lawall wrote:
> > > 
> > > On Tue, 28 Apr 2020, Paul Chaignon wrote:

[...]

> > Unfortunately, my semantic patch now leads to the following changes:
> > 
> >   -	struct icmphdr icmphdr __align_stack_8;
> >   +	struct icmphdr icmphdr __align_stack_8 __align_stack_8;
> > 
> 
> Hi Paul,
> 
> Just FYI if you want to avoid the double attribute problem there,
> disable the optional_attributes isomorphism for the rule where you add
> the __align_stack_8 attribute. Example:
> 
> @disable optional_attributes@
> attribute name __align_stack_8;
> @@
> 
>  int foo
> +	__align_stack_8
> 	= 2;
> 
> With this, any attributes that you don't specify in your SmPL won't be
> matched in source code either.

That works great! Thanks a lot!

[...]

Paul
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-28 10:11 [Cocci] Using a macro for variable attributes Paul Chaignon
2020-04-28 17:44 ` Julia Lawall
2020-04-28 18:17   ` Paul Chaignon
2020-04-28 18:41     ` Julia Lawall
2020-04-28 18:58       ` Paul Chaignon
2020-04-30 15:40     ` Jaskaran Singh
2020-05-01 13:45       ` Paul Chaignon

Coccinelle archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/cocci/0 cocci/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 cocci cocci/ https://lore.kernel.org/cocci \
		cocci@systeme.lip6.fr
	public-inbox-index cocci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/fr.lip6.systeme.cocci


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