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 compatibility with AARCH64 #101
Conversation
public final Unsigned32 st_mode = new Unsigned32(); | ||
public final Unsigned32 st_uid = new Unsigned32(); | ||
public final Unsigned32 st_gid = new Unsigned32(); | ||
public final Signed32 __pad0 = new Signed32(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etehtsea Is __pad0 missing from all 64 bit Linux systems? This seems like everything would be way off here if this were the case. Assuming it was ever tested against any other 64 bit linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally I tried on Debian 8.8 / cc (Debian 4.9.2-10) 4.9.2 / Xeon E5620
. At the moment I rechecked on CentOS 7.3.1611/ cc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-11) / i7-4770HQ
.
And also using clang on CentOS
(clang version 3.4.2 (tags/RELEASE_34/dot2-final)
) and got the same result everywhere.
P.S. I'm not sure but one C dev who I know said that this should be guaranteed by C ABI (https://en.m.wikipedia.org/wiki/Application_binary_interface).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etehtsea yeah I guess I am mostly asking because by ABI then this should have never worked until this PR. So that makes me wonder if some Linux variants are actually missing a field that the system you tested have (which might just be a literal pad).
Unfortunately, this structure since the very beginning has not had that extra 32 bit pad. We can probably land this as-is and write off the old code as a mistake but I will poke around and see if I can understand why there is a pad (or more importantly why we committed something without the pad).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like these pads are in for word alignment purposes. Our .java files are pretty old. I wonder if they corrected alignment of stat in the last 10 years and ours are out of date with newer linuxes. I see in FC25 the same pads. I wish history was documented better in software.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made sure that StructLayout
handles padding automatically so I updated PR.
ioprioThreadedTest(jnr.posix.LinuxPOSIXTest) Time elapsed: 0.038 sec <<< ERROR! java.lang.IllegalStateException: ioprio_set is not implemented in jnr-posix at jnr.posix.util.DefaultPOSIXHandler.unimplementedError(DefaultPOSIXHandler.java:28) at jnr.posix.LinuxPOSIX.ioprio_set(LinuxPOSIX.java:254) at jnr.posix.LinuxPOSIXTest.ioprioThreadedTest(LinuxPOSIXTest.java:44)
66f5619
to
714362c
Compare
I added/,odified Linux
stat64
struct layouts according topahole
output:xtest.c
On
AARCH64
:On
X86_64
:This PR fixes jruby/jruby#4600