[Bug analyzer/95007] New: RFE: -fanalyzer should complain about writes to string literals

[Bug analyzer/95007] New: RFE: -fanalyzer should complain about writes to string literals

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95007

            Bug ID: 95007
           Summary: RFE: -fanalyzer should complain about writes to string
                    literals
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
  Target Milestone: ---

(Motivated by PR analyzer/95000, which has a false-positive path to a write to
a string literal)

Consider:

void test (void)
{
  char *s = "foo";
  s[0] = 'g';
}

-fanalyzer should complain about the write to s[0], but currently doesn't

See e.g.
https://wiki.sei.cmu.edu/confluence/display/c/STR30-C.+Do+not+attempt+to+modify+string+literals

[Bug analyzer/95007] RFE: -fanalyzer should complain about writes to string literals

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95007

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=95000,
                   |                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=61579
           Keywords|                            |diagnostic
                 CC|                            |egallager at gcc dot gnu.org

--- Comment #1 from Eric Gallager <egallager at gcc dot gnu.org> ---
You don't even need the analyzer for this; -Wwrite-strings already catches it:

$ /usr/local/bin/gcc -c -Wall -Wextra -Wwrite-strings 95007.c
95007.c: In function 'test':
95007.c:3:12: warning: initialization discards 'const' qualifier from pointer
target type [-Wdiscarded-qualifiers]
    3 |  char *s = "foo";
      |            ^~~~~
$

If you edit the source code to make 's' const to fix the -Wdiscarded-qualifiers
warning, you then get this error instead:

$ /usr/local/bin/gcc -c -Wall -Wextra -Wwrite-strings 95007.c
95007.c: In function 'test':
95007.c:4:7: error: assignment of read-only location '*s'
    4 |  s[0] = 'g';
      |       ^
$

Although, I guess it is true that there are some drawbacks to using
-Wwrite-strings, for example those described in bug 61579, so maybe having a
separate analyzer warning for this could still be useful...

[Bug analyzer/95007] RFE: -fanalyzer should complain about writes to string literals

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95007

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
I guess I'm thinking about the case where a string literal has been exposed as
a non-const "char *": the RFE here is for -fanalyzer to be able to detect if we
ever write to such a string (with interprocedural analysis etc etc).

[Bug analyzer/95007] RFE: -fanalyzer should complain about writes to string literals

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95007

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:3175d40fc52fb8eb3c3b18cc343d773da24434fb

commit r11-3829-g3175d40fc52fb8eb3c3b18cc343d773da24434fb
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Oct 7 18:34:09 2020 -0400

    analyzer: add warnings about writes to constant regions [PR95007]

    This patch adds two new warnings:
      -Wanalyzer-write-to-const
      -Wanalyzer-write-to-string-literal
    for code paths where the analyzer detects a write to a constant region.

    As noted in the documentation part of the patch, the analyzer doesn't
    prioritize detection of such writes, in that the state-merging logic
    will blithely lose the distinction between const and non-const regions.
    Hence false negatives are likely to arise due to state-merging.

    However, if the analyzer does happen to spot such a write, it seems worth
    reporting, hence this patch.

    gcc/analyzer/ChangeLog:
            * analyzer.opt (Wanalyzer-write-to-const): New.
            (Wanalyzer-write-to-string-literal): New.
            * region-model-impl-calls.cc (region_model::impl_call_memcpy):
            Call check_for_writable_region.
            (region_model::impl_call_memset): Likewise.
            (region_model::impl_call_strcpy): Likewise.
            * region-model.cc (class write_to_const_diagnostic): New.
            (class write_to_string_literal_diagnostic): New.
            (region_model::check_for_writable_region): New.
            (region_model::set_value): Call check_for_writable_region.
            * region-model.h (region_model::check_for_writable_region): New
            decl.

    gcc/ChangeLog:
            * doc/invoke.texi: Document -Wanalyzer-write-to-const and
            -Wanalyzer-write-to-string-literal.

    gcc/testsuite/ChangeLog:
            PR c/83347
            PR middle-end/90404
            PR analyzer/95007
            * gcc.dg/analyzer/write-to-const-1.c: New test.
            * gcc.dg/analyzer/write-to-string-literal-1.c: New test.

[Bug analyzer/95007] RFE: -fanalyzer should complain about writes to string literals

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95007

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Implemented by the above patch, with the caveat noted there.