Skip to content

Commit

Permalink
Remove the dynamic_cast from OffsetReader constructor
Browse files Browse the repository at this point in the history
We used this previously to collapse chains of offset readers. Instead,
just implement "view" on OffsetReaeder so it happens there, and use it
consistently.
  • Loading branch information
peadar committed Sep 28, 2024
1 parent 2b55394 commit ca9701a
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 18 deletions.
2 changes: 1 addition & 1 deletion dwarf_macros.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ Macros::readD5(const Info &dwarf, intmax_t offset)
table.emplace_back(dr.getu8());
}
}
io = std::make_shared<OffsetReader>("macro subsection", macrosh.io(), dr.getOffset());
io = macrosh.io()->view("macro subsection", dr.getOffset());
}

bool
Expand Down
2 changes: 1 addition & 1 deletion libpstack/elf.h
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ class Notes::iterator {
}
iterator &operator++();
NoteDesc operator *() {
return NoteDesc(curNote, std::make_shared<const OffsetReader>("note content", io, offset));
return NoteDesc(curNote, io->view("note content", offset));
}
};

Expand Down
6 changes: 4 additions & 2 deletions libpstack/proc.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ struct AddressRange {
std::set<VmFlag> vmflags;
};

using AddressSpace = std::vector<AddressRange>;

// An ELF object mapped at an address. We don't actually create the Elf::Object
// until the first time you call "object" here. This avoids needless I/O, esp.
// on resource constrained systems.
Expand Down Expand Up @@ -256,7 +258,7 @@ class Process : public ps_prochandle {
Elf::Object::sptr vdsoImage;
protected:
std::string abiPrefix;
static std::vector<AddressRange> procAddressSpace(const std::string &fn); // utility to parse contents of /proc/pid/maps
static AddressSpace procAddressSpace(const std::string &fn); // utility to parse contents of /proc/pid/maps

public:
std::pair<Elf::Addr, Elf::Object::sptr> getElfObject(Elf::Addr addr);
Expand Down Expand Up @@ -302,7 +304,7 @@ class Process : public ps_prochandle {
virtual ~Process();
void load();
virtual pid_t getPID() const = 0;
virtual std::vector<AddressRange> addressSpace() const = 0;
virtual AddressSpace addressSpace() const = 0;
static std::shared_ptr<Process> load(Elf::Object::sptr exe, std::string id, const PstackOptions &options, Dwarf::ImageCache &cache);
};

Expand Down
6 changes: 6 additions & 0 deletions libpstack/reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ class OffsetReader final : public Reader {
}
Off size() const override;
std::string filename() const override { return upstream->filename(); }

// Implement "view" on the parent with our offset added.
Reader::csptr view( const std::string &name, Off offset_, Off size) const {
return upstream->view( name, offset + offset_, size );
}

};

Reader::csptr loadFile(const std::string &path);
Expand Down
2 changes: 1 addition & 1 deletion live.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ std::vector<AddressRange>
LiveProcess::addressSpace() const { return procAddressSpace(procname(pid, "smaps")); }

template < typename Separator >
std::string_view nextTok( std::string_view &total, Separator sep ) {
static std::string_view nextTok( std::string_view &total, Separator sep ) {
auto startPos = total.find_first_not_of(' '); // skip whitespace at the start.
size_t sepPos = total.find_first_of( sep, startPos );
std::string_view res;
Expand Down
17 changes: 4 additions & 13 deletions reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -243,24 +243,15 @@ MemReader::view(const std::string &name, Off offset, Off size) const {
return std::make_shared<MemOffsetReader>(name, this, offset, size);
}


OffsetReader::OffsetReader(std::string name_, Reader::csptr upstream_, Off offset_, Off length_)
: upstream(upstream_)
, offset(offset_)
, name(std::move(name_))
{
// If we create an offset reader with the upstream being another offset
// reader, we can just add the offsets, and use the
// upstream-of-the-upstream instead.
for (;;) {
auto orReader = dynamic_cast<const OffsetReader *>(upstream.get());
if (!orReader)
break;
if (verbose > 2)
*debug << "optimize: collapse offset reader : " << *upstream.get() << "->" << *orReader->upstream.get() << "\n";
offset += orReader->offset;
upstream = orReader->upstream;
}
// We used to have an explicit cast here to find the nearest non-offset
// reader, to collapse the stack. But now we use "view" rather than
// creating offset readers separately, that happens via dynamic dispatch
// when we create the view
length = length_ == std::numeric_limits<Off>::max() ? upstream->size() - offset : length_;
}

Expand Down

0 comments on commit ca9701a

Please sign in to comment.