Attention is currently required from: Arthur Heymans, Cliff Huang, Elyes Haouas, Hung-Te Lin, Jérémy Compostella, Lance Zhao, Tim Wawrzynczak, Yidi Lin, Yu-Ping Wu.
Julius Werner has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84208?usp=email )
Change subject: soc/mt6366: Work around GCC LTO build ......................................................................
Patch Set 8:
(1 comment)
File src/soc/mediatek/mt8186/mt6366.c:
https://review.coreboot.org/c/coreboot/+/84208/comment/1994176a_7bcac456?usp... : PS8, Line 10: #define NO_BUILDTIME_ASSERT
Code like […]
Sure, but is doing this by file more robust? Maybe I'm missing something here but to me it sounds like whether GCC bugs out on a particular assert() statement or not seems almost entirely random (within some constraints, e.g. the value needs to depend on passed-in function parameters). There's no reason why we should be saying that the mt6366.c file is more likely to see this issue than any other files. As far as I understand, all Arthur has been doing here is to try it out, see which files had errors and then add the `#define` to those files... and the next time anyone changes the code and the errors move around to other assert statements, we'll have to just try it out and add the `#define` to those files as well.
So it seems to me that until GCC fixes this bug, our only choice (other than disabling build-time assertions completely for the GCC+LTO combo, which is also worth considering) is to keep playing whack-a-mole with this bug whenever it crops up somewhere. The only question is whether we want to do that at the level of whole files or at the level of single statements. Since as far as I understand there should be no reason to assume that just because one assert statement in a file had this issue the other statements in that file are somehow more likely to also have it (compared to the rest of the code base), I don't think it makes sense to aggregate this workaround on a per-file basis.
(The syntax can of course be bikeshed, e.g. if you want it to look less jarring it could be `assert()` and `assertX()` or something like that. I don't really care about the naming.)