On Fri, Mar 02, 2018 at 09:31:16AM +0100, Richard Biener wrote:
> On Fri, 2 Mar 2018, Jakub Jelinek wrote:
>
> > On Fri, Mar 02, 2018 at 09:15:07AM +0100, Richard Biener wrote:
> > > You probably need a virtual return thunk as otherwise we expand them
> > > directly to asm?
> >
> > I was trying x86_64 -m32 -fpic regparm (3) method with thunks so that
> > the asm isn't emitted. But the thunk was still using call to .LTHUNKN
> > rather than the actual method FUNCTION_DECL. Perhaps on targets without
> > proper alias support...
> >
> > > > Would you prefer just being silent in all thunks?
> > >
> > > Yes, I think all warnings from thunks are ultimately going to be bogus...
> >
> > Ok, I'll change the patch.
Unfortunately it doesn't work, see patch below.
The cgraph code unhelpfully clears the thunk.thunk_p bit very early, in
cgraph_node::expand_thunk:
/* Since we want to emit the thunk, we explicitly mark its name as
referenced. */
thunk.thunk_p = false;
lowered = true;
bitmap_obstack_release (NULL);
and afterwards it is not possible to find out it is a thunk.
Honza, could we have some other bit which would tell us if a cgraph node is
a thunk that would stay until expansion?
> > > > That said, wonder about thunks (the non-ICF ones) from false-negative
> > > > diagnostic point as well, if I have some method with error/warning
> > > > attribute
> > > > and call a thunk instead, wonder if we get the diagnostic or not, thunks
> > > > likely don't have the attribute copied over to them.
> > >
> > > True...
> > >
> > > I guess we should not warn from thunks but instead move those attributes
> > > to the thunks so see if those get called in the end.
> >
> > Or in the expr.c code look through thunks to find the underlying function
> > and take DECL_ARGUMENTS from there.
>
> If that's easily possible that sounds good as well of course.
It isn't possible, due to the same reason as above, plus also expand_thunk
clearing thunk.thunk_p for the asm thunks too:
targetm.asm_out.output_mi_thunk (asm_out_file, thunk_fndecl,
fixed_offset, virtual_value, alias);
assemble_end_function (thunk_fndecl, fnname);
insn_locations_finalize ();
init_insn_lengths ();
free_after_compilation (cfun);
TREE_ASM_WRITTEN (thunk_fndecl) = 1;
thunk.thunk_p = false;
A testcase is like:
struct A {
int a;
__attribute__((noinline, warning ("bar1"))) virtual int
bar () { asm (""); return 1; }
};
struct B {
int a;
__attribute__((noinline, warning ("bar2"))) virtual int
bar () { asm (""); asm (""); return 2; }
};
struct C : public A, public B {
int c;
__attribute__((noinline, warning ("bar3"))) virtual int
bar () { asm (""); asm (""); asm (""); return 3; }
};
int
baz (B *b)
{
return b->bar ();
}
void
foo ()
{
B b;
baz (&b);
C c;
baz (&c);
}
where we do warning: call to ‘B::bar’ declared with attribute warning: bar2
i.e. on call _ZN1B3barEv
but not on call _ZThn16_N1C3barEv
Though, admittedly warning/error attribute on a method which often is called
only indirectly doesn't make a lot of sense, the indirect calls will not
emit any warning or error, so it is all matter of luck or lack thereof if
you get diagnostic or not (whether devirtualization manages to devirtualize
and on the other side we don't inline it). So perhaps that isn't worth the
trouble.
Here is the unsuccessful attempt.
I lean towards the the original patch unless we want to add another cgraph
bit for the thunks.
2018-03-02 Jakub Jelinek <ja...@redhat.com>
PR ipa/84628
* expr.c (expand_expr_real_1): Don't warn about calls to fndecls
with error or warning attributes from thunks.
* gcc.dg/pr84628.c: New test.
--- gcc/expr.c.jj 2018-02-09 19:11:29.094068531 +0100
+++ gcc/expr.c 2018-03-02 10:26:51.718741734 +0100
@@ -10961,14 +10961,19 @@ expand_expr_real_1 (tree exp, rtx target
error ("%Kinvalid use of %<__builtin_va_arg_pack ()%>", exp);
{
tree fndecl = get_callee_fndecl (exp), attr;
+ struct cgraph_node *node = cgraph_node::get (current_function_decl);
+ /* Don't diagnose these attributes in thunks, those are artificially
+ created. */
if (fndecl
+ && (!node || !node->thunk.thunk_p)
&& (attr = lookup_attribute ("error",
DECL_ATTRIBUTES (fndecl))) != NULL)
error ("%Kcall to %qs declared with attribute error: %s",
exp, identifier_to_locale (lang_hooks.decl_printable_name
(fndecl, 1)),
TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
if (fndecl
+ && (!node || !node->thunk.thunk_p)
&& (attr = lookup_attribute ("warning",
DECL_ATTRIBUTES (fndecl))) != NULL)
warning_at (tree_nonartificial_location (exp),
--- gcc/testsuite/gcc.dg/pr84628.c.jj 2018-03-02 10:24:08.975815667 +0100
+++ gcc/testsuite/gcc.dg/pr84628.c 2018-03-02 10:24:08.975815667 +0100
@@ -0,0 +1,8 @@
+/* PR ipa/84628 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int f0 (void);
+__attribute__((error ("err"))) void f1 (void) { f0 (); f0 (); }
+__attribute__((error ("err"))) void f2 (void) { f0 (); f0 (); }
+/* { dg-bogus "declared with attribute error" "" { target *-*-* } 0 } */
Jakub
