Re: [HACKERS] codlin_month is up and complain - PL/Python crash

2010-02-18 Thread Tom Lane
Zdenek Kotala  writes:
> Dne 17.02.10 18:39, Peter Eisentraut napsal(a):
>> FWIW, this is a Sun Studio build that is complaining here.

> Yes It is SS12. I add volatile keyword and problem disappears.

OK, I've applied that change in CVS.  Please change codlin_moth back to
the higher optimization setting.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] codlin_month is up and complain - PL/Python crash

2010-02-18 Thread Zdenek Kotala

Dne 17.02.10 18:39, Peter Eisentraut napsal(a):

On ons, 2010-02-17 at 11:26 -0500, Tom Lane wrote:

But the behavior gcc appears to exhibit is that it won't warn about
variables that are only assigned once before the PG_TRY is entered,
and that seems reasonable to me since such a variable ought to have
the correct value either way.


FWIW, this is a Sun Studio build that is complaining here.



Yes It is SS12. I add volatile keyword and problem disappears. The code 
difference is following:



< PLy_spi_execute+0x742:  83 ec 0c   subl   $0xc,%esp
< PLy_spi_execute+0x745:  ff b5 b8 f9 ff ff  pushl  0xf9b8(%ebp)
< PLy_spi_execute+0x74b:  e8 fc ff ff ff call   MemoryContextSwitch


> PLy_spi_execute+0x742:  8b 85 cc f9 ff ff  movl 
0xf9cc(%ebp),%eax

> PLy_spi_execute+0x748:  83 ec 0c   subl   $0xc,%esp
> PLy_spi_execute+0x74b:  50 pushl  %eax
> PLy_spi_execute+0x74c:  e8 fc ff ff ff call   MemoryContextSwitch

Good to mention that SS inline PLy_spi_execute_query inside 
PLy_spi_execute(), because it is only one caller.



Zdenek

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] codlin_month is up and complain - PL/Python crash

2010-02-17 Thread Peter Eisentraut
On ons, 2010-02-17 at 11:26 -0500, Tom Lane wrote:
> But the behavior gcc appears to exhibit is that it won't warn about
> variables that are only assigned once before the PG_TRY is entered,
> and that seems reasonable to me since such a variable ought to have
> the correct value either way. 

FWIW, this is a Sun Studio build that is complaining here.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] codlin_month is up and complain - PL/Python crash

2010-02-17 Thread Tom Lane
Peter Eisentraut  writes:
> On ons, 2010-02-17 at 11:05 -0500, Tom Lane wrote:
>> Since oldcontext is only set in the one place, it really shouldn't
>> require "volatile" decoration, but maybe it does.

> It is my understanding that local automatic variables may be clobbered
> by [sig]longjmp unless they are marked volatile.  The PG_CATCH branch is
> reached by means of a [sig]longjmp.  So that would mean that any
> variable that you want to use both before the TRY and inside the CATCH
> has to be volatile.

If the rule were quite that strict then we'd need many more "volatile"
markers than we have.  I believe the actual implementation issue is that
longjmp restores the register contents to what they were at the time of
the setjmp call, and thus a variable allocated in a register would get
restored to the value it had at entry to PG_TRY whereas a variable
allocated on the stack would still have an up-to-date value.  Now the
picture isn't quite that simple since a sufficiently smart compiler
might move the variable's value around within the routine.  But the
behavior gcc appears to exhibit is that it won't warn about variables
that are only assigned once before the PG_TRY is entered, and that seems
reasonable to me since such a variable ought to have the correct value
either way.

It might be interesting to modify these bits of code so that the
oldcontext variables are assigned only at declaration:

MemoryContext oldcontext = CurrentMemoryContext;

...
PG_TRY();

and see if that makes the issue go away.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] codlin_month is up and complain - PL/Python crash

2010-02-17 Thread Peter Eisentraut
On ons, 2010-02-17 at 11:05 -0500, Tom Lane wrote:
> All of the MemoryContextSwitchTo calls in plpython seem to be in
> patterns like this:
> 
> MemoryContext oldcontext;
> 
> oldcontext = CurrentMemoryContext;
> PG_TRY();
> {
> ... do something ...
> }
> PG_CATCH();
> {
> MemoryContextSwitchTo(oldcontext);
> 
> Since oldcontext is only set in the one place, it really shouldn't
> require "volatile" decoration, but maybe it does.

It is my understanding that local automatic variables may be clobbered
by [sig]longjmp unless they are marked volatile.  The PG_CATCH branch is
reached by means of a [sig]longjmp.  So that would mean that any
variable that you want to use both before the TRY and inside the CATCH
has to be volatile.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] codlin_month is up and complain - PL/Python crash

2010-02-17 Thread Tom Lane
Zdenek Kotala  writes:
> I revived codlin_month and it falls during PL/Python test:
> http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=codlin_moth&dt=2010-02-16%2015:09:05

All of the MemoryContextSwitchTo calls in plpython seem to be in
patterns like this:

MemoryContext oldcontext;

oldcontext = CurrentMemoryContext;
PG_TRY();
{
... do something ...
}
PG_CATCH();
{
MemoryContextSwitchTo(oldcontext);

Since oldcontext is only set in the one place, it really shouldn't
require "volatile" decoration, but maybe it does.  Can you do some
testing to see if that would fix it?

(Of course, really plpython's bogus approach to error handling ought
to get thrown out and rewritten from scratch, but that's not happening
right now.)

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers