5 ms·

You shouldn't be sending structs over the network, since their specific layout depends on the compiler (and version). At least make them packed if you're gonna

You shouldn't be sending structs over the network, since their specific layout depends on the compiler (and version). At least make them packed if you're gonna do undefined behavior... But you should just do it the right way and have a wrapper that deals with endianness differences and such: struct S my_struct; // ... push_int64(&output_stream, &my_struct.value1); push_string(&output_stream, my_struct.value2); push_int32(&output_stream, &my_struct.value3); And on the receiving side: struct S my_struct; next_int64(&input_stream, &my_struct.value1); next_string(&input_stream, my_struct.value2, sizeof(my_struct.value2)); next_int32(&input_stream, &my_struct.value3);

olliej 7y ago
The problem is not sending structs across the network. The problem is processing any data from the network. You have to assume that data is malicious, and will attack any weakness. I’m not talking about serialisation at all - I agreed entirely that sending struct padding across the net is less than optimal :)
- saagarjha 7y ago
  > You have to assume that data is malicious, and will attack any weakness. The point is that the standard cannot help you here because malicious data attacks things outside the scope of the standard.
  - olliej 7y ago
    The standard gives compilers leeway to handle “undefined” behaviour however they want. The original purpose of “undefined behaviour” was to deal with the myriad different platform behaviours of the era (remember at that point in history 1 byte was not necessary 8 bits). At some point the compiler writers decided to extend the effect of that to “if this behaviour is undefined in the spec” then we are allowed to treat the code as doing anything at all - so we went from this may do different things on different platforms to “the compiler can emit code that does something other than the clear intent or the platform specific behaviour”. This change in mindset from the compiler devs has been necessary to improve performance in the spec benchmarks, and other micro-benchmarks created primarily to justify the arbitrary definition of “undefined behaviour”. To give an idea of how capricious the compiler devs are they implemented optimizations that ended up breaking spec (it has undefined behavior), and so rolled those optimizations out. Yet when they literally add security bugs to code that has been secure for a decade, their response has been “well you’re relying on undefined behaviour”. The whole nonsense can be summed up as a (according to the insane definition of “correct” used by the compiler writers) Struct {char c;int i;} a,b; Bzero(&a, size of a) Bzero(&b, size of b) If (something) return memcmp(&Zaki, &b, size of a); Return 3; Could legitimately be optimized to a function that just returns 3. Because the writes to padding in bzero aren’t uneeefined, as are the reads in memcmp. Therefore the memcmp is undefined, and the compiler can assume undefined behavior cannot occur, therefore that branch can be removed. Whether the compiler does do that today isn’t relevant: the definition of “correct” used by compiler developers is that that is valid behaviour from them. The reality is that modern c/c++ developers need to treat the compiler as an adversary, because of a nonsensical definition of “correct code generation” assumed by the compiler writers.
    - saagarjha 7y ago
      Believe me, I know how compilers exploit undefined behavior and how this can lead to security issues. While you may disagree, there are certain useful optimizations that all but require the compiler assuming that undefined behavior cannot occur. Yes, occasionally compilers end up doing somewhat stupid and annoying things in the process or go "overboard" (Clang is notorious for this–I think some old version would just stop writing out code if it hit undefined behavior, so control would just run off the bottom of the procedure into the next one) but in general there are good reasons behind this choice being made. > The original purpose of “undefined behaviour” was to deal with the myriad different platform behaviours of the era (remember at that point in history 1 byte was not necessary 8 bits). FWIW, CHAR_BIT is implementation defined (not undefined) to be at least 8. > Therefore the memcmp is undefined, and the compiler can assume undefined behavior cannot occur, therefore that branch can be removed. I am fairly certain that this memcmp is "implementation-defined" (namely, reading the value of padding bytes is valid but not guaranteed to produce anything useful), which allows the compiler to pick whether it wants to emit that branch (but it must still return some value). If the memcmp was actually undefined the compiler would be "justified" in producing code that wiped your hard drive.
    - Matheus28 7y ago
      Undefined behavior != implementation defined. Also, is it that much harder to do this (C++ for example): struct S { char c = 0; int i = 0; bool operator==(const S &other) const { return c == other.c && i == other.i; } }; S a, b; if(something) return !(a == b); return 3; Bam, no undefined behavior and you don't have to remember to manually zero out the structure everywhere. If you're using memcmp on an object, you should be asserting that std::has_unique_object_representations<T> is true for it (unfortunately this is only available after C++17). Also, if you are okay with anonymous field names: std::tuple<char, int> a, b; if(something) return a != b; return 3;

olliej 7y ago

The problem is not sending structs across the network. The problem is processing any data from the network. You have to assume that data is malicious, and will attack any weakness. I’m not talking about serialisation at all - I agreed entirely that sending struct padding across the net is less than optimal :)

saagarjha 7y ago
> You have to assume that data is malicious, and will attack any weakness. The point is that the standard cannot help you here because malicious data attacks things outside the scope of the standard.
- olliej 7y ago
  The standard gives compilers leeway to handle “undefined” behaviour however they want. The original purpose of “undefined behaviour” was to deal with the myriad different platform behaviours of the era (remember at that point in history 1 byte was not necessary 8 bits). At some point the compiler writers decided to extend the effect of that to “if this behaviour is undefined in the spec” then we are allowed to treat the code as doing anything at all - so we went from this may do different things on different platforms to “the compiler can emit code that does something other than the clear intent or the platform specific behaviour”. This change in mindset from the compiler devs has been necessary to improve performance in the spec benchmarks, and other micro-benchmarks created primarily to justify the arbitrary definition of “undefined behaviour”. To give an idea of how capricious the compiler devs are they implemented optimizations that ended up breaking spec (it has undefined behavior), and so rolled those optimizations out. Yet when they literally add security bugs to code that has been secure for a decade, their response has been “well you’re relying on undefined behaviour”. The whole nonsense can be summed up as a (according to the insane definition of “correct” used by the compiler writers) Struct {char c;int i;} a,b; Bzero(&a, size of a) Bzero(&b, size of b) If (something) return memcmp(&Zaki, &b, size of a); Return 3; Could legitimately be optimized to a function that just returns 3. Because the writes to padding in bzero aren’t uneeefined, as are the reads in memcmp. Therefore the memcmp is undefined, and the compiler can assume undefined behavior cannot occur, therefore that branch can be removed. Whether the compiler does do that today isn’t relevant: the definition of “correct” used by compiler developers is that that is valid behaviour from them. The reality is that modern c/c++ developers need to treat the compiler as an adversary, because of a nonsensical definition of “correct code generation” assumed by the compiler writers.
  - saagarjha 7y ago
    Believe me, I know how compilers exploit undefined behavior and how this can lead to security issues. While you may disagree, there are certain useful optimizations that all but require the compiler assuming that undefined behavior cannot occur. Yes, occasionally compilers end up doing somewhat stupid and annoying things in the process or go "overboard" (Clang is notorious for this–I think some old version would just stop writing out code if it hit undefined behavior, so control would just run off the bottom of the procedure into the next one) but in general there are good reasons behind this choice being made. > The original purpose of “undefined behaviour” was to deal with the myriad different platform behaviours of the era (remember at that point in history 1 byte was not necessary 8 bits). FWIW, CHAR_BIT is implementation defined (not undefined) to be at least 8. > Therefore the memcmp is undefined, and the compiler can assume undefined behavior cannot occur, therefore that branch can be removed. I am fairly certain that this memcmp is "implementation-defined" (namely, reading the value of padding bytes is valid but not guaranteed to produce anything useful), which allows the compiler to pick whether it wants to emit that branch (but it must still return some value). If the memcmp was actually undefined the compiler would be "justified" in producing code that wiped your hard drive.
  - Matheus28 7y ago
    Undefined behavior != implementation defined. Also, is it that much harder to do this (C++ for example): struct S { char c = 0; int i = 0; bool operator==(const S &other) const { return c == other.c && i == other.i; } }; S a, b; if(something) return !(a == b); return 3; Bam, no undefined behavior and you don't have to remember to manually zero out the structure everywhere. If you're using memcmp on an object, you should be asserting that std::has_unique_object_representations<T> is true for it (unfortunately this is only available after C++17). Also, if you are okay with anonymous field names: std::tuple<char, int> a, b; if(something) return a != b; return 3;