1247505 - woff2 build failure: invalid cast from type ‘int’ to type ‘uint16_t

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Steve Singer (:stevensn)]

Build error on my ppc64 machine from woff2

/modules/woff2/Unified_cpp_modules_woff20.cpp:29:
/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/modules/woff2/src/./store_bytes.h: In function ‘size_t woff2::Store16(uint8_t*, size_t, int)’:
/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/modules/woff2/src/./store_bytes.h:40:76: error: invalid cast from type ‘int’ to type ‘uint16_t {aka short unsigned int}’
   *reinterpret_cast<uint16_t*>(dst + offset) = reinterpret_cast<uint16_t>(x);

Filed upstream issue at https://github.com/google/woff2/issues/44

Comment 1

[Steve Singer (:stevensn)]

Attachment: Fix woff2 compile error on big endian

Upstream pull request https://github.com/google/woff2/pull/45

Comment 2

[Martin Husemann]

A lhs pointer cast and dereference always makes me a bit nervous - are we sure the target address is properly aligned? If not, the third ifdef alternative would be preferable (or needed at least on strict alignment platforms)

Comment 3

[Steve Singer (:stevensn)]

Martin, would it be better to write the middle block as 

  uint16_t * dst_16 = reinterpret_cast<uint16_t*>(dst);
  dst_16[offset] = static_cast<uint16_t>(x) | static_cast<uint16_t>(x>>8);

Does the first LITTLE_ENDIAN case need adjusting as well?

Comment 4

[Martin Husemann]

The question is weather we do know for sure that both dst and offset are properly aligned. If we do, all is fine. If not, both cases would need adjustment for some CPUs.

Is there an easy way to exercise this code?

However, all this is a separate issue, the compile issue you raised should be fixed, it breaks my build as well (and this discussion maybe should happen upstream)

Comment 5

[Frédéric Wang Nélar]

Comment on attachment 8719188 [details] [diff] [review]
Fix woff2 compile error on big endian

Review of attachment 8719188 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks. Let's just take this patch now that it is merged upstream.

Comment 6

[Steve Singer (:stevensn)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=68ef675c3288

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ecf4dfdbf9e8

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ecf4dfdbf9e8