[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG25bbceb047a3: [LLDB] Fix how ValueObjectChild handles 
bit-fields stored in a Scalar in… (authored by shafik).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_data_bit_offset-DW_OP_stack_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_data_bit_offset-DW_OP_stack_value.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_data_bit_offset-DW_OP_stack_value.s
@@ -0,0 +1,312 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 1688469761
+# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
+# CHECK: }
+
+# We are testing how ValueObject deals with bit-fields when an argument is
+# passed by register. Compiling at -O1 allows us to capture this case and
+# test it.
+#
+# typedef union {
+#   unsigned raw;
+#   struct {
+#  unsigned a : 8;
+#  unsigned b : 8;
+#  unsigned c : 6;
+#  unsigned d : 2;
+#  unsigned e : 6;
+#  unsigned f : 2;
+#   };
+# } U;
+#
+# // This appears first in the debug info and pulls the type definition in...
+# static U __attribute__((used)) _type_anchor;
+# // ... then our useful variable appears last in the debug info and we can
+# // tweak the assembly without needing to edit a lot of offsets by hand.
+# static U ug;
+#
+# extern void f(U);
+#
+# // Omit debug info for main.
+# __attribute__((nodebug))
+# int main() {
+#   ug.raw = 0x64A40101;
+#   f(ug);
+#   f((U)ug.raw);
+# }
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 weird.c -S -o weird.s
+#
+# Then the DWARF was hand modified to get DW_AT_LOCATION for ug from:
+#
+#   DW_AT_location	(DW_OP_addr 0x3f8, DW_OP_deref, DW_OP_constu 0x64a40101, DW_OP_mul, DW_OP_lit0, DW_OP_plus, DW_OP_stack_value)
+#
+# to this:
+#
+#   DW_AT_location	(DW_OP_constu 0x64a40101, DW_OP_stack_value)
+#
+# to work-around a seperate bug.
+
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+	.zero 138
+	.asciz	"_type_anchor"  ## string offset=138
+	.asciz	"U" ## string offset=151
+	.asciz	"raw"   ## string offset=153
+	.asciz	"unsigned int"  ## string offset=157
+	.asciz	"a" ## string offset=170
+	.asciz	"b" ## string offset=172
+	.asciz	"c" ## string offset=174
+	.asciz	"d" ## string offset=176
+	.asciz	"e" ## string offset=178
+	.asciz	"f" ## string offset=180
+	.asciz	"ug"## string offset=182
+	.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
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.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	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.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)
+	.

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 285423.
shafik marked an inline comment as done.
shafik added a comment.

Update test name


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

https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/Shell/SymbolFile/DWARF/DW_AT_data_bit_offset-DW_OP_stack_value.s

Index: lldb/test/Shell/SymbolFile/DWARF/DW_AT_data_bit_offset-DW_OP_stack_value.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/DW_AT_data_bit_offset-DW_OP_stack_value.s
@@ -0,0 +1,312 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 1688469761
+# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
+# CHECK: }
+
+# We are testing how ValueObject deals with bit-fields when an argument is
+# passed by register. Compiling at -O1 allows us to capture this case and
+# test it.
+#
+# typedef union {
+#   unsigned raw;
+#   struct {
+#  unsigned a : 8;
+#  unsigned b : 8;
+#  unsigned c : 6;
+#  unsigned d : 2;
+#  unsigned e : 6;
+#  unsigned f : 2;
+#   };
+# } U;
+#
+# // This appears first in the debug info and pulls the type definition in...
+# static U __attribute__((used)) _type_anchor;
+# // ... then our useful variable appears last in the debug info and we can
+# // tweak the assembly without needing to edit a lot of offsets by hand.
+# static U ug;
+#
+# extern void f(U);
+#
+# // Omit debug info for main.
+# __attribute__((nodebug))
+# int main() {
+#   ug.raw = 0x64A40101;
+#   f(ug);
+#   f((U)ug.raw);
+# }
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 weird.c -S -o weird.s
+#
+# Then the DWARF was hand modified to get DW_AT_LOCATION for ug from:
+#
+#   DW_AT_location	(DW_OP_addr 0x3f8, DW_OP_deref, DW_OP_constu 0x64a40101, DW_OP_mul, DW_OP_lit0, DW_OP_plus, DW_OP_stack_value)
+#
+# to this:
+#
+#   DW_AT_location	(DW_OP_constu 0x64a40101, DW_OP_stack_value)
+#
+# to work-around a seperate bug.
+
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+	.zero 138
+	.asciz	"_type_anchor"  ## string offset=138
+	.asciz	"U" ## string offset=151
+	.asciz	"raw"   ## string offset=153
+	.asciz	"unsigned int"  ## string offset=157
+	.asciz	"a" ## string offset=170
+	.asciz	"b" ## string offset=172
+	.asciz	"c" ## string offset=174
+	.asciz	"d" ## string offset=176
+	.asciz	"e" ## string offset=178
+	.asciz	"f" ## string offset=180
+	.asciz	"ug"## string offset=182
+	.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
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.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	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.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	22  ## DW_TAG_typedef
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## 

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

This looks good to me. I appreciate the efforts taken to reduce the test size.

The trick for controlling the debug info order is neat, and I may end up using 
it some time. FWIW, the way I usually handle these things is by first replacing 
all constant debug info offsets with symbolic references (`.long 123` -> `.long 
.Lmytype-.debug_info`). After that, it's possible to freely manipulate any 
debug info entry. (At that point I usually delete all DW_AT_decl_file/lines and 
other boring attributes, which tends to reduce the file a lot).




Comment at: lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s:1
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s

I don't think the test file name matches what is being tested anymore. Maybe 
name it something like `DW_AT_data_bit_offset-DW_OP_stack_value` (in line with 
other tests in this folder)?


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 285233.
shafik marked 4 inline comments as done.
shafik added a comment.

Updated test using more compact code from Fred.


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

https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s

Index: lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s
@@ -0,0 +1,312 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 1688469761
+# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
+# CHECK: }
+
+# We are testing how ValueObject deals with bit-fields when an argument is
+# passed by register. Compiling at -O1 allows us to capture this case and
+# test it.
+#
+# typedef union {
+#   unsigned raw;
+#   struct {
+#  unsigned a : 8;
+#  unsigned b : 8;
+#  unsigned c : 6;
+#  unsigned d : 2;
+#  unsigned e : 6;
+#  unsigned f : 2;
+#   };
+# } U;
+#
+# // This appears first in the debug info and pulls the type definition in...
+# static U __attribute__((used)) _type_anchor;
+# // ... then our useful variable appears last in the debug info and we can
+# // tweak the assembly without needing to edit a lot of offsets by hand.
+# static U ug;
+#
+# extern void f(U);
+#
+# // Omit debug info for main.
+# __attribute__((nodebug))
+# int main() {
+#   ug.raw = 0x64A40101;
+#   f(ug);
+#   f((U)ug.raw);
+# }
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 weird.c -S -o weird.s
+#
+# Then the DWARF was hand modified to get DW_AT_LOCATION for ug from:
+#
+#   DW_AT_location	(DW_OP_addr 0x3f8, DW_OP_deref, DW_OP_constu 0x64a40101, DW_OP_mul, DW_OP_lit0, DW_OP_plus, DW_OP_stack_value)
+#
+# to this:
+#
+#   DW_AT_location	(DW_OP_constu 0x64a40101, DW_OP_stack_value)
+#
+# to work-around a seperate bug.
+
+.zerofill __DATA,__bss,__type_anchor,4,2 ## @_type_anchor
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+	.no_dead_strip	__type_anchor
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+	.zero 138
+	.asciz	"_type_anchor"  ## string offset=138
+	.asciz	"U" ## string offset=151
+	.asciz	"raw"   ## string offset=153
+	.asciz	"unsigned int"  ## string offset=157
+	.asciz	"a" ## string offset=170
+	.asciz	"b" ## string offset=172
+	.asciz	"c" ## string offset=174
+	.asciz	"d" ## string offset=176
+	.asciz	"e" ## string offset=178
+	.asciz	"f" ## string offset=180
+	.asciz	"ug"## string offset=182
+	.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
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_present
+	.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	14  ## DW_FORM_strp
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.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	22  ## DW_TAG_typedef
+	.byte	0   ## DW_CHILDREN_no
+	.byte	73  ## DW_AT_type
+	.byt

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.



Comment at: lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s:14-40
+# typedef union
+# {
+#   unsigned raw;
+#   struct
+#   {
+# unsigned a : 8;
+# unsigned b : 8;

This gives a much more compact debug info section:
```
typedef union {
  unsigned raw;
  struct {
 unsigned a : 8;
 unsigned b : 8;
 unsigned c : 6;
 unsigned d : 2;
 unsigned e : 6;
 unsigned f : 2;
  };
} U;

// This appears first in the debug info and pulls the type definition in...
static U __attribute__((used)) _type_anchor;
// ... then our useful variable appears last in the debug info and we can
// tweak the assembly without needing to edit a lot of offsets by hand.
static U ug;

extern void f(U);

// Omit debug info for main.
__attribute__((nodebug))
int main() {
  ug.raw = 0x64A40101;
  f(ug);
  f((U)ug.raw);
}
```

You can easily edit out the TEXT section, the line table and the accelerator 
tables and patch the location expression to give you a minimal binary.



Comment at: lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s:62-63
+
+   .section__TEXT,__text,regular,pure_instructions
+   .build_version macos, 10, 15sdk_version 10, 15, 6
+   .file   1 "/tmp" "weird.c"

I don't think you need the TEXT segment at all, or at least you can make it 
empty.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I'll leave the test review to Pavel who knows that much better, but I have two 
last nits about the test.




Comment at: lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s:59
+#
+#  86dea1f39bd127776b999e10dff212003068d30a
+#

I think this was still generated with system clang. info_string below says this 
was compiled by `Apple clang version 12.0.0 (clang-1200.0.31.1)` and not the 
listed commit (which would create an info_string like `clang version 12.0.0 
(https://github.com/llvm/llvm-project 
86dea1f39bd127776b999e10dff212003068d30a)`.)



Comment at: lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s:149
+   .asciz  "MacOSX10.15.sdk"   ## string offset=117
+   .asciz  "/Users/friss/dev/stash/xnu" ## string offset=133
+   .asciz  "ug"## string offset=160

You can avoid these system-specific paths by compiling the file in /tmp with 
your cwd in /tmp and passing `-isysroot /` to the clang invocation. This way 
this section would look like this for everyone independently of their system 
username or macOS version (which will make updating this much easier):
```
lang=python
  .asciz  "clang version 12.0.0 (https://github.com/llvm/llvm-project 
6acb897dfbc0ec22007cde50b3bc9c60f4674fb2)" ## string offset=0
  .asciz  "/tmp/weird.c"  ## string offset=101  
  .asciz  "/" ## string offset=114  
  .asciz  "/tmp"  ## string offset=116  
  .asciz  "ug"## string offset=121  
  .asciz  "U" ## string offset=124  
  .asciz  "raw"   ## string offset=126  
  .asciz  "unsigned int"  ## string offset=130  
  .asciz  "a" ## string offset=143 
```


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 285176.
shafik added a comment.

Replacing python test with Shell test


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

https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s

Index: lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/valueobject-pass-by-reg.s
@@ -0,0 +1,550 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable ug" -b | FileCheck %s
+
+# CHECK: (lldb) target variable ug
+# CHECK: (U) ug = {
+# CHECK:   raw = 1688469761
+# CHECK:= (a = 1, b = 1, c = 36, d = 2, e = 36, f = 1)
+# CHECK: }
+
+# We are testing how ValueObject deals with bit-fields when an argument is
+# passed by register. Compiling at -O1 allows us to capture this case and
+# test it.
+#
+# 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 ug= (U)(unsigned)0x0;
+#
+# void f(U u) {
+#   printf( "%d\n", u.raw);
+#   return;
+# }
+#
+# int main() {
+#   ug.raw = 0x64A40101;
+#
+#   f(ug);
+#   printf( "%d\n", ug.raw);
+# }
+#
+#
+# Compiled as follows:
+#
+#   clang -O1 -gdwarf-4 weird.c -S -o weird.s
+#
+# Then the DWARF was hand modified to get DW_AT_LOCATION for ug from:
+#
+#   DW_AT_location	(DW_OP_addr 0x12010, DW_OP_deref_size 0x1, DW_OP_constu 0x64a40101, DW_OP_mul, DW_OP_lit0, DW_OP_plus, DW_OP_stack_value)
+#
+# to this:
+#
+#   DW_AT_location	(DW_OP_constu 0x64a40101, DW_OP_stack_value)
+#
+# to work-around a seperate bug.
+#
+# clang is built from llvm.org and the last commit was:
+#
+#  86dea1f39bd127776b999e10dff212003068d30a
+#
+
+	.section	__TEXT,__text,regular,pure_instructions
+	.build_version macos, 10, 15	sdk_version 10, 15, 6
+	.file	1 "/tmp" "weird.c"
+	.globl	_f  ## -- Begin function f
+	.p2align	4, 0x90
+_f: ## @f
+Lfunc_begin0:
+	.loc	1 16 0  ## /tmp/weird.c:16:0
+	.cfi_startproc
+## %bb.0:
+	##DEBUG_VALUE: f:u <- $edi
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	movl	%edi, %esi
+Ltmp0:
+	##DEBUG_VALUE: f:u <- $esi
+	.loc	1 17 3 prologue_end ## /tmp/weird.c:17:3
+	leaq	L_.str(%rip), %rdi
+	##DEBUG_VALUE: f:u <- $esi
+	xorl	%eax, %eax
+	callq	_printf
+Ltmp1:
+	.loc	1 18 3  ## /tmp/weird.c:18:3
+	popq	%rbp
+	retq
+Ltmp2:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.globl	_main   ## -- Begin function main
+	.p2align	4, 0x90
+_main:  ## @main
+Lfunc_begin1:
+	.loc	1 20 0  ## /tmp/weird.c:20:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+Ltmp3:
+	pushq	%rbx
+	pushq	%rax
+	.cfi_offset %rbx, -24
+	.loc	1 22 10 prologue_end## /tmp/weird.c:22:10
+	movb	$1, _ug.0(%rip)
+	movl	$1688469761, %ebx   ## imm = 0x64A40101
+	.loc	1 26 3  ## /tmp/weird.c:26:3
+	movl	$1688469761, %edi   ## imm = 0x64A40101
+	callq	_f
+Ltmp4:
+	.loc	1 0 3 is_stmt 0 ## /tmp/weird.c:0:3
+	xorl	%eax, %eax
+	.loc	1 27 22 is_stmt 1   ## /tmp/weird.c:27:22
+	cmpb	$0, _ug.0(%rip)
+	cmovel	%eax, %ebx
+	.loc	1 27 3 is_stmt 0## /tmp/weird.c:27:3
+	leaq	L_.str(%rip), %rdi
+	movl	%ebx, %esi
+	xorl	%eax, %eax
+	callq	_printf
+Ltmp5:
+	.loc	1 28 1 is_stmt 1## /tmp/weird.c:28:1
+	xorl	%eax, %eax
+	addq	$8, %rsp
+	popq	%rbx
+	popq	%rbp
+	retq
+Ltmp6:
+Lfunc_end1:
+	.cfi_endproc
+## -- End function
+	.section	__TEXT,__cstring,cstring_literals
+L_.str: ## @.str
+	.asciz	"%d\n"
+
+.zerofill __DATA,__bss,_ug.0,1,2## @ug.0
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+	.asciz	"Apple clang version 12.0.0 (clang-1200.0.31.1)" ## string offset=0
+	.asciz	"/tmp/weird.c"  ## string offset=47
+	.asciz	"/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk" ## string offset=60
+	.asciz	"MacOSX10.15.sdk"   ## string offset=117
+	.asciz	"/Users/friss/dev/stash/xnu" ## string offset=133
+	.asciz	"ug"## string offset=160
+	.asciz	"U" ## string offset=163
+	.asciz	"raw"   ## string offset=165
+	.asciz	"unsigned int"  ## string offset=169
+	.asciz	"a" ## string offset=182
+	.asciz	"b" ## string offset=184
+	.asciz	"c" ## string offset=186
+	.asciz	"d" ## string offset=188
+	.asciz	"e" ## string 

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

We found a way to hand modify the assembly and it looks good, I just need to 
convert it to a test.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

In D85376#2209638 , @labath wrote:

> This manifested itself for variables in registers because those end up being 
> described as `eValueTypeScalar`, is that so?
>
> If that's the case, then I think this would also manifest for variables 
> described via `DW_OP_constu 0xdead, DW_OP_stack_value`. And if *that* is 
> true, then this could be tested a lot simpler by having a global variable 
> described by DW_OP_stack_value and "target variable"-ing it -- no makefiles, 
> no python, just a simple .s file. And it would run on all platforms, not just 
> darwin.
>
> `test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s` tests a very similar thing 
> in precisely this way. I suspect you could just take that test as a template 
> and replace the struct definition with one that contains bitfieds.

This sounds like a great approach, I have unfortunately been struggling to get 
a test case that works. It looks like I am hitting another bug, I am not 
surprised b/c using slight variations on this code I have found other three 
other bugs with how we deal with `DW_OP_piece`, `DW_OP_bit_piece` and I think 
`DW_AT_const_value` respectively.

I have been working w/ this:

  #include 
  
  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 ug= (U)(unsigned)0x0;
  
  void f(U u) {
printf( "%d\n", u.raw);
return;
  }
  
  int main() {
ug.raw = 0x64A40101;
  
f(ug);
printf( "%d\n", ug.raw);
  }

and compiling as using `clang -O1 -gdwarf-4` but we obtain bad values:

  (lldb) target variable ug
  (U) ug = {
raw = 3395301392
 = (a = 16, b = 48, c = 32, d = 1, e = 10, f = 3)
  }

I tried generating assembly and manually adjusting the debug info but I was not 
able to get very far there.

FYI this was the DWARF expression it was generating:

  DW_AT_location(DW_OP_addr 0x12010, DW_OP_deref_size 0x1, 
DW_OP_constu 0x64a40101, DW_OP_mul, DW_OP_lit0, DW_OP_plus, DW_OP_stack_value)

If you can come up w/ a way to generate a test that produces correct values I 
am happy to use it!


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This manifested itself for variables in registers because those end up being 
described as `eValueTypeScalar`, is that so?

If that's the case, then I think this would also manifest for variables 
described via `DW_OP_constu 0xdead, DW_OP_stack_value`. And if *that* is true, 
then this could be tested a lot simpler by having a global variable described 
by DW_OP_stack_value and "target variable"-ing it -- no makefiles, no python, 
just a simple .s file. And it would run on all platforms, not just darwin.

`test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s` tests a very similar thing 
in precisely this way. I suspect you could just take that test as a template 
and replace the struct definition with one that contains bitfieds.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 284507.
shafik added a comment.

Updated to use llvm.org clang, remove header files from the main.c and add the 
commit hash for the clang so that it is easier to replicate the main.s in the 
future.


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

https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile
  
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s

Index: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
@@ -0,0 +1,611 @@
+## This was generated from the following code:
+##
+## We are testing how ValueObject deals with bit-fields when an argument is
+## passed by register. Compiling at -O1 allows us to capture this case and
+## test it.
+##
+## typedef union
+## {
+##   unsigned raw;
+##   struct
+##   {
+## unsigned a : 8;
+## unsigned b : 8;
+## unsigned c : 6;
+## unsigned d : 2;
+## unsigned e : 6;
+## unsigned f : 2;
+##   } ;
+## } U;
+##
+## unsigned f(U u) {
+##   asm("");
+##   return u.raw;
+## }
+##
+## int main() {
+##   U u;
+##   u.raw = 0x64A40101;
+##
+##   return f(u);
+## }
+##
+## Compiled as follows:
+##
+##
+## clang -g -O1 main.c  -o main
+## clang -g -O1 main.c -S -o main.s
+##
+## clang is built from llvm.org and the last commit was:
+##
+##  86dea1f39bd127776b999e10dff212003068d30a
+##
+	.section	__TEXT,__text,regular,pure_instructions
+	.build_version macos, 10, 15
+	.globl	_f  ## -- Begin function f
+	.p2align	4, 0x90
+_f: ## @f
+Lfunc_begin0:
+	.file	1 "/Users/shafik/code" "main.c"
+	.loc	1 15 0  ## main.c:15:0
+	.cfi_startproc
+## %bb.0:   ## %entry
+	##DEBUG_VALUE: f:u <- $edi
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	movl	%edi, %eax
+Ltmp0:
+	##DEBUG_VALUE: f:u <- $eax
+	.loc	1 16 3 prologue_end ## main.c:16:3
+	## InlineAsm Start
+	## InlineAsm End
+	##DEBUG_VALUE: f:u <- $eax
+	.loc	1 17 3  ## main.c:17:3
+	popq	%rbp
+	retq
+Ltmp1:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.globl	_main   ## -- Begin function main
+	.p2align	4, 0x90
+_main:  ## @main
+Lfunc_begin1:
+	.loc	1 20 0  ## main.c:20:0
+	.cfi_startproc
+## %bb.0:   ## %entry
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+Ltmp2:
+	##DEBUG_VALUE: main:u <- 1688469761
+	.loc	1 24 10 prologue_end## main.c:24:10
+	movl	$1688469761, %edi   ## imm = 0x64A40101
+	callq	_f
+Ltmp3:
+	.loc	1 24 3 is_stmt 0## main.c:24:3
+	movl	$1688469761, %eax   ## imm = 0x64A40101
+	popq	%rbp
+	retq
+Ltmp4:
+Lfunc_end1:
+	.cfi_endproc
+## -- End function
+	.section	__DWARF,__debug_loc,regular,debug
+Lsection_debug_loc:
+Ldebug_loc0:
+.set Lset0, Lfunc_begin0-Lfunc_begin0
+	.quad	Lset0
+.set Lset1, Ltmp0-Lfunc_begin0
+	.quad	Lset1
+	.short	1   ## Loc expr size
+	.byte	85  ## super-register DW_OP_reg5
+.set Lset2, Ltmp0-Lfunc_begin0
+	.quad	Lset2
+.set Lset3, Lfunc_end0-Lfunc_begin0
+	.quad	Lset3
+	.short	1   ## Loc expr size
+	.byte	80  ## super-register DW_OP_reg0
+	.quad	0
+	.quad	0
+	.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
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\341\177"  ## DW_AT_APPLE_optimized
+	.byte	25  ## DW_FORM_flag_presen

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I just have a small comment about the test. If you build the test with an 
llvm.org version of clang (best if it contains the git hash it was build from) 
and you don't include headers (they don't seem to be required for the test), 
then the file would be much easier to update/extend for other people.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Do we have an end-to-end (with source code instead of assembler) test for ObjC 
bitfields, too? If not, it might still be a good a idea to add one even if it 
doesn't add coverage for this particular change.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added a comment.

The test LGTM now! Please be sure to address Fred's comment before committing.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 283703.
shafik added a comment.

- Add more tests


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

https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile
  
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s

Index: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
@@ -0,0 +1,653 @@
+## This was generated from the following code:
+##
+## We are testing how ValueObject deals with bit-fields when an argument is
+## passed by register. Compiling at -O1 allows us to capture this case and
+## test it.
+##
+## #include 
+## #include 
+##
+## typedef union
+## {
+##  uint32_t raw;
+##  struct
+##  {
+##uint32_t a : 8;
+##uint32_t b : 8;
+##uint32_t c : 6;
+##uint32_t d : 2;
+##uint32_t e : 6;
+##uint32_t f : 2;
+##  } ;
+## } U;
+##
+## void f(U u) {
+##   printf( "%d\n", u.raw);
+##   return;
+## }
+##
+## int main() {
+##   U u;
+##   u.raw = 0x64A40101;
+##
+##   f(u);
+## }
+##
+## Compiled as follows:
+##
+##
+## clang -g -O1 main.c  -o main
+## clang -g -O1 main.c -S -o main.s
+##
+	.section	__TEXT,__text,regular,pure_instructions
+	.build_version macos, 10, 15	sdk_version 10, 15
+	.globl	_f  ## -- Begin function f
+	.p2align	4, 0x90
+_f: ## @f
+Lfunc_begin0:
+	.file	1 "/Users/shafik/code" "main.c"
+	.loc	1 18 0  ## main.c:18:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	##DEBUG_VALUE: f:u <- $edi
+	movl	%edi, %esi
+Ltmp0:
+	##DEBUG_VALUE: f:u <- $esi
+	.loc	1 19 3 prologue_end ## main.c:19:3
+	leaq	L_.str(%rip), %rdi
+	##DEBUG_VALUE: f:u <- $esi
+	xorl	%eax, %eax
+	popq	%rbp
+	jmp	_printf ## TAILCALL
+Ltmp1:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.globl	_main   ## -- Begin function main
+	.p2align	4, 0x90
+_main:  ## @main
+Lfunc_begin1:
+	.loc	1 23 0  ## main.c:23:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+Ltmp2:
+	##DEBUG_VALUE: main:u <- 1688469761
+	.loc	1 27 3 prologue_end ## main.c:27:3
+	movl	$1688469761, %edi   ## imm = 0x64A40101
+	callq	_f
+Ltmp3:
+	.loc	1 28 1  ## main.c:28:1
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+Ltmp4:
+Lfunc_end1:
+	.cfi_endproc
+## -- End function
+	.section	__TEXT,__cstring,cstring_literals
+L_.str: ## @.str
+	.asciz	"%d\n"
+
+	.file	2 "/Applications/Xcode5.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/_types" "_uint32_t.h"
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+	.asciz	"Apple clang version 11.0.0 (clang-1100.0.31.5)" ## string offset=0
+	.asciz	"main.c"## string offset=47
+	.asciz	"/Users/shafik/code"## string offset=54
+	.asciz	"f" ## string offset=73
+	.asciz	"main"  ## string offset=75
+	.asciz	"int"   ## string offset=80
+	.asciz	"u" ## string offset=84
+	.asciz	"U" ## string offset=86
+	.asciz	"raw"   ## string offset=88
+	.asciz	"uint32_t"  ## string offset=92
+	.asciz	"unsigned int"  ## string offset=101
+	.asciz	"a" ## string offset=114
+	.asciz	"b" ## string offset=116
+	.asciz	"c" ## string offset=118
+	.asciz	"d" ## string offset=120
+	.asciz	"e" ## string offset=122
+	.section	__DWARF,__debug_loc,regular,debug
+Lsection_debug_loc:
+Ldebug_loc0:
+.set Lset0, Lfunc_begin0-Lfunc_begin0
+	.quad	Lset0
+.set Lset1, Ltmp0-Lfunc_begin0
+	.quad	Lset1
+	.short	1   ## Loc expr size
+	.byte	85  ## super-register DW_OP_reg5
+.set Lset2, Ltmp0-Lfunc_begin0
+	.quad	Lset2
+.set Lset3, Ltmp1-Lfunc_begin0
+	.quad	Lset3
+	.short	1   ## Loc expr size
+	.byte	84  ## super-register DW_OP_reg4
+	.quad	0
+	.quad	0
+	.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 

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Core/ValueObjectChild.cpp:202-205
-if (m_bitfield_bit_size)
-  scalar.ExtractBitfield(m_bitfield_bit_size,
- m_bitfield_bit_offset);
-else

shafik wrote:
> friss wrote:
> > shafik wrote:
> > > friss wrote:
> > > > Why remove the code in `ValueObject` rather than avoid re-extracting at 
> > > > printing time? I'm not sure which one is correct. If you get your hands 
> > > > on a `ValueObject` for the field in your test, what will 
> > > > `GetValueAsUnsigned` return? it should give the correct field value.
> > > `lldb_private::DumpDataExtractor(…)` is general purpose and it used by a 
> > > lot of other code, it does know the value comes from a `Scalar` or 
> > > otherwise it is just receiving a `DataExtractor` and obtaining the data 
> > > from there. 
> > You didn't answer the most important question. Will `GetValueAsUnsigned` 
> > return the correct value on such a ValueObject once you remove this code?
> apologies, misunderstood.
> 
> Yes, it does:
> 
> ```
> (lldb) script var = lldb.frame.FindVariable("u")
> (lldb) script print(var.GetChildMemberWithName('raw'))
> (uint32_t) raw = 1688469761
> (lldb) script print(var.GetChildMemberWithName('a'))
> (uint32_t:8) a = 1
> (lldb) script print(var.GetChildMemberWithName('b'))
> (uint32_t:8) b = 1
> (lldb) script print(var.GetChildMemberWithName('c'))
> (uint32_t:6) c = 36
> (lldb) script print(var.GetChildMemberWithName('d'))
> (uint32_t:2) d = 2
> (lldb) script print(var.GetChildMemberWithName('e'))
> (uint32_t:6) e = 36
> (lldb) script print(var.GetChildMemberWithName('f'))
> (uint32_t:2) f = 1
> 
> ```
Whoops, copy-pasta:

```
(lldb) script print(var.GetChildMemberWithName('raw').GetValueAsUnsigned())
1688469761
(lldb) script print(var.GetChildMemberWithName('a').GetValueAsUnsigned())
1
(lldb) script print(var.GetChildMemberWithName('b').GetValueAsUnsigned())
1
(lldb) script print(var.GetChildMemberWithName('c').GetValueAsUnsigned())
36
(lldb) script print(var.GetChildMemberWithName('d').GetValueAsUnsigned())
2
(lldb) script print(var.GetChildMemberWithName('e').GetValueAsUnsigned())
36
(lldb) script print(var.GetChildMemberWithName('f').GetValueAsUnsigned())
1
```


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Core/ValueObjectChild.cpp:202-205
-if (m_bitfield_bit_size)
-  scalar.ExtractBitfield(m_bitfield_bit_size,
- m_bitfield_bit_offset);
-else

friss wrote:
> shafik wrote:
> > friss wrote:
> > > Why remove the code in `ValueObject` rather than avoid re-extracting at 
> > > printing time? I'm not sure which one is correct. If you get your hands 
> > > on a `ValueObject` for the field in your test, what will 
> > > `GetValueAsUnsigned` return? it should give the correct field value.
> > `lldb_private::DumpDataExtractor(…)` is general purpose and it used by a 
> > lot of other code, it does know the value comes from a `Scalar` or 
> > otherwise it is just receiving a `DataExtractor` and obtaining the data 
> > from there. 
> You didn't answer the most important question. Will `GetValueAsUnsigned` 
> return the correct value on such a ValueObject once you remove this code?
apologies, misunderstood.

Yes, it does:

```
(lldb) script var = lldb.frame.FindVariable("u")
(lldb) script print(var.GetChildMemberWithName('raw'))
(uint32_t) raw = 1688469761
(lldb) script print(var.GetChildMemberWithName('a'))
(uint32_t:8) a = 1
(lldb) script print(var.GetChildMemberWithName('b'))
(uint32_t:8) b = 1
(lldb) script print(var.GetChildMemberWithName('c'))
(uint32_t:6) c = 36
(lldb) script print(var.GetChildMemberWithName('d'))
(uint32_t:2) d = 2
(lldb) script print(var.GetChildMemberWithName('e'))
(uint32_t:6) e = 36
(lldb) script print(var.GetChildMemberWithName('f'))
(uint32_t:2) f = 1

```


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.



Comment at: lldb/source/Core/ValueObjectChild.cpp:202-205
-if (m_bitfield_bit_size)
-  scalar.ExtractBitfield(m_bitfield_bit_size,
- m_bitfield_bit_offset);
-else

shafik wrote:
> friss wrote:
> > Why remove the code in `ValueObject` rather than avoid re-extracting at 
> > printing time? I'm not sure which one is correct. If you get your hands on 
> > a `ValueObject` for the field in your test, what will `GetValueAsUnsigned` 
> > return? it should give the correct field value.
> `lldb_private::DumpDataExtractor(…)` is general purpose and it used by a lot 
> of other code, it does know the value comes from a `Scalar` or otherwise it 
> is just receiving a `DataExtractor` and obtaining the data from there. 
You didn't answer the most important question. Will `GetValueAsUnsigned` return 
the correct value on such a ValueObject once you remove this code?


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 283658.
shafik added a comment.

- Refactored the Makefile and test based on offline comments from Adrian.


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

https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile
  
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s

Index: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
@@ -0,0 +1,653 @@
+## This was generated from the following code:
+##
+## We are testing how ValueObject deals with bit-fields when an argument is
+## passed by register. Compiling at -O1 allows us to capture this case and
+## test it.
+##
+## #include 
+## #include 
+##
+## typedef union
+## {
+##  uint32_t raw;
+##  struct
+##  {
+##uint32_t a : 8;
+##uint32_t b : 8;
+##uint32_t c : 6;
+##uint32_t d : 2;
+##uint32_t e : 6;
+##uint32_t f : 2;
+##  } ;
+## } U;
+##
+## void f(U u) {
+##   printf( "%d\n", u.raw);
+##   return;
+## }
+##
+## int main() {
+##   U u;
+##   u.raw = 0x64A40101;
+##
+##   f(u);
+## }
+##
+## Compiled as follows:
+##
+##
+## clang -g -O1 main.c  -o main
+## clang -g -O1 main.c -S -o main.s
+##
+	.section	__TEXT,__text,regular,pure_instructions
+	.build_version macos, 10, 15	sdk_version 10, 15
+	.globl	_f  ## -- Begin function f
+	.p2align	4, 0x90
+_f: ## @f
+Lfunc_begin0:
+	.file	1 "/Users/shafik/code" "main.c"
+	.loc	1 18 0  ## main.c:18:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	##DEBUG_VALUE: f:u <- $edi
+	movl	%edi, %esi
+Ltmp0:
+	##DEBUG_VALUE: f:u <- $esi
+	.loc	1 19 3 prologue_end ## main.c:19:3
+	leaq	L_.str(%rip), %rdi
+	##DEBUG_VALUE: f:u <- $esi
+	xorl	%eax, %eax
+	popq	%rbp
+	jmp	_printf ## TAILCALL
+Ltmp1:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.globl	_main   ## -- Begin function main
+	.p2align	4, 0x90
+_main:  ## @main
+Lfunc_begin1:
+	.loc	1 23 0  ## main.c:23:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+Ltmp2:
+	##DEBUG_VALUE: main:u <- 1688469761
+	.loc	1 27 3 prologue_end ## main.c:27:3
+	movl	$1688469761, %edi   ## imm = 0x64A40101
+	callq	_f
+Ltmp3:
+	.loc	1 28 1  ## main.c:28:1
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+Ltmp4:
+Lfunc_end1:
+	.cfi_endproc
+## -- End function
+	.section	__TEXT,__cstring,cstring_literals
+L_.str: ## @.str
+	.asciz	"%d\n"
+
+	.file	2 "/Applications/Xcode5.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/_types" "_uint32_t.h"
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+	.asciz	"Apple clang version 11.0.0 (clang-1100.0.31.5)" ## string offset=0
+	.asciz	"main.c"## string offset=47
+	.asciz	"/Users/shafik/code"## string offset=54
+	.asciz	"f" ## string offset=73
+	.asciz	"main"  ## string offset=75
+	.asciz	"int"   ## string offset=80
+	.asciz	"u" ## string offset=84
+	.asciz	"U" ## string offset=86
+	.asciz	"raw"   ## string offset=88
+	.asciz	"uint32_t"  ## string offset=92
+	.asciz	"unsigned int"  ## string offset=101
+	.asciz	"a" ## string offset=114
+	.asciz	"b" ## string offset=116
+	.asciz	"c" ## string offset=118
+	.asciz	"d" ## string offset=120
+	.asciz	"e" ## string offset=122
+	.section	__DWARF,__debug_loc,regular,debug
+Lsection_debug_loc:
+Ldebug_loc0:
+.set Lset0, Lfunc_begin0-Lfunc_begin0
+	.quad	Lset0
+.set Lset1, Ltmp0-Lfunc_begin0
+	.quad	Lset1
+	.short	1   ## Loc expr size
+	.byte	85  ## super-register DW_OP_reg5
+.set Lset2, Ltmp0-Lfunc_begin0
+	.quad	Lset2
+.set Lset3, Ltmp1-Lfunc_begin0
+	.quad	Lset3
+	.short	1   ## Loc expr size
+	.byte	84  ## super-register DW_OP_reg4
+	.quad	0
+	.quad	0
+	.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
+	

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 283647.
shafik added a comment.

Removing use of `-O1` from Makefile.


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

https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile
  
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s

Index: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
@@ -0,0 +1,653 @@
+## This was generated from the following code:
+##
+## We are testing how ValueObject deals with bit-fields when an argument is
+## passed by register. Compiling at -O1 allows us to capture this case and
+## test it.
+##
+## #include 
+## #include 
+##
+## typedef union
+## {
+##  uint32_t raw;
+##  struct
+##  {
+##uint32_t a : 8;
+##uint32_t b : 8;
+##uint32_t c : 6;
+##uint32_t d : 2;
+##uint32_t e : 6;
+##uint32_t f : 2;
+##  } ;
+## } U;
+##
+## void f(U u) {
+##   printf( "%d\n", u.raw);
+##   return;
+## }
+##
+## int main() {
+##   U u;
+##   u.raw = 0x64A40101;
+##
+##   f(u);
+## }
+##
+## Compiled as follows:
+##
+##
+## clang -g -O1 main.c  -o main
+## clang -g -O1 main.c -S -o main.s
+##
+	.section	__TEXT,__text,regular,pure_instructions
+	.build_version macos, 10, 15	sdk_version 10, 15
+	.globl	_f  ## -- Begin function f
+	.p2align	4, 0x90
+_f: ## @f
+Lfunc_begin0:
+	.file	1 "/Users/shafik/code" "main.c"
+	.loc	1 18 0  ## main.c:18:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	##DEBUG_VALUE: f:u <- $edi
+	movl	%edi, %esi
+Ltmp0:
+	##DEBUG_VALUE: f:u <- $esi
+	.loc	1 19 3 prologue_end ## main.c:19:3
+	leaq	L_.str(%rip), %rdi
+	##DEBUG_VALUE: f:u <- $esi
+	xorl	%eax, %eax
+	popq	%rbp
+	jmp	_printf ## TAILCALL
+Ltmp1:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.globl	_main   ## -- Begin function main
+	.p2align	4, 0x90
+_main:  ## @main
+Lfunc_begin1:
+	.loc	1 23 0  ## main.c:23:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+Ltmp2:
+	##DEBUG_VALUE: main:u <- 1688469761
+	.loc	1 27 3 prologue_end ## main.c:27:3
+	movl	$1688469761, %edi   ## imm = 0x64A40101
+	callq	_f
+Ltmp3:
+	.loc	1 28 1  ## main.c:28:1
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+Ltmp4:
+Lfunc_end1:
+	.cfi_endproc
+## -- End function
+	.section	__TEXT,__cstring,cstring_literals
+L_.str: ## @.str
+	.asciz	"%d\n"
+
+	.file	2 "/Applications/Xcode5.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/_types" "_uint32_t.h"
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+	.asciz	"Apple clang version 11.0.0 (clang-1100.0.31.5)" ## string offset=0
+	.asciz	"main.c"## string offset=47
+	.asciz	"/Users/shafik/code"## string offset=54
+	.asciz	"f" ## string offset=73
+	.asciz	"main"  ## string offset=75
+	.asciz	"int"   ## string offset=80
+	.asciz	"u" ## string offset=84
+	.asciz	"U" ## string offset=86
+	.asciz	"raw"   ## string offset=88
+	.asciz	"uint32_t"  ## string offset=92
+	.asciz	"unsigned int"  ## string offset=101
+	.asciz	"a" ## string offset=114
+	.asciz	"b" ## string offset=116
+	.asciz	"c" ## string offset=118
+	.asciz	"d" ## string offset=120
+	.asciz	"e" ## string offset=122
+	.section	__DWARF,__debug_loc,regular,debug
+Lsection_debug_loc:
+Ldebug_loc0:
+.set Lset0, Lfunc_begin0-Lfunc_begin0
+	.quad	Lset0
+.set Lset1, Ltmp0-Lfunc_begin0
+	.quad	Lset1
+	.short	1   ## Loc expr size
+	.byte	85  ## super-register DW_OP_reg5
+.set Lset2, Ltmp0-Lfunc_begin0
+	.quad	Lset2
+.set Lset3, Ltmp1-Lfunc_begin0
+	.quad	Lset3
+	.short	1   ## Loc expr size
+	.byte	84  ## super-register DW_OP_reg4
+	.quad	0
+	.quad	0
+	.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_F

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a subscriber: JDevlieghere.
shafik added inline comments.



Comment at: lldb/source/Core/ValueObjectChild.cpp:202-205
-if (m_bitfield_bit_size)
-  scalar.ExtractBitfield(m_bitfield_bit_size,
- m_bitfield_bit_offset);
-else

friss wrote:
> Why remove the code in `ValueObject` rather than avoid re-extracting at 
> printing time? I'm not sure which one is correct. If you get your hands on a 
> `ValueObject` for the field in your test, what will `GetValueAsUnsigned` 
> return? it should give the correct field value.
`lldb_private::DumpDataExtractor(…)` is general purpose and it used by a lot of 
other code, it does know the value comes from a `Scalar` or otherwise it is 
just receiving a `DataExtractor` and obtaining the data from there. 



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2
+EXE := a.out
+CFLAGS := -O1
+

aprantl wrote:
> davide wrote:
> > davide wrote:
> > > This is fundamentally a no-go. Depending on the optimization pipeline 
> > > passes this in a register is an assumption that might go away at some 
> > > point in the future and this test won't test what it has to & will still 
> > > pass silently.
> > > 
> > > Something like this might work:
> > > https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
> > *depending on the optimization pipeline, the fact that is passed in a 
> > register is an assumption that
> Given that the source code is a .s file, I think the -O1 is just redundant 
> and can be removed. Using assembler is fine for this purpose.
I did try using Specifying Registers for Local Variables and it did not work :-(

but in good news, I don't need the `-O1` it was left over when I was struggling 
to get the test going.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2
+EXE := a.out
+CFLAGS := -O1
+

shafik wrote:
> aprantl wrote:
> > davide wrote:
> > > davide wrote:
> > > > This is fundamentally a no-go. Depending on the optimization pipeline 
> > > > passes this in a register is an assumption that might go away at some 
> > > > point in the future and this test won't test what it has to & will 
> > > > still pass silently.
> > > > 
> > > > Something like this might work:
> > > > https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
> > > *depending on the optimization pipeline, the fact that is passed in a 
> > > register is an assumption that
> > Given that the source code is a .s file, I think the -O1 is just redundant 
> > and can be removed. Using assembler is fine for this purpose.
> I did try using Specifying Registers for Local Variables and it did not work 
> :-(
> 
> but in good news, I don't need the `-O1` it was left over when I was 
> struggling to get the test going.
Yes, this was left over when I was experimenting trying to get the test to work 
I do not need the `-O1`.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:8
+   $(CC) $(CFLAGS) $(SRCDIR)/main.s -c -o main.o
+   $(CC) $(CFLAGS) main.o -o a.out

aprantl wrote:
> Is it possible to just override the rule that produces the .o file? Otherwise 
> you are dropping the codesign and dsymutil phase.
Let me see if I can remove the `.o` step.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py:10
+
+@skipUnlessDarwin
+def test(self):

friss wrote:
> If the test in assembly is what we want, this is also architecture specific. 
> It should be restricted to x86_64
Yes.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py:15
+self.runCmd("b f");
+self.runCmd("run");
+

aprantl wrote:
> lldbutil has a helper for running to a breakpoint by name.
I could not get it to work using `lldbutil` I was working w/ @JDevlieghere on 
this and he thought that made sense. IIUC I would have rewrite the assembly 
file to make it work.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:1
+EXE := a.out
+CFLAGS := -O1

I think this is redundant. The default is a.out



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2
+EXE := a.out
+CFLAGS := -O1
+

davide wrote:
> davide wrote:
> > This is fundamentally a no-go. Depending on the optimization pipeline 
> > passes this in a register is an assumption that might go away at some point 
> > in the future and this test won't test what it has to & will still pass 
> > silently.
> > 
> > Something like this might work:
> > https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
> *depending on the optimization pipeline, the fact that is passed in a 
> register is an assumption that
Given that the source code is a .s file, I think the -O1 is just redundant and 
can be removed. Using assembler is fine for this purpose.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:8
+   $(CC) $(CFLAGS) $(SRCDIR)/main.s -c -o main.o
+   $(CC) $(CFLAGS) main.o -o a.out

Is it possible to just override the rule that produces the .o file? Otherwise 
you are dropping the codesign and dsymutil phase.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py:15
+self.runCmd("b f");
+self.runCmd("run");
+

lldbutil has a helper for running to a breakpoint by name.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2
+EXE := a.out
+CFLAGS := -O1
+

davide wrote:
> This is fundamentally a no-go. Depending on the optimization pipeline passes 
> this in a register is an assumption that might go away at some point in the 
> future and this test won't test what it has to & will still pass silently.
> 
> Something like this might work:
> https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
*depending on the optimization pipeline, the fact that is passed in a register 
is an assumption that


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Davide Italiano via Phabricator via lldb-commits]

davide requested changes to this revision.
davide added inline comments.
This revision now requires changes to proceed.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile:2
+EXE := a.out
+CFLAGS := -O1
+

This is fundamentally a no-go. Depending on the optimization pipeline passes 
this in a register is an assumption that might go away at some point in the 
future and this test won't test what it has to & will still pass silently.

Something like this might work:
https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.



Comment at: lldb/source/Core/ValueObjectChild.cpp:202-205
-if (m_bitfield_bit_size)
-  scalar.ExtractBitfield(m_bitfield_bit_size,
- m_bitfield_bit_offset);
-else

Why remove the code in `ValueObject` rather than avoid re-extracting at 
printing time? I'm not sure which one is correct. If you get your hands on a 
`ValueObject` for the field in your test, what will `GetValueAsUnsigned` 
return? it should give the correct field value.



Comment at: 
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py:10
+
+@skipUnlessDarwin
+def test(self):

If the test in assembly is what we want, this is also architecture specific. It 
should be restricted to x86_64


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

Note: for the test I did not want to rely on clang choosing to pass the union 
by register, so I used assembly which ensures I will obtain the behavior I am 
looking to capture for the test.


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

https://reviews.llvm.org/D85376

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

[Lldb-commits] [PATCH] D85376: Fix how ValueObjectChild handles bit-fields stored in a Scalar in UpdateValue()

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: aprantl, jingham, vsk.
shafik requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

When bit-field data was stored in a `Scalar` in `ValueObjectChild` during 
`UpdateValue()` it was extracting the bit-field value. Later on in 
`lldb_private::DumpDataExtractor(…)` we were again attempting to extract the 
bit-field:

  s->Printf("%" PRIu64,
  DE.GetMaxU64Bitfield(&offset, item_byte_size, item_bit_size,
   item_bit_offset));

which would then not obtain the correct value. This will remove the extra 
extraction in `UpdateValue()`.

We hit this specific case when values are passed in registers, which we could 
only reproduce in an optimized build.


https://reviews.llvm.org/D85376

Files:
  lldb/source/Core/ValueObjectChild.cpp
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/Makefile
  
lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/TestValueObjPassByRef.py
  lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s

Index: lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/valueobj-pass-by-reg/main.s
@@ -0,0 +1,653 @@
+## This was generated from the following code:
+##
+## We are testing how ValueObject deals with bit-fields when an argument is
+## passed by register. Compiling at -O1 allows us to capture this case and
+## test it.
+##
+## #include 
+## #include 
+##
+## typedef union
+## {
+##  uint32_t raw;
+##  struct
+##  {
+##uint32_t a : 8;
+##uint32_t b : 8;
+##uint32_t c : 6;
+##uint32_t d : 2;
+##uint32_t e : 6;
+##uint32_t f : 2;
+##  } ;
+## } U;
+##
+## void f(U u) {
+##   printf( "%d\n", u.raw);
+##   return;
+## }
+##
+## int main() {
+##   U u;
+##   u.raw = 0x64A40101;
+##
+##   f(u);
+## }
+##
+## Compiled as follows:
+##
+##
+## clang -g -O1 main.c  -o main
+## clang -g -O1 main.c -S -o main.s
+##
+	.section	__TEXT,__text,regular,pure_instructions
+	.build_version macos, 10, 15	sdk_version 10, 15
+	.globl	_f  ## -- Begin function f
+	.p2align	4, 0x90
+_f: ## @f
+Lfunc_begin0:
+	.file	1 "/Users/shafik/code" "main.c"
+	.loc	1 18 0  ## main.c:18:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	##DEBUG_VALUE: f:u <- $edi
+	movl	%edi, %esi
+Ltmp0:
+	##DEBUG_VALUE: f:u <- $esi
+	.loc	1 19 3 prologue_end ## main.c:19:3
+	leaq	L_.str(%rip), %rdi
+	##DEBUG_VALUE: f:u <- $esi
+	xorl	%eax, %eax
+	popq	%rbp
+	jmp	_printf ## TAILCALL
+Ltmp1:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.globl	_main   ## -- Begin function main
+	.p2align	4, 0x90
+_main:  ## @main
+Lfunc_begin1:
+	.loc	1 23 0  ## main.c:23:0
+	.cfi_startproc
+## %bb.0:
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+Ltmp2:
+	##DEBUG_VALUE: main:u <- 1688469761
+	.loc	1 27 3 prologue_end ## main.c:27:3
+	movl	$1688469761, %edi   ## imm = 0x64A40101
+	callq	_f
+Ltmp3:
+	.loc	1 28 1  ## main.c:28:1
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+Ltmp4:
+Lfunc_end1:
+	.cfi_endproc
+## -- End function
+	.section	__TEXT,__cstring,cstring_literals
+L_.str: ## @.str
+	.asciz	"%d\n"
+
+	.file	2 "/Applications/Xcode5.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/_types" "_uint32_t.h"
+	.section	__DWARF,__debug_str,regular,debug
+Linfo_string:
+	.asciz	"Apple clang version 11.0.0 (clang-1100.0.31.5)" ## string offset=0
+	.asciz	"main.c"## string offset=47
+	.asciz	"/Users/shafik/code"## string offset=54
+	.asciz	"f" ## string offset=73
+	.asciz	"main"  ## string offset=75
+	.asciz	"int"   ## string offset=80
+	.asciz	"u" ## string offset=84
+	.asciz	"U" ## string offset=86
+	.asciz	"raw"   ## string offset=88
+	.asciz	"uint32_t"  ## string offset=92
+	.asciz	"unsigned int"  ## string offset=101
+	.asciz	"a" ## string offset=114
+	.asciz	"b" ## string offset=116
+	.asciz	"c" ## string offset=118
+	.asciz	"d" ## string offset=120
+	.asciz	"e" ## string offset=122
+	.section	__DWARF,__debug_loc,regular,debug
+Lsection_debug_loc:
+Ldebug_loc0:
+.set Lset0, Lfunc_begin0-Lfunc_begin0
+	.quad	Lset0
+.set Lset1, Ltmp0-Lfunc_begin0
+	.quad	Lset1
+	.short	1