Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Leak Sanitizer - how to suppress leaks



On 13.09.2019 14:03, Robert Elz wrote:

>   | I think we need to specify the definition. Leak is a memory object
>   | without a pointer referencing it.
> 
> I can accept that as a definition.   But we also need to recognise that
> there are no leaks after the program has finished.   And the program has
> finished as soon as any of exit()/exec*()/_exit() is called (and succeeds)
> or when main() returns.   After that there's nothing left of interest.
> Leak detection needs to happen before one of those events occurs.

No. There are destructors, there are atexit and atexit-like (per-thread)
callbacks.

We still can register new atexit entry from executing atexit handler. We
can legally allocate resources in atexit memory and free it in another
atexit.

It's similar with destructors.

It's not something that happens frequently, but it shows that main() is
not special. On the other hand fork(2)/exit(3)/exec(3) are special cases.

> 
>   | A special type of a leak is referencing memory that is no longer needed
>   | and could be freed.
> 
> That one will be interesting to find...
> 

Probably Coverity or a similar tool.

> 
>   | From the compiler point of view it is only slightly different than any
>   | regular "int foo(int, char**)".
> 
> From the compiler's viewpoint you're right, main() is just a function.
> But from the overall system's viewpoint it isn't.
> 

Sanitizers are a compiler feature, not looking at it from system's
viewpoint.

>   | From sanitizer point of view it is not distinguishable and we cannot
>   | (easily) make any conditions based on this.
> 
> This is a problem, as return from main, and a call to exit() are
> equivalent.   Perhaps we should stop returning from main, and always
> explicitly call exit() ?
> 

As already noted, main() is not special neither interesting.

>   | Actually globals are not used in LSan and can be ignored.
> 
> I don't know how to interpret that.   In many programs the pointers
> to allocated memory are stored in (or found from other data stored in)
> global vars.
> 
>   | Dynamic linker execution time is a part of execution time of the whole
>   | application.
> 
> Of course.
> 
>   | This implementation of __suppress_leak() takes 9-10 amd64 CPU
>   | instructions for each __suppress_leak() call.
> 
> What does each of those calls accomplish?   That is, how many of
> them are needed?
> 

It was proposed to mark a resource as legitimate leak.

>   | Before getting measurable performance impact of __suppress_leak() we
>   | will can go out of memory..
> 
> It isn't the call itself which is probably the issue, but traversing
> all the data structs (some of which may not have been used in ages, and
> might be paged out) in order to find all the pointers to the memory that
> is allocated.   Alternatively, we could do:
> 
> 	void *
> 	xmalloc(size_t n)
> 	{
> 		void *p = malloc(n);
> 
> 		if (p == NULL) {
> 			err(1, "....");	/* or whatever is appropriate *.
> 		}
> 
> 		__suppress_leak(p);
> 
> 		return p;
> 	}
> 
> but is that going to help anything at all, ever?
> 

I was thinking about a variation of malloc(3) that can leak.
Unfortunately there are multiple types of allocators and certain
functions allocate resources internally, inside libraries.


>   | LSan does not look look for unclosed file descriptors so we can skip
>   | discussing the file descriptor leaks as offtopic here.
> 
> We can - but the issue is essentially the same.
> 

Valgrind could detect file descriptor leak. We can reschedule it for later.

Coverity should be able to perform the same task.

A problem here is that we inherit file descriptors from parent.
Detecting it and adding missing close-on-exec is however a good idea for
optimization in future.

LSan does not detect it as of today so it

>   | > If not, your program, according to you, is leaking file descriptors.
>   | No.
> 
> What is the difference?
> 

It is hypercorrectness, same like checking errno from printf(3) and
attempt to change the process of plugging resource leaks to absurd.

>   | Actually whenever I open a file descriptor I try to close it when no
>   | longer needed.
> 
> That's fine, and if you were to do, in some function
> 
> 	func()
> 	{
> 		char buf[BIG_ENOUGH];
> 		int fd;
> 
> 		fd = open("myfile", 0);
> 		if (fd < 0)	/* whatever */;
> 		read(fd, buf, BIG_ENOUGH);
> 
> 		return buf[0] == '#';
> 	}
> 
> I think we'd all agree that there's a fd leak in there.   Just the
> same as if fd was a pointer, and the open() was malloc() (and some other
> code replaced the read) it would be a memory leak.
> 

err(3) / abort(3) etc, are abnormal exit. Abnormal one differs from
normal/clean. For the same reason nobody registers a SIGFPU signal
handler to free resources.

The philosophy differs in fault tolerant environments, but it would by a
different topic.

> 
>   |  - use free(3) optionally wrapped in #ifdef _NO_LEAKS
>   |  - use __suppress_leak() as a generic approach for all leak detecting
>   | software with a negligible cost on
>   |  - use per-tool approach
> 
>      - understand that exit() is "free everything"
> 
> That one is my preferred choice. 

OK, thank you. So to sum it up. Plugging resource leaks costs time and
human effort and this is what is not wanted by some people. For those
who find it useless are not forced to do it, so can ignore it. For
others it can be useful activity.

Thanks! I will go for __NO_LEAKS ifdef.

Attachment: signature.asc
Description: OpenPGP digital signature



Home | Main Index | Thread Index | Old Index