Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v12]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 68 commits: - Exclude TestDwarf.java when run with product because TraceDwarf is a develop flag - Merge branch 'master' into JDK-8242181 - Merge branch 'master' into JDK-8242181 - Fix TestDwarf for older GCC versions - Change logging from UL to tty based with new TraceDwarfLevel develop flag - Add support to parse the .debug_line section in DWARF 2 as emitted by GCC 8, add some comments - Merge branch 'master' into JDK-8242181 - Merge branch 'master' into JDK-8242181 - Merge branch 'master' into JDK-8242181 - Fix build, add GCC flag gdwarf-4 to exclude DWARF 5, add assertions - ... and 58 more: https://git.openjdk.org/jdk/compare/c2ccf4ca...cb030f10 - Changes: https://git.openjdk.org/jdk/pull/7126/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=11 Stats: 2781 lines in 18 files changed: 2684 ins; 41 del; 56 mod Patch: https://git.openjdk.org/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.org/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.org/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v11]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 66 commits: - Merge branch 'master' into JDK-8242181 - Fix TestDwarf for older GCC versions - Change logging from UL to tty based with new TraceDwarfLevel develop flag - Add support to parse the .debug_line section in DWARF 2 as emitted by GCC 8, add some comments - Merge branch 'master' into JDK-8242181 - Merge branch 'master' into JDK-8242181 - Merge branch 'master' into JDK-8242181 - Fix build, add GCC flag gdwarf-4 to exclude DWARF 5, add assertions - Apply remaining review comments from Thomas Stuefe - Change load_dwarf_file with DwarfFilePath and DebugInfo - ... and 56 more: https://git.openjdk.java.net/jdk/compare/42261d75...dbb687ef - Changes: https://git.openjdk.java.net/jdk/pull/7126/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=10 Stats: 2784 lines in 18 files changed: 2687 ins; 41 del; 56 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v9]
On Wed, 25 May 2022 09:05:13 GMT, Thomas Stuefe wrote: >> Thanks Thomas for doing the testing! > >> Thanks Thomas for doing the testing! > > Hi Christian, > > I can see no problems on ppcle attributable to your test. However, I may > overlook something since tests are still failing all over the place because > of loom. > > I see test errors in TestDwarf on ppcle and x64, however. On x64: > > > --System.out:(52/5198)-- > Command line: > [/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/bin/java -cp > /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/JTwork/classes/runtime/ErrorHandling/TestDwarf.d:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/grmpf/testdata/jtreg/jtreg_test_19/test/hotspot/jtreg/runtime/ErrorHandling:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/JTwork/classes:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/grmpf/testdata/jtreg/jtreg_test_19/test/hotspot/jtreg:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/JTwork/classes/test/lib:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/grmpf/testdata/jtreg/jtreg_test_19/test/lib:/net/sapmnt.sapjvm_work/openjdk/tools/jtreg-6+2/lib/javatest.jar:/net/sapmnt.sapjvm_work/openjdk/tools/jtreg-6+2/lib/jtreg.jar > -Xmx768m -Djava.awt.headless=true > -Djava.util.prefs.userRoot=/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hot spot_tier1_work/tmp -Djava.io.tmpdir=/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/tmp -Dtest.getfreeport.max.tries=40 -ea -esa -Xlog:dwarf=info -XX:-CreateCoredumpOnCrash -Xcomp -XX:CICrashAt=1 --version ] > [2022-05-24T18:37:00.974121691Z] Gathering output for process 44490 > [2022-05-24T18:37:02.872100892Z] Waiting for completion for process 44490 > [2022-05-24T18:37:02.872338192Z] Waiting for completion finished for process > 44490 > Output and diagnostic info for process 44490 was saved into > 'pid-44490-output.log' > [2022-05-24T18:37:02.889455876Z] Waiting for completion for process 44490 > [2022-05-24T18:37:02.889604189Z] Waiting for completion finished for process > 44490 > # To suppress the following error report, specify this argument > # after -XX: or in .hotspotrc: SuppressErrorAt=/c1_Compilation.cpp:616 > # > # A fatal error has been detected by the Java Runtime Environment: > # > # Internal Error > (/sapmnt/sapjvm_work/openjdk/nb/linuxx86_64/jdk-dev/src/hotspot/share/c1/c1_Compilation.cpp:616), > pid=44490, tid=44505 > # assert(CICrashAt < 0 || (uintx)_env->compile_id() != (uintx)CICrashAt) > failed: just as planned > # > # JRE version: OpenJDK Runtime Environment (19.0) (fastdebug build > 19-internal-adhoc.openjdk.jdk-dev) > # Java VM: OpenJDK 64-Bit Server VM (fastdebug > 19-internal-adhoc.openjdk.jdk-dev, compiled mode, sharing, tiered, compressed > oops, compressed class ptrs, g1 gc, linux-amd64) > # Problematic frame: > # V [libjvm.so+0x73ca34] Compilation::~Compilation()+0x84 > # > # CreateCoredumpOnCrash turned off, no core file dumped > # > # An error report file with more information is saved as: > # > /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/JTwork/scratch/hs_err_pid44490.log > [1.835s][info][dwarf] Open DWARF file: > /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.debuginfo > [1.842s][info][dwarf] Failed to parse the line number program header > correctly. > [1.842s][info][dwarf] Failed to process the line number program correctly. > [1.842s][info][dwarf] Failed to retrieve file and line number information for > /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.so > at offset: 0x0074176a > [1.843s][info][dwarf] Failed to parse the line number program header > correctly. > [1.843s][info][dwarf] Failed to process the line number program correctly. > [1.843s][info][dwarf] Failed to retrieve file and line number information for > /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.so > at offset: 0x00a05011 > [1.845s][info][dwarf] Failed to parse the line number program header > correctly. > [1.845s][info][dwarf] Failed to process the line number program correctly. > [1.845s][info][dwarf] Failed to retrieve file and line number information for > /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.so > at offset: 0x00a05b78 > [1.849s][info][dwarf] Failed to parse the line number program header > correctly. > [1.849s][info][dwarf] Failed to process the line number program correctly. > [1.849s][info][dwarf] Failed to retrieve file and line number information for > /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.so > at offset: 0x018d49d3 > [1.852s][info][dwarf] Failed to parse the line number program header > correctly. > [1.852s][info][dwarf] Failed to process the line number program correctly. > [1.852s]
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v10]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 65 commits: - Fix TestDwarf for older GCC versions - Change logging from UL to tty based with new TraceDwarfLevel develop flag - Add support to parse the .debug_line section in DWARF 2 as emitted by GCC 8, add some comments - Merge branch 'master' into JDK-8242181 - Merge branch 'master' into JDK-8242181 - Merge branch 'master' into JDK-8242181 - Fix build, add GCC flag gdwarf-4 to exclude DWARF 5, add assertions - Apply remaining review comments from Thomas Stuefe - Change load_dwarf_file with DwarfFilePath and DebugInfo - Revert renaming on Windows - ... and 55 more: https://git.openjdk.java.net/jdk/compare/6e55a72f...8c2c4ab4 - Changes: https://git.openjdk.java.net/jdk/pull/7126/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=09 Stats: 2784 lines in 18 files changed: 2687 ins; 41 del; 56 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v9]
On Fri, 20 May 2022 09:37:03 GMT, Christian Hagedorn wrote: >> When printing the native stack trace on Linux (mostly done for hs_err >> files), it only prints the method with its parameters and a relative offset >> in the method: >> >> Stack: [0x7f6e01739000,0x7f6e0183a000], sp=0x7f6e01838110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> >> This makes it sometimes difficult to see where exactly the methods were >> called from and sometimes almost impossible when there are multiple >> invocations of the same method within one method. >> >> This patch improves this by providing source information (filename + line >> number) to the native stack traces on Linux similar to what's already done >> on Windows (see >> [JDK-8185712](https://bugs.openjdk.java.net/browse/JDK-8185712)): >> >> Stack: [0x7f34fca18000,0x7f34fcb19000], sp=0x7f34fcb17110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> (c1_Compilation.cpp:607) >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec (c1_Compiler.cpp:250) >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> (compileBroker.cpp:2291) >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> (compileBroker.cpp:1966) >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 (compilerThread.cpp:59) >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> (thread.cpp:1297) >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 (thread.cpp:1280) >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 (thread.cpp:358) >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> (os_linux.cpp:705) >> >> For Linux, we need to parse the debug symbols which are generated by GCC in >> DWARF - a standardized debugging format. This patch adds support for DWARF >> 4, the default of GCC 10.x, for 32 and 64 bit architectures (tested with >> x86_32, x86_64 and AArch64). DWARF 5 is not supported as it was still >> experimental and not generated for HotSpot. However, newer GCC version may >> soon generate DWARF 5 by default in which case this parser either needs to >> be extended or the build of HotSpot configured to only emit DWARF 4. >> >> The code follows the parsing steps described in the official DWARF 4 spec: >> https://dwarfstd.org/doc/DWARF4.pdf >> I added references to the corresponding sections throughout the code. >> However, I tried to explain the steps from the DWARF spec directly in the >> code (method names, comments etc.). This allows to follow the code without >> the need to actually deep dive into the spec. >> >> The comments at the `Dwarf` class in the `elf.hpp` file explain in more >> detail how a DWARF file is structured and how the parsing algorithm works to >> get to the filename and line number information. There are more class >> comments throughout the `elf.hpp` file about how different DWARF sections >> are structured and how the parsing algorithm needs to fetch the required >> information. Therefore, I will not repeat the exact workings of the >> algorithm here but refer to the code comments. I've tried to add as much >> information as possible to improve the readability. >> >> Generally, I've tried to stay away from adding any assertions as this code >> is almost always executed when already processing a VM error. Instead, the >> DWARF parser aims to just exit gracefully and possibly omit source >> information for a stack frame instead of risking to stop writing the hs_err >> file when an assertion
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v8]
On Fri, 8 Apr 2022 11:11:29 GMT, Christian Hagedorn wrote: >> When printing the native stack trace on Linux (mostly done for hs_err >> files), it only prints the method with its parameters and a relative offset >> in the method: >> >> Stack: [0x7f6e01739000,0x7f6e0183a000], sp=0x7f6e01838110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> >> This makes it sometimes difficult to see where exactly the methods were >> called from and sometimes almost impossible when there are multiple >> invocations of the same method within one method. >> >> This patch improves this by providing source information (filename + line >> number) to the native stack traces on Linux similar to what's already done >> on Windows (see >> [JDK-8185712](https://bugs.openjdk.java.net/browse/JDK-8185712)): >> >> Stack: [0x7f34fca18000,0x7f34fcb19000], sp=0x7f34fcb17110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> (c1_Compilation.cpp:607) >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec (c1_Compiler.cpp:250) >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> (compileBroker.cpp:2291) >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> (compileBroker.cpp:1966) >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 (compilerThread.cpp:59) >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> (thread.cpp:1297) >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 (thread.cpp:1280) >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 (thread.cpp:358) >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> (os_linux.cpp:705) >> >> For Linux, we need to parse the debug symbols which are generated by GCC in >> DWARF - a standardized debugging format. This patch adds support for DWARF >> 4, the default of GCC 10.x, for 32 and 64 bit architectures (tested with >> x86_32, x86_64 and AArch64). DWARF 5 is not supported as it was still >> experimental and not generated for HotSpot. However, newer GCC version may >> soon generate DWARF 5 by default in which case this parser either needs to >> be extended or the build of HotSpot configured to only emit DWARF 4. >> >> The code follows the parsing steps described in the official DWARF 4 spec: >> https://dwarfstd.org/doc/DWARF4.pdf >> I added references to the corresponding sections throughout the code. >> However, I tried to explain the steps from the DWARF spec directly in the >> code (method names, comments etc.). This allows to follow the code without >> the need to actually deep dive into the spec. >> >> The comments at the `Dwarf` class in the `elf.hpp` file explain in more >> detail how a DWARF file is structured and how the parsing algorithm works to >> get to the filename and line number information. There are more class >> comments throughout the `elf.hpp` file about how different DWARF sections >> are structured and how the parsing algorithm needs to fetch the required >> information. Therefore, I will not repeat the exact workings of the >> algorithm here but refer to the code comments. I've tried to add as much >> information as possible to improve the readability. >> >> Generally, I've tried to stay away from adding any assertions as this code >> is almost always executed when already processing a VM error. Instead, the >> DWARF parser aims to just exit gracefully and possibly omit source >> information for a stack frame instead of risking to stop writing the hs_err >> file when an assertion
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v9]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 61 commits: - Merge branch 'master' into JDK-8242181 - Merge branch 'master' into JDK-8242181 - Fix build, add GCC flag gdwarf-4 to exclude DWARF 5, add assertions - Apply remaining review comments from Thomas Stuefe - Change load_dwarf_file with DwarfFilePath and DebugInfo - Revert renaming on Windows - Merge branch 'master' into JDK-8242181 - Updating some comments - Cleanup loading dwarf file and add summary - Review comments of first pass by Thomas except dwarf file loading - ... and 51 more: https://git.openjdk.java.net/jdk/compare/69ff86a3...5e105465 - Changes: https://git.openjdk.java.net/jdk/pull/7126/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=08 Stats: 2703 lines in 18 files changed: 2606 ins; 41 del; 56 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v8]
On Fri, 13 May 2022 10:04:25 GMT, Magnus Ihse Bursie wrote: >> I'm back to work again. I also had a look but could not find something on >> Google, either. I then skimmed through the old GCC manuals. I found the >> first occurrence of `-gdwarf-4` in the manual for GCC 4.5.4 >> [here](https://gcc.gnu.org/onlinedocs/gcc-4.5.4/gcc.pdf): >> >> >> - gdwarf-version >> Produce debugging information in DWARF format (if that is supported). >> This >> is the format used by DBX on IRIX 6. The value of version may be either >> 2, 3 >> or 4; the default version is 2. >> >> While the manual for GCC 4.4.7 only mentions `-gdwarf-2`: >> >> -gdwarf-2 >> Produce debugging information in DWARF version 2 format (if that is >> supported). This is the format used by DBX on >> IRIX 6. With this option, GCC >> uses features of DWARF version 3 when they are useful; version 3 is >> upward >> compatible with version 2, but may still cause problems for older >> debuggers. >> >> >> The minimum accepted GCC version is currently 5.0 according to: >> https://github.com/openjdk/jdk/blob/d5ae3833b1b71eb84fadb69c0c92851400f8921c/doc/building.md?plain=1#L341-L344 >> >> This suggests that all our supported GCC versions should accept `-gdwarf-4`. > > @chhagedorn Thanks for the research. You provide more than necessary reason > to accept `-gdwarf-4` without any further checks. Great, thanks for confirming this and reviewing the build changes! - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v8]
On Fri, 8 Apr 2022 13:34:03 GMT, Magnus Ihse Bursie wrote: >> make/autoconf/flags-cflags.m4 line 116: >> >>> 114: fi >>> 115: >>> 116: CFLAGS_DEBUG_SYMBOLS="-g -gdwarf-4" >> >> We may need to guard this with a FLAGS_COMPILER_CHECK_ARGUMENTS. Perhaps it >> should also be applied only on Linux since the whole feature is Linux only. >> What do you think @magicus? > > I'm googling around for some information about -gdwarf-4 but is mostly coming > up empty handed. :( I found > [this](https://www.phoronix.com/scan.php?page=news_item&px=GCC-11-DWARF-5-Possible-Default) > saying that dwarf-5 became default in gcc11. It also claims dwarf-4 is about > 10 years old. My guess is that all our supported gcc versions do support > -gdwarf-4, but this needs to be verified. > > In practice, we only use gcc on linux so I'm not convinced we need to have an > addition check for that. If we ever are going to support gcc on other OSes I > think we'll have many more, much harder problems, than to remove the > -gdwarf-4 flag. I'm back to work again. I also had a look but could not find something on Google, either. I then skimmed through the old GCC manuals. I found the first occurrence of `-gdwarf-4` in the manual for GCC 4.5.4 [here](https://gcc.gnu.org/onlinedocs/gcc-4.5.4/gcc.pdf): - gdwarf-version Produce debugging information in DWARF format (if that is supported). This is the format used by DBX on IRIX 6. The value of version may be either 2, 3 or 4; the default version is 2. While the manual for GCC 4.4.7 only mentions `-gdwarf-2`: -gdwarf-2 Produce debugging information in DWARF version 2 format (if that is supported). This is the format used by DBX on IRIX 6. With this option, GCC uses features of DWARF version 3 when they are useful; version 3 is upward compatible with version 2, but may still cause problems for older debuggers. The minimum accepted GCC version is currently 5.0 according to: https://github.com/openjdk/jdk/blob/d5ae3833b1b71eb84fadb69c0c92851400f8921c/doc/building.md?plain=1#L341-L344 This suggests that all our supported GCC versions should accept `-gdwarf-4`. - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v8]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 60 commits: - Merge branch 'master' into JDK-8242181 - Fix build, add GCC flag gdwarf-4 to exclude DWARF 5, add assertions - Apply remaining review comments from Thomas Stuefe - Change load_dwarf_file with DwarfFilePath and DebugInfo - Revert renaming on Windows - Merge branch 'master' into JDK-8242181 - Updating some comments - Cleanup loading dwarf file and add summary - Review comments of first pass by Thomas except dwarf file loading - Merge branch 'master' into JDK-8242181 - ... and 50 more: https://git.openjdk.java.net/jdk/compare/60281810...229f1b90 - Changes: https://git.openjdk.java.net/jdk/pull/7126/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=07 Stats: 2704 lines in 18 files changed: 2606 ins; 41 del; 57 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v2]
On Wed, 26 Jan 2022 14:23:14 GMT, Erik Joelsson wrote: >> Christian Hagedorn has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java >> >>Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> >> - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java >> >>Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> > > Build changes look good. I've pushed an update to fix the builds, add some assertions and to add the `gdwarf-4` GCC flag for now to exclude DWARF 5 and let the tests pass again (@erikj79 is that the right place? Do I need to add it at other places as well?). I think this `gdwarf-4` flag is currently the only option to eventually move forward with this RFE without extending the parser to support DWARF 5. I could still do that as a follow-up task and then remove the `gdwarf-4` flag again. But also adding DWARF 5 support in this patch is probably too much. What do you think about that? As mentioned above, I'm going to be away for the rest of the month. I will get back to this PR at the start of May again. Cheers, Christian - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v7]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision: Fix build, add GCC flag gdwarf-4 to exclude DWARF 5, add assertions - Changes: - all: https://git.openjdk.java.net/jdk/pull/7126/files - new: https://git.openjdk.java.net/jdk/pull/7126/files/06489da2..f24c284d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=05-06 Stats: 20 lines in 4 files changed: 10 ins; 5 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v5]
On Wed, 30 Mar 2022 06:35:27 GMT, Christian Hagedorn wrote: >> Hi Christian, >> >> this is impressive work. It's a big change, and I had a look at part of it. >> I'll continue tomorrow. >> >> In general, I'm concerned with the use of both UL and ResourceArea in this >> code. I know the use of UL has been discussed, but still. Use of RA will >> prevent us from getting useful callstacks if we crash and Thread::current is >> NULL or invalid. I'd feel better if we were to consistently rely on an >> outside scratch buffer (like we usually do in error reporting). Even raw >> ::malloc would be better IMHO. >> >> Another concern was safety, since this is a potential attack vector with >> manipulated Dwarf files, if someone manages to provove a crash. Maybe far >> fetched, but still. Would be good to get SonarCloud readings for this code >> e.g. >> >> More remarks inline. >> >> Cheers, Thomas > >> this is impressive work. It's a big change, and I had a look at part of it. >> I'll continue tomorrow. > > Thanks a lot Thomas for your careful review! I'm in the process of working > through your comments and will come back with an update today or later this > week. > >> In general, I'm concerned with the use of both UL and ResourceArea in this >> code. I know the use of UL has been discussed, but still. > > I agree that it is problematic but I think it would be good to keep some > logging around when later coming back to the parser code (and that's the only > reason I think that you ever want to turn these logs on). I can currently > think of two options: > > - Leave UL in and just guard it with an additional new develop flag to > exclude the logs from unfiltered UL logging. This would allow us to kinda > accept the risks for debugging purposes. That's not really a good design > though but we could keep the log levels with their time stamps. > - Replace all UL calls with `tty` and also guard them with a new develop flag > and play around with `Verbose` and `WizardMode` to keep the different log > levels. That's not great either but I think it's safer to use and we only > want the logs on rare occasions anyways - so it might be acceptable to use > these verbose flags even though we should generally get away from them. > >> Use of RA will prevent us from getting useful callstacks if we crash and >> Thread::current is NULL or invalid. I'd feel better if we were to >> consistently rely on an outside scratch buffer (like we usually do in error >> reporting). Even raw ::malloc would be better IMHO. > > The idea of a scratch buffer sounds good. I'll check if I can replace all the > `NEW_RESOURCE_ARRAY` usages with it. > >> Another concern was safety, since this is a potential attack vector with >> manipulated Dwarf files, if someone manages to provove a crash. Maybe far >> fetched, but still. Would be good to get SonarCloud readings for this code >> e.g. > > I was also concerned about that and I'm very thankful that you've spotted > some issues already! I think minimizing the risk of a potential attack should > be a top priority. We should definitely add some more checks. What do you > think about the usage of `_JVM_DWARF_PATH` to load a DWARF file? I'm not sure > how safe it is. I originally had it enabled for debug builds only. > >> We see test errors on Linux ppcle and x64 in gtests: > > Could you try running it with `-Xlog:dwarf=info/debug` in order to find out > why it failed? It might not have found the symbols. Is the JTreg test > `TestDwarf.java` working? But there is now another problem that since using > GCC 11.2 (change done for Oracle builds with > [JDK-8283057](https://bugs.openjdk.java.net/browse/JDK-8283057)), it emits > unsupported DWARF 5 for some DWARF sections, at least on my machine, which is > unfortunate. Maybe that's also the reason you see the failures if you use GCC > 11.2. Maybe we can mitigate this problem by forcing GCC to use DWARF 4 for > now. Could that be done by using the `-gdwarf` GCC flag? @erikj79 > >> We also see Problems in runtime/ErrorHandling and in >> jfr/jvm/TestDumpOnCrash. Mostly, these tests now have much longer runtimes >> (about factor 2). With TestDumpOnCrash, both the error file writer and the >> test itself timeouted on some of our slower machines. > > Are these timeouts on ppcle and x64? We could also try to add > `-Xlog:dwarf=info/debug` to the runs to get some rough idea of the time > required to parse DWARF. I'll have a look at the these tests. >
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v5]
On Mon, 28 Mar 2022 13:14:50 GMT, Thomas Stuefe wrote: > this is impressive work. It's a big change, and I had a look at part of it. > I'll continue tomorrow. Thanks a lot Thomas for your careful review! I'm in the process of working through your comments and will come back with an update today or later this week. > In general, I'm concerned with the use of both UL and ResourceArea in this > code. I know the use of UL has been discussed, but still. I agree that it is problematic but I think it would be good to keep some logging around when later coming back to the parser code (and that's the only reason I think that you ever want to turn these logs on). I can currently think of two options: - Leave UL in and just guard it with an additional new develop flag to exclude the logs from unfiltered UL logging. This would allow us to kinda accept the risks for debugging purposes. That's not really a good design though but we could keep the log levels with their time stamps. - Replace all UL calls with `tty` and also guard them with a new develop flag and play around with `Verbose` and `WizardMode` to keep the different log levels. That's not great either but I think it's safer to use and we only want the logs on rare occasions anyways - so it might be acceptable to use these verbose flags even though we should generally get away from them. > Use of RA will prevent us from getting useful callstacks if we crash and > Thread::current is NULL or invalid. I'd feel better if we were to > consistently rely on an outside scratch buffer (like we usually do in error > reporting). Even raw ::malloc would be better IMHO. The idea of a scratch buffer sounds good. I'll check if I can replace all the `NEW_RESOURCE_ARRAY` usages with it. > Another concern was safety, since this is a potential attack vector with > manipulated Dwarf files, if someone manages to provove a crash. Maybe far > fetched, but still. Would be good to get SonarCloud readings for this code > e.g. I was also concerned about that and I'm very thankful that you've spotted some issues already! I think minimizing the risk of a potential attack should be a top priority. We should definitely add some more checks. What do you think about the usage of `_JVM_DWARF_PATH` to load a DWARF file? I'm not sure how safe it is. I originally had it enabled for debug builds only. > We see test errors on Linux ppcle and x64 in gtests: Could you try running it with `-Xlog:dwarf=info/debug` in order to find out why it failed? It might not have found the symbols. Is the JTreg test `TestDwarf.java` working? But there is now another problem that since using GCC 11.2 (change done for Oracle builds with [JDK-8283057](https://bugs.openjdk.java.net/browse/JDK-8283057)), it emits unsupported DWARF 5 for some DWARF sections, at least on my machine, which is unfortunate. Maybe that's also the reason you see the failures if you use GCC 11.2. Maybe we can mitigate this problem by forcing GCC to use DWARF 4 for now. Could that be done by using the `-gdwarf` GCC flag? @erikj79 > We also see Problems in runtime/ErrorHandling and in jfr/jvm/TestDumpOnCrash. > Mostly, these tests now have much longer runtimes (about factor 2). With > TestDumpOnCrash, both the error file writer and the test itself timeouted on > some of our slower machines. Are these timeouts on ppcle and x64? We could also try to add `-Xlog:dwarf=info/debug` to the runs to get some rough idea of the time required to parse DWARF. I'll have a look at the these tests. Thanks, Christian - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v5]
On Mon, 28 Mar 2022 09:13:39 GMT, Yasumasa Suenaga wrote: > As I said before, I think it would be nice to share DWARF parser between SA > and HotSpot. Can you expose these mechanisms? It may be another RFE, and may > need to think other platforms. That would be good to have. However, I'm not familiar with the SA code and how it works to share code with HotSpot. And I'm also not sure how much overlap the two parsers actually have. I quickly skimmed through the DWARF parsing code of the SA and it seems that it's main usage is for parsing call frame information (as described in section 6.4 of the DWARF 4 spec) which is not supported/needed in this patch. There is still some code that could be shared though like opening a DWARF file with its checks or reading an LEB 128 etc. Might be worth to investigate further if the two implementations can be merged/reused to some extent. But I propose to file a separate RFE for that. What do you think? - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v5]
On Mon, 28 Mar 2022 08:44:10 GMT, Thomas Stuefe wrote: > > Ping - may I get another review for this change? > > I'll take a look later today or tomorrow. This is nice stuff, I'm looking > forward to having it upstream. That's great, thanks Thomas! - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v5]
On Mon, 28 Feb 2022 16:22:25 GMT, Christian Hagedorn wrote: >> When printing the native stack trace on Linux (mostly done for hs_err >> files), it only prints the method with its parameters and a relative offset >> in the method: >> >> Stack: [0x7f6e01739000,0x7f6e0183a000], sp=0x7f6e01838110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> >> This makes it sometimes difficult to see where exactly the methods were >> called from and sometimes almost impossible when there are multiple >> invocations of the same method within one method. >> >> This patch improves this by providing source information (filename + line >> number) to the native stack traces on Linux similar to what's already done >> on Windows (see >> [JDK-8185712](https://bugs.openjdk.java.net/browse/JDK-8185712)): >> >> Stack: [0x7f34fca18000,0x7f34fcb19000], sp=0x7f34fcb17110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> (c1_Compilation.cpp:607) >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec (c1_Compiler.cpp:250) >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> (compileBroker.cpp:2291) >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> (compileBroker.cpp:1966) >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 (compilerThread.cpp:59) >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> (thread.cpp:1297) >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 (thread.cpp:1280) >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 (thread.cpp:358) >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> (os_linux.cpp:705) >> >> For Linux, we need to parse the debug symbols which are generated by GCC in >> DWARF - a standardized debugging format. This patch adds support for DWARF >> 4, the default of GCC 10.x, for 32 and 64 bit architectures (tested with >> x86_32, x86_64 and AArch64). DWARF 5 is not supported as it was still >> experimental and not generated for HotSpot. However, newer GCC version may >> soon generate DWARF 5 by default in which case this parser either needs to >> be extended or the build of HotSpot configured to only emit DWARF 4. >> >> The code follows the parsing steps described in the official DWARF 4 spec: >> https://dwarfstd.org/doc/DWARF4.pdf >> I added references to the corresponding sections throughout the code. >> However, I tried to explain the steps from the DWARF spec directly in the >> code (method names, comments etc.). This allows to follow the code without >> the need to actually deep dive into the spec. >> >> The comments at the `Dwarf` class in the `elf.hpp` file explain in more >> detail how a DWARF file is structured and how the parsing algorithm works to >> get to the filename and line number information. There are more class >> comments throughout the `elf.hpp` file about how different DWARF sections >> are structured and how the parsing algorithm needs to fetch the required >> information. Therefore, I will not repeat the exact workings of the >> algorithm here but refer to the code comments. I've tried to add as much >> information as possible to improve the readability. >> >> Generally, I've tried to stay away from adding any assertions as this code >> is almost always executed when already processing a VM error. Instead, the >> DWARF parser aims to just exit gracefully and possibly omit source >> information for a stack frame instead of risking to stop writing the hs_err >> file when an assertion
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v5]
On Mon, 28 Feb 2022 16:22:25 GMT, Christian Hagedorn wrote: >> When printing the native stack trace on Linux (mostly done for hs_err >> files), it only prints the method with its parameters and a relative offset >> in the method: >> >> Stack: [0x7f6e01739000,0x7f6e0183a000], sp=0x7f6e01838110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> >> This makes it sometimes difficult to see where exactly the methods were >> called from and sometimes almost impossible when there are multiple >> invocations of the same method within one method. >> >> This patch improves this by providing source information (filename + line >> number) to the native stack traces on Linux similar to what's already done >> on Windows (see >> [JDK-8185712](https://bugs.openjdk.java.net/browse/JDK-8185712)): >> >> Stack: [0x7f34fca18000,0x7f34fcb19000], sp=0x7f34fcb17110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> (c1_Compilation.cpp:607) >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec (c1_Compiler.cpp:250) >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> (compileBroker.cpp:2291) >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> (compileBroker.cpp:1966) >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 (compilerThread.cpp:59) >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> (thread.cpp:1297) >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 (thread.cpp:1280) >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 (thread.cpp:358) >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> (os_linux.cpp:705) >> >> For Linux, we need to parse the debug symbols which are generated by GCC in >> DWARF - a standardized debugging format. This patch adds support for DWARF >> 4, the default of GCC 10.x, for 32 and 64 bit architectures (tested with >> x86_32, x86_64 and AArch64). DWARF 5 is not supported as it was still >> experimental and not generated for HotSpot. However, newer GCC version may >> soon generate DWARF 5 by default in which case this parser either needs to >> be extended or the build of HotSpot configured to only emit DWARF 4. >> >> The code follows the parsing steps described in the official DWARF 4 spec: >> https://dwarfstd.org/doc/DWARF4.pdf >> I added references to the corresponding sections throughout the code. >> However, I tried to explain the steps from the DWARF spec directly in the >> code (method names, comments etc.). This allows to follow the code without >> the need to actually deep dive into the spec. >> >> The comments at the `Dwarf` class in the `elf.hpp` file explain in more >> detail how a DWARF file is structured and how the parsing algorithm works to >> get to the filename and line number information. There are more class >> comments throughout the `elf.hpp` file about how different DWARF sections >> are structured and how the parsing algorithm needs to fetch the required >> information. Therefore, I will not repeat the exact workings of the >> algorithm here but refer to the code comments. I've tried to add as much >> information as possible to improve the readability. >> >> Generally, I've tried to stay away from adding any assertions as this code >> is almost always executed when already processing a VM error. Instead, the >> DWARF parser aims to just exit gracefully and possibly omit source >> information for a stack frame instead of risking to stop writing the hs_err >> file when an assertion
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v5]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 54 commits: - Updating some comments - Cleanup loading dwarf file and add summary - Review comments of first pass by Thomas except dwarf file loading - Merge branch 'master' into JDK-8242181 - Make dwarf tag NOT_PRODUCT - Change log_* to log_develop_* and log_warning to log_develop_info - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Better formatting of trace output - some code move and more cleanups - ... and 44 more: https://git.openjdk.java.net/jdk/compare/efd3967b...5bea4841 - Changes: https://git.openjdk.java.net/jdk/pull/7126/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=04 Stats: 2665 lines in 19 files changed: 2524 ins; 76 del; 65 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v4]
On Tue, 22 Feb 2022 09:59:36 GMT, Thomas Schatzl wrote:
>> Christian Hagedorn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Make dwarf tag NOT_PRODUCT
>
> src/hotspot/share/utilities/elfFile.cpp line 319:
>
>> 317: }
>> 318: log_develop_info(dwarf)("No separate .debuginfo file for library
>> %s. It already contains the required DWARF sections.", _filepath);
>> 319: _dwarf_file = new (std::nothrow) DwarfFile(_filepath);
>
> Would it be useful to explicitly bail out on a `nullptr` value here to avoid
> crashes below?
Yes, I think that's the right way. I changed other allocations as well to bail
out.
> src/hotspot/share/utilities/elfFile.cpp line 357:
>
>> 355: }
>> 356:
>> 357: strcpy(debug_pathname, _filepath);
>
> I'm always a bit uneasy using "raw" `strcpy` instead of `strncpy` and
> friends. The code seems to be correct though.
Yes that's true. I updated usages while introducing a new helper class
`DwarfFilePath`.
> src/hotspot/share/utilities/elfFile.cpp line 784:
>
>> 782: }
>> 783:
>> 784: if (!_reader.read_byte(&_header._address_size) ||
>> NOT_LP64(_header._address_size != 4) LP64_ONLY( _header._address_size !=
>> 8)) {
>
> Since this is the second time for the clause `||
> NOT_LP64(_header._address_size != 4) LP64_ONLY( _header._address_size != 8)`
> maybe it is useful to make a constant out of the accepted address size
> somewhere instead of repeating this over and over.
> It's value could even be something like `sizeof(intptr_t)` or so.
I agree, I introduced a new constant `DwarfFile::ADDRESS_SIZE`.
> src/hotspot/share/utilities/elfFile.cpp line 1070:
>
>> 1068: // reason, GCC is currently using version 3 as specified in the
>> DWARF 3 spec for the line number program even though GCC should
>> 1069: // be using version 4 for DWARF 4 as it emits DWARF 4 by default.
>> 1070: return false;
>
> According to the specification (pg112):
>
>> `version (uhalf)`
>> A version number (see Appendix F). This number is specific to the line
>> number information
>> and is independent of the DWARF version number.
>
> So this is just fine - actually things may break if the code accepted version
> 4 here assuming that there are breaking differences.
> On the other hand Appendix F mentions that DWARF4 contains .debug_line
> information in version 4.
The `LineNumberProgram` class should be able to handle both version 3 and 4.
There are some differences (see `_dwarf_version` checks). But I found that GCC
even mixes version 3 and 4:
https://github.com/chhagedorn/jdk/blob/820f0da65ab06b28ac75eec96d35269addda0246/src/hotspot/share/utilities/elfFile.cpp#L1302-L1308
> src/hotspot/share/utilities/elfFile.hpp line 211:
>
>> 209:
>> 210: // Load the DWARF file (.debuginfo) that belongs to this file.
>> 211: bool load_dwarf_file();
>
> It would be nice to summarize from which places this methods tries to load
> the debug info to prevent the need for digging for it in the method
> implementation.
Good suggestion. I added a summary and refactored the different loading
attempts into separate methods together with a new class `DwarfFilePath` which
makes it easier to prepare the different paths.
-
PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v4]
On Tue, 8 Feb 2022 08:17:17 GMT, Christian Hagedorn wrote: >> When printing the native stack trace on Linux (mostly done for hs_err >> files), it only prints the method with its parameters and a relative offset >> in the method: >> >> Stack: [0x7f6e01739000,0x7f6e0183a000], sp=0x7f6e01838110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> >> This makes it sometimes difficult to see where exactly the methods were >> called from and sometimes almost impossible when there are multiple >> invocations of the same method within one method. >> >> This patch improves this by providing source information (filename + line >> number) to the native stack traces on Linux similar to what's already done >> on Windows (see >> [JDK-8185712](https://bugs.openjdk.java.net/browse/JDK-8185712)): >> >> Stack: [0x7f34fca18000,0x7f34fcb19000], sp=0x7f34fcb17110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> (c1_Compilation.cpp:607) >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec (c1_Compiler.cpp:250) >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> (compileBroker.cpp:2291) >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> (compileBroker.cpp:1966) >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 (compilerThread.cpp:59) >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> (thread.cpp:1297) >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 (thread.cpp:1280) >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 (thread.cpp:358) >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> (os_linux.cpp:705) >> >> For Linux, we need to parse the debug symbols which are generated by GCC in >> DWARF - a standardized debugging format. This patch adds support for DWARF >> 4, the default of GCC 10.x, for 32 and 64 bit architectures (tested with >> x86_32, x86_64 and AArch64). DWARF 5 is not supported as it was still >> experimental and not generated for HotSpot. However, newer GCC version may >> soon generate DWARF 5 by default in which case this parser either needs to >> be extended or the build of HotSpot configured to only emit DWARF 4. >> >> The code follows the parsing steps described in the official DWARF 4 spec: >> https://dwarfstd.org/doc/DWARF4.pdf >> I added references to the corresponding sections throughout the code. >> However, I tried to explain the steps from the DWARF spec directly in the >> code (method names, comments etc.). This allows to follow the code without >> the need to actually deep dive into the spec. >> >> The comments at the `Dwarf` class in the `elf.hpp` file explain in more >> detail how a DWARF file is structured and how the parsing algorithm works to >> get to the filename and line number information. There are more class >> comments throughout the `elf.hpp` file about how different DWARF sections >> are structured and how the parsing algorithm needs to fetch the required >> information. Therefore, I will not repeat the exact workings of the >> algorithm here but refer to the code comments. I've tried to add as much >> information as possible to improve the readability. >> >> Generally, I've tried to stay away from adding any assertions as this code >> is almost always executed when already processing a VM error. Instead, the >> DWARF parser aims to just exit gracefully and possibly omit source >> information for a stack frame instead of risking to stop writing the hs_err >> file when an assertion
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v4]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision: Make dwarf tag NOT_PRODUCT - Changes: - all: https://git.openjdk.java.net/jdk/pull/7126/files - new: https://git.openjdk.java.net/jdk/pull/7126/files/698663b9..820f0da6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v3]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision: Change log_* to log_develop_* and log_warning to log_develop_info - Changes: - all: https://git.openjdk.java.net/jdk/pull/7126/files - new: https://git.openjdk.java.net/jdk/pull/7126/files/7ddb7737..698663b9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=01-02 Stats: 74 lines in 2 files changed: 0 ins; 0 del; 74 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files
On Mon, 31 Jan 2022 22:12:30 GMT, David Holmes wrote: > Hi Christian, > > Sorry for the delay in coming back to this, I wanted to see what other > feedback arose. No problem, thanks for your feedback David! > > > That's a valid concern. I've also asked myself this question when I had > > > initially started using some assertions. We should not crash again during > > > error reporting. I've therefore tried to be as conservative as possible > > > and added bailouts instead, also in loops when reading data. But of > > > course, this is just a best effort and by no means a guarantee to be safe > > > (especially in terms of crashes). What could be alternatives to make this > > > better? > > > > > > If the parsing code turns out to be very problematic in a signal handling > > context, then we could disable it in that context. So we really want to try > > and do a lot of testing by throwing random signals at the VM and see what > > breaks. > > Source information in hs-err file stacks can be tremendously useful. Lets try > the retry-callstack-dumping without features idea in case of a secondary > crash, outlined above, first. Should we still handle that in a separate RFE later or should this go along with this patch/prerequisite? What do you think? > > > > Secondly, on the same issue the use of unified logging within this code > > > > seems even more likely to be problematic - I'm not aware of us > > > > currently using UL during error reporting. It may work in basic > > > > usecases but if it triggers logfile rotation or other more complex > > > > actions what then? > > > > > > > > > I haven't thought about this before. To be honest, I think UL printing of > > > the `dwarf` tag is only useful during development when adding something > > > new to the parser or when debugging. I don't see much value of these > > > messages otherwise - even less for a Java user. As a first step, I could > > > change the logs from `log_X()` to `log_develop_X()` but that just shifts > > > the problem to non-product builds. Another option (or additional thing) > > > could be to guard the log messages with a new develop flag that's > > > disabled by default. By setting it for development, we accept that it > > > might be unsafe which should be fine. > > > > > > I think changing the logging to develop only is a reasonable step. I don't > > see logging of crash handling / error reporting as generally useful for the > > end user. > > I think the right way to go longterm would be to give us a minimalistic safe > logging API for these cases (signal handling, pre-initialization) or make UL > safe to use always. That would be ideal if UL usage could be made safe in the future for these cases. But as for now, I will start by changing the logging to develop to limit UL usage to debug builds only which does not affect end users anymore. - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v2]
On Fri, 28 Jan 2022 09:58:26 GMT, Thomas Stuefe wrote: > > > Two general remarks. One concern I have is that the new functionality > > > should be super stable, since nothing is more annoying than to crash > > > during stack dumping in hs-err file; I much rather have a call stack > > > without bells and whistles than an abridged one. Maybe we could, in > > > hs-err printing, if we got secondary crashes during callstack dumping, > > > repeat the step with all optional features (also name demangling) > > > disabled? This could also be done in a separate RFE. We'll know when this > > > happens, we can react then. > > > > > > I absolutely agree - stability should be the primary concern. An incomplete > > hs-err file should be avoided at any cost. Doing an additional "catch and > > repeat without optional features" sounds interesting to get more safety. > > Would such a thing be easy to add? Yes, it might be better to do that in a > > separate RFE. > > It is probably easy, but I also thing this would be better in a separate RFE. > And we already have a timeout per reporting step since JDK-8166944, so that > long-running steps don't spoil error reporting for everyone. We can just add > a second call stack print step if the first one failed. That sounds great. And good to know about JDK-8166944! > > The parsing of a single pc takes a little less than 0.01s. Of course, this > > is not a great way to measure performance. It also highly depends on the > > source files themselves, the machine setup etc. Thus, this cannot be > > considered a valid performance test. But still, I think these numbers can > > give us some indication of the order of magnitude. Compared to the current > > `ErrorLogTimeout` default value of 2min this looks promising. > > Okay, this looks reasonable. In our case, I remember having a very slow file > system and an overloaded machine. But this would be solved also by just > repeating call stack printing if the first attempt times out. Yes, I think so, too. Ah, I see, yes that would be an option in this case. > Cheers, and thanks for this patch! You're welcome! I'm glad this can help with the analysis of crashes in the future :-) Cheers, Christian - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v2]
On Fri, 28 Jan 2022 02:26:21 GMT, Yasumasa Suenaga wrote: > I think this feature is very useful, thanks Christian! Thanks Yasumasa! > SA already has similar feature to gather call stacks with DWARF, so it would > be nice to share DWARF parser between SA and HotSpot. I agree, that would be a good idea! > P.S. I've proposed to use elfutils to parse DWARF in SA in > [JDK-8245234](https://bugs.openjdk.java.net/browse/JDK-8245234). Ah that's interesting, I've missed that. I had a look at SA when I've started to do some work on this patch back in 2020 and even took some things over. - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v2]
On Thu, 27 Jan 2022 13:43:00 GMT, Zhengyu Gu wrote: > > That's interesting. Is this implementation still around somewhere? I'm glad > > that some of the mentioned things are not a problem anymore. > > Not I know. IIRC, it was based on DWARF 2. Okay, thanks. > > > > * Different compiler (and different version of the same compiler) can > > > generate DWARF with different version, may not be compatible with each > > > other, as DWARF allows custom fields. > > > * Maintenance cost to catch up DWARF spec/compiler changes. > > > > > > That's indeed a problem of facing different DWARF versions. For this > > parser, I tried to support the current default of GCC 10.x which is DWARF > > 4. This standard was introduced in 2010 and is probably used by most > > compilers nowadays at least (if not already DWARF 5 which was introduced in > > 2017). However, even with GCC 10.x, it emitted DWARF 3 for one of the > > sections (I'm not sure why) which I also needed to support - thus you can > > never be sure. > > DWARF 5 is still experimental for GCC 10.x and had some issues when I tried > > that out back there - so I stayed away from implementing parsing steps for > > it. But now with GCC 11.x, DWARF 5 seems to have become the default. I > > might have to try out what's being emitted for HotSpot. But I think for > > now, it is better to only focus on DWARF 4 instead of trying to support > > various versions in one patch - we could still come back to that later if > > it becomes widely used. Even if DWARF 5 is emitted, GCC could be > > configured, for example, to emit DWARF 4 only which is probably an > > acceptable workaround for testing environments. > > I think maintenance and test could be major pain points. Based on build.html, > we can use gcc version anywhere between 5.0 and 10.2, it could be a challenge > to ensure all supported version work correctly. I agree, that wide range is a problem and older GCC versions emitting older DWARF version are not covered with this patch. If I parse a DWARF section header with an unsupported version I will just bail out and it falls back to the stack trace we are seeing today without source information. That's probably fine for the scope of this patch. We could still come back and add support for other missing versions. - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v2]
On Thu, 27 Jan 2022 09:26:50 GMT, Thomas Stuefe wrote: > Hi Christian, this is very nice and useful! Thanks Thomas! > Two general remarks. One concern I have is that the new functionality should > be super stable, since nothing is more annoying than to crash during stack > dumping in hs-err file; I much rather have a call stack without bells and > whistles than an abridged one. Maybe we could, in hs-err printing, if we got > secondary crashes during callstack dumping, repeat the step with all optional > features (also name demangling) disabled? This could also be done in a > separate RFE. We'll know when this happens, we can react then. I absolutely agree - stability should be the primary concern. An incomplete hs-err file should be avoided at any cost. Doing an additional "catch and repeat without optional features" sounds interesting to get more safety. Would such a thing be easy to add? Yes, it might be better to do that in a separate RFE. > Another small concern, we parse the Elf file while dumping the stack, right? > I remember having a lot of problems on Solaris when dumping callstacks, > because there parsing the elf file was really slow. And that delayed call > stack printing by a lot, so much that the ErrorCrashTimeout often kicked in > and spoiled the crash logs for us. Yes, a pc for a frame is directly parsed when printing the corresponding frame. It takes some more time to do the additional parsing but not that much. These are the timestamps from a quick `-XX:CICrashAt=1` run with `-Xlog:dwarf=info` on my local machine on `Ubuntu 20.04` with a `fastdebug` build: [1.862s][info][dwarf] Open DWARF file: /home/christian/Downloads/test/jdk-19/fastdebug/lib/server/libjvm.debuginfo [1.867s][info][dwarf] pc: 0x7ffa35c8a9cf, offset: 0x007749cf, filename: c1_Compiler.cpp, line: 250 [1.871s][info][dwarf] pc: 0x7ffa35fbfb28, offset: 0x00aa9b28, filename: compileBroker.cpp, line: 2291 [1.876s][info][dwarf] pc: 0x7ffa35fc08e8, offset: 0x00aaa8e8, filename: compileBroker.cpp, line: 1966 [1.881s][info][dwarf] pc: 0x7ffa36e50cca, offset: 0x0193acca, filename: thread.cpp, line: 1297 [1.890s][info][dwarf] pc: 0x7ffa36e59010, offset: 0x01943010, filename: thread.cpp, line: 358 [1.897s][info][dwarf] pc: 0x7ffa36b3c524, offset: 0x01626524, filename: os_linux.cpp, line: 705 The parsing of a single pc takes a little less than 0.01s. Of course, this is not a great way to measure performance. It also highly depends on the source files themselves, the machine setup etc. Thus, this cannot be considered a valid performance test. But still, I think these numbers can give us some indication of the order of magnitude. Compared to the current `ErrorLogTimeout` default value of 2min this looks promising. - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v2]
On Tue, 25 Jan 2022 15:06:41 GMT, Christian Hagedorn wrote: > Build changes look good. Thanks Erik! > Personally, I am in favor of this project. Actually, I were experimenting it > with libdwarf. > > I would like to add some historical background on this topic, just for > consideration. Thanks Zhengyu for sharing some background! > We had a dwarf parser over a decade ago, a little after elf parser, but never > made to mainline. There were several reasons at the time. Good news, some are > no longer applied today :-) That's interesting. Is this implementation still around somewhere? I'm glad that some of the mentioned things are not a problem anymore. > * Different compiler (and different version of the same compiler) can > generate DWARF with different version, may not be compatible with each other, > as DWARF allows custom fields. > * Maintenance cost to catch up DWARF spec/compiler changes. That's indeed a problem of facing different DWARF versions. For this parser, I tried to support the current default of GCC 10.x which is DWARF 4. This standard was introduced in 2010 and is probably used by most compilers nowadays at least (if not already DWARF 5 which was introduced in 2017). However, even with GCC 10.x, it emitted DWARF 3 for one of the sections (I'm not sure why) which I also needed to support - thus you can never be sure. DWARF 5 is still experimental for GCC 10.x and had some issues when I tried that out back there - so I stayed away from implementing parsing steps for it. But now with GCC 11.x, DWARF 5 seems to have become the default. I might have to try out what's being emitted for HotSpot. But I think for now, it is better to only focus on DWARF 4 instead of trying to support various versions in one patch - we could still come back to that later if it becomes widely used. Even if DWARF 5 is emitted, GCC could be configured, for example, to emit DWARF 4 only which is probably an acceptable workaround for testing environments. - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v2]
On Tue, 25 Jan 2022 14:00:49 GMT, Erik Joelsson wrote: > Build changes look good. The reference to the new env variable is missing the > initial underscore in two places. Thanks Erik for reviewing the build changes. I updated the places. - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v2]
F parsing could correctly > find the filename and line number. For normal HotSpot files, I could not > check against hardcoded filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity checks in the form of "found a non-empty file" and > "found a non-zero line number". On top of that, I added tests that let the VM > crash in custom C files (which will not change). This enables an additional > verification of hardcoded filenames and line numbers. > - Gtests: Directly calling the `get_source()` method which initiates DWARF > parsing. Tested some special cases, for example, having a buffer that is not > big enough to store the filename. > > On top of that, there are also existing JTreg tests that call > `-XX:NativeMemoryTracking=detail` which will print a native stack trace with > the new source information. These tests were also run as part of the standard > tier testing and can be considered as sanity tests for this implementation. > > To make tests work in our infrastructure or if some other setups want to have > debug symbols at different locations, I've added support for an additional > `_JVM_DWARF_PATH` environment variable. This variable can specify a path from > which the DWARF symbol file should be read by the parser if the default > locations do not contain debug symbols (required some `make` changes). This > is similar to what's done on Windows with `_NT_SYMBOL_PATH`. The JTreg test, > however, also works if there are no symbols available. In that case, the test > just skips all the assertion checks for the filename and line number. > > I haven't run any specific performance testing as this new code is mainly > executed when an error will exit the VM and only if symbol files are > available (which is normally not the case when using Java release builds as a > user). > > Special thanks to @tschatzl for giving me some pointers to start based on his > knowledge from a DWARF 2 parser he once wrote in Pascal and for discussing > approaches on how to retrieve the source information and to @erikj79 for > providing help for the changes required for `make`! > > Thanks, > Christian Christian Hagedorn has updated the pull request incrementally with two additional commits since the last revision: - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Update test/hotspot/jtreg/runtime/ErrorHandling/TestDwarf.java Co-authored-by: Erik Joelsson <37597443+erik...@users.noreply.github.com> - Changes: - all: https://git.openjdk.java.net/jdk/pull/7126/files - new: https://git.openjdk.java.net/jdk/pull/7126/files/f8c98a29..7ddb7737 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7126&range=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7126.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7126/head:pull/7126 PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files
On Tue, 18 Jan 2022 13:19:39 GMT, Christian Hagedorn wrote: > When printing the native stack trace on Linux (mostly done for hs_err files), > it only prints the method with its parameters and a relative offset in the > method: > > Stack: [0x7f6e01739000,0x7f6e0183a000], sp=0x7f6e01838110, free > space=1020k > Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native > code) > V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 > V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, > bool, DirectiveSet*)+0xec > V [libjvm.so+0x8303ef] > CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 > V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df > V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, > JavaThread*)+0x69 > V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d > V [libjvm.so+0x12091c9] JavaThread::run()+0x167 > V [libjvm.so+0x1206ada] Thread::call_run()+0x180 > V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f > > This makes it sometimes difficult to see where exactly the methods were > called from and sometimes almost impossible when there are multiple > invocations of the same method within one method. > > This patch improves this by providing source information (filename + line > number) to the native stack traces on Linux similar to what's already done on > Windows (see [JDK-8185712](https://bugs.openjdk.java.net/browse/JDK-8185712)): > > Stack: [0x7f34fca18000,0x7f34fcb19000], sp=0x7f34fcb17110, free > space=1020k > Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native > code) > V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 > (c1_Compilation.cpp:607) > V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, > bool, DirectiveSet*)+0xec (c1_Compiler.cpp:250) > V [libjvm.so+0x8303ef] > CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 > (compileBroker.cpp:2291) > V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df > (compileBroker.cpp:1966) > V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, > JavaThread*)+0x69 (compilerThread.cpp:59) > V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d > (thread.cpp:1297) > V [libjvm.so+0x12091c9] JavaThread::run()+0x167 (thread.cpp:1280) > V [libjvm.so+0x1206ada] Thread::call_run()+0x180 (thread.cpp:358) > V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f > (os_linux.cpp:705) > > For Linux, we need to parse the debug symbols which are generated by GCC in > DWARF - a standardized debugging format. This patch adds support for DWARF 4, > the default of GCC 10.x, for 32 and 64 bit architectures (tested with x86_32, > x86_64 and AArch64). DWARF 5 is not supported as it was still experimental > and not generated for HotSpot. However, newer GCC version may soon generate > DWARF 5 by default in which case this parser either needs to be extended or > the build of HotSpot configured to only emit DWARF 4. > > The code follows the parsing steps described in the official DWARF 4 spec: > https://dwarfstd.org/doc/DWARF4.pdf > I added references to the corresponding sections throughout the code. > However, I tried to explain the steps from the DWARF spec directly in the > code (method names, comments etc.). This allows to follow the code without > the need to actually deep dive into the spec. > > The comments at the `Dwarf` class in the `elf.hpp` file explain in more > detail how a DWARF file is structured and how the parsing algorithm works to > get to the filename and line number information. There are more class > comments throughout the `elf.hpp` file about how different DWARF sections are > structured and how the parsing algorithm needs to fetch the required > information. Therefore, I will not repeat the exact workings of the algorithm > here but refer to the code comments. I've tried to add as much information as > possible to improve the readability. > > Generally, I've tried to stay away from adding any assertions as this code is > almost always executed when already processing a VM error. Instead, the DWARF > parser aims to just exit gracefully and possibly omit source information for > a stack frame instead of risking to stop writing the hs_err file when an > assertion would have failed. To debug failures, `-Xlog:dwarf` can be used > with `info`, `debug` or `trace` which provides logging messages throughout > parsing. > > **Testing:** > Apart from manual testing, I've added two kinds of tests: > - A JTreg test: Spawns new VMs to let them crash in various ways. T
Re: RFR: 8274048 IGV: Replace usages of Collections.sort with List.sort call
On Mon, 23 Aug 2021 20:52:22 GMT, Andrey Turbanov wrote: > Collections.sort is just a wrapper, so it is better to use an instance method > directly. Looks good and trivial. - Marked as reviewed by chagedorn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5228
Re: RFR: 8265440: IGV: make node selection more visible
On Wed, 2 Jun 2021 06:22:44 GMT, Koichi Sakata wrote: > This pull request makes node selection more visible. > > At present when selecting node, node name is bolder. In addition to that, > thickness of the border is bolder after applying this code. > > I tested manually. Here are the images that I took before and after applying > this patch as follows. > > ### After Applying the Code > src="https://user-images.githubusercontent.com/60008/120432008-425f3f00-c3b4-11eb-91ed-2f940279affd.png";> width="300" alt="スクリーンショット 2021-06-02 14 40 43" > src="https://user-images.githubusercontent.com/60008/120432019-44c19900-c3b4-11eb-81ad-9412869e1c28.png";> > > Left: "0 Root" and "3 Start" are selected. > Right: "0 Root" and "3 Start" are selected. "3 Start" and "7 Parm" are > highlighted. > > src="https://user-images.githubusercontent.com/60008/120432028-468b5c80-c3b4-11eb-8673-9172761a15b2.png";> width="200" alt="スクリーンショット 2021-06-02 14 44 11" > src="https://user-images.githubusercontent.com/60008/120432029-4723f300-c3b4-11eb-833e-f6cb8dd30129.png";> > > Left: The upper right slot of "20 MergeMem" are selected. > Right: The upper right slot of "20 MergeMem" are selected and highlighted. > > ### Before Applying the Code > src="https://user-images.githubusercontent.com/60008/120432024-45f2c600-c3b4-11eb-879d-7977caaa0e39.png";> width="300" alt="スクリーンショット 2021-06-02 14 42 29" > src="https://user-images.githubusercontent.com/60008/120432025-45f2c600-c3b4-11eb-85ed-6dd8a0c8ff60.png";> > > Left: "0 Root" and "3 Start" are selected. > Right: "0 Root" and "3 Start" are selected. "3 Start" and "7 Parm" are > highlighted. Looks good! - Marked as reviewed by chagedorn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4298
