Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

binaryen fails to build from source with LTO (link time optimization) enabled #5946

Open
apoleon opened this issue Sep 15, 2023 · 4 comments

Comments

@apoleon
Copy link
Contributor

apoleon commented Sep 15, 2023

Hi

I'm hereby forwarding Debian bug #1015358

Version: 116
OS: Debian unstable
Compiler: GCC 13

Debian wants to enable link time optimization in the near future but binaryen 116 fails to build from source when LTO is enabled.

The error seems to be caused by third party code in third_party/llvm-project

third_party/llvm-project/./third_party/llvm-project/include/llvm/ADT/edit_distance.h: In function ‘ComputeEditDistance’:
third_party/llvm-project/./third_party/llvm-project/include/llvm/ADT/edit_distance.h:96:12: error: ‘SmallBuffer’ may be used uninitialized [-Werror=maybe-uninitialized]
third_party/llvm-project/./third_party/llvm-project/include/llvm/ADT/edit_distance.h:61:12: note: ‘SmallBuffer’ declared here
lto1: all warnings being treated as errors
make[4]: *** [/tmp/ccrE0idZ.mk:365: /tmp/ccqqy1Bf.ltrans121.ltrans.o] Error 1
make[4]: *** Waiting for unfinished jobs....
lto-wrapper: fatal error: make returned 2 exit status
compilation terminated.
/usr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status
@kripken
Copy link
Member

kripken commented Sep 18, 2023

Would it be possible to build there with -DBUILD_LLVM_DWARF=OFF? That would disable using this third-party code, which is only used for Binaryen's partial DWARF support (edit: because it's partial, I don't think many people actually depend on it. we don't build it into binaryen.js for example).

@apoleon
Copy link
Contributor Author

apoleon commented Sep 18, 2023

Hi,
then I get another error because Allocator.h is part of the disabled llvm-project directory

In file included from /<<PKGBUILDDIR>>/src/support/suffix_tree.cpp:16:
/<<PKGBUILDDIR>>/src/support/suffix_tree.h:38:10: fatal error: llvm/Support/Allocator.h: No such file or directory
   38 | #include "llvm/Support/Allocator.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~

@kripken
Copy link
Member

kripken commented Sep 18, 2023

Hmm, looks related to @ashleynh's work on the suffix tree. Building just that without LLVM errors it seems. Looks like we don't have a flag to disable the suffix tree, and instead we manually disable it in binaryen.js (emscripten) builds, which do not build LLVM:

# The below condition is intended for removal once the suffix_tree and
# suffix_tree_node source files no longer depend on LLVM code in the
# third_party folder
if(EMSCRIPTEN)
add_library(support OBJECT ${support_SOURCES})
else()
set(support_with_suffix_tree_SOURCES
suffix_tree.cpp
suffix_tree_node.cpp
${support_SOURCES}
)
add_library(support OBJECT ${support_with_suffix_tree_SOURCES})
endif()

Perhaps we should add such a flag? Another option might be to find a workaround in that source file. I'm not sure how to do that, though, since LTO shouldn't affect compile-time errors... so that error is odd. But @apoleon , does this patch fix things for you?

diff --git a/third_party/llvm-project/include/llvm/ADT/edit_distance.h b/third_party/llvm-project/include/llvm/ADT/edit_distance.h
index 4f5134008..784547108 100644
--- a/third_party/llvm-project/include/llvm/ADT/edit_distance.h
+++ b/third_party/llvm-project/include/llvm/ADT/edit_distance.h
@@ -19,6 +19,11 @@
 #include <algorithm>
 #include <memory>
 
+// XXX BINARYEN avoid a compiler warning here
+//     https://github.com/WebAssembly/binaryen/issues/5946
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+
 namespace llvm {
 
 /// Determine the edit distance between two sequences.
@@ -99,4 +104,7 @@ unsigned ComputeEditDistance(ArrayRef<T> FromArray, ArrayRef<T> ToArray,
 
 } // End llvm namespace
 
+// XXX BINARYEN
+#pragma GCC diagnostic pop
+
 #endif

If it works then it might be the quickest fix here.

@apoleon
Copy link
Contributor Author

apoleon commented Sep 18, 2023

The patch didn't make a difference for me. I still get the same error. But when I set the ENABLE_WERROR flag in CMakeLists to OFF instead, it works. Maybe there is a better solution but as a quick workaround it already helps.

kripken added a commit that referenced this issue Oct 28, 2024
Similar to

* #6330
* #6311
* #6912
* #5946

This extends the region we ignore the gcc warning in.

The warning:

ninja: job failed: /usr/bin/c++  -I/src/src -I/src/third_party/FP16/include -I/src/third_party/llvm-project/include -I/src -static -DBUILD_LLVM_DWARF -Wall -Werror -Wextra -Wno-unused-parameter -Wno-dangling-pointer -fno-omit-frame-pointer -fno-rtti -Wno-implicit-int-float-conversion -Wno-unknown-warning-option -Wswitch -Wimplicit-fallthrough -Wnon-virtual-dtor -fPIC -fdiagnostics-color=always -O3 -DNDEBUG -UNDEBUG -std=c++17 -MD -MT src/passes/CMakeFiles/passes.dir/Precompute.cpp.o -MF src/passes/CMakeFiles/passes.dir/Precompute.cpp.o.d -o src/passes/CMakeFiles/passes.dir/Precompute.cpp.o -c /src/src/passes/Precompute.cpp
In file included from /src/src/literal.h:27,
                 from /src/src/wasm.h:36,
                 from /src/src/ir/boolean.h:20,
                 from /src/src/ir/bits.h:20,
                 from /src/src/ir/properties.h:20,
                 from /src/src/ir/iteration.h:20,
                 from /src/src/passes/Precompute.cpp:30:
In copy constructor 'wasm::SmallVector<T, N>::SmallVector(const wasm::SmallVector<T, N>&) [with T = wasm::Expression*; long unsigned int N = 10]',
    inlined from 'constexpr std::pair<_T1, _T2>::pair(const _T1&, const _T2&) [with _U1 = wasm::Select* const; _U2 = wasm::SmallVector<wasm::Expression*, 10>; typename std::enable_if<(std::_PCC<true, _T1, _T2>::_ConstructiblePair<_U1, _U2>() && std::_PCC<true, _T1, _T2>::_ImplicitlyConvertiblePair<_U1, _U2>()), bool>::type <anonymous> = true; _T1 = wasm::Select* const; _T2 = wasm::SmallVector<wasm::Expression*, 10>]' at /usr/include/c++/13.2.1/bits/stl_pair.h:559:21,
    inlined from 'T& wasm::InsertOrderedMap<Key, T>::operator[](const Key&) [with Key = wasm::Select*; T = wasm::SmallVector<wasm::Expression*, 10>]' at /src/src/support/insert_ordered.h:112:29,
    inlined from 'void wasm::Precompute::partiallyPrecompute(wasm::Function*)::StackFinder::visitSelect(wasm::Select*)' at /src/src/passes/Precompute.cpp:571:24,
    inlined from 'static void wasm::Walker<SubType, VisitorType>::doVisitSelect(SubType*, wasm::Expression**) [with SubType = wasm::Precompute::partiallyPrecompute(wasm::Function*)::StackFinder; VisitorType = wasm::Visitor<wasm::Precompute::partiallyPrecompute(wasm::Function*)::StackFinder, void>]' at /src/src/wasm-delegations.def:50:1:
/src/src/support/small_vector.h:69:35: error: '<unnamed>.wasm::SmallVector<wasm::Expression*, 10>::fixed' may be used uninitialized [-Werror=maybe-uninitialized]
   69 |     : usedFixed(other.usedFixed), fixed(other.fixed), flexible(other.flexible) {
      |                                   ^~~~~~~~~~~~~~~~~~
In file included from /src/src/passes/Precompute.cpp:37:
/src/src/support/insert_ordered.h: In static member function 'static void wasm::Walker<SubType, VisitorType>::doVisitSelect(SubType*, wasm::Expression**) [with SubType = wasm::Precompute::partiallyPrecompute(wasm::Function*)::StackFinder; VisitorType = wasm::Visitor<wasm::Precompute::partiallyPrecompute(wasm::Function*)::StackFinder, void>]':
/src/src/support/insert_ordered.h:112:29: note: '<anonymous>' declared here
  112 |     std::pair<const Key, T> kv = {k, {}};
      |                             ^~
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants