RFR: 8307855: update for deprecated sprintf for src/utils [v2]
Xue-Lei Andrew Fan
xuelei at openjdk.org
Mon May 15 06:31:59 UTC 2023
On Sun, 14 May 2023 00:44:45 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision:
>>
>> update typo in copyright statement
>
> src/utils/hsdis/binutils/hsdis-binutils.c line 251:
>
>> 249: if (type) snprintf(p += strlen(p), remaining_buflen(buf, bufsize, p), " type='%s'", type);
>> 250: if (dsize) snprintf(p += strlen(p), remaining_buflen(buf, bufsize, p), " dsize='%d'", dsize);
>> 251: if (delays) snprintf(p += strlen(p), remaining_buflen(buf, bufsize, p), " delay='%d'", delays);
>
> What is the value for p that is used in the call to remaining_buflen?
> It is being assumed to be the value after the assignment by the first
> argument. However, according to the standard, it is unspecified, and
> the whole snprintf call invokes UB. This is because there aren't any
> sequence points between the update of p in the first argument and that
> reference. (C++17 changes this, but we aren't using C++17 yet.)
For the 1st line (line 249), it is fine to use bufsize directly. The p in remaining_buflen is used to calculate the remaining length of the target buffer. To know the remaining buffer length, I need to know the pointer to the beginning buffer (the 'buf' parameter), the pointer to the beginning of the unused buffer (the 'p' parameter), and the total size of the buffer (the 'bufsize' parameter). The initial value of p is assigned to buf(p = buf), and then move forward by the used size ( p += strlen(p)).
It's a good point to me about the update sequence of p, and I will make an update in a new PR. Thank you very much!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13915#discussion_r1193376513
More information about the build-dev
mailing list