[Bug c/65238] New: [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[Bug c/65238] New: [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

            Bug ID: 65238
           Summary: [5 Regression] __has_attribute is not handled properly
                    with -traditional-cpp.
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: iains at gcc dot gnu.org

Created attachment 34890
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34890&action=edit
trivial test-case.

between r218893 and 218951 the handling of __has_attribute has regressed for
-traditional-cpp (-E)

(as an example) gcc.dg/cpp/trad/include.c has started to fail with excess
errors - if run manually:

$ ./gcc/xgcc -Bgcc /GCC/gcc-trunk/gcc/testsuite/gcc.dg/cpp/trad/include.c
-traditional-cpp  -E -m32 -o include.i
In file included from /usr/include/Availability.h:145:0,
                 from /usr/include/stdlib.h:62,
                 from
/GCC/gcc-trunk/gcc/testsuite/gcc.dg/cpp/trad/include.c:13:
/usr/include/AvailabilityInternal.h:1036:0: error: missing '(' after
"__has_attribute"
     #ifdef __has_attribute
 ^
/usr/include/AvailabilityInternal.h:1037:0: error: missing '(' after
"__has_attribute"
         #if __has_attribute(availability)
 ^
=====

The trivial test-case compiles fine with:
$ ./gcc/xgcc -Bgcc has-attr.c -E -o t.i

and with :
$ ./gcc/xgcc -Bgcc has-attr.c -E -traditional-cpp -o t.i
has-attr.c:3:0: error: missing '(' after "__has_attribute"
 # if __has_attribute(__totally_non_existant__)
 ^
cc1(52597) malloc: *** error for object 0x1421194c8: incorrect checksum for
freed object - object was probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug
cc1(52597) malloc: *** error for object 0x142119438: incorrect checksum for
freed object - object was probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug

====

Of course, it seems reasonable to exclude __has_attribute from traditional-cpp
- but, in that case:
(a) the testsuite needs amendment in some places, and
(b) use of the tag should not cause the free error.

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |mpolacek at gcc dot gnu.org

--- Comment #2 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
Let me have a look.

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

Marek Polacek <mpolacek at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW

--- Comment #3 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
This patch seems to make __has_attribute work even in traditional cpp:

--- a/libcpp/traditional.c
+++ b/libcpp/traditional.c
@@ -497,6 +497,18 @@ _cpp_scan_out_logical_line (cpp_reader *pfile, cpp_macro
*macro)
                      fmacro.line = pfile->line_table->highest_line;
                      continue;
                    }
+                 else if ((node->flags & NODE_BUILTIN) != 0
+                          && node->value.builtin == BT_HAS_ATTRIBUTE)
+                   {
+                     pfile->out.cur = out_start;
+                     /* Skip "__has_attribute".  */
+                     _cpp_lex_token (pfile);
+                     push_replacement_text (pfile, node);
+                     /* Get rid of "(arg)" in the context.  */
+                     CUR (context) = pfile->buffer->cur;
+                     lex_state = ls_none;
+                     goto new_context;
+                   }
                  else if (!recursive_macro (pfile, node))
                    {
                      /* Remove the object-like macro's name from the


However there are other issues, so I'm afraid this will need more surgery than
that, so I'm dropping this for now :(.

1) We ICE with or without the patch on following valid code:

#define D unused
#if __has_attribute(D)
int i;
#endif

2) We ICE on the following invalid code with the patch:

#if __has_attribute(
int i;
#endif

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

--- Comment #4 from Iain Sandoe <iains at gcc dot gnu.org> ---
well, I suppose, before digging deeper into fixing this, the question is
"should __has_attribute/__has_cpp_attribute be enabled for traditional"?

if not ..  (using the 'keep non-traditional cpp builtins at the end' from
init.c) then I suppose something like:

diff --git a/libcpp/init.c b/libcpp/init.c
index 45a4d13..3f6cd4d 100644
--- a/libcpp/init.c
+++ b/libcpp/init.c
@@ -455,7 +455,7 @@ cpp_init_special_builtins (cpp_reader *pfile)
   size_t n = ARRAY_SIZE (builtin_array);

   if (CPP_OPTION (pfile, traditional))
-    n -= 2;
+    n -= 4;
   else if (! CPP_OPTION (pfile, stdc_0_in_system_headers)
           || CPP_OPTION (pfile, std))
     n--;


.. would work.

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

--- Comment #5 from Marek Polacek <mpolacek at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #4)
> well, I suppose, before digging deeper into fixing this, the question is
> "should __has_attribute/__has_cpp_attribute be enabled for traditional"?

I believe it should, e.g. __has_include works in traditional cpp.

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[ktietz at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ktietz at gcc dot gnu.org

--- Comment #6 from Kai Tietz <ktietz at gcc dot gnu.org> ---
Well, the new ICE (with your patch applied) is a call off free on a
damaged/wrong pointer for macro.
For testing I disabled free for mc-pointer in question, but this leads to new
issues about too big text-buffer containing random values.  So I guess we have
here an inconsistency about traditional whitespace-handling and
none-traditional one.

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |emsr at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org,
                   |                            |jason at gcc dot gnu.org

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I guess we should start with whether we preprocessing of the
__has_cpp_attribute and __has_attribute argument is desirable or not.
I've looked at
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4200
and whether attribute-token is macro expanded or not is not really clear from
that.
Trying Marek's 1) testcase with normal -E rather than -traditional-cpp, it is
macro expanded in gcc (int i; is in the output), while it is not in clang (at
least 3.5.0).
If we don't want to expand macros, then we'd need to increment
state.prevent_expansion around the processing of the __has*_attribute built-in
macros.
If we do want to expand macros, then I'd say we most likely just don't want to
support it at all in -traditional-cpp mode (i.e. do the n -= 4 thing), because
to me expansion of the arguments looks fundamentally incompatible with the
traditional preprocessing, which works by recursively feeding the result of
macro expansion to the preprocessor again.
So, Ed/Jason, what does C++ intend here?

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 35014
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35014&action=edit
gcc5-pr65238.patch

Untested fix.

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Mon Mar 23 08:02:39 2015
New Revision: 221587

URL: https://gcc.gnu.org/viewcvs?rev=221587&root=gcc&view=rev
Log:
    PR preprocessor/65238
    * internal.h (_cpp_scan_out_logical_line): Add third argument.
    * directives.c (prepare_directive_trad): Pass false to it.
    * traditional.c (_cpp_read_logical_line_trad,
    _cpp_create_trad_definition): Likewise.
    (struct fun_macro): Add paramc field.
    (fun_like_macro): New function.
    (maybe_start_funlike): Handle NODE_BUILTIN macros.  Initialize
    macro->paramc field.
    (save_argument): Use macro->paramc instead of
    macro->node->value.macro->paramc.
    (push_replacement_text): Formatting fix.
    (recursive_macro): Use fun_like_macro helper.
    (_cpp_scan_out_logical_line): Likewise.  Add BUILTIN_MACRO_ARG
    argument.  Initialize fmacro.paramc field.  Handle builtin
    function-like macros.

    * c-c++-common/cpp/pr65238-1.c: New test.
    * gcc.dg/cpp/pr65238-2.c: New test.
    * gcc.dg/cpp/trad/pr65238-3.c: New test.
    * gcc.dg/cpp/trad/pr65238-4.c: New test.

Added:
    trunk/gcc/testsuite/c-c++-common/cpp/pr65238-1.c
    trunk/gcc/testsuite/gcc.dg/cpp/pr65238-2.c
    trunk/gcc/testsuite/gcc.dg/cpp/trad/pr65238-3.c
    trunk/gcc/testsuite/gcc.dg/cpp/trad/pr65238-4.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/libcpp/ChangeLog
    trunk/libcpp/directives.c
    trunk/libcpp/internal.h
    trunk/libcpp/traditional.c

[Bug preprocessor/65238] [5 Regression] __has_attribute is not handled properly with -traditional-cpp.

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65238

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.