Closed Bug 1411027 Opened 2 years ago Closed 2 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: Sylvestre, Assigned: andi)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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
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 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)
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
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee: nobody → bpostelnicu
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.