Closed Bug 980753 Opened 6 years ago Closed 6 years ago
Ref Ptr moveable
No description provided.
froydnj reviewed this in bug 969532, however it seems to cause a crash when running xpcshell to make the precompile cache. I don't immediately have more time to figure out why so I'm posting it here. If I get some time I'll try and look into it, but I'd be happy if someone else took it first.
Attachment #8387342 - Flags: review+
So, for whatever reason gcc (and I think other compilers too?) miscompile nsDNSService.cpp with this patch, the first case of this is in nsDNSService::Init() where mRes = res is compiled to just a mov from res to mRes without either nulling out res or calling AddRef.
ok, so from nsDNSService2.cpp.003t.original before we have (void) nsRefPtr<nsHostResolver>::operator= (&((struct nsDNSService *) this)->mResolver, (const struct nsRefPtr &) (const struct nsRefPtr *) &res) >>>>>; and after the patch the same line is (void) (((struct nsDNSService *) this)->mResolver = *(const struct nsRefPtr &) (const struct nsRefPtr *) &res) >>>>>; which seems pretty suspicious
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bab9339e6d4 is kind of messy, but afaik its all unrelated to this patch.
Comment on attachment 8412632 [details] [diff] [review] 0001-bug-980753-make-nsRefPtr-movable-r-froydnj.patch Hooray, this fixes the miscompilation, then? Are you filing a GCC bug on this?
Attachment #8412632 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd (:froydnj) from comment #7) > Comment on attachment 8412632 [details] [diff] [review] > 0001-bug-980753-make-nsRefPtr-movable-r-froydnj.patch > > Hooray, this fixes the miscompilation, then? > > Are you filing a GCC bug on this? tbh I'm not absolutely sure something isn't wrong the code when we template everything, but I guess a reduced test case shouldn't be hard so filing and letting someone else sort that out might not be bad.
Assignee: nobody → trev.saunders
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.