Re: [PATCH 2/6] New warnings -Wstring-plus-{char, int} (PR c++/62181)

[Martin Sebor <msebor@gmail.com>]

On 06/19/2017 11:28 AM, Xi Ruoyao wrote:
> On 2017-06-19 10:51 -0600, Martin Sebor wrote:
>> On 06/11/2017 07:32 PM, Xi Ruoyao wrote:
>>> This patch adds warning option -Wstring-plus-int for C/C++.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2017-06-12  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>
>>>
>>> 	* c-family/c.opt: New option -Wstring-plus-int.
>>> 	* c-family/c-common.c (pointer_int_sum): Checking for
>>> 	-Wstring-plus-int.
>>
>> This is a very useful warning but I would suggest to word it
>> in terms of what it actually does rather than what it might be
>> intended to do.  E.g., for
>>
>>    const char *p = "123" + 7;
>>
>> issue
>>
>>    warning: offset 7 exceeds the upper bound 3 of the array
>>
>> rather than
>>
>>    warning: adding 'int' to a string does not append to the string
>>
>> (I have trouble envisioning on what grounds someone might expect
>> the addition to have this effect.)
>
> How about something like `const char *p = "123" + getchar();` ?

I'm not sure I correctly understand the question (or whether
it's meant in response to my comment in parentheses) but let
me clarify what I meant.

In my view, the group of C++ (and certainly C) programmers who
might expect "123" + i to append the string representation of
the integer result to a string literal isn't significant enough
to focus the warning on.

Whether or not the addition is valid depends on the value of
the integer operand.  There are three sets of cases where the
addition is or may be invalid:

1) the integer operand is an out of bounds constant
2) the integer operand's non-constant value or the lower bound
    of its range is known to be out of bounds,
3) the lower bound of the operand's range is in bounds but
    the upper bound is out of bounds (as in the getchar example).

(1) can be handled with lexical analysis alone (as in you parch)
but it's prone to a high rate of false negatives.  (3) can also
be handled by lexical analysis alone but it's prone to a high
rate of false positives.  (2) has no false positives but some
false negatives.  It can only be detected with optimization.

With that in mind the warning would serve a greater purpose
by being aimed more broadly and describing the nature of the
error: forming an invalid pointer.  I believe it would best
be implemented analogously to or even integrated into
-Warray-bounds.  I.e., I suggest covering set (2) above.

>
> I'd like this for -Wstring-plus-int=1:
>
>     warning: adding 'int' to a string does not append to the string
>     [-Wstring-plus-int=]
>         const char *p = "123" + 7;
>                               ^
>     note: offset 7 exceeds the size 4 of the string, using the result
>     may lead to undefined behaviour.

The addition itself is undefined, regardless of whether or not
the result is used.

>
> (Clang permits "123" + 4 since its result is well defined in standard.
> Maybe we could permit "123" + 3 only.)

"123" is an array of 4 elements, with "123" + 4 pointing just past
the last (NUL) element.  It's valid to form a pointer past the last
element of an array and warning about it would likely be viewed as
a false positive (certainly if it were an out-of-bounds type of
warning).

Martin