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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfe8b2236ef9c: [analyzer] Fix "RhsLoc and LhsLoc bitwidth must be same" (authored by vabridgers, committed by einvbri

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

[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: clang/lib/StaticAnalyzer/Core/SValBui

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Other than that, looks great now! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ https://reviews.llvm.org/D122513

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

NoQ added inline comments. Comment at: clang/test/Analysis/addrspace-null.c:19 + +#if defined(AMDGCN) +// this crashes Shouldn't this be `AMDGCN_TRIPLE`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ h

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

[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, QualType

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

NoQ added a comment. `getSVal` is probably not at fault, it simply retrieves the value it was previously told to put there, it doesn't care what the value is. You probably want to look at `ExprEngine::VisitCast()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

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

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

NoQ added a comment. I mean, regardless of whether we need to report a warning or an error against this code, the AST says that the RHS type and the LHS type are the same. This means that the same should have applied to our SVals that represent LHS and RHS, rendering `std::max(APSIntType(LHSVal

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

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

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

NoQ added a comment. Why doesn't the AST handle this for us through implicit casts? Do we really need to duplicate type promotion logic in the static analyzer? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122513/new/ https://reviews.llvm.org/D122

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