Skip to content
Closed
2 changes: 1 addition & 1 deletion src/build/mkrules/cflags.env.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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) )
Expand Down
2 changes: 1 addition & 1 deletion src/include/kernel/cpumgr.H
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/include/usr/devicefw/driverif.H
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}

Expand Down
7 changes: 7 additions & 0 deletions src/include/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.H
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,13 @@ namespace getAttrData
uint8_t iv_systemType;
uint8_t iv_systemType_ext;
uint8_t iv_dataVersion;
public:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#define _HWP_GETMBVPDMEMDATAVERSION_

#include <fapi.H>
#define VM_KEYWORD_DEFAULT_VALUE 0x00000000

// function pointer typedef definition for HWP call support
typedef fapi::ReturnCode (*getMBvpdMemoryDataVersion_FP_t)
Expand Down
6 changes: 3 additions & 3 deletions src/kernel/cpumgr.C
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)) ||
Expand Down Expand Up @@ -461,7 +461,7 @@ size_t CpuManager::getThreadCount()

void CpuManager::forceMemoryPeriodic()
{
cv_forcedMemPeriodic = true;
cv_forcedMemPeriodic = 1;
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/*---------------------------------------------------------------------*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/usr/hwpf/hwp/mvpd_accessors/getMBvpdAttr.C
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/usr/hwpf/hwp/mvpd_accessors/getMBvpdMemoryDataVersion.C
Original file line number Diff line number Diff line change
Expand Up @@ -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 ");
Expand Down Expand Up @@ -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 ",
Expand Down
14 changes: 11 additions & 3 deletions src/usr/hwpf/makefile
Original file line number Diff line number Diff line change
Expand Up @@ -402,26 +402,34 @@ $(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 $@

$(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 $@

Expand Down