This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] PR28901 Add two levels for -Wunused-const-variable.
- From: Mark Wielaard <mjw at redhat dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: "H.J. Lu" <hjl dot tools at gmail dot com>, Jeff Law <law at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Bernd Schmidt <bschmidt at redhat dot com>, Martin Sebor <msebor at gmail dot com>, Steve Ellcey <sellcey at imgtec dot com>, Manuel López-Ibáñez <lopezibanez at gmail dot com>
- Date: Tue, 23 Feb 2016 09:53:57 +0100
- Subject: Re: [PATCH] PR28901 Add two levels for -Wunused-const-variable.
- Authentication-results: sourceware.org; auth=none
- References: <5603E4D3 dot 4050806 at redhat dot com> <1456018952-25270-1-git-send-email-mjw at redhat dot com> <56CB5A34 dot 8030301 at redhat dot com> <20160222224312 dot GK2586 at blokker dot redhat dot com> <CAMe9rOq7EMCU7gZmOFn9Np-4ujqR-w02dBhNyEyS0HCqMYBZ7Q at mail dot gmail dot com> <1456214140 dot 7770 dot 101 dot camel at redhat dot com> <20160223082652 dot GV3017 at tucnak dot redhat dot com>
On Tue, 2016-02-23 at 09:26 +0100, Jakub Jelinek wrote:
> On Tue, Feb 23, 2016 at 08:55:40AM +0100, Mark Wielaard wrote:
> > On Mon, 2016-02-22 at 19:20 -0800, H.J. Lu wrote:
> > > It caused:
> > >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69911
> > Apologies. Apparently main_input_filename can be NULL. I am not entirely
> > sure when that happens. Or how I failed to see that test failure. I
> > think I didn't have java enabled, causing libffi to be skipped.
> > I am testing this patch that skips the test in that case:
> Are you sure that is the problem?
No :) I was still bootstrapping. I did see it didn't solve the issue but
hadn't understood why yet...
> I think it doesn't hurt to check for non-NULL main_input_filename, perhaps
> some non-c-family languages might not set it, and this is in generic code,
> but at least on the gcc.target/i386/iamcu/test_passing_structs.c testcase and on one
> randomly selected libffi testcase I see the ICE from completely different
> reason - what is NULL is DECL_SOURCE_FILE (decl).
Thanks, that makes more sense than my first hypothesis.
> We are not really going to warn about this anyway, e.g. because
> it is DECL_ARTIFICIAL, but that is checked only in a much later
> condition. So, I think you should check also for NULL DECL_SOURCE_FILE
> (and treat it as possibly in a header). Unfortunately
> DECL_SOURCE_FILE involves a function call, so you might want to cache
> it in some automatic variable.
> && (warn_unused_const_variable == 2
> || (main_input_filename != NULL
> && (decl_file = DECL_SOURCE_FILE (decl)) != NULL
> && filename_cmp (main_input_filename,
> decl_file) == 0))))
That looks right. Now testing:
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 49f6c25..e9e1aab 100644
@@ -1,3 +1,10 @@
+2016-02-23 Mark Wielaard <email@example.com>
+ Jakub Jelinek <firstname.lastname@example.org>
+ PR c/69911
+ * cgraphunit.c (check_global_declaration): Check main_input_filename
+ and DECL_SOURCE_FILE are not NULL.
2016-02-20 Mark Wielaard <email@example.com>
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 27a073a..8b3fddc 100644
@@ -917,6 +917,7 @@ walk_polymorphic_call_targets (hash_set<void *> *reachable_call_targets,
check_global_declaration (symtab_node *snode)
+ const char *decl_file;
tree decl = snode->decl;
/* Warn about any function declared static but not defined. We don't
@@ -944,8 +945,10 @@ check_global_declaration (symtab_node *snode)
|| (((warn_unused_variable && ! TREE_READONLY (decl))
|| (warn_unused_const_variable > 0 && TREE_READONLY (decl)
&& (warn_unused_const_variable == 2
- || filename_cmp (main_input_filename,
- DECL_SOURCE_FILE (decl)) == 0)))
+ || (main_input_filename != NULL
+ && (decl_file = DECL_SOURCE_FILE (decl)) != NULL
+ && filename_cmp (main_input_filename,
+ decl_file) == 0))))
&& TREE_CODE (decl) == VAR_DECL))
&& ! DECL_IN_SYSTEM_HEADER (decl)
&& ! snode->referred_to_p (/*include_self=*/false)