Opened 6 years ago

Closed 4 years ago

#3894 closed defect (fixed)

Fix compilation on FreeBSD 4

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version: Tor: 0.2.4.15-rc
Severity: Keywords: tor-client freebsd4
Cc: grarpamp@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Grarpamp reports on tor-talk that Tor 0.2.2 doesn't build on freebsd 4, whereas 0.2.1 did. I've got a partial patch series at branch called "gcc-295-fix" in my public repository; I should get him to test it, test it more on other platforms, and make sure that it covers all the issues he mentioned in his emails.

Child Tickets

Attachments (1)

patch1.txt (3.5 KB) - added by grarpamp 6 years ago.
patch1

Download all attachments as: .zip

Change History (29)

comment:1 Changed 6 years ago by nickm

Status: newneeds_review

comment:2 Changed 6 years ago by nickm

Just added a changes file, and confirmed that there shouldn't be any remaining blocking issues from the emails here. please review?

comment:3 Changed 6 years ago by arma

Looks plausible to me. Has "grarpamp" said it works?

comment:4 Changed 6 years ago by nickm

grarpamp tested out some related stuff, ignored some of the patches (afaict), and did his/her own thing to fix some of the other stuff here but leave a lot of warnings in.

So, "partially".

comment:5 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merging into 0.2.2.x and later. If there's more to fix, we can reopen.

Changed 6 years ago by grarpamp

Attachment: patch1.txt added

patch1

comment:6 Changed 6 years ago by grarpamp

Cc: grarpamp@… added
Component: Tor RelayTor Client
Priority: minornormal
Resolution: fixed
Status: closedreopened
Version: Tor: 0.2.2.32Tor: 0.2.2.35

Here's the next installment. I'm writing this via the working binary
produced using the following components...

FreeBSD RELENG_4 i386 gcc 2.95.4
zlib 1.2.6
openssl 0.9.8u
libevent 1.4.14b
tor 0.2.2.35
the attached patch

The older patchwork on this was lost, and the email exchange looked
reasonable at the time, so I just accepted 0.2.2.35 as is and fixed
the problems evident in it alone.

The following warnings are all that remain on RELENG_4 if someone
wants to address them before commit...

Making all in or
control.c: In function `handle_control_attachstream':
control.c:2450: warning: `exit_digest' might be used uninitialized in this function
routerparse.c: In function `router_parse_list_from_string':
routerparse.c:1166: warning: `signed_desc' might be used uninitialized in this function

Patch attached for everything else but the above.

Regarding RELENG_8 with update to openssl 1.0.1 and libevent 2.0.18 ...
The two warnings above do not exist (tested), the patch generates
no additional warnings (tested), and the binary resulting from the
patch is presumed to work as well (untested).

comment:7 Changed 6 years ago by nickm

Status: reopenedneeds_revision

Hm. I'm not too happy about using "#ifdef FreeBSD" instead of actual autoconf checks or #ifdef checks for questions like "Is SIZE_MAX defined." Similarly, I'm not too happy with defining MAX_CAPACITY based on UINT_MAX -- doesn't FreeBSD work on 64-bit systems? On such systems, size_t can get up to UINT64_MAX. It's probably better to define SIZE_MAX in torint.h , if it's not already defined.

Similarly, for the MIN/MAX stuff, why not include sys/param.h if it exists, and then let the #ifndef MIN and #ifndef MAX .

The %lf -> %f fixes are legit and should go in as-is.

It's fine to initialize the two variables above at declaration time to suppress those warnings.

comment:8 Changed 6 years ago by grarpamp

Hi :) I just have no idea how to do anything with autoconf so I
didn't try that. And not being a programmer, sure, maybe on a C
level some parts could go closer to torint.h and compat.h. I'll
try a sleepy reply though...

It's probably better to define SIZE_MAX in torint.h, if it's not
already defined.

SIZE_MAX is not defined in 4.x's /usr/include or in Tor, unless
this limited scope is counted...

./src/common/OpenBSD_malloc_Linux.c:#define SIZE_MAX SIZE_T_MAX
/usr/include/machine/limits.h:#define SIZE_T_MAX UINT_MAX

I'm pretty sure it was born past the noted FreeBSD_version as...

/usr/include/machine/_stdint.h:#define SIZE_MAX UINT32_MAX

happy with... #ifdef checks for questions like "Is SIZE_MAX
defined."

Maybe must be something better to put in place of the literal
'SIZE_MAX' then, even maybe something local defined based on the
various systems, so as not to ever redefine/undeclare the literal
up against any system provided entity, or lack thereof. Which I'm
guessing is the base problem.

doesn't FreeBSD work on 64-bit systems? On such systems, size_t
can get up to UINT64_MAX.

Yes it does work, with non-ancient releases... there is no such
UINT64_MAX on 4.11 :) And both 4.x and 8.x have SIZE_T_MAX as a
32bit UINT_MAX-alike. FreeBSD became 64bit aware around the version
noted in the patch, which is a bit before 5.1 in layman term.

A non exhaustive inventory yielded some 'sys' stuff already directly
included and some various comparisons, some some of this stuff may
lint deeper...

./src/common/OpenBSD_malloc_Linux.c:#include <sys/param.h>
./src/common/address.c:#include <sys/param.h>
./src/common/compat.c:#include <sys/param.h>
./src/or/or.h:#include <sys/param.h>

./src/common/OpenBSD_malloc_Linux.c:#ifdef FreeBSD
./src/common/torint.h:#if !defined(FreeBSD) && !defined(FreeBSD_kernel)

Here are some links.

http://www.freebsd.org/cgi/cvsweb.cgi/src/include/?only_with_tag=RELENG_4
http://www.freebsd.org/cgi/cvsweb.cgi/src/include/?only_with_tag=RELENG_9
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/i386/include/?only_with_tag=RELENG_4
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/i386/include/?only_with_tag=RELENG_9
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/amd64/include/?only_with_tag=RELENG_9

At least it's identified and fix[ed/able], just how best.

fine to initialize the two variables above at declaration time
to suppress those warnings.

Never got that far yet :)

comment:9 Changed 6 years ago by nickm

Status: needs_revisionneeds_review

Updated/cleaned in branches "format_doubles" and "bug3894" in my public repository.

comment:10 Changed 6 years ago by grarpamp

I applied the two patches against the parent:

https://gitweb.torproject.org/nickm/tor.git/snapshot/75fc4dbbcabaedc715f0f9e883ccab1c9634e787.tar.gz
https://gitweb.torproject.org/nickm/tor.git/patch/0bec9f320b81a85ba3596e9780b1cf7c770dfb31
https://gitweb.torproject.org/nickm/tor.git/patch/f35271bf3eceadd03bff26d34c1d020892d6c6f0
autoreconf -fiv
configure
make
make install

The build emits only the uninitialized warnings now.
Otherwise it compiles and runs fine.
The parent snapshot appears to be roughly 2.2.34-dev, I didn't work
the patches into 2.2.36 yet, should be fine though. Thanks.

comment:11 Changed 6 years ago by nickm

Milestone: Tor: 0.2.2.x-finalTor: 0.2.3.x-final

Great, and thank you!

In the interests of making minimal changes to 0.2.2, I'm actually probably going to apply this to 0.2.3.x.

comment:12 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged to 0.2.3.x. Thanks again! (Please reopen if there are compilation issues remaining in 0.2.3.x.)

comment:13 Changed 6 years ago by grarpamp

Adding note for users of this platform...
that other than the previously noted uninitialized warnings:

  • 0.2.2.37 now appears fine in production, with the two most recently noted patches.
  • 0.2.3.16 now appears fine in production, as shipped.

Current in branch releases of above component dependencies works, so I have no plans to work on getting newest branches of openssl (101c) or libevent (2019) working.

comment:14 Changed 5 years ago by nickm

Keywords: tor-client added

comment:15 Changed 5 years ago by nickm

Component: Tor ClientTor

comment:16 Changed 5 years ago by grarpamp

Milestone: Tor: 0.2.3.x-finalTor: 0.2.4.x-final
Resolution: fixed
Status: closedreopened
Version: Tor: 0.2.2.35Tor: 0.2.3.25

Tor: 0.2.3.25, 0.2.4.10
Broken: NumCPUs via HAVE_PTHREAD_CREATE
Fix: LDFLAGS=-pthread ./configure ...

comment:17 Changed 5 years ago by grarpamp

Tor: 0.2.4.10
Is probably inttypes.h on this box.

make all-am
src/ext/curve25519_donna/curve25519-donna.c:50: stdint.h: No such file or directory
* Error code 1

comment:18 in reply to:  16 Changed 5 years ago by grarpamp

Replying to grarpamp:

Fix: LDFLAGS=-pthread ./configure ...

Well, that got it linked to libc_r, but it's still running 100%/0%, one process roaming between cpu's, and any threads aren't visible with older ps/top. I think I'm stuck. 'numcpus' doesn't have to be 'NumCPUs' does it? Ideas?

comment:19 Changed 5 years ago by nickm

Hm. It's been so long, I don't much remember the properties of FreeBSD 4's threading system. Did it even have kernel threads? I suppose it must have, since you're expecting this to work.

In any case, if there's a clean patch or two to fix FreeBSD4 compilation, I'd take it. Getting sane multiprocessor performance may be tricker -- I'd only expect multiple cores to be getting load at this point if this is a busy server.

comment:20 Changed 5 years ago by grarpamp

I now think FreeBSD4 has only SMP and user pthreads, not kernel threads (and that those didn't come till FreeBSD5).
I definitely remember seeing parent and child Tor processes with some Tor version maybe as far back as 0.2.0.x or farther. I'm not sure about taking load because I do recall always seeing nearly idle cpu time on one of the parent or child, though load was low anyways. Then something changed with Tor's CPU bits and it was back to one process.
Fast forward a few years to yesterday's thought to get back to two processes as I'm now maxing out any single CPU.

./test
test.util/fgets_eagain: SKIPPED
util/threads: OK
89 tests ok. (1 skipped)

If I knew the mechanism Tor used back then, or the release it changed in (?) I could play with an old version to compare behavior. Or I could work back down the majors trying the last minor of each.

I'll see about fixing that include for sure.

comment:21 Changed 5 years ago by grarpamp

build fix for 0.2.4.10

--- src/ext/curve25519_donna/curve25519-donna.c.orig Fri Jan 25 13:52:45 2013
+++ src/ext/curve25519_donna/curve25519-donna.c
@@ -47,7 +47,11 @@

*/


#include <string.h>

+#ifndef HAVE_STDINT_H
+#include <inttypes.h>
+#else

#include <stdint.h>

+#endif

typedef uint8_t u8;
typedef int32_t s32;

Remaining 0.2.4.10 nits:

src/or/circuitmux.c:1011: warning: `last_searched_direction' might be used uninitialized in this function
src/or/control.c:2686: warning: `exit_digest' might be used uninitialized in this function
src/or/routerparse.c:947: warning: `signed_desc' might be used uninitialized in this function

comment:22 Changed 5 years ago by nickm

That patch isn't going to work on any platform that lacks stdint: curve25519_donna isn't our code, and it isn't (currently) using our autoconf macros. (Also, the right thing to check is HAVE_INTTYPES_H, surely.)

comment:23 Changed 5 years ago by grarpamp

releng_4 has only inttypes and lacks stdint.
releng_8 has both inttypes and stdint.
That's regarding /usr/includes, and matches orconfig.h on both platforms.
The patch compiles and seems to at least reach bootstrap 100% on releng_4.
As yet untested on releng_8.
I'm barely any sort of programmer but happy to poke and push patches through the box if people like.

comment:24 Changed 5 years ago by nickm

Status: reopenedneeds_review

Try branch "integers_donna" in my public git repository -- does that work for you?

comment:25 Changed 5 years ago by grarpamp

I don't need 0.2.4.x on fbsd 4.x, just doing this for whoever might and the fixes are small so far.
integers_donna... compiles ok on fbsd 4.x and 8.x.
misc warns from that branch:
src/test/test_util.c:2716: warning: 'joined_argv' may be used uninitialized in this function

comment:26 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged integers_donna, and a fix for the joined_argv warning. Thanks!

(If you find more FreeBSD4 issues, let's open a new ticket for them?)

comment:27 Changed 4 years ago by grarpamp

Resolution: fixed
Status: closedreopened
Version: Tor: 0.2.3.25Tor: 0.2.4.15-rc

Was attempting to track this rarer old platform here as a platform ticket. That way people could pick fixes from it or not depending on need or interest in supporting it. It probably only exists in small embedded systems these days.

This is a current summary of the current tor versions...

FreeBSD 4.x i386 2.95.4

tor-0.2.3.25
control.c: In function `handle_control_attachstream':
control.c:2642: warning: `exit_digest' might be used uninitialized in this function
routerparse.c: In function `router_parse_list_from_string':
routerparse.c:1168: warning: `signed_desc' might be used uninitialized in this function

tor-0.2.4.15
src/or/circuitmux.c: In function `circuitmux_detach_circuit':
src/or/circuitmux.c:1011: warning: `last_searched_direction' might be used uninitialized in this function
src/or/control.c: In function `handle_control_attachstream':
src/or/control.c:2684: warning: `exit_digest' might be used uninitialized in this function
src/or/entrynodes.c: In function `entry_guards_parse_state':
src/or/entrynodes.c:1225: warning: use of l' length character with f' type character
src/or/entrynodes.c:1225: warning: use of l' length character with f' type character
src/or/entrynodes.c:1290: warning: use of l' length character with f' type character
src/or/entrynodes.c:1290: warning: use of l' length character with f' type character
src/or/routerparse.c: In function `router_parse_list_from_string':
src/or/routerparse.c:973: warning: `signed_desc' might be used uninitialized in this function
src/or/status.c: In function `log_heartbeat':
src/or/status.c:121: warning: .' not followed by *' or digit in format

master 6848e29
src/or/circuitmux.c: In function `circuitmux_detach_circuit':
src/or/circuitmux.c:1082: warning: `last_searched_direction' might be used uninitialized in this function
src/or/control.c: In function `handle_control_attachstream':
src/or/control.c:2685: warning: `exit_digest' might be used uninitialized in this function
src/or/entrynodes.c: In function `entry_guards_parse_state':
src/or/entrynodes.c:1229: warning: use of l' length character with f' type character
src/or/entrynodes.c:1229: warning: use of l' length character with f' type character
src/or/entrynodes.c:1294: warning: use of l' length character with f' type character
src/or/entrynodes.c:1294: warning: use of l' length character with f' type character
src/or/routerparse.c: In function `router_parse_list_from_string':
src/or/routerparse.c:973: warning: `signed_desc' might be used uninitialized in this function
src/or/status.c: In function `log_heartbeat':
src/or/status.c:121: warning: .' not followed by *' or digit in format

comment:28 Changed 4 years ago by nickm

Keywords: freebsd4 added
Resolution: fixed
Status: reopenedclosed

Quoting from above:

(If you find more FreeBSD4 issues, let's open a new ticket for them?)

I'm opening a new ticket as #9256 , and tagging both of them with a "freebsd4" keyword.

Note: See TracTickets for help on using tickets.