Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make symbolize.cc thread safe even on shared fds #388

Merged
merged 1 commit into from
Nov 14, 2018
Merged

Make symbolize.cc thread safe even on shared fds #388

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 23 additions & 24 deletions src/symbolize.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,20 @@ _END_GOOGLE_NAMESPACE_

_START_GOOGLE_NAMESPACE_

// Read up to "count" bytes from file descriptor "fd" into the buffer
// starting at "buf" while handling short reads and EINTR. On
// success, return the number of bytes read. Otherwise, return -1.
static ssize_t ReadPersistent(const int fd, void *buf, const size_t count) {
// Read up to "count" bytes from "offset" in the file pointed by file
// descriptor "fd" into the buffer starting at "buf" while handling short reads
// and EINTR. On success, return the number of bytes read. Otherwise, return
// -1.
static ssize_t ReadFromOffset(const int fd, void *buf, const size_t count,
const off_t offset) {
SAFE_ASSERT(fd >= 0);
SAFE_ASSERT(count <= std::numeric_limits<ssize_t>::max());
char *buf0 = reinterpret_cast<char *>(buf);
ssize_t num_bytes = 0;
while (num_bytes < count) {
ssize_t len;
NO_INTR(len = read(fd, buf0 + num_bytes, count - num_bytes));
NO_INTR(len = pread(fd, buf0 + num_bytes, count - num_bytes,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it available on windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, but IIUC, this is not used on Windows as we have defined(OS_WINDOWS) || defined(OS_CYGWIN) branch below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, ok.

offset + num_bytes));
if (len < 0) { // There was an error other than EINTR.
return -1;
}
Expand All @@ -159,18 +162,6 @@ static ssize_t ReadPersistent(const int fd, void *buf, const size_t count) {
return num_bytes;
}

// Read up to "count" bytes from "offset" in the file pointed by file
// descriptor "fd" into the buffer starting at "buf". On success,
// return the number of bytes read. Otherwise, return -1.
static ssize_t ReadFromOffset(const int fd, void *buf,
const size_t count, const off_t offset) {
off_t off = lseek(fd, offset, SEEK_SET);
if (off == (off_t)-1) {
return -1;
}
return ReadPersistent(fd, buf, count);
}

// Try reading exactly "count" bytes from "offset" bytes in a file
// pointed by "fd" into the buffer starting at "buf" while handling
// short reads and EINTR. On success, return true. Otherwise, return
Expand Down Expand Up @@ -398,9 +389,14 @@ struct FileDescriptor {
// and snprintf().
class LineReader {
public:
explicit LineReader(int fd, char *buf, int buf_len) : fd_(fd),
buf_(buf), buf_len_(buf_len), bol_(buf), eol_(buf), eod_(buf) {
}
explicit LineReader(int fd, char *buf, int buf_len, off_t offset)
: fd_(fd),
buf_(buf),
buf_len_(buf_len),
offset_(offset),
bol_(buf),
eol_(buf),
eod_(buf) {}

// Read '\n'-terminated line from file. On success, modify "bol"
// and "eol", then return true. Otherwise, return false.
Expand All @@ -409,10 +405,11 @@ class LineReader {
// dropped. It's an intentional behavior to make the code simple.
bool ReadLine(const char **bol, const char **eol) {
if (BufferIsEmpty()) { // First time.
const ssize_t num_bytes = ReadPersistent(fd_, buf_, buf_len_);
const ssize_t num_bytes = ReadFromOffset(fd_, buf_, buf_len_, offset_);
if (num_bytes <= 0) { // EOF or error.
return false;
}
offset_ += num_bytes;
eod_ = buf_ + num_bytes;
bol_ = buf_;
} else {
Expand All @@ -425,11 +422,12 @@ class LineReader {
// Read text from file and append it.
char * const append_pos = buf_ + incomplete_line_length;
const int capacity_left = buf_len_ - incomplete_line_length;
const ssize_t num_bytes = ReadPersistent(fd_, append_pos,
capacity_left);
const ssize_t num_bytes =
ReadFromOffset(fd_, append_pos, capacity_left, offset_);
if (num_bytes <= 0) { // EOF or error.
return false;
}
offset_ += num_bytes;
eod_ = append_pos + num_bytes;
bol_ = buf_;
}
Expand Down Expand Up @@ -474,6 +472,7 @@ class LineReader {
const int fd_;
char * const buf_;
const int buf_len_;
off_t offset_;
char *bol_;
char *eol_;
const char *eod_; // End of data in "buf_".
Expand Down Expand Up @@ -532,7 +531,7 @@ OpenObjectFileContainingPcAndGetStartAddress(uint64_t pc,
// look into the symbol tables inside.
char buf[1024]; // Big enough for line of sane /proc/self/maps
int num_maps = 0;
LineReader reader(wrapped_maps_fd.get(), buf, sizeof(buf));
LineReader reader(wrapped_maps_fd.get(), buf, sizeof(buf), 0);
while (true) {
num_maps++;
const char *cursor;
Expand Down