r309386 - Recommit r308327 3rd time: Add a warning for missing

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 28 16:56:49 PDT 2017


On Fri, Jul 28, 2017 at 7:41 AM, Alex Lorenz via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: arphaman
> Date: Fri Jul 28 07:41:21 2017
> New Revision: 309386
>
> URL: http://llvm.org/viewvc/llvm-project?rev=309386&view=rev
> Log:
> Recommit r308327 3rd time: Add a warning for missing
> '#pragma pack (pop)' and suspicious uses of '#pragma pack' in included files
>
> The second recommit (r309106) was reverted because the "non-default #pragma
> pack value chages the alignment of struct or union members in the included file"
> warning proved to be too aggressive for external projects like Chromium
> (https://bugs.chromium.org/p/chromium/issues/detail?id=749197). This recommit
> makes the problematic warning a non-default one, and gives it the
> -Wpragma-pack-suspicious-include warning option.
>
> The first recommit (r308441) caused a "non-default #pragma pack value might
> change the alignment of struct or union members in the included file" warning
> in LLVM itself. This recommit tweaks the added warning to avoid warnings for
> #includes that don't have any records that are affected by the non-default
> alignment. This tweak avoids the previously emitted warning in LLVM.
>
> Original message:
>
> This commit adds a new -Wpragma-pack warning. It warns in the following cases:
>
> - When a translation unit is missing terminating #pragma pack (pop) directives.
> - When entering an included file if the current alignment value as determined
>   by '#pragma pack' directives is different from the default alignment value.
> - When leaving an included file that changed the state of the current alignment
>   value.
>
> rdar://10184173
>
> Differential Revision: https://reviews.llvm.org/D35484

We did get warnings with this version too, but this time it looks like
a real bug :-)

C:\b\c\builder\ClangToTWin_dll_\src\third_party\usrsctp\usrsctplib\usrsctplib\netinet\sctp.h(51,9):
 error: unterminated '#pragma pack (push, ...)' at end of file
[-Werror,-Wpragma-pack]

The interesting thing is that the file does reset the alignment, but
it does it with a "#pragma pack()" which doesn't pop the stack, but
just resets to the default value.

It might be worth tweaking the warning to give a better message for
this. A "#pragma pack(push, ..)" followed by "#pragma pack()" seems
pretty dodgy since there's no point in pushing if the code isn't going
to pop.


More information about the cfe-commits mailing list