Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

object.__new__() crash #1203

Closed
pyway opened this issue Apr 27, 2015 · 10 comments
Closed

object.__new__() crash #1203

pyway opened this issue Apr 27, 2015 · 10 comments
Labels

Comments

@pyway
Copy link

pyway commented Apr 27, 2015

object.new(int) will cause a crash on msys2 or linux , maybe this is not a right usage, but should do something to void the crash.

@gst
Copy link

gst commented Apr 27, 2015

Hi,
just following your project from a high view point.

This doesn't "crash" in python2.7 as well as python3.4 but it's anyway not possible to do it:

>>> object.__new__(int)
Traceback (most recent call last):
  File "<pyshell#1>", line 1, in <module>
    object.__new__(int)
TypeError: object.__new__(int) is not safe, use int.__new__()
>>> 

(exact same exception in both python versions)

not sure what you wanted to achieve initially ..

@pfalcon pfalcon added the bug label Apr 27, 2015
@pfalcon
Copy link
Contributor

pfalcon commented Apr 27, 2015

@pyway : This is known issue, I thought we have a ticket for it, but seems we don't, but that's Known Issue No.1 as described in https://github.com/micropython/micropython/wiki/Differences . Yes, this needs to be fixed - eventually. Not fixed so far as we didn't find (well, didn't try to find) elegant way to solve it. Non-elegant is to add checks to each and every function, regress performance, grow binary size by 10% - all to reinsure against usage which makes no sense and thus won't be sensibly used by anyone.

@pyway
Copy link
Author

pyway commented Apr 29, 2015

hi, pfalcon, really maybe there is a "regress performance" problem...to the related source code, the c function has a assert staement, i thought c assert is not suitable for upy level operations...

@pfalcon
Copy link
Contributor

pfalcon commented May 3, 2015

Yes, all asserts are essentially TODO markers.

@pfalcon
Copy link
Contributor

pfalcon commented May 3, 2015

@dpgeorge : Do you know of ticket which better covers this problem? As I wrote above, I didn't find one - I guess we just always kept this issue in mind, but it wasn't ticketed. If so, should we rename this ticket and start to brainstorm how to deal with it?

@dpgeorge
Copy link
Member

dpgeorge commented May 3, 2015

@pfalcon no there is no ticket for this general problem (whose archetypal example of a crash is list.append(1, 2)).

But, I consider this bug not to fall under that category for 2 reasons:

  1. object.new is a staticmethod and therefore should not make assumptions about its first arg.
  2. The assertion at the start of instance_make_new is not strict enough, there should also be the following assertion: assert(is_instance_type(self));. With that assertion added the OP's example fails the assertion instead of crashing.

BTW, it may not be that hard to provide a generic check for the "self" argument to guard against list.append(1, 2). For load_method opcodes no check is needed because you are always guaranteed that "self" is the correct type. It's only when load_attr is called to get a non-static and non-class method (ie an instance method) that checking needs to take place on the call. These cases are rare and we could wrap the returned function in a checker. (I think this is all related to your question on py-dev about magic static methods: https://mail.python.org/pipermail/python-dev/2015-March/138950.html)

@dpgeorge
Copy link
Member

dpgeorge commented May 3, 2015

In #1215 is my proposed fix for this.

@pfalcon
Copy link
Contributor

pfalcon commented May 3, 2015

@pfalcon no there is no ticket for this general problem (whose archetypal example of a crash is list.append(1, 2)).

Split off to #1216, so we can close this.

dpgeorge added a commit that referenced this issue May 4, 2015
@dpgeorge
Copy link
Member

dpgeorge commented May 4, 2015

Fixed.

@dpgeorge dpgeorge closed this as completed May 4, 2015
@pyway
Copy link
Author

pyway commented May 6, 2015

test object.new(int),object.new(type), object.new("abc"), object.new(123)...
ok. does as below

class d:
pass
object.new(d)

tannewt added a commit to tannewt/circuitpython that referenced this issue Sep 21, 2018
nrf: multiple i2c and spi instances; improve nrfx_config.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants