Hi Varun,
So today beyond sending out emails first to folks who have set an account on the Synopsys online Coverity service which we also now forward to the TF-A mailing list there is no TF-A tracking. The expectation/hope is that people read these emails and do the "right thing". Yes that’s not ideal but its where we are today. As I said if we can get this Coverity run on the patch submissions it would be better to address them there in the patch review.
If we are forced to have to do the runs after patch summits because of tooling issues we could make use of the phabricator task system to record them and track them but that would be a very manual process and we have no way to enforce the issues reported are addressed. So not too keen on going that route. Issues are tracked in Synopsys online Coverity service anyway which everyone has access to if they create an account. In most cases contributors do the "right thing" and look to address once they are notified or see these. I use to manually ping the submitters and owners about new issues when we started sharing this but now I'm expecting people to see and read this email notification on the mailing list.
So room for improvement but lets be careful on not making an onerous manual process here.
Cheers
Joanna
On 22/07/2020, 23:03, "Varun Wadekar" vwadekar@nvidia.com wrote:
Thanks Joanna. I understand the testing part of the puzzle and don’t see any problems with it. As you said, it would be great if we can integrate it in OpenCI.
I guess, I was trying to understand the other part of the puzzle. Once we identify the code issues, how do we fix them? Is it the responsibility of the code owner to keep an eye on the emails and then provide fixes? Or do we create tickets manually for tracking?
-----Original Message----- From: Joanna Farley Joanna.Farley@arm.com Sent: Wednesday, July 22, 2020 10:27 AM To: Varun Wadekar vwadekar@nvidia.com; tf-a@lists.trustedfirmware.org Subject: Re: [TF-A] New Defects reported by Coverity Scan for ARM-software/arm-trusted-firmware
External email: Use caution opening links or attachments
Hi Varun,
I agree things are less than ideal at the moment. We take advantage as an open source project to use the Synopsys online Coverity service and this points to the master branch (actually currently the mirror on github) which means these coverity tests are ran on changes after they have been submitted. Less than ideal I know but better than not being ran at all. Really we would like these to be ran as part of the OpenCI as part of patch reviews but we are not there yet. Not sure if the Synopsys online Coverity service can be setup to support this via Gerrit reviews or if this will have to be done after patch submittals. If it has to be done after then some clever CI scripting would need to be done to identify where the offending change came from.
Would welcome other ideas to make this experience better, but for now this is better than nothing.
Cheers
Joanna
On 22/07/2020, 18:09, "TF-A on behalf of Varun Wadekar via TF-A" <tf-a-bounces@lists.trustedfirmware.org on behalf of tf-a@lists.trustedfirmware.org> wrote:
Hi,
Is there a way to create a ticket and assign Coverity flagged issues to the code owner automatically?
-Varun
-----Original Message----- From: TF-A tf-a-bounces@lists.trustedfirmware.org On Behalf Of scan-admin--- via TF-A Sent: Wednesday, July 22, 2020 8:17 AM To: tf-a@lists.trustedfirmware.org Subject: [TF-A] New Defects reported by Coverity Scan for ARM-software/arm-trusted-firmware
External email: Use caution opening links or attachments
Hi,
Please find the latest report on new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan.
3 new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 3 of 3 defect(s)
** CID 360935: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /drivers/st/scmi-msg/clock.c: 273 in write_rate_desc_array_in_buffer()
________________________________________________________________________________________________________ *** CID 360935: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /drivers/st/scmi-msg/clock.c: 273 in write_rate_desc_array_in_buffer() 267 size_t n; 268 269 ASSERT_SYM_PTR_ALIGN(out); 270 271 for (n = 0U; n < nb_elt; n++) { 272 out[2 * n] = (uint32_t)rates[n]; >>> CID 360935: Integer handling issues (CONSTANT_EXPRESSION_RESULT) >>> "(uint64_t)rates[n] >> 32" is 0 regardless of the values of its operands. This occurs as the operand of assignment. 273 out[2 * n + 1] = (uint32_t)((uint64_t)rates[n] >> 32); 274 } 275 } 276 277 static void scmi_clock_describe_rates(struct scmi_msg *msg) 278 {
** CID 360934: Integer handling issues (BAD_SHIFT) /drivers/st/scmi-msg/clock.c: 273 in write_rate_desc_array_in_buffer()
________________________________________________________________________________________________________ *** CID 360934: Integer handling issues (BAD_SHIFT) /drivers/st/scmi-msg/clock.c: 273 in write_rate_desc_array_in_buffer() 267 size_t n; 268 269 ASSERT_SYM_PTR_ALIGN(out); 270 271 for (n = 0U; n < nb_elt; n++) { 272 out[2 * n] = (uint32_t)rates[n]; >>> CID 360934: Integer handling issues (BAD_SHIFT) >>> In expression "(uint64_t)rates[n] >> 32", right shifting "rates[n]" by more than 31 bits always yields zero. The shift amount is 32. 273 out[2 * n + 1] = (uint32_t)((uint64_t)rates[n] >> 32); 274 } 275 } 276 277 static void scmi_clock_describe_rates(struct scmi_msg *msg) 278 {
** CID 360933: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /drivers/st/scmi-msg/clock.c: 191 in scmi_clock_rate_get()
________________________________________________________________________________________________________ *** CID 360933: Integer handling issues (CONSTANT_EXPRESSION_RESULT) /drivers/st/scmi-msg/clock.c: 191 in scmi_clock_rate_get() 185 return; 186 } 187 188 rate = plat_scmi_clock_get_rate(msg->agent_id, clock_id); 189 190 return_values.rate[0] = (uint32_t)rate; >>> CID 360933: Integer handling issues (CONSTANT_EXPRESSION_RESULT) >>> "(uint64_t)rate >> 32" is 0 regardless of the values of its operands. This occurs as the operand of assignment. 191 return_values.rate[1] = (uint32_t)((uint64_t)rate >> 32); 192 193 scmi_write_response(msg, &return_values, sizeof(return_values)); 194 } 195 196 static void scmi_clock_rate_set(struct scmi_msg *msg)
________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0...
-- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a -- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a