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

Error building from develop branch, cpuid problems #937

Closed
Vascom opened this issue Apr 20, 2020 · 7 comments · Fixed by #938
Closed

Error building from develop branch, cpuid problems #937

Vascom opened this issue Apr 20, 2020 · 7 comments · Fixed by #938

Comments

@Vascom
Copy link
Collaborator

Vascom commented Apr 20, 2020

I try build commit c7874f8
And it failed with this errors:

/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c: In function 'main':
/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c:34:9: error: 'cpuid.revision' may be used uninitialized in this func
tion [-Werror=maybe-uninitialized]
   34 |         printf("cpuid:part = %#x, rev = %#x\n", cpuid.part, cpuid.revision);
      |         ^
/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c:31:27: note: 'cpuid.revision' was declared here
   31 |         cortex_m3_cpuid_t cpuid;
      |                           ^
/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c:34:9: error: 'cpuid.part' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   34 |         printf("cpuid:part = %#x, rev = %#x\n", cpuid.part, cpuid.revision);
      |         ^
/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c:31:27: note: 'cpuid.part' was declared here
   31 |         cortex_m3_cpuid_t cpuid;
      |                           ^
/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c:33:9: error: 'cpuid.variant' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   33 |         printf("cpuid:impl_id = %0#x, variant = %#x\n", cpuid.implementer_id, cpuid.variant);
      |         ^
/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c:31:27: note: 'cpuid.variant' was declared here
   31 |         cortex_m3_cpuid_t cpuid;
      |                           ^
/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c:33:9: error: 'cpuid.implementer_id' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   33 |         printf("cpuid:impl_id = %0#x, variant = %#x\n", cpuid.implementer_id, cpuid.variant);
      |         ^
/builddir/build/BUILD/stlink-org-stlink-c7874f8/tests/usb.c:31:27: note: 'cpuid.implementer_id' was declared here
   31 |         cortex_m3_cpuid_t cpuid;
      |                           ^
lto1: all warnings being treated as errors

How it can be fixed?

@chenguokai
Copy link
Collaborator

On macOS, the code builds well.

In src/common.c

int stlink_cpu_id(stlink_t *sl, cortex_m3_cpuid_t *cpuid) {
    uint32_t raw;

    if (stlink_read_debug32(sl, STLINK_REG_CM3_CPUID, &raw))
        return -1;

    cpuid->implementer_id = (raw >> 24) & 0x7f;
    cpuid->variant = (raw >> 20) & 0xf;
    cpuid->part = (raw >> 4) & 0xfff;
    cpuid->revision = raw & 0xf;
    return 0;
}

It seems that cpuid.part and cpuid.revision is indeed initialized here. Maybe a compiler misbehave?

@Vascom
Copy link
Collaborator Author

Vascom commented Apr 20, 2020

    if (stlink_read_debug32(sl, STLINK_REG_CM3_CPUID, &raw))
        return -1;

In this case cpuid.* stay uninitialized.

@Ant-ON
Copy link
Collaborator

Ant-ON commented Apr 20, 2020

On Ubuntu successful compiled too. Variable cpuid may use uninited in the case of error read register.

--- a/tests/usb.c
+++ b/tests/usb.c
@@ -29,7 +29,11 @@ int main(int ac, char** av) {
printf("-- core_id: %#x\n", sl->core_id);

     cortex_m3_cpuid_t cpuid;
  •    stlink_cpu_id(sl, &cpuid);
    
  •    if (stlink_cpu_id(sl, &cpuid))
    
  •    {
    
  •        printf("unknown chip\n");
    
  •        memset(&cpuid, 0, sizeof(cortex_m3_cpuid_t))
    
  •    }
       printf("cpuid:impl_id = %0#x, variant = %#x\n", cpuid.implementer_id, cpuid.variant);
       printf("cpuid:part = %#x, rev = %#x\n", cpuid.part, cpuid.revision);
    

@chenguokai
Copy link
Collaborator

I think it can be fixed by adding a return value check on stlink_cpu_id and initialize those values to zero. I will give it a try.

@Vascom
Copy link
Collaborator Author

Vascom commented Apr 20, 2020

I have -Werror=maybe-uninitialized because during configure:

-- Performing Test C_SUPPORTS_WMAYBE_UNINITIALIZED
-- Performing Test C_SUPPORTS_WMAYBE_UNINITIALIZED - Success

@chenguokai
Copy link
Collaborator

@Vascom May you test my branch https://github.com/chenguokai/stlink/tree/issue937 ?
If it works, I will raise a PR.

@Vascom
Copy link
Collaborator Author

Vascom commented Apr 20, 2020

@Vascom May you test my branch https://github.com/chenguokai/stlink/tree/issue937 ?
If it works, I will raise a PR.

Yes, it works.

@Nightwalker-87 Nightwalker-87 added this to To do in Release v1.6.1 via automation Apr 20, 2020
@Nightwalker-87 Nightwalker-87 added this to the v1.6.1 milestone Apr 20, 2020
Release v1.6.1 automation moved this from To do to Done Apr 20, 2020
@stlink-org stlink-org locked as resolved and limited conversation to collaborators May 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants