<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Issue</th>
<td>
<a href=https://github.com/llvm/llvm-project/issues/61985>61985</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>
[clang static analyzer] bug repair in conversion between unsigned/signed SymbolVal
</td>
</tr>
<tr>
<th>Labels</th>
<td>
</td>
</tr>
<tr>
<th>Assignees</th>
<td>
</td>
</tr>
<tr>
<th>Reporter</th>
<td>
tianxinghe
</td>
</tr>
</table>
<pre>
version: clang/llvm 0e37487df8e0 April 5, 2023
args: `clang --analyze --analyzer-no-default-checks -Xanalyzer -analyzer-checker=core.NullDereference -Xanalyzer -analyzer-output=text 1.c`
Hi! I try to identify the cause of the bug in #61515
My test case is:
`#include <stdint.h>
#include <stdio.h>
void init(uint32_t, uint8_t**);
uint8_t* malloc(uint64_t);
uint64_t atol(uint8_t*);
int main(int, char **);
uint8_t* memset(uint8_t*, uint32_t, uint64_t);
uint64_t* args;
void init(uint32_t argc, uint8_t** argv) {
args = (uint64_t*) malloc(8 * sizeof(uint64_t));
for (int32_t i = 1; i < argc; ++i) {
args[i - 1] = atol(argv[i]);
}
}
int main(int argc, char ** argv) {
uint32_t llvm_cbe_argc = (uint32_t)argc;
uint8_t** llvm_cbe_argv = (uint8_t**)argv;
uint8_t* llvm_cbe_llvm_cbe_a1; /* Address-exposed local */
uint8_t* _1;
uint64_t _2;
uint64_t _3;
_1 = ((uint8_t*)(&llvm_cbe_llvm_cbe_a1));
llvm_cbe_llvm_cbe_a1 = ((uint8_t*)/*NULL*/0);
init(llvm_cbe_argc, llvm_cbe_argv);
_2 = *args;
_3 = *args;
if ((int64_t)_3 < (int64_t)UINT64_C(3369728583)) {
if((int64_t)_2 >= (int64_t)UINT64_C(-1)){
**(&llvm_cbe_llvm_cbe_a1);
}
}
return 0;
}
`
The function `VisitNonLocLocAsInteger` in `clang/lib/StaticAnalyzer/SValBuilder.cpp` fail to convert the symbolval from unsigned to signed when the engine visits the cast `(int64_t)_2` in IfStmt
and evaluate the binop '(int64_t)_2 >= (int64_t)UINT64_C(-1)' later as unsigned
ShouldSupportSymbolicIntegerCasts is false in `clang/lib/StaticAnalyzer/SValBuilder.cpp` function `VisitNonLocSymbolVal`
**The engine thinks the symbolval should not be converted to signed when visit the cast (int64_t)_2 but converts -1 to unsigned when visit the binaryoperator (int64_t)_2 >= (int64_t)UINT64_C(-1)**
**So signed binop is calculated unsigned!**
in `clang/lib/StaticAnalyzer`
1. visitCast `(int64_t)_2`
- `visitCast` in `clang/lib/StaticAnalyzer/ExprEngineC.cpp`
- `switch (CastE->getCastKind())`
- `case CK_IntegralCast` `V = svalBuilder.evalIntegralCast(state, V, T, ExTy);`
- `evalIntegralCast` in `clang/lib/StaticAnalyzer/SValBuilder.cpp`
- `if (getContext().getTypeSize(castTy) >= getContext().getTypeSize(originalTy))
return evalCast(val, castTy, originalTy);`
- `evalCast` in `clang/lib/StaticAnalyzer/SValBuilder.cpp`
- `return TRV.Visit(V);`
- `Visit` in `clang/lib/StaticAnalyzer/SValBuilder.cpp`
- `evalCast` in `clang/lib/StaticAnalyzer/SValBuilder.cpp`
- `VisitNonLocSymbolVal` in `clang/lib/StaticAnalyzer/SValBuilder.cpp`
2. visitBinaryOperator `(int64_t)_2 >= (int64_t)UINT64_C(-1)`
- `visitBinaryOperator` in `clang/lib/StaticAnalyzer/ExprEngineC.cpp`
- `SVal Result = evalBinOp(state, Op, LeftV, RightV, B->getType());`
- `evalBinOp` in `ExprEngine.h`
- `evalBinOp` in `clang/lib/StaticAnalyzer/SValBuilder.cpp`
- `return evalBinOpNN(state, op, lhs.castAs<NonLoc>(), rhs.castAs<NonLoc>(),
type);`
- `evalBinOpNN` in `clang/lib/StaticAnalyzer/SimpleSValBuilder.cpp`
- `switch (lhs.getSubKind())`
- `case nonloc::SymbolValKind`
- `return MakeSymIntVal(symIntExpr, op, *RHSValue, resultTy);`
- `MakeSymIntVal` in `clang/lib/StaticAnalyzer/SimpleSValBuilder.cpp`
- ` if (RHS.isSigned() && !SymbolType->isSignedIntegerOrEnumerationType())
ConvertedRHS = &BasicVals.Convert(SymbolType, RHS);`
I try to fix it by add code as follows in `clang/lib/StaticAnalyzer/SValBuilder.cpp` function `VisitNonLocSymbolVal` :
` if (T->isIntegralOrUnscopedEnumerationType() &&
CastTy->isIntegralOrUnscopedEnumerationType()) {
AnalyzerOptions &Opts = VB.getStateManager()
.getOwningEngine()
.getAnalysisManager()
.getAnalyzerOptions();
// If appropriate option is disabled, ignore the cast.
// NOTE: ShouldSupportSymbolicIntegerCasts is `false` by default.
+ if(T->isIntegerType() && CastTy->isIntegerType()){
+ if(CastTy->isSignedIntegerType() != T->isSignedIntegerType()){
+ return simplifySymbolCast(V, CastTy);
+ }
+ }
if (!Opts.ShouldSupportSymbolicIntegerCasts)
return V;
return simplifySymbolCast(V, CastTy);
}`
This is a more complex test case. Through fix above, the checker successfully finds the NPD bug.
[1.zip](https://github.com/llvm/llvm-project/files/11172051/1.zip)
This bug exists at least in llvm9.
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJy0WVtz2rzW_jXKzRoYH4IxF7kAkkwzb5t8E2jmu8vItgzaFZLHkmnor9-zJBnbQNPDm93puMZax2edJJVqzTeSsRsyWZDJ7RVtzFbVN4ZT-cblZsuuMlUcbvas1lxJEs8hF1RuSHQvxH4HAYun1-m0KFMWwLyquYAJiZYQBVFMglsSzGm90chGksBywmhEJRWHH6x7q0dSjQpW0kaYUb5l-TcNo_9vF6Ejs2usJvFtrmo2fmyEuGU1K1nNZM4u86jGVI0h8a1hbwbCcU6SwJnmnp84iUJ4AFMfwCjgBZOGlwcwWwY5bTQDVdofWbMBLoFEcRJOwgk49i8HMEwbyKlmwJ2rTnoSkCjmMhdNwYDES20KLs14S-I7T3G2rOwq9M3bK14Al9yQKG24NHH0ahBhfE_xdW7_zki8cAzdAuyoECr3jMk1fhzS4TegRglP0wrsqLg0sKNckijl0irOt7SG99WynWbmRKSzuG_9qUFDs1CQy53Fr-BAuvwME_y6J9EMyNRLAPyDMoHEtzCABV3p4ErRP9D8B1PlCXo9e624UiEYaWsJt6JDEi_s69KZFi-ARAsSLfjQHuvfZMFhBCGZ3FpeHw1r_GTByeT2TCeZ3npIji8XYnVEpRewS5gcUcSCfs0z9oqMfYhc0GbelT5fB3afed9n7ueo1X5JQsfeybEgorfRPVLMi6JmWo_YW6U0K0ConArn1v0Fga_hiSKb6q_Rxa9x7-tr2Fp_VhP4Kblo6WliXCL6uVj07_Hr58_OmWAoyuf6IDgY1QHgQ5bXyOuaDwoI4DW-uHBUVXr7uny3HEsYfPv68LhOrl-XJErjOJlNo3SSxg6CYa3x8kwcWnbngbgocdSCOSjaY795LwL9GjlWRu8NamaaWkLQOT6sn5PBsN4yKBuZG64kjq8Xrrl5VPKzyj-rfK4fpGEbVpMksGPBzzecjDwj0f3KUMPzuR9D-OGFikXDRcHqcV5VyFdSLnDo5EruWW3smNGHXabEngooa7WDRtoBXSCZf_u-ZdKSMrnhksEeDdN-YGkDdvIMUPc2PpQrszN-LssC2J6KhhrmxhuXqgISTf8uYlMQ1LAaqD6a3AdztVWNKFZNVanarKyLPPcILqk2GriGkgocon8L5uVYOWUvVHThtcm07gA0Wy6_6RP0tbUYpDKQsTZC53Gw4PewPwEva0zLq2EUIvcxoif8GZe0PqiK1dQcp8qfhsE61_NydbTWxZdryKnIGwxW0UUqCgesfpz8Mg5J4Pcq4dg5svxp_jm6ES4eKX-7dO7eqvrOxmrZRrurdStTf-cm3yI6KPhuROK7DbNK_uGysI3DtpVzTrtxW_7zapOxpqI1DLPItku971INK2ZAGKXaUMOwI7_gY42Pu7f1wXekgeNn3P-idZx44Xo3uqwkbnSdx-MNM-tDxVb8ByNRihlqLWuT6Rf0quYbLqlw3kSzTqdvpeiQh2FPhd1seBVLGDJfgqKF4KMw8Eatn1_Gtv5JlL6c6T52hw_Q_NGejN5pXf9WtntGvk4Xttc8HXvNWcX-Xrs5L-uh4A8scHQMnpluhLFVibAvuHyq-iWIv5bwmZXGFuMz32zd28I3BEzurhlcSkon82h3Z9h4eyHwJ9QflcJH0Y-PffeUdU9s9RjLbK5JvHSJgsHyXi2hfn_dazQWiXcweHz8fb_4rhLs1951TRp92DCzarLfas9SSTyZxXMSz49FYTlb208R_EK_sdVh9yANlk-UavuO0eyAJNH8-RNa3Vh0a5tcl9vVUNxH4-L2uK6FP39ajble-bGc2mYdJSRKgESh8x2TGPO5JfPbqKf6TjY7LDyu5DDRnbplu4d5_rTyB4FkQTXPX6jQY79KorTTYovo0-oEEfc83piU_A24gewAtCggVwXDTWCphFDf9f9yNweYEO3OvY_h2qHTjtqn-qvUuapYcREgj28Lkh1gfyTg_JIBAFrvniok1qjkqTLu6uFlYZMfy_oLlRQPEU6Qk4CLT98llxvXec4WrWzN9c-YT3T79fjEQnvwvIeHEmhV1aqqOR4FlOXBfWLBNc0EZuES-Eaqmh23ueOLkh6f1ncknsNv7fVJEtjtPoYuO4C__TsTvPDHyH5IWX0WvPOw9YnOz5NetJPd5x1U1EBNiJFbv0v1cz3HHZPGlsDLg4PF753siFq2m7MLgVoMjrS9zycn2HapPcmHmHLjX8ZjsLWDvrkvnTF_7QDaeHqy5jYHKOwwq3KFbfKtu0cdw3pbq2aztb2FZmpvO5HNPncDDLrJc6Z12QhxgJLLwh3fHv_vFrJm49OITBbh-Aev7CVaujWm0tgybLZuuNk22ThXO3-X7f8ZVbX6D8sNie5LLpgm0X0YhtMomIT4asW1cPW8yZoNsDeOyU0NCIbnIC7tLc1sfFXcxMUsntErdhMmaTCdXSfT-Gp7M0mn13k2C7KwSOPp9SxI0mBWsHIWhbNymuRX_CYKoji4DpJwEl3H8ThkcZFOJwmbTsqABim5DtiOcjFGTWNVb6641g27ScJZOrkSNGNCt3f79Y11MGs2mlwHAo3t2Aw3wv4vgLul17ZFQ3uLTiYWV6hZRXmNnrlDrcZekTHznTHZO03e-zPnsVtfNbW4-eMAWFcwAtab_wYAAP__N6Fi5A">