warning C4789: destination of memory copy is too small in nsCSSScanner.cpp

NEW
Unassigned

Status

()

Core
Layout
7 years ago
4 years ago

People

(Reporter: RyanVM, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
This warning showed up recently on my own builds. Doesn't seem to be showing up on Tinderbox. I do PGO builds with MSVC 2010 with -O2 -GL.
c:\mozbuild\mozilla-central\layout\style\nscssscanner.cpp(736) : warning C4789: destination of memory copy is too small
(Reporter)

Updated

7 years ago
Whiteboard: [build_warning]
(Reporter)

Comment 1

7 years ago
http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSScanner.cpp?mark=736#721

721 PRBool
722 nsCSSScanner::Next(nsCSSToken& aToken)
...
734     // IDENT
735     if (StartsIdent(ch, Peek()))
736       return ParseIdent(ch, aToken);
(Reporter)

Comment 2

6 years ago
This seems to have been tied to the optimization settings I was using. Whatever the reason, it's gone now.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 3

5 years ago
I still see this on MSVC 2010 PGO builds with no optimizations enabled. This looks potentially bad. Maybe a bug with MSVC, but is there a way to mitigate it?
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(Reporter)

Updated

5 years ago
Status: REOPENED → NEW
What line is the trunk warning on?
(Reporter)

Comment 5

5 years ago
726 now.
http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSScanner.cpp?mark=726#711
Erm.  Line 726 shouldn't have any memory copies....
(Reporter)

Comment 7

5 years ago
Dunno, but I just checked again.
c:\mozbuild\mozilla-central\layout\style\nscssscanner.cpp(726) : warning C4789: destination of memory copy is too small
If you inline ParseIdent at that point manually, which line does it complain on?
(Reporter)

Comment 9

5 years ago
Created attachment 583951 [details] [diff] [review]
Inline ParseIdent

Here's what I'm trying now. Will post back in a couple hours with results.
(Reporter)

Comment 10

5 years ago
Now it's giving the warning on this line:
if (!GatherIdent(ch, ident)) {
Alright.  Can you try inlining that too?
(Reporter)

Comment 12

5 years ago
Created attachment 583987 [details] [diff] [review]
Inline ParseIdent and GatherIdent

This is what I'm testing now. I'm pretty sure I did it right, but I would appreciate if you could give it a quick look to make sure I didn't screw something up.
Attachment #583951 - Attachment is obsolete: true
Comment on attachment 583987 [details] [diff] [review]
Inline ParseIdent and GatherIdent

Yeah, looks reasonable.
(Reporter)

Comment 14

5 years ago
      else if (0 < ch) {
>>>     ident.Append(ch);
      }
Hrm.  And this is a _compile_ time warning???
(Reporter)

Comment 16

5 years ago
No, while linking xul.lib on the second pass.
(Reporter)

Comment 17

5 years ago
Is there anything more I can do to help move this along?
I have no idea what's going on here.  :(  In general, our string append code should.. kinda work.  We use it all over....
(Reporter)

Comment 19

5 years ago
This also happens with tinderbox PGO builds now that they've been switched over to using VC10.
https://tbpl.mozilla.org/php/getParsedLog.php?id=9016860&tree=Firefox&full=1
Blocks: 563318
Blocks: 187528
(Reporter)

Comment 20

5 years ago
At the risk of shouting more into the Grand Canyon, can someone at least say if this bug is just a nuisance or something worse? This is shipping in nightlies now.

From MSDN:
Warns about buffer overrun when specific C run-time (CRT) functions are used, parameters are passed, and assignments are performed, such that the data sizes are known at compile time. This warning is for situations that might elude typical data-size mismatch detection.
That's the problem.  I wish I could tell what MSVC _thinks_ is going on here....  :(
You need to log in before you can comment on or make changes to this bug.