-
Notifications
You must be signed in to change notification settings - Fork 32
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
Enabling libasan causes failures during tests #43
Comments
Thanks @tmiw. If you think you have a quick fix and don't mind working on it pls feel free to go ahead and raise a PR. |
I created #45 with what I was able to get to tonight. There's still something like 14 ctest failures with libasan on, but I suspect at least a couple are due to configuration issues on my end. |
@tmiw - did any of the issues detected by AddressSanitizer cause any actual, real world issues, when running say freedv-gui? The changes in #45 are minor tweaks mainly in test code for soon to be deprecated modes. All this code was running fine before this tool was applied. We already have a suite of |
This originally kicked off because @barjac couldn't build with AddressSanitizer while attempting to debug a freedv-gui issue. Fixing that of course ended up kicking off additional issues (for example, the behavior apparently varying wildly depending on what locale freedv-gui is set to). Also, I'm fairly sure he was able to duplicate the crashes in freedv-gui without AddressSanitizer enabled at one point, too. For reference, the most recent crashes seemed to be happening while trying to decode 800XA:
(and earlier, were happening when decoding 2020 too) There are also security implications, too. While admittedly unlikely, I imagine someone could come up with an OTA signal (or even just a WAV file that they convinced someone to play back in freedv-gui) that triggered one of the buffer overflows fixed by this PR and caused them to gain control of the user's system or something. Anyway, that's a long way of saying that these modes should be fixed as long as the modes are still being used by something (e.g. freedv-gui). :) |
@tmiw - I'm not convinced this is an issue we should be addressing at this time. We made a decision at PLT that non-critical bugs - especially in deprecated modes - would not be addressed at this time. Fixing ctests in deprecated code (that pass just fine) is not where we should working right now. As codec 2 lead I'm making the call to pause any further libcodec2 work in this area - if you'd like to discuss further please raise at PLT. |
@barjac reported an AddressSanitizer error during freedv-gui build:
Additionally, it appears a large number of ctests fail with AddressSanitizer enabled. Building with the following on an aarch64 Ubuntu VM:
(More tests were failing without disabling the ODR detection. I disabled above since it seemed like a false alarm.)
Example of one of the ctest failures:
A quick look seems to indicate that it's due to the following definition for
my_cb_state
(in bothfreedv_rx
andfreedv_tx
):This definition should be moved to the top of
main()
to eliminate this error.The text was updated successfully, but these errors were encountered: