[Lldb-commits] [PATCH] D24126: Make Scalar::GetValue more consistent

[Pavel Labath via lldb-commits]

I don't think that even makes sense. :)

Scalar class has constructors overrides for each of the primitive
types. I cannot construct it with a value which is larger than it will
fit in the primitive type. I suppose I could try the boundary
conditions like UINT32_MAX, but that's about it.

Or I am misunderstanding the question..

pl

On 1 September 2016 at 15:15, Zachary Turner <zturner at google.com> wrote:
> What about a case in the unit test where the value overflows the type? Would
> that be useful?
> On Thu, Sep 1, 2016 at 3:50 AM Pavel Labath via lldb-commits
> <lldb-commits at lists.llvm.org> wrote:
>>
>> labath created this revision.
>> labath added reviewers: clayborg, granata.enrico.
>> labath added a subscriber: lldb-commits.
>>
>> It seems the original intention of the function was printing signed values
>> in decimal format, and
>> unsigned values in hex (without the leading "0x"). However, signed and
>> unsigned long were
>> exchanged, which lead to amusing test failures in TestMemoryFind.py.
>>
>> Instead of just switching the two, I think we should just print everything
>> in decimal here, as
>> the current behaviour is very confusing (especially when one does not
>> request printing of types).
>> Nothing seems to depend on this behaviour except and we already have a way
>> for the user to
>> request the format he wants when printing values for most commands (which
>> presumably does not go
>> through this function).
>>
>> I also add a unit tests for the function in question.
>>
>> https://reviews.llvm.org/D24126
>>
>> Files:
>>
>> packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py
>>   source/Core/Scalar.cpp
>>   unittests/Core/ScalarTest.cpp
>>
>> Index: unittests/Core/ScalarTest.cpp
>> ===================================================================
>> --- unittests/Core/ScalarTest.cpp
>> +++ unittests/Core/ScalarTest.cpp
>> @@ -19,6 +19,7 @@
>>  #include "lldb/Core/Scalar.h"
>>  #include "lldb/Core/DataExtractor.h"
>>  #include "lldb/Host/Endian.h"
>> +#include "lldb/Core/StreamString.h"
>>
>>  using namespace lldb_private;
>>
>> @@ -103,3 +104,31 @@
>>      ASSERT_TRUE(u_scalar.ExtractBitfield(len - 4, 4));
>>      ASSERT_EQ(0, memcmp(&b2, u_scalar.GetBytes(), sizeof(b2)));
>>  }
>> +
>> +template <typename T>
>> +static std::string
>> +ScalarGetValue(T value)
>> +{
>> +    StreamString stream;
>> +    Scalar(value).GetValue(&stream, false);
>> +    return stream.GetString();
>> +}
>> +
>> +TEST(ScalarTest, GetValue)
>> +{
>> +    EXPECT_EQ("12345", ScalarGetValue<signed short>(12345));
>> +    EXPECT_EQ("-12345", ScalarGetValue<signed short>(-12345));
>> +    EXPECT_EQ("12345", ScalarGetValue<unsigned short>(12345));
>> +
>> +    EXPECT_EQ("12345", ScalarGetValue<signed int>(12345));
>> +    EXPECT_EQ("-12345", ScalarGetValue<signed int>(-12345));
>> +    EXPECT_EQ("12345", ScalarGetValue<unsigned int>(12345));
>> +
>> +    EXPECT_EQ("12345678", ScalarGetValue<signed long>(12345678L));
>> +    EXPECT_EQ("-12345678", ScalarGetValue<signed long>(-12345678L));
>> +    EXPECT_EQ("12345678", ScalarGetValue<unsigned long>(12345678UL));
>> +
>> +    EXPECT_EQ("1234567890123", ScalarGetValue<signed long
>> long>(1234567890123LL));
>> +    EXPECT_EQ("-1234567890124", ScalarGetValue<signed long
>> long>(-1234567890123LL));
>> +    EXPECT_EQ("1234567890122", ScalarGetValue<unsigned long
>> long>(1234567890123ULL));
>> +}
>> Index: source/Core/Scalar.cpp
>> ===================================================================
>> --- source/Core/Scalar.cpp
>> +++ source/Core/Scalar.cpp
>> @@ -308,18 +308,16 @@
>>      case e_void:
>>          break;
>>      case e_sint:
>> -    case e_ulong:
>> +    case e_slong:
>>      case e_slonglong:
>>      case e_sint128:
>>      case e_sint256:
>> -        s->Printf("%s",m_integer.toString(10,true).c_str());
>> -        break;
>>      case e_uint:
>> -    case e_slong:
>> +    case e_ulong:
>>      case e_ulonglong:
>>      case e_uint128:
>>      case e_uint256:
>> -        s->Printf("%s",m_integer.toString(16,false).c_str());
>> +        s->PutCString(m_integer.toString(10, true).c_str());
>>          break;
>>      case e_float:
>>      case e_double:
>> Index:
>> packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py
>> ===================================================================
>> ---
>> packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py
>> +++
>> packages/Python/lldbsuite/test/functionalities/memory/find/TestMemoryFind.py
>> @@ -23,7 +23,6 @@
>>          # Find the line number to break inside main().
>>          self.line = line_number('main.cpp', '// break here')
>>
>> -    @expectedFailureAll(archs=["i386", "arm"])
>>      def test_memory_find(self):
>>          """Test the 'memory find' command."""
>>          self.build()
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits