public inbox for gdb@sourceware.org
 help / color / mirror / Atom feed
* Re: 'char **environ' woes with cygwin
@ 2000-08-25 11:23 Michael Elizabeth Chastain
  2000-08-25 12:40 ` Chris Faylor
  2000-09-25 12:28 ` Andrew Cagney
  0 siblings, 2 replies; 20+ messages in thread
From: Michael Elizabeth Chastain @ 2000-08-25 11:23 UTC (permalink / raw)
  To: cgf, chastain, dj; +Cc: gdb

Ok, here's a patch against sourceware that rips out the "environ" crap.
I haven't even built with it.

After doing the math, the fixed address "&environ" cancels out.  So this
won't make things any worse (or better) for djgpp or other systems where
malloc() does more than increment an sbrk pointer.

Chris, do you think this makes your life easier enough to warrant some
testing and integration into sourceware?

Michael

===

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.626
diff -c -3 -p -r1.626 ChangeLog
*** ChangeLog	2000/08/24 10:48:22	1.626
--- ChangeLog	2000/08/25 18:18:45
***************
*** 1,3 ****
--- 1,16 ----
+ 2000-08-25  Michael Chastain  <chastain@redhat.com>
+ 
+ 	* event-top.c (command_handler): Change space reporting to
+ 	compare sbrk(0) values directly without using &environ.
+ 	* top.c (command_loop): Ditto.
+ 	* main.c (captured_main): Remove initial space reporting message.
+ 	* gdbserver/low-hppabsd.c : Remove extraneous declaration of environ.
+ 	* gdbserver/low-linux.c: Ditto.
+ 	* gdbserver/low-nbsd.c: Ditto.
+ 	* gdbserver/low-sparc.c: Ditto.
+ 	* gdbserver/low-sun3.c: Ditto.
+ 	* inftarg.c: Ditto.
+ 
  2000-08-20  Michael Chastain  <chastain@redhat.com>
  
          * remote.c (read_frame): Handle SERIAL_TIMEOUT while reading
Index: event-top.c
===================================================================
RCS file: /cvs/src/src/gdb/event-top.c,v
retrieving revision 1.8
diff -c -3 -p -r1.8 event-top.c
*** event-top.c	2000/07/30 01:48:25	1.8
--- event-top.c	2000/08/25 18:18:45
*************** command_handler (char *command)
*** 475,481 ****
    struct continuation_arg *arg2;
    long time_at_cmd_start;
  #ifdef HAVE_SBRK
!   long space_at_cmd_start = 0;
  #endif
    extern int display_time;
    extern int display_space;
--- 475,481 ----
    struct continuation_arg *arg2;
    long time_at_cmd_start;
  #ifdef HAVE_SBRK
!   char * sbrk_at_cmd_start = (char *) 0;
  #endif
    extern int display_time;
    extern int display_space;
*************** command_handler (char *command)
*** 505,514 ****
    if (display_space)
      {
  #ifdef HAVE_SBRK
!       extern char **environ;
!       char *lim = (char *) sbrk (0);
! 
!       space_at_cmd_start = (long) (lim - (char *) &environ);
  #endif
      }
  
--- 505,511 ----
    if (display_space)
      {
  #ifdef HAVE_SBRK
!       sbrk_at_cmd_start = (char *) sbrk (0);
  #endif
      }
  
*************** command_handler (char *command)
*** 526,532 ****
        arg1->next = arg2;
        arg2->next = NULL;
        arg1->data.integer = time_at_cmd_start;
!       arg2->data.integer = space_at_cmd_start;
        add_continuation (command_line_handler_continuation, arg1);
      }
  
--- 523,529 ----
        arg1->next = arg2;
        arg2->next = NULL;
        arg1->data.integer = time_at_cmd_start;
!       arg2->data.pointer = sbrk_at_cmd_start;
        add_continuation (command_line_handler_continuation, arg1);
      }
  
*************** command_handler (char *command)
*** 549,563 ****
        if (display_space)
  	{
  #ifdef HAVE_SBRK
! 	  extern char **environ;
! 	  char *lim = (char *) sbrk (0);
! 	  long space_now = lim - (char *) &environ;
! 	  long space_diff = space_now - space_at_cmd_start;
! 
! 	  printf_unfiltered ("Space used: %ld (%c%ld for this command)\n",
! 			     space_now,
! 			     (space_diff >= 0 ? '+' : '-'),
! 			     space_diff);
  #endif
  	}
      }
--- 546,553 ----
        if (display_space)
  	{
  #ifdef HAVE_SBRK
! 	  printf_unfiltered ("Space used for this command: %ld\n",
! 			     sbrk(0) - sbrk_at_cmd_start);
  #endif
  	}
      }
*************** command_line_handler_continuation (struc
*** 573,579 ****
    extern int display_space;
  
    long time_at_cmd_start  = arg->data.longint;
!   long space_at_cmd_start = arg->next->data.longint;
  
    bpstat_do_actions (&stop_bpstat);
    /*do_cleanups (old_chain); *//*?????FIXME????? */
--- 563,569 ----
    extern int display_space;
  
    long time_at_cmd_start  = arg->data.longint;
!   char * sbrk_at_cmd_start = arg->next->data.pointer;
  
    bpstat_do_actions (&stop_bpstat);
    /*do_cleanups (old_chain); *//*?????FIXME????? */
*************** command_line_handler_continuation (struc
*** 588,602 ****
    if (display_space)
      {
  #ifdef HAVE_SBRK
!       extern char **environ;
!       char *lim = (char *) sbrk (0);
!       long space_now = lim - (char *) &environ;
!       long space_diff = space_now - space_at_cmd_start;
! 
!       printf_unfiltered ("Space used: %ld (%c%ld for this command)\n",
! 			 space_now,
! 			 (space_diff >= 0 ? '+' : '-'),
! 			 space_diff);
  #endif
      }
  }
--- 578,585 ----
    if (display_space)
      {
  #ifdef HAVE_SBRK
!       printf_unfiltered ("Space used for this command: %ld\n",
! 			 sbrk(0) - sbrk_at_cmd_start);
  #endif
      }
  }
Index: inftarg.c
===================================================================
RCS file: /cvs/src/src/gdb/inftarg.c,v
retrieving revision 1.4
diff -c -3 -p -r1.4 inftarg.c
*** inftarg.c	2000/07/30 01:48:25	1.4
--- inftarg.c	2000/08/25 18:18:45
*************** int child_thread_alive (int);
*** 91,98 ****
  
  static void init_child_ops (void);
  
- extern char **environ;
- 
  struct target_ops child_ops;
  
  int child_suppress_run = 0;	/* Non-zero if inftarg should pretend not to
--- 91,96 ----
Index: main.c
===================================================================
RCS file: /cvs/src/src/gdb/main.c,v
retrieving revision 1.3
diff -c -3 -p -r1.3 main.c
*** main.c	2000/05/28 01:12:28	1.3
--- main.c	2000/08/25 18:18:45
*************** extern int gdbtk_test (char *);
*** 685,691 ****
  
    END_PROGRESS (argv[0]);
  
!   /* Show time and/or space usage.  */
  
    if (display_time)
      {
--- 685,691 ----
  
    END_PROGRESS (argv[0]);
  
!   /* Show time usage.  */
  
    if (display_time)
      {
*************** extern int gdbtk_test (char *);
*** 693,709 ****
  
        printf_unfiltered ("Startup time: %ld.%06ld\n",
  			 init_time / 1000000, init_time % 1000000);
-     }
- 
-   if (display_space)
-     {
- #ifdef HAVE_SBRK
-       extern char **environ;
-       char *lim = (char *) sbrk (0);
- 
-       printf_unfiltered ("Startup size: data size %ld\n",
- 			 (long) (lim - (char *) &environ));
- #endif
      }
  
    /* The default command loop. 
--- 693,698 ----
Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.17
diff -c -3 -p -r1.17 top.c
*** top.c	2000/08/01 05:06:03	1.17
--- top.c	2000/08/25 18:18:45
*************** command_loop (void)
*** 1559,1565 ****
    int stdin_is_tty = ISATTY (stdin);
    long time_at_cmd_start;
  #ifdef HAVE_SBRK
!   long space_at_cmd_start = 0;
  #endif
    extern int display_time;
    extern int display_space;
--- 1559,1565 ----
    int stdin_is_tty = ISATTY (stdin);
    long time_at_cmd_start;
  #ifdef HAVE_SBRK
!   char * sbrk_at_cmd_start = (char *) 0;
  #endif
    extern int display_time;
    extern int display_space;
*************** command_loop (void)
*** 1601,1610 ****
        if (display_space)
  	{
  #ifdef HAVE_SBRK
! 	  extern char **environ;
! 	  char *lim = (char *) sbrk (0);
! 
! 	  space_at_cmd_start = (long) (lim - (char *) &environ);
  #endif
  	}
  
--- 1601,1607 ----
        if (display_space)
  	{
  #ifdef HAVE_SBRK
! 	  sbrk_at_cmd_start = (char *) sbrk (0);
  #endif
  	}
  
*************** command_loop (void)
*** 1624,1638 ****
        if (display_space)
  	{
  #ifdef HAVE_SBRK
! 	  extern char **environ;
! 	  char *lim = (char *) sbrk (0);
! 	  long space_now = lim - (char *) &environ;
! 	  long space_diff = space_now - space_at_cmd_start;
! 
! 	  printf_unfiltered ("Space used: %ld (%c%ld for this command)\n",
! 			     space_now,
! 			     (space_diff >= 0 ? '+' : '-'),
! 			     space_diff);
  #endif
  	}
      }
--- 1621,1628 ----
        if (display_space)
  	{
  #ifdef HAVE_SBRK
! 	  printf_unfiltered ("Space used for this command: %ld\n",
! 			     sbrk(0) - sbrk_at_cmd_start);
  #endif
  	}
      }
Index: gdbserver/low-hppabsd.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/low-hppabsd.c,v
retrieving revision 1.2
diff -c -3 -p -r1.2 low-hppabsd.c
*** low-hppabsd.c	2000/07/30 01:48:28	1.2
--- low-hppabsd.c	2000/08/25 18:18:45
*************** char buf2[MAX_REGISTER_RAW_SIZE];
*** 47,53 ****
  #include <sys/ptrace.h>
  #include <machine/reg.h>
  
- extern char **environ;
  extern int errno;
  extern int inferior_pid;
  void quit (), perror_with_name ();
--- 47,52 ----
Index: gdbserver/low-linux.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/low-linux.c,v
retrieving revision 1.4
diff -c -3 -p -r1.4 low-linux.c
*** low-linux.c	2000/07/30 01:48:28	1.4
--- low-linux.c	2000/08/25 18:18:45
*************** char buf2[MAX_REGISTER_RAW_SIZE];
*** 53,59 ****
  #define PTRACE_XFER_TYPE int
  #endif
  
- extern char **environ;
  extern int errno;
  extern int inferior_pid;
  void quit (), perror_with_name ();
--- 53,58 ----
Index: gdbserver/low-nbsd.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/low-nbsd.c,v
retrieving revision 1.3
diff -c -3 -p -r1.3 low-nbsd.c
*** low-nbsd.c	2000/07/30 01:48:28	1.3
--- low-nbsd.c	2000/08/25 18:18:45
*************** char buf2[MAX_REGISTER_RAW_SIZE];
*** 42,48 ****
  
  extern int sys_nerr;
  // extern char **sys_errlist;
- extern char **environ;
  extern int inferior_pid;
  void quit (), perror_with_name ();
  
--- 42,47 ----
Index: gdbserver/low-sparc.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/low-sparc.c,v
retrieving revision 1.2
diff -c -3 -p -r1.2 low-sparc.c
*** low-sparc.c	2000/07/30 01:48:28	1.2
--- low-sparc.c	2000/08/25 18:18:45
*************** char buf2[MAX_REGISTER_RAW_SIZE];
*** 52,58 ****
  
  extern int sys_nerr;
  extern char **sys_errlist;
- extern char **environ;
  extern int errno;
  extern int inferior_pid;
  void quit (), perror_with_name ();
--- 52,57 ----
Index: gdbserver/low-sun3.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/low-sun3.c,v
retrieving revision 1.2
diff -c -3 -p -r1.2 low-sun3.c
*** low-sun3.c	2000/07/30 01:48:28	1.2
--- low-sun3.c	2000/08/25 18:18:45
*************** char buf2[MAX_REGISTER_RAW_SIZE];
*** 49,55 ****
  
  extern int sys_nerr;
  extern char **sys_errlist;
- extern char **environ;
  extern int errno;
  extern int inferior_pid;
  void quit (), perror_with_name ();
--- 49,54 ----

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25 11:23 'char **environ' woes with cygwin Michael Elizabeth Chastain
@ 2000-08-25 12:40 ` Chris Faylor
  2000-09-25 12:28 ` Andrew Cagney
  1 sibling, 0 replies; 20+ messages in thread
From: Chris Faylor @ 2000-08-25 12:40 UTC (permalink / raw)
  To: gdb

On Fri, Aug 25, 2000 at 11:25:32AM -0700, Michael Elizabeth Chastain wrote:
>Ok, here's a patch against sourceware that rips out the "environ" crap.
>I haven't even built with it.
>
>After doing the math, the fixed address "&environ" cancels out.  So this
>won't make things any worse (or better) for djgpp or other systems where
>malloc() does more than increment an sbrk pointer.
>
>Chris, do you think this makes your life easier enough to warrant some
>testing and integration into sourceware?

It doesn't really affect me.  I proposed eliminating the use of 'extern
char **environ' throughout gdb.  Your patch removes the extern... just
like mine does, so it's fine with me.

I do agree that we shouldn't be using environ like this, though, so
eliminating it is a good idea.

However, I tried your patch and it failed here on cygwin:

i686-pc-cygwin-gcc -c -g -O2    -I. -I/cygnus/src/sourceware/gdb -I/cygnus/src/sourceware/gdb/config -DHAVE_CONFIG_H -I/cygnus/src/sourceware/gdb/../include/opcode -I/cygnus/src/sourceware/gdb/../readline/.. -I../bfd -I/cygnus/src/sourceware/gdb/../bfd  -I/cygnus/src/sourceware/gdb/../include -I../intl -I/cygnus/src/sourceware/gdb/../intl -DGDBTK -Wimplicit -Wreturn-type -Wcomment -Wtrigraphs -Wformat -Wparentheses -Wpointer-arith -Wuninitialized  /cygnus/src/sourceware/gdb/event-top.c
/cygnus/src/sourceware/gdb/event-top.c: In function `command_handler':
/cygnus/src/sourceware/gdb/event-top.c:550: invalid operands to binary -
/cygnus/src/sourceware/gdb/event-top.c: In function `command_line_handler_continuation':
/cygnus/src/sourceware/gdb/event-top.c:582: invalid operands to binary -

Coercing 'sbrk(0)' to '(char *) sbrk(0)' fixed this.

cgf

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25 11:23 'char **environ' woes with cygwin Michael Elizabeth Chastain
  2000-08-25 12:40 ` Chris Faylor
@ 2000-09-25 12:28 ` Andrew Cagney
  1 sibling, 0 replies; 20+ messages in thread
From: Andrew Cagney @ 2000-09-25 12:28 UTC (permalink / raw)
  To: Michael Elizabeth Chastain; +Cc: cgf, dj, gdb

Michael Elizabeth Chastain wrote:
> 
> Ok, here's a patch against sourceware that rips out the "environ" crap.
> I haven't even built with it.
> 
> After doing the math, the fixed address "&environ" cancels out.  So this
> won't make things any worse (or better) for djgpp or other systems where
> malloc() does more than increment an sbrk pointer.
> 
> Chris, do you think this makes your life easier enough to warrant some
> testing and integration into sourceware?
> 
> Michael

Can this change be simplified further? Vis:

	o	save an sbrk() call
		at the start to get the current
		top of heap

	o	call  sbrk () at the display time
		time and use that to compute
		the size.

Also can the nasty bit of math used to make the computation be pushed
into a function somewhere?

I think your proposed change just needs to be pushed a little bit
further so that all the nasties are eliminated :-)

	enjoy,
		Andrew

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-09-25 12:42 Michael Elizabeth Chastain
@ 2000-09-25 13:30 ` Chris Faylor
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Faylor @ 2000-09-25 13:30 UTC (permalink / raw)
  To: Michael Elizabeth Chastain; +Cc: ac131313, dj, gdb

On Mon, Sep 25, 2000 at 12:45:09PM -0700, Michael Elizabeth Chastain wrote:
>Andrew Cagney wrote:
>
>> I think your proposed change just needs to be pushed a little bit
>> further so that all the nasties are eliminated :-)
>
>OK, I will push it a little bit further and submit it as a sourceware
>patch.  But it's a weekend task rather than a weekday task.

I agree.

Also, for what it's worth, this change is no longer required for Cygwin.
I was trying to work around a header file change that turned out to be
an actual bug.

cgf

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
@ 2000-09-25 12:42 Michael Elizabeth Chastain
  2000-09-25 13:30 ` Chris Faylor
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Elizabeth Chastain @ 2000-09-25 12:42 UTC (permalink / raw)
  To: ac131313, chastain; +Cc: cgf, dj, gdb

Andrew Cagney wrote:

> I think your proposed change just needs to be pushed a little bit
> further so that all the nasties are eliminated :-)

OK, I will push it a little bit further and submit it as a sourceware
patch.  But it's a weekend task rather than a weekday task.

Michael

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25 11:06 Michael Elizabeth Chastain
@ 2000-08-25 11:12 ` DJ Delorie
  0 siblings, 0 replies; 20+ messages in thread
From: DJ Delorie @ 2000-08-25 11:12 UTC (permalink / raw)
  To: chastain; +Cc: gdb

>     space_used = marker_2 - marker_1;

This won't work in DJGPP either.  sbrk() allocates memory from
wherever the system can find it - it doesn't always show up in
contiguous blocks.  Not that many DJGPP users care how much memory gdb
is using.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
@ 2000-08-25 11:06 Michael Elizabeth Chastain
  2000-08-25 11:12 ` DJ Delorie
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Elizabeth Chastain @ 2000-08-25 11:06 UTC (permalink / raw)
  To: chastain, dj; +Cc: gdb

> 162Mb, but that's not what I meant when I said "it works fine".  I
> meant gcc could handle the syntax fine.

Right.  That's what I thought you meant, too.

> If gdb wants a variable in its own data area, it should declare one
> just for that purpose.

Violently agree.

gdb would still have the problem of subtracting sbrk(0) - &global.
That worked fine in 1980 but addresses in different segments can
be relocated anywhere now.  It really should be doing:

    marker_1 = sbrk(0);
    ... compute ... compute ...
    marker_2 = sbrk(0);
    space_used = marker_2 - marker_1;

Michael

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25 10:14 Michael Elizabeth Chastain
@ 2000-08-25 10:43 ` DJ Delorie
  0 siblings, 0 replies; 20+ messages in thread
From: DJ Delorie @ 2000-08-25 10:43 UTC (permalink / raw)
  To: chastain; +Cc: gdb

162Mb, but that's not what I meant when I said "it works fine".  I
meant gcc could handle the syntax fine.

If gdb wants a variable in its own data area, it should declare one
just for that purpose.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
@ 2000-08-25 10:14 Michael Elizabeth Chastain
  2000-08-25 10:43 ` DJ Delorie
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Elizabeth Chastain @ 2000-08-25 10:14 UTC (permalink / raw)
  To: chastain, dj; +Cc: gdb

> I just tested it, and it works fine.

Ok, I am curious.  What does "gdb --statistics" look like?
In particular, the "Space used" line?

    (gdb) file /bin/bash
    Reading symbols from /bin/bash...(no debugging symbols found)...done.
    Command execution time: 0.010000
    Space used: 279516 (+69632 for this command)

Michael

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25  9:35   ` Chris Faylor
@ 2000-08-25 10:09     ` Eli Zaretskii
  0 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2000-08-25 10:09 UTC (permalink / raw)
  To: gdb

> From: Chris Faylor <cgf@cygnus.com>
> Date: Fri, 25 Aug 2000 12:35:14 -0400
>
> So, for Windows, environ needs to be:
> 
> extern char __declspec(dllimport) **environ;

I think some other packages I've seen define a preprocessor symbol
DLLIMPORT or some such, whose definition is empty for all platforms
except Windows.

Of course, your suggestion is okay as well.

> >> #ifndef HAVE_ENVIRON
> >> char **environ;
> >> #endif
> >
> >This would probably break some ports, at least the DJGPP one (unless
> >I'm missing something): the DJGPP startup code includes this
> >definition, but no DJGPP header declares environ.  So we will wind up
> >having multiple definitions at link time.
> 
> Sorry.  I should have added an extern.
> 
> >I think HAVE_ENVIRON cannot be based on examining the headers alone;
> >you must link a test program which says "extern char **environ" and
> >tries to access environ: if linking succeeds, you can define
> >HAVE_ENVIRON.  The program needs to include unistd.h and any other
> >header which might declare environ.
> 
> I don't see why.

That's because I thought you *really* wanted to declare "char **environ"
instead of "extern char **environ".  A misunderstanding.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25  9:56 Michael Elizabeth Chastain
  2000-08-25 10:05 ` DJ Delorie
@ 2000-08-25 10:06 ` Chris Faylor
  1 sibling, 0 replies; 20+ messages in thread
From: Chris Faylor @ 2000-08-25 10:06 UTC (permalink / raw)
  To: gdb

On Fri, Aug 25, 2000 at 09:58:09AM -0700, Michael Elizabeth Chastain wrote:
>> #define environ (*(__get_environ()))
>> -or-
>> extern __declspec(dllimport) char **environ;
>
>The "#define environ" macro is going to yield interesting results
>when various places call "&environ".  Then again, maybe none of those
>places are part of gdb on Cygwin.

Dunno where that __get_environ comes from but I've already built a
cygwin gdb with my proposed changes, of course.

cgf

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25  9:56 Michael Elizabeth Chastain
@ 2000-08-25 10:05 ` DJ Delorie
  2000-08-25 10:06 ` Chris Faylor
  1 sibling, 0 replies; 20+ messages in thread
From: DJ Delorie @ 2000-08-25 10:05 UTC (permalink / raw)
  To: chastain; +Cc: gdb

> > #define environ (*(__get_environ()))
> 
> The "#define environ" macro is going to yield interesting results
> when various places call "&environ".  Then again, maybe none of those
> places are part of gdb on Cygwin.

I just tested it, and it works fine.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
@ 2000-08-25  9:56 Michael Elizabeth Chastain
  2000-08-25 10:05 ` DJ Delorie
  2000-08-25 10:06 ` Chris Faylor
  0 siblings, 2 replies; 20+ messages in thread
From: Michael Elizabeth Chastain @ 2000-08-25  9:56 UTC (permalink / raw)
  To: dj, gdb

> #define environ (*(__get_environ()))
> -or-
> extern __declspec(dllimport) char **environ;

The "#define environ" macro is going to yield interesting results
when various places call "&environ".  Then again, maybe none of those
places are part of gdb on Cygwin.

Michael

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25  1:17 ` Eli Zaretskii
  2000-08-25  9:35   ` Chris Faylor
@ 2000-08-25  9:45   ` DJ Delorie
  1 sibling, 0 replies; 20+ messages in thread
From: DJ Delorie @ 2000-08-25  9:45 UTC (permalink / raw)
  To: gdb

Eli Zaretskii wrote:
> Could you please explain more why is this a problem.  I'm afraid I
> don't know enough about the Cygwin DLL to get the idea.

Cygwin has something like one of these:

#define environ (*(__get_environ()))
-or-
extern __declspec(dllimport) char **environ;

DLLs are better at exporting functions than variables (think DXE).

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-25  1:17 ` Eli Zaretskii
@ 2000-08-25  9:35   ` Chris Faylor
  2000-08-25 10:09     ` Eli Zaretskii
  2000-08-25  9:45   ` DJ Delorie
  1 sibling, 1 reply; 20+ messages in thread
From: Chris Faylor @ 2000-08-25  9:35 UTC (permalink / raw)
  To: gdb

On Fri, Aug 25, 2000 at 04:17:21AM -0400, Eli Zaretskii wrote:
>> From: Chris Faylor <cgf@cygnus.com>
>> Date: Thu, 24 Aug 2000 16:33:39 -0400
>> 
>> I've recently painfully learned that gdb explicitly declares 'char **environ'
>> all over the place.
>
>I see "extern char **environ", not "char **environ", the only
>exception being kdb-start.c.

Um.  Right.  The point is that these are being explicitly declared.

>> This presents a problem for cygwin since environ is now being imported from
>> the cygwin DLL.
>
>Could you please explain more why is this a problem.  I'm afraid I
>don't know enough about the Cygwin DLL to get the idea.

Variables and functions imported from a DLL need to include:

__declspec(dllimport)

So, for Windows, environ needs to be:

extern char __declspec(dllimport) **environ;

>> I've modified configure.in to egrep unistd.h for an environ declaration and
>> define HAVE_ENVIRON if unistd.h contains a declaration.
>
>What about systems where environ is declared in some other header?

Then, I presume that they will continue to work as they always have.  The
'extern char **environ' seems to have been in gdb for some time.

>> The question is, where do I put a:
>> 
>> #ifndef HAVE_ENVIRON
>> char **environ;
>> #endif
>
>This would probably break some ports, at least the DJGPP one (unless
>I'm missing something): the DJGPP startup code includes this
>definition, but no DJGPP header declares environ.  So we will wind up
>having multiple definitions at link time.

Sorry.  I should have added an extern.

>I think HAVE_ENVIRON cannot be based on examining the headers alone;
>you must link a test program which says "extern char **environ" and
>tries to access environ: if linking succeeds, you can define
>HAVE_ENVIRON.  The program needs to include unistd.h and any other
>header which might declare environ.

I don't see why.  Are you asserting that a unistd.h file will declare
'environ' but will not actually define it in the C library?  If that is
the case, then how did gdb ever work?  AFAICT, gdb relies on the existence
of environ.

If 'environ' is defined in unistd.h (as it is in cygwin and most of the
UNIX systems I surveyed) then the 'extern char **environ;' won't be
activated.  Otherwise, if there is no declaration in unistd.h then there
will be an 'extern char **environ;'.  The only thing 

I can't make determinations of what header files to search on systems
for which I don't have access, and it apparently doesn't matter anyway.
If this is a problem for some future port, then configure.in will have
to be changed.  Otherwise, this change should only affect targets which
define environ in unistd.h.

I was trying to avoid adding an "#ifdef __CYGWIN__" because I hate using
system-specific solutions for what is actually a generic (if minor)
problem.

cgf

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-24 13:34 Chris Faylor
@ 2000-08-25  1:17 ` Eli Zaretskii
  2000-08-25  9:35   ` Chris Faylor
  2000-08-25  9:45   ` DJ Delorie
  0 siblings, 2 replies; 20+ messages in thread
From: Eli Zaretskii @ 2000-08-25  1:17 UTC (permalink / raw)
  To: gdb

> From: Chris Faylor <cgf@cygnus.com>
> Date: Thu, 24 Aug 2000 16:33:39 -0400
> 
> I've recently painfully learned that gdb explicitly declares 'char **environ'
> all over the place.

I see "extern char **environ", not "char **environ", the only
exception being kdb-start.c.

> This presents a problem for cygwin since environ is now being imported from
> the cygwin DLL.

Could you please explain more why is this a problem.  I'm afraid I
don't know enough about the Cygwin DLL to get the idea.

> I've modified configure.in to egrep unistd.h for an environ declaration and
> define HAVE_ENVIRON if unistd.h contains a declaration.

What about systems where environ is declared in some other header?

> The question is, where do I put a:
> 
> #ifndef HAVE_ENVIRON
> char **environ;
> #endif

This would probably break some ports, at least the DJGPP one (unless
I'm missing something): the DJGPP startup code includes this
definition, but no DJGPP header declares environ.  So we will wind up
having multiple definitions at link time.

I think HAVE_ENVIRON cannot be based on examining the headers alone;
you must link a test program which says "extern char **environ" and
tries to access environ: if linking succeeds, you can define
HAVE_ENVIRON.  The program needs to include unistd.h and any other
header which might declare environ.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
@ 2000-08-24 15:16 Michael Elizabeth Chastain
  0 siblings, 0 replies; 20+ messages in thread
From: Michael Elizabeth Chastain @ 2000-08-24 15:16 UTC (permalink / raw)
  To: cgf, gdb

> That doesn't seem equivalent to the code that uses environ.  I don't
> understand what the code is actually trying to do, but it seems
> incredibly ill-advised if it is relying on `environ' being in the heap
> somewhere.

Cheesy ascii art coming up ...


					     sbrk(0) ->	+-------+
							|	|
							| heap	|
    							|	|
     sbrk(0) ->	+-------+				+-------+
		|	|				|	|
		|  ...  |				|  ...	|
		|	|				|	|
    &environ ->	|environ|		    &environ -> |environ|
		|	|				|	|
		|  ...	|				|  ...	|
		+-------+				+-------+

  extern char **environ;	       extern char **environ;
  char *lim = (char *) sbrk(0);        char *lim = (char *) sbrk(0);
  space_at_cmd_start = (long) \        long space_now = lim - (char *) &environ;
    (lim - (char *) &environ);         long space_diff = \
    					  space_now - space_at_cmd_start;



Note that this code uses &environ.  It never uses the *value* of environ,
just its address.  As far as this code is concerned, "environ" is just a
variable that is guaranteed to be in the data segment.

If you run gdb as "gdb --statistics", then you will see "space_now"
and "space_diff" printed after each command.  The initial "startup size"
is bogus because &environ can be anywhere in the data segment.

"space_now" - "space_at_cmd_start" is equal to the amount of memory that's
been allocated from sbrk.  It would be simpler to compute this directly
and leave &environ out of the picture.  It would also eliminate the
crufty pointer arithmetic between the brk section and the data section.

Michael

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
  2000-08-24 14:11 Michael Elizabeth Chastain
@ 2000-08-24 14:52 ` Chris Faylor
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Faylor @ 2000-08-24 14:52 UTC (permalink / raw)
  To: Michael Elizabeth Chastain; +Cc: gdb

On Thu, Aug 24, 2000 at 02:13:22PM -0700, Michael Elizabeth Chastain wrote:
>> I've recently painfully learned that gdb explicitly declares 'char **environ'
>> all over the place.
>
>Quickie look:
>
>I see a bunch of strange uses in event-top.c, main.c, top.c.  There is
>a bunch of (replicated) code that wants to compute the memory in
>use, so from time to time it takes sbrk(0) - &some_data_variable.
>The "&some_data_variable" is simply an arbitrary invariant address in
>the data segment.  I bet that the author chose "&environ" for the marker
>variable to use simply because it's a variable that's usually there.
>
>Better code:
>
>  extern char * sbrk_initial;
>
>  ...
>  sbrk_initial = sbrk(0);
>  ...
>
>  space = sbrk(0) - sbrk_initial;

That doesn't seem equivalent to the code that uses environ.  I don't
understand what the code is actually trying to do, but it seems
incredibly ill-advised if it is relying on `environ' being in the heap
somewhere.

However, if it is safe to make this assumption, I guess we could set
"sbrk_initial" in gdb's main() from the third argument to main().

cgf

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 'char **environ' woes with cygwin
@ 2000-08-24 14:11 Michael Elizabeth Chastain
  2000-08-24 14:52 ` Chris Faylor
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Elizabeth Chastain @ 2000-08-24 14:11 UTC (permalink / raw)
  To: cgf, gdb

> I've recently painfully learned that gdb explicitly declares 'char **environ'
> all over the place.

Quickie look:

I see a bunch of strange uses in event-top.c, main.c, top.c.  There is
a bunch of (replicated) code that wants to compute the memory in
use, so from time to time it takes sbrk(0) - &some_data_variable.
The "&some_data_variable" is simply an arbitrary invariant address in
the data segment.  I bet that the author chose "&environ" for the marker
variable to use simply because it's a variable that's usually there.

Better code:

  extern char * sbrk_initial;

  ...
  sbrk_initial = sbrk(0);
  ...

  space = sbrk(0) - sbrk_initial;

> My initial choice was gdb's "environ.h" but that means that a number of files
> need to include this just to get this declaration.

I think if you fix the "compute space in use" code that a lot of those
references to &environ will evaporate.  I also don't see the point of
the declarations in gdbserver/low-{hppabsd,linux,nbsd,sparc,sun3}.c.
and inftarg.c.

That leaves environ.c, fork-child.c, go32-nat.c, infcmd.c, inferior.h,
kdb-start.c.  Those look like files that need to include "environ.h"
because they are using or changing the environment.

Michael Chastain
<chastain@redhat.com>
"love without fear"

^ permalink raw reply	[flat|nested] 20+ messages in thread

* 'char **environ' woes with cygwin
@ 2000-08-24 13:34 Chris Faylor
  2000-08-25  1:17 ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Chris Faylor @ 2000-08-24 13:34 UTC (permalink / raw)
  To: gdb

I've recently painfully learned that gdb explicitly declares 'char **environ'
all over the place.

This presents a problem for cygwin since environ is now being imported from
the cygwin DLL.

I've modified configure.in to egrep unistd.h for an environ declaration and
define HAVE_ENVIRON if unistd.h contains a declaration.

The question is, where do I put a:

#ifndef HAVE_ENVIRON
char **environ;
#endif

?

My initial choice was gdb's "environ.h" but that means that a number of files
need to include this just to get this declaration.  So, I'm wondering if putting
this in defs.h is a better choice.

Can anyone tell me the best place to include this?  I assume that
everyone agrees that putting the above in one header file is better than
sprinkling it throughout the sources.

cgf

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2000-09-25 13:30 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2000-08-25 11:23 'char **environ' woes with cygwin Michael Elizabeth Chastain
2000-08-25 12:40 ` Chris Faylor
2000-09-25 12:28 ` Andrew Cagney
  -- strict thread matches above, loose matches on Subject: below --
2000-09-25 12:42 Michael Elizabeth Chastain
2000-09-25 13:30 ` Chris Faylor
2000-08-25 11:06 Michael Elizabeth Chastain
2000-08-25 11:12 ` DJ Delorie
2000-08-25 10:14 Michael Elizabeth Chastain
2000-08-25 10:43 ` DJ Delorie
2000-08-25  9:56 Michael Elizabeth Chastain
2000-08-25 10:05 ` DJ Delorie
2000-08-25 10:06 ` Chris Faylor
2000-08-24 15:16 Michael Elizabeth Chastain
2000-08-24 14:11 Michael Elizabeth Chastain
2000-08-24 14:52 ` Chris Faylor
2000-08-24 13:34 Chris Faylor
2000-08-25  1:17 ` Eli Zaretskii
2000-08-25  9:35   ` Chris Faylor
2000-08-25 10:09     ` Eli Zaretskii
2000-08-25  9:45   ` DJ Delorie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).