450765 - Fix type-punned warnings in LIR.h

Status: RESOLVED FIXED

(Tamarin Graveyard :: Tracing Virtual Machine, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

When compiling nanojit, I see lots of:

nanojit/LIR.h: In member function ‘uint64_t nanojit::LIns::constvalq() const’:
nanojit/LIR.h:281: warning: dereferencing type-punned pointer will break strict-aliasing rules
nanojit/LIR.h: In member function ‘double nanojit::LIns::constvalf() const’:
nanojit/LIR.h:305: warning: dereferencing type-punned pointer will break strict-aliasing rules

AFAICT, the warnings are real and easy to fix.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Note: due to time pressure, I've checked this patch into the tracemonkey tree as http://hg.mozilla.org/tracemonkey/index.cgi/rev/84b79f7695c2

Comment 2

[Steven Johnson]

That change probably needs to be endian-specific.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Only the second hunk needs to be, right? The first bit is basically performing a memcpy.

Comment 4

[Steven Johnson]

depends on how the values are inserted into the LIR stream in the first place -- if we're always pulling them out in the same order as we put them in, it probably doesn't matter... but it's probably good form to always store them in platform order

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Wait, I lost it. There's no endian difference between the new code and old code here. The cast and union have exactly the same semantics, except the union tells the compiler which names refer to which memory so it doesn't over optimize. If the new code needs to be endian specific, so does the old code.

Comment 6

[Steven Johnson]

True. Maybe it didn't matter before because we were always shoving them in, and pulling them out, in the same order? Or maybe we just hadn't tested on a big-endian machine? 

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

I don't know what this code has been tested on, but it's true that it didn't matter before because you're just shoving them in and pulling them out. I don't know enough of nanojit's internals to know how we get here in the first place.

Comment 8

[Edwin Smith]

Comment on attachment 333960 [details] [diff] [review]
Fix

Contact [e-mail] if you need someone to push this once/if it's updated to TT tip.  thanks.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tamarin-tracing/rev/0766a2611038 fixes this.