Enable -fcomplete-member-pointers flag globally |
|
Issue descriptionThis flag was added in LLVM r333498 and it prevents member pointers of incomplete base type from being used in cases where they might cause problems under the Microsoft ABI. It also enables additional semantic analysis that we'll need in order to correctly implement -fsanitize=cfi for member function pointer calls. I kicked off a build of Chrome for Linux with this flag enabled. So far I've only seen error in one place in swiftshader, and that seems easy to clean up. I also kicked off a Windows cross build, and we'll see how that goes. So it should hopefully be pretty easy to enable globally once we roll clang past the change that added the flag.
,
May 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/088233a24965efea5a225c2155200248a328ded8 commit 088233a24965efea5a225c2155200248a328ded8 Author: Peter Collingbourne <pcc@chromium.org> Date: Thu May 31 00:09:52 2018 Move a template specialization for a member pointer after the declaration of the base class. This allows compilers targeting the MS ABI to select the correct inheritance model for the member function pointer type RunnerMethodType using the complete type of TaskHandle::Runner. It will allow us to enable the new Clang flag -fcomplete-member-pointers globally. Bug: 847724 Change-Id: Ib716e3c0774b7d6b46087e46057ca249e9aa72e7 Reviewed-on: https://chromium-review.googlesource.com/1079852 Reviewed-by: Nico Weber <thakis@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#563078} [modify] https://crrev.com/088233a24965efea5a225c2155200248a328ded8/third_party/blink/renderer/platform/web_task_runner.cc
,
Jun 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee76bddf0afa55b36d9b8fc79d6b5e698e704d7e commit ee76bddf0afa55b36d9b8fc79d6b5e698e704d7e Author: Nicolas Capens <capn@chromium.org> Date: Fri Jun 01 17:37:12 2018 Roll SwiftShader 4b74373..419e8a7 https://swiftshader.googlesource.com/SwiftShader.git/+log/4b74373..419e8a7 BUG=chromium:847724 TBR=kbr@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_chromium_cfi_rel_ng;luci.chromium.try:android_optional_gpu_tests_rel Change-Id: I22ac7195358bab6470e2179873747094fd287743 Reviewed-on: https://chromium-review.googlesource.com/1082600 Reviewed-by: Alexis Hétu <sugoi@chromium.org> Commit-Queue: Nicolas Capens <capn@chromium.org> Cr-Commit-Position: refs/heads/master@{#563705} [modify] https://crrev.com/ee76bddf0afa55b36d9b8fc79d6b5e698e704d7e/DEPS
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3105b4b4381a42d4b426e46a0ccd901582b0f921 commit 3105b4b4381a42d4b426e46a0ccd901582b0f921 Author: Peter Collingbourne <pcc@chromium.org> Date: Wed Jun 13 20:04:56 2018 gn: Swap the order of two declarations. This allows compilers targeting the MS ABI to select the correct inheritance model for the member function pointer types PrefixFunc and InfixFunc using the complete type of Parser. It will allow us to enable the new Clang flag -fcomplete-member-pointers globally. Bug: 847724 Change-Id: I5b3488ce0b64b3ac4884bc499cd355e1651d12b3 Reviewed-on: https://chromium-review.googlesource.com/1098515 Commit-Queue: Peter Collingbourne <pcc@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Cr-Commit-Position: refs/heads/master@{#566964} [modify] https://crrev.com/3105b4b4381a42d4b426e46a0ccd901582b0f921/tools/gn/parser.h
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fea64fd2f17f40d572923de0705def0016ad4a9f commit fea64fd2f17f40d572923de0705def0016ad4a9f Author: Peter Collingbourne <pcc@chromium.org> Date: Wed Jun 13 22:17:49 2018 zucchini: Swap the order of two declarations. This allows compilers targeting the MS ABI to select the correct inheritance model for the member function pointer types ReaderFactory and WriterFactory using the complete type of Disassembler. It will allow us to enable the new Clang flag -fcomplete-member-pointers globally. Bug: 847724 Change-Id: Iead5fd97da5f67c336d8904a7a305a34df37065b Reviewed-on: https://chromium-review.googlesource.com/1098470 Commit-Queue: Peter Collingbourne <pcc@chromium.org> Reviewed-by: Samuel Huang <huangs@chromium.org> Cr-Commit-Position: refs/heads/master@{#567016} [modify] https://crrev.com/fea64fd2f17f40d572923de0705def0016ad4a9f/components/zucchini/disassembler.h
,
Jun 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a2a5c4aa89b2718f60d775896784a96068fcacf commit 4a2a5c4aa89b2718f60d775896784a96068fcacf Author: Peter Collingbourne <pcc@chromium.org> Date: Thu Jun 14 01:47:20 2018 build: Pass -fcomplete-member-pointers when building with clang. This prevents member pointers of incomplete base type from being used in cases where they might cause problems under the Microsoft ABI. Specifically, the Microsoft ABI has different kinds of member pointers with different sizes, and the choice of member pointer representation depends on the inheritance hierarchy of the member pointer's base type. C++ allows a member pointer's base type to be incomplete, so if it is incomplete at the point where a variable of that member pointer type is declared, that forces the compiler to pick the most general (i.e. largest) one. That can lead to ODR violations since the most general representation wouldn't necessarily be the one that would be chosen if the base type happened to be complete at the point where the variable was declared. It can also be less size efficient because the compiler will generally be able to choose a smaller representation than the most general one if it were complete at the point where it is needed. This flag also enables additional semantic analysis that we'll need in order to correctly implement -fsanitize=cfi for member function pointer calls. This is because the inheritance hierarchy of the base type must be available in order to make the CFI checks as precise as possible. Note that the flag is a -f flag rather than a -W flag. This is because requiring member pointer base types to be complete is technically a non-conforming language extension, as it may, for example, cause templates to be instantiated which would otherwise not be, which may be observable after code generation in conforming programs that were crafted to observe it. However, the effects of this language extension should not be observable in most ordinary programs. Bug: 847724 Change-Id: I8d823fd4a6f21dfcadba55eefc0a69ef2e0c3479 Reviewed-on: https://chromium-review.googlesource.com/1098217 Commit-Queue: Peter Collingbourne <pcc@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#567086} [modify] https://crrev.com/4a2a5c4aa89b2718f60d775896784a96068fcacf/build/config/compiler/BUILD.gn
,
Jun 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64397686ae3dd0b0eee8b7990e368d6b8a0c5421 commit 64397686ae3dd0b0eee8b7990e368d6b8a0c5421 Author: John Budorick <jbudorick@chromium.org> Date: Thu Jun 14 02:19:05 2018 Revert "build: Pass -fcomplete-member-pointers when building with clang." This reverts commit 4a2a5c4aa89b2718f60d775896784a96068fcacf. Reason for revert: seems to be a bit too new of a flag for the xcode version of clang: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/ios-simulator-xcode-clang/51865 Original change's description: > build: Pass -fcomplete-member-pointers when building with clang. > > This prevents member pointers of incomplete base type from being used in > cases where they might cause problems under the Microsoft ABI. > > Specifically, the Microsoft ABI has different kinds of member pointers with > different sizes, and the choice of member pointer representation depends on > the inheritance hierarchy of the member pointer's base type. C++ allows a > member pointer's base type to be incomplete, so if it is incomplete at the > point where a variable of that member pointer type is declared, that forces > the compiler to pick the most general (i.e. largest) one. That can lead to > ODR violations since the most general representation wouldn't necessarily > be the one that would be chosen if the base type happened to be complete at > the point where the variable was declared. It can also be less size efficient > because the compiler will generally be able to choose a smaller representation > than the most general one if it were complete at the point where it is needed. > > This flag also enables additional semantic analysis that we'll need in order > to correctly implement -fsanitize=cfi for member function pointer calls. This > is because the inheritance hierarchy of the base type must be available in > order to make the CFI checks as precise as possible. > > Note that the flag is a -f flag rather than a -W flag. This is because > requiring member pointer base types to be complete is technically a > non-conforming language extension, as it may, for example, cause templates > to be instantiated which would otherwise not be, which may be observable > after code generation in conforming programs that were crafted to observe > it. However, the effects of this language extension should not be observable > in most ordinary programs. > > Bug: 847724 > Change-Id: I8d823fd4a6f21dfcadba55eefc0a69ef2e0c3479 > Reviewed-on: https://chromium-review.googlesource.com/1098217 > Commit-Queue: Peter Collingbourne <pcc@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#567086} TBR=thakis@chromium.org,pcc@chromium.org Change-Id: Ieb56b1bb4236a874ecc4846eb3ad73a87d9bdf82 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 847724 Reviewed-on: https://chromium-review.googlesource.com/1100516 Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: John Budorick <jbudorick@chromium.org> Cr-Commit-Position: refs/heads/master@{#567094} [modify] https://crrev.com/64397686ae3dd0b0eee8b7990e368d6b8a0c5421/build/config/compiler/BUILD.gn
,
Jun 14 2018
The following revision refers to this bug: https://pdfium.googlesource.com/pdfium/+/bb3d6313f278feee76f5fba17feff3aee65fb928 commit bb3d6313f278feee76f5fba17feff3aee65fb928 Author: Peter Collingbourne <pcc@google.com> Date: Thu Jun 14 18:56:08 2018 Move definition of XFA_SCRIPTATTRIBUTEINFO into cjx_object.h. This allows compilers targeting the MS ABI to select the correct inheritance model for the member function pointer type XFA_ATTRIBUTE_CALLBACK using the complete type of CJX_Object. It will allow us to enable the new Clang flag -fcomplete-member-pointers globally. Bug: chromium:847724 Change-Id: I90cedde8c5355e5eb896a93f0e43e6a1e1d09dbc Reviewed-on: https://pdfium-review.googlesource.com/35190 Commit-Queue: Tom Sepez <tsepez@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> [modify] https://crrev.com/bb3d6313f278feee76f5fba17feff3aee65fb928/fxjs/xfa/cjx_object.h [modify] https://crrev.com/bb3d6313f278feee76f5fba17feff3aee65fb928/xfa/fxfa/fxfa_basic.h
,
Jun 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5da61dbe887e02c83b99f8ba72e85087f205a397 commit 5da61dbe887e02c83b99f8ba72e85087f205a397 Author: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Date: Fri Jun 15 00:47:33 2018 Roll src/third_party/pdfium e16ffd4fc3f2..ee3e3a4f3cba (4 commits) https://pdfium.googlesource.com/pdfium.git/+log/e16ffd4fc3f2..ee3e3a4f3cba git log e16ffd4fc3f2..ee3e3a4f3cba --date=short --no-merges --format='%ad %ae %s' 2018-06-14 npm@chromium.org Merge 3 methods of CJBig2_GRDProc together 2018-06-14 dsinclair@chromium.org [formcalc] Calculate length of string when calling FXSYS_wcstof 2018-06-14 pcc@google.com Move definition of XFA_SCRIPTATTRIBUTEINFO into cjx_object.h. 2018-06-14 hnakashima@chromium.org Rewrite content stream regeneration. Created with: gclient setdep -r src/third_party/pdfium@ee3e3a4f3cba The AutoRoll server is located here: https://pdfium-roll.skia.org Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG= chromium:846104 ,chromium:847724 TBR=dsinclair@chromium.org Change-Id: Ib391fca7b13757f3b2dc733c4d20fd67d42d1d96 Reviewed-on: https://chromium-review.googlesource.com/1101720 Reviewed-by: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Commit-Queue: pdfium-chromium-autoroll <pdfium-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#567501} [modify] https://crrev.com/5da61dbe887e02c83b99f8ba72e85087f205a397/DEPS
,
Jun 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10bcc4d1db5c4b506aff7fa9f5e9e0d8226c84c6 commit 10bcc4d1db5c4b506aff7fa9f5e9e0d8226c84c6 Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Jun 15 00:52:04 2018 Reland "build: Pass -fcomplete-member-pointers when building with clang." This is a reland of 4a2a5c4aa89b2718f60d775896784a96068fcacf with an additional check for use_xcode_clang. Original change's description: > build: Pass -fcomplete-member-pointers when building with clang. > > This prevents member pointers of incomplete base type from being used in > cases where they might cause problems under the Microsoft ABI. > > Specifically, the Microsoft ABI has different kinds of member pointers with > different sizes, and the choice of member pointer representation depends on > the inheritance hierarchy of the member pointer's base type. C++ allows a > member pointer's base type to be incomplete, so if it is incomplete at the > point where a variable of that member pointer type is declared, that forces > the compiler to pick the most general (i.e. largest) one. That can lead to > ODR violations since the most general representation wouldn't necessarily > be the one that would be chosen if the base type happened to be complete at > the point where the variable was declared. It can also be less size efficient > because the compiler will generally be able to choose a smaller representation > than the most general one if it were complete at the point where it is needed. > > This flag also enables additional semantic analysis that we'll need in order > to correctly implement -fsanitize=cfi for member function pointer calls. This > is because the inheritance hierarchy of the base type must be available in > order to make the CFI checks as precise as possible. > > Note that the flag is a -f flag rather than a -W flag. This is because > requiring member pointer base types to be complete is technically a > non-conforming language extension, as it may, for example, cause templates > to be instantiated which would otherwise not be, which may be observable > after code generation in conforming programs that were crafted to observe > it. However, the effects of this language extension should not be observable > in most ordinary programs. > > Bug: 847724 > Change-Id: I8d823fd4a6f21dfcadba55eefc0a69ef2e0c3479 > Reviewed-on: https://chromium-review.googlesource.com/1098217 > Commit-Queue: Peter Collingbourne <pcc@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#567086} Bug: 847724 Change-Id: I1b97980691914492945d170931d33438c68e8d0b Reviewed-on: https://chromium-review.googlesource.com/1101477 Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#567505} [modify] https://crrev.com/10bcc4d1db5c4b506aff7fa9f5e9e0d8226c84c6/build/config/compiler/BUILD.gn
,
Jun 15 2018
Is anything required to update development environments to use this flag? After a git pull today, I get many errors like the following: clang: error: unknown argument: '-fcomplete-member-pointers'
,
Jun 15 2018
Did you run `gclient sync`? What's `third_party/llvm-build/Release+Asserts/bin/clang --version` for you?
,
Jun 28 2018
As mentioned here [https://groups.google.com/forum/#!topic/discuss-webrtc/-8eyqp6qw0A] that flag causes compiler failure on Linux if use_custom_libcxx is set to false to use the host's libstdc++. It's failing with the current latest libstdc++ 6.0.25. Appending another conditional to check for use_custom_libcxx fixes it.
,
Jun 28 2018
Are you synced past https://chromium-review.googlesource.com/c/chromium/src/+/1114848 ? If not, that'll fix it.
,
Jun 28 2018
I'm building webrtc and it doesn't look it picked that up just yet... But applying that diff manually does fix it in the interim. Thanks. |
|
►
Sign in to add a comment |
|
Comment 1 by bugdroid1@chromium.org
, May 30 2018