Skip to content

Conversation

@lyakh
Copy link
Contributor

@lyakh lyakh commented Jul 11, 2025

Some (broken) compilers fail to handle

struct x {
	union {
		type_a *a;
		type_b *b;
	};
};

correctly. Replace that with an equivalent but simpler

union x {
	type_a *a;
	type_b *b;
};

Sample build failure log: https://sof-ci.01.org/sofpr/PR10113/build13856/build/firmware_community/tgl.log

Some (broken) compilers fail to handle

struct x {
	union {
		type_a *a;
		type_b *b;
	};
};

correctly. Replace that with an equivalent but simpler

union x {
	type_a *a;
	type_b *b;
};

Signed-off-by: Guennadi Liakhovetski <[email protected]>
@sonarqubecloud
Copy link

@lyakh
Copy link
Contributor Author

lyakh commented Jul 11, 2025

As explained in #93020 this bug can also be reproduced by building pure Zephyr for intel_adsp/cavs25 using the same Cadence toolchain

@henrikbrixandersen
Copy link
Member

Some (broken) compilers fail to handle

struct x {
	union {
		type_a *a;
		type_b *b;
	};
};

correctly. Replace that with an equivalent but simpler

union x {
	type_a *a;
	type_b *b;
};

This removes the encapsulation, though. With a struct, more members can be added later without breaking an API.

@cfriedt
Copy link
Member

cfriedt commented Jul 11, 2025

@lyakh - it looks like the code is being compiled with -std=c99. However, anonymous unions were not added to ISO C until C11.

Maybe the compiler isn't actually broken.

Can you try building with the appropriate settings?

With Zephyr, you should be able to use CONFIG_STD_C11=y.

@lyakh
Copy link
Contributor Author

lyakh commented Jul 14, 2025

Some (broken) compilers fail to handle

struct x {
	union {
		type_a *a;
		type_b *b;
	};
};

correctly. Replace that with an equivalent but simpler

union x {
	type_a *a;
	type_b *b;
};

This removes the encapsulation, though. With a struct, more members can be added later without breaking an API.

@henrikbrixandersen correct, and it does look like a compiler bug. And we don't know what exactly it failed to digest - an anonymous union within a structure (which would be a bit strange, because there are probably more examples of that both in Zephyr and SOF, for which that compiler is routinely used), or such a union as a single element of a structure, which might indeed be rather rare, so maybe it's exactly this that tripped the compiler. So, my "hope" is, that if we do need to add more fields in the future, we restore the structure, but then the union won't be the only member there, so maybe that will satisfy the compiler. Or maybe by then we'll update the compiler.

@lyakh
Copy link
Contributor Author

lyakh commented Jul 14, 2025

@lyakh - it looks like the code is being compiled with -std=c99. However, anonymous unions were not added to ISO C until C11.

Maybe the compiler isn't actually broken.

Can you try building with the appropriate settings?

With Zephyr, you should be able to use CONFIG_STD_C11=y.

@cfriedt have just tried that:

xt-xcc ERROR:  -std=c11 and -std=c++11 require -clang and Xtensa C Library
ninja: build stopped: subcommand failed.

and we do have multple locations in SOF with anonymous unions, e.g. https://github.com/thesofproject/sof/blob/97fbd9938f72a5cdabc76fddc626c24c2858e6cd/src/audio/mux/mux.h#L169 and it does get used in those builds - I've checked it by adding a bug there, and (without the added bug) that union builds just fine

@lyakh
Copy link
Contributor Author

lyakh commented Jul 21, 2025

After the revert #93158 this isn't needed. @nashif @henrikbrixandersen @cfriedt I suppose we can close this as long as we don't plan to re-instate those changes.

@lyakh lyakh closed this Jul 21, 2025
@lyakh lyakh deleted the init branch July 21, 2025 14:56
@tbursztyka
Copy link
Contributor

@lyakh interesting finding. Weird for a gcc based compiler. I will revisit the original PR, but it might need to get a non anonymized union instead then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants