Closed Bug 1749665 Opened 2 years ago Closed 2 years ago

Use inline assembly for atomic operation intrinsics

Categories

(Core :: JavaScript Engine: JIT, task, P3)

Product:
Core
Component:
JavaScript Engine: JIT
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
98 Branch
Tracking Flags:
Tracking Status
firefox98 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(5 files)

Bug 1749665 part 1 - Remove MSVC header that's no longer used. r?lth!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1749665 part 2 - Initialize CPU flags in InitializeJit. r?lth!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1749665 part 3 - Replace atomic operation stubs with inline assembly. r?lth!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1749665 part 4 - Add cross-referencing comments. r?lth!
48 bytes, text/x-phabricator-request
Details | Review
Bug 1749665 part 5 - Remove AutoSuppressGCAnalysis instances now that indirect calls are gone. r?lth!
48 bytes, text/x-phabricator-request
Details | Review

For bug 1732362 we want to add a process-wide flag to disable any executable code generation. This can then be used for the socket process so that we can enable more security mitigations there.

This is currently blocked on the trampoline code we generate for atomic operations because these stubs are used from VM code. We can fix this by converting those to inline assembly instead. This seems to work well and should also be a little more efficient because it removes an indirect call.

MSVC support was dropped a while ago.

We currently rely on this happening when compiling the atomic operation stubs,
but that will change with the next patch.

Depends on D135982

Goal of this patch is to allow disabling all JIT codegen in certain Firefox processes
to let us enable more security mitigations. The JS VM uses the atomic operation stubs
we compile at runtime so this patch converts those to inline assembly instead.

This adds a Python script to generate a header file that has all the inline assembly code.
The inline assembly instructions are based on the JIT-compiled trampoline code.

The generated header is used for all x86/x64/arm32/arm64 builds, so this now includes
no-JIT builds as well.

Depends on D135983

See Also: → 1751204
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e02c59ccee17
part 1 - Remove MSVC header that's no longer used. r=lth
https://hg.mozilla.org/integration/autoland/rev/012177f72587
part 2 - Initialize CPU flags in InitializeJit. r=lth
https://hg.mozilla.org/integration/autoland/rev/1942703cc665
part 3 - Replace atomic operation stubs with inline assembly. r=lth
https://hg.mozilla.org/integration/autoland/rev/7a30186c853a
part 4 - Add cross-referencing comments. r=lth
https://hg.mozilla.org/integration/autoland/rev/750012d34b73
part 5 - Remove AutoSuppressGCAnalysis instances now that indirect calls are gone. r=lth

Backed out for causing hazard failures.

Push with failures

Failure log

Backout link

[task 2022-01-23T21:26:31.936Z] PATH="/builds/worker/fetches/gcc/bin:/builds/worker/fetches/sixgill/usr/bin:${PATH}" XDB='/builds/worker/fetches/sixgill/usr/bin/xdb.so' SOURCE='/builds/worker/checkouts/gecko' ANALYZED_OBJDIR='/builds/worker/workspace/obj-analyzed-browser' /builds/worker/workspace/obj-haz-shell/dist/bin/js /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/analyzeHeapWrites.js > heapWriteHazards.txt
[task 2022-01-23T21:26:31.937Z] Spawned process 20257
[task 2022-01-23T21:27:14.200Z] Renaming heapwrites.tmp1 -> heapWriteHazards.txt
[task 2022-01-23T21:27:14.264Z] + check_hazards /builds/worker/workspace/haz-browser
[task 2022-01-23T21:27:14.265Z] + set +e
[task 2022-01-23T21:27:14.265Z] ++ grep -c 'Function.*has unrooted.*live across GC call' /builds/worker/workspace/haz-browser/rootingHazards.txt
[task 2022-01-23T21:27:14.267Z] + NUM_HAZARDS=2
[task 2022-01-23T21:27:14.267Z] ++ grep -c '^Function.*takes unsafe address of unrooted' /builds/worker/workspace/haz-browser/refs.txt
[task 2022-01-23T21:27:14.269Z] + NUM_UNSAFE=230
[task 2022-01-23T21:27:14.269Z] ++ grep -c '^Function.* has unnecessary root' /builds/worker/workspace/haz-browser/unnecessary.txt
[task 2022-01-23T21:27:14.271Z] + NUM_UNNECESSARY=1190
[task 2022-01-23T21:27:14.271Z] ++ grep -c '^Dropped CFG' /builds/worker/workspace/haz-browser/build_xgill.log
[task 2022-01-23T21:27:14.587Z] + NUM_DROPPED=0
[task 2022-01-23T21:27:14.587Z] ++ perl -lne 'print $1 if m!found (\d+)/\d+ allowed errors!' /builds/worker/workspace/haz-browser/heapWriteHazards.txt
[task 2022-01-23T21:27:14.591Z] + NUM_WRITE_HAZARDS=0
[task 2022-01-23T21:27:14.592Z] ++ grep -c '^Function.*expected hazard.*but none were found' /builds/worker/workspace/haz-browser/rootingHazards.txt
[task 2022-01-23T21:27:14.593Z] + NUM_MISSING=0
[task 2022-01-23T21:27:14.593Z] + set +x
[task 2022-01-23T21:27:14.593Z] TinderboxPrint: rooting hazards<br/>2
[task 2022-01-23T21:27:14.593Z] TinderboxPrint: (unsafe references to unrooted GC pointers)<br/>230
[task 2022-01-23T21:27:14.593Z] TinderboxPrint: (unnecessary roots)<br/>1190
[task 2022-01-23T21:27:14.593Z] TinderboxPrint: missing expected hazards<br/>0
[task 2022-01-23T21:27:14.593Z] TinderboxPrint: heap write hazards<br/>0
[task 2022-01-23T21:27:14.594Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'nogc:2' of type 'JS::AutoCheckCannotGC' live across GC call at js/src/ctypes/CTypes.cpp:3675
[task 2022-01-23T21:27:14.594Z] TEST-UNEXPECTED-FAIL | hazards | unrooted 'nogc:3' of type 'JS::AutoCheckCannotGC' live across GC call at js/src/ctypes/CTypes.cpp:3699
[task 2022-01-23T21:27:14.594Z] TEST-UNEXPECTED-FAIL | hazards | 2 rooting hazards detected
[task 2022-01-23T21:27:14.594Z] TinderboxPrint: documentation<br/><a href='https://wiki.mozilla.org/Javascript:Hazard_Builds#Diagnosing_a_rooting_hazards_failure'>static rooting hazard analysis failures</a>, visit "Inspect Task" link for hazard details
[task 2022-01-23T21:27:14.595Z] + grab_artifacts
[task 2022-01-23T21:27:14.595Z] + local artifacts
[task 2022-01-23T21:27:14.595Z] + artifacts=/builds/worker/artifacts
[task 2022-01-23T21:27:14.595Z] + '[' -d /builds/worker/workspace/haz-browser ']'
[task 2022-01-23T21:27:14.595Z] + cd /builds/worker/workspace/haz-browser
[task 2022-01-23T21:27:14.595Z] + ls -lah
[task 2022-01-23T21:27:14.601Z] total 10G
Flags: needinfo?(jdemooij)

CTypes false positive...

Flags: needinfo?(jdemooij)
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d72017f8d68f
part 1 - Remove MSVC header that's no longer used. r=lth
https://hg.mozilla.org/integration/autoland/rev/b89fee5c2372
part 2 - Initialize CPU flags in InitializeJit. r=lth
https://hg.mozilla.org/integration/autoland/rev/3fba20138922
part 3 - Replace atomic operation stubs with inline assembly. r=lth
https://hg.mozilla.org/integration/autoland/rev/7ec36b8d2ba5
part 4 - Add cross-referencing comments. r=lth
https://hg.mozilla.org/integration/autoland/rev/705ad42aa25a
part 5 - Remove AutoSuppressGCAnalysis instances now that indirect calls are gone. r=lth
Regressions: 1752080
Regressions: 1756347
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: