src/js/src/jit/x86-shared/Assembler-x86-shared.h:4795:5: runtime error: store to misaligned address 0x3a4d55f6288d for type 'int32_t' (aka 'int'), which requires 4 byte alignment
Categories
(Core :: JavaScript Engine: JIT, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox105 | --- | fixed |
People
(Reporter: tsmith, Assigned: nbp)
References
(Blocks 2 open bugs)
Details
(Keywords: csectype-undefined)
Attachments
(1 file)
This was found by enabling the alignment
check in UBSan and launching the browser. This type of issue can create inconsistencies across platforms, architectures and optimization levels.
Found with m-c 20220728-c067b27a1842
To enable this check add the following to your mozconfig:
ac_add_options --enable-undefined-sanitizer="alignment"
src/js/src/jit/x86-shared/Assembler-x86-shared.h:4795:5: runtime error: store to misaligned address 0x3a4d55f6288d for type 'int32_t' (aka 'int'), which requires 4 byte alignment
0x3a4d55f6288d: note: pointer points here
0e 48 8d 1d 00 00 00 00 ff 24 cb 49 bb 00 00 00 00 00 80 f9 ff 41 53 49 83 c6 01 0f 1f 44 00 00
^
#0 0x7f01d86debb5 in js::jit::AssemblerX86Shared::PatchWrite_Imm32(js::jit::CodeLocationLabel, js::jit::Imm32) src/js/src/jit/x86-shared/Assembler-x86-shared.h:4795:38
#1 0x7f01d86de7b9 in js::jit::MacroAssembler::patchNearAddressMove(js::jit::CodeLocationLabel, js::jit::CodeLocationLabel) src/js/src/jit/x64/MacroAssembler-x64.cpp:1599:3
#2 0x7f01d7c3f7f3 in js::jit::BaselineInterpreterGenerator::generate(js::jit::BaselineInterpreter&) src/js/src/jit/BaselineCodeGen.cpp:6646:7
#3 0x7f01d7cd7578 in js::jit::GenerateBaselineInterpreter(JSContext*, js::jit::BaselineInterpreter&) src/js/src/jit/BaselineJIT.cpp:952:22
#4 0x7f01d93ca14e in js::jit::JitRuntime::initialize(JSContext*) src/js/src/jit/Ion.cpp:123:8
#5 0x7f01d5c4ada1 in JSRuntime::createJitRuntime(JSContext*) src/js/src/vm/Realm.cpp:138:21
#6 0x7f01d546f10d in JS::InitSelfHostedCode(JSContext*, mozilla::Span<unsigned char const, 18446744073709551615ul>, bool (*)(JSContext*, mozilla::Span<unsigned char const, 18446744073709551615ul>)) src/js/src/vm/Initialization.cpp:237:14
#7 0x7f01a07673c5 in XPCJSContext::Initialize() src/js/xpconnect/src/XPCJSContext.cpp:1369:8
#8 0x7f01a0769512 in XPCJSContext::NewXPCJSContext() src/js/xpconnect/src/XPCJSContext.cpp:1405:23
#9 0x7f01a08de749 in nsXPConnect::InitJSContext() src/js/xpconnect/src/nsXPConnect.cpp:88:25
#10 0x7f01a08dea40 in xpc::InitializeJSContext() src/js/xpconnect/src/nsXPConnect.cpp:103:35
#11 0x7f01d43c3c3f in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:5415:7
#12 0x7f01d43c7ede in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5894:8
#13 0x7f01d43c87ea in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:5962:21
#14 0x7f01d4408fc0 in mozilla::BootstrapImpl::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/Bootstrap.cpp:45:12
#15 0x55ce43bb7f44 in do_main(int, char**, char**) src/browser/app/nsBrowserApp.cpp:227:22
#16 0x55ce43bb6826 in main src/browser/app/nsBrowserApp.cpp:414:16
#17 0x7f020dbe1c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310
#18 0x55ce43af5ec8 in _start (src/objdir-ff-ubsan/dist/bin/firefox+0x31aec8) (BuildId: 9a24bec1c7ae3ec1a6e75a9712b8aed683e61a7e)
Assignee | ||
Comment 1•3 years ago
|
||
Well …, I completely disagree with this UBSan report.
Is there a way to add this one to an allow-list?
The alternative would be add a new type named unaligned_int32_t
which behaves like an int32_t and can be copied from/to an int32_t
such that UBSan does not complain about it.
Reporter | ||
Comment 2•3 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #1)
Well …, I completely disagree with this UBSan report.
An acceptable way to handle this would be to use memcpy.
Perhaps some thing like (untested):
memcpy(*((int32_t*)dataLabel.raw() - 1), &toWrite.value, sizeof(int32_t));
The call to memcpy should be optimized out and this not undefined behavior (in C).
Is there a way to add this one to an allow-list?
I can add it if needed, but it's much nicer to update the code than maintain a suppression list if possible. I'd be happy to test any patches if needed.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 3•3 years ago
|
||
This patch changes the x86/x64 backend to:
- avoid unaligned write of int32_t, unaligned read of uintptr_t values.
- replace typed pointer arithmetic by byte offsets only.
- add comments with opcode when patching assembly.
Comment 5•3 years ago
|
||
bugherder |
Description
•