Stream: git-wasmtime

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


view this post on Zulip Wasmtime GitHub notifications bot (May 04 2022 at 22:43):

cfallin commented on issue #1283:

This is an old issue referring to Lucet, which has been end-of-lifed; and it looks like the initial question had been somewhat clarified in any case. I will go ahead and close but please feel free to open new related issues as necessary.

view this post on Zulip Wasmtime GitHub notifications bot (May 04 2022 at 22:43):

cfallin closed 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 (May 05 2022 at 13:13):

bjorn3 commented on issue #1283:

This doesn't just affect lucet. It affects anyone trying to use define_zeroinit in combination with relocations. This is neither implemented correctly as using the .data.rel.ro section, nor as giving an error. Instead it either gives a linker error, a runtime error or silently goes wrong I think.

view this post on Zulip Wasmtime GitHub notifications bot (May 07 2022 at 10:48):

bjorn3 commented on issue #1283:

@cfallin could you reopen this?

view this post on Zulip Wasmtime GitHub notifications bot (May 07 2022 at 15:06):

cfallin commented on issue #1283:

Yep, sorry, re-opening!

view this post on Zulip Wasmtime GitHub notifications bot (May 07 2022 at 15:06):

cfallin reopened 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?


Last updated: Nov 22 2024 at 17:03 UTC