Skip to content

Commit

Permalink
Fix http://dartbug.com/23578:
Browse files Browse the repository at this point in the history 
- Complete revamp of isolate-specific flags.
- Associate flags with the isolate on creation.
- Dart_CreateIsolate and associate callback do take an
  extra flags argument.
- Make sure to clear IC data array when clearing code.

BUG=23578

Review URL: https://codereview.chromium.org//1162033005
  • Loading branch information
@iposva-google
iposva-google committed Jun 7, 2015
1 parent deb1697 commit f5e3f94
Show file tree
Hide file tree
Showing 45 changed files with 388 additions and 226 deletions.
5 changes: 3 additions & 2 deletions runtime/bin/gen_snapshot.cc
View file
Expand Up @@ -554,7 +554,8 @@ int main(int argc, char** argv) {
}

char* error;
Dart_Isolate isolate = Dart_CreateIsolate(NULL, NULL, NULL, NULL, &error);
Dart_Isolate isolate = Dart_CreateIsolate(
NULL, NULL, NULL, NULL, NULL, &error);
if (isolate == NULL) {
Log::PrintErr("Error: %s", error);
free(error);
Expand Down Expand Up @@ -600,7 +601,7 @@ int main(int argc, char** argv) {

// Now we create an isolate into which we load all the code that needs to
// be in the snapshot.
if (Dart_CreateIsolate(NULL, NULL, NULL, NULL, &error) == NULL) {
if (Dart_CreateIsolate(NULL, NULL, NULL, NULL, NULL, &error) == NULL) {
fprintf(stderr, "%s", error);
free(error);
exit(255);
Expand Down
8 changes: 8 additions & 0 deletions runtime/bin/main.cc
View file
Expand Up @@ -579,6 +579,7 @@ static Dart_Handle EnvironmentCallback(Dart_Handle name) {
static Dart_Isolate CreateIsolateAndSetupHelper(const char* script_uri,
const char* main,
const char* package_root,
Dart_IsolateFlags* flags,
char** error,
int* exit_code) {
ASSERT(script_uri != NULL);
Expand All @@ -588,6 +589,7 @@ static Dart_Isolate CreateIsolateAndSetupHelper(const char* script_uri,
isolate = Dart_CreateIsolate(script_uri,
main,
isolate_snapshot_buffer,
flags,
isolate_data,
error);

Expand Down Expand Up @@ -673,7 +675,11 @@ static Dart_Isolate CreateIsolateAndSetupHelper(const char* script_uri,
static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
const char* main,
const char* package_root,
Dart_IsolateFlags* flags,
void* data, char** error) {
// The VM should never call the isolate helper with a NULL flags.
ASSERT(flags != NULL);
ASSERT(flags->version == DART_FLAGS_CURRENT_VERSION);
IsolateData* parent_isolate_data = reinterpret_cast<IsolateData*>(data);
int exit_code = 0;
if (script_uri == NULL) {
Expand All @@ -697,6 +703,7 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
return CreateIsolateAndSetupHelper(script_uri,
main,
package_root,
flags,
error,
&exit_code);
}
Expand Down Expand Up @@ -967,6 +974,7 @@ void main(int argc, char** argv) {
Dart_Isolate isolate = CreateIsolateAndSetupHelper(script_name,
"main",
commandline_package_root,
NULL,
&error,
&exit_code);
if (isolate == NULL) {
Expand Down
1 change: 1 addition & 0 deletions runtime/bin/vmservice_dartium.cc
View file
Expand Up @@ -45,6 +45,7 @@ Dart_Isolate VmServiceServer::CreateIsolate(const uint8_t* snapshot_buffer) {
"main",
snapshot_buffer,
NULL,
NULL,
&error);
if (!isolate) {
fprintf(stderr, "Dart_CreateIsolate failed: %s\n", error);
Expand Down
23 changes: 23 additions & 0 deletions runtime/include/dart_api.h
View file
Expand Up @@ -685,6 +685,23 @@ DART_EXPORT Dart_Handle Dart_VisitPrologueWeakHandles(
*/
DART_EXPORT const char* Dart_VersionString();


/**
* Isolate specific flags are set when creating a new isolate using the
* Dart_IsolateFlags structure.
*
* Current version of flags is encoded in a 32-bit integer with 16 bits used
* for each part.
*/

#define DART_FLAGS_CURRENT_VERSION (0x00000001)

typedef struct {
int32_t version;
bool enable_type_checks;
bool enable_asserts;
} Dart_IsolateFlags;

/**
* An isolate creation and initialization callback function.
*
Expand Down Expand Up @@ -723,6 +740,9 @@ DART_EXPORT const char* Dart_VersionString();
* \param package_root The package root path for this isolate to resolve
* package imports against. If this parameter is NULL, the package root path
* of the parent isolate should be used.
* \param flags Default flags for this isolate being spawned. Either inherited
* from the spawning isolate or passed as parameters when spawning the
* isolate from Dart code.
* \param callback_data The callback data which was passed to the
* parent isolate when it was created by calling Dart_CreateIsolate().
* \param error A structure into which the embedder can place a
Expand All @@ -734,6 +754,7 @@ DART_EXPORT const char* Dart_VersionString();
typedef Dart_Isolate (*Dart_IsolateCreateCallback)(const char* script_uri,
const char* main,
const char* package_root,
Dart_IsolateFlags* flags,
void* callback_data,
char** error);

Expand Down Expand Up @@ -894,6 +915,7 @@ DART_EXPORT bool Dart_IsVMFlagSet(const char* flag_name);
* Provided only for advisory purposes to improve debugging messages.
* \param snapshot A buffer containing a snapshot of the isolate or
* NULL if no snapshot is provided.
* \param flags Pointer to VM specific flags or NULL for default flags.
* \param callback_data Embedder data. This data will be passed to
* the Dart_IsolateCreateCallback when new isolates are spawned from
* this parent isolate.
Expand All @@ -905,6 +927,7 @@ DART_EXPORT bool Dart_IsVMFlagSet(const char* flag_name);
DART_EXPORT Dart_Isolate Dart_CreateIsolate(const char* script_uri,
const char* main,
const uint8_t* snapshot,
Dart_IsolateFlags* flags,
void* callback_data,
char** error);
/* TODO(turnidge): Document behavior when there is already a current
Expand Down
36 changes: 16 additions & 20 deletions runtime/lib/isolate.cc
View file
Expand Up @@ -171,21 +171,20 @@ static bool CreateIsolate(Isolate* parent_isolate,
return false;
}

Dart_IsolateFlags api_flags;
state->isolate_flags()->CopyTo(&api_flags);

void* init_data = parent_isolate->init_callback_data();
Isolate* child_isolate = reinterpret_cast<Isolate*>(
(callback)(state->script_url(),
state->function_name(),
state->package_root(),
&api_flags,
init_data,
error));
if (child_isolate == NULL) {
return false;
}
// TODO(iposva): Evaluate whether it's ok to override the embedder's setup.
// Currently the strict_compilation flag is ignored if it's false and
// checked-mode was enabled using a command-line flag. The command-line flag
// overrides the user code's request.
child_isolate->set_strict_compilation(state->checked_mode());
if (!state->is_spawn_uri()) {
// For isolates spawned using the spawn semantics we set
// the origin_id to the origin_id of the parent isolate.
Expand Down Expand Up @@ -234,12 +233,10 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 4) {
#endif
// Get the parent function so that we get the right function name.
func = func.parent_function();
bool checkedFlag = isolate->strict_compilation();
Spawn(isolate, new IsolateSpawnState(port.Id(),
func,
message,
paused.value(),
checkedFlag));
paused.value()));
return Object::null();
}
}
Expand Down Expand Up @@ -278,20 +275,19 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnUri, 7) {
utf8_package_root[len] = '\0';
}

bool checkedFlag;
if (checked.IsNull()) {
checkedFlag = isolate->strict_compilation();
} else {
checkedFlag = checked.value();
IsolateSpawnState* state = new IsolateSpawnState(port.Id(),
canonical_uri,
utf8_package_root,
args,
message,
paused.value());
// If we were passed a value then override the default flags state for
// checked mode.
if (!checked.IsNull()) {
state->isolate_flags()->set_checked(checked.value());
}

Spawn(isolate, new IsolateSpawnState(port.Id(),
canonical_uri,
utf8_package_root,
args,
message,
paused.value(),
checkedFlag));
Spawn(isolate, state);
return Object::null();
}

Expand Down
2 changes: 1 addition & 1 deletion runtime/lib/object.cc
View file
Expand Up @@ -271,7 +271,7 @@ DEFINE_NATIVE_ENTRY(Object_as, 4) {
location, instance_type_name, type_name,
dst_name, Object::null_string());
} else {
ASSERT(Isolate::Current()->TypeChecksEnabled());
ASSERT(isolate->flags().type_checks());
bound_error_message = String::New(bound_error.ToErrorCString());
Exceptions::CreateAndThrowTypeError(
location, instance_type_name, Symbols::Empty(),
Expand Down
53 changes: 31 additions & 22 deletions runtime/vm/ast.cc
View file
Expand Up @@ -517,7 +517,7 @@ AstNode* LoadStaticFieldNode::MakeAssignmentNode(AstNode* rhs) {
if (field().is_final()) {
return NULL;
}
if (Isolate::Current()->TypeChecksEnabled()) {
if (Isolate::Current()->flags().type_checks()) {
rhs = new AssignableNode(
field().token_pos(),
rhs,
Expand Down Expand Up @@ -557,11 +557,14 @@ AstNode* LoadIndexedNode::MakeAssignmentNode(AstNode* rhs) {


AstNode* StaticGetterNode::MakeAssignmentNode(AstNode* rhs) {
Thread* thread = Thread::Current();
Zone* zone = thread->zone();
Isolate* isolate = thread->isolate();
if (is_super_getter()) {
ASSERT(receiver() != NULL);
const String& setter_name =
String::ZoneHandle(Field::SetterSymbol(field_name_));
Function& setter = Function::ZoneHandle(
String::ZoneHandle(zone, Field::SetterSymbol(field_name_));
Function& setter = Function::ZoneHandle(zone,
Resolver::ResolveDynamicAnyArgs(cls(), setter_name));
if (setter.IsNull() || setter.is_abstract()) {
// No instance setter found in super class chain,
Expand Down Expand Up @@ -591,25 +594,27 @@ AstNode* StaticGetterNode::MakeAssignmentNode(AstNode* rhs) {
return new StaticSetterNode(token_pos(), NULL, cls(), field_name_, rhs);
}

Object& obj = Object::Handle(prefix.LookupObject(field_name_));
Object& obj = Object::Handle(zone, prefix.LookupObject(field_name_));
if (obj.IsField()) {
const Field& field = Field::ZoneHandle(Field::Cast(obj).raw());
const Field& field = Field::ZoneHandle(zone, Field::Cast(obj).raw());
if (!field.is_final()) {
if (Isolate::Current()->TypeChecksEnabled()) {
if (isolate->flags().type_checks()) {
rhs = new AssignableNode(field.token_pos(),
rhs,
AbstractType::ZoneHandle(field.type()),
AbstractType::ZoneHandle(zone, field.type()),
field_name_);
}
return new StoreStaticFieldNode(token_pos(), field, rhs);
}
}

// No field found in prefix. Look for a setter function.
const String& setter_name = String::Handle(Field::SetterName(field_name_));
const String& setter_name = String::Handle(zone,
Field::SetterName(field_name_));
obj = prefix.LookupObject(setter_name);
if (obj.IsFunction()) {
const Function& setter = Function::ZoneHandle(Function::Cast(obj).raw());
const Function& setter = Function::ZoneHandle(zone,
Function::Cast(obj).raw());
ASSERT(setter.is_static() && setter.IsSetterFunction());
return new StaticSetterNode(
token_pos(), NULL, field_name_, setter, rhs);
Expand All @@ -622,25 +627,27 @@ AstNode* StaticGetterNode::MakeAssignmentNode(AstNode* rhs) {

if (owner().IsLibrary()) {
const Library& library = Library::Cast(owner());
Object& obj = Object::Handle(library.ResolveName(field_name_));
Object& obj = Object::Handle(zone, library.ResolveName(field_name_));
if (obj.IsField()) {
const Field& field = Field::ZoneHandle(Field::Cast(obj).raw());
const Field& field = Field::ZoneHandle(zone, Field::Cast(obj).raw());
if (!field.is_final()) {
if (Isolate::Current()->TypeChecksEnabled()) {
if (isolate->flags().type_checks()) {
rhs = new AssignableNode(field.token_pos(),
rhs,
AbstractType::ZoneHandle(field.type()),
AbstractType::ZoneHandle(zone, field.type()),
field_name_);
}
return new StoreStaticFieldNode(token_pos(), field, rhs);
}
}

// No field found in library. Look for a setter function.
const String& setter_name = String::Handle(Field::SetterName(field_name_));
const String& setter_name = String::Handle(zone,
Field::SetterName(field_name_));
obj = library.ResolveName(setter_name);
if (obj.IsFunction()) {
const Function& setter = Function::ZoneHandle(Function::Cast(obj).raw());
const Function& setter = Function::ZoneHandle(zone,
Function::Cast(obj).raw());
ASSERT(setter.is_static() && setter.IsSetterFunction());
return new StaticSetterNode(token_pos(), NULL, field_name_, setter, rhs);
}
Expand All @@ -651,15 +658,16 @@ AstNode* StaticGetterNode::MakeAssignmentNode(AstNode* rhs) {
}

const Function& setter =
Function::ZoneHandle(cls().LookupSetterFunction(field_name_));
Function::ZoneHandle(zone, cls().LookupSetterFunction(field_name_));
if (!setter.IsNull() && setter.IsStaticFunction()) {
return new StaticSetterNode(token_pos(), NULL, field_name_, setter, rhs);
}
// Could not find a static setter. Look for a field.
// Access to a lazily initialized static field that has not yet been
// initialized is compiled to a static implicit getter.
// A setter may not exist for such a field.
const Field& field = Field::ZoneHandle(cls().LookupStaticField(field_name_));
const Field& field = Field::ZoneHandle(zone,
cls().LookupStaticField(field_name_));
if (!field.IsNull()) {
if (field.is_final()) {
// Attempting to assign to a final variable will cause a NoSuchMethodError
Expand All @@ -668,18 +676,19 @@ AstNode* StaticGetterNode::MakeAssignmentNode(AstNode* rhs) {
return new StaticSetterNode(token_pos(), NULL, cls(), field_name_, rhs);
}
#if defined(DEBUG)
const String& getter_name = String::Handle(Field::GetterName(field_name_));
const String& getter_name = String::Handle(zone,
Field::GetterName(field_name_));
const Function& getter =
Function::Handle(cls().LookupStaticFunction(getter_name));
Function::Handle(zone, cls().LookupStaticFunction(getter_name));
ASSERT(!getter.IsNull() &&
(getter.kind() == RawFunction::kImplicitStaticFinalGetter));
#endif
if (Isolate::Current()->TypeChecksEnabled()) {
if (isolate->flags().type_checks()) {
rhs = new AssignableNode(
field.token_pos(),
rhs,
AbstractType::ZoneHandle(field.type()),
String::ZoneHandle(field.name()));
AbstractType::ZoneHandle(zone, field.type()),
String::ZoneHandle(zone, field.name()));
}
return new StoreStaticFieldNode(token_pos(), field, rhs);
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/vm/benchmark_test.h
View file
Expand Up @@ -81,7 +81,7 @@ class Benchmark {

Dart_Isolate CreateIsolate(const uint8_t* buffer) {
char* err = NULL;
isolate_ = Dart_CreateIsolate(NULL, NULL, buffer, NULL, &err);
isolate_ = Dart_CreateIsolate(NULL, NULL, buffer, NULL, NULL, &err);
EXPECT(isolate_ != NULL);
free(err);
return isolate_;
Expand Down

0 comments on commit f5e3f94

Please sign in to comment.