On 04/13/2016 11:54 AM, Jason Merrill wrote:
On 04/13/2016 11:32 AM, Jakub Jelinek wrote:
On Tue, Apr 12, 2016 at 04:27:48PM -0400, Jason Merrill wrote:
Unfortunately, a drawback of doing this in the front end is that it's
difficult to warn only about affected cases; the front end doesn't know
what's actually going to be emitted, and has to warn conservatively,
leading
to false positives particularly for inline functions.
The third patch is a sketch of an attempt to address this by delaying
the
warning until expand time. We can't use front end information at
this point
because it has been thrown away by pass_ipa_free_lang_data, so I'm
resorting
to pre-generating the warning and stashing it away. This is several
kinds of
kludge, including hashing on a call location, but it greatly improves
the
accuracy of the warning. I'm hoping that someone will have a better
idea
about how to approach this.
Is the intent that we emit different warnings between e.g.
-fkeep-inline-functions vs. without that, or -O0 vs. -O2?
Do you want to warn just on the out of line definitions (visible
outside of
the containing TU), or also calls to those?
That's what I'm thinking, since only actually emitted definitions/calls
have ABI.
For e.g. warning on calls to those functions it could be similar to the
warning attribute - where we warn if a call to function with that
attribute
is expanded - if it is inlined or optimized away etc., we don't warn.
Ah, that sounds promising, thanks.
Here's a patch along those lines, that passes testing. Does it look OK?
commit 761983a023b5217ef831a43f423779940c788ecf
Author: Jason Merrill <ja...@redhat.com>
Date: Tue Apr 12 13:16:50 2016 -0400
gcc/
* cfgexpand.c (pass_expand::execute): Handle attribute abi_warning.
* expr.c (expand_expr_real_1): Likewise.
gcc/cp/
* call.c (empty_class_msg, mark_for_abi_warning): New.
(build_call_a): Use them.
* decl.c (store_parm_decls): Use mark_for_abi_warning.
* error.c (pp_format_to_string): New.
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 1341c14..4e5ccbb 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see
#include "builtins.h"
#include "tree-chkp.h"
#include "rtl-chkp.h"
+#include "langhooks.h"
/* Some systems use __main in a way incompatible with its use in gcc, in these
cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN to
@@ -6221,6 +6222,15 @@ pass_expand::execute (function *fun)
(int) PARAM_VALUE (PARAM_SSP_BUFFER_SIZE));
}
+ if (warn_abi)
+ if (tree attr = lookup_attribute ("abi_warning",
+ DECL_ATTRIBUTES (current_function_decl)))
+ warning_at (DECL_SOURCE_LOCATION (current_function_decl),
+ OPT_Wabi, "definition of %qs: %s",
+ identifier_to_locale (lang_hooks.decl_printable_name
+ (current_function_decl, 1)),
+ TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
+
/* Set up parameters and prepare for return, for the function. */
expand_function_start (current_function_decl);
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 9cee3b7..97d790f 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -401,7 +401,10 @@ build_call_a (tree function, int n, tree *argarray)
else if (!warned && empty_arg)
{
location_t loc = EXPR_LOC_OR_LOC (empty_arg, input_location);
- warn_empty_class_abi (empty_arg, loc);
+ if (decl && !varargs_function_p (decl))
+ mark_for_abi_warning (decl, empty_arg);
+ else
+ warn_empty_class_abi (empty_arg, loc);
warned = true;
}
}
@@ -7019,8 +7022,19 @@ empty_class_arg (tree val)
return build2 (COMPOUND_EXPR, etype, val, empty);
}
-/* Warn about the change in empty class parameter passing ABI. Returns true
- if we warned. */
+/* Generate a message warning about the change in empty class parameter passing ABI. */
+
+static tree
+empty_class_msg (tree type)
+{
+ if (!TYPE_P (type))
+ type = TREE_TYPE (type);
+
+ return pp_format_to_string ("empty class %qT parameter passing ABI "
+ "changes in -fabi-version=10 (GCC 6)", type);
+}
+
+/* Warn immediately about the change in empty class parameter ABI. */
void
warn_empty_class_abi (tree arg, location_t loc)
@@ -7028,19 +7042,24 @@ warn_empty_class_abi (tree arg, location_t loc)
if (!warn_abi || !abi_version_crosses (10))
return;
- tree type;
- if (TYPE_P (arg))
- type = arg;
- else
- {
- if (TREE_TYPE (arg) == empty_struct_type
- && TREE_CODE (arg) == COMPOUND_EXPR)
- arg = TREE_OPERAND (arg, 0);
- type = TREE_TYPE (arg);
- }
+ warning_at (loc, OPT_Wabi, "%E", empty_class_msg (arg));
+}
- warning_at (loc, OPT_Wabi, "empty class %qT parameter passing ABI "
- "changes in -fabi-version=10 (GCC 6)", type);
+/* Tack a warning about the change in empty class parameter ABI onto FN, so
+ that we get a warning if a definition or call is emitted. */
+
+void
+mark_for_abi_warning (tree fn, tree type)
+{
+ if (!warn_abi || !abi_version_crosses (10))
+ return;
+ if (lookup_attribute ("abi_warning", DECL_ATTRIBUTES (fn)))
+ return;
+
+ tree msg = empty_class_msg (type);
+ msg = build_tree_list (NULL_TREE, msg);
+ DECL_ATTRIBUTES (fn) = tree_cons (get_identifier ("abi_warning"), msg,
+ DECL_ATTRIBUTES (fn));
}
/* Returns the type which will really be used for passing an argument of
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 60abbaa..8d721c7 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5540,6 +5540,7 @@ extern tree build_addr_func (tree, tsubst_flags_t);
extern void set_flags_from_callee (tree);
extern tree build_call_a (tree, int, tree*);
extern tree build_call_n (tree, int, ...);
+extern void mark_for_abi_warning (tree, tree);
extern void warn_empty_class_abi (tree, location_t);
extern bool null_ptr_cst_p (tree);
extern bool null_member_pointer_value_p (tree);
@@ -5895,6 +5896,7 @@ extern bool pedwarn_cxx98 (location_t, int, const char *,
extern location_t location_of (tree);
extern void qualified_name_lookup_error (tree, tree, tree,
location_t);
+extern tree pp_format_to_string (const char *, ...);
/* in except.c */
extern void init_exception_processing (void);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0326900..7099199 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -14357,7 +14357,7 @@ store_parm_decls (tree current_function_parms)
&& (saw_nonempty
|| varargs_function_p (current_function_decl)))
{
- warn_empty_class_abi (parm, DECL_SOURCE_LOCATION (parm));
+ mark_for_abi_warning (current_function_decl, type);
warned = true;
}
}
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index aa5fd41..5aaa177 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3718,3 +3718,25 @@ qualified_name_lookup_error (tree scope, tree name,
suggest_alternatives_for (location, name);
}
}
+
+/* Like error et al, but return the formatted message as a STRING_CST. */
+
+tree
+pp_format_to_string (const char *msg, ...)
+{
+ pretty_printer *pp = global_dc->printer;
+ text_info text;
+ va_list ap;
+
+ va_start (ap, msg);
+ text.err_no = errno;
+ text.args_ptr = ≈
+ text.format_spec = msg;
+ pp_format (pp, &text);
+ pp_output_formatted_text (pp);
+ va_end (ap);
+ const char *fmt = pp_formatted_text (pp);
+ tree str = build_string (strlen (fmt) + 1, fmt);
+ pp_clear_output_area (pp);
+ return str;
+}
diff --git a/gcc/expr.c b/gcc/expr.c
index 29d22b0..f852c51 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -10579,6 +10579,13 @@ expand_expr_real_1 (tree exp, rtx target, machine_mode tmode,
0, "%Kcall to %qs declared with attribute warning: %s",
exp, identifier_to_locale (lang_hooks.decl_printable_name (fndecl, 1)),
TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
+ if (warn_abi && fndecl
+ && (attr = lookup_attribute ("abi_warning",
+ DECL_ATTRIBUTES (fndecl))) != NULL)
+ warning_at (tree_nonartificial_location (exp),
+ OPT_Wabi, "%Kcall to %qs: %s",
+ exp, identifier_to_locale (lang_hooks.decl_printable_name (fndecl, 1)),
+ TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
/* Check for a built-in function. */
if (fndecl && DECL_BUILT_IN (fndecl))
diff --git a/gcc/testsuite/g++.dg/abi/empty20.C b/gcc/testsuite/g++.dg/abi/empty20.C
new file mode 100644
index 0000000..be1e826
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/empty20.C
@@ -0,0 +1,25 @@
+// { dg-options "-Wabi=9 -O0" }
+
+struct A { };
+
+void f(A, A) { } // No warning, trailing parms all empty
+void f(A, A, int) { } // { dg-warning "ABI" }
+__attribute__ ((always_inline))
+inline void f(A a, int i) { } // No warning, always inlined
+__attribute__ ((always_inline))
+inline void f2(A a, int i) // But the call within the fn gets a warning
+{ // when it's inlined into main
+ f(a,a,i); // { dg-warning "ABI" }
+}
+inline void f3(A a, int i) // This one is never called
+{
+ f(a,a,i);
+}
+int main()
+{
+ A a;
+ f(a,a);
+ f(a,a,42); // { dg-warning "ABI" }
+ f(a,42);
+ f2(a,42);
+}
