[U-Boot] [PATCH v4 05/16] sandbox: Add a setjmp() implementation

[Alexander Graf]

> Am 15.06.2018 um 17:16 schrieb Simon Glass <sjg at chromium.org>:
> 
> Hi Alex,
> 
>> On 15 June 2018 at 06:01, Alexander Graf <agraf at suse.de> wrote:
>> 
>> 
>>> On 16.05.18 17:42, Simon Glass wrote:
>>> Add an implementation of setjmp() and longjmp() which rely on the
>>> underlying host C library. Since we cannot know how large the jump buffer
>>> needs to be, pick something that should be suitable and check it at
>>> runtime. At present we need access to the underlying struct as well.
>>> 
>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>> ---
>>> 
>>> Changes in v4:
>>> - Fix up the sizeof() operations on jmp_buf
>>> - Update SPDX tags
>>> 
>>> Changes in v3: None
>>> Changes in v2: None
>>> 
>>> arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
>>> arch/sandbox/cpu/os.c             | 23 +++++++++++++++++++++++
>>> arch/sandbox/include/asm/setjmp.h | 30 ++++++++++++++++++++++++++++++
>>> include/os.h                      | 21 +++++++++++++++++++++
>>> 4 files changed, 87 insertions(+)
>>> create mode 100644 arch/sandbox/include/asm/setjmp.h
>>> 
>>> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
>>> index d4ad020012e..cde0b055a67 100644
>>> --- a/arch/sandbox/cpu/cpu.c
>>> +++ b/arch/sandbox/cpu/cpu.c
>>> @@ -9,6 +9,7 @@
>>> #include <linux/libfdt.h>
>>> #include <os.h>
>>> #include <asm/io.h>
>>> +#include <asm/setjmp.h>
>>> #include <asm/state.h>
>>> #include <dm/root.h>
>>> 
>>> @@ -164,3 +165,15 @@ ulong timer_get_boot_us(void)
>>> 
>>>      return (count - base_count) / 1000;
>>> }
>>> +
>>> +int setjmp(jmp_buf jmp)
>>> +{
>>> +     return os_setjmp((ulong *)jmp, sizeof(*jmp));
>> 
>> So, this doesn't work. Function returns increase the stack pointer which
>> means after setjmp() you are not allowed to return until the longjmp
>> occured. The documentation is quite clear about this:
>> 
>> 
>> DESCRIPTION
>>       setjmp() and longjmp(3) are useful for dealing with errors and
>> interrupts encountered in a low-level subroutine of a program.  setjmp()
>> saves the stack context/environment in env for later use by longjmp(3).
>> The stack context will be invalidated if the function which called
>> setjmp() returns.
>> 
>> 
>> So we need to find a way to call setjmp() directly from the code point
>> where we want to call it, rather than jump through helper functions, as
>> these break its functionality.
> 
> That sounds hard but perhaps we can do something with inline
> functions? It's hard because we want to call the C library in the
> sandbox case. Perhaps we should rename the U-Boot version of the
> function.
> 
> I wonder if in my test I just relied on hello world returning, rather
> than calling the exit() function? BTW we need to find a way for
> efi_selftest() to exit cleanly too. At the moment it resets.
> 
>> 
>> Also, os_longjmp() is broken. It calls longjmp() which however is not
>> the system longjmp, but the U-Boot internal one that again calls
>> os_longjmp. My quick fix was to make it call _longjmp() instead - that
>> at least makes that part work.
> 
> But it hangs after that?

It hangs in the return path of setjmp afterwards because of the clobbered stack. See my patch in the latest series for reference. With that patch it works (might be  glibc only though).

Alex