Prepare CaGe to be extensible via plugins#129
Conversation
jplehr
left a comment
jplehr
left a comment
There was a problem hiding this comment.
A few comments and please reformat with clang-format.
I don't think this complies with formatting rules.
tools/cage/src/CaGe.cpp
Outdated
|
|
||
| Plugin* loadPlugin(const std::string& pluginPath) { | ||
| metacg::MCGLogger::instance().getConsole()->debug("Loading plugin"); | ||
| void* handle = dlopen(pluginPath.c_str(), 1); |
There was a problem hiding this comment.
I believe we rely on LLVM anyway here, right?
Can you double check whether LLVM offers helpers to work with dlfcn? I vaguely recall there to be helper that we use(d) in libomptarget.
There was a problem hiding this comment.
Didn't know this existed, thank you.
Swapped for LLVM's helper functions.
This seems to work based on preliminary testing, but I am not sure if they have behave identically.
If I understood correctly, one can define a search order for which LLVM's helper function traverse possible candidates for getting symbols from libraries, which I have not yet found for the dlopen/dlsym version.
TimHeldmann
force-pushed
the
feat/CagePlugins
branch
from
337a7ac to
8118279
Compare
jplehr
left a comment
jplehr
left a comment
There was a problem hiding this comment.
Seems reasonable.
Do we have tests for it?
tools/cage/src/CaGe.cpp
Outdated
| return nullptr; | ||
| } | ||
| auto getPlugin = reinterpret_cast<Plugin* (*)()>(sym); | ||
| Plugin* loadedPlugin=getPlugin(); |
There was a problem hiding this comment.
I think this is still not correctly formatted.
There was a problem hiding this comment.
As far as I can tell, it is.
Does it format differently on your machine?
There was a problem hiding this comment.
I would expect it to be
| Plugin* loadedPlugin=getPlugin(); | |
| Plugin* loadedPlugin = getPlugin(); |
There was a problem hiding this comment.
Yes you are right, I don't know why I didn't get the formatter to do it for me the first time.
Changed
…d interface library for client tools to link against
TimHeldmann
force-pushed
the
feat/CagePlugins
branch
from
35a9fd2 to
caddb7c
Compare
We currently do not have tests as part of this Repository, but @ahueck and I have developed a plugin based on this here: https://github.com/TimHeldmann/ACGPlugin/tree/master. I want to touch up the CGC2 plugin interface and its accompanying test plugin with the learnings gathered from this PR anyway, so I propose I add the example plugin in the second PR? |
WFM |
2 participants
Similar to CGC2, we want users to be able to add custom collection and metadata logic to CaGe without modifying CaGe itself.
This PR prepares CaGe to be used with plugins.
It clears up the naming confunsion, by renaming CaGe's main file from Plugin, to CaGe.
This allows the filename Plugin to now refer to CaGe plugins.
We allow to add Plugins via commandline and load them at runtime.
We augment the existing CallGraphConsumer interface to add an option CallGraphAugmentation step.