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

Implementing __sanitizer_start_switch_fiber and __sanitizer_finish_switch_fiber for uthread to enable address sanitizer #178

Closed
1 of 2 tasks
ChuanqiXu9 opened this issue Oct 31, 2022 · 9 comments · Fixed by #273

Comments

@ChuanqiXu9
Copy link
Collaborator

Search before asking

  • I searched the issues and found no similar issues.

What happened + What you expected to happen

Currently, the address sanitizer can't support uthread well. See google/sanitizers#189 for details. So we disabled the address sanitizer for uthread now to make the CI passes stably.

The proper fix may be implement __asan_enter_fiber and __asan_exit_fiber in uthread to tell the sanitizer this is a stackful coroutine.

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!
@RainMark
Copy link
Collaborator

RainMark commented Nov 1, 2022

The uthread implement referenced seastar's thread and deleted the asan support at beginning. detail see: https://github.com/scylladb/seastar/blob/master/src/core/thread.cc#L42

@ChuanqiXu9 ChuanqiXu9 changed the title Implementing __asan_enter_fiber and __asan_exit_fiber for uthread to enable address sanitizer Implementing __sanitizer_start_switch_fiber and __sanitizer_finish_switch_fiber for uthread to enable address sanitizer Nov 1, 2022
@4kangjc
Copy link
Collaborator

4kangjc commented Nov 24, 2022

hmmm, 我对这个不是很熟,Asan__sanitizer_start_switch_fiber不能满足需求吗

@4kangjc
Copy link
Collaborator

4kangjc commented Nov 24, 2022

hmmm, 我对这个不是很熟,Asan__sanitizer_start_switch_fiber不能满足需求吗

为什么我们要自己实现一个

@ChuanqiXu9
Copy link
Collaborator Author

hmmm, 我对这个不是很熟,Asan__sanitizer_start_switch_fiber不能满足需求吗

为什么我们要自己实现一个

不是要实现一个,是要在我们的代码里使用 __sanitizer_start_switch_fiber 这个来提示 asan 当前处于有栈协程中。比较麻烦的地方是需要实现两个版本的代码,你看 https://github.com/scylladb/seastar/blob/master/src/core/thread.cc#L42 这里,Debug 和 Release 的代码是分成两份来实现的

@4kangjc
Copy link
Collaborator

4kangjc commented Nov 24, 2022

hmmm, 我对这个不是很熟,Asan__sanitizer_start_switch_fiber不能满足需求吗

为什么我们要自己实现一个

不是要实现一个,是要在我们的代码里使用 __sanitizer_start_switch_fiber 这个来提示 asan 当前处于有栈协程中。比较麻烦的地方是需要实现两个版本的代码,你看 https://github.com/scylladb/seastar/blob/master/src/core/thread.cc#L42 这里,Debug 和 Release 的代码是分成两份来实现的

hmm,用__has_feature检查是否支持address_sanitizer

@4kangjc
Copy link
Collaborator

4kangjc commented Nov 24, 2022

hmmm, 我对这个不是很熟,Asan__sanitizer_start_switch_fiber不能满足需求吗

为什么我们要自己实现一个

不是要实现一个,是要在我们的代码里使用 __sanitizer_start_switch_fiber 这个来提示 asan 当前处于有栈协程中。比较麻烦的地方是需要实现两个版本的代码,你看 https://github.com/scylladb/seastar/blob/master/src/core/thread.cc#L42 这里,Debug 和 Release 的代码是分成两份来实现的

hmm,用__has_feature检查是否支持?

我们是DEBUG才需要这个是吗,通过宏NDEBUG判断一下?

@ChuanqiXu9
Copy link
Collaborator Author

hmmm, 我对这个不是很熟,Asan__sanitizer_start_switch_fiber不能满足需求吗

为什么我们要自己实现一个

不是要实现一个,是要在我们的代码里使用 __sanitizer_start_switch_fiber 这个来提示 asan 当前处于有栈协程中。比较麻烦的地方是需要实现两个版本的代码,你看 https://github.com/scylladb/seastar/blob/master/src/core/thread.cc#L42 这里,Debug 和 Release 的代码是分成两份来实现的

hmm,用__has_feature检查是否支持?

我们是DEBUG才需要这个是吗,通过宏NDEBUG判断一下?

是的,可以这样

hmm,用__has_feature检查是否支持address_sanitizer?

不太确认能不能这样,最坏情况就是检测编译器是否是 clang 以及是否开 NDEBUG 这样。。

@4kangjc
Copy link
Collaborator

4kangjc commented Nov 24, 2022

hmmm, 我对这个不是很熟,Asan__sanitizer_start_switch_fiber不能满足需求吗

为什么我们要自己实现一个

不是要实现一个,是要在我们的代码里使用 __sanitizer_start_switch_fiber 这个来提示 asan 当前处于有栈协程中。比较麻烦的地方是需要实现两个版本的代码,你看 https://github.com/scylladb/seastar/blob/master/src/core/thread.cc#L42 这里,Debug 和 Release 的代码是分成两份来实现的

hmm,用__has_feature检查是否支持?

我们是DEBUG才需要这个是吗,通过宏NDEBUG判断一下?

是的,可以这样

hmm,用__has_feature检查是否支持address_sanitizer?

不太确认能不能这样,最坏情况就是检测编译器是否是 clang 以及是否开 NDEBUG 这样。。

GNU8以上不是也支持ASan么

@ChuanqiXu9
Copy link
Collaborator Author

hmmm, 我对这个不是很熟,Asan__sanitizer_start_switch_fiber不能满足需求吗

为什么我们要自己实现一个

不是要实现一个,是要在我们的代码里使用 __sanitizer_start_switch_fiber 这个来提示 asan 当前处于有栈协程中。比较麻烦的地方是需要实现两个版本的代码,你看 https://github.com/scylladb/seastar/blob/master/src/core/thread.cc#L42 这里,Debug 和 Release 的代码是分成两份来实现的

hmm,用__has_feature检查是否支持?

我们是DEBUG才需要这个是吗,通过宏NDEBUG判断一下?

是的,可以这样

hmm,用__has_feature检查是否支持address_sanitizer?

不太确认能不能这样,最坏情况就是检测编译器是否是 clang 以及是否开 NDEBUG 这样。。

GNU8以上不是也支持ASan么

是的。主要是我们内部就不怎么用 GCC 编这些新特性了,然后主要关注 clang,当然有 GCC 相关的改进肯定也都不会拒绝

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants