linux-kernel - Re: [PATCH v4 01/17] selftests/resctrl: Fix compilation issues for global variables

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Jan 2021 18:22:25 -0700
From:   Shuah Khan <skhan@...uxfoundation.org>
To:     Fenghua Yu <fenghua.yu@...el.com>, Shuah Khan <shuah@...nel.org>,
        Tony Luck <tony.luck@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        David Binderman <dcb314@...mail.com>,
        Babu Moger <babu.moger@....com>,
        James Morse <james.morse@....com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        Shuah Khan <skhan@...uxfoundation.org>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 01/17] selftests/resctrl: Fix compilation issues for
 global variables

On 11/30/20 1:19 PM, Fenghua Yu wrote:
> Reinette reported following compilation issue on Fedora 32, gcc version
> 10.1.1
> 
> /usr/bin/ld: cqm_test.o:<src_dir>/cqm_test.c:22: multiple definition of
> `cache_size'; cat_test.o:<src_dir>/cat_test.c:23: first defined here
> 
> The same issue is reported for long_mask, cbm_mask, count_of_bits etc
> variables as well. Compiler isn't happy because these variables are
> defined globally in two .c files namely cqm_test.c and cat_test.c and
> the compiler during compilation finds that the variable is already
> defined (multiple definition error).
> 
> Taking a closer look at the usage of these variables reveals that these
> variables are used only locally to functions such as cqm_resctrl_val()
> (defined in cqm_test.c) and cat_perf_miss_val() (defined in cat_test.c).
> These variables are not shared between those functions. So, there is no
> need for these variables to be global. Hence, fix this issue by making
> them local variables to the functions where they are used.
> 

Easy fix to this problem would be making these variables static to
these files. I am not seeing any real advantage to changing these
variable to local variables.

diff --git a/tools/testing/selftests/resctrl/cat_test.c 
b/tools/testing/selftests/resctrl/cat_test.c
index 5da43767b973..360456b8a1b6 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -17,10 +17,10 @@
  #define MAX_DIFF_PERCENT       4
  #define MAX_DIFF               1000000

-int count_of_bits;
-char cbm_mask[256];
-unsigned long long_mask;
-unsigned long cache_size;
+static int count_of_bits;
+static cbm_mask[256];
+static unsigned long long_mask;
+static unsigned long cache_size;

Same changes made to tools/testing/selftests/resctrl/cqm_test.c

> To fix issues for other global variables (e.g: bm_pid, ppid, llc_occup_path
> and is_amd) that are used across .c files, declare them as extern.
> 

This change is fine. Make this a separate patch.

This way, we can take it as a fixes to stables.

With the above the test builds fine. There is a bigger problem that
needs fixing: (thix might have been fixed in later patches perhaps):

cqm_test.c: In function ‘check_results’:
cqm_test.c:89:9: warning: ‘fgets’ writing 1024 bytes into a region of 
size 512 overflows the destination [-Wstringop-overflow=]
    89 |  while (fgets(temp, 1024, fp)) {
       |         ^~~~~~~~~~~~~~~~~~~~~
In file included from resctrl.h:5,
                  from cqm_test.c:11:
/usr/include/stdio.h:568:14: note: in a call to function ‘fgets’ 
declared with attribute ‘write_only (1, 2)’
   568 | extern char *fgets (char *__restrict __s, int __n, FILE 
*__restrict __stream)
       |              ^~~~~


thanks,
-- Shuah