Re: [PATCH] Add a -Wcast-align=strict warning
On Wed, 13 Sep 2017, Bernd Edlinger wrote: > So you suggest to use min_align_of_type instead of TYPE_ALIGN. > > That would also make sense for the traditional -Wcast-align on > strict-alignment targets, right? Yes, and yes (though I'm not sure if any strict-alignment targets have this peculiarity). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add a -Wcast-align=strict warning
On 09/13/17 22:03, Joseph Myers wrote: > On Wed, 13 Sep 2017, Bernd Edlinger wrote: > >> On 09/13/17 19:06, Joseph Myers wrote: >>> What does this warning do in cases where a type has different alignments >>> inside and outside structs? I'm thinking of something like >>> >>> struct s { long long x; } *p; >>> /* ... */ >>> (long long *)p >>> >>> on 32-bit x86 - where long long's preferred alignment is 8 bytes, but in >>> structures it's 4 bytes. (Likewise for double in place of long long.) I >>> think a warning for a (long long *)p cast might be surprising in that >>> case. >>> >> >> Well, yes this does get a warning. But doesn't that cast then violate >> the underlying alignment requirement of long long* ? > > That's the difference between preferred alignment (__alignof__) and > alignment required in all contexts (C11 _Alignof). The above seems valid, > just like it's valid to take the address of a long long struct element. > That is, the alignment for the target of a pointer to long long is really > 4 bytes here, even though the alignment for a standalone long long object > is 8 bytes. And there's a case for the warning to look at the required > alignment in all contexts, not TYPE_ALIGN. > So you suggest to use min_align_of_type instead of TYPE_ALIGN. That would also make sense for the traditional -Wcast-align on strict-alignment targets, right? Thanks, Bernd.
Re: [PATCH] Add a -Wcast-align=strict warning
On Wed, 13 Sep 2017, Bernd Edlinger wrote: > On 09/13/17 19:06, Joseph Myers wrote: > > What does this warning do in cases where a type has different alignments > > inside and outside structs? I'm thinking of something like > > > > struct s { long long x; } *p; > > /* ... */ > > (long long *)p > > > > on 32-bit x86 - where long long's preferred alignment is 8 bytes, but in > > structures it's 4 bytes. (Likewise for double in place of long long.) I > > think a warning for a (long long *)p cast might be surprising in that > > case. > > > > Well, yes this does get a warning. But doesn't that cast then violate > the underlying alignment requirement of long long* ? That's the difference between preferred alignment (__alignof__) and alignment required in all contexts (C11 _Alignof). The above seems valid, just like it's valid to take the address of a long long struct element. That is, the alignment for the target of a pointer to long long is really 4 bytes here, even though the alignment for a standalone long long object is 8 bytes. And there's a case for the warning to look at the required alignment in all contexts, not TYPE_ALIGN. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add a -Wcast-align=strict warning
On 09/13/17 19:06, Joseph Myers wrote: > What does this warning do in cases where a type has different alignments > inside and outside structs? I'm thinking of something like > > struct s { long long x; } *p; > /* ... */ > (long long *)p > > on 32-bit x86 - where long long's preferred alignment is 8 bytes, but in > structures it's 4 bytes. (Likewise for double in place of long long.) I > think a warning for a (long long *)p cast might be surprising in that > case. > Well, yes this does get a warning. But doesn't that cast then violate the underlying alignment requirement of long long* ? Of course there is probably a reason why -Wcast-align is not enabled by default, and likewise this warning emits a fair amount of false positives, but nevertheless I think it is often worth looking at the places where this warning flags a possible alignment issue. However, neither -Wcast-align nor -Wcast-align=strict are enabled unless explicitly requested. Bernd.
Re: [PATCH] Add a -Wcast-align=strict warning
What does this warning do in cases where a type has different alignments inside and outside structs? I'm thinking of something like struct s { long long x; } *p; /* ... */ (long long *)p on 32-bit x86 - where long long's preferred alignment is 8 bytes, but in structures it's 4 bytes. (Likewise for double in place of long long.) I think a warning for a (long long *)p cast might be surprising in that case. -- Joseph S. Myers jos...@codesourcery.com
[PING] [PATCH] Add a -Wcast-align=strict warning
Ping... On 09/04/17 10:07, Bernd Edlinger wrote: > Hi, > > as you know we have a -Wcast-align warning which works only for > STRICT_ALIGNMENT targets. But occasionally it would be nice to be > able to switch this warning on even for other targets. > > Therefore I would like to add a strict version of this option > which can be invoked with -Wcast-align=strict. With the only > difference that it does not depend on STRICT_ALIGNMENT. > > I used the code from check_effective_target_non_strict_align > in target-supports.exp for the first version of the test case, > where we have this: > > return [check_no_compiler_messages non_strict_align assembly { > char *y; > typedef char __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__))) c; > c *z; > void foo(void) { z = (c *) y; } > } "-Wcast-align"] > > ... and to my big surprise it did _not_ work for C++ as-is, > because same_type_p considers differently aligned types identical, > and therefore cp_build_c_cast tries the conversion first via a > const_cast which succeeds, but did not emit the cast-align warning > in this case. > > As a work-around I had to check the alignment in build_const_cast_1 > as well. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk? > > > Thanks > Bernd. >
[PATCH] Add a -Wcast-align=strict warning
Hi, as you know we have a -Wcast-align warning which works only for STRICT_ALIGNMENT targets. But occasionally it would be nice to be able to switch this warning on even for other targets. Therefore I would like to add a strict version of this option which can be invoked with -Wcast-align=strict. With the only difference that it does not depend on STRICT_ALIGNMENT. I used the code from check_effective_target_non_strict_align in target-supports.exp for the first version of the test case, where we have this: return [check_no_compiler_messages non_strict_align assembly { char *y; typedef char __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__))) c; c *z; void foo(void) { z = (c *) y; } } "-Wcast-align"] ... and to my big surprise it did _not_ work for C++ as-is, because same_type_p considers differently aligned types identical, and therefore cp_build_c_cast tries the conversion first via a const_cast which succeeds, but did not emit the cast-align warning in this case. As a work-around I had to check the alignment in build_const_cast_1 as well. Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. gcc: 2017-09-03 Bernd Edlinger* common.opt (Wcast-align=strict): New warning option. * doc/invoke.texi: Document -Wcast-align=strict. c: 2017-09-03 Bernd Edlinger * c-typeck.c (build_c_cast): Implement -Wcast-align=strict. cp: 2017-09-03 Bernd Edlinger * typeck.c (build_reinterpret_cast_1, build_const_cast_1): Implement -Wcast-align=strict. testsuite: 2017-09-03 Bernd Edlinger * c-c++-common/Wcast-align.c: New test. Index: gcc/c/c-typeck.c === --- gcc/c/c-typeck.c (revision 251617) +++ gcc/c/c-typeck.c (working copy) @@ -5578,7 +5578,7 @@ build_c_cast (location_t loc, tree type, tree expr } /* Warn about possible alignment problems. */ - if (STRICT_ALIGNMENT + if ((STRICT_ALIGNMENT || warn_cast_align == 2) && TREE_CODE (type) == POINTER_TYPE && TREE_CODE (otype) == POINTER_TYPE && TREE_CODE (TREE_TYPE (otype)) != VOID_TYPE Index: gcc/common.opt === --- gcc/common.opt (revision 251617) +++ gcc/common.opt (working copy) @@ -564,6 +564,10 @@ Wcast-align Common Var(warn_cast_align) Warning Warn about pointer casts which increase alignment. +Wcast-align=strict +Common Var(warn_cast_align,2) Warning +Warn about pointer casts which increase alignment. + Wcpp Common Var(warn_cpp) Init(1) Warning Warn when a #warning directive is encountered. Index: gcc/cp/typeck.c === --- gcc/cp/typeck.c (revision 251617) +++ gcc/cp/typeck.c (working copy) @@ -7265,8 +7265,8 @@ build_reinterpret_cast_1 (tree type, tree expr, bo complain)) return error_mark_node; /* Warn about possible alignment problems. */ - if (STRICT_ALIGNMENT && warn_cast_align - && (complain & tf_warning) + if ((STRICT_ALIGNMENT || warn_cast_align == 2) + && (complain & tf_warning) && !VOID_TYPE_P (type) && TREE_CODE (TREE_TYPE (intype)) != FUNCTION_TYPE && COMPLETE_TYPE_P (TREE_TYPE (type)) @@ -7273,7 +7273,7 @@ build_reinterpret_cast_1 (tree type, tree expr, bo && COMPLETE_TYPE_P (TREE_TYPE (intype)) && TYPE_ALIGN (TREE_TYPE (type)) > TYPE_ALIGN (TREE_TYPE (intype))) warning (OPT_Wcast_align, "cast from %qH to %qI " - "increases required alignment of target type", intype, type); + "increases required alignment of target type", intype, type); /* We need to strip nops here, because the front end likes to create (int *) for array-to-pointer decay, instead of [0]. */ @@ -7447,6 +7447,14 @@ build_const_cast_1 (tree dst_type, tree expr, tsub the user is making a potentially unsafe cast. */ check_for_casting_away_constness (src_type, dst_type, CAST_EXPR, complain); + /* ??? comp_ptr_ttypes_const ignores TYPE_ALIGN. */ + if ((STRICT_ALIGNMENT || warn_cast_align == 2) + && (complain & tf_warning) + && TYPE_ALIGN (TREE_TYPE (dst_type)) + > TYPE_ALIGN (TREE_TYPE (src_type))) + warning (OPT_Wcast_align, "cast from %qH to %qI " + "increases required alignment of target type", + src_type, dst_type); } if (reference_type) { Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 251617) +++ gcc/doc/invoke.texi (working copy) @@ -266,7 +266,8 @@ Objective-C and Objective-C++ Dialects}. -Wno-attributes -Wbool-compare -Wbool-operation @gol -Wno-builtin-declaration-mismatch @gol -Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol --Wc++-compat -Wc++11-compat