Fix #392 - Ensure XSI-compliant strerror_r is used. #669

DavidKeller · 2016-06-10T17:17:49Z

This _XOPEN_SOURCE macro was used after some standard header includes. Which is not POSIX compliant.

Depending on libc implementation, if one of theses headers were using <string.h>, the later presence of _XOPEN_SOURCE macro would't matter as the <string.h> would't be included again.

This prevents uninitialized buffer from being passed to printf (search for STR_ERROR in dispatch.c).

Signed-off-by: David Keller [email protected]

twall · 2016-06-13T20:41:24Z

Maybe it makes more sense to pass it in as a compiler flag, ensuring that it's defined everywhere regardless of code paths?

DavidKeller · 2016-06-14T08:35:01Z

Yes, and I'd use an inline function instead of a macro to ensure the XSI function is called.

DavidKeller · 2016-06-14T12:38:16Z

Passing the definition as a compiler flag (to libjnidispatch sources) produces undefined behavior (jvm crash) on my Fedora 23 x86_64 system.
Some anonymous code (jit ?) performs invalid access (SIGSEV)

#0  0x00007effdf4cba28 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55
#1  0x00007effdf4cd62a in __GI_abort () at abort.c:89
#2  0x00007effdee0ed69 in os::abort (dump_core=<optimized out>) at /usr/src/debug/java-1.8.0-openjdk-1.8.0.92-1.b14.fc23.x86_64/openjdk/hotspot/src/os/linux/vm/os_linux.cpp:1500
#3  0x00007effdefc0117 in VMError::report_and_die (this=this@entry=0x7effe049d4d0) at /usr/src/debug/java-1.8.0-openjdk-1.8.0.92-1.b14.fc23.x86_64/openjdk/hotspot/src/share/vm/utilities/vmError.cpp:1060
#4  0x00007effdee1804f in JVM_handle_linux_signal (sig=sig@entry=11, info=info@entry=0x7effe049d770, ucVoid=ucVoid@entry=0x7effe049d640, abort_if_unrecognized=abort_if_unrecognized@entry=1)
    at /usr/src/debug/java-1.8.0-openjdk-1.8.0.92-1.b14.fc23.x86_64/openjdk/hotspot/src/os_cpu/linux_x86/vm/os_linux_x86.cpp:541
#5  0x00007effdee0bea8 in signalHandler (sig=11, info=0x7effe049d770, uc=0x7effe049d640) at /usr/src/debug/java-1.8.0-openjdk-1.8.0.92-1.b14.fc23.x86_64/openjdk/hotspot/src/os/linux/vm/os_linux.cpp:4233
#6  <signal handler called>
#7  0x00007effc91f78b5 in ?? ()
#8  0x00000000d77ff258 in ?? ()
#9  0x00007effc914d2dc in ?? ()
#10 0x00007effb3d35c30 in ?? ()
#11 0x00007effd800a000 in ?? ()
#12 0x000000070000081a in ?? ()
#13 0x0000000000000000 in ?? ()

I may have a closer look by performing dichotomy to isolate faulty header later, depending on my workload.

DavidKeller · 2016-09-02T09:04:18Z

My workload prevents me from looking at the SEGFAULT when the flag is set globally.

In the meantime, what about applying this patch as a first step ?

matthiasblaesing · 2016-09-11T16:42:09Z

@twall: Timothy, could you please have another look at this? The arguments from David looks sane - but I'm not a C/POSIX expert.
I'd cherry pick 88a6b35 into current master if this is ok.

This macro was used after some standard header includes. Depending on implementation, if one of theses headers were using <string.h>, the later presence of _XOPEN_SOURCE macro would'nt matter. This prevents uninitialized buffer from being passed to snprintf. Signed-off-by: David Keller <[email protected]>

twall · 2016-09-11T23:41:41Z

cherry-pick should be ok.

On Sun, Sep 11, 2016 at 12:42 PM, matthiasblaesing <[email protected]

wrote:

@twall https://github.com/twall: Timothy, could you please have another
look at this? The arguments from David looks sane - but I'm not a C/POSIX
expert.
I'd cherry pick 88a6b35
88a6b35
into current master if this is ok.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#669 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAuMrYAG6_Ycaw6JvsJ6tUZJBV4FJCi7ks5qpC9kgaJpZM4IzJp0
.

matthiasblaesing · 2016-09-13T19:24:26Z

@DavidKeller please revisit this. Your modifications breaks the windows build:

     [exec] cl -MT -nologo -W3 -DHAVE_PROTECTION -DPSAPI_VERSION='1' -DFFI_BUILDING -DUNICODE -D_UNICODE -I"c:\Program Files\Java\jdk1.8.0_92\jre/../include" -I"c:\Program Files\Java\jdk1.8.0_92\jre/../include/win32" -I"C:\jnalib2\build\native-win32-x86-64" -I"../build/native-win32-x86-64/libffi/include" -DJNA_JNI_VERSION='"5.0.0"' -DCHECKSUM='"4f72f2799dfee6008a386bc40afd7428"' -c dispatch.c -Fo../build/native-win32-x86-64/dispatch.o -FdC:/jnalib2/build/native-win32-x86-64/dispatch -FpC:/jnalib2/build/native-win32-x86-64/dispatch -FaC:/jnalib2/build/native-win32-x86-64/dispatch
     [exec] dispatch.c
     [exec] c:\jnalib2\native\protect.h(48) : error C2143: syntax error : missing ')' before '*'
     [exec] c:\jnalib2\native\protect.h(48) : error C2143: syntax error : missing '{' before '*'
     [exec] c:\jnalib2\native\protect.h(48) : error C2059: syntax error : ')'
     [exec] c:\jnalib2\native\protect.h(52) : error C2061: syntax error : identifier 'PEXCEPTION_HANDLER'
     [exec] c:\jnalib2\native\protect.h(53) : error C2059: syntax error : '}'
     [exec] c:\jnalib2\native\protect.h(58) : error C2016: C requires that a struct or union has at least one member
     [exec] c:\jnalib2\native\protect.h(58) : error C2061: syntax error : identifier 'EXCEPTION_REGISTRATION'
     [exec] c:\jnalib2\native\protect.h(60) : error C2079: 'er' uses undefined struct '_EXCEPTION_RECORD'
     [exec] c:\jnalib2\native\protect.h(61) : error C2059: syntax error : '}'
     [exec] c:\jnalib2\native\protect.h(63) : error C2143: syntax error : missing '{' before '__cdecl'
     [exec] c:\jnalib2\native\protect.h(68) : error C2065: 'exc_rec' : undeclared identifier
     [exec] c:\jnalib2\native\protect.h(68) : error C2065: 'xer' : undeclared identifier
     [exec] c:\jnalib2\native\protect.h(68) : error C2065: 'exc_rec' : undeclared identifier
     [exec] c:\jnalib2\native\protect.h(68) : error C2059: syntax error : ')'
     [exec] c:\jnalib2\native\protect.h(69) : error C2065: 'xer' : undeclared identifier
     [exec] c:\jnalib2\native\protect.h(69) : error C2223: left of '->er' must point to struct/union
     [exec] c:\jnalib2\native\protect.h(70) : error C2065: 'xer' : undeclared identifier
     [exec] c:\jnalib2\native\protect.h(70) : error C2223: left of '->buf' must point to struct/union
     [exec] c:\jnalib2\native\protect.h(70) : error C2037: left of 'ExceptionCode' specifies undefined struct/union '_EXCEPTION_RECORD'
     [exec] c:\jnalib2\native\protect.h(70) : error C2198: 'longjmp' : too few arguments for call
     [exec] c:\jnalib2\native\protect.h(72) : error C2065: 'ExceptionContinueExecution' : undeclared identifier
     [exec] dispatch.c(88) : fatal error C1083: Cannot open include file: 'alloca.h': No such file or directory
     [exec] /cygdrive/c/jnalib2/native/libffi/msvcc.sh -m64 -W -Wall -Wno-unused -Wno-parentheses    -DHAVE_PROTECTION -DPSAPI_VERSION=1 -DFFI_BUILDING -DUNICODE -D_UNICODE -DUSE_STATIC_RTL -I"c:\Program Files\Java\jdk1.8.0_92\jre/../include" -I"c:\Program Files\Java\jdk1.8.0_92\jre/../include/win32" -I"C:\jnalib2\build\native-win32-x86-64" -I../build/native-win32-x86-64/libffi/include -DJNA_JNI_VERSION='"5.0.0"' -DCHECKSUM='"4f72f2799dfee6008a386bc40afd7428"' -c callback.c -o ../build/native-win32-x86-64/callback.o

DavidKeller · 2016-09-17T17:13:45Z

In order to build on Windows 10/Visual studio 2015/MinGW I had to patch native/libffi/msvcc.sh.
There were also some compilation warnings/errors (corrected in last commit).

@matthiasblaesing which version of Windows/Visual Studio are you using ?

matthiasblaesing · 2016-09-17T20:31:31Z

@DavidKeller I'm building with Windows SDK 7.1 (https://www.microsoft.com/en-us/download/details.aspx?id=8279) - It was a bit tricky to install, but after that it beautifully build JNA. My complete cheat sheet for this:

========================= Windows ========================

0. Start-Point: A clean Windows 10 Installation with all patches as of 2016-06-10
1. Install Windows SDK 7.1:

Version registry key:

HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\NET Framework Setup\NDP\v4\Client
HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\NET Framework Setup\NDP\v4\Full

Relevant attribute: VERSION

1. Note down the values in the version value (Windows 10 pure with patches as of 2016-06-10: 4.6.01038)
2. Change ownership of the registry keys to your current user (Open permissions for the key and choose "Extended")
3. Add full access righs for your current user to the permissions
4. Change both version attributes to 4.0.30319
5. Download and Install Windows SDK 7.1 with defaults: http://www.microsoft.com/en-us/download/details.aspx?id=8279
6. Restore Version from first item of this list


