From: Jakub Jelinek <jakub@redhat.com>
To: Jeff Law <law@redhat.com>, Richard Biener <rguenther@suse.de>,
Martin Sebor <msebor@gmail.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH] Fix attribute access issues
Date: Sat, 23 Nov 2019 01:10:00 -0000 [thread overview]
Message-ID: <20191123010321.GG2466@tucnak> (raw)
In-Reply-To: <8d333632-6f51-6604-b7f9-57018997853f@gmail.com>
Hi!
On Thu, Nov 21, 2019 at 06:09:34PM -0700, Martin Sebor wrote:
> > > PR middle-end/83859
> > > * c-attribs.c (handle_access_attribute): New function.
> > > (c_common_attribute_table): Add new attribute.
> > > (get_argument_type): New function.
> > > (append_access_attrs): New function.
I'm getting
+FAIL: gcc.dg/Wstringop-overflow-24.c (internal compiler error)
+FAIL: gcc.dg/Wstringop-overflow-24.c (test for excess errors)
on i686-linux, while it succeeds on x86_64-linux. On a closer look,
there is a buffer overflow even on x86_64-linux as can be seen under
valgrind, plus memory leak.
The buffer overflow is in append_access_attrs:
==9759== Command: ./cc1 -quiet -Wall Wstringop-overflow-24.c
==9759==
==9759== Invalid write of size 1
==9759== at 0x483BD9F: strcpy (vg_replace_strmem.c:513)
==9759== by 0xA11FF4: append_access_attrs(tree_node*, tree_node*, char const*, char, long*) (c-attribs.c:3934)
==9759== by 0xA12AD3: handle_access_attribute(tree_node**, tree_node*, tree_node*, int, bool*) (c-attribs.c:4158)
==9759== by 0x88E1BF: decl_attributes(tree_node**, tree_node*, int, tree_node*) (attribs.c:728)
==9759== by 0x8A6A9B: c_decl_attributes(tree_node**, tree_node*, int) (c-decl.c:4944)
==9759== by 0x8A6FE2: start_decl(c_declarator*, c_declspecs*, bool, tree_node*) (c-decl.c:5083)
==9759== by 0x91CB15: c_parser_declaration_or_fndef(c_parser*, bool, bool, bool, bool, bool, tree_node**, vec<c_token, va_heap, vl_ptr>, bool, tree_node*, oacc_routine_data*, bool*) (c-parser.c:2216)
==9759== by 0x91B742: c_parser_external_declaration(c_parser*) (c-parser.c:1690)
==9759== by 0x91B25E: c_parser_translation_unit(c_parser*) (c-parser.c:1563)
==9759== by 0x9590A4: c_parse_file() (c-parser.c:21524)
==9759== by 0x9E308E: c_common_parse_file() (c-opts.c:1185)
==9759== by 0x1211AEE: compile_file() (toplev.c:458)
==9759== Address 0x5113f68 is 0 bytes after a block of size 8 alloc'd
==9759== at 0x483880B: malloc (vg_replace_malloc.c:309)
==9759== by 0x229BF17: xmalloc (xmalloc.c:147)
==9759== by 0xA11FC0: append_access_attrs(tree_node*, tree_node*, char const*, char, long*) (c-attribs.c:3932)
==9759== by 0xA12AD3: handle_access_attribute(tree_node**, tree_node*, tree_node*, int, bool*) (c-attribs.c:4158)
==9759== by 0x88E1BF: decl_attributes(tree_node**, tree_node*, int, tree_node*) (attribs.c:728)
==9759== by 0x8A6A9B: c_decl_attributes(tree_node**, tree_node*, int) (c-decl.c:4944)
==9759== by 0x8A6FE2: start_decl(c_declarator*, c_declspecs*, bool, tree_node*) (c-decl.c:5083)
==9759== by 0x91CB15: c_parser_declaration_or_fndef(c_parser*, bool, bool, bool, bool, bool, tree_node**, vec<c_token, va_heap, vl_ptr>, bool, tree_node*, oacc_routine_data*, bool*) (c-parser.c:2216)
==9759== by 0x91B742: c_parser_external_declaration(c_parser*) (c-parser.c:1690)
==9759== by 0x91B25E: c_parser_translation_unit(c_parser*) (c-parser.c:1563)
==9759== by 0x9590A4: c_parse_file() (c-parser.c:21524)
==9759== by 0x9E308E: c_common_parse_file() (c-opts.c:1185)
If n2 != 0, newlen is computed as n1 + n2, but that doesn't take into
account for the , that is added in between the two.
The following patch ought to fix both the buffer overflow (by adding 1 if n2
is non-zero), memory leak (freeing newspec buffer after creating the string;
I've considered using XALLOCAVEC instead, but I believe the string can be
arbitrarily long on functions with thousands of arguments), using XNEWVEC
instead of (type *) xmalloc, using auto_diagnostic_group to bind warning +
inform together and fixes a typo in the documentation.
Ok for trunk if it passes bootstrap/regtest on x86_64-linux and i686-linux?
2019-11-23 Jakub Jelinek <jakub@redhat.com>
PR middle-end/83859
* doc/extend.texi (attribute access): Fix a typo.
* c-attribs.c (append_access_attrs): Avoid buffer overflow. Avoid
memory leak. Use XNEWVEC macro. Use auto_diagnostic_group to
group warning with inform together.
(handle_access_attribute): Formatting fix.
--- gcc/doc/extend.texi.jj 2019-11-22 19:11:53.634970558 +0100
+++ gcc/doc/extend.texi 2019-11-23 01:34:33.344849287 +0100
@@ -2490,7 +2490,7 @@ The following attributes are supported o
The @code{access} attribute enables the detection of invalid or unsafe
accesses by functions to which they apply to or their callers, as well
-as wite-only accesses to objects that are never read from. Such accesses
+as write-only accesses to objects that are never read from. Such accesses
may be diagnosed by warnings such as @option{-Wstringop-overflow},
@option{-Wunnitialized}, @option{-Wunused}, and others.
--- gcc/c-family/c-attribs.c.jj 2019-11-22 19:11:54.000000000 +0100
+++ gcc/c-family/c-attribs.c 2019-11-23 01:44:50.306617000 +0100
@@ -3840,7 +3840,7 @@ append_access_attrs (tree t, tree attrs,
if (idxs[1])
n2 = sprintf (attrspec + n1 + 1, "%u", (unsigned) idxs[1] - 1);
- size_t newlen = n1 + n2;
+ size_t newlen = n1 + n2 + !!n2;
char *newspec = attrspec;
if (tree acs = lookup_attribute ("access", attrs))
@@ -3869,6 +3869,7 @@ append_access_attrs (tree t, tree attrs,
if (*attrspec != pos[-1])
{
/* Mismatch in access mode. */
+ auto_diagnostic_group d;
if (warning (OPT_Wattributes,
"attribute %qs mismatch with mode %qs",
attrstr,
@@ -3884,6 +3885,7 @@ append_access_attrs (tree t, tree attrs,
if ((n2 && pos[n1 - 1] != ','))
{
/* Mismatch in the presence of the size argument. */
+ auto_diagnostic_group d;
if (warning (OPT_Wattributes,
"attribute %qs positional argument 2 conflicts "
"with previous designation",
@@ -3897,6 +3899,7 @@ append_access_attrs (tree t, tree attrs,
if (!n2 && pos[n1 - 1] == ',')
{
/* Mismatch in the presence of the size argument. */
+ auto_diagnostic_group d;
if (warning (OPT_Wattributes,
"attribute %qs missing positional argument 2 "
"provided in previous designation",
@@ -3910,6 +3913,7 @@ append_access_attrs (tree t, tree attrs,
if (n2 && strncmp (attrstr + n1 + 1, pos + n1, n2))
{
/* Mismatch in the value of the size argument. */
+ auto_diagnostic_group d;
if (warning (OPT_Wattributes,
"attribute %qs mismatch positional argument "
"values %i and %i",
@@ -3929,7 +3933,7 @@ append_access_attrs (tree t, tree attrs,
attrspec[n1] = ',';
size_t len = strlen (str);
- newspec = (char *) xmalloc (newlen + len + 1);
+ newspec = XNEWVEC (char, newlen + len + 1);
strcpy (newspec, str);
strcpy (newspec + len, attrspec);
newlen += len;
@@ -3938,7 +3942,10 @@ append_access_attrs (tree t, tree attrs,
/* Connect the two substrings formatted above into a single one. */
attrspec[n1] = ',';
- return build_string (newlen + 1, newspec);
+ tree ret = build_string (newlen + 1, newspec);
+ if (newspec != attrspec)
+ XDELETEVEC (newspec);
+ return ret;
}
/* Handle the access attribute (read_only, write_only, and read_write). */
@@ -4168,7 +4175,8 @@ handle_access_attribute (tree *node, tre
{
/* Repeat for the previously declared type. */
attrs = TYPE_ATTRIBUTES (TREE_TYPE (node[1]));
- tree new_attrs = append_access_attrs (node[1], attrs, attrstr, code, idxs);
+ tree new_attrs
+ = append_access_attrs (node[1], attrs, attrstr, code, idxs);
if (!new_attrs)
return NULL_TREE;
Jakub
next prev parent reply other threads:[~2019-11-23 1:03 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-29 19:52 [WIP PATCH] add object access attributes (PR 83859) Martin Sebor
2019-09-30 7:37 ` Richard Biener
2019-09-30 15:41 ` Martin Sebor
2019-09-30 21:34 ` Joseph Myers
2019-10-01 2:36 ` Martin Sebor
2019-10-17 16:44 ` [PING] " Martin Sebor
2019-10-24 14:42 ` [PING 2] " Martin Sebor
2019-10-27 17:37 ` Jeff Law
2019-10-28 10:18 ` Richard Biener
2019-11-15 21:41 ` Martin Sebor
2019-11-18 9:00 ` Richard Biener
2019-11-18 16:46 ` Martin Sebor
2019-11-19 8:57 ` Richard Biener
2019-11-21 17:12 ` [PATCH v3] " Martin Sebor
2019-11-21 22:40 ` Jeff Law
2019-11-22 1:12 ` Martin Sebor
2019-11-23 1:10 ` Jakub Jelinek [this message]
2019-11-23 10:04 ` [PATCH] Fix attribute access issues Richard Biener
2019-11-25 2:24 ` Martin Sebor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191123010321.GG2466@tucnak \
--to=jakub@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=msebor@gmail.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).