All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] tracepoint: mark __tracepoint_string's __used
       [not found] <20200723205341.1099742-1-ndesaulniers@google.com>
@ 2020-07-23 20:53 ` Nick Desaulniers
  2020-07-23 20:53 ` [PATCH 2/2] tracepoint: used attribute definitions from compiler_attributes.h Nick Desaulniers
  1 sibling, 0 replies; 3+ messages in thread
From: Nick Desaulniers @ 2020-07-23 20:53 UTC (permalink / raw)
  To: Steven Rostedt, Ingo Molnar
  Cc: Miguel Ojeda, Nick Desaulniers, stable, Tim Murray,
	Simon MacMullen, Greg Hackmann, linux-kernel, clang-built-linux

__tracepoint_string's have their string data stored in .rodata, and an
address to that data stored in the "__tracepoint_str" section. Functions
that refer to those strings refer to the symbol of the address. Compiler
optimization can replace those address references with references
directly to the string data. If the address doesn't appear to have other
uses, then it appears dead to the compiler and is removed. This can
break the /tracing/printk_formats sysfs node which iterates the
addresses stored in the "__tracepoint_str" section.

Like other strings stored in custom sections in this header, mark these
__used to inform the compiler that there are other non-obvious users of
the address, so they should still be emitted.

Cc: stable@vger.kernel.org
Reported-by: Tim Murray <timmurray@google.com>
Reported-by: Simon MacMullen <simonmacm@google.com>
Suggested-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
We observe this in Clang; it seems that GCC doesn't do the "cleanup" of
the dead address.

Specifically, the Clang passes "Interprocedural Sparse Conditional
Constant Propagation" (IPSCCP) and GlobalOpt both try to removed the
address if no other uses exist after inlining the reference directly to
the string data.

We don't want to change the linkage of these variables, but we kind of
want optimization behavior to treat these function static strings as if
they had `extern` linkage, at least by not removing the address of the
string data from the custom section.

 include/linux/tracepoint.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index a1fecf311621..3a5b717d92e8 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -361,7 +361,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 		static const char *___tp_str __tracepoint_string = str; \
 		___tp_str;						\
 	})
-#define __tracepoint_string	__attribute__((section("__tracepoint_str")))
+#define __tracepoint_string	__attribute__((section("__tracepoint_str"), used))
 #else
 /*
  * tracepoint_string() is used to save the string address for userspace
-- 
2.28.0.rc0.105.gf9edc3c819-goog


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

* [PATCH 2/2] tracepoint: used attribute definitions from compiler_attributes.h
       [not found] <20200723205341.1099742-1-ndesaulniers@google.com>
  2020-07-23 20:53 ` [PATCH 1/2] tracepoint: mark __tracepoint_string's __used Nick Desaulniers
@ 2020-07-23 20:53 ` Nick Desaulniers
  2020-07-24  9:24   ` Miguel Ojeda
  1 sibling, 1 reply; 3+ messages in thread
From: Nick Desaulniers @ 2020-07-23 20:53 UTC (permalink / raw)
  To: Steven Rostedt, Ingo Molnar; +Cc: Miguel Ojeda, Nick Desaulniers, linux-kernel

Just a small cleanup while I was touching this header.
compiler_attributes.h does feature detection of these __attributes__(())
and provides more concise ways to invoke them.

Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 include/linux/tracepoint.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 3a5b717d92e8..598fec9f9dbf 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -116,8 +116,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 
 #define __TRACEPOINT_ENTRY(name)					 \
 	static tracepoint_ptr_t __tracepoint_ptr_##name __used		 \
-	__attribute__((section("__tracepoints_ptrs"))) =		 \
-		&__tracepoint_##name
+	__section(__tracepoints_ptrs) = &__tracepoint_##name
 #endif
 
 #endif /* _LINUX_TRACEPOINT_H */
@@ -280,9 +279,9 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
  */
 #define DEFINE_TRACE_FN(name, reg, unreg)				 \
 	static const char __tpstrtab_##name[]				 \
-	__attribute__((section("__tracepoints_strings"))) = #name;	 \
-	struct tracepoint __tracepoint_##name				 \
-	__attribute__((section("__tracepoints"), used)) =		 \
+	__section(__tracepoints_strings) = #name;			 \
+	struct tracepoint __tracepoint_##name __used			 \
+	__section(__tracepoints) =					 \
 		{ __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
 	__TRACEPOINT_ENTRY(name);
 
@@ -361,7 +360,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 		static const char *___tp_str __tracepoint_string = str; \
 		___tp_str;						\
 	})
-#define __tracepoint_string	__attribute__((section("__tracepoint_str"), used))
+#define __tracepoint_string	__used __section(__tracepoint_str)
 #else
 /*
  * tracepoint_string() is used to save the string address for userspace
-- 
2.28.0.rc0.105.gf9edc3c819-goog


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

* Re: [PATCH 2/2] tracepoint: used attribute definitions from compiler_attributes.h
  2020-07-23 20:53 ` [PATCH 2/2] tracepoint: used attribute definitions from compiler_attributes.h Nick Desaulniers
@ 2020-07-24  9:24   ` Miguel Ojeda
  0 siblings, 0 replies; 3+ messages in thread
From: Miguel Ojeda @ 2020-07-24  9:24 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: Steven Rostedt, Ingo Molnar, linux-kernel

Hi Nick,

On Thu, Jul 23, 2020 at 10:54 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Just a small cleanup while I was touching this header.
> compiler_attributes.h does feature detection of these __attributes__(())
> and provides more concise ways to invoke them.

Thanks for taking the time to add this patch to the other to clean this up!

Acked-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>

Cheers,
Miguel

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

end of thread, other threads:[~2020-07-24  9:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200723205341.1099742-1-ndesaulniers@google.com>
2020-07-23 20:53 ` [PATCH 1/2] tracepoint: mark __tracepoint_string's __used Nick Desaulniers
2020-07-23 20:53 ` [PATCH 2/2] tracepoint: used attribute definitions from compiler_attributes.h Nick Desaulniers
2020-07-24  9:24   ` Miguel Ojeda

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.