Bug 57427 - ARM redboot_linux_exec should be changed to avoid miscompilation
ARM redboot_linux_exec should be changed to avoid miscompilation
Status: CLOSED WONTFIX
Product: eCos
Classification: Retired
Component: HAL (Show other bugs)
CVS
other Linux
medium Severity medium
: ---
: ---
Assigned To: Jesper Skov
Nick Garnett
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2001-12-12 03:08 EST by Jonathan Larmour
Modified: 2007-04-18 12:38 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2003-06-20 12:08:17 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jonathan Larmour 2001-12-12 03:08:02 EST
The redboot_linux_exec.c's for arm9 excalibur, aaed2000, ebsa285, mahwah,
sa110/var and sa11x0/var (at least right now) contain stuff that copies
around assembler fragments using labels e.g. &&end1

However as revealed in newer compilers, gcc can move these labels around if
they are not referenced to by gotos. A thread called "Re: asm volatile bug"
on gcc-local revealed that this was not going to change, so eCos should be
fixed.

The aaed2000 added the workaround:

#if 1
    // This is here to work around a arm-linux-gnu-gcc compiler bug
    if (entry)
        goto lab1;
    else
        goto end1;
#endif

at the end. The addition of the gotos fixes the labels. This could be added
for the other targets. The correct fix though is as rth wrote:

This code fragment should be written

asm ("
asm_start:
        code
asm_end:
");

with this whole bit outside _any_ function.  Then you write

        extern unsigned long asm_start[];
        extern unsigned long asm_end[];

        _prg = (unsigned long *) 0x1F00;
        n = asm_end - asm_start;
        for (i = 0; i < n; ++i)
          *_prg[i] = asm_start[i];

        _fn = (void (*)(void)) _prg;
        _fn();

Not sure what to do about the "returns to function's return address",
since I don't know if you can properly unwind the function's stack frame,
or if that even matters in this context.
Comment 1 Jesper Skov 2001-12-12 04:50:57 EST
Unfortunately, this doesn't work for the other cases where we use &&,
such as in the GDB locking macros. This is a grep for &&[^ \t] where I've pruned
out false hits:


./devs/flash/arm/mahwah/current/src/flash_query.c:87:    __mem_fault_handler =
&&no_device;
./hal/arm/arch/current/include/arm_stub.h:129:   
cyg_hal_gdb_place_break((target_register_t)((unsigned
long)&&cyg_hal_gdb_break_place + 1));\
./hal/arm/arch/current/include/arm_stub.h:137:   
cyg_hal_gdb_place_break((target_register_t)&&cyg_hal_gdb_break_place ); \
./hal/arm/arch/current/include/arm_stub.h:146:    cyg_hal_gdb_remove_break(
(target_register_t)&&cyg_hal_gdb_break_place ); \
./hal/arm/arm9/excalibur/current/src/redboot_linux_exec.c:307:    ip = (unsigned
long *)&&lab1;
./hal/arm/arm9/excalibur/current/src/redboot_linux_exec.c:312:    for (i = 0;  i
< (unsigned long)&&end1-(unsigned long)&&lab1;  i++) {
./hal/arm/arm9/aaed2000/current/src/redboot_linux_exec.c:305:    ip = (unsigned
long *)&&lab1;
./hal/arm/arm9/aaed2000/current/src/redboot_linux_exec.c:310:    for (i = 0;  i
< (unsigned long)&&end1-(unsigned long)&&lab1;  i++) {
./hal/arm/ebsa285/current/src/redboot_cmds.c:132:    ip = (unsigned long *)&&lab1;
./hal/arm/ebsa285/current/src/redboot_cmds.c:134:    for (i = 0;  i < (unsigned
long)&&end1-(unsigned long)&&lab1;  i++) {
./hal/arm/sa110/mahwah/current/src/redboot_cmds.c:132:    ip = (unsigned long
*)&&lab1;
./hal/arm/sa110/mahwah/current/src/redboot_cmds.c:134:    for (i = 0;  i <
(unsigned long)&&end1-(unsigned long)&&lab1;  i++) {
./hal/arm/sa110/var/current/src/redboot_cmds.c:89:    ip = (unsigned long *)&&lab1;
./hal/arm/sa110/var/current/src/redboot_cmds.c:90:    for (i = 0;  i < (unsigned
long)&&end1-(unsigned long)&&lab1;  i++) {
./hal/arm/sa11x0/var/current/src/redboot_linux_exec.c:302:    ip = (unsigned
long *)&&lab1;
./hal/arm/sa11x0/var/current/src/redboot_linux_exec.c:307:    for (i = 0;  i <
(unsigned long)&&end1-(unsigned long)&&lab1;  i++) {
./hal/common/current/include/hal_stub.h:229:    cyg_hal_gdb_place_break(
(target_register_t)&&cyg_hal_gdb_break_place ); \
./hal/common/current/include/hal_stub.h:237:    cyg_hal_gdb_remove_break(
(target_register_t)&&cyg_hal_gdb_break_place ); \
./hal/common/current/src/hal_misc.c:64:  (_x_) =
(CYG_ADDRWORD)&&__backup_return_address
./hal/common/current/src/hal_stub.c:766:    __mem_fault_handler = &&err;
./hal/common/current/src/hal_stub.c:847:    __mem_fault_handler = &&err;
./hal/common/current/src/hal_stub.c:901:    __mem_fault_handler = &&err;
./hal/i386/arch/current/src/i386_stub.c:280:    __mem_fault_handler = &&err;
./hal/i386/arch/current/src/i386_stub.c:306:    __mem_fault_handler = &&err;
./hal/frv/arch/current/include/frv_stub.h:135:   
cyg_hal_gdb_place_break((target_register_t)&&cyg_hal_gdb_break_place ); \
./kernel/current/tests/kcache2.c:645:    *start = (unsigned long) &&label_start;
./kernel/current/tests/kcache2.c:646:    *end = (unsigned long) &&label_end;
Comment 2 Nick Garnett 2002-01-02 09:49:31 EST
This has nothing to do with me!
Comment 3 Jonathan Larmour 2002-01-03 18:46:53 EST
Jesper was the one who "owned" this....
Comment 4 Alex Schuilenburg 2003-06-20 12:08:17 EDT
This bug has moved to http://bugs.ecos.sourceware.org/show_bug.cgi?id=57427

Note You need to log in before you can comment on or make changes to this bug.