macOS Crash with non-ascii strings in wxgrid with nativecol headers enabled

[marekr]

Just throwing it on here. I sadly can't create a repro sample atm because I actually have no idea how to operate homebrew and build against wxwidgets in a separate app without some effort.

The issue was reported here that I reporoduced: https://gitlab.com/kicad/code/kicad/-/issues/17743

We have this very simple dialog

We used UseNativeColHeader( true ) within the dialog constructor to turn on more native headers on the wxGrid.

The problem is, when non-ascii, aka "名称" for "Name" after translation from _() was fed in, there would be a crash attempting to paint the control.

In particular the crash would occur here: https://github.com/wxWidgets/wxWidgets/blob/661442a4bb8d8716ae362abba6cdf63bd423cf73/src/osx/cocoa/renderer.mm#L737

title was assigned above it title = wxCFStringRef(params->m_labelText).AsNSString();

but the moment an attempt is made to assign title to cell.title, an access exception occurs

Using basic english text seems to work just fine, and disabling usenativecolheader on macos also works fine.

I also have no idea how objective-c works to understand the correctness of anything down in the depths of renderer.mm properly

Platform and version information

• wxWidgets version you use: 3.2.4
• wxWidgets port you use: wxOSX
• OS and its version: macOS 14.4.1

[vadz]

Thanks for reporting, I'll at least try to reproduce it — if I can do this, fixing the crash shouldn't hopefully be too complicated.

[marekr]

FWIW, the dialog code is here:

https://gitlab.com/kicad/code/kicad/-/blob/master/common/dialogs/dialog_configure_paths.cpp I did #if 0 all our own code in that class while testing, seemed like the crash was outside of that/in the dialog painting level instead

and it uses the base class generated from wxFB https://gitlab.com/kicad/code/kicad/-/blob/master/common/dialogs/dialog_configure_paths_base.cpp https://gitlab.com/kicad/code/kicad/-/blob/master/common/dialogs/dialog_configure_paths_base.fbp

The only thing custom is a WX_GRID class we have for some things but I didn't see that in the call stack of the crash https://gitlab.com/kicad/code/kicad/-/blob/master/common/widgets/wx_grid.cpp

Thanks for going to look at it!

[marekr]

@vadz

So I did some debugging and I think this is the problem

        title = wxCFStringRef(params->m_labelText).AsNSString();

wxCFStringRef is a temporary object, it allocates a CFString inside.

Then we grab a pointer from AsNSString which I believe just does some casting?

Then the ~wxCFStringRef destructor is immediately called when stepping over the line, it appears the compiler thinks the ojbect is no longer required

Inside of that destructor is a reset...which proceeds to call wxCFRelease on the saved CFString pointer.

Image of this destructor call with memory address

Then we assign the now freed string to cell.title and it crashes once I step over here: Note the watch of result = still has the freed memory address

[vadz]

Yes, I think you're right, in which case this should be fixed by just this:

diff --git a/src/osx/cocoa/renderer.mm b/src/osx/cocoa/renderer.mm
index 484de678ee..a3aa3d3769 100644
--- a/src/osx/cocoa/renderer.mm
+++ b/src/osx/cocoa/renderer.mm
@@ -713,12 +713,9 @@ void wxRendererMac::DrawMacHeaderCell(wxWindow *win,

         ApplyMacControlFlags(win, cell, flags);

-        NSString* title = @("");
-        NSTextAlignment alignment = NSTextAlignmentLeft;
-
         if ( params )
         {
-            title = wxCFStringRef(params->m_labelText).AsNSString();
+            cell.title = wxCFStringRef(params->m_labelText).AsNSString();
             switch( params->m_labelAlignment )
             {
                 case wxALIGN_CENTER:
@@ -733,9 +730,11 @@ void wxRendererMac::DrawMacHeaderCell(wxWindow *win,
             }

         }
-
-        cell.title = title;
-        cell.alignment = alignment;
+        else
+        {
+            cell.title = @("");
+            cell.alignment = NSTextAlignmentLeft;
+        }

         wxGCDCImpl *impl = (wxGCDCImpl*) dc.GetImpl();
         CGContextRef cgContext = (CGContextRef) impl->GetGraphicsContext()->GetNativeContext();

(sorry, I still didn't have time to reproduce/test it, so didn't check this myself) which should also, incidentally, fix the alignment not being taken into account (because it was always overwritten after assigning to it).

[vadz]

I've now checked that the current code crashes when using ASAN and works correctly with the fix above, so I'm pushing it.

And the grid sample now also shows correctly centered labels when using the native header, as expected.

Thanks for reporting it and finding the problem!

[marekr]

Yea that looks good, it should be back ported to 3.2

[vadz]

Yea that looks good, it should be back ported to 3.2

I don't see this code in 3.2, do you see the problem there too?

[marekr]

Yea that looks good, it should be back ported to 3.2

I don't see this code in 3.2, do you see the problem there too?

Ahhh it's the curse of the kicad wx 3.2 fork that has the change that introduced renderer.mm. it appears backported to fix another bug lol