[PATCH v2 2/2] test: unit test for longjmp

[Sean Anderson <seanga2@gmail.com>]

On 3/22/21 12:42 PM, Heinrich Schuchardt wrote:
> On 22.03.21 14:30, Sean Anderson wrote:
>>
>> On 3/22/21 9:23 AM, Sean Anderson wrote:
>>>
>>> On 3/22/21 7:02 AM, Heinrich Schuchardt wrote:
>>>> Provide a unit test for the longjmp() library function
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> ---
>>>> v2:
>>>>  ??? no change
>>>> ---
>>>>  ? test/lib/Makefile? |? 1 +
>>>>  ? test/lib/longjmp.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  ? 2 files changed, 45 insertions(+)
>>>>  ? create mode 100644 test/lib/longjmp.c
>>>>
>>>> diff --git a/test/lib/Makefile b/test/lib/Makefile
>>>> index 97c11e35a8..a30f615aa9 100644
>>>> --- a/test/lib/Makefile
>>>> +++ b/test/lib/Makefile
>>>> @@ -7,6 +7,7 @@ obj-$(CONFIG_EFI_LOADER) += efi_device_path.o
>>>>  ? obj-$(CONFIG_EFI_SECURE_BOOT) += efi_image_region.o
>>>>  ? obj-y += hexdump.o
>>>>  ? obj-y += lmb.o
>>>> +obj-y += longjmp.o
>>>>  ? obj-$(CONFIG_CONSOLE_RECORD) += test_print.o
>>>>  ? obj-$(CONFIG_SSCANF) += sscanf.o
>>>>  ? obj-y += string.o
>>>> diff --git a/test/lib/longjmp.c b/test/lib/longjmp.c
>>>> new file mode 100644
>>>> index 0000000000..7571540ffc
>>>> --- /dev/null
>>>> +++ b/test/lib/longjmp.c
>>>> @@ -0,0 +1,44 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * Test setjmp(), longjmp()
>>>> + *
>>>> + * Copyright (c) 2021, Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> + */
>>>> +
>>>> +#include <common.h>
>>>> +#include <test/lib.h>
>>>> +#include <test/test.h>
>>>> +#include <test/ut.h>
>>>> +#include <asm/setjmp.h>
>>>> +
>>>> +/**
>>>> + * test_longjmp_ret() - get longjmp() return value
>>>> + *
>>>> + * @i:??????? value passed to longjmp()
>>>> + * Return:??? value returned by longjmp()
>>>> + */
>>>> +int test_longjmp_ret(int i)
>>>> +{
>>>> +??? jmp_buf env;
>>>> +??? int ret;
>>>> +
>>>> +??? ret = setjmp(env);
>>>> +??? if (ret)
>>>> +??????? return ret;
>>>> +??? longjmp(env, i);
>>>> +??? /* We should not arrive here */
>>>> +??? return 0x1000;
>>>> +}
>>>> +
>>>> +static int lib_test_longjmp(struct unit_test_state *uts)
>>>> +{
>>>> +??? int i;
>>>> +
>>>> +??? for (i = -3; i < 0; ++i)
>>>> +??????? ut_asserteq(i, test_longjmp_ret(i));
>>>> +??? ut_asserteq(1, test_longjmp_ret(0));
>>>> +??? for (i = 1; i < 4; ++i)
>>>> +??????? ut_asserteq(i, test_longjmp_ret(i));
>>>> +??? return 0;
>>>> +}
>>>> +LIB_TEST(lib_test_longjmp, 0);
>>>> -- 
>>>> 2.30.2
>>>>
>>>
>>> Reviewed-by: Sean Anderson <seanga2@gmail.com>
>>> Tested-by: Sean Anderson <seanga2@gmail.com>
>>>
>>> Though I would like to test that variables are set correctly e.g. by
>>> doing
>>>
>>> int test_longjmp_ret(int i)
>>> {
>>>  ???? jmp_buf env;
>>>  ???? int ret;
>>>
>>>  ???? ret = setjmp(env);
>>>  ???? if (ret)
>>>  ???????? return ret;
>>>  ???? ret = 0x1000;
>>>  ???? longjmp(env, i);
>>>  ???? /* We should not arrive here */
>>>  ???? return ret;
>>> }
>>>
>>> --Sean
>>
>> err, rather by doing
>>
>> int test_longjmp_ret(int i)
>> {
>>  ???? jmp_buf env;
>>  ???? int ret;
>>  ???? int foo = i;
>>
>>  ???? ret = setjmp(env);
>>  ???? if (ret)
>>  ???????? return foo;
>>  ???? foo = 0x1000;
>>  ???? longjmp(env, i);
>>  ???? /* We should not arrive here */
>>  ???? return foo;
>> }
>>
>> or something else which demonstrates that variables get reset to their
>> earlier values.
>>
>> --Sean
> 
> Hello Sean,
> 
> thank you for reviewing.
> 
> Would the following make sense to you to check that the stack pointer is
> correctly restored?
> 
> 
> struct test_jmp_buf {
>          jmp_buf env;
>          int val;
> };
> 
> /**
>   * test_longjmp() - test longjmp function
>   *
>   * @i is passed to longjmp.
>   * @i << 8 is set in the environment structure.
>   *
>   * @env:        environment
>   * @i:          value passed to longjmp()
>   */
> static void noinline test_longjmp(struct test_jmp_buf *env, int i)
> {
>          env->val = i << 8;
>          longjmp(env->env, i);
> }
> 
> /**
>   * test_setjmp() - test setjmp function
>   *
>   * setjmp() will return the value @i passed to longjmp() if @i is non-zero.
>   * For @i == 0 we expect return value 1.
>   *
>   * @i << 8 will be set by test_longjmp in the environment structure.
>   * This value can be used to check that the stack frame is restored.
>   *
>   * We return the XORed values to allow simply check both at once.
>   *
>   * @i:          value passed to longjmp()
>   * Return:      values return byby longjmp()

nit: by

>   */
> static int test_setjmp(int i)
> {
>          struct test_jmp_buf env;
>          int ret;
> 
>          env.val = -1;
>          ret = setjmp(env.env);
>          if (ret)
>                  return ret ^ env.val;
>          test_longjmp(&env, i);
>          /* We should not arrive here */
>          return 0x1000;
> }
> 
> Best regards
> 
> Heinrich
> 

Yes, this looks good.

--Sean