This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] MC: Fix value of R_WEBASSEMBLY_TABLE_INDEX relocations
ClosedPublic

Authored by sbc100 on Jun 6 2017, 2:52 PM.

Details

Summary

Previously we were writing the value function index space
value but for these types of relocations we want to be
writing the table element index space value.

Add a test case for these relocation types that fails
without this change.

Event Timeline

sbc100 created this revision.Jun 6 2017, 2:52 PM
sbc100 updated this revision to Diff 101629.Jun 6 2017, 2:55 PM

Add comments

sbc100 updated this revision to Diff 101632.Jun 6 2017, 2:59 PM
  • revert part
sbc100 edited the summary of this revision. (Show Details)Jun 6 2017, 2:59 PM
sbc100 added reviewers: sunfish, jgravelle-google.
sbc100 updated this revision to Diff 101633.Jun 6 2017, 3:00 PM
sbc100 edited the summary of this revision. (Show Details)
  • spelling fix
sbc100 updated this revision to Diff 101634.Jun 6 2017, 3:04 PM
  • check for elem section
sbc100 edited the summary of this revision. (Show Details)Jun 7 2017, 2:40 PM

Looks fine but I don't understand it really. @sunfish ?

lib/MC/WasmObjectWriter.cpp
633–634

Should probably either maintain the horizontal alignment of these comments, or remove the alignment for both

sbc100 updated this revision to Diff 102054.Jun 9 2017, 10:57 AM
  • Merge remote-tracking branch 'origin/master' into fix_table_index_reloc
  • Merge remote-tracking branch 'origin/master' into fix_table_index_reloc
  • Merge remote-tracking branch 'origin/master' into fix_table_index_reloc
  • Merge remote-tracking branch 'origin/master' into fix_table_index_reloc
  • update comments
sbc100 marked an inline comment as done.Jun 9 2017, 10:58 AM
sbc100 updated this revision to Diff 102055.Jun 9 2017, 10:59 AM
  • remove extra newline

@sunfish, could you that a look? I think this should be fairly non-controversial.

sunfish accepted this revision.Jun 12 2017, 4:09 PM
sunfish added inline comments.
lib/MC/WasmObjectWriter.cpp
155–156

Just curious: what's behind this change? At a quick glance, MCSymbols's operator<< (ultimately, MCSymbol::print) seems pretty sane.

This revision is now accepted and ready to land.Jun 12 2017, 4:09 PM
sbc100 added inline comments.Jun 12 2017, 4:21 PM
lib/MC/WasmObjectWriter.cpp
155–156

Oh, I see. I probably should just deference the pointer (Not really part of this change anyway).

sbc100 updated this revision to Diff 102257.Jun 12 2017, 4:26 PM
  • revert change to symbol printing
This revision was automatically updated to reflect the committed changes.