Skip to content
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

Added suppression rules and suppressions file to contrib/valgrind #393

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

ashu3103
Copy link
Contributor

@ashu3103 ashu3103 commented Jan 2, 2024

WORKING IN PROGRESS

@jesperpedersen Kindly review the pull request, Thank you!

About This Commit

  1. Tried to generate the suppression rules as per https://valgrind.org/docs/manual/manual-core.html#manual-core.suppress for memory leak errors by tracing the path from main(int argc, char** argv) function in main.c that leads to malloc() or calloc() or realloc().
  2. Currently, for leakage type matching, the following configuration is done -> match-leak-kinds: all to track all kinds of memory leaks like definite, indirect, possible and reachable

Provide feedback on

  1. The choice of the name of the suppression rules (as I have simply concatenated names of some of the functions in that suppression rule)
  2. Can we incorporate the use of wildcards like (...) in suppression rules to avoid redundancy; for example:
{
   ev_io_start_realloc1
   Memcheck:Leak
   match-leak-kinds: all
   fun:realloc
   fun:ev_realloc
   fun:ev_io_start.cold
   fun:start_management
   fun:main
}

{
   ev_io_start_realloc2
   Memcheck:Leak
   match-leak-kinds: all
   fun:realloc
   fun:ev_realloc
   fun:ev_io_start.cold
   fun:start_mgt
   fun:main
}

{
   ev_io_start_realloc3
   Memcheck:Leak
   match-leak-kinds: all
   fun:realloc
   fun:ev_realloc
   fun:ev_io_start.cold
   fun:start_uds
   fun:main
}

The above code can be compressed to ->

{
   ev_io_start_realloc
   Memcheck:Leak
   match-leak-kinds: all
   fun:realloc
   fun:ev_realloc
   fun:ev_io_start.cold
   ...
   fun:main
}

(There are many such cases)

  1. Any other comments

@jesperpedersen
Copy link
Collaborator

You can use wildcards for all our external dependencies.

However, for leaks flagged for pgagroal you need to fix them

@jesperpedersen jesperpedersen self-requested a review January 5, 2024 17:03
@jesperpedersen jesperpedersen added the feature New feature label Jan 5, 2024
ashu3103 added a commit to ashu3103/pgagroal that referenced this pull request Jan 27, 2024
@ashu3103
Copy link
Contributor Author

@jesperpedersen PTAL

About This Commit

  1. Removed all redundant suppression rules (In which dynamic memory is freed in the code itself)
  2. Removed all non-pgagroal rules for now.

@jesperpedersen
Copy link
Collaborator

There must be stacks in 3rd party libraries that we need to exclude - you had those before.

So, the .supp file need to be everything from 3rd party libraries, and the stuff we don't want to fix in pgagroal. And, the rest should be the fixes to the pgagroal code such that valgrind will run "clean"

Also, squash your commits - https://www.git-tower.com/learn/git/faq/git-squash

ashu3103 added a commit to ashu3103/pgagroal that referenced this pull request Jan 28, 2024
ashu3103 added a commit to ashu3103/pgagroal that referenced this pull request Jan 28, 2024
@jesperpedersen
Copy link
Collaborator

Can you add a README.md file as well - like https://github.com/pgmoneta/pgmoneta/blob/main/contrib/valgrind/README.md

ashu3103 added a commit to ashu3103/pgagroal that referenced this pull request Jan 30, 2024
@ashu3103
Copy link
Contributor Author

Can you add a README.md file as well - like https://github.com/pgmoneta/pgmoneta/blob/main/contrib/valgrind/README.md

@jesperpedersen PTAL

@jesperpedersen
Copy link
Collaborator

There are still 3rd party traces,

