mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* bugfix: _barebox_image_size wrong if enable
@ 2019-06-17 13:37 张忠山
  2019-06-18  7:46 ` Sascha Hauer
  0 siblings, 1 reply; 7+ messages in thread
From: 张忠山 @ 2019-06-17 13:37 UTC (permalink / raw)
  To: barebox

When a pbl image uncompress and call the normal barebox, In
barebox_non_pbl_start() it should call relocate_to_adr() to relocate barebox
to barebox_base. Ofcouse CONFIG_RELOCATABLE enabled.

And barebox_base calculated by:

	#define barebox_image_size	(unsigned int)&_barebox_image_size

	unsigned long barebox_size = barebox_image_size +
		((unsigned long)&__bss_stop - (unsigned long)&__bss_start);

_barebox_image_size, __bss_stop, __bss_start all defined in linkscript
"arch/arm/lib32/barebox.lds.S"

But when I print there value in function barebox_non_pbl_start() with the
flowwing code:

	putc_ll('X');
	putc_ll('\r');
	putc_ll('\n');
	PUTHEX_LL(barebox_size);
	putc_ll('\r');
	putc_ll('\n');
	PUTHEX_LL((unsigned long)&__bss_start);
	putc_ll('\r');
	putc_ll('\n');
	PUTHEX_LL((unsigned long)&__bss_stop);
	putc_ll('\r');
	putc_ll('\n');

The result as floww:

	X
	00000000
	00000000
	00000000

It's so strange, SO objdump it:

00017424 <barebox_non_pbl_start>:
   17424:       e59f61a0        ldr     r6, [pc, #416]  ; 175cc <barebox_non_pbl_start+0x1a8>
   17428:       e080a001        add     sl, r0, r1
   1742c:       e59f919c        ldr     r9, [pc, #412]  ; 175d0 <barebox_non_pbl_start+0x1ac>
   17430:       e1a08000        mov     r8, r0
   17434:       e92d4890        push    {r4, r7, fp, lr}
   17438:       e0895006        add     r5, r9, r6
   1743c:       e59fb190        ldr     fp, [pc, #400]  ; 175d4 <barebox_non_pbl_start+0x1b0>
   17440:       e3a00058        mov     r0, #88 ; 0x58
   17444:       e1a07002        mov     r7, r2
   17448:       ebfffca7        bl      166ec <PUTC_LL>
   1744c:       e06b5005        rsb     r5, fp, r5
   17450:       e3a0000d        mov     r0, #13
   17454:       ebfffca4        bl      166ec <PUTC_LL>
   17458:       e3a0000a        mov     r0, #10
   1745c:       ebfffca2        bl      166ec <PUTC_LL>
   17460:       e1a00005        mov     r0, r5
   17464:       ebffffe0        bl      173ec <PUTHEX_LL>
   17468:       e3a0000d        mov     r0, #13
   1746c:       ebfffc9e        bl      166ec <PUTC_LL>
   17470:       e3a0000a        mov     r0, #10
   17474:       ebfffc9c        bl      166ec <PUTC_LL>
   17478:       e1a0000b        mov     r0, fp
   1747c:       ebffffda        bl      173ec <PUTHEX_LL>
   17480:       e3a0000d        mov     r0, #13
   17484:       ebfffc98        bl      166ec <PUTC_LL>
   17488:       e3a0000a        mov     r0, #10
   1748c:       ebfffc96        bl      166ec <PUTC_LL>
   17490:       e1a00006        mov     r0, r6
   17494:       e24a6906        sub     r6, sl, #98304  ; 0x18000
   17498:       ebffffd3        bl      173ec <PUTHEX_LL>
   1749c:       e3c66dff        bic     r6, r6, #16320  ; 0x3fc0
   174a0:       e3a0000d        mov     r0, #13
   ......
   175c4:       ebffa2c5        bl      e0 <mem_malloc_init>
   175c8:       ebffa5a5        bl      c64 <start_barebox>
   175cc:       00000000        andeq   r0, r0, r0
   175d0:       00000000        andeq   r0, r0, r0
   175d4:       00000000        andeq   r0, r0, r0
   175d8:       00022448        andeq   r2, r2, r8, asr #8
   175dc:       d00dfeed        andle   pc, sp, sp, ror #29


We can see it save __bss_start and __bss_stop in local literal pool located at
175cc and 175d0, The value is zero.  But in barebox.map it's

                0x00000000000207b0                . = ALIGN (0x4)
                0x00000000000207b0                __bss_start = .
		......
                0x0000000000022458                __bss_stop = .
                0x0000000000022458                _end = .
                0x00000000000207b0                _barebox_image_size = __bss_start


Why?? It's so strange! Is it a bug of toolchain?

My toolchain is:

	arm-poky-linux-gnueabi-gcc (GCC) 5.3.0
	GNU ld (GNU Binutils) 2.26.0.20160214


I test this situation with a newer toolchain:

	arm-poky-eabi-gcc (GCC) 8.2.0
	GNU ld (GNU Binutils) 2.31.1.20180818

With this toolchain the printhex_ll output value for __bss_start and
__bss_stop same as them in barebox.map. This cofused me more!!

And with new toolchain there is a new thing: As barebox_non_pbl_start()
running address is not it's link address. So if use PUTS_LL(const char *str)
It should crash. but it sure works. I check the dis-asm code. Found the
linker/gcc add some code to using the string ptr pc relatived. Does this the
new toolchain's benifit? And who ? gcc or linker ?


The flowwing patch let _barebox_image_size has right value

-----------------------------------8<----------------------------- 
From 29420237496b23c97de03c189529b223902653aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=BC=A0=E5=BF=A0=E5=B1=B1?= <zzs213@126.com>
Date: Mon, 17 Jun 2019 17:43:44 +0800
Subject: [PATCH] bugfix: _barebox_image_size wrong if enable
 CONFIG_RELOCATABLE
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: 张忠山 <zzs213@126.com>
---
 arch/arm/lib32/barebox.lds.S | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/arm/lib32/barebox.lds.S b/arch/arm/lib32/barebox.lds.S
index 53a5f55cc..49224a79c 100644
--- a/arch/arm/lib32/barebox.lds.S
+++ b/arch/arm/lib32/barebox.lds.S
@@ -20,16 +20,18 @@
 
 #include <asm-generic/barebox.lds.h>
 
+#ifdef CONFIG_RELOCATABLE
+#define BASE 0x0
+#else
+#define BASE TEXT_BASE
+#endif
+
 OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
 OUTPUT_ARCH(arm)
 ENTRY(start)
 SECTIONS
 {
-#ifdef CONFIG_RELOCATABLE
-	. = 0x0;
-#else
-	. = TEXT_BASE;
-#endif
+	. = BASE;
 
 #ifndef CONFIG_PBL_IMAGE
 	PRE_IMAGE
@@ -124,5 +126,5 @@ SECTIONS
 	.bss : { *(.bss*) }
 	__bss_stop = .;
 	_end = .;
-	_barebox_image_size = __bss_start - TEXT_BASE;
+	_barebox_image_size = __bss_start - BASE;
 }
-- 
2.21.0

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: bugfix: _barebox_image_size wrong if enable
  2019-06-17 13:37 bugfix: _barebox_image_size wrong if enable 张忠山
@ 2019-06-18  7:46 ` Sascha Hauer
  2019-06-18 10:17   ` 张忠山
  0 siblings, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2019-06-18  7:46 UTC (permalink / raw)
  To: 张忠山; +Cc: barebox

On Mon, Jun 17, 2019 at 09:37:19PM +0800, 张忠山 wrote:
> When a pbl image uncompress and call the normal barebox, In
> barebox_non_pbl_start() it should call relocate_to_adr() to relocate barebox
> to barebox_base. Ofcouse CONFIG_RELOCATABLE enabled.
> 
> And barebox_base calculated by:
> 
> 	#define barebox_image_size	(unsigned int)&_barebox_image_size
> 
> 	unsigned long barebox_size = barebox_image_size +
> 		((unsigned long)&__bss_stop - (unsigned long)&__bss_start);
> 
> _barebox_image_size, __bss_stop, __bss_start all defined in linkscript
> "arch/arm/lib32/barebox.lds.S"
> 
> But when I print there value in function barebox_non_pbl_start() with the
> flowwing code:
> 
> 	putc_ll('X');
> 	putc_ll('\r');
> 	putc_ll('\n');
> 	PUTHEX_LL(barebox_size);
> 	putc_ll('\r');
> 	putc_ll('\n');
> 	PUTHEX_LL((unsigned long)&__bss_start);
> 	putc_ll('\r');
> 	putc_ll('\n');
> 	PUTHEX_LL((unsigned long)&__bss_stop);
> 	putc_ll('\r');
> 	putc_ll('\n');
> 
> The result as floww:
> 
> 	X
> 	00000000
> 	00000000
> 	00000000
> 
> It's so strange, SO objdump it:
> 
> 00017424 <barebox_non_pbl_start>:
>    17424:       e59f61a0        ldr     r6, [pc, #416]  ; 175cc <barebox_non_pbl_start+0x1a8>
>    17428:       e080a001        add     sl, r0, r1
>    1742c:       e59f919c        ldr     r9, [pc, #412]  ; 175d0 <barebox_non_pbl_start+0x1ac>
>    17430:       e1a08000        mov     r8, r0
>    17434:       e92d4890        push    {r4, r7, fp, lr}
>    17438:       e0895006        add     r5, r9, r6
>    1743c:       e59fb190        ldr     fp, [pc, #400]  ; 175d4 <barebox_non_pbl_start+0x1b0>
>    17440:       e3a00058        mov     r0, #88 ; 0x58
>    17444:       e1a07002        mov     r7, r2
>    17448:       ebfffca7        bl      166ec <PUTC_LL>
>    1744c:       e06b5005        rsb     r5, fp, r5
>    17450:       e3a0000d        mov     r0, #13
>    17454:       ebfffca4        bl      166ec <PUTC_LL>
>    17458:       e3a0000a        mov     r0, #10
>    1745c:       ebfffca2        bl      166ec <PUTC_LL>
>    17460:       e1a00005        mov     r0, r5
>    17464:       ebffffe0        bl      173ec <PUTHEX_LL>
>    17468:       e3a0000d        mov     r0, #13
>    1746c:       ebfffc9e        bl      166ec <PUTC_LL>
>    17470:       e3a0000a        mov     r0, #10
>    17474:       ebfffc9c        bl      166ec <PUTC_LL>
>    17478:       e1a0000b        mov     r0, fp
>    1747c:       ebffffda        bl      173ec <PUTHEX_LL>
>    17480:       e3a0000d        mov     r0, #13
>    17484:       ebfffc98        bl      166ec <PUTC_LL>
>    17488:       e3a0000a        mov     r0, #10
>    1748c:       ebfffc96        bl      166ec <PUTC_LL>
>    17490:       e1a00006        mov     r0, r6
>    17494:       e24a6906        sub     r6, sl, #98304  ; 0x18000
>    17498:       ebffffd3        bl      173ec <PUTHEX_LL>
>    1749c:       e3c66dff        bic     r6, r6, #16320  ; 0x3fc0
>    174a0:       e3a0000d        mov     r0, #13
>    ......
>    175c4:       ebffa2c5        bl      e0 <mem_malloc_init>
>    175c8:       ebffa5a5        bl      c64 <start_barebox>
>    175cc:       00000000        andeq   r0, r0, r0
>    175d0:       00000000        andeq   r0, r0, r0
>    175d4:       00000000        andeq   r0, r0, r0
>    175d8:       00022448        andeq   r2, r2, r8, asr #8
>    175dc:       d00dfeed        andle   pc, sp, sp, ror #29
> 
> 
> We can see it save __bss_start and __bss_stop in local literal pool located at
> 175cc and 175d0, The value is zero.  But in barebox.map it's
> 
>                 0x00000000000207b0                . = ALIGN (0x4)
>                 0x00000000000207b0                __bss_start = .
> 		......
>                 0x0000000000022458                __bss_stop = .
>                 0x0000000000022458                _end = .
>                 0x00000000000207b0                _barebox_image_size = __bss_start
> 
> 
> Why?? It's so strange! Is it a bug of toolchain?

No, it is corrected during runtime in relocate_to_current_adr(). It
seems older compilers need a runtime relocation fixup for this.

AFAIR this only happened for linker variables that point to absolute
addresses. Differences between addresses also worked with the older
compilers, and I think this is what your patch does: With this the
linker is smart enough to recognize _barebox_image_size as a relative
size and not an absolute address. So I think your patch is correct.

Unfortunately I can't find any toolchain anymore to reproduce this
issue.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: bugfix: _barebox_image_size wrong if enable
  2019-06-18  7:46 ` Sascha Hauer
@ 2019-06-18 10:17   ` 张忠山
  2019-06-24  3:37     ` 张忠山
  2019-06-26  7:20     ` Sascha Hauer
  0 siblings, 2 replies; 7+ messages in thread
From: 张忠山 @ 2019-06-18 10:17 UTC (permalink / raw)
  To: barebox; +Cc:> No, it is corrected during runtime in relocate_to_current_adr(). It
> seems older compilers need a runtime relocation fixup for this.

Yes, in relocate_to_current_adr() the address all be fixed up.

But if _barebox_image_size, __bss_start and __bss_stop all zero. the
barebox_base calculated by arm_mem_barebox_image() would wrong. but because it
align the base to 1M. So mostly it works fine. If the barebox size larger than
1M. It should fail.

>
> AFAIR this only happened for linker variables that point to absolute
> addresses. Differences between addresses also worked with the older
> compilers, and I think this is what your patch does:

No. my patch just for new toolchain. with old toolchain it has no effect,
Because all of the size is zero.

By using the newer toolchain:
	arm-poky-eabi-gcc (GCC) 8.2.0
	GNU ld (GNU Binutils) 2.31.1.20180818

In barebox config file:
        CONFIG_TEXT_BASE=0x23e00000
        CONFIG_RELOCATABLE=y

Without my patch, in barebox.map

                0x000000000001ff40                __bss_start = .

                0x0000000000021bec                __bss_stop = .
                0x0000000000021bec                _end = .
                0xffffffffdc21ff40                _barebox_image_size = (__bss_start - 0x23e00000)

In code, printout barebox_base calculated by arm_mem_barebox_image():

        barebox_image_size     : 0xdc21ff40
        __bss_stop     : 0x00021bec
        __bss_start     : 0x0001ff40
        membase     : 0x20000000
        endmem      : 0x60000000
        barebox_base: 0x83d00000

barebox_base outof memory region!

After using my patch, the outpus is:

        barebox_image_size     : 0x0001ff40
        __bss_stop     : 0x00021bec
        __bss_start     : 0x0001ff40
        membase     : 0x20000000
        endmem      : 0x60000000
        barebox_base: 0x5ff00000


-- 
Best Regards,
zzs


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: bugfix: _barebox_image_size wrong if enable
  2019-06-18 10:17   ` 张忠山
