* [PATCH] RISC-V: gas: Change initial CFI operation from DW_CFA_def_cfa_register to DW_CFA_def_cfa [not found] <CGME20240513081154epcas1p365a9507c6e31950c50569b9ffdd63f15@epcas1p3.samsung.com> @ 2024-05-13 8:11 ` Sung-hun Kim 2024-05-14 23:19 ` Nelson Chu 2024-05-17 13:57 ` Andrew Burgess 0 siblings, 2 replies; 4+ messages in thread From: Sung-hun Kim @ 2024-05-13 8:11 UTC (permalink / raw) To: binutils; +Cc: sungguk.na, dongkyun.s, sw0312.kim, sfoon.kim The DWARF specification (especially, DWARF4 and 5 [1,2]) states that DW_CFA_def_cfa_register cannot be used as the first CFI operation. It said DW_CFA_def_cfa_register as follows: ... This operation is valid only if the current CFA rule is defined to use a register and offset. So, DW_CFA_def_cfa_register can be used after that other definition operation such as DW_CFA_def_cfa is called. However, the current gas code emits DW_CFA_def_cfa_register as an initial CFI operation for RISCV. In the libgcc, the unwinding function does not care about it, so it can unwind the call stack. However, on the third party library such as libunwindstack in Android, it causes a fatal error. This patch changes the initial CFI operation to DW_CFA_def_cfa with offset 0. It works as same as the previous one, but it does not have any limitation so it satisfies the DWARF spec. This change resolves the compatibility issue while preserving the original behaviour. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31733 [1] DWARF4 specification, https://dwarfstd.org/doc/DWARF4.pdf [2] DWARF5 specification, https://dwarfstd.org/doc/DWARF5.pdf Signed-off-by: Sung-hun Kim <sfoon.kim@samsung.com> --- gas/config/tc-riscv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c index 1757ff6e9e2..8d749581c1d 100644 --- a/gas/config/tc-riscv.c +++ b/gas/config/tc-riscv.c @@ -5209,7 +5209,7 @@ RISC-V options:\n\ void riscv_cfi_frame_initial_instructions (void) { - cfi_add_CFA_def_cfa_register (X_SP); + cfi_add_CFA_def_cfa (X_SP, 0); } int -- 2.25.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: gas: Change initial CFI operation from DW_CFA_def_cfa_register to DW_CFA_def_cfa 2024-05-13 8:11 ` [PATCH] RISC-V: gas: Change initial CFI operation from DW_CFA_def_cfa_register to DW_CFA_def_cfa Sung-hun Kim @ 2024-05-14 23:19 ` Nelson Chu 2024-05-17 13:57 ` Andrew Burgess 1 sibling, 0 replies; 4+ messages in thread From: Nelson Chu @ 2024-05-14 23:19 UTC (permalink / raw) To: Sung-hun Kim, gdb-patches; +Cc: binutils, sungguk.na, dongkyun.s, sw0312.kim [-- Attachment #1: Type: text/plain, Size: 1920 bytes --] Also send this to gdb mailing list as there should be more DWARF experts out there. On Mon, May 13, 2024 at 4:13 PM Sung-hun Kim <sfoon.kim@samsung.com> wrote: > The DWARF specification (especially, DWARF4 and 5 [1,2]) states that > DW_CFA_def_cfa_register cannot be used as the first CFI operation. > It said DW_CFA_def_cfa_register as follows: > > ... This operation is valid only if the current CFA rule is defined > to use a register and offset. > > So, DW_CFA_def_cfa_register can be used after that other definition > operation such as DW_CFA_def_cfa is called. However, the current gas > code emits DW_CFA_def_cfa_register as an initial CFI operation for RISCV. > > In the libgcc, the unwinding function does not care about it, so it can > unwind the call stack. However, on the third party library such as > libunwindstack in Android, it causes a fatal error. > > This patch changes the initial CFI operation to DW_CFA_def_cfa with > offset 0. It works as same as the previous one, but it does not have > any limitation so it satisfies the DWARF spec. This change resolves > the compatibility issue while preserving the original behaviour. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31733 > > [1] DWARF4 specification, https://dwarfstd.org/doc/DWARF4.pdf > [2] DWARF5 specification, https://dwarfstd.org/doc/DWARF5.pdf > > Signed-off-by: Sung-hun Kim <sfoon.kim@samsung.com> > --- > gas/config/tc-riscv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c > index 1757ff6e9e2..8d749581c1d 100644 > --- a/gas/config/tc-riscv.c > +++ b/gas/config/tc-riscv.c > @@ -5209,7 +5209,7 @@ RISC-V options:\n\ > void > riscv_cfi_frame_initial_instructions (void) > { > - cfi_add_CFA_def_cfa_register (X_SP); > + cfi_add_CFA_def_cfa (X_SP, 0); > } > > int > -- > 2.25.1 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: gas: Change initial CFI operation from DW_CFA_def_cfa_register to DW_CFA_def_cfa 2024-05-13 8:11 ` [PATCH] RISC-V: gas: Change initial CFI operation from DW_CFA_def_cfa_register to DW_CFA_def_cfa Sung-hun Kim 2024-05-14 23:19 ` Nelson Chu @ 2024-05-17 13:57 ` Andrew Burgess 2024-05-20 2:54 ` Nelson Chu 1 sibling, 1 reply; 4+ messages in thread From: Andrew Burgess @ 2024-05-17 13:57 UTC (permalink / raw) To: Sung-hun Kim, binutils; +Cc: sungguk.na, dongkyun.s, sw0312.kim, sfoon.kim Sung-hun Kim <sfoon.kim@samsung.com> writes: > The DWARF specification (especially, DWARF4 and 5 [1,2]) states that > DW_CFA_def_cfa_register cannot be used as the first CFI operation. > It said DW_CFA_def_cfa_register as follows: > > ... This operation is valid only if the current CFA rule is defined > to use a register and offset. > > So, DW_CFA_def_cfa_register can be used after that other definition > operation such as DW_CFA_def_cfa is called. However, the current gas > code emits DW_CFA_def_cfa_register as an initial CFI operation for RISCV. > > In the libgcc, the unwinding function does not care about it, so it can > unwind the call stack. However, on the third party library such as > libunwindstack in Android, it causes a fatal error. > > This patch changes the initial CFI operation to DW_CFA_def_cfa with > offset 0. It works as same as the previous one, but it does not have > any limitation so it satisfies the DWARF spec. This change resolves > the compatibility issue while preserving the original behaviour. > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31733 I'm not a binutils maintainer, so can't approve this patch. But, for what it's worth, I think this is the right change to make. Reviewed-By: Andrew Burgess <aburgess@redhat.com> Thanks, Andrew > > [1] DWARF4 specification, https://dwarfstd.org/doc/DWARF4.pdf > [2] DWARF5 specification, https://dwarfstd.org/doc/DWARF5.pdf > > Signed-off-by: Sung-hun Kim <sfoon.kim@samsung.com> > --- > gas/config/tc-riscv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c > index 1757ff6e9e2..8d749581c1d 100644 > --- a/gas/config/tc-riscv.c > +++ b/gas/config/tc-riscv.c > @@ -5209,7 +5209,7 @@ RISC-V options:\n\ > void > riscv_cfi_frame_initial_instructions (void) > { > - cfi_add_CFA_def_cfa_register (X_SP); > + cfi_add_CFA_def_cfa (X_SP, 0); > } > > int > -- > 2.25.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: gas: Change initial CFI operation from DW_CFA_def_cfa_register to DW_CFA_def_cfa 2024-05-17 13:57 ` Andrew Burgess @ 2024-05-20 2:54 ` Nelson Chu 0 siblings, 0 replies; 4+ messages in thread From: Nelson Chu @ 2024-05-20 2:54 UTC (permalink / raw) To: Andrew Burgess; +Cc: Sung-hun Kim, binutils, sungguk.na, dongkyun.s, sw0312.kim [-- Attachment #1: Type: text/plain, Size: 2265 bytes --] Thanks for helping, Andrew :-) Approved and committed. Nelson On Fri, May 17, 2024 at 9:59 PM Andrew Burgess <aburgess@redhat.com> wrote: > Sung-hun Kim <sfoon.kim@samsung.com> writes: > > > The DWARF specification (especially, DWARF4 and 5 [1,2]) states that > > DW_CFA_def_cfa_register cannot be used as the first CFI operation. > > It said DW_CFA_def_cfa_register as follows: > > > > ... This operation is valid only if the current CFA rule is defined > > to use a register and offset. > > > > So, DW_CFA_def_cfa_register can be used after that other definition > > operation such as DW_CFA_def_cfa is called. However, the current gas > > code emits DW_CFA_def_cfa_register as an initial CFI operation for RISCV. > > > > In the libgcc, the unwinding function does not care about it, so it can > > unwind the call stack. However, on the third party library such as > > libunwindstack in Android, it causes a fatal error. > > > > This patch changes the initial CFI operation to DW_CFA_def_cfa with > > offset 0. It works as same as the previous one, but it does not have > > any limitation so it satisfies the DWARF spec. This change resolves > > the compatibility issue while preserving the original behaviour. > > > > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31733 > > I'm not a binutils maintainer, so can't approve this patch. > > But, for what it's worth, I think this is the right change to make. > > Reviewed-By: Andrew Burgess <aburgess@redhat.com> > > Thanks, > Andrew > > > > > [1] DWARF4 specification, https://dwarfstd.org/doc/DWARF4.pdf > > [2] DWARF5 specification, https://dwarfstd.org/doc/DWARF5.pdf > > > > Signed-off-by: Sung-hun Kim <sfoon.kim@samsung.com> > > --- > > gas/config/tc-riscv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c > > index 1757ff6e9e2..8d749581c1d 100644 > > --- a/gas/config/tc-riscv.c > > +++ b/gas/config/tc-riscv.c > > @@ -5209,7 +5209,7 @@ RISC-V options:\n\ > > void > > riscv_cfi_frame_initial_instructions (void) > > { > > - cfi_add_CFA_def_cfa_register (X_SP); > > + cfi_add_CFA_def_cfa (X_SP, 0); > > } > > > > int > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-05-20 2:54 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20240513081154epcas1p365a9507c6e31950c50569b9ffdd63f15@epcas1p3.samsung.com> 2024-05-13 8:11 ` [PATCH] RISC-V: gas: Change initial CFI operation from DW_CFA_def_cfa_register to DW_CFA_def_cfa Sung-hun Kim 2024-05-14 23:19 ` Nelson Chu 2024-05-17 13:57 ` Andrew Burgess 2024-05-20 2:54 ` Nelson Chu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).