public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* Systemtap's DTRACE_PROBE and clang
@ 2013-09-20  9:46 Martin Martin
  2013-09-20 18:59 ` Frank Ch. Eigler
  2013-09-20 19:25 ` Josh Stone
  0 siblings, 2 replies; 7+ messages in thread
From: Martin Martin @ 2013-09-20  9:46 UTC (permalink / raw)
  To: systemtap

Hi,

I've been submitting bug reports to Clang to get DTRACE_PROBE to
compile.  They implemented section flag "?", which solved the first
problem.

The remaining problem is that systemtap is using "note" in place of
"@note."  llvm-mc only accepts the @note form, which is the only form
gas is documented to accept:
https://sourceware.org/binutils/docs/as/Section.html#Section
(.pushsection refers to the documentation for .section for syntax).

The clang people write:

How onerous is it to change the code here? On the other side, it'd be
helpful to understand why gas is accepting (and people are writing) a
form of .pushsection that isn't in the documentation.

Workaround for now, it does work if you change it to:
  .pushsection .note.stapsdt,"?",@note

See:

http://llvm.org/bugs/show_bug.cgi?id=17270

The source was simply:

#include <sys/sdt.h>

int main() {
    DTRACE_PROBE(a, b);
}

Best,
Martin

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Systemtap's DTRACE_PROBE and clang
  2013-09-20  9:46 Systemtap's DTRACE_PROBE and clang Martin Martin
@ 2013-09-20 18:59 ` Frank Ch. Eigler
  2013-09-20 20:02   ` Josh Stone
  2013-09-20 19:25 ` Josh Stone
  1 sibling, 1 reply; 7+ messages in thread
From: Frank Ch. Eigler @ 2013-09-20 18:59 UTC (permalink / raw)
  To: Martin Martin; +Cc: systemtap


martin wrote:

> [...]
> How onerous is it to change the code here? [...]
> Workaround for now, it does work if you change it to:
>   .pushsection .note.stapsdt,"?",@note

I don't recall any deep discusson on that topic.  It could be
just to immunize it from being misinterpreted as a comment
(see the @-comment-on-arm line on the same AS doc page).  The
binutils change that allows "note" as @note appears to be

Author: Richard Henderson <rth@redhat.com>
Date:   Sat Jun 5 23:15:34 1999 +0000

so has been available for quite a few years.  If the LLVM folks aren't
willing to support it, I guess we could take some sort of ugly
conditional into sys/sdt.h.

- FChE

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Systemtap's DTRACE_PROBE and clang
  2013-09-20  9:46 Systemtap's DTRACE_PROBE and clang Martin Martin
  2013-09-20 18:59 ` Frank Ch. Eigler
@ 2013-09-20 19:25 ` Josh Stone
  1 sibling, 0 replies; 7+ messages in thread
From: Josh Stone @ 2013-09-20 19:25 UTC (permalink / raw)
  To: Martin Martin; +Cc: systemtap

On 09/20/2013 02:45 AM, Martin Martin wrote:
> Hi,
> 
> I've been submitting bug reports to Clang to get DTRACE_PROBE to
> compile.  They implemented section flag "?", which solved the first
> problem.

Great!

> The remaining problem is that systemtap is using "note" in place of
> "@note."  llvm-mc only accepts the @note form, which is the only form
> gas is documented to accept:
> https://sourceware.org/binutils/docs/as/Section.html#Section
> (.pushsection refers to the documentation for .section for syntax).
> 
> The clang people write:
> 
> How onerous is it to change the code here? On the other side, it'd be
> helpful to understand why gas is accepting (and people are writing) a
> form of .pushsection that isn't in the documentation.
> 
> Workaround for now, it does work if you change it to:
>   .pushsection .note.stapsdt,"?",@note

Interesting.  I'm not sure why we have "note" rather than @note, but
it's been that way at least since the beginning of Roland's SDTv3
rewrite.  But I just tried @note, like the patch below, and it looks ok
both on Fedora 19 and on RHEL5 (roughly the oldest we support).

I'll run it through our full testsuite, and assuming nothing comes up
then I'm happy to commit the change.


(quoted just so my mailer won't break lines)
> diff --git a/includes/sys/sdt.h b/includes/sys/sdt.h
> index ba04c12..c623caf 100644
> --- a/includes/sys/sdt.h
> +++ b/includes/sys/sdt.h
> @@ -173,7 +173,7 @@ __extension__ extern unsigned long long __sdt_unsp;
>  
>  #define _SDT_ASM_BODY(provider, name, pack_args, args)			      \
>    _SDT_ASM_1(990:	_SDT_NOP)					      \
> -  _SDT_ASM_3(		.pushsection .note.stapsdt,_SDT_ASM_AUTOGROUP,"note") \
> +  _SDT_ASM_3(		.pushsection .note.stapsdt,_SDT_ASM_AUTOGROUP,@note)  \
>    _SDT_ASM_1(		.balign 4)					      \
>    _SDT_ASM_3(		.4byte 992f-991f, 994f-993f, _SDT_NOTE_TYPE)	      \
>    _SDT_ASM_1(991:	.asciz _SDT_NOTE_NAME)				      \

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Systemtap's DTRACE_PROBE and clang
  2013-09-20 18:59 ` Frank Ch. Eigler
@ 2013-09-20 20:02   ` Josh Stone
  2013-09-20 20:26     ` Martin Martin
  0 siblings, 1 reply; 7+ messages in thread
From: Josh Stone @ 2013-09-20 20:02 UTC (permalink / raw)
  To: Martin Martin; +Cc: Frank Ch. Eigler, systemtap

[-- Attachment #1: Type: text/plain, Size: 763 bytes --]

On 09/20/2013 11:59 AM, Frank Ch. Eigler wrote:
> 
> martin wrote:
> 
>> [...]
>> How onerous is it to change the code here? [...]
>> Workaround for now, it does work if you change it to:
>>   .pushsection .note.stapsdt,"?",@note
> 
> I don't recall any deep discusson on that topic.  It could be
> just to immunize it from being misinterpreted as a comment
> (see the @-comment-on-arm line on the same AS doc page).

Based on that, I'm experimenting with %note instead.  It's a little more
complicated since that has to be escaped to %%note for C probes, but not
escaped for probes in pure ASM source files.  But that's just an #ifdef
__ASSEMBLER__, and so far this seems to work.

I'd appreciate if you can confirm it with your "?"-patched clang.

Thanks,
Josh

[-- Attachment #2: sdt_asm_note.patch --]
[-- Type: text/x-patch, Size: 1074 bytes --]

diff --git a/includes/sys/sdt.h b/includes/sys/sdt.h
index ba04c12..fbf5bc3 100644
--- a/includes/sys/sdt.h
+++ b/includes/sys/sdt.h
@@ -171,9 +171,20 @@ __extension__ extern unsigned long long __sdt_unsp;
 # define _SDT_ASM_AUTOGROUP ""
 #endif
 
+/* We used to have just "note", but some assemblers don't support that.
+ * GAS documents that it should be @note, but since ARM uses @ for
+ * comments, it also supports %note.  It appears we can get away with
+ * the latter everywhere, so long as we escape it depending on mode.  */
+#ifdef __ASSEMBLER__
+#define _SDT_ASM_NOTE %note
+#else
+#define _SDT_ASM_NOTE %%note
+#endif
+
 #define _SDT_ASM_BODY(provider, name, pack_args, args)			      \
   _SDT_ASM_1(990:	_SDT_NOP)					      \
-  _SDT_ASM_3(		.pushsection .note.stapsdt,_SDT_ASM_AUTOGROUP,"note") \
+  _SDT_ASM_3(		.pushsection .note.stapsdt,_SDT_ASM_AUTOGROUP,	      \
+							_SDT_ASM_NOTE)	      \
   _SDT_ASM_1(		.balign 4)					      \
   _SDT_ASM_3(		.4byte 992f-991f, 994f-993f, _SDT_NOTE_TYPE)	      \
   _SDT_ASM_1(991:	.asciz _SDT_NOTE_NAME)				      \

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Systemtap's DTRACE_PROBE and clang
  2013-09-20 20:02   ` Josh Stone
@ 2013-09-20 20:26     ` Martin Martin
  2013-09-20 21:27       ` Josh Stone
  0 siblings, 1 reply; 7+ messages in thread
From: Martin Martin @ 2013-09-20 20:26 UTC (permalink / raw)
  To: Josh Stone; +Cc: Frank Ch. Eigler, systemtap

You also need % for the other section in sys/sdt.h, progbits, and then it works!

On Fri, Sep 20, 2013 at 4:02 PM, Josh Stone <jistone@redhat.com> wrote:
> On 09/20/2013 11:59 AM, Frank Ch. Eigler wrote:
>>
>> martin wrote:
>>
>>> [...]
>>> How onerous is it to change the code here? [...]
>>> Workaround for now, it does work if you change it to:
>>>   .pushsection .note.stapsdt,"?",@note
>>
>> I don't recall any deep discusson on that topic.  It could be
>> just to immunize it from being misinterpreted as a comment
>> (see the @-comment-on-arm line on the same AS doc page).
>
> Based on that, I'm experimenting with %note instead.  It's a little more
> complicated since that has to be escaped to %%note for C probes, but not
> escaped for probes in pure ASM source files.  But that's just an #ifdef
> __ASSEMBLER__, and so far this seems to work.
>
> I'd appreciate if you can confirm it with your "?"-patched clang.
>
> Thanks,
> Josh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Systemtap's DTRACE_PROBE and clang
  2013-09-20 20:26     ` Martin Martin
@ 2013-09-20 21:27       ` Josh Stone
  2013-09-20 21:40         ` Martin Martin
  0 siblings, 1 reply; 7+ messages in thread
From: Josh Stone @ 2013-09-20 21:27 UTC (permalink / raw)
  To: Martin Martin; +Cc: Frank Ch. Eigler, systemtap

On 09/20/2013 01:26 PM, Martin Martin wrote:
> You also need % for the other section in sys/sdt.h, progbits, and then it works!

In my brief attempt, it appears that GCC *doesn't* want that escaped to
%%progbits - I guess because there are no operands to expand in that
particular asm statement?  Is that true for clang too?

> 
> On Fri, Sep 20, 2013 at 4:02 PM, Josh Stone <jistone@redhat.com> wrote:
>> On 09/20/2013 11:59 AM, Frank Ch. Eigler wrote:
>>>
>>> martin wrote:
>>>
>>>> [...]
>>>> How onerous is it to change the code here? [...]
>>>> Workaround for now, it does work if you change it to:
>>>>   .pushsection .note.stapsdt,"?",@note
>>>
>>> I don't recall any deep discusson on that topic.  It could be
>>> just to immunize it from being misinterpreted as a comment
>>> (see the @-comment-on-arm line on the same AS doc page).
>>
>> Based on that, I'm experimenting with %note instead.  It's a little more
>> complicated since that has to be escaped to %%note for C probes, but not
>> escaped for probes in pure ASM source files.  But that's just an #ifdef
>> __ASSEMBLER__, and so far this seems to work.
>>
>> I'd appreciate if you can confirm it with your "?"-patched clang.
>>
>> Thanks,
>> Josh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Systemtap's DTRACE_PROBE and clang
  2013-09-20 21:27       ` Josh Stone
@ 2013-09-20 21:40         ` Martin Martin
  0 siblings, 0 replies; 7+ messages in thread
From: Martin Martin @ 2013-09-20 21:40 UTC (permalink / raw)
  To: Josh Stone; +Cc: Frank Ch. Eigler, systemtap

Yes, single % works, but %% doesn't in Clang.

On Fri, Sep 20, 2013 at 5:27 PM, Josh Stone <jistone@redhat.com> wrote:
> On 09/20/2013 01:26 PM, Martin Martin wrote:
>> You also need % for the other section in sys/sdt.h, progbits, and then it works!
>
> In my brief attempt, it appears that GCC *doesn't* want that escaped to
> %%progbits - I guess because there are no operands to expand in that
> particular asm statement?  Is that true for clang too?
>
>>
>> On Fri, Sep 20, 2013 at 4:02 PM, Josh Stone <jistone@redhat.com> wrote:
>>> On 09/20/2013 11:59 AM, Frank Ch. Eigler wrote:
>>>>
>>>> martin wrote:
>>>>
>>>>> [...]
>>>>> How onerous is it to change the code here? [...]
>>>>> Workaround for now, it does work if you change it to:
>>>>>   .pushsection .note.stapsdt,"?",@note
>>>>
>>>> I don't recall any deep discusson on that topic.  It could be
>>>> just to immunize it from being misinterpreted as a comment
>>>> (see the @-comment-on-arm line on the same AS doc page).
>>>
>>> Based on that, I'm experimenting with %note instead.  It's a little more
>>> complicated since that has to be escaped to %%note for C probes, but not
>>> escaped for probes in pure ASM source files.  But that's just an #ifdef
>>> __ASSEMBLER__, and so far this seems to work.
>>>
>>> I'd appreciate if you can confirm it with your "?"-patched clang.
>>>
>>> Thanks,
>>> Josh
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-09-20 21:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-20  9:46 Systemtap's DTRACE_PROBE and clang Martin Martin
2013-09-20 18:59 ` Frank Ch. Eigler
2013-09-20 20:02   ` Josh Stone
2013-09-20 20:26     ` Martin Martin
2013-09-20 21:27       ` Josh Stone
2013-09-20 21:40         ` Martin Martin
2013-09-20 19:25 ` Josh Stone

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).