566168 - Source and destination overlap in memcpy [@ SetBackgroundList]

Status: RESOLVED INVALID

(Core :: CSS Parsing and Computation, defect)

Description

[Jesse Ruderman]

Attachment: testcase

(Only tested with a debug build of Firefox on Mac OS X 10.5.x)

==69551== Source and destination overlap in memcpy(0x30e46064, 0x30e46064, 10)
==69551==    at 0x1A224: memcpy (mc_replace_strmem.c:497)
==69551==    by 0x3AAAF95: void SetBackgroundList<nsCSSValuePairList, nsStyleBackground::Size>(nsStyleContext*, nsCSSValuePairList const*, nsAutoTArray<nsStyleBackground::Layer, 1u>&, nsAutoTArray<nsStyleBackground::Layer, 1u> const&, nsStyleBackground::Size nsStyleBackground::Layer::*, nsStyleBackground::Size, unsigned int, unsigned int&, unsigned int&, int&, int&) (nsRuleNode.cpp:4608)
==69551==    by 0x3AAFD6A: nsRuleNode::ComputeBackgroundData(void*, nsCSSStruct const&, nsStyleContext*, nsRuleNode*, nsRuleNode::RuleDetail, int) (nsRuleNode.cpp:4735)
==69551==    by 0x3AA4201: nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*, nsRuleData*, nsCSSStruct*) (nsStyleStructList.h:79)
==69551==    by 0x3AA72BB: nsRuleNode::GetBackgroundData(nsStyleContext*) (nsRuleNode.cpp:1749)
==69551==    by 0x3AA73EC: nsRuleNode::GetStyleBackground(nsStyleContext*, int) (nsStyleStructList.h:79)
==69551==    by 0x3A8B82B: nsStyleContext::DoGetStyleBackground(int) (nsStyleStructList.h:79)
==69551==    by 0x3A8B84C: nsStyleContext::GetStyleBackground() (nsStyleStructList.h:79)
==69551==    by 0x389E47C: nsIFrame::GetStyleBackground() const (nsStyleStructList.h:79)
==69551==    by 0x3952C75: nsIFrame::CheckInvalidateSizeChange(nsRect const&, nsRect const&, nsSize const&) (nsFrame.cpp:4041)
==69551==    by 0x3954489: nsFrame::CheckInvalidateSizeChange(nsHTMLReflowMetrics&) (nsFrame.cpp:3969)
==69551==    by 0x392DF5F: nsBlockFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) (nsBlockFrame.cpp:1161)

nsRuleNode.cpp:4608 is:
  aLayers[i].*aResultLocation = aParentLayers[i].*aResultLocation;

So I guess gcc is generating a memcpy here.  This could be a bug in gcc, such as http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39480.  That was fixed in gcc 4.3 and up; my build was apparently compiled with "gcc version 4.2.1 (Apple Inc. build 5574)".

Comment 1

[Jesse Ruderman]

Or http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32667

Comment 2

[Jesse Ruderman]

But is it a bug for aLayers to be equal to aParentLayers?

Comment 3

[Jesse Ruderman]

Btw bug 413173 is another example of gcc generating an incorrect memcpy.  It was wontfixed.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #2)
> But is it a bug for aLayers to be equal to aParentLayers?

Not if either:
 (1) we know that we don't need aParentLayers, or
 (2) we're on the root (which means 'inherit' == 'initial')

I think this is invalid; I think the assignment in this case is fine, and the only time it's an assignment-to-self is when somebody uses 'inherit' on the root element, which is not a case worth optimizing for.

I'd also note that I don't think the valgrind warning about src and destination overlapping poses a real problem when they're identical.  The real difference between memcpy and memmove is that memcpy just starts at one end and copies to the other end, whereas memmove copies end to beginning when dest > src (or dest >= src) and beginning to end otherwise.