1248770 - sdp_unittests fails when libc++ is used

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Nathan Froyd [:froydnj]]

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.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: 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...

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/edd76ca3717f

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/edd76ca3717f