Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow C99 flexible array members in unions and alone in structs like 0-length arrays #84565

Closed
kees opened this issue Mar 8, 2024 · 3 comments · Fixed by #84428
Closed

Allow C99 flexible array members in unions and alone in structs like 0-length arrays #84565

kees opened this issue Mar 8, 2024 · 3 comments · Fixed by #84428
Labels
c99 clang:frontend Language frontend issues, e.g. anything involving "Sema" extension:gnu extension:microsoft

Comments

@kees
Copy link
Contributor

kees commented Mar 8, 2024

GNU and MSVC have extensions where flexible array members (or their equivalent) can be in unions or alone in structs. This is already fully supported in Clang through the 0-sized array ("fake flexible array") extension or when C99 flexible array members have been syntactically obfuscated.

Please explicitly allow these extensions directly for C99 flexible arrays, since they are common code patterns in active use by the Linux kernel (and other projects). Such projects have been using either 0-sized arrays (which is considered deprecated in favor of C99 flexible array members) or via obfuscated syntax, both of which complicate their code bases.

For example, these do not error by default:

union one {
	int a;
	int b[0];
};

union two {
	int a;
	struct {
		struct { } __empty;
		int b[];
	};
};

But this does:

union three {
	int a;
	int b[];
};

Matching request has been made to GCC as well: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53548

@nickdesaulniers

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 8, 2024

@llvm/issue-subscribers-clang-frontend

Author: Kees Cook (kees)

GNU and MSVC have extensions where flexible array members (or their equivalent) can be in unions or alone in structs. This is already fully supported in Clang through the 0-sized array ("fake flexible array") extension or when C99 flexible array members have been syntactically obfuscated.

Please explicitly allow these extensions directly for C99 flexible arrays, since they are common code patterns in active use by the Linux kernel (and other projects). Such projects have been using either 0-sized arrays (which is considered deprecated in favor of C99 flexible array members) or via obfuscated syntax, both of which complicate their code bases.

For example, these do not error by default:

union one {
	int a;
	int b[0];
};

union two {
	int a;
	struct {
		struct { } __empty;
		int b[];
	};
};

But this does:

union three {
	int a;
	int b[];
};

Matching request has been made to GCC as well: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53548

@nickdesaulniers

kees added a commit to kees/llvm-project that referenced this issue Mar 8, 2024
GNU and MSVC have extensions where flexible array members (or their
equivalent) can be in unions or alone in structs. This is already fully
supported in Clang through the 0-sized array ("fake flexible array")
extension or when C99 flexible array members have been syntactically
obfuscated.

Clang needs to explicitly allow these extensions directly for C99
flexible arrays, since they are common code patterns in active use by the
Linux kernel (and other projects). Such projects have been using either
0-sized arrays (which is considered deprecated in favor of C99 flexible
array members) or via obfuscated syntax, both of which complicate their
code bases.

For example, these do not error by default:

union one {
	int a;
	int b[0];
};

union two {
	int a;
	struct {
		struct { } __empty;
		int b[];
	};
};

But this does:

union three {
	int a;
	int b[];
};

Remove the default error diagnostics for this but continue to provide
warnings under Microsoft or GNU extensions checks. This will allow for
a seamless transition for code bases away from 0-sized arrays without
losing existing code patterns. Add explicit checking for the warnings
under various constructions.

Fixes llvm#84565
@shafik
Copy link
Collaborator

shafik commented Mar 9, 2024

CC @AaronBallman @erichkeane

@kees
Copy link
Contributor Author

kees commented Mar 9, 2024

PR #84428 has been proposed to fix this.

kees added a commit that referenced this issue Mar 27, 2024
…84428)

GNU and MSVC have extensions where flexible array members (or their
equivalent) can be in unions or alone in structs. This is already fully
supported in Clang through the 0-sized array ("fake flexible array")
extension or when C99 flexible array members have been syntactically
obfuscated.

Clang needs to explicitly allow these extensions directly for C99
flexible arrays, since they are common code patterns in active use by
the
Linux kernel (and other projects). Such projects have been using either
0-sized arrays (which is considered deprecated in favor of C99 flexible
array members) or via obfuscated syntax, both of which complicate their
code bases.

For example, these do not error by default:
```
union one {
	int a;
	int b[0];
};

union two {
	int a;
	struct {
		struct { } __empty;
		int b[];
	};
};
```
But this does:
```
union three {
	int a;
	int b[];
};
```
Remove the default error diagnostics for this but continue to provide
warnings under Microsoft or GNU extensions checks. This will allow for
a seamless transition for code bases away from 0-sized arrays without
losing existing code patterns. Add explicit checking for the warnings
under various constructions.

Additionally fixes a CodeGen bug with flexible array members in unions
in C++, which was found when adding a testcase for:
```
union { char x[]; } z = {0};
```
which only had Sema tests originally.

Fixes #84565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c99 clang:frontend Language frontend issues, e.g. anything involving "Sema" extension:gnu extension:microsoft
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants