Moving Mac build to GN adds static initializer[s] / causes sizes issues |
|||||||
Issue descriptionAfter flipping the rest of the Mac bots to GN[1] (and subsequent compile fixes[2]), the sizes step on Mac clobber builder continues to run red with additional static initializers[2]. dpranke@ believe the issue is here[3], in third_party/angle, which causes a static initialize for the GN build (but not GYP). The Mac bots have been out of commission a lot today between compile failures and sizes issues, but are still (at this time) running GN. dpranke@ mentioned that we may need to roll third_party/angle to pick up a fix. The latency involved with this may encourage flipping Mac builder back to GYP. I know very little about all this stuff; just happened to be sheriff on this fun day! [1] https://codereview.chromium.org/2143863002 [2] https://build.chromium.org/p/chromium/builders/Mac/builds/17638 [3] https://cs.chromium.org/chromium/src/third_party/angle/src/common/mathutil.cpp?q=mathutil.cpp&sq=package:chromium&dr=C&l=34
,
Jul 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/944255248a81d03034cfaf5d12255557d64a6dea commit 944255248a81d03034cfaf5d12255557d64a6dea Author: dpranke <dpranke@chromium.org> Date: Thu Jul 14 01:18:26 2016 Flip the /p/chromium Mac builder (and trybot) back to GYP. Until we get the fix for the `sizes` regression rolled in, we should flip this builder back to GYP to keep it green. TBR=rsesek@chromium.org, thakis@chromium.org, danbeam@chromium.org NOTRY=true BUG= 628052 , 618468 Review-Url: https://codereview.chromium.org/2152633002 Cr-Commit-Position: refs/heads/master@{#405382} [modify] https://crrev.com/944255248a81d03034cfaf5d12255557d64a6dea/tools/mb/mb_config.pyl
,
Jul 14 2016
In addition to the initializer, the symbolization of the initializers is also broken: /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python /b/build/slave/Mac/build/src/infra/scripts/legacy/scripts/slave/chromium/sizes.py --json /tmp/tmpevJu0b error: unable to open executable '/b/build/slave/Mac/build/src/out/Release/Chromium Framework.framework.dSYM/Contents/Resources/DWARF/Chromium Framework' I guess the dsym is generated in the wrong place.
,
Jul 14 2016
Just to document how I found that it was mathutil.cpp in angle:
% otool -l out/bot/Chromium\ Framework.framework/Chromium\ Framework | grep -C5 mod
nreloc 0
flags 0x00000007
reserved1 3318 (index into indirect symbol table)
reserved2 0
Section
sectname __mod_init_func
segname __DATA
addr 0x0000000006cb37b0
size 0x0000000000000008
offset 113981360
align 2^3 (8)
% xxd -g64 -s 0x0000000006cb37b0 -l 0x0000000000000008 out/bot/Chromium\ Framework.framework/Chromium\ Framework
6cb37b0: 90888c0300000000 ........
% dwarfdump --lookup 0x00000000038c8890 ./out/bot/Chromium\ Framework.dSYM/Contents/Resources/DWARF/Chromium\ Framework
0x22137948: Compile Unit: length = 0x00003f24 version = 0x0002 abbr_offset = 0x00000000 addr_size = 0x08 (next CU at 0x2213b870)
0x22137953: TAG_compile_unit [612] *
AT_producer( "clang version 3.9.0 (trunk 274369)" )
AT_language( DW_LANG_C_plus_plus )
AT_name( "../../third_party/angle/src/common/mathutil.cpp" )
AT_stmt_list( 0x03faca2b )
AT_comp_dir( "/Volumes/Build/src/out/bot" )
AT_APPLE_optimized( 0x01 )
AT_low_pc( 0x00000000038c8890 )
AT_ranges( 0x0226fcb0
[0x00000000038c8890 - 0x00000000038c88a0)
End )
0x2213b838: TAG_subprogram [809] *
AT_low_pc( 0x00000000038c8890 )
AT_high_pc( 0x00000000038c88a0 )
AT_frame_base( rbp )
AT_MIPS_linkage_name( "_GLOBAL__sub_I_mathutil.cpp" )
AT_artificial( 0x01 )
AT_APPLE_optimized( 0x01 )
0x22137953: TAG_compile_unit [612] *
AT_producer( "clang version 3.9.0 (trunk 274369)" )
AT_language( DW_LANG_C_plus_plus )
AT_name( "../../third_party/angle/src/common/mathutil.cpp" )
AT_stmt_list( 0x03faca2b )
AT_comp_dir( "/Volumes/Build/src/out/bot" )
AT_APPLE_optimized( 0x01 )
AT_low_pc( 0x00000000038c8890 )
AT_ranges( 0x0226fcb0
[0x00000000038c8890 - 0x00000000038c88a0)
End )
Line table dir : '../../third_party/angle/src/common'
Line table file: 'mathutil.cpp' line 0, column 0 with start address 0x00000000038c8890
Looking up address: 0x00000000038c8890 in .debug_frame... not found.
,
Jul 14 2016
The bot runs tools/mac/dump-static-initializers.py, which also prints where the initializer is from iirc
,
Jul 14 2016
> I guess the dsym is generated in the wrong place. The "dsyms" on the chromium/Mac bot are fake dsyms. We decided that these are bad and don't produce them in GN. This poses an interesting problem for dump-static-initializers.py because the build products are stripped. GYP config: test_isolation_mode=noop component=static_library use_goma=1 mac_strip_release=1 GN config: enable_stripping = true is_component_build = false is_debug = false use_goma = true The difference is in GYP, mac_strip_release=1 and mac_want_real_dsyms=default. This causes mac_real_dsym=0, which then generates the fake dSYM by copying the unstripped executable into a dSYM bundle and then stripping the build output. In GN, the output is just stripped. I'm not sure how we want to handle the dump-static-initializers.py now. We can not strip the build products (much larger build sizes), generate dsyms (takes a long time), or rely on the unstripped shared library in the out/gn/obj directory.
,
Jul 14 2016
Maybe revisit the decision to not produce fake dsyms :-P
,
Jul 14 2016
Mark was pretty explicit about not wanting to produce them. I also don't think they're a good idea.
,
Jul 14 2016
If you come up with something else that allows the same functionality, that's fine with me too of course. (As a general point, I think it's a good idea to not change too many things at once. gyp->gn is already a big change, so I'd try to keep that as small in scope as possible, and then do other changes like eg getting rid of fake dsyms and whatnot later)
,
Jul 14 2016
Having proper symbols for the continuous builds would probably be helpful, and we do provide them for Windows. The problem is that dsymutil is rather slow, but given that this bot is always a clobber bot, it is already pretty slow. Comparing the build times on my machine: enable_dsyms = true ninja -C out/bot -j250 all 8711.03s user 6259.28s system 462% cpu 53:59.09 total enable_dsyms = false ninja -C out/bot -j250 all 1407.27s user 1266.77s system 112% cpu 39:42.65 total If we don't think it's advisable to turn on real dSYMs, then I can think of two alternatives to the fake dSYMs: - Save a copy of the binaries in an "unstripped" location (or with an ".unstripped" suffix). This is all that fake dSYMs do, but without putting it in a bogus wrapper. - Run nm on the product before stripping and write it to a file. That has enough information to print the name of the static initializer, which could be used by the sizes step.
,
Jul 14 2016
Either sounds fine; the .unstripped variant sounds a bit more flexible.
,
Jul 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/313d944745601ba9b371866760f0496670c1cc43 commit 313d944745601ba9b371866760f0496670c1cc43 Author: Dirk Pranke <dpranke@chromium.org> Date: Thu Jul 14 01:01:55 2016 Stop linking a static_initializer into the Mac chromium build. The :angle-image_util target was a source_set rather than a static_library(), causing us to link in loadimage.cpp, which pulled in mathutils.cpp, which contains a static initializer. Switching :angle_image_util to a static library (which is what GYP does) solved the problem. R=rsesek@chromium.org, cwallez@chromium.org BUG= 628052 Change-Id: I63387b3fc9d799c92f7c1b49a1c7c7435e70a951 Reviewed-on: https://chromium-review.googlesource.com/360228 Reviewed-by: Corentin Wallez <cwallez@chromium.org> Commit-Queue: Corentin Wallez <cwallez@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> [modify] https://crrev.com/313d944745601ba9b371866760f0496670c1cc43/BUILD.gn
,
Jul 15 2016
Triaging ouf ot -Infra; +Infra>CQ, just because I can't find any better specific component to assign this to.
,
Jul 15 2016
I think we can always save .unstripped when we strip. It doesn’t have to be tied to whether we’re making a .dSYM or not.
,
Jul 17 2016
Bouncing to rsesek@ for the symbols issue; as far as I know, the static initializer issue has been worked around (or fixed, depending on how you look at it) by switching to use a static_library for that target.
,
Jul 18 2016
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0db21f9f2bc7bee9c40c64c5ad192297d32a567c commit 0db21f9f2bc7bee9c40c64c5ad192297d32a567c Author: jmadill <jmadill@chromium.org> Date: Mon Jul 18 18:45:41 2016 Roll ANGLE 4c32feb..5bc93c4 https://chromium.googlesource.com/angle/angle.git/+log/4c32feb..5bc93c4 BUG= 621031 , 628052 , 612066 TBR=cwallez@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2158003002 Cr-Commit-Position: refs/heads/master@{#406049} [modify] https://crrev.com/0db21f9f2bc7bee9c40c64c5ad192297d32a567c/DEPS
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7fa0d497278e20eb8f38e02269f0e88d8cffa9c2 commit 7fa0d497278e20eb8f38e02269f0e88d8cffa9c2 Author: rsesek <rsesek@chromium.org> Date: Mon Jul 18 21:54:39 2016 [Mac/GN] Add a new linker_driver.py option to save unstripped linker output. This declares a new GN arg save_unstripped_output that can be enabled if enable_stripping=true. If true, then an unstripped copy of the linker output will be saved in the output directory with an ".unstripped" suffix. BUG= 628052 R=mark@chromium.org Review-Url: https://codereview.chromium.org/2157573002 Cr-Commit-Position: refs/heads/master@{#406110} [modify] https://crrev.com/7fa0d497278e20eb8f38e02269f0e88d8cffa9c2/build/config/mac/BUILD.gn [modify] https://crrev.com/7fa0d497278e20eb8f38e02269f0e88d8cffa9c2/build/config/mac/symbols.gni [modify] https://crrev.com/7fa0d497278e20eb8f38e02269f0e88d8cffa9c2/build/toolchain/mac/BUILD.gn [modify] https://crrev.com/7fa0d497278e20eb8f38e02269f0e88d8cffa9c2/build/toolchain/mac/linker_driver.py
,
Jul 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/03a19d56c6b11996704a6871d52d05bc6f252328 commit 03a19d56c6b11996704a6871d52d05bc6f252328 Author: Robert Sesek <rsesek@chromium.org> Date: Tue Jul 19 18:24:51 2016 Add tools/mac/show_mod_init_func.py to symbolize static initializers without a dSYM. BUG= 628052 R=mark@chromium.org Review URL: https://codereview.chromium.org/2161093002 . Cr-Commit-Position: refs/heads/master@{#406325} [modify] https://crrev.com/03a19d56c6b11996704a6871d52d05bc6f252328/tools/mac/dump-static-initializers.py [add] https://crrev.com/03a19d56c6b11996704a6871d52d05bc6f252328/tools/mac/show_mod_init_func.py
,
Jul 19 2016
,
Jul 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b53b935fb9febdbc52456d8643b02a39c83b737 commit 1b53b935fb9febdbc52456d8643b02a39c83b737 Author: dpranke <dpranke@chromium.org> Date: Tue Jul 19 19:16:40 2016 Downgrade Mac GN arg save_unstripped_output to just a variable. It's not clear that we really need the GN arg `save_unstripped_output` to be a user-configurable thing, so this change removes it from the declare_arg() block in the build file and instead becomes a shorthand for (enable_stripping && !enable_dsyms). R=rsesek@chromium.org BUG= 623685 , 628052 Review-Url: https://codereview.chromium.org/2163533002 Cr-Commit-Position: refs/heads/master@{#406338} [modify] https://crrev.com/1b53b935fb9febdbc52456d8643b02a39c83b737/build/config/mac/symbols.gni
,
Jul 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ca703764dc9a08fa26537ca96ca5cc0edb01df4 commit 7ca703764dc9a08fa26537ca96ca5cc0edb01df4 Author: rsesek <rsesek@chromium.org> Date: Fri Jul 22 00:03:55 2016 [Mac/GN] Update sizes.py to use unstripped build output for reporting static initializers. This uses the new tool in //tools/mac/show_mod_init_func.py to display the function names. BUG= 628052 R=mark@chromium.org,dpranke@chromium.org Review-Url: https://codereview.chromium.org/2163373008 Cr-Commit-Position: refs/heads/master@{#406999} [modify] https://crrev.com/7ca703764dc9a08fa26537ca96ca5cc0edb01df4/infra/scripts/legacy/scripts/slave/chromium/sizes.py
,
Jul 22 2016
,
Jul 29 2016
Thank you for making the sizes step work in gn builds! |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by dpranke@chromium.org
, Jul 14 2016