<div dir="ltr">Hi phil,<div><br></div><div>The analysis is correct. The only part you / gcc are missing is that hb_font_funcs_set_glyph_func() starts with:</div><div><br></div><div>  if (hb_object_is_immutable (ffuncs)) <br>  { <br>    if (destroy) <br>      destroy (user_data); <br>    return; <br>  }</div><div><br></div><div>So we know ffuncs is not immutable.</div><div><br></div><div>At any rate, I moved the code around, so the compiler should not be confused anymore:</div><div><br></div><div>  <a href="https://github.com/harfbuzz/harfbuzz/commit/b58bfd9818243fc1178ecad0731fa24a5aa3f235">https://github.com/harfbuzz/harfbuzz/commit/b58bfd9818243fc1178ecad0731fa24a5aa3f235</a></div><div><br></div><div>Hope that helps, </div><div><br><div><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature">behdad<br><a href="http://behdad.org/" target="_blank">http://behdad.org/</a></div></div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, May 11, 2022 at 8:32 PM Philip Race <<a href="mailto:philip.race@oracle.com">philip.race@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Someone tried building OpenJDK (which includes harfbuzz) using gcc 12 <br>
and got this warning<br>
<br>
In function 'void trampoline_reference(hb_trampoline_closure_t*)',<br>
     inlined from 'void hb_font_funcs_set_glyph_func(hb_font_funcs_t*, <br>
hb_font_get_glyph_func_t, void*, hb_destroy_func_t)' at <br>
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2368:24:<br>
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2286:12: <br>
error: pointer 'trampoline' used after 'void free(void*)' <br>
[-Werror=use-after-free]<br>
  2286 |   closure->ref_count++;<br>
       |   ~~~~~~~~~^~~~~~~~~<br>
     inlined from 'void trampoline_destroy(void*)' at <br>
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2290:1,<br>
     inlined from 'void <br>
hb_font_funcs_set_nominal_glyph_func(hb_font_funcs_t*, <br>
hb_font_get_nominal_glyph_func_t, void*, hb_destroy_func_t)' at <br>
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:733:1,<br>
     inlined from 'void hb_font_funcs_set_glyph_func(hb_font_funcs_t*, <br>
hb_font_get_glyph_func_t, void*, hb_destroy_func_t)' at <br>
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2363:40:<br>
/home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libharfbuzz/hb-font.cc:2299:8: <br>
note: call to 'void free(void*)' here<br>
  2299 |   free (closure);<br>
<br>
The line numbers don't match current git sources since OpenJDK has an <br>
older version but I think upstream is the same in this regard.<br>
<br>
It is referring to this code<br>
----------<br>
  hb_font_funcs_set_nominal_glyph_func (ffuncs,<br>
hb_font_get_nominal_glyph_trampoline,<br>
                                         trampoline,<br>
                                         trampoline_destroy);<br>
<br>
   trampoline_reference (&trampoline->closure);<br>
--------<br>
I *think* the potential problem is this<br>
void \<br>
hb_font_funcs_set_##name##_func (hb_font_funcs_t *ffuncs,    \<br>
                                  hb_font_get_##name##_func_t func,      \<br>
                                  void *user_data, \<br>
                                  hb_destroy_func_t destroy)   \<br>
{ \<br>
   if (hb_object_is_immutable (ffuncs))                                   \<br>
{ \<br>
     if (destroy)                                                         \<br>
       destroy (user_data);                                               \<br>
return; \<br>
   }<br>
<br>
The macro expansion of that is hb_font_funcs_set_nominal_glyph_func <br>
which is called from<br>
hb_font_funcs_set_glyph_func and on return if that <br>
hb_object_is_immutable code path was taken<br>
then the trampoline reference will have been freed already since we have<br>
<br>
static void<br>
trampoline_destroy (void *user_data)<br>
{<br>
   hb_trampoline_closure_t *closure = (hb_trampoline_closure_t *) user_data;<br>
<br>
   if (--closure->ref_count)<br>
     return;<br>
<br>
   if (closure->destroy)<br>
     closure->destroy (closure->user_data);<br>
   free (closure);<br>
}<br>
<br>
I suppose if it is never immutable no problem, but I don't know how you <br>
prove that.<br>
And it isn't clear what fix would be wanted.<br>
<br>
<br>
-phil<br>
</blockquote></div>