| Message ID | 20180618152315.34233-16-agraf@suse.de |
|---|---|
| State | Superseded |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | sandbox: efi_loader support | expand |
Hi Alex, On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote: > In sandbox, longjmp returns to itself in an endless loop. Cut this through > by calling the real OS function. > > Setjmp on the other hand must not return. So here we have to call the OS > setjmp function straight from the code where the setjmp call happens. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > arch/sandbox/cpu/cpu.c | 5 ----- > arch/sandbox/cpu/os.c | 20 ++------------------ > arch/sandbox/include/asm/setjmp.h | 4 +++- > 3 files changed, 5 insertions(+), 24 deletions(-) > I think we do need to do something like this. But I cannot find the man page for _longjmp(). Are you sure this is a valid function on all systems? Or is it Linux only? Regards. Simon
On 06/21/2018 04:02 AM, Simon Glass wrote: > Hi Alex, > > On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote: >> In sandbox, longjmp returns to itself in an endless loop. Cut this through >> by calling the real OS function. >> >> Setjmp on the other hand must not return. So here we have to call the OS >> setjmp function straight from the code where the setjmp call happens. >> >> Signed-off-by: Alexander Graf <agraf@suse.de> >> --- >> arch/sandbox/cpu/cpu.c | 5 ----- >> arch/sandbox/cpu/os.c | 20 ++------------------ >> arch/sandbox/include/asm/setjmp.h | 4 +++- >> 3 files changed, 5 insertions(+), 24 deletions(-) >> > I think we do need to do something like this. But I cannot find the > man page for _longjmp(). Are you sure this is a valid function on all > systems? Or is it Linux only? I'm afraid it may be Linux (glibc) only. I guess the alternative to this would be to not use system setjmp/longjmp at all and instead use the in-U-Boot versions of them. Alex
Hi Alex, On 21 June 2018 at 03:41, Alexander Graf <agraf@suse.de> wrote: > > On 06/21/2018 04:02 AM, Simon Glass wrote: >> >> Hi Alex, >> >> On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote: >>> >>> In sandbox, longjmp returns to itself in an endless loop. Cut this through >>> by calling the real OS function. >>> >>> Setjmp on the other hand must not return. So here we have to call the OS >>> setjmp function straight from the code where the setjmp call happens. >>> >>> Signed-off-by: Alexander Graf <agraf@suse.de> >>> --- >>> arch/sandbox/cpu/cpu.c | 5 ----- >>> arch/sandbox/cpu/os.c | 20 ++------------------ >>> arch/sandbox/include/asm/setjmp.h | 4 +++- >>> 3 files changed, 5 insertions(+), 24 deletions(-) >>> >> I think we do need to do something like this. But I cannot find the >> man page for _longjmp(). Are you sure this is a valid function on all >> systems? Or is it Linux only? > > > I'm afraid it may be Linux (glibc) only. > > I guess the alternative to this would be to not use system setjmp/longjmp at all and instead use the in-U-Boot versions of them. So can we update this patch to check for Linux glibc? Failing that, perhaps setjmp() could return a negative error value? Then (in other patches) the caller can make a note that longjmp() cannot be used, and will return errors from things that need to call it. Regards, Simon
On 06/21/2018 09:45 PM, Simon Glass wrote: > Hi Alex, > > On 21 June 2018 at 03:41, Alexander Graf <agraf@suse.de> wrote: >> On 06/21/2018 04:02 AM, Simon Glass wrote: >>> Hi Alex, >>> >>> On 18 June 2018 at 09:23, Alexander Graf <agraf@suse.de> wrote: >>>> In sandbox, longjmp returns to itself in an endless loop. Cut this through >>>> by calling the real OS function. >>>> >>>> Setjmp on the other hand must not return. So here we have to call the OS >>>> setjmp function straight from the code where the setjmp call happens. >>>> >>>> Signed-off-by: Alexander Graf <agraf@suse.de> >>>> --- >>>> arch/sandbox/cpu/cpu.c | 5 ----- >>>> arch/sandbox/cpu/os.c | 20 ++------------------ >>>> arch/sandbox/include/asm/setjmp.h | 4 +++- >>>> 3 files changed, 5 insertions(+), 24 deletions(-) >>>> >>> I think we do need to do something like this. But I cannot find the >>> man page for _longjmp(). Are you sure this is a valid function on all >>> systems? Or is it Linux only? >> >> I'm afraid it may be Linux (glibc) only. >> >> I guess the alternative to this would be to not use system setjmp/longjmp at all and instead use the in-U-Boot versions of them. > So can we update this patch to check for Linux glibc? Failing that, > perhaps setjmp() could return a negative error value? Then (in other > patches) the caller can make a note that longjmp() cannot be used, and > will return errors from things that need to call it. I actually think there is a much easier way. We can just directly export the symbols for libc's setjmp/longjmp functions in the sandbox setjmp.h header. That way we avoid all the nasty problems we've encountered so far. I've cooked up a patch and it seems to work fine. Alex
diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c index 29dac8dfda..9087026d71 100644 --- a/arch/sandbox/cpu/cpu.c +++ b/arch/sandbox/cpu/cpu.c @@ -167,11 +167,6 @@ ulong timer_get_boot_us(void) return (count - base_count) / 1000; } -int setjmp(jmp_buf jmp) -{ - return os_setjmp((ulong *)jmp, sizeof(*jmp)); -} - void longjmp(jmp_buf jmp, int ret) { os_longjmp((ulong *)jmp, ret); diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c index 5839932b00..9cfdc9f31f 100644 --- a/arch/sandbox/cpu/os.c +++ b/arch/sandbox/cpu/os.c @@ -630,24 +630,8 @@ void os_localtime(struct rtc_time *rt) rt->tm_isdst = tm->tm_isdst; } -int os_setjmp(ulong *jmp, int size) -{ - jmp_buf dummy; - - /* - * We cannot rely on the struct name that jmp_buf uses, so use a - * local variable here - */ - if (size < sizeof(dummy)) { - printf("setjmp: jmpbuf is too small (%d bytes, need %d)\n", - size, sizeof(jmp_buf)); - return -ENOSPC; - } - - return setjmp((struct __jmp_buf_tag *)jmp); -} - void os_longjmp(ulong *jmp, int ret) { - longjmp((struct __jmp_buf_tag *)jmp, ret); + /* Call the OS longjmp function directly */ + _longjmp((struct __jmp_buf_tag *)jmp, ret); } diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h index 1fe37c91cc..e103d170e0 100644 --- a/arch/sandbox/include/asm/setjmp.h +++ b/arch/sandbox/include/asm/setjmp.h @@ -24,7 +24,9 @@ struct jmp_buf_data { typedef struct jmp_buf_data jmp_buf[1]; -int setjmp(jmp_buf jmp); +/* Call the OS setjmp function directly, so that we don't introduce returns */ +#define setjmp _setjmp +int _setjmp(jmp_buf jmp); __noreturn void longjmp(jmp_buf jmp, int ret); #endif /* _SETJMP_H_ */
In sandbox, longjmp returns to itself in an endless loop. Cut this through by calling the real OS function. Setjmp on the other hand must not return. So here we have to call the OS setjmp function straight from the code where the setjmp call happens. Signed-off-by: Alexander Graf <agraf@suse.de> --- arch/sandbox/cpu/cpu.c | 5 ----- arch/sandbox/cpu/os.c | 20 ++------------------ arch/sandbox/include/asm/setjmp.h | 4 +++- 3 files changed, 5 insertions(+), 24 deletions(-)