Bug 25189 - glibc's __glibc_has_include causes issues with clang -frewrite-includes
Summary: glibc's __glibc_has_include causes issues with clang -frewrite-includes
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: 2.31
Assignee: Florian Weimer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-12 18:24 UTC by Emilio Cobos Álvarez (:emilio)
Modified: 2019-11-22 12:28 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2019-11-21 00:00:00
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Emilio Cobos Álvarez (:emilio) 2019-11-12 18:24:55 UTC
Which is necessary to to e.g., submit the translation unit to a distributed compiler.

See https://bugs.llvm.org/show_bug.cgi?id=43982 for a reduced test-case since I  
initially thought this was a clang bug.                                          
                                                                                 
Apparently doing this is invalid C++ per:                                        
                                                                                 
 * http://eel.is/c++draft/cpp.cond#7.sentence-2                                  
                                                                                 
(See https://bugs.llvm.org/show_bug.cgi?id=37990)
Comment 1 Emilio Cobos Álvarez (:emilio) 2019-11-12 18:35:43 UTC
I sent a patch for this: https://sourceware.org/ml/libc-alpha/2019-11/msg00431.html

First patch, so please bear with me :)
Comment 2 Emilio Cobos Álvarez (:emilio) 2019-11-19 13:26:14 UTC
Let me know if I've missed any part of the procedure to get the patch in.

Looking at https://sourceware.org/glibc/wiki/Contribution%20checklist I think the only bits that I don't know if I followed properly are:

 * Copyright assignment stuff (I don't know what to do there, but the patch seems trivial enough to not be "legally significant").
 * Changelog (though I see a bunch of patches being posted without changelog updates).

Also I see that some changes in the mailing list are being sent via gerrit. Not sure where the docs for that are, if any, but I'm happy to give that a shot as well if needed.
Comment 3 Florian Weimer 2019-11-19 13:38:11 UTC
Sorry, you didn't do anything wrong. I looked at the patch and was just disappointed that it makes ugly source code even uglier.
Comment 4 Emilio Cobos Álvarez (:emilio) 2019-11-19 14:33:09 UTC
Yeah, I agree with that sentiment :/
Comment 5 Emilio Cobos Álvarez (:emilio) 2019-11-19 16:58:46 UTC
(though to be clear, I still think it should be landed as otherwise icecc / sccache users can't use clang, or get unscrutable build errors)
Comment 6 Sourceware Commits 2019-11-21 16:57:19 UTC
The master branch has been updated by Florian Weimer <fw@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=bfa864e1645e140da2e1aae3cf0d0ba0674f6eb5

commit bfa864e1645e140da2e1aae3cf0d0ba0674f6eb5
Author: Emilio Cobos �lvarez <emilio@crisal.io>
Date:   Tue Nov 12 19:18:32 2019 +0100

    Don't use a custom wrapper macro around __has_include (bug 25189).
    
    This causes issues when using clang with -frewrite-includes to e.g.,
    submit the translation unit to a distributed compiler.
    
    In my case, I was building Firefox using sccache.
    
    See [1] for a reduced test-case since I initially thought this was a
    clang bug, and [2] for more context.
    
    Apparently doing this is invalid C++ per [cpp.cond], which mentions [3]:
    
    > The #ifdef and #ifndef directives, and the defined conditional
    > inclusion operator, shall treat __has_include and __has_cpp_attribute
    > as if they were the names of defined macros.  The identifiers
    > __has_include and __has_cpp_attribute shall not appear in any context
    > not mentioned in this subclause.
    
    [1]: https://bugs.llvm.org/show_bug.cgi?id=43982
    [2]: https://bugs.llvm.org/show_bug.cgi?id=37990
    [3]: http://eel.is/c++draft/cpp.cond#7.sentence-2
    
    Change-Id: Id4b8ee19176a9e4624b533087ba870c418f27e60
Comment 7 Florian Weimer 2019-11-21 16:59:05 UTC
Fixed in glibc 2.31.
Comment 8 Emilio Cobos Álvarez (:emilio) 2019-11-22 11:58:44 UTC
Thanks Florian!
Comment 9 Sourceware Commits 2019-11-22 12:28:29 UTC
The release/2.30/master branch has been updated by Florian Weimer <fw@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=a4b3bbf71e8a608ac6ee4f423b5e2db50e06b846

commit a4b3bbf71e8a608ac6ee4f423b5e2db50e06b846
Author: Emilio Cobos �lvarez <emilio@crisal.io>
Date:   Tue Nov 12 19:18:32 2019 +0100

    Don't use a custom wrapper macro around __has_include (bug 25189).
    
    This causes issues when using clang with -frewrite-includes to e.g.,
    submit the translation unit to a distributed compiler.
    
    In my case, I was building Firefox using sccache.
    
    See [1] for a reduced test-case since I initially thought this was a
    clang bug, and [2] for more context.
    
    Apparently doing this is invalid C++ per [cpp.cond], which mentions [3]:
    
    > The #ifdef and #ifndef directives, and the defined conditional
    > inclusion operator, shall treat __has_include and __has_cpp_attribute
    > as if they were the names of defined macros.  The identifiers
    > __has_include and __has_cpp_attribute shall not appear in any context
    > not mentioned in this subclause.
    
    [1]: https://bugs.llvm.org/show_bug.cgi?id=43982
    [2]: https://bugs.llvm.org/show_bug.cgi?id=37990
    [3]: http://eel.is/c++draft/cpp.cond#7.sentence-2
    
    Change-Id: Id4b8ee19176a9e4624b533087ba870c418f27e60
    (cherry picked from commit bfa864e1645e140da2e1aae3cf0d0ba0674f6eb5)