Eliminate unused Subzero source and functionality [179832693]

Internal Cleanup

Status Update

No update yet.

Description

ni...@google.com

created issue #1

Feb 10, 2021 03:24AM

third_party/subzero/src/ contains a fair amount of unused source files. We haven't needed them in years, so they can be removed. Many of them don't even build, because of dependencies on LLVM headers and source that we don't provide in third_party/llvm-subzero, which we'd also like to eliminate, not expand.

Other things like sandboxing and code randomization should also go. We want to use Project Bunker as a longer-term security solution which doesn't negatively impact performance instead.

Comments

ap...@google.com <ap...@google.com> #2Feb 19, 2021 06:30PM

Project: SwiftShader
Branch: master

commit 6649bd025e0106c372d624a736a6dfc7b8f37404
Author: Nicolas Capens <capn@google.com>
Date: Thu Feb 11 13:07:43 2021

Eliminate code randomization support from Subzero

SwiftShader has no use for this since shader execution with robustness
features enabled can not access memory outside of the graphics
resources. For Chromium it also runs in the GPU process, which isolates
it from browser-wide and even tab renderer data and code, and also has
its own sandboxing.

If we ever do need randomization to prevent attacks, and project Bunker
doesn't provide the needed site isolation, it should be implemented
either at the Reactor level or as separate transformation passes where
possible.

While previously this feature was already disabled, there might have
been inadvertent randomization which could explain our test time
variability. It may also improve code generation performance a bit to
not have this code around any more.

Bug:

b/179832693

Change-Id: If1ccb54981edb61f443dd5949c56b75bab07c7c2
Reviewed-on:

https://swiftshader-review.googlesource.com/c/SwiftShader/+/52808
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>

M src/Reactor/BUILD.gn
M third_party/subzero/CMakeLists.txt
M third_party/subzero/docs/DESIGN.rst
M third_party/subzero/src/IceCfg.cpp
M third_party/subzero/src/IceCfg.h
M third_party/subzero/src/IceCfgNode.cpp
M third_party/subzero/src/IceCfgNode.h
M third_party/subzero/src/IceClFlags.def
M third_party/subzero/src/IceDefs.h
M third_party/subzero/src/IceELFObjectWriter.cpp
M third_party/subzero/src/IceGlobalContext.cpp
M third_party/subzero/src/IceGlobalContext.h
M third_party/subzero/src/IceOperand.h
D third_party/subzero/src/IceRNG.cpp
D third_party/subzero/src/IceRNG.h
M third_party/subzero/src/IceRegAlloc.cpp
M third_party/subzero/src/IceRegAlloc.h
M third_party/subzero/src/IceTargetLowering.cpp
M third_party/subzero/src/IceTargetLowering.h
M third_party/subzero/src/IceTargetLoweringARM32.cpp
M third_party/subzero/src/IceTargetLoweringARM32.h
M third_party/subzero/src/IceTargetLoweringMIPS32.cpp
M third_party/subzero/src/IceTargetLoweringMIPS32.h
M third_party/subzero/src/IceTargetLoweringX8632Traits.h
M third_party/subzero/src/IceTargetLoweringX8664Traits.h
M third_party/subzero/src/IceTargetLoweringX86Base.h
M third_party/subzero/src/IceTargetLoweringX86BaseImpl.h
M third_party/subzero/src/IceTimerTree.def

https://swiftshader-review.googlesource.com/52808

ap...@google.com <ap...@google.com> #3Jul 7, 2021 07:18PM

Project: SwiftShader
Branch: master

commit 97e0693ec9a95ddda8af392fa61bb5e5d8a3df49
Author: Nicolas Capens <capn@google.com>
Date: Mon Jul 05 23:28:28 2021

Eliminate support for the PNaCl ABI

PNaCl uses the ILP32 data model, even on x86-64 (cf. the x32 ABI).
Reactor instead always uses LP64 on 64-bit.

The main technical difference is the ILP32 on x86-64 used the address-
size override prefix (0x67) for every instruction that accesses
memory, which makes it look at only the lower 32-bit part of base and
index registers. Note this only works in a sandbox-like environment,
where memory is allocated in the lower 4 GiB range, such as PNaCl.

Bug:

b/179832693

Change-Id: I068ddd12b1827e3babc49a06ddf26b932d2082c5
Reviewed-on:

https://swiftshader-review.googlesource.com/c/SwiftShader/+/55468
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>

M src/Reactor/SubzeroReactor.cpp
M third_party/subzero/src/IceAssemblerX86Base.h
M third_party/subzero/src/IceAssemblerX86BaseImpl.h
M third_party/subzero/src/IceClFlags.def
M third_party/subzero/src/IceDefs.h
M third_party/subzero/src/IceELFObjectWriter.cpp
M third_party/subzero/src/IceInstX8664.cpp
M third_party/subzero/src/IceTargetLoweringARM32.cpp
M third_party/subzero/src/IceTargetLoweringMIPS32.cpp
M third_party/subzero/src/IceTargetLoweringX8664.cpp
M third_party/subzero/src/IceTargetLoweringX8664.h
M third_party/subzero/src/IceTargetLoweringX86BaseImpl.h

https://swiftshader-review.googlesource.com/55468

ap...@google.com <ap...@google.com> #4Jul 7, 2021 07:19PM

Project: SwiftShader
Branch: master

commit 52cb3d1b89614a69f197a7b64568364e07d3976f
Author: Nicolas Capens <capn@google.com>
Date: Tue Jul 06 12:42:05 2021

Assume Microsoft ABI on Windows

The SUBZERO_USE_MICROSOFT_ABI macro definition was used to indicate that
we want to use the Microsoft x86-64 calling convention, instead of
the System V one which PNaCl assumes (even on Windows). Using the
standard _WIN64 macro instead makes us not require defining the custom
one as part of our build.

SUBZERO_USE_MICROSOFT_ABI was also being used to decide whether to emit
stack probes. For 32-bit Windows targets, use the _WIN32 macro instead.
Note that when _WIN64 is defined, _WIN32 is also defined, but to avoid
confusion we use _WIN64 in the X8664 backend.

Bug:

b/179832693

Change-Id: Ic2e62d3dc26543876673c07b9ccc01e9d92762bf
Reviewed-on:

https://swiftshader-review.googlesource.com/c/SwiftShader/+/55528
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>

M src/Reactor/BUILD.gn
M third_party/subzero/CMakeLists.txt
M third_party/subzero/src/IceInstX8664.def
M third_party/subzero/src/IceTargetLoweringX8632.cpp
M third_party/subzero/src/IceTargetLoweringX8664.cpp
M third_party/subzero/src/IceTargetLoweringX8664Traits.h
M third_party/subzero/src/IceTargetLoweringX86BaseImpl.h

https://swiftshader-review.googlesource.com/55528

ap...@google.com <ap...@google.com> #5Jul 7, 2021 09:08PM

Project: SwiftShader
Branch: master

commit 9534228df856f618086d8640fd67b2888f172c16
Author: Nicolas Capens <capn@google.com>
Date: Tue Jul 06 12:45:02 2021

Eliminate Subzero sandboxing support

Subzero supported sandboxing for the PNaCl platform. Reactor does not
support sandboxing at the JIT level, and we don't have a need for it
since Chromium provides sandboxing as part of the "GPU process". Thus
we can remove it and reduce code complexity.

Note that Subzero's sandboxing implementation comes at a performance
penalty. Project Bunker provides a better solution for SwiftShader,
which is probably also more secure in light of speculative execution
vulnerabilities.

If we ever do need sandboxing support in Reactor itself (e.g. outside of
SwiftShader, when process isolation is not feasible), it is best to
use an actively developed JIT-compiler where security always takes
priority over peformance, like Chromium's WebAssembly JIT.

Bug:

b/179832693

Change-Id: I7364d22183e123c5145caae9f546d3855012d73e
Reviewed-on:

https://swiftshader-review.googlesource.com/c/SwiftShader/+/55488
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Commit-Queue: Nicolas Capens <nicolascapens@google.com>

