[RFA] Reference value coercion

[Paul Hilfinger <hilfingr at gnat dot com>]

Consider the following situation in C++:

    class Parent { ... };
    class Child : public Parent { ... };

    int f1 (Parent& P) { ... }

    int f2 (Child& C) { 
        // BP here
    }

where GDB is stopped at the point of the comment.  On (at least)
Linux with the most current GDB sources, the command

    (gdb) print f1(C)

at this point will eventually cause a segfault when f1 attempts to access
a field of its argument.  This is because at the moment, since the type
of C and the formal, P, differ, value_cast dereferences C.  This is
a VERY bad idea generally in C++, for other reasons, but what's worse is
that not only do we dereference C, but we then proceed to pass the 
dereferenced value to f1 as if it were the address.  

The simple patch below to valops.c seems to solve this problem.  However,
although I have presented this as a C++ problem, I naturally discovered it
as an Ada problem, and it would be particularly good for a C++ person to
consider whether other related changes to value_cast are in order.  

The patch also contains a suggested addition to the C++ testsuite.

Comments?

Paul Hilfinger


Changelog:

2006-03-31  Paul N. Hilfinger <Hilfinger@adacore.com>

	* valops.c (value_cast): If first argument is a reference type, do not
	apply coerce_ref to the second.

Testsuite Changelog:

2006-03-31  Paul N. Hilfinger <Hilfinger@adacore.com>

	* gdb.cp/ref-params.exp: New test.
	* gdb.cp/ref-params.cc: New source file.
	* gdb.cp/Makefile.in (EXECUTABLES): Add ref-params.

Index: current-public.148/gdb/valops.c
--- current-public.148/gdb/valops.c Wed, 21 Dec 2005 02:40:29 -0800 hilfingr (GdbPub/l/17_valops.c 1.1.1.24 644)
+++ submit.118(w)/gdb/valops.c Thu, 30 Mar 2006 23:54:34 -0800 hilfingr (GdbPub/l/17_valops.c 1.1.1.24.1.1 644)
@@ -221,7 +221,8 @@ value_cast (struct type *type, struct va
 
   CHECK_TYPEDEF (type);
   code1 = TYPE_CODE (type);
-  arg2 = coerce_ref (arg2);
+  if (code1 != TYPE_CODE_REF)
+    arg2 = coerce_ref (arg2);
   type2 = check_typedef (value_type (arg2));
 
   /* A cast to an undetermined-length array_type, such as (TYPE [])OBJECT,


Index: current-public.148/gdb/testsuite/gdb.cp/Makefile.in
--- current-public.148/gdb/testsuite/gdb.cp/Makefile.in Tue, 17 Jun 2003 02:27:41 -0700 hilfingr (GdbPub/J/36_Makefile.i 1.2 644)
+++ submit.118(w)/gdb/testsuite/gdb.cp/Makefile.in Thu, 30 Mar 2006 03:01:18 -0800 hilfingr (GdbPub/J/36_Makefile.i 1.3 644)
@@ -3,7 +3,8 @@ srcdir = @srcdir@
 
 EXECUTABLES = ambiguous annota2 anon-union cplusfuncs cttiadd \
 	derivation inherit local member-ptr method misc \
-        overload ovldbreak ref-typ ref-typ2 templates userdef virtfunc namespace ref-types
+        overload ovldbreak ref-typ ref-typ2 templates userdef virtfunc namespace \
+	ref-types ref-params
 
 all info install-info dvi install uninstall installcheck check:
 	@echo "Nothing to be done for $@..."
Index: current-public.148/gdb/testsuite/gdb.cp/ref-params.exp
--- current-public.148/gdb/testsuite/gdb.cp/ref-params.exp Fri, 31 Mar 2006 02:25:08 -0800 hilfingr ()
+++ submit.118(w)/gdb/testsuite/gdb.cp/ref-params.exp Thu, 30 Mar 2006 23:53:44 -0800 hilfingr (GdbPub/E/c/40_ref-params 1.1 644)
@@ -0,0 +1,93 @@
+# Tests for reference parameters of types and their subtypes in GDB.
+# Copyright 2006 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 2 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, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+
+# written by Paul N. Hilfinger (Hilfinger@adacore.com)
+
+if $tracelevel then {
+        strace $tracelevel
+        }
+
+#
+# test running programs
+#
+set prms_id 0
+set bug_id 0
+
+if { [skip_cplus_tests] } { continue }
+
+set testfile "ref-params"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+     gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+proc get_to_mark { marker func } {
+    #
+    # set it up at a breakpoint so we can play with the variable values
+    #
+    if ![runto_main] then {
+	perror "couldn't run to breakpoint"
+	continue
+    }
+
+    if ![runto ${marker}] then {
+	perror "couldn't run to ${marker}"
+	continue
+    }
+
+    gdb_test "up" ".* ${func} .*" "up from ${marker}"
+}
+
+proc gdb_start_again { marker func } {
+    global srcdir
+    global subdir
+    global binfile
+    global gdb_prompt
+    global decimal
+
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load ${binfile}
+
+    get_to_mark ${marker} ${func}
+}
+
+get_to_mark marker1 main
+gdb_test "print Q" ".*id = 42.*" "print value of a Child in main"
+gdb_test "print f1(Q)" ".* = 42.*" "print value of f1 on Child in main"
+gdb_test "print f2(Q)" ".* = 42.*" "print value of f2 on Child in main"
+
+gdb_start_again marker1 main
+gdb_test "print f1(QR)" ".* = 42.*" "print value of f1 on (Child&) in main"
+
+gdb_start_again marker1 main
+gdb_test "print f2(QR)" ".* = 42.*" "print value of f2 on (Child&) in main"
+
+gdb_start_again marker2 f2
+gdb_test "print C" ".*id = 42.*" "print value of Child& in f2"
+gdb_test "print f1(C)" ".* = 42.*" "print value of f1 on Child& in f2"
+
+gdb_start_again marker3 f1
+gdb_test "print R" ".*id = 42.*" "print value of Parent& in f1"
+
Index: current-public.148/gdb/testsuite/gdb.cp/ref-params.cc
--- current-public.148/gdb/testsuite/gdb.cp/ref-params.cc Fri, 31 Mar 2006 02:25:08 -0800 hilfingr ()
+++ submit.118(w)/gdb/testsuite/gdb.cp/ref-params.cc Fri, 31 Mar 2006 02:24:09 -0800 hilfingr (GdbPub/E/c/41_ref-params 1.1 644)
@@ -0,0 +1,73 @@
+/* This test script is part of GDB, the GNU debugger.
+
+   Copyright 2006
+   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 2 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, write to the Free Software
+   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+   */
+
+/* Author: Paul N. Hilfinger, AdaCore Inc. */
+
+struct Parent {
+  Parent (int id0) : id(id0) { }
+  int id;
+};
+
+struct Child : public Parent {
+  Child (int id0) : Parent(id0) { }
+};
+
+void marker1 (void)
+{
+
+}
+
+void marker2 (void)
+{
+
+}
+
+void marker3 (void)
+{
+
+}
+
+int f1(Parent& R)
+{
+  marker3();
+  return R.id;
+}
+
+int f2(Child& C)
+{
+  marker2();
+  return f1(C);
+}
+
+int main(void) 
+{
+  Child Q(42);
+  Child& QR = Q;
+
+  #ifdef usestubs
+     set_debug_traps();
+     breakpoint();
+  #endif
+  marker1();
+
+  f2(Q);
+  f2(QR);
+}
+