RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Severin Gehwolf-3
The suggestion is to rename 'jvm' variable in `libjavajpeg` to `the_jvm` so this conflict no longer occurs when `libjavajpeg.a` and `libawt.a` are being linked into one native image.

Testing: test/jdk/javax/imageio jtreg tests. GHA pre-integration tests running too.

Thoughts?

-------------

Commit messages:
 - 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Changes: https://git.openjdk.java.net/jdk/pull/3155/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3155&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264047
  Stats: 14 lines in 2 files changed: 0 ins; 0 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3155.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3155/head:pull/3155

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Severin Gehwolf-3
On Tue, 23 Mar 2021 15:18:13 GMT, Severin Gehwolf <[hidden email]> wrote:

> The suggestion is to rename 'jvm' variable in `libjavajpeg` to `the_jvm` so this conflict no longer occurs when `libjavajpeg.a` and `libawt.a` are being linked into one native image.
>
> Testing: test/jdk/javax/imageio jtreg tests. GHA pre-integration tests running too.
>
> Thoughts?

Anyone?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Phil Race
On Thu, 25 Mar 2021 17:54:35 GMT, Severin Gehwolf <[hidden email]> wrote:

> Anyone?

I guess I don't understand why this is the right solution.
If the symbol -in both cases - is made internal won't that be better ? I can't think why it needs to be exported.
Is the AWT one being used by some other client library ? If so may be that other library needs its own.
And calling it "jvm" is common so are these the only two ?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Severin Gehwolf-3
On Thu, 25 Mar 2021 18:20:36 GMT, Phil Race <[hidden email]> wrote:

> > Anyone?
>
> I guess I don't understand why this is the right solution.

Thanks for the feedback. I'm open to other approaches. I'm by no means an expert in this area, so you probably know better what the best solution would be.

> If the symbol -in both cases - is made internal won't that be better ? I can't think why it needs to be exported.

I don't know if that'll help the static linking case, I'll give it a try.

> Is the AWT one being used by some other client library ? If so may be that other library needs its own.

I don't think it is. `libawt.so` and `libjavajpeg.so` have each their own. The problem starts when static libraries of them `libawt.a` and `libjavajpeg.a` get linked into one executable. Then we end up with a name clash on the `jvm` symbol. See the bug for some details.

> And calling it "jvm" is common so are these the only two ?

Apparently so. At least no other conflicts were noticed when the following client libraries are in the mix:

libjavajpeg.a
liblcms.a
libfontmanager.a
libawt_headless.a
libawt.a
libharfbuzz.a

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Phil Race
On Fri, 26 Mar 2021 10:04:07 GMT, Severin Gehwolf <[hidden email]> wrote:

>>> Anyone?
>>
>> I guess I don't understand why this is the right solution.
>> If the symbol -in both cases - is made internal won't that be better ? I can't think why it needs to be exported.
>> Is the AWT one being used by some other client library ? If so may be that other library needs its own.
>> And calling it "jvm" is common so are these the only two ?
>
>> > Anyone?
>>
>> I guess I don't understand why this is the right solution.
>
> Thanks for the feedback. I'm open to other approaches. I'm by no means an expert in this area, so you probably know better what the best solution would be.
>
>> If the symbol -in both cases - is made internal won't that be better ? I can't think why it needs to be exported.
>
> I don't know if that'll help the static linking case, I'll give it a try.
>
>> Is the AWT one being used by some other client library ? If so may be that other library needs its own.
>
> I don't think it is. `libawt.so` and `libjavajpeg.so` have each their own. The problem starts when static libraries of them `libawt.a` and `libjavajpeg.a` get linked into one executable. Then we end up with a name clash on the `jvm` symbol. See the bug for some details.
>
>> And calling it "jvm" is common so are these the only two ?
>
> Apparently so. At least no other conflicts were noticed when the following client libraries are in the mix:
>
> libjavajpeg.a
> liblcms.a
> libfontmanager.a
> libawt_headless.a
> libawt.a
> libharfbuzz.a

The list of libraries doesn't seem to include xawt. Although I'd be astonished if it defined one since it links with awt.
But it also suggests you are only looking at one platform and perhaps that is because the problem is very specific
to your toolchain ? Actually why are you seeing it now and it has not been a problem before ?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Severin Gehwolf-3
On Fri, 26 Mar 2021 14:49:02 GMT, Phil Race <[hidden email]> wrote:

> The list of libraries doesn't seem to include xawt.

Yes. This is in context of Graal VM's native images. AFAIK there is no full xawt support yet. This is about the conflict of libawt and libjavajpeg. There are probably more.

> But it also suggests you are only looking at one platform and perhaps that is because the problem is very specific
> to your toolchain ? Actually why are you seeing it now and it has not been a problem before ?

Yes. This is on Linux x86_64 and with an OpenJDK built with GCC 10+. With gcc 10+ `-fno-common` is default:
https://gcc.gnu.org/gcc-10/porting_to.html

So the `libjavajpeg.a`, and `libawt.a` got compiled with `-fno-common`. Then when being linked into an executable together, the linker screams with this:


collect2: error: ld returned 1 exit status

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Sergey Bylokhov-2
On Fri, 26 Mar 2021 14:59:14 GMT, Severin Gehwolf <[hidden email]> wrote:

>> The list of libraries doesn't seem to include xawt. Although I'd be astonished if it defined one since it links with awt.
>> But it also suggests you are only looking at one platform and perhaps that is because the problem is very specific
>> to your toolchain ? Actually why are you seeing it now and it has not been a problem before ?
>
>> The list of libraries doesn't seem to include xawt.
>
> Yes. This is in context of Graal VM's native images. AFAIK there is no full xawt support yet. This is about the conflict of libawt and libjavajpeg. There are probably more.
>
>> But it also suggests you are only looking at one platform and perhaps that is because the problem is very specific
>> to your toolchain ? Actually why are you seeing it now and it has not been a problem before ?
>
> Yes. This is on Linux x86_64 and with an OpenJDK built with GCC 10+. With gcc 10+ `-fno-common` is default:
> https://gcc.gnu.org/gcc-10/porting_to.html
>
> So the `libjavajpeg.a`, and `libawt.a` got compiled with `-fno-common`. Then when being linked into an executable together, the linker screams with this:
>
>
> collect2: error: ld returned 1 exit status

Probably it will be easy to remove this "jvm" variable in the jpeg library? Looks like it is used to call JNU_GetEnv, but the JNIEnv could be accessed from the first parameter of the jni method.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Sergey Bylokhov-2
On Sat, 27 Mar 2021 00:57:34 GMT, Sergey Bylokhov <[hidden email]> wrote:

> Probably it will be easy to remove this "jvm" variable in the jpeg library? Looks like it is used to call JNU_GetEnv, but the JNIEnv could be accessed from the first parameter of the jni method.

Seems these methods are used as callbacks...

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Severin Gehwolf-3
On Sat, 27 Mar 2021 01:09:40 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Probably it will be easy to remove this "jvm" variable in the jpeg library? Looks like it is used to call JNU_GetEnv, but the JNIEnv could be accessed from the first parameter of the jni method.
>
>> Probably it will be easy to remove this "jvm" variable in the jpeg library? Looks like it is used to call JNU_GetEnv, but the JNIEnv could be accessed from the first parameter of the jni method.
>
> Seems these methods are used as callbacks...

So what's the verdict with this one? OK to rename the variable or go a different route?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Sergey Bylokhov-2
In reply to this post by Severin Gehwolf-3
On Tue, 23 Mar 2021 15:18:13 GMT, Severin Gehwolf <[hidden email]> wrote:

> The suggestion is to rename 'jvm' variable in `libjavajpeg` to `the_jvm` so this conflict no longer occurs when `libjavajpeg.a` and `libawt.a` are being linked into one native image.
>
> Testing: test/jdk/javax/imageio jtreg tests. GHA pre-integration tests running too.
>
> Thoughts?

Marked as reviewed by serb (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Severin Gehwolf-3
On Wed, 31 Mar 2021 20:37:32 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> The suggestion is to rename 'jvm' variable in `libjavajpeg` to `the_jvm` so this conflict no longer occurs when `libjavajpeg.a` and `libawt.a` are being linked into one native image.
>>
>> Testing: test/jdk/javax/imageio jtreg tests. GHA pre-integration tests running too.
>>
>> Thoughts?
>
> Marked as reviewed by serb (Reviewer).

@mrserb Thanks for the review!

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155
Reply | Threaded
Open this post in threaded view
|

Integrated: 8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Severin Gehwolf-3
In reply to this post by Severin Gehwolf-3
On Tue, 23 Mar 2021 15:18:13 GMT, Severin Gehwolf <[hidden email]> wrote:

> The suggestion is to rename 'jvm' variable in `libjavajpeg` to `the_jvm` so this conflict no longer occurs when `libjavajpeg.a` and `libawt.a` are being linked into one native image.
>
> Testing: test/jdk/javax/imageio jtreg tests. GHA pre-integration tests running too.
>
> Thoughts?

This pull request has now been integrated.

Changeset: eb6330e4
Author:    Severin Gehwolf <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/eb6330e4
Stats:     14 lines in 2 files changed: 0 ins; 0 del; 14 mod

8264047: Duplicate global variable 'jvm' in libjavajpeg and libawt

Reviewed-by: serb

-------------

PR: https://git.openjdk.java.net/jdk/pull/3155