This is GCC Bugzilla
This is GCC Bugzilla Version 2.20+
View Bug Activity | Format For Printing | Clone This Bug
[forwarded from http://bugs.debian.org/356569] works with 4.0.3, 4.1.0 -O0, 4.1.0 -Ox segfaults. g++ -O3 -c 3ddeskd.ii 3ddeskd.cpp: In function 'void draw_digit(int, float, float)': 3ddeskd.cpp:634: internal compiler error: Segmentation fault Please submit a full bug report, with preprocessed source if appropriate. See <URL:http://gcc.gnu.org/bugs.html> for instructions.
Created an attachment (id=11069) [edit] preprocessed source
Reducing.
DCE is removing some statements: Marking useful stmt: alarm (D.3630_3); Marking useful stmt: return; Processing worklist: processing: return; processing: alarm (D.3630_3); processing: D.3630_3 = (unsigned int) D.3629_2; Eliminating unnecessary statements: Deleting : cfg.20_1 = cfg; Deleting : D.3629_2 = cfg.20_1->autoacquire; Removed 2 of 5 statements (40%) Removed 0 of 0 PHI nodes (0%) void acquire_desktop(int) (rv) { unsigned int D.3630; int D.3629; struct Config * cfg.20; <bb 0>: D.3630_3 = (unsigned int) D.3629_2; # cfg_4 = V_MAY_DEF <cfg_2>; alarm (D.3630_3); return; } I don't understand why (this is using a reduced version of the source) because it is obvious it is used.
I could not reduce this to a good point (at least today).
Here's a reduced version that gives me an ICE on FC5's gcc 4.1.0 with: g++ -O -c 3ddeskd.cpp ===================================== class Config { public: int texture_size; int bg_size; int autoacquire; }; class Face { public: void load_texture_data (int texture_size) { extern Config *cfg; cfg->bg_size++; } }; class FaceSet { private: Face faces; public: void load_texture_data (int size) { faces.load_texture_data(size); }; }; Config *cfg; FaceSet *face_set; void load_background_image(void) { cfg->bg_size++; extern Config *cfg; cfg->autoacquire++; } void humbug () { if (cfg->autoacquire) face_set->load_texture_data(cfg->texture_size); } =====================================
Sorry for the bugspam, but I noticed it could be reduced a bit more. ===================================== struct Config { int bg_size; int autoacquire; }; Config *cfg; void load_background_image(void) { cfg->bg_size++; extern Config *cfg; cfg->autoacquire++; } void humbug () { if (cfg->autoacquire) cfg->bg_size++; } =====================================
I am starting to think this is a front-end issue, producing two decls for cfg.
Confirmed, this only happens with the C++ Front-end, why I don't know why. Anyways here is a testcase which should go into the testsuite for also testing correct code: #include <stdlib.h> typedef struct Config { int bg_size; int autoacquire; }Config; Config *cfg; void load_background_image(void) { cfg->bg_size++; extern Config *cfg; cfg->autoacquire++; } void humbug () { if (cfg->autoacquire) cfg->bg_size++; } int main(void) { Config a; cfg = &a; load_background_image(); humbug(); if (cfg->bg_size != 2) abort(); if (cfg->autoacquire != 1) abort(); return 0; } ---- And shows that it is only the C++ front-end also.
*** Bug 27056 has been marked as a duplicate of this bug. ***
using the program from comment #5, I get IL for load_background_image that looks like: void load_background_image() () { int D.1767; int D.1766; struct Config * cfg.2; int D.1764; int D.1763; struct Config * cfg.1; <bb 0>: cfg.1 = cfg; D.1763 = cfg.1->bg_size; D.1764 = D.1763 + 1; cfg.1->bg_size = D.1764; cfg.2 = cfg; D.1766 = cfg.2->autoacquire; D.1767 = D.1766 + 1; cfg.2->autoacquire = D.1767; return; } THe two assignments of cfg into temps, (cfg.1 and cfg.2) are using DIFFERENT cfg variables (ie, different addresses) on the RHS with the same UID. very bad. either they should be the same var_decl (which I presume is what is intended), or they should have different UIDs. This patchlet provides an assert the triggers when it happens: Index: tree-dfa.c =================================================================== *** tree-dfa.c (revision 112248) --- tree-dfa.c (working copy) *************** referenced_var_insert (unsigned int uid, *** 610,615 **** --- 610,619 ---- h->uid = uid; h->to = to; loc = htab_find_slot_with_hash (referenced_vars, h, uid, INSERT); + /* This assert can only trigger is a variable with the same UID has been + inserted already, but has a different pointer value. ie, we have 2 + different variables with the same UID. Bug 26757. */ + gcc_assert ((*(struct int_tree_map **)loc) == NULL); *(struct int_tree_map **) loc = h; } what happens is that the referenced_var list hashes based on the UID of a variable. referenced_var_insert is only called when a new variable is discovered, so it presumes that the hash slot is already empty. add_referenced_var calls referenced_var_insert when it sees a new variable, and it determines a variable is new by hashing on the address of the variable in the walk_state->vars_found table. since there are 2 different 'cfg' vars, referenced_var_insert overwrote the first 'cfg''s address when it looked up the UID. WHen we go out of ssa, delete_tree_ssa removes all the var annotations of referenced vars. The original 'cfg' no longer occurs in the referenced var list, so it is not cleared. Along comes the next function, and it is using the original 'cfg', and voila, it still has a var_annotation, complete with current_def and lots of fun things set. I havent tracked down where the cfg variable is created, but Im willing to bet a C++ front end person knows right off the bat by looking at the function.
*** Bug 27143 has been marked as a duplicate of this bug. ***
This is a latent bug as show by PR 16792 and PR 20357. Both show the same problem of two DECLs pointing to the same variable and having the same UID.
This has been introduced in http://gcc.gnu.org/ml/gcc-patches/2004-06/msg01594.html Will try to revert the name-lookup.c part and see what exactly breaks and if we couldn't tell about this to cgraph rather than doing ugly hacks.
The reversal of the DECL_UID (x) = DECL_UID (t); assignment in name-lookup.c causes FAIL: g++.dg/opt/inline2.C (test for excess errors) as well as // { dg-do link } // { dg-options "-O1" } static int g; void f() { extern int g; g++; } int main () { f (); } regression. For some of the duplicate decls in different binding levels I'm pretty sure we could just duplicate_decls instead of keeping two decls around, but as the comment explains e.g. for static int g (int x, int y = 8); void f () { int g (int x, int y = 12); g (6); } int g (int x, int y) { return x + y; } int main () { f (); g (6); } we need two decls, at least throughout the frontend. On the other side, the two decls are really supposed to share all cgraph {,varpool} node properties, except the decl itself (and uid), so I wonder if we couldn't have indirect nodes in cgraph_hash and cgraph_varpool_hash or something similar for this purpose.
(In reply to comment #14) > The reversal of the DECL_UID (x) = DECL_UID (t); assignment in name-lookup.c > causes Yes because they are the same decl, if the C++ only uses one decl instead of creating a seperate one, it would just work. Zack's patch did not cause but did expose one issue. Again see PR 20357 for why this was not caused by any of the patches mentioned and why this is a latent bug from before any of the patches mentioned and the correct method is to create one decl with one decl UID and forget about a seperate decl with the copy. Fortran has the same issue and I was trying to fix some of those issue but that work got stalled by me joining Sony and also not having time to work on any of GCC in the last days of college.
The correct fix for this issue is for the C++ front-end to save and restore information about a declaration as scopes are entered and exited. For example, given: extern void f(int); void g() { extern void f(int = 7); f(); } void h() { extern void f(int = 3); f(); } we should use a single FUNCTION_DECL (and a single DECL_UID), add the default argument when the declaration is encountered in g, and then remove the default argument when we exit that scope. This example shows why you can't just keep calling duplicate_decls; you can actually change the values of the default arguments in different scopes, and, in scopes with no default argument declared, the user must supply all arguments. Default arguments are only part of the problem; the same issue applies to things like: extern int a[]; void f() { extern int a[10]; } It will not be particularly entertaining to fix this in the C++ front-end, though it is indeed something that should be done. It's probably doable for 4.2, but I'd think it to be a very risk change for 4.1. Is there a work-around in the middle end that can be applied on the 4.1 branch?
Created an attachment (id=11336) [edit] Possible patch Maybe. there is a work around I have an initial patch for. It re-covers up the problem I think :-) The reason it is triggering in the ssa portion is because of the Schizophrenic way we find referenced variables. find_referenced_vars() uses a hash table of variable addresses (vars_found in the walk_state struct) to determine whether a variable has been seen or not. If it hasn't been seen, it eventually calls add_referenced_var(). add_referenced_var() assumes if it is called that the variable truly hasn't been added, and it creates an entry in the referenced_var hash table, which is hashed by DECL_UID rather than by address. What happens in this test case is with two addresses with the same DECL_UID, add_referenced_var() gets back the same hash slot for the second variable, and simply overwrites the hash slot info with the second variable's info (so the first one's info is gone). When SSA is deleted, the second variables annotation is deleted because it is in the referenced_var hash table, but the first one's is never seen. Next function comes along, and boom, the first var already has ann annotation and hellfire begins to spread. The fix that I have removes the first hashtable. Diego thinks it may have been there for historical reasons, but there is certainly no need for it now. Instead of maintaining and checking thats vars_found table, we can simple check the referenced_var table to see if we have seen a given DECL_UID yet. This should be fixed anyway since it is wasteful, and as a bonus it appears to cover the problem back up again. I ran this a couple of weeks ago on 4.1 but never got back to it. Someone want to try it out and see if it causes any other problems? It bootstrapped and caused no new testsuite regressions on x86-linux then. I'll re-run it tonight.
Andrew -- Thanks for investigating this, and for attempting to tolerate this bit of weirdness from the front end. -- Mark
(In reply to comment #17) > Created an attachment (id=11336) [edit] > Possible patch > > Maybe. there is a work around I have an initial patch for. It re-covers up > the problem I think :-) Andrew, what's the status of this patch?
Andrew -- Is there any news on this patch? Is there anyone else available to review/test Andrew's patch? Thanks, -- Mark
Actually, no, no new news :-). I had forgotten about this one. I'll run it again today and check it in if no problems show up. You want it against 4.1 and mainline? or do you want to try it against mainline for a few days first? Risk is minor, but always present. :-P Andrew
Bootstraps on the 4.1 branch on i686-pc-linux-gnu and produces no new testsuite failures. It also allows the original testcase to compile. The mainline version is now running. Since it seems to be blocking other cases and there is some interest, I'm going to check it into 4.1, and then mainline later. If any issues pop up, we'll deal with them then. Andrew
Subject: Bug 26757 Author: amacleod Date: Tue May 16 20:51:14 2006 New Revision: 113829 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=113829 Log: Remove redundant hash table lookup when finding referenced vars. 2006-05-16 Andrew MacLeod <amacleod@redhat.com> PR c++/26757 * tree-dfa.c (struct walk_state): Remove. (add_referenced_var): Change Parameters. (find_referenced_vars): Done use a walk_state. (find_vars_r): Unused parameter and change parms to add_referenced_var. (referenced_var_insert): Assert same UID has not been inserted. (add_referenced_var): Check if var exists via referenced_var table. (get_virtual_var): Call add_referenced_var with new parameter. Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/tree-dfa.c
Removing the 4.1 marker, as this is now fixed on the 4.1 branch.
Subject: Bug 26757 Author: amacleod Date: Tue May 23 14:07:21 2006 New Revision: 114018 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=114018 Log: 2006-05-23 Andrew MacLeod <amacleod@redhat.com> PR c++/26757 * tree-ssa-loop-im.c (determine_invariantness_stmt): Use add_referenced_var instead of add_referenced_tmp_var. * tree-complex.c (create_one_component_var): Use add_referenced_var. * tree-ssa-loop-manip.c (create_iv, tree_unroll_loop): Use add_referenced_var. * tree-tailcall.c (adjust_accumulator_values, adjust_return_value, tree_optimize_tail_calls_1): Use add_referenced_var. * tree-ssa-loop-ivopts.c (create_new_iv): Use add_referenced_var. * tree-ssa-alias.c (create_memory_tag, create_global_var, create_sft): Use add_referenced_var. * tree-if-conv.c (ifc_temp_var): Use add_referenced_var. * gimplify.c (force_gimple_operand): Use add_referenced_var. * tree-ssa-phiopt.c (conditional_replacement, abs_replacement): Use add_referenced_var. * tree-dfa.c (struct walk_state): Remove. (find_referenced_vars): Remove walk state and vars_found hash table. (make_rename_temp): Use add_referenced_var. (find_vars_r): Pass less parameters to add_referenced_var. (referenced_var_p): New. Is var in referenced_var hash table. (referenced_var_insert): Assert var isn't already in hash table. (add_referenced_var): Don't need walk_state parameter. Add var if it isn't already in the hash table. (add_referenced_tmp_var): Remove. (find_new_referenced_vars_1): Use add_referenced_var. * tree-ssa-pre.c (create_expression_by_pieces, insert_into_preds_of_block, insert_extra_phis, realify_fake_stores): Use add_referenced_var. * tree-vect-patterns.c (vect_pattern_recog_1): Use add_referenced_var. * lambda-code.c (lbv_to_gcc_expression, lle_to_gcc_expression, lambda_loopnest_to_gcc_loopnest, perfect_nestify): Use add_referenced_var. * tree-vect-transform.c (vect_create_addr_base_for_vector_ref, vect_create_data_ref_ptr, vect_create_destination_var, vect_init_vector, vect_build_loop_niters, vect_generate_tmps_on_preheader, vect_update_ivs_after_vectorizer, vect_gen_niters_for_prolog_loop, vect_create_cond_for_align_checks): Use add_referenced_var. * tree-outof-ssa.c (create_temp): Use add_referenced_var. * tree-flow.h (add_referenced_tmp_var): Remove prototype (add_referenced_var): Add prototype. * tree-ssa-structalias.c (get_constraint_for, intra_create_variable_infos): Use add_referenced_var. Modified: trunk/gcc/ChangeLog trunk/gcc/gimplify.c trunk/gcc/lambda-code.c trunk/gcc/tree-complex.c trunk/gcc/tree-dfa.c trunk/gcc/tree-flow.h trunk/gcc/tree-if-conv.c trunk/gcc/tree-outof-ssa.c trunk/gcc/tree-ssa-alias.c trunk/gcc/tree-ssa-loop-im.c trunk/gcc/tree-ssa-loop-ivopts.c trunk/gcc/tree-ssa-loop-manip.c trunk/gcc/tree-ssa-phiopt.c trunk/gcc/tree-ssa-pre.c trunk/gcc/tree-ssa-structalias.c trunk/gcc/tree-tailcall.c trunk/gcc/tree-vect-patterns.c trunk/gcc/tree-vect-transform.c
Looks like it's fixed.
(In reply to comment #26) > Looks like it's fixed. No, just worked around.
Subject: Bug 26757 Author: jakub Date: Tue Jun 13 09:21:30 2006 New Revision: 114607 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=114607 Log: PR middle-end/27793 * cp-tree.h (cxx_int_tree_map): New struct. (struct language_function): Add extern_decl_map field. * name-lookup.c (pushdecl_maybe_friend): Add x -> t mapping to cp_function_chain->extern_decl_map hash table instead of copying over DECL_UID. * cp-gimplify.c (cxx_int_tree_map_eq, cxx_int_tree_map_hash): New functions. (cp_genericize_r): Remap DECL_EXTERN local decls using cp_function_chain->extern_decl_map hash table. * decl.c (finish_function): Clear extern_decl_map. PR c++/26757 PR c++/27894 * g++.dg/tree-ssa/pr26757.C: New test. * g++.dg/tree-ssa/pr27894.C: New test. Added: trunk/gcc/testsuite/g++.dg/tree-ssa/pr26757.C trunk/gcc/testsuite/g++.dg/tree-ssa/pr27894.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/cp-gimplify.c trunk/gcc/cp/cp-tree.h trunk/gcc/cp/decl.c trunk/gcc/cp/name-lookup.c trunk/gcc/testsuite/ChangeLog
This has now been fixed in 4.2.0.
Subject: Bug 26757 Author: jakub Date: Sun Aug 13 20:26:38 2006 New Revision: 116115 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116115 Log: 2006-08-13 Andrew MacLeod <amacleod@redhat.com> PR middle-end/27793 * tree-dfa.c (add_referenced_var): Assert DECL_UID is unique for different decls. 2006-08-13 Jakub Jelinek <jakub@redhat.com> PR c++/28677 PR middle-end/27793 * cp-tree.h (cxx_int_tree_map): New struct. (struct language_function): Add extern_decl_map field. * name-lookup.c (pushdecl_maybe_friend): Add x -> t mapping to cp_function_chain->extern_decl_map hash table instead of copying over DECL_UID. * cp-gimplify.c (cxx_int_tree_map_eq, cxx_int_tree_map_hash): New functions. (cp_genericize_r): Remap DECL_EXTERN local decls using cp_function_chain->extern_decl_map hash table. * decl.c (finish_function): Clear extern_decl_map. Revert: 2006-06-06 Andrew MacLeod <amacleod@redhat.com> PR middle-end/27793 * tree-dfa.c (referenced_vars_dup_list): New. List of duplicate referenced_variables with matching DECL_UID's. (find_referenced_vars): Make sure duplicate list is empty to start. (add_referenced_var): Add var to duplicate list if required. * tree-ssa.c (delete_tree_ssa): Clear var_ann's on duplicates. * tree-flow.h (referenced_vars_dup_list): External declaration. PR c++/26757 PR c++/27894 * g++.dg/tree-ssa/pr26757.C: New test. * g++.dg/tree-ssa/pr27894.C: New test. Added: branches/gcc-4_1-branch/gcc/testsuite/g++.dg/tree-ssa/pr26757.C branches/gcc-4_1-branch/gcc/testsuite/g++.dg/tree-ssa/pr27894.C Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/cp/ChangeLog branches/gcc-4_1-branch/gcc/cp/cp-gimplify.c branches/gcc-4_1-branch/gcc/cp/cp-tree.h branches/gcc-4_1-branch/gcc/cp/decl.c branches/gcc-4_1-branch/gcc/cp/name-lookup.c branches/gcc-4_1-branch/gcc/testsuite/ChangeLog branches/gcc-4_1-branch/gcc/tree-dfa.c branches/gcc-4_1-branch/gcc/tree-flow.h branches/gcc-4_1-branch/gcc/tree-ssa.c
*** Bug 28841 has been marked as a duplicate of this bug. ***