search

carbon-language / carbon-lang

Carbon Language's main repository: documents, design, implementation, and related tools. (NOTE: Carbon Language is experimental; see README)
http://docs.carbon-lang.dev/
Other
32.28k stars 1.48k forks source link

Bison 3.3.2 dependency won't compile with `_FORTIFY_SOURCE=2` #2561

Closed aaronmondal closed 9 months ago

aaronmondal commented 1 year ago

Description of the bug:

I'm trying to build carbon with a more-or-less upstream Clang. The Bison dependency won't build. I'm not sure what exactly causes it, but the same toolchain is able to build Bison 3.8.2. I think something in clang has changed that makes the old Bison 3.3.2 build fail.

Edit: It's caused by glibc's _FORTIFY_SOURCE in features.h.

What did you do, or what's a simple way to reproduce the bug?

This happens with Gentoo, Kernel 6.1.8-gentoo, clang profile, LLVM toolchain from portage version 16.0.0_pre20230107. Note that this is not yet intended for general use, but I'd expect the issue to hit users as soon as LLVM 16 is released.

What did you expect to happen?

No errors.

What actually happened?

Many errors from the bison dependency, seemingly triggered by an issue with gnulib/config-linux/config.h:

ERROR: external/bison_v3.3.2/BUILD.bazel:26:11: Compiling src/derives.c [for tool] failed: (Exit 1): clang failed: error executing command (from target @bison_v3.3.2//:bison_lib) /usr/lib/llvm/16/bin/clang -no-canonical-prefixes -fcolor-diagnostics -Werror -Wall -Wextra -Wthread-safety -Wself-assign -Wimplicit-fallthrough -Wctad-maybe-unsupported -Wnon-virtual-dtor ... (remaining 44 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from external/bison_v3.3.2/src/derives.c:21:
external/bison_v3.3.2/gnulib/config-linux/config.h:238:5: error: expected parameter declarator
int obstack_printf(struct obstack *obs, const char *format, ...);
    ^
/usr/include/bits/stdio2.h:162:34: note: expanded from macro 'obstack_printf'
  __obstack_printf_chk (obstack, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__)
                                 ^
/usr/include/features.h:426:31: note: expanded from macro '__USE_FORTIFY_LEVEL'
#  define __USE_FORTIFY_LEVEL 2
                              ^
In file included from external/bison_v3.3.2/src/derives.c:21:
external/bison_v3.3.2/gnulib/config-linux/config.h:238:5: error: expected ')'
/usr/include/bits/stdio2.h:162:34: note: expanded from macro 'obstack_printf'
  __obstack_printf_chk (obstack, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__)
                                 ^
/usr/include/features.h:426:31: note: expanded from macro '__USE_FORTIFY_LEVEL'
#  define __USE_FORTIFY_LEVEL 2
                              ^
external/bison_v3.3.2/gnulib/config-linux/config.h:238:5: note: to match this '('
/usr/include/bits/stdio2.h:162:24: note: expanded from macro 'obstack_printf'
  __obstack_printf_chk (obstack, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__)
                       ^
In file included from external/bison_v3.3.2/src/derives.c:21:
external/bison_v3.3.2/gnulib/config-linux/config.h:238:5: error: conflicting types for '__obstack_printf_chk'
int obstack_printf(struct obstack *obs, const char *format, ...);
    ^
/usr/include/bits/stdio2.h:162:3: note: expanded from macro 'obstack_printf'
  __obstack_printf_chk (obstack, __USE_FORTIFY_LEVEL - 1, __VA_ARGS__)
  ^
/usr/include/bits/stdio2-decl.h:73:12: note: previous declaration is here
extern int __obstack_printf_chk (struct obstack *__restrict __obstack,
           ^
3 errors generated.

Any other information, logs, or outputs that you want to share?

ldd --version: ldd (Gentoo 2.36-r5 p5) 2.36
uname -r: 6.1.8-gentoo
aaronmondal commented 1 year ago

Note to avoid confusion: This is not related to the bundled LLVM version that carbon itself uses. The host toolchain used to build the dependencies is the issue.

aaronmondal commented 1 year ago

I wonder why I didn't see this immediately but the logs clearly show that this is an issue with __USE_FORTIFY_LEVEL from the glibc header features.h. The macro is set depending on the value of _FORTIFY_SOURCE.

Seems like Bison 3.3.2 doesn't support _FORTIFY_SOURCE=2. Setting it to 0 works but doesn't seem like a good way to solve this :disappointed:

chandlerc commented 1 year ago

I think @jonmeow was expecting still other issues with LLVM 16 and Bison, but maybe its just this issue?

I think he has tried to upgrade Bison but it looks .... difficult. Maybe he can relay current status there and what (if anything) next steps might look like.

There is also a discussion thread where Jon shared his attempt to move us to ANTLR which looks better supported but sadly had.... really challenging performance: https://github.com/carbon-language/carbon-lang/discussions/2574

Definitely something we're interested in figuring out, although I'm worried that Jon at least has already used up the time he could devote to this.

FWIW, I'm also happy for us to just force _FORTIFY_SOURCE off in our builds. We're a long way from our libraries needing to work in a broader ecosystem where this would be a problem, especially the Bison parser. And fortify provides near zero value for us in the daily development of Carbon. If that's the only issue, I'd be happy for a patch to our toolchain configuration to just override this? (If its not clear where, let me know and I can help with that part.)

jonmeow commented 1 year ago

I think @jonmeow was expecting still other issues with LLVM 16 and Bison, but maybe its just this issue?

Just this.

I think he has tried to upgrade Bison but it looks .... difficult. Maybe he can relay current status there and what (if anything) next steps might look like.

Someone just needs to get Bison 3.8.2 building with Bazel, maybe by contributing patches to https://github.com/jmillikin/rules_bison. But the files changed at least a little that it's not just "add the new version", and it needs to be verified with llvm-16 which isn't straightforward for me to do, especially given it should really be verified on linux+mac+win.

Note, an alternative is to just stop providing a hermetic bison and flex, similar to clang/llvm. But I assume @chandlerc won't like that path.

Definitely something we're interested in figuring out, although I'm worried that Jon at least has already used up the time he could devote to this.

Yes, I'm going to let other priorities rise.

aaronmondal commented 1 year ago

@jonmeow I intended to add bzlmod support to the bison/flex/m4 rules a while ago. Back then I dropped work on this for lack of time, but now I'm already familiar with the rules and how to update/modify them.

I'll try to find some time to send PRs to rules_{m4|flex|bison}. I can also test this with llvm16 on linux since this is what I'm doing all day every day anyways :smile:

What I can't do is test mac and Windows, but I'm sure once things work for linux adding the other platforms shouldn't be too hard.

I'm also all for sticking to the hermetic bundles. I feel like adapting dependencies is much less painful than having to deal with "doesn't work on my machine" type issues :relaxed:

github-actions[bot] commented 1 year ago

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time. \n\n\n This issue is labeled inactive because the last activity was over 90 days ago.

chandlerc commented 9 months ago

Closing explorer-specific issues as not-planned for now due to our decision to prioritize working on the toolchain over other implementation work in the near term: https://github.com/carbon-language/carbon-lang/blob/trunk/proposals/p3532.md