From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7197 invoked by alias); 30 Jan 2020 07:10:03 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 7189 invoked by uid 89); 30 Jan 2020 07:10:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.9 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.1 spammy=states, agent X-HELO: esa1.mentor.iphmx.com Received: from esa1.mentor.iphmx.com (HELO esa1.mentor.iphmx.com) (68.232.129.153) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 30 Jan 2020 07:09:59 +0000 IronPort-SDR: o1tvb9bRlDp6bXfosY2gJdlSh7kh2CPuUna38Khi5fpFpKjG1J+Hzgp19WbqlHp3T3GBaXMLD7 wsqh9B4OUDomcdVnRvSdfzDQWrupFeTvOkZRnw9UWzFCrJW1NKptj05PhhrIL9eE0zLt9+hlMY ZYHWPfrtaamRYyuXLRYuEM/1lu6EoQ/piwSBu9GPbTcODKvQUjgidSeFzj+WjZ0geEgH9mN15s wEao4O9ksKaLkoNkzGxZJev/lF2/0n+b3cVVw9X9v1dckmXWU0dPkXT5uUWkDiXEDdmsCVwnhm Akg= Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa1.mentor.iphmx.com with ESMTP; 29 Jan 2020 23:09:57 -0800 IronPort-SDR: QnhT62Z8S8Opt892Ka7amaKlar/dKY0tp8ArCE4DzlTiYdcTYPcwGqHR5jcVXy6L2WH69YsI3l 5bDrh3UF1aUA== Subject: Re: [PATCH] Add OpenACC acc_get_property support for AMD GCN To: Thomas Schwinge CC: Jakub Jelinek , Andrew Stubbs , References: <1c2e2d57-31ce-8e4d-c8b9-c2fbc7091e86@codesourcery.com> <0f59690f-5247-9b6e-169a-9a6ef16c41e1@codesourcery.com> <472b4749-7752-1527-c59c-1ab348854f32@codesourcery.com> <87h80ema2b.fsf@euler.schwinge.homeip.net> From: "Harwath, Frederik" X-Pep-Version: 2.0 Message-ID: Date: Thu, 30 Jan 2020 08:04:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <87h80ema2b.fsf@euler.schwinge.homeip.net> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Return-Path: frederik@codesourcery.com X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg01983.txt.bz2 Hi Thomas, On 29.01.20 18:44, Thomas Schwinge wrote: >> + size_t len =3D sizeof hsa_context.driver_version_s; >> + int printed =3D snprintf (hsa_context.driver_version_s, len, >> + "HSA Runtime %hu.%hu", (unsigned short int)major, >> + (unsigned short int)minor); >> + if (printed >=3D len) >> + GCN_WARNING ("HSA runtime version string was truncated." >> + "Version %hu.%hu is too long.", (unsigned short int)major, >> + (unsigned short int)minor); >=20 > (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...) ;-) >=20 Yes, "printed > len" can happen. Seems that I have chosen a bad variable na= me. "actual_len" (of the formatted string that should have been written - excluding the terminating '\0') would have been more appropriate. > For 'printed =3D len' does or doesn't 'snprintf' store the terminating > 'NUL' character, or do we manually have to set: >=20 > hsa_context.driver_version_s[len - 1] =3D '\0'; >=20 > ... in that case? No, in this case, the printed string is missing the last character, but the terminating '\0' has been written. Consider: #include int main () { char s[] =3D "foo"; char buf[3]; // buf is too short to hold terminating '\0' int actual_len =3D snprintf (buf, 3, "%s", s); printf ("buf: %s\n", buf); printf ("actual_len: %d\n", actual_len); } Output: buf: fo actual_len: 3 >=20 >> @@ -3410,15 +3432,19 @@ GOMP_OFFLOAD_init_device (int n) >=20 >> - char buf[64]; >> status =3D hsa_fns.hsa_agent_get_info_fn (agent->id, HSA_AGENT_INFO_N= AME, >> - &buf); >> + &agent->name); >> if (status !=3D HSA_STATUS_SUCCESS) >> return hsa_error ("Error querying the name of the agent", status); >=20 > (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_Reference= s/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