[RFA] Fix display of array of unspecified length inside structures

[RFA] Fix display of array of unspecified length inside structures

[Pierre Muller]

The following code shows a
problem in current display of 
char arrays of zero length,
the embedded_offset parameter in c_val_print
gets forgotten, resulting in wrong display.



typedef struct test_struct {
  int x,y;
  char name[0];
} test_t;

test_t * test;

#define TESTNAME "dummy test"

int
main ()
{
  test = alloca (sizeof (test_t) + sizeof (TESTNAME) + 1);
  test->x = 7;
  test->y = -6;
  strcpy (test->name, TESTNAME);

  return 0;
}

  I had to debug gdb using 'set print infrun 1'
to understand that the value of the name filed was 
read at the address of test.x, rather than test.name.

The patch below fixes that problem.
Tested on x86_64-unknown-linux-gnu
no regression found.
  c_val_print and p_val_print functions
are the two only occurrences of this print_unpacked_pointer 'goto',
but I am not sure if this problem can also 
arise for other languages.

  When you look at the source,
you will see that just before the 
print_unpacked_pointer label,
addr is calculated with the use of embedded_offset.




Pierre Muller
GDB pascal language maintainer

PS: It could be wise to add some test in the testsuite for
this, but I have no idea where I could insert this kind of test,
any ideas?
PS2: It is probably impossible to make such a test without
alloca or some other memory allocation function, no?
Are there any system restriction for this?


2011-02-18  Pierre Muller  <muller@ics.u-strasbg.fr>

	* c-valprint.c (c_val_print): Add embedded_offset to address
	for arrays of unspecified length.
	* p-valprint.c (pascal_val_print): Likewise.

Index: src/gdb/c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.85
diff -u -p -r1.85 c-valprint.c
--- src/gdb/c-valprint.c	14 Feb 2011 11:33:24 -0000	1.85
+++ src/gdb/c-valprint.c	18 Feb 2011 10:27:41 -0000
@@ -240,7 +240,7 @@ c_val_print (struct type *type, const gd
 	}
       /* Array of unspecified length: treat like pointer to first
 	 elt.  */
-      addr = address;
+      addr = address + embedded_offset;
       goto print_unpacked_pointer;
 
     case TYPE_CODE_MEMBERPTR:
Index: src/gdb/p-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/p-valprint.c,v
retrieving revision 1.87
diff -u -p -r1.87 p-valprint.c
--- src/gdb/p-valprint.c	14 Feb 2011 11:35:45 -0000	1.87
+++ src/gdb/p-valprint.c	18 Feb 2011 10:27:41 -0000
@@ -128,7 +128,7 @@ pascal_val_print (struct type *type, con
 	  break;
 	}
       /* Array of unspecified length: treat like pointer to first elt.  */
-      addr = address;
+      addr = address + embedded_offset;
       goto print_unpacked_pointer;
 
     case TYPE_CODE_PTR:

Re: [RFA] Fix display of array of unspecified length inside structures

[Pedro Alves]

On Friday 18 February 2011 11:08:38, Pierre Muller wrote:

> PS: It could be wise to add some test in the testsuite for
> this, but I have no idea where I could insert this kind of test,
> any ideas?

Yes, please.  We have surprisingly few tests for this sort of
thing, AFAICS.  I'm not even sure this is a regression from
my recent changes, I think it may well not be.

