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

XFAIL payload_test.phpt when run with ASan #6966

Closed
wants to merge 5 commits into from

Conversation

cmb69
Copy link
Contributor

@cmb69 cmb69 commented May 10, 2021

The test leaks memory for yet unknown reasons. Maybe these are caused
by a incomplete payload. For no we mark the test as xfail, to avoid
failing CI for the nightly runs.


I'm not able to reproduce the leak on Windows (while there is ASan support, there is no support for LSan yet). I can reproduce in a Ubuntu VM, but I'm not able to get debug symbols (version mismatch of latest packages), so this is the best I can come up with for now.

The test leaks memory for yet unknown reasons.  Maybe these are caused
by a incomplete payload.  For no we mark the test as xfail, to avoid
failing CI for the nightly runs.
@nikic
Copy link
Member

nikic commented May 10, 2021

Output with debug symbols if it helps: https://gist.github.com/nikic/3c058e3fbe90e4c4eb82f9cc494f502f

@nikic
Copy link
Member

nikic commented May 10, 2021

It looks like the same leaks occur on all firebird tests, we just don't currently run them in CI.

@cmb69 cmb69 marked this pull request as draft May 10, 2021 12:45
@cmb69
Copy link
Contributor Author

cmb69 commented May 10, 2021

Ah! Then I'll have a closer look later. Thanks for the debug output!

@cmb69
Copy link
Contributor Author

cmb69 commented May 11, 2021

Indeed, I can reproduce the leaks with a minimal <?php new PDO(…);?> script. Under the hood we're calling isc_attach_database() and isc_detach_database() one time each. It seems to me that these memory leak reports are either caused by libfbclient, or are bogus valgrind issues (maybe C++ related). @MartinKoeditz, are you aware of such issues, or is PDO_Firebird missing some cleanup call?

@nikic
Copy link
Member

nikic commented May 12, 2021

Thanks for looking into it. I think in that case we might be best off with adding a suppression for isc_attach_database to https://github.com/php/php-src/blob/master/azure/lsan-suppressions.txt. Otherwise we'd have to skip all firebird tests under asan, which is not ideal.

@cmb69
Copy link
Contributor Author

cmb69 commented May 12, 2021

I've pushed that, but locally I still get something like:

==1332==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 112 byte(s) in 1 object(s) allocated from:
    #0 0x7f2a72764517 in malloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb0517)
    #1 0x7f2a721d3544 in __gconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x29544)
    #2 0x7f2a721d3097 in iconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x29097)
    #3 0x60200000292f  (<unknown module>)

Indirect leak of 32640 byte(s) in 1 object(s) allocated from:
    #0 0x7f2a72764517 in malloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb0517)
    #1 0x7f2a721d35bf in __gconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x295bf)
    #2 0x7f2a721d3097 in iconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x29097)
    #3 0x60200000292f  (<unknown module>)

Indirect leak of 208 byte(s) in 1 object(s) allocated from:
    #0 0x7f2a72764517 in malloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb0517)
    #1 0x7f2a721de4f6  (/lib/x86_64-linux-gnu/libc.so.6+0x344f6)

@nikic
Copy link
Member

nikic commented May 12, 2021

That's unfortunate :( We'd have to use ASAN_OPTIONS=fast_unwind_on_malloc=0 to avoid that, and that makes asan much slower.

So I think the best thing we can do is disable leak checking (while still running asan) using:

--ENV--
LSAN_OPTIONS=detect_leaks=0 

@cmb69
Copy link
Contributor Author

cmb69 commented May 12, 2021

ACK

@cmb69 cmb69 marked this pull request as ready for review May 12, 2021 15:55
@cmb69 cmb69 closed this in f6c15e2 May 13, 2021
@cmb69 cmb69 deleted the cmb/payload-test-leaks branch May 13, 2021 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants