Skip to content

geninterop.py doesn't handle c_ast.FuncDecl; creates broken interop38.cs #995

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.

Sign up for GitHub

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

Jump to bottom
Closed
Jeff17Robbins opened this issue Nov 27, 2019 · 7 comments
Closed

geninterop.py doesn't handle c_ast.FuncDecl; creates broken interop38.cs #995

Jeff17Robbins opened this issue Nov 27, 2019 · 7 comments

Comments

@Jeff17Robbins
Copy link
Contributor

Jeff17Robbins commented Nov 27, 2019 •
edited
Loading

Details

I'm concerned that the automated process pythonnet uses to generate the interop[version].cs file might be flawed. This is the relevant snippet from object.h

    unsigned int tp_version_tag;
    destructor tp_finalize;
    vectorcallfunc tp_vectorcall;
    /* bpo-37250: kept for backwards compatibility in CPython 3.8 only */
    Py_DEPRECATED(3.8) int (*tp_print)(PyObject *, FILE *, int);

this is how I mapped those fields in my handmade interop38.cs

        public static int tp_version_tag = 0;
        public static int tp_finalize = 0;
        public static int tp_vectorcall = 0;
        public static int tp_print = 0;
        public static int am_await = 0;

but here's how the auto-generated interop38.cs maps them

        public static int tp_version_tag = 0;
        public static int tp_finalize = 0;
        public static int tp_vectorcall = 0;
        public static int am_await = 0;

Notice the missing public static int tp_print = 0;

I'm thinking that the Py_DEPRECATED(3.8) int (*tp_print)(PyObject *, FILE *, int); messed up the auto-gen technique? Won't all fields following tp_vectorcall be offset incorrectly due to this?
@filmor
Copy link
Member

filmor commented Nov 27, 2019

If I get https://docs.python.org/3/c-api/typeobj.html#c.PyVectorcall_Call correctly, tp_vectorcall replaces tp_print. I'll check what happens in this header.

@Jeff17Robbins
Copy link
Contributor Author

Jeff17Robbins commented Nov 27, 2019 •
edited
Loading

[EDITED]I think Py_ssize_t tp_vectorcall_offset; replaced the previous location of public static int tp_print = 0;, and then

vectorcallfunc tp_vectorcall;
Py_DEPRECATED(3.8) int (*tp_print)(PyObject *, FILE *, int);

were added after public static int tp_finalize = 0;

Since qualname is way at the end, I think I can come up with a test to show that the auto-gen'ed offset for it (qualname) is either correct or incorrect.

@Jeff17Robbins
Copy link
Contributor Author

typemanager.cs assigns the type's name into qualname via this code

#if PYTHON3
            // For python3 we leak two objects. One for the ASCII representation
            // required for tp_name, and another for the Unicode representation
            // for ht_name.
            IntPtr temp = Runtime.PyBytes_FromString(name);
            IntPtr raw = Runtime.PyBytes_AS_STRING(temp);
            temp = Runtime.PyUnicode_FromString(name);
#elif PYTHON2
            IntPtr temp = Runtime.PyString_FromString(name);
            IntPtr raw = Runtime.PyString_AsString(temp);
#endif
            Marshal.WriteIntPtr(type, TypeOffset.tp_name, raw);
            Marshal.WriteIntPtr(type, TypeOffset.name, temp);

#if PYTHON3
            Marshal.WriteIntPtr(type, TypeOffset.qualname, temp);
#endif

The repo's current interop38.cs results in 0x358 as the value of TypeOffset.qualname. But this is wrong...it should be 0x360.

You can see it crash in the REPL

>>> import clr
>>> import System
>>> System.String.__qualname__

@Jeff17Robbins Jeff17Robbins changed the title interop38.cs might have a problem interop38.cs has a problem Nov 27, 2019
@Jeff17Robbins
Copy link
Contributor Author

If you manually fix interop38.cs to add the missing field

>>> import clr
>>> import System
>>> System.String.__qualname__
'String'

@Jeff17Robbins
Copy link
Contributor Author

pycparser doesn't support __attribute__ (and presumably also doesn't support __declspec)

https://github.com/eliben/pycparser/wiki/FAQ#does-pycparser-support-gnuvisual-c-extensions

so I'm guessing that geninterop.py threw a warning when interop38.cs was generated. I'm having trouble running geninterop.py on my Windows machine, but I can see that clang.exe did the preprocess step and then pycparser isn't liking the mixture of slashes and backslashes.

But ignoring my Windows issues, there's a problem in the automated workflow producing interop38.cs.

@Jeff17Robbins
Copy link
Contributor Author

I got geninterop.py to run on Windows by adding more things for clang to ignore. But still was missing tp_print. Then I realized what was wrong:

the visit function was missing a case for c_ast.FuncDecl, that's what int (*tp_print)(PyObject *, FILE *, int); has inside the c_ast.PtrDecl.

I added this case to visit

    def visit(self, node):
        if isinstance(node, c_ast.FileAST):
            self.visit_ast(node)
        elif isinstance(node, c_ast.Typedef):
            self.visit_typedef(node)
        elif isinstance(node, c_ast.TypeDecl):
            self.visit_typedecl(node)
        elif isinstance(node, c_ast.Struct):
            self.visit_struct(node)
        elif isinstance(node, c_ast.Decl):
            self.visit_decl(node)
        elif isinstance(node, c_ast.PtrDecl):
            self.visit_ptrdecl(node)
        elif isinstance(node, c_ast.FuncDecl):
            self.visit_funcdecl(node)
        elif isinstance(node, c_ast.IdentifierType):
            self.visit_identifier(node)

and implemented it as

    def visit_funcdecl(self, funcdecl):
        self.visit(funcdecl.type)

And now I get the correct mapping struct, with a tp_print member.

So this is a bug in geninterop.py.

For those curious, this is how pycparser prints out the node for tp_print:

decl=Decl(name='tp_print',
     quals=[
           ],
     storage=[
             ],
     funcspec=[
              ],
     type=PtrDecl(quals=[
                        ],
                  type=FuncDecl(args=ParamList(params=[Typename(name=None,
                                                                quals=[
                                                                      ],
                                                                type=PtrDecl(quals=[
                                                                                   ],
                                                                             type=TypeDecl(declname=None,
                                                                                           quals=[
                                                                                                 ],
                                                                                           type=IdentifierType(names=['PyObject'
                                                                                                                     ]
                                                                                                               )
                                                                                           )
                                                                             )
                                                                ),
                                                       Typename(name=None,
                                                                quals=[
                                                                      ],
                                                                type=PtrDecl(quals=[
                                                                                   ],
                                                                             type=TypeDecl(declname=None,
                                                                                           quals=[
                                                                                                 ],
                                                                                           type=IdentifierType(names=['FILE'
                                                                                                                     ]
                                                                                                               )
                                                                                           )
                                                                             )
                                                                ),
                                                       Typename(name=None,
                                                                quals=[
                                                                      ],
                                                                type=TypeDecl(declname=None,
                                                                              quals=[
                                                                                    ],
                                                                              type=IdentifierType(names=['int'
                                                                                                        ]
                                                                                                  )
                                                                              )
                                                                )
                                                      ]
                                               ),
                                type=TypeDecl(declname='tp_print',
                                              quals=[
                                                    ],
                                              type=IdentifierType(names=['int'
                                                                        ]
                                                                  )
                                              )
                                )
                  ),
     init=None,
     bitsize=None
     )

@Jeff17Robbins Jeff17Robbins changed the title interop38.cs has a problem geninterop.py doesn't handle c_ast.FuncDecl; creates broken interop38.cs Nov 27, 2019
@Jeff17Robbins Jeff17Robbins mentioned this issue Nov 28, 2019
Closed
@filmor
Copy link
Member

filmor commented Nov 28, 2019

Fixed in 627cac0.

@filmor filmor closed this as completed Nov 28, 2019
@Jeff17Robbins Jeff17Robbins mentioned this issue Dec 17, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@filmor @Jeff17Robbins