-
Notifications
You must be signed in to change notification settings - Fork 138
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
Export of runtime in a separate repository #162
Conversation
CMakeLists.txt
Outdated
compiler/Symbolizer.cpp | ||
compiler/Pass.cpp | ||
compiler/Runtime.cpp | ||
compiler/Main.cpp) | ||
|
||
set_target_properties(Symcc PROPERTIES OUTPUT_NAME "symcc") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should keep the library name libSymbolize.so
🤔 These are breaking changes anyway, but maybe we can save the users some work by not changing the library name. (My employer is one such user, and I'm the person who will have to fix the build scripts, so I'm not being entirely altruistic here 🙃)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I knew when writing this line it would be problematic :)
I was thinking it would be nice to keep consistent with other libraries in general, which seems to be lib<lowercase_project_name>.so
.
If it is too much work to change this, let's keep the original name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really a big change for the "users"? Taking the opportunity to changing to something that makes more sense seem appealing ;)
compiler/symcc.in
Outdated
-Wl,-rpath,"$runtime_dir" \ | ||
-lstdc++ \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we link against libstdc++
here? I guess this is to support the static version of the runtime; if that's the case then I'm not convinced that it's the right approach. We can't be sure that the runtime has been built with libstdc++
(vs some other C++ runtime like libc++
) 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I don't really like this fix as well.
Without this flag, I get this error:
Symbolizer module instrumentation
Symbolizing function main
/usr/bin/ld: /tmp/sample-f61b56.o: undefined reference to symbol '_ZSt3cin@@GLIBCXX_3.4'
/usr/bin/ld: /usr/lib/libstdc++.so.6: error adding symbols: DSO missing from command line
clang: error: linker command failed with exit code 1 (use -v to see invocation)
There is a StackOverflow post about this issue, maybe we could use g++ instead of gcc as suggested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. After the tests pass :)
CMakeLists.txt
Outdated
compiler/Symbolizer.cpp | ||
compiler/Pass.cpp | ||
compiler/Runtime.cpp | ||
compiler/Main.cpp) | ||
|
||
set_target_properties(Symcc PROPERTIES OUTPUT_NAME "symcc") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really a big change for the "users"? Taking the opportunity to changing to something that makes more sense seem appealing ;)
We are working on the separation of the
runtime
subdirectory into a separate repository, symcc-rt.The idea is to decorrelate the compilation of the runtime part of SymCC from SymCC itself since it is already used in different projects (like SymQEMU). It would make the integration of the runtime in these projects more smooth, especially when interacting with other build systems like meson.
Also, SymCC Runtime can now be compiled statically.
This is a WIP, and any comments are welcome.
There are also minor improvements here and there in the CMakeLists.
Notably, the minimum version of cmake is bumped to 3.16.
More details are available in the compagnon PR of symcc-rt.
Another compagnon PR for SymQEMU is also available.
For now, there is a problem with the Dockerfile line 107 in case someone wants to check the issue.