Java: `InstanceInitializer` body control flow is wrong

[Marcono1234]

Version

CodeQL CLI 2.5.1

Description

It appears the body of InstanceInitializer is incorrect (and therefore its control flow is wrong). Instead of having the same order as the Java bytecode it always orders field initializations to the beginning, followed by the explicit initializer blocks.
StaticInitializer does not seem to be affected.

Example

Create a database from the following Java source file:

class InitOrder {
    static String printStr(String s) {
        System.out.println(s);
        return s;
    }
    
    static String s1 = printStr("static field 1");
    static {
        System.out.println("static block 1");
    }
    
    static String s2 = printStr("static field 2");
    static {
        System.out.println("static block 2");
    }
    
    String i1 = printStr("instance field 1");
    {
        System.out.println("instance block 1");
    }
    
    String i2 = printStr("instance field 2");
    {
        System.out.println("instance block 2");
    }
    
    public InitOrder() {
        System.out.println("Constructor");
    }
    
    public static void main(String... args) {
        new InitOrder();
    }
}

Run the following CodeQL query (thanks intrigus-lgtm for pointing me to the path explanations):

/**
 * @kind path-problem
 */

import java

query predicate edges(ControlFlowNode a, ControlFlowNode b) { a.getASuccessor() = b }

from ControlFlowNode start, ControlFlowNode end
where
  not exists(start.getAPredecessor())
  and not exists(end.getASuccessor())
  and edges+(start, end)
select end, start, end, "Control flow path"

You will notice that it shows that "instance field 1" and "instance field 2" will be processed first before the explicit initializer blocks.
Compare that with the javap -v output of the .class file, or the result of running java InitOrder.java:

instance field 1
instance block 1
instance field 2
instance block 2

[hmakholm]

Thanks for the bug report. @github/codeql-java, can you look into this, please?

[aschackmull]

Interesting. I was not aware that multiple initializer blocks were allowed. I can't imagine that it's very common - at least I can't recall having seen this.
But indeed, we want to get control-flow like this right, so this is something we should fix (although probably not with a very high priority).