[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

This revision was automatically updated to reflect the committed changes. Closed by commit rG5882e6f36fd9: [analyzer] Escape symbols conjured into specific regions during a conservative… (authored by xazax.hun). Changed prior to commit: https://reviews.llvm.org/D71224?vs=233235=233431#toc

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks!! Nothing controversial remains here, right? :) Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:91 + /// that the analyzer cannot follow during a

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun updated this revision to Diff 233235. xazax.hun added a comment. - Rebasing after reverting the pre-escaping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ added a comment. In D71224#1778332 , @xazax.hun wrote: > So basically what I am wonder/worrying about is the following: > The analyzer core will decide that the stack region is escaped and the > checkers has no word about this. Yup, you got me.

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun updated this revision to Diff 233200. xazax.hun added a comment. - First take on trying to reuse `processPointerEscapedOnBind`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files:

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun added a comment. I think I found the main problem with the current model, at least for the FuchsiaHandleCheck. Consider the following two snippets: zx_handle_t *get_handle_address(); void escape_store_to_escaped_region01() { zx_handle_t sb; if (zx_channel_create(0,

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun added a comment. In D71224#1778303 , @NoQ wrote: > In D71224#1778284 , @xazax.hun wrote: > > > So I was wondering if we got the default right. Maybe a checker should do > > more work to get the escaping

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ added a comment. In D71224#1778284 , @xazax.hun wrote: > So I was wondering if we got the default right. Maybe a checker should do > more work to get the escaping rather than more work preventing it? But that's exactly how it works right now(?) If

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun added a comment. In D71224#1778231 , @NoQ wrote: > In D71224#1778204 , @xazax.hun wrote: > > > I don't think this is a good enough model currently. The problem is that, > > it does not play well with

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ added a comment. In D71224#1778204 , @xazax.hun wrote: > I don't think this is a good enough model currently. The problem is that, it > does not play well with annotations. E.g. the checker can see a symbol > escaping, but it does not have a whole

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun added a comment. In D71224#1778179 , @NoQ wrote: > In any case, every checker is allowed to make their own decisions about > escaping. Escape on its own is not material, it's all about how the checker > reacts to escapes. Say, it's up to

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ added a comment. In any case, every checker is allowed to make their own decisions about escaping. Escape on its own is not material, it's all about how the checker reacts to escapes. Say, it's up to MallocChecker to decide whether the function may or may not release memory that escapes on

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624 +if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion()) + if (!MR->hasStackStorage()) +

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624 +if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion()) + if (!MR->hasStackStorage()) +

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624 +if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion()) + if (!MR->hasStackStorage()) +

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:632 + /// of some values. + ProgramStateRef escapeValue(ProgramStateRef State, ArrayRef Vs, PointerEscapeKind K) const;

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun updated this revision to Diff 232982. xazax.hun added a comment. - Reuse `ExprEngine::escapeValue`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State;

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State; xazax.hun wrote: > NoQ wrote: > > xazax.hun

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:648 + continue; +State->scanReachableSymbols(Call.getArgSVal(Arg), Scanner); + } NoQ wrote: >

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State;

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State; xazax.hun wrote: > NoQ wrote: > > WDYT about

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun marked an inline comment as done. xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State;

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603 + // point. + class CollectReachableSymbolsCallback final : public SymbolVisitor { +ProgramStateRef State; WDYT about re-using `ExprEngine::escapeValue()`

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun updated this revision to Diff 232968. xazax.hun edited the summary of this revision. xazax.hun added a comment. - Added a test. More rigorous tests will come in the FuchsiaHandleChecker. - Added a new PSK_ entry. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

xazax.hun updated this revision to Diff 232955. xazax.hun added a comment. - Do not track explicitly if a call was conservative. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71224/new/ https://reviews.llvm.org/D71224 Files: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp