Szelethus (Umann Kristóf)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 19 2017, 6:59 AM (40 w, 2 h)

Recent Activity

Today

Szelethus updated the diff for D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

In this diff I

  • added a Pedantic flag that is set to false by default to filter out results from objects that don't have a single field initialized,
  • made it so that fields that are declared in system headers are now ignored,
  • refactored isFullyInitialized to hasUnintializedFields (it returned true when there were in fact uninit fields),
  • fixed everything mentioned in inline comments aside from the naming and the category,
  • added TODOs for FieldChainInfo::toString, I decided to fix those in a later patch to keep the diff just a little bit smaller,
  • added many more test cases, including tests for the Pedantic flag
  • added support for arrays. Granted, they worked wonderfully with the checker before, but there was nothing mentioned about them in the code.
Wed, Apr 25, 2:35 AM

Yesterday

Szelethus added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

I'd also like to point out that as I mentioned before, the checker's name itself is misleading (it is a leftover from an earlier implementation of this checker). Here are just some ideas I came up with:

  • UninitializedObjectChecker
  • UninitializedFieldsChecker
  • UninitializedFieldsAfterConstructionChecker
  • UninitializedMembersChecker
  • UninitializedMembersAfterConstructionChecker

Of these I like the first the most, but I'm open for anything, if you have an idea for it.

In D45532#1075789, @NoQ wrote:

Guys, what do you think about a checker that warns on uninitialized fields only when at least one field is initialized? I'd be much more confident about turning such check on by default. We can still keep a pedantic version.

Sounds good! I just finished implementing it along with a few minor (like some TODOs and fixes according to inline comments) and not-so-minor (like ignoring fields from system headers) changes. I'll update the diff and post results on it once I finish checking the LLVM/Clang project. I feel very confident about the upcoming version.

Tue, Apr 24, 10:08 AM
Szelethus added a comment to D45407: [StaticAnalyzer] Added notes to the plist output.

Can someone commit this for me? I don't have a write access.

Tue, Apr 24, 3:52 AM · Restricted Project

Tue, Apr 17

Szelethus updated the diff for D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Also, I managed to cause a crash with the class linked_ptr_internal from google's boringssl when I analyzed the grpc project. I'll look deeper into this, but I have a strong suspicion that the error lies within the CSA core.

Tue, Apr 17, 4:44 AM

Mon, Apr 16

Szelethus updated the diff for D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Would be interesting to extend this checker (maybe in an upcoming patch) to report on uninitialized members not only in constructors, but also copy constructors and move constructors.

Added 3 new test cases. to cover them. Interestingly, move constructors don't emit any warnings - the core can only assert that the fields after a move construction are valid (returns true for Val::isValid()`).
Came to think of it, I'm not 100% confident in the checkers name. It could be misleading, as this checker doesn't check constructors, but rather objects after construction. The end of a constructor call is only the point at which we know that analysis can be done.

This bug report also mentions assignment operator. But for that a warning may be not so useful. In that case the members of the assigned to object should have some initialized value already which the programmer may not want to overwrite in the assignment operator.

I believe there's a checker for that already, but I'm really not sure whether UndefinedAssignmentChecker covers all such cases.

Mon, Apr 16, 6:23 AM
Szelethus added a comment to D45407: [StaticAnalyzer] Added notes to the plist output.

I just had a look with -analyzer-config notes-as-events=true, and it works (with CodeChecker, at least). I'd still implement a better support for notes, but this is a great workaround for now. Thanks!

Mon, Apr 16, 3:00 AM · Restricted Project

Sun, Apr 15

Szelethus added a comment to D45407: [StaticAnalyzer] Added notes to the plist output.

Btw, what sort of UI are you trying to make these extra note pieces of mine work with?

I'm also developing a checker: https://reviews.llvm.org/D45532. This checker emits very important information in notes. When I tried testing it with Ericsson's CodeChecker, I realized that notes aren't displayed. I'm planning to extend it so it will be able to.

Sun, Apr 15, 11:29 AM · Restricted Project
Szelethus added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Note that there was a comment made about the test files being too long. I still haven't split them, as I didn't find a good "splitting point". Is this okay, or shall I try to split these into numerous smaller ones?

Sun, Apr 15, 11:24 AM
Szelethus updated the diff for D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Among many other things:

  • The checker class is now on top of the file.
  • Reviewed all comments, fixed typos, tried to make the general idea more understandable.
  • Removed all (at least, all I could find) unnecessary functions and function arguments.
  • Removed support for unions entirely.
Sun, Apr 15, 11:17 AM
Szelethus added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

I'm about to update the diff, I changed a quite a lot of stuff, so I'm not sure that I'd be able to respond to these inline comments.

Sun, Apr 15, 10:51 AM

Thu, Apr 12

Szelethus added inline comments to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
Thu, Apr 12, 10:11 AM
Szelethus added a comment to D45407: [StaticAnalyzer] Added notes to the plist output.

Did you have some time to check on those programs? :)

Thu, Apr 12, 8:31 AM · Restricted Project
Szelethus added inline comments to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
Thu, Apr 12, 8:29 AM
Szelethus added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Thank you for all your comments so far! I'll probably only be able to update the diff tomorrow (with me being in the GMT + 1 timezone).

Thu, Apr 12, 8:27 AM

Wed, Apr 11

Szelethus added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

I wasn't too clear on this one: unknown fields are regarded as uninitialized, what I wrote was a temporary change in the code so I could check whether it would work.

Wed, Apr 11, 12:17 PM
Szelethus added a comment to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
In D45532#1064652, @NoQ wrote:

In most cases, all fields of a union is regarded as unknown. I checked these cases by regarding unknown fields as uninitialized.

The whole point of making them unknown, as far as i understand, was to suppress uninitialized field warnings. So i feel what we're doing here is a bit upside down.

Wed, Apr 11, 11:53 AM
Szelethus added inline comments to D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
Wed, Apr 11, 11:50 AM
Szelethus updated the diff for D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.

Reuploaded the diff with full context.

Wed, Apr 11, 11:19 AM
Szelethus created D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call.
Wed, Apr 11, 11:06 AM

Sun, Apr 8

Szelethus added a comment to D45407: [StaticAnalyzer] Added notes to the plist output.

Thanks!

Sun, Apr 8, 1:13 AM · Restricted Project

Sat, Apr 7

Szelethus created D45407: [StaticAnalyzer] Added notes to the plist output.
Sat, Apr 7, 1:34 PM · Restricted Project

Feb 14 2018

Szelethus added a comment to D43120: [clang-tidy] New checker for exceptions that are created but not thrown.

Just noticed that I can't mark your inline comment as done, since the file got renamed. The typo is also fixed (Classname -> Class name).

Feb 14 2018, 1:54 AM · Restricted Project
Szelethus updated the diff for D43120: [clang-tidy] New checker for exceptions that are created but not thrown.

Renamed the checker from misc-throw-keyword-missing to bugprone-throw-keyword-missing.

Feb 14 2018, 1:52 AM · Restricted Project

Feb 13 2018

Szelethus updated the diff for D43120: [clang-tidy] New checker for exceptions that are created but not thrown.
Feb 13 2018, 1:28 AM · Restricted Project

Feb 12 2018

Szelethus added inline comments to D43120: [clang-tidy] New checker for exceptions that are created but not thrown.
Feb 12 2018, 7:53 AM · Restricted Project
Szelethus updated the diff for D43120: [clang-tidy] New checker for exceptions that are created but not thrown.

Fixed almost everything mentioned in comments. The warning message now also contains the object's type (more on that in an inline comment).

Feb 12 2018, 7:43 AM · Restricted Project

Feb 9 2018

Szelethus updated the diff for D43120: [clang-tidy] New checker for exceptions that are created but not thrown.
Feb 9 2018, 6:16 AM · Restricted Project
Szelethus added inline comments to D43120: [clang-tidy] New checker for exceptions that are created but not thrown.
Feb 9 2018, 6:06 AM · Restricted Project
Szelethus updated the diff for D43120: [clang-tidy] New checker for exceptions that are created but not thrown.

Changes made according to @whisperity's comments.

Feb 9 2018, 6:06 AM · Restricted Project
Szelethus added a comment to D5767: Template Instantiation Observer + a few other templight-related changes.
  1. What do you mean by regression tests? We have run the clang-test target successfully on the patched code (which has the hook). Note that the hook this pull request provides is implemented as a ProgramAction. It is not intended to be used together with other program actions, I don't see how we could turn it on for other tests (if that is what you referred to).

I would bet the sema tests are full of tricky edge cases. So running templight on those tests would be a helpful exercise. I am only interested in assertion fails, so we do not need to check the output. One option to do so would be to add the -templight-dump option to the %clang_cc1 variable when running the tests. Note that the tests are likely to fail because the output will change, but if there are no crashes, it is fine.

Feb 9 2018, 5:56 AM
Szelethus added reviewers for D43120: [clang-tidy] New checker for exceptions that are created but not thrown: alexfh, aaron.ballman.
Feb 9 2018, 4:11 AM · Restricted Project
Szelethus created D43120: [clang-tidy] New checker for exceptions that are created but not thrown.
Feb 9 2018, 4:03 AM · Restricted Project

Feb 8 2018

Szelethus added inline comments to D5767: Template Instantiation Observer + a few other templight-related changes.
Feb 8 2018, 6:45 AM