181309 2018-01-04 14:54:38 -0800 Sandbox preprocessing should handle C++ comments 2018-03-31 23:03:08 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED DUPLICATE 184166 InRadar P2 Normal --- 1 bfulgham bfulgham achristensen ap bfulgham darin ggaren webkit-bug-importer oldest_to_newest 1385948 0 bfulgham 2018-01-04 14:54:38 -0800 The text preprocessing rules in WebKit's DerivedSources.make is now only used for processing our sandbox profiles. Sometimes the headers and availability macros included on macOS and iOS SDK's have spurious C++-style comments in them, which are not stripped out during preprocessing since we were telling clang to use C89 rules for preprocessing. Instead, we should switch to C99 so that C++-style comments will get properly ignored, and we won't confuse the Sandbox library when it tries the read the finished file. 1385949 1 webkit-bug-importer 2018-01-04 14:59:33 -0800 <rdar://problem/36306874> 1385950 2 330490 bfulgham 2018-01-04 15:02:03 -0800 Created attachment 330490 Patch 1385987 3 330490 ap 2018-01-04 16:16:54 -0800 Comment on attachment 330490 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330490&action=review > Source/WebKit/ChangeLog:9 > + Switch to C99 rules for preprocessing our sandbox profiles. No other code uses the TEXT_PREPROCESSOR_FLAGS Can you diff processed profiles? This seems likely to break all comments with "rdar://" in them. Theoretically, it can break actual rules if they have double slashes, but we don't have any like that now. 1385990 4 330490 bfulgham 2018-01-04 16:25:27 -0800 Comment on attachment 330490 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330490&action=review >> Source/WebKit/ChangeLog:9 >> + Switch to C99 rules for preprocessing our sandbox profiles. No other code uses the TEXT_PREPROCESSOR_FLAGS > > Can you diff processed profiles? This seems likely to break all comments with "rdar://" in them. Theoretically, it can break actual rules if they have double slashes, but we don't have any like that now. Yes -- it definitely cuts off everything after the "//" in the comment, so maybe we don't want to do that. We should probably create a WebKit-specific filter program that does exactly what we want. 1386606 5 330490 darin 2018-01-07 22:56:44 -0800 Comment on attachment 330490 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330490&action=review >>> Source/WebKit/ChangeLog:9 >>> + Switch to C99 rules for preprocessing our sandbox profiles. No other code uses the TEXT_PREPROCESSOR_FLAGS >> >> Can you diff processed profiles? This seems likely to break all comments with "rdar://" in them. Theoretically, it can break actual rules if they have double slashes, but we don't have any like that now. > > Yes -- it definitely cuts off everything after the "//" in the comment, so maybe we don't want to do that. > > We should probably create a WebKit-specific filter program that does exactly what we want. No, I don’t think this will break /* */ comments with // inside them. Those work fine in C++; why wouldn’t clang handle that properly? Actual rules with double slashes is something different, though. Maybe that’s important? 1386764 6 ap 2018-01-08 09:35:04 -0800 The comments in .sb.in files are of this form: ; "com.apple.coremedia.endpointpicker.xpc" can be removed when <rdar://problem/30081582> is resolved. And with this patch, the built .sb file has lines like this: ; "com.apple.coremedia.endpointpicker.xpc" can be removed when <rdar: I don't think that it's particularly important to preserve comments in sandbox profiles, so maybe the solution is to switch from ";" to "//" for comments. The only downside will be that comments will be inconsistent between processed and non-processed sandbox profile sources (e.g. Source/WebKit/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in vs. Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.Networking.sb). And yes, rules with double slashes are probably not important. If one is ever introduced, this will almost certainly break profile compilation, so bots will quickly tell us about the problem. 1410943 7 ap 2018-03-31 23:03:08 -0700 *** This bug has been marked as a duplicate of bug 184166 *** 330490 2018-01-04 15:02:03 -0800 2018-01-04 16:27:04 -0800 Patch bug-181309-20180104150202.patch text/plain 1638 bfulgham SW5kZXg6IFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvQ2hhbmdlTG9nCShyZXZpc2lvbiAyMjY0MjQpCisrKyBTb3VyY2UvV2ViS2l0L0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDE4LTAxLTA0ICBCcmVudCBGdWxn aGFtICA8YmZ1bGdoYW1AYXBwbGUuY29tPgorCisgICAgICAgIFNhbmRib3ggcHJlcHJvY2Vzc2lu ZyBzaG91bGQgaGFuZGxlIEMrKyBjb21tZW50cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTgxMzA5CisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8zNjMw Njg3ND4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBT d2l0Y2ggdG8gQzk5IHJ1bGVzIGZvciBwcmVwcm9jZXNzaW5nIG91ciBzYW5kYm94IHByb2ZpbGVz LiBObyBvdGhlciBjb2RlIHVzZXMgdGhlIFRFWFRfUFJFUFJPQ0VTU09SX0ZMQUdTCisgICAgICAg IG1hY3JvIGFueW1vcmUsIHNvIGdldCByaWQgb2YgZmFsbGJhY2sgY29kZSBmb3IgYnVpbGRpbmcg d2l0aCA1KyB5ZWFyLW9sZCBjbGFuZy4KKworICAgICAgICAqIERlcml2ZWRTb3VyY2VzLm1ha2U6 CisKIDIwMTgtMDEtMDQgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CiAK ICAgICAgICAgV0tXZWJWaWV3IGxvc2VzIG1pbmltdW0gbGF5b3V0IHNpemUgb3ZlcnJpZGVzIHRo YXQgaGFwcGVuIHdoaWxlIHRoZSBwcm9jZXNzIGlzIHRlcm1pbmF0ZWQKSW5kZXg6IFNvdXJjZS9X ZWJLaXQvRGVyaXZlZFNvdXJjZXMubWFrZQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L0Rl cml2ZWRTb3VyY2VzLm1ha2UJKHJldmlzaW9uIDIyNjM3MykKKysrIFNvdXJjZS9XZWJLaXQvRGVy aXZlZFNvdXJjZXMubWFrZQkod29ya2luZyBjb3B5KQpAQCAtMjEzLDE0ICsyMTMsNyBAQCBhbGwg OiBcCiAJQHB5dGhvbiAkKFdlYktpdDIpL1NjcmlwdHMvZ2VuZXJhdGUtbWVzc2FnZXMtaGVhZGVy LnB5ICQ8ID4gJEAKIAogCi0jIFNvbWUgdmVyc2lvbnMgb2YgY2xhbmcgaW5jb3JyZWN0bHkgc3Ry aXAgb3V0IC8vIGNvbW1lbnRzIGluIGM4OSBjb2RlLgotIyBVc2UgLXRyYWRpdGlvbmFsIGFzIGEg d29ya2Fyb3VuZCwgYnV0IG9ubHkgd2hlbiBuZWVkZWQgc2luY2UgdGhhdCBjYXVzZXMKLSMgb3Ro ZXIgcHJvYmxlbXMgd2l0aCBsYXRlciB2ZXJzaW9ucyBvZiBjbGFuZy4KLWlmZXEgKCQoc2hlbGwg ZWNobyAnLy94JyB8ICQoQ0MpIC1FIC1QIC14IGMgLXN0ZD1jODkgLSB8IGdyZXAgeCksKQotVEVY VF9QUkVQUk9DRVNTT1JfRkxBR1M9LUUgLVAgLXggYyAtdHJhZGl0aW9uYWwgLXcKLWVsc2UKLVRF WFRfUFJFUFJPQ0VTU09SX0ZMQUdTPS1FIC1QIC14IGMgLXN0ZD1jODkgLXcKLWVuZGlmCitURVhU X1BSRVBST0NFU1NPUl9GTEFHUz0tRSAtUCAteCBjIC1zdGQ9Yzk5IC13CiAKIGlmbmVxICgkKFNE S1JPT1QpLCkKIAlTREtfRkxBR1M9LWlzeXNyb290ICQoU0RLUk9PVCkK