Bug 93812 - [9/10 Regression] ICE on redeclaration of an attribute format function without protoype
Summary: [9/10 Regression] ICE on redeclaration of an attribute format function withou...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 10.0
: P2 normal
Target Milestone: 9.3
Assignee: Martin Sebor
URL:
Keywords: ice-on-valid-code, patch
Depends on:
Blocks:
 
Reported: 2020-02-18 17:36 UTC by G. Steinmetz
Modified: 2020-03-02 00:45 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-02-18 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description G. Steinmetz 2020-02-18 17:36:42 UTC
Changed between 20181111 and 20181118 :


$ cat z1.c
void f(void)
{
  __attribute__((__format__(__gcc_gfc__, 1, 2))) void f() ;
  f();
}


$ gcc-9-20181111 -c z1.c -Wall
$
$ gcc-10-20200216 -c z1.c -Wall
z1.c: In function 'f':
z1.c:4:3: warning: 'format' attribute argument 2 value '1' exceeds the number of function parameters 0 [-Wattributes]
    4 |   f();
      |   ^
z1.c:4:3: internal compiler error: in get_constant, at c-family/c-format.c:325
0x6a1b4b get_constant
        ../../gcc/c-family/c-format.c:325
0x6a1b4b decode_format_attr
        ../../gcc/c-family/c-format.c:378
0x6a1f77 check_function_format(tree_node const*, tree_node*, int, tree_node**, vec<unsigned int, va_heap, vl_ptr>*)
        ../../gcc/c-family/c-format.c:1183
0x6960e5 check_function_arguments(unsigned int, tree_node const*, tree_node const*, int, tree_node**, vec<unsigned int, va_heap, vl_ptr>*)
        ../../gcc/c-family/c-common.c:5726
0x62e77c build_function_call_vec(unsigned int, vec<unsigned int, va_heap, vl_ptr>, tree_node*, vec<tree_node*, va_gc, vl_embed>*, vec<tree_node*, va_gc, vl_embed>*, tree_node*)
        ../../gcc/c/c-typeck.c:3117
0x64c4c1 c_parser_postfix_expression_after_primary
        ../../gcc/c/c-parser.c:10494
0x63e66b c_parser_postfix_expression
        ../../gcc/c/c-parser.c:10169
0x648252 c_parser_unary_expression
        ../../gcc/c/c-parser.c:8266
0x64927f c_parser_cast_expression
        ../../gcc/c/c-parser.c:8108
0x649539 c_parser_binary_expression
        ../../gcc/c/c-parser.c:7911
0x64a3e5 c_parser_conditional_expression
        ../../gcc/c/c-parser.c:7645
0x64a930 c_parser_expr_no_commas
        ../../gcc/c/c-parser.c:7562
0x64ab92 c_parser_expression
        ../../gcc/c/c-parser.c:10630
0x64b22b c_parser_expression_conv
        ../../gcc/c/c-parser.c:10663
0x65ac7b c_parser_statement_after_labels
        ../../gcc/c/c-parser.c:6294
0x65cd82 c_parser_compound_statement_nostart
        ../../gcc/c/c-parser.c:5800
0x65d516 c_parser_compound_statement
        ../../gcc/c/c-parser.c:5616
0x65ed35 c_parser_declaration_or_fndef
        ../../gcc/c/c-parser.c:2504
0x665d67 c_parser_external_declaration
        ../../gcc/c/c-parser.c:1746
0x666889 c_parser_translation_unit
        ../../gcc/c/c-parser.c:1619
Comment 1 Marek Polacek 2020-02-18 17:41:50 UTC
Started with r9-4163-g1d24950977c730f5e955060057b1dd0b5c051adb
Comment 2 Jakub Jelinek 2020-02-26 14:51:11 UTC
__attribute__((__format__(__printf__, 1, 2))) void foo ();
void foo (void);

void
bar (void)
{
  foo ();
}

ICEs too, no need for nested prototypes or recursive calls.
Comment 3 Jakub Jelinek 2020-02-26 15:18:28 UTC
The problem is that c-format.c assumes that when the "format" attribute is present, handle_format_attribute has already verified it and it was ok.
Unfortunately, that is not the case, because if the attribute appears on a !prototype_p function declaration, then fewer checks are performed on it compared to when it appears on a prototype_p function, and if the !prototype_p function decl is then merged with a prototype_p function decl, nothing performs the checking.
So, one way out of this is drop the validated_p stuff and always validate,, it will then behave similarly to how:
__attribute__((nonnull(2))) void foo (void);
warns and doesn't add the attribute, but
__attribute__((nonnull(2))) void bar ();
void bar (void);
adds it silently (but in this case I believe it doesn't ICE when actually using the attribute).
Another possibility would be to repeat the checking of the attributes which need positional_argument (nonnull, format*, alloc_size, alloc_align) when merging a !prototype_p decl with prototype_p one.
Comment 4 Martin Sebor 2020-02-26 20:25:54 UTC
I'm not sure it makes sense to accept attribute format on functions without a prototype.  It certainly doesn't make sense to keep it after a declaration to which it can't be applied has been seen.  The only other kind of a declaration that the attribute can meaningfully be applied is a variadic one but such is not accepted after one without a prototype.

GCC 8 does accept the attribute on functions without a prototype and even seems to do behave sensibly for "sensible" calls with the format argument in the right place, but without -Wformat-nonliteral it doesn't do anything useful if the format argument is not of the right type (e.g., because it's misplaced) such as in:

  __attribute__((__format__(__printf__, 1, 2))) void foo ();

  void bar (void)
  {
    foo (1, "%s", 2);   // no warning
  }

(As an aside, not diagnosing at least the call if not the declaration seems like a bug because GCC doesn't accept attribute format on variadic functions where the format doesn't have a declared type compatible with char* and where the first-to-check argument isn't the position of the ellipsis (i.e., it can't be an arbitrary position within the variable argument list).)

A simple solution is to simply drop the attribute from functions without a prototype with a warning like Clang does:

  warning: '__format__' attribute only applies to Objective-C
        methods, blocks, and non-K&R-style functions [-Wignored-attributes]

A slightly more involved but less intrusive (to user code) solution is to drop it when a redeclaration is seen with a prototype.
Comment 5 Martin Sebor 2020-02-26 22:51:56 UTC
Patch: https://gcc.gnu.org/ml/gcc-patches/2020-02/msg01503.html
Comment 6 GCC Commits 2020-03-02 00:43:36 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:a499c2f899961f2c09db2dc33e60b66e8d770092

commit r10-6960-ga499c2f899961f2c09db2dc33e60b66e8d770092
Author: Martin Sebor <msebor@redhat.com>
Date:   Sun Mar 1 17:41:45 2020 -0700

    PR c/93812 - ICE on redeclaration of an attribute format function without protoype
    
    gcc/c/ChangeLog:
    
    	PR c/93812
    	* c-typeck.c (build_functype_attribute_variant): New function.
    	(composite_type): Call it.
    
    gcc/testsuite/ChangeLog:
    
    	PR c/93812
    	* gcc.dg/format/proto.c: New test.
Comment 7 Martin Sebor 2020-03-02 00:45:35 UTC
Fixed.