[no subject]
(หากคุณกำลังมองหาเงินทุนหมุนเวียนระยะสั้น) สำหรับผู้ประกอบการ โรงงานฯ หจก. บริษัท ธุรกิจ SME อนุมัติง่ายกว่าธนาคาร | ไม่เช็คบูโร | ลดต้น ลดดอกเบี้ย | ไม่ต้องมีคนค้ำ | อนุมัติทันที หลังส่งเอกสารครบถ้วน 1 ช.ม หากคุณสนใจบริการของเรา โทรด่วนหาเรา 📞โทร 082 5928519 คุณเอก 📞โทร 063 2543219 ตะวัน 💬ไลน์ไอดี esc.credit ✅ดอกเบี้ยเริ่มต้น 1.5% ✅ปิดยอดได้ตลอดเวลา ไม่ต้องรอให้ครบสัญญา ✅ฟรีค่าธรรมเนียม ไม่เรียกเก็บเงินก่อนทำสัญญาทุกกรณี
isl 0.26 C++17 issues when bootstrapping with GCC
Hi, I tried adding isl 0.26 to a 13.2 GCC build using Iain's macOS aarch64 patches: https://github.com/iains/gcc-13-branch It seems that the bootstrap's `CXX='g++ -std=c++11'` confuses isl's build where C++17 is expected to work by disabling C++17 behind its back. Should GCC not add this flag for its dependencies but only its own build perhaps? isl bug report thread (with isl configure logs and the like): https://groups.google.com/g/isl-development/c/ShnQcW_35ZQ Thanks, --Ben
Re: isl 0.26 C++17 issues when bootstrapping with GCC
> Am 29.08.2023 um 16:30 schrieb Ben Boeckel via Gcc : > > Hi, > > I tried adding isl 0.26 to a 13.2 GCC build using Iain's macOS aarch64 > patches: > >https://github.com/iains/gcc-13-branch > > It seems that the bootstrap's `CXX='g++ -std=c++11'` confuses isl's > build where C++17 is expected to work by disabling C++17 behind its > back. Should GCC not add this flag for its dependencies but only its own > build perhaps? I suppose for bootstrapping we could disable ISL during stage1 since it enables an optional feature only. Other than that GCC only requires a C++11 compiler for building, so ISL breaks that constraint with requiring C++17. Richard > isl bug report thread (with isl configure logs and the like): > >https://groups.google.com/g/isl-development/c/ShnQcW_35ZQ > > Thanks, > > --Ben
[PATCH] analyzer: implement reference count checking for CPython plugin [PR107646]
Additionally, by using the old model and the pointer per your suggestion,
we are able to find the representative tree and emit a more accurate diagnostic!
rc3.c:23:10: warning: expected ‘item’ to have reference count: ‘1’ but
ob_refcnt field is: ‘2’
23 | return list;
| ^~~~
‘create_py_object’: events 1-4
|
|4 | PyObject* item = PyLong_FromLong(3);
| |^~
| ||
| |(1) when ‘PyLong_FromLong’ succeeds
|5 | PyObject* list = PyList_New(1);
| |~
| ||
| |(2) when ‘PyList_New’ succeeds
|..
| 14 | PyList_Append(list, item);
| | ~
| | |
| | (3) when ‘PyList_Append’ succeeds, moving buffer
|..
| 23 | return list;
| |
| | |
| | (4) here
|
If a representative tree is not found, I decided we should just bail out
of emitting a diagnostic for now, to avoid confusing the user on what
the problem is.
I've attached the patch for this (on top of the previous one) below. If
it also looks good, I can merge it with the last patch and push it in at
the same time.
Best,
Eric
---
gcc/analyzer/region-model.cc | 3 +-
gcc/analyzer/region-model.h | 7 ++--
.../gcc.dg/plugin/analyzer_cpython_plugin.c | 35 +++
3 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index eb4f976b83a..c1d266d351b 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -5391,6 +5391,7 @@ region_model::pop_frame (tree result_lvalue,
{
gcc_assert (m_current_frame);
+ const region_model pre_popped_model = *this;
const frame_region *frame_reg = m_current_frame;
/* Notify state machines. */
@@ -5424,7 +5425,7 @@ region_model::pop_frame (tree result_lvalue,
}
unbind_region_and_descendents (frame_reg,POISON_KIND_POPPED_STACK);
- notify_on_pop_frame (this, retval, ctxt);
+ notify_on_pop_frame (this, &pre_popped_model, retval, ctxt);
}
/* Get the number of frames in this region_model's stack. */
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index 440ea6d828d..b89c6f6c649 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -237,6 +237,7 @@ public:
struct append_regions_cb_data;
typedef void (*pop_frame_callback) (const region_model *model,
+ const region_model *prev_model,
const svalue *retval,
region_model_context *ctxt);
@@ -543,11 +544,13 @@ class region_model
}
static void
- notify_on_pop_frame (const region_model *model, const svalue *retval,
+ notify_on_pop_frame (const region_model *model,
+ const region_model *prev_model,
+ const svalue *retval,
region_model_context *ctxt)
{
for (auto &callback : pop_frame_callbacks)
- callback (model, retval, ctxt);
+ callback (model, prev_model, retval, ctxt);
}
private:
diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
index b2caed8fc1b..6f0a355fe30 100644
--- a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
+++ b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c
@@ -318,11 +318,10 @@ public:
auto actual_refcnt
= m_actual_refcnt->dyn_cast_constant_svalue ()->get_constant ();
auto ob_refcnt = m_ob_refcnt->dyn_cast_constant_svalue ()->get_constant ();
-warned = warning_meta (
- rich_loc, m, get_controlling_option (),
- "expected to have "
- "reference count: %qE but ob_refcnt field is: %qE",
- actual_refcnt, ob_refcnt);
+warned = warning_meta (rich_loc, m, get_controlling_option (),
+ "expected %qE to have "
+ "reference count: %qE but ob_refcnt field is: %qE",
+ m_reg_tree, actual_refcnt, ob_refcnt);
// location_t loc = rich_loc->get_loc ();
// foo (loc);
@@ -425,7 +424,8 @@ count_expected_pyobj_references (const region_model *model,
/* Compare ob_refcnt field vs the actual reference count of a region */
static void
-check_refcnt (const region_model *model, region_model_context *ctxt,
+check_refcnt (const region_model *model, const region_model *old_model,
+ region_model_context *ctxt,
const hash_map::iterator::reference_pair region_refcnt)
{
@@ -438,8 +438,11 @@ check_refcnt (const region_model *model,
region_model_context *ctxt,
if (ob_refcnt_sval != actual_refcnt_sval)
{
-// todo: fix this
-tree reg_tree = model->get_representativ
Re: [PATCH] analyzer: implement reference count checking for CPython plugin [PR107646]
On Tue, 2023-08-29 at 00:31 -0400, Eric Feng wrote:
> Hi Dave,
Hi Eric.
Thanks for the updated patch.
A few nits below; this is OK for trunk with them fixed...
[...snip...]
>
> gcc/analyzer/ChangeLog:
> PR analyzer/107646
> * engine.cc (impl_region_model_context::warn): New optional parameter.
> * exploded-graph.h (class impl_region_model_context): Likewise.
> * region-model.cc (region_model::pop_frame): New callback feature for
> * region_model::pop_frame.
> * region-model.h (struct append_regions_cb_data): Likewise.
> (class region_model): Likewise.
> (class region_model_context): New optional parameter.
> (class region_model_context_decorator): Likewise.
>
> gcc/testsuite/ChangeLog:
> PR analyzer/107646
> * gcc.dg/plugin/analyzer_cpython_plugin.c: Implements reference count
> * checking for PyObjects.
> * gcc.dg/plugin/cpython-plugin-test-2.c: Moved to...
> * gcc.dg/plugin/cpython-plugin-test-PyList_Append.c: ...here (and
> * added more tests).
> * gcc.dg/plugin/cpython-plugin-test-1.c: Moved to...
> * gcc.dg/plugin/cpython-plugin-test-no-plugin.c: ...here (and added
> * more tests).
> * gcc.dg/plugin/plugin.exp: New tests.
> * gcc.dg/plugin/cpython-plugin-test-PyList_New.c: New test.
> * gcc.dg/plugin/cpython-plugin-test-PyLong_FromLong.c: New test.
> * gcc.dg/plugin/cpython-plugin-test-refcnt-checking.c: New test.
The ChangeLog formatting here seems wrong; lines starting with a '*'
should refer to a filename. Continuation lines begin with just a tab
character.
[...snip...]
> diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
> index 10b2a59e787..440ea6d828d 100644
> --- a/gcc/analyzer/region-model.h
> +++ b/gcc/analyzer/region-model.h
[...snip...]
> @@ -840,7 +865,8 @@ private:
> class region_model_context_decorator : public region_model_context
> {
> public:
> - bool warn (std::unique_ptr d) override
> + bool warn (std::unique_ptr d,
> + const stmt_finder *custom_finder)
>{
> if (m_inner)
>return m_inner->warn (std::move (d));
This should presumably pass the custom_finder on to the 2nd argument of
m_inner->warn, rather than have the inner call to warn implicitly use
the NULL default arg.
[...snip...]
> diff --git a/gcc/testsuite/gcc.dg/plugin/cpython-plugin-test-1.c
> b/gcc/testsuite/gcc.dg/plugin/cpython-plugin-test-no-plugin.c
> similarity index 100%
> rename from gcc/testsuite/gcc.dg/plugin/cpython-plugin-test-1.c
> rename to gcc/testsuite/gcc.dg/plugin/cpython-plugin-test-no-plugin.c
Looks like
"-no-Python-h.c"
would be a better suffix than
"-no-plugin.c"
as it's the include that's missing, not the plugin.
[...snip...]
> diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp
> b/gcc/testsuite/gcc.dg/plugin/plugin.exp
> index e1ed2d2589e..cbef6da8d86 100644
> --- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
> +++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
> @@ -161,8 +161,9 @@ set plugin_test_list [list \
> taint-CVE-2011-0521-6.c \
> taint-antipatterns-1.c } \
> { analyzer_cpython_plugin.c \
> - cpython-plugin-test-1.c \
> - cpython-plugin-test-2.c } \
> + cpython-plugin-test-PyList_Append.c \
> + cpython-plugin-test-PyList_New.c \
> + cpython-plugin-test-PyLong_FromLong.c } \
Looks like this is missing:
cpython-plugin-test-no-plugin.c
and
cpython-plugin-test-refcnt-checking.c
(though as noted above"cpython-plugin-test-no-Python-h.c" would be a
better name for the former)
so it wasn't actually compiling these tests.
Be sure to doublecheck that these tests pass when updating.
[...snip...]
OK for trunk with the above nits fixed.
Dave
Re: [PATCH] analyzer: implement reference count checking for CPython plugin [PR107646]
On Tue, 2023-08-29 at 13:28 -0400, Eric Feng wrote: > Additionally, by using the old model and the pointer per your > suggestion, > we are able to find the representative tree and emit a more accurate > diagnostic! > > rc3.c:23:10: warning: expected ‘item’ to have reference count: ‘1’ > but ob_refcnt field is: ‘2’ > 23 | return list; > | ^~~~ > ‘create_py_object’: events 1-4 > | > | 4 | PyObject* item = PyLong_FromLong(3); > | | ^~ > | | | > | | (1) when ‘PyLong_FromLong’ succeeds > | 5 | PyObject* list = PyList_New(1); > | | ~ > | | | > | | (2) when ‘PyList_New’ succeeds > |.. > | 14 | PyList_Append(list, item); > | | ~ > | | | > | | (3) when ‘PyList_Append’ succeeds, moving buffer > |.. > | 23 | return list; > | | > | | | > | | (4) here > | Excellent, that's a big improvement. > > If a representative tree is not found, I decided we should just bail > out > of emitting a diagnostic for now, to avoid confusing the user on what > the problem is. Fair enough. > > I've attached the patch for this (on top of the previous one) below. > If > it also looks good, I can merge it with the last patch and push it in > at > the same time. I don't mind either way, but please can you update the tests so that we have some automated test coverage that the correct name is being printed in the warning. Thanks Dave
Re: analyzer: Weekly update on extending C++ support (3)
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
Re: analyzer: Weekly update on extending C++ support (3)
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:
>
> :
> i_23 = EVP_PKEY_asn1_get_count ();
> goto ; [INV]
>
> :
> ameth_27 = EVP_PKEY_asn1_get0 (i_24);
> _2 = ameth_27->pkey_flags;
> _3 = _2 & 1;
> if (_3 != 0)
> goto ; [INV]
> else
> goto ; [INV]
>
> :
> // predicted unlikely by continue predictor.
> goto ; [INV]
>
> :
> _28 = ameth_27;
> goto ; [INV]
>
> :
> # i_5 = PHI
> i.4_4 = i_5;
> i_24 = i.4_4 + -1;
> if (i.4_4 > 0)
> goto ; [INV]
> else
> goto ; [INV]
>
> ... and as C++
>
> :
> i_23 = EVP_PKEY_asn1_get_count ();
> goto ; [INV]
>
> :
> ameth_28 = EVP_PKEY_asn1_get0 (i_24);
> _2 = ameth_28->pkey_flags;
> _3 = _2 & 1;
> if (_3 != 0)
> goto ; [INV]
> else
> goto ; [INV]
>
> :
> // predicted unlikely by continue predictor.
> goto ; [INV]
>
> :
> _29 = ameth_28;
> goto ; [INV]
>
> :
> # i_5 = PHI
> 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 ; [INV]
> else
> goto ; [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
Re: isl 0.26 C++17 issues when bootstrapping with GCC
On Tue, Aug 29, 2023 at 18:57:37 +0200, Richard Biener wrote: > I suppose for bootstrapping we could disable ISL during stage1 since > it enables an optional feature only. Other than that GCC only > requires a C++11 compiler for building, so ISL breaks that constraint > with requiring C++17. Note that it doesn't *require* it per sé; the tests that try it are compiled if C++17 support was detected at all. The headers seem to just have optional header-only `std::any`-using APIs if C++17 is around. `isl` supporting a flag to disable the tests would also work, but that doesn't fix 0.26. It also doesn't mean it won't start requiring C++17 at some point in the future. In light of that, I feel that skipping it for bootstrap is probably the right solution here. Alas, my skill with autotools is closer to the caveman-with-club level rather than that of a surgeon. --Ben
vect_recog_mixed_size_cond_pattern dead code now
Hi, I was reading some code in tree-vect-patterns.cc and I came across vect_recog_mixed_size_cond_pattern . The code tries to handle comparisons from COND_EXPR but that cannot happen any more (after r13-707-g68e0063397ba82). Should this code be removed now? Thanks, Andrew
