Re: RFR: JDK-8324243: Compilation failures in java.desktop module with gcc 14 [v3]
On Sat, 3 Feb 2024 10:07:26 GMT, Sam James wrote: >> This fixes building with GCC 14: >> * ~Cherry-pick a fix from Harfbuzz upstream~ >> * Apply other `-Wcalloc-transposed-args` fixes to the JDK sources >> >> -Wcalloc-transposed-args errors out with GCC 14 as the OpenJDK build uses >> -Werror. >> >> The calloc prototype is: >> >> void *calloc(size_t nmemb, size_t size); >> >> >> So, just swap the number of members and size arguments to match the >> prototype, as >> we're initialising 1 struct of size `sizeof(struct ...)`. GCC then sees >> we're not >> doing anything wrong. > > Sam James has updated the pull request incrementally with one additional > commit since the last revision: > > whitespace Thank you! - PR Comment: https://git.openjdk.org/jdk/pull/17506#issuecomment-1958804515
Integrated: JDK-8324243: Compilation failures in java.desktop module with gcc 14
On Sat, 20 Jan 2024 10:15:02 GMT, Sam James wrote: > This fixes building with GCC 14: > * ~Cherry-pick a fix from Harfbuzz upstream~ > * Apply other `-Wcalloc-transposed-args` fixes to the JDK sources > > -Wcalloc-transposed-args errors out with GCC 14 as the OpenJDK build uses > -Werror. > > The calloc prototype is: > > void *calloc(size_t nmemb, size_t size); > > > So, just swap the number of members and size arguments to match the > prototype, as > we're initialising 1 struct of size `sizeof(struct ...)`. GCC then sees we're > not > doing anything wrong. This pull request has now been integrated. Changeset: 8e5f6ddb Author:Sam James Committer: Julian Waters URL: https://git.openjdk.org/jdk/commit/8e5f6ddb68572c0cc8b6e256e423706f6f7cec94 Stats: 6 lines in 4 files changed: 2 ins; 0 del; 4 mod 8324243: Compilation failures in java.desktop module with gcc 14 Reviewed-by: jwaters, ihse, kbarrett, prr - PR: https://git.openjdk.org/jdk/pull/17506
Re: RFR: JDK-8324243: Compilation failures in java.desktop module with gcc 14 [v3]
On Sat, 3 Feb 2024 10:07:26 GMT, Sam James wrote: >> This fixes building with GCC 14: >> * ~Cherry-pick a fix from Harfbuzz upstream~ >> * Apply other `-Wcalloc-transposed-args` fixes to the JDK sources >> >> -Wcalloc-transposed-args errors out with GCC 14 as the OpenJDK build uses >> -Werror. >> >> The calloc prototype is: >> >> void *calloc(size_t nmemb, size_t size); >> >> >> So, just swap the number of members and size arguments to match the >> prototype, as >> we're initialising 1 struct of size `sizeof(struct ...)`. GCC then sees >> we're not >> doing anything wrong. > > Sam James has updated the pull request incrementally with one additional > commit since the last revision: > > whitespace Thanks, I hadn't gone back and read the original comment. - PR Comment: https://git.openjdk.org/jdk/pull/17506#issuecomment-1958506042
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v11]
On Mon, 12 Feb 2024 08:01:23 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains 15 commits: > > - Merge branch 'master' into jdk-FOB64 > - Fix indentation > - Once more, remove AIX dirent64 et al defines > - Also fix fstatvfs on AIX > - Redefine statvfs as statvfs64 on AIX > - Add kludge to BufferedRenderPipe.c for AIX > - Merge branch 'master' into jdk-FOB64 > - Remove all system includes from debug_util.h > - Merge branch 'master' into jdk-FOB64 > - Move #include out of debug_util.h > - ... and 5 more: https://git.openjdk.org/jdk/compare/efa071dd...caccbf9b Thank you again for this! - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1938202537
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v10]
On Tue, 6 Feb 2024 16:10:38 GMT, Magnus Ihse Bursie wrote: >> make/modules/jdk.hotspot.agent/Lib.gmk line 31: >> >>> 29: >>> 30: ifeq ($(call isTargetOs, linux), true) >>> 31: SA_CFLAGS := -D_FILE_OFFSET_BITS=64 >> >> We have two choices to feel a bit more comfortable: >> 1) We unconditionally `static_assert` in a few places for large `off_t`, or >> 2) We only do it for platforms we know definitely support F_O_B and aren't >> AIX (which we've handled separately). >> >> Not sure that's strictly necessary though. Also, if something understands >> LARGEFILE*_SOURCE, then it probably understood F_O_B, or at least has some >> macro to control it. Just thinking aloud. > > @thesamesam Do you want a `static_assert`? As I said, please let me know > where you want to put it. I don't think it provides much, but if it makes you > feel more comfortable, I'm okay with adding it. Sorry! I think I'm okay with it as-is. - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1482529665
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v10]
On Thu, 8 Feb 2024 07:41:02 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Once more, remove AIX dirent64 et al defines Marked as reviewed by thesame...@github.com (no known OpenJDK username). - PR Review: https://git.openjdk.org/jdk/pull/17538#pullrequestreview-1869449960
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Fri, 2 Feb 2024 15:49:59 GMT, Magnus Ihse Bursie wrote: >> I wrote earlier: >> >>> There is one change that merit highlighting: In >>> src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c, I kept the >>> dlsym lookup for openat64, fstatat64 and fdopendir64, on non-BSD OSes (i.e. >>> Linux and AIX), and on AIX, respectively. This seems to me to be the safest >>> choice, as we do not need to know if a lookup of openat would yield a >>> 32-bit or a 64-bit version. (I frankly don't know, but I'm guessing it >>> would yield the 32-bit version.) > > Basically, my understanding is that a call to "openat" in the source file > will be converted into a call to openat64 on 32-bit platforms. When we look > up the function using dlsym, I assume we need to find the 64-bit version > directly. > > Even if this is incorrect, it seems at least *safe* to do it this way. The redirection is done via aliases in the headers, so you're right. Thanks! - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1482403071
Re: RFR: 8324243: Fix GCC 14 build [v3]
> This fixes building with GCC 14: > * Cherry-pick a fix from Harfbuzz upstream > * Apply other `-Wcalloc-transposed-args` fixes to the JDK sources > > -Wcalloc-transposed-args errors out with GCC 14 as the OpenJDK build uses > -Werror. > > The calloc prototype is: > > void *calloc(size_t nmemb, size_t size); > > > So, just swap the number of members and size arguments to match the > prototype, as > we're initialising 1 struct of size `sizeof(struct ...)`. GCC then sees we're > not > doing anything wrong. Sam James has updated the pull request incrementally with one additional commit since the last revision: whitespace - Changes: - all: https://git.openjdk.org/jdk/pull/17506/files - new: https://git.openjdk.org/jdk/pull/17506/files/ef698d82..ef79e7ab Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17506&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17506&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17506.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17506/head:pull/17506 PR: https://git.openjdk.org/jdk/pull/17506
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Fri, 2 Feb 2024 06:55:19 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add kludge to BufferedRenderPipe.c for AIX src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c line 365: > 363: #else > 364: // Make sure we link to the 64-bit version of the functions > 365: my_openat_func = (openat_func*) dlsym(RTLD_DEFAULT, "openat64"); Explain this part to me, if you wouldn't mind? (Why do we keep the `64` variants?) - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1475642841
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Fri, 2 Feb 2024 06:55:19 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add kludge to BufferedRenderPipe.c for AIX make/modules/jdk.hotspot.agent/Lib.gmk line 31: > 29: > 30: ifeq ($(call isTargetOs, linux), true) > 31: SA_CFLAGS := -D_FILE_OFFSET_BITS=64 We have two choices to feel a bit more comfortable: 1) We unconditionally `static_assert` in a few places for large `off_t`, or 2) We only do it for platforms we know definitely support F_O_B and aren't AIX (which we've handled separately). Not sure that's strictly necessary though. Also, if something understands LARGEFILE*_SOURCE, then it probably understood F_O_B, or at least has some macro to control it. Just thinking aloud. src/java.base/linux/native/libnio/ch/FileDispatcherImpl.c line 94: > 92: return IOS_UNSUPPORTED_CASE; > 93: > 94: loff_t offset = (loff_t)position; Is this `loff_t` for the benefit of `copy_file_range`? - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1475635336 PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1475636686
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs [v7]
On Fri, 2 Feb 2024 06:55:19 GMT, Magnus Ihse Bursie wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Add kludge to BufferedRenderPipe.c for AIX First, huge thanks for doing this. I did have a very rough cut of this locally which I'd put to one side, and you and I have done essentially the same thing (but yours with more tact). That's a positive sign. > > Can you confirm that you've run tier1-4 at least? Some of the library code > > that is changed here is not tested in the lower tiers. > > I have run tier1-4 now, and it passes (bar the tests that are currently > failing in mainline). However, this only tests 64-bit builds, and these > changes do not affect 64-bit builds, only 32-bit linux. So the tier1-4 is > more of a sanity check that I did not inadvertenly broke any 64-bit code. > > To really test that this works properly, a 32-bit linux with an assortment of > operations on > 2GB files would be needed. To the best of my knowledge, we > have no such test environment available, and I could not even try to think of > how to create such a test setup that does anything useful. (That is, if I > even were to spend any time on creating new tests for 32-bit platforms...) Yeah, let's not, I think. The only way of doing this is with libc shims and a huge mess. As long as we have tests which handle > 2GB files in general, and then also we can get someone to run this on a 32-bit system and tell us if the test suite passes as-is, then we're fine. Really, even if it builds on a 32-bit system with strict `-Werror=x` for pointer conversions and such, then we're OK. I'll leave comments inline for the rest. - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1923093781
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs
On Mon, 29 Jan 2024 14:07:49 GMT, Joachim Kern wrote: >> Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we >> should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK >> native libraries. > > src/java.base/unix/native/libjava/UnixFileSystem_md.c line 64: > >> 62: #define closedir closedir64 >> 63: #define stat stat64 >> 64: #endif > > same as above https://www.ibm.com/docs/en/aix/7.1?topic=volumes-writing-programs-that-access-large-files implies _some_ (like `stat`) are redirected by the macro, just not all (e.g. `readdir` isn't), I think? - PR Review Comment: https://git.openjdk.org/jdk/pull/17538#discussion_r1469646177
Re: RFR: 8324539: Do not use LFS64 symbols in JDK libs
On Tue, 23 Jan 2024 15:42:55 GMT, Magnus Ihse Bursie wrote: > Similar to [JDK-8318696](https://bugs.openjdk.org/browse/JDK-8318696), we > should use -D_FILE_OFFSET_BITS=64, and not -D_LARGEFILE64_SOURCE in the JDK > native libraries. (I'll try look at this during this week.) - PR Comment: https://git.openjdk.org/jdk/pull/17538#issuecomment-1914508224
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v5]
On Fri, 19 Jan 2024 23:50:46 GMT, Sam James wrote: >> The LFS64 symbols provided by glibc are not part of any standard and were >> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). >> This commit replaces the usage of LFS64 symbols with their regular >> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions >> will always act as their -64 variants on glibc. > > Sam James has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Merge master > - crank copyright > - sendfile64 -> sendfile > >Signed-off-by: Sam James > - buf64->buf > >Signed-off-by: Sam James > - Add message for assert > >Not all C++ stds implement it w/o. > - Add off_t static_asserts > >Signed-off-by: Sam James > - Do not use LFS64 symbols on Linux > >The LFS64 symbols provided by glibc are not part of any standard and >were gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in >1.2.5). This commit replaces the usage of LFS64 symbols with their >regular counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that > functions >will always act as their -64 variants on glibc. > >Signed-off-by: Sam James I'll take a look, give my thoughts on the problem overall. Let's discuss it over on that side if that's OK. - PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1906394971
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v5]
On Fri, 19 Jan 2024 23:50:46 GMT, Sam James wrote: >> The LFS64 symbols provided by glibc are not part of any standard and were >> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). >> This commit replaces the usage of LFS64 symbols with their regular >> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions >> will always act as their -64 variants on glibc. > > Sam James has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Merge master > - crank copyright > - sendfile64 -> sendfile > >Signed-off-by: Sam James > - buf64->buf > >Signed-off-by: Sam James > - Add message for assert > >Not all C++ stds implement it w/o. > - Add off_t static_asserts > >Signed-off-by: Sam James > - Do not use LFS64 symbols on Linux > >The LFS64 symbols provided by glibc are not part of any standard and >were gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in >1.2.5). This commit replaces the usage of LFS64 symbols with their >regular counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that > functions >will always act as their -64 variants on glibc. > >Signed-off-by: Sam James Could someone run the other commands again for older branches for me as well? I don't have access. - PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1903889096
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v5]
On Fri, 19 Jan 2024 23:50:46 GMT, Sam James wrote: >> The LFS64 symbols provided by glibc are not part of any standard and were >> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). >> This commit replaces the usage of LFS64 symbols with their regular >> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions >> will always act as their -64 variants on glibc. > > Sam James has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Merge master > - crank copyright > - sendfile64 -> sendfile > >Signed-off-by: Sam James > - buf64->buf > >Signed-off-by: Sam James > - Add message for assert > >Not all C++ stds implement it w/o. > - Add off_t static_asserts > >Signed-off-by: Sam James > - Do not use LFS64 symbols on Linux > >The LFS64 symbols provided by glibc are not part of any standard and >were gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in >1.2.5). This commit replaces the usage of LFS64 symbols with their >regular counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that > functions >will always act as their -64 variants on glibc. > >Signed-off-by: Sam James We will need to backport this to at least OpenJDK 17 as well as OpenJDK 11, if possible. We had downstream reports of failure there in Gentoo. I haven't checked OpenJDK 8 although it's almost certainly needed there too. - PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1902058430
Integrated: 8318696: Do not use LFS64 symbols on Linux
On Tue, 24 Oct 2023 01:36:32 GMT, Sam James wrote: > The LFS64 symbols provided by glibc are not part of any standard and were > gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). > This commit replaces the usage of LFS64 symbols with their regular > counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will > always act as their -64 variants on glibc. This pull request has now been integrated. Changeset: f4d08ccf Author:Sam James Committer: Kim Barrett URL: https://git.openjdk.org/jdk/commit/f4d08ccf80812d4f26a148fac6bf99b96672a63f Stats: 26 lines in 5 files changed: 5 ins; 0 del; 21 mod 8318696: Do not use LFS64 symbols on Linux Reviewed-by: ihse, dholmes, kbarrett, mbaesken - PR: https://git.openjdk.org/jdk/pull/16329
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v5]
On Fri, 19 Jan 2024 23:50:46 GMT, Sam James wrote: >> The LFS64 symbols provided by glibc are not part of any standard and were >> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). >> This commit replaces the usage of LFS64 symbols with their regular >> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions >> will always act as their -64 variants on glibc. > > Sam James has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Merge master > - crank copyright > - sendfile64 -> sendfile > >Signed-off-by: Sam James > - buf64->buf > >Signed-off-by: Sam James > - Add message for assert > >Not all C++ stds implement it w/o. > - Add off_t static_asserts > >Signed-off-by: Sam James > - Do not use LFS64 symbols on Linux > >The LFS64 symbols provided by glibc are not part of any standard and >were gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in >1.2.5). This commit replaces the usage of LFS64 symbols with their >regular counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that > functions >will always act as their -64 variants on glibc. > >Signed-off-by: Sam James Yes, please. I don't think I'm able to file one myself. - PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1901868552
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v5]
> The LFS64 symbols provided by glibc are not part of any standard and were > gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). > This commit replaces the usage of LFS64 symbols with their regular > counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will > always act as their -64 variants on glibc. Sam James has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Merge master - crank copyright - sendfile64 -> sendfile Signed-off-by: Sam James - buf64->buf Signed-off-by: Sam James - Add message for assert Not all C++ stds implement it w/o. - Add off_t static_asserts Signed-off-by: Sam James - Do not use LFS64 symbols on Linux The LFS64 symbols provided by glibc are not part of any standard and were gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). This commit replaces the usage of LFS64 symbols with their regular counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will always act as their -64 variants on glibc. Signed-off-by: Sam James - Changes: - all: https://git.openjdk.org/jdk/pull/16329/files - new: https://git.openjdk.org/jdk/pull/16329/files/5a66616f..29e5c55f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=03-04 Stats: 46198 lines in 1358 files changed: 28040 ins; 12096 del; 6062 mod Patch: https://git.openjdk.org/jdk/pull/16329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16329/head:pull/16329 PR: https://git.openjdk.org/jdk/pull/16329
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v4]
On Fri, 19 Jan 2024 10:37:42 GMT, Sam James wrote: >> The LFS64 symbols provided by glibc are not part of any standard and were >> gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). >> This commit replaces the usage of LFS64 symbols with their regular >> counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions >> will always act as their -64 variants on glibc. > > Sam James has updated the pull request incrementally with two additional > commits since the last revision: > > - sendfile64 -> sendfile > >Signed-off-by: Sam James > - buf64->buf > >Signed-off-by: Sam James Thank you all for the helpful review comments and see you for round 2 soon :) - PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1900182026
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v3]
On Fri, 19 Jan 2024 10:06:33 GMT, Kim Barrett wrote:
>> Sam James has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add message for assert
>>
>> Not all C++ stds implement it w/o.
>
> src/hotspot/os/linux/os_linux.cpp line 4252:
>
>> 4250: // otherwise, returns -1 that implies an error
>> 4251: jlong os::Linux::sendfile(int out_fd, int in_fd, jlong* offset, jlong
>> count) {
>> 4252: return ::sendfile64(out_fd, in_fd, (off_t*)offset, (size_t)count);
>
> Why is this continuing to use sendfile64, rather than sendfile?
oh, great spot! I'll take you up on the recommendation for banning the *64
functions in one of the followups.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16329#discussion_r1458756249
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v4]
> The LFS64 symbols provided by glibc are not part of any standard and were > gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). > This commit replaces the usage of LFS64 symbols with their regular > counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will > always act as their -64 variants on glibc. Sam James has updated the pull request incrementally with two additional commits since the last revision: - sendfile64 -> sendfile Signed-off-by: Sam James - buf64->buf Signed-off-by: Sam James - Changes: - all: https://git.openjdk.org/jdk/pull/16329/files - new: https://git.openjdk.org/jdk/pull/16329/files/e02b0715..5a66616f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=02-03 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/16329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16329/head:pull/16329 PR: https://git.openjdk.org/jdk/pull/16329
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v3]
> The LFS64 symbols provided by glibc are not part of any standard and were > gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). > This commit replaces the usage of LFS64 symbols with their regular > counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will > always act as their -64 variants on glibc. Sam James has updated the pull request incrementally with one additional commit since the last revision: Add message for assert Not all C++ stds implement it w/o. - Changes: - all: https://git.openjdk.org/jdk/pull/16329/files - new: https://git.openjdk.org/jdk/pull/16329/files/4df24d81..e02b0715 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=01-02 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16329/head:pull/16329 PR: https://git.openjdk.org/jdk/pull/16329
Re: RFR: 8318696: Do not use LFS64 symbols on Linux [v2]
> The LFS64 symbols provided by glibc are not part of any standard and were > gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). > This commit replaces the usage of LFS64 symbols with their regular > counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will > always act as their -64 variants on glibc. Sam James has updated the pull request incrementally with one additional commit since the last revision: Add off_t static_asserts Signed-off-by: Sam James - Changes: - all: https://git.openjdk.org/jdk/pull/16329/files - new: https://git.openjdk.org/jdk/pull/16329/files/4432b9d8..4df24d81 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=00-01 Stats: 5 lines in 3 files changed: 5 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16329/head:pull/16329 PR: https://git.openjdk.org/jdk/pull/16329
Re: RFR: 8318696: Do not use LFS64 symbols on Linux
On Tue, 24 Oct 2023 01:36:32 GMT, Sam James wrote: > The LFS64 symbols provided by glibc are not part of any standard and were > gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). > This commit replaces the usage of LFS64 symbols with their regular > counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will > always act as their -64 variants on glibc. Ah, sorry, I thought I'd pushed those asserts ;) Yes, this PR would be identical under the plan other than adding the asserts I promised and forgot about. Let me look now. - PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1899812128
Re: RFR: 8318696: Do not use LFS64 symbols on Linux
On Fri, 15 Dec 2023 12:28:48 GMT, David Holmes wrote: >From what version of glibc can we be sure that the non-64 version is exactly >equivalent to the 64 version for all these functions? Fortunately, support for `_FILE_OFFSET_BITS=64` making `off_t` 64-bit has been supported for quite a long time in glibc (unlike 64-bit `time_t` on 32-bit arches which is _way_ more recent, like the last 2 years...). It dates back to 1997. The exception is for `fts.h` which got wired up in 2015 (8b7b7f75d91f7bac323dd6a370aeb3e9c5c4a7d5) (glibc-2.23). OpenJDK doesn't use fts, so we're fine. It errored out before then anyway, it didn't silently fallback to 32-bit. >There are also uses of some 64 functions in the JDK code as well. This requires closer inspection to determine how broken they are outside of these changes. For some of the cases in the JDK code, `foo64()` functions are only used in some cases on AIX, leaving `foo()` used on e.g. 32-bit Linux. Some of them might be semantically broken already, not just at build-time with newer musl where `off_t` is always 64-bit. My preference for moving forward is: 1) I add some `static_assert` for `off_t` to the modified JVM bits to be safe (that's what I tested with); 2) We keep this PR for the build-only fixes which are semantically identical to the previous code - this PR currently "preserves" bitness, it doesn't fix anything other than avoiding use of `foo64()` where it's unnecessary by converting to foo, but the `foo64()` use was right - just a glibcism - until now. From what I can tell, the JVM code touched here was correct, just relying on glibcisms; 3) In another PR, we look at the general 32-bit LFS situation which may involve runtime fixes if - as it appears - `off_t` is actually 32-bit on x86 systems right now and non `foo64()` functions are being used. Then we either port more stuff to use `foo64()` (not ideal), or consistently use `foo()` in more places with `FILE_OFFSET_BITS`. My hope is to separate these semantic changes from the equivalent-behaviour stuff both to ease review but also because there's a bunch of projects which need noop fixes like this still that I need to look at. I can't promise to be able to do 3) immediately though. I can probably start it soon, I just can't promise to get the whole thing done quickly. What do you think? - PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1858739783
Re: RFR: 8318696: Do not use LFS64 symbols on Linux
On Tue, 24 Oct 2023 01:36:32 GMT, Sam James wrote: > The LFS64 symbols provided by glibc are not part of any standard and were > gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). > This commit replaces the usage of LFS64 symbols with their regular > counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will > always act as their -64 variants on glibc. I've already got an OCA processed for MySQL in the past. I can't see a signup button for the bug tracker so I can't file an issue to include as a link. Thanks! I need to pipe config.h through to handle the x86 failure. I'll do that either today or the day after tomorrow. OK, this should be better now. I've checked with static asserts that the definition propagates and it also errors out without those anyway on incompat. ptr. types. - PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1776330393 PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1776334231 PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1777669228 PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-126306 PR Comment: https://git.openjdk.org/jdk/pull/16329#issuecomment-1857301791
RFR: 8318696: Do not use LFS64 symbols on Linux
The LFS64 symbols provided by glibc are not part of any standard and were gated behind -D_LARGEFILE64_SOURCE in musl 1.2.4 (to be removed in 1.2.5). This commit replaces the usage of LFS64 symbols with their regular counterparts and defines -D_FILE_OFFSET_BITS=64, ensuring that functions will always act as their -64 variants on glibc. - Commit messages: - Do not use LFS64 symbols on Linux Changes: https://git.openjdk.org/jdk/pull/16329/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16329&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8318696 Stats: 18 lines in 5 files changed: 0 ins; 0 del; 18 mod Patch: https://git.openjdk.org/jdk/pull/16329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16329/head:pull/16329 PR: https://git.openjdk.org/jdk/pull/16329
