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

[Jose E. Marchesi]

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