[Cocci] Analysis for Linux source files

[Cocci] Analysis for Linux source files

[Nic Volanschi (R&D)]

If this may help, I stumbled against the same error in a different
setting, but I think the ultimate cause is the same:

$ cat in.c
int foo()
{
  register rI;
  return rI;
}

$ cat in.cocci
@f1@
identifier f;
position p;
type T;
@@
* T at p f;

The reported error is the same: << Fatal error: exception Failure("empty
list, max_min_ii_by_pos") >>
I localized the problem on the variable declaration <<register rI;>>
where the type is missing (int by default). As soon as you add the
missing "int", the error disappears. 
In fact, the error seems somewhat justified, as you are taking the
position of an inexistent type.

I guess that in the big file indicated by Markus, the problematic
declaration is:


> static DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };


where we have the same missing, implicit int return type.

Hope it helps,
Nic.

On Wed, 2013-04-03 at 12:00 +0200, cocci-request at systeme.lip6.fr wrote:

> ----------------------------------------------------------------------
> 
> Message: 1
> Date: Tue, 02 Apr 2013 17:37:30 +0200
> From: SF Markus Elfring <elfring@users.sourceforge.net>



> Would you like to reproduce my issue with the following SmPL filter pattern on
> the source file
> "https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/init/calibrate.c?id=8595c539f0360477189eef91f6337ba44962f72d"?



> ------------------------------
> 
> Message: 3
> Date: Tue, 2 Apr 2013 18:21:22 +0200 (CEST)
> From: Julia Lawall <julia.lawall@lip6.fr>



> I get the problem on the following code:
> 
> irqreturn_t
> nouveau_irq_handler(DRM_IRQ_ARGS)
> {
>         struct drm_device *dev = arg;
>         struct nouveau_device *device = nouveau_dev(dev);
>         struct nouveau_mc *pmc = nouveau_mc(device);
>         u32 stat;
> 
>         stat = nv_rd32(device, 0x000100);
>         if (stat == 0 || stat == ~0)
>                 return IRQ_NONE;
> 
>         nv_subdev(pmc)->intr(nv_subdev(pmc));
>         return IRQ_HANDLED;
> }
> 
> I would suspect that there is some inconsistency in how it is parsing this
> kind of argument list.  It seems to parse it OK, but then not be able to
> match it against a pattern like ...,x,...  I can look at it in more detail
> tomorrow.
> 
> julia
> 


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20130408/f4c99e4b/attachment.html>

[Cocci] Analysis for Linux source files

[Julia Lawall]

Thanks very much for the report.  I thought that my fix had solved the
problem, but it doesn't seem to have been a complete solution, because I
still see the problem on this example.

I will look into it.

julia

On Mon, 8 Apr 2013, Nic Volanschi (R&D) wrote:

> If this may help, I stumbled against the same error in a different setting, but I think the ultimate cause is the same:
>
> $ cat in.c
> int foo()
> {
> ? register rI;
> ? return rI;
> }
>
> $ cat in.cocci
> @f1@
> identifier f;
> position p;
> type T;
> @@
> * T at p f;
>
> The reported error is the same: << Fatal error: exception Failure("empty list, max_min_ii_by_pos") >>
> I localized the problem on the variable declaration <<register rI;>> where the type is missing (int by default). As soon as
> you add the missing "int", the error disappears.
> In fact, the error seems somewhat justified, as you are taking the position of an inexistent type.
>
> I guess that in the big file indicated by Markus, the problematic declaration is:
>
> static DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
>
>
> where we have the same missing, implicit int return type.
>
> Hope it helps,
> Nic.
>
> On Wed, 2013-04-03 at 12:00 +0200, cocci-request at systeme.lip6.fr wrote:
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Tue, 02 Apr 2013 17:37:30 +0200
> From: SF Markus Elfring <elfring@users.sourceforge.net>
>
>
> Would you like to reproduce my issue with the following SmPL filter pattern on
> the source file
> "https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/init/calibrate.c?id=8595c539f0360477189eef91f6337b
> a44962f72d"?
>
>
> ------------------------------
>
> Message: 3
> Date: Tue, 2 Apr 2013 18:21:22 +0200 (CEST)
> From: Julia Lawall <julia.lawall@lip6.fr>
>
>
> I get the problem on the following code:
>
> irqreturn_t
> nouveau_irq_handler(DRM_IRQ_ARGS)
> {
>         struct drm_device *dev = arg;
>         struct nouveau_device *device = nouveau_dev(dev);
>         struct nouveau_mc *pmc = nouveau_mc(device);
>         u32 stat;
>
>         stat = nv_rd32(device, 0x000100);
>         if (stat == 0 || stat == ~0)
>                 return IRQ_NONE;
>
>         nv_subdev(pmc)->intr(nv_subdev(pmc));
>         return IRQ_HANDLED;
> }
>
> I would suspect that there is some inconsistency in how it is parsing this
> kind of argument list.  It seems to parse it OK, but then not be able to
> match it against a pattern like ...,x,...  I can look at it in more detail
> tomorrow.
>
> julia
>
>
>
>

