[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH][for-4.19] domain: add ASSERT to help static analysis tools


  • To: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 8 Nov 2023 12:19:27 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=WPwGrlmT1yZA/HeLt1AaT2UJDJ1p9vKOM1seUBu/eXQ=; b=QJlQh//JEIZSjy/uyWfZ7e/bX9GQx18C1g6TNuWNNwdkXk/LHmvtaHNhERWAoXrB1TvN58EYZSkUPadLU//8yJT61yUuHe0VnFw5nnZT75QCau9YWg9dZTvOfQLn4wz/+YwO23XZuAsyRY+BbWpMmUz1ZYYfKs/8XxNxcFeS8umMw4EwW5ubM9SDhZW0KVGfuuUj6uqNKfYJ7DQGic0AunGpbYeTzfLT69AanPaQrC4hzuqGNAGjGFYLsgLkdxhpqLMd4V2f1fu7/+P13SXyvDyJMmJ8qT74oXWg9OO6R1GyEZKzQ1Wo05tFqGSQZ2IL9zILIdQzPAJXJrhgN8hhMg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AI21flVfNPgCWL5lzCH+p+tH0GgUMemDfqairI9dran7M92YTV748/DfFC4GFKJf2p8uXhSlkhWy0Ft1MTnL9+cVvSqsVveiDBvyWJpHmRoofy39LLNSuHQJu4lgA90xuD6RCAqjPflilF7MZEeUUlefSmxYRTJySyn3aLbEHt2xi71HdQy6kqMDniBx8Rr/tfHmtM0MRD8BcFJN7WnOPikhfX9PVWyzkbflH42/IcpX+UtbnHvYd3huH0vM8LGQ7DluvIDOamys2A6qvrghKr6XFqvIE68DhsHQvI2nxiLc1YudhlJTgbmXYTw0qcZCD7ogRFeBuUhkhC2TXWW37w==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: sstabellini@xxxxxxxxxx, michal.orzel@xxxxxxx, xenia.ragiadakou@xxxxxxx, ayan.kumar.halder@xxxxxxx, consulting@xxxxxxxxxxx, andrew.cooper3@xxxxxxxxxx, roger.pau@xxxxxxxxxx, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 08 Nov 2023 11:19:44 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 08.11.2023 12:03, Nicola Vetrini wrote:
> On 2023-11-08 09:24, Jan Beulich wrote:
>> On 03.11.2023 18:58, Nicola Vetrini wrote:
>>> Static analysis tools may detect a possible null
>>> pointer dereference at line 760 (the memcpy call)
>>> of xen/common/domain.c. This ASSERT helps them in
>>> detecting that such a condition is not possible
>>> and also provides a basic sanity check.
>>
>> I disagree with this being a possible justification for adding such a
>> redundant assertion. More detail is needed on what is actually
>> (suspected to be) confusing the tool. Plus it also needs explaining
>> why (a) adding such an assertion helps and (b) how that's going to
>> cover release builds.
>>
> 
> How about:
> "Static analysis tools may detect a possible null pointer dereference
> at line 760 (config->handle) due to config possibly being NULL.
> 
> However, given that all system domains, including IDLE, have a NULL
> config and in the code path leading to the assertion only real domains
> (which have a non-NULL config) can be present."
> 
> On point b): this finding is a false positive, therefore even if the 
> ASSERT is
> expanded to effectively a no-op, there is no inherent problem with Xen's 
> code.
> The context in which the patch was suggested [1] hinted at avoiding 
> inserting in
> the codebase false positive comments.

Which I largely agree with. What I don't agree with is adding an
assertion which is only papering over the issue, and only in debug
builds. So perhaps instead we need a different way of tracking
false positives (which need to be tied to specific checker versions
anyway).

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.