* [PATCH] analyzer: call off a superseding when diagnostics are unrelated [PR110830]
@ 2023-09-01 19:59 priour.be
2023-09-01 23:48 ` David Malcolm
0 siblings, 1 reply; 6+ messages in thread
From: priour.be @ 2023-09-01 19:59 UTC (permalink / raw)
To: gcc-patches; +Cc: dmalcolm, benjamin priour
From: benjamin priour <vultkayn@gcc.gnu.org>
Hi,
Patch succesfully regstrapped off trunk 7f2ed06ddc825e8a4e0edfd1d66b5156e6dc1d34
on x86_64-linux-gnu.
Is it OK for trunk ?
Thanks,
Benjamin.
Patch below.
---
Before this patch, a saved_diagnostic would supersede another at
the same statement if and only its vfunc supercedes_p returned true
for the other diagnostic's kind.
That both warning were unrelated, that is resolving one would not fix
the other was not considered in making the above choice.
This patch makes it so that two saved_diagnostics taking a different
outcome of at least one common conditional branching cannot supersede
each other.
Signed-off-by: benjamin priour <vultkayn@gcc.gnu.org>
gcc/analyzer/ChangeLog:
PR analyzer/110830
* diagnostic-manager.cc
(compatible_epaths_p): New function.
(saved_diagnostic::supercedes_p): Now calls the above
to determine if the diagnostics do overlap and the superseding
may proceed.
gcc/testsuite/ChangeLog:
PR analyzer/110830
* c-c++-common/analyzer/pr110830.c: New test.
---
gcc/analyzer/diagnostic-manager.cc | 89 +++++++++++++-
.../c-c++-common/analyzer/pr110830.c | 111 ++++++++++++++++++
2 files changed, 199 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/c-c++-common/analyzer/pr110830.c
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 10fea486b8c..7cf181e7972 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -887,6 +887,87 @@ saved_diagnostic::add_duplicate (saved_diagnostic *other)
m_duplicates.safe_push (other);
}
+/* Walk up the two paths to each of their common conditional
+ branching. At each branching, make sure both diagnostics'
+ paths branched similarly. If there is at least one where
+ both paths go down a different outcome, then the paths
+ are incompatible and this function returns FALSE.
+ Otherwise return TRUE.
+
+ Incompatible paths:
+
+ <cond Y>
+ / \
+ / \
+ true false
+ | |
+ ... ...
+ | |
+ ... stmt x
+ |
+ stmt x
+
+ Both LHS_PATH and RHS_PATH final enodes should be
+ over the same gimple statement. */
+
+static bool
+compatible_epath_p (const exploded_path *lhs_path,
+ const exploded_path *rhs_path)
+{
+ gcc_assert (lhs_path);
+ gcc_assert (rhs_path);
+ int i;
+ const exploded_edge *outer_eedge;
+ FOR_EACH_VEC_ELT_REVERSE (lhs_path->m_edges, i, outer_eedge)
+ {
+ const superedge *outer_sedge = outer_eedge->m_sedge;
+ if (!outer_sedge || !outer_eedge->m_src)
+ continue;
+ const program_point &outer_src_point = outer_eedge->m_src->get_point ();
+ switch (outer_src_point.get_kind ())
+ {
+ case PK_AFTER_SUPERNODE:
+ if (const cfg_superedge *cfg_outer_sedge
+ = outer_sedge->dyn_cast_cfg_superedge ())
+ {
+ int j;
+ const exploded_edge *inner_eedge;
+ FOR_EACH_VEC_ELT_REVERSE (rhs_path->m_edges, j, inner_eedge)
+ {
+ const superedge *inner_sedge = inner_eedge->m_sedge;
+ if (!inner_sedge || !inner_eedge->m_src)
+ continue;
+ const program_point &inner_src_point
+ = inner_eedge->m_src->get_point ();
+ switch (inner_src_point.get_kind ())
+ {
+ case PK_AFTER_SUPERNODE:
+ if (inner_src_point.get_stmt ()
+ != outer_src_point.get_stmt ())
+ continue;
+ if (const cfg_superedge *cfg_inner_sedge
+ = inner_sedge->dyn_cast_cfg_superedge ())
+ {
+ if (cfg_inner_sedge->true_value_p ()
+ != cfg_outer_sedge->true_value_p ())
+ return false;
+ }
+ break;
+ default:
+ break;
+ }
+ }
+ }
+ break;
+
+ default:
+ break;
+ }
+ }
+ return true;
+}
+
+
/* Return true if this diagnostic supercedes OTHER, and that OTHER should
therefore not be emitted. */
@@ -896,7 +977,13 @@ saved_diagnostic::supercedes_p (const saved_diagnostic &other) const
/* They should be at the same stmt. */
if (m_stmt != other.m_stmt)
return false;
- return m_d->supercedes_p (*other.m_d);
+ /* return early if OTHER won't be superseded anyway. */
+ if (!m_d->supercedes_p (*other.m_d))
+ return false;
+
+ /* If the two saved_diagnostics' path are not compatible
+ then they cannot supersede one another. */
+ return compatible_epath_p (m_best_epath.get (), other.m_best_epath.get ());
}
/* Move any saved checker_events from this saved_diagnostic to
diff --git a/gcc/testsuite/c-c++-common/analyzer/pr110830.c b/gcc/testsuite/c-c++-common/analyzer/pr110830.c
new file mode 100644
index 00000000000..9f6675ab693
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/pr110830.c
@@ -0,0 +1,111 @@
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *);
+void *malloc(__SIZE_TYPE__);
+
+extern int ext();
+
+void test_supersedes ()
+{
+ int *p = (int *)malloc(sizeof(int));
+ free(p);
+ int x = *p + 4; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-bogus "use of uninitialized value '\\*p" "" { target *-*-* } .-1 } */
+}
+
+int *called_by_test0()
+{
+ int *p = 0;
+ if (ext())
+ {
+ p = (int *)malloc(sizeof(int));
+ free(p);
+ return p;
+ }
+ else
+ return (int *)malloc(sizeof(int));
+}
+
+void test0()
+{
+ int *y = called_by_test0();
+ int x = 0;
+ if (y != 0)
+ x = *y; /* { dg-warning "use after 'free' of 'y'" } */
+ /* { dg-warning "use of uninitialized value '\\*y" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+ free(y); /* { dg-warning "double-'free'" } */
+}
+
+void test1()
+{
+ int *p = 0;
+ if (ext())
+ {
+ p = (int *)malloc(sizeof(int));
+ free(p);
+ }
+ else
+ p = (int *)malloc(sizeof(int));
+
+ int x = 0;
+ if (p != 0)
+ x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-warning "use of uninitialized value '\\*p" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+ free(p); /* { dg-warning "double-'free'" } */
+}
+
+void test2()
+{
+ int *p = 0;
+ p = (int *)malloc(sizeof(int));
+ if (ext())
+ free(p);
+
+ int x = 0;
+ if (p != 0)
+ x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-warning "use of uninitialized value '\\*p" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+ free(p); /* { dg-warning "double-'free'" } */
+}
+
+void test3()
+{
+ int *p = 0;
+ p = (int *)malloc(sizeof(int));
+ int i = 100;
+ while (i--)
+ {
+ int x = 0;
+ if (p != 0)
+ x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-warning "use of uninitialized value '\\*p" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+ p = (int *)malloc(sizeof(int));
+ free(p);
+ }
+
+ free(p); /* { dg-warning "double-'free'" } */
+}
+
+
+void test4()
+{
+ int *p = 0;
+ if (ext())
+ {
+ p = (int *) malloc(sizeof(int));
+ if (ext () > 5)
+ {
+ mal:
+ free (p);
+ }
+ }
+ else {
+ goto mal;
+ }
+
+ int x = 0;
+ if (p != 0)
+ x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-warning "use of uninitialized value '\\*p" "" { target *-*-* } .-1 } */
+ free(p); /* { dg-warning "double-'free'" } */
+}
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] analyzer: call off a superseding when diagnostics are unrelated [PR110830]
2023-09-01 19:59 [PATCH] analyzer: call off a superseding when diagnostics are unrelated [PR110830] priour.be
@ 2023-09-01 23:48 ` David Malcolm
2023-09-06 19:16 ` [PATCH v2] analyzer: Call " priour.be
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2023-09-01 23:48 UTC (permalink / raw)
To: priour.be, gcc-patches; +Cc: benjamin priour
On Fri, 2023-09-01 at 21:59 +0200, priour.be@gmail.com wrote:
> From: benjamin priour <vultkayn@gcc.gnu.org>
>
> Hi,
>
> Patch succesfully regstrapped off trunk
> 7f2ed06ddc825e8a4e0edfd1d66b5156e6dc1d34
> on x86_64-linux-gnu.
>
> Is it OK for trunk ?
>
> Thanks,
> Benjamin.
>
[...snip...]
>
> +/* Walk up the two paths to each of their common conditional
> + branching. At each branching, make sure both diagnostics'
> + paths branched similarly. If there is at least one where
> + both paths go down a different outcome, then the paths
> + are incompatible and this function returns FALSE.
> + Otherwise return TRUE.
> +
> + Incompatible paths:
> +
> + <cond Y>
> + / \
> + / \
> + true false
> + | |
> + ... ...
> + | |
> + ... stmt x
> + |
> + stmt x
> +
> + Both LHS_PATH and RHS_PATH final enodes should be
> + over the same gimple statement. */
> +
> +static bool
> +compatible_epath_p (const exploded_path *lhs_path,
> + const exploded_path *rhs_path)
> +{
> + gcc_assert (lhs_path);
> + gcc_assert (rhs_path);
> + int i;
> + const exploded_edge *outer_eedge;
> + FOR_EACH_VEC_ELT_REVERSE (lhs_path->m_edges, i, outer_eedge)
> + {
> + const superedge *outer_sedge = outer_eedge->m_sedge;
> + if (!outer_sedge || !outer_eedge->m_src)
> + continue;
> + const program_point &outer_src_point = outer_eedge->m_src->get_point ();
> + switch (outer_src_point.get_kind ())
> + {
> + case PK_AFTER_SUPERNODE:
> + if (const cfg_superedge *cfg_outer_sedge
> + = outer_sedge->dyn_cast_cfg_superedge ())
> + {
> + int j;
> + const exploded_edge *inner_eedge;
> + FOR_EACH_VEC_ELT_REVERSE (rhs_path->m_edges, j, inner_eedge)
> + {
> + const superedge *inner_sedge = inner_eedge->m_sedge;
> + if (!inner_sedge || !inner_eedge->m_src)
> + continue;
> + const program_point &inner_src_point
> + = inner_eedge->m_src->get_point ();
> + switch (inner_src_point.get_kind ())
> + {
> + case PK_AFTER_SUPERNODE:
> + if (inner_src_point.get_stmt ()
> + != outer_src_point.get_stmt ())
> + continue;
> + if (const cfg_superedge *cfg_inner_sedge
> + = inner_sedge->dyn_cast_cfg_superedge ())
> + {
> + if (cfg_inner_sedge->true_value_p ()
> + != cfg_outer_sedge->true_value_p ())
> + return false;
> + }
> + break;
> + default:
> + break;
> + }
> + }
> + }
> + break;
> +
> + default:
> + break;
> + }
> + }
> + return true;
> +}
[...snip...]
Thanks for the patch. I think the high-level idea is good, but I'm not
sure the implementation is correct:
- it is O(n^2), where n is the length of exploded_path.
- it walks backwards through the LHS path, and for each eedge from a
PK_AFTER_SUPERNODE it walks backwards from the end of the RHS epath; it
only looks at the "true" flag on CFG edges. I think this works for
simple cases, but the way it restarts the rhs_path iteration from the
end of the rhs_path each time "feels" incorrect.
An eedge from a PK_AFTER_SUPERNODE is presumably just an eedge that has
a non-NULL m_sedge i.e. an exploded edge relating to an edge in the
supergraph. Rather than looking at flags, can we simply compare
superedge pointers? For example, if we care that we followed the
"true" path of a conditional in both lhs and rhs epaths, we can look to
see if both have an eedge where the superedge is the cfg_superedge
wrapping the CFG "true" edge i.e. I think we can simply compare the
superedge pointers.
Or is there some detail here that I'm misunderstanding?
I *think* it's possible to implement it in O(n) with something like
this: (warning: untested code follows!)
/* For compatibility, there should effectively be the same
vector of superedges followed in both epaths.
Walk backwards through each epath, looking at the superedges. */
// FIXME: really? Benjamin, have I understood this correctly?
gcc_assert (lhs_path->length () > 0);
gcc_assert (rhs_path->length () > 0);
int lhs_idx = lhs_path->length () - 1;
int rhs_idx = rhs_path->length () - 1;
while (lhs_idx >= 0 && rhs_idx >= 0)
{
/* Find next LHS superedge, if any. */
while (lhs_idx >= 0)
{
const exploded_edge *lhs_eedge = lhs_path->m_edges[lhs_idx];
if (lhs_eedge->m_sedge)
break;
else
lhs_idx--;
}
/* Find next RHS superedge, if any. */
while (rhs_idx >= 0)
{
const exploded_edge *rhs_eedge = rhs_path->m_edges[rhs_idx];
if (rhs_eedge->m_sedge)
break;
else
rhs_idx--;
}
const exploded_edge *lhs_eedge
(lhs_idx >= 0 ? lhs_path->m_edges[lhs_idx] : nullptr);
const exploded_edge *rhs_eedge
(rhs_idx >= 0 ? rhs_path->m_edges[rhs_idx] : nullptr);
if (lhs_eedge && rhs_edge)
{
/* If we followed different superedges, the paths are
not compatible. */
if (lhs_eedge->m_sedge != rhs_eedge->m_sedge)
return false;
/* Otherwise, we found an (LHS, RHS) pair of eedges
both relating to the same superedge. */
lhs_idx--;
rhs_idx--;
continue;
}
else if (lhs_eedge == nullptr && rhs_eedge == nullptr)
{
/* Finished traversing both epaths; they are compatible. */
return true;
}
/* Otherwise, one epath ran out of superedges before the other;
they are not compatible. */
return false;
}
Does that make any sense, or have I misunderstood?
Thanks
Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] analyzer: Call off a superseding when diagnostics are unrelated [PR110830]
2023-09-01 23:48 ` David Malcolm
@ 2023-09-06 19:16 ` priour.be
2023-09-06 20:30 ` David Malcolm
2023-09-11 8:23 ` Andreas Schwab
0 siblings, 2 replies; 6+ messages in thread
From: priour.be @ 2023-09-06 19:16 UTC (permalink / raw)
To: gcc-patches; +Cc: dmalcolm, benjamin priour
From: benjamin priour <vultkayn@gcc.gnu.org>
Hi,
Second version of this patch after David's suggestions.
Thanks David for pointing out how I could implement it using sedges.
I hadn't thought of them being independent of the exploded path taken,
and unique for a conditional block's outcome. I had mistaken them with
eedges, those that we see in the *exploded*-graph, therefore since the
two saved_diagnostics can have arbitrary different paths I had to
use a nested loop.
It is much more efficient this way.
Regstrapped off trunk a7d052b3200c7928d903a0242b8cfd75d131e374 on
x86_64-linux-gnu.
Is it ready for trunk ?
Thanks,
Benjamin.
Patch below.
---
Before this patch, a saved_diagnostic would supersede another at
the same statement if and only its vfunc supercedes_p returned true
for the other diagnostic's kind.
That both warning were unrelated - i.e. resolving one would not fix
the other - was not considered in making the above choice.
This patch makes it so that two saved_diagnostics taking a different
outcome of at least one common conditional branching cannot supersede
each other.
Signed-off-by: benjamin priour <vultkayn@gcc.gnu.org>
Co-authored-by: david malcolm <dmalcolm@redhat.com>
gcc/analyzer/ChangeLog:
PR analyzer/110830
* diagnostic-manager.cc
(compatible_epaths_p): New function.
(saved_diagnostic::supercedes_p): Now calls the above
to determine if the diagnostics do overlap and the superseding
may proceed.
gcc/testsuite/ChangeLog:
PR analyzer/110830
* c-c++-common/analyzer/pr110830.c: New test.
---
gcc/analyzer/diagnostic-manager.cc | 90 +++++++++++++-
.../c-c++-common/analyzer/pr110830.c | 111 ++++++++++++++++++
2 files changed, 200 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/c-c++-common/analyzer/pr110830.c
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 10fea486b8c..90c56a350e7 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -887,6 +887,88 @@ saved_diagnostic::add_duplicate (saved_diagnostic *other)
m_duplicates.safe_push (other);
}
+/* Walk up the sedges of each of the two paths.
+ If the two sequences of sedges do not perfectly correspond,
+ then paths are incompatible.
+ If there is at least one sedge that either cannot be paired up
+ or its counterpart is not equal, then the paths are incompatible
+ and this function returns FALSE.
+ Otherwise return TRUE.
+
+ Incompatible paths:
+
+ <cond Y>
+ / \
+ / \
+ true false
+ | |
+ ... ...
+ | |
+ ... stmt x
+ |
+ stmt x
+
+ Both LHS_PATH and RHS_PATH final enodes should be
+ over the same gimple statement. */
+
+static bool
+compatible_epath_p (const exploded_path *lhs_path,
+ const exploded_path *rhs_path)
+{
+ gcc_assert (lhs_path);
+ gcc_assert (rhs_path);
+ gcc_assert (rhs_path->length () > 0);
+ gcc_assert (rhs_path->length () > 0);
+ int lhs_eedge_idx = lhs_path->length () -1;
+ int rhs_eedge_idx = rhs_path->length () -1;
+ const exploded_edge *lhs_eedge;
+ const exploded_edge *rhs_eedge;
+
+ while (lhs_eedge_idx >= 0 && rhs_eedge_idx >= 0)
+ {
+ while (lhs_eedge_idx >= 0)
+ {
+ /* Find LHS_PATH's next superedge. */
+ lhs_eedge = lhs_path->m_edges[lhs_eedge_idx];
+ if (lhs_eedge->m_sedge)
+ break;
+ else
+ lhs_eedge_idx--;
+ }
+ while (rhs_eedge_idx >= 0)
+ {
+ /* Find RHS_PATH's next superedge. */
+ rhs_eedge = rhs_path->m_edges[rhs_eedge_idx];
+ if (rhs_eedge->m_sedge)
+ break;
+ else
+ rhs_eedge_idx--;
+ }
+
+ if (lhs_eedge->m_sedge && rhs_eedge->m_sedge)
+ {
+ if (lhs_eedge->m_sedge != rhs_eedge->m_sedge)
+ /* Both superedges do not match.
+ Superedges are not dependent on the exploded path, so even
+ different epaths will have similar sedges if they follow
+ the same outcome of a conditional node. */
+ return false;
+
+ lhs_eedge_idx--;
+ rhs_eedge_idx--;
+ continue;
+ }
+ else if (lhs_eedge->m_sedge == nullptr && rhs_eedge->m_sedge == nullptr)
+ /* Both paths were drained up entirely.
+ No discriminant was found. */
+ return true;
+
+ /* A superedge was found for only one of the two paths. */
+ return false;
+ }
+}
+
+
/* Return true if this diagnostic supercedes OTHER, and that OTHER should
therefore not be emitted. */
@@ -896,7 +978,13 @@ saved_diagnostic::supercedes_p (const saved_diagnostic &other) const
/* They should be at the same stmt. */
if (m_stmt != other.m_stmt)
return false;
- return m_d->supercedes_p (*other.m_d);
+ /* return early if OTHER won't be superseded anyway. */
+ if (!m_d->supercedes_p (*other.m_d))
+ return false;
+
+ /* If the two saved_diagnostics' path are not compatible
+ then they cannot supersede one another. */
+ return compatible_epath_p (m_best_epath.get (), other.m_best_epath.get ());
}
/* Move any saved checker_events from this saved_diagnostic to
diff --git a/gcc/testsuite/c-c++-common/analyzer/pr110830.c b/gcc/testsuite/c-c++-common/analyzer/pr110830.c
new file mode 100644
index 00000000000..f5a39b7a067
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/pr110830.c
@@ -0,0 +1,111 @@
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *);
+void *malloc(__SIZE_TYPE__);
+
+extern int ext();
+
+void test_supersedes ()
+{
+ int *p = (int *)malloc(sizeof(int));
+ free(p);
+ int x = *p + 4; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-bogus "use of uninitialized value '\\*p" "" { target *-*-* } .-1 } */
+}
+
+int *called_by_test0()
+{
+ int *p = 0;
+ if (ext())
+ {
+ p = (int *)malloc(sizeof(int));
+ free(p);
+ return p;
+ }
+ else
+ return (int *)malloc(sizeof(int));
+}
+
+void test0()
+{
+ int *y = called_by_test0();
+ int x = 0;
+ if (y != 0)
+ x = *y; /* { dg-warning "use after 'free' of 'y'" } */
+ /* { dg-warning "use of uninitialized value '\\*y'" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+ free(y); /* { dg-warning "double-'free'" } */
+}
+
+void test1()
+{
+ int *p = 0;
+ if (ext())
+ {
+ p = (int *)malloc(sizeof(int));
+ free(p);
+ }
+ else
+ p = (int *)malloc(sizeof(int));
+
+ int x = 0;
+ if (p != 0)
+ x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-warning "use of uninitialized value '\\*p'" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+ free(p); /* { dg-warning "double-'free'" } */
+}
+
+void test2()
+{
+ int *p = 0;
+ p = (int *)malloc(sizeof(int));
+ if (ext())
+ free(p);
+
+ int x = 0;
+ if (p != 0)
+ x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-warning "use of uninitialized value '\\*p'" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+ free(p); /* { dg-warning "double-'free'" } */
+}
+
+void test3()
+{
+ int *p = 0;
+ p = (int *)malloc(sizeof(int));
+ int i = 100;
+ while (i--)
+ {
+ int x = 0;
+ if (p != 0)
+ x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-warning "use of uninitialized value '\\*p'" "don't supersede warnings with incompatible cfg path" { target *-*-* } .-1 } */
+ p = (int *)malloc(sizeof(int));
+ free(p);
+ }
+
+ free(p); /* { dg-warning "double-'free'" } */
+}
+
+
+void test4()
+{
+ int *p = 0;
+ if (ext())
+ {
+ p = (int *) malloc(sizeof(int));
+ if (ext () > 5)
+ {
+ mal:
+ free (p);
+ }
+ }
+ else {
+ goto mal;
+ }
+
+ int x = 0;
+ if (p != 0)
+ x = *p; /* { dg-warning "use after 'free' of 'p'" } */
+ /* { dg-warning "use of uninitialized value '\\*p'" "" { target *-*-* } .-1 } */
+ free(p); /* { dg-warning "double-'free'" } */
+}
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] analyzer: Call off a superseding when diagnostics are unrelated [PR110830]
2023-09-06 19:16 ` [PATCH v2] analyzer: Call " priour.be
@ 2023-09-06 20:30 ` David Malcolm
2023-09-11 8:23 ` Andreas Schwab
1 sibling, 0 replies; 6+ messages in thread
From: David Malcolm @ 2023-09-06 20:30 UTC (permalink / raw)
To: priour.be, gcc-patches; +Cc: benjamin priour
On Wed, 2023-09-06 at 21:16 +0200, priour.be@gmail.com wrote:
[...snip...]
> Signed-off-by: benjamin priour <vultkayn@gcc.gnu.org>
> Co-authored-by: david malcolm <dmalcolm@redhat.com>
Please also add:
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
[...snip...]
>
> +static bool
> +compatible_epath_p (const exploded_path *lhs_path,
> + const exploded_path *rhs_path)
> +{
> + gcc_assert (lhs_path);
> + gcc_assert (rhs_path);
> + gcc_assert (rhs_path->length () > 0);
> + gcc_assert (rhs_path->length () > 0);
> + int lhs_eedge_idx = lhs_path->length () -1;
> + int rhs_eedge_idx = rhs_path->length () -1;
Minor formatting nit: there should be a space between the '-' and the
'1' in the above lines, hence:
int lhs_eedge_idx = lhs_path->length () - 1;
int rhs_eedge_idx = rhs_path->length () - 1;
[...snip...]
OK for trunk with those changes
Thanks
Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] analyzer: Call off a superseding when diagnostics are unrelated [PR110830]
2023-09-06 19:16 ` [PATCH v2] analyzer: Call " priour.be
2023-09-06 20:30 ` David Malcolm
@ 2023-09-11 8:23 ` Andreas Schwab
2023-09-14 20:39 ` [pushed] analyzer: fix missing return in compatible_epath_p David Malcolm
1 sibling, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2023-09-11 8:23 UTC (permalink / raw)
To: Benjamin Priour via Gcc-patches; +Cc: priour.be, benjamin priour
../../gcc/analyzer/diagnostic-manager.cc: In function 'bool ana::compatible_epath_p(const exploded_path*, const exploded_path*)':
../../gcc/analyzer/diagnostic-manager.cc:969:1: warning: control reaches end of non-void function [-Wreturn-type]
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 6+ messages in thread
* [pushed] analyzer: fix missing return in compatible_epath_p
2023-09-11 8:23 ` Andreas Schwab
@ 2023-09-14 20:39 ` David Malcolm
0 siblings, 0 replies; 6+ messages in thread
From: David Malcolm @ 2023-09-14 20:39 UTC (permalink / raw)
To: Andreas Schwab, gcc-patches; +Cc: priour.be, benjamin priour, David Malcolm
On Mon, 2023-09-11 at 10:23 +0200, Andreas Schwab via Gcc-patches wrote:
> ../../gcc/analyzer/diagnostic-manager.cc: In function 'bool ana::compatible_epath_p(const exploded_path*, const exploded_path*)':
> ../../gcc/analyzer/diagnostic-manager.cc:969:1: warning: control reaches end of non-void function [-Wreturn-type]
Sorry about this; should be fixed by the following patch.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-4005-g59f6185b59f711.
gcc/analyzer/ChangeLog:
* diagnostic-manager.cc (compatible_epath_p): Fix missing return.
---
gcc/analyzer/diagnostic-manager.cc | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index a010f4ba1e1..b3da2a982f2 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -966,6 +966,14 @@ compatible_epath_p (const exploded_path *lhs_path,
/* A superedge was found for only one of the two paths. */
return false;
}
+
+ /* A superedge was found for only one of the two paths. */
+ if (lhs_eedge_idx >= 0 || rhs_eedge_idx >= 0)
+ return false;
+
+ /* Both paths were drained up entirely.
+ No discriminant was found. */
+ return true;
}
--
2.26.3
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-14 20:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-01 19:59 [PATCH] analyzer: call off a superseding when diagnostics are unrelated [PR110830] priour.be
2023-09-01 23:48 ` David Malcolm
2023-09-06 19:16 ` [PATCH v2] analyzer: Call " priour.be
2023-09-06 20:30 ` David Malcolm
2023-09-11 8:23 ` Andreas Schwab
2023-09-14 20:39 ` [pushed] analyzer: fix missing return in compatible_epath_p David Malcolm
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).