RFR: 8288396: Always create reproducible builds
When we started introducing some possibly more intrusive compiler flags and functionality for reproducible builds, we also introduced a flag to turn this off out of an abundance of caution. But we have been been using this configuration for a year or so internally within Oracle, with no issues. So there's really no reason to be able to turn this off. (If you were to ask me, the fact that compilers and build tools ever started to produce non-deterministic output has been a bug from day one.) With this fix, all randomness should be gone from our builds, at least on linux and windows. There are no more `__DATE__` and `__TIME__` macros in the source code. - Commit messages: - 8288396: Always create reproducible builds Changes: https://git.openjdk.org/jdk/pull/9152/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9152=00 Issue: https://bugs.openjdk.org/browse/JDK-8288396 Stats: 94 lines in 15 files changed: 11 ins; 63 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/9152.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9152/head:pull/9152 PR: https://git.openjdk.org/jdk/pull/9152
Re: RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
On Mon, 13 Jun 2022 09:47:48 GMT, Severin Gehwolf wrote: >> What do you mean by "build time information"? > > @magicus I think it's `-Xinternalversion` which has different output between > Windows and Linux of the same build. But to me that's a feature not a bug. > From the PR description: > >> With default configuration shows `-Xinternalversion` output same as Windows, >> [...] @jerboaa I agree. If anything, this PR makes me want to remove the option to *not* build reproducible. That way, there will be no discrepancies between platforms. - PR: https://git.openjdk.org/jdk/pull/9081
Re: RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
On Wed, 8 Jun 2022 07:47:24 GMT, KIRIYAMA Takuya wrote: > At default configuration, SOURCE_DATE_EPOCH is exported as environment > variable in SetupReproducibleBuild. Then, gcc is affected of > SOURCE_DATE_EPOCH environment variable. This value is used only to set > SOURCE_DATE_ISO_8601 (except below), so I removed "export" for > SOURCE_DATE_EPOCH in SetupReproducibleBuild. And, at building ct.sym, > SOURCE_DATE_EPOCH environment variable is needed. So I added setting routine > if SOURCE_DATE_EPOCH is not set. > > This fix works fine. With default configuration shows -Xinternalversion > output same as Windows, and with --with-source-date configuration shows > -Xinternalversion output specified timestamp. Would you please review this > fix? Is the problem that on Windows we get -Xinternalversion in local timezone but on Linux we get UTC? That's a difference that may warrant fixing, but I don't think this is the way to do it. I don't think it makes a practical difference if the timestamp is taken at the start of the build or when a certain src file is compiled. - PR: https://git.openjdk.org/jdk/pull/9081
Re: RFR: 8288396: Always create reproducible builds
On Tue, 14 Jun 2022 09:48:25 GMT, Magnus Ihse Bursie wrote: > When we started introducing some possibly more intrusive compiler flags and > functionality for reproducible builds, we also introduced a flag to turn this > off out of an abundance of caution. But we have been been using this > configuration for a year or so internally within Oracle, with no issues. So > there's really no reason to be able to turn this off. (If you were to ask me, > the fact that compilers and build tools ever started to produce > non-deterministic output has been a bug from day one.) > > With this fix, all randomness should be gone from our builds, at least on > linux and windows. There are no more `__DATE__` and `__TIME__` macros in the > source code. This PR also include a more "radical" version of JDK-8287894, which probably should have been adopted by JDK-8287894 in the first place. There is no need to include the build date in the assert strings for shmem on Windows. - PR: https://git.openjdk.org/jdk/pull/9152
Re: RFR: 8288396: Always create reproducible builds [v2]
On Tue, 14 Jun 2022 10:09:37 GMT, Magnus Ihse Bursie wrote: >> When we started introducing some possibly more intrusive compiler flags and >> functionality for reproducible builds, we also introduced a flag to turn >> this off out of an abundance of caution. But we have been been using this >> configuration for a year or so internally within Oracle, with no issues. So >> there's really no reason to be able to turn this off. (If you were to ask >> me, the fact that compilers and build tools ever started to produce >> non-deterministic output has been a bug from day one.) >> >> With this fix, all randomness should be gone from our builds, at least on >> linux and windows. There are no more `__DATE__` and `__TIME__` macros in the >> source code. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix exitTransportWithError signature I made some additional cleanup associated with shmem. I think that `sysAssert` can (and probably should) be replaced with `SHMEM_ASSERT`. I can fix that as well, if someone from serviceability says that I should do it. - PR: https://git.openjdk.org/jdk/pull/9152
Integrated: 8287906: Rewrite of GitHub Actions (GHA) sanity tests
On Tue, 7 Jun 2022 13:05:49 GMT, Magnus Ihse Bursie wrote: > With project Skara, the ability to run a set of sanity build and test jobs on > selected platforms was added. This functionality was driven by > `.github/workflows/submit.yml`. This file unfortunately lacks any real > structure, and contains a lot of code duplication and redundancy. This has > made it hard to add functionality, and new platforms to test, and it has made > it even harder to debug issues. (This is hard enough as it is, since we have > no direct access to the platforms that GHA runs on.) > > Since the GHA tests are important for a large subset of the community, we > need to do better. > > ## GitHub Actions framework rewrite > > This is a complete overhaul of the GHA testing framework. I started out > trying to just tease the old `submit.yml` apart, trying to de-duplicate code, > but I soon realized a much more thorough rework was needed. > > ### Design description > > The principle for the new design was to avoid code duplication, and to > improve readability of the code. The latter is extra important since the GHA > "language" is very limited, needs a lot of quirks and workarounds, and is > probably not well known by many OpenJDK developers. I've strived to find > useful layers of abstraction to make the expressions as clear as possible. > > Unfortunately, the Workflow/Action YAML language is quite limited. There are > two ways to avoid duplication, "local composite actions" and "callable > workflows". They both have several limitations: > > * "Callable workflows" can only be used in a single redirection. They are > (apparently) inlined into the "calling workflow" at run time, and as such, > they are present without having to check out the source code. (Which is a > lengthy process.) > > * "Local composite actions" can use other actions, but you must start by > checking out the repo. > > To use the strength of both kinds of sub-modules, I'm using "callable > workflows" from `main.yml` to call `build-.yml` and `test.yml`. It > is not allowed to mix "strategies" (that is, the method of automatically > creating a test matrix) when calling callable workflows, so I needed to have > some amount of duplication in `main.yml` that could have been avoided > otherwise. > > All the callable workflows need to check out the source code anyway, so there > is no real additional cost of using "local composite actions" for abstraction > of these workflows. (A bit of a lucky break.) I've created "high level" > actions, corresponding to something like a function call. The goal here was > both to avoid duplication, and to improve readability of the workflows. > > The four `build-.yml` files are very similar. But in the end of the > day, only like 50% of the source code is shared, and the platform specific > changes permeate the files. So I decided to keep them separately, since > mixing them all into one would have made a mess, due to the lack of proper > abstraction mechanisms. But that also mean that if we change platform > independent code in building, we need to remember to update it in all four > places. > > In the strictest sense, this is a "refactoring" in that the functionality > should be equal to the old `submit.yml`. The same platforms should build, > with the same arguments, and the same tests should run. When I look at the > code now, I see lots of potential for improvement here, by rethinking what we > do run. But let's save that discussion for the next PR. > > There is one major change, though. Windows is no longer running on Cygwin, > but on MSYS2. This was not really triggered by the recurring build issues on > Cygwin (though that certainly did help me in thinking I made the right > choice), but the sheer impossibility of getting Cygwin to behave as a normal > unix shell on GHA Windows hosts. I spent countless hours trying to work out > limitations, by setting `SHELLOPTS=igncr`, by running `set +x posix` to turn > of the POSIX compliance mode that kept turning on by itself and made bash > choke on several of our scripts, by playing tricks with the `PATH`, but in > the end to no avail. There were no single combination of hacks and > workarounds that could get us past the entire chain from configure, to build, > to testing. (The old solution user PowerShell instead to get around these > limitations.) I'm happy to report that I have had absolutely zero issues with > MSYS2 since I made the switch (and understood how to set the PATH properly), > and I'm seriously co nsidering switching stance to recommend using MSYS2 instead of Cygwin as the primary winenv for building the JDK. > > ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating
Re: RFR: 8288396: Always create reproducible builds [v2]
> When we started introducing some possibly more intrusive compiler flags and > functionality for reproducible builds, we also introduced a flag to turn this > off out of an abundance of caution. But we have been been using this > configuration for a year or so internally within Oracle, with no issues. So > there's really no reason to be able to turn this off. (If you were to ask me, > the fact that compilers and build tools ever started to produce > non-deterministic output has been a bug from day one.) > > With this fix, all randomness should be gone from our builds, at least on > linux and windows. There are no more `__DATE__` and `__TIME__` macros in the > source code. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix exitTransportWithError signature - Changes: - all: https://git.openjdk.org/jdk/pull/9152/files - new: https://git.openjdk.org/jdk/pull/9152/files/8bc40ddb..e2f5fc05 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9152=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9152=00-01 Stats: 33 lines in 5 files changed: 0 ins; 27 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/9152.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9152/head:pull/9152 PR: https://git.openjdk.org/jdk/pull/9152
Re: RFR: 8288396: Always create reproducible builds [v2]
On Tue, 14 Jun 2022 11:54:40 GMT, Erik Joelsson wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix exitTransportWithError signature > > make/autoconf/flags-ldflags.m4 line 132: > >> 130: >> 131: if test "x$TOOLCHAIN_TYPE" = xmicrosoft; then >> 132: REPRODUCIBLE_LDFLAGS="-experimental:deterministic" > > For the cflag, we check that the compiler supports it, but for the linker > flag you are just setting it without a check. Before this patch, if we got > here and ENABLE_REPRODUCIBLE_BUILD was true, it meant that the test had > passed for the compiler, from which we could assume it would also work for > the linker, but that is no longer the case. Good point. - PR: https://git.openjdk.org/jdk/pull/9152
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v12]
> 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. The test > reads the created hs_err files to check if the DWARF 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
Re: RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
On Mon, 13 Jun 2022 06:50:54 GMT, KIRIYAMA Takuya wrote: >> At default configuration, SOURCE_DATE_EPOCH is exported as environment >> variable in SetupReproducibleBuild. Then, gcc is affected of >> SOURCE_DATE_EPOCH environment variable. This value is used only to set >> SOURCE_DATE_ISO_8601 (except below), so I removed "export" for >> SOURCE_DATE_EPOCH in SetupReproducibleBuild. And, at building ct.sym, >> SOURCE_DATE_EPOCH environment variable is needed. So I added setting routine >> if SOURCE_DATE_EPOCH is not set. >> >> This fix works fine. With default configuration shows -Xinternalversion >> output same as Windows, and with --with-source-date configuration shows >> -Xinternalversion output specified timestamp. Would you please review this >> fix? > > Thank you for your comments. > I understood the goal of reproducible build. But now, > ENABLE_REPRODUCIBLE_BUILD is set to false in default configuration. > Then I think minimize the effort of SOURCE_DATE_EPOCH when reproducible build > is disabled. I wonder why build time information is different from Windows > and Linux. @tkiriyama I have created https://bugs.openjdk.org/browse/JDK-8288396 for the approach I championed above, i.e. to always build reproducible instead. As part of this, the `-Xinternalversion` time stamp should be of the same format for all platforms, given the same set of options to `configure`. The associated PR is https://github.com/openjdk/jdk/pull/9152. Can you verify if this solves your issues, and if so, close this bug? - PR: https://git.openjdk.org/jdk/pull/9081
Re: RFR: 8288396: Always create reproducible builds [v2]
On Tue, 14 Jun 2022 10:09:37 GMT, Magnus Ihse Bursie wrote: >> When we started introducing some possibly more intrusive compiler flags and >> functionality for reproducible builds, we also introduced a flag to turn >> this off out of an abundance of caution. But we have been been using this >> configuration for a year or so internally within Oracle, with no issues. So >> there's really no reason to be able to turn this off. (If you were to ask >> me, the fact that compilers and build tools ever started to produce >> non-deterministic output has been a bug from day one.) >> >> With this fix, all randomness should be gone from our builds, at least on >> linux and windows. There are no more `__DATE__` and `__TIME__` macros in the >> source code. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix exitTransportWithError signature make/autoconf/flags-ldflags.m4 line 132: > 130: > 131: if test "x$TOOLCHAIN_TYPE" = xmicrosoft; then > 132: REPRODUCIBLE_LDFLAGS="-experimental:deterministic" For the cflag, we check that the compiler supports it, but for the linker flag you are just setting it without a check. Before this patch, if we got here and ENABLE_REPRODUCIBLE_BUILD was true, it meant that the test had passed for the compiler, from which we could assume it would also work for the linker, but that is no longer the case. - PR: https://git.openjdk.org/jdk/pull/9152
