This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Demangle symbol name in export error msg when -demangle is specified
ClosedPublic

Authored by oontvoo on May 16 2022, 4:24 PM.

Diff Detail

Event Timeline

oontvoo created this revision.May 16 2022, 4:24 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 16 2022, 4:25 PM
oontvoo requested review of this revision.May 16 2022, 4:25 PM
keith accepted this revision as: keith.May 16 2022, 4:28 PM
keith added a subscriber: keith.
keith added inline comments.
lld/test/MachO/demangle.s
22

super nit: missing space after :

This revision is now accepted and ready to land.May 16 2022, 4:28 PM
oontvoo updated this revision to Diff 429892.May 16 2022, 4:33 PM

nop update to test arc diff

Herald added a project: Restricted Project. · View Herald TranscriptMay 16 2022, 4:33 PM
oontvoo updated this revision to Diff 429895.May 16 2022, 4:44 PM
oontvoo marked an inline comment as done.

updated diff:

  • addressed review comment
  • trimmed error msg
This revision was landed with ongoing or failed builds.May 16 2022, 4:48 PM
This revision was automatically updated to reflect the committed changes.

Seemed to fail on windows bot:

$ ":" "RUN: at line 12"
$ "not" "ld64.lld" "-arch" "x86_64" "-platform_version" "macos" "11.0" "11.0" "-syslibroot" "C:/buildbot-root/llvm-clang-x86_64-sie-win/llvm-project/lld/test\MachO\Inputs\MacOSX.sdk" "-fatal_warnings" "-exported_symbol" "__ZTIN3foo3bar4MethE" "-exported_symbol" "__ZTSN3foo3bar4MethE" "C:\buildbot-root\llvm-clang-x86_64-sie-win\build\tools\lld\test\MachO\Output\demangle.s.tmp/export-symbols.o" "-o" "/dev/null"
$ "c:\buildbot-root\llvm-clang-x86_64-sie-win\build\bin\filecheck.exe" "--check-prefix=EXPORT" "C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\test\MachO\demangle.s"
$ ":" "RUN: at line 13"
$ "not" "ld64.lld" "-arch" "x86_64" "-platform_version" "macos" "11.0" "11.0" "-syslibroot" "C:/buildbot-root/llvm-clang-x86_64-sie-win/llvm-project/lld/test\MachO\Inputs\MacOSX.sdk" "-fatal_warnings" "-demangle" "-exported_symbol" "__ZTIN3foo3bar4MethE" "-exported_symbol" "__ZTSN3foo3bar4MethE" "C:\buildbot-root\llvm-clang-x86_64-sie-win\build\tools\lld\test\MachO\Output\demangle.s.tmp/export-symbols.o" "-o" "/dev/null"
$ "c:\buildbot-root\llvm-clang-x86_64-sie-win\build\bin\filecheck.exe" "--check-prefix=DEMANGLE-EXPORT" "C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\test\MachO\demangle.s"
# command stderr:
C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\test\MachO\demangle.s:22:20: error: DEMANGLE-EXPORT: expected string not found in input
# DEMANGLE-EXPORT: cannot export hidden symbol typeinfo for foo::bar::Meth
                   ^
<stdin>:4:78: note: scanning from here
ld64.lld: error: cannot export hidden symbol typeinfo name for foo::bar::Meth
                                                                             ^
<stdin>:5:101: note: possible intended match here
>>> defined in C:\buildbot-root\llvm-clang-x86_64-sie-win\build\tools\lld\test\MachO\Output\demangle.s.tmp/export-symbols.o
                                                                                                    ^

Input file: <stdin>
Check file: C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\test\MachO\demangle.s

-dump-input=help explains the following input dump.

Oh I see - the test is bad because it was expecting the error messages to be in a particular order. (it shouldn't because the export symbol list is processed concurrentlly)

thakis added a subscriber: thakis.May 16 2022, 6:32 PM

Linker behavior, including error output order, should be deterministic. (This allows caching results etc)

So it's not the test that's bad, it's the code.

I'd advise reverting and fixing async.

oontvoo added a comment.EditedMay 16 2022, 10:15 PM

Linker behavior, including error output order, should be deterministic. (This allows caching results etc)

I agree.

So it's not the test that's bad, it's the code.

I'd advise reverting and fixing async.

However the indeterminism isn't introduced by this patch. It's been there already (that is, the code that checks the list of exported symbols runs in parallel)
will look into how to fix the indeterminism there.

All this patch does is to change how the symbol names get printed, not when.