Re: [RFA]: Fix gdb.base/remote for embedded targets (HC11)

Re: [RFA]: Fix gdb.base/remote for embedded targets (HC11)

[Andrew Cagney]

> Hi!
> 
> The gdb.base/remote test fails for 68HC11/68HC12:
> 
>   - The gdb.base/remote.c defines a data table that is too large to link for
>     68HC11 targets (it requires 48K of data).  
> 
>   - For HC11/HC12, the data section is installed by the crt0.  The data section
>     has its image in ROM and it is copied to RAM by the startup code.  With a
>     48K data table, it requires 96K of memory (48K rom, 48K ram).
> 
>     After the load, the data table is not initialized.  It is initialized when
>     we reached the 'main'.
> 
> The following patch improves the gdb.base/remote test:
> 
>   - The remote.c data table is reduced to 1K for 68HC11 (only), and the
>     test verifies only the 1K load for 68HC11 (other platforms still use 48K).
> 
>   - It checks for a new 'gdb,noloaddata' configuration variable and
>     runs to the main before verifying the load.
> 
> Can you approve this patch?
> 
> Thanks,
> 	Stephane
> 
> 2001-05-20  Stephane Carrez  <Stephane.Carrez@worldnet.fr>
> 
> * gdb.base/remote.c (RANDOM_DATA_SIZE): New define, defaults to 48K 
> 	and defined to 1K for m68hc11.
> 	(random_data): Reduce table to 1K for embedded platforms (68hc11).
> 	* gdb.base/remote.exp (remote_data_size): New variable to tell the size
> 	of the data table; don't test past this size; check for gdb,noloaddata
> 	and run to the main if it is set (data section installed by crt0).


This problem definitly needs to be fixed.  remote.c is currently very 
16bit unfriendly.  A few thoughts - can your change be simplified slightly:

o 
Always run-to-main?
	I don't think it would do any
	harm to always do this

o 
can GDB extract the array size from the target?
	See sizeof.exp:get_sizeof()

o 
as for remote.c:RANDOM_DATA_SIZE
	I can't think of another way of
	doing this.  Anyone?

	Andrew

Re: [RFA]: Fix gdb.base/remote for embedded targets (HC11)

[Stephane Carrez]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 7444 bytes --]

Hi!

Andrew Cagney a écrit :
> 
> > Hi!
> >
> > The gdb.base/remote test fails for 68HC11/68HC12:
> >
> >   - The gdb.base/remote.c defines a data table that is too large to link for
> >     68HC11 targets (it requires 48K of data).
> >
> >   - For HC11/HC12, the data section is installed by the crt0.  The data section
> >     has its image in ROM and it is copied to RAM by the startup code.  With a
> >     48K data table, it requires 96K of memory (48K rom, 48K ram).
> >
> >     After the load, the data table is not initialized.  It is initialized when
> >     we reached the 'main'.
> >
> > The following patch improves the gdb.base/remote test:
> >
> >   - The remote.c data table is reduced to 1K for 68HC11 (only), and the
> >     test verifies only the 1K load for 68HC11 (other platforms still use 48K).
> >
> >   - It checks for a new 'gdb,noloaddata' configuration variable and
> >     runs to the main before verifying the load.
> >
> > Can you approve this patch?
> >
> > Thanks,
> >       Stephane
> >
> > 2001-05-20  Stephane Carrez  <Stephane.Carrez@worldnet.fr>
> >
> > * gdb.base/remote.c (RANDOM_DATA_SIZE): New define, defaults to 48K
> >       and defined to 1K for m68hc11.
> >       (random_data): Reduce table to 1K for embedded platforms (68hc11).
> >       * gdb.base/remote.exp (remote_data_size): New variable to tell the size
> >       of the data table; don't test past this size; check for gdb,noloaddata
> >       and run to the main if it is set (data section installed by crt0).
> 
> This problem definitly needs to be fixed.  remote.c is currently very
> 16bit unfriendly.  A few thoughts - can your change be simplified slightly:
> 
> o
> Always run-to-main?
>         I don't think it would do any
>         harm to always do this
> 
Ok

> o
> can GDB extract the array size from the target?
>         See sizeof.exp:get_sizeof()
> 
ok

> o
> as for remote.c:RANDOM_DATA_SIZE
>         I can't think of another way of
>         doing this.  Anyone?
> 
>         Andrew

The patch below follows your recommendations.

Is it ok?

	Stephane

2001-07-16  Stephane Carrez  <Stephane.Carrez@worldnet.fr>

	* gdb.base/remote.c (RANDOM_DATA_SIZE): New define, defaults to 48K 
	and defined to 1K for m68hc11.
	(random_data): Reduce table to 1K for embedded platforms (68hc11).
	* gdb.base/remote.exp (get_sizeof): New function from sizeof.exp.
	(sizeof_random_data): New variable to tell the size of the data table;
	don't test past this size; always run to main.
