Skip to content

Expose new JITModule::read_got_entry function#2865

Merged
cfallin merged 1 commit intobytecodealliance:mainfrom
eggyal:expose_get_got_address
May 4, 2021
Merged

Expose new JITModule::read_got_entry function#2865
cfallin merged 1 commit intobytecodealliance:mainfrom
eggyal:expose_get_got_address

Conversation

@eggyal
Copy link
Contributor

@eggyal eggyal commented Apr 29, 2021

This hasn't been discussed in a wasmtime issue, but (together with #2786) it's needed it for bjorn3/rustc_codegen_cranelift#1166.

It exposes JITModule::get_got_address a new JITModule::read_got_entry function so that, when jitting lazily, cranelift users can inspect the GOT to determine whether a given function's entry is still their shim trampoline or not (and therefore whether the function has already been jitted). This prevents the function from being jitted differently from simultaneous requests arising from different threads.

No test cases added, because exposing a function obviously doesn't change any behaviour.

Sorry, I don't know who would be an appropriate reviewer for this.

@bjorn3
Copy link
Contributor

bjorn3 commented Apr 29, 2021

Instead of exposing get_got_address directly, can you expose a function accepting a FuncId that reads the got entry?

@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Apr 29, 2021
@eggyal eggyal force-pushed the expose_get_got_address branch from 3df6087 to e4f9eeb Compare April 29, 2021 08:29
@eggyal eggyal changed the title Expose JITModule::get_got_address Expose new JITModule::read_got_entry function Apr 29, 2021
Copy link
Contributor

@bjorn3 bjorn3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cfallin Could you take a look at this?

@cfallin
Copy link
Member

cfallin commented May 4, 2021

Some exciting but almost certainly unrelated CI failures on Windows -- corruption issue of some sort?

     Running target\release\deps\strings-bc2f113fd401e08a.exe

running 3 tests
test hello_string ... ok
test overlapping_string ... ok
test multi_string ... FAILED

failures:

---- multi_string stdout ----
proptest: FileFailurePersistence::SourceParallel set, but failed to find lib.rs or main.rs
len=839, a='Πῌ𝈍ᾲὴὝ𝈡ῳὼ΄𝈘ᾶΘΊἝΈͽͿΆΆᴨφͺὗΊͿ῁𐅔ὕΖϴὌῪ ...

(This seems to be in wiggle -- cc @pchickey @sunfishcode)

restarting CI.

@cfallin cfallin merged commit 6a1a169 into bytecodealliance:main May 4, 2021
@eggyal eggyal mentioned this pull request May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants