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

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

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

Comment 1

[Jeff Walden [:Waldo]]

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.

Comment 2

[Andi [:andi]]

Attachment: Bug 1411027 - avoid using memset on a not-trivial type like nsTabSizes.

Review commit: https://reviewboard.mozilla.org/r/192438/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/192438/

Comment 3

[Nathan Froyd [:froydnj]]

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.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8921396 [details]
Bug 1411027 - avoid using memset on a not-trivial type like nsTabSizes.

Sigh, mozreview.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50877116db1a
avoid using memset on a not-trivial type like nsTabSizes. r=froydnj

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/50877116db1a