diff mbox series

selftests/harness: Prevent infinite loop due to Assert in FIXTURE_TEARDOWN

Message ID 20240314224323.23201-1-shengyu.li.evgeny@gmail.com
State Superseded
Headers show
Series selftests/harness: Prevent infinite loop due to Assert in FIXTURE_TEARDOWN | expand

Commit Message

Shengyu Li March 14, 2024, 10:43 p.m. UTC
This patch addresses an issue in the selftests/harness where an assertion within FIXTURE_TEARDOWN could trigger an infinite loop. The problem arises because the teardown procedure is meant to execute once, but the presence of failing assertions (ASSERT_EQ(0, 1)) leads to repeated attempts to execute teardown due to the long jump mechanism used by the harness for handling assertions.

To resolve this, the patch ensures that the teardown process runs only once, regardless of assertion outcomes, preventing the infinite loop and allowing tests to fail.

Signed-off-by: Shengyu Li <shengyu.li.evgeny@gmail.com>
---
 tools/testing/selftests/kselftest_harness.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Shuah Khan March 26, 2024, 7:28 p.m. UTC | #1
On 3/14/24 16:43, Shengyu Li wrote:
> This patch addresses an issue in the selftests/harness where an assertion within FIXTURE_TEARDOWN could trigger an infinite loop. The problem arises because the teardown procedure is meant to execute once, but the presence of failing assertions (ASSERT_EQ(0, 1)) leads to repeated attempts to execute teardown due to the long jump mechanism used by the harness for handling assertions.
> 
> To resolve this, the patch ensures that the teardown process runs only once, regardless of assertion outcomes, preventing the infinite loop and allowing tests to fail.
> 

Please make sure change log is formatted with 70-75 character
long lines.

Please include information on how you found this problem.

Send v2 with that change.

> Signed-off-by: Shengyu Li <shengyu.li.evgeny@gmail.com>
> ---
>   tools/testing/selftests/kselftest_harness.h | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 4fd735e48ee7..230d62884885 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -383,6 +383,7 @@
>   		FIXTURE_DATA(fixture_name) self; \
>   		pid_t child = 1; \
>   		int status = 0; \
> +		bool jmp = false; \
>   		memset(&self, 0, sizeof(FIXTURE_DATA(fixture_name))); \
>   		if (setjmp(_metadata->env) == 0) { \
>   			/* Use the same _metadata. */ \
> @@ -399,8 +400,10 @@
>   				_metadata->exit_code = KSFT_FAIL; \
>   			} \
>   		} \
> +		else \
> +			jmp = true; \
>   		if (child == 0) { \
> -			if (_metadata->setup_completed && !_metadata->teardown_parent) \
> +			if (_metadata->setup_completed && !_metadata->teardown_parent && !jmp) \
>   				fixture_name##_teardown(_metadata, &self, variant->data); \
>   			_exit(0); \
>   		} \

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 4fd735e48ee7..230d62884885 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -383,6 +383,7 @@ 
 		FIXTURE_DATA(fixture_name) self; \
 		pid_t child = 1; \
 		int status = 0; \
+		bool jmp = false; \
 		memset(&self, 0, sizeof(FIXTURE_DATA(fixture_name))); \
 		if (setjmp(_metadata->env) == 0) { \
 			/* Use the same _metadata. */ \
@@ -399,8 +400,10 @@ 
 				_metadata->exit_code = KSFT_FAIL; \
 			} \
 		} \
+		else \
+			jmp = true; \
 		if (child == 0) { \
-			if (_metadata->setup_completed && !_metadata->teardown_parent) \
+			if (_metadata->setup_completed && !_metadata->teardown_parent && !jmp) \
 				fixture_name##_teardown(_metadata, &self, variant->data); \
 			_exit(0); \
 		} \