On Thu, May 2, 2024 at 1:22 PM Ilya Maximets <[email protected]> wrote:
> On 5/2/24 12:22, Ales Musil wrote: > > The has was passed to memcpy as uin32_t *, however the hash bytes > > 'The has was passed' ? :) > Oops :) > > > might be unaligned at that point. Case it to uint8_t * instead > > which has only single byte alignment requirement. > > > > lib/hash.c:46:22: runtime error: load of misaligned address > 0x507000000065 for type 'const uint32_t *' (aka 'const unsigned int *'), > which requires 4 byte alignment > > 0x507000000065: note: pointer points here > > 73 62 2e 73 6f 63 6b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 00 00 00 00 00 00 00 00 00 > > ^ > > Please, wrap these lines. > Ack > > > #0 0x6191cb in hash_bytes /workspace/ovn/ovs/lib/hash.c:46:9 > > #1 0x69d064 in hash_string /workspace/ovn/ovs/lib/hash.h:404:12 > > #2 0x69d064 in hash_name /workspace/ovn/ovs/lib/shash.c:29:12 > > #3 0x69d064 in shash_find /workspace/ovn/ovs/lib/shash.c:237:49 > > #4 0x69dada in shash_find_data /workspace/ovn/ovs/lib/shash.c:251:31 > > #5 0x507987 in add_remote > /workspace/ovn/ovs/ovsdb/ovsdb-server.c:1382:15 > > #6 0x507987 in parse_options > /workspace/ovn/ovs/ovsdb/ovsdb-server.c:2659:13 > > #7 0x507987 in main /workspace/ovn/ovs/ovsdb/ovsdb-server.c:751:5 > > #8 0x7f47e3997087 in __libc_start_call_main > (/lib64/libc.so.6+0x2a087) (BuildId: > b098f1c75a76548bb230d8f551eae07a2aeccf06) > > #9 0x7f47e399714a in __libc_start_main@GLIBC_2.2.5 > (/lib64/libc.so.6+0x2a14a) (BuildId: > b098f1c75a76548bb230d8f551eae07a2aeccf06) > > #10 0x42de64 in _start > (/workspace/ovn/ovs/ovsdb/ovsdb-server+0x42de64) (BuildId: > 6c3f4e311556b29f84c9c4a5d6df5114dc08a12e) > > > > Please, remove the '#' signs as github misinterprets them as PR/issue > reference. And, please, remove the unnecessary info from the trace, > e.g. BuildId, '/workspace/ovn/' part of the paths and maybe some other > parts of the base libc frames. > Ack > > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior lib/hash.c:46:22 > > > > Signed-off-by: Ales Musil <[email protected]> > > --- > > lib/hash.c | 2 +- > > lib/jhash.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/lib/hash.c b/lib/hash.c > > index c722f3c3c..986fa6643 100644 > > --- a/lib/hash.c > > +++ b/lib/hash.c > > @@ -43,7 +43,7 @@ hash_bytes(const void *p_, size_t n, uint32_t basis) > > if (n) { > > uint32_t tmp = 0; > > > > - memcpy(&tmp, p, n); > > + memcpy(&tmp, (const uint8_t *) p, n); > > We may accept the change, however, this looks more like a compiler > bug to me. memcpy() accepts void pointers, so there is already an > implicit cast. I didn't look into assembly, but I'd guess clang > inlines the call and while doing that assumes the type. I'm not > sure it is allowed to do that. Also, the 'n' here is always less > than 4, so alignment should not be a problem because we can't copy > the whole thing in a single aligned instruction (maybe there are > instructions that can copy just 3 bytes without touching the 4th, > but idk). > > Did you have a look at the asm by any chance? > > As discussed offline, it doesn't inline and does the function call instead. > > Best regards, Ilya Maximets. > > Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
