[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR(m): 8185712: [windows] Improve native symbol decoder
From:       Thomas_Stüfe <thomas.stuefe () gmail ! com>
Date:       2017-08-30 12:35:31
Message-ID: CAA-vtUyq2TRjw3g6aj8G0rMPfuGMGz8GBr=J6vLMFxJjRoerqQ () mail ! gmail ! com
[Download RAW message or body]

P.S. As usual, I built slowdebug and release on x86 and x64. I ran gtests
on these platforms and jtreg tests from "hotspot/runtime/ErrorHandling.

On Wed, Aug 30, 2017 at 2:33 PM, Thomas St=C3=BCfe <thomas.stuefe@gmail.com=
>
wrote:

> Hi all,
>
> May I please have reviews for the following change.
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8185712
> Webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
> 8185712-windows-improve-native-symbol-resolver/webrev.01/webrev/
>
> (This is the followup to: https://bugs.openjdk.java.net/browse/JDK-818634=
9
> )
>
> -------------
>
> Basically, this is a reimplementation of the layer around the Windows
> Symbol API (the API used to resolve debug symbols). The old implementatio=
n
> had a number of errors and shortcomings which together caused the Windows
> native symbol resolution (and hence callstacks in error logs) to be a bit
> of a lottery. The aim of this reimplementation is to make the code more
> robust and easier to maintain.
>
> The problems with the existing implementation are listed in detail in the
> bug description.
>
> The new implementation:
>
> - uses the new centralized WindowsDbgHelper class, which wraps the
> dbghelp.dll loading, introduced with JDK-8186349
>
> - Completely bypasses the "create two instances of AbstractDecoder class
> and synchronize access to them" scheme in decoder.cpp. It does not make
> sense for windows, where we have to synchronize each access to the
> dbghelp.dll anyway - this is done one layer below in WindowsDbgHelper. Th=
e
> static methods of the shared Decoder class now directly access the static
> methods in the new SymbolEngine class, see decoder_windows.cpp.
>
> - The layer wrapping the Symbol API lives in the new symbolengine.cpp/hpp
> files. The coding takes care of properly initializing (once) the symbol A=
PI
> and of assembling the pdb search path.
>
> - Pdb search path construction is changed: where before we just added jdk
> and jvm bin directories, we now just add all directories of all loaded DL=
Ls
> (which, of course, include the jdk and jvm bin directories). That way we
> have a high chance of catching pdb files of third party libraries, as lon=
g
> as they follow the convention of putting the pdb files beside the dlls.
> This means it is easier to analyse crashes where third party DLLs are
> involved.
>
> - On Windows, we now have source file and line number in the callstack.
>
> - There is a new parameter, diagnostic and windows-only, called "Initiali=
zeDbgHelpEarly".
> That parameter is by default off. If on, it causes the symbol engine to b=
e
> initialized early, which increases the chance of good callstacks later on
> (because the initialization does not have to run in an error situation).
>
> - Added tests: gtests and a jtreg test which tests the callstack printing=
.
> All tests windows only. There is no technical reason for making them
> windows only, but I wanted to keep disturbances to other platforms to a
> minimum and these kind of tests can be shaky.
>
> Thanks a lot for reviewing this!
>
> Kind Regards, Thomas
>
>
>
>
>
[prev in list] [next in list] [prev in thread] [next in thread]