From c125d0b765062cfe3a97aee984a591c403bd1c99 Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Thu, 25 Aug 2016 20:22:30 +1000 Subject: [PATCH 1/8] Default to std=gnu++03 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 https://github.com/open-power/hostboot/pull/62#discussion_r76830612 ) Change-Id: I6661b5b61319c85c2a90926beb6e2662e96dcf06 Signed-off-by: Stewart Smith --- src/build/mkrules/cflags.env.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/build/mkrules/cflags.env.mk b/src/build/mkrules/cflags.env.mk index c805999a558..0bae0eca52e 100644 --- a/src/build/mkrules/cflags.env.mk +++ b/src/build/mkrules/cflags.env.mk @@ -41,7 +41,7 @@ CFLAGS += $(COMMONFLAGS) -mcpu=power7 -nostdinc -g -mno-vsx -mno-altivec\ -ffunction-sections -fdata-sections -ffreestanding -mbig-endian ASMFLAGS += $(COMMONFLAGS) -mcpu=power7 -mbig-endian CXXFLAGS += $(CFLAGS) -nostdinc++ -fno-rtti -fno-exceptions -Wall \ - -fuse-cxa-atexit + -fuse-cxa-atexit -std=gnu++03 LDFLAGS += --nostdlib --sort-common -EB $(COMMONFLAGS) INCFLAGS = $(addprefix -I, $(INCDIR) ) From 118c19b0fdd3470da70cc0e61d9b5768353fa98f Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Thu, 25 Aug 2016 19:22:00 +1000 Subject: [PATCH 2/8] fix build error: return-statement with a value, in function returning 'void' Change-Id: I9dc8b698fec95488e209cbc40b928769a26b6de6 Signed-off-by: Stewart Smith --- src/include/usr/devicefw/driverif.H | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/include/usr/devicefw/driverif.H b/src/include/usr/devicefw/driverif.H index a148725b5fd..fb43073514c 100644 --- a/src/include/usr/devicefw/driverif.H +++ b/src/include/usr/devicefw/driverif.H @@ -266,7 +266,7 @@ namespace DeviceFW TargType i_targetType, deviceOp_t i_regRoute) { - return InvalidParameterType(); // Cause a compile fail if not one of + InvalidParameterType(); // Cause a compile fail if not one of // the explicit template specializations. } From d19728daf7e6ab7202db45239b45aa9443943745 Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Thu, 25 Aug 2016 19:27:04 +1000 Subject: [PATCH 3/8] error: dereferencing type-punned pointer will break strict-aliasing rules Change-Id: I63accd3e881c941736ece4b4498c2c9d06ff8761 Signed-off-by: Stewart Smith --- src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.H | 7 +++++++ .../hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.H | 1 - .../hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.C | 6 +++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.H b/src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.H index e5af2c9e134..7edf2bfa0bc 100755 --- a/src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.H +++ b/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: + MBvpdVMKeyword() : iv_version(0),iv_systemType(0), + iv_systemType_ext(0),iv_dataVersion(0) {}; + operator uint32_t() const { + return iv_version << 24 | iv_systemType << 16 | + iv_systemType_ext << 8 | iv_dataVersion; + } }; // Attribute definition struct MBvpdAttrDef diff --git a/src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.H b/src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.H index f172508cb5e..85460b2107c 100644 --- a/src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.H +++ b/src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.H @@ -33,7 +33,6 @@ #define _HWP_GETMBVPDMEMDATAVERSION_ #include -#define VM_KEYWORD_DEFAULT_VALUE 0x00000000 // function pointer typedef definition for HWP call support typedef fapi::ReturnCode (*getMBvpdMemoryDataVersion_FP_t) diff --git a/src/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.C b/src/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.C index 2f83fa234d2..ef6451588a8 100644 --- a/src/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.C +++ b/src/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.C @@ -50,7 +50,7 @@ fapi::ReturnCode getMBvpdMemoryDataVersion( fapi::ReturnCode l_fapirc; DimmType l_dimmType = ISDIMM; fapi::MBvpdRecord l_record = fapi::MBVPD_RECORD_SPDX; - uint32_t l_vpdMemoryDataVersion = VM_KEYWORD_DEFAULT_VALUE; + MBvpdVMKeyword l_vpdMemoryDataVersion; uint32_t l_bufSize = sizeof(l_vpdMemoryDataVersion); FAPI_DBG("getMBvpdMemoryDataVersion: entry "); @@ -140,8 +140,8 @@ fapi::ReturnCode getMBvpdMemoryDataVersion( } // Check if the format byte in the value returned is in between valid range - if (( ((MBvpdVMKeyword *)(&l_vpdMemoryDataVersion))->iv_version > VM_SUPPORTED_HIGH_VER )|| - ( ((MBvpdVMKeyword *)(&l_vpdMemoryDataVersion))->iv_version == VM_NOT_SUPPORTED )) + if ((l_vpdMemoryDataVersion.iv_version > VM_SUPPORTED_HIGH_VER )|| + (l_vpdMemoryDataVersion.iv_version == VM_NOT_SUPPORTED )) { FAPI_ERR("getMBvpdMemoryDataVersion:" " keyword data returned is invalid : %d ", From 375727247c5f02fcc144f3e21d02056acd412571 Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Thu, 25 Aug 2016 19:33:42 +1000 Subject: [PATCH 4/8] Change cv_forcedMemPeriodic to int as bool is invalid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/include/kernel/cpumgr.H | 2 +- src/kernel/cpumgr.C | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/include/kernel/cpumgr.H b/src/include/kernel/cpumgr.H index 68f8972784a..562c27cb39b 100644 --- a/src/include/kernel/cpumgr.H +++ b/src/include/kernel/cpumgr.H @@ -215,7 +215,7 @@ class CpuManager */ static uint64_t cv_cpuSeq; - static bool cv_forcedMemPeriodic; //!< force free / coalesce. + static int cv_forcedMemPeriodic; //!< force free / coalesce. // If a shutdown of all CPUs is requested static bool cv_shutdown_requested; diff --git a/src/kernel/cpumgr.C b/src/kernel/cpumgr.C index 44f61a17393..5dc1833408b 100644 --- a/src/kernel/cpumgr.C +++ b/src/kernel/cpumgr.C @@ -50,7 +50,7 @@ cpu_t** CpuManager::cv_cpus[KERNEL_MAX_SUPPORTED_NODES]; bool CpuManager::cv_shutdown_requested = false; uint64_t CpuManager::cv_shutdown_status = 0; size_t CpuManager::cv_cpuSeq = 0; -bool CpuManager::cv_forcedMemPeriodic = false; +int CpuManager::cv_forcedMemPeriodic = 0; InteractiveDebug CpuManager::cv_interactive_debug; CpuManager::CpuManager() : iv_lastStartTimebase(0) @@ -361,7 +361,7 @@ void CpuManager::executePeriodics(cpu_t * i_cpu) } bool forceMemoryPeriodic = __sync_fetch_and_and(&cv_forcedMemPeriodic, - false); + 0); ++(i_cpu->periodic_count); if((0 == (i_cpu->periodic_count % CPU_PERIODIC_CHECK_MEMORY)) || @@ -461,7 +461,7 @@ size_t CpuManager::getThreadCount() void CpuManager::forceMemoryPeriodic() { - cv_forcedMemPeriodic = true; + cv_forcedMemPeriodic = 1; } From 17f5649ef96e05f05a167b6d9dd62d98fba59430 Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Thu, 25 Aug 2016 20:13:11 +1000 Subject: [PATCH 5/8] =?UTF-8?q?error:=20the=20compiler=20can=20assume=20th?= =?UTF-8?q?at=20the=20address=20of=20=E2=80=98r=E2=80=99=20will=20never=20?= =?UTF-8?q?be=20NULL?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRDF_ASSERT( &r != NULL ); Change-Id: I2d60075f9e2232512efe45a5c6aa5563f3a565f5 Signed-off-by: Stewart Smith --- src/usr/diag/prdf/common/framework/register/prdfErrorRegister.C | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/usr/diag/prdf/common/framework/register/prdfErrorRegister.C b/src/usr/diag/prdf/common/framework/register/prdfErrorRegister.C index 9ee1358eb3b..ef7279b9deb 100755 --- a/src/usr/diag/prdf/common/framework/register/prdfErrorRegister.C +++ b/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 ); - PRDF_ASSERT( &rm != NULL ); } /*---------------------------------------------------------------------*/ From 32dc01647c68c5e9e6d65ae44ba1d3203d50898f Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Thu, 25 Aug 2016 20:07:58 +1000 Subject: [PATCH 6/8] Fix compiler can assume address will never be NULL error with GCC6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../framework/register/prdfOperatorRegister.H | 16 ++++++++-------- .../common/framework/register/prdfScanFacility.C | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H b/src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H index b0513e48378..a26a76e05ec 100755 --- a/src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H +++ b/src/usr/diag/prdf/common/framework/register/prdfOperatorRegister.H @@ -434,15 +434,15 @@ class AttnTypeRegister : public SCAN_COMM_REGISTER_CLASS iv_bs = &iv_iBS; } - AttnTypeRegister( SCAN_COMM_REGISTER_CLASS & i_check, - SCAN_COMM_REGISTER_CLASS & i_recov, - SCAN_COMM_REGISTER_CLASS & i_special, - SCAN_COMM_REGISTER_CLASS & i_proccs ) : + AttnTypeRegister( SCAN_COMM_REGISTER_CLASS *i_check, + SCAN_COMM_REGISTER_CLASS *i_recov, + SCAN_COMM_REGISTER_CLASS *i_special, + SCAN_COMM_REGISTER_CLASS *i_proccs ) : SCAN_COMM_REGISTER_CLASS( ), - iv_check( NULL == &i_check ? &cv_null : &i_check), - iv_recov( NULL == &i_recov ? &cv_null : &i_recov), - iv_special(NULL == &i_special ? &cv_null : &i_special), - iv_proccs( NULL == &i_proccs ? &cv_null : &i_proccs), + iv_check( NULL == i_check ? &cv_null : i_check), + iv_recov( NULL == i_recov ? &cv_null : i_recov), + iv_special(NULL == i_special ? &cv_null : i_special), + iv_proccs( NULL == i_proccs ? &cv_null : i_proccs), iv_iBS(0) // will fully initialize this inside ctor. { uint32_t l_length = 1024; diff --git a/src/usr/diag/prdf/common/framework/register/prdfScanFacility.C b/src/usr/diag/prdf/common/framework/register/prdfScanFacility.C index 0d379cfb672..cad5ce89ffd 100755 --- a/src/usr/diag/prdf/common/framework/register/prdfScanFacility.C +++ b/src/usr/diag/prdf/common/framework/register/prdfScanFacility.C @@ -166,7 +166,7 @@ SCAN_COMM_REGISTER_CLASS & ScanFacility::GetAttnTypeRegister( SCAN_COMM_REGISTER_CLASS * i_special, SCAN_COMM_REGISTER_CLASS * i_proccs ) { - AttnTypeRegister r(*i_check, *i_recov, *i_special, *i_proccs); + AttnTypeRegister r(i_check, i_recov, i_special, i_proccs); return iv_attnRegFw.get(r); } From 2b077be8c456ba47935760fae5d46e0784a32b36 Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Wed, 31 Aug 2016 13:07:17 +1000 Subject: [PATCH 7/8] =?UTF-8?q?error:=20in=20C++98=20=E2=80=98l=5FvmVersio?= =?UTF-8?q?nBuf=E2=80=99=20must=20be=20initialized=20by=20constructor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix GCC6 thrown error Change-Id: I9aa508843f54c99ebb59822c39590811423ad0b1 Signed-off-by: Stewart Smith --- src/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.C | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.C b/src/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.C index 3739c757134..a4685ad04cb 100755 --- a/src/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.C +++ b/src/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.C @@ -392,7 +392,7 @@ fapi::ReturnCode getVersion (const fapi::Target & i_mbaTarget, fapi::Target l_mbTarget; fapi::MBvpdKeyword l_keyword = fapi::MBVPD_KEYWORD_VM; // try VM first fapi::MBvpdRecord l_record = fapi::MBVPD_RECORD_SPDX; // default to SPDX - MBvpdVMKeyword l_vmVersionBuf={}; + MBvpdVMKeyword l_vmVersionBuf; uint32_t l_version = 0; uint32_t l_vmBufSize = sizeof(MBvpdVMKeyword); // VM keyword is of 4 bytes. uint16_t l_versionBuf = 0; From 61b675f931633e12b46e06a08f953f4fc470c4f0 Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Thu, 3 Nov 2016 14:36:19 +1100 Subject: [PATCH 8/8] Use -std=gnu++03 for host g++ invocations 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 --- src/usr/hwpf/makefile | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/usr/hwpf/makefile b/src/usr/hwpf/makefile index 8d0bb723fc0..b6d2205dec3 100644 --- a/src/usr/hwpf/makefile +++ b/src/usr/hwpf/makefile @@ -402,18 +402,26 @@ $(call GENTARGET, ${IF_CMP_FLEX_TARGET}) : \ $(C2) " FLEX $(notdir $<)" $(C1)flex -o$@ $^ +try = $(shell set -e; if ($(1)) >/dev/null 2>&1; \ + then echo "$(2)"; \ + else echo "$(3)"; fi ) + +try-cflag = $(call try,$(1) $(2) -x c -c /dev/null -o /dev/null,$(2)) +HOSTCFLAGS:=-O3 +HOSTCFLAGS+=$(call try-cflag,$(HOST_PREFIX)g++,-std=gnu++03) + $(GENDIR)/$(IF_CMP_SUBDIR)/%.host.o: \ ifcompiler/%.C $(IF_COMPILER_H_FILES) \ $(GENDIR)/$(IF_CMP_YACC_H_TARGET) $(C2) " CXX $(notdir $<)" - $(C1)$(CCACHE) $(HOST_PREFIX)g++ -O3 $< -I ifcompiler -I $(GENDIR) \ + $(C1)$(CCACHE) $(HOST_PREFIX)g++ $(HOSTCFLAGS) $< -I ifcompiler -I $(GENDIR) \ -I $(GENDIR)/$(IF_CMP_SUBDIR) \ -I $(ROOTPATH)/src/include/usr/hwpf/hwp -c -o $@ $(GENDIR)/$(IF_CMP_YACC_C_TARGET:.c=.host.o): \ $(GENDIR)/$(IF_CMP_YACC_C_TARGET) $(IF_COMPILER_H_FILES) $(C2) " CXX $(notdir $<)" - $(C1)$(CCACHE) $(HOST_PREFIX)g++ -O3 $< -I ifcompiler -I $(GENDIR) \ + $(C1)$(CCACHE) $(HOST_PREFIX)g++ $(HOSTCFLAGS) $< -I ifcompiler -I $(GENDIR) \ -I $(GENDIR)/$(IF_CMP_SUBDIR) \ -I $(ROOTPATH)/src/include/usr/hwpf/hwp -c -o $@ @@ -421,7 +429,7 @@ $(GENDIR)/$(IF_CMP_FLEX_TARGET:.c=.host.o): \ $(GENDIR)/$(IF_CMP_FLEX_TARGET) $(IF_COMPILER_H_FILES) \ $(GENDIR)/$(IF_CMP_YACC_H_TARGET) $(C2) " CXX $(notdir $<)" - $(C1)$(CCACHE) $(HOST_PREFIX)g++ -O3 -DHOSTBOOT_COMPILE $< -I ifcompiler -I $(GENDIR) \ + $(C1)$(CCACHE) $(HOST_PREFIX)g++ $(HOSTCFLAGS) -DHOSTBOOT_COMPILE $< -I ifcompiler -I $(GENDIR) \ -I $(GENDIR)/$(IF_CMP_SUBDIR) \ -I $(ROOTPATH)/src/include/usr/hwpf/hwp -c -o $@