llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ilia Kuklin (kuilpd) <details> <summary>Changes</summary> This patch does 2 things: 1. Use `llvm::Expected` to create and return errors in `ValueObject::CastToBasicType` and `ValueObject::CastToEnumType` 2. Fix how DIL uses these functions and reports the error. Before this patch, if they generated an error, DIL would try to use the empty ValueObject with an error status, which resulted in a wrong error message or an LLDB crash. --- Full diff: https://github.com/llvm/llvm-project/pull/217431.diff 4 Files Affected: - (modified) lldb/include/lldb/ValueObject/ValueObject.h (+2-2) - (modified) lldb/source/ValueObject/DILEval.cpp (+37-10) - (modified) lldb/source/ValueObject/ValueObject.cpp (+41-71) - (modified) lldb/test/API/commands/frame/var-dil/expr/Casts/TestFrameVarDILCast.py (+14) ``````````diff diff --git a/lldb/include/lldb/ValueObject/ValueObject.h b/lldb/include/lldb/ValueObject/ValueObject.h index c370993d372b7..5d35c30bcf262 100644 --- a/lldb/include/lldb/ValueObject/ValueObject.h +++ b/lldb/include/lldb/ValueObject/ValueObject.h @@ -667,11 +667,11 @@ class ValueObject { // Take a ValueObject that contains a scalar, enum or pointer type, and // cast it to a "basic" type (integer, float or boolean). - lldb::ValueObjectSP CastToBasicType(CompilerType type); + llvm::Expected<lldb::ValueObjectSP> CastToBasicType(CompilerType type); // Take a ValueObject that contain an integer, float or enum, and cast it // to an enum. - lldb::ValueObjectSP CastToEnumType(CompilerType type); + llvm::Expected<lldb::ValueObjectSP> CastToEnumType(CompilerType type); /// If this object represents a C++ class with a vtable, return an object /// that represents the virtual function table. If the object isn't a class diff --git a/lldb/source/ValueObject/DILEval.cpp b/lldb/source/ValueObject/DILEval.cpp index d448444b43eba..c196509850a6a 100644 --- a/lldb/source/ValueObject/DILEval.cpp +++ b/lldb/source/ValueObject/DILEval.cpp @@ -88,10 +88,20 @@ Interpreter::UnaryConversion(lldb::ValueObjectSP valobj, uint32_t location) { if (!uint_bit_size) return uint_bit_size.takeError(); if (bitfield_size < *int_bit_size || - (in_type.IsSigned() && bitfield_size == *int_bit_size)) - return valobj->CastToBasicType(int_type); - if (bitfield_size <= *uint_bit_size) - return valobj->CastToBasicType(uint_type); + (in_type.IsSigned() && bitfield_size == *int_bit_size)) { + auto value_or_err = valobj->CastToBasicType(int_type); + if (!value_or_err) + return llvm::make_error<DILDiagnosticError>( + m_expr, llvm::toString(value_or_err.takeError()), location); + return *value_or_err; + } + if (bitfield_size <= *uint_bit_size) { + auto value_or_err = valobj->CastToBasicType(uint_type); + if (!value_or_err) + return llvm::make_error<DILDiagnosticError>( + m_expr, llvm::toString(value_or_err.takeError()), location); + return *value_or_err; + } // Re-create as a const value with the same underlying type Scalar scalar; bool resolved = valobj->ResolveValue(scalar); @@ -107,8 +117,13 @@ Interpreter::UnaryConversion(lldb::ValueObjectSP valobj, uint32_t location) { CompilerType promoted_type = valobj->GetCompilerType().GetPromotedIntegerType(); - if (promoted_type) - return valobj->CastToBasicType(promoted_type); + if (promoted_type) { + auto value_or_err = valobj->CastToBasicType(promoted_type); + if (!value_or_err) + return llvm::make_error<DILDiagnosticError>( + m_expr, llvm::toString(value_or_err.takeError()), location); + return *value_or_err; + } return valobj; } @@ -1933,14 +1948,26 @@ llvm::Expected<lldb::ValueObjectSP> Interpreter::Visit(const CastNode &node) { case CastKind::eEnumeration: { // FIXME: is this correct for float vector types? if (op_type.GetTypeInfo() & lldb::eTypeIsFloat || op_type.IsInteger() || - op_type.IsEnumerationType()) - return operand->CastToEnumType(target_type); + op_type.IsEnumerationType()) { + auto value_or_err = operand->CastToEnumType(target_type); + if (!value_or_err) + return llvm::make_error<DILDiagnosticError>( + m_expr, llvm::toString(value_or_err.takeError()), + node.GetLocation()); + return *value_or_err; + } break; } case CastKind::eArithmetic: { if (op_type.IsPointerType() || op_type.IsNullPtrType() || - op_type.IsScalarType() || op_type.IsEnumerationType()) - return operand->CastToBasicType(target_type); + op_type.IsScalarType() || op_type.IsEnumerationType()) { + auto value_or_err = operand->CastToBasicType(target_type); + if (!value_or_err) + return llvm::make_error<DILDiagnosticError>( + m_expr, llvm::toString(value_or_err.takeError()), + node.GetLocation()); + return *value_or_err; + } break; } case CastKind::ePointer: { diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp index 5571507a7d545..39516d3359b1a 100644 --- a/lldb/source/ValueObject/ValueObject.cpp +++ b/lldb/source/ValueObject/ValueObject.cpp @@ -3190,7 +3190,8 @@ ValueObject::CastBaseToDerivedType(CompilerType type, uint64_t offset) { return value->Dereference(error); } -lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { +llvm::Expected<lldb::ValueObjectSP> +ValueObject::CastToBasicType(CompilerType type) { bool is_scalar = GetCompilerType().IsScalarType(); bool is_enum = GetCompilerType().IsEnumerationType(); bool is_pointer = @@ -3200,14 +3201,11 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { ExecutionContext exe_ctx(GetExecutionContextRef()); if (!type.IsScalarType()) - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString("target type must be a scalar")); + return llvm::createStringError("target type must be a scalar"); if (!is_scalar && !is_enum && !is_pointer) - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString("argument must be a scalar, enum, or pointer")); + return llvm::createStringError( + "argument must be a scalar, enum, or pointer"); lldb::TargetSP target = GetTargetSP(); uint64_t type_byte_size = 0; @@ -3220,14 +3218,11 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { if (is_pointer) { if (!type.IsInteger() && !type.IsBoolean()) - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString("target type must be an integer or boolean")); + return llvm::createStringError( + "target type must be an integer or boolean"); if (!type.IsBoolean() && type_byte_size < val_byte_size) - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString( - "target type cannot be smaller than the pointer type")); + return llvm::createStringError( + "target type cannot be smaller than the pointer type"); } if (type.IsBoolean()) { @@ -3242,11 +3237,9 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { exe_ctx, type.GetTypeSystem().GetSharedPointer(), !float_value_or_err->isZero(), "result"); else - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorStringWithFormat( - "cannot get value as APFloat: %s", - llvm::toString(float_value_or_err.takeError()).c_str())); + return llvm::createStringErrorV( + "cannot get value as APFloat: {0}", + llvm::toString(float_value_or_err.takeError())); } } @@ -3261,11 +3254,9 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { return ValueObject::CreateValueObjectFromAPInt(exe_ctx, ext, type, "result"); } else - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorStringWithFormat( - "cannot get value as APSInt: %s", - llvm::toString(int_value_or_err.takeError()).c_str())); + return llvm::createStringErrorV( + "cannot get value as APSInt: {0}", + llvm::toString(int_value_or_err.takeError())); } else if (is_scalar && is_float) { llvm::APSInt integer(type_byte_size * CHAR_BIT, !type.IsSigned()); bool is_exact; @@ -3278,13 +3269,13 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { // Casting floating point values that are out of bounds of the target // type is undefined behaviour. if (status & llvm::APFloatBase::opInvalidOp) - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorStringWithFormat( - "invalid type cast detected: %s", - llvm::toString(float_value_or_err.takeError()).c_str())); + return llvm::createStringError("invalid cast from float to integer"); return ValueObject::CreateValueObjectFromAPInt(exe_ctx, integer, type, "result"); + } else { + return llvm::createStringErrorV( + "cannot get value as APFloat: {0}", + llvm::toString(float_value_or_err.takeError())); } } } @@ -3301,11 +3292,9 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { return ValueObject::CreateValueObjectFromAPFloat(exe_ctx, f, type, "result"); } else { - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorStringWithFormat( - "cannot get value as APSInt: %s", - llvm::toString(int_value_or_err.takeError()).c_str())); + return llvm::createStringErrorV( + "cannot get value as APSInt: {0}", + llvm::toString(int_value_or_err.takeError())); } } else { if (is_integer) { @@ -3317,11 +3306,9 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { return ValueObject::CreateValueObjectFromAPFloat(exe_ctx, f, type, "result"); } else { - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorStringWithFormat( - "cannot get value as APSInt: %s", - llvm::toString(int_value_or_err.takeError()).c_str())); + return llvm::createStringErrorV( + "cannot get value as APSInt: {0}", + llvm::toString(int_value_or_err.takeError())); } } if (is_float) { @@ -3333,37 +3320,30 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) { return ValueObject::CreateValueObjectFromAPFloat(exe_ctx, f, type, "result"); } else { - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorStringWithFormat( - "cannot get value as APFloat: %s", - llvm::toString(float_value_or_err.takeError()).c_str())); + return llvm::createStringErrorV( + "cannot get value as APFloat: {0}", + llvm::toString(float_value_or_err.takeError())); } } } } - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString("Unable to perform requested cast")); + return llvm::createStringError("Unable to perform requested cast"); } -lldb::ValueObjectSP ValueObject::CastToEnumType(CompilerType type) { +llvm::Expected<lldb::ValueObjectSP> +ValueObject::CastToEnumType(CompilerType type) { bool is_enum = GetCompilerType().IsEnumerationType(); bool is_integer = GetCompilerType().IsInteger(); bool is_float = HasFloatingRepresentation(GetCompilerType()); ExecutionContext exe_ctx(GetExecutionContextRef()); if (!is_enum && !is_integer && !is_float) - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString( - "argument must be an integer, a float, or an enum")); + return llvm::createStringError( + "argument must be an integer, a float, or an enum"); if (!type.IsEnumerationType()) - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString("target type must be an enum")); + return llvm::createStringError("target type must be an enum"); lldb::TargetSP target = GetTargetSP(); uint64_t byte_size = 0; @@ -3382,17 +3362,12 @@ lldb::ValueObjectSP ValueObject::CastToEnumType(CompilerType type) { // Casting floating point values that are out of bounds of the target // type is undefined behaviour. if (status & llvm::APFloatBase::opInvalidOp) - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString("invalid cast from float to integer")); + return llvm::createStringError("invalid cast from float to integer"); return ValueObject::CreateValueObjectFromAPInt(exe_ctx, integer, type, "result"); } else - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorStringWithFormatv( - "cannot get value as APFloat: {0}", - llvm::toString(value_or_err.takeError()))); + return llvm::createStringErrorV("cannot get value as APFloat: {0}", + llvm::toString(value_or_err.takeError())); } else { // Get the value as APSInt and extend or truncate it to the requested size. auto value_or_err = GetValueAsAPSInt(); @@ -3401,15 +3376,10 @@ lldb::ValueObjectSP ValueObject::CastToEnumType(CompilerType type) { return ValueObject::CreateValueObjectFromAPInt(exe_ctx, ext, type, "result"); } else - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorStringWithFormat( - "cannot get value as APSInt: %s", - llvm::toString(value_or_err.takeError()).c_str())); + return llvm::createStringErrorV("cannot get value as APSInt: {0}", + llvm::toString(value_or_err.takeError())); } - return ValueObjectConstResult::Create( - exe_ctx.GetBestExecutionContextScope(), - Status::FromErrorString("Cannot perform requested cast")); + return llvm::createStringError("Cannot perform requested cast"); } ValueObject::EvaluationPoint::EvaluationPoint() : m_mod_id(), m_exe_ctx_ref() {} diff --git a/lldb/test/API/commands/frame/var-dil/expr/Casts/TestFrameVarDILCast.py b/lldb/test/API/commands/frame/var-dil/expr/Casts/TestFrameVarDILCast.py index 567981cd670e9..7bde0e91cada8 100644 --- a/lldb/test/API/commands/frame/var-dil/expr/Casts/TestFrameVarDILCast.py +++ b/lldb/test/API/commands/frame/var-dil/expr/Casts/TestFrameVarDILCast.py @@ -322,3 +322,17 @@ def test_type_cast(self): error=True, substrs=["Cast from 'InnerFoo' to 'UnscopedEnum' is not allowed"], ) + + # Check that failed casts output errors with diagnostics + self.expect( + "script lldb.frame.GetValueForVariablePath('+(int) finf')", + substrs=["<user expression>:1:2: invalid cast from float to integer"], + ) + self.expect( + "script lldb.frame.GetValueForVariablePath('+(float) *((int *) 0)')", + substrs=["<user expression>:1:2: cannot get value as APSInt"], + ) + self.expect( + "script lldb.frame.GetValueForVariablePath('+(UnscopedEnum) *((float *) 0)')", + substrs=["<user expression>:1:2: cannot get value as APFloat"], + ) `````````` </details> https://github.com/llvm/llvm-project/pull/217431 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
