Bug 1526443 - Pick up LLVM fix for CFG on arm64-windows builds r=froydnj
☠☠ backed out by 8fe96c8d21b7 ☠ ☠
authorDavid Major <dmajor@mozilla.com>
Mon, 13 May 2019 18:38:23 +0000
changeset 474528 98013639d60026eb3a0344fa499a321aec176d2b
parent 474527 e8ac4b512f9d03dae670be39b399ea8189e1ca05
child 474529 50ef8f8c65587d511f6671d7d41bd6788bffeb0e
push id36042
push userdvarga@mozilla.com
push dateTue, 21 May 2019 04:19:40 +0000
treeherdermozilla-central@ca560ff55451 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1526443, 39799
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1526443 - Pick up LLVM fix for CFG on arm64-windows builds r=froydnj Cherry-picks https://bugs.llvm.org/show_bug.cgi?id=39799 and enables CFG on aarch64-windows automation. It's keyed on an explicit --enable-hardening to avoid breaking the local builds of developers who haven't yet picked up the compiler patch. Differential Revision: https://phabricator.services.mozilla.com/D28236
browser/config/mozconfigs/win64-aarch64/common-win64
build/build-clang/clang-win64.json
build/build-clang/r355141-arm64-cfg.patch
build/moz.configure/toolchain.configure
--- a/browser/config/mozconfigs/win64-aarch64/common-win64
+++ b/browser/config/mozconfigs/win64-aarch64/common-win64
@@ -1,3 +1,7 @@
 # This file is used by all AArch64 Win64 builds
 
 ac_add_options --target=aarch64-windows-mingw32
+
+# Temporary signal to toolchain.configure that our compiler is patched to
+# support CFG, until such support can be assumed.
+ac_add_options --enable-hardening
--- a/build/build-clang/clang-win64.json
+++ b/build/build-clang/clang-win64.json
@@ -12,12 +12,13 @@
     "python_path": "c:/mozilla-build/python/python.exe",
     "cc": "cl.exe",
     "cxx": "cl.exe",
     "ml": "ml64.exe",
     "patches": [
       "workaround-issue38586.patch",
       "unpoison-thread-stacks.patch",
       "downgrade-mangling-error.patch",
+      "r355141-arm64-cfg.patch",
       "loosen-msvc-detection.patch",
       "revert-r355311.patch"
     ]
 }
new file mode 100644
--- /dev/null
+++ b/build/build-clang/r355141-arm64-cfg.patch
@@ -0,0 +1,112 @@
+[COFF] Add address-taken import thunks to the fid table
+
+https://bugs.llvm.org/show_bug.cgi?id=39799
+https://reviews.llvm.org/D58739
+
+--- a/lld/COFF/Writer.cpp
++++ b/lld/COFF/Writer.cpp
+@@ -1390,19 +1390,47 @@
+ // symbol in an executable section.
+ static void maybeAddAddressTakenFunction(SymbolRVASet &AddressTakenSyms,
+                                          Symbol *S) {
+-  auto *D = dyn_cast_or_null<DefinedCOFF>(S);
+-
+-  // Ignore undefined symbols and references to non-functions (e.g. globals and
+-  // labels).
+-  if (!D ||
+-      D->getCOFFSymbol().getComplexType() != COFF::IMAGE_SYM_DTYPE_FUNCTION)
++  if (!S)
+     return;
+ 
+-  // Mark the symbol as address taken if it's in an executable section.
+-  Chunk *RefChunk = D->getChunk();
+-  OutputSection *OS = RefChunk ? RefChunk->getOutputSection() : nullptr;
+-  if (OS && OS->Header.Characteristics & IMAGE_SCN_MEM_EXECUTE)
+-    addSymbolToRVASet(AddressTakenSyms, D);
++  switch (S->kind()) {
++  case Symbol::DefinedLocalImportKind:
++  case Symbol::DefinedImportDataKind:
++    // Defines an __imp_ pointer, so it is data, so it is ignored.
++    break;
++  case Symbol::DefinedCommonKind:
++    // Common is always data, so it is ignored.
++    break;
++  case Symbol::DefinedAbsoluteKind:
++  case Symbol::DefinedSyntheticKind:
++    // Absolute is never code, synthetic generally isn't and usually isn't
++    // determinable.
++    break;
++  case Symbol::LazyKind:
++  case Symbol::UndefinedKind:
++    // Undefined symbols resolve to zero, so they don't have an RVA. Lazy
++    // symbols shouldn't have relocations.
++    break;
++
++  case Symbol::DefinedImportThunkKind:
++    // Thunks are always code, include them.
++    addSymbolToRVASet(AddressTakenSyms, cast<Defined>(S));
++    break;
++
++  case Symbol::DefinedRegularKind: {
++    // This is a regular, defined, symbol from a COFF file. Mark the symbol as
++    // address taken if the symbol type is function and it's in an executable
++    // section.
++    auto *D = cast<DefinedRegular>(S);
++    if (D->getCOFFSymbol().getComplexType() == COFF::IMAGE_SYM_DTYPE_FUNCTION) {
++      Chunk *RefChunk = D->getChunk();
++      OutputSection *OS = RefChunk ? RefChunk->getOutputSection() : nullptr;
++      if (OS && OS->Header.Characteristics & IMAGE_SCN_MEM_EXECUTE)
++        addSymbolToRVASet(AddressTakenSyms, D);
++    }
++    break;
++  }
++  }
+ }
+ 
+ // Visit all relocations from all section contributions of this object file and
+--- a/lld/test/COFF/guardcf-thunk.s
++++ b/lld/test/COFF/guardcf-thunk.s
+@@ -0,0 +1,43 @@
++# REQUIRES: x86
++
++# Make a DLL that exports exportfn1.
++# RUN: yaml2obj < %p/Inputs/export.yaml > %t.obj
++# RUN: lld-link /out:%t.dll /dll %t.obj /export:exportfn1 /implib:%t.lib
++
++# Make an obj that takes the address of that exported function.
++# RUN: llvm-mc -filetype=obj -triple=x86_64-windows-msvc %s -o %t2.obj
++# RUN: lld-link -entry:main -guard:cf %t2.obj %t.lib -nodefaultlib -out:%t.exe
++# RUN: llvm-readobj -coff-load-config %t.exe | FileCheck %s
++
++# Check that the gfids table contains *exactly* two entries, one for exportfn1
++# and one for main.
++# CHECK: GuardFidTable [
++# CHECK-NEXT: 0x{{[0-9A-Fa-f]+0$}}
++# CHECK-NEXT: 0x{{[0-9A-Fa-f]+0$}}
++# CHECK-NEXT: ]
++
++
++        .def     @feat.00;
++        .scl    3;
++        .type   0;
++        .endef
++        .globl  @feat.00
++@feat.00 = 0x001
++
++        .section .text,"rx"
++        .def     main; .scl    2; .type   32; .endef
++        .global main
++main:
++        leaq exportfn1(%rip), %rax
++        retq
++
++        .section .rdata,"dr"
++.globl _load_config_used
++_load_config_used:
++        .long 256
++        .fill 124, 1, 0
++        .quad __guard_fids_table
++        .quad __guard_fids_count
++        .long __guard_flags
++        .fill 128, 1, 0
++
--- a/build/moz.configure/toolchain.configure
+++ b/build/moz.configure/toolchain.configure
@@ -1661,19 +1661,20 @@ def security_hardening_cflags(hardening_
         # ASLR ------------------------------------------------
         # ASLR (dynamicbase) is enabled by default in clang-cl; but the
         # mingw-clang build requires it to be explicitly enabled
         if target.os == 'WINNT' and c_compiler.type == 'clang':
             ldflags.append("-Wl,--dynamicbase")
             js_ldflags.append("-Wl,--dynamicbase")
 
         # Control Flow Guard (CFG) ----------------------------
-        # See Bug 1525588 for why this doesn't work on Windows ARM
+        # On aarch64, this is enabled only with explicit --enable-hardening
+        # (roughly: automation) due to a dependency on a patched clang-cl.
         if c_compiler.type == 'clang-cl' and c_compiler.version >= '8' and \
-           target.cpu != 'aarch64':
+           (target.cpu != 'aarch64' or hardening_flag):
             flags.append("-guard:cf")
             js_flags.append("-guard:cf")
             # nolongjmp is needed because clang doesn't emit the CFG tables of
             # setjmp return addresses https://bugs.llvm.org/show_bug.cgi?id=40057
             ldflags.append("-guard:cf,nolongjmp")
             js_ldflags.append("-guard:cf,nolongjmp")
 
     # ----------------------------------------------------------