Between revisions156515 and 156527 many new failures have appeared in the objc test suite (see http://gcc.gnu.org/ml/gcc-regression/2010-02/msg00013.html ). These errors are still there at revision 156749 (see http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg01291.html or http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg01290.html ) and seem to need '-On -fnext-runtime' with n!=0. Note that the errors do not appear on the Jack Howarth tests for x86_64-apple-darwin10.3.0 (see http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg01258.html ) while they do on x86_64-apple-darwin10.2 (see http://gcc.gnu.org/ml/gcc-testresults/2010-02/msg00747.html ).
confirmed, this can be reproduced outside the testsuite framework: for example... [ppc/darwin9/156749]; $ ./gcc/xgcc -B gcc ../gcc-4-5-trunk/gcc/testsuite/objc/execute/cascading-1.m -fnext-runtime -O1 -o tc -lobjc -g -save-temps $ gdb tc GNU gdb 6.3.50-20050815 (Apple version gdb-967) (Tue Jul 14 02:15:14 UTC 2009) Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "powerpc-apple-darwin"...Reading symbols for shared libraries ..... done (gdb) run Starting program: /Volumes/ScratchCS/gcc-4-5-trunk-build/tc Reading symbols for shared libraries +++... done Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x466f6f20 0x9622dfd8 in objc_msgSend () (gdb) backtrace #0 0x9622dfd8 in objc_msgSend () #1 0x00001f8c in main (argc=<value temporarily unavailable, due to optimizations>, argv=<value temporarily unavailable, due to optimizations>) at ../gcc-4-5-trunk/gcc/testsuite/objc/execute/cascading-1.m:33 (gdb)
Track down the regression that caused this and see what actually is the difference in generated code and/or tree/rtl dumps. Access to non-free operating systems is restricted.
(In reply to comment #2) > Track down the regression that caused this r156519 >and see what actually is the difference in generated code and/or tree/rtl dumps. what output would be the most useful?
(In reply to comment #3) > (In reply to comment #2) > > Track down the regression that caused this > > r156519 > > >and see what actually is the difference in generated code and/or tree/rtl dumps. > > what output would be the most useful? Make one directory for each revision for outputs from -fdump-tree-all, tar them up and attach them. A single testcase that passes before and fails after the revision should be enough. You can leave out dumps that are identical for both revisions.
Created attachment 19860 [details] generated asm differences with/without r156519
Created attachment 19861 [details] optimized tree diffs.
Created attachment 19862 [details] diffs for all fdump-tree-all output
Hm. So CCP through get_symbol_constant_value causes <bb 2>: - _OBJC_CLASS_REFERENCES_0.2_1 = _OBJC_CLASS_REFERENCES_0; + _OBJC_CLASS_REFERENCES_0.2_1 = (struct objc_class *) &_OBJC_CLASS_NAME_0; _OBJC_CLASS_REFERENCES_0.3_2 = (struct objc_object *) _OBJC_CLASS_REFERENCES_0.2_1; - _OBJC_SELECTOR_REFERENCES_0.4_3 = _OBJC_SELECTOR_REFERENCES_0; + _OBJC_SELECTOR_REFERENCES_0.4_3 = (struct objc_selector *) &_OBJC_METH_VAR_NAME_1; D.3683_4 = OBJ_TYPE_REF(objc_msgSend;_OBJC_CLASS_REFERENCES_0.3_2->0) (_OBJC_CLASS_REFERENCES_0.3_2, _OBJC_SELECTOR_REFERENCES_0.4_3); - _OBJC_SELECTOR_REFERENCES_1.5_5 = _OBJC_SELECTOR_REFERENCES_1; + _OBJC_SELECTOR_REFERENCES_1.5_5 = (struct objc_selector *) &_OBJC_METH_VAR_NAME_0; OBJ_TYPE_REF(objc_msgSend;D.3683_4->0) (D.3683_4, _OBJC_SELECTOR_REFERENCES_1.5_5); return 0; which means that _OBJC_CLASS_REFERENCES_0 must be a TREE_READONLY static with an initializer. I wonder how exactly that tree looks like, so can you run the compile inside gdb, break on get_symbol_constant_value until you get _OBJC_CLASS_REFERENCES_0 as argument (may happen multiple times) and paste the output of (gdb) call debug_tree (sym) (gdb) call debug_tree (sym->decl_common.initial) ? Thanks.
(In reply to comment #8) > Hm. So CCP through get_symbol_constant_value causes > you run the compile inside gdb, break on get_symbol_constant_value maybe I've messed something up - but setting a break on get_symbol_constant_value .. .. the cc1obj just completes with "Program exited normally'.
(In reply to comment #9) > (In reply to comment #8) > > Hm. So CCP through get_symbol_constant_value causes > > > you run the compile inside gdb, break on get_symbol_constant_value > > maybe I've messed something up - but setting a break on > get_symbol_constant_value .. > .. the cc1obj just completes with "Program exited normally'. That can't be. Make sure to enable optimization though.
Btw, I cannot make -fnext-runtime work on i?86-linux, it errors at link time with undefined references. Any configure options I need to supply?
(In reply to comment #11) > Btw, I cannot make -fnext-runtime work on i?86-linux, it errors at link time > with > undefined references. Any configure options I need to supply? > The next runtime only works on Darwin.
Created attachment 19864 [details] gdb-output for CLASS_REFERENCES_0
That doesn't make sense. The symbol is not TREE_READONLY. Was that dump from inside get_symbol_constant_value? As the extract only happens from CCP2 I suppose that ipa-reference might be setting TREE_READONLY on the decl becaue it's static and not written to? So, can you try with -fno-ipa-reference? (-fdump-ipa-reference should show "read-only var OBJC_CLASS_REFERENCES_0" if that is the problem) Can it be that the next runtime causes functions to be emitted in the back of cgraph? Or maybe the objc frontend fails to set TREE_ADDRESSABLE on those vars? Or it forgets to pass them to the varpool? Can you attach the full ipa-reference dump and the full 004.gimple dump?
(In reply to comment #14) > That doesn't make sense. The symbol is not TREE_READONLY. > > Was that dump from inside get_symbol_constant_value? yes. that was from a clean bootstrap of trunk 156760. > As the extract only happens from CCP2 I suppose that ipa-reference might > be setting TREE_READONLY on the decl becaue it's static and not written to? > So, can you try with -fno-ipa-reference? (-fdump-ipa-reference should > show "read-only var OBJC_CLASS_REFERENCES_0" if that is the problem) -fdump-ipa-reference - is unrecognized on my build - do I need a config. option to enable? -fdump-ipa-all doesn't seem to give a file named as reference...
Created attachment 19866 [details] gimple for cascading-1.m @ trunk 156760
(In reply to comment #15) > (In reply to comment #14) > > That doesn't make sense. The symbol is not TREE_READONLY. > > > > Was that dump from inside get_symbol_constant_value? > > yes. > that was from a clean bootstrap of trunk 156760. > > > As the extract only happens from CCP2 I suppose that ipa-reference might > > be setting TREE_READONLY on the decl becaue it's static and not written to? > > So, can you try with -fno-ipa-reference? (-fdump-ipa-reference should > > show "read-only var OBJC_CLASS_REFERENCES_0" if that is the problem) > > -fdump-ipa-reference - is unrecognized on my build - do I need a config. option > to enable? > -fdump-ipa-all doesn't seem to give a file named as reference... Ah, sorry. The dump file is called static-var.
Created attachment 19867 [details] -fdump-ipa-all/static-var cascading-1.m @ trunk 156760 this is with normal options (i.e. the default for ipa-reference)
Created attachment 19868 [details] cascading-1.m/-fdump-ipa-all/pure-const @ trunk 15670 this is with -fno-ipa-reference.
Not TREE_ADDRESSABLE var _OBJC_CLASS_REFERENCES_0 Not TREE_ADDRESSABLE var _OBJC_SELECTOR_REFERENCES_0 Not TREE_ADDRESSABLE var _OBJC_SELECTOR_REFERENCES_1 read-only var _OBJC_CLASS_REFERENCES_0 read-only var _OBJC_SELECTOR_REFERENCES_0 read-only var _OBJC_SELECTOR_REFERENCES_1 So it's indeed ipa-reference that promotes the static vars and CCP that propagates the initializer. I see nothing wrong here, so the ObjC frontend must do something behind the middle-ends back. If you look at the final assembler code, is there anything modifying the contents of those three variables?
Created attachment 19870 [details] asm out from -O1 -g cascading-1.m @trunk 156760 this is the current asm output - it segfaults on the objc msg call.
There are no modifications visible in the assembly. But there is magic: .objc_cls_refs .align 2 L_OBJC_CLASS_REFERENCES_0: .long L_OBJC_CLASS_NAME_0 what is .objc_cls_refs? Well, obviously some Objective-C maintainer needs to chime in and explain things.
(In reply to comment #22) > There are no modifications visible in the assembly. But there is magic: > > .objc_cls_refs > .align 2 > L_OBJC_CLASS_REFERENCES_0: > .long L_OBJC_CLASS_NAME_0 > > what is .objc_cls_refs? DEF_SECTION (objc_cls_refs_section, SECTION_MERGE, ".objc_cls_refs", 1) If I've got this correct - this is implementing the constraint that class names are global in the NeXT runtime [at least at release 0/1]. I assume that the section merge arranges that all references to class FOO point to the same and unique class definition. > > Well, obviously some Objective-C maintainer needs to chime in and explain > things. indeed.
Yes, I think IainS is on the right track, all things in objc_cls_refs escape and can be read and written to in unexpected ways by the runtime. Setting TREE_ADDRESSABLE sounds like a reasonable step forward.
Hm. I tried this trivial patch: Index: gcc/objc/objc-act.c =================================================================== --- gcc/objc/objc-act.c (revision 156760) +++ gcc/objc/objc-act.c (working copy) @@ -2533,7 +2533,8 @@ sprintf (buf, "_OBJC_SELECTOR_REFERENCES_%d", selector_reference_idx++); decl = start_var_decl (objc_selector_type, buf); - + if (flag_next_runtime) + TREE_ADDRESSABLE (decl) = 1; return decl; } @@ -2733,6 +2734,8 @@ sprintf (buf, "_OBJC_CLASS_REFERENCES_%d", class_reference_idx++); decl = start_var_decl (objc_class_type, buf); + if (flag_next_runtime) + TREE_ADDRESSABLE (decl) = 1; return decl; } and when I check in gdb the trees are marked as addressable [checking in objc-act.c/finish_var_decl() ]. ... but something is un-doing this later - and _OBJC_CLASS_REFERENCES_0 is ending up marked as not tree addressable according to the ipa dump. any ideas where I'm going wrong?
Addressability is recomputed several times. What you probably want is mark the decl with the used attribute (i.e. add "used" attribute to it, set TREE_USED (decl) = 1 and DECL_PRESERVE_P (decl) = 1).
(In reply to comment #26) > Addressability is recomputed several times. What you probably want is mark the > decl with the used attribute (i.e. add "used" attribute to it, set TREE_USED > (decl) = 1 and DECL_PRESERVE_P (decl) = 1). finish_var_decl() contains: mark_decl_referenced(var); - although this doesn't do anything for csts. and TREE_USED (var) = 1; adding DECL_PRESERVE_P (decl) = 1 doesn't solve the problem either.. I'll have to dig deeper into what's actually happening/required.
DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("used"), NULL, DECL_ATTRIBUTES (decl)); is also needed (no idea why ipa*.c/cgraphunit.c use lookup_attribute instead of testing DECL_PRESERVED_P, but they do).
Created attachment 19884 [details] attach "used" attribute as well as marking vars used. Jakub's comment seems to do the trick - thanks. I've applied a "used" attribute in the same place that the TREE_USED is placed.
apropos http://gcc.gnu.org/ml/gcc-patches/2010-02/msg00587.html do you think that _OBJC_CLASS_REFERENCES_* and _OBJC_SELECTOR_REFERENCES_* should be marked as TREE_ADDRESSABLE and DECL_PRESERVE_P in anticipation that this change (ipa* and cgraphunit responding to PRESERVE_P) will occur in the future? (I took those changes out of this patch on the grounds that they didn't currently achieve anything).
Not sure about TREE_ADDRESSABLE, but certainly DECL_PRESERVED_P should be set as well.
(In reply to comment #24) > Yes, I think IainS is on the right track, all things in objc_cls_refs escape > and can be read and written to in unexpected ways by the runtime. Setting > TREE_ADDRESSABLE sounds like a reasonable step forward. Why are they marked TREE_READONLY then? That sounds like the bug.
(In reply to comment #32) > (In reply to comment #24) > > Yes, I think IainS is on the right track, all things in objc_cls_refs escape > > and can be read and written to in unexpected ways by the runtime. Setting > > TREE_ADDRESSABLE sounds like a reasonable step forward. > > Why are they marked TREE_READONLY then? That sounds like the bug. Err, forget that ;) Still early today...
Created attachment 19889 [details] patch with CLASS and SELECTOR refs. marked as TREE_ADDRESSABLE & DECL_PRESERVE_P (for NeXT runtime) In view of the comments in http://gcc.gnu.org/ml/gcc-patches/2010-02/msg00597.html and http://gcc.gnu.org/ml/gcc-patches/2010-02/msg00587.html it's clear that people feel that the bug is actually in ipa* and cgraphunit.c. However, a grep of lookup_attribute ipa* etc. suggests that altering these has possibly wide-reaching effects, which might not be welcome at this stage. I've added back in the marking of CLASS_REFERENCES_* and SELECTOR_REFERENCES_* as TREE_ADDRESSABLE and DECL_PRESERVE_P. Of course, this doesn't solve the problem unless the addition of the "used" attribute in finish_var_decl() is left in place too (although that can be removed once the underlying problem is resolved). Would it be best to close this bug with the lookup_attribute fix - and open a new bug for ipa*.c and cgraphunit.c to support DECL_PRESERVE_P and/or TREE_USED? In fact, I'm a bit confused about what combination of flags is really needed. TREE_USED appears to match in intent with "used" attribute - which is placed on the vars in objc-act.c/finish_var_decl(). DECL_PRESERVE_P presumably should only be used on the CLASS_REFERENCES/SELECTOR_REFERENCES - and thus I've placed it local to those two definitions.
Ok to the last patch for now. Feel free to file a bug about checking DECL_PRESERVE_P and add a pointer to remove the setting of DECL_ATTRIBUTES from the frontend, thanks.
I've checked in a slightly updated fix in r156877. Let us know if all the regressions are fixed now.
(In reply to comment #36) > I've checked in a slightly updated fix in r156877. Let us know if all the > regressions are fixed now. i686/powerpc-darwin9 - YES, thanks.
Subject: Bug 43061 Author: mrs Date: Fri Feb 19 19:06:38 2010 New Revision: 156907 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156907 Log: PR objc/43061 * cgraphunit.c (process_function_and_variable_attributes): Check DECL_PRESERVE_P instead of looking up attribute "used". * ipa-pure-const.c (check_decl): Likewise. * ipa-reference.c (has_proper_scope_for_analysis): Likewise. * ipa-type-escape.c (has_proper_scope_for_analysis): Likewise. * config/sol2.c (solaris_insert_attributes): Set DECL_PRESERVE_P instead of attribute "used". * config/sol2-c.c (solaris_pragma_init): Likewise. (solaris_pragma_fini): Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/cgraphunit.c trunk/gcc/config/sol2-c.c trunk/gcc/config/sol2.c trunk/gcc/ipa-pure-const.c trunk/gcc/ipa-reference.c trunk/gcc/ipa-type-escape.c trunk/gcc/objc/objc-act.c
I checked in the real back end change in r156907.
(In reply to comment #39) > I checked in the real back end change in r156907. OK on i686/powerpc d9
AFAICT this pr and the side effects have been fixed, hence closing. If someone disagree, please reopen. Thanks all for the work.