User Details
- User Since
- Dec 19 2016, 5:58 AM (213 w, 4 d)
Nov 28 2020
On bitcoin v0.18.1, there is an assertion introduced by this change.
The TU that can be used for reproduction is src/script/interpreter.cpp.
Assertion message:
CTU loaded AST file: /home/gamesh411/bitcoin/src/script/script.cpp clang: /home/gamesh411/llvm-project/clang/lib/AST/ASTContext.cpp:4411: clang::QualType clang::ASTContext::getInjectedClassNameType(clang::CXXRecordDecl*, clang::QualType) const: Assertion `NeedsInjectedC lassNameType(Decl)' failed.
Nov 25 2020
Nov 23 2020
Oct 31 2020
Just to make sure we're on the same page -- the current approach is not flow-sensitive, and so my concern is that it won't report any true positives (not that it will be prone to false positives).
Sorry about that. You are absolutely right; what I was trying to say is CallGraph-based.
Oct 30 2020
...
Have you run this check over any large code bases to see if it currently catches any true positive diagnostics?
Oct 19 2020
Oct 16 2020
Sep 28 2020
Do you have some thoughts about this, should this be pursued, or do you think the use-case is not relevant?
Sep 17 2020
Update commit message
Tidy up commit message
Update commit msg with example
Reformat diagnostic message
Use explicit name longjmp instead of jump function
Fix liberal auto inside Collector
Note that there are no negative test cases that assert that we do NOT report in case a custom or an anonymous namespace is used. For that I would need a small patch in the testing infrastructure.
Patch needed in check_clang_tidy.py:
--- a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py +++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py @@ -167,7 +167,7 @@ def run_test_once(args, extra_args): subprocess.check_output( ['FileCheck', '-input-file=' + temp_file_name, input_file_name, '-check-prefixes=' + ','.join(check_fixes_prefixes), - '-strict-whitespace'], + '-strict-whitespace', '--allow-empty'], stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: print('FileCheck failed:\n' + e.output.decode()) @@ -180,7 +180,7 @@ def run_test_once(args, extra_args): subprocess.check_output( ['FileCheck', '-input-file=' + messages_file, input_file_name, '-check-prefixes=' + ','.join(check_messages_prefixes), - '-implicit-check-not={{warning|error}}:'], + '-implicit-check-not={{warning|error}}:', '--allow-empty'], stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: print('FileCheck failed:\n' + e.output.decode()) @@ -195,7 +195,7 @@ def run_test_once(args, extra_args): subprocess.check_output( ['FileCheck', '-input-file=' + notes_file, input_file_name, '-check-prefixes=' + ','.join(check_notes_prefixes), - '-implicit-check-not={{note|warning|error}}:'], + '-implicit-check-not={{note|warning|error}}:', '--allow-empty'], stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: print('FileCheck failed:\n' + e.output.decode())
Add abort and terminate handling
Extend tests to cover every exit functions
Extract matcher bind labels
Sep 2 2020
only consider global and ::std scope handlers
Aug 31 2020
CReduce did not manage to produce any meaningful result after a week worth of runtime (more than ~2000 lines of code still remaining after reduction). We could track this down by tracing the ExprEngine code that assigns the Undefined SVal but that seems a huge effort as well. That could be done by debugging the SVal-assigning statements, and setting conditional breakpoints (ie. only break when the value is Undefined). When a breakpoint is hit, we could dump the statement that triggered it and try to reason about the conditions at that point. I also recommend using the rr tool as it allows you to use fixed pointer values while debugging.
Aug 14 2020
Thanks! LGTM now.
Aug 13 2020
Aside from infrastructural questions which I am not qualified ( nor particularly knowledgeable :3 ) to address, this looks good to me.
Aug 12 2020
Aug 4 2020
rename file name in header
I would add one more test for the undefined case. Like a local array variable that is uninitialized. That could mirror some of the null-dereference cases.
Aug 3 2020
rebase
I have updated the revision to use llvm::move algorithm (available thanks to @njames93).
Friendly ping :)
ensure lint in path
use llvm::move algorithm
arcanist misfire, see D83717
use llvm::move algorithm
Jul 21 2020
I experienced 2 crashes with and without this patch using commit 1af9fc82132da7c876e8f70c4e986cc9c59010ee on master:
I have used the clang built on that revision to analyse itself, and also used the patched version (with this current revision applied) to do the same.
To further investigate the moving story, I also created a small example on Godbolt:
https://godbolt.org/z/5qMf1o
Jul 17 2020
LGTM!
I am happy to see this change, as I could use it inside my clang-tidy check as well!
rename Offset -> Amount inside handleRandomIncrOrDecl
minor local variable renaming
apply review suggestions
- rename variables
- remove 1 assert
- rename tests
Thanks for reviewing this patch this quickly!
I have updated the diff according to your suggestions, but I will not land it till I run a llvm+clang analysis with it.
Do you think non-ctu mode is enough to test the stability?
It would be nice to see the measurements on llvm as this patch introduced some (IMO reasonable) asserts. Also in the unlikely case, there is an expression like 1 + iter, there could be more results.
rebase upon hotfix patch
extend test cases
add comments to non-obvious cases
move tests to one file
I am now experimenting with the suggestions. The other issues are on the worklist. Thanks for these so far.
Jul 16 2020
Jul 15 2020
@Eugene.Zelenko I have just rebase-d, and seen that the release notes page itself was bumped to clang-tidy 12. I have added my check as a new check there. Should I also add the other subsections (like improvements in existing checks, and new check aliases), or authors will add them as needed?
rebase
tidy up release notes, make all new check entries uniform
add missing newline in release notes
.
fix documentation header
@steakhal @Eugene.Zelenko Thanks for checking this patch! I have tried my best to adhere to your suggestions.
use move instead of copy
fix documentation issues
fix tests
Jul 14 2020
Thanks @njames93 :) I have extended the check with notes, but I have to figure out how to appease file-check, so its still WIP until I do.
extend with notes
apply minor fixes
tests are WIP until I figure out how to properly use file-check
Jul 13 2020
mention iterator header
use std::copy algorithm
polish
fix test diag location changes resulting from autoformatting
simplify VisitCallExpr
update includes
Jul 10 2020
In addition, IMHO the test infra should then be extended to throw an exception, when a feature is referenced that is not mentioned anywhere. That would be helpful in the future.
Good thing you were looking inside the test infra code to catch this, kudos!
Jul 9 2020
@Szelethus thanks for being watchful, appreciated c:
It will be interesting to see how different C and C++ projects will prove in terms of AST complexity, and Decl-size, but I understand that for now, these two options are necessary to not penalize C project analysis because of C++ AST complexity.
Jul 8 2020
apply review suggestions
Jul 7 2020
fix tidy diagnostic
The checker has been updated, the comments and the logic polished. I would still take a stab at this being a ClangSa checker (as opposed to being a Tidy check). What do you think?
fix detection logic
fix license header
add missing warning to test