Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 27 Aug 2024 01:09:09 GMT, Dean Long wrote: >> David Holmes has updated the pull request incrementally with one additional >> commit since the last revision: >> >> more missing casts > > src/hotspot/share/classfile/javaClasses.cpp line 307: > >> 305: { >> 306: ResourceMark rm; >

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 27 Aug 2024 03:13:59 GMT, Dean Long wrote: >> David Holmes has updated the pull request incrementally with one additional >> commit since the last revision: >> >> more missing casts > > src/hotspot/share/classfile/javaClasses.cpp line 588: > >> 586: size_t utf8_len = static_cast(

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 27 Aug 2024 03:36:00 GMT, Dean Long wrote: >> David Holmes has updated the pull request incrementally with one additional >> commit since the last revision: >> >> more missing casts > > src/hotspot/share/classfile/javaClasses.cpp line 633: > >> 631: } >> 632: >> 633: int java_lang_S

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 20 Aug 2024 04:09:04 GMT, David Holmes wrote: >> This work has been split out from JDK-8328877: [JNI] The JNI Specification >> needs to address the limitations of integer UTF-8 String lengths >> >> The modified UTF-8 format used by the VM can require up to six bytes to >> represent one

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Tue, 27 Aug 2024 07:07:11 GMT, David Holmes wrote: >> src/hotspot/share/classfile/javaClasses.cpp line 307: >> >>> 305: { >>> 306: ResourceMark rm; >>> 307: size_t utf8_len = static_cast(length); >> >> I think there should be an assert that length is not negative, probably at >> t

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Tue, 27 Aug 2024 07:09:33 GMT, David Holmes wrote: >> src/hotspot/share/classfile/javaClasses.cpp line 555: >> >>> 553: bool is_latin1 = java_lang_String::is_latin1(java_string); >>> 554: >>> 555: if (length == 0) return nullptr; >> >> Should this be checking for length <= 0? It l

Re: RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v6]

2024-08-27 Thread Hamlin Li
On Tue, 27 Aug 2024 05:37:30 GMT, Fei Yang wrote: >> src/hotspot/cpu/riscv/c1_MacroAssembler_riscv.cpp line 170: >> >>> 168: mv(tmp1, (int32_t)(intptr_t)markWord::prototype().value()); >>> 169: sd(tmp1, Address(obj, oopDesc::mark_offset_in_bytes())); >>> 170: // Todo UseCompactObjectHeaders

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Tue, 27 Aug 2024 07:20:27 GMT, David Holmes wrote: >> src/hotspot/share/classfile/javaClasses.cpp line 588: >> >>> 586: size_t utf8_len = static_cast(length); >>> 587: const char* base = UNICODE::as_utf8(position, utf8_len); >>> 588: Symbol* sym = SymbolTable::new_symbol(base, >>

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Tue, 27 Aug 2024 07:23:14 GMT, David Holmes wrote: >> src/hotspot/share/classfile/javaClasses.cpp line 633: >> >>> 631: } >>> 632: >>> 633: int java_lang_String::utf8_length_as_int(oop java_string, typeArrayOop >>> value) { >> >> Why not call java_lang_String::utf8_length() here instead of

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Tue, 20 Aug 2024 04:09:04 GMT, David Holmes wrote: >> This work has been split out from JDK-8328877: [JNI] The JNI Specification >> needs to address the limitations of integer UTF-8 String lengths >> >> The modified UTF-8 format used by the VM can require up to six bytes to >> represent one

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 27 Aug 2024 08:22:57 GMT, Dean Long wrote: >> David Holmes has updated the pull request incrementally with one additional >> commit since the last revision: >> >> more missing casts > > src/hotspot/share/utilities/utf8.cpp line 127: > >> 125: prev = c; >> 126: } >> 127: retur

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 27 Aug 2024 07:51:38 GMT, Dean Long wrote: >> I'm trying to reason if on 32-bit we could even create a large enough string >> for this to be a problem? Once we have the giant string `as_utf8` will have >> to allocate an array that is just as large if not larger. So for overflow to >> b

Re: RFR: 8204681: Option to include timestamp in hprof filename

2024-08-27 Thread Andrey Turbanov
On Tue, 13 Aug 2024 15:07:17 GMT, Sonia Zaldana Calles wrote: > Hi all, > > This PR addresses [8204681](https://bugs.openjdk.org/browse/JDK-8204681) > enabling support for timestamp expansion in filenames specified in > `-XX:HeapDumpPath` using `%t`. > > As mentioned in this comments for t

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Thomas Stuefe
On Tue, 27 Aug 2024 12:20:04 GMT, David Holmes wrote: >> I think the Java string would only need to be INT_MAX/3 in length, if all >> the characters require surrogate encoding. > > IIUC for compact strings, with non-latin-1 each pair of bytes would require > at most 3-bytes to encode so you'd n

Re: RFR: 8338768: Introduce runtime lookup to check for static builds [v2]

2024-08-27 Thread Magnus Ihse Bursie
On Mon, 26 Aug 2024 09:39:28 GMT, Magnus Ihse Bursie wrote: >> I understand the cost overhead experienced by any individual Java run may be >> lost in the noise, but it still impacts every single Java run just to save >> some time/resources for the handful of builders of statically linked VMs.

Re: RFR: 8338768: Introduce runtime lookup to check for static builds [v2]

2024-08-27 Thread Magnus Ihse Bursie
On Wed, 21 Aug 2024 22:14:40 GMT, Magnus Ihse Bursie wrote: >> As a preparation for Hermetic Java, we need to have a way to look up during >> runtime if we are using a statically linked library or not. >> >> This change will be the first step needed towards compiling the object files >> only o

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Tue, 27 Aug 2024 12:10:36 GMT, David Holmes wrote: >> src/hotspot/share/utilities/utf8.cpp line 127: >> >>> 125: prev = c; >>> 126: } >>> 127: return checked_cast(num_chars); >> >> Ideally, this function would return size_t. > > Why? I think that would have a large flow on effect. An

Re: RFR: 8338768: Introduce runtime lookup to check for static builds [v2]

2024-08-27 Thread Jiangli Zhou
On Tue, 27 Aug 2024 13:55:51 GMT, Magnus Ihse Bursie wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Also update build to link properly > > And the discussion whether the checks are made "dynamically" or "sta

Re: RFR: 8338768: Introduce runtime lookup to check for static builds [v2]

2024-08-27 Thread Jiangli Zhou
On Wed, 21 Aug 2024 22:14:40 GMT, Magnus Ihse Bursie wrote: >> As a preparation for Hermetic Java, we need to have a way to look up during >> runtime if we are using a statically linked library or not. >> >> This change will be the first step needed towards compiling the object files >> only o

Re: RFR: 8319873: Add windows implementation for jcmd System.map and System.dump_map [v6]

2024-08-27 Thread Simon Tooke
> This is a port of [JDK-8318636](https://github.com/openjdk/jdk/pull/16301) to > Windows. > > System.map and System.dump_map are implemented using the Windows API and > provide roughly the same information in the same format. Most of the heavy > lifting was implemented by @tstuefe in #16301 -

Re: RFR: 8319873: Add windows implementation for jcmd System.map and System.dump_map [v6]

2024-08-27 Thread Simon Tooke
On Thu, 22 Aug 2024 16:30:56 GMT, Simon Tooke wrote: >> But (here and in other places) raw-printing the unknown constant may not be >> the best way. Chances are this code is executed rarely and most people will >> just be perplexed at the weird printouts. >> >> We could assert here: After all,

Re: RFR: 8319873: Add windows implementation for jcmd System.map and System.dump_map [v7]

2024-08-27 Thread Simon Tooke
> This is a port of [JDK-8318636](https://github.com/openjdk/jdk/pull/16301) to > Windows. > > System.map and System.dump_map are implemented using the Windows API and > provide roughly the same information in the same format. Most of the heavy > lifting was implemented by @tstuefe in #16301 -

Re: RFR: 8319873: Add windows implementation for jcmd System.map and System.dump_map [v7]

2024-08-27 Thread Simon Tooke
On Thu, 22 Aug 2024 16:28:19 GMT, Simon Tooke wrote: >> src/hotspot/os/windows/memMapPrinter_windows.cpp line 162: >> >>> 160: outputStream* st = session.out(); >>> 161: os::print_os_info(st); >>> 162: os::print_memory_info(st); >> >> The "danger" here is that both of these function

Re: RFR: 8337317: serviceability/jvmti tests failed with FATAL ERROR in native method: Failed during the GetClassSignature call [v3]

2024-08-27 Thread Leonid Mesnik
On Tue, 27 Aug 2024 01:18:46 GMT, Alex Menkov wrote: >> The fix adds guards against JVMTI_ERROR_WRONG_PHASE error in event handlers >> >> Testing: hotspot/jtreg/serviceability/jvmti on all platforms > > Alex Menkov has updated the pull request incrementally with one additional > commit since th

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 27 Aug 2024 13:06:26 GMT, Thomas Stuefe wrote: >> IIUC for compact strings, with non-latin-1 each pair of bytes would require >> at most 3-bytes to encode so you'd need 2/3 of INT_MAX. With latin-1 it >> would be 1/2 INT_MAX. But yes I suppose in theory you might be able to get >> an o

Re: RFR: 8337563: NMT: rename MEMFLAGS to MemFlag

2024-08-27 Thread Gerard Ziemski
On Thu, 15 Aug 2024 06:31:14 GMT, David Holmes wrote: >>> I agree with @tstuefe here. MemFlag and MemType sound far too general when >>> this is NMT specific. >> >> Yes, it is not very specific, but it also not hard to learn and then know >> what this type is all about. >> >>> My preference t

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 27 Aug 2024 16:51:21 GMT, Dean Long wrote: >> Why? I think that would have a large flow on effect. And this length does >> fit in an int. > > The worse case is len == SIZE_MAX and therefore num_chars == SIZE_MAX, which > won't fit in an int. If we say this will never happen because cur

Re: RFR: 8311993: Test serviceability/sa/UniqueVtableTest.java failed: duplicate vtables detected [v2]

2024-08-27 Thread Alex Menkov
On Tue, 27 Aug 2024 05:18:27 GMT, Chris Plummer wrote: > I found this: > > ``` > // should we parse DLL symbol table in Java code or use > // Windbg's native lookup facility? By default, we use > // native lookup so that we can take advantage of '.pdb' > // files, if available. >

Re: RFR: 8339120: Use more fine-granular gcc unused warnings

2024-08-27 Thread Magnus Ihse Bursie
On Tue, 27 Aug 2024 20:04:21 GMT, Magnus Ihse Bursie wrote: > Currently, we issue -Wno-unused for all files in gcc, which is a rather big > sledgehammer to get rid of some warnings that proliferate in a few areas of > the build. > > We should instead leave -Wunused turned on (as done by -Wall)

RFR: 8339120: Use more fine-granular gcc unused warnings

2024-08-27 Thread Magnus Ihse Bursie
Currently, we issue -Wno-unused for all files in gcc, which is a rather big sledgehammer to get rid of some warnings that proliferate in a few areas of the build. We should instead leave -Wunused turned on (as done by -Wall) and use a much more fine-grained approach to disabling specific warnin

Re: RFR: 8319873: Add windows implementation for jcmd System.map and System.dump_map [v8]

2024-08-27 Thread Simon Tooke
> This is a port of [JDK-8318636](https://github.com/openjdk/jdk/pull/16301) to > Windows. > > System.map and System.dump_map are implemented using the Windows API and > provide roughly the same information in the same format. Most of the heavy > lifting was implemented by @tstuefe in #16301 -

Re: RFR: 8337317: serviceability/jvmti tests failed with FATAL ERROR in native method: Failed during the GetClassSignature call [v4]

2024-08-27 Thread Alex Menkov
> The fix adds guards against JVMTI_ERROR_WRONG_PHASE error in event handlers > > Testing: hotspot/jtreg/serviceability/jvmti on all platforms Alex Menkov has updated the pull request incrementally with one additional commit since the last revision: replace printf with LOG - Cha

Re: RFR: 8337317: serviceability/jvmti tests failed with FATAL ERROR in native method: Failed during the GetClassSignature call [v3]

2024-08-27 Thread Alex Menkov
On Tue, 27 Aug 2024 19:38:53 GMT, Leonid Mesnik wrote: >> Alex Menkov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> removed unneeded include > > test/hotspot/jtreg/serviceability/jvmti/HiddenClass/libHiddenClassSigTest.cpp > line 333:

Re: RFR: 8338768: Introduce runtime lookup to check for static builds [v2]

2024-08-27 Thread Jiangli Zhou
On Tue, 27 Aug 2024 13:55:51 GMT, Magnus Ihse Bursie wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Also update build to link properly > > And the discussion whether the checks are made "dynamically" or "sta

Re: RFR: 8337317: serviceability/jvmti tests failed with FATAL ERROR in native method: Failed during the GetClassSignature call [v3]

2024-08-27 Thread Leonid Mesnik
On Tue, 27 Aug 2024 23:02:57 GMT, Alex Menkov wrote: >> test/hotspot/jtreg/serviceability/jvmti/HiddenClass/libHiddenClassSigTest.cpp >> line 333: >> >>> 331: } >>> 332: >>> 333: err = jvmti->CreateRawMonitor("Event Monitor", &event_mon); >> >> Sorry, forgot to mention. >> There is a func

Re: RFR: 8337317: serviceability/jvmti tests failed with FATAL ERROR in native method: Failed during the GetClassSignature call [v4]

2024-08-27 Thread Leonid Mesnik
On Tue, 27 Aug 2024 23:12:45 GMT, Alex Menkov wrote: >> The fix adds guards against JVMTI_ERROR_WRONG_PHASE error in event handlers >> >> Testing: hotspot/jtreg/serviceability/jvmti on all platforms > > Alex Menkov has updated the pull request incrementally with one additional > commit since th

Re: RFR: 8337317: serviceability/jvmti tests failed with FATAL ERROR in native method: Failed during the GetClassSignature call [v4]

2024-08-27 Thread Serguei Spitsyn
On Tue, 27 Aug 2024 23:12:45 GMT, Alex Menkov wrote: >> The fix adds guards against JVMTI_ERROR_WRONG_PHASE error in event handlers >> >> Testing: hotspot/jtreg/serviceability/jvmti on all platforms > > Alex Menkov has updated the pull request incrementally with one additional > commit since th

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Tue, 27 Aug 2024 21:21:01 GMT, David Holmes wrote: > If you try to accommodate arbitrary future use then every method in the VM > would need to enforce every single precondition and invariant it expects > "just in case" and that is not practical. I'm basically arguing for Functional Testing

Re: RFR: 8337563: NMT: rename MEMFLAGS to MemFlag

2024-08-27 Thread David Holmes
On Wed, 7 Aug 2024 17:13:06 GMT, Gerard Ziemski wrote: > Please review this cleanup, where we rename `MEMFLAGS` to `MemType`. > > `MEMFLAGS` implies that we can use more than one at the same time, but those > are exclusive values, so `MemType` is much more suitable name. > > There is a bunch o

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Tue, 27 Aug 2024 23:54:08 GMT, Dean Long wrote: >> If you try to accommodate arbitrary future use then every method in the VM >> would need to enforce every single precondition and invariant it expects >> "just in case" and that is not practical. Code can and does take advantage >> of the e

Re: RFR: 8311993: Test serviceability/sa/UniqueVtableTest.java failed: duplicate vtables detected [v2]

2024-08-27 Thread Chris Plummer
On Mon, 26 Aug 2024 22:25:34 GMT, Alex Menkov wrote: >> On Windows SA agent gets a class vtable from symbols, exported from jvm.dll >> (it exports symbols like "??_7" + type + "@@6B@"). >> But symbol lookup function first requests WinDbg about the symbol. >> Sometimes WinDbg routine IDebugSymbol

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Tue, 20 Aug 2024 04:09:04 GMT, David Holmes wrote: >> This work has been split out from JDK-8328877: [JNI] The JNI Specification >> needs to address the limitations of integer UTF-8 String lengths >> >> The modified UTF-8 format used by the VM can require up to six bytes to >> represent one

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread Dean Long
On Wed, 28 Aug 2024 01:24:43 GMT, David Holmes wrote: >>> If you try to accommodate arbitrary future use then every method in the VM >>> would need to enforce every single precondition and invariant it expects >>> "just in case" and that is not practical. >> >> I'm basically arguing for Functi

Re: RFR: 8339120: Use more fine-granular gcc unused warnings

2024-08-27 Thread Julian Waters
On Tue, 27 Aug 2024 20:04:21 GMT, Magnus Ihse Bursie wrote: > Currently, we issue -Wno-unused for all files in gcc, which is a rather big > sledgehammer to get rid of some warnings that proliferate in a few areas of > the build. > > We should instead leave -Wunused turned on (as done by -Wall)

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v5]

2024-08-27 Thread David Holmes
On Wed, 28 Aug 2024 03:46:43 GMT, Dean Long wrote: >> Note that I do already document the assumptions here in the general comment >> in utf8.hpp: >> >> There is an additional assumption/expectation that our UTF8 API's are never >> dealing with >> invalid UTF8, and more generally that all UTF8

Re: RFR: 8338257: UTF8 lengths should be size_t not int [v6]

2024-08-27 Thread David Holmes
> This work has been split out from JDK-8328877: [JNI] The JNI Specification > needs to address the limitations of integer UTF-8 String lengths > > The modified UTF-8 format used by the VM can require up to six bytes to > represent one unicode character, but six byte characters are stored as UTF

Re: RFR: 8319873: Add windows implementation for jcmd System.map and System.dump_map [v8]

2024-08-27 Thread Thomas Stuefe
On Tue, 27 Aug 2024 22:58:52 GMT, Simon Tooke wrote: >> This is a port of [JDK-8318636](https://github.com/openjdk/jdk/pull/16301) >> to Windows. >> >> System.map and System.dump_map are implemented using the Windows API and >> provide roughly the same information in the same format. Most of

Re: RFR: 8337563: NMT: rename MEMFLAGS to MemFlag

2024-08-27 Thread Thomas Stuefe
On Wed, 7 Aug 2024 17:13:06 GMT, Gerard Ziemski wrote: > Please review this cleanup, where we rename `MEMFLAGS` to `MemType`. > > `MEMFLAGS` implies that we can use more than one at the same time, but those > are exclusive values, so `MemType` is much more suitable name. > > There is a bunch o

Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) [v6]

2024-08-27 Thread Sebastian Lövdahl
On Tue, 2 Jul 2024 11:07:56 GMT, Sebastian Lövdahl wrote: >> 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid >> (Kubernetes debug container) > > Sebastian Lövdahl has updated the pull request incrementally with one > additional commit since the last revision: > > Clarify

Re: RFR: 8339120: Use more fine-granular gcc unused warnings

2024-08-27 Thread Kim Barrett
On Tue, 27 Aug 2024 20:04:21 GMT, Magnus Ihse Bursie wrote: > Currently, we issue -Wno-unused for all files in gcc, which is a rather big > sledgehammer to get rid of some warnings that proliferate in a few areas of > the build. > > We should instead leave -Wunused turned on (as done by -Wall)