public inbox for prelink@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am
@ 2013-05-07 12:34 Tom de Vries
  2013-05-07 12:40 ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2013-05-07 12:34 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: prelink

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]

Jakub,

this patch allows me to run all the tests successfully on x86_64 with
LINKOPTS="-Wl,--no-as-needed" set in the environment.

When I built prelink on x86_64 and ran the tests, I found I had a number of test
failures. I investigated a failure (tls5) and found that my compiler uses
--as-needed by default, and that using --no-as-needed allowed the test to pass.

Rerunning with LINKOPTS="-Wl,--no-as-needed" allowed 3 more tests to pass, but
still a number of tests were failing. I investigated another test (cycle1) and
found the reason for failure was that LINKOPTS was not passed to CCLINK and
CXXLINK in the makefile.

This patch fixes that. Build and tested on x86_64.

OK?

Thanks,
- Tom

2013-05-07  Tom de Vries  <tom@codesourcery.com>

	* testsuite/Makefile.am (TESTS_ENVIRONMENT): Add LINKOPTS to CCLINK and
	CXXLINK.
	* testsuite/Makefile.in: Propagate.


[-- Attachment #2: cclink.patch --]
[-- Type: text/x-patch, Size: 1554 bytes --]

diff --git a/trunk/testsuite/Makefile.am b/trunk/testsuite/Makefile.am
index b80eb48..5b22934 100644
--- a/trunk/testsuite/Makefile.am
+++ b/trunk/testsuite/Makefile.am
@@ -19,8 +19,8 @@ TESTS = movelibs.sh \
 	undosyslibs.sh
 TESTS_ENVIRONMENT = \
 	PRELINK="../src/prelink -c ./prelink.conf -C ./prelink.cache --ld-library-path=. --dynamic-linker=`echo ./ld*.so.*[0-9]`" \
-	CC="$(CC) $(LINKOPTS)" CCLINK="$(CC) -Wl,--dynamic-linker=`echo ./ld*.so.*[0-9]`" \
-	CXX="$(CXX) $(LINKOPTS)" CXXLINK="$(CXX) -Wl,--dynamic-linker=`echo ./ld*.so.*[0-9]`" \
+	CC="$(CC) $(LINKOPTS)" CCLINK="$(CC) $(LINKOPTS) -Wl,--dynamic-linker=`echo ./ld*.so.*[0-9]`" \
+	CXX="$(CXX) $(LINKOPTS)" CXXLINK="$(CXX) $(LINKOPTS) -Wl,--dynamic-linker=`echo ./ld*.so.*[0-9]`" \
 	$(SHELL)
 
 extra_DIST = $(TESTS) functions.sh
diff --git a/trunk/testsuite/Makefile.in b/trunk/testsuite/Makefile.in
index 84e0ea2..e3d48cb 100644
--- a/trunk/testsuite/Makefile.in
+++ b/trunk/testsuite/Makefile.in
@@ -115,8 +115,8 @@ TESTS = movelibs.sh \
 
 TESTS_ENVIRONMENT = \
 	PRELINK="../src/prelink -c ./prelink.conf -C ./prelink.cache --ld-library-path=. --dynamic-linker=`echo ./ld*.so.*[0-9]`" \
-	CC="$(CC) $(LINKOPTS)" CCLINK="$(CC) -Wl,--dynamic-linker=`echo ./ld*.so.*[0-9]`" \
-	CXX="$(CXX) $(LINKOPTS)" CXXLINK="$(CXX) -Wl,--dynamic-linker=`echo ./ld*.so.*[0-9]`" \
+	CC="$(CC) $(LINKOPTS)" CCLINK="$(CC) $(LINKOPTS) -Wl,--dynamic-linker=`echo ./ld*.so.*[0-9]`" \
+	CXX="$(CXX) $(LINKOPTS)" CXXLINK="$(CXX) $(LINKOPTS) -Wl,--dynamic-linker=`echo ./ld*.so.*[0-9]`" \
 	$(SHELL)
 
 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am
  2013-05-07 12:34 [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am Tom de Vries
@ 2013-05-07 12:40 ` Jakub Jelinek
  2013-05-07 14:22   ` Tom de Vries
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2013-05-07 12:40 UTC (permalink / raw)
  To: Tom de Vries; +Cc: prelink

On Tue, May 07, 2013 at 02:33:52PM +0200, Tom de Vries wrote:
> this patch allows me to run all the tests successfully on x86_64 with
> LINKOPTS="-Wl,--no-as-needed" set in the environment.

That shouldn't be needed anymore, are you using latest SVN prelink?
Before those testsuite changes, one could just do
make check CC='gcc -Wl,--no-as-needed' CXX='g++ -Wl,--no-as-needed'
but as I said, it should now work as is.

	Jakub

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am
  2013-05-07 12:40 ` Jakub Jelinek
@ 2013-05-07 14:22   ` Tom de Vries
  2013-05-07 14:28     ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2013-05-07 14:22 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: prelink

On 07/05/13 14:40, Jakub Jelinek wrote:
> On Tue, May 07, 2013 at 02:33:52PM +0200, Tom de Vries wrote:
>> this patch allows me to run all the tests successfully on x86_64 with
>> LINKOPTS="-Wl,--no-as-needed" set in the environment.
> 
> That shouldn't be needed anymore, are you using latest SVN prelink?

Jakub,

I'm using r205, dated 2013-05-03.

The failures I see with that version are:
...
FAIL: layout1.sh
FAIL: layout2.sh
FAIL: tls5.sh
FAIL: tls6.sh
FAIL: cxx1.sh
FAIL: cxx2.sh
FAIL: cxx3.sh
FAIL: quick1.sh
FAIL: quick2.sh
FAIL: cycle1.sh
FAIL: cycle2.sh
...

Running tls5 by hand gives:
...
$ ../../src/testsuite/tls5.sh  ; echo $?
../src/prelink: tls5lib3.so does not have .gnu.prelink_undo section
4
...

Without --no-as-needed, tls5lib1.so is not dependent on tls5lib3.so, so the
prelink command ignores it:
...
../src/prelink -c ./prelink.conf -C ./prelink.cache --ld-library-path=.
--dynamic-linker=./ld-linux-x86-64.so.2 -vm ./tls5
Laying out 2 libraries in virtual address space 0000003000000000-0000004000000000
Assigned virtual address space slots for libraries:
./ld-linux-x86-64.so.2
0000003000000000-00000030002242c8
./tls5lib1.so
0000003000400000-0000003000602088
./tls5lib2.so
0000003000800000-0000003000a01030
./libc.so.6
0000003000c00000-0000003000fbe4d8
Prelinking /home/vries/upstream/prelink/builds/prelink/build/testsuite/tls5lib2.so
Prelinking /home/vries/upstream/prelink/builds/prelink/build/testsuite/tls5lib1.so
Prelinking /home/vries/upstream/prelink/builds/prelink/build/testsuite/tls5
...
and the undo of the prelink of tls5lib3.so fails.

Setting LINKOPTS with --no-as-needed allows the test to pass:
...
$ LINKOPTS="-Wl,--no-as-needed" ../../src/testsuite/tls5.sh  ; echo $?
0
...

> Before those testsuite changes, one could just do
> make check CC='gcc -Wl,--no-as-needed' CXX='g++ -Wl,--no-as-needed'

That works also for me.

I used LINKOPTS because I read functions.sh and saw how it's used there (CC
includes LINKOPTS, CCLINK includes CC which includes LINKOPTS). It made sense to
me to use LINKOPTS because --no-as-needed it is a link opt, and we need to pass
it only once (instead of in both CC and CXX).

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am
  2013-05-07 14:22   ` Tom de Vries
@ 2013-05-07 14:28     ` Jakub Jelinek
  2013-05-07 14:42       ` Tom de Vries
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2013-05-07 14:28 UTC (permalink / raw)
  To: Tom de Vries; +Cc: prelink

On Tue, May 07, 2013 at 04:21:55PM +0200, Tom de Vries wrote:
> I'm using r205, dated 2013-05-03.

Yes, that is the latest.

> The failures I see with that version are:
> ...
> FAIL: layout1.sh
> FAIL: layout2.sh
> FAIL: tls5.sh
> FAIL: tls6.sh
> FAIL: cxx1.sh
> FAIL: cxx2.sh
> FAIL: cxx3.sh
> FAIL: quick1.sh
> FAIL: quick2.sh
> FAIL: cycle1.sh
> FAIL: cycle2.sh
> ...

No such failures here.  What linker are you using?

	Jakub

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am
  2013-05-07 14:28     ` Jakub Jelinek
@ 2013-05-07 14:42       ` Tom de Vries
  2013-05-07 14:47         ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2013-05-07 14:42 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: prelink

On 07/05/13 16:28, Jakub Jelinek wrote:
> On Tue, May 07, 2013 at 04:21:55PM +0200, Tom de Vries wrote:
>> I'm using r205, dated 2013-05-03.
> 
> Yes, that is the latest.
> 
>> The failures I see with that version are:
>> ...
>> FAIL: layout1.sh
>> FAIL: layout2.sh
>> FAIL: tls5.sh
>> FAIL: tls6.sh
>> FAIL: cxx1.sh
>> FAIL: cxx2.sh
>> FAIL: cxx3.sh
>> FAIL: quick1.sh
>> FAIL: quick2.sh
>> FAIL: cycle1.sh
>> FAIL: cycle2.sh
>> ...
> 
> No such failures here.  What linker are you using?
> 
> 	Jakub
> 

...
$ ld --version
GNU ld (GNU Binutils for Ubuntu) 2.22
...

I'll try to build and run the tests using a build of upstream binutils, and see
if that fixes it.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am
  2013-05-07 14:42       ` Tom de Vries
@ 2013-05-07 14:47         ` Jakub Jelinek
  2013-05-07 17:10           ` Tom de Vries
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2013-05-07 14:47 UTC (permalink / raw)
  To: Tom de Vries; +Cc: prelink

On Tue, May 07, 2013 at 04:42:18PM +0200, Tom de Vries wrote:
> $ ld --version
> GNU ld (GNU Binutils for Ubuntu) 2.22
> ...
> 
> I'll try to build and run the tests using a build of upstream binutils, and see
> if that fixes it.

The only issue I was aware of were ifunc*.sh failures (caused by some
recentish ifunc handling change in ld.bfd, fixed recently), but that was
just a few months, and also another December 2012ish ld change that broke
the --as-needed handling (the recent testsuite changes should fix that up).

	Jakub

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am
  2013-05-07 14:47         ` Jakub Jelinek
@ 2013-05-07 17:10           ` Tom de Vries
  0 siblings, 0 replies; 7+ messages in thread
From: Tom de Vries @ 2013-05-07 17:10 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: prelink

On 07/05/13 16:47, Jakub Jelinek wrote:
> On Tue, May 07, 2013 at 04:42:18PM +0200, Tom de Vries wrote:
>> $ ld --version
>> GNU ld (GNU Binutils for Ubuntu) 2.22
>> ...
>>
>> I'll try to build and run the tests using a build of upstream binutils, and see
>> if that fixes it.
> 
> The only issue I was aware of were ifunc*.sh failures (caused by some
> recentish ifunc handling change in ld.bfd, fixed recently), but that was
> just a few months, and also another December 2012ish ld change that broke
> the --as-needed handling (the recent testsuite changes should fix that up).
> 

The failures where not due to ld, but due to ubuntu gcc, which passes
--as-needed to the linker by default.

Using a gcc build from upstream, all the tests pass.

The failures I saw can still be reproduced with upstream gcc by setting
--as-needed like this:
...
$ make check CC='gcc -Wl,--as-needed' CXX='g++ -Wl,--as-needed'
...

I think the changes in the testsuite that you were referring to earlier in this
thread were related to --add-needed/--no-add-needed, not
--as-needed/--no-as-needed. The option names -add-needed/--no-add-needed have
already been deprecated for being too much like -as-needed/--no-as-needed.

Thanks,
- Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-05-07 17:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-07 12:34 [PATCH] Add LINKOPTS to CCLINK and CXXLINK in testsuite/Makefile.am Tom de Vries
2013-05-07 12:40 ` Jakub Jelinek
2013-05-07 14:22   ` Tom de Vries
2013-05-07 14:28     ` Jakub Jelinek
2013-05-07 14:42       ` Tom de Vries
2013-05-07 14:47         ` Jakub Jelinek
2013-05-07 17:10           ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).