[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

vabridgers added a comment. @balazske , I tested these changes - again - and it all seems to work for me. I would have preferred we do not see the build status failures before doing this, but maybe we can be sure to avoid this next time (because I'll want to test again before our final merge).

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

vabridgers added a comment. Hi @balazske . Can we go back to the original proposed fix and treat the new issue separately? We have an internal crash open that is corrected by the original patch I proposed, and passes all LITs and unit tests. I think it would be better to separate these

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

vabridgers added a comment. Hi @balazske , all LIT and unittests pass with this change. By your logic, then we are missing some LIT or unittest cases that support your statement. Can you think of a case that demonstrates this? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

vabridgers added a comment. @balazske and I discussed, he will be commandeering this patch to improve. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145868/new/ https://reviews.llvm.org/D145868 ___

[PATCH] D145479: [clang][ASTImporter] Import typedefs to distinct records as distinct nodes.

vabridgers accepted this revision. vabridgers added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145479/new/ https://reviews.llvm.org/D145479

[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

vabridgers created this revision. vabridgers added reviewers: a.sidorin, shafik, donat.nagy, gamesh411, balazske. Herald added a subscriber: martong. Herald added a project: All. vabridgers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D144273: [clang][ASTImporter] Add VaList declaration to lookup table.

vabridgers accepted this revision. vabridgers added a comment. This revision is now accepted and ready to land. LGTM. Let's accept, merge and then watch to make sure we can keep the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144273/new/

[PATCH] D144622: [clang][ASTImporter] Import TemplateName correctly

vabridgers accepted this revision. vabridgers added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144622/new/ https://reviews.llvm.org/D144622

[PATCH] D140562: [clang][ASTImporter] Improve import of InjectedClassNameType.

vabridgers accepted this revision. vabridgers added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140562/new/ https://reviews.llvm.org/D140562 ___ cfe-commits mailing list

[PATCH] D140562: [clang][ASTImporter] Improve import of InjectedClassNameType.

vabridgers added a comment. Hi @donat.nagy , no problem. That's ok for me. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140562/new/ https://reviews.llvm.org/D140562 ___ cfe-commits mailing list

[PATCH] D140562: [clang][ASTImporter] Improve import of InjectedClassNameType.

vabridgers added a comment. Patch D144622 should be integrated into this one when a reduced reproducer has been prepared as a unittest and/or LIT case. I verified the patch stack D144273 , D140562

[PATCH] D144622: [clang[[ASTImporter] Import TemplateName correctly

vabridgers added a comment. This patch needs a unit test (as @balazske mentioned). So far, the case we have is too large to be a suitable unittest or lit case - so requires reduction. @balazske , will you be adding this as a unittest or lit case? Also, I think this patch needs to be integrated

[PATCH] D144273: [clang][ASTImporter] Add VaList declaration to lookup table.

vabridgers added a comment. I tested this change in the cases that previously failed for us, and this patch addresses our problems. It would be good to note in the commit header that this patch is intended to fix the problem first described by review D136886 .

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers planned changes to this revision. vabridgers added a comment. Changes are planned. Please do not waste any more time on this for now. This will probably be abandoned. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142822/new/

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers added a comment. @donat.nagy , regarding the namespace leaking, there was a change -> https://reviews.llvm.org/D116774 that modified the behavior for aarch64 and arm. While not incorrect, it may offer some historical view or examples of how to address the current cases. @whisperity

[PATCH] D138037: [analyzer] Remove unjustified assertion from EQClass::simplify

vabridgers accepted this revision. vabridgers added a comment. This revision is now accepted and ready to land. I got back to testing this through a large, internal set of builds, and do not see anymore problems. If everyone is ok with that, could this be merged? Repository: rG LLVM Github

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers updated this revision to Diff 493709. vabridgers added a comment. rebase, bump review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142822/new/ https://reviews.llvm.org/D142822 Files:

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers added subscribers: DavidSpickett, jrtc27. vabridgers added a comment. @DavidSpickett and/or @jrtc27 , I started with @mizvekov 's patch ( D136886 ) and attempted to address the problems with that patch on arm and aarch64. Is it possible for you to

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers updated this revision to Diff 493553. vabridgers added a subscriber: balazske. vabridgers added a comment. Updates per suggestion from @balazske Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142822/new/ https://reviews.llvm.org/D142822

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers created this revision. vabridgers added reviewers: aaron.ballman, mizvekov. Herald added subscribers: carlosgalvezp, martong, kristof.beyls. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a reviewer: njames93. Herald added a project: All. vabridgers

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers added a comment. @mizvekov, will you be picking this change up and finishing this, or do you mind if I have a go at finishing this patch? Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ https://reviews.llvm.org/D136886

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers added a comment. I've made a very simple reproducer for this crash, clang-tidy executes on an x86 machine. Requires aarch64 and/or arm support compiled in. crashes - clang-tidy crash.cpp -checks="cert-dcl58-cpp" -- -target arm crashes - clang-tidy crash.cpp -checks="cert-dcl58-cpp"

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers added a comment. @balazske , could you please share how to repro this problem on an x86 machine? Thank you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ https://reviews.llvm.org/D136886

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers added a comment. It appears to me this change https://reviews.llvm.org/D116774 is responsible for the unexpected behavior. Question for @jrtc27 : do you think if we could make this change consistent with https://reviews.llvm.org/D116774 that the problem would be addressed? Looks

[PATCH] D142627: [analyzer] Fix crash exposed by D140059

vabridgers created this revision. Herald added subscribers: manas, steakhal, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, hiraditya, xazax.hun. Herald added a project: All. vabridgers requested review of this revision. Herald added

[PATCH] D140059: [APSInt] Fix bug in APSInt mentioned in https://github.com/llvm/llvm-project/issues/59515

vabridgers added a comment. Thanks @Peter, I will try that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140059/new/ https://reviews.llvm.org/D140059 ___ cfe-commits mailing list

[PATCH] D140059: [APSInt] Fix bug in APSInt mentioned in https://github.com/llvm/llvm-project/issues/59515

vabridgers added a comment. Hello, it appears this patch causes a crash when I analyze this reproducer, with Z3 enabled. In the case shown here, the analyzer finds that 'f' to the call a(f) is uninitialized, and then is attempted to be refuted through SMTConv, leading to the assertion. If I

[PATCH] D138037: [analyzer] Remove unjustified assertion from EQClass::simplify

vabridgers added a comment. Hi @steakhal, thanks for the suggested change. How we can help move this forward? From what I'm comprehending from the notes, perhaps we could try running this change through our internal systems level test and fuzzer. Unfortunately, I'd not be able to say more than

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers added a comment. I think that addresses the last comments. Thanks again :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139759/new/ https://reviews.llvm.org/D139759 ___ cfe-commits mailing

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers updated this revision to Diff 481951. vabridgers marked 7 inline comments as done. vabridgers added a comment. clean up pass per comments from @steakhal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139759/new/

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers marked an inline comment as done. vabridgers added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:157-158 + Loc::isLocType(T)); return APSIntType(Ctx.getIntWidth(T),

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers updated this revision to Diff 481940. vabridgers added a comment. correct handling of sign type per @steakhal comments. Thank you, sir :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139759/new/ https://reviews.llvm.org/D139759 Files:

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers marked an inline comment as done. vabridgers added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:157-158 + Loc::isLocType(T)); return APSIntType(Ctx.getIntWidth(T),

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers marked 2 inline comments as done. vabridgers added a comment. Thanks Balazs, I think the comments have been addressed. Let me know if there's anything else to do, or if this is ready to land. Best! Comment at:

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers updated this revision to Diff 481882. vabridgers edited the summary of this revision. vabridgers added a comment. correct formatting of test case and expand test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139759/new/

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers added a comment. Thanks for the comments. I assumed git-clang-format cleaned up the cruft, but it didn't - that's disappointing. I'll try these things and update the review. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers added a comment. Some debug context ... (gdb) frame 4 #4 0x06f55b73 in clang::ento::BasicValueFactory::getAPSIntType (this=0x1088e470, T=...) at /clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h:155 155

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers updated this revision to Diff 481809. vabridgers added a comment. update commit header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139759/new/ https://reviews.llvm.org/D139759 Files:

[PATCH] D139759: [analyzer] Fix assertion in getAPSIntType

vabridgers created this revision. Herald added subscribers: steakhal, manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. vabridgers requested review of this revision.

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

vabridgers added a comment. @mizvekov, thanks for posting a fix. LGTM, but someone else must approve. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ https://reviews.llvm.org/D136886 ___

[PATCH] D136886: [clang] [ASTImporter] RFC: Correct importer to not duplicate sugared types

vabridgers added a subscriber: gamesh411. vabridgers added a comment. Adding more information, seems this patch's "hack" returns the following QualType(T, 0).getDesugaredType(T->getDecl()->getASTContext()); -> ConstantArrayType 0x11413090 'struct __va_list_tag[1]' imported 1 `-RecordType

[PATCH] D136886: [clang] [ASTImporter] RFC: Correct importer to not duplicate sugared types

vabridgers added a comment. In D136886#3892261 , @balazske wrote: > `ASTImporterLookupTable` do not contain an entry for `__va_list_tag`, I do > not know why it is missing. If it is added "manually" the crash disappears > (without fix in

[PATCH] D136886: [clang] [ASTImporter] RFC: Correct importer to not duplicate sugared types

vabridgers updated this revision to Diff 471328. vabridgers added a comment. remove commented code from test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ https://reviews.llvm.org/D136886 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D136886: [ASTImporter] RFC: Correct importer to not duplicate sugared types

vabridgers updated this revision to Diff 471303. vabridgers added a comment. remove commented line Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ https://reviews.llvm.org/D136886 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D136886: [ASTImporter] RFC: Correct importer to not duplicate sugared types

vabridgers created this revision. vabridgers added a reviewer: mizvekov. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: All. vabridgers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I'm not

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

vabridgers updated this revision to Diff 443658. vabridgers edited the summary of this revision. vabridgers added a comment. update per comments from @martong Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129269/new/

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

vabridgers added a comment. Thanks @martong , I understand. I'll make the changes and resubmit. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129269/new/ https://reviews.llvm.org/D129269 ___

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

vabridgers updated this revision to Diff 443101. vabridgers added a comment. a proposal to handle embedded null case caught by @steakhal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129269/new/ https://reviews.llvm.org/D129269 Files:

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

vabridgers added a comment. Thanks Balazs, you mean something like this correct? void strcpy_no_overflow_2(char *y) { char x[3]; strcpy(x, "12\0"); // this produces a warning, but should not. } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129269: [analyzer] Fix use of length in CStringChecker

vabridgers created this revision. vabridgers added reviewers: martong, steakhal. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All.

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

vabridgers marked 8 inline comments as done. vabridgers added a comment. I think all comments have been addressed, please let me know if I missed some detail :) Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127105/new/

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

vabridgers updated this revision to Diff 434821. vabridgers added a comment. address @steakhal comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127105/new/ https://reviews.llvm.org/D127105 Files:

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

vabridgers added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:168 -CastToTy->getAsCXXRecordDecl()->getNameAsString() : -CastToTy->getPointeeCXXRecordDecl()->getNameAsString(); Out << ' ' <<

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

vabridgers updated this revision to Diff 434746. vabridgers added a comment. Use QualType::getAsString() per suggestion from martong Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127105/new/ https://reviews.llvm.org/D127105 Files:

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

vabridgers added a comment. I was able to find and add a covering test case. I understand the fix may not be "correct", but it does avoid the crash demonstrated by the test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127105/new/

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

vabridgers updated this revision to Diff 434628. vabridgers added a comment. add test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127105/new/ https://reviews.llvm.org/D127105 Files: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

vabridgers planned changes to this revision. vabridgers added a comment. I know this will need a reproducer, and I'm working on that. That work is still in progress. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127105/new/

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

vabridgers created this revision. vabridgers added reviewers: martong, steakhal, NoQ. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. vabridgers requested

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers marked an inline comment as done. vabridgers added a comment. Thanks @martong. I fixed the "typo" :/, landing. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124349/new/ https://reviews.llvm.org/D124349

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers updated this revision to Diff 427246. vabridgers added a comment. fix a stupid leftover cut/paste mistake in the test case :/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124349/new/ https://reviews.llvm.org/D124349 Files:

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers updated this revision to Diff 427106. vabridgers added a comment. Address comments from @martong Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124349/new/ https://reviews.llvm.org/D124349 Files:

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers added a comment. Thanks for the comments! Per our discussion, I'll create a different test case with a pinned target and add the additional test case to be sure we see expected behavior. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers added a comment. polite ping to @NoQ and/or @martong. @steakhal is ok with this change, are you ok if I land this? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124349/new/ https://reviews.llvm.org/D124349

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers updated this revision to Diff 424844. vabridgers added a comment. update LIT per comments from @steakhal Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124349/new/ https://reviews.llvm.org/D124349 Files:

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers added inline comments. Comment at: clang/test/Analysis/array-struct-region.c:362-366 +int array_struct_bitfield_1() { + BITFIELD_CAST ff = {0}; + BITFIELD_CAST *pff = + return *((int *)pff + 1); +} steakhal wrote: > ```lang=c > void

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers updated this revision to Diff 424796. vabridgers added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124349/new/ https://reviews.llvm.org/D124349 Files: clang/lib/StaticAnalyzer/Core/RegionStore.cpp

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

vabridgers created this revision. vabridgers added reviewers: NoQ, steakhal. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. vabridgers requested review of this

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers updated this revision to Diff 424752. vabridgers added a comment. add back amdgcn cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122841/new/ https://reviews.llvm.org/D122841 Files: clang/docs/analyzer/checkers.rst

[PATCH] D123464: [analyzer] Clean checker options from bool to DefaultBool (NFC)

vabridgers added a comment. Hi @steakhal, I believe I've addressed the comments. Please have a look and let me know if this is what you were after. Best! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123464/new/ https://reviews.llvm.org/D123464

[PATCH] D123464: [analyzer] Clean checker options from bool to DefaultBool (NFC)

vabridgers updated this revision to Diff 424720. vabridgers added a comment. refactor patch to remove DefaultBool, replace with bool as suggested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123464/new/ https://reviews.llvm.org/D123464 Files:

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers marked an inline comment as done. vabridgers added a comment. I believe all comments have been addressed. After discussion with @steakhal, we're leaning towards removing the DefaultBool class and refactoring in favor of the language native bool type - mainly because DefaultBool

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers updated this revision to Diff 424711. vabridgers added a comment. Update test cases, address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122841/new/ https://reviews.llvm.org/D122841 Files: clang/docs/analyzer/checkers.rst

[PATCH] D123464: [analyzer] Clean checker options from bool to DefaultBool (NFC)

vabridgers created this revision. vabridgers added reviewers: NoQ, steakhal. Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. vabridgers requested review of this

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers marked 3 inline comments as done. vabridgers added a comment. The two small issues are addressed, I think the only outstanding issue is the question about the test case. Please let me know a preference and I'll implement accordingly. Best! Repository: rG LLVM Github Monorepo

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers updated this revision to Diff 421759. vabridgers added a comment. This revision is now accepted and ready to land. update per @steakhal's latest comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122841/new/

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers planned changes to this revision. vabridgers added a comment. Thanks for the the comments. I'll make the changes and resubmit. Please let me know your preferences for the test case (default vs pinned x86 or same). Best! Comment at:

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers marked 3 inline comments as done. vabridgers added a comment. I believe all points have been addressed. Thanks for the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122841/new/ https://reviews.llvm.org/D122841

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers updated this revision to Diff 421706. vabridgers added a comment. update per @steakhal's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122841/new/ https://reviews.llvm.org/D122841 Files: clang/docs/analyzer/checkers.rst

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers added a comment. I'll address, thanks for the comments. Comment at: clang/docs/analyzer/checkers.rst:69-74 +Check for dereferences of null pointers. This checker specifically does +not report null pointer dereferences for x86 and x86-64 targets when the +address

[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

vabridgers updated this revision to Diff 421521. vabridgers edited the summary of this revision. vabridgers added a comment. clean up a few debug edits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122841/new/ https://reviews.llvm.org/D122841

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

vabridgers updated this revision to Diff 421519. vabridgers added a comment. add option, retain legacy behavior per comments. update rst Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122841/new/ https://reviews.llvm.org/D122841 Files:

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

vabridgers added a comment. Ahhh, I see - thanks for the references and discussion. I'll craft a patch to comprehend the use case and user-facing option, and post for review. We'll see how it goes :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

vabridgers added a comment. @steakhal, good question. Digging into the change archaeology, I see this change was made by Anna Zaks in Jan 2016 - see commit header below. There's no reference to a review with comments, and I don't know how to find those if they exist, perhaps @NoQ could help

[PATCH] D122841: [analyzer] Consider all addrspaces in null dereference check

vabridgers created this revision. vabridgers added reviewers: NoQ, steakhal, martong. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. vabridgers requested

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

vabridgers updated this revision to Diff 419462. vabridgers added a comment. This revision is now accepted and ready to land. make assert function void, update summary Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

vabridgers planned changes to this revision. vabridgers added a comment. I need to make a few changes. Please hold off any comments until the next patch, coming soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

vabridgers added a comment. I added #ifndef NDEBUG around the assert since I'm not sure if downstream consumers that use different pointer sizes by address space will hit this assert. Our downstream target causes this assert to fire, so I will be continuing to find and fix these issues,

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

vabridgers updated this revision to Diff 419438. vabridgers added a comment. add NDEBUG around assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/ https://reviews.llvm.org/D118050 Files:

[PATCH] D118050: [analyzer] Different address spaces cannot overlap

vabridgers updated this revision to Diff 419013. vabridgers added a comment. Herald added a project: All. rebase, check ci Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/ https://reviews.llvm.org/D118050 Files:

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

vabridgers marked an inline comment as done. vabridgers added inline comments. Comment at: clang/test/Analysis/addrspace-null.c:19 + +#if defined(AMDGCN) +// this crashes NoQ wrote: > Shouldn't this be `AMDGCN_TRIPLE`? fixed, thank you. Repository: rG LLVM

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

vabridgers updated this revision to Diff 418996. vabridgers added a comment. fix typo in test - ready to land Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ https://reviews.llvm.org/D122513 Files:

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

vabridgers updated this revision to Diff 418749. vabridgers added a comment. Come up with a more principaled fix, thanks @NoQ :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ https://reviews.llvm.org/D122513 Files:

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

vabridgers added a comment. I think I got it, looks like we're losing the width information at line 685, 686 below. Looks like I need to adjust the bitwidth of V before returning. clang/lib/StaticAnalyzer/Core/SValBuilder.cpp 671 SVal SValBuilder::evalCastSubKind(loc::ConcreteInt V,

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

vabridgers added a comment. Ahhh, gotcha. I'll run that down and report back. Thanks ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ https://reviews.llvm.org/D122513 ___ cfe-commits mailing

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

vabridgers added a comment. Apparently, it's up to an implementation to determine the specific relations that can be computed between different address spaces. Perhaps a better way to deal with this for now, to avoid crashes, is follow the DereferenceChecker model. That checker punts on

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

vabridgers added a comment. Hi @NoQ, good question :) When I looked into the existing SimpleSValBuilder.cpp code, I found a few places that did width modifications so I concluded this was the correct and accepted way to handle this. A few notes to help decide if my suggestion is correct or is

[PATCH] D122513: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same"

vabridgers created this revision. vabridgers added reviewers: NoQ, steakhal, martong. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. vabridgers requested

[PATCH] D122277: [analyzer] Fix crash in RangedConstraintManager.cpp

vabridgers updated this revision to Diff 417525. vabridgers added a comment. update comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122277/new/ https://reviews.llvm.org/D122277 Files:

[PATCH] D122277: [analyzer] Fix crash in RangedConstraintManager.cpp

vabridgers updated this revision to Diff 417450. vabridgers added a comment. remove assert that was commented out Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122277/new/ https://reviews.llvm.org/D122277 Files:

[PATCH] D122277: [analyzer] Fix crash in RangedConstraintManager.cpp

vabridgers created this revision. vabridgers added reviewers: martong, NoQ. Herald added subscribers: manas, steakhal, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. vabridgers requested

  1   2   3   >