[Lldb-commits] [lldb] r268520 - Fix a SIGSEGV caused by dereferencing a pointer without a null check

[Bryan Chan via lldb-commits]

Author: bryanpkc
Date: Wed May  4 12:24:31 2016
New Revision: 268520

URL: http://llvm.org/viewvc/llvm-project?rev=268520&view=rev
Log:
Fix a SIGSEGV caused by dereferencing a pointer without a null check

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp?rev=268520&r1=268519&r2=268520&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp Wed May  4 
12:24:31 2016
@@ -943,7 +943,7 @@ DWARFCompileUnit::IndexPrivate (DWARFCom
 // as our name. If it starts with '_', then it is ok, else 
compare
 // the string to make sure it isn't the same and we don't 
end up
 // with duplicate entries
-if (name != mangled_cstr && ((mangled_cstr[0] == '_') || 
(name && ::strcmp(name, mangled_cstr) != 0)))
+if (name && name != mangled_cstr && ((mangled_cstr[0] == 
'_') || (::strcmp(name, mangled_cstr) != 0)))
 {
 Mangled mangled (ConstString(mangled_cstr), true);
 func_fullnames.Insert (mangled.GetMangledName(), 
DIERef(cu_offset, die.GetOffset()));
@@ -966,7 +966,7 @@ DWARFCompileUnit::IndexPrivate (DWARFCom
 // as our name. If it starts with '_', then it is ok, else 
compare
 // the string to make sure it isn't the same and we don't 
end up
 // with duplicate entries
-if (name != mangled_cstr && ((mangled_cstr[0] == '_') || 
(::strcmp(name, mangled_cstr) != 0)))
+if (name && name != mangled_cstr && ((mangled_cstr[0] == 
'_') || (::strcmp(name, mangled_cstr) != 0)))
 {
 Mangled mangled (ConstString(mangled_cstr), true);
 func_fullnames.Insert (mangled.GetMangledName(), 
DIERef(cu_offset, die.GetOffset()));


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D20355: Avoid an assertion failure when a bit field is extracted from a value of the same size.

[Bryan Chan via lldb-commits]

bryanpkc created this revision.
bryanpkc added a reviewer: uweigand.
bryanpkc added a subscriber: lldb-commits.

One of the cases handled by ValueObjectChild::UpdateValue() uses the entire 
width of the parent's scalar value as the size of the child, and extracts the 
child by calling Scalar::ExtractBitfield(). This seems valid but 
APInt::trunc(), APInt::sext() and APInt::zext() assert that the bit field must 
not have the same size as the parent scalar. Replacing those calls with 
sextOrTrunc(), zextOrTrunc(), sextOrSelf() and zextOrSelf() fixes the assertion 
failures.

http://reviews.llvm.org/D20355

Files:
  source/Core/Scalar.cpp

Index: source/Core/Scalar.cpp
===
--- source/Core/Scalar.cpp
+++ source/Core/Scalar.cpp
@@ -2788,15 +2788,15 @@
 case Scalar::e_slonglong:
 case Scalar::e_sint128:
 case Scalar::e_sint256:
-m_integer = m_integer.ashr(bit_offset).trunc(bit_size).sext(8 * 
GetByteSize());
+m_integer = 
m_integer.ashr(bit_offset).sextOrTrunc(bit_size).sextOrSelf(8 * GetByteSize());
 return true;
 
 case Scalar::e_uint:
 case Scalar::e_ulong:
 case Scalar::e_ulonglong:
 case Scalar::e_uint128:
 case Scalar::e_uint256:
-m_integer = m_integer.lshr(bit_offset).trunc(bit_size).zext(8 * 
GetByteSize());
+m_integer = 
m_integer.lshr(bit_offset).zextOrTrunc(bit_size).zextOrSelf(8 * GetByteSize());
 return true;
 }
 return false;


Index: source/Core/Scalar.cpp
===
--- source/Core/Scalar.cpp
+++ source/Core/Scalar.cpp
@@ -2788,15 +2788,15 @@
 case Scalar::e_slonglong:
 case Scalar::e_sint128:
 case Scalar::e_sint256:
-m_integer = m_integer.ashr(bit_offset).trunc(bit_size).sext(8 * GetByteSize());
+m_integer = m_integer.ashr(bit_offset).sextOrTrunc(bit_size).sextOrSelf(8 * GetByteSize());
 return true;
 
 case Scalar::e_uint:
 case Scalar::e_ulong:
 case Scalar::e_ulonglong:
 case Scalar::e_uint128:
 case Scalar::e_uint256:
-m_integer = m_integer.lshr(bit_offset).trunc(bit_size).zext(8 * GetByteSize());
+m_integer = m_integer.lshr(bit_offset).zextOrTrunc(bit_size).zextOrSelf(8 * GetByteSize());
 return true;
 }
 return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D20355: Avoid an assertion failure when a bit field is extracted from a value of the same size.

[Bryan Chan via lldb-commits]

bryanpkc updated this revision to Diff 57695.
bryanpkc added a comment.

Added a unit test in ScalarTest.cpp that catches this particular error.


http://reviews.llvm.org/D20355

Files:
  source/Core/Scalar.cpp
  unittests/Core/ScalarTest.cpp

Index: unittests/Core/ScalarTest.cpp
===
--- unittests/Core/ScalarTest.cpp
+++ unittests/Core/ScalarTest.cpp
@@ -79,3 +79,27 @@
 ASSERT_EQ((unsigned long long)a, a_scalar.ULongLong());
 }
 
+TEST(ScalarTest, ExtractBitfield)
+{
+uint32_t len = sizeof(long long) * 8;
+
+long long a1 = 0xf1f2f3f4f5f6f7f8LL;
+long long b1 = 0xff1f2f3f4f5f6f7fLL;
+Scalar s_scalar(a1);
+ASSERT_TRUE(s_scalar.ExtractBitfield(0, 0));
+ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+ASSERT_TRUE(s_scalar.ExtractBitfield(len, 0));
+ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+ASSERT_TRUE(s_scalar.ExtractBitfield(len - 4, 4));
+ASSERT_EQ(0, memcmp(&b1, s_scalar.GetBytes(), sizeof(b1)));
+
+unsigned long long a2 = 0xf1f2f3f4f5f6f7f8ULL;
+unsigned long long b2 = 0x0f1f2f3f4f5f6f7fULL;
+Scalar u_scalar(a2);
+ASSERT_TRUE(u_scalar.ExtractBitfield(0, 0));
+ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2)));
+ASSERT_TRUE(u_scalar.ExtractBitfield(len, 0));
+ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2)));
+ASSERT_TRUE(u_scalar.ExtractBitfield(len - 4, 4));
+ASSERT_EQ(0, memcmp(&b2, u_scalar.GetBytes(), sizeof(b2)));
+}
Index: source/Core/Scalar.cpp
===
--- source/Core/Scalar.cpp
+++ source/Core/Scalar.cpp
@@ -2788,15 +2788,15 @@
 case Scalar::e_slonglong:
 case Scalar::e_sint128:
 case Scalar::e_sint256:
