Convert internal char* to nsCString in DNS.h

RESOLVED FIXED in Firefox 59

Status

()

enhancement
P5
normal
RESOLVED FIXED
a year ago
a month ago

People

(Reporter: jthemphill, Assigned: jthemphill, Mentored)

Tracking

(Blocks 1 bug)

58 Branch
mozilla62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed, firefox62 fixed)

Details

(Whiteboard: [necko-triaged])

Attachments

(3 attachments)

(Assignee)

Description

a year ago
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.
(Assignee)

Updated

a year ago
Blocks: 1185120
Priority: -- → P5
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8928903 - Flags: review?(valentin.gosu)
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
Attachment #8928903 - Flags: review?(valentin.gosu)
(Assignee)

Comment 3

a year ago
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?
Flags: needinfo?(valentin.gosu)
Maybe. Why not.
I say we update and land your patch, then open a new bug for rewriting it in Rust.
Flags: needinfo?(valentin.gosu)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

a year ago
mozreview-review
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.
(Assignee)

Comment 7

a year ago
mozreview-review
Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review205514
Attachment #8928903 - Flags: review?(jthemphill)
(Assignee)

Comment 8

a year ago
mozreview-review
Comment on attachment 8928903 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/200230/#review205516
Attachment #8928903 - Flags: review?(jthemphill)
Component: Untriaged → Networking: DNS
Product: Firefox → Core
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.
Attachment #8928903 - Flags: review?(valentin.gosu) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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.
Attachment #8928903 - Flags: review+
Comment hidden (mozreview-request)
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
Attachment #8928903 - Flags: review?(valentin.gosu)
Comment hidden (mozreview-request)
(Assignee)

Comment 16

a year ago
mozreview-review-reply
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 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.
Attachment #8928903 - Flags: review?(valentin.gosu)
(Assignee)

Comment 18

a year ago
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.
(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 hidden (mozreview-request)
(Assignee)

Comment 21

a year ago
I'm looking forward to that. Phabricator and Differential are really good.
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 :)
Attachment #8928903 - Flags: review?(valentin.gosu) → review+
(Assignee)

Comment 23

a year ago
mozreview-review-reply
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?
Whiteboard: [necko-triaged]
Assignee: nobody → jthemphill
(Assignee)

Updated

a year ago
Depends on: 1420677
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Keywords: checkin-needed
(Assignee)

Comment 27

a year ago
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?
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.
Keywords: checkin-needed
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 31

a year ago
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/34cfc821e335
Convert internal char* to nsCString in DNS.h r=valentin

Comment 32

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/34cfc821e335
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (mozreview-request)

Comment 36

10 months ago
mozreview-review
Comment on attachment 8985342 [details]
Bug 1417827: Convert internal char* to nsCString in DNS.h

https://reviewboard.mozilla.org/r/250962/#review257198
Attachment #8985342 - Flags: review?(daniel) → review+

Comment 37

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

https://reviewboard.mozilla.org/r/250964/#review257200
Attachment #8985343 - Flags: review?(daniel) → review+

Comment 38

10 months ago
Pushed by valentin.gosu@gmail.com:
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
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?
Flags: needinfo?(valentin.gosu)

Comment 40

10 months ago
Pushed by rgurzau@mozilla.com:
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

10 months ago
Backout by rgurzau@mozilla.com:
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

10 months ago
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
Flags: needinfo?(jthemphill)
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(jthemphill)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 46

10 months ago
Pushed by valentin.gosu@gmail.com:
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

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/535a93ad81a1
https://hg.mozilla.org/mozilla-central/rev/cb84b0e19643
Status: REOPENED → RESOLVED
Last Resolved: a year ago10 months ago
Resolution: --- → FIXED
Target Milestone: mozilla59 → mozilla62
You need to log in before you can comment on or make changes to this bug.