| Message ID | 20230412060845.981953-1-guojiufu@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | testsuite: filter out warning noise for CWE-1341 test | expand |
Add more reviewers. :) Jiufu Guo <guojiufu@linux.ibm.com> writes: > Hi, > > The case file-CWE-1341-example.c checkes [CWE-1341](`double-fclose`). > While on some systems, besides [CWE-1341], a message of [CWE-415] is > also reported. On those systems, attribute `malloc` may be attached on > fopen: > ``` > # 258 "/usr/include/stdio.h" 3 4 > extern FILE *fopen (const char *__restrict __filename, > const char *__restrict __modes) > __attribute__ ((__malloc__)) __attribute__ ((__malloc__ (fclose, 1))) ; > > or say: __attribute_malloc__ __attr_dealloc_fclose __wur; > ``` > > It would be ok to suppress other message except CWE-1341 for this case. > This patch add -Wno-analyzer-double-free to make this case pass on > those systems. > > Tested on ppc64 both BE and LE. > Is this ok for trunk? > > BR, > Jeff (Jiufu) > > gcc/testsuite/ChangeLog: > > PR target/108722 > * gcc.dg/analyzer/file-CWE-1341-example.c: Update. > > --- > gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c b/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c > index 2add3cb109b..830cb0376ea 100644 > --- a/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c > +++ b/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c > @@ -19,6 +19,9 @@ > > IN NO EVENT SHALL THE CONTRIBUTOR, THE ORGANIZATION HE/SHE REPRESENTS OR IS SPONSORED BY (IF ANY), THE MITRE CORPORATION, ITS BOARD OF TRUSTEES, OFFICERS, AGENTS, AND EMPLOYEES BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE INFORMATION OR THE USE OR OTHER DEALINGS IN THE CWE. */ > > +/* This case checks double-fclose only, suppress other warning. */ > +/* { dg-additional-options -Wno-analyzer-double-free } */ > + > #include <stdio.h> > #include <stdlib.h> > #include <string.h>
On Thu, 13 Apr 2023, Jiufu Guo wrote: > > Add more reviewers. :) > > Jiufu Guo <guojiufu@linux.ibm.com> writes: > > > Hi, > > > > The case file-CWE-1341-example.c checkes [CWE-1341](`double-fclose`). > > While on some systems, besides [CWE-1341], a message of [CWE-415] is > > also reported. On those systems, attribute `malloc` may be attached on > > fopen: > > ``` > > # 258 "/usr/include/stdio.h" 3 4 > > extern FILE *fopen (const char *__restrict __filename, > > const char *__restrict __modes) > > __attribute__ ((__malloc__)) __attribute__ ((__malloc__ (fclose, 1))) ; Ouch. I think this should be fixed in the analyzer, "stripping" malloc tracking from fopen/fclose since it does this manually. I've adjusted the bug accordingly. The workaround in the testsuite is OK for trunk. Thanks, Richard. > > or say: __attribute_malloc__ __attr_dealloc_fclose __wur; > > ``` > > > > It would be ok to suppress other message except CWE-1341 for this case. > > This patch add -Wno-analyzer-double-free to make this case pass on > > those systems. > > > > Tested on ppc64 both BE and LE. > > Is this ok for trunk? > > > > BR, > > Jeff (Jiufu) > > > > gcc/testsuite/ChangeLog: > > > > PR target/108722 > > * gcc.dg/analyzer/file-CWE-1341-example.c: Update. > > > > --- > > gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c b/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c > > index 2add3cb109b..830cb0376ea 100644 > > --- a/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c > > +++ b/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c > > @@ -19,6 +19,9 @@ > > > > IN NO EVENT SHALL THE CONTRIBUTOR, THE ORGANIZATION HE/SHE REPRESENTS OR IS SPONSORED BY (IF ANY), THE MITRE CORPORATION, ITS BOARD OF TRUSTEES, OFFICERS, AGENTS, AND EMPLOYEES BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE INFORMATION OR THE USE OR OTHER DEALINGS IN THE CWE. */ > > > > +/* This case checks double-fclose only, suppress other warning. */ > > +/* { dg-additional-options -Wno-analyzer-double-free } */ > > + > > #include <stdio.h> > > #include <stdlib.h> > > #include <string.h> >
On Thu, Apr 13, 2023 at 07:39:01AM +0000, Richard Biener wrote: > On Thu, 13 Apr 2023, Jiufu Guo wrote: > I think this should be fixed in the analyzer, "stripping" malloc > tracking from fopen/fclose since it does this manually. I've adjusted > the bug accordingly. Yeah. > > > +/* This case checks double-fclose only, suppress other warning. */ > > > +/* { dg-additional-options -Wno-analyzer-double-free } */ So please add "(PR108722)" or such to the comment here? That is enough for future people to see if this is still necessary, to maybe remove it from the testcase here, but certainly not cargo-cult it to other testcases! Thanks, Segher
Hi, On 2023-04-13 20:08, Segher Boessenkool wrote: > On Thu, Apr 13, 2023 at 07:39:01AM +0000, Richard Biener wrote: >> On Thu, 13 Apr 2023, Jiufu Guo wrote: >> I think this should be fixed in the analyzer, "stripping" malloc >> tracking from fopen/fclose since it does this manually. I've adjusted >> the bug accordingly. > > Yeah. > >> > > +/* This case checks double-fclose only, suppress other warning. */ >> > > +/* { dg-additional-options -Wno-analyzer-double-free } */ > > So please add "(PR108722)" or such to the comment here? That is enough > for future people to see if this is still necessary, to maybe remove it > from the testcase here, but certainly not cargo-cult it to other > testcases! Good suggestions, thanks! Committed via r13-7176-gedc6659c97c4a7. BR, Jeff (Jiufu) > > Thanks, > > > Segher
diff --git a/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c b/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c index 2add3cb109b..830cb0376ea 100644 --- a/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c +++ b/gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c @@ -19,6 +19,9 @@ IN NO EVENT SHALL THE CONTRIBUTOR, THE ORGANIZATION HE/SHE REPRESENTS OR IS SPONSORED BY (IF ANY), THE MITRE CORPORATION, ITS BOARD OF TRUSTEES, OFFICERS, AGENTS, AND EMPLOYEES BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE INFORMATION OR THE USE OR OTHER DEALINGS IN THE CWE. */ +/* This case checks double-fclose only, suppress other warning. */ +/* { dg-additional-options -Wno-analyzer-double-free } */ + #include <stdio.h> #include <stdlib.h> #include <string.h>
Hi, The case file-CWE-1341-example.c checkes [CWE-1341](`double-fclose`). While on some systems, besides [CWE-1341], a message of [CWE-415] is also reported. On those systems, attribute `malloc` may be attached on fopen: ``` # 258 "/usr/include/stdio.h" 3 4 extern FILE *fopen (const char *__restrict __filename, const char *__restrict __modes) __attribute__ ((__malloc__)) __attribute__ ((__malloc__ (fclose, 1))) ; or say: __attribute_malloc__ __attr_dealloc_fclose __wur; ``` It would be ok to suppress other message except CWE-1341 for this case. This patch add -Wno-analyzer-double-free to make this case pass on those systems. Tested on ppc64 both BE and LE. Is this ok for trunk? BR, Jeff (Jiufu) gcc/testsuite/ChangeLog: PR target/108722 * gcc.dg/analyzer/file-CWE-1341-example.c: Update. --- gcc/testsuite/gcc.dg/analyzer/file-CWE-1341-example.c | 3 +++ 1 file changed, 3 insertions(+)