[XEN,v5,13/16] xen,symbols: rework file symbols selection

Commit Message

Anthony PERARD April 21, 2020, 4:12 p.m. UTC

We want to use the same rune to build mm/*/guest_*.o as the one use to
build every other *.o object. The consequence it that file symbols that
the program ./symbols prefer changes with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y.

For example, when building arch/x86/mm/guest_walk_2.o from guest_walk.c,
this would be the difference of file symbol present in the object when
building with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y:

(1) Currently we have those two file symbols:
    guest_walk.c
    guest_walk_2.o
(2) When building with the same rune, we will have:
    arch/x86/mm/guest_walk.c
    guest_walk_2.o

The order in which those symbols are present may be different. Building
without CONFIG_ENFORCE_UNIQUE_SYMBOLS will result in (1).

Currently, in case (1) ./symbols chooses the *.o symbol (object file
name). But in case (2), may choose the *.c symbol (source file name with
path component) if it is first

We want to have ./symbols choose the object file name symbol in both
cases. So this patch changes that ./symbols prefer the "object file
name" symbol over the "source file name with path component" symbols.

The new intended order of preference is:
    - first object file name symbol (present only in object files
      produced from multiply-compiled sources)
    - first source file name with path components symbol
    - last source file name without any path component symbol

Signed-off-by: Anthony PERARD <[e-mail]>
---

Notes:
    v5:
    - reword part of the commit message
    
    v4:
    - rescope enum symbol_type
    - remove setting values to enums, as it's not needed.
    - rename the enumeration symbols

 xen/tools/symbols.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Jan Beulich April 28, 2020, 2:05 p.m. UTC | #1

On 21.04.2020 18:12, Anthony PERARD wrote:
> We want to use the same rune to build mm/*/guest_*.o as the one use to
> build every other *.o object. The consequence it that file symbols that
> the program ./symbols prefer changes with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y.
> 
> For example, when building arch/x86/mm/guest_walk_2.o from guest_walk.c,
> this would be the difference of file symbol present in the object when
> building with CONFIG_ENFORCE_UNIQUE_SYMBOLS=y:
> 
> (1) Currently we have those two file symbols:
>     guest_walk.c
>     guest_walk_2.o
> (2) When building with the same rune, we will have:
>     arch/x86/mm/guest_walk.c
>     guest_walk_2.o

So I had to go to the v4 discussion to understand this again. As said
there, it may be obvious to you but despite having been the one to
introduce the objcopy step there, it is not to me. Hence my continued
desire to have this at least briefly mentioned here, as it's not
otherwise noticeable from looking at the patch itself.

The code change itself looks okay to me, but I'll want to ack this
change only once I can follow the description from a single pass of
reading through it. I'm sorry for the extra trouble.

Jan

Patch

diff --git a/xen/tools/symbols.c b/xen/tools/symbols.c
index 9f9e2c990061..b3a9465b32d3 100644
--- a/xen/tools/symbols.c
+++ b/xen/tools/symbols.c
@@ -84,7 +84,12 @@  static int read_symbol(FILE *in, struct sym_entry *s)
 {
 	char str[500], type[20] = "";
 	char *sym, stype;
-	static enum { symbol, single_source, multi_source } last;
+	static enum symbol_type {
+		symbol,
+		file_source,
+		path_source,
+		obj_file,
+	} last;
 	static char *filename;
 	int rc = -1;
 
@@ -125,13 +130,20 @@  static int read_symbol(FILE *in, struct sym_entry *s)
 		 * prefer the first one if that names an object file or has a
 		 * directory component (to cover multiply compiled files).
 		 */
-		bool multi = strchr(str, '/') || (sym && sym[1] == 'o');
+		enum symbol_type current;
 
-		if (multi || last != multi_source) {
+		if (sym && sym[1] == 'o')
+		    current = obj_file;
+		else if (strchr(str, '/'))
+		    current = path_source;
+		else
+		    current = file_source;
+
+		if (current > last || last == file_source) {
 			free(filename);
 			filename = *str ? strdup(str) : NULL;
+			last = current;
 		}
-		last = multi ? multi_source : single_source;
 		goto skip_tail;
 	}