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

Jump to bottom

Bad bytecode generated by scalac allowing null values through "if (x != null)" blocks #10041

Closed
scabug opened this issue Nov 10, 2016 · 5 comments
Closed

Bad bytecode generated by scalac allowing null values through "if (x != null)" blocks #10041

scabug opened this issue Nov 10, 2016 · 5 comments
Assignees
@retronym
Labels
bytecode

Comments

@scabug
Copy link

scabug commented Nov 10, 2016 •
edited by retronym

I have some code like this:

     if (ccy != null && rate != null) {
        ltub.settlement(ccy, rate.bigDecimal)                   //non-deterministic strangeness
        if (ltub.getDescriptor.getSettlementFxRate.isSet && ltub.getDescriptor.getSettlementFxRate.getValue == null) {
          if (rate.bigDecimal == null) println("NULL NOOOO!!!") //NPE WAT?
          a += 1
          println(a)
        }
      }

Within this block we are seeing a NullPointerException thrown at runtime on the indicated line. This should obviously be impossible because that can only happen if rate is null and yet the code is inside an if-block protected against that condition. I have the following observations:

  • Failures are non-deterministic running from the same class files (sometimes the NPE is thrown, sometimes it isn't)
  • The block is being invoked hundreds of thousands of times. Investigations have revealed sometimes there are no failures, sometimes a few thousand and sometimes many hundreds of thousands
  • The code has been in existence and running against scala 2.11 with no issues for over 6 months
  • The code is single threaded - this is not a concurrency issue
  • I think the issue might be connected to JIT because of the above (i.e. sometimes the bytecode works, sometimes it doesn't). Having said that, successes happen after failures, so it is not as simple as "the code gets compiled by JIT and then starts failing". Having said that, I cannot reproduce the issue with -Djava.compiler=NONE and @oxntr (on Twitter) said this description "screams JIT"
  • The slightest change to the code (adding a different print statement in the block) can cause the issue to go away
  • In investigating this, I've observed other horrendous behaviour - for example, the line ltub.settlement(ccy, rate.bigDecimal) is supposed to set update descriptor fields for currency and rate; I have seen the currency field get set to the rate (the field is a generic class and hence the type it is holding is erased)

I have tried to put together a minimal example but have been unable to do so (i.e. I cannot reproduce the problem), hence I'm supplying the disassembled bytecode (using javap), assuming that people who can read bytecode might be able to find some obvious issue in what has been generated (although this may be tricky, if the issue is connected to how JIT is compiling that bytecode).

I have supplied some minimal source code; this depends on our projects and scalaz - you will not be able to compile or run it. I hope, however, that you will be able to see how the problematic lines (13-19 in the attached file) relate to the generated bytecode (for example, I'm pretty sure that line 14 in the scala file relates to line 185 in the bytecode).
@scabug
Copy link
Author

scabug commented Nov 10, 2016

Imported From: https://issues.scala-lang.org/browse/SI-10041?orig=1
Reporter: @oxbowlakes
Affected Versions: 2.12.0
See #9828
Attachments:

@scabug
Copy link
Author

scabug commented Nov 10, 2016

@adriaanm said:
Could you try with the latest build of jdk 8?

@scabug
Copy link
Author

scabug commented Nov 10, 2016

@retronym said:
Extract from the attachment:

  public static final scala.Tuple2 $anonfun$parse$38(scala.runtime.IntRef, long, gsa.shared.units.Currency, scala.math.BigDecimal, gsa.shared.accounting.AccountingDate, gsa.furnace.core.EventState);
    descriptor: (Lscala/runtime/IntRef;JLgsa/shared/units/Currency;Lscala/math/BigDecimal;Lgsa/shared/accounting/AccountingDate;Lgsa/furnace/core/EventState;)Lscala/Tuple2;
    flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL, ACC_SYNTHETIC
    Code:
      stack=7, locals=8, args_size=6
         0: lload_1
         1: lload_1
         2: invokestatic  #351                // Method gsa/furnace/client/ListedTransactionUpdateBuilder.from:(JJ)Lgsa/furnace/client/ListedTransactionUpdateBuilder;
         5: astore        7
         7: aload_3
         8: ifnull        105
        11: aload         4
        13: ifnull        105
        16: aload         7
        18: aload_3
        19: aload         4
        21: invokevirtual #357                // Method scala/math/BigDecimal.bigDecimal:()Ljava/math/BigDecimal;
        24: invokevirtual #361                // Method gsa/furnace/client/ListedTransactionUpdateBuilder.settlement:(Lgsa/shared/units/Currency;Ljava/math/BigDecimal;)Lgsa/furnace/client/ListedTransactionUpdateBuilder;
        27: pop
        28: aload         7
        30: invokevirtual #365                // Method gsa/furnace/client/ListedTransactionUpdateBuilder.getDescriptor:()Lgsa/furnace/common/UpdateListedTransactionDescriptor;
        33: invokevirtual #371                // Method gsa/furnace/common/UpdateListedTransactionDescriptor.getSettlementFxRate:()Lgsa/furnace/common/msg/OptionalField;
        36: invokevirtual #376                // Method gsa/furnace/common/msg/OptionalField.isSet:()Z
        39: ifeq          102
        42: aload         7
        44: invokevirtual #365                // Method gsa/furnace/client/ListedTransactionUpdateBuilder.getDescriptor:()Lgsa/furnace/common/UpdateListedTransactionDescriptor;
        47: invokevirtual #371                // Method gsa/furnace/common/UpdateListedTransactionDescriptor.getSettlementFxRate:()Lgsa/furnace/common/msg/OptionalField;
        50: invokevirtual #380                // Method gsa/furnace/common/msg/OptionalField.getValue:()Ljava/lang/Object;
        53: ifnonnull     102
        56: aload         4
        58: invokevirtual #357                // Method scala/math/BigDecimal.bigDecimal:()Ljava/math/BigDecimal;
        61: ifnonnull     76
        64: getstatic     #297                // Field scala/Predef$.MODULE$:Lscala/Predef$;
        67: ldc_w         #382                // String NULL NOOOO!!!
        70: invokevirtual #386                // Method scala/Predef$.println:(Ljava/lang/Object;)V
        73: goto          76
        76: aload_0
        77: aload_0
        78: getfield      #390                // Field scala/runtime/IntRef.elem:I
        81: iconst_1
        82: iadd
        83: putfield      #390                // Field scala/runtime/IntRef.elem:I
        86: getstatic     #297                // Field scala/Predef$.MODULE$:Lscala/Predef$;
        89: aload_0
        90: getfield      #390                // Field scala/runtime/IntRef.elem:I
        93: invokestatic  #396                // Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
        96: invokevirtual #386                // Method scala/Predef$.println:(Ljava/lang/Object;)V
        99: goto          102
       102: goto          105
       105: aload         5
       107: ifnull        120
       110: aload         7
       112: aload         5
       114: invokevirtual #400                // Method gsa/furnace/client/ListedTransactionUpdateBuilder.settlementDate:(Lgsa/shared/accounting/AccountingDate;)Lgsa/furnace/client/ListedTransactionUpdateBuilder;
       117: goto          123
       120: getstatic     #406                // Field scala/runtime/BoxedUnit.UNIT:Lscala/runtime/BoxedUnit;
       123: pop
       124: aload         6
       126: ifnull        139
       129: aload         7
       131: aload         6
       133: invokevirtual #409                // Method gsa/furnace/client/ListedTransactionUpdateBuilder.state:(Lgsa/furnace/core/EventState;)Lgsa/furnace/client/ListedTransactionUpdateBuilder;
       136: goto          142
       139: getstatic     #406                // Field scala/runtime/BoxedUnit.UNIT:Lscala/runtime/BoxedUnit;
       142: pop
       143: getstatic     #412                // Field scala/Predef$ArrowAssoc$.MODULE$:Lscala/Predef$ArrowAssoc$;
       146: getstatic     #297                // Field scala/Predef$.MODULE$:Lscala/Predef$;
       149: new           #13                 // class gsa/furnace/scala/package$OrigEvId
       152: dup
       153: getstatic     #415                // Field gsa/furnace/scala/package$TxnId$.MODULE$:Lgsa/furnace/scala/package$TxnId$;
       156: lload_1
       157: invokevirtual #418                // Method gsa/furnace/scala/package$TxnId$.apply:(J)J
       160: invokespecial #421                // Method gsa/furnace/scala/package$OrigEvId."<init>":(J)V
       163: invokevirtual #424                // Method scala/Predef$.ArrowAssoc:(Ljava/lang/Object;)Ljava/lang/Object;
       166: aload         7
       168: invokevirtual #428                // Method scala/Predef$ArrowAssoc$.$minus$greater$extension:(Ljava/lang/Object;Ljava/lang/Object;)Lscala/Tuple2;
       171: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            5     166     7  ltub   Lgsa/furnace/client/ListedTransactionUpdateBuilder;
            0     172     0   a$1   Lscala/runtime/IntRef;
            0     172     1 txnId$2   J
            0     172     3 ccy$1   Lgsa/shared/units/Currency;
            0     172     4 rate$1   Lscala/math/BigDecimal;
            0     172     5 settDate$2   Lgsa/shared/accounting/AccountingDate;
            0     172     6 state   Lgsa/furnace/core/EventState;
      LineNumberTable:
        line 214: 0
        line 215: 7
        line 216: 16
        line 217: 28
        line 218: 56
        line 219: 76
        line 220: 86
        line 217: 102
        line 223: 105
        line 224: 124
        line 226: 146
      StackMapTable: number_of_entries = 7
        frame_type = 252 /* append */
          offset_delta = 76
          locals = [ class gsa/furnace/client/ListedTransactionUpdateBuilder ]
        frame_type = 25 /* same */
        frame_type = 2 /* same */
        frame_type = 14 /* same */
        frame_type = 66 /* same_locals_1_stack_item */
          stack = [ class java/lang/Object ]
        frame_type = 15 /* same */
        frame_type = 66 /* same_locals_1_stack_item */
          stack = [ class java/lang/Object ]
    MethodParameters:
      Name                           Flags
      a$1                            final
      txnId$2                        final
      ccy$1                          final
      rate$1                         final
      settDate$2                     final
      state                          final

@scabug
Copy link
Author

scabug commented Nov 11, 2016

@retronym said (edited on Nov 11, 2016 12:51:47 AM UTC):
This might be #9828 (aka https://github.com/lrytz/jit-bug/tree/master, https://bugs.openjdk.java.net/browse/JDK-8160702, https://bugs.openjdk.java.net/browse/JDK-8148752)

Should be fixed in JDK 1.8.0_112 and higher.

@scabug
Copy link
Author

scabug commented Nov 11, 2016

@retronym said:
Speculatively closing, please re-open if the JDK upgrdade doesn't help.

@scabug scabug closed this as completed Nov 11, 2016
@scabug scabug added the bytecode label Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@retronym retronym

Labels
bytecode
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@retronym @scabug