From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x32.google.com (mail-oa1-x32.google.com [IPv6:2001:4860:4864:20::32]) by sourceware.org (Postfix) with ESMTPS id CE0863858D1E for ; Mon, 19 Dec 2022 16:56:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CE0863858D1E 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-oa1-x32.google.com with SMTP id 586e51a60fabf-12c8312131fso12239914fac.4 for ; Mon, 19 Dec 2022 08:56:07 -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=OP+21fVV+CMlKKXJ0rhWk1KAKYoe5qVXjXsHpJHN/P8=; b=Tgcq0GK2RuTFoJIGRwYmNg3uxGJxEFxi+dmPHHPUUG8PQU35+LixbcRKMamaBQ22gn QzL3N+DLcsei8BUGq9/2fFjyktgIY/QswV0tWt5R3IszSqmU3vNJ4fL9Wo6Zc/3Aml0U kL2WfRASW6WKvGRP2g2tJk6APUD9MYPPr0mWdh9RMoyxgpcI1CjihI5u4dHXQBgEh8y0 YYFTRwX039wmwQIzoPgSuISQQjndsQpdMIXJgnkiaEHZyXoBJyvD0HamclgsOJGZ0KN5 DuU7L/H+MGP55MR9xiNSk6Odh1Ky2sRCQtzNKM+KQ2e+9jZ0ntb0D+iDPWEGY4FqQkQW rX/A== 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=OP+21fVV+CMlKKXJ0rhWk1KAKYoe5qVXjXsHpJHN/P8=; b=F9Zsqvr6RUTgDLgbzeEE8QNsdWb6OMK1oPYDaUDlswegQ2OiNwqHDmStys05QUlOE0 6OkBQ+Qpm/1ylb5zL6ZYEMBlkU4UGW5vDOUXAYPMqVdIDTbtH/rjiqXTVh2FjbWC4Psy 5DpHCEXYLy6qwxfOyzbEm7Hi40wllXzQAhZ8H7SHlSGxtGEl9ieMiYhhfnGbJtfEIAqs FshDozWnVsbLzqZaUZiXrGK9+87xJJgItSqPkSZpemava7+aCnAA2ikTTSoVc+m8InRj +Z3zwK2lMRsjEVHU2JbfqFOq5A2D678W3A6Tt2V0ipvwy2nEpdn1hF4uQ7Nfzp+ecezy X1oA== X-Gm-Message-State: AFqh2kof/d/9FW4hDN62LZjH1uZxJAaCdb6J4Y3xBiyNSnNTZ9/Pzktf 6rAP3aZePSYhqDlcTILIAO2vXSbFvMMqrLyjUIU= X-Google-Smtp-Source: AMrXdXu09xsBdk/tz4mpaF0+joDe5So/lIxBTy5s/Yf2lTQgSVhPgb5dN1tV7olFYRxqR8LDojEKzuEJFT+0LIImjNE= X-Received: by 2002:a05:6870:8087:b0:148:2d85:b411 with SMTP id q7-20020a056870808700b001482d85b411mr1902261oab.266.1671468967091; Mon, 19 Dec 2022 08:56:07 -0800 (PST) MIME-Version: 1.0 References: <20221214213053.4174989-1-hjl.tools@gmail.com> In-Reply-To: From: "H.J. Lu" Date: Mon, 19 Dec 2022 08:55:30 -0800 Message-ID: Subject: Re: [PATCH v5] x86: Add a testcase for BZ #29863 To: Noah Goldstein Cc: libc-alpha@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3022.3 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 6:39 AM Noah Goldstein wrote: > > On Wed, Dec 14, 2022 at 1:30 PM H.J. Lu wrote: > > > > When a thread is updating the memory area of memcmp and another thread > > is doing memcmp, the memcmp return value is undefined. Add a testcase > > to verify that memcmp won't segfault in this case. > > --- > > sysdeps/x86/Makefile | 8 +++ > > sysdeps/x86/tst-memcmp-race.c | 106 ++++++++++++++++++++++++++++++++++ > > 2 files changed, 114 insertions(+) > > create mode 100644 sysdeps/x86/tst-memcmp-race.c > > > > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile > > index 56fd5fc805..c3f05a06ab 100644 > > --- a/sysdeps/x86/Makefile > > +++ b/sysdeps/x86/Makefile > > @@ -257,3 +257,11 @@ tests += \ > > tests-static += \ > > tst-sysconf-cache-linesize-static > > endif > > + > > +ifeq ($(subdir),nptl) > > +tests += \ > > + tst-memcmp-race \ > > +# tests > > + > > +CFLAGS-tst-memcmp-race.c += -O0 > > +endif > > diff --git a/sysdeps/x86/tst-memcmp-race.c b/sysdeps/x86/tst-memcmp-race.c > > new file mode 100644 > > index 0000000000..eefa9e43c4 > > --- /dev/null > > +++ b/sysdeps/x86/tst-memcmp-race.c > > @@ -0,0 +1,106 @@ > > +/* 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. */ > > + > > +#define TEST_MAIN > > +#define TEST_NAME "memcmp" > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define NUM_THREADS 2 > > +#define LOOP1 10000 > > +#define LOOP2 1000000 > > + > > +typedef int (*proto_t) (const void *, const void *, size_t); > > + > > +IMPL (memcmp, 1) > > + > > +struct arg > > +{ > > + proto_t func; > > + size_t len; > > + int i; > > +}; > > + > > +static void * > > +childThread (void *tArgs) > > +{ > > + struct arg *a = (struct arg *) tArgs; > > + int i; > > + > > + if (0 == a->i % 2) > > + { > > + for (i = 0; i < LOOP1; i++) > > + { > > + int result = a->func (buf1, buf2, a->len); > > So the test can catch more subtle errors can we test > with memcmp args at begining and end of a page. > > i.e buf1/buf2 and (buf1 + page_size - a->len)/(buf2 + page_size - a->len). I'd like to keep it as simple as possible to cover this particular race condition. Make it more complex may not provide additional coverage for race conditions. > > > + printf ("i = %d : result = %d\n", i, result); > > + } > > + } > > + else > > + { > > + size_t offset = a->len > 16 ? a->len - 16 : 1; > > + for (i = 0; i < LOOP2; i++) > > + buf1[offset] = i & 1; > > + } > > + > > + return NULL; > > +} > > + > > +static void > > +do_one_test (proto_t func, size_t len) > > +{ > > + int i; > > + pthread_t threads[NUM_THREADS]; > > + struct arg a[NUM_THREADS]; > > + > > + for (i = 0; i < NUM_THREADS; ++i) > > + { > > + a[i].func = func; > > + a[i].len = len; > > + a[i].i = 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 (buf1, 1, page_size); > > + memset (buf2, 1, page_size); > > + > > + for (size_t i = 0; i <= 11; i++) > > + { > > + FOR_EACH_IMPL (impl, 0) > > + do_one_test ((proto_t) impl->fn, (1 << i) + 1); > can you also test 2^i - 1. Will add it. > > + } > > + > > + return 0; > > +} > > + > > +#include > > -- > > 2.38.1 > > > > Can you move this to the string directory (so we run this on all > arch) and add corresponding tests for memcmpeq/wmemcmp. I can move it to nptl since it is a threaded test. I will take a look at memcmpeq and wmemcmp. -- H.J.