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

Add LLVM-pass plugin support to LDC. Commandline flag: -plugin=.... #2554

Merged
merged 10 commits into from
Feb 13, 2018

Conversation

JohanEngelen
Copy link
Member

This adds functionality to load plugins such as the AFL llvm-mode plugin: https://github.com/mirrorer/afl/blob/master/llvm_mode/afl-llvm-pass.so.cc

Adds a simple test that demonstrates how to create a plugin.
Test only enabled on Darwin and Linux.

Adds a simple test that demonstrates how to create a plugin.
Test only enabled on Darwin and Linux.
@JohanEngelen
Copy link
Member Author

resolves #2547

@JohanEngelen
Copy link
Member Author

Interesting: on Linux this needs ldc2 to be built with -Wl,--export-dynamic.

@JohanEngelen
Copy link
Member Author

(the ./ in front of the .so name is needed on Linux because of this: https://stackoverflow.com/questions/17715774/c-linux-dlopen-cant-find-so-library )

@JohanEngelen
Copy link
Member Author

Green :)
Comments @klickverbot ?

@kinke
Copy link
Member

kinke commented Feb 7, 2018

on Linux this needs ldc2 to be built with -Wl,--export-dynamic.

I just built this PR and current master with LLVM 5.0.0 (Release + asserts), Release config, on Linux x64. The executable size increases by 20% (50 MB => 60 MB).

@JohanEngelen
Copy link
Member Author

JohanEngelen commented Feb 7, 2018

The executable size increases by 20% (50 MB => 60 MB).

Yeah, I was afraid it'd be a lot :/ Any alternatives? (I'm having a hard time figuring out what Clang does to provide the symbols for the plugin)

Edit: the symbols that are reported missing without -Wl,--export-dynamic are shown by nm ldc2. But things didn't work.

@kinke
Copy link
Member

kinke commented Feb 7, 2018

I don't know, but simply making the plugin support opt-in (or -out) via CMake sounds good enough to me, so that ARM builds etc. can remain 'small' if need be.

@JohanEngelen
Copy link
Member Author

I'll add a (default off) CMake option for it. But I'm sure it'll mean that very few people will be able to use plugins then... :/
LLVM/Clang has similar issues it looks like. When LLVM is built/used as shared lib, things are OK though.
https://bugs.llvm.org/show_bug.cgi?id=18407
https://bugs.llvm.org/show_bug.cgi?id=13144
https://bugs.llvm.org/show_bug.cgi?id=19333

CMakeLists.txt Outdated
@@ -570,6 +570,7 @@ else()
endif()
set(LDC_ENABLE_PLUGINS ${LDC_ENABLE_PLUGINS_DEFAULT} CACHE BOOL "Build LDC with plugin support (increases binary size)")
if(LDC_ENABLE_PLUGINS)
message(STATUS "Building LDC with plugin support")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a default of 'off' means that few people will try it, perhaps you should add a status "Building LDC without plugin support"?

@kinke
Copy link
Member

kinke commented Feb 10, 2018

But I'm sure it'll mean that very few people will be able to use plugins then... :/

I'd be fine with it defaulting to ON on non-Windows. Alternatively, enabling it explicitly for the Circle build is another way to significantly increase the feature availability (and additionally makes sure it is tested on Linux too, not just OSX as of now).

@JohanEngelen
Copy link
Member Author

Plugins are default on on Linux+Apple systems. Can be turned off with -D LDC_ENABLE_PLUGINS=OFF.
Good to squash+merge?

@kinke
Copy link
Member

kinke commented Feb 12, 2018

Lgtm.

@JohanEngelen JohanEngelen merged commit 16ecb3e into ldc-developers:master Feb 13, 2018
@JohanEngelen JohanEngelen deleted the plugin branch February 13, 2018 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants