On 10/31/2016 06:39 AM, Tobias Burnus wrote:
Martin Sebor wrote:
Attached is an updated patch that adds checks for excessive sizes
and bounds (those in excess of SIZE_MAX / 2), and also enables
the same checking for strcat and strncat). This version also
fixes an issue with the interpretation of anti-ranges in the
first patch. The improvements exposed two bugs in the regression
tests.
If I apply this patch to my local trunk - and try to bootstrap GCC,
bootstrapping fails (on x86-64_gnu-linux) as following. I have not
tried to figure out whether the warning (-Werror) makes sense or not.
../../gcc/emit-rtl.c: In function ‘rtx_note* make_note_raw(insn_note)’:
../../gcc/emit-rtl.c:3933:59: error: void* memset(void*, int, size_t) writing 8
bytes into a region of size 0 overflows the destination
[-Werror=stringop-overflow]
memset (&NOTE_DATA (note), 0, sizeof (NOTE_DATA (note)));
where NOTE_DATA is defined in rtl.h as
/* Opaque data. */
#define NOTE_DATA(INSN) RTL_CHECKC1 (INSN, 3, NOTE)
Thanks for trying it out! The patch bootstrapped and passed regression
tests for me yesterday, also on x86_64, and today on powepc64le, but
just now I reproduced the error with the top of today's trunk on
x86_64. I think the error is justified because the call to memset
in the make_note_raw function references a fourth element
(rtx_note::rtx_insn::rtx_def::u.fld[3]) of what is just a single
element array. After macro expansion, the memset call itself:
memset (&((note)->u.fld[3]), 0, sizeof (((note)->u.fld[3])));
is within the bounds of the object pointed to by note but the index
3 is out of bounds for note->u.fld and so undefined. An unpatched
GCC issues a similar error when the call to memset is replaced with
__builtin___memset_chk like so:
__builtin___memset_chk (&((note)->u.fld[3]), 0,
sizeof (((note)->u.fld[3])),
__builtin_object_size (&((note)->u.fld[3]), 1));
/src/gcc/53562/gcc/emit-rtl.c: In function ‘rtx_note*
make_note_raw(insn_note)’:
/src/gcc/53562/gcc/emit-rtl.c:3941:53: warning: call to void*
__builtin___memset_chk(void*, int, long unsigned int, long unsigned int)
will always overflow destination buffer
The code can be made valid and the warning avoided by deriving
the same address not from an invalid pointer/subscript but rather
from a pointer to the beginning of the union itself and adding
the offset of the fourth element, like so:
char *p = (char*) &(note)->u.fld[0];
p += sizeof (((note)->u.fld[0])) * 3;
memset (p, 0, sizeof *p);
Unfortunately, because the invalid subscript is the result of
the expansion of the RTL_CHECK1() macro fixing this isn't as
straightforward as that. What really should be fixed is the macro
itself. Until then, it can be worked (hacked would be a better
term) around it by storing the result of &NOTE_DATA (note)
expression in a volatile pointer, like so:
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 8afcfbe..6dd9439 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3930,7 +3930,8 @@ make_note_raw (enum insn_note subtype)
INSN_UID (note) = cur_insn_uid++;
NOTE_KIND (note) = subtype;
BLOCK_FOR_INSN (note) = NULL;
- memset (&NOTE_DATA (note), 0, sizeof (NOTE_DATA (note)));
+ void* volatile p = &NOTE_DATA (note);
+ memset (p, 0, sizeof (NOTE_DATA (note)));
return note;
}
Martin
