[HarfBuzz] GCC 12 warning of use after free in hbfont.cc

[Philip Race]

Someone tried building OpenJDK (which includes harfbuzz) using gcc 12 
and got this warning

In function 'void trampoline_reference(hb_trampoline_closure_t*)',
     inlined from 'void hb_font_funcs_set_glyph_func(hb_font_funcs_t*, 
hb_font_get_glyph_func_t, void*, hb_destroy_func_t)' at 
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2368:24:
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2286:12: 
error: pointer 'trampoline' used after 'void free(void*)' 
[-Werror=use-after-free]
  2286 |   closure->ref_count++;
       |   ~~~~~~~~~^~~~~~~~~
     inlined from 'void trampoline_destroy(void*)' at 
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2290:1,
     inlined from 'void 
hb_font_funcs_set_nominal_glyph_func(hb_font_funcs_t*, 
hb_font_get_nominal_glyph_func_t, void*, hb_destroy_func_t)' at 
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:733:1,
     inlined from 'void hb_font_funcs_set_glyph_func(hb_font_funcs_t*, 
hb_font_get_glyph_func_t, void*, hb_destroy_func_t)' at 
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2363:40:
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2299:8: 
note: call to 'void free(void*)' here
  2299 |   free (closure);

The line numbers don't match current git sources since OpenJDK has an 
older version but I think upstream is the same in this regard.

It is referring to this code
----------
  hb_font_funcs_set_nominal_glyph_func (ffuncs,
hb_font_get_nominal_glyph_trampoline,
                                         trampoline,
                                         trampoline_destroy);

   trampoline_reference (&trampoline->closure);
--------
I *think* the potential problem is this
void \
hb_font_funcs_set_##name##_func (hb_font_funcs_t *ffuncs,    \
                                  hb_font_get_##name##_func_t func,      \
                                  void *user_data, \
                                  hb_destroy_func_t destroy)   \
{ \
   if (hb_object_is_immutable (ffuncs))                                   \
{ \
     if (destroy)                                                         \
       destroy (user_data);                                               \
return; \
   }

The macro expansion of that is hb_font_funcs_set_nominal_glyph_func 
which is called from
hb_font_funcs_set_glyph_func and on return if that 
hb_object_is_immutable code path was taken
then the trampoline reference will have been freed already since we have

static void
trampoline_destroy (void *user_data)
{
   hb_trampoline_closure_t *closure = (hb_trampoline_closure_t *) user_data;

   if (--closure->ref_count)
     return;

   if (closure->destroy)
     closure->destroy (closure->user_data);
   free (closure);
}

I suppose if it is never immutable no problem, but I don't know how you 
prove that.
And it isn't clear what fix would be wanted.


-phil