-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
make check
fails with 8.7.0
#1141
Comments
It segfaults with no arguments too which is much simpler, but it's at a different line, so a different problem I think:
|
I think |
Thanks @thesamesam. Yeah, the crash when invoking I think the crash in the log you provided is different, however. I think it is another instance of GH #1134. Let me try to get my Gentoo system updated. Emerge is always a battle... |
Thanks for the speedy response as usual! For the test failure (first issue), bisect says:
I'm around for maybe an hour or so more, and then later today, so if you want me to help at all on the emerge side, happy to do so here / via email / via IRC and get you sorted quickly. |
I have not been able to get emerge to update my Gentoo machine. It is still failing with complaints about Python. In the meantime, I can duplicate the issue on Fedora and Debian (by removing work-arounds). On Fedora and Debian, building at Can you drop Crypto++ to As a next step, I'm going to see what is enabled at According to GCC 12 Optimizations,
I don't know how to disable Ping @gcsideal and @Vascom. Debian and Fedora may see more problems with this issue. |
I filed a report with the GCC folks, but it is a shitty report since we don't have a reproducer: Issue 106568, -freorder-blocks-algorithm appears to causes a crash in stable code, no way to disable it. I would not be surprised if the GCC folks ignored it. But I hope they at least tell us a way to disable |
We had to approach this by starting at
I am not sure which option causes things to go sideways. I've had problems with And Andrew Pinski commented, "... the way Also see Comment 16 and Comment 20 in the GCC bug report. |
As noted previously, with upstream build flags Crypto++ can be built with GCC 12 on Debian and 'make check' succeeds. But with distribution build flags testing fails very similar like in Gentoo. Disabled LTO and it didn't help. Testing with '-fsanitize=unreachable' might give you more information:
How can I get you more information? |
Thanks @gcsideal.
Yeah, we papered over the problem but did not fix it. All we did was move the problem around.
Does anyone know how to tell GCC to stop removing bits of code, like a member function? |
Indeed, |
Ok, I think I fixed ARIA at Commit 09ad51cf9ea9. I don't think there was an out-of-bounds read. Rather, I think GCC was having trouble with analysis due to the use of macros. I cut-over to inline functions and I am not seeing the build failure under my Debian Chroot for ARMEL. I also fixed the broken x86 builds at Commit 01a18bdbcb22. The broken x86 build was disappointing because it slipped through my fingers during testing. I tried to test on my 32-bit Ubuntu 14 VM, but GitHub was down so the Can you give the build another try using Master, please? (I still don't have an answer to the initial problem of GCC removing the code for ECGDSA's |
I'm going to check it in some hours how the build goes with current master HEAD. Meanwhile do you have a check if you need to link with atomic (for example on powerpc) and where the extra ubsan linking (for example on hppa) comes? |
Thanks.
Oh, I do not. That sounds like it could be another GCC bug. The GCC folks tell us to use the compiler driver and all necessary libs will be added when using the sanitizers. If the libs are not being added, then the compiler driver is to blame. But stepping back a bit... On Fedora, we need to install libasan and libubsan (in addition to gcc, g++ and friends). Does powerpc and hppa need a separate libasan and libubsan install? Let me ping the Debian powerpc mailing list. John Paul Adrian Glaubitz may know the answer. See Need for manual link to libatomic on powerpc? on the debian-powerpc mailing list. |
Seems to be GCC bug(s) [1][2].
Isn't libasan and libubsan are for profiling only? On the other hand, GCC 12.2.0 is recently packaged for Debian. Several architectures still building it. May you have a day waiting for it? It might solve some problems - or introduce others. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81358 |
The thing that strikes me about libasan and libubsan is, they are separate installs. They don't get installed automatically with the compiler. And if they are not installed, options like
Ok, sounds good. Let me try some things in the Debian Chroot and come up with some ideas.
:) |
I was able to compile and link the library using a Debian Chroot for powerpc. It looks like all the test passed, even the problem one for ECGDSA+RIPEMD. However, I only used the flags If I use |
…1#1141) This is not a fix since it only treats the symptom of GCC removing live code. We do not know why GCC is doing it.
@thesamesam, @gcsideal, @mouse07410, I'm working on my testing fork. I checked in the changes required to test for GCC 12 and If you have no objections I'll merge it into Master. Note that this is not a fix since it only treats the symptom of GCC removing live code. We still do not know why GCC is doing it. |
…1#1141) This is not a fix since it only treats the symptom of GCC removing live code. We do not know why GCC is doing it.
I also compile it with GCC-12 (on Mac). No objection to merging. |
…1#1141) This is not a fix since it only treats the symptom of GCC removing live code. We do not know why GCC is doing it.
…1#1141) This is not a fix since it only treats the symptom of GCC removing live code. We do not know why GCC is doing it.
So you did not experience a crash with |
Yes, that is correct. And recently I've been compiling with the flag |
Thanks.
I guess in that case we should limit the workaround to Linux. I'll get the code updated and merged. |
…1#1141) This is not a fix since it only treats the symptom of GCC removing live code. We do not know why GCC is doing it.
…1#1141) This is not a fix since it only treats the symptom of GCC removing live code. We do not know why GCC is doing it.
EDIT: wait a minute, actually |
I merged the fix from my testing fork into Master at Commit 0b5747421b27. Can you test Master when you have some time, please? |
Okay, so I cloned master just now (I was testing your fork with the patch on top at first, but was wondering if I missed a commit) and it still seems to fail:
(Ignore the GCC 13 here, it's because I have sources for 13 installed, but it was built with 12.2.0, I promise)
|
This use of
This should accomplish what you want. It uses your flags, and allows us to add our flags:
And you don't need
Finally, we always honor a user's flags. We don't stomp on them like many projects. That's why our makefile tracks user flags in ###########################################################
##### Add our flags to user flags #####
###########################################################
# This ensures we don't add flags when the user forbids
# use of customary library flags, like -fPIC. Make will
# ignore this assignment when CXXFLAGS is passed as an
# argument to the make program: make CXXFLAGS="..."
CPPFLAGS := $(strip $(CRYPTOPP_CPPFLAGS) $(CPPFLAGS))
CXXFLAGS := $(strip $(CRYPTOPP_CXXFLAGS) $(CXXFLAGS))
ASFLAGS := $(strip $(CRYPTOPP_ASFLAGS) $(ASFLAGS))
LDFLAGS := $(strip $(CRYPTOPP_LDFLAGS) $(LDFLAGS)) Your flags will always take precedence over our flags. |
Ah, thanks, that makes complete sense (yeah, re CFLAGS -- I tend to just default to writing it out because some projects are naughty). I'm sorry, I was rushing and didn't think properly! All tests pass now, including with Thank you for your patience and help! |
The CMake project is going to need the workaround in this bug report for the problem of GCC 12 removing live code. The workaround is for GCC 12 (and above) on Linux, and it adds Adding the Here's the check-in CMake needs to duplicate: Commit 0b5747421b27. The commit adds a test program and the makefile runs the test program with the option |
@gcsideal, @thesamesam, @abdes, @mouse07410, We released Crypto++ 8,7 on August 7, 2022. I am planning for a Crypto++ 8.8 release around September 7 to get the latest problems off the books. The latest problems are (1) the no-arg cryptest.exe crash, (2) this problem, and (3) the x86 failed build due to inline assembly clobber registers. Those three problems are going to generate a lot of bug reports and mailing list noise. I need to avoid that since I have fewer spare cycles nowadays. Here is a rollup of patches for the issues: rollup.diff.zip |
OK, packaged current master HEAD and force fixed the atomic linking. Did some preliminary testing on mips64el and powerpc and those were successful. Just uploaded the package for official building. |
Thansk @gcsideal,
I don't think you need libatomic. That gets brought in by the sanitizers. We don't use the sanitizers for release builds.
Very good.
Awesome, thanks. |
Then I built incorrectly before? On two architectures it failed to build due to missing link with atomic.
Close to all builds came in [1] and was successful. Well, on hppa first build produced an ICE [2] but second try [3] was built correctly. |
Hi @gcsideal,
I would not say incorrectly since it uncovered problems with GCC for the platforms. But we don't need the sanitizer flags or libatomic to build the library.
Uh oh... more GCC problems. Its been a bad week for the compiler guys :) |
Ok, I think - |
Crypto++ Issue Report
8.7.0 fails tests with a segfault. 8.6.0 passes fine.
The full build log is available here: build.log
Note that this is with debugging symbols on for everything:
System information:
Let me know if you need more from me.
The text was updated successfully, but these errors were encountered: