DynamoRIO / DynamoRIO/dynamorio

Convert reader_t and file_reader_t into templates

Open
#5,727 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
3.2k
Forks
629
Avg merge
2d 15h
Merged PRs (30d)
31

Description

Convert reader_t and file_reader_t into templates to support serial iteration over trace_entry_t.
`reader_tmpl_t` and `file_reader_tmpl_t` where `T` is one of {`gzip_reader_t`,
`zipfile_reader_t`, `snappy_reader_t`, `std::ifstream*`}, and `U` is one of {`memref_t`,
`trace_entry_t`}. Then, typedef the `trace_entry_t` specializations as `record_reader_t` and
`record_file_reader_t` respectively to replace the other definitions added by #5720.
This will allow significant code reuse, particularly for serializing multiple thread traces into a single stream.

Since the current `file_reader_t` is already a template on `T`, adding the second template
parameter `U` is complex. Note that we cannot have partial specialization of
member functions in C++. This complicates implementation of various
`file_reader_tmpl` specializations for `T`, as we would need to duplicate
the implementation for each candidate of `U`.

We have two options:
1. For each member function specialized for some `T`, duplicate the definition for
`file_reader_tmpl` and `file_reader_tmpl`. This has
the obvious disadvantage of code duplication, which can be mitigated to some
extent by extracting common logic in static routines.
2. For each specialization of `T`, create a subclass that is template `U` inheriting
from `file_reader_tmpl<_, U>`. E.g. for `T = gzip_reader_t`, create
`class gzip_file_reader_t: public file_reader_tmpl`
This has the disadvantage of breaking backward-compatibility of the existing
reader interface. Users that define their own readers outside DR will need to
adapt to this change. The advantage of this approach is that it is somewhat
cleaner to have proper classes instead of template specializations for file
readers.

We prefer Option 2, since it has higher merit.

Currently we do not have any use-case that needs this design, but when we need to
support serial iteration over `trace_entry_t`, we would want to do this to reuse the
existing multiple trace serialization code in `file_reader_t`.

On a related note: `file_reader_t` hides
some `trace_entry_t` entries today (like `TRACE_TYPE_THREAD`, `TRACE_TYPE_PID`, etc); we
would also need to avoid doing that since `record_reader_t` is expected to provide
the exact stream of `trace_entry_t` as stored on disk.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.