Description of problem: Current gcc apparently thinks it can discard the first assignment to a static variable in a sequence like staticvar = something; externalfunction(); staticvar = somethingelse; I suppose the theory is that the external function can't see the static variable, but this theory is wrong on at least two grounds: (1) the external function could call back into the current module, and (2) the external function could longjmp, in which case control doesn't reach the second assignment. This misoptimization is breaking postgresql in rawhide because of an instance of (2). The error doesn't seem to occur in perfectly straight-line cases like the above, but I'm observing it in code that involves setjmp/longjmp, as per attached example. Version-Release number of selected component (if applicable): gcc-4.6.2-1.fc17.1 How reproducible: 100% Steps to Reproduce: 1. Compile attached source code with gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -DLINUX_OOM_ADJ=0 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wformat-security -fno-strict-aliasing -fwrapv -fpic -I. -I. -I/usr/include/python2.7 -I../../../src/include -D_GNU_SOURCE -I/usr/include/libxml2 -c plpython.c 2. Examine assembly code. Note lack of any assignment corresponding to source line 159. Actual results: Assignment is removed from generated code. Expected results: Assignment should occur. Additional info: I'm seeing this on both x86_64 and i386. Compiling at -O0 makes it go away, as does declaring the static variable as global. The same code has been compiled successfully under many previous gcc versions; the last successful build in rawhide was in late September.
Created attachment 532907 [details] original source code, edited to remove extraneous functions
Created attachment 532908 [details] -E output, so you don't need a postgresql source tree to compile
Might be the leaf attribute that was recently added to glibc triggering the problem.
Yup. The __leaf__ attribute on the sigsetjmp prototype is causing FRE to decide it's OK to remove the "redundant" assignment to PLy_curr_procedure.
setjmp/__sigsetjmp/_setjmp definitely must be __THROWNL rather than __THROW. Similarly setcontext and swapcontext. They all have side-effects which can modify module static variables that don't have address taken.
(In reply to comment #5) > setjmp/__sigsetjmp/_setjmp definitely must be __THROWNL rather than __THROW. Oh ... perhaps that explains why it seemed that the behavioral change was triggered by the most recent glibc update. Could it be that they had them correctly declared previously, and reverted the change? See my previous complaint about this issue at bug #752883.
AFAIK for F16 the solution was just to revert the git commit that added leaf attribute to __THROW macro (glibc-2.14.90-14). Guess -15 (the .1 should be removed for -16 I'd say) this revert has been removed and instead the upstream patch from Andreas is in, but that doesn't handle setjmp etc. side-effects correctly.
(In reply to comment #4) > Yup. The __leaf__ attribute on the sigsetjmp prototype is causing FRE to > decide it's OK to remove the "redundant" assignment to PLy_curr_procedure. I don't pretend to understand this in any detail, but the function that's called between the two PLy_curr_procedure assignments doesn't have any such decoration. So isn't the optimization still wrong, regardless of how sigsetjmp is marked?
glibc-2.14.90-15.2 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/FEDORA-2011-15723
The glibc 15.2 update does fix this problem for me ... but I'm still wondering why the leaf attribute on sigsetjmp should have allowed removal of an assignment that was *after* that call.
One of the side-effects of the leaf attribute is that static CU local variables that don't have address taken can be considered unchanged during the call to a leaf function (changing such a variable can be done only from the same compilation unit and leaf attribute says that the function doesn't call anything in the current function). So the compiler can e.g. optimize: static int var_with_address_not_taken; ... var_with_address_not_taken = 5; leaf_function (); // where leaf_function () isn't defined in the current CU return var_with_address_not_taken; into ... var_with_address_not_taken = 5; leaf_function (); return 5; because the leaf function couldn't have changed the var_with_address_not_taken. Now, if you use leaf attribute on setjmp and do: var_with_address_not_taken = 5; if (!setjmp (&jmpbuf)) { .. var_with_address_not_taken = x; } return var_with_address_not_taken; gcc might optimize it similarly, thinking that var_with_address_not_taken couldn't have changed during the call. Now, certainly gcc could just ignore leaf attribute on functions with returns_twice attribute (or those that get it as builtins automatically), but GCC 4.6 (which introduced leaf attribute) wasn't doing that, nobody thought that somebody would mark functions with such side-effects as leaf. So it is just better to keep this on the glibc side where it has been introduced a few days ago.
(In reply to comment #11) > One of the side-effects of the leaf attribute is that static CU local variables > that don't have address taken can be considered unchanged during the call to a > leaf function (changing such a variable can be done only from the same > compilation unit and leaf attribute says that the function doesn't call > anything in the current function). Right, makes sense. But the case I was dealing with here was essentially if (setjmp (&jmpbuf)) { .. var_with_address_not_taken = x; non_leaf_function(); } var_with_address_not_taken = y; ISTM that removing the first assignment is wrong, because of the presence of the non-leaf function, no matter how setjmp() is marked. I still think there's an additional gcc bug lurking here.
No. You are dealing essentially with: localvar = var_with_address_not_taken; if (setjmp (&jmpbuf)) { var_with_address_not_taken = localvar; non_leaf_noreturn_function (); } else { ... } And, when you promise to the compiler that the setjmp call will not change var_with_address_not_taken by using leaf attribute for it, then the store to var_with_address_not_taken can be optimized away, because it is dead. The compiler has the right to assume that it already contains the value that the store would set it to (remember, var_with_address_not_taken isn't volatile, doesn't have address taken and the setjmp call with leaf attribute isn't supposed to call, even indirectly, anything in the current CU.
Oh, it's dependent on the value being stored. Got it. Thanks for the explanation.
Package glibc-2.14.90-15.2: * should fix your issue, * was pushed to the Fedora 16 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing glibc-2.14.90-15.2' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2011-15723 then log in and leave karma (feedback).
This package has changed ownership in the Fedora Package Database. Reassigning to the new owner of this component.
Package glibc-2.14.90-16: * should fix your issue, * was pushed to the Fedora 16 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing glibc-2.14.90-16' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2011-15723 then log in and leave karma (feedback).
glibc-2.14.90-18 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/glibc-2.14.90-18
Bogus updates which caused this problem have been reverted/unpushed (-15, -16).