Skip to content

Missing dynamic checks in DDC with call method allows heap unsoundness #29915

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

Closed
eernstg opened this issue Jun 16, 2017 · 4 comments
Closed

Missing dynamic checks in DDC with call method allows heap unsoundness #29915

eernstg opened this issue Jun 16, 2017 · 4 comments
Labels
closed-duplicate Closed in favor of an existing report soundness web-dev-compiler

Comments

@eernstg
Copy link
Member

eernstg commented Jun 16, 2017

As a variant of the example shown in #29913, the following example illustrates that DDC does not generate all the needed checks in order to ensure heap soundness when we mix covariance (with a generic class) and contravariance (with a function type), using a call method. The program is accepted by dartanalyzer --strong n018.dart with no issues.

class A {}
class B extends A {}
class C extends B {}

typedef void F<T>(T t);

class ClassF<T> {
  T x;
  void call(T t) {
    x = t;
  }
}

main() {
  ClassF<C> cc = new ClassF<C>();
  ClassF<A> ca = cc; // An upcast, per covariance.
  F<A> f = ca; // "No-op cast to function", but should fail in strong mode.
  // Still running, though!
  print(f.runtimeType); // 'ClassF<C>', cannot be considered an `F<A>`.
  f(new A()); // Statically safe, but dynamically the arg should be a C.
}

When compiling and executing the program using dartdevc.dart from 5982ace, 2017-06-15 14:18:09, ddc n018.dart, the resulting output is as follows:

ClassF<C>

/usr/local/google/home/eernst/devel/dart/work/sdk/pkg/dev_compiler/lib/js/common/dart_sdk.js:9877
          throw e;
          ^
Type 'A' is not a subtype of type 'C'

This differs from the situation in #29913 in that we do get the dynamic argument type check at the invocation of the call method (presumably because that's a normal class-covariance check), but we still have an unsound heap for a while, because f in main refers to an instance of ClassF<C>, and such an object cannot be considered to have type F<A> as declared.

@vsmenon
Copy link
Member

vsmenon commented Jun 19, 2017

@jmesserly
Copy link

yes this should be covered already in my work-in-progress CL. I will make sure we have a test for it.

@jmesserly
Copy link

BTW, my CL will not have it fail on the F<A> f = ca line. Instead it will fail on the f(new A()); call

@jmesserly
Copy link

thanks for the lovely example. going to merge this into #27259

@jmesserly jmesserly added the closed-duplicate Closed in favor of an existing report label Jun 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-duplicate Closed in favor of an existing report soundness web-dev-compiler
Projects
None yet
Development

No branches or pull requests

4 participants