[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

This revision was automatically updated to reflect the committed changes. Closed by commit rGd882750f1105: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in… (authored by OikawaKirie, committed by steakhal). Changed prior to commit:

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2715097 , @OikawaKirie wrote: > In D83660#2675064 , @mikhail.ramalho > wrote: > >> Indeed it looks like a copy & paste error, I'm surprised no one found it >> earlier. >> >>

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

OikawaKirie added a comment. In D83660#2675064 , @mikhail.ramalho wrote: > 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

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

mikhail.ramalho added a comment. 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

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2661646 , @OikawaKirie wrote: > 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.

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

OikawaKirie added a comment. In D83660#2661369 , @martong wrote: >> 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

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2661369 , @martong wrote: > 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 >

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added inline comments. Comment at: clang/test/Analysis/z3/Inputs/MockZ3_solver_check.c:6 + +// Mock implementation: return UNDEF for the 5th invocation, otherwise it just +// returns the result of the real invocation. martong wrote: > It's not clear why

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

martong accepted this revision. martong added a comment. 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

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added reviewers: martong, balazske, Szelethus, ASDenysPetrov. steakhal added a comment. Herald added a subscriber: rnkovacs. spam reviewers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83660/new/ https://reviews.llvm.org/D83660 ___

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. Lets see what others think about this. Im fine with it on my part. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83660/new/ https://reviews.llvm.org/D83660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

OikawaKirie updated this revision to Diff 333708. OikawaKirie added a comment. In D83660#2597025 , @steakhal wrote: > I'm somewhat bothered that this patch is not landed yet. :| > > I made a **crude** mock to trigger the bug using `LD_PRELOAD`. F15707493:

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. I'm somewhat bothered that this patch is not landed yet. :| I made a **crude** mock to trigger the bug using `LD_PRELOAD`. F15707493: LD_PRELOAD-workaround.patch The test reproduces the issue and passes if you apply the fix from

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2264963 , @OikawaKirie wrote: > After reviewing the code of this snippet, I think it would be very difficult > to make a regression test case for the crash, as far as what I know about Z3 > and SMT solvers. > > First

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

OikawaKirie added a comment. After reviewing the code of this snippet, I think it would be very difficult to make a regression test case for the crash, as far as what I know about Z3 and SMT solvers. First of all, all calls to `Solver->check()` will return `true` for sat, `false` for unsat,

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

OikawaKirie added a comment. We have tried to trigger the crash with the original project where the crash was encountered. But the problem is we cannot trigger the crash with the project, and we have lost all the previous records about this crash. Besides, both I and the bug reporter himself

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. Ping @OikawaKirie . How should we proceed? I would happily participate in creating a minimal repro for this, but I need at least one crash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83660/new/

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. It seems that this patch is stuck. How can I reproduce the crash? @OikawaKirie The closest I could come up with was: void clang_analyzer_eval(int); void clang_analyzer_printState(); void foo(int a, int b) { if (a > 5) return; if (b < 4)

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

vsavchenko added a comment. In D83660#2167391 , @NoQ wrote: > In D83660#2166917 , @steakhal wrote: > > > Although I'm not the most experienced one here, I think it's good to go. > > > Let's wait for the test to be

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

NoQ added a comment. In D83660#2166917 , @steakhal wrote: > Although I'm not the most experienced one here, I think it's good to go. Let's wait for the test to be added, sounds like it's close. (@OikawaKirie, you can use delta debugging tools like

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

NoQ added a comment. In D83660#2149470 , @OikawaKirie wrote: > It seems to be better to write a checker to report assertion failures. No no no. No no no no no. Assertions don't say "the program crashes if this happens", assertions say "this doesn't

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. Although I'm not the most experienced one here, I think it's good to go. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83660/new/

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2149509 , @OikawaKirie wrote: > > Might in the future I would spend some time on it - we will see. > > [...] If there is anything that our research group can do, you are free to > contact us. I would use the official

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

OikawaKirie added a comment. > Might in the future I would spend some time on it - we will see. @steakhal My boss always asks me about how to improve the performance for SMT solver based constraint solving, on both the engine side and the SMT solver side. If there is anything that our research

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

steakhal added a comment. In D83660#2148834 , @NoQ wrote: > Looks like a copy-paste error indeed. > > @OikawaKirie do you accidentally have a test case to reproduce the crash, so > that we could add it to our regression test suite? 'Cause traditionally

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

OikawaKirie added a comment. > do you accidentally have a test case to reproduce the crash @NoQ I am now working with the reporter of this bug to make a simple test case to trigger the crash. > looks like a good opportunity for `SmartPtr` checker and `llvm::Optional`! @vsavchenko It seems not

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

vsavchenko added a subscriber: vrnithinkumar. vsavchenko added a comment. In D83660#2148834 , @NoQ wrote: > Looks like a copy-paste error indeed. @vrnithinkumar @NoQ looks like a good opportunity for `SmartPtr` checker and `llvm::Optional`!

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

NoQ added reviewers: vsavchenko, steakhal. NoQ added a comment. Looks like a copy-paste error indeed. @OikawaKirie do you accidentally have a test case to reproduce the crash, so that we could add it to our regression test suite? 'Cause traditionally we add a test to every patch. Note that

[PATCH] D83660: [analyzer] Fix a crash for dereferencing an empty llvm::Optional variable in SMTConstraintManager.h.

OikawaKirie created this revision. OikawaKirie added reviewers: NoQ, dcoughlin. OikawaKirie added a project: clang. Herald added subscribers: cfe-commits, ASDenysPetrov, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. The first crash