* [U-Boot] [BUG] x86: invalid size calculations in interrupts.c with newer GCC
@ 2017-02-05 15:54 J. Tang
2017-02-06 6:35 ` Bin Meng
0 siblings, 1 reply; 4+ messages in thread
From: J. Tang @ 2017-02-05 15:54 UTC (permalink / raw)
To: u-boot
On latest master (commit 0ff27d), when using the stock qemu-x86_defconfig, it is possible to build an invalid U-Boot. I traced the issue to arch/x86/cpu/interrupts.c. The problem is that cpu_init_interrupts() assumes that each IRQ entry consumes the same number of bytes. Depending upon the compiler used, this assumption is incorrect.
Commit 564a9984 added this hunk:
+void irq_0(void);
+void irq_1(void);
int cpu_init_interrupts(void)
{
int i;
+ int irq_entry_size = irq_1 - irq_0;
+ void *irq_entry = (void *)irq_0;
When I used a Buildroot derived compiler, I got this assembly code for arch/x86/cpu/interrupts.o:
$ /home/tang/buildroot-x86/output/host/usr/bin/x86_64-linux-gcc --version
x86_64-linux-gcc.br_real (Buildroot 2016.11.1) 5.4.0
$ objdump -d interrupts.o
<snip>
00000207 <irq_18>:
207: 6a 12 push $0x12
209: eb 85 jmp 190 <irq_common_entry>
0000020b <irq_19>:
20b: 6a 13 push $0x13
20d: eb 81 jmp 190 <irq_common_entry>
0000020f <irq_20>:
20f: 6a 14 push $0x14
211: e9 7a ff ff ff jmp 190 <irq_common_entry>
00000216 <irq_21>:
216: 6a 15 push $0x15
218: e9 73 ff ff ff jmp 190 <irq_common_entry>
Based upon the offset difference, the newer GCC optimizer will use a shorter jmp opcode instruction. This results in varying size IRQ entries. As a comparison, a stock Debian compiler produced this assembly:
$ gcc --version
gcc (Debian 4.9.2-10) 4.9.2
$ objdump -d interrupts.o
<snip>
00000240 <irq_18>:
240: 6a 12 push $0x12
242: e9 fc ff ff ff jmp 243 <irq_18+0x3>
00000247 <irq_19>:
247: 6a 13 push $0x13
249: e9 fc ff ff ff jmp 24a <irq_19+0x3>
0000024e <irq_20>:
24e: 6a 14 push $0x14
250: e9 fc ff ff ff jmp 251 <irq_20+0x3>
00000255 <irq_21>:
255: 6a 15 push $0x15
257: e9 fc ff ff ff jmp 258 <irq_21+0x3>
I got around this temporarily by having directly referencing all 256 IRQ symbols, rather than compute the next IRQ address relative to the previous. Hopefully someone can come up with a more elegant solution.
--
Jason Tang / tang at jtang.org
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [BUG] x86: invalid size calculations in interrupts.c with newer GCC
2017-02-05 15:54 [U-Boot] [BUG] x86: invalid size calculations in interrupts.c with newer GCC J. Tang
@ 2017-02-06 6:35 ` Bin Meng
2017-02-07 2:51 ` J. Tang
0 siblings, 1 reply; 4+ messages in thread
From: Bin Meng @ 2017-02-06 6:35 UTC (permalink / raw)
To: u-boot
+Simon,
On Sun, Feb 5, 2017 at 11:54 PM, J. Tang <tang@jtang.org> wrote:
> On latest master (commit 0ff27d), when using the stock qemu-x86_defconfig, it is possible to build an invalid U-Boot. I traced the issue to arch/x86/cpu/interrupts.c. The problem is that cpu_init_interrupts() assumes that each IRQ entry consumes the same number of bytes. Depending upon the compiler used, this assumption is incorrect.
>
> Commit 564a9984 added this hunk:
>
> +void irq_0(void);
> +void irq_1(void);
>
> int cpu_init_interrupts(void)
> {
> int i;
>
> + int irq_entry_size = irq_1 - irq_0;
> + void *irq_entry = (void *)irq_0;
>
> When I used a Buildroot derived compiler, I got this assembly code for arch/x86/cpu/interrupts.o:
>
> $ /home/tang/buildroot-x86/output/host/usr/bin/x86_64-linux-gcc --version
> x86_64-linux-gcc.br_real (Buildroot 2016.11.1) 5.4.0
> $ objdump -d interrupts.o
> <snip>
> 00000207 <irq_18>:
> 207: 6a 12 push $0x12
> 209: eb 85 jmp 190 <irq_common_entry>
>
> 0000020b <irq_19>:
> 20b: 6a 13 push $0x13
> 20d: eb 81 jmp 190 <irq_common_entry>
>
> 0000020f <irq_20>:
> 20f: 6a 14 push $0x14
> 211: e9 7a ff ff ff jmp 190 <irq_common_entry>
>
> 00000216 <irq_21>:
> 216: 6a 15 push $0x15
> 218: e9 73 ff ff ff jmp 190 <irq_common_entry>
>
> Based upon the offset difference, the newer GCC optimizer will use a shorter jmp opcode instruction. This results in varying size IRQ entries. As a comparison, a stock Debian compiler produced this assembly:
>
> $ gcc --version
> gcc (Debian 4.9.2-10) 4.9.2
> $ objdump -d interrupts.o
> <snip>
> 00000240 <irq_18>:
> 240: 6a 12 push $0x12
> 242: e9 fc ff ff ff jmp 243 <irq_18+0x3>
>
> 00000247 <irq_19>:
> 247: 6a 13 push $0x13
> 249: e9 fc ff ff ff jmp 24a <irq_19+0x3>
>
> 0000024e <irq_20>:
> 24e: 6a 14 push $0x14
> 250: e9 fc ff ff ff jmp 251 <irq_20+0x3>
>
> 00000255 <irq_21>:
> 255: 6a 15 push $0x15
> 257: e9 fc ff ff ff jmp 258 <irq_21+0x3>
>
> I got around this temporarily by having directly referencing all 256 IRQ symbols, rather than compute the next IRQ address relative to the previous. Hopefully someone can come up with a more elegant solution.
>
Thanks for reporting this!
I do not have a GCC5 toolchain to test this. I suspect this is only
exposed with GCC5, or GCC 5.4? Is there any parameter to control the
behavior?
Regards,
Bin
^ permalink raw reply [flat|nested] 4+ messages in thread* [U-Boot] [BUG] x86: invalid size calculations in interrupts.c with newer GCC
2017-02-06 6:35 ` Bin Meng
@ 2017-02-07 2:51 ` J. Tang
2017-02-10 1:40 ` Bin Meng
0 siblings, 1 reply; 4+ messages in thread
From: J. Tang @ 2017-02-07 2:51 UTC (permalink / raw)
To: u-boot
> On 2017-02-06, at 01:35, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> +Simon,
>
> I do not have a GCC5 toolchain to test this. I suspect this is only
> exposed with GCC5, or GCC 5.4? Is there any parameter to control the
> behavior?
I observed a similar behavior with GCC 5.3.
As an experiment, I disabled CONFIG_CC_OPTIMIZE_FOR_SIZE. This did not change the sizes; the handler for IRQs 0 through 19 were still 4 bytes while the rest were 7 bytes.
Although I am not an expert x86 assembly writer, I was able to force the assembler to generate 32-bit jumps with the following:
diff --git a/arch/x86/cpu/interrupts.c b/arch/x86/cpu/interrupts.c
index 5f6cdd3..9917d09 100644
--- a/arch/x86/cpu/interrupts.c
+++ b/arch/x86/cpu/interrupts.c
@@ -32,7 +32,7 @@ DECLARE_GLOBAL_DATA_PTR;
".type irq_"#x", @function\n" \
"irq_"#x":\n" \
"pushl $"#x"\n" \
- "jmp irq_common_entry\n"
+ "jmp.d32 irq_common_entry\n"
static char *exceptions[] = {
"Divide Error",
This worked for both GCC 5.4 and 4.9.
--
Jason Tang / tang at jtang.org
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [BUG] x86: invalid size calculations in interrupts.c with newer GCC
2017-02-07 2:51 ` J. Tang
@ 2017-02-10 1:40 ` Bin Meng
0 siblings, 0 replies; 4+ messages in thread
From: Bin Meng @ 2017-02-10 1:40 UTC (permalink / raw)
To: u-boot
On Tue, Feb 7, 2017 at 10:51 AM, J. Tang <tang@jtang.org> wrote:
>
>> On 2017-02-06, at 01:35, Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> +Simon,
>>
>> I do not have a GCC5 toolchain to test this. I suspect this is only
>> exposed with GCC5, or GCC 5.4? Is there any parameter to control the
>> behavior?
>
> I observed a similar behavior with GCC 5.3.
>
> As an experiment, I disabled CONFIG_CC_OPTIMIZE_FOR_SIZE. This did not change the sizes; the handler for IRQs 0 through 19 were still 4 bytes while the rest were 7 bytes.
>
> Although I am not an expert x86 assembly writer, I was able to force the assembler to generate 32-bit jumps with the following:
>
> diff --git a/arch/x86/cpu/interrupts.c b/arch/x86/cpu/interrupts.c
> index 5f6cdd3..9917d09 100644
> --- a/arch/x86/cpu/interrupts.c
> +++ b/arch/x86/cpu/interrupts.c
> @@ -32,7 +32,7 @@ DECLARE_GLOBAL_DATA_PTR;
> ".type irq_"#x", @function\n" \
> "irq_"#x":\n" \
> "pushl $"#x"\n" \
> - "jmp irq_common_entry\n"
> + "jmp.d32 irq_common_entry\n"
>
> static char *exceptions[] = {
> "Divide Error",
>
> This worked for both GCC 5.4 and 4.9.
>
Looks good. Can you prepare a patch?
Regards,
Bin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-02-10 1:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-05 15:54 [U-Boot] [BUG] x86: invalid size calculations in interrupts.c with newer GCC J. Tang
2017-02-06 6:35 ` Bin Meng
2017-02-07 2:51 ` J. Tang
2017-02-10 1:40 ` Bin Meng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.