Subject: bug#13783: simplify data_start configuration for Emacs

From: Eli Zaretskii <[email-masked]>
Date: Fri, 22 Feb 2013 15:05:35 +0200

> Date: Fri, 22 Feb 2013 00:18:37 -0800
> From: Paul Eggert <[email-masked]>
> CC: Eli Zaretskii <[email-masked]>
> 
> Attached is a proposed patch that follows on to the Bug#13650
> fixes, by simplifying the code to remove most of the
> data_start stuff, which is obsolete.  This affects the
> Windows port so I'm CC:ing this to Eli.

Could you please explain more about what this is intended to
accomplish?  Some things seem wrong (a few specific comments below),
but I don't feel I understand this enough to suggest better/more
correct ways to do what you want.  E.g., are we removing the
data-start thing on all platforms where GCPRO is a no-op, i.e. those
which support USE_LSB_TAG?

> === modified file 'src/unexcoff.c'
> --- src/unexcoff.c	2013-01-02 16:13:04 +0000
> +++ src/unexcoff.c	2013-02-22 08:11:05 +0000
> @@ -99,7 +99,7 @@
>  
>  #include <sys/file.h>
>  
> -#include "mem-limits.h"
> +extern unsigned char *get_data_start (void);
>  
>  static long block_copy_start;		/* Old executable start point */
>  static struct filehdr f_hdr;		/* File header */
> @@ -168,7 +168,7 @@
>    pagemask = getpagesize () - 1;
>  
>    /* Adjust text/data boundary. */
> -  data_start = (int) start_of_data ();
> +  data_start = (int) get_data_start ();
>    data_start = ADDR_CORRECT (data_start);
>    data_start = data_start & ~pagemask; /* (Down) to page boundary. */

This part is wrong: unexcoff.c is used only by the MSDOS build, but
get_data_start is defined in w32heap.c, which is for the MS-Windows
build.  I'm not sure how to DTRT here (see above); all I can say at
this point is that the MSDOS build currently defines DATA_START, see
msdos/sed2v2.inp, and that definition is then used in start_of_data.

> +/* Start of data.  It is OK if this is approximate; it's used only as
> +   a heuristic.  */
> +extern char data_start[];
> +#ifndef HAVE_DATA_START
> +/* Initialize to nonzero, so that it's put into data and not bss.
> +   Link this file's object code first, so that this symbol is near the
> +   start of data.  */
> +char data_start[1] = { 1 };
> +#endif

Should platforms that HAVE_DATA_START initialize data_start to some
value?  Should _all_ platforms do that?  If not, which ones should?

>  #ifdef REL_ALLOC
> -  extern POINTER (*real_morecore) (ptrdiff_t);
> +  extern void *(*real_morecore) (ptrdiff_t);

This needs corresponding changes in ralloc.c, I think, at least for
consistency if nothing else.

> -  extern POINTER (*__morecore) (ptrdiff_t);
> +  extern void *(*__morecore) (ptrdiff_t);

Likewise.

Thanks.