RFR: JDK-8238380: java.base/unix/native/libjava/childproc.c "multiple definition" link errors with GCC10

Thomas Stüfe thomas.stuefe at gmail.com
Thu Feb 6 06:48:48 UTC 2020


I can sponsor this for you.

.. Thomas

On Thu, Feb 6, 2020, 04:35 Patrick Zhang OS <patrick at os.amperecomputing.com>
wrote:

> Does this require one more “yes” from any other reviewer?
>
>
>
> And may I ask for any committer’s help to sponsor and push it (once
> approved)? Thanks in advance.
>
>
>
> Regards
>
> Patrick
>
>
>
> *From:* Thomas Stüfe <thomas.stuefe at gmail.com>
> *Sent:* Wednesday, February 5, 2020 11:16 PM
> *To:* Patrick Zhang OS <patrick at os.amperecomputing.com>
> *Cc:* core-libs-dev <core-libs-dev at openjdk.java.net>
> *Subject:* Re: RFR: JDK-8238380:
> java.base/unix/native/libjava/childproc.c "multiple definition" link errors
> with GCC10
>
>
>
> All still good.
>
>
>
> ...Thomas
>
>
>
> On Wed, Feb 5, 2020 at 3:42 PM Patrick Zhang OS <
> patrick at os.amperecomputing.com> wrote:
>
> Thanks for your comments, Thomas.
>
>
>
> I have no accurate knowledge regarding why parentPathv was initially
> written as a global var instead of a member in ChildStuff struct
> initialized in jspawnhelper.c. However the suggested change might not be
> very straight-forward as there are other references such as:
> Java_java_lang_ProcessImpl_init(), spawnChild() in
> libjava\ProcessImpl_md.c, etc. And this goes beyond my original intention
> to fix the build errors purely, and try best to avoid any side effect. So I
> would leave the further improvement to others, and keep it as-is. Thanks.
>
>
>
> Updated the change a bit (the header comments):
> http://cr.openjdk.java.net/~qpzhang/8238380/webrev.02/
>
>
>
> Regards
>
> Patrick
>
>
>
> *From:* Thomas Stüfe <thomas.stuefe at gmail.com>
> *Sent:* Wednesday, February 5, 2020 2:30 PM
> *To:* Patrick Zhang OS <patrick at os.amperecomputing.com>
> *Cc:* core-libs-dev <core-libs-dev at openjdk.java.net>
> *Subject:* Re: RFR: JDK-8238380:
> java.base/unix/native/libjava/childproc.c "multiple definition" link errors
> with GCC10
>
>
>
> Hi Patrick,
>
>
>
> You patch looks alright.
>
>
>
> But I wonder whether a cleaner solution would not be to add a parentPathv
> pointer to the ChildStuff structure and pass it down to JDK_execvpe that
> way, like we do for all other input arguments of that function. If it is an
> input argument, seems strange to pass it as global variable. I leave that
> for others to decide though, your patch certainly works.
>
>
>
> Cheers, Thomas
>
>
>
> On Tue, Feb 4, 2020 at 3:39 PM Patrick Zhang OS <
> patrick at os.amperecomputing.com> wrote:
>
> Hi
>
> I split the original patch into parts for corresponding function review,
> here is the very simple change to java.base/unix/native/libjava/childproc.c
> and .h.
> Could core-libs-dev group help review and sponsor this? thanks in advance.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8238380 (a sub-task of
> JDK-8235903)
> Webrev: http://cr.openjdk.java.net/~qpzhang/8238380/webrev.01/
> Test: ran jtreg tier1, jdk built with GCC4.8.5, GCC9, and GCC10, no
> regression found. I don’t have any specific function tests, so this is just
> a smoke test.
>
> Regards
> Patrick
>
> From: Martin Buchholz <martinrb at google.com<mailto:martinrb at google.com>>
> Sent: Monday, December 16, 2019 10:44 AM
> To: Patrick Zhang OS <patrick at os.amperecomputing.com<mailto:
> patrick at os.amperecomputing.com>>; net-dev <net-dev at openjdk.java.net
> <mailto:net-dev at openjdk.java.net>>; OpenJDK <security-dev at openjdk.java.net
> <mailto:security-dev at openjdk.java.net>>
> Cc: core-libs-dev <core-libs-dev at openjdk.java.net<mailto:
> core-libs-dev at openjdk.java.net>>
> Subject: Re: RFR: JDK-8235903: GCC default -fno-common exposes "multiple
> definition" link errors
>
> forwarded to other teams for review.
>
> On Fri, Dec 13, 2019 at 3:14 AM Patrick Zhang OS <
> patrick at os.amperecomputing.com<mailto:patrick at os.amperecomputing.com>>
> wrote:
> Hi
>
> Please review this patch, if it should be reviewed by any group other than
> core-libs, please help forward it. Thanks.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8235903
> Webrev: http://cr.openjdk.java.net/~qpzhang/8235903/webrev.01/
>
> A recent GCC patch (supposed to be in GCC 10) exposes a couple of
> "multiple definition" link errors when building the jdk tip.
>
> [PATCH] PR85678: Change default to -fno-common
> https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01847.html
>
> For example, the error message looks like:
> * For target support_native_java.base_libjava_BUILD_LIBJAVA_link:
> build/support/native/java.base/libjava/childproc.o:(.bss+0x0): multiple
> definition of `parentPathv'
> build/support/native/java.base/libjava/ProcessImpl_md.o:(.bss+0x0): first
> defined here
> collect2: error: ld returned 1 exit status
>
> This was not an issue because the original default -fcommon allowed
> "global variables defined without an initializer" be handled as COMMON
> symbols, so it would not warn the problem like "same variable is
> accidentally defined in more than one compilation unit".
>
> About -fcommon vs -fno-cmmon:
> https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#index-fno-common
>
> Moving forward, building jdk with latest versions of GCC will trigger this
> error. Specifying "--with-extra-cflags='-fcommon'"  can make it work, but
> it just got things hidden again.
>
> In addition, -fcommon's behavior "is inconsistent with C++, and on many
> targets implies a speed and code size penalty on global variable
> references. It is mainly useful to enable legacy code to link without
> errors."
>
> Last, in case that other jdk developers would revisit this problem once
> again, I suggest fixing the error explicitly instead of using "-fcommon"
>
> Regards
> Patrick
>
>


More information about the core-libs-dev mailing list