Revert "Reland "Reland "Add toolchains without PartitionAlloc-Everywhere for dump_syms et al"""
This reverts commit 5e6def5bd2aa9cdbf69608a8edc32b5de696c091.
Reason for revert: official/win-clang is still broken:
https://ci.chromium.org/ui/p/chrome/builders/official/win-clang/8651/overview
Original change's description:
> Reland "Reland "Add toolchains without PartitionAlloc-Everywhere for dump_syms et al""
>
> This is a reland of commit 818e126f4350095dd0a54566ded107f0a5065f6f
> Which was a reland of commit 38c00784bc98a5dc885b06f5a0e738386c5f7df7
>
> When PartitionAlloc is linked into an executable, it takes over the
> system allocator (malloc, new, etc), which is called PartitionAlloc-
> Everywhere (or PA-E). When this occurs in dump_syms, we see that PA
> hits OOM and causes dump_syms to crash while generating the symbols
> for chrome. It's not at all clear why PA hits OOM but the system
> allocator does not, it occurs during construction of a std::string (on
> my machine anyway when I am running it in gdb, maybe elsewhere on bots).
> This happens on all platforms that we run dump_syms on as part of the
> official build: on linux and on mac, building for at least linux,
> android, chromeos, and mac. See also crbug.com/345514993.
>
> So we want to build dump_syms and other breakpad executables in a way
> that uses the system allocator. To do that we need to disable the
> use_partition_alloc_as_malloc GN variable. As this variable is global,
> we need a separate toolchain in order to disable it.
>
> We introduce a new toolchain with the suffix `_with_system_allocator`
> that can be used for this purpose. Initially we intended to use the
> Rust host build tools toolchain for this purpose, however we require
> careful naming to avoid toolchain collisions. For instance if building
> on a Linux x64 machine with an Other x64 target, we can have two
> toolchains:
> - default_toolchain: //build/toolchain/other:clang_x64
> - host_toolchain: //build/toolchain/other:clang_x64
>
> While these have different labels, it is the name at the end that is
> used as their output directory (this is hardcoded in GN). But they
> avoid colliding because the default toolchain is not placed in a
> subdirectory and uses the `root_build_dir`. However when we add another
> toolchain with them, they both get subdirs and collide:
> - for target: //build/toolchain/other:clang_x64_with_system_allocator
> - for host: //build/toolchain/linux:clang_x64_with_system_allocator
>
> Now both toolchains try to write to the clang_x64_with_system_allocator
> directory which causes errors. To avoid this, we actually make two
> toolchains per toolchain, one with a `host_` tag inside it.
> - target: //build/toolchain/other:clang_x64_with_system_allocator
> - unused: //build/toolchain/linux:clang_x64_with_system_allocator
> - unused: //build/toolchain/other:clang_x64_host_with_system_allocator
> - host: //build/toolchain/linux:clang_x64_host_with_system_allocator
>
> Then, when building for the host we choose the `host_` variety, which
> is specified in the `host_system_allocator_toolchain variable`. And
> when building for default target, we choose the non-`host_` one, which
> is specified in the `default_system_allocator_toolchain` variable.
>
> More clever strategies that try to avoid creating the unused toolchains
> above do not seem possible. Inside the toolchain-creating template,
> it is not clear how to determine which toolchain is being created, as
> the get_label_info() function on `target_name` does not produce
> anything that matches exactly with the string in `default_toolchain` or
> `host_toolchain`. We also tried using the current_cpu and current_os,
> however the `toolchain_args.current_os` is not actually set correctly in
> the default toolchain when targeting ChromeOS. The current_os variable
> is "chromeos" but inside the toolchain_args, it is "linux". So we just
> make extra toolchains (which can't be used or they'd make build errors)
> and we don't refer to them from the `host_system_allocator_toolchain`
> and `default_system_allocator_toolchain` variables, which makes them
> effectively inaccessible.
>
> In the process we learnt many things about how the breakpad executables
> are built. When you build them for the default toolchain, such as by
> building `//third_party/breakpad:dump_syms`, it redirects to the *host*
> toolchain on many platforms, but not on all platforms. This ends up
> putting a binary that may not work on the target machine in the
> `root_build_dir` which is highly unusual, but it is required by our
> testing scripts/infra.
>
> The key insight added here is that the toolchain that it should be
> built with is the platform from where the tests on the target will be
> *launched*. On Android, iOS, and ChromeOS, the tests are launched from
> the host machine and that's where the breakpad executables are run. We
> encode this explicitly in the breakpad GN file.
>
> One additional exception is that the breakpad tools do not build on
> Windows ARM, so when building on Windows x64 for Windows ARM, while
> the tests are launched from the ARM machine, we target the host x64
> machine still. This relies on the ARM machine being able to run the
> x64 binaries through emulation.
>
> There's no change here in how the breakpad binaries are built, but it
> is now more explicitly encoded and documented. What did change is that
> since we use a separate toolchain for building these tools, we also
> turn off component build in them. This allows us to replace the use
> of symlinks with copying (or hardlinking) the binaries from the
> toolchain's root directory up to the root_build_dir. This enables
> support for building these tools in the default_toolchain on Windows,
> something which was not possible before.
>
> Additional fixes from the original CL:
>
> MSAN is disabled in the toolchain with the system allocator as we only
> support MSAN in the default toolchain. If another toolchain has MSAN
> enabled it will try to also generate the MSAN instrumented libraries
> in the default toolchain's directory and they collide. This is similar
> to the rust host build tools toolchain, but there we disable all
> sanitizers. For the system allocator toolchain, we disable MSAN but
> retain the ability to build these tools with ASAN or UBSAN if needed.
>
> Angle's GN generation is fixed by not setting the PA variable directly
> from the toolchain. We add a variable in toolchain.gni that is always
> present, and set that. Then in the PA gni files, we check for that
> variable before enabling PA-Everywhere (and BRP, etc).
>
> Devtools standalone overrides BUILDCONFIG.gn but was not re-defining
> the TESTONLY_AND_VISIBILITY variable, so this is done in
> https://chrome-internal-review.googlesource.com/c/devtools/devtools-internal/+/7412037
>
> iOS official internal builders are now using the path to the
> root_build_dir for its dump_syms exe path from
> https://chrome-internal-review.googlesource.com/c/chrome/ios_internal/+/7411376
> and it expects the executable to be for the host. A TODO is added
> in the breakpad BUILD.gn file regarding cross-compiling for a
> different mac machine architecture that will upload/launch tests to
> the iOS device.
>
> Mac and Windows internal official builders are fixed by having the
> symupload tool depended on and built for the default toolchain so that
> it's present in the root_build_dir, but making this binary always
> redirect through the host_system_allocator_toolchain. The symupload
> binary is only run on official builders, it's not part of test
> failure reporting like dump_syms.
>
> Clank orderfile generator had a GN error due to PA-E being off but
> BRP being enabled. This is resolved by the fix for Angle, by turning
> off all PA-E and BRP related stuff when the toolchain turns off PA-E.
> See https://crbug.com/347976629.
>
> Bug: 345514993, b/342251590, 347976629
> Change-Id: Id953eb91ee73486c8e4f7dfc2bffe12fedd8d629
> Cq-Include-Trybots: luci.chromium.try:linux-official,android-official,win-official,mac-official
> Cq-Include-Trybots: luci.chromium.try:linux_chromium_msan_rel_ng
> Cq-Include-Trybots: luci.chromium.try:linux_chromium_asan_rel_ng
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5645742
> Commit-Queue: danakj <danakj@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Owners-Override: danakj <danakj@chromium.org>
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1318726}
Bug: 345514993, b/342251590, 347976629
Change-Id: Idf770547eed6a319ea21b79518d3926bd48ac823
Cq-Include-Trybots: luci.chromium.try:linux-official,android-official,win-official,mac-official
Cq-Include-Trybots: luci.chromium.try:linux_chromium_msan_rel_ng
Cq-Include-Trybots: luci.chromium.try:linux_chromium_asan_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5655811
Commit-Queue: Will Yeager <wyeager@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Keybo Qian <keybo@google.com>
Reviewed-by: Junji Watanabe <jwata@google.com>
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/heads/main@{#1319570}
NOKEYCHECK=True
GitOrigin-RevId: c1b50eb6a5c07ad63f119e3f3db53d4b2414c685
4 files changed
