Re: [PATCH v3 07/58] objtool/klp: Give each run one working directory, one per test inside it

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] objtool/klp: test harness leaks top-level tmpdir on interrupt
--

commit e3c2fe244875e7154ea4380414b6732281ba9cd6
Author: Song Liu <song@kernel.org>

objtool/klp: Give each run one working directory, one per test inside it

Each test made its own mktemp directory, so a run scattered them through
/tmp among everything else using the same names, and there was no way to
inspect what a failing test built -- the objects are gone the moment it
exits, which is right every time but the one that matters.

Give the run one directory and each test a subdirectory of it, named for
the test and nested the way the source is.

> diff --git a/tools/objtool/tests/run-tests.sh b/tools/objtool/tests/run-tests.sh
> index 6b937fc7f5bda..e6f1ac1b5d0da 100755
> --- a/tools/objtool/tests/run-tests.sh
> +++ b/tools/objtool/tests/run-tests.sh
[ ... ]
> @@ -98,12 +103,18 @@ else
>  	done
>  fi
>  
> +# One directory for the whole run, one per test inside it, mirroring the
> +# source layout.  A run then leaves a single thing behind instead of 39
> +# scattered among everything else using mktemp.
> +rundir="$(mktemp -d "${TMPDIR:-/tmp}/klp-tests.XXXXXXXX")" ||
> +	{ echo "Bail out! cannot create a working directory" >&2; exit 1; }
> +

[Severity: Medium]
Could this leak the top-level temporary directory if the test suite is aborted
with a shell signal like SIGINT?

Since there is no trap registered for shell signals or EXIT, an abnormal
termination will bypass the cleanup logic at the end of the script.

[ ... ]
> @@ -156,4 +167,15 @@ done
>  echo "# pass:$pass fail:$fail static-skip:$static_skip" \
>       "probe-skip:$probe_skip xfail:$xfail xpass:$xpass"
>  
> +# A failure is the one time the objects matter, and by default they are
> +# already gone.  Say so then rather than in the usage text nobody reads while
> +# something is broken.
> +if [ -n "${KLP_TEST_KEEP:-}" ]; then
> +	echo "# working directories kept in $rundir -- inspect, then rm -rf it"
> +elif ! rmdir "$rundir"/*/ "$rundir" 2>/dev/null; then

[Severity: Medium]
Will this cleanup logic be completely bypassed if the script exits early due
to an interrupt?

Setting a trap could ensure this directory removal runs even when the script
terminates abnormally.

> +	echo "# $rundir was not empty; a test did not clean up after itself"
> +elif [ "$fail" != 0 ] || [ "$xpass" != 0 ]; then
> +	echo "# re-run with --keep to hold on to what a failing test built"
> +fi
> +
>  [ "$fail" = 0 ] && [ "$xpass" = 0 ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914230355.3422996-1-song@kernel.org?part=7