Zero-length arrays (as poor man's flexible arrays) are supported
in GNU C as an extension.  To be portable, you'd
need to use an array of length 1 (or c99's real flexible arrays),
but that won't trigger the bug.
I'd point at printcmds.exp, but I'm not sure if there are compilers
out there that choke on the construct...  There's always a
new test file option...  

> PS2: It is probably impossible to make such a test without
> alloca or some other memory allocation function, no?
> Are there any system restriction for this?

Should be fine.

> 2011-02-18  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* c-valprint.c (c_val_print): Add embedded_offset to address
> 	for arrays of unspecified length.
> 	* p-valprint.c (pascal_val_print): Likewise.

Okay, thanks.

-- 
Pedro Alves

RE: [RFA] Fix display of array of unspecified length inside structures

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : vendredi 18 février 2011 12:48
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFA] Fix display of array of unspecified length inside
> structures
> 
> On Friday 18 February 2011 11:08:38, Pierre Muller wrote:
> 
> > PS: It could be wise to add some test in the testsuite for
> > this, but I have no idea where I could insert this kind of test,
> > any ideas?
> 
> Yes, please.  We have surprisingly few tests for this sort of
> thing, AFAICS.  I'm not even sure this is a regression from
> my recent changes, I think it may well not be.

  I checked out gdb version 7.2 shows this regression,
as compared to Cygwin 6.8 at least...
Which means that the regression is not really recent.

  This might means that we should also merge this patch to
the branch, no?
 
> Zero-length arrays (as poor man's flexible arrays) are supported
> in GNU C as an extension.  To be portable, you'd
> need to use an array of length 1 (or c99's real flexible arrays),
> but that won't trigger the bug.
  Apparently there is also the flexible array member construct
see
http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Zero-Length.html

  
> I'd point at printcmds.exp, but I'm not sure if there are compilers
> out there that choke on the construct...  There's always a
> new test file option...
> 
> > PS2: It is probably impossible to make such a test without
> > alloca or some other memory allocation function, no?
> > Are there any system restriction for this?

  There is a long check at start of gdb.base/funcargs.c
but it might just be to really check that alloca really uses
the stack...

 
> > 2011-02-18  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> > 	* c-valprint.c (c_val_print): Add embedded_offset to address
> > 	for arrays of unspecified length.
> > 	* p-valprint.c (pascal_val_print): Likewise.
> 
> Okay, thanks.
  Thanks for the approval,
committed to main branch.

Pierre

Re: [RFA] Fix display of array of unspecified length inside structures

[Pedro Alves]

On Friday 18 February 2011 14:54:03, Pierre Muller wrote:

> > On Friday 18 February 2011 11:08:38, Pierre Muller wrote:
> > 
> > > PS: It could be wise to add some test in the testsuite for
> > > this, but I have no idea where I could insert this kind of test,
> > > any ideas?
> > 
> > Yes, please.  We have surprisingly few tests for this sort of
> > thing, AFAICS.  I'm not even sure this is a regression from
> > my recent changes, I think it may well not be.
> 
>   I checked out gdb version 7.2 shows this regression,
> as compared to Cygwin 6.8 at least...
> Which means that the regression is not really recent.
> 
>   This might means that we should also merge this patch to
> the branch, no?

Sound fine to me.

>  
> > Zero-length arrays (as poor man's flexible arrays) are supported
> > in GNU C as an extension.  To be portable, you'd
> > need to use an array of length 1 (or c99's real flexible arrays),
> > but that won't trigger the bug.
>   Apparently there is also the flexible array member construct
> see
> http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Zero-Length.html

That just confirms what I said.  :-)  The flexible array
member construst is C99 only, so it's likely that other
compilers choke on it by default.

> > I'd point at printcmds.exp, but I'm not sure if there are compilers
> > out there that choke on the construct...  There's always a
> > new test file option...
> > 
> > > PS2: It is probably impossible to make such a test without
> > > alloca or some other memory allocation function, no?
> > > Are there any system restriction for this?
> 
>   There is a long check at start of gdb.base/funcargs.c
> but it might just be to really check that alloca really uses
> the stack...

Irk.  Just use malloc then?  It's not really crutial that
the test runs on all targets/hosts.  As long as it runs
on the targets must people are developing on (GNU/Linux,Windows),
it's fine, we're reasonably well covered.

-- 
Pedro Alves

RE: [RFA] Fix display of array of unspecified length inside structures

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : vendredi 18 février 2011 16:25
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix display of array of unspecified length inside
> structures
> 
> On Friday 18 February 2011 14:54:03, Pierre Muller wrote:
> 
> > > On Friday 18 February 2011 11:08:38, Pierre Muller wrote:
> > >
> > > > PS: It could be wise to add some test in the testsuite for
> > > > this, but I have no idea where I could insert this kind of test,
> > > > any ideas?
> > >
> > > Yes, please.  We have surprisingly few tests for this sort of
> > > thing, AFAICS.  I'm not even sure this is a regression from
> > > my recent changes, I think it may well not be.
> >
> >   I checked out gdb version 7.2 shows this regression,
> > as compared to Cygwin 6.8 at least...
> > Which means that the regression is not really recent.
> >
> >   This might means that we should also merge this patch to
> > the branch, no?
> 
> Sound fine to me.

  Tested, the problem existed on 7.2 branch
and was fixed by the patch, thus I applied
the patch to 7.2 branch.

Pierre

> > > Zero-length arrays (as poor man's flexible arrays) are supported
> > > in GNU C as an extension.  To be portable, you'd
> > > need to use an array of length 1 (or c99's real flexible arrays),
> > > but that won't trigger the bug.
> >   Apparently there is also the flexible array member construct
> > see
> > http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Zero-Length.html
> 
> That just confirms what I said.  :-)  The flexible array
> member construst is C99 only, so it's likely that other
> compilers choke on it by default.

  But this means that other compilers
will at least accept this flexible array construct if
they conform to ISO C99.

 
> > > I'd point at printcmds.exp, but I'm not sure if there are compilers
> > > out there that choke on the construct...  There's always a
> > > new test file option...
> > >
> > > > PS2: It is probably impossible to make such a test without
> > > > alloca or some other memory allocation function, no?
> > > > Are there any system restriction for this?
> >
> >   There is a long check at start of gdb.base/funcargs.c
> > but it might just be to really check that alloca really uses
> > the stack...
> 
> Irk.  Just use malloc then?  It's not really crutial that
> the test runs on all targets/hosts.  As long as it runs
> on the targets must people are developing on (GNU/Linux,Windows),
> it's fine, we're reasonably well covered.

  malloc should be OK,
does it require a header?

Pierre

Re: [RFA] Fix display of array of unspecified length inside structures

[Pedro Alves]

On Friday 18 February 2011 17:21:34, Pierre Muller wrote:
> > That just confirms what I said.  :-)  The flexible array
> > member construst is C99 only, so it's likely that other
> > compilers choke on it by default.
> 
>   But this means that other compilers
> will at least accept this flexible array construct if
> they conform to ISO C99.

Fine.

>   malloc should be OK,
> does it require a header?

stdlib.h.

-- 
Pedro Alves