From: Masami Hiramatsu <masami.hiramatsu@linaro.org>
To: Siddhesh Poyarekar <siddhesh@gotplt.org>
Cc: linux-kselftest@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
Linux kernel mailing list <linux-kernel@vger.kernel.org>,
Tim Bird <tim.bird@sony.com>
Subject: Re: [PATCH] kselftest: Minimise dependency of get_size on C library interfaces
Date: Wed, 15 Jan 2020 15:14:02 +0900 [thread overview]
Message-ID: <CAA93ih1KXO5oSKAB6PmQc6xOw4fX5T+2+zx91BD18YUxL+nWzQ@mail.gmail.com> (raw)
In-Reply-To: <20200113164158.15803-1-siddhesh@gotplt.org>
Hi,
[Cc: Tim Bird]
2020年1月14日(火) 1:43 Siddhesh Poyarekar <siddhesh@gotplt.org>:
>
> It was observed[1] on arm64 that __builtin_strlen led to an infinite
> loop in the get_size selftest. This is because __builtin_strlen (and
> other builtins) may sometimes result in a call to the C library
> function. The C library implementation of strlen uses an IFUNC
> resolver to load the most efficient strlen implementation for the
> underlying machine and hence has a PLT indirection even for static
> binaries. Because this binary avoids the C library startup routines,
> the PLT initialization never happens and hence the program gets stuck
> in an infinite loop.
>
> On x86_64 the __builtin_strlen just happens to expand inline and avoid
> the call but that is not always guaranteed.
>
> Further, while testing on x86_64 (Fedora 31), it was observed that the
> test also failed with a segfault inside write() because the generated
> code for the write function in glibc seems to access TLS before the
> syscall (probably due to the cancellation point check) and fails
> because TLS is not initialised.
>
> To mitigate these problems, this patch reduces the interface with the
> C library to just the syscall function. The syscall function still
> sets errno on failure, which is undesirable but for now it only
> affects cases where syscalls fail.
>
> [1] https://bugs.linaro.org/show_bug.cgi?id=5479
>
Thank you for the fix! I confirmed this fixes the issue.
----
root@devnote2:/opt/kselftest/size# ./get_size
TAP version 13
# Testing system size.
ok 1 get runtime memory use
# System runtime memory report (units in Kilobytes):
---
Total: 16085116
Free: 2042880
Buffer: 814052
In use: 13228184
...
1..1
----
Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> Reported-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
> tools/testing/selftests/size/get_size.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/size/get_size.c b/tools/testing/selftests/size/get_size.c
> index d4b59ab979a0..f55943b6d1e2 100644
> --- a/tools/testing/selftests/size/get_size.c
> +++ b/tools/testing/selftests/size/get_size.c
> @@ -12,23 +12,35 @@
> * own execution. It also attempts to have as few dependencies
> * on kernel features as possible.
> *
> - * It should be statically linked, with startup libs avoided.
> - * It uses no library calls, and only the following 3 syscalls:
> + * It should be statically linked, with startup libs avoided. It uses
> + * no library calls except the syscall() function for the following 3
> + * syscalls:
> * sysinfo(), write(), and _exit()
> *
> * For output, it avoids printf (which in some C libraries
> * has large external dependencies) by implementing it's own
> * number output and print routines, and using __builtin_strlen()
> + *
> + * The test may crash if any of the above syscalls fails because in some
> + * libc implementations (e.g. the GNU C Library) errno is saved in
> + * thread-local storage, which does not get initialized due to avoiding
> + * startup libs.
> */
>
> #include <sys/sysinfo.h>
> #include <unistd.h>
> +#include <sys/syscall.h>
>
> #define STDOUT_FILENO 1
>
> static int print(const char *s)
> {
> - return write(STDOUT_FILENO, s, __builtin_strlen(s));
> + size_t len = 0;
> +
> + while (s[len] != '\0')
> + len++;
> +
> + return syscall(SYS_write, STDOUT_FILENO, s, len);
> }
>
> static inline char *num_to_str(unsigned long num, char *buf, int len)
> @@ -80,12 +92,12 @@ void _start(void)
> print("TAP version 13\n");
> print("# Testing system size.\n");
>
> - ccode = sysinfo(&info);
> + ccode = syscall(SYS_sysinfo, &info);
> if (ccode < 0) {
> print("not ok 1");
> print(test_name);
> print(" ---\n reason: \"could not get sysinfo\"\n ...\n");
> - _exit(ccode);
> + syscall(SYS_exit, ccode);
> }
> print("ok 1");
> print(test_name);
> @@ -101,5 +113,5 @@ void _start(void)
> print(" ...\n");
> print("1..1\n");
>
> - _exit(0);
> + syscall(SYS_exit, 0);
> }
> --
> 2.24.1
>
--
Masami Hiramatsu
next prev parent reply other threads:[~2020-01-15 6:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-13 16:41 [PATCH] kselftest: Minimise dependency of get_size on C library interfaces Siddhesh Poyarekar
2020-01-15 6:14 ` Masami Hiramatsu [this message]
2020-01-16 17:32 ` Tim.Bird
2020-01-17 5:21 ` Siddhesh Poyarekar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAA93ih1KXO5oSKAB6PmQc6xOw4fX5T+2+zx91BD18YUxL+nWzQ@mail.gmail.com \
--to=masami.hiramatsu@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=shuah@kernel.org \
--cc=siddhesh@gotplt.org \
--cc=tim.bird@sony.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).