| Message ID | alpine.LSU.2.20.1705190950570.20726@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
On 05/19/2017 03:51 AM, Richard Biener wrote: > you are commoning 'hwi' here. Also a bad (very short) name for a global > (even if static). > > I'll leave review to Nathan anyway. IMHO the C FE changes are obvious, with the fixing of the hwi name I guess. I'll go rename fn1..6 in the C++ FE once this lands. nathan
On 05/19/17 09:51, Richard Biener wrote: > On Thu, 18 May 2017, Bernd Edlinger wrote: > >> Hi, >> >> unfortunately the first patch was still insufficient. I identified many >> more relatively new places where static tree objects are invisible to >> GC. >> >> Nathan, whatever you are doing, please do it a bit more slowly, thanks. >> >> Bootstrap and reg-testing on x86_64-pc-linux-gnu in progress. >> Is it OK after successful reg-testing? > > Index: gcc/c-family/c-format.c > =================================================================== > --- gcc/c-family/c-format.c (revision 248242) > +++ gcc/c-family/c-format.c (working copy) > @@ -55,6 +55,8 @@ struct function_format_info > > /* Initialized in init_dynamic_diag_info. */ > static GTY(()) tree local_tree_type_node; > +static GTY(()) tree hwi; > +static GTY(()) tree locus; > > static bool decode_format_attr (tree, function_format_info *, int); > static int decode_format_type (const char *); > @@ -3675,8 +3677,6 @@ find_length_info_modifier_index (const format_leng > static void > init_dynamic_asm_fprintf_info (void) > { > - static tree hwi; > - > if (!hwi) > { > format_length_info *new_asm_fprintf_length_specs; > @@ -3734,8 +3734,6 @@ init_dynamic_asm_fprintf_info (void) > static void > init_dynamic_gfc_info (void) > { > - static tree locus; > - > if (!locus) > { > static format_char_info *gfc_fci; > @@ -3828,8 +3826,6 @@ init_dynamic_diag_info (void) > local_tree_type_node = void_type_node; > } > > - static tree hwi; > - > > you are commoning 'hwi' here. Also a bad (very short) name for a global > (even if static). > > I'll leave review to Nathan anyway. > > Richard. > Hmm, yes, I'll drop the 'hwi' change, thus only move 'locus' because: hwi cannot be the root cause of the problem, because it can only be long_integer_type_node or long_long_integer_type_node, otherwise an error would be triggered. >> >> Thanks >> Bernd. >> >> >> On 05/18/17 17:33, Richard Biener wrote: >>> On May 18, 2017 5:15:43 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: >>>> Hi, >>>> >>>> >>>> this attempts to fix occasional segmentation faults that are present in >>>> the current snapshot, while previous snapshot was stable. >>>> >>>> I observed numerous crashes but all were non-reproducible, >>>> like the following example: >>>> >>>> In file included from >>>> /home/ed/gnu/gcc-build-1/x86_64-pc-linux-gnu/libstdc++-v3/include/string:52:0, >>>> from >>>> /home/ed/gnu/gcc-8-20170514-1/gcc/testsuite/g++.dg/asan/asan_test_config.h:19, >>>> from >>>> /home/ed/gnu/gcc-8-20170514-1/gcc/testsuite/g++.dg/asan/asan_test_utils.h:17, >>>> from >>>> /home/ed/gnu/gcc-8-20170514-1/gcc/testsuite/g++.dg/asan/asan_globals_test.cc:12, >>>> from >>>> /home/ed/gnu/gcc-8-20170514-1/gcc/testsuite/g++.dg/asan/asan_globals_test-wrapper.cc:2: >>>> /home/ed/gnu/gcc-build-1/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/basic_string.h:6277:22: >>>> >>>> internal compiler error: Segmentation fault >>>> 0xd7e17f crash_signal >>>> ../../gcc-8-20170514-1/gcc/toplev.c:337 >>>> 0x8f23fe ggc_set_mark(void const*) >>>> ../../gcc-8-20170514-1/gcc/ggc-page.c:1546 >>>> 0x7e6a5f gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:133 >>>> 0x7e8c7a gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:235 >>>> 0x7e8882 gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:365 >>>> 0x81b26d gt_ggc_mx_cp_binding_level(void*) >>>> ./gt-cp-name-lookup.h:72 >>>> 0x7e6d85 gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:648 >>>> 0x7e8ad2 gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:221 >>>> 0x7e8eeb gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:337 >>>> 0x7e8a3c gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:441 >>>> 0x7e7304 gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:606 >>>> 0x81b352 gt_ggc_mx_cxx_binding(void*) >>>> ./gt-cp-name-lookup.h:60 >>>> 0x7e6d85 gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:648 >>>> 0x7e8ef5 gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:336 >>>> 0x7e8a3c gt_ggc_mx_lang_tree_node(void*) >>>> ./gt-cp-tree.h:441 >>>> 0xb2edbe void gt_ggc_mx<tree_node*>(vec<tree_node*, va_gc, vl_embed>*) >>>> ../../gcc-8-20170514-1/gcc/vec.h:1110 >>>> 0xb2edbe gt_ggc_mx_vec_tree_va_gc_(void*) >>>> /home/ed/gnu/gcc-build-1/gcc/gtype-desc.c:1737 >>>> 0xac59f5 ggc_mark_root_tab >>>> ../../gcc-8-20170514-1/gcc/ggc-common.c:77 >>>> 0xac5c50 ggc_mark_roots() >>>> ../../gcc-8-20170514-1/gcc/ggc-common.c:94 >>>> 0x8f2de7 ggc_collect() >>>> ../../gcc-8-20170514-1/gcc/ggc-page.c:2206 >>>> Please submit a full bug report, >>>> with preprocessed source if appropriate. >>>> Please include the complete backtrace with any bug report. >>>> >>>> >>>> The following patch fixes one rather suspicious static tree >>>> object that did not have the GTY attribute, and was therefore >>>> apparently not in the GC root set. >>>> >>>> >>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >>>> Is it OK for trunk? >>> >>> OK. >>> >>> Richard. >>> >>>> >>>> Thanks >>>> Bernd. >>> >> >
On 05/19/2017 10:05 AM, Bernd Edlinger wrote: > hwi cannot be the root cause of the problem, > because it can only be long_integer_type_node > or long_long_integer_type_node, otherwise > an error would be triggered. that's the error I made with the static fns. PCH moves things around, so anything that can be streamed via PCH must be GTY marked. Also, I'm not sure if the current GC is a compacting GC (given PCH can compact things?) nathan
On 05/19/17 16:10, Nathan Sidwell wrote: > On 05/19/2017 10:05 AM, Bernd Edlinger wrote: > >> hwi cannot be the root cause of the problem, >> because it can only be long_integer_type_node >> or long_long_integer_type_node, otherwise >> an error would be triggered. > > that's the error I made with the static fns. PCH moves things around, > so anything that can be streamed via PCH must be GTY marked. Also, I'm > not sure if the current GC is a compacting GC (given PCH can compact > things?) > > Nathan I did three boot-straps with this patch on the 0515-snapshot, and the results are not yet completely stable: One time stage 3 failed as: lto/lto-lang.o lto/lto.o lto/lto-object.o attribs.o lto/lto-partition.o lto/lto-symtab.o libbackend.a main.o libcommon-target.a libcommon.a ../libcpp/libcpp.a ../libdecnumber/libdecnumber.a -L/home/ed/gnu/gcc-build/./isl/.libs -lisl -L/home/ed/gnu/gcc-build/./gmp/.libs -L/home/ed/gnu/gcc-build/./mpfr/src/.libs -L/home/ed/gnu/gcc-build/./mpc/src/.libs -lmpc -lmpfr -lgmp -rdynamic -ldl -L./../zlib -lz libcommon.a ../libcpp/libcpp.a ../libbacktrace/.libs/libbacktrace.a ../libiberty/libiberty.a ../libdecnumber/libdecnumber.a tree-tailcall.o:(.debug_info+0x142b0): relocation truncated to fit: R_X86_64_8 against `.debug_str' collect2: Fehler: ld gab 1 als Ende-Status zurück make[3]: *** [lto1] Fehler 1 make[3]: *** Warte auf noch nicht beendete Prozesse... rm fsf-funding.pod gcov.pod gfdl.pod cpp.pod gpl.pod gcc.pod gcov-tool.pod gcov-dump.pod gfortran.pod gccgo.pod make[3]: Verlasse Verzeichnis '/home/ed/gnu/gcc-build/gcc' make[2]: *** [all-stage3-gcc] Fehler 2 make[2]: Verlasse Verzeichnis '/home/ed/gnu/gcc-build' make[1]: *** [stage3-bubble] Fehler 2 make[1]: Verlasse Verzeichnis '/home/ed/gnu/gcc-build' make: *** [all] Fehler 2 No idea if this is caused by gcc or binutils, I use binutils-2.28. Two times boot-strap succeeded, but once in the test suite I saw this: FAIL: gfortran.dg/minloc_3.f90 -O1 (internal compiler error) FAIL: gfortran.dg/minloc_3.f90 -O1 (test for excess errors) Excess errors: /home/ed/gnu/gcc-8-20170514/gcc/testsuite/gfortran.dg/minloc_3.f90:48:0: internal compiler error: tree check: expected class 'type', have 'exceptional' (error_mark) in create_tmp_var, at gimple-expr.c:474 0xe90a67 tree_class_check_failed(tree_node const*, tree_code_class, char const*, int, char const*) ../../gcc-8-20170514/gcc/tree.c:9911 0x943bbd tree_class_check(tree_node*, tree_code_class, char const*, int, char const*) ../../gcc-8-20170514/gcc/tree.h:3206 0x943bbd create_tmp_var(tree_node*, char const*) ../../gcc-8-20170514/gcc/gimple-expr.c:474 0x967ccc voidify_wrapper_expr(tree_node*, tree_node*) ../../gcc-8-20170514/gcc/gimplify.c:1073 0x96f841 gimplify_statement_list ../../gcc-8-20170514/gcc/gimplify.c:1712 0x96f841 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int) ../../gcc-8-20170514/gcc/gimplify.c:11686 0x973ec8 gimplify_stmt(tree_node**, gimple**) ../../gcc-8-20170514/gcc/gimplify.c:6517 0x97935e gimplify_cond_expr ../../gcc-8-20170514/gcc/gimplify.c:3991 0x96ffe8 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int) ../../gcc-8-20170514/gcc/gimplify.c:11215 0x973ec8 gimplify_stmt(tree_node**, gimple**) ../../gcc-8-20170514/gcc/gimplify.c:6517 0x96f87b gimplify_statement_list ../../gcc-8-20170514/gcc/gimplify.c:1718 0x96f87b gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int) ../../gcc-8-20170514/gcc/gimplify.c:11686 0x973ec8 gimplify_stmt(tree_node**, gimple**) ../../gcc-8-20170514/gcc/gimplify.c:6517 0x97033c gimplify_and_add(tree_node*, gimple**) ../../gcc-8-20170514/gcc/gimplify.c:435 0x97033c gimplify_loop_expr ../../gcc-8-20170514/gcc/gimplify.c:1692 0x97033c gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int) ../../gcc-8-20170514/gcc/gimplify.c:11462 0x973ec8 gimplify_stmt(tree_node**, gimple**) ../../gcc-8-20170514/gcc/gimplify.c:6517 0x96f87b gimplify_statement_list ../../gcc-8-20170514/gcc/gimplify.c:1718 0x96f87b gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int) ../../gcc-8-20170514/gcc/gimplify.c:11686 0x973ec8 gimplify_stmt(tree_node**, gimple**) ../../gcc-8-20170514/gcc/gimplify.c:6517 Anyway, I am going to commit my changes now, as at least the crashes which go thru gt_ggc_mx_cxx_binding have stopped. Bernd.
On Fri, May 19, 2017 at 10:10:23AM -0400, Nathan Sidwell wrote: > On 05/19/2017 10:05 AM, Bernd Edlinger wrote: > > > hwi cannot be the root cause of the problem, > > because it can only be long_integer_type_node > > or long_long_integer_type_node, otherwise > > an error would be triggered. > > that's the error I made with the static fns. PCH moves things around, so > anything that can be streamed via PCH must be GTY marked. Also, I'm not > sure if the current GC is a compacting GC (given PCH can compact things?) other than pch which I'm not sure about I'm pretty sure its not, and is just basic mark and sweep. That said if modules can get merged for gcc 8 I'm very tempted to rm pch now. Trev > > nathan > -- > Nathan Sidwell
On Wed, May 24, 2017 at 7:04 AM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote: > On Fri, May 19, 2017 at 10:10:23AM -0400, Nathan Sidwell wrote: >> On 05/19/2017 10:05 AM, Bernd Edlinger wrote: >> >> > hwi cannot be the root cause of the problem, >> > because it can only be long_integer_type_node >> > or long_long_integer_type_node, otherwise >> > an error would be triggered. >> >> that's the error I made with the static fns. PCH moves things around, so >> anything that can be streamed via PCH must be GTY marked. Also, I'm not >> sure if the current GC is a compacting GC (given PCH can compact things?) > > other than pch which I'm not sure about I'm pretty sure its not, and is > just basic mark and sweep. > > That said if modules can get merged for gcc 8 I'm very tempted to rm pch > now. I think it would be premature to remove PCH from GCC 8. The Modules specification is far from stable, and moving current PCH users to Modules will take significant work (at least if we get something like the current draft rather than the relatively transparent scheme that clang uses). Jason
On 05/24/2017 03:02 PM, Jason Merrill wrote: > I think it would be premature to remove PCH from GCC 8. The Modules > specification is far from stable, and moving current PCH users to > Modules will take significant work (at least if we get something like > the current draft rather than the relatively transparent scheme that > clang uses). I agree. Although I now have function decls and inline function bodies working. Everything else is missing, and I know I'm not handling some things 'correctly'. nathan
Index: gcc/c-family/c-format.c =================================================================== --- gcc/c-family/c-format.c (revision 248242) +++ gcc/c-family/c-format.c (working copy) @@ -55,6 +55,8 @@ struct function_format_info /* Initialized in init_dynamic_diag_info. */ static GTY(()) tree local_tree_type_node; +static GTY(()) tree hwi; +static GTY(()) tree locus; static bool decode_format_attr (tree, function_format_info *, int); static int decode_format_type (const char *); @@ -3675,8 +3677,6 @@ find_length_info_modifier_index (const format_leng static void init_dynamic_asm_fprintf_info (void) { - static tree hwi; - if (!hwi) { format_length_info *new_asm_fprintf_length_specs; @@ -3734,8 +3734,6 @@ init_dynamic_asm_fprintf_info (void) static void init_dynamic_gfc_info (void) { - static tree locus; - if (!locus) { static format_char_info *gfc_fci; @@ -3828,8 +3826,6 @@ init_dynamic_diag_info (void) local_tree_type_node = void_type_node; } - static tree hwi; - you are commoning 'hwi' here. Also a bad (very short) name for a global (even if static).