Message ID | 1573867416-55618-21-git-send-email-dmalcolm@redhat.com |
---|---|
State | New |
Headers | show |
Series | RFC: Add a static analysis framework to GCC | expand |
On 11/15/19 6:23 PM, David Malcolm wrote: > gcc/ChangeLog: > * builtins.def (BUILT_IN_ANALYZER_BREAK): New builtin. > (BUILT_IN_ANALYZER_DUMP): New builtin. > (BUILT_IN_ANALYZER_DUMP_EXPLODED_NODES): New builtin. > (BUILT_IN_ANALYZER_DUMP_NUM_HEAP_REGIONS): New builtin. > (BUILT_IN_ANALYZER_DUMP_PATH): New builtin. > (BUILT_IN_ANALYZER_DUMP_REGION_MODEL): New builtin. > (BUILT_IN_ANALYZER_EVAL): New builtin. I have two comments: 1) builtins.def is getting very large and I think it would make it easier to work with if new related sets of built-ins were added in their own files. (In full disclosure, I tried breaking it up once and ran into weird build errors and gave up, so it may not be entirely straightforward, not like making other changes to the compiler ;) 2) Since there are no changes to the manual documenting these, I assume they are internal only, not to be called by user code? (Should they be marked as internal functions then, to prevent them from being called?) Even so, it would help GCC developers to document them somewhere and add a comment pointing at the documentation here. If they can be called by user code, I think they should be mentioned in the manual even if (and I'd say especially if) we don't users calling them directly. Martin > --- > gcc/builtins.def | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/gcc/builtins.def b/gcc/builtins.def > index d8233f5..f34e95f 100644 > --- a/gcc/builtins.def > +++ b/gcc/builtins.def > @@ -1107,4 +1107,37 @@ DEF_GCC_BUILTIN (BUILT_IN_LINE, "LINE", BT_FN_INT, ATTR_NOTHROW_LEAF_LIST) > /* HSAIL/BRIG frontend builtins. */ > #include "brig-builtins.def" > > + > +/* Analyzer builtins. */ > +DEF_BUILTIN (BUILT_IN_ANALYZER_BREAK, "__analyzer_break", > + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, > + false, false, false, ATTR_NULL, true, true) > + > +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP, "__analyzer_dump", > + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, > + false, false, false, ATTR_NULL, true, true) > + > +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP_EXPLODED_NODES, > + "__analyzer_dump_exploded_nodes", > + BUILT_IN_NORMAL, BT_FN_VOID_INT, BT_LAST, > + false, false, false, ATTR_NULL, true, true) > + > +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP_NUM_HEAP_REGIONS, > + "__analyzer_dump_num_heap_regions", > + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, > + false, false, false, ATTR_NULL, true, true) > + > +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP_PATH, "__analyzer_dump_path", > + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, > + false, false, false, ATTR_NULL, true, true) > + > +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP_REGION_MODEL, > + "__analyzer_dump_region_model", > + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, > + false, false, false, ATTR_NULL, true, true) > + > +DEF_BUILTIN (BUILT_IN_ANALYZER_EVAL, "__analyzer_eval", > + BUILT_IN_NORMAL, BT_FN_VOID_INT, BT_LAST, > + false, false, false, ATTR_NULL, true, true) > + > #undef DEF_BUILTIN >
On Wed, 2019-12-04 at 11:11 -0700, Martin Sebor wrote: > On 11/15/19 6:23 PM, David Malcolm wrote: > > gcc/ChangeLog: > > * builtins.def (BUILT_IN_ANALYZER_BREAK): New builtin. > > (BUILT_IN_ANALYZER_DUMP): New builtin. > > (BUILT_IN_ANALYZER_DUMP_EXPLODED_NODES): New builtin. > > (BUILT_IN_ANALYZER_DUMP_NUM_HEAP_REGIONS): New builtin. > > (BUILT_IN_ANALYZER_DUMP_PATH): New builtin. > > (BUILT_IN_ANALYZER_DUMP_REGION_MODEL): New builtin. > > (BUILT_IN_ANALYZER_EVAL): New builtin. > > I have two comments: > > 1) builtins.def is getting very large and I think it would make > it easier to work with if new related sets of built-ins were added > in their own files. (In full disclosure, I tried breaking it up > once and ran into weird build errors and gave up, so it may not > be entirely straightforward, not like making other changes to > the compiler ;) > > 2) Since there are no changes to the manual documenting these, > I assume they are internal only, not to be called by user code? > (Should they be marked as internal functions then, to prevent > them from being called?) Even so, it would help GCC developers > to document them somewhere and add a comment pointing at > the documentation here. If they can be called by user code, > I think they should be mentioned in the manual even if (and > I'd say especially if) we don't users calling them directly. Thanks for the feedback. FWIW I've reworked things so that the analyzer no longer has builtins, but instead recognizes certain "magic" function names (for use in DejaGnu and when debugging): https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01310.html Dave
diff --git a/gcc/builtins.def b/gcc/builtins.def index d8233f5..f34e95f 100644 --- a/gcc/builtins.def +++ b/gcc/builtins.def @@ -1107,4 +1107,37 @@ DEF_GCC_BUILTIN (BUILT_IN_LINE, "LINE", BT_FN_INT, ATTR_NOTHROW_LEAF_LIST) /* HSAIL/BRIG frontend builtins. */ #include "brig-builtins.def" + +/* Analyzer builtins. */ +DEF_BUILTIN (BUILT_IN_ANALYZER_BREAK, "__analyzer_break", + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, + false, false, false, ATTR_NULL, true, true) + +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP, "__analyzer_dump", + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, + false, false, false, ATTR_NULL, true, true) + +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP_EXPLODED_NODES, + "__analyzer_dump_exploded_nodes", + BUILT_IN_NORMAL, BT_FN_VOID_INT, BT_LAST, + false, false, false, ATTR_NULL, true, true) + +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP_NUM_HEAP_REGIONS, + "__analyzer_dump_num_heap_regions", + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, + false, false, false, ATTR_NULL, true, true) + +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP_PATH, "__analyzer_dump_path", + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, + false, false, false, ATTR_NULL, true, true) + +DEF_BUILTIN (BUILT_IN_ANALYZER_DUMP_REGION_MODEL, + "__analyzer_dump_region_model", + BUILT_IN_NORMAL, BT_FN_VOID, BT_LAST, + false, false, false, ATTR_NULL, true, true) + +DEF_BUILTIN (BUILT_IN_ANALYZER_EVAL, "__analyzer_eval", + BUILT_IN_NORMAL, BT_FN_VOID_INT, BT_LAST, + false, false, false, ATTR_NULL, true, true) + #undef DEF_BUILTIN