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

Incorrect error handling in codegenerator.py file, calc_packing function if packing of a structure is failed #332

Open
rafikor opened this issue Jul 27, 2022 · 14 comments
Labels
help wanted Extra attention is needed

Comments

@rafikor
Copy link

rafikor commented Jul 27, 2022

If structure packing is failed, then function calc_packing is failed too due to incorrect error handing ("local variable 'details' referenced before assignment" in line 137).
Here is proposed patch to fix (can't create branch for merge request). Last proposed change in line 552 is needed to skip failed structure, otherwise type library wouldn't loaded even after fixing "details" variable (oading would fail in generated code from gen directory in line like assert sizeof(SomeStructure) == 24, sizeof(SomeStructure)).

diff --git a/comtypes/tools/codegenerator.py b/comtypes/tools/codegenerator.py
index 708b7b4..c1d898e 100644
--- a/comtypes/tools/codegenerator.py
+++ b/comtypes/tools/codegenerator.py
@@ -126,7 +126,8 @@ def calc_packing(struct, fields):
     for pack in [None, 16*8, 8*8, 4*8, 2*8, 1*8]:
         try:
             _calc_packing(struct, fields, pack, isStruct)
-        except PackingError as details:
+        except PackingError as e:
+            details = e
             continue
         else:
             if pack is None:
@@ -549,6 +550,7 @@ class Generator(object):
                 warnings.warn(message, UserWarning)
                 print("# WARNING: %s" % details, file=self.stream)
                 self.last_item_class = False
+                return # do not process struct with failed packing, otherwise generated code for type library will fail on e.g. sizeof asserts during its loading
 
         if fields:
             if body.struct.bases:
@rafikor rafikor changed the title Incorrect error handling if structure packing is failed Incorrect error handling in codegenerator file, calc_packing function if structure packing is failed Jul 27, 2022
@rafikor rafikor changed the title Incorrect error handling in codegenerator file, calc_packing function if structure packing is failed Incorrect error handling in codegenerator.py file, calc_packing function if structure packing is failed Jul 27, 2022
@rafikor rafikor changed the title Incorrect error handling in codegenerator.py file, calc_packing function if structure packing is failed Incorrect error handling in codegenerator.py file, calc_packing function if structure packing is failed Jul 27, 2022
@rafikor rafikor changed the title Incorrect error handling in codegenerator.py file, calc_packing function if structure packing is failed Incorrect error handling in 'codegenerator.py' file, 'calc_packing' function if packing of a structure is failed Jul 27, 2022
@rafikor rafikor changed the title Incorrect error handling in 'codegenerator.py' file, 'calc_packing' function if packing of a structure is failed Incorrect error handling in codegenerator.py file, calc_packing function if packing of a structure is failed Jul 27, 2022
@junkmd
Copy link
Collaborator

junkmd commented Jul 27, 2022

Is this error thrown by calling CreateObject or GetModule?

If so, please write down the code and/or traceback so that I may be able to assist you in resolving this issue.

@rafikor
Copy link
Author

rafikor commented Jul 28, 2022

Error is thrown in both cases (CreateObject calls GetModule if parameter 'dynamic' is False as in default case). But when GetModule is called with absolute path of *.tlb file, then the error does not occur. It is strange for me, I thougth that processing of *.tlb is the same through registry or absolute path.

Traceback for app=comtypes.client.CreateObject('SomeApp.Application'):

---------------------------------------------------------------------------
UnboundLocalError                         Traceback (most recent call last)
<ipython-input-2-86e7177bf653> in <module>()
----> 1 app=cc.CreateObject('SomeApp.Application')

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\client\__init__.py in CreateObject(progid, clsctx, machine, interface, dynamic, pServerInfo)
    248     if dynamic:
    249         return comtypes.client.dynamic.Dispatch(obj)
--> 250     return _manage(obj, clsid, interface=interface)
    251 
    252 def CoGetObject(displayname, interface=None, dynamic=False):

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\client\__init__.py in _manage(obj, clsid, interface)
    186     obj.__dict__['__clsid'] = str(clsid)
    187     if interface is None:
--> 188         obj = GetBestInterface(obj)
    189     return obj
    190 

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\client\__init__.py in GetBestInterface(punk)
    108 
    109     # import the wrapper, generating it on demand
--> 110     mod = GetModule(tlib)
    111     # Python interface class
    112     interface = getattr(mod, itf_name)

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\client\_generate.py in GetModule(tlib)
    157 
    158     # create and import the module
--> 159     mod = _CreateWrapper(tlib, pathname)
    160     try:
    161         modulename = tlib.GetDocumentation(-1)[0]

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\client\_generate.py in _CreateWrapper(tlib, pathname)
    225     # XXX use logging!
    226     logger.info("# Generating comtypes.gen.%s", modname)
--> 227     generate_module(tlib, ofi, pathname)
    228 
    229     if comtypes.client.gen_dir is None:

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\tlbparser.py in generate_module(tlib, ofi, pathname)
    765                     )
    766 
--> 767     gen.generate_code(list(items.values()), filename=pathname)
    768 
    769 # -eof-

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\codegenerator.py in generate_code(self, items, filename)
    258             loops += 1
    259             self.more = set()
--> 260             self.generate_all(items)
    261 
    262             items |= self.more

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\codegenerator.py in generate_all(self, items)
    198     def generate_all(self, items):
    199         for item in items:
--> 200             self.generate(item)
    201 
    202     def _make_relative_path(self, path1, path2):

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\codegenerator.py in generate(self, item)
    194         # before actually generating the code.
    195         self.done.add(item)
--> 196         mth(item)
    197 
    198     def generate_all(self, items):

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\codegenerator.py in Structure(self, struct)
    476         self._structures += 1
    477         self.generate(struct.get_head())
--> 478         self.generate(struct.get_body())
    479 
    480     Union = Structure

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\codegenerator.py in generate(self, item)
    194         # before actually generating the code.
    195         self.done.add(item)
--> 196         mth(item)
    197 
    198     def generate_all(self, items):

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\codegenerator.py in StructureBody(self, body)
    500         if not methods:
    501             try:
--> 502                 pack = calc_packing(body.struct, fields)
    503                 if pack is not None:
    504                     print("%s._pack_ = %s" % (body.struct.name, pack), file=self.stream)

C:\WinPython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\codegenerator.py in calc_packing(struct, fields)
    132                 return None
    133             return pack/8
--> 134     raise PackingError("PACKING FAILED: %s" % details)
    135 
    136 class PackingError(Exception):

UnboundLocalError: local variable 'details' referenced before assignment

Result of calling _calc_packing(struct, fields, pack, isStruct) through pdb.pm():

> c:\winpython-32bit-3.4.4.2\python-3.4.4\lib\site-packages\comtypes\tools\codegenerator.py(134)calc_packing()
-> raise PackingError("PACKING FAILED: %s" % details)
(Pdb) _calc_packing(struct, fields, pack, isStruct)
*** comtypes.tools.codegenerator.PackingError: field m_segments offset (32/64)

The interface of the application 'SomeApp.Application' has some unnecessary functions with SAFEARRAY of structures containing double fields as arguments. This error is thrown only when python is x86 and the application 'SomeApp.Application' is x64. Some problems with alignment in 'SomeApp.Application'.
So, the error seems to be combination of alignment error in 'SomeApp.Application' and incorrect processing of this error in comtypes. I had fixed the error in comtypes at my computer. Additional proposed change in comtypes is to skip buggy structures in order to make it possible to use the rest of an application's functionality, otherwise entire application is not usable by comtypes. win32com library can load such applications and use them. Maybe, fix given in the description of the issue will be helpful.

@junkmd
Copy link
Collaborator

junkmd commented Jul 28, 2022

@rafikor

From your comment, I'm guessing that 'SomeApp.Application' is specific to your environment and is a class that can be called from its VersionIndependentProgID(, isn't it?).

I have been a contributor to this repository recently, but mostly working about the Python API, not so much about the c-FFI.