@ 2019-06-24  3:37     ` 张忠山
  2019-06-26  7:20     ` Sascha Hauer
  1 sibling, 0 replies; 7+ messages in thread
From: 张忠山 @ 2019-06-24  3:37 UTC (permalink / raw)
  To: barebox

How about this? Am wrong?

-- 
Best Regards,
zzs


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: bugfix: _barebox_image_size wrong if enable
  2019-06-18 10:17   ` 张忠山
  2019-06-24  3:37     ` 张忠山
@ 2019-06-26  7:20     ` Sascha Hauer
  2019-06-26  9:39       ` 张忠山
  1 sibling, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2019-06-26  7:20 UTC (permalink / raw)
  To: 张忠山; +Cc: barebox

On Tue, Jun 18, 2019 at 06:17:52PM +0800, 张忠山 wrote:
> > No, it is corrected during runtime in relocate_to_current_adr(). It
> > seems older compilers need a runtime relocation fixup for this.
> 
> Yes, in relocate_to_current_adr() the address all be fixed up.
> 
> But if _barebox_image_size, __bss_start and __bss_stop all zero. the
> barebox_base calculated by arm_mem_barebox_image() would wrong. but because it
> align the base to 1M. So mostly it works fine. If the barebox size larger than
> 1M. It should fail.
> 
> >
> > AFAIR this only happened for linker variables that point to absolute
> > addresses. Differences between addresses also worked with the older
> > compilers, and I think this is what your patch does:
> 
> No. my patch just for new toolchain. with old toolchain it has no effect,
> Because all of the size is zero.
> 
> By using the newer toolchain:
> 	arm-poky-eabi-gcc (GCC) 8.2.0
> 	GNU ld (GNU Binutils) 2.31.1.20180818
> 
> In barebox config file:
>         CONFIG_TEXT_BASE=0x23e00000
>         CONFIG_RELOCATABLE=y

With CONFIG_RELOCATABLE enabled TEXT_BASE should be 0x0 and this should
be enforced by Kconfig dependencies. Which barebox version are you on
and which SoC are you compiling for?

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: bugfix: _barebox_image_size wrong if enable
  2019-06-26  7:20     ` Sascha Hauer
@ 2019-06-26  9:39       ` 张忠山
  2019-06-27 10:30         ` Sascha Hauer
  0 siblings, 1 reply; 7+ messages in thread
From: 张忠山 @ 2019-06-26  9:39 UTC (permalink / raw)
  To: barebox

>>>
>>> AFAIR this only happened for linker variables that point to absolute
>>> addresses. Differences between addresses also worked with the older
>>> compilers, and I think this is what your patch does:
>>
>> No. my patch just for new toolchain. with old toolchain it has no effect,
>> Because all of the size is zero.
>>
>> By using the newer toolchain:
>> 	arm-poky-eabi-gcc (GCC) 8.2.0
>> 	GNU ld (GNU Binutils) 2.31.1.20180818
>>
>> In barebox config file:
>>         CONFIG_TEXT_BASE=0x23e00000
>>         CONFIG_RELOCATABLE=y
> 
> With CONFIG_RELOCATABLE enabled TEXT_BASE should be 0x0 and this should
> be enforced by Kconfig dependencies. Which barebox version are you on
> and which SoC are you compiling for?

I'm using v2016.10.0 and my custom board based \aon friendlyarm-tiny2
-- 
Best Regards,
zzs


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: bugfix: _barebox_image_size wrong if enable
  2019-06-26  9:39       ` 张忠山
@ 2019-06-27 10:30         ` Sascha Hauer
  0 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2019-06-27 10:30 UTC (permalink / raw)
  To: 张忠山; +Cc: barebox

On Wed, Jun 26, 2019 at 05:39:09PM +0800, 张忠山 wrote:
> >>>
> >>> AFAIR this only happened for linker variables that point to absolute
> >>> addresses. Differences between addresses also worked with the older
> >>> compilers, and I think this is what your patch does:
> >>
> >> No. my patch just for new toolchain. with old toolchain it has no effect,
> >> Because all of the size is zero.
> >>
> >> By using the newer toolchain:
> >> 	arm-poky-eabi-gcc (GCC) 8.2.0
> >> 	GNU ld (GNU Binutils) 2.31.1.20180818
> >>
> >> In barebox config file:
> >>         CONFIG_TEXT_BASE=0x23e00000
> >>         CONFIG_RELOCATABLE=y
> > 
> > With CONFIG_RELOCATABLE enabled TEXT_BASE should be 0x0 and this should
> > be enforced by Kconfig dependencies. Which barebox version are you on
> > and which SoC are you compiling for?
> 
> I'm using v2016.10.0 and my custom board based \aon friendlyarm-tiny2

This is really old. Please update to something more recent.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-06-27 10:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17 13:37 bugfix: _barebox_image_size wrong if enable 张忠山
2019-06-18  7:46 ` Sascha Hauer
2019-06-18 10:17   ` 张忠山
2019-06-24  3:37     ` 张忠山
2019-06-26  7:20     ` Sascha Hauer
2019-06-26  9:39       ` 张忠山
2019-06-27 10:30         ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox