[U-Boot] [PATCH v5 04/10] sandbox: Fix setjmp/longjmp

[Simon Glass]

Hi Alex,

On 23 June 2018 at 01:36, Alexander Graf <agraf at suse.de> wrote:
>
>
> On 22.06.18 21:28, Simon Glass wrote:
>> Hi Alex,
>>
>> On 22 June 2018 at 06:44, Alexander Graf <agraf at suse.de> wrote:
>>> In sandbox, longjmp returns to itself in an endless loop because
>>> os_longjmp() calls into longjmp() which is provided by U-Boot which
>>> again calls os_longjmp().
>>>
>>> Setjmp on the other hand must not return because otherwise the
>>> return freees up stack elements that we need during longjmp().
>>>
>>> The only straight forward fix that doesn't involve nasty hacks I
>>> could find is to directly link against the system setjmp/longjmp
>>> implementations. That means we just provide the compiler with
>>> hints that the symbol will be available and actually fill them
>>> out with versions from libc.
>>>
>>> This approach should be reasonably platform agnostic
>>>
>>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>>
>>> ---
>>>
>>> v4 -> v5:
>>>
>>>   - Use system setjmp/longjmp directly from target code
>>> ---
>>>  arch/sandbox/cpu/cpu.c            | 12 ------------
>>>  arch/sandbox/cpu/os.c             | 22 ----------------------
>>>  arch/sandbox/include/asm/setjmp.h |  5 +++++
>>>  include/os.h                      | 21 ---------------------
>>>  4 files changed, 5 insertions(+), 55 deletions(-)
>>
>> I was wondering if that would work. It seems much better to me.
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> Awesome ;).
>
> So the way forward that I would envision is that once the merge window
> is open, I'll send out a pull request for efi-next to Tom. Pretty much
> all the foundational work should be in upstream by then. The only
> remaining parts are x86, distro and sandbox specific.
>
> For these (basically all the patches of this patch set) I think it makes
> sense if you send them via your tree then.
>
> Beware that currently travis complains about sandbox failing with this
> patch set applied. I assume it's your mao_to_sysmem() patch though:
>
>   https://travis-ci.org/agraf/u-boot/jobs/395446382

Yes that looks like a bug although I don't see it with my series.

Regards,
Simon