Christopher - had a chance to look into this any further? Roman - I'm OK either way on that. I don't think it's the most costly false positive - not too much code is probably freeing via a reference (rather than a pointer) to allocated memory.
On Fri, Apr 30, 2021 at 10:08 AM Roman Lebedev <lebedev...@gmail.com> wrote: > > Should the diagnostic be backed out until then? > > Roman > > On Fri, Apr 30, 2021 at 7:52 PM Christopher Di Bella via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > > > Sorry, not yet. I'll talk with my TL to see if I can get some time allotted > > for this in the next few weeks. > > > > On Thu, 29 Apr 2021 at 16:13, David Blaikie <dblai...@gmail.com> wrote: > >> > >> Ping on this - have you had a chance to look at this false positive? > >> > >> On Sat, Apr 3, 2021 at 4:29 PM David Blaikie <dblai...@gmail.com> wrote: > >> > > >> > Looks like this has a false positive (that's firing on some mlir code, > >> > committed a workaround in 499571ea835daf786626a0db1e12f890b6cd8f8d ) > >> > like this: > >> > > >> > $ cat test.cpp > >> > #include <stdlib.h> > >> > void f1(int & x) { free(&x); } > >> > int main() { f1(*(int*)malloc(sizeof(int))); } > >> > > >> > Could you fix that? (& then revert the workaround) > >> > > >> > On Fri, Jan 15, 2021 at 1:44 PM Christopher Di Bella via cfe-commits > >> > <cfe-commits@lists.llvm.org> wrote: > >> > > > >> > > > >> > > Author: Christopher Di Bella > >> > > Date: 2021-01-15T21:38:47Z > >> > > New Revision: 4a47da2cf440c2f2006d9b04acfef4292de1e263 > >> > > > >> > > URL: > >> > > https://github.com/llvm/llvm-project/commit/4a47da2cf440c2f2006d9b04acfef4292de1e263 > >> > > DIFF: > >> > > https://github.com/llvm/llvm-project/commit/4a47da2cf440c2f2006d9b04acfef4292de1e263.diff > >> > > > >> > > LOG: [Sema] turns -Wfree-nonheap-object on by default > >> > > > >> > > We'd discussed adding the warning to -Wall in D89988. This patch > >> > > honours that. > >> > > > >> > > Added: > >> > > > >> > > > >> > > Modified: > >> > > clang/include/clang/Basic/DiagnosticGroups.td > >> > > clang/include/clang/Basic/DiagnosticSemaKinds.td > >> > > clang/test/Analysis/NewDelete-intersections.mm > >> > > clang/test/Analysis/free.c > >> > > > >> > > Removed: > >> > > > >> > > > >> > > > >> > > ################################################################################ > >> > > diff --git a/clang/include/clang/Basic/DiagnosticGroups.td > >> > > b/clang/include/clang/Basic/DiagnosticGroups.td > >> > > index d500ab321058..04ba89aa457e 100644 > >> > > --- a/clang/include/clang/Basic/DiagnosticGroups.td > >> > > +++ b/clang/include/clang/Basic/DiagnosticGroups.td > >> > > @@ -110,6 +110,7 @@ def FloatConversion : > >> > > FloatZeroConversion]>; > >> > > > >> > > def FrameAddress : DiagGroup<"frame-address">; > >> > > +def FreeNonHeapObject : DiagGroup<"free-nonheap-object">; > >> > > def DoublePromotion : DiagGroup<"double-promotion">; > >> > > def EnumTooLarge : DiagGroup<"enum-too-large">; > >> > > def UnsupportedNan : DiagGroup<"unsupported-nan">; > >> > > > >> > > diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td > >> > > b/clang/include/clang/Basic/DiagnosticSemaKinds.td > >> > > index 7d36397a7993..e93657898f58 100644 > >> > > --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td > >> > > +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td > >> > > @@ -7609,7 +7609,7 @@ def err_no_typeid_with_fno_rtti : Error< > >> > > def err_no_dynamic_cast_with_fno_rtti : Error< > >> > > "use of dynamic_cast requires -frtti">; > >> > > def warn_no_dynamic_cast_with_rtti_disabled: Warning< > >> > > - "dynamic_cast will not work since RTTI data is disabled by " > >> > > + "dynamic_cast will not work since RTTI data is disabled by " > >> > > "%select{-fno-rtti-data|/GR-}0">, InGroup<RTTI>; > >> > > def warn_no_typeid_with_rtti_disabled: Warning< > >> > > "typeid will not work since RTTI data is disabled by " > >> > > @@ -7625,8 +7625,7 @@ def warn_condition_is_assignment : > >> > > Warning<"using the result of an " > >> > > InGroup<Parentheses>; > >> > > def warn_free_nonheap_object > >> > > : Warning<"attempt to call %0 on non-heap object %1">, > >> > > - InGroup<DiagGroup<"free-nonheap-object">>, > >> > > - DefaultIgnore; // FIXME: add to -Wall after sufficient testing > >> > > + InGroup<FreeNonHeapObject>; > >> > > > >> > > // Completely identical except off by default. > >> > > def warn_condition_is_idiomatic_assignment : Warning<"using the > >> > > result " > >> > > > >> > > diff --git a/clang/test/Analysis/NewDelete-intersections.mm > >> > > b/clang/test/Analysis/NewDelete-intersections.mm > >> > > index f01d62f8d365..6f81034ee349 100644 > >> > > --- a/clang/test/Analysis/NewDelete-intersections.mm > >> > > +++ b/clang/test/Analysis/NewDelete-intersections.mm > >> > > @@ -24,9 +24,6 @@ > >> > > extern "C" void free(void *); > >> > > > >> > > void testMallocFreeNoWarn() { > >> > > - int i; > >> > > - free(&i); // no warn > >> > > - > >> > > int *p1 = (int *)malloc(sizeof(int)); > >> > > free(++p1); // no warn > >> > > > >> > > @@ -51,7 +48,7 @@ void testDeleteMalloced() { > >> > > > >> > > int *p2 = (int *)__builtin_alloca(sizeof(int)); > >> > > delete p2; // no warn > >> > > -} > >> > > +} > >> > > > >> > > void testUseZeroAllocatedMalloced() { > >> > > int *p1 = (int *)malloc(0); > >> > > @@ -79,13 +76,13 @@ void testObjcFreeNewed() { > >> > > } > >> > > > >> > > void testFreeAfterDelete() { > >> > > - int *p = new int; > >> > > + int *p = new int; > >> > > delete p; > >> > > free(p); // newdelete-warning{{Use of memory after it is freed}} > >> > > } > >> > > > >> > > void testStandardPlacementNewAfterDelete() { > >> > > - int *p = new int; > >> > > + int *p = new int; > >> > > delete p; > >> > > p = new (p) int; // newdelete-warning{{Use of memory after it is > >> > > freed}} > >> > > } > >> > > > >> > > diff --git a/clang/test/Analysis/free.c b/clang/test/Analysis/free.c > >> > > index 0d29bacf274c..b50145713924 100644 > >> > > --- a/clang/test/Analysis/free.c > >> > > +++ b/clang/test/Analysis/free.c > >> > > @@ -12,17 +12,23 @@ void *alloca(size_t); > >> > > > >> > > void t1 () { > >> > > int a[] = { 1 }; > >> > > - free(a); // expected-warning {{Argument to free() is the address of > >> > > the local variable 'a', which is not memory allocated by malloc()}} > >> > > + free(a); > >> > > + // expected-warning@-1{{Argument to free() is the address of the > >> > > local variable 'a', which is not memory allocated by malloc()}} > >> > > + // expected-warning@-2{{attempt to call free on non-heap object > >> > > 'a'}} > >> > > } > >> > > > >> > > void t2 () { > >> > > int a = 1; > >> > > - free(&a); // expected-warning {{Argument to free() is the address > >> > > of the local variable 'a', which is not memory allocated by malloc()}} > >> > > + free(&a); > >> > > + // expected-warning@-1{{Argument to free() is the address of the > >> > > local variable 'a', which is not memory allocated by malloc()}} > >> > > + // expected-warning@-2{{attempt to call free on non-heap object > >> > > 'a'}} > >> > > } > >> > > > >> > > void t3 () { > >> > > static int a[] = { 1 }; > >> > > - free(a); // expected-warning {{Argument to free() is the address of > >> > > the static variable 'a', which is not memory allocated by malloc()}} > >> > > + free(a); > >> > > + // expected-warning@-1{{Argument to free() is the address of the > >> > > static variable 'a', which is not memory allocated by malloc()}} > >> > > + // expected-warning@-2{{attempt to call free on non-heap object > >> > > 'a'}} > >> > > } > >> > > > >> > > void t4 (char *x) { > >> > > @@ -71,12 +77,16 @@ void t13 () { > >> > > } > >> > > > >> > > void t14 (char a) { > >> > > - free(&a); // expected-warning {{Argument to free() is the address > >> > > of the parameter 'a', which is not memory allocated by malloc()}} > >> > > + free(&a); > >> > > + // expected-warning@-1{{Argument to free() is the address of the > >> > > parameter 'a', which is not memory allocated by malloc()}} > >> > > + // expected-warning@-2{{attempt to call free on non-heap object > >> > > 'a'}} > >> > > } > >> > > > >> > > static int someGlobal[2]; > >> > > void t15 () { > >> > > - free(someGlobal); // expected-warning {{Argument to free() is the > >> > > address of the global variable 'someGlobal', which is not memory > >> > > allocated by malloc()}} > >> > > + free(someGlobal); > >> > > + // expected-warning@-1{{Argument to free() is the address of the > >> > > global variable 'someGlobal', which is not memory allocated by > >> > > malloc()}} > >> > > + // expected-warning@-2{{attempt to call free on non-heap object > >> > > 'someGlobal'}} > >> > > } > >> > > > >> > > void t16 (char **x, int offset) { > >> > > > >> > > > >> > > > >> > > _______________________________________________ > >> > > cfe-commits mailing list > >> > > cfe-commits@lists.llvm.org > >> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > > > > > -- > > Kind regards, > > > > Christopher Di Bella (he/him/his) > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits