Skip to content
This repository has been archived by the owner on Apr 15, 2022. It is now read-only.

Remove asm feature #44

Closed
rvolosatovs opened this issue Jan 5, 2022 · 0 comments · Fixed by #74
Closed

Remove asm feature #44

rvolosatovs opened this issue Jan 5, 2022 · 0 comments · Fixed by #74
Assignees
Labels
debt Issues to deal with later discussion This requires discussion before action can be taken
Milestone

Comments

@rvolosatovs
Copy link
Contributor

rvolosatovs commented Jan 5, 2022

asm is getting stabilized upstream and the feature is not strictly necessary anymore for the implementation.
Note that only the host side needs it, currently exclusively for the purpose of executing syscalls.

Unfortunately miri does not support inline assembly at the time of writing (rust-lang/miri#1045, rust-lang/miri#11 is the tracking issue)

Few options here:

  1. Remove miri and asm feature - we lose potential for testing against UB on CI
  2. Disable miri for tests involving host side - greatly diminishes the benefit of actually using miri
  3. Disable miri selectively for each test involving host side and inline assembly - this is going to be quite tedious to maintain
  4. It's not broken, so don't fix it - we can simply wait until inline assembly is supported by miri. That may take long and that means we have to maintain the feature within the implementation, but that is actually fairly trivial, since the host simply ignores the syscalls when feature is disabled. Note, that it is possible that will have some mechanism in place for selectively disabling/enabling host-side call handlers at runtime in the future, if we had such functionality, we could then simply disable all calls requiring asm when miri is enabled and drop the feature.

For now, I vote for 4. and suggest to revisit this in a few sprints.

Refs rust-lang/rust#91728 enarx/enarx#1178

@rvolosatovs rvolosatovs added debt Issues to deal with later discussion This requires discussion before action can be taken labels Jan 5, 2022
@rvolosatovs rvolosatovs added this to the Backlog milestone Jan 6, 2022
@platten platten assigned bstrie and unassigned npmccallum Jan 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Issues to deal with later discussion This requires discussion before action can be taken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants