[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

[Pavel Labath via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG82982304d709: [lldb/DWARF] More DW_AT_const_value fixes 
(authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D86348?vs=287023=287916#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value-bitfields.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -1,409 +1,158 @@
+# Test handling of (optimized-out/location-less) variables whose value is
+# specified by DW_AT_const_value
+
 # REQUIRES: x86
 
-# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
-# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
+# RUN: %lldb %t \
+# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4" \
+# RUN:   -o exit | FileCheck %s
+
+# CHECK-LABEL: target variable udata data1 data2 data4 data8 string strp ref4
+## Variable specified via DW_FORM_udata. This is typical for clang (10).
+# CHECK: (unsigned long) udata = 4742474247424742
+## Variables specified via fixed-size forms. This is typical for gcc (9).
+# CHECK: (unsigned long) data1 = 47
+# CHECK: (unsigned long) data2 = 4742
+# CHECK: (unsigned long) data4 = 47424742
+# CHECK: (unsigned long) data8 = 4742474247424742
+## Variables specified using string forms. This behavior purely speculative -- I
+## don't know of any compiler that would represent character strings this way.
+# CHECK: (char [7]) string = "string"
+# CHECK: (char [7]) strp = "strp"
+## Bogus attribute form. Let's make sure we don't crash at least.
+# CHECK: (char [7]) ref4 = 
+
+.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   8   # DW_FORM_string
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   4   # Abbreviation Code
+.byte   1   # DW_TAG_array_type
+.byte   1   # DW_CHILDREN_yes
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   5   # Abbreviation Code
+.byte   33  # DW_TAG_subrange_type
+.byte   0   # DW_CHILDREN_no
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   55  # DW_AT_count
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   6   # Abbreviation Code
+.byte   36  # DW_TAG_base_type
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   11  # DW_AT_byte_size
+.byte   11  # DW_FORM_data1
+.byte   62  # DW_AT_encoding
+.byte   11  # DW_FORM_data1
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.macro var code, form
+.byte   \code   # Abbreviation Code
+.byte   52  # DW_TAG_variable
+.byte   0   # DW_CHILDREN_no
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   73  # DW_AT_type
+.byte   19  # DW_FORM_ref4
+.byte   28  # DW_AT_const_value
+.byte   \form
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.endm
+var 10, 0xf # DW_FORM_udata
+var 11, 0xb # DW_FORM_data1
+var 12, 0x5 # 

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Sounds good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D86348#2231094 , @aprantl wrote:

> This looks nice! I'm somewhat suspicious that the new test doesn't 
> specifically test the union case of the old test, but I'm assuming that would 
> still work and your simpler tests covers the same code?

For a while I did want to just delete that union+bitfield test, but I 
eventually concluded that it is interesting to keep it. This patch keeps the 
original tests, only it renames it to a less generic name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

LGTM with that caveat


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

This looks nice! I'm somewhat suspicious that the new test doesn't specifically 
test the union case of the old test, but I'm assuming that would still work and 
your simpler tests covers the same code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Thank you finding these additional fixed, it looks good but I don't know some 
of the details as well as Adrian so I would prefer he gives the LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

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

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 287023.
labath added a comment.

Actually, keep the old const_value test, but give it a bitfield-specific name. 
Testing bitfields+const_values is still interesting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86348

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value-bitfields.s
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -1,408 +1,158 @@
-# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
-# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+# Test handling of (optimized-out/location-less) variables whose value is
+# specified by DW_AT_const_value
 
-# CHECK: (lldb) target variable constant
-# CHECK: (U) constant = {
-# CHECK:   raw = 1688469761
-# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
-# CHECK: }
+# REQUIRES: x86
 
-# This is testing when how ValueObjectVariable handles the case where the
-# DWARFExpression holds the data that represents a constant value.
-# Compile at -O1 allows us to capture this case. Below is the code used
-# to generate the assembly:
-#
-# typedef union
-# {
-#   unsigned raw;
-#   struct
-#   {
-# unsigned a : 8;
-# unsigned b : 8;
-# unsigned c : 6;
-# unsigned d : 2;
-# unsigned e : 6;
-# unsigned f : 2;
-#   } ;
-# } U;
-#
-# static U __attribute__((used)) _type_anchor;
-# static const int constant = 0x64A40101;
-#
-# int g() { return constant; }
-#
-# int main() {
-#   U u;
-#   u.raw = 0x64A40101;
-# }
-#
-# Compiled as follows:
-#
-#   clang -gdwarf-4 -O1 dw_at_const_value_bug.c -S -o dw_at_const_value_bug.s
-#
-# I was able to obtain a global of type U with DW_AT_const_value but was able
-# to using int. This required modifying the DW_AT_type of constant to be type
-# U. After that stripping as much of the assembly as possible to give us a
-# smaller reproducer.
+# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
+# RUN: %lldb %t \
+# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4" \
+# RUN:   -o exit | FileCheck %s
 
+# CHECK-LABEL: target variable udata data1 data2 data4 data8 string strp ref4
+## Variable specified via DW_FORM_udata. This is typical for clang (10).
+# CHECK: (unsigned long) udata = 4742474247424742
+## Variables specified via fixed-size forms. This is typical for gcc (9).
+# CHECK: (unsigned long) data1 = 47
+# CHECK: (unsigned long) data2 = 4742
+# CHECK: (unsigned long) data4 = 47424742
+# CHECK: (unsigned long) data8 = 4742474247424742
+## Variables specified using string forms. This behavior purely speculative -- I
+## don't know of any compiler that would represent character strings this way.
+# CHECK: (char [7]) string = "string"
+# CHECK: (char [7]) strp = "strp"
+## Bogus attribute form. Let's make sure we don't crash at least.
+# CHECK: (char [7]) ref4 = 
 
-.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
-	.no_dead_strip	__type_anchor
-	.section	__DWARF,__debug_str,regular,debug
-Linfo_string:
-  .zero 90
-	.asciz	"constant"  ## string offset=90
-	.asciz	"int"   ## string offset=99
-	.asciz	"_type_anchor"  ## string offset=103
-	.asciz	"U" ## string offset=116
-	.asciz	"raw"   ## string offset=118
-	.asciz	"unsigned int"  ## string offset=122
-	.asciz	"a" ## string offset=135
-	.asciz	"b" ## string offset=137
-	.asciz	"c" ## string offset=139
-	.asciz	"d" ## string offset=141
-	.asciz	"e" ## string offset=143
-	.asciz	"f" ## string offset=145
-	.asciz	"g" ## string offset=147
-	.asciz	"main"  ## string offset=149
-	.asciz	"u" ## string offset=154
-	.section	__DWARF,__debug_abbrev,regular,debug
-Lsection_abbrev:
-	.byte	1   ## Abbreviation Code
-	.byte	17  ## DW_TAG_compile_unit
-	.byte	1   ## DW_CHILDREN_yes
-	.byte	37  ## DW_AT_producer
-	.byte	14  ## DW_FORM_strp
-	.byte	19  ## DW_AT_language
-	.byte	5   ## DW_FORM_data2
-	.byte	3   ## DW_AT_name
-	.byte	14  ## DW_FORM_strp
-	.byte	66  ## DW_AT_stmt_list
-	.byte	23  ## DW_FORM_sec_offset
-	.byte	27  ## DW_AT_comp_dir
-	.byte	14  ## DW_FORM_strp
-	.ascii	"\264B" ## DW_AT_GNU_pubnames
-	.byte	25  

[Lldb-commits] [PATCH] D86348: [lldb/DWARF] More DW_AT_const_value fixes

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: shafik, aprantl.
Herald added a project: LLDB.
labath requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: sstefan1, JDevlieghere.

This fixes several issues in handling of DW_AT_const_value attributes:

- the first is that the size of the data given by data forms does not need to 
match the size of the underlying variable. We already had the case to handle 
this for DW_FORM_(us)data -- this extends the handling to other data forms. The 
main reason this was not picked up is because clang uses leb forms in these 
cases while gcc prefers the fixed-size ones.
- The handling of DW_AT_strp form was completely broken -- we would end up 
using the pointer value as the result. I've reorganized this code so that it 
handles all string forms uniformly.
- In case of a completely bogus form we would crash due to strlen(nullptr).

Depends on D86311 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86348

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
===
--- lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s
@@ -1,408 +1,158 @@
-# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
-# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+# Test handling of (optimized-out/location-less) variables whose value is
+# specified by DW_AT_const_value
 
-# CHECK: (lldb) target variable constant
-# CHECK: (U) constant = {
-# CHECK:   raw = 1688469761
-# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
-# CHECK: }
+# REQUIRES: x86
 
-# This is testing when how ValueObjectVariable handles the case where the
-# DWARFExpression holds the data that represents a constant value.
-# Compile at -O1 allows us to capture this case. Below is the code used
-# to generate the assembly:
-#
-# typedef union
-# {
-#   unsigned raw;
-#   struct
-#   {
-# unsigned a : 8;
-# unsigned b : 8;
-# unsigned c : 6;
-# unsigned d : 2;
-# unsigned e : 6;
-# unsigned f : 2;
-#   } ;
-# } U;
-#
-# static U __attribute__((used)) _type_anchor;
-# static const int constant = 0x64A40101;
-#
-# int g() { return constant; }
-#
-# int main() {
-#   U u;
-#   u.raw = 0x64A40101;
-# }
-#
-# Compiled as follows:
-#
-#   clang -gdwarf-4 -O1 dw_at_const_value_bug.c -S -o dw_at_const_value_bug.s
-#
-# I was able to obtain a global of type U with DW_AT_const_value but was able
-# to using int. This required modifying the DW_AT_type of constant to be type
-# U. After that stripping as much of the assembly as possible to give us a
-# smaller reproducer.
+# RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux %s -o %t
+# RUN: %lldb %t \
+# RUN:   -o "target variable udata data1 data2 data4 data8 string strp ref4" \
+# RUN:   -o exit | FileCheck %s
 
+# CHECK-LABEL: target variable udata data1 data2 data4 data8 string strp ref4
+## Variable specified via DW_FORM_udata. This is typical for clang (10).
+# CHECK: (unsigned long) udata = 4742474247424742
+## Variables specified via fixed-size forms. This is typical for gcc (9).
+# CHECK: (unsigned long) data1 = 47
+# CHECK: (unsigned long) data2 = 4742
+# CHECK: (unsigned long) data4 = 47424742
+# CHECK: (unsigned long) data8 = 4742474247424742
+## Variables specified using string forms. This behavior purely speculative -- I
+## don't know of any compiler that would represent character strings this way.
+# CHECK: (char [7]) string = "string"
+# CHECK: (char [7]) strp = "strp"
+## Bogus attribute form. Let's make sure we don't crash at least.
+# CHECK: (char [7]) ref4 = 
 
-.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
-	.no_dead_strip	__type_anchor
-	.section	__DWARF,__debug_str,regular,debug
-Linfo_string:
-  .zero 90
-	.asciz	"constant"  ## string offset=90
-	.asciz	"int"   ## string offset=99
-	.asciz	"_type_anchor"  ## string offset=103
-	.asciz	"U" ## string offset=116
-	.asciz	"raw"   ## string offset=118
-	.asciz	"unsigned int"  ## string offset=122
-	.asciz	"a" ## string offset=135
-	.asciz	"b" ## string offset=137
-	.asciz	"c" ## string offset=139
-	.asciz	"d" ## string offset=141
-	.asciz	"e" ## string offset=143
-	.asciz	"f" ## string offset=145
-	.asciz	"g" ## string offset=147
-	.asciz	"main"  ## string offset=149
-	.asciz	"u" ## string offset=154
-	.section	__DWARF,__debug_abbrev,regular,debug
-Lsection_abbrev:
-	.byte	1   ## Abbreviation Code
-	.byte	17