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

Include missing functions from llvm's compiler-rt into Jitllvm #1153

Merged
merged 1 commit into from
May 24, 2020

Conversation

aguinet
Copy link
Contributor

@aguinet aguinet commented Mar 5, 2020

It's still a hack to fix the "divsion" problem with LLVM, but seems to work under Linux! (haven't tried under Windows yet but hopefully it works)

@jbgalet jbgalet mentioned this pull request Apr 30, 2020
@serpilliere
Copy link
Contributor

serpilliere commented May 21, 2020

Hi @aguinet !
Sorry for the delay.
Thanks a lot for the idea: I tested on archlinux, we need one more function (divti3)
I did a PR on your PR to fix this, but maybe you prefer redo your PR with the new divti3 added.
I didn't investigate why it still fails on travis, but it seems to be an interesting work around

@aguinet
Copy link
Contributor Author

aguinet commented May 22, 2020

I've squash your commit into mine.
It works for Linux, but IIRC under Windows the hack does not work.

The main remaining issue is how to have distutils build a shared library in a portable way. We might also fix this by launching command line by hand to generate this .so/.dll ...

@aguinet aguinet force-pushed the hack/div branch 3 times, most recently from 157d7f0 to fde9b81 Compare May 23, 2020 10:41
@serpilliere
Copy link
Contributor

Can you rebase on master @aguinet ? I fixed the tipo erros in codespell to pass travis tests.
(sorry for that)

@aguinet
Copy link
Contributor Author

aguinet commented May 23, 2020

Done!

@serpilliere
Copy link
Contributor

Hope the Travis Gods will be on our side.

@serpilliere
Copy link
Contributor

The Travis Gods are pleased with your sacrifice.
But maybe it's time to remove Win32 support.

@serpilliere
Copy link
Contributor

I am pushing a dummy PR to see if the windows 32 bit tests are due to this PR or if it's a coincidence.

@aguinet
Copy link
Contributor Author

aguinet commented May 23, 2020

So I will squash these commits into one!

About Windows 32, I will try on my local VM, but it seems to be a bigger issue.

There is also this https://ci.appveyor.com/project/cea-sec/miasm/builds/33068672/job/701prnol8kpicw4d, with an issue on dse_crackme.py, that can't delete a temporary file. Maybe a race between two tests?

@serpilliere
Copy link
Contributor

There is an issue on dse which appears at runtime randomly, but in most cases, the build is ok on appveyor.
See this #1235 I just did: https://ci.appveyor.com/project/cea-sec/miasm/builds/33069250
The windows tests passed on windows 32 bit.

But it seems your PR broke the test at build time, right?

@aguinet
Copy link
Contributor Author

aguinet commented May 23, 2020

Yes the issue on win 32 bits is at build time. I seem to be able to reproduce this locally. I'm looking into it.

@aguinet
Copy link
Contributor Author

aguinet commented May 23, 2020

So the issue under 32 bits is that clang still builds 64 bits binaries. Indeed, the choice is normally made with the VS environment that is choosen. I will try and fix that properly :)

@aguinet
Copy link
Contributor Author

aguinet commented May 23, 2020

Other problem is that uint128_t does not exist for Windows 32-bit, even with Clang. IIRC it is the same for Linux 32 bits. I guess we have to disable the mn_div test for 32-bit builds!

@aguinet aguinet force-pushed the hack/div branch 2 times, most recently from 95ad2c7 to eb3de69 Compare May 23, 2020 13:30
@aguinet
Copy link
Contributor Author

aguinet commented May 23, 2020

Fixed by forcing the compilation with clang only for 64-bit Windows systems. I also disabled the mn_div.py test if we are not running on a 64-bit system.

@aguinet aguinet changed the title Expose missing functions from llvm's compiler-rt through a shared library Include missing functions from llvm's compiler-rt into Jitllvm May 23, 2020
@aguinet
Copy link
Contributor Author

aguinet commented May 24, 2020

All green @serpilliere !

setup.py Outdated Show resolved Hide resolved
Under Windows, we also need to compile these with clang (because MSVCRT doesn't seem to
have proper uint128_t support), so automatically check if clang is
installed (through the registry) and force distutils to use it (with a
not so nice hack).

Last but not least, this only works for 64 bit systems, so disable the mn_div
test if this is not the case.
@serpilliere serpilliere merged commit 43bdc4c into cea-sec:master May 24, 2020
@serpilliere
Copy link
Contributor

Thanks a lot for the fix @aguinet !

@aguinet aguinet deleted the hack/div branch May 24, 2020 10:27
w4kfu pushed a commit to w4kfu/miasm that referenced this pull request Jun 16, 2020
Include missing functions from llvm's compiler-rt into Jitllvm
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.

2 participants