Bug 82880 - [6 Regression] gcc --help=target --help=optimizers hangs on mips
Summary: [6 Regression] gcc --help=target --help=optimizers hangs on mips
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 7.2.0
: P2 normal
Target Milestone: 6.5
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-07 13:40 UTC by James Cowgill
Modified: 2018-06-25 18:08 UTC (History)
1 user (show)

See Also:
Host:
Target: mips
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-11-21 00:00:00


Attachments
mips workaround (253 bytes, patch)
2017-11-07 13:40 UTC, James Cowgill
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Cowgill 2017-11-07 13:40:23 UTC
Created attachment 42555 [details]
mips workaround

From Debian: https://bugs.debian.org/880962

As the title says, running "gcc --help=target --help=optimizers" on mips (including cross compilers targeting mips) hangs. The bug only occurs on gcc-7-branch and not in any released version of gcc 7.

Bisected (on gcc-7-branch) to:
> 4972d77dcbe0c4186dabc67a0edb7f3f2df4e646 is the first bad commit
> commit 4972d77dcbe0c4186dabc67a0edb7f3f2df4e646
> Author: marxin <marxin@138bc75d-0d04-0410-961f-82ee72b054a4>
> Date:   Fri Sep 15 08:18:34 2017 +0000
> 
>     Subject: Backport r251400
>     
>     2017-09-15  Martin Liska  <mliska@suse.cz>
>     
>             Backport from mainline
>             2017-08-29  Martin Liska  <mliska@suse.cz>
>     
>             PR other/39851
[...]
>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-7-branch@252787 138bc75d-0d04-0410-961f-82ee72b054a4
> 
> :040000 040000 435ac2ed3f121183790fa89d3120400725e3c6a0 424643290a7f6c41e0abe05fb6c596113ed6e611 M	gcc

After this commit, mips_option_override is called twice (once for each --help option). This results in a pass being registered twice (mips_register_frame_header_opt) which causes register_pass to hang in an infinite loop inside position_pass. The reason the same pass is registered twice is becaise the register_pass_info struct in mips_register_frame_header_opt is declared static and is therefore only initialized once. Having said that, the documentation of TARGET_OPTION_OVERRIDE seems to imply this hook should only ever be executed once, so I think the actual bug is in the generic code which calls the hook more than once.

Hanging backtrace for reference:
> (gdb) bt
> #0  position_pass (new_pass_info=0x1cd3780 <mips_register_frame_header_opt()::f>, pass_list=0x1e4d048) at ../../gcc/gcc/passes.c:1325
> #1  0x0000000000c1385e in gcc::pass_manager::register_pass (this=0x1e4d030, pass_info=0x1cd3780 <mips_register_frame_header_opt()::f>) at ../../gcc/gcc/passes.c:1463
> #2  0x0000000000c136ea in register_pass (pass_info=0x1cd3780 <mips_register_frame_header_opt()::f>) at ../../gcc/gcc/passes.c:1418
> #3  0x00000000010b38f9 in mips_register_frame_header_opt () at ../../gcc/gcc/config/mips/frame-header-opt.c:103
> #4  0x00000000010aa8b1 in mips_option_override () at ../../gcc/gcc/config/mips/mips.c:20171
> #5  0x0000000001486ee3 in common_handle_option (opts=0x1e131e0 <global_options>, opts_set=0x1e14000 <global_options_set>, decoded=0x1e632e0, lang_mask=16, kind=0, loc=0, handlers=0x7fffffffdd70, 
>     dc=0x1e14ee0 <global_diagnostic_context>, target_option_override_hook=0x10a91ef <mips_option_override()>) at ../../gcc/gcc/opts.c:1857
> #6  0x000000000148b23c in handle_option (opts=0x1e131e0 <global_options>, opts_set=0x1e14000 <global_options_set>, decoded=0x1e632e0, lang_mask=16, kind=0, loc=0, handlers=0x7fffffffdd70, generated_p=false, 
>     dc=0x1e14ee0 <global_diagnostic_context>) at ../../gcc/gcc/opts-common.c:989
> #7  0x000000000148b990 in read_cmdline_option (opts=0x1e131e0 <global_options>, opts_set=0x1e14000 <global_options_set>, decoded=0x1e632e0, loc=0, lang_mask=16, handlers=0x7fffffffdd70, 
>     dc=0x1e14ee0 <global_diagnostic_context>) at ../../gcc/gcc/opts-common.c:1218
> #8  0x0000000000c10a91 in read_cmdline_options (opts=0x1e131e0 <global_options>, opts_set=0x1e14000 <global_options_set>, decoded_options=0x1e63240, decoded_options_count=3, loc=0, lang_mask=16, 
>     handlers=0x7fffffffdd70, dc=0x1e14ee0 <global_diagnostic_context>) at ../../gcc/gcc/opts-global.c:228
> #9  0x0000000000c10c3e in decode_options (opts=0x1e131e0 <global_options>, opts_set=0x1e14000 <global_options_set>, decoded_options=0x1e63240, decoded_options_count=3, loc=0, 
>     dc=0x1e14ee0 <global_diagnostic_context>, target_option_override_hook=0x10a91ef <mips_option_override()>) at ../../gcc/gcc/opts-global.c:309
> #10 0x0000000000d35574 in toplev::main (this=0x7fffffffde7e, argc=3, argv=0x7fffffffdf78) at ../../gcc/gcc/toplev.c:2116
> #11 0x00000000014828f3 in main (argc=3, argv=0x7fffffffdf78) at ../../gcc/gcc/main.c:39
Note the current and next passes are identical:
> (gdb) p pass
> $28 = (opt_pass *) 0x1e2a360
> (gdb) p pass->next
> $29 = (opt_pass *) 0x1e2a360
The attached patch works around this in the mips code, although as I said above, I don't think this is the correct fix.
Comment 1 Jakub Jelinek 2017-11-10 17:53:51 UTC
Your patch is the right fix, when it is static on the second invocation the new p is not stored into it, and register_pass doesn't save the address it is passed anywhere, just remembers the pass and from the other fields determines where to put it.
Please post it to gcc-patches with a ChangeLog entry.
Comment 2 Jakub Jelinek 2017-11-21 14:50:35 UTC
Author: jakub
Date: Tue Nov 21 14:50:03 2017
New Revision: 255004

