Win64 and abi_prefix.hpp/abi_suffix.hpp warnings
I know there have been previous discussions about the warnings emitted from abi_prefix.hpp and abi_suffix.hpp on Win64. Was there ever any resolution? To recap, for every header which uses the ABI bookends, the compiler emits four warnings: abi_prefix.hpp(19) : warning C4103: 'abi_prefix.hpp' : alignment changed after including header, may be due to missing #pragma pack(pop) exceptions.hpp(22) : warning C4103: 'exceptions.hpp' : alignment changed after including header, may be due to missing #pragma pack(pop) abi_suffix.hpp(20) : warning C4103: 'abi_suffix.hpp' : alignment changed after including header, may be due to missing #pragma pack(pop) exceptions.hpp(108) : warning C4103: 'exceptions.hpp' : alignment changed after including header, may be due to missing #pragma pack(pop) I'm unclear on the rationale for forcing 8-byte alignment in config/abi/msvc_(pre|suf)fix.hpp: 8-byte alignment is the compiler default. Is there concern over Boost headers being included in a context where the alignment has been otherwise artificially altered? Assuming that's the case, I would like to propose adding a warning suppression to config/warning_disable.hpp to silence this particular warning. Here's a diff of what I'm proposing: @@ -43,5 +43,14 @@ // std library function is encountered. # pragma warning(disable:1786) #endif +#if defined(_MSC_VER) && defined(_M_X64) + // warning: 'file': alignment changed after including header, may + // be due to missing #pragma pack(pop) + // + // Caused by MSVC-specific abi_prefix and abi_suffix bookends which + // twiddle the default structure alignment via #pragma pack(); this + // results in a warning only on x86_64 targets. +# pragma warning(disable:4103) +#endif #endif // BOOST_CONFIG_WARNING_DISABLE_HPP I'd like to get something in for Boost 1.37 as this warning is responsible for a significant amount of spam in our build logs. Comments? -Chris
From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Chris Newbold Sent: Thursday, September 25, 2008 10:35 AM
Assuming that's the case, I would like to propose adding a warning suppression to config/warning_disable.hpp to silence this particular warning. Here's a diff of what I'm proposing:
@@ -43,5 +43,14 @@ // std library function is encountered. # pragma warning(disable:1786) #endif +#if defined(_MSC_VER) && defined(_M_X64) + // warning: 'file': alignment changed after including header, may + // be due to missing #pragma pack(pop) + // + // Caused by MSVC-specific abi_prefix and abi_suffix bookends which + // twiddle the default structure alignment via #pragma pack(); this + // results in a warning only on x86_64 targets. +# pragma warning(disable:4103) +#endif
#endif // BOOST_CONFIG_WARNING_DISABLE_HPP
Of course that doesn't work, because warning_disable.hpp isn't universally included. My bad. Here's an alternate proposal that does work: add a warning suppression to config/abi/msvc_prefix.hpp: @@ -3,6 +3,18 @@ // Boost Software License, Version 1.0. (See accompanying file // LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +#if defined(_M_X64) + // warning: 'file': alignment changed after including header, may + // be due to missing #pragma pack(pop) + // + // Changing the default structure alignment across header file + // boundaries causes a warning on the x86_64 target. Unfortunately, + // there is no way to do a push-pop on the state of this warning + // since popping it back to the 'on' state results in a warning as + // the compiler exits the suffix file +# pragma warning(disable:4103) +#endif + #pragma pack(push,8) Does this seem reasonable? -Chris
Chris Newbold wrote:
Here's an alternate proposal that does work: add a warning suppression to config/abi/msvc_prefix.hpp:
@@ -3,6 +3,18 @@ // Boost Software License, Version 1.0. (See accompanying file // LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
+#if defined(_M_X64) + // warning: 'file': alignment changed after including header, may + // be due to missing #pragma pack(pop) + // + // Changing the default structure alignment across header file + // boundaries causes a warning on the x86_64 target. Unfortunately, + // there is no way to do a push-pop on the state of this warning + // since popping it back to the 'on' state results in a warning as + // the compiler exits the suffix file +# pragma warning(disable:4103) +#endif + #pragma pack(push,8)
Does this seem reasonable?
I'd say msvc_suffix should reenable the warning with #pragma warning(default:4103). Sebastian
From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Sebastian Redl
Chris Newbold wrote:
Here's an alternate proposal that does work: add a warning suppression to config/abi/msvc_prefix.hpp:
@@ -3,6 +3,18 @@ // Boost Software License, Version 1.0. (See accompanying file // LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
+#if defined(_M_X64) + // warning: 'file': alignment changed after including header, may + // be due to missing #pragma pack(pop) + // + // Changing the default structure alignment across header file + // boundaries causes a warning on the x86_64 target. Unfortunately, + // there is no way to do a push-pop on the state of this warning + // since popping it back to the 'on' state results in a warning as + // the compiler exits the suffix file +# pragma warning(disable:4103) +#endif + #pragma pack(push,8)
Does this seem reasonable?
I'd say msvc_suffix should reenable the warning with #pragma warning(default:4103).
Unfortunately that only pushes the problem one level higher. It seems that MSVC is not particularly intelligent about how and when to issue this warning. The only way I've been able to successfully suppress it is to permanently disable it in msvc_prefix.hpp. -Chris
From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Chris Newbold Sent: Thursday, September 25, 2008 10:35 AM
I'm unclear on the rationale for forcing 8-byte alignment in config/abi/msvc_(pre|suf)fix.hpp: 8-byte alignment is the compiler default. Is there concern over Boost headers being included in a context where the alignment has been otherwise artificially altered?
I'm starting to question the wisdom of unconditionally forcing 8-byte alignment in the MSVC ABI headers, particularly given that this happens on two architectures (x86 and x86_64) that have different pointer widths. I can see some merit to enforcing a uniform alignment for Boost to avoid ABI issues due to users' project settings, etc., but I think that we need to account for the fact that different architectures have different default alignments. On x86_64, the default alignment for MSVC is 16 bytes. However, changing msvc_prefix.hpp to look like... #if defined(_M_X64) # pragma pack(push,16) #else # pragma pack(push,8) #endif ... is only an incomplete solution. It suppresses the warnings in the default case where the user has not already changed the alignment. If, for whatever reason, the user has changed the default alignment they're going to start getting the warnings again. I think the only solution is to combine this change (so that Boost uses the "true" default alignment on x86_64) with the big-hammer disable-the-warning-for-good change. -Chris
Chris Newbold wrote:
I'm unclear on the rationale for forcing 8-byte alignment in config/abi/msvc_(pre|suf)fix.hpp: 8-byte alignment is the compiler default. Is there concern over Boost headers being included in a context where the alignment has been otherwise artificially altered?
Yes: it's to ensure that the included header is binary compatible with the boost binaries, which are all built with default alignment settings. Obviously for header only libraries, including the abi_prefix/suffix headers is unnecessary.
I'm starting to question the wisdom of unconditionally forcing 8-byte alignment in the MSVC ABI headers, particularly given that this happens on two architectures (x86 and x86_64) that have different pointer widths.
I can see some merit to enforcing a uniform alignment for Boost to avoid ABI issues due to users' project settings, etc., but I think that we need to account for the fact that different architectures have different default alignments. On x86_64, the default alignment for MSVC is 16 bytes.
However, changing msvc_prefix.hpp to look like...
#if defined(_M_X64) # pragma pack(push,16) #else # pragma pack(push,8) #endif
... is only an incomplete solution. It suppresses the warnings in the default case where the user has not already changed the alignment. If, for whatever reason, the user has changed the default alignment they're going to start getting the warnings again.
Sigh, yes indeed. But we should definitely make that change IMO.
I think the only solution is to combine this change (so that Boost uses the "true" default alignment on x86_64) with the big-hammer disable-the-warning-for-good change.
I'm less keen on that as it may suppress warnings about genuine bugs, but don't know what else to suggest :-( Unless there are any violent objections, I'm going to commit the x64 fix soon, but what do folks feel about suppressing the warnings globally as well? Thanks for raising this, John.
Hi John ! On Friday 26 September 2008 10:40:32 John Maddock wrote:
Chris Newbold wrote:
I'm unclear on the rationale for forcing 8-byte alignment in config/abi/msvc_(pre|suf)fix.hpp: 8-byte alignment is the compiler default. Is there concern over Boost headers being included in a context where the alignment has been otherwise artificially altered?
Yes: it's to ensure that the included header is binary compatible with the boost binaries, which are all built with default alignment settings.
Could you please add the sentence as comment to prefix.hpp ? This would make things much clearer for the reader ;-))
Unless there are any violent objections, I'm going to commit the x64 fix soon, but what do folks feel about suppressing the warnings globally as well?
It seems the x64 fix is the right way. Simply disabling the warning is a bad thing. But disabling and reenabling the warning in every place is a tedious job. Please find my try at this attached. I think if someone is messing with the aligment she will want to know about the forced alignment in Boost libraries. I'm +1 for the x64 fix and +1 for "don't suppress the warning". Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !
Juergen Hunold wrote:
Could you please add the sentence as comment to prefix.hpp ? This would make things much clearer for the reader ;-))
Done, also committed the fix to trunk.
It seems the x64 fix is the right way. Simply disabling the warning is a bad thing. But disabling and reenabling the warning in every place is a tedious job. Please find my try at this attached.
Can you file Trac-tickets for these and assign to the appropriate library authors? Many thanks, John.
Hi John ! On Wednesday 01 October 2008 13:32:19 John Maddock wrote:
Juergen Hunold wrote:
Could you please add the sentence as comment to prefix.hpp ? This would make things much clearer for the reader ;-))
Done, also committed the fix to trunk.
Thanks a lot !
It seems the x64 fix is the right way. Simply disabling the warning is a bad thing. But disabling and reenabling the warning in every place is a tedious job. Please find my try at this attached.
Can you file Trac-tickets for these and assign to the appropriate library authors?
I thought a little bit about it and think that disabling the warnings with #pragmas is the wrong solution. Tinkering with the alignment with msvc is dangerous and can (well, will ;-))) lead to mysterious bugs (been there, done that :-(( ) so I think that anyone who changes the alignment from the default should be made aware that those libs are using a "fixed" alignment. She can then decide to change the aligment in abi_prefix.hpp and compile a customized version or disabled the warning on a per-project base. And our current complains are just about the "wrong" alignment on x64 which has been fixed. I think disabling the warnings is simply over-engineering in this case. And we don't have tests for it... Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !
From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of John Maddock Sent: Friday, September 26, 2008 4:41 AM
Unless there are any violent objections, I'm going to commit the x64 fix soon, but what do folks feel about suppressing the warnings globally as well?
Getting the x86_64 fix in will basically address the spam we're seeing from the ABI headers and seems like the obvious thing to do. -Chris
participants (4)
-
Chris Newbold -
John Maddock -
Juergen Hunold -
Sebastian Redl