Re: [BUGS] BUG #6672: Memory leaks in dumputils.c
Peter, Some of the false positives can be suppressed by teaching the analyzer about the codebase. For example, many of them are due to the custom assertion/error handlers, where you stop execution on an error path by calling, for example, 'elog'. You can drastically reduce the number of false positives by annotating these few functions as 'noreturn'. See "Custom Assertion Handlers" in http://clang-analyzer.llvm.org/annotations.html. If you run into other false positives please feel free to file bugs against the analyser or ask questions on clang-dev mailing list (http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev). Cheers, Anna. On Fri, Jun 1, 2012 at 4:02 AM, Peter Geoghegan wrote: > On 1 June 2012 06:06, Tom Lane wrote: >> There were no html reports attached, and I'd prefer plain text >> anyway please ... > > I saw a number of false positives when I ran the Clang static analyser > a few months ago. As I recall, I could see why the tool concluded that > certain lines of code may have contained errors, even though it was > evident to me that they actually did not. That said, it probably > wouldn't hurt to give it another try sometime soon. > > -- > Peter Geoghegan http://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6672: Memory leaks in dumputils.c
On Fri, Jun 1, 2012 at 11:13 AM, Josh Kupershmidt wrote: > On Thu, May 31, 2012 at 10:06 PM, Tom Lane wrote: >> zaks.a...@gmail.com writes: > >>> There are two memory leaks in dumputils (v9.2.0beta1): >> >>> 1) >>> File: src/bin/scripts/dumputils.c >>> Location: line 604, column 11 >>> Description: Memory is never released; potential leak of memory >>> pointed to by 'aclitems' >> >>> 2) >>> File: src/bin/scripts/dumputils.c >>> Location: line 793, column 10 >>> Description: Memory is never released; potential leak of memory >>> pointed to by 'eqpos' >> >> This is a remarkably unhelpful report. I do not see any memory >> allocation occurring on either line 604 or line 793 of dumputils.c, >> in either 9.2beta1 or 9.1.3. Could you perhaps provide source code >> extracts rather than line numbers that reference indeterminate versions >> of files? > > I suspect the first complaint is about this bit in git head's > ./src/bin/pg_dump/dumputils.c: > > if (!parseAclItem(aclitems[i], type, name, subname, remoteVersion, > grantee, grantor, privs, privswgo)) > return false; > > since 'aclitems' isn't being freed before the return. And the second > complaint seems to concern parseAclItem() not freeing 'buf' when it > returns false. Both of these errors seem academic, since the callers > of buildACLCommands() will bail out with exit_horribly() or > exit_nicely() if it returns false. But IMO it's worth fixing anyway, > to keep the compilers happy or in case of future code calling > buildACLCommands() or parseAclItem(). > > Attached is a patch to hopefully fix those two errors. I couldn't > quite verify this fixes the OP's error messages, since "checker-266" > isn't done running make after several hours on this OS X machine. > Josh, What kid of machine are you using? Please, let me know how long it took after it's done (It takes about one and a half hours on mine). Thanks, Anna. > Josh -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6672: Memory leaks in dumputils.c
On Fri, Jun 1, 2012 at 11:53 AM, Josh Kupershmidt wrote: > On Fri, Jun 1, 2012 at 11:38 AM, Anna Zaks wrote: >> Josh, >> >> What kid of machine are you using? Please, let me know how long it >> took after it's done (It takes about one and a half hours on mine). > > It just finished, actually: took about 7 hours to run, not counting > the time the machine was asleep last night. Specs: Macbook Pro, 2.53 > GHz i5 with 8GB RAM, OS X 10.6.8, usually modest load on the machine. > I downloaded checker-266.tar.bz2 from http://clang-analyzer.llvm.org/ > , and then ran: > > /path/to/checker-266/scan-build ./configure > --prefix=/Users/josh/runtime/ --enable-cassert --enable-debug > /path/to/checker-266/scan-build make > Any way to know if it was swapping? It should not be this slow.. > Josh -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6672: Memory leaks in dumputils.c
Josh, I opened an analyzer Bugzilla report for this issue in case you 'd like to follow up there: http://llvm.org/bugs/show_bug.cgi?id=13010 Thanks, Anna. On Fri, Jun 1, 2012 at 3:19 PM, Anna Zaks wrote: > On Fri, Jun 1, 2012 at 11:53 AM, Josh Kupershmidt wrote: >> On Fri, Jun 1, 2012 at 11:38 AM, Anna Zaks wrote: >>> Josh, >>> >>> What kid of machine are you using? Please, let me know how long it >>> took after it's done (It takes about one and a half hours on mine). >> >> It just finished, actually: took about 7 hours to run, not counting >> the time the machine was asleep last night. Specs: Macbook Pro, 2.53 >> GHz i5 with 8GB RAM, OS X 10.6.8, usually modest load on the machine. >> I downloaded checker-266.tar.bz2 from http://clang-analyzer.llvm.org/ >> , and then ran: >> >> /path/to/checker-266/scan-build ./configure >> --prefix=/Users/josh/runtime/ --enable-cassert --enable-debug >> /path/to/checker-266/scan-build make >> > > Any way to know if it was swapping? It should not be this slow.. > >> Josh -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
Re: [BUGS] BUG #6672: Memory leaks in dumputils.c
On Fri, Jun 1, 2012 at 9:26 PM, Josh Kupershmidt wrote: > On Fri, Jun 1, 2012 at 4:23 PM, Anna Zaks wrote: > >> I opened an analyzer Bugzilla report for this issue in case you 'd >> like to follow up there: >> http://llvm.org/bugs/show_bug.cgi?id=13010 > > Thanks, I'll try to schedule another run tonight and post additional > details on that ticket. > > On Fri, Jun 1, 2012 at 4:02 AM, Peter Geoghegan wrote: > >> I saw a number of false positives when I ran the Clang static analyser >> a few months ago. As I recall, I could see why the tool concluded that >> certain lines of code may have contained errors, even though it was >> evident to me that they actually did not. That said, it probably >> wouldn't hurt to give it another try sometime soon. > > Yeah, there seems to be a lot of noise in the results, but I found > them interesting nonetheless. I put up the HTML report at: > http://kupershmidt.org/pg/scan-build-2012-05-31-1/ > > since it's way too big to mail even a tarball. It's too bad the clang > doesn't understand our ereport(ERROR, ...) calls don't return to the > caller, as those seem to account for a fair bit of the spurious > warnings. I haven't seen anything which I'd call an outright bug, > though there are e.g. non-kosher uses of malloc() which could > certainly be improved. > > Hrm, I wonder if proc_exit() and ExitPostmaster() could be declared > with __attribute__((noreturn)) , that seems like it would quiet a few > errors. > Keep in mind that the analyser does not perform analyses across translation units (files), so it will not 'see' implementation of the custom assertion handler unless it's defined in the same file. Cheers, Anna. > Josh -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
