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

Fix jitter on Windows #1199

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Fix jitter on Windows #1199

merged 2 commits into from
Apr 28, 2020

Conversation

jbgalet
Copy link
Contributor

@jbgalet jbgalet commented Apr 27, 2020

On Windows with Python38 the Jitter fails to compile mostly because of the native extension filenames .cp38-win_amd64.lib.
Therefore:

  • during setup, some libs are misplaced
  • bad extension in jitcore_cc_base.py (sysconfig.get_config_var('EXT_SUFFIX') is .pyd but we need .lib here)
  • use hardcoded python27.lib

@serpilliere
Copy link
Contributor

Hi @jbgalet !
If windows with python38 is now supported, can you upgrade the .appveyor.yml file in this PR to add the regression test for this platform?

@serpilliere
Copy link
Contributor

If you are not familiar with appveyor yml, don't hesitate to tell me, I will PR your PR with the fix!

@serpilliere
Copy link
Contributor

Thanks a lot @jbgalet!

Don't worry for the travis bugs, it's a llvm bug we are facing for some times.
Just a last request if you are ok: Can you merge your first and second commit ?
(the second one is just a fixup)

jbgalet and others added 2 commits April 28, 2020 13:59
* Appveyor
* Fix typo, update jitcore_llvm.py, update llvmlite to 0.32.0 to have python3.8 wheels
* Add all platforms in AppVeyor. Use -m to avoid WinError 32
* Use llvmlite 0.31.0, latest version to provide Python2.7 support AND Python3.8
* Remove -m
@jbgalet
Copy link
Contributor Author

jbgalet commented Apr 28, 2020

Should be good, note that I had to update the llvmlite requirement to 0.31.0 to make it work, but it breaks travis (since llvmlite 0.31 requires llvm 7).
I have a .travis.yml that almost works here: https://github.com/jbgalet/miasm/blob/test-travis/.travis.yml
I can merge it as needed.

@serpilliere serpilliere merged commit ecd5efb into cea-sec:master Apr 28, 2020
@serpilliere
Copy link
Contributor

Thank you @jbgalet !
If you are ok, you can PR the fix for the travis!

@jbgalet jbgalet deleted the fix-windows branch April 28, 2020 17:27
@serpilliere
Copy link
Contributor

@jbgalet , with your travis file, did you manage to make llvm tests pass again on travis ?

@@ -248,7 +249,8 @@ def buil_all():
for lib in libs:
filename = os.path.basename(lib)
dst = os.path.join(build_base, lib_dirname, "miasm", "jitter")
if filename not in ["VmMngr.lib", "Jitgcc.lib", "Jitllvm.lib"]:
# Windows built libraries may have a name like VmMngr.cp38-win_amd64.lib
if not any([fnmatch.fnmatch(filename, pattern) for pattern in ["VmMngr.*lib", "Jitgcc.*lib", "Jitllvm.*lib"]]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's too late, but you don't need to build a list in such situations; this works:

Suggested change
if not any([fnmatch.fnmatch(filename, pattern) for pattern in ["VmMngr.*lib", "Jitgcc.*lib", "Jitllvm.*lib"]]):
if not any(fnmatch.fnmatch(filename, pattern) for pattern in ["VmMngr.*lib", "Jitgcc.*lib", "Jitllvm.*lib"]):

@jbgalet
Copy link
Contributor Author

jbgalet commented Apr 30, 2020

@jbgalet , with your travis file, did you manage to make llvm tests pass again on travis ?

Not yet. I've talked to @commial and it seems there are some issues related to #1153

@serpilliere
Copy link
Contributor

Oh ok!
I thought you have found it was fixed in the last version of llvm.
Noproblemo.

w4kfu pushed a commit to w4kfu/miasm that referenced this pull request Jun 16, 2020
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

Successfully merging this pull request may close these issues.

3 participants