This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Avoid using bump-alloc in TrieBuider
ClosedPublic

Authored by oontvoo on Mar 14 2022, 2:01 PM.

Details

Reviewers
int3
Group Reviewers
Restricted Project
Commits
rGe049a87f04cf: [lld-macho] Avoid using bump-alloc in TrieBuider
Summary

The code can be used in multi-threads and the allocator is not thread safe.

fixes PR/54378

Diff Detail

Event Timeline

oontvoo created this revision.Mar 14 2022, 2:01 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 14 2022, 2:01 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
oontvoo requested review of this revision.Mar 14 2022, 2:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2022, 2:01 PM
int3 accepted this revision.Mar 14 2022, 2:09 PM
int3 added inline comments.
lld/MachO/ExportTrie.cpp
151–152

I don't think these calls are necessary; clear() will automatically happen when the std::vector destructors get called

153

nit: newline between functions

lld/MachO/ExportTrie.h
25

do we need this?

This revision is now accepted and ready to land.Mar 14 2022, 2:09 PM
oontvoo updated this revision to Diff 415222.Mar 14 2022, 2:13 PM
oontvoo marked 2 inline comments as done.

review comment

int3 added inline comments.Mar 14 2022, 2:17 PM
lld/MachO/ExportTrie.h
25

sorry, I misread this as a ctor instead of a dtor. But I think explicit only applies to ctors, right?

oontvoo updated this revision to Diff 415223.Mar 14 2022, 2:18 PM

removed explicit on dtor

This revision was landed with ongoing or failed builds.Mar 14 2022, 2:23 PM
This revision was automatically updated to reflect the committed changes.

This change breaks the lld build with:

ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ac6467df252ff017b0d6ba076db369db/external/llvm-project/lld/BUILD.bazel:191:11: Compiling lld/MachO/ExportTrie.cpp failed: (Exit 1): clang failed: error executing command /usr/local/bin/clang -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -fcolor-diagnostics -fno-omit-frame-pointer '-std=c++0x' -MD -MF ... (remaining 81 argument(s) skipped)
external/llvm-project/lld/MachO/ExportTrie.cpp:148:11: error: definition of implicitly declared destructor
TrieNode::~TrieNode() {
          ^
external/llvm-project/lld/MachO/ExportTrie.cpp:149:25: error: use of undeclared identifier 'nodes'; did you mean 'node'?
  for (TrieNode *node : nodes)
                        ^~~~~
                        node
external/llvm-project/lld/MachO/ExportTrie.cpp:149:18: note: 'node' declared here
  for (TrieNode *node : nodes)
                 ^
external/llvm-project/lld/MachO/ExportTrie.cpp:149:23: error: invalid range expression of type 'lld::macho::TrieNode *'; no viable 'begin' function available
  for (TrieNode *node : nodes)
                      ^ ~~~~~
3 errors generated.

Reverting shortly.

Or, if you prefer the cmake version:

[159/255] Building CXX object tools/lld/MachO/CMakeFiles/lldMachO.dir/ExportTrie.cpp.o
FAILED: tools/lld/MachO/CMakeFiles/lldMachO.dir/ExportTrie.cpp.o 
/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/usr/local/google/home/saugustine/llvm/build/tools/lld/MachO -I/usr/local/google/home/saugustine/llvm/llvm-project/lld/MachO -I/usr/local/google/home/saugustine/llvm/llvm-project/lld/include -I/usr/local/google/home/saugustine/llvm/build/tools/lld/include -I/usr/local/google/home/saugustine/llvm/build/include -I/usr/local/google/home/saugustine/llvm/llvm-project/llvm/include -I/usr/local/google/home/saugustine/llvm/llvm-project/llvm/../libunwind/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -g  -fno-exceptions -fno-rtti -std=c++17 -MD -MT tools/lld/MachO/CMakeFiles/lldMachO.dir/ExportTrie.cpp.o -MF tools/lld/MachO/CMakeFiles/lldMachO.dir/ExportTrie.cpp.o.d -o tools/lld/MachO/CMakeFiles/lldMachO.dir/ExportTrie.cpp.o -c /usr/local/google/home/saugustine/llvm/llvm-project/lld/MachO/ExportTrie.cpp
/usr/local/google/home/saugustine/llvm/llvm-project/lld/MachO/ExportTrie.cpp:148:11: error: definition of implicitly declared destructor
TrieNode::~TrieNode() {
          ^
/usr/local/google/home/saugustine/llvm/llvm-project/lld/MachO/ExportTrie.cpp:149:25: error: use of undeclared identifier 'nodes'; did you mean 'node'?
  for (TrieNode *node : nodes)
                        ^~~~~
                        node
/usr/local/google/home/saugustine/llvm/llvm-project/lld/MachO/ExportTrie.cpp:149:18: note: 'node' declared here
  for (TrieNode *node : nodes)
                 ^
/usr/local/google/home/saugustine/llvm/llvm-project/lld/MachO/ExportTrie.cpp:149:23: error: invalid range expression of type 'lld::macho::TrieNode *'; no viable 'begin' function available
  for (TrieNode *node : nodes)
                      ^ ~~~~~
3 errors generated.
codemzs added a subscriber: codemzs.EditedMar 14 2022, 3:19 PM

This change has broken windows builds:

FAILED: tools/lld/MachO/CMakeFiles/lldMachO.dir/ExportTrie.cpp.obj
C:\PROGRA2\MICROS1\2019\ENTERP1\VC\Tools\MSVC\14291.301\bin\Hostx64\x64\cl.exe /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -DSTDC_CONSTANT_MACROS -DSTDC_FORMAT_MACROS -DSTDC_LIMIT_MACROS -ID:\a_work\1\b\llvm\Debug\tools\lld\MachO -ID:\a_work\1\llvm-project\lld\MachO -ID:\a_work\1\llvm-project\lld\include -ID:\a_work\1\b\llvm\Debug\tools\lld\include -ID:\a_work\1\b\llvm\Debug\include -ID:\a_work\1\llvm-project\llvm\include -ID:\a_work\1\llvm-project\llvm..\libunwind\include /DWIN32 /D_WINDOWS /Zc:inline /Zc:cplusplus /Zc:strictStrings /Oi /Zc:rvalueCast /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /MDd /Zi /Ob0 /Od /RTC1 -wd4530 -wd4062 /EHs-c- /GR- -std:c++14 /showIncludes /Fotools\lld\MachO\CMakeFiles\lldMachO.dir\ExportTrie.cpp.obj /Fdtools\lld\MachO\CMakeFiles\lldMachO.dir\lldMachO.pdb /FS -c D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): error C2600: 'lld::macho::TrieNode::~TrieNode': cannot define a compiler-generated special member function (must be declared in the class first)
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(149): error C2065: 'nodes': undeclared identifier
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(149): error C2672: 'begin': no matching overloaded function found
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): error C2893: Failed to specialize function template 'unknown-type std::begin(_Container &)'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\xutility(1916): note: see declaration of 'std::begin'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): note: With the following template arguments:
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): note: '_Container=unknown-type'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): error C2784: 'const _Elem *std::begin(std::initializer_list<_Elem>) noexcept': could not deduce template argument for 'std::initializer_list<_Elem>' from 'unknown-type'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\initializer_list(57): note: see declaration of 'std::begin'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(149): error C2672: 'end': no matching overloaded function found
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): error C2893: Failed to specialize function template 'unknown-type std::end(_Container &)'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\xutility(1926): note: see declaration of 'std::end'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): note: With the following template arguments:
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): note: '_Container=unknown-type'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): error C2784: 'const _Elem *std::end(std::initializer_list<_Elem>) noexcept': could not deduce template argument for 'std::initializer_list<_Elem>' from 'unknown-type'
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\initializer_list(63): note: see declaration of 'std::end'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): error C3536: '$L0': cannot be used before it is initialized
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(150): error C3536: '$L0': cannot be used before it is initialized
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(149): error C2100: illegal indirection
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(148): note: This diagnostic occurred in the compiler generated function 'lld::macho::TrieNode::~TrieNode(void)'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(149): error C2440: 'initializing': cannot convert from 'int' to 'lld::macho::TrieNode *'
D:\a_work\1\llvm-project\lld\MachO\ExportTrie.cpp(149): note: Conversion from integral type to pointer type requires reinterpret_cast, C-style cast or function-style cast

Thanks for the revert! @saugustine