This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
[PATCH] pei386, ld auto-import
- To: binutils at sources dot redhat dot com
- Subject: [PATCH] pei386, ld auto-import
- From: Charles Wilson <cwilson at ece dot gatech dot edu>
- Date: Sun, 23 Sep 2001 17:51:43 -0400
In this message:
Missing variable 'pe_data_import_dll'
http://sources.redhat.com/ml/binutils/2001-08/msg00186.html
Nick Clifton reported that the recently accepted auto-import changes to
ld on pei386 caused a build problem on other pe platforms that don't
support DLLs (specifically arm-epoc-pe).
This was because the auto-import changes added a reference in pe-dll.c
to a variable in pe.em -- but that variable wasn't defined unless
DLL_SUPPORT was defined.
Now, Nick posted a workaround to the problem here:
http://sources.redhat.com/ml/binutils/2001-08/msg00187.html
but AFAICT it was never applied to CVS. (Plus, it was just a
workaround). In that message, Nick also suggested the following:
I still
feel however, that there ought to be a better way to solve this
problem. My suggestion is that the definition of DLL_SUPPORT ought
to be set in ld/configure.tgt rather than ld/emultemp/pe.em and then
tested in ld/pe-dll.c before it uses variables that are only defined
in pe.em.
That's not what I did, because ld/configure.tgt seems to only set shell
variables, which are later used to control Makefile options. However,
the list of acceptable shell variables is fixed: targ_emul,
targ_extra_emuls, targ_extra_libpath, and targ_extra_ofiles. These are
then (eventually) used by configure to fixup the Makefile.in's when
generating the Makefile's. So, sure, it's *possible* to do what Nick
suggests -- by changing configure.in/configure so that it will replace
@DLL_SUPPORT@, adding a few more rules to Makefile.in, adding
DLL_SUPPORT where appropriate in configure.tgt, etc., but ...
It really doesn't seem right to me that "DLL_SUPPORT" be added to the
list of stuff controlled by configure.tgt, which is currently limited to
things like targ_*. Comments?
So, the attached patch is identical to the one I posted a few days ago,
but the Subject line of that earlier message was probably misleading.
Anyway, the attached patch fixes this problem by:
a) make the offending variable (pe_data_import_dll) static within pe.em
b) add a new function in pe.em (pe_get_data_import_dll_name) which
either returns this variable (#ifdef DLL_SUPPORT), or returns a constant
string (#else). However, at least the function itself is always defined.
Could somebody on arm-epoc-pe or other non-DLL-supporting pe platform
please verify that this patch allows successful compilation/operation of
binutils? the patched version does work on cygwin (but the unpatched
version worked on cygwin, too)
Thanks,
Chuck
2001-09-21 Charles Wilson <cwilson@ece.gatech.edu>
* emultempl/pe.em(pe_data_import_dll): Make static.
(pe_get_data_import_dll_name): New accessor function.
* pe-dll.c(pe_create_import_fixup): call
pe_get_data_import_dll_name() from pe.em, instead of
directly accessing pe_data_import_dll variable from pe.em.
Index: ld/pe-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.c,v
retrieving revision 1.30
diff -u -r1.30 old/ld/pe-dll.c new/ld/pe-dll.c
--- old/ld/pe-dll.c 2001/09/19 05:33:33 1.30
+++ new/ld/pe-dll.c 2001/09/21 06:09:49
@@ -123,6 +123,9 @@
static void
add_bfd_to_link PARAMS ((bfd *, const char *, struct bfd_link_info *));
+extern char *
+pe_get_data_import_dll_name PARAMS(( )); /* Defined in emultempl/pe.em. */
+
/* For emultempl/pe.em. */
def_file * pe_def_file = 0;
@@ -2065,10 +2068,9 @@
}
{
- extern char * pe_data_import_dll; /* Defined in emultempl/pe.em. */
-
- bfd *b = make_import_fixup_entry (name, fixup_name, pe_data_import_dll,
- output_bfd);
+ bfd *b = make_import_fixup_entry (name, fixup_name,
+ pe_get_data_import_dll_name(),
+ output_bfd);
add_bfd_to_link (b, b->filename, &link_info);
}
}
Index: ld/emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.52
diff -u -r1.52 old/ld/emultempl/pe.em new/ld/emultempl/pe.em
--- old/ld/emultempl/pe.em 2001/09/18 10:10:21 1.52
+++ new/ld/emultempl/pe.em 2001/09/21 06:09:51
@@ -153,6 +153,7 @@
static char *pe_implib_filename = NULL;
static int pe_enable_auto_image_base = 0;
static char *pe_dll_search_prefix = NULL;
+static char *pe_data_import_dll = NULL;
#endif
extern const char *output_filename;
@@ -879,7 +880,32 @@
return 1;
}
-char *pe_data_import_dll;
+/* Previously, pe-dll.c directly accessed pe_data_import_dll,
+ * which was only defined if DLL_SUPPORT. This cause a build
+ * failure on certain targets. At least this function will
+ * exist regardless of whether DLL_SUPPORT is defined or not.
+ *
+ * However, it's still a kludge. pe-dll.c shouldn't directly
+ * call any functions other than the gld_${EMULATION_NAME}_*
+ * Nick suggests the following method:
+ * I still feel however, that there ought to be a better
+ * way to solve this problem. My suggestion is that the
+ * definition of DLL_SUPPORT ought to be set in ld/configure.tgt
+ * rather than ld/emultemp/pe.em and then tested in ld/pe-dll.c
+ * before it uses variables that are only defined in pe.em.
+ * However, I don't understand this. ld/configure.tgt sets SHELL
+ * variables, not #define variables. You'd need #define variables
+ * to #ifdef out the offending sections of code from pe-dll.c
+ */
+char *
+pe_get_data_import_dll_name ()
+{
+#ifdef DLL_SUPPORT
+ return pe_data_import_dll;
+#else
+ return "unknown";
+#endif
+}
static void
pe_find_data_imports ()