Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1583313 > unrolled thread

[PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC Jiri Slaby <jslaby@suse.cz> - 2017-02-17 12:00 +0100
    Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to  ENTRY/ENDPROC Josh Poimboeuf <jpoimboe@redhat.com> - 2017-02-17 14:20 +0100
      Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to  ENTRY/ENDPROC Jiri Slaby <jslaby@suse.cz> - 2017-02-17 14:40 +0100
        Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to  ENTRY/ENDPROC Josh Poimboeuf <jpoimboe@redhat.com> - 2017-02-17 15:10 +0100
          Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to  ENTRY/ENDPROC Jiri Slaby <jslaby@suse.cz> - 2017-02-17 15:30 +0100
            Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to  ENTRY/ENDPROC Josh Poimboeuf <jpoimboe@redhat.com> - 2017-02-17 22:20 +0100

#1583313 — [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC

This is just a preview, not to merged now, only later with DWARF
unwinder series. This is what the series will serve for (aside from
cleanup and unification).

I am aware of CFI_STARTPROC and CFI_ENDPROC left defined in other archs
in spite of cfi annotations removal ages ago. For simplicity. I am using
DW_ prefix here.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 include/linux/linkage.h | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/linkage.h b/include/linux/linkage.h
index fe5bbdac719b..af14364a63ef 100644
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -72,6 +72,14 @@
 #define __ALIGN_STR	".align 4,0x90"
 #endif
 
+#ifdef CONFIG_X86 /* to be replaced by CONFIG_DWARF_UNWIND after 01 tests */
+#define DW_CFI_STARTPROC .cfi_startproc
+#define DW_CFI_ENDPROC .cfi_endproc
+#else
+#define DW_CFI_STARTPROC
+#define DW_CFI_ENDPROC
+#endif
+
 #ifdef __ASSEMBLY__
 
 #ifndef LINKER_SCRIPT
@@ -92,7 +100,8 @@
 
 #ifndef ENTRY_LOCAL
 #define ENTRY_LOCAL(name) \
-	ENTRY_LOCAL_ALIAS(name)
+	ENTRY_LOCAL_ALIAS(name) ASM_NL \
+	DW_CFI_STARTPROC
 #endif
 
 #ifndef ENTRY
@@ -126,6 +135,7 @@
  */
 #ifndef ENDPROC
 #define ENDPROC(name) \
+	DW_CFI_ENDPROC ASM_NL \
 	END_ALIAS(name)
 #endif
 
-- 
2.11.1

[toc] | [next] | [standalone]

#1583397 — Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC

On Fri, Feb 17, 2017 at 11:47:57AM +0100, Jiri Slaby wrote:
> This is just a preview, not to merged now, only later with DWARF
> unwinder series. This is what the series will serve for (aside from
> cleanup and unification).
> 
> I am aware of CFI_STARTPROC and CFI_ENDPROC left defined in other archs
> in spite of cfi annotations removal ages ago. For simplicity. I am using
> DW_ prefix here.

If objtool is going to be generating CFI instructions, why not have it
generate .cfi_startproc and .cfi_endproc too?

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1583406 — Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC

On 02/17/2017, 02:16 PM, Josh Poimboeuf wrote:
> On Fri, Feb 17, 2017 at 11:47:57AM +0100, Jiri Slaby wrote:
>> This is just a preview, not to merged now, only later with DWARF
>> unwinder series. This is what the series will serve for (aside from
>> cleanup and unification).
>>
>> I am aware of CFI_STARTPROC and CFI_ENDPROC left defined in other archs
>> in spite of cfi annotations removal ages ago. For simplicity. I am using
>> DW_ prefix here.
> 
> If objtool is going to be generating CFI instructions, why not have it
> generate .cfi_startproc and .cfi_endproc too?

I tried that, but in many places it is very hard to recognize start
and/or end of a function. Having .cfi_startproc and .cfi_endproc in
place makes it rather easy, actually reduced to "emit dwarf instructions
for this code between here and there". Plus pre-prepared .eh_frame only
to be extended. (.eh_frame_hdr has to be rehashed of course.)

thanks,
-- 
js
suse labs

[toc] | [prev] | [next] | [standalone]

#1583430 — Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC

On Fri, Feb 17, 2017 at 02:36:15PM +0100, Jiri Slaby wrote:
> On 02/17/2017, 02:16 PM, Josh Poimboeuf wrote:
> > On Fri, Feb 17, 2017 at 11:47:57AM +0100, Jiri Slaby wrote:
> >> This is just a preview, not to merged now, only later with DWARF
> >> unwinder series. This is what the series will serve for (aside from
> >> cleanup and unification).
> >>
> >> I am aware of CFI_STARTPROC and CFI_ENDPROC left defined in other archs
> >> in spite of cfi annotations removal ages ago. For simplicity. I am using
> >> DW_ prefix here.
> > 
> > If objtool is going to be generating CFI instructions, why not have it
> > generate .cfi_startproc and .cfi_endproc too?
> 
> I tried that, but in many places it is very hard to recognize start
> and/or end of a function.

How so?  objtool already knows where the start and end of every function
is due to ENDPROC's use of the .type and .size macros.

> Having .cfi_startproc and .cfi_endproc in place makes it rather easy,
> actually reduced to "emit dwarf instructions for this code between
> here and there". Plus pre-prepared .eh_frame only to be extended.
> (.eh_frame_hdr has to be rehashed of course.)

Hm, but now objtool has to read *and* write CFI instead of just writing.
It would help to see the generation code.  Have you written it?

-- 
Josh

[toc] | [prev] | [next] | [standalone]

#1583461 — Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC

On 02/17/2017, 03:07 PM, Josh Poimboeuf wrote:
> On Fri, Feb 17, 2017 at 02:36:15PM +0100, Jiri Slaby wrote:
>> On 02/17/2017, 02:16 PM, Josh Poimboeuf wrote:
>>> On Fri, Feb 17, 2017 at 11:47:57AM +0100, Jiri Slaby wrote:
>>>> This is just a preview, not to merged now, only later with DWARF
>>>> unwinder series. This is what the series will serve for (aside from
>>>> cleanup and unification).
>>>>
>>>> I am aware of CFI_STARTPROC and CFI_ENDPROC left defined in other archs
>>>> in spite of cfi annotations removal ages ago. For simplicity. I am using
>>>> DW_ prefix here.
>>>
>>> If objtool is going to be generating CFI instructions, why not have it
>>> generate .cfi_startproc and .cfi_endproc too?
>>
>> I tried that, but in many places it is very hard to recognize start
>> and/or end of a function.
> 
> How so?  objtool already knows where the start and end of every function
> is due to ENDPROC's use of the .type and .size macros.

Right, I did not realized that we are writing about different things. So
yes, when the series is applied, we have ENTRY, ENDPROC et al. at all
appropriate places. We can indeed use that info.

>> Having .cfi_startproc and .cfi_endproc in place makes it rather easy,
>> actually reduced to "emit dwarf instructions for this code between
>> here and there". Plus pre-prepared .eh_frame only to be extended.
>> (.eh_frame_hdr has to be rehashed of course.)
> 
> Hm, but now objtool has to read *and* write CFI instead of just writing.

That is needed due to inline assembly anyway :/. So the way I wanted to
go was: here you have code with possibly incomplete (but at least some)
CFIs, fix the broken ones. Otherwise we would have to differentiate 2
kind of files:
* .c files with inline assembly (read-write = update CFIs)
* .S files with "native" assembly (write eh_frame header and also CFIs)

> It would help to see the generation code.  Have you written it?

I started writing it and it is complete crap so far :P. Please see
"objtool: generate dwarf for asm" at:
https://git.kernel.org/cgit/linux/kernel/git/jirislaby/linux.git/log/?h=devel

And yes, the current code is for .S files without any .eh_frame. Then I
decided to clean up ENTRY/ENDPROC etc. first. It's up to discussion what
route we want to go, but I would prefer the "read-write" since we have
to implement it anyway.

thanks,
-- 
js
suse labs

[toc] | [prev] | [next] | [standalone]

#1583747 — Re: [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC

On Fri, Feb 17, 2017 at 03:26:36PM +0100, Jiri Slaby wrote:
> On 02/17/2017, 03:07 PM, Josh Poimboeuf wrote:
> > On Fri, Feb 17, 2017 at 02:36:15PM +0100, Jiri Slaby wrote:
> >> On 02/17/2017, 02:16 PM, Josh Poimboeuf wrote:
> >>> On Fri, Feb 17, 2017 at 11:47:57AM +0100, Jiri Slaby wrote:
> >>>> This is just a preview, not to merged now, only later with DWARF
> >>>> unwinder series. This is what the series will serve for (aside from
> >>>> cleanup and unification).
> >>>>
> >>>> I am aware of CFI_STARTPROC and CFI_ENDPROC left defined in other archs
> >>>> in spite of cfi annotations removal ages ago. For simplicity. I am using
> >>>> DW_ prefix here.
> >>>
> >>> If objtool is going to be generating CFI instructions, why not have it
> >>> generate .cfi_startproc and .cfi_endproc too?
> >>
> >> I tried that, but in many places it is very hard to recognize start
> >> and/or end of a function.
> > 
> > How so?  objtool already knows where the start and end of every function
> > is due to ENDPROC's use of the .type and .size macros.
> 
> Right, I did not realized that we are writing about different things. So
> yes, when the series is applied, we have ENTRY, ENDPROC et al. at all
> appropriate places. We can indeed use that info.
> 
> >> Having .cfi_startproc and .cfi_endproc in place makes it rather easy,
> >> actually reduced to "emit dwarf instructions for this code between
> >> here and there". Plus pre-prepared .eh_frame only to be extended.
> >> (.eh_frame_hdr has to be rehashed of course.)
> > 
> > Hm, but now objtool has to read *and* write CFI instead of just writing.
> 
> That is needed due to inline assembly anyway :/. So the way I wanted to
> go was: here you have code with possibly incomplete (but at least some)
> CFIs, fix the broken ones. Otherwise we would have to differentiate 2
> kind of files:
> * .c files with inline assembly (read-write = update CFIs)
> * .S files with "native" assembly (write eh_frame header and also CFIs)

Have you seen any real inline asm issues?  I had been thinking they
weren't a problem, because CFI instructions are only emitted for the
function prologue and epilogue.  Running objtool with CFI analysis
seemed to confirm that -- I don't remember seeing any inline asm issues
with CFI like we did with frame pointers.

So my thinking for objtool CFI was:

.c files: read-only
.S files: write-only

> > It would help to see the generation code.  Have you written it?
> 
> I started writing it and it is complete crap so far :P. Please see
> "objtool: generate dwarf for asm" at:
> https://git.kernel.org/cgit/linux/kernel/git/jirislaby/linux.git/log/?h=devel
> 
> And yes, the current code is for .S files without any .eh_frame.

Nice!  Does it work?

> Then I decided to clean up ENTRY/ENDPROC etc. first. It's up to
> discussion what route we want to go, but I would prefer the
> "read-write" since we have to implement it anyway.

Yes, that would make sense if we have to do read-write for .c files.

-- 
Josh

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web