RFR: JDK-8296995: ostream should handle snprintf(3) errors in release builds [v2]
Martin Doerr
mdoerr at openjdk.org
Wed Dec 7 12:01:34 UTC 2022
On Mon, 5 Dec 2022 08:07:25 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Small fix for a very unlikely problem.
>>
>> All streams in ostream.hpp end up using `os::snprintf()`, which uses `::vsnprintf()`. `vsnprintf(3)`can fail and return -1.
>>
>> The chance for this to happen is small. snprintf errors are usually encoding errors though not always (see third example at https://stackoverflow.com/questions/65334245/what-is-an-encoding-error-for-sprintf-that-should-return-1). I found "%ls" in one place in windows coding, so I am not sure we can always exclude the possibility of wide strings being used in our code base, or that of printing with outside-provided format strings.
>>
>> In case of an error, we assert in debug builds but don't handle it in release. There, this situation gets misdiagnosed later as a buffer overflow because we cast the signedness of the result away (see `outputStream::do_vsnprintf()`).
>>
>> ---
>>
>> The patch is trivial. The most exciting thing is the gtest, I guess.
>>
>> In release builds, we now treat this condition as an empty string write. I considered printing a clear marker into the stream instead, e.g. "ENCODING ERROR", but ultimately did not do it.
>
> Thomas Stuefe 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 one additional commit since the last revision:
>
> JDK-8296995-ostream-handle-sprintf-errors
test/hotspot/gtest/utilities/test_ostream.cpp line 130:
> 128: ::memset(buf, 'X', sizeof(buf));
> 129: int n = ::snprintf(buf + 1, sizeof(buf) - 2, "HALLO %ls", w);
> 130: if (n == -1) { // yes, we get an error. Retry using stringStream.
Can it happen that we don't get an error, here? If so, the ASSERT below still expects one, so, the test will fail. Correct?
-------------
PR: https://git.openjdk.org/jdk/pull/11160
More information about the hotspot-dev
mailing list