Skip to content

Commit

Permalink
Add infrastructure to enable source block testing with a default value
Browse files Browse the repository at this point in the history 
Summary:
Adds some basic piping and infra to allow developers to set a default value for all source blocks. Can be used to test violations that arise irrespective of input profiling data.

I put the code in the `InsertHelper` class in hopes that if we later implement better violation testing (e.g. fuzzing) we could follow a similar paradigm to this diff. However, after looking at the code I can see an argument for creating a completely separate pass if we pursue fuzzing testing in the future. With that said, this diff's changes could be contained solely in `InsertSourceBlocks.cpp` file. I don't feel strongly either way.

Reviewed By: jimmycFB

Differential Revision: D61743203

fbshipit-source-id: 46fe6ad40d569b595ef4dd99f35e85dad57be117
  • Loading branch information
@facebook-github-bot
Koby Chan authored and facebook-github-bot committed Aug 27, 2024
1 parent c141fd4 commit 99c9444
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 43 deletions.
29 changes: 20 additions & 9 deletions libredex/SourceBlocks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ constexpr SourceBlock::Val kXVal = SourceBlock::Val::none();

static SourceBlockConsistencyCheck s_sbcc;

const SourceBlock::Val global_default_val = SourceBlock::Val(1, 1);

struct InsertHelper {
std::ostringstream oss;
const DexString* method;
Expand Down Expand Up @@ -161,12 +163,12 @@ struct InsertHelper {
return SourceBlock::Val{nested_val, appear100};
}

void start(Block* cur) {
void start(Block* cur, bool use_global_default_value) {
if (serialize) {
oss << "(" << id;
}

auto val = start_profile(cur);
auto val = start_profile(cur, use_global_default_value, false);

source_blocks::impl::BlockAccessor::push_source_block(
cur, std::make_unique<SourceBlock>(method, id, val));
Expand Down Expand Up @@ -209,7 +211,8 @@ struct InsertHelper {
oss << "(" << id << ")";
}

auto nested_val = start_profile(cur, /*empty_inner_tail=*/true);
auto nested_val = start_profile(cur, use_global_default_value,
/*empty_inner_tail=*/true);
it = source_blocks::impl::BlockAccessor::insert_source_block_after(
cur, insert_after,
std::make_unique<SourceBlock>(method, id, nested_val));
Expand All @@ -220,18 +223,21 @@ struct InsertHelper {
}

std::vector<SourceBlock::Val> start_profile(Block* cur,
bool use_global_default_value,
bool empty_inner_tail = false) {
std::vector<SourceBlock::Val> ret;
ret.reserve(parser_state.size());
for (auto& p_state : parser_state) {
ret.emplace_back(start_profile_one(cur, empty_inner_tail, p_state));
ret.emplace_back(start_profile_one(cur, empty_inner_tail, p_state,
use_global_default_value));
}
return ret;
}

SourceBlock::Val start_profile_one(Block* cur,
bool empty_inner_tail,
ProfileParserState& p_state) {
ProfileParserState& p_state,
bool use_global_default_value) {
if (p_state.had_profile_failure) {
return kFailVal;
}
Expand Down Expand Up @@ -259,7 +265,9 @@ struct InsertHelper {
if (empty_inner_tail) {
redex_assert(matcher_state.inner_tail.is_nil());
}
auto val = parse_val(*matcher_state.val_str_ptr);
auto val = (use_global_default_value)
? global_default_val
: parse_val(*matcher_state.val_str_ptr);
TRACE(MMINL,
5,
"Started block with val=%f/%f. Popping %s, pushing %s + %s",
Expand Down Expand Up @@ -413,11 +421,13 @@ SourceBlockConsistencyCheck& get_sbcc() { return s_sbcc; }

InsertResult insert_source_blocks(DexMethod* method,
ControlFlowGraph* cfg,
bool use_global_default_value,
const std::vector<ProfileData>& profiles,
bool serialize,
bool insert_after_excs) {
return insert_source_blocks(&method->get_deobfuscated_name(), cfg, profiles,
serialize, insert_after_excs);
return insert_source_blocks(&method->get_deobfuscated_name(), cfg,
use_global_default_value, profiles, serialize,
insert_after_excs);
}

static std::string get_serialized_idom_map(ControlFlowGraph* cfg) {
Expand Down Expand Up @@ -471,13 +481,14 @@ static std::string get_serialized_idom_map(ControlFlowGraph* cfg) {

InsertResult insert_source_blocks(const DexString* method,
ControlFlowGraph* cfg,
bool use_global_default_value,
const std::vector<ProfileData>& profiles,
bool serialize,
bool insert_after_excs) {
InsertHelper helper(method, profiles, serialize, insert_after_excs);

impl::visit_in_order(
cfg, [&](Block* cur) { helper.start(cur); },
cfg, [&](Block* cur) { helper.start(cur, use_global_default_value); },
[&](Block* cur, const Edge* e) { helper.edge(cur, e); },
[&](Block* cur) { helper.end(cur); });

Expand Down
2 changes: 2 additions & 0 deletions libredex/SourceBlocks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,14 @@ using ProfileData =

InsertResult insert_source_blocks(const DexString* method,
ControlFlowGraph* cfg,
bool use_global_default_value,
const std::vector<ProfileData>& profiles = {},
bool serialize = true,
bool insert_after_excs = false);

InsertResult insert_source_blocks(DexMethod* method,
ControlFlowGraph* cfg,
bool use_global_default_value,
const std::vector<ProfileData>& profiles = {},
bool serialize = true,
bool insert_after_excs = false);
Expand Down
35 changes: 22 additions & 13 deletions opt/insert-source-blocks/InsertSourceBlocks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ bool is_numeric(const std::string_view& s) {
[](auto c) { return '0' <= c && c <= '9'; });
}

SourceBlock::Val DEFAULT_VALUE = SourceBlock::Val(1, 1);

namespace hasher {

uint64_t stable_hash_value(const std::string& s) {
Expand Down Expand Up @@ -383,10 +385,13 @@ struct Injector {
ConfigFiles& conf;
std::vector<std::unique_ptr<ProfileFile>> profile_files;
std::vector<std::string> interactions;
bool use_default_value;
bool always_inject;

Injector(ConfigFiles& conf, bool always_inject)
: conf(conf), always_inject(always_inject) {
Injector(ConfigFiles& conf, bool always_inject, bool use_default_value)
: conf(conf),
use_default_value(use_default_value),
always_inject(always_inject) {
// Prefetch the method profiles. We may need them when block profiles
// are missing and it's easier to do it here than have to synchronize
// loading later. (It's probably also amortized with later passes.)
Expand Down Expand Up @@ -626,14 +631,6 @@ struct Injector {
hasher::hashed_name(hash_value, access_method_name);
}

auto profiles =
find_profiles(method, access_method_type, access_method_name,
access_method_hash_name);
if (!profiles.second && !always_inject) {
// Skip without profile.
return InsertResult(access_method ? 1 : 0, 1);
}

auto* sb_name = [&]() {
if (!access_method) {
return &method->get_deobfuscated_name();
Expand All @@ -647,8 +644,17 @@ struct Injector {
return DexString::make_string(new_name);
}();

auto res = source_blocks::insert_source_blocks(
sb_name, &cfg, profiles.first, serialize, exc_inject);
source_blocks::InsertResult res;
auto profiles =
find_profiles(method, access_method_type, access_method_name,
access_method_hash_name);
if (!profiles.second && !always_inject) {
// Skip without profile.
return InsertResult(access_method ? 1 : 0, 1);
}
res = source_blocks::insert_source_blocks(
sb_name, &cfg, use_default_value, profiles.first, serialize,
exc_inject);

smi.add({sb_name, std::move(res.serialized),
std::move(res.serialized_idom_map)});
Expand Down Expand Up @@ -854,6 +860,9 @@ void InsertSourceBlocksPass::bind_config() {
"Force serialization of the CFGs. Testing only.");
bind("insert_after_excs", m_insert_after_excs, m_insert_after_excs);
bind("profile_files", "", m_profile_files);
bind("default_value", m_use_default_value, m_use_default_value,
"Use a default value for the inserted source blocks. The default value "
"is defined in SourceBlocks.cpp");
}

void InsertSourceBlocksPass::run_pass(DexStoresVector& stores,
Expand All @@ -862,7 +871,7 @@ void InsertSourceBlocksPass::run_pass(DexStoresVector& stores,
bool is_instr_mode = mgr.get_redex_options().instrument_pass_enabled;
bool always_inject = m_always_inject || m_force_serialize || is_instr_mode;

Injector inj(conf, always_inject);
Injector inj(conf, always_inject, m_use_default_value);

inj.prepare_profile_files_and_interactions(m_profile_files);
inj.write_unresolved_methods(
Expand Down
1 change: 1 addition & 0 deletions opt/insert-source-blocks/InsertSourceBlocks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class InsertSourceBlocksPass : public Pass {
bool m_force_run{false};
bool m_insert_after_excs{true};
bool m_always_inject{true};
bool m_use_default_value{false};

friend class SourceBlocksTest;
friend class SourceBlocksDedupTest;
Expand Down
Loading

0 comments on commit 99c9444

Please sign in to comment.