Re: [PATCH] Fix implicit-signed-integer-truncation in hash function

[Tim Rühsen]

Pushed.

On 18.05.20 10:18, Jose E. Marchesi wrote:
> 
>     >     The ASAN report was:
>     >     pkl-env.c:74:12: runtime error: implicit conversion from type 
> 'unsigned long'
>     >     of value 591105371825 (64-bit, unsigned) to type 'int' changed the 
> value to
>     >     -1600115023 (32-bit, signed)
>     >     
>     >     2020-05-17  Tim Rühsen  <address@hidden>
>     >     
>     >             * libpoke/pkl-env.c (hash_string): Suppress ASAN runtime 
> error
>     >             "implicit-signed-integer-truncation-or-sign-change".
>     >     ---
>     >      ChangeLog         | 5 +++++
>     >      libpoke/pkl-env.c | 4 +++-
>     >      2 files changed, 8 insertions(+), 1 deletion(-)
>     >     
>     >     diff --git a/libpoke/pkl-env.c b/libpoke/pkl-env.c
>     >     index e1a74d42..636d0a84 100644
>     >     --- a/libpoke/pkl-env.c
>     >     +++ b/libpoke/pkl-env.c
>     >     @@ -60,7 +60,9 @@ struct pkl_env
>     >     
>     >      /* The hash tables above are handled using the following
>     >         functions.  */
>     >     -
>     >     +#ifdef __clang__
>     >     +__attribute__((no_sanitize("integer")))
>     >     +#endif
>     >      static int
>     >      hash_string (const char *name)
>     >      {
>     > 
>     > Hm is the sanitizer not reporting the error in GCC?
>     
>     No, not for gcc-9.
>     
>     gcc (at least gcc-9) doesn't have an "integer" check mode, which groups
>     several integer-related tests.
>     
>     In the past, new sanitizer options were introduced in llvm/clang first,
>     then slowly picked up by gcc. So my hope is, gcc picks up "integer" mode
>     at some day.
> 
> Yes, GCC updates its port of the sanitizers from time to time.
>     
>     From my experience, testing ASAN and UBSAN with clang finds way more
>     issues than gcc does.
>     
>     In this special case, I prefer an attribute in the code. Because the
>     hash functions often rely on integer overflow (and other integer stuff
>     that might trigger a sanitizer).
>     
>     To handle false positives or otherwise unfixable code, I prefer a
>     suppression file.
> 
> OK for master then.
> Thanks!
>

signature.asc
Description: OpenPGP digital signature