Skip to content

Commit c40794c

Browse files
scc-twscc-tw
committed
refactor: Migrate loader editors to CRTP + std::variant dispatch
Port all three binary editors from virtual BinaryEditor base to CRTP EditorBase<Derived>: - ELFEditor : EditorBase<ELFEditor> - PEEditor : EditorBase<PEEditor> - MachOEditor : EditorBase<MachOEditor> Each editor's interface methods renamed to *_impl() suffix, called through CRTP forwarding in EditorBase. extend_text() kept as a public editor-specific method (RESERVED, not in CRTP interface). BinaryEditor.hpp: virtual base class removed, replaced with: using AnyEditor = std::variant<ELFEditor, PEEditor, MachOEditor>; Factory open_binary() returns AnyEditor variant. Loader.cpp: patch() uses std::visit on AnyEditor for all editor operations. Each concrete editor's methods are resolved at compile time (zero vtable overhead). StubEmitter still uses virtual dispatch (future migration). test_loader.cpp: MockEditor updated to use EditorBase<MockEditor> CRTP. 685 total tests passing, zero regressions.
1 parent b76fdd7 commit c40794c

11 files changed

Lines changed: 215 additions & 293 deletions

‎loader/include/BinaryEditor.hpp‎

Lines changed: 16 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -3,104 +3,34 @@
33
#pragma once
44

55
/// @file BinaryEditor.hpp
6-
/// @brief Abstract interface for format-specific binary mutation.
6+
/// @brief Variant-based type-erased editor and factory for binary mutation.
77
///
8-
/// Each platform (ELF, PE, Mach-O) implements this interface.
9-
/// The Loader orchestrator calls these methods without knowing the format.
8+
/// Each platform editor (ELFEditor, PEEditor, MachOEditor) inherits from
9+
/// EditorBase<Derived> (CRTP). At the factory boundary, runtime format
10+
/// selection is done via std::variant<ELFEditor, PEEditor, MachOEditor>.
1011
///
11-
/// Source: D13§D1-D3 (binary patching / loader).
12+
/// Source: D13 D1-D3 (binary patching / loader).
1213

1314
#include <LoaderTypes.hpp>
1415
#include <diagnostic_collector.hpp>
1516

17+
#include <ELFEditor.hpp>
18+
#include <MachOEditor.hpp>
19+
#include <PEEditor.hpp>
20+
1621
#include <tl/expected.hpp>
1722

18-
#include <cstdint>
19-
#include <memory>
20-
#include <string>
21-
#include <string_view>
22-
#include <vector>
23+
#include <variant>
2324

2425
namespace VMPilot::Loader {
2526

26-
class BinaryEditor {
27-
public:
28-
virtual ~BinaryEditor() = default;
29-
30-
// --- Query (read-only, no mutation) ---
31-
32-
/// Get .text section VA and size.
33-
[[nodiscard]] virtual TextSectionInfo text_section() const noexcept = 0;
34-
35-
/// Compute the VA where the next segment would be placed, without
36-
/// actually creating it. Enables build_payload() to resolve all
37-
/// displacements before injection.
38-
[[nodiscard]] virtual uint64_t
39-
next_segment_va(uint64_t alignment) const noexcept = 0;
40-
41-
/// Check if the binary enforces Control-Flow Integrity.
42-
/// ELF: .note.gnu.property with GNU_PROPERTY_X86_FEATURE_1_IBT
43-
/// or GNU_PROPERTY_AARCH64_FEATURE_1_BTI
44-
/// PE: IMAGE_DLLCHARACTERISTICS_CET_COMPAT in DllCharacteristics
45-
/// MachO: Always false for now (Apple controls BTI via code signing)
46-
/// Loader uses this to emit diagnostics confirming stubs have landing pads.
47-
[[nodiscard]] virtual bool cfi_enforced() const noexcept = 0;
48-
49-
/// Find usable gaps (NOP sleds, INT3/BRK padding, alignment fill)
50-
/// in .text that are at least `min_size` bytes.
51-
/// Returned sorted by size descending (largest first).
52-
///
53-
/// @note RESERVED — not wired to patch(). Current implementations have
54-
/// known limitations (false positives on embedded data, no cross-
55-
/// reference checks). See GitHub issue #9 for the rewrite plan.
56-
[[nodiscard]] virtual std::vector<TextGap>
57-
find_text_gaps(std::size_t min_size) const noexcept = 0;
58-
59-
// --- Mutate ---
60-
61-
/// Create a new loadable segment/section with the given data.
62-
/// This is the ONLY injection method currently used by patch().
63-
[[nodiscard]] virtual tl::expected<NewSegmentInfo, Common::DiagnosticCode>
64-
add_segment(std::string_view name, const std::vector<uint8_t>& data,
65-
uint64_t alignment,
66-
Common::DiagnosticCollector& diag) noexcept = 0;
67-
68-
/// Extend the .text section/segment to accommodate additional data.
69-
/// Returns the VA where data was placed (at the end of .text, aligned).
70-
///
71-
/// @warning RESERVED — not wired to patch(). All three platform
72-
/// implementations currently perform binary shifting, which
73-
/// corrupts metadata (linkedit offsets, PE headers, section VAs).
74-
/// Must be rewritten as slack-space-only before use.
75-
/// See GitHub issue #9.
76-
[[nodiscard]] virtual tl::expected<NewSegmentInfo, Common::DiagnosticCode>
77-
extend_text(const std::vector<uint8_t>& data,
78-
uint64_t alignment,
79-
Common::DiagnosticCollector& diag) noexcept = 0;
80-
81-
/// Overwrite bytes in .text at the given VA.
82-
[[nodiscard]] virtual tl::expected<void, Common::DiagnosticCode>
83-
overwrite_text(uint64_t va, const uint8_t* data, size_t len,
84-
Common::DiagnosticCollector& diag) noexcept = 0;
85-
86-
/// Add a runtime library dependency (LC_LOAD_DYLIB / DT_NEEDED / import).
87-
[[nodiscard]] virtual tl::expected<void, Common::DiagnosticCode>
88-
add_runtime_dep(std::string_view install_name,
89-
Common::DiagnosticCollector& diag) noexcept = 0;
90-
91-
/// Invalidate old code signature. Called before save().
92-
/// Mach-O: zero LC_CODE_SIGNATURE. PE: zero Certificate Table. ELF: no-op.
93-
virtual void invalidate_signature() noexcept = 0;
94-
95-
// --- Persist ---
96-
97-
[[nodiscard]] virtual tl::expected<void, Common::DiagnosticCode>
98-
save(const std::string& path,
99-
Common::DiagnosticCollector& diag) noexcept = 0;
100-
};
27+
/// Runtime-polymorphic editor handle. std::visit dispatches to the
28+
/// concrete CRTP editor without vtable indirection.
29+
using AnyEditor = std::variant<ELFEditor, PEEditor, MachOEditor>;
10130

102-
/// Factory. Opens a binary file and returns the appropriate editor.
103-
[[nodiscard]] tl::expected<std::unique_ptr<BinaryEditor>, Common::DiagnosticCode>
31+
/// Factory. Opens a binary file and returns the appropriate editor
32+
/// wrapped in the AnyEditor variant.
33+
[[nodiscard]] tl::expected<AnyEditor, Common::DiagnosticCode>
10434
open_binary(const std::string& path, Common::FileFormat format,
10535
Common::DiagnosticCollector& diag) noexcept;
10636

‎loader/include/ELFEditor.hpp‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,55 +2,58 @@
22
#define __LOADER_ELF_EDITOR_HPP__
33
#pragma once
44

5-
#include <BinaryEditor.hpp>
5+
#include <editor_base.hpp>
66

77
#include <memory>
88

99
namespace VMPilot::Loader {
1010

1111
/// ELF binary editor using ELFIO. Pimpl to keep elfio out of the header.
12-
class ELFEditor : public BinaryEditor {
12+
class ELFEditor : public EditorBase<ELFEditor> {
13+
friend class EditorBase<ELFEditor>;
14+
1315
public:
14-
~ELFEditor() override;
16+
~ELFEditor();
1517
ELFEditor(ELFEditor&&) noexcept;
1618
ELFEditor& operator=(ELFEditor&&) noexcept;
1719

1820
[[nodiscard]] static tl::expected<ELFEditor, Common::DiagnosticCode>
1921
open(const std::string& path,
2022
Common::DiagnosticCollector& diag) noexcept;
2123

22-
// --- BinaryEditor interface ---
23-
[[nodiscard]] TextSectionInfo text_section() const noexcept override;
24-
[[nodiscard]] uint64_t next_segment_va(uint64_t alignment) const noexcept override;
24+
// --- EditorBase _impl() interface ---
25+
26+
[[nodiscard]] TextSectionInfo text_section_impl() const noexcept;
27+
[[nodiscard]] uint64_t next_segment_va_impl(uint64_t alignment) const noexcept;
2528

2629
[[nodiscard]] tl::expected<NewSegmentInfo, Common::DiagnosticCode>
27-
add_segment(std::string_view name, const std::vector<uint8_t>& data,
28-
uint64_t alignment,
29-
Common::DiagnosticCollector& diag) noexcept override;
30+
add_segment_impl(std::string_view name, const std::vector<uint8_t>& data,
31+
uint64_t alignment,
32+
Common::DiagnosticCollector& diag) noexcept;
3033

31-
[[nodiscard]] bool cfi_enforced() const noexcept override;
34+
[[nodiscard]] bool cfi_enforced_impl() const noexcept;
3235

3336
[[nodiscard]] std::vector<TextGap>
34-
find_text_gaps(std::size_t min_size) const noexcept override;
37+
find_text_gaps_impl(std::size_t min_size) const noexcept;
3538

3639
[[nodiscard]] tl::expected<NewSegmentInfo, Common::DiagnosticCode>
3740
extend_text(const std::vector<uint8_t>& data,
38-
uint64_t alignment,
39-
Common::DiagnosticCollector& diag) noexcept override;
41+
uint64_t alignment,
42+
Common::DiagnosticCollector& diag) noexcept;
4043

4144
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
42-
overwrite_text(uint64_t va, const uint8_t* data, size_t len,
43-
Common::DiagnosticCollector& diag) noexcept override;
45+
overwrite_text_impl(uint64_t va, const uint8_t* data, size_t len,
46+
Common::DiagnosticCollector& diag) noexcept;
4447

4548
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
46-
add_runtime_dep(std::string_view install_name,
47-
Common::DiagnosticCollector& diag) noexcept override;
49+
add_runtime_dep_impl(std::string_view install_name,
50+
Common::DiagnosticCollector& diag) noexcept;
4851

49-
void invalidate_signature() noexcept override;
52+
void invalidate_signature_impl() noexcept;
5053

5154
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
52-
save(const std::string& path,
53-
Common::DiagnosticCollector& diag) noexcept override;
55+
save_impl(const std::string& path,
56+
Common::DiagnosticCollector& diag) noexcept;
5457

5558
private:
5659
ELFEditor();

‎loader/include/MachOEditor.hpp‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#define __LOADER_MACHO_EDITOR_HPP__
33
#pragma once
44

5-
#include <BinaryEditor.hpp>
5+
#include <editor_base.hpp>
66
#include <MachOStructs.hpp>
77

88
#include <cstdint>
@@ -12,45 +12,46 @@
1212

1313
namespace VMPilot::Loader {
1414

15-
/// Mach-O 64-bit binary editor. Implements BinaryEditor.
16-
class MachOEditor : public BinaryEditor {
15+
/// Mach-O 64-bit binary editor. Implements EditorBase via CRTP.
16+
class MachOEditor : public EditorBase<MachOEditor> {
17+
friend class EditorBase<MachOEditor>;
1718
public:
1819
[[nodiscard]] static tl::expected<MachOEditor, Common::DiagnosticCode>
1920
open(const std::string& path,
2021
Common::DiagnosticCollector& diag) noexcept;
2122

22-
// --- BinaryEditor interface ---
23-
[[nodiscard]] TextSectionInfo text_section() const noexcept override;
24-
[[nodiscard]] uint64_t next_segment_va(uint64_t alignment) const noexcept override;
23+
// --- EditorBase CRTP interface ---
24+
[[nodiscard]] TextSectionInfo text_section_impl() const noexcept;
25+
[[nodiscard]] uint64_t next_segment_va_impl(uint64_t alignment) const noexcept;
2526

2627
[[nodiscard]] tl::expected<NewSegmentInfo, Common::DiagnosticCode>
27-
add_segment(std::string_view name, const std::vector<uint8_t>& data,
28+
add_segment_impl(std::string_view name, const std::vector<uint8_t>& data,
2829
uint64_t alignment,
29-
Common::DiagnosticCollector& diag) noexcept override;
30+
Common::DiagnosticCollector& diag) noexcept;
3031

31-
[[nodiscard]] bool cfi_enforced() const noexcept override;
32+
[[nodiscard]] bool cfi_enforced_impl() const noexcept;
3233

3334
[[nodiscard]] std::vector<TextGap>
34-
find_text_gaps(std::size_t min_size) const noexcept override;
35+
find_text_gaps_impl(std::size_t min_size) const noexcept;
3536

3637
[[nodiscard]] tl::expected<NewSegmentInfo, Common::DiagnosticCode>
3738
extend_text(const std::vector<uint8_t>& data,
3839
uint64_t alignment,
39-
Common::DiagnosticCollector& diag) noexcept override;
40+
Common::DiagnosticCollector& diag) noexcept;
4041

4142
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
42-
overwrite_text(uint64_t va, const uint8_t* data, size_t len,
43-
Common::DiagnosticCollector& diag) noexcept override;
43+
overwrite_text_impl(uint64_t va, const uint8_t* data, size_t len,
44+
Common::DiagnosticCollector& diag) noexcept;
4445

4546
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
46-
add_runtime_dep(std::string_view install_name,
47-
Common::DiagnosticCollector& diag) noexcept override;
47+
add_runtime_dep_impl(std::string_view install_name,
48+
Common::DiagnosticCollector& diag) noexcept;
4849

49-
void invalidate_signature() noexcept override;
50+
void invalidate_signature_impl() noexcept;
5051

5152
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
52-
save(const std::string& path,
53-
Common::DiagnosticCollector& diag) noexcept override;
53+
save_impl(const std::string& path,
54+
Common::DiagnosticCollector& diag) noexcept;
5455

5556
private:
5657
MachOEditor() = default;

‎loader/include/PEEditor.hpp‎

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,60 +2,63 @@
22
#define __LOADER_PE_EDITOR_HPP__
33
#pragma once
44

5-
#include <BinaryEditor.hpp>
5+
#include <editor_base.hpp>
66

77
#include <memory>
88

99
namespace VMPilot::Loader {
1010

1111
/// PE binary editor using COFFI. Pimpl to keep coffi out of the header.
12-
class PEEditor : public BinaryEditor {
12+
class PEEditor : public EditorBase<PEEditor> {
13+
friend class EditorBase<PEEditor>;
14+
1315
public:
14-
~PEEditor() override;
16+
~PEEditor();
1517
PEEditor(PEEditor&&) noexcept;
1618
PEEditor& operator=(PEEditor&&) noexcept;
1719

1820
[[nodiscard]] static tl::expected<PEEditor, Common::DiagnosticCode>
1921
open(const std::string& path,
2022
Common::DiagnosticCollector& diag) noexcept;
2123

22-
// --- BinaryEditor interface ---
23-
[[nodiscard]] TextSectionInfo text_section() const noexcept override;
24-
[[nodiscard]] uint64_t next_segment_va(uint64_t alignment) const noexcept override;
24+
// --- PE-specific (not part of EditorBase) ---
2525

2626
[[nodiscard]] tl::expected<NewSegmentInfo, Common::DiagnosticCode>
27-
add_segment(std::string_view name, const std::vector<uint8_t>& data,
27+
extend_text(const std::vector<uint8_t>& data,
2828
uint64_t alignment,
29-
Common::DiagnosticCollector& diag) noexcept override;
29+
Common::DiagnosticCollector& diag) noexcept;
30+
31+
private:
32+
PEEditor();
33+
struct Impl;
34+
std::unique_ptr<Impl> impl_;
3035

31-
[[nodiscard]] bool cfi_enforced() const noexcept override;
36+
// --- EditorBase CRTP impl methods ---
37+
[[nodiscard]] TextSectionInfo text_section_impl() const noexcept;
38+
[[nodiscard]] uint64_t next_segment_va_impl(uint64_t alignment) const noexcept;
39+
[[nodiscard]] bool cfi_enforced_impl() const noexcept;
3240

3341
[[nodiscard]] std::vector<TextGap>
34-
find_text_gaps(std::size_t min_size) const noexcept override;
42+
find_text_gaps_impl(std::size_t min_size) const noexcept;
3543

3644
[[nodiscard]] tl::expected<NewSegmentInfo, Common::DiagnosticCode>
37-
extend_text(const std::vector<uint8_t>& data,
38-
uint64_t alignment,
39-
Common::DiagnosticCollector& diag) noexcept override;
45+
add_segment_impl(std::string_view name, const std::vector<uint8_t>& data,
46+
uint64_t alignment,
47+
Common::DiagnosticCollector& diag) noexcept;
4048

4149
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
42-
overwrite_text(uint64_t va, const uint8_t* data, size_t len,
43-
Common::DiagnosticCollector& diag) noexcept override;
50+
overwrite_text_impl(uint64_t va, const uint8_t* data, size_t len,
51+
Common::DiagnosticCollector& diag) noexcept;
4452

4553
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
46-
add_runtime_dep(std::string_view install_name,
47-
Common::DiagnosticCollector& diag) noexcept override;
54+
add_runtime_dep_impl(std::string_view install_name,
55+
Common::DiagnosticCollector& diag) noexcept;
4856

49-
void invalidate_signature() noexcept override;
57+
void invalidate_signature_impl() noexcept;
5058

5159
[[nodiscard]] tl::expected<void, Common::DiagnosticCode>
52-
save(const std::string& path,
53-
Common::DiagnosticCollector& diag) noexcept override;
54-
55-
private:
56-
PEEditor();
57-
struct Impl;
58-
std::unique_ptr<Impl> impl_;
60+
save_impl(const std::string& path,
61+
Common::DiagnosticCollector& diag) noexcept;
5962
};
6063

6164
} // namespace VMPilot::Loader

‎loader/include/editor_base.hpp‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -161,23 +161,6 @@ class EmitterBase {
161161
}
162162
};
163163

164-
// ─────────────────────────────────────────────────────────────────────────────
165-
// Forward declarations for variant types (concrete editors defined elsewhere)
166-
// ─────────────────────────────────────────────────────────────────────────────
167-
168-
// NOTE: These forward declarations are here so that AnyEditor/AnyEmitter
169-
// can be used as return types. The actual class definitions are in their
170-
// respective headers (elf_editor_v2.hpp, etc.). For now, during the
171-
// incremental migration, the existing ELFEditor/PEEditor/MachOEditor
172-
// still use the old virtual BinaryEditor base. When a concrete editor
173-
// is ported to CRTP, it will be added to AnyEditor.
174-
175-
// Future:
176-
// class ELFEditorV2;
177-
// class PEEditorV2;
178-
// class MachOEditorV2;
179-
// using AnyEditor = std::variant<ELFEditorV2, PEEditorV2, MachOEditorV2>;
180-
181164
} // namespace VMPilot::Loader
182165

183166
#endif // __LOADER_EDITOR_BASE_HPP__

0 commit comments

Comments
 (0)