kunit vs structleak

RFC In review Arnd Bergmann 4 participants 7 messages Jan 26, 2021 – Jan 29, 2021

Subsystems (7)

USB Kbuild Kselftest Library code +3 more subsystems

lore

Share & tools

lore

Arnd Bergmann

author

2021-01-26 06:01 UTC #0 lore raw

cover kunit vs structleak

From: Arnd Bergmann <[email protected]>

I ran into a couple of problems with kunit tests taking too much stack space, sometimes dangerously so. These the the three instances that cause an increase over the warning limit of some architectures:

lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] drivers/base/test/property-entry-test.c:481:1: error: the frame size of 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

Ideally there should be a way to rewrite the kunit infrastructure that avoids the explosion of stack data when the structleak plugin is used.

A rather drastic measure would be to use Kconfig logic to make the two options mutually exclusive. This would clearly work, but is probably not needed.

As a simpler workaround, this disables the plugin for the three files in which the excessive stack usage was observed.

Arnd

Arnd Bergmann (3):
bitfield: build kunit tests without structleak plugin drivers/base: build kunit tests without structleak plugin
thunderbolt: build kunit tests without structleak plugin

 drivers/base/test/Makefile   | 1 +
 drivers/thunderbolt/Makefile | 1 +
 lib/Makefile                 | 1 +
 3 files changed, 3 insertions(+)

Cc Kees Cook <[email protected]>

Cc Brendan Higgins <[email protected]>

Cc Shuah Khan <[email protected]>

Cc Geert Uytterhoeven <[email protected]>

Cc Alan Maguire <[email protected]>

Cc Dmitry Torokhov <[email protected]>

Cc Mika Westerberg <[email protected]>

Cc Vitor Massaru Iha <[email protected]>

Cc [email protected]

Signature · 1 line

2.29.2

Arnd Bergmann

author

2021-01-26 05:51 UTC #1 lore raw

RFC 1/3 bitfield: build kunit tests without structleak plugin

From: Arnd Bergmann <[email protected]>

The structleak plugin causes the stack frame size to grow immensely:

lib/bitfield_kunit.c: In function 'test_bitfields_constants':
lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]

Turn it off in this file.

Signed-off-by Arnd Bergmann <[email protected]>

---
 lib/Makefile | 1 +
 1 file changed, 1 insertion(+)

lib/Makefile +1 -0

index b5307d3eec1a..6a7300f0f02f 100644
@@ -348,6 +348,7 @@ obj-$(CONFIG_PLDMFW) += pldmfw/
 
 # KUnit tests
 obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
+CFLAGS_REMOVE_bitfield_kunit.o += -fplugin-arg-structleak_plugin-byref -fplugin-arg-structleak_plugin-byref-all
 obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
 obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
 obj-$(CONFIG_BITS_TEST) += test_bits.o

Signature · 1 line

2.29.2

Arnd Bergmann

author

2021-01-26 05:51 UTC #2 lore raw

RFC 2/3 drivers/base: build kunit tests without structleak plugin

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The structleak plugin causes the stack frame size to grow immensely:

drivers/base/test/property-entry-test.c: In function 'pe_test_reference':
drivers/base/test/property-entry-test.c:481:1: error: the frame size of 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]
481 | }

| ^

drivers/base/test/property-entry-test.c: In function 'pe_test_uints':
drivers/base/test/property-entry-test.c:99:1: error: the frame size of 2592 bytes is larger than 2048 bytes [-Werror=frame-larger-than=]

Turn it off in this file.

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/base/test/Makefile | 1 +
 1 file changed, 1 insertion(+)

drivers/base/test/Makefile +1 -0

index 3ca56367c84b..2f15fae8625f 100644
@@ -2,3 +2,4 @@
 obj-$(CONFIG_TEST_ASYNC_DRIVER_PROBE)	+= test_async_driver_probe.o
 
 obj-$(CONFIG_KUNIT_DRIVER_PE_TEST) += property-entry-test.o
+CFLAGS_REMOVE_property-entry-test.o += -fplugin-arg-structleak_plugin-byref -fplugin-arg-structleak_plugin-byref-all

Signature · 1 line

2.29.2

Arnd Bergmann

author

2021-01-26 05:41 UTC #3 lore raw

RFC 3/3 thunderbolt: build kunit tests without structleak plugin

In reply to Arnd Bergmann

From: Arnd Bergmann <[email protected]>

The structleak plugin causes the stack frame size to grow immensely:

drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

Turn it off in this file.

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/thunderbolt/Makefile | 1 +
 1 file changed, 1 insertion(+)

drivers/thunderbolt/Makefile +1 -0

index 7aa48f6c41d9..e571c0495a84 100644
@@ -7,6 +7,7 @@ thunderbolt-objs += nvm.o retimer.o quirks.o
 thunderbolt-${CONFIG_ACPI} += acpi.o
 thunderbolt-$(CONFIG_DEBUG_FS) += debugfs.o
 thunderbolt-${CONFIG_USB4_KUNIT_TEST} += test.o
+CFLAGS_REMOVE_test.o += -fplugin-arg-structleak_plugin-byref -fplugin-arg-structleak_plugin-byref-all
 
 thunderbolt_dma_test-${CONFIG_USB4_DMA_TEST} += dma_test.o
 obj-$(CONFIG_USB4_DMA_TEST) += thunderbolt_dma_test.o

