[Cocci] Using a macro for variable attributes

[Cocci] Using a macro for variable attributes

[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

Re: [Cocci] Using a macro for variable attributes

[Julia Lawall]

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

Re: [Cocci] Using a macro for variable attributes

[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

Re: [Cocci] Using a macro for variable attributes

[Julia Lawall]

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

Re: [Cocci] Using a macro for variable attributes

[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

Re: [Cocci] Using a macro for variable attributes

[Jaskaran Singh]

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

Re: [Cocci] Using a macro for variable attributes

[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