1500102 - Build mingw libc++ with debug information

Status: RESOLVED FIXED

(Firefox Build System :: General: Unsupported Platforms, enhancement, P5)

Description

[Tom Ritter [:tjr] (OOTO Oct 18-26)]

From Bug 1497645 Comment 9

> (In reply to David Major [:dmajor] from comment #8)
> > Interestingly, the push in bug 1475562 comment 20, with `-g`, fixes most of
> > the symbol names, but not the stdlib ones.
> 
> We use static libc++, so it ends up linked into xul.dll, but it's built without
> debugging information. I guess we could change toolchain build script to build 
> libc++ with debug info.

Comment 1

[Tom Ritter [:tjr] (OOTO Oct 18-26)]

Attachment: Bug 1500102 Build the mingw static libc++ with debug information

Patch to add debug information to the static c++ lib for mingw.

Comment 2

[Tom Ritter [:tjr] (OOTO Oct 18-26)]

Try Run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=613caaa9dd22d522290de6e19d49d4110da7a08c

:dmajor, would you be able to confirm that this did what you expected?

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 9018320 [details] [diff] [review]
Bug 1500102 Build the mingw static libc++ with debug information

Review of attachment 9018320 [details] [diff] [review]:
-----------------------------------------------------------------

::: taskcluster/scripts/misc/build-clang-trunk-mingw.sh
@@ +170,5 @@
>  build_libcxx() {
> +  # Below, we specify -g -gcodeview to build static libraries with debug information.
> +  # Because we're not distributing these builds, this is fine. If one were to distribute
> +  # the builds, perhaps one would want to make those flags conditional or investigation
> +  # other options.

This feels like the sort of thing that's easy to forget about if we *did* start shipping the builds...but OK.  I guess these builds are really there to try to ensure we don't break Tor badly, and they're the ones shipping the builds.

@@ +192,5 @@
>        -DLIBUNWIND_USE_COMPILER_RT=TRUE \
>        -DLIBUNWIND_ENABLE_THREADS=TRUE \
>        -DLIBUNWIND_ENABLE_SHARED=FALSE \
>        -DLIBUNWIND_ENABLE_CROSS_UNWINDING=FALSE \
> +      -DCMAKE_CXX_FLAGS="-g -gcodeview -nostdinc++ -I$SRC_DIR/libcxx/include -DPSAPI_VERSION=2" \

I have a small preference for putting:

let debug_flags="-g -gcodeview"

at the top of this function, and then:

  -DCMAKE_CXX_FLAGS="${debug_flags} ..."

everywhere, but do as you like.

Comment 4

[(Away)]

(In reply to Tom Ritter [:tjr] from comment #2)
> Try Run:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=613caaa9dd22d522290de6e19d49d4110da7a08c
> 
> :dmajor, would you be able to confirm that this did what you expected?

Looks great, thanks!

00 xul!std::__1::__num_put<char>::__widen_and_group_float
01 xul!std::__1::num_put<char,std::__1::ostreambuf_iterator<char,std::__1::char_traits<char> > >::do_put
02 xul!std::__1::basic_ostream<char,std::__1::char_traits<char> >::operator<<
03 xul!mozilla::layers::CheckerboardEvent::LogInfo

Comment 5

[Tom Ritter [:tjr] (OOTO Oct 18-26)]

Attachment: Bug 1500102 - Build the mingw static libc++ with debug information r=froydnj

Carrying over r+ from Comment 3

Comment 6

[Cosmin Sabou [:CosminS]]

Hi! This patch failed to apply on m-i. Please take a look.

applying https://bug1500102.bmoattachments.org/attachment.cgi?id=9019484
patching file taskcluster/scripts/misc/build-clang-trunk-mingw.sh
Hunk #1 FAILED at 162
Hunk #2 FAILED at 182
Hunk #3 FAILED at 210
Hunk #4 FAILED at 243
4 out of 4 hunks FAILED -- saving rejects to file taskcluster/scripts/misc/build-clang-trunk-mingw.sh.rej
abort: patch failed to apply

Comment 7

[Tom Ritter [:tjr] (OOTO Oct 18-26)]

Weird... seems to work for me. I'll wait a day and then re-apply, rebase if needed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0efa0731412
Build the mingw static libc++ with debug information r=froydnj

Comment 9

[Cosmin Sabou [:CosminS]]

The problem was difference in line endings but with help from Aryx I managed to apply the patch cleanly.

Comment 10

[Cosmin Sabou [:CosminS]]

Backed out for clang build bustages.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=busted&fromchange=b0efa0731412c1fef077a02c3683a90a36352218&tochange=6d09c69fd7e699339628f3c398445318ffe59efc&selectedJob=207377146

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=207392000&repo=mozilla-inbound

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/6d09c69fd7e699339628f3c398445318ffe59efc

Comment 11

[Tom Ritter [:tjr] (OOTO Oct 18-26)]

Attachment: Bug 1500102 - Build the mingw static libc++ with debug information r=froydnj

The bustage was definitely not your fault; I had an error in my script.

The line endings I don't understand. I do the same thing every time to post my patches; and other bugs land fine (most recently Bug 1500803 and Bug 1500477).  I don't use Windows at any point.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/52d8c77e0718
Build the mingw static libc++ with debug information r=froydnj

Comment 13

[Cosmin Sabou [:CosminS]]

I cannot explain that either. This time it applied without problems. And the time before I tried several times. Anyway thanks for fixing this.

Comment 14

[Arthur Iakab [arthur_iakab]]

Tried to land this patch but received error.
This is the rej file:

--- build-clang-trunk-mingw.sh
+++ build-clang-trunk-mingw.sh
@@ -163,16 +163,22 @@ ADDLIB $2
 SAVE
 END
 EOF
   llvm-ranlib tmp.a
   mv tmp.a $1
 }
 
 build_libcxx() {
+  # Below, we specify -g -gcodeview to build static libraries with debug information.
+  # Because we're not distributing these builds, this is fine. If one were to distribute
+  # the builds, perhaps one would want to make those flags conditional or investigation
+  # other options.
+  DEBUG_FLAGS="-g -gcodeview"
+
   mkdir libunwind
   pushd libunwind
   cmake \
       -DCMAKE_BUILD_TYPE=Release \
       -DCMAKE_INSTALL_PREFIX=$CROSS_PREFIX_DIR \
       -DCMAKE_C_COMPILER=$CC \
       -DCMAKE_CXX_COMPILER=$CXX \
       -DCMAKE_CROSSCOMPILING=TRUE \
@@ -183,17 +189,17 @@ build_libcxx() {
       -DCMAKE_AR=$INSTALL_DIR/bin/llvm-ar \
       -DCMAKE_RANLIB=$INSTALL_DIR/bin/llvm-ranlib \
       -DLLVM_NO_OLD_LIBSTDCXX=TRUE \
       -DCXX_SUPPORTS_CXX11=TRUE \
       -DLIBUNWIND_USE_COMPILER_RT=TRUE \
       -DLIBUNWIND_ENABLE_THREADS=TRUE \
       -DLIBUNWIND_ENABLE_SHARED=FALSE \
       -DLIBUNWIND_ENABLE_CROSS_UNWINDING=FALSE \
-      -DCMAKE_CXX_FLAGS="-nostdinc++ -I$SRC_DIR/libcxx/include -DPSAPI_VERSION=2" \
+      -DCMAKE_CXX_FLAGS="${DEBUG_FLAGS} -nostdinc++ -I$SRC_DIR/libcxx/include -DPSAPI_VERSION=2" \
       $SRC_DIR/libunwind
   make $make_flags
   make $make_flags install
   popd
 
   mkdir libcxxabi
   pushd libcxxabi
   cmake \
@@ -211,17 +217,17 @@ build_libcxx() {
       -DLIBCXXABI_USE_COMPILER_RT=ON \
       -DLIBCXXABI_ENABLE_EXCEPTIONS=ON \
       -DLIBCXXABI_ENABLE_THREADS=ON \
       -DLIBCXXABI_TARGET_TRIPLE=$machine-w64-mingw32 \
       -DLIBCXXABI_ENABLE_SHARED=OFF \
       -DLIBCXXABI_LIBCXX_INCLUDES=$SRC_DIR/libcxx/include \
       -DLLVM_NO_OLD_LIBSTDCXX=TRUE \
       -DCXX_SUPPORTS_CXX11=TRUE \
-      -DCMAKE_CXX_FLAGS="-D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_HAS_THREAD_API_WIN32" \
+      -DCMAKE_CXX_FLAGS="${DEBUG_FLAGS} -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_HAS_THREAD_API_WIN32" \
       $SRC_DIR/libcxxabi
   make $make_flags VERBOSE=1
   popd
 
   mkdir libcxx
   pushd libcxx
   cmake \
       -DCMAKE_BUILD_TYPE=Release \
@@ -244,17 +250,17 @@ build_libcxx() {
       -DLIBCXX_SUPPORTS_STD_EQ_CXX11_FLAG=TRUE \
       -DLIBCXX_HAVE_CXX_ATOMICS_WITHOUT_LIB=TRUE \
       -DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY=OFF \
       -DLIBCXX_ENABLE_FILESYSTEM=OFF \
       -DLIBCXX_ENABLE_STATIC_ABI_LIBRARY=TRUE \
       -DLIBCXX_CXX_ABI=libcxxabi \
       -DLIBCXX_CXX_ABI_INCLUDE_PATHS=$SRC_DIR/libcxxabi/include \
       -DLIBCXX_CXX_ABI_LIBRARY_PATH=../libcxxabi/lib \
-      -DCMAKE_CXX_FLAGS="-D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS" \
+      -DCMAKE_CXX_FLAGS="${DEBUG_FLAGS} -D_LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS" \
       $SRC_DIR/libcxx
   make $make_flags VERBOSE=1
   make $make_flags install
 
   # libc++.a depends on libunwind.a. Whild linker will automatically link
   # to libc++.a in C++ mode, it won't pick libunwind.a, requiring caller
   # to explicitly pass -lunwind. Wo work around that, we merge libunwind.a
   # into libc++.a.

Comment 15

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/52d8c77e0718

Comment 16

[Tom Ritter [:tjr] (OOTO Oct 18-26)]

Can we take this into esr-60? It's pretty simple, and can be applied (as of today) without any rebasing/dependencies.

Comment 17

[Tom Prince [:tomprince]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/0a7d7f1a3b49