From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) by sourceware.org (Postfix) with ESMTPS id 6F79F3858C60 for ; Tue, 2 Nov 2021 17:09:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6F79F3858C60 Received: by mail-qv1-xf36.google.com with SMTP id gh1so13722967qvb.8 for ; Tue, 02 Nov 2021 10:09:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=R7HY2tPXdYuco6T3P1PHffXw4b4StGmyVahEa0Z0rVk=; b=oqhYeuhG2EbYsaSb6/sG+w41Q2LnEWuT65broOD9U7W8G4kq3DHYfvjfIYZx5WSh16 iAPkBAQECyvBJT4+lnRXEjYVPHpCvrtxsqZ38pi4M1KRjhmRueId9dAcuMx0WgybBjqg BNlFN7O6iFp1hpHOpXIlVz6MSkS95HXyzCYn+76r7uhhHKdxTXL4mNxGnzGXYtQLhS5l qjkVySg0ZkwWoFR6PfsfjafQ/RHmGTovEstPlNXvb9Y8OY/ip7xBwAg7epddpWcbebZ+ PG5mCqEhAjaBbUuDIQiWHBpBcU/uqsuGYkb6iRQJQOMbqLuzzXv+oUixELAs6uYrsdBS REVA== X-Gm-Message-State: AOAM532wjhnBkqqFcHVXJ1+4psA1LDpudYQAFlsvtoYXaxufnMteLJD3 r+c5lV6JYI5HYEbnJmfp2TjBK01TkAI= X-Google-Smtp-Source: ABdhPJziwalYYJ2V/22OZBD96/+uKj0rVGf7yXTaVKSQw786zqptBOZxq1TAGJ5W61/NAQkGrno+/A== X-Received: by 2002:a05:6214:2426:: with SMTP id gy6mr37140396qvb.12.1635872990919; Tue, 02 Nov 2021 10:09:50 -0700 (PDT) Received: from [192.168.0.41] (184-96-250-76.hlrn.qwest.net. [184.96.250.76]) by smtp.gmail.com with ESMTPSA id u17sm7724620qtx.51.2021.11.02.10.09.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 02 Nov 2021 10:09:50 -0700 (PDT) Subject: Re: [PATCH 1/2] add -Wuse-after-free To: Eric Gallager Cc: gcc-patches References: From: Martin Sebor Message-ID: <749d99d6-75fe-e2ed-676b-fef5531f4a16@gmail.com> Date: Tue, 2 Nov 2021 11:09:49 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Nov 2021 17:09:53 -0000 On 11/1/21 11:32 PM, Eric Gallager wrote: > On Mon, Nov 1, 2021 at 6:18 PM Martin Sebor via Gcc-patches > wrote: >> >> Patch 1 in the series detects a small subset of uses of pointers >> made indeterminate by calls to deallocation functions like free >> or C++ operator delete. To control the conditions the warnings >> are issued under the new -Wuse-after-free= option provides three >> levels. At the lowest level the warning triggers only for >> unconditional uses of freed pointers and doesn't warn for uses >> in equality expressions. Level 2 warns also for come conditional >> uses, and level 3 also for uses in equality expressions. >> >> I debated whether to make level 2 or 3 the default included in >> -Wall. I decided on 3 for two reasons: 1) to raise awareness >> of both the problem and GCC's new ability to detect it: using >> a pointer after it's been freed, even only in principle, by >> a successful call to realloc, is undefined, and 2) because >> it's trivial to lower the level either globally, or locally >> by suppressing the warning around such misuses. >> >> I've tested the patch on x86_64-linux and by building Glibc >> and Binutils/GDB. It triggers a number of times in each, all >> due to comparing invalidated pointers for equality (i.e., level >> 3). I have suppressed these in GCC (libiberty) by a #pragma, >> and will see how the Glibc folks want to deal with theirs (I >> track them in BZ #28521). >> >> The tests contain a number of xfails due to limitations I'm >> aware of. I marked them pr?????? until the patch is approved. >> I will open bugs for them before committing if I don't resolve >> them in a followup. >> >> Martin > > Hi, I'm just wondering how this fares compared to the static > analyzer's -Wanalyzer-use-after-free; could you compare and contrast > them for us? Good question. The analyzer does a far more exhaustive, interprocedural analysis of (most) paths through a program, symbolically evaluating the conditions under which statements are evaluated to determine reachability. This initial implementation of -Wuse-after-free does only a superficial analysis of a few nearby statements in a single function (plus those inlined into it), those with direct dependencies of uses on the deallocation statements. It doesn't do any evaluation of conditions which limits how far it can go in its checking. If it sees a pointer used after a free call as in free (p); return *p; // used after free (level 1) it triggers. If it sees a conditional use as in free (p); if (cond) return *p; // may be used after free (level 2) it triggers at level 2, but only if the free is unconditional and flows directly into the condition guarding the use. If the free is guarded by another condition it doesn't trigger: if (c_1) free (p); if (c_2) return *p; This last case is the consequence of not doing any condition evaluation (c_1 could be mutually exclusive with c_2). Adding support for it is a future enhancement, something I'm out of time for in this stage 1 but I'd like to tackle for GCC 13. Both GCC's and Clang's analyzers detect all three cases. Unlike GCC's analyzer (but like Clang's), this new warning flags all pointer uses, not just their derefernces (operands of equality expressions only at level 3; Clang doesn't seem to diagnose uses in equality tests at all). So unlike GCC's analyzer, it will trigger in the first two example above even if it's the pointer itself being returned and not what it pointed to before it was freed . This is because a freed pointer is invalid the same way an uninitialized variable is invalid, and using it in any way is strictly undefined (returning it from a function or passing as an argument to another can lead to memory corruption or unintentional information disclosure). Martin PS The distinction between levels 1 (unconditional uses) and 2 (conditional) is more like that between the levels of -Warray-bounds -- level 2 catching slightly more cases at the expense of potentially some, albeit rare, false positives. It's not like between -Wuninitialized and -Wmaybe-uninitialized where the uninitialized checker employs limited symbolic predicate analysis and the need for separate options came out of the desire to be able to control separately the false positives due to the imperfections inherent in this strategy (or incidental to its implementation in GCC: the limited ability to determine that two conditions are mutually exclusive.