Re: Warnings in Renesas common files
Hello Team, We are trying to include Renesas RX65N architecture in GitHub pre-check which requires code to have 0 warnings to be successful. Call to up_copystate() in the below functions produces warnings in NuttX common files: arch/renesas/src/common/up_blocktask.c: up_block_task() arch/renesas/src/common/up_releasepending: up_release_pending() arch/renesas/src/common/up_reprioritizertr: up_reprioritize_rtr() arch/renesas/src/common/up_unblocktask: up_unblock_task() Changes in the above files will affect the other Renesas architectures (m16c and sh1) as well. The function prototype of up_copystate() in file: "arch/renesas/src/common/up_internal.h" is as follows: void up_copystate(uint32_t *dest, uint32_t *src); g_current_regs is also defined in the same file as: extern volatile uint32_t *g_current_regs; Calling up_copystate() with g_current_regs as 2nd argument gives the following warning: passing argument 2 of ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type [-Werror] up_copystate(rtcb->xcp.regs, g_current_regs); ^ The above call causes the 'volatile' nature of g_current_regs to be lost. We understand that casting away the 'volatile' type qualifier of a variable should be avoided if possible. We intend to do a PR by modifying the prototype of the above function to: void up_copystate(uint32_t *dest, volatile uint32_t *src); This modification produces the same warning in memcpy() function call. Typecasting the argument when calling memcpy() alone will resolve this warning. This retains the volatile nature of the variable as well as resolves the warnings produced due to the above function call with volatile g_current_regs argument. Are there any concerns with the above mentioned modifications? Regards, Bhindhiya Raja. ____ From: Bhindhiya Raja Sent: Monday, September 14, 2020 5:09 PM To: dev@nuttx.apache.org Subject: Re: Warnings in Renesas common files Hello Greg and Nathan, Any input on the warnings generated due to the g_current_regs volatile type qualifier? Best Regards, Bhindhiya Raja SBU | Engineer TATA ELXSI LIMITED ITPB Road Whitefield Bengaluru 560 048 India www.tataelxsi.com<https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.tataelxsi.com%2F=02%7C01%7Cbhindhiya.r%40tataelxsi.co.in%7C27f3ef9461c64949d6e508d832f8ecbc%7Cad6a39dd96b6436882daf2ec4d92e26a%7C0%7C0%7C637315393618704041=FdDPWsO9WgR7HscuY%2BLQkX1oTtr%2BuNpCgEl35LtrZZ8%3D=0> From: Nathan Hartman Sent: Saturday, September 5, 2020 3:51 AM To: dev@nuttx.apache.org Subject: Re: Warnings in Renesas common files **This is an external email. Please check the sender’s full email address (not just the sender name) and exercise caution before you respond or click any embedded link/attachment.** On Fri, Sep 4, 2020 at 9:40 AM Bhindhiya Raja wrote: > > Hello Nathan, > > Thank you for your response. We have submitted 2 separate PRs as recommended > by you to resolve warnings 6 (#1695) and 8 (#1702). > > About warnings 1 to 4, we understand that casting away the 'volatile' type > qualifier of a variable should be avoided if possible. Can we consider > modifying the prototype of the function up_copystate() in up_internal.h file, > from: > void up_copystate(uint32_t *dest, uint32_t *src); > to: > void up_copystate(uint32_t *dest, volatile uint32_t *src); > in order to resolve this warning. > > We have tried the above fix in our local repository and checked build. > We found that warnings 1 to 4 are resolved, however, it generates a new > warning in file: > arch/renesas/src/rx65n/rx65n_copystate.c: passing argument 2 of > ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type > memcpy(dest, src, XCPTCONTEXT_SIZE); > > expected ‘const void *’ but argument is of type ‘volatile uint32_t *’ > FAR void *memcpy(FAR void *dest, FAR const void *src, size_t n); > > This can be resolved by typecasting the variables when calling memcpy() alone. > > As these files are common to other Renesas architectures as well, please > confirm if the above modifications are okay. Hello Bhindhiya, I am not sure what is the best choice here. I looked through NuttX sources and I did not see any up_copystate() (for any arch) that had volatile qualifiers. That said, looking at the call to up_copystate() in up_unblock_task(), g_current_regs is indeed volatile (for good reasons), so I wonder if we should propagate that as far as possible, until the memcpy(), like you're suggesting, or, alternatively, if it is safe to discard volatile like is being done already, because we are switching tasks. I need to think about this some more. Maybe Greg will share some of his wisdom with us. :-) Nathan Disclaimer: This email and a
Re: Warnings in Renesas common files
Hello Greg and Nathan, Any input on the warnings generated due to the g_current_regs volatile type qualifier? Best Regards, Bhindhiya Raja SBU | Engineer TATA ELXSI LIMITED ITPB Road Whitefield Bengaluru 560 048 India www.tataelxsi.com<https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.tataelxsi.com%2F=02%7C01%7Cbhindhiya.r%40tataelxsi.co.in%7C27f3ef9461c64949d6e508d832f8ecbc%7Cad6a39dd96b6436882daf2ec4d92e26a%7C0%7C0%7C637315393618704041=FdDPWsO9WgR7HscuY%2BLQkX1oTtr%2BuNpCgEl35LtrZZ8%3D=0> From: Nathan Hartman Sent: Saturday, September 5, 2020 3:51 AM To: dev@nuttx.apache.org Subject: Re: Warnings in Renesas common files **This is an external email. Please check the sender’s full email address (not just the sender name) and exercise caution before you respond or click any embedded link/attachment.** On Fri, Sep 4, 2020 at 9:40 AM Bhindhiya Raja wrote: > > Hello Nathan, > > Thank you for your response. We have submitted 2 separate PRs as recommended > by you to resolve warnings 6 (#1695) and 8 (#1702). > > About warnings 1 to 4, we understand that casting away the 'volatile' type > qualifier of a variable should be avoided if possible. Can we consider > modifying the prototype of the function up_copystate() in up_internal.h file, > from: > void up_copystate(uint32_t *dest, uint32_t *src); > to: > void up_copystate(uint32_t *dest, volatile uint32_t *src); > in order to resolve this warning. > > We have tried the above fix in our local repository and checked build. > We found that warnings 1 to 4 are resolved, however, it generates a new > warning in file: > arch/renesas/src/rx65n/rx65n_copystate.c: passing argument 2 of > ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type > memcpy(dest, src, XCPTCONTEXT_SIZE); > > expected ‘const void *’ but argument is of type ‘volatile uint32_t *’ > FAR void *memcpy(FAR void *dest, FAR const void *src, size_t n); > > This can be resolved by typecasting the variables when calling memcpy() alone. > > As these files are common to other Renesas architectures as well, please > confirm if the above modifications are okay. Hello Bhindhiya, I am not sure what is the best choice here. I looked through NuttX sources and I did not see any up_copystate() (for any arch) that had volatile qualifiers. That said, looking at the call to up_copystate() in up_unblock_task(), g_current_regs is indeed volatile (for good reasons), so I wonder if we should propagate that as far as possible, until the memcpy(), like you're suggesting, or, alternatively, if it is safe to discard volatile like is being done already, because we are switching tasks. I need to think about this some more. Maybe Greg will share some of his wisdom with us. :-) Nathan Disclaimer: This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you are not the intended recipient of this message , or if this message has been addressed to you in error, please immediately alert the sender by reply email and then delete this message and any attachments. If you are not the intended recipient, you are hereby notified that any use, dissemination, copying, or storage of this message or its attachments is strictly prohibited. Email transmission cannot be guaranteed to be secure or error-free, as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender, therefore, does not accept liability for any errors, omissions or contaminations in the contents of this message which might have occurred as a result of email transmission. If verification is required, please request for a hard-copy version.
Re: Warnings in Renesas common files
Hello Nathan, Thank you for your response. We have submitted 2 separate PRs as recommended by you to resolve warnings 6 (#1695) and 8 (#1702). About warnings 1 to 4, we understand that casting away the 'volatile' type qualifier of a variable should be avoided if possible. Can we consider modifying the prototype of the function up_copystate() in up_internal.h file, from: void up_copystate(uint32_t *dest, uint32_t *src); to: void up_copystate(uint32_t *dest, volatile uint32_t *src); in order to resolve this warning. We have tried the above fix in our local repository and checked build. We found that warnings 1 to 4 are resolved, however, it generates a new warning in file: arch/renesas/src/rx65n/rx65n_copystate.c: passing argument 2 of ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type memcpy(dest, src, XCPTCONTEXT_SIZE); expected ‘const void *’ but argument is of type ‘volatile uint32_t *’ FAR void *memcpy(FAR void *dest, FAR const void *src, size_t n); This can be resolved by typecasting the variables when calling memcpy() alone. As these files are common to other Renesas architectures as well, please confirm if the above modifications are okay. Regards, Bhindhiya Raja From: Nathan Hartman Sent: Tuesday, September 1, 2020 6:32 PM To: dev@nuttx.apache.org Subject: Re: Warnings in Renesas common files **This is an external email. Please check the sender’s full email address (not just the sender name) and exercise caution before you respond or click any embedded link/attachment.** On Tue, Sep 1, 2020 at 8:12 AM Bhindhiya Raja wrote: > Hello Team, > > We are working on adding pre-check support for Renesas RX65N MCU. We are > getting the below warnings in the common files used across the Renesas MCUs > (M16, SH1 and RX65N). > > > 1. arch/renesas/src/common/up_blocktask.c: passing argument 2 of > ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type > 2. arch/renesas/src/common/up_releasepending.c: passing argument 2 of > ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type > 3. arch/renesas/src/common/up_reprioritizertr.c: passing argument 2 of > ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type > 4. arch/renesas/src/common/up_unblocktask.c: passing argument 2 of > ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type > 5. arch/renesas/src/common/up_doirq.c: assignment discards ‘volatile’ > qualifier from pointer target type > 6. arch/renesas/src/common/up_initialize.c: implicit declaration of > function ‘up_serialinit’ > 7. arch/renesas/src/common/up_usestack.c: assignment makes pointer from > integer without a cast > 8. net/mld/mld_group.c: label ‘errout_with_sem’ defined but not used > > The following modifications would resolve the above mentioned warnings. > > Warnings 1, 2, 3 and 4 mentioned above can be resolved by typecasting the > argument 2 of ‘up_copystate’ to the type of the actual parameter while > invoking the function as follows: > up_copystate(rtcb->xcp.regs, (uint32_t *)up_current_regs); > > Warning 5 can be resolved similarly by typecasting the variable upon > assignment as follows: > regs = (uint32_t *)g_current_regs; Should we cast away the volatile modifier or is this something that should be looked into? (i.e., should those functions be modified to take a volatile uint32_t *?) Warning 6 can be resolved by including the function declaration for > up_serialinit() in the header file up_internal.h. Warning 7 can be resolved by typecasting the variable top_of_stack to (void > *) as follows: > tcb->adj_stack_ptr = (void *)top_of_stack; > > Warning 8 can be resolved by removing the label 'errout_with_sem' as it is > simply defined but not used anywhere in the function. I will need to examine the code to be sure (I'm replying on my phone now) but the fix for 6 and 8 sounds reasonable. Unless these warnings are cleared, RX65N cannot be added to the pre-check > since due to the [-Werror] flag, the build fails. > As the above mentioned files are common files to m16 and sh1 architectures > also, can you confirm whether the above modifications are okay. I recommend to submit separate PRs for fixes 6 and 8. But I think the use of volatile modifiers needs to be looked at more closely. I prefer not to cast away volatile-ness unless no other choice. Also not sure about 7. Cheers, Nathan Disclaimer: This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you are not the intended recipient of this message , or if this message has been addressed to you in error, please immediately alert the send
Warnings in Renesas common files
Hello Team, We are working on adding pre-check support for Renesas RX65N MCU. We are getting the below warnings in the common files used across the Renesas MCUs (M16, SH1 and RX65N). 1. arch/renesas/src/common/up_blocktask.c: passing argument 2 of ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type 2. arch/renesas/src/common/up_releasepending.c: passing argument 2 of ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type 3. arch/renesas/src/common/up_reprioritizertr.c: passing argument 2 of ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type 4. arch/renesas/src/common/up_unblocktask.c: passing argument 2 of ‘up_copystate’ discards ‘volatile’ qualifier from pointer target type 5. arch/renesas/src/common/up_doirq.c: assignment discards ‘volatile’ qualifier from pointer target type 6. arch/renesas/src/common/up_initialize.c: implicit declaration of function ‘up_serialinit’ 7. arch/renesas/src/common/up_usestack.c: assignment makes pointer from integer without a cast 8. net/mld/mld_group.c: label ‘errout_with_sem’ defined but not used The following modifications would resolve the above mentioned warnings. Warnings 1, 2, 3 and 4 mentioned above can be resolved by typecasting the argument 2 of ‘up_copystate’ to the type of the actual parameter while invoking the function as follows: up_copystate(rtcb->xcp.regs, (uint32_t *)up_current_regs); Warning 5 can be resolved similarly by typecasting the variable upon assignment as follows: regs = (uint32_t *)g_current_regs; Warning 6 can be resolved by including the function declaration for up_serialinit() in the header file up_internal.h. Warning 7 can be resolved by typecasting the variable top_of_stack to (void *) as follows: tcb->adj_stack_ptr = (void *)top_of_stack; Warning 8 can be resolved by removing the label 'errout_with_sem' as it is simply defined but not used anywhere in the function. Unless these warnings are cleared, RX65N cannot be added to the pre-check since due to the [-Werror] flag, the build fails. As the above mentioned files are common files to m16 and sh1 architectures also, can you confirm whether the above modifications are okay. Regards, Bhindhiya Raja Disclaimer: This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you are not the intended recipient of this message , or if this message has been addressed to you in error, please immediately alert the sender by reply email and then delete this message and any attachments. If you are not the intended recipient, you are hereby notified that any use, dissemination, copying, or storage of this message or its attachments is strictly prohibited. Email transmission cannot be guaranteed to be secure or error-free, as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender, therefore, does not accept liability for any errors, omissions or contaminations in the contents of this message which might have occurred as a result of email transmission. If verification is required, please request for a hard-copy version.
Request to join Mailing List
Hi I would like to join the mailing list. Regards Bhindhiya Raja Disclaimer: This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you are not the intended recipient of this message , or if this message has been addressed to you in error, please immediately alert the sender by reply email and then delete this message and any attachments. If you are not the intended recipient, you are hereby notified that any use, dissemination, copying, or storage of this message or its attachments is strictly prohibited. Email transmission cannot be guaranteed to be secure or error-free, as information could be intercepted, corrupted, lost, destroyed, arrive late or incomplete, or contain viruses. The sender, therefore, does not accept liability for any errors, omissions or contaminations in the contents of this message which might have occurred as a result of email transmission. If verification is required, please request for a hard-copy version.
