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

Debug build of Zed violates unsafe precondition #8658

Closed
1 task done
Liamolucko opened this issue Mar 1, 2024 · 4 comments · Fixed by #8691
Closed
1 task done

Debug build of Zed violates unsafe precondition #8658

Liamolucko opened this issue Mar 1, 2024 · 4 comments · Fixed by #8691
Assignees
Labels
bug [core label] panic / crash [core label]

Comments

@Liamolucko
Copy link
Contributor

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

When I try to run a debug build of Zed I get this error:

thread 'main' panicked at library/core/src/panicking.rs:155:5:
unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread caused non-unwinding panic. aborting.

The issue's in this bit of code:

fn app_version(&self) -> Result<SemanticVersion> {
unsafe {
let bundle: id = NSBundle::mainBundle();
if bundle.is_null() {
Err(anyhow!("app is not running inside a bundle"))
} else {
let version: id = msg_send![bundle, objectForInfoDictionaryKey: ns_string("CFBundleShortVersionString")];
let len = msg_send![version, lengthOfBytesUsingEncoding: NSUTF8StringEncoding];
let bytes = version.UTF8String() as *const u8;
let version = str::from_utf8(slice::from_raw_parts(bytes, len)).unwrap();
version.parse()
}
}
}

It's assumed that if the app's running inside a bundle, that bundle will have a version associated with it. But it doesn't, so bytes is null causing the failed unsafe precondition.

len is also 0, which is presumably why it works fine in release mode.

Environment

Zed: v0.124.8 (Zed)
OS: macOS 14.3.1
Memory: 16 GiB
Architecture: aarch64

(zed version isn't really relevant because that's the version I have installed, not the version I'm building (eb1ab69))

If applicable, add mockups / screenshots to help explain present your vision of the feature

I already have a fix for the issue:

diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs
index 473fb5978..8b35114b7 100644
--- a/crates/gpui/src/platform/mac/platform.rs
+++ b/crates/gpui/src/platform/mac/platform.rs
@@ -5,7 +5,7 @@ use crate::{
     MenuItem, PathPromptOptions, Platform, PlatformDisplay, PlatformInput, PlatformTextSystem,
     PlatformWindow, Result, SemanticVersion, Task, WindowAppearance, WindowOptions,
 };
-use anyhow::anyhow;
+use anyhow::{anyhow, bail};
 use block::ConcreteBlock;
 use cocoa::{
     appkit::{
@@ -685,6 +685,9 @@ impl Platform for MacPlatform {
                 Err(anyhow!("app is not running inside a bundle"))
             } else {
                 let version: id = msg_send![bundle, objectForInfoDictionaryKey: ns_string("CFBundleShortVersionString")];
+                if version.is_null() {
+                    bail!("bundle does not have version");
+                }
                 let len = msg_send![version, lengthOfBytesUsingEncoding: NSUTF8StringEncoding];
                 let bytes = version.UTF8String() as *const u8;
                 let version = str::from_utf8(slice::from_raw_parts(bytes, len)).unwrap();

I'm just opening this issue first because it seems odd that I seem to be the only one having this issue. Surely I'm not the only one building debug versions of Zed during development?

The fact that Zed's running inside a bundle shouldn't be due to any kind of weird way I'm running it, it's just a plain cargo run.

The docs for NsBundle::mainBundle() say that 'This method may return a valid bundle object even for unbundled apps', so it doesn't seem like it's out of the ordinary for Zed to be running inside a bundle even when it's not been manually packaged into one.

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

If you only need the most recent lines, you can run the zed: open log command palette action to see the last 1000.

No response

@Liamolucko Liamolucko added admin read Pending admin review bug [core label] panic / crash [core label] triage Maintainer needs to classify the issue labels Mar 1, 2024
@osiewicz
Copy link
Contributor

osiewicz commented Mar 1, 2024

Hey, thanks for the report!
It looks like this doesn't crash at runtime because assert_unsafe_precondition (which is what gets called to verify these preconditions in from_raw_parts) is a noop in release build per docs

This macro expands to a check at runtime if debug_assertions is set.

Still, this looks like a valid issue for dev builds that are not bundled.
What rustc version are you building Zed with?

@osiewicz
Copy link
Contributor

osiewicz commented Mar 1, 2024

If you're using nightly then I suppose this is a prime suspect as to why you're experiencing it: rust-lang/rust#120594
I occasionally build with nightly, though I never run it.
Edit:
Yep, reproduces just fine on nightly for me:

thread 'main' panicked at library/core/src/panicking.rs:155:5:
unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread caused non-unwinding panic. aborting.
[1]    12956 abort      cargo +nightly run

Thanks for catching that!

@osiewicz
Copy link
Contributor

osiewicz commented Mar 1, 2024

The patch you've provided looks fine to me, feel free to submit it as a PR :)

@Liamolucko
Copy link
Contributor Author

If you're using nightly then I suppose this is a prime suspect as to why you're experiencing it

Ah, yep, that's the reason: I set it up last month (for -Zthreads=8) and then forgot about it 😅. Anyway, I've opened #8691.

@Moshyfawn Moshyfawn removed the triage Maintainer needs to classify the issue label Mar 2, 2024
osiewicz pushed a commit that referenced this issue Mar 2, 2024
@JosephTLyons JosephTLyons removed the admin read Pending admin review label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [core label] panic / crash [core label]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants