Valgrind Support Queries

[ashu3103]

Hi @jesperpedersen, Continuing the discussion from email -

Ideally there should be no "pgagroal" definitions in the .supp file --
everything should be fixed.

Can you again specify what kind of pgagroal definitions in the pgagroal.supp file are not allowed (with respect to commit in #393), like the idea of pgagroal definitions is not clear to me.

I would appreciate if you could provide an example/reference to further elaborate.

Thanks

[jesperpedersen]

You should fix something like https://github.com/agroal/pgagroal/pull/393/files#diff-5f8b9d4a62f8f10ba4b8fe6bbb1c24b440f7df44378436e5efbada96592df835R6 so it doesn't show up.

So, all pgagroal rules needs to be looked at.

[ashu3103]

Okay, So that means for any suppression rule, if any function in the call stack is used in a definition expression, we have to discard that rule

Like in the above example -

Highlighted Rule-

{
	pgagroal_read_superuser_conf_master_key_base64_decode
	Memcheck:Leak
	match-leak-kinds: all
	fun:calloc
	fun:pgagroal_base64_decode
	fun:pgagroal_get_master_key
	fun:pgagroal_read_superuser_configuration
	fun:main
}

So here pgagroal_read_superuser_configuration() is used in a definition expression ->
ret = pgagroal_read_superuser_configuration(shmem, superuser_path); inside main function, so it has to be discarded.

[jesperpedersen]

Yes, because it means there is a leak within our code, and we should fix that.

In the case above - where is pgagroal_read_superuser_conf_master_key_base64_decode called from ? And where should the allocation be free'd

[ashu3103]

Okay,

So the definition/name of the supp rule should be such that it should reflect the function that caused the leak and the function where the leak is intended to be free, right?

like here ‘pgagroal_read_superuser_conf_master_key_base64_decode’ could not capture where the leak would be freed

[jesperpedersen]

Apparently there is a leak with the pgagroal_read_superuser_conf_master_key_base64_decode call stack. That should be fixed such that the rule can be removed from the .supp file.

The .supp should be as small as possible.

[ashu3103]

Ahh, finally got your point 🤕

On it!!

Thanks

[ashu3103]

Updates on issue #393 @jesperpedersen

1. I have fixed/removed all extra/redundant suppression rules from .supp file in which allocated memory is properly free'd in the code itself (Will make the commit on the same PR).

2. In non-pgagroal rules as the library functions of libev is dynamically loaded from .so files the errors are not properly supressed as per the rule for example -

suppressions generated without suppression file (just --gen-suppressions=all)-

corresponding suppression rule (*The functions in the call stack are as per the libev library code repository) -

In the final suppression report these rules are not reflected may because the Valgrind is not able to store the debug information of the leaks reports involving shared objects/libraries (reference: https://valgrind.org/docs/manual/faq.html#faq.unhelpful) and also because the shared libraries may be not compiled and linked with -g flag (reference: https://stackoverflow.com/questions/9217592/referencing-source-files-of-shared-libraries-in-valgrind).

[jesperpedersen]

Sounds like good progress.

We can exclude all non-pgagroal stuff for sure.

[jesperpedersen]

Once you are happy with everything send a "PTAL" on the pull request(s)

[ashu3103]

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

@jesperpedersen, I have added these rules but I am facing a problem in tracing the call stack functions of the 3rd party libraries as its showing ??? instead of the actual function name or the point of invoke like the line number or the file name.

I have tried searching a lot but couldn't find an appropriate solution to this.

[ashu3103]

I am using Ubuntu 20.04, kernel version: 5.15.0-89-generic

[jesperpedersen]

Do you have https://launchpad.net/ubuntu/noble/+package/libev4-dbgsym ?

[ashu3103]

Nope, I was using libev-dev ....I can try building with that

[jesperpedersen]

You need -dbgsym for all dependencies

[ashu3103]

Noted!!