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

Inlined functions from CPython API #55

Closed
karlotness opened this issue Mar 7, 2023 · 3 comments
Closed

Inlined functions from CPython API #55

karlotness opened this issue Mar 7, 2023 · 3 comments

Comments

@karlotness
Copy link

Thanks for the project, seems like a very useful tool and it's nice to have it to check things over.

I notice in the README you mention that abi3audit also checks symbols for inlined non-exported functions (giving _Py_DECREF) as an example. I think I've run into some issues with those checks.

In particular, I think there may be an issue with the tool flagging some functions that are part of the limited API and implemented as static inline functions vs. those which are imported from CPython and part of the stable ABI (and explicitly listed in the manifest).

I wonder if the checks looking at symtab might be too strict?

For example, I've tested this on a limited ABI wheel, that sets the Py_LIMITED_API macro. If I disable optimizations (to keep functions from being inlined) I see errors for:

  • _Py_INCREF
  • _Py_DECREF
  • _Py_IS_TYPE

which, from what I can tell, are implemented in Python 3.10 as static inline functions (and were in 3.8 as well)

It does seem like CPython anticipates these being used in limited API modules even if they aren't in the manifest.

A bit of digging on those trying to double check:

_Py_INCREF and _Py_DECREF have preprocessor conditions internally depending on the limited API state in Python 3.10. The Py_{INC,DEC}REF macros are also mentioned in PEP683 as a consideration for implementing immortal objects and the _Py_{INC,DEC}REF are part of their inlined implementations.

The _Py_IS_TYPE comes from a few places: such as a use of PyCapsule_CheckExact (where the *_Check macros are mentioned in PEP 384) and PyObject_TypeCheck (which is implemented with Py_IS_TYPE and PyType_IsSubtype which is part of the limited ABI). Py_IS_TYPE also gets special limited API handling on the current main branch.

Thanks again!

@woodruffw
Copy link
Member

Thanks @karlotness!

Yeah, there's a good chance that abi3audit is more conservative than strictly necessary here -- CPython is somewhat malleable in terms of its stability guarantees (as a function of how difficult it is to control API/ABI leakage), so abi3audit only tests for the things that it knows are considered stable.

Those static inline functions are an interesting case -- I think you're right that they alone shouldn't be considered an ABI violation, since they can appear when their implementation is solely the stable ABI implementation.

Do you have a shared object/Python wheel I can test with?

@karlotness
Copy link
Author

Thanks for taking a look. Here's one I was testing with (attached), built from here at -O0

adrt-1.0.0.dev0-cp310-abi3-linux_x86_64.whl.zip

I did patch the limited api definition to #define Py_LIMITED_API 0x030A0000 so it would match the wheel abi tag, but I get the same results building with 3.10 with the tag for 3.8, as it is in that commit.

@woodruffw
Copy link
Member

Closing in favor of #85, where we'll be tracking a general fix for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants