Skip to content

[SR-486] swiftc does not properly handle include of headers already imported from headers in other modules. #43103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
phausler opened this issue Jan 7, 2016 · 7 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler itself

Comments

@phausler
Copy link
Contributor

phausler commented Jan 7, 2016

Previous ID SR-486
Radar None
Original Reporter @phausler
Type Bug
Additional Detail from JIRA
Votes 1
Component/s Compiler
Labels Bug, ClangImporter
Assignee None
Priority Medium

md5: cc5f2e28759eb92ea98f4446682f6e9e

Issue Description:

swiftc does not properly handle include of headers already imported from headers in other modules; this is a regression in that previously it would emit a warning and now is emitting an error

x86_64/Foundation/Foundation/NSLock.swift.o.swiftdeps -module-cache-path ../Ninja-ReleaseAssert/foundation-linux-x86_64
Foundation/NSLock.swift:147:52: error: ambiguous use of 'PTHREAD_MUTEX_RECURSIVE'
pthread_mutexattr_settype(attrs, Int32(PTHREAD_MUTEX_RECURSIVE))
^
SwiftGlibc.PTHREAD_MUTEX_RECURSIVE:1:12: note: found this candidate
public var PTHREAD_MUTEX_RECURSIVE: Int { get }
^
CoreFoundation.PTHREAD_MUTEX_RECURSIVE:1:12: note: found this candidate
public var PTHREAD_MUTEX_RECURSIVE: Int { get }

@phausler
Copy link
Contributor Author

phausler commented Jan 7, 2016

Swift version 2.2-dev (LLVM 3ebdbb2c7e, Clang f66c5bb67b, Swift faba6e5)
Target: x86_64-unknown-linux-gnu
this was a working version

@belkadan
Copy link
Contributor

belkadan commented Jan 7, 2016

I can't think of anything that changed recently. @DougGregor?

(Regardless, it's not really Swift's fault. "Headers already imported from headers in other modules" means that the header belongs in its own module. A header can't belong to two modules.)

@swift-ci
Copy link
Contributor

swift-ci commented Jan 7, 2016

Comment by Mish Awadah (JIRA)

CI began hitting this at the following revisions:

Commit 1ea703c by nrotem
[Compression] Implement a new strategy for finding frequently used substrings.
The file was modified utils/name-compression/CBCGen.py

Commit f182d39 by nrotem
[Mangler] Move the body of the 'finalize' methods into the cpp files.
The file was modified include/swift/AST/Mangle.h
The file was modified lib/AST/Mangle.cpp

Commit c3c0f23 by lukeh
[SR-440] warnings cleanup

Define CFBool when MACTYPES and _OS_OSTYPES_H are defined
The file was modified CoreFoundation/RunLoop.subproj/CFRunLoop.h
The file was modified CoreFoundation/RunLoop.subproj/CFMachPort.c
The file was modified TestFoundation/TestNSString.swift
The file was modified CoreFoundation/Base.subproj/CFBase.h

@phausler
Copy link
Contributor Author

phausler commented Jan 7, 2016

The previous behavior was a warning

<unknown>:0: warning: ambiguous use of internal linkage declaration 'PTHREAD_MUTEX_RECURSIVE' defined in multiple modules
/usr/include/pthread.h:51:3: note: declared here in module 'SwiftGlibc.POSIX.pthread'
PTHREAD_MUTEX_RECURSIVE = PTHREAD_MUTEX_RECURSIVE_NP,
^
/usr/include/pthread.h:51:3: note: declared here in module 'CoreFoundation.CFBase'
PTHREAD_MUTEX_RECURSIVE = PTHREAD_MUTEX_RECURSIVE_NP,
^

@swift-ci
Copy link
Contributor

Comment by Bryan Chan (JIRA)

With a recent version of the master branch, I am seeing a similar issue when building a Swift package that imports a C library whose header includes pthread.h. Is it the same bug?

~/src/ModuleBug/SwiftLibFoo$ swift build -v
/opt/swift/bin/swiftc --driver-mode=swift -I /opt/swift/lib/swift/pm -L /opt/swift/lib/swift/pm -lPackageDescription /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Package.swift -fileno 3
/usr/bin/git clone --recursive --depth 10 /home/bryanpkc/src/ModuleBug/CLibFoo /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo
Cloning into '/home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo'...
warning: --depth is ignored in local clones; use file:// instead.
done.
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo fetch --tags origin
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo tag -l
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo tag -l
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo tag -l
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo reset --hard 1.0.0
HEAD is now at 3aa2da7 Update path to header
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo branch -m 1.0.0
Resolved version: 1.0.0
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo config --get remote.origin.url
/opt/swift/bin/swiftc --driver-mode=swift -I /opt/swift/lib/swift/pm -L /opt/swift/lib/swift/pm -lPackageDescription /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo/Package.swift -fileno 3
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo rev-parse --abbrev-ref HEAD
/usr/bin/git -C /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo-1.0.0 config --get remote.origin.url
/opt/swift/bin/swiftc --driver-mode=swift -I /opt/swift/lib/swift/pm -L /opt/swift/lib/swift/pm -lPackageDescription /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo-1.0.0/Package.swift -fileno 3
/opt/swift/bin/swift-build-tool -f /home/bryanpkc/src/ModuleBug/SwiftLibFoo/.build/debug.yaml -v
/opt/swift/bin/swiftc -module-name SwiftLibFoo -incremental -emit-dependencies -emit-module -emit-module-path /home/bryanpkc/src/ModuleBug/SwiftLibFoo/.build/debug/SwiftLibFoo.swiftmodule -output-file-map /home/bryanpkc/src/ModuleBug/SwiftLibFoo/.build/debug/SwiftLibFoo.build/output-file-map.json -num-threads 8 -c /home/bryanpkc/src/ModuleBug/SwiftLibFoo/Sources/SwiftLibFoo/main.swift -I /home/bryanpkc/src/ModuleBug/SwiftLibFoo/.build/debug -j8 -D SWIFT_PACKAGE -Onone -g -enable-testing -Xcc -fmodule-map-file=/home/bryanpkc/src/ModuleBug/SwiftLibFoo/Packages/CLibFoo-1.0.0/module.modulemap -module-cache-path /home/bryanpkc/src/ModuleBug/SwiftLibFoo/.build/debug/ModuleCache
<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "/home/bryanpkc/src/ModuleBug/libfoo/foo.h"
         ^
/home/bryanpkc/src/ModuleBug/libfoo/foo.h:5:1: error: declaration of 'pthread_mutex_t' must be imported from module 'SwiftGlibc.C.signal' before it is required
pthread_mutex_t  *getFooMutex();
^
/usr/include/x86_64-linux-gnu/bits/pthreadtypes.h:128:3: note: previous declaration is here
} pthread_mutex_t;
  ^
/home/bryanpkc/src/ModuleBug/SwiftLibFoo/Sources/SwiftLibFoo/main.swift:2:8: error: could not build Objective-C module 'CLibFoo'
import CLibFoo
       ^
<unknown>:0: error: build had 1 command failures
error: exit(1): /opt/swift/bin/swift-build-tool -f /home/bryanpkc/src/ModuleBug/SwiftLibFoo/.build/debug.yaml -v

The module map in CLibFoo looks like this:

module CLibFoo [system] {
  header "/home/bryanpkc/src/ModuleBug/libfoo/foo.h"
  link "foo"
  export *
}

The header file foo.h contains only this:

#include <pthread.h>

pthread_mutex_t  *getFooMutex();

@swift-ci
Copy link
Contributor

Comment by Bryan Chan (JIRA)

Apparently the issue I encountered has been discussed on the mailing list. Replacing <pthread.h> with <bits/pthreadtypes.h> works around the problem. However, it is not always possible to modify the header files of the C library one wishes to import into Swift.

@finagolfin
Copy link
Member

I think I've hit this long-standing issue when natively building Swift on Android now, looks to me the FreeBSD issue detailed in SR-6034 is likely the same. After drodriguez's recent pull to add more Bionic headers to the stdlib modulemap, I started getting errors about multiple declarations of fcntl:

While building module 'SwiftOverlayShims':
In file included from <module-includes>:1:
/data/data/com.termux/files/home/src/swift/build/Ninja-Release/swift-android-aarch64/./lib/swift/shims/LibcOverlayShims.h:43:10: error: conflicting types for 'fcntl'
  return fcntl(fd, cmd, value);
         ^
<module-includes>:1:10: note: in file included from <module-includes>:1:
#include "LibcOverlayShims.h"
         ^
/data/data/com.termux/files/home/src/swift/build/Ninja-Release/swift-android-aarch64/./lib/swift/shims/LibcOverlayShims.h:43:10: error: conflicting types for 'fcntl'
  return fcntl(fd, cmd, value);
         ^
/data/data/com.termux/files/usr/include/bits/fcntl.h:36:5: note: previous declaration is here
int fcntl(int __fd, int __cmd, ...);
    ^
/data/data/com.termux/files/usr/include/bits/fcntl.h:36:5: note: previous declaration is here
int fcntl(int __fd, int __cmd, ...);
    ^

Note that the Bionic fcntl is declared in the nested header bits/fcntl.h, a nested declaration similar to the FreeBSD issue mentioned above, not directly in fcntl.h as in glibc.

Looking into it further, the issue appears to be that the Termux environment modifies Bionic's syslog.h and ifaddrs.h to include unistd.h, which also includes bits/fcntl.h just like fcntl.h, and so runs into this nested header problem with the Swift clang setup.

My best guess is that the ClangImporter in the Swift compiler doesn't keep preprocessor defines from nested headers like bits/fcntl.h, so include guards in those headers are ignored and cause havoc with modulemaps, when Swift code tries to use declarations from nested headers that might be included by multiple headers listed in the modulemap. This seems to be confirmed by this patch that worked around the problem for me:

diff --git a/stdlib/public/Platform/glibc.modulemap.gyb b/stdlib/public/Platform/glibc.modulemap.gyb                                                                                              index 92a0948505..ad2f2c7b78 100644                                                              --- a/stdlib/public/Platform/glibc.modulemap.gyb                                                 +++ b/stdlib/public/Platform/glibc.modulemap.gyb
@@ -248,10 +250,12 @@ module SwiftGlibc [system] {                                                      header "${GLIBC_INCLUDE_PATH}/netdb.h"                                                           export *                                                                                       }                                                                                           +% if CMAKE_SDK != "ANDROID":                                                                         module ifaddrs {                                                                                   header "${GLIBC_INCLUDE_PATH}/ifaddrs.h"                                                         export *                                                                                       }                                                                                           +% end
module search {                                                                                    header "${GLIBC_INCLUDE_PATH}/search.h"                                                          export *
@@ -260,10 +264,12 @@ module SwiftGlibc [system] {                                                      header "${GLIBC_INCLUDE_PATH}/spawn.h"                                                           export *                                                                                       }                                                                                           +% if CMAKE_SDK != "ANDROID":                                                                         module syslog {                                                                                    header "${GLIBC_INCLUDE_PATH}/syslog.h"                                                          export *                                                                                       }                                                                                           +% end
module tar {                                                                                       header "${GLIBC_INCLUDE_PATH}/tar.h"                                                             export *                                                                                  @@ -521,6 +531,14 @@ module SwiftGlibc [system] {                                                       header "${GLIBC_INCLUDE_PATH}/unistd.h"                                                          export *                                                                                       }                                                                                           +    module ifaddrs {                                                                            +      header "${GLIBC_INCLUDE_PATH}/ifaddrs.h"                                                  +      export *                                                                                  +    }                                                                                           +    module syslog {                                                                             +      header "${GLIBC_INCLUDE_PATH}/syslog.h"                                                   +      export *                                                                                  +    }                                                                                                module utime {                                                                                     header "${GLIBC_INCLUDE_PATH}/utime.h"                                                           export *

All this patch does is reorder the headers in the libc modulemap so that both ifaddrs.h and syslog.h come after unistd.h, so that its include guard is defined by then and unistd.h is presumably ignored for those two headers after that.

I came up with that workaround before finding this Jira issue, I just tried this workaround to directly list the nested header in the modulemap and it also seems to work:

@@ -341,6 +341,14 @@ module SwiftGlibc [system] {                                                       header "${GLIBC_INCLUDE_PATH}/fcntl.h"                                                           export *                                                                                       }                                                                                           +    module bits {                                                                               +      export *                                                                                  +                                                                                                +    module fcntl {                                                                              +      header "${GLIBC_INCLUDE_PATH}/bits/fcntl.h"                                               +      export *                                                                                  +    }                                                                                           +   }                                                                                                 module fnmatch {                                                                                   header "${GLIBC_INCLUDE_PATH}/fnmatch.h"                                                         export *

I'm also able to reproduce this nested header problem on linux, say if I try to invoke a type from bits/types.h in LibcOverlayShims.h:

diff --git a/stdlib/public/SwiftShims/LibcOverlayShims.h b/stdlib/public/SwiftShims/LibcOverlayShims.h                                                                                            index a4ec14402a..fbdb498ffb 100644                                                              --- a/stdlib/public/SwiftShims/LibcOverlayShims.h                                                +++ b/stdlib/public/SwiftShims/LibcOverlayShims.h                                                @@ -40,7 +40,8 @@ typedef int mode_t;                                                             // File control <fcntl.h>                                                                        #if !defined(_WIN32) || defined(__CYGWIN__)                                                      static inline int _swift_stdlib_fcntl(int fd, int cmd, int value) {                             -  return fcntl(fd, cmd, value);                                                                 + __uint8_t foo;                                                                                 + return fcntl(fd, cmd, value);                                                                   }                                                                                                                                                                                                 static inline int _swift_stdlib_fcntlPtr(int fd, int cmd, void* ptr) {

I then get a similar error since bits/types.h is included all over the place in glibc:

<module-includes>:1:10: note: in file included from <module-includes>:1:                         #include "LibcOverlayShims.h"                                                                             ^                                                                                       /home/butta/swift/build/Ninja-Release/swift-linux-x86_64/./lib/swift/shims/LibcOverlayShims.h:43:2: error: declaration of '__uint8_t' must be imported from module 'SwiftGlibc.POSIX.termios' before it is required                                                                                   __uint8_t foo;                                                                                   ^                                                                                               //usr/include/bits/types.h:38:23: note: previous declaration is here                             typedef unsigned char __uint8_t;                                                                                       ^                                                                          /home/butta/swift/swift/stdlib/public/Platform/Platform.swift:14:8: error: could not build C module 'SwiftOverlayShims'                                                                             import SwiftOverlayShims                                                                                ^

Of course, nobody would actually directly invoke __uint8_t: this is just to demonstrate the nested header problem is there on linux too, not just Android and FreeBSD.

I was unable to follow Bryan Chan's gmane link above to a swift-devel discussion about this issue, as that gmane site seems to be down, or find that thread elsewhere: maybe this problem is well-known. If it's not going to be fixed in the compiler, it should be documented prominently when using modulemaps.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler itself
Projects
None yet
Development

No branches or pull requests

4 participants