[OpenJDK 2D-Dev] JDK-8081295: Build failed with GCC 5.1.1

[Alexander Zvegintsev]

Hi Yasumasa,

the fix looks good to me.

Thanks,

Alexander.

On 05/28/2015 05:41 PM, Yasumasa Suenaga wrote:
> Hi Alexander,
>
> I've uploaded new webrev:
>   http://cr.openjdk.java.net/~ysuenaga/JDK-8081295/webrev.02/
>
> This patch excludes libsplashscreen, and fix Awt2dLibraries.gmk .
> Could you review it?
>
>
> Thanks,
>
> Yasumasa
>
>
>
> On 2015/05/28 22:00, Alexander Zvegintsev wrote:
>> I think that you can just exclude libsplashscreen part from your fix,
>> since it is covered by JDK-8080695.
>>
>> Thanks,
>>
>> Alexander.
>>
>> On 05/28/2015 02:52 PM, Yasumasa Suenaga wrote:
>>>
>>> Hi Alexander,
>>>
>>> Thank you for your comment.
>>>
>>> I agree to use DISABLED_WARNINGS_gcc.
>>>
>>> Should I make a patch afterJDK-8080695 
>>> <https://bugs.openjdk.java.net/browse/JDK-8080695> committed?
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>> 2015/05/28 18:19 "Alexander Zvegintsev" 
>>> <alexander.zvegintsev at oracle.com 
>>> <mailto:alexander.zvegintsev at oracle.com>>:
>>>
>>>     Hello Yasumasa,
>>>
>>>     I prefer to avoid such pragma usage, as for me code is more 
>>> readable without it.
>>>     So we can add array-bounds to DISABLED_WARNINGS_gcc in 
>>> make/lib/Awt2dLibraries.gmk for BUILD_LIBJAVAJPEG and 
>>> BUILD_LIBMLIB_IMAGE.
>>>
>>>     We can nullify row_pointers and image_data after setjmp call to 
>>> remove warnings in splashscreen_png.c:
>>>     diff -r 729dffc8afa0 
>>> src/java.desktop/share/native/libsplashscreen/splashscreen_png.c
>>>     --- 
>>> a/src/java.desktop/share/native/libsplashscreen/splashscreen_png.c
>>>     +++ 
>>> b/src/java.desktop/share/native/libsplashscreen/splashscreen_png.c
>>>     @@ -77,6 +77,8 @@ SplashDecodePng(Splash * splash, png_rw_
>>>      #else
>>>          if (setjmp(png_jmpbuf(png_ptr))) {
>>>      #endif
>>>     +        row_pointers = NULL;
>>>     +        image_data = NULL;
>>>              goto done;
>>>          }
>>>     (or declare them as volatile, but I prefer the first option).
>>>     BTW, there is another issue about this splashscreen_png.c gcc 
>>> build failure [1].
>>>
>>>     [1] https://bugs.openjdk.java.net/browse/JDK-8080695
>>>
>>>     Thanks,
>>>
>>>     Alexander.
>>>
>>>
>>>     On 05/28/2015 06:27 AM, Yasumasa Suenaga wrote:
>>>
>>>         I've uploaded webrev:
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8081295/webrev.01/ 
>>> <http://cr.openjdk.java.net/%7Eysuenaga/JDK-8081295/webrev.01/>
>>>
>>>         Please review.
>>>
>>>
>>>         Thanks,
>>>
>>>         Yasumasa
>>>
>>>
>>>         On 2015/05/28 12:19, Yasumasa Suenaga wrote:
>>>
>>>             Hi all,
>>>
>>>             I tried to build jdk9/dev on Fedora22 with GCC 5.1.1, 
>>> however, it was failed.
>>>             I found several problems:
>>>
>>>
>>>                System:
>>>                 Fedora release 22 (Twenty Two) x86_64
>>>                  - gcc-5.1.1-1.fc22.x86_64
>>>
>>>
>>>                Problems:
>>>                  1. Array bounds check in GCC
>>>                    - 
>>> jdk/src/java.desktop/share/native/libjavajpeg/jcmaster.c
>>>                    - 
>>> jdk/src/java.desktop/share/native/libjavajpeg/jquant1.c
>>>                    - 
>>> jdk/src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_64.c
>>>                    - 
>>> jdk/src/java.desktop/share/native/libmlib_image/mlib_c_ImageLookUp_f.c
>>>
>>>                    It seems to be bug of GCC:
>>>                      Bug 59124: [4.8/4.9/5/6 Regression] Wrong 
>>> warnings "array subscript is above array bounds"
>>>             https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59124
>>>
>>>                    I think implementations of these files have no 
>>> problem.
>>>                    So I propose to ignore warning(s) of compiler 
>>> through pragma option as
>>>                    workaround when we use GCC [1].
>>>
>>>
>>>                  2. Local variables might be clobbered
>>>                    - 
>>> jdk/src/java.desktop/share/native/libsplashscreen/splashscreen_png.c
>>>
>>>                    SplashDecodePng() calls setjmp(3).
>>>                    Some local variables initialize before setjmp() 
>>> call, and use after it.
>>>                    Their initial values are only used at cleanup 
>>> code (*done* label) and
>>>                    actual values are stored only after setjmp() call.
>>>                    So I think we can ignore this error through 
>>> pragma option [1].
>>>
>>>
>>>                  3. Incorrect condition
>>>                    - 
>>> jdk/src/jdk.jdwp.agent/share/native/libjdwp/eventFilter.c
>>>
>>>                    searchAllSourceNames() returns int value. 
>>> However, branch condition in
>>>                    eventFilterRestricted_passesFilter() treats it as 
>>> boolean value.
>>>
>>>
>>>             I received a comment from Erik to use 
>>> --disable-warnings-as-errors,
>>>             however I could not avoid error in 3.
>>>
>>>
>>>             Thanks,
>>>
>>>             Yasumasa
>>>
>>>
>>>             [1] 
>>> https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html
>>>
>>>
>>