StanfordLegion / legion

The Legion Parallel Programming System
https://legion.stanford.edu
Apache License 2.0
677 stars 145 forks source link

Segfault when serializing and deserializing a SSETLBitMask<T> #9

Closed spad12 closed 9 years ago

spad12 commented 9 years ago

I found an issue on a machine which supports SSE2 instructions but not AVX and some issue with the serializer. Both template inline void Deserializer::deserialize(T &element) and template inline void Serializer::serialize(const T &element) segfault when trying to call SSETLBitMask<1024u>::operator=(). I'm assuming that an alignment issue is occuring when an array of char* is cast as an SSETLBitMask<1024u>*, which is causing the operator=() to fail.

I have performed a hack-ish fix by changing ((T)(buffer+index)) = element; to memcpy(buffer+index,&element,sizeof(T)); and vice-versa for the deserialize.

lightsighter commented 9 years ago

That change will break other things. The code should never be invoking the general forms of serialize and deserialize for any of the bit mask types. Notice the explicit specializations of the serialize and deserialize methods in legion_utilities.h. Check to make sure that your compiler is picking them up. Another thing to check is that your compiler is providing the SSE2 macro which protects the explicit specializations. If it's not then we need to understand why (I'm guessing you're not using a very old processor that doesn't have SSE2 instructions).

spad12 commented 9 years ago

Yeah I figured that the change would break other things, but it seemed to work for short term. I see a specialization of serialize and deserialize for SSEBitMask through a typedef of: "typedef SSEBitMask FieldMask;", but there is no specialization for the two level bit mask, SSETLBitMask. Yes the compiler is setting the SSE2 macro. It would appear that the appropriate fix would to be a specialization of serialize and deserialize for NodeMask, which is based on the two-level bit mask.

lightsighter commented 9 years ago

I'm not sure I know what you mean by a typedef. Look at this template specialization:

https://github.com/StanfordLegion/legion/blob/master/runtime/legion_utilities.h#L1187

The compiler should be invoking that function whenever it needs to serialize a SSETLBitMask.

lightsighter commented 9 years ago

Also, where do you see NodeMask still being used. I realize the typedefs are still there, but everything should have been switched over to NodeSets now (with the minor exception of one case in the low-level runtime which doesn't get serialized or deserialized anyway).

spad12 commented 9 years ago

Ahh, my version of legion_utilities.h must be out of date, as that file looks very different from the version I've got. The typedefs I am referring to are found in legion_types.h, lines 386 - 449, where FieldMask and NodeMask are set based on the instruction set. The version of legion_utilities.h that I have has Serializer::serialize, and Deserializer::deserialize specialized on FieldMask, but not NodeMask, and does not include instruction-set-macro based specializations.

On Thu, May 14, 2015 at 12:45 PM, Mike Bauer notifications@github.com wrote:

I'm not sure I know what you mean by a typedef. Look at this template specialization:

https://github.com/StanfordLegion/legion/blob/master/runtime/legion_utilities.h#L1187

The compiler should be invoking that function whenever it needs to serialize a SSETLBitMask.

— Reply to this email directly or view it on GitHub https://github.com/StanfordLegion/legion/issues/9#issuecomment-102132563 .

lightsighter commented 9 years ago

Ah, that makes sense. I was pretty sure I fixed those issues a long time ago.

spad12 commented 9 years ago

Ok, my bad.

On Thu, May 14, 2015 at 12:52 PM, Mike Bauer notifications@github.com wrote:

Ah, that makes sense. I was pretty sure I fixed those issues a long time ago.

— Reply to this email directly or view it on GitHub https://github.com/StanfordLegion/legion/issues/9#issuecomment-102133992 .