* [PATCH] RISC-V: Restore the typcast to 64bit type
@ 2021-03-28 22:49 Khem Raj
2021-03-29 10:24 ` Andreas Schwab
0 siblings, 1 reply; 6+ messages in thread
From: Khem Raj @ 2021-03-28 22:49 UTC (permalink / raw)
To: grub-devel
Cc: Khem Raj, Andreas Schwab, Daniel Kiper, Chester Lin,
Nikita Ermakov, Alistair Francis
this makes the type promotions clear and explicit
It was already typecasted to long but was accidentally dropped in [1]
which stated to cause failures on riscv32 as reported in [2]
[1] https://git.savannah.gnu.org/cgit/grub.git/commit/?id=2bf40e9e5be9808b17852e688eead87acff14420
[2] https://savannah.gnu.org/bugs/index.php?60283
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Cc: Andreas Schwab <schwab@suse.de>
Cc: Daniel Kiper <daniel.kiper@oracle.com>
Cc: Chester Lin <clin@suse.com>
Cc: Nikita Ermakov <arei@altlinux.org>
Cc: Alistair Francis <alistair.francis@wdc.com>
---
util/grub-mkimagexx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index 00f49ccaa..ac677d03d 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -1242,7 +1242,7 @@ SUFFIX (relocate_addrs) (Elf_Ehdr *e, struct section_metadata *smd,
*/
sym_addr += addend;
- off = sym_addr - target_section_addr - offset - image_target->vaddr_offset;
+ off = (grub_int64_t)sym_addr - target_section_addr - offset - image_target->vaddr_offset;
switch (ELF_R_TYPE (info))
{
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Restore the typcast to 64bit type
2021-03-28 22:49 [PATCH] RISC-V: Restore the typcast to 64bit type Khem Raj
@ 2021-03-29 10:24 ` Andreas Schwab
2021-03-29 18:33 ` Khem Raj
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2021-03-29 10:24 UTC (permalink / raw)
To: Khem Raj
Cc: grub-devel, Daniel Kiper, Chester Lin, Nikita Ermakov, Alistair Francis
On Mär 28 2021, Khem Raj wrote:
> this makes the type promotions clear and explicit
> It was already typecasted to long but was accidentally dropped in [1]
Note that long is 32-bit on riscv32, so the title is misleading (there
was no cast to 64-bit type before). The cast really was a no-op, as it
didn't change the resulting type. The issue is that the types of
sym_addr, target_section_addr and offset are Elf_Addr (aka Elf32_Addr),
which is an unsigned type (image_target->vaddr_offset is signed, but not
wider than Elf32_Addr, so it doesn't change the overall type). Thus
when the result of the expression is extended to grub_int64_t it is
always a positive value instead of the desired sign-extended one.
> which stated to cause failures on riscv32 as reported in [2]
Most likely the missing addend masked the error.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Restore the typcast to 64bit type
2021-03-29 10:24 ` Andreas Schwab
@ 2021-03-29 18:33 ` Khem Raj
2021-03-29 22:29 ` Daniel Kiper
0 siblings, 1 reply; 6+ messages in thread
From: Khem Raj @ 2021-03-29 18:33 UTC (permalink / raw)
To: Andreas Schwab
Cc: The development of GNU GRUB, Daniel Kiper, Chester Lin,
Nikita Ermakov, Alistair Francis
On Mon, Mar 29, 2021 at 3:24 AM Andreas Schwab <schwab@suse.de> wrote:
>
> On Mär 28 2021, Khem Raj wrote:
>
> > this makes the type promotions clear and explicit
> > It was already typecasted to long but was accidentally dropped in [1]
>
> Note that long is 32-bit on riscv32, so the title is misleading (there
> was no cast to 64-bit type before). The cast really was a no-op, as it
> didn't change the resulting type. The issue is that the types of
> sym_addr, target_section_addr and offset are Elf_Addr (aka Elf32_Addr),
> which is an unsigned type (image_target->vaddr_offset is signed, but not
> wider than Elf32_Addr, so it doesn't change the overall type). Thus
> when the result of the expression is extended to grub_int64_t it is
> always a positive value instead of the desired sign-extended one.
>
This is right I think, I initially typecasted it to long and then
changed to using grub_int64_t
and could have done better on commit message. Do you prefer a new revision with
better commit msg.
> > which stated to cause failures on riscv32 as reported in [2]
>
> Most likely the missing addend masked the error.
yes that's likely since addressed where it fails are 0xfffff....
>
> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Restore the typcast to 64bit type
2021-03-29 18:33 ` Khem Raj
@ 2021-03-29 22:29 ` Daniel Kiper
2021-04-12 13:07 ` Daniel Kiper
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Kiper @ 2021-03-29 22:29 UTC (permalink / raw)
To: Khem Raj
Cc: Andreas Schwab, The development of GNU GRUB, Chester Lin,
Nikita Ermakov, Alistair Francis
On Mon, Mar 29, 2021 at 11:33:11AM -0700, Khem Raj wrote:
> On Mon, Mar 29, 2021 at 3:24 AM Andreas Schwab <schwab@suse.de> wrote:
> > On Mär 28 2021, Khem Raj wrote:
> > > this makes the type promotions clear and explicit
> > > It was already typecasted to long but was accidentally dropped in [1]
> >
> > Note that long is 32-bit on riscv32, so the title is misleading (there
> > was no cast to 64-bit type before). The cast really was a no-op, as it
> > didn't change the resulting type. The issue is that the types of
> > sym_addr, target_section_addr and offset are Elf_Addr (aka Elf32_Addr),
> > which is an unsigned type (image_target->vaddr_offset is signed, but not
> > wider than Elf32_Addr, so it doesn't change the overall type). Thus
> > when the result of the expression is extended to grub_int64_t it is
> > always a positive value instead of the desired sign-extended one.
> >
>
> This is right I think, I initially typecasted it to long and then
> changed to using grub_int64_t
> and could have done better on commit message. Do you prefer a new revision with
> better commit msg.
If long is 32-bit on riscv32 should not we cast to grub_int32_t then?
And then next question: Why off is defined as grub_int64_t?
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: Restore the typcast to 64bit type
2021-03-29 22:29 ` Daniel Kiper
@ 2021-04-12 13:07 ` Daniel Kiper
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Kiper @ 2021-04-12 13:07 UTC (permalink / raw)
To: Khem Raj
Cc: grub-devel, Andreas Schwab, Chester Lin, Nikita Ermakov,
Alistair Francis
On Tue, Mar 30, 2021 at 12:29:56AM +0200, Daniel Kiper wrote:
> On Mon, Mar 29, 2021 at 11:33:11AM -0700, Khem Raj wrote:
> > On Mon, Mar 29, 2021 at 3:24 AM Andreas Schwab <schwab@suse.de> wrote:
> > > On Mär 28 2021, Khem Raj wrote:
> > > > this makes the type promotions clear and explicit
> > > > It was already typecasted to long but was accidentally dropped in [1]
> > >
> > > Note that long is 32-bit on riscv32, so the title is misleading (there
> > > was no cast to 64-bit type before). The cast really was a no-op, as it
> > > didn't change the resulting type. The issue is that the types of
> > > sym_addr, target_section_addr and offset are Elf_Addr (aka Elf32_Addr),
> > > which is an unsigned type (image_target->vaddr_offset is signed, but not
> > > wider than Elf32_Addr, so it doesn't change the overall type). Thus
> > > when the result of the expression is extended to grub_int64_t it is
> > > always a positive value instead of the desired sign-extended one.
> > >
> >
> > This is right I think, I initially typecasted it to long and then
> > changed to using grub_int64_t
> > and could have done better on commit message. Do you prefer a new revision with
> > better commit msg.
>
> If long is 32-bit on riscv32 should not we cast to grub_int32_t then?
>
> And then next question: Why off is defined as grub_int64_t?
Ping? I want get that fixed in 2.06 release...
Daniel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] RISC-V: Restore the typcast to 64bit type
@ 2021-03-28 22:45 Khem Raj
0 siblings, 0 replies; 6+ messages in thread
From: Khem Raj @ 2021-03-28 22:45 UTC (permalink / raw)
To: grub-devel
Cc: Khem Raj, Andreas Schwab, Daniel Kiper, Chester Lin, Alistair Francis
this makes the type promotions clear and explicit
It was already typecasted to long but was accidentally dropped in [1]
which stated to cause failures on riscv32 as reported in [2]
[1] https://git.savannah.gnu.org/cgit/grub.git/commit/?id=2bf40e9e5be9808b17852e688eead87acff14420
[2] https://savannah.gnu.org/bugs/index.php?60283
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Cc: Andreas Schwab <schwab@suse.de>
Cc: Daniel Kiper <daniel.kiper@oracle.com>
Cc: Chester Lin <clin@suse.com>
Cc: Alistair Francis <alistair.francis@wdc.com>
---
util/grub-mkimagexx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
index 00f49ccaa..ac677d03d 100644
--- a/util/grub-mkimagexx.c
+++ b/util/grub-mkimagexx.c
@@ -1242,7 +1242,7 @@ SUFFIX (relocate_addrs) (Elf_Ehdr *e, struct section_metadata *smd,
*/
sym_addr += addend;
- off = sym_addr - target_section_addr - offset - image_target->vaddr_offset;
+ off = (grub_int64_t)sym_addr - target_section_addr - offset - image_target->vaddr_offset;
switch (ELF_R_TYPE (info))
{
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-12 13:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-28 22:49 [PATCH] RISC-V: Restore the typcast to 64bit type Khem Raj
2021-03-29 10:24 ` Andreas Schwab
2021-03-29 18:33 ` Khem Raj
2021-03-29 22:29 ` Daniel Kiper
2021-04-12 13:07 ` Daniel Kiper
-- strict thread matches above, loose matches on Subject: below --
2021-03-28 22:45 Khem Raj
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.