[vm/ffi] MSAN unpoison stores into Pointers and TypedDatas
#52399
Assignees
Labels
area-vm
Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
library-ffi
Comments
dcharkes
added
area-vm
dcharkes
changed the title
PointersPointers and TypedDatas
|
repro: import 'dart:ffi';
main() {
// final memory = calloc(8, 1).cast<Int8>(); // fine
final memory = malloc(8).cast<Int8>(); // error
final typedList1 = memory.asTypedList(8);
final readVal = typedList1[0];
print(readVal);
}
@Native<Pointer<Void> Function(IntPtr num, IntPtr size)>(isLeaf: true)
external Pointer<Void> calloc(int num, int size);
@Native<Pointer<Void> Function(IntPtr)>(isLeaf: true)
external Pointer<Void> malloc(int size); |
|
|
|
From an offline discussion with @mraleph:
I've explored this a bit, and found two implementation strategies:
I have made a prototype for the IL approach for FFI stores in: https://dart-review.googlesource.com/c/sdk/+/303360/5 This fixes the reproduction I pasted above. Before I continue on this path, maybe we should consider the MC implementation instead. It will have to be duplicated for all archs likely (maybe we can use arch agnostic assembler commands), however, we don't have to bother constructing duplicate IL in various places. |
|
StoreIndexedInstr on x64: https://dart-review.googlesource.com/c/sdk/+/303360 |
copybara-service bot
pushed a commit
that referenced
this issue
Sep 19, 2023
TEST=ffi/function_callbacks_structs_by_value_generated_test TEST=ffi/function_callbacks_structs_by_value_native_callable_generated_test TEST=ffi/regress_52399_test.dart Bug: #52399 Change-Id: Id16ccea5645d9b14a8f2726cb896b99266bba5a2 Cq-Include-Trybots: luci.dart.try:vm-msan-linux-release-x64-try,vm-aot-msan-linux-release-x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303360 Commit-Queue: Daco Harkes <dacoharkes@google.com> Reviewed-by: Ryan Macnak <rmacnak@google.com>
copybara-service bot
pushed a commit
that referenced
this issue
Sep 22, 2023
TEST=ffi/function_structs_by_value_generated_ret_arg_native_test TEST=MSAN SDK build, exercises this instruction a lot. Bug: #52399 Change-Id: Id65a6c4e5500afd2a155d609f8e0144a157aa3b0 Cq-Include-Trybots: luci.dart.try:vm-msan-linux-release-x64-try,vm-aot-msan-linux-release-x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/327201 Reviewed-by: Tess Strickland <sstrickl@google.com> Commit-Queue: Daco Harkes <dacoharkes@google.com>
copybara-service bot
pushed a commit
that referenced
this issue
Nov 28, 2023
TEST=local Bug: #52399 Change-Id: I6af1954c70208534e87b6b87dd5cf62aaf595bd3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329582 Commit-Queue: Ryan Macnak <rmacnak@google.com> Reviewed-by: Daco Harkes <dacoharkes@google.com>
|
@dcharkes I noticed you have submitted some PRs here, is this issue fixed and can be closed ? |
|
The current results only shows 1 test failing on ubsan, and that test is failing on other configurations as well. So, I believe we've covered everything. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We don't unpoisen the memory when doing storing data into
Pointers.If we lookup
mallocin the process, thismallocis instrumented, we store data into the memory withdart:ffiand later we use that memory inC++code again which is instrumented, the memory shows up as uninitialized in MSAN.We should add unpoison calls to the FFI stores. We already do something similar in FFI calls:
sdk/runtime/vm/compiler/backend/il_x64.cc
Lines 1302 to 1330 in 022d92e
We should also then cover the TypedData's as with
Pointer.asTypedListwe can also emit stores from that machine code.Instructions to cover:
.asTypedList().setRange())Architectures to cover:
RISC-V (but our current clang does not support RISC-V, so MSAN is not run on RISC-V currently)The text was updated successfully, but these errors were encountered: