[PATCH,take2] Add plugin interface to LD, respun. [1..6/6]

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH,take2] Add plugin interface to LD, respun. [1..6/6]

Dave Korn-9

    Good morning Richard and list,

  I've respun all the LD plugin patches to fix the issues raised, rebased them
up to more recent CVS HEAD, updated the ChangeLogs, and I've rebuilt and
retested on i686-pc-cygwin and i686-pc-linux-gnu.

  I've also built but not tested (owing to lack of a compiler)
mips-unknown-ecoff, alpha-unknown-linuxecoff, alpha-unknown-vms,
pdp11-unknown-aout, rs6000-ibm-aix5.3.1 and i386-unknown-msdos to make sure
I've at least not broken the build on ECOFF, XCOFF and AOUT targets.

  I think the one thing we didn't explicitly discuss was what to do with the
symbol resolution stuff.  I rewrote that based on the logic in
gold/plugin.cc#Pluginobj::get_symbol_resolution_info(), so it should work
correctly now on ELF as well as COFF and its derivatives.

  I plan to follow up shortly with a final patch to libldtl-ize the plugin
loading, so that MinGW can work.

  Is it OK now to commit the entire series?

    cheers,
      DaveK

ld-plugin-api-1-infra.diff (53K) Download Attachment
ld-plugin-api-2-claimfiles-addsyms.diff (33K) Download Attachment
ld-plugin-api-3-get-symbols.diff (15K) Download Attachment
ld-plugin-api-4-add-files.diff (15K) Download Attachment
ld-plugin-api-5-elf-vis.diff (3K) Download Attachment
ld-plugin-api-6-libs.diff (35K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take2] Add plugin interface to LD, respun. [1..6/6]

Richard Henderson-2
On 10/10/2010 10:56 PM, Dave Korn wrote:

>
>     Good morning Richard and list,
>
>   I've respun all the LD plugin patches to fix the issues raised, rebased them
> up to more recent CVS HEAD, updated the ChangeLogs, and I've rebuilt and
> retested on i686-pc-cygwin and i686-pc-linux-gnu.
>
>   I've also built but not tested (owing to lack of a compiler)
> mips-unknown-ecoff, alpha-unknown-linuxecoff, alpha-unknown-vms,
> pdp11-unknown-aout, rs6000-ibm-aix5.3.1 and i386-unknown-msdos to make sure
> I've at least not broken the build on ECOFF, XCOFF and AOUT targets.
>
>   I think the one thing we didn't explicitly discuss was what to do with the
> symbol resolution stuff.  I rewrote that based on the logic in
> gold/plugin.cc#Pluginobj::get_symbol_resolution_info(), so it should work
> correctly now on ELF as well as COFF and its derivatives.
>
>   I plan to follow up shortly with a final patch to libldtl-ize the plugin
> loading, so that MinGW can work.
>
>   Is it OK now to commit the entire series?
>
>     cheers,
>       DaveK

Without --enable-plugins:

../../git-binu/ld/ldmain.c: In function ‘add_archive_element’:
../../git-binu/ld/ldmain.c:800: error: unused variable ‘fildes’
../../git-binu/ld/ldmain.c:796: error: unused parameter ‘subsbfd’

which can be fixed in the obvious way.

With --enable-plugins (x86_64-linux):

libtool: link: gcc -shared  .libs/libldtestplug_la-testplug.o   -ldl  ../libiberty/libiberty.a   -Wl,-soname -Wl,libldtestplug.so.0 -o .libs/libldtestplug.so.0.0.0
/usr/bin/ld: ../libiberty/libiberty.a(xmalloc.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC

It may be easier to avoid libiberty in the test plugin than to
work out how to coordinate -fpic over there without libtool too.

For testing purposes, I just built the libiberty directory with
-fpic added manually.  At which point I get

Running /local/rth/gcc/git-binu/ld/testsuite/ld-plugin/plugin.exp ...
ERROR: tcl error sourcing /local/rth/gcc/git-binu/ld/testsuite/ld-plugin/plugin.exp.
ERROR: couldn't save command result in variable
    while executing
"catch "exec $NM tmpdir/func.o" nm_output"
    invoked from within
"if { ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/main.c tmpdir/main.o]
        || ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/func.c tmpdir/func.o]
        || ![ld_co..."
    (file "/local/rth/gcc/git-binu/ld/testsuite/ld-plugin/plugin.exp" line 64)
    invoked from within
"source /local/rth/gcc/git-binu/ld/testsuite/ld-plugin/plugin.exp"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 source /local/rth/gcc/git-binu/ld/testsuite/ld-plugin/plugin.exp"
    invoked from within
"catch "uplevel #0 source $test_file_name""


----


>  ldfile_try_open_bfd (const char *attempt,
>                      lang_input_statement_type *entry)
>  {
> +#ifdef ENABLE_PLUGINS
> +  int fildes;
> +#endif /* ENABLE_PLUGINS */

Please move that down to the use and add { }.


r~
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take2] Add plugin interface to LD, respun. [1..6/6]

Dave Korn-9
On 11/10/2010 18:22, Richard Henderson wrote:

> Without --enable-plugins:
>
> ../../git-binu/ld/ldmain.c: In function ‘add_archive_element’:
> ../../git-binu/ld/ldmain.c:800: error: unused variable ‘fildes’
> ../../git-binu/ld/ldmain.c:796: error: unused parameter ‘subsbfd’
>
> which can be fixed in the obvious way.
>
> With --enable-plugins (x86_64-linux):

  Err, I think the first thing I have to do is figure out why --enable-plugins
makes a difference at all considering I removed the AC_ARG_ENABLE!  It should
all be automatic based on the availability of dlfcn.h, and the linkability of
dlopen/dlsym/dlclose.

> libtool: link: gcc -shared  .libs/libldtestplug_la-testplug.o   -ldl  ../libiberty/libiberty.a   -Wl,-soname -Wl,libldtestplug.so.0 -o .libs/libldtestplug.so.0.0.0
> /usr/bin/ld: ../libiberty/libiberty.a(xmalloc.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC

  Eeep!

> It may be easier to avoid libiberty in the test plugin than to
> work out how to coordinate -fpic over there without libtool too.

  Yes, I think you're right, particularly considering:

> For testing purposes, I just built the libiberty directory with
> -fpic added manually.  At which point I get
>
> Running /local/rth/gcc/git-binu/ld/testsuite/ld-plugin/plugin.exp ...
> ERROR: tcl error sourcing /local/rth/gcc/git-binu/ld/testsuite/ld-plugin/plugin.exp.

... this must be some sort of run path problem.

>>  ldfile_try_open_bfd (const char *attempt,
>>                      lang_input_statement_type *entry)
>>  {
>> +#ifdef ENABLE_PLUGINS
>> +  int fildes;
>> +#endif /* ENABLE_PLUGINS */
>
> Please move that down to the use and add { }.

  Oops, missed one.  I'll factor the ternary that sets it into an if() to
provide a scope.

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take2] Add plugin interface to LD, respun. [1..6/6]

Richard Henderson-2
On 10/11/2010 11:15 AM, Dave Korn wrote:
>> With --enable-plugins (x86_64-linux):
>
>   Err, I think the first thing I have to do is figure out why --enable-plugins
> makes a difference at all considering I removed the AC_ARG_ENABLE!  It should
> all be automatic based on the availability of dlfcn.h, and the linkability of
> dlopen/dlsym/dlclose.

I believe I wrote this before I figured out that I had to manually
re-generate the autofoo files.  Then I forgot to edit it.  But the
point still stands about the variables being unused.


r~
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take2] Add plugin interface to LD, respun. [1..6/6]

Ralf Wildenhues
* Richard Henderson wrote on Mon, Oct 11, 2010 at 07:58:51PM CEST:

> On 10/11/2010 11:15 AM, Dave Korn wrote:
> >> With --enable-plugins (x86_64-linux):
> >
> >   Err, I think the first thing I have to do is figure out why
> >   --enable-plugins makes a difference at all considering I removed
> >   the AC_ARG_ENABLE!  It should all be automatic based on the
> >   availability of dlfcn.h, and the linkability of
> >   dlopen/dlsym/dlclose.
>
> I believe I wrote this before I figured out that I had to manually
> re-generate the autofoo files.  Then I forgot to edit it.  But the
> point still stands about the variables being unused.

FWIW, you could configure --enable-maintainer-mode in order to not have
to think about this.  That's what I do, at least.  It does, however,
cause lots of spurious .pot file changes and similar.

Cheers,
Ralf
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take2] Add plugin interface to LD, respun. [1..6/6]

Richard Henderson-2
On 10/11/2010 11:01 AM, Ralf Wildenhues wrote:
> FWIW, you could configure --enable-maintainer-mode in order to not have
> to think about this.  That's what I do, at least.  It does, however,
> cause lots of spurious .pot file changes and similar.

I hardly ever have the exactly correct versions of autofoo in
my path.  Thus I almost always want to regenerate by hand.


r~
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take2] Add plugin interface to LD, respun. [1..6/6]

Dave Korn-9
In reply to this post by Richard Henderson-2
On 11/10/2010 18:58, Richard Henderson wrote:

> On 10/11/2010 11:15 AM, Dave Korn wrote:
>>> With --enable-plugins (x86_64-linux):
>>   Err, I think the first thing I have to do is figure out why --enable-plugins
>> makes a difference at all considering I removed the AC_ARG_ENABLE!  It should
>> all be automatic based on the availability of dlfcn.h, and the linkability of
>> dlopen/dlsym/dlclose.
>
> I believe I wrote this before I figured out that I had to manually
> re-generate the autofoo files.  Then I forgot to edit it.  But the
> point still stands about the variables being unused.

  Yes, agreed.  I followed the standard practice of not posting diffs for the
generated files, but could always leave them in next time I repost if you'd
prefer.

    cheers,
      DaveK
Reply | Threaded
Open this post in threaded view
|

[PATCH,take3] Add plugin interface to LD, respun. [1..6/6]

Dave Korn-9
In reply to this post by Richard Henderson-2
On 11/10/2010 18:22, Richard Henderson wrote:

> ../../git-binu/ld/ldmain.c: In function ‘add_archive_element’:
> ../../git-binu/ld/ldmain.c:800: error: unused variable ‘fildes’
> ../../git-binu/ld/ldmain.c:796: error: unused parameter ‘subsbfd’
>
> which can be fixed in the obvious way.

  Fixed in the obvious way.

> It may be easier to avoid libiberty in the test plugin than to
> work out how to coordinate -fpic over there without libtool too.

  Avoided.

> Please move that down to the use and add { }.

  Moved to the end and { } added.

  Plus replaced concat-and-free combination by ACONCAT.

  I found two further problems.  The ld static bootstrap test failed:

> Executing on host: sh -c {/gnu/binutils/obj.patch/ld/ld-new -m elf_i386  -o tmpdir/ld1 -dynamic-linker /lib/ld-linux.so.2 /usr/lib/crt1.o /usr/lib/crti.o /opt/gold/lib/gcc/i686-pc-linux-gnu/4.6.0/crtb
> egin.o --static tmpdir/ld-partial.o ../bfd/.libs/libbfd.a ../libiberty/libiberty.a   -lz -ldl  -L/usr/lib --start-group /opt/gold/lib/gcc/i686-pc-linux-gnu/4.6.0/libgcc.a /opt/gold/lib/gcc/i686-pc-lin
> ux-gnu/4.6.0/libgcc_eh.a -lc --end-group /opt/gold/lib/gcc/i686-pc-linux-gnu/4.6.0/crtend.o /usr/lib/crtn.o 2>&1}  /dev/null ld.tmp (timeout = 300)
> tmpdir/ld-partial.o: In function `plugin_opt_plugin':
> /gnu/binutils/src/ld/plugin.c:160: warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
> tmpdir/ld-partial.o: In function `plugin_opt_plugin':
> /gnu/binutils/src/ld/plugin.c:160: warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
> FAIL: bootstrap with --static

... and as this is caused by a warning symbol in the static libdl.a, I don't
think there's any way to turn it off, so I decided the best bet would be to
UNSUPPORTED that test for plugin-enabled builds.  Fixed in bootstrap.exp which
already skips the static link test when plugins are enabled in BFD, by also
skipping the test when plugins are enabled in LD.

  Secondly, ...

> Running /gnu/binutils/src/ld/testsuite/ld-pe/pe.exp ...
> Running /gnu/binutils/src/ld/testsuite/ld-pie/pie.exp ...
> Running /gnu/binutils/src/ld/testsuite/ld-plugin/plugin.exp ...
> ERROR: tcl error sourcing /gnu/binutils/src/ld/testsuite/ld-plugin/plugin.exp.
> ERROR: couldn't save command result in variable
>     while executing
> "catch "exec $NM tmpdir/func.o" nm_output"
>     invoked from within
> "if { ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/main.c tmpdir/main.o]
> || ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/func.c tmpdir/func.o]
> || ![ld_co..."
>     (file "/gnu/binutils/src/ld/testsuite/ld-plugin/plugin.exp" line 64)
>     invoked from within
> "source /gnu/binutils/src/ld/testsuite/ld-plugin/plugin.exp"
>     ("uplevel" body line 1)
>     invoked from within
> "uplevel #0 source /gnu/binutils/src/ld/testsuite/ld-plugin/plugin.exp"
>     invoked from within
> "catch "uplevel #0 source $test_file_name""
> Running /gnu/binutils/src/ld/testsuite/ld-powerpc/aix52.exp ...
> Running /gnu/binutils/src/ld/testsuite/ld-powerpc/powerpc.exp ...
... this turns out not to be to do with pic in libiberty or runpaths or
anything like that at all.  It happens whenever you run "make check", but not
if you run "make check RUNTESTFLAGS=plugin.exp" (I was leaving comparing full
testsuite before-and-after runs for last thing, which is why I only just had
it now).  This is because I used a variable named "nm_output" and it's
clashing in some way with the identically-named variable in ld_compile,
depending which order things get run and hence defined in, so I cut through
the whole mess and renamed it.  (It seems you're not allowed to have
identically-named local variables in separate enclosing scopes in tcl, iiuc.)

  Rebuilt and retested without regressions on i686-pc-cygwin, also verified
that it still builds when !ENABLE_PLUGINS by hacking about my config.cache to
set "ac_cv_header_dlfcn_h=no" and running "./config.status --recheck" then
"./config.status Makefile" and building.

  Rechecked the obscure targets still build, verified it didn't cause any
regressions in those parts of their testsuites that I could run without having
cross-compilers available.

  Re-tested applying the whole series one-by-one on i686-pc-linux rebuilding
and testing at each step, again no regressions, all plugin tests pass.

  Ah, there's one last thing.  I get this non-fatal warning when building on
i686-pc-linux with --enable-64-bit-bfd:

> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I/gnu/binutils/src/ld  -I. -I/gnu/binutils/src/ld -I../bfd -I/gnu/binutils/src/ld/../bfd -I/gnu/binutils/src/ld/../include  -g -O2 -DENABLE_PLUGINS -DLOCALEDIR="\"/opt/gold/share/locale\""  -g -O2 -g -O2 -MT libldtestplug_la-testplug.lo -MD -MP -MF .deps/libldtestplug_la-testplug.Tpo -c -o libldtestplug_la-testplug.lo `test -f 'testplug.c' || echo '/gnu/binutils/src/ld/'`testplug.c
> libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I/gnu/binutils/src/ld -I. -I/gnu/binutils/src/ld -I../bfd -I/gnu/binutils/src/ld/../bfd -I/gnu/binutils/src/ld/../include -g -O2 -DENABLE_PLUGINS -DLOCALEDIR=\"/opt/gold/share/locale\" -g -O2 -g -O2 -MT libldtestplug_la-testplug.lo -MD -MP -MF .deps/libldtestplug_la-testplug.Tpo -c /gnu/binutils/src/ld/testplug.c  -fPIC -DPIC -o .libs/libldtestplug_la-testplug.o
> /gnu/binutils/src/ld/testplug.c: In function ‘dump_tv_tag’:
> /gnu/binutils/src/ld/testplug.c:371:4: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

  It comes from this code:

>    368        case LDPT_ADD_INPUT_LIBRARY:
>    369        case LDPT_SET_EXTRA_LIBRARY_PATH:
>    370          TV_MESSAGE (LDPL_INFO, "func@0x%v\n",
>    371                          (bfd_vma)(tv->tv_u.tv_message));
>    372          break;

  With 64-bit-bfd on a 32-bit host, bfd_vma is an unsigned long long (64-bit)
int, and the tv_message is a 32-bit function pointer, so the warning is true,
but it seems pretty useless to me.  What's so bad about a cast that doesn't
lose any data?  It doesn't break the build because there's no -Werror in the
test plugin's CFLAGS, and it's of no functional consequence, so I'm not sure
whether to do anything about it.

    cheers,
      DaveK


ld-plugin-api-1-infra.diff (54K) Download Attachment
ld-plugin-api-2-claimfiles-addsyms.diff (33K) Download Attachment
ld-plugin-api-3-get-symbols.diff (15K) Download Attachment
ld-plugin-api-4-add-files.diff (15K) Download Attachment
ld-plugin-api-5-elf-vis.diff (3K) Download Attachment
ld-plugin-api-6-libs.diff (35K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take3] Add plugin interface to LD, respun. [1..6/6]

H.J. Lu-30
On Tue, Oct 12, 2010 at 9:22 PM, Dave Korn <[hidden email]> wrote:

>  Ah, there's one last thing.  I get this non-fatal warning when building on
> i686-pc-linux with --enable-64-bit-bfd:
>
>> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I/gnu/binutils/src/ld  -I. -I/gnu/binutils/src/ld -I../bfd -I/gnu/binutils/src/ld/../bfd -I/gnu/binutils/src/ld/../include  -g -O2 -DENABLE_PLUGINS -DLOCALEDIR="\"/opt/gold/share/locale\""  -g -O2 -g -O2 -MT libldtestplug_la-testplug.lo -MD -MP -MF .deps/libldtestplug_la-testplug.Tpo -c -o libldtestplug_la-testplug.lo `test -f 'testplug.c' || echo '/gnu/binutils/src/ld/'`testplug.c
>> libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I/gnu/binutils/src/ld -I. -I/gnu/binutils/src/ld -I../bfd -I/gnu/binutils/src/ld/../bfd -I/gnu/binutils/src/ld/../include -g -O2 -DENABLE_PLUGINS -DLOCALEDIR=\"/opt/gold/share/locale\" -g -O2 -g -O2 -MT libldtestplug_la-testplug.lo -MD -MP -MF .deps/libldtestplug_la-testplug.Tpo -c /gnu/binutils/src/ld/testplug.c  -fPIC -DPIC -o .libs/libldtestplug_la-testplug.o
>> /gnu/binutils/src/ld/testplug.c: In function ‘dump_tv_tag’:
>> /gnu/binutils/src/ld/testplug.c:371:4: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>
>  It comes from this code:
>
>>    368        case LDPT_ADD_INPUT_LIBRARY:
>>    369        case LDPT_SET_EXTRA_LIBRARY_PATH:
>>    370          TV_MESSAGE (LDPL_INFO, "func@0x%v\n",
>>    371                          (bfd_vma)(tv->tv_u.tv_message));
>>    372          break;
>
>  With 64-bit-bfd on a 32-bit host, bfd_vma is an unsigned long long (64-bit)
> int, and the tv_message is a 32-bit function pointer, so the warning is true,
> but it seems pretty useless to me.  What's so bad about a cast that doesn't
> lose any data?  It doesn't break the build because there's no -Werror in the
> test plugin's CFLAGS, and it's of no functional consequence, so I'm not sure
> whether to do anything about it.

You can cast pointer to uintptr_t/intptr_t first.


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take3] Add plugin interface to LD, respun. [1..6/6]

Dave Korn-9
On 13/10/2010 05:38, H.J. Lu wrote:

> On Tue, Oct 12, 2010 at 9:22 PM, Dave Korn <[hidden email]> wrote:
>>  Ah, there's one last thing.  I get this non-fatal warning when building on
>> i686-pc-linux with --enable-64-bit-bfd:
>>
>>> /bin/sh ./libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I/gnu/binutils/src/ld  -I. -I/gnu/binutils/src/ld -I../bfd -I/gnu/binutils/src/ld/../bfd -I/gnu/binutils/src/ld/../include  -g -O2 -DENABLE_PLUGINS -DLOCALEDIR="\"/opt/gold/share/locale\""  -g -O2 -g -O2 -MT libldtestplug_la-testplug.lo -MD -MP -MF .deps/libldtestplug_la-testplug.Tpo -c -o libldtestplug_la-testplug.lo `test -f 'testplug.c' || echo '/gnu/binutils/src/ld/'`testplug.c
>>> libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I/gnu/binutils/src/ld -I. -I/gnu/binutils/src/ld -I../bfd -I/gnu/binutils/src/ld/../bfd -I/gnu/binutils/src/ld/../include -g -O2 -DENABLE_PLUGINS -DLOCALEDIR=\"/opt/gold/share/locale\" -g -O2 -g -O2 -MT libldtestplug_la-testplug.lo -MD -MP -MF .deps/libldtestplug_la-testplug.Tpo -c /gnu/binutils/src/ld/testplug.c  -fPIC -DPIC -o .libs/libldtestplug_la-testplug.o
>>> /gnu/binutils/src/ld/testplug.c: In function ‘dump_tv_tag’:
>>> /gnu/binutils/src/ld/testplug.c:371:4: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>>  It comes from this code:
>>
>>>    368        case LDPT_ADD_INPUT_LIBRARY:
>>>    369        case LDPT_SET_EXTRA_LIBRARY_PATH:
>>>    370          TV_MESSAGE (LDPL_INFO, "func@0x%v\n",
>>>    371                          (bfd_vma)(tv->tv_u.tv_message));
>>>    372          break;
>>  With 64-bit-bfd on a 32-bit host, bfd_vma is an unsigned long long (64-bit)
>> int, and the tv_message is a 32-bit function pointer, so the warning is true,
>> but it seems pretty useless to me.  What's so bad about a cast that doesn't
>> lose any data?  It doesn't break the build because there's no -Werror in the
>> test plugin's CFLAGS, and it's of no functional consequence, so I'm not sure
>> whether to do anything about it.
>
> You can cast pointer to uintptr_t/intptr_t first.

  That's not C90 though.  I couldn't find it used anywhere else in
bfd/binutils/gas/ld, only gold.  I think that means I shouldn't use it.

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take3] Add plugin interface to LD, respun. [1..6/6]

Dave Korn-9
On 13/10/2010 16:37, Dave Korn wrote:
> On 13/10/2010 05:38, H.J. Lu wrote:
>> On Tue, Oct 12, 2010 at 9:22 PM, Dave Korn <[hidden email]> wrote:

>>>>    368        case LDPT_ADD_INPUT_LIBRARY:
>>>>    369        case LDPT_SET_EXTRA_LIBRARY_PATH:
>>>>    370          TV_MESSAGE (LDPL_INFO, "func@0x%v\n",
>>>>    371                          (bfd_vma)(tv->tv_u.tv_message));
>>>>    372          break;
>>>  With 64-bit-bfd on a 32-bit host, bfd_vma is an unsigned long long (64-bit)
>>> int, and the tv_message is a 32-bit function pointer, so the warning is true,
>>> but it seems pretty useless to me.  What's so bad about a cast that doesn't
>>> lose any data?  It doesn't break the build because there's no -Werror in the
>>> test plugin's CFLAGS, and it's of no functional consequence, so I'm not sure
>>> whether to do anything about it.
>> You can cast pointer to uintptr_t/intptr_t first.
>
>   That's not C90 though.  I couldn't find it used anywhere else in
> bfd/binutils/gas/ld, only gold.  I think that means I shouldn't use it.

  I think the cleanest solution would be to add a "%p" type to
ldmisc.c/vfinfo(), and cast it to (void*) in the TV_MESSAGE call.  I'll do that.

    cheers,
      DaveK

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take3] Add plugin interface to LD, respun. [1..6/6]

Dave Korn-9
On 13/10/2010 16:47, Dave Korn wrote:

>
>   I think the cleanest solution would be to add a "%p" type to
> ldmisc.c/vfinfo(), and cast it to (void*) in the TV_MESSAGE call.  I'll do that.


  I did that.  It worked.  I won't repost the whole 165kb patchset unless
anyone specifically wants me to; the attached file shows the diff-of-diffs
across the whole set.  (It applies cleanly to the last set of patches I posted
if anyone wants to update their copy.)

    cheers,
      DaveK
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH,take3] Add plugin interface to LD, respun. [1..6/6]

Richard Henderson-2
In reply to this post by Dave Korn-9
On 10/12/2010 09:22 PM, Dave Korn wrote:
> ... this turns out not to be to do with pic in libiberty or runpaths or
> anything like that at all.  It happens whenever you run "make check", but not
> if you run "make check RUNTESTFLAGS=plugin.exp" (I was leaving comparing full
> testsuite before-and-after runs for last thing, which is why I only just had
> it now).  This is because I used a variable named "nm_output" and it's
> clashing in some way with the identically-named variable in ld_compile,
> depending which order things get run and hence defined in, so I cut through
> the whole mess and renamed it.  (It seems you're not allowed to have
> identically-named local variables in separate enclosing scopes in tcl, iiuc.)

*shudder* I knew I hated tcl.

Re %p downthread, I agree that's the best solution.

Regarding the actual patch commit, I don't think it's worth keeping
the patches separate.  You can commit them in one go if you like.

Patches ok.


r~