public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Sebor <msebor@gmail.com>
To: Bernd Edlinger <bernd.edlinger@hotmail.de>,
	Gcc Patch List <gcc-patches@gcc.gnu.org>
Subject: Re: PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) )
Date: Thu, 02 Aug 2018 20:34:00 -0000	[thread overview]
Message-ID: <39ae51d8-0807-7604-d1f5-78d31d6fece1@gmail.com> (raw)
In-Reply-To: <AM5PR0701MB2657FD7CA43337837EBA02C7E42C0@AM5PR0701MB2657.eurprd07.prod.outlook.com>

On 08/02/2018 12:56 PM, Bernd Edlinger wrote:
> On 08/02/18 15:26, Bernd Edlinger wrote:
>>>
>>>    /* If the length can be computed at compile-time, return it.  */
>>> -  len = c_strlen (src, 0);
>>> +  tree array;
>>> +  tree len = c_strlen (src, 0, &array);
>>
>> You know the c_strlen tries to compute wide character sizes,
>> but strlen does not do that, strlen (L"abc") should give 1
>> (or 0 on a BE machine)
>> I wonder if that is correct.
>>
> [snip]
>>>
>>>  static tree
>>> -fold_builtin_strlen (location_t loc, tree type, tree arg)
>>> +fold_builtin_strlen (location_t loc, tree fndecl, tree type, tree arg)
>>>  {
>>>    if (!validate_arg (arg, POINTER_TYPE))
>>>      return NULL_TREE;
>>>    else
>>>      {
>>> -      tree len = c_strlen (arg, 0);
>>> -
>>> +      tree arr = NULL_TREE;
>>> +      tree len = c_strlen (arg, 0, &arr);
>>
>> Is it possible to write a test case where strlen(L"test") reaches this point?
>> what will c_strlen return then?
>>
>
> Yes, of course it is:
>
> $ cat y.c
> int f(char *x)
> {
>    return __builtin_strlen(x);
> }
>
> int main ()
> {
>    return f((char*)&L"abcdef"[0]);
> }
> $ gcc -O3 -S y.c
> $ cat y.s
> main:
> .LFB1:
> 	.cfi_startproc
> 	movl	$6, %eax
> 	ret
> 	.cfi_endproc
>
> The reason is that c_strlen tries to fold wide chars at all.
> I do not know when that was introduced, was that already before your last patches?

The function itself was introduced in 1992 if not earlier,
before wide strings even existed.  AFAICS, it has always
accepted strings of all widths.  Until r241489 (in GCC 7)
it computed their length in bytes, not characters.  I don't
know if that was on purpose or if it was just never changed
to compute the length in characters when wide strings were
first introduced.  From the name I assume it's the latter.
The difference wasn't detected until sprintf tests were added
for wide string directives.  The ChangeLog description for
the change reads: Correctly handle wide strings.  I didn't
consider pathological cases like strlen (L"abc").  It
shouldn't be difficult to change to fix this case.

Martin

  reply	other threads:[~2018-08-02 20:34 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-19 20:09 [PATCH] warn for strlen of arrays with missing nul (PR 86552) Martin Sebor
2018-07-25 23:38 ` PING " Martin Sebor
2018-07-30 19:18   ` Martin Sebor
2018-08-02  2:44     ` PING [PATCH] warn for strlen of arrays with missing nul (PR 86552, 86711, 86714) ) Martin Sebor
2018-08-02 13:26       ` Bernd Edlinger
2018-08-02 18:56         ` Bernd Edlinger
2018-08-02 20:34           ` Martin Sebor [this message]
2018-08-03 13:01             ` Bernd Edlinger
2018-08-03 19:59               ` Martin Sebor
2018-08-15  5:31               ` Jeff Law
2018-08-29 17:17           ` Jeff Law
2018-08-24  6:36         ` Jeff Law
2018-08-24 12:28           ` Bernd Edlinger
2018-08-24 16:04             ` Jeff Law
2018-08-24 21:56               ` Bernd Edlinger
2018-08-24 16:51         ` Jeff Law
2018-08-24 17:26           ` Bernd Edlinger
2018-08-24 23:54             ` Jeff Law
2018-08-25  6:32               ` Bernd Edlinger
2018-08-25 17:33                 ` Jeff Law
2018-08-25 18:36                   ` Bernd Edlinger
2018-08-25 19:02                     ` Jeff Law
2018-08-25 19:32                       ` Bernd Edlinger
2018-08-25 20:42                         ` Martin Sebor
2018-08-26 10:20                           ` Bernd Edlinger
2018-08-25 23:22                         ` Jeff Law
2018-08-17  5:15       ` Jeff Law
2018-08-17 14:38         ` Martin Sebor
2018-08-13 21:23   ` [PATCH 0/6] improve handling of char arrays with missing nul (PR 86552, 86711, 86714) Martin Sebor
2018-08-13 21:25     ` [PATCH 1/6] prevent folding of unterminated const arrays in memchr calls (PR " Martin Sebor
2018-08-13 21:27     ` [PATCH 3/6] detect unterminated const arrays in strcpy calls (PR 86552) Martin Sebor
2018-08-30 22:31       ` Jeff Law
2018-08-13 21:28     ` [PATCH 4/6] detect unterminated const arrays in sprintf " Martin Sebor
2018-08-30 22:55       ` Jeff Law
2018-08-13 21:29     ` [PATCH 5/6] detect unterminated const arrays in stpcpy " Martin Sebor
2018-08-30 23:07       ` Jeff Law
2018-09-14 18:39       ` Jeff Law
2018-08-13 21:29     ` [PATCH 6/6] detect unterminated const arrays in strnlen " Martin Sebor
2018-08-30 23:25       ` Jeff Law
2018-10-01 21:49       ` Jeff Law
2018-08-14  3:21     ` [PATCH 2/6] detect unterminated const arrays in strlen " Martin Sebor
2018-08-30 22:15       ` Jeff Law
2018-08-31  2:25         ` Martin Sebor
2018-08-15  6:02     ` [PATCH 0/6] improve handling of char arrays with missing nul (PR 86552, 86711, 86714) Jeff Law
2018-08-15 14:47       ` Martin Sebor
2018-08-15 15:42         ` Jeff Law
2018-08-24 10:13           ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=39ae51d8-0807-7604-d1f5-78d31d6fece1@gmail.com \
    --to=msebor@gmail.com \
    --cc=bernd.edlinger@hotmail.de \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).