Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FFI regression on macOS affecting package:sqlite3 #53928

Closed
simolus3 opened this issue Nov 1, 2023 · 7 comments
Closed

FFI regression on macOS affecting package:sqlite3 #53928

simolus3 opened this issue Nov 1, 2023 · 7 comments

Comments

@simolus3
Copy link
Contributor

simolus3 commented Nov 1, 2023

This snippet works with Dart 3.1.5 (stable), but crashes under the latest Dart Beta (3.2.0-210.4.beta) and Dev Builds (3.3.0-65.0.dev)

import 'dart:io';

import 'package:sqlite3/sqlite3.dart';

void main() {
  final db = sqlite3.openInMemory();
  db.createFunction(
    functionName: 'dart_version',
    function: (args) => Platform.version,
  );
  print(db.select('SELECT dart_version()'));
  db.dispose();
}

To reproduce this:

git clone https://github.com/simolus3/sqlite3.dart.git -b repro-macos-issue --depth 1
dart pub get
dart run example/main.dart 
zsh: killed     dart run example/main.dart

I'm running macOS 14.0 on ARM. The stack trace and error code makes it seem like this might be related to flutter/flutter#136980. If it matters, that version of the sqlite3 package is using Pointer.fromFunction to implement callbacks, not NativeCallables.

Crashed Thread:        10  DartWorker

Exception Type:        EXC_BAD_ACCESS (SIGKILL (Code Signature Invalid))
Exception Codes:       UNKNOWN_0x32 at 0x0000000103043a90
Exception Codes:       0x0000000000000032, 0x0000000103043a90

Termination Reason:    Namespace CODESIGNING, Code 2 Invalid Page

[...]

Thread 10 Crashed:: DartWorker
0   ???                           	       0x103043a90 ???
1   libsqlite3.dylib              	       0x193dda594 sqlite3_step + 996
2   ???                           	       0x103107a28 ???
3   ???                           	       0x103956ec4 ???
4   ???                           	       0x10395695c ???
5   ???                           	       0x1039567c4 ???
6   ???                           	       0x103956674 ???
7   ???                           	       0x103955f0c ???
8   ???                           	       0x103953728 ???
9   ???                           	       0x103953404 ???
10  ???                           	       0x10394cc60 ???
11  ???                           	       0x10392e3d8 ???
12  ???                           	       0x10392e27c ???
13  ???                           	       0x10392e158 ???
14  ???                           	       0x10392c9a8 ???
15  ???                           	       0x10392c600 ???
16  ???                           	       0x10392b0e8 ???
17  ???                           	       0x103103404 ???
18  dart                          	       0x1009f183c 0x10088c000 + 1464380
19  dart                          	       0x1009f32f4 0x10088c000 + 1471220
20  dart                          	       0x100a10be4 0x10088c000 + 1592292
21  dart                          	       0x100a33830 0x10088c000 + 1734704
22  dart                          	       0x100a33e5c 0x10088c000 + 1736284
23  dart                          	       0x100b3bed0 0x10088c000 + 2817744
24  dart                          	       0x100b3c148 0x10088c000 + 2818376
25  dart                          	       0x100ac64a0 0x10088c000 + 2335904
26  libsystem_pthread.dylib       	       0x18d11b034 _pthread_start + 136
27  libsystem_pthread.dylib       	       0x18d115e3c thread_start + 8
@mraleph
Copy link
Member

mraleph commented Nov 1, 2023

Yeah, we overlooked this. Fix: https://dart-review.googlesource.com/c/sdk/+/333260

@athomas is it possible to setup a bot which runs tests using signed SDK on Mac?

@athomas
Copy link
Member

athomas commented Nov 2, 2023

@mraleph Possible, yes. But we currently have nothing like it. We'd need:

  • Sign SDK builds on main.
  • Ensure that test.py works for the relevant testing using an SDK distribution.
  • Setup a new builder that downloads a signed SDK instead of building it and then runs tests.
  • Trigger that builder from the SDK builder (or as a cron job).

@dcharkes
Copy link
Contributor

dcharkes commented Nov 2, 2023

@mraleph I assume we want a cherry pick for this?

@mraleph
Copy link
Member

mraleph commented Nov 2, 2023

I will let it cook a bit while I am changing build files to make it possible to run tests on our bots. But yep - I will also file a cherry pick request for it.

@mraleph
Copy link
Member

mraleph commented Nov 2, 2023

@athomas I have an alternative proposal. I have massaged GN rules to produce signed binaries using the identity supplied in through a GN arg. We should be able to use - identity to produce ad-hoc signed binaries. I am doing CQ run now to see if this is good enough. If it is - then we can change our bots to build with DART_GN_ARGS='codesigning_identity="-"' and that should cover our testing needs.

copybara-service bot pushed a commit that referenced this issue Nov 3, 2023
This can be controlled via  codesigning_identity GN arg.

For example, setting codesigning_identity="-"
would produce ad-hoc signed binaries.

This CL also includes changes in vm/cc tests which are needed
for tests to be green when running with hardened runtime.

Issue #53928

Tested: enabled ad-hoc signing and tested on bots.
Cq-Include-Trybots: luci.dart.try:vm-mac-debug-arm64-try,vm-mac-release-arm64-try,vm-mac-release-x64-try,vm-mac-debug-x64-try
Change-Id: I3c3a6265c62b2904e43a326b7d8223bcfd393577
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/333401
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
copybara-service bot pushed a commit that referenced this issue Nov 3, 2023
This would allow us to catch issue with hardened runtime
which previously would slip through and only be discovered
after release.

Issue #53928

R=whesse@google.com

Change-Id: Iea1ced5433a3cf753d251a519a1de7bd0673ea3c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/333822
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
@dcharkes
Copy link
Contributor

dcharkes commented Nov 8, 2023

I will let it cook a bit while I am changing build files to make it possible to run tests on our bots. But yep - I will also file a cherry pick request for it.

@mraleph Friendly bump so this doesn't fall through the cracks.

@mraleph
Copy link
Member

mraleph commented Nov 13, 2023

CP request filed.

copybara-service bot pushed a commit that referenced this issue Nov 14, 2023
This is follow up to bd57548 which switched us to use manual copying
for call back pages on Mac OS and iOS. However these newly allocated
pages need to be created with MAP_JIT flag otherwise OS will kill
us with code signing violation if hardened runtime is enabled.

This can only be observed when the binary is signed that's why
we have not seen it on CI.

TEST=manually signed and tested that it no longer crashes.

Fixes #53928

Changelog-Exempt: Changing base 3.2.0 release
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/333260
Cherry-pick-request: #54035
Change-Id: Ia096e4b6fe59a2a34bcea9e7501c289f831c1406
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/335881
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants