8317510: Change Windows debug symbol files naming to avoid losing info when an executable and a library share the same name

Reviewed-by: ihse, erikj
2023-10-25 17:30:21 +00:00 · 2023-10-25 17:30:21 +00:00 · d96f38b80c
--- a/make/CreateJmods.gmk
+++ b/make/CreateJmods.gmk
@ -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
@ -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), \
@ -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
--- a/make/Images.gmk
+++ b/make/Images.gmk
@ -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), \
@ -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)) \
    )
--- a/make/ZipSecurity.gmk
+++ b/make/ZipSecurity.gmk
@ -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)
--- a/make/common/NativeCompilation.gmk
+++ b/make/common/NativeCompilation.gmk
@ -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
@ -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
--- a/make/hotspot/test/GtestImage.gmk
+++ b/make/hotspot/test/GtestImage.gmk
@ -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)) \
    ) \
--- a/make/ide/visualstudio/hotspot/src/classes/build/tools/projectcreator/WinGammaPlatformVC10.java
+++ b/make/ide/visualstudio/hotspot/src/classes/build/tools/projectcreator/WinGammaPlatformVC10.java
@ -329,7 +329,7 @@ class CompilerInterfaceVC10 extends CompilerInterface {
        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.
@ -409,7 +409,7 @@ class CompilerInterfaceVC10 extends CompilerInterface {
        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");
--- a/make/scripts/compare_exceptions.sh.incl
+++ b/make/scripts/compare_exceptions.sh.incl
@ -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
--- a/test/hotspot/jtreg/runtime/ErrorHandling/TestSymbolsInHsErrFile.java
+++ b/test/hotspot/jtreg/runtime/ErrorHandling/TestSymbolsInHsErrFile.java
@ -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).*"));
+
+  }
+
+}
+
+