URL: https://gcc.gnu.org/viewcvs?rev=255004&root=gcc&view=rev
Log:
	PR target/82880
	* config/mips/frame-header-opt.c (mips_register_frame_header_opt):
	Remove static keyword from f variable.

	* gcc.dg/opts-8.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/opts-8.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/mips/frame-header-opt.c
    trunk/gcc/testsuite/ChangeLog
Comment 3 Jakub Jelinek 2017-11-21 14:50:56 UTC
Fixed on the trunk so far.
Comment 4 Jakub Jelinek 2017-12-14 13:05:26 UTC
Note, of course even better would be to switch the mips backend to use the PASSES_EXTRA stuff + mips-passes.def.  But that is something that should be done by the target maintainers.
Comment 5 Jakub Jelinek 2017-12-15 21:52:37 UTC
Author: jakub
Date: Fri Dec 15 21:52:06 2017
New Revision: 255709

URL: https://gcc.gnu.org/viewcvs?rev=255709&root=gcc&view=rev
Log:
	Backported from mainline
	2017-11-21  James Cowgill  <James.Cowgill@imgtec.com>
		    Jakub Jelinek  <jakub@redhat.com>

	PR target/82880
	* config/mips/frame-header-opt.c (mips_register_frame_header_opt):
	Remove static keyword from f variable.

	* gcc.dg/opts-8.c: New test.

Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.dg/opts-8.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/mips/frame-header-opt.c
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
Comment 6 Jakub Jelinek 2017-12-16 09:00:04 UTC
Fixed for 7.3+ too.
Comment 7 Jakub Jelinek 2018-06-25 16:51:30 UTC
Author: jakub
Date: Mon Jun 25 16:50:58 2018
New Revision: 262033

URL: https://gcc.gnu.org/viewcvs?rev=262033&root=gcc&view=rev
Log:
	Backported from mainline
	2017-11-21  James Cowgill  <James.Cowgill@imgtec.com>
		    Jakub Jelinek  <jakub@redhat.com>

	PR target/82880
	* config/mips/frame-header-opt.c (mips_register_frame_header_opt):
	Remove static keyword from f variable.

	* gcc.dg/opts-8.c: New test.

Added:
    branches/gcc-6-branch/gcc/testsuite/gcc.dg/opts-8.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/config/mips/frame-header-opt.c
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
Comment 8 Jakub Jelinek 2018-06-25 18:08:46 UTC
Fixed for 6.5 too.