Hi Thomas,
On 29.01.20 18:44, Thomas Schwinge wrote:
>> + size_t len = sizeof hsa_context.driver_version_s;
>> + int printed = snprintf (hsa_context.driver_version_s, len,
>> + "HSA Runtime %hu.%hu", (unsigned short int)major,
>> + (unsigned short int)minor);
>> + if (printed >= len)
>> + GCN_WARNING ("HSA runtime version string was truncated."
>> + "Version %hu.%hu is too long.", (unsigned short int)major,
>> + (unsigned short int)minor);
>
> (Can it actually happen that 'snprintf' returns 'printed > len' --
> meaning that it's written into random memory? I thought 'snprintf' has a
> hard stop at 'len'? Or does this indicate the amount of memory it
> would've written? I should re-read the manpage at some point...) ;-)
>
Yes, "printed > len" can happen. Seems that I have chosen a bad variable name.
"actual_len" (of the formatted string that should have been written -
excluding the terminating '\0') would have been more appropriate.
> For 'printed = len' does or doesn't 'snprintf' store the terminating
> 'NUL' character, or do we manually have to set:
>
> hsa_context.driver_version_s[len - 1] = '\0';
>
> ... in that case?
No, in this case, the printed string is missing the last character, but the
terminating '\0' has been written. Consider:
#include <stdio.h>
int main () {
char s[] = "foo";
char buf[3];
// buf is too short to hold terminating '\0'
int actual_len = snprintf (buf, 3, "%s", s);
printf ("buf: %s\n", buf);
printf ("actual_len: %d\n", actual_len);
}
Output:
buf: fo
actual_len: 3
>
>> @@ -3410,15 +3432,19 @@ GOMP_OFFLOAD_init_device (int n)
>
>> - char buf[64];
>> status = hsa_fns.hsa_agent_get_info_fn (agent->id, HSA_AGENT_INFO_NAME,
>> - &buf);
>> + &agent->name);
>> if (status != HSA_STATUS_SUCCESS)
>> return hsa_error ("Error querying the name of the agent", status);
>
> (That's of course pre-existing, but) this looks like a dangerous API,
> given that 'hsa_agent_get_info_fn' doesn't know 'sizeof agent->name' (or
> 'sizeof buf' before)...
The API documentation
(cf.
https://rocm-documentation.readthedocs.io/en/latest/ROCm_API_References/ROCr-API.html)
states that "the type of this attribute is a NUL-terminated char[64]".
But, right, should this ever change, we might not notice it.
Best regards,
Frederik
