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

Do Not Allow Static Objects to be Deallocated #101265

Open
ericsnowcurrently opened this issue Jan 23, 2023 · 6 comments
Open

Do Not Allow Static Objects to be Deallocated #101265

ericsnowcurrently opened this issue Jan 23, 2023 · 6 comments
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Jan 23, 2023

The runtime currently has many objects that are statically defined. See Include/internal/pycore_global_objects.h. For the singletons, all instances of the type are statically defined. Otherwise only some instances are.

(affected types)

The following types are those that have only some statically defined instances (not counting deep-frozen objects):

  • int
  • bytes
  • str
  • tuple

Deepfreeze adds the following:

  • float
  • complex
  • slice [actually not added]
  • code
  • (more bytes, int, tuple, & str objects)

The problem is that if tp_dealloc() for one of the affected types tries to free a static object then it will crash. We've set the refcount for such objects to a really high number to avoid this, but it is still possible. The risk rises a bit for immortal objects (see PEP 683).

For all types with static objects, we need to add a check in tp_dealloc() to either fail or reset the refcount to the really high number. We already have a check like this for the singletons and for str. For the other types we need to identify if the object is static and respond accordingly. The catch is that identifying that can be expensive, which is problematic for types that are deallocated frequently.

@ericsnowcurrently ericsnowcurrently added type-feature A feature request or enhancement 3.12 bugs and security fixes labels Jan 23, 2023
@ericsnowcurrently
Copy link
Member Author

Possible solutions:

  • check address range
  • check a per-object flag
(We'd probably hide the check behind one or more internal/static helper functions.)
int _Py_IsStaticObject(PyObject *obj);

// or, in the respective Objects/*.c files:

int is_static_bytes(PyObject *obj);
int is_static_int(PyObject *obj);
int is_static_tuple(PyObject *obj);
...

(For efficiency those could also be static inline or even macros.)

Check Address Range

An "obvious" solution is to check if the pointer is in the address range of known static objects (e.g. _PyRuntime.static_objects). However, the C standard says that checking if a pointer is in an address range is undefined. We've have to get clever about it.

Per Raymond Chen, for architectures with "flat" addressing and where the compiler's pointer-to-integer conversion produces the numeric value of the linear address, we can convert-then-compare for address ranges. So we'd add something to Include/pyport.h to encapsulate that.

For architectures/compilers that don't support that, we'd have to do a linear search for the address.

Check a Per-Object Flag

For every static object we would set a bit somewhere on the object indicating that it is static. Then we could check that.

@ericsnowcurrently
Copy link
Member Author

@gvanrossum
Copy link
Member

We already have a check like this for the singletons and for str.

How is it done for str? Is there a spare bit in the string object?

Also I've lost track of the draft PR where Eddie's work is happening. Can someone link to it?

@erlend-aasland
Copy link
Contributor

Also I've lost track of the draft PR where Eddie's work is happening. Can someone link to it?

@kumaraditya303
Copy link
Contributor

Deepfreeze adds the following: slice

Unless I missed something, deepfreeze does not generates static slice objects.

@eduardo-elizondo
Copy link
Contributor

eduardo-elizondo commented Jan 24, 2023

Unless I missed something, deepfreeze does not generates static slice objects.

Correct, there's no slice object in deepfreeze, it's these types: bytes, code, complex, float, int, tuple, and str

How is it done for str?

Upstream we have unicode_is_singleton but it's not all-inclusive at the moment. The function only compiles in debug mode and it only checks for a certain category of static strings (like the empty string and some single code point unicode objects)

In the Immortal PR, I added extra signaling by extending PyASCIIObject.state.interned from 1 bit to 2 bits to include: SSTATE_INTERNED_IMMORTAL. This works for deepfreeze str objects since we call _PyStaticCode_Init which then interns these objects, though I don't think we do this for pycore_global_objects .h yet

This gives us two options for strings, either guaranteeing a call to intern all static strings (through the interpreter initialization), or just adding an extra bit in the unicode state, say: PyASCIIObject.state.static

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

6 participants