[LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[Markos Chandras]

Commit 6f22494d19b605ded308dc0fa713e91cb873f44a
"Move sctp to utils and bump it to 1.0.15"

introduced a build failure for the prefixed architectures.
We need to take into consideration the __USER_LABEL_PREFIX__
for architectures that define it when creating symbol aliases.

The following upstream patch (0600c8968cc2dea04cbf13ec739216e2939d08fe)
fixes the build for the Meta(metag) architecture

[...]
utils/sctp/func_tests/test_connectx.c:151: undefined reference to `_sctp_connectx'
utils/sctp/func_tests/test_connectx.c:163: undefined reference to `_sctp_connectx'
[...]

Build tested on x86_64 and metag.

Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
Upstream:
https://github.com/borkmann/lksctp-tools/commit/0600c8968cc2dea04cbf13ec739216e2939d08fe
---
 utils/sctp/lib/connectx.c | 11 +++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/utils/sctp/lib/connectx.c b/utils/sctp/lib/connectx.c
index 50cf4c8..8e5a1f8 100644
--- a/utils/sctp/lib/connectx.c
+++ b/utils/sctp/lib/connectx.c
@@ -179,7 +179,10 @@ int sctp_connectx3(int fd, struct sockaddr *addrs, int addrcnt,
 	return __connectx(fd, addrs, addrs_size, id);
 }
 
-__asm__(".symver __sctp_connectx, sctp_connectx@");
-__asm__(".symver sctp_connectx_orig, sctp_connectx@VERS_1");
-__asm__(".symver sctp_connectx2, sctp_connectx@VERS_2");
-__asm__(".symver sctp_connectx3, sctp_connectx@@VERS_3");
+#define __SYMPFX(pfx, sym) #pfx sym
+#define _SYMPFX(pfx, sym) __SYMPFX(pfx, sym)
+#define SYMPFX(sym) _SYMPFX(__USER_LABEL_PREFIX__, #sym)
+#define SYMVER(name, name2) __asm__(".symver " SYMPFX(name) "," SYMPFX(name2))
+
+SYMVER(__sctp_connectx, sctp_connectx@);
+SYMVER(sctp_connectx_orig, sctp_connectx@VERS_1);
+SYMVER(sctp_connectx2, sctp_connectx@VERS_2);
+SYMVER(sctp_connectx3, sctp_connectx@@VERS_3);
-- 
1.8.5.1



------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 1001 bytes --]

On Friday 13 December 2013 07:31:02 Markos Chandras wrote:
> Commit 6f22494d19b605ded308dc0fa713e91cb873f44a
> "Move sctp to utils and bump it to 1.0.15"
> 
> introduced a build failure for the prefixed architectures.
> We need to take into consideration the __USER_LABEL_PREFIX__
> for architectures that define it when creating symbol aliases.
> 
> The following upstream patch (0600c8968cc2dea04cbf13ec739216e2939d08fe)
> fixes the build for the Meta(metag) architecture
> 
> [...]
> utils/sctp/func_tests/test_connectx.c:151: undefined reference to
> `_sctp_connectx' utils/sctp/func_tests/test_connectx.c:163: undefined
> reference to `_sctp_connectx' [...]
> 
> Build tested on x86_64 and metag.

we really should fix this instead by not using any symbol renaming.  symbol 
versions might make sense when dealing with a released project and a stable 
shared library API, but it makes no sense in LTP.

perhaps better to have a compile time knob to disable it all ?
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 455 bytes --]

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[Markos Chandras]

On 12/16/2013 06:58 AM, Mike Frysinger wrote:
> On Friday 13 December 2013 07:31:02 Markos Chandras wrote:
>> Commit 6f22494d19b605ded308dc0fa713e91cb873f44a
>> "Move sctp to utils and bump it to 1.0.15"
>>
>> introduced a build failure for the prefixed architectures.
>> We need to take into consideration the __USER_LABEL_PREFIX__
>> for architectures that define it when creating symbol aliases.
>>
>> The following upstream patch (0600c8968cc2dea04cbf13ec739216e2939d08fe)
>> fixes the build for the Meta(metag) architecture
>>
>> [...]
>> utils/sctp/func_tests/test_connectx.c:151: undefined reference to
>> `_sctp_connectx' utils/sctp/func_tests/test_connectx.c:163: undefined
>> reference to `_sctp_connectx' [...]
>>
>> Build tested on x86_64 and metag.
>
> we really should fix this instead by not using any symbol renaming.  symbol
> versions might make sense when dealing with a released project and a stable
> shared library API, but it makes no sense in LTP.
>
> perhaps better to have a compile time knob to disable it all ?
> -mike
>
Well whatever works best. Personally I would avoid deviating from 
third-party code unless it's really necessary.

-- 
markos


------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 2140 bytes --]

On Friday 20 December 2013 07:45:17 Markos Chandras wrote:
> On 12/16/2013 06:58 AM, Mike Frysinger wrote:
> > On Friday 13 December 2013 07:31:02 Markos Chandras wrote:
> >> Commit 6f22494d19b605ded308dc0fa713e91cb873f44a
> >> "Move sctp to utils and bump it to 1.0.15"
> >> 
> >> introduced a build failure for the prefixed architectures.
> >> We need to take into consideration the __USER_LABEL_PREFIX__
> >> for architectures that define it when creating symbol aliases.
> >> 
> >> The following upstream patch (0600c8968cc2dea04cbf13ec739216e2939d08fe)
> >> fixes the build for the Meta(metag) architecture
> >> 
> >> [...]
> >> utils/sctp/func_tests/test_connectx.c:151: undefined reference to
> >> `_sctp_connectx' utils/sctp/func_tests/test_connectx.c:163: undefined
> >> reference to `_sctp_connectx' [...]
> >> 
> >> Build tested on x86_64 and metag.
> > 
> > we really should fix this instead by not using any symbol renaming. 
> > symbol versions might make sense when dealing with a released project
> > and a stable shared library API, but it makes no sense in LTP.
> > 
> > perhaps better to have a compile time knob to disable it all ?
> > -mike
> 
> Well whatever works best. Personally I would avoid deviating from
> third-party code unless it's really necessary.

sure, but i'm not sure this change covers everything

there's still:
extern int sctp_connectx_orig (int)
    __attribute ((alias ("__sctp_connectx")));

i vaguely recall gcc doesn't add symbol prefixes to the alias attribute.  the 
documentation implies that too:

The alias attribute causes the declaration to be emitted as an alias for 
another symbol, which must be specified. For instance,
          void __f () { /* Do something. */; }
          void f () __attribute__ ((weak, alias ("__f")));
defines ‘f’ to be a weak alias for ‘__f’. In C++, the mangled name for the 
target must be used. It is an error if ‘__f’ is not defined in the same 
translation unit.

since this still needs fixing upstream, might be a good time to also include a 
knob to turn this crap off entirely ? :)
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 455 bytes --]

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[chrubis]

Hi!
> sure, but i'm not sure this change covers everything
> 
> there's still:
> extern int sctp_connectx_orig (int)
>     __attribute ((alias ("__sctp_connectx")));
> 
> i vaguely recall gcc doesn't add symbol prefixes to the alias attribute.  the 
> documentation implies that too:
> 
> The alias attribute causes the declaration to be emitted as an alias for 
> another symbol, which must be specified. For instance,
>           void __f () { /* Do something. */; }
>           void f () __attribute__ ((weak, alias ("__f")));
> defines ???f??? to be a weak alias for ???__f???. In C++, the mangled name for the 
> target must be used. It is an error if ???__f??? is not defined in the same 
> translation unit.
> 
> since this still needs fixing upstream, might be a good time to also include a 
> knob to turn this crap off entirely ? :)

What is the status? This is a build failure and so should be fixed
before the release which should happen soon enough (generally right
after we get rid of build failures on older distros and exotic
hardware).

We have a few options here:

* Apply the patch that is not clean but was applied in upstream

* Disable the sctp build temporarily

* Fix this correctly (and I'm not going to do this myself)

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[chrubis]

Hi!
Unfortunatelly after the patch is applied the build on x86_64 fails
(you have to do a make clean before the test to get rid of object files)

gcc -g -O2 -g -O2 -fno-strict-aliasing -pipe -Wall  -I/home/metan/Work/git/ltp-dev/testcases/kernel/include -I/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/../include -I/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/../testlib -DLTP -I../../../include -I../../../include   -L/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/../lib -L/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/../testlib -L../../../lib  test_connectx.c   -lltp -lsctputil -lsctp -lpthread -o test_connectx
/tmp/cc8twJXc.o: In function `main':
/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/test_connectx.c:151: undefined reference to `sctp_connectx'
/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/test_connectx.c:163: undefined reference to `sctp_connectx'
/tmp/cc8twJXc.o: In function `test_connectx':
/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/../testlib/sctputil.h:175: undefined reference to `sctp_connectx'
/tmp/cc8twJXc.o: In function `main':
/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/test_connectx.c:234: undefined reference to `sctp_connectx'
/home/metan/Work/git/ltp-dev/utils/sctp/func_tests/test_connectx.c:245: undefined reference to `sctp_connectx'
collect2: error: ld returned 1 exit status
make[1]: *** [test_connectx] Error 1
make[1]: Leaving directory `/home/metan/Work/git/ltp-dev/utils/sctp/func_tests'
make: *** [all] Error 2

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[chrubis]

Hi!
> if it's in upstream, then i don't see a problem merging it into ltp.  it isn't 
> 100% correct, but it still seems better than the status quo.

And seems like you were right, I've tried the patch and the build fails
for me on x86_64.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH v2] utils: sctp: Fix build for prefixed architectures

[chrubis]

Hi!
> I am also able to build the upstream code directly from their repository.
> 
> Could you please double check the patch you applied? :)

And indeed it was misapplied, for same strange reason the 'git am' dropped
last two lines of the patch. Which keeps me wonder how many patches has
been corrupted silently :(

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today. 
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list