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

only consider global and ::std scope handlers

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.

Thanks! LGTM now.

Aside from infrastructural questions which I am not qualified ( nor particularly knowledgeable :3 ) to address, this looks good to me.

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.

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

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

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.

In D83717#2155066, @njames93 wrote:

In D83717#2152762, @gamesh411 wrote:

In D83717#2150977, @njames93 wrote:

Alternatively you could do something like this, though it would be a pain https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-gmock.cpp#L86

I have bitten the bullet, and have gone down this route. With relative numbering, the sections themselves are at least translation-invariant. Not the prettiest sight, tho.
Thanks!

I almost think it would be nice FileCheck supported some directive like:

// LINE-NAME: <LINENAME>

And corresponding check lines:

[[<LINENAME>]]
[[<LINENAME>+N]] 
[[<LINENAME>-N]]

Would result in the ability to write checks like this:

void longjmp_handler() {// LINE-NAME: LONGJMP
    longjmp(env, 255); 
}

...

void foo(){
  atexit(longjmp_handler);
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: exit-handler potentially calls a jump function. Handlers should terminate by returning [cert-env32-c]
  // CHECK-NOTES: :[[LONGJMP]]:1: note: handler function declared here
  // CHECK-NOTES: :[[LONGJMP+1]]:3: note: jump function called here
}

Anyway that's a story for another day.

In D83717#2153246, @Eugene.Zelenko wrote:

In D83717#2153245, @gamesh411 wrote:

@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?

No, that sections will be filled when related changes will be made. Release Notes are reset to blank state in master after branching release.

@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.

In D83717#2150977, @njames93 wrote:

Alternatively you could do something like this, though it would be a pain https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-gmock.cpp#L86

use move instead of copy
fix documentation issues
fix tests

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

mention iterator header

use std::copy algorithm
polish

fix test diag location changes resulting from autoformatting

simplify VisitCallExpr

update includes

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!

@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.

apply review suggestions

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

implement traditional iterator support as well

In order to test the non-pointer iterators, an extension has to be made to the system header simulator. Please check the related patch (parent of this one in the stack) https://reviews.llvm.org/D83226.

rename misleading variables
apply local consts

modernize the memory modeling code, still WIP

make single bug-type
fix checker registration

move to core.alpha

change license

remove unrelated comment formatting