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.

Sign up for GitHub

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

fix(interface-types) Casting i32 to usize may cause problems #1334

Closed
Hywan opened this issue Mar 26, 2020 · 0 comments · Fixed by #1337
Closed

fix(interface-types) Casting i32 to usize may cause problems #1334

Hywan opened this issue Mar 26, 2020 · 0 comments · Fixed by #1337
Assignees
@Hywan
Labels
bug Something isn't working

Comments

@Hywan
Copy link
Contributor

Hywan commented Mar 26, 2020

In WIT instructions, we cast i32 to usize. It may cause problems, as noticed in https://github.com/wasmerio/wasmer/pull/1329/files#r397396580. We must handle those cases.
@Hywan Hywan added bug Something isn't working 📦 lib-interface-types labels Mar 26, 2020
@Hywan Hywan self-assigned this Mar 26, 2020
@Hywan Hywan mentioned this issue Mar 26, 2020
Merged
bors bot added a commit that referenced this issue Mar 26, 2020
@bors @Hywan
Merge #1337
273e70b 
1337: feat(interface-types) Better handling of i32 to usize casts r=Hywan a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>
bors bot added a commit that referenced this issue Mar 26, 2020
@bors @Hywan
Merge #1337
64cf18c 
1337: feat(interface-types) Better handling of i32 to usize casts r=Hywan a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>
bors bot added a commit that referenced this issue Mar 26, 2020
@bors @Hywan
Merge #1337
8446667 
1337: feat(interface-types) Better handling of i32 to usize casts r=syrusakbary a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>
bors bot added a commit that referenced this issue Mar 26, 2020
@bors @Hywan
Merge #1337
ecef633 
1337: feat(interface-types) Better handling of i32 to usize casts r=MarkMcCaskey a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>
bors bot added a commit that referenced this issue Mar 26, 2020
@bors @Hywan
Merge #1337
a7b203f 
1337: feat(interface-types) Better handling of i32 to usize casts r=MarkMcCaskey a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>
bors bot added a commit that referenced this issue Mar 27, 2020
@bors @Hywan
Merge #1337
3167b4f 
1337: feat(interface-types) Better handling of i32 to usize casts r=Hywan a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>
bors bot added a commit that referenced this issue Mar 28, 2020
@bors @Hywan
Merge #1335 #1337
742d44c 
1335:  fix(runtime-c-api) Change mutability of `memory` to`const` in `wasmer_memory_data_length` r=Hywan a=Hywan

This PR changes mutability of `memory` for `const` in `wasmer_memory_data_length` to be more consistent regarding the returned value of `wasmer_instance_context_memory`.

Fixes #1314. 
Fixes wasmerio/docs.wasmer.io#43.

1337: feat(interface-types) Better handling of i32 to usize casts r=Hywan a=Hywan

Fix #1334 

This PR introduces a new `NegativeValue` error.
This PR fixes some `usize` and `u32` types for indexes (small changes).
Finally, this PR casts `i32` to `usize` by using the `TryFrom` implementation. That way:

```rust
let pointer = to_native::<i32>(&inputs[0], instruction)? as usize;
```

becomes:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)?.try_into().map_err(|_| {
    InstructionError::new(
        instruction,
        InstructionErrorKind::NegativeValue { subject: "pointer" },
    )
})?;
```

It's more verbose, but it handles the casting correctly.

Note: Maybe we should implement `From<TryFromIntError>` for `InstructionErrorKind`? What do you think?

Edit: With `From<TryFromIntError>`, it looks like this:

```rust
let pointer: usize = to_native::<i32>(&inputs[0], instruction)? // convert from WIT to native `i32`
    .try_into() // convert to `usize`
    .map_err(|e| (e, "pointer").into()) // convert the `TryFromIntError` to `InstructionErrorKind::NegativeValue`
    .map_err(|k| InstructionError::new(instruction, k))?; // convert to an `InstructionError`
```

It is indeed simpler. Keeping things like this.

Co-authored-by: Ivan Enderlin <ivan.enderlin@hoa-project.net>
Co-authored-by: Ivan Enderlin <ivan.enderlin@wanadoo.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Hywan Hywan

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(interface-types) Better handling of i32 to usize casts Hywan/wasmer
1 participant
@Hywan