fails with -Werror=class-memaccess on mozilla/PodOperations.h

RESOLVED FIXED in Firefox 58

Status

()

Core
DOM
RESOLVED FIXED
27 days ago
26 days ago

People

(Reporter: sylvestre, Unassigned)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

27 days ago
1:00.16 /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/PodOperations.h: In instantiation of 'void mozilla::PodZero(T*) [with T = nsTabSizes]':
 1:00.16 /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/nsWindowSizes.h:22:39:   required from here
 1:00.16 /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/PodOperations.h:32:9: error: 'void* memset(void*, int, size_t)' clearing an object of non-trivial type 'class nsTabSizes'; use assignment or value-initialization instead [-Werror=class-memaccess]
 1:00.16    memset(aT, 0, sizeof(T));
 1:00.16    ~~~~~~^~~~~~~~~~~~~~~~~~
 1:00.16 In file included from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/nsINode.h:20:0,
 1:00.16                  from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/nsIContent.h:12,
 1:00.16                  from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/RangeBoundary.h:11,
 1:00.16                  from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/nsContentUtils.h:40,
 1:00.16                  from /root/firefox-gcc-last/intl/strres/nsStringBundle.cpp:27,
 1:00.16                  from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/intl/strres/Unified_cpp_intl_strres0.cpp:2:
 1:00.16 /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/nsWindowSizes.h:14:7: note: 'class nsTabSizes' declared here
 1:00.16  class nsTabSizes {
 1:00.16        ^~~~~~~~~~
 1:02.62 cc1plus: all warnings being treated as errors
(Reporter)

Updated

27 days ago
Blocks: 1411029
class nsTabSizes {
public:
  enum Kind {
      DOM,        // DOM stuff.
      Style,      // Style stuff.
      Other       // Everything else.
  };

  nsTabSizes() { mozilla::PodZero(this); }

  void add(Kind kind, size_t n)
  {
    switch (kind) {
      case DOM:   mDom   += n; break;
      case Style: mStyle += n; break;
      case Other: mOther += n; break;
      default:    MOZ_CRASH("bad nsTabSizes kind");
    }
  }

  size_t mDom;
  size_t mStyle;
  size_t mOther;
};

nsTabSizes is non-trivial only because of the user-defined constructor, if memory serves.  The idea desired here is certainly to zero all the members without listing them -- but the very act of doing so, with a user-defined constructor, makes the idea impossible.

Arguably this is something that is permissible in the language, and that the warning should be tailored to permit.  I don't think this falls afoul of any of the issues flagged in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg01527.html for example.  In the meantime, just explicitly zeroing the three member fields is easy and eliminates the warning.

FYI -- this warning, appearing for this code in PodOperations.h, is never going to be an mfbt bug.  It'll be an issue with whatever's calling PodZero.
Component: MFBT → DOM
Comment hidden (mozreview-request)
Comment on attachment 8921396 [details]
Bug 1411027 - avoid using memset on a not-trivial type like nsTabSizes.

https://reviewboard.mozilla.org/r/192438/#review197652

Zeroing all members without listing them is nice, but we also have static analyses that will catch uninitialized class members nowadays (I think?).  In any event, this code is pretty stable, and listing all the members here is not onerous.
Attachment #8921396 - Flags: review+
Comment on attachment 8921396 [details]
Bug 1411027 - avoid using memset on a not-trivial type like nsTabSizes.

Sigh, mozreview.
Attachment #8921396 - Flags: review?(jwalden+bmo)

Comment 5

26 days ago
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/50877116db1a
avoid using memset on a not-trivial type like nsTabSizes. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/50877116db1a
Status: NEW → RESOLVED
Last Resolved: 26 days ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.