Bug 1696458

Summary: PString Mid() fails due to gcc signed int overflow optimization breaking Ekiga
Product: [Fedora] Fedora Reporter: Krzysztof Halasa <khalasa>
Component: ptlibAssignee: Peter Robinson <pbrobinson>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 29CC: jwakely, pbrobinson, redhat-bugzilla, veillard
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: ptlib-2.10.11-3.fc30 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2019-05-30 12:51:34 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Krzysztof Halasa 2019-04-04 21:29:43 UTC
Description of problem:
PString Mid() tries to detect signed integer overflow, but the test is 'undefined behaviour' and gcc happily optimizes it out:

PString PString::Mid(PINDEX start, PINDEX len) const
{
  if (len <= 0 || start < 0)
    return Empty();

  if (start+len < start) // Beware of wraparound
^^^^^^^^^^^^^^^^^^^^^^^^^^^
Recent gcc optimizes the above to if (len < 0)

    return operator()(start, P_MAX_INDEX);
  else
    return operator()(start, start+len-1);
}

This breaks (at least) Ekiga's SIP.

Version-Release number of selected component (if applicable):
ptlib-2.10.11-1.fc29.x86_64

How reproducible:
always

Steps to Reproduce:
1. Setup 2 computers with Ekiga on each
2. run ekiga -d 8 on each computer
3. try to call the 2nd computer using sip:IP_address

Actual results:
The 2nd Ekiga ignores the call, debug output contains information about invalid version of SIP packet:
SIP     Invalid version (0) received on udp$x.x.x.x:5060<if=udp$*:5060>

Expected results:
The 2nd Ekiga should ring.

Additional info:
A quick and dirty workaround would be adding -fwrapv to gcc options.

Comment 1 Robert Scheck 2019-05-14 23:19:42 UTC
Could you try to replace

> if (start+len < start) // Beware of wraparound

by

> if (len == P_MAX_INDEX || start+len < start) // If open ended or check for wraparound

and test whether it works?

Comment 2 Jonathan Wakely 2019-05-16 14:00:07 UTC
Or change it to 'if (start > INT_MAX - len)'

Comment 3 Jonathan Wakely 2019-05-16 14:01:22 UTC
Oh but it's not an int. So then:

  if (start > std::numeric_limits<PINDEX>::max() - len)

Comment 4 Krzysztof Halasa 2019-05-20 18:49:58 UTC
Both first
 if (len == P_MAX_INDEX || start+len < start) // If open ended or check for wraparound

and the second
 #include <limits>
 if (start > std::numeric_limits<PINDEX>::max() - len)

work.

OTOH I'm not sure the first test is entirely valid in case of a generic library like this. len can be a bit less than P_MAX_INDEX (it's just an arbitrary limit) and still cause the "start + len - 1" to fail.
The second test guarantees that start <= max - len and thus start + len - 1 < max.

Thanks for looking into this.

Comment 5 Robert Scheck 2019-05-20 20:17:40 UTC
My suggestion in comment #2 is simply from what latest upstream uses. If we need to propose something better to upstream, I would need your help in order to do so.

Comment 6 Robert Scheck 2019-05-20 20:18:16 UTC
Sorry, I meant comment #1 rather comment #2.

Comment 7 Fedora Update System 2019-05-21 08:11:25 UTC
ekiga-4.0.1-44.fc30 ptlib-2.10.11-3.fc30 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-c5e3dc44ba

Comment 8 Krzysztof Halasa 2019-05-21 16:09:14 UTC
Right. I just wasn't aware that the ptlib version used by Ekiga is 7+ years old.

Comment 9 Fedora Update System 2019-05-22 01:44:08 UTC
ekiga-4.0.1-44.fc30, ptlib-2.10.11-3.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-c5e3dc44ba

Comment 10 Fedora Update System 2019-05-30 12:51:34 UTC
ekiga-4.0.1-44.fc30, ptlib-2.10.11-3.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.