On Fri, May 21, 2021 at 1:01 PM Calvin Buckley <cal...@cmpct.info> wrote: > > Hi internals@, > > I maintain an extension and I suspect there are some issues in the code. As > such, I’ve been trying various tools to try to make it easier to catch the > issues. (For the curious: I’ve tried *San, which I feel doesn’t work very > well unless you /totally control/ the entire stack, which I didn’t have the > luxury of. I also tried Valgrind, but I need ro revisit this to deal with > possible false positives in the library.) This time, I decided to try static > analysis through LLVM. > > Luckily, clang-analyzer is pretty simple. Just prepending “scan-build” to my > make invocation. Easy, right? Unfortunately, I noticed that due to an > inconsistency in the codebase (a use of realloc instead of erealloc), that it > doesn’t seem to account for i.e emalloc vs. malloc. Possible leaks “went > away” from the output when I converted them to the PHP memory management > functions. > > Has anyone ever used clang-analyzer with PHP before? I noticed there was some > tooling for a previous PHP transition [1], but I don’t know if anyone’s > tackled the low-hanging fruit of memory functions. I suppose I could just > redefine emalloc and friends, but I feel that would probably be inaccurate > with things like zend_string. > > Regards, > Calvin > > [1]: https://github.com/johannes/clang-php-checker > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php >
Just to check: are you setting the environment variable USE_ZEND_ALLOC to 0? This causes the engine to use malloc: https://heap.space/xref/PHP-7.4/Zend/zend_alloc.c?r=600402d9#2738. For what it's worth, I was recently annoyed _again_ by valgrind being so noisy because zend_string_equal_val intentionally reads past the end of a zend_string. The allocator ensures that memory was allocated, but it isn't guaranteed to be initialized. We should find some way to initialize this memory for future releases -- maybe add a function which null terminates a zend string by adding not 1 null byte but as many as necessary to reach the end of the allocation. This should be trivial enough in cost to do, compared to some other solutions like always zero'ing out the whole memory block or initializing the trailing bytes at zend_string_alloc time. Also, I'm not sure this read-past-the-end technique is actually safe, such as when USE_ZEND_ALLOC is set to zero and we use malloc directly, which does not make the same guarantees about alignment and padding on the string... Nikita pushed up this change only today, but it would theoretically help with valgrind being used a runtime but not compiled with valgrind support: https://github.com/php/php-src/commit/a0c44fbaf19841164c7984a6c21b364d391f3750. I say theoretically only because I haven't tested it yet. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