[Cocci] Analysis for Linux source files

[Julia Lawall]

Actually, what result would you prefer?  In the previous fix that I made,
I just caused the missing type, which was hidden under a macro, to make
the match fail.  In this case, though perhaps you would prefer T to be
bound to int?  That might cause some problemes, though, because it is not
a real "int" in the source code...

julia

On Mon, 8 Apr 2013, Nic Volanschi (R&D) wrote:

> If this may help, I stumbled against the same error in a different setting, but I think the ultimate cause is the same:
>
> $ cat in.c
> int foo()
> {
> ? register rI;
> ? return rI;
> }
>
> $ cat in.cocci
> @f1@
> identifier f;
> position p;
> type T;
> @@
> * T at p f;
>
> The reported error is the same: << Fatal error: exception Failure("empty list, max_min_ii_by_pos") >>
> I localized the problem on the variable declaration <<register rI;>> where the type is missing (int by default). As soon as
> you add the missing "int", the error disappears.
> In fact, the error seems somewhat justified, as you are taking the position of an inexistent type.
>
> I guess that in the big file indicated by Markus, the problematic declaration is:
>
> static DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
>
>
> where we have the same missing, implicit int return type.
>
> Hope it helps,
> Nic.
>
> On Wed, 2013-04-03 at 12:00 +0200, cocci-request at systeme.lip6.fr wrote:
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Tue, 02 Apr 2013 17:37:30 +0200
> From: SF Markus Elfring <elfring@users.sourceforge.net>
>
>
> Would you like to reproduce my issue with the following SmPL filter pattern on
> the source file
> "https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/init/calibrate.c?id=8595c539f0360477189eef91f6337b
> a44962f72d"?
>
>
> ------------------------------
>
> Message: 3
> Date: Tue, 2 Apr 2013 18:21:22 +0200 (CEST)
> From: Julia Lawall <julia.lawall@lip6.fr>
>
>
> I get the problem on the following code:
>
> irqreturn_t
> nouveau_irq_handler(DRM_IRQ_ARGS)
> {
>         struct drm_device *dev = arg;
>         struct nouveau_device *device = nouveau_dev(dev);
>         struct nouveau_mc *pmc = nouveau_mc(device);
>         u32 stat;
>
>         stat = nv_rd32(device, 0x000100);
>         if (stat == 0 || stat == ~0)
>                 return IRQ_NONE;
>
>         nv_subdev(pmc)->intr(nv_subdev(pmc));
>         return IRQ_HANDLED;
> }
>
> I would suspect that there is some inconsistency in how it is parsing this
> kind of argument list.  It seems to parse it OK, but then not be able to
> match it against a pattern like ...,x,...  I can look at it in more detail
> tomorrow.
>
> julia
>
>
>
>

[Cocci] Analysis for Linux source files

[Nic Volanschi (R&D)]

I guess that failing the match is OK. So taking the position of a type
could be a way of imposing an explicit type, isn't it.

Nic.

On Mon, 2013-04-08 at 16:56 +0200, Julia Lawall wrote:

> Actually, what result would you prefer?  In the previous fix that I made,
> I just caused the missing type, which was hidden under a macro, to make
> the match fail.  In this case, though perhaps you would prefer T to be
> bound to int?  That might cause some problemes, though, because it is not
> a real "int" in the source code...
> 
> julia
> 
> On Mon, 8 Apr 2013, Nic Volanschi (R&D) wrote:
> 
> > If this may help, I stumbled against the same error in a different setting, but I think the ultimate cause is the same:
> >
> > $ cat in.c
> > int foo()
> > {
> >   register rI;
> >   return rI;
> > }
> >
> > $ cat in.cocci
> > @f1@
> > identifier f;
> > position p;
> > type T;
> > @@
> > * T at p f;
> >
> > The reported error is the same: << Fatal error: exception Failure("empty list, max_min_ii_by_pos") >>
> > I localized the problem on the variable declaration <<register rI;>> where the type is missing (int by default). As soon as
> > you add the missing "int", the error disappears.
> > In fact, the error seems somewhat justified, as you are taking the position of an inexistent type.
> >
> > I guess that in the big file indicated by Markus, the problematic declaration is:
> >
> > static DEFINE_PER_CPU(unsigned long, cpu_loops_per_jiffy) = { 0 };
> >
> >
> > where we have the same missing, implicit int return type.
> >
> > Hope it helps,
> > Nic.
> >
> > On Wed, 2013-04-03 at 12:00 +0200, cocci-request at systeme.lip6.fr wrote:
> >
> > ----------------------------------------------------------------------
> >
> > Message: 1
> > Date: Tue, 02 Apr 2013 17:37:30 +0200
> > From: SF Markus Elfring <elfring@users.sourceforge.net>
> >
> >
> > Would you like to reproduce my issue with the following SmPL filter pattern on
> > the source file
> > "https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/init/calibrate.c?id=8595c539f0360477189eef91f6337b
> > a44962f72d"?
> >
> >
> > ------------------------------
> >
> > Message: 3
> > Date: Tue, 2 Apr 2013 18:21:22 +0200 (CEST)
> > From: Julia Lawall <julia.lawall@lip6.fr>
> >
> >
> > I get the problem on the following code:
> >
> > irqreturn_t
> > nouveau_irq_handler(DRM_IRQ_ARGS)
> > {
> >         struct drm_device *dev = arg;
> >         struct nouveau_device *device = nouveau_dev(dev);
> >         struct nouveau_mc *pmc = nouveau_mc(device);
> >         u32 stat;
> >
> >         stat = nv_rd32(device, 0x000100);
> >         if (stat == 0 || stat == ~0)
> >                 return IRQ_NONE;
> >
> >         nv_subdev(pmc)->intr(nv_subdev(pmc));
> >         return IRQ_HANDLED;
> > }
> >
> > I would suspect that there is some inconsistency in how it is parsing this
> > kind of argument list.  It seems to parse it OK, but then not be able to
> > match it against a pattern like ...,x,...  I can look at it in more detail
> > tomorrow.
> >
> > julia
> >
> >
> >
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20130408/0b5f7eeb/attachment-0001.html>

