1417827 - Convert internal char* to nsCString in DNS.h

Status: RESOLVED FIXED

(Core :: Networking: DNS, enhancement, P5)

Description

[jthemphill]

DNS::mHostName and DNS::mCanonicalName are private member fields currently represented as `char*`. We have our own strings, with length fields and memory safety. Let's use those.

Comment 1

[jthemphill]

Attachment: Bug 1417827: Convert internal char* to nsCString in DNS.h

Review commit: https://reviewboard.mozilla.org/r/200230/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/200230/

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review205384

::: netwerk/dns/DNS.h:140
(Diff revision 1)
>  class AddrInfo {
>  public:
>    // Creates an AddrInfo object. It calls the AddrInfo(const char*, const char*)
>    // to initialize the host and the cname.
> +  AddrInfo(const nsCString &host, const PRAddrInfo *prAddrInfo, bool disableIPv4,
> +           bool filterNameCollision, const nsCString &cname);

Instead of `nsCString&` use `const nsACString&`.
That way we can pass nsAutoCString to the constructor.

::: netwerk/dns/DNS.h:146
(Diff revision 1)
>    AddrInfo(const char *host, const PRAddrInfo *prAddrInfo, bool disableIPv4,
>             bool filterNameCollision, const char *cname);
>  
>    // Creates a basic AddrInfo object (initialize only the host and the cname).
> +  AddrInfo(const nsCString &host, const nsCString &cname);
>    AddrInfo(const char *host, const char *cname);

Don't add new constructors, instead change the old ones. There are only two calls to it anyway
https://searchfox.org/mozilla-central/rev/550148ab69b2879bfb82ffa698720ede1fa626f2/netwerk/dns/GetAddrInfo.cpp#286
https://searchfox.org/mozilla-central/rev/550148ab69b2879bfb82ffa698720ede1fa626f2/netwerk/dns/nsHostResolver.cpp#884

::: netwerk/dns/DNS.cpp:296
(Diff revision 1)
>  }
>  
>  NetAddrElement::~NetAddrElement() = default;
>  
> -AddrInfo::AddrInfo(const char *host, const PRAddrInfo *prAddrInfo,
> -                   bool disableIPv4, bool filterNameCollision, const char *cname)
> +AddrInfo::AddrInfo(const nsCString &host, const PRAddrInfo *prAddrInfo,
> +                   bool disableIPv4, bool filterNameCollision, const nsCString &cname)

Same. Use nsACString&

::: netwerk/dns/DNS.cpp:318
(Diff revision 1)
>          mAddresses.insertBack(addrElement);
>      }
>    } while (iter);
>  }
>  
> -AddrInfo::AddrInfo(const char *host, const char *cname)
> +AddrInfo::AddrInfo(const char *host, const PRAddrInfo *prAddrInfo,

Let's change the host and cname types to be nsACString& as well.

::: netwerk/dns/DNS.cpp:332
(Diff revision 1)
>    , ttl(NO_TTL_DATA)
>  {
> -  Init(host, cname);
>  }
>  
> +AddrInfo::AddrInfo(const char *host, const char *cname)

Remove this constructor

Comment 3

[jthemphill]

Actually, wait. DNS.h and DNS.cpp don't include any Firefox-specific C++ headers. And the weird ancient internet libraries which they do include (arpa/inet.h) are probably plain C. Maybe this is a good class to rewrite in Rust?

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Maybe. Why not.
I say we update and land your patch, then open a new bug for rewriting it in Rust.

Comment 5

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/1-2/

Comment 6

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review205512

Accepting `const nsACString&` in all function parameters, actually passing `nsAutoCString` into the constructors and storing one internally.

Comment 7

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review205514

Comment 8

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review205516

Comment 9

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review205968

::: netwerk/dns/nsDNSService2.cpp
(Diff revision 2)
>          SizeOfIncludingThis(DNSServiceMallocSizeOf),
>          "Memory used for the DNS service.");
>  
>      return NS_OK;
>  }
> -

no whitespace only changes please.

Comment 10

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/2-3/

Comment 11

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/3-4/

Comment 12

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review206198

::: netwerk/dns/GetAddrInfo.cpp:286
(Diff revision 4)
>    if (aFlags & nsHostResolver::RES_CANON_NAME) {
>      canonName = PR_GetCanonNameFromAddrInfo(prai);
>    }
>  
>    bool filterNameCollision = !(aFlags & nsHostResolver::RES_ALLOW_NAME_COLLISION);
> -  nsAutoPtr<AddrInfo> ai(new AddrInfo(aCanonHost, prai, disableIPv4,
> +  nsAutoPtr<AddrInfo> ai(new AddrInfo(nsDependentCString(aCanonHost), prai,

nsTDependentString.h says the following:
`This class does not own its data, so the creator of objects of this type must take care to ensure that a nsTDependentString continues to reference valid memory for the duration of its use.`

I don't think this is true at the moment.

Comment 13

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/4-5/

Comment 14

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review206210

::: netwerk/dns/GetAddrInfo.cpp:287
(Diff revision 5)
> -    canonName = PR_GetCanonNameFromAddrInfo(prai);
> +    canonName.Rebind(PR_GetCanonNameFromAddrInfo(prai));
>    }
>  
>    bool filterNameCollision = !(aFlags & nsHostResolver::RES_ALLOW_NAME_COLLISION);
> -  nsAutoPtr<AddrInfo> ai(new AddrInfo(aCanonHost, prai, disableIPv4,
> -                                      filterNameCollision, canonName));
> +  nsAutoPtr<AddrInfo> ai(new AddrInfo(nsDependentCString(aCanonHost), prai,
> +                                      disableIPv4, filterNameCollision,

I'm still not sure about this. It might work now, but passing the strings to something that outlives `prai` would really screw things up.
Just use nsCString here instead of nsDependentCString

Comment 15

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/5-6/

Comment 16

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review205384

> Let's change the host and cname types to be nsACString& as well.

I think this is resolved? If this isn't resolved, it's because I don't understand what this comment means.

Comment 17

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review206214

You can take a look at https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Internal_strings for more info about how the string classes work, and which one is most appropriate.

::: netwerk/dns/GetAddrInfo.cpp:286
(Diff revisions 5 - 6)
>    if (aFlags & nsHostResolver::RES_CANON_NAME) {
> -    canonName.Rebind(PR_GetCanonNameFromAddrInfo(prai));
> +    canonName = PR_GetCanonNameFromAddrInfo(prai);
>    }
>  
>    bool filterNameCollision = !(aFlags & nsHostResolver::RES_ALLOW_NAME_COLLISION);
> -  nsAutoPtr<AddrInfo> ai(new AddrInfo(nsDependentCString(aCanonHost), prai,
> +  nsAutoPtr<AddrInfo> ai(new AddrInfo(nsAutoCString(aCanonHost), prai,

Since we're passing the value to another object, there's no point in using the auto version - that would mean an extra copy - from the nsAutoCString internal buffer, to the resulting nsCString. Come to think about it, it would be a lot more efficient if you made the strings in AddrInfo be nsCString as well.

::: netwerk/dns/DNS.h:153
(Diff revision 6)
>    void AddAddress(NetAddrElement *address);
>  
>    size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
>  
> -  char *mHostName;
> -  char *mCanonicalName;
> +  nsAutoCString mHostName;
> +  nsAutoCString mCanonicalName;

Let's make this nsCString. See previous comment.

Comment 18

[jthemphill]

Sorry, I'm still getting used to MozReview. When I clicked "publish", it wrote an old comment (that I hadn't published before) but none of my new comments. It's probably for the best, because I now think those comments are wrong :^)

Anyway, I think I'm getting a better mental model of these string classes. I didn't realize they used a reference-count internally.

Comment 19

[Valentin Gosu [:valentin] (he/him)]

(In reply to jthemphill from comment #18)
> Sorry, I'm still getting used to MozReview. When I clicked "publish", it
> wrote an old comment (that I hadn't published before) but none of my new
> comments. It's probably for the best, because I now think those comments are
> wrong :^)

There are still some bugs in MozReview. But I think we will be switching to something better™ in a couple of months (Phabricator).

> Anyway, I think I'm getting a better mental model of these string classes. I
> didn't realize they used a reference-count internally.

No problem. Working with strings takes a while to get used to, but it's very rewarding in the end.
Thanks for your contributions. We really appreciate it.

Comment 20

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/6-7/

Comment 21

[jthemphill]

I'm looking forward to that. Phabricator and Differential are really good.

Comment 22

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review206220

Looks good now. But looking at the try run, there's also something that looks like a leak. There's a chance we are triggering bug 1183781 with this.

Also, I want to encourage you to be more conservative when pushing to try.
Use https://mozilla-releng.net/trychooser/ to select just the platforms and tests you want to run. Pushing with all/all, will dozens of builds and hundreds of tests on all platforms, and that's just wasting resources. I usually test with linux64-all.

Now lets see if we can track down this leak :)

Comment 23

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review206220

Okay, I was wondering if my try runs were excessive. Note that the changes I'm making to GetAddrInfo.cpp are exclusive to Windows, though.

If this patch makes the leak more pronounced, should I fix the leak before asking for checkin?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60456eb8f659&selectedJob=145935629 seems like a promising job, and I'd like to repro it on my local Mac. How can I extract a repro from these logs?

Comment 24

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/7-8/

Comment 25

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/8-9/

Comment 26

[jthemphill]

Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200230/diff/9-10/

Comment 27

[jthemphill]

I think Bug 1420677 fixed the memory leak that was blocking this patch. If so, it's odd that changing from malloc to nsCString would reveal that leak. Maybe malloc doesn't cooperate with leakcheck in the same way that nsStringBuffer does?

Comment 28

[Valentin Gosu [:valentin] (he/him)]

I think the leak might have only occurred in certain situations - mainly during shutdown. Indeed, I think malloc might be counted in a different way than the string buffers.

Anyway, I was looking to see if any crashes similar to bug 1186435 occur, and they do.
I think that's because during shutdown, we delete addr_info without holding the lock.
Clearing the checkin-needed flag until we can confirm this.

Comment 29

[Valentin Gosu [:valentin] (he/him)]

Considering this patch doesn't cause bug 1422173, it might actually be helpful to land. Worst case scenario we'll back it out. I've triggered it, and it should land when the tree opens.

Comment 30

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4bbf9be9e3d076c22df6f42d54a31f6bb8833bcf

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34cfc821e335
Convert internal char* to nsCString in DNS.h r=valentin

Comment 32

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/34cfc821e335

Comment 33

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bf1aaf9c71f487c900191ba1f5b266ad55bee7b

Comment 34

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1417827: Convert internal char* to nsCString in DNS.h

Review commit: https://reviewboard.mozilla.org/r/250962/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250962/

Comment 35

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1417827 - Pass DNS arguments as nsACString& instead of char*

Review commit: https://reviewboard.mozilla.org/r/250964/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250964/

Comment 36

[Daniel Stenberg [:bagder]]

Comment on attachment 8985342 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/250962/#review257198

Comment 37

[Daniel Stenberg [:bagder]]

Comment on attachment 8985343 [details]
Bug 1417827 - Pass DNS arguments as nsACString& instead of char*

https://reviewboard.mozilla.org/r/250964/#review257200

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc388a747aba
Convert internal char* to nsCString in DNS.h r=bagder
https://hg.mozilla.org/integration/autoland/rev/190c4f057ffa
Pass DNS arguments as nsACString& instead of char* r=bagder

Comment 39

[Raul Gurzau (:RaulG)]

As this previous push (https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=8f3285a5c28c843002fa53ffe236fba6ad277b78) had failed on Gecko decision task, this current push also has failed gecko and no tests had run, can this be relanded to have tests running on it now that the problem is fixed?

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9606d0d95b53
Convert internal char* to nsCString in DNS.h r=bagder
https://hg.mozilla.org/integration/autoland/rev/35655153f9c9
Pass DNS arguments as nsACString& instead of char* r=bagder

Comment 41

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9beb489ae61e
Backed out 2 changesets backed out due to gecko decision task failure, tests did not run

Comment 42

[Dorel Luca [:dluca]]

Backed out 2 changesets (bug 1417827) for windows build bustage. CLOSED TREE.

Log:
https://treeherder.mozilla.org/logviewer.html#?job_id=183214078&repo=autoland&lineNumber=30147

mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/netwerk/dns'
18:35:27     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.6.6/VC/bin/Hostx64/x86/cl.exe -FoUnified_cpp_netwerk_dns0.obj -c -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DWIN32_LEAN_AND_MEAN -D_WIN32 -DWIN32 -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DOS_WIN=1 -D_UNICODE -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DUNICODE -D_WINDOWS -D_SECURE_ATL -DCOMPILER_MSVC -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/netwerk/dns -Iz:/build/build/src/obj-firefox/netwerk/dns -Iz:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -Iz:/build/build/src/ipc/chromium/src -Iz:/build/build/src/ipc/glue -Iz:/build/build/src/netwerk/base -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -TP -nologo -w15038 -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -wd4065 -we4553 -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -O1 -Oi -Oy- -WX  -deps.deps/Unified_cpp_netwerk_dns0.obj.pp    z:/build/build/src/obj-firefox/netwerk/dns/Unified_cpp_netwerk_dns0.cpp
18:35:27     INFO -  Unified_cpp_netwerk_dns0.cpp
18:35:27     INFO -  z:/build/build/src/netwerk/dns/GetAddrInfo.cpp(225): error C2039: 'get': is not a member of 'nsTSubstring<char>'
18:35:27     INFO -  z:\build\build\src\obj-firefox\dist\include\nsStringFwd.h(60): note: see declaration of 'nsTSubstring<char>'
18:35:27     INFO -  z:/build/build/src/netwerk/dns/GetAddrInfo.cpp(225): error C2660: 'mozilla::net::_GetMinTTLForRequestType_Windows': function does not take 3 arguments
18:35:27     INFO -  z:/build/build/src/netwerk/dns/GetAddrInfo.cpp(228): error C2039: 'get': is not a member of 'nsTSubstring<char>'
18:35:27     INFO -  z:\build\build\src\obj-firefox\dist\include\nsStringFwd.h(60): note: see declaration of 'nsTSubstring<char>'
18:35:27     INFO -  z:/build/build/src/netwerk/dns/GetAddrInfo.cpp(228): error C2660: 'mozilla::net::_GetMinTTLForRequestType_Windows': function does not take 3 arguments
18:35:27     INFO -  z:/build/build/src/netwerk/dns/GetAddrInfo.cpp(354): error C2677: binary '&&': no global operator found which takes type 'nsCString' (or there is no acceptable conversion)
18:35:27     INFO -  z:/build/build/src/config/rules.mk:1031: recipe for target 'Unified_cpp_netwerk_dns0.obj' failed
18:35:27     INFO -  mozmake.EXE[4]: *** [Unified_cpp_netwerk_dns0.obj] Error 2
18:35:27     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/netwerk/dns'
18:35:27     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/security/nss/lib/ssl/ssl_ssl'
18:35:27     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.6.6/VC/bin/Hostx64/x86/cl.exe -Fosslver.obj -c -DDEBUG -DNSS_ALLOW_SSLKEYLOGFILE=1 -DNSS_FIPS_DISABLED -DNSS_NO_INIT_SUPPORT -DNSS_X86_OR_X64 -DNSS_X86 -DIN_LIBSSL -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -D_WINDOWS -DWIN95 -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -DNSS_DISABLE_LIBPKIX -DWIN32 -Iz:/build/build/src/security/nss/lib/ssl -Iz:/build/build/src/obj-firefox/security/nss/lib/ssl/ssl_ssl -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/private/nss -Iz:/build/build/src/obj-firefox/dist/include/nss -Iz:/build/build/src/obj-firefox/dist/include -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -nologo -wd4091 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4244 -wd4267 -we4553 -Z7 -O1 -Oi -Oy-  -deps.deps/sslver.obj.pp    z:/build/build/src/security/nss/lib/ssl/sslver.c
18:35:27     INFO -  sslver.c
18:35:27     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/security/nss/lib/ssl/ssl_ssl'
18:35:27     INFO -  z:/build/build/src/config/recurse.mk:74: recipe for target 'netwerk/dns/target' failed
18:35:27     INFO -  mozmake.EXE[3]: *** [netwerk/dns/target] Error 2
18:35:27     INFO -  mozmake.EXE[3]: *** Waiting for unfinished jobs....
18:35:27     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/vr/openvr'
18:35:27     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/vr/openvr'
18:35:27     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/thebes'
18:35:27     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/thebes'
18:35:27     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/media/mtransport/third_party/nICEr/nicer_nicer'
18:35:27     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/media/mtransport/third_party/nICEr/nicer_nicer'
18:35:27     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/thebes'
18:35:27     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/thebes'
18:35:27     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/security/nss/lib/ssl/ssl_ssl'
18:35:27     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.6.6/VC/bin/Hostx64/x86/cl.exe -Fossltrace.obj -c -DDEBUG -DNSS_ALLOW_SSLKEYLOGFILE=1 -DNSS_FIPS_DISABLED -DNSS_NO_INIT_SUPPORT -DNSS_X86_OR_X64 -DNSS_X86 -DIN_LIBSSL -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -D_WINDOWS -DWIN95 -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -DNSS_DISABLE_LIBPKIX -DWIN32 -Iz:/build/build/src/security/nss/lib/ssl -Iz:/build/build/src/obj-firefox/security/nss/lib/ssl/ssl_ssl -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/private/nss -Iz:/build/build/src/obj-firefox/dist/include/nss -Iz:/build/build/src/obj-firefox/dist/include -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -nologo -wd4091 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4244 -wd4267 -we4553 -Z7 -O1 -Oi -Oy-  -deps.deps/ssltrace.obj.pp    z:/build/build/src/security/nss/lib/ssl/ssltrace.c
18:35:27     INFO -  ssltrace.c
18:35:27     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/security/nss/lib/ssl/ssl_ssl'
18:35:27     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/security/nss/lib/ssl/ssl_ssl'
18:35:27     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/security/nss/lib/ssl/ssl_ssl'
18:35:27     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/media/mtransport/third_party/nICEr/nicer_nicer'
18:35:27     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.6.6/VC/bin/Hostx64/x86/cl.exe -Foaddrs.obj -c -DDEBUG=1 -DWIN32 -D_WINDOWS -DNOMINMAX -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DWIN32_LEAN_AND_MEAN -D_ATL_NO_OPENGL -D_HAS_EXCEPTIONS=0 -D_SECURE_ATL -DCHROMIUM_BUILD -DTOOLKIT_VIEWS=1 -DUSE_LIBJPEG_TURBO=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_CONFIGURATION_POLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_GPU=1 -DENABLE_EGLIMAGE=1 -DUSE_SKIA=1 -D__STD_C -D_CRT_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_DEPRECATE -DENABLE_TASK_MANAGER=1 -DENABLE_WEB_INTENTS=1 -DENABLE_EXTENSIONS=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PROTECTOR_SERVICE=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_BACKGROUND=1 -DENABLE_AUTOMATION=1 -DENABLE_PRINTING=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DSANITY_CHECKS -DUSE_TURN -DUSE_ICE -DUSE_RFC_3489_BACKWARDS_COMPATIBLE -DUSE_STUND_0_96 -DUSE_STUN_PEDANTIC -DNR_SOCKET_IS_VOID_PTR -Drestrict= '-DR_PLATFORM_INT_TYPES=<stdint.h>' -DR_DEFINED_INT2=int16_t -DR_DEFINED_UINT2=uint16_t -DR_DEFINED_INT4=int32_t -DR_DEFINED_UINT4=uint32_t -DR_DEFINED_INT8=int64_t -DR_DEFINED_UINT8=uint64_t -D_WINSOCK_DEPRECATED_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS -D__UNUSED__= -DHAVE_STRDUP -DNO_REG_RPC -DDONT_HAVE_ETHTOOL_SPEED_HI -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DUNICODE -D_UNICODE -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/media/mtransport/third_party/nICEr -Iz:/build/build/src/obj-firefox/media/mtransport/third_party/nICEr/nicer_nicer -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/event -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/log -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/plugin -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/registry -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/share -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/stats -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/util -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/util/libekr -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/port/generic/include -Iz:/build/build/src/media/mtransport/third_party/nICEr/src/crypto -Iz:/build/build/src/media/mtransport/third_party/nICEr/src/ice -Iz:/build/build/src/media/mtransport/third_party/nICEr/src/net -Iz:/build/build/src/media/mtransport/third_party/nICEr/src/stun -Iz:/build/build/src/media/mtransport/third_party/nICEr/src/util -Iz:/build/build/src/media/mtransport/third_party/nrappkit/src/port/win32/include -Iz:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -Iz:/build/build/src/ipc/chromium/src -Iz:/build/build/src/ipc/glue -Iz:/build/build/src/obj-firefox/dist/include -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -utf-8 -nologo -wd4091 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4244 -wd4267 -we4553 -Z7 -O1 -Oi -Oy- -WX  -deps.deps/addrs.obj.pp    z:/build/build/src/media/mtransport/third_party/nICEr/src/stun/addrs.c
18:35:27     INFO -  addrs.c

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=35655153f9c927031e251bfd04c8a2effb51d6ae&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

Backout:
https://hg.mozilla.org/integration/autoland/rev/24bfb36d26e86f8c8a2b8e03a82081f4041c57f2

Comment 43

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8985343 [details]
Bug 1417827 - Pass DNS arguments as nsACString& instead of char*

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250964/diff/1-2/

Comment 44

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8985342 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250962/diff/1-2/

Comment 45

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8985343 [details]
Bug 1417827 - Pass DNS arguments as nsACString& instead of char*

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250964/diff/2-3/

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/535a93ad81a1
Convert internal char* to nsCString in DNS.h r=bagder
https://hg.mozilla.org/integration/autoland/rev/cb84b0e19643
Pass DNS arguments as nsACString& instead of char* r=bagder

Comment 47

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/535a93ad81a1
https://hg.mozilla.org/mozilla-central/rev/cb84b0e19643