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

interplay inlineExceptionHandler and target:jvm-1.6 #5805

Closed
scabug opened this Issue May 18, 2012 · 15 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

scabug commented May 18, 2012

There appears to be a problem with InlineExceptionHandlers.

I noticed after compiling the compiler with -target:jvm-1.6 -optimize
as of scala/scala@446de86
and later trying to run with the usual VM options that activate the new verifier:
java -Xverify:all -XX:+UseSplitVerifier -XX:-FailOverToOldVerifier

There is some mismatch between stack map frames as emitted vs as computed by the verifier:

Exception in thread "main" java.lang.VerifyError: 
Bad type on operand stack in method
scala.reflect.internal.BaseTypeSeqs$BaseTypeSeq.liftedTree1$1(ILscala/reflect/internal/Types$RefinedType;)Lscala/reflect/internal/Types$Type; 
at offset 259
        at scala.reflect.internal.BaseTypeSeqs$class.newBaseTypeSeq(BaseTypeSeqs.scala:33)
        at scala.reflect.internal.SymbolTable.newBaseTypeSeq(SymbolTable.scala:13)
        at scala.reflect.internal.BaseTypeSeqs$class.$init$(BaseTypeSeqs.scala:176)
        at scala.reflect.internal.SymbolTable.<init>(SymbolTable.scala:13)
        at scala.tools.nsc.symtab.SymbolTable.<init>(SymbolTable.scala:9)
        at scala.tools.nsc.Global.<init>(Global.scala:34)
        at scala.tools.nsc.Global$.apply(Global.scala:1625)
        at scala.tools.nsc.Main$.newCompiler(Main.scala:76)
        at scala.tools.nsc.Driver.process(Driver.scala:45)
        at scala.tools.nsc.Driver.main(Driver.scala:65)
        at scala.tools.nsc.Main.main(Main.scala)

The problem does not surface when the compiler is compiled with
-Yinline -Yclosure-elim -Ydead-code

There are two candidate culprits:

  • GenASM, which delegates to ASM the computation of stack-maps, but computes some input for that: inameToSymbol(), getCommonSuperClass().
  • InlineExceptionHandlers

GenASM seems not to be the culprit because inameToSymbol() doesn't run at all (rather, getCommonSuperClass() always hits the reverseJavaName cache). And if getCommonSuperClass() had a bug, we would have noticed already.

First I tried diagnosing with ASM's CheckClassAdapter , http://asm.ow2.org/doc/faq.html#Q4 , but it finds no error.

Thus we're left with javap -c -private -verbose , which does show a difference for the method in question:

// version 1: compiled with -optimize
private final scala.reflect.internal.Types$Type liftedTree1$1(int, scala.reflect.internal.Types$RefinedType);
  Code:
   Stack=5, Locals=7, Args_size=3
   0:	aload_0
   1:	invokevirtual	#63; //Method scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer:()Lscala/reflect/internal/SymbolTable;
   4:	aload_2
   5:	invokevirtual	#91; //Method scala/reflect/internal/Types$RefinedType.parents:()Lscala/collection/immutable/List;
   8:	iconst_m1
   9:	aload_0
   10:	invokevirtual	#63; //Method scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer:()Lscala/reflect/internal/SymbolTable;
   13:	aload_2
   14:	invokevirtual	#91; //Method scala/reflect/internal/Types$RefinedType.parents:()Lscala/collection/immutable/List;
   17:	invokeinterface	#381,  2; //InterfaceMethod scala/reflect/internal/Types.lubDepth:(Lscala/collection/immutable/List;)I
   22:	invokeinterface	#385,  4; //InterfaceMethod scala/reflect/internal/Types.mergePrefixAndArgs:(Lscala/collection/immutable/List;II)Lscala/Option;
   27:	astore	5
   29:	aload	5
   31:	instanceof	#387; //class scala/Some
   34:	ifeq	86
   37:	aload	5
   39:	checkcast	#387; //class scala/Some
   42:	astore_3
   43:	aload_3
   44:	ifnull	86
   47:	aload_0
   48:	invokespecial	#47; //Method pending:()Lscala/collection/mutable/BitSet;
   51:	iload_1
   52:	invokestatic	#393; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
   55:	iconst_0
   56:	invokeinterface	#399,  3; //InterfaceMethod scala/collection/mutable/SetLike.update:(Ljava/lang/Object;Z)V
   61:	aload_0
   62:	invokevirtual	#40; //Method scala$reflect$internal$BaseTypeSeqs$$elems:()[Lscala/reflect/internal/Types$Type;
   65:	iload_1
   66:	aload_3
   67:	invokevirtual	#402; //Method scala/Some.x:()Ljava/lang/Object;
   70:	checkcast	#84; //class scala/reflect/internal/Types$Type
   73:	aastore
   74:	aload_3
   75:	invokevirtual	#402; //Method scala/Some.x:()Ljava/lang/Object;
   78:	checkcast	#84; //class scala/reflect/internal/Types$Type
   81:	astore	4
   83:	aload	4
   85:	areturn
   86:	getstatic	#407; //Field scala/None$.MODULE$:Lscala/None$;
   89:	dup
   90:	ifnonnull	102
   93:	pop
   94:	aload	5
   96:	ifnull	110
   99:	goto	156
   102:	aload	5
   104:	invokevirtual	#411; //Method java/lang/Object.equals:(Ljava/lang/Object;)Z
   107:	ifeq	156
   110:	aload_0
   111:	new	#349; //class scala/collection/mutable/StringBuilder
   114:	dup
   115:	invokespecial	#351; //Method scala/collection/mutable/StringBuilder."<init>":()V
   118:	ldc_w	#413; //String no common type instance of base types 
   121:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   124:	aload_2
   125:	invokevirtual	#91; //Method scala/reflect/internal/Types$RefinedType.parents:()Lscala/collection/immutable/List;
   128:	ldc_w	#415; //String , and 
   131:	invokeinterface	#362,  2; //InterfaceMethod scala/collection/TraversableOnce.mkString:(Ljava/lang/String;)Ljava/lang/String;
   136:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   139:	ldc_w	#417; //String  exists.
   142:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   145:	invokevirtual	#368; //Method scala/collection/mutable/StringBuilder.toString:()Ljava/lang/String;
   148:	invokespecial	#419; //Method typeError:(Ljava/lang/String;)Lscala/runtime/Nothing$;
   151:	astore	6
   153:	goto	167
   156:	new	#421; //class scala/MatchError
   159:	dup
   160:	aload	5
   162:	invokespecial	#424; //Method scala/MatchError."<init>":(Ljava/lang/Object;)V
   165:	astore	6
   167:	aload_0
   168:	invokevirtual	#63; //Method scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer:()Lscala/reflect/internal/SymbolTable;
   171:	invokeinterface	#69,  1; //InterfaceMethod scala/reflect/internal/BaseTypeSeqs.CyclicInheritance:()Ljava/lang/Throwable;
   176:	dup
   177:	ifnull	198
   180:	goto	207
   183:	astore	6
   185:	aload_0
   186:	invokevirtual	#63; //Method scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer:()Lscala/reflect/internal/SymbolTable;
   189:	invokeinterface	#69,  1; //InterfaceMethod scala/reflect/internal/BaseTypeSeqs.CyclicInheritance:()Ljava/lang/Throwable;
   194:	dup
   195:	ifnonnull	207
   198:	pop
   199:	aload	6
   201:	ifnull	215
   204:	goto	257
   207:	aload	6
   209:	invokevirtual	#411; //Method java/lang/Object.equals:(Ljava/lang/Object;)Z
   212:	ifeq	257
   215:	aload_0
   216:	new	#349; //class scala/collection/mutable/StringBuilder
   219:	dup
   220:	invokespecial	#351; //Method scala/collection/mutable/StringBuilder."<init>":()V
   223:	ldc_w	#426; //String computing the common type instance of base types 
   226:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   229:	aload_2
   230:	invokevirtual	#91; //Method scala/reflect/internal/Types$RefinedType.parents:()Lscala/collection/immutable/List;
   233:	ldc_w	#415; //String , and 
   236:	invokeinterface	#362,  2; //InterfaceMethod scala/collection/TraversableOnce.mkString:(Ljava/lang/String;)Ljava/lang/String;
   241:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   244:	ldc_w	#428; //String  leads to a cycle.
   247:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   250:	invokevirtual	#368; //Method scala/collection/mutable/StringBuilder.toString:()Ljava/lang/String;
   253:	invokespecial	#419; //Method typeError:(Ljava/lang/String;)Lscala/runtime/Nothing$;
   256:	athrow
   257:	aload	6
   259:	athrow
  Exception table:
   from   to  target type
    86   167   183   any
     0    85   183   any
  LocalVariableTable: 
   Start  Length  Slot  Name   Signature
   0      260      0    this       Lscala/reflect/internal/BaseTypeSeqs$BaseTypeSeq;
   0      260      1    i$1       I
   0      260      2    x2$1       Lscala/reflect/internal/Types$RefinedType;
   43      217      3    x2       Lscala/Some;

  LineNumberTable: 
   line 66: 0
   line 59: 4
   line 66: 8
   line 59: 13
   line 66: 17
   line 67: 29
   line 68: 47
   line 69: 61
   line 66: 66
   line 69: 73
   line 66: 74
   line 67: 81
   line 66: 83
   line 65: 85
   line 71: 86
   line 72: 110
   line 73: 111
   line 59: 124
   line 73: 128
   line 72: 148
   line 66: 156
   line 76: 167
   line 65: 167
   line 65: 183
   line 76: 185
   line 77: 215
   line 78: 216
   line 59: 229
   line 78: 233
   line 77: 253
   line 65: 257

  StackMapTable: number_of_entries = 10
   frame_type = 254 /* append */
     offset_delta = 86
     locals = [ bogus, bogus, class scala/Option ]
   frame_type = 79 /* same_locals_1_stack_item */
     stack = [ class scala/None$ ]
   frame_type = 7 /* same */
   frame_type = 45 /* same */
   frame_type = 252 /* append */
     offset_delta = 10
     locals = [ class java/lang/Object ]
   frame_type = 255 /* full_frame */
     offset_delta = 15
     locals = [ class scala/reflect/internal/BaseTypeSeqs$BaseTypeSeq, int, class scala/reflect/internal/Types$RefinedType ]
     stack = [ class java/lang/Throwable ]
   frame_type = 255 /* full_frame */
     offset_delta = 14
     locals = [ class scala/reflect/internal/BaseTypeSeqs$BaseTypeSeq, int, class scala/reflect/internal/Types$RefinedType, bogus, bogus, bogus, class java/lang/Object ]
     stack = [ class java/lang/Throwable ]
   frame_type = 72 /* same_locals_1_stack_item */
     stack = [ class java/lang/Throwable ]
   frame_type = 7 /* same */
   frame_type = 41 /* same */
// version 2: compiled with -Yinline -Yclosure-elim -Ydead-code
private final scala.reflect.internal.Types$Type liftedTree1$1(int, scala.reflect.internal.Types$RefinedType);
  Code:
   Stack=5, Locals=7, Args_size=3
   0:	aload_0
   1:	invokevirtual	#63; //Method scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer:()Lscala/reflect/internal/SymbolTable;
   4:	aload_2
   5:	invokevirtual	#91; //Method scala/reflect/internal/Types$RefinedType.parents:()Lscala/collection/immutable/List;
   8:	iconst_m1
   9:	aload_0
   10:	invokevirtual	#63; //Method scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer:()Lscala/reflect/internal/SymbolTable;
   13:	aload_2
   14:	invokevirtual	#91; //Method scala/reflect/internal/Types$RefinedType.parents:()Lscala/collection/immutable/List;
   17:	invokeinterface	#381,  2; //InterfaceMethod scala/reflect/internal/Types.lubDepth:(Lscala/collection/immutable/List;)I
   22:	invokeinterface	#385,  4; //InterfaceMethod scala/reflect/internal/Types.mergePrefixAndArgs:(Lscala/collection/immutable/List;II)Lscala/Option;
   27:	astore	5
   29:	aload	5
   31:	instanceof	#387; //class scala/Some
   34:	ifeq	86
   37:	aload	5
   39:	checkcast	#387; //class scala/Some
   42:	astore_3
   43:	aload_3
   44:	ifnull	86
   47:	aload_0
   48:	invokespecial	#47; //Method pending:()Lscala/collection/mutable/BitSet;
   51:	iload_1
   52:	invokestatic	#393; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer;
   55:	iconst_0
   56:	invokeinterface	#399,  3; //InterfaceMethod scala/collection/mutable/SetLike.update:(Ljava/lang/Object;Z)V
   61:	aload_0
   62:	invokevirtual	#40; //Method scala$reflect$internal$BaseTypeSeqs$$elems:()[Lscala/reflect/internal/Types$Type;
   65:	iload_1
   66:	aload_3
   67:	invokevirtual	#402; //Method scala/Some.x:()Ljava/lang/Object;
   70:	checkcast	#84; //class scala/reflect/internal/Types$Type
   73:	aastore
   74:	aload_3
   75:	invokevirtual	#402; //Method scala/Some.x:()Ljava/lang/Object;
   78:	checkcast	#84; //class scala/reflect/internal/Types$Type
   81:	astore	4
   83:	aload	4
   85:	areturn
   86:	getstatic	#407; //Field scala/None$.MODULE$:Lscala/None$;
   89:	dup
   90:	ifnonnull	102
   93:	pop
   94:	aload	5
   96:	ifnull	110
   99:	goto	152
   102:	aload	5
   104:	invokevirtual	#411; //Method java/lang/Object.equals:(Ljava/lang/Object;)Z
   107:	ifeq	152
   110:	aload_0
   111:	new	#349; //class scala/collection/mutable/StringBuilder
   114:	dup
   115:	invokespecial	#351; //Method scala/collection/mutable/StringBuilder."<init>":()V
   118:	ldc_w	#413; //String no common type instance of base types 
   121:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   124:	aload_2
   125:	invokevirtual	#91; //Method scala/reflect/internal/Types$RefinedType.parents:()Lscala/collection/immutable/List;
   128:	ldc_w	#415; //String , and 
   131:	invokeinterface	#362,  2; //InterfaceMethod scala/collection/TraversableOnce.mkString:(Ljava/lang/String;)Ljava/lang/String;
   136:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   139:	ldc_w	#417; //String  exists.
   142:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   145:	invokevirtual	#368; //Method scala/collection/mutable/StringBuilder.toString:()Ljava/lang/String;
   148:	invokespecial	#419; //Method typeError:(Ljava/lang/String;)Lscala/runtime/Nothing$;
   151:	athrow
   152:	new	#421; //class scala/MatchError
   155:	dup
   156:	aload	5
   158:	invokespecial	#424; //Method scala/MatchError."<init>":(Ljava/lang/Object;)V
   161:	athrow
   162:	astore	6
   164:	aload_0
   165:	invokevirtual	#63; //Method scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer:()Lscala/reflect/internal/SymbolTable;
   168:	invokeinterface	#69,  1; //InterfaceMethod scala/reflect/internal/BaseTypeSeqs.CyclicInheritance:()Ljava/lang/Throwable;
   173:	dup
   174:	ifnonnull	186
   177:	pop
   178:	aload	6
   180:	ifnull	194
   183:	goto	236
   186:	aload	6
   188:	invokevirtual	#411; //Method java/lang/Object.equals:(Ljava/lang/Object;)Z
   191:	ifeq	236
   194:	aload_0
   195:	new	#349; //class scala/collection/mutable/StringBuilder
   198:	dup
   199:	invokespecial	#351; //Method scala/collection/mutable/StringBuilder."<init>":()V
   202:	ldc_w	#426; //String computing the common type instance of base types 
   205:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   208:	aload_2
   209:	invokevirtual	#91; //Method scala/reflect/internal/Types$RefinedType.parents:()Lscala/collection/immutable/List;
   212:	ldc_w	#415; //String , and 
   215:	invokeinterface	#362,  2; //InterfaceMethod scala/collection/TraversableOnce.mkString:(Ljava/lang/String;)Ljava/lang/String;
   220:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   223:	ldc_w	#428; //String  leads to a cycle.
   226:	invokevirtual	#357; //Method scala/collection/mutable/StringBuilder.append:(Ljava/lang/Object;)Lscala/collection/mutable/StringBuilder;
   229:	invokevirtual	#368; //Method scala/collection/mutable/StringBuilder.toString:()Ljava/lang/String;
   232:	invokespecial	#419; //Method typeError:(Ljava/lang/String;)Lscala/runtime/Nothing$;
   235:	athrow
   236:	aload	6
   238:	athrow
  Exception table:
   from   to  target type
    86   162   162   any
     0    85   162   any
  LocalVariableTable: 
   Start  Length  Slot  Name   Signature
   0      239      0    this       Lscala/reflect/internal/BaseTypeSeqs$BaseTypeSeq;
   0      239      1    i$1       I
   0      239      2    x2$1       Lscala/reflect/internal/Types$RefinedType;
   43      196      3    x2       Lscala/Some;

  LineNumberTable: 
   line 66: 0
   line 59: 4
   line 66: 8
   line 59: 13
   line 66: 17
   line 67: 29
   line 68: 47
   line 69: 61
   line 66: 66
   line 69: 73
   line 66: 74
   line 67: 81
   line 66: 83
   line 65: 85
   line 71: 86
   line 72: 110
   line 73: 111
   line 59: 124
   line 73: 128
   line 72: 148
   line 66: 152
   line 65: 162
   line 76: 164
   line 77: 194
   line 78: 195
   line 59: 208
   line 78: 212
   line 77: 232
   line 65: 236

  StackMapTable: number_of_entries = 8
   frame_type = 254 /* append */
     offset_delta = 86
     locals = [ bogus, bogus, class scala/Option ]
   frame_type = 79 /* same_locals_1_stack_item */
     stack = [ class scala/None$ ]
   frame_type = 7 /* same */
   frame_type = 41 /* same */
   frame_type = 255 /* full_frame */
     offset_delta = 9
     locals = [ class scala/reflect/internal/BaseTypeSeqs$BaseTypeSeq, int, class scala/reflect/internal/Types$RefinedType ]
     stack = [ class java/lang/Throwable ]
   frame_type = 255 /* full_frame */
     offset_delta = 23
     locals = [ class scala/reflect/internal/BaseTypeSeqs$BaseTypeSeq, int, class scala/reflect/internal/Types$RefinedType, bogus, bogus, bogus, class java/lang/Throwable ]
     stack = [ class java/lang/Throwable ]
   frame_type = 7 /* same */
   frame_type = 41 /* same */

A question we could ask ourselves is: What performance gain (if any) does InlineExceptionHandlers buy us?

If negative, null, or negligible, then, shouldn't we consider removing InlineExceptionHandlers from the pipeline? This question is unrelated to this bug but becomes more pressing because of it.

Given that the build optimizes library and compiler, this bug means our build can't target 1.6 classfiles.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 18, 2012

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 19, 2012

@magarciaEPFL said:
-Xprint:cleanup shows more clearly the pattern matching at play:

      final private[this] def liftedTree1$1(i$1: Int, x2$1: scala.reflect.internal.Types$RefinedType): scala.reflect.internal.Types$Type = try {
        case <synthetic> val x1: Option = BaseTypeSeqs$BaseTypeSeq.this.scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer().mergePrefixAndArgs(x2$1.parents(), -1, BaseTypeSeqs$BaseTypeSeq.this.scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer().lubDepth(x2$1.parents()));
        case5(){
          if (x1.$isInstanceOf[Some]())
            {
              val x2: Some = (x1.$asInstanceOf[Some](): Some);
              if (x2.ne(null))
                matchEnd4({
                  BaseTypeSeqs$BaseTypeSeq.this.pending().update(scala.Int.box(i$1), false);
                  BaseTypeSeqs$BaseTypeSeq.this.scala$reflect$internal$BaseTypeSeqs$$elems().update(i$1, x2.x().$asInstanceOf[scala.reflect.internal.Types$Type]());
                  x2.x().$asInstanceOf[scala.reflect.internal.Types$Type]()
                })
              else
                case6()
            }
          else
            case6()
        };
        case6(){
          if (scala.None.==(x1))
            {
              val x3: Option = x1;
              matchEnd4(BaseTypeSeqs$BaseTypeSeq.this.typeError("no common type instance of base types ".+(x2$1.parents().mkString(", and ")).+(" exists.")))
            }
          else
            case7()
        };
        case7(){
          matchEnd4(throw new MatchError(x1))
        };
        matchEnd4(x: scala.reflect.internal.Types$Type){
          x
        }
      } catch {
        case (ex5 @ _) => {
          val x3: Throwable = ex5;
          case7(){
            if (BaseTypeSeqs$BaseTypeSeq.this.scala$reflect$internal$BaseTypeSeqs$BaseTypeSeq$$$outer().CyclicInheritance().==(x3))
              {
                val x4: Throwable = x3;
                matchEnd6(BaseTypeSeqs$BaseTypeSeq.this.typeError("computing the common type instance of base types ".+(x2$1.parents().mkString(", and ")).+(" leads to a cycle.")))
              }
            else
              case8()
          };
          case8(){
            matchEnd6(throw ex5)
          };
          matchEnd6(x: scala.reflect.internal.Types$Type){
            x
          }
        }
      };
@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 19, 2012

@magarciaEPFL said:
And here's the source for the above:

// snippet.scala
            try {
              mergePrefixAndArgs(variants, -1, lubDepth(variants)) match {
                case Some(tp0) =>
                  pending(i) = false
                  elems(i) = tp0
                  tp0
                case None =>
                  typeError(
                    "no common type instance of base types "+(variants mkString ", and ")+" exists.")
              }
            } catch {
              case CyclicInheritance =>
                typeError(
                  "computing the common type instance of base types "+(variants mkString ", and ")+" leads to a cycle.")
            }

which is part of method:

// method.scala
    def apply(i: Int): Type =
      if(pending contains i) {
        pending.clear()
        throw CyclicInheritance
      } else
        elems(i) match {
          case rtp @ RefinedType(variants, decls) =>
            // can't assert decls.isEmpty; see t0764
            //if (!decls.isEmpty) assert(false, "computing closure of "+this+":"+this.isInstanceOf[RefinedType]+"/"+closureCache(j))
            //Console.println("compute closure of "+this+" => glb("+variants+")")
            pending += i
            try {
              mergePrefixAndArgs(variants, -1, lubDepth(variants)) match {
                case Some(tp0) =>
                  pending(i) = false
                  elems(i) = tp0
                  tp0
                case None =>
                  typeError(
                    "no common type instance of base types "+(variants mkString ", and ")+" exists.")
              }
            } catch {
              case CyclicInheritance =>
                typeError(
                  "computing the common type instance of base types "+(variants mkString ", and ")+" leads to a cycle.")
            }
          case tp =>
            tp
        }
@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 21, 2012

@magarciaEPFL said:
@VladUreche, do we have any profiling comparison with and without InlineExceptionHandlers ?

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 21, 2012

@VladUreche said:
No, unfortunately we don't. There's a benchmarking project posted since the time inlineEH was ready, but nobody was interested so far.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 22, 2012

@magarciaEPFL said:
@VladUreche, it would be great if you could minimize this bug. Doing so doesn't require GenASM knowledge, but looking for the code pattern that triggers a VerifyError under -target:jvm-1.6. The before and after code to compare involves -Yinline-handlers only, no other optimization phase changes the verification-error outcome.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 23, 2012

@magarciaEPFL said (edited on May 23, 2012 1:22:47 PM UTC):
Partial diagnosis: the VerifyError goes away when scala.reflect.internal.BaseTypeSeqs$BaseTypeSeq.typeError is emitted from a modified version:

    private def typeError(msg: String): TypeError =
      new TypeError(
        "the type intersection "+(parents mkString " with ")+" is malformed"+
        "\n --- because ---\n"+msg)

(its callsites updated to read throw typeError(...). With those changes, the result of that method can be stored into a local var, just like InlineExceptionHandlers wants.

Therefore it's not a GenASM bug.

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 23, 2012

@magarciaEPFL said:
Still trying to find out why InlineExceptionHandlers believes there's a value on the operand stack when there isn't.

One of the problematic snippets is:

  typeError(
    "no common type instance of base types "+(variants mkString ", and ")+" exists.")

The ICode below corresponds to the above, and is received as input by InlineExceptionHandlers (please notice Inliner didn't inline anything):

  21: 
    71	LOAD_LOCAL(value x1)
    71	STORE_LOCAL(value x3)
    71	SCOPE_ENTER value x3
    72	THIS(BaseTypeSeqs$BaseTypeSeq)
    73	CALL_PRIMITIVE(StartConcat)
    73	CONSTANT("no common type instance of base types ")
    73	CALL_PRIMITIVE(StringConcat(REF(class String)))
    59	LOAD_LOCAL(value x2$1)
    59	CALL_METHOD scala.reflect.internal.Types$RefinedType.parents (dynamic)
    73	CONSTANT(", and ")
    73	CALL_METHOD scala.collection.TraversableOnce.mkString (dynamic)
    73	CALL_PRIMITIVE(StringConcat(REF(class String)))
    73	CONSTANT(" exists.")
    73	CALL_PRIMITIVE(StringConcat(REF(class String)))
    73	CALL_PRIMITIVE(EndConcat)
    72	CALL_METHOD scala.reflect.internal.BaseTypeSeqs$BaseTypeSeq.typeError (static-instance)
    72	THROW(Throwable)

i.e. the THROW instruction expects an exception that isn't there, because typeError() only returns by throwing an exception:

    private def typeError(msg: String): Nothing =
      throw new TypeError(
        "the type intersection "+(parents mkString " with ")+" is malformed"+
        "\n --- because ---\n"+msg)

Looking at the expression with -Xprint:clean -Yshow-trees we find a typeError() callsite providing a Nothing argument to a "matchEnd4" Apply (what is that supposed to be? A forward jump? A callsite?), whose tpe is scala.reflect.internal.Types$Type. What that is must play a role in how GenICode emits that THROW out of place (ceteris paribus).

    Apply( // case def matchEnd4(x: scala.reflect.internal.Types$Type): scala.reflect.internal.Types$Type, tree.tpe=scala.reflect.internal.Types$Type
      "matchEnd4" // case def matchEnd4(x: scala.reflect.internal.Types$Type): scala.reflect.internal.Types$Type, tree.tpe=(x: scala.reflect.internal.Types$Type)scala.reflect.internal.Types$Type
      Apply( // private def typeError(msg: String): Nothing in class BaseTypeSeqs$BaseTypeSeq, tree.tpe=Nothing
        BaseTypeSeqs$BaseTypeSeq.this."typeError" // private def typeError(msg: String): Nothing in class BaseTypeSeqs$BaseTypeSeq, tree.tpe=(msg: String)Nothing
        Apply( // final def +(x$1: Object): String in class String, tree.tpe=String
          "no common type instance of base types ".+(x2$1.parents().mkString(", and "))."$plus" // final def +(x$1: Object): String in class String, tree.tpe=(x$1: Object)String
          " exists."
        )
      )
    )
@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 24, 2012

@magarciaEPFL said:
Minimized:

object Test {

  val CertainException: Throwable = new Throwable

  def error(msg: String): Nothing = throw new Throwable

  def main(args: Array[String]): Unit = {
    try {
      error("abc")
    } catch {
      case CertainException =>
        error("xyz")
    }
  }

}
@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 24, 2012

@VladUreche said:
It's very nice too see all that progress on the bug. :)
I can check it out today. But since you've been at it for quite some time, should I just let you finish it?

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 24, 2012

@magarciaEPFL said:
@VladUreche, I'll be looking into getting valid bytecode (including passing the 1.6 verifier) for the example, not so much out of interest in InlineExceptionHandlers but the 1.6 verifier, looking carefully at those assignments to locals (as needed by LabelDef) which are eaten away by BasicBlock.enterIgnoreMode. That happens in ICodePhase.adapt(), as in the example, right after an invocation to a Nothing method. In the example, that invocation is an argument to a jump, ie Nothing is supposed to be assigned to a local.

      } else if (from == NothingReference) {
        ctx.bb.emit(THROW(ThrowableClass))
        ctx.bb.enterIgnoreMode

I'll look into this bug but any implications for tail-calls you'll have to figure out yourself anyway! :)

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 24, 2012

@magarciaEPFL said:
It was a bug in GenASM's getCommonSuperclass(), which failed to account for the scala.runtime.Nothing$ we emit for methods with Nothing return-type.

The JVM-wise lub was set to j.l.Object when it should have been j.l.Throwable as shown below (lub of
j.l.Throwable and scala.runtime.Nothing$ is j.l.Throwable )

With -Yinline-handlers, the protected minimized snippet includes two assignments to local slot 1 (ASM notation):

// single covered block
   L0
    ALOAD 0
    LDC "abc"
    INVOKEVIRTUAL Test.error (Ljava/lang/String;)Lscala/runtime/Nothing$;
    ASTORE 1 // without inlineEH there's a THROW here
. . . 
// start of the start block of the exception handler
   FRAME SAME1 java/lang/Throwable
    ASTORE 1
    ALOAD 0
    INVOKEVIRTUAL Test.CertainException ()Ljava/lang/Throwable;
. . .
@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 24, 2012

@magarciaEPFL said:
Pull request with fix at scala/scala#619

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 24, 2012

@VladUreche said:
Yeey, it wasn't the inlineEH phase! :))

@scabug

This comment has been minimized.

Copy link
Author

scabug commented May 25, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment