[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1 1/5] xsplice: Design document.



>>> On 16.09.15 at 23:01, <konrad.wilk@xxxxxxxxxx> wrote:
> +### Symbol names
> +
> +
> +Xen as it is now, has a couple of non-unique symbol names which will
> +make runtime symbol identification hard.  Sometimes, static symbols
> +simply have the same name in C files, sometimes such symbols get
> +included via header files, and some C files are also compiled
> +multiple times and linked under different names (guest_walk.c).
> +
> +As such we need to modify the linker to make sure that the symbol
> +table qualifies also symbols by their source file name.
> +
> +For the awkward situations in which C-files are compiled multiple
> +times patches we would need to some modification in the Xen code.
> +
> +
> +The convention for file-type symbols (that would allow to map many
> +symbols to their compilation unit) says that only the basename (i.e.,
> +without directories) is embedded.  This creates another layer of
> +confusion for duplicate file names in the build tree.
> +
> +That would have to be resolved.

So here's a draft (some debugging code left and not yet tested
on a really old tool chain) patch doing the prefixing. I also have
9 more patches on top of this, most dealing with individual
symbols that are still left as duplicates (some also do other
cleanup I no longer directly need with the approach now taken).
Sadly the set of duplicates depends on the compiler version in
some cases, hence at the end of the full current series there
are still some duplicate symbol warnings left. I think we can live
with those though for the moment.

Jan

TODO: remove //temp-s

Note: Not warning about duplicate symbols in the EFI case for now, as
a binutils bug causes misnamed file name entries to appear in EFI
binaries' symbol tables when the file name is longer than 18 chars.
(Not doing so also avoids other duplicates getting printed twice.)

--- unstable.orig/xen/arch/x86/Makefile
+++ unstable/xen/arch/x86/Makefile
@@ -106,11 +106,13 @@ $(BASEDIR)/common/symbols-dummy.o:
 $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
        $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
            $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
-       $(NM) -n $(@D)/.$(@F).0 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).0.S
+       $(NM) -pa --format=sysv $(@D)/.$(@F).0 \
+               | $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S
        $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
        $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
            $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
-       $(NM) -n $(@D)/.$(@F).1 | $(BASEDIR)/tools/symbols >$(@D)/.$(@F).1.S
+       $(NM) -pa --format=sysv $(@D)/.$(@F).1 \
+               | $(BASEDIR)/tools/symbols --sysv --sort --warn-dup 
>$(@D)/.$(@F).1.S
        $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
        $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
            $(@D)/.$(@F).1.o -o $@
@@ -133,13 +135,15 @@ $(TARGET).efi: prelink-efi.o efi.lds efi
                  $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< 
efi/relocs-dummy.o \
                        $(BASEDIR)/common/symbols-dummy.o -o 
$(@D)/.$(@F).$(base).0 &&) :
        $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) 
$(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S
-       $(guard) $(NM) -n $(@D)/.$(@F).$(VIRT_BASE).0 | $(guard) 
$(BASEDIR)/tools/symbols >$(@D)/.$(@F).0s.S
+       $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \
+               | $(guard) $(BASEDIR)/tools/symbols --sysv --sort 
>$(@D)/.$(@F).0s.S
        $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o 
$(@D)/.$(@F).0s.o
        $(foreach base, $(VIRT_BASE) $(ALT_BASE), \
                  $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \
                        $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o -o 
$(@D)/.$(@F).$(base).1 &&) :
        $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) 
$(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S
-       $(guard) $(NM) -n $(@D)/.$(@F).$(VIRT_BASE).1 | $(guard) 
$(BASEDIR)/tools/symbols >$(@D)/.$(@F).1s.S
+       $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \
+               | $(guard) $(BASEDIR)/tools/symbols --sysv --sort 
>$(@D)/.$(@F).1s.S
        $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o 
$(@D)/.$(@F).1s.o
        $(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \
                        $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o -o $@
--- unstable.orig/xen/arch/x86/time.c
+++ unstable/xen/arch/x86/time.c
@@ -2059,6 +2059,7 @@ static struct keyhandler dump_softtsc_ke
 static int __init setup_dump_softtsc(void)
 {
     register_keyhandler('s', &dump_softtsc_keyhandler);
+dump_execution_state();//temp
     return 0;
 }
 __initcall(setup_dump_softtsc);
--- unstable.orig/xen/tools/symbols.c
+++ unstable/xen/tools/symbols.c
@@ -30,6 +30,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdint.h>
+#include <stdbool.h>
 #include <ctype.h>
 
 #define KSYM_NAME_LEN          127
@@ -40,13 +41,14 @@ struct sym_entry {
        unsigned int len;
        unsigned char *sym;
 };
-
+#define SYMBOL_NAME(s) ((char *)(s)->sym + 1)
 
 static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
 static unsigned long long _stext, _etext, _sinittext, _einittext, _sextratext, 
_eextratext;
 static int all_symbols = 0;
 static char symbol_prefix_char = '\0';
+static enum { fmt_bsd, fmt_sysv } input_format;
 
 int token_profit[0x10000];
 
@@ -73,11 +75,25 @@ static inline int is_arm_mapping_symbol(
 
 static int read_symbol(FILE *in, struct sym_entry *s)
 {
-       char str[500];
+       char str[500], type[20] = "";
        char *sym, stype;
-       int rc;
-
-       rc = fscanf(in, "%llx %c %499s\n", &s->addr, &stype, str);
+       static enum { symbol, source_file, object_file } last;
+       static char *filename;
+       int rc = -1;
+
+       switch (input_format) {
+       case fmt_bsd:
+               rc = fscanf(in, "%llx %c %499s\n", &s->addr, &stype, str);
+               break;
+       case fmt_sysv:
+               while (fscanf(in, "\n") == 1)
+                       /* nothing */;
+               rc = fscanf(in, "%499[^ |] |%llx | %c |",
+                           str, &s->addr, &stype);
+               if (rc == 3 && fscanf(in, " %19[^ |] |", type) != 1)
+                       *type = '\0';
+               break;
+       }
        if (rc != 3) {
                if (rc != EOF) {
                        /* skip line */
@@ -87,6 +103,28 @@ static int read_symbol(FILE *in, struct 
                return -1;
        }
 
+       sym = strrchr(str, '.');
+       if (strcasecmp(type, "FILE") == 0 ||
+           (/* GNU nm prior to XXX doesn't produce a type for EFI binaries. */
+            input_format == fmt_sysv && !*type && stype == '?' && sym &&
+            sym[1] && strchr("cSsoh", sym[1]) && !sym[2])) {
+               /*
+                * gas prior to XXX outputs symbol table entries resulting
+                * from .file in reverse order. If we get two consecutive file
+                * symbols, prefer the first one if that names an object file
+                * (to cover multiply compiled files).
+                */
+               if ((sym && sym[1] == 'o') || last != object_file) {
+                       free(filename);
+                       filename = *str ? strdup(str) : NULL;
+               }
+               last = !sym || sym[1] != 'o' ? source_file : object_file;
+               goto skip_tail;
+       }
+
+       last = symbol;
+       rc = -1;
+
        sym = str;
        /* skip prefix char */
        if (symbol_prefix_char && str[0] == symbol_prefix_char)
@@ -109,24 +147,38 @@ static int read_symbol(FILE *in, struct 
        {
                /* Keep these useful absolute symbols */
                if (strcmp(sym, "__gp"))
-                       return -1;
-
+                       goto skip_tail;
        }
        else if (toupper((uint8_t)stype) == 'U' ||
+                toupper((uint8_t)stype) == 'N' ||
                 is_arm_mapping_symbol(sym))
-               return -1;
+               goto skip_tail;
        /* exclude also MIPS ELF local symbols ($L123 instead of .L123) */
        else if (str[0] == '$')
-               return -1;
+               goto skip_tail;
 
        /* include the type field in the symbol name, so that it gets
         * compressed together */
        s->len = strlen(str) + 1;
+       if (islower(stype) && filename)
+               s->len += strlen(filename) + 1;
        s->sym = malloc(s->len + 1);
-       strcpy((char *)s->sym + 1, str);
+       sym = SYMBOL_NAME(s);
+       if (islower(stype) && filename) {
+               sym = stpcpy(sym, filename);
+               *sym++ = '#';
+       }
+       strcpy(sym, str);
        s->sym[0] = stype;
+//if(input_format == fmt_sysv) fprintf(stderr, "%c: %s %llx\n", s->sym[0], 
s->sym + 1, s->addr);//temp
 
-       return 0;
+       rc = 0;
+
+ skip_tail:
+       if (input_format == fmt_sysv)
+               fgets(str, 500, in); /* discard rest of line */
+
+       return rc;
 }
 
 static int symbol_valid(struct sym_entry *s)
@@ -460,11 +512,37 @@ static void optimize_token_table(void)
        optimize_result();
 }
 
+static int compare_value(const void *p1, const void *p2)
+{
+       const struct sym_entry *sym1 = p1;
+       const struct sym_entry *sym2 = p2;
+
+       if (sym1->addr < sym2->addr)
+               return -1;
+       if (sym1->addr > sym2->addr)
+               return +1;
+       /* Prefer global symbols. */
+       if (isupper(*sym1->sym))
+               return -1;
+       if (isupper(*sym2->sym))
+               return +1;
+       return 0;
+}
+
+static int compare_name(const void *p1, const void *p2)
+{
+       const struct sym_entry *sym1 = p1;
+       const struct sym_entry *sym2 = p2;
+
+       return strcmp(SYMBOL_NAME(sym1), SYMBOL_NAME(sym2));
+}
 
 int main(int argc, char **argv)
 {
+       unsigned int i;
+       bool unsorted = false, warn_dup = false;
+
        if (argc >= 2) {
-               int i;
                for (i = 1; i < argc; i++) {
                        if(strcmp(argv[i], "--all-symbols") == 0)
                                all_symbols = 1;
@@ -474,13 +552,36 @@ int main(int argc, char **argv)
                                if ((*p == '"' && *(p+2) == '"') || (*p == '\'' 
&& *(p+2) == '\''))
                                        p++;
                                symbol_prefix_char = *p;
-                       } else
+                       } else if (strcmp(argv[i], "--sysv") == 0)
+                               input_format = fmt_sysv;
+                       else if (strcmp(argv[i], "--sort") == 0)
+                               unsorted = true;
+                       else if (strcmp(argv[i], "--warn-dup") == 0)
+                               warn_dup = true;
+                       else
                                usage();
                }
        } else if (argc != 1)
                usage();
 
        read_map(stdin);
+
+       if (warn_dup) {
+               qsort(table, table_cnt, sizeof(*table), compare_name);
+               for (i = 1; i < table_cnt; ++i)
+                       if (strcmp(SYMBOL_NAME(table + i - 1),
+                                  SYMBOL_NAME(table + i)) == 0 &&
+                           table[i - 1].addr != table[i].addr)
+                               fprintf(stderr,
+                                       "Duplicate symbol '%s' (%llx != 
%llx)\n",
+                                       SYMBOL_NAME(table + i),
+                                       table[i].addr, table[i - 1].addr);
+               unsorted = true;
+       }
+
+       if (unsorted)
+               qsort(table, table_cnt, sizeof(*table), compare_value);
+
        optimize_token_table();
        write_src();
 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.