feat: Linker #87
feat: Linker #87
Conversation
github-actions
bot
added
execution
validation
tests
priority-high
priority-medium
labels
|
Blocked on #88 |
… for linker Signed-off-by: George Cosma <[email protected]>
george-cosma
force-pushed
the
dev/george-linker
branch
3 times, most recently
from
737c1a5
to
9e29849
Compare
Codecov ReportAttention: Patch coverage is
|
| // TODO: what do we want to do if there is a missing import/export pair? Currently we fail the entire | ||
| // operation. Should it be a RuntimeError if said missing pair is called? |
There was a problem hiding this comment.
As per internal discussion, "better safe than sorry"
|
|
||
| // TODO: how do we handle the start function, if we don't have a LUT yet? |
There was a problem hiding this comment.
If a module has a start function, this function will error out if we attempt to run "start" if there are any unmet imports, even if they are not used in "start" itself.
There was a problem hiding this comment.
Running an example where we have an import and it's not met in wasm-interp leads to the same error: invalid import. I think this is the safe way to go about it.
(wasm-interp actually errors out even if we don't run any of the code:
(module
(import "console" "log" (func $log (param i32)))
(func $dummy
i32.const 0
drop
)
;; (start $dummy)
))
Signed-off-by: George Cosma <[email protected]>
george-cosma
force-pushed
the
dev/george-linker
branch
from
9e29849
to
bf9202b
Compare
george-cosma
requested review from
valexandru,
wucke13,
nerodesu017 and
florianhartung
| @@ -288,20 +357,30 @@ where | |||
| let (module_idx, func_idx) = | |||
| self.get_indicies(&function_ref.module_name, &function_ref.function_name)?; | |||
|
|
|||
| if module_idx != function_ref.module_index || func_idx != function_ref.function_index { | |||
| // TODO: should we return a different error here? | |||
| if module_idx != function_ref.module_index { | |||
There was a problem hiding this comment.
Maybe just one if statement here with both conditions combined?
There was a problem hiding this comment.
The reason I wanted to split them off was to be able to discern which reference is wrong - the module or the function. This was done to conform to the two errors: ModuleNotFound and FunctionNotFound. However, it would also be valid to say there should be a 3rd error type to indicate a stale or incorrect reference
There was a problem hiding this comment.
I see, that makes sense. I also agree on the 3rd error type for an incorrect reference.
Pull Request Overview
This pull request adds a "Lookup table" linker. See #83 first.
Basically, after each time a new module is added we attempt to create a "lookup table". This lookup table translate the imported functions from each module to the exported local function in the already-existing modules.
Resumability is kept by passing all module-specific details (like the wasm binary, the readers, function types, store, etc.) to the interpreter_loop::run function. These details have been compiled into a structure called
ExecutionInfo.Also of note, I split the "FunctionInstance" structure into two variants - local and imported, for clarity sake. This comes with the needed modification to the validation process and store-creation process.
Testing Strategy
This pull request was tested by writing a new test, and tested against the existing tests.
Formatting
cargo fmtcargo checkcargo buildcargo docnix fmtGithub Issue
This pull request closes #83