Index: gdb.base/remote.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/remote.exp,v
retrieving revision 1.2
diff -u -p -r1.2 remote.exp
--- remote.exp	2001/03/06 08:21:51	1.2
+++ remote.exp	2001/07/17 18:33:57
@@ -1,4 +1,4 @@
-#   Copyright 1999 Free Software Foundation, Inc.
+#   Copyright 1999, 2001 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
@@ -30,7 +30,6 @@ if {! [is_remote target]} {
     return
 }
 
-
 set testfile "remote"
 set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
@@ -138,10 +137,37 @@ gdb_load_timed $binfile 0 "fixed" 16385
 # fall back to the default
 gdb_load_timed $binfile 0 "limit" 0
 
+# Get size of data uploaded
 
 #
+# Query GDB for the size of various types
+#
+
+proc get_sizeof { type default } {
+    global gdb_prompt
+    send_gdb "print/d sizeof (${type})\n"
+    gdb_expect {
+	-re "\\$\[0-9\]* = (\[0-9\]*).*$gdb_prompt $" {
+	    set size $expect_out(1,string)
+	    pass "get sizeof ${type} ($size)"
+	}
+	timeout {
+	    set size ${default}
+	    fail "get sizeof ${type} (timeout)"
+	}
+    }
+    return ${size}
+}
+
+# Get the size of random_data table (defaults to 48K).
+set sizeof_random_data [get_sizeof "random_data" 48*1024]
+
+#
 # Part THREE: Check the upload behavour
 #
+if ![runto_main] then {
+    fail "Cannot run to main"
+}
 
 # Carefully check memory around each of the most common packet edge
 # conditions
@@ -152,9 +178,10 @@ gdb_test "x/8ub random_data" \
 gdb_test "x/8ub random_data + 400 - 4" \
 	"<random_data\\+396>:\[ \t\]+185\[ \t\]+255\[ \t\]+50\[ \t\]+140\[ \t\]+237\[ \t\]+172\[ \t\]+143\[ \t\]+93"
 
-gdb_test "x/8ub random_data + 16384 - 4" \
+if {$sizeof_random_data > 16380 } then {
+    gdb_test "x/8ub random_data + 16384 - 4" \
 	"<random_data\\+16380>:\[ \t\]+178\[ \t\]+180\[ \t\]+135\[ \t\]+93\[ \t\]+70\[ \t\]+62\[ \t\]+205\[ \t\]+76"
-
+}
 
 # Read a chunk just larger than the packet size (reduce the packet
 # size to make life easier)
Index: gdb.base/remote.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/remote.c,v
retrieving revision 1.1.1.2
diff -u -p -r1.1.1.2 remote.c
--- remote.c	1999/12/07 03:56:16	1.1.1.2
+++ remote.c	2001/07/17 18:34:30
@@ -23,12 +23,21 @@ BEGIN {
 
 */
 
+#ifdef mc68hc11
+# define RANDOM_DATA_SIZE (1024)
+#endif
+
 /* Use a character buffer to avoid byte order problems. 48k is
    chosen so that the buffer required at least 3 16k packets but
    targets often have no more than 64k of data. */
 /* If you change this data, you will also have to change the checks
    for the data in remote.c */
-unsigned char random_data[3 * 2048 * 8] = {
+#ifndef RANDOM_DATA_SIZE
+# define RANDOM_DATA_SIZE (3 * 2048 * 8)
+# define BIG_RANDOM_DATA
+#endif
+
+unsigned char random_data[RANDOM_DATA_SIZE] = {
   60,  74, 216,  38, 149,  49, 207,  44,
  124,  38,  93, 125, 232,  67, 228,  56,
  161, 146,  85,  26, 128, 145, 218,  10,
@@ -157,6 +166,7 @@ unsigned char random_data[3 * 2048 * 8] 
  196,  13, 161, 122, 145, 141, 102, 233,
  227, 112, 121,  67, 111, 148, 160,  32,
  199, 117, 223, 105, 184, 131, 119, 182,
+#ifdef BIG_RANDOM_DATA
   60,  26, 169, 194, 173, 164, 249, 135,
  178,  57,  50,  44,  12, 159, 167, 240,
  249, 188,  86, 192,  73,  47,  74,  77,
@@ -6173,6 +6183,7 @@ unsigned char random_data[3 * 2048 * 8] 
   97, 106, 152,  12, 243, 240,  41, 251,
   35, 249, 105, 228,  53,  94,  43, 119,
   61, 162, 192,  78,  58,  46,  84, 110,
+#endif
 };
 
 int
From Stephane.Carrez@worldnet.fr Tue Jul 17 11:42:00 2001
From: Stephane Carrez <Stephane.Carrez@worldnet.fr>
To: Elena Zannoni <ezannoni@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA]: Cleanup tui/tui-file.[ch]
Date: Tue, 17 Jul 2001 11:42:00 -0000
Message-id: <3B548825.773525AC@worldnet.fr>
References: <3B536421.C4F0E04D@worldnet.fr> <15188.29985.447487.713030@krustylu.cygnus.com>
X-SW-Source: 2001-07/msg00428.html
Content-length: 798

Hi!

Elena Zannoni a écrit :
> 
> Stephane,
> I am getting a warning in tui-file.c (I am building with -Werror) about
> tuiPuts_unfiltered being implicitly declared.

Ah!!!  The marvellous -Werror that I'm currently figthing with in ChorusOS
to remove all our warnings..... ... ...

> I think it is because you removed the #include "tuiIO.h".
> 
> Can I put it back?

Sure!

	Stephane

-----------------------------------------------------------------------
         Home                               Office
E-mail: stcarrez@worldnet.fr               Stephane.Carrez@sun.com
WWW:    http://home.worldnet.fr/stcarrez   http://www.sun.com
Mail:   17, rue Foucher Lepelletier        6, avenue Gustave Eiffel
        92130 Issy Les Moulineaux          78182 Saint Quentin en Yvelines
        France

Re: [RFA]: Fix gdb.base/remote for embedded targets (HC11)

[Fernando Nasser]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 7831 bytes --]

Stephane Carrez wrote:
> 
> Hi!
> 
> Andrew Cagney a écrit :
> >
> > > Hi!
> > >
> > > The gdb.base/remote test fails for 68HC11/68HC12:
> > >
> > >   - The gdb.base/remote.c defines a data table that is too large to link for
> > >     68HC11 targets (it requires 48K of data).
> > >
> > >   - For HC11/HC12, the data section is installed by the crt0.  The data section
> > >     has its image in ROM and it is copied to RAM by the startup code.  With a
> > >     48K data table, it requires 96K of memory (48K rom, 48K ram).
> > >
> > >     After the load, the data table is not initialized.  It is initialized when
> > >     we reached the 'main'.
> > >
> > > The following patch improves the gdb.base/remote test:
> > >
> > >   - The remote.c data table is reduced to 1K for 68HC11 (only), and the
> > >     test verifies only the 1K load for 68HC11 (other platforms still use 48K).
> > >
> > >   - It checks for a new 'gdb,noloaddata' configuration variable and
> > >     runs to the main before verifying the load.
> > >
> > > Can you approve this patch?
> > >
> > > Thanks,
> > >       Stephane
> > >
> > > 2001-05-20  Stephane Carrez  <Stephane.Carrez@worldnet.fr>
> > >
> > > * gdb.base/remote.c (RANDOM_DATA_SIZE): New define, defaults to 48K
> > >       and defined to 1K for m68hc11.
> > >       (random_data): Reduce table to 1K for embedded platforms (68hc11).
> > >       * gdb.base/remote.exp (remote_data_size): New variable to tell the size
> > >       of the data table; don't test past this size; check for gdb,noloaddata
> > >       and run to the main if it is set (data section installed by crt0).
> >
> > This problem definitly needs to be fixed.  remote.c is currently very
> > 16bit unfriendly.  A few thoughts - can your change be simplified slightly:
> >
> > o
> > Always run-to-main?
> >         I don't think it would do any
> >         harm to always do this
> >
> Ok
> 
> > o
> > can GDB extract the array size from the target?
> >         See sizeof.exp:get_sizeof()
> >
> ok
> 
> > o
> > as for remote.c:RANDOM_DATA_SIZE
> >         I can't think of another way of
> >         doing this.  Anyone?
> >
> >         Andrew
> 
> The patch below follows your recommendations.
> 
> Is it ok?
> 
>         Stephane
> 


Approved.

Thanks.




> 2001-07-16  Stephane Carrez  <Stephane.Carrez@worldnet.fr>
> 
>         * gdb.base/remote.c (RANDOM_DATA_SIZE): New define, defaults to 48K
>         and defined to 1K for m68hc11.
>         (random_data): Reduce table to 1K for embedded platforms (68hc11).
>         * gdb.base/remote.exp (get_sizeof): New function from sizeof.exp.
>         (sizeof_random_data): New variable to tell the size of the data table;
>         don't test past this size; always run to main.
> 
>                                                   ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> Index: gdb.base/remote.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/remote.exp,v
> retrieving revision 1.2
> diff -u -p -r1.2 remote.exp
> --- remote.exp  2001/03/06 08:21:51     1.2
> +++ remote.exp  2001/07/17 18:33:57
> @@ -1,4 +1,4 @@
> -#   Copyright 1999 Free Software Foundation, Inc.
> +#   Copyright 1999, 2001 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
> @@ -30,7 +30,6 @@ if {! [is_remote target]} {
>      return
>  }
> 
> -
>  set testfile "remote"
>  set srcfile ${testfile}.c
>  set binfile ${objdir}/${subdir}/${testfile}
> @@ -138,10 +137,37 @@ gdb_load_timed $binfile 0 "fixed" 16385
>  # fall back to the default
>  gdb_load_timed $binfile 0 "limit" 0
> 
> +# Get size of data uploaded
> 
>  #
> +# Query GDB for the size of various types
> +#
> +
> +proc get_sizeof { type default } {
> +    global gdb_prompt
> +    send_gdb "print/d sizeof (${type})\n"
> +    gdb_expect {
> +       -re "\\$\[0-9\]* = (\[0-9\]*).*$gdb_prompt $" {
> +           set size $expect_out(1,string)
> +           pass "get sizeof ${type} ($size)"
> +       }
> +       timeout {
> +           set size ${default}
> +           fail "get sizeof ${type} (timeout)"
> +       }
> +    }
> +    return ${size}
> +}
> +
> +# Get the size of random_data table (defaults to 48K).
> +set sizeof_random_data [get_sizeof "random_data" 48*1024]
> +
> +#
>  # Part THREE: Check the upload behavour
>  #
> +if ![runto_main] then {
> +    fail "Cannot run to main"
> +}
> 
>  # Carefully check memory around each of the most common packet edge
>  # conditions
> @@ -152,9 +178,10 @@ gdb_test "x/8ub random_data" \
>  gdb_test "x/8ub random_data + 400 - 4" \
>         "<random_data\\+396>:\[ \t\]+185\[ \t\]+255\[ \t\]+50\[ \t\]+140\[ \t\]+237\[ \t\]+172\[ \t\]+143\[ \t\]+93"
> 
> -gdb_test "x/8ub random_data + 16384 - 4" \
> +if {$sizeof_random_data > 16380 } then {
> +    gdb_test "x/8ub random_data + 16384 - 4" \
>         "<random_data\\+16380>:\[ \t\]+178\[ \t\]+180\[ \t\]+135\[ \t\]+93\[ \t\]+70\[ \t\]+62\[ \t\]+205\[ \t\]+76"
> -
> +}
> 
>  # Read a chunk just larger than the packet size (reduce the packet
>  # size to make life easier)
> Index: gdb.base/remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/remote.c,v
> retrieving revision 1.1.1.2
> diff -u -p -r1.1.1.2 remote.c
> --- remote.c    1999/12/07 03:56:16     1.1.1.2
> +++ remote.c    2001/07/17 18:34:30
> @@ -23,12 +23,21 @@ BEGIN {
> 
>  */
> 
> +#ifdef mc68hc11
> +# define RANDOM_DATA_SIZE (1024)
> +#endif
> +
>  /* Use a character buffer to avoid byte order problems. 48k is
>     chosen so that the buffer required at least 3 16k packets but
>     targets often have no more than 64k of data. */
>  /* If you change this data, you will also have to change the checks
>     for the data in remote.c */
> -unsigned char random_data[3 * 2048 * 8] = {
> +#ifndef RANDOM_DATA_SIZE
> +# define RANDOM_DATA_SIZE (3 * 2048 * 8)
> +# define BIG_RANDOM_DATA
> +#endif
> +
> +unsigned char random_data[RANDOM_DATA_SIZE] = {
>    60,  74, 216,  38, 149,  49, 207,  44,
>   124,  38,  93, 125, 232,  67, 228,  56,
>   161, 146,  85,  26, 128, 145, 218,  10,
> @@ -157,6 +166,7 @@ unsigned char random_data[3 * 2048 * 8]
>   196,  13, 161, 122, 145, 141, 102, 233,
>   227, 112, 121,  67, 111, 148, 160,  32,
>   199, 117, 223, 105, 184, 131, 119, 182,
> +#ifdef BIG_RANDOM_DATA
>    60,  26, 169, 194, 173, 164, 249, 135,
>   178,  57,  50,  44,  12, 159, 167, 240,
>   249, 188,  86, 192,  73,  47,  74,  77,
> @@ -6173,6 +6183,7 @@ unsigned char random_data[3 * 2048 * 8]
>    97, 106, 152,  12, 243, 240,  41, 251,
>    35, 249, 105, 228,  53,  94,  43, 119,
>    61, 162, 192,  78,  58,  46,  84, 110,
> +#endif
>  };
> 
>  int

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9