[RFC] kallsyms C_SYMBOL_PREFIX support

[RFC] kallsyms C_SYMBOL_PREFIX support

[Yoshinori Sato]

kallsyms does not consider SYMBOL_PREFIX of C.
Consequently do not work in architecture using prefix character (h8300, v850) really.

Because I can want to use this, I made a patch.
Please comment.

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

===== scripts/kallsyms.c 1.16 vs edited =====
--- 1.16/scripts/kallsyms.c	2005-01-01 19:22:13 +09:00
+++ edited/scripts/kallsyms.c	2005-04-12 15:50:47 +09:00
@@ -69,6 +69,7 @@
 static int size, cnt;
 static unsigned long long _stext, _etext, _sinittext, _einittext;
 static int all_symbols = 0;
+static char symbol_prefix_char = '\0';
 
 struct token {
 	unsigned char data[MAX_TOK_SIZE];
@@ -93,7 +94,7 @@
 static void
 usage(void)
 {
-	fprintf(stderr, "Usage: kallsyms [--all-symbols] < in.map > out.S\n");
+	fprintf(stderr, "Usage: kallsyms [--all-symbols] [--symbol-prefix=<prefix char>] < in.map > out.S\n");
 	exit(1);
 }
 
@@ -112,6 +113,7 @@
 read_symbol(FILE *in, struct sym_entry *s)
 {
 	char str[500];
+	char *sym;
 	int rc;
 
 	rc = fscanf(in, "%llx %c %499s\n", &s->addr, &s->type, str);
@@ -123,27 +125,32 @@
 		return -1;
 	}
 
+	sym = str;
+	/* skip prefix char */
+	if (symbol_prefix_char && str[0] == symbol_prefix_char)
+		sym++;
+
 	/* Ignore most absolute/undefined (?) symbols. */
-	if (strcmp(str, "_stext") == 0)
+	if (strcmp(sym, "_stext") == 0)
 		_stext = s->addr;
-	else if (strcmp(str, "_etext") == 0)
+	else if (strcmp(sym, "_etext") == 0)
 		_etext = s->addr;
-	else if (strcmp(str, "_sinittext") == 0)
+	else if (strcmp(sym, "_sinittext") == 0)
 		_sinittext = s->addr;
-	else if (strcmp(str, "_einittext") == 0)
+	else if (strcmp(sym, "_einittext") == 0)
 		_einittext = s->addr;
 	else if (toupper(s->type) == 'A')
 	{
 		/* Keep these useful absolute symbols */
-		if (strcmp(str, "__kernel_syscall_via_break") &&
-		    strcmp(str, "__kernel_syscall_via_epc") &&
-		    strcmp(str, "__kernel_sigtramp") &&
-		    strcmp(str, "__gp"))
+		if (strcmp(sym, "__kernel_syscall_via_break") &&
+		    strcmp(sym, "__kernel_syscall_via_epc") &&
+		    strcmp(sym, "__kernel_sigtramp") &&
+		    strcmp(sym, "__gp"))
 			return -1;
 
 	}
 	else if (toupper(s->type) == 'U' ||
-		 is_arm_mapping_symbol(str))
+		 is_arm_mapping_symbol(sym))
 		return -1;
 
 	/* include the type field in the symbol name, so that it gets
@@ -177,6 +184,11 @@
 		"_SDA2_BASE_",		/* ppc */
 		NULL };
 	int i;
+	int offset = 1;
+
+	/* skip prefix char */
+	if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
+		offset++;
 
 	/* if --all-symbols is not specified, then symbols outside the text
 	 * and inittext sections are discarded */
@@ -190,17 +202,17 @@
 		 * they may get dropped in pass 2, which breaks the kallsyms
 		 * rules.
 		 */
-		if ((s->addr == _etext && strcmp(s->sym + 1, "_etext")) ||
-		    (s->addr == _einittext && strcmp(s->sym + 1, "_einittext")))
+		if ((s->addr == _etext && strcmp(s->sym + offset, "_etext")) ||
+		    (s->addr == _einittext && strcmp(s->sym + offset, "_einittext")))
 			return 0;
 	}
 
 	/* Exclude symbols which vary between passes. */
-	if (strstr(s->sym + 1, "_compiled."))
+	if (strstr(s->sym + offset, "_compiled."))
 		return 0;
 
 	for (i = 0; special_symbols[i]; i++)
-		if( strcmp(s->sym + 1, special_symbols[i]) == 0 )
+		if( strcmp(s->sym + offset, special_symbols[i]) == 0 )
 			return 0;
 
 	return 1;
@@ -225,9 +237,15 @@
 
 static void output_label(char *label)
 {
-	printf(".globl %s\n",label);
+	if (symbol_prefix_char)
+		printf(".globl %c%s\n", symbol_prefix_char, label);
+	else
+		printf(".globl %s\n", label);
 	printf("\tALGN\n");
-	printf("%s:\n",label);
+	if (symbol_prefix_char)
+		printf("%c%s:\n", symbol_prefix_char, label);
+	else
+		printf("%s:\n", label);
 }
 
 /* uncompress a compressed symbol. When this function is called, the best table
@@ -665,6 +683,13 @@
 
 	insert_real_symbols_in_table();
 
+	/* When valid symbol is not registered, exit to error */
+	if (good_head.left == good_head.right &&
+	    bad_head.left == bad_head.right) {
+		fprintf(stderr, "No valid symbol.\n");
+		exit(1);
+	}
+
 	optimize_result();
 }
 
@@ -672,9 +697,21 @@
 int
 main(int argc, char **argv)
 {
-	if (argc == 2 && strcmp(argv[1], "--all-symbols") == 0)
-		all_symbols = 1;
-	else if (argc != 1)
+	if (argc >= 2) {
+		int i;
+		for (i = 1; i < argc; i++) { 
+			if(strcmp(argv[i], "--all-symbols") == 0)
+				all_symbols = 1;
+			else if (strncmp(argv[i], "--symbol-prefix=", 16) == 0) {
+				char *p = &argv[i][16];
+				/* skip quote */
+				if ((*p == '"' && *(p+2) == '"') || (*p == '\'' && *(p+2) == '\''))
+					p++;
+				symbol_prefix_char = *p;
+			} else
+				usage();
+		}
+	} else if (argc != 1)
 		usage();
 
 	read_map(stdin);
@@ -683,4 +720,3 @@
 
 	return 0;
 }
-

Re: [RFC] kallsyms C_SYMBOL_PREFIX support

[Paulo Marques]

Yoshinori Sato wrote:
> kallsyms does not consider SYMBOL_PREFIX of C.
> Consequently do not work in architecture using prefix character (h8300, v850) really.
> 
> Because I can want to use this, I made a patch.
> Please comment.
> 
> [...]

> @@ -177,6 +184,11 @@
>  		"_SDA2_BASE_",		/* ppc */
>  		NULL };
>  	int i;
> +	int offset = 1;
> +
> +	/* skip prefix char */
> +	if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
> +		offset++;

maybe something like:

char *sname;
sname = s->sym + 1;
if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
	sname++;

would avoid all the "(s->sym + offset)" below, turning them to just "sname".

I know that it was "(s->sym + 1)" before, so its really not your fault, 
but you could take this opportunity to clean that up, too.

>  
>  	/* if --all-symbols is not specified, then symbols outside the text
>  	 * and inittext sections are discarded */
> @@ -190,17 +202,17 @@
>  		 * they may get dropped in pass 2, which breaks the kallsyms
>  		 * rules.
>  		 */
> -		if ((s->addr == _etext && strcmp(s->sym + 1, "_etext")) ||
> -		    (s->addr == _einittext && strcmp(s->sym + 1, "_einittext")))
> +		if ((s->addr == _etext && strcmp(s->sym + offset, "_etext")) ||
> +		    (s->addr == _einittext && strcmp(s->sym + offset, "_einittext")))
>  			return 0;
>  	}
>  
>  	/* Exclude symbols which vary between passes. */
> -	if (strstr(s->sym + 1, "_compiled."))
> +	if (strstr(s->sym + offset, "_compiled."))
>  		return 0;
>  
>  	for (i = 0; special_symbols[i]; i++)
> -		if( strcmp(s->sym + 1, special_symbols[i]) == 0 )
> +		if( strcmp(s->sym + offset, special_symbols[i]) == 0 )
>  			return 0;
>  
>  	return 1;
> @@ -225,9 +237,15 @@

>  [...]
>  
>  /* uncompress a compressed symbol. When this function is called, the best table
> @@ -665,6 +683,13 @@
>  
>  	insert_real_symbols_in_table();
>  
> +	/* When valid symbol is not registered, exit to error */
> +	if (good_head.left == good_head.right &&
> +	    bad_head.left == bad_head.right) {
> +		fprintf(stderr, "No valid symbol.\n");
> +		exit(1);
> +	}
> +
>  	optimize_result();
>  }

This should only trigger if there are no symbols at all, or if there are 
some symbols that are considered invalid, and do not go into the final 
result.

Maybe we should just do a return here instead of exit, so that even if 
this happens, kallsyms will still produce an empty result, that will at 
least allow the kernel to compile.

It should give the error output to warn the user that there is something 
fishy, nevertheless. Maybe even a bigger message, since this should not 
happen at all, and if this triggers it means that something is seriously 
wrong.

> @@ -672,9 +697,21 @@
>  int
>  main(int argc, char **argv)
>  {
> -	if (argc == 2 && strcmp(argv[1], "--all-symbols") == 0)
> -		all_symbols = 1;
> -	else if (argc != 1)
> +	if (argc >= 2) {

This test is unnecessary.

> +		int i;
> +		for (i = 1; i < argc; i++) { 
> +			if(strcmp(argv[i], "--all-symbols") == 0)
> +				all_symbols = 1;
> +			else if (strncmp(argv[i], "--symbol-prefix=", 16) == 0) {
> +				char *p = &argv[i][16];
> +				/* skip quote */
> +				if ((*p == '"' && *(p+2) == '"') || (*p == '\'' && *(p+2) == '\''))
> +					p++;
> +				symbol_prefix_char = *p;
> +			} else
> +				usage();
> +		}
> +	} else if (argc != 1)
>  		usage();

and so is this.

>  
>  	read_map(stdin);
> @@ -683,4 +720,3 @@
>  
>  	return 0;
>  }

At least the patch seems to not affect architectures that don't use the 
"--symbol-prefix" option, so it should be harmless for most.

Anyway, appart from the few comments, it has my acknowledge.

-- 
Paulo Marques - www.grupopie.com

All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)

Re: [RFC] kallsyms C_SYMBOL_PREFIX support

[Yoshinori Sato]

At Tue, 12 Apr 2005 12:43:34 +0100,
Paulo Marques wrote:
> 
> Yoshinori Sato wrote:
> > kallsyms does not consider SYMBOL_PREFIX of C.
> > Consequently do not work in architecture using prefix character (h8300, v850) really.
> > 
> > Because I can want to use this, I made a patch.
> > Please comment.
> > 
> > [...]
> 
> > @@ -177,6 +184,11 @@
> >  		"_SDA2_BASE_",		/* ppc */
> >  		NULL };
> >  	int i;
> > +	int offset = 1;
> > +
> > +	/* skip prefix char */
> > +	if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
> > +		offset++;
> 
> maybe something like:
> 
> char *sname;
> sname = s->sym + 1;
> if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
> 	sname++;
> 
> would avoid all the "(s->sym + offset)" below, turning them to just "sname".
> 
> I know that it was "(s->sym + 1)" before, so its really not your fault, 
> but you could take this opportunity to clean that up, too.

This one is fine.
 