2. Install Oracle JDK 8u92 (64 bit)
3. Install Cygwin (https://cygwin.com/install.html)
    - make
    - automake
    - automake1.15
    - libtool
    - mingw64-x86_64-gcc-g++ (Version 5.3.0-1)
    - mingw64-x86_64-gcc-core (Version 5.3.0-1)
    - gcc-g++
    - diffutils
4. Ensure ant, maven, cygwin are accessible from the PATH
5. Run 
    C:\Program Files\Microsoft SDKs\Windows\v7.1\Bin\SetEnv.Cmd /Release /x64
   inside a windows command prompt
6. Run native build


For 32bit on 64 Build host:

- mingw64-i686-gcc-g++ (Version 5.3.0-1)
- mingw64-i686-gcc-core (Version 5.3.0-1)

None the less the changes you made to the build to be able to build with VS 2015 would be very interesting. There is #603, which looks as if it covers the whole problem. While I'm happy that I was able to setup a build environment that works for me, the enviroment uses old software and I see the day coming microsoft pull the 7.1 SDK or it is not installable (even with tricks...). Could you comment with your patches to the build system for VS2015 on that bug?

With regard to the changes you did - I just did a quick sanity check (test on linux64, windows 32bit, windows 64bit) and this worked out beautifully.

I'll look into this in the next few days, as I also want to merge #700 and both of these PRs need the native parts rebuild, I plan to rebuild the native parts (as far as I can do) after both are merged.

matthiasblaesing · 2016-09-21T19:26:59Z

@DavidKeller sorry for the late reply, but I'm unable to confirm this fix. This is what I see:

With the 4.2.2 source I can reproduce the problem described in Corrupted error messages on Linux #392 with the pre-build native parts
With the 4.2.2 source I can't reproduce the problem when I build the native parts with my toolchain (current ubuntu)
With the master version of the source I can reproduce the problem described in Corrupted error messages on Linux #392 with the pre-build native parts
With the master version of the source I can't reproduce the problem when I build the native parts with my toolchain (current ubuntu)

The imported symbols are interesting (this was pointed out in #392):

pre-build:

matthias@athena:~/src/jnalib$ nm x/libjnidispatch.so | grep strerror
                 U strerror_r@@GLIBC_2.2.5

my build:

matthias@athena:~/src/jnalib$ nm x/libjnidispatch.so | grep strerror
                 U __xpg_strerror_r@@GLIBC_2.3.4

The man-page says:

This function is available in two versions: an XSI-compliant version specified in POSIX.1-2001 (available since glibc 2.3.4, but not POSIX-compliant until glibc 2.13), and a GNU-specific version (available since glibc 2.0).

I tried debian oldstable, but that already carries glibc 2.13, so I could not check if building against a glibc prior to 2.3.4 would result in the problem.

At this point I'm reluctant to merge the changes, as a current tool chain produces correct behavior.

@twall could you have a look at your build environment and check the libc that is used/present?

I'm setting up a build environment - I'm able to build:

windows x86 and x64
linux x64
linux 32bit (chroot, native)
linux-ppc*, linux-armel (qemu)

I'm looking into qemu-debootstrap, which should make it possible to create chroot without literally putting hours into it for one build. For other architectures I'll experiment, but won't promise.

twall · 2016-09-22T00:03:14Z

I've got an old linux VM with an old glibc version (2.3.6), have done
builds with that for the purpose of supporting older environments.

What exactly do you want to validate?

On Wed, Sep 21, 2016 at 3:27 PM, matthiasblaesing [email protected]
wrote:

@DavidKeller https://github.com/DavidKeller sorry for the late reply,
but I'm unable to confirm this fix. This is what I see:

With the 4.2.2 source I can reproduce the problem described in Corrupted error messages on Linux #392
Corrupted error messages on Linux #392 with the
pre-build native parts

With the 4.2.2 source I can't reproduce the problem when I build
the native parts with my toolchain (current ubuntu)

With the master version of the source I can reproduce the problem
described in Corrupted error messages on Linux #392
Corrupted error messages on Linux #392 with the
pre-build native parts

With the master version of the source I can't reproduce the problem
when I build the native parts with my toolchain (current ubuntu)

The imported symbols are interesting (this was pointed out in #392
#392):

pre-build:

matthias@athena:~/src/jnalib$ nm x/libjnidispatch.so | grep strerror
U strerror_r@@GLIBC_2.2.5

my build:

matthias@athena:~/src/jnalib$ nm x/libjnidispatch.so | grep strerror
U __xpg_strerror_r@@GLIBC_2.3.4

The man-page says:

This function is available in two versions: an XSI-compliant version
specified in POSIX.1-2001 (available since glibc 2.3.4, but not
POSIX-compliant until glibc 2.13), and a GNU-specific version (available
since glibc 2.0).

I tried debian oldstable, but that already carries glibc 2.13, so I could
not check if building against a glibc prior to 2.3.4 would result in the
problem.

At this point I'm reluctant to merge the changes, as a current tool chain
produces correct behavior.

@twall https://github.com/twall could you have a look at your build
environment and check the libc that is used/present?

I'm setting up a build environment - I'm able to build:

windows x86 and x64

linux x64

linux 32bit (chroot, native)

linux-ppc*, linux-armel (qemu)

I'm looking into qemu-debootstrap, which should make it possible to create
chroot without literally putting hours into it for one build. For other
architectures I'll experiment, but won't promise.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#669 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAuMrXglryJeki5sHIcAoL607M6LBjIVks5qsYUGgaJpZM4IzJp0
.

DavidKeller · 2016-09-22T08:22:08Z

@matthiasblaesing According to man strerror(3):

If no feature test macros are explicitly defined, then (since glibc 2.4) _POSIX_C_SOURCE is defined by default with the value 200112L, so that the XSI-compliant version of strerror_r() is provided by default

So the problem is present with any glibc version < 2.4, that's why binaries from @twall build system (glibc 2.3.6) suffer from bug #392.

Anyway, even if the build system gets upgraded, I believe theses commits should be applied for tree reasons:

They fix an undefined behavior:

no header defined by IEEE Std 1003.1-2001 shall be included prior to the definition of the feature test macro [...] If the definition of the macro does not precede the #include, the result is undefined

I'm not sure the fact that _POSIX_C_SOURCE is defined by default since glibc 2.4 remove this restriction.

They fix the build with visual studio 2015.
They fix the strerror_r function usage, as it may fail and the current code doesn't handle this.

matthiasblaesing · 2016-09-24T11:14:54Z

@DavidKeller Thank you! As promised I merged the PRs and rebuild the native libraries. To prevent regressions I added a unittest to check the "correct" return from strerror_r. The build script ensures an unlocalized locale, so I have some hope, that the message for a bad filedescriptor is always the same.

DavidKeller · 2016-09-25T10:22:56Z

Thank you @matthiasblaesing.

I'm afraid the strerror(EBADF) can also produce Bad file number depending on the libc (e.g. Android libc vs Glibc).

If I were you, I'd remove the strerror_r() unit test as the new code will trigger a warning if the wrong strerror_r() is picked (invalid conversion from int to char*: warning: initialization makes pointer from integer without a cast) and treat warnings as errors with -Werror.

Motivation: There was a new netty release Modifications: Upgrade to 4.1.107.Final Result: Use latest netty version

matthiasblaesing mentioned this pull request Sep 4, 2016

Pass objects into JNI code in addition to raw pointer value (prevent premature GC) #700

Merged

Fix header inclusion on Visual Studio/MinGW

c74e734

DavidKeller mentioned this pull request Sep 22, 2016

Correct Windows/Cygwin build error as reported in issue #603 #708

Merged

matthiasblaesing merged commit c74e734 into java-native-access:master Sep 24, 2016

matthiasblaesing mentioned this pull request Sep 24, 2016

Corrupted error messages on Linux #392

Closed

mstyura pushed a commit to mstyura/jna that referenced this pull request Sep 9, 2024

Upgrade to netty 4.1.107.Final (java-native-access#669)

1ddf332

Motivation: There was a new netty release Modifications: Upgrade to 4.1.107.Final Result: Use latest netty version

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #392 - Ensure XSI-compliant strerror_r is used. #669

Fix #392 - Ensure XSI-compliant strerror_r is used. #669

DavidKeller commented Jun 10, 2016

twall commented Jun 13, 2016

DavidKeller commented Jun 14, 2016

DavidKeller commented Jun 14, 2016

DavidKeller commented Sep 2, 2016

matthiasblaesing commented Sep 11, 2016

twall commented Sep 11, 2016

matthiasblaesing commented Sep 13, 2016

DavidKeller commented Sep 17, 2016

matthiasblaesing commented Sep 17, 2016

matthiasblaesing commented Sep 21, 2016

twall commented Sep 22, 2016

DavidKeller commented Sep 22, 2016

matthiasblaesing commented Sep 24, 2016

DavidKeller commented Sep 25, 2016

Fix #392 - Ensure XSI-compliant strerror_r is used. #669

Fix #392 - Ensure XSI-compliant strerror_r is used. #669

Conversation

DavidKeller commented Jun 10, 2016

twall commented Jun 13, 2016

DavidKeller commented Jun 14, 2016

DavidKeller commented Jun 14, 2016

DavidKeller commented Sep 2, 2016

matthiasblaesing commented Sep 11, 2016

twall commented Sep 11, 2016

matthiasblaesing commented Sep 13, 2016

DavidKeller commented Sep 17, 2016

matthiasblaesing commented Sep 17, 2016

matthiasblaesing commented Sep 21, 2016

twall commented Sep 22, 2016

DavidKeller commented Sep 22, 2016

matthiasblaesing commented Sep 24, 2016

DavidKeller commented Sep 25, 2016