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

Reply via email to