Skip to content
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

Failed to build using -isystem #3206

Closed
pedrokiefer opened this issue Jun 17, 2017 · 8 comments
Closed

Failed to build using -isystem #3206

pedrokiefer opened this issue Jun 17, 2017 · 8 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@pedrokiefer
Copy link
Contributor

Description of the problem / feature request / question:

I know this is an issue with the toolchain, I've just filed a bug report it. But I think bazel should be able to handle this situation better.

The toolchain in question: https://developer.arm.com/open-source/gnu-toolchain/gnu-rm/downloads

This is just sample code:

cc_library(
   name = "headers",
   hdrs = [ "Inc/fileA.h", "Inc/fileB.h"],
   includes = ["Inc/"]
)

cc_binary(
   name = "bin",
   srcs = ["Src/fileA.cpp"],
   deps = [":headers"]
)

I would get a build with something like this:

external/stm32/tools/arm_compiler/arm_none_gcc/arm-none-eabi-gcc '-mcpu=cortex-m4' -mthumb '-mfloat-abi=hard' '-mfpu=fpv4-sp-d16' -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -I external/com_arm_developer_toolchain_gcc_6_2/arm-none-eabi/include/c++/6.3.1/arm-none-eabi -I external/com_arm_developer_toolchain_gcc_6_2/arm-none-eabi/include/c++/6.3.1 -I external/com_arm_developer_toolchain_gcc_6_2/arm-none-eabi/libc/usr/include '-std=c++11' -MD -MF bazel-out/clang_linux_arm-fastbuild/bin/_objs/Src/fileA.d '-frandom-seed=bazel-out/clang_linux_arm-fastbuild/bin/_objs/Src/fileA.o' -iquote .  -iquote external/bazel_tools -iquote bazel-out/clang_linux_arm-fastbuild/genfiles/external/bazel_tools -isystem Inc -no-canonical-prefixes -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' -c Src/fileA.cpp -o bazel-out/clang_linux_arm-fastbuild/bin/_objs/Src/fileA.o

The issue being the -isystem Inc. If bazel handled it as -IInc this issues doesn't happen, so for now I'm using copts = ["-IInc"], but that doesn't propagate to other libraries as includes does.

If possible, provide a minimal example to reproduce the problem:

Environment info

  • Operating System: ubuntu 16.04
  • Bazel version (output of bazel info release): release 0.5.1
@hlopko hlopko self-assigned this Jun 26, 2017
@hlopko hlopko added category: rules > C++ P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request labels Jun 26, 2017
@kersson
Copy link

kersson commented Jul 26, 2017

Yes, can someone explain the reasoning behind propagating includes as -isystem and not -I? We have a similar issue where compile warnings are being ignored in our headers because they're being treated as system headers when propagated to projects that depend on them.

@hlopko
Copy link
Member

hlopko commented Aug 17, 2017

Well the reasoning is simple, although not what you'd expect. Internally we don't use <> includes at all, and includes attribute was only introduced for the third_party code, for which we didn't want to show warnings. We will definitely improve the support for includes, but not in the short term, as we need to finish higher priority features first.

@nicolov
Copy link
Contributor

nicolov commented Aug 27, 2017

From what I can tell, using strip_include_prefix on libA adds a flag that looks like:

-Ibazel-out/local-fastbuild/bin/src/package_name/_virtual_includes/libA

And the flag is indeed passed on to dependents.

@driveraid
Copy link

driveraid commented Jun 14, 2018

We were just hit by this in our company, wasted 6 hours debugging due to a missing return statement in a header file exposed by includes, in a build with -Werror enabled. -Wsystem-headers is hopeless due to our big exposure against system and 3rdparty stuff. Any chance this could be fixed near term?

@hlopko
Copy link
Member

hlopko commented Jun 15, 2018

Unfortunately unlikely in 2018 (roadmap: https://docs.google.com/document/d/1K3Dq1JDDWjGtvTyFcIwy_-Crm7ZuD9FzSxjDawDhJRA/edit)

@jgavris
Copy link
Contributor

jgavris commented Jan 14, 2019

If you're using the Clang compiler, you can workaround this by marking certain include paths as 'not system headers'.

https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-in-system-headers

@nathaniel-brough
Copy link
Contributor

I've been working on a few work arounds for this. Though I have had little success at this point. What I have tried so far;

First Method:

The first method I tried was to string replace all -isystem to -I flags inside a wrapper script for gcc.
e.g.

#!/bin/bash
set -euo pipefail
# Workaround to replace all system includes with user includes
external/arm-none-eabi-compiler/bin/arm-none-eabi-cpp "${@//-isystem/-I}"
# Original wrapper command
#external/arm-none-eabi-compiler/bin/arm-none-eabi-cpp "$@"

However for whatever reason this results in an error about undeclared includusions:

undeclared inclusion(s) in rule '//libs/dummy:target':
this rule is missing dependency declarations for the following files included by 'libs/dummy/target.cc':
  '/home/user/.cache/bazel/_bazel_user752ded695e9654632311c0b5e834cd35/external/arm-none-eabi-compiler/lib/gcc/arm-none-eabi/8.2.1/include/stdint.h'

File list above has been truncated for brevity.

For reference here is the relevant section of my toolchain_config _impl.

_ARM_NONE_EABI_VERSION = "8.2.1"

def gcc_include_paths():
    # GCC version independant include directories
    include_path_templates = [
        "external/arm-none-eabi-compiler/arm-none-eabi/include/c++/{arm_none_eabi_gcc_ver}",
        "external/arm-none-eabi-compiler/arm-none-eabi/include/c++/{arm_none_eabi_gcc_ver}/arm-none-eabi",
        "external/arm-none-eabi-compiler/arm-none-eabi/include/c++/{arm_none_eabi_gcc_ver}/backward",
        "external/arm-none-eabi-compiler/lib/gcc/arm-none-eabi/{arm_none_eabi_gcc_ver}/include",
        "external/arm-none-eabi-compiler/lib/gcc/arm-none-eabi/{arm_none_eabi_gcc_ver}/include-fixed",
        "external/arm-none-eabi-compiler/arm-none-eabi/include",
    ]

    # Create version specific include paths
    include_paths = []
    for template in include_path_templates:
        include_paths += ["-isystem", template.format(arm_none_eabi_gcc_ver = _ARM_NONE_EABI_VERSION)]
    return include_paths

def _impl(ctx):
    tool_paths = [
        tool_path(
            name = "gcc",
            path = "gcc_wrappers/gcc",
        ),
        tool_path(
            name = "ld",
            path = "gcc_wrappers/ld",
        ),
        tool_path(
            name = "ar",
            path = "gcc_wrappers/ar",
        ),
        tool_path(
            name = "cpp",
            path = "gcc_wrappers/cpp",
        ),
       # ... Truncated for brevity
    ]

    toolchain_include_directories_feature = feature(
        name = "toolchain_include_directories",
        enabled = True,
        flag_sets = [
            flag_set(
                actions = [
                    "ACTION_NAMES.assemble",
                    "ACTION_NAMES.preprocess_assemble",
                    "ACTION_NAMES.linkstamp_compile",
                    "ACTION_NAMES.c_compile",
                    "ACTION_NAMES.cpp_compile",
                    "ACTION_NAMES.cpp_header_parsing",
                    "ACTION_NAMES.cpp_module_compile",
                    "ACTION_NAMES.cpp_module_codegen",
                    "ACTION_NAMES.lto_backend",
                    "ACTION_NAMES.clif_match",
                ],
                flag_groups = [
                    flag_group(
                        flags = gcc_include_paths(),
                    ),
                ],
            ),
        ],
    )

    toolchain = cc_common.create_cc_toolchain_config_info(
        ctx = ctx,
        toolchain_identifier = "arm-none-eabi-toolchain",
        host_system_name = "i686-unknown-linux-gnu",
        target_system_name = "arm-none-eabi",
        target_cpu = "arm-none-eabi",
        target_libc = "unknown",
        compiler = "arm-none-eabi",
        abi_version = "eabi",
        abi_libc_version = "unknown",
        tool_paths = tool_paths,
        features = [toolchain_include_directories_feature],
    )
    return toolchain

NOTE: All "missing dependency" errors are about files that have been included using the "toolchain_include_directories" feature.

I am not sure how to go about fixing these errors, but I suspect it is something to do with the {target}.d files that are generated by bazel with string replacement from -isystem >> -I.

Second Method:

The second method was to include the system headers with the -I flag directly in the bazel toolchain. In other words the include path function changes the -isystem flag to -I. This would only be a partial fix as the -isystem flag is used for all targets normally.

def gcc_include_paths():
    # GCC version independant include directories
    include_path_templates = [
        "external/arm-none-eabi-compiler/arm-none-eabi/include/c++/{arm_none_eabi_gcc_ver}",
        "external/arm-none-eabi-compiler/arm-none-eabi/include/c++/{arm_none_eabi_gcc_ver}/arm-none-eabi",
        "external/arm-none-eabi-compiler/arm-none-eabi/include/c++/{arm_none_eabi_gcc_ver}/backward",
        "external/arm-none-eabi-compiler/lib/gcc/arm-none-eabi/{arm_none_eabi_gcc_ver}/include",
        "external/arm-none-eabi-compiler/lib/gcc/arm-none-eabi/{arm_none_eabi_gcc_ver}/include-fixed",
        "external/arm-none-eabi-compiler/arm-none-eabi/include",
    ]

    # Create version specific include paths
    include_paths = []
    for template in include_path_templates:
       # Old
       # include_paths += ["-isystem ", template.format(arm_none_eabi_gcc_ver = _ARM_NONE_EABI_VERSION)]
       # New
        include_paths += ["-I ",  template.format(arm_none_eabi_gcc_ver = _ARM_NONE_EABI_VERSION)]
    return include_paths

This however resulted in the same errors as seen in the first method.

Future Approaches

  • Switch to an llvm based toolchain. This however adds complexity as the llvm toolchain for bare-metal ARM depends on gcc-arm-none-eabi headers, linkers and static libs.
  • Compile my own distribution of GCC that does not wrap c++ files in and extern C block. Doable but will take time.
  • Modify bazel to use -I instead of -isystem`. Doable but I wouldn't have any idea about where to start or what side effects it may have on the build system.

If you have any suggestions or see any workarounds based on what I have above let me know. At this point I am at a bit of a loss. I am definitely a novice user of starlark, so I'd be keen for a hand if anyone has an idea as to why I might be getting these errors.

Environment Info

Manjaro Linux (Rolling Release)
bazel info release
release 0.23.0

@hlopko hlopko assigned scentini and unassigned hlopko Mar 4, 2019
@sgowroji sgowroji added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 3, 2023
@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

9 participants