[patch]: dllwrap ported for x64 and support for new --[no-]leading-underscore option

View: New views
5 Messages — Rating Filter:   Alert me  

[patch]: dllwrap ported for x64 and support for new --[no-]leading-underscore option

by Kai Tietz-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hello all,

To make port of binutils tools a bit more complete for x64 Windows,
the dllwrap tool had some issues this patch fixes. Also it allows now
the --[no-}leading-underscore options, too. I am not sure, if this
tool is very useful for people in general, but I see that its main-use
seems to be for x86 cygwin and mingw toolchains. I extended it, so
that it can make a difference between different architectures. This is
mainly necessary for the @<n> post-fix, which has a meaning just for
x86 AFAIU.

2009-10-29  Kai Tietz  <kai.tietz@...>

        * dllwrap.c (is_leading_underscore): New variable.
        (host_type): New enum type.
        (which_host): New variable.
        (usage): Add new options --no-leading-underscore
        and --leading-underscore.
        (long_options): Likewise.
        (OPTION_NO_LEADING_UNDERSCORE): New define.
        (OPTION_LEADING_UNDERSCORE): Likewise.
        (main): Initialize which_host, pass new options
        to dlltool, do underscoring dependent to
        is_leading_underscore, and do '@12' decoration
        only for x86.

Tested for x86_64-pc-mingw32, i686-pc-mingw32, and for i686-cygwin. Ok
for apply?

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination


dllwrap_x64.diff (6K) Download Attachment

Re: [patch]: dllwrap ported for x64 and support for new --[no-]leading-underscore option

by Dave Korn-6 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Kai Tietz wrote:

> Tested for x86_64-pc-mingw32, i686-pc-mingw32, and for i686-cygwin. Ok
> for apply?

  Not quite.  What is all this stuff about the "host"?  You appear to be
referring to the cpu type of the *target* by it, rather than any property of
the host machine, and of course I don't suppose you actually mean to imply
that the output should depend on properties of the system on which the
toolchain is running, rather than the properties of the system on which the
output executable images will be run.

  The terms "host" and "target" are already well defined in the gnu build
system and we should not muddy the waters like this.  Please rename host_type
and which_host to completely remove the word 'host' in all its forms; I would
suggest you replace it by "target_cpu" or similar.

  Other than that:

> +      const char *preFix = (is_leading_underscore != 0 ? "_" : "");
> +      const char *postFix = "";
> +      const char *nameEntry;

  There is no camel-case anywhere else in this file that I could see.  Please
don't gratuitously introduce new and inconsistent coding style just to satisfy
a personal preference.  I wouldn't complain if the file was already full of
mixed styles, but it isn't yet, so let's not start.

  The rest of it looks ok.  There's one place the formatting looked a little
peculiar:

> +  else if (!strncmp (target, "x86_64", 6)
> +           || !strncmp (target, "athlon64", 8)
> +   || !strncmp (target, "amd64", 5))
> +    which_host = X64_HOST;
> +  else if (target[0] == 'i' && (target[1] >= '3' && target[1] <= '6')

but I won't worry about formatting issues until we see your respin without the
"host" confusion.  Thanks for your patience.

    cheers,
      DaveK



Re: [patch]: dllwrap ported for x64 and support for new --[no-]leading-underscore option

by Kai Tietz-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Hi Dave,

2009/11/4 Dave Korn <dave.korn.cygwin@...>:

> Kai Tietz wrote:
>
>> Tested for x86_64-pc-mingw32, i686-pc-mingw32, and for i686-cygwin. Ok
>> for apply?
>
>  Not quite.  What is all this stuff about the "host"?  You appear to be
> referring to the cpu type of the *target* by it, rather than any property of
> the host machine, and of course I don't suppose you actually mean to imply
> that the output should depend on properties of the system on which the
> toolchain is running, rather than the properties of the system on which the
> output executable images will be run.
Well, I changed it. What me confused here is, that normally for
configure --host is used as build target, and --target isn't necessary
the same. So as this tool is a host tool, I was confused here.

>  The terms "host" and "target" are already well defined in the gnu build
> system and we should not muddy the waters like this.  Please rename host_type
> and which_host to completely remove the word 'host' in all its forms; I would
> suggest you replace it by "target_cpu" or similar.
>
>  Other than that:
>
>> +      const char *preFix = (is_leading_underscore != 0 ? "_" : "");
>> +      const char *postFix = "";
>> +      const char *nameEntry;
>
>  There is no camel-case anywhere else in this file that I could see.  Please
> don't gratuitously introduce new and inconsistent coding style just to satisfy
> a personal preference.  I wouldn't complain if the file was already full of
> mixed styles, but it isn't yet, so let's not start.
Agreed, just done by habit ;)

>  The rest of it looks ok.  There's one place the formatting looked a little
> peculiar:
>
>> +  else if (!strncmp (target, "x86_64", 6)
>> +           || !strncmp (target, "athlon64", 8)
>> +        || !strncmp (target, "amd64", 5))
>> +    which_host = X64_HOST;
>> +  else if (target[0] == 'i' && (target[1] >= '3' && target[1] <= '6')
Intend was ok, just one time with spaces, and the other time with tab
and spaces. I correct it, so that both lines are using the same
spacing.

> but I won't worry about formatting issues until we see your respin without the
> "host" confusion.  Thanks for your patience.
>
>    cheers,
>      DaveK
>
>
>

Here is the updated patch

2009-11-04  Kai Tietz  <kai.tietz@...>

       * dllwrap.c (is_leading_underscore): New variable.
       (cpu_type): New enum type.
       (which_cpu): New variable.
       (usage): Add new options --no-leading-underscore
       and --leading-underscore.
       (long_options): Likewise.
       (OPTION_NO_LEADING_UNDERSCORE): New define.
       (OPTION_LEADING_UNDERSCORE): Likewise.
       (main): Initialize which_host, pass new options
       to dlltool, do underscoring dependent to
       is_leading_underscore, and do '@12' decoration
       only for x86.


Cheers,
Kai


--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination


dllwrap_x64.diff (6K) Download Attachment

Re: [patch]: dllwrap ported for x64 and support for new --[no-]leading-underscore option

by Dave Korn-6 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Kai Tietz wrote:

>>  Not quite.  What is all this stuff about the "host"?  You appear to be
>> referring to the cpu type of the *target* by it, rather than any property of
>> the host machine, and of course I don't suppose you actually mean to imply
>> that the output should depend on properties of the system on which the
>> toolchain is running, rather than the properties of the system on which the
>> output executable images will be run.

> Well, I changed it. What me confused here is, that normally for
> configure --host is used as build target, and --target isn't necessary
> the same. So as this tool is a host tool, I was confused here.

  *All* tools are "host" tools, in the sense that they all *run on* the "host"
machine; that is what host means.

  It is only with generator tools, like the compiler and binutils, that there
arises a question of what machine the code they generate is targeted for -
hence anything to do with the output is a target matter.  When running native
binutils, the target is the same machine as the host, but not when running
cross binutils; and what we are discussing here is the cpu of the target
machine, on which the generated dll will run.

  Dllwrap can be run on a linux *host*, and will still generate a dll suitable
for an x86, x64 or Arm PE *target*.  (It can *only* be configured as a cross
tool on Linux, since it does not support Linux targets.)

[  This does occasionally result in the need for a switch of perspective, e.g.
when building a cross-compiler, you are making an executable that runs on the
host and emits output suitable for the target, but as part of that compiler
build you need to build target libraries, and when those target libraries are
configured, because they are applications that run on a platform and not
generators of any kind, they have only a host setting, and what was the target
for the compiler has to become the host for the target library, and the target
library does not have a target configure option!  But it's all perfectly
logical when you work it through, and generally only surprises people the
first time they realise it.  ]

> Here is the updated patch

  OK, with just ...

>        (main): Initialize which_host, pass new options
>        to dlltool, do underscoring dependent to
>        is_leading_underscore, and do '@12' decoration

  "do X dependent to Y" -> "do X dependent on Y".  Thanks!

    cheers,
      DaveK



Re: [patch]: dllwrap ported for x64 and support for new --[no-]leading-underscore option

by Kai Tietz-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

2009/11/4 Dave Korn <dave.korn.cygwin@...>:

> Kai Tietz wrote:
>
>>>  Not quite.  What is all this stuff about the "host"?  You appear to be
>>> referring to the cpu type of the *target* by it, rather than any property of
>>> the host machine, and of course I don't suppose you actually mean to imply
>>> that the output should depend on properties of the system on which the
>>> toolchain is running, rather than the properties of the system on which the
>>> output executable images will be run.
>
>> Well, I changed it. What me confused here is, that normally for
>> configure --host is used as build target, and --target isn't necessary
>> the same. So as this tool is a host tool, I was confused here.
>
>  *All* tools are "host" tools, in the sense that they all *run on* the "host"
> machine; that is what host means.
>
>  It is only with generator tools, like the compiler and binutils, that there
> arises a question of what machine the code they generate is targeted for -
> hence anything to do with the output is a target matter.  When running native
> binutils, the target is the same machine as the host, but not when running
> cross binutils; and what we are discussing here is the cpu of the target
> machine, on which the generated dll will run.
>
>  Dllwrap can be run on a linux *host*, and will still generate a dll suitable
> for an x86, x64 or Arm PE *target*.  (It can *only* be configured as a cross
> tool on Linux, since it does not support Linux targets.)
>
> [  This does occasionally result in the need for a switch of perspective, e.g.
> when building a cross-compiler, you are making an executable that runs on the
> host and emits output suitable for the target, but as part of that compiler
> build you need to build target libraries, and when those target libraries are
> configured, because they are applications that run on a platform and not
> generators of any kind, they have only a host setting, and what was the target
> for the compiler has to become the host for the target library, and the target
> library does not have a target configure option!  But it's all perfectly
> logical when you work it through, and generally only surprises people the
> first time they realise it.  ]

Well, thanks. Only question remaining for me here is, shouldn't we
pass in dllwrap, if not already specified as argument, the
corresponding --machine option to dlltool, too?

>> Here is the updated patch
>
>  OK, with just ...
>
>>        (main): Initialize which_host, pass new options
>>        to dlltool, do underscoring dependent to
>>        is_leading_underscore, and do '@12' decoration
>
>  "do X dependent to Y" -> "do X dependent on Y".  Thanks!
>
>    cheers,
>      DaveK

Ok, applied.

Cheers,
Kai

--
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination