This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Add WebAssembly support to llvm-symbolizer
ClosedPublic

Authored by dschuff on Jan 22 2020, 5:41 PM.

Details

Summary

The only thing missing for basic llvm-symbolizer support is the ability on lib/Object to get a wasm symbol's section ID, which allows sorting and computation of the symbols' sizes.

Also, when the WasmAsmParser switches sections on new functions, also add the section to the list of Dwarf sections if Dwarf is being generated for assembly;
this allows writing of simple tests.

Diff Detail

Event Timeline

dschuff created this revision.Jan 22 2020, 5:41 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2020, 5:41 PM

@sbc100 I haven't added any tests yet but does this look like a reasonable use of the various Symbol/Ref types?

Unit tests: pass. 62114 tests passed, 0 failed and 808 were skipped.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Seems reasonable to me. Does llvm-symbolizer crash today when passed wasm object files?

dschuff updated this revision to Diff 239905.Jan 23 2020, 7:51 AM
dschuff removed a subscriber: merge_guards_bot.
  • remove debug print

Unit tests: pass. 62114 tests passed, 0 failed and 808 were skipped.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

dschuff updated this revision to Diff 240031.Jan 23 2020, 3:34 PM
  • Add test and Fix WebAssemblyAsmParser to emit dwarf for all text sections
dschuff updated this revision to Diff 240032.Jan 23 2020, 3:35 PM
  • clang-format
dschuff updated this revision to Diff 240033.Jan 23 2020, 3:39 PM
  • fix newline
dschuff retitled this revision from [WIP] Add WebAssembly support to llvm-symbolizer to [WebAssembly] Add WebAssembly support to llvm-symbolizer.Jan 23 2020, 3:44 PM
dschuff edited the summary of this revision. (Show Details)
dschuff added a reviewer: aardappel.

Unit tests: fail. 62110 tests passed, 6 failed and 807 were skipped.

failed: LLVM.tools/llvm-symbolizer/wasm-basic.s
failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 2 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

aardappel accepted this revision.Jan 23 2020, 4:29 PM
This revision is now accepted and ready to land.Jan 23 2020, 4:29 PM

Unit tests: fail. 62111 tests passed, 5 failed and 807 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 2 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

dschuff updated this revision to Diff 240047.Jan 23 2020, 4:36 PM
  • fix clang-tidy, add comment, fix whitespace

Unit tests: fail. 62110 tests passed, 6 failed and 807 were skipped.

failed: LLVM.tools/llvm-symbolizer/wasm-basic.s
failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 2 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

Unit tests: fail. 62111 tests passed, 5 failed and 807 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

jhenderson added inline comments.Jan 27 2020, 6:44 AM
llvm/test/tools/llvm-symbolizer/wasm-basic.s
4

Nit: no need for two blank lines.

16–18

Nit: Use two '##' characters for comments for readability. Also, consider putting the TODO on its own line to make it stand out better. Finally, missing trailing full stop.

dschuff updated this revision to Diff 240703.Jan 27 2020, 3:31 PM
dschuff marked 2 inline comments as done.
  • review comments
dschuff edited the summary of this revision. (Show Details)Jan 27 2020, 3:38 PM

Unit tests: fail. 62182 tests passed, 1 failed and 815 were skipped.

failed: LLVM.tools/llvm-symbolizer/wasm-basic.s

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

dschuff updated this revision to Diff 240727.Jan 27 2020, 4:42 PM
  • Update test CHECKs to account for removing the blank line (turns out the code actually works :)

Unit tests: pass. 62183 tests passed, 0 failed and 815 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

Looks good from my point of view, but a wasm expert should probably sign off.

Thanks! sbc and aardappel are the relevant wasm experts here.

This revision was automatically updated to reflect the committed changes.