Signature · 1 line

2.29.2

Mika Westerberg

2021-01-27 12:57 UTC #4 lore raw

Re: [RFC 3/3] thunderbolt: build kunit tests without structleak plugin

Hi Arnd,

Quoted 9 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>
The structleak plugin causes the stack frame size to grow immensely:
drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Turn it off in this file.
Signed-off-by Arnd Bergmann <[email protected]>

To me this is a reasonable work around so I can pick this up to Thunderbolt tree if no objections.

Thanks BTW, for doing this. I got a report from buildbot some time ago about the this but did not have time to figure out how to fix it :)

Kees Cook

2021-01-27 20:16 UTC #5 lore raw

In reply to Arnd Bergmann

Quoted 28 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>
I ran into a couple of problems with kunit tests taking too much stack space, sometimes dangerously so. These the the three instances that cause an increase over the warning limit of some architectures:
lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] drivers/base/test/property-entry-test.c:481:1: error: the frame size of 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Ideally there should be a way to rewrite the kunit infrastructure that avoids the explosion of stack data when the structleak plugin is used.
A rather drastic measure would be to use Kconfig logic to make the two options mutually exclusive. This would clearly work, but is probably not needed.
As a simpler workaround, this disables the plugin for the three files in which the excessive stack usage was observed.
Arnd
Arnd Bergmann (3):
bitfield: build kunit tests without structleak plugin drivers/base: build kunit tests without structleak plugin
thunderbolt: build kunit tests without structleak plugin

Quoted 4 lines

 drivers/base/test/Makefile   | 1 +
 drivers/thunderbolt/Makefile | 1 +
 lib/Makefile                 | 1 +
 3 files changed, 3 insertions(+)

I think I'd prefer centralizing the disabling, as done with the other plugins, instead of sprinkling "open coded" command-line options around the kernel's Makefiles. :)

For example:

scripts/Makefile.gcc-plugins +4 -0

index 952e46876329..2d5009e3b593 100644
@@ -21,6 +21,10 @@ gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL)	\
 		+= -fplugin-arg-structleak_plugin-byref-all
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK)		\
 		+= -DSTRUCTLEAK_PLUGIN
+ifdef CONFIG_GCC_PLUGIN_STRUCTLEAK
+    DISABLE_STRUCTLEAK_PLUGIN += -fplugin-arg-structleak_plugin-disable
+endif
+export DISABLE_STRUCTLEAK_PLUGIN
 
 gcc-plugin-$(CONFIG_GCC_PLUGIN_RANDSTRUCT)	+= randomize_layout_plugin.so
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT)		\

And then use DISABLE_STRUCTLEAK_PLUGIN.

Signature · 1 line

Kees Cook

Brendan Higgins

2021-01-29 21:30 UTC #6 lore raw

Quoted 59 lines · Kees Cook

Quoted 28 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
I ran into a couple of problems with kunit tests taking too much stack space, sometimes dangerously so. These the the three instances that cause an increase over the warning limit of some architectures:
lib/bitfield_kunit.c:93:1: error: the frame size of 7440 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] drivers/base/test/property-entry-test.c:481:1: error: the frame size of 2640 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] drivers/thunderbolt/test.c:1529:1: error: the frame size of 1176 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Ideally there should be a way to rewrite the kunit infrastructure that avoids the explosion of stack data when the structleak plugin is used.
A rather drastic measure would be to use Kconfig logic to make the two options mutually exclusive. This would clearly work, but is probably not needed.
As a simpler workaround, this disables the plugin for the three files in which the excessive stack usage was observed.
Arnd
Arnd Bergmann (3):
bitfield: build kunit tests without structleak plugin drivers/base: build kunit tests without structleak plugin
thunderbolt: build kunit tests without structleak plugin
Quoted 4 lines
 drivers/base/test/Makefile   | 1 +
 drivers/thunderbolt/Makefile | 1 +
 lib/Makefile                 | 1 +
 3 files changed, 3 insertions(+)
I think I'd prefer centralizing the disabling, as done with the other plugins, instead of sprinkling "open coded" command-line options around the kernel's Makefiles. :)
For example:
scripts/Makefile.gcc-plugins +4 -0
index 952e46876329..2d5009e3b593 100644
@@ -21,6 +21,10 @@ gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL)  \
                += -fplugin-arg-structleak_plugin-byref-all
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STRUCTLEAK)              \
                += -DSTRUCTLEAK_PLUGIN
+ifdef CONFIG_GCC_PLUGIN_STRUCTLEAK
+    DISABLE_STRUCTLEAK_PLUGIN += -fplugin-arg-structleak_plugin-disable
+endif
+export DISABLE_STRUCTLEAK_PLUGIN
 
 gcc-plugin-$(CONFIG_GCC_PLUGIN_RANDSTRUCT)     += randomize_layout_plugin.so
 gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_RANDSTRUCT)              \

Quoted 1 line

And then use DISABLE_STRUCTLEAK_PLUGIN.

This looks fine to me. Does somebody want me to send this out as a patch? Don't want to steal anyone's thunder :-)