==3388495== 32 bytes in 1 blocks are still reachable in loss record 4 of 13
==3388495==    at 0x484A074: realloc (vg_replace_malloc.c:1690)
==3388495==    by 0x4859BA3: ev_realloc (ev.c:2053)
==3388495==    by 0x4859D6B: ev_feed_event.cold (ev.c:2323)
==3388495==    by 0x485C09A: fd_event_nocheck (ev.c:2368)
==3388495==    by 0x485C09A: fd_event (ev.c:2380)
==3388495==    by 0x485C09A: epoll_poll (ev_epoll.c:215)
==3388495==    by 0x485E913: ev_run (ev.c:4161)
==3388495==    by 0x485E913: ev_run (ev.c:4021)
==3388495==    by 0x10B888: ev_loop (ev.h:841)
==3388495==    by 0x10F1E6: main (main.c:1151)
==3388495== 
==3388495== 34 bytes in 1 blocks are still reachable in loss record 5 of 13
==3388495==    at 0x484280F: malloc (vg_replace_malloc.c:442)
==3388495==    by 0x4024CFF: malloc (rtld-malloc.h:56)
==3388495==    by 0x4024CFF: strdup (strdup.c:42)
==3388495==    by 0x4008A9B: _dl_map_object (dl-load.c:2179)
==3388495==    by 0x400C75B: dl_open_worker_begin (dl-open.c:578)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x400BE8F: dl_open_worker (dl-open.c:803)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x400C2E3: _dl_open (dl-open.c:905)
==3388495==    by 0x4FE4663: dlopen_doit (dlopen.c:56)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x4001678: _dl_catch_error (dl-catch.c:256)
==3388495==    by 0x4FE4142: _dlerror_run (dlerror.c:138)
==3388495== 
==3388495== 34 bytes in 1 blocks are still reachable in loss record 6 of 13
==3388495==    at 0x484280F: malloc (vg_replace_malloc.c:442)
==3388495==    by 0x400BC00: UnknownInlinedFun (rtld-malloc.h:56)
==3388495==    by 0x400BC00: _dl_new_object (dl-object.c:199)
==3388495==    by 0x400708E: _dl_map_object_from_fd (dl-load.c:1053)
==3388495==    by 0x4008B48: _dl_map_object (dl-load.c:2246)
==3388495==    by 0x400C75B: dl_open_worker_begin (dl-open.c:578)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x400BE8F: dl_open_worker (dl-open.c:803)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x400C2E3: _dl_open (dl-open.c:905)
==3388495==    by 0x4FE4663: dlopen_doit (dlopen.c:56)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x4001678: _dl_catch_error (dl-catch.c:256)
==3388495== 
==3388495== 144 bytes in 1 blocks are still reachable in loss record 8 of 13
==3388495==    at 0x4849E60: calloc (vg_replace_malloc.c:1595)
==3388495==    by 0x40140DF: UnknownInlinedFun (rtld-malloc.h:44)
==3388495==    by 0x40140DF: _dl_check_map_versions (dl-version.c:280)
==3388495==    by 0x400C80A: dl_open_worker_begin (dl-open.c:646)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x400BE8F: dl_open_worker (dl-open.c:803)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x400C2E3: _dl_open (dl-open.c:905)
==3388495==    by 0x4FE4663: dlopen_doit (dlopen.c:56)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x4001678: _dl_catch_error (dl-catch.c:256)
==3388495==    by 0x4FE4142: _dlerror_run (dlerror.c:138)
==3388495==    by 0x4FE471E: UnknownInlinedFun (dlopen.c:71)
==3388495==    by 0x4FE471E: dlopen@@GLIBC_2.34 (dlopen.c:81)
==3388495== 
==3388495== 768 bytes in 1 blocks are still reachable in loss record 10 of 13
==3388495==    at 0x484A074: realloc (vg_replace_malloc.c:1690)
==3388495==    by 0x4859BA3: ev_realloc (ev.c:2053)
==3388495==    by 0x485A917: epoll_init (ev_epoll.c:274)
==3388495==    by 0x485A917: epoll_init (ev_epoll.c:264)
==3388495==    by 0x485A917: loop_init (ev.c:3339)
==3388495==    by 0x485ADA4: ev_default_loop (ev.c:3710)
==3388495==    by 0x10DF38: main (main.c:942)
==3388495== 
==3388495== 1,258 bytes in 1 blocks are still reachable in loss record 11 of 13
==3388495==    at 0x4849E60: calloc (vg_replace_malloc.c:1595)
==3388495==    by 0x400B91D: UnknownInlinedFun (rtld-malloc.h:44)
==3388495==    by 0x400B91D: _dl_new_object (dl-object.c:92)
==3388495==    by 0x400708E: _dl_map_object_from_fd (dl-load.c:1053)
==3388495==    by 0x4008B48: _dl_map_object (dl-load.c:2246)
==3388495==    by 0x400C75B: dl_open_worker_begin (dl-open.c:578)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x400BE8F: dl_open_worker (dl-open.c:803)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x400C2E3: _dl_open (dl-open.c:905)
==3388495==    by 0x4FE4663: dlopen_doit (dlopen.c:56)
==3388495==    by 0x4001522: _dl_catch_exception (dl-catch.c:237)
==3388495==    by 0x4001678: _dl_catch_error (dl-catch.c:256)

Also add yourself to the AUTHORS file

ashu3103 added a commit to ashu3103/pgagroal that referenced this pull request Jan 31, 2024
Modified AUTHORS file
ashu3103 added a commit to ashu3103/pgagroal that referenced this pull request Feb 1, 2024
- added suppression rules in contrib/valgrind/pgagroal.supp file
- added README for valgrind support in pgagroal
- modified AUTHOR file
ashu3103 added a commit to ashu3103/pgagroal that referenced this pull request Feb 4, 2024
- added suppression rules in contrib/valgrind/pgagroal.supp file
- added README for valgrind support in pgagroal
- modified AUTHOR file
- Added suppression rules in contrib/valgrind/pgagroal.supp file
- Added README page for valgrind support
- Modified AUTHORS file
@ashu3103
Copy link
Contributor Author

ashu3103 commented Feb 4, 2024

@jesperpedersen PTAL

@jesperpedersen jesperpedersen merged commit 981b4ff into agroal:master Feb 7, 2024
2 checks passed
@jesperpedersen
Copy link
Collaborator

Merged.

Thanks for your contribution ! And, welcome to the community !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants