From eb7bc45f22e034751a76bf02445c669471e60780 Mon Sep 17 00:00:00 2001
From: Andrii Nakryiko <andriin@fb.com>
Date: Mon, 11 Dec 2017 22:05:33 -0800
Subject: [PATCH] Store filename and provide detailed message on data access
 assertion failure.

Summary:
It looks like not having offset/size/filename information is way more
harmful, than storing filename just for the sake of this error message.

Reviewed By: yfeldblum

Differential Revision: D6536616

fbshipit-source-id: 469fbdf1deedd76ebd79cf98716c2c269cb10e4d
---
 folly/experimental/symbolizer/Elf.cpp | 10 ++++++++++
 folly/experimental/symbolizer/Elf.h   | 18 +++++++++++++++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/folly/experimental/symbolizer/Elf.cpp b/folly/experimental/symbolizer/Elf.cpp
index 9cf3d137..2ff63863 100644
--- a/folly/experimental/symbolizer/Elf.cpp
+++ b/folly/experimental/symbolizer/Elf.cpp
@@ -20,6 +20,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include <cstring>
 #include <string>
 
 #include <glog/logging.h>
@@ -64,6 +65,7 @@ int ElfFile::openNoThrow(
     bool readOnly,
     const char** msg) noexcept {
   FOLLY_SAFE_CHECK(fd_ == -1, "File already open");
+  strncat(filepath_, name, kFilepathMaxLen - 1);
   fd_ = ::open(name, readOnly ? O_RDONLY : O_RDWR);
   if (fd_ == -1) {
     if (msg) {
@@ -157,6 +159,9 @@ ElfFile::ElfFile(ElfFile&& other) noexcept
       file_(other.file_),
       length_(other.length_),
       baseAddress_(other.baseAddress_) {
+  // copy other.filepath_, leaving filepath_ zero-terminated, always.
+  strncat(filepath_, other.filepath_, kFilepathMaxLen - 1);
+  other.filepath_[0] = 0;
   other.fd_ = -1;
   other.file_ = static_cast<char*>(MAP_FAILED);
   other.length_ = 0;
@@ -167,11 +172,14 @@ ElfFile& ElfFile::operator=(ElfFile&& other) {
   assert(this != &other);
   reset();
 
+  // copy other.filepath_, leaving filepath_ zero-terminated, always.
+  strncat(filepath_, other.filepath_, kFilepathMaxLen - 1);
   fd_ = other.fd_;
   file_ = other.file_;
   length_ = other.length_;
   baseAddress_ = other.baseAddress_;
 
+  other.filepath_[0] = 0;
   other.fd_ = -1;
   other.file_ = static_cast<char*>(MAP_FAILED);
   other.length_ = 0;
@@ -181,6 +189,8 @@ ElfFile& ElfFile::operator=(ElfFile&& other) {
 }
 
 void ElfFile::reset() {
+  filepath_[0] = 0;
+
   if (file_ != MAP_FAILED) {
     munmap(file_, length_);
     file_ = static_cast<char*>(MAP_FAILED);
diff --git a/folly/experimental/symbolizer/Elf.h b/folly/experimental/symbolizer/Elf.h
index a4f5afd2..a31ee581 100644
--- a/folly/experimental/symbolizer/Elf.h
+++ b/folly/experimental/symbolizer/Elf.h
@@ -241,9 +241,19 @@ class ElfFile {
   template <class T>
   const typename std::enable_if<std::is_pod<T>::value, T>::type& at(
       ElfOff offset) const {
-    FOLLY_SAFE_CHECK(
-        offset + sizeof(T) <= length_,
-        "Offset is not contained within our mmapped file");
+    if (offset + sizeof(T) > length_) {
+      char msg[kFilepathMaxLen + 128];
+      snprintf(
+          msg,
+          sizeof(msg),
+          "Offset (%lu + %lu) is not contained within our mmapped"
+          " file (%s) of length %zu",
+          offset,
+          sizeof(T),
+          filepath_,
+          length_);
+      FOLLY_SAFE_CHECK(offset + sizeof(T) <= length_, msg);
+    }
 
     return *reinterpret_cast<T*>(file_ + offset);
   }
@@ -270,6 +280,8 @@ class ElfFile {
     return at<T>(section.sh_offset + (addr - section.sh_addr));
   }
 
+  static constexpr size_t kFilepathMaxLen = 512;
+  char filepath_[kFilepathMaxLen] = {};
   int fd_;
   char* file_; // mmap() location
   size_t length_; // mmap() length
-- 
2.34.1