From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com [IPv6:2a00:1450:4864:20::62e]) by sourceware.org (Postfix) with ESMTPS id F14563858D20 for ; Thu, 29 Dec 2022 16:41:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F14563858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ej1-x62e.google.com with SMTP id x22so46159720ejs.11 for ; Thu, 29 Dec 2022 08:41:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=aUyr4Hg4iy/ncYyKHnH3ebe6N32TuJcrHDsZMbzolwM=; b=f0XYXziFveDg2AALGd7gO/O5zbKJCkf3sVILmtvC5P0f3kUiWg9xlTAowuQz3aTFHl c1LXT/HKVXk+/M5N4oGJ0MFgWlXF7rWcSKm9dOI/U27hgzHMCGHH0vcUSJkwLb7/m9BI gmyvQGkeNfYrLW7nccgFclIlKFizxHu99mzHPbck9+rZhFGKdT/Of5i8B+medhBuxeSR KwXoWHhr9oE5lF6dm2PEJw5jQsS+XXCIL3WJASivD8WpN9GNq8zDdDpcG06celuRP5Yu D8VTCp4+A2FPvPc4eq71ntGGKNyiN8rFBkoe+8xf/d6e29p3T3sqj8rXN3+x+DwnJd+M Fmcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=aUyr4Hg4iy/ncYyKHnH3ebe6N32TuJcrHDsZMbzolwM=; b=I7G3ckndJse58DWy8YSFCV2qVomw8z809SIjeieOH/mLxReg6Gq+3lM8gOZ501dKP9 kkF5pZ0uYkjTvtFW+9/dfhXUMNc33a/NhbMVHShCeg4hag2Ra7Q2ojO0BoWrnFgiQrFD DBCV25os7OieBDcaHuc20pYihZOQz79dVgsPQq6SWZXnbC42OTgyhzXKtt6CwLvDMcsW fUWoawJCW1mc1lnWVn7r0X/CggVSe633EXhIZ9v8dQFGzab/+QX+dTtHSR5h7REllOXm croOBIN5BUKO99fP57NXoV9UZCONR0NEXh8gZxFoy1L15/X6a+d/EdQjb4+aVFUygxWa /E5w== X-Gm-Message-State: AFqh2kpCCX555kMoqRGf1TYxmbMPFrQWNwa4uGcvxxMJlxhXwFeMg0+x /Sj/5O6YoNQTDFBj588d9iYdNCRDb+jCnxmcDF0C//6Z X-Google-Smtp-Source: AMrXdXujbi/IxqXJWG949P7pF5FMikgPmo1/WEarllzSGl4oMg5yHDiDteHvlNdTPBlwcxY+c4tRaI2Ca2lk5vPOZ9s= X-Received: by 2002:a17:906:281b:b0:7c1:98f:be57 with SMTP id r27-20020a170906281b00b007c1098fbe57mr1638856ejc.97.1672332096293; Thu, 29 Dec 2022 08:41:36 -0800 (PST) MIME-Version: 1.0 References: <20221219192726.999818-1-goldstein.w.n@gmail.com> <20221219192726.999818-2-goldstein.w.n@gmail.com> In-Reply-To: From: Noah Goldstein Date: Thu, 29 Dec 2022 08:41:24 -0800 Message-ID: Subject: Re: [PATCH v1 2/2] nptl: Add a testcase for inputs racey to {w}memcmp{eq} BZ #29863 To: libc-alpha@sourceware.org Cc: hjl.tools@gmail.com, carlos@systemhalted.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_NUMSUBJECT,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Dec 19, 2022 at 11:28 AM Noah Goldstein wrote: > > On Mon, Dec 19, 2022 at 11:27 AM Noah Goldstein wrote: > > > > Add test that modifies data as {w}memcmp{eq} runs (creating a race > > condition). Failures from this tests do not inherently mean the > > {w}memcmp{eq} implementation is buggy, but is at the very least > > something we should be aware of. > > > > Success is no SIGSEGV. Failure is a SIGSEGV. > > > > Verified test failed 10/10 times without: > > > > commit b712be52645282c706a5faa038242504feb06db5 > > Author: Noah Goldstein > > Date: Wed Dec 14 10:52:10 2022 -0800 > > > > x86: Prevent SIGSEGV in memcmp-sse2 when data is concurrently modified [BZ #29863] > > > > And passes with the fix. > > NB: Forget tag "Co-authored-by: H.J. Lu ", > will add before commit or next version if more revisions are needed. > > --- > > nptl/Makefile | 7 ++ > > nptl/tst-memcmp-race.c | 149 +++++++++++++++++++++++++++++++++++++++ > > nptl/tst-memcmpeq-race.c | 19 +++++ > > nptl/tst-wmemcmp-race.c | 20 ++++++ > > 4 files changed, 195 insertions(+) > > create mode 100644 nptl/tst-memcmp-race.c > > create mode 100644 nptl/tst-memcmpeq-race.c > > create mode 100644 nptl/tst-wmemcmp-race.c > > > > diff --git a/nptl/Makefile b/nptl/Makefile > > index fc955cd604..5f56bdc80a 100644 > > --- a/nptl/Makefile > > +++ b/nptl/Makefile > > @@ -285,6 +285,8 @@ tests = \ > > tst-exec4 \ > > tst-exec5 \ > > tst-initializers1 $(addprefix tst-initializers1-,c89 gnu89 c99 gnu99 c11 gnu11) \ > > + tst-memcmp-race \ > > + tst-memcmpeq-race \ > > tst-minstack-cancel \ > > tst-minstack-exit \ > > tst-minstack-throw \ > > @@ -348,8 +350,13 @@ tests = \ > > tst-thread_local1 \ > > tst-tsd3 \ > > tst-tsd4 \ > > + tst-wmemcmp-race \ > > # tests > > > > +CFLAGS-tst-memcmp-race.c += -O0 > > +CFLAGS-tst-memcmpeq-race.c += -O0 > > +CFLAGS-tst-wmemcmp-race.c += -O0 > > + > > tests-nolibpthread = \ > > tst-pthread_exit-nothreads \ > > tst-pthread_exit-nothreads-static \ > > diff --git a/nptl/tst-memcmp-race.c b/nptl/tst-memcmp-race.c > > new file mode 100644 > > index 0000000000..56e62b2f7b > > --- /dev/null > > +++ b/nptl/tst-memcmp-race.c > > @@ -0,0 +1,149 @@ > > +/* Test case for memcmp with race condition. > > + Copyright (C) 2022 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + > > + The GNU C Library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later version. > > + > > + The GNU C Library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with the GNU C Library; if not, see > > + . */ > > + > > +/* Verify that there is no segfault when one thread is updating the > > + memory block of memcmp and the other thread is doing memcmp. > > + > > + NOTE: This test failing does not automatically mean the > > + {w}memcmp{eq} is incorrect. This is testing UB and behavior that > > + is supported. That being said, users may expect that the mem* > > + functions never access out of bounds data (even with data races) > > + and we should be aware of the cases when we do. */ > > + > > +#define TEST_MAIN > > +#define MIN_PAGE_SIZE 4096 > > + > > +#ifdef TEST_MEMCMPEQ > > +# define MEMCMP __memcmpeq > > +# define TEST_NAME "__memcmpeq" > > +#elif defined WIDE > > +# define MEMCMP wmemcmp > > +# define TEST_NAME "wmemcmp" > > +#else > > +# define MEMCMP memcmp > > +# define TEST_NAME "memcmp" > > +#endif > > + > > +#ifdef WIDE > > +# define MEMSET wmemset > > +# define CHAR wchar_t > > +#else > > +# define MEMSET memset > > +# define CHAR char > > +#endif > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define NUM_THREADS 2 > > +#define LOOP1 10000 > > +#define LOOP2 1000000 > > + > > +typedef int (*proto_t) (const CHAR *, const CHAR *, size_t); > > + > > +IMPL (MEMCMP, 1) > > + > > +struct arg > > +{ > > + proto_t func; > > + CHAR *a; > > + CHAR *b; > > + size_t len; > > + size_t wpos; > > + int todo; > > +}; > > + > > +static void * > > +childThread (void *tArgs) > > +{ > > + struct arg *args = (struct arg *) tArgs; > > + int i; > > + if (0 == args->todo % 2) > > + { > > + for (i = 0; i < LOOP1; i++) > > + { > > + volatile int result = args->func (args->a, args->b, args->len); > > + (void)(result); > > + } > > + } > > + else > > + { > > + for (i = 0; i < LOOP2; i++) > > + args->a[args->wpos] = i & 1; > > + args->a[args->wpos] = 1; > > + } > > + return NULL; > > +} > > + > > +static void > > +do_one_test (proto_t func, size_t len) > > +{ > > + int r; > > + if (len * sizeof (CHAR) > page_size) > > + return; > > + for (r = 0; r < 2; ++r) > > + { > > + size_t wpos; > > + for (wpos = 1; wpos < 128 && wpos <= len; wpos = wpos + wpos + 1) > > + { > > + int i; > > + size_t off; > > + pthread_t threads[NUM_THREADS]; > > + struct arg a[NUM_THREADS]; > > + > > + off = r ? (page_size - len * sizeof (CHAR)) : 0; > > + for (i = 0; i < NUM_THREADS; ++i) > > + { > > + a[i].func = func; > > + a[i].a = (CHAR *) (buf1 + off); > > + a[i].b = (CHAR *) (buf2 + off); > > + a[i].len = len; > > + a[i].wpos = len - wpos; > > + a[i].todo = i; > > + threads[i] = xpthread_create (NULL, childThread, (void *) &a[i]); > > + } > > + > > + for (i = 0; i < NUM_THREADS; ++i) > > + xpthread_join (threads[i]); > > + } > > + } > > +} > > + > > +int > > +test_main (void) > > +{ > > + test_init (); > > + > > + MEMSET ((CHAR *) buf1, 1, page_size / sizeof (CHAR)); > > + MEMSET ((CHAR *) buf2, 1, page_size / sizeof (CHAR)); > > + for (size_t i = 1; i <= 1024; i += i) > > + { > > + FOR_EACH_IMPL (impl, 0) > > + { > > + do_one_test ((proto_t) impl->fn, i); > > + do_one_test ((proto_t) impl->fn, i + 1); > > + do_one_test ((proto_t) impl->fn, i - 1); > > + } > > + } > > + return 0; > > +} > > + > > +#include > > diff --git a/nptl/tst-memcmpeq-race.c b/nptl/tst-memcmpeq-race.c > > new file mode 100644 > > index 0000000000..7088928f49 > > --- /dev/null > > +++ b/nptl/tst-memcmpeq-race.c > > @@ -0,0 +1,19 @@ > > +/* Test case for __memcmpeq with race condition. > > + Copyright (C) 2022 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + > > + The GNU C Library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later version. > > + > > + The GNU C Library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with the GNU C Library; if not, see > > + . */ > > +#define TEST_MEMCMPEQ 1 > > +#include "tst-memcmp-race.c" > > diff --git a/nptl/tst-wmemcmp-race.c b/nptl/tst-wmemcmp-race.c > > new file mode 100644 > > index 0000000000..8278f71842 > > --- /dev/null > > +++ b/nptl/tst-wmemcmp-race.c > > @@ -0,0 +1,20 @@ > > +/* Test case for wmemcmp with race condition. > > + Copyright (C) 2022 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + > > + The GNU C Library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later version. > > + > > + The GNU C Library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with the GNU C Library; if not, see > > + . */ > > +#include > > +#define WIDE 1 > > +#include "tst-memcmp-race.c" > > -- > > 2.34.1 > > ping.