Detect more invalid initializers

in presence of invalid source code we can't rely on the next token to determine if we have or haven't already parsed an initializer element, we really have to track it in some separate state; it's a flag, so merge it with the other two we have (size_only and first). Also add some syntax checks for situations which formerly lead to vstack leaks, see the added testcases.

Michael Matz committed 7 years ago

Commit e6980f61 parent d72b8773 files · +57 −35Browse files

Unified · Split

3 files

tccgen.c +38 −35
tests/tests2/60_errors_and_warnings.c +10 −0
tests/tests2/60_errors_and_warnings.expect +9 −0

tccgen.c+38−35

 static CType *type_decl(CType *type, AttributeDef *ad, int *v, int td);
 static void parse_expr_type(CType *type);
 static void init_putv(CType *type, Section *sec, unsigned long c);
-static void decl_initializer(CType *type, Section *sec, unsigned long c, int first, int size_only);
+static void decl_initializer(CType *type, Section *sec, unsigned long c, int flags);
 static void block(int *bsym, int *csym, int is_expr);
 static void decl_initializer_alloc(CType *type, AttributeDef *ad, int r, int has_init, int v, int scope);
 static void decl(int l);
     }
 }
+#define DIF_FIRST     1
+#define DIF_SIZE_ONLY 2
+#define DIF_HAVE_ELEM 4
 /* t is the array or struct type. c is the array or struct
    address. cur_field is the pointer to the current
    field, for arrays the 'c' member contains the current start
-   index.  'size_only' is true if only size info is needed (only used
+   index.  'flags' is as in decl_initializer.
-   in arrays).  al contains the already initialized length of the
+   'al' contains the already initialized length of the
    current container (starting at c).  This returns the new length of that.  */
 static int decl_designator(CType *type, Section *sec, unsigned long c,
-                           Sym **cur_field, int size_only, int al)
+                           Sym **cur_field, int flags, int al)
 {
     Sym *s, *f;
     int index, index_last, align, l, nb_elems, elem_size;
     elem_size = 0;
     nb_elems = 1;
+    if (flags & DIF_HAVE_ELEM)
+        goto no_designator;
     if (gnu_ext && (l = is_label()) != 0)
         goto struct_field;
     /* NOTE: we only support ranges for last designator */
 	    expect("=");
         }
     } else {
+    no_designator:
         if (type->t & VT_ARRAY) {
 	    index = (*cur_field)->c;
 	    if (type->ref->c >= 0 && index >= type->ref->c)
     }
     /* must put zero in holes (note that doing it that way
        ensures that it even works with designators) */
-    if (!size_only && c - corig > al)
+    if (!(flags & DIF_SIZE_ONLY) && c - corig > al)
 	init_putz(sec, corig + al, c - corig - al);
-    decl_initializer(type, sec, c, 0, size_only);
+    decl_initializer(type, sec, c, flags & ~DIF_FIRST);
     /* XXX: make it more general */
-    if (!size_only && nb_elems > 1) {
+    if (!(flags & DIF_SIZE_ONLY) && nb_elems > 1) {
         unsigned long c_end;
         uint8_t *src, *dst;
         int i;
 /* 't' contains the type and storage info. 'c' is the offset of the
    object in section 'sec'. If 'sec' is NULL, it means stack based
-   allocation. 'first' is true if array '{' must be read (multi
+   allocation. 'flags & DIF_FIRST' is true if array '{' must be read (multi
-   dimension implicit array init handling). 'size_only' is true if
+   dimension implicit array init handling). 'flags & DIF_SIZE_ONLY' is true if
    size only evaluation is wanted (only for arrays). */
 static void decl_initializer(CType *type, Section *sec, unsigned long c,
-                             int first, int size_only)
+                             int flags)
 {
     int len, n, no_oblock, nb, i;
     int size1, align1;
-    int have_elem;
     Sym *s, *f;
     Sym indexsym;
     CType *t1;
-    /* If we currently are at an '}' or ',' we have read an initializer
+    if (!(flags & DIF_HAVE_ELEM) && tok != '{' &&
-       element in one of our callers, and not yet consumed it.  */
-    have_elem = tok == '}' || tok == ',';
-    if (!have_elem && tok != '{' &&
 	/* In case of strings we have special handling for arrays, so
 	   don't consume them as initializer value (which would commit them
 	   to some anonymous symbol).  */
 	tok != TOK_LSTR && tok != TOK_STR &&
-	!size_only) {
+	!(flags & DIF_SIZE_ONLY)) {
 	parse_init_elem(!sec ? EXPR_ANY : EXPR_CONST);
-	have_elem = 1;
+        flags |= DIF_HAVE_ELEM;
     }
-    if (have_elem &&
+    if ((flags & DIF_HAVE_ELEM) &&
 	!(type->t & VT_ARRAY) &&
 	/* Use i_c_parameter_t, to strip toplevel qualifiers.
 	   The source type might have VT_CONSTANT set, which is
         size1 = type_size(t1, &align1);
         no_oblock = 1;
-        if ((first && tok != TOK_LSTR && tok != TOK_STR) ||
+        if (((flags & DIF_FIRST) && tok != TOK_LSTR && tok != TOK_STR) ||
             tok == '{') {
             if (tok != '{')
                 tcc_error("character array initializer must be a literal,"
                     " optionally enclosed in braces");
             skip('{');
             no_oblock = 0;
-	    if (tok == ',')
-		tcc_error("unexpected ','");
         }
         /* only parse strings here if correct type (otherwise: handle
                 nb = cstr_len;
                 if (n >= 0 && nb > (n - len))
                     nb = n - len;
-                if (!size_only) {
+                if (!(flags & DIF_SIZE_ONLY)) {
                     if (cstr_len > nb)
                         tcc_warning("initializer-string for array is too long");
                     /* in order to go faster for common case (char
             /* only add trailing zero if enough storage (no
                warning in this case since it is standard) */
             if (n < 0 || len < n) {
-                if (!size_only) {
+                if (!(flags & DIF_SIZE_ONLY)) {
 		    vpushi(0);
                     init_putv(t1, sec, c + (len * size1));
                 }
           do_init_list:
 	    len = 0;
-	    while (tok != '}' || have_elem) {
+	    while (tok != '}' || (flags & DIF_HAVE_ELEM)) {
-		len = decl_designator(type, sec, c, &f, size_only, len);
+		len = decl_designator(type, sec, c, &f, flags, len);
-		have_elem = 0;
+		flags &= ~DIF_HAVE_ELEM;
 		if (type->t & VT_ARRAY) {
 		    ++indexsym.c;
 		    /* special test for multi dimensional arrays (may not
 	    }
         }
         /* put zeros at the end */
-	if (!size_only && len < n*size1)
+	if (!(flags & DIF_SIZE_ONLY) && len < n*size1)
 	    init_putz(sec, c + len, n*size1 - len);
         if (!no_oblock)
             skip('}');
     } else if ((type->t & VT_BTYPE) == VT_STRUCT) {
 	size1 = 1;
         no_oblock = 1;
-        if (first || tok == '{') {
+        if ((flags & DIF_FIRST) || tok == '{') {
             skip('{');
             no_oblock = 0;
         }
         n = s->c;
 	goto do_init_list;
     } else if (tok == '{') {
+        if (flags & DIF_HAVE_ELEM)
+          skip(';');
         next();
-        decl_initializer(type, sec, c, first, size_only);
+        decl_initializer(type, sec, c, flags & ~DIF_HAVE_ELEM);
         skip('}');
-    } else if (size_only) {
+    } else if ((flags & DIF_SIZE_ONLY)) {
 	/* If we supported only ISO C we wouldn't have to accept calling
-	   this on anything than an array size_only==1 (and even then
+	   this on anything than an array if DIF_SIZE_ONLY (and even then
 	   only on the outermost level, so no recursion would be needed),
 	   because initializing a flex array member isn't supported.
 	   But GNU C supports it, so we need to recurse even into
-	   subfields of structs and arrays when size_only is set.  */
+	   subfields of structs and arrays when DIF_SIZE_ONLY is set.  */
         /* just skip expression */
         skip_or_save_block(NULL);
     } else {
-	if (!have_elem) {
+	if (!(flags & DIF_HAVE_ELEM)) {
 	    /* This should happen only when we haven't parsed
 	       the init element above for fear of committing a
 	       string constant to memory too early.  */
         /* compute size */
         begin_macro(init_str, 1);
         next();
-        decl_initializer(type, NULL, 0, 1, 1);
+        decl_initializer(type, NULL, 0, DIF_FIRST | DIF_SIZE_ONLY);
         /* prepare second initializer parsing */
         macro_ptr = init_str->str;
         next();
 	size_t oldreloc_offset = 0;
 	if (sec && sec->reloc)
 	  oldreloc_offset = sec->reloc->data_offset;
-        decl_initializer(type, sec, addr, 1, 0);
+        decl_initializer(type, sec, addr, DIF_FIRST);
 	if (sec && sec->reloc)
 	  squeeze_multi_relocs(sec, oldreloc_offset);
         /* patch flexible array member size back to -1, */

tests/tests2/60_errors_and_warnings.c+10−0

@@ -140,5 +140,15 @@ int bar (const char (g)()) // should match this 'g' argument · 139 unmodified lines
140	int foo(int ()) // abstract decl is wrong in definitions	140	int foo(int ()) // abstract decl is wrong in definitions
141	{	141	{
142	return 0;	142	return 0;
		143	#elif defined test_invalid_1
		144	void f(char*);
		145	void g(void) {
		146	f((char[]){1, ,});
143	}	147	}
		148	#elif defined test_invalid_2
		149	int ga = 0.42 { 2 };
		150	#elif defined test_invalid_3
		151	struct S { int a, b; };
		152	struct T { struct S x; };
		153	struct T gt = { 42 a: 1, 43 };
144	#endif	154	#endif

tests/tests2/60_errors_and_warnings.expect+9−0

@@ -62,3 +62,12 @@ · 61 unmodified lines
62		62
63	[test_abstract_decls]	63	[test_abstract_decls]
64	60_errors_and_warnings.c:141: error: identifier expected	64	60_errors_and_warnings.c:141: error: identifier expected
		65
		66	[test_invalid_1]
		67	60_errors_and_warnings.c:146: error: identifier expected
		68
		69	[test_invalid_2]
		70	60_errors_and_warnings.c:149: error: ';' expected (got "{")
		71
		72	[test_invalid_3]
		73	60_errors_and_warnings.c:153: error: ',' expected (got "a")

Detect more invalid initializers

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code