Fixes to build under Xcode12 for iOS ARM64 #593

agnickolov · 2021-06-14T22:02:30Z

Typecasts
Added bundle identifiers to executables

paullouisageneau · 2021-06-20T09:17:54Z

@tuexen Would you consider the changes? I'd be useful for libdatachannel.

Thanks for your work on usrsctp!

tuexen · 2021-06-20T12:22:55Z

Sure. Most of them are related to implicit conversions from time_t and size_t to 32-bit integers, right? Need to go through the changes, possibly changing some types of variables.

paullouisageneau · 2021-06-21T15:11:04Z

@tuexen Indeed, and a property added in CMakeLists. Thanks.

tuexen · 2021-06-21T16:05:20Z

@weinrank Are you fine with the cmake related change?

weinrank · 2021-06-21T16:25:50Z

You're setting these properties for all platforms.
Can you provide a condition that only relevant platform get these properties?

Best
Felix

agnickolov · 2021-06-22T04:31:58Z

The CMake property is only used by the Xcode generator. It only applies to iOS and OS X with no adverse effect on either platform.

paullouisageneau · 2021-06-29T20:14:56Z

Any news on this? @agnickolov could you introduce a if(IOS) condition anyway?

tuexen · 2021-07-12T22:29:40Z

@weinrank What do you think about the cmake related changes?

agnickolov · 2021-07-29T18:45:31Z

Is there still anything wrong with this PR?

tuexen · 2021-07-29T19:32:57Z

No it looks good. Just need to find some time to apply this in parts to the FreeBSD code repo and then merging this...

agnickolov · 2021-08-27T06:14:44Z

Any estimate as to when can we expect this to get merged?

tuexen · 2021-08-27T06:55:53Z

Any estimate as to when can we expect this to get merged?

I plan to work on it next week.

tuexen · 2021-09-05T19:58:08Z

programs/CMakeLists.txt

+	if(APPLE)
+		set_target_properties(${source_file_we} PROPERTIES
+			XCODE_ATTRIBUTE_PRODUCT_BUNDLE_IDENTIFIER com.github.sctplab.usrsctp.${source_file_we})
+	endif()
 endforeach ()


Committed in d52bb17 with whitespace changes.

This was brought to my attention in #593

tuexen · 2021-09-05T20:53:54Z

programs/st_client.c

@@ -78,7 +78,7 @@ get_milliseconds_count(void)
 	unsigned int milliseconds;

 	gettimeofday(&tv, NULL); /* get current time */
-	milliseconds = tv.tv_sec*1000LL + tv.tv_usec/1000; /* calculate milliseconds */
+	milliseconds = (unsigned int)(tv.tv_sec*1000LL + tv.tv_usec/1000); /* calculate milliseconds */


Doesn't this always overflows nowadays? Avoiding these overflows by moving to 64-bit entities in 7077e0e.

tuexen · 2021-09-05T22:36:21Z

usrsctplib/user_recv_thread.c

@@ -32,6 +32,9 @@
 #include <sys/types.h>
 #if !defined(_WIN32)
 #include <sys/socket.h>
+#if defined(__APPLE__) && !defined(__APPLE_USE_RFC_2292)
+# define __APPLE_USE_RFC_2292
+#endif


Why can't you set this in your build environment?

This was reported in #593

tuexen · 2021-09-05T22:41:38Z

usrsctplib/netinet/sctp_input.c

@@ -2821,7 +2821,7 @@ sctp_handle_cookie_echo(struct mbuf *m, int iphlen, int offset,
 		if ((uint32_t)diff.tv_sec > UINT32_MAX / 1000000) {
 			staleness = UINT32_MAX;
 		} else {
-			staleness = diff.tv_sec * 1000000;
+			staleness = (uint32_t)diff.tv_sec * 1000000;


Fixed in 87859a5.

This is based on #593

tuexen · 2021-09-06T10:54:07Z

usrsctplib/user_recv_thread.c

@@ -346,7 +349,7 @@ recv_function_raw(void *arg)
 		msg.msg_iovlen = MAXLEN_MBUF_CHAIN;
 		msg.msg_control = NULL;
 		msg.msg_controllen = 0;
-		ncounter = n = recvmsg(SCTP_BASE_VAR(userspace_rawsctp), &msg, 0);
+		ncounter = n = (int)recvmsg(SCTP_BASE_VAR(userspace_rawsctp), &msg, 0);


Fixed in a8adc4a

tuexen · 2021-09-06T10:54:17Z

usrsctplib/user_recv_thread.c

@@ -545,7 +548,7 @@ recv_function_raw6(void *arg)
 		msg.msg_controllen = (socklen_t)CMSG_SPACE(sizeof (struct in6_pktinfo));
 		msg.msg_flags = 0;

-		ncounter = n = recvmsg(SCTP_BASE_VAR(userspace_rawsctp6), &msg, 0);
+		ncounter = n = (int)recvmsg(SCTP_BASE_VAR(userspace_rawsctp6), &msg, 0);


Fixed in a8adc4a

tuexen · 2021-09-06T10:54:27Z

usrsctplib/user_recv_thread.c

@@ -721,7 +724,7 @@ recv_function_udp(void *arg)
 		msg.msg_controllen = sizeof(cmsgbuf);
 		msg.msg_flags = 0;

-		ncounter = n = recvmsg(SCTP_BASE_VAR(userspace_udpsctp), &msg, 0);
+		ncounter = n = (int)recvmsg(SCTP_BASE_VAR(userspace_udpsctp), &msg, 0);


Fixed in a8adc4a

tuexen · 2021-09-06T10:54:35Z

usrsctplib/user_recv_thread.c

@@ -930,7 +933,7 @@ recv_function_udp6(void *arg)
 		msg.msg_controllen = (socklen_t)CMSG_SPACE(sizeof (struct in6_pktinfo));
 		msg.msg_flags = 0;

-		ncounter = n = recvmsg(SCTP_BASE_VAR(userspace_udpsctp6), &msg, 0);
+		ncounter = n = (int)recvmsg(SCTP_BASE_VAR(userspace_udpsctp6), &msg, 0);


Fixed in a8adc4a

Taken from #593

tuexen · 2021-09-06T11:05:09Z

usrsctplib/netinet/sctp_usrreq.c

@@ -3396,7 +3396,7 @@ sctp_getopt(struct socket *so, int optname, void *optval, size_t *optsize,
 				sasoc->sasoc_asocmaxrxt = inp->sctp_ep.max_send_times;
 				sasoc->sasoc_number_peer_destinations = 0;
 				sasoc->sasoc_peer_rwnd = 0;
-				sasoc->sasoc_local_rwnd = sbspace(&inp->sctp_socket->so_rcv);
+				sasoc->sasoc_local_rwnd = (uint32_t)sbspace(&inp->sctp_socket->so_rcv);


Fixed in f2efa09

This was brought up in #593

tuexen · 2021-09-07T09:43:04Z

usrsctplib/netinet/sctp_timer.c

@@ -864,7 +864,7 @@ sctp_t3rxt_timer(struct sctp_inpcb *inp,

 				(void)SCTP_GETTIME_TIMEVAL(&now);
 				if (net->last_sent_time.tv_sec) {
-					ms_goneby = (now.tv_sec - net->last_sent_time.tv_sec) * 1000;
+					ms_goneby = (unsigned int)(now.tv_sec - net->last_sent_time.tv_sec) * 1000u;


Fixed in 8463f3a.

tuexen · 2021-09-07T13:26:14Z

usrsctplib/netinet/sctputil.c

@@ -6704,10 +6704,10 @@ sctp_sorecvmsg(struct socket *so,
 					if (SCTP_BASE_SYSCTL(sctp_logging_level) & SCTP_SB_LOGGING_ENABLE) {
 						sctp_sblog(&so->so_rcv, control->do_not_ref_stcb?NULL:stcb, SCTP_LOG_SBFREE, (int)cp_len);
 					}
-					atomic_subtract_int(&so->so_rcv.sb_cc, cp_len);
+					atomic_subtract_int(&so->so_rcv.sb_cc, (int32_t)cp_len);


Fixed in 078bd24.

tuexen · 2021-09-07T13:26:20Z

usrsctplib/netinet/sctputil.c

 					if ((control->do_not_ref_stcb == 0) &&
 					    stcb) {
-						atomic_subtract_int(&stcb->asoc.sb_cc, cp_len);
+						atomic_subtract_int(&stcb->asoc.sb_cc, (int32_t)cp_len);


Fixed in 078bd24.

tuexen · 2021-09-07T13:26:26Z

usrsctplib/netinet/sctputil.c

@@ -6716,7 +6716,7 @@ sctp_sorecvmsg(struct socket *so,
 						sctp_sblog(&so->so_rcv, control->do_not_ref_stcb?NULL:stcb,
 							   SCTP_LOG_SBRESULT, 0);
 					}
-					atomic_subtract_int(&control->length, cp_len);
+					atomic_subtract_int(&control->length, (int32_t)cp_len);


Fixed in 078bd24.

Stay with 32-bit, since the chunk is only 32-bit aligned. This was reported in #593

tuexen · 2021-09-07T13:33:31Z

usrsctplib/netinet/sctp_output.c

@@ -12084,7 +12084,7 @@ sctp_send_hb(struct sctp_tcb *stcb, struct sctp_nets *net,int so_locked)
 	/* Fill out hb parameter */
 	hb->heartbeat.hb_info.ph.param_type = htons(SCTP_HEARTBEAT_INFO);
 	hb->heartbeat.hb_info.ph.param_length = htons(sizeof(struct sctp_heartbeat_info_param));
-	hb->heartbeat.hb_info.time_value_1 = now.tv_sec;
+	hb->heartbeat.hb_info.time_value_1 = (uint32_t)now.tv_sec;


Fixed in 2d65548.

Taken from #593

tuexen · 2021-09-07T13:51:26Z

usrsctplib/netinet/sctputil.c

@@ -2009,7 +2009,7 @@ sctp_timeout_handler(void *t)
 			 type, inp, stcb, net));
 		SCTP_STAT_INCR(sctps_timosecret);
 		(void)SCTP_GETTIME_TIMEVAL(&tv);
-		inp->sctp_ep.time_of_secret_change = tv.tv_sec;
+		inp->sctp_ep.time_of_secret_change = (unsigned int)tv.tv_sec;


Fixed in f2c2b60.

tuexen · 2021-09-07T13:51:48Z

usrsctplib/netinet/sctp_pcb.c

@@ -3004,7 +3004,7 @@ sctp_inpcb_alloc(struct socket *so, uint32_t vrf_id)

 	/* Setup the initial secret */
 	(void)SCTP_GETTIME_TIMEVAL(&time);
-	m->time_of_secret_change = time.tv_sec;
+	m->time_of_secret_change = (unsigned int)time.tv_sec;


Fixed in f2c2b60.

tuexen · 2021-09-08T23:27:55Z

usrsctplib/netinet/sctp_pcb.c

@@ -5311,7 +5311,7 @@ sctp_add_vtag_to_timewait(uint32_t tag, uint32_t time, uint16_t lport, uint16_t
 			if ((twait_block->vtag_block[i].v_tag == 0) &&
 			    !set) {
 				twait_block->vtag_block[i].tv_sec_at_expire =
-					now.tv_sec + time;
+					(uint32_t)now.tv_sec + time;


Should be fixed in 4ec13f7.

tuexen · 2021-09-08T23:28:02Z

usrsctplib/netinet/sctp_pcb.c

@@ -5325,7 +5325,7 @@ sctp_add_vtag_to_timewait(uint32_t tag, uint32_t time, uint16_t lport, uint16_t
 				twait_block->vtag_block[i].rport = 0;
 				if (set == 0) {
 					/* Reuse it for my new tag */
-					twait_block->vtag_block[i].tv_sec_at_expire = now.tv_sec + time;
+					twait_block->vtag_block[i].tv_sec_at_expire = (uint32_t)now.tv_sec + time;


Should be fixed in 4ec13f7.

tuexen · 2021-09-08T23:28:09Z

usrsctplib/netinet/sctp_pcb.c

@@ -5350,7 +5350,7 @@ sctp_add_vtag_to_timewait(uint32_t tag, uint32_t time, uint16_t lport, uint16_t
 		}
 		memset(twait_block, 0, sizeof(struct sctp_tagblock));
 		LIST_INSERT_HEAD(chain, twait_block, sctp_nxt_tagblock);
-		twait_block->vtag_block[0].tv_sec_at_expire = now.tv_sec + time;
+		twait_block->vtag_block[0].tv_sec_at_expire = (uint32_t)now.tv_sec + time;


Should be fixed in 4ec13f7.

tuexen · 2021-09-08T23:29:20Z

I think all issues except for the one, which I think should be fixed in the build environment, should be fixed now. Please test and report.

tuexen · 2021-09-09T17:40:35Z

@agnickolov Why can't you define __APPLE_USE_RFC_2292 in whatever build environment you use like we currently do for autotools, cmake, and meson?

agnickolov · 2021-09-09T17:48:46Z

I moved the preprocessor symbol definition into the CMake project. There was another compilation issue so I had to add another typecast too. This one was probably introduced recently. It'd be nice if an iOS build target is introduced into the build system so these are caught early on.

This was reported in #593

tuexen · 2021-09-09T18:10:40Z

usrsctplib/netinet/sctp_pcb.c

@@ -5293,7 +5293,7 @@ sctp_add_vtag_to_timewait(uint32_t tag, uint16_t lport, uint16_t rport)

 	SCTP_INP_INFO_WLOCK_ASSERT();
 	(void)SCTP_GETTIME_TIMEVAL(&now);
-	time = now.tv_sec + SCTP_BASE_SYSCTL(sctp_vtag_time_wait);
+	time = (uint32_t)(now.tv_sec + SCTP_BASE_SYSCTL(sctp_vtag_time_wait));


Fixed in 6470706

tuexen · 2021-09-09T18:21:42Z

usrsctplib/CMakeLists.txt

+if(APPLE)
+	target_compile_definitions(usrsctp PRIVATE __APPLE_USE_RFC_2292)
+endif()
+


Why doesn't work https://github.com/sctplab/usrsctp/blob/master/usrsctplib/CMakeLists.txt#L87 for you?

Ah, I didn't notice that one. You asked me to move the fix to the build system so I did. Your change should work just as fine I expect.

tuexen · 2021-09-09T18:57:09Z

I moved the preprocessor symbol definition into the CMake project. There was another compilation issue so I had to add another typecast too. This one was probably introduced recently.

Yepp, sorry I rewrote the timewait stuff (since there was dead code and locking issues) and introduced that. Sorry about that.
It is fixed.

It'd be nice if an iOS build target is introduced into the build system so these are caught early on.

You mean a buildbot instance? @weinrank can we build on the Mac OS buildbot for iOS?

tuexen · 2021-09-09T20:59:56Z

@agnickolov Does the following patch fix your build issue with respect to __APPLE_USE_RFC_2292:

diff --git a/programs/CMakeLists.txt b/programs/CMakeLists.txt
index ad13ea7..8b740a2 100644
--- a/programs/CMakeLists.txt
+++ b/programs/CMakeLists.txt
@@ -52,7 +52,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "Linux")
        add_definitions(-D_GNU_SOURCE)
 endif ()
 
-if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
+if (CMAKE_SYSTEM_NAME MATCHES "Darwin" OR CMAKE_SYSTEM_NAME MATCHES "iOS")
        add_definitions(-D__APPLE_USE_RFC_2292)
 endif ()
 
diff --git a/usrsctplib/CMakeLists.txt b/usrsctplib/CMakeLists.txt
index 845772f..e937f70 100644
--- a/usrsctplib/CMakeLists.txt
+++ b/usrsctplib/CMakeLists.txt
@@ -83,7 +83,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "Linux")
        add_definitions(-D_GNU_SOURCE)
 endif ()
 
-if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
+if (CMAKE_SYSTEM_NAME MATCHES "Darwin" OR CMAKE_SYSTEM_NAME MATCHES "iOS")
        add_definitions(-D__APPLE_USE_RFC_2292)
 endif ()

Typecasts Added bundle identifiers to executables

agnickolov · 2021-09-09T21:36:56Z

Yes, those changes work for iOS. The symbol is only needed for the library, but it shouldn't hurt to have it defined for the apps as well.

This was proposed in #593.

tuexen · 2021-09-09T21:40:54Z

Fixed in b56b430.

This should address all issues brought up. Thanks for reporting and the patience!

paullouisageneau · 2021-09-12T09:30:26Z

@tuexen Thank you!

agnickolov force-pushed the feature/xcode12-ios-build branch from c6c9676 to 2827f88 Compare June 16, 2021 00:25

paullouisageneau mentioned this pull request Jun 19, 2021

iOS support (Xcode 12) paullouisageneau/libdatachannel#444

Closed

agnickolov force-pushed the feature/xcode12-ios-build branch from 2827f88 to c5e73c2 Compare July 12, 2021 22:04

tuexen reviewed Sep 5, 2021

View reviewed changes

tuexen added a commit that referenced this pull request Sep 5, 2021

Avoid overflows.

7077e0e

This was brought to my attention in #593

tuexen reviewed Sep 5, 2021

View reviewed changes

tuexen added a commit that referenced this pull request Sep 5, 2021

Use explict casts.

87859a5

This was reported in #593

tuexen reviewed Sep 5, 2021

View reviewed changes

tuexen added a commit that referenced this pull request Sep 6, 2021

Use explicit casts

a8adc4a

This is based on #593

tuexen reviewed Sep 6, 2021

View reviewed changes

tuexen added a commit that referenced this pull request Sep 6, 2021

Use explicit cast.

f2efa09

Taken from #593

tuexen reviewed Sep 6, 2021

View reviewed changes

tuexen added a commit that referenced this pull request Sep 7, 2021

Cleanup

8463f3a

This was brought up in #593

tuexen reviewed Sep 7, 2021

View reviewed changes

tuexen added a commit that referenced this pull request Sep 7, 2021

Use explicit cast.

2d65548

Stay with 32-bit, since the chunk is only 32-bit aligned. This was reported in #593

tuexen reviewed Sep 7, 2021

View reviewed changes

tuexen added a commit that referenced this pull request Sep 7, 2021

Explicit cast.

f2c2b60

Taken from #593

tuexen reviewed Sep 7, 2021

View reviewed changes

tuexen reviewed Sep 8, 2021

View reviewed changes

agnickolov force-pushed the feature/xcode12-ios-build branch from c5e73c2 to 15e54d2 Compare September 9, 2021 17:27

agnickolov force-pushed the feature/xcode12-ios-build branch from 15e54d2 to 5c96330 Compare September 9, 2021 17:45

tuexen added a commit that referenced this pull request Sep 9, 2021

Add missing cast.

6470706

This was reported in #593

tuexen reviewed Sep 9, 2021

View reviewed changes

agnickolov force-pushed the feature/xcode12-ios-build branch from 5c96330 to 11595f6 Compare September 9, 2021 21:33

Fixes to build under Xcode12 for iOS ARM64

dabf6b0

Typecasts Added bundle identifiers to executables

agnickolov force-pushed the feature/xcode12-ios-build branch from 11595f6 to dabf6b0 Compare September 9, 2021 21:34

tuexen added a commit that referenced this pull request Sep 9, 2021

Improve building for iOS.

b56b430

This was proposed in #593.

tuexen closed this Sep 9, 2021

paullouisageneau mentioned this pull request Sep 12, 2021

Update usrsctp for iOS support paullouisageneau/libdatachannel#478

Merged

Fixes to build under Xcode12 for iOS ARM64 #593

Fixes to build under Xcode12 for iOS ARM64 #593

Conversation

agnickolov commented Jun 14, 2021

paullouisageneau commented Jun 20, 2021

tuexen commented Jun 20, 2021

paullouisageneau commented Jun 21, 2021

tuexen commented Jun 21, 2021

weinrank commented Jun 21, 2021

agnickolov commented Jun 22, 2021

paullouisageneau commented Jun 29, 2021

tuexen commented Jul 12, 2021

agnickolov commented Jul 29, 2021

tuexen commented Jul 29, 2021

agnickolov commented Aug 27, 2021

tuexen commented Aug 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuexen commented Sep 8, 2021

tuexen commented Sep 9, 2021

agnickolov commented Sep 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuexen commented Sep 9, 2021

tuexen commented Sep 9, 2021

agnickolov commented Sep 9, 2021

tuexen commented Sep 9, 2021

paullouisageneau commented Sep 12, 2021