diff mbox series

efi_loader: helloworld.c: Explicitly use .rodata for loaded_image_guid

Message ID 20171211084511.50385-1-agraf@suse.de
State Accepted
Commit ae67dca5e61867cda886bdd2943709a19c45d76a
Headers show
Series efi_loader: helloworld.c: Explicitly use .rodata for loaded_image_guid | expand

Commit Message

Alexander Graf Dec. 11, 2017, 8:45 a.m. UTC
Commit bbf75dd9345d0b ("efi_loader: output load options in helloworld")
introduced a const variable in efi_main() called loaded_image_guid which
got populated from a constant struct.

While you would usually expect a compiler to realize that this variable
should really just be a global pointer to .rodata, gcc disagrees and instead
puts it on the stack. Unfortunately in some implementations of gcc it does
so my calling memcpy() which we do not implement in our hello world
environment.

So let's explicitly move it to a global variable which in turn puts it in
.rodata reliably and gets rid of the memcpy().

Fixes: bbf75dd9345d0b ("efi_loader: output load options in helloworld")
Reported-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 lib/efi_loader/helloworld.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Heinrich Schuchardt Dec. 11, 2017, 12:09 p.m. UTC | #1
On 12/11/2017 09:45 AM, Alexander Graf wrote:
> Commit bbf75dd9345d0b ("efi_loader: output load options in helloworld")
> introduced a const variable in efi_main() called loaded_image_guid which
> got populated from a constant struct.
> 
> While you would usually expect a compiler to realize that this variable
> should really just be a global pointer to .rodata, gcc disagrees and instead
> puts it on the stack. Unfortunately in some implementations of gcc it does
> so my calling memcpy() which we do not implement in our hello world
> environment.
> 
> So let's explicitly move it to a global variable which in turn puts it in
> .rodata reliably and gets rid of the memcpy().
> 
> Fixes: bbf75dd9345d0b ("efi_loader: output load options in helloworld")
> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>   lib/efi_loader/helloworld.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c
> index e59c24c788..b8c147d7f2 100644
> --- a/lib/efi_loader/helloworld.c
> +++ b/lib/efi_loader/helloworld.c
> @@ -13,6 +13,8 @@
>   #include <common.h>
>   #include <efi_api.h>
>   
> +static const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
> +
>   /*
>    * Entry point of the EFI application.
>    *
> @@ -26,7 +28,6 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
>   	struct efi_simple_text_output_protocol *con_out = systable->con_out;
>   	struct efi_boot_services *boottime = systable->boottime;
>   	struct efi_loaded_image *loaded_image;
> -	const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
>   	efi_status_t ret;
>   
>   	con_out->output_string(con_out, L"Hello, world!\n");
> 
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Florian Fainelli Dec. 11, 2017, 6:54 p.m. UTC | #2
On 12/11/2017 12:45 AM, Alexander Graf wrote:
> Commit bbf75dd9345d0b ("efi_loader: output load options in helloworld")
> introduced a const variable in efi_main() called loaded_image_guid which
> got populated from a constant struct.
> 
> While you would usually expect a compiler to realize that this variable
> should really just be a global pointer to .rodata, gcc disagrees and instead
> puts it on the stack. Unfortunately in some implementations of gcc it does
> so my calling memcpy() which we do not implement in our hello world
> environment.
> 
> So let's explicitly move it to a global variable which in turn puts it in
> .rodata reliably and gets rid of the memcpy().
> 
> Fixes: bbf75dd9345d0b ("efi_loader: output load options in helloworld")
> Reported-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks Alexander!

> ---
>  lib/efi_loader/helloworld.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c
> index e59c24c788..b8c147d7f2 100644
> --- a/lib/efi_loader/helloworld.c
> +++ b/lib/efi_loader/helloworld.c
> @@ -13,6 +13,8 @@
>  #include <common.h>
>  #include <efi_api.h>
>  
> +static const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
> +
>  /*
>   * Entry point of the EFI application.
>   *
> @@ -26,7 +28,6 @@ efi_status_t EFIAPI efi_main(efi_handle_t handle,
>  	struct efi_simple_text_output_protocol *con_out = systable->con_out;
>  	struct efi_boot_services *boottime = systable->boottime;
>  	struct efi_loaded_image *loaded_image;
> -	const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
>  	efi_status_t ret;
>  
>  	con_out->output_string(con_out, L"Hello, world!\n");
>
diff mbox series

Patch

diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c
index e59c24c788..b8c147d7f2 100644
--- a/lib/efi_loader/helloworld.c
+++ b/lib/efi_loader/helloworld.c
@@ -13,6 +13,8 @@ 
 #include <common.h>
 #include <efi_api.h>
 
+static const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
+
 /*
  * Entry point of the EFI application.
  *
@@ -26,7 +28,6 @@  efi_status_t EFIAPI efi_main(efi_handle_t handle,
 	struct efi_simple_text_output_protocol *con_out = systable->con_out;
 	struct efi_boot_services *boottime = systable->boottime;
 	struct efi_loaded_image *loaded_image;
-	const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
 	efi_status_t ret;
 
 	con_out->output_string(con_out, L"Hello, world!\n");