[Bug jit/98586] New: libgccjit crashes with segmentation fault on failed gcc_assert

[Bug jit/98586] New: libgccjit crashes with segmentation fault on failed gcc_assert

[keith.marshall at mailinator dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98586

            Bug ID: 98586
           Summary: libgccjit crashes with segmentation fault on failed
                    gcc_assert
           Product: gcc
           Version: 9.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: jit
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: keith.marshall at mailinator dot com
  Target Milestone: ---

In response to a feature request by Eli Zaretskii, with my follow-up as
detailed at https://osdn.net/projects/mingw/ticket/41070, I have been
evaluating the feasibility of providing a libgccjit.dll implementation for
mingw32.  Since GCC-9.2.0 is the most recent version, for which I have a
successful build of GCC itself, my initial efforts have been focussed on a
libgccjit implementation for that version.

With a series of patches, as attached to the OSDN ticket, I have successfully
compiled the shared library, as libgccjit-0.dll; I am even able to successfully
compile, and link, the tut01-hello-world.c example from your own online manual,
at https://gcc.gnu.org/onlinedocs/jit/intro/tutorial01.html.  Unfortunately
however, when attempting to run this program, it crashes with a segmentation
fault, and no useful diagnostic message.

I have traced the origin of the crash to a failing assertion, at line 54 in
gcc/jit/jit-tempdir.c.  The failure of the assertion, as explained in the OSDN
ticket, is due to it testing an invalid assumption — on MS-Windows, both '/'
and '\\' are valid directory name separator characters, but the assertion
requires '/', whereas the libiberty.a choose_tmpdir() function returns a path
containing, and ending with, only '\\'.

While I can easily, and will, correct the invalid assumption, on which the
assertion fails, that the failed assertion terminates in a segmentation fault,
rather than a graceful termination, with an appropriate diagnostic message,
indicates that there is a deeper seated underlying defect.  The segmentation
fault actually occurs in function pp_format(), from gcc/pretty-print.c, when it
attempts to dereference a NULL pointer, passed as the printer member of the
global_dc structure.  I guess that this structure has not been initialized;
perhaps a call to diagnostic_initialize() has been omitted, but I have no idea
where to insert it.

[Bug jit/98586] libgccjit crashes with segmentation fault on failed gcc_assert

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98586

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2021-01-07

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Thanks for filing this.  I'm able to reproduce the issue by hacking in a 
  gcc_assert (0);
into that routine.

I looked at calling diagnostic_initialize.

Unfortunately, libgccjit supports being linked into multithreaded processes,
and it guards all of the regular compiler state with a single big mutex,
including the diagnostic subsystem (and the "global_dc" pointer implicitly used
by fancy_abort).  This failure is happening before the mutex is acquired.

Some possible solutions:

(a) guard more code with the mutex, and fix the ICE-handling to ensure that
diagnostic_initialize is called if need be so the final message can be printed

(b) make the ICE handler detect this case, and use a custom diagnostic context
for the final message (though it's accessing data outside of the mutex in this
case, albeit just one pointer that rarely changes, and in crash handling)

(c) don't use fancy_abort within libgccjit, perhaps overriding the defn of the
abort macro in system.h

(d) rewrite fancy_abort so that it detects the case somehow

(e) have a custom assertion macro for the parts of libgccjit that aren't
guarded by the mutex

I'm not sure yet what the best fix is.

[Bug jit/98586] libgccjit crashes with segmentation fault on failed gcc_assert

[keith.marshall at mailinator dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98586

--- Comment #2 from Keith Marshall <keith.marshall at mailinator dot com> ---
(In reply to David Malcolm from comment #1)
> I looked at calling diagnostic_initialize.
> 
> Unfortunately, libgccjit supports being linked into multithreaded processes,
> and it guards all of the regular compiler state with a single big mutex,
> including the diagnostic subsystem (and the "global_dc" pointer implicitly
> used by fancy_abort).  This failure is happening before the mutex is
> acquired.

Indeed, yes.  I actually patched the mutex acquisition/release code, to use a
native MS-Windows critical section, in preference to the alien pthreads mutex:

  https://osdn.net/ticket/download.php?group_id=3917&tid=41070&file_id=5791

I thought that, maybe, it was my modification which led to the crash; I was
surprised to find that execution never reached my modified code.

> I'm not sure yet what the best fix is.

I don't know the GCC internals well enough, to advise on this, but I'm willing
to assist with testing, in any way that I can.  In the meantime, I've patched
around the flawed assumption, which leads to the failing assertion, in the
first place:

  https://osdn.net/ticket/download.php?group_id=3917&tid=41070&file_id=5799

Thanks for your attention.

[Bug jit/98586] libgccjit crashes with segmentation fault on failed gcc_assert

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98586

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Patch posted as
https://gcc.gnu.org/pipermail/gcc-patches/2021-January/563266.html

[Bug jit/98586] libgccjit crashes with segmentation fault on failed gcc_assert

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98586

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:387f6c15d303a8f8da508e419fea10a6ef0e2590

commit r11-6700-g387f6c15d303a8f8da508e419fea10a6ef0e2590
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Jan 14 17:02:28 2021 -0500

    Handle fancy_abort before diagnostic initialization [PR98586]

    If fancy_abort is called before the diagnostic subsystem is initialized,
    internal_error will crash internally in a way that prevents a useful
    message reaching the user.

    This can happen with libgccjit in the case of gcc_assert failures
    that occur outside of the libgccjit mutex that guards the rest of
    gcc's state, including global_dc (when global_dc may not be
    initialized yet, or might be in use by another thread).

    I tried a few approaches to fixing this as noted in PR jit/98586
    e.g. using a temporary diagnostic_context and initializing it for
    the call to internal_error, however the more code that runs, the
    more chance there is for other errors to occur.

    The best fix appears to be to simply fall back to a minimal abort
    implementation that only relies on i18n, as implemented by this
    patch.

    gcc/ChangeLog:
            PR jit/98586
            * diagnostic.c (diagnostic_kind_text): Break out this array
            from...
            (diagnostic_build_prefix): ...here.
            (fancy_abort): Detect when diagnostic_initialize has not yet been
            called and fall back to a minimal implementation of printing the
            ICE, rather than segfaulting in internal_error.

[Bug jit/98586] libgccjit crashes with segmentation fault on failed gcc_assert

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98586

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #5 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed by the above commit.

[Bug jit/98586] libgccjit crashes with segmentation fault on failed gcc_assert

[keith.marshall at mailinator dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98586

--- Comment #6 from Keith Marshall <keith.marshall at mailinator dot com> ---
(In reply to David Malcolm from comment #5)
> Should be fixed by the above commit.

I applied your patch, and disabled (by changing "#ifdef _WIN32" to "#if 0") the
effect of my own, so that the invalid assumption regarding Win32 DIR_SEPARATOR
is not corrected.  I now see:

 $ ./tut01-hello-world.exe
 internal compiler error: in make_tempdir_path_template, at
jit/jit-tempdir.c:73

 This application has requested the Runtime to terminate it in an unusual way.
 Please contact the application's support team for more information.

so all looks good, thanks.  Just one residual irritation: after displaying this
error message, in the console, MS-Windows pops up a dialogue box to tell me
that the program has stopped working, and invites me to (pointlessly) forward
an error report to Microsoft, before finally terminating the process.  Just
another annoying Windows feature, which I don't know how to circumvent, short
of hacking the registry, and the control panel settings, to disable Windows
error reporting altogether.

BTW, you may wish to consider the patches, which I have attached to:

 https://osdn.net/projects/mingw/ticket/41070

They do help those building libgccjit for MS-Windows.