Skip to content

Commit

Permalink
8317510: Change Windows debug symbol files naming to avoid losing inf…
Browse files Browse the repository at this point in the history 
…o when an executable and a library share the same name

Reviewed-by: ihse, erikj
  • Loading branch information
@fthevenet @erikj79
fthevenet authored and erikj79 committed Oct 25, 2023
1 parent 10427c0 commit d96f38b
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 36 deletions.
20 changes: 3 additions & 17 deletions make/CreateJmods.gmk
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved.
# Copyright (c) 2014, 2023, Oracle and/or its affiliates. All rights reserved.
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
#
# This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -81,13 +81,11 @@ endif
ifneq ($(CMDS_DIR), )
DEPS += $(call FindFiles, $(CMDS_DIR))
ifeq ($(call isTargetOs, windows)+$(SHIP_DEBUG_SYMBOLS), true+public)
# For public debug symbols on Windows, we have to use stripped pdbs, rename them
# and filter out a few launcher pdbs where there's a lib that goes by the same name
# For public debug symbols on Windows, we have to use stripped pdbs and rename them
rename_stripped = $(patsubst %.stripped.pdb,%.pdb,$1)
CMDS_DIR_FILTERED := $(subst modules_cmds,modules_cmds_filtered, $(CMDS_DIR))
FILES_CMDS := $(filter-out %.pdb, $(call FindFiles, $(CMDS_DIR))) \
$(filter-out %jimage.stripped.pdb %jpackage.stripped.pdb %java.stripped.pdb, \
$(filter %.stripped.pdb, $(call FindFiles, $(CMDS_DIR))))
$(filter %.stripped.pdb, $(call FindFiles, $(CMDS_DIR)))
$(eval $(call SetupCopyFiles, COPY_FILTERED_CMDS, \
SRC := $(CMDS_DIR), \
DEST := $(CMDS_DIR_FILTERED), \
Expand All @@ -96,18 +94,6 @@ ifneq ($(CMDS_DIR), )
))
DEPS += $(COPY_FILTERED_CMDS)
JMOD_FLAGS += --cmds $(CMDS_DIR_FILTERED)
else ifeq ($(call isTargetOs, windows)+$(SHIP_DEBUG_SYMBOLS), true+full)
# For full debug symbols on Windows, we have to filter out a few launcher pdbs
# where there's a lib that goes by the same name
CMDS_DIR_FILTERED := $(subst modules_cmds,modules_cmds_filtered, $(CMDS_DIR))
$(eval $(call SetupCopyFiles, COPY_FILTERED_CMDS, \
SRC := $(CMDS_DIR), \
DEST := $(CMDS_DIR_FILTERED), \
FILES := $(filter-out %jimage.pdb %jpackage.pdb %java.pdb, \
$(call FindFiles, $(CMDS_DIR))), \
))
DEPS += $(COPY_FILTERED_CMDS)
JMOD_FLAGS += --cmds $(CMDS_DIR_FILTERED)
else
JMOD_FLAGS += --cmds $(CMDS_DIR)
endif
Expand Down
7 changes: 2 additions & 5 deletions make/Images.gmk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,6 @@ else
endif
endif

FILTERED_PDBS := %jimage.stripped.pdb %jpackage.stripped.pdb %java.stripped.pdb \
%jimage.pdb %jpackage.pdb %java.pdb %jimage.map %jpackage.map %java.map

# Param 1 - either JDK or JRE
SetupCopyDebuginfo = \
$(foreach m, $(ALL_$1_MODULES), \
Expand All @@ -290,8 +287,8 @@ SetupCopyDebuginfo = \
$(eval $(call SetupCopyFiles, COPY_$1_CMDS_DEBUGINFO_$m, \
SRC := $(SUPPORT_OUTPUTDIR)/modules_cmds/$m, \
DEST := $($1_IMAGE_DIR)/$(CMDS_TARGET_SUBDIR), \
FILES := $(filter-out $(FILTERED_PDBS), $(call FindDebuginfoFiles, \
$(SUPPORT_OUTPUTDIR)/modules_cmds/$m)), \
FILES := $(call FindDebuginfoFiles, \
$(SUPPORT_OUTPUTDIR)/modules_cmds/$m), \
)) \
$(eval $1_TARGETS += $$(COPY_$1_CMDS_DEBUGINFO_$m)) \
)
Expand Down
6 changes: 3 additions & 3 deletions make/ZipSecurity.gmk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ ifeq ($(call isTargetOs, windows), true)
$(eval $(call SetupZipArchive,BUILD_JGSS_BIN_ZIP, \
SRC := $(SUPPORT_OUTPUTDIR), \
INCLUDE_FILES := modules_libs/java.security.jgss/w2k_lsa_auth.dll \
modules_libs/java.security.jgss/w2k_lsa_auth.diz \
modules_libs/java.security.jgss/w2k_lsa_auth.map \
modules_libs/java.security.jgss/w2k_lsa_auth.pdb, \
modules_libs/java.security.jgss/w2k_lsa_auth.dll.diz \
modules_libs/java.security.jgss/w2k_lsa_auth.dll.map \
modules_libs/java.security.jgss/w2k_lsa_auth.dll.pdb, \
ZIP := $(IMAGES_OUTPUTDIR)/$(JGSS_ZIP_NAME)))

TARGETS += $(IMAGES_OUTPUTDIR)/$(JGSS_ZIP_NAME)
Expand Down
16 changes: 10 additions & 6 deletions make/common/NativeCompilation.gmk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1073,13 +1073,13 @@ define SetupNativeCompilationBody
ifneq ($$($1_TYPE), STATIC_LIBRARY)
# Generate debuginfo files.
ifeq ($(call isTargetOs, windows), true)
$1_EXTRA_LDFLAGS += -debug "-pdb:$$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).pdb" \
"-map:$$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).map"
$1_EXTRA_LDFLAGS += -debug "-pdb:$$($1_SYMBOLS_DIR)/$$($1_BASENAME).pdb" \
"-map:$$($1_SYMBOLS_DIR)/$$($1_BASENAME).map"
ifeq ($(SHIP_DEBUG_SYMBOLS), public)
$1_EXTRA_LDFLAGS += "-pdbstripped:$$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).stripped.pdb"
$1_EXTRA_LDFLAGS += "-pdbstripped:$$($1_SYMBOLS_DIR)/$$($1_BASENAME).stripped.pdb"
endif
$1_DEBUGINFO_FILES := $$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).pdb \
$$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).map
$1_DEBUGINFO_FILES := $$($1_SYMBOLS_DIR)/$$($1_BASENAME).pdb \
$$($1_SYMBOLS_DIR)/$$($1_BASENAME).map

else ifeq ($(call isTargetOs, linux), true)
$1_DEBUGINFO_FILES := $$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).debuginfo
Expand Down Expand Up @@ -1127,7 +1127,11 @@ define SetupNativeCompilationBody
$1 += $$($1_DEBUGINFO_FILES)

ifeq ($$($1_ZIP_EXTERNAL_DEBUG_SYMBOLS), true)
$1_DEBUGINFO_ZIP := $$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).diz
ifeq ($(call isTargetOs, windows), true)
$1_DEBUGINFO_ZIP := $$($1_SYMBOLS_DIR)/$$($1_BASENAME).diz
else
$1_DEBUGINFO_ZIP := $$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).diz
endif
$1 += $$($1_DEBUGINFO_ZIP)

