[RFA] struct context moved

[RFA] struct context moved

[Michal Ludvig]

[-- Attachment #1: Type: text/plain, Size: 427 bytes --]

Hi all,
the appended patch moved declaration of struct context and struct 
context_reg to frame.h, where these two are used in struct frame_info.
OK to commit?

2002-07-08  Michal Ludvig  <mludvig@suse.cz>

	* dwarf2cfi.c (struct context, struct context_reg): Moved...
	* farme.h (struct context, struct context_reg): ...here.

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* +420 2 9654 5373   * http://www.suse.cz

[-- Attachment #2: context-1.diff --]
[-- Type: text/plain, Size: 1776 bytes --]

Index: dwarf2cfi.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2cfi.c,v
retrieving revision 1.12
diff -u -p -r1.12 dwarf2cfi.c
--- dwarf2cfi.c	4 Jul 2002 14:43:51 -0000	1.12
+++ dwarf2cfi.c	8 Jul 2002 14:24:42 -0000
@@ -88,37 +88,6 @@ struct fde_array
   int array_size;
 };
 
-struct context_reg
-{
-  union
-  {
-    unsigned int reg;
-    long offset;
-    CORE_ADDR addr;
-  }
-  loc;
-  enum
-  {
-    REG_CTX_UNSAVED,
-    REG_CTX_SAVED_OFFSET,
-    REG_CTX_SAVED_REG,
-    REG_CTX_SAVED_ADDR,
-    REG_CTX_VALUE,
-  }
-  how;
-};
-
-/* This is the register and unwind state for a particular frame.  */
-struct context
-{
-  struct context_reg *reg;
-
-  CORE_ADDR cfa;
-  CORE_ADDR ra;
-  void *lsda;
-  int args_size;
-};
-
 struct frame_state_reg
 {
   union
Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.22
diff -u -p -r1.22 frame.h
--- frame.h	2 Jul 2002 19:08:53 -0000	1.22
+++ frame.h	8 Jul 2002 14:24:42 -0000
@@ -64,6 +64,37 @@ struct frame_saved_regs
   };
 #endif
 
+struct context_reg
+{
+  union
+  {
+    unsigned int reg;
+    long offset;
+    CORE_ADDR addr;
+  }
+  loc;
+  enum
+  {
+    REG_CTX_UNSAVED,
+    REG_CTX_SAVED_OFFSET,
+    REG_CTX_SAVED_REG,
+    REG_CTX_SAVED_ADDR,
+    REG_CTX_VALUE,
+  }
+  how;
+};
+
+/* This is the register and unwind state for a particular frame.  */
+struct context
+{
+  struct context_reg *reg;
+
+  CORE_ADDR cfa;
+  CORE_ADDR ra;
+  void *lsda;
+  int args_size;
+};
+
 /* We keep a cache of stack frames, each of which is a "struct
    frame_info".  The innermost one gets allocated (in
    wait_for_inferior) each time the inferior stops; current_frame

Re: [RFA] struct context moved

[Andrew Cagney]

> Hi all,
> the appended patch moved declaration of struct context and struct context_reg to frame.h, where these two are used in struct frame_info.
> OK to commit?
> 
> 2002-07-08  Michal Ludvig  <mludvig@suse.cz>
> 
>     * dwarf2cfi.c (struct context, struct context_reg): Moved...
>     * farme.h (struct context, struct context_reg): ...here.
> 
> Michal Ludvig 

Michael,

I don't understand the rationale behind this.

As a general trend, frame.h / struct frame is becomming increasingly 
opaque.  I don't see a reason for moving dwarf2cfi specific stuff into 
frame.h.

Can I also suggest looking at the unwind functions - they take an 
unwind_cache parameter that can be used to save unwind specific information.

Andrew

Re: [RFA] struct context moved

[Michal Ludvig]

On Mon, 8 Jul 2002, Andrew Cagney wrote:

> > Hi all,
> > the appended patch moved declaration of struct context and struct context_reg to frame.h, where these two are used in struct frame_info.
> > OK to commit?
> >
> > 2002-07-08  Michal Ludvig  <mludvig@suse.cz>
> >
> >     * dwarf2cfi.c (struct context, struct context_reg): Moved...
> >     * farme.h (struct context, struct context_reg): ...here.
> >
> > Michal Ludvig
>
> Michael,
>
> I don't understand the rationale behind this.
>
> As a general trend, frame.h / struct frame is becomming increasingly
> opaque.  I don't see a reason for moving dwarf2cfi specific stuff into
> frame.h.

Because we have
	struct context *context;
in the declaration of "struct frame_info", I thought it was logical to
declare "struct context" in the same file. Otherwise, when debugging gdb
itself, I'm getting "incomplete type" message when examining the content
of struct frame_info. AFAIK it doesn't increase the size of the code, it
just gives to debugger the appropriate information about the type of
the structure.

Or am I wrong?

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* +420 2 9654 5373   * http://www.suse.cz

Re: [RFA] struct context moved

[Andrew Cagney]

>> Michael,
>>
>> I don't understand the rationale behind this.
>>
>> As a general trend, frame.h / struct frame is becomming increasingly
>> opaque.  I don't see a reason for moving dwarf2cfi specific stuff into
>> frame.h.
> 
> 
> Because we have
> 	struct context *context;
> in the declaration of "struct frame_info", I thought it was logical to
> declare "struct context" in the same file. Otherwise, when debugging gdb
> itself, I'm getting "incomplete type" message when examining the content
> of struct frame_info. AFAIK it doesn't increase the size of the code, it
> just gives to debugger the appropriate information about the type of
> the structure.

Ah!

GDB is going in the direction of reducing the information visible in 
header files.  ``struct gdbarch'', ``struct cmd_list_element'' and 
``struct regcache'', for instance, are all opaque.

Consequently, and very likely, ``struct frame_info'' will eventually 
become opaque.

This approach puts very strong controls over how an object can be 
accessed.  In the past GDB has relied on convention (macros) and code 
review as a way of ensuring this this is followed.  Unfortunatly, as an 
examination of the code reveals, this has proven unreliable.

As a very up-to-date example of the problem, consider the fixes Martin 
Hunt recently committed (added cmd_func()).  GDB, for some time has been 
doing the very non-portable equivalent of:

	void f (int, float);
	(void (*)(int, float, void *)) f (1, 1.0, &foo);

(assming I've got my C right :-) This was fixed in core GDB, 
Ufortunatly, because Insight (which uses GDB) was still accessing 
``struct cmd_list_element'' internals, it was missed, and that lead to 
the unintentional introduction of a core dump.  If the ``struct 
cmd_list_element'' had always been opaque, the internals change needed 
to fix this could have been implemented more robustly - the probably of 
the changes causing insight to crash would have been significantly reduced.

Anyway, I'm some what puzzled by your problem.  I suspect a stabs vs 
dwarf2 thing :-(  When I debug, I have no problems displaying things 
like ``struct gdbarch'' which is opaque.  The only other wild guess is 
to add the opaque declaration of ``struct context;'' to the top of 
"frame.h"??

enjoy,
Andrew

Re: [RFA] struct context moved

[Michael Snyder]

Michal Ludvig wrote:
> 
> On Mon, 8 Jul 2002, Andrew Cagney wrote:
> 
> > > Hi all,
> > > the appended patch moved declaration of struct context and struct context_reg to frame.h, where these two are used in struct frame_info.
> > > OK to commit?
> > >
> > > 2002-07-08  Michal Ludvig  <mludvig@suse.cz>
> > >
> > >     * dwarf2cfi.c (struct context, struct context_reg): Moved...
> > >     * farme.h (struct context, struct context_reg): ...here.
> > >
> > > Michal Ludvig
> >
> > Michael,
> >
> > I don't understand the rationale behind this.
> >
> > As a general trend, frame.h / struct frame is becomming increasingly
> > opaque.  I don't see a reason for moving dwarf2cfi specific stuff into
> > frame.h.
> 
> Because we have
>         struct context *context;
> in the declaration of "struct frame_info", I thought it was logical to
> declare "struct context" in the same file. Otherwise, when debugging gdb
> itself, I'm getting "incomplete type" message when examining the content
> of struct frame_info. AFAIK it doesn't increase the size of the code, it
> just gives to debugger the appropriate information about the type of
> the structure.
> 
> Or am I wrong?

I don't know if this thread is still alive, but 
this change certainly seems wrong to me.  Most of GDB
does not need to know about these structures.  I assume
their use is strictly contained within dwarf2cfi.