Page MenuHomePhabricator

steakhal (Balázs Benics)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 7 2019, 1:49 AM (119 w, 15 h)

Recent Activity

Yesterday

steakhal added a comment to D103967: [Analyzer][solver] Add dump methods for (dis)equality classes..

I suspect that the exploded-graph-rewriter should be updated as well so that the HTML dumps also carry this information.

Wed, Jun 16, 9:49 AM · Restricted Project

Fri, Jun 4

steakhal added inline comments to D97699: [analyzer] Add InvalidPtrChecker.
Fri, Jun 4, 2:27 PM · Restricted Project
steakhal added a comment to D97699: [analyzer] Add InvalidPtrChecker.

Overall I think it's a useful checker not only for checking the getenv() but a bunch of other functions as well, which might return a pointer to a statically allocated buffer.
The implementation could be polished a bit but it's ok I think.

Fri, Jun 4, 1:16 AM · Restricted Project
steakhal added a comment to D103096: [analyzer] Implement cast for ranges of symbolic integers..

There is so much fancy stuff going on upstream. Awesome to see.
I'm trying to catch up ASAP, I'm finally done with my master's thesis.

Fri, Jun 4, 12:51 AM · Restricted Project

Wed, Jun 2

steakhal added a comment to D103511: [clang-tidy] Special member functions check should allow sole dtors by default.

I think the C++ Core Guideline wording is... confusing. The rule title is C.21: If you define or =delete any copy, move, or destructor function, define or =delete them all, which I take literally. Using = default defines the function, so:

Okay, then I'm going to abandon this in a few days if nothing interesting shows up.
Thanks for the quick response!

Wed, Jun 2, 5:22 AM · Restricted Project
steakhal requested review of D103511: [clang-tidy] Special member functions check should allow sole dtors by default.
Wed, Jun 2, 3:15 AM · Restricted Project

Tue, May 25

steakhal added a comment to D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

I've decided to land this without the test.
Thank you for your contribution.

Tue, May 25, 1:32 AM · Restricted Project
steakhal committed rGd59b4acf80d5: [analyzer][ctu] Reland "Avoid parsing invocation list again and again.. (authored by OikawaKirie).
[analyzer][ctu] Reland "Avoid parsing invocation list again and again..
Tue, May 25, 12:45 AM
steakhal added a reverting change for rGdb8af0f21dc9: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand…: rGf05b70c23687: Revert "[analyzer][ctu] Avoid parsing invocation list again and again during on….
Tue, May 25, 12:31 AM
steakhal committed rGf05b70c23687: Revert "[analyzer][ctu] Avoid parsing invocation list again and again during on… (authored by steakhal).
Revert "[analyzer][ctu] Avoid parsing invocation list again and again during on…
Tue, May 25, 12:31 AM
steakhal added a reverting change for D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU: rGf05b70c23687: Revert "[analyzer][ctu] Avoid parsing invocation list again and again during on….
Tue, May 25, 12:31 AM · Restricted Project
steakhal committed rGdb8af0f21dc9: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand… (authored by OikawaKirie).
[analyzer][ctu] Avoid parsing invocation list again and again during on-demand…
Tue, May 25, 12:21 AM
steakhal closed D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.
Tue, May 25, 12:21 AM · Restricted Project

Mon, May 24

steakhal accepted D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

It seems like everything passes. Yeey, good job!
Shall I commit this tomorrow on your behalf?

Mon, May 24, 11:44 AM · Restricted Project
steakhal added a comment to D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

Could you please reupload your diff to retrigger the bots?
I'd like to make sure that everything passes, even the tests that do not belong to you.
Unfortunately, I don't know any better way of retriggering the pre-merge checks - I don't have permission to manually retrigger builds.

Mon, May 24, 9:22 AM · Restricted Project
steakhal committed rG058f384ae94a: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr (authored by tomasz-kaminski-sonarsource).
[analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr
Mon, May 24, 1:21 AM
steakhal closed D102835: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr.
Mon, May 24, 1:20 AM · Restricted Project

Fri, May 21

steakhal added a comment to D102696: [Analyzer] Find constraints that are directly attached to a BinOp.

I think It would be also beneficial to document the semantics. Whenever we say $x + $y it's not entirely clear whether we talk about the addition operation in a mathematical sense or we follow C/C++ language semantics. Right now it's mostly mixed within the analyzer. It would be really nice to see for example how and when wrapping is considered.
What if the types of the operands don't match in bitwidth or signess. This is sort of the reason why ArrayBoundV2 has false positives in some scenarios.

Fri, May 21, 10:24 AM · Restricted Project
steakhal accepted D102835: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr.

I do not have commit rights. If there are no additional changes needed, would it be possible to you to commit the changes on my behalf?

Fri, May 21, 9:35 AM · Restricted Project
steakhal committed rG3febf0b507e6: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks (authored by steakhal).
[analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks
Fri, May 21, 2:36 AM
steakhal closed D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks.
Fri, May 21, 2:36 AM · Restricted Project
steakhal updated the diff for D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks.

Nothing changed, I'm just retriggering the pre-merge bots as my parent revision landed.

Fri, May 21, 12:15 AM · Restricted Project

Thu, May 20

steakhal added a comment to D102835: [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr.

Looks good to me.

Thu, May 20, 5:10 AM · Restricted Project

Wed, May 19

steakhal added a comment to D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file.

There are quite a few places where the extdef mappings should be updated.
For discovering them I suggest you asserting the new file format (only for detecting them!). This way if you miss one, it wouldn't silently 'work' somehow, but raise your attention.

Wed, May 19, 7:11 AM · Restricted Project
steakhal added a comment to D102149: [analyzer][ctu] Allow loading invocation list from a compilation database automatically detected from the ctu-dir.
Wed, May 19, 7:10 AM · Restricted Project
steakhal updated the diff for D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks.

Introduce the test fixture, and use its setUp() method for implementing this GTEST_SKIP stuff.

Wed, May 19, 3:23 AM · Restricted Project

May 18 2021

steakhal added a comment to D102640: [ASTimporter] Remove decl from lookup only if it has decl context.

This is a good catch, thank you for fixing this!

May 18 2021, 12:45 AM · Restricted Project
steakhal committed rG88ee91cd8779: [ASTimporter] Remove decl from lookup only if it has decl context (authored by steakhal).
[ASTimporter] Remove decl from lookup only if it has decl context
May 18 2021, 12:44 AM
steakhal closed D102640: [ASTimporter] Remove decl from lookup only if it has decl context.
May 18 2021, 12:44 AM · Restricted Project
steakhal added a reviewer for D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file: balazske.

I don't really like having multiple files with the same name.
And the importer TU should be simple to be simply cat-ed into a temporal file.
At that point, you could put the importee's content into this file. It would result in a single, self-contained test case.

May 18 2021, 12:40 AM · Restricted Project
steakhal updated the diff for D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks.

now it should build

May 18 2021, 12:16 AM · Restricted Project

May 17 2021

steakhal requested review of D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks.
May 17 2021, 11:29 AM · Restricted Project
steakhal committed rGebcf030efc5e: [analyzer] Engine: fix crash with SEH __leave keyword (authored by AbbasSabra).
[analyzer] Engine: fix crash with SEH __leave keyword
May 17 2021, 11:12 AM
steakhal closed D102280: [analyzer] Engine: fix crash with SEH __leave keyword.
May 17 2021, 11:12 AM · Restricted Project
steakhal added a comment to D102136: [NewPM] Add C bindings for new pass manager.

[NewPM] Remove documentation comments for parameters that were refactored into LLVMPassBuilderOptionsRef

I think it doesn't build. Could you check if this is the case? @aeubanks

May 17 2021, 10:56 AM · Restricted Project
steakhal requested review of D102640: [ASTimporter] Remove decl from lookup only if it has decl context.
May 17 2021, 10:35 AM · Restricted Project
steakhal accepted D102280: [analyzer] Engine: fix crash with SEH __leave keyword.

I think it's good to go. Thank you!

May 17 2021, 1:49 AM · Restricted Project

May 15 2021

steakhal added a comment to D102280: [analyzer] Engine: fix crash with SEH __leave keyword.

Is it a requirement for you to build them on Linux? Maybe you can try to build them on windows with clang-cl?

It is a hard requirement. I'm gonna think more about this in the far future I guess. Thanks for the insight though.

May 15 2021, 6:08 AM · Restricted Project

May 14 2021

steakhal added a comment to D102280: [analyzer] Engine: fix crash with SEH __leave keyword.

Do you have any good (mature, big enough) open-source projects for these msvc constructs?

https://github.com/microsoft/winfile and https://github.com/chakra-core/ChakraCore seem to be using SEH but not heavily.

Well, and how can I build them on Linux xD
I tried mingw64, but failed :D

May 14 2021, 10:27 AM · Restricted Project
steakhal added a comment to D102280: [analyzer] Engine: fix crash with SEH __leave keyword.

Do you have any good (mature, big enough) open-source projects for these msvc constructs?

May 14 2021, 3:38 AM · Restricted Project

May 13 2021

steakhal accepted D102240: [analyzer][solver] Prevent use of a null state.

I couldn't transform c == b into a condition, so it crashes.

Well, that's interesting. It might worth investigating. @NoQ ?
Regardless, it's an improvement, let's land it :D

May 13 2021, 9:29 AM · Restricted Project

May 12 2021

steakhal added a comment to D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference.

By checking the line coverage of the LoopUnrolling.cpp test file, looks like all lines are covered you touched.

May 12 2021, 4:52 AM · Restricted Project
steakhal added reviewers for D102280: [analyzer] Engine: fix crash with SEH __leave keyword: NoQ, vsavchenko, steakhal.

Please, add these reviewers for your upcoming [analyzer] patches.
Inline a couple of nits.
Nice to see some fixes for visual c++ stuff.

May 12 2021, 12:43 AM · Restricted Project

May 11 2021

steakhal added a comment to D99260: [analyzer] Fix false positives in inner pointer checker (PR49628).
In D99260#2704102, @NoQ wrote:

In https://bugs.llvm.org/show_bug.cgi?id=45786 the godbolt link shows that there are still problems with addressof (yes, their "trunk" clang is fresh enough). They seem to have __addressof instead of addressof so maybe we should cover that case real quick. Or maybe outright suppress reference invalidation on double-underscore functions because the checker generally relies on the number of standard functions that don't invalidate references while taking a non-const reference being very limited but this limit definitely doesn't take double-underscore functions into account.

May 11 2021, 1:20 PM · Restricted Project
steakhal added reviewers for D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference: NoQ, vsavchenko, steakhal.

I'm gonna have a look at this tomorrow.

May 11 2021, 1:16 PM · Restricted Project
steakhal added a comment to D102240: [analyzer][solver] Prevent use of a null state.

Dam, we will be always haunted by these.
Looks good btw.
The test looks somewhat artificial - like a raw creduce result.

May 11 2021, 9:25 AM · Restricted Project

May 8 2021

steakhal added a comment to D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

I do not know why the test case always fails on the build server, it runs perfectly on my computer. : (

I've locally run your patch and seemed good to me, I regret not having a look at the actual build bot.

May 8 2021, 3:02 AM · Restricted Project

May 7 2021

steakhal added a comment to D102062: [analyzer][ctu] Append ctu-dir to ctu-invocation-list for non-absolute paths.

On second thought the current behavior is reasonable.
We call clang from a command line, so I think it's fair to expect that relative paths are resolved using the CWD.
AFAIK if one does not supply the ctu-invocation-list, then it would be substituted by ctu-dir/invocations.yaml anyway.

May 7 2021, 4:41 AM · Restricted Project
steakhal accepted D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

I would love to see PreviousParsingResult combined with InvocationList using a llvm::Error. I'm pretty sure it can be done.
Regardless, I think it's already better than it was.

May 7 2021, 4:37 AM · Restricted Project
steakhal added a comment to D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

I would rather match on the complete line (except the file descriptor ofc).

May 7 2021, 1:55 AM · Restricted Project

May 6 2021

steakhal added a comment to D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

Okay, so you 'just' want an indication for the given open call. What about using strace?
strace -e trace=openat %clang_cc1 ... 2>&1 | grep '"invocations.yaml"' | FileCheck %s

May 6 2021, 3:57 AM · Restricted Project
steakhal added a comment to D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

Overall, it looks promising. But I don't quite get this test.
There is no invocation yaml in the temp directory. So, you are probably not testing the right thing.
You wanted to test if the invocation yaml exists, and could be opened but the parsing fails.
You should demonstrate that when a parsing error happens, the error code has recoded and it won't try to reparse the invocation yaml again and again.

May 6 2021, 2:58 AM · Restricted Project

May 4 2021

steakhal committed rGd882750f1105: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in… (authored by OikawaKirie).
[analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in…
May 4 2021, 7:50 AM
steakhal closed D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h..
May 4 2021, 7:50 AM · Restricted Project

May 3 2021

steakhal added a comment to D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU.

Awesome! Seems good to me. Though I've got limited experience on CTU stuff.
It would be nice to have tests, but it seems pretty hard to come up with one for this. Given that this is just a 'performance' issue, I'm fine with it.
Somehow try to check if this resolved your original concern.

May 3 2021, 10:47 AM · Restricted Project
steakhal added inline comments to D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.
May 3 2021, 4:28 AM · Restricted Project

May 1 2021

steakhal added a comment to D101635: [analyzer] Fix assertion in SVals.h.

I don't know how did we miss this. I run your patch on several projects and it seemed good. Does anyone have an idea how to prevent such a silly mistake from happening again? I was thinking of coverage data, but that wouldn't be enough for this example.

May 1 2021, 4:13 AM · Restricted Project

Apr 29 2021

steakhal added inline comments to D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.
Apr 29 2021, 7:43 AM · Restricted Project

Apr 26 2021

steakhal added a comment to D101041: [analyzer] Find better description for tracked symbolic values.

This is an improvement! Good job.
I had no time reviewing this, but I think it's already in a pretty good shape.

Apr 26 2021, 4:54 AM · Restricted Project
steakhal added a comment to D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h..

Indeed it looks like a copy & paste error, I'm surprised no one found it earlier.

Regarding the tests, we used to have make check-clang-analysis-z3 (or something similar) that would run only the analyzer's tests, but using Z3 as the constraint solver. It looks like this change broke it: https://reviews.llvm.org/D62445

It might worth investigating this build 'target'.

Apr 26 2021, 1:45 AM · Restricted Project

Apr 20 2021

steakhal added a comment to D97183: [analyzer] Add NoteTag for smart-ptr get().

For the following function:

void foo(std::unique_ptr<A> P) {
  A* praw = P.get();
  A* other = praw;
  if (other) {}
  P->foo();
}

Where do we expect a note? Where praw is initialized, where other is initialized or both?

I would expect no notes at all, since there is no bug.

According to the existing analyzer logic, there is a bug. If you check other for null, we can conclude that there are circumstances when it is null indeed.

I think we can conclude that P must be non-null (since it was unconditionally dereferenced), thus the previous check on the inner pointer and the branch it guards must be dead! This fact deserves a report, you are right. My bad.

Apr 20 2021, 6:02 AM · Restricted Project
steakhal updated the diff for D100829: [analyzer][docs] Highlight some differences between ArrayBound and V2.

Add 'Limitations and bugs' section with a false-positive example.
It would also help users classifying certain types of false-positive reports.

Apr 20 2021, 2:45 AM · Restricted Project
steakhal added inline comments to D100829: [analyzer][docs] Highlight some differences between ArrayBound and V2.
Apr 20 2021, 1:58 AM · Restricted Project
steakhal requested review of D100829: [analyzer][docs] Highlight some differences between ArrayBound and V2.
Apr 20 2021, 1:55 AM · Restricted Project

Apr 19 2021

steakhal added a comment to D97183: [analyzer] Add NoteTag for smart-ptr get().

For the following function:

void foo(std::unique_ptr<A> P) {
  A* praw = P.get();
  A* other = praw;
  if (other) {}
  P->foo();
}

Where do we expect a note? Where praw is initialized, where other is initialized or both?

I would expect no notes at all, since there is no bug.

Apr 19 2021, 12:12 PM · Restricted Project
steakhal added a comment to D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine.

I really want to move this forward so I made a further evaluation on this, on the MongoDB project.
The analysis took approx. 22 and half hours on 24 cores for the baseline and for this revision as well.

Apr 19 2021, 4:21 AM · Restricted Project

Apr 8 2021

steakhal added a comment to D99260: [analyzer] Fix false positives in inner pointer checker (PR49628).

I'm still not satisfied with the addressof, but I won't block this either.

Apr 8 2021, 9:52 AM · Restricted Project

Apr 7 2021

steakhal accepted D99262: [analyzer] Fix dead store checker false positive.

Looks good. Thank you.

Apr 7 2021, 6:14 AM · Restricted Project
steakhal abandoned D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions.
In D99658#2671747, @NoQ wrote:

I mean, the extent of an ElementRegion is the size of a single element. The reason why our intrinsic isn't doing what you expect is because we represent the pointer with offset as ElementRegion regardless of whether operator [] was used. On the other hand, an SVal doesn't ever represent a region at all, it only points to its first byte, regardless of the structure of MemRegion inside it; for that reason clang_analyzer_getExtent() is impossible to implement correctly in our current model.

So i think both behaviors are incorrect but if you think it makes it easier to write tests then absolutely go for it!

Oh, I got it. Hm, yes, both are wrong xD

Apr 7 2021, 4:06 AM · Restricted Project

Apr 6 2021

steakhal abandoned D99659: [analyzer][taint] Extent of heap regions should get taint sometimes.

Obsoleted by D69726.

Apr 6 2021, 9:14 AM · Restricted Project
steakhal requested review of D99959: [analyzer][NFC] Add tests for extents.
Apr 6 2021, 8:00 AM · Restricted Project
steakhal added a comment to D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions.
In D99658#2665730, @NoQ wrote:

What about clang_analyzer_getExtent(&x[2]) then?

I guess (extent of heap segment that starts at symbol of type 'int *' conjured at statement 'new int [ext]') - 8 is the value I expect - which is the size of the remaining part of the memory region from x+2 in bytes.
We can argue about the semantics of this debug intrinsic or the name of it though.

Apr 6 2021, 7:32 AM · Restricted Project

Apr 1 2021

steakhal accepted D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC)..
Apr 1 2021, 12:56 AM · Restricted Project

Mar 31 2021

steakhal updated the diff for D99659: [analyzer][taint] Extent of heap regions should get taint sometimes.

Add a FIXME about placing a NoteTag describing why the extent was getting tainted.

Mar 31 2021, 10:03 AM · Restricted Project
steakhal added a comment to D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h..

Can we automatically enable all test cases requiring z3 if clang is built with z3? I do not think the patch D83677 really make the problem fixed.

Z3 constraint manager is unsupported, thus no test runs those parts. And yea, crashes more often than not.

Mar 31 2021, 10:01 AM · Restricted Project
steakhal updated the diff for D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions.

Fix comments.
I could not manage to create an unknown extent, where the behavior would diverge.

Mar 31 2021, 9:50 AM · Restricted Project
steakhal added a comment to D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h..

I went through the change and it looks good, seems like this is indeed a copy paste error from line 132.
I checked the related conversation, and thanks for all the effort spent with the test.

BTW, I was obstructed by the z3 requirement in the regression test case when I tried to understand your test case. Even though I set the variables LLVM_Z3_INSTALL_DIR and LLVM_ENABLE_Z3_SOLVER during CMake, and the CSA can be correctly compiled with Z3, I still cannot make the test case run during make check-all. Therefore, this case was manually executed with llvm-lit -DUSE_Z3_SOLVER=1. Could you please tell me how to enable Z3 during llvm-lit.

I am a bit unhappy that we cannot run the test automatically, but maybe in the future.
@steakhal, https://reviews.llvm.org/D83677 seems to be related, should we push that?

I wouldn't mind fixing that yes, but right now I'm busy with other things.
Let's come back to it later.

Mar 31 2021, 8:21 AM · Restricted Project
steakhal planned changes to D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions.

Ah, I see you need nonloc::SymbolVal in your next patch, and getDynamicSizeWithOffset returns an Unknown if the extend is symbolic.

Anyway, I still feel misleading that clang_analyzer_getExtent does not handle the offset. Could we change getDynamicSizeWithOffset to return with a symbolic offset instead unknown without regression?

I'm gonna investigate. Thank you all for the snappy review!

Mar 31 2021, 7:39 AM · Restricted Project
steakhal added inline comments to D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h..
Mar 31 2021, 7:20 AM · Restricted Project
steakhal requested review of D99659: [analyzer][taint] Extent of heap regions should get taint sometimes.
Mar 31 2021, 6:31 AM · Restricted Project
steakhal requested review of D99658: [analyzer] Fix clang_analyzer_getExtent for heap regions.
Mar 31 2021, 6:02 AM · Restricted Project
steakhal added inline comments to D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC)..
Mar 31 2021, 5:41 AM · Restricted Project
steakhal added inline comments to D99576: [ASTImporter][NFC] Improve test coverage.
Mar 31 2021, 3:51 AM · Restricted Project
steakhal added inline comments to D99576: [ASTImporter][NFC] Improve test coverage.
Mar 31 2021, 3:49 AM · Restricted Project
steakhal added inline comments to D99576: [ASTImporter][NFC] Improve test coverage.
Mar 31 2021, 3:28 AM · Restricted Project
steakhal added reviewers for D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.: martong, balazske, Szelethus, ASDenysPetrov.

spam reviewers

Mar 31 2021, 1:54 AM · Restricted Project
steakhal added reviewers for D89649: Fix __has_unique_object_representations with no_unique_address: erichkeane, rnk, majnemer, EricWF.

I'm adding a couple of reviewers to get this going.

Mar 31 2021, 12:35 AM · Restricted Project
steakhal accepted D99181: [analyzer] Fix crash on spaceship operator (PR47511).
Mar 31 2021, 12:32 AM · Restricted Project

Mar 30 2021

steakhal accepted D98502: [clang][Checkers] Extend PthreadLockChecker state dump (NFC)..

Everything looks fine.
I like that regexp matcher so much. <3

Mar 30 2021, 4:06 AM · Restricted Project
steakhal accepted D99344: [Analyzer] Track RValue expressions.

land it

Mar 30 2021, 3:50 AM · Restricted Project
steakhal requested review of D99576: [ASTImporter][NFC] Improve test coverage.
Mar 30 2021, 3:34 AM · Restricted Project
steakhal added inline comments to D99344: [Analyzer] Track RValue expressions.
Mar 30 2021, 1:44 AM · Restricted Project

Mar 28 2021

steakhal added a comment to D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h..

Lets see what others think about this.
Im fine with it on my part.

Mar 28 2021, 7:49 AM · Restricted Project

Mar 26 2021

steakhal accepted D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast.

I don't know. I think it's already way better than it was.
I think we can reiterate this later.

Mar 26 2021, 3:25 AM · Restricted Project
steakhal added a comment to D99344: [Analyzer] Track RValue expressions.

Some minor logical issues inline.

Mar 26 2021, 3:18 AM · Restricted Project

Mar 25 2021

steakhal added a comment to D99344: [Analyzer] Track RValue expressions.

I really like it. Looks good.
I'm letting someone else accept this as I've not really touched the trackExpression parts.

Mar 25 2021, 8:57 AM · Restricted Project
steakhal added inline comments to D97183: [analyzer] Add NoteTag for smart-ptr get().
Mar 25 2021, 5:24 AM · Restricted Project
steakhal added a comment to D99260: [analyzer] Fix false positives in inner pointer checker (PR49628).

I recommend splitting this into two. I would happily accept the part about std::data().

Mar 25 2021, 5:09 AM · Restricted Project
steakhal added a comment to D97183: [analyzer] Add NoteTag for smart-ptr get().

@NoQ, why does the following trigger a null-dereference warning? (https://godbolt.org/z/Kxox8qd16)

void g(std::unique_ptr<A> a) {
  A *aptr = a.get();
  if (!aptr) {}
  a->foo();
}

When a->foo() is called, the constraint !aptr is no longer valid and so InnerPointerVal corresponding to a is no longer constrained to be null.
Am I missing something?

When the if's condition is evaluated, it probably triggered a state split. On one path the aptr (aka. the inner pointer) will be constrained to null.
The only way to be sure is by checking the exploded graph and see where it goes.

Mar 25 2021, 4:54 AM · Restricted Project

Mar 24 2021

steakhal added a comment to D99262: [analyzer] Fix dead store checker false positive.

I see your point.

Would it report this issue?

int test() {
  struct Foo foo = {0, 0}; // I would expect a warning here, that 'foo' was initialized but never read.
  (void)foo;
 return 0;
}

Only nits besides this.

We got one big fat complaint about that, and I can see the point.

We should at least document it as testcase.

Mar 24 2021, 12:26 PM · Restricted Project