From patchwork Mon Jun 27 10:17:52 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 55437 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E671538418B1 for ; Mon, 27 Jun 2022 10:24:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E671538418B1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1656325483; bh=bCR94tN6oUNiFgQ5h2RNPyolCDWb7pJUdDNaFey7hr4=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=hyWD4yZLLeCSZjXT3rfrRmLZA4lNefwAy86Hs10Nxe3XAtjIOkTFmHvEBnssPuD5Z foryMqji42m5jZPbYx73+y0wbrUthUhpA506j0h6y8CPrfLoEJNQpTkwgAQi2ilia0 N+69YT/qdexkFwmIpcDwavCgdJJE1xmCsNeU87OI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 647FB384188A for ; Mon, 27 Jun 2022 10:17:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 647FB384188A Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-611-ZOZQBnJnP3iQ_Ny_v285aA-1; Mon, 27 Jun 2022 06:17:54 -0400 X-MC-Unique: ZOZQBnJnP3iQ_Ny_v285aA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id BB5162806AAF; Mon, 27 Jun 2022 10:17:53 +0000 (UTC) Received: from localhost (unknown [10.33.36.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6A8282166B26; Mon, 27 Jun 2022 10:17:53 +0000 (UTC) To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: testsuite: avoid predicable mkstemp Date: Mon, 27 Jun 2022 11:17:52 +0100 Message-Id: <20220627101752.34067-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jonathan Wakely via Gcc-patches From: Jonathan Wakely Reply-To: Jonathan Wakely Cc: Joel Brobecker Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" From: Joel Brobecker Tested powerpc64le-linux, pushed to trunk. -- >8 -- We have noticed that, on RTEMS, a small number of testscases are failing because two calls to this method return the same filename. This happens for instance in 27_io/filesystem/operations/copy_file.cc where it does: auto from = __gnu_test::nonexistent_path(); auto to = __gnu_test::nonexistent_path(); We tracked this issue down to the fact that the implementation of mkstemp on that system appears to use a very predictable algorithm for chosing the name of the temporary file, where the same filename appears to be tried in the same order, regardless of past calls. So, as long as the file gets deleted after a call to mkstemp (something we do here in our nonexistent_path method), the next call to mkstemps ends up returning the same filename, causing the collision we se above. This commit enhances the __gnu_test::nonexistent_path method to introduce in the filename being returned a counter which gets incremented at every call of this method. Co-authored-by: Jonathan Wakely libstdc++-v3/ChangeLog: * testsuite/util/testsuite_fs.h (__gnu_test::nonexistent_path): Always include a counter in the filename returned. --- libstdc++-v3/testsuite/util/testsuite_fs.h | 57 ++++++++++++++-------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h index 29d0b029b75..908fcdbcaee 100644 --- a/libstdc++-v3/testsuite/util/testsuite_fs.h +++ b/libstdc++-v3/testsuite/util/testsuite_fs.h @@ -32,14 +32,14 @@ namespace test_fs = std::experimental::filesystem; #endif #include #include +#include // std::random_device #include +#include #include #include // unlink, close, getpid, geteuid #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L #include // mkstemp -#else -#include // std::random_device #endif #ifndef _GLIBCXX_HAVE_SYMLINK @@ -123,32 +123,51 @@ namespace __gnu_test if (pos != file.npos) file.erase(0, pos+1); + file.reserve(file.size() + 40); + file.insert(0, "filesystem-test."); + + // A counter, starting from a random value, to be included as part + // of the filename being returned, and incremented each time + // this function is used. It allows us to ensure that two calls + // to this function can never return the same filename, something + // testcases do when they need multiple non-existent filenames + // for their purposes. + static unsigned counter = std::random_device{}(); + file += '.'; + file += std::to_string(counter++); + file += '.'; + test_fs::path p; #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L - char tmp[] = "filesystem-test.XXXXXX"; - int fd = ::mkstemp(tmp); + + // Use mkstemp to determine the name of a file which does not exist yet. + // + // Note that we have seen on some systems (such as RTEMS, for instance) + // that mkstemp behaves very predictably, causing it to always try + // the same sequence of file names. In other words, if we call mkstemp + // with a pattern, delete the file it created (which is what we do, here), + // and call mkstemp with the same pattern again, it returns the same + // filename once more. While most implementations introduce a degree + // of randomness, it is not mandated by the standard, and this is why + // we also include a counter in the template passed to mkstemp. + file += "XXXXXX"; + int fd = ::mkstemp(&file[0]); if (fd == -1) throw test_fs::filesystem_error("mkstemp failed", std::error_code(errno, std::generic_category())); - ::unlink(tmp); + ::unlink(file.c_str()); ::close(fd); - if (!file.empty()) - file.insert(0, 1, '-'); - file.insert(0, tmp); - p = file; + p = std::move(file); #else if (file.length() > 64) file.resize(64); - char buf[128]; - static unsigned counter = std::random_device{}(); -#if _GLIBCXX_USE_C99_STDIO - std::snprintf(buf, 128, -#else - std::sprintf(buf, -#endif - "filesystem-test.%u.%lu-%s", counter++, (unsigned long) ::getpid(), - file.c_str()); - p = buf; + // The combination of random counter and PID should be unique for a given + // run of the testsuite. + file += std::to_string(::getpid()); + p = std::move(file); + if (test_fs::exists(p)) + throw test_fs::filesystem_error("Failed to generate unique pathname", p, + std::make_error_code(std::errc::file_exists)); #endif return p; }