[PATCH] D133574: [C2x] reject type definitions in offsetof

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133574#3816066 <https://reviews.llvm.org/D133574#3816066>, @nathanchance wrote:

> In D133574#3815951 <https://reviews.llvm.org/D133574#3815951>, @aaron.ballman wrote:
>
>> LGTM, thank you! Please don't land until you have some indication from the kernel folks that this won't be super disruptive for them. CC @nickdesaulniers and @nathanchance for awareness.
>
> Thank you a lot for the heads up! I see an in-flight change around this from YingChi on the mailing list:
>
> https://lore.kernel.org/20220925153151.2467884-1-me@inclyc.cn/
>
> It would be good to have that change accepted into a maintainer's tree and on its way to mainline (Linus's tree) and by extension, the stable releases before merging this into main, as we and several other groups are actively testing all Linux branches with LLVM main to catch other regressions and this error would require patching of our CI, which we can definitely do but prefer to avoid because of the maintenance overhead (we try to carry zero patches at all times).

I don't think there's a rush to land this immediately, so we can definitely wait for you to give the all-clear.

> I am still running my set of builds against the kernel with this LLVM change and the aforementioned Linux change but I do see another error:
>
>   drivers/media/platform/nvidia/tegra-vde/v4l2.c:816:49: error: '' cannot be defined in 'offsetof'
>           ctx = kzalloc(offsetof(struct tegra_ctx, ctrls[ARRAY_SIZE(ctrl_cfgs)]),
>                                                          ^
>   include/linux/kernel.h:55:59: note: expanded from macro 'ARRAY_SIZE'
>   #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
>                                                             ^
>   include/linux/compiler.h:240:28: note: expanded from macro '__must_be_array'
>   #define __must_be_array(a)      BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
>                                   ^
>   include/linux/build_bug.h:16:44: note: expanded from macro 'BUILD_BUG_ON_ZERO'
>   #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
>                                              ^
>   1 error generated.
>
> I will report back with any additional problems found.

Oh wow, that's a fun one for a few reasons. 1) Oops, we print `''` instead of mention that it's an anonymous struct, 2) that structure is actually declared in the scope surrounding the `sizeof` so it is technically being defined within `offsetof`, but 3) The standard requirement is "If the specified type defines a new type or if the specified member is a bit-field, the behavior is undefined." where "type" is the first parameter of the `offsetof` macro and this example does not define a new type there.

So I *think* this situation is actually a false positive.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133574/new/

https://reviews.llvm.org/D133574