bug#63495: 28.2; menu crashes on macos

[Alan Third <alan@idiocy.org>]

On Mon, Jul 10, 2023 at 11:00:49PM +0200, Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> Eshel Yaron via "Bug reports for GNU Emacs, the Swiss army knife of text
> editors" <bug-gnu-emacs@gnu.org> writes:
> 
> >>> When using the menu - I can call up and see the menu, but if I try to
> >>> select a menu item this always results in emacs crashing.
> >>
> >> I can't reproduce this bug on Emacs 29.  Could you try to reproduce it
> >> on this version?  I remember fixing some problems with the macOS menu
> >> bar recently, so perhaps this bug is already fixed.
> >
> > FWIW this seems very similar to Bug#62402.  At least over here (macOS
> > 13.4, Emacs master), Emacs still reliably crashes when selecting an item
> > in a popup menu.
> 
> Thanks, I can reproduce the issue with the recipe in Bug#62402.  I see
> that this is a regression in Emacs 28 and Emacs 29.  I can't reproduce
> the bug in Emacs 27.
> 
> A bisect shows that this is the first commit that introduced the bug:
> 
> commit c9b37634b131f3617314bd5a38090e96d0b465cf
> Author: Alan Third <alan@idiocy.org>
> Date:   Wed Dec 23 20:12:02 2020 +0000
> 
>     Remove NS menu synthesized events (bug#44333)
>     
>     Remove the frame tracking stuff as it's not used for anything, and
>     move the update tracking into the EmacsMenu class.
>     
>     * src/nsmenu.m (ns_update_menubar): Copy the parsing code from xmenu.c
>     and rework the NS specific code around to update the existing menus
>     instead of rebuilding them completely.
>     (ns_activate_menubar):
>     ([EmacsMenu trackingNotification:]):
>     ([EmacsMenu menuWillOpen:]):
>     ([EmacsMenu menuDidClose:]): Remove unused functions.
>     ([EmacsMenu menuNeedsUpdate:]): Remove menu tracking code and add code
>     to check whether an update is required.
>     ([EmacsMenu fillWithWidgetValue:]):
>     ([EmacsMenu addSubmenuWithTitle:]):
>     ([EmacsMenu initWithTitle:]): Remove references to frame.
>     ([EmacsMenu setFrame:]): Remove method.
>     ([EmacsMenu clear]): Rename to removeAllItems.
>     ([EmacsMenu removeAllItems]): Use built-in removeAllItems, if
>     available.
>     (syms_of_nsmenu): Remove tracking code.
>     * src/nsterm.m (ns_check_menu_open):
>     (ns_check_pending_open_menu):
>     (ns_create_terminal): Remove unused functions.
>     (ns_term_init): Get rid of menu tracking.
>     * src/nsterm.h (EmacsMenu): Remove frame, add needsUpdate and update
>     method definitions.
> 
> One notable thing in that patch is that, in menuNeedsUpdate:,
> ns_update_menubar is now called, if needed, in the Cocoa and the GNUstep
> build.  If I put the call to ns_update_menubar inside the #ifdef
> NS_IMPL_GNUSTEP (so that it's not run in the Cocoa build), I can't
> reproduce the crash anymore.  But I don't know if there are any side
> effects or what motivated the change in the first place.

I can't remember exactly why it was done as it was, but I do remember
there were big problems with getting the menus to update in a timely
manner.

It's interesting that the crash is happening in
runMenuAt:forFrame:keymaps: with it trying to access memory at address
0x3. There's nothing obvious in that function that would be doing
that, so it would be good if we could get a backtrace from a debugger
showing the exact line that's causing the problem and, if possible,
which variable has the value 3.
-- 
Alan Third