From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 070793858C55 for ; Thu, 23 Jun 2022 16:37:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 070793858C55 Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-182-WhVoHFwwMced6SfMFH21ug-1; Thu, 23 Jun 2022 12:37:13 -0400 X-MC-Unique: WhVoHFwwMced6SfMFH21ug-1 Received: by mail-ed1-f70.google.com with SMTP id z20-20020a05640235d400b0042dfc1c0e80so16086977edc.21 for ; Thu, 23 Jun 2022 09:37:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=LGM1jmoDONakItNk5g27GAvHZ7UJvST5x9DfsSrpLJs=; b=yK6/zafw2Pm+9P5xdOZwshg3cvD7NihE7AIVly1JWW7hzFM0KAle3NeBCE8bbUfLG2 pKY8LCh0IO2iZawHRxE7B2P7pVfvjqe55nCZTtM0euOtmptDcoSIn4Wx80Av2AiHGFVd S57/S7kUswfs0G36g74PtQ4AkQDM9gnI18AztrmJ6hXpMcbHv7gp/IOmXjYrUxbO9mT5 Nq7D0J3kEJo80qr0KgHbOdgnN23ZarcEQ2SpMMsQOrlkmIYaKWUkLlsCQvxNSmyIuc4/ 7IIEQAhn4yecsCD71iDtKVCDGU/dggXS8PjNdOONZZdkSAqELERMiNykGbAUPNcALzae eABQ== X-Gm-Message-State: AJIora8iOp4o7fpVd2m+SDcYVnMWN2AMrTYtgJzcwh/4A7Glhzv3P4i8 fsY6uoIZMHQG7WXkJR+nmIHFKIq5hIPIA1EkLAHq7KstmUeioo5OBUElhuCYGMeiN9r+npphopN bpN54+K5fZosf3x/HWJCXxxtGTsRdo9M= X-Received: by 2002:a17:906:8301:b0:6e4:896d:59b1 with SMTP id j1-20020a170906830100b006e4896d59b1mr8774155ejx.396.1656002231271; Thu, 23 Jun 2022 09:37:11 -0700 (PDT) X-Google-Smtp-Source: AGRyM1sP0ijmszbh4aI6E8S9ozA4HrB5K8ljXdM0/VCzXZhYadOK7GnWcVBE0JdVmsZq5hTuRG06DCjcDns8F0O25Ik= X-Received: by 2002:a17:906:8301:b0:6e4:896d:59b1 with SMTP id j1-20020a170906830100b006e4896d59b1mr8774124ejx.396.1656002230906; Thu, 23 Jun 2022 09:37:10 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Jonathan Wakely Date: Thu, 23 Jun 2022 17:36:59 +0100 Message-ID: Subject: Re: [PATCH] libstdc++: testsuite: avoid predictable mkstemp To: Alexandre Oliva , Joel Brobecker Cc: gcc Patches , "libstdc++" X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="000000000000b8a29a05e220103d" X-Spam-Status: No, score=-13.1 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=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Jun 2022 16:37:18 -0000 --000000000000b8a29a05e220103d Content-Type: text/plain; charset="UTF-8" On Thu, 23 Jun 2022 at 12:39, Alexandre Oliva wrote: > > On Jun 22, 2022, Jonathan Wakely wrote: > > > On Wed, 22 Jun 2022 at 07:05, Alexandre Oliva via Libstdc++ > > wrote: > > >> It was prompted by a target system with a non-random implementation of > >> mkstemp, that returns a predictable sequence of filenames and selects > >> the first one that isn't already taken. > > > OK > > And here's the patch that enabled me to stop worrying about the above. > Regstrapped on x86_64-linux-gnu, also tested with a cross to > aarch64-rtems6. Ok to install? > > > __gnu_test::nonexistent_path: Always include counter in filename returned > > From: Joel Brobecker > > 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. > > 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 | 31 ++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h > index 037d9ffc0f429..206ea67779003 100644 > --- a/libstdc++-v3/testsuite/util/testsuite_fs.h > +++ b/libstdc++-v3/testsuite/util/testsuite_fs.h > @@ -38,9 +38,9 @@ namespace test_fs = std::experimental::filesystem; > > #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L > #include // mkstemp > -#else > -#include // std::random_device > +#include // strcpy > #endif > +#include // std::random_device > > #if defined(__MINGW32__) || defined(__MINGW64__) \ > || !defined (_GLIBCXX_HAVE_SYMLINK) > @@ -125,8 +125,32 @@ namespace __gnu_test > file.erase(0, pos+1); > > test_fs::path p; > + // A counter, starting from a random value, to be included as part > + // of the filename being returned, and incremented each time > + // this method is used. It allows us to ensure that two calls > + // to this method 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{}(); > + > #if defined(_GNU_SOURCE) || _XOPEN_SOURCE >= 500 || _POSIX_C_SOURCE >= 200112L > - char tmp[] = "filesystem-test.XXXXXX"; > + // 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 include a counter in the template passed to mkstemp. > + std::string mkstemp_template ("filesystem-test."); > + mkstemp_template.append(std::to_string (counter++)); > + mkstemp_template.append(".XXXXXX"); > + > + char tmp[mkstemp_template.length() + 1]; > + std::strcpy (tmp, mkstemp_template.c_str()); std::string::copy can be used to copy a string into a buffer, but there's no reason to use another buffer anyway. The attached makes this a bit more efficient, and makes more of the code common to the mkstemp and non-mkstmp branches. I'll wait to hear back from you before pushing it (since it has Joel's name on the patch). --000000000000b8a29a05e220103d Content-Type: text/plain; charset="US-ASCII"; name="patch.txt" Content-Disposition: attachment; filename="patch.txt" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_l4r8xtc50 Y29tbWl0IDgzMzMzN2Q4NjFlMzEyNmY2MzA0ZThiYzlkOGU3NDMwMjJhMjE0MjQKQXV0aG9yOiBK b2VsIEJyb2JlY2tlciA8YnJvYmVja2VyQGFkYWNvcmUuY29tPgpEYXRlOiAgIFRodSBKdW4gMjMg MTM6MTI6MTIgMjAyMgoKICAgIGxpYnN0ZGMrKzogdGVzdHN1aXRlOiBhdm9pZCBwcmVkaWNhYmxl IG1rc3RlbXAKICAgIAogICAgV2UgaGF2ZSBub3RpY2VkIHRoYXQsIG9uIFJURU1TLCBhIHNtYWxs IG51bWJlciBvZiB0ZXN0c2Nhc2VzIGFyZQogICAgZmFpbGluZyBiZWNhdXNlIHR3byBjYWxscyB0 byB0aGlzIG1ldGhvZCByZXR1cm4gdGhlIHNhbWUgZmlsZW5hbWUuCiAgICBUaGlzIGhhcHBlbnMg Zm9yIGluc3RhbmNlIGluIDI3X2lvL2ZpbGVzeXN0ZW0vb3BlcmF0aW9ucy9jb3B5X2ZpbGUuY2MK ICAgIHdoZXJlIGl0IGRvZXM6CiAgICAKICAgICAgYXV0byBmcm9tID0gX19nbnVfdGVzdDo6bm9u ZXhpc3RlbnRfcGF0aCgpOwogICAgICBhdXRvIHRvID0gX19nbnVfdGVzdDo6bm9uZXhpc3RlbnRf cGF0aCgpOwogICAgCiAgICBXZSB0cmFja2VkIHRoaXMgaXNzdWUgZG93biB0byB0aGUgZmFjdCB0 aGF0IHRoZSBpbXBsZW1lbnRhdGlvbiBvZgogICAgbWtzdGVtcCBvbiB0aGF0IHN5c3RlbSBhcHBl YXJzIHRvIHVzZSBhIHZlcnkgcHJlZGljdGFibGUgYWxnb3JpdGhtCiAgICBmb3IgY2hvc2luZyB0 aGUgbmFtZSBvZiB0aGUgdGVtcG9yYXJ5IGZpbGUsIHdoZXJlIHRoZSBzYW1lIGZpbGVuYW1lCiAg ICBhcHBlYXJzIHRvIGJlIHRyaWVkIGluIHRoZSBzYW1lIG9yZGVyLCByZWdhcmRsZXNzIG9mIHBh c3QgY2FsbHMuCiAgICBTbywgYXMgbG9uZyBhcyB0aGUgZmlsZSBnZXRzIGRlbGV0ZWQgYWZ0ZXIg YSBjYWxsIHRvIG1rc3RlbXAgKHNvbWV0aGluZwogICAgd2UgZG8gaGVyZSBpbiBvdXIgbm9uZXhp c3RlbnRfcGF0aCBtZXRob2QpLCB0aGUgbmV4dCBjYWxsIHRvIG1rc3RlbXBzCiAgICBlbmRzIHVw IHJldHVybmluZyB0aGUgc2FtZSBmaWxlbmFtZSwgY2F1c2luZyB0aGUgY29sbGlzaW9uIHdlIHNl IGFib3ZlLgogICAgCiAgICBUaGlzIGNvbW1pdCBlbmhhbmNlcyB0aGUgX19nbnVfdGVzdDo6bm9u ZXhpc3RlbnRfcGF0aCBtZXRob2QgdG8KICAgIGludHJvZHVjZSBpbiB0aGUgZmlsZW5hbWUgYmVp bmcgcmV0dXJuZWQgYSBjb3VudGVyIHdoaWNoIGdldHMKICAgIGluY3JlbWVudGVkIGF0IGV2ZXJ5 IGNhbGwgb2YgdGhpcyBtZXRob2QuCiAgICAKICAgIENvLWF1dGhvcmVkLWJ5OiBKb25hdGhhbiBX YWtlbHkgPGp3YWtlbHlAcmVkaGF0LmNvbT4KICAgIAogICAgbGlic3RkYysrLXYzL0NoYW5nZUxv ZzoKICAgIAogICAgICAgICAgICAqIHRlc3RzdWl0ZS91dGlsL3Rlc3RzdWl0ZV9mcy5oIChfX2du dV90ZXN0Ojpub25leGlzdGVudF9wYXRoKToKICAgICAgICAgICAgQWx3YXlzIGluY2x1ZGUgYSBj b3VudGVyIGluIHRoZSBmaWxlbmFtZSByZXR1cm5lZC4KCmRpZmYgLS1naXQgYS9saWJzdGRjKyst djMvdGVzdHN1aXRlL3V0aWwvdGVzdHN1aXRlX2ZzLmggYi9saWJzdGRjKystdjMvdGVzdHN1aXRl L3V0aWwvdGVzdHN1aXRlX2ZzLmgKaW5kZXggOTM1OGEwNGU1NmMuLjNhMTk3YzcwYTFlIDEwMDY0 NAotLS0gYS9saWJzdGRjKystdjMvdGVzdHN1aXRlL3V0aWwvdGVzdHN1aXRlX2ZzLmgKKysrIGIv bGlic3RkYysrLXYzL3Rlc3RzdWl0ZS91dGlsL3Rlc3RzdWl0ZV9mcy5oCkBAIC0zMiwxNCArMzIs MTQgQEAgbmFtZXNwYWNlIHRlc3RfZnMgPSBzdGQ6OmV4cGVyaW1lbnRhbDo6ZmlsZXN5c3RlbTsK ICNlbmRpZgogI2luY2x1ZGUgPGFsZ29yaXRobT4KICNpbmNsdWRlIDxmc3RyZWFtPgorI2luY2x1 ZGUgPHJhbmRvbT4gICAvLyBzdGQ6OnJhbmRvbV9kZXZpY2UKICNpbmNsdWRlIDxzdHJpbmc+Cisj aW5jbHVkZSA8c3lzdGVtX2Vycm9yPgogI2luY2x1ZGUgPGNzdGRpbz4KICNpbmNsdWRlIDx1bmlz dGQuaD4gLy8gdW5saW5rLCBjbG9zZSwgZ2V0cGlkLCBnZXRldWlkCiAKICNpZiBkZWZpbmVkKF9H TlVfU09VUkNFKSB8fCBfWE9QRU5fU09VUkNFID49IDUwMCB8fCBfUE9TSVhfQ19TT1VSQ0UgPj0g MjAwMTEyTAogI2luY2x1ZGUgPHN0ZGxpYi5oPiAvLyBta3N0ZW1wCi0jZWxzZQotI2luY2x1ZGUg PHJhbmRvbT4gICAvLyBzdGQ6OnJhbmRvbV9kZXZpY2UKICNlbmRpZgogCiBuYW1lc3BhY2UgX19n bnVfdGVzdApAQCAtMTA5LDMyICsxMDksNTEgQEAgbmFtZXNwYWNlIF9fZ251X3Rlc3QKICAgICBp ZiAocG9zICE9IGZpbGUubnBvcykKICAgICAgIGZpbGUuZXJhc2UoMCwgcG9zKzEpOwogCisgICAg ZmlsZS5yZXNlcnZlKGZpbGUuc2l6ZSgpICsgNDApOworICAgIGZpbGUuaW5zZXJ0KDAsICJmaWxl c3lzdGVtLXRlc3QuIik7CisKKyAgICAvLyBBIGNvdW50ZXIsIHN0YXJ0aW5nIGZyb20gYSByYW5k b20gdmFsdWUsIHRvIGJlIGluY2x1ZGVkIGFzIHBhcnQKKyAgICAvLyBvZiB0aGUgZmlsZW5hbWUg YmVpbmcgcmV0dXJuZWQsIGFuZCBpbmNyZW1lbnRlZCBlYWNoIHRpbWUKKyAgICAvLyB0aGlzIGZ1 bmN0aW9uIGlzIHVzZWQuICBJdCBhbGxvd3MgdXMgdG8gZW5zdXJlIHRoYXQgdHdvIGNhbGxzCisg ICAgLy8gdG8gdGhpcyBmdW5jdGlvbiBjYW4gbmV2ZXIgcmV0dXJuIHRoZSBzYW1lIGZpbGVuYW1l LCBzb21ldGhpbmcKKyAgICAvLyB0ZXN0Y2FzZXMgZG8gd2hlbiB0aGV5IG5lZWQgbXVsdGlwbGUg bm9uLWV4aXN0ZW50IGZpbGVuYW1lcworICAgIC8vIGZvciB0aGVpciBwdXJwb3Nlcy4KKyAgICBz dGF0aWMgdW5zaWduZWQgY291bnRlciA9IHN0ZDo6cmFuZG9tX2RldmljZXt9KCk7CisgICAgZmls ZSArPSAnLic7CisgICAgZmlsZSArPSBzdGQ6OnRvX3N0cmluZyhjb3VudGVyKyspOworICAgIGZp bGUgKz0gJy4nOworCiAgICAgdGVzdF9mczo6cGF0aCBwOwogI2lmIGRlZmluZWQoX0dOVV9TT1VS Q0UpIHx8IF9YT1BFTl9TT1VSQ0UgPj0gNTAwIHx8IF9QT1NJWF9DX1NPVVJDRSA+PSAyMDAxMTJM Ci0gICAgY2hhciB0bXBbXSA9ICJmaWxlc3lzdGVtLXRlc3QuWFhYWFhYIjsKLSAgICBpbnQgZmQg PSA6Om1rc3RlbXAodG1wKTsKKworICAgIC8vIFVzZSBta3N0ZW1wIHRvIGRldGVybWluZSB0aGUg bmFtZSBvZiBhIGZpbGUgd2hpY2ggZG9lcyBub3QgZXhpc3QgeWV0LgorICAgIC8vCisgICAgLy8g Tm90ZSB0aGF0IHdlIGhhdmUgc2VlbiBvbiBzb21lIHN5c3RlbXMgKHN1Y2ggYXMgUlRFTVMsIGZv ciBpbnN0YW5jZSkKKyAgICAvLyB0aGF0IG1rc3RlbXAgYmVoYXZlcyB2ZXJ5IHByZWRpY3RhYmx5 LCBjYXVzaW5nIGl0IHRvIGFsd2F5cyB0cnkKKyAgICAvLyB0aGUgc2FtZSBzZXF1ZW5jZSBvZiBm aWxlIG5hbWVzLiAgSW4gb3RoZXIgd29yZHMsIGlmIHdlIGNhbGwgbWtzdGVtcAorICAgIC8vIHdp dGggYSBwYXR0ZXJuLCBkZWxldGUgdGhlIGZpbGUgaXQgY3JlYXRlZCAod2hpY2ggaXMgd2hhdCB3 ZSBkbywgaGVyZSksCisgICAgLy8gYW5kIGNhbGwgbWtzdGVtcCB3aXRoIHRoZSBzYW1lIHBhdHRl cm4gYWdhaW4sIGl0IHJldHVybnMgdGhlIHNhbWUKKyAgICAvLyBmaWxlbmFtZSBvbmNlIG1vcmUu ICBXaGlsZSBtb3N0IGltcGxlbWVudGF0aW9ucyBpbnRyb2R1Y2UgYSBkZWdyZWUKKyAgICAvLyBv ZiByYW5kb21uZXNzLCBpdCBpcyBub3QgbWFuZGF0ZWQgYnkgdGhlIHN0YW5kYXJkLCBhbmQgdGhp cyBpcyB3aHkKKyAgICAvLyB3ZSBhbHNvIGluY2x1ZGUgYSBjb3VudGVyIGluIHRoZSB0ZW1wbGF0 ZSBwYXNzZWQgdG8gbWtzdGVtcC4KKyAgICBmaWxlICs9ICJYWFhYWFgiOworICAgIGludCBmZCA9 IDo6bWtzdGVtcCgmZmlsZVswXSk7CiAgICAgaWYgKGZkID09IC0xKQogICAgICAgdGhyb3cgdGVz dF9mczo6ZmlsZXN5c3RlbV9lcnJvcigibWtzdGVtcCBmYWlsZWQiLAogCSAgc3RkOjplcnJvcl9j b2RlKGVycm5vLCBzdGQ6OmdlbmVyaWNfY2F0ZWdvcnkoKSkpOwotICAgIDo6dW5saW5rKHRtcCk7 CisgICAgOjp1bmxpbmsoZmlsZS5jX3N0cigpKTsKICAgICA6OmNsb3NlKGZkKTsKLSAgICBpZiAo IWZpbGUuZW1wdHkoKSkKLSAgICAgIGZpbGUuaW5zZXJ0KDAsIDEsICctJyk7Ci0gICAgZmlsZS5p bnNlcnQoMCwgdG1wKTsKLSAgICBwID0gZmlsZTsKKyAgICBwID0gc3RkOjptb3ZlKGZpbGUpOwog I2Vsc2UKICAgICBpZiAoZmlsZS5sZW5ndGgoKSA+IDY0KQogICAgICAgZmlsZS5yZXNpemUoNjQp OwotICAgIGNoYXIgYnVmWzEyOF07Ci0gICAgc3RhdGljIHVuc2lnbmVkIGNvdW50ZXIgPSBzdGQ6 OnJhbmRvbV9kZXZpY2V7fSgpOwotI2lmIF9HTElCQ1hYX1VTRV9DOTlfU1RESU8KLSAgICBzdGQ6 OnNucHJpbnRmKGJ1ZiwgMTI4LAotI2Vsc2UKLSAgICBzdGQ6OnNwcmludGYoYnVmLAotI2VuZGlm Ci0gICAgICAiZmlsZXN5c3RlbS10ZXN0LiV1LiVsdS0lcyIsIGNvdW50ZXIrKywgKHVuc2lnbmVk IGxvbmcpIDo6Z2V0cGlkKCksCi0gICAgICBmaWxlLmNfc3RyKCkpOwotICAgIHAgPSBidWY7Cisg ICAgLy8gVGhlIGNvbWJpbmF0aW9uIG9mIHJhbmRvbSBjb3VudGVyIGFuZCBQSUQgc2hvdWxkIGJl IHVuaXF1ZSBmb3IgYSBnaXZlbgorICAgIC8vIHJ1biBvZiB0aGUgdGVzdHN1aXRlLgorICAgIGZp bGUgKz0gc3RkOjp0b19zdHJpbmcoOjpnZXRwaWQoKSk7CisgICAgcCA9IHN0ZDo6bW92ZShmaWxl KTsKKyAgICBpZiAodGVzdF9mczo6ZXhpc3RzKHApKQorICAgICAgdGhyb3cgdGVzdF9mczo6Zmls ZXN5c3RlbV9lcnJvcigiRmFpbGVkIHRvIGdlbmVyYXRlIHVuaXF1ZSBwYXRobmFtZSIsIHAsCisJ ICBzdGQ6Om1ha2VfZXJyb3JfY29kZShzdGQ6OmVycmM6OmZpbGVfZXhpc3RzKSk7CiAjZW5kaWYK ICAgICByZXR1cm4gcDsKICAgfQo= --000000000000b8a29a05e220103d--