From 729dc4372292e976372e5cbba45af50dfba66f29 Mon Sep 17 00:00:00 2001 From: Rene Gollent Date: Sun, 22 Dec 2013 09:31:29 -0500 Subject: [PATCH] Debugger: Fix #10334. - In the case of DWARF4's DW_OP_implicit_value, respective handling in Value{Piece}Location wasn't entirely correct, which would lead to double frees in various cases, in particular when copying such objects on the stack. As such, introduce an explicit Copy() member to handle replicating the stored implicit data to avoid that issue. Furthermore, the use of Array<> in ValueLocation wasn't really suitable anymore since ValuePieceLocation now needs its constructor/destructor to be executed, while Array is intended for POD types; consequently, replace that with std::vector and adjust handling code accordingly. --- src/apps/debugger/debug_info/DwarfTypes.cpp | 34 ++++++++++---- src/apps/debugger/types/ValueLocation.cpp | 51 +++++++++++++-------- src/apps/debugger/types/ValueLocation.h | 44 ++++++++++++++++-- 3 files changed, 98 insertions(+), 31 deletions(-) diff --git a/src/apps/debugger/debug_info/DwarfTypes.cpp b/src/apps/debugger/debug_info/DwarfTypes.cpp index 34b26e933b..fdf15f774b 100644 --- a/src/apps/debugger/debug_info/DwarfTypes.cpp +++ b/src/apps/debugger/debug_info/DwarfTypes.cpp @@ -306,7 +306,10 @@ DwarfType::ResolveObjectDataLocation(const ValueLocation& objectLocation, if (count == 0) return B_BAD_VALUE; - ValuePieceLocation piece = objectLocation.PieceAt(0); + ValuePieceLocation piece; + if (!piece.Copy(objectLocation.PieceAt(0))) + return B_NO_MEMORY; + if (count > 1 || piece.type != VALUE_PIECE_LOCATION_MEMORY || piece.size != 0 || piece.bitSize != 0) { ValueLocation* location @@ -376,7 +379,10 @@ DwarfType::ResolveLocation(DwarfTypeContext* typeContext, bool bigEndian = typeContext->GetArchitecture()->IsBigEndian(); int32 count = _location.CountPieces(); for (int32 i = 0; i < count; i++) { - ValuePieceLocation piece = _location.PieceAt(i); + ValuePieceLocation piece; + if (!piece.Copy(_location.PieceAt(i))) + return B_NO_MEMORY; + if (piece.type == VALUE_PIECE_LOCATION_REGISTER) { int32 reg = typeContext->FromDwarfRegisterMap()->MapRegisterIndex( piece.reg); @@ -403,17 +409,22 @@ DwarfType::ResolveLocation(DwarfTypeContext* typeContext, } piece.Normalize(bigEndian); - _location.SetPieceAt(i, piece); + if (!_location.SetPieceAt(i, piece)) + return B_NO_MEMORY; } // If we only have one piece and that doesn't have a size, try to retrieve // the size of the type. if (count == 1) { - ValuePieceLocation piece = _location.PieceAt(0); + ValuePieceLocation piece; + if (!piece.Copy(_location.PieceAt(0))) + return B_NO_MEMORY; + if (piece.IsValid() && piece.size == 0 && piece.bitSize == 0) { piece.SetSize(ByteSize()); // TODO: Use bit size and bit offset, if specified! - _location.SetPieceAt(0, piece); + if (!_location.SetPieceAt(0, piece)) + return B_NO_MEMORY; TRACE_LOCALS(" set single piece size to %" B_PRIu64 "\n", ByteSize()); @@ -904,7 +915,8 @@ DwarfCompoundType::_ResolveDataMemberLocation(DwarfType* memberType, // location to be a memory location. if (parentLocation.CountPieces() != 1) return B_BAD_VALUE; - ValuePieceLocation piece = parentLocation.PieceAt(0); + const ValuePieceLocation& piece = parentLocation.PieceAt(0); + if (piece.type != VALUE_PIECE_LOCATION_MEMORY) return B_BAD_VALUE; @@ -938,7 +950,10 @@ DwarfCompoundType::_ResolveDataMemberLocation(DwarfType* memberType, // the location by hand since we don't want the size difference // between the overall union and the member being // factored into the assigned address. - ValuePieceLocation piece = parentLocation.PieceAt(0); + ValuePieceLocation piece; + if (!piece.Copy(parentLocation.PieceAt(0))) + return B_NO_MEMORY; + piece.SetSize(memberType->ByteSize()); if (!location->AddPiece(piece)) return B_NO_MEMORY; @@ -1123,7 +1138,10 @@ DwarfArrayType::ResolveElementLocation(const ArrayIndexPath& indexPath, // If we have a single memory piece location for the array, we compute the // element's location by hand -- not uncommonly the array size isn't known. if (parentLocation.CountPieces() == 1) { - ValuePieceLocation piece = parentLocation.PieceAt(0); + ValuePieceLocation piece; + if (!piece.Copy(parentLocation.PieceAt(0))) + return B_NO_MEMORY; + if (piece.type == VALUE_PIECE_LOCATION_MEMORY) { int64 byteOffset = elementOffset >= 0 ? elementOffset / 8 : (elementOffset - 7) / 8; diff --git a/src/apps/debugger/types/ValueLocation.cpp b/src/apps/debugger/types/ValueLocation.cpp index c8c0713017..beda735bab 100644 --- a/src/apps/debugger/types/ValueLocation.cpp +++ b/src/apps/debugger/types/ValueLocation.cpp @@ -4,6 +4,7 @@ * Distributed under the terms of the MIT License. */ +#include #include "ValueLocation.h" @@ -101,7 +102,7 @@ ValueLocation::SetTo(const ValueLocation& other, uint64 bitOffset, int32 count = other.CountPieces(); uint64 totalBitSize = 0; for (int32 i = 0; i < count; i++) { - ValuePieceLocation piece = other.PieceAt(i); + const ValuePieceLocation &piece = other.PieceAt(i); totalBitSize += piece.bitSize; } @@ -122,10 +123,13 @@ ValueLocation::SetTo(const ValueLocation& other, uint64 bitOffset, int32 i; ValuePieceLocation piece; for (i = 0; i < count; i++) { - piece = other.PieceAt(i); - if (piece.bitSize > bitsToSkip) + const ValuePieceLocation& tempPiece = other.PieceAt(i); + if (tempPiece.bitSize > bitsToSkip) { + if (!piece.Copy(tempPiece)) + return false; break; - bitsToSkip -= piece.bitSize; + } + bitsToSkip -= tempPiece.bitSize; } // handle partial piece @@ -151,7 +155,8 @@ ValueLocation::SetTo(const ValueLocation& other, uint64 bitOffset, if (++i >= count) break; - piece = other.PieceAt(i); + if (!piece.Copy(other.PieceAt(i))) + return false; } } else { // Little endian: Skip the superfluous least significant bits, copy the @@ -163,9 +168,12 @@ ValueLocation::SetTo(const ValueLocation& other, uint64 bitOffset, int32 i; ValuePieceLocation piece; for (i = 0; i < count; i++) { - piece = other.PieceAt(i); - if (piece.bitSize > bitsToSkip) + const ValuePieceLocation& tempPiece = other.PieceAt(i); + if (tempPiece.bitSize > bitsToSkip) { + if (!piece.Copy(tempPiece)) + return false; break; + } bitsToSkip -= piece.bitSize; } @@ -192,7 +200,8 @@ ValueLocation::SetTo(const ValueLocation& other, uint64 bitOffset, if (++i >= count) break; - piece = other.PieceAt(i); + if (!piece.Copy(other.PieceAt(i))) + return false; } } @@ -203,7 +212,7 @@ ValueLocation::SetTo(const ValueLocation& other, uint64 bitOffset, void ValueLocation::Clear() { - fPieces.Clear(); + fPieces.clear(); } @@ -212,34 +221,40 @@ ValueLocation::AddPiece(const ValuePieceLocation& piece) { // Just add, don't normalize. This allows for using the class with different // semantics (e.g. in the DWARF code). - return fPieces.Add(piece); + try { + fPieces.push_back(piece); + } catch (...) { + return false; + } + + return true; } int32 ValueLocation::CountPieces() const { - return fPieces.Size(); + return fPieces.size(); } ValuePieceLocation ValueLocation::PieceAt(int32 index) const { - if (index < 0 || index >= fPieces.Size()) + if (index < 0 || index >= (int32)fPieces.size()) return ValuePieceLocation(); - return fPieces.ElementAt(index); + return fPieces[index]; } -void +bool ValueLocation::SetPieceAt(int32 index, const ValuePieceLocation& piece) { - if (index < 0 || index >= fPieces.Size()) - return; + if (index < 0 || index >= (int32)fPieces.size()) + return false; - fPieces.ElementAt(index) = piece; + return fPieces[index].Copy(piece); } @@ -255,7 +270,7 @@ ValueLocation::operator=(const ValueLocation& other) void ValueLocation::Dump() const { - int32 count = fPieces.Size(); + int32 count = fPieces.size(); printf("ValueLocation: %s endian, %" B_PRId32 " pieces:\n", fBigEndian ? "big" : "little", count); diff --git a/src/apps/debugger/types/ValueLocation.h b/src/apps/debugger/types/ValueLocation.h index ede62a8208..ac6eb3b964 100644 --- a/src/apps/debugger/types/ValueLocation.h +++ b/src/apps/debugger/types/ValueLocation.h @@ -6,8 +6,11 @@ #ifndef VALUE_LOCATION_H #define VALUE_LOCATION_H +#include + +#include +#include -#include #include #include "Types.h" @@ -43,12 +46,43 @@ struct ValuePieceLocation { { } + ValuePieceLocation(const ValuePieceLocation& other) + { + if (!Copy(other)) + throw std::bad_alloc(); + } + ~ValuePieceLocation() { if (value != NULL) free(value); } + ValuePieceLocation& operator=(const ValuePieceLocation& other) + { + if (!Copy(other)) + throw std::bad_alloc(); + + return *this; + } + + bool Copy(const ValuePieceLocation& other) + { + memcpy(this, &other, sizeof(ValuePieceLocation)); + if (type == VALUE_PIECE_LOCATION_IMPLICIT) { + void* tempValue = malloc(size); + if (tempValue == NULL) { + type = VALUE_PIECE_LOCATION_INVALID; + return false; + } + + memcpy(tempValue, value, other.size); + value = tempValue; + } + + return true; + } + bool IsValid() const { return type != VALUE_PIECE_LOCATION_INVALID; @@ -107,6 +141,7 @@ public: ValueLocation(bool bigEndian); ValueLocation(bool bigEndian, const ValuePieceLocation& piece); + ValueLocation(const ValueLocation& other); bool SetToByteOffset(const ValueLocation& other, @@ -123,18 +158,17 @@ public: int32 CountPieces() const; ValuePieceLocation PieceAt(int32 index) const; - void SetPieceAt(int32 index, + bool SetPieceAt(int32 index, const ValuePieceLocation& piece); - ValueLocation& operator=(const ValueLocation& other); void Dump() const; private: - typedef Array PieceArray; + typedef std::vector PieceVector; private: - PieceArray fPieces; + PieceVector fPieces; bool fBigEndian; };