* [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)
@ 2020-11-03 23:56 Martin Sebor
2020-11-17 0:54 ` Jeff Law
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Martin Sebor @ 2020-11-03 23:56 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]
Attached is a simple middle end implementation of detection of
mismatched pairs of calls to C++ new and delete, along with
a substantially enhanced implementation of -Wfree-nonheap-object.
The latter option has been in place since 2011 but detected only
the most trivial bugs.
Unlike the Clang -Wmismatched-new-delete which diagnoses
declarations of "overloaded operator new() and operator delete()
functions that do not have a corresponding free store function
defined within the same scope", this patch detects mismatches
between calls to allocation and deallocation functions, such as
calling free() on the result of new, of delete on the result of
array new. The functionality provided by Clang can be added on
top of what this feature does and since they are so close I think
it's fine to have both under the same option (a new level could
be introduced to distinguish the two).
The -Wfree-nonheap-object enhancement lets the warning detect all
calls to free, realloc, or C++ delete, with pointers that can be
proven not to point to the first byte of an allocated object.
The patch relies on the well-tested compute_objsize() function
for the determination of pointer provenance and makes use of
the changes in the following patch submitted for review just
yesterday:
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557807.html
As usual, I tested on x86_64-linux with Glibc & Binutils/GDB with
no new false positives.
Martin
PS A few words on the implementation choices:
The new code is in builtins.c only because -Wfree-nonheap-object
is there. I still plan to move all of the invalid access checking
code into its own module or pass at some point but I didn't want
to make this improvement contingent on that restructuring.
Even though it's all in builtins.c, the code is called from calls.c.
This is so that simple mismatches can be diagnosed even when free
isn't handed in builtins.c (i.e., without optimization).
The warning makes no attempt to analyze the CFG or handle
conditional mismatches. That will have to wait until the code
is moved to a GIMPLE pass.
[-- Attachment #2: gcc-90629.diff --]
[-- Type: text/x-patch, Size: 50995 bytes --]
PR c++/90629 - Support for -Wmismatched-new-delete
gcc/ChangeLog:
PR c++/90629
* builtins.c (access_ref::access_ref): Initialize new member.
(compute_objsize): Use access_ref::deref. Handle simple pointer
assignment.
(expand_builtin): Remove handling of the free built-in.
(find_assignment_location): New function.
(gimple_call_alloc_p): Same.
(gimple_call_dealloc_argno): Same.
(gimple_call_dealloc_p): Same.
(matching_alloc_calls_p): Same.
(warn_dealloc_offset): Same.
* builtins.h (struct access_ref): Declare new member.
(maybe_emit_free_warning): Make extern. Make use of access_ref.
Handle -Wmismatched-new-delete.
* calls.c (initialize_argument_information): Call
maybe_emit_free_warning.
* doc/invoke.texi (-Wfree-nonheap-object): Expand documentation.
(-Wmismatched-new-delete): Document new option.
gcc/c-family/ChangeLog:
PR c++/90629
* c.opt (-Wmismatched-new-delete): New option.
gcc/testsuite/ChangeLog:
PR c++/90629
* g++.dg/warn/delete-array-1.C: Add expected warning.
* g++.old-deja/g++.other/delete2.C: Add expected warning.
* g++.dg/warn/Wfree-nonheap-object-2.C: New test.
* g++.dg/warn/Wfree-nonheap-object.C: New test.
* g++.dg/warn/Wmismatched-new-delete.C: New test.
* gcc.dg/Wfree-nonheap-object-2.c: New test.
* gcc.dg/Wfree-nonheap-object-3.c: New test.
* gcc.dg/Wfree-nonheap-object.c: New test.
diff --git a/gcc/builtins.c b/gcc/builtins.c
index 5d60eab6ba2..1b8a5b82dac 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see
#include "gomp-constants.h"
#include "omp-general.h"
#include "tree-dfa.h"
+#include "gimple-iterator.h"
#include "gimple-ssa.h"
#include "tree-ssa-live.h"
#include "tree-outof-ssa.h"
@@ -182,7 +183,6 @@ static rtx expand_builtin_memory_chk (tree, rtx, machine_mode,
enum built_in_function);
static void maybe_emit_chk_warning (tree, enum built_in_function);
static void maybe_emit_sprintf_chk_warning (tree, enum built_in_function);
-static void maybe_emit_free_warning (tree);
static tree fold_builtin_object_size (tree, tree);
static bool check_read_access (tree, tree, tree = NULL_TREE, int = 1);
static bool compute_objsize (tree, int, access_ref *, bitmap *, range_query *);
@@ -200,8 +200,8 @@ static void expand_builtin_sync_synchronize (void);
access_ref::access_ref (tree bound /* = NULL_TREE */,
bool minaccess /* = false */)
-: ref (), eval ([](tree x){ return x; }), trail1special (true), base0 (true),
- parmarray ()
+: ref (), eval ([](tree x){ return x; }), deref (), trail1special (true),
+ base0 (true), parmarray ()
{
/* Set to valid. */
offrng[0] = offrng[1] = 0;
@@ -5053,7 +5053,10 @@ compute_objsize (tree ptr, int ostype, access_ref *pref, bitmap *visited,
const bool addr = TREE_CODE (ptr) == ADDR_EXPR;
if (addr)
- ptr = TREE_OPERAND (ptr, 0);
+ {
+ --pref->deref;
+ ptr = TREE_OPERAND (ptr, 0);
+ }
if (DECL_P (ptr))
{
@@ -5154,9 +5157,11 @@ compute_objsize (tree ptr, int ostype, access_ref *pref, bitmap *visited,
if (code == ARRAY_REF || code == MEM_REF)
{
+ ++pref->deref;
+
tree ref = TREE_OPERAND (ptr, 0);
tree reftype = TREE_TYPE (ref);
- if (code == ARRAY_REF
+ if (!addr && code == ARRAY_REF
&& TREE_CODE (TREE_TYPE (reftype)) == POINTER_TYPE)
/* Avoid arrays of pointers. FIXME: Hande pointers to arrays
of known bound. */
@@ -5267,6 +5272,7 @@ compute_objsize (tree ptr, int ostype, access_ref *pref, bitmap *visited,
if (code == STRING_CST)
{
pref->sizrng[0] = pref->sizrng[1] = TREE_STRING_LENGTH (ptr);
+ pref->ref = ptr;
return true;
}
@@ -5276,6 +5282,10 @@ compute_objsize (tree ptr, int ostype, access_ref *pref, bitmap *visited,
if (!compute_objsize (ref, ostype, pref, visited, rvals))
return false;
+ /* Clear DEREF since the offset is being applied to the target
+ of the dereference. */
+ pref->deref = 0;
+
offset_int orng[2];
tree off = pref->eval (TREE_OPERAND (ptr, 1));
if (get_offset_range (off, NULL, orng, rvals))
@@ -5413,9 +5423,9 @@ compute_objsize (tree ptr, int ostype, access_ref *pref, bitmap *visited,
return true;
}
- if (code == ADDR_EXPR)
+ if (code == ADDR_EXPR || code == SSA_NAME)
return compute_objsize (ptr, ostype, pref, visited, rvals);
-
+
/* This could be an assignment from a nonlocal pointer. Save PTR
to mention in diagnostics but otherwise treat it as a pointer
to an unknown object. */
@@ -10281,11 +10291,6 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
maybe_emit_sprintf_chk_warning (exp, fcode);
break;
- case BUILT_IN_FREE:
- if (warn_free_nonheap_object)
- maybe_emit_free_warning (exp);
- break;
-
case BUILT_IN_THREAD_POINTER:
return expand_builtin_thread_pointer (exp, target);
@@ -12589,30 +12594,336 @@ maybe_emit_sprintf_chk_warning (tree exp, enum built_in_function fcode)
access_write_only);
}
-/* Emit warning if a free is called with address of a variable. */
+/* Return the location of the assignment STMT if it has one, or another
+ assignment on the chain that has one. Used to improve the location
+ of informational notes. */
-static void
+static location_t
+find_assignment_location (tree var)
+{
+ gimple *stmt = SSA_NAME_DEF_STMT (var);
+
+ for (gimple *asgn = stmt; ; )
+ {
+ if (gimple_has_location (asgn))
+ return gimple_location (asgn);
+
+ if (!is_gimple_assign (asgn))
+ break;
+
+ tree rhs = gimple_assign_rhs1 (asgn);
+ if (TREE_CODE (rhs) != SSA_NAME)
+ break;
+
+ asgn = SSA_NAME_DEF_STMT (rhs);
+ }
+
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ gsi_next_nondebug (&gsi);
+ if (gimple *next_stmt = gsi_stmt (gsi))
+ {
+ if (gimple_has_location (next_stmt))
+ return gimple_location (next_stmt);
+ }
+ else
+ {
+ gsi = gsi_for_stmt (stmt);
+ gsi_prev_nondebug (&gsi);
+ if (gimple *prev_stmt = gsi_stmt (gsi))
+ if (gimple_has_location (prev_stmt))
+ return gimple_location (prev_stmt);
+ }
+
+ /* As a last resort fallback, return the location of the closing
+ curly brace. */
+ return cfun->function_end_locus;
+}
+
+/* Return true if STMT is a call to an allocation function. Unless
+ ALL_ALLOC is set, consider only functions that return dynmamically
+ allocated objects. Otherwise return true even for all forms of
+ alloca (including VLA). */
+
+static bool
+gimple_call_alloc_p (gimple *stmt, bool all_alloc = false)
+{
+ tree fndecl = gimple_call_fndecl (stmt);
+ if (!fndecl)
+ return false;
+
+ /* A call to operator new isn't recognized as one to a built-in. */
+ if (DECL_IS_OPERATOR_NEW_P (fndecl))
+ return true;
+
+ /* TODO: Handle user-defined functions with attribute malloc. */
+ if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
+ return false;
+
+ switch (DECL_FUNCTION_CODE (fndecl))
+ {
+ case BUILT_IN_ALLOCA:
+ case BUILT_IN_ALLOCA_WITH_ALIGN:
+ return all_alloc;
+ case BUILT_IN_CALLOC:
+ case BUILT_IN_MALLOC:
+ case BUILT_IN_REALLOC:
+ case BUILT_IN_STRDUP:
+ case BUILT_IN_STRNDUP:
+ return true;
+ default:
+ return false;
+ }
+}
+
+/* Return the zero-based number corresponding to the argument being
+ deallocated if STMT is a call to a deallocation function or UINT_MAX
+ if it isn't. */
+
+static unsigned
+gimple_call_dealloc_argno (gimple *stmt)
+{
+ tree fndecl = gimple_call_fndecl (stmt);
+ if (!fndecl)
+ return UINT_MAX;
+
+ /* A call to operator delete isn't recognized as one to a built-in. */
+ if (DECL_IS_OPERATOR_DELETE_P (fndecl))
+ return 0;
+
+ /* TODO: Handle user-defined functions with attribute malloc? Handle
+ known non-built-ins like fopen? */
+ if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
+ return UINT_MAX;
+
+ switch (DECL_FUNCTION_CODE (fndecl))
+ {
+ case BUILT_IN_FREE:
+ case BUILT_IN_REALLOC:
+ return 0;
+ default:
+ return UINT_MAX;
+ }
+}
+
+/* Return true if STMT is a call to a deallocation function. */
+
+static inline bool
+gimple_call_dealloc_p (gimple *stmt)
+{
+ return gimple_call_dealloc_argno (stmt) != UINT_MAX;
+}
+
+/* Return true if DEALLOC_DECL is a function suitable to deallocate
+ objects allocated by calls to ALLOC_DECL. */
+
+static bool
+matching_alloc_calls_p (tree alloc_decl, tree dealloc_decl)
+{
+ if (DECL_IS_OPERATOR_NEW_P (alloc_decl))
+ {
+ if (!DECL_IS_OPERATOR_DELETE_P (dealloc_decl))
+ return false;
+
+ /* Return true iff both functions are of the same array or
+ singleton form. */
+ tree alloc_id = DECL_NAME (alloc_decl);
+ tree dealloc_id = DECL_NAME (dealloc_decl);
+ const char *alloc_fname = IDENTIFIER_POINTER (alloc_id);
+ const char *dealloc_fname = IDENTIFIER_POINTER (dealloc_id);
+ return !strchr (alloc_fname, '[') == !strchr (dealloc_fname, '[');
+ }
+
+ return !DECL_IS_OPERATOR_DELETE_P (dealloc_decl);
+}
+
+/* Return true if DEALLOC_DECL is a function suitable to deallocate
+ objectes allocated by the ALLOC call. */
+
+static bool
+matching_alloc_calls_p (gimple *alloc, tree dealloc_decl)
+{
+ tree alloc_decl = gimple_call_fndecl (alloc);
+ if (!alloc_decl)
+ return true;
+
+ return matching_alloc_calls_p (alloc_decl, dealloc_decl);
+}
+
+/* Diagnose a call to FNDECL to deallocate a pointer referenced by
+ AREF that includes a nonzero offset. Such a pointer cannot refer
+ to the beginning of an allocated object. A negative offset may
+ refer to it only if the target pointer is unknown. */
+
+static bool
+warn_dealloc_offset (location_t loc, tree exp, tree fndecl,
+ const access_ref &aref)
+{
+ char offstr[80];
+ offstr[0] = '\0';
+ if (wi::fits_shwi_p (aref.offrng[0]))
+ {
+ if (aref.offrng[0] == aref.offrng[1]
+ || !wi::fits_shwi_p (aref.offrng[1]))
+ sprintf (offstr, " %lli",
+ (long long)aref.offrng[0].to_shwi ());
+ else
+ sprintf (offstr, " [%lli, %lli]",
+ (long long)aref.offrng[0].to_shwi (),
+ (long long)aref.offrng[1].to_shwi ());
+ }
+
+ if (!warning_at (loc, OPT_Wfree_nonheap_object,
+ "%K%qD called on pointer %qE with nonzero offset%s",
+ exp, fndecl, aref.ref, offstr))
+ return false;
+
+ if (DECL_P (aref.ref))
+ inform (DECL_SOURCE_LOCATION (aref.ref), "declared here");
+ else if (TREE_CODE (aref.ref) == SSA_NAME)
+ {
+ gimple *def_stmt = SSA_NAME_DEF_STMT (aref.ref);
+ if (is_gimple_call (def_stmt))
+ {
+ tree alloc_decl = gimple_call_fndecl (def_stmt);
+ inform (gimple_location (def_stmt),
+ "returned from a call to %qD", alloc_decl);
+ }
+ }
+
+ return true;
+}
+
+/* Issue a warning if a deallocation function such as free, realloc,
+ or C++ operator delete is called with an argument not returned by
+ a matching allocation function such as malloc or the corresponding
+ form of C++ operatorn new. */
+
+void
maybe_emit_free_warning (tree exp)
{
- if (call_expr_nargs (exp) != 1)
+ tree fndecl = get_callee_fndecl (exp);
+ if (!fndecl)
return;
- tree arg = CALL_EXPR_ARG (exp, 0);
+ if (fndecl_built_in_p (fndecl, BUILT_IN_NORMAL))
+ {
+ built_in_function code = DECL_FUNCTION_CODE (fndecl);
+ if (code != BUILT_IN_FREE && code != BUILT_IN_REALLOC)
+ return;
+ }
+ else if (!DECL_IS_OPERATOR_DELETE_P (fndecl))
+ return;
- STRIP_NOPS (arg);
- if (TREE_CODE (arg) != ADDR_EXPR)
+ if (call_expr_nargs (exp) < 1)
return;
- arg = get_base_address (TREE_OPERAND (arg, 0));
- if (arg == NULL || INDIRECT_REF_P (arg) || TREE_CODE (arg) == MEM_REF)
+ tree ptr = CALL_EXPR_ARG (exp, 0);
+ if (integer_zerop (ptr))
return;
- if (SSA_VAR_P (arg))
- warning_at (tree_nonartificial_location (exp), OPT_Wfree_nonheap_object,
- "%Kattempt to free a non-heap object %qD", exp, arg);
- else
- warning_at (tree_nonartificial_location (exp), OPT_Wfree_nonheap_object,
- "%Kattempt to free a non-heap object", exp);
+ access_ref aref;
+ if (!compute_objsize (ptr, 0, &aref))
+ return;
+
+ tree ref = aref.ref;
+
+ if (integer_zerop (ref))
+ return;
+
+ tree dealloc_decl = get_callee_fndecl (exp);
+ location_t loc = tree_nonartificial_location (exp);
+ loc = expansion_point_location_if_in_system_header (loc);
+
+ if (DECL_P (ref) || EXPR_P (ref))
+ {
+ /* Diagnose freeing a declared object. */
+ if (aref.ref_declared ()
+ && warning_at (loc, OPT_Wfree_nonheap_object,
+ "%K%qD called on unallocated object %qD",
+ exp, dealloc_decl, ref))
+ {
+ inform (DECL_SOURCE_LOCATION (ref),
+ "declared here");
+ return;
+ }
+
+ /* Diagnose freeing a pointer that includes a positive offset.
+ Such a pointer cannot refer to the beginning of an allocated
+ object. A negative offset may refer to it. */
+ if (!aref.deref
+ && aref.sizrng[0] != aref.sizrng[1]
+ && aref.offrng[0] > 0 && aref.offrng[1] > 0
+ && warn_dealloc_offset (loc, exp, dealloc_decl, aref))
+ return;
+ }
+ else if (CONSTANT_CLASS_P (ref))
+ {
+ if (warning_at (loc, OPT_Wfree_nonheap_object,
+ "%K%qD called on a pointer to an unallocated "
+ "object %qE", exp, dealloc_decl, ref))
+ {
+ if (TREE_CODE (ptr) == SSA_NAME)
+ {
+ gimple *def_stmt = SSA_NAME_DEF_STMT (ptr);
+ if (is_gimple_assign (def_stmt))
+ {
+ location_t loc = find_assignment_location (ptr);
+ inform (loc, "assigned here");
+ }
+ }
+ return;
+ }
+ }
+ else if (TREE_CODE (ref) == SSA_NAME)
+ {
+ /* Also warn if the pointer argument refers to the result
+ of an allocation call like alloca or VLA. */
+ gimple *def_stmt = SSA_NAME_DEF_STMT (ref);
+ if (is_gimple_call (def_stmt))
+ {
+ bool warned = false;
+ if (gimple_call_alloc_p (def_stmt))
+ {
+ if (matching_alloc_calls_p (def_stmt, dealloc_decl))
+ {
+ if (!aref.deref
+ && aref.offrng[0] > 0 && aref.offrng[1] > 0
+ && warn_dealloc_offset (loc, exp, dealloc_decl, aref))
+ return;
+ }
+ else
+ warned = warning_at (loc, OPT_Wmismatched_new_delete,
+ "%K%qD called on pointer returned "
+ "from a mismatched allocation "
+ "function", exp, dealloc_decl);
+ }
+ else if (gimple_call_builtin_p (def_stmt, BUILT_IN_ALLOCA)
+ || gimple_call_builtin_p (def_stmt,
+ BUILT_IN_ALLOCA_WITH_ALIGN))
+ warned = warning_at (loc, OPT_Wfree_nonheap_object,
+ "%K%qD called on pointer to "
+ "an unallocated object",
+ exp, dealloc_decl);
+ if (warned)
+ {
+ tree fndecl = gimple_call_fndecl (def_stmt);
+ inform (gimple_location (def_stmt),
+ "returned from a call to %qD", fndecl);
+ return;
+ }
+ }
+ else if (gimple_nop_p (def_stmt))
+ {
+ ref = SSA_NAME_VAR (ref);
+ /* Diagnose freeing a pointer that includes a positive offset. */
+ if (TREE_CODE (ref) == PARM_DECL
+ && !aref.deref
+ && aref.sizrng[0] != aref.sizrng[1]
+ && aref.offrng[0] > 0 && aref.offrng[1] > 0
+ && warn_dealloc_offset (loc, exp, dealloc_decl, aref))
+ return;
+ }
+ }
}
/* Fold a call to __builtin_object_size with arguments PTR and OST,
diff --git a/gcc/builtins.h b/gcc/builtins.h
index 4a2e34351be..11f2c1e3eb5 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -185,6 +185,12 @@ struct access_ref
argument to the minimum. */
offset_int size_remaining (offset_int * = NULL) const;
+ /* Return true if *THIS is an access to a declared object. */
+ bool ref_declared () const
+ {
+ return DECL_P (ref) && base0 && deref < 1;
+ }
+
/* Set the size range to the maximum. */
void set_max_size_range ()
{
@@ -226,6 +232,9 @@ struct access_ref
/* Used to fold integer expressions when called from front ends. */
tree (*eval)(tree);
+ /* Positive when REF is dereferenced, negative when its address is
+ taken. */
+ int deref;
/* Set if trailing one-element arrays should be treated as flexible
array members. */
bool trail1special;
@@ -266,5 +275,6 @@ extern tree compute_objsize (tree, int, tree * = NULL, tree * = NULL,
range_query * = NULL);
extern bool check_access (tree, tree, tree, tree, tree,
access_mode, const access_data * = NULL);
+extern void maybe_emit_free_warning (tree);
#endif /* GCC_BUILTINS_H */
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 10e53ea67c9..d82899a5691 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -777,6 +777,11 @@ Wmisleading-indentation
C C++ Common Var(warn_misleading_indentation) Warning LangEnabledBy(C C++,Wall)
Warn when the indentation of the code does not reflect the block structure.
+Wmismatched-new-delete
+C++ ObjC++ Var(warn_mismatched_new_delete) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn for deallocation calls with arguments returned from calls to mismatched
+allocation functions.
+
Wmismatched-tags
C++ ObjC++ Var(warn_mismatched_tags) Warning
Warn when a class is redeclared or referenced using a mismatched class-key.
diff --git a/gcc/calls.c b/gcc/calls.c
index a8f459632f2..d61eb815636 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -2623,6 +2623,10 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
/* Check attribute access arguments. */
maybe_warn_rdwr_sizes (&rdwr_idx, fndecl, fntype, exp);
+
+ /* Check calls to operator new for mismatched forms and attempts
+ to deallocate unallocated objects. */
+ maybe_emit_free_warning (exp);
}
/* Update ARGS_SIZE to contain the total size for the argument block.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 89168be1d2f..b50ac0a659b 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -242,7 +242,8 @@ in the following sections.
-Wno-deprecated-enum-enum-conversion -Wno-deprecated-enum-float-conversion @gol
-Weffc++ -Wextra-semi -Wno-inaccessible-base @gol
-Wno-inherited-variadic-ctor -Wno-init-list-lifetime @gol
--Wno-invalid-offsetof -Wno-literal-suffix -Wmismatched-tags @gol
+-Wno-invalid-offsetof -Wno-literal-suffix @gol
+-Wno-mismatched-new-delete -Wmismatched-tags @gol
-Wmultiple-inheritance -Wnamespaces -Wnarrowing @gol
-Wnoexcept -Wnoexcept-type -Wnon-virtual-dtor @gol
-Wpessimizing-move -Wno-placement-new -Wplacement-new=@var{n} @gol
@@ -3818,6 +3819,30 @@ The warning is inactive inside a system header file, such as the STL, so
one can still use the STL. One may also instantiate or specialize
templates.
+@item -Wno-mismatched-new-delete @r{(C++ and Objective-C++ only)}
+@opindex Wmismatched-new-delete
+@opindex Wno-mismatched-new-delete
+Warn for invocations of C++ @code{operator delete} with pointer arguments
+returned from either mismatched forms of @code{operator new}, or from other
+functions that allocate objects for which the @code{operator delete} isn't
+a suitable deallocator. For example, the @code{delete} expression in
+the function below is diagnosed because it doesn't match the array form
+of the @code{new} expression the pointer argument was returned from.
+Similarly, the call to @code{free} is also diagnosed.
+
+@smallexample
+void f ()
+@{
+ int *a = new int[n];
+ delete a; // warning: mismatch in array forms of expressions
+
+ char *p = new char[n];
+ free (p); // warning: mismatch between new and free
+@}
+@end smallexample
+
+@option{-Wmismatched-new-delete} is enabled by default.
+
@item -Wmismatched-tags @r{(C++ and Objective-C++ only)}
@opindex Wmismatched-tags
@opindex Wno-mismatched-tags
@@ -7699,8 +7724,23 @@ to @option{-Wframe-larger-than=}@samp{SIZE_MAX} or larger.
@item -Wno-free-nonheap-object
@opindex Wno-free-nonheap-object
@opindex Wfree-nonheap-object
-Do not warn when attempting to free an object that was not allocated
-on the heap.
+Warn when attempting to deallocate an object that was either not allocated
+on the heap, or by using a pointer that was not returned from a prior call
+to the corresponding allocation function. For example, because the call
+to @code{stpcpy} returns a pointer to the terminating nul character and
+not to the begginning of the object, the call to @code{free} below is
+diagnosed.
+
+@smallexample
+void f (char *p)
+@{
+ p = stpcpy (p, "abc");
+ // ...
+ free (p); // warning
+@}
+@end smallexample
+
+@option{-Wfree-nonheap-object} is enabled by default.
@item -Wstack-usage=@var{byte-size}
@opindex Wstack-usage
diff --git a/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object-2.C b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object-2.C
new file mode 100644
index 00000000000..9d4d2a393ea
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object-2.C
@@ -0,0 +1,274 @@
+/* PR ????? - No warning on attempts to access free object
+ Verify that freeing unallocated objects referenced either directly
+ or through pointers is diagnosed.
+ { dg-do compile }
+ { dg-options "-O2 -Wall -Wfree-nonheap-object" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+typedef __SIZE_TYPE__ size_t;
+
+extern "C"
+{
+ void free (void*);
+ extern void* malloc (size_t);
+ extern void* realloc (void *p, size_t);
+}
+
+void sink (void*, ...);
+#define sink(...) sink (0, __VA_ARGS__)
+
+extern char ecarr[];
+extern void* eparr[];
+
+extern char *eptr;
+
+void* source (void);
+
+void nowarn_free (void *p, void **pp, size_t n, intptr_t iptr)
+{
+ free (p);
+
+ p = 0;
+ free (p);
+
+ p = malloc (n);
+ sink (p);
+ free (p);
+
+ p = malloc (n);
+ sink (p);
+
+ p = realloc (p, n * 2);
+ sink (p);
+ free (p);
+
+ free ((void*)iptr);
+
+ p = source ();
+ free (p);
+
+ p = source ();
+ p = (char*)p - 1;
+ free (p);
+
+ free (*pp);
+}
+
+void warn_free_extern_arr (void)
+{
+ free (ecarr); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_extern_arr_offset (int i)
+{
+ char *p = ecarr + i;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_cstint (void)
+{
+ void *p = (void*)1;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_func (void)
+{
+ void *p = (void*)warn_free_func;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_string (int i)
+{
+ {
+ char *p = (char*)"123";
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ char *p = (char*)"234" + 1;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ char *p = (char*)"345" + i;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+
+ if (i >= 0)
+ {
+ char *p = (char*)"456" + i;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+}
+
+void warn_free_local_arr (int i)
+{
+ {
+ char a[4];
+ sink (a);
+ free (a); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ char b[5];
+ sink (b);
+
+ char *p = b + 1;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ char c[6];
+ sink (c);
+
+ char *p = c + i;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+}
+
+
+void warn_free_vla (int n, int i)
+{
+ {
+ int vla[n], *p = vla;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+
+ {
+ int vla[n + 1], *p = vla + 1;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ int vla[n + 2], *p = vla + i;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+}
+
+
+void nowarn_free_extern_ptrarr (void)
+{
+ free (*eparr);
+}
+
+void nowarn_free_extern_ptrarr_offset (int i)
+{
+ void *p = eparr[i];
+ free (p);
+}
+
+
+void warn_free_extern_ptrarr (void)
+{
+ free (eparr); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_extern_ptrarr_offset (int i)
+{
+ void *p = &eparr[i];
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void nowarn_free_local_ptrarr (int i)
+{
+ void* a[4];
+ sink (a);
+ free (a[0]);
+ free (a[1]);
+ free (a[i]);
+}
+
+
+void nowarn_free_extern_ptr (void)
+{
+ free (eptr);
+}
+
+void nowarn_free_extern_ptr_offset (int i)
+{
+ char *p = eptr + i;
+ free (p);
+}
+
+void warn_free_extern_ptr_pos_offset (int i)
+{
+ if (i <= 0)
+ i = 1;
+
+ char *q = eptr + i;
+ free (q); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void nowarn_free_parm_offset (char *p, int i)
+{
+ char *q = p + i;
+ free (q);
+}
+
+void nowarn_free_parm_neg_offset (char *p, int i)
+{
+ if (i >= 0)
+ i = -1;
+
+ char *q = p + i;
+ free (q);
+}
+
+void warn_free_parm_pos_offset (char *p, int i)
+{
+ if (i <= 0)
+ i = 1;
+
+ char *q = p + i;
+ free (q); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+struct Members
+{
+ char a[4], *p, *q;
+};
+
+extern struct Members em;
+
+void nowarn_free_member_ptr (struct Members *pm, int i)
+{
+ char *p = em.p;
+ free (p);
+ p = em.q + i;
+ free (p);
+
+ free (pm->q);
+ p = pm->p;
+ free (pm);
+ free (p);
+}
+
+void nowarn_free_struct_cast (intptr_t *p)
+{
+ struct Members *q = (struct Members*)*p;
+ if (q->p == 0)
+ free (q); // { dg-bogus "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_member_array (void)
+{
+ char *p = em.a;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_member_array_off (int i)
+{
+ char *p = em.a + i;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.C b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.C
new file mode 100644
index 00000000000..5bb0e84eb66
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.C
@@ -0,0 +1,124 @@
+/* PR ????? - No warning on attempts to access free object
+ Verify that attempts to deallocate objects by pointers with nonzero
+ offsets is diagnosed.
+ { dg-do compile }
+ { dg-options "-O2 -Wall -Wfree-nonheap-object" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+typedef __SIZE_TYPE__ size_t;
+
+void sink (void*, ...);
+
+extern char ecarr[];
+extern void* eparr[];
+
+extern char *eptr;
+
+void* source (void);
+
+void nowarn_op_delete (void *p, void ***ppp, size_t n, intptr_t iptr)
+{
+ operator delete (p);
+
+ p = 0;
+ operator delete (p);
+
+ p = operator new (n);
+ sink (p);
+ operator delete (p);
+
+ p = operator new (n);
+ sink (p);
+
+ operator delete ((void*)iptr);
+
+ p = source ();
+ operator delete (p);
+
+ p = source ();
+ p = (char*)p - 1;
+ operator delete (p);
+
+ operator delete (**ppp);
+ operator delete (*ppp);
+ operator delete (pppp);
+}
+
+void warn_op_delete_cstaddr (void *p)
+{
+ operator delete (p);
+ p = (void*)~0;
+ operator delete (p); // { dg-warning "called on a pointer to an unallocated object" } */
+}
+
+void warn_op_delete_funcaddr ()
+{
+ void *p = (void*)&warn_op_delete_funcaddr;
+ operator delete (p); // { dg-warning "called on unallocated object 'void warn_op_delete_funcaddr()" } */
+}
+
+void warn_op_delete_string (void *p)
+{
+ operator delete (p);
+ p = (void*)"";
+ operator delete (p); // { dg-warning "called on a pointer to an unallocated object" } */
+}
+
+void warn_op_delete_ptr_to_self (void *p)
+{
+ operator delete (p);
+ p = &p;
+ operator delete (p); // { dg-warning "called on unallocated object 'p'" } */
+}
+
+void nowarn_op_new_delete (size_t n)
+{
+ void *p = operator new (n);
+ sink (p);
+ operator delete (p);
+}
+
+void nowarn_op_new_delete_ptr_plus (size_t n)
+{
+ void *p0_1 = operator new (n);
+ void *p1 = (char*)p0_1 + 1;
+ sink (p0_1, p1);
+ void *p0_2 = (char*)p1 - 1;
+ sink (p0_1, p1, p0_2);
+ operator delete (p0_2);
+}
+
+void warn_op_new_delete_cstoff (size_t n)
+{
+ void *p = operator new (n);
+ void *q = (char*)p + 1;
+ sink (p, q);
+ operator delete (q); // { dg-warning "'void operator delete\\\(void\\\*\\\)' called on pointer '\[^'\]+' with nonzero offset 1" }
+}
+
+void warn_op_new_delete_ptr_plus (size_t n)
+{
+ char *p = (char*)operator new (n);
+ sink (++p);
+ operator delete (p); // { dg-warning "called on pointer '\[^']+' with nonzero offset 1" }
+}
+
+void warn_op_delete_funcret_plus (size_t n)
+{
+ char *p = source ();
+ sink (++p);
+ operator delete (p); // { dg-warning "called on pointer '\[^']+' with nonzero offset 1" }
+}
+
+void warn_op_delete_eptr_plus (int i)
+{
+ extern char *ecp;
+
+ if (i < 1)
+ i = 1;
+
+ char *p = ecp + i;
+ sink (p);
+
+ operator delete (p); // { dg-warning "called on pointer '\[^']+' with nonzero offset 1" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete.C b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete.C
new file mode 100644
index 00000000000..ed1090be5c5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wmismatched-new-delete.C
@@ -0,0 +1,212 @@
+/* PR c++/90629 - Support for -Wmismatched-new-delete
+ The detection doesn't require optimization.
+ { dg-do compile }
+ { dg-options "-Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern "C" {
+ void free (void *);
+ void* malloc (size_t);
+ void* realloc (void *, size_t);
+ char* strdup (const char *);
+ char* strndup (const char *, size_t);
+}
+
+void sink (void *);
+
+void nowarn_op_new_delete (int n)
+{
+ void *p = operator new (n);
+ sink (p);
+ operator delete (p);
+}
+
+void nowarn_new_delete (int n)
+{
+ {
+ char *p = new char;
+ sink (p);
+ delete p;
+ }
+
+ {
+ char *p = new char[n];
+ sink (p);
+ delete[] p;
+ }
+}
+
+/* Verify a warning for calls to free() with a pointer returned from
+ a call to operator new() or the new expressopm. */
+
+void warn_new_free (int n)
+{
+ {
+ void *p = operator new (n);
+ // { dg-message "returned from a call to 'void\\\* operator new\\\(" "note" { target *-*-* } .-1 }
+ sink (p);
+ free (p);
+ // { dg-warning "'void free\\\(void\\\*\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+ {
+ char *p = new char[n];
+ // { dg-message "returned from a call to 'void\\\* operator new \\\[" "note" { target *-*-* } .-1 }
+ sink (p);
+ free (p);
+ // { dg-warning "'void free\\\(void\\\*\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+}
+
+
+/* Verify a warning for calls to realloc() with a pointer returned from
+ a call to operator new() or the new expressopm. */
+
+void warn_new_realloc (int n)
+{
+ {
+ void *p = operator new (n);
+ // { dg-message "returned from a call to 'void\\\* operator new\\\(" "note" { target *-*-* } .-1 }
+ sink (p);
+ p = realloc (p, n * 2);
+ // { dg-warning "'void\\\* realloc\\\(\[^)\]+\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ sink (p);
+ }
+ {
+ void *p = new char[n];
+ // { dg-message "returned from a call to 'void\\\* operator new \\\[" "note" { target *-*-* } .-1 }
+ sink (p);
+ p = realloc (p, n * 2);
+ // { dg-warning "'void\\\* realloc\\\(\[^)\]+\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ sink (p);
+ }
+}
+
+
+/* Verify a warning for a call to operator_delete() with a pointer returned
+ from a call to malloc(). */
+
+void warn_malloc_op_delete (int n)
+{
+ char *p = (char *)malloc (n);
+ // { dg-message "returned from a call to 'void\\\* malloc\\\(" "note" { target *-*-* } .-1 }
+ sink (p);
+ operator delete (p);
+ // { dg-warning "'void operator delete\\\(void\\\*\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+}
+
+
+/* Verify a warning for an invocation of either form of the delete
+ expression with a pointer returned from a call to malloc(). */
+
+void warn_malloc_delete (int n)
+{
+ {
+ char *p = (char *)malloc (n);
+ // { dg-message "returned from a call to 'void\\\* malloc\\\(" "note" { target *-*-* } .-1 }
+ sink (p);
+ /* C++98 calls operator delete (void*) but later versions call
+ operator delete (void*, size_t). The difference doesn't matter
+ here so verify just that some operator delete is called. */
+ delete p;
+ // { dg-warning "'void operator delete\\\(\[^)\]+\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+
+ {
+ char *p = (char *)malloc (n);
+ // { dg-message "returned from a call to 'void\\\* malloc\\\(" "note" { target *-*-* } .-1 }
+ sink (p);
+ delete[] p;
+ // { dg-warning "'void operator delete \\\[]\\\(void\\\*\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+}
+
+
+/* Verify a warning for an invocation of either form of the delete
+ expression with a pointer returned from a call to realloc(). */
+
+void warn_realloc_delete (void *p1, void *p2, int n)
+{
+ {
+ char *q = (char *)realloc (p1, n);
+ // { dg-message "returned from a call to 'void\\\* realloc\\\(" "note" { target *-*-* } .-1 }
+ sink (q);
+ /* C++98 calls operator delete (void*) but later versions call
+ operator delete (void*, size_t). The difference doesn't matter
+ here so verify just that some operator delete is called. */
+ delete q;
+ // { dg-warning "'void operator delete\\\(\[^)\]+\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+
+ {
+ char *q = (char *)realloc (p2, n);
+ // { dg-message "returned from a call to 'void\\\* realloc\\\(" "note" { target *-*-* } .-1 }
+ sink (q);
+ delete[] q;
+ // { dg-warning "'void operator delete \\\[]\\\(void\\\*\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+}
+
+
+/* Verify a warning for an invocation of either form of the delete
+ expression with a pointer returned from a call to strdup(). */
+
+void warn_strdup_delete (const char *s1, const char *s2)
+{
+ {
+ char *q = strdup (s1);
+ // { dg-message "returned from a call to 'char\\\* strdup\\\(" "note" { target *-*-* } .-1 }
+ sink (q);
+ /* C++98 calls operator delete (void*) but later versions call
+ operator delete (void*, size_t). The difference doesn't matter
+ here so verify just that some operator delete is called. */
+ delete q;
+ // { dg-warning "'void operator delete\\\(\[^)\]+\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+
+ {
+ char *q = strdup (s2);
+ // { dg-message "returned from a call to 'char\\\* strdup\\\(" "note" { target *-*-* } .-1 }
+ sink (q);
+ delete[] q;
+ // { dg-warning "'void operator delete \\\[]\\\(void\\\*\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+}
+
+
+
+/* Verify a warning for an invocation of either form of the delete
+ expression with a pointer returned from a call to strndup(). */
+
+void warn_strdup_delete (const char *s1, const char *s2, size_t n)
+{
+ {
+ char *q = strndup (s1, n);
+ // { dg-message "returned from a call to 'char\\\* strndup\\\(" "note" { target *-*-* } .-1 }
+ sink (q);
+ /* C++98 calls operator delete (void*) but later versions call
+ operator delete (void*, size_t). The difference doesn't matter
+ here so verify just that some operator delete is called. */
+ delete q;
+ // { dg-warning "'void operator delete\\\(\[^)\]+\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+
+ {
+ char *q = strndup (s2, n);
+ // { dg-message "returned from a call to 'char\\\* strndup\\\(" "note" { target *-*-* } .-1 }
+ sink (q);
+ delete[] q;
+ // { dg-warning "'void operator delete \\\[]\\\(void\\\*\\\)' called on pointer returned from a mismatched allocation function" "" { target *-*-* } .-1 }
+ }
+}
+
+
+struct Base { virtual ~Base (); };
+struct Derived: Base { };
+
+void warn_new_free_base_derived ()
+{
+ Base *p = new Derived ();
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wmismatched-new-delete" }
+}
diff --git a/gcc/testsuite/g++.dg/warn/delete-array-1.C b/gcc/testsuite/g++.dg/warn/delete-array-1.C
index c3af71323ac..36e3637d251 100644
--- a/gcc/testsuite/g++.dg/warn/delete-array-1.C
+++ b/gcc/testsuite/g++.dg/warn/delete-array-1.C
@@ -6,6 +6,9 @@ struct S { int a [1]; } s;
void foo (S *p)
{
delete a; // { dg-warning "deleting array" }
+ // { dg-warning "-Wfree-nonheap-object" "" { target *-*-* } .-1 }
delete s.a; // { dg-warning "deleting array" }
+ // { dg-warning "-Wfree-nonheap-object" "" { target *-*-* } .-1 }
delete p->a; // { dg-warning "deleting array" }
+ // { dg-warning "-Wfree-nonheap-object" "" { target *-*-* } .-1 }
}
diff --git a/gcc/testsuite/g++.old-deja/g++.other/delete2.C b/gcc/testsuite/g++.old-deja/g++.other/delete2.C
index 1d0554f2d95..76ae3558a1d 100644
--- a/gcc/testsuite/g++.old-deja/g++.other/delete2.C
+++ b/gcc/testsuite/g++.old-deja/g++.other/delete2.C
@@ -9,5 +9,7 @@ void bar(foo a) {
delete[] a; // should be accepted
char b[1];
delete b; // { dg-warning "deleting array" } expecting pointer type
+ // { dg-warning "-Wfree-nonheap-object" "" { target *-*-* } .-1 }
delete[] b; // { dg-warning "deleting array" } expecting pointer type
+ // { dg-warning "-Wfree-nonheap-object" "" { target *-*-* } .-1 }
}
diff --git a/gcc/testsuite/gcc.dg/Wfree-nonheap-object-2.c b/gcc/testsuite/gcc.dg/Wfree-nonheap-object-2.c
new file mode 100644
index 00000000000..2b00d77e8b8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wfree-nonheap-object-2.c
@@ -0,0 +1,279 @@
+/* PR ????? - No warning on attempts to access free object
+ Verify that attempting to reallocate unallocated objects referenced
+ either directly or through pointers is diagnosed.
+ { dg-do compile }
+ { dg-options "-O2 -Wall -Wfree-nonheap-object" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern void free (void*);
+extern void* alloca (size_t);
+extern void* realloc (void*, size_t);
+
+void sink (void*, ...);
+
+extern void* eparr[];
+extern char *eptr;
+
+extern size_t n;
+
+
+void nowarn_realloc (void *p, size_t n)
+{
+ char *q = realloc (p, n);
+ sink (q);
+
+ q = realloc (0, n);
+ sink (q);
+
+ q = realloc (q, n * 2);
+ sink (q);
+}
+
+/* Verify that calling realloc on a pointer to an unknown object minus
+ some nonzero offset isn't diagnosed, but a pointer plus a positive
+ offset is (a positive offset cannot point at the beginning). */
+
+void test_realloc_offset (char *p1, char *p2, char *p3, size_t n, int i)
+{
+ char *q;
+ q = realloc (p1 - 1, n);
+ sink (q);
+
+ q = realloc (p2 + 1, n); // { dg-warning "'realloc' called on pointer 'p2' with nonzero offset 1" }
+ sink (q);
+
+ q = realloc (p3 + i, n);
+ sink (q);
+}
+
+void warn_realloc_extern_arr (void)
+{
+ extern char ecarr[]; // { gg-message "declared here" }
+ char *p = ecarr;
+ char *q = realloc (p, n); // { dg-warning "'realloc' called on unallocated object 'ecarr'" }
+ sink (q);
+}
+
+void warn_realloc_extern_arr_offset (int i)
+{
+ extern char ecarr[];
+ char *p = ecarr + i;
+ char *q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+}
+
+
+void warn_realloc_string (int i)
+{
+ char *p, *q;
+ {
+ p = "123";
+ sink (p);
+ q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+ {
+ p = "234" + 1;
+ sink (p);
+ q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+ {
+ p = "123" + i;
+ sink (p);
+ q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+}
+
+
+void warn_realloc_alloca (int n, int i)
+{
+ char *p, *q;
+ {
+ p = alloca (n);
+ sink (p);
+ q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+ {
+ p = (char*)alloca (n + 1);
+ sink (p);
+ q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+ {
+ p = (char*)alloca (n + 2) + i;
+ sink (p);
+ q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+}
+
+
+void warn_realloc_local_arr (int i)
+{
+ char *q;
+ {
+ char a[4];
+ sink (a);
+ q = realloc (a, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+
+ {
+ char b[5];
+ sink (b);
+ q = realloc (b + 1, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+
+ {
+ char c[6];
+ sink (c);
+ q = realloc (&c[2], n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+
+ {
+ char d[7];
+ sink (d);
+ q = realloc (&d[i], n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+}
+
+void warn_realloc_vla (int n1, int n2, int i)
+{
+ char *q;
+ {
+ char vla[n1];
+ sink (vla);
+ q = realloc (vla, n2); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+
+ {
+ char vlb[n1 + 1];
+ sink (vlb);
+ q = realloc (vlb + 1, n2);// { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+
+ {
+ char vlc[n1 + 2];
+ sink (vlc);
+ q = realloc (&vlc[2], n2);// { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+
+ {
+ char vld[7];
+ sink (vld);
+ q = realloc (&vld[i], n2);// { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+ }
+}
+
+void nowarn_realloc_extern_ptrarr (void)
+{
+ char *q = realloc (*eparr, n);
+ sink (q);
+}
+
+void nowarn_realloc_extern_ptrarr_offset (int i)
+{
+ char *p = eparr[i];
+ char *q = realloc (p, n);
+ sink (q);
+}
+
+
+void warn_realloc_extern_ptrarr (void)
+{
+ char *q = realloc (eparr, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+}
+
+void warn_realloc_extern_ptrarr_offset (int i)
+{
+ void *p = eparr + i;
+ void *q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+}
+
+
+void nowarn_realloc_extern_ptr (void)
+{
+ char *q = realloc (eptr, n);
+ sink (q);
+}
+
+void nowarn_realloc_extern_ptr_offset (int i)
+{
+ char *p = eptr + i;
+ char *q = realloc (p, n);
+ sink (q);
+}
+
+
+void warn_realloc_extern_ptr_pos_offset (int i)
+{
+ if (i <= 0)
+ i = 1;
+
+ char *p = eptr + i;
+ char *q = realloc (p, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+}
+
+
+void nowarn_realloc_parm_offset (char *p, int i)
+{
+ char *q = p + i;
+ q = realloc (q, n);
+ sink (q);
+}
+
+void nowarn_realloc_parm_neg_offset (char *p, int i)
+{
+ if (i >= 0)
+ i = -1;
+
+ char *q = p + i;
+ q = realloc (q, n);
+ sink (q);
+}
+
+void warn_realloc_parm_pos_offset (char *p, int i)
+{
+ if (i <= 0)
+ i = 1;
+
+ char *q = p + i;
+ q = realloc (q, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+}
+
+void nowarn_realloc_deref_parm_pos_offset (void **p, int i)
+{
+ if (i <= 0)
+ i = 1;
+
+ // The offset is from p, not *p.
+ void *q = *(p + i);
+ q = realloc (q, n);
+ sink (q);
+}
+
+void warn_realloc_deref_parm_pos_offset (void **p, int i)
+{
+ if (i <= 0)
+ i = 1;
+
+ // Unlike in the function above the offset is from *p.
+ void *q = *p + i;
+ q = realloc (q, n); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ sink (q);
+}
diff --git a/gcc/testsuite/gcc.dg/Wfree-nonheap-object-3.c b/gcc/testsuite/gcc.dg/Wfree-nonheap-object-3.c
new file mode 100644
index 00000000000..a472b93fb87
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wfree-nonheap-object-3.c
@@ -0,0 +1,57 @@
+/* PR ????? - No warning on attempts to access free object
+ Verify that freeing unallocated objects referenced indirectly through
+ pointers obtained from function calls is diagnosed.
+ { dg-do compile }
+ { dg-options "-O2 -Wall -Wfree-nonheap-object" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern void free (void*);
+extern char* memchr (const void*, int, size_t);
+extern char* strchr (const char*, int);
+
+void sink (void*, ...);
+
+extern char ecarr[];
+extern void* eparr[];
+
+extern char *eptr;
+
+
+void warn_free_memchr_ecarr (int x, size_t n)
+{
+ char *p = memchr (ecarr, x, n);
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_memchr_ecarr_offset (int i, int j, int x, size_t n)
+{
+ char *p = memchr (ecarr + i, x, n);
+ char *q = p + j;
+ sink (p, q);
+ free (q); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_memchr_local_arr (int x, size_t n)
+{
+ char a[8];
+ sink (a);
+
+ char *p = memchr (a, x, n);
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_memchr_local_arr_offset (int i, int j, int x, size_t n)
+{
+ char a[8];
+ sink (a);
+
+ char *p = memchr (a + i, x, n);
+ char *q = p + j;
+ sink (p, q);
+ free (q); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
diff --git a/gcc/testsuite/gcc.dg/Wfree-nonheap-object.c b/gcc/testsuite/gcc.dg/Wfree-nonheap-object.c
new file mode 100644
index 00000000000..bb222ccf6ab
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wfree-nonheap-object.c
@@ -0,0 +1,273 @@
+/* PR ????? - No warning on attempts to access free object
+ Verify that freeing unallocated objects referenced either directly
+ or through pointers is diagnosed. In most cases this doesn't require
+ optimization.
+ { dg-do compile }
+ { dg-options "-Wall -Wfree-nonheap-object" } */
+
+typedef __INTPTR_TYPE__ intptr_t;
+typedef __SIZE_TYPE__ size_t;
+
+extern void free (void*);
+extern void* malloc (size_t);
+extern void* realloc (void *p, size_t);
+
+void sink (void*, ...);
+
+extern char ecarr[];
+extern void* eparr[];
+
+extern char *eptr;
+
+void* source (void);
+
+void nowarn_free (void *p, void **pp, size_t n, intptr_t iptr)
+{
+ free (p);
+
+ p = 0;
+ free (p);
+
+ p = malloc (n);
+ sink (p);
+ free (p);
+
+ p = malloc (n);
+ sink (p);
+
+ p = realloc (p, n * 2);
+ sink (p);
+ free (p);
+
+ free ((void*)iptr);
+
+ p = source ();
+ free (p);
+
+ p = source ();
+ p = (char*)p - 1;
+ free (p);
+
+ free (*pp);
+}
+
+void warn_free_extern_arr (void)
+{
+ free (ecarr); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_extern_arr_offset (int i)
+{
+ char *p = ecarr + i;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_cstint (void)
+{
+ void *p = (void*)1;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_func (void)
+{
+ void *p = warn_free_func;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_string (int i)
+{
+ {
+ char *p = "123";
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ char *p = "234" + 1;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ char *p = "345" + i;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+
+ if (i >= 0)
+ {
+ char *p = "456" + i;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+}
+
+void warn_free_local_arr (int i)
+{
+ {
+ char a[4];
+ sink (a);
+ free (a); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ char b[5];
+ sink (b);
+
+ char *p = b + 1;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ char c[6];
+ sink (c);
+
+ char *p = c + i;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+}
+
+
+void warn_free_vla (int n, int i)
+{
+ {
+ int vla[n], *p = vla;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+
+ {
+ int vla[n + 1], *p = vla + 1;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+ {
+ int vla[n + 2], *p = vla + i;
+ sink (p);
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+ }
+}
+
+
+void nowarn_free_extern_ptrarr (void)
+{
+ free (*eparr);
+}
+
+void nowarn_free_extern_ptrarr_offset (int i)
+{
+ char *p = eparr[i];
+ free (p);
+}
+
+
+void warn_free_extern_ptrarr (void)
+{
+ free (eparr); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_extern_ptrarr_offset (int i)
+{
+ void *p = &eparr[i];
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+void nowarn_free_local_ptrarr (int i)
+{
+ void* a[4];
+ sink (a);
+ free (a[0]);
+ free (a[1]);
+ free (a[i]);
+}
+
+
+void nowarn_free_extern_ptr (void)
+{
+ free (eptr);
+}
+
+void nowarn_free_extern_ptr_offset (int i)
+{
+ char *p = eptr + i;
+ free (p);
+}
+
+void nowarn_free_parm_offset (char *p, int i)
+{
+ char *q = p + i;
+ free (q);
+}
+
+void nowarn_free_parm_neg_offset (char *p, int i)
+{
+ if (i >= 0)
+ i = -1;
+
+ char *q = p + i;
+ free (q);
+}
+
+struct Members
+{
+ char a[4], *p, *q;
+};
+
+extern struct Members em;
+
+void nowarn_free_member_ptr (struct Members *pm, int i)
+{
+ char *p = em.p;
+ free (p);
+ p = em.q + i;
+ free (p);
+
+ free (pm->q);
+ p = pm->p;
+ free (pm);
+ free (p);
+}
+
+void nowarn_free_struct_cast (intptr_t *p)
+{
+ struct Members *q = (struct Members*)*p;
+ if (q->p == 0)
+ free (q); // { dg-bogus "\\\[-Wfree-nonheap-object" }
+}
+
+
+void warn_free_member_array (void)
+{
+ char *p = em.a;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_member_array_off (int i)
+{
+ char *p = em.a + i;
+ free (p); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+
+// Range information requires optimization.
+#pragma GCC optimize "1"
+
+void warn_free_extern_ptr_pos_offset (int i)
+{
+ if (i <= 0)
+ i = 1;
+
+ char *q = eptr + i;
+ free (q); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
+
+void warn_free_parm_pos_offset (char *p, int i)
+{
+ if (i <= 0)
+ i = 1;
+
+ char *q = p + i;
+ free (q); // { dg-warning "\\\[-Wfree-nonheap-object" }
+}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)
2020-11-03 23:56 [PATCH] add -Wmismatched-new-delete to middle end (PR 90629) Martin Sebor
@ 2020-11-17 0:54 ` Jeff Law
2020-11-17 18:02 ` Martin Sebor
2020-12-06 9:26 ` Martin Liška
2023-06-07 14:54 ` Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s' (was: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)) Thomas Schwinge
2 siblings, 1 reply; 7+ messages in thread
From: Jeff Law @ 2020-11-17 0:54 UTC (permalink / raw)
To: Martin Sebor, gcc-patches
On 11/3/20 4:56 PM, Martin Sebor via Gcc-patches wrote:
> Attached is a simple middle end implementation of detection of
> mismatched pairs of calls to C++ new and delete, along with
> a substantially enhanced implementation of -Wfree-nonheap-object.
> The latter option has been in place since 2011 but detected only
> the most trivial bugs.
>
> Unlike the Clang -Wmismatched-new-delete which diagnoses
> declarations of "overloaded operator new() and operator delete()
> functions that do not have a corresponding free store function
> defined within the same scope", this patch detects mismatches
> between calls to allocation and deallocation functions, such as
> calling free() on the result of new, of delete on the result of
> array new. The functionality provided by Clang can be added on
> top of what this feature does and since they are so close I think
> it's fine to have both under the same option (a new level could
> be introduced to distinguish the two).
>
> The -Wfree-nonheap-object enhancement lets the warning detect all
> calls to free, realloc, or C++ delete, with pointers that can be
> proven not to point to the first byte of an allocated object.
>
> The patch relies on the well-tested compute_objsize() function
> for the determination of pointer provenance and makes use of
> the changes in the following patch submitted for review just
> yesterday:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557807.html
>
> As usual, I tested on x86_64-linux with Glibc & Binutils/GDB with
> no new false positives.
>
> Martin
>
> PS A few words on the implementation choices:
>
> The new code is in builtins.c only because -Wfree-nonheap-object
> is there. I still plan to move all of the invalid access checking
> code into its own module or pass at some point but I didn't want
> to make this improvement contingent on that restructuring.
> Even though it's all in builtins.c, the code is called from calls.c.
> This is so that simple mismatches can be diagnosed even when free
> isn't handed in builtins.c (i.e., without optimization).
> The warning makes no attempt to analyze the CFG or handle
> conditional mismatches. That will have to wait until the code
> is moved to a GIMPLE pass.
>
> gcc-90629.diff
>
> PR c++/90629 - Support for -Wmismatched-new-delete
>
> gcc/ChangeLog:
>
> PR c++/90629
> * builtins.c (access_ref::access_ref): Initialize new member.
> (compute_objsize): Use access_ref::deref. Handle simple pointer
> assignment.
> (expand_builtin): Remove handling of the free built-in.
> (find_assignment_location): New function.
> (gimple_call_alloc_p): Same.
> (gimple_call_dealloc_argno): Same.
> (gimple_call_dealloc_p): Same.
> (matching_alloc_calls_p): Same.
> (warn_dealloc_offset): Same.
> * builtins.h (struct access_ref): Declare new member.
> (maybe_emit_free_warning): Make extern. Make use of access_ref.
> Handle -Wmismatched-new-delete.
> * calls.c (initialize_argument_information): Call
> maybe_emit_free_warning.
> * doc/invoke.texi (-Wfree-nonheap-object): Expand documentation.
> (-Wmismatched-new-delete): Document new option.
>
> gcc/c-family/ChangeLog:
>
> PR c++/90629
> * c.opt (-Wmismatched-new-delete): New option.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/90629
> * g++.dg/warn/delete-array-1.C: Add expected warning.
> * g++.old-deja/g++.other/delete2.C: Add expected warning.
> * g++.dg/warn/Wfree-nonheap-object-2.C: New test.
> * g++.dg/warn/Wfree-nonheap-object.C: New test.
> * g++.dg/warn/Wmismatched-new-delete.C: New test.
> * gcc.dg/Wfree-nonheap-object-2.c: New test.
> * gcc.dg/Wfree-nonheap-object-3.c: New test.
> * gcc.dg/Wfree-nonheap-object.c: New test.
So do we need to reconcile with David M's patch that adds the
"deallocated_by" attribute. In that thread I raised the question about
using the same attribute to track both pointers as well as things like
file descriptors. It looks like this patch ignores everything except
builtins/language intrinsics as allocation functions. ISTM that would
need to be fixed for David's patch to be useful here, right?
As we discussed privately, this can trigger false positives,
particularly for unoptimized code as we've seen in Fedora testing. But
I'm not terribly worried about these.
I haven't seen anything failing with a valid error because of this, but
that's because the packages where it's triggering aren't compiling with
-Werror. I did some quick grepping of the logs and I think I found
instances of each of the new warnings.
If you're interested torsion.usersys.redhat.com:/opt/notnfs/law/WARNINGS
contains all the lines with "warning:" from all the Fedora test builds.
Warning (pun intended), it's big... 10G, so don't try to download it
:-) But it is faster than find | xargs zgrep across all the build logs :-)
>
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 5d60eab6ba2..1b8a5b82dac 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -12589,30 +12594,336 @@ maybe_emit_sprintf_chk_warning (tree exp, enum built_in_function fcode)
> access_write_only);
> }
>
> -/* Emit warning if a free is called with address of a variable. */
> +/* Return the location of the assignment STMT if it has one, or another
> + assignment on the chain that has one. Used to improve the location
> + of informational notes. */
>
> -static void
> +static location_t
> +find_assignment_location (tree var)
> +{
> + gimple *stmt = SSA_NAME_DEF_STMT (var);
> +
> + for (gimple *asgn = stmt; ; )
> + {
> + if (gimple_has_location (asgn))
> + return gimple_location (asgn);
> +
> + if (!is_gimple_assign (asgn))
> + break;
> +
> + tree rhs = gimple_assign_rhs1 (asgn);
> + if (TREE_CODE (rhs) != SSA_NAME)
> + break;
> +
> + asgn = SSA_NAME_DEF_STMT (rhs);
> + }
What is this code for ^^^
Under what conditions does the assignment not have a location? Perhaps
if it's a default definition?
All the nonsense walking up the use-def chain seems unnecessary and
probably misleading when we actually issue the diagnostic. Why are you
going through the trouble to do that?
+
> + gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
> + gsi_next_nondebug (&gsi);
> + if (gimple *next_stmt = gsi_stmt (gsi))
> + {
> + if (gimple_has_location (next_stmt))
> + return gimple_location (next_stmt);
> + }
> + else
> + {
> + gsi = gsi_for_stmt (stmt);
> + gsi_prev_nondebug (&gsi);
> + if (gimple *prev_stmt = gsi_stmt (gsi))
> + if (gimple_has_location (prev_stmt))
> + return gimple_location (prev_stmt);
> + }
> +
> + /* As a last resort fallback, return the location of the closing
> + curly brace. */
> + return cfun->function_end_locus;
> +}
And if you clean up the code above, then all this becomes pointless, no?
Jeff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)
2020-11-17 0:54 ` Jeff Law
@ 2020-11-17 18:02 ` Martin Sebor
2020-12-02 0:09 ` Jeff Law
0 siblings, 1 reply; 7+ messages in thread
From: Martin Sebor @ 2020-11-17 18:02 UTC (permalink / raw)
To: Jeff Law, gcc-patches
On 11/16/20 5:54 PM, Jeff Law wrote:
>
> On 11/3/20 4:56 PM, Martin Sebor via Gcc-patches wrote:
>> Attached is a simple middle end implementation of detection of
>> mismatched pairs of calls to C++ new and delete, along with
>> a substantially enhanced implementation of -Wfree-nonheap-object.
>> The latter option has been in place since 2011 but detected only
>> the most trivial bugs.
>>
>> Unlike the Clang -Wmismatched-new-delete which diagnoses
>> declarations of "overloaded operator new() and operator delete()
>> functions that do not have a corresponding free store function
>> defined within the same scope", this patch detects mismatches
>> between calls to allocation and deallocation functions, such as
>> calling free() on the result of new, of delete on the result of
>> array new. The functionality provided by Clang can be added on
>> top of what this feature does and since they are so close I think
>> it's fine to have both under the same option (a new level could
>> be introduced to distinguish the two).
>>
>> The -Wfree-nonheap-object enhancement lets the warning detect all
>> calls to free, realloc, or C++ delete, with pointers that can be
>> proven not to point to the first byte of an allocated object.
>>
>> The patch relies on the well-tested compute_objsize() function
>> for the determination of pointer provenance and makes use of
>> the changes in the following patch submitted for review just
>> yesterday:
>> https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557807.html
>>
>> As usual, I tested on x86_64-linux with Glibc & Binutils/GDB with
>> no new false positives.
>>
>> Martin
>>
>> PS A few words on the implementation choices:
>>
>> The new code is in builtins.c only because -Wfree-nonheap-object
>> is there. I still plan to move all of the invalid access checking
>> code into its own module or pass at some point but I didn't want
>> to make this improvement contingent on that restructuring.
>> Even though it's all in builtins.c, the code is called from calls.c.
>> This is so that simple mismatches can be diagnosed even when free
>> isn't handed in builtins.c (i.e., without optimization).
>> The warning makes no attempt to analyze the CFG or handle
>> conditional mismatches. That will have to wait until the code
>> is moved to a GIMPLE pass.
>>
>> gcc-90629.diff
>>
>> PR c++/90629 - Support for -Wmismatched-new-delete
>>
>> gcc/ChangeLog:
>>
>> PR c++/90629
>> * builtins.c (access_ref::access_ref): Initialize new member.
>> (compute_objsize): Use access_ref::deref. Handle simple pointer
>> assignment.
>> (expand_builtin): Remove handling of the free built-in.
>> (find_assignment_location): New function.
>> (gimple_call_alloc_p): Same.
>> (gimple_call_dealloc_argno): Same.
>> (gimple_call_dealloc_p): Same.
>> (matching_alloc_calls_p): Same.
>> (warn_dealloc_offset): Same.
>> * builtins.h (struct access_ref): Declare new member.
>> (maybe_emit_free_warning): Make extern. Make use of access_ref.
>> Handle -Wmismatched-new-delete.
>> * calls.c (initialize_argument_information): Call
>> maybe_emit_free_warning.
>> * doc/invoke.texi (-Wfree-nonheap-object): Expand documentation.
>> (-Wmismatched-new-delete): Document new option.
>>
>> gcc/c-family/ChangeLog:
>>
>> PR c++/90629
>> * c.opt (-Wmismatched-new-delete): New option.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR c++/90629
>> * g++.dg/warn/delete-array-1.C: Add expected warning.
>> * g++.old-deja/g++.other/delete2.C: Add expected warning.
>> * g++.dg/warn/Wfree-nonheap-object-2.C: New test.
>> * g++.dg/warn/Wfree-nonheap-object.C: New test.
>> * g++.dg/warn/Wmismatched-new-delete.C: New test.
>> * gcc.dg/Wfree-nonheap-object-2.c: New test.
>> * gcc.dg/Wfree-nonheap-object-3.c: New test.
>> * gcc.dg/Wfree-nonheap-object.c: New test.
>
> So do we need to reconcile with David M's patch that adds the
> "deallocated_by" attribute. In that thread I raised the question about
> using the same attribute to track both pointers as well as things like
> file descriptors. It looks like this patch ignores everything except
> builtins/language intrinsics as allocation functions. ISTM that would
> need to be fixed for David's patch to be useful here, right?
Our emails have crossed mid air. I replied to your comments on
David's patch here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559309.html
The work I point to there extends this warning to user-defined
functions.
I also gave some preliminary comments on David's initial RFC.
It has some more background:
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555667.html
> As we discussed privately, this can trigger false positives,
> particularly for unoptimized code as we've seen in Fedora testing. But
> I'm not terribly worried about these.
>
> I haven't seen anything failing with a valid error because of this, but
> that's because the packages where it's triggering aren't compiling with
> -Werror. I did some quick grepping of the logs and I think I found
> instances of each of the new warnings.
>
>
> If you're interested torsion.usersys.redhat.com:/opt/notnfs/law/WARNINGS
> contains all the lines with "warning:" from all the Fedora test builds.
> Warning (pun intended), it's big... 10G, so don't try to download it
> :-) But it is faster than find | xargs zgrep across all the build logs :-)
There are quite a few (411 instances of -Wmismatched-new-delete to
be exact) but without more context (at least the informational notes)
they're hard to analyze. I looked at just the first one and it points
to this bug:
./gengetopt/builds/80/log.gz:gengetopt.cc:581:12: warning: 'free' called
on pointer returned from a mismatched allocation function
[-Wmismatched-new-delete]
gengetopt_create_option (gengetopt_option *&n, const char * long_opt,
char short_opt,
const char * desc,
int type, int flagstat, int required,
const char * default_value,
const char * group_value,
const char * mode_value,
const char * type_str,
const AcceptedValues *acceptedvalues,
int multiple,
int argoptional)
{
if ((long_opt == NULL) ||
(long_opt[0] == 0) ||
(desc == NULL))
return FOUND_BUG;
n = new gengetopt_option; <<< allocate by new
if (n == NULL)
return NOT_ENOUGH_MEMORY;
// here we will set required anyway
n->required_set = true;
n->long_opt = strdup (long_opt);
if (n->long_opt == NULL)
{
free (n); <<< deallocate by free
return NOT_ENOUGH_MEMORY;
}
Based on what others have said about what some static analyzers
report I expect this to be a common bug (the "operator delete"
message accounts for 336 instances out of the 411).
>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>> index 5d60eab6ba2..1b8a5b82dac 100644
>> --- a/gcc/builtins.c
>> +++ b/gcc/builtins.c
>> @@ -12589,30 +12594,336 @@ maybe_emit_sprintf_chk_warning (tree exp, enum built_in_function fcode)
>> access_write_only);
>> }
>>
>> -/* Emit warning if a free is called with address of a variable. */
>> +/* Return the location of the assignment STMT if it has one, or another
>> + assignment on the chain that has one. Used to improve the location
>> + of informational notes. */
>>
>> -static void
>> +static location_t
>> +find_assignment_location (tree var)
>> +{
>> + gimple *stmt = SSA_NAME_DEF_STMT (var);
>> +
>> + for (gimple *asgn = stmt; ; )
>> + {
>> + if (gimple_has_location (asgn))
>> + return gimple_location (asgn);
>> +
>> + if (!is_gimple_assign (asgn))
>> + break;
>> +
>> + tree rhs = gimple_assign_rhs1 (asgn);
>> + if (TREE_CODE (rhs) != SSA_NAME)
>> + break;
>> +
>> + asgn = SSA_NAME_DEF_STMT (rhs);
>> + }
>
> What is this code for ^^^
>
>
> Under what conditions does the assignment not have a location? Perhaps
> if it's a default definition?
I've seen all sorts of assignments with no location, including
ASSERT_EXPRs, BIT_AND_EXPRs, as well as MIN/MAX_EXPs, clobber
statements and others. I didn't try to understand the pattern
behind them.
> All the nonsense walking up the use-def chain seems unnecessary and
> probably misleading when we actually issue the diagnostic. Why are you
> going through the trouble to do that?
The "nonsense" is me doing my best to provide at least some context
for warnings that might otherwise not have any and be harder to track
down to the root cause. A message like:
warning: ‘free’ called on pointer ‘<unknown>’ with nonzero offset
with nothing else for a function hundreds of lines long makes it
harder to track down the cause of the problem than with a note
pointing to at least a location of the closest assignment to
the unknown pointer.
That said, in this patch the function is only used for constants
and AFAICS, doesn't get exercised by any tests so it can probably
be removed with no adverse effect. I do expect it to need to come
back in some form because of the missing location problem. (It
comes from a bigger patch in progress where it's used more
extensively.)
>> + gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
>> + gsi_next_nondebug (&gsi);
>> + if (gimple *next_stmt = gsi_stmt (gsi))
>> + {
>> + if (gimple_has_location (next_stmt))
>> + return gimple_location (next_stmt);
>> + }
>> + else
>> + {
>> + gsi = gsi_for_stmt (stmt);
>> + gsi_prev_nondebug (&gsi);
>> + if (gimple *prev_stmt = gsi_stmt (gsi))
>> + if (gimple_has_location (prev_stmt))
>> + return gimple_location (prev_stmt);
>> + }
>> +
>> + /* As a last resort fallback, return the location of the closing
>> + curly brace. */
>> + return cfun->function_end_locus;
>> +}
>
> And if you clean up the code above, then all this becomes pointless, no?
I'm not sure what you mean but as I said above, the function
doesn't appear to be essential to this patch and can be removed.
Martin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)
2020-11-17 18:02 ` Martin Sebor
@ 2020-12-02 0:09 ` Jeff Law
0 siblings, 0 replies; 7+ messages in thread
From: Jeff Law @ 2020-12-02 0:09 UTC (permalink / raw)
To: Martin Sebor, gcc-patches
On 11/17/20 11:02 AM, Martin Sebor wrote:
>
>>
>> If you're interested
>> torsion.usersys.redhat.com:/opt/notnfs/law/WARNINGS contains all the
>> lines with "warning:" from all the Fedora test builds. Warning (pun
>> intended), it's big... 10G, so don't try to download it :-) But it
>> is faster than find | xargs zgrep across all the build logs :-)
>
> There are quite a few (411 instances of -Wmismatched-new-delete to
> be exact) but without more context (at least the informational notes)
> they're hard to analyze. I looked at just the first one and it points
> to this bug:
>
> ./gengetopt/builds/80/log.gz:gengetopt.cc:581:12: warning: 'free'
> called on pointer returned from a mismatched allocation function
> [-Wmismatched-new-delete]
>
> gengetopt_create_option (gengetopt_option *&n, const char * long_opt,
> char short_opt,
> const char * desc,
> int type, int flagstat, int required,
> const char * default_value,
> const char * group_value,
> const char * mode_value,
> const char * type_str,
> const AcceptedValues *acceptedvalues,
> int multiple,
> int argoptional)
> {
> if ((long_opt == NULL) ||
> (long_opt[0] == 0) ||
> (desc == NULL))
> return FOUND_BUG;
>
> n = new gengetopt_option; <<< allocate by new
> if (n == NULL)
> return NOT_ENOUGH_MEMORY;
>
> // here we will set required anyway
> n->required_set = true;
>
> n->long_opt = strdup (long_opt);
> if (n->long_opt == NULL)
> {
> free (n); <<< deallocate by free
> return NOT_ENOUGH_MEMORY;
> }
>
> Based on what others have said about what some static analyzers
> report I expect this to be a common bug (the "operator delete"
> message accounts for 336 instances out of the 411).
Yea, I suspect there's a lot of these lying around and probably will
continue to do so as long as folks aren't using -Werror. Hence my
desire to have an opt-in mechanism in Fedora that turns on -Werror
within redhat-rpm-config for opted-in packages.
>
>>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>>> index 5d60eab6ba2..1b8a5b82dac 100644
>>> --- a/gcc/builtins.c
>>> +++ b/gcc/builtins.c
>>> @@ -12589,30 +12594,336 @@ maybe_emit_sprintf_chk_warning (tree exp,
>>> enum built_in_function fcode)
>>> access_write_only);
>>> }
>>> -/* Emit warning if a free is called with address of a variable. */
>>> +/* Return the location of the assignment STMT if it has one, or
>>> another
>>> + assignment on the chain that has one. Used to improve the location
>>> + of informational notes. */
>>> -static void
>>> +static location_t
>>> +find_assignment_location (tree var)
>>> +{
>>> + gimple *stmt = SSA_NAME_DEF_STMT (var);
>>> +
>>> + for (gimple *asgn = stmt; ; )
>>> + {
>>> + if (gimple_has_location (asgn))
>>> + return gimple_location (asgn);
>>> +
>>> + if (!is_gimple_assign (asgn))
>>> + break;
>>> +
>>> + tree rhs = gimple_assign_rhs1 (asgn);
>>> + if (TREE_CODE (rhs) != SSA_NAME)
>>> + break;
>>> +
>>> + asgn = SSA_NAME_DEF_STMT (rhs);
>>> + }
>>
>> What is this code for ^^^
>>
>>
>> Under what conditions does the assignment not have a location?
>> Perhaps if it's a default definition?
>
> I've seen all sorts of assignments with no location, including
> ASSERT_EXPRs, BIT_AND_EXPRs, as well as MIN/MAX_EXPs, clobber
> statements and others. I didn't try to understand the pattern
> behind them.
I'd rather not include that hunk and instead xfail any tests affected by
the missing locations until such time as we fix them rather than just
papering over the problem. Fixing locations helps diagnostics, our
ability to suppress them via pragmas, etc. So it's in our best
interest to fix these issues.
>
>> All the nonsense walking up the use-def chain seems unnecessary and
>> probably misleading when we actually issue the diagnostic. Why are
>> you going through the trouble to do that?
>
> The "nonsense" is me doing my best to provide at least some context
> for warnings that might otherwise not have any and be harder to track
> down to the root cause. A message like:
>
> warning: ‘free’ called on pointer ‘<unknown>’ with nonzero offset
>
> with nothing else for a function hundreds of lines long makes it
> harder to track down the cause of the problem than with a note
> pointing to at least a location of the closest assignment to
> the unknown pointer.
>
> That said, in this patch the function is only used for constants
> and AFAICS, doesn't get exercised by any tests so it can probably
> be removed with no adverse effect. I do expect it to need to come
> back in some form because of the missing location problem. (It
> comes from a bigger patch in progress where it's used more
> extensively.)
ACK. So let's avoid it for now and try to tackle the missing location
problems as we trip over them.
OK with those bits removed.
jeff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)
2020-11-03 23:56 [PATCH] add -Wmismatched-new-delete to middle end (PR 90629) Martin Sebor
2020-11-17 0:54 ` Jeff Law
@ 2020-12-06 9:26 ` Martin Liška
2023-06-07 14:54 ` Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s' (was: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)) Thomas Schwinge
2 siblings, 0 replies; 7+ messages in thread
From: Martin Liška @ 2020-12-06 9:26 UTC (permalink / raw)
To: Martin Sebor, gcc-patches
Hello Martin.
I've just noticed that this commit introduced the following clang
warning:
/home/marxin/BIG/buildbot/buildworker/marxinbox-gcc-clang/build/gcc/builtins.c:13071:1: warning: unused function 'call_dealloc_p' [-Wunused-function]
Can you please take a look?
Martin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s' (was: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629))
2020-11-03 23:56 [PATCH] add -Wmismatched-new-delete to middle end (PR 90629) Martin Sebor
2020-11-17 0:54 ` Jeff Law
2020-12-06 9:26 ` Martin Liška
@ 2023-06-07 14:54 ` Thomas Schwinge
2023-06-07 17:10 ` Mike Stump
2 siblings, 1 reply; 7+ messages in thread
From: Thomas Schwinge @ 2023-06-07 14:54 UTC (permalink / raw)
To: gcc-patches, Rainer Orth, Mike Stump; +Cc: Martin Sebor
[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]
Hi!
On 2020-11-03T16:56:48-0700, Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> Attached is a simple middle end implementation of detection of
> mismatched pairs of calls to C++ new and delete, along with
> a substantially enhanced implementation of -Wfree-nonheap-object.
This eventually became commit dce6c58db87ebf7f4477bd3126228e73e4eeee97
"Add support for detecting mismatched allocation/deallocation calls".
Already in this original patch submission:
> diff --git a/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s
> new file mode 100644
> index 00000000000..e69de29bb2d
OK to push the attached
"Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s'"?
Grüße
Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-gcc-testsuite-g-.dg-warn-Wfree-nonheap-object.patch --]
[-- Type: text/x-diff, Size: 820 bytes --]
From d04c97b40a07bd2a3205d9de8577024f5d26aba0 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 7 Jun 2023 16:01:39 +0200
Subject: [PATCH] Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s'
..., which, presumably, was added by mistake in
commit dce6c58db87ebf7f4477bd3126228e73e4eeee97
"Add support for detecting mismatched allocation/deallocation calls".
gcc/testsuite/
* g++.dg/warn/Wfree-nonheap-object.s: Remove.
---
gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s | 0
1 file changed, 0 insertions(+), 0 deletions(-)
delete mode 100644 gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s
diff --git a/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s
deleted file mode 100644
index e69de29bb2d..00000000000
--
2.34.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s' (was: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629))
2023-06-07 14:54 ` Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s' (was: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)) Thomas Schwinge
@ 2023-06-07 17:10 ` Mike Stump
0 siblings, 0 replies; 7+ messages in thread
From: Mike Stump @ 2023-06-07 17:10 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: gcc-patches, Rainer Orth, Martin Sebor
On Jun 7, 2023, at 7:54 AM, Thomas Schwinge <thomas@codesourcery.com> wrote:
>
> On 2020-11-03T16:56:48-0700, Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> Attached is a simple middle end implementation of detection of
>> mismatched pairs of calls to C++ new and delete, along with
>> a substantially enhanced implementation of -Wfree-nonheap-object.
>
> This eventually became commit dce6c58db87ebf7f4477bd3126228e73e4eeee97
> "Add support for detecting mismatched allocation/deallocation calls".
> Already in this original patch submission:
>
>> diff --git a/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s b/gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s
>> new file mode 100644
>> index 00000000000..e69de29bb2d
>
> OK to push the attached
> "Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s'"?
Ok.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-07 17:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 23:56 [PATCH] add -Wmismatched-new-delete to middle end (PR 90629) Martin Sebor
2020-11-17 0:54 ` Jeff Law
2020-11-17 18:02 ` Martin Sebor
2020-12-02 0:09 ` Jeff Law
2020-12-06 9:26 ` Martin Liška
2023-06-07 14:54 ` Remove 'gcc/testsuite/g++.dg/warn/Wfree-nonheap-object.s' (was: [PATCH] add -Wmismatched-new-delete to middle end (PR 90629)) Thomas Schwinge
2023-06-07 17:10 ` Mike Stump
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).