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