From 3264051f8d0a2f3451183d019cbd4286903376c1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 19 Mar 2025 20:55:41 -0700 Subject: [PATCH] apprt/embedded: utf8 encoding buffer lifetime must extend beyond call Fixes #6821 UTF8 translation using KeymapDarwin requires a buffer and the buffer was stack allocated in the coreKeyEvent call and returned from the function. We need the buffer to live longer than this. Long term, we're removing KeymapDarwin (there is a whole TODO comment in there about how to do it), but this fixes a real problem today. --- src/Surface.zig | 2 +- src/apprt/embedded.zig | 18 ++++++++++++++++-- src/build/Config.zig | 6 ++++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 3bee52196..7e3bad06d 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1693,7 +1693,7 @@ pub fn keyCallback( self: *Surface, event: input.KeyEvent, ) !InputEffect { - // log.debug("text keyCallback event={}", .{event}); + // log.warn("text keyCallback event={}", .{event}); // Crash metadata in case we crash in here crash.sentry.thread_state = self.crashThreadState(); diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 18674bc38..19e31ca9f 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -149,8 +149,17 @@ pub const App = struct { } /// Convert a C key event into a Zig key event. + /// + /// The buffer is needed for possibly storing translated UTF-8 text. + /// This buffer may (or may not) be referenced by the resulting KeyEvent + /// so it should be valid for the lifetime of the KeyEvent. + /// + /// The size of the buffer doesn't need to be large, we always + /// used to hardcode 128 bytes and never ran into issues. If it isn't + /// large enough an error will be returned. fn coreKeyEvent( self: *App, + buf: []u8, target: KeyTarget, event: KeyEvent, ) !?input.KeyEvent { @@ -217,7 +226,6 @@ pub const App = struct { // Translate our key using the keymap for our localized keyboard layout. // We only translate for keydown events. Otherwise, we only care about // the raw keycode. - var buf: [128]u8 = undefined; const result: input.Keymap.Translation = if (is_down) translate: { // If the event provided us with text, then we use this as a result // and do not do manual translation. @@ -226,7 +234,7 @@ pub const App = struct { .composing = event.composing, .mods = translate_mods, } else try self.keymap.translate( - &buf, + buf, switch (target) { .app => &self.keymap_state, .surface => |surface| &surface.keymap_state, @@ -360,7 +368,9 @@ pub const App = struct { event: KeyEvent, ) !bool { // Convert our C key event into a Zig one. + var buf: [128]u8 = undefined; const input_event: input.KeyEvent = (try self.coreKeyEvent( + &buf, target, event, )) orelse return false; @@ -1425,7 +1435,9 @@ pub const CAPI = struct { app: *App, event: KeyEvent, ) bool { + var buf: [128]u8 = undefined; const core_event = app.coreKeyEvent( + &buf, .app, event.keyEvent(), ) catch |err| { @@ -1677,7 +1689,9 @@ pub const CAPI = struct { surface: *Surface, event: KeyEvent, ) bool { + var buf: [128]u8 = undefined; const core_event = surface.app.coreKeyEvent( + &buf, // Note: this "app" target here looks like a bug, but it is // intentional. coreKeyEvent uses the target only as a way to // trigger preedit callbacks for keymap translation and we don't diff --git a/src/build/Config.zig b/src/build/Config.zig index 7c8605c73..222ce9822 100644 --- a/src/build/Config.zig +++ b/src/build/Config.zig @@ -70,8 +70,10 @@ pub fn init(b: *std.Build) !Config { // If we're building for macOS and we're on macOS, we need to // use a generic target to workaround compilation issues. - if (result.result.os.tag == .macos and builtin.target.isDarwin()) { - result = genericMacOSTarget(b, null); + if (result.result.os.tag == .macos and + builtin.target.os.tag.isDarwin()) + { + result = genericMacOSTarget(b, result.query.cpu_arch); } // If we have no minimum OS version, we set the default based on