When instrumenting, Soot eliminates the synchronizing semantic if a method to be inserted is declared synchronized

[RichardHoOoOo]

Suppose I have declared a class A and it contains a synchronized method increase that updates the static count field of A.

Class A {
  private static int count = 0;
  public static synchronized void increase() {
       count++;
  }
}

Let's say I instrument a program by inserting increase after a unit.

SootClass A = Scene.v().loadClassAndSupport("A");
A.setApplicationClass();
SootMethod increase = A.getMethodByName("increase");
// Suppose units is the unit list of a body and u is a unit in units
units.insertAfter(Jimple.v().newStaticInvokeExpr(increase.makeRef()), u);

Once the program has been successfully instrumented, I observe that increase is actually not synchronized, and it can be proved if you look at the Jimple code of it.

If I change the increase method by explicitly synchronizing A.class, everything works.

public static synchronized void increase() {
   synchronize(A.class) {
      count++;
   } 
}

So it seems soot will eliminate the synchronizing semantic if a method to be inserted is declared synchronized.

[mbenz89]

Interesting! Thanks for reporting this issue!

Could you also post the Jimple code before and after instrumenting? We would be very happy to accept a pr fixing the issue if that's a possibility for you.

[RichardHoOoOo]

Hi, thanks for your reply! These are the 2 Jimple code snippets for the 2 versions of increase

public static synchronized void increase() {
      long $l0, $l1;
      $l0 = <A: int count>;
      $l1 = $l0 + 1;
      <A: int count> = $l1;
      return;
}

public static synchronized void increase() {
        long $l0, $l1;
        java.lang.Throwable $r1;
        entermonitor class "A;";
     label1:
        $l0 = <A: int count>;
        $l1 = $l0 + 1;
        <A: int count> = $l1;
        exitmonitor class "A;";
     label2:
        goto label5;
     label3:
        $r1 := @caughtexception;
        exitmonitor class "A;";
     label4:
        throw $r1;
     label5:
        return;
        catch java.lang.Throwable from label1 to label2 with label3;
        catch java.lang.Throwable from label3 to label4 with label3;
}

[RichardHoOoOo]

BTW, I found this issue when I was instrumenting an Android app. I'm not sure whether this will occur on other types of Java programs.

[mbenz89]

Thanks for the Jimple code! Is it correct that the first Jimple snippet ist the increase() method after instrumentation and the second before the instrumentation?

Any ideas what causes this strange behavior?

[RichardHoOoOo]

Hi I think you misunderstand what I mean. The increase() method is not the one that I want to instrument. It is the one that is injected into the code that I want to instrument. For example, the increase() method can be injected after every unit to measure statement coverage.

So the first Jimple snippet is the increase() method that is declared as synchronized, the second Jimple snippet is the increase() method that is explicitly synchronized by a synchronization block. What I show here is what they look like after being injected into the code that I want to instrument. Obviously the first one is not thread safe.

[mbenz89]

How does the increase() method look before you instrument a call to it? Probably the instrumentation does not change anything here. I guess the issue stems from the synchronized keyword on the method level. I would expect that usually, this is translated to bytecode and Soot should catch it up this way. The code you show, however, does not support this assumption.

I think it would be good to see the byte-/dexcode of the method and check how the keyword is implemented there first. I would really be surprised if this is not just syntactic sugar which is explicit in the bytecode. Maybe Android handles things differently and we have to implement special handling here.

[RichardHoOoOo]

Hi I think what increase() looks like doesn't matter because itself is not instrumented. It's just something that I inject into the code that is instrumented. So it should looks the same way "before" and "after" instrumenting.

One additional thing I want to note is that if a method that is declared synchronized is in the code that is going to be instrumented, it's Jimple code is correct. For example, if the synchronized increase() is instrumented, I can see the monitor block in the Jimple code.