Bug 50584 - No warning for passing small array to C99 static array declarator
Summary: No warning for passing small array to C99 static array declarator
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: 11.0
Assignee: Martin Sebor
URL:
Keywords: diagnostic, patch
: 77577 (view as bug list)
Depends on:
Blocks: 67793
  Show dependency treegraph
 
Reported: 2011-09-30 17:05 UTC by Ian Lance Taylor
Modified: 2022-08-24 01:47 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-05-19 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Lance Taylor 2011-09-30 17:05:20 UTC
C99 introduced the idea of using static in an array declarator in a function prototype parameter.  My reading of the standard is that using static means that the actual argument must be at least that size.  GCC should at least issue a warning when that requirement is clearly violated.

For example, this program


extern void bar (void *a[static 6]);
void foo() {
  void *a1;
  void *a2[5];
  void *a3[6];
  void *a4[10];

  bar (&a1);
  bar (a2);
  bar (a3);
  bar (a4);
}

should issue a warning, or perhaps even an error, for the first two calls to bar, as the compiler can clearly see that the values are not large enough.
Comment 1 Andi Kleen 2013-02-19 06:51:18 UTC
Confirmed. Still happens with 4.7/4.8
Comment 2 Malcolm Inglis 2013-08-14 04:46:50 UTC
GCC 4.8.1 is still failing to warn for static array indices in function parameters.

The C99 status pages say this feature has been done since 3.1: http://gcc.gnu.org/gcc-3.1/c99status.html

Was there a regression?
Comment 3 jsm-csl@polyomino.org.uk 2013-08-21 22:34:03 UTC
The point of this language feature is for optimization, not diagnostics - 
but there is no requirement for either; GCC does all the checks required 
by C99 on the contexts in which [static] may be used.  c99status.html 
specifically notes that it is not used for optimization.
Comment 4 Malcolm Inglis 2013-09-25 10:21:42 UTC
I don't have a copy of the C99 standard, but IBM says [1] that if the function is called with a pointer to a smaller array than specified with `static`, then the behavior is undefined. Ergo, there should be a warning.

Could someone change the status of this bug?

I'm about 60% sure that I once played around with this in GCC, and it correctly reported an error. This wouldn't have been too long ago; maybe 4.6 or even 4.7.

Clang emits a warning for this. [2]

I think this is a fantastic language feature. It's a shame GCC doesn't support it yet.

[1]: http://publib.boulder.ibm.com/infocenter/comphelp/v8v101/topic/com.ibm.xlcpp8a.doc/language/ref/param_decl.htm

[2]: http://hamberg.no/erlend/posts/2013-02-18-static-array-indices.html
Comment 5 jimis 2014-04-10 12:28:36 UTC
I'm currently using this C99 feature. A warning would be nice if NULL or small arrays are passed.
Comment 6 Marek Polacek 2014-04-10 12:51:16 UTC
I don't recommend this kind of usage of the static keyword.  There was even the possibility of removing/deprecating this feature.  Quoting from DR#205:
"There was a unanimous vote that the feature is ugly, and a good consensus that its incorporation into the standard at the 11th hour was an unfortunate decision."
Comment 7 Serg Iv 2015-07-03 01:20:22 UTC
Some excerpts from the C11 standard:

/-----
If the keyword static also appears within the [ and ] of the array type derivation, then for each call to the function, the value of the corresponding
actual argument shall provide access to the first element of an array with at least as many elements as specified by the size expression.
\-----

And another one:

/-----
The following are all compatible function prototype declarators:
[..snip..]
void f(double a[restrict 3][5]);
void f(double a[restrict static 3][5]);

(Note that the last declaration also specifies that the argument corresponding to a in any call to f must be a non-null pointer to the first of at least three arrays of 5 doubles, which the others do not.)
\-----

I'm not sure about warnings (the meaning of the word "shall" is unclear for me), but IMO according to the standard null-pointers should issue an *error*.
Comment 8 Serg Iv 2015-07-03 01:41:25 UTC
Forgot to say that C99 standard has the same sentences.

Useful links:

C99 draft http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf

C11 draft http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1570.pdf
Comment 9 Manuel López-Ibáñez 2015-07-03 09:34:14 UTC
(In reply to Malcolm Inglis from comment #4)
> Could someone change the status of this bug?

Notwithstanding whether the feature is pretty or ugly, GCC does accept this code and warnings for undefined behavior, when possible, are desired. Thus, confirmed. 

This does not mean anyone is going to work on fixing this. Please, if you are using this feature and would like to see this warning in GCC, please consider contributing it: https://gcc.gnu.org/wiki/GettingStarted#Basics:_Contributing_to_GCC_in_10_easy_steps

If you start working on this, it would be good to say so here.
Comment 10 Serg Iv 2015-07-03 12:50:31 UTC
Some thoughts after I read C99 rationale.

Actually, [static N] it's a PROMISE to a compiler, that programmer will always provide N pieces of data. *ALWAYS*.
Therefore compiler can do with this data whatever it wants. It can, for instance, copy it (all N elements) somewhere to some local cache for optimization. Or send it into the space towards the Ape Planet to use brand new Appe book for calculations.
If the programmer didn't provide N pieces of the information - we may have segfault, crash, bsod or nuclear war, it depends. It not a compiler's headache, it's a programmer fault.

So, IMO the right way for GCC is 
a) if it get NULL pointer through all optimization or smaller array is provided - error in compile time
b) if func's supplied with an array lenght >= N, compile without errors and warnings
c) otherwise - warning.
Comment 11 jsm-csl@polyomino.org.uk 2015-07-24 22:52:16 UTC
On Fri, 3 Jul 2015, sergei.ivn+bugzilla at gmail dot com wrote:

> Some excerpts from the C11 standard:
> 
> /-----
> If the keyword static also appears within the [ and ] of the array type
> derivation, then for each call to the function, the value of the corresponding
> actual argument shall provide access to the first element of an array with at
> least as many elements as specified by the size expression.
> \-----

This is in a Semantics section, not Constraints.

> I'm not sure about warnings (the meaning of the word "shall" is unclear for
> me), but IMO according to the standard null-pointers should issue an *error*.

"shall" is defined in clause 4, paragraph 2: 'If a "shall" or "shall not" 
requirement that appears outside of a constraint or runtime-constraint is 
violated, the behavior is undefined.'

In this case, the "shall" relates to a property of an execution of a 
program, not a property of the program itself.  Thus, undefined behavior 
only occurs on such an execution.  In particular, a program with such a 
call inside if (0) - or in any code that the compiler cannot prove will 
always be executed for all executions of the program - must *not* produce 
an error at compile time.  To quote the response to DR#109, "A conforming 
implementation must not fail to translate a strictly conforming program 
simply because some possible execution of that program would result in 
undefined behavior.".

Some such cases of runtime-undefined behavior get compiled into aborts, 
but this is only valid if the abort only occurs when the undefined 
function call would definitely be executed - not, for example, before the 
evaluation of another argument to the function that might exit the program 
or call longjmp (see previous bug fixes in this regard).
Comment 12 nsz 2016-01-07 12:25:16 UTC
(1) consider a simplified example from bug 17308

  __attribute__((nonnull(1)))
  void foo(char *bar) { if (!bar) __builtin_abort(); }

with gcc -O2 -fno-asynchronous-unwind-tables -fomit-frame-pointer -std=c99 -Wall
i get warnings:

  a.c: In function 'foo':
  a.c:2:27: warning: nonnull argument 'bar' compared to NULL [-Wnonnull]
   void foo(char *bar) { if (!bar) __builtin_abort(); }
                           ^
and asm code:

  foo:
  	rep ret

(2) however code with similar semantics:

  void foo(char bar[static 1]) { if (!bar) __builtin_abort(); }

gives no warnings and the generated asm is

  foo:
  	testq	%rdi, %rdi
  	je	.L7
  	rep ret
  .L7:
  	pushq	%rax
  	call	abort

the code in (2) should at least imply nonnull argument, since this is
the idiomatic nonnull annotation in c since c99.

(unfortunately it does not work for void* arguments, otherwise i think
it is useful for static analysis, but it won't be used in practice
unless compilers make use of it.)
Comment 13 Markus Trippelsdorf 2016-09-13 06:16:05 UTC
*** Bug 77577 has been marked as a duplicate of this bug. ***
Comment 14 Fredrik Hederstierna 2017-05-16 14:21:22 UTC
Still present in GCC-7.1.0

Simple test code:

-------------------------------------
int s[1] = { 1 };

void test(int v[static 2])
{
  printf("%d\n", v[1]);
  v[1] = 0;
}

int main(void)
{
  test(s);
  return 0;
}

-------------------------------------
No warnings by GCC:

> gcc -c test.c -W -Wall -Wextra -Warray-bounds -O2 -std=c99

-------------------------------------
But with CLANG-3.8.0 we get warnings:

> clang -c test.c

test.c:13:3: warning: array argument is too small; contains 1 elements, callee
      requires at least 2 [-Warray-bounds]
  test(s);
  ^    ~
test.c:5:15: note: callee declares array parameter as static here
void test(int v[static 2])
              ^~~~~~~~~~~
1 warning generated.
Comment 15 Martin Sebor 2020-05-19 20:55:47 UTC
GCC 10 introduced attribute access to associate a pointer argument with a size of the object it points to.  Although the GCC 10 implementation of the attribute cannot express the same relationship for VLA arguments, conceptually, it should only take a small extension to do it.  I plan to work on it for GCC 11.
Comment 17 GCC Commits 2020-09-19 23:57:25 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:6450f07388f9fe575a489c9309c36012b17b88b0

commit r11-3303-g6450f07388f9fe575a489c9309c36012b17b88b0
Author: Martin Sebor <msebor@redhat.com>
Date:   Sat Sep 19 17:21:52 2020 -0600

    Infrastructure & C front end changes for array parameter checking (PR c/50584).
    
    gcc/ChangeLog:
    
            PR c/50584
            * attribs.c (decl_attributes): Also pass decl along with type
            attributes to handlers.
            (init_attr_rdwr_indices): Change second argument to attribute chain.
            Handle internal attribute representation in addition to external.
            (get_parm_access): New function.
            (attr_access::to_internal_string): Define new member function.
            (attr_access::to_external_string): Define new member function.
            (attr_access::vla_bounds): Define new member function.
            * attribs.h (struct attr_access): Declare new members.
            (attr_access::from_mode_char): Define new member function.
            (get_parm_access): Declare new function.
            * calls.c (initialize_argument_information): Pass function type
            attributes to init_attr_rdwr_indices.
            * doc/invoke.texi (-Warray-parameter, -Wvla-parameter): Document.
            * tree-pretty-print.c (dump_generic_node): Correct handling of
            qualifiers.
            * tree-ssa-uninit.c (maybe_warn_pass_by_reference): Same.
            * tree.h (access_mode): Add new enumerator.
    
    gcc/c-family/ChangeLog:
    
            PR c/50584
            * c-attribs.c (c_common_attribute_table): Add "arg spec" attribute.
            (handle_argspec_attribute): New function.
            (get_argument, get_argument_type): New functions.
            (append_access_attrs): Add overload.  Handle internal attribute
            representation in addition to external.
            (handle_access_attribute): Handle internal attribute representation
            in addition to external.
            (build_attr_access_from_parms): New function.
    
    gcc/c-family/ChangeLog:
    
            PR c/50584
            * c-common.h (warn_parm_array_mismatch): Declare new function.
            (has_attribute): Move declaration of an existing function.
            (build_attr_access_from_parms): Declare new function.
            * c-warn.c (parm_array_as_string): Define new function.
            (plus_one):  Define new function.
            (warn_parm_ptrarray_mismatch): Define new function.
            (warn_parm_array_mismatch):  Define new function.
            (vla_bound_parm_decl): New function.
            * c.opt (-Warray-parameter, -Wvla-parameter): New options.
            * c-pretty-print.c (pp_c_type_qualifier_list): Don't print array type
            qualifiers here...
            (c_pretty_printer::direct_abstract_declarator): ...but instead print
            them in brackets here.  Also print [static].  Strip extraneous
            expressions from VLA bounds.
    
    gcc/c/ChangeLog:
    
            PR c/50584
            * c-decl.c (lookup_last_decl): Define new function.
            (c_decl_attributes): Call it.
            (start_decl): Add argument and use it.
            (finish_decl): Call build_attr_access_from_parms and decl_attributes.
            (get_parm_array_spec): Define new function.
            (push_parm_decl): Call get_parm_array_spec.
            (start_function): Call warn_parm_array_mismatch.  Build attribute
            access and add it to current function.
            * c-parser.c (c_parser_declaration_or_fndef): Diagnose mismatches
            in forms of array parameters.
            * c-tree.h (start_decl): Add argument.
    
    gcc/testsuite/ChangeLog:
    
            PR c/50584
            * gcc.dg/attr-access-read-write-2.c: Adjust text of expected message.
            * c-c++-common/Warray-bounds-6.c: Correct C++ declaration, adjust
            text of expected diagnostics.
            * gcc.dg/Wbuiltin-declaration-mismatch-9.c: Prune expected warning.
            * gcc.dg/Warray-parameter-2.c: New test.
            * gcc.dg/Warray-parameter-3.c: New test.
            * gcc.dg/Warray-parameter-4.c: New test.
            * gcc.dg/Warray-parameter-5.c: New test.
            * gcc.dg/Warray-parameter.c: New test.
            * gcc.dg/Wvla-parameter-2.c: New test.
            * gcc.dg/Wvla-parameter-3.c: New test.
            * gcc.dg/Wvla-parameter.c: New test.
Comment 18 GCC Commits 2020-09-19 23:57:35 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

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

commit r11-3305-gbaad4c48a85a354d2bf1b17e5aff71203c08adea
Author: Martin Sebor <msebor@redhat.com>
Date:   Sat Sep 19 17:37:05 2020 -0600

    Extend -Wstringop-overflow to detect out-of-bounds accesses to array parameters.
    
    gcc/ChangeLog:
    
            PR c/50584
            * builtins.c (warn_for_access): Add argument.  Distinguish between
            reads and writes.
            (check_access): Add argument.  Distinguish between reads and writes.
            (gimple_call_alloc_size): Set range even on failure.
            (gimple_parm_array_size): New function.
            (compute_objsize): Call it.
            (check_memop_access): Pass check_access an additional argument.
            (expand_builtin_memchr, expand_builtin_strcat): Same.
            (expand_builtin_strcpy, expand_builtin_stpcpy_1): Same.
            (expand_builtin_stpncpy, check_strncat_sizes): Same.
            (expand_builtin_strncat, expand_builtin_strncpy): Same.
            (expand_builtin_memcmp): Same.
            * builtins.h (compute_objsize): Declare a new overload.
            (gimple_parm_array_size): Declare.
            (check_access): Add argument.
            * calls.c (append_attrname): Simplify.
            (maybe_warn_rdwr_sizes): Handle internal attribute access.
            * tree-ssa-uninit.c (maybe_warn_pass_by_reference): Avoid adding
            quotes.
    
    gcc/testsuite/ChangeLog:
    
            PR c/50584
            * c-c++-common/Wsizeof-pointer-memaccess1.c: Disable new expected
            warnings.
            * g++.dg/ext/attr-access.C: Update text of expected warnings.
            * gcc.dg/Wstringop-overflow-23.c: Same.
            * gcc.dg/Wstringop-overflow-24.c: Same.
            * gcc.dg/attr-access-none.c: Same.
            * gcc.dg/dfp/composite-type.c: Prune expected warnings.
            * gcc.dg/torture/pr57147-1.c: Add a member to an otherwise empty
            struct to avoid a warning.
            * gcc.dg/torture/pr57147-3.c: Same.
            * gcc.dg/Warray-bounds-30.c: Adjust.
            * gcc.dg/attr-access-none.c: Same.
            * gcc.dg/Wstringop-overflow-40.c: New test.
            * gcc.dg/attr-access-2.c: New test.
Comment 19 GCC Commits 2020-09-19 23:57:41 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:3f9a497d1b0dd9da87908a11b59bf364ad40ddca

commit r11-3306-g3f9a497d1b0dd9da87908a11b59bf364ad40ddca
Author: Martin Sebor <msebor@redhat.com>
Date:   Sat Sep 19 17:47:29 2020 -0600

    Extend -Warray-bounds to detect out-of-bounds accesses to array parameters.
    
    gcc/ChangeLog:
    
            PR middle-end/82608
            PR middle-end/94195
            PR c/50584
            PR middle-end/84051
            * gimple-array-bounds.cc (get_base_decl): New function.
            (get_ref_size): New function.
            (trailing_array): New function.
            (array_bounds_checker::check_array_ref): Call them.  Handle arrays
            declared in function parameters.
            (array_bounds_checker::check_mem_ref):  Same.  Handle references to
            dynamically allocated arrays.
    
    gcc/testsuite/ChangeLog:
    
            PR middle-end/82608
            PR middle-end/94195
            PR c/50584
            PR middle-end/84051
            * c-c++-common/Warray-bounds.c: Adjust.
            * gcc.dg/Wbuiltin-declaration-mismatch-9.c: Adjust.
            * gcc.dg/Warray-bounds-63.c: New test.
            * gcc.dg/Warray-bounds-64.c: New test.
            * gcc.dg/Warray-bounds-65.c: New test.
            * gcc.dg/Warray-bounds-66.c: New test.
            * gcc.dg/Warray-bounds-67.c: New test.
Comment 20 Martin Sebor 2020-09-20 00:03:04 UTC
Done for GCC 11.
Comment 21 GCC Commits 2020-09-21 20:55:33 UTC
The master branch has been updated by Martin Sebor <msebor@gcc.gnu.org>:

https://gcc.gnu.org/g:05193687dde2e5a6337164182a1946b584acfada

commit r11-3333-g05193687dde2e5a6337164182a1946b584acfada
Author: Martin Sebor <msebor@redhat.com>
Date:   Mon Sep 21 14:33:29 2020 -0600

    Avoid incidental failures due to implicit attribute access.
    
    gcc/testsuite/ChangeLog:
    
            PR c/50584
            * gcc.dg/ipa/ipa-sra-1.c: Use a plain pointer for argv instead of array.
            * gcc.dg/ipa/ipa-sra-12.c: Same.
            * gcc.dg/ipa/ipa-sra-13.c: Same.
            * gcc.dg/ipa/ipa-sra-14.c: Same.
            * gcc.dg/ipa/ipa-sra-15.c: Same.