This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix PR78154
On 11/16/2016 02:21 PM, Marc Glisse wrote:
On Wed, 16 Nov 2016, Martin Sebor wrote:
On 11/16/2016 11:49 AM, Prathamesh Kulkarni wrote:
Hi Richard,
Following your suggestion in PR78154, the patch checks if stmt
contains call to memmove (and friends) in gimple_stmt_nonzero_warnv_p
and returns true in that case.
Nice. I think the list should also include mempcpy, stpcpy, and
stpncpy, and probably also the corresponding checking built-ins
such as __builtin___memcpy_chk.
FWIW, a more general solution to consider (possibly for GCC 8)
might be to extend attribute nonnull to apply to a functions return
value as well (e.g., use zero as the index for that), to indicate
that a pointer returned from it is not null. That would let library
implementers annotate other functions (such as strerror)
We already have that, under the name returns_nonnull. IIRC, people found
a new name clearer than using position 0, when I posted the patch. Note
also that memcpy already has both an attribute that says that it returns
its first argument, and an attribute that says that said first argument
is nonnull.
Ah, right! Thanks for the reminder!
__builtin_memcpy and __builtin_strcpy are both declared with
the same attribute list ATTR_RET1_NOTHROW_NONNULL_LEAF (as are
their checking versions) and that's defined like so:
DEF_ATTR_TREE_LIST (ATTR_RET1_NOTHROW_NONNULL_LEAF, ATTR_FNSPEC,
ATTR_LIST_STR1, ATTR_NOTHROW_NONNULL_LEAF)
ATTR_NOTHROW_NONNULL_LEAF doesn't include returns_nonull and neither
does ATTR_FNSPEC ATTR_LIST_STR1 (unless it's meant to imply that).
In any event, lookup_attribute ("returns_nonnull") fails for both
of these functions so I think the fix might be as "simple" as adding
the attribute. Alternatively, if attribute fn spec str1 should imply
returns_nonnull when nonnull is set because it says (IIUC) that
the function returns the first (non-null) argument, the changes will
be a bit more involved and require adjusting other places besides
VRP (I see it used in fold-const.c as well similarly as in VRP).
(FWIW, I quoted "simple" above because it recently took me the better
part of an afternoon to figure out how to add attribute alloc_size to
malloc.)
(I've heard some noise in C++-land about making memcpy(0,0,0) valid, but
that may have just been noise)
We may have read the same discussion. It would make some things
a little easier in C++ (and remove what most people view as yet
another unnecessary gotcha in the language).
Martin