Skip to content

[Clang SA]: custom malloc/free checker #76861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mat-c opened this issue Jan 3, 2024 · 14 comments · Fixed by #98941
Closed

[Clang SA]: custom malloc/free checker #76861

mat-c opened this issue Jan 3, 2024 · 14 comments · Fixed by #98941
Labels
clang:static analyzer enhancement Improving things as opposed to bug fixing, e.g. new or missing feature good first issue https://github.com/llvm/llvm-project/contribute

Comments

@mat-c
Copy link

mat-c commented Jan 3, 2024

Hi,

today unix.malloc can be customized with attribute with
__attribute((ownership_returns(malloc, size_pos))
__attribute((ownership_takes(malloc, free_ptr_pos)))

But in some system there can be several allocator. I could be usefull to check that malloc_1 are free with free_1, malloc_2, free with free_2.

Also how do you model custom realloc ?

@github-actions github-actions bot added the clang Clang issues not falling into any other category label Jan 3, 2024
@EugeneZelenko EugeneZelenko added clang:static analyzer and removed clang Clang issues not falling into any other category labels Jan 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 3, 2024

@llvm/issue-subscribers-clang-static-analyzer

Author: None (mat-c)

Hi,

today unix.malloc can be customized with attribute with
__attribute((ownership_returns(malloc, size_pos))
__attribute((ownership_takes(malloc, free_ptr_pos)))

But in some system there can be several allocator. I could be usefull to check that malloc_1 are free with free_1, malloc_2, free with free_2.

Also how do you model custom realloc ?

@steakhal
Copy link
Contributor

steakhal commented Jan 3, 2024

AFAIK CSA might accept, but for sure does not check the semantics of the attributes as expected.
It shouldn't be hard implement though. It might be a good first issue.

@steakhal steakhal added the good first issue https://github.com/llvm/llvm-project/contribute label Jan 3, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 3, 2024

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. In the comments of the issue, request for it to be assigned to you.
  2. Fix the issue locally.
  3. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  4. Create a Git commit.
  5. Run git clang-format HEAD~1 to format your changes.
  6. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

@llvmbot
Copy link
Member

llvmbot commented Jan 3, 2024

@llvm/issue-subscribers-good-first-issue

Author: None (mat-c)

Hi,

today unix.malloc can be customized with attribute with
__attribute((ownership_returns(malloc, size_pos))
__attribute((ownership_takes(malloc, free_ptr_pos)))

But in some system there can be several allocator. I could be usefull to check that malloc_1 are free with free_1, malloc_2, free with free_2.

Also how do you model custom realloc ?

@steakhal steakhal added the enhancement Improving things as opposed to bug fixing, e.g. new or missing feature label Jan 3, 2024
@changkhothuychung
Copy link
Contributor

I would like to take this issue if this is available! And would appreciate if there can be some pointers how to get started.

@Sh0g0-1758
Copy link
Member

Hello, I am new to LLVM and would like to work on this issue.

@x14ngch3n
Copy link

Hello, I have some experience in developing ClangSA and would like to work on this issue

@EugeneZelenko
Copy link
Contributor

@cascades-sjtu: Just create pull request and mention it on this page.

@kheelz
Copy link

kheelz commented Feb 12, 2024

Hi, it's my first work in the LLVM project and I would like to try to fix this one.

@Atousa
Copy link
Contributor

Atousa commented Feb 29, 2024

@junglipe are you working on this issue ?

@Atousa
Copy link
Contributor

Atousa commented Feb 29, 2024

@mat-c does anyone is working on that issue?

@Sh0g0-1758
Copy link
Member

@steakhal , does this issue aim to explore the implementation of a custom realloc also?

@Sh0g0-1758
Copy link
Member

Sh0g0-1758 commented Mar 2, 2024

I suppose the changes need to be implemented in /clang/lib/StaticAnalyzer/MallocChecker.cpp, particularly MismatchedDeallocatorChecker needs to be modified.

@Sh0g0-1758
Copy link
Member

Sh0g0-1758 commented Mar 2, 2024

So to be clear about what needs to be done here, the code below should raise a warning that malloc_1 should be freed with free_1 only : But then how exactly are you identifying as to which free belongs to which user malloc implementation? Since these are just variables, or maybe we are limiting ourselves to names like _1 only.

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

// Allocator 1: Custom malloc and free functions
void* malloc_1(size_t size) {
    return malloc(size);
}

void free_1(void* ptr) {
    free(ptr);
}

// Allocator 2: Custom malloc and free functions
void* malloc_2(size_t size) {
    return malloc(size);
}

void free_2(void* ptr) {
    free(ptr);
}

int main() {
    // Using Allocator 1
    int* ptr1 = (int*)malloc_1(sizeof(int));
    if (ptr1 != NULL) {
        *ptr1 = 10;
        printf("Allocated memory with allocator 1 successfully!\n");
    } else {
        printf("Memory allocation with allocator 1 failed!\n");
        return -1;
    }

    // Using Allocator 2
    int* ptr2 = (int*)malloc_2(sizeof(int));
    if (ptr2 != NULL) {
        *ptr2 = 20;
        printf("Allocated memory with allocator 2 successfully!\n");
    } else {
        printf("Memory allocation with allocator 2 failed!\n");
        return -1;
    }

    // Deallocate memory allocated with Allocator 1
    free_2(ptr1);
    printf("Memory deallocated using allocator 1.\n");

    // Deallocate memory allocated with Allocator 2
    free_2(ptr2);
    printf("Memory deallocated using allocator 2.\n");

    return 0;
}

yuxuanchen1997 pushed a commit that referenced this issue Jul 25, 2024
)

Summary:
Add support for checking mismatched ownership_returns/ownership_takes attributes.

Closes #76861

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250804
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer enhancement Improving things as opposed to bug fixing, e.g. new or missing feature good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants