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' ? :)

> 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.

>     #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.

> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior lib/hash.c:46:22
> 
> Signed-off-by: Ales Musil <amu...@redhat.com>
> ---
>  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?

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to