public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* analyzer: Weekly update on extending C++ support (3)
@ 2023-08-26 14:44 Benjamin Priour
  2023-08-28 15:13 ` Benjamin Priour
  2023-08-29 23:01 ` David Malcolm
  0 siblings, 2 replies; 5+ messages in thread
From: Benjamin Priour @ 2023-08-26 14:44 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc

[-- Attachment #1: Type: text/plain, Size: 3547 bytes --]

Hi David,

Sorry I missed out on your answer about the operator new patch on the IRC.
Should I submit the first bit of the operator new patch ? Putting aside for
now fixing the
"uninitialized value" that accompanies "null deref" of nothrow new variants
?

About generalizing tests to C++, I'll soon have a second batch of similar
size ready,
probably by Monday. I try to find exactly one "real" bug to build each
batch around, to not simply
have a collection of C made C++ friendly.

A few questions on that point.

Test gcc.dg/analyzer/pr103892.c requires -O2.
As such the IPA generated from C and C++ differ,
C++ optimization cuts off function argstr_get_word from the IPA,
hence reducing the number of SN nodes from the SG.
This lesser number of SN is sufficient to make the analysis trips over
being too-complex.
The current formula for that upper limit is
limit = m_sg.num_nodes () * param_analyzer_bb_explosion_factor;
Thus shorter programs - such as the analyzer tests - are more likely to
be diagnosed as too complex. To avoid false positives perhaps we should
add an offset, so that short SG get their chance ?
This is just a tweak, and pr103892.c could as well be discarded for C++,
divergent IPA between C and C++ are unavoidable at some point, and some
tests won't make the transition anyway.

In gcc.dg/analyzer/malloc-1.c:test_50{b,c}, C++ is imprecise as of the
memory freed.

void test_50b (void)
{
  free ((void *) test_50a); /* { dg-warning "'free' of '&test_50a' which
points to memory not on the heap \\\[CWE-590\\\]" } */
  /* { dg-bogus "'free' of 'test_50a' which points to memory not on the
heap \\\[CWE-590\\\]" "c++ lacks precision of freed memory" { xfail c++ }
.-1 } */
}

void test_50c (void)
{
 my_label:
  free (&&my_label); /* { dg-warning "'free' of '&my_label' which points to
memory not on the heap \\\[CWE-590\\\]" } */
  /* { dg-warning "'free' of '&& my_label' which points to memory not on
the heap \\\[CWE-590\\\]" "" { xfail c++ } .-1 } */
}

What do you think of this ?
I've checked malloc_state_machine::handle_free_of_non_heap, arg and
diag_arg are strictly equal.
There might be something to be done with how a tree is transformed into a
diagnostic tree by get_diagnostic_tree,
but I'll wait for your feedback first.

Test gcc.dg/analyzer/pr94362-1.c actually has an additional null_deref
warning in C++, which is not affected by exceptions
or optimizations. I will keep updated on that one. Note that no warnings
are emitted for this test in C.

analyzer/pr94362-1.c: In function ‘const EVP_PKEY_ASN1_METHOD*
EVP_PKEY_asn1_find_str(ENGINE**, const char*, int)’:
analyzer/pr94362-1.c:55:16: warning: dereference of NULL ‘ameth’ [CWE-476]
[-Wanalyzer-null-dereference]
analyzer/pr94362-1.c:39:29: note: (1) entry to ‘EVP_PKEY_asn1_find_str’
analyzer/pr94362-1.c:42:31: note: (2) ‘ameth’ is NULL
analyzer/pr94362-1.c:53:43: note: (3) following ‘true’ branch...
analyzer/pr94362-1.c:54:31: note: (4) ...to here
analyzer/pr94362-1.c:54:31: note: (5) calling ‘EVP_PKEY_asn1_get0’ from
‘EVP_PKEY_asn1_find_str’
analyzer/pr94362-1.c:29:29: note: (6) entry to ‘EVP_PKEY_asn1_get0’
analyzer/pr94362-1.c:32:53: note: (7) ‘0’ is NULL
analyzer/pr94362-1.c:54:31: note: (8) returning to ‘EVP_PKEY_asn1_find_str’
from ‘EVP_PKEY_asn1_get0’
analyzer/pr94362-1.c:54:31: note: (9) ‘ameth’ is NULL
analyzer/pr94362-1.c:55:16: note: (10) dereference of NULL ‘ameth’


Cheers,
Benjamin.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: analyzer: Weekly update on extending C++ support (3)
  2023-08-26 14:44 analyzer: Weekly update on extending C++ support (3) Benjamin Priour
@ 2023-08-28 15:13 ` Benjamin Priour
  2023-08-29 23:04   ` David Malcolm
  2023-08-29 23:01 ` David Malcolm
  1 sibling, 1 reply; 5+ messages in thread
From: Benjamin Priour @ 2023-08-28 15:13 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc

[-- Attachment #1: Type: text/plain, Size: 4587 bytes --]

Hi,

Test gcc.dg/analyzer/pr94362-1.c actually has an additional null_deref
> warning in C++, which is not affected by exceptions
> or optimizations. I will keep updated on that one. Note that no warnings
> are emitted for this test in C.
>
> analyzer/pr94362-1.c: In function ‘const EVP_PKEY_ASN1_METHOD*
> EVP_PKEY_asn1_find_str(ENGINE**, const char*, int)’:
> analyzer/pr94362-1.c:55:16: warning: dereference of NULL ‘ameth’ [CWE-476]
> [-Wanalyzer-null-dereference]
> analyzer/pr94362-1.c:39:29: note: (1) entry to ‘EVP_PKEY_asn1_find_str’
> analyzer/pr94362-1.c:42:31: note: (2) ‘ameth’ is NULL
> analyzer/pr94362-1.c:53:43: note: (3) following ‘true’ branch...
> analyzer/pr94362-1.c:54:31: note: (4) ...to here
> analyzer/pr94362-1.c:54:31: note: (5) calling ‘EVP_PKEY_asn1_get0’ from
> ‘EVP_PKEY_asn1_find_str’
> analyzer/pr94362-1.c:29:29: note: (6) entry to ‘EVP_PKEY_asn1_get0’
> analyzer/pr94362-1.c:32:53: note: (7) ‘0’ is NULL
> analyzer/pr94362-1.c:54:31: note: (8) returning to
> ‘EVP_PKEY_asn1_find_str’ from ‘EVP_PKEY_asn1_get0’
> analyzer/pr94362-1.c:54:31: note: (9) ‘ameth’ is NULL
> analyzer/pr94362-1.c:55:16: note: (10) dereference of NULL ‘ameth’
>
>
> Cheers,
> Benjamin.
>
>
As promised a quick update on that front. I've managed to pinpoint the
difference between the C and C++.
Compiling with -fno-exceptions -O0 the two IPA's seen by the analyzer are
nearly identical, if not for one difference.

const EVP_PKEY_ASN1_METHOD *EVP_PKEY_asn1_find_str(ENGINE **pe, const char
*str,
                                                   int len)
{
  int i;
  const EVP_PKEY_ASN1_METHOD *ameth = (const EVP_PKEY_ASN1_METHOD *) ((void
*)0);
  ...
  for (i = EVP_PKEY_asn1_get_count(); i-- > 0;) {
    ameth = EVP_PKEY_asn1_get0(i); /* At this point, i >= 0 */
    if (ameth->pkey_flags & 0x1)
      continue;
    return ameth;
  }
  ...
}

IPA when compiled by as C:

 <bb 6> :
  i_23 = EVP_PKEY_asn1_get_count ();
  goto <bb 10>; [INV]

  <bb 7> :
  ameth_27 = EVP_PKEY_asn1_get0 (i_24);
  _2 = ameth_27->pkey_flags;
  _3 = _2 & 1;
  if (_3 != 0)
    goto <bb 8>; [INV]
  else
    goto <bb 9>; [INV]

  <bb 8> :
  // predicted unlikely by continue predictor.
  goto <bb 10>; [INV]

  <bb 9> :
  _28 = ameth_27;
  goto <bb 12>; [INV]

  <bb 10> :
  # i_5 = PHI <i_23(6), i_24(8)>
  i.4_4 = i_5;
  i_24 = i.4_4 + -1;
  if (i.4_4 > 0)
    goto <bb 7>; [INV]
  else
    goto <bb 11>; [INV]

... and as C++

<bb 6> :
  i_23 = EVP_PKEY_asn1_get_count ();
  goto <bb 10>; [INV]

  <bb 7> :
  ameth_28 = EVP_PKEY_asn1_get0 (i_24);
  _2 = ameth_28->pkey_flags;
  _3 = _2 & 1;
  if (_3 != 0)
    goto <bb 8>; [INV]
  else
    goto <bb 9>; [INV]

  <bb 8> :
  // predicted unlikely by continue predictor.
  goto <bb 10>; [INV]

  <bb 9> :
  _29 = ameth_28;
  goto <bb 12>; [INV]

  <bb 10> :
  # i_5 = PHI <i_23(6), i_24(8)>
  i.5_4 = i_5;
  i_24 = i.5_4 + -1;
  retval.4_25 = i.5_4 > 0;       /* Difference here. C++ stores the
comparison's result before using it */
  if (retval.4_25 != 0)
    goto <bb 7>; [INV]
  else
    goto <bb 11>; [INV]

This slight difference causes i_24 to not be part of an equivalence class
in C++, although it is part of one in C.
Therefore when calling ameth_28 = EVP_PKEY_asn1_get0 (i_24);, i_24 won't
appear as constrained in C++,
although we know it to be positive. Info is lost.
Further down the line, because of this missing ec
constraint_manager::eval_condition will evaluate to UNKNOWN,
and the pending diagnostic path won't be refused but accepted as feasible
in C++.

constraints and equivalence classes in C:

ec2: {(CONJURED(_8 = sk_EVP_PKEY_ASN1_METHOD_num (app_methos.1_2);,
_8)+(int)1)}
ec3: {(int)0 == [m_constant]'0'} /* int since constraining 'int i_24' */
ec4: {CONJURED(_8 = sk_EVP_PKEY_ASN1_METHOD_num (app_methos.1_2);, _8)} /*
This is the ec of i_24 */
ec5: {(int)-1 == [m_constant]'-1'}
constraints:
1: ec5 < ec4
2: ec3 < ec2

... vs C++'s:

ec2: {((CONJURED(_8 = sk_EVP_PKEY_ASN1_METHOD_num (app_methos.1_2);,
_8)+(int)1)>(int)0)}
ec3: {(bool)0 == [m_constant]'0'} /* bool instead of int cuz constraining
'bool retval.4_25' */
constraints:
1: ec2 != ec3

As you can see, i_24 equivalence class does not appear in C++.
I'm considering if in a condition lhs or rhs is a logical binop svalue, we
should unwrap one level, so that the condition's constraint is actually
applied on
the inner svalue.

Cheers,
Benjamin.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: analyzer: Weekly update on extending C++ support (3)
  2023-08-26 14:44 analyzer: Weekly update on extending C++ support (3) Benjamin Priour
  2023-08-28 15:13 ` Benjamin Priour
@ 2023-08-29 23:01 ` David Malcolm
  2023-08-31 11:20   ` Benjamin Priour
  1 sibling, 1 reply; 5+ messages in thread
From: David Malcolm @ 2023-08-29 23:01 UTC (permalink / raw)
  To: Benjamin Priour; +Cc: gcc

On Sat, 2023-08-26 at 16:44 +0200, Benjamin Priour wrote:
> Hi David,
> 
> Sorry I missed out on your answer about the operator new patch on the
> IRC.
> Should I submit the first bit of the operator new patch ? Putting
> aside for
> now fixing the
> "uninitialized value" that accompanies "null deref" of nothrow new
> variants
> ?

Yes; please submit it, so that we can work towards getting what you
have into trunk.

Given that we don't properly support C++ yet, improvements to C++
support don't have to be perfect.

> 
> About generalizing tests to C++, I'll soon have a second batch of
> similar
> size ready,
> probably by Monday. I try to find exactly one "real" bug to build
> each
> batch around, to not simply
> have a collection of C made C++ friendly.

(nods)

Thanks for pushing the 1st patch.  I've updated my working copies to
try to ensure that my new tests go in c-c++-common as far as possible.

> 
> A few questions on that point.
> 
> Test gcc.dg/analyzer/pr103892.c requires -O2.
> As such the IPA generated from C and C++ differ,
> C++ optimization cuts off function argstr_get_word from the IPA,
> hence reducing the number of SN nodes from the SG.
> This lesser number of SN is sufficient to make the analysis trips
> over
> being too-complex.
> The current formula for that upper limit is
> limit = m_sg.num_nodes () * param_analyzer_bb_explosion_factor;
> Thus shorter programs - such as the analyzer tests - are more likely
> to
> be diagnosed as too complex. To avoid false positives perhaps we
> should
> add an offset, so that short SG get their chance ?

That's an interesting idea...

> This is just a tweak, and pr103892.c could as well be discarded for
> C++,
> divergent IPA between C and C++ are unavoidable at some point, and
> some
> tests won't make the transition anyway.

...but this approach is simpler, so maybe go with that.

> 
> In gcc.dg/analyzer/malloc-1.c:test_50{b,c}, C++ is imprecise as of
> the
> memory freed.
> 
> void test_50b (void)
> {
>   free ((void *) test_50a); /* { dg-warning "'free' of '&test_50a'
> which
> points to memory not on the heap \\\[CWE-590\\\]" } */
>   /* { dg-bogus "'free' of 'test_50a' which points to memory not on
> the
> heap \\\[CWE-590\\\]" "c++ lacks precision of freed memory" { xfail
> c++ }
> .-1 } */
> }
> 
> void test_50c (void)
> {
>  my_label:
>   free (&&my_label); /* { dg-warning "'free' of '&my_label' which
> points to
> memory not on the heap \\\[CWE-590\\\]" } */
>   /* { dg-warning "'free' of '&& my_label' which points to memory not
> on
> the heap \\\[CWE-590\\\]" "" { xfail c++ } .-1 } */
> }
> 
> What do you think of this ?
> I've checked malloc_state_machine::handle_free_of_non_heap, arg and
> diag_arg are strictly equal.
> There might be something to be done with how a tree is transformed
> into a
> diagnostic tree by get_diagnostic_tree,
> but I'll wait for your feedback first.

What does g++ emit for this with your updated test?

> 
> Test gcc.dg/analyzer/pr94362-1.c actually has an additional
> null_deref
> warning in C++, which is not affected by exceptions
> or optimizations. I will keep updated on that one. 

[...snip...; I see you covered this in a followup]

BTW, was there any other work of yours that I need to review? (I know
about the work on placement-new and testsuite migration)

Thanks again for your work on this
Dave


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: analyzer: Weekly update on extending C++ support (3)
  2023-08-28 15:13 ` Benjamin Priour
@ 2023-08-29 23:04   ` David Malcolm
  0 siblings, 0 replies; 5+ messages in thread
From: David Malcolm @ 2023-08-29 23:04 UTC (permalink / raw)
  To: Benjamin Priour; +Cc: gcc

On Mon, 2023-08-28 at 17:13 +0200, Benjamin Priour wrote:
> Hi,
> 
> Test gcc.dg/analyzer/pr94362-1.c actually has an additional
> null_deref
> > warning in C++, which is not affected by exceptions
> > or optimizations. I will keep updated on that one. Note that no
> > warnings
> > are emitted for this test in C.
> > 
> > analyzer/pr94362-1.c: In function ‘const EVP_PKEY_ASN1_METHOD*
> > EVP_PKEY_asn1_find_str(ENGINE**, const char*, int)’:
> > analyzer/pr94362-1.c:55:16: warning: dereference of NULL ‘ameth’
> > [CWE-476]
> > [-Wanalyzer-null-dereference]
> > analyzer/pr94362-1.c:39:29: note: (1) entry to
> > ‘EVP_PKEY_asn1_find_str’
> > analyzer/pr94362-1.c:42:31: note: (2) ‘ameth’ is NULL
> > analyzer/pr94362-1.c:53:43: note: (3) following ‘true’ branch...
> > analyzer/pr94362-1.c:54:31: note: (4) ...to here
> > analyzer/pr94362-1.c:54:31: note: (5) calling ‘EVP_PKEY_asn1_get0’
> > from
> > ‘EVP_PKEY_asn1_find_str’
> > analyzer/pr94362-1.c:29:29: note: (6) entry to ‘EVP_PKEY_asn1_get0’
> > analyzer/pr94362-1.c:32:53: note: (7) ‘0’ is NULL
> > analyzer/pr94362-1.c:54:31: note: (8) returning to
> > ‘EVP_PKEY_asn1_find_str’ from ‘EVP_PKEY_asn1_get0’
> > analyzer/pr94362-1.c:54:31: note: (9) ‘ameth’ is NULL
> > analyzer/pr94362-1.c:55:16: note: (10) dereference of NULL ‘ameth’
> > 
> > 
> > Cheers,
> > Benjamin.
> > 
> > 
> As promised a quick update on that front. I've managed to pinpoint
> the
> difference between the C and C++.
> Compiling with -fno-exceptions -O0 the two IPA's seen by the analyzer
> are
> nearly identical, if not for one difference.
> 
> const EVP_PKEY_ASN1_METHOD *EVP_PKEY_asn1_find_str(ENGINE **pe, const
> char
> *str,
>                                                    int len)
> {
>   int i;
>   const EVP_PKEY_ASN1_METHOD *ameth = (const EVP_PKEY_ASN1_METHOD *)
> ((void
> *)0);
>   ...
>   for (i = EVP_PKEY_asn1_get_count(); i-- > 0;) {
>     ameth = EVP_PKEY_asn1_get0(i); /* At this point, i >= 0 */
>     if (ameth->pkey_flags & 0x1)
>       continue;
>     return ameth;
>   }
>   ...
> }
> 
> IPA when compiled by as C:
> 
>  <bb 6> :
>   i_23 = EVP_PKEY_asn1_get_count ();
>   goto <bb 10>; [INV]
> 
>   <bb 7> :
>   ameth_27 = EVP_PKEY_asn1_get0 (i_24);
>   _2 = ameth_27->pkey_flags;
>   _3 = _2 & 1;
>   if (_3 != 0)
>     goto <bb 8>; [INV]
>   else
>     goto <bb 9>; [INV]
> 
>   <bb 8> :
>   // predicted unlikely by continue predictor.
>   goto <bb 10>; [INV]
> 
>   <bb 9> :
>   _28 = ameth_27;
>   goto <bb 12>; [INV]
> 
>   <bb 10> :
>   # i_5 = PHI <i_23(6), i_24(8)>
>   i.4_4 = i_5;
>   i_24 = i.4_4 + -1;
>   if (i.4_4 > 0)
>     goto <bb 7>; [INV]
>   else
>     goto <bb 11>; [INV]
> 
> ... and as C++
> 
> <bb 6> :
>   i_23 = EVP_PKEY_asn1_get_count ();
>   goto <bb 10>; [INV]
> 
>   <bb 7> :
>   ameth_28 = EVP_PKEY_asn1_get0 (i_24);
>   _2 = ameth_28->pkey_flags;
>   _3 = _2 & 1;
>   if (_3 != 0)
>     goto <bb 8>; [INV]
>   else
>     goto <bb 9>; [INV]
> 
>   <bb 8> :
>   // predicted unlikely by continue predictor.
>   goto <bb 10>; [INV]
> 
>   <bb 9> :
>   _29 = ameth_28;
>   goto <bb 12>; [INV]
> 
>   <bb 10> :
>   # i_5 = PHI <i_23(6), i_24(8)>
>   i.5_4 = i_5;
>   i_24 = i.5_4 + -1;
>   retval.4_25 = i.5_4 > 0;       /* Difference here. C++ stores the
> comparison's result before using it */
>   if (retval.4_25 != 0)
>     goto <bb 7>; [INV]
>   else
>     goto <bb 11>; [INV]
> 
> This slight difference causes i_24 to not be part of an equivalence
> class
> in C++, although it is part of one in C.
> Therefore when calling ameth_28 = EVP_PKEY_asn1_get0 (i_24);, i_24
> won't
> appear as constrained in C++,
> although we know it to be positive. Info is lost.
> Further down the line, because of this missing ec
> constraint_manager::eval_condition will evaluate to UNKNOWN,
> and the pending diagnostic path won't be refused but accepted as
> feasible
> in C++.
> 
> constraints and equivalence classes in C:
> 
> ec2: {(CONJURED(_8 = sk_EVP_PKEY_ASN1_METHOD_num (app_methos.1_2);,
> _8)+(int)1)}
> ec3: {(int)0 == [m_constant]'0'} /* int since constraining 'int i_24'
> */
> ec4: {CONJURED(_8 = sk_EVP_PKEY_ASN1_METHOD_num (app_methos.1_2);,
> _8)} /*
> This is the ec of i_24 */
> ec5: {(int)-1 == [m_constant]'-1'}
> constraints:
> 1: ec5 < ec4
> 2: ec3 < ec2
> 
> ... vs C++'s:
> 
> ec2: {((CONJURED(_8 = sk_EVP_PKEY_ASN1_METHOD_num (app_methos.1_2);,
> _8)+(int)1)>(int)0)}
> ec3: {(bool)0 == [m_constant]'0'} /* bool instead of int cuz
> constraining
> 'bool retval.4_25' */
> constraints:
> 1: ec2 != ec3
> 
> As you can see, i_24 equivalence class does not appear in C++.
> I'm considering if in a condition lhs or rhs is a logical binop
> svalue, we
> should unwrap one level, so that the condition's constraint is
> actually
> applied on
> the inner svalue.

Aha - good debugging!

Your proposed fix sounds worth investigating.  Let me know if you'd
like help with it.

Thanks
Dave


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: analyzer: Weekly update on extending C++ support (3)
  2023-08-29 23:01 ` David Malcolm
@ 2023-08-31 11:20   ` Benjamin Priour
  0 siblings, 0 replies; 5+ messages in thread
From: Benjamin Priour @ 2023-08-31 11:20 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc

[-- Attachment #1: Type: text/plain, Size: 4961 bytes --]

Hi David,


On Wed, Aug 30, 2023 at 1:01 AM David Malcolm <dmalcolm@redhat.com> wrote:

>
> > ?
>
> Yes; please submit it, so that we can work towards getting what you
> have into trunk.
>
> Given that we don't properly support C++ yet, improvements to C++
> support don't have to be perfect.
>
>
It is next in queue for regstrapping, and great news, I nailed the
"supersedes" issue correctly.
I'll split it in two patches, first one for operator new per se that should
fix PR105948,
and all non user-defined variants of operator new should be supported, with
and without
exceptions enabled. Second will be a fix of PR110830, to prevent
superseding warnings
when their saved_diagnostic path are actually divergent.

My current implementation of the latter is a bit naive but works, thus I'm
trying to improve
its running time.


> >
> > About generalizing tests to C++, I'll soon have a second batch of
> > similar
> > size ready,
> > probably by Monday. I try to find exactly one "real" bug to build
> > each
> > batch around, to not simply
> > have a collection of C made C++ friendly.
>
> (nods)
>
> Thanks for pushing the 1st patch.  I've updated my working copies to
> try to ensure that my new tests go in c-c++-common as far as possible.
>
> >
> > A few questions on that point.
> >
> > Test gcc.dg/analyzer/pr103892.c requires -O2.
> > As such the IPA generated from C and C++ differ,
> > C++ optimization cuts off function argstr_get_word from the IPA,
> > hence reducing the number of SN nodes from the SG.
> > This lesser number of SN is sufficient to make the analysis trips
> > over
> > being too-complex.
> > The current formula for that upper limit is
> > limit = m_sg.num_nodes () * param_analyzer_bb_explosion_factor;
> > Thus shorter programs - such as the analyzer tests - are more likely
> > to
> > be diagnosed as too complex. To avoid false positives perhaps we
> > should
> > add an offset, so that short SG get their chance ?
>
> That's an interesting idea...
>
> > This is just a tweak, and pr103892.c could as well be discarded for
> > C++,
> > divergent IPA between C and C++ are unavoidable at some point, and
> > some
> > tests won't make the transition anyway.
>
> ...but this approach is simpler, so maybe go with that.
>
>
Nods.

>
> > In gcc.dg/analyzer/malloc-1.c:test_50{b,c}, C++ is imprecise as of
> > the
> > memory freed.
> >
> > void test_50b (void)
> > {
> >   free ((void *) test_50a); /* { dg-warning "'free' of '&test_50a'
> > which
> > points to memory not on the heap \\\[CWE-590\\\]" } */
> >   /* { dg-bogus "'free' of 'test_50a' which points to memory not on
> > the
> > heap \\\[CWE-590\\\]" "c++ lacks precision of freed memory" { xfail
> > c++ }
> > .-1 } */
> > }
> >
> > void test_50c (void)
> > {
> >  my_label:
> >   free (&&my_label); /* { dg-warning "'free' of '&my_label' which
> > points to
> > memory not on the heap \\\[CWE-590\\\]" } */
> >   /* { dg-warning "'free' of '&& my_label' which points to memory not
> > on
> > the heap \\\[CWE-590\\\]" "" { xfail c++ } .-1 } */
> > }
> >
> > What do you think of this ?
> > I've checked malloc_state_machine::handle_free_of_non_heap, arg and
> > diag_arg are strictly equal.
> > There might be something to be done with how a tree is transformed
> > into a
> > diagnostic tree by get_diagnostic_tree,
> > but I'll wait for your feedback first.
>
> What does g++ emit for this with your updated test?
>
>
I'm not sure what you meant here.
For a free ((void *) test_50a); gcc emits "'free' of '&test_50a'", whereas
g++ emits "'free' of 'test_50a'",
which is less precise about the actually memory freed. This however only
seems to occur with this
function pointers and labels.


> >
> > Test gcc.dg/analyzer/pr94362-1.c actually has an additional
> > null_deref
> > warning in C++, which is not affected by exceptions
> > or optimizations. I will keep updated on that one.
>
> [...snip...; I see you covered this in a followup]
>
>
The fix worked and even a few other XFAILs pass in other tests.
I am regstrapping the second batch of transitioned test,
following up with the "operator new" stuff.
I had an issue with the regstrap, I don't why but comparing the *.sum
files from my control and patched versions gives me unintelligible output.
It warns me about previous tests having disappeared, even some totally
unrelated to the analyzer, although all of them are still here and manually
running
dejagnu on each of them - one by one - gives the exact same output as
control.

So I'm cleaning up my control and patched folders, and re-regstrap
everything.
Something broke and I don't know what.


> BTW, was there any other work of yours that I need to review? (I know
> about the work on placement-new and testsuite migration)
>
> Thanks again for your work on this
> Dave
>
>
Thanks,
Benjamin.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-08-31 11:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-26 14:44 analyzer: Weekly update on extending C++ support (3) Benjamin Priour
2023-08-28 15:13 ` Benjamin Priour
2023-08-29 23:04   ` David Malcolm
2023-08-29 23:01 ` David Malcolm
2023-08-31 11:20   ` Benjamin Priour

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).