[llvm] 4687b45 - Reland D78945 TarWriter: Only use 137 of the 155 prefix bytes.

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2020-04-27T10:37:23-07:00
New Revision: 4687b454339b9072e15df55e52ae8bc2a266225d

URL: https://github.com/llvm/llvm-project/commit/4687b454339b9072e15df55e52ae8bc2a266225d
DIFF: https://github.com/llvm/llvm-project/commit/4687b454339b9072e15df55e52ae8bc2a266225d.diff

LOG: Reland D78945 TarWriter: Only use 137 of the 155 prefix bytes.

With a fix to unittests/Support/TarWriterTest.cpp

This makes lld's --reproduce output more compatible with tar 1.13 and
before.  This is a very old version of tar, but it's the version in
both gnuwin and unxutils, and the cost for supporting them are very
low, so we might as well just do that.

https://bugs.chromium.org/p/chromium/issues/detail?id=1073524#c21
and onward has more details.

Differential Revision: https://reviews.llvm.org/D78945

Added: 
    

Modified: 
    llvm/lib/Support/TarWriter.cpp
    llvm/unittests/Support/TarWriterTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/TarWriter.cpp b/llvm/lib/Support/TarWriter.cpp
index 6c62f8861afd..c7a744f0fc98 100644
--- a/llvm/lib/Support/TarWriter.cpp
+++ b/llvm/lib/Support/TarWriter.cpp
@@ -131,7 +131,17 @@ static bool splitUstar(StringRef Path, StringRef &Prefix, StringRef &Name) {
     return true;
   }
 
-  size_t Sep = Path.rfind('/', sizeof(UstarHeader::Prefix) + 1);
+  // tar 1.13 and earlier unconditionally look at the tar header interpreted
+  // as an 'oldgnu_header', which has an 'isextended' byte at offset 482 in the
+  // header, corresponding to offset 137 in the prefix. That's the version of
+  // tar in gnuwin, so only use 137 of the 155 bytes in the prefix. This means
+  // we'll need a pax header after 237 bytes of path instead of after 255,
+  // but in return paths up to 237 bytes work with gnuwin, instead of just
+  // 137 bytes of directory + 100 bytes of basename previously.
+  // (tar-1.13 also doesn't support pax headers, but in practice all paths in
+  // llvm's test suite are short enough for that to not matter.)
+  const int MaxPrefix = 137;
+  size_t Sep = Path.rfind('/', MaxPrefix + 1);
   if (Sep == StringRef::npos)
     return false;
   if (Path.size() - Sep - 1 >= sizeof(UstarHeader::Name))

diff  --git a/llvm/unittests/Support/TarWriterTest.cpp b/llvm/unittests/Support/TarWriterTest.cpp
index bd67e036518f..5bbc6615ea5d 100644
--- a/llvm/unittests/Support/TarWriterTest.cpp
+++ b/llvm/unittests/Support/TarWriterTest.cpp
@@ -81,30 +81,30 @@ TEST_F(TarWriterTest, Basics) {
 }
 
 TEST_F(TarWriterTest, LongFilename) {
-  std::string x154(154, 'x');
-  std::string x155(155, 'x');
+  std::string x136(136, 'x');
+  std::string x137(137, 'x');
   std::string y99(99, 'y');
   std::string y100(100, 'y');
 
-  UstarHeader Hdr1 = createUstar("", x154 + "/" + y99);
-  EXPECT_EQ("/" + x154, StringRef(Hdr1.Prefix));
+  UstarHeader Hdr1 = createUstar("", x136 + "/" + y99);
+  EXPECT_EQ("/" + x136, StringRef(Hdr1.Prefix));
   EXPECT_EQ(y99, StringRef(Hdr1.Name));
 
-  UstarHeader Hdr2 = createUstar("", x155 + "/" + y99);
+  UstarHeader Hdr2 = createUstar("", x137 + "/" + y99);
   EXPECT_EQ("", StringRef(Hdr2.Prefix));
   EXPECT_EQ("", StringRef(Hdr2.Name));
 
-  UstarHeader Hdr3 = createUstar("", x154 + "/" + y100);
+  UstarHeader Hdr3 = createUstar("", x136 + "/" + y100);
   EXPECT_EQ("", StringRef(Hdr3.Prefix));
   EXPECT_EQ("", StringRef(Hdr3.Name));
 
-  UstarHeader Hdr4 = createUstar("", x155 + "/" + y100);
+  UstarHeader Hdr4 = createUstar("", x137 + "/" + y100);
   EXPECT_EQ("", StringRef(Hdr4.Prefix));
   EXPECT_EQ("", StringRef(Hdr4.Name));
 
   std::string yz = "yyyyyyyyyyyyyyyyyyyy/zzzzzzzzzzzzzzzzzzzz";
-  UstarHeader Hdr5 = createUstar("", x154 + "/" + yz);
-  EXPECT_EQ("/" + x154, StringRef(Hdr5.Prefix));
+  UstarHeader Hdr5 = createUstar("", x136 + "/" + yz);
+  EXPECT_EQ("/" + x136, StringRef(Hdr5.Prefix));
   EXPECT_EQ(yz, StringRef(Hdr5.Name));
 }