Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#2337 closed defect (fixed)

Check for signed size_t during configure

Reported by: Sebastian Owned by:
Priority: Medium Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Our code breaks badly in a few places when size_t is signed. Apparently this can happen with an old gcc, and maybe even with a few other compilers. For now I think we should add a configure check, and only if it turns out that anyone important is actually affected make sure that we can handle it.

Discovered by doorss, analyzed with help from rransom

Child Tickets

Change History (8)

comment:1 Changed 9 years ago by Sebastian

Component: - Select a componentTor Client
Milestone: Tor: 0.2.2.x-final

comment:2 Changed 9 years ago by Sebastian

While we're at it: Platforms with a 128 bits wide long long are an issue too:

#define INT64_MAX 0x7fffffffffffffffll
#define SSIZE_T_MAX INT64_MAX
#define SIZE_T_CEILING (SSIZE_T_MAX-16)

thanks to doorss again

comment:3 Changed 9 years ago by nickm

Status: newneeds_review

sebastian: what the trouble with a 128-bit long long?

re the original bug: how does branch bug2337 in my public repository look?

comment:4 Changed 9 years ago by Sebastian

I was confused about long long, but we should probably add a cast to size_t to the SIZE_T_CEILING definition, yes?

You branch is on 0.2.1, is that on purpose? Also, it adds a new autoconf warning that we had been trying to get rid of; fix in bug2337 in my branch, feel free to squash that in. Otherwise, the test looks good - I didn't get it to fail tho due to lack of a gcc old enough.

comment:5 Changed 9 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

I'm not sure what we get with adding a cast to the SIZE_T_CEILING definition, other than making the value unsigned, which isn't necessary afaict. (Please comment here if I'm missing something.)

Merging your bug2337.

comment:6 Changed 9 years ago by Sebastian

doorss and rransom felt that we should make sure all comparisons involving SIZE_T_CEILING should be unsigned:

<rransom> Sebastian: I think it's mostly C-phobia, but we do want to make sure that all comparisons involving SIZE_T_CEILING are unsigned comparisons.

comment:7 Changed 9 years ago by nickm

oops; hadn't seen the note here. In the future, when adding an action-suggesting note to a closed ticket, please either reopen the ticket, or open a new ticket. Closed tickets shouldn't have actions on them.

(Yes, I know I said "please comment here". Dumb me. :p )

Opening a new ticket as #2475 .

comment:8 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.