[sheepdog] [PATCH] Fix hardcode AMD64 assembly code (RSP)

Liu Yuan namei.unix at gmail.com
Mon Jul 29 08:41:35 CEST 2013


On Sun, Jul 28, 2013 at 02:05:49AM +0800, YunQiang Su wrote:
> RSP is a register name only for AMD64 (x86_64) platform,
> while on other platform it may be another name.
> 
> On i386 (x86_32) platform, it is "esp".
> On most of RISC platform (PowerPC, SPARC, MIPS, ARM, etc),
> it is "sp".
> ---
>  include/logger.h | 23 ++++++++++++++++++++++-
>  lib/logger.c     |  5 -----
>  2 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/include/logger.h b/include/logger.h
> index 79eb108..81691b2 100644
> --- a/include/logger.h
> +++ b/include/logger.h
> @@ -38,10 +38,31 @@ void log_write(int prio, const char *func, int line, const char *fmt, ...)
>  void set_thread_name(const char *name, bool show_idx);
>  void get_thread_name(char *name);
>  
> +#ifdef __i386__
> +#define SP_REG "esp"
> +#elif defined(__x86_64__)
> +#define SP_REG "rsp"
> +#elif defined(__mips__) || defined(__sparc__) || \
> +      defined (__arm__) || (defined __powerpc__) || \
> +      defined (__s390__) || (defined __s390x__) || \
> +      defined (__ia64__)
> +#define SP_REG "sp"
> +#endif
> +
> +#ifdef SP_REG
> +#define dump_stack_frames() ({			\
> +	register void *current_sp asm(SP_REG);	\
> +	__dump_stack_frames(current_sp);	\
> +})
>  #define sd_dump_variable(var) ({		\
> -	register void *current_sp asm("rsp");	\
> +	register void *current_sp asm(SP_REG);	\
>  	__sd_dump_variable(#var, current_sp);	\
>  })
> +#else
> +#define dump_stack_frames() ()
> +#define sd_dump_variable(var) ()
> +#endif
> +
>  int __sd_dump_variable(const char *var, const void *base_sp);
>  void sd_backtrace(void);
>  
> diff --git a/lib/logger.c b/lib/logger.c
> index 253163d..ba55adc 100644
> --- a/lib/logger.c
> +++ b/lib/logger.c
> @@ -742,11 +742,6 @@ notrace int __sd_dump_variable(const char *var, const void *base_sp)
>  	return 0;
>  }
>  
> -#define dump_stack_frames() ({			\
> -	register void *current_sp asm("rsp");	\
> -	__dump_stack_frames(current_sp);	\
> -})
> -
>  __attribute__ ((__noinline__))
>  static notrace int __dump_stack_frames(const void *base_sp)
>  {
> -- 

I've come up with a better fix. Which use the gcc builtin function to get the
stack pointer.

>From 44300c3fe580a677757803cc30838923ccc5b748 Mon Sep 17 00:00:00 2001
From: Liu Yuan <namei.unix at gmail.com>
Date: Mon, 29 Jul 2013 14:33:58 +0800
Subject: [PATCH] sheep: use gcc builtin to get stack frame pointer

This remove hardcoded assembly which is not portable.

Signed-off-by: Liu Yuan <namei.unix at gmail.com>

diff --git a/include/logger.h b/include/logger.h
index 79eb108..9253548 100644
--- a/include/logger.h
+++ b/include/logger.h
@@ -39,10 +39,9 @@ void set_thread_name(const char *name, bool show_idx);
 void get_thread_name(char *name);
 
 #define sd_dump_variable(var) ({		\
-	register void *current_sp asm("rsp");	\
-	__sd_dump_variable(#var, current_sp);	\
+	__sd_dump_variable(#var);		\
 })
-int __sd_dump_variable(const char *var, const void *base_sp);
+int __sd_dump_variable(const char *var);
 void sd_backtrace(void);
 
 /* sheep log priorities, comliant with syslog spec */
diff --git a/lib/logger.c b/lib/logger.c
index 253163d..da1221b 100644
--- a/lib/logger.c
+++ b/lib/logger.c
@@ -697,11 +697,15 @@ static bool check_gdb(void)
 	return system("which gdb > /dev/null") == 0;
 }
 
+#define STACK_POINTER	\
+	((char *)__builtin_frame_address(0) + __SIZEOF_POINTER__ * 2)
+
 __attribute__ ((__noinline__))
-notrace int __sd_dump_variable(const char *var, const void *base_sp)
+notrace int __sd_dump_variable(const char *var)
 {
 	char cmd[ARG_MAX], path[PATH_MAX], info[256];
 	FILE *f = NULL;
+	void *base_sp = STACK_POINTER;
 
 	if (!check_gdb()) {
 		sd_dprintf("cannot find gdb");
@@ -742,16 +746,12 @@ notrace int __sd_dump_variable(const char *var, const void *base_sp)
 	return 0;
 }
 
-#define dump_stack_frames() ({			\
-	register void *current_sp asm("rsp");	\
-	__dump_stack_frames(current_sp);	\
-})
-
 __attribute__ ((__noinline__))
-static notrace int __dump_stack_frames(const void *base_sp)
+static notrace int dump_stack_frames(void)
 {
 	char path[PATH_MAX];
 	int i, stack_no = 0;
+	void *base_sp = STACK_POINTER;
 
 	if (!check_gdb()) {
 		sd_dprintf("cannot find gdb");




More information about the sheepdog mailing list