This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Fix assert in lookup of section symbols
ClosedPublic

Authored by aardappel on Feb 10 2021, 5:36 PM.

Details

Summary

Fixes assert in: https://bugs.llvm.org/show_bug.cgi?id=49036

getWasmSection creates sections if they don't exist, but doesn't add them to the Symbols table. This may cause problems in subsequent calls to getOrCreateSymbol which checks this table, the calls createSymbol assuming it doesn't exist, which then checks UsedNames and finds out it does exist, causing an assert on trying to rename a non-temp symbol.

I tried also fixing the somewhat unintuitive forced suffixing (adding 0), but it turns out that WasmObjectWriter currently assumes these section symbols are unique, so that may have to be a separate fix.

Also worth noting is that getWasmSection calling createSymbol may not be correct to start with, given that createSymbol seems to assume it is creating non-section symbols. But again, for a future fix.

Related: where some of this was introduced: https://github.com/llvm/llvm-project/commit/8d396acac3bc21f688ac707bb42e4698dbdcab7e

Diff Detail

Event Timeline

aardappel created this revision.Feb 10 2021, 5:36 PM
aardappel requested review of this revision.Feb 10 2021, 5:36 PM
Herald added a project: Restricted Project. · View Herald Transcript
aardappel updated this revision to Diff 322878.Feb 10 2021, 5:37 PM
sbc100 accepted this revision.Feb 11 2021, 4:40 PM
sbc100 added inline comments.
llvm/test/MC/WebAssembly/section-symbol.s
10

This does seem rather odd. It seems like this should be a error... if we are going to add a test for this seemly broken behaviour, perhaps we should add a TODO with a link to a bug to address it?

This revision is now accepted and ready to land.Feb 11 2021, 4:40 PM
aardappel marked an inline comment as done.Feb 18 2021, 11:45 AM
aardappel added inline comments.
llvm/test/MC/WebAssembly/section-symbol.s
10
aardappel marked an inline comment as done.

Added bug todo

This revision was automatically updated to reflect the committed changes.