26997 2009-07-06 13:28:48 -0700 [Chromium] Remove buggy usage of DEFINE_STATIC_LOCAL in V8Binding.cpp 2009-07-10 13:30:25 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED FIXED P2 Critical --- 0 japhet webkit-unassigned darin dglazkov eric oldest_to_newest 129986 0 japhet 2009-07-06 13:28:48 -0700 ....this usage of DEFINE_STATIC_LOCAL is causing a bunch of layout test failures for Chromium. 129988 1 32313 japhet 2009-07-06 13:31:44 -0700 Created attachment 32313 patch 129998 2 32313 dglazkov 2009-07-06 13:46:56 -0700 Comment on attachment 32313 patch ok. 130038 3 eric 2009-07-06 15:34:01 -0700 Why is this bad usage? 130039 4 eric 2009-07-06 15:34:24 -0700 CCing Darin who knows more about this macro than I do. 130040 5 japhet 2009-07-06 15:39:34 -0700 To be honest, I'm not 100% sure what I did wrong here. All I know is that I clearly didn't initialize it correctly, because the test shell was consistently crashing when lowNumbers[foo] was used a couple lines below the DEFINE_STATIC_LOCAL(). I probably should have included a FIXME to use DEFINE_STATIC_LOCAL properly instead of the traditional static variable declaration. I just wanted to get it working so that we could finish upstreaming V8Binding and get the chromium canary builder green again. However, it's looking like I'm going to be rolling this change and the one that introduced the bug back due to the chromium build mess and the fact that I won't be able to clean up after myself tonight. If someone can help me figure out how to correctly use DEFINE_STATIC_LOCAL in this situation, I'll happily do it right when I re-upstream V8Binding.cpp tomorrow. 130044 6 eric 2009-07-06 15:51:28 -0700 From StdLibExtras.h: // Use these to declare and define a static local variable (static T;) so that // it is leaked so that its destructors are not called at exit. Using this // macro also allows workarounds a compiler bug present in Apple's version of GCC 4.0.1. #if COMPILER(GCC) && defined(__APPLE_CC__) && __GNUC__ == 4 && __GNUC_MINOR__ == 0 && __GNUC_PATCHLEVEL__ == 1 #define DEFINE_STATIC_LOCAL(type, name, arguments) \ static type* name##Ptr = new type arguments; \ type& name = *name##Ptr #else #define DEFINE_STATIC_LOCAL(type, name, arguments) \ static type& name = *new type arguments #endif These exist only to work around bugs in GCC 4.0.1 on Macs. I don't know enough about the bug to comment further however. 130928 7 dglazkov 2009-07-10 13:22:29 -0700 Has this landed? 130933 8 japhet 2009-07-10 13:30:25 -0700 This has landed, but I left it open since I wasn't sure whether the conversation was going to continue regarding my original usage of DEFINE_STATIC_LOCAL. The current version of the file in the repository does not use DEFINE_STATIC_LOCAL. If it should, I'm happy to take care of it, but I guess I'll close this issue out in the meantime. 32313 2009-07-06 13:31:44 -0700 2009-07-06 13:46:56 -0700 patch patch.txt text/plain 1519 japhet SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NTU2MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTQgQEAKKzIwMDktMDctMDYgIE5hdGUgQ2hhcGluICA8amFwaGV0QGNocm9taXVt Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBG aXggYSBidW5jaCBvZiBsYXlvdXQgdGVzdCBjcmFoc2VzIGluIENocm9taXVtIGNhdXNlZCBieSBh IGJhZCB1c2FnZSBvZiBERUZJTkVfU1RBVElDX0xPQ0FMLgorCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yNjk5NworCisgICAgICAgICogYmluZGluZ3Mv djgvVjhCaW5kaW5nLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OnY4VmFsdWVUb1dlYkNvcmVTdHJp bmcpOiBVc2UgYSByZWd1bGFyIHN0YXRpYyBkZWNsYXJhdGlvbiBpbnN0ZWFkIG9mIERFRklORV9T VEFUSUNfTE9DQUwuCisKIDIwMDktMDctMDYgIE5hdGUgQ2hhcGluICA8amFwaGV0QGNocm9taXVt Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXZpZCBMZXZpbi4KSW5kZXg6IFdlYkNvcmUv YmluZGluZ3MvdjgvVjhCaW5kaW5nLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2JpbmRpbmdz L3Y4L1Y4QmluZGluZy5jcHAJKHJldmlzaW9uIDQ1NTYwKQorKysgV2ViQ29yZS9iaW5kaW5ncy92 OC9WOEJpbmRpbmcuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zOCw3ICszOCw2IEBACiAjaW5jbHVk ZSAiU3RyaW5nQnVmZmVyLmgiCiAKICNpbmNsdWRlIDx2OC5oPgotI2luY2x1ZGUgPHd0Zi9TdGRM aWJFeHRyYXMuaD4KIAogbmFtZXNwYWNlIFdlYkNvcmUgewogCkBAIC0xMDIsNyArMTAxLDcgQEAg U3RyaW5nIHY4VmFsdWVUb1dlYkNvcmVTdHJpbmcodjg6OkhhbmRsZQogICAgICAgICBpbnQgdmFs dWUgPSBvYmplY3QtPkludDMyVmFsdWUoKTsKICAgICAgICAgLy8gTW9zdCBudW1iZXJzIHVzZWQg YXJlIDw9IDEwMC4gRXZlbiBpZiB0aGV5IGFyZW4ndCB1c2VkIHRoZXJlJ3MgdmVyeSBsaXR0bGUg aW4gdXNpbmcgdGhlIHNwYWNlLgogICAgICAgICBjb25zdCBpbnQga0xvd051bWJlcnMgPSAxMDA7 Ci0gICAgICAgIERFRklORV9TVEFUSUNfTE9DQUwoQXRvbWljU3RyaW5nKiwgbG93TnVtYmVycywg W2tMb3dOdW1iZXJzICsgMV0pOworICAgICAgICBzdGF0aWMgQXRvbWljU3RyaW5nIGxvd051bWJl cnNba0xvd051bWJlcnMgKyAxXTsKICAgICAgICAgU3RyaW5nIHdlYkNvcmVTdHJpbmc7CiAgICAg ICAgIGlmICgwIDw9IHZhbHVlICYmIHZhbHVlIDw9IGtMb3dOdW1iZXJzKSB7CiAgICAgICAgICAg ICB3ZWJDb3JlU3RyaW5nID0gbG93TnVtYmVyc1t2YWx1ZV07Cg==