On Thu, May 29, 2025 at 11:02:19AM +0200, Peter Zijlstra wrote:
On Wed, May 28, 2025 at 03:47:42PM -0700, Kees Cook wrote:
On Mon, May 26, 2025 at 01:27:51PM +0000, Alessandro Carminati wrote:
Some unit tests intentionally trigger warning backtraces by passing bad parameters to kernel API functions. Such unit tests typically check the return value from such calls, not the existence of the warning backtrace.
Such intentionally generated warning backtraces are neither desirable nor useful for a number of reasons:
- They can result in overlooked real problems.
- A warning that suddenly starts to show up in unit tests needs to be investigated and has to be marked to be ignored, for example by adjusting filter scripts. Such filters are ad hoc because there is no real standard format for warnings. On top of that, such filter scripts would require constant maintenance.
Solve the problem by providing a means to identify and suppress specific warning backtraces while executing test code. Support suppressing multiple backtraces while at the same time limiting changes to generic code to the absolute minimum.
Implementation details: Check suppression directly in the `WARN()` Macros. This avoids the need for function symbol resolution or ELF section modification. Suppression is implemented directly in the `WARN*()` macros.
A helper function, `__kunit_is_suppressed_warning()`, is used to determine whether suppression applies. It is marked as `noinstr`, since some `WARN*()` sites reside in non-instrumentable sections. As it uses `strcmp`, a `noinstr` version of `strcmp` was introduced. The implementation is deliberately simple and avoids architecture-specific optimizations to preserve portability. Since this mechanism compares function names and is intended for test usage only, performance is not a primary concern.
Signed-off-by: Guenter Roeck linux@roeck-us.net
I like this -- it's very simple, it doesn't need to be fast-path, so a linear list walker with strcmp is fine. Nice!
But it is on the fast path! This is still bloating every UD2 site instead of doing it on the other end.
Doing it on the other end doesn't look great (see the other reply). I was suggesting it's not on fast path because the added code is a dependant conditional following an "unlikley" conditional. But if you wanted to push it totally out of line, we'd likely need to pass __func__ into warn_slowpath_fmt() and __warn_printk(), and then have __warn_printk() return bool to make the call to __WARN_FLAGS() conditional. e.g.:
- warn_slowpath_fmt(__FILE__, __LINE__, taint, arg); \ + warn_slowpath_fmt(__FILE__, __LINE__, __func__, taint, arg); \
and:
- __warn_printk(arg); \ - __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\ + if (__warn_printk(__func__, arg)) \ + __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
But it still leaves bare __WARN unhandled...