Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kamon emits invalid bytecode when instrumenting sqlite versions > 3.32.3.2 #1008

Closed
Falmarri opened this issue Apr 26, 2021 · 8 comments
Closed

Comments

@Falmarri
Copy link
Contributor

java.lang.VerifyError: Expecting a stackmap frame at branch target 20
Exception Details:
  Location:
    org/sqlite/jdbc3/JDBC3Statement.executeBatch()[I @17: goto
  Reason:
    Expected stackmap frame at this location.
  Bytecode:
    0000000: b201 6c2a b601 6fa7 000e 1301 4d5f b801
    0000010: 53a7 0003 014c 2a4d 2cb6 0002 2cb4 0024
    0000020: c600 0a2c b400 259a 0009 03bc 0aa7 00ac
    0000030: 2cb4 0025 bc0a 4e2c b400 04b6 0005 3a04
    0000040: 1904 593a 05c2 0336 0615 062d bea2 006a
    0000050: 2c2c b400 2415 0632 c000 71b5 0007 1904
    0000060: 2cb6 0008 2d15 0619 042c 01b6 002a 4f19
    0000070: 042c b600 2b57 a700 3b3a 07bb 002c 59bb
    0000080: 002d 59b7 002e 122f b600 3015 06b6 0031
    0000090: 1232 b600 3019 07b6 0033 b600 30b6 0034
    00000a0: 2db7 0035 bf3a 0819 042c b600 2b57 1908
    00000b0: bf84 0601 a7ff 952c b600 36a7 000c 3a09
    00000c0: 2cb6 0036 1909 bf19 05c3 a700 0b3a 0a19
    00000d0: 05c3 190a bf2d a700 034d 014e a700 064e
    00000e0: 014d b201 6c2b 2db6 0170 a700 0d13 014d
    00000f0: 5fb8 0153 a700 032d c600 052d bf2c b0  
  Exception Handler Table:
    bci [80, 111] => handler: 121
    bci [80, 111] => handler: 165
    bci [121, 167] => handler: 165
    bci [70, 183] => handler: 190
    bci [190, 192] => handler: 190
    bci [70, 202] => handler: 205
    bci [205, 210] => handler: 205
    bci [0, 10] => handler: 10
    bci [24, 217] => handler: 223
    bci [226, 237] => handler: 237
  Stackmap Table:
    same_locals_1_stack_item_frame(@10,Object[#114])
    same_locals_1_stack_item_frame(@21,Object[#341])
    append_frame(@22,Object[#341])
    append_frame(@24,Object[#112])
    same_frame(@42)
    same_frame(@48)
    full_frame(@73,{Object[#112],Object[#341],Object[#112],Object[#131],Object[#193],Object[#132],Integer},{})
    same_locals_1_stack_item_frame(@121,Object[#133])
    same_locals_1_stack_item_frame(@165,Object[#114])
    same_frame(@177)
    chop_frame(@183,1)
    same_locals_1_stack_item_frame(@190,Object[#114])
    same_frame(@199)
    same_locals_1_stack_item_frame(@205,Object[#114])
    chop_frame(@213,1)
    full_frame(@217,{Object[#112],Object[#341]},{Object[#131]})
    same_locals_1_stack_item_frame(@223,Object[#114])
    append_frame(@226,Object[#131],Object[#114])
    same_locals_1_stack_item_frame(@237,Object[#114])
    same_frame(@247)
    same_frame(@253)

    at org.sqlite.jdbc4.JDBC4Connection.createStatement(JDBC4Connection.java:28)
    at org.sqlite.jdbc3.JDBC3Connection.createStatement(JDBC3Connection.java:152)
    at org.sqlite.SQLiteConfig.apply(SQLiteConfig.java:189)
    at org.sqlite.SQLiteConnection.<init>(SQLiteConnection.java:65)
    at org.sqlite.jdbc3.JDBC3Connection.<init>(JDBC3Connection.java:28)
    at org.sqlite.jdbc4.JDBC4Connection.<init>(JDBC4Connection.java:21)
    at org.sqlite.JDBC.createConnection(JDBC.java:115)
    at org.sqlite.JDBC.connect(JDBC.java:90)
    at java.sql/java.sql.DriverManager.getConnection(DriverManager.java:677)
    at java.sql/java.sql.DriverManager.getConnection(DriverManager.java:189)

See thread here https://gitter.im/kamon-io/Kamon?at=60823ed7a2ac0d38e7d277ef

Falmarri pushed a commit to Falmarri/Kamon that referenced this issue Apr 27, 2021
This is more of a temporary workaround than a fix. It disables
instrumentation on the non-prepared statement versions of queries
Falmarri pushed a commit to Falmarri/Kamon that referenced this issue Apr 27, 2021
This is more of a temporary workaround than a fix. It disables
instrumentation on the non-prepared statement versions of queries
Falmarri pushed a commit to Falmarri/Kamon that referenced this issue Apr 27, 2021
This is more of a temporary workaround than a fix. It disables
instrumentation on the non-prepared statement versions of queries
Falmarri pushed a commit to Falmarri/Kamon that referenced this issue Apr 30, 2021
This is more of a temporary workaround than a fix. It disables
instrumentation on the non-prepared statement versions of queries
@SimunKaracic
Copy link
Contributor

@dpsoft can you offer some assistance here?
I've checked out the code and don't see any differences that could cause this issue, and Topo said that you might be able to help with these sorts of weird issues

@dpsoft
Copy link
Contributor

dpsoft commented May 13, 2021

@SimunKaracic @Falmarri Hi guys!, any running example?

@SimunKaracic
Copy link
Contributor

Are our tests enough?
You should just bump sqlite-jdbc to 3.32.3.3 and the tests should break in the same way.
The offending instrumentation is StatementInstrumentation.scala, specifically all the advice on org.sqlite.jdbc3.JDBC3Statement

@dpsoft
Copy link
Contributor

dpsoft commented May 13, 2021

I've just found a workaround, but i want to understand the underlying reason of the error.

@SimunKaracic
Copy link
Contributor

Hmmm, I'm trying to instrument a Redis client, and I'm getting the same error. Can you post the workaround while you're trying to get to the root cause?

@dpsoft
Copy link
Contributor

dpsoft commented May 17, 2021

Hey @SimunKaracic long story short:

change this

  @Advice.OnMethodEnter(suppress = classOf[Throwable])
  @Advice.OnMethodExit(onThrowable = classOf[Throwable], suppress = classOf[Throwable])

for

  @Advice.OnMethodEnter
  @Advice.OnMethodExit(onThrowable = classOf[Throwable])

I'm investigating why the emitted code is invalid e.g.

with suppress

public boolean execute(String string) throws SQLException {
        boolean bl;
        boolean bl2;
        Option option;
        block8: {
            Option option2;
            try {
                option2 = StatementExecuteMethodAdvisor$.MODULE$.executeStart((Object)this, string);
            }
            catch (Throwable throwable) {
                LoggerHandler.error((String)"An error occurred while trying to apply an advisor", (Throwable)throwable);
                option2 = null;
            }
            option = option2;
            JDBC3Statement jDBC3Statement = this;
            String sql = string;
            try {
                this.internalClose();
                ExtendedCommand.SQLExtension ext = ExtendedCommand.parse((String)sql);
                if (ext != null) {
                    ext.execute(this.conn.getDatabase());
                    bl2 = false;
                    break block8;
                }
                this.sql = sql;
                this.conn.getDatabase().prepare((CoreStatement)this);
                bl2 = this.exec();
            }
            catch (Throwable throwable) {
                bl = false;
            }
        }
        bl = bl2;
        Object var4_5 = null;
        try {
            StatementExecuteMethodAdvisor$.MODULE$.executeEnd(option, (Throwable)var4_5);
        }
        catch (Throwable throwable) {
            LoggerHandler.error((String)"An error occurred while trying to apply an advisor", (Throwable)throwable);
        }
        if (var4_5 != null) {
            throw var4_5;
        }
        return bl;
    }

whithout

 public boolean execute(String string) throws SQLException {
        boolean bl;
        boolean bl2;
        Option option;
        block4: {
            option = StatementExecuteMethodAdvisor$.MODULE$.executeStart((Object)this, string);
            JDBC3Statement jDBC3Statement = this;
            String sql = string;
            try {
                this.internalClose();
                ExtendedCommand.SQLExtension ext = ExtendedCommand.parse((String)sql);
                if (ext != null) {
                    ext.execute(this.conn.getDatabase());
                    bl2 = false;
                    break block4;
                }
                this.sql = sql;
                this.conn.getDatabase().prepare((CoreStatement)this);
                bl2 = this.exec();
            }
            catch (Throwable throwable) {
                bl = false;
            }
        }
        bl = bl2;
        Object var4_5 = null;
        StatementExecuteMethodAdvisor$.MODULE$.executeEnd(option, (Throwable)var4_5);
        if (var4_5 != null) {
            throw var4_5;
        }
        return bl;
    }

Reviewing all this I think that also we need a mechanism in kanela to configure the ExeptionHandler between test and runtime(production) where our advices should never throw exceptions, but that can be difficult to detect errors when they are supressed. Also another aspect to this is how much bytecode is added to the target method by default, I think that some advices simply cannot fail, and it shouldn’t pay for the weight of error handling.

I will continue investigating a little and try with the latest version of kanela(last bytebuddy version) in order to found something...

@SimunKaracic
Copy link
Contributor

Great, the workaround worked 🧙
Thanks @dpsoft !

@SimunKaracic
Copy link
Contributor

Fixed in 2.1.19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants