Various Dwarf::Path fixes

Summary:
- Allow both `baseDir` and `subDir` to be empty.  This can happen
if DW_AT_comp_dir is `.` and gets simplified to the empty string.
(The DWARF standard doesn't appear to disallow relative dirs here,
but I couldn't find anthing definitive).
- Fix `./` prefix  stripping to avoid making paths like `.///hello` absolute.
- Fix `/` suffix stripping to avoid making the root dir reference (`/`) empty.
- Do `baseDir` and `subDir` swapping after simplification.

Test Plan: Added unittests.

Reviewed By: njormrod@fb.com

Subscribers: trunkagent, sdwilsh, folly-diffs@

FB internal diff: D1747978

Signature: t1:1747978:1419014942:87f14cb31b8c19b524d7a95b14d63cf5661a8634

diff --git a/folly/experimental/symbolizer/Dwarf.cpp b/folly/experimental/symbolizer/Dwarf.cpp
index 04473cfa01e7b3fe4123d18cb6b7891d22a6c93b..d2385aa55619269a5accce649add0edb800a0aa6 100644 (file)
--- a/folly/experimental/symbolizer/Dwarf.cpp
+++ b/folly/experimental/symbolizer/Dwarf.cpp
@@ -131,6 +131,10 @@ void simplifyPath(folly::StringPiece& sp) {
     }
 
     if (sp.removePrefix("./")) {
+      // Also remove any subsequent slashes to avoid making this path absolute.
+      while (sp.startsWith('/')) {
+        sp.advance(1);
+      }
       continue;
     }
 
@@ -143,8 +147,8 @@ void simplifyPath(folly::StringPiece& sp) {
       return;
     }
 
-    // Strip trailing slashes
-    while (sp.removeSuffix('/')) { }
+    // Strip trailing slashes, except when this is the root path.
+    while (sp.size() > 1 && sp.removeSuffix('/')) { }
 
     if (sp.removeSuffix("/.")) {
       continue;
@@ -180,29 +184,42 @@ Dwarf::Path::Path(folly::StringPiece baseDir, folly::StringPiece subDir,
     baseDir_.clear();  // subDir_ is absolute
   }
 
-  // Make sure that baseDir_ isn't empty; subDir_ may be
-  if (baseDir_.empty()) {
-    swap(baseDir_, subDir_);
-  }
-
   simplifyPath(baseDir_);
   simplifyPath(subDir_);
   simplifyPath(file_);
+
+  // Make sure it's never the case that baseDir_ is empty, but subDir_ isn't.
+  if (baseDir_.empty()) {
+    swap(baseDir_, subDir_);
+  }
 }
 
 size_t Dwarf::Path::size() const {
-  if (baseDir_.empty()) {
-    assert(subDir_.empty());
-    return file_.size();
+  size_t size = 0;
+  bool needsSlash = false;
+
+  if (!baseDir_.empty()) {
+    size += baseDir_.size();
+    needsSlash = !baseDir_.endsWith('/');
+  }
+
+  if (!subDir_.empty()) {
+    size += needsSlash;
+    size += subDir_.size();
+    needsSlash = !subDir_.endsWith('/');
   }
 
-  return
-    baseDir_.size() + !subDir_.empty() + subDir_.size() + !file_.empty() +
-    file_.size();
+  if (!file_.empty()) {
+    size += needsSlash;
+    size += file_.size();
+  }
+
+  return size;
 }
 
 size_t Dwarf::Path::toBuffer(char* buf, size_t bufSize) const {
   size_t totalSize = 0;
+  bool needsSlash = false;
 
   auto append = [&] (folly::StringPiece sp) {
     if (bufSize >= 2) {
@@ -216,14 +233,17 @@ size_t Dwarf::Path::toBuffer(char* buf, size_t bufSize) const {
 
   if (!baseDir_.empty()) {
     append(baseDir_);
+    needsSlash = !baseDir_.endsWith('/');
   }
   if (!subDir_.empty()) {
-    assert(!baseDir_.empty());
-    append("/");
+    if (needsSlash) {
+      append("/");
+    }
     append(subDir_);
+    needsSlash = !subDir_.endsWith('/');
   }
   if (!file_.empty()) {
-    if (!baseDir_.empty()) {
+    if (needsSlash) {
       append("/");
     }
     append(file_);
@@ -237,17 +257,23 @@ size_t Dwarf::Path::toBuffer(char* buf, size_t bufSize) const {
 
 void Dwarf::Path::toString(std::string& dest) const {
   size_t initialSize = dest.size();
+  bool needsSlash = false;
   dest.reserve(initialSize + size());
   if (!baseDir_.empty()) {
     dest.append(baseDir_.begin(), baseDir_.end());
+    needsSlash = baseDir_.endsWith('/');
   }
   if (!subDir_.empty()) {
-    assert(!baseDir_.empty());
-    dest.push_back('/');
+    if (needsSlash) {
+      dest.push_back('/');
+    }
     dest.append(subDir_.begin(), subDir_.end());
+    needsSlash = subDir_.endsWith('/');
   }
   if (!file_.empty()) {
-    dest.push_back('/');
+    if (needsSlash) {
+      dest.push_back('/');
+    }
     dest.append(file_.begin(), file_.end());
   }
   assert(dest.size() == initialSize + size());

diff --git a/folly/experimental/symbolizer/test/DwarfTests.cpp b/folly/experimental/symbolizer/test/DwarfTests.cpp
new file mode 100644 (file)
index 0000000..ab037a7
--- /dev/null
+++ b/folly/experimental/symbolizer/test/DwarfTests.cpp
@@ -0,0 +1,98 @@
+/*
+ * Copyright 2014 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <folly/experimental/symbolizer/Dwarf.h>
+
+#include <gtest/gtest.h>
+
+using folly::symbolizer::Dwarf;
+
+void checkPath(
+    std::string expectedPath,
+    std::string expectedBaseDir,
+    std::string expectedSubDir,
+    std::string expectedFile,
+    std::string rawBaseDir,
+    std::string rawSubDir,
+    std::string rawFile) {
+  Dwarf::Path path(rawBaseDir, rawSubDir, rawFile);
+
+  CHECK_EQ(expectedBaseDir, path.baseDir())
+    << "Path(" << rawBaseDir << ", " << rawSubDir << ", " << rawFile << ")";
+  CHECK_EQ(expectedSubDir, path.subDir())
+    << "Path(" << rawBaseDir << ", " << rawSubDir << ", " << rawFile << ")";
+  CHECK_EQ(expectedFile, path.file())
+    << "Path(" << rawBaseDir << ", " << rawSubDir << ", " << rawFile << ")";
+
+  CHECK_EQ(expectedPath, path.toString());
+
+  // Check the the `toBuffer` function.
+  char buf[1024];
+  size_t len;
+  len = path.toBuffer(buf, 1024);
+  CHECK_EQ(expectedPath, std::string(buf, len));
+}
+
+TEST(Dwarf, Path) {
+  checkPath(
+    "hello.cpp",
+    "",
+    "",
+    "hello.cpp",
+    "",
+    "",
+    "hello.cpp");
+  checkPath(
+    "foo/hello.cpp",
+    "foo",
+    "",
+    "hello.cpp",
+    "foo",
+    "",
+    "hello.cpp");
+  checkPath(
+    "foo/hello.cpp",
+    "foo",
+    "",
+    "hello.cpp",
+    "",
+    "foo",
+    "hello.cpp");
+  checkPath(
+    "hello.cpp",
+    "",
+    "",
+    "hello.cpp",
+    "./////",
+    "./////",
+    "hello.cpp");
+  checkPath(
+    "/hello.cpp",
+    "/",
+    "",
+    "hello.cpp",
+    "/////",
+    "./////",
+    "hello.cpp");
+  checkPath(
+    "/hello.cpp",
+    "/",
+    "",
+    "hello.cpp",
+    "/./././././././",
+    "",
+    "hello.cpp");
+}