[Cocci] Analysis for Linux source files

[Julia Lawall]

Attached is a patch.  The line numbers may be off, but it should apply to
the latest version.  It causes a match failure for the following semantic
patch:

@r@
type T;
identifier x;
position p;
@@

T at p x;

@script:python@
T << r.T;
@@

print "before"
print T
print "after"

when applied to the following code:

int main () {
  register x;
  return 0;
}

The type metavariable would have been matched to the implicit "int", but
this implicit int doesn't have a position, so the match of the position
variable fails.  On the other hand, there is no more crash.

julia
-------------- next part --------------
diff --git a/engine/cocci_vs_c.ml b/engine/cocci_vs_c.ml
index 4d12515..26a479c 100644
--- a/engine/cocci_vs_c.ml
+++ b/engine/cocci_vs_c.ml
@@ -2820,7 +2820,13 @@ and (fullTypebis: (A.typeC, Ast_c.fullType) matcher) =
 	match ty with
 	  B.NoType -> false
 	| _ -> true in
-      if type_present
+      let position_required_but_unavailable =
+	match A.get_pos_var ida with
+	  [] -> false
+	| _ ->
+	    let (tyq, (ty, tyii)) = typb in
+	    List.for_all Ast_c.is_fake tyii in
+      if type_present && not position_required_but_unavailable
       then
 	let max_min _ =
 	  Lib_parsing_c.lin_col_by_pos (Lib_parsing_c.ii_of_type typb) in

[Cocci] Handling of omitted data type specification?

[SF Markus Elfring]

> In the previous fix that I made, I just caused the missing type,
> which was hidden under a macro, to make the match fail.

How do you think about to make this detail configurable?


> In this case, though perhaps you would prefer T to be bound to int?

Does the C programming language specify that integers are used as the default
data type (or if modifiers are used)?
http://stackoverflow.com/questions/2099830/unsigned-keyword-in-c#2099873
https://www.cs50.net/resources/cppreference.com/data_types.html
http://en.cppreference.com/w/cpp/language/types


> That might cause some problemes, though, because it is not a real "int"
> in the source code...

Does the semantic patch language support to filter for omitted data types?

Would you like to distinguish the use case if this item was just forgotten or
intentionally omitted?

Regards,
Markus

[Cocci] Handling of omitted data type specification?

[Julia Lawall]

On Mon, 8 Apr 2013, SF Markus Elfring wrote:

> > In the previous fix that I made, I just caused the missing type,
> > which was hidden under a macro, to make the match fail.
>
> How do you think about to make this detail configurable?

On this case, there is nothing to configure.  If there is no known type,
then matching a type variable cannot succeed.

> > In this case, though perhaps you would prefer T to be bound to int?
>
> Does the C programming language specify that integers are used as the default
> data type (or if modifiers are used)?
> http://stackoverflow.com/questions/2099830/unsigned-keyword-in-c#2099873
> https://www.cs50.net/resources/cppreference.com/data_types.html
> http://en.cppreference.com/w/cpp/language/types

Coccibelle already takes case of eg signed x;  It should do the same for
register, for consistency.

> > That might cause some problemes, though, because it is not a real "int"
> > in the source code...
>
> Does the semantic patch language support to filter for omitted data types?
>
> Would you like to distinguish the use case if this item was just forgotten or
> intentionally omitted?

Perhaps it is useful.  There may be an isomorphism for deciding whether or
not int is assumed in the signed etc case.

julia