Have a class that defines a placement new like varient that updates a pointer to raw storage via a reference argument. Here's a stripped down code fragment: #include <stddef.h> // size_t class T { public: void *operator new(size_t size, char *&p); T( int &rc); } ; void *T::operator new(size_t size, char *&p) { void *o; o = (void *) p; p += size; return(o); } T * f ( char *& cur_vardatap ) { int rc ; T * subTypep = new((char *&)cur_vardatap) T( rc ); return subTypep ; } If I build this code with GCC4.3 it doesn't want to generate code that calls it: r2.C: In function 'T* f(char*&)': r2.C:29: error: no matching function for call to 'T::operator new(long unsigned int, char*)' r2.C:13: note: candidates are: static void* T::operator new(size_t, char*&) This code seems a bit fishy to me (casting the input parameter to a reference type seems odd, and I'm wondering if that cast was added because some other compiler wouldn't call this operator new either). This can be worked around this easily enough by changing the code in question to call global placement new and then increment cur_vardatap by sizeof(T) afterwards (this specialized new operator is only called in two places so in all honesty I don't know why the original developer bothered doing all this). However, regardless of whether this is recoded, is GCC4.3 is correct rejecting this, or is this a GCC 4.3 regression? GCC 4.2 and previous compilers accept this syntax (as do a number of other compilers, IBM xlC, Sun WSPro, intel icpc, msdev, ...)
Apparently caused by my PR33025 fix, will look into this.
Actually, only the diagnostics is a regression introduced by PR33025 fix. Before that (but after PR29286 fix) at -O1 and higher cc1plus ICEd in gimplify_expr and at -O0 it silently miscompiled e.g.: typedef __SIZE_TYPE__ size_t; extern "C" void abort (); struct T { void *operator new (size_t, char *&); T () { i[0] = 1; i[1] = 2; } int i[2]; }; void *T::operator new (size_t size, char *&p) { void *o = (void *) p; p += size; return o; } T *f (char *&x) { return new (x) T (); } int main () { char buf[10 * sizeof (T)] __attribute__((aligned (__alignof (T)))); char *p = buf; f (p); if (p != buf + sizeof (T)) abort (); } Here get_temp_regvar created a temporary for the placement expr and that temporary was passed by reference to operator new. That means only the temporary was modified, not the user variable. Do we still need avoid_placement_new_aliasing stuff now that we have PR33407 fix in? If yes, we either need to do replacement of the original expression with get_target_expr after build_new_method_call/build_operator_new_call (and replace it both in the newly created argument list, in placement and placement_expr, and only if the pointer was passed by value, not by reference), or somehow undo this target_expr if a reference is needed.
To answer my question, new16.C fails if the build_new_1 setting of placement_expr and both calls to avoid_placement_new_aliasing are commented out. I believe PR33407 is meant to handle this, unfortunately there is nothing that prevents coalescing DECL_NO_TBAA_P variables with non-DECL_NO_TBAA_P ones. At *.cleanup_cfg2 time we have in the inner loop: lD.2031_7 = (intD.2 *) pD.2025_6(D); *lD.2031_7 ={v} 0; D.2058_13 = pD.2025_6(D); D.2036_8 = D.2058_13; fD.2029_9 = (long intD.5 *) D.2036_8; *fD.2029_9 ={v} -1; iD.2030_10 = iD.2030_2 + 1; where D.2058 is DECL_NO_TBAA_P. But: Try : D.2058_13(P13) & p_6(D)(P6) : Incompatible types. No coalesce. Try : D.2036_8(P8) & D.2058_13(P13) --> P8 D.2036 Coalesced D.2058_13 to D.2036_8 already at *.copyrename1 time and DECL_NO_TBAA_P is lost. If DECL_NO_TBAA_P decls are lost this easily, I guess PR33407 was fixed just by luck that in that case it is not optimized out away.
I guess we need to treat DECL_NO_TBAA_P in more places to make it really work, I suppose the CHANGE_DYNAMIC_TYPE_EXPR fix also doesn't work in all cases for the same reason. But as it is mainly the may_alias passes that need the bit, we can probably use something like a NEW_STMT <target_ptr, expr> that is not a GIMPLE assignment. Oh well.
We should possibly separate the two issues, rejects-valid and wrong-code, as only rejects-valid is a regression.
The c#2 testcase compiles and works with 4.1.x, so all of rejects-valid (rev 127639+), wrong-code (rev 125653 .. rev 127638 at -O0) and ice-on-valid-code (rev 125653 .. rev 127638 at -O2) are regressions.
Created attachment 15036 [details] DECL_NO_TBAA patch With regard to comment #3, I just bootstrapped and tested this patch on i686-pc-linux-gnu. Any opinions on whether this should go into 4.3? It seems to me that it should, to avoid any problems with inlining operator new. I believe this patch is safe, but I don't have a test case which it fixes.
Hm, isn't it even 'correct' to propagate a DECL_NO_TBAA_P pointer into uses of a non-DECL_NO_TBAA_P pointer? Of course this shouldn't happen, as then the IL would be wrong. I suppose we can even enforce this via verifying that such copies do not exist (for 4.4). Otherwise I think the patch should go in 4.3, too. Thanks. Richard.
Ian, the testcase would be new16.C after removing CHANGE_DYNAMIC_TYPE_EXPR stuff (to fix this PR). Or do you propose to keep CHANGE_DYNAMIC_TYPE_EXPR in 4.3 (which looks redundant with PR33407) and just hack it up to avoid doing it if operator new second argument is a reference to pointer rather than a pointer?
I'm not proposing to remove CHANGE_DYNAMIC_TYPE_EXPR from 4.3 at this point. We have some experience with it in, and I don't think we should take it out. I haven't looked at the actual bug here yet, I was responding to your point in comment #3, where I think you found a theoretical possibility which could break the patch for PR 33407.
Subject: Bug 34862 Author: ian Date: Mon Jan 28 19:43:51 2008 New Revision: 131916 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131916 Log: PR c++/34862 PR c++/33407 * tree-ssa-copyrename.c (copy_rename_partition_coalesce): Don't coalesce pointers if they have different DECL_NO_TBAA_P values. * tree-ssa-copy.c (may_propagate_copy): Don't propagate copies between variables with different DECL_NO_TBAA_P values. Modified: trunk/gcc/ChangeLog trunk/gcc/tree-ssa-copy.c trunk/gcc/tree-ssa-copyrename.c
Created attachment 15039 [details] Possible patch Here is a very ugly patch which appears to fix the problem in mainline. I haven't tested it, though. Any opinions?
Subject: Bug 34862 Author: jakub Date: Tue Feb 12 16:25:47 2008 New Revision: 132257 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132257 Log: PR c++/34862 * init.c (build_new_1): Don't create placement_expr before constructing alloc_call. Verify that the pointer is passed by value to operator new. * g++.dg/init/new27.C: New test. Added: trunk/gcc/testsuite/g++.dg/init/new27.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/init.c trunk/gcc/testsuite/ChangeLog
Fixed. For 4.4 we should nuke CHANGE_DYNAMIC_TYPE_EXPR.