[v2] Fixed core dump from incorrect location expression on bad dwarfs

Message ID 20210830201045.28139-1-blarsen@redhat.com
State New
Headers show
Series
  • [v2] Fixed core dump from incorrect location expression on bad dwarfs
Related show

Commit Message

Tom de Vries via Gdb-patches Aug. 30, 2021, 8:10 p.m.
Some incorrectly constructed location expressions in inheritance members
of a class could lead to a core dump, or printing garbage instead of a
correct value. The added test case always core dumped during my
testing, but it could be changed to print garbage by changing the
location expression on the exp file to not include DW_OP_stack_value,
but just use a large constant value.

The solution is, when copying contents of a value struct, check if
contents will actually be copied (ie length > 0) and if the
offset of the copied member is greater than the size of the struct
itself, raising an error if so.
---

Changes for V2:
* fixed compilation issue, brought up by ahajkova

---
 .../dw2-inheritance-stack-location.c          |  50 +++++
 .../dw2-inheritance-stack-location.exp        | 185 ++++++++++++++++++
 gdb/value.c                                   |   5 +
 3 files changed, 240 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inheritance-stack-location.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inheritance-stack-location.exp

-- 
2.27.0

Patch

diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inheritance-stack-location.c b/gdb/testsuite/gdb.dwarf2/dw2-inheritance-stack-location.c
new file mode 100644
index 00000000000..ef661af9d92
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inheritance-stack-location.c
@@ -0,0 +1,50 @@ 
+/* 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 x1;
+    long b;
+} B;
+
+struct B g_B = { 8, 9, 10 };
+
+B
+foo ()
+{						/* foo prologue */
+  asm ("foo_label: .globl foo_label");
+  return g_B;					/* foo return */
+}						/* foo end */
+
+int
+main (void)
+{						/* main prologue */
+  B v;
+  asm ("main_label: .globl main_label");
+
+  v = foo ();					/* main foo call */
+
+  asm ("main_label2: .globl main_label2");
+  return 0;					/* main return */
+}						/* main end */
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-inheritance-stack-location.exp b/gdb/testsuite/gdb.dwarf2/dw2-inheritance-stack-location.exp
new file mode 100644
index 00000000000..5b8aba3ef6c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inheritance-stack-location.exp
@@ -0,0 +1,185 @@ 
+# 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 .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]
+
+# Create the DWARF.
+Dwarf::assemble ${asm_file} {
+    global srcdir subdir srcfile srcfile2
+    global long_size addr_size struct_A_size struct_B_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
+
+	    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 "a"}
+		    {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_subprogram {
+		{MACRO_AT_func { "foo" }}
+		{DW_AT_type :${class_B_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"
+
+	# 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_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}
+
+	}
+    }
+}
+
+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 = <error reading variable>" "finish out of foo"
+
+if ![runto_main] {
+    return -1
+}
diff --git a/gdb/value.c b/gdb/value.c
index 2cbaadc3641..4de3074157a 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1328,6 +1328,11 @@  value_contents_copy_raw (struct value *dst, LONGEST dst_offset,
 					     TARGET_CHAR_BIT * dst_offset,
 					     TARGET_CHAR_BIT * length));
 
+  if ((length > 0)
+      && (dst_offset >= TYPE_LENGTH (value_enclosing_type (dst))
+      || src_offset >= TYPE_LENGTH (value_enclosing_type (src))))
+      error (_("overly large offset found while copying value."));
+
   /* Copy the data.  */
   memcpy (value_contents_all_raw (dst) + dst_offset * unit_size,
 	  value_contents_all_raw (src) + src_offset * unit_size,