Bug 99126 - Compilation ICE trying insert trap
Summary: Compilation ICE trying insert trap
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: jit (show other bugs)
Version: 11.0
: P3 normal
Target Milestone: 11.0
Assignee: David Malcolm
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-16 19:42 UTC by akrl
Modified: 2024-06-25 23:34 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-02-17 00:00:00


Attachments
reproducer (138.88 KB, application/gzip)
2021-02-16 19:42 UTC, akrl
Details
Minimal reproducer as a test case (884 bytes, patch)
2021-02-18 00:07 UTC, David Malcolm
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description akrl 2021-02-16 19:42:55 UTC
Created attachment 50205 [details]
reproducer

Hi all,

to reproduce with the attached:

=========
$ gcc libgccjit_repro.c -lgccjit
$ ./a.out
Segmentation fault (core dumped)
=========

This is my understanding of what is going on here: we have a some
generated code that in GIMPLE is proved to dereference a null pointer
(BTW this code should be unreachable).

MEM[(struct comp_Lisp_Cons *)0B].u.s.car = _35;

From the 'F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_0'
function.

Running pass 'isolate-paths' we try to inject a trap but
'builtin_decl_explicit (BUILT_IN_TRAP)' is returning NULL as the
builtin declaration that we should find in 'builtin_info' is never
initialized by the frontend (libgccjit).  As a consequence we ICE in
'gimple_build_call'.

Here the backtrace:

#0  gimple_build_call (fn=0x0, nargs=nargs@entry=0) at ../../gcc/gimple.c:264
#1  0x00007ffff7450cf8 in insert_trap (si_p=si_p@entry=0x7fffffffde20, op=0x7ffff5654fc0) at ../../gcc/gimple-ssa-isolate-paths.c:93
#2  0x00007ffff745302b in find_explicit_erroneous_behavior () at ../../gcc/gimple-ssa-isolate-paths.c:863
#3  gimple_ssa_isolate_erroneous_paths () at ../../gcc/gimple-ssa-isolate-paths.c:928
#4  (anonymous namespace)::pass_isolate_erroneous_paths::execute (this=<optimized out>) at ../../gcc/gimple-ssa-isolate-paths.c:979
#5  0x00007ffff6a7fc3d in execute_one_pass (pass=0x6255a0) at ../../gcc/passes.c:2509
#6  0x00007ffff6a803a8 in execute_pass_list_1 (pass=0x6255a0) at ../../gcc/passes.c:2597
#7  0x00007ffff6a803ba in execute_pass_list_1 (pass=0x624990) at ../../gcc/passes.c:2598
#8  0x00007ffff6a80405 in execute_pass_list (fn=0x7ffff5849790, pass=<optimized out>) at ../../gcc/passes.c:2608
#9  0x00007ffff6790428 in cgraph_node::expand (this=0x7ffff5850ca8) at ../../gcc/context.h:48
#10 0x00007ffff6791a6d in expand_all_functions () at ../../gcc/cgraphunit.c:2476
[...]

This ICE breaks Emacs bootstrap on 32bit machines configuring with
'--with-wide-int'.

A reduced reduced should be writable (manually as there's no creduce
for libgccjit) ATM I'm posting what I have from Emacs.

GCC 10 is likely to be affected too, I'll try it soon, GCC 9 is
working and does not show this bug.

  Andrea
Comment 1 Andrea Corallo 2021-02-16 20:26:18 UTC
This is the bt of how the C front-end is initializing these
declarations:

#0  set_builtin_decl (implicit_p=<optimized out>, 
    decl=<function_decl 0x7ffff72a0800 __builtin_return_address>, 
    fncode=<optimized out>) at ../../gcc/tree.h:5662
#1  def_builtin_1 (fncode=<optimized out>, name=<optimized out>, 
    fntype=<optimized out>, libtype=<tree 0x0>, both_p=<optimized out>, 
    fallback_p=<optimized out>, nonansi_p=false, 
    fnattrs=<tree_list 0x7ffff7204f78>, implicit_p=true, fnclass=BUILT_IN_NORMAL)
    at ../../gcc/c-family/c-common.c:4729
#2  0x0000000000a291c9 in c_define_builtins (
    va_list_arg_type_node=<optimized out>, va_list_ref_type_node=<optimized out>)
    at ../../gcc/builtins.def:933

Thinking loud: I guess in jit-builtins.c we should loop once over all
the builtins calling 'set_builtin_decl'?  Probably in the constructor
for gcc::jit::builtins_manager?
Comment 2 Martin Liška 2021-02-17 08:18:17 UTC
Thanks for the report.

> This is my understanding of what is going on here: we have a some
> generated code that in GIMPLE is proved to dereference a null pointer
> (BTW this code should be unreachable).

That's fine.

> 
> MEM[(struct comp_Lisp_Cons *)0B].u.s.car = _35;
> 

So I guess JIT should really initialize the builtins.
I tried manually calling:
gcc_jit_context_get_builtin_function (ctxt_0x8892590, "__builtin_trap");

and then your reproducer is fine.
Leaving to David.
Comment 3 David Malcolm 2021-02-18 00:07:11 UTC
Created attachment 50216 [details]
Minimal reproducer as a test case

Attached is a minimal reproducer, as a test case.  I don't have a fix for this yet.
Comment 4 David Malcolm 2021-02-18 21:07:47 UTC
Am testing a fix.
Comment 5 Andrea Corallo 2021-02-18 21:19:26 UTC
"dmalcolm at gcc dot gnu.org via Gcc-bugs" <gcc-bugs@gcc.gnu.org>
writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99126
>
> David Malcolm <dmalcolm at gcc dot gnu.org> changed:
>
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>              Status|NEW                         |ASSIGNED
>
> --- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
> Am testing a fix.

Nice!

As a side question: do you guys think disabling "isolate-paths" is the
right workaround for the affected versions or might have harmful side
effects?

Thanks

  Andrea
Comment 6 GCC Commits 2021-02-19 02:29:40 UTC
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:b258e263e0d74ca1f76aeaac5f4d1abef6b13707

commit r11-7288-gb258e263e0d74ca1f76aeaac5f4d1abef6b13707
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Thu Feb 18 21:28:26 2021 -0500

    jit: fix ICE on BUILT_IN_TRAP [PR99126]
    
    gcc/jit/ChangeLog:
            PR jit/99126
            * jit-builtins.c
            (gcc::jit::builtins_manager::get_builtin_function_by_id):
            Update assertion to reject BUILT_IN_NONE.
            (gcc::jit::builtins_manager::ensure_optimization_builtins_exist):
            New.
            * jit-builtins.h
            (gcc::jit::builtins_manager::ensure_optimization_builtins_exist):
            New decl.
            * jit-playback.c (gcc::jit::playback::context::replay): Call it.
            Remove redundant conditional on bm.
    
    gcc/testsuite/ChangeLog:
            PR jit/99126
            * jit.dg/test-trap.c: New test.
Comment 7 David Malcolm 2021-02-19 02:34:15 UTC
(In reply to Andrea Corallo from comment #5)
> As a side question: do you guys think disabling "isolate-paths" is the
> right workaround for the affected versions or might have harmful side
> effects?

It ought to stop the crash; given that the code path happens on places where the compiler predicts a NULL dereference, I don't think it can cause additional problems.
Comment 8 David Malcolm 2021-02-19 02:35:24 UTC
(In reply to CVS Commits from comment #6)
> The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:
> 
> https://gcc.gnu.org/g:b258e263e0d74ca1f76aeaac5f4d1abef6b13707
> 
> commit r11-7288-gb258e263e0d74ca1f76aeaac5f4d1abef6b13707

Fixed on trunk for gcc 11.  Andrea, do you need my to backport this?  What GCC versions are you targeting with your emacs project?
Comment 9 Andrea Corallo 2021-02-22 13:29:23 UTC
"dmalcolm at gcc dot gnu.org via Gcc-bugs" <gcc-bugs@gcc.gnu.org>
writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99126
>
> --- Comment #8 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
> (In reply to CVS Commits from comment #6)
>> The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:
>> 
>> https://gcc.gnu.org/g:b258e263e0d74ca1f76aeaac5f4d1abef6b13707
>> 
>> commit r11-7288-gb258e263e0d74ca1f76aeaac5f4d1abef6b13707
>
> Fixed on trunk for gcc 11.  Andrea, do you need my to backport this?  What GCC
> versions are you targeting with your emacs project?

We are targetting them all, but to my test I could not trigger this bug
on GCC9 so I guess we could backport on GCC10 only?
Comment 10 Andrew Pinski 2024-06-25 23:34:05 UTC
Fixed. GCC 10.x is no longer supported.