diff mbox series

[20/49] analyzer: new builtins

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

Commit Message

David Malcolm Nov. 16, 2019, 1:23 a.m. UTC
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.
---
 gcc/builtins.def | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Martin Sebor Dec. 4, 2019, 6:11 p.m. UTC | #1
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
>
David Malcolm Dec. 19, 2019, 3:13 p.m. UTC | #2
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 mbox series

Patch

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