> >  
> >  	/* if --all-symbols is not specified, then symbols outside the text
> >  	 * and inittext sections are discarded */
> > @@ -190,17 +202,17 @@
> >  		 * they may get dropped in pass 2, which breaks the kallsyms
> >  		 * rules.
> >  		 */
> > -		if ((s->addr == _etext && strcmp(s->sym + 1, "_etext")) ||
> > -		    (s->addr == _einittext && strcmp(s->sym + 1, "_einittext")))
> > +		if ((s->addr == _etext && strcmp(s->sym + offset, "_etext")) ||
> > +		    (s->addr == _einittext && strcmp(s->sym + offset, "_einittext")))
> >  			return 0;
> >  	}
> >  
> >  	/* Exclude symbols which vary between passes. */
> > -	if (strstr(s->sym + 1, "_compiled."))
> > +	if (strstr(s->sym + offset, "_compiled."))
> >  		return 0;
> >  
> >  	for (i = 0; special_symbols[i]; i++)
> > -		if( strcmp(s->sym + 1, special_symbols[i]) == 0 )
> > +		if( strcmp(s->sym + offset, special_symbols[i]) == 0 )
> >  			return 0;
> >  
> >  	return 1;
> > @@ -225,9 +237,15 @@
> 
> >  [...]
> >  
> >  /* uncompress a compressed symbol. When this function is called, the best table
> > @@ -665,6 +683,13 @@
> >  
> >  	insert_real_symbols_in_table();
> >  
> > +	/* When valid symbol is not registered, exit to error */
> > +	if (good_head.left == good_head.right &&
> > +	    bad_head.left == bad_head.right) {
> > +		fprintf(stderr, "No valid symbol.\n");
> > +		exit(1);
> > +	}
> > +
> >  	optimize_result();
> >  }
> 
> This should only trigger if there are no symbols at all, or if there are 
> some symbols that are considered invalid, and do not go into the final 
> result.
> 
> Maybe we should just do a return here instead of exit, so that even if 
> this happens, kallsyms will still produce an empty result, that will at 
> least allow the kernel to compile.
> 
> It should give the error output to warn the user that there is something 
> fishy, nevertheless. Maybe even a bigger message, since this should not 
> happen at all, and if this triggers it means that something is seriously 
> wrong.

Not surely possible normally.
But raise SEGV if there is not a check. I do not think that this is good action.
 
> > @@ -672,9 +697,21 @@
> >  int
> >  main(int argc, char **argv)
> >  {
> > -	if (argc == 2 && strcmp(argv[1], "--all-symbols") == 0)
> > -		all_symbols = 1;
> > -	else if (argc != 1)
> > +	if (argc >= 2) {
> 
> This test is unnecessary.
> 
> > +		int i;
> > +		for (i = 1; i < argc; i++) { 
> > +			if(strcmp(argv[i], "--all-symbols") == 0)
> > +				all_symbols = 1;
> > +			else if (strncmp(argv[i], "--symbol-prefix=", 16) == 0) {
> > +				char *p = &argv[i][16];
> > +				/* skip quote */
> > +				if ((*p == '"' && *(p+2) == '"') || (*p == '\'' && *(p+2) == '\''))
> > +					p++;
> > +				symbol_prefix_char = *p;
> > +			} else
> > +				usage();
> > +		}
> > +	} else if (argc != 1)
> >  		usage();
> 
> and so is this.
> 
> >  
> >  	read_map(stdin);
> > @@ -683,4 +720,3 @@
> >  
> >  	return 0;
> >  }
> 
> At least the patch seems to not affect architectures that don't use the 
> "--symbol-prefix" option, so it should be harmless for most.

I think of the same thread which are an issue with it being worked if do not appoint "--symbol-prefix".
Use CONFIG_ARCH and may judge it, but will it be good to refer to include/linux here?
 
> Anyway, appart from the few comments, it has my acknowledge.
> 
> -- 
> Paulo Marques - www.grupopie.com
> 
> All that is necessary for the triumph of evil is that good men do nothing.
> Edmund Burke (1729 - 1797)

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>