- User Since
- Oct 23 2020, 1:49 AM (12 w, 5 d)
Dec 20 2020
Hi, could anyone help to commit this?
Dec 14 2020
Dec 9 2020
Dec 8 2020
(One idea we've been playing with is that traversal of the AST usually goes through TranslationUnitDecls::decls_begin(), which triggers loading from the preamble. We could add a stateful flag to DeclContext that causes this to behave like noload_decls_begin instead...)
Simplify the test case
Dec 7 2020
Use a vector to collect arguments
Dec 6 2020
Add tests to check if attributes are attached to AST
Dec 4 2020
Nov 27 2020
- code completion: this shouldn't run plugins I believe: latency is critical and we don't emit any diagnostics
- main file AST build: Important to run plugins here but it's critical they don't traverse the whole preamble (can degrade latency by orders of magnitude).
Well, Sound reasonable, but what if getTranslationUnitDecl() is indeed needed instead of using TraverseAST (I didn't came up with an example yet)? In this case I think clangd should allow a plugin to do its job, even if it is performance hostile.
- preamble build: this is tricky, we *rarely* emit diagnostics from this step, and it's very performance-sensitive. It's a separate action from the main file AST build so plugins won't be able to see the results of analysis. I think we shouldn't run plugins here.
I got a question, if "we *rarely* emit diagnostics from this step", what will happen if I open the source file of the header? Can I consider that the diagnostic from the header is not generated by creating PCH (Simply speaking preamble build do not generate diags, and diags from a header file do not generated from preamble build stage)? If so, I think no plugin loading in this step is fine.
- background indexing: again, no reason to run plugins here
Agree, but I got a question, what if a error diagnostic exist in the code, will the index procedure do?
- we need to ensure the traversal scope is already set by the time plugin hooks that may perform traversals (e.g. ASTConsumer::handleTranslationUnit) are called
I think it's hard to implement, and even so a plugin may still need to traversal out of scope, and I think we should let them do that.
This is exactly the problem, filenames specified in *clang* flags are *supposed* to be read from the VFS. (In practice this probably just means we'd need to disable this feature in environments where VFS is used)
I don't thinks so, plugins are loaded by using -fplugin=xxxx.so, this is a flag being parsed and saved in FrontendOptions::Plugins, which is a string vector. As you can see, xxx.so is treat as a string, not a file, vfs didn't load it to memory like normal files (for example main.cpp) . So I think whether VFS is used or not has nothing to do with plugin loading procedure.
Sure. TL;DR is: clangd flags are configured by the user, user can be fully responsible for security/stability.
clang flags are configured by the project. If they're bad, we can e.g. give bad diagnostics, but can't crash or compromise security.
In the simplest possible case, clangd is configured as follows:
- user downloads clangd binary
- user installs an LSP plugin for their editor, and configures the plugin to use /usr/bin/clangd for C++ files. clangd starts when the editor does
- the build system for $PROJECT generates $PROJECT/compile_commands.json
- when the user opens $PROJECT/src/foo.cpp in the editor, it notifies clangd. clangd searches for $PROJECT/compile_commands.json, finds the clang arguments, and uses them to parse foo.cpp
*clangd* command-line flags would be added explicitly by the user at step 2. We can reasonably ask the user to be aware/responsible for security/stability implications of doing this, including with their particular clangd version. We can also ask them to run clangd --check without the plugin flag to test whether the plugin is causing a stability problem.
*clang* command-line flags are added implicitly in step 3. Or they could simply be checked into the repository - nothing ensures they were generated locally by the build system. The point is in typical usage they are not controlled by the user directly, and from a security perspective are not trusted (as safely opening files from untrusted repos is a reasonable expectation). So if we're loading plugins based on instructions in clang command-line flags, clangd bears most of the responsibility for making sure that's safe and correct (and I don't see a way to do that).
Nov 26 2020
clangd (and other clang tools) get deployed in environments where all access to the filesystem goes through a llvm::vfs::Filesystem, and all filenames referred to in the compile command are within that virtual filesystem rather than the real one.
(Again, this isn't an issue if we require these paths to be specified on the *clangd* startup command line, as opposed to the clang flags that form the compile command)
Right, I understand (a little bit, at least!) what plugins *can* do. What I'm asking specifically is: this feature has a cost, how important is supporting it? Are there codebases where these attributes are widely used, and enforcement at development time is particularly valuable?
And I checked the CMakeList.txt for the driver, if CLANG_PLUGIN_SUPPORT is off, I guess the "undefined symbol` problem will also happens, I think this maybe one reason that option CLANG_PLUGIN_SUPPORT is as ON by default.
Nov 25 2020
Fix some syntax
Nov 23 2020
Nov 19 2020
@aaron.ballman That would be nice if your could help, and Yafei Liu <email@example.com> is okay.
Nov 18 2020
Simplify the test case
Nov 17 2020
use VerifyDiagnosticConsumer (-verify) instead of FileCheck for syntax only feature test.
remove illustration in ClangPlugins.rst (which is not very appropriate)
Nov 15 2020
Fix the grammar
Simplify the code logic
Simplify the test case
Nov 12 2020
I just enabled LLVM_ENABLE_DOXYGEN LLVM_BUILD_DOCS, and use make doxygen-clang, it seems no error with my newest patch, is this
fix grammer errors in warning messages and documents
Nov 11 2020
Nov 10 2020
Add more details in ClangPlugins.rst.
Re-design overridden usage checking.
Fix some code style issues.
Nov 9 2020
add final attribute checking