[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 clang/unittests/

[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 c

[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 `VisitTypedefTy

[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 0x

[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] 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 mo

[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 cfe-commits@lists.llvm.

[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 projects

[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 like

[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. 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. @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] 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 req

[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 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 try

[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: clang-tools-extra/clang-tidy/altera/StructPackAli

[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 review

[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/ https://reviews.llvm.or

[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 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/ https://re

[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: clang/lib/Stat

[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 << ' ' << ((CastToTyVec.si

[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: clang/lib/StaticAnalyzer/Checkers/CastValue

[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/ https://reviews.llvm.

[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

vabridgers created this revision. vabridgers added reviewers: martong, NoQ. Herald added subscribers: ASDenysPetrov, Charusso, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun. Herald added a project: clang. This check was causing a crash in a test case where the 0th

[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

vabridgers updated this revision to Diff 253419. vabridgers added a comment. fix pre-merge lint checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77012/new/ https://reviews.llvm.org/D77012 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFu

[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

vabridgers updated this revision to Diff 253434. vabridgers added a comment. fix pre-merge lint checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77012/new/ https://reviews.llvm.org/D77012 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFu

[PATCH] D77012: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check

This revision was automatically updated to reflect the committed changes. Closed by commit rGdefd95ef4517: [analyzer] Fix StdLibraryFunctionsChecker NotNull Constraint Check (authored by vabridgers, committed by einvbri ). Repository: rG LLVM Github Monorepo CHA

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

vabridgers created this revision. vabridgers added reviewers: martong, Szelethus, NoQ. Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, kristof.beyls, xazax.hun. Herald added a project: clang. This change is a follow up to commit 5

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

vabridgers updated this revision to Diff 247936. vabridgers added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75529/new/ https://reviews.llvm.org/D75529 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunc

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

vabridgers marked 2 inline comments as done. vabridgers added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:514 + // Set UCharMax to min of int or uchar maximum value. + // The C standard states that functions like isalpha must be

[PATCH] D70836: [analysis] Fix value tracking for pointers to qualified types

vabridgers created this revision. vabridgers added reviewers: dcoughlin, dergachev.a. vabridgers added a project: clang. Herald added a subscriber: cfe-commits. This change fixes part 1 described by Artem in the Bugzilla report 43364. The comparison done was on a canonical, but should have been d

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

vabridgers added a comment. Herald added a subscriber: danielkiss. @Charusso, I cannot find a way to create a test for this change, since this was found using an architecture that supports 16-bit chars. I understand the need to create test cases for changes. In this case, I believe a rationale f

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

vabridgers updated this revision to Diff 249680. vabridgers added a comment. Address Artem's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75529/new/ https://reviews.llvm.org/D75529 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFu

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

vabridgers updated this revision to Diff 250109. vabridgers marked 2 inline comments as not done. vabridgers added a comment. fix pre merge checkswq Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75529/new/ https://reviews.llvm.org/D75529 Files:

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

vabridgers marked 6 inline comments as done. vabridgers added a comment. I believe all comments have been addressed. Please let me know if there's anything else required. Thanks Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:519 + // architecture

[PATCH] D75529: [analyzer] Limit UCharMax to min of max uchar or max int

vabridgers marked an inline comment as done. vabridgers added a comment. @Charusso -- I do not have commit access, but I will request. Thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75529/new/ https://reviews.llvm.org/D75529 __

[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 concerns

[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] 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] 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] 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] 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 cf

[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 cfe-commits@lis

[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] 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] 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. Fi

[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 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 ___ cfe-c

[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 A

[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 Mo

[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] 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/ https://rev

[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. Her

[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: clang/include/clang/StaticAnalyzer/Core/PathSensi

[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 assert(T->isIntegralOrEnumera

[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 https://reviews.llvm.org/D139759/new

[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/ https://rev

[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: clang/include/clang/StaticAnalyzer/Core/PathSensitive/

[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), !T->isSi

[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), !T->isSi

[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/ https://reviews.llvm.org/D

[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 li

[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] D118050: [analyzer] Different address spaces cannot overlap

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

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

vabridgers added a comment. Good thoughts, I will try those things and get back to you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/ https://reviews.llvm.org/D118050 ___ cfe-commits mailing

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

vabridgers updated this revision to Diff 403276. vabridgers added a comment. - update test case - add initial Loc bitwidth check for evalBinOpLL for review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/ https://reviews.llvm.org/D118050

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

vabridgers added a comment. I updated the test case after verifying I can reproduce the crash without the fix. I also experimented with adding a bitwidth check for the lhs and rhs loc parameters to evalBinOpLL, and verified this assert catches the negative (crash) case. I explored adding a 'me

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

vabridgers marked an inline comment as done. vabridgers added inline comments. Comment at: clang/test/Analysis/cstring-checker-addressspace.c:14 +// Copy from host to device memory. +DEVICE void *memcpy(DEVICE void *dst, const void *src, unsigned long len); + ste

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

vabridgers marked an inline comment as done. vabridgers added a comment. Thanks for the comments, I'll address. Best Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/ https://reviews.llvm.org/D118050 ___

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

vabridgers added a comment. I took a run at the improvements, and the rabbit hole keeps getting deeper :/ @steakhal, I'll contact you offline to work out the problems. Best Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118050/new/ https://review

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

vabridgers updated this revision to Diff 404940. vabridgers added a comment. Simplify assertion per comments. Add a "temp" fix in SValBuilder that permits the additional test cases to pass without crash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

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

vabridgers added a comment. I think @steakhal will have a more comprehensive change to back out the makeNull() calls to makeNullWithWidth() calls. I'll back out the change to SValBuilder.cpp once that change is pushed. For now, this is my update. Thanks Repository: rG LLVM Github Monorepo C

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

vabridgers marked 7 inline comments as done. vabridgers added a comment. I think the comments have been addressed for now. Please let me know if I missed something, or anything else needs to be done (besides back out the change to SValBuilder.cpp when ready). Thanks for the comments. Best Repo

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth

vabridgers marked 5 inline comments as done. vabridgers added a comment. I believe I've addressed all of Artem's comments thus far. This is a NFC patch, so will include no test cases. We detected these inconsistencies in getting NULL pointers in our downstream target, that supports 2 different p

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

vabridgers updated this revision to Diff 409896. vabridgers added a comment. refactor based on @steakhal comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119601/new/ https://reviews.llvm.org/D119601 Files: clang/include/clang/StaticAnalyze

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

vabridgers marked an inline comment as done. vabridgers added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:372 - Loc makeNull() { -return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth()); } steakhal wr

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

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

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

vabridgers updated this revision to Diff 410493. vabridgers added a comment. Update per @steakhal comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120134/new/ https://reviews.llvm.org/D120134 Files: clang/include/clang/StaticAnalyzer/Core/

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

vabridgers marked 2 inline comments as done. vabridgers added a comment. I believe the comments have been addressed. Thank you - Vince Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120134/new/ https://reviews.llvm.org/D120134 _

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

vabridgers added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:726-727 + LhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(LhsLoc.getType(Ctx)); + if (RhsBitwidth && LhsBitwidth && + (LhsLoc.getSubKind() == RhsLoc.getSubKind())) { +

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

vabridgers marked an inline comment as done. vabridgers added a comment. Herald added a project: All. @NoQ - ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119601/new/ https://reviews.llvm.org/D119601 _

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

vabridgers added a comment. Herald added a subscriber: manas. Herald added a project: All. @NoQ - ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120134/new/ https://reviews.llvm.org/D120134 ___ cfe-

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

vabridgers added a comment. Is it ok to land this change? Or shall we continue to wait on @NoQ ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120134/new/ https://reviews.llvm.org/D120134 ___ cfe-commits

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

vabridgers added a comment. Is it ok to land this change? Or shall we continue to wait on @NoQ ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119601/new/ https://reviews.llvm.org/D119601 ___ cfe-commits

[PATCH] D101635: [analyzer] Fix assertion in SVals.h

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

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

vabridgers added a comment. Ping @rsmith . Are you ok if I rebase and push this change? Or do you have specific items that require attention? I've spoken to @ilya , he's ok if I clean this up and push with credit to him. Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

vabridgers updated this revision to Diff 317252. vabridgers added a comment. Rebase, commandeer this patch from Ilya. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71714/new/ https://reviews.llvm.org/D71714 Files: clang/include/clang/Sema/Sema.h

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

vabridgers added a comment. Ping. Could someone pick up review of this patch again, please? Ilya left off with a question to @rsmith , and at that point all activity fell off. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71714/new/ https:

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

vabridgers planned changes to this revision. vabridgers added a comment. I'm refactoring the code change a bit. I'll push another update soon. Thanks for the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110625/new/ https://reviews.llvm.

[PATCH] D110625: [analyzer] canonicalize special case of structure/pointer deref

vabridgers updated this revision to Diff 377309. vabridgers added a comment. This revision is now accepted and ready to land. Refactor compare a little bit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110625/new/ https://reviews.llvm.org/D110625

<    1   2   3