RFR: 8296812: sprintf is deprecated in Xcode 14 [v17]
Xue-Lei Andrew Fan
xuelei at openjdk.org
Wed Dec 7 21:25:13 UTC 2022
On Wed, 7 Dec 2022 08:46:45 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision:
>>
>> comment for snprintf_checked
>
> src/hotspot/share/adlc/output_c.cpp line 46:
>
>> 44: va_end(args);
>> 45: assert(result >= 0, "snprintf error");
>> 46: assert(static_cast<size_t>(result) < len, "snprintf truncated");
>
> Question, what is this assert? The standard CRT assert? Will it fire always, or just in debug?
The assert is defined in adlc.hpp, as follow:
`#define assert(cond, msg) { if (!(cond)) { fprintf(stderr, "assert fails %s %d: %s\n", __FILE__, __LINE__, msg); abort(); }}
`
I think it will fire always.
> src/hotspot/share/runtime/os.cpp line 102:
>
>> 100: }
>> 101:
>> 102: int os::snprintf_checked(char* buf, size_t len, const char* fmt, ...) {
>
> Can you please assert for buffer len > 0 and < some reasonable max, e.g. 1 gb? Protects us against neg overflows and other errors.
I may be hesitate for the checking. The behavior maybe unclear if a size_t value could be negative and if it is possible the max limit is too small in practice. If it is necessary, the svnprintf() could do that instead.
> src/hotspot/share/runtime/os.cpp line 108:
>
>> 106: va_end(args);
>> 107: assert(result >= 0, "os::snprintf error");
>> 108: assert(static_cast<size_t>(result) < len, "os::snprintf truncated");
>
> Can you please provide a printout for the truncated string? Proposal:
>
> ```assert(static_cast<size_t>(result) < len, "os::snprintf truncated ("%.200s...")", buf);```
I'm not sure if there is sensitive information in the buf. Dumping the string may result in sensitive information leaking.
> src/hotspot/share/utilities/utf8.cpp line 227:
>
>> 225: } else {
>> 226: if (p + 6 >= end) break; // string is truncated
>> 227: os::snprintf_checked(p, 7, "\\u%04x", c); // counting terminating zero in
>
> I had to think a while until I was sure this is correct :-)
> We are sure that c can never be > 0xFFFFFF (6 digits), right? But if that is false, code had been wrong before too.
I think so as it used to work.
-------------
PR: https://git.openjdk.org/jdk/pull/11115
More information about the hotspot-dev
mailing list