On Sat, 28 Jan 2023, Luca Coelho wrote:

> On Fri, 2023-01-27 at 21:52 +0100, Julia Lawall wrote:
> >
> > On Fri, 27 Jan 2023, Luca Coelho wrote:
> >
> > > Hi,
> > >
> > > I've been struggling with a change I want to make, so I hope someone
> > > can help.
> > >
> > > Basically I have something like this in the code:
> > >
> > > /* regs below are bits 15:0 */
> > > #define _MIPIA_HSYNC_PADDING_COUNT     (_MIPI_MMIO_BASE(dev_priv) + 0xb028)
> > > #define _MIPIC_HSYNC_PADDING_COUNT     (_MIPI_MMIO_BASE(dev_priv) + 0xb828)
> > > #define MIPI_HSYNC_PADDING_COUNT(port) _MMIO_MIPI(port, _MIPIA_HSYNC_PADDING_COUNT, _MIPIC_HSYNC_PADDING_COUNT)
> > >
> > > As you can see, these macros assume that the calling functions have a
> > > local called dev_priv, and I want to fix that.
> > >
> > > So, I'm trying to find all users of macros that have this construct.
> > > My problem is matching the nesting in the macros.
> > >
> > > I have tried this:
> > >
> > > @macros_noargs@
> > > identifier m;
> > > expression e =~ "dev_priv";
> > > @@
> > > #define m <+...e...+>
> > >
> > > @nested_macros@
> > > identifier macros_noargs.m;
> > > identifier nm;
> > > @@
> > > #define nm(...) <+...m...+>
> > >
> > > @@
> > > identifier nested_macros.nm;
> > > @@
> > > -nm
> > > +foo
> > >
> > > (this last rule is just for testing if anything is matching, obviously)
> > >
> > > But the "nested_macros" rule doesn't seem to work.  Does anyone know
> > > how I can solve this?
> >
> > I think that it works already.  If you add the following code to the .c
> > file, then the call gets changed:
> >
> > int main() {
> >   MIPI_HSYNC_PADDING_COUNT(12);
> > }
> >
> > I think that the problem is that a rule that replaces an identifier by
> > another doesn't match a macro definition.  Is that actually the precise
> > thing you want to do?  Or do you want to do something with the uses of the
> > macro.
>
> This is the initial thing I want to do:
>
> @@
> identifier nested_macros.nm;
> function f;
> identifier dev_priv;
> expression e;
> @@
> f(...) {
> 	...
> -	struct drm_i915_private *dev_priv = e;
> +	struct drm_i915_private *i915 = e;
>
> 	<+...nm(...)...+>
> }
>
> And it indeed works with a handcrafted file for testing (attached as
> i915_cocci_test.c), but it doesn't work IRL, for instance in the
> drivers/gpu/drm/i915/display/vlv_dsi.c file in the kernel (also
> attached)...
>
> I would expect there to be a match in the static void
> bxt_dsi_get_pipe_config() function...

To match the function, the semantic patch has to be able to see the macro
definitions.  They are not in the .c file.  Maybe you need a command line
argument like --all-includes (take into account all of the files mentioned
with #include, but not what they include) or --recursive-includes.  You
can give a search path if needed with -I.

You can use --debug to get some information about what rules are being
matched successfully.

julia


>
>
> > If you want to change the definition, you can also add a rule like this:
> >
> > @@
> > identifier nested_macros.nm;
> > identifier list is;
> > expression e;
> > @@
> >
> > -#define nm(is) e
> > +#define foo(is) e
>
> This is not what I actually want to do, I just created the simplest
> rule I could come up with to actually see the match (and replacement)
> happening. 😉
>
> --
> Cheers,
> Luca.
>