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.

Sign up for GitHub

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

Jump to bottom

Fix build warnings and initialize functions local variables #146

Closed
wants to merge 4 commits into from
Closed

Fix build warnings and initialize functions local variables #146

wants to merge 4 commits into from

Conversation

etienne-lms
Copy link
Contributor

A first commit to fix a build issue reported by Buildroot team.
A second commit to initialize (almost) all functions local variables. May prevent build errors in the future.

@etienne-lms
libteec: fix build warnings
57683e5 
Fix build warnings reported by the Buildroot team [1]:

/home/thomas/projets/outputs/armv5-ctng-linux-gnueabi/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function 'TEEC_InitializeContext':
/home/thomas/projets/outputs/armv5-ctng-linux-gnueabi/build/optee-client-3.4.0/libteec/src/tee_client_api.c:149:28: error: 'gen_caps' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    ctx->reg_mem = gen_caps & TEE_GEN_CAP_REG_MEM;
                            ^
/home/thomas/projets/buildroot/output/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_OpenSession’:
/home/thomas/projets/buildroot/output/build/optee-client-3.4.0/libteec/src/tee_client_api.c:507:8: error: cast increases required alignment of target type [-Werror=cast-align]
  arg = (struct tee_ioctl_open_session_arg *)buf;
        ^
/home/thomas/projets/buildroot/output/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_InvokeCommand’:
/home/thomas/projets/buildroot/output/build/optee-client-3.4.0/libteec/src/tee_client_api.c:581:8: error: cast increases required alignment of target type [-Werror=cast-align]
  arg = (struct tee_ioctl_invoke_arg *)buf;
        ^

[1] http://lists.busybox.net/pipermail/buildroot/2019-February/243437.html

Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
@etienne-lms
explicitly initialize local variables
a6382cd 
This change initializes all local variables to prevent build issues
(warnings and/or errors) in OP-TEE client package.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
@jbech-linaro
Copy link
Contributor

Thanks @etienne-lms for doing this (a tedious task). I've test built both using GNU Make as well as with CMake, both works fine. I've also reviewed the code, so ...
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

jenswi-linaro
jenswi-linaro reviewed Feb 28, 2019
View reviewed changes
Copy link
Contributor

@jenswi-linaro jenswi-linaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to block this pull request with this comment, but I think you should be aware of this.

libteec/src/tee_client_api.c Outdated Show resolved Hide resolved
@etienne-lms
[review] explicitly initialize local variables
1edfcb4 
Use memset() to init structured and typed variables instead of = { 0 }.
This change changes ordering in the local variable definition block
at function head. Structured variables are defined below, right above
the memset() call block for their initialization, when possible.

Restore gprof_process() that initialized a string variable with an
explicit empty string prior the change.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
jenswi-linaro
jenswi-linaro reviewed Mar 1, 2019
View reviewed changes
libteec/src/tee_client_api.c Outdated
@@ -504,7 +511,7 @@ TEEC_Result TEEC_OpenSession(TEEC_Context *ctx, TEEC_Session *session,
buf_data.buf_ptr = (uintptr_t)buf;
buf_data.buf_len = sizeof(buf);

arg = (struct tee_ioctl_open_session_arg *)buf;
arg = (struct tee_ioctl_open_session_arg *)(void *)buf;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the extra cast? I was expecting the alignment of buf to be large enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is this setup where GCC (--version returns ls (GNU coreutils) 8.21) complains, see http://lists.busybox.net/pipermail/buildroot/2019-February/243437.html.

/home/thomas/projets/buildroot/output/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_InvokeCommand’:
/home/thomas/projets/buildroot/output/build/optee-client-3.4.0/libteec/src/tee_client_api.c:581:8: error: cast increases required alignment of target type [-Werror=cast-align]
  arg = (struct tee_ioctl_invoke_arg *)buf;
        ^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is surprising since buf already is supposed to have the alignment of uint64_t. Are we missing something? How come struct tee_ioctl_invoke_arg needs even greater alignment?
This cast could hide a real problem.
Perhaps it's better to do something like:

union {
    struct tee_ioctl_open_session_arg arg;
    uint8_t data[sizeof(struct tee_ioctl_open_session_arg) + TEEC_CONFIG_PAYLOAD_REF_COUNT * sizeof(struct tee_ioctl_param)];
} buf;

arg = &buf.arg;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it related to the structure, or something toolchains are likely to complain when converting opaque references into effective memory pointers?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not found of the union I could not find another way aside intermediate cast to void *.
I've pushed a fixup with the change proposed by @jenswi-linaro.

@etienne-lms
[review] libteec: fix build warnings
8071a80 
Use union structure to ensure alignment of IOCTL message buffer
and structured message header.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
etienne-lms added a commit to etienne-lms/buildroot that referenced this pull request Mar 18, 2019
@etienne-lms
package/optee-client: fix build warnings from 3.4.0 (TODO: comment)
7f7e592 
      PATCHv2: https://patchwork.ozlabs.org/patch/1053398/
      (TODO: remove this comment)

Add a patch over current optee-client 3.4.0 to fix build issues
reported by some toolchains with traces like:

  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function 'TEEC_InitializeContext':
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:149:28: error: 'gen_caps' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    ctx->reg_mem = gen_caps & TEE_GEN_CAP_REG_MEM;
                            ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_OpenSession’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:507:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_open_session_arg *)buf;
          ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_InvokeCommand’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:581:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_invoke_arg *)buf;

The optee-client patch is under review in the OP-TEE project [1] and
should be merged in the next OP-TEE release 3.5.0.

Fixes [2], [3], [4] and other failing builds reported by autobuild tests.

[1] OP-TEE/optee_client#146
[2] http://autobuild.buildroot.net/results/80e78cb0bb955e912d6cbe5b30c9b024e7efc802
[3] http://autobuild.buildroot.net/results/a42c19897d03beb02fde2e7e6da25532be27d5ca
[4] http://autobuild.buildroot.net/results/827087f91b7481d1c3effd615172bbee86317962

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v2 -> v3:
  - Update patch from [1] outcome

Changes v1 -> v2:
  - Updated commit comment with traces of the build issues that are fixed.
@etienne-lms
Copy link
Contributor Author

etienne-lms commented Mar 19, 2019 •
edited

Superseded.
I've opened #150 to focus on the build warning issue.
I will push a later P-R for the local variables init once #150 completes.

edited: local variables init P-R available at #151.

@etienne-lms etienne-lms mentioned this pull request Mar 20, 2019
Closed
etienne-lms added a commit to etienne-lms/buildroot that referenced this pull request Mar 22, 2019
@etienne-lms
package/optee-client: fix build warnings from 3.4.0
7e57795 
Add a patch over current optee-client 3.4.0 to fix build issues
reported by some toolchains with traces like:

  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function 'TEEC_InitializeContext':
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:149:28: error: 'gen_caps' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    ctx->reg_mem = gen_caps & TEE_GEN_CAP_REG_MEM;
                            ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_OpenSession’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:507:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_open_session_arg *)buf;
          ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_InvokeCommand’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:581:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_invoke_arg *)buf;

The optee-client patch is under review in the OP-TEE project [1] and
should be merged in the next OP-TEE release 3.5.0.

Fixes [2], [3], [4] and other failing builds reported by autobuild tests.

[1] OP-TEE/optee_client#146
[2] http://autobuild.buildroot.net/results/80e78cb0bb955e912d6cbe5b30c9b024e7efc802
[3] http://autobuild.buildroot.net/results/a42c19897d03beb02fde2e7e6da25532be27d5ca
[4] http://autobuild.buildroot.net/results/827087f91b7481d1c3effd615172bbee86317962

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v2 -> v3:
  - Update patch from [1] outcome

Changes v1 -> v2:
  - Updated commit comment with traces of the build issues that are fixed.
etienne-lms added a commit to etienne-lms/buildroot that referenced this pull request Mar 22, 2019
@etienne-lms
package/optee-client: fix build warnings from 3.4.0
f2fb753 
Add a patch over current optee-client 3.4.0 to fix build issues
reported by some toolchains with traces like:

  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function 'TEEC_InitializeContext':
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:149:28: error: 'gen_caps' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    ctx->reg_mem = gen_caps & TEE_GEN_CAP_REG_MEM;
                            ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_OpenSession’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:507:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_open_session_arg *)buf;
          ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_InvokeCommand’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:581:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_invoke_arg *)buf;

The optee-client patches have been in the OP-TEE project [1] & [2] and
will be available in the OP-TEE next release planned 3.5.0.

Fixes [3], [4], [5] and other failing builds reported by autobuild tests.

[1] OP-TEE/optee_client#146
[2] OP-TEE/optee_client#153
[3] http://autobuild.buildroot.net/results/80e78cb0bb955e912d6cbe5b30c9b024e7efc802
[4] http://autobuild.buildroot.net/results/a42c19897d03beb02fde2e7e6da25532be27d5ca
[5] http://autobuild.buildroot.net/results/827087f91b7481d1c3effd615172bbee86317962

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v3 -> v4:
  - Update patch from [2] outcome

Changes v2 -> v3:
  - Update patch from [1] outcome

Changes v1 -> v2:
  - Updated commit comment with traces of the build issues that are fixed.
etienne-lms added a commit to etienne-lms/buildroot that referenced this pull request Mar 25, 2019
@etienne-lms
package/optee-client: fix build warnings from 3.4.0
01fa059 
Add a patch over current optee-client 3.4.0 to fix build issues
reported by some toolchains with traces like:

  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function 'TEEC_InitializeContext':
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:149:28: error: 'gen_caps' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    ctx->reg_mem = gen_caps & TEE_GEN_CAP_REG_MEM;
                            ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_OpenSession’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:507:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_open_session_arg *)buf;
          ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_InvokeCommand’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:581:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_invoke_arg *)buf;

The optee-client patches have been in the OP-TEE project [1] & [2] and
will be available in the OP-TEE next release planned 3.5.0.

Fixes [3], [4], [5] and other failing builds reported by autobuild tests.

[1] OP-TEE/optee_client#146
[2] OP-TEE/optee_client#153
[3] http://autobuild.buildroot.net/results/80e78cb0bb955e912d6cbe5b30c9b024e7efc802
[4] http://autobuild.buildroot.net/results/a42c19897d03beb02fde2e7e6da25532be27d5ca
[5] http://autobuild.buildroot.net/results/827087f91b7481d1c3effd615172bbee86317962

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v4 -> v5:
  - Fix buggy content sent in v3. Apologies.

Changes v3 -> v4:
  - Update patch from [2] outcome

Changes v2 -> v3:
  - Update patch from [1] outcome

Changes v1 -> v2:
  - Updated commit comment with traces of the build issues that are fixed.
etienne-lms added a commit to etienne-lms/buildroot that referenced this pull request Mar 26, 2019
@etienne-lms
package/optee-client: fix build warnings from 3.4.0
deaf2d0 
Add a patch over current optee-client 3.4.0 to fix build issues
reported by some toolchains with traces like:

  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function 'TEEC_InitializeContext':
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:149:28: error: 'gen_caps' may be used uninitialized in this function [-Werror=maybe-uninitialized]
    ctx->reg_mem = gen_caps & TEE_GEN_CAP_REG_MEM;
                            ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_OpenSession’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:507:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_open_session_arg *)buf;
          ^
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c: In function ‘TEEC_InvokeCommand’:
  /path/to/build/optee-client-3.4.0/libteec/src/tee_client_api.c:581:8: error: cast increases required alignment of target type [-Werror=cast-align]
    arg = (struct tee_ioctl_invoke_arg *)buf;

The optee-client patches have been in the OP-TEE project [1] & [2] and
will be available in the OP-TEE next release planned 3.5.0.

Fixes [3], [4], [5] and other failing builds reported by autobuild tests.

[1] OP-TEE/optee_client#146
[2] OP-TEE/optee_client#153
[3] http://autobuild.buildroot.net/results/80e78cb0bb955e912d6cbe5b30c9b024e7efc802
[4] http://autobuild.buildroot.net/results/a42c19897d03beb02fde2e7e6da25532be27d5ca
[5] http://autobuild.buildroot.net/results/827087f91b7481d1c3effd615172bbee86317962

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes v5 -> v6:
  - Cleanup patches applied on optee-client: apply changes following
    related commits ordering in the package.

Changes v4 -> v5:
  - Fix buggy content sent in v3. Apologies.

Changes v3 -> v4:
  - Update patch from [2] outcome

Changes v2 -> v3:
  - Update patch from [1] outcome

Changes v1 -> v2:
  - Updated commit comment with traces of the build issues that are fixed.
@etienne-lms etienne-lms deleted the warnings branch April 10, 2019 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenswi-linaro jenswi-linaro jenswi-linaro left review comments

@jbech-linaro jbech-linaro jbech-linaro left review comments

@jforissier jforissier jforissier left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@etienne-lms @jbech-linaro @jenswi-linaro @jforissier