From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf42.google.com (mail-qv1-xf42.google.com [IPv6:2607:f8b0:4864:20::f42]) by sourceware.org (Postfix) with ESMTPS id D2C8C386F81C for ; Mon, 14 Dec 2020 21:39:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D2C8C386F81C Received: by mail-qv1-xf42.google.com with SMTP id 4so8542674qvh.1 for ; Mon, 14 Dec 2020 13:39:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=BwHvLzmO41y9Wv+QNrK0RUTS9zsBjPJo20EHvPP1Xqc=; b=G8eLMeUQnaIvtW5D1sHfZRSoy1E7rmuHILYAjTGgyX3JUEUUCLxo/ccJw+yXHKzWKZ CZlbs4U/okk1CzU1v4WjZ4x+xg6C1JCADJ44eE0dgY0YgSIMCG0bw1A5c3QMt4ByjpS9 ItvjA4yaPRLRej/bUeJa5PX23INvKM3gYJUy+mPmtMqXvjWOxZxSp6MT4ueNyCqqD8Vo DLMyFFakY3BCGQkq47jZ89tkt+tCprKok4A6mNOxh+KnIPW/+jWXBsB73E+lXGmOxvsG HRtcO3+7dXhD30xKp40TvIrC9Cy3RXXH2hql7BlHQZMaauQA4HS7h+udjiizm3HMuovC xKvQ== X-Gm-Message-State: AOAM530LrUzy8HfbueeJCaEaSC5kwNEO7bibXKqOMWjanQDrd3gTNi/a lbQxa6b+wMYFafMARFFP57U= X-Google-Smtp-Source: ABdhPJwTMR3Fs3T1kK1Q+KczXydx4/uXWRO0q18HLkXC1eBYVP+7rzSeOsM05TY0EgoIdiff1e/2vw== X-Received: by 2002:a0c:ab08:: with SMTP id h8mr34710064qvb.38.1607981998464; Mon, 14 Dec 2020 13:39:58 -0800 (PST) Received: from [192.168.0.41] (174-16-97-231.hlrn.qwest.net. [174.16.97.231]) by smtp.gmail.com with ESMTPSA id z20sm15093127qto.40.2020.12.14.13.39.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 14 Dec 2020 13:39:57 -0800 (PST) Subject: Re: [PATCH] add support for -Wmismatched-dealloc From: Martin Sebor To: Joseph Myers Cc: GNU C Library , Florian Weimer References: <74efece7-9a4b-83ee-7fdd-475c0d514378@gmail.com> Message-ID: <758e723b-67cf-a211-7bc2-2ccd3fc744e5@gmail.com> Date: Mon, 14 Dec 2020 14:39:56 -0700 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: <74efece7-9a4b-83ee-7fdd-475c0d514378@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, NICE_REPLY_A, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Dec 2020 21:40:00 -0000 On 12/11/20 7:25 PM, Martin Sebor wrote: > On 12/8/20 5:07 PM, Joseph Myers wrote: >> I don't see any definition of __attr_dealloc (presumably should be a >> macro >> in misc/sys/cdefs.h) in this patch (or in the glibc source tree). > > Whoops! > >> >> Given all the functions in stdio.h with the same list of deallocation >> functions, there should probably be another macro there to expand to >> __attribute_malloc__ __attr_dealloc (fclose, 1) __attr_dealloc (freopen, >> 3) __attr_dealloc_freopen64.  (That would also apply to >> open_wmemstream in >> wchar.h, but I suppose you have the issue there with functions not being >> declared in wchar.h, only in stdio.h?) > > You're right that adding the attribute to open_wmemstream runs into > the same problem as declaring the ctermid argument with L_ctermid > elements in : fclose isn't declared in .  I did > some more work on adding the attribute to other functions and found > out that the same problem also happens between tempnam() in > and reallocarray() in . > > I spent some time working around this but in the end it turned out > to be too convoluted so I decided to make the attribute a little > smarter.  Instead of associating all allocation functions with all > deallocation functions (such as fdopen, fopen, fopen64, etc. with > fclose, freopen, and freopen64) I changed it so that an allocator > only needs to be associated with a single deallocator (a reallocator > also needs to be associated with itself).  That makes things quite > a bit simpler. > > The attached patch implements this for , , and > .  To get around the dependency on it > uses __REDIRECT to introduce a reserved alias for fclose. > > Besides running the test suite I tested it with my own test and also > by adding the same declarations to the GCC test suite and verifying > it triggers warnings as expected. > > The GCC patches needed to make this simpler scheme work haven't been > reviewed yet so this work has a dependency on them getting approved. The GCC patches have now been committed and the dependency resolved. Florian asked this morning if getaddrinfo() and freeaddrinfo() are covered by this change. They're not because getaddrinfo() returns the allocated memory indirectly, via an argument. To handle those kinds of APIs that return a pointer to the allocated object indirectly, through an argument, the attribute will need to be enhanced somehow. > > I grepped for __attribute_malloc__ in Glibc headers to see if there > are other APIs that would benefit from the same annotation but found > none.  At the same time, I don't have the impression that malloc is > used on all the APIs it could be.  Are there any that you or anyone > else can think of that might be worth looking at? > > Martin