-
Notifications
You must be signed in to change notification settings - Fork 192
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
Crash with -D_FORTIFY_SOURCE=2 on Linux #176
Comments
That is an issue with the c-preprocessor. We use the BOOST C-preprocessor called wave, which we had to bundle with soufflé because the llvm/clang c-preprocessor is not compatible for our purposes. I have checked several sprintf() statements in the source code and I see that some have fixed buffer sizes. Is there a possibility to see which sprintf() statement fails in the source-code? What I am asking is, can you relate the machine code address /tmp/nix-build-souffle-1.0.0.drv-0/souffle-1.0.0-src/src/souffle-wave[0x44fa5d] to an actual source code line (For example by adding the flag -g etc.). |
@b-scholz hmm, I added In the meantime, here's a backtrace with function symbols that gdb was at least able to provide:
|
No worries. I believe it is related to the buffer sizes of the sprintf() statements in the file https://github.com/souffle-lang/souffle/blob/master/src/wavelib/util/cpp_macromap_predef.hpp It could be that your compiler uses unicode instead of UTF8 for the buffer definitions (i.e. char buffer[...]) If this is the case all constants in the buffer length definitions will need to be doubled. For example, Could you give it a try? |
Patching all occurrences of Still surprised that's happening though. I've never seen a C compiler do that, so I took my compiler and compiled this with it:
and it prints 14, so the |
We enable the C++ 2011 standard for compilation. It could be the specific flags we use. The bottom line is that there should be no sprintf() in the code. We should rewrite the code such that string-streams in C++ are used. I will put it on my TODO list. |
Alright, that sounds right. Either way, your fix seemed to address the issue 😄 Thanks! |
Is it worth fixing (like I could submit a PR) the +1 vs. +2 thing for now or do you think the |
It would be great if you could submit a PR because it is a bug. We will replace the sprintf() as soon as possible. |
Should resolve souffle-lang#176, although it's more of a stopgap measure until this code gets replaced with C++ string stream operations.
Should resolve souffle-lang#176, although it's more of a stopgap measure until this code gets replaced with C++ string stream operations.
I've packaged souffle on Nix (a purely functional package manager and distribution) and noticed that with the Nix default compile-time hardening parameters,
souffle
crashes as soon as it does any actual work.To reproduce:
Note that in my case all the dependencies are provided by Nix so that's why there are the weird paths in the memory dump.
I don't get the same problem on Darwin, possibly because clang/llvm (not the standard Apple one) implements the fortification measures differently.
If you have trouble reproducing I can try to get the thing to fail in the same way using a more conventional (non-Nix) build.
The text was updated successfully, but these errors were encountered: