Coccinelle archive mirror
 help / color / mirror / Atom feed
From: Mats Kindahl <mats.kindahl@gmail.com>
To: cocci@inria.fr
Subject: [cocci] Depends on with negative matches
Date: Thu, 19 Dec 2024 08:25:46 +0100	[thread overview]
Message-ID: <f0764391-b0d3-48fa-8a54-6edddf8d2c8d@gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 5359 bytes --]

Hi all,

I want have a case where I want to find missing resource releases. The 
real case is a different one, so I am creating a similar case using 
malloc() and want to find any malloc() that is either not freeing the 
memory or returning it to the caller. Here are a few cases:

    #include <stdio.h>
    #include <stdlib.h>

    struct something {
       const char *here;
       int ok;
    };

    /* This should match */
    void func_1() {
       struct something *foo;
       foo = malloc(sizeof(struct something));
       if (!foo) {
         printf("Error %s", foo->here);
         return;
       }
       foo->ok = 1;
    }

    /* This should NOT match */
    struct something *func_2() {
       struct something *foo;
       foo = malloc(sizeof(struct something));
       if (foo) {
         foo->ok = 1;
         return foo;
       }
       printf("Error %s", foo->here);
       return NULL;
    }

    /* This should NOT match */
    struct something *func_3() {
       struct something *foo;
       foo = malloc(sizeof(struct something));
       if (!foo) {
         printf("Error %s", foo->here);
         return NULL;
       }
       foo->ok = 1;
       return foo;
    }

    /* This should match */
    void func_4(struct something *foo) {
       if (!foo)
         foo = malloc(sizeof(struct something));
       foo->ok = 1;
       printf("Error %s", foo->here);
    }

    /* This should NOT match */
    void func_5(struct something *foo) {
       if (!foo)
         foo = malloc(sizeof(struct something));
       foo->ok = 1;
       printf("Error %s", foo->here);
       free(foo);
    }

    /* This should NOT match */
    struct something *func_6(struct something *foo) {
       if (!foo)
         foo = malloc(sizeof(struct something));
       foo->ok = 1;
       printf("Error %s", foo->here);
       return foo;
    }

I tried this semantic patch, but unfortunately it does not seem to 
capture the dependencies correctly.

    // Capture any malloc() to a local variable
    @r@
    local idexpression var;
    position p;
    @@
       var@p = malloc(...);

    // Detect cases that either free the variable or return it. Expect an
    // intermediate NULL check with error handling code that can do
    // anything it likes.
    @r1@
    local idexpression r.var;
    position r.p;
    statement S;
    @@
       var@p = malloc(...);
       ...
       if (var == NULL) S
       ...
    (
       free(var);
    |
       return var;
    )

    // Detect cases that either free the variable or return it. Expect an
    // non-NULL check that then either returns or free the variable. We
    // ignore any code that follows this if statement and assume that is
    // error handling code.
    @r2@
    local idexpression r.var;
    position r.p;
    @@
       var@p = malloc(...);
       ...
       if (var != NULL) {
         ...
    (
         free(var);
         ...
    |
         return var;
    )
       }

    // If neither of the above cases match, then we have a bad malloc, so
    // mark it.
    @depends on !r1 && !r2@
    local idexpression r.var;
    position r.p;
    @@
    * var@p = malloc(...);

Unfortunately, it matches correctly in most cases, but some of the cases 
are not detected correctly.

    mats@abzu:~/lang/cocci/tests$ spatch --sp-file missing_free.cocci
    code/malloc-example.c
    init_defs_builtins: /usr/lib/coccinelle/standard.h
    HANDLING: code/malloc-example.c
    diff =
    --- code/malloc-example.c
    +++ /tmp/cocci-output-172050-d31474-malloc-example.c
    @@ -9,7 +9,6 @@ struct something {
    /* This should match */
    void func_1() {
       struct something *foo;
    -  foo = malloc(sizeof(struct something));
       if (!foo) {
         printf("Error %s", foo->here);
         return;
    @@ -44,7 +43,6 @@ struct something *func_3() {
    /* This should match */
    void func_4(struct something *foo) {
       if (!foo)
    -    foo = malloc(sizeof(struct something));
       foo->ok = 1;
       printf("Error %s", foo->here);
    }
    @@ -52,7 +50,6 @@ void func_4(struct something *foo) {
    /* This should NOT match */
    void func_5(struct something *foo) {
       if (!foo)
    -    foo = malloc(sizeof(struct something));
       foo->ok = 1;
       printf("Error %s", foo->here);
       free(foo);
    @@ -61,7 +58,6 @@ void func_5(struct something *foo) {
    /* This should NOT match */
    struct something *func_6(struct something *foo) {
       if (!foo)
    -    foo = malloc(sizeof(struct something));
       foo->ok = 1;
       printf("Error %s", foo->here);
       return foo;

I tried to look at the kmalloc() examples in the coccinellery and also 
some other examples in the source code that seems to do similar things, 
but none have this case with "either free or return" and multiple cases 
with negative dependencies. It seems like you can get it mostly correct 
if you just keep the first rule only (with the NULL check), but I want 
to capture the "positive branch" case as well, where you check for 
non-NULL and then execute some code that should end with either a free 
or a return.

Anybody have some insight into what I've done wrong or if there is a 
better way to solve this problem?

Best wishes,
Mats Kindahl


[-- Attachment #2: Type: text/html, Size: 8384 bytes --]

             reply	other threads:[~2024-12-19  7:25 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-19  7:25 Mats Kindahl [this message]
2024-12-19  7:53 ` [cocci] Depends on with negative matches Julia Lawall
2024-12-19 18:55   ` Mats Kindahl
2024-12-19 19:54     ` Markus Elfring
2024-12-20 21:54     ` Julia Lawall
2024-12-21  9:08       ` Markus Elfring
2024-12-21 14:24       ` Mats Kindahl
2024-12-21 14:25         ` Julia Lawall
2024-12-21 16:14         ` Markus Elfring
2024-12-19  9:52 ` [cocci] Detecting missing resource releases (with SmPL)? Markus Elfring
2024-12-19 11:32 ` [cocci] Scope for source code analyses " Markus Elfring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f0764391-b0d3-48fa-8a54-6edddf8d2c8d@gmail.com \
    --to=mats.kindahl@gmail.com \
    --cc=cocci@inria.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).