[PATCH v3 1/2] [gdb] Untied inheritance virtuality and loc kind

[Bruno Larsen <blarsen@redhat.com>]

DWARF specifications, when talking about C++ virtual classes, don't tie
their existance or non-existance to how the location is described, but
the original offset decoding did, assuming that we could use BITPOS for
non-virtual inheritance.

The new code assumes that if there isn't virtuality at play, we probably
have BITPOS, but we may need to calculate an expression, so allows for
fallthrough. Also, the special case on value_primitive_field, on
gdb/value.c, was changed so we call baseclass_offset on non-trivial
expressions.

A new testcase was added, where we use a trivial locexpr instead of a
bitpos, to showcase one of this instances
---
 gdb/gnu-v3-abi.c                              |  15 +-
 .../gdb.dwarf2/dw2-inheritance-locexpr-1.exp  | 233 ++++++++++++++++++
 .../gdb.dwarf2/dw2-inheritance-locexpr.c      |  69 ++++++
 gdb/value.c                                   |   8 +-
 4 files changed, 319 insertions(+), 6 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inheritance-locexpr-1.exp
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inheritance-locexpr.c

diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 45e57c210cb..55e0cf24deb 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -458,10 +458,17 @@ gnuv3_baseclass_offset (struct type *type, int index,
   gdbarch = type->arch ();
   ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
 
-  /* If it isn't a virtual base, this is easy.  The offset is in the
-     type definition.  */
-  if (!BASETYPE_VIA_VIRTUAL (type, index))
-    return TYPE_BASECLASS_BITPOS (type, index) / 8;
+  if (!BASETYPE_VIA_VIRTUAL (type, index)){
+      /* If it isn't a virtual base, this is easy.
+         The offset is probably in the type definition.
+	 However, the DWARF specification doesn't tie virtuality with location
+	 so we may need to resolve location expressions.
+	 To reuse code, we just fall-through in that case */
+      if (TYPE_FIELD_LOC_KIND (type, index) == FIELD_LOC_KIND_BITPOS){
+	  cur_base_offset = TYPE_BASECLASS_BITPOS (type, index) / 8;
+	return TYPE_BASECLASS_BITPOS (type, index) / 8;
+      }
+  }
 
   /* If we have a DWARF expression for the offset, evaluate it.  */
   if (TYPE_FIELD_LOC_KIND (type, index) == FIELD_LOC_KIND_DWARF_BLOCK)
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inheritance-locexpr-1.exp b/gdb/testsuite/gdb.dwarf2/dw2-inheritance-locexpr-1.exp
new file mode 100644
index 00000000000..c1a7bea1b88
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inheritance-locexpr-1.exp
@@ -0,0 +1,233 @@
+# Copyright 2018-2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if ![dwarf2_support] {
+    return 0
+}
+
+# We'll place the output of Dwarf::assemble in pr28030.S.
+standard_testfile dw2-inheritance-locexpr.c .S
+
+# ${testfile} is now "pr28030".  srcfile2 is "pr28030.S".
+set executable ${testfile}
+set asm_file [standard_output_file ${srcfile2}]
+
+# We need to know the size of integer and address types in order
+# to write some of the debugging info we'd like to generate.
+#
+# For that, we ask GDB by debugging our pr28030 program.
+# Any program would do, but since we already have pr28030
+# specifically for this testcase, might as well use that.
+if [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] {
+    return -1
+}
+set long_size [get_sizeof "long" -1]
+# gdb always assumes references are implemented as pointers.
+set addr_size [get_sizeof "void *" -1]
+set struct_A_size [get_sizeof "g_A" 4]
+set struct_B_size [get_sizeof "g_B" 4]
+set struct_C_size [get_sizeof "g_C" 4]
+
+# Create the DWARF.
+Dwarf::assemble ${asm_file} {
+    global srcdir subdir srcfile srcfile2
+    global long_size addr_size struct_A_size struct_B_size struct_C_size
+    declare_labels L
+
+    cu {} {
+	DW_TAG_compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	    {name pr28030.c}
+	    {stmt_list $L DW_FORM_sec_offset}
+        } {
+	    declare_labels int_label class_A_label class_B_label class_C_label
+
+	    int_label: DW_TAG_base_type {
+		{DW_AT_byte_size ${long_size} DW_FORM_udata}
+		{DW_AT_encoding @DW_ATE_signed}
+		{DW_AT_name "int"}
+	    }
+
+	    class_A_label: DW_TAG_class_type {
+		{DW_AT_name "A"}
+		{DW_AT_byte_size $struct_A_size DW_FORM_sdata}
+	    } {
+		DW_TAG_member {
+		    {DW_AT_name "a"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location 0*$long_size DW_FORM_udata}
+		}
+		DW_TAG_member {
+		    {DW_AT_name "x"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location 1*$long_size DW_FORM_udata}
+		}
+	    }
+
+	    class_B_label: DW_TAG_class_type {
+		{DW_AT_name "B"}
+		{DW_AT_byte_size $struct_B_size DW_FORM_sdata}
+	    } {
+		DW_TAG_inheritance {
+		    {DW_AT_type :$class_A_label}
+		    {DW_AT_data_member_location {
+			DW_OP_constu 0
+			DW_OP_plus
+			DW_OP_stack_value } SPECIAL_expr}
+		    {DW_AT_accessibility 1 DW_FORM_data1}
+		}
+		DW_TAG_member {
+		    {DW_AT_name "b"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location 2*$long_size DW_FORM_udata}
+		}
+	    }
+
+	    class_C_label: DW_TAG_class_type {
+		{DW_AT_name "C"}
+		{DW_AT_byte_size $struct_C_size DW_FORM_sdata}
+	    } {
+		DW_TAG_inheritance {
+		    {DW_AT_type :$class_A_label}
+		    {DW_AT_data_member_location { DW_OP_constu 9000 } SPECIAL_expr}
+		    {DW_AT_accessibility 1 DW_FORM_data1}
+		}
+		DW_TAG_member {
+		    {DW_AT_name "b"}
+		    {DW_AT_type :$int_label}
+		    {DW_AT_data_member_location 2*$long_size DW_FORM_udata}
+		}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "g_A"}
+		{DW_AT_type :$class_A_label}
+		{DW_AT_external 1 flag}
+		{DW_AT_location {DW_OP_addr [gdb_target_symbol "g_A"]} SPECIAL_expr}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "g_B"}
+		{DW_AT_type :$class_B_label}
+		{DW_AT_external 1 flag}
+		{DW_AT_location {DW_OP_addr [gdb_target_symbol "g_B"]} SPECIAL_expr}
+	    }
+
+	    DW_TAG_variable {
+		{DW_AT_name "g_C"}
+		{DW_AT_type :$class_C_label}
+		{DW_AT_external 1 flag}
+		{DW_AT_location {DW_OP_addr [gdb_target_symbol "g_C"]} SPECIAL_expr}
+	    }
+
+	    DW_TAG_subprogram {
+		{MACRO_AT_func { "foo" }}
+		{DW_AT_type :${class_B_label}}
+		{DW_AT_external 1 flag}
+	    }
+	    DW_TAG_subprogram {
+		{MACRO_AT_func { "bar" }}
+		{DW_AT_type :${class_C_label}}
+		{DW_AT_external 1 flag}
+	    }
+	    DW_TAG_subprogram {
+		{MACRO_AT_func { "main" }}
+		{DW_AT_type :${int_label}}
+		{DW_AT_external 1 DW_FORM_flag}
+	    }
+	}
+    }
+
+    lines {version 2} L {
+	include_dir "${srcdir}/${subdir}"
+	file_name "$srcfile" 1
+
+	lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \
+	    main_start main_len
+	set main_end "$main_start + $main_len"
+	lassign [function_range foo [list ${srcdir}/${subdir}/$srcfile]] \
+	    foo_start foo_len
+	set foo_end "$foo_start + $foo_len"
+	lassign [function_range bar [list ${srcdir}/${subdir}/$srcfile]] \
+	    bar_start bar_len
+	set bar_end "$bar_start + $bar_len"
+
+	# Generate a line table program.  An attempt was made to make it
+	# reasonably accurate as it made debugging the test case easier.
+	program {
+	    {DW_LNE_set_address $main_start}
+	    {line [gdb_get_line_number "main prologue"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address main_label}
+	    {line [gdb_get_line_number "main foo call"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address main_bar_label}
+	    {line [gdb_get_line_number "main bar call"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address main_label2}
+	    {line [gdb_get_line_number "main return"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address $main_end}
+	    {line [expr [gdb_get_line_number "main end"] + 1]}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address $foo_start}
+	    {line [gdb_get_line_number "foo prologue"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address foo_label}
+	    {line [gdb_get_line_number "foo return"]}
+	    {DW_LNS_copy}
+	    {line [gdb_get_line_number "foo end"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address $foo_end}
+	    {DW_LNS_advance_line 1}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	    {DW_LNE_set_address $bar_start}
+	    {line [gdb_get_line_number "bar prologue"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address bar_label}
+	    {line [gdb_get_line_number "bar return"]}
+	    {DW_LNS_copy}
+	    {line [gdb_get_line_number "bar end"]}
+	    {DW_LNS_copy}
+	    {DW_LNE_set_address $bar_end}
+	    {DW_LNS_advance_line 1}
+	    {DW_LNS_copy}
+	    {DW_LNE_end_sequence}
+
+	}
+    }
+}
+
+if [prepare_for_testing "failed to prepare" ${executable} [list ${asm_file} ${srcfile}] {}] {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_test "step" "foo .. at .* foo end.*" "step into foo"
+gdb_test "finish" "= {<A> = {a = 5, x = 6}, b = 7}" "finish out of foo"
+
+if ![runto_main] {
+    return -1
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inheritance-locexpr.c b/gdb/testsuite/gdb.dwarf2/dw2-inheritance-locexpr.c
new file mode 100644
index 00000000000..0de0cb04b34
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inheritance-locexpr.c
@@ -0,0 +1,69 @@
+/* Copyright (C) 2018-2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+typedef struct A {
+    long a;
+    long x;
+} A;
+
+struct A g_A = {3, 4};
+
+typedef struct B {
+    long a;
+    long x;
+    long b;
+} B;
+
+struct B g_B = { 5, 6, 7 };
+
+typedef struct C {
+    long a;
+    long x;
+    long b;
+} C;
+
+struct C g_C = { 8, 9, 10 };
+
+B
+foo ()
+{						/* foo prologue */
+  asm ("foo_label: .globl foo_label");
+  return g_B;					/* foo return */
+}						/* foo end */
+
+C
+bar()
+{						/* bar prologue */
+  asm ("bar_label: .globl bar_label");
+  return g_C;					/* bar return */
+}						/* bar end */
+
+int
+main (void)
+{						/* main prologue */
+  B v;
+  C ret;
+  asm ("main_label: .globl main_label");
+
+  v = foo ();					/* main foo call */
+
+  asm ("main_bar_label: .globl main_bar_label");
+  ret = bar();					/* main bar call */
+
+  asm ("main_label2: .globl main_label2");
+  return 0;					/* main return */
+}						/* main end */
diff --git a/gdb/value.c b/gdb/value.c
index 3a2bc13985e..c3f4b263e69 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3046,8 +3046,12 @@ value_primitive_field (struct value *arg1, LONGEST offset,
 
       /* We special case virtual inheritance here because this
 	 requires access to the contents, which we would rather avoid
-	 for references to ordinary fields of unavailable values.  */
-      if (BASETYPE_VIA_VIRTUAL (arg_type, fieldno))
+	 for references to ordinary fields of unavailable values.  
+	 TODO: we should probably try to change this special case, we are
+	 starting to add validation to baseclass_offset.
+	 */
+      if (BASETYPE_VIA_VIRTUAL (arg_type, fieldno) || 
+	  TYPE_FIELD_LOC_KIND(arg_type, fieldno) != FIELD_LOC_KIND_BITPOS)
 	boffset = baseclass_offset (arg_type, fieldno,
 				    value_contents (arg1),
 				    value_embedded_offset (arg1),
-- 
2.27.0