Re: RFR(XS) 8161915: Linking gtestLauncher may end up linking with non-gtest libjvm
On 2016-07-20, Mikael Gerdin wrote: > Hi all, > > Please review this small fix to work around a problem where incremental > builds of hotspot would fail due to the symbol runUnitTests missing. > > The cause for the problem is that the unit tests launcher linker command > line contains a -L path containing the standard libjvm.so > [10] LDFLAGS := -Wl,-z,defs -Wl,-z,now -Wl,-z,relro > -Wl,--allow-shlib-undefined > -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/support/modules_libs/java.base/amd64 > > -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/support/modules_libs/java.base/amd64/server > > [11] LDFLAGS_unix := > -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/hotspot/variant-server/libjvm/gtest > -Wl,-z,origin -Wl,-rpath,\$$ORIGIN > [13] LIBS_unix := -ljvm > > This is caused by BUILD_GTEST_LAUNCHER picking up LDFLAGS_TESTEXE which was > recently modified to include -L path to the standard libjvm.so > > My suggested fix is to instead use LDFLAGS_JDKEXE for the gtest launcher > since the only difference between them is that TESTEXE appends > JAVA_BASE_LDFLAGS: > flags.m4, 687: LDFLAGS_TESTEXE="$LDFLAGS_JDKEXE $JAVA_BASE_LDFLAGS" > > Since the fix is only one line, I'll paste it inline here: > diff --git a/make/lib/CompileGtest.gmk b/make/lib/CompileGtest.gmk > --- a/make/lib/CompileGtest.gmk > +++ b/make/lib/CompileGtest.gmk > @@ -104,7 +104,7 @@ > -I$(GTEST_FRAMEWORK_SRC)/include, \ > CFLAGS_DEBUG_SYMBOLS := $(JVM_CFLAGS_SYMBOLS), \ > CXXFLAGS_DEBUG_SYMBOLS := $(JVM_CFLAGS_SYMBOLS), \ > -LDFLAGS := $(LDFLAGS_TESTEXE), \ > +LDFLAGS := $(LDFLAGS_JDKEXE), \ > LDFLAGS_unix := -L$(JVM_OUTPUTDIR)/gtest $(call > SET_SHARED_LIBRARY_ORIGIN), \ > LDFLAGS_solaris := -library=stlport4, \ > LIBS_unix := -ljvm, \ Looks good, Reviewed. Thanks, Erik > Testing: JPRT -testset hotspot > Bug link: https://bugs.openjdk.java.net/browse/JDK-8161915 > > Please let me know if there is any further testing I should do before > pushing this. > > Thanks > /Mikael
Re: RFR(XS) 8161915: Linking gtestLauncher may end up linking with non-gtest libjvm
Hi Mikael, On 20/07/2016 11:54 PM, Mikael Gerdin wrote: Hi all, Please review this small fix to work around a problem where incremental builds of hotspot would fail due to the symbol runUnitTests missing. The cause for the problem is that the unit tests launcher linker command line contains a -L path containing the standard libjvm.so [10] LDFLAGS := -Wl,-z,defs -Wl,-z,now -Wl,-z,relro -Wl,--allow-shlib-undefined -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/support/modules_libs/java.base/amd64 -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/support/modules_libs/java.base/amd64/server [11] LDFLAGS_unix := -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/hotspot/variant-server/libjvm/gtest -Wl,-z,origin -Wl,-rpath,\$$ORIGIN [13] LIBS_unix := -ljvm This is caused by BUILD_GTEST_LAUNCHER picking up LDFLAGS_TESTEXE which was recently modified to include -L path to the standard libjvm.so My suggested fix is to instead use LDFLAGS_JDKEXE for the gtest launcher since the only difference between them is that TESTEXE appends JAVA_BASE_LDFLAGS: flags.m4, 687: LDFLAGS_TESTEXE="$LDFLAGS_JDKEXE $JAVA_BASE_LDFLAGS" Since the fix is only one line, I'll paste it inline here: diff --git a/make/lib/CompileGtest.gmk b/make/lib/CompileGtest.gmk --- a/make/lib/CompileGtest.gmk +++ b/make/lib/CompileGtest.gmk @@ -104,7 +104,7 @@ -I$(GTEST_FRAMEWORK_SRC)/include, \ CFLAGS_DEBUG_SYMBOLS := $(JVM_CFLAGS_SYMBOLS), \ CXXFLAGS_DEBUG_SYMBOLS := $(JVM_CFLAGS_SYMBOLS), \ -LDFLAGS := $(LDFLAGS_TESTEXE), \ +LDFLAGS := $(LDFLAGS_JDKEXE), \ LDFLAGS_unix := -L$(JVM_OUTPUTDIR)/gtest $(call SET_SHARED_LIBRARY_ORIGIN), \ LDFLAGS_solaris := -library=stlport4, \ LIBS_unix := -ljvm, \ This seems like a reasonable solution at least in the short-term. It may be that gtest launcher needs it own compiler/linker options independent of other "executables" in the build. Thanks, David Testing: JPRT -testset hotspot Bug link: https://bugs.openjdk.java.net/browse/JDK-8161915 Please let me know if there is any further testing I should do before pushing this. Thanks /Mikael
RFR(XS) 8161915: Linking gtestLauncher may end up linking with non-gtest libjvm
Hi all, Please review this small fix to work around a problem where incremental builds of hotspot would fail due to the symbol runUnitTests missing. The cause for the problem is that the unit tests launcher linker command line contains a -L path containing the standard libjvm.so [10] LDFLAGS := -Wl,-z,defs -Wl,-z,now -Wl,-z,relro -Wl,--allow-shlib-undefined -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/support/modules_libs/java.base/amd64 -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/support/modules_libs/java.base/amd64/server [11] LDFLAGS_unix := -L/home/mgerdin/work/repos/hg/jdk9/hs-rt-work/build/linux-x64-slowdebug/hotspot/variant-server/libjvm/gtest -Wl,-z,origin -Wl,-rpath,\$$ORIGIN [13] LIBS_unix := -ljvm This is caused by BUILD_GTEST_LAUNCHER picking up LDFLAGS_TESTEXE which was recently modified to include -L path to the standard libjvm.so My suggested fix is to instead use LDFLAGS_JDKEXE for the gtest launcher since the only difference between them is that TESTEXE appends JAVA_BASE_LDFLAGS: flags.m4, 687: LDFLAGS_TESTEXE="$LDFLAGS_JDKEXE $JAVA_BASE_LDFLAGS" Since the fix is only one line, I'll paste it inline here: diff --git a/make/lib/CompileGtest.gmk b/make/lib/CompileGtest.gmk --- a/make/lib/CompileGtest.gmk +++ b/make/lib/CompileGtest.gmk @@ -104,7 +104,7 @@ -I$(GTEST_FRAMEWORK_SRC)/include, \ CFLAGS_DEBUG_SYMBOLS := $(JVM_CFLAGS_SYMBOLS), \ CXXFLAGS_DEBUG_SYMBOLS := $(JVM_CFLAGS_SYMBOLS), \ -LDFLAGS := $(LDFLAGS_TESTEXE), \ +LDFLAGS := $(LDFLAGS_JDKEXE), \ LDFLAGS_unix := -L$(JVM_OUTPUTDIR)/gtest $(call SET_SHARED_LIBRARY_ORIGIN), \ LDFLAGS_solaris := -library=stlport4, \ LIBS_unix := -ljvm, \ Testing: JPRT -testset hotspot Bug link: https://bugs.openjdk.java.net/browse/JDK-8161915 Please let me know if there is any further testing I should do before pushing this. Thanks /Mikael
