sdp_unittests fails when libc++ is used

RESOLVED FIXED in Firefox 47

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: froydnj, Unassigned)

Tracking

Trunk
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

I'm updating our Mac builds to use libc++; initial tests look pretty good, but I'm seeing failures in the cppunittests, in sdp_unittests:

https://treeherder.mozilla.org/logviewer.html#?job_id=16785306&repo=try

with failures like:

10:35:15     INFO -  /builds/slave/try-m64-0000000000000000000000/build/src/media/webrtc/signaling/test/sdp_unittests.cpp:2816: Failure
10:35:15     INFO -  Value of: static_cast<size_t>(is.tellg())
10:35:15     INFO -    Actual: 2
10:35:15     INFO -  Expected: last
10:35:15     INFO -  Which is: 1
10:35:15     INFO -  Parse failed at unexpected location:
10:35:15     INFO -  [x
10:35:15     INFO -    ^

10:35:15     INFO -  /builds/slave/try-m64-0000000000000000000000/build/src/media/webrtc/signaling/test/sdp_unittests.cpp:2816: Failure
10:35:15     INFO -  Value of: static_cast<size_t>(is.tellg())
10:35:15     INFO -    Actual: 6
10:35:15     INFO -  Expected: last
10:35:15     INFO -  Which is: 5
10:35:15     INFO -  Parse failed at unexpected location:
10:35:15     INFO -  [0.2-x
10:35:15     INFO -        ^

10:35:15     INFO -  /builds/slave/try-m64-0000000000000000000000/build/src/media/webrtc/signaling/test/sdp_unittests.cpp:2816: Failure
10:35:15     INFO -  Value of: static_cast<size_t>(is.tellg())
10:35:15     INFO -    Actual: 9
10:35:15     INFO -  Expected: last
10:35:15     INFO -  Which is: 8
10:35:15     INFO -  Parse failed at unexpected location:
10:35:15     INFO -  [0.2-0.3x
10:35:15     INFO -           ^

and so on.  I don't understand all the failures, but it looks like there's some disagreement between the tests and how the spec sets up numeric parsing in num_get::get: the tests expect the parsing to terminate as soon as a invalid character is detected, whereas the spec seems to want invalid characters to be accumulated and failure detected after they are accumulated.  Part of the trouble may be that the chosen invalid character in the tests, 'x', is permitted anywhere in the string, rather than just as part of a 0x prefix or similar; fixing the tests may be as simple as changing the 'x' to 'v' or similar.
Created attachment 8720417 [details] [diff] [review]
change sdp_unittests to cope with diverse c++ standard interpretations

Apologies for the long explanation in the commit message, but I thought it
would be useful for review purposes and for future archaeologists.

I'm not completely convinced of this patch, since there are a few remaining
cases of 'x' in strings that *do* return the correct position.  I attempted to
set the inputstream's base to std::dec in SdpAttribute.cpp:GetUnsigned, but
that didn't seem to work for floating-point values.  It's possible this is a
bug in libc++; the version I'm using on Mac is the same version that Chromium's
build scripts seem to use.  I don't think it's worth waiting for a fix, though,
and we'd probably have to use a buggy version on Android (out of the NDK?) as
well...
Attachment #8720417 - Flags: review?(rjesup)

Updated

3 years ago
Attachment #8720417 - Flags: review?(rjesup) → review+

Comment 3

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/edd76ca3717f
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.