Bug 18075 - [4.0 Regression] #pragma implementation broken in presence of #pragma ident
Summary: [4.0 Regression] #pragma implementation broken in presence of #pragma ident
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.0.0
: P2 critical
Target Milestone: 4.0.0
Assignee: Not yet assigned to anyone
URL:
Keywords: build, wrong-code
Depends on:
Blocks: 17265
  Show dependency treegraph
 
Reported: 2004-10-20 11:41 UTC by Eric Botcazou
Modified: 2004-10-27 17:31 UTC (History)
3 users (show)

See Also:
Host: *-*-*
Target: *-*-*
Build: *-*-*
Known to work:
Known to fail:
Last reconfirmed: 2004-10-20 12:56:10


Attachments
Testcase extracted from libgcj. (548 bytes, text/plain)
2004-10-20 11:43 UTC, Eric Botcazou
Details
pr18075.diff (1.38 KB, patch)
2004-10-27 05:47 UTC, Zack Weinberg
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Botcazou 2004-10-20 11:41:19 UTC
I get with the testcase I'm about to attach:

[eric@localhost]$ gcc-4.0 -S -Wall pragma_bug.ii
/home/eric/cvs/gcc/libjava/java/lang/natClass.cc:34: warning: ignoring #pragma
ident implementation

With 3.4.3pre, all works fine.  This is critical on Solaris because there are
#pragma ident all over the place in the header files.  This also prevents libgcj
from building on Solaris.
Comment 1 Eric Botcazou 2004-10-20 11:43:07 UTC
Created attachment 7385 [details]
Testcase extracted from libgcj.
Comment 2 Andrew Pinski 2004-10-20 12:56:08 UTC
: Search converges between 2004-09-20-161001-trunk (#551) and 2004-09-21-094824-trunk 
(#552).
Almost certain caused by:
2004-09-20  Matt Austern <austern@apple.com>
            Zack Weinberg  <zack@codesourcery.com>
        
        * decl.c (make_rtl_for_nonlocal_decl, start_preparsed_function):
        Apply lbasename to input_filename before passing to get_fileinfo.
        * semantics.c (begin_class_definition): Likewise.
        * lex.c (handle_pragma_interface): Apply get_fileinfo to the
        correct filename.  Rename variables to be less confusing.
        (handle_pragma_implementation): Likewise.  Disable "appears
        after file is included" diagnostic.
        
        * parser.c (struct cp_token): Add in_system_header fiag.
        (CP_TOKEN_BLOCK_NUM_TOKENS, struct cp_token_block)
Comment 3 Eric Botcazou 2004-10-26 07:08:14 UTC
Zack, Matt, may I ask you to take a look at this one?  This is really critical
on Solaris for the reasons I exposed in my initial filing.  And I'd like to have
the sparc*-sun-solaris2* platforms bootstrap before 4.0.0 is branched.  TIA.
Comment 4 Zack Weinberg 2004-10-27 05:46:56 UTC
Subject: Re:  [4.0 Regression] #pragma implementation broken in presence of #pragma ident

Matt Austern <austern@apple.com> writes:
> Never mind, I see that there is a test case.  Unfortunately, #pragma 
> ident is ignored on both Linux and Darwin, and, as I said, I don't have 
> access to a Solaris system.  Any suggestions about other systems where 
> #pragma ident might do something?

It turns out that #pragma ident is ignored on Solaris, too.  The bug
is actually with the logic that issues the (optional, but on with
-Wall) warning for skipping an unknown #pragma.  This logic tries to
figure out whether it's in a system header, on the usual theory that
you don't want warnings from the system headers.  I'm not exactly sure
how it's doing that, but it is confused by lex-up-front operation,
such that it thinks the last #pragma ident in the test case is _not_
in a system header.  Also, the data structures used to _print_ the
diagnostic are not valid in lex-up-front operation either, which is
why it says the nonsensical "ignoring #pragma ident implementation".

The most sensible fix seems to be to not defer unknown pragmas in the
first place.  This means cb_def_pragma will be called at a point when
the data structures it's trying to use are still valid, and everything
just works.  This also addresses Joseph's comment on the original
#pragma-handling speedup, that unknown pragmas ought to be ignored
wherever they appear.

Due to indentation changes the patch is a lot longer than it ought to
be.  The functional change is to move the 'else if (defer_pragmas)'
block inside the 'if (p)' block.

In case anyone is wondering, this should not have any noticeable
effect on -E mode; all pragmas go through cb_def_pragma in that mode,
but it's a different implementation of the callback.

(The change to cb_def_pragma in the patch below is not directly
germane to the bugfix.  I thought the code there might be, at least,
the cause of the completely bogus line number in the diagnostic.  This
proved not to be so, but what was being done was horrible and just
plain needed killin', anyway.)

Eric, can you test this patch in your Solaris setup?  I have not tried
to build libjava on our Solaris machine before, and in any case that
machine is appallingly slow.

zw

Comment 5 Zack Weinberg 2004-10-27 05:47:06 UTC
Created attachment 7415 [details]
pr18075.diff
Comment 6 Eric Botcazou 2004-10-27 09:22:38 UTC
> It turns out that #pragma ident is ignored on Solaris, too.

Yes, the bug is fully reproducible anywhere.  That's why I put *-*-* as the
triplets.

> Eric, can you test this patch in your Solaris setup?

It works just fine (libjava builds and gives decent testsuite results).

> I have not tried to build libjava on our Solaris machine before, and in any
> case that machine is appallingly slow.

You certainly meant libjava is appallingly bloated. ;-)  Yes, I confirm.

Thanks for quickly tackling the problem.
Comment 7 Zack Weinberg 2004-10-27 17:18:44 UTC
Comment on attachment 7415 [details]
pr18075.diff

fix mime type, mark as patch
Comment 8 CVS Commits 2004-10-27 17:29:56 UTC
Subject: Bug 18075

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	zack@gcc.gnu.org	2004-10-27 17:29:31

Modified files:
	libcpp         : ChangeLog directives.c 

Log message:
	PR 18075
	* directives.c (do_pragma): Do not defer pragmas which are unknown.
	(cpp_handle_deferred_pragma): Add cast to silence warning.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libcpp/ChangeLog.diff?cvsroot=gcc&r1=1.35&r2=1.36
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libcpp/directives.c.diff?cvsroot=gcc&r1=1.7&r2=1.8

Comment 9 Zack Weinberg 2004-10-27 17:31:41 UTC
fixed.