From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS17314 8.43.84.0/22 X-Spam-Status: No, score=-2.7 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, PDS_RDNS_DYNAMIC_FP,RCVD_IN_DNSWL_MED,RDNS_DYNAMIC,SPF_HELO_PASS, SPF_PASS,URIBL_BLACK shortcircuit=no autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id DC1181F601 for ; Fri, 2 Dec 2022 01:40:14 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="mr4gsmOF"; dkim-atps=neutral Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D42F5385840C for ; Fri, 2 Dec 2022 01:40:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D42F5385840C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1669945213; bh=Ucz3sVzZgGgaRK1iHSEcCbumd7hEDZBdVUyVZjdaPKo=; h=References:In-Reply-To:Date:Subject:To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=mr4gsmOFNBkrPu/avdSbWBFb+ffSyTwHQaJbwjC7jPVetBvA0NcTiGO14X5kA2fge 71Fb3kNbe0iHlhAt+FXKWpc/ciTPeyxXxpS+79GQQJFhbOWuXJpB2rNOc8stv6aPRV 25WjqO5NwzCxK/R6AqdQtPNzhamzYv3RgqgmryS8= Received: from mail-ej1-x635.google.com (mail-ej1-x635.google.com [IPv6:2a00:1450:4864:20::635]) by sourceware.org (Postfix) with ESMTPS id 9C78A385841E for ; Fri, 2 Dec 2022 01:39:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9C78A385841E Received: by mail-ej1-x635.google.com with SMTP id o13so8398502ejm.1 for ; Thu, 01 Dec 2022 17:39:23 -0800 (PST) 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=Ucz3sVzZgGgaRK1iHSEcCbumd7hEDZBdVUyVZjdaPKo=; b=v3ueej2hCnevRdDJ/+xHX0659YJpZoezD0S5ss4Bgt8O4DACh0QUM0hPerYdRr7QXL ApwCpFj/vh7m77mJLUHMakJeWZhtMGTPE5fg4p31uibLnQZ7bmvUGMUCCABrLsehM/Qx doZw33W4sCnCYovZgqMuSLXhkH3aULyG6I0DfEmcicAnLQmK/gPgdhvymFwqNQCCVoXX Cj+TbtpvrMJ8cFN9uUzVPVkZ3MwUT+5lJoTu4/WJzat2WSO57KWY6H0hyYkTUfymDj6U S5PFMkZ+pcYTN04L3gJMmxjn9ITzYXhyJUskPfxjLQxQ65vBsV0+YZtC27YRZbNVVMIG 6lig== X-Gm-Message-State: ANoB5plYG5G61jCqF/q8Hg58ZwHA6m8WSkJZEYraAD9kc8080koNv/vY EdSKukB11qvNzm4gSqx6rQXgiXvj1c0YP7gOpOml4BwFtws= X-Google-Smtp-Source: AA0mqf5ZJbazL2tyOosZ3GMqDAGZTprYzoIfFZIBD3A67h0d5j5CQY0iG7SYx742ex/Jp4psPPsJ0G2WTLx2CXPiYC0= X-Received: by 2002:a17:906:4c98:b0:7ad:b9f3:a66a with SMTP id q24-20020a1709064c9800b007adb9f3a66amr43606120eju.282.1669945162367; Thu, 01 Dec 2022 17:39:22 -0800 (PST) MIME-Version: 1.0 References: <20221202003711.358774-1-hjl.tools@gmail.com> In-Reply-To: <20221202003711.358774-1-hjl.tools@gmail.com> Date: Thu, 1 Dec 2022 17:39:10 -0800 Message-ID: Subject: Re: [PATCH] x86-64 strncat: Properly handle the length parameter [BZ# 24097] To: "H.J. Lu" Cc: libc-alpha@sourceware.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Noah Goldstein via Libc-alpha Reply-To: Noah Goldstein Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On Thu, Dec 1, 2022 at 4:37 PM H.J. Lu wrote: > > On x32, the size_t parameter may be passed in the lower 32 bits of a > 64-bit register with the non-zero upper 32 bits. The string/memory > functions written in assembly can only use the lower 32 bits of a > 64-bit register as length or must clear the upper 32 bits before using > the full 64-bit register for length. > > This pach fixes strncat for x32. Tested on x86-64 and x32. On x86-64, > libc.so is the same with and without the fix. > --- > .../x86_64/multiarch/strcat-sse2-unaligned.S | 4 ++ > sysdeps/x86_64/multiarch/strncat-avx2.S | 4 ++ > sysdeps/x86_64/multiarch/strncat-evex.S | 5 ++ > sysdeps/x86_64/x32/Makefile | 2 +- > sysdeps/x86_64/x32/tst-size_t-strncat.c | 59 +++++++++++++++++++ > 5 files changed, 73 insertions(+), 1 deletion(-) > create mode 100644 sysdeps/x86_64/x32/tst-size_t-strncat.c > > diff --git a/sysdeps/x86_64/multiarch/strcat-sse2-unaligned.S b/sysdeps/x86_64/multiarch/strcat-sse2-unaligned.S > index 9d2ca1d504..b0f9c5fa83 100644 > --- a/sysdeps/x86_64/multiarch/strcat-sse2-unaligned.S > +++ b/sysdeps/x86_64/multiarch/strcat-sse2-unaligned.S > @@ -35,6 +35,10 @@ > ENTRY (STRCAT) > mov %rdi, %r9 > # ifdef USE_AS_STRNCAT > +# ifdef __ILP32__ > + /* Clear the upper 32 bits. */ > + movl %edx, %edx > +# endif > mov %rdx, %r8 > # endif > > diff --git a/sysdeps/x86_64/multiarch/strncat-avx2.S b/sysdeps/x86_64/multiarch/strncat-avx2.S > index ffa58bd0de..73baeee473 100644 > --- a/sysdeps/x86_64/multiarch/strncat-avx2.S > +++ b/sysdeps/x86_64/multiarch/strncat-avx2.S > @@ -51,6 +51,10 @@ > > .section SECTION(.text), "ax", @progbits > ENTRY(STRNCAT) > +# ifdef __ILP32__ > + /* Clear the upper 32 bits. */ > + movl %edx, %edx > +# endif > /* Filter zero length strings and very long strings. Zero > length strings just return, very long strings are handled by > using the non-length variant {wcs|str}cat. */ > diff --git a/sysdeps/x86_64/multiarch/strncat-evex.S b/sysdeps/x86_64/multiarch/strncat-evex.S > index bced4e8944..1ef3a22209 100644 > --- a/sysdeps/x86_64/multiarch/strncat-evex.S > +++ b/sysdeps/x86_64/multiarch/strncat-evex.S > @@ -79,6 +79,11 @@ > > .section SECTION(.text), "ax", @progbits > ENTRY(STRNCAT) > +# ifdef __ILP32__ > + /* Clear the upper 32 bits. */ > + movl %edx, %edx > +# endif > + > movq %rdi, %rax > > /* NB: It's safe to filter out zero-length strings WITHOUT > diff --git a/sysdeps/x86_64/x32/Makefile b/sysdeps/x86_64/x32/Makefile > index 31732aa248..a015789a4f 100644 > --- a/sysdeps/x86_64/x32/Makefile > +++ b/sysdeps/x86_64/x32/Makefile > @@ -10,7 +10,7 @@ ifeq ($(subdir),string) > tests += tst-size_t-memchr tst-size_t-memcmp tst-size_t-memcpy \ > tst-size_t-memrchr tst-size_t-memset tst-size_t-strncasecmp \ > tst-size_t-strncmp tst-size_t-strncpy tst-size_t-strnlen \ > - tst-size_t-memcmp-2 > + tst-size_t-memcmp-2 tst-size_t-strncat > endif > > ifeq ($(subdir),wcsmbs) > diff --git a/sysdeps/x86_64/x32/tst-size_t-strncat.c b/sysdeps/x86_64/x32/tst-size_t-strncat.c > new file mode 100644 > index 0000000000..eda93f5af9 > --- /dev/null > +++ b/sysdeps/x86_64/x32/tst-size_t-strncat.c > @@ -0,0 +1,59 @@ > +/* Test strncat with size_t in the lower 32 bits of 64-bit register. > + 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_NAME "strncat" > +#include "test-size_t.h" > + > +IMPL (strncat, 1) > + > +typedef char *(*proto_t) (char *, const char*, size_t); > + > +static void * > +__attribute__ ((noinline, noclone)) > +do_strncat (parameter_t a, parameter_t b) > +{ > + return CALL (&b, a.p, b.p, a.len); > +} > + > +static int > +test_main (void) > +{ > + test_init (); > + > + parameter_t dest = { { page_size - 1 }, buf1 }; > + parameter_t src = { { 0 }, buf2 }; > + > + int ret = 0; > + FOR_EACH_IMPL (impl, 0) > + { > + src.fn = impl->fn; > + buf1[0] = '\0'; > + do_strncat (dest, src); > + int res = strncmp (dest.p, src.p, dest.len); > + if (res) > + { > + error (0, 0, "Wrong result in function %s: %i != 0", > + impl->name, res); > + ret = 1; > + } > + } > + > + return ret ? EXIT_FAILURE : EXIT_SUCCESS; > +} > + > +#include > -- > 2.38.1 > LGTM. Reviewed-by: Noah Goldstein