Gcc6 fixes #62

ghost · 2016-08-26T07:04:08Z

With these fixes, it's possible to build (and boot!) a full op-build master-next build with GCC6.

None of these changes should be controversial, and GCC6 did pick up actual problems such as:

not specifying what C++ version to build (and there are changes that matter)
returning something from a void method
invalid __sync ops (can't use a bool)
relying on the address of a passed-by-reference object able to be NULL (which is not at all what anyone would expect, and it appears the compiler may have been okay to assume it could never be null)

This change is

shenki · 2016-08-29T05:49:43Z

Nice work!

dcrowell77 · 2016-08-30T14:39:27Z

src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.H

@@ -227,6 +227,13 @@ namespace getAttrData
        uint8_t              iv_systemType;
        uint8_t              iv_systemType_ext;
        uint8_t              iv_dataVersion;
+    public:


Surprised these changes were required, happen to remember what failed before you changed them?

Yes, without this patch I got (as the commit message for 99e4065 states): " error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]"

Which was due to alternately treating it as u32, u8[] and MBvpdVMKeyword. I didn't check the asm, but I would assume that the compiler is smart enough to effectively compile down the code to nothing.

dcrowell77 · 2016-08-30T14:44:49Z

I'll get this pulled into our internal repo for review and testing.

Seeing as no C++ dialect was previously selected, GCC < 6 defaulted to GNU++98 as the standard C++ mode. However... it seems that C++03 is a better match for HostBoot code (confirmed by Patrick Williams in open-power#62 (comment) ) Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

ghost · 2016-08-31T03:27:28Z

Note, for gnu++03, there's one extra build error that needs fixing: .//mvpd_accessors/getMBvpdAttr.C:395:36: error: in C++98 ‘l_vmVersionBuf’ must be initialized by constructor, not by ‘{...}’

New patch series fixes that issue too. Seems to boot on firestone!

zane131 · 2016-09-01T15:33:23Z

src/usr/diag/prdf/common/framework/register/prdfErrorRegister.C

@@ -120,8 +120,6 @@ ErrorRegister::ErrorRegister( SCAN_COMM_REGISTER_CLASS & r, ResolutionMap & rm,
    ErrorRegisterType(), scr(r), scr_rc(SUCCESS), rMap(rm),
    xNoErrorOnZeroScr(false), xScrId(scrId)
 {
-    PRDF_ASSERT( &r  != NULL );


I understand &r is ambiguous and not allowed anymore, however, I'd still like to have some sort of protection against something like this:

SCAN_COMM_REGISTER_CLASS * p = NULL; ErrorRegister( *p, ...);

Any suggestions?

Hrm.... so dereferencing a null pointer is undefined behavior.

From the standard:

A reference shall be initialized to refer to a valid object or function. [ Note: in particular, a null reference cannot exist in a well-defined program, because the only way to create such a reference would be to bind it to the “object” obtained by dereferencing a null pointer, which causes undefined behavior. As described in 9.6, a reference cannot be bound directly to a bit-field. — end note ]

So if there isn't a suitable check that p != NULL then I guess either the compiler or a static analysis tool should warn/error.... as it seems the check of the address of the reference would itself be undefined behavior for a null pointer.

While hostboot is not yet ready for GCC6, although there is a pull request for it: open-power/hostboot#62 , other platforms that don't build hostboot (Firenze, mambo simulator targets) can move to GCC6 now. Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

ghost · 2016-09-20T04:52:45Z

Any news?

ghost · 2016-10-21T03:13:47Z

ping?

Seeing as no C++ dialect was previously selected, GCC < 6 defaulted to GNU++98 as the standard C++ mode. However... it seems that C++03 is a better match for HostBoot code (confirmed by Patrick Williams in open-power#62 (comment) ) Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

ghost · 2016-11-03T03:41:39Z

Rebased and also addreses #69

We bring in P8 Hostboot patches from open-power/hostboot#62 as we wait for Hostboot development team to merge them. OCC patch is from open-power/occ#15 . It simply sets the language to gnu89 with -std=gnu89, which is the default prior to GCC5 (see https://gcc.gnu.org/gcc-5/porting_to.html ) Fixes: open-power/occ#8 Fixes: open-power/hostboot#69 Fixes: open-power/hostboot#61 Fixes: open-power#355 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

ghost · 2016-11-03T05:28:18Z

We have to detect the ability of the host g++ to accept the -std=gnu++03 as RHEL6.4 contains some kind of stone age GCC.

We bring in P8 Hostboot patches from open-power/hostboot#62 as we wait for Hostboot development team to merge them. OCC patch is from open-power/occ#15 . It simply sets the language to gnu89 with -std=gnu89, which is the default prior to GCC5 (see https://gcc.gnu.org/gcc-5/porting_to.html ) Fixes: open-power/occ#8 Fixes: open-power/hostboot#69 Fixes: open-power/hostboot#61 Fixes: open-power#355 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

ghost · 2016-11-03T07:45:30Z

Note that without this patch I cannot build hostboot on my laptop (Fedora 24)

Seeing as no C++ dialect was previously selected, GCC < 6 defaulted to GNU++98 as the standard C++ mode. However... it seems that C++03 is a better match for HostBoot code (confirmed by Patrick Williams in open-power#62 (comment) ) Change-Id: I6661b5b61319c85c2a90926beb6e2662e96dcf06 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

… 'void' Change-Id: I9dc8b698fec95488e209cbc40b928769a26b6de6 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

…ules Change-Id: I63accd3e881c941736ece4b4498c2c9d06ff8761 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

GCC6 throws the following error: operand type ‘bool*’ is incompatible with argument 1 of ‘__sync_fetch_and_and’ GCC documents that bool is invalid for __sync builtins over at: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html#g_t_005f_005fsync-Builtins " GCC allows any scalar type that is 1, 2, 4 or 8 bytes in size other than the C type _Bool or the C++ type bool" Change-Id: Iab6415348cb19f590921d8ccc5349867fa57a42d Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

…NULL PRDF_ASSERT( &r != NULL ); Change-Id: I2d60075f9e2232512efe45a5c6aa5563f3a565f5 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

So, it turns out that relying on the address of something passed by reference being able to be NULL isn't exactly a good idea, or remotely obvious code. So, instead, do the sane thing and pass a pointer and check it. ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H: In constructor ‘PRDF::AttnTypeRegister::AttnTypeRegister(PRDF::SCAN_COMM_REGISTE R_CLASS&, PRDF::SCAN_COMM_REGISTER_CLASS&, PRDF::SCAN_COMM_REGISTER_CLASS&, PRDF ::SCAN_COMM_REGISTER_CLASS&)’: ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H:4 42:21: error: the compiler can assume that the address of ‘i_check’ will never b e NULL [-Werror=address] iv_check( NULL == &i_check ? &cv_null : &i_check), ~~^~~~~~~~~~~ ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H:4 43:21: error: the compiler can assume that the address of ‘i_recov’ will never b e NULL [-Werror=address] iv_recov( NULL == &i_recov ? &cv_null : &i_recov), ~~^~~~~~~~~~~ ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H:4 44:22: error: the compiler can assume that the address of ‘i_special’ will never be NULL [-Werror=address] iv_special(NULL == &i_special ? &cv_null : &i_special), ~~^~~~~~~~~~~~~ ../../../../src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H:4 45:22: error: the compiler can assume that the address of ‘i_proccs’ will never be NULL [-Werror=address] iv_proccs( NULL == &i_proccs ? &cv_null : &i_proccs), ~~^~~~~~~~~~~~ Change-Id: Iecd8636da67aac24f64f73fd82b1f7ccbfced900 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

fix GCC6 thrown error Change-Id: I9aa508843f54c99ebb59822c39590811423ad0b1 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

Seeing as the ancient GCC on RHEL6 doesn't actually support -std=gnu++03 we have to go through some hoops to detect it (we use the same magic Make as we use in skiboot to do the same) Change-Id: I338560ae2ebdac842c8055c07519d542564c3923 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

We bring in P8 Hostboot patches from open-power/hostboot#62 as we wait for Hostboot development team to merge them. OCC patch is from open-power/occ#15 . It simply sets the language to gnu89 with -std=gnu89, which is the default prior to GCC5 (see https://gcc.gnu.org/gcc-5/porting_to.html ) Fixes: open-power/occ#8 Fixes: open-power/hostboot#69 Fixes: open-power/hostboot#61 Fixes: open-power#355 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

We bring in P8 Hostboot patches from open-power/hostboot#62 as we wait for Hostboot development team to merge them. Fixes: open-power/hostboot#69 Fixes: open-power/hostboot#61 Fixes: open-power#355 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

(everything but Witherspoon) We bring in P8 Hostboot patches from open-power/hostboot#62 as we wait for Hostboot development team to merge them. Fixes: open-power/hostboot#69 Fixes: open-power/hostboot#61 Fixes: open-power#355 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

Seeing as no C++ dialect was previously selected, GCC < 6 defaulted to GNU++98 as the standard C++ mode. However... it seems that C++03 is a better match for HostBoot code (confirmed by Patrick Williams in open-power#62 (comment) ) Change-Id: I6661b5b61319c85c2a90926beb6e2662e96dcf06 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

dcrowell77 · 2019-04-12T21:54:45Z

Is this PR still relevant? I see a fork used it but I don't know what fspbld is.

ghost · 2019-07-04T04:36:34Z

Is this PR still relevant? I see a fork used it but I don't know what fspbld is.

No longer relevant, we have these bits upstream (somehow, I forget how), and there's now the GCC8 patches

Seeing as no C++ dialect was previously selected, GCC < 6 defaulted to GNU++98 as the standard C++ mode. However... it seems that C++03 is a better match for HostBoot code (confirmed by Patrick Williams in open-power#62 (comment) ) Change-Id: I6661b5b61319c85c2a90926beb6e2662e96dcf06 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

ghost mentioned this pull request Aug 26, 2016

Doesn't build with GCC5 #61

Closed

ghost mentioned this pull request Aug 30, 2016

Move to GCC 6 open-power/op-build#355

Closed

dcrowell77 reviewed Aug 30, 2016
View reviewed changes

ghost force-pushed the gcc6-fixes branch from 99cad57 to a98d682 Compare August 31, 2016 03:27

zane131 reviewed Sep 1, 2016
View reviewed changes

ghost mentioned this pull request Sep 6, 2016

Switch to GCC6 for Firenze and Mambo platforms open-power/op-build#622

Merged

ghost force-pushed the gcc6-fixes branch from a98d682 to 41104a9 Compare November 3, 2016 03:41

ghost mentioned this pull request Nov 3, 2016

Use GCC6 for all platforms open-power/op-build#721

Merged

ghost force-pushed the gcc6-fixes branch from 41104a9 to 54b6416 Compare November 3, 2016 05:24

stewartsmith added 6 commits November 4, 2016 09:30

fix build error: return-statement with a value, in function returning…

118c19b

… 'void' Change-Id: I9dc8b698fec95488e209cbc40b928769a26b6de6 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

error: dereferencing type-punned pointer will break strict-aliasing r…

d19728d

…ules Change-Id: I63accd3e881c941736ece4b4498c2c9d06ff8761 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

error: the compiler can assume that the address of ‘r’ will never be …

17f5649

…NULL PRDF_ASSERT( &r != NULL ); Change-Id: I2d60075f9e2232512efe45a5c6aa5563f3a565f5 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

stewartsmith added 2 commits November 4, 2016 09:31

error: in C++98 ‘l_vmVersionBuf’ must be initialized by constructor

2b077be

fix GCC6 thrown error Change-Id: I9aa508843f54c99ebb59822c39590811423ad0b1 Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

ghost force-pushed the gcc6-fixes branch from 54b6416 to 61b675f Compare November 3, 2016 22:39

ghost closed this Jul 4, 2019

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gcc6 fixes #62

Gcc6 fixes #62

ghost commented Aug 26, 2016 •

edited by williamspatrick

shenki commented Aug 29, 2016

dcrowell77 Aug 30, 2016

ghost Aug 31, 2016

dcrowell77 commented Aug 30, 2016

ghost commented Aug 31, 2016

zane131 Sep 1, 2016

ghost Sep 6, 2016

ghost commented Sep 20, 2016

ghost commented Oct 21, 2016

ghost commented Nov 3, 2016

ghost commented Nov 3, 2016

ghost commented Nov 3, 2016

dcrowell77 commented Apr 12, 2019

ghost commented Jul 4, 2019

Gcc6 fixes #62

Gcc6 fixes #62

Conversation

ghost commented Aug 26, 2016 • edited by williamspatrick

shenki commented Aug 29, 2016

dcrowell77 Aug 30, 2016

Choose a reason for hiding this comment

ghost Aug 31, 2016

Choose a reason for hiding this comment

dcrowell77 commented Aug 30, 2016

ghost commented Aug 31, 2016

zane131 Sep 1, 2016

Choose a reason for hiding this comment

ghost Sep 6, 2016

Choose a reason for hiding this comment

ghost commented Sep 20, 2016

ghost commented Oct 21, 2016

ghost commented Nov 3, 2016

ghost commented Nov 3, 2016

ghost commented Nov 3, 2016

dcrowell77 commented Apr 12, 2019

ghost commented Jul 4, 2019

ghost commented Aug 26, 2016 •

edited by williamspatrick