Stream: git-wasmtime

Topic: wasmtime / Issue #1283 cranelift-module: clarify behavior...

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2020 at 15:53):

froydnj opened Issue #1283:

While trying to update lucet to the most recent cranelift, I ran into some issues that stemmed from bytecodealliance/cranelift#1209 changing the semantics of what zero-initialized data did. Previously, zero-initialized data was essentially just like normal data, except that cranelift-module took care of generating the initial vector of zeros to modify. After that PR, zero-initialized data now had special semantics of not actually existing in the generated object, but living in .bss (or similar) and being zero-initialized by the runtime loader.

This was an excellent change! However, lucetc depended on the old behavior, in particular the application of relocations to the declared data. I think lucetc's behavior is wrong, and I intend to write up a PR for that.

I don't think relocations (declare_func_in_data, etc.) should actually apply to zero-initialized data...but cranelift-module happily lets you declare zero-initialized data and also declare that there should be various relocations applied to said data, which I don't think is going to translate well. (If it is supposed to translate well, faerie, at least, currently has bugs in that area.)

Is that line of thinking correct, i.e. there should be some additional checks around:

https://github.com/bytecodealliance/wasmtime/blob/f76b36f73720508caa1a50ce4cf388b42855789a/cranelift/module/src/module.rs#L646-L652

regarding zero-initialized data prior to handing things off to the backend?

view this post on Zulip Wasmtime GitHub notifications bot (Mar 11 2020 at 17:10):

fitzgen labeled Issue #1283:

While trying to update lucet to the most recent cranelift, I ran into some issues that stemmed from bytecodealliance/cranelift#1209 changing the semantics of what zero-initialized data did. Previously, zero-initialized data was essentially just like normal data, except that cranelift-module took care of generating the initial vector of zeros to modify. After that PR, zero-initialized data now had special semantics of not actually existing in the generated object, but living in .bss (or similar) and being zero-initialized by the runtime loader.

This was an excellent change! However, lucetc depended on the old behavior, in particular the application of relocations to the declared data. I think lucetc's behavior is wrong, and I intend to write up a PR for that.

I don't think relocations (declare_func_in_data, etc.) should actually apply to zero-initialized data...but cranelift-module happily lets you declare zero-initialized data and also declare that there should be various relocations applied to said data, which I don't think is going to translate well. (If it is supposed to translate well, faerie, at least, currently has bugs in that area.)

Is that line of thinking correct, i.e. there should be some additional checks around:

https://github.com/bytecodealliance/wasmtime/blob/f76b36f73720508caa1a50ce4cf388b42855789a/cranelift/module/src/module.rs#L646-L652

regarding zero-initialized data prior to handing things off to the backend?

view this post on Zulip Wasmtime GitHub notifications bot (Mar 12 2020 at 00:11):

philipc commented on Issue #1283:

Yeah, cranelift-object won't handle this correctly either. If it had to, the best it could do is choose which section based on the number of relocs (similiar to what read only data does). But I don't see a problem with disallowing these relocations in cranelift-module.

Last updated: Nov 22 2024 at 16:03 UTC