Opened 3 years ago

Closed 3 years ago

#23583 closed defect (fixed)

ext/timeouts/timeout-bitops.c:234: bad shift

Reported by: dcb314@… Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 030-backport 029-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


tor-]: (error) Shifting 31-bit value by 63 bits is undefined behaviour

Source code is

for (i = 0; i <= 63; ++i) {

uint64_t x = 1 << i;

If you are going to shift something by 60+ bits reliably,
you need 1UL, not 1.

Child Tickets

Change History (5)

comment:1 Changed 3 years ago by nickm

Component: - Select a componentCore Tor/Tor
Keywords: 030-backport 029-backport added
Milestone: Tor: 0.3.1.x-final
Owner: set to nickm
Status: newaccepted

comment:2 Changed 3 years ago by nickm

Milestone: Tor: 0.3.1.x-finalTor: 0.3.2.x-final
Status: acceptedneeds_review

Fix in branch bug23583_029 in my public repository. I suggest no backport, since this code is not actually compiled into Tor -- it's conditional on the timeout.c bitops tests being run.

comment:3 Changed 3 years ago by catalyst

Status: needs_reviewmerge_ready

Looks good to me. I think I slightly prefer x = (uint64_t)1 << i but not strongly.

comment:4 Changed 3 years ago by nickm

I think I slightly prefer x = (uint64_t)1 << i but not strongly.

I can never remember whether the order of operations works with that one, or whether I need to say ((uint64_t)1) :)


comment:5 Changed 3 years ago by nickm

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.