Closed
Bug 980753
Opened 10 years ago
Closed 10 years ago
make nsRefPtr moveable
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: tbsaunde, Assigned: tbsaunde)
Details
Attachments
(1 file, 2 obsolete files)
1.80 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
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+
Assignee | ||
Comment 2•10 years ago
|
||
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.
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Comment 4•10 years ago
|
||
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
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8387342 -
Attachment is obsolete: true
Attachment #8411162 -
Attachment is obsolete: true
Attachment #8412632 -
Flags: review?(nfroyd)
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bab9339e6d4 is kind of messy, but afaik its all unrelated to this patch.
Comment 7•10 years ago
|
||
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+
Assignee | ||
Comment 8•10 years ago
|
||
(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 | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/db9956f94680
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/db9956f94680
Assignee: nobody → trev.saunders
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•