In r14-1497-gef768035ae8090 I added some support to the analyzer for
__atomic_ builtins (enough to fix false positives I was seeing in
my integration tests).
Unfortunately I messed up the implementation of
__atomic_{exchange,load,store}, leading to ICEs seen in
PR analyzer/114286.
Fixed thusly, fixing the ICEs. Given that we're in stage 4, the patch
doesn't add support for any of the various __atomic_compare_exchange
builtins, so that these continue to fall back to the analyzer's
"anything could happen" handling of unknown functions.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Successful run of analyzer integration tests on x86_64-pc-linux-gnu.
Pushed to trunk as r14-9544-gc7a774edbf802d.
gcc/analyzer/ChangeLog:
PR analyzer/114286
* kf.cc (class kf_atomic_exchange): Reimplement based on signature
seen in gimple, rather than user-facing signature.
(class kf_atomic_load): Likewise.
(class kf_atomic_store): New.
(register_atomic_builtins): Register kf_atomic_store.
gcc/testsuite/ChangeLog:
PR analyzer/114286
* c-c++-common/analyzer/atomic-builtins-pr114286.c: New test.
Signed-off-by: David Malcolm <dmalc...@redhat.com>
---
gcc/analyzer/kf.cc | 135 +++++++++++++-----
.../analyzer/atomic-builtins-pr114286.c | 48 +++++++
2 files changed, 150 insertions(+), 33 deletions(-)
create mode 100644
gcc/testsuite/c-c++-common/analyzer/atomic-builtins-pr114286.c
diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc
index ed48ffbcba2..d197ccbd0f0 100644
--- a/gcc/analyzer/kf.cc
+++ b/gcc/analyzer/kf.cc
@@ -116,39 +116,54 @@ kf_alloca::impl_call_pre (const call_details &cd) const
cd.maybe_set_lhs (ptr_sval);
}
-/* Handler for:
- void __atomic_exchange (type *ptr, type *val, type *ret, int memorder). */
+/* Handler for __atomic_exchange.
+ Although the user-facing documentation specifies it as having this
+ signature:
+ void __atomic_exchange (type *ptr, type *val, type *ret, int memorder)
+
+ by the time the C/C++ frontends have acted on it, any calls that
+ can't be mapped to a _N variation end up with this signature:
+
+ void
+ __atomic_exchange (size_t sz, void *ptr, void *val, void *ret,
+ int memorder)
+
+ as seen in the gimple seen by the analyzer, and as specified
+ in sync-builtins.def. */
class kf_atomic_exchange : public internal_known_function
{
public:
/* This is effectively:
- *RET = *PTR;
- *PTR = *VAL;
+ tmpA = *PTR;
+ tmpB = *VAL;
+ *PTR = tmpB;
+ *RET = tmpA;
*/
void impl_call_pre (const call_details &cd) const final override
{
- const svalue *ptr_ptr_sval = cd.get_arg_svalue (0);
- tree ptr_ptr_tree = cd.get_arg_tree (0);
- const svalue *val_ptr_sval = cd.get_arg_svalue (1);
- tree val_ptr_tree = cd.get_arg_tree (1);
- const svalue *ret_ptr_sval = cd.get_arg_svalue (2);
- tree ret_ptr_tree = cd.get_arg_tree (2);
+ const svalue *num_bytes_sval = cd.get_arg_svalue (0);
+ const svalue *ptr_sval = cd.get_arg_svalue (1);
+ tree ptr_tree = cd.get_arg_tree (1);
+ const svalue *val_sval = cd.get_arg_svalue (2);
+ tree val_tree = cd.get_arg_tree (2);
+ const svalue *ret_sval = cd.get_arg_svalue (3);
+ tree ret_tree = cd.get_arg_tree (3);
/* Ignore the memorder param. */
region_model *model = cd.get_model ();
region_model_context *ctxt = cd.get_ctxt ();
- const region *val_region
- = model->deref_rvalue (val_ptr_sval, val_ptr_tree, ctxt);
- const svalue *star_val_sval = model->get_store_value (val_region, ctxt);
- const region *ptr_region
- = model->deref_rvalue (ptr_ptr_sval, ptr_ptr_tree, ctxt);
- const svalue *star_ptr_sval = model->get_store_value (ptr_region, ctxt);
- const region *ret_region
- = model->deref_rvalue (ret_ptr_sval, ret_ptr_tree, ctxt);
- model->set_value (ptr_region, star_val_sval, ctxt);
- model->set_value (ret_region, star_ptr_sval, ctxt);
+ const region *ptr_reg = model->deref_rvalue (ptr_sval, ptr_tree, ctxt);
+ const region *val_reg = model->deref_rvalue (val_sval, val_tree, ctxt);
+ const region *ret_reg = model->deref_rvalue (ret_sval, ret_tree, ctxt);
+
+ const svalue *tmp_a_sval
+ = model->read_bytes (ptr_reg, ptr_tree, num_bytes_sval, ctxt);
+ const svalue *tmp_b_sval
+ = model->read_bytes (val_reg, val_tree, num_bytes_sval, ctxt);
+ model->write_bytes (ptr_reg, num_bytes_sval, tmp_b_sval, ctxt);
+ model->write_bytes (ret_reg, num_bytes_sval, tmp_a_sval, ctxt);
}
};
@@ -265,32 +280,85 @@ private:
enum tree_code m_op;
};
-/* Handler for:
- void __atomic_load (type *ptr, type *ret, int memorder). */
+/* Handler for __atomic_load.
+ Although the user-facing documentation specifies it as having this
+ signature:
+
+ void __atomic_load (type *ptr, type *ret, int memorder)
+
+ by the time the C/C++ frontends have acted on it, any calls that
+ can't be mapped to a _N variation end up with this signature:
+
+ void __atomic_load (size_t sz, const void *src, void *dst, int memorder);
+
+ as seen in the gimple seen by the analyzer, and as specified
+ in sync-builtins.def. */
class kf_atomic_load : public internal_known_function
{
public:
/* This is effectively:
- *RET = *PTR;
+ memmove (dst, src, sz);
*/
void impl_call_pre (const call_details &cd) const final override
{
- const svalue *ptr_ptr_sval = cd.get_arg_svalue (0);
- tree ptr_ptr_tree = cd.get_arg_tree (0);
- const svalue *ret_ptr_sval = cd.get_arg_svalue (1);
- tree ret_ptr_tree = cd.get_arg_tree (1);
+ const svalue *num_bytes_sval = cd.get_arg_svalue (0);
+ const svalue *src_sval = cd.get_arg_svalue (1);
+ tree src_tree = cd.get_arg_tree (1);
+ const svalue *dst_sval = cd.get_arg_svalue (2);
+ tree dst_tree = cd.get_arg_tree (2);
/* Ignore the memorder param. */
region_model *model = cd.get_model ();
region_model_context *ctxt = cd.get_ctxt ();
- const region *ptr_region
- = model->deref_rvalue (ptr_ptr_sval, ptr_ptr_tree, ctxt);
- const svalue *star_ptr_sval = model->get_store_value (ptr_region, ctxt);
- const region *ret_region
- = model->deref_rvalue (ret_ptr_sval, ret_ptr_tree, ctxt);
- model->set_value (ret_region, star_ptr_sval, ctxt);
+ const region *dst_reg = model->deref_rvalue (dst_sval, dst_tree, ctxt);
+ const region *src_reg = model->deref_rvalue (src_sval, src_tree, ctxt);
+
+ const svalue *data_sval
+ = model->read_bytes (src_reg, src_tree, num_bytes_sval, ctxt);
+ model->write_bytes (dst_reg, num_bytes_sval, data_sval, ctxt);
+ }
+};
+
+/* Handler for __atomic_store.
+ Although the user-facing documentation specifies it as having this
+ signature:
+
+ void __atomic_store (type *ptr, type *val, int memorder)
+
+ by the time the C/C++ frontends have acted on it, any calls that
+ can't be mapped to a _N variation end up with this signature:
+
+ void __atomic_store (size_t sz, type *dst, type *src, int memorder)
+
+ as seen in the gimple seen by the analyzer, and as specified
+ in sync-builtins.def. */
+
+class kf_atomic_store : public internal_known_function
+{
+public:
+ /* This is effectively:
+ memmove (dst, src, sz);
+ */
+ void impl_call_pre (const call_details &cd) const final override
+ {
+ const svalue *num_bytes_sval = cd.get_arg_svalue (0);
+ const svalue *dst_sval = cd.get_arg_svalue (1);
+ tree dst_tree = cd.get_arg_tree (1);
+ const svalue *src_sval = cd.get_arg_svalue (2);
+ tree src_tree = cd.get_arg_tree (2);
+ /* Ignore the memorder param. */
+
+ region_model *model = cd.get_model ();
+ region_model_context *ctxt = cd.get_ctxt ();
+
+ const region *dst_reg = model->deref_rvalue (dst_sval, dst_tree, ctxt);
+ const region *src_reg = model->deref_rvalue (src_sval, src_tree, ctxt);
+
+ const svalue *data_sval
+ = model->read_bytes (src_reg, src_tree, num_bytes_sval, ctxt);
+ model->write_bytes (dst_reg, num_bytes_sval, data_sval, ctxt);
}
};
@@ -2021,6 +2089,7 @@ register_atomic_builtins (known_function_manager &kfm)
kfm.add (BUILT_IN_ATOMIC_LOAD_4, make_unique<kf_atomic_load_n> ());
kfm.add (BUILT_IN_ATOMIC_LOAD_8, make_unique<kf_atomic_load_n> ());
kfm.add (BUILT_IN_ATOMIC_LOAD_16, make_unique<kf_atomic_load_n> ());
+ kfm.add (BUILT_IN_ATOMIC_STORE, make_unique<kf_atomic_store> ());
kfm.add (BUILT_IN_ATOMIC_STORE_N, make_unique<kf_atomic_store_n> ());
kfm.add (BUILT_IN_ATOMIC_STORE_1, make_unique<kf_atomic_store_n> ());
kfm.add (BUILT_IN_ATOMIC_STORE_2, make_unique<kf_atomic_store_n> ());
diff --git a/gcc/testsuite/c-c++-common/analyzer/atomic-builtins-pr114286.c
b/gcc/testsuite/c-c++-common/analyzer/atomic-builtins-pr114286.c
new file mode 100644
index 00000000000..1ff47ffaec8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/atomic-builtins-pr114286.c
@@ -0,0 +1,48 @@
+#include "analyzer-decls.h"
+
+struct S { long long a[16]; } s;
+
+struct S
+test_atomic_load (void)
+{
+ struct S r;
+ __atomic_load (&s, &r, __ATOMIC_RELAXED);
+ __analyzer_eval (r.a[0] == s.a[0]); /* { dg-warning "TRUE" } */
+ __analyzer_eval (r.a[15] == s.a[15]); /* { dg-warning "TRUE" } */
+ return r;
+}
+
+void
+test_atomic_store (struct S x)
+{
+ __atomic_store (&s, &x, __ATOMIC_RELAXED);
+ __analyzer_eval (s.a[0] == x.a[0]); /* { dg-warning "TRUE" } */
+ __analyzer_eval (s.a[15] == x.a[15]); /* { dg-warning "TRUE" } */
+}
+
+struct S
+test_atomic_exchange (struct S x)
+{
+ struct S init_x, init_s;
+ struct S r;
+
+ /* Capture initial values of x and s for comparison below. */
+ __atomic_load (&x, &init_x, __ATOMIC_RELAXED);
+ __atomic_load (&s, &init_s, __ATOMIC_RELAXED);
+
+ __atomic_exchange (&s, &x, &r, __ATOMIC_RELAXED);
+
+ __analyzer_eval (s.a[0] == init_x.a[0]); /* { dg-warning "TRUE" } */
+ __analyzer_eval (s.a[15] == init_x.a[15]); /* { dg-warning "TRUE" } */
+ __analyzer_eval (r.a[0] == init_s.a[0]); /* { dg-warning "TRUE" } */
+ __analyzer_eval (r.a[15] == init_s.a[15]); /* { dg-warning "TRUE" } */
+
+ return r;
+}
+
+int
+test_atomic_compare_exchange (struct S *e, struct S *d)
+{
+ return __atomic_compare_exchange (&s, e, d, 0,
+ __ATOMIC_RELAXED, __ATOMIC_RELAXED);
+}
--
2.26.3
