OSSM-1063: Switch to clang #154
Conversation
|
/retest |
| @@ -7,7 +7,7 @@ def envoy_copts(repository, test = False): | |||
| posix_options = [ | |||
| "-Wall", | |||
| "-Wextra", | |||
| "-Werror", | |||
| # "-Werror", FIXME: https://issues.redhat.com/browse/OSSM-1201 | |||
There was a problem hiding this comment.
We could, but I slightly prefer to keep those around, as they keep bothering us, thus motivating us to not forget about them.
| # unwinding are not affected -- the assembler part has frame unwind | ||
| # information and GCC emits it where needed (x64) or with -g (see CCDEBUG). | ||
| -CCOPT= -O2 -fomit-frame-pointer | ||
| +CCOPT= -O2 -fomit-frame-pointer -fPIC -fPIE -fplugin=annobin -fstack-protector-strong -fstack-protector-all |
There was a problem hiding this comment.
question: this looks like annobin & at least some of the hardening flags work with annobin?
There was a problem hiding this comment.
yeah, for now I'm removing everything. We can work towards re-enabling them with the possible exception of annobin.
| @@ -821,10 +821,7 @@ def _com_googlesource_chromium_v8(): | |||
| name = "com_googlesource_chromium_v8", | |||
| genrule_cmd_file = "@envoy//bazel/external:wee8.genrule_cmd", | |||
| build_file = "@envoy//bazel/external:wee8.BUILD", | |||
| patches = [ | |||
| "@envoy//bazel/external:wee8.patch", | |||
| "@envoy//bazel/external:wee8-annobin.patch", | |||
There was a problem hiding this comment.
is the plan to re-enable this patch for annobin in a follow up?
There was a problem hiding this comment.
Kind of. The plan is to investigate which options are necessary. If we figure out we don't need any of them, we should get rid of this patch permanently.
| @@ -0,0 +1,5 @@ | |||
| # FIXME: https://issues.redhat.com/browse/OSSM-1201 | |||
| build --cxxopt -w | |||
There was a problem hiding this comment.
Do we still need this? (No objections to merging as-is and mopping up later if so, just curious)
There was a problem hiding this comment.
We do. I had a hard time trying to workaround some warnings/errors (-Werror=xxx) in the proxy build. Adding back this flag (we had it in 2.1) is the way to move on with it.
| @@ -16,13 +16,12 @@ export BUILD_SCM_REVISION="Maistra PR #${PULL_NUMBER:-undefined}" | |||
| export BUILD_SCM_STATUS="SHA=${PULL_PULL_SHA:-undefined}" | |||
|
|
|||
| COMMON_FLAGS="\ | |||
| --incompatible_linkopts_to_linklibs \ | |||
| --config=clang \ | |||
There was a problem hiding this comment.
I wonder, can we do without this --config=clang?
There was a problem hiding this comment.
Apparently there's a bazel build that does not identify correctly the compiler, which makes bazel/envoy think it's gcc and then enabling gcc-specific options. Then we get lots of warnings about unknown flags in clang. This basically declares the variable BAZEL_COMPILER=clang
There was a problem hiding this comment.
Aaah I see, sounds like there's good a good reason for it to be there 👍
No description provided.