This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Fix dlls for non underscored targets.
Christopher Faylor escreveu:
On Thu, Sep 21, 2006 at 03:09:46AM +0100, Pedro Alves wrote:
Christopher Faylor escreveu:
for (i = 0; i < d->num_exports; i++)
@@ -617,8 +619,8 @@ process_def_file (bfd *abfd ATTRIBUTE_UN
/* We should not re-export imported stuff. */
{
- char *name = xmalloc (strlen (sn) + 2 + 6);
- sprintf (name, "%s%s", U("_imp_"), sn);
+ char *name = xmalloc (sizeof ("__imp_") - 1 + strlen
(sn) + 1);
Wouldn't it be better to just get rid of the "- 1 ... + 1" above?
Well, I left it for clarity. The first is because sizeof ("string") returns
the length+1 and the last is for the terminating '\0' of the full built
'char *name'.
It think it is clearer this way than how it was.
If you're going for clarity, then it is clearer to just use strlen
("__imp_") since that should just resolve to a constant 6.
Can/could/should but not must. With sizeof, it is a given.
Nevertheless, I don't agree that the +1 - 1 is clearer. Any C
programmer should know that sizeof ("a string") includes the NUL byte
and the additional arithmetic actually gave me pause for a second.
Right, that is why I don't understand why you don't think it is clearer
than `strlen (sn) + 2 + 6`. It gave you a second pause for +1 -1, but it
gave me a two sec pause on that +2 +6 which is an off-by-one.
Well, you have to agree that this falls into the nit arena, so I'm
changing to strlen
anyway ;).
I'm also wondering if it would make sense to just put the "__imp_"/"_imp_"
string in a
variable somewhere and just use that.
Humm, not sure what you mean? With this patch it is __imp_
everywhere, except one place (auto_export) that expects a symbol name
stripped from the underscore.
Maybe you meant a define?
I am not at a place where I can check the source code right now but I'm not
clear on why you remove the U ("_head_") in some cases and add it in
others.
Where?
I should have just said that "remove the U()", as in:
@@ -1918,7 +1920,7 @@ make_one (def_file_export *exp, bfd *par
BSF_GLOBAL, 0);
if (! exp->flag_data)
quick_symbol (abfd, "", exp->internal_name, "", tx, BSF_GLOBAL, 0);
- quick_symbol (abfd, U ("_imp_"), exp->internal_name, "", id5,
+ quick_symbol (abfd, "__imp_", exp->internal_name, "", id5,
BSF_GLOBAL, 0);
Because only __imp_ is needed to be compatible with MSFT.
Correct me if I'm wrong, but U ("_head_")<sym> as a *named* symbol isn't
exported from any lib/dll generated from any MSFT tool. Also _nm_ is
ld/auto-import specific.
This patch makes pe-dll.c inline with what dlltool does. Dlltool treats
all symbols but __imp_
the same, even _head_. Check out dlltool.c:make_label, and
dlltool.c:make_imp_label.
Another reason for not removing the U on _head_ or _nm_, is that this way
I wont have to split autofilter_symbolprefixlist into two.
All in all, it was a decision based on make-it-work, break the least and
keep binary compatibility (not many users, but still a nuisance).||
Here is an updated patch, that moves the __imp_ handling out of
auto_export,
so it is always treated with the underscore.
It also fixes another place in pe.em where the underscore was being
unconditionally added.
Tested on cross to arm-wince-pe and native i686-pc-cygwin.
Please review and commit.
Cheers,
Pedro Alves
---
2006-09-25 Pedro Alves <pedro_alves@portugalmail.pt>
ld/ChangeLog
* pe-dll.c : Fix typo.
(autofilter_symbolprefixlist) : Remove __imp_.
(is_import) : New.
(auto-export) : Remove re-import check. Moved to callers.
(process_def_file) : Check is symbol is an import. Always underscore
__imp_.
Only skip underscore on underscored targets.
(make_one) : Always underscore __imp_.
(pe_create_runtime_relocator_reference) : Only underscore
_pei386_runtime_relocator on underscored targets.
(pe_process_import_defs) : Always underscore __imp_.
* pe.em (U) : New macro.
(set_pe_subsystem) : Remove underscore from _WinMainCRTStartup
on wince subsystem case.
(pe_find_data_imports) : Use U on "_head_".
(gld_${EMULATION_NAME}_unrecognized_file) : Use U.
Index: pe-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.c,v
retrieving revision 1.89
diff -u -p -r1.89 pe-dll.c
--- pe-dll.c 20 Sep 2006 11:35:09 -0000 1.89
+++ pe-dll.c 26 Sep 2006 12:45:10 -0000
@@ -123,7 +123,7 @@
(so, DLL name is referenced by multiple entries), and pointer to symbol
name thunk. Symbol name thunk is singleton vector (__nm_th_<symbol>)
pointing to IMAGE_IMPORT_BY_NAME structure (__nm_<symbol>) directly
- containing imported name. Here comes that "om the edge" problem mentioned
+ containing imported name. Here comes that "on the edge" problem mentioned
above: PE specification rambles that name vector (OriginalFirstThunk)
should run in parallel with addresses vector (FirstThunk), i.e. that they
should have same number of elements and terminated with zero. We violate
@@ -331,12 +331,13 @@ static autofilter_entry_type autofilter_
static autofilter_entry_type autofilter_symbolprefixlist[] =
{
- { STRING_COMMA_LEN ("__imp_") },
- /* Do __imp_ explicitly to save time. */
+ /* _imp_ is treated specially, as it is always underscored. */
+ /* { STRING_COMMA_LEN ("_imp_") }, */
+ /* Don't export some c++ symbols. */
{ STRING_COMMA_LEN ("__rtti_") },
+ { STRING_COMMA_LEN ("__builtin_") },
/* Don't re-export auto-imported symbols. */
{ STRING_COMMA_LEN ("_nm_") },
- { STRING_COMMA_LEN ("__builtin_") },
/* Don't export symbols specifying internal DLL layout. */
{ STRING_COMMA_LEN ("_head_") },
{ STRING_COMMA_LEN (NULL) }
@@ -445,6 +446,11 @@ pe_dll_add_excludes (const char *new_exc
free (local_copy);
}
+static bfd_boolean
+is_import (const char* n)
+{
+ return (CONST_STRNEQ (n, "__imp_"));
+}
/* abfd is a bfd containing n (or NULL)
It can be used for contextual checks. */
@@ -459,10 +465,6 @@ auto_export (bfd *abfd, def_file *d, con
if (abfd && abfd->my_archive)
libname = lbasename (abfd->my_archive->filename);
- /* We should not re-export imported stuff. */
- if (CONST_STRNEQ (n, "_imp_"))
- return 0;
-
for (i = 0; i < d->num_exports; i++)
if (strcmp (d->exports[i].name, n) == 0)
return 0;
@@ -617,8 +619,11 @@ process_def_file (bfd *abfd ATTRIBUTE_UN
/* We should not re-export imported stuff. */
{
- char *name = xmalloc (strlen (sn) + 2 + 6);
- sprintf (name, "%s%s", U("_imp_"), sn);
+ if (is_import (sn))
+ continue;
+
+ char *name = xmalloc (strlen ("__imp_") + strlen (sn) + 1);
+ sprintf (name, "%s%s", "__imp_", sn);
blhe = bfd_link_hash_lookup (info->hash, name,
FALSE, FALSE, FALSE);
@@ -628,7 +633,7 @@ process_def_file (bfd *abfd ATTRIBUTE_UN
continue;
}
- if (*sn == '_')
+ if (pe_details->underscored && *sn == '_')
sn++;
if (auto_export (b, pe_def_file, sn))
@@ -674,6 +679,9 @@ process_def_file (bfd *abfd ATTRIBUTE_UN
{
for (i = 0; i < NE; i++)
{
+ if (is_import (pe_def_file->exports[i].name))
+ continue;
+
if (strchr (pe_def_file->exports[i].name, '@'))
{
int lead_at = (*pe_def_file->exports[i].name == '@');
@@ -1918,7 +1926,7 @@ make_one (def_file_export *exp, bfd *par
BSF_GLOBAL, 0);
if (! exp->flag_data)
quick_symbol (abfd, "", exp->internal_name, "", tx, BSF_GLOBAL, 0);
- quick_symbol (abfd, U ("_imp_"), exp->internal_name, "", id5,
+ quick_symbol (abfd, "__imp_", exp->internal_name, "", id5,
BSF_GLOBAL, 0);
/* Fastcall applies only to functions,
so no need for auto-import symbol. */
@@ -1930,12 +1938,12 @@ make_one (def_file_export *exp, bfd *par
if (! exp->flag_data)
quick_symbol (abfd, U (""), exp->internal_name, "", tx,
BSF_GLOBAL, 0);
- quick_symbol (abfd, U ("_imp__"), exp->internal_name, "", id5,
+ quick_symbol (abfd, "__imp_", U (""), exp->internal_name, id5,
BSF_GLOBAL, 0);
/* Symbol to reference ord/name of imported
data symbol, used to implement auto-import. */
if (exp->flag_data)
- quick_symbol (abfd, U("_nm__"), exp->internal_name, "", id6,
+ quick_symbol (abfd, U ("_nm_"), U (""), exp->internal_name, id6,
BSF_GLOBAL,0);
}
if (pe_dll_compat_implib)
@@ -2259,7 +2267,7 @@ pe_create_runtime_relocator_reference (b
symtab = xmalloc (2 * sizeof (asymbol *));
extern_rt_rel = quick_section (abfd, ".rdata", SEC_HAS_CONTENTS, 2);
- quick_symbol (abfd, "", "__pei386_runtime_relocator", "", UNDSEC,
+ quick_symbol (abfd, "", U ("_pei386_runtime_relocator"), "", UNDSEC,
BSF_NO_FLAGS, 0);
bfd_set_section_size (abfd, extern_rt_rel, 4);
@@ -2460,7 +2468,7 @@ pe_process_import_defs (bfd *output_bfd,
char *name = xmalloc (len + 2 + 6);
if (lead_at)
- sprintf (name, "%s%s", "",
+ sprintf (name, "%s",
pe_def_file->imports[i].internal_name);
else
sprintf (name, "%s%s",U (""),
@@ -2472,10 +2480,10 @@ pe_process_import_defs (bfd *output_bfd,
if (!blhe || (blhe && blhe->type != bfd_link_hash_undefined))
{
if (lead_at)
- sprintf (name, "%s%s", U ("_imp_"),
+ sprintf (name, "%s%s", "__imp_",
pe_def_file->imports[i].internal_name);
else
- sprintf (name, "%s%s", U ("_imp__"),
+ sprintf (name, "%s%s%s", "__imp_", U (""),
pe_def_file->imports[i].internal_name);
blhe = bfd_link_hash_lookup (link_info->hash, name,
Index: emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.120
diff -u -p -r1.120 pe.em
--- emultempl/pe.em 16 Sep 2006 18:12:16 -0000 1.120
+++ emultempl/pe.em 26 Sep 2006 12:47:50 -0000
@@ -115,6 +115,7 @@ cat >>e${EMULATION_NAME}.c <<EOF
#define PE_DEF_FILE_ALIGNMENT 0x00000200
#endif
+#define U(S) ${INITIAL_SYMBOL_CHAR} S
static struct internal_extra_pe_aouthdr pe;
static int dll;
@@ -400,7 +401,7 @@ set_pe_subsystem (void)
{ "windows", 2, "WinMainCRTStartup" },
{ "console", 3, "mainCRTStartup" },
{ "posix", 7, "__PosixProcessStartup"},
- { "wince", 9, "_WinMainCRTStartup" },
+ { "wince", 9, "WinMainCRTStartup" },
{ "xbox", 14, "mainCRTStartup" },
{ NULL, 0, NULL }
};
@@ -925,14 +926,14 @@ pe_find_data_imports (void)
for (i = 0; i < nsyms; i++)
{
- if (! CONST_STRNEQ (symbols[i]->name, "__head_"))
+ if (! CONST_STRNEQ (symbols[i]->name, U ("_head_")))
continue;
if (pe_dll_extra_pe_debug)
printf ("->%s\n", symbols[i]->name);
pe_data_import_dll = (char*) (symbols[i]->name +
- sizeof ("__head_") - 1);
+ sizeof (U ("_head_")) - 1);
break;
}
@@ -1342,7 +1343,7 @@ gld_${EMULATION_NAME}_unrecognized_file
{
struct bfd_link_hash_entry *h;
- sprintf (buf, "_%s", pe_def_file->exports[i].internal_name);
+ sprintf (buf, "%s%s", U (""), pe_def_file->exports[i].internal_name);
h = bfd_link_hash_lookup (link_info.hash, buf, TRUE, TRUE, TRUE);
if (h == (struct bfd_link_hash_entry *) NULL)