Re: [PATCH v2] tools lib traceevent: Robustify do_generate_dynamic_list_file

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Feb 08, 2017 at 09:44:50AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Feb 07, 2017 at 09:28:40PM -0800, David Carrillo-Cisneros escreveu:
> > v2: Accept "W" and "w" symbol options.
> > 
> > The dynamic-list-file used to export dynamic symbols introduced in
> > 
> > commit e3d09ec8126f ("tools lib traceevent: Export dynamic symbols
> > used by traceevent plugins")
> > 
> > is generated without any sort of error checking.
> > 
> > I experienced problems due to an old version of nm (v 0.158) that outputs
> > in a format distinct from the assumed by the script.
> > 
> > Robustify the built of dynamic symbol list  by enforcing that the second
> > column of $(NM) -u <files> is either "U" (Undefined), "W" or "w" (undefined
> > weak), which are the possible outputs from non-ancient $(NM) versions.
> > Print an error if format is unexpected.
> 
> The problem persists, now with a slightly different message, including
> your latest effort at weak symbols, that looked unrelated at first:
> 
>   GEN      /tmp/build/perf/pmu-events/pmu-events.c
> /bin/sh: 1: [: U W w: unexpected operator
> Either missing one of [ /tmp/build/perf/plugin_jbd2.so /tmp/build/perf/plugin_hrtimer.so /tmp/build/perf/plugin_kmem.so /tmp/build/perf/plugin_kvm.so /tmp/build/perf/plugin_mac80211.so /tmp/build/perf/plugin_sched_switch.so /tmp/build/perf/plugin_function.so /tmp/build/perf/plugin_xen.so /tmp/build/perf/plugin_scsi.so /tmp/build/perf/plugin_cfg80211.so] or bad version of nm
>   CC       /tmp/build/perf/ui/gtk/hists.o

So, after I applied this patch:

---------------------
diff --git a/tools/lib/traceevent/Makefile b/tools/lib/traceevent/Makefile
index a7046ec01a5b..47076b15eebe 100644
--- a/tools/lib/traceevent/Makefile
+++ b/tools/lib/traceevent/Makefile
@@ -259,7 +259,7 @@ endef
 define do_generate_dynamic_list_file
 	symbol_type=`$(NM) -u -D $1 | awk 'NF>1 {print $$1}' | \
 	xargs echo "U W w" | tr ' ' '\n' | sort -u | xargs echo`;\
-	if [ "$$symbol_type" == "U W w" ];then				\
+	if [ "$$symbol_type" = "U W w" ];then				\
 		(echo '{';						\
 		$(NM) -u -D $1 | awk 'NF>1 {print "\t"$$2";"}' | sort -u;\
 		echo '};';						\
---------------------

It works.

Look at the file produced by that:

In Ubuntu 16.10:

perfbuilder@341e71da5019:/git/linux$ cat /tmp/build/perf/libtraceevent-dynamic-list
{
	__stack_chk_fail;
	free;
	memset;
	pevent_find_any_field;
	pevent_find_field;
	pevent_find_function;
	pevent_find_function_address;
	pevent_get_field_raw;
	pevent_get_field_val;
	pevent_print_func_field;
	pevent_print_num_field;
	pevent_read_number_field;
	pevent_register_comm;
	pevent_register_event_handler;
	pevent_register_print_function;
	pevent_unregister_event_handler;
	pevent_unregister_print_function;
	realloc;
	sprintf;
	strcmp;
	strdup;
	strncmp;
	trace_seq_printf;
	trace_seq_putc;
	trace_seq_puts;
	trace_seq_terminate;
	traceevent_plugin_add_options;
	traceevent_plugin_remove_options;
	warning;
};
perfbuilder@341e71da5019:/git/linux$

And in fedora 25, with or without this patch, i.e. with = or ==:

[acme@jouet linux]$ cat /tmp/build/perf/libtraceevent-dynamic-list
{
	free;
	memset;
	pevent_find_any_field;
	pevent_find_field;
	pevent_find_function;
	pevent_find_function_address;
	pevent_get_field_raw;
	pevent_get_field_val;
	pevent_print_func_field;
	pevent_print_num_field;
	pevent_read_number_field;
	pevent_register_comm;
	pevent_register_event_handler;
	pevent_register_print_function;
	pevent_unregister_event_handler;
	pevent_unregister_print_function;
	realloc;
	sprintf;
	strcmp;
	strdup;
	strncmp;
	trace_seq_printf;
	trace_seq_putc;
	trace_seq_puts;
	trace_seq_terminate;
	traceevent_plugin_add_options;
	traceevent_plugin_remove_options;
	warning;
};
[acme@jouet linux]$ 

Then try to find the '==' operator on 'man test'. Albeit command line tests,
outside the Makefile shows it works, it is not documented there, just "STRING =
STRING". Some subtlety:

Fedora 25:

[acme@jouet linux]$ symbol_type="U W w"
[acme@jouet linux]$ [ "$symbol_type" == "U W w" ] && echo matches
matches
[acme@jouet linux]$ [ "$symbol_type" == "U W w A" ] && echo matches
[acme@jouet linux]$ [ "$symbol_type" = "U W w" ] && echo matches
matches
[acme@jouet linux]$ [ "$symbol_type" = "U W w A" ] && echo matches
[acme@jouet linux]$

[acme@jouet linux]$ rpm -qf /bin/\[ 
coreutils-8.25-15.fc25.x86_64
[acme@jouet linux]$ rpm -q bash
bash-4.3.43-4.fc25.x86_64

Ubuntu 16.10:

perfbuilder@341e71da5019:/git/linux$ echo $symbol_type
U W w
perfbuilder@341e71da5019:/git/linux$ [ "$symbol_type" == "U W w" ] && echo matches
matches
perfbuilder@341e71da5019:/git/linux$ [ "$symbol_type" == "U W w AA" ] && echo matches
perfbuilder@341e71da5019:/git/linux$ [ "$symbol_type" = "U W w" ] && echo matches
matches
perfbuilder@341e71da5019:/git/linux$ [ "$symbol_type" = "U W w AA" ] && echo matches
perfbuilder@341e71da5019:/git/linux$ 

perfbuilder@341e71da5019:/git/linux$ ls -la /bin/\[
ls: cannot access '/bin/[': No such file or directory
perfbuilder@341e71da5019:/git/linux$ ls -la /bin/[
ls: cannot access '/bin/[': No such file or directory
perfbuilder@341e71da5019:/git/linux$
perfbuilder@341e71da5019:/git/linux$ dpkg -l | grep coreutils
ii  coreutils                          8.25-2ubuntu2                  amd64        GNU core utilities
perfbuilder@341e71da5019:/git/linux$ dpkg -l | grep bash
ii  bash                               4.3-15ubuntu1                  amd64        GNU Bourne Again SHell


bash has some shell opts to control some aspects of compatibility with string
comparisions that could have some default different from ubuntu and fedora,
I couldn't find one that looked related, sigh.

Anyway, the difference in the order of the symbol types is related to locales, which
are fixed in the Makefiles to avoid those problems, so in both Fedora and Ubuntu
that symbol_type evaluates to "U W w" and the file is generated as intended.

So I'm just replacing == with = and running a new set of container based distro tests.

- Arnaldo