-m_integer = m_integer.ashr(bit_offset).trunc(bit_size).sext(8 * 
GetByteSize());
+m_integer = 
m_integer.ashr(bit_offset).sextOrTrunc(bit_size).sextOrSelf(8 * GetByteSize());
 return true;
 
 case Scalar::e_uint:
 case Scalar::e_ulong:
 case Scalar::e_ulonglong:
 case Scalar::e_uint128:
 case Scalar::e_uint256:
-m_integer = m_integer.lshr(bit_offset).trunc(bit_size).zext(8 * 
GetByteSize());
+m_integer = 
m_integer.lshr(bit_offset).zextOrTrunc(bit_size).zextOrSelf(8 * GetByteSize());
 return true;
 }
 return false;


Index: unittests/Core/ScalarTest.cpp
===
--- unittests/Core/ScalarTest.cpp
+++ unittests/Core/ScalarTest.cpp
@@ -79,3 +79,27 @@
 ASSERT_EQ((unsigned long long)a, a_scalar.ULongLong());
 }
 
+TEST(ScalarTest, ExtractBitfield)
+{
+uint32_t len = sizeof(long long) * 8;
+
+long long a1 = 0xf1f2f3f4f5f6f7f8LL;
+long long b1 = 0xff1f2f3f4f5f6f7fLL;
+Scalar s_scalar(a1);
+ASSERT_TRUE(s_scalar.ExtractBitfield(0, 0));
+ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+ASSERT_TRUE(s_scalar.ExtractBitfield(len, 0));
+ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+ASSERT_TRUE(s_scalar.ExtractBitfield(len - 4, 4));
+ASSERT_EQ(0, memcmp(&b1, s_scalar.GetBytes(), sizeof(b1)));
+
+unsigned long long a2 = 0xf1f2f3f4f5f6f7f8ULL;
+unsigned long long b2 = 0x0f1f2f3f4f5f6f7fULL;
+Scalar u_scalar(a2);
+ASSERT_TRUE(u_scalar.ExtractBitfield(0, 0));
+ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2)));
+ASSERT_TRUE(u_scalar.ExtractBitfield(len, 0));
+ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2)));
+ASSERT_TRUE(u_scalar.ExtractBitfield(len - 4, 4));
+ASSERT_EQ(0, memcmp(&b2, u_scalar.GetBytes(), sizeof(b2)));
+}
Index: source/Core/Scalar.cpp
===
--- source/Core/Scalar.cpp
+++ source/Core/Scalar.cpp
@@ -2788,15 +2788,15 @@
 case Scalar::e_slonglong:
 case Scalar::e_sint128:
 case Scalar::e_sint256:
-m_integer = m_integer.ashr(bit_offset).trunc(bit_size).sext(8 * GetByteSize());
+m_integer = m_integer.ashr(bit_offset).sextOrTrunc(bit_size).sextOrSelf(8 * GetByteSize());
 return true;
 
 case Scalar::e_uint:
 case Scalar::e_ulong:
 case Scalar::e_ulonglong:
 case Scalar::e_uint128:
 case Scalar::e_uint256:
-m_integer = m_integer.lshr(bit_offset).trunc(bit_size).zext(8 * GetByteSize());
+m_integer = m_integer.lshr(bit_offset).zextOrTrunc(bit_size).zextOrSelf(8 * GetByteSize());
 return true;
 }
 return false;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D20355: Avoid an assertion failure when a bit field is extracted from a value of the same size.

[Bryan Chan via lldb-commits]

bryanpkc added a comment.

In http://reviews.llvm.org/D20355#434117, @labath wrote:

> Do you have commit access?


Yes I do. Thanks for your review!



Comment at: unittests/Core/ScalarTest.cpp:90
@@ +89,3 @@
+ASSERT_TRUE(s_scalar.ExtractBitfield(0, 0));
+ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+ASSERT_TRUE(s_scalar.ExtractBitfield(len, 0));

labath wrote:
> Is there a reason this couldn't be written as `ASSERT_EQ(a1, 
> s_scalar.SLongLong())` ?
> If there isn't one, I think this would make the check more readable.
SLongLong() invokes sextOrTrunc() and getSExtValue(), potentially further 
changing the contents of the underlying m_integer. I felt that checking with 
memcmp() immediately after ExtractBitfield() is a more fool-proof way to 
confirm the behaviour of ExtractBitfield(). There are precedents for using 
memcmp() in the file, and it is not that much more unreadable.


http://reviews.llvm.org/D20355



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r270062 - Avoid an assertion failure when a bit field is extracted from a value of the same size.

[Bryan Chan via lldb-commits]

Author: bryanpkc
Date: Thu May 19 08:51:20 2016
New Revision: 270062

URL: http://llvm.org/viewvc/llvm-project?rev=270062&view=rev
Log:
Avoid an assertion failure when a bit field is extracted from a value of the 
same size.

Summary: One of the cases handled by ValueObjectChild::UpdateValue() uses the 
entire width of the parent's scalar value as the size of the child, and 
extracts the child by calling Scalar::ExtractBitfield(). This seems valid but 
APInt::trunc(), APInt::sext() and APInt::zext() assert that the bit field must 
not have the same size as the parent scalar. Replacing those calls with 
sextOrTrunc(), zextOrTrunc(), sextOrSelf() and zextOrSelf() fixes the assertion 
failures.

Reviewers: uweigand, labath

Subscribers: labath, lldb-commits

Differential Revision: http://reviews.llvm.org/D20355

Modified:
lldb/trunk/source/Core/Scalar.cpp
lldb/trunk/unittests/Core/ScalarTest.cpp

Modified: lldb/trunk/source/Core/Scalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Scalar.cpp?rev=270062&r1=270061&r2=270062&view=diff
==
--- lldb/trunk/source/Core/Scalar.cpp (original)
+++ lldb/trunk/source/Core/Scalar.cpp Thu May 19 08:51:20 2016
@@ -2788,7 +2788,7 @@ Scalar::ExtractBitfield (uint32_t bit_si
 case Scalar::e_slonglong:
 case Scalar::e_sint128:
 case Scalar::e_sint256:
-m_integer = m_integer.ashr(bit_offset).trunc(bit_size).sext(8 * 
GetByteSize());
+m_integer = 
m_integer.ashr(bit_offset).sextOrTrunc(bit_size).sextOrSelf(8 * GetByteSize());
 return true;
 
 case Scalar::e_uint:
@@ -2796,7 +2796,7 @@ Scalar::ExtractBitfield (uint32_t bit_si
 case Scalar::e_ulonglong:
 case Scalar::e_uint128:
 case Scalar::e_uint256:
-m_integer = m_integer.lshr(bit_offset).trunc(bit_size).zext(8 * 
GetByteSize());
+m_integer = 
m_integer.lshr(bit_offset).zextOrTrunc(bit_size).zextOrSelf(8 * GetByteSize());
 return true;
 }
 return false;

Modified: lldb/trunk/unittests/Core/ScalarTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Core/ScalarTest.cpp?rev=270062&r1=270061&r2=270062&view=diff
==
--- lldb/trunk/unittests/Core/ScalarTest.cpp (original)
+++ lldb/trunk/unittests/Core/ScalarTest.cpp Thu May 19 08:51:20 2016
@@ -79,3 +79,27 @@ TEST(ScalarTest, CastOperations)
 ASSERT_EQ((unsigned long long)a, a_scalar.ULongLong());
 }
 
+TEST(ScalarTest, ExtractBitfield)
+{
+uint32_t len = sizeof(long long) * 8;
+
+long long a1 = 0xf1f2f3f4f5f6f7f8LL;
+long long b1 = 0xff1f2f3f4f5f6f7fLL;
+Scalar s_scalar(a1);
+ASSERT_TRUE(s_scalar.ExtractBitfield(0, 0));
+ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+ASSERT_TRUE(s_scalar.ExtractBitfield(len, 0));
+ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+ASSERT_TRUE(s_scalar.ExtractBitfield(len - 4, 4));
+ASSERT_EQ(0, memcmp(&b1, s_scalar.GetBytes(), sizeof(b1)));
+
+unsigned long long a2 = 0xf1f2f3f4f5f6f7f8ULL;
+unsigned long long b2 = 0x0f1f2f3f4f5f6f7fULL;
+Scalar u_scalar(a2);
+ASSERT_TRUE(u_scalar.ExtractBitfield(0, 0));
+ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2)));
+ASSERT_TRUE(u_scalar.ExtractBitfield(len, 0));
+ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2)));
+ASSERT_TRUE(u_scalar.ExtractBitfield(len - 4, 4));
+ASSERT_EQ(0, memcmp(&b2, u_scalar.GetBytes(), sizeof(b2)));
+}


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits