Skip to contentSkip to conversation

tools: Fix rtla and rv problems (found) with clang

PATCH In review Daniel Bristot de Oliveira 2 participants 9 messages Feb 6, 2024 – Feb 8, 2024

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover tools: Fix rtla and rv problems (found) with clang

RHEL people reported some errors when compiling rtla and rv with clang. The command line used to compile the tools is:

$ make HOSTCC=clang CC=clang LLVM_IAS=1

The first problem is two unsupported flags passed to the compiler:
-ffat-lto-objects and -Wno-maybe-uninitialized. They will be removed if the compile is clang.

Also, the clang linker does not automatically recognize the -flto=auto option used at compilation time, so it is explicitly set.

With the compiler working, it starts pointing to some warnings and errors about uninitialized variables, variable size, and an unused function. These problems are also fixed.

Daniel Bristot de Oliveira (6):
tools/rtla: Fix Makefile compiler options for clang tools/rtla: Fix uninitialized bucket/data->bucket_size warning tools/rtla: Fix clang warning about mount_point var size tools/rtla: Remove unused sched_getattr() function tools/rv: Fix Makefile compiler options for clang tools/rv: Fix curr_reactor uninitialized variable

 tools/tracing/rtla/Makefile            | 7 ++++++-
 tools/tracing/rtla/src/osnoise_hist.c  | 3 +--
 tools/tracing/rtla/src/timerlat_hist.c | 3 +--
 tools/tracing/rtla/src/utils.c         | 8 +-------
 tools/verification/rv/Makefile         | 7 ++++++-
 tools/verification/rv/src/in_kernel.c  | 2 +-
 6 files changed, 16 insertions(+), 14 deletions(-)
Signature · 1 line
2.43.0
#1 lore raw

PATCH 1/6 tools/rtla: Fix Makefile compiler options for clang

The following errors are showing up when compiling rtla with clang:

 $ make HOSTCC=clang CC=clang LLVM_IAS=1
 [...]

clang -O -g -DVERSION=\"6.8.0-rc1\" -flto=auto -ffat-lto-objects

	-fexceptions -fstack-protector-strong
	-fasynchronous-unwind-tables -fstack-clash-protection  -Wall
	-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
	-Wp,-D_GLIBCXX_ASSERTIONS -Wno-maybe-uninitialized
	$(pkg-config --cflags libtracefs)    -c -o src/utils.o src/utils.c

clang: warning: optimization flag '-ffat-lto-objects' is not supported [-Wignored-optimization-argument]
warning: unknown warning option '-Wno-maybe-uninitialized'; did you mean '-Wno-uninitialized'? [-Wunknown-warning-option] 1 warning generated.

  clang -o rtla -ggdb  src/osnoise.o src/osnoise_hist.o src/osnoise_top.o
  src/rtla.o src/timerlat_aa.o src/timerlat.o src/timerlat_hist.o
  src/timerlat_top.o src/timerlat_u.o src/trace.o src/utils.o $(pkg-config --libs libtracefs)

src/osnoise.o: file not recognized: file format not recognized
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile:110: rtla] Error 1

Solve these issues by:
- removing -ffat-lto-objects and -Wno-maybe-uninitialized if using clang
- informing the linker about -flto=auto

Fixes 1a7b22ab15eb ("tools/rtla: Build with EXTRA_{C,LD}FLAGS")
---
 tools/tracing/rtla/Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
tools/tracing/rtla/Makefile +6 -1
index 2456a399eb9a..afd18c678ff5 100644
@@ -28,10 +28,15 @@ FOPTS	:=	-flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong \
 		-fasynchronous-unwind-tables -fstack-clash-protection
 WOPTS	:= 	-Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -Wno-maybe-uninitialized
 
+ifeq ($(CC),clang)
+  FOPTS := $(filter-out -ffat-lto-objects, $(FOPTS))
+  WOPTS := $(filter-out -Wno-maybe-uninitialized, $(WOPTS))
+endif
+
 TRACEFS_HEADERS	:= $$($(PKG_CONFIG) --cflags libtracefs)
 
 CFLAGS	:=	-O -g -DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(TRACEFS_HEADERS) $(EXTRA_CFLAGS)
-LDFLAGS	:=	-ggdb $(EXTRA_LDFLAGS)
+LDFLAGS	:=	-flto=auto -ggdb $(EXTRA_LDFLAGS)
 LIBS	:=	$$($(PKG_CONFIG) --libs libtracefs)
 
 SRC	:=	$(wildcard src/*.c)
Signature · 1 line
2.43.0
#2 lore raw

Re: [PATCH 1/6] tools/rtla: Fix Makefile compiler options for clang

Quoted 5 lines · Daniel Bristot de Oliveira

The following errors are showing up when compiling rtla with clang:

 $ make HOSTCC=clang CC=clang LLVM_IAS=1
 [...]
Quoted 8 lines

clang -O -g -DVERSION=\"6.8.0-rc1\" -flto=auto -ffat-lto-objects

	-fexceptions -fstack-protector-strong
	-fasynchronous-unwind-tables -fstack-clash-protection  -Wall
	-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
	-Wp,-D_GLIBCXX_ASSERTIONS -Wno-maybe-uninitialized
	$(pkg-config --cflags libtracefs)    -c -o src/utils.o src/utils.c

clang: warning: optimization flag '-ffat-lto-objects' is not supported [-Wignored-optimization-argument]

For what it's worth, this flag is supported in clang 17.0.0 and newer:

https://github.com/llvm/llvm-project/commit/610fc5cbcc8b68879c562f6458608afe2473ab7f

But if it is not critical, just dropping the flag like you have done here rather than conditionally supporting it is probably easier.

Quoted 47 lines

warning: unknown warning option '-Wno-maybe-uninitialized'; did you mean '-Wno-uninitialized'? [-Wunknown-warning-option] 1 warning generated.

  clang -o rtla -ggdb  src/osnoise.o src/osnoise_hist.o src/osnoise_top.o
  src/rtla.o src/timerlat_aa.o src/timerlat.o src/timerlat_hist.o
  src/timerlat_top.o src/timerlat_u.o src/trace.o src/utils.o $(pkg-config --libs libtracefs)

src/osnoise.o: file not recognized: file format not recognized
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile:110: rtla] Error 1

Solve these issues by:
- removing -ffat-lto-objects and -Wno-maybe-uninitialized if using clang
- informing the linker about -flto=auto

Fixes 1a7b22ab15eb ("tools/rtla: Build with EXTRA_{C,LD}FLAGS")
---
 tools/tracing/rtla/Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
tools/tracing/rtla/Makefile +6 -1
index 2456a399eb9a..afd18c678ff5 100644
@@ -28,10 +28,15 @@ FOPTS	:=	-flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong \
 		-fasynchronous-unwind-tables -fstack-clash-protection
 WOPTS	:= 	-Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -Wno-maybe-uninitialized
 
+ifeq ($(CC),clang)
+  FOPTS := $(filter-out -ffat-lto-objects, $(FOPTS))
+  WOPTS := $(filter-out -Wno-maybe-uninitialized, $(WOPTS))
+endif
+
 TRACEFS_HEADERS	:= $$($(PKG_CONFIG) --cflags libtracefs)
 
 CFLAGS	:=	-O -g -DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(TRACEFS_HEADERS) $(EXTRA_CFLAGS)
-LDFLAGS	:=	-ggdb $(EXTRA_LDFLAGS)
+LDFLAGS	:=	-flto=auto -ggdb $(EXTRA_LDFLAGS)
 LIBS	:=	$$($(PKG_CONFIG) --libs libtracefs)
 
 SRC	:=	$(wildcard src/*.c)
Signature · 1 line
2.43.0
#3 lore raw

Re: [PATCH 1/6] tools/rtla: Fix Makefile compiler options for clang

Quoted 13 lines · Nathan Chancellor
Quoted 12 lines · Daniel Bristot de Oliveira

The following errors are showing up when compiling rtla with clang:

 $ make HOSTCC=clang CC=clang LLVM_IAS=1
 [...]

clang -O -g -DVERSION=\"6.8.0-rc1\" -flto=auto -ffat-lto-objects

	-fexceptions -fstack-protector-strong
	-fasynchronous-unwind-tables -fstack-clash-protection  -Wall
	-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
	-Wp,-D_GLIBCXX_ASSERTIONS -Wno-maybe-uninitialized
	$(pkg-config --cflags libtracefs)    -c -o src/utils.o src/utils.c
Quoted 5 lines
Quoted 1 line

clang: warning: optimization flag '-ffat-lto-objects' is not supported [-Wignored-optimization-argument]

For what it's worth, this flag is supported in clang 17.0.0 and newer:

https://github.com/llvm/llvm-project/commit/610fc5cbcc8b68879c562f6458608afe2473ab7f

Good! still, I am getting this error on fedora, with this clang version:

bristot@x1:~/src/git/linux/tools/tracing/rtla$ clang --version
clang version 17.0.6 (Fedora 17.0.6-1.fc39)
Target: x86_64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

:-(

Quoted 2 lines

But if it is not critical, just dropping the flag like you have done here rather than conditionally supporting it is probably easier.

Yeah, I will drop it for now, and keep monitoring.

Thanks Natan!
-- Daniel

#4 lore raw

PATCH 2/6 tools/rtla: Fix uninitialized bucket/data->bucket_size warning

In reply to Daniel Bristot de Oliveira

When compiling rtla with clang, I am getting the following warnings:

$ make HOSTCC=clang CC=clang LLVM_IAS=1

[..]
clang -O -g -DVERSION=\"6.8.0-rc3\" -flto=auto -fexceptions
-fstack-protector-strong -fasynchronous-unwind-tables
-fstack-clash-protection -Wall -Werror=format-security
-Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS
$(pkg-config --cflags libtracefs)
-c -o src/osnoise_hist.o src/osnoise_hist.c
src/osnoise_hist.c:138:6: warning: variable 'bucket' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
138 | if (data->bucket_size)

      |             ^~~~~~~~~~~~~~~~~
src/osnoise_hist.c:149:6: note: uninitialized use occurs here
  149 |         if (bucket < entries)
      |             ^~~~~~
src/osnoise_hist.c:138:2: note: remove the 'if' if its condition is always true
  138 |         if (data->bucket_size)
      |         ^~~~~~~~~~~~~~~~~~~~~~
  139 |                 bucket = duration / data->bucket_size;
src/osnoise_hist.c:132:12: note: initialize the variable 'bucket' to silence this warning
  132 |         int bucket;
      |                   ^
      |                    = 0

1 warning generated.

[...]

clang -O -g -DVERSION=\"6.8.0-rc3\" -flto=auto -fexceptions
-fstack-protector-strong -fasynchronous-unwind-tables
-fstack-clash-protection -Wall -Werror=format-security
-Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS
$(pkg-config --cflags libtracefs)
-c -o src/timerlat_hist.o src/timerlat_hist.c
src/timerlat_hist.c:181:6: warning: variable 'bucket' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
181 | if (data->bucket_size)

      |             ^~~~~~~~~~~~~~~~~
src/timerlat_hist.c:204:6: note: uninitialized use occurs here
  204 |         if (bucket < entries)
      |             ^~~~~~
src/timerlat_hist.c:181:2: note: remove the 'if' if its condition is always true
  181 |         if (data->bucket_size)
      |         ^~~~~~~~~~~~~~~~~~~~~~
  182 |                 bucket = latency / data->bucket_size;
src/timerlat_hist.c:175:12: note: initialize the variable 'bucket' to silence this warning
  175 |         int bucket;
      |                   ^
      |                    = 0

1 warning generated.

This is a legit warning, but data->bucket_size is always > 0 (see timerlat_hist_parse_args()), so the if is not necessary.

Remove the unneeded if (data->bucket_size) to avoid the warning.

Fixes 1eeb6328e8b3 ("rtla/timerlat: Add timerlat hist mode")
Fixes 829a6c0b5698 ("rtla/osnoise: Add the hist mode")
---
 tools/tracing/rtla/src/osnoise_hist.c  | 3 +--
 tools/tracing/rtla/src/timerlat_hist.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)
tools/tracing/rtla/src/osnoise_hist.c +1 -2
index 8f81fa007364..67128171c29d 100644
@@ -135,8 +135,7 @@ static void osnoise_hist_update_multiple(struct osnoise_tool *tool, int cpu,
 	if (params->output_divisor)
 		duration = duration / params->output_divisor;
 
-	if (data->bucket_size)
-		bucket = duration / data->bucket_size;
+	bucket = duration / data->bucket_size;
 
 	total_duration = duration * count;
 
tools/tracing/rtla/src/timerlat_hist.c +1 -2
index 47d3d8b53cb2..3a5b8c409e7d 100644
@@ -178,8 +178,7 @@ timerlat_hist_update(struct osnoise_tool *tool, int cpu,
 	if (params->output_divisor)
 		latency = latency / params->output_divisor;
 
-	if (data->bucket_size)
-		bucket = latency / data->bucket_size;
+	bucket = latency / data->bucket_size;
 
 	if (!context) {
 		hist = data->hist[cpu].irq;
Signature · 1 line
2.43.0
#5 lore raw

PATCH 3/6 tools/rtla: Fix clang warning about mount_point var size

In reply to Daniel Bristot de Oliveira

clang is reporting this warning:

$ make HOSTCC=clang CC=clang LLVM_IAS=1
[...]
clang -O -g -DVERSION=\"6.8.0-rc3\" -flto=auto -fexceptions
	-fstack-protector-strong -fasynchronous-unwind-tables
	-fstack-clash-protection  -Wall -Werror=format-security
	-Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS
	$(pkg-config --cflags libtracefs)    -c -o src/utils.o src/utils.c

src/utils.c:548:66: warning: 'fscanf' may overflow; destination buffer in argument 3 has size 1024, but the corresponding specifier may require size 1025 [-Wfortify-source]
548 | while (fscanf(fp, "%*s %" STR(MAX_PATH) "s %99s %*s %*d %*d\n", mount_point, type) == 2) {
| ^

Increase mount_point variable size to MAX_PATH+1 to avoid the overflow.

Fixes a957cbc02531 ("rtla: Add -C cgroup support")
---
 tools/tracing/rtla/src/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/tracing/rtla/src/utils.c +1 -1
index c769d7b3842c..b998b24edf1e 100644
@@ -536,7 +536,7 @@ int set_cpu_dma_latency(int32_t latency)
  */
 static const int find_mount(const char *fs, char *mp, int sizeof_mp)
 {
-	char mount_point[MAX_PATH];
+	char mount_point[MAX_PATH+1];
 	char type[100];
 	int found = 0;
 	FILE *fp;
Signature · 1 line
2.43.0
#6 lore raw

PATCH 4/6 tools/rtla: Remove unused sched_getattr() function

In reply to Daniel Bristot de Oliveira

Clang is reporting:

$ make HOSTCC=clang CC=clang LLVM_IAS=1
[...]
clang -O -g -DVERSION=\"6.8.0-rc3\" -flto=auto -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection  -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS $(pkg-config --cflags libtracefs)    -c -o src/utils.o src/utils.c
src/utils.c:241:19: warning: unused function 'sched_getattr' [-Wunused-function]
  241 | static inline int sched_getattr(pid_t pid, struct sched_attr *attr,
      |                   ^~~~~~~~~~~~~
1 warning generated.

Which is correct, so remove the unused function.

Fixes b1696371d865 ("rtla: Helper functions for rtla")
---
 tools/tracing/rtla/src/utils.c | 6 ------
 1 file changed, 6 deletions(-)
tools/tracing/rtla/src/utils.c +0 -6
index b998b24edf1e..5fcd6495ff05 100644
@@ -238,12 +238,6 @@ static inline int sched_setattr(pid_t pid, const struct sched_attr *attr,
 	return syscall(__NR_sched_setattr, pid, attr, flags);
 }
 
-static inline int sched_getattr(pid_t pid, struct sched_attr *attr,
-				unsigned int size, unsigned int flags)
-{
-	return syscall(__NR_sched_getattr, pid, attr, size, flags);
-}
-
 int __set_sched_attr(int pid, struct sched_attr *attr)
 {
 	int flags = 0;
Signature · 1 line
2.43.0
#7 lore raw

PATCH 5/6 tools/rv: Fix Makefile compiler options for clang

In reply to Daniel Bristot de Oliveira

The following errors are showing up when compiling rv with clang:

 $ make HOSTCC=clang CC=clang LLVM_IAS=1
 [...]
  clang -O -g -DVERSION=\"6.8.0-rc1\" -flto=auto -ffat-lto-objects
  -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables
  -fstack-clash-protection  -Wall -Werror=format-security
  -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS
  -Wno-maybe-uninitialized $(pkg-config --cflags libtracefs)
  -I include   -c -o src/utils.o src/utils.c
  clang: warning: optimization flag '-ffat-lto-objects' is not supported [-Wignored-optimization-argument]
  warning: unknown warning option '-Wno-maybe-uninitialized'; did you mean '-Wno-uninitialized'? [-Wunknown-warning-option]
  1 warning generated.

clang -o rv -ggdb src/in_kernel.o src/rv.o src/trace.o src/utils.o $(pkg-config --libs libtracefs) src/in_kernel.o: file not recognized: file format not recognized
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile:110: rv] Error 1

Solve these issues by:
- removing -ffat-lto-objects and -Wno-maybe-uninitialized if using clang
- informing the linker about -flto=auto

Fixes 4bc4b131d44c ("rv: Add rv tool")
---
 tools/verification/rv/Makefile | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
tools/verification/rv/Makefile +6 -1
index 3d0f3888a58c..485f8aeddbe0 100644
@@ -28,10 +28,15 @@ FOPTS	:=	-flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong \
 		-fasynchronous-unwind-tables -fstack-clash-protection
 WOPTS	:= 	-Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -Wno-maybe-uninitialized
 
+ifeq ($(CC),clang)
+  FOPTS := $(filter-out -ffat-lto-objects, $(FOPTS))
+  WOPTS := $(filter-out -Wno-maybe-uninitialized, $(WOPTS))
+endif
+
 TRACEFS_HEADERS	:= $$($(PKG_CONFIG) --cflags libtracefs)
 
 CFLAGS	:=	-O -g -DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(TRACEFS_HEADERS) $(EXTRA_CFLAGS) -I include
-LDFLAGS	:=	-ggdb $(EXTRA_LDFLAGS)
+LDFLAGS	:=	-flto=auto -ggdb $(EXTRA_LDFLAGS)
 LIBS	:=	$$($(PKG_CONFIG) --libs libtracefs)
 
 SRC	:=	$(wildcard src/*.c)
Signature · 1 line
2.43.0
#8 lore raw

PATCH 6/6 tools/rv: Fix curr_reactor uninitialized variable

In reply to Daniel Bristot de Oliveira

clang is reporting:

$ make HOSTCC=clang CC=clang LLVM_IAS=1

clang -O -g -DVERSION=\"6.8.0-rc3\" -flto=auto -fexceptions
-fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS $(pkg-config --cflags libtracefs) -I include -c -o src/in_kernel.o src/in_kernel.c
[...]

src/in_kernel.c:227:6: warning: variable 'curr_reactor' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
227 | if (!end)

      |             ^~~~
src/in_kernel.c:242:9: note: uninitialized use occurs here
  242 |         return curr_reactor;
      |                ^~~~~~~~~~~~
src/in_kernel.c:227:2: note: remove the 'if' if its condition is always false
  227 |         if (!end)
      |         ^~~~~~~~~
  228 |                 goto out_free;
      |                 ~~~~~~~~~~~~~
src/in_kernel.c:221:6: warning: variable 'curr_reactor' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
  221 |         if (!start)
      |             ^~~~~~
src/in_kernel.c:242:9: note: uninitialized use occurs here
  242 |         return curr_reactor;
      |                ^~~~~~~~~~~~
src/in_kernel.c:221:2: note: remove the 'if' if its condition is always false
  221 |         if (!start)
      |         ^~~~~~~~~~~
  222 |                 goto out_free;
      |                 ~~~~~~~~~~~~~
src/in_kernel.c:215:20: note: initialize the variable 'curr_reactor' to silence this warning
  215 |         char *curr_reactor;
      |                           ^
      |                            = NULL

2 warnings generated.

Which is correct. Setting curr_reactor to NULL avoids the problem.

Fixes 6d60f89691fc ("tools/rv: Add in-kernel monitor interface")
---
 tools/verification/rv/src/in_kernel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/verification/rv/src/in_kernel.c +1 -1
index ad28582bcf2b..f04479ecc96c 100644
@@ -210,9 +210,9 @@ static char *ikm_read_reactor(char *monitor_name)
 static char *ikm_get_current_reactor(char *monitor_name)
 {
 	char *reactors = ikm_read_reactor(monitor_name);
+	char *curr_reactor = NULL;
 	char *start;
 	char *end;
-	char *curr_reactor;
 
 	if (!reactors)
 		return NULL;
Signature · 1 line
2.43.0