diff mbox series

[U-Boot,08/16] sandbox: Add a setjmp() implementation

Message ID 20170917225927.117917-9-sjg@chromium.org
State Superseded
Delegated to: Alexander Graf
Headers show
Series efi: Enable basic sandbox support for EFI loader | expand

Commit Message

Simon Glass Sept. 17, 2017, 10:59 p.m. UTC 
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@chromium.org>
---

 arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
 arch/sandbox/cpu/os.c             | 17 +++++++++++++++++
 arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++++++++
 include/os.h                      | 21 +++++++++++++++++++++
 4 files changed, 72 insertions(+)
 create mode 100644 arch/sandbox/include/asm/setjmp.h

Comments

Heinrich Schuchardt Sept. 18, 2017, 6:01 a.m. UTC | #1 
On 09/18/2017 12:59 AM, 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@chromium.org>
> ---
> 
>  arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
>  arch/sandbox/cpu/os.c             | 17 +++++++++++++++++
>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++++++++
>  include/os.h                      | 21 +++++++++++++++++++++
>  4 files changed, 72 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 01991049cc..de5862a53b 100644
> --- a/arch/sandbox/cpu/cpu.c
> +++ b/arch/sandbox/cpu/cpu.c
> @@ -9,6 +9,7 @@
>  #include <libfdt.h>
>  #include <os.h>
>  #include <asm/io.h>
> +#include <asm/setjmp.h>
>  #include <asm/state.h>
>  #include <dm/root.h>
>  
> @@ -154,3 +155,15 @@ 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);
> +	while (1)
> +		;
> +}
> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
> index 22d6aab534..909034fa4b 100644
> --- a/arch/sandbox/cpu/os.c
> +++ b/arch/sandbox/cpu/os.c
> @@ -7,6 +7,7 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <getopt.h>
> +#include <setjmp.h>
>  #include <stdio.h>
>  #include <stdint.h>
>  #include <stdlib.h>
> @@ -609,3 +610,19 @@ void os_localtime(struct rtc_time *rt)
>  	rt->tm_yday = tm->tm_yday;
>  	rt->tm_isdst = tm->tm_isdst;
>  }
> +
> +int os_setjmp(ulong *jmp, int size)
> +{
> +	if (size < sizeof(jmp_buf)) {
> +		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);
> +}
> diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
> new file mode 100644
> index 0000000000..e25f50107c
> --- /dev/null
> +++ b/arch/sandbox/include/asm/setjmp.h
> @@ -0,0 +1,21 @@
> +/*
> + * (C) Copyright 2016
> + * Alexander Graf <agraf@suse.de>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef _SETJMP_H_
> +#define _SETJMP_H_	1
> +
> +struct jmp_buf_data {
> +	/* We're not sure how long this should be */
> +	ulong data[32];
> +};
> +
> +typedef struct jmp_buf_data jmp_buf[1];
> +
> +int setjmp(jmp_buf jmp);
> +__noreturn void longjmp(jmp_buf jmp, int ret);
> +
> +#endif /* _SETJMP_H_ */
> diff --git a/include/os.h b/include/os.h
> index 2bf4bdb1b8..ad1836ac9f 100644
> --- a/include/os.h
> +++ b/include/os.h
> @@ -310,4 +310,25 @@ int os_spl_to_uboot(const char *fname);
>   */
>  void os_localtime(struct rtc_time *rt);
>  
> +/**
> + * os_setjmp() - Call setjmp()
> + *
> + * Call the host system's setjmp() function.
> + *
> + * @jmp: Buffer to store current execution state
> + * @size: Size of buffer
> + * @return normal setjmp() value if OK, -ENOSPC if @size is too small
> + */
> +int os_setjmp(ulong *jmp, int size);
> +
> +/**
> + * os_longjmp() - Call longjmp()
> + *
> + * Call the host system's longjmp() function.
> + *
> + * @jmp: Buffer where previous execution state was stored
> + * @ret: Value to pass to longjmp()
> + */
> +void os_longjmp(ulong *jmp, int ret);
> +
>  #endif
> 

Please, fix this warning:

arch/sandbox/cpu/cpu.c: In function ‘setjmp’:
arch/sandbox/cpu/cpu.c:161:39: warning: ‘sizeof’ on array function
parameter ‘jmp’ will return size of ‘struct jmp_buf_data *’
[-Wsizeof-array-argument]
  return os_setjmp((ulong *)jmp, sizeof(jmp));
                                       ^
arch/sandbox/cpu/cpu.c:159:20: note: declared here
 int setjmp(jmp_buf jmp)

Regards

Heinrich
Rob Clark Sept. 18, 2017, 7:10 p.m. UTC | #2 
On Mon, Sep 18, 2017 at 2:01 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/18/2017 12:59 AM, 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@chromium.org>
>> ---
>>
>>  arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
>>  arch/sandbox/cpu/os.c             | 17 +++++++++++++++++
>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++++++++
>>  include/os.h                      | 21 +++++++++++++++++++++
>>  4 files changed, 72 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 01991049cc..de5862a53b 100644
>> --- a/arch/sandbox/cpu/cpu.c
>> +++ b/arch/sandbox/cpu/cpu.c
>> @@ -9,6 +9,7 @@
>>  #include <libfdt.h>
>>  #include <os.h>
>>  #include <asm/io.h>
>> +#include <asm/setjmp.h>
>>  #include <asm/state.h>
>>  #include <dm/root.h>
>>
>> @@ -154,3 +155,15 @@ 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);
>> +     while (1)
>> +             ;
>> +}
>> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
>> index 22d6aab534..909034fa4b 100644
>> --- a/arch/sandbox/cpu/os.c
>> +++ b/arch/sandbox/cpu/os.c
>> @@ -7,6 +7,7 @@
>>  #include <errno.h>
>>  #include <fcntl.h>
>>  #include <getopt.h>
>> +#include <setjmp.h>
>>  #include <stdio.h>
>>  #include <stdint.h>
>>  #include <stdlib.h>
>> @@ -609,3 +610,19 @@ void os_localtime(struct rtc_time *rt)
>>       rt->tm_yday = tm->tm_yday;
>>       rt->tm_isdst = tm->tm_isdst;
>>  }
>> +
>> +int os_setjmp(ulong *jmp, int size)
>> +{
>> +     if (size < sizeof(jmp_buf)) {
>> +             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);
>> +}
>> diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
>> new file mode 100644
>> index 0000000000..e25f50107c
>> --- /dev/null
>> +++ b/arch/sandbox/include/asm/setjmp.h
>> @@ -0,0 +1,21 @@
>> +/*
>> + * (C) Copyright 2016
>> + * Alexander Graf <agraf@suse.de>
>> + *
>> + * SPDX-License-Identifier:  GPL-2.0+
>> + */
>> +
>> +#ifndef _SETJMP_H_
>> +#define _SETJMP_H_   1
>> +
>> +struct jmp_buf_data {
>> +     /* We're not sure how long this should be */
>> +     ulong data[32];
>> +};
>> +
>> +typedef struct jmp_buf_data jmp_buf[1];
>> +
>> +int setjmp(jmp_buf jmp);
>> +__noreturn void longjmp(jmp_buf jmp, int ret);
>> +
>> +#endif /* _SETJMP_H_ */
>> diff --git a/include/os.h b/include/os.h
>> index 2bf4bdb1b8..ad1836ac9f 100644
>> --- a/include/os.h
>> +++ b/include/os.h
>> @@ -310,4 +310,25 @@ int os_spl_to_uboot(const char *fname);
>>   */
>>  void os_localtime(struct rtc_time *rt);
>>
>> +/**
>> + * os_setjmp() - Call setjmp()
>> + *
>> + * Call the host system's setjmp() function.
>> + *
>> + * @jmp: Buffer to store current execution state
>> + * @size: Size of buffer
>> + * @return normal setjmp() value if OK, -ENOSPC if @size is too small
>> + */
>> +int os_setjmp(ulong *jmp, int size);
>> +
>> +/**
>> + * os_longjmp() - Call longjmp()
>> + *
>> + * Call the host system's longjmp() function.
>> + *
>> + * @jmp: Buffer where previous execution state was stored
>> + * @ret: Value to pass to longjmp()
>> + */
>> +void os_longjmp(ulong *jmp, int ret);
>> +
>>  #endif
>>
>
> Please, fix this warning:
>
> arch/sandbox/cpu/cpu.c: In function ‘setjmp’:
> arch/sandbox/cpu/cpu.c:161:39: warning: ‘sizeof’ on array function
> parameter ‘jmp’ will return size of ‘struct jmp_buf_data *’
> [-Wsizeof-array-argument]
>   return os_setjmp((ulong *)jmp, sizeof(jmp));
>                                        ^
> arch/sandbox/cpu/cpu.c:159:20: note: declared here
>  int setjmp(jmp_buf jmp)
>

with the sizeof changed to sizeof(jmp_buf), I'm getting:

...
EFI: Entry efi_exit(00007fffffffc198, 2147483662, 0, 0000000000000000)

Program received signal SIGSEGV, Segmentation fault.
os_longjmp (jmp=0x7fffffffc200,
    jmp@entry=<error reading variable: DWARF-2 expression error: Loop
detected (257).>, ret=1,
    ret@entry=<error reading variable: DWARF-2 expression error: Loop
detected (257).>)
    at ../arch/sandbox/cpu/os.c:627
627        longjmp((struct __jmp_buf_tag *)jmp, ret);


So it seems like something not quite right still..

BR,
-R
Simon Glass Sept. 25, 2017, 2:12 a.m. UTC | #3 
Hi,

On 18 September 2017 at 13:10, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 2:01 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 09/18/2017 12:59 AM, 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@chromium.org>
>>> ---
>>>
>>>  arch/sandbox/cpu/cpu.c            | 13 +++++++++++++
>>>  arch/sandbox/cpu/os.c             | 17 +++++++++++++++++
>>>  arch/sandbox/include/asm/setjmp.h | 21 +++++++++++++++++++++
>>>  include/os.h                      | 21 +++++++++++++++++++++
>>>  4 files changed, 72 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 01991049cc..de5862a53b 100644
>>> --- a/arch/sandbox/cpu/cpu.c
>>> +++ b/arch/sandbox/cpu/cpu.c
>>> @@ -9,6 +9,7 @@
>>>  #include <libfdt.h>
>>>  #include <os.h>
>>>  #include <asm/io.h>
>>> +#include <asm/setjmp.h>
>>>  #include <asm/state.h>
>>>  #include <dm/root.h>
>>>
>>> @@ -154,3 +155,15 @@ 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);
>>> +     while (1)
>>> +             ;
>>> +}
>>> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
>>> index 22d6aab534..909034fa4b 100644
>>> --- a/arch/sandbox/cpu/os.c
>>> +++ b/arch/sandbox/cpu/os.c
>>> @@ -7,6 +7,7 @@
>>>  #include <errno.h>
>>>  #include <fcntl.h>
>>>  #include <getopt.h>
>>> +#include <setjmp.h>
>>>  #include <stdio.h>
>>>  #include <stdint.h>
>>>  #include <stdlib.h>
>>> @@ -609,3 +610,19 @@ void os_localtime(struct rtc_time *rt)
>>>       rt->tm_yday = tm->tm_yday;
>>>       rt->tm_isdst = tm->tm_isdst;
>>>  }
>>> +
>>> +int os_setjmp(ulong *jmp, int size)
>>> +{
>>> +     if (size < sizeof(jmp_buf)) {
>>> +             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);
>>> +}
>>> diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
>>> new file mode 100644
>>> index 0000000000..e25f50107c
>>> --- /dev/null
>>> +++ b/arch/sandbox/include/asm/setjmp.h
>>> @@ -0,0 +1,21 @@
>>> +/*
>>> + * (C) Copyright 2016
>>> + * Alexander Graf <agraf@suse.de>
>>> + *
>>> + * SPDX-License-Identifier:  GPL-2.0+
>>> + */
>>> +
>>> +#ifndef _SETJMP_H_
>>> +#define _SETJMP_H_   1
>>> +
>>> +struct jmp_buf_data {
>>> +     /* We're not sure how long this should be */
>>> +     ulong data[32];
>>> +};
>>> +
>>> +typedef struct jmp_buf_data jmp_buf[1];
>>> +
>>> +int setjmp(jmp_buf jmp);
>>> +__noreturn void longjmp(jmp_buf jmp, int ret);
>>> +
>>> +#endif /* _SETJMP_H_ */
>>> diff --git a/include/os.h b/include/os.h
>>> index 2bf4bdb1b8..ad1836ac9f 100644
>>> --- a/include/os.h
>>> +++ b/include/os.h
>>> @@ -310,4 +310,25 @@ int os_spl_to_uboot(const char *fname);
>>>   */
>>>  void os_localtime(struct rtc_time *rt);
>>>
>>> +/**
>>> + * os_setjmp() - Call setjmp()
>>> + *
>>> + * Call the host system's setjmp() function.
>>> + *
>>> + * @jmp: Buffer to store current execution state
>>> + * @size: Size of buffer
>>> + * @return normal setjmp() value if OK, -ENOSPC if @size is too small
>>> + */
>>> +int os_setjmp(ulong *jmp, int size);
>>> +
>>> +/**
>>> + * os_longjmp() - Call longjmp()
>>> + *
>>> + * Call the host system's longjmp() function.
>>> + *
>>> + * @jmp: Buffer where previous execution state was stored
>>> + * @ret: Value to pass to longjmp()
>>> + */
>>> +void os_longjmp(ulong *jmp, int ret);
>>> +
>>>  #endif
>>>
>>
>> Please, fix this warning:
>>
>> arch/sandbox/cpu/cpu.c: In function ‘setjmp’:
>> arch/sandbox/cpu/cpu.c:161:39: warning: ‘sizeof’ on array function
>> parameter ‘jmp’ will return size of ‘struct jmp_buf_data *’
>> [-Wsizeof-array-argument]
>>   return os_setjmp((ulong *)jmp, sizeof(jmp));
>>                                        ^
>> arch/sandbox/cpu/cpu.c:159:20: note: declared here
>>  int setjmp(jmp_buf jmp)
>>
>
> with the sizeof changed to sizeof(jmp_buf), I'm getting:
>
> ...
> EFI: Entry efi_exit(00007fffffffc198, 2147483662, 0, 0000000000000000)
>
> Program received signal SIGSEGV, Segmentation fault.
> os_longjmp (jmp=0x7fffffffc200,
>     jmp@entry=<error reading variable: DWARF-2 expression error: Loop
> detected (257).>, ret=1,
>     ret@entry=<error reading variable: DWARF-2 expression error: Loop
> detected (257).>)
>     at ../arch/sandbox/cpu/os.c:627
> 627        longjmp((struct __jmp_buf_tag *)jmp, ret);
>
>
> So it seems like something not quite right still..

Hmm I'm not sure what, yet.

Regards,
Simon
diff mbox series

Patch

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 01991049cc..de5862a53b 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -9,6 +9,7 @@ 
 #include <libfdt.h>
 #include <os.h>
 #include <asm/io.h>
+#include <asm/setjmp.h>
 #include <asm/state.h>
 #include <dm/root.h>
 
@@ -154,3 +155,15 @@  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);
+	while (1)
+		;
+}
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 22d6aab534..909034fa4b 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -7,6 +7,7 @@ 
 #include <errno.h>
 #include <fcntl.h>
 #include <getopt.h>
+#include <setjmp.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -609,3 +610,19 @@  void os_localtime(struct rtc_time *rt)
 	rt->tm_yday = tm->tm_yday;
 	rt->tm_isdst = tm->tm_isdst;
 }
+
+int os_setjmp(ulong *jmp, int size)
+{
+	if (size < sizeof(jmp_buf)) {
+		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);
+}
diff --git a/arch/sandbox/include/asm/setjmp.h b/arch/sandbox/include/asm/setjmp.h
new file mode 100644
index 0000000000..e25f50107c
--- /dev/null
+++ b/arch/sandbox/include/asm/setjmp.h
@@ -0,0 +1,21 @@ 
+/*
+ * (C) Copyright 2016
+ * Alexander Graf <agraf@suse.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _SETJMP_H_
+#define _SETJMP_H_	1
+
+struct jmp_buf_data {
+	/* We're not sure how long this should be */
+	ulong data[32];
+};
+
+typedef struct jmp_buf_data jmp_buf[1];
+
+int setjmp(jmp_buf jmp);
+__noreturn void longjmp(jmp_buf jmp, int ret);
+
+#endif /* _SETJMP_H_ */
diff --git a/include/os.h b/include/os.h
index 2bf4bdb1b8..ad1836ac9f 100644
--- a/include/os.h
+++ b/include/os.h
@@ -310,4 +310,25 @@  int os_spl_to_uboot(const char *fname);
  */
 void os_localtime(struct rtc_time *rt);
 
+/**
+ * os_setjmp() - Call setjmp()
+ *
+ * Call the host system's setjmp() function.
+ *
+ * @jmp: Buffer to store current execution state
+ * @size: Size of buffer
+ * @return normal setjmp() value if OK, -ENOSPC if @size is too small
+ */
+int os_setjmp(ulong *jmp, int size);
+
+/**
+ * os_longjmp() - Call longjmp()
+ *
+ * Call the host system's longjmp() function.
+ *
+ * @jmp: Buffer where previous execution state was stored
+ * @ret: Value to pass to longjmp()
+ */
+void os_longjmp(ulong *jmp, int ret);
+
 #endif