diff mbox

[v2,1/2] tools/foreign: Avoid using alignment directives when not appropriate

Message ID 1457550196-4287-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper March 9, 2016, 7:03 p.m. UTC
The foreign header generation blindly replaces 'uint64_t' with '__align8__
uint64_t', to get correct alignment when built as 32bit.  This is correct in
most circumstances, but Clang objects to two specific uses.

 * Inside a sizeof() expression
 * As part of a typecast

An example error looks like:

/local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44:
error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes]
    __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8];
                                           ^~~~~~~~~~
/local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36:
note: expanded from macro '__align8__'
                                   ^~~~~~~~~~~

This sedary is sufficient to fix all the bad examples without touching any of
the legitimate uses, and is more simple than teaching mkheader.py how to parse
C.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Julien Grall <julien.grall@arm.com>
CC: Jan Beulich <JBeulich@suse.com>

v2: Use .tmp file.  Make sedary able to deal with int64_t
---
 tools/include/xen-foreign/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Wei Liu March 9, 2016, 7:56 p.m. UTC | #1
On Wed, Mar 09, 2016 at 07:03:15PM +0000, Andrew Cooper wrote:
> The foreign header generation blindly replaces 'uint64_t' with '__align8__
> uint64_t', to get correct alignment when built as 32bit.  This is correct in
> most circumstances, but Clang objects to two specific uses.
> 
>  * Inside a sizeof() expression
>  * As part of a typecast
> 
> An example error looks like:
> 
> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44:
> error: 'aligned' attribute ignored when parsing type [-Werror,-Wignored-attributes]
>     __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8];
>                                            ^~~~~~~~~~
> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36:
> note: expanded from macro '__align8__'
>                                    ^~~~~~~~~~~
> 
> This sedary is sufficient to fix all the bad examples without touching any of
> the legitimate uses, and is more simple than teaching mkheader.py how to parse
> C.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Jan Beulich <JBeulich@suse.com>
> 
> v2: Use .tmp file.  Make sedary able to deal with int64_t
> ---
>  tools/include/xen-foreign/Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/include/xen-foreign/Makefile b/tools/include/xen-foreign/Makefile
> index 80a446a..bb09c93 100644
> --- a/tools/include/xen-foreign/Makefile
> +++ b/tools/include/xen-foreign/Makefile
> @@ -34,7 +34,10 @@ x86_32.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_32.h $(ROOT)/arch-x86/
>  	$(PYTHON) $< $* $@ $(filter %.h,$^)
>  
>  x86_64.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_64.h $(ROOT)/arch-x86/xen.h $(ROOT)/xen.h
> -	$(PYTHON) $< $* $@ $(filter %.h,$^)
> +	$(PYTHON) $< $* $@.tmp $(filter %.h,$^)
> +	#Avoid mixing an alignment directive with a uint64_t cast or sizeof expression
> +	sed 's/(__align8__ \(u\?int64_t\))/(\1)/g' -i $@.tmp
> +	$(call move-if-changed,$@.tmp,$@)
>  
>  checker.c: mkchecker.py structs.py
>  	$(PYTHON) $< $@ $(architectures)
> -- 
> 2.1.4
>
Jan Beulich March 10, 2016, 8:31 a.m. UTC | #2
>>> On 09.03.16 at 20:03, <andrew.cooper3@citrix.com> wrote:
> The foreign header generation blindly replaces 'uint64_t' with '__align8__
> uint64_t', to get correct alignment when built as 32bit.  This is correct in
> most circumstances, but Clang objects to two specific uses.
> 
>  * Inside a sizeof() expression
>  * As part of a typecast
> 
> An example error looks like:
> 
> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44:
> error: 'aligned' attribute ignored when parsing type 
> [-Werror,-Wignored-attributes]
>     __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8];
>                                            ^~~~~~~~~~
> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36:
> note: expanded from macro '__align8__'
>                                    ^~~~~~~~~~~
> 
> This sedary is sufficient to fix all the bad examples without touching any 
> of
> the legitimate uses, and is more simple than teaching mkheader.py how to 
> parse
> C.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich March 10, 2016, 8:33 a.m. UTC | #3
>>> On 09.03.16 at 20:56, <wei.liu2@citrix.com> wrote:
> On Wed, Mar 09, 2016 at 07:03:15PM +0000, Andrew Cooper wrote:
>> The foreign header generation blindly replaces 'uint64_t' with '__align8__
>> uint64_t', to get correct alignment when built as 32bit.  This is correct in
>> most circumstances, but Clang objects to two specific uses.
>> 
>>  * Inside a sizeof() expression
>>  * As part of a typecast
>> 
>> An example error looks like:
>> 
>> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44:
>> error: 'aligned' attribute ignored when parsing type 
> [-Werror,-Wignored-attributes]
>>     __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8];
>>                                            ^~~~~~~~~~
>> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36:
>> note: expanded from macro '__align8__'
>>                                    ^~~~~~~~~~~
>> 
>> This sedary is sufficient to fix all the bad examples without touching any 
> of
>> the legitimate uses, and is more simple than teaching mkheader.py how to 
> parse
>> C.
>> 
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Should I apply this and the 2nd patch, or can/should this wait
for Ian to be back?

Jan
Wei Liu March 10, 2016, 10:52 a.m. UTC | #4
On Thu, Mar 10, 2016 at 01:33:16AM -0700, Jan Beulich wrote:
> >>> On 09.03.16 at 20:56, <wei.liu2@citrix.com> wrote:
> > On Wed, Mar 09, 2016 at 07:03:15PM +0000, Andrew Cooper wrote:
> >> The foreign header generation blindly replaces 'uint64_t' with '__align8__
> >> uint64_t', to get correct alignment when built as 32bit.  This is correct in
> >> most circumstances, but Clang objects to two specific uses.
> >> 
> >>  * Inside a sizeof() expression
> >>  * As part of a typecast
> >> 
> >> An example error looks like:
> >> 
> >> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44:
> >> error: 'aligned' attribute ignored when parsing type 
> > [-Werror,-Wignored-attributes]
> >>     __align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8];
> >>                                            ^~~~~~~~~~
> >> /local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36:
> >> note: expanded from macro '__align8__'
> >>                                    ^~~~~~~~~~~
> >> 
> >> This sedary is sufficient to fix all the bad examples without touching any 
> > of
> >> the legitimate uses, and is more simple than teaching mkheader.py how to 
> > parse
> >> C.
> >> 
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > 
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Should I apply this and the 2nd patch, or can/should this wait
> for Ian to be back?
> 

You can go ahead if convenient.

Wei.

> Jan
>
diff mbox

Patch

diff --git a/tools/include/xen-foreign/Makefile b/tools/include/xen-foreign/Makefile
index 80a446a..bb09c93 100644
--- a/tools/include/xen-foreign/Makefile
+++ b/tools/include/xen-foreign/Makefile
@@ -34,7 +34,10 @@  x86_32.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_32.h $(ROOT)/arch-x86/
 	$(PYTHON) $< $* $@ $(filter %.h,$^)
 
 x86_64.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_64.h $(ROOT)/arch-x86/xen.h $(ROOT)/xen.h
-	$(PYTHON) $< $* $@ $(filter %.h,$^)
+	$(PYTHON) $< $* $@.tmp $(filter %.h,$^)
+	#Avoid mixing an alignment directive with a uint64_t cast or sizeof expression
+	sed 's/(__align8__ \(u\?int64_t\))/(\1)/g' -i $@.tmp
+	$(call move-if-changed,$@.tmp,$@)
 
 checker.c: mkchecker.py structs.py
 	$(PYTHON) $< $@ $(architectures)