# The dependency on TARGET is needed for debuginfo files
Expand Down
2 changes: 1 addition & 1 deletion make/hotspot/test/GtestImage.gmk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ ifeq ($(call isTargetOs, windows), true)
$(eval $(call SetupCopyFiles, COPY_GTEST_PDB_$v, \
SRC := $(HOTSPOT_OUTPUTDIR)/variant-$v/libjvm/gtest, \
DEST := $(TEST_IMAGE_DIR)/hotspot/gtest/$v, \
FILES := jvm.pdb gtestLauncher.pdb, \
FILES := jvm.dll.pdb gtestLauncher.exe.pdb, \
)) \
$(eval TARGETS += $$(COPY_GTEST_PDB_$v)) \
) \
Expand Down
4 changes: 2 additions & 2 deletions ...ide/visualstudio/hotspot/src/classes/build/tools/projectcreator/WinGammaPlatformVC10.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ Vector getBaseCompilerFlags(Vector defines, Vector includes, String outDir) {
addAttr(rv, "PrecompiledHeaderOutputFile", outDir+Util.sep+"vm.pch");
addAttr(rv, "AssemblerListingLocation", outDir);
addAttr(rv, "ObjectFileName", outDir+Util.sep);
addAttr(rv, "ProgramDataBaseFileName", outDir+Util.sep+"jvm.pdb");
addAttr(rv, "ProgramDataBaseFileName", outDir+Util.sep+"jvm.dll.pdb");
// Set /nologo option
addAttr(rv, "SuppressStartupBanner", "true");
// Surpass the default /Tc or /Tp.
Expand Down Expand Up @@ -409,7 +409,7 @@ Vector getBaseLinkerFlags(String outDir, String outDll, String platformName) {
addAttr(rv, "OutputFile", outDll);
addAttr(rv, "SuppressStartupBanner", "true");
addAttr(rv, "ModuleDefinitionFile", outDir+Util.sep+"vm.def");
addAttr(rv, "ProgramDatabaseFile", outDir+Util.sep+"jvm.pdb");
addAttr(rv, "ProgramDatabaseFile", outDir+Util.sep+"jvm.dll.pdb");
addAttr(rv, "SubSystem", "Windows");
addAttr(rv, "BaseAddress", "0x8000000");
addAttr(rv, "ImportLibrary", outDir+Util.sep+"jvm.lib");
Expand Down
4 changes: 2 additions & 2 deletions make/scripts/compare_exceptions.sh.incl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ elif [ "$OPENJDK_TARGET_OS" = "windows" ]; then
SKIP_BIN_DIFF="true"
SKIP_FULLDUMP_DIFF="true"
ACCEPTED_JARZIP_CONTENTS="
/modules_libs/java.security.jgss/w2k_lsa_auth.pdb
/modules_libs/java.security.jgss/w2k_lsa_auth.map
/modules_libs/java.security.jgss/w2k_lsa_auth.dll.pdb
/modules_libs/java.security.jgss/w2k_lsa_auth.dll.map
/modules_libs/java.security.jgss/w2k_lsa_auth.dll
"
elif [ "$OPENJDK_TARGET_OS" = "macosx" ]; then
Expand Down
76 changes: 76 additions & 0 deletions test/hotspot/jtreg/runtime/ErrorHandling/TestSymbolsInHsErrFile.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, Red Hat, Inc. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/


/*
* @test symbolsHsErr
* @summary Test that function names are present in native frames of hs-err file as a proof that symbols are available.
* @library /test/lib
* @requires vm.flagless
* @requires vm.debug
* @requires os.family == "windows"
* @modules java.base/jdk.internal.misc
* java.management
* @run driver TestSymbolsInHsErrFile
*/

import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;

public class TestSymbolsInHsErrFile {

public static void main(String[] args) throws Exception {

// Start a jvm and cause a SIGSEGV / ACCESS_VIOLATION
ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
"-XX:+UnlockDiagnosticVMOptions",
"-Xmx100M",
"-XX:-CreateCoredumpOnCrash",
"-XX:ErrorHandlerTest=14",
"-version");

OutputAnalyzer output = new OutputAnalyzer(pb.start());
output.shouldNotHaveExitValue(0);

// Verify that the hs_err problematic frame contains a function name that points to origin of the crash;
// on Windows/MSVC, if symbols are present and loaded, we should see a ref to either 'crash_with_segfault'
// 'VMError::controlled_crash' depending on whether the compile optimizations (i.e. crash_with_segfault
// was inlined or not):
// # Problematic frame:
// # V [jvm.dll+0x.....] crash_with_segfault+0x10
// or
// # V [jvm.dll+0x.....] VMError::controlled_crash+0x99
//
// If symbols could not be loaded, however, then the frame will contain not function name at all, i.e.
// # Problematic frame:
// # V [jvm.dll+0x.....]
// NB: this is not true for other OS/Compilers, where the functions names are present even with no symbols,
// hence this test being restricted to Windows only.
output.shouldMatch(("# V \\[jvm.dll.*\\].*(crash_with_segfault|controlled_crash).*"));

}

}


5 comments on commit d96f38b

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

@JesperIRL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/tag jdk-22+21

@openjdk
Copy link

@openjdk openjdk bot commented on d96f38b Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JesperIRL The tag jdk-22+21 was successfully created.

@fthevenet
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk21u

@openjdk
Copy link

@openjdk openjdk bot commented on d96f38b Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fthevenet the backport was successfully created on the branch fthevenet-backport-d96f38b8 in my personal fork of openjdk/jdk21u. To create a pull request with this backport targeting openjdk/jdk21u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit d96f38b8 from the openjdk/jdk repository.

The commit being backported was authored by Frederic Thevenet on 25 Oct 2023 and was reviewed by Magnus Ihse Bursie and Erik Joelsson.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u:

$ git fetch https://github.com/openjdk-bots/jdk21u.git fthevenet-backport-d96f38b8:fthevenet-backport-d96f38b8
$ git checkout fthevenet-backport-d96f38b8
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u.git fthevenet-backport-d96f38b8

Please sign in to comment.