M third_party/subzero/docs/DESIGN.rst
M third_party/subzero/src/IceAssemblerARM32.cpp
M third_party/subzero/src/IceAssemblerARM32.h
M third_party/subzero/src/IceAssemblerX86BaseImpl.h
M third_party/subzero/src/IceCfg.cpp
M third_party/subzero/src/IceCfg.h
M third_party/subzero/src/IceCfgNode.cpp
M third_party/subzero/src/IceClFlags.def
M third_party/subzero/src/IceFixups.cpp
M third_party/subzero/src/IceInst.cpp
M third_party/subzero/src/IceInst.h
M third_party/subzero/src/IceInstARM32.cpp
M third_party/subzero/src/IceInstARM32.h
M third_party/subzero/src/IceInstX8632.cpp
M third_party/subzero/src/IceInstX8664.cpp
M third_party/subzero/src/IceInstX86Base.h
M third_party/subzero/src/IceInstX86BaseImpl.h
M third_party/subzero/src/IceIntrinsics.h
M third_party/subzero/src/IceTargetLowering.cpp
M third_party/subzero/src/IceTargetLowering.h
M third_party/subzero/src/IceTargetLoweringARM32.cpp
M third_party/subzero/src/IceTargetLoweringARM32.h
M third_party/subzero/src/IceTargetLoweringMIPS32.cpp
M third_party/subzero/src/IceTargetLoweringMIPS32.h
M third_party/subzero/src/IceTargetLoweringX8632.cpp
M third_party/subzero/src/IceTargetLoweringX8632.h
M third_party/subzero/src/IceTargetLoweringX8664.cpp
M third_party/subzero/src/IceTargetLoweringX8664.h
M third_party/subzero/src/IceTargetLoweringX8664Traits.h
M third_party/subzero/src/IceTargetLoweringX86Base.h
M third_party/subzero/src/IceTargetLoweringX86BaseImpl.h

https://swiftshader-review.googlesource.com/55488

ap...@google.com <ap...@google.com> #6Oct 5, 2022 07:47PM

Project: SwiftShader
Branch: master

commit 2b155acd6a90b51009c60d397969fbad7ca771b5
Author: Nicolas Capens <capn@google.com>
Date: Wed Oct 05 15:12:43 2022

Remove Unicode handling from Subzero

This functionality isn't used by SwiftShader. The source files are
licensed under a custom license which may complicate their use in
downstream projects. Note that upstream LLVM has a different license
for these source files already.

Bug:

b/179832693

Change-Id: I55142ff07e7c5c2c84c051a5818731b854b17155
Reviewed-on:

https://swiftshader-review.googlesource.com/c/SwiftShader/+/68728
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>

M third_party/llvm-subzero/include/llvm/Support/Program.h
M third_party/llvm-subzero/lib/Support/Unix/Program.inc
M third_party/llvm-subzero/CMakeLists.txt
M src/Reactor/BUILD.gn
M third_party/llvm-subzero/Android.bp
D third_party/llvm-subzero/lib/Support/ConvertUTF.cpp
M third_party/llvm-subzero/lib/Support/Windows/Program.inc
M third_party/llvm-subzero/lib/Support/CommandLine.cpp
D third_party/llvm-subzero/include/llvm/Support/ConvertUTF.h
D third_party/llvm-subzero/lib/Support/ConvertUTFWrapper.cpp

https://swiftshader-review.googlesource.com/68728

is...@google.com <is...@google.com> Jan 25, 2023 03:56PM

Status: New

Issue 179832693

Description

Issue summary

Comments

ap...@google.com <ap...@google.com> #2Feb 19, 2021 06:30PM

ap...@google.com <ap...@google.com> #3Jul 7, 2021 07:18PM

ap...@google.com <ap...@google.com> #4Jul 7, 2021 07:19PM

ap...@google.com <ap...@google.com> #5Jul 7, 2021 09:08PM

ap...@google.com <ap...@google.com> #6Oct 5, 2022 07:47PM

is...@google.com <is...@google.com> Jan 25, 2023 03:56PM

Add comment

Issue metadata