splashscreen.so is missing pnggccrd.c

[Martin Buchholz]

I'm thinking:
- the MMX support is in pnggccrd.c,
- but that file is never compiled in OpenJDK
- so... how could png ever work when PNG_NO_MMX_CODE is not defined,
  on any platform?

I am suspicious that png splashscreens are actually broken
without my proposed patch.  Has this actually been tested?

I googled a little and have a theory.
"-z defs" is supposed to be ignored for shared libs
(but why are we using it to build libsplashscreen.so?)
but very recent gcc toolchains don't ignore it the way they
perhaps should.

E.g. Here's a debian bug report
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=489142
On hppa -Wl,-z,defs is not ignored for shared libraries

But using "-z defs" is actually good engineering practice,
since it catches problems at link time instead of run time.
Whether or not it's a bug in my toolchain, my toolchain
detects the unsatisfied symbols in the link.
There's a good chance that my change will be necessary
to build OpenJDK on future stock Linux systems.

Martin


On Thu, Aug 28, 2008 at 3:11 AM, Anthony Petrov <Anthony.Petrov at sun.com> wrote:
> Hello Martin,
>
> Thank you for the patch. Though there's a little concern: how does the bare
> libpng deal with this issue when compiled using the toolchain you use? I
> mean, the problem is not reproducible when compiling on a "regular" 32-bit
> system, so it might be important to identify the root cause of this issue.
> Could you please take a look at the libpng source (the version that was
> forked for the purposes of OpenJDK is 1.2.18) and try compiling it with your
> tools (by the way, what are they?)? If this is the problem with this
> particular version of libpng, and newer versions do not experience this
> problem, then this might be probably a justification for updating our
> version of libpng rather than disabling the usage of MMX for all platforms.
> What do you think?
>
> --
> best regards,
> Anthony
>
>
> On 08/28/2008 02:23 AM Martin Buchholz wrote:
>>
>> Anthony,
>>
>> Thanks for your patience; you were entirely right,
>> it was indeed necessary to define PNG_NO_MMX_CODE.
>> Since pnggccrd.c is never compiled, and it contains the MMX code,
>> the only correct thing appears to be to define this macro unconditionally.
>> The obvious patch follows (tested on 32-bit Ubuntu Linux with
>> very recent gcc toolchain)
>>
>> (Has anyone actually tested png splashscreen images on 32 or 64-bit
>> Linux?)
>>
>> # HG changeset patch
>> # User martin
>> # Date 1219875308 25200
>> # Node ID 86b160e4bc1aae927aa71dfd712bb2365ebc0e1c
>> # Parent  1267605489211c6c162bb246f653759e933bd06e
>> 6613927: Compilation of splashscreen png library failed on Ubuntu 7.04
>> (64bit)
>> Summary: Define -DPNG_NO_MMX_CODE unconditionally, not just on 64-bit
>> Linux
>> Reviewed-by: anthony
>>
>> diff --git a/make/sun/splashscreen/Makefile
>> b/make/sun/splashscreen/Makefile
>> --- a/make/sun/splashscreen/Makefile
>> +++ b/make/sun/splashscreen/Makefile
>> @@ -85,13 +85,6 @@
>>  CPPFLAGS += -I$(PLATFORM_SRC)/native/$(PKGDIR)/splashscreen
>> -I$(SHARE_SRC)/native/$(PKGDIR)/splashscreen
>>  CPPFLAGS += -I$(SHARE_SRC)/native/$(PKGDIR)/image/jpeg
>> -I$(SHARE_SRC)/native/java/util/zip/zlib-1.1.3
>>
>> -ifeq ($(PLATFORM), linux)
>> -  ifeq ($(ARCH_DATA_MODEL), 64)
>> -    # 64-bit gcc has problems compiling MMX instructions.
>> -    # Google it for more details. Possibly the newer versions of
>> -    # the PNG-library and/or the new compiler will not need this
>> -    # option in the future.
>> -    CPPFLAGS += -DPNG_NO_MMX_CODE
>> -  endif
>> -endif
>> -
>> +# Shun the less than portable MMX assembly code in pnggccrd.c.
>> +# Newer versions of the PNG-library may not need this option.
>> +CPPFLAGS += -DPNG_NO_MMX_CODE
>>
>> Martin
>>
>> On Wed, Aug 27, 2008 at 5:48 AM, Anthony Petrov <Anthony.Petrov at sun.com>
>> wrote:
>>>
>>> Hi Martin,
>>>
>>> On 08/25/2008 08:17 PM Martin Buchholz wrote:
>>>>
>>>> Thanks for the link, but...
>>>> - The fix for 6613927 is applied in my workspace
>>>
>>> And it gets activated when building on a 64-bit system only. Could you
>>> please take a look at the make/sun/splashscreen/Makefile file and make
>>> sure
>>> it defines the "CPPFLAGS += -DPNG_NO_MMX_CODE" in every case. It is
>>> really
>>> very interesting if this will resolve the issue. Thank you in advance.
>>
>