I would like to hear the opinion from @vasily-v-ryabov, who is leading the maintenance of this repository.

@rafikor
Copy link
Author

rafikor commented Jul 28, 2022

@junkmd Yes, 'SomeApp.Application' is specific to my environment and has its VersionIndependentProgID ('SomeApp.Application') used to call the application

@junkmd
Copy link
Collaborator

junkmd commented Jul 28, 2022

@rafikor

But when GetModule is called with absolute path of *.tlb file, then the error does not occur. It is strange for me, I thougth that processing of *.tlb is the same through registry or absolute path.

I also find it strange that such a thing would occur.

I assume that the module generated by GetModule("full/path/to/someapp/*tlb") has a definition of class Application(CoClass).

Please try executing the code below to see whether an instance can be created.

SomeApp = GetModule("full/path/to/someapp/*tlb")
# `CreateObject` can take CoClass type which has `_reg_clsid_` attribute.
# see definition of `client.CreateObject`, `client._manage`, and `GUID.from_progid`
CreateObject(SomeApp.Application)
# or `comtypes.CoCreateInstance(SomeApp.Application().IPersist_GetClassID())`

@rafikor
Copy link
Author

rafikor commented Aug 1, 2022

@rafikor

But when GetModule is called with absolute path of *.tlb file, then the error does not occur. It is strange for me, I thougth that processing of *.tlb is the same through registry or absolute path.

I also find it strange that such a thing would occur.

I assume that the module generated by GetModule("full/path/to/someapp/*tlb") has a definition of class Application(CoClass).

Please try executing the code below to see whether an instance can be created.

SomeApp = GetModule("full/path/to/someapp/*tlb")
# `CreateObject` can take CoClass type which has `_reg_clsid_` attribute.
# see definition of `client.CreateObject`, `client._manage`, and `GUID.from_progid`
CreateObject(SomeApp.Application)
# or `comtypes.CoCreateInstance(SomeApp.Application().IPersist_GetClassID())`

This code works as intended, instance is created (the use of comtypes.CoCreateInstance(SomeApp.Application().IPersist_GetClassID() gave only IUnknown interface)

I have found through pdb.pm() that difference between GetModule("full/path/to/someapp/*tlb") and app=comtypes.client.CreateObject('SomeApp.Application') during running of function calc_packing(struct, fields) from comtypes\tools\codegenerator.py is as follows. In first case, structure SomeStruct has size 128, align 32, and second field from fields array has offset 32, whereas in the second case (through CreateObject, which fails), structure SomeStruct has size 192, align 64, and second field from fields array has offset 64.
storage(fields[0].typ) and storage(fields[1].typ) is (32,32) in both cases.

fields array contains four items. The first item desctibes safearray of structures (the structure is three double numbers), the second item desctibes safearray of structures (the structure is double number and internal structure with three other double numbers). Last two items are just numbers.

Maybe, comtypes is confused with x64 and x86 platforms when CreateObject is called (python is x86, the application is x64, and the OS is x64)

@junkmd
Copy link
Collaborator

junkmd commented Aug 2, 2022

@rafikor

Thank you for your investigation.

It seems to me that it would be reasonable to ensure that CreateObject(prog_id_str) returns object and generates a module equivalent to the code generated by GetModule("full/path/to/app/*tlb") (with no exceptions).

Do you know where need to be corrected in the code?

@rafikor
Copy link
Author

rafikor commented Aug 3, 2022

@junkmd
Yes, I also think that functions CreateObject(prog_id_str) and GetModule("full/path/to/app/*tlb") must do the same things regarding to code generation.
Currently, I did not found lines to correct in code. During calling of GetModule, too many functions are called one frome other, it is hard to find a difference in behavior. I have tried to search "x64" through code of comtypes and set breakpoints, but no results - all specific "x64" behavior found by me works only when x64 python process works with an x86 application, but no otherwise.
Maybe you know places in comtypes where some code specific to platform is located (like, e.g., definition of whether called application is x86 or x64 according to the registry)?

@junkmd
Copy link
Collaborator

junkmd commented Aug 3, 2022

@rafikor

Maybe you know places in comtypes where some code specific to platform is located (like, e.g., definition of whether called application is x86 or x64 according to the registry)?

Oh, I have encountered similar problems. There is two VBA dll in the same environment.
https://stackoverflow.com/questions/63011181/vba-runtimes-msvbvm60-dll-vs-vbe7-dll

Similarly, there may be multiple DLLs in the environment and the DLL referenced by CreateObject("SomeApp.Application") may not be "full/path/to/app/*tlb".

To check, once you have to delete ... /comtypes/gen folder.
After that, create the object, ignoring the exception thrown when calc_packing with CreateObject.
Then check the typelib_path in the _xxxxxxxxxxxx_xxxxxxxx_xxxxxxxx_xxxxxxxxxxxx_x_x_x.py module generated by the side effect.

@vasily-v-ryabov
Copy link
Collaborator

@junkmd I'm mostly reviewing and publishing releases of comtypes. I didn't dive into comtypes code so much to CFFI level. I want to do it (even to dive into libffi C code), but I always have higher priorities. Maybe I'll take 2 months when I change my job next time to cleanup my open-source backlog. :)

I remember some cases in pywinauto that don't work with 64-bit explorer.exe from 32-bit Python. It is usually solved by moving to 64-bit Python so I wouldn't consider it as something critical. Of course, any suggestions and PRs are always welcome.

@junkmd
Copy link
Collaborator

junkmd commented Aug 20, 2022

@vasily-v-ryabov

Thanks for your response.

I am not very familiar with c-ffi or the c language itself.

I'm trying to learn, but for me, adding type hints to this package and refactoring old code in preparation for that is a high priority.

My hope is that as more developers use comtypes, someone familiar with c-ffi will join the contributors.

@junkmd junkmd added the help wanted Extra attention is needed label Nov 26, 2022
@kdschlosser
Copy link
Contributor

ok with respect to this error.

I want to clear a few things up so everyone is on the same page.

Typelibs do not live in the registry. they get registered and that registration is stored in the registry and the file that the typelib is stored in also gets stored with the type lib GUID and name but other than that it's a file.

The only reason why you would have different output when using the generator and passing a dll vs using a registered name is they are not using the same file. There is another one wandering around on your system somewhere that has different typelib information.

Since the files are getting made in your case if you go to comntypes/gen you will see the generated files... If you move or delete everything except for __init__ and run the code generator again using the module name then you will be able to go into the files one at time and look for a line like this,

typelib_path = 'C:\\WINDOWS\\System32\\DriverStore\\FileRepository\\realtekservice.inf_amd64_550508a90a3c9a47\\RtkAudUService64.exe'

it will be right at the top of the file. This tells you exactly where the file is located that the type lib information is being read from. Lets see if the path and file is the same as the one you are manually passing in, I am willing to bet they aren't and there could be some difference between them.

@kdschlosser
Copy link
Contributor

These 2 commands function very close to being the same. The GetModule function Loads the typelib using the Windows API while CreateObject collects the typelib GUID that is registered to the name that is passed to it. From that GUID the location the typelib is stored in is able to be collected and then the code generator would get run. The GetModule since it was passed the location doesn't need the first bunch of steps but once CreateObject collects the path the mechanics are identical after that. So the ONLY way to end up with 2 completely different generated code files is because there are 2 completely different typelibs that are being used.

SomeApp = GetModule("full/path/to/someapp/*tlb")

CreateObject(SomeApp.Application)

@kdschlosser
Copy link
Contributor

Now I have a question. Is the file being passed to GetModule the path to a .tlb file? If it is then whatever you used to generate the .tlb may not have generated it properly. More than likely a missed compiler option, when a typelib gets compiled into a dll the compiler arguments used are what determine how the typelib resource gets added to the dll. if generating the type lib outside of the normal compilation process for the dll then you need to ensure the proper compiler arguments are being used to generate the tlb file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants