[Lldb-commits] [PATCH] D146919: Fix Issue #61706

[LU Hongyi via Phabricator via lldb-commits]

jwnhy created this revision.
jwnhy added reviewers: clayborg, jingham.
Herald added a project: All.
jwnhy requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

As described in #61706 , the 
`ValueObjectChild::UpdateValue(...)` may underflow the unsigned 
`m_bitfield_bit_offset`.

This is due to that in `ValueObjectChild::UpdateValue(...)`, the moved 
`overhang_bytes` only considers the `bitfield_end - *type_bit_size`. However, 
under certain cases, the `m_bitfield_bit_offset` may not strictly aligned to 
`*type_bit_size`, e.g. 27 < 32.

This results 27 - 32 = -5 underflows the unsigned `uint8_t 
m_bitfield_bit_offset`.

This patch fixes this issue by setting the `overhang_bytes` to the smaller 
value between `bitfield_end - *type_bit_size` and `m_bitfield_bit_offset` to 
avoid underflow.

I am quite new to this community, any helps/guidance are much appreciated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146919

Files:
  lldb/source/Core/ValueObjectChild.cpp


Index: lldb/source/Core/ValueObjectChild.cpp
===
--- lldb/source/Core/ValueObjectChild.cpp
+++ lldb/source/Core/ValueObjectChild.cpp
@@ -170,6 +170,8 @@
   if (bitfield_end > *type_bit_size) {
 uint64_t overhang_bytes =
 (bitfield_end - *type_bit_size + 7) / 8;
+if (overhang_bytes > m_bitfield_bit_offset / 8)
+  overhang_bytes = m_bitfield_bit_offset / 8;
 m_byte_offset += overhang_bytes;
 m_bitfield_bit_offset -= overhang_bytes * 8;
   }


Index: lldb/source/Core/ValueObjectChild.cpp
===
--- lldb/source/Core/ValueObjectChild.cpp
+++ lldb/source/Core/ValueObjectChild.cpp
@@ -170,6 +170,8 @@
   if (bitfield_end > *type_bit_size) {
 uint64_t overhang_bytes =
 (bitfield_end - *type_bit_size + 7) / 8;
+if (overhang_bytes > m_bitfield_bit_offset / 8)
+  overhang_bytes = m_bitfield_bit_offset / 8;
 m_byte_offset += overhang_bytes;
 m_bitfield_bit_offset -= overhang_bytes * 8;
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D138197: [lldb] Fix bitfield incorrectly printing when field crosses a storage unit

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

Hi, guys, I have met this issue #61706 
.

The case I provided shows that even without DWARF v2, this problem still exists.

I wonder if such modification is really needed or not.

Also, I am not sure changing all these to `int` actually fixes the issue here.

In fact, in `UpdateValue`, the `m_byte_offset` also changed by the 
`overhang_bytes` in line .
For a bitfield not aligned to `*type_bit_size`, moving `overhang_bytes` will 
cause truncation in the bitfield.

  #pragma pack(1)
  struct  {
signed f0 : 27;
unsigned f5 : 30;
  } g_96 = {5070, 1795821};
  int main() {return 0;}

The structure in memory is represented as `0xdb37680013ce`, adding 
`m_byte_offset` with `overhang_bytes` results in `0xdb37`, with last five bits 
lost.

This patchset may fix the underflow problem of `m_bitfield_bit_offset`, but 
does not fix the truncation issue...

Not knowing there is a WIP patch, I have created another smaller patch, which 
just checks... (if it's not ok to hijack the thread, plz tell me...I am really 
newbie...)
https://reviews.llvm.org/D146919

I wonder if it's ok I take over this issue?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138197/new/

https://reviews.llvm.org/D138197

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

[Lldb-commits] [PATCH] D146919: Fix Issue #61706

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

It seems this is more complicated than I originally thought.
My fix is also flawed when it comes the following program.
Let `overhang_bytes = 0` will overflow the otherside by 1 bit and thus output a 
wrong value.

But changing everything to signed does not resolve this issue as the following 
line is a concern.

  m_value.GetScalar() += m_byte_offset; // ValueObjectChild.cpp:181



  #pragma pack(1)
  struct  {
signed long long f0 : 1;
unsigned f5 : 32;
  } g_96 = {1, 0x};
  int main() {return 0;}


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146919/new/

https://reviews.llvm.org/D146919

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy created this revision.
jwnhy added reviewers: Michael137, DavidSpickett.
Herald added a project: All.
jwnhy requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch resolves an issue where a value 
is incorrectly displayed if it is represented 
by DW_OP_div.

This issue is caused by lldb evaluating 
operands of DW_OP_div as unsigned
and performed unintended unsigned 
division.

This issue is resolved by creating two
temporary signed scalar and performing
signed division.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1436,8 +1436,12 @@
   return false;
 } else {
   stack.pop_back();
-  stack.back() =
-  stack.back().ResolveValue(exe_ctx) / tmp.ResolveValue(exe_ctx);
+  Scalar divisor, dividend;
+  divisor = tmp.ResolveValue(exe_ctx);
+  dividend = stack.back().ResolveValue(exe_ctx);
+  divisor.MakeSigned();
+  dividend.MakeSigned();
+  stack.back() = dividend / divisor;
   if (!stack.back().ResolveValue(exe_ctx).IsValid()) {
 if (error_ptr)
   error_ptr->SetErrorString("Divide failed.");


Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1436,8 +1436,12 @@
   return false;
 } else {
   stack.pop_back();
-  stack.back() =
-  stack.back().ResolveValue(exe_ctx) / tmp.ResolveValue(exe_ctx);
+  Scalar divisor, dividend;
+  divisor = tmp.ResolveValue(exe_ctx);
+  dividend = stack.back().ResolveValue(exe_ctx);
+  divisor.MakeSigned();
+  dividend.MakeSigned();
+  stack.back() = dividend / divisor;
   if (!stack.back().ResolveValue(exe_ctx).IsValid()) {
 if (error_ptr)
   error_ptr->SetErrorString("Divide failed.");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy updated this revision to Diff 510285.
jwnhy added a comment.

Update the patch to include a small
regression test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/API/commands/expression/signed-dw-op-div/Makefile
  lldb/test/API/commands/expression/signed-dw-op-div/TestSignedDiv.py
  lldb/test/API/commands/expression/signed-dw-op-div/main.c


Index: lldb/test/API/commands/expression/signed-dw-op-div/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-div/main.c
@@ -0,0 +1,14 @@
+#include "stdint.h"
+static volatile uint64_t g = 0;
+
+static const int32_t f() {
+  uint32_t i;
+  for (i = 0; (i != 10); i++)
+++g; // break here
+  return 0;
+}
+
+int main() {
+  f();
+  return 0;
+}
Index: lldb/test/API/commands/expression/signed-dw-op-div/TestSignedDiv.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-div/TestSignedDiv.py
@@ -0,0 +1,14 @@
+"""Test that we can print values that represented via DW_OP_div"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+class SignedDivTestCase(TestBase):
+def test_signed_div(self):
+"""Test that we can print values that represented via DW_OP_div"""
+self.build()
+_, process, _, bkpt = lldbutil.run_to_source_breakpoint(self, '// 
break here', lldb.SBFileSpec('main.c'))
+self.expect_expr('i', result_type='uint32_t', result_value='0')
+for i in range(1, 10):
+lldbutil.continue_to_breakpoint(process, bkpt)
+self.expect_expr('i', result_type='uint32_t', result_value=str(i))
Index: lldb/test/API/commands/expression/signed-dw-op-div/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-div/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -g -O1
+
+include Makefile.rules
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1436,8 +1436,12 @@
   return false;
 } else {
   stack.pop_back();
-  stack.back() =
-  stack.back().ResolveValue(exe_ctx) / tmp.ResolveValue(exe_ctx);
+  Scalar divisor, dividend;
+  divisor = tmp.ResolveValue(exe_ctx);
+  dividend = stack.back().ResolveValue(exe_ctx);
+  divisor.MakeSigned();
+  dividend.MakeSigned();
+  stack.back() = dividend / divisor;
   if (!stack.back().ResolveValue(exe_ctx).IsValid()) {
 if (error_ptr)
   error_ptr->SetErrorString("Divide failed.");


Index: lldb/test/API/commands/expression/signed-dw-op-div/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-div/main.c
@@ -0,0 +1,14 @@
+#include "stdint.h"
+static volatile uint64_t g = 0;
+
+static const int32_t f() {
+  uint32_t i;
+  for (i = 0; (i != 10); i++)
+++g; // break here
+  return 0;
+}
+
+int main() {
+  f();
+  return 0;
+}
Index: lldb/test/API/commands/expression/signed-dw-op-div/TestSignedDiv.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-div/TestSignedDiv.py
@@ -0,0 +1,14 @@
+"""Test that we can print values that represented via DW_OP_div"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+class SignedDivTestCase(TestBase):
+def test_signed_div(self):
+"""Test that we can print values that represented via DW_OP_div"""
+self.build()
+_, process, _, bkpt = lldbutil.run_to_source_breakpoint(self, '// break here', lldb.SBFileSpec('main.c'))
+self.expect_expr('i', result_type='uint32_t', result_value='0')
+for i in range(1, 10):
+lldbutil.continue_to_breakpoint(process, bkpt)
+self.expect_expr('i', result_type='uint32_t', result_value=str(i))
Index: lldb/test/API/commands/expression/signed-dw-op-div/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-div/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -g -O1
+
+include Makefile.rules
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1436,8 +1436,12 @@
   return false;
 } else {
   stack.pop_back();
-  stack.back() =
-  stack.back().ResolveValue(exe_ctx) / tmp.ResolveValue

[Lldb-commits] [PATCH] D147425: [lldb] fix #61728 wrong parameter value display represented via DW_OP_deref and DW_OP_lt

[LU Hongyi via Phabricator via lldb-commits]

jwnhy created this revision.
jwnhy added a reviewer: JDevlieghere.
Herald added a project: All.
jwnhy requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch resolves an issue where a value is incorrectly displayed 
if it is represented by DW_OP_deref and DW_OP_lt combined.

This issue is caused by lldb treating DW_OP_deref generic types 
as unsigned by default, while DW_OP_lt should perform signed
comparison with these generic types by DWARF v5 standard.

This patch resolves this issue by making everything loaded by
DW_OP_deref as signed. As lldb does not really support generic
type, this is probably the best we can do.

A small regression test is also included in this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147425

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/API/commands/expression/signed-dw-op-deref/Makefile
  lldb/test/API/commands/expression/signed-dw-op-deref/TestSignedDeref.py
  lldb/test/API/commands/expression/signed-dw-op-deref/main.c


Index: lldb/test/API/commands/expression/signed-dw-op-deref/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-deref/main.c
@@ -0,0 +1,12 @@
+#include 
+int a;
+signed char b = -48;
+static int func_12(int p_13) {
+  for (;a;); // break here
+  return 0;
+}
+int main() {
+  func_12(0 > b);
+  printf(0);
+  return 0;
+}
Index: lldb/test/API/commands/expression/signed-dw-op-deref/TestSignedDeref.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-deref/TestSignedDeref.py
@@ -0,0 +1,11 @@
+"""Test that we can print values represented via DW_OP_deref and DW_OP_lt"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+class SignedDerefTestCase(TestBase):
+def test_signed_div(self):
+"""Test that we can print values represented via DW_OP_deref and 
DW_OP_lt"""
+self.build()
+_, process, _, bkpt = lldbutil.run_to_source_breakpoint(self, '// 
break here', lldb.SBFileSpec('main.c'))
+self.expect_expr('p_13', result_type='int', result_value='1')
Index: lldb/test/API/commands/expression/signed-dw-op-deref/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-deref/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -g -O1
+override CC := gcc
+
+include Makefile.rules
Index: lldb/source/Expression/DWARFExpression.cpp
===
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -1030,6 +1030,7 @@
   if (ABISP abi_sp = process->GetABI())
 pointer_value = abi_sp->FixCodeAddress(pointer_value);
   stack.back().GetScalar() = pointer_value;
+  stack.back().GetScalar().MakeSigned();
   stack.back().ClearContext();
 } else {
   if (error_ptr)
@@ -1120,6 +1121,7 @@
   break;
 }
 stack.back().GetScalar() = ptr;
+stack.back().GetScalar().MakeSigned();
 stack.back().ClearContext();
   } break;
   case Value::ValueType::FileAddress: {
@@ -1147,6 +1149,7 @@
 
 stack.back().GetScalar() = DerefSizeExtractDataHelper(
 addr_bytes, size, objfile->GetByteOrder(), size);
+stack.back().GetScalar().MakeSigned();
 stack.back().ClearContext();
 break;
   } else {
@@ -1177,6 +1180,7 @@
   stack.back().GetScalar() =
   DerefSizeExtractDataHelper(addr_bytes, sizeof(addr_bytes),
  process->GetByteOrder(), size);
+  stack.back().GetScalar().MakeSigned();
   stack.back().ClearContext();
 } else {
   if (error_ptr)


Index: lldb/test/API/commands/expression/signed-dw-op-deref/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-deref/main.c
@@ -0,0 +1,12 @@
+#include 
+int a;
+signed char b = -48;
+static int func_12(int p_13) {
+  for (;a;); // break here
+  return 0;
+}
+int main() {
+  func_12(0 > b);
+  printf(0);
+  return 0;
+}
Index: lldb/test/API/commands/expression/signed-dw-op-deref/TestSignedDeref.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/signed-dw-op-deref/TestSignedDeref.py
@@ -0,0 +1,11 @@
+"""Test that we can print values represented via DW_OP_deref and DW_OP_lt"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+class SignedDerefTestCase(TestBase):
+def test_signed_div(self):
+"""Test that we can print values represented via DW_OP_deref and DW_OP_lt"""
+

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy created this revision.
jwnhy added reviewers: Michael137, DavidSpickett, JDevlieghere.
Herald added a project: All.
jwnhy requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch resolves an issue that lldb not be able to
match the correct range of certain address.

This issue caused by other compilers like gcc generates
"empty ranges" like [address, address) in the DIE. These
"empty ranges" cause lldb matches address with these
ranges unintentionally and causes unpredictable result.
(In #61942, a variable dispearred due to this issue)

This patch resolves this issue by discarding these "empty
ranges" during the parsing of debugging information.

Another approach in fixing this might be changing the
logic of "FindEntryThatContains" and let it try harder
if met "empty ranges".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147606

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1314,10 +1314,11 @@
 for (size_t i = 0; i < num_ranges; ++i) {
   const DWARFRangeList::Entry &range = ranges.GetEntryRef(i);
   const addr_t range_base = range.GetRangeBase();
-  if (range_base >= subprogram_low_pc)
-block->AddRange(Block::Range(range_base - subprogram_low_pc,
+  if (range_base >= subprogram_low_pc) {
+if (range.IsValid())
+  block->AddRange(Block::Range(range_base - subprogram_low_pc,
  range.GetByteSize()));
-  else {
+  } else {
 GetObjectFile()->GetModule()->ReportError(
 "{0:x8}: adding range [{1:x16}-{2:x16}) which has a base "
 "that is less than the function's low PC {3:x16}. Please file "
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1065,8 +1065,9 @@
 
   DWARFRangeList ranges;
   for (const llvm::DWARFAddressRange &llvm_range : *llvm_ranges) {
-ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
-llvm_range.HighPC - llvm_range.LowPC));
+if (llvm_range.HighPC - llvm_range.LowPC > 0)
+  ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
+  llvm_range.HighPC - 
llvm_range.LowPC));
   }
   return ranges;
 }


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1314,10 +1314,11 @@
 for (size_t i = 0; i < num_ranges; ++i) {
   const DWARFRangeList::Entry &range = ranges.GetEntryRef(i);
   const addr_t range_base = range.GetRangeBase();
-  if (range_base >= subprogram_low_pc)
-block->AddRange(Block::Range(range_base - subprogram_low_pc,
+  if (range_base >= subprogram_low_pc) {
+if (range.IsValid())
+  block->AddRange(Block::Range(range_base - subprogram_low_pc,
  range.GetByteSize()));
-  else {
+  } else {
 GetObjectFile()->GetModule()->ReportError(
 "{0:x8}: adding range [{1:x16}-{2:x16}) which has a base "
 "that is less than the function's low PC {3:x16}. Please file "
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -1065,8 +1065,9 @@
 
   DWARFRangeList ranges;
   for (const llvm::DWARFAddressRange &llvm_range : *llvm_ranges) {
-ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
-llvm_range.HighPC - llvm_range.LowPC));
+if (llvm_range.HighPC - llvm_range.LowPC > 0)
+  ranges.Append(DWARFRangeList::Entry(llvm_range.LowPC,
+  llvm_range.HighPC - llvm_range.LowPC));
   }
   return ranges;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

In D147606#4245804 , @DavidSpickett 
wrote:

> I'm not familiar with this code, but the issue as explained I think I 
> understand.
>
> It seems like you're checking for empty ranges in two places, what does each 
> one do?
>
> Is there anything else these ranges indicate or do we think it is simply a 
> missed optimisation in the debug info producer? If they do sometimes mean 
> something, then perhaps improving the searching of the ranges is preferable 
> to discarding the empty ones.
> (though if all tests pass with this patch then these empty ranges can't be 
> that important to us can they)

Thanks for the comments, I didn't improve the search because that searching 
logic seems being used everywhere and not just restricted to filter through 
these "address ranges", I am a bit afraid of changing that also changes 
something subtle.
though it seems not making much sense to returning a "empty range" as searching 
results.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

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

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

In D147606#4246832 , @JDevlieghere 
wrote:

> The change looks fine and regardless of whether this makes sense or even 
> complies with the standard, we should be resilient against it. I would like 
> to see a test though.

Thanks a lot for the comment, I am new to lldb community, and got one thing a 
bit silly to ask.
Up to now, a few patches I submitted is kind of "depending on the 
compiler-generated" binary?
What am I supposed to do to **ensure the compiler generates these 
"easy-to-fault" binaries in the test?**

Like in this one, not every compiler will generate "empty ranges", and in the 
other one that is "DW_OP_div"...

I would like to add tests once I figure these out...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy updated this revision to Diff 512350.
jwnhy added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: jplehr, sstefan1.

Thanks a lot for the suggestion.
I added a assembly testcase for "DW_OP_div".
Also, since I don't have the permission to push,
if the patch is good to you, could you please 
help me merge it?

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
@@ -0,0 +1,448 @@
+  # Test handling of values represented via DW_OP_div
+
+  # RUN: %clang --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i" \
+  # RUN: -o "exit" | FileCheck %s
+
+  # Failing case was:
+  # (uint32_t) $0 = 0
+  # CHECK: (uint32_t) $0 = 2
+	.text
+	.file	"signed_dw_op_div.c"
+	.file	1 "/usr/local/include/bits" "types.h" md5 0x96c0983c9cdaf387938a8268d00aa594
+	.file	2 "/usr/local/include/bits" "stdint-uintn.h" md5 0xbedfab747425222bb150968c14e40abd
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 13 0  # signed_dw_op_div.c:13:0
+	.cfi_startproc
+# %bb.0:
+	movl	$3, %eax
+	#DEBUG_VALUE: f:i <- 0
+	.p2align	4, 0x90
+.LBB0_1:# =>This Inner Loop Header: Depth=1
+.Ltmp0:
+	#DEBUG_VALUE: f:i <- [DW_OP_consts 3, DW_OP_minus, DW_OP_consts 18446744073709551615, DW_OP_div, DW_OP_stack_value] $eax
+	.loc	0 8 7 prologue_end  # signed_dw_op_div.c:8:7
+	incq	g(%rip)
+.Ltmp1:
+	#DEBUG_VALUE: f:i <- [DW_OP_consts 3, DW_OP_minus, DW_OP_consts 18446744073709551615, DW_OP_div, DW_OP_consts 1, DW_OP_plus, DW_OP_stack_value] $eax
+	.loc	0 7 20  # signed_dw_op_div.c:7:20
+	decl	%eax
+.Ltmp2:
+	.loc	0 7 5 is_stmt 0 # signed_dw_op_div.c:7:5
+	jne	.LBB0_1
+.Ltmp3:
+# %bb.2:
+	.loc	0 15 5 is_stmt 1# signed_dw_op_div.c:15:5
+	xorl	%eax, %eax
+	retq
+.Ltmp4:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+	.file	3 "/usr/local/include/bits" "stdint-intn.h" md5 0x90039fb90b44dcbf118222513050fe57
+# -- End function
+	.type	g,@object   # @g
+	.local	g
+	.comm	g,8,8
+	.section	.debug_loclists,"",@progbits
+	.long	.Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
+.Ldebug_list_header_start0:
+	.short	5   # Version
+	.byte	8   # Address size
+	.byte	0   # Segment selector size
+	.long	1   # Offset entry count
+.Lloclists_table_base0:
+	.long	.Ldebug_loc0-.Lloclists_table_base0
+.Ldebug_loc0:
+	.byte	4   # DW_LLE_offset_pair
+	.uleb128 .Ltmp0-.Lfunc_begin0   #   starting offset
+	.uleb128 .Ltmp1-.Lfunc_begin0   #   ending offset
+	.byte	16  # Loc expr size
+	.byte	112 # DW_OP_breg0
+	.byte	0   # 0
+	.byte	16  # DW_OP_constu
+	.byte	255 # 4294967295
+	.byte	255 # 
+	.byte	255 # 
+	.byte	255 # 
+	.byte	15  # 
+	.byte	26  # DW_OP_and
+	.byte	17  # DW_OP_consts
+	.byte	3   # 3
+	.byte	28  # DW_OP_minus
+	.byte	17  # DW_OP_consts
+	.byte	127 # -1
+	.byte	27  # DW_OP_div
+	.byte	159 # DW_OP_stack_value
+	.byte	4   # DW_LLE_offset_pair
+	.uleb128 .Ltmp1-.Lfunc_begin0   #   starting offset
+	.uleb128 .Ltmp2-.Lfunc_begin0   #   ending offset
+	.byte	19  # Loc expr size
+	.byte	112 # DW_OP_breg0
+	.byte	0   # 0
+	.byte	16  # DW_OP_constu
+	.byte	255 # 4294967295
+	.byte	255 # 
+	.byte	255 # 
+	.byte	255 # 
+	.byte	15  # 
+	.byte	26  # DW_OP_and
+	.byte	17  # DW_OP_consts
+	.byte	3   # 3
+	.byte	28  # DW_OP_minus
+	.byte	17  # DW_O

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy updated this revision to Diff 512362.
jwnhy added a comment.

Updated the patch to remove unncessary checks and added a testcase.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/debug_empty_ranges.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/debug_empty_ranges.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/debug_empty_ranges.s
@@ -0,0 +1,481 @@
+# Test handling of empty DWARF ranges generated by GCC
+
+# RUN: %clang --target=x86_64-pc-linux -o %t %s 
+# RUN: %lldb %t -o "b func" -o "r" -o "expression -T -- i" \
+# RUN: -o "exit" | FileCheck %s
+
+# Failing case was:
+# error: expression failed to parse:
+# error: :1:1: use of undeclared identifier 'i'
+# CHECK: (int) $0 = 10
+	.file	"r.c"
+	.file 1 "r.c"
+	.text
+.Ltext0:
+	.file 0 "" "r.c"
+	.globl	main
+	.type	main, @function
+main:
+.LFB1:
+	.file 1 "r.c"
+	.loc 1 12 12
+	.cfi_startproc
+	.loc 1 12 14
+	movl	b(%rip), %eax
+.LBB5:
+.LBB6:
+	.loc 1 4 3
+.LVL0:
+	.loc 1 5 3
+	.loc 1 5 12
+.LBE6:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB7:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE7:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB8:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE8:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB9:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE9:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB10:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE10:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB11:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE11:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB12:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE12:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB13:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE13:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB14:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE14:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB15:
+	.loc 1 5 19
+	.loc 1 5 12
+.LBE15:
+	.loc 1 6 5
+	.loc 1 7 5
+.LBB16:
+	.loc 1 5 19
+	.loc 1 5 12
+	.loc 1 9 3
+	.loc 1 9 5 is_stmt 0
+	movl	$0, a(%rip)
+	.loc 1 10 3 is_stmt 1
+.LVL1:
+.LBE16:
+.LBE5:
+	.loc 1 12 26 is_stmt 0
+	movl	$0, %eax
+	ret
+	.cfi_endproc
+.LFE1:
+	.size	main, .-main
+	.globl	b
+	.bss
+	.align 4
+	.type	b, @object
+	.size	b, 4
+b:
+	.zero	4
+	.globl	a
+	.align 4
+	.type	a, @object
+	.size	a, 4
+a:
+	.zero	4
+	.text
+.Letext0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	0xd9
+	.value	0x5
+	.byte	0x1
+	.byte	0x8
+	.long	.Ldebug_abbrev0
+	.uleb128 0x3
+	.long	.LASF2
+	.byte	0x1d
+	.long	.LASF0
+	.long	.LASF1
+	.quad	.Ltext0
+	.quad	.Letext0-.Ltext0
+	.long	.Ldebug_line0
+	.uleb128 0x1
+	.string	"a"
+	.byte	0x1
+	.byte	0x5
+	.long	0x41
+	.uleb128 0x9
+	.byte	0x3
+	.quad	a
+	.uleb128 0x4
+	.byte	0x4
+	.byte	0x5
+	.string	"int"
+	.uleb128 0x5
+	.long	0x41
+	.uleb128 0x1
+	.string	"b"
+	.byte	0x2
+	.byte	0xe
+	.long	0x48
+	.uleb128 0x9
+	.byte	0x3
+	.quad	b
+	.uleb128 0x6
+	.long	.LASF3
+	.byte	0x1
+	.byte	0xc
+	.byte	0x5
+	.long	0x41
+	.quad	.LFB1
+	.quad	.LFE1-.LFB1
+	.uleb128 0x1
+	.byte	0x9c
+	.long	0xb0
+	.uleb128 0x7
+	.long	0xb0
+	.quad	.LBB5
+	.quad	.LBE5-.LBB5
+	.byte	0x1
+	.byte	0xc
+	.byte	0xe
+	.uleb128 0x8
+	.long	0xbd
+	.uleb128 0x9
+	.long	.LLRL0
+	.uleb128 0xa
+	.long	0xc7
+	.long	.LLST1
+	.byte	0
+	.byte	0
+	.byte	0
+	.uleb128 0xb
+	.long	.LASF4
+	.byte	0x1
+	.byte	0x3
+	.byte	0xc
+	.long	0x41
+	.byte	0x1
+	.uleb128 0xc
+	.string	"b"
+	.byte	0x1
+	.byte	0x3
+	.byte	0x18
+	.long	0x41
+	.uleb128 0x2
+	.string	"i"
+	.byte	0x4
+	.byte	0x7
+	.long	0x41
+	.uleb128 0xd
+	.uleb128 0x2
+	.string	"c"
+	.byte	0x6
+	.byte	0x9
+	.long	0x41
+	.byte	0
+	.byte	0
+	.byte	0
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1
+	.uleb128 0x34
+	.byte	0
+	.uleb128 0x3
+	.uleb128 0x8
+	.uleb128 0x3a
+	.uleb128 0x21
+	.sleb128 1
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x3f
+	.uleb128 0x19
+	.uleb128 0x2
+	.uleb128 0x18
+	.byte	0
+	.byte	0
+	.uleb128 0x2
+	.uleb128 0x34
+	.byte	0
+	.uleb128 0x3
+	.uleb128 0x8
+	.uleb128 0x3a
+	.uleb128 0x21
+	.sleb128 1
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.byte	0
+	.byte	0
+	.uleb128 0x3
+	.uleb128 0x11
+	.byte	0x1
+	.uleb128 0x25
+	.uleb128 0xe
+	.uleb128 0x13
+	.uleb128 0xb
+	.uleb128 0x3
+	.uleb128 0x1f
+	.uleb128 0x1b
+	.uleb128 0x1f
+	.uleb128 0x11
+	.uleb128 0x1
+	.uleb128 0x12
+	.uleb128 0x7
+	.uleb128 0x10
+	.uleb128 0x17
+	.byte	0
+	.byte	0
+	.uleb128 0x4
+	.uleb128 0x24
+	.byte	0
+	.uleb128 0xb
+	.uleb128 0xb
+	.uleb128 0x3e
+	.uleb128 0xb
+	.uleb128 0x3
+	.uleb128 0x8
+	.byte	0
+	.byte	0
+	.uleb128 0x5
+	.uleb128 0x35
+	.byte	0
+	.uleb128 0x49
+	.uleb128 0x13
+	.byte	0
+	.byte	0
+	.uleb128 0x6
+	.uleb128 0x2e
+	.byte	0x1
+	.uleb128 0x3f
+	.uleb128 0x19
+	.uleb128 0x3
+	.uleb128 0xe
+	.uleb128 0x3a
+	.uleb128 0xb
+	.uleb128 0x3b
+	.uleb128 0xb
+	.uleb128 0x39
+	.uleb128 0xb
+	.uleb128 0x49
+	.uleb128 0x13
+	.uleb128 0x11
+	.uleb128 0x1
+	.uleb128 0x12
+	.uleb128 0x7
+	.uleb128 0x40
+	.uleb128 0x18
+	.uleb128 0x7a
+	.uleb128

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

In D147606#4246832 , @JDevlieghere 
wrote:

> The change looks fine and regardless of whether this makes sense or even 
> complies with the standard, we should be resilient against it. I would like 
> to see a test though.

It seems that DWARFv5 indeed specifies how to deal with these empty ranges

> A bounded range entry whose beginning and ending address offsets are equal
> (including zero) indicates an empty range and may be ignored.

Also, I kind of searched through the codebase, and noticed that there are 
multiple places like this, and they adapt a similar approach discarding those 
empty ranges.
e.g. in `DWARFDebugRanges.cpp`

// Filter out empty ranges
if (begin < end)
  range_list.Append(DWARFRangeList::Entry(begin + base_addr, end - begin));
  }


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

Ping!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147606: [lldb] fix #61942, discard "empty" ranges in DWARF to better handle gcc binary

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

Ping!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147606/new/

https://reviews.llvm.org/D147606

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy updated this revision to Diff 517784.
jwnhy marked an inline comment as done.
jwnhy added a comment.

Updated the patch to include the original code.

Could you please help me push it to the main codebase?, as I don't have the 
push right to the repo.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
@@ -0,0 +1,468 @@
+  # Test handling of values represented via DW_OP_div
+
+  # RUN: %clang --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i" \
+  # RUN: -o "exit" | FileCheck %s
+
+  # Failing case was:
+  # (uint32_t) $0 = 0
+  # CHECK: (uint32_t) $0 = 2
+	
+  # This case is generated from the following code:
+  # #include "stdint.h"
+  # static volatile uint64_t g = 0;
+  #  static const int32_t f()
+  #  {
+  #uint32_t i;
+  #for (i = 0; (i != 10); i++)
+  #  ++g;
+  #return 0;
+  #  
+  #  }
+  #
+  #  int main()
+  #  {
+  #f();
+  #return 0;
+  #
+  #  }
+
+  .text
+	.file	"signed_dw_op_div.c"
+	.file	1 "/usr/local/include/bits" "types.h" md5 0x96c0983c9cdaf387938a8268d00aa594
+	.file	2 "/usr/local/include/bits" "stdint-uintn.h" md5 0xbedfab747425222bb150968c14e40abd
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 13 0  # signed_dw_op_div.c:13:0
+	.cfi_startproc
+# %bb.0:
+	movl	$3, %eax
+	#DEBUG_VALUE: f:i <- 0
+	.p2align	4, 0x90
+.LBB0_1:# =>This Inner Loop Header: Depth=1
+.Ltmp0:
+	#DEBUG_VALUE: f:i <- [DW_OP_consts 3, DW_OP_minus, DW_OP_consts 18446744073709551615, DW_OP_div, DW_OP_stack_value] $eax
+	.loc	0 8 7 prologue_end  # signed_dw_op_div.c:8:7
+	incq	g(%rip)
+.Ltmp1:
+	#DEBUG_VALUE: f:i <- [DW_OP_consts 3, DW_OP_minus, DW_OP_consts 18446744073709551615, DW_OP_div, DW_OP_consts 1, DW_OP_plus, DW_OP_stack_value] $eax
+	.loc	0 7 20  # signed_dw_op_div.c:7:20
+	decl	%eax
+.Ltmp2:
+	.loc	0 7 5 is_stmt 0 # signed_dw_op_div.c:7:5
+	jne	.LBB0_1
+.Ltmp3:
+# %bb.2:
+	.loc	0 15 5 is_stmt 1# signed_dw_op_div.c:15:5
+	xorl	%eax, %eax
+	retq
+.Ltmp4:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+	.file	3 "/usr/local/include/bits" "stdint-intn.h" md5 0x90039fb90b44dcbf118222513050fe57
+# -- End function
+	.type	g,@object   # @g
+	.local	g
+	.comm	g,8,8
+	.section	.debug_loclists,"",@progbits
+	.long	.Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
+.Ldebug_list_header_start0:
+	.short	5   # Version
+	.byte	8   # Address size
+	.byte	0   # Segment selector size
+	.long	1   # Offset entry count
+.Lloclists_table_base0:
+	.long	.Ldebug_loc0-.Lloclists_table_base0
+.Ldebug_loc0:
+	.byte	4   # DW_LLE_offset_pair
+	.uleb128 .Ltmp0-.Lfunc_begin0   #   starting offset
+	.uleb128 .Ltmp1-.Lfunc_begin0   #   ending offset
+	.byte	16  # Loc expr size
+	.byte	112 # DW_OP_breg0
+	.byte	0   # 0
+	.byte	16  # DW_OP_constu
+	.byte	255 # 4294967295
+	.byte	255 # 
+	.byte	255 # 
+	.byte	255 # 
+	.byte	15  # 
+	.byte	26  # DW_OP_and
+	.byte	17  # DW_OP_consts
+	.byte	3   # 3
+	.byte	28  # DW_OP_minus
+	.byte	17  # DW_OP_consts
+	.byte	127 # -1
+	.byte	27  # DW_OP_div
+	.byte	159 # DW_OP_stack_value
+	.byte	4   # DW_LLE_offset_pair
+	.uleb128 .Ltmp1-.Lfunc_begin0   #   starting offset
+	.uleb128 .Ltmp2-.Lfunc_begin0   #   ending offset
+	.byte	19  # Loc expr size
+	.byte	112 # DW_OP_breg0
+	.byte	0   # 0
+	.byte	16  # DW_OP_constu
+	.byte	255 # 4294967295
+	.byte	255 # 
+	.byte	255 # 
+	.byte	255 # 
+	.by

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy marked an inline comment as done.
jwnhy added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s:3
+
+  # RUN: %clang --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "b f" -o "r" -o "c" -o "c" -o "expression -T -- i" \

Michael137 wrote:
> other tests use `llvm-mc` as the assembler
> E.g., `llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s`
> 
> I think it'll marginally speed things up
There are some issues on llvm-mc, it complains about .loc directives.
So I guess I will just go with Clang...



Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s:6
+  # RUN: -o "exit" | FileCheck %s
+
+  # Failing case was:

Michael137 wrote:
> Can you paste the source code here which you used to derive this test case? 
> For future readers
Update the patch to include the original sources.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy marked an inline comment as done.
jwnhy added a comment.

In D147370#4309481 , @Michael137 
wrote:

> @jwnhy Let me know which name and email I should attribute the patch to

Oh, sorry.
I am LU Hongyi, and the email is jwn...@gmail.com, my github account is also 
jwnhy.

Thank you very much.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy updated this revision to Diff 518975.
jwnhy added a comment.

Update the testcase to resolve cross-platform issue.
This new testcase is hand-crafted with DW_OP_div.
Therefore, it no longer require to be linked and executed.
Just directly call lldb on this object file and use "p g" to 
see if the value represented by DW_OP_div is correct or not.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
@@ -0,0 +1,230 @@
+  # Test handling of values represented via DW_OP_div
+  # RUN: %clang -c --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "expression -T -- g" -o "exit" | FileCheck %s
+
+  # Failing case was:
+  # (int) $0 = 0
+  # CHECK: (int) $0 = 2
+  #
+  # This file is crafted by hand on the basis of a very simple C program.
+
+  # The original (NOT THIS FILE) error-triggering code is:
+  # #include "stdint.h"
+  # static volatile uint64_t g = 0;
+  #  static const int32_t f()
+  #  {
+  #uint32_t i;
+  #for (i = 0; (i != 10); i++)
+  #  ++g;
+  #return 0;
+  #  
+  #  }
+  #
+  #  int main()
+  #  {
+  #f();
+  #return 0;
+  #
+  #  }
+	# When the above code is compiled by Clang 15.0.7 with -O1, it generates desired DW_OP_div for testing.
+	# Due to cross-platform issue, this case is hand-crafted for testing.
+	.text
+	.file	"DW_OP_div-with-signed.c"
+	.globl	main# -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 4 0   # DW_OP_div-with-signed.c:4:0
+	.cfi_startproc
+# %bb.0:
+	.loc	0 5 5 prologue_end  # DW_OP_div-with-signed.c:5:5
+	xorl	%eax, %eax
+	retq
+.Ltmp0:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.type	g,@object   # @g
+	.data
+	.globl	g
+	.p2align	2, 0x0
+g:
+	.long	3735928559  # 0xdeadbeef
+	.size	g, 4
+
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	37  # DW_FORM_strx1
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	114 # DW_AT_str_offsets_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	37  # DW_FORM_strx1
+	.byte	17  # DW_AT_low_pc
+	.byte	27  # DW_FORM_addrx
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	115 # DW_AT_addr_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	52  # DW_TAG_variable
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	63  # DW_AT_external
+	.byte	25  # DW_FORM_flag_present
+	.byte	58  # DW_AT_decl_file
+	.byte	11  # DW_FORM_data1
+	.byte	59  # DW_AT_decl_line
+	.byte	11  # DW_FORM_data1
+	.byte	2   # DW_AT_location
+	.byte	24  # DW_FORM_exprloc
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	3   # Abbreviation Code
+	.byte	36  # DW_TAG_base_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	62

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

I have updated the testcase in the patch and remove the linker stage (using 
"-c").
But I don't have an Arm devices, can anyone help me test it?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy added a comment.

In D147370#4314617 , @jwnhy wrote:

> I have updated the testcase in the patch and remove the linker stage (using 
> "-c").
> But I don't have an Arm devices, can anyone help me test it?

Asked one of my friend to tested on M1  Arm, 
"clang -c" works.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

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

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy updated this revision to Diff 519733.
jwnhy added a comment.

Update the patch to pass the assertion.
Sorry I forget to enable these on my machine.

Given that said, this assertion seems pretty weird to me.
Since this assembly is also generated by clang (with handcrafted DWARF, 
this assertion means that any assembly generated from a single file will
be rejected?

Anyway, I have added a dummy ".file 1" directive to pass
this assertion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
@@ -0,0 +1,231 @@
+  # Test handling of values represented via DW_OP_div
+  # RUN: %clang -c --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "expression -T -- g" -o "exit" | FileCheck %s
+
+  # Failing case was:
+  # (int) $0 = 0
+  # CHECK: (int) $0 = 2
+  #
+  # This file is crafted by hand on the basis of a very simple C program.
+
+  # The original (NOT THIS FILE) error-triggering code is:
+  # #include "stdint.h"
+  # static volatile uint64_t g = 0;
+  #  static const int32_t f()
+  #  {
+  #uint32_t i;
+  #for (i = 0; (i != 10); i++)
+  #  ++g;
+  #return 0;
+  #  
+  #  }
+  #
+  #  int main()
+  #  {
+  #f();
+  #return 0;
+  #
+  #  }
+	# When the above code is compiled by Clang 15.0.7 with -O1, it generates desired DW_OP_div for testing.
+	# Due to cross-platform issue, this case is hand-crafted for testing.
+	.text
+	.file	"DW_OP_div-with-signed.c"
+  .file 1 "DUMMY.c" # to pass assertion
+	.globl	main  # -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 4 0   # DW_OP_div-with-signed.c:4:0
+	.cfi_startproc
+# %bb.0:
+	.loc	0 5 5 prologue_end  # DW_OP_div-with-signed.c:5:5
+	xorl	%eax, %eax
+	retq
+.Ltmp0:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.type	g,@object   # @g
+	.data
+	.globl	g
+	.p2align	2, 0x0
+g:
+	.long	3735928559  # 0xdeadbeef
+	.size	g, 4
+
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	37  # DW_FORM_strx1
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	114 # DW_AT_str_offsets_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	37  # DW_FORM_strx1
+	.byte	17  # DW_AT_low_pc
+	.byte	27  # DW_FORM_addrx
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	115 # DW_AT_addr_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	52  # DW_TAG_variable
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	63  # DW_AT_external
+	.byte	25  # DW_FORM_flag_present
+	.byte	58  # DW_AT_decl_file
+	.byte	11  # DW_FORM_data1
+	.byte	59  # DW_AT_decl_line
+	.byte	11  # DW_FORM_data1
+	.byte	2   # DW_AT_location
+	.byte	24  # DW_FORM_exprloc
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	3   # Abbreviation Code
+	.byte	36  # DW_TAG_base_type
+	.byte	0   # DW_CHI

[Lldb-commits] [PATCH] D147370: [lldb] fixing #61727 fixing incorrect variable displaying with DW_OP_div

[LU Hongyi via Phabricator via lldb-commits]

jwnhy updated this revision to Diff 519735.
jwnhy added a comment.

Update the patch for better naming.
The assertion is correct, I somehow delete 
a file directive from the assembly generated
by clang.

It should work now...Thanks a lot for checking
this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147370/new/

https://reviews.llvm.org/D147370

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/DW_OP_div-with-signed.s
@@ -0,0 +1,232 @@
+  # Test handling of values represented via DW_OP_div
+  # RUN: %clang -c --target=x86_64-pc-linux -o %t %s
+  # RUN: %lldb %t -o "expression -T -- g" -o "exit" | FileCheck %s
+
+  # Failing case was:
+  # (int) $0 = 0
+  # CHECK: (int) $0 = 2
+  #
+  # This file is crafted by hand on the basis of a very simple C program.
+
+  # The original (NOT THIS FILE) error-triggering code is:
+  # #include "stdint.h"
+  # static volatile uint64_t g = 0;
+  #  static const int32_t f()
+  #  {
+  #uint32_t i;
+  #for (i = 0; (i != 10); i++)
+  #  ++g;
+  #return 0;
+  #  
+  #  }
+  #
+  #  int main()
+  #  {
+  #f();
+  #return 0;
+  #
+  #  }
+	# When the above code is compiled by Clang 15.0.7 with -O1, it generates desired DW_OP_div for testing.
+	# Due to cross-platform issue, this case is hand-crafted for testing.
+
+	.text
+	.file "DW_OP_div-with-signed.c"
+	.file	0 "DW_OP_div-with-signed.c" # Dummy file
+	.globl	main  # -- Begin function main
+	.p2align	4, 0x90
+	.type	main,@function
+main:   # @main
+.Lfunc_begin0:
+	.loc	0 4 0   # DW_OP_div-with-signed.c:4:0
+	.cfi_startproc
+# %bb.0:
+	.loc	0 5 5 prologue_end  # DW_OP_div-with-signed.c:5:5
+	xorl	%eax, %eax
+	retq
+.Ltmp0:
+.Lfunc_end0:
+	.size	main, .Lfunc_end0-main
+	.cfi_endproc
+# -- End function
+	.type	g,@object   # @g
+	.data
+	.globl	g
+	.p2align	2, 0x0
+g:
+	.long	3735928559  # 0xdeadbeef
+	.size	g, 4
+
+	.section	.debug_abbrev,"",@progbits
+	.byte	1   # Abbreviation Code
+	.byte	17  # DW_TAG_compile_unit
+	.byte	1   # DW_CHILDREN_yes
+	.byte	37  # DW_AT_producer
+	.byte	37  # DW_FORM_strx1
+	.byte	19  # DW_AT_language
+	.byte	5   # DW_FORM_data2
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	114 # DW_AT_str_offsets_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	16  # DW_AT_stmt_list
+	.byte	23  # DW_FORM_sec_offset
+	.byte	27  # DW_AT_comp_dir
+	.byte	37  # DW_FORM_strx1
+	.byte	17  # DW_AT_low_pc
+	.byte	27  # DW_FORM_addrx
+	.byte	18  # DW_AT_high_pc
+	.byte	6   # DW_FORM_data4
+	.byte	115 # DW_AT_addr_base
+	.byte	23  # DW_FORM_sec_offset
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	2   # Abbreviation Code
+	.byte	52  # DW_TAG_variable
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	73  # DW_AT_type
+	.byte	19  # DW_FORM_ref4
+	.byte	63  # DW_AT_external
+	.byte	25  # DW_FORM_flag_present
+	.byte	58  # DW_AT_decl_file
+	.byte	11  # DW_FORM_data1
+	.byte	59  # DW_AT_decl_line
+	.byte	11  # DW_FORM_data1
+	.byte	2   # DW_AT_location
+	.byte	24  # DW_FORM_exprloc
+	.byte	0   # EOM(1)
+	.byte	0   # EOM(2)
+	.byte	3   # Abbreviation Code
+	.byte	36  # DW_TAG_base_type
+	.byte	0   # DW_CHILDREN_no
+	.byte	3   # DW_AT_name
+	.byte	37  # DW_FORM_strx1
+	.byte	62  # DW_AT_encoding
+	.byte	11