[ python-Bugs-1005248 ] new.code() not cleanly checking its arguments

[SourceForge.net]

Bugs item #1005248, was opened at 2004-08-07 20:01
Message generated for change (Comment added) made by arigo
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1005248&group_id=5470

Category: Python Interpreter Core
Group: None
Status: Closed
Resolution: Fixed
Priority: 5
Submitted By: Armin Rigo (arigo)
Assigned to: Michael Hudson (mwh)
Summary: new.code() not cleanly checking its arguments

Initial Comment:
new.code() does not check its arguments properly:

>>> new.code(1,1,1,0,"123",(None,),(5,),(5,),"123","321",2,"")
Fatal Python error: non-string found in code slot

Here the tuple (5,) is used for co_names and co_varnames.

More generally it looks like this part of compile.c could do with quite some cleaning up.

----------------------------------------------------------------------

>Comment By: Armin Rigo (arigo)
Date: 2004-08-13 08:30

Message:
Logged In: YES 
user_id=4771

Raymond said: not worth the burden.  The optimizer is "experimental and not fully developed".  It is also superfast, so it doesn't matter.  The AST branch will change all this anyway.

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2004-08-12 17:58

Message:
Logged In: YES 
user_id=6656

OK, checked in as

Lib/test/test_new.py revision 1.18
Python/compile.c revision 2.314

I also rewrote all the tests of new.code() to use realistic 
arguments.

> On a side note, PyCode_New() calls optimize_code(), but
> PyCode_New() is also used when un-marshalling .pyc files.

Yes, I noticed that too, and wondered some of the things you 
wondered.  Ask Raymond?  I think it's his code.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2004-08-12 16:39

Message:
Logged In: YES 
user_id=4771

Sounds reasonable.  The patch looks OK.

On a side note, PyCode_New() calls optimize_code(), but
PyCode_New() is also used when un-marshalling .pyc files.  I
wonder how much time is lost re-optimizing in vain the code
loaded from .pyc files, and I wonder why optimization is not
done just as the last step of the compiler instead.  While
I'm wondering, I also wonder if some user bytecode hacks
could be broken by optimize_code().

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2004-08-09 11:23

Message:
Logged In: YES 
user_id=6656

> Why do you check for argcount<0 but not nlocals<0, which
> also raises a SystemError?

Because I didn't read that far...

Obvious extension of patch added.

Agree that current behaviour of PyCode_New is a bit crazy.  
OTOH, not changing PyCode_New itself seems a principle no worse 
than any other for the moment.

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2004-08-09 10:54

Message:
Logged In: YES 
user_id=4771

Why do you check for argcount<0 but not nlocals<0, which also raises a SystemError?

More to the point, why on earth does PyCode_New() check for negative values of argcount and nlocals and not the other values?  A negative stacksize, for example, is likely to crash the interpreter too.  On the other hand, validating these numbers more precisely is much more complex, and not something we're trying to do here.  So I don't understand why these two values in particular are checked for < 0.  We should as well drop the check altogether.

My position here is that calling new.code() shouldn't do anything bad to the interpreter; but actually executing the resulting code object is allowed to destroy the sun (provided, of course, that CPython runs in an environment that allows that).

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2004-08-09 10:40

Message:
Logged In: YES 
user_id=6656

Maybe new is meant to be scary, but current behaviour is going 
well above and beyond the effort required.  new.code can mutate 
a tuple containing string subclasses into a tuple containing exact 
strings!

Armin, what do you think of the attached?

----------------------------------------------------------------------

Comment By: Anthony Baxter (anthonybaxter)
Date: 2004-08-08 13:58

Message:
Logged In: YES 
user_id=29957

I thought there was a general concensus that many things in
the 'new' module could be used to make a mess, and that
trying to fix them all was a pretty much open-ended task?


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1005248&group_id=5470