Bug 1523526 Comment 13 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.


Changes submitted for bug 1526443
    Email sent to 38 recipients. (show)

Bug 1526443
Figure out arm64 CFG issues
NEW Assigned to dmajor
(NeedInfo from dmajor
)
Get help with this page
Product:
Core ▾
Component:
Security ▾
Type:
task
Status:
NEW
Reported:
3 months ago
Modified:
just now
Assignee:
David Major [:dmajor]
Reporter:
David Major [:dmajor]
Triage Owner:
Wennie
NeedInfo From:
dmajor
just now
CC:
▸ 3 people including you
Version:
unspecified
Target:
---
Points:
---
Depends on:
1523526
Blocks:
1485016
Dependency tree / graph

This bug is not currently tracked.
Whiteboard:
---
Votes:
1 vote
ID 	Title 	Author 	Status 	Reviewers
D28236	Pick up LLVM fix for CFG on arm64-windows builds	dmajor	Accepted	
	froydnj
Pick up LLVM fix for CFG on arm64-windows builds
a month ago David Major [:dmajor]
47 bytes, text/x-phabricator-request
		Details | Review
Attach File
	David Major [:dmajor]
Assignee 	
Description • 3 months ago

Bug 1525588 disabled CFG on aarch64 because some xul functions aren't getting marked as valid call targets, so the browser fails to start at all.

We should still try to figure out what's going on and re-enable if possible.

rnk and pcc suggested stepping through markSymbolsWithRelocations in lld as a starting point.
	David Major [:dmajor]
Assignee 	
Comment 1 • 3 months ago

Specifically, the problematic function is: xul!std::sys::windows::thread_local::on_tls_callback

(Or at least that's the first one we fail on, there may be more)
	temp.stuff
New to Bugzilla 	
Comment 2 • 3 months ago

You might want to put your own GuardCFCheckFunctionPointer and GuardCFDispatchFunctionPointer functions in the load config directory and use these functions to build a list of indirect targets not found in the table.
	temp.stuff
New to Bugzilla 	
Comment 3 • 3 months ago

Or even better - simply compare the ARM64 and the AMD64 lists...
	David Major [:dmajor]
Assignee 	
Comment 4 • 3 months ago

(In reply to temp.stuff from comment #3)

    Or even better - simply compare the ARM64 and the AMD64 lists...

The trouble is a bunch of the entries in dumpbin -loadconfig don't resolve to an address, e.g.:

    Guard CF Function Table

          Address
          --------
[...snip...]
          000000018000ABB0  msgpack_zone_init
          000000018000AC10  msgpack_zone_new
          000000018000AC80  msgpack_zone_free
          000000018000B420
          000000018000CEE0
          000000018000D1E0

Do you know of another way to do the comparison?
	temp.stuff
New to Bugzilla 	
Comment 5 • 3 months ago

dumpbin doesn't load PDBs for the modules it dumps AFAIK and therefore doesn't know about most symbols. The few it did display are probably exported.

You should either use a WinDbg script to get symbol names for the dumpbin output, or use WinDbg to both get the table entries and the corresponding symbols, or use the DbgEng API from C++ in your own client. (Or some other similar tool.)

Here's an example of how to do it from WinDbg:

0:000> dt xul!IMAGE_LOAD_CONFIG_DIRECTORY64 GuardCFFunctionTable GuardCFFunctionCount
+0x080 GuardCFFunctionTable : Uint8B
+0x088 GuardCFFunctionCount : Uint8B

0:000> !dh -f xul

File Type: DLL
FILE HEADER VALUES
    8664 machine (X64)
    A number of sections
5C712F6F time date stamp Sat Feb 23 13:33:03 2019

    0 file pointer to symbol table
    0 number of symbols
    F0 size of optional header
    2022 characteristics
            Executable
            App can handle >2gb addresses
            DLL

OPTIONAL HEADER VALUES
    20B magic #
14.00 linker version
498B200 size of code
195B400 size of initialized data
    0 size of uninitialized data
4987AB0 address of entry point
    1000 base of code
        ----- new -----
0000000180000000 image base
    1000 section alignment
    200 file alignment
    2 subsystem (Windows GUI)
    6.01 operating system version
    0.00 image version
    6.01 subsystem version
63CB000 size of image
    400 size of headers
62F31FE checksum
0000000000100000 size of stack reserve
0000000000001000 size of stack commit
0000000000100000 size of heap reserve
0000000000001000 size of heap commit
    4160  DLL characteristics
            High entropy VA supported
            Dynamic base
            NX compatible
            Guard
545F647 [     B6B] address [size] of Export Directory
54601B2 [     398] address [size] of Import Directory
632A000 [    24D0] address [size] of Resource Directory
55B7000 [  204858] address [size] of Exception Directory
62E6C00 [    1DD0] address [size] of Security Directory
632D000 [   9D954] address [size] of Base Relocation Directory
54086A2 [      1C] address [size] of Debug Directory
      0 [       0] address [size] of Description Directory
      0 [       0] address [size] of Special Directory
51602C0 [      28] address [size] of Thread Storage Directory
4AEC160 [     100] address [size] of Load Configuration Directory
      0 [       0] address [size] of Bound Import Directory
5464608 [    40B8] address [size] of Import Address Table Directory
545EE58 [     160] address [size] of Delay Import Directory
      0 [       0] address [size] of COR20 Header Directory
      0 [       0] address [size] of Reserved Directory

0:000> dps xul+4AEC160+80 L2
00000001`84aec1e0  00000001`85408717 xul!__guard_fids_table
00000001`84aec1e8  00000000`000158e0

0:000> dd xul!__guard_fids_table L8
00000001`85408717  000029b0 00002ab0 00002b10 00002b80
00000001`85408727  00002bc0 000056e0 00006c80 00006cb0

0:000> .foreach /pS 1 /ps 1 (addr { dd /c1 xul!__guard_fids_table L8 } ) { .printf "%y\n", xul+${addr} }
xul!XRE_GetBootstrap (00000001`800029b0)
xul!mozilla::BootstrapImpl::NS_LogInit (00000001`80002ab0)
xul!mozilla::BootstrapImpl::XRE_StartupTimelineRecord (00000001`80002b10)
xul!mozilla::BootstrapImpl::XRE_EnableSameExecutableForContentProc (00000001`80002b80)
xul!mozilla::BootstrapImpl::XRE_main (00000001`80002bc0)
xul!std::_Func_impl_no_alloc<`lambda at z:/task_1550916350/build/src/xpcom/base/Logging.cpp:197:34',void,const nsTSubstring<char> &>::_Delete_this (00000001`800056e0)
xul!nsTHashtable<nsBaseHashtableET<nsCharPtrHashKey,nsAutoPtr<mozilla::LogModule> > >::s_HashKey (00000001`80006c80)
xul!nsTHashtable<nsBaseHashtableET<nsCharPtrHashKey,nsAutoPtr<mozilla::LogModule> > >::s_InitEntry (00000001`80006cb0)

You'd want to use 0x158e0 (or the correct size for another module) instead of 8 in the last command, obviously.
	David Major [:dmajor]
Assignee 	
Comment 6 • 3 months ago

(In reply to temp.stuff from comment #5)

    dumpbin doesn't load PDBs for the modules it dumps AFAIK and therefore
    doesn't know about most symbols. The few it did display are probably
    exported.

I see way more entries in dumpbin's list than there are exports, so there must be something going on, but in any case, thanks very much for the tip!
	David Major [:dmajor]
Assignee 	
Comment 7 • 3 months ago

It's a bit tricky because in general arm64 has way more entries than x86_64, but while looking in a diff viewer at only the lines that are missing on arm64, one thing stands out: a lot of Rust functions are getting left out.

$ grep -c std::io *
arm64.txt:0
x86_64.txt:91

$ grep -c style:: *
arm64.txt:1
x86_64.txt:1955

$ grep -c core:: *
arm64.txt:1
x86_64.txt:1308

$ grep -c webrender:: *
arm64.txt:0
x86_64.txt:665

$ grep -c cranelift *
arm64.txt:0
x86_64.txt:620

$ grep -c baldrdash:: *
arm64.txt:0
x86_64.txt:23

$ grep -c alloc:: *
arm64.txt:0
x86_64.txt:492

It's not 100% broken though: notice that 1 result for core and stylo.
core::ptr::write<style::values::generics::counters::CounterPair<i32>> is in the CFG table.
	David Major [:dmajor]
Assignee 	
Comment 8 • 3 months ago

On arm64, rustc was not marking these functions as IMAGE_SYM_DTYPE_FUNCTION, so we didn't get past the test in lld's maybeAddAddressTakenFunction.

This sounds exactly like https://reviews.llvm.org/D53540#1326473, which was fixed in LLVM r348875.

The fix ought to be in nightly rustc already, at least as far back as https://github.com/rust-lang/rust/commit/df0466d0bb807a7266cc8ac9931cd43b3e84b62e (16 January).
	temp.stuff
New to Bugzilla 	
Comment 9 • 3 months ago

    I see way more entries in dumpbin's list than there are exports, so there must be something going on, but in any case, thanks very much for the tip!

That's why I said probably exported. :) It just wasn't important enough to dig into given that a solution was available.

(In reply to David Major [:dmajor] from comment #8)

    On arm64, rustc was not marking these functions as IMAGE_SYM_DTYPE_FUNCTION, so we didn't get past the test in lld's maybeAddAddressTakenFunction.

    This sounds exactly like https://reviews.llvm.org/D53540#1326473, which was fixed in LLVM r348875.

    The fix ought to be in nightly rustc already, at least as far back as https://github.com/rust-lang/rust/commit/df0466d0bb807a7266cc8ac9931cd43b3e84b62e (16 January).

Great job. I'm looking forward to a new nightly of Firefox using a new version of Rust with CFG on ARM64 back on, even if it takes some time.
	David Major [:dmajor]
Assignee 	
Comment 10 • 2 months ago

In addition to an updated Rust, we will also need to pick up https://bugs.llvm.org/show_bug.cgi?id=39799 / https://reviews.llvm.org/rLLD355141 for our lld.
	David Major [:dmajor]
Assignee 	
Comment 11 • a month ago
Posted file Pick up LLVM fix for CFG on arm64-windows builds — Details

Cherry-picks https://bugs.llvm.org/show_bug.cgi?id=39799.
	David Major [:dmajor]
Assignee 	
Updated • 27 days ago
Depends on: 1523526
	David Major [:dmajor]
Assignee 	
Updated • 19 days ago
Type: enhancement → task
	Pulsebot
	
Comment 12 • 3 hours ago

Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/98013639d600
Pick up LLVM fix for CFG on arm64-windows builds r=froydnj

	Cristina Coroiu [:ccoroiu]
	
Comment 13 • just now

Backed out 2 changesets (bug 1523526, bug 1526443) for Be bustage on Windows AArch

Backout: https://hg.mozilla.org/integration/autoland/rev/8fe96c8d21b7a9fdebdf3300de2367f2cc9232c3

Failure push: uhttps://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&group_state=expanded&revision=98013639d60026eb3a0344fa499a321aec176d2b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=247379721&repo=autoland&lineNumber=930

16:52:09 INFO - MOZILLA_OFFICIAL=1
16:52:09 INFO - CFLAGS= -fcrash-diagnostics-dir=z:/task_1558369429/public/build
16:52:09 INFO - WIN32_REDIST_DIR=z:/task_1558369429/build/src/vs2017_15.9.6/VC/redist/arm64/Microsoft.VC141.CRT
16:52:09 INFO - VSPATH=/z/task_1558369429/build/src/vs2017_15.9.6
16:52:09 INFO - TOOLTOOL_DIR=z:/task_1558369429/build/src
16:52:09 INFO - MOZ_REQUIRE_SIGNING=0
16:52:09 INFO - DIAGNOSTICS_DIR=/z/task_1558369429/public/build
16:52:09 INFO - VSWINPATH=z:/task_1558369429/build/src/vs2017_15.9.6
16:52:09 INFO - NO_CACHE=1
16:52:09 INFO - MOZ_AUTOMATION_ARTIFACT_BUILDS=1
16:52:09 INFO - checking for vcs source checkout... hg
16:52:09 INFO - checking for a shell... C:/mozilla-build/msys/bin/sh.exe
16:52:09 INFO - checking for host system type... x86_64-pc-mingw32
16:52:09 INFO - checking for target system type... aarch64-windows-mingw32
16:52:09 INFO - checking whether cross compiling... yes
16:52:09 ERROR - Traceback (most recent call last):
16:52:09 INFO - File "z:/task_1558369429/build/src/configure.py", line 132, in <module>
16:52:09 INFO - sys.exit(main(sys.argv))
16:52:09 INFO - File "z:/task_1558369429/build/src/configure.py", line 38, in main
16:52:09 INFO - sandbox.run(os.path.join(os.path.dirname(file), 'moz.configure'))
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init_.py", line 441, in run
16:52:09 INFO - self.value_for(option)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init.py", line 528, in _value_for
16:52:09 INFO - return self.value_for_option(obj)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\util.py", line 947, in method_call
16:52:09 INFO - cache[args] = self.func(instance, *args)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init.py", line 591, in _value_for_option
16:52:09 INFO - % option_string.split('=', 1)[0])
16:52:09 INFO - mozbuild.configure.options.InvalidOptionError: --enable-hardening is not available in this configuration
16:52:09 INFO - *** Fix above errors and then restart with
16:52:09 INFO - "./mach build"
16:52:09 INFO - client.mk:111: recipe for target 'configure' failed
16:52:09 INFO - mozmake.EXE: *** [configure] Error 1
16:52:10 ERROR - Return code: 2
16:52:10 WARNING - setting return code to 2
16:52:10 FATAL - 'mach build -v' did not run successfully. Please check log for errors.
16:52:10 FATAL - Running post_fatal callback...
16:52:10 FATAL - Exiting -1
16:52:10 INFO - [mozharness: 2019-05-20 16:52:10.017000Z] Finished build step (failed)
16:52:10 INFO - Running post-run listener: _parse_build_tests_ccov
16:52:10 INFO - Running post-run listener: _shutdown_sccache
16:52:10 INFO - Running post-run listener: _summarize
16:52:10 ERROR - # TBPL FAILURE #
16:52:10 INFO - [mozharness: 2019-05-20 16:52:10.018000Z] FxDesktopBuild summary:
16:52:10 ERROR - # TBPL FAILURE #
Flags: needinfo?(dmajor@mozilla.com)

    Add Comment
    Preview

Markdown styling now supported
Comments Subject to Etiquette and Contributor Guidelines
	
	
Resolve as
Backed out 2 changesets (bug 1523526, bug 1526443) for Be bustage on Windows AArch

Backout: https://hg.mozilla.org/integration/autoland/rev/8fe96c8d21b7a9fdebdf3300de2367f2cc9232c3

Failure push: uhttps://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&group_state=expanded&revision=98013639d60026eb3a0344fa499a321aec176d2b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=247379721&repo=autoland&lineNumber=930

16:52:09 INFO - MOZILLA_OFFICIAL=1
16:52:09 INFO - CFLAGS= -fcrash-diagnostics-dir=z:/task_1558369429/public/build
16:52:09 INFO - WIN32_REDIST_DIR=z:/task_1558369429/build/src/vs2017_15.9.6/VC/redist/arm64/Microsoft.VC141.CRT
16:52:09 INFO - VSPATH=/z/task_1558369429/build/src/vs2017_15.9.6
16:52:09 INFO - TOOLTOOL_DIR=z:/task_1558369429/build/src
16:52:09 INFO - MOZ_REQUIRE_SIGNING=0
16:52:09 INFO - DIAGNOSTICS_DIR=/z/task_1558369429/public/build
16:52:09 INFO - VSWINPATH=z:/task_1558369429/build/src/vs2017_15.9.6
16:52:09 INFO - NO_CACHE=1
16:52:09 INFO - MOZ_AUTOMATION_ARTIFACT_BUILDS=1
16:52:09 INFO - checking for vcs source checkout... hg
16:52:09 INFO - checking for a shell... C:/mozilla-build/msys/bin/sh.exe
16:52:09 INFO - checking for host system type... x86_64-pc-mingw32
16:52:09 INFO - checking for target system type... aarch64-windows-mingw32
16:52:09 INFO - checking whether cross compiling... yes
16:52:09 ERROR - Traceback (most recent call last):
16:52:09 INFO - File "z:/task_1558369429/build/src/configure.py", line 132, in <module>
16:52:09 INFO - sys.exit(main(sys.argv))
16:52:09 INFO - File "z:/task_1558369429/build/src/configure.py", line 38, in main
16:52:09 INFO - sandbox.run(os.path.join(os.path.dirname(file), 'moz.configure'))
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init_.py", line 441, in run
16:52:09 INFO - self.value_for(option)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init.py", line 528, in _value_for
16:52:09 INFO - return self.value_for_option(obj)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\util.py", line 947, in method_call
16:52:09 INFO - cache[args] = self.func(instance, *args)
16:52:09 INFO - File "z:\task_1558369429\build\src\python\mozbuild\mozbuild\configure_init.py", line 591, in _value_for_option
16:52:09 INFO - % option_string.split('=', 1)[0])
16:52:09 INFO - mozbuild.configure.options.InvalidOptionError: --enable-hardening is not available in this configuration
16:52:09 INFO - *** Fix above errors and then restart with
16:52:09 INFO - "./mach build"
16:52:09 INFO - client.mk:111: recipe for target 'configure' failed
16:52:09 INFO - mozmake.EXE: *** [configure] Error 1
16:52:10 ERROR - Return code: 2
16:52:10 WARNING - setting return code to 2
16:52:10 FATAL - 'mach build -v' did not run successfully. Please check log for errors.
16:52:10 FATAL - Running post_fatal callback...
16:52:10 FATAL - Exiting -1
16:52:10 INFO - [mozharness: 2019-05-20 16:52:10.017000Z] Finished build step (failed)
16:52:10 INFO - Running post-run listener: _parse_build_tests_ccov
16:52:10 INFO - Running post-run listener: _shutdown_sccache
16:52:10 INFO - Running post-run listener: _summarize
16:52:10 ERROR - # TBPL FAILURE #
16:52:10 INFO - [mozharness: 2019-05-20 16:52:10.018000Z] FxDesktopBuild summary:
16:52:10 ERROR - # TBPL FAILURE #

Back to Bug 1523526 Comment 13