RFR: 8329728: Read arbitrarily long lines in ClassListParser

[Matias Saavedra Silva]

On Mon, 8 Apr 2024 04:51:31 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> Today the `ClassListParser` has a hard-coded limit of 4096 chars for each line in the CDS class list file. However, it's possible for a line to be much longer than than (64KB for the class name, plus extra information that can include path names, IDs, etc).
> 
> I wrote a utility class `LineReader` that automatically allocates a buffer before calling `fgets()`. Hopefully this can be useful for other cases where we call `fgets()` with a fixed buffer size.

Overall this looks good! I had some considerations that we discussed offline which I posted below. 

As we talked about, a size limit of `INT_MAX` is probably too large and is difficult to test or could lead to crashes in the case of overflows due to the use of fgets and os::malloc. You mentioned 1MB as a reasonable max size but I think you may want a larger value since the max symbol size is 64k. Is 16 symbols per line enough?

src/hotspot/share/utilities/lineReader.cpp line 51:

> 49: }
> 50: 
> 51: char* LineReader::read_line() {

Do you think it's worth adding an assert here to make sure the `lineReader `has been initialized?

test/hotspot/jtreg/runtime/cds/appcds/customLoader/ClassListFormatA.java line 131:

> 129:         CDSTestUtils.createArchiveAndCheck(opts)
> 130:             .shouldContain("Preload Warning: Cannot find " + longName)
> 131:             .shouldContain("Preload Warning: Cannot find No/Such/ClassABCD");

Could you add a test that checks a line of max size to test overflows?

-------------

PR Review: https://git.openjdk.org/jdk/pull/18669#pullrequestreview-1987399960
PR Review Comment: https://git.openjdk.org/jdk/pull/18669#discussion_r1556429231
PR Review Comment: https://git.openjdk.org/jdk/pull/18669#discussion_r1556428800