Page MenuHomePhabricator

martong (Gabor Marton)
Codasip

Projects

User does not belong to any projects.

User Details

User Since
Oct 10 2017, 8:01 AM (285 w, 2 d)

Recent Activity

Feb 13 2023

martong added a comment to D142725: [DFAJumpThreading] Enable DFAJT with LTO.

Ping.
@fhahn Could you please take a look, if you have some time? Would be great to hear your thoughts.

Feb 13 2023, 2:53 AM · Restricted Project, Restricted Project

Feb 1 2023

martong added a comment to D142725: [DFAJumpThreading] Enable DFAJT with LTO.

Hey guys, thank you for the review so far. But I am not sure if we have a full consensus yet.
@nikic Do you think that the arguments above are reasonable for this change?

Feb 1 2023, 12:51 AM · Restricted Project, Restricted Project

Jan 27 2023

martong added a comment to D142725: [DFAJumpThreading] Enable DFAJT with LTO.

@martong Would the optimization also work if -Xclang -enable-dfa-jump-thread were specified in CFLAGS instead? If "yes", then I'd say this is firmly in the realm of user error and we should not make this change.

Yes, that would work with -mllvm -enable-dfa-jump-thread. And the Coremark score is roughly the same (within measurement error). I agree that it would be better if the user added the DFAJT optimization to CFLAGS rather than to the LDFLAGS, but do we really expect the users to have such a subtle knowledge about the opt passes? In particular, it might be surprising that this opt does not have any effect when it is used with LTO at the moment.

This is an internal LLVM option, it is not intended for end-user consumption. If somebody does want to use it, they had better know what they are doing.

The DFA jump threading pass is intended to be enabled by default in the future, at which point no option will be necessary, and this will just leave behind an (to our knowledge) unnecessary pass run in the LTO pipeline.

Jan 27 2023, 11:08 AM · Restricted Project, Restricted Project
martong updated martong.
Jan 27 2023, 9:15 AM
martong updated martong.
Jan 27 2023, 9:14 AM
martong added a comment to D142725: [DFAJumpThreading] Enable DFAJT with LTO.

@martong Would the optimization also work if -Xclang -enable-dfa-jump-thread were specified in CFLAGS instead? If "yes", then I'd say this is firmly in the realm of user error and we should not make this change.

Jan 27 2023, 9:08 AM · Restricted Project, Restricted Project
martong added a comment to D142725: [DFAJumpThreading] Enable DFAJT with LTO.

Thanks for the patch.

Question: why LTO? I'm no longer with the company where I worked on this and my recollections are starting to get fuzzy but AFAIR this optimization doesn't need the whole program. If so, you'll want to place this opt into the regular pipeline, where compilation of different translation units is parallelized.

Jan 27 2023, 8:25 AM · Restricted Project, Restricted Project
martong requested review of D142725: [DFAJumpThreading] Enable DFAJT with LTO.
Jan 27 2023, 7:43 AM · Restricted Project, Restricted Project

Oct 28 2022

martong added a comment to D136603: [analyzer] getBinding should auto-detect type only if it was not given.

Previously, even in the RegionStoreManager::getBinding() even if T was non-null, we replaced it with TVR->getValueType() in case the MR was TypedValueRegion.

Yeah, that means, we actually evaded a cast, am I right?

Oct 28 2022, 4:37 AM · Restricted Project, Restricted Project

Oct 26 2022

martong added a comment to D133698: [clang][dataflow] Implement transferBranch.

FWIW, it looks like this change broke the AIX bot: https://lab.llvm.org/buildbot/#/builders/214/builds/3932

Oct 26 2022, 10:23 PM · Restricted Project, Restricted Project
martong added inline comments to D133698: [clang][dataflow] Implement transferBranch.
Oct 26 2022, 8:30 AM · Restricted Project, Restricted Project
martong committed rG5bd142ca265d: [clang][dataflow] Remove unused 'Analysis' field from 'TerminatorVisitor' (authored by martong).
[clang][dataflow] Remove unused 'Analysis' field from 'TerminatorVisitor'
Oct 26 2022, 8:30 AM · Restricted Project, Restricted Project
martong added inline comments to D133698: [clang][dataflow] Implement transferBranch.
Oct 26 2022, 8:25 AM · Restricted Project, Restricted Project
martong committed rG82a50812f7e5: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg (authored by martong).
[analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg
Oct 26 2022, 7:35 AM · Restricted Project, Restricted Project
martong added a comment to D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.

Looks great to me, thanks!!

Oct 26 2022, 7:35 AM · Restricted Project, Restricted Project
martong closed D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.
Oct 26 2022, 7:34 AM · Restricted Project, Restricted Project
martong added a comment to D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker..

If we look only at the C standard we can not tell much about if the functions should set errno. It seems that setting errno is totally implementation-dependent, except for a few functions that mention errno. These are fgetpos, fsetpos, ftell and should set errno to a positive value on error (but there is no guarantee that value of errno is preserved if no error occurs). And errno is always positive. This is different than what the checker currently does (with the new patches). Probably this should be a discourse question?

Oct 26 2022, 6:55 AM · Restricted Project, Restricted Project
martong added a comment to D136684: [clang][ASTImporter] Remove use of ParentMapContext..

This is really a NFC-like change but not NFC because it has visible effects of removing some crashes. I could not produce a test that provokes the wrong case (when getParents returns an empty list). Is it enough to have no new test here? If we can get a test for the crash it needs more investigation to check if RecursiveASTVisitor works correct and probably a fix at other place.

Oct 26 2022, 6:48 AM · Restricted Project, Restricted Project
martong committed rGbb72d0dde29e: [clang][dataflow] Implement transferBranch (authored by martong).
[clang][dataflow] Implement transferBranch
Oct 26 2022, 6:25 AM · Restricted Project, Restricted Project
martong added inline comments to D133698: [clang][dataflow] Implement transferBranch.
Oct 26 2022, 6:25 AM · Restricted Project, Restricted Project
martong closed D133698: [clang][dataflow] Implement transferBranch.
Oct 26 2022, 6:25 AM · Restricted Project, Restricted Project
martong committed rG93ce23adb548: [clang][dataflow] Add initial sign analysis (authored by martong).
[clang][dataflow] Add initial sign analysis
Oct 26 2022, 6:20 AM · Restricted Project, Restricted Project
martong closed D136668: [clang][dataflow] Add initial sign analysis.
Oct 26 2022, 6:20 AM · Restricted Project, Restricted Project
martong updated the diff for D136668: [clang][dataflow] Add initial sign analysis.
  • Address ymandel's comments
Oct 26 2022, 6:08 AM · Restricted Project, Restricted Project
martong added a comment to D136668: [clang][dataflow] Add initial sign analysis.

Nice!! Just nits.

At a high level, I'm a little concerned about this as a demo, since I wouldn't recommend this implementation in practice (for various reasons, e.g. I would encode with 2 booleans since there are only 3 values). But, I think this approach has the advantage of clarity over optimized approaches. It might be worth pointing this out in comments at places where you made a decision for purposes of clarity/readability, if any come to mind.

Oct 26 2022, 6:08 AM · Restricted Project, Restricted Project
martong updated the diff for D133698: [clang][dataflow] Implement transferBranch.
  • Address ymandel's comments
Oct 26 2022, 2:12 AM · Restricted Project, Restricted Project
martong added a comment to D133698: [clang][dataflow] Implement transferBranch.

Thanks for the assiduous review @ymandel !

Oct 26 2022, 2:12 AM · Restricted Project, Restricted Project

Oct 25 2022

martong added inline comments to D133698: [clang][dataflow] Implement transferBranch.
Oct 25 2022, 8:48 AM · Restricted Project, Restricted Project
martong accepted D136684: [clang][ASTImporter] Remove use of ParentMapContext..

It is very good that we can get rid of the ParentMapContext because it was sub-optimal to build that up for all declaration contexts of the translation unit. Now we discover the parent-child relations for ourselves and only for the needed declaration contexts. This is the proper way to go.

Oct 25 2022, 8:37 AM · Restricted Project, Restricted Project
martong added a comment to D136603: [analyzer] getBinding should auto-detect type only if it was not given.

PS: I'm not sure how/when we should get rid of the LocAsInteger and
represent this by a SymbolCast.
Maybe @ASDenysPetrov or @martong could help me review this.

Oct 25 2022, 8:32 AM · Restricted Project, Restricted Project
martong added a comment to D136603: [analyzer] getBinding should auto-detect type only if it was not given.

I am not sure, if the ExprEngine::VisitCast is the proper place to add these new modifications and call SValBuilder's evalCast. I think it might be better positioned in RegionStoreManager::getBinding. Considering, we already do a cast evaluation for certain kind of memregions there before returning with the stored value. (Actually, this is again a legacy hack, which is needed because we do not emit all SymbolCasts and thus we store the SVals with an improper type).

if (const FieldRegion* FR = dyn_cast<FieldRegion>(R))
  return svalBuilder.evalCast(getBindingForField(B, FR), T, QualType{});
Oct 25 2022, 8:28 AM · Restricted Project, Restricted Project
martong accepted D135375: [analyzer] Initialize regions returned by CXXNew to undefined.

Seems like the issues mentioned above are real, but orthogonal to this patch. Would it be okay to address them in followup patches? @martong @NoQ

Oct 25 2022, 7:25 AM · Restricted Project, Restricted Project
martong updated the summary of D136668: [clang][dataflow] Add initial sign analysis.
Oct 25 2022, 2:05 AM · Restricted Project, Restricted Project
martong requested review of D136668: [clang][dataflow] Add initial sign analysis.
Oct 25 2022, 2:04 AM · Restricted Project, Restricted Project

Oct 21 2022

martong added a comment to D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.

Ping

Oct 21 2022, 9:16 AM · Restricted Project, Restricted Project
martong accepted D136162: [analyzer] Fix assertion failure with conflicting prototype calls.

Thanks for the update. Nice Work!

Oct 21 2022, 9:12 AM · Restricted Project, Restricted Project
martong added inline comments to D133698: [clang][dataflow] Implement transferBranch.
Oct 21 2022, 8:50 AM · Restricted Project, Restricted Project
martong updated the diff for D133698: [clang][dataflow] Implement transferBranch.
  • Rebase to latest llvm/main
Oct 21 2022, 8:47 AM · Restricted Project, Restricted Project
martong updated the summary of D133698: [clang][dataflow] Implement transferBranch.
Oct 21 2022, 8:32 AM · Restricted Project, Restricted Project
martong retitled D133698: [clang][dataflow] Implement transferBranch from [clang][dataflow] SignAnalysis, edgeTransfer, branchTransfer to [clang][dataflow] Implement transferBranch.
Oct 21 2022, 8:32 AM · Restricted Project, Restricted Project
martong updated the diff for D133698: [clang][dataflow] Implement transferBranch.
  • Add comments
  • Assumption -> ConditionValue
  • Use CRTP
  • branchTransfer -> transferBranch
  • Make just one simple test case for transferBranch
Oct 21 2022, 8:31 AM · Restricted Project, Restricted Project

Oct 19 2022

martong added a comment to D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker..

If StdCLibraryFunctionsChecker is added as dependency to StreamChecker and no other thing is changed these tests fail, and I could not find out the reason:
Errors in test stream.c:

error: 'warning' diagnostics expected but not seen: 
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 7: Stream pointer might be NULL
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 13: Stream pointer might be NULL
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream.c Line 76: Stream pointer might be NULL

Errors in test stream-error.c:

error: 'warning' diagnostics expected but not seen: 
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 97: might be 'indeterminate'
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 100: Stream might be already closed
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 113: Read function called when stream is in EOF state
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 114: Read function called when stream is in EOF state
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 123: FALSE
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 124: FALSE
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 128: FALSE
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 129: TRUE
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 133: TRUE
  File /local/clang/llvm2/llvm-project/clang/test/Analysis/stream-error.c Line 134: FALSE

Probably the StdLibraryFunctionsChecker runs before the StreamChecker, and runs always after it if no dependency is there? But this does not explain all test errors and should cause no errors at all. Adding the dependency itself is not enough, ModelPOSIX option should be true too. If the option is set to true in the test file even more test errors appear, and it does not work even when the (StdLibraryFunctionsChecker) checker is added (to the RUN command). Without the dependency the tests do not fail if the order of checkers in the enabled checkers list is changed.

Oct 19 2022, 1:56 AM · Restricted Project, Restricted Project

Oct 18 2022

martong added a comment to D136162: [analyzer] Fix assertion failure with conflicting prototype calls.

A similar situation could happen if we reinterpret cast pointers, etc. so the situation is not limited to conflicting function prototypes.

Oct 18 2022, 6:41 AM · Restricted Project, Restricted Project
martong added a comment to D136162: [analyzer] Fix assertion failure with conflicting prototype calls.

Hmm, seems like the conflicting prototype (i.e. the obsolescent use of zero parameters) is needed to reproduce the assertion failure. That makes me wonder, how does the redecl chain of b looks like? Is void b() chained with void b(int*), or are they represented independently from each other? I guess they form the same redecl chain. Which drives us to the next questions.
When the analyzer reaches the CallExpr b(&buffer) which FunctionDecl does it see? Is it b() or b(int*)? My bet, it sees and works with b(). Could we detect if the arguments of the CallExpr does not match the parameters of the FunctionDecl? And if that is the case, could we iterate through the redecl chain to find an appropriate matching FunctionDecl? That would be b(int*) in this case ... and the original bindArray() should work then.

Oct 18 2022, 6:40 AM · Restricted Project, Restricted Project
martong accepted D135136: [analyzer] Make directly bounded LazyCompoundVal as lazily copied.

LGTM

Oct 18 2022, 6:13 AM · Restricted Project, Restricted Project

Oct 17 2022

martong added inline comments to D135136: [analyzer] Make directly bounded LazyCompoundVal as lazily copied.
Oct 17 2022, 7:49 AM · Restricted Project, Restricted Project
martong added inline comments to D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker..
Oct 17 2022, 7:32 AM · Restricted Project, Restricted Project
martong added a comment to D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker..

I found some anomalies during development:

  • If the checker StdCLibraryFunctions is added as dependency for alpha.unix.Stream in checkers.td I get some "unexplainable" test failures.

Could you please elaborate? I don't see how to help you with it without seeing more details.

Oct 17 2022, 7:29 AM · Restricted Project, Restricted Project
martong added a comment to D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker..

Thanks for patch and the many test cases.

Oct 17 2022, 7:25 AM · Restricted Project, Restricted Project
martong added a comment to D135375: [analyzer] Initialize regions returned by CXXNew to undefined.

I am happy with the change and with the result. However, this colorSpace related report is a bit concerning. Could you please check if colorSpace at step 1 is evaluated as Undefined? Could that happen? Or that is (should be) Unknown since that is a parameter of a top-level analyzed function? Did you have this report before your change? I think this would worth to be further analyzed, maybe just directing the analyzer to analyze only the QImage::convertedToColorSpace top-level function.

Oct 17 2022, 7:05 AM · Restricted Project, Restricted Project
martong added a comment to D135375: [analyzer] Initialize regions returned by CXXNew to undefined.

Some early results:

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxnew_patched&diff-type=New

I'm still waiting on a few projects, but this is something. A few reports from alpha.security.ArrayBound, I still need to look into that.

This report from core.uninitialized.Assign is interesting. First off, I'd appreciate if the "storing uninitialized value" was placed inside the notes about the call to QScopedArrayPointer's constructor. Otherwise, the report looks correct.

Not too happy about this report by core.CallAndMessage. Message 9 is placed on the line

applyColorTransform(d->colorSpace.transformationToColorSpace(colorSpace));

Messag 10 talks about what happens in applyColorTransform, but the important thing happens at the evaluation of the argument, which is not described, so this isn't a very good bug report, can't tell whether its false.

Yes, I agree, this bug-report is hard to follow. Seems like the colorSpace argument from step 1 is flowing through step 10. I wonder if colorSpace at step 1 is evaluated as Undefined? Could that happen? Did you have this report before your change?

Oct 17 2022, 7:00 AM · Restricted Project, Restricted Project

Oct 15 2022

martong updated subscribers of D134902: [clang] Implement -fstrict-flex-arrays=3.
Oct 15 2022, 2:54 AM · Restricted Project, Restricted Project

Oct 14 2022

martong added a comment to D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.

Hi, looks great! I found a couple of typos and the amount of changes in tests is suspiciously low. And I want to make sure that the promise to change "arg" -> "argument" isn't lost (but I'll be happy if it's addressed in a follow-up patch).

Oct 14 2022, 7:25 AM · Restricted Project, Restricted Project
martong updated the diff for D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.
  • Changed to be more verbose: "arg" -> "argument"
  • Fixed "less than" to "greater than"
  • Added new tests
  • Fixed typos
Oct 14 2022, 7:22 AM · Restricted Project, Restricted Project
martong updated the diff for D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.
  • Rebase
Oct 14 2022, 7:12 AM · Restricted Project, Restricted Project

Oct 6 2022

martong added a comment to D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker..

The two checkers work together and 'apiModeling.StdCLibraryFunctions'
and its 'ModelPOSIX=true' option should be now a dependency of
checker 'alpha.unix.Stream'.

Oct 6 2022, 3:10 AM · Restricted Project, Restricted Project
martong added a comment to D126481: [analyzer] Handle SymbolCast in SValBuilder.

I don't see this case different to the unary expressions. Consider the unary minus for example. Let's say, the symbol a is constrained to [1, 1] and then -a is constrained to [-2, -2]. This is clearly an infeasible state and the analyzer will terminate the path. So, no further call of SValBuilder should happen from that point. That is, it is meaningless to evaluate -(-a) if there is a contradiction in the constraints that are assigned to the sub-symbols of the the symbol tree of -(-a).
Here is a test case that demonstrates this (this passes with latest llvm/main):

// RUN: %clang_analyze_cc1 %s \
// RUN:   -analyzer-checker=core \
// RUN:   -analyzer-checker=debug.ExprInspection \
// RUN:   -analyzer-config eagerly-assume=false \
// RUN:   -verify
Oct 6 2022, 3:00 AM · Restricted Project, Restricted Project

Oct 3 2022

martong accepted D134947: [analyzer] Fix the liveness of Symbols for values in regions referred by LazyCompoundVal.

Thanks for the updates. I am okay with it now. LGTM. But please wait for NoQ's approval. So, this is a gentle ping for you @NoQ :)

Oct 3 2022, 7:31 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
martong accepted D134941: [analyzer][NFC] Add tests for D132236.

Thanks for the update! LGTM.

Oct 3 2022, 5:42 AM · Restricted Project, Restricted Project
martong added a comment to D134947: [analyzer] Fix the liveness of Symbols for values in regions referred by LazyCompoundVal.

I like the approach of this patch and I think this is somewhat aligned with @NoQ's ideas about

Oct 3 2022, 5:34 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
martong added a comment to D134941: [analyzer][NFC] Add tests for D132236.

Thank you! Increasing coverage in tests is always great.

Oct 3 2022, 4:51 AM · Restricted Project, Restricted Project
martong accepted D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'..

I added a simple detection of create a copy of p before p = realloc(p, ...). This can remove the warning at very obvious cases when a copy of p is created (but even if the copy is made inside an if branch for example).

Oct 3 2022, 2:27 AM · Restricted Project, Restricted Project

Sep 29 2022

martong added a comment to D126481: [analyzer] Handle SymbolCast in SValBuilder.

Actually, you already have a logic for checking if there is a contradiction in your D103096 patch, in ConstraintAssignor::updateExistingConstraints:

// Update constraints in the map which bitwidth is equal or lower then
// `MinBitWidth`.
if (CM) {
  for (auto &Item : *CM) {
    // Stop after reaching a bigger bitwidth.
    if (Item.first > MinBitWidth)
      break;
    RangeSet RS = RangeFactory.intersect(Item.second, CastTo(Item.first));
    // If the intersection is empty, then the branch is infisible.
    if (RS.isEmpty()) {
      State = nullptr;
      return false;
    }
    NewCM = CMF.add(NewCM, Item.first, RS);
  }
}
Sep 29 2022, 2:35 AM · Restricted Project, Restricted Project
martong added a comment to D126481: [analyzer] Handle SymbolCast in SValBuilder.

Suppose we have found the way to handle it. But what if we find a contradiction while simplifying, like (short)(int x) = 0 and (int x) = 1. So that we would already know that it is impossible. What SVal should we return in such case? Undef? Unknown? Original one?

Sep 29 2022, 2:28 AM · Restricted Project, Restricted Project

Sep 28 2022

martong added inline comments to D132249: [clang][analyzer] Add some functions to StreamChecker with errno modeling..
Sep 28 2022, 12:46 AM · Restricted Project, Restricted Project
martong accepted D132017: [clang][analyzer] Add errno modeling to StreamChecker.

LGTM, thanks!

Sep 28 2022, 12:31 AM · Restricted Project, Restricted Project

Sep 27 2022

martong added a comment to D126481: [analyzer] Handle SymbolCast in SValBuilder.

Yeah okay. I get it now. Thank you for your patience and your time on elaborating the issue.

Sep 27 2022, 11:36 AM · Restricted Project, Restricted Project
martong added a comment to D126481: [analyzer] Handle SymbolCast in SValBuilder.

Assume we have (int)(short)(int x). VisitSymbolCast will try to get the constant recursively in the next order:

  • (short)(int x)
  • (int x)

And here is my concern. Whether it is correct to get the range for int x and consider it as a correct simplification of (int)(short)(int x). IMO it can't be simplified like that. It should go through the set of conventions and intersections.

Sep 27 2022, 2:17 AM · Restricted Project, Restricted Project

Sep 26 2022

martong accepted D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'..

We had a discussion about this with @dkrupp . We think that the p = realloc(p, var) construct in itself is an error-prone style of using realloc. This style does not necessarily give birth to erroneous code (see the error-free escaping example in foo above from @steakhal). However, the form p = realloc(p, var) is an indication that the programmer might missed the non-trivial error handling case, and chances are high. Thus, one should use the p2 = realloc(p1, var) form as a best practice. So, this looks good to me, but please consider this a weak approve and wait for someone else's approve who has more confidence in clang-tidy (@whisperity could you please take a look?).

Sep 26 2022, 7:41 AM · Restricted Project, Restricted Project
martong added inline comments to D134549: [clang] Add fix-it note to defaulted-function-deleted warning.
Sep 26 2022, 6:56 AM · Restricted Project, Restricted Project

Sep 22 2022

martong added inline comments to D134303: [AST] Preserve more structure in UsingEnumDecl node..
Sep 22 2022, 1:57 AM · Restricted Project, Restricted Project, Restricted Project

Sep 20 2022

martong planned changes to D133698: [clang][dataflow] Implement transferBranch.

Aligned with the RFC, I am going to dissect this patch into two patches:

Sep 20 2022, 4:46 AM · Restricted Project, Restricted Project

Sep 16 2022

martong accepted D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails.

LGTM, Thanks!

Sep 16 2022, 1:42 AM · Restricted Project, Restricted Project, Restricted Project
martong accepted D133944: [lldb][tests][gmodules] Test for expression evaluator crash for types that reference same template class.

Looks good to me!

Sep 16 2022, 12:40 AM · Restricted Project, Restricted Project

Sep 15 2022

martong added inline comments to D133931: [clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `clang/Analysis/FlowSensitive`..
Sep 15 2022, 7:24 AM · Restricted Project, Restricted Project
martong added a comment to D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.

Gentle ping @steakhal @NoQ
Trying to revive this after a year :) I am sorry it took so long to get back to this.

Sep 15 2022, 3:11 AM · Restricted Project, Restricted Project
martong updated the diff for D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints.
  • Rebase
  • move Msg into the lambda
Sep 15 2022, 3:09 AM · Restricted Project, Restricted Project
Herald added a project to D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints: Restricted Project.
Sep 15 2022, 3:09 AM · Restricted Project, Restricted Project

Sep 14 2022

martong accepted D133851: [analyzer] Initialize ShouldEmitErrorsOnInvalidConfigValue analyzer option.

Okay, I don't see any problem with this change. LGTM.

Sep 14 2022, 6:53 AM · Restricted Project, Restricted Project

Sep 12 2022

martong updated the summary of D133698: [clang][dataflow] Implement transferBranch.
Sep 12 2022, 8:14 AM · Restricted Project, Restricted Project
martong updated the summary of D133698: [clang][dataflow] Implement transferBranch.
Sep 12 2022, 7:19 AM · Restricted Project, Restricted Project
martong requested review of D133698: [clang][dataflow] Implement transferBranch.
Sep 12 2022, 7:15 AM · Restricted Project, Restricted Project

Sep 5 2022

martong accepted D133152: [analyzer][draft] Explain where references are coming from in the use after move check.

LGTM, with nits.

Sep 5 2022, 5:00 AM · Restricted Project
martong accepted D131262: [analyzer] Track trivial copy/move constructors and initializer lists in the BugReporter.

Thanks, you have nice tests with good coverage! LGTM!

Sep 5 2022, 4:39 AM · Restricted Project, Restricted Project

Sep 1 2022

martong accepted D132109: [analyzer] Dump the environment entry kind as well.

LGTM

Sep 1 2022, 4:28 AM · Restricted Project, Restricted Project
martong added inline comments to D131858: [clang] Track the templated entity in type substitution..
Sep 1 2022, 4:23 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project, Restricted Project
martong added inline comments to D132142: [analyzer] Prefer wrapping SymbolicRegions by ElementRegions.
Sep 1 2022, 4:16 AM · Restricted Project, Restricted Project

Aug 31 2022

martong accepted D132142: [analyzer] Prefer wrapping SymbolicRegions by ElementRegions.

I am okay with this change, it does give a proper canonical form, which is good. On the other hand, I agree that (on the long term) base regions and offsets would be a better memory model than what we have now with field and element regions.

Aug 31 2022, 9:35 AM · Restricted Project, Restricted Project
martong accepted D131879: [clang][analyzer] Errno modeling code refactor (NFC)..

LGTM!

Aug 31 2022, 8:30 AM · Restricted Project, Restricted Project

Aug 30 2022

martong added a comment to D131707: [analyzer][NFC] Cache the result of getLocationType in TypedValueRegion.

So I think the most valuable optimizations are low-level optimizations to ImmutableMap. There were a few suggestions on the mailing list to use something more modern than the AVL trees under the hood but I don't think authors found much success with those.

Aug 30 2022, 10:03 AM · Restricted Project, Restricted Project

Aug 9 2022

martong added a comment to D131299: [analyzer] Warn if the size of the array in `new[]` is undefined.

@martong I kept your idea in mind for this warning message and implemented it as a new checker if you don't mind it. If the size of an array is Undefined when it's allocated, this patch emits a warning, and ends the analysis.

Thank you for taking care of this!

Aug 9 2022, 6:24 AM · Restricted Project, Restricted Project

Aug 8 2022

martong accepted D130705: [clang][ASTImporter] Improve import of functions with auto return type..

Thank you for the update! LGTM!

Aug 8 2022, 6:54 AM · Restricted Project, Restricted Project

Aug 4 2022

martong added inline comments to D131006: [analyzer] Use DisequalityMap while inferring constraints.
Aug 4 2022, 11:45 AM · Restricted Project, Restricted Project
martong accepted D131067: [analyzer] Treat values passed as parameter as escaped.

LGTM

Aug 4 2022, 5:10 AM · Restricted Project, Restricted Project
martong added a comment to D130705: [clang][ASTImporter] Improve import of functions with auto return type..

Thank you, nice and assiduous work!

Aug 4 2022, 3:39 AM · Restricted Project, Restricted Project
martong added a comment to D131067: [analyzer] Treat values passed as parameter as escaped.

Thanks, looks good with some nits.

Aug 4 2022, 3:11 AM · Restricted Project, Restricted Project
martong added inline comments to D130737: [analyzer] Process non-POD array element destructors.
Aug 4 2022, 3:04 AM · Restricted Project, Restricted Project
martong added inline comments to D130737: [analyzer] Process non-POD array element destructors.
Aug 4 2022, 3:03 AM · Restricted Project, Restricted Project
martong accepted D130470: [clang][analyzer] Add more wide-character functions to CStringChecker.

I think there's only some very minor style nits which are remained, but that should not block this any further. Let's land it.

Aug 4 2022, 2:59 AM · Restricted Project, Restricted Project
martong added inline comments to D131006: [analyzer] Use DisequalityMap while inferring constraints.
Aug 4 2022, 2:53 AM · Restricted Project, Restricted Project