This is follow-up of PR78886.
Created attachment 40399 [details] Patch candidate Would be such change acceptable in GCC 7, or should be wait for GCC 8?
The p=malloc(0) transformation looks strange. (I never know if we are supposed to unlink_stmt_vdef, etc)
(In reply to Marc Glisse from comment #2) > The p=malloc(0) transformation looks strange. > (I never know if we are supposed to unlink_stmt_vdef, etc) Yep, it's strange, should be p = NULL. As mentioned in MAN page: If size is 0, then malloc() returns either NULL, or a unique pointer value that can later be successfully passed to free(). I'll prepare regular patch and send it to ML.
(In reply to Martin Liška from comment #3) > Yep, it's strange, should be p = NULL. As mentioned in MAN page: > If size is 0, then malloc() returns either NULL, or a unique pointer value > that can later be successfully passed to free(). While I would personally be happy with malloc(0) always returning 0, IIRC some platforms actually guarantee that malloc(0) returns a unique non-null pointer and may be unhappy about the compiler contradicting them. I may misremember though. (thanks for the PR and the patch, by the way)
Created attachment 40401 [details] Patch candidate v2 Well, I guess it's GCC 8 issue. I can imagine to introduce an option that would enable optimization of malloc(0) to return NULL? Similarly, we may want a warning for both missing LHS and malloc(0). Is it a desired feature?
Warning on malloc with an unused return value sounds like a good idea to me (in fact, it seems that all allocation functions to be declared with warn_unused_result; i.e., all those declared with attribute alloc_size). I also think warning on malloc(0) can be useful. GCC 7 has -Walloc-zero that warns on all zero-size allocations. Unfortunately, it's not in -Wall or -Wextra and has to be explicitly enabled. Unconditionally turning malloc(0) into NULL wouldn't be safe since the call is allowed to return a unique non-null pointer and there are implementations (e.g., Glibc) that do return one. But doing that under an option might be useful on targets where the system malloc returns null (though it could break with superimposition). I'm not sure that eliminating calls to malloc whose return value is unused is a safe optimization. Malloc can be superimposed and the replacement version might have important side-effects that the optimization would prevent.
(In reply to Martin Sebor from comment #6) > Warning on malloc with an unused return value sounds like a good idea to me > (in fact, it seems that all allocation functions to be declared with > warn_unused_result; i.e., all those declared with attribute alloc_size). Good, I can do that in next stage1. > > I also think warning on malloc(0) can be useful. GCC 7 has -Walloc-zero > that warns on all zero-size allocations. Unfortunately, it's not in -Wall > or -Wextra and has to be explicitly enabled. That's a pity, why was the option not enabled in either -Wall or -Wextra? > > Unconditionally turning malloc(0) into NULL wouldn't be safe since the call > is allowed to return a unique non-null pointer and there are implementations > (e.g., Glibc) that do return one. But doing that under an option might be > useful on targets where the system malloc returns null (though it could > break with superimposition). Or when an application does not rely on result being non-null. > > I'm not sure that eliminating calls to malloc whose return value is unused > is a safe optimization. Malloc can be superimposed and the replacement > version might have important side-effects that the optimization would > prevent. Both clang and gcc (in 037t.cddce1) can optimize out: int main() { __builtin_malloc(123); return 0; }
On December 22, 2016 5:36:56 PM GMT+01:00, "msebor at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78902 > >Martin Sebor <msebor at gcc dot gnu.org> changed: > > What |Removed |Added >---------------------------------------------------------------------------- > CC| |msebor at gcc dot gnu.org > >--- Comment #6 from Martin Sebor <msebor at gcc dot gnu.org> --- >Warning on malloc with an unused return value sounds like a good idea >to me (in >fact, it seems that all allocation functions to be declared with >warn_unused_result; i.e., all those declared with attribute >alloc_size). > >I also think warning on malloc(0) can be useful. GCC 7 has >-Walloc-zero that >warns on all zero-size allocations. Unfortunately, it's not in -Wall >or >-Wextra and has to be explicitly enabled. > >Unconditionally turning malloc(0) into NULL wouldn't be safe since the >call is >allowed to return a unique non-null pointer and there are >implementations >(e.g., Glibc) that do return one. But doing that under an option might >be >useful on targets where the system malloc returns null (though it could >break >with superimposition). > >I'm not sure that eliminating calls to malloc whose return value is >unused is a >safe optimization. Malloc can be superimposed and the replacement >version >might have important side-effects that the optimization would prevent. Given we remove malloc/free pairs it might be OK I think.
(In reply to Martin Liška from comment #7) > > I also think warning on malloc(0) can be useful. GCC 7 has -Walloc-zero > > that warns on all zero-size allocations. Unfortunately, it's not in -Wall > > or -Wextra and has to be explicitly enabled. > > That's a pity, why was the option not enabled in either -Wall or -Wextra? I think one of the reasons was that it is a late warning, so it has similar problems -Wnonnull had, warning even just in cases where path isolation decided to isolate some call where it just isn't called with those arguments. So perhaps moving it to post_ipa_warn pass would help with that part.
I'm renaming that and re-assigning that to Martin Sebor. Note that malloc w/o is optimized out, so only missing possibility was malloc (0), but it's problematic as some targets may return an uniq pointer.
GCC 9.1 has been released.
I'm working on addition of warn_unused_result attribute.
Author: marxin Date: Fri Jun 7 05:33:11 2019 New Revision: 272028 URL: https://gcc.gnu.org/viewcvs?rev=272028&root=gcc&view=rev Log: Add warn_unused_result for malloc-like functions (PR tree-optimization/78902). 2019-06-07 Martin Liska <mliska@suse.cz> PR tree-optimization/78902 * builtin-attrs.def (ATTR_WARN_UNUSED_RESULT): New. (ATTR_MALLOC_NOTHROW_LEAF_LIST): Remove. (ATTR_WARN_UNUSED_RESULT_NOTHROW_LEAF_LIST): New. (ATTR_MALLOC_WARN_UNUSED_RESULT_NOTHROW_LEAF_LIST): New. (ATTR_ALLOC_SIZE_2_NOTHROW_LIST): Remove. (ATTR_MALLOC_SIZE_1_NOTHROW_LEAF_LIST): Remove. (ATTR_MALLOC_WARN_UNUSED_RESULT_NOTHROW_LIST): New. (ATTR_ALLOC_WARN_UNUSED_RESULT_SIZE_2_NOTHROW_LIST): New. (ATTR_MALLOC_WARN_UNUSED_RESULT_SIZE_1_NOTHROW_LEAF_LIST): New. (ATTR_ALLOCA_SIZE_1_NOTHROW_LEAF_LIST): Remove. (ATTR_ALLOCA_WARN_UNUSED_RESULT_SIZE_1_NOTHROW_LEAF_LIST): New. (ATTR_MALLOC_SIZE_1_2_NOTHROW_LEAF_LIST): Remove. (ATTR_MALLOC_WARN_UNUSED_RESULT_SIZE_1_2_NOTHROW_LEAF_LIST): New. (ATTR_ALLOC_SIZE_2_NOTHROW_LEAF_LIST): Remove. (ATTR_ALLOC_WARN_UNUSED_RESULT_SIZE_2_NOTHROW_LEAF_LIST): New. (ATTR_MALLOC_NOTHROW_NONNULL): Remove. (ATTR_WARN_UNUSED_RESULT_NOTHROW_NONNULL): New. (ATTR_MALLOC_WARN_UNUSED_RESULT_NOTHROW_NONNULL): New. (ATTR_MALLOC_NOTHROW_NONNULL_LEAF): Remove. (ATTR_WARN_UNUSED_RESULT_NOTHROW_NONNULL_LEAF): New. (ATTR_MALLOC_WARN_UNUSED_RESULT_NOTHROW_NONNULL_LEAF): New. * builtins.def (BUILT_IN_ALIGNED_ALLOC): Change to use warn_unused_result attribute. (BUILT_IN_STRDUP): Likewise. (BUILT_IN_STRNDUP): Likewise. (BUILT_IN_ALLOCA): Likewise. (BUILT_IN_CALLOC): Likewise. (BUILT_IN_MALLOC): Likewise. (BUILT_IN_REALLOC): Likewise. 2019-06-07 Martin Liska <mliska@suse.cz> PR tree-optimization/78902 * c-c++-common/asan/alloca_loop_unpoisoning.c: Use result of __builtin_alloca. * c-c++-common/asan/pr88619.c: Likewise. * g++.dg/overload/using2.C: Likewise for malloc. * gcc.dg/attr-alloc_size-5.c: Add new dg-warning. * gcc.dg/nonnull-3.c: Use result of __builtin_strdup. * gcc.dg/pr43643.c: Likewise. * gcc.dg/pr59717.c: Likewise for calloc. * gcc.dg/torture/pr71816.c: Likewise. * gcc.dg/tree-ssa/pr78886.c: Likewise. * gcc.dg/tree-ssa/pr79697.c: Likewise. * gcc.dg/pr78902.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr78902.c Modified: trunk/gcc/ChangeLog trunk/gcc/builtin-attrs.def trunk/gcc/builtins.def trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/asan/alloca_loop_unpoisoning.c trunk/gcc/testsuite/c-c++-common/asan/pr88619.c trunk/gcc/testsuite/g++.dg/overload/using2.C trunk/gcc/testsuite/gcc.dg/attr-alloc_size-5.c trunk/gcc/testsuite/gcc.dg/nonnull-3.c trunk/gcc/testsuite/gcc.dg/pr43643.c trunk/gcc/testsuite/gcc.dg/pr59717.c trunk/gcc/testsuite/gcc.dg/torture/pr71816.c trunk/gcc/testsuite/gcc.dg/tree-ssa/pr78886.c trunk/gcc/testsuite/gcc.dg/tree-ssa/pr79697.c
Implemented.