Closed Bug 980753 Opened 10 years ago Closed 10 years ago

make nsRefPtr moveable

Categories

(Core :: XPCOM, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Details

Attachments

(1 file, 2 obsolete files)

      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.
Attached file gcc-dumps (obsolete) —
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
Attachment #8387342 - Attachment is obsolete: true
Attachment #8411162 - Attachment is obsolete: true
Attachment #8412632 - Flags: review?(nfroyd)
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.
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.