Clang warning "increases required alignment" with -Wcast-align

[ravone]

When using macro HASH_ADD_INORDER I get warning:

warning: cast from 'char *' to 'UT_hash_handle *' (aka 'struct UT_hash_handle *') increases required alignment from 1 to 8
uthash.h:405:3: note: expanded from macro 'HASH_ADD_INORDER'
uthash.h:398:3: note: expanded from macro 'HASH_ADD_KEYPTR_INORDER'
uthash.h:377:5: note: expanded from macro 'HASH_ADD_KEYPTR_BYHASHVALUE_INORDER'
uthash.h:303:24: note: expanded from macro 'HASH_AKBI_INNER_LOOP'
uthash.h:147:32: note: expanded from macro 'HH_FROM_ELMT'

Perhaps, it's enough just fix this macro

#define HH_FROM_ELMT(tbl,elp) ((UT_hash_handle *)(((char*)(elp)) + ((tbl)->hho)))

to

#define HH_FROM_ELMT(tbl,elp) ((UT_hash_handle *)(void*)(((char*)(elp)) + ((tbl)->hho)))

[Quuxplusone]

177 is related.

Are you getting -Wcast-align from -Wall, or are you deliberately enabling it for some actual reason (and if so, is it warning you correctly about some real issue that should not be ignored on your platform)?

[ravone]

@Quuxplusone I get it from -Wall, no any other actual reason. Yeah, it does not seem to be real issue in my code and probably I can ignore it. But maybe we can cast through (void*) and make analyzer happy?

Pull request is totally related and I think that you can close this issue.

P.S. Thank you for your work of maintaining this project!

[Quuxplusone]

@ravone: I just checked Clang's source code, and I don't see any way to trigger that warning except to pass -Wcast-align explicitly (or to pass the magic driver option -Weverything, which you shouldn't expect to be clean). It's definitely not part of -Wall in Clang trunk, anyway.

If -Wcast-align were part of -Wall, I'd probably add the cast to shut up the warning: warning-cleanliness is important. But until then, I think maintaining the status quo is more important. In #177, it seemed that @brooksdavis's runtime platform maybe legitimately did not support the kind of cast that -Wcast-qual was warning about, so the warning may have been actually valid for that platform. Round-tripping through (void*) wouldn't change the runtime behavior of the code; it would merely obfuscate what the code was doing, enough to trick the compiler for a while... but then, if a later release of Clang got smart enough to see through the cast, it might just start warning again, right?

[ravone]

@Quuxplusone Think I figured out problem. For my project I use Qt Creator and CMake. In CMakeLists.txt I put -Wall -Wextra and I thought that this flags was used for analysis.

But it turned out that Qt Creator have built-in Clang code analyzer with using another predefined rules for checking. And you are right, this rule define -Weverything. Moreover this rule is default (at least for me).

[brooksdavis]

If -Wcast-align were part of -Wall, I'd probably add the cast to shut up the warning: warning-cleanliness is important. But until then, I think maintaining the status quo is more important. In #177, it seemed that @brooksdavis's runtime platform maybe legitimately did not support the kind of cast that -Wcast-qual was warning about, so the warning may have been actually valid for that platform. Round-tripping through (void*) wouldn't change the runtime behavior of the code; it would merely obfuscate what the code was doing, enough to trick the compiler for a while... but then, if a later release of Clang got smart enough to see through the cast, it might just start warning again, right?

I'm not terribly concerned about the compiler seeing through the cast. Standards people seem to view a (void *) cast as an escape hatch of the "I know what I'm doing, the assurance that this is aligned isn't visible in this translation unit" variety. It's plausible that a future standard might introduce a different, preferred spelling of this, but I'd expect (void *) to work through C2x.

[ravone]

Here is thread about extend -Wcast-align to make it more strict. Double casts through (void *) must be special case, without any warnings.

As @rjmccall (one of the key Clang contributor) said:

One of the fundamental design principles to keep in mind when implementing warnings like -Wcast-align is that we're trying to warn users about having written something dangerous, not scold them for writing code a particular way. Users do need to do these casts sometimes, and there has to be some way of doing them without a warning. So the question of when to warn has to consider whether the code is explicitly acknowledging the danger. It would, for example, be a mistake to warn in C about double-casts through (void*), because that is not something that people do accidentally; it is very likely that it is an attempt to suppress the warning.

[Quuxplusone]

Hmm, I suppose if we've got @rjmccall on record that (T*)(void*)charptr is a blessed way to suppress the -Wcast-align warning, then it wouldn't do any harm to add the (void*) to this macro and also the couple of other places it crops up (_utarray_eltptr and HASH_SELECT).

I started making a patch that would make all the tests -Wcast-align-clean, but then I saw that the warning also triggers on user code in test62.c and test90.c. I suspect both of those tests will need some changes. test62.c and in fact the entire MurmurHash/HASH_USING_NO_STRICT_ALIASING business seem more than a little sketchy in this decade. I might open a PR to remove MurmurHash entirely, I'm not sure yet.