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 unittests for windows 32bit intel #1278

Merged

Conversation

matthiasblaesing
Copy link
Member

With the improvements done by @tresf in #1264 three failing windows unittests on x86 were identified:

  • GetModuleFileNameExW can fail on 64bit platforms, if the calling programm is 32bit. I did not find official documentation about this, but the indication I found on the net speculate, that the problem is observed, when the target process is mapped beyon the memory range, that a 32 bit process can observe. GetProcessImageFileName can be used as an alternative, but returns a different format (device path) and needs to be converted, to return the expected value.
  • testCreateRemoteThread creates a thread by writing the binary directly into memory and starting the thread with the memory location. That binary was only valid for 64bit, but nor for 32 bit (different calling convention). The binary for x86 was retrieved by combining observations of binaries and assembler output from mingw and reading the opcode documentation for x86.
  • testWindowMesssages failed because SetWindowLongPtr only exists on x86-64, on x86 it is only present as a header based alias for SetWindowLong

@matthiasblaesing
Copy link
Member Author

@dbwiddis could you please have a look?

Copy link
Contributor

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Left some comments on the first branch. The second two look sane to me but I'm not experienced enough with inline assembly or callbacks to conduct a more substantive review.

@matthiasblaesing matthiasblaesing force-pushed the win-x86-unittests branch 2 times, most recently from 3cbe93a to 7da6577 Compare December 3, 2020 19:15
@matthiasblaesing
Copy link
Member Author

matthiasblaesing commented Dec 3, 2020

Thank you for the review Daniel. I admit, that I did not want to do that many changes, but you are right, using the intended function is the sane approach. It might also help to fix #1269. Could you have another look, I had some WTF moments looking at the various path options windows offers.

I'll take care of the merge conflict once this is merge ready.

@dbwiddis
Copy link
Contributor

dbwiddis commented Dec 3, 2020

Looks good to me!

@matthiasblaesing matthiasblaesing merged commit 3b47eff into java-native-access:master Dec 3, 2020
@matthiasblaesing matthiasblaesing deleted the win-x86-unittests branch December 6, 2020 12:09
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