From 3468cb59a4676d26ebbb93ed80018d7a9a9ff262 Mon Sep 17 00:00:00 2001 From: Andrew Gallagher Date: Fri, 19 Dec 2014 21:56:23 -0800 Subject: [PATCH] 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 --- folly/experimental/symbolizer/Dwarf.cpp | 64 ++++++++---- .../symbolizer/test/DwarfTests.cpp | 98 +++++++++++++++++++ 2 files changed, 143 insertions(+), 19 deletions(-) create mode 100644 folly/experimental/symbolizer/test/DwarfTests.cpp diff --git a/folly/experimental/symbolizer/Dwarf.cpp b/folly/experimental/symbolizer/Dwarf.cpp index 04473cfa..d2385aa5 100644 --- 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 index 00000000..ab037a7c --- /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 + +#include + +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"); +} -- 2.34.1