redex icon indicating copy to clipboard operation
redex copied to clipboard

ControlFlow.order() cause a crash on android 5.x

Open fengruisd opened this issue 3 years ago • 6 comments

Run redex on apk, it will crash on android 5.x.

the logcat like this

I/art     (31281): instance field access on object that has non-reference type Conflictoolean): [0x60]
I/art     (31281): void com.xxxx.tux.status.refresh.XxxRefreshHeader.a(int, int, boolean) failed to verify: unexpected value in v8 of type Conflict but expected Integer for putoolean): [0x60]
E/art     (31281): Verification failed on class com.xxxx.tux.status.refresh.XxxRefreshHeader

At the beginning, I thought some pass might cause the crash, but when I remove all passes in the configuration and run redex again, it still crash. Here is the code_item before and after redex handle it. code_item_before.txt code_item_after.txt

Finally I found that ControlFlow.order() cause the problem, order() will change the order of blocks, if I modify ControlFlowGraph::linearize() as follows, it will not crash. But I am not sure if the modification will cause other issue or not, and I am not understand clearly what order() does. Maybe there is a bug on android 5.x and redex hit it.

IRList* ControlFlowGraph::linearize() {
  always_assert(m_editable);
  sanity_check();
  IRList* result = new IRList;

  TRACE_NO_LINE(CFG, 5, "before linearize:\n%s", SHOW(*this));

  const std::vector<Block*>& ordering = blocks(); // change order() into blocks() and it will be fine
  insert_branches_and_targets(ordering);
  insert_try_catch_markers(ordering);

  for (Block* b : ordering) {
    result->splice(result->end(), b->m_entries);
  }
  remove_duplicate_positions(result);

  return result;
}

fengruisd avatar May 14 '21 13:05 fengruisd

order() builds chains of blocks that need to be kept together. For example, the blocks ending with a throwing "primary" instruction with their corresponding move-result opcodes. However blocks() outputs an order that is internal to the CFG. The order becomes more arbitrary the more modification you make to the CFG. That being said, replacing order() with blocks() is going to be problematic. It may not put move-results and their "primary" instructions together.

yuxuanchen1997 avatar May 14 '21 15:05 yuxuanchen1997

but order() would cause crash on android 5.x, do you have any suggestion about how to resolve it?

fengruisd avatar May 15 '21 02:05 fengruisd

At the beginning, I thought some pass might cause the crash, but when I remove all passes in the configuration and run redex again, it still crash.

Just to confirm, are you using the lower_with_cfg option? I don't think otherwise the cfg is being used when there's no pass at all. Anyway, we can't avoid using CFG, so finding the issue would still be necessary.

yuxuanchen1997 avatar May 17 '21 15:05 yuxuanchen1997

@fengruisd, if you don't mind us looking at it, can you send the APK and the config you are working on to us? My email is listed on my profile.

yuxuanchen1997 avatar May 17 '21 16:05 yuxuanchen1997

@fengruisd, if you don't mind us looking at it, can you send the APK and the config you are working on to us? My email is listed on my profile.

@yuxuanchen1997 , Due to company reasons, i can't send the apk, sorry for that. I create a demo to reproduce this issue, hope it would be helpful.

Here is the source code soruce-code.zip

apk generated by the source code, it can run normally on android 5.x app-release.apk.zip

then run redex on the apk

redex -c redex.config -P simple-configuration.txt -o app-release-redex.apk --sign xxxxx app-release.apk

here is redex.config and simple-configuration.txt config.zip

then, I got app-release-redex.apk, run it on android 5.x, it will be crash app-release-redex.apk.zip

here is the crash log

I art (12622): void com.crashtest.crash.CrashOn5x.testCrash(int, int, boolean) failed to verify: void com.crashtest.crash.CrashOn5x.testCrash(int, int, boolean) [0x74] instance field access on object that has non-reference type Conflict
E art (12622): Verification failed on class com.crashtest.crash.CrashOn5x in /data/app/com.crashtest-1/base.apk because Verifier rejected class com.crashtest.crash.CrashOn5x due to bad method void com.crashtest.crash.CrashOn5x.testCrash(int, int, boolean)
D AndroidRuntime(12622) Shutting down VM
E AndroidRuntime(12622): FATAL EXCEPTION main
E AndroidRuntime(12622): Process: com.crashtest, PID 12622
E AndroidRuntime(12622): java.lang.VerifyError Verifier rejected class com.crashtest.crash.CrashOn5x due to bad method void com.crashtest.crash.CrashOn5x.testCrash(int, int, boolean) (declaration of 'com.crashtest.crash.CrashOn5x' appears in /data/app/com.crashtest-1/base.apk)
E AndroidRuntime(12622): at com.crashtest.MainActivity.onCreate(MainActivity.java 15)
E AndroidRuntime(12622): at android.app.Activity.performCreate(Activity.java 6083)
E AndroidRuntime(12622): at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java 1115)
E AndroidRuntime(12622): at android.app.ActivityThread.performLaunchActivity(ActivityThread.java 2357)
E AndroidRuntime(12622): at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java 2466)
E AndroidRuntime(12622): at android.app.ActivityThread.access$900(ActivityThread.java 175)
E AndroidRuntime(12622): at android.app.ActivityThread$H.handleMessage(ActivityThread.java 1369)
E AndroidRuntime(12622): at android.os.Handler.dispatchMessage(Handler.java 102)
E AndroidRuntime(12622): at android.os.Looper.loop(Looper.java 135)
E AndroidRuntime(12622): at android.app.ActivityThread.main(ActivityThread.java 5418)
E AndroidRuntime(12622) at java.lang.reflect.Method.invoke(Native Method)
E AndroidRuntime(12622): at java.lang.reflect.Method.invoke(Method.java 372)
E AndroidRuntime(12622): at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java 1037)
E AndroidRuntime(12622): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java 832)

fengruisd avatar May 18 '21 10:05 fengruisd

Just to confirm, are you using the lower_with_cfg option? I don't think otherwise the cfg is being used when there's no pass at all. Anyway, we can't avoid using CFG, so finding the issue would still be necessary.

@yuxuanchen1997 , As i can see, the default vaule of lower_with_cfg is true and i didn't change it.

{
  bool lower_with_cfg = true;
  conf.get_json_config().get("lower_with_cfg", true, lower_with_cfg);
  Timer t("Instruction lowering");
  instruction_lowering_stats =
      instruction_lowering::run(stores, lower_with_cfg);
}

After I add the log into redex, I find that cfg is used is such functions even there's no pass.

void redex_frontend(...) {
  ...
  {
    Timer t("No Optimizations Rules");
    // this will change rstate of methods
    keep_rules::process_no_optimizations_rules(
        conf.get_no_optimizations_annos(), scope);
    monitor_count::mark_sketchy_methods_with_no_optimize(scope); // cfg is used
  }
  ...
}


void redex_backend(...) {
  ...
  instruction_lowering::run(stores, lower_with_cfg); // cfg is used
  ...
}

fengruisd avatar May 18 '21 10:05 fengruisd