Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC 8 format-truncation error #126

Closed
afaerber opened this issue Jun 17, 2018 · 13 comments
Closed

GCC 8 format-truncation error #126

afaerber opened this issue Jun 17, 2018 · 13 comments
Labels

Comments

@afaerber
Copy link

Building v3.1.0 on openSUSE Tumbleweed with its GCC 8.1.1 leads to the following build error:

[   67s] /home/abuild/rpmbuild/BUILD/optee_client-3.1.0/libteec/src/teec_trace.c: In function '_dprintf':
[   67s] /home/abuild/rpmbuild/BUILD/optee_client-3.1.0/libteec/src/teec_trace.c:110:24: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 246 [-Werror=format-truncation=]
[   67s]      "%s [%d] %s:%s:%d: %s",
[   67s]                         ^~
[   67s] /home/abuild/rpmbuild/BUILD/optee_client-3.1.0/libteec/src/teec_trace.c:112:11:
[   67s]      line, raw);
[   67s]            ~~~           
[   67s] In file included from /usr/include/stdio.h:862,
[   67s]                  from /home/abuild/rpmbuild/BUILD/optee_client-3.1.0/libteec/src/teec_trace.c:27:
[   67s] /usr/include/bits/stdio2.h:64:10: note: '__builtin___snprintf_chk' output 11 or more bytes (assuming 266) into a destination of size 256
[   67s]    return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
[   67s]           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[   67s]         __bos (__s), __fmt, __va_arg_pack ());
[   67s]         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[   67s] cc1: all warnings being treated as errors
[   67s] make[2]: *** [libteec/CMakeFiles/teec.dir/build.make:79: libteec/CMakeFiles/teec.dir/src/teec_trace.c.o] Error 1
@afaerber
Copy link
Author

@scarabeusiv

@simonqhughes
Copy link

simonqhughes commented Jun 18, 2018

Hi

This is also an issue for us on our development project, which has broken our CI an resulted in us pinning our repo manifests to get over the problem. Any escalation on priority to fix this would be much appreciated:

> ===================================================================
> 02:48:15 ERROR: Logfile of failure stored in: /work/machine-imx7s-warp-xxx/xxx-manifest/build-xxx/tmp-xxx-glibc/work/cortexa7hf-neon-oe-linux-gnueabi/optee-client/2.6.0+gitAUTOINC+73b4e490a8-r0/temp/log.do_compile.12265
> 02:48:15 Log data follows:
> 02:48:15 | DEBUG: Executing shell function do_compile
> 02:48:15 | NOTE: make -j 24
> 02:48:15 | Building libteec.so
> 02:48:15 |   CC      src/tee_client_api.c
> 02:48:15 |   CC      src/teec_trace.c
> 02:48:15 | src/teec_trace.c: In function '_dprintf':
> 02:48:15 | src/teec_trace.c:110:24: error: '%s' directive output may be truncated writing up to 255 bytes into a region of size 246 [-Werror=format-truncation=]
> 02:48:15 |      "%s [%d] %s:%s:%d: %s",
> 02:48:15 |                         ^~
> 02:48:15 | src/teec_trace.c:112:11:
> 02:48:15 |      line, raw);
> 02:48:15 |            ~~~
> 02:48:15 | src/teec_trace.c:109:3: note: 'snprintf' output 11 or more bytes (assuming 266) into a destination of size 256
> 02:48:15 |    snprintf(prefixed, MAX_PRINT_SIZE,
> 02:48:15 |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 02:48:15 |      "%s [%d] %s:%s:%d: %s",
> 02:48:15 |      ~~~~~~~~~~~~~~~~~~~~~~~
> 02:48:15 |      trace_level_strings[level], thread_id, prefix, func,
> 02:48:15 |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 02:48:15 |      line, raw);
> 02:48:15 |      ~~~~~~~~~~
> 02:48:15 | cc1: all warnings being treated as errors
> 02:48:15 | Makefile:52: recipe for target '/work/machine-imx7s-warp-xxx/xxx-manifest/build-xxx/tmp-xxx-glibc/work/cortexa7hf-neon-oe-linux-gnueabi/optee-client/2.6.0+gitAUTOINC+73b4e490a8-r0/git/libteec/../out/libteec/teec_trace.o' failed
> 02:48:15 | make[1]: *** [/work/machine-imx7s-warp-xxx/xxx-manifest/build-xxx/tmp-xxx-glibc/work/cortexa7hf-neon-oe-linux-gnueabi/optee-client/2.6.0+gitAUTOINC+73b4e490a8-r0/git/libteec/../out/libteec/teec_trace.o] Error 1
> 02:48:15 | make[1]: *** Waiting for unfinished jobs....
> 02:48:15 | Makefile:31: recipe for target 'build-libteec' failed
> 02:48:15 | make: *** [build-libteec] Error 2
> 02:48:15 | ERROR: oe_runmake failed
> 02:48:15 | WARNING: exit code 1 from a shell command.
> 02:48:15 | ERROR: Function failed: do_compile (log file is located at /work/machine-imx7s-warp-xxx/xxx-manifest/build-xxx/tmp-xxx-glibc/work/cortexa7hf-neon-oe-linux-gnueabi/optee-client/2.6.0+gitAUTOINC+73b4e490a8-r0/temp/log.do_compile.12265)
> 02:48:15 NOTE: recipe optee-client-2.6.0+gitAUTOINC+73b4e490a8-r0: task do_compile: Failed
> 02:48:15 NOTE: recipe alsa-state-0.2.0-r5: task do_install: Succeeded
> 02:48:15 ERROR: Task (/work/machine-imx7s-warp-xxx/xxx-manifest/build-xxx/conf/../../layers/meta-linaro/meta-optee/recipes-security/optee/optee-client.bb:do_compile) failed with exit code '1'
> 02:48:15 NOTE: recipe modutils-initscripts-1.0-r7: task do_install: Started
> ===================================================================
>  
> between
> openembedded-core master commit bad: 8ab5b439ea82ac775494a0ce7a6f3615b61c94be
> openembedded-core master commit good:23f15c63777020f5d43b070a1eb2bcf246c19ff8
>  
>  
>  
> Here is the diff between xxx-manifest pinned-manifest.xml's between <bad build and >good build
>  
> simhug01@xxx-test:/mnt/data/default_20180613_122141/xxx-manifest$ cat ../manifest_xml_changes.txt 
> 20,21c20,21
> <   <project name="openembedded/meta-openembedded" path="layers/meta-openembedded" remote="github" revision="d9e257abbe16b9d30171493fa8f1d7e2d24cefe5" upstream="master"/>
> <   <project name="openembedded/openembedded-core" path="layers/openembedded-core" remote="github" revision="8ab5b439ea82ac775494a0ce7a6f3615b61c94be" upstream="master"/>
> ---
> >   <project name="openembedded/meta-openembedded" path="layers/meta-openembedded" remote="github" revision="bb57bac845f3cd1634862fa9868bc8e294ba74a9" upstream="master"/>
> >   <project name="openembedded/openembedded-core" path="layers/openembedded-core" remote="github" revision="23f15c63777020f5d43b070a1eb2bcf246c19ff8" upstream="master"/>
>  
>  
> Other triage information: 
>  
> triage:
> ◾This build succeeded: http://xxxxx/job/xxx-master/559/
> ◾This build failed: http://xxxxx//job/xxx-master/560/
> ◾The only projects that added changes between these to versions are openembedded-core and meta-openembedded (not meta-linaro containing meta-optee which holds the breaking optee-client.bb recipe).
> ◾Pinning oe-core in default.xml manifest to build 559 pin builds successfully (leaving meta-oe at head of master).
> ◾Pinning meta-oe in default manifest to build 559 pin doesn't build successfully (leaving oe-core at head of master).
> ◾Numerous oe-core changes have been added wrt gcc compiler recipes. Likely that a compiler option has been added to treat certain warnings as errors causing the latest break.
> ◾Should notify linaro project of the break and see if they fix it, or whether oe-core changes accommodate (less likely).
>  

@jforissier
Copy link
Contributor

Hi,

I don't have GCC 8.1.1 so I can't reproduce the issue (I'm running Ubuntu 16.04.4). I suspect increasing the size of char prefixed[MAX_PRINT_SIZE] by 11 could very well fix the problem.

@afaerber
Copy link
Author

Why 11? 255-246=9, which appears to assume that all preceding %s are empty and %d is only one char, otherwise it's at least 13 by my count. I rather feared that we may need to use %*s and pass a suitable length < 256-x there?

If needed, a Tumbleweed rootfs for chroot can be found here: https://download.opensuse.org/ports/aarch64/tumbleweed/images/openSUSE-Tumbleweed-ARM-JeOS.aarch64-rootfs.aarch64-Current.tar.xz (it may need a zypper up to update to gcc8)

@jforissier
Copy link
Contributor

I should have said 10 = 266 - 256. Any chance you could try the following patch?

diff --git a/libteec/src/teec_trace.c b/libteec/src/teec_trace.c
index 78b79d6..54a53ec 100644
--- a/libteec/src/teec_trace.c
+++ b/libteec/src/teec_trace.c
@@ -73,7 +73,7 @@ int _dprintf(const char *function, int flen, int line, int level,
             const char *prefix, const char *fmt, ...)
 {
        char raw[MAX_PRINT_SIZE];
-       char prefixed[MAX_PRINT_SIZE];
+       char prefixed[MAX_PRINT_SIZE + 10];
        char *to_print = NULL;
        const char *func;
        int err;

@scarabeusiv
Copy link

That will not fix this error. For our openSUSE purposes the problem is default for Werror, which I just disabled -> solved as warning only for us.

@simonqhughes
Copy link

@scarabeusiv do you have a patch that we could use? We could use it temporarily before (@jforissier) a better fix is tested.

@afaerber
Copy link
Author

@omnium21
Copy link

@jforissier, I tried the patch and it didn't work, because the old length was still being passed into snprintf. so this patch worked:

diff --git a/libteec/src/teec_trace.c b/libteec/src/teec_trace.c
index 78b79d6..7901deb 100644
--- a/libteec/src/teec_trace.c
+++ b/libteec/src/teec_trace.c
@@ -73,7 +73,7 @@ int _dprintf(const char *function, int flen, int line, int level,
 	     const char *prefix, const char *fmt, ...)
 {
 	char raw[MAX_PRINT_SIZE];
-	char prefixed[MAX_PRINT_SIZE];
+	char prefixed[MAX_PRINT_SIZE + 10];
 	char *to_print = NULL;
 	const char *func;
 	int err;
@@ -106,7 +106,7 @@ int _dprintf(const char *function, int flen, int line, int level,
 		 */
 		int thread_id = syscall(SYS_gettid);	/* perf issue ? */
 
-		snprintf(prefixed, MAX_PRINT_SIZE,
+		snprintf(prefixed, MAX_PRINT_SIZE + 10,
 			 "%s [%d] %s:%s:%d: %s",
 			 trace_level_strings[level], thread_id, prefix, func,
 			 line, raw);

Of course, it's not nice to add 10 to two places, and saying as the define is MAX_PRINT_SIZE, we probably shouldn't be adding to it.

This patch looked simpler to me:

diff --git a/libteec/src/teec_trace.c b/libteec/src/teec_trace.c
index 78b79d6..4056661 100644
--- a/libteec/src/teec_trace.c
+++ b/libteec/src/teec_trace.c
@@ -72,7 +72,7 @@ static const char * const trace_level_strings[] = {
 int _dprintf(const char *function, int flen, int line, int level,
 	     const char *prefix, const char *fmt, ...)
 {
-	char raw[MAX_PRINT_SIZE];
+	char raw[MAX_PRINT_SIZE - 10];
 	char prefixed[MAX_PRINT_SIZE];
 	char *to_print = NULL;
 	const char *func;

However, 10 probably isn't the correct value anyway.

The prefix string could potentially be longer than 10 characters. It's likely to be 13 or more characters, based on this format string "%s [%d] %s:%s:%d: %s" and only if each variable is 1 character long in the final string.

But trace_level_strings can be 0 or 3 chars long; thread id is likely to be many characters long, potentially up to 11 character for a 32-bit int or 20 characters for a 64 bit int; prefix and func are be variable length strings passed in, so it might be better to use %{value}s to limit their length and take that into account; then there's the final line int, which again could be up to 20 characters long.

That makes 3 + 20 + prefix string + func string + 20 + 3 spaces + 3 colons + 2 brackets == 51 + prefix string + func string.

So how about this instead?

diff --git a/libteec/src/teec_trace.c b/libteec/src/teec_trace.c
index 78b79d6..440b579 100644
--- a/libteec/src/teec_trace.c
+++ b/libteec/src/teec_trace.c
@@ -72,7 +72,7 @@ static const char * const trace_level_strings[] = {
 int _dprintf(const char *function, int flen, int line, int level,
 	     const char *prefix, const char *fmt, ...)
 {
-	char raw[MAX_PRINT_SIZE];
+	char raw[MAX_PRINT_SIZE - 100];
 	char prefixed[MAX_PRINT_SIZE];
 	char *to_print = NULL;
 	const char *func;
@@ -107,7 +107,7 @@ int _dprintf(const char *function, int flen, int line, int level,
 		int thread_id = syscall(SYS_gettid);	/* perf issue ? */
 
 		snprintf(prefixed, MAX_PRINT_SIZE,
-			 "%s [%d] %s:%s:%d: %s",
+			 "%3s [%d] %24s:%24s:%d: %s",
 			 trace_level_strings[level], thread_id, prefix, func,
 			 line, raw);
 		to_print = prefixed;

I'm happy to submit a patch if you tell me how you'd prefer to do it?

@simonqhughes
Copy link

simonqhughes commented Jun 22, 2018

Please find attached a fix for this problem. I can submit as PR if given an indication its acceptable.

Its been through the following testing:

  • built with multiple other projects of multiple projects including optee_client and openembedded-core both at tip of master.
From 30dd2986fb64aba7ee78d4e231c344e2c39d7999 Mon Sep 17 00:00:00 2001
From: Simon Hughes <simon.hughes@arm.com>
Date: Thu, 21 Jun 2018 17:22:23 +0100
Subject: [PATCH] Fix for teec_trace.c snprintf -Werror=format-truncation=
 error.

Signed-off-by: Simon Hughes <simon.hughes@arm.com>
---
 libteec/src/teec_trace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libteec/src/teec_trace.c b/libteec/src/teec_trace.c
index 78b79d6..c91bc43 100644
--- a/libteec/src/teec_trace.c
+++ b/libteec/src/teec_trace.c
@@ -106,7 +106,8 @@ int _dprintf(const char *function, int flen, int line, int level,
 		 */
 		int thread_id = syscall(SYS_gettid);	/* perf issue ? */
 
-		snprintf(prefixed, MAX_PRINT_SIZE,
+		int len = 0;
+		len  = snprintf(prefixed+len, MAX_PRINT_SIZE,
 			 "%s [%d] %s:%s:%d: %s",
 			 trace_level_strings[level], thread_id, prefix, func,
 			 line, raw);
-- 
2.7.4

See also:

OP-TEE/optee_test#296

0001-Fix-for-teec_trace.c-snprintf-Werror-format-truncati.patch.txt

@omnium21
Copy link

Both Simon and my fixes build OK for me with GCC8 in my Yocto environment. As per xtest issue 296, xtest is giving a segmentation fault before outputting anything. I believe this is unrelated to either solution and both work fine when built with GCC 7.

@jforissier
Copy link
Contributor

Thanks for the patches. I have chosen a more drastic approach in #128. Please check it out and let me know if it works for you.

EmbeddedAndroid pushed a commit to open-nav/meta-lmp that referenced this issue Sep 26, 2018
Fix for GCC 8.1, as described at
OP-TEE/optee_client#126

Final fix to be provided via upstream and meta-linaro.

Signed-off-by: Ricardo Salveti <ricardo@opensourcefoundries.com>
KhiemNguyenT pushed a commit to renesas-rcar/meta-renesas that referenced this issue Feb 28, 2019
This commit applies a W/A patch from optee upstream to fix
GCC 8 format-truncation errors.

[1] OP-TEE/optee_client#126

Signed-off-by: Thuy Tran <thuy.tran.xh@renesas.com>
Signed-off-by: Takamitsu Honda <takamitsu.honda.pv@renesas.com>
khangnguyenrvc pushed a commit to renesas-rcar/meta-renesas that referenced this issue Jun 19, 2019
This commit applies a W/A patch from optee upstream to fix
GCC 8 format-truncation errors.

[1] OP-TEE/optee_client#126

Signed-off-by: Thuy Tran <thuy.tran.xh@renesas.com>
Signed-off-by: Khang Nguyen <khang.nguyen.xw@renesas.com>
Signed-off-by: Takamitsu Honda <takamitsu.honda.pv@renesas.com>
khangnguyenrvc added a commit to renesas-rcar/meta-renesas that referenced this issue Sep 30, 2019
This commit applies a W/A patch from optee upstream to fix
GCC 9 format-truncation errors.

[1] OP-TEE/optee_client#126

Signed-off-by: Khang Nguyen <khang.nguyen.xw@renesas.com>
Signed-off-by: Takamitsu Honda <takamitsu.honda.pv@renesas.com>
Change-Id: Iafdda8934e315366d0d4546d37b63c5bdb4b5a3e
@github-actions
Copy link

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants