Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetAlignmentOf returns 4 on Sparc when T=word64 #691

Closed
noloader opened this issue Jul 21, 2018 · 1 comment
Closed

GetAlignmentOf returns 4 on Sparc when T=word64 #691

noloader opened this issue Jul 21, 2018 · 1 comment

Comments

@noloader
Copy link
Collaborator

noloader commented Jul 21, 2018

We recently received access to the OpenCSW compile farm. The farm provides access to Solaris 9 through 11 on x86 and Sparc hardware. The machines make both GCC and SunCC available.

The cryptest.exe test program experienced a string of SIGBUS crashes. We kind of expected some trouble due to Issue 403. First it was SHA-384 (Issue 689), and then it was Tiger (Issue 690). Once Tiger was cleared VMAC crashed with a SIGBUS. It became obvious these were not one-offs.

We added code at Commit 0c0b68a4a28b to align the buffers in IteratedHashBase::HashMultipleBlocks() which should have avoided the SIGBUS:

if (noReverse)
{
   if (IsAligned<HashWordType>(input))
   {
      // Sparc bus error with non-aligned input.
      this->HashEndianCorrectedBlock(input);
   }
   else
   {
      std::memcpy(dataBuf, input, blockSize);
      this->HashEndianCorrectedBlock(dataBuf);
   }
}
else
{
   if (IsAligned<HashWordType>(input))
   {
      // Sparc bus error with non-aligned input.
      ByteReverse(dataBuf, input, blockSize);
      this->HashEndianCorrectedBlock(dataBuf);
   }
   else
   {
      std::memcpy(dataBuf, input, blockSize);
      ByteReverse(dataBuf, dataBuf, blockSize);
      this->HashEndianCorrectedBlock(dataBuf);
   }
}

We were still catching SIGBUSes after the change. Eventually the issue was tracked back to IsAligned<HashWordType>(), which uses GetAlignmentOf(). Notice there is no path for the SunCC compiler.

template <class T>
inline unsigned int GetAlignmentOf()
{
#if defined(CRYPTOPP_CXX11_ALIGNOF)
	return alignof(T);
#elif (_MSC_VER >= 1300)
	return __alignof(T);
#elif defined(__GNUC__)
	return __alignof__(T);
#elif CRYPTOPP_BOOL_SLOW_WORD64
	return UnsignedMin(4U, sizeof(T));
#else
# if __BIGGEST_ALIGNMENT__
	if (__BIGGEST_ALIGNMENT__ < sizeof(T))
		return __BIGGEST_ALIGNMENT__;
	else
# endif
	return sizeof(T);
#endif
}

With SunCC we landed in the UnsignedMin(4U, sizeof(T)) code path. Things worked fine with -xmemalign=4i but failed without it because alignment requirements of word64 is 8 without -xmemalign. Lack of -xmemalign is a good thing and means SunCC will generate a super-fast ldx machine instruction to load a 64-bit word (but it requires a 8-byte aligned load address).

The fix was simple. Add another code path for SunCC. The addition below was tested on the compile farm back to SunCC 5.8 on Solaris 9:

#elif defined(__GNUC__)
	return __alignof__(T);
#elif defined(__SUNPRO_CC)
	return __alignof__(T);
noloader added a commit to noloader/cryptopp-cmake that referenced this issue Jul 21, 2018
We recently received access to the OpenCSW compile farm (https://www.opencsw.org/extend-it/signup/to-upstream-maintainers/). The farm provides access to Solaris 9 through 11 on x86 and Sparc hardware.
We cleared the issue causing the SIGBUS. Also see Issue 691, GetAlignmentOf returns 4 on Sparc when T=word64 (weidai11/cryptopp#691).
noloader added a commit to noloader/cryptopp-autotools that referenced this issue Jul 21, 2018
We recently received access to the OpenCSW compile farm (https://www.opencsw.org/extend-it/signup/to-upstream-maintainers/). The farm provides access to Solaris 9 through 11 on x86 and Sparc hardware.
We cleared the issue causing the SIGBUS. Also see Issue 691, GetAlignmentOf returns 4 on Sparc when T=word64 (weidai11/cryptopp#691).
noloader added a commit that referenced this issue Jul 21, 2018
The extra code paths added at GH #689 were no longer needed after GH #691
@noloader
Copy link
Collaborator Author

Cleared at Commit d4f86d73209c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant