[cfe-dev] [RFC] Sort out and correct pragma align/pack stack effect

[Xiangling Liao via cfe-dev]

Hi,



Recently we are trying to implement AIX pragma align/pack stack effect, but
noticed current implementation model in Clang has some limitations.



That results from mixing `#pragma pack` and `#pragma align` by using only an
unsigned number to represent both so that they could override each other.



Some examples we have been aware of are:



1. The current diagnostic for pragma pack does not produce a valid
diagnostic when align pragma is involved. For example,



*$ cat test_diag.c*

*#pragma align = natural *



*$ clang test_diag.c -S *



*test_diag.c:1:9: warning: unterminated '#pragma pack (push, ...)' at end
of file [-Wpragma-pack]*

*#pragma align = natural *

*        ^*

*test_diag.c:1:9: note: did you intend to use '#pragma pack (pop)' instead
of '#pragma pack()'?*

*1 warning generated.*



The diagnostic is misleading by indicating some `*'#pragma pack *` is
unterminated. This is caused by not differentiating between align pragma
and pack pragma in clang’s current align and pack stack model.





2*.*There exists platforms like AIX that cannot have pragma align/pack
stack effect simply represented by an alignment value. The stack effect
between align and pack pragmas on AIX is different.



(1)They don’t override but create combinations with each other sometimes.



Eg.

#pragma align(natural)

#pragma pack(2)



In current Clang’s implementation we get pack(2) in effect here without
knowing align mode information above it;



However, XL on AIX would need to align objects to natural alignment
first and then reduced by pack(2).



(2)Additional complication comes in with `#pragma align(reset)` and
`#pragma pack(pop)`.



Eg.

#pragma pack(2)

#pragma align = packed

#pragma pack(pop)



In current Clang’s implementation we get pack(2) in effect here.



However, for XL on AIX, `#pragma pack(pop)` won’t be able to pop the
stack, because `#pragma align` sets a baseline, and there is no pragma
pack applied to it. Without distinguishing pragma align and pramgma
pack, the `#pragma align = packed` would be popped out mistakenly for
AIX.



*3.* Features added in later like PCH only record pragma alignment number.
In platforms like AIX, baseline align mode information will be lost.




Based on current limitations, we are proposing a new model by splitting up
the alignment value into an align mode and a pack number so that we would
be able to know what is the combined effect of align and pack in our stack.



The proposed patch has been posted here as a reference:

https://reviews.llvm.org/D87702



This patch implements the proposed new model,  implements AIX pragma
align/stack effect based on that, and updates pragma align/pack related PCH
handling process.



But there are several things I would like to point out particularly:



(1) In `clang/include/clang/Sema/Sema.h`,

    bool operator==(const AlignPackInfo &Info) const {

      return std::tie(AlignMode, PackNumber, PackAttr, AIXStack) ==

             std::tie(Info.AlignMode, Info.PackNumber, Info.PackAttr,

                      Info.AIXStack);

}



We are trying to pursue a stricter “equal” definition of if two pragma are
equal. That would change some previous behavior, for example:



*a) *

*#pragma align(packed)*

*#pragma pack(1) // AlignMode = Packed,PackNumber = 1,IsPackAttr*



*b)*

*#pragma align(packed) //AlignMode = Packed,PackNumber = 1,  *

*                        IsAlignAttr*





Since we only care about unsigned numbers previously, these two pragma
settings would be treated as equal.



But now, they would not. So on platforms where the previous result is
required, we would need to compare the `PackNumber` of `AlignPackInfo`
objects instead of comparing themselves directly.



(2) In the first diagnostic limitation mentioned above, we handle it by
leaving a FIXME in the current patch in order to avoid scope creep. But we
believe the new `AlignPackInfo` struct provides a good foundation to
address this issue in the future.



Please let me know if there are any questions about our current proposal
and design. Your feedback is appreciated.



Regards,

Xiangling Liao
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20201127/4e797e25/attachment.html>