[cfe-dev] CheckConstantExpression issue
Serge Preis via cfe-dev
cfe-dev at lists.llvm.org
Fri Jul 7 02:32:05 PDT 2017
Thank you, Mikhail.
Yes, this definitely works. My question is not about making my code working: I already fixed it. My question is what checks should belong to callee (VarDecl::evaluateValue()) opposed to caller (my code) and what actions are to be taken upon these checks.
I believe that something broken in VarDecl::evaluateValue() logic:
(a) It already encorporates quite a few checks and returns nullptr on a quite a few occasions
(b) On some other occasions (pretty similar IMO to ones above) it only has asserts
(c) Callee lacks some checks even in form of assetions making assumptions those are not that trivial and basic.
So I started this thread not in order to make my code working, but in attempt to seek a way to improve clang usability around VarDecl::evaluateValue() functionality. So maybe it is worth adding
+ if (getDeclContext()->isDependentContext()) {
+ return nullptr;
+ }
to VarDecl::evaluateValue() implementation (although this may be too broad check and varDecl->getType()->isDependentType() is more precise and appropriate).
I would also added the following checks:
+ if (!ensureEvaluatedStmt() || !ensureEvaluatedStmt()->Value) {
+ return nullptr
+ }
and
const auto *Init = cast<Expr>(Eval->Value);
+ if (!Init || Init->getType().isNull() || Init->isValueDependent() || Init->isTypeDependent()) {
+ return nullptr;
+ }
Thank you,
Serge.
06.07.2017, 21:34, "Mikhail Ramalho" <mikhail.ramalho at gmail.com>:
> Have you tried to check if varDecl is in a dependent context before calling the CheckConstantExpression?
>
> varDecl->getDeclContext()->isDependentContext()
>
> Thank you,
>
> 2017-07-06 5:48 GMT+01:00 Serge Preis via cfe-dev <cfe-dev at lists.llvm.org>:
>> Hello,
>>
>> thank you for you prompt reply.
>>
>> I did exactly this in my code, but I am not sure asserts are enough. I already added checks to avoid some issues like this:
>>
>>>> varDecl->ensureEvaluatedStmt() != nullptr &&
>>>> varDecl->ensureEvaluatedStmt()->Value != nullptr)
>>>> !init->isValueDependent() &&
>>
>> Against following code in Decl.cpp:
>>
>> ....
>> APValue *VarDecl::evaluateValue(
>> SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
>> EvaluatedStmt *Eval = ensureEvaluatedStmt();
>> ... // .... skipped comment ...
>> if (Eval->WasEvaluated) /// <<<<< Here non-NULL pointer is blindly assumed
>> return Eval->Evaluated.isUninit() ? nullptr : &Eval->Evaluated;
>>
>> const auto *Init = cast<Expr>(Eval->Value);
>> assert(!Init->isValueDependent()); /// <<<< Here is just assert which only fires in debug mode
>>
>> if (Eval->IsEvaluating) {
>> ...
>>
>> I think at least asserts should be added to VarDecl::evaluateValue() to ensure Eval is not NULL and the type of 'this' VarDecl is not DependentType. But in this case all prerequisites for evaluateValue are better to be somehow documented.
>>
>> Now for the simple varDecl->evaluateValue() we have at least (+ maybe something else?)
>> (varDecl->getAnyInitializer() != nullptr) /// This
>> !init->getType().isNull() && /// and this
>> !init->isValueDependent() &&
>> !init->isTypeDependent() &&
>> !varDecl->getType().isNull() && /// and this --- seem reasonable to have outside the call, but I am not that sure about all others
>> !varDecl->getType()->isDependentType() &&
>> varDecl->ensureEvaluatedStmt() != nullptr &&
>> varDecl->ensureEvaluatedStmt()->Value != nullptr)
>>
>> Alternatively let the function return NULL in all unsupported cases basically placing all required checks insde the function. This function already returns NULL on various occasions, so these additional checks seem to me natutal fit to the function's logic.
>>
>> I am ready to prepare patch for review based on agreed decision.
>>
>> Regards,
>> Serge.
>>
>> 05.07.2017, 22:47, "Alex L" <arphaman at gmail.com>:
>>> It looks like you're trying to evaluate a variable with a dependent type. I think that `evaluateValue` function is not intended for such variables. I think that it might be a good idea to put an assertion into that function that checks if the variables or its type is dependent, to complement the assertion for the dependent initializer in that function.
>>>
>>> On 5 July 2017 at 09:42, Serge Preis via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>>>> Hello,
>>>>
>>>> I encountered the following issue in lib/AST/ExprConstant.cpp:
>>>>
>>>> In function CheckConstantExpr there is a code:
>>>>
>>>> if (Value.isUnion() && Value.getUnionField()) {
>>>> return CheckConstantExpression(Info, DiagLoc,
>>>> Value.getUnionField()->getType(),
>>>> Value.getUnionValue());
>>>> }
>>>> if (Value.isStruct()) {
>>>> RecordDecl *RD = Type->castAs<RecordType>()->getDecl(); //// <<<<<< The problematic line
>>>> if (const CXXRecordDecl *CD = dyn_cast<CXXRecordDecl>(RD)) {
>>>> unsigned BaseIndex = 0;
>>>> for (CXXRecordDecl::base_class_const_iterator I = CD->bases_begin(),
>>>> End = CD->bases_end(); I != End; ++I, ++BaseIndex) {
>>>> if (!CheckConstantExpression(Info, DiagLoc, I->getType(),
>>>> Value.getStructBase(BaseIndex)))
>>>> return false;
>>>>
>>>> The problematic line causes a crash because Type cannot be cast to RecordType.
>>>>
>>>> This looks like an issue in clang to me, but the question is what's wrong: Value.isStruct() returning true for constant of template parameter type (see below) or absence of check for Type to be RecordType.
>>>> I will definitely impelement workaround in my code, but I would also like to investingate and fix this issue in clang.
>>>>
>>>> Regards,
>>>> Serge.
>>>>
>>>> ----- useful info ----
>>>>
>>>> The code which triggering this behavior looks like the following:
>>>> ...
>>>> template<class T>
>>>> void DoTestMax() {
>>>> const T max = Max();
>>>> ...
>>>>
>>>> I am trying to access and print constant initializer for 'max' variable under following checks:
>>>> } else if (auto* varDecl = llvm::dyn_cast<clang::VarDecl>(decl)) {
>>>> if (!clang::isa<clang::ParmVarDecl>(varDecl) && (varDecl->isConstexpr() || varDecl->getType().isConstQualified())) {
>>>> const clang::Expr* init = varDecl->getAnyInitializer();
>>>> if (init != nullptr &&
>>>> !init->getType().isNull() &&
>>>> !init->isValueDependent() &&
>>>> varDecl->ensureEvaluatedStmt() != nullptr &&
>>>> varDecl->ensureEvaluatedStmt()->Value != nullptr)
>>>> {
>>>> const clang::APValue* val = varDecl->evaluateValue(); /// <<< Problem is here
>>>>
>>>> The relevant stack for debug clang (trunk) looks like below:
>>>> #3 0x00007ffff6bb2ca2 in __GI___assert_fail (assertion=0x2661ae0 "isa<X>(Val) && \"cast<Ty>() argument of incompatible type!\"", file=0x2661aa0 "/home/spreis/LLVM/latest/llvm/include/llvm/Support/Casting.h", line=241,
>>>> function=0x2663de0 <std::enable_if<!llvm::is_simple_type<clang::QualType>::value, llvm::cast_retty<clang::RecordType, clang::QualType const>::ret_type>::type llvm::cast<clang::RecordType, clang::QualType>(clang::QualType const&)::__PRETTY_FUNCTION__> "typename std::enable_if<(! llvm::is_simple_type<Y>::value), typename llvm::cast_retty<X, const Y>::ret_type>::type llvm::cast(const Y&) [with X = clang::RecordType; Y = clang::QualType; typename std::"...)
>>>> at assert.c:101
>>>> #4 0x00000000010cf75e in llvm::cast<clang::RecordType, clang::QualType> (Val=...)
>>>> at /home/spreis/LLVM/latest/llvm/include/llvm/Support/Casting.h:241
>>>> #5 0x00000000010ce4fa in clang::Type::castAs<clang::RecordType> (this=0x554a880)
>>>> at /home/spreis/LLVM/latest/llvm/tools/clang/include/clang/AST/TypeNodes.def:122
>>>> #6 0x0000000001c0b810 in CheckConstantExpression (Info=..., DiagLoc=..., Type=..., Value=...)
>>>> at /home/spreis/LLVM/latest/llvm/tools/clang/lib/AST/ExprConstant.cpp:1737
>>>> #7 0x0000000001c2f1b0 in clang::Expr::EvaluateAsInitializer (this=0x554ab50, Value=..., Ctx=..., VD=0x554a9d8, Notes=...)
>>>> at /home/spreis/LLVM/latest/llvm/tools/clang/lib/AST/ExprConstant.cpp:10079
>>>> #8 0x0000000001b923fa in clang::VarDecl::evaluateValue (this=0x554a9d8, Notes=...)
>>>> at /home/spreis/LLVM/latest/llvm/tools/clang/lib/AST/Decl.cpp:2172
>>>> #9 0x0000000001b9229c in clang::VarDecl::evaluateValue (this=0x554a9d8)
>>>> at /home/spreis/LLVM/latest/llvm/tools/clang/lib/AST/Decl.cpp:2146
>>>>
>>>> The var:
>>>> (gdb) frame 9
>>>> 2146 return evaluateValue(Notes);
>>>> (gdb) call this->dump()
>>>> VarDecl 0x554a9d8 <..snip..snip> referenced max 'const T' cinit
>>>> `-CallExpr 0x554ab50 <col:23, col:27> '::NPrivate::TMax':'struct NPrivate::TMax'
>>>> `-ImplicitCastExpr 0x554ab38 <col:23> '::NPrivate::TMax (*)(void) noexcept' <FunctionToPointerDecay>
>>>> `-DeclRefExpr 0x554aab8 <col:23> '::NPrivate::TMax (void) noexcept' lvalue Function 0x3f82c28 'Max' '::NPrivate::TMax (void) noexcept'
>>>>
>>>> The type:
>>>> (gdb) frame 6
>>>> 1737 RecordDecl *RD = Type->castAs<RecordType>()->getDecl();
>>>> (gdb) call Type.dump()
>>>> QualType 0x554a881 'const T' const
>>>> `-TemplateTypeParmType 0x554a880 'T' dependent depth 0 index 0
>>>> `-TemplateTypeParm 0x554a840 'T'
>>>>
>>>> _______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
> --
>
> Mikhail Ramalho.
More information about the cfe-dev
mailing list