Skip to content
Snippets Groups Projects
Commit 6141fd55 authored by Jakub Jelinek's avatar Jakub Jelinek Committed by Jakub Jelinek
Browse files

c++: check_flexarray fixes [PR117516]

On the pr117516.C testcase check_flexarrays and its helper functions
have exponential complexity, plus it reports the same bug over and over
again in some cases instead of reporting perhaps other bugs.
The functions want to diagnose flexible array member (and strangely [0]
arrays too) followed by some other non-empty or array members in the same
strcuture, or followed by other non-empty or array members in a containing
structure (any of them), or flexible array members/[0] arrays in structures
with no other non-empty members, or those nested in other structures.
Strangely it doesn't complain if flexible array member is in a structure
used in an array.

As can be seeen on e.g. the flexary41.C test, it keeps reporting the
same bug over and over:
flexary41.C:5:24: error: flexible array member ‘A::b’ not at end of ‘struct A’
flexary41.C:5:24: error: flexible array member ‘A::b’ not at end of ‘struct B’
flexary41.C:5:24: error: flexible array member ‘A::b’ not at end of ‘struct C’
flexary41.C:5:24: error: flexible array member ‘A::b’ not at end of ‘struct D’
flexary41.C:13:39: error: flexible array member ‘E::<unnamed struct>::n’ not at end of ‘struct E’
flexary41.C:18:23: error: flexible array member ‘H::t’ not at end of ‘struct K’
flexary41.C:25:36: note: next member ‘int K::ab’ declared here
flexary41.C:25:8: note: in the definition of ‘struct K’
The bug that A::b is followed by A::c is one bug reported 4 times, while it
doesn't report the other bugs, that B::e flexarray is followed by B::f
and that C::h flexarray is followed by C::i.
That is because it always walks all the structures/unions of all the members
and just finds the first flexarray in there.

Now, this has horrible complexity plus it doesn't seem really useful to
users.  So, for cases where a flexible array member is followed by a
non-empty other member in the same structure, the following patch just
reports it once when finalizing that structure, and otherwise just recurses
in structures solely into the last member, so that it can report cases like
struct X { int a; int b[]; };
struct Y { X c; int d; };
or
struct Z { X c; };
i.e. correct use of flexarray in X but following it by another member in Y
or just nesting it (the former is error, the latter pedwarn as before).
By only looking at the last member for structures we get rid of the complexity.

Note, the patch doesn't do anything about unions, I think we still could
spend a lot of time compiling.
struct S { char s; };
union U0 { S a, b; };
union U1 { union U0 a, b; };
union U2 { union U1 a, b; };
...
union U32 { union U31 a, b; };
struct T { union U32 a; int b; };
Not really sure what we could do about that, all the elements are "last"
(but admittedly I haven't studied in detail how the original code worked
in union, there is fmem->after[pun] where pun is whether it is somewhere
inside of a union).  Perhaps in a hash table marking unions which don't have
any flexarrays at the end, nested or not, so that we don't walk them again?
Plus if we find some with flexarray at the end, maybe there is no point
to look other union members?  In any case, I think that is less severe,
because people usually don't nest unions deeply.

2025-02-01  Jakub Jelinek  <jakub@redhat.com>

	PR c++/117516
	* class.cc (field_nonempty_p): Formatting fixes.  Use
	integer_zerop instead of tree_int_cst_equal with size_zero_node.
	(struct flexmems_t): Change type of first member from tree to bool.
	(find_flexarrays): Add nested_p argument.  Change pun argument type
	from tree to bool, adjust uses.  Formatting fixes.  If BASE_P or
	NESTED_P and T is RECORD_TYPE, start looking only at the last
	non-empty or array FIELD_DECL.  Adjust recursive call, set first
	if it was a nested call and found an array.
	(diagnose_invalid_flexarray, diagnose_flexarrays, check_flexarrays):
	Formatting fixes.

	* g++.dg/ext/flexary9.C: Expect different wording of one of the
	warnings and at a different line.
	* g++.dg/ext/flexary19.C: Likewise.
	* g++.dg/ext/flexary42.C: New test.
	* g++.dg/other/pr117516.C: New test.
parent a9172b10
No related branches found
No related tags found
No related merge requests found
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment