On 28/09/16 13:08, Steffan Karger wrote: > Fixes compiler warnings (undefined behavior) by making the copy explicit > to comply to strict aliasing rules. With newer GCC the old code could > actually lead to undefined behaviour. > > See e.g. http://blog.regehr.org/archives/959.
Hmm ... This is an interesting blog post. And testing the examples
provided there on my own system, and I see the same results.
So I wanted to compare those findings with what happens with our own
code, so I extracted the needed pieces and tried the same (source
attached). But for us the result is not so clear and obvious at all.
Unless I've overlooked something bloody obvious .... but the result I
get is:
-----------------------------------------
mroute_extract_in_addr_t:
.LFB29:
.cfi_startproc
movb $2, 2(%rdi)
movb $0, 3(%rdi)
movb $4, (%rdi)
bswap %esi
movl %esi, 4(%rdi)
ret
.cfi_endproc
-----------------------------------------
To test it:
gcc -O1 -S -c mroute-test.c
gcc -O1 -S -c mroute-test.c -DPATCHED
I've also tried -O2. Without -O{1,2} I get a very different result,
which is expected as that's not optimized - but it is again identical -
regardless of which approach being used. The reason might be that the
example which is clearly obvious in the blog post uses union.
Now ... I do recognise that the type-punned stuff is critical, in fact
my compiler does warn about it with -O2. So I think moving over to
memcpy() makes sense ... So I do give this a Feature ACK. But we need
to have some proper testing so we're sure we don't break anything.
--
kind regards,
David Sommerseth
#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include <arpa/inet.h>
#define MR_MAX_ADDR_LEN 20
#define MR_ADDR_IPV4 2
#define MR_ADDR_MASK 3
struct mroute_addr {
uint8_t len; /* length of address */
uint8_t unused;
uint8_t type; /* MR_ADDR/MR_WITH flags */
uint8_t netbits; /* number of bits in network part of address,
valid if MR_WITH_NETBITS is set */
uint8_t addr[MR_MAX_ADDR_LEN]; /* actual address */
};
void
mroute_extract_in_addr_t (struct mroute_addr *dest, const in_addr_t src)
{
dest->type = MR_ADDR_IPV4;
dest->netbits = 0;
dest->len = 4;
#ifndef PATCHED
#warning Unpatched version
*(in_addr_t*)dest->addr = htonl (src);
#else
#warning PATCHED/memcpy version
in_addr_t tmp_addr = htonl (src);
memcpy(dest->addr, &tmp_addr, sizeof(uint32_t));
#endif
}
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
