Build mingw libc++ with debug information

RESOLVED FIXED in Firefox -esr60

Status

enhancement
P5
normal
RESOLVED FIXED
10 months ago
4 months ago

People

(Reporter: tjr, Assigned: tjr)

Tracking

Trunk
mozilla65
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 fixed, firefox65 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Patch to add debug information to the static c++ lib for mingw.
Assignee: nobody → tom
Attachment #9018320 - Flags: review?(core-build-config-reviews)
Try Run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=613caaa9dd22d522290de6e19d49d4110da7a08c

:dmajor, would you be able to confirm that this did what you expected?
Flags: needinfo?(dmajor)
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.
Attachment #9018320 - Flags: review?(core-build-config-reviews) → review+
(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
Flags: needinfo?(dmajor)
Carrying over r+ from Comment 3
Attachment #9018320 - Attachment is obsolete: true
Attachment #9019484 - Flags: review+
Attachment #9019484 - Attachment is patch: true
Keywords: checkin-needed
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
Flags: needinfo?(tom)
Weird... seems to work for me. I'll wait a day and then re-apply, rebase if needed.
Flags: needinfo?(tom)
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0efa0731412
Build the mingw static libc++ with debug information r=froydnj
Keywords: checkin-needed
The problem was difference in line endings but with help from Aryx I managed to apply the patch cleanly.
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.
Attachment #9019484 - Attachment is obsolete: true
Flags: needinfo?(tom)
Attachment #9019574 - Flags: review+
Keywords: checkin-needed
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/52d8c77e0718
Build the mingw static libc++ with debug information r=froydnj
Keywords: checkin-needed
I cannot explain that either. This time it applied without problems. And the time before I tried several times. Anyway thanks for fixing this.
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.
Flags: needinfo?(tom)
https://hg.mozilla.org/mozilla-central/rev/52d8c77e0718
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Flags: needinfo?(tom)
Can we take this into esr-60? It's pretty simple, and can be applied (as of today) without any rebasing/dependencies.
Flags: needinfo?(mozilla)
Flags: needinfo?(mozilla)
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.