Skip to content

Commit

Permalink
Patch synth methods overriding annotated methods
Browse files Browse the repository at this point in the history
Summary:
This issue was found while handling Kotlin SAMs https://kotlinlang.org/docs/fun-interfaces.html#sam-conversions

Make this step a bit more generic and useful by patching all synthetic methods that override annotated methods

An example of the pattern generated by SAM (inside a lambda accessing a private field too): P1530177966
Example class: P1531602975
The overridden method: P1536221348

Reviewed By: thezhangwei

Differential Revision: D61681031

fbshipit-source-id: 65644f5073e366194b86412934b52c0bc323e45d
  • Loading branch information
itang00 authored and facebook-github-bot committed Aug 30, 2024
1 parent 7dd2803 commit 3915d3b
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 0 deletions.
55 changes: 55 additions & 0 deletions opt/typedef-anno-checker/TypedefAnnoCheckerPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,60 @@ void patch_return_anno_from_get(type_inference::TypeInference& inference,

} // namespace

// https://kotlinlang.org/docs/fun-interfaces.html#sam-conversions
// sam conversions appear in Kotlin and provide a more concise way to override
// methods. This method handles sam conversiona and all synthetic methods that
// override methods with return or parameter annotations
bool SynthAccessorPatcher::patch_synth_methods_overriding_annotated_methods(
DexMethod* m) {
DexClass* cls = type_class(m->get_class());
if (!klass::maybe_anonymous_class(cls)) {
return false;
}

auto callees = mog::get_overridden_methods(m_method_override_graph, m,
true /*include_interfaces*/);
for (auto callee : callees) {
auto return_anno =
type_inference::get_typedef_anno_from_member(callee, m_typedef_annos);

if (return_anno != boost::none) {
DexAnnotationSet anno_set = DexAnnotationSet();
anno_set.add_annotation(std::make_unique<DexAnnotation>(
DexType::make_type(return_anno.get()->get_name()), DAV_RUNTIME));
add_annotations(m, &anno_set);
}

if (callee->get_param_anno() == nullptr) {
continue;
}
for (auto const& param_anno : *callee->get_param_anno()) {
auto annotation = type_inference::get_typedef_annotation(
param_anno.second->get_annotations(), m_typedef_annos);
if (annotation == boost::none) {
continue;
}

DexAnnotationSet anno_set = DexAnnotationSet();
anno_set.add_annotation(std::make_unique<DexAnnotation>(
DexType::make_type(annotation.get()->get_name()), DAV_RUNTIME));
if (m->get_param_anno()) {
m->get_param_anno()
->at(param_anno.first)
->add_annotation(std::make_unique<DexAnnotation>(
DexType::make_type(annotation.get()->get_name()), DAV_RUNTIME));
} else {
DexAccessFlags access = m->get_access();
m->set_access(ACC_SYNTHETIC);
m->attach_param_annotation_set(
param_anno.first, std::make_unique<DexAnnotationSet>(anno_set));
m->set_access(access);
}
}
}
return false;
}

// check if the field has any typedef annotations. If it does, patch the method
// return if it's a getter or the parameter if it's a setter
void SynthAccessorPatcher::try_adding_annotation_to_accessor(
Expand Down Expand Up @@ -356,6 +410,7 @@ void SynthAccessorPatcher::run(const Scope& scope) {
if (is_synthetic_kotlin_annotations_method(m)) {
patch_kotlin_annotations(m);
}
patch_synth_methods_overriding_annotated_methods(m);
if (is_constructor(m)) {
if (m->get_param_anno()) {
patch_synth_cls_fields_from_ctor_param(m);
Expand Down
2 changes: 2 additions & 0 deletions opt/typedef-anno-checker/TypedefAnnoCheckerPass.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ class SynthAccessorPatcher {
void patch_kotlin_companion_property_accessor(DexMethod* m);
void patch_kotlin_property_private_getter(DexMethod* m);

bool patch_synth_methods_overriding_annotated_methods(DexMethod* m);

void collect_accessors(DexMethod* method);

void patch_kotlin_annotations(DexMethod* method);
Expand Down
28 changes: 28 additions & 0 deletions test/integ/TypedefAnnoCheckerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1560,3 +1560,31 @@ TEST_F(TypedefAnnoCheckerTest, TestClassPrivatePropertySetter) {
auto checker = run_checker(scope, method, *method_override_graph);
EXPECT_TRUE(checker.complete());
}

TEST_F(TypedefAnnoCheckerTest, TestSAM) {
auto scope = build_class_scope(stores);
build_cfg(scope);
auto* method = DexMethod::get_method(
"Lcom/facebook/redextest/"
"TypedefAnnoCheckerKtTest$sam_interface$1;."
"setString:(Ljava/lang/String;)V")
->as_def();

// set the deobfuscated name manually since it doesn't get set by default in
// integ tests
method->set_deobfuscated_name(
"Lcom/facebook/redextest/"
"TypedefAnnoCheckerKtTest$sam_interface$1;."
"setString:(Ljava/lang/String;)V");
std::cerr << SHOW(method->get_code()->cfg()) << "\n";

auto method_override_graph = mog::build_graph(scope);

DexClass* synth_class = type_class(method->get_class());
synth_class->set_deobfuscated_name(synth_class->get_name()->c_str());

run_patcher(scope, *method_override_graph);

auto checker = run_checker(scope, method, *method_override_graph);
EXPECT_TRUE(checker.complete());
}
5 changes: 5 additions & 0 deletions test/integ/TypedefAnnoCheckerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ package com.facebook.redextest
import integ.TestIntDef
import integ.TestStringDef

fun interface SAMInterface {
fun setString(@TestStringDef sam_string: String)
}

public class ClassWithParams(@TestIntDef var int_field: Int) {}

public class ClassWithDefaultParams(@TestIntDef var int_field: Int = TestIntDef.THREE) {}
Expand All @@ -20,6 +24,7 @@ public class TypedefAnnoCheckerKtTest {
@TestStringDef var var_field: String = TestStringDef.THREE
@NotSafeAnno val field_not_safe: String = "4"
@TestIntDef var field_int: Int = TestIntDef.ONE
private val sam_interface = SAMInterface { sam_string -> var_field = sam_string }

companion object {
@TestStringDef val companion_val: String = TestStringDef.ONE
Expand Down

0 comments on commit 3915d3b

Please sign in to comment.