Page MenuHomePhabricator

[lld][WebAssembly] Allow `atomics` feature with unshared memory
ClosedPublic

Authored by tlively on May 6 2020, 5:09 PM.

Details

Summary

https://github.com/WebAssembly/threads/issues/144 updated the
WebAssembly threads proposal to make atomic operations on unshared
memories valid. This change updates the feature checking in the linker
accordingly. Production WebAssembly engines have not yet been updated
for this change, so after this change users who accidentally use
atomics with unshared memories will get validation errors at runtime
rather than link errors. This is a small usability regression, but I
think it is warranted because 1) such users are already off the
well-beaten path and 2) as engines do begin to implement this change I
don't want the tools to block experimentation and use of the relaxed
rules.

Diff Detail

Event Timeline

tlively created this revision.May 6 2020, 5:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2020, 5:09 PM
sbc100 added a comment.May 6 2020, 5:52 PM

So this will effect users who accidentally use -pthread in their cflags but not their ldflags?

If a user uses -pthreadon some object files we have a different way to catch that a link time right? (I think that is more the common problem).

Imagine a project that links statically without threads but its as dependency on some autoconf or cmake library that happens to throw in the -pthread... such users would hit this error today so I guess its only new users who do this that would be a runtime rather than link time error after this change.

Do you think this change makes it much hard to go from the error message to figuring out that you need to remove a stray -pthread from some CFLAGS somewhere? At least the current error tells you which object is the problem.

sbc100 accepted this revision.Sep 24 2020, 3:14 PM

But I guess its only temporary as soon engines will catch up and the error will go away.

This revision is now accepted and ready to land.Sep 24 2020, 3:14 PM

But I guess its only temporary as soon engines will catch up and the error will go away.

In fact they have already caught up. I will update the commit message before landing.

This revision was landed with ongoing or failed builds.Sep 24 2020, 8:35 PM
This revision was automatically updated to reflect the committed changes.