Re: [PATCH v1 0/6] Simplify linking against tools/perf code

[Adrian Hunter <adrian.hunter@intel.com>]

On 28/03/23 19:14, Ian Rogers wrote:
> On Tue, Mar 28, 2023 at 6:24 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 28/03/23 04:40, Ian Rogers wrote:
>>> When fuzzing something like parse-events, having the main function in
>>> perf.c alongside global variables like input_name means that
>>> input_name must be redeclared with the fuzzer function's
>>> main. However, as the fuzzer is using the tools/perf code as a library
>>> this causes backward linking reference that the linker may warn
>>> about. Reorganize perf.c and perf.h to avoid potential backward
>>> references, or so that the declaration/definition locations are more
>>> consistent.
>>>
>>
>> Seems like it could be a pain to maintain.
>>
>> Did you consider just adding:
>>
>> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>> index 82bbe0ca858b..a75dd47d68ee 100644
>> --- a/tools/perf/perf.c
>> +++ b/tools/perf/perf.c
>> @@ -456,6 +456,7 @@ static int libperf_print(enum libperf_print_level level,
>>         return veprintf(level, verbose, fmt, ap);
>>  }
>>
>> +#ifndef CUSTOM_MAIN
>>  int main(int argc, const char **argv)
>>  {
>>         int err;
>> @@ -576,3 +577,4 @@ int main(int argc, const char **argv)
>>  out:
>>         return 1;
>>  }
>> +#endif
>>
> 
> It's possible. Would need to make the static functions not warn about
> being declared and not used. I still think that just aligning
> definitions and declarations yields the most expected code and will
> lead to fewer problems in the long run.

Making perf source dependent on an unknown derivative makes
things more complicated.

If you are not going to contribute it to perf, then a
suggestion is along the lines of the following:

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 82bbe0ca858b..6a7fe1534664 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -456,7 +456,18 @@ static int libperf_print(enum libperf_print_level level,
 	return veprintf(level, verbose, fmt, ap);
 }
 
$ git diff
+#ifdef CUSTOM_MAIN
+int main(void)
+{
+	printf("This is not perf\n");
+	return 0;
+}
+
+int perf_main(int argc, const char **argv);
+int perf_main(int argc, const char **argv)
+#else
 int main(int argc, const char **argv)
+#endif
 {
 	int err;
 	const char *cmd;
$ make EXTRA_CFLAGS="-DCUSTOM_MAIN" NO_BPF_SKEL=1 -C tools/perf >/dev/null
Warning: Kernel ABI header at 'tools/include/uapi/linux/in.h' differs from latest version at 'include/uapi/linux/in.h'
Warning: Kernel ABI header at 'tools/arch/x86/include/asm/cpufeatures.h' differs from latest version at 'arch/x86/include/asm/cpufeatures.h'
Warning: Kernel ABI header at 'tools/arch/arm64/include/uapi/asm/perf_regs.h' differs from latest version at 'arch/arm64/include/uapi/asm/perf_regs.h'
Warning: Kernel ABI header at 'tools/include/linux/coresight-pmu.h' differs from latest version at 'include/linux/coresight-pmu.h'
Makefile.config:587: No sys/sdt.h found, no SDT events are defined, please install systemtap-sdt-devel or systemtap-sdt-dev
Makefile.config:805: Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libperl-dev
Makefile.config:1046: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev
Makefile.config:1075: No alternatives command found, you need to set JDIR= to point to the root of your Java directory
Makefile.config:1137: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
$ tools/perf/perf version
This is not perf