[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #5 from Markus Trippelsdorf trippels at gcc dot gnu.org --- See PR65709 for a similar issue.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852
--- Comment #7 from Markus Trippelsdorf trippels at gcc dot gnu.org ---
(In reply to Richard Biener from comment #6)
So I suppose the IsAligned template is wrong.
Yes.
390 template class T
391 inline unsigned int GetAlignmentOf(T *dummy=NULL) // VC60 workaround
392 {
393 #ifdef CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS
394 if (sizeof(T) 16)
395 return 1;
396 #endif
397
398 #if (_MSC_VER = 1300)
399 return __alignof(T);
400 #elif defined(__GNUC__)
401 return __alignof__(T);
402 #elif CRYPTOPP_BOOL_SLOW_WORD64
403 return UnsignedMin(4U, sizeof(T));
404 #else
405 return sizeof(T);
406 #endif
407 }
408
409 inline bool IsAlignedOn(const void *p, unsigned int alignment)
410 {
411 return alignment==1 || (IsPowerOf2(alignment) ?
ModPowerOf2((size_t)p, alignment) == 0 : (size_t)p % alignment == 0);
412 }
413
414 template class T
415 inline bool IsAligned(const void *p, T *dummy=NULL) // VC60 workaround
416 {
417 return IsAlignedOn(p, GetAlignmentOfT());
418 }
and
345 #if CRYPTOPP_BOOL_X64 || CRYPTOPP_BOOL_X86 || defined(__powerpc__)
346 #define CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS
347 #endif
The only architecture where the assumption is valid is POWER8, where
unaligned vector loads are preferable to ones with forced-alignment.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #8 from Jonathan Wakely redi at gcc dot gnu.org --- The code in algparam.h is just disgusting. AssignFromHelperClass binds a reference to NULL just to default-construct a temporary of some type, then binds a const-reference to that temporary, then casts away const to modify the value of that temporary. What seems to be a VC60 workaround introduces undefined behaviour by trying to be far too clever. Apparently also too clever to use compiler warnings.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #9 from Jeffrey Walton noloader at gmail dot com --- (In reply to Jonathan Wakely from comment #8) The code in algparam.h is just disgusting. AssignFromHelperClass binds a reference to NULL just to default-construct a temporary of some type, then binds a const-reference to that temporary, then casts away const to modify the value of that temporary. What seems to be a VC60 workaround introduces undefined behaviour by trying to be far too clever. Apparently also too clever to use compiler warnings. Lol... Yeah, some of the legacy code is awful. I'm not throwing stones because the library supported so many compilers and IDEs back then. I think its time to start cleaning up some of the cruft. I'm going to cite this comment when I propose some of the changes.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 Markus Trippelsdorf trippels at gcc dot gnu.org changed: What|Removed |Added Status|WAITING |RESOLVED CC||trippels at gcc dot gnu.org Resolution|--- |INVALID --- Comment #4 from Markus Trippelsdorf trippels at gcc dot gnu.org --- If you build with -fsanitize=undefined you'll see: algparam.h:204:87: runtime error: reference binding to null pointer of type 'const struct Integer' algparam.h:217:88: runtime error: reference binding to null pointer of type 'const struct Integer' algparam.h:220:88: runtime error: reference binding to null pointer of type 'const struct Integer' crc.cpp:134:28: runtime error: load of misaligned address 0x0127df9f for type 'const word32', which requires 4 byte alignment filters.cpp:280:39: runtime error: null pointer passed as argument 1, which is declared to never be null filters.cpp:280:39: runtime error: null pointer passed as argument 2, which is declared to never be null filters.cpp:281:50: runtime error: null pointer passed as argument 1, which is declared to never be null filters.cpp:281:50: runtime error: null pointer passed as argument 2, which is declared to never be null filters.cpp:291:28: runtime error: null pointer passed as argument 1, which is declared to never be null filters.cpp:518:84: runtime error: null pointer passed as argument 1, which is declared to never be null filters.cpp:518:84: runtime error: null pointer passed as argument 2, which is declared to never be null filters.cpp:676:35: runtime error: null pointer passed as argument 2, which is declared to never be null misc.cpp:101:29: runtime error: load of misaligned address 0x7ffdab82d529 for type 'word32', which requires 4 byte alignment misc.cpp:101:50: runtime error: load of misaligned address 0x0127df95 for type 'word32', which requires 4 byte alignment misc.cpp:26:43: runtime error: load of misaligned address 0x7ffdab82c713 for type 'word64', which requires 8 byte alignment misc.cpp:35:43: runtime error: load of misaligned address 0x03c65c4a for type 'word32', which requires 4 byte alignment misc.cpp:56:68: runtime error: store to misaligned address 0x03c67653 for type 'word64', which requires 8 byte alignment misc.cpp:66:67: runtime error: store to misaligned address 0x03c67651 for type 'word32', which requires 4 byte alignment misc.cpp:91:30: runtime error: load of misaligned address 0x7ffdab82d511 for type 'word64', which requires 8 byte alignment misc.h:1163:31: runtime error: load of misaligned address 0x0127d8aa for type 'const unsigned int', which requires 4 byte alignment misc.h:1181:2: runtime error: load of misaligned address 0x03c67669 for type 'const unsigned int', which requires 4 byte alignment misc.h:177:26: runtime error: null pointer passed as argument 2, which is declared to never be null misc.h:623:22: runtime error: shift exponent 32 is too large for 32-bit type 'unsigned int' misc.h:635:22: runtime error: shift exponent 32 is too large for 32-bit type 'unsigned int' misc.h:641:22: runtime error: shift exponent 32 is too large for 32-bit type 'unsigned int' misc.h:962:23: runtime error: load of misaligned address 0x7ffdab82c702 for type 'const long long unsigned int', which requires 8 byte alignment panama.cpp:371:11: runtime error: load of misaligned address 0x7ffdab82c716 for type 'const word32', which requires 4 byte alignment panama.cpp:371:18: runtime error: load of misaligned address 0x7ffdab82c71a for type 'const word32', which requires 4 byte alignment panama.cpp:371:25: runtime error: load of misaligned address 0x7ffdab82c71e for type 'const word32', which requires 4 byte alignment ... etc.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #11 from Jeffrey Walton noloader at gmail dot com --- (In reply to Jonathan Wakely from comment #8) The code in algparam.h is just disgusting. AssignFromHelperClass binds a reference to NULL just to default-construct a temporary of some type, then binds a const-reference to that temporary, then casts away const to modify the value of that temporary. What seems to be a VC60 workaround introduces undefined behaviour by trying to be far too clever. Apparently also too clever to use compiler warnings. The project is finally using -Wall, and its not flagging any issues with that code. I know -Wdelete-non-virtual-dtor is a problem (and potential security problem), and I'm trying to figure how how to proceed. Cf.https://groups.google.com/d/msg/cryptopp-users/__m0euOdbEQ/tvINItctzjAJ, What additional warnings would you suggest?
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 Richard Biener rguenth at gcc dot gnu.org changed: What|Removed |Added Target||x86_64-*-* Status|UNCONFIRMED |WAITING Last reconfirmed||2015-07-13 CC||rguenth at gcc dot gnu.org Version|4.9.0 |4.9.2 Ever confirmed|0 |1 --- Comment #3 from Richard Biener rguenth at gcc dot gnu.org --- Please at least attach preprocessed source of the file containing xorbuf ()
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #10 from Jeffrey Walton noloader at gmail dot com --- (In reply to Richard Biener from comment #6) So I suppose the IsAligned template is wrong. So I'm clear (please forgive my ignorance)... According to http://www.felixcloutier.com/x86/MOVDQA.html, vmovdqa requires values double quad word alignment (16-byte). A word64 is aligned on 8-bytes, and does not meet the precondition to use the instruction. Naively, that seems like a problem to me. Will that create problems with GCC and vectorizations?
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #12 from Markus Trippelsdorf trippels at gcc dot gnu.org --- (In reply to Jeffrey Walton from comment #10) (In reply to Richard Biener from comment #6) So I suppose the IsAligned template is wrong. So I'm clear (please forgive my ignorance)... According to http://www.felixcloutier.com/x86/MOVDQA.html, vmovdqa requires values double quad word alignment (16-byte). A word64 is aligned on 8-bytes, and does not meet the precondition to use the instruction. Naively, that seems like a problem to me. Will that create problems with GCC and vectorizations? See PR65709 Comment 9 and followups.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #6 from Richard Biener rguenth at gcc dot gnu.org --- So I suppose the IsAligned template is wrong.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #13 from Jeffrey Walton noloader at gmail dot com --- A quick update We did out best to take the advice of Jakub, Janathan, Markus and others: https://github.com/weidai11/cryptopp/commit/9bf0eed0f6ff6d0b0a2d277e5401d69dc8c0e394. We are paying for past transgressions, and cleanup is not as easy as we would like. Eventually, we want to enable CRYPTOPP_NO_UNALIGNED_DATA_ACCESS by default. It removes all related undefined behavior flagged by UBsan. The only thing stopping us is the politics of such a move.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852 --- Comment #1 from Jeffrey Walton noloader at gmail dot com --- This also appears to be an issue with GCC 4.8 and 5.1. See https://groups.google.com/d/msg/cryptopp-users/BlPiQ2eAWhg/IsX18wAT9ZAJ.
[Bug c++/66852] vmovdqa instructions issued on 64-bit aligned array, causes segfault
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66852
--- Comment #2 from Jeffrey Walton noloader at gmail dot com ---
My apologies for *not* having a minimum working example.
If you want to duplicate it, then:
git clone https://github.com/weidai11/cryptopp.git
Open GNUmakefile, and change to (around line 3):
OPTIMIZE ?= -O3
Depending on when we checkin the fix, you may (or may not) experience the bug.
I don't know how to tell git to checkout a particular revision because it does
not use them. I guess you should do something to get a version of the sources
prior to Sunday, July 12, 2015.
If the proposed fix is applied, then remove the optimize pragma from misc.cpp
(begins around line 20):
#pragma GCC push_options
#pragma GCC optimize (-O2)
void xorbuf(byte *buf, const byte *mask, size_t count)
{
...
}
#pragma GCC pop_options
