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

[asan] add address sanitizer support for syncd #1020

Merged
merged 2 commits into from
Mar 29, 2022

Conversation

Yakiv-Huryk
Copy link
Contributor

  • add --enable-asan for syncd compilation
  • add SIGTERM handler that calls __lsan_do_leak_check() to generate a
    report

Signed-off-by: Yakiv Huryk yhuryk@nvidia.com

* add --enable-asan for syncd compilation
* add SIGTERM handler that calls __lsan_do_leak_check() to generate a
report

Signed-off-by: Yakiv Huryk <yhuryk@nvidia.com>
syncd/main.cpp Outdated
int syncd_main(int argc, char **argv);

int main(int argc, char **argv)
{
SWSS_LOG_ENTER();

#if defined(ASAN_ENABLED)
if (signal(SIGTERM, sigterm_handler) == SIG_ERR)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need to install signal handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Mellanox platform, we don't stop the syncd via syncd_request_shutdown (https://github.com/Azure/sonic-buildimage/blob/master/files/scripts/syncd.sh#L55).
So, in order for us to generate the asan report, we need to catch the SIGTERM and generate it explicitly via __lsan_do_leak_check().
Another benefit of this approach is that in such case, the report will not contain any records of memory with the program's lifetime.

@liat-grozovik
Copy link
Collaborator

@kcudnik , @prsunny could you please signoff?

@kcudnik
Copy link
Collaborator

kcudnik commented Mar 23, 2022

would it be possible to make this as separate file and signal handler marka as attribute((constructor)) ? And include this file for compilation only when asan is enabled?

kcudnik
kcudnik previously approved these changes Mar 23, 2022
Signed-off-by: Yakiv Huryk <yhuryk@nvidia.com>
@Yakiv-Huryk Yakiv-Huryk dismissed stale reviews from oleksandrivantsiv and kcudnik via 7a0ad21 March 28, 2022 19:25
@Yakiv-Huryk
Copy link
Contributor Author

would it be possible to make this as separate file and signal handler marka as attribute((constructor)) ? And include this file for compilation only when asan is enabled?

Good idea, thanks.
I've updated the PR - 7a0ad21

@kcudnik kcudnik merged commit e3af0df into sonic-net:master Mar 29, 2022
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
add --enable-asan for syncd compilation
add SIGTERM handler that calls __lsan_do_leak_check() to generate a report
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.

4 participants