Re: options: Remove 'gcc/c-family/c.opt:Wuse-after-free' option definition record (was: [PATCH v2 1/2] add -Wuse-after-free)
On Tue, 29 Mar 2022, Thomas Schwinge wrote: > | --- gcc/c-family/c.opt > | +++ gcc/c-family/c.opt > | [...] > | +# Defining these options here in addition to common.opt is necessary > | +# in order for the default -Wall setting of -Wuse-after-free=2 to take > | +# effect. > | + > | +Wuse-after-free > | +LangEnabledBy(C ObjC C++ LTO ObjC++) > | +; in common.opt > | + > | +Wuse-after-free= > | +LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0) > | +; in common.opt > | [...] > > OK to push the attached "options: Remove > 'gcc/c-family/c.opt:Wuse-after-free' option definition record"? OK. -- Joseph S. Myers jos...@codesourcery.com
Re: options: Remove 'gcc/c-family/c.opt:Wuse-after-free' option definition record (was: [PATCH v2 1/2] add -Wuse-after-free)
On 3/29/22 03:24, Thomas Schwinge wrote: Hi! On 2022-01-15T17:00:11-0700, Martin Sebor via Gcc-patches wrote: On 1/11/22 15:40, Jason Merrill wrote: On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote: [default setting of the option] Let's put =2 in -Wall for now. I've adjusted [...] and pushed r12-6605 [...] That's from commit 671a283636de75f7ed638ee6b01ed2d44361b8b6 "Add -Wuse-after-free [PR80532]": | --- gcc/common.opt | +++ gcc/common.opt | [...] | +Wuse-after-free | +Common Var(warn_use_after_free) Warning | +Warn for uses of pointers to deallocated strorage. | + | +Wuse-after-free= | +Common Joined RejectNegative UInteger Var(warn_use_after_free) Warning IntegerRange(0, 3) | +Warn for uses of pointers to deallocated strorage. | [...] | --- gcc/c-family/c.opt | +++ gcc/c-family/c.opt | [...] | +# Defining these options here in addition to common.opt is necessary | +# in order for the default -Wall setting of -Wuse-after-free=2 to take | +# effect. | + | +Wuse-after-free | +LangEnabledBy(C ObjC C++ LTO ObjC++) | +; in common.opt | + | +Wuse-after-free= | +LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0) | +; in common.opt | [...] OK to push the attached "options: Remove 'gcc/c-family/c.opt:Wuse-after-free' option definition record"? It's fine with me if it passes tests. I remember noticing a subtle distinction in how option aliases are sometimes treated in #pragma GCC diagnostic but not the exact details. I added tests to make sure it has the expected effect without the trailing =. See the comment in c-c++-common/Wuse-after-free.c. Thanks Martin 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
options: Remove 'gcc/c-family/c.opt:Wuse-after-free' option definition record (was: [PATCH v2 1/2] add -Wuse-after-free)
Hi! On 2022-01-15T17:00:11-0700, Martin Sebor via Gcc-patches wrote: > On 1/11/22 15:40, Jason Merrill wrote: >> On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote: >>> [default setting of the option] >> Let's put =2 in -Wall for now. > I've adjusted [...] and pushed r12-6605 [...] That's from commit 671a283636de75f7ed638ee6b01ed2d44361b8b6 "Add -Wuse-after-free [PR80532]": | --- gcc/common.opt | +++ gcc/common.opt | [...] | +Wuse-after-free | +Common Var(warn_use_after_free) Warning | +Warn for uses of pointers to deallocated strorage. | + | +Wuse-after-free= | +Common Joined RejectNegative UInteger Var(warn_use_after_free) Warning IntegerRange(0, 3) | +Warn for uses of pointers to deallocated strorage. | [...] | --- gcc/c-family/c.opt | +++ gcc/c-family/c.opt | [...] | +# Defining these options here in addition to common.opt is necessary | +# in order for the default -Wall setting of -Wuse-after-free=2 to take | +# effect. | + | +Wuse-after-free | +LangEnabledBy(C ObjC C++ LTO ObjC++) | +; in common.opt | + | +Wuse-after-free= | +LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0) | +; in common.opt | [...] OK to push the attached "options: Remove 'gcc/c-family/c.opt:Wuse-after-free' option definition record"? 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 >From eda478e23a611611353d22e11727a797b9f321f9 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Sat, 26 Mar 2022 22:07:54 +0100 Subject: [PATCH] options: Remove 'gcc/c-family/c.opt:Wuse-after-free' option definition record A one-argument form of the 'LangEnabledBy' option property isn't defined, and effectively appears to be a no-op. Removing that one, the 'gcc/c-family/c.opt:Wuse-after-free' option definition record becomes empty, and doesn't add anything over 'gcc/common.opt:Wuse-after-free', and may thus be removed entirely. This only changes 'build-gcc/gcc/optionlist' accordingly, but no other generated files. Clean-up after recent commit 671a283636de75f7ed638ee6b01ed2d44361b8b6 "Add -Wuse-after-free [PR80532]". gcc/c-family/ * c.opt (Wuse-after-free): Remove. --- gcc/c-family/c.opt | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 3c2ec7744b0..1034a1b3946 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1373,14 +1373,10 @@ Wunused-const-variable= C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable) Warning LangEnabledBy(C ObjC,Wunused-variable, 1, 0) IntegerRange(0, 2) Warn when a const variable is unused. -; Defining these options here in addition to common.opt is necessary +; Defining this option here in addition to common.opt is necessary ; in order for the default -Wall setting of -Wuse-after-free=2 to take ; effect. -Wuse-after-free -LangEnabledBy(C ObjC C++ LTO ObjC++) -; in common.opt - Wuse-after-free= LangEnabledBy(C ObjC C++ LTO ObjC++, Wall,2,0) ; in common.opt -- 2.25.1
Remove mysterious '-# Defining these options here in addition to common.opt is necessary' command-line option (was: [PATCH v2 1/2] add -Wuse-after-free)
Hi!
On 2022-01-15T17:00:11-0700, Martin Sebor via Gcc-patches
wrote:
> On 1/11/22 15:40, Jason Merrill wrote:
>> On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote:
>>> [default setting of the option]
>> Let's put =2 in -Wall for now.
> I've adjusted [...] and pushed r12-6605 [...]
Pushed to master branch commit 43911ddd18b97d8ebd17d2959f36efa539d359b7
"Remove mysterious '-# Defining these options here in addition to
common.opt is necessary' command-line option", see attached. ;'-)
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
>From 43911ddd18b97d8ebd17d2959f36efa539d359b7 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Thu, 24 Mar 2022 22:34:30 +0100
Subject: [PATCH] Remove mysterious '-# Defining these options here in addition
to common.opt is necessary' command-line option
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Before:
$ [...]/gcc '-# Defining these options here in addition to common.opt is necessary' -S -x c /dev/null && echo MYSTERIOUS
MYSTERIOUS
After:
$ [...]/gcc '-# Defining these options here in addition to common.opt is necessary' -S -x c /dev/null && echo MYSTERIOUS
gcc: error: unrecognized command-line option ‘-# Defining these options here in addition to common.opt is necessary’
This commit changes:
--- [...]/build-gcc/gcc/optionlist 2022-03-24 22:12:07.936746648 +0100
+++ [...]/build-gcc/gcc/optionlist 2022-03-24 22:30:06.976737341 +0100
@@ -1,4 +1,3 @@
-# Defining these options here in addition to common.opt is necessary # in order for the default -Wall setting of -Wuse-after-free=2 to take # effect.
### Driver
-all-warnings Ada AdaWhy AdaSCIL Alias(Wall)
-all-warnings C ObjC C++ ObjC++ Warning Alias(Wall)
[...]
--- [...]/build-gcc/gcc/options.cc 2022-03-24 22:12:09.548727738 +0100
+++ [...]/build-gcc/gcc/options.cc 2022-03-24 22:30:08.904727249 +0100
@@ -3222,15 +3222,6 @@
const struct cl_option cl_options[] =
{
/* [0] = */ {
-"-# Defining these options here in addition to common.opt is necessary",
-"# effect.",
-NULL,
-NULL,
-NULL, NULL, N_OPTS, N_OPTS, 68, /* .neg_idx = */ -1,
-0,
-0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-(unsigned short) -1, 0, CLVC_INTEGER, 0, -1, -1 },
- /* [1] = */ {
"-###",
NULL,
NULL,
[...]
..., and re-numbers all following options.
--- [...]/build-gcc/gcc/options.h 2022-03-21 23:24:25.894226828 +0100
+++ [...]/build-gcc/gcc/options.h 2022-03-24 22:30:07.288735708 +0100
@@ -9753,2118 +9753,2117 @@
enum opt_code
{
- OPT___Defining_these_options_here_in_addition_to_common_opt_is_necessary = 0,/* -# Defining these options here in addition to common.opt is necessary */
[...]
..., and likewise re-numbers all following options.
Clean-up for commit 671a283636de75f7ed638ee6b01ed2d44361b8b6
"Add -Wuse-after-free [PR80532]".
gcc/c-family/
* c.opt: Properly quote comment.
---
gcc/c-family/c.opt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9a4828ebe37..790d47caf0a 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1377,9 +1377,9 @@ Wunused-const-variable=
C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable) Warning LangEnabledBy(C ObjC,Wunused-variable, 1, 0) IntegerRange(0, 2)
Warn when a const variable is unused.
-# Defining these options here in addition to common.opt is necessary
-# in order for the default -Wall setting of -Wuse-after-free=2 to take
-# effect.
+; Defining these options here in addition to common.opt is necessary
+; in order for the default -Wall setting of -Wuse-after-free=2 to take
+; effect.
Wuse-after-free
LangEnabledBy(C ObjC C++ LTO ObjC++)
--
2.35.1
Re: [PATCH v2 1/2] add -Wuse-after-free
On 1/11/2022 3:40 PM, Jason Merrill wrote: On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote: Attached is a revised patch with the following changes based on your comments: 1) Set and use statement uids to determine which statement precedes which in the same basic block. 2) Avoid testing flag_isolate_erroneous_paths_dereference. 3) Use post-dominance to decide whether to use the "maybe" phrasing vs a definite form. David raised (and in our offline discussion today reiterated) an objection to the default setting of the option being the strictest. I have not changed that in this revision. See my rationale for this choice in my reply below: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html In the latest C2x draft I see in the list of undefined behavior "The value of a pointer that refers to space deallocated by a call to the free or realloc function is used (7.22.3)." So the case that would be technically undefined would be comparing the reallocated pointer to the old pointer which has been deallocated. The C++ draft is more nuanced: it says, "When the end of the duration of a region of storage is reached, the values of all pointers representing the address of any part of that region of storage become invalid pointer values (6.8.3). Indirection through an invalid pointer value and passing an invalid pointer value to a deallocation function have undefined behavior. Any other use of an invalid pointer value has implementation-defined behavior." So the case above is implementation-defined in C++, not undefined. Let's put =2 in -Wall for now. With that change, this and the -Wdangling-pointer patch are OK on Friday afternoon if there are no other comments before then. THanks for picking this up. I've been busier than expected the last several weeks. jeff
Re: [PATCH v2 1/2] add -Wuse-after-free
On 1/11/22 15:40, Jason Merrill wrote: On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote: Attached is a revised patch with the following changes based on your comments: 1) Set and use statement uids to determine which statement precedes which in the same basic block. 2) Avoid testing flag_isolate_erroneous_paths_dereference. 3) Use post-dominance to decide whether to use the "maybe" phrasing vs a definite form. David raised (and in our offline discussion today reiterated) an objection to the default setting of the option being the strictest. I have not changed that in this revision. See my rationale for this choice in my reply below: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html In the latest C2x draft I see in the list of undefined behavior "The value of a pointer that refers to space deallocated by a call to the free or realloc function is used (7.22.3)." So the case that would be technically undefined would be comparing the reallocated pointer to the old pointer which has been deallocated. The C++ draft is more nuanced: it says, "When the end of the duration of a region of storage is reached, the values of all pointers representing the address of any part of that region of storage become invalid pointer values (6.8.3). Indirection through an invalid pointer value and passing an invalid pointer value to a deallocation function have undefined behavior. Any other use of an invalid pointer value has implementation-defined behavior." So the case above is implementation-defined in C++, not undefined. Let's put =2 in -Wall for now. With that change, this and the -Wdangling-pointer patch are OK on Friday afternoon if there are no other comments before then. I've adjusted both patches and pushed r12-6605 and r12-6606 after retesting with a few packages. In -Wdangling-pointer I reworded the warning to refer to "unnamed temporary" instead of "compound literal" since it triggers for those as well. While rebuilding LLVM and a few its projects with the patches I noticed the -Wdangling-pointer change exposes what seems like a Ranger bug for one translation unit (it enters an infinite loop): I opened pr104038 for it. Martin On 11/23/21 2:16 PM, Martin Sebor wrote: On 11/22/21 6:32 PM, Jeff Law wrote: On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote: Patch 1 in the series detects a small subset of uses of pointers made indeterminate by calls to deallocation functions like free or C++ operator delete. To control the conditions the warnings are issued under the new -Wuse-after-free= option provides three levels. At the lowest level the warning triggers only for unconditional uses of freed pointers and doesn't warn for uses in equality expressions. Level 2 warns also for come conditional uses, and level 3 also for uses in equality expressions. I debated whether to make level 2 or 3 the default included in -Wall. I decided on 3 for two reasons: 1) to raise awareness of both the problem and GCC's new ability to detect it: using a pointer after it's been freed, even only in principle, by a successful call to realloc, is undefined, and 2) because it's trivial to lower the level either globally, or locally by suppressing the warning around such misuses. I've tested the patch on x86_64-linux and by building Glibc and Binutils/GDB. It triggers a number of times in each, all due to comparing invalidated pointers for equality (i.e., level 3). I have suppressed these in GCC (libiberty) by a #pragma, and will see how the Glibc folks want to deal with theirs (I track them in BZ #28521). The tests contain a number of xfails due to limitations I'm aware of. I marked them pr?? until the patch is approved. I will open bugs for them before committing if I don't resolve them in a followup. Martin gcc-63272-1.diff Add -Wuse-after-free. gcc/c-family/ChangeLog * c.opt (-Wuse-after-free): New options. gcc/ChangeLog: * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle OPT_Wreturn_local_addr and OPT_Wuse_after_free_. * diagnostic-spec.h (NW_DANGLING): New enumerator. * doc/invoke.texi (-Wuse-after-free): Document new option. * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename... (pass_waccess::check_call_access): ...to this. (pass_waccess::check): Rename... (pass_waccess::check_block): ...to this. (pass_waccess::check_pointer_uses): New function. (pass_waccess::gimple_call_return_arg): New function. (pass_waccess::warn_invalid_pointer): New function. (pass_waccess::check_builtin): Handle free and realloc. (gimple_use_after_inval_p): New function. (get_realloc_lhs): New function. (maybe_warn_mismatched_realloc): New function. (pointers_related_p): New function. (pass_waccess::check_call): Call check_pointer_uses. (pass_waccess::execute): Compute and free dominance info. libcpp/ChangeLog: * files.c (_cpp_find_file): Substitute a valid pointer for
Re: [PATCH v2 1/2] add -Wuse-after-free
On 11/30/21 17:32, Martin Sebor via Gcc-patches wrote: Attached is a revised patch with the following changes based on your comments: 1) Set and use statement uids to determine which statement precedes which in the same basic block. 2) Avoid testing flag_isolate_erroneous_paths_dereference. 3) Use post-dominance to decide whether to use the "maybe" phrasing vs a definite form. David raised (and in our offline discussion today reiterated) an objection to the default setting of the option being the strictest. I have not changed that in this revision. See my rationale for this choice in my reply below: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html In the latest C2x draft I see in the list of undefined behavior "The value of a pointer that refers to space deallocated by a call to the free or realloc function is used (7.22.3)." So the case that would be technically undefined would be comparing the reallocated pointer to the old pointer which has been deallocated. The C++ draft is more nuanced: it says, "When the end of the duration of a region of storage is reached, the values of all pointers representing the address of any part of that region of storage become invalid pointer values (6.8.3). Indirection through an invalid pointer value and passing an invalid pointer value to a deallocation function have undefined behavior. Any other use of an invalid pointer value has implementation-defined behavior." So the case above is implementation-defined in C++, not undefined. Let's put =2 in -Wall for now. With that change, this and the -Wdangling-pointer patch are OK on Friday afternoon if there are no other comments before then. On 11/23/21 2:16 PM, Martin Sebor wrote: On 11/22/21 6:32 PM, Jeff Law wrote: On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote: Patch 1 in the series detects a small subset of uses of pointers made indeterminate by calls to deallocation functions like free or C++ operator delete. To control the conditions the warnings are issued under the new -Wuse-after-free= option provides three levels. At the lowest level the warning triggers only for unconditional uses of freed pointers and doesn't warn for uses in equality expressions. Level 2 warns also for come conditional uses, and level 3 also for uses in equality expressions. I debated whether to make level 2 or 3 the default included in -Wall. I decided on 3 for two reasons: 1) to raise awareness of both the problem and GCC's new ability to detect it: using a pointer after it's been freed, even only in principle, by a successful call to realloc, is undefined, and 2) because it's trivial to lower the level either globally, or locally by suppressing the warning around such misuses. I've tested the patch on x86_64-linux and by building Glibc and Binutils/GDB. It triggers a number of times in each, all due to comparing invalidated pointers for equality (i.e., level 3). I have suppressed these in GCC (libiberty) by a #pragma, and will see how the Glibc folks want to deal with theirs (I track them in BZ #28521). The tests contain a number of xfails due to limitations I'm aware of. I marked them pr?? until the patch is approved. I will open bugs for them before committing if I don't resolve them in a followup. Martin gcc-63272-1.diff Add -Wuse-after-free. gcc/c-family/ChangeLog * c.opt (-Wuse-after-free): New options. gcc/ChangeLog: * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle OPT_Wreturn_local_addr and OPT_Wuse_after_free_. * diagnostic-spec.h (NW_DANGLING): New enumerator. * doc/invoke.texi (-Wuse-after-free): Document new option. * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename... (pass_waccess::check_call_access): ...to this. (pass_waccess::check): Rename... (pass_waccess::check_block): ...to this. (pass_waccess::check_pointer_uses): New function. (pass_waccess::gimple_call_return_arg): New function. (pass_waccess::warn_invalid_pointer): New function. (pass_waccess::check_builtin): Handle free and realloc. (gimple_use_after_inval_p): New function. (get_realloc_lhs): New function. (maybe_warn_mismatched_realloc): New function. (pointers_related_p): New function. (pass_waccess::check_call): Call check_pointer_uses. (pass_waccess::execute): Compute and free dominance info. libcpp/ChangeLog: * files.c (_cpp_find_file): Substitute a valid pointer for an invalid one to avoid -Wuse-0after-free. libiberty/ChangeLog: * regex.c: Suppress -Wuse-after-free. gcc/testsuite/ChangeLog: * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free. * gcc.dg/Wmismatched-dealloc-3.c: Same. * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free. * gcc.dg/attr-alloc_size-7.c: Same. * c-c++-common/Wuse-after-free-2.c: New test. * c-c++-common/Wuse-after-free-3.c: New test. * c-c++-common/Wuse-after-free-4.c: New test. * c-c++-common/
PING 4 [PATCH v2 1/2] add -Wuse-after-free
Last ping before stage 3 ends: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 1/4/22 11:01, Martin Sebor wrote: Ping. (CC'ing Jason as requested.) https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 12/13/21 9:48 AM, Martin Sebor wrote: Ping. Jeff, I addressed your comments in the updated patch. If there are no other changes is the last revision okay to commit? https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 12/6/21 5:50 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 11/30/21 3:32 PM, Martin Sebor wrote: Attached is a revised patch with the following changes based on your comments: 1) Set and use statement uids to determine which statement precedes which in the same basic block. 2) Avoid testing flag_isolate_erroneous_paths_dereference. 3) Use post-dominance to decide whether to use the "maybe" phrasing vs a definite form. David raised (and in our offline discussion today reiterated) an objection to the default setting of the option being the strictest. I have not changed that in this revision. See my rationale for this choice in my reply below: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html Martin On 11/23/21 2:16 PM, Martin Sebor wrote: On 11/22/21 6:32 PM, Jeff Law wrote: On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote: Patch 1 in the series detects a small subset of uses of pointers made indeterminate by calls to deallocation functions like free or C++ operator delete. To control the conditions the warnings are issued under the new -Wuse-after-free= option provides three levels. At the lowest level the warning triggers only for unconditional uses of freed pointers and doesn't warn for uses in equality expressions. Level 2 warns also for come conditional uses, and level 3 also for uses in equality expressions. I debated whether to make level 2 or 3 the default included in -Wall. I decided on 3 for two reasons: 1) to raise awareness of both the problem and GCC's new ability to detect it: using a pointer after it's been freed, even only in principle, by a successful call to realloc, is undefined, and 2) because it's trivial to lower the level either globally, or locally by suppressing the warning around such misuses. I've tested the patch on x86_64-linux and by building Glibc and Binutils/GDB. It triggers a number of times in each, all due to comparing invalidated pointers for equality (i.e., level 3). I have suppressed these in GCC (libiberty) by a #pragma, and will see how the Glibc folks want to deal with theirs (I track them in BZ #28521). The tests contain a number of xfails due to limitations I'm aware of. I marked them pr?? until the patch is approved. I will open bugs for them before committing if I don't resolve them in a followup. Martin gcc-63272-1.diff Add -Wuse-after-free. gcc/c-family/ChangeLog * c.opt (-Wuse-after-free): New options. gcc/ChangeLog: * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle OPT_Wreturn_local_addr and OPT_Wuse_after_free_. * diagnostic-spec.h (NW_DANGLING): New enumerator. * doc/invoke.texi (-Wuse-after-free): Document new option. * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename... (pass_waccess::check_call_access): ...to this. (pass_waccess::check): Rename... (pass_waccess::check_block): ...to this. (pass_waccess::check_pointer_uses): New function. (pass_waccess::gimple_call_return_arg): New function. (pass_waccess::warn_invalid_pointer): New function. (pass_waccess::check_builtin): Handle free and realloc. (gimple_use_after_inval_p): New function. (get_realloc_lhs): New function. (maybe_warn_mismatched_realloc): New function. (pointers_related_p): New function. (pass_waccess::check_call): Call check_pointer_uses. (pass_waccess::execute): Compute and free dominance info. libcpp/ChangeLog: * files.c (_cpp_find_file): Substitute a valid pointer for an invalid one to avoid -Wuse-0after-free. libiberty/ChangeLog: * regex.c: Suppress -Wuse-after-free. gcc/testsuite/ChangeLog: * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free. * gcc.dg/Wmismatched-dealloc-3.c: Same. * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free. * gcc.dg/attr-alloc_size-7.c: Same. * c-c++-common/Wuse-after-free-2.c: New test. * c-c++-common/Wuse-after-free-3.c: New test. * c-c++-common/Wuse-after-free-4.c: New test. * c-c++-common/Wuse-after-free-5.c: New test. * c-c++-common/Wuse-after-free-6.c: New test. * c-c++-common/Wuse-after-free-7.c: New test. * c-c++-common/Wuse-after-free.c: New test. * g++.dg/warn/Wdangling-pointer.C: New test. * g++.dg/warn/Wmismatched-dealloc-3.C: New test. * g++.dg/warn/Wuse-after-free.C: New test. diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index 63fc27a1
PING 3 [PATCH v2 1/2] add -Wuse-after-free
Ping. (CC'ing Jason as requested.) https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 12/13/21 9:48 AM, Martin Sebor wrote: Ping. Jeff, I addressed your comments in the updated patch. If there are no other changes is the last revision okay to commit? https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 12/6/21 5:50 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 11/30/21 3:32 PM, Martin Sebor wrote: Attached is a revised patch with the following changes based on your comments: 1) Set and use statement uids to determine which statement precedes which in the same basic block. 2) Avoid testing flag_isolate_erroneous_paths_dereference. 3) Use post-dominance to decide whether to use the "maybe" phrasing vs a definite form. David raised (and in our offline discussion today reiterated) an objection to the default setting of the option being the strictest. I have not changed that in this revision. See my rationale for this choice in my reply below: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html Martin On 11/23/21 2:16 PM, Martin Sebor wrote: On 11/22/21 6:32 PM, Jeff Law wrote: On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote: Patch 1 in the series detects a small subset of uses of pointers made indeterminate by calls to deallocation functions like free or C++ operator delete. To control the conditions the warnings are issued under the new -Wuse-after-free= option provides three levels. At the lowest level the warning triggers only for unconditional uses of freed pointers and doesn't warn for uses in equality expressions. Level 2 warns also for come conditional uses, and level 3 also for uses in equality expressions. I debated whether to make level 2 or 3 the default included in -Wall. I decided on 3 for two reasons: 1) to raise awareness of both the problem and GCC's new ability to detect it: using a pointer after it's been freed, even only in principle, by a successful call to realloc, is undefined, and 2) because it's trivial to lower the level either globally, or locally by suppressing the warning around such misuses. I've tested the patch on x86_64-linux and by building Glibc and Binutils/GDB. It triggers a number of times in each, all due to comparing invalidated pointers for equality (i.e., level 3). I have suppressed these in GCC (libiberty) by a #pragma, and will see how the Glibc folks want to deal with theirs (I track them in BZ #28521). The tests contain a number of xfails due to limitations I'm aware of. I marked them pr?? until the patch is approved. I will open bugs for them before committing if I don't resolve them in a followup. Martin gcc-63272-1.diff Add -Wuse-after-free. gcc/c-family/ChangeLog * c.opt (-Wuse-after-free): New options. gcc/ChangeLog: * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle OPT_Wreturn_local_addr and OPT_Wuse_after_free_. * diagnostic-spec.h (NW_DANGLING): New enumerator. * doc/invoke.texi (-Wuse-after-free): Document new option. * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename... (pass_waccess::check_call_access): ...to this. (pass_waccess::check): Rename... (pass_waccess::check_block): ...to this. (pass_waccess::check_pointer_uses): New function. (pass_waccess::gimple_call_return_arg): New function. (pass_waccess::warn_invalid_pointer): New function. (pass_waccess::check_builtin): Handle free and realloc. (gimple_use_after_inval_p): New function. (get_realloc_lhs): New function. (maybe_warn_mismatched_realloc): New function. (pointers_related_p): New function. (pass_waccess::check_call): Call check_pointer_uses. (pass_waccess::execute): Compute and free dominance info. libcpp/ChangeLog: * files.c (_cpp_find_file): Substitute a valid pointer for an invalid one to avoid -Wuse-0after-free. libiberty/ChangeLog: * regex.c: Suppress -Wuse-after-free. gcc/testsuite/ChangeLog: * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free. * gcc.dg/Wmismatched-dealloc-3.c: Same. * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free. * gcc.dg/attr-alloc_size-7.c: Same. * c-c++-common/Wuse-after-free-2.c: New test. * c-c++-common/Wuse-after-free-3.c: New test. * c-c++-common/Wuse-after-free-4.c: New test. * c-c++-common/Wuse-after-free-5.c: New test. * c-c++-common/Wuse-after-free-6.c: New test. * c-c++-common/Wuse-after-free-7.c: New test. * c-c++-common/Wuse-after-free.c: New test. * g++.dg/warn/Wdangling-pointer.C: New test. * g++.dg/warn/Wmismatched-dealloc-3.C: New test. * g++.dg/warn/Wuse-after-free.C: New test. diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index 63fc27a1487..2065402a2b9 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -3397,33 +3417,460 @@ pass_waccess::mayb
PING 2 [PATCH v2 1/2] add -Wuse-after-free
Ping. Jeff, I addressed your comments in the updated patch. If there are no other changes is the last revision okay to commit? https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 12/6/21 5:50 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 11/30/21 3:32 PM, Martin Sebor wrote: Attached is a revised patch with the following changes based on your comments: 1) Set and use statement uids to determine which statement precedes which in the same basic block. 2) Avoid testing flag_isolate_erroneous_paths_dereference. 3) Use post-dominance to decide whether to use the "maybe" phrasing vs a definite form. David raised (and in our offline discussion today reiterated) an objection to the default setting of the option being the strictest. I have not changed that in this revision. See my rationale for this choice in my reply below: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html Martin On 11/23/21 2:16 PM, Martin Sebor wrote: On 11/22/21 6:32 PM, Jeff Law wrote: On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote: Patch 1 in the series detects a small subset of uses of pointers made indeterminate by calls to deallocation functions like free or C++ operator delete. To control the conditions the warnings are issued under the new -Wuse-after-free= option provides three levels. At the lowest level the warning triggers only for unconditional uses of freed pointers and doesn't warn for uses in equality expressions. Level 2 warns also for come conditional uses, and level 3 also for uses in equality expressions. I debated whether to make level 2 or 3 the default included in -Wall. I decided on 3 for two reasons: 1) to raise awareness of both the problem and GCC's new ability to detect it: using a pointer after it's been freed, even only in principle, by a successful call to realloc, is undefined, and 2) because it's trivial to lower the level either globally, or locally by suppressing the warning around such misuses. I've tested the patch on x86_64-linux and by building Glibc and Binutils/GDB. It triggers a number of times in each, all due to comparing invalidated pointers for equality (i.e., level 3). I have suppressed these in GCC (libiberty) by a #pragma, and will see how the Glibc folks want to deal with theirs (I track them in BZ #28521). The tests contain a number of xfails due to limitations I'm aware of. I marked them pr?? until the patch is approved. I will open bugs for them before committing if I don't resolve them in a followup. Martin gcc-63272-1.diff Add -Wuse-after-free. gcc/c-family/ChangeLog * c.opt (-Wuse-after-free): New options. gcc/ChangeLog: * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle OPT_Wreturn_local_addr and OPT_Wuse_after_free_. * diagnostic-spec.h (NW_DANGLING): New enumerator. * doc/invoke.texi (-Wuse-after-free): Document new option. * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename... (pass_waccess::check_call_access): ...to this. (pass_waccess::check): Rename... (pass_waccess::check_block): ...to this. (pass_waccess::check_pointer_uses): New function. (pass_waccess::gimple_call_return_arg): New function. (pass_waccess::warn_invalid_pointer): New function. (pass_waccess::check_builtin): Handle free and realloc. (gimple_use_after_inval_p): New function. (get_realloc_lhs): New function. (maybe_warn_mismatched_realloc): New function. (pointers_related_p): New function. (pass_waccess::check_call): Call check_pointer_uses. (pass_waccess::execute): Compute and free dominance info. libcpp/ChangeLog: * files.c (_cpp_find_file): Substitute a valid pointer for an invalid one to avoid -Wuse-0after-free. libiberty/ChangeLog: * regex.c: Suppress -Wuse-after-free. gcc/testsuite/ChangeLog: * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free. * gcc.dg/Wmismatched-dealloc-3.c: Same. * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free. * gcc.dg/attr-alloc_size-7.c: Same. * c-c++-common/Wuse-after-free-2.c: New test. * c-c++-common/Wuse-after-free-3.c: New test. * c-c++-common/Wuse-after-free-4.c: New test. * c-c++-common/Wuse-after-free-5.c: New test. * c-c++-common/Wuse-after-free-6.c: New test. * c-c++-common/Wuse-after-free-7.c: New test. * c-c++-common/Wuse-after-free.c: New test. * g++.dg/warn/Wdangling-pointer.C: New test. * g++.dg/warn/Wmismatched-dealloc-3.C: New test. * g++.dg/warn/Wuse-after-free.C: New test. diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index 63fc27a1487..2065402a2b9 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call (gcall *call) } } +/* Return true if either USE_STMT's basic block (that of a pointer's use) + is dominated by INV
PING [PATCH v2 1/2] add -Wuse-after-free
Ping: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/585816.html On 11/30/21 3:32 PM, Martin Sebor wrote: Attached is a revised patch with the following changes based on your comments: 1) Set and use statement uids to determine which statement precedes which in the same basic block. 2) Avoid testing flag_isolate_erroneous_paths_dereference. 3) Use post-dominance to decide whether to use the "maybe" phrasing vs a definite form. David raised (and in our offline discussion today reiterated) an objection to the default setting of the option being the strictest. I have not changed that in this revision. See my rationale for this choice in my reply below: https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html Martin On 11/23/21 2:16 PM, Martin Sebor wrote: On 11/22/21 6:32 PM, Jeff Law wrote: On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote: Patch 1 in the series detects a small subset of uses of pointers made indeterminate by calls to deallocation functions like free or C++ operator delete. To control the conditions the warnings are issued under the new -Wuse-after-free= option provides three levels. At the lowest level the warning triggers only for unconditional uses of freed pointers and doesn't warn for uses in equality expressions. Level 2 warns also for come conditional uses, and level 3 also for uses in equality expressions. I debated whether to make level 2 or 3 the default included in -Wall. I decided on 3 for two reasons: 1) to raise awareness of both the problem and GCC's new ability to detect it: using a pointer after it's been freed, even only in principle, by a successful call to realloc, is undefined, and 2) because it's trivial to lower the level either globally, or locally by suppressing the warning around such misuses. I've tested the patch on x86_64-linux and by building Glibc and Binutils/GDB. It triggers a number of times in each, all due to comparing invalidated pointers for equality (i.e., level 3). I have suppressed these in GCC (libiberty) by a #pragma, and will see how the Glibc folks want to deal with theirs (I track them in BZ #28521). The tests contain a number of xfails due to limitations I'm aware of. I marked them pr?? until the patch is approved. I will open bugs for them before committing if I don't resolve them in a followup. Martin gcc-63272-1.diff Add -Wuse-after-free. gcc/c-family/ChangeLog * c.opt (-Wuse-after-free): New options. gcc/ChangeLog: * diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle OPT_Wreturn_local_addr and OPT_Wuse_after_free_. * diagnostic-spec.h (NW_DANGLING): New enumerator. * doc/invoke.texi (-Wuse-after-free): Document new option. * gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename... (pass_waccess::check_call_access): ...to this. (pass_waccess::check): Rename... (pass_waccess::check_block): ...to this. (pass_waccess::check_pointer_uses): New function. (pass_waccess::gimple_call_return_arg): New function. (pass_waccess::warn_invalid_pointer): New function. (pass_waccess::check_builtin): Handle free and realloc. (gimple_use_after_inval_p): New function. (get_realloc_lhs): New function. (maybe_warn_mismatched_realloc): New function. (pointers_related_p): New function. (pass_waccess::check_call): Call check_pointer_uses. (pass_waccess::execute): Compute and free dominance info. libcpp/ChangeLog: * files.c (_cpp_find_file): Substitute a valid pointer for an invalid one to avoid -Wuse-0after-free. libiberty/ChangeLog: * regex.c: Suppress -Wuse-after-free. gcc/testsuite/ChangeLog: * gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free. * gcc.dg/Wmismatched-dealloc-3.c: Same. * gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free. * gcc.dg/attr-alloc_size-7.c: Same. * c-c++-common/Wuse-after-free-2.c: New test. * c-c++-common/Wuse-after-free-3.c: New test. * c-c++-common/Wuse-after-free-4.c: New test. * c-c++-common/Wuse-after-free-5.c: New test. * c-c++-common/Wuse-after-free-6.c: New test. * c-c++-common/Wuse-after-free-7.c: New test. * c-c++-common/Wuse-after-free.c: New test. * g++.dg/warn/Wdangling-pointer.C: New test. * g++.dg/warn/Wmismatched-dealloc-3.C: New test. * g++.dg/warn/Wuse-after-free.C: New test. diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc index 63fc27a1487..2065402a2b9 100644 --- a/gcc/gimple-ssa-warn-access.cc +++ b/gcc/gimple-ssa-warn-access.cc @@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call (gcall *call) } } +/* Return true if either USE_STMT's basic block (that of a pointer's use) + is dominated by INVAL_STMT's (that of a pointer's invalidating statement, + which is either a clobber or a deallocation call), or if they're in + the same block, USE_STMT follows INVAL_STMT. */ + +static bool +gimple_use_after_inval_p (gimple *inval_st
[PATCH v2 1/2] add -Wuse-after-free
Attached is a revised patch with the following changes based
on your comments:
1) Set and use statement uids to determine which statement
precedes which in the same basic block.
2) Avoid testing flag_isolate_erroneous_paths_dereference.
3) Use post-dominance to decide whether to use the "maybe"
phrasing vs a definite form.
David raised (and in our offline discussion today reiterated)
an objection to the default setting of the option being
the strictest. I have not changed that in this revision.
See my rationale for this choice in my reply below:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583176.html
Martin
On 11/23/21 2:16 PM, Martin Sebor wrote:
On 11/22/21 6:32 PM, Jeff Law wrote:
On 11/1/2021 4:17 PM, Martin Sebor via Gcc-patches wrote:
Patch 1 in the series detects a small subset of uses of pointers
made indeterminate by calls to deallocation functions like free
or C++ operator delete. To control the conditions the warnings
are issued under the new -Wuse-after-free= option provides three
levels. At the lowest level the warning triggers only for
unconditional uses of freed pointers and doesn't warn for uses
in equality expressions. Level 2 warns also for come conditional
uses, and level 3 also for uses in equality expressions.
I debated whether to make level 2 or 3 the default included in
-Wall. I decided on 3 for two reasons: 1) to raise awareness
of both the problem and GCC's new ability to detect it: using
a pointer after it's been freed, even only in principle, by
a successful call to realloc, is undefined, and 2) because
it's trivial to lower the level either globally, or locally
by suppressing the warning around such misuses.
I've tested the patch on x86_64-linux and by building Glibc
and Binutils/GDB. It triggers a number of times in each, all
due to comparing invalidated pointers for equality (i.e., level
3). I have suppressed these in GCC (libiberty) by a #pragma,
and will see how the Glibc folks want to deal with theirs (I
track them in BZ #28521).
The tests contain a number of xfails due to limitations I'm
aware of. I marked them pr?? until the patch is approved.
I will open bugs for them before committing if I don't resolve
them in a followup.
Martin
gcc-63272-1.diff
Add -Wuse-after-free.
gcc/c-family/ChangeLog
* c.opt (-Wuse-after-free): New options.
gcc/ChangeLog:
* diagnostic-spec.c (nowarn_spec_t::nowarn_spec_t): Handle
OPT_Wreturn_local_addr and OPT_Wuse_after_free_.
* diagnostic-spec.h (NW_DANGLING): New enumerator.
* doc/invoke.texi (-Wuse-after-free): Document new option.
* gimple-ssa-warn-access.cc (pass_waccess::check_call): Rename...
(pass_waccess::check_call_access): ...to this.
(pass_waccess::check): Rename...
(pass_waccess::check_block): ...to this.
(pass_waccess::check_pointer_uses): New function.
(pass_waccess::gimple_call_return_arg): New function.
(pass_waccess::warn_invalid_pointer): New function.
(pass_waccess::check_builtin): Handle free and realloc.
(gimple_use_after_inval_p): New function.
(get_realloc_lhs): New function.
(maybe_warn_mismatched_realloc): New function.
(pointers_related_p): New function.
(pass_waccess::check_call): Call check_pointer_uses.
(pass_waccess::execute): Compute and free dominance info.
libcpp/ChangeLog:
* files.c (_cpp_find_file): Substitute a valid pointer for
an invalid one to avoid -Wuse-0after-free.
libiberty/ChangeLog:
* regex.c: Suppress -Wuse-after-free.
gcc/testsuite/ChangeLog:
* gcc.dg/Wmismatched-dealloc-2.c: Avoid -Wuse-after-free.
* gcc.dg/Wmismatched-dealloc-3.c: Same.
* gcc.dg/attr-alloc_size-6.c: Disable -Wuse-after-free.
* gcc.dg/attr-alloc_size-7.c: Same.
* c-c++-common/Wuse-after-free-2.c: New test.
* c-c++-common/Wuse-after-free-3.c: New test.
* c-c++-common/Wuse-after-free-4.c: New test.
* c-c++-common/Wuse-after-free-5.c: New test.
* c-c++-common/Wuse-after-free-6.c: New test.
* c-c++-common/Wuse-after-free-7.c: New test.
* c-c++-common/Wuse-after-free.c: New test.
* g++.dg/warn/Wdangling-pointer.C: New test.
* g++.dg/warn/Wmismatched-dealloc-3.C: New test.
* g++.dg/warn/Wuse-after-free.C: New test.
diff --git a/gcc/gimple-ssa-warn-access.cc
b/gcc/gimple-ssa-warn-access.cc
index 63fc27a1487..2065402a2b9 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -3397,33 +3417,460 @@ pass_waccess::maybe_check_dealloc_call
(gcall *call)
}
}
+/* Return true if either USE_STMT's basic block (that of a pointer's
use)
+ is dominated by INVAL_STMT's (that of a pointer's invalidating
statement,
+ which is either a clobber or a deallocation call), or if they're in
+ the same block, USE_STMT follows INVAL_STMT. */
+
+static bool
+gimple_use_after_inval_p (gimple *inval_stmt, gimple *use_stmt,
+ bool last_block = false)
+{
+ tree clobvar =
+ gimple_clobber_p